Re: [Xen-devel] [RFC 12/19] xen/passthrough: iommu_deassign_device_dt: By default reassign device to nobody

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2014-06-16 at 17:17 +0100, 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 corrupt or
> expose some part of DOM0 memory.

"corruption".

I'd go further and say "and 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).

Can you describe here what happen in this case (I presume Xen tears down
the iommu to quiesce them somehow?)

> 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>
> ---
>  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 afb4dfc..8a4bc69 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;
>  
> -    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);



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