All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefano Stabellini <sstabellini@kernel.org>
To: julien@xen.org
Cc: lucmiccio@gmail.com, sstabellini@kernel.org,
	xen-devel@lists.xenproject.org, Bertrand.Marquis@arm.com,
	Volodymyr_Babchuk@epam.com, Rahul.Singh@arm.com,
	Stefano Stabellini <stefano.stabellini@xilinx.com>
Subject: [PATCH v2] xen/arm: fix gnttab_need_iommu_mapping
Date: Mon,  8 Feb 2021 10:49:32 -0800	[thread overview]
Message-ID: <20210208184932.23468-1-sstabellini@kernel.org> (raw)

Commit 91d4eca7add broke gnttab_need_iommu_mapping on ARM.
The offending chunk is:

 #define gnttab_need_iommu_mapping(d)                    \
-    (is_domain_direct_mapped(d) && need_iommu(d))
+    (is_domain_direct_mapped(d) && need_iommu_pt_sync(d))

On ARM we need gnttab_need_iommu_mapping to be true for dom0 when it is
directly mapped and IOMMU is enabled for the domain, like the old check
did, but the new check is always false.

In fact, need_iommu_pt_sync is defined as dom_iommu(d)->need_sync and
need_sync is set as:

    if ( !is_hardware_domain(d) || iommu_hwdom_strict )
        hd->need_sync = !iommu_use_hap_pt(d);

iommu_use_hap_pt(d) means that the page-table used by the IOMMU is the
P2M. It is true on ARM. need_sync means that you have a separate IOMMU
page-table and it needs to be updated for every change. need_sync is set
to false on ARM. Hence, gnttab_need_iommu_mapping(d) is false too,
which is wrong.

As a consequence, when using PV network from a domU on a system where
IOMMU is on from Dom0, I get:

(XEN) smmu: /smmu@fd800000: Unhandled context fault: fsr=0x402, iova=0x8424cb148, fsynr=0xb0001, cb=0
[   68.290307] macb ff0e0000.ethernet eth0: DMA bus error: HRESP not OK

The fix is to go back to something along the lines of the old
implementation of gnttab_need_iommu_mapping.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
Fixes: 91d4eca7add
Backport: 4.12+

---

Given the severity of the bug, I would like to request this patch to be
backported to 4.12 too, even if 4.12 is security-fixes only since Oct
2020.

For the 4.12 backport, we can use iommu_enabled() instead of
is_iommu_enabled() in the implementation of gnttab_need_iommu_mapping.

Changes in v2:
- improve commit message
- add is_iommu_enabled(d) to the check
---
 xen/include/asm-arm/grant_table.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/include/asm-arm/grant_table.h b/xen/include/asm-arm/grant_table.h
index 6f585b1538..0ce77f9a1c 100644
--- a/xen/include/asm-arm/grant_table.h
+++ b/xen/include/asm-arm/grant_table.h
@@ -89,7 +89,7 @@ int replace_grant_host_mapping(unsigned long gpaddr, mfn_t mfn,
     (((i) >= nr_status_frames(t)) ? INVALID_GFN : (t)->arch.status_gfn[i])
 
 #define gnttab_need_iommu_mapping(d)                    \
-    (is_domain_direct_mapped(d) && need_iommu_pt_sync(d))
+    (is_domain_direct_mapped(d) && is_iommu_enabled(d))
 
 #endif /* __ASM_GRANT_TABLE_H__ */
 /*
-- 
2.17.1



             reply	other threads:[~2021-02-08 18:49 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-08 18:49 Stefano Stabellini [this message]
2021-02-08 20:02 ` [PATCH v2] xen/arm: fix gnttab_need_iommu_mapping Julien Grall
2021-02-08 20:24   ` Stefano Stabellini
2021-02-08 21:23     ` Julien Grall
2021-02-09  1:57       ` Stefano Stabellini
2021-02-09  8:44         ` Julien Grall
2021-02-09 10:02     ` Jan Beulich
2021-02-09 14:03       ` Ian Jackson
2021-02-09 17:31         ` Stefano Stabellini
2021-02-09 21:04           ` Julien Grall
2021-02-10  1:35             ` Stefano Stabellini
2021-02-10 11:04           ` George Dunlap
2021-02-09 13:13 ` Rahul Singh
2021-02-09 20:36   ` Stefano Stabellini
2021-02-09 21:15     ` Stefano Stabellini
2021-02-10 15:06     ` Rahul Singh
2021-02-10 17:34       ` Julien Grall
2021-02-10 18:08         ` Rahul Singh
2021-02-10 19:52           ` Julien Grall
2021-02-11 13:20             ` Rahul Singh
2021-02-11 13:52               ` Julien Grall
2021-02-11 16:05                 ` Rahul Singh
2021-02-14 14:32                   ` Julien Grall
2021-02-15  9:12                     ` Rahul Singh
2021-02-15 11:34                       ` Julien Grall
2021-02-11 20:55                 ` Stefano Stabellini
2021-02-12 13:30                   ` Julien Grall
2021-02-12 18:56                     ` Stefano Stabellini
2021-02-10 21:13       ` Stefano Stabellini
2021-02-11 12:03         ` Rahul Singh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210208184932.23468-1-sstabellini@kernel.org \
    --to=sstabellini@kernel.org \
    --cc=Bertrand.Marquis@arm.com \
    --cc=Rahul.Singh@arm.com \
    --cc=Volodymyr_Babchuk@epam.com \
    --cc=julien@xen.org \
    --cc=lucmiccio@gmail.com \
    --cc=stefano.stabellini@xilinx.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.