xenbus_client.c contains some functions specific for pv guests.
Enclose them with #ifdef CONFIG_XEN_PV to avoid compiling them when
they are not needed (e.g. on ARM).
Signed-off-by: Juergen Gross <[email protected]>
---
drivers/xen/xenbus/xenbus_client.c | 130 +++++++++++++++++++------------------
1 file changed, 67 insertions(+), 63 deletions(-)
diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c
index 82a8866758ee..a1c17000129b 100644
--- a/drivers/xen/xenbus/xenbus_client.c
+++ b/drivers/xen/xenbus/xenbus_client.c
@@ -519,64 +519,6 @@ static int __xenbus_map_ring(struct xenbus_device *dev,
return err;
}
-static int xenbus_map_ring_valloc_pv(struct xenbus_device *dev,
- grant_ref_t *gnt_refs,
- unsigned int nr_grefs,
- void **vaddr)
-{
- struct xenbus_map_node *node;
- struct vm_struct *area;
- pte_t *ptes[XENBUS_MAX_RING_GRANTS];
- phys_addr_t phys_addrs[XENBUS_MAX_RING_GRANTS];
- int err = GNTST_okay;
- int i;
- bool leaked;
-
- *vaddr = NULL;
-
- if (nr_grefs > XENBUS_MAX_RING_GRANTS)
- return -EINVAL;
-
- node = kzalloc(sizeof(*node), GFP_KERNEL);
- if (!node)
- return -ENOMEM;
-
- area = alloc_vm_area(XEN_PAGE_SIZE * nr_grefs, ptes);
- if (!area) {
- kfree(node);
- return -ENOMEM;
- }
-
- for (i = 0; i < nr_grefs; i++)
- phys_addrs[i] = arbitrary_virt_to_machine(ptes[i]).maddr;
-
- err = __xenbus_map_ring(dev, gnt_refs, nr_grefs, node->handles,
- phys_addrs,
- GNTMAP_host_map | GNTMAP_contains_pte,
- &leaked);
- if (err)
- goto failed;
-
- node->nr_handles = nr_grefs;
- node->pv.area = area;
-
- spin_lock(&xenbus_valloc_lock);
- list_add(&node->next, &xenbus_valloc_pages);
- spin_unlock(&xenbus_valloc_lock);
-
- *vaddr = area->addr;
- return 0;
-
-failed:
- if (!leaked)
- free_vm_area(area);
- else
- pr_alert("leaking VM area %p size %u page(s)", area, nr_grefs);
-
- kfree(node);
- return err;
-}
-
struct map_ring_valloc_hvm
{
unsigned int idx;
@@ -725,6 +667,65 @@ int xenbus_unmap_ring_vfree(struct xenbus_device *dev, void *vaddr)
}
EXPORT_SYMBOL_GPL(xenbus_unmap_ring_vfree);
+#ifdef CONFIG_XEN_PV
+static int xenbus_map_ring_valloc_pv(struct xenbus_device *dev,
+ grant_ref_t *gnt_refs,
+ unsigned int nr_grefs,
+ void **vaddr)
+{
+ struct xenbus_map_node *node;
+ struct vm_struct *area;
+ pte_t *ptes[XENBUS_MAX_RING_GRANTS];
+ phys_addr_t phys_addrs[XENBUS_MAX_RING_GRANTS];
+ int err = GNTST_okay;
+ int i;
+ bool leaked;
+
+ *vaddr = NULL;
+
+ if (nr_grefs > XENBUS_MAX_RING_GRANTS)
+ return -EINVAL;
+
+ node = kzalloc(sizeof(*node), GFP_KERNEL);
+ if (!node)
+ return -ENOMEM;
+
+ area = alloc_vm_area(XEN_PAGE_SIZE * nr_grefs, ptes);
+ if (!area) {
+ kfree(node);
+ return -ENOMEM;
+ }
+
+ for (i = 0; i < nr_grefs; i++)
+ phys_addrs[i] = arbitrary_virt_to_machine(ptes[i]).maddr;
+
+ err = __xenbus_map_ring(dev, gnt_refs, nr_grefs, node->handles,
+ phys_addrs,
+ GNTMAP_host_map | GNTMAP_contains_pte,
+ &leaked);
+ if (err)
+ goto failed;
+
+ node->nr_handles = nr_grefs;
+ node->pv.area = area;
+
+ spin_lock(&xenbus_valloc_lock);
+ list_add(&node->next, &xenbus_valloc_pages);
+ spin_unlock(&xenbus_valloc_lock);
+
+ *vaddr = area->addr;
+ return 0;
+
+failed:
+ if (!leaked)
+ free_vm_area(area);
+ else
+ pr_alert("leaking VM area %p size %u page(s)", area, nr_grefs);
+
+ kfree(node);
+ return err;
+}
+
static int xenbus_unmap_ring_vfree_pv(struct xenbus_device *dev, void *vaddr)
{
struct xenbus_map_node *node;
@@ -788,6 +789,12 @@ static int xenbus_unmap_ring_vfree_pv(struct xenbus_device *dev, void *vaddr)
return err;
}
+static const struct xenbus_ring_ops ring_ops_pv = {
+ .map = xenbus_map_ring_valloc_pv,
+ .unmap = xenbus_unmap_ring_vfree_pv,
+};
+#endif
+
struct unmap_ring_vfree_hvm
{
unsigned int idx;
@@ -916,11 +923,6 @@ enum xenbus_state xenbus_read_driver_state(const char *path)
}
EXPORT_SYMBOL_GPL(xenbus_read_driver_state);
-static const struct xenbus_ring_ops ring_ops_pv = {
- .map = xenbus_map_ring_valloc_pv,
- .unmap = xenbus_unmap_ring_vfree_pv,
-};
-
static const struct xenbus_ring_ops ring_ops_hvm = {
.map = xenbus_map_ring_valloc_hvm,
.unmap = xenbus_unmap_ring_vfree_hvm,
@@ -928,8 +930,10 @@ static const struct xenbus_ring_ops ring_ops_hvm = {
void __init xenbus_ring_ops_init(void)
{
+#ifdef CONFIG_XEN_PV
if (!xen_feature(XENFEAT_auto_translated_physmap))
ring_ops = &ring_ops_pv;
else
+#endif
ring_ops = &ring_ops_hvm;
}
--
2.12.3
On 09/14/2017 08:38 AM, Juergen Gross wrote:
> xenbus_client.c contains some functions specific for pv guests.
> Enclose them with #ifdef CONFIG_XEN_PV to avoid compiling them when
> they are not needed (e.g. on ARM).
>
> Signed-off-by: Juergen Gross <[email protected]>
> ---
> drivers/xen/xenbus/xenbus_client.c | 130 +++++++++++++++++++------------------
> 1 file changed, 67 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c
> index 82a8866758ee..a1c17000129b 100644
> --- a/drivers/xen/xenbus/xenbus_client.c
> +++ b/drivers/xen/xenbus/xenbus_client.c
> @@ -519,64 +519,6 @@ static int __xenbus_map_ring(struct xenbus_device *dev,
> return err;
> }
>
> -static int xenbus_map_ring_valloc_pv(struct xenbus_device *dev,
> - grant_ref_t *gnt_refs,
> - unsigned int nr_grefs,
> - void **vaddr)
> -{
> - struct xenbus_map_node *node;
> - struct vm_struct *area;
> - pte_t *ptes[XENBUS_MAX_RING_GRANTS];
> - phys_addr_t phys_addrs[XENBUS_MAX_RING_GRANTS];
> - int err = GNTST_okay;
> - int i;
> - bool leaked;
> -
> - *vaddr = NULL;
> -
> - if (nr_grefs > XENBUS_MAX_RING_GRANTS)
> - return -EINVAL;
> -
> - node = kzalloc(sizeof(*node), GFP_KERNEL);
> - if (!node)
> - return -ENOMEM;
> -
> - area = alloc_vm_area(XEN_PAGE_SIZE * nr_grefs, ptes);
> - if (!area) {
> - kfree(node);
> - return -ENOMEM;
> - }
> -
> - for (i = 0; i < nr_grefs; i++)
> - phys_addrs[i] = arbitrary_virt_to_machine(ptes[i]).maddr;
> -
> - err = __xenbus_map_ring(dev, gnt_refs, nr_grefs, node->handles,
> - phys_addrs,
> - GNTMAP_host_map | GNTMAP_contains_pte,
> - &leaked);
> - if (err)
> - goto failed;
> -
> - node->nr_handles = nr_grefs;
> - node->pv.area = area;
> -
> - spin_lock(&xenbus_valloc_lock);
> - list_add(&node->next, &xenbus_valloc_pages);
> - spin_unlock(&xenbus_valloc_lock);
> -
> - *vaddr = area->addr;
> - return 0;
> -
> -failed:
> - if (!leaked)
> - free_vm_area(area);
> - else
> - pr_alert("leaking VM area %p size %u page(s)", area, nr_grefs);
> -
> - kfree(node);
> - return err;
> -}
> -
> struct map_ring_valloc_hvm
> {
> unsigned int idx;
> @@ -725,6 +667,65 @@ int xenbus_unmap_ring_vfree(struct xenbus_device *dev, void *vaddr)
> }
> EXPORT_SYMBOL_GPL(xenbus_unmap_ring_vfree);
>
> +#ifdef CONFIG_XEN_PV
> +static int xenbus_map_ring_valloc_pv(struct xenbus_device *dev,
> + grant_ref_t *gnt_refs,
> + unsigned int nr_grefs,
> + void **vaddr)
> +{
> + struct xenbus_map_node *node;
> + struct vm_struct *area;
> + pte_t *ptes[XENBUS_MAX_RING_GRANTS];
> + phys_addr_t phys_addrs[XENBUS_MAX_RING_GRANTS];
> + int err = GNTST_okay;
> + int i;
> + bool leaked;
> +
> + *vaddr = NULL;
> +
> + if (nr_grefs > XENBUS_MAX_RING_GRANTS)
> + return -EINVAL;
> +
> + node = kzalloc(sizeof(*node), GFP_KERNEL);
> + if (!node)
> + return -ENOMEM;
> +
> + area = alloc_vm_area(XEN_PAGE_SIZE * nr_grefs, ptes);
> + if (!area) {
> + kfree(node);
> + return -ENOMEM;
> + }
> +
> + for (i = 0; i < nr_grefs; i++)
> + phys_addrs[i] = arbitrary_virt_to_machine(ptes[i]).maddr;
> +
> + err = __xenbus_map_ring(dev, gnt_refs, nr_grefs, node->handles,
> + phys_addrs,
> + GNTMAP_host_map | GNTMAP_contains_pte,
> + &leaked);
> + if (err)
> + goto failed;
> +
> + node->nr_handles = nr_grefs;
> + node->pv.area = area;
> +
> + spin_lock(&xenbus_valloc_lock);
> + list_add(&node->next, &xenbus_valloc_pages);
> + spin_unlock(&xenbus_valloc_lock);
> +
> + *vaddr = area->addr;
> + return 0;
> +
> +failed:
> + if (!leaked)
> + free_vm_area(area);
> + else
> + pr_alert("leaking VM area %p size %u page(s)", area, nr_grefs);
> +
> + kfree(node);
> + return err;
> +}
> +
Did you make any changes in xenbus_map_ring_valloc_pv()? I don't see any
but the diff looks pretty big --- I'd expect only the preprocessor
directives to show up.
-boris
On 14/09/17 16:00, Boris Ostrovsky wrote:
> On 09/14/2017 08:38 AM, Juergen Gross wrote:
>> xenbus_client.c contains some functions specific for pv guests.
>> Enclose them with #ifdef CONFIG_XEN_PV to avoid compiling them when
>> they are not needed (e.g. on ARM).
>>
>> Signed-off-by: Juergen Gross <[email protected]>
>> ---
>> drivers/xen/xenbus/xenbus_client.c | 130 +++++++++++++++++++------------------
>> 1 file changed, 67 insertions(+), 63 deletions(-)
>>
>> diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c
>> index 82a8866758ee..a1c17000129b 100644
>> --- a/drivers/xen/xenbus/xenbus_client.c
>> +++ b/drivers/xen/xenbus/xenbus_client.c
>> @@ -519,64 +519,6 @@ static int __xenbus_map_ring(struct xenbus_device *dev,
>> return err;
>> }
>>
>> -static int xenbus_map_ring_valloc_pv(struct xenbus_device *dev,
>> - grant_ref_t *gnt_refs,
>> - unsigned int nr_grefs,
>> - void **vaddr)
>> -{
>> - struct xenbus_map_node *node;
>> - struct vm_struct *area;
>> - pte_t *ptes[XENBUS_MAX_RING_GRANTS];
>> - phys_addr_t phys_addrs[XENBUS_MAX_RING_GRANTS];
>> - int err = GNTST_okay;
>> - int i;
>> - bool leaked;
>> -
>> - *vaddr = NULL;
>> -
>> - if (nr_grefs > XENBUS_MAX_RING_GRANTS)
>> - return -EINVAL;
>> -
>> - node = kzalloc(sizeof(*node), GFP_KERNEL);
>> - if (!node)
>> - return -ENOMEM;
>> -
>> - area = alloc_vm_area(XEN_PAGE_SIZE * nr_grefs, ptes);
>> - if (!area) {
>> - kfree(node);
>> - return -ENOMEM;
>> - }
>> -
>> - for (i = 0; i < nr_grefs; i++)
>> - phys_addrs[i] = arbitrary_virt_to_machine(ptes[i]).maddr;
>> -
>> - err = __xenbus_map_ring(dev, gnt_refs, nr_grefs, node->handles,
>> - phys_addrs,
>> - GNTMAP_host_map | GNTMAP_contains_pte,
>> - &leaked);
>> - if (err)
>> - goto failed;
>> -
>> - node->nr_handles = nr_grefs;
>> - node->pv.area = area;
>> -
>> - spin_lock(&xenbus_valloc_lock);
>> - list_add(&node->next, &xenbus_valloc_pages);
>> - spin_unlock(&xenbus_valloc_lock);
>> -
>> - *vaddr = area->addr;
>> - return 0;
>> -
>> -failed:
>> - if (!leaked)
>> - free_vm_area(area);
>> - else
>> - pr_alert("leaking VM area %p size %u page(s)", area, nr_grefs);
>> -
>> - kfree(node);
>> - return err;
>> -}
>> -
>> struct map_ring_valloc_hvm
>> {
>> unsigned int idx;
>> @@ -725,6 +667,65 @@ int xenbus_unmap_ring_vfree(struct xenbus_device *dev, void *vaddr)
>> }
>> EXPORT_SYMBOL_GPL(xenbus_unmap_ring_vfree);
>>
>> +#ifdef CONFIG_XEN_PV
>> +static int xenbus_map_ring_valloc_pv(struct xenbus_device *dev,
>> + grant_ref_t *gnt_refs,
>> + unsigned int nr_grefs,
>> + void **vaddr)
>> +{
>> + struct xenbus_map_node *node;
>> + struct vm_struct *area;
>> + pte_t *ptes[XENBUS_MAX_RING_GRANTS];
>> + phys_addr_t phys_addrs[XENBUS_MAX_RING_GRANTS];
>> + int err = GNTST_okay;
>> + int i;
>> + bool leaked;
>> +
>> + *vaddr = NULL;
>> +
>> + if (nr_grefs > XENBUS_MAX_RING_GRANTS)
>> + return -EINVAL;
>> +
>> + node = kzalloc(sizeof(*node), GFP_KERNEL);
>> + if (!node)
>> + return -ENOMEM;
>> +
>> + area = alloc_vm_area(XEN_PAGE_SIZE * nr_grefs, ptes);
>> + if (!area) {
>> + kfree(node);
>> + return -ENOMEM;
>> + }
>> +
>> + for (i = 0; i < nr_grefs; i++)
>> + phys_addrs[i] = arbitrary_virt_to_machine(ptes[i]).maddr;
>> +
>> + err = __xenbus_map_ring(dev, gnt_refs, nr_grefs, node->handles,
>> + phys_addrs,
>> + GNTMAP_host_map | GNTMAP_contains_pte,
>> + &leaked);
>> + if (err)
>> + goto failed;
>> +
>> + node->nr_handles = nr_grefs;
>> + node->pv.area = area;
>> +
>> + spin_lock(&xenbus_valloc_lock);
>> + list_add(&node->next, &xenbus_valloc_pages);
>> + spin_unlock(&xenbus_valloc_lock);
>> +
>> + *vaddr = area->addr;
>> + return 0;
>> +
>> +failed:
>> + if (!leaked)
>> + free_vm_area(area);
>> + else
>> + pr_alert("leaking VM area %p size %u page(s)", area, nr_grefs);
>> +
>> + kfree(node);
>> + return err;
>> +}
>> +
>
> Did you make any changes in xenbus_map_ring_valloc_pv()? I don't see any
> but the diff looks pretty big --- I'd expect only the preprocessor
> directives to show up.
I moved the functions to require only one #ifdef (plus 1 for setting
the pv variants).
Juergen
>> Did you make any changes in xenbus_map_ring_valloc_pv()? I don't see any
>> but the diff looks pretty big --- I'd expect only the preprocessor
>> directives to show up.
> I moved the functions to require only one #ifdef (plus 1 for setting
> the pv variants).
Oh, OK, I didn't notice that.
Reviewed-by: Boris Ostrovsky <[email protected]>
Hi Juergen,
On Thu, Sep 14, 2017 at 02:38:58PM +0200, Juergen Gross wrote:
> xenbus_client.c contains some functions specific for pv guests.
> Enclose them with #ifdef CONFIG_XEN_PV to avoid compiling them when
> they are not needed (e.g. on ARM).
>
> Signed-off-by: Juergen Gross <[email protected]>
Thanks for this! I think we also need to drop the old definition,
something like the below. Can you fold this in or should I send it
separately?
Cheers,
Tycho
>From 410a0c15c354f1ba387bdac6837d0a2031744c56 Mon Sep 17 00:00:00 2001
From: Tycho Andersen <[email protected]>
Date: Thu, 14 Sep 2017 08:57:30 -0700
Subject: [PATCH] xen, arm64: drop dummy lookup_address()
This is unused, and conflicts with the definition that we'll add for XPFO.
Signed-off-by: Tycho Andersen <[email protected]>
---
include/xen/arm/page.h | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/include/xen/arm/page.h b/include/xen/arm/page.h
index 415dbc6e43fd..6adc2a955340 100644
--- a/include/xen/arm/page.h
+++ b/include/xen/arm/page.h
@@ -84,16 +84,6 @@ static inline xmaddr_t arbitrary_virt_to_machine(void *vaddr)
BUG();
}
-/* TODO: this shouldn't be here but it is because the frontend drivers
- * are using it (its rolled in headers) even though we won't hit the code path.
- * So for right now just punt with this.
- */
-static inline pte_t *lookup_address(unsigned long address, unsigned int *level)
-{
- BUG();
- return NULL;
-}
-
extern int set_foreign_p2m_mapping(struct gnttab_map_grant_ref *map_ops,
struct gnttab_map_grant_ref *kmap_ops,
struct page **pages, unsigned int count);
--
2.11.0
On 14/09/17 18:31, Tycho Andersen wrote:
> Hi Juergen,
>
> On Thu, Sep 14, 2017 at 02:38:58PM +0200, Juergen Gross wrote:
>> xenbus_client.c contains some functions specific for pv guests.
>> Enclose them with #ifdef CONFIG_XEN_PV to avoid compiling them when
>> they are not needed (e.g. on ARM).
>>
>> Signed-off-by: Juergen Gross <[email protected]>
>
> Thanks for this! I think we also need to drop the old definition,
> something like the below. Can you fold this in or should I send it
> separately?
Please send it separately, as it touches ARM code only and thus should
be Acked by the ARM Xen maintainer.
Juergen
>
> Cheers,
>
> Tycho
>
> From 410a0c15c354f1ba387bdac6837d0a2031744c56 Mon Sep 17 00:00:00 2001
> From: Tycho Andersen <[email protected]>
> Date: Thu, 14 Sep 2017 08:57:30 -0700
> Subject: [PATCH] xen, arm64: drop dummy lookup_address()
>
> This is unused, and conflicts with the definition that we'll add for XPFO.
>
> Signed-off-by: Tycho Andersen <[email protected]>
> ---
> include/xen/arm/page.h | 10 ----------
> 1 file changed, 10 deletions(-)
>
> diff --git a/include/xen/arm/page.h b/include/xen/arm/page.h
> index 415dbc6e43fd..6adc2a955340 100644
> --- a/include/xen/arm/page.h
> +++ b/include/xen/arm/page.h
> @@ -84,16 +84,6 @@ static inline xmaddr_t arbitrary_virt_to_machine(void *vaddr)
> BUG();
> }
>
> -/* TODO: this shouldn't be here but it is because the frontend drivers
> - * are using it (its rolled in headers) even though we won't hit the code path.
> - * So for right now just punt with this.
> - */
> -static inline pte_t *lookup_address(unsigned long address, unsigned int *level)
> -{
> - BUG();
> - return NULL;
> -}
> -
> extern int set_foreign_p2m_mapping(struct gnttab_map_grant_ref *map_ops,
> struct gnttab_map_grant_ref *kmap_ops,
> struct page **pages, unsigned int count);
>
On 09/14/2017 08:38 AM, Juergen Gross wrote:
> xenbus_client.c contains some functions specific for pv guests.
> Enclose them with #ifdef CONFIG_XEN_PV to avoid compiling them when
> they are not needed (e.g. on ARM).
>
> Signed-off-by: Juergen Gross <[email protected]>
Applied to for-linus-14b.
-boris