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

[Juergen Gross <jgross@xxxxxxxx>]

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 <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.

I moved the functions to require only one #ifdef (plus 1 for setting
the pv variants).


Juergen

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