Re: [Xen-devel] [PATCH] xen: don't compile pv-specific parts if XEN_PV isn't configured

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

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 <jgross@xxxxxxxx>
> ---
>  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



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel