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

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

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@suse.com>
> ---
>  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