From: Stefano Stabellini <sstabellini@kernel.org> To: Robin Murphy <robin.murphy@arm.com> Cc: Chuck Zmudzinski <brchuckz@aol.com>, linux-arm-kernel@lists.infradead.org, Russell King <linux@armlinux.org.uk>, Juergen Gross <jgross@suse.com>, Stefano Stabellini <sstabellini@kernel.org>, Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>, linux-kernel@vger.kernel.org, xen-devel@lists.xenproject.org, Marek Szyprowski <m.szyprowski@samsung.com>, Andrew Morton <akpm@linux-foundation.org>, Mathieu Poirier <mathieu.poirier@linaro.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, "Matthew Wilcox (Oracle)" <willy@infradead.org>, Linus Walleij <linus.walleij@linaro.org>, Jason Gunthorpe <jgg@ziepe.ca>, Arnd Bergmann <arnd@arndb.de>, Julien Grall <julien@xen.org>, Mario Marietto <marietto2008@gmail.com>, Bertrand Marquis <bertrand.marquis@arm.com> Subject: Re: [PATCH] arm/mm: add option to prefer IOMMU ops for DMA on Xen Date: Tue, 14 Nov 2023 14:20:13 -0800 (PST) [thread overview] Message-ID: <alpine.DEB.2.22.394.2311141407140.160649@ubuntu-linux-20-04-desktop> (raw) In-Reply-To: <e5ebfde9-7a74-4908-b0b9-db47c4e76315@arm.com> On Tue, 14 Nov 2023, Robin Murphy wrote: > On 11/11/2023 6:45 pm, Chuck Zmudzinski wrote: > > Enabling the new option, ARM_DMA_USE_IOMMU_XEN, fixes this error when > > attaching the Exynos mixer in Linux dom0 on Xen on the Chromebook Snow > > (and probably on other devices that use the Exynos mixer): > > > > [drm] Exynos DRM: using 14400000.fimd device for DMA mapping operations > > exynos-drm exynos-drm: bound 14400000.fimd (ops 0xc0d96354) > > exynos-mixer 14450000.mixer: [drm:exynos_drm_register_dma] *ERROR* Device > > 14450000.mixer lacks support for IOMMU > > exynos-drm exynos-drm: failed to bind 14450000.mixer (ops 0xc0d97554): -22 > > exynos-drm exynos-drm: adev bind failed: -22 > > exynos-dp: probe of 145b0000.dp-controller failed with error -22 > > > > Linux normally uses xen_swiotlb_dma_ops for DMA for all devices when > > xen_swiotlb is detected even when Xen exposes an IOMMU to Linux. Enabling > > the new config option allows devices such as the Exynos mixer to use the > > IOMMU instead of xen_swiotlb_dma_ops for DMA and this fixes the error. > > > > The new config option is not set by default because it is likely some > > devices that use IOMMU for DMA on Xen will cause DMA errors and memory > > corruption when Xen PV block and network drivers are in use on the system. > > > > Link: > > https://lore.kernel.org/xen-devel/acfab1c5-eed1-4930-8c70-8681e256c820@netscape.net/ > > > > Signed-off-by: Chuck Zmudzinski <brchuckz@aol.com> > > --- > > The reported error with the Exynos mixer is not fixed by default by adding > > a second patch to select the new option in the Kconfig definition for the > > Exynos mixer if EXYNOS_IOMMU and SWIOTLB_XEN are enabled because it is > > not certain setting the config option is suitable for all cases. So it is > > necessary to explicitly select the new config option during the config > > stage of the Linux kernel build to fix the reported error or similar > > errors that have the same cause of lack of support for IOMMU on Xen. This > > is necessary to avoid any regressions that might be caused by enabling the > > new option by default for the Exynos mixer. > > arch/arm/mm/dma-mapping.c | 6 ++++++ > > drivers/xen/Kconfig | 16 ++++++++++++++++ > > 2 files changed, 22 insertions(+) > > > > diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c > > index 5409225b4abc..ca04fdf01be3 100644 > > --- a/arch/arm/mm/dma-mapping.c > > +++ b/arch/arm/mm/dma-mapping.c > > @@ -1779,6 +1779,12 @@ void arch_setup_dma_ops(struct device *dev, u64 > > dma_base, u64 size, > > if (iommu) > > arm_setup_iommu_dma_ops(dev, dma_base, size, iommu, coherent); > > +#ifdef CONFIG_ARM_DMA_USE_IOMMU_XEN > > FWIW I don't think this really needs a config option - if Xen *has* made an > IOMMU available, then there isn't really much reason not to use it, and if for > some reason someone really didn't want to then they could simply disable the > IOMMU driver anyway. The fact that the Exynos IOMMU is exposed to Linux is a mistake. Xen doesn't recognize the Exynos IOMMU (it is not one of the IOMMUs Xen has a driver for) so it assigns the IOMMU to Dom0. It doesn't happen on purpose, it happens by accident. Certain things are going to break, specifically I am fairly certain PV drivers are going to break. If Xen recognized the Exynos IOMMU as an IOMMU it would probably hide it from Dom0. (Today Xen doesn't have a list of IOMMUs Xen recognizes but doesn't have a driver for.) I think it is OK for Chuck and others to play around with this configuration but I wouldn't add a new kconfig option to Linux to support it. If we do want a kconfig option, I would add a kconfig option or Linux command line option to enable/disable swiotlb-xen. Basically a way to force-enable or force-disable xen_swiotlb_detect(). That could be generally useful for debugging and would also solve the problem here as it could be used to force-disable swiotlb-xen. I would imagine that the end result is the same: the default ops (iommu_ops) are used. > > + if (dev->dma_ops == &iommu_ops) { > > + dev->archdata.dma_ops_setup = true; > > The existing assignment is effectively unconditional by this point anyway, so > could probably just be moved earlier to save duplicating it (or perhaps just > make the xen_setup_dma_ops() call conditional instead to save the early return > as well). > > However, are the IOMMU DMA ops really compatible with Xen? The comments about > hypercalls and foreign memory in xen_arch_need_swiotlb() leave me concerned > that assuming non-coherent DMA to any old Dom0 page is OK might not actually > work in general :/ Xen has (not yet upstreaming) support for nested IOMMU (Xen uses the IOMMU while also it exposes a virtual IOMMU to guests.) In those cases the iommu_ops should be compatible with Xen. swiotlb-xen is useful in cases where there is no IOMMU on the platform (or the IOMMU doesn't cover all DMA-capable devices) and Dom0 is 1:1 mapped. See include/xen/arm/swiotlb-xen.h:xen_swiotlb_detect. If Dom0 is not 1:1 mapped swiotlb-xen doesn't work. If an IOMMU is present and covers all DMA-capable devices, then swiotlb-xen is superfluous. This last case is the interesting case for virtual IOMMU and Linux usage of iommu_ops.
WARNING: multiple messages have this Message-ID (diff)
From: Stefano Stabellini <sstabellini@kernel.org> To: Robin Murphy <robin.murphy@arm.com> Cc: Chuck Zmudzinski <brchuckz@aol.com>, linux-arm-kernel@lists.infradead.org, Russell King <linux@armlinux.org.uk>, Juergen Gross <jgross@suse.com>, Stefano Stabellini <sstabellini@kernel.org>, Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>, linux-kernel@vger.kernel.org, xen-devel@lists.xenproject.org, Marek Szyprowski <m.szyprowski@samsung.com>, Andrew Morton <akpm@linux-foundation.org>, Mathieu Poirier <mathieu.poirier@linaro.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, "Matthew Wilcox (Oracle)" <willy@infradead.org>, Linus Walleij <linus.walleij@linaro.org>, Jason Gunthorpe <jgg@ziepe.ca>, Arnd Bergmann <arnd@arndb.de>, Julien Grall <julien@xen.org>, Mario Marietto <marietto2008@gmail.com>, Bertrand Marquis <bertrand.marquis@arm.com> Subject: Re: [PATCH] arm/mm: add option to prefer IOMMU ops for DMA on Xen Date: Tue, 14 Nov 2023 14:20:13 -0800 (PST) [thread overview] Message-ID: <alpine.DEB.2.22.394.2311141407140.160649@ubuntu-linux-20-04-desktop> (raw) In-Reply-To: <e5ebfde9-7a74-4908-b0b9-db47c4e76315@arm.com> On Tue, 14 Nov 2023, Robin Murphy wrote: > On 11/11/2023 6:45 pm, Chuck Zmudzinski wrote: > > Enabling the new option, ARM_DMA_USE_IOMMU_XEN, fixes this error when > > attaching the Exynos mixer in Linux dom0 on Xen on the Chromebook Snow > > (and probably on other devices that use the Exynos mixer): > > > > [drm] Exynos DRM: using 14400000.fimd device for DMA mapping operations > > exynos-drm exynos-drm: bound 14400000.fimd (ops 0xc0d96354) > > exynos-mixer 14450000.mixer: [drm:exynos_drm_register_dma] *ERROR* Device > > 14450000.mixer lacks support for IOMMU > > exynos-drm exynos-drm: failed to bind 14450000.mixer (ops 0xc0d97554): -22 > > exynos-drm exynos-drm: adev bind failed: -22 > > exynos-dp: probe of 145b0000.dp-controller failed with error -22 > > > > Linux normally uses xen_swiotlb_dma_ops for DMA for all devices when > > xen_swiotlb is detected even when Xen exposes an IOMMU to Linux. Enabling > > the new config option allows devices such as the Exynos mixer to use the > > IOMMU instead of xen_swiotlb_dma_ops for DMA and this fixes the error. > > > > The new config option is not set by default because it is likely some > > devices that use IOMMU for DMA on Xen will cause DMA errors and memory > > corruption when Xen PV block and network drivers are in use on the system. > > > > Link: > > https://lore.kernel.org/xen-devel/acfab1c5-eed1-4930-8c70-8681e256c820@netscape.net/ > > > > Signed-off-by: Chuck Zmudzinski <brchuckz@aol.com> > > --- > > The reported error with the Exynos mixer is not fixed by default by adding > > a second patch to select the new option in the Kconfig definition for the > > Exynos mixer if EXYNOS_IOMMU and SWIOTLB_XEN are enabled because it is > > not certain setting the config option is suitable for all cases. So it is > > necessary to explicitly select the new config option during the config > > stage of the Linux kernel build to fix the reported error or similar > > errors that have the same cause of lack of support for IOMMU on Xen. This > > is necessary to avoid any regressions that might be caused by enabling the > > new option by default for the Exynos mixer. > > arch/arm/mm/dma-mapping.c | 6 ++++++ > > drivers/xen/Kconfig | 16 ++++++++++++++++ > > 2 files changed, 22 insertions(+) > > > > diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c > > index 5409225b4abc..ca04fdf01be3 100644 > > --- a/arch/arm/mm/dma-mapping.c > > +++ b/arch/arm/mm/dma-mapping.c > > @@ -1779,6 +1779,12 @@ void arch_setup_dma_ops(struct device *dev, u64 > > dma_base, u64 size, > > if (iommu) > > arm_setup_iommu_dma_ops(dev, dma_base, size, iommu, coherent); > > +#ifdef CONFIG_ARM_DMA_USE_IOMMU_XEN > > FWIW I don't think this really needs a config option - if Xen *has* made an > IOMMU available, then there isn't really much reason not to use it, and if for > some reason someone really didn't want to then they could simply disable the > IOMMU driver anyway. The fact that the Exynos IOMMU is exposed to Linux is a mistake. Xen doesn't recognize the Exynos IOMMU (it is not one of the IOMMUs Xen has a driver for) so it assigns the IOMMU to Dom0. It doesn't happen on purpose, it happens by accident. Certain things are going to break, specifically I am fairly certain PV drivers are going to break. If Xen recognized the Exynos IOMMU as an IOMMU it would probably hide it from Dom0. (Today Xen doesn't have a list of IOMMUs Xen recognizes but doesn't have a driver for.) I think it is OK for Chuck and others to play around with this configuration but I wouldn't add a new kconfig option to Linux to support it. If we do want a kconfig option, I would add a kconfig option or Linux command line option to enable/disable swiotlb-xen. Basically a way to force-enable or force-disable xen_swiotlb_detect(). That could be generally useful for debugging and would also solve the problem here as it could be used to force-disable swiotlb-xen. I would imagine that the end result is the same: the default ops (iommu_ops) are used. > > + if (dev->dma_ops == &iommu_ops) { > > + dev->archdata.dma_ops_setup = true; > > The existing assignment is effectively unconditional by this point anyway, so > could probably just be moved earlier to save duplicating it (or perhaps just > make the xen_setup_dma_ops() call conditional instead to save the early return > as well). > > However, are the IOMMU DMA ops really compatible with Xen? The comments about > hypercalls and foreign memory in xen_arch_need_swiotlb() leave me concerned > that assuming non-coherent DMA to any old Dom0 page is OK might not actually > work in general :/ Xen has (not yet upstreaming) support for nested IOMMU (Xen uses the IOMMU while also it exposes a virtual IOMMU to guests.) In those cases the iommu_ops should be compatible with Xen. swiotlb-xen is useful in cases where there is no IOMMU on the platform (or the IOMMU doesn't cover all DMA-capable devices) and Dom0 is 1:1 mapped. See include/xen/arm/swiotlb-xen.h:xen_swiotlb_detect. If Dom0 is not 1:1 mapped swiotlb-xen doesn't work. If an IOMMU is present and covers all DMA-capable devices, then swiotlb-xen is superfluous. This last case is the interesting case for virtual IOMMU and Linux usage of iommu_ops. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-11-14 22:20 UTC|newest] Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <20231111184538.2371-1-brchuckz.ref@aol.com> 2023-11-11 18:45 ` [PATCH] arm/mm: add option to prefer IOMMU ops for DMA on Xen Chuck Zmudzinski 2023-11-11 18:45 ` Chuck Zmudzinski 2023-11-14 13:47 ` Robin Murphy 2023-11-14 13:47 ` Robin Murphy 2023-11-14 22:20 ` Stefano Stabellini [this message] 2023-11-14 22:20 ` Stefano Stabellini 2023-11-15 17:56 ` Chuck Zmudzinski 2023-11-15 17:56 ` Chuck Zmudzinski 2023-11-15 19:41 ` Mario Marietto 2023-11-16 9:51 ` Mario Marietto 2023-11-16 15:13 ` Mario Marietto 2023-11-16 23:12 ` Stefano Stabellini 2023-11-16 23:12 ` Stefano Stabellini 2023-11-17 3:18 ` Chuck Zmudzinski 2023-11-17 3:18 ` Chuck Zmudzinski 2023-11-17 19:25 ` Fr. Chuck Zmudzinski, C.P.M. 2023-11-17 19:25 ` Fr. Chuck Zmudzinski, C.P.M. 2023-11-17 21:25 ` Stefano Stabellini 2023-11-17 21:25 ` Stefano Stabellini
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=alpine.DEB.2.22.394.2311141407140.160649@ubuntu-linux-20-04-desktop \ --to=sstabellini@kernel.org \ --cc=akpm@linux-foundation.org \ --cc=arnd@arndb.de \ --cc=bertrand.marquis@arm.com \ --cc=brchuckz@aol.com \ --cc=gregkh@linuxfoundation.org \ --cc=jgg@ziepe.ca \ --cc=jgross@suse.com \ --cc=julien@xen.org \ --cc=linus.walleij@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@armlinux.org.uk \ --cc=m.szyprowski@samsung.com \ --cc=marietto2008@gmail.com \ --cc=mathieu.poirier@linaro.org \ --cc=oleksandr_tyshchenko@epam.com \ --cc=robin.murphy@arm.com \ --cc=willy@infradead.org \ --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: linkBe 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.