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

Re: [PATCH] xen/arm: fix gnttab_need_iommu_mapping

  • To: Stefano Stabellini <sstabellini@xxxxxxxxxx>
  • From: Rahul Singh <Rahul.Singh@xxxxxxx>
  • Date: Mon, 8 Feb 2021 18:06:39 +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=i0/3jX6KzFfFsjKfa2HZ9RmN8D8cy6Ubu3tZljfLVVQ=; b=Xi2AmpwMdwTBwnrmQRz1yAAbJa7r84XaN4/lecqy8rg2aYzZXtV8e5UmzpiBXpiI/+nnPAJVv5ijdhjTi9WS86eMUWYo8/A4kN4szAzIzCTvJvWicLBLk5KeIZvAaeqsBAwlGtqcyO7jhqtLdz7HLyBSVlGZo5ck7eH2rNJpp2ho0R+HkReJCK0duzA+86YrIv29v6/xi6WmIVX5noyu8r0FCM0xr3JtSV7UshCCmBIdmwRFG4w81Zyn/EbrjsQIXh/4NEqxogfV4LzWS9FBof4Hz1dMh5kuz5MRD+mLXHQcfrIVEca73gHxMJTt5SfZx3cLmR0nfIpDPcZRamOtzA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=cPEN9LvjwGTXYO2lW2SagdjD2aHSAsOJRGc0WhapZ47Q/XtLcuySzWMWvbBgbI0d0dfIEt6XhCwF12l8xY6iaAgwoQ2xohc2gLxn+STZfhuvIEdipUZ6O16UJCI2wZlECWdE4yWYu4PnfxQbXY9mzCFBaq1lq1QDCvKKDtog6p7o7GKdWFIgkfY4Rav9FnXj7cKbxvILAb5JgbAJ7p6R/fKoUz1XPYF23iiTDWkUzwF2G91E9Jb6yBjOk0442z97WuChuOK9EItZ0xhBBsX7VvvAg6mODzWT6ZqIAUXKn+McyXWI9IjZ2qWaN4XVXfU2vvYaEThFpKjm0nO5U47rXg==
  • Authentication-results-original: kernel.org; dkim=none (message not signed) header.d=none;kernel.org; dmarc=none action=none header.from=arm.com;
  • Cc: Julien Grall <julien@xxxxxxx>, "lucmiccio@xxxxxxxxx" <lucmiccio@xxxxxxxxx>, xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Bertrand Marquis <Bertrand.Marquis@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • Delivery-date: Mon, 08 Feb 2021 18:07:18 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: kernel.org; dkim=none (message not signed) header.d=none;kernel.org; dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHW/CCVIBhvrRIcEE2wSNDNMu6y5KpOkgeA
  • Thread-topic: [PATCH] xen/arm: fix gnttab_need_iommu_mapping

Hello Stefano,

> On 6 Feb 2021, at 12:38 am, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
> 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, 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_hwdom_strict is actually supposed to be ignored on ARM, see the
> definition in docs/misc/xen-command-line.pandoc:
>    This option is hardwired to true for x86 PVH dom0's (as RAM belonging to
>    other domains in the system don't live in a compatible address space), and
>    is ignored for ARM.
> But aside from that, the issue is that iommu_use_hap_pt(d) is true,
> hence, hd->need_sync is false, and gnttab_need_iommu_mapping(d) is false
> too.
> 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

I also observed the IOMMU fault when DOMU guest is created and great table is 
used when IOMMU is enabled. I fixed the error in different way but I am not 
sure if you also observing the same error. I submitted the patch to 
pci-passthrough integration branch. Please have a look once if that make sense.


> The fix is to go back to the old implementation of
> gnttab_need_iommu_mapping.  However, we don't even need to specify &&
> need_iommu(d) since we don't actually need to check for the IOMMU to be
> enabled (iommu_map does it for us at the beginning of the function.)
> This fix is preferrable to changing the implementation of need_sync or
> iommu_use_hap_pt because "need_sync" is not really the reason why we
> want gnttab_need_iommu_mapping to return true.
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> Backport: 4.12+ 
> ---
> It is incredible that it was missed for this long, but it takes a full
> PV drivers test using DMA from a non-coherent device to trigger it, e.g.
> wget from a domU to an HTTP server on a different machine, ping or
> connections to dom0 won't trigger the bug.
> It is interesting that given that IOMMU is on for dom0, Linux could
> have just avoided using swiotlb-xen and everything would have just
> worked. It is worth considering introducing a feature flag (e.g.
> XENFEAT_ARM_dom0_iommu) to let dom0 know that the IOMMU is on and
> swiotlb-xen is not necessary. Then Linux can avoid initializing
> swiotlb-xen and just rely on the IOMMU for translation.
> diff --git a/xen/include/asm-arm/grant_table.h 
> b/xen/include/asm-arm/grant_table.h
> index 6f585b1538..2a154d1851 100644
> --- a/xen/include/asm-arm/grant_table.h
> +++ b/xen/include/asm-arm/grant_table.h
> @@ -88,8 +88,7 @@ int replace_grant_host_mapping(unsigned long gpaddr, mfn_t 
> mfn,
> #define gnttab_status_gfn(d, t, i)                                       \
>     (((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))
> +#define gnttab_need_iommu_mapping(d)  (is_domain_direct_mapped(d))
> #endif /* __ASM_GRANT_TABLE_H__ */
> /*



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