Re: [Xen-devel] [PATCH v2 16/21] xen/passthrough: iommu_deassign_device_dt: By default reassign device to nobody

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 31 Jul 2014, 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.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> 
> ---
>     Changes in v2:
>         - Fix typoes in the commit message
>         - Update commit message
> ---
>  xen/drivers/passthrough/arm/smmu.c    |    7 ++++---
>  xen/drivers/passthrough/device_tree.c |    8 +++-----
>  2 files changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/arm/smmu.c 
> b/xen/drivers/passthrough/arm/smmu.c
> index f4eb2a2..b25034e 100644
> --- a/xen/drivers/passthrough/arm/smmu.c
> +++ b/xen/drivers/passthrough/arm/smmu.c
> @@ -1245,8 +1245,8 @@ static int arm_smmu_reassign_dt_dev(struct domain *s, 
> struct domain *t,
>  {
>      int ret = 0;
>  
> -    /* Don't allow remapping on other domain than hwdom */
> -    if ( t != hardware_domain )
> +    /* Allow remapping either on the hardware domain or to nothing */
> +    if ( t && t != hardware_domain )
>          return -EPERM;
>  
>      if ( t == s )
> @@ -1256,7 +1256,8 @@ static int arm_smmu_reassign_dt_dev(struct domain *s, 
> struct domain *t,
>      if ( ret )
>          return ret;
>  
> -    ret = arm_smmu_attach_dev(t, dev);
> +    if ( t )
> +        ret = arm_smmu_attach_dev(t, dev);
>  
>      return ret;
>  }
> diff --git a/xen/drivers/passthrough/device_tree.c 
> b/xen/drivers/passthrough/device_tree.c
> index 45d4a59..f8affa0 100644
> --- a/xen/drivers/passthrough/device_tree.c
> +++ b/xen/drivers/passthrough/device_tree.c
> @@ -75,14 +75,12 @@ int iommu_deassign_dt_device(struct domain *d, struct 
> dt_device_node *dev)
>  
>      spin_lock(&dtdevs_lock);
>  
> -    rc = hd->platform_ops->reassign_dt_device(d, hardware_domain, dev);
> +    rc = hd->platform_ops->reassign_dt_device(d, NULL, dev);
>      if ( rc )
>          goto fail;

At this point, isn't it better just to call arm_smmu_detach_dev?


> -    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);
> -- 
> 1.7.10.4
> 

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