linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/24] iommu: Refactor DMA domain strictness
@ 2021-07-28 15:58 Robin Murphy
  2021-07-28 15:58 ` [PATCH v2 01/24] iommu: Pull IOVA cookie management into the core Robin Murphy
                   ` (26 more replies)
  0 siblings, 27 replies; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders, Marek Szyprowski,
	Yoshihiro Shimoda, Geert Uytterhoeven, Yong Wu, Heiko Stuebner,
	Chunyan Zhang, Maxime Ripard, Jean-Philippe Brucker

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(-)

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH v2 01/24] iommu: Pull IOVA cookie management into the core
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-07-30  6:06   ` Lu Baolu
  2021-07-30  9:32   ` Jean-Philippe Brucker
  2021-07-28 15:58 ` [PATCH v2 02/24] iommu/amd: Drop IOVA cookie management Robin Murphy
                   ` (25 subsequent siblings)
  26 siblings, 2 replies; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders, Marek Szyprowski,
	Yoshihiro Shimoda, Geert Uytterhoeven, Yong Wu, Heiko Stuebner,
	Chunyan Zhang, Maxime Ripard, Jean-Philippe Brucker

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 <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>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/iommu.c | 7 +++++++
 include/linux/iommu.h | 3 ++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index f2cda9950bd5..ea5a9ea8d431 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -7,6 +7,7 @@
 #define pr_fmt(fmt)    "iommu: " fmt
 
 #include <linux/device.h>
+#include <linux/dma-iommu.h>
 #include <linux/kernel.h>
 #include <linux/bits.h>
 #include <linux/bug.h>
@@ -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 ignore -EEXIST while drivers still get their own cookies */
+	if (type == IOMMU_DOMAIN_DMA && iommu_get_dma_cookie(domain) == -ENOMEM) {
+		iommu_domain_free(domain);
+		domain = NULL;
+	}
 	return domain;
 }
 
@@ -1957,6 +1963,7 @@ EXPORT_SYMBOL_GPL(iommu_domain_alloc);
 
 void iommu_domain_free(struct iommu_domain *domain)
 {
+	iommu_put_dma_cookie(domain);
 	domain->ops->domain_free(domain);
 }
 EXPORT_SYMBOL_GPL(iommu_domain_free);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 4997c78e2670..141779d76035 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -40,6 +40,7 @@ struct iommu_domain;
 struct notifier_block;
 struct iommu_sva;
 struct iommu_fault_event;
+struct iommu_dma_cookie;
 
 /* iommu fault flags */
 #define IOMMU_FAULT_READ	0x0
@@ -86,7 +87,7 @@ struct iommu_domain {
 	iommu_fault_handler_t handler;
 	void *handler_token;
 	struct iommu_domain_geometry geometry;
-	void *iova_cookie;
+	struct iommu_dma_cookie *iova_cookie;
 };
 
 enum iommu_cap {
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 02/24] iommu/amd: Drop IOVA cookie management
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
  2021-07-28 15:58 ` [PATCH v2 01/24] iommu: Pull IOVA cookie management into the core Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-07-28 15:58 ` [PATCH v2 03/24] iommu/arm-smmu: " Robin Murphy
                   ` (24 subsequent siblings)
  26 siblings, 0 replies; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders

The core code bakes its own cookies now.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/amd/iommu.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 52fe2326042a..0fd98d35d73b 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -1918,16 +1918,7 @@ static struct iommu_domain *amd_iommu_domain_alloc(unsigned type)
 	domain->domain.geometry.aperture_end   = ~0ULL;
 	domain->domain.geometry.force_aperture = true;
 
-	if (type == IOMMU_DOMAIN_DMA &&
-	    iommu_get_dma_cookie(&domain->domain) == -ENOMEM)
-		goto free_domain;
-
 	return &domain->domain;
-
-free_domain:
-	protection_domain_free(domain);
-
-	return NULL;
 }
 
 static void amd_iommu_domain_free(struct iommu_domain *dom)
@@ -1944,9 +1935,6 @@ static void amd_iommu_domain_free(struct iommu_domain *dom)
 	if (!dom)
 		return;
 
-	if (dom->type == IOMMU_DOMAIN_DMA)
-		iommu_put_dma_cookie(&domain->domain);
-
 	if (domain->flags & PD_IOMMUV2_MASK)
 		free_gcr3_table(domain);
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 03/24] iommu/arm-smmu: Drop IOVA cookie management
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
  2021-07-28 15:58 ` [PATCH v2 01/24] iommu: Pull IOVA cookie management into the core Robin Murphy
  2021-07-28 15:58 ` [PATCH v2 02/24] iommu/amd: Drop IOVA cookie management Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-07-28 15:58 ` [PATCH v2 04/24] iommu/vt-d: " Robin Murphy
                   ` (23 subsequent siblings)
  26 siblings, 0 replies; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders

The core code bakes its own cookies now.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  7 -------
 drivers/iommu/arm/arm-smmu/arm-smmu.c       | 15 ++++-----------
 drivers/iommu/arm/arm-smmu/qcom_iommu.c     |  8 --------
 3 files changed, 4 insertions(+), 26 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 6346f21726f4..4c648da447bf 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1984,12 +1984,6 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 	if (!smmu_domain)
 		return NULL;
 
-	if (type == IOMMU_DOMAIN_DMA &&
-	    iommu_get_dma_cookie(&smmu_domain->domain)) {
-		kfree(smmu_domain);
-		return NULL;
-	}
-
 	mutex_init(&smmu_domain->init_mutex);
 	INIT_LIST_HEAD(&smmu_domain->devices);
 	spin_lock_init(&smmu_domain->devices_lock);
@@ -2021,7 +2015,6 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
 	struct arm_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 */
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index ac21170fa208..970d9e4dcd69 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -868,10 +868,10 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 {
 	struct arm_smmu_domain *smmu_domain;
 
-	if (type != IOMMU_DOMAIN_UNMANAGED &&
-	    type != IOMMU_DOMAIN_DMA &&
-	    type != IOMMU_DOMAIN_IDENTITY)
-		return NULL;
+	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_IDENTITY) {
+		if (using_legacy_binding || type != IOMMU_DOMAIN_DMA)
+			return NULL;
+	}
 	/*
 	 * Allocate the domain and initialise some of its data structures.
 	 * We can't really do anything meaningful until we've added a
@@ -881,12 +881,6 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 	if (!smmu_domain)
 		return NULL;
 
-	if (type == IOMMU_DOMAIN_DMA && (using_legacy_binding ||
-	    iommu_get_dma_cookie(&smmu_domain->domain))) {
-		kfree(smmu_domain);
-		return NULL;
-	}
-
 	mutex_init(&smmu_domain->init_mutex);
 	spin_lock_init(&smmu_domain->cb_lock);
 
@@ -901,7 +895,6 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
 	 * Free the domain resources. We assume that all devices have
 	 * already been detached.
 	 */
-	iommu_put_dma_cookie(domain);
 	arm_smmu_destroy_domain_context(domain);
 	kfree(smmu_domain);
 }
diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
index 021cf8f65ffc..4b7eca5f5148 100644
--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -335,12 +335,6 @@ static struct iommu_domain *qcom_iommu_domain_alloc(unsigned type)
 	if (!qcom_domain)
 		return NULL;
 
-	if (type == IOMMU_DOMAIN_DMA &&
-	    iommu_get_dma_cookie(&qcom_domain->domain)) {
-		kfree(qcom_domain);
-		return NULL;
-	}
-
 	mutex_init(&qcom_domain->init_mutex);
 	spin_lock_init(&qcom_domain->pgtbl_lock);
 
@@ -351,8 +345,6 @@ static void qcom_iommu_domain_free(struct iommu_domain *domain)
 {
 	struct qcom_iommu_domain *qcom_domain = to_qcom_iommu_domain(domain);
 
-	iommu_put_dma_cookie(domain);
-
 	if (qcom_domain->iommu) {
 		/*
 		 * NOTE: unmap can be called after client device is powered
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 04/24] iommu/vt-d: Drop IOVA cookie management
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (2 preceding siblings ...)
  2021-07-28 15:58 ` [PATCH v2 03/24] iommu/arm-smmu: " Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-07-30  6:07   ` Lu Baolu
  2021-07-28 15:58 ` [PATCH v2 05/24] iommu/exynos: " Robin Murphy
                   ` (22 subsequent siblings)
  26 siblings, 1 reply; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders

The core code bakes its own cookies now.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/intel/iommu.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index c12cc955389a..7e168634c433 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -1979,10 +1979,6 @@ static void domain_exit(struct dmar_domain *domain)
 	/* Remove associated devices and clear attached or cached domains */
 	domain_remove_dev_info(domain);
 
-	/* destroy iovas */
-	if (domain->domain.type == IOMMU_DOMAIN_DMA)
-		iommu_put_dma_cookie(&domain->domain);
-
 	if (domain->pgd) {
 		struct page *freelist;
 
@@ -4544,10 +4540,6 @@ static struct iommu_domain *intel_iommu_domain_alloc(unsigned type)
 			return NULL;
 		}
 
-		if (type == IOMMU_DOMAIN_DMA &&
-		    iommu_get_dma_cookie(&dmar_domain->domain))
-			return NULL;
-
 		domain = &dmar_domain->domain;
 		domain->geometry.aperture_start = 0;
 		domain->geometry.aperture_end   =
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 05/24] iommu/exynos: Drop IOVA cookie management
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (3 preceding siblings ...)
  2021-07-28 15:58 ` [PATCH v2 04/24] iommu/vt-d: " Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-07-28 15:58 ` [PATCH v2 06/24] iommu/ipmmu-vmsa: " Robin Murphy
                   ` (21 subsequent siblings)
  26 siblings, 0 replies; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders, Marek Szyprowski

The core code bakes its own cookies now.

CC: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/exynos-iommu.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index d0fbf1d10e18..34085d069cda 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -735,20 +735,16 @@ static struct iommu_domain *exynos_iommu_domain_alloc(unsigned type)
 	/* Check if correct PTE offsets are initialized */
 	BUG_ON(PG_ENT_SHIFT < 0 || !dma_dev);
 
+	if (type != IOMMU_DOMAIN_DMA && type != IOMMU_DOMAIN_UNMANAGED)
+		return NULL;
+
 	domain = kzalloc(sizeof(*domain), GFP_KERNEL);
 	if (!domain)
 		return NULL;
 
-	if (type == IOMMU_DOMAIN_DMA) {
-		if (iommu_get_dma_cookie(&domain->domain) != 0)
-			goto err_pgtable;
-	} else if (type != IOMMU_DOMAIN_UNMANAGED) {
-		goto err_pgtable;
-	}
-
 	domain->pgtable = (sysmmu_pte_t *)__get_free_pages(GFP_KERNEL, 2);
 	if (!domain->pgtable)
-		goto err_dma_cookie;
+		goto err_pgtable;
 
 	domain->lv2entcnt = (short *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 1);
 	if (!domain->lv2entcnt)
@@ -779,9 +775,6 @@ static struct iommu_domain *exynos_iommu_domain_alloc(unsigned type)
 	free_pages((unsigned long)domain->lv2entcnt, 1);
 err_counter:
 	free_pages((unsigned long)domain->pgtable, 2);
-err_dma_cookie:
-	if (type == IOMMU_DOMAIN_DMA)
-		iommu_put_dma_cookie(&domain->domain);
 err_pgtable:
 	kfree(domain);
 	return NULL;
@@ -809,9 +802,6 @@ static void exynos_iommu_domain_free(struct iommu_domain *iommu_domain)
 
 	spin_unlock_irqrestore(&domain->lock, flags);
 
-	if (iommu_domain->type == IOMMU_DOMAIN_DMA)
-		iommu_put_dma_cookie(iommu_domain);
-
 	dma_unmap_single(dma_dev, virt_to_phys(domain->pgtable), LV1TABLE_SIZE,
 			 DMA_TO_DEVICE);
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 06/24] iommu/ipmmu-vmsa: Drop IOVA cookie management
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (4 preceding siblings ...)
  2021-07-28 15:58 ` [PATCH v2 05/24] iommu/exynos: " Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-07-28 15:58 ` [PATCH v2 07/24] iommu/mtk: " Robin Murphy
                   ` (20 subsequent siblings)
  26 siblings, 0 replies; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders, Yoshihiro Shimoda,
	Geert Uytterhoeven

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>
---
 drivers/iommu/ipmmu-vmsa.c | 27 ++++-----------------------
 1 file changed, 4 insertions(+), 23 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 51ea6f00db2f..31252268f0d0 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -564,10 +564,13 @@ static irqreturn_t ipmmu_irq(int irq, void *dev)
  * IOMMU Operations
  */
 
-static struct iommu_domain *__ipmmu_domain_alloc(unsigned type)
+static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
 {
 	struct ipmmu_vmsa_domain *domain;
 
+	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
+		return NULL;
+
 	domain = kzalloc(sizeof(*domain), GFP_KERNEL);
 	if (!domain)
 		return NULL;
@@ -577,27 +580,6 @@ static struct iommu_domain *__ipmmu_domain_alloc(unsigned type)
 	return &domain->io_domain;
 }
 
-static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
-{
-	struct iommu_domain *io_domain = NULL;
-
-	switch (type) {
-	case IOMMU_DOMAIN_UNMANAGED:
-		io_domain = __ipmmu_domain_alloc(type);
-		break;
-
-	case IOMMU_DOMAIN_DMA:
-		io_domain = __ipmmu_domain_alloc(type);
-		if (io_domain && iommu_get_dma_cookie(io_domain)) {
-			kfree(io_domain);
-			io_domain = NULL;
-		}
-		break;
-	}
-
-	return io_domain;
-}
-
 static void ipmmu_domain_free(struct iommu_domain *io_domain)
 {
 	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
@@ -606,7 +588,6 @@ static void ipmmu_domain_free(struct iommu_domain *io_domain)
 	 * Free the domain resources. We assume that all devices have already
 	 * been detached.
 	 */
-	iommu_put_dma_cookie(io_domain);
 	ipmmu_domain_destroy_context(domain);
 	free_io_pgtable_ops(domain->iop);
 	kfree(domain);
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 07/24] iommu/mtk: Drop IOVA cookie management
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (5 preceding siblings ...)
  2021-07-28 15:58 ` [PATCH v2 06/24] iommu/ipmmu-vmsa: " Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-07-28 15:58 ` [PATCH v2 08/24] iommu/rockchip: " Robin Murphy
                   ` (19 subsequent siblings)
  26 siblings, 0 replies; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders, Yong Wu

The core code bakes its own cookies now.

CC: Yong Wu <yong.wu@mediatek.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/mtk_iommu.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 6f7c69688ce2..e39a6d1da28d 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -441,17 +441,11 @@ static struct iommu_domain *mtk_iommu_domain_alloc(unsigned type)
 	if (!dom)
 		return NULL;
 
-	if (iommu_get_dma_cookie(&dom->domain)) {
-		kfree(dom);
-		return NULL;
-	}
-
 	return &dom->domain;
 }
 
 static void mtk_iommu_domain_free(struct iommu_domain *domain)
 {
-	iommu_put_dma_cookie(domain);
 	kfree(to_mtk_domain(domain));
 }
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 08/24] iommu/rockchip: Drop IOVA cookie management
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (6 preceding siblings ...)
  2021-07-28 15:58 ` [PATCH v2 07/24] iommu/mtk: " Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-07-28 15:58 ` [PATCH v2 09/24] iommu/sprd: " Robin Murphy
                   ` (18 subsequent siblings)
  26 siblings, 0 replies; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders, Heiko Stuebner

The core code bakes its own cookies now.

CC: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/rockchip-iommu.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 9febfb7f3025..c24561f54f32 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -1074,10 +1074,6 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type)
 	if (!rk_domain)
 		return NULL;
 
-	if (type == IOMMU_DOMAIN_DMA &&
-	    iommu_get_dma_cookie(&rk_domain->domain))
-		goto err_free_domain;
-
 	/*
 	 * rk32xx iommus use a 2 level pagetable.
 	 * Each level1 (dt) and level2 (pt) table has 1024 4-byte entries.
@@ -1085,7 +1081,7 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type)
 	 */
 	rk_domain->dt = (u32 *)get_zeroed_page(GFP_KERNEL | GFP_DMA32);
 	if (!rk_domain->dt)
-		goto err_put_cookie;
+		goto err_free_domain;
 
 	rk_domain->dt_dma = dma_map_single(dma_dev, rk_domain->dt,
 					   SPAGE_SIZE, DMA_TO_DEVICE);
@@ -1106,9 +1102,6 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type)
 
 err_free_dt:
 	free_page((unsigned long)rk_domain->dt);
-err_put_cookie:
-	if (type == IOMMU_DOMAIN_DMA)
-		iommu_put_dma_cookie(&rk_domain->domain);
 err_free_domain:
 	kfree(rk_domain);
 
@@ -1137,8 +1130,6 @@ static void rk_iommu_domain_free(struct iommu_domain *domain)
 			 SPAGE_SIZE, DMA_TO_DEVICE);
 	free_page((unsigned long)rk_domain->dt);
 
-	if (domain->type == IOMMU_DOMAIN_DMA)
-		iommu_put_dma_cookie(&rk_domain->domain);
 	kfree(rk_domain);
 }
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 09/24] iommu/sprd: Drop IOVA cookie management
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (7 preceding siblings ...)
  2021-07-28 15:58 ` [PATCH v2 08/24] iommu/rockchip: " Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-07-28 15:58 ` [PATCH v2 10/24] iommu/sun50i: " Robin Murphy
                   ` (17 subsequent siblings)
  26 siblings, 0 replies; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders, Chunyan Zhang

The core code bakes its own cookies now.

CC: Chunyan Zhang <chunyan.zhang@unisoc.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/sprd-iommu.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/iommu/sprd-iommu.c b/drivers/iommu/sprd-iommu.c
index 73dfd9946312..2bc1de6e823d 100644
--- a/drivers/iommu/sprd-iommu.c
+++ b/drivers/iommu/sprd-iommu.c
@@ -144,11 +144,6 @@ static struct iommu_domain *sprd_iommu_domain_alloc(unsigned int domain_type)
 	if (!dom)
 		return NULL;
 
-	if (iommu_get_dma_cookie(&dom->domain)) {
-		kfree(dom);
-		return NULL;
-	}
-
 	spin_lock_init(&dom->pgtlock);
 
 	dom->domain.geometry.aperture_start = 0;
@@ -161,7 +156,6 @@ static void sprd_iommu_domain_free(struct iommu_domain *domain)
 {
 	struct sprd_iommu_domain *dom = to_sprd_domain(domain);
 
-	iommu_put_dma_cookie(domain);
 	kfree(dom);
 }
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 10/24] iommu/sun50i: Drop IOVA cookie management
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (8 preceding siblings ...)
  2021-07-28 15:58 ` [PATCH v2 09/24] iommu/sprd: " Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-07-28 15:58 ` [PATCH v2 11/24] iommu/virtio: " Robin Murphy
                   ` (16 subsequent siblings)
  26 siblings, 0 replies; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders, Maxime Ripard

The core code bakes its own cookies now.

CC: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/sun50i-iommu.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
index 181bb1c3437c..c349a95ec7bd 100644
--- a/drivers/iommu/sun50i-iommu.c
+++ b/drivers/iommu/sun50i-iommu.c
@@ -610,14 +610,10 @@ static struct iommu_domain *sun50i_iommu_domain_alloc(unsigned type)
 	if (!sun50i_domain)
 		return NULL;
 
-	if (type == IOMMU_DOMAIN_DMA &&
-	    iommu_get_dma_cookie(&sun50i_domain->domain))
-		goto err_free_domain;
-
 	sun50i_domain->dt = (u32 *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
 						    get_order(DT_SIZE));
 	if (!sun50i_domain->dt)
-		goto err_put_cookie;
+		goto err_free_domain;
 
 	refcount_set(&sun50i_domain->refcnt, 1);
 
@@ -627,10 +623,6 @@ static struct iommu_domain *sun50i_iommu_domain_alloc(unsigned type)
 
 	return &sun50i_domain->domain;
 
-err_put_cookie:
-	if (type == IOMMU_DOMAIN_DMA)
-		iommu_put_dma_cookie(&sun50i_domain->domain);
-
 err_free_domain:
 	kfree(sun50i_domain);
 
@@ -644,8 +636,6 @@ static void sun50i_iommu_domain_free(struct iommu_domain *domain)
 	free_pages((unsigned long)sun50i_domain->dt, get_order(DT_SIZE));
 	sun50i_domain->dt = NULL;
 
-	iommu_put_dma_cookie(domain);
-
 	kfree(sun50i_domain);
 }
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 11/24] iommu/virtio: Drop IOVA cookie management
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (9 preceding siblings ...)
  2021-07-28 15:58 ` [PATCH v2 10/24] iommu/sun50i: " Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-07-30  9:20   ` Jean-Philippe Brucker
  2021-07-28 15:58 ` [PATCH v2 12/24] iommu/dma: Unexport " Robin Murphy
                   ` (15 subsequent siblings)
  26 siblings, 1 reply; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders, Jean-Philippe Brucker

The core code bakes its own cookies now.

CC: Jean-Philippe Brucker <jean-philippe@linaro.org>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/virtio-iommu.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 6abdcab7273b..80930ce04a16 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -598,12 +598,6 @@ static struct iommu_domain *viommu_domain_alloc(unsigned type)
 	spin_lock_init(&vdomain->mappings_lock);
 	vdomain->mappings = RB_ROOT_CACHED;
 
-	if (type == IOMMU_DOMAIN_DMA &&
-	    iommu_get_dma_cookie(&vdomain->domain)) {
-		kfree(vdomain);
-		return NULL;
-	}
-
 	return &vdomain->domain;
 }
 
@@ -643,8 +637,6 @@ static void viommu_domain_free(struct iommu_domain *domain)
 {
 	struct viommu_domain *vdomain = to_viommu_domain(domain);
 
-	iommu_put_dma_cookie(domain);
-
 	/* Free all remaining mappings (size 2^64) */
 	viommu_del_mappings(vdomain, 0, 0);
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 12/24] iommu/dma: Unexport IOVA cookie management
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (10 preceding siblings ...)
  2021-07-28 15:58 ` [PATCH v2 11/24] iommu/virtio: " Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-07-30  6:07   ` Lu Baolu
  2021-07-30  9:21   ` Jean-Philippe Brucker
  2021-07-28 15:58 ` [PATCH v2 13/24] iommu/dma: Remove redundant "!dev" checks Robin Murphy
                   ` (14 subsequent siblings)
  26 siblings, 2 replies; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders

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.

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(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 98ba927aee1a..10067fbc4309 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -98,9 +98,6 @@ static struct iommu_dma_cookie *cookie_alloc(enum iommu_dma_cookie_type type)
 /**
  * iommu_get_dma_cookie - Acquire DMA-API resources for a domain
  * @domain: IOMMU domain to prepare for DMA-API usage
- *
- * IOMMU drivers should normally call this from their domain_alloc
- * callback when domain->type == IOMMU_DOMAIN_DMA.
  */
 int iommu_get_dma_cookie(struct iommu_domain *domain)
 {
@@ -113,7 +110,6 @@ int iommu_get_dma_cookie(struct iommu_domain *domain)
 
 	return 0;
 }
-EXPORT_SYMBOL(iommu_get_dma_cookie);
 
 /**
  * iommu_get_msi_cookie - Acquire just MSI remapping resources
@@ -151,8 +147,6 @@ EXPORT_SYMBOL(iommu_get_msi_cookie);
  * iommu_put_dma_cookie - Release a domain's DMA mapping resources
  * @domain: IOMMU domain previously prepared by iommu_get_dma_cookie() or
  *          iommu_get_msi_cookie()
- *
- * IOMMU drivers should normally call this from their domain_free callback.
  */
 void iommu_put_dma_cookie(struct iommu_domain *domain)
 {
@@ -172,7 +166,6 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
 	kfree(cookie);
 	domain->iova_cookie = NULL;
 }
-EXPORT_SYMBOL(iommu_put_dma_cookie);
 
 /**
  * iommu_dma_get_resv_regions - Reserved region driver helper
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index ea5a9ea8d431..fa8109369f74 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -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 ignore -EEXIST while drivers still get their own cookies */
-	if (type == IOMMU_DOMAIN_DMA && iommu_get_dma_cookie(domain) == -ENOMEM) {
+	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

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 13/24] iommu/dma: Remove redundant "!dev" checks
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (11 preceding siblings ...)
  2021-07-28 15:58 ` [PATCH v2 12/24] iommu/dma: Unexport " Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-07-30  6:08   ` Lu Baolu
  2021-07-28 15:58 ` [PATCH v2 14/24] iommu: Introduce explicit type for non-strict DMA domains Robin Murphy
                   ` (13 subsequent siblings)
  26 siblings, 1 reply; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders

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 <john.garry@huawei.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/dma-iommu.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 10067fbc4309..e28396cea6eb 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -363,7 +363,7 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
 
 	init_iova_domain(iovad, 1UL << order, base_pfn);
 
-	if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) &&
+	if (!cookie->fq_domain && !dev_is_untrusted(dev) &&
 	    domain->ops->flush_iotlb_all && !iommu_get_dma_strict(domain)) {
 		if (init_iova_flush_queue(iovad, iommu_dma_flush_iotlb_all,
 					  iommu_dma_entry_dtor))
@@ -372,9 +372,6 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
 			cookie->fq_domain = domain;
 	}
 
-	if (!dev)
-		return 0;
-
 	return iova_reserve_iommu_regions(dev, domain);
 }
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 14/24] iommu: Introduce explicit type for non-strict DMA domains
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (12 preceding siblings ...)
  2021-07-28 15:58 ` [PATCH v2 13/24] iommu/dma: Remove redundant "!dev" checks Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-07-30  6:08   ` Lu Baolu
  2021-07-30  9:23   ` Jean-Philippe Brucker
  2021-07-28 15:58 ` [PATCH v2 15/24] iommu/amd: Prepare for multiple DMA domain types Robin Murphy
                   ` (12 subsequent siblings)
  26 siblings, 2 replies; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders

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.

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(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index e28396cea6eb..8b3545c01077 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -1311,7 +1311,7 @@ void iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 dma_limit)
 	 * The IOMMU core code allocates the default DMA domain, which the
 	 * underlying IOMMU driver needs to support via the dma-iommu layer.
 	 */
-	if (domain->type == IOMMU_DOMAIN_DMA) {
+	if (iommu_is_dma_domain(domain)) {
 		if (iommu_dma_init_domain(domain, dma_base, dma_limit, dev))
 			goto out_err;
 		dev->dma_ops = &iommu_dma_ops;
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index fa8109369f74..982545234cf3 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -115,6 +115,7 @@ static const char *iommu_domain_type_str(unsigned int t)
 	case IOMMU_DOMAIN_UNMANAGED:
 		return "Unmanaged";
 	case IOMMU_DOMAIN_DMA:
+	case IOMMU_DOMAIN_DMA_FQ:
 		return "Translated";
 	default:
 		return "Unknown";
@@ -552,6 +553,9 @@ static ssize_t iommu_group_show_type(struct iommu_group *group,
 		case IOMMU_DOMAIN_DMA:
 			type = "DMA\n";
 			break;
+		case IOMMU_DOMAIN_DMA_FQ:
+			type = "DMA-FQ\n";
+			break;
 		}
 	}
 	mutex_unlock(&group->mutex);
@@ -765,7 +769,7 @@ static int iommu_create_device_direct_mappings(struct iommu_group *group,
 	unsigned long pg_size;
 	int ret = 0;
 
-	if (!domain || domain->type != IOMMU_DOMAIN_DMA)
+	if (!domain || !iommu_is_dma_domain(domain))
 		return 0;
 
 	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;
 	}
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 141779d76035..046ba4d54cd2 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -61,6 +61,7 @@ struct iommu_domain_geometry {
 #define __IOMMU_DOMAIN_DMA_API	(1U << 1)  /* Domain for use in DMA-API
 					      implementation              */
 #define __IOMMU_DOMAIN_PT	(1U << 2)  /* Domain is identity mapped   */
+#define __IOMMU_DOMAIN_DMA_FQ	(1U << 3)  /* DMA-API uses flush queue    */
 
 /*
  * This are the possible domain-types
@@ -73,12 +74,17 @@ struct iommu_domain_geometry {
  *	IOMMU_DOMAIN_DMA	- Internally used for DMA-API implementations.
  *				  This flag allows IOMMU drivers to implement
  *				  certain optimizations for these domains
+ *	IOMMU_DOMAIN_DMA_FQ	- As above, but definitely using batched TLB
+ *				  invalidation.
  */
 #define IOMMU_DOMAIN_BLOCKED	(0U)
 #define IOMMU_DOMAIN_IDENTITY	(__IOMMU_DOMAIN_PT)
 #define IOMMU_DOMAIN_UNMANAGED	(__IOMMU_DOMAIN_PAGING)
 #define IOMMU_DOMAIN_DMA	(__IOMMU_DOMAIN_PAGING |	\
 				 __IOMMU_DOMAIN_DMA_API)
+#define IOMMU_DOMAIN_DMA_FQ	(__IOMMU_DOMAIN_PAGING |	\
+				 __IOMMU_DOMAIN_DMA_API |	\
+				 __IOMMU_DOMAIN_DMA_FQ)
 
 struct iommu_domain {
 	unsigned type;
@@ -90,6 +96,11 @@ struct iommu_domain {
 	struct iommu_dma_cookie *iova_cookie;
 };
 
+static inline bool iommu_is_dma_domain(struct iommu_domain *domain)
+{
+	return domain->type & __IOMMU_DOMAIN_DMA_API;
+}
+
 enum iommu_cap {
 	IOMMU_CAP_CACHE_COHERENCY,	/* IOMMU can enforce cache coherent DMA
 					   transactions */
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 15/24] iommu/amd: Prepare for multiple DMA domain types
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (13 preceding siblings ...)
  2021-07-28 15:58 ` [PATCH v2 14/24] iommu: Introduce explicit type for non-strict DMA domains Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-07-28 15:58 ` [PATCH v2 16/24] iommu/arm-smmu: " Robin Murphy
                   ` (11 subsequent siblings)
  26 siblings, 0 replies; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders

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(-)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 0fd98d35d73b..02f9b4fffe90 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -1707,14 +1707,9 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)
 
 static void amd_iommu_probe_finalize(struct device *dev)
 {
-	struct iommu_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);
 }
 
 static void amd_iommu_release_device(struct device *dev)
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 16/24] iommu/arm-smmu: Prepare for multiple DMA domain types
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (14 preceding siblings ...)
  2021-07-28 15:58 ` [PATCH v2 15/24] iommu/amd: Prepare for multiple DMA domain types Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-07-28 15:58 ` [PATCH v2 17/24] iommu/vt-d: " Robin Murphy
                   ` (10 subsequent siblings)
  26 siblings, 0 replies; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders

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(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 4c648da447bf..a1f0d83d1eb5 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1972,6 +1972,7 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 
 	if (type != IOMMU_DOMAIN_UNMANAGED &&
 	    type != IOMMU_DOMAIN_DMA &&
+	    type != IOMMU_DOMAIN_DMA_FQ &&
 	    type != IOMMU_DOMAIN_IDENTITY)
 		return NULL;
 
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 970d9e4dcd69..936c5e9d5e82 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -869,7 +869,8 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 	struct arm_smmu_domain *smmu_domain;
 
 	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_IDENTITY) {
-		if (using_legacy_binding || type != IOMMU_DOMAIN_DMA)
+		if (using_legacy_binding ||
+		    (type != IOMMU_DOMAIN_DMA && type != IOMMU_DOMAIN_DMA_FQ))
 			return NULL;
 	}
 	/*
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 17/24] iommu/vt-d: Prepare for multiple DMA domain types
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (15 preceding siblings ...)
  2021-07-28 15:58 ` [PATCH v2 16/24] iommu/arm-smmu: " Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-07-30  6:09   ` Lu Baolu
  2021-07-28 15:58 ` [PATCH v2 18/24] iommu: Express DMA strictness via the domain type Robin Murphy
                   ` (9 subsequent siblings)
  26 siblings, 1 reply; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders

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.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/intel/iommu.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 7e168634c433..8fc46c9d6b96 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -582,7 +582,7 @@ struct intel_iommu *domain_get_iommu(struct dmar_domain *domain)
 	int iommu_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)))
 		return NULL;
 
 	for_each_domain_iommu(iommu_id, domain)
@@ -1034,7 +1034,7 @@ static struct dma_pte *pfn_to_dma_pte(struct dmar_domain *domain,
 			pteval = ((uint64_t)virt_to_dma_pfn(tmp_page) << VTD_PAGE_SHIFT) | DMA_PTE_READ | DMA_PTE_WRITE;
 			if (domain_use_first_level(domain)) {
 				pteval |= DMA_FL_PTE_XD | DMA_FL_PTE_US;
-				if (domain->domain.type == IOMMU_DOMAIN_DMA)
+				if (iommu_is_dma_domain(&domain->domain))
 					pteval |= DMA_FL_PTE_ACCESS;
 			}
 			if (cmpxchg64(&pte->val, 0ULL, pteval))
@@ -2345,7 +2345,7 @@ __domain_mapping(struct dmar_domain *domain, unsigned long iov_pfn,
 	if (domain_use_first_level(domain)) {
 		attr |= DMA_FL_PTE_XD | DMA_FL_PTE_US;
 
-		if (domain->domain.type == IOMMU_DOMAIN_DMA) {
+		if (iommu_is_dma_domain(&domain->domain)) {
 			attr |= DMA_FL_PTE_ACCESS;
 			if (prot & DMA_PTE_WRITE)
 				attr |= DMA_FL_PTE_DIRTY;
@@ -4528,6 +4528,7 @@ static struct iommu_domain *intel_iommu_domain_alloc(unsigned type)
 
 	switch (type) {
 	case IOMMU_DOMAIN_DMA:
+	case IOMMU_DOMAIN_DMA_FQ:
 	case IOMMU_DOMAIN_UNMANAGED:
 		dmar_domain = alloc_domain(0);
 		if (!dmar_domain) {
@@ -5197,12 +5198,8 @@ static void intel_iommu_release_device(struct device *dev)
 
 static void intel_iommu_probe_finalize(struct device *dev)
 {
-	struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
-
-	if (domain && 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);
 }
 
 static void intel_iommu_get_resv_regions(struct device *device,
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 18/24] iommu: Express DMA strictness via the domain type
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (16 preceding siblings ...)
  2021-07-28 15:58 ` [PATCH v2 17/24] iommu/vt-d: " Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-07-29  7:13   ` Lu Baolu
  2021-07-28 15:58 ` [PATCH v2 19/24] iommu: Expose DMA domain strictness via sysfs Robin Murphy
                   ` (8 subsequent siblings)
  26 siblings, 1 reply; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders

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.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  2 +-
 drivers/iommu/arm/arm-smmu/arm-smmu.c       |  2 +-
 drivers/iommu/dma-iommu.c                   |  9 +++++----
 drivers/iommu/iommu.c                       | 12 +++---------
 include/linux/iommu.h                       |  1 -
 5 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index a1f0d83d1eb5..19400826eba7 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2175,7 +2175,7 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain,
 		.iommu_dev	= smmu->dev,
 	};
 
-	if (!iommu_get_dma_strict(domain))
+	if (domain->type == IOMMU_DOMAIN_DMA_FQ)
 		pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
 
 	pgtbl_ops = alloc_io_pgtable_ops(fmt, &pgtbl_cfg, smmu_domain);
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 936c5e9d5e82..109e4723f9f5 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -765,7 +765,7 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 		.iommu_dev	= smmu->dev,
 	};
 
-	if (!iommu_get_dma_strict(domain))
+	if (domain->type == IOMMU_DOMAIN_DMA_FQ)
 		pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
 
 	if (smmu->impl && smmu->impl->init_context) {
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 8b3545c01077..7f3968865387 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -363,13 +363,14 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
 
 	init_iova_domain(iovad, 1UL << order, base_pfn);
 
-	if (!cookie->fq_domain && !dev_is_untrusted(dev) &&
-	    domain->ops->flush_iotlb_all && !iommu_get_dma_strict(domain)) {
+	if (domain->type == IOMMU_DOMAIN_DMA_FQ && !cookie->fq_domain) {
 		if (init_iova_flush_queue(iovad, iommu_dma_flush_iotlb_all,
-					  iommu_dma_entry_dtor))
+					  iommu_dma_entry_dtor)) {
 			pr_warn("iova flush queue initialization failed\n");
-		else
+			domain->type = IOMMU_DOMAIN_DMA;
+		} else {
 			cookie->fq_domain = domain;
+		}
 	}
 
 	return iova_reserve_iommu_regions(dev, domain);
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 982545234cf3..eecb5657de69 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -136,6 +136,9 @@ static int __init iommu_subsys_init(void)
 		}
 	}
 
+	if (!iommu_default_passthrough() && !iommu_dma_strict)
+		iommu_def_domain_type = IOMMU_DOMAIN_DMA_FQ;
+
 	pr_info("Default domain type: %s %s\n",
 		iommu_domain_type_str(iommu_def_domain_type),
 		(iommu_cmd_line & IOMMU_CMD_LINE_DMA_API) ?
@@ -357,15 +360,6 @@ void iommu_set_dma_strict(void)
 	iommu_dma_strict = true;
 }
 
-bool iommu_get_dma_strict(struct iommu_domain *domain)
-{
-	/* only allow lazy flushing for DMA domains */
-	if (domain->type == IOMMU_DOMAIN_DMA)
-		return iommu_dma_strict;
-	return true;
-}
-EXPORT_SYMBOL_GPL(iommu_get_dma_strict);
-
 static ssize_t iommu_group_attr_show(struct kobject *kobj,
 				     struct attribute *__attr, char *buf)
 {
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 046ba4d54cd2..edfe2fdb8368 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -498,7 +498,6 @@ int iommu_set_pgtable_quirks(struct iommu_domain *domain,
 		unsigned long quirks);
 
 void iommu_set_dma_strict(void);
-bool iommu_get_dma_strict(struct iommu_domain *domain);
 
 extern int report_iommu_fault(struct iommu_domain *domain, struct device *dev,
 			      unsigned long iova, int flags);
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 19/24] iommu: Expose DMA domain strictness via sysfs
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (17 preceding siblings ...)
  2021-07-28 15:58 ` [PATCH v2 18/24] iommu: Express DMA strictness via the domain type Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-07-30  6:10   ` Lu Baolu
                     ` (2 more replies)
  2021-07-28 15:58 ` [PATCH v2 20/24] iommu: Merge strictness and domain type configs Robin Murphy
                   ` (7 subsequent siblings)
  26 siblings, 3 replies; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders

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.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 Documentation/ABI/testing/sysfs-kernel-iommu_groups | 2 ++
 drivers/iommu/iommu.c                               | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-kernel-iommu_groups b/Documentation/ABI/testing/sysfs-kernel-iommu_groups
index eae2f1c1e11e..43ba764ba5b7 100644
--- a/Documentation/ABI/testing/sysfs-kernel-iommu_groups
+++ b/Documentation/ABI/testing/sysfs-kernel-iommu_groups
@@ -42,6 +42,8 @@ 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.
 		identity  All the DMA transactions from the device in this group
 		          are not translated by the iommu.
 		auto      Change to the type the device was booted with.
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index eecb5657de69..5a08e0806cbb 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -3265,6 +3265,8 @@ static ssize_t iommu_group_store_type(struct iommu_group *group,
 		req_type = IOMMU_DOMAIN_IDENTITY;
 	else if (sysfs_streq(buf, "DMA"))
 		req_type = IOMMU_DOMAIN_DMA;
+	else if (sysfs_streq(buf, "DMA-FQ"))
+		req_type = IOMMU_DOMAIN_DMA_FQ;
 	else if (sysfs_streq(buf, "auto"))
 		req_type = 0;
 	else
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 20/24] iommu: Merge strictness and domain type configs
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (18 preceding siblings ...)
  2021-07-28 15:58 ` [PATCH v2 19/24] iommu: Expose DMA domain strictness via sysfs Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-07-30  6:10   ` Lu Baolu
                     ` (2 more replies)
  2021-07-28 15:58 ` [PATCH v2 21/24] iommu/dma: Factor out flush queue init Robin Murphy
                   ` (6 subsequent siblings)
  26 siblings, 3 replies; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders

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>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/Kconfig | 80 +++++++++++++++++++++----------------------
 drivers/iommu/iommu.c |  2 +-
 2 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index c84da8205be7..6e06f876d75a 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -79,55 +79,55 @@ config IOMMU_DEBUGFS
 	  debug/iommu directory, and then populate a subdirectory with
 	  entries as required.
 
-config IOMMU_DEFAULT_PASSTHROUGH
-	bool "IOMMU passthrough by default"
-	depends on IOMMU_API
-	help
-	  Enable passthrough by default, removing the need to pass in
-	  iommu.passthrough=on or iommu=pt through command line. If this
-	  is enabled, you can still disable with iommu.passthrough=off
-	  or iommu=nopt depending on the architecture.
-
-	  If unsure, say N here.
-
 choice
-	prompt "IOMMU default DMA IOTLB invalidation mode"
-	depends on IOMMU_DMA
-
-	default IOMMU_DEFAULT_LAZY if (AMD_IOMMU || INTEL_IOMMU)
-	default IOMMU_DEFAULT_STRICT
+	prompt "IOMMU default domain type"
+	depends on IOMMU_API
+	default IOMMU_DEFAULT_DMA_LAZY if AMD_IOMMU || INTEL_IOMMU
+	default IOMMU_DEFAULT_DMA_STRICT
 	help
-	  This option allows an IOMMU DMA IOTLB invalidation mode to be
-	  chosen at build time, to override the default mode of each ARCH,
-	  removing the need to pass in kernel parameters through command line.
-	  It is still possible to provide common boot params to override this
-	  config.
+	  Choose the type of IOMMU domain used to manage DMA API usage by
+	  device drivers. The options here typically represent different
+	  levels of tradeoff between robustness/security and performance,
+	  depending on the IOMMU driver. Not all IOMMUs support all options.
+	  This choice can be overridden at boot via the command line, and for
+	  some devices also at runtime via sysfs.
 
 	  If unsure, keep the default.
 
-config IOMMU_DEFAULT_STRICT
-	bool "strict"
+config IOMMU_DEFAULT_DMA_STRICT
+	bool "Translated - Strict"
 	help
-	  For every IOMMU DMA unmap operation, the flush operation of IOTLB and
-	  the free operation of IOVA are guaranteed to be done in the unmap
-	  function.
+	  Trusted devices use translation to restrict their access to only
+	  DMA-mapped pages, with strict TLB invalidation on unmap. Equivalent
+	  to passing "iommu.passthrough=0 iommu.strict=1" on the command line.
 
-config IOMMU_DEFAULT_LAZY
-	bool "lazy"
+	  Untrusted devices always use this mode, with an additional layer of
+	  bounce-buffering such that they cannot gain access to any unrelated
+	  data within a mapped page.
+
+config IOMMU_DEFAULT_DMA_LAZY
+	bool "Translated - Lazy"
 	help
-	  Support lazy mode, where for every IOMMU DMA unmap operation, the
-	  flush operation of IOTLB and the free operation of IOVA are deferred.
-	  They are only guaranteed to be done before the related IOVA will be
-	  reused.
+	  Trusted devices use translation to restrict their access to only
+	  DMA-mapped pages, but with "lazy" batched TLB invalidation. This
+	  mode allows higher performance with some IOMMUs due to reduced TLB
+	  flushing, but at the cost of reduced isolation since devices may be
+	  able to access memory for some time after it has been unmapped.
+	  Equivalent to passing "iommu.passthrough=0 iommu.strict=0" on the
+	  command line.
 
-	  The isolation provided in this mode is not as secure as STRICT mode,
-	  such that a vulnerable time window may be created between the DMA
-	  unmap and the mappings cached in the IOMMU IOTLB or device TLB
-	  finally being invalidated, where the device could still access the
-	  memory which has already been unmapped by the device driver.
-	  However this mode may provide better performance in high throughput
-	  scenarios, and is still considerably more secure than passthrough
-	  mode or no IOMMU.
+	  If this mode is not supported by the IOMMU driver, the effective
+	  runtime default will fall back to IOMMU_DEFAULT_DMA_STRICT.
+
+config IOMMU_DEFAULT_PASSTHROUGH
+	bool "Passthrough"
+	help
+	  Trusted devices are identity-mapped, giving them unrestricted access
+	  to memory with minimal performance overhead. Equivalent to passing
+	  "iommu.passthrough=1" (historically "iommu=pt") on the command line.
+
+	  If this mode is not supported by the IOMMU driver, the effective
+	  runtime default will fall back to IOMMU_DEFAULT_DMA_STRICT.
 
 endchoice
 
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 5a08e0806cbb..25c1adc1ec67 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -31,7 +31,7 @@ static struct kset *iommu_group_kset;
 static DEFINE_IDA(iommu_group_ida);
 
 static unsigned int iommu_def_domain_type __read_mostly;
-static bool iommu_dma_strict __read_mostly = IS_ENABLED(CONFIG_IOMMU_DEFAULT_STRICT);
+static bool iommu_dma_strict __read_mostly = IS_ENABLED(CONFIG_IOMMU_DEFAULT_DMA_STRICT);
 static u32 iommu_cmd_line __read_mostly;
 
 struct iommu_group {
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 21/24] iommu/dma: Factor out flush queue init
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (19 preceding siblings ...)
  2021-07-28 15:58 ` [PATCH v2 20/24] iommu: Merge strictness and domain type configs Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-07-30  6:11   ` Lu Baolu
  2021-07-30  9:20   ` John Garry
  2021-07-28 15:58 ` [PATCH v2 22/24] iommu: Allow enabling non-strict mode dynamically Robin Murphy
                   ` (5 subsequent siblings)
  26 siblings, 2 replies; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders

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.

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(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 7f3968865387..304a3ec71223 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -310,6 +310,25 @@ static bool dev_is_untrusted(struct device *dev)
 	return dev_is_pci(dev) && to_pci_dev(dev)->untrusted;
 }
 
+int iommu_dma_init_fq(struct iommu_domain *domain)
+{
+	struct iommu_dma_cookie *cookie = domain->iova_cookie;
+
+	if (domain->type != IOMMU_DOMAIN_DMA_FQ)
+		return -EINVAL;
+	if (cookie->fq_domain)
+		return 0;
+
+	if (init_iova_flush_queue(&cookie->iovad, iommu_dma_flush_iotlb_all,
+				  iommu_dma_entry_dtor)) {
+		pr_warn("iova flush queue initialization failed\n");
+		domain->type = IOMMU_DOMAIN_DMA;
+		return -ENODEV;
+	}
+	cookie->fq_domain = domain;
+	return 0;
+}
+
 /**
  * iommu_dma_init_domain - Initialise a DMA mapping domain
  * @domain: IOMMU domain previously prepared by iommu_get_dma_cookie()
@@ -362,16 +381,7 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
 	}
 
 	init_iova_domain(iovad, 1UL << order, base_pfn);
-
-	if (domain->type == IOMMU_DOMAIN_DMA_FQ && !cookie->fq_domain) {
-		if (init_iova_flush_queue(iovad, iommu_dma_flush_iotlb_all,
-					  iommu_dma_entry_dtor)) {
-			pr_warn("iova flush queue initialization failed\n");
-			domain->type = IOMMU_DOMAIN_DMA;
-		} else {
-			cookie->fq_domain = domain;
-		}
-	}
+	iommu_dma_init_fq(domain);
 
 	return iova_reserve_iommu_regions(dev, domain);
 }
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 758ca4694257..81ab647f1618 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -20,6 +20,7 @@ void iommu_put_dma_cookie(struct iommu_domain *domain);
 
 /* Setup call for arch DMA mapping code */
 void iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 dma_limit);
+int iommu_dma_init_fq(struct iommu_domain *domain);
 
 /* The DMA API isn't _quite_ the whole story, though... */
 /*
@@ -37,9 +38,6 @@ void iommu_dma_compose_msi_msg(struct msi_desc *desc,
 
 void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list);
 
-void iommu_dma_free_cpu_cached_iovas(unsigned int cpu,
-		struct iommu_domain *domain);
-
 extern bool iommu_dma_forcedac;
 
 #else /* CONFIG_IOMMU_DMA */
@@ -54,6 +52,11 @@ static inline void iommu_setup_dma_ops(struct device *dev, u64 dma_base,
 {
 }
 
+static inline int iommu_dma_init_fq(struct iommu_domain *domain)
+{
+	return -EINVAL;
+}
+
 static inline int iommu_get_dma_cookie(struct iommu_domain *domain)
 {
 	return -ENODEV;
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 22/24] iommu: Allow enabling non-strict mode dynamically
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (20 preceding siblings ...)
  2021-07-28 15:58 ` [PATCH v2 21/24] iommu/dma: Factor out flush queue init Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-07-30  6:11   ` Lu Baolu
  2021-07-30  9:24   ` John Garry
  2021-07-28 15:58 ` [PATCH v2 23/24] iommu/arm-smmu: Allow non-strict in pgtable_quirks interface Robin Murphy
                   ` (4 subsequent siblings)
  26 siblings, 2 replies; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders

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.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/iommu.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 25c1adc1ec67..be399d630953 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -3200,6 +3200,13 @@ static int iommu_change_dev_def_domain(struct iommu_group *group,
 		goto out;
 	}
 
+	/* 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);
+		goto out;
+	}
+
 	/* Sets group->default_domain to the newly allocated domain */
 	ret = iommu_group_alloc_default_domain(dev->bus, group, type);
 	if (ret)
@@ -3240,9 +3247,9 @@ static int iommu_change_dev_def_domain(struct iommu_group *group,
 }
 
 /*
- * Changing the default domain through sysfs requires the users to ubind the
- * drivers from the devices in the iommu group. Return failure if this doesn't
- * meet.
+ * Changing the default domain through sysfs requires the users to unbind the
+ * drivers from the devices in the iommu group, except for a DMA -> DMA-FQ
+ * transition. Return failure if this isn't met.
  *
  * We need to consider the race between this and the device release path.
  * device_lock(dev) is used here to guarantee that the device release path
@@ -3318,7 +3325,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;
 		goto out;
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 23/24] iommu/arm-smmu: Allow non-strict in pgtable_quirks interface
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (21 preceding siblings ...)
  2021-07-28 15:58 ` [PATCH v2 22/24] iommu: Allow enabling non-strict mode dynamically Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-08-02 13:04   ` Will Deacon
  2021-07-28 15:58 ` [PATCH v2 24/24] iommu: Only log strictness for DMA domains Robin Murphy
                   ` (3 subsequent siblings)
  26 siblings, 1 reply; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders

To make io-pgtable aware of a flush queue being dynamically enabled,
allow IO_PGTABLE_QUIRK_NON_STRICT to be set even after a domain has been
attached to, and hook up the final piece of the puzzle in iommu-dma.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 15 +++++++++++++++
 drivers/iommu/arm/arm-smmu/arm-smmu.c       | 11 +++++++++++
 drivers/iommu/dma-iommu.c                   |  3 +++
 3 files changed, 29 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 19400826eba7..40fa9cb382c3 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2711,6 +2711,20 @@ static int arm_smmu_enable_nesting(struct iommu_domain *domain)
 	return ret;
 }
 
+static int arm_smmu_set_pgtable_quirks(struct iommu_domain *domain,
+		unsigned long quirks)
+{
+	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+
+	if (quirks == IO_PGTABLE_QUIRK_NON_STRICT && smmu_domain->pgtbl_ops) {
+		struct io_pgtable *iop = io_pgtable_ops_to_pgtable(smmu_domain->pgtbl_ops);
+
+		iop->cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
+		return 0;
+	}
+	return -EINVAL;
+}
+
 static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
 {
 	return iommu_fwspec_add_ids(dev, args->args, 1);
@@ -2825,6 +2839,7 @@ static struct iommu_ops arm_smmu_ops = {
 	.release_device		= arm_smmu_release_device,
 	.device_group		= arm_smmu_device_group,
 	.enable_nesting		= arm_smmu_enable_nesting,
+	.set_pgtable_quirks	= arm_smmu_set_pgtable_quirks,
 	.of_xlate		= arm_smmu_of_xlate,
 	.get_resv_regions	= arm_smmu_get_resv_regions,
 	.put_resv_regions	= generic_iommu_put_resv_regions,
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 109e4723f9f5..f18684f308b9 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1518,6 +1518,17 @@ static int arm_smmu_set_pgtable_quirks(struct iommu_domain *domain,
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
 	int ret = 0;
 
+	if (quirks == IO_PGTABLE_QUIRK_NON_STRICT) {
+		struct io_pgtable *iop;
+
+		if (!smmu_domain->pgtbl_ops)
+			return -EINVAL;
+
+		iop = io_pgtable_ops_to_pgtable(smmu_domain->pgtbl_ops);
+		iop->cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
+		return 0;
+	}
+
 	mutex_lock(&smmu_domain->init_mutex);
 	if (smmu_domain->smmu)
 		ret = -EPERM;
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 304a3ec71223..6e3eca778267 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -16,6 +16,7 @@
 #include <linux/huge_mm.h>
 #include <linux/iommu.h>
 #include <linux/iova.h>
+#include <linux/io-pgtable.h>
 #include <linux/irq.h>
 #include <linux/mm.h>
 #include <linux/mutex.h>
@@ -326,6 +327,8 @@ int iommu_dma_init_fq(struct iommu_domain *domain)
 		return -ENODEV;
 	}
 	cookie->fq_domain = domain;
+	if (domain->ops->set_pgtable_quirks)
+		domain->ops->set_pgtable_quirks(domain, IO_PGTABLE_QUIRK_NON_STRICT);
 	return 0;
 }
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 24/24] iommu: Only log strictness for DMA domains
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (22 preceding siblings ...)
  2021-07-28 15:58 ` [PATCH v2 23/24] iommu/arm-smmu: Allow non-strict in pgtable_quirks interface Robin Murphy
@ 2021-07-28 15:58 ` Robin Murphy
  2021-07-29  9:04   ` John Garry
  2021-07-30  6:12   ` Lu Baolu
  2021-07-29  2:55 ` [PATCH v2 00/24] iommu: Refactor DMA domain strictness chenxiang (M)
                   ` (2 subsequent siblings)
  26 siblings, 2 replies; 65+ messages in thread
From: Robin Murphy @ 2021-07-28 15:58 UTC (permalink / raw)
  To: joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders

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.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/iommu.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index be399d630953..87d7b299436e 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -144,10 +144,11 @@ static int __init iommu_subsys_init(void)
 		(iommu_cmd_line & IOMMU_CMD_LINE_DMA_API) ?
 			"(set via kernel command line)" : "");
 
-	pr_info("DMA domain TLB invalidation policy: %s mode %s\n",
-		iommu_dma_strict ? "strict" : "lazy",
-		(iommu_cmd_line & IOMMU_CMD_LINE_STRICT) ?
-			"(set via kernel command line)" : "");
+	if (!iommu_default_passthrough())
+		pr_info("DMA domain TLB invalidation policy: %s mode %s\n",
+			iommu_dma_strict ? "strict" : "lazy",
+			(iommu_cmd_line & IOMMU_CMD_LINE_STRICT) ?
+				"(set via kernel command line)" : "");
 
 	return 0;
 }
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 00/24] iommu: Refactor DMA domain strictness
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (23 preceding siblings ...)
  2021-07-28 15:58 ` [PATCH v2 24/24] iommu: Only log strictness for DMA domains Robin Murphy
@ 2021-07-29  2:55 ` chenxiang (M)
  2021-07-29 10:59   ` Robin Murphy
  2021-07-29 15:04 ` Heiko Stübner
  2021-07-29 22:33 ` Doug Anderson
  26 siblings, 1 reply; 65+ messages in thread
From: chenxiang (M) @ 2021-07-29  2:55 UTC (permalink / raw)
  To: Robin Murphy, joro, will
  Cc: Maxime Ripard, Jean-Philippe Brucker, Heiko Stuebner,
	Geert Uytterhoeven, linux-kernel, Chunyan Zhang, dianders, iommu,
	linux-arm-kernel, linuxarm

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(-)
>



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 18/24] iommu: Express DMA strictness via the domain type
  2021-07-28 15:58 ` [PATCH v2 18/24] iommu: Express DMA strictness via the domain type Robin Murphy
@ 2021-07-29  7:13   ` Lu Baolu
  2021-07-29  9:36     ` Robin Murphy
  0 siblings, 1 reply; 65+ messages in thread
From: Lu Baolu @ 2021-07-29  7:13 UTC (permalink / raw)
  To: Robin Murphy, joro, will
  Cc: baolu.lu, iommu, linux-arm-kernel, linux-kernel,
	suravee.suthikulpanit, john.garry, dianders

Hi Robin,

On 7/28/21 11:58 PM, Robin Murphy wrote:
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 982545234cf3..eecb5657de69 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -136,6 +136,9 @@ static int __init iommu_subsys_init(void)
>   		}
>   	}
>   
> +	if (!iommu_default_passthrough() && !iommu_dma_strict)
> +		iommu_def_domain_type = IOMMU_DOMAIN_DMA_FQ;

iommu_dma_strict could be changed later by the vendor iommu driver via
iommu_set_dma_strict(). This seems not to be the right place to set
iommu_def_domain_type.

> +
>   	pr_info("Default domain type: %s %s\n",
>   		iommu_domain_type_str(iommu_def_domain_type),
>   		(iommu_cmd_line & IOMMU_CMD_LINE_DMA_API) ?

Best regards,
baolu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 24/24] iommu: Only log strictness for DMA domains
  2021-07-28 15:58 ` [PATCH v2 24/24] iommu: Only log strictness for DMA domains Robin Murphy
@ 2021-07-29  9:04   ` John Garry
  2021-07-30  6:12   ` Lu Baolu
  1 sibling, 0 replies; 65+ messages in thread
From: John Garry @ 2021-07-29  9:04 UTC (permalink / raw)
  To: Robin Murphy, joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, dianders

On 28/07/2021 16:58, Robin Murphy wrote:
> 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.
> 
> Signed-off-by: Robin Murphy<robin.murphy@arm.com>

FWIW, small comment below,

Reviewed-by: John Garry <john.garry@huawei.com>

> ---
>   drivers/iommu/iommu.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index be399d630953..87d7b299436e 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -144,10 +144,11 @@ static int __init iommu_subsys_init(void)
>   		(iommu_cmd_line & IOMMU_CMD_LINE_DMA_API) ?
>   			"(set via kernel command line)" : "");
>   
> -	pr_info("DMA domain TLB invalidation policy: %s mode %s\n",
> -		iommu_dma_strict ? "strict" : "lazy",
> -		(iommu_cmd_line & IOMMU_CMD_LINE_STRICT) ?
> -			"(set via kernel command line)" : "");
> +	if (!iommu_default_passthrough())

I suppose that you could also do an early return to save indenting...


> +		pr_info("DMA domain TLB invalidation policy: %s mode %s\n",
> +			iommu_dma_strict ? "strict" : "lazy",
> +			(iommu_cmd_line & IOMMU_CMD_LINE_STRICT) ?
> +				"(set via kernel command line)" : "");
>   
>   	return 0;
>   }


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 18/24] iommu: Express DMA strictness via the domain type
  2021-07-29  7:13   ` Lu Baolu
@ 2021-07-29  9:36     ` Robin Murphy
  2021-07-29 12:42       ` Lu Baolu
  2021-07-30  6:09       ` Lu Baolu
  0 siblings, 2 replies; 65+ messages in thread
From: Robin Murphy @ 2021-07-29  9:36 UTC (permalink / raw)
  To: Lu Baolu, joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	john.garry, dianders

On 2021-07-29 08:13, Lu Baolu wrote:
> Hi Robin,
> 
> On 7/28/21 11:58 PM, Robin Murphy wrote:
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 982545234cf3..eecb5657de69 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -136,6 +136,9 @@ static int __init iommu_subsys_init(void)
>>           }
>>       }
>> +    if (!iommu_default_passthrough() && !iommu_dma_strict)
>> +        iommu_def_domain_type = IOMMU_DOMAIN_DMA_FQ;
> 
> iommu_dma_strict could be changed later by the vendor iommu driver via
> iommu_set_dma_strict(). This seems not to be the right place to set
> iommu_def_domain_type.

Ah yes, good catch once again, thanks!

I think this *is* the right place to initially set it to honour the 
command-line option, since that matches what we do for passthrough. 
However also like passthrough we'll need to keep things in sync if it's 
updated later, like this:


diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 87d7b299436e..593d4555bc57 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -359,6 +359,8 @@ early_param("iommu.strict", iommu_dma_setup);
  void iommu_set_dma_strict(void)
  {
         iommu_dma_strict = true;
+       if (iommu_def_domain_type == IOMMU_DOMAIN_DMA_FQ)
+               iommu_def_domain_type = IOMMU_DOMAIN_DMA;
  }

  static ssize_t iommu_group_attr_show(struct kobject *kobj,


Does that seem reasonable? I'm not sure there's any cleaner way to do it 
since we don't want to inadvertently clobber the default type if the 
user has given us something funky like "intel_iommu=strict 
iommu.passthrough=1".

Cheers,
Robin.

> 
>> +
>>       pr_info("Default domain type: %s %s\n",
>>           iommu_domain_type_str(iommu_def_domain_type),
>>           (iommu_cmd_line & IOMMU_CMD_LINE_DMA_API) ?
> 
> Best regards,
> baolu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 00/24] iommu: Refactor DMA domain strictness
  2021-07-29  2:55 ` [PATCH v2 00/24] iommu: Refactor DMA domain strictness chenxiang (M)
@ 2021-07-29 10:59   ` Robin Murphy
  2021-07-30  1:21     ` chenxiang (M)
  0 siblings, 1 reply; 65+ messages in thread
From: Robin Murphy @ 2021-07-29 10:59 UTC (permalink / raw)
  To: chenxiang (M), joro, will
  Cc: Maxime Ripard, Jean-Philippe Brucker, Heiko Stuebner,
	Geert Uytterhoeven, linux-kernel, Chunyan Zhang, dianders, iommu,
	linux-arm-kernel, linuxarm

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(-)
>>
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 18/24] iommu: Express DMA strictness via the domain type
  2021-07-29  9:36     ` Robin Murphy
@ 2021-07-29 12:42       ` Lu Baolu
  2021-07-30  6:09       ` Lu Baolu
  1 sibling, 0 replies; 65+ messages in thread
From: Lu Baolu @ 2021-07-29 12:42 UTC (permalink / raw)
  To: Robin Murphy, joro, will
  Cc: baolu.lu, iommu, linux-arm-kernel, linux-kernel,
	suravee.suthikulpanit, john.garry, dianders

On 2021/7/29 17:36, Robin Murphy wrote:
> On 2021-07-29 08:13, Lu Baolu wrote:
>> Hi Robin,
>>
>> On 7/28/21 11:58 PM, Robin Murphy wrote:
>>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>>> index 982545234cf3..eecb5657de69 100644
>>> --- a/drivers/iommu/iommu.c
>>> +++ b/drivers/iommu/iommu.c
>>> @@ -136,6 +136,9 @@ static int __init iommu_subsys_init(void)
>>>           }
>>>       }
>>> +    if (!iommu_default_passthrough() && !iommu_dma_strict)
>>> +        iommu_def_domain_type = IOMMU_DOMAIN_DMA_FQ;
>>
>> iommu_dma_strict could be changed later by the vendor iommu driver via
>> iommu_set_dma_strict(). This seems not to be the right place to set
>> iommu_def_domain_type.
> 
> Ah yes, good catch once again, thanks!
> 
> I think this *is* the right place to initially set it to honour the 
> command-line option, since that matches what we do for passthrough. 
> However also like passthrough we'll need to keep things in sync if it's 
> updated later, like this:
> 
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 87d7b299436e..593d4555bc57 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -359,6 +359,8 @@ early_param("iommu.strict", iommu_dma_setup);
>   void iommu_set_dma_strict(void)
>   {
>          iommu_dma_strict = true;
> +       if (iommu_def_domain_type == IOMMU_DOMAIN_DMA_FQ)
> +               iommu_def_domain_type = IOMMU_DOMAIN_DMA;
>   }
> 
>   static ssize_t iommu_group_attr_show(struct kobject *kobj,
> 
> 
> Does that seem reasonable? I'm not sure there's any cleaner way to do it 
> since we don't want to inadvertently clobber the default type if the 
> user has given us something funky like "intel_iommu=strict 
> iommu.passthrough=1".

Yeah! It's reasonable as far as I can see.

Best regards,
baolu

> 
> Cheers,
> Robin.
> 
>>
>>> +
>>>       pr_info("Default domain type: %s %s\n",
>>>           iommu_domain_type_str(iommu_def_domain_type),
>>>           (iommu_cmd_line & IOMMU_CMD_LINE_DMA_API) ?
>>
>> Best regards,
>> baolu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 00/24] iommu: Refactor DMA domain strictness
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (24 preceding siblings ...)
  2021-07-29  2:55 ` [PATCH v2 00/24] iommu: Refactor DMA domain strictness chenxiang (M)
@ 2021-07-29 15:04 ` Heiko Stübner
  2021-07-29 15:43   ` Robin Murphy
  2021-07-29 22:33 ` Doug Anderson
  26 siblings, 1 reply; 65+ messages in thread
From: Heiko Stübner @ 2021-07-29 15:04 UTC (permalink / raw)
  To: joro, will, Robin Murphy
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders, Marek Szyprowski,
	Yoshihiro Shimoda, Geert Uytterhoeven, Yong Wu, Chunyan Zhang,
	Maxime Ripard, Jean-Philippe Brucker

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(-)
> 
> 





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 00/24] iommu: Refactor DMA domain strictness
  2021-07-29 15:04 ` Heiko Stübner
@ 2021-07-29 15:43   ` Robin Murphy
  2021-07-29 15:53     ` Heiko Stübner
  0 siblings, 1 reply; 65+ messages in thread
From: Robin Murphy @ 2021-07-29 15:43 UTC (permalink / raw)
  To: Heiko Stübner, joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders, Marek Szyprowski,
	Yoshihiro Shimoda, Geert Uytterhoeven, Yong Wu, Chunyan Zhang,
	Maxime Ripard, Jean-Philippe Brucker

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(-)
>>
>>
> 
> 
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 00/24] iommu: Refactor DMA domain strictness
  2021-07-29 15:43   ` Robin Murphy
@ 2021-07-29 15:53     ` Heiko Stübner
  2021-07-29 16:29       ` Robin Murphy
  0 siblings, 1 reply; 65+ messages in thread
From: Heiko Stübner @ 2021-07-29 15:53 UTC (permalink / raw)
  To: joro, will, Robin Murphy
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders, Marek Szyprowski,
	Yoshihiro Shimoda, Geert Uytterhoeven, Yong Wu, Chunyan Zhang,
	Maxime Ripard, Jean-Philippe Brucker

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(-)
> >>
> >>
> > 
> > 
> > 
> > 
> 





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 00/24] iommu: Refactor DMA domain strictness
  2021-07-29 15:53     ` Heiko Stübner
@ 2021-07-29 16:29       ` Robin Murphy
  0 siblings, 0 replies; 65+ messages in thread
From: Robin Murphy @ 2021-07-29 16:29 UTC (permalink / raw)
  To: Heiko Stübner, joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, john.garry, dianders, Marek Szyprowski,
	Yoshihiro Shimoda, Geert Uytterhoeven, Yong Wu, Chunyan Zhang,
	Maxime Ripard, Jean-Philippe Brucker

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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 00/24] iommu: Refactor DMA domain strictness
  2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
                   ` (25 preceding siblings ...)
  2021-07-29 15:04 ` Heiko Stübner
@ 2021-07-29 22:33 ` Doug Anderson
  2021-07-30  0:06   ` Doug Anderson
  26 siblings, 1 reply; 65+ messages in thread
From: Doug Anderson @ 2021-07-29 22:33 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Joerg Roedel, Will Deacon,
	list@263.net:IOMMU DRIVERS
	<iommu@lists.linux-foundation.org>,
	Joerg Roedel <joro@8bytes.org>, ,
	Linux ARM, LKML, suravee.suthikulpanit, Lu Baolu, John Garry,
	Marek Szyprowski, Yoshihiro Shimoda, Geert Uytterhoeven, Yong Wu,
	Heiko Stuebner, Chunyan Zhang, Maxime Ripard,
	Jean-Philippe Brucker

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>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 00/24] iommu: Refactor DMA domain strictness
  2021-07-29 22:33 ` Doug Anderson
@ 2021-07-30  0:06   ` Doug Anderson
  0 siblings, 0 replies; 65+ messages in thread
From: Doug Anderson @ 2021-07-30  0:06 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Joerg Roedel, Will Deacon,
	list@263.net:IOMMU DRIVERS
	<iommu@lists.linux-foundation.org>,
	Joerg Roedel <joro@8bytes.org>, ,
	Linux ARM, LKML, suravee.suthikulpanit, Lu Baolu, John Garry,
	Marek Szyprowski, Yoshihiro Shimoda, Geert Uytterhoeven, Yong Wu,
	Heiko Stuebner, Chunyan Zhang, Maxime Ripard,
	Jean-Philippe Brucker

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 00/24] iommu: Refactor DMA domain strictness
  2021-07-29 10:59   ` Robin Murphy
@ 2021-07-30  1:21     ` chenxiang (M)
  0 siblings, 0 replies; 65+ messages in thread
From: chenxiang (M) @ 2021-07-30  1:21 UTC (permalink / raw)
  To: Robin Murphy, joro, will
  Cc: Maxime Ripard, Jean-Philippe Brucker, Heiko Stuebner,
	Geert Uytterhoeven, linux-kernel, Chunyan Zhang, dianders, iommu,
	linux-arm-kernel, linuxarm



在 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(-)
>>>
>>
>>
>
> .
>



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 01/24] iommu: Pull IOVA cookie management into the core
  2021-07-28 15:58 ` [PATCH v2 01/24] iommu: Pull IOVA cookie management into the core Robin Murphy
@ 2021-07-30  6:06   ` Lu Baolu
  2021-07-30  9:32   ` Jean-Philippe Brucker
  1 sibling, 0 replies; 65+ messages in thread
From: Lu Baolu @ 2021-07-30  6:06 UTC (permalink / raw)
  To: Robin Murphy, joro, will
  Cc: baolu.lu, iommu, linux-arm-kernel, linux-kernel,
	suravee.suthikulpanit, john.garry, dianders, Marek Szyprowski,
	Yoshihiro Shimoda, Geert Uytterhoeven, Yong Wu, Heiko Stuebner,
	Chunyan Zhang, Maxime Ripard, Jean-Philippe Brucker

On 7/28/21 11:58 PM, 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 <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>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>   drivers/iommu/iommu.c | 7 +++++++
>   include/linux/iommu.h | 3 ++-
>   2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index f2cda9950bd5..ea5a9ea8d431 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -7,6 +7,7 @@
>   #define pr_fmt(fmt)    "iommu: " fmt
>   
>   #include <linux/device.h>
> +#include <linux/dma-iommu.h>
>   #include <linux/kernel.h>
>   #include <linux/bits.h>
>   #include <linux/bug.h>
> @@ -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 ignore -EEXIST while drivers still get their own cookies */
> +	if (type == IOMMU_DOMAIN_DMA && iommu_get_dma_cookie(domain) == -ENOMEM) {
> +		iommu_domain_free(domain);
> +		domain = NULL;
> +	}
>   	return domain;
>   }
>   
> @@ -1957,6 +1963,7 @@ EXPORT_SYMBOL_GPL(iommu_domain_alloc);
>   
>   void iommu_domain_free(struct iommu_domain *domain)
>   {
> +	iommu_put_dma_cookie(domain);
>   	domain->ops->domain_free(domain);
>   }
>   EXPORT_SYMBOL_GPL(iommu_domain_free);
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 4997c78e2670..141779d76035 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -40,6 +40,7 @@ struct iommu_domain;
>   struct notifier_block;
>   struct iommu_sva;
>   struct iommu_fault_event;
> +struct iommu_dma_cookie;
>   
>   /* iommu fault flags */
>   #define IOMMU_FAULT_READ	0x0
> @@ -86,7 +87,7 @@ struct iommu_domain {
>   	iommu_fault_handler_t handler;
>   	void *handler_token;
>   	struct iommu_domain_geometry geometry;
> -	void *iova_cookie;
> +	struct iommu_dma_cookie *iova_cookie;
>   };
>   
>   enum iommu_cap {
> 

Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

Best regards,
baolu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 04/24] iommu/vt-d: Drop IOVA cookie management
  2021-07-28 15:58 ` [PATCH v2 04/24] iommu/vt-d: " Robin Murphy
@ 2021-07-30  6:07   ` Lu Baolu
  0 siblings, 0 replies; 65+ messages in thread
From: Lu Baolu @ 2021-07-30  6:07 UTC (permalink / raw)
  To: Robin Murphy, joro, will
  Cc: baolu.lu, iommu, linux-arm-kernel, linux-kernel,
	suravee.suthikulpanit, john.garry, dianders

On 7/28/21 11:58 PM, Robin Murphy wrote:
> The core code bakes its own cookies now.
> 
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>   drivers/iommu/intel/iommu.c | 8 --------
>   1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index c12cc955389a..7e168634c433 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -1979,10 +1979,6 @@ static void domain_exit(struct dmar_domain *domain)
>   	/* Remove associated devices and clear attached or cached domains */
>   	domain_remove_dev_info(domain);
>   
> -	/* destroy iovas */
> -	if (domain->domain.type == IOMMU_DOMAIN_DMA)
> -		iommu_put_dma_cookie(&domain->domain);
> -
>   	if (domain->pgd) {
>   		struct page *freelist;
>   
> @@ -4544,10 +4540,6 @@ static struct iommu_domain *intel_iommu_domain_alloc(unsigned type)
>   			return NULL;
>   		}
>   
> -		if (type == IOMMU_DOMAIN_DMA &&
> -		    iommu_get_dma_cookie(&dmar_domain->domain))
> -			return NULL;
> -
>   		domain = &dmar_domain->domain;
>   		domain->geometry.aperture_start = 0;
>   		domain->geometry.aperture_end   =
> 


Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

Best regards,
baolu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 12/24] iommu/dma: Unexport IOVA cookie management
  2021-07-28 15:58 ` [PATCH v2 12/24] iommu/dma: Unexport " Robin Murphy
@ 2021-07-30  6:07   ` Lu Baolu
  2021-07-30  9:21   ` Jean-Philippe Brucker
  1 sibling, 0 replies; 65+ messages in thread
From: Lu Baolu @ 2021-07-30  6:07 UTC (permalink / raw)
  To: Robin Murphy, joro, will
  Cc: baolu.lu, iommu, linux-arm-kernel, linux-kernel,
	suravee.suthikulpanit, john.garry, dianders

On 7/28/21 11:58 PM, Robin Murphy wrote:
> 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.
> 
> 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(-)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 98ba927aee1a..10067fbc4309 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -98,9 +98,6 @@ static struct iommu_dma_cookie *cookie_alloc(enum iommu_dma_cookie_type type)
>   /**
>    * iommu_get_dma_cookie - Acquire DMA-API resources for a domain
>    * @domain: IOMMU domain to prepare for DMA-API usage
> - *
> - * IOMMU drivers should normally call this from their domain_alloc
> - * callback when domain->type == IOMMU_DOMAIN_DMA.
>    */
>   int iommu_get_dma_cookie(struct iommu_domain *domain)
>   {
> @@ -113,7 +110,6 @@ int iommu_get_dma_cookie(struct iommu_domain *domain)
>   
>   	return 0;
>   }
> -EXPORT_SYMBOL(iommu_get_dma_cookie);
>   
>   /**
>    * iommu_get_msi_cookie - Acquire just MSI remapping resources
> @@ -151,8 +147,6 @@ EXPORT_SYMBOL(iommu_get_msi_cookie);
>    * iommu_put_dma_cookie - Release a domain's DMA mapping resources
>    * @domain: IOMMU domain previously prepared by iommu_get_dma_cookie() or
>    *          iommu_get_msi_cookie()
> - *
> - * IOMMU drivers should normally call this from their domain_free callback.
>    */
>   void iommu_put_dma_cookie(struct iommu_domain *domain)
>   {
> @@ -172,7 +166,6 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
>   	kfree(cookie);
>   	domain->iova_cookie = NULL;
>   }
> -EXPORT_SYMBOL(iommu_put_dma_cookie);
>   
>   /**
>    * iommu_dma_get_resv_regions - Reserved region driver helper
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index ea5a9ea8d431..fa8109369f74 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -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 ignore -EEXIST while drivers still get their own cookies */
> -	if (type == IOMMU_DOMAIN_DMA && iommu_get_dma_cookie(domain) == -ENOMEM) {
> +	if (type == IOMMU_DOMAIN_DMA && iommu_get_dma_cookie(domain)) {
>   		iommu_domain_free(domain);
>   		domain = NULL;
>   	}
> 


Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

Best regards,
baolu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 13/24] iommu/dma: Remove redundant "!dev" checks
  2021-07-28 15:58 ` [PATCH v2 13/24] iommu/dma: Remove redundant "!dev" checks Robin Murphy
@ 2021-07-30  6:08   ` Lu Baolu
  0 siblings, 0 replies; 65+ messages in thread
From: Lu Baolu @ 2021-07-30  6:08 UTC (permalink / raw)
  To: Robin Murphy, joro, will
  Cc: baolu.lu, iommu, linux-arm-kernel, linux-kernel,
	suravee.suthikulpanit, john.garry, dianders

On 7/28/21 11:58 PM, Robin Murphy wrote:
> 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 <john.garry@huawei.com>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>   drivers/iommu/dma-iommu.c | 5 +----
>   1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 10067fbc4309..e28396cea6eb 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -363,7 +363,7 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>   
>   	init_iova_domain(iovad, 1UL << order, base_pfn);
>   
> -	if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) &&
> +	if (!cookie->fq_domain && !dev_is_untrusted(dev) &&
>   	    domain->ops->flush_iotlb_all && !iommu_get_dma_strict(domain)) {
>   		if (init_iova_flush_queue(iovad, iommu_dma_flush_iotlb_all,
>   					  iommu_dma_entry_dtor))
> @@ -372,9 +372,6 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>   			cookie->fq_domain = domain;
>   	}
>   
> -	if (!dev)
> -		return 0;
> -
>   	return iova_reserve_iommu_regions(dev, domain);
>   }
>   
> 


Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

Best regards,
baolu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 14/24] iommu: Introduce explicit type for non-strict DMA domains
  2021-07-28 15:58 ` [PATCH v2 14/24] iommu: Introduce explicit type for non-strict DMA domains Robin Murphy
@ 2021-07-30  6:08   ` Lu Baolu
  2021-07-30  9:23   ` Jean-Philippe Brucker
  1 sibling, 0 replies; 65+ messages in thread
From: Lu Baolu @ 2021-07-30  6:08 UTC (permalink / raw)
  To: Robin Murphy, joro, will
  Cc: baolu.lu, iommu, linux-arm-kernel, linux-kernel,
	suravee.suthikulpanit, john.garry, dianders

On 7/28/21 11:58 PM, Robin Murphy wrote:
> 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.
> 
> 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(-)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index e28396cea6eb..8b3545c01077 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -1311,7 +1311,7 @@ void iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 dma_limit)
>   	 * The IOMMU core code allocates the default DMA domain, which the
>   	 * underlying IOMMU driver needs to support via the dma-iommu layer.
>   	 */
> -	if (domain->type == IOMMU_DOMAIN_DMA) {
> +	if (iommu_is_dma_domain(domain)) {
>   		if (iommu_dma_init_domain(domain, dma_base, dma_limit, dev))
>   			goto out_err;
>   		dev->dma_ops = &iommu_dma_ops;
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index fa8109369f74..982545234cf3 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -115,6 +115,7 @@ static const char *iommu_domain_type_str(unsigned int t)
>   	case IOMMU_DOMAIN_UNMANAGED:
>   		return "Unmanaged";
>   	case IOMMU_DOMAIN_DMA:
> +	case IOMMU_DOMAIN_DMA_FQ:
>   		return "Translated";
>   	default:
>   		return "Unknown";
> @@ -552,6 +553,9 @@ static ssize_t iommu_group_show_type(struct iommu_group *group,
>   		case IOMMU_DOMAIN_DMA:
>   			type = "DMA\n";
>   			break;
> +		case IOMMU_DOMAIN_DMA_FQ:
> +			type = "DMA-FQ\n";
> +			break;
>   		}
>   	}
>   	mutex_unlock(&group->mutex);
> @@ -765,7 +769,7 @@ static int iommu_create_device_direct_mappings(struct iommu_group *group,
>   	unsigned long pg_size;
>   	int ret = 0;
>   
> -	if (!domain || domain->type != IOMMU_DOMAIN_DMA)
> +	if (!domain || !iommu_is_dma_domain(domain))
>   		return 0;
>   
>   	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;
>   	}
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 141779d76035..046ba4d54cd2 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -61,6 +61,7 @@ struct iommu_domain_geometry {
>   #define __IOMMU_DOMAIN_DMA_API	(1U << 1)  /* Domain for use in DMA-API
>   					      implementation              */
>   #define __IOMMU_DOMAIN_PT	(1U << 2)  /* Domain is identity mapped   */
> +#define __IOMMU_DOMAIN_DMA_FQ	(1U << 3)  /* DMA-API uses flush queue    */
>   
>   /*
>    * This are the possible domain-types
> @@ -73,12 +74,17 @@ struct iommu_domain_geometry {
>    *	IOMMU_DOMAIN_DMA	- Internally used for DMA-API implementations.
>    *				  This flag allows IOMMU drivers to implement
>    *				  certain optimizations for these domains
> + *	IOMMU_DOMAIN_DMA_FQ	- As above, but definitely using batched TLB
> + *				  invalidation.
>    */
>   #define IOMMU_DOMAIN_BLOCKED	(0U)
>   #define IOMMU_DOMAIN_IDENTITY	(__IOMMU_DOMAIN_PT)
>   #define IOMMU_DOMAIN_UNMANAGED	(__IOMMU_DOMAIN_PAGING)
>   #define IOMMU_DOMAIN_DMA	(__IOMMU_DOMAIN_PAGING |	\
>   				 __IOMMU_DOMAIN_DMA_API)
> +#define IOMMU_DOMAIN_DMA_FQ	(__IOMMU_DOMAIN_PAGING |	\
> +				 __IOMMU_DOMAIN_DMA_API |	\
> +				 __IOMMU_DOMAIN_DMA_FQ)
>   
>   struct iommu_domain {
>   	unsigned type;
> @@ -90,6 +96,11 @@ struct iommu_domain {
>   	struct iommu_dma_cookie *iova_cookie;
>   };
>   
> +static inline bool iommu_is_dma_domain(struct iommu_domain *domain)
> +{
> +	return domain->type & __IOMMU_DOMAIN_DMA_API;
> +}
> +
>   enum iommu_cap {
>   	IOMMU_CAP_CACHE_COHERENCY,	/* IOMMU can enforce cache coherent DMA
>   					   transactions */
> 

Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

Best regards,
baolu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 17/24] iommu/vt-d: Prepare for multiple DMA domain types
  2021-07-28 15:58 ` [PATCH v2 17/24] iommu/vt-d: " Robin Murphy
@ 2021-07-30  6:09   ` Lu Baolu
  0 siblings, 0 replies; 65+ messages in thread
From: Lu Baolu @ 2021-07-30  6:09 UTC (permalink / raw)
  To: Robin Murphy, joro, will
  Cc: baolu.lu, iommu, linux-arm-kernel, linux-kernel,
	suravee.suthikulpanit, john.garry, dianders

On 7/28/21 11:58 PM, Robin Murphy wrote:
> 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.
> 
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>   drivers/iommu/intel/iommu.c | 15 ++++++---------
>   1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 7e168634c433..8fc46c9d6b96 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -582,7 +582,7 @@ struct intel_iommu *domain_get_iommu(struct dmar_domain *domain)
>   	int iommu_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)))
>   		return NULL;
>   
>   	for_each_domain_iommu(iommu_id, domain)
> @@ -1034,7 +1034,7 @@ static struct dma_pte *pfn_to_dma_pte(struct dmar_domain *domain,
>   			pteval = ((uint64_t)virt_to_dma_pfn(tmp_page) << VTD_PAGE_SHIFT) | DMA_PTE_READ | DMA_PTE_WRITE;
>   			if (domain_use_first_level(domain)) {
>   				pteval |= DMA_FL_PTE_XD | DMA_FL_PTE_US;
> -				if (domain->domain.type == IOMMU_DOMAIN_DMA)
> +				if (iommu_is_dma_domain(&domain->domain))
>   					pteval |= DMA_FL_PTE_ACCESS;
>   			}
>   			if (cmpxchg64(&pte->val, 0ULL, pteval))
> @@ -2345,7 +2345,7 @@ __domain_mapping(struct dmar_domain *domain, unsigned long iov_pfn,
>   	if (domain_use_first_level(domain)) {
>   		attr |= DMA_FL_PTE_XD | DMA_FL_PTE_US;
>   
> -		if (domain->domain.type == IOMMU_DOMAIN_DMA) {
> +		if (iommu_is_dma_domain(&domain->domain)) {
>   			attr |= DMA_FL_PTE_ACCESS;
>   			if (prot & DMA_PTE_WRITE)
>   				attr |= DMA_FL_PTE_DIRTY;
> @@ -4528,6 +4528,7 @@ static struct iommu_domain *intel_iommu_domain_alloc(unsigned type)
>   
>   	switch (type) {
>   	case IOMMU_DOMAIN_DMA:
> +	case IOMMU_DOMAIN_DMA_FQ:
>   	case IOMMU_DOMAIN_UNMANAGED:
>   		dmar_domain = alloc_domain(0);
>   		if (!dmar_domain) {
> @@ -5197,12 +5198,8 @@ static void intel_iommu_release_device(struct device *dev)
>   
>   static void intel_iommu_probe_finalize(struct device *dev)
>   {
> -	struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
> -
> -	if (domain && 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);
>   }
>   
>   static void intel_iommu_get_resv_regions(struct device *device,
> 


Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

Best regards,
baolu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 18/24] iommu: Express DMA strictness via the domain type
  2021-07-29  9:36     ` Robin Murphy
  2021-07-29 12:42       ` Lu Baolu
@ 2021-07-30  6:09       ` Lu Baolu
  1 sibling, 0 replies; 65+ messages in thread
From: Lu Baolu @ 2021-07-30  6:09 UTC (permalink / raw)
  To: Robin Murphy, joro, will
  Cc: baolu.lu, iommu, linux-arm-kernel, linux-kernel,
	suravee.suthikulpanit, john.garry, dianders

On 7/29/21 5:36 PM, Robin Murphy wrote:
> On 2021-07-29 08:13, Lu Baolu wrote:
>> Hi Robin,
>>
>> On 7/28/21 11:58 PM, Robin Murphy wrote:
>>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>>> index 982545234cf3..eecb5657de69 100644
>>> --- a/drivers/iommu/iommu.c
>>> +++ b/drivers/iommu/iommu.c
>>> @@ -136,6 +136,9 @@ static int __init iommu_subsys_init(void)
>>>           }
>>>       }
>>> +    if (!iommu_default_passthrough() && !iommu_dma_strict)
>>> +        iommu_def_domain_type = IOMMU_DOMAIN_DMA_FQ;
>>
>> iommu_dma_strict could be changed later by the vendor iommu driver via
>> iommu_set_dma_strict(). This seems not to be the right place to set
>> iommu_def_domain_type.
> 
> Ah yes, good catch once again, thanks!
> 
> I think this *is* the right place to initially set it to honour the 
> command-line option, since that matches what we do for passthrough. 
> However also like passthrough we'll need to keep things in sync if it's 
> updated later, like this:
> 
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 87d7b299436e..593d4555bc57 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -359,6 +359,8 @@ early_param("iommu.strict", iommu_dma_setup);
>   void iommu_set_dma_strict(void)
>   {
>          iommu_dma_strict = true;
> +       if (iommu_def_domain_type == IOMMU_DOMAIN_DMA_FQ)
> +               iommu_def_domain_type = IOMMU_DOMAIN_DMA;
>   }
> 
>   static ssize_t iommu_group_attr_show(struct kobject *kobj,
> 
> 
> Does that seem reasonable? I'm not sure there's any cleaner way to do it 
> since we don't want to inadvertently clobber the default type if the 
> user has given us something funky like "intel_iommu=strict 
> iommu.passthrough=1".
> 
> Cheers,
> Robin.
> 
>>
>>> +
>>>       pr_info("Default domain type: %s %s\n",
>>>           iommu_domain_type_str(iommu_def_domain_type),
>>>           (iommu_cmd_line & IOMMU_CMD_LINE_DMA_API) ?
>>
>> Best regards,
>> baolu

With above fixed,

Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

Best regards,
baolu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 19/24] iommu: Expose DMA domain strictness via sysfs
  2021-07-28 15:58 ` [PATCH v2 19/24] iommu: Expose DMA domain strictness via sysfs Robin Murphy
@ 2021-07-30  6:10   ` Lu Baolu
  2021-07-30  9:28   ` Jean-Philippe Brucker
  2021-07-30 10:20   ` John Garry
  2 siblings, 0 replies; 65+ messages in thread
From: Lu Baolu @ 2021-07-30  6:10 UTC (permalink / raw)
  To: Robin Murphy, joro, will
  Cc: baolu.lu, iommu, linux-arm-kernel, linux-kernel,
	suravee.suthikulpanit, john.garry, dianders

On 7/28/21 11:58 PM, Robin Murphy wrote:
> 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.
> 
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>   Documentation/ABI/testing/sysfs-kernel-iommu_groups | 2 ++
>   drivers/iommu/iommu.c                               | 2 ++
>   2 files changed, 4 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-kernel-iommu_groups b/Documentation/ABI/testing/sysfs-kernel-iommu_groups
> index eae2f1c1e11e..43ba764ba5b7 100644
> --- a/Documentation/ABI/testing/sysfs-kernel-iommu_groups
> +++ b/Documentation/ABI/testing/sysfs-kernel-iommu_groups
> @@ -42,6 +42,8 @@ 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.
>   		identity  All the DMA transactions from the device in this group
>   		          are not translated by the iommu.
>   		auto      Change to the type the device was booted with.
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index eecb5657de69..5a08e0806cbb 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -3265,6 +3265,8 @@ static ssize_t iommu_group_store_type(struct iommu_group *group,
>   		req_type = IOMMU_DOMAIN_IDENTITY;
>   	else if (sysfs_streq(buf, "DMA"))
>   		req_type = IOMMU_DOMAIN_DMA;
> +	else if (sysfs_streq(buf, "DMA-FQ"))
> +		req_type = IOMMU_DOMAIN_DMA_FQ;
>   	else if (sysfs_streq(buf, "auto"))
>   		req_type = 0;
>   	else
> 


Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

Best regards,
baolu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 20/24] iommu: Merge strictness and domain type configs
  2021-07-28 15:58 ` [PATCH v2 20/24] iommu: Merge strictness and domain type configs Robin Murphy
@ 2021-07-30  6:10   ` Lu Baolu
  2021-07-30  9:29   ` Jean-Philippe Brucker
  2021-07-30  9:33   ` John Garry
  2 siblings, 0 replies; 65+ messages in thread
From: Lu Baolu @ 2021-07-30  6:10 UTC (permalink / raw)
  To: Robin Murphy, joro, will
  Cc: baolu.lu, iommu, linux-arm-kernel, linux-kernel,
	suravee.suthikulpanit, john.garry, dianders

On 7/28/21 11:58 PM, 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>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>   drivers/iommu/Kconfig | 80 +++++++++++++++++++++----------------------
>   drivers/iommu/iommu.c |  2 +-
>   2 files changed, 41 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index c84da8205be7..6e06f876d75a 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -79,55 +79,55 @@ config IOMMU_DEBUGFS
>   	  debug/iommu directory, and then populate a subdirectory with
>   	  entries as required.
>   
> -config IOMMU_DEFAULT_PASSTHROUGH
> -	bool "IOMMU passthrough by default"
> -	depends on IOMMU_API
> -	help
> -	  Enable passthrough by default, removing the need to pass in
> -	  iommu.passthrough=on or iommu=pt through command line. If this
> -	  is enabled, you can still disable with iommu.passthrough=off
> -	  or iommu=nopt depending on the architecture.
> -
> -	  If unsure, say N here.
> -
>   choice
> -	prompt "IOMMU default DMA IOTLB invalidation mode"
> -	depends on IOMMU_DMA
> -
> -	default IOMMU_DEFAULT_LAZY if (AMD_IOMMU || INTEL_IOMMU)
> -	default IOMMU_DEFAULT_STRICT
> +	prompt "IOMMU default domain type"
> +	depends on IOMMU_API
> +	default IOMMU_DEFAULT_DMA_LAZY if AMD_IOMMU || INTEL_IOMMU
> +	default IOMMU_DEFAULT_DMA_STRICT
>   	help
> -	  This option allows an IOMMU DMA IOTLB invalidation mode to be
> -	  chosen at build time, to override the default mode of each ARCH,
> -	  removing the need to pass in kernel parameters through command line.
> -	  It is still possible to provide common boot params to override this
> -	  config.
> +	  Choose the type of IOMMU domain used to manage DMA API usage by
> +	  device drivers. The options here typically represent different
> +	  levels of tradeoff between robustness/security and performance,
> +	  depending on the IOMMU driver. Not all IOMMUs support all options.
> +	  This choice can be overridden at boot via the command line, and for
> +	  some devices also at runtime via sysfs.
>   
>   	  If unsure, keep the default.
>   
> -config IOMMU_DEFAULT_STRICT
> -	bool "strict"
> +config IOMMU_DEFAULT_DMA_STRICT
> +	bool "Translated - Strict"
>   	help
> -	  For every IOMMU DMA unmap operation, the flush operation of IOTLB and
> -	  the free operation of IOVA are guaranteed to be done in the unmap
> -	  function.
> +	  Trusted devices use translation to restrict their access to only
> +	  DMA-mapped pages, with strict TLB invalidation on unmap. Equivalent
> +	  to passing "iommu.passthrough=0 iommu.strict=1" on the command line.
>   
> -config IOMMU_DEFAULT_LAZY
> -	bool "lazy"
> +	  Untrusted devices always use this mode, with an additional layer of
> +	  bounce-buffering such that they cannot gain access to any unrelated
> +	  data within a mapped page.
> +
> +config IOMMU_DEFAULT_DMA_LAZY
> +	bool "Translated - Lazy"
>   	help
> -	  Support lazy mode, where for every IOMMU DMA unmap operation, the
> -	  flush operation of IOTLB and the free operation of IOVA are deferred.
> -	  They are only guaranteed to be done before the related IOVA will be
> -	  reused.
> +	  Trusted devices use translation to restrict their access to only
> +	  DMA-mapped pages, but with "lazy" batched TLB invalidation. This
> +	  mode allows higher performance with some IOMMUs due to reduced TLB
> +	  flushing, but at the cost of reduced isolation since devices may be
> +	  able to access memory for some time after it has been unmapped.
> +	  Equivalent to passing "iommu.passthrough=0 iommu.strict=0" on the
> +	  command line.
>   
> -	  The isolation provided in this mode is not as secure as STRICT mode,
> -	  such that a vulnerable time window may be created between the DMA
> -	  unmap and the mappings cached in the IOMMU IOTLB or device TLB
> -	  finally being invalidated, where the device could still access the
> -	  memory which has already been unmapped by the device driver.
> -	  However this mode may provide better performance in high throughput
> -	  scenarios, and is still considerably more secure than passthrough
> -	  mode or no IOMMU.
> +	  If this mode is not supported by the IOMMU driver, the effective
> +	  runtime default will fall back to IOMMU_DEFAULT_DMA_STRICT.
> +
> +config IOMMU_DEFAULT_PASSTHROUGH
> +	bool "Passthrough"
> +	help
> +	  Trusted devices are identity-mapped, giving them unrestricted access
> +	  to memory with minimal performance overhead. Equivalent to passing
> +	  "iommu.passthrough=1" (historically "iommu=pt") on the command line.
> +
> +	  If this mode is not supported by the IOMMU driver, the effective
> +	  runtime default will fall back to IOMMU_DEFAULT_DMA_STRICT.
>   
>   endchoice
>   
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 5a08e0806cbb..25c1adc1ec67 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -31,7 +31,7 @@ static struct kset *iommu_group_kset;
>   static DEFINE_IDA(iommu_group_ida);
>   
>   static unsigned int iommu_def_domain_type __read_mostly;
> -static bool iommu_dma_strict __read_mostly = IS_ENABLED(CONFIG_IOMMU_DEFAULT_STRICT);
> +static bool iommu_dma_strict __read_mostly = IS_ENABLED(CONFIG_IOMMU_DEFAULT_DMA_STRICT);
>   static u32 iommu_cmd_line __read_mostly;
>   
>   struct iommu_group {
> 


Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

Best regards,
baolu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 21/24] iommu/dma: Factor out flush queue init
  2021-07-28 15:58 ` [PATCH v2 21/24] iommu/dma: Factor out flush queue init Robin Murphy
@ 2021-07-30  6:11   ` Lu Baolu
  2021-07-30  9:20   ` John Garry
  1 sibling, 0 replies; 65+ messages in thread
From: Lu Baolu @ 2021-07-30  6:11 UTC (permalink / raw)
  To: Robin Murphy, joro, will
  Cc: baolu.lu, iommu, linux-arm-kernel, linux-kernel,
	suravee.suthikulpanit, john.garry, dianders

On 7/28/21 11:58 PM, Robin Murphy wrote:
> 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.
> 
> 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(-)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 7f3968865387..304a3ec71223 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -310,6 +310,25 @@ static bool dev_is_untrusted(struct device *dev)
>   	return dev_is_pci(dev) && to_pci_dev(dev)->untrusted;
>   }
>   
> +int iommu_dma_init_fq(struct iommu_domain *domain)
> +{
> +	struct iommu_dma_cookie *cookie = domain->iova_cookie;
> +
> +	if (domain->type != IOMMU_DOMAIN_DMA_FQ)
> +		return -EINVAL;
> +	if (cookie->fq_domain)
> +		return 0;
> +
> +	if (init_iova_flush_queue(&cookie->iovad, iommu_dma_flush_iotlb_all,
> +				  iommu_dma_entry_dtor)) {
> +		pr_warn("iova flush queue initialization failed\n");
> +		domain->type = IOMMU_DOMAIN_DMA;
> +		return -ENODEV;
> +	}
> +	cookie->fq_domain = domain;
> +	return 0;
> +}
> +
>   /**
>    * iommu_dma_init_domain - Initialise a DMA mapping domain
>    * @domain: IOMMU domain previously prepared by iommu_get_dma_cookie()
> @@ -362,16 +381,7 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>   	}
>   
>   	init_iova_domain(iovad, 1UL << order, base_pfn);
> -
> -	if (domain->type == IOMMU_DOMAIN_DMA_FQ && !cookie->fq_domain) {
> -		if (init_iova_flush_queue(iovad, iommu_dma_flush_iotlb_all,
> -					  iommu_dma_entry_dtor)) {
> -			pr_warn("iova flush queue initialization failed\n");
> -			domain->type = IOMMU_DOMAIN_DMA;
> -		} else {
> -			cookie->fq_domain = domain;
> -		}
> -	}
> +	iommu_dma_init_fq(domain);
>   
>   	return iova_reserve_iommu_regions(dev, domain);
>   }
> diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
> index 758ca4694257..81ab647f1618 100644
> --- a/include/linux/dma-iommu.h
> +++ b/include/linux/dma-iommu.h
> @@ -20,6 +20,7 @@ void iommu_put_dma_cookie(struct iommu_domain *domain);
>   
>   /* Setup call for arch DMA mapping code */
>   void iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 dma_limit);
> +int iommu_dma_init_fq(struct iommu_domain *domain);
>   
>   /* The DMA API isn't _quite_ the whole story, though... */
>   /*
> @@ -37,9 +38,6 @@ void iommu_dma_compose_msi_msg(struct msi_desc *desc,
>   
>   void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list);
>   
> -void iommu_dma_free_cpu_cached_iovas(unsigned int cpu,
> -		struct iommu_domain *domain);
> -
>   extern bool iommu_dma_forcedac;
>   
>   #else /* CONFIG_IOMMU_DMA */
> @@ -54,6 +52,11 @@ static inline void iommu_setup_dma_ops(struct device *dev, u64 dma_base,
>   {
>   }
>   
> +static inline int iommu_dma_init_fq(struct iommu_domain *domain)
> +{
> +	return -EINVAL;
> +}
> +
>   static inline int iommu_get_dma_cookie(struct iommu_domain *domain)
>   {
>   	return -ENODEV;
> 


Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

Best regards,
baolu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 22/24] iommu: Allow enabling non-strict mode dynamically
  2021-07-28 15:58 ` [PATCH v2 22/24] iommu: Allow enabling non-strict mode dynamically Robin Murphy
@ 2021-07-30  6:11   ` Lu Baolu
  2021-07-30  9:24   ` John Garry
  1 sibling, 0 replies; 65+ messages in thread
From: Lu Baolu @ 2021-07-30  6:11 UTC (permalink / raw)
  To: Robin Murphy, joro, will
  Cc: baolu.lu, iommu, linux-arm-kernel, linux-kernel,
	suravee.suthikulpanit, john.garry, dianders

On 7/28/21 11:58 PM, 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.
> 
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>   drivers/iommu/iommu.c | 16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 25c1adc1ec67..be399d630953 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -3200,6 +3200,13 @@ static int iommu_change_dev_def_domain(struct iommu_group *group,
>   		goto out;
>   	}
>   
> +	/* 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);
> +		goto out;
> +	}
> +
>   	/* Sets group->default_domain to the newly allocated domain */
>   	ret = iommu_group_alloc_default_domain(dev->bus, group, type);
>   	if (ret)
> @@ -3240,9 +3247,9 @@ static int iommu_change_dev_def_domain(struct iommu_group *group,
>   }
>   
>   /*
> - * Changing the default domain through sysfs requires the users to ubind the
> - * drivers from the devices in the iommu group. Return failure if this doesn't
> - * meet.
> + * Changing the default domain through sysfs requires the users to unbind the
> + * drivers from the devices in the iommu group, except for a DMA -> DMA-FQ
> + * transition. Return failure if this isn't met.
>    *
>    * We need to consider the race between this and the device release path.
>    * device_lock(dev) is used here to guarantee that the device release path
> @@ -3318,7 +3325,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;
>   		goto out;
> 


Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

Best regards,
baolu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 24/24] iommu: Only log strictness for DMA domains
  2021-07-28 15:58 ` [PATCH v2 24/24] iommu: Only log strictness for DMA domains Robin Murphy
  2021-07-29  9:04   ` John Garry
@ 2021-07-30  6:12   ` Lu Baolu
  1 sibling, 0 replies; 65+ messages in thread
From: Lu Baolu @ 2021-07-30  6:12 UTC (permalink / raw)
  To: Robin Murphy, joro, will
  Cc: baolu.lu, iommu, linux-arm-kernel, linux-kernel,
	suravee.suthikulpanit, john.garry, dianders

On 7/28/21 11:58 PM, Robin Murphy wrote:
> 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.
> 
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>   drivers/iommu/iommu.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index be399d630953..87d7b299436e 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -144,10 +144,11 @@ static int __init iommu_subsys_init(void)
>   		(iommu_cmd_line & IOMMU_CMD_LINE_DMA_API) ?
>   			"(set via kernel command line)" : "");
>   
> -	pr_info("DMA domain TLB invalidation policy: %s mode %s\n",
> -		iommu_dma_strict ? "strict" : "lazy",
> -		(iommu_cmd_line & IOMMU_CMD_LINE_STRICT) ?
> -			"(set via kernel command line)" : "");
> +	if (!iommu_default_passthrough())
> +		pr_info("DMA domain TLB invalidation policy: %s mode %s\n",
> +			iommu_dma_strict ? "strict" : "lazy",
> +			(iommu_cmd_line & IOMMU_CMD_LINE_STRICT) ?
> +				"(set via kernel command line)" : "");
>   
>   	return 0;
>   }
> 

Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

Best regards,
baolu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 11/24] iommu/virtio: Drop IOVA cookie management
  2021-07-28 15:58 ` [PATCH v2 11/24] iommu/virtio: " Robin Murphy
@ 2021-07-30  9:20   ` Jean-Philippe Brucker
  0 siblings, 0 replies; 65+ messages in thread
From: Jean-Philippe Brucker @ 2021-07-30  9:20 UTC (permalink / raw)
  To: Robin Murphy; +Cc: joro, will, linux-kernel, dianders, iommu, linux-arm-kernel

On Wed, Jul 28, 2021 at 04:58:32PM +0100, Robin Murphy wrote:
> The core code bakes its own cookies now.
> 
> CC: Jean-Philippe Brucker <jean-philippe@linaro.org>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>

Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

Also tested on the Seattle (vsmmuv3/viommu with lazy/strict, still need to
test the runtime switch).

> ---
>  drivers/iommu/virtio-iommu.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> index 6abdcab7273b..80930ce04a16 100644
> --- a/drivers/iommu/virtio-iommu.c
> +++ b/drivers/iommu/virtio-iommu.c
> @@ -598,12 +598,6 @@ static struct iommu_domain *viommu_domain_alloc(unsigned type)
>  	spin_lock_init(&vdomain->mappings_lock);
>  	vdomain->mappings = RB_ROOT_CACHED;
>  
> -	if (type == IOMMU_DOMAIN_DMA &&
> -	    iommu_get_dma_cookie(&vdomain->domain)) {
> -		kfree(vdomain);
> -		return NULL;
> -	}
> -
>  	return &vdomain->domain;
>  }
>  
> @@ -643,8 +637,6 @@ static void viommu_domain_free(struct iommu_domain *domain)
>  {
>  	struct viommu_domain *vdomain = to_viommu_domain(domain);
>  
> -	iommu_put_dma_cookie(domain);
> -
>  	/* Free all remaining mappings (size 2^64) */
>  	viommu_del_mappings(vdomain, 0, 0);
>  
> -- 
> 2.25.1
> 
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 21/24] iommu/dma: Factor out flush queue init
  2021-07-28 15:58 ` [PATCH v2 21/24] iommu/dma: Factor out flush queue init Robin Murphy
  2021-07-30  6:11   ` Lu Baolu
@ 2021-07-30  9:20   ` John Garry
  1 sibling, 0 replies; 65+ messages in thread
From: John Garry @ 2021-07-30  9:20 UTC (permalink / raw)
  To: Robin Murphy, joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, dianders

On 28/07/2021 16:58, Robin Murphy wrote:
> 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.
> 
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>

Reviewed-by: John Garry <john.garry@huawei.com>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 12/24] iommu/dma: Unexport IOVA cookie management
  2021-07-28 15:58 ` [PATCH v2 12/24] iommu/dma: Unexport " Robin Murphy
  2021-07-30  6:07   ` Lu Baolu
@ 2021-07-30  9:21   ` Jean-Philippe Brucker
  1 sibling, 0 replies; 65+ messages in thread
From: Jean-Philippe Brucker @ 2021-07-30  9:21 UTC (permalink / raw)
  To: Robin Murphy; +Cc: joro, will, linux-kernel, dianders, iommu, linux-arm-kernel

On Wed, Jul 28, 2021 at 04:58:33PM +0100, Robin Murphy wrote:
> 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.
> 
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>

Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 14/24] iommu: Introduce explicit type for non-strict DMA domains
  2021-07-28 15:58 ` [PATCH v2 14/24] iommu: Introduce explicit type for non-strict DMA domains Robin Murphy
  2021-07-30  6:08   ` Lu Baolu
@ 2021-07-30  9:23   ` Jean-Philippe Brucker
  1 sibling, 0 replies; 65+ messages in thread
From: Jean-Philippe Brucker @ 2021-07-30  9:23 UTC (permalink / raw)
  To: Robin Murphy; +Cc: joro, will, linux-kernel, dianders, iommu, linux-arm-kernel

On Wed, Jul 28, 2021 at 04:58:35PM +0100, Robin Murphy wrote:
> 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.
> 
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>

Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

> ---
>  drivers/iommu/dma-iommu.c |  2 +-
>  drivers/iommu/iommu.c     |  8 ++++++--
>  include/linux/iommu.h     | 11 +++++++++++
>  3 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index e28396cea6eb..8b3545c01077 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -1311,7 +1311,7 @@ void iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 dma_limit)
>  	 * The IOMMU core code allocates the default DMA domain, which the
>  	 * underlying IOMMU driver needs to support via the dma-iommu layer.
>  	 */
> -	if (domain->type == IOMMU_DOMAIN_DMA) {
> +	if (iommu_is_dma_domain(domain)) {
>  		if (iommu_dma_init_domain(domain, dma_base, dma_limit, dev))
>  			goto out_err;
>  		dev->dma_ops = &iommu_dma_ops;
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index fa8109369f74..982545234cf3 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -115,6 +115,7 @@ static const char *iommu_domain_type_str(unsigned int t)
>  	case IOMMU_DOMAIN_UNMANAGED:
>  		return "Unmanaged";
>  	case IOMMU_DOMAIN_DMA:
> +	case IOMMU_DOMAIN_DMA_FQ:
>  		return "Translated";
>  	default:
>  		return "Unknown";
> @@ -552,6 +553,9 @@ static ssize_t iommu_group_show_type(struct iommu_group *group,
>  		case IOMMU_DOMAIN_DMA:
>  			type = "DMA\n";
>  			break;
> +		case IOMMU_DOMAIN_DMA_FQ:
> +			type = "DMA-FQ\n";
> +			break;
>  		}
>  	}
>  	mutex_unlock(&group->mutex);
> @@ -765,7 +769,7 @@ static int iommu_create_device_direct_mappings(struct iommu_group *group,
>  	unsigned long pg_size;
>  	int ret = 0;
>  
> -	if (!domain || domain->type != IOMMU_DOMAIN_DMA)
> +	if (!domain || !iommu_is_dma_domain(domain))
>  		return 0;
>  
>  	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;
>  	}
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 141779d76035..046ba4d54cd2 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -61,6 +61,7 @@ struct iommu_domain_geometry {
>  #define __IOMMU_DOMAIN_DMA_API	(1U << 1)  /* Domain for use in DMA-API
>  					      implementation              */
>  #define __IOMMU_DOMAIN_PT	(1U << 2)  /* Domain is identity mapped   */
> +#define __IOMMU_DOMAIN_DMA_FQ	(1U << 3)  /* DMA-API uses flush queue    */
>  
>  /*
>   * This are the possible domain-types
> @@ -73,12 +74,17 @@ struct iommu_domain_geometry {
>   *	IOMMU_DOMAIN_DMA	- Internally used for DMA-API implementations.
>   *				  This flag allows IOMMU drivers to implement
>   *				  certain optimizations for these domains
> + *	IOMMU_DOMAIN_DMA_FQ	- As above, but definitely using batched TLB
> + *				  invalidation.
>   */
>  #define IOMMU_DOMAIN_BLOCKED	(0U)
>  #define IOMMU_DOMAIN_IDENTITY	(__IOMMU_DOMAIN_PT)
>  #define IOMMU_DOMAIN_UNMANAGED	(__IOMMU_DOMAIN_PAGING)
>  #define IOMMU_DOMAIN_DMA	(__IOMMU_DOMAIN_PAGING |	\
>  				 __IOMMU_DOMAIN_DMA_API)
> +#define IOMMU_DOMAIN_DMA_FQ	(__IOMMU_DOMAIN_PAGING |	\
> +				 __IOMMU_DOMAIN_DMA_API |	\
> +				 __IOMMU_DOMAIN_DMA_FQ)
>  
>  struct iommu_domain {
>  	unsigned type;
> @@ -90,6 +96,11 @@ struct iommu_domain {
>  	struct iommu_dma_cookie *iova_cookie;
>  };
>  
> +static inline bool iommu_is_dma_domain(struct iommu_domain *domain)
> +{
> +	return domain->type & __IOMMU_DOMAIN_DMA_API;
> +}
> +
>  enum iommu_cap {
>  	IOMMU_CAP_CACHE_COHERENCY,	/* IOMMU can enforce cache coherent DMA
>  					   transactions */
> -- 
> 2.25.1
> 
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 22/24] iommu: Allow enabling non-strict mode dynamically
  2021-07-28 15:58 ` [PATCH v2 22/24] iommu: Allow enabling non-strict mode dynamically Robin Murphy
  2021-07-30  6:11   ` Lu Baolu
@ 2021-07-30  9:24   ` John Garry
  1 sibling, 0 replies; 65+ messages in thread
From: John Garry @ 2021-07-30  9:24 UTC (permalink / raw)
  To: Robin Murphy, joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, dianders

On 28/07/2021 16:58, 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.
> 
> Signed-off-by: Robin Murphy<robin.murphy@arm.com>

Reviewed-by: John Garry <john.garry@huawei.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 19/24] iommu: Expose DMA domain strictness via sysfs
  2021-07-28 15:58 ` [PATCH v2 19/24] iommu: Expose DMA domain strictness via sysfs Robin Murphy
  2021-07-30  6:10   ` Lu Baolu
@ 2021-07-30  9:28   ` Jean-Philippe Brucker
  2021-07-30 10:20   ` John Garry
  2 siblings, 0 replies; 65+ messages in thread
From: Jean-Philippe Brucker @ 2021-07-30  9:28 UTC (permalink / raw)
  To: Robin Murphy; +Cc: joro, will, linux-kernel, dianders, iommu, linux-arm-kernel

On Wed, Jul 28, 2021 at 04:58:40PM +0100, Robin Murphy wrote:
> 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.
> 
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>  Documentation/ABI/testing/sysfs-kernel-iommu_groups | 2 ++
>  drivers/iommu/iommu.c                               | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-kernel-iommu_groups b/Documentation/ABI/testing/sysfs-kernel-iommu_groups
> index eae2f1c1e11e..43ba764ba5b7 100644
> --- a/Documentation/ABI/testing/sysfs-kernel-iommu_groups
> +++ b/Documentation/ABI/testing/sysfs-kernel-iommu_groups
> @@ -42,6 +42,8 @@ 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.

It might be useful to desribe the security/performance tradeoff here as
well. I guess a normal user is more likely to look at the doc for this
sysfs knob than the kernel config or parameters.

Thanks,
Jean

>  		identity  All the DMA transactions from the device in this group
>  		          are not translated by the iommu.
>  		auto      Change to the type the device was booted with.
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index eecb5657de69..5a08e0806cbb 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -3265,6 +3265,8 @@ static ssize_t iommu_group_store_type(struct iommu_group *group,
>  		req_type = IOMMU_DOMAIN_IDENTITY;
>  	else if (sysfs_streq(buf, "DMA"))
>  		req_type = IOMMU_DOMAIN_DMA;
> +	else if (sysfs_streq(buf, "DMA-FQ"))
> +		req_type = IOMMU_DOMAIN_DMA_FQ;
>  	else if (sysfs_streq(buf, "auto"))
>  		req_type = 0;
>  	else
> -- 
> 2.25.1
> 
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 20/24] iommu: Merge strictness and domain type configs
  2021-07-28 15:58 ` [PATCH v2 20/24] iommu: Merge strictness and domain type configs Robin Murphy
  2021-07-30  6:10   ` Lu Baolu
@ 2021-07-30  9:29   ` Jean-Philippe Brucker
  2021-07-30  9:33   ` John Garry
  2 siblings, 0 replies; 65+ messages in thread
From: Jean-Philippe Brucker @ 2021-07-30  9:29 UTC (permalink / raw)
  To: Robin Murphy; +Cc: joro, will, linux-kernel, dianders, iommu, linux-arm-kernel

On Wed, Jul 28, 2021 at 04:58:41PM +0100, 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>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>

kernel-parameters.txt still refers to the old config options

Otherwise
Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

> ---
>  drivers/iommu/Kconfig | 80 +++++++++++++++++++++----------------------
>  drivers/iommu/iommu.c |  2 +-
>  2 files changed, 41 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index c84da8205be7..6e06f876d75a 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -79,55 +79,55 @@ config IOMMU_DEBUGFS
>  	  debug/iommu directory, and then populate a subdirectory with
>  	  entries as required.
>  
> -config IOMMU_DEFAULT_PASSTHROUGH
> -	bool "IOMMU passthrough by default"
> -	depends on IOMMU_API
> -	help
> -	  Enable passthrough by default, removing the need to pass in
> -	  iommu.passthrough=on or iommu=pt through command line. If this
> -	  is enabled, you can still disable with iommu.passthrough=off
> -	  or iommu=nopt depending on the architecture.
> -
> -	  If unsure, say N here.
> -
>  choice
> -	prompt "IOMMU default DMA IOTLB invalidation mode"
> -	depends on IOMMU_DMA
> -
> -	default IOMMU_DEFAULT_LAZY if (AMD_IOMMU || INTEL_IOMMU)
> -	default IOMMU_DEFAULT_STRICT
> +	prompt "IOMMU default domain type"
> +	depends on IOMMU_API
> +	default IOMMU_DEFAULT_DMA_LAZY if AMD_IOMMU || INTEL_IOMMU
> +	default IOMMU_DEFAULT_DMA_STRICT
>  	help
> -	  This option allows an IOMMU DMA IOTLB invalidation mode to be
> -	  chosen at build time, to override the default mode of each ARCH,
> -	  removing the need to pass in kernel parameters through command line.
> -	  It is still possible to provide common boot params to override this
> -	  config.
> +	  Choose the type of IOMMU domain used to manage DMA API usage by
> +	  device drivers. The options here typically represent different
> +	  levels of tradeoff between robustness/security and performance,
> +	  depending on the IOMMU driver. Not all IOMMUs support all options.
> +	  This choice can be overridden at boot via the command line, and for
> +	  some devices also at runtime via sysfs.
>  
>  	  If unsure, keep the default.
>  
> -config IOMMU_DEFAULT_STRICT
> -	bool "strict"
> +config IOMMU_DEFAULT_DMA_STRICT
> +	bool "Translated - Strict"
>  	help
> -	  For every IOMMU DMA unmap operation, the flush operation of IOTLB and
> -	  the free operation of IOVA are guaranteed to be done in the unmap
> -	  function.
> +	  Trusted devices use translation to restrict their access to only
> +	  DMA-mapped pages, with strict TLB invalidation on unmap. Equivalent
> +	  to passing "iommu.passthrough=0 iommu.strict=1" on the command line.
>  
> -config IOMMU_DEFAULT_LAZY
> -	bool "lazy"
> +	  Untrusted devices always use this mode, with an additional layer of
> +	  bounce-buffering such that they cannot gain access to any unrelated
> +	  data within a mapped page.
> +
> +config IOMMU_DEFAULT_DMA_LAZY
> +	bool "Translated - Lazy"
>  	help
> -	  Support lazy mode, where for every IOMMU DMA unmap operation, the
> -	  flush operation of IOTLB and the free operation of IOVA are deferred.
> -	  They are only guaranteed to be done before the related IOVA will be
> -	  reused.
> +	  Trusted devices use translation to restrict their access to only
> +	  DMA-mapped pages, but with "lazy" batched TLB invalidation. This
> +	  mode allows higher performance with some IOMMUs due to reduced TLB
> +	  flushing, but at the cost of reduced isolation since devices may be
> +	  able to access memory for some time after it has been unmapped.
> +	  Equivalent to passing "iommu.passthrough=0 iommu.strict=0" on the
> +	  command line.
>  
> -	  The isolation provided in this mode is not as secure as STRICT mode,
> -	  such that a vulnerable time window may be created between the DMA
> -	  unmap and the mappings cached in the IOMMU IOTLB or device TLB
> -	  finally being invalidated, where the device could still access the
> -	  memory which has already been unmapped by the device driver.
> -	  However this mode may provide better performance in high throughput
> -	  scenarios, and is still considerably more secure than passthrough
> -	  mode or no IOMMU.
> +	  If this mode is not supported by the IOMMU driver, the effective
> +	  runtime default will fall back to IOMMU_DEFAULT_DMA_STRICT.
> +
> +config IOMMU_DEFAULT_PASSTHROUGH
> +	bool "Passthrough"
> +	help
> +	  Trusted devices are identity-mapped, giving them unrestricted access
> +	  to memory with minimal performance overhead. Equivalent to passing
> +	  "iommu.passthrough=1" (historically "iommu=pt") on the command line.
> +
> +	  If this mode is not supported by the IOMMU driver, the effective
> +	  runtime default will fall back to IOMMU_DEFAULT_DMA_STRICT.
>  
>  endchoice
>  
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 5a08e0806cbb..25c1adc1ec67 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -31,7 +31,7 @@ static struct kset *iommu_group_kset;
>  static DEFINE_IDA(iommu_group_ida);
>  
>  static unsigned int iommu_def_domain_type __read_mostly;
> -static bool iommu_dma_strict __read_mostly = IS_ENABLED(CONFIG_IOMMU_DEFAULT_STRICT);
> +static bool iommu_dma_strict __read_mostly = IS_ENABLED(CONFIG_IOMMU_DEFAULT_DMA_STRICT);
>  static u32 iommu_cmd_line __read_mostly;
>  
>  struct iommu_group {
> -- 
> 2.25.1
> 
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 01/24] iommu: Pull IOVA cookie management into the core
  2021-07-28 15:58 ` [PATCH v2 01/24] iommu: Pull IOVA cookie management into the core Robin Murphy
  2021-07-30  6:06   ` Lu Baolu
@ 2021-07-30  9:32   ` Jean-Philippe Brucker
  1 sibling, 0 replies; 65+ messages in thread
From: Jean-Philippe Brucker @ 2021-07-30  9:32 UTC (permalink / raw)
  To: Robin Murphy
  Cc: joro, will, Maxime Ripard, Heiko Stuebner, Geert Uytterhoeven,
	linux-kernel, Chunyan Zhang, dianders, iommu, linux-arm-kernel

On Wed, Jul 28, 2021 at 04:58:22PM +0100, 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 <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>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>

Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

> ---
>  drivers/iommu/iommu.c | 7 +++++++
>  include/linux/iommu.h | 3 ++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index f2cda9950bd5..ea5a9ea8d431 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -7,6 +7,7 @@
>  #define pr_fmt(fmt)    "iommu: " fmt
>  
>  #include <linux/device.h>
> +#include <linux/dma-iommu.h>
>  #include <linux/kernel.h>
>  #include <linux/bits.h>
>  #include <linux/bug.h>
> @@ -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 ignore -EEXIST while drivers still get their own cookies */
> +	if (type == IOMMU_DOMAIN_DMA && iommu_get_dma_cookie(domain) == -ENOMEM) {
> +		iommu_domain_free(domain);
> +		domain = NULL;
> +	}
>  	return domain;
>  }
>  
> @@ -1957,6 +1963,7 @@ EXPORT_SYMBOL_GPL(iommu_domain_alloc);
>  
>  void iommu_domain_free(struct iommu_domain *domain)
>  {
> +	iommu_put_dma_cookie(domain);
>  	domain->ops->domain_free(domain);
>  }
>  EXPORT_SYMBOL_GPL(iommu_domain_free);
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 4997c78e2670..141779d76035 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -40,6 +40,7 @@ struct iommu_domain;
>  struct notifier_block;
>  struct iommu_sva;
>  struct iommu_fault_event;
> +struct iommu_dma_cookie;
>  
>  /* iommu fault flags */
>  #define IOMMU_FAULT_READ	0x0
> @@ -86,7 +87,7 @@ struct iommu_domain {
>  	iommu_fault_handler_t handler;
>  	void *handler_token;
>  	struct iommu_domain_geometry geometry;
> -	void *iova_cookie;
> +	struct iommu_dma_cookie *iova_cookie;
>  };
>  
>  enum iommu_cap {
> -- 
> 2.25.1
> 
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 20/24] iommu: Merge strictness and domain type configs
  2021-07-28 15:58 ` [PATCH v2 20/24] iommu: Merge strictness and domain type configs Robin Murphy
  2021-07-30  6:10   ` Lu Baolu
  2021-07-30  9:29   ` Jean-Philippe Brucker
@ 2021-07-30  9:33   ` John Garry
  2 siblings, 0 replies; 65+ messages in thread
From: John Garry @ 2021-07-30  9:33 UTC (permalink / raw)
  To: Robin Murphy, joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, dianders

On 28/07/2021 16:58, Robin Murphy wrote:
>   
> -config IOMMU_DEFAULT_LAZY
> -	bool "lazy"
> +	  Untrusted devices always use this mode, with an additional layer of
> +	  bounce-buffering such that they cannot gain access to any unrelated
> +	  data within a mapped page.
> +
> +config IOMMU_DEFAULT_DMA_LAZY
> +	bool "Translated - Lazy"
>   	help

Since these are being renamed, can you update the kernel-parameters.txt:


	iommu.strict=	[ARM64, X86] Configure TLB invalidation behaviour


....
		  DMA unmap operations invalidate IOMMU hardware TLBs
		  synchronously.
		unset - Use value of CONFIG_IOMMU_DEFAULT_{LAZY,STRICT}.

					^^^^^

Thanks

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 19/24] iommu: Expose DMA domain strictness via sysfs
  2021-07-28 15:58 ` [PATCH v2 19/24] iommu: Expose DMA domain strictness via sysfs Robin Murphy
  2021-07-30  6:10   ` Lu Baolu
  2021-07-30  9:28   ` Jean-Philippe Brucker
@ 2021-07-30 10:20   ` John Garry
  2 siblings, 0 replies; 65+ messages in thread
From: John Garry @ 2021-07-30 10:20 UTC (permalink / raw)
  To: Robin Murphy, joro, will
  Cc: iommu, linux-arm-kernel, linux-kernel, suravee.suthikulpanit,
	baolu.lu, dianders

On 28/07/2021 16:58, Robin Murphy wrote:
> 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.
> 
> Signed-off-by: Robin Murphy<robin.murphy@arm.com>
> ---

Reviewed-by: John Garry <john.garry@huawei.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 23/24] iommu/arm-smmu: Allow non-strict in pgtable_quirks interface
  2021-07-28 15:58 ` [PATCH v2 23/24] iommu/arm-smmu: Allow non-strict in pgtable_quirks interface Robin Murphy
@ 2021-08-02 13:04   ` Will Deacon
  2021-08-02 14:15     ` Robin Murphy
  0 siblings, 1 reply; 65+ messages in thread
From: Will Deacon @ 2021-08-02 13:04 UTC (permalink / raw)
  To: Robin Murphy
  Cc: joro, iommu, linux-arm-kernel, linux-kernel,
	suravee.suthikulpanit, baolu.lu, john.garry, dianders

On Wed, Jul 28, 2021 at 04:58:44PM +0100, Robin Murphy wrote:
> To make io-pgtable aware of a flush queue being dynamically enabled,
> allow IO_PGTABLE_QUIRK_NON_STRICT to be set even after a domain has been
> attached to, and hook up the final piece of the puzzle in iommu-dma.
> 
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 15 +++++++++++++++
>  drivers/iommu/arm/arm-smmu/arm-smmu.c       | 11 +++++++++++
>  drivers/iommu/dma-iommu.c                   |  3 +++
>  3 files changed, 29 insertions(+)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 19400826eba7..40fa9cb382c3 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2711,6 +2711,20 @@ static int arm_smmu_enable_nesting(struct iommu_domain *domain)
>  	return ret;
>  }
>  
> +static int arm_smmu_set_pgtable_quirks(struct iommu_domain *domain,
> +		unsigned long quirks)
> +{
> +	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> +
> +	if (quirks == IO_PGTABLE_QUIRK_NON_STRICT && smmu_domain->pgtbl_ops) {
> +		struct io_pgtable *iop = io_pgtable_ops_to_pgtable(smmu_domain->pgtbl_ops);
> +
> +		iop->cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
> +		return 0;
> +	}
> +	return -EINVAL;
> +}

I don't see anything serialising this against a concurrent iommu_unmap(), so
the ordering and atomicity looks quite suspicious to me here. I don't think
it's just the page-table quirks either, but also setting cookie->fq_domain.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 23/24] iommu/arm-smmu: Allow non-strict in pgtable_quirks interface
  2021-08-02 13:04   ` Will Deacon
@ 2021-08-02 14:15     ` Robin Murphy
  2021-08-03 10:36       ` Will Deacon
  0 siblings, 1 reply; 65+ messages in thread
From: Robin Murphy @ 2021-08-02 14:15 UTC (permalink / raw)
  To: Will Deacon
  Cc: joro, iommu, linux-arm-kernel, linux-kernel,
	suravee.suthikulpanit, baolu.lu, john.garry, dianders

On 2021-08-02 14:04, Will Deacon wrote:
> On Wed, Jul 28, 2021 at 04:58:44PM +0100, Robin Murphy wrote:
>> To make io-pgtable aware of a flush queue being dynamically enabled,
>> allow IO_PGTABLE_QUIRK_NON_STRICT to be set even after a domain has been
>> attached to, and hook up the final piece of the puzzle in iommu-dma.
>>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> ---
>>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 15 +++++++++++++++
>>   drivers/iommu/arm/arm-smmu/arm-smmu.c       | 11 +++++++++++
>>   drivers/iommu/dma-iommu.c                   |  3 +++
>>   3 files changed, 29 insertions(+)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> index 19400826eba7..40fa9cb382c3 100644
>> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> @@ -2711,6 +2711,20 @@ static int arm_smmu_enable_nesting(struct iommu_domain *domain)
>>   	return ret;
>>   }
>>   
>> +static int arm_smmu_set_pgtable_quirks(struct iommu_domain *domain,
>> +		unsigned long quirks)
>> +{
>> +	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
>> +
>> +	if (quirks == IO_PGTABLE_QUIRK_NON_STRICT && smmu_domain->pgtbl_ops) {
>> +		struct io_pgtable *iop = io_pgtable_ops_to_pgtable(smmu_domain->pgtbl_ops);
>> +
>> +		iop->cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
>> +		return 0;
>> +	}
>> +	return -EINVAL;
>> +}
> 
> I don't see anything serialising this against a concurrent iommu_unmap(), so
> the ordering and atomicity looks quite suspicious to me here. I don't think
> it's just the page-table quirks either, but also setting cookie->fq_domain.

Heh, I confess to very much taking the cheeky "let's say nothing and see 
what Will thinks about concurrency" approach here :)

The beauty of only allowing relaxation in the strict->non-strict 
direction is that it shouldn't need serialising as such - it doesn't 
matter if the update to cookie->fq_domain is observed between 
iommu_unmap() and iommu_dma_free_iova(), since there's no correctness 
impact to queueing IOVAs which may already have been invalidated and may 
or may not have been synced. AFAICS the only condition which matters is 
that the setting of the io-pgtable quirk must observe fq_domain being 
set. It feels like there must be enough dependencies on the read side, 
but we might need an smp_wmb() between the two in iommu_dma_init_fq()?

I've also flip-flopped a bit on whether fq_domain needs to be accessed 
with READ_ONCE/WRITE_ONCE - by the time of posting I'd convinced myself 
that it was probably OK, but looking again now I suppose this wacky 
reordering is theoretically possible:


	iommu_dma_unmap() {
		bool free_fq = cookie->fq_domain; // == false

		iommu_unmap();

		if (!cookie->fq_domain) // observes new non-NULL value
			iommu_tlb_sync(); // skipped

		iommu_dma_free_iova { // inlined
			if (free_fq) // false
				queue_iova();
			else
				free_iova_fast(); // Uh-oh!
		}
	}

so although I still can't see atomicity being a problem I guess we do 
need it for the sake of reordering at least.

Cheers,
Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 23/24] iommu/arm-smmu: Allow non-strict in pgtable_quirks interface
  2021-08-02 14:15     ` Robin Murphy
@ 2021-08-03 10:36       ` Will Deacon
  2021-08-03 12:13         ` Robin Murphy
  0 siblings, 1 reply; 65+ messages in thread
From: Will Deacon @ 2021-08-03 10:36 UTC (permalink / raw)
  To: Robin Murphy
  Cc: joro, iommu, linux-arm-kernel, linux-kernel,
	suravee.suthikulpanit, baolu.lu, john.garry, dianders

On Mon, Aug 02, 2021 at 03:15:50PM +0100, Robin Murphy wrote:
> On 2021-08-02 14:04, Will Deacon wrote:
> > On Wed, Jul 28, 2021 at 04:58:44PM +0100, Robin Murphy wrote:
> > > To make io-pgtable aware of a flush queue being dynamically enabled,
> > > allow IO_PGTABLE_QUIRK_NON_STRICT to be set even after a domain has been
> > > attached to, and hook up the final piece of the puzzle in iommu-dma.
> > > 
> > > Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> > > ---
> > >   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 15 +++++++++++++++
> > >   drivers/iommu/arm/arm-smmu/arm-smmu.c       | 11 +++++++++++
> > >   drivers/iommu/dma-iommu.c                   |  3 +++
> > >   3 files changed, 29 insertions(+)
> > > 
> > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > index 19400826eba7..40fa9cb382c3 100644
> > > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > @@ -2711,6 +2711,20 @@ static int arm_smmu_enable_nesting(struct iommu_domain *domain)
> > >   	return ret;
> > >   }
> > > +static int arm_smmu_set_pgtable_quirks(struct iommu_domain *domain,
> > > +		unsigned long quirks)
> > > +{
> > > +	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> > > +
> > > +	if (quirks == IO_PGTABLE_QUIRK_NON_STRICT && smmu_domain->pgtbl_ops) {
> > > +		struct io_pgtable *iop = io_pgtable_ops_to_pgtable(smmu_domain->pgtbl_ops);
> > > +
> > > +		iop->cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
> > > +		return 0;
> > > +	}
> > > +	return -EINVAL;
> > > +}
> > 
> > I don't see anything serialising this against a concurrent iommu_unmap(), so
> > the ordering and atomicity looks quite suspicious to me here. I don't think
> > it's just the page-table quirks either, but also setting cookie->fq_domain.
> 
> Heh, I confess to very much taking the cheeky "let's say nothing and see
> what Will thinks about concurrency" approach here :)

Damnit, I fell for that didn't I?

Overall, I'm really nervous about the concurrency here and think we'd be
better off requiring the unbind as we have for the other domain changes.

> The beauty of only allowing relaxation in the strict->non-strict direction
> is that it shouldn't need serialising as such - it doesn't matter if the
> update to cookie->fq_domain is observed between iommu_unmap() and
> iommu_dma_free_iova(), since there's no correctness impact to queueing IOVAs
> which may already have been invalidated and may or may not have been synced.
> AFAICS the only condition which matters is that the setting of the
> io-pgtable quirk must observe fq_domain being set. It feels like there must
> be enough dependencies on the read side, but we might need an smp_wmb()
> between the two in iommu_dma_init_fq()?
> 
> I've also flip-flopped a bit on whether fq_domain needs to be accessed with
> READ_ONCE/WRITE_ONCE - by the time of posting I'd convinced myself that it
> was probably OK, but looking again now I suppose this wacky reordering is
> theoretically possible:
> 
> 
> 	iommu_dma_unmap() {
> 		bool free_fq = cookie->fq_domain; // == false
> 
> 		iommu_unmap();
> 
> 		if (!cookie->fq_domain) // observes new non-NULL value
> 			iommu_tlb_sync(); // skipped
> 
> 		iommu_dma_free_iova { // inlined
> 			if (free_fq) // false
> 				queue_iova();
> 			else
> 				free_iova_fast(); // Uh-oh!
> 		}
> 	}
> 
> so although I still can't see atomicity being a problem I guess we do need
> it for the sake of reordering at least.

With your changes, I think quite a few things can go wrong.

  * cookie->fq_domain may be observed but iovad->fq could be NULL
  * We can miss the smp_wmb() in the pgtable code but end up deferring the
    IOVA reclaim
  * iommu_change_dev_def_domain() only holds the group mutex afaict, so can
    possibly run concurrently with itself on the same domain?
  * iommu_dma_init_fq() can flip the domain type back from
    IOMMU_DOMAIN_DMA_FQ to IOMMU_DOMAIN_DMA on the error path
  * set_pgtable_quirks() isn't atomic, which probably is ok for now, but the
    moment we use it anywhere else it's dangerous

and I suspect there are more :/

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 23/24] iommu/arm-smmu: Allow non-strict in pgtable_quirks interface
  2021-08-03 10:36       ` Will Deacon
@ 2021-08-03 12:13         ` Robin Murphy
  2021-08-03 12:35           ` Will Deacon
  0 siblings, 1 reply; 65+ messages in thread
From: Robin Murphy @ 2021-08-03 12:13 UTC (permalink / raw)
  To: Will Deacon
  Cc: joro, iommu, linux-arm-kernel, linux-kernel,
	suravee.suthikulpanit, baolu.lu, john.garry, dianders

On 2021-08-03 11:36, Will Deacon wrote:
> On Mon, Aug 02, 2021 at 03:15:50PM +0100, Robin Murphy wrote:
>> On 2021-08-02 14:04, Will Deacon wrote:
>>> On Wed, Jul 28, 2021 at 04:58:44PM +0100, Robin Murphy wrote:
>>>> To make io-pgtable aware of a flush queue being dynamically enabled,
>>>> allow IO_PGTABLE_QUIRK_NON_STRICT to be set even after a domain has been
>>>> attached to, and hook up the final piece of the puzzle in iommu-dma.
>>>>
>>>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>>>> ---
>>>>    drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 15 +++++++++++++++
>>>>    drivers/iommu/arm/arm-smmu/arm-smmu.c       | 11 +++++++++++
>>>>    drivers/iommu/dma-iommu.c                   |  3 +++
>>>>    3 files changed, 29 insertions(+)
>>>>
>>>> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>>>> index 19400826eba7..40fa9cb382c3 100644
>>>> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>>>> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>>>> @@ -2711,6 +2711,20 @@ static int arm_smmu_enable_nesting(struct iommu_domain *domain)
>>>>    	return ret;
>>>>    }
>>>> +static int arm_smmu_set_pgtable_quirks(struct iommu_domain *domain,
>>>> +		unsigned long quirks)
>>>> +{
>>>> +	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
>>>> +
>>>> +	if (quirks == IO_PGTABLE_QUIRK_NON_STRICT && smmu_domain->pgtbl_ops) {
>>>> +		struct io_pgtable *iop = io_pgtable_ops_to_pgtable(smmu_domain->pgtbl_ops);
>>>> +
>>>> +		iop->cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
>>>> +		return 0;
>>>> +	}
>>>> +	return -EINVAL;
>>>> +}
>>>
>>> I don't see anything serialising this against a concurrent iommu_unmap(), so
>>> the ordering and atomicity looks quite suspicious to me here. I don't think
>>> it's just the page-table quirks either, but also setting cookie->fq_domain.
>>
>> Heh, I confess to very much taking the cheeky "let's say nothing and see
>> what Will thinks about concurrency" approach here :)
> 
> Damnit, I fell for that didn't I?
> 
> Overall, I'm really nervous about the concurrency here and think we'd be
> better off requiring the unbind as we have for the other domain changes.

Sure, the dynamic switch is what makes it ultimately work for Doug's 
use-case (where the unbind isn't viable), but I had every expectation 
that we might need to hold back those two patches for much deeper 
consideration. It's no accident that the first 22 patches can still be 
usefully applied without them!

In all honesty I don't really like this particular patch much, mostly 
because I increasingly dislike IO_PGTABLE_QUIRK_NON_STRICT at all, but 
since the interface was there it made it super easy to prove the 
concept. I have a more significant refactoring of the io-pgtable code 
sketched out in my mind already, it's just going to be more work.

>> The beauty of only allowing relaxation in the strict->non-strict direction
>> is that it shouldn't need serialising as such - it doesn't matter if the
>> update to cookie->fq_domain is observed between iommu_unmap() and
>> iommu_dma_free_iova(), since there's no correctness impact to queueing IOVAs
>> which may already have been invalidated and may or may not have been synced.
>> AFAICS the only condition which matters is that the setting of the
>> io-pgtable quirk must observe fq_domain being set. It feels like there must
>> be enough dependencies on the read side, but we might need an smp_wmb()
>> between the two in iommu_dma_init_fq()?
>>
>> I've also flip-flopped a bit on whether fq_domain needs to be accessed with
>> READ_ONCE/WRITE_ONCE - by the time of posting I'd convinced myself that it
>> was probably OK, but looking again now I suppose this wacky reordering is
>> theoretically possible:
>>
>>
>> 	iommu_dma_unmap() {
>> 		bool free_fq = cookie->fq_domain; // == false
>>
>> 		iommu_unmap();
>>
>> 		if (!cookie->fq_domain) // observes new non-NULL value
>> 			iommu_tlb_sync(); // skipped
>>
>> 		iommu_dma_free_iova { // inlined
>> 			if (free_fq) // false
>> 				queue_iova();
>> 			else
>> 				free_iova_fast(); // Uh-oh!
>> 		}
>> 	}
>>
>> so although I still can't see atomicity being a problem I guess we do need
>> it for the sake of reordering at least.
> 
> With your changes, I think quite a few things can go wrong.
> 
>    * cookie->fq_domain may be observed but iovad->fq could be NULL

Good point, I guess that already technically applies (if iovad->fq sat 
in a write buffer long enough), but it certainly becomes far easier to 
provoke. However a barrier after assigning fq_domain (as mentioned 
above) paired with the control dependency around the queue_iova() call 
would also fix that, right?

>    * We can miss the smp_wmb() in the pgtable code but end up deferring the
>      IOVA reclaim
>    * iommu_change_dev_def_domain() only holds the group mutex afaict, so can
>      possibly run concurrently with itself on the same domain?
>    * iommu_dma_init_fq() can flip the domain type back from
>      IOMMU_DOMAIN_DMA_FQ to IOMMU_DOMAIN_DMA on the error path
>    * set_pgtable_quirks() isn't atomic, which probably is ok for now, but the
>      moment we use it anywhere else it's dangerous

In other words, IO_PGTABLE_QUIRK_NON_STRICT is definitely the problem. 
I'll have a hack on that this afternoon, and if it starts to look 
rabbit-holey I'll split this bit off and post v3 of the rest of the series.

If all the io-pgtable and fq behaviour for a given call could be 
consistent based on a single READ_ONCE(cookie->fq_domain) in 
iommu_dma_unmap(), do you see any remaining issues other than the point 
above?

Thanks,
Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 23/24] iommu/arm-smmu: Allow non-strict in pgtable_quirks interface
  2021-08-03 12:13         ` Robin Murphy
@ 2021-08-03 12:35           ` Will Deacon
  0 siblings, 0 replies; 65+ messages in thread
From: Will Deacon @ 2021-08-03 12:35 UTC (permalink / raw)
  To: Robin Murphy
  Cc: joro, iommu, linux-arm-kernel, linux-kernel,
	suravee.suthikulpanit, baolu.lu, john.garry, dianders

On Tue, Aug 03, 2021 at 01:13:12PM +0100, Robin Murphy wrote:
> On 2021-08-03 11:36, Will Deacon wrote:
> > Overall, I'm really nervous about the concurrency here and think we'd be
> > better off requiring the unbind as we have for the other domain changes.
> 
> Sure, the dynamic switch is what makes it ultimately work for Doug's
> use-case (where the unbind isn't viable), but I had every expectation that
> we might need to hold back those two patches for much deeper consideration.
> It's no accident that the first 22 patches can still be usefully applied
> without them!

Oh, the rest of the series looks great which is why I jumped on this bit!

> In all honesty I don't really like this particular patch much, mostly
> because I increasingly dislike IO_PGTABLE_QUIRK_NON_STRICT at all, but since
> the interface was there it made it super easy to prove the concept. I have a
> more significant refactoring of the io-pgtable code sketched out in my mind
> already, it's just going to be more work.

Intriguing... Move the smp_wmb() into the IOVA code instead?

> > With your changes, I think quite a few things can go wrong.
> > 
> >    * cookie->fq_domain may be observed but iovad->fq could be NULL
> 
> Good point, I guess that already technically applies (if iovad->fq sat in a
> write buffer long enough), but it certainly becomes far easier to provoke.
> However a barrier after assigning fq_domain (as mentioned above) paired with
> the control dependency around the queue_iova() call would also fix that,
> right?
> 
> >    * We can miss the smp_wmb() in the pgtable code but end up deferring the
> >      IOVA reclaim
> >    * iommu_change_dev_def_domain() only holds the group mutex afaict, so can
> >      possibly run concurrently with itself on the same domain?
> >    * iommu_dma_init_fq() can flip the domain type back from
> >      IOMMU_DOMAIN_DMA_FQ to IOMMU_DOMAIN_DMA on the error path
> >    * set_pgtable_quirks() isn't atomic, which probably is ok for now, but the
> >      moment we use it anywhere else it's dangerous
> 
> In other words, IO_PGTABLE_QUIRK_NON_STRICT is definitely the problem. I'll
> have a hack on that this afternoon, and if it starts to look rabbit-holey
> I'll split this bit off and post v3 of the rest of the series.
> 
> If all the io-pgtable and fq behaviour for a given call could be consistent
> based on a single READ_ONCE(cookie->fq_domain) in iommu_dma_unmap(), do you
> see any remaining issues other than the point above?

I'd have to see the patches, and I didn't look exhaustively at the current
stuff. But yes, I think the basic flow needs to be that there is an atomic
flag (i.e. cookie->fq_domain) which indicates which mode is being used
and if you flip that concurrently then you need to guarantee that everybody
is either using the old more or the new mode and not some halfway state in
between.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 65+ messages in thread

end of thread, other threads:[~2021-08-03 12:58 UTC | newest]

Thread overview: 65+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-28 15:58 [PATCH v2 00/24] iommu: Refactor DMA domain strictness Robin Murphy
2021-07-28 15:58 ` [PATCH v2 01/24] iommu: Pull IOVA cookie management into the core Robin Murphy
2021-07-30  6:06   ` Lu Baolu
2021-07-30  9:32   ` Jean-Philippe Brucker
2021-07-28 15:58 ` [PATCH v2 02/24] iommu/amd: Drop IOVA cookie management Robin Murphy
2021-07-28 15:58 ` [PATCH v2 03/24] iommu/arm-smmu: " Robin Murphy
2021-07-28 15:58 ` [PATCH v2 04/24] iommu/vt-d: " Robin Murphy
2021-07-30  6:07   ` Lu Baolu
2021-07-28 15:58 ` [PATCH v2 05/24] iommu/exynos: " Robin Murphy
2021-07-28 15:58 ` [PATCH v2 06/24] iommu/ipmmu-vmsa: " Robin Murphy
2021-07-28 15:58 ` [PATCH v2 07/24] iommu/mtk: " Robin Murphy
2021-07-28 15:58 ` [PATCH v2 08/24] iommu/rockchip: " Robin Murphy
2021-07-28 15:58 ` [PATCH v2 09/24] iommu/sprd: " Robin Murphy
2021-07-28 15:58 ` [PATCH v2 10/24] iommu/sun50i: " Robin Murphy
2021-07-28 15:58 ` [PATCH v2 11/24] iommu/virtio: " Robin Murphy
2021-07-30  9:20   ` Jean-Philippe Brucker
2021-07-28 15:58 ` [PATCH v2 12/24] iommu/dma: Unexport " Robin Murphy
2021-07-30  6:07   ` Lu Baolu
2021-07-30  9:21   ` Jean-Philippe Brucker
2021-07-28 15:58 ` [PATCH v2 13/24] iommu/dma: Remove redundant "!dev" checks Robin Murphy
2021-07-30  6:08   ` Lu Baolu
2021-07-28 15:58 ` [PATCH v2 14/24] iommu: Introduce explicit type for non-strict DMA domains Robin Murphy
2021-07-30  6:08   ` Lu Baolu
2021-07-30  9:23   ` Jean-Philippe Brucker
2021-07-28 15:58 ` [PATCH v2 15/24] iommu/amd: Prepare for multiple DMA domain types Robin Murphy
2021-07-28 15:58 ` [PATCH v2 16/24] iommu/arm-smmu: " Robin Murphy
2021-07-28 15:58 ` [PATCH v2 17/24] iommu/vt-d: " Robin Murphy
2021-07-30  6:09   ` Lu Baolu
2021-07-28 15:58 ` [PATCH v2 18/24] iommu: Express DMA strictness via the domain type Robin Murphy
2021-07-29  7:13   ` Lu Baolu
2021-07-29  9:36     ` Robin Murphy
2021-07-29 12:42       ` Lu Baolu
2021-07-30  6:09       ` Lu Baolu
2021-07-28 15:58 ` [PATCH v2 19/24] iommu: Expose DMA domain strictness via sysfs Robin Murphy
2021-07-30  6:10   ` Lu Baolu
2021-07-30  9:28   ` Jean-Philippe Brucker
2021-07-30 10:20   ` John Garry
2021-07-28 15:58 ` [PATCH v2 20/24] iommu: Merge strictness and domain type configs Robin Murphy
2021-07-30  6:10   ` Lu Baolu
2021-07-30  9:29   ` Jean-Philippe Brucker
2021-07-30  9:33   ` John Garry
2021-07-28 15:58 ` [PATCH v2 21/24] iommu/dma: Factor out flush queue init Robin Murphy
2021-07-30  6:11   ` Lu Baolu
2021-07-30  9:20   ` John Garry
2021-07-28 15:58 ` [PATCH v2 22/24] iommu: Allow enabling non-strict mode dynamically Robin Murphy
2021-07-30  6:11   ` Lu Baolu
2021-07-30  9:24   ` John Garry
2021-07-28 15:58 ` [PATCH v2 23/24] iommu/arm-smmu: Allow non-strict in pgtable_quirks interface Robin Murphy
2021-08-02 13:04   ` Will Deacon
2021-08-02 14:15     ` Robin Murphy
2021-08-03 10:36       ` Will Deacon
2021-08-03 12:13         ` Robin Murphy
2021-08-03 12:35           ` Will Deacon
2021-07-28 15:58 ` [PATCH v2 24/24] iommu: Only log strictness for DMA domains Robin Murphy
2021-07-29  9:04   ` John Garry
2021-07-30  6:12   ` Lu Baolu
2021-07-29  2:55 ` [PATCH v2 00/24] iommu: Refactor DMA domain strictness chenxiang (M)
2021-07-29 10:59   ` Robin Murphy
2021-07-30  1:21     ` chenxiang (M)
2021-07-29 15:04 ` Heiko Stübner
2021-07-29 15:43   ` Robin Murphy
2021-07-29 15:53     ` Heiko Stübner
2021-07-29 16:29       ` Robin Murphy
2021-07-29 22:33 ` Doug Anderson
2021-07-30  0:06   ` Doug Anderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).