Hello Stefano, > On 6 Feb 2021, at 12:38 am, Stefano Stabellini <sstabell...@kernel.org> 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. https://gitlab.com/xen-project/fusa/xen-integration/-/commit/43a1a6ec91c4e3e28fa54dcbecdc8a2917836765 Regards, Rahul > > 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.stabell...@xilinx.com> > 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__ */ > /* >
