Re: [PATCH v3 18/24] xen/passthrough: iommu_deassign_device_dt: By default reassign device to nobody

[Ian Campbell <ian.campbell@citrix.com>]

On Tue, 2015-01-13 at 14:25 +0000, Julien Grall wrote:
> Currently, when the device is deassigned from a domain, we directly reassign
> to DOM0.
> 
> As the device may not have been correctly reset, this may lead to corruption or
> expose some part of DOM0 memory. Also, we may have no way to reset some
> platform devices.
> 
> If Xen reassigns the device to "nobody", it may receive some global/context
> fault because the transaction has failed (indeed the context has been
> marked invalid). Unfortunately there is no simple way to quiesce a buggy
> hardware. I think we could live with that for a first version of platform
> device passthrough.
> 
> DOM0 will have to issue an hypercall to assign the device to itself if it
> wants to use it.

Does this behaviour differ from x86? If so then it is worth calling that
out explicitly (even if not, good to know I think!)

> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Cc: Jan Beulich <jbeulich@suse.com>
> 
> ---
>     Changes in v3:
>         - Use the coding style of the new SMMU drivers
> 
>     Changes in v2:
>         - Fix typoes in the commit message
>         - Update commit message
> ---
>  xen/drivers/passthrough/arm/smmu.c    | 8 +++++++-
>  xen/drivers/passthrough/device_tree.c | 9 +++------
>  2 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
> index 3cf1773..45a2db8 100644
> --- a/xen/drivers/passthrough/arm/smmu.c
> +++ b/xen/drivers/passthrough/arm/smmu.c
> @@ -2774,7 +2774,7 @@ static int arm_smmu_reassign_dev(struct domain *s, struct domain *t,
>  	int ret = 0;
>  
>  	/* Don't allow remapping on other domain than hwdom */
> -	if (t != hardware_domain)
> +	if (t && t != hardware_domain)
>  		return -EPERM;
>  
>  	if (t == s)
> @@ -2784,6 +2784,12 @@ static int arm_smmu_reassign_dev(struct domain *s, struct domain *t,
>  	if (ret)
>  		return ret;
>  
> +	if (t) {
> +		ret = arm_smmu_assign_dev(t, devfn, dev);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	return 0;
>  }
>  
> diff --git a/xen/drivers/passthrough/device_tree.c b/xen/drivers/passthrough/device_tree.c
> index e7eb34f..d9b486e 100644
> --- a/xen/drivers/passthrough/device_tree.c
> +++ b/xen/drivers/passthrough/device_tree.c
> @@ -72,15 +72,12 @@ int iommu_deassign_dt_device(struct domain *d, struct dt_device_node *dev)
>  
>      spin_lock(&dtdevs_lock);
>  
> -    rc = hd->platform_ops->reassign_device(d, hardware_domain,
> -                                           0, dt_to_dev(dev));
> +    rc = hd->platform_ops->reassign_device(d, NULL, 0, dt_to_dev(dev));
>      if ( rc )
>          goto fail;
>  
> -    list_del(&dev->domain_list);
> -
> -    dt_device_set_used_by(dev, hardware_domain->domain_id);
> -    list_add(&dev->domain_list, &domain_hvm_iommu(hardware_domain)->dt_devices);
> +    list_del_init(&dev->domain_list);
> +    dt_device_set_used_by(dev, DOMID_IO);
>  
>  fail:
>      spin_unlock(&dtdevs_lock);