From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-04 17:16:07
v1: https://lore.kernel.org/linux-iommu/cover.1626888444.git.robin.murphy@arm.com/
v2: https://lore.kernel.org/linux-iommu/cover.1627468308.git.robin.murphy@arm.com/
Hi all,
Round 3, and the patch count has crept up yet again. But the overall
diffstat is even more negative, so that's good, right? :)
Once again, to driver/platform maintainers CC'd on cookie cleanup
patches this is just a heads-up and the rest of the changes should not
affect your platforms. I hope I've now fixed the silly bug which broke
bisection between patches #1 and #12 on 32-bit Arm.
The new patches are in the middle, reworking how the SMMU drivers and
io-pgtable implement non-strict mode such that the later changes fall
into place even more easily. Turns out I didn't need the major
refactoring of io-pgtable that I had in mind, and I'm almost kicking
myself that as soon as I put the option of *not* using the existing
quirk on the table, an even cleaner and more logical solution was
staring right out at me.
Due to that signifcant change and the consequent redesign of the final
patch to make dynamic switching look viable in the face of concurrency,
I have not applied the tested-by tags from v2. They were very much
appreciated though, thanks!
Proper changelogs on the individual patches this time since otherwise
I'd have lost track...
Cheers,
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 <redacted>
CC: Maxime Ripard <mripard@kernel.org>
CC: Jean-Philippe Brucker <redacted>
CC: Sai Praneeth Prakhya <redacted>
Robin Murphy (25):
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: Indicate queued flushes via gather data
iommu/io-pgtable: Remove non-strict quirk
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: Only log strictness for DMA domains
iommu: Merge strictness and domain type configs
iommu/dma: Factor out flush queue init
iommu: Allow enabling non-strict mode dynamically
.../ABI/testing/sysfs-kernel-iommu_groups | 6 +-
.../admin-guide/kernel-parameters.txt | 8 +-
drivers/iommu/Kconfig | 80 +++++++++----------
drivers/iommu/amd/iommu.c | 22 +----
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 11 +--
rivers/iommu/arm/arm-smmu/arm-smmu.c | 19 ++---
drivers/iommu/arm/arm-smmu/qcom_iommu.c | 9 ---
drivers/iommu/dma-iommu.c | 63 +++++++++------
drivers/iommu/exynos-iommu.c | 19 +----
drivers/iommu/intel/iommu.c | 23 ++----
drivers/iommu/io-pgtable-arm-v7s.c | 12 +--
drivers/iommu/io-pgtable-arm.c | 12 +--
drivers/iommu/iommu.c | 55 ++++++++-----
drivers/iommu/iova.c | 12 ++-
drivers/iommu/ipmmu-vmsa.c | 28 +------
drivers/iommu/mtk_iommu.c | 7 --
drivers/iommu/mtk_iommu_v1.c | 1 -
drivers/iommu/rockchip-iommu.c | 12 +--
drivers/iommu/sprd-iommu.c | 7 --
drivers/iommu/sun50i-iommu.c | 13 +--
drivers/iommu/virtio-iommu.c | 8 --
include/linux/dma-iommu.h | 9 ++-
include/linux/io-pgtable.h | 5 --
include/linux/iommu.h | 23 +++++-
24 files changed, 187 insertions(+), 277 deletions(-)
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-04 17:16:14
Now that everyone has converged on iommu-dma for IOMMU_DOMAIN_DMA
support, we can abandon the notion of drivers being responsible for the
cookie type, and consolidate all the management into the core code.
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 <redacted>
CC: Maxime Ripard <mripard@kernel.org>
Reviewed-by: Jean-Philippe Brucker <redacted>
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v3: Use a simpler temporary check instead of trying to be clever with
the error code
---
drivers/iommu/iommu.c | 7 +++++++
include/linux/iommu.h | 3 ++-
2 files changed, 9 insertions(+), 1 deletion(-)
@@ -1946,6 +1947,11 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,/* Assume all sizes by default; the driver may override this later */domain->pgsize_bitmap=bus->iommu_ops->pgsize_bitmap;+/* Temporarily avoid -EEXIST while drivers still get their own cookies */+if(type==IOMMU_DOMAIN_DMA&&!domain->iova_cookie&&iommu_get_dma_cookie(domain)){+iommu_domain_free(domain);+domain=NULL;+}returndomain;}
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-04 17:16:16
The core code bakes its own cookies now.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v3: Also remove unneeded include
---
drivers/iommu/amd/iommu.c | 13 -------------
1 file changed, 13 deletions(-)
@@ -2021,7 +2015,6 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)structarm_smmu_domain*smmu_domain=to_smmu_domain(domain);structarm_smmu_device*smmu=smmu_domain->smmu;-iommu_put_dma_cookie(domain);free_io_pgtable_ops(smmu_domain->pgtbl_ops);/* Free the CD and ASID, if we allocated them */
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-04 17:16:25
The core code bakes its own cookies now.
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/intel/iommu.c | 8 --------
1 file changed, 8 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-04 17:16:30
The core code bakes its own cookies now.
CC: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v3: Also remove unneeded include
---
drivers/iommu/exynos-iommu.c | 19 ++++---------------
1 file changed, 4 insertions(+), 15 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-04 17:16:42
The core code bakes its own cookies now.
CC: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v3: Also remove unneeded include
---
drivers/iommu/rockchip-iommu.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-04 17:16:44
The core code bakes its own cookies now.
CC: Chunyan Zhang <redacted>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v3: Also remove unneeded include
---
drivers/iommu/sprd-iommu.c | 7 -------
1 file changed, 7 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-04 17:16:45
The core code bakes its own cookies now.
CC: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v3: Also remove unneeded include
---
drivers/iommu/sun50i-iommu.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-04 17:16:54
iommu_dma_init_domain() is now only called from iommu_setup_dma_ops(),
which has already assumed dev to be non-NULL.
Reviewed-by: John Garry <redacted>
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/dma-iommu.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-04 17:16:56
IOVA cookies are now got and put by core code, so we no longer need to
export these to modular drivers. The export for getting MSI cookies
stays, since VFIO can still be a module, but it was already relying on
someone else putting them, so that aspect is unaffected.
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Jean-Philippe Brucker <redacted>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/dma-iommu.c | 7 -------
drivers/iommu/iommu.c | 3 +--
2 files changed, 1 insertion(+), 9 deletions(-)
@@ -1947,8 +1947,7 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,/* Assume all sizes by default; the driver may override this later */domain->pgsize_bitmap=bus->iommu_ops->pgsize_bitmap;-/* Temporarily avoid -EEXIST while drivers still get their own cookies */-if(type==IOMMU_DOMAIN_DMA&&!domain->iova_cookie&&iommu_get_dma_cookie(domain)){+if(type==IOMMU_DOMAIN_DMA&&iommu_get_dma_cookie(domain)){iommu_domain_free(domain);domain=NULL;}
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-04 17:16:58
Since iommu_iotlb_gather exists to help drivers optimise flushing for a
given unmap request, it is also the logical place to indicate whether
the unmap is strict or not, and thus help them further optimise for
whether to expect a sync or a flush_all subsequently. As part of that,
it also seems fair to make the flush queue code take responsibility for
enforcing the really subtle ordering requirement it brings, so that we
don't need to worry about forgetting that if new drivers want to add
flush queue support, and can consolidate the existing versions.
While we're adding to the kerneldoc, also fill in some info for
@freelist which was overlooked previously.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v3: New
---
drivers/iommu/dma-iommu.c | 1 +
drivers/iommu/iova.c | 7 +++++++
include/linux/iommu.h | 8 +++++++-
3 files changed, 15 insertions(+), 1 deletion(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-04 17:17:00
IO_PGTABLE_QUIRK_NON_STRICT was never a very comfortable fit, since it's
not a quirk of the pagetable format itself. Now that we have a more
appropriate way to convey non-strict unmaps, though, this last of the
non-quirk quirks can also go, and with the flush queue code also now
enforcing its own ordering we can have a lovely cleanup all round.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v3: New
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 3 ---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 3 ---
drivers/iommu/io-pgtable-arm-v7s.c | 12 ++----------
drivers/iommu/io-pgtable-arm.c | 12 ++----------
include/linux/io-pgtable.h | 5 -----
5 files changed, 4 insertions(+), 31 deletions(-)
@@ -791,8 +784,7 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,if(cfg->quirks&~(IO_PGTABLE_QUIRK_ARM_NS|IO_PGTABLE_QUIRK_NO_PERMS|-IO_PGTABLE_QUIRK_ARM_MTK_EXT|-IO_PGTABLE_QUIRK_NON_STRICT))+IO_PGTABLE_QUIRK_ARM_MTK_EXT))returnNULL;/* If ARM_MTK_4GB is enabled, the NO_PERMS is also expected. */
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-04 17:17:02
Promote the difference between strict and non-strict DMA domains from an
internal detail to a distinct domain feature and type, to pave the road
for exposing it through the sysfs default domain interface.
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Jean-Philippe Brucker <redacted>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/dma-iommu.c | 2 +-
drivers/iommu/iommu.c | 8 ++++++--
include/linux/iommu.h | 11 +++++++++++
3 files changed, 18 insertions(+), 3 deletions(-)
@@ -765,7 +769,7 @@ static int iommu_create_device_direct_mappings(struct iommu_group *group,unsignedlongpg_size;intret=0;-if(!domain||domain->type!=IOMMU_DOMAIN_DMA)+if(!domain||!iommu_is_dma_domain(domain))return0;BUG_ON(!domain->pgsize_bitmap);
@@ -1947,7 +1951,7 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,/* Assume all sizes by default; the driver may override this later */domain->pgsize_bitmap=bus->iommu_ops->pgsize_bitmap;-if(type==IOMMU_DOMAIN_DMA&&iommu_get_dma_cookie(domain)){+if(iommu_is_dma_domain(domain)&&iommu_get_dma_cookie(domain)){iommu_domain_free(domain);domain=NULL;}
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-04 17:17:05
The DMA ops reset/setup can simply be unconditional, since
iommu-dma already knows only to touch DMA domains.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/amd/iommu.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
@@ -1706,14 +1706,9 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)staticvoidamd_iommu_probe_finalize(structdevice*dev){-structiommu_domain*domain;-/* Domains are initialized for this device - have a look what we ended up with */-domain=iommu_get_domain_for_dev(dev);-if(domain->type==IOMMU_DOMAIN_DMA)-iommu_setup_dma_ops(dev,0,U64_MAX);-else-set_dma_ops(dev,NULL);+set_dma_ops(dev,NULL);+iommu_setup_dma_ops(dev,0,U64_MAX);}staticvoidamd_iommu_release_device(structdevice*dev)
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-04 17:17:08
In preparation for the strict vs. non-strict decision for DMA domains to
be expressed in the domain type, make sure we expose our flush queue
awareness by accepting the new domain type.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 1 +
drivers/iommu/arm/arm-smmu/arm-smmu.c | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-04 17:17:11
In preparation for the strict vs. non-strict decision for DMA domains
to be expressed in the domain type, make sure we expose our flush queue
awareness by accepting the new domain type, and test the specific
feature flag where we want to identify DMA domains in general. The DMA
ops reset/setup can simply be made unconditional, since iommu-dma
already knows only to touch DMA domains.
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/intel/iommu.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
@@ -582,7 +582,7 @@ struct intel_iommu *domain_get_iommu(struct dmar_domain *domain)intiommu_id;/* si_domain and vm domain should not get here. */-if(WARN_ON(domain->domain.type!=IOMMU_DOMAIN_DMA))+if(WARN_ON(!iommu_is_dma_domain(&domain->domain)))returnNULL;for_each_domain_iommu(iommu_id,domain)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-04 17:17:17
Eliminate the iommu_get_dma_strict() indirection and pipe the
information through the domain type from the beginning. Besides
the flow simplification this also has several nice side-effects:
- Automatically implies strict mode for untrusted devices by
virtue of their IOMMU_DOMAIN_DMA override.
- Ensures that we only end up using flush queues for drivers
which are aware of them and can actually benefit.
- Allows us to handle flush queue init failure by falling back
to strict mode instead of leaving it to possibly blow up later.
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v3: Remember to update iommu_def_domain_type accordingly from
iommu_set_dma_strict() too
---
drivers/iommu/dma-iommu.c | 9 +++++----
drivers/iommu/iommu.c | 14 +++++---------
include/linux/iommu.h | 1 -
3 files changed, 10 insertions(+), 14 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-04 17:17:19
The sysfs interface for default domain types exists primarily so users
can choose the performance/security tradeoff relevant to their own
workload. As such, the choice between the policies for DMA domains fits
perfectly as an additional point on that scale - downgrading a
particular device from a strict default to non-strict may be enough to
let it reach the desired level of performance, while still retaining
more peace of mind than with a wide-open identity domain. Now that we've
abstracted non-strict mode as a distinct type of DMA domain, allow it to
be chosen through the user interface as well.
CC: Sai Praneeth Prakhya <redacted>
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: John Garry <redacted>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v3: Summarise the implications in the documentation for completeness
---
Documentation/ABI/testing/sysfs-kernel-iommu_groups | 6 +++++-
drivers/iommu/iommu.c | 2 ++
2 files changed, 7 insertions(+), 1 deletion(-)
@@ -42,8 +42,12 @@ Description: /sys/kernel/iommu_groups/<grp_id>/type shows the type of default ======== ====================================================== DMA All the DMA transactions from the device in this group are translated by the iommu.+ DMA-FQ As above, but using batched invalidation to lazily+ remove translations after use. This may offer reduced+ overhead at the cost of reduced memory protection. identity All the DMA transactions from the device in this group- are not translated by the iommu.+ are not translated by the iommu. Maximum performance+ but zero protection. auto Change to the type the device was booted with. ======== ======================================================
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-04 17:17:23
When passthrough is enabled, the default strictness policy becomes
irrelevant, since any subsequent runtime override to a DMA domain type
now embodies an explicit choice of strictness as well. Save on noise by
only logging the default policy when it is meaningfully in effect.
Reviewed-by: John Garry <redacted>
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/iommu.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-04 17:17:27
Factor out flush queue setup from the initial domain init so that we
can potentially trigger it from sysfs later on in a domain's lifetime.
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: John Garry <redacted>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/dma-iommu.c | 30 ++++++++++++++++++++----------
include/linux/dma-iommu.h | 9 ++++++---
2 files changed, 26 insertions(+), 13 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-04 17:17:29
To parallel the sysfs behaviour, merge the new build-time option
for DMA domain strictness into the default domain type choice.
Suggested-by: Joerg Roedel <joro@8bytes.org>
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Jean-Philippe Brucker <redacted>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v3: Remember to update parameter documentation as well
---
.../admin-guide/kernel-parameters.txt | 8 +-
drivers/iommu/Kconfig | 80 +++++++++----------
drivers/iommu/iommu.c | 2 +-
3 files changed, 44 insertions(+), 46 deletions(-)
@@ -2045,11 +2045,9 @@ 1 - Strict mode. DMA unmap operations invalidate IOMMU hardware TLBs synchronously.- unset - Use value of CONFIG_IOMMU_DEFAULT_{LAZY,STRICT}.- Note: on x86, the default behaviour depends on the- equivalent driver-specific parameters, but a strict- mode explicitly specified by either method takes- precedence.+ unset - Use value of CONFIG_IOMMU_DEFAULT_DMA_{LAZY,STRICT}.+ Note: on x86, strict mode specified via one of the+ legacy driver-specific options takes precedence. iommu.passthrough= [ARM64, X86] Configure DMA to bypass the IOMMU by default.
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-04 17:17:34
Allocating and enabling a flush queue is in fact something we can
reasonably do while a DMA domain is active, without having to rebuild it
from scratch. Thus we can allow a strict -> non-strict transition from
sysfs without requiring to unbind the device's driver, which is of
particular interest to users who want to make selective relaxations to
critical devices like the one serving their root filesystem.
Disabling and draining a queue also seems technically possible to
achieve without rebuilding the whole domain, but would certainly be more
involved. Furthermore there's not such a clear use-case for tightening
up security *after* the device may already have done whatever it is that
you don't trust it not to do, so we only consider the relaxation case.
CC: Sai Praneeth Prakhya <redacted>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v3: Actually think about concurrency, rework most of the fq data
accesses to be (hopefully) safe and comment it all
---
drivers/iommu/dma-iommu.c | 25 ++++++++++++++++++-------
drivers/iommu/iommu.c | 16 ++++++++++++----
drivers/iommu/iova.c | 9 ++++++---
3 files changed, 36 insertions(+), 14 deletions(-)
@@ -325,7 +331,12 @@ int iommu_dma_init_fq(struct iommu_domain *domain)domain->type=IOMMU_DOMAIN_DMA;return-ENODEV;}-cookie->fq_domain=domain;+/*+*Preventincompleteiovad->fqbeingobservable.Pairswithpathfrom+*__iommu_dma_unmap()throughiommu_dma_free_iova()toqueue_iova()+*/+smp_wmb();+WRITE_ONCE(cookie->fq_domain,domain);return0;}
@@ -456,17 +467,17 @@ static dma_addr_t iommu_dma_alloc_iova(struct iommu_domain *domain,}staticvoidiommu_dma_free_iova(structiommu_dma_cookie*cookie,-dma_addr_tiova,size_tsize,structpage*freelist)+dma_addr_tiova,size_tsize,structiommu_iotlb_gather*gather){structiova_domain*iovad=&cookie->iovad;/* The MSI case is only ever cleaning up its most recent allocation */if(cookie->type==IOMMU_DMA_MSI_COOKIE)cookie->msi_iova-=size;-elseif(cookie->fq_domain)/* non-strict mode */+elseif(gather&&gather->queued)queue_iova(iovad,iova_pfn(iovad,iova),size>>iova_shift(iovad),-(unsignedlong)freelist);+(unsignedlong)gather->freelist);elsefree_iova_fast(iovad,iova_pfn(iovad,iova),size>>iova_shift(iovad));
@@ -3203,6 +3203,13 @@ static int iommu_change_dev_def_domain(struct iommu_group *group,gotoout;}+/* We can bring up a flush queue without tearing down the domain */+if(type==IOMMU_DOMAIN_DMA_FQ&&prev_dom->type==IOMMU_DOMAIN_DMA){+prev_dom->type=IOMMU_DOMAIN_DMA_FQ;+ret=iommu_dma_init_fq(prev_dom);+gotoout;+}+/* Sets group->default_domain to the newly allocated domain */ret=iommu_group_alloc_default_domain(dev->bus,group,type);if(ret)
@@ -3243,9 +3250,9 @@ static int iommu_change_dev_def_domain(struct iommu_group *group,}/*-*Changingthedefaultdomainthroughsysfsrequirestheuserstoubindthe-*driversfromthedevicesintheiommugroup.Returnfailureifthisdoesn't-*meet.+*Changingthedefaultdomainthroughsysfsrequirestheuserstounbindthe+*driversfromthedevicesintheiommugroup,exceptforaDMA->DMA-FQ+*transition.Returnfailureifthisisn'tmet.**Weneedtoconsidertheracebetweenthisandthedevicereleasepath.*device_lock(dev)isusedheretoguaranteethatthedevicereleasepath
@@ -3321,7 +3328,8 @@ static ssize_t iommu_group_store_type(struct iommu_group *group,/* Check if the device in the group still has a driver bound to it */device_lock(dev);-if(device_is_bound(dev)){+if(device_is_bound(dev)&&!(req_type==IOMMU_DOMAIN_DMA_FQ&&+group->default_domain->type==IOMMU_DOMAIN_DMA)){pr_err_ratelimited("Device is still bound to driver\n");ret=-EBUSY;gotoout;
Am Mittwoch, 4. August 2021, 19:15:29 CEST schrieb Robin Murphy:
Now that everyone has converged on iommu-dma for IOMMU_DOMAIN_DMA
support, we can abandon the notion of drivers being responsible for the
cookie type, and consolidate all the management into the core code.
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 <redacted>
CC: Maxime Ripard <mripard@kernel.org>
Reviewed-by: Jean-Philippe Brucker <redacted>
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
On a Rockchip rk3288 (arm32), rk3399 (arm64) and px30 (arm64)
with the graphics pipeline using the iommu
Tested-by: Heiko Stuebner <heiko@sntech.de>
Heiko
quoted hunk
---
v3: Use a simpler temporary check instead of trying to be clever with
the error code
---
drivers/iommu/iommu.c | 7 +++++++
include/linux/iommu.h | 3 ++-
2 files changed, 9 insertions(+), 1 deletion(-)
@@ -1946,6 +1947,11 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,/* Assume all sizes by default; the driver may override this later */domain->pgsize_bitmap=bus->iommu_ops->pgsize_bitmap;+/* Temporarily avoid -EEXIST while drivers still get their own cookies */+if(type==IOMMU_DOMAIN_DMA&&!domain->iova_cookie&&iommu_get_dma_cookie(domain)){+iommu_domain_free(domain);+domain=NULL;+}returndomain;}
Am Mittwoch, 4. August 2021, 19:15:36 CEST schrieb Robin Murphy:
The core code bakes its own cookies now.
CC: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
On a Rockchip rk3288 (arm32), rk3399 (arm64) and px30 (arm64)
with the graphics pipeline using the iommu
Tested-by: Heiko Stuebner <heiko@sntech.de>
Acked-by: Heiko Stuebner <heiko@sntech.de>
Works now nicely on both arm32 and arm64
Thanks
Heiko
quoted hunk
---
v3: Also remove unneeded include
---
drivers/iommu/rockchip-iommu.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2021-08-05 07:19:08
On 04.08.2021 19:15, Robin Murphy wrote:
Now that everyone has converged on iommu-dma for IOMMU_DOMAIN_DMA
support, we can abandon the notion of drivers being responsible for the
cookie type, and consolidate all the management into the core code.
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 <redacted>
CC: Maxime Ripard <mripard@kernel.org>
Reviewed-by: Jean-Philippe Brucker <redacted>
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
quoted hunk
---
v3: Use a simpler temporary check instead of trying to be clever with
the error code
---
drivers/iommu/iommu.c | 7 +++++++
include/linux/iommu.h | 3 ++-
2 files changed, 9 insertions(+), 1 deletion(-)
@@ -1946,6 +1947,11 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,/* Assume all sizes by default; the driver may override this later */domain->pgsize_bitmap=bus->iommu_ops->pgsize_bitmap;+/* Temporarily avoid -EEXIST while drivers still get their own cookies */+if(type==IOMMU_DOMAIN_DMA&&!domain->iova_cookie&&iommu_get_dma_cookie(domain)){+iommu_domain_free(domain);+domain=NULL;+}returndomain;}
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
drivers/iommu/amd/iommu.c:1757:3: error: implicit declaration of function 'iommu_setup_dma_ops'; did you mean 'arch_setup_dma_ops'? [-Werror=implicit-function-declaration]
1757 | iommu_setup_dma_ops(dev, 0, U64_MAX);
| ^~~~~~~~~~~~~~~~~~~
| arch_setup_dma_ops
cc1: some warnings being treated as errors
vim +1757 drivers/iommu/amd/iommu.c
1ac4cbbc5eb56d arch/x86/kernel/amd_iommu.c Joerg Roedel 2008-12-10 1749
dce8d6964ebdb3 drivers/iommu/amd_iommu.c Joerg Roedel 2020-04-29 1750 static void amd_iommu_probe_finalize(struct device *dev)
dce8d6964ebdb3 drivers/iommu/amd_iommu.c Joerg Roedel 2020-04-29 1751 {
dce8d6964ebdb3 drivers/iommu/amd_iommu.c Joerg Roedel 2020-04-29 1752 struct iommu_domain *domain;
ac1534a55d1e87 drivers/iommu/amd_iommu.c Joerg Roedel 2012-06-21 1753
07ee86948c9111 drivers/iommu/amd_iommu.c Joerg Roedel 2015-05-28 1754 /* Domains are initialized for this device - have a look what we ended up with */
07ee86948c9111 drivers/iommu/amd_iommu.c Joerg Roedel 2015-05-28 1755 domain = iommu_get_domain_for_dev(dev);
57f9842e488406 drivers/iommu/amd_iommu.c Joerg Roedel 2020-04-29 1756 if (domain->type == IOMMU_DOMAIN_DMA)
ac6d704679d343 drivers/iommu/amd/iommu.c Jean-Philippe Brucker 2021-06-18 @1757 iommu_setup_dma_ops(dev, 0, U64_MAX);
d6177a6556f853 drivers/iommu/amd/iommu.c Jean-Philippe Brucker 2021-04-22 1758 else
d6177a6556f853 drivers/iommu/amd/iommu.c Jean-Philippe Brucker 2021-04-22 1759 set_dma_ops(dev, NULL);
e275a2a0fc9e21 arch/x86/kernel/amd_iommu.c Joerg Roedel 2008-12-10 1760 }
e275a2a0fc9e21 arch/x86/kernel/amd_iommu.c Joerg Roedel 2008-12-10 1761
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
From: Robin Murphy, Sent: Thursday, August 5, 2021 2:15 AM
Now that everyone has converged on iommu-dma for IOMMU_DOMAIN_DMA
support, we can abandon the notion of drivers being responsible for the
cookie type, and consolidate all the management into the core code.
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 <redacted>
CC: Maxime Ripard <mripard@kernel.org>
Reviewed-by: Jean-Philippe Brucker <redacted>
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Thank you for the patch!
I tested on my environment (r8a77951-salvator-xs),
and I didn't observe any regression. So,
Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Best regards,
Yoshihiro Shimoda
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Robin Murphy, Sent: Thursday, August 5, 2021 2:16 AM
The core code bakes its own cookies now.
CC: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
CC: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Thank you for the patch!
I tested on my environment (r8a77951-salvator-xs),
and I didn't observe any regression. So,
Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Best regards,
Yoshihiro Shimoda
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-05 09:38:09
On 2021-08-04 18:15, Robin Murphy wrote:
quoted hunk
The core code bakes its own cookies now.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v3: Also remove unneeded include
---
drivers/iommu/amd/iommu.c | 13 -------------
1 file changed, 13 deletions(-)
Oh dear, how embarrassing... I went through all the drivers making that
decision based on iommu_dma* references but totally forgot about
iommu_setup_dma_ops() here. And then of course fell into the trap of
"such a minor change I don't need to re-rest it" hubris... sigh, roll
back to v2 for this one.
Apologies,
Robin.
From: John Garry <hidden> Date: 2021-08-06 09:16:11
On 04/08/2021 18:15, Robin Murphy wrote:
To parallel the sysfs behaviour, merge the new build-time option
for DMA domain strictness into the default domain type choice.
Suggested-by: Joerg Roedel<joro@8bytes.org>
Reviewed-by: Lu Baolu<baolu.lu@linux.intel.com>
Reviewed-by: Jean-Philippe Brucker<redacted>
Signed-off-by: Robin Murphy<robin.murphy@arm.com>
From: Will Deacon <will@kernel.org> Date: 2021-08-09 12:49:43
On Wed, Aug 04, 2021 at 06:15:53PM +0100, Robin Murphy wrote:
Allocating and enabling a flush queue is in fact something we can
reasonably do while a DMA domain is active, without having to rebuild it
from scratch. Thus we can allow a strict -> non-strict transition from
sysfs without requiring to unbind the device's driver, which is of
particular interest to users who want to make selective relaxations to
critical devices like the one serving their root filesystem.
Disabling and draining a queue also seems technically possible to
achieve without rebuilding the whole domain, but would certainly be more
involved. Furthermore there's not such a clear use-case for tightening
up security *after* the device may already have done whatever it is that
you don't trust it not to do, so we only consider the relaxation case.
CC: Sai Praneeth Prakhya <redacted>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v3: Actually think about concurrency, rework most of the fq data
accesses to be (hopefully) safe and comment it all
---
drivers/iommu/dma-iommu.c | 25 ++++++++++++++++++-------
drivers/iommu/iommu.c | 16 ++++++++++++----
drivers/iommu/iova.c | 9 ++++++---
3 files changed, 36 insertions(+), 14 deletions(-)
I failed to break this, so hopefully you've caught everything now.
Only thing I wasn't sure of is why we still need the smp_wmb() in
init_iova_flush_queue(). Can we remove it now that we have one before
assigning into the cookie?
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-08-09 12:52:46
On Wed, Aug 04, 2021 at 06:15:52PM +0100, Robin Murphy wrote:
quoted hunk
Factor out flush queue setup from the initial domain init so that we
can potentially trigger it from sysfs later on in a domain's lifetime.
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: John Garry <redacted>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/dma-iommu.c | 30 ++++++++++++++++++++----------
include/linux/dma-iommu.h | 9 ++++++---
2 files changed, 26 insertions(+), 13 deletions(-)
I do find this a bit odd: we assert that the caller has set domain->type
to IOMMU_DOMAIN_DMA_FQ but then on failure we reset it to IOMMU_DOMAIN_DMA
here. I think it would be less error-prone if the setting of domain->type
was handled in the same function.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-09 13:40:15
On 2021-08-09 13:49, Will Deacon wrote:
On Wed, Aug 04, 2021 at 06:15:53PM +0100, Robin Murphy wrote:
quoted
Allocating and enabling a flush queue is in fact something we can
reasonably do while a DMA domain is active, without having to rebuild it
from scratch. Thus we can allow a strict -> non-strict transition from
sysfs without requiring to unbind the device's driver, which is of
particular interest to users who want to make selective relaxations to
critical devices like the one serving their root filesystem.
Disabling and draining a queue also seems technically possible to
achieve without rebuilding the whole domain, but would certainly be more
involved. Furthermore there's not such a clear use-case for tightening
up security *after* the device may already have done whatever it is that
you don't trust it not to do, so we only consider the relaxation case.
CC: Sai Praneeth Prakhya <redacted>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v3: Actually think about concurrency, rework most of the fq data
accesses to be (hopefully) safe and comment it all
---
drivers/iommu/dma-iommu.c | 25 ++++++++++++++++++-------
drivers/iommu/iommu.c | 16 ++++++++++++----
drivers/iommu/iova.c | 9 ++++++---
3 files changed, 36 insertions(+), 14 deletions(-)
I failed to break this, so hopefully you've caught everything now.
Only thing I wasn't sure of is why we still need the smp_wmb() in
init_iova_flush_queue(). Can we remove it now that we have one before
assigning into the cookie?
Mostly because I failed to spot it, I think :)
Indeed now that we don't have any callers other than iommu_dma_init_fq()
to worry about, I don't think that one matters any more. It would if
were testing cookie->iovad->fq directly as our indicator instead of
cookie->fq_domain, but then we'd still need the new barrier to ensure
iommu_dma_flush_iotlb_all() properly observes the latter, so we may as
well rely on that everywhere and let it fully replace the old one.
Thanks,
Robin.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-09 14:47:41
On 2021-08-09 13:52, Will Deacon wrote:
On Wed, Aug 04, 2021 at 06:15:52PM +0100, Robin Murphy wrote:
quoted
Factor out flush queue setup from the initial domain init so that we
can potentially trigger it from sysfs later on in a domain's lifetime.
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: John Garry <redacted>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/dma-iommu.c | 30 ++++++++++++++++++++----------
include/linux/dma-iommu.h | 9 ++++++---
2 files changed, 26 insertions(+), 13 deletions(-)
I do find this a bit odd: we assert that the caller has set domain->type
to IOMMU_DOMAIN_DMA_FQ but then on failure we reset it to IOMMU_DOMAIN_DMA
here. I think it would be less error-prone if the setting of domain->type
was handled in the same function.
On reflection I think I agree. For some reason I settled on the idea of
doing this to make the callers simpler, but it turns out that unpicking
it to flow logically is in fact a +4/-5 diff essentially just moving all
the same statements to different places, and that's before I update
comments since that theoretical race between the sysfs and DMA ops paths
only exists because of sysfs having to dance around the type check here...
I'll send v4 later today or possibly tomorrow, but not in such a hurry
that I skimp on the build-testing this time!
Cheers,
Robin.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Rajat Jain <hidden> Date: 2021-08-09 19:06:02
On Wed, Aug 4, 2021 at 10:16 AM Robin Murphy [off-list ref] wrote:
quoted hunk
Factor out flush queue setup from the initial domain init so that we
can potentially trigger it from sysfs later on in a domain's lifetime.
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: John Garry <redacted>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/dma-iommu.c | 30 ++++++++++++++++++++----------
include/linux/dma-iommu.h | 9 ++++++---
2 files changed, 26 insertions(+), 13 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-09 19:59:44
On 2021-08-09 20:05, Rajat Jain wrote:
On Wed, Aug 4, 2021 at 10:16 AM Robin Murphy [off-list ref] wrote:
quoted
Factor out flush queue setup from the initial domain init so that we
can potentially trigger it from sysfs later on in a domain's lifetime.
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: John Garry <redacted>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/dma-iommu.c | 30 ++++++++++++++++++++----------
include/linux/dma-iommu.h | 9 ++++++---
2 files changed, 26 insertions(+), 13 deletions(-)
This looks like an unrelated code cleanup. Should this be a separate patch?
Ha, busted! Much of this was done in the "stream of consciousness" style
where I made a big sprawling mess then split it up into patches and
branches afterwards. TBH it was already feeling pretty tenuous having a
separate patch just to move this one function, and it only gets more so
with the simplification Will pointed out earlier. I think I'll squash
iommu_dma_init_fq() into the next patch then do a thorough header sweep,
since I've now spotted some things in iova.h which could probably go as
well.
Thanks for the poke!
Robin.
From: Rajat Jain <hidden> Date: 2021-08-09 20:16:27
On Mon, Aug 9, 2021 at 12:59 PM Robin Murphy [off-list ref] wrote:
On 2021-08-09 20:05, Rajat Jain wrote:
quoted
On Wed, Aug 4, 2021 at 10:16 AM Robin Murphy [off-list ref] wrote:
quoted
Factor out flush queue setup from the initial domain init so that we
can potentially trigger it from sysfs later on in a domain's lifetime.
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: John Garry <redacted>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/dma-iommu.c | 30 ++++++++++++++++++++----------
include/linux/dma-iommu.h | 9 ++++++---
2 files changed, 26 insertions(+), 13 deletions(-)
This looks like an unrelated code cleanup. Should this be a separate patch?
Ha, busted! Much of this was done in the "stream of consciousness" style
where I made a big sprawling mess then split it up into patches and
branches afterwards. TBH it was already feeling pretty tenuous having a
separate patch just to move this one function, and it only gets more so
with the simplification Will pointed out earlier. I think I'll squash
iommu_dma_init_fq() into the next patch then do a thorough header sweep,
since I've now spotted some things in iova.h which could probably go as
well.
Thank you. I chanced upon this only because I've backported your
patchset (and some other changes that it depends on) to 5.10 which is
the kernel we currently use for our Intel platforms, and this cleanup
hunk was creating a problem (since 5.10 still uses the symbol you
removed). I'll be giving your v3 patchset a spin in my setup and
update you in case I see any issue.
Thanks,
Rajat