| Message ID | cover.1627468308.git.robin.murphy@arm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | iommu: Refactor DMA domain strictness | expand |
Hi Robin, 在 2021/7/28 23:58, Robin Murphy 写道: > Hi all, > > Here's v2 where things start to look more realistic, hence the expanded > CC list. The patches are now based on the current iommu/core branch to > take John's iommu_set_dma_strict() cleanup into account. > > The series remiains in two (or possibly 3) logical parts - for people > CC'd on cookie cleanup patches, the later parts should not affect you > since your drivers don't implement non-strict mode anyway; the cleanup > is all pretty straightforward, but please do yell at me if I've managed > to let a silly mistake slip through and broken your driver. > > This time I have also build-tested x86 as well as arm64 :) I have tested those patchset on ARM64 with SMMUV3, and the testcases are as follows: - Boot with iommu.strict=0, running fio and it works well; - Boot with iommu.strict=1, running fio and it works well; - Change strict mode to lazy mode when building, the change takes effect; - Boot without iommu.strict(default strict mode), change the sysfs interface type from DMA to DMA-FQ dynamically during running fio, and it works well; - Boot without iommu.strict(default strict mode), change the sysfs interface type from DMA-FQ to DMA dynamically, and it is not allowed and print "Device or resource busy" (i know it is qualified, and we can change no-strict mode to strict by unbind the driver -> change the sysfs interface (type)->bind the driver (tested this and it works well), but i have a small question: is it also possible to change from DMA-FQ to DMA dynamically? ) Anyway, please feel free to add : Tested-by: Xiang Chen <chenxiang66@hisilicon.com> > > Changes in v2: > > - Add iommu_is_dma_domain() helper to abstract flag check (and help > avoid silly typos like the one in v1). > - Tweak a few commit messages for spelling and (hopefully) clarity. > - Move the iommu_create_device_direct_mappings() update to patch #14 > where it should have been. > - Rewrite patch #20 as a conversion of the now-existing option. > - Clean up the ops->flush_iotlb_all check which is also made redundant > by the new domain type > - Add patch #24, which is arguably tangential, but it was something I > spotted during the rebase, so... > > Once again, the whole lot is available on a branch here: > > https://gitlab.arm.com/linux-arm/linux-rm/-/tree/iommu/fq > > Thanks, > Robin. > > > CC: Marek Szyprowski <m.szyprowski@samsung.com> > CC: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > CC: Geert Uytterhoeven <geert+renesas@glider.be> > CC: Yong Wu <yong.wu@mediatek.com> > CC: Heiko Stuebner <heiko@sntech.de> > CC: Chunyan Zhang <chunyan.zhang@unisoc.com> > CC: Chunyan Zhang <chunyan.zhang@unisoc.com> > CC: Maxime Ripard <mripard@kernel.org> > CC: Jean-Philippe Brucker <jean-philippe@linaro.org> > > Robin Murphy (24): > iommu: Pull IOVA cookie management into the core > iommu/amd: Drop IOVA cookie management > iommu/arm-smmu: Drop IOVA cookie management > iommu/vt-d: Drop IOVA cookie management > iommu/exynos: Drop IOVA cookie management > iommu/ipmmu-vmsa: Drop IOVA cookie management > iommu/mtk: Drop IOVA cookie management > iommu/rockchip: Drop IOVA cookie management > iommu/sprd: Drop IOVA cookie management > iommu/sun50i: Drop IOVA cookie management > iommu/virtio: Drop IOVA cookie management > iommu/dma: Unexport IOVA cookie management > iommu/dma: Remove redundant "!dev" checks > iommu: Introduce explicit type for non-strict DMA domains > iommu/amd: Prepare for multiple DMA domain types > iommu/arm-smmu: Prepare for multiple DMA domain types > iommu/vt-d: Prepare for multiple DMA domain types > iommu: Express DMA strictness via the domain type > iommu: Expose DMA domain strictness via sysfs > iommu: Merge strictness and domain type configs > iommu/dma: Factor out flush queue init > iommu: Allow enabling non-strict mode dynamically > iommu/arm-smmu: Allow non-strict in pgtable_quirks interface > iommu: Only log strictness for DMA domains > > .../ABI/testing/sysfs-kernel-iommu_groups | 2 + > drivers/iommu/Kconfig | 80 +++++++++---------- > drivers/iommu/amd/iommu.c | 21 +---- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 25 ++++-- > drivers/iommu/arm/arm-smmu/arm-smmu.c | 29 ++++--- > drivers/iommu/arm/arm-smmu/qcom_iommu.c | 8 -- > drivers/iommu/dma-iommu.c | 44 +++++----- > drivers/iommu/exynos-iommu.c | 18 +---- > drivers/iommu/intel/iommu.c | 23 ++---- > drivers/iommu/iommu.c | 53 +++++++----- > drivers/iommu/ipmmu-vmsa.c | 27 +------ > drivers/iommu/mtk_iommu.c | 6 -- > drivers/iommu/rockchip-iommu.c | 11 +-- > drivers/iommu/sprd-iommu.c | 6 -- > drivers/iommu/sun50i-iommu.c | 12 +-- > drivers/iommu/virtio-iommu.c | 8 -- > include/linux/dma-iommu.h | 9 ++- > include/linux/iommu.h | 15 +++- > 18 files changed, 171 insertions(+), 226 deletions(-) >
On 2021-07-29 03:55, chenxiang (M) wrote: > Hi Robin, > > > 在 2021/7/28 23:58, Robin Murphy 写道: >> Hi all, >> >> Here's v2 where things start to look more realistic, hence the expanded >> CC list. The patches are now based on the current iommu/core branch to >> take John's iommu_set_dma_strict() cleanup into account. >> >> The series remiains in two (or possibly 3) logical parts - for people >> CC'd on cookie cleanup patches, the later parts should not affect you >> since your drivers don't implement non-strict mode anyway; the cleanup >> is all pretty straightforward, but please do yell at me if I've managed >> to let a silly mistake slip through and broken your driver. >> >> This time I have also build-tested x86 as well as arm64 :) > > I have tested those patchset on ARM64 with SMMUV3, and the testcases are > as follows: > - Boot with iommu.strict=0, running fio and it works well; > - Boot with iommu.strict=1, running fio and it works well; > - Change strict mode to lazy mode when building, the change takes effect; > - Boot without iommu.strict(default strict mode), change the sysfs > interface type from DMA to DMA-FQ dynamically during running fio, and it > works well; > - Boot without iommu.strict(default strict mode), change the sysfs > interface type from DMA-FQ to DMA dynamically, and it is not allowed and > print "Device or resource busy" > (i know it is qualified, and we can change no-strict mode to strict by > unbind the driver -> change the sysfs interface (type)->bind the driver > (tested this and it works well), > but i have a small question: is it also possible to change from DMA-FQ > to DMA dynamically? ) As patch #22 mentions, I think it's possible in principle, but it's certainly trickier. When enabling a flush queue, it doesn't matter if it takes a while for other threads to notice that cookie->fq_domain is now set and stop doing synchronous invalidations (and in the SMMU case it seems like there are probably enough dependencies to additionally prevent the io_pgtable quirk being observable before that). However when disabling, we'd need to be absolutely sure that the driver *has* started invalidating strictly before we stop queueing freed IOVAs, plus we need to be absolutely sure that we've stopped queueing freed IOVAs before we attempt to tear down the flush queue itself. I'm not sure off-hand how feasible it would be to put all that synchronisation in the right places without it also impacting normal operation. Furthermore, as also noted, there doesn't seem to be a good reason for ever actually needing to do that. If a device isn't trusted, it should be given a strict domain *before* any driver has a chance to start doing anything, or your trust model is broken and pretty useless. I can imagine some niche debugging/benchmarking cases where it might help save a bit of effort, but nothing with a strong enough justification to be worth supporting in mainline. > Anyway, please feel free to add : > Tested-by: Xiang Chen <chenxiang66@hisilicon.com> That's great, thanks! Robin. >> Changes in v2: >> >> - Add iommu_is_dma_domain() helper to abstract flag check (and help >> avoid silly typos like the one in v1). >> - Tweak a few commit messages for spelling and (hopefully) clarity. >> - Move the iommu_create_device_direct_mappings() update to patch #14 >> where it should have been. >> - Rewrite patch #20 as a conversion of the now-existing option. >> - Clean up the ops->flush_iotlb_all check which is also made redundant >> by the new domain type >> - Add patch #24, which is arguably tangential, but it was something I >> spotted during the rebase, so... >> >> Once again, the whole lot is available on a branch here: >> >> https://gitlab.arm.com/linux-arm/linux-rm/-/tree/iommu/fq >> >> Thanks, >> Robin. >> >> >> CC: Marek Szyprowski <m.szyprowski@samsung.com> >> CC: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> >> CC: Geert Uytterhoeven <geert+renesas@glider.be> >> CC: Yong Wu <yong.wu@mediatek.com> >> CC: Heiko Stuebner <heiko@sntech.de> >> CC: Chunyan Zhang <chunyan.zhang@unisoc.com> >> CC: Chunyan Zhang <chunyan.zhang@unisoc.com> >> CC: Maxime Ripard <mripard@kernel.org> >> CC: Jean-Philippe Brucker <jean-philippe@linaro.org> >> >> Robin Murphy (24): >> iommu: Pull IOVA cookie management into the core >> iommu/amd: Drop IOVA cookie management >> iommu/arm-smmu: Drop IOVA cookie management >> iommu/vt-d: Drop IOVA cookie management >> iommu/exynos: Drop IOVA cookie management >> iommu/ipmmu-vmsa: Drop IOVA cookie management >> iommu/mtk: Drop IOVA cookie management >> iommu/rockchip: Drop IOVA cookie management >> iommu/sprd: Drop IOVA cookie management >> iommu/sun50i: Drop IOVA cookie management >> iommu/virtio: Drop IOVA cookie management >> iommu/dma: Unexport IOVA cookie management >> iommu/dma: Remove redundant "!dev" checks >> iommu: Introduce explicit type for non-strict DMA domains >> iommu/amd: Prepare for multiple DMA domain types >> iommu/arm-smmu: Prepare for multiple DMA domain types >> iommu/vt-d: Prepare for multiple DMA domain types >> iommu: Express DMA strictness via the domain type >> iommu: Expose DMA domain strictness via sysfs >> iommu: Merge strictness and domain type configs >> iommu/dma: Factor out flush queue init >> iommu: Allow enabling non-strict mode dynamically >> iommu/arm-smmu: Allow non-strict in pgtable_quirks interface >> iommu: Only log strictness for DMA domains >> >> .../ABI/testing/sysfs-kernel-iommu_groups | 2 + >> drivers/iommu/Kconfig | 80 +++++++++---------- >> drivers/iommu/amd/iommu.c | 21 +---- >> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 25 ++++-- >> drivers/iommu/arm/arm-smmu/arm-smmu.c | 29 ++++--- >> drivers/iommu/arm/arm-smmu/qcom_iommu.c | 8 -- >> drivers/iommu/dma-iommu.c | 44 +++++----- >> drivers/iommu/exynos-iommu.c | 18 +---- >> drivers/iommu/intel/iommu.c | 23 ++---- >> drivers/iommu/iommu.c | 53 +++++++----- >> drivers/iommu/ipmmu-vmsa.c | 27 +------ >> drivers/iommu/mtk_iommu.c | 6 -- >> drivers/iommu/rockchip-iommu.c | 11 +-- >> drivers/iommu/sprd-iommu.c | 6 -- >> drivers/iommu/sun50i-iommu.c | 12 +-- >> drivers/iommu/virtio-iommu.c | 8 -- >> include/linux/dma-iommu.h | 9 ++- >> include/linux/iommu.h | 15 +++- >> 18 files changed, 171 insertions(+), 226 deletions(-) >> > >
Hi Robin, Am Mittwoch, 28. Juli 2021, 17:58:21 CEST schrieb Robin Murphy: > Hi all, > > Here's v2 where things start to look more realistic, hence the expanded > CC list. The patches are now based on the current iommu/core branch to > take John's iommu_set_dma_strict() cleanup into account. > > The series remiains in two (or possibly 3) logical parts - for people > CC'd on cookie cleanup patches, the later parts should not affect you > since your drivers don't implement non-strict mode anyway; the cleanup > is all pretty straightforward, but please do yell at me if I've managed > to let a silly mistake slip through and broken your driver. > > This time I have also build-tested x86 as well as arm64 :) TL;DR: arm64 yay, arm32 nay ;-) testcase: 5.14-rc3 + iommu/next + patches 1+8 (the ones you cc'd me on) iommu: Pull IOVA cookie management into the core iommu/rockchip: Drop IOVA cookie management rk3399+hdmi (puma): boots with graphics rk3399+edp (kevin): boots with graphics px30+dsi (minievb): boots with graphics rk3288 (arm32, veyron-pinky): hangs when trying to start the rockchip-drm at some points the rest of the system recovers and fills the log with [ 47.193776] [drm:drm_crtc_commit_wait] *ERROR* flip_done timed out [ 47.193867] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [PLANE:31:plane-0] commit wait timed out [ 57.433743] [drm:drm_crtc_commit_wait] *ERROR* flip_done timed out [ 57.433828] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [PLANE:40:plane-4] commit wait timed out spews testcase 2: 5.14-rc3 + iommu/next all works fine on both arm32+arm64 That whole iommu voodoo is a bit over my head right now, so I'm not sure what to poke to diagnose this. Heiko > Changes in v2: > > - Add iommu_is_dma_domain() helper to abstract flag check (and help > avoid silly typos like the one in v1). > - Tweak a few commit messages for spelling and (hopefully) clarity. > - Move the iommu_create_device_direct_mappings() update to patch #14 > where it should have been. > - Rewrite patch #20 as a conversion of the now-existing option. > - Clean up the ops->flush_iotlb_all check which is also made redundant > by the new domain type > - Add patch #24, which is arguably tangential, but it was something I > spotted during the rebase, so... > > Once again, the whole lot is available on a branch here: > > https://gitlab.arm.com/linux-arm/linux-rm/-/tree/iommu/fq > > Thanks, > Robin. > > > CC: Marek Szyprowski <m.szyprowski@samsung.com> > CC: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > CC: Geert Uytterhoeven <geert+renesas@glider.be> > CC: Yong Wu <yong.wu@mediatek.com> > CC: Heiko Stuebner <heiko@sntech.de> > CC: Chunyan Zhang <chunyan.zhang@unisoc.com> > CC: Chunyan Zhang <chunyan.zhang@unisoc.com> > CC: Maxime Ripard <mripard@kernel.org> > CC: Jean-Philippe Brucker <jean-philippe@linaro.org> > > Robin Murphy (24): > iommu: Pull IOVA cookie management into the core > iommu/amd: Drop IOVA cookie management > iommu/arm-smmu: Drop IOVA cookie management > iommu/vt-d: Drop IOVA cookie management > iommu/exynos: Drop IOVA cookie management > iommu/ipmmu-vmsa: Drop IOVA cookie management > iommu/mtk: Drop IOVA cookie management > iommu/rockchip: Drop IOVA cookie management > iommu/sprd: Drop IOVA cookie management > iommu/sun50i: Drop IOVA cookie management > iommu/virtio: Drop IOVA cookie management > iommu/dma: Unexport IOVA cookie management > iommu/dma: Remove redundant "!dev" checks > iommu: Introduce explicit type for non-strict DMA domains > iommu/amd: Prepare for multiple DMA domain types > iommu/arm-smmu: Prepare for multiple DMA domain types > iommu/vt-d: Prepare for multiple DMA domain types > iommu: Express DMA strictness via the domain type > iommu: Expose DMA domain strictness via sysfs > iommu: Merge strictness and domain type configs > iommu/dma: Factor out flush queue init > iommu: Allow enabling non-strict mode dynamically > iommu/arm-smmu: Allow non-strict in pgtable_quirks interface > iommu: Only log strictness for DMA domains > > .../ABI/testing/sysfs-kernel-iommu_groups | 2 + > drivers/iommu/Kconfig | 80 +++++++++---------- > drivers/iommu/amd/iommu.c | 21 +---- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 25 ++++-- > drivers/iommu/arm/arm-smmu/arm-smmu.c | 29 ++++--- > drivers/iommu/arm/arm-smmu/qcom_iommu.c | 8 -- > drivers/iommu/dma-iommu.c | 44 +++++----- > drivers/iommu/exynos-iommu.c | 18 +---- > drivers/iommu/intel/iommu.c | 23 ++---- > drivers/iommu/iommu.c | 53 +++++++----- > drivers/iommu/ipmmu-vmsa.c | 27 +------ > drivers/iommu/mtk_iommu.c | 6 -- > drivers/iommu/rockchip-iommu.c | 11 +-- > drivers/iommu/sprd-iommu.c | 6 -- > drivers/iommu/sun50i-iommu.c | 12 +-- > drivers/iommu/virtio-iommu.c | 8 -- > include/linux/dma-iommu.h | 9 ++- > include/linux/iommu.h | 15 +++- > 18 files changed, 171 insertions(+), 226 deletions(-) > >
On 2021-07-29 16:04, Heiko Stübner wrote: > Hi Robin, > > Am Mittwoch, 28. Juli 2021, 17:58:21 CEST schrieb Robin Murphy: >> Hi all, >> >> Here's v2 where things start to look more realistic, hence the expanded >> CC list. The patches are now based on the current iommu/core branch to >> take John's iommu_set_dma_strict() cleanup into account. >> >> The series remiains in two (or possibly 3) logical parts - for people >> CC'd on cookie cleanup patches, the later parts should not affect you >> since your drivers don't implement non-strict mode anyway; the cleanup >> is all pretty straightforward, but please do yell at me if I've managed >> to let a silly mistake slip through and broken your driver. >> >> This time I have also build-tested x86 as well as arm64 :) > > TL;DR: arm64 yay, arm32 nay ;-) Cheers Heiko! > testcase: > 5.14-rc3 > + iommu/next > + patches 1+8 (the ones you cc'd me on) > iommu: Pull IOVA cookie management into the core > iommu/rockchip: Drop IOVA cookie management > > rk3399+hdmi (puma): boots with graphics > rk3399+edp (kevin): boots with graphics > px30+dsi (minievb): boots with graphics > > rk3288 (arm32, veyron-pinky): hangs when trying to start the rockchip-drm > at some points the rest of the system recovers and fills the log with > > [ 47.193776] [drm:drm_crtc_commit_wait] *ERROR* flip_done timed out > [ 47.193867] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [PLANE:31:plane-0] commit wait timed out > [ 57.433743] [drm:drm_crtc_commit_wait] *ERROR* flip_done timed out > [ 57.433828] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [PLANE:40:plane-4] commit wait timed out > > spews > > testcase 2: > 5.14-rc3 > + iommu/next > > all works fine on both arm32+arm64 > > > That whole iommu voodoo is a bit over my head right now, so I'm not sure > what to poke to diagnose this. Dang, this wasn't supposed to affect 32-bit Arm at all, since that doesn't touch any of the default domain stuff either way. I have both my RK3288 box (which IIRC doesn't currently boot) and an Odroid-U3 in the "desk pile" right in front of me, so at worst I'll try bringing one of those to life to see what silly thing I have indeed done to break 32-bit. I have a vague idea forming already, which suggests that it might get better again once patch #12 is applied, but even if so there's no excuse not to be bisectable, so I need to dig in and fix it - many thanks for yelling as requested :D Robin. > > > Heiko > > >> Changes in v2: >> >> - Add iommu_is_dma_domain() helper to abstract flag check (and help >> avoid silly typos like the one in v1). >> - Tweak a few commit messages for spelling and (hopefully) clarity. >> - Move the iommu_create_device_direct_mappings() update to patch #14 >> where it should have been. >> - Rewrite patch #20 as a conversion of the now-existing option. >> - Clean up the ops->flush_iotlb_all check which is also made redundant >> by the new domain type >> - Add patch #24, which is arguably tangential, but it was something I >> spotted during the rebase, so... >> >> Once again, the whole lot is available on a branch here: >> >> https://gitlab.arm.com/linux-arm/linux-rm/-/tree/iommu/fq >> >> Thanks, >> Robin. >> >> >> CC: Marek Szyprowski <m.szyprowski@samsung.com> >> CC: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> >> CC: Geert Uytterhoeven <geert+renesas@glider.be> >> CC: Yong Wu <yong.wu@mediatek.com> >> CC: Heiko Stuebner <heiko@sntech.de> >> CC: Chunyan Zhang <chunyan.zhang@unisoc.com> >> CC: Chunyan Zhang <chunyan.zhang@unisoc.com> >> CC: Maxime Ripard <mripard@kernel.org> >> CC: Jean-Philippe Brucker <jean-philippe@linaro.org> >> >> Robin Murphy (24): >> iommu: Pull IOVA cookie management into the core >> iommu/amd: Drop IOVA cookie management >> iommu/arm-smmu: Drop IOVA cookie management >> iommu/vt-d: Drop IOVA cookie management >> iommu/exynos: Drop IOVA cookie management >> iommu/ipmmu-vmsa: Drop IOVA cookie management >> iommu/mtk: Drop IOVA cookie management >> iommu/rockchip: Drop IOVA cookie management >> iommu/sprd: Drop IOVA cookie management >> iommu/sun50i: Drop IOVA cookie management >> iommu/virtio: Drop IOVA cookie management >> iommu/dma: Unexport IOVA cookie management >> iommu/dma: Remove redundant "!dev" checks >> iommu: Introduce explicit type for non-strict DMA domains >> iommu/amd: Prepare for multiple DMA domain types >> iommu/arm-smmu: Prepare for multiple DMA domain types >> iommu/vt-d: Prepare for multiple DMA domain types >> iommu: Express DMA strictness via the domain type >> iommu: Expose DMA domain strictness via sysfs >> iommu: Merge strictness and domain type configs >> iommu/dma: Factor out flush queue init >> iommu: Allow enabling non-strict mode dynamically >> iommu/arm-smmu: Allow non-strict in pgtable_quirks interface >> iommu: Only log strictness for DMA domains >> >> .../ABI/testing/sysfs-kernel-iommu_groups | 2 + >> drivers/iommu/Kconfig | 80 +++++++++---------- >> drivers/iommu/amd/iommu.c | 21 +---- >> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 25 ++++-- >> drivers/iommu/arm/arm-smmu/arm-smmu.c | 29 ++++--- >> drivers/iommu/arm/arm-smmu/qcom_iommu.c | 8 -- >> drivers/iommu/dma-iommu.c | 44 +++++----- >> drivers/iommu/exynos-iommu.c | 18 +---- >> drivers/iommu/intel/iommu.c | 23 ++---- >> drivers/iommu/iommu.c | 53 +++++++----- >> drivers/iommu/ipmmu-vmsa.c | 27 +------ >> drivers/iommu/mtk_iommu.c | 6 -- >> drivers/iommu/rockchip-iommu.c | 11 +-- >> drivers/iommu/sprd-iommu.c | 6 -- >> drivers/iommu/sun50i-iommu.c | 12 +-- >> drivers/iommu/virtio-iommu.c | 8 -- >> include/linux/dma-iommu.h | 9 ++- >> include/linux/iommu.h | 15 +++- >> 18 files changed, 171 insertions(+), 226 deletions(-) >> >> > > > >
Am Donnerstag, 29. Juli 2021, 17:43:07 CEST schrieb Robin Murphy: > On 2021-07-29 16:04, Heiko Stübner wrote: > > Hi Robin, > > > > Am Mittwoch, 28. Juli 2021, 17:58:21 CEST schrieb Robin Murphy: > >> Hi all, > >> > >> Here's v2 where things start to look more realistic, hence the expanded > >> CC list. The patches are now based on the current iommu/core branch to > >> take John's iommu_set_dma_strict() cleanup into account. > >> > >> The series remiains in two (or possibly 3) logical parts - for people > >> CC'd on cookie cleanup patches, the later parts should not affect you > >> since your drivers don't implement non-strict mode anyway; the cleanup > >> is all pretty straightforward, but please do yell at me if I've managed > >> to let a silly mistake slip through and broken your driver. > >> > >> This time I have also build-tested x86 as well as arm64 :) > > > > TL;DR: arm64 yay, arm32 nay ;-) > > Cheers Heiko! > > > testcase: > > 5.14-rc3 > > + iommu/next > > + patches 1+8 (the ones you cc'd me on) > > iommu: Pull IOVA cookie management into the core > > iommu/rockchip: Drop IOVA cookie management > > > > rk3399+hdmi (puma): boots with graphics > > rk3399+edp (kevin): boots with graphics > > px30+dsi (minievb): boots with graphics > > > > rk3288 (arm32, veyron-pinky): hangs when trying to start the rockchip-drm > > at some points the rest of the system recovers and fills the log with > > > > [ 47.193776] [drm:drm_crtc_commit_wait] *ERROR* flip_done timed out > > [ 47.193867] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [PLANE:31:plane-0] commit wait timed out > > [ 57.433743] [drm:drm_crtc_commit_wait] *ERROR* flip_done timed out > > [ 57.433828] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [PLANE:40:plane-4] commit wait timed out > > > > spews > > > > testcase 2: > > 5.14-rc3 > > + iommu/next > > > > all works fine on both arm32+arm64 > > > > > > That whole iommu voodoo is a bit over my head right now, so I'm not sure > > what to poke to diagnose this. > > Dang, this wasn't supposed to affect 32-bit Arm at all, since that > doesn't touch any of the default domain stuff either way. I have both my > RK3288 box (which IIRC doesn't currently boot) and an Odroid-U3 in the > "desk pile" right in front of me, so at worst I'll try bringing one of > those to life to see what silly thing I have indeed done to break 32-bit. > > I have a vague idea forming already, which suggests that it might get > better again once patch #12 is applied, but even if so there's no excuse > not to be bisectable, so I need to dig in and fix it - many thanks for > yelling as requested :D That vague idea was actually quite correct, applying iommu/dma: Unexport IOVA cookie management on top of the the two patches makes my rk3288 boot correctly again and the display also works again. Heiko > > Robin. > > > > > > > Heiko > > > > > >> Changes in v2: > >> > >> - Add iommu_is_dma_domain() helper to abstract flag check (and help > >> avoid silly typos like the one in v1). > >> - Tweak a few commit messages for spelling and (hopefully) clarity. > >> - Move the iommu_create_device_direct_mappings() update to patch #14 > >> where it should have been. > >> - Rewrite patch #20 as a conversion of the now-existing option. > >> - Clean up the ops->flush_iotlb_all check which is also made redundant > >> by the new domain type > >> - Add patch #24, which is arguably tangential, but it was something I > >> spotted during the rebase, so... > >> > >> Once again, the whole lot is available on a branch here: > >> > >> https://gitlab.arm.com/linux-arm/linux-rm/-/tree/iommu/fq > >> > >> Thanks, > >> Robin. > >> > >> > >> CC: Marek Szyprowski <m.szyprowski@samsung.com> > >> CC: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > >> CC: Geert Uytterhoeven <geert+renesas@glider.be> > >> CC: Yong Wu <yong.wu@mediatek.com> > >> CC: Heiko Stuebner <heiko@sntech.de> > >> CC: Chunyan Zhang <chunyan.zhang@unisoc.com> > >> CC: Chunyan Zhang <chunyan.zhang@unisoc.com> > >> CC: Maxime Ripard <mripard@kernel.org> > >> CC: Jean-Philippe Brucker <jean-philippe@linaro.org> > >> > >> Robin Murphy (24): > >> iommu: Pull IOVA cookie management into the core > >> iommu/amd: Drop IOVA cookie management > >> iommu/arm-smmu: Drop IOVA cookie management > >> iommu/vt-d: Drop IOVA cookie management > >> iommu/exynos: Drop IOVA cookie management > >> iommu/ipmmu-vmsa: Drop IOVA cookie management > >> iommu/mtk: Drop IOVA cookie management > >> iommu/rockchip: Drop IOVA cookie management > >> iommu/sprd: Drop IOVA cookie management > >> iommu/sun50i: Drop IOVA cookie management > >> iommu/virtio: Drop IOVA cookie management > >> iommu/dma: Unexport IOVA cookie management > >> iommu/dma: Remove redundant "!dev" checks > >> iommu: Introduce explicit type for non-strict DMA domains > >> iommu/amd: Prepare for multiple DMA domain types > >> iommu/arm-smmu: Prepare for multiple DMA domain types > >> iommu/vt-d: Prepare for multiple DMA domain types > >> iommu: Express DMA strictness via the domain type > >> iommu: Expose DMA domain strictness via sysfs > >> iommu: Merge strictness and domain type configs > >> iommu/dma: Factor out flush queue init > >> iommu: Allow enabling non-strict mode dynamically > >> iommu/arm-smmu: Allow non-strict in pgtable_quirks interface > >> iommu: Only log strictness for DMA domains > >> > >> .../ABI/testing/sysfs-kernel-iommu_groups | 2 + > >> drivers/iommu/Kconfig | 80 +++++++++---------- > >> drivers/iommu/amd/iommu.c | 21 +---- > >> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 25 ++++-- > >> drivers/iommu/arm/arm-smmu/arm-smmu.c | 29 ++++--- > >> drivers/iommu/arm/arm-smmu/qcom_iommu.c | 8 -- > >> drivers/iommu/dma-iommu.c | 44 +++++----- > >> drivers/iommu/exynos-iommu.c | 18 +---- > >> drivers/iommu/intel/iommu.c | 23 ++---- > >> drivers/iommu/iommu.c | 53 +++++++----- > >> drivers/iommu/ipmmu-vmsa.c | 27 +------ > >> drivers/iommu/mtk_iommu.c | 6 -- > >> drivers/iommu/rockchip-iommu.c | 11 +-- > >> drivers/iommu/sprd-iommu.c | 6 -- > >> drivers/iommu/sun50i-iommu.c | 12 +-- > >> drivers/iommu/virtio-iommu.c | 8 -- > >> include/linux/dma-iommu.h | 9 ++- > >> include/linux/iommu.h | 15 +++- > >> 18 files changed, 171 insertions(+), 226 deletions(-) > >> > >> > > > > > > > > >
On 2021-07-29 16:53, Heiko Stübner wrote: > Am Donnerstag, 29. Juli 2021, 17:43:07 CEST schrieb Robin Murphy: >> On 2021-07-29 16:04, Heiko Stübner wrote: >>> Hi Robin, >>> >>> Am Mittwoch, 28. Juli 2021, 17:58:21 CEST schrieb Robin Murphy: >>>> Hi all, >>>> >>>> Here's v2 where things start to look more realistic, hence the expanded >>>> CC list. The patches are now based on the current iommu/core branch to >>>> take John's iommu_set_dma_strict() cleanup into account. >>>> >>>> The series remiains in two (or possibly 3) logical parts - for people >>>> CC'd on cookie cleanup patches, the later parts should not affect you >>>> since your drivers don't implement non-strict mode anyway; the cleanup >>>> is all pretty straightforward, but please do yell at me if I've managed >>>> to let a silly mistake slip through and broken your driver. >>>> >>>> This time I have also build-tested x86 as well as arm64 :) >>> >>> TL;DR: arm64 yay, arm32 nay ;-) >> >> Cheers Heiko! >> >>> testcase: >>> 5.14-rc3 >>> + iommu/next >>> + patches 1+8 (the ones you cc'd me on) >>> iommu: Pull IOVA cookie management into the core >>> iommu/rockchip: Drop IOVA cookie management >>> >>> rk3399+hdmi (puma): boots with graphics >>> rk3399+edp (kevin): boots with graphics >>> px30+dsi (minievb): boots with graphics >>> >>> rk3288 (arm32, veyron-pinky): hangs when trying to start the rockchip-drm >>> at some points the rest of the system recovers and fills the log with >>> >>> [ 47.193776] [drm:drm_crtc_commit_wait] *ERROR* flip_done timed out >>> [ 47.193867] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [PLANE:31:plane-0] commit wait timed out >>> [ 57.433743] [drm:drm_crtc_commit_wait] *ERROR* flip_done timed out >>> [ 57.433828] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [PLANE:40:plane-4] commit wait timed out >>> >>> spews >>> >>> testcase 2: >>> 5.14-rc3 >>> + iommu/next >>> >>> all works fine on both arm32+arm64 >>> >>> >>> That whole iommu voodoo is a bit over my head right now, so I'm not sure >>> what to poke to diagnose this. >> >> Dang, this wasn't supposed to affect 32-bit Arm at all, since that >> doesn't touch any of the default domain stuff either way. I have both my >> RK3288 box (which IIRC doesn't currently boot) and an Odroid-U3 in the >> "desk pile" right in front of me, so at worst I'll try bringing one of >> those to life to see what silly thing I have indeed done to break 32-bit. >> >> I have a vague idea forming already, which suggests that it might get >> better again once patch #12 is applied, but even if so there's no excuse >> not to be bisectable, so I need to dig in and fix it - many thanks for >> yelling as requested :D > > That vague idea was actually quite correct, applying > iommu/dma: Unexport IOVA cookie management > on top of the the two patches makes my rk3288 boot correctly again > and the display also works again. Yup, since the !CONFIG_IOMMU_DMA stub for iommu_get_dma_cookie() returns -ENODEV, rather than the -ENOMEM that the temporary special case is expecting from the real function, it will inadvertently allow the default domain to be created (when it wasn't before). I still have no idea why that causes a problem though, since arm_iommu_attach_device() should end up kicking a default domain out of the way even if one does exist... :/ Either way I'll fix my bug - indeed it was an oversight that I hadn't considered which exact error code the stub "fails" with - to avoid the temporary change in behaviour, but I'll have to keep digging into the arch/arm code and rockchip-iommu to see if something's also off there. Cheers, Robin.
Hi, On Wed, Jul 28, 2021 at 8:59 AM Robin Murphy <robin.murphy@arm.com> wrote: > > Hi all, > > Here's v2 where things start to look more realistic, hence the expanded > CC list. The patches are now based on the current iommu/core branch to > take John's iommu_set_dma_strict() cleanup into account. > > The series remiains in two (or possibly 3) logical parts - for people > CC'd on cookie cleanup patches, the later parts should not affect you > since your drivers don't implement non-strict mode anyway; the cleanup > is all pretty straightforward, but please do yell at me if I've managed > to let a silly mistake slip through and broken your driver. > > This time I have also build-tested x86 as well as arm64 :) > > Changes in v2: > > - Add iommu_is_dma_domain() helper to abstract flag check (and help > avoid silly typos like the one in v1). > - Tweak a few commit messages for spelling and (hopefully) clarity. > - Move the iommu_create_device_direct_mappings() update to patch #14 > where it should have been. > - Rewrite patch #20 as a conversion of the now-existing option. > - Clean up the ops->flush_iotlb_all check which is also made redundant > by the new domain type > - Add patch #24, which is arguably tangential, but it was something I > spotted during the rebase, so... > > Once again, the whole lot is available on a branch here: > > https://gitlab.arm.com/linux-arm/linux-rm/-/tree/iommu/fq > > Thanks, > Robin. > > > CC: Marek Szyprowski <m.szyprowski@samsung.com> > CC: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > CC: Geert Uytterhoeven <geert+renesas@glider.be> > CC: Yong Wu <yong.wu@mediatek.com> > CC: Heiko Stuebner <heiko@sntech.de> > CC: Chunyan Zhang <chunyan.zhang@unisoc.com> > CC: Chunyan Zhang <chunyan.zhang@unisoc.com> > CC: Maxime Ripard <mripard@kernel.org> > CC: Jean-Philippe Brucker <jean-philippe@linaro.org> > > Robin Murphy (24): > iommu: Pull IOVA cookie management into the core > iommu/amd: Drop IOVA cookie management > iommu/arm-smmu: Drop IOVA cookie management > iommu/vt-d: Drop IOVA cookie management > iommu/exynos: Drop IOVA cookie management > iommu/ipmmu-vmsa: Drop IOVA cookie management > iommu/mtk: Drop IOVA cookie management > iommu/rockchip: Drop IOVA cookie management > iommu/sprd: Drop IOVA cookie management > iommu/sun50i: Drop IOVA cookie management > iommu/virtio: Drop IOVA cookie management > iommu/dma: Unexport IOVA cookie management > iommu/dma: Remove redundant "!dev" checks > iommu: Introduce explicit type for non-strict DMA domains > iommu/amd: Prepare for multiple DMA domain types > iommu/arm-smmu: Prepare for multiple DMA domain types > iommu/vt-d: Prepare for multiple DMA domain types > iommu: Express DMA strictness via the domain type > iommu: Expose DMA domain strictness via sysfs > iommu: Merge strictness and domain type configs > iommu/dma: Factor out flush queue init > iommu: Allow enabling non-strict mode dynamically > iommu/arm-smmu: Allow non-strict in pgtable_quirks interface > iommu: Only log strictness for DMA domains > > .../ABI/testing/sysfs-kernel-iommu_groups | 2 + > drivers/iommu/Kconfig | 80 +++++++++---------- > drivers/iommu/amd/iommu.c | 21 +---- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 25 ++++-- > drivers/iommu/arm/arm-smmu/arm-smmu.c | 29 ++++--- > drivers/iommu/arm/arm-smmu/qcom_iommu.c | 8 -- > drivers/iommu/dma-iommu.c | 44 +++++----- > drivers/iommu/exynos-iommu.c | 18 +---- > drivers/iommu/intel/iommu.c | 23 ++---- > drivers/iommu/iommu.c | 53 +++++++----- > drivers/iommu/ipmmu-vmsa.c | 27 +------ > drivers/iommu/mtk_iommu.c | 6 -- > drivers/iommu/rockchip-iommu.c | 11 +-- > drivers/iommu/sprd-iommu.c | 6 -- > drivers/iommu/sun50i-iommu.c | 12 +-- > drivers/iommu/virtio-iommu.c | 8 -- > include/linux/dma-iommu.h | 9 ++- > include/linux/iommu.h | 15 +++- > 18 files changed, 171 insertions(+), 226 deletions(-) I ran with: a) mainline Linux (at commit 4010a528219e) b) pulled iommu/next (at commit 9be9f5580ab6) c) picked from patchwork your series ...and I ran on sc7180-trogdor-lazor. Things worked OK and I could transition my eMMC to non-strict mode with: echo DMA-FQ > /sys/devices/platform/soc@0/7c4000.sdhci/iommu_group/type I was definitely getting some inconsistencies in my tests where the eMMC speeds were getting into a bad state, but I don't believe it's related to your patch series. I could transition myself back to strict DMA with this (only got one unrelated warn splat about dev_pm_opp_put_clkname when unbinding) because I was booted up from USB for testing: cd /sys/bus/mmc/drivers/mmcblk echo mmc1:0001 > unbind cd /sys/bus/platform/drivers/sdhci_msm/ echo 7c4000.sdhci > unbind echo DMA > /sys/devices/platform/soc@0/7c4000.sdhci/iommu_group/type echo 7c4000.sdhci > bind ...and it was consistently faster with non-strict than with strict so whatever bad state I sometimes managed to get in it affected both modes. ;-) So I guess that's a long-winded way to say this: Tested-by: Douglas Anderson <dianders@chromium.org>
Hi, On Thu, Jul 29, 2021 at 3:33 PM Doug Anderson <dianders@chromium.org> wrote: > > I was definitely getting some inconsistencies in my tests where the > eMMC speeds were getting into a bad state, but I don't believe it's > related to your patch series. I think this was just me being an idiot. I forgot that I'd been running with KASAN, so that explains why my speeds were so much slower than usual and probably also explains how it could get in a bad state (I guess it also explains why sugov was eating up 30% of my CPU time since that went away too!). No mystery here aside from why it took me this long to realize it. I'm now getting ~213 MB/s without forcing it to lazy and ~261 MB/s with forcing it to lazy through sysfs (and without any other cpufreq hacks). -Doug
在 2021/7/29 18:59, Robin Murphy 写道: > On 2021-07-29 03:55, chenxiang (M) wrote: >> Hi Robin, >> >> >> 在 2021/7/28 23:58, Robin Murphy 写道: >>> Hi all, >>> >>> Here's v2 where things start to look more realistic, hence the expanded >>> CC list. The patches are now based on the current iommu/core branch to >>> take John's iommu_set_dma_strict() cleanup into account. >>> >>> The series remiains in two (or possibly 3) logical parts - for people >>> CC'd on cookie cleanup patches, the later parts should not affect you >>> since your drivers don't implement non-strict mode anyway; the cleanup >>> is all pretty straightforward, but please do yell at me if I've managed >>> to let a silly mistake slip through and broken your driver. >>> >>> This time I have also build-tested x86 as well as arm64 :) >> >> I have tested those patchset on ARM64 with SMMUV3, and the testcases >> are as follows: >> - Boot with iommu.strict=0, running fio and it works well; >> - Boot with iommu.strict=1, running fio and it works well; >> - Change strict mode to lazy mode when building, the change takes >> effect; >> - Boot without iommu.strict(default strict mode), change the sysfs >> interface type from DMA to DMA-FQ dynamically during running fio, and >> it works well; >> - Boot without iommu.strict(default strict mode), change the sysfs >> interface type from DMA-FQ to DMA dynamically, and it is not allowed >> and print "Device or resource busy" >> (i know it is qualified, and we can change no-strict mode to strict >> by unbind the driver -> change the sysfs interface (type)->bind the >> driver (tested this and it works well), >> but i have a small question: is it also possible to change from >> DMA-FQ to DMA dynamically? ) > > As patch #22 mentions, I think it's possible in principle, but it's > certainly trickier. When enabling a flush queue, it doesn't matter if > it takes a while for other threads to notice that cookie->fq_domain is > now set and stop doing synchronous invalidations (and in the SMMU case > it seems like there are probably enough dependencies to additionally > prevent the io_pgtable quirk being observable before that). However > when disabling, we'd need to be absolutely sure that the driver *has* > started invalidating strictly before we stop queueing freed IOVAs, > plus we need to be absolutely sure that we've stopped queueing freed > IOVAs before we attempt to tear down the flush queue itself. I'm not > sure off-hand how feasible it would be to put all that synchronisation > in the right places without it also impacting normal operation. > > Furthermore, as also noted, there doesn't seem to be a good reason for > ever actually needing to do that. If a device isn't trusted, it should > be given a strict domain *before* any driver has a chance to start > doing anything, or your trust model is broken and pretty useless. I > can imagine some niche debugging/benchmarking cases where it might > help save a bit of effort, but nothing with a strong enough > justification to be worth supporting in mainline. Ok, thanks. > >> Anyway, please feel free to add : >> Tested-by: Xiang Chen <chenxiang66@hisilicon.com> > > That's great, thanks! > > Robin. > >>> Changes in v2: >>> >>> - Add iommu_is_dma_domain() helper to abstract flag check (and help >>> avoid silly typos like the one in v1). >>> - Tweak a few commit messages for spelling and (hopefully) clarity. >>> - Move the iommu_create_device_direct_mappings() update to patch #14 >>> where it should have been. >>> - Rewrite patch #20 as a conversion of the now-existing option. >>> - Clean up the ops->flush_iotlb_all check which is also made redundant >>> by the new domain type >>> - Add patch #24, which is arguably tangential, but it was something I >>> spotted during the rebase, so... >>> >>> Once again, the whole lot is available on a branch here: >>> >>> https://gitlab.arm.com/linux-arm/linux-rm/-/tree/iommu/fq >>> >>> Thanks, >>> Robin. >>> >>> >>> CC: Marek Szyprowski <m.szyprowski@samsung.com> >>> CC: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> >>> CC: Geert Uytterhoeven <geert+renesas@glider.be> >>> CC: Yong Wu <yong.wu@mediatek.com> >>> CC: Heiko Stuebner <heiko@sntech.de> >>> CC: Chunyan Zhang <chunyan.zhang@unisoc.com> >>> CC: Chunyan Zhang <chunyan.zhang@unisoc.com> >>> CC: Maxime Ripard <mripard@kernel.org> >>> CC: Jean-Philippe Brucker <jean-philippe@linaro.org> >>> >>> Robin Murphy (24): >>> iommu: Pull IOVA cookie management into the core >>> iommu/amd: Drop IOVA cookie management >>> iommu/arm-smmu: Drop IOVA cookie management >>> iommu/vt-d: Drop IOVA cookie management >>> iommu/exynos: Drop IOVA cookie management >>> iommu/ipmmu-vmsa: Drop IOVA cookie management >>> iommu/mtk: Drop IOVA cookie management >>> iommu/rockchip: Drop IOVA cookie management >>> iommu/sprd: Drop IOVA cookie management >>> iommu/sun50i: Drop IOVA cookie management >>> iommu/virtio: Drop IOVA cookie management >>> iommu/dma: Unexport IOVA cookie management >>> iommu/dma: Remove redundant "!dev" checks >>> iommu: Introduce explicit type for non-strict DMA domains >>> iommu/amd: Prepare for multiple DMA domain types >>> iommu/arm-smmu: Prepare for multiple DMA domain types >>> iommu/vt-d: Prepare for multiple DMA domain types >>> iommu: Express DMA strictness via the domain type >>> iommu: Expose DMA domain strictness via sysfs >>> iommu: Merge strictness and domain type configs >>> iommu/dma: Factor out flush queue init >>> iommu: Allow enabling non-strict mode dynamically >>> iommu/arm-smmu: Allow non-strict in pgtable_quirks interface >>> iommu: Only log strictness for DMA domains >>> >>> .../ABI/testing/sysfs-kernel-iommu_groups | 2 + >>> drivers/iommu/Kconfig | 80 >>> +++++++++---------- >>> drivers/iommu/amd/iommu.c | 21 +---- >>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 25 ++++-- >>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 29 ++++--- >>> drivers/iommu/arm/arm-smmu/qcom_iommu.c | 8 -- >>> drivers/iommu/dma-iommu.c | 44 +++++----- >>> drivers/iommu/exynos-iommu.c | 18 +---- >>> drivers/iommu/intel/iommu.c | 23 ++---- >>> drivers/iommu/iommu.c | 53 +++++++----- >>> drivers/iommu/ipmmu-vmsa.c | 27 +------ >>> drivers/iommu/mtk_iommu.c | 6 -- >>> drivers/iommu/rockchip-iommu.c | 11 +-- >>> drivers/iommu/sprd-iommu.c | 6 -- >>> drivers/iommu/sun50i-iommu.c | 12 +-- >>> drivers/iommu/virtio-iommu.c | 8 -- >>> include/linux/dma-iommu.h | 9 ++- >>> include/linux/iommu.h | 15 +++- >>> 18 files changed, 171 insertions(+), 226 deletions(-) >>> >> >> > > . >