[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] xen/iommu: arm: Don't insert an IOMMU mapping when the grantee and granter...


  • To: Julien Grall <julien@xxxxxxx>
  • From: Rahul Singh <Rahul.Singh@xxxxxxx>
  • Date: Tue, 16 Feb 2021 18:52:11 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=0NsScOyP/RjOMccIdFQRHi2n/MAzJVlLB32h7r44Nd8=; b=fL2y8/v1oz3JV5KLaoL6JnaQyxxSLWOUp692miXzHBAEYrtPgo9Y47jFKhufbB8YGSY1XoRFYpW23r7P2oJ2ybkcioREMxzRdwiLaoblbVn/EwhCw4jXd7dXwPVpWU0y4OXNnqfR+kSrH26ZzYcAAWUNiCwx59o4CGeISxPEKh/H6aklLmGLN0CgDPswWGlDu1TbsmTUBe8klV+gB6fqKz5SRXvtQIi7kfyKIRpRgfiWE8onJMKJjFVJjdB3yoFqfpwyb72jYb/iMBmIJ5jUTmrVe0vhYMiXfdJajII6PWYykPQoG6nRtSP68Brq/DtNxP4iMwZFtd7ZuFfNKDk6tg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=nXZ+IHgEs58YwGZBm9ANnWUYgVknWz8R/fiEvtqAaCNeC1apNOp+YY7i2+N7f8e9i1iNvj1vl/urr9m3FbxnSblzVHo5yIBjNgsCDX48JpzqYLt6QxvCPMFyn286FtfnIXR94RedTqPb6CY9yQxFe9OyrskQBQgDeJ8CUPX2t6sKI9G4IedWEhSa01uFnqmnYkfRYJgkngBKT5+FcPL09eMs0x5MYzXbP33m2+fgEX7Qmo1L+JJXD3SQVkMKtr3LgDukMp9EqhBG2ft4OIq8r9Q0uMCXMjOG3Yjle8CFd7ua3/nhdr9KrJ4f3soIM1Ed3FCJTgzpBP0POSAgHCWhoA==
  • Authentication-results-original: xen.org; dkim=none (message not signed) header.d=none;xen.org; dmarc=none action=none header.from=arm.com;
  • Cc: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Julien Grall <jgrall@xxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • Delivery-date: Tue, 16 Feb 2021 18:52:28 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: xen.org; dkim=none (message not signed) header.d=none;xen.org; dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHXAt6oFaIGN7XEykaD6M/rWzXvuKpbI+sA
  • Thread-topic: [PATCH] xen/iommu: arm: Don't insert an IOMMU mapping when the grantee and granter...

Hello Julien,

> On 14 Feb 2021, at 2:35 pm, Julien Grall <julien@xxxxxxx> wrote:
> 
> From: Julien Grall <jgrall@xxxxxxxxxx>
> 
> ... are the same.
> 
> When the IOMMU is enabled and the domain is direct mapped (e.g. Dom0),
> Xen will insert a 1:1 mapping for each grant mapping in the P2M to
> allow DMA.
> 
> This works quite well when the grantee and granter and

/s/and/are

> not the same
> because the GFN in the P2M should not be mapped. However, if they are
> the same, we will overwrite the mapping. Worse, it will be completely
> removed when the grant is unmapped.
> 
> As the domain is direct mapped, a 1:1 mapping should always present in
> the P2M. This is not 100% guaranteed if the domain decides to mess with
> the P2M. However, such domain would already end up in trouble as the
> page would be soon be freed (when the last reference dropped).
> 
> Add an additional check in arm_iommu_{,un}map_page() to check whether
> the page belongs to the domain. If it is belongs to it, then ignore the
> request.
> 
> Note that we don't need to hold an extra reference on the page because
> the grant code will already do it for us.
> 
> Reported-by: Rahul Singh <Rahul.Singh@xxxxxxx>
> Fixes: 552710b38863 ("xen/arm: grant: Add another entry to map MFN 1:1 in 
> dom0 p2m")
> Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>

With the typo fixed.

Reviewed-by: Rahul Singh <rahul.singh@xxxxxxx>
Tested-by: Rahul Singh <rahul.singh@xxxxxxx>

Regards,
Rahul
> 
> ---
> 
> This is a candidate for 4.15. Without the patch, it would not be
> possible to get the frontend and backend in the same domain (useful when
> trying to map the guest block device in dom0).
> ---
> xen/drivers/passthrough/arm/iommu_helpers.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
> 
> diff --git a/xen/drivers/passthrough/arm/iommu_helpers.c 
> b/xen/drivers/passthrough/arm/iommu_helpers.c
> index a36e2b8e6c42..35a813308b8c 100644
> --- a/xen/drivers/passthrough/arm/iommu_helpers.c
> +++ b/xen/drivers/passthrough/arm/iommu_helpers.c
> @@ -53,6 +53,17 @@ int __must_check arm_iommu_map_page(struct domain *d, 
> dfn_t dfn, mfn_t mfn,
> 
>     t = (flags & IOMMUF_writable) ? p2m_iommu_map_rw : p2m_iommu_map_ro;
> 
> +    /*
> +     * The granter and grantee can be the same domain. In normal
> +     * condition, the 1:1 mapping should already present in the P2M so
> +     * we want to avoid overwriting it.
> +     *
> +     * Note that there is no guarantee the 1:1 mapping will be present
> +     * if the domain decides to mess with the P2M.
> +     */
> +    if ( page_get_owner(mfn_to_page(mfn)) == d )
> +        return 0;
> +
>     /*
>      * The function guest_physmap_add_entry replaces the current mapping
>      * if there is already one...
> @@ -71,6 +82,13 @@ int __must_check arm_iommu_unmap_page(struct domain *d, 
> dfn_t dfn,
>     if ( !is_domain_direct_mapped(d) )
>         return -EINVAL;
> 
> +    /*
> +     * When the granter and grantee are the same, we didn't insert a
> +     * mapping. So ignore the unmap request.
> +     */
> +    if ( page_get_owner(mfn_to_page(_mfn(dfn_x(dfn)))) == d )
> +        return 0;
> +
>     return guest_physmap_remove_page(d, _gfn(dfn_x(dfn)), _mfn(dfn_x(dfn)), 
> 0);
> }
> 
> -- 
> 2.17.1
> 




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.