linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] iommu: Make blocking domain static for group
@ 2022-05-16  1:57 Lu Baolu
  2022-05-16  1:57 ` [PATCH 1/5] iommu: Rename attach_dev to set_dev in domain ops Lu Baolu
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Lu Baolu @ 2022-05-16  1:57 UTC (permalink / raw)
  To: Joerg Roedel, Jason Gunthorpe, Christoph Hellwig, Kevin Tian,
	Ashok Raj, Will Deacon, Robin Murphy, Jean-Philippe Brucker
  Cc: Eric Auger, Liu Yi L, Jacob jun Pan, iommu, linux-kernel, Lu Baolu

Hi folks,

This is a follow-up series after several discussions on blocking domain.
The latest discussion could be found here.

https://lore.kernel.org/linux-iommu/20220510140238.GD49344@nvidia.com/

This makes blocking domain static by:

- Each IOMMU driver is required to report domain ops for the blocking
  domain in its iommu_ops. Some IOMMU drivers support detaching domain
  by clearing an entry in the device context, while others not. To
  distinguish this capability among the IOMMU drivers, a flag is added
  to the domain ops.

- Similar to the default domain, each iommu group also has a static
  blokcing domain. The blocking domain is allocated when the first
  device joins the group and freed after the last device leaves.

- As .detach_dev equals to either setting the default domain or blocking
  domain to the device, this callback is not needed anymore. It is
  removed in this series.

Please kindly review and suggest. Very appreciated.

Best regards,
baolu 

Lu Baolu (5):
  iommu: Rename attach_dev to set_dev in domain ops
  iommu: Add blocking_domain_ops field in iommu_ops
  iommu: Make blocking domain static for iommu group
  iommu: Use blocking domain for empty domain attaching
  iommu: Remove .detach_dev from iommu domain ops

 include/linux/iommu.h                       |  13 ++-
 include/trace/events/iommu.h                |   7 --
 drivers/iommu/amd/iommu.c                   |  15 ++-
 drivers/iommu/apple-dart.c                  |  15 ++-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |   5 +-
 drivers/iommu/arm/arm-smmu/arm-smmu.c       |   5 +-
 drivers/iommu/arm/arm-smmu/qcom_iommu.c     |  15 ++-
 drivers/iommu/exynos-iommu.c                |  15 ++-
 drivers/iommu/fsl_pamu_domain.c             |  15 ++-
 drivers/iommu/intel/iommu.c                 |  15 ++-
 drivers/iommu/iommu-traces.c                |   1 -
 drivers/iommu/iommu.c                       | 122 ++++++++++----------
 drivers/iommu/ipmmu-vmsa.c                  |  15 ++-
 drivers/iommu/msm_iommu.c                   |  15 ++-
 drivers/iommu/mtk_iommu.c                   |  15 ++-
 drivers/iommu/mtk_iommu_v1.c                |  15 ++-
 drivers/iommu/omap-iommu.c                  |  15 ++-
 drivers/iommu/rockchip-iommu.c              |  15 ++-
 drivers/iommu/s390-iommu.c                  |  15 ++-
 drivers/iommu/sprd-iommu.c                  |  14 ++-
 drivers/iommu/sun50i-iommu.c                |  15 ++-
 drivers/iommu/tegra-gart.c                  |  15 ++-
 drivers/iommu/tegra-smmu.c                  |  15 ++-
 drivers/iommu/virtio-iommu.c                |   5 +-
 24 files changed, 299 insertions(+), 113 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread
* [PATCH 1/5] iommu: Rename attach_dev to set_dev in domain ops
  2022-05-16  1:57 [PATCH 0/5] iommu: Make blocking domain static for group Lu Baolu
@ 2022-05-16  1:57 ` Lu Baolu
  2022-05-16  1:57 ` [PATCH 2/5] iommu: Add blocking_domain_ops field in iommu_ops Lu Baolu
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 18+ messages in thread
From: Lu Baolu @ 2022-05-16  1:57 UTC (permalink / raw)
  To: Joerg Roedel, Jason Gunthorpe, Christoph Hellwig, Kevin Tian,
	Ashok Raj, Will Deacon, Robin Murphy, Jean-Philippe Brucker
  Cc: Eric Auger, Liu Yi L, Jacob jun Pan, iommu, linux-kernel, Lu Baolu

The detach callback of the iommu domain ops is not used in some IOMMU
drivers. The detach_dev actually means setting a default domain or a
blocking domain to the device. As attach_dev actually acts as setting
domain for a device, this renames attach_dev to set_dev to reflect the
actual usage.

Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 include/linux/iommu.h                       | 4 ++--
 drivers/iommu/amd/iommu.c                   | 2 +-
 drivers/iommu/apple-dart.c                  | 2 +-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
 drivers/iommu/arm/arm-smmu/arm-smmu.c       | 2 +-
 drivers/iommu/arm/arm-smmu/qcom_iommu.c     | 2 +-
 drivers/iommu/exynos-iommu.c                | 2 +-
 drivers/iommu/fsl_pamu_domain.c             | 2 +-
 drivers/iommu/intel/iommu.c                 | 2 +-
 drivers/iommu/iommu.c                       | 6 +++---
 drivers/iommu/ipmmu-vmsa.c                  | 2 +-
 drivers/iommu/msm_iommu.c                   | 2 +-
 drivers/iommu/mtk_iommu.c                   | 2 +-
 drivers/iommu/mtk_iommu_v1.c                | 2 +-
 drivers/iommu/omap-iommu.c                  | 2 +-
 drivers/iommu/rockchip-iommu.c              | 2 +-
 drivers/iommu/s390-iommu.c                  | 2 +-
 drivers/iommu/sprd-iommu.c                  | 2 +-
 drivers/iommu/sun50i-iommu.c                | 2 +-
 drivers/iommu/tegra-gart.c                  | 2 +-
 drivers/iommu/tegra-smmu.c                  | 2 +-
 drivers/iommu/virtio-iommu.c                | 2 +-
 22 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 5e1afe169549..572399ac1d83 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -261,7 +261,7 @@ struct iommu_ops {
 
 /**
  * struct iommu_domain_ops - domain specific operations
- * @attach_dev: attach an iommu domain to a device
+ * @set_dev: set an iommu domain to a device
  * @detach_dev: detach an iommu domain from a device
  * @map: map a physically contiguous memory region to an iommu domain
  * @map_pages: map a physically contiguous set of pages of the same size to
@@ -281,7 +281,7 @@ struct iommu_ops {
  * @free: Release the domain after use.
  */
 struct iommu_domain_ops {
-	int (*attach_dev)(struct iommu_domain *domain, struct device *dev);
+	int (*set_dev)(struct iommu_domain *domain, struct device *dev);
 	void (*detach_dev)(struct iommu_domain *domain, struct device *dev);
 
 	int (*map)(struct iommu_domain *domain, unsigned long iova,
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 840831d5d2ad..01b8668ef46d 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2285,7 +2285,7 @@ const struct iommu_ops amd_iommu_ops = {
 	.pgsize_bitmap	= AMD_IOMMU_PGSIZES,
 	.def_domain_type = amd_iommu_def_domain_type,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
-		.attach_dev	= amd_iommu_attach_device,
+		.set_dev	= amd_iommu_attach_device,
 		.detach_dev	= amd_iommu_detach_device,
 		.map		= amd_iommu_map,
 		.unmap		= amd_iommu_unmap,
diff --git a/drivers/iommu/apple-dart.c b/drivers/iommu/apple-dart.c
index 8af0242a90d9..a0b7281f1989 100644
--- a/drivers/iommu/apple-dart.c
+++ b/drivers/iommu/apple-dart.c
@@ -775,7 +775,7 @@ static const struct iommu_ops apple_dart_iommu_ops = {
 	.pgsize_bitmap = -1UL, /* Restricted during dart probe */
 	.owner = THIS_MODULE,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
-		.attach_dev	= apple_dart_attach_dev,
+		.set_dev	= apple_dart_attach_dev,
 		.detach_dev	= apple_dart_detach_dev,
 		.map_pages	= apple_dart_map_pages,
 		.unmap_pages	= apple_dart_unmap_pages,
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 88817a3376ef..7e7d9e0b7aee 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2859,7 +2859,7 @@ static struct iommu_ops arm_smmu_ops = {
 	.pgsize_bitmap		= -1UL, /* Restricted during device attach */
 	.owner			= THIS_MODULE,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
-		.attach_dev		= arm_smmu_attach_dev,
+		.set_dev		= arm_smmu_attach_dev,
 		.map_pages		= arm_smmu_map_pages,
 		.unmap_pages		= arm_smmu_unmap_pages,
 		.flush_iotlb_all	= arm_smmu_flush_iotlb_all,
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 2ed3594f384e..c91d12b7e283 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1597,7 +1597,7 @@ static struct iommu_ops arm_smmu_ops = {
 	.pgsize_bitmap		= -1UL, /* Restricted during device attach */
 	.owner			= THIS_MODULE,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
-		.attach_dev		= arm_smmu_attach_dev,
+		.set_dev		= arm_smmu_attach_dev,
 		.map_pages		= arm_smmu_map_pages,
 		.unmap_pages		= arm_smmu_unmap_pages,
 		.flush_iotlb_all	= arm_smmu_flush_iotlb_all,
diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
index 4c077c38fbd6..cf624bd305e0 100644
--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -596,7 +596,7 @@ static const struct iommu_ops qcom_iommu_ops = {
 	.of_xlate	= qcom_iommu_of_xlate,
 	.pgsize_bitmap	= SZ_4K | SZ_64K | SZ_1M | SZ_16M,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
-		.attach_dev	= qcom_iommu_attach_dev,
+		.set_dev	= qcom_iommu_attach_dev,
 		.detach_dev	= qcom_iommu_detach_dev,
 		.map		= qcom_iommu_map,
 		.unmap		= qcom_iommu_unmap,
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 71f2018e23fe..797348f3440e 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1315,7 +1315,7 @@ static const struct iommu_ops exynos_iommu_ops = {
 	.pgsize_bitmap = SECT_SIZE | LPAGE_SIZE | SPAGE_SIZE,
 	.of_xlate = exynos_iommu_of_xlate,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
-		.attach_dev	= exynos_iommu_attach_device,
+		.set_dev	= exynos_iommu_attach_device,
 		.detach_dev	= exynos_iommu_detach_device,
 		.map		= exynos_iommu_map,
 		.unmap		= exynos_iommu_unmap,
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 94b4589dc67c..7512c8e007e9 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -458,7 +458,7 @@ static const struct iommu_ops fsl_pamu_ops = {
 	.release_device	= fsl_pamu_release_device,
 	.device_group   = fsl_pamu_device_group,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
-		.attach_dev	= fsl_pamu_attach_device,
+		.set_dev	= fsl_pamu_attach_device,
 		.detach_dev	= fsl_pamu_detach_device,
 		.iova_to_phys	= fsl_pamu_iova_to_phys,
 		.free		= fsl_pamu_domain_free,
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index e56b3a4b6998..6abc1fbbd461 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4925,7 +4925,7 @@ const struct iommu_ops intel_iommu_ops = {
 	.page_response		= intel_svm_page_response,
 #endif
 	.default_domain_ops = &(const struct iommu_domain_ops) {
-		.attach_dev		= intel_iommu_attach_device,
+		.set_dev		= intel_iommu_attach_device,
 		.detach_dev		= intel_iommu_detach_device,
 		.map_pages		= intel_iommu_map_pages,
 		.unmap_pages		= intel_iommu_unmap_pages,
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 847ad47a2dfd..8eba26be4363 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1963,10 +1963,10 @@ static int __iommu_attach_device(struct iommu_domain *domain,
 {
 	int ret;
 
-	if (unlikely(domain->ops->attach_dev == NULL))
+	if (unlikely(domain->ops->set_dev == NULL))
 		return -ENODEV;
 
-	ret = domain->ops->attach_dev(domain, dev);
+	ret = domain->ops->set_dev(domain, dev);
 	if (!ret)
 		trace_attach_device_to_domain(dev);
 	return ret;
@@ -2142,7 +2142,7 @@ static int __iommu_group_set_domain(struct iommu_group *group,
 	}
 
 	/*
-	 * Changing the domain is done by calling attach_dev() on the new
+	 * Changing the domain is done by calling set_dev() on the new
 	 * domain. This switch does not have to be atomic and DMA can be
 	 * discarded during the transition. DMA must only be able to access
 	 * either new_domain or group->domain, never something else.
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 8fdb84b3642b..b361a4cff688 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -877,7 +877,7 @@ static const struct iommu_ops ipmmu_ops = {
 	.pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
 	.of_xlate = ipmmu_of_xlate,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
-		.attach_dev	= ipmmu_attach_device,
+		.set_dev	= ipmmu_attach_device,
 		.detach_dev	= ipmmu_detach_device,
 		.map		= ipmmu_map,
 		.unmap		= ipmmu_unmap,
diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index f09aedfdd462..569d36840b67 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -682,7 +682,7 @@ static struct iommu_ops msm_iommu_ops = {
 	.pgsize_bitmap = MSM_IOMMU_PGSIZES,
 	.of_xlate = qcom_iommu_of_xlate,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
-		.attach_dev	= msm_iommu_attach_dev,
+		.set_dev	= msm_iommu_attach_dev,
 		.detach_dev	= msm_iommu_detach_dev,
 		.map		= msm_iommu_map,
 		.unmap		= msm_iommu_unmap,
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index bb9dd92c9898..33ec401d40eb 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -937,7 +937,7 @@ static const struct iommu_ops mtk_iommu_ops = {
 	.pgsize_bitmap	= SZ_4K | SZ_64K | SZ_1M | SZ_16M,
 	.owner		= THIS_MODULE,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
-		.attach_dev	= mtk_iommu_attach_device,
+		.set_dev	= mtk_iommu_attach_device,
 		.detach_dev	= mtk_iommu_detach_device,
 		.map		= mtk_iommu_map,
 		.unmap		= mtk_iommu_unmap,
diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
index e1cb51b9866c..fb55802fb841 100644
--- a/drivers/iommu/mtk_iommu_v1.c
+++ b/drivers/iommu/mtk_iommu_v1.c
@@ -594,7 +594,7 @@ static const struct iommu_ops mtk_iommu_v1_ops = {
 	.pgsize_bitmap	= ~0UL << MT2701_IOMMU_PAGE_SHIFT,
 	.owner          = THIS_MODULE,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
-		.attach_dev	= mtk_iommu_v1_attach_device,
+		.set_dev	= mtk_iommu_v1_attach_device,
 		.detach_dev	= mtk_iommu_v1_detach_device,
 		.map		= mtk_iommu_v1_map,
 		.unmap		= mtk_iommu_v1_unmap,
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index d9cf2820c02e..6720dcb437a0 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -1739,7 +1739,7 @@ static const struct iommu_ops omap_iommu_ops = {
 	.device_group	= omap_iommu_device_group,
 	.pgsize_bitmap	= OMAP_IOMMU_PGSIZES,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
-		.attach_dev	= omap_iommu_attach_dev,
+		.set_dev	= omap_iommu_attach_dev,
 		.detach_dev	= omap_iommu_detach_dev,
 		.map		= omap_iommu_map,
 		.unmap		= omap_iommu_unmap,
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index ab57c4b8fade..0a4196c34179 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -1193,7 +1193,7 @@ static const struct iommu_ops rk_iommu_ops = {
 	.pgsize_bitmap = RK_IOMMU_PGSIZE_BITMAP,
 	.of_xlate = rk_iommu_of_xlate,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
-		.attach_dev	= rk_iommu_attach_device,
+		.set_dev	= rk_iommu_attach_device,
 		.detach_dev	= rk_iommu_detach_device,
 		.map		= rk_iommu_map,
 		.unmap		= rk_iommu_unmap,
diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c
index 3833e86c6e7b..cde01c72f573 100644
--- a/drivers/iommu/s390-iommu.c
+++ b/drivers/iommu/s390-iommu.c
@@ -368,7 +368,7 @@ static const struct iommu_ops s390_iommu_ops = {
 	.device_group = generic_device_group,
 	.pgsize_bitmap = S390_IOMMU_PGSIZES,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
-		.attach_dev	= s390_iommu_attach_device,
+		.set_dev	= s390_iommu_attach_device,
 		.detach_dev	= s390_iommu_detach_device,
 		.map		= s390_iommu_map,
 		.unmap		= s390_iommu_unmap,
diff --git a/drivers/iommu/sprd-iommu.c b/drivers/iommu/sprd-iommu.c
index bd409bab6286..45b58845f5f8 100644
--- a/drivers/iommu/sprd-iommu.c
+++ b/drivers/iommu/sprd-iommu.c
@@ -423,7 +423,7 @@ static const struct iommu_ops sprd_iommu_ops = {
 	.pgsize_bitmap	= ~0UL << SPRD_IOMMU_PAGE_SHIFT,
 	.owner		= THIS_MODULE,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
-		.attach_dev	= sprd_iommu_attach_device,
+		.set_dev	= sprd_iommu_attach_device,
 		.detach_dev	= sprd_iommu_detach_device,
 		.map		= sprd_iommu_map,
 		.unmap		= sprd_iommu_unmap,
diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
index c54ab477b8fd..041b30463552 100644
--- a/drivers/iommu/sun50i-iommu.c
+++ b/drivers/iommu/sun50i-iommu.c
@@ -766,7 +766,7 @@ static const struct iommu_ops sun50i_iommu_ops = {
 	.probe_device	= sun50i_iommu_probe_device,
 	.release_device	= sun50i_iommu_release_device,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
-		.attach_dev	= sun50i_iommu_attach_device,
+		.set_dev	= sun50i_iommu_attach_device,
 		.detach_dev	= sun50i_iommu_detach_device,
 		.flush_iotlb_all = sun50i_iommu_flush_iotlb_all,
 		.iotlb_sync	= sun50i_iommu_iotlb_sync,
diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c
index a6700a40a6f8..f083a9fba4d0 100644
--- a/drivers/iommu/tegra-gart.c
+++ b/drivers/iommu/tegra-gart.c
@@ -278,7 +278,7 @@ static const struct iommu_ops gart_iommu_ops = {
 	.pgsize_bitmap	= GART_IOMMU_PGSIZES,
 	.of_xlate	= gart_iommu_of_xlate,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
-		.attach_dev	= gart_iommu_attach_dev,
+		.set_dev	= gart_iommu_attach_dev,
 		.detach_dev	= gart_iommu_detach_dev,
 		.map		= gart_iommu_map,
 		.unmap		= gart_iommu_unmap,
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 1fea68e551f1..36e9c2864e3f 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -971,7 +971,7 @@ static const struct iommu_ops tegra_smmu_ops = {
 	.of_xlate = tegra_smmu_of_xlate,
 	.pgsize_bitmap = SZ_4K,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
-		.attach_dev	= tegra_smmu_attach_dev,
+		.set_dev	= tegra_smmu_attach_dev,
 		.detach_dev	= tegra_smmu_detach_dev,
 		.map		= tegra_smmu_map,
 		.unmap		= tegra_smmu_unmap,
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 25be4b822aa0..ce2bd01806d8 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -1017,7 +1017,7 @@ static struct iommu_ops viommu_ops = {
 	.of_xlate		= viommu_of_xlate,
 	.owner			= THIS_MODULE,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
-		.attach_dev		= viommu_attach_dev,
+		.set_dev		= viommu_attach_dev,
 		.map			= viommu_map,
 		.unmap			= viommu_unmap,
 		.iova_to_phys		= viommu_iova_to_phys,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 2/5] iommu: Add blocking_domain_ops field in iommu_ops
  2022-05-16  1:57 [PATCH 0/5] iommu: Make blocking domain static for group Lu Baolu
  2022-05-16  1:57 ` [PATCH 1/5] iommu: Rename attach_dev to set_dev in domain ops Lu Baolu
@ 2022-05-16  1:57 ` Lu Baolu
  2022-05-16  7:27   ` Christoph Hellwig
                     ` (2 more replies)
  2022-05-16  1:57 ` [PATCH 3/5] iommu: Make blocking domain static for iommu group Lu Baolu
                   ` (2 subsequent siblings)
  4 siblings, 3 replies; 18+ messages in thread
From: Lu Baolu @ 2022-05-16  1:57 UTC (permalink / raw)
  To: Joerg Roedel, Jason Gunthorpe, Christoph Hellwig, Kevin Tian,
	Ashok Raj, Will Deacon, Robin Murphy, Jean-Philippe Brucker
  Cc: Eric Auger, Liu Yi L, Jacob jun Pan, iommu, linux-kernel, Lu Baolu

Each IOMMU driver must provide a blocking domain ops. If the hardware
supports detaching domain from device, setting blocking domain equals
detaching the existing domain from the deivce. Otherwise, an UNMANAGED
domain without any mapping will be used instead.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 include/linux/iommu.h                       |  7 +++++++
 drivers/iommu/amd/iommu.c                   | 12 ++++++++++++
 drivers/iommu/apple-dart.c                  | 12 ++++++++++++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  3 +++
 drivers/iommu/arm/arm-smmu/arm-smmu.c       |  3 +++
 drivers/iommu/arm/arm-smmu/qcom_iommu.c     | 12 ++++++++++++
 drivers/iommu/exynos-iommu.c                | 12 ++++++++++++
 drivers/iommu/fsl_pamu_domain.c             | 12 ++++++++++++
 drivers/iommu/intel/iommu.c                 | 12 ++++++++++++
 drivers/iommu/ipmmu-vmsa.c                  | 12 ++++++++++++
 drivers/iommu/msm_iommu.c                   | 12 ++++++++++++
 drivers/iommu/mtk_iommu.c                   | 12 ++++++++++++
 drivers/iommu/mtk_iommu_v1.c                | 12 ++++++++++++
 drivers/iommu/omap-iommu.c                  | 12 ++++++++++++
 drivers/iommu/rockchip-iommu.c              | 12 ++++++++++++
 drivers/iommu/s390-iommu.c                  | 12 ++++++++++++
 drivers/iommu/sprd-iommu.c                  | 11 +++++++++++
 drivers/iommu/sun50i-iommu.c                | 12 ++++++++++++
 drivers/iommu/tegra-gart.c                  | 12 ++++++++++++
 drivers/iommu/tegra-smmu.c                  | 12 ++++++++++++
 drivers/iommu/virtio-iommu.c                |  3 +++
 21 files changed, 219 insertions(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 572399ac1d83..5e228aad0ef6 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -216,6 +216,7 @@ struct iommu_iotlb_gather {
  *		- IOMMU_DOMAIN_DMA: must use a dma domain
  *		- 0: use the default setting
  * @default_domain_ops: the default ops for domains
+ * @blocking_domain_ops: the blocking ops for domains
  * @pgsize_bitmap: bitmap of all possible supported page sizes
  * @owner: Driver module providing these ops
  */
@@ -255,6 +256,7 @@ struct iommu_ops {
 	int (*def_domain_type)(struct device *dev);
 
 	const struct iommu_domain_ops *default_domain_ops;
+	const struct iommu_domain_ops *blocking_domain_ops;
 	unsigned long pgsize_bitmap;
 	struct module *owner;
 };
@@ -279,6 +281,9 @@ struct iommu_ops {
  * @enable_nesting: Enable nesting
  * @set_pgtable_quirks: Set io page table quirks (IO_PGTABLE_QUIRK_*)
  * @free: Release the domain after use.
+ * @blocking_domain_detach: iommu hardware support detaching a domain from
+ *		a device, hence setting blocking domain to a device equals to
+ *		detach the existing domain from it.
  */
 struct iommu_domain_ops {
 	int (*set_dev)(struct iommu_domain *domain, struct device *dev);
@@ -310,6 +315,8 @@ struct iommu_domain_ops {
 				  unsigned long quirks);
 
 	void (*free)(struct iommu_domain *domain);
+
+	unsigned int blocking_domain_detach:1;
 };
 
 /**
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 01b8668ef46d..c66713439824 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2272,6 +2272,14 @@ static bool amd_iommu_enforce_cache_coherency(struct iommu_domain *domain)
 	return true;
 }
 
+static int amd_blocking_domain_set_dev(struct iommu_domain *domain,
+				       struct device *dev)
+{
+	amd_iommu_detach_device(domain, dev);
+
+	return 0;
+}
+
 const struct iommu_ops amd_iommu_ops = {
 	.capable = amd_iommu_capable,
 	.domain_alloc = amd_iommu_domain_alloc,
@@ -2295,6 +2303,10 @@ const struct iommu_ops amd_iommu_ops = {
 		.iotlb_sync	= amd_iommu_iotlb_sync,
 		.free		= amd_iommu_domain_free,
 		.enforce_cache_coherency = amd_iommu_enforce_cache_coherency,
+	},
+	.blocking_domain_ops = &(const struct iommu_domain_ops) {
+		.set_dev	= amd_blocking_domain_set_dev,
+		.blocking_domain_detach = true,
 	}
 };
 
diff --git a/drivers/iommu/apple-dart.c b/drivers/iommu/apple-dart.c
index a0b7281f1989..3c37762e01ec 100644
--- a/drivers/iommu/apple-dart.c
+++ b/drivers/iommu/apple-dart.c
@@ -763,6 +763,14 @@ static void apple_dart_get_resv_regions(struct device *dev,
 	iommu_dma_get_resv_regions(dev, head);
 }
 
+static int apple_dart_blocking_domain_set_dev(struct iommu_domain *domain,
+					      struct device *dev)
+{
+	apple_dart_detach_dev(domain, dev);
+
+	return 0;
+}
+
 static const struct iommu_ops apple_dart_iommu_ops = {
 	.domain_alloc = apple_dart_domain_alloc,
 	.probe_device = apple_dart_probe_device,
@@ -784,6 +792,10 @@ static const struct iommu_ops apple_dart_iommu_ops = {
 		.iotlb_sync_map	= apple_dart_iotlb_sync_map,
 		.iova_to_phys	= apple_dart_iova_to_phys,
 		.free		= apple_dart_domain_free,
+	},
+	.blocking_domain_ops = &(const struct iommu_domain_ops) {
+		.set_dev	= apple_dart_blocking_domain_set_dev,
+		.blocking_domain_detach = true,
 	}
 };
 
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 7e7d9e0b7aee..4b0ec5bef63b 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2867,6 +2867,9 @@ static struct iommu_ops arm_smmu_ops = {
 		.iova_to_phys		= arm_smmu_iova_to_phys,
 		.enable_nesting		= arm_smmu_enable_nesting,
 		.free			= arm_smmu_domain_free,
+	},
+	.blocking_domain_ops = &(const struct iommu_domain_ops) {
+		.set_dev		= arm_smmu_attach_dev,
 	}
 };
 
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index c91d12b7e283..0723f7c97763 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1606,6 +1606,9 @@ static struct iommu_ops arm_smmu_ops = {
 		.enable_nesting		= arm_smmu_enable_nesting,
 		.set_pgtable_quirks	= arm_smmu_set_pgtable_quirks,
 		.free			= arm_smmu_domain_free,
+	},
+	.blocking_domain_ops = &(const struct iommu_domain_ops) {
+		.set_dev		= arm_smmu_attach_dev,
 	}
 };
 
diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
index cf624bd305e0..dee9b5a3a324 100644
--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -587,6 +587,14 @@ static int qcom_iommu_of_xlate(struct device *dev, struct of_phandle_args *args)
 	return iommu_fwspec_add_ids(dev, &asid, 1);
 }
 
+static int qcom_blocking_domain_set_dev(struct iommu_domain *domain,
+					struct device *dev)
+{
+	qcom_iommu_detach_dev(domain, dev);
+
+	return 0;
+}
+
 static const struct iommu_ops qcom_iommu_ops = {
 	.capable	= qcom_iommu_capable,
 	.domain_alloc	= qcom_iommu_domain_alloc,
@@ -604,6 +612,10 @@ static const struct iommu_ops qcom_iommu_ops = {
 		.iotlb_sync	= qcom_iommu_iotlb_sync,
 		.iova_to_phys	= qcom_iommu_iova_to_phys,
 		.free		= qcom_iommu_domain_free,
+	},
+	.blocking_domain_ops = &(const struct iommu_domain_ops) {
+		.set_dev	= qcom_blocking_domain_set_dev,
+		.blocking_domain_detach = true,
 	}
 };
 
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 797348f3440e..bbecb9a2a554 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1307,6 +1307,14 @@ static int exynos_iommu_of_xlate(struct device *dev,
 	return 0;
 }
 
+static int exynos_blocking_domain_set_dev(struct iommu_domain *domain,
+					  struct device *dev)
+{
+	exynos_iommu_detach_device(domain, dev);
+
+	return 0;
+}
+
 static const struct iommu_ops exynos_iommu_ops = {
 	.domain_alloc = exynos_iommu_domain_alloc,
 	.device_group = generic_device_group,
@@ -1321,6 +1329,10 @@ static const struct iommu_ops exynos_iommu_ops = {
 		.unmap		= exynos_iommu_unmap,
 		.iova_to_phys	= exynos_iommu_iova_to_phys,
 		.free		= exynos_iommu_domain_free,
+	},
+	.blocking_domain_ops = &(const struct iommu_domain_ops) {
+		.set_dev	= exynos_blocking_domain_set_dev,
+		.blocking_domain_detach = true,
 	}
 };
 
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 7512c8e007e9..92fc320f8c83 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -451,6 +451,14 @@ static void fsl_pamu_release_device(struct device *dev)
 {
 }
 
+static int fsl_pamu_blocking_domain_set_dev(struct iommu_domain *domain,
+					    struct device *dev)
+{
+	fsl_pamu_detach_device(domain, dev);
+
+	return 0;
+}
+
 static const struct iommu_ops fsl_pamu_ops = {
 	.capable	= fsl_pamu_capable,
 	.domain_alloc	= fsl_pamu_domain_alloc,
@@ -462,6 +470,10 @@ static const struct iommu_ops fsl_pamu_ops = {
 		.detach_dev	= fsl_pamu_detach_device,
 		.iova_to_phys	= fsl_pamu_iova_to_phys,
 		.free		= fsl_pamu_domain_free,
+	},
+	.blocking_domain_ops = &(const struct iommu_domain_ops) {
+		.set_dev	= fsl_pamu_blocking_domain_set_dev,
+		.blocking_domain_detach = true,
 	}
 };
 
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 6abc1fbbd461..2060e8a540b3 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4904,6 +4904,14 @@ static void intel_iommu_iotlb_sync_map(struct iommu_domain *domain,
 	}
 }
 
+static int intel_blocking_domain_set_dev(struct iommu_domain *domain,
+					 struct device *dev)
+{
+	intel_iommu_detach_device(domain, dev);
+
+	return 0;
+}
+
 const struct iommu_ops intel_iommu_ops = {
 	.capable		= intel_iommu_capable,
 	.domain_alloc		= intel_iommu_domain_alloc,
@@ -4935,6 +4943,10 @@ const struct iommu_ops intel_iommu_ops = {
 		.iova_to_phys		= intel_iommu_iova_to_phys,
 		.free			= intel_iommu_domain_free,
 		.enforce_cache_coherency = intel_iommu_enforce_cache_coherency,
+	},
+	.blocking_domain_ops = &(const struct iommu_domain_ops) {
+		.set_dev		= intel_blocking_domain_set_dev,
+		.blocking_domain_detach = true,
 	}
 };
 
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index b361a4cff688..72982d1277c2 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -867,6 +867,14 @@ static struct iommu_group *ipmmu_find_group(struct device *dev)
 	return group;
 }
 
+static int ipmmu_blocking_domain_set_dev(struct iommu_domain *domain,
+					 struct device *dev)
+{
+	ipmmu_detach_device(domain, dev);
+
+	return 0;
+}
+
 static const struct iommu_ops ipmmu_ops = {
 	.domain_alloc = ipmmu_domain_alloc,
 	.probe_device = ipmmu_probe_device,
@@ -885,6 +893,10 @@ static const struct iommu_ops ipmmu_ops = {
 		.iotlb_sync	= ipmmu_iotlb_sync,
 		.iova_to_phys	= ipmmu_iova_to_phys,
 		.free		= ipmmu_domain_free,
+	},
+	.blocking_domain_ops = &(const struct iommu_domain_ops) {
+		.set_dev	= ipmmu_blocking_domain_set_dev,
+		.blocking_domain_detach = true,
 	}
 };
 
diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index 569d36840b67..b0471d03db60 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -674,6 +674,14 @@ irqreturn_t msm_iommu_fault_handler(int irq, void *dev_id)
 	return 0;
 }
 
+static int msm_blocking_domain_set_dev(struct iommu_domain *domain,
+				       struct device *dev)
+{
+	msm_iommu_detach_dev(domain, dev);
+
+	return 0;
+}
+
 static struct iommu_ops msm_iommu_ops = {
 	.domain_alloc = msm_iommu_domain_alloc,
 	.probe_device = msm_iommu_probe_device,
@@ -696,6 +704,10 @@ static struct iommu_ops msm_iommu_ops = {
 		.iotlb_sync_map	= msm_iommu_sync_map,
 		.iova_to_phys	= msm_iommu_iova_to_phys,
 		.free		= msm_iommu_domain_free,
+	},
+	.blocking_domain_ops = &(const struct iommu_domain_ops) {
+		.set_dev	= msm_blocking_domain_set_dev,
+		.blocking_domain_detach = true,
 	}
 };
 
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 33ec401d40eb..cc8d80290498 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -926,6 +926,14 @@ static void mtk_iommu_get_resv_regions(struct device *dev,
 	}
 }
 
+static int mtk_blocking_domain_set_dev(struct iommu_domain *domain,
+				       struct device *dev)
+{
+	mtk_iommu_detach_device(domain, dev);
+
+	return 0;
+}
+
 static const struct iommu_ops mtk_iommu_ops = {
 	.domain_alloc	= mtk_iommu_domain_alloc,
 	.probe_device	= mtk_iommu_probe_device,
@@ -946,6 +954,10 @@ static const struct iommu_ops mtk_iommu_ops = {
 		.iotlb_sync_map	= mtk_iommu_sync_map,
 		.iova_to_phys	= mtk_iommu_iova_to_phys,
 		.free		= mtk_iommu_domain_free,
+	},
+	.blocking_domain_ops = &(const struct iommu_domain_ops) {
+		.set_dev	= mtk_blocking_domain_set_dev,
+		.blocking_domain_detach = true,
 	}
 };
 
diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
index fb55802fb841..894d2526ba4c 100644
--- a/drivers/iommu/mtk_iommu_v1.c
+++ b/drivers/iommu/mtk_iommu_v1.c
@@ -584,6 +584,14 @@ static int mtk_iommu_v1_hw_init(const struct mtk_iommu_v1_data *data)
 	return 0;
 }
 
+static int mtk_blocking_domain_set_dev(struct iommu_domain *domain,
+				       struct device *dev)
+{
+	mtk_iommu_v1_detach_device(domain, dev);
+
+	return 0;
+}
+
 static const struct iommu_ops mtk_iommu_v1_ops = {
 	.domain_alloc	= mtk_iommu_v1_domain_alloc,
 	.probe_device	= mtk_iommu_v1_probe_device,
@@ -600,6 +608,10 @@ static const struct iommu_ops mtk_iommu_v1_ops = {
 		.unmap		= mtk_iommu_v1_unmap,
 		.iova_to_phys	= mtk_iommu_v1_iova_to_phys,
 		.free		= mtk_iommu_v1_domain_free,
+	},
+	.blocking_domain_ops = &(const struct iommu_domain_ops) {
+		.set_dev	= mtk_blocking_domain_set_dev,
+		.blocking_domain_detach = true,
 	}
 };
 
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 6720dcb437a0..7e6ba6f1218d 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -1732,6 +1732,14 @@ static struct iommu_group *omap_iommu_device_group(struct device *dev)
 	return group;
 }
 
+static int omap_blocking_domain_set_dev(struct iommu_domain *domain,
+					struct device *dev)
+{
+	omap_iommu_detach_dev(domain, dev);
+
+	return 0;
+}
+
 static const struct iommu_ops omap_iommu_ops = {
 	.domain_alloc	= omap_iommu_domain_alloc,
 	.probe_device	= omap_iommu_probe_device,
@@ -1745,6 +1753,10 @@ static const struct iommu_ops omap_iommu_ops = {
 		.unmap		= omap_iommu_unmap,
 		.iova_to_phys	= omap_iommu_iova_to_phys,
 		.free		= omap_iommu_domain_free,
+	},
+	.blocking_domain_ops = &(const struct iommu_domain_ops) {
+		.set_dev	= omap_blocking_domain_set_dev,
+		.blocking_domain_detach = true,
 	}
 };
 
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 0a4196c34179..29c803759a0b 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -1185,6 +1185,14 @@ static int rk_iommu_of_xlate(struct device *dev,
 	return 0;
 }
 
+static int rk_blocking_domain_set_dev(struct iommu_domain *domain,
+				      struct device *dev)
+{
+	rk_iommu_detach_device(domain, dev);
+
+	return 0;
+}
+
 static const struct iommu_ops rk_iommu_ops = {
 	.domain_alloc = rk_iommu_domain_alloc,
 	.probe_device = rk_iommu_probe_device,
@@ -1199,6 +1207,10 @@ static const struct iommu_ops rk_iommu_ops = {
 		.unmap		= rk_iommu_unmap,
 		.iova_to_phys	= rk_iommu_iova_to_phys,
 		.free		= rk_iommu_domain_free,
+	},
+	.blocking_domain_ops = &(const struct iommu_domain_ops) {
+		.set_dev	= rk_blocking_domain_set_dev,
+		.blocking_domain_detach = true,
 	}
 };
 
diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c
index cde01c72f573..a03bff562a44 100644
--- a/drivers/iommu/s390-iommu.c
+++ b/drivers/iommu/s390-iommu.c
@@ -360,6 +360,14 @@ void zpci_destroy_iommu(struct zpci_dev *zdev)
 	iommu_device_sysfs_remove(&zdev->iommu_dev);
 }
 
+static int s390_blocking_domain_set_dev(struct iommu_domain *domain,
+					struct device *dev)
+{
+	s390_iommu_detach_device(domain, dev);
+
+	return 0;
+}
+
 static const struct iommu_ops s390_iommu_ops = {
 	.capable = s390_iommu_capable,
 	.domain_alloc = s390_domain_alloc,
@@ -374,6 +382,10 @@ static const struct iommu_ops s390_iommu_ops = {
 		.unmap		= s390_iommu_unmap,
 		.iova_to_phys	= s390_iommu_iova_to_phys,
 		.free		= s390_domain_free,
+	},
+	.blocking_domain_ops = &(const struct iommu_domain_ops) {
+		.set_dev		= s390_blocking_domain_set_dev,
+		.blocking_domain_detach = true,
 	}
 };
 
diff --git a/drivers/iommu/sprd-iommu.c b/drivers/iommu/sprd-iommu.c
index 45b58845f5f8..ab62063cee5b 100644
--- a/drivers/iommu/sprd-iommu.c
+++ b/drivers/iommu/sprd-iommu.c
@@ -413,6 +413,13 @@ static int sprd_iommu_of_xlate(struct device *dev, struct of_phandle_args *args)
 	return 0;
 }
 
+static int sprd_blocking_domain_set_dev(struct iommu_domain *domain,
+					struct device *dev)
+{
+	sprd_iommu_detach_device(domain, dev);
+
+	return 0;
+}
 
 static const struct iommu_ops sprd_iommu_ops = {
 	.domain_alloc	= sprd_iommu_domain_alloc,
@@ -431,6 +438,10 @@ static const struct iommu_ops sprd_iommu_ops = {
 		.iotlb_sync	= sprd_iommu_sync,
 		.iova_to_phys	= sprd_iommu_iova_to_phys,
 		.free		= sprd_iommu_domain_free,
+	},
+	.blocking_domain_ops = &(const struct iommu_domain_ops) {
+		.set_dev		= sprd_blocking_domain_set_dev,
+		.blocking_domain_detach = true,
 	}
 };
 
diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
index 041b30463552..a51a55ae0634 100644
--- a/drivers/iommu/sun50i-iommu.c
+++ b/drivers/iommu/sun50i-iommu.c
@@ -758,6 +758,14 @@ static int sun50i_iommu_of_xlate(struct device *dev,
 	return iommu_fwspec_add_ids(dev, &id, 1);
 }
 
+static int sun50i_blocking_domain_set_dev(struct iommu_domain *domain,
+					  struct device *dev)
+{
+	sun50i_iommu_detach_device(domain, dev);
+
+	return 0;
+}
+
 static const struct iommu_ops sun50i_iommu_ops = {
 	.pgsize_bitmap	= SZ_4K,
 	.device_group	= sun50i_iommu_device_group,
@@ -774,6 +782,10 @@ static const struct iommu_ops sun50i_iommu_ops = {
 		.map		= sun50i_iommu_map,
 		.unmap		= sun50i_iommu_unmap,
 		.free		= sun50i_iommu_domain_free,
+	},
+	.blocking_domain_ops = &(const struct iommu_domain_ops) {
+		.set_dev		= sun50i_blocking_domain_set_dev,
+		.blocking_domain_detach = true,
 	}
 };
 
diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c
index f083a9fba4d0..7e2aef7ae72e 100644
--- a/drivers/iommu/tegra-gart.c
+++ b/drivers/iommu/tegra-gart.c
@@ -270,6 +270,14 @@ static void gart_iommu_sync(struct iommu_domain *domain,
 	gart_iommu_sync_map(domain, gather->start, length);
 }
 
+static int gart_blocking_domain_set_dev(struct iommu_domain *domain,
+					struct device *dev)
+{
+	gart_iommu_detach_dev(domain, dev);
+
+	return 0;
+}
+
 static const struct iommu_ops gart_iommu_ops = {
 	.domain_alloc	= gart_iommu_domain_alloc,
 	.probe_device	= gart_iommu_probe_device,
@@ -286,6 +294,10 @@ static const struct iommu_ops gart_iommu_ops = {
 		.iotlb_sync_map	= gart_iommu_sync_map,
 		.iotlb_sync	= gart_iommu_sync,
 		.free		= gart_iommu_domain_free,
+	},
+	.blocking_domain_ops = &(const struct iommu_domain_ops) {
+		.set_dev		= gart_blocking_domain_set_dev,
+		.blocking_domain_detach = true,
 	}
 };
 
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 36e9c2864e3f..fe2c463db230 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -963,6 +963,14 @@ static int tegra_smmu_of_xlate(struct device *dev,
 	return iommu_fwspec_add_ids(dev, &id, 1);
 }
 
+static int tegra_smmu_blocking_domain_set_dev(struct iommu_domain *domain,
+					      struct device *dev)
+{
+	tegra_smmu_detach_dev(domain, dev);
+
+	return 0;
+}
+
 static const struct iommu_ops tegra_smmu_ops = {
 	.domain_alloc = tegra_smmu_domain_alloc,
 	.probe_device = tegra_smmu_probe_device,
@@ -977,6 +985,10 @@ static const struct iommu_ops tegra_smmu_ops = {
 		.unmap		= tegra_smmu_unmap,
 		.iova_to_phys	= tegra_smmu_iova_to_phys,
 		.free		= tegra_smmu_domain_free,
+	},
+	.blocking_domain_ops = &(const struct iommu_domain_ops) {
+		.set_dev		= tegra_smmu_blocking_domain_set_dev,
+		.blocking_domain_detach = true,
 	}
 };
 
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index ce2bd01806d8..5054ebaf9654 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -1023,6 +1023,9 @@ static struct iommu_ops viommu_ops = {
 		.iova_to_phys		= viommu_iova_to_phys,
 		.iotlb_sync		= viommu_iotlb_sync,
 		.free			= viommu_domain_free,
+	},
+	.blocking_domain_ops = &(const struct iommu_domain_ops) {
+		.set_dev		= viommu_attach_dev,
 	}
 };
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 3/5] iommu: Make blocking domain static for iommu group
  2022-05-16  1:57 [PATCH 0/5] iommu: Make blocking domain static for group Lu Baolu
  2022-05-16  1:57 ` [PATCH 1/5] iommu: Rename attach_dev to set_dev in domain ops Lu Baolu
  2022-05-16  1:57 ` [PATCH 2/5] iommu: Add blocking_domain_ops field in iommu_ops Lu Baolu
@ 2022-05-16  1:57 ` Lu Baolu
  2022-05-16  1:57 ` [PATCH 4/5] iommu: Use blocking domain for empty domain attaching Lu Baolu
  2022-05-16  1:57 ` [PATCH 5/5] iommu: Remove .detach_dev from iommu domain ops Lu Baolu
  4 siblings, 0 replies; 18+ messages in thread
From: Lu Baolu @ 2022-05-16  1:57 UTC (permalink / raw)
  To: Joerg Roedel, Jason Gunthorpe, Christoph Hellwig, Kevin Tian,
	Ashok Raj, Will Deacon, Robin Murphy, Jean-Philippe Brucker
  Cc: Eric Auger, Liu Yi L, Jacob jun Pan, iommu, linux-kernel, Lu Baolu

This makes the blocking_domain static for an iommu group. It's allocated
when the first device joins the group and freed after the last device
leaves. Essentially the blocking domain is a dummy domain used to remove
the domain from IOMMU's device context. Unfortunately, some IOMMU devices
don't provide such capability. In this case, we use an UNMANAGED domain
without any mapping instead.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/iommu.c | 85 +++++++++++++++++++++++++++----------------
 1 file changed, 54 insertions(+), 31 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 8eba26be4363..dcbc55c9d8d7 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -604,8 +604,6 @@ static void iommu_group_release(struct kobject *kobj)
 
 	if (group->default_domain)
 		iommu_domain_free(group->default_domain);
-	if (group->blocking_domain)
-		iommu_domain_free(group->blocking_domain);
 
 	kfree(group->name);
 	kfree(group);
@@ -854,6 +852,46 @@ static bool iommu_is_attach_deferred(struct device *dev)
 	return false;
 }
 
+static int iommu_group_alloc_blocking_domain(struct iommu_group *group,
+					     struct device *dev)
+{
+	struct iommu_domain *domain;
+	const struct iommu_ops *iommu_ops = dev_iommu_ops(dev);
+	const struct iommu_domain_ops *ops = iommu_ops->blocking_domain_ops;
+
+	if (group->blocking_domain)
+		return 0;
+
+	if (ops->blocking_domain_detach) {
+		domain = kzalloc(sizeof(*domain), GFP_KERNEL);
+		if (domain)
+			domain->type = IOMMU_DOMAIN_BLOCKED;
+	} else {
+		domain = __iommu_domain_alloc(dev->bus, IOMMU_DOMAIN_UNMANAGED);
+	}
+
+	if (!domain)
+		return -ENOMEM;
+
+	domain->ops = ops;
+	group->blocking_domain = domain;
+
+	return 0;
+}
+
+static void iommu_group_free_blocking_domain(struct iommu_group *group,
+					     struct device *dev)
+{
+	struct iommu_domain *domain = group->blocking_domain;
+
+	if (domain->type == IOMMU_DOMAIN_BLOCKED)
+		kfree(domain);
+	else
+		iommu_domain_free(domain);
+
+	group->blocking_domain = NULL;
+}
+
 /**
  * iommu_group_add_device - add a device to an iommu group
  * @group: the group into which to add the device (reference should be held)
@@ -867,6 +905,12 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev)
 	int ret, i = 0;
 	struct group_device *device;
 
+	mutex_lock(&group->mutex);
+	ret = iommu_group_alloc_blocking_domain(group, dev);
+	mutex_unlock(&group->mutex);
+	if (ret)
+		return -ENODEV;
+
 	device = kzalloc(sizeof(*device), GFP_KERNEL);
 	if (!device)
 		return -ENOMEM;
@@ -961,6 +1005,8 @@ void iommu_group_remove_device(struct device *dev)
 			break;
 		}
 	}
+	if (list_empty(&group->devices))
+		iommu_group_free_blocking_domain(group, dev);
 	mutex_unlock(&group->mutex);
 
 	if (!device)
@@ -1961,12 +2007,16 @@ static void __iommu_group_set_core_domain(struct iommu_group *group)
 static int __iommu_attach_device(struct iommu_domain *domain,
 				 struct device *dev)
 {
+	const struct iommu_domain_ops *ops = domain->ops;
 	int ret;
 
-	if (unlikely(domain->ops->set_dev == NULL))
+	if (unlikely(ops->set_dev == NULL))
 		return -ENODEV;
 
-	ret = domain->ops->set_dev(domain, dev);
+	if (domain->type == IOMMU_DOMAIN_BLOCKED)
+		domain = iommu_get_domain_for_dev(dev);
+
+	ret = ops->set_dev(domain, dev);
 	if (!ret)
 		trace_attach_device_to_domain(dev);
 	return ret;
@@ -3146,29 +3196,6 @@ void iommu_device_unuse_default_domain(struct device *dev)
 	iommu_group_put(group);
 }
 
-static int __iommu_group_alloc_blocking_domain(struct iommu_group *group)
-{
-	struct group_device *dev =
-		list_first_entry(&group->devices, struct group_device, list);
-
-	if (group->blocking_domain)
-		return 0;
-
-	group->blocking_domain =
-		__iommu_domain_alloc(dev->dev->bus, IOMMU_DOMAIN_BLOCKED);
-	if (!group->blocking_domain) {
-		/*
-		 * For drivers that do not yet understand IOMMU_DOMAIN_BLOCKED
-		 * create an empty domain instead.
-		 */
-		group->blocking_domain = __iommu_domain_alloc(
-			dev->dev->bus, IOMMU_DOMAIN_UNMANAGED);
-		if (!group->blocking_domain)
-			return -EINVAL;
-	}
-	return 0;
-}
-
 /**
  * iommu_group_claim_dma_owner() - Set DMA ownership of a group
  * @group: The group.
@@ -3192,10 +3219,6 @@ int iommu_group_claim_dma_owner(struct iommu_group *group, void *owner)
 			goto unlock_out;
 		}
 
-		ret = __iommu_group_alloc_blocking_domain(group);
-		if (ret)
-			goto unlock_out;
-
 		ret = __iommu_group_set_domain(group, group->blocking_domain);
 		if (ret)
 			goto unlock_out;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 4/5] iommu: Use blocking domain for empty domain attaching
  2022-05-16  1:57 [PATCH 0/5] iommu: Make blocking domain static for group Lu Baolu
                   ` (2 preceding siblings ...)
  2022-05-16  1:57 ` [PATCH 3/5] iommu: Make blocking domain static for iommu group Lu Baolu
@ 2022-05-16  1:57 ` Lu Baolu
  2022-05-16  1:57 ` [PATCH 5/5] iommu: Remove .detach_dev from iommu domain ops Lu Baolu
  4 siblings, 0 replies; 18+ messages in thread
From: Lu Baolu @ 2022-05-16  1:57 UTC (permalink / raw)
  To: Joerg Roedel, Jason Gunthorpe, Christoph Hellwig, Kevin Tian,
	Ashok Raj, Will Deacon, Robin Murphy, Jean-Philippe Brucker
  Cc: Eric Auger, Liu Yi L, Jacob jun Pan, iommu, linux-kernel, Lu Baolu

If a NULL domain is about to set to a device, let's set the blocking
domain instead.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/iommu.c | 35 +++--------------------------------
 1 file changed, 3 insertions(+), 32 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index dcbc55c9d8d7..ba0f427c2823 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2058,16 +2058,6 @@ int iommu_deferred_attach(struct device *dev, struct iommu_domain *domain)
 	return 0;
 }
 
-static void __iommu_detach_device(struct iommu_domain *domain,
-				  struct device *dev)
-{
-	if (iommu_is_attach_deferred(dev))
-		return;
-
-	domain->ops->detach_dev(domain, dev);
-	trace_detach_device_from_domain(dev);
-}
-
 void iommu_detach_device(struct iommu_domain *domain, struct device *dev)
 {
 	struct iommu_group *group;
@@ -2160,15 +2150,6 @@ int iommu_attach_group(struct iommu_domain *domain, struct iommu_group *group)
 }
 EXPORT_SYMBOL_GPL(iommu_attach_group);
 
-static int iommu_group_do_detach_device(struct device *dev, void *data)
-{
-	struct iommu_domain *domain = data;
-
-	__iommu_detach_device(domain, dev);
-
-	return 0;
-}
-
 static int __iommu_group_set_domain(struct iommu_group *group,
 				    struct iommu_domain *new_domain)
 {
@@ -2177,19 +2158,9 @@ static int __iommu_group_set_domain(struct iommu_group *group,
 	if (group->domain == new_domain)
 		return 0;
 
-	/*
-	 * New drivers should support default domains and so the detach_dev() op
-	 * will never be called. Otherwise the NULL domain represents some
-	 * platform specific behavior.
-	 */
-	if (!new_domain) {
-		if (WARN_ON(!group->domain->ops->detach_dev))
-			return -EINVAL;
-		__iommu_group_for_each_dev(group, group->domain,
-					   iommu_group_do_detach_device);
-		group->domain = NULL;
-		return 0;
-	}
+	/* The NULL domain represents some platform specific behavior. */
+	if (!new_domain)
+		new_domain = group->blocking_domain;
 
 	/*
 	 * Changing the domain is done by calling set_dev() on the new
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 5/5] iommu: Remove .detach_dev from iommu domain ops
  2022-05-16  1:57 [PATCH 0/5] iommu: Make blocking domain static for group Lu Baolu
                   ` (3 preceding siblings ...)
  2022-05-16  1:57 ` [PATCH 4/5] iommu: Use blocking domain for empty domain attaching Lu Baolu
@ 2022-05-16  1:57 ` Lu Baolu
  4 siblings, 0 replies; 18+ messages in thread
From: Lu Baolu @ 2022-05-16  1:57 UTC (permalink / raw)
  To: Joerg Roedel, Jason Gunthorpe, Christoph Hellwig, Kevin Tian,
	Ashok Raj, Will Deacon, Robin Murphy, Jean-Philippe Brucker
  Cc: Eric Auger, Liu Yi L, Jacob jun Pan, iommu, linux-kernel, Lu Baolu

The .detach_dev callback is not used anymore. Reomve it to avoid dead
code.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 include/linux/iommu.h                   | 2 --
 include/trace/events/iommu.h            | 7 -------
 drivers/iommu/amd/iommu.c               | 1 -
 drivers/iommu/apple-dart.c              | 1 -
 drivers/iommu/arm/arm-smmu/qcom_iommu.c | 1 -
 drivers/iommu/exynos-iommu.c            | 1 -
 drivers/iommu/fsl_pamu_domain.c         | 1 -
 drivers/iommu/intel/iommu.c             | 1 -
 drivers/iommu/iommu-traces.c            | 1 -
 drivers/iommu/ipmmu-vmsa.c              | 1 -
 drivers/iommu/msm_iommu.c               | 1 -
 drivers/iommu/mtk_iommu.c               | 1 -
 drivers/iommu/mtk_iommu_v1.c            | 1 -
 drivers/iommu/omap-iommu.c              | 1 -
 drivers/iommu/rockchip-iommu.c          | 1 -
 drivers/iommu/s390-iommu.c              | 1 -
 drivers/iommu/sprd-iommu.c              | 1 -
 drivers/iommu/sun50i-iommu.c            | 1 -
 drivers/iommu/tegra-gart.c              | 1 -
 drivers/iommu/tegra-smmu.c              | 1 -
 20 files changed, 27 deletions(-)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 5e228aad0ef6..a8c87ebef02d 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -264,7 +264,6 @@ struct iommu_ops {
 /**
  * struct iommu_domain_ops - domain specific operations
  * @set_dev: set an iommu domain to a device
- * @detach_dev: detach an iommu domain from a device
  * @map: map a physically contiguous memory region to an iommu domain
  * @map_pages: map a physically contiguous set of pages of the same size to
  *             an iommu domain.
@@ -287,7 +286,6 @@ struct iommu_ops {
  */
 struct iommu_domain_ops {
 	int (*set_dev)(struct iommu_domain *domain, struct device *dev);
-	void (*detach_dev)(struct iommu_domain *domain, struct device *dev);
 
 	int (*map)(struct iommu_domain *domain, unsigned long iova,
 		   phys_addr_t paddr, size_t size, int prot, gfp_t gfp);
diff --git a/include/trace/events/iommu.h b/include/trace/events/iommu.h
index 29096fe12623..70743db1fb75 100644
--- a/include/trace/events/iommu.h
+++ b/include/trace/events/iommu.h
@@ -76,13 +76,6 @@ DEFINE_EVENT(iommu_device_event, attach_device_to_domain,
 	TP_ARGS(dev)
 );
 
-DEFINE_EVENT(iommu_device_event, detach_device_from_domain,
-
-	TP_PROTO(struct device *dev),
-
-	TP_ARGS(dev)
-);
-
 TRACE_EVENT(map,
 
 	TP_PROTO(unsigned long iova, phys_addr_t paddr, size_t size),
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index c66713439824..9ccf289196be 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2294,7 +2294,6 @@ const struct iommu_ops amd_iommu_ops = {
 	.def_domain_type = amd_iommu_def_domain_type,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.set_dev	= amd_iommu_attach_device,
-		.detach_dev	= amd_iommu_detach_device,
 		.map		= amd_iommu_map,
 		.unmap		= amd_iommu_unmap,
 		.iotlb_sync_map	= amd_iommu_iotlb_sync_map,
diff --git a/drivers/iommu/apple-dart.c b/drivers/iommu/apple-dart.c
index 3c37762e01ec..640f2ebeba0c 100644
--- a/drivers/iommu/apple-dart.c
+++ b/drivers/iommu/apple-dart.c
@@ -784,7 +784,6 @@ static const struct iommu_ops apple_dart_iommu_ops = {
 	.owner = THIS_MODULE,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.set_dev	= apple_dart_attach_dev,
-		.detach_dev	= apple_dart_detach_dev,
 		.map_pages	= apple_dart_map_pages,
 		.unmap_pages	= apple_dart_unmap_pages,
 		.flush_iotlb_all = apple_dart_flush_iotlb_all,
diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
index dee9b5a3a324..b6400458bcdb 100644
--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -605,7 +605,6 @@ static const struct iommu_ops qcom_iommu_ops = {
 	.pgsize_bitmap	= SZ_4K | SZ_64K | SZ_1M | SZ_16M,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.set_dev	= qcom_iommu_attach_dev,
-		.detach_dev	= qcom_iommu_detach_dev,
 		.map		= qcom_iommu_map,
 		.unmap		= qcom_iommu_unmap,
 		.flush_iotlb_all = qcom_iommu_flush_iotlb_all,
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index bbecb9a2a554..0ed3ac3f6b28 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1324,7 +1324,6 @@ static const struct iommu_ops exynos_iommu_ops = {
 	.of_xlate = exynos_iommu_of_xlate,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.set_dev	= exynos_iommu_attach_device,
-		.detach_dev	= exynos_iommu_detach_device,
 		.map		= exynos_iommu_map,
 		.unmap		= exynos_iommu_unmap,
 		.iova_to_phys	= exynos_iommu_iova_to_phys,
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 92fc320f8c83..8e32d7942835 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -467,7 +467,6 @@ static const struct iommu_ops fsl_pamu_ops = {
 	.device_group   = fsl_pamu_device_group,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.set_dev	= fsl_pamu_attach_device,
-		.detach_dev	= fsl_pamu_detach_device,
 		.iova_to_phys	= fsl_pamu_iova_to_phys,
 		.free		= fsl_pamu_domain_free,
 	},
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 2060e8a540b3..9b6b3d38de46 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4934,7 +4934,6 @@ const struct iommu_ops intel_iommu_ops = {
 #endif
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.set_dev		= intel_iommu_attach_device,
-		.detach_dev		= intel_iommu_detach_device,
 		.map_pages		= intel_iommu_map_pages,
 		.unmap_pages		= intel_iommu_unmap_pages,
 		.iotlb_sync_map		= intel_iommu_iotlb_sync_map,
diff --git a/drivers/iommu/iommu-traces.c b/drivers/iommu/iommu-traces.c
index 1e9ca7789de1..23416bf76df9 100644
--- a/drivers/iommu/iommu-traces.c
+++ b/drivers/iommu/iommu-traces.c
@@ -18,7 +18,6 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(remove_device_from_group);
 
 /* iommu_device_event */
 EXPORT_TRACEPOINT_SYMBOL_GPL(attach_device_to_domain);
-EXPORT_TRACEPOINT_SYMBOL_GPL(detach_device_from_domain);
 
 /* iommu_map_unmap */
 EXPORT_TRACEPOINT_SYMBOL_GPL(map);
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 72982d1277c2..5803f6175a46 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -886,7 +886,6 @@ static const struct iommu_ops ipmmu_ops = {
 	.of_xlate = ipmmu_of_xlate,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.set_dev	= ipmmu_attach_device,
-		.detach_dev	= ipmmu_detach_device,
 		.map		= ipmmu_map,
 		.unmap		= ipmmu_unmap,
 		.flush_iotlb_all = ipmmu_flush_iotlb_all,
diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index b0471d03db60..3f3ce24e1db6 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -691,7 +691,6 @@ static struct iommu_ops msm_iommu_ops = {
 	.of_xlate = qcom_iommu_of_xlate,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.set_dev	= msm_iommu_attach_dev,
-		.detach_dev	= msm_iommu_detach_dev,
 		.map		= msm_iommu_map,
 		.unmap		= msm_iommu_unmap,
 		/*
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index cc8d80290498..63cb4379314c 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -946,7 +946,6 @@ static const struct iommu_ops mtk_iommu_ops = {
 	.owner		= THIS_MODULE,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.set_dev	= mtk_iommu_attach_device,
-		.detach_dev	= mtk_iommu_detach_device,
 		.map		= mtk_iommu_map,
 		.unmap		= mtk_iommu_unmap,
 		.flush_iotlb_all = mtk_iommu_flush_iotlb_all,
diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
index 894d2526ba4c..3ffaadcf9a30 100644
--- a/drivers/iommu/mtk_iommu_v1.c
+++ b/drivers/iommu/mtk_iommu_v1.c
@@ -603,7 +603,6 @@ static const struct iommu_ops mtk_iommu_v1_ops = {
 	.owner          = THIS_MODULE,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.set_dev	= mtk_iommu_v1_attach_device,
-		.detach_dev	= mtk_iommu_v1_detach_device,
 		.map		= mtk_iommu_v1_map,
 		.unmap		= mtk_iommu_v1_unmap,
 		.iova_to_phys	= mtk_iommu_v1_iova_to_phys,
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 7e6ba6f1218d..33ba73be932a 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -1748,7 +1748,6 @@ static const struct iommu_ops omap_iommu_ops = {
 	.pgsize_bitmap	= OMAP_IOMMU_PGSIZES,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.set_dev	= omap_iommu_attach_dev,
-		.detach_dev	= omap_iommu_detach_dev,
 		.map		= omap_iommu_map,
 		.unmap		= omap_iommu_unmap,
 		.iova_to_phys	= omap_iommu_iova_to_phys,
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 29c803759a0b..3d3baad1bc36 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -1202,7 +1202,6 @@ static const struct iommu_ops rk_iommu_ops = {
 	.of_xlate = rk_iommu_of_xlate,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.set_dev	= rk_iommu_attach_device,
-		.detach_dev	= rk_iommu_detach_device,
 		.map		= rk_iommu_map,
 		.unmap		= rk_iommu_unmap,
 		.iova_to_phys	= rk_iommu_iova_to_phys,
diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c
index a03bff562a44..e9ad001d086b 100644
--- a/drivers/iommu/s390-iommu.c
+++ b/drivers/iommu/s390-iommu.c
@@ -377,7 +377,6 @@ static const struct iommu_ops s390_iommu_ops = {
 	.pgsize_bitmap = S390_IOMMU_PGSIZES,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.set_dev	= s390_iommu_attach_device,
-		.detach_dev	= s390_iommu_detach_device,
 		.map		= s390_iommu_map,
 		.unmap		= s390_iommu_unmap,
 		.iova_to_phys	= s390_iommu_iova_to_phys,
diff --git a/drivers/iommu/sprd-iommu.c b/drivers/iommu/sprd-iommu.c
index ab62063cee5b..b732cd894725 100644
--- a/drivers/iommu/sprd-iommu.c
+++ b/drivers/iommu/sprd-iommu.c
@@ -431,7 +431,6 @@ static const struct iommu_ops sprd_iommu_ops = {
 	.owner		= THIS_MODULE,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.set_dev	= sprd_iommu_attach_device,
-		.detach_dev	= sprd_iommu_detach_device,
 		.map		= sprd_iommu_map,
 		.unmap		= sprd_iommu_unmap,
 		.iotlb_sync_map	= sprd_iommu_sync_map,
diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
index a51a55ae0634..12da8e4fdef9 100644
--- a/drivers/iommu/sun50i-iommu.c
+++ b/drivers/iommu/sun50i-iommu.c
@@ -775,7 +775,6 @@ static const struct iommu_ops sun50i_iommu_ops = {
 	.release_device	= sun50i_iommu_release_device,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.set_dev	= sun50i_iommu_attach_device,
-		.detach_dev	= sun50i_iommu_detach_device,
 		.flush_iotlb_all = sun50i_iommu_flush_iotlb_all,
 		.iotlb_sync	= sun50i_iommu_iotlb_sync,
 		.iova_to_phys	= sun50i_iommu_iova_to_phys,
diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c
index 7e2aef7ae72e..152a04dd5753 100644
--- a/drivers/iommu/tegra-gart.c
+++ b/drivers/iommu/tegra-gart.c
@@ -287,7 +287,6 @@ static const struct iommu_ops gart_iommu_ops = {
 	.of_xlate	= gart_iommu_of_xlate,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.set_dev	= gart_iommu_attach_dev,
-		.detach_dev	= gart_iommu_detach_dev,
 		.map		= gart_iommu_map,
 		.unmap		= gart_iommu_unmap,
 		.iova_to_phys	= gart_iommu_iova_to_phys,
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index fe2c463db230..f35ee4d07a33 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -980,7 +980,6 @@ static const struct iommu_ops tegra_smmu_ops = {
 	.pgsize_bitmap = SZ_4K,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.set_dev	= tegra_smmu_attach_dev,
-		.detach_dev	= tegra_smmu_detach_dev,
 		.map		= tegra_smmu_map,
 		.unmap		= tegra_smmu_unmap,
 		.iova_to_phys	= tegra_smmu_iova_to_phys,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] iommu: Add blocking_domain_ops field in iommu_ops
  2022-05-16  1:57 ` [PATCH 2/5] iommu: Add blocking_domain_ops field in iommu_ops Lu Baolu
@ 2022-05-16  7:27   ` Christoph Hellwig
  2022-05-16 13:05     ` Jason Gunthorpe
  2022-05-16 11:22   ` Robin Murphy
  2022-05-20  8:45   ` Joerg Roedel
  2 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2022-05-16  7:27 UTC (permalink / raw)
  To: Lu Baolu
  Cc: Joerg Roedel, Jason Gunthorpe, Christoph Hellwig, Kevin Tian,
	Ashok Raj, Will Deacon, Robin Murphy, Jean-Philippe Brucker,
	Eric Auger, Liu Yi L, Jacob jun Pan, iommu, linux-kernel

On Mon, May 16, 2022 at 09:57:56AM +0800, Lu Baolu wrote:
> Each IOMMU driver must provide a blocking domain ops. If the hardware
> supports detaching domain from device, setting blocking domain equals
> detaching the existing domain from the deivce. Otherwise, an UNMANAGED
> domain without any mapping will be used instead.

blocking in this case means not allowing any access?  The naming
sounds a bit odd to me as blocking in the kernel has a specific
meaning.  Maybe something like noaccess ops might be a better name?

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] iommu: Add blocking_domain_ops field in iommu_ops
  2022-05-16  1:57 ` [PATCH 2/5] iommu: Add blocking_domain_ops field in iommu_ops Lu Baolu
  2022-05-16  7:27   ` Christoph Hellwig
@ 2022-05-16 11:22   ` Robin Murphy
  2022-05-16 13:43     ` Baolu Lu
  2022-05-16 13:57     ` Jason Gunthorpe
  2022-05-20  8:45   ` Joerg Roedel
  2 siblings, 2 replies; 18+ messages in thread
From: Robin Murphy @ 2022-05-16 11:22 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel, Jason Gunthorpe, Christoph Hellwig,
	Kevin Tian, Ashok Raj, Will Deacon, Jean-Philippe Brucker
  Cc: Eric Auger, Liu Yi L, Jacob jun Pan, iommu, linux-kernel

On 2022-05-16 02:57, Lu Baolu wrote:
> Each IOMMU driver must provide a blocking domain ops. If the hardware
> supports detaching domain from device, setting blocking domain equals
> detaching the existing domain from the deivce. Otherwise, an UNMANAGED
> domain without any mapping will be used instead.

Unfortunately that's backwards - most of the implementations of 
.detach_dev are disabling translation entirely, meaning the device ends 
up effectively in passthrough rather than blocked. Conversely, at least 
arm-smmu and arm-smmu-v3 could implement IOMMU_DOMAIN_BLOCKED properly 
with fault-type S2CRs and STEs respectively, it just needs a bit of 
wiring up.

Thanks,
Robin.

> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>   include/linux/iommu.h                       |  7 +++++++
>   drivers/iommu/amd/iommu.c                   | 12 ++++++++++++
>   drivers/iommu/apple-dart.c                  | 12 ++++++++++++
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  3 +++
>   drivers/iommu/arm/arm-smmu/arm-smmu.c       |  3 +++
>   drivers/iommu/arm/arm-smmu/qcom_iommu.c     | 12 ++++++++++++
>   drivers/iommu/exynos-iommu.c                | 12 ++++++++++++
>   drivers/iommu/fsl_pamu_domain.c             | 12 ++++++++++++
>   drivers/iommu/intel/iommu.c                 | 12 ++++++++++++
>   drivers/iommu/ipmmu-vmsa.c                  | 12 ++++++++++++
>   drivers/iommu/msm_iommu.c                   | 12 ++++++++++++
>   drivers/iommu/mtk_iommu.c                   | 12 ++++++++++++
>   drivers/iommu/mtk_iommu_v1.c                | 12 ++++++++++++
>   drivers/iommu/omap-iommu.c                  | 12 ++++++++++++
>   drivers/iommu/rockchip-iommu.c              | 12 ++++++++++++
>   drivers/iommu/s390-iommu.c                  | 12 ++++++++++++
>   drivers/iommu/sprd-iommu.c                  | 11 +++++++++++
>   drivers/iommu/sun50i-iommu.c                | 12 ++++++++++++
>   drivers/iommu/tegra-gart.c                  | 12 ++++++++++++
>   drivers/iommu/tegra-smmu.c                  | 12 ++++++++++++
>   drivers/iommu/virtio-iommu.c                |  3 +++
>   21 files changed, 219 insertions(+)
> 
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 572399ac1d83..5e228aad0ef6 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -216,6 +216,7 @@ struct iommu_iotlb_gather {
>    *		- IOMMU_DOMAIN_DMA: must use a dma domain
>    *		- 0: use the default setting
>    * @default_domain_ops: the default ops for domains
> + * @blocking_domain_ops: the blocking ops for domains
>    * @pgsize_bitmap: bitmap of all possible supported page sizes
>    * @owner: Driver module providing these ops
>    */
> @@ -255,6 +256,7 @@ struct iommu_ops {
>   	int (*def_domain_type)(struct device *dev);
>   
>   	const struct iommu_domain_ops *default_domain_ops;
> +	const struct iommu_domain_ops *blocking_domain_ops;
>   	unsigned long pgsize_bitmap;
>   	struct module *owner;
>   };
> @@ -279,6 +281,9 @@ struct iommu_ops {
>    * @enable_nesting: Enable nesting
>    * @set_pgtable_quirks: Set io page table quirks (IO_PGTABLE_QUIRK_*)
>    * @free: Release the domain after use.
> + * @blocking_domain_detach: iommu hardware support detaching a domain from
> + *		a device, hence setting blocking domain to a device equals to
> + *		detach the existing domain from it.
>    */
>   struct iommu_domain_ops {
>   	int (*set_dev)(struct iommu_domain *domain, struct device *dev);
> @@ -310,6 +315,8 @@ struct iommu_domain_ops {
>   				  unsigned long quirks);
>   
>   	void (*free)(struct iommu_domain *domain);
> +
> +	unsigned int blocking_domain_detach:1;
>   };
>   
>   /**
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 01b8668ef46d..c66713439824 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -2272,6 +2272,14 @@ static bool amd_iommu_enforce_cache_coherency(struct iommu_domain *domain)
>   	return true;
>   }
>   
> +static int amd_blocking_domain_set_dev(struct iommu_domain *domain,
> +				       struct device *dev)
> +{
> +	amd_iommu_detach_device(domain, dev);
> +
> +	return 0;
> +}
> +
>   const struct iommu_ops amd_iommu_ops = {
>   	.capable = amd_iommu_capable,
>   	.domain_alloc = amd_iommu_domain_alloc,
> @@ -2295,6 +2303,10 @@ const struct iommu_ops amd_iommu_ops = {
>   		.iotlb_sync	= amd_iommu_iotlb_sync,
>   		.free		= amd_iommu_domain_free,
>   		.enforce_cache_coherency = amd_iommu_enforce_cache_coherency,
> +	},
> +	.blocking_domain_ops = &(const struct iommu_domain_ops) {
> +		.set_dev	= amd_blocking_domain_set_dev,
> +		.blocking_domain_detach = true,
>   	}
>   };
>   
> diff --git a/drivers/iommu/apple-dart.c b/drivers/iommu/apple-dart.c
> index a0b7281f1989..3c37762e01ec 100644
> --- a/drivers/iommu/apple-dart.c
> +++ b/drivers/iommu/apple-dart.c
> @@ -763,6 +763,14 @@ static void apple_dart_get_resv_regions(struct device *dev,
>   	iommu_dma_get_resv_regions(dev, head);
>   }
>   
> +static int apple_dart_blocking_domain_set_dev(struct iommu_domain *domain,
> +					      struct device *dev)
> +{
> +	apple_dart_detach_dev(domain, dev);
> +
> +	return 0;
> +}
> +
>   static const struct iommu_ops apple_dart_iommu_ops = {
>   	.domain_alloc = apple_dart_domain_alloc,
>   	.probe_device = apple_dart_probe_device,
> @@ -784,6 +792,10 @@ static const struct iommu_ops apple_dart_iommu_ops = {
>   		.iotlb_sync_map	= apple_dart_iotlb_sync_map,
>   		.iova_to_phys	= apple_dart_iova_to_phys,
>   		.free		= apple_dart_domain_free,
> +	},
> +	.blocking_domain_ops = &(const struct iommu_domain_ops) {
> +		.set_dev	= apple_dart_blocking_domain_set_dev,
> +		.blocking_domain_detach = true,
>   	}
>   };
>   
> 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 7e7d9e0b7aee..4b0ec5bef63b 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2867,6 +2867,9 @@ static struct iommu_ops arm_smmu_ops = {
>   		.iova_to_phys		= arm_smmu_iova_to_phys,
>   		.enable_nesting		= arm_smmu_enable_nesting,
>   		.free			= arm_smmu_domain_free,
> +	},
> +	.blocking_domain_ops = &(const struct iommu_domain_ops) {
> +		.set_dev		= arm_smmu_attach_dev,
>   	}
>   };
>   
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index c91d12b7e283..0723f7c97763 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -1606,6 +1606,9 @@ static struct iommu_ops arm_smmu_ops = {
>   		.enable_nesting		= arm_smmu_enable_nesting,
>   		.set_pgtable_quirks	= arm_smmu_set_pgtable_quirks,
>   		.free			= arm_smmu_domain_free,
> +	},
> +	.blocking_domain_ops = &(const struct iommu_domain_ops) {
> +		.set_dev		= arm_smmu_attach_dev,
>   	}
>   };
>   
> diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> index cf624bd305e0..dee9b5a3a324 100644
> --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> @@ -587,6 +587,14 @@ static int qcom_iommu_of_xlate(struct device *dev, struct of_phandle_args *args)
>   	return iommu_fwspec_add_ids(dev, &asid, 1);
>   }
>   
> +static int qcom_blocking_domain_set_dev(struct iommu_domain *domain,
> +					struct device *dev)
> +{
> +	qcom_iommu_detach_dev(domain, dev);
> +
> +	return 0;
> +}
> +
>   static const struct iommu_ops qcom_iommu_ops = {
>   	.capable	= qcom_iommu_capable,
>   	.domain_alloc	= qcom_iommu_domain_alloc,
> @@ -604,6 +612,10 @@ static const struct iommu_ops qcom_iommu_ops = {
>   		.iotlb_sync	= qcom_iommu_iotlb_sync,
>   		.iova_to_phys	= qcom_iommu_iova_to_phys,
>   		.free		= qcom_iommu_domain_free,
> +	},
> +	.blocking_domain_ops = &(const struct iommu_domain_ops) {
> +		.set_dev	= qcom_blocking_domain_set_dev,
> +		.blocking_domain_detach = true,
>   	}
>   };
>   
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 797348f3440e..bbecb9a2a554 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -1307,6 +1307,14 @@ static int exynos_iommu_of_xlate(struct device *dev,
>   	return 0;
>   }
>   
> +static int exynos_blocking_domain_set_dev(struct iommu_domain *domain,
> +					  struct device *dev)
> +{
> +	exynos_iommu_detach_device(domain, dev);
> +
> +	return 0;
> +}
> +
>   static const struct iommu_ops exynos_iommu_ops = {
>   	.domain_alloc = exynos_iommu_domain_alloc,
>   	.device_group = generic_device_group,
> @@ -1321,6 +1329,10 @@ static const struct iommu_ops exynos_iommu_ops = {
>   		.unmap		= exynos_iommu_unmap,
>   		.iova_to_phys	= exynos_iommu_iova_to_phys,
>   		.free		= exynos_iommu_domain_free,
> +	},
> +	.blocking_domain_ops = &(const struct iommu_domain_ops) {
> +		.set_dev	= exynos_blocking_domain_set_dev,
> +		.blocking_domain_detach = true,
>   	}
>   };
>   
> diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
> index 7512c8e007e9..92fc320f8c83 100644
> --- a/drivers/iommu/fsl_pamu_domain.c
> +++ b/drivers/iommu/fsl_pamu_domain.c
> @@ -451,6 +451,14 @@ static void fsl_pamu_release_device(struct device *dev)
>   {
>   }
>   
> +static int fsl_pamu_blocking_domain_set_dev(struct iommu_domain *domain,
> +					    struct device *dev)
> +{
> +	fsl_pamu_detach_device(domain, dev);
> +
> +	return 0;
> +}
> +
>   static const struct iommu_ops fsl_pamu_ops = {
>   	.capable	= fsl_pamu_capable,
>   	.domain_alloc	= fsl_pamu_domain_alloc,
> @@ -462,6 +470,10 @@ static const struct iommu_ops fsl_pamu_ops = {
>   		.detach_dev	= fsl_pamu_detach_device,
>   		.iova_to_phys	= fsl_pamu_iova_to_phys,
>   		.free		= fsl_pamu_domain_free,
> +	},
> +	.blocking_domain_ops = &(const struct iommu_domain_ops) {
> +		.set_dev	= fsl_pamu_blocking_domain_set_dev,
> +		.blocking_domain_detach = true,
>   	}
>   };
>   
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 6abc1fbbd461..2060e8a540b3 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -4904,6 +4904,14 @@ static void intel_iommu_iotlb_sync_map(struct iommu_domain *domain,
>   	}
>   }
>   
> +static int intel_blocking_domain_set_dev(struct iommu_domain *domain,
> +					 struct device *dev)
> +{
> +	intel_iommu_detach_device(domain, dev);
> +
> +	return 0;
> +}
> +
>   const struct iommu_ops intel_iommu_ops = {
>   	.capable		= intel_iommu_capable,
>   	.domain_alloc		= intel_iommu_domain_alloc,
> @@ -4935,6 +4943,10 @@ const struct iommu_ops intel_iommu_ops = {
>   		.iova_to_phys		= intel_iommu_iova_to_phys,
>   		.free			= intel_iommu_domain_free,
>   		.enforce_cache_coherency = intel_iommu_enforce_cache_coherency,
> +	},
> +	.blocking_domain_ops = &(const struct iommu_domain_ops) {
> +		.set_dev		= intel_blocking_domain_set_dev,
> +		.blocking_domain_detach = true,
>   	}
>   };
>   
> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> index b361a4cff688..72982d1277c2 100644
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -867,6 +867,14 @@ static struct iommu_group *ipmmu_find_group(struct device *dev)
>   	return group;
>   }
>   
> +static int ipmmu_blocking_domain_set_dev(struct iommu_domain *domain,
> +					 struct device *dev)
> +{
> +	ipmmu_detach_device(domain, dev);
> +
> +	return 0;
> +}
> +
>   static const struct iommu_ops ipmmu_ops = {
>   	.domain_alloc = ipmmu_domain_alloc,
>   	.probe_device = ipmmu_probe_device,
> @@ -885,6 +893,10 @@ static const struct iommu_ops ipmmu_ops = {
>   		.iotlb_sync	= ipmmu_iotlb_sync,
>   		.iova_to_phys	= ipmmu_iova_to_phys,
>   		.free		= ipmmu_domain_free,
> +	},
> +	.blocking_domain_ops = &(const struct iommu_domain_ops) {
> +		.set_dev	= ipmmu_blocking_domain_set_dev,
> +		.blocking_domain_detach = true,
>   	}
>   };
>   
> diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
> index 569d36840b67..b0471d03db60 100644
> --- a/drivers/iommu/msm_iommu.c
> +++ b/drivers/iommu/msm_iommu.c
> @@ -674,6 +674,14 @@ irqreturn_t msm_iommu_fault_handler(int irq, void *dev_id)
>   	return 0;
>   }
>   
> +static int msm_blocking_domain_set_dev(struct iommu_domain *domain,
> +				       struct device *dev)
> +{
> +	msm_iommu_detach_dev(domain, dev);
> +
> +	return 0;
> +}
> +
>   static struct iommu_ops msm_iommu_ops = {
>   	.domain_alloc = msm_iommu_domain_alloc,
>   	.probe_device = msm_iommu_probe_device,
> @@ -696,6 +704,10 @@ static struct iommu_ops msm_iommu_ops = {
>   		.iotlb_sync_map	= msm_iommu_sync_map,
>   		.iova_to_phys	= msm_iommu_iova_to_phys,
>   		.free		= msm_iommu_domain_free,
> +	},
> +	.blocking_domain_ops = &(const struct iommu_domain_ops) {
> +		.set_dev	= msm_blocking_domain_set_dev,
> +		.blocking_domain_detach = true,
>   	}
>   };
>   
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 33ec401d40eb..cc8d80290498 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -926,6 +926,14 @@ static void mtk_iommu_get_resv_regions(struct device *dev,
>   	}
>   }
>   
> +static int mtk_blocking_domain_set_dev(struct iommu_domain *domain,
> +				       struct device *dev)
> +{
> +	mtk_iommu_detach_device(domain, dev);
> +
> +	return 0;
> +}
> +
>   static const struct iommu_ops mtk_iommu_ops = {
>   	.domain_alloc	= mtk_iommu_domain_alloc,
>   	.probe_device	= mtk_iommu_probe_device,
> @@ -946,6 +954,10 @@ static const struct iommu_ops mtk_iommu_ops = {
>   		.iotlb_sync_map	= mtk_iommu_sync_map,
>   		.iova_to_phys	= mtk_iommu_iova_to_phys,
>   		.free		= mtk_iommu_domain_free,
> +	},
> +	.blocking_domain_ops = &(const struct iommu_domain_ops) {
> +		.set_dev	= mtk_blocking_domain_set_dev,
> +		.blocking_domain_detach = true,
>   	}
>   };
>   
> diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
> index fb55802fb841..894d2526ba4c 100644
> --- a/drivers/iommu/mtk_iommu_v1.c
> +++ b/drivers/iommu/mtk_iommu_v1.c
> @@ -584,6 +584,14 @@ static int mtk_iommu_v1_hw_init(const struct mtk_iommu_v1_data *data)
>   	return 0;
>   }
>   
> +static int mtk_blocking_domain_set_dev(struct iommu_domain *domain,
> +				       struct device *dev)
> +{
> +	mtk_iommu_v1_detach_device(domain, dev);
> +
> +	return 0;
> +}
> +
>   static const struct iommu_ops mtk_iommu_v1_ops = {
>   	.domain_alloc	= mtk_iommu_v1_domain_alloc,
>   	.probe_device	= mtk_iommu_v1_probe_device,
> @@ -600,6 +608,10 @@ static const struct iommu_ops mtk_iommu_v1_ops = {
>   		.unmap		= mtk_iommu_v1_unmap,
>   		.iova_to_phys	= mtk_iommu_v1_iova_to_phys,
>   		.free		= mtk_iommu_v1_domain_free,
> +	},
> +	.blocking_domain_ops = &(const struct iommu_domain_ops) {
> +		.set_dev	= mtk_blocking_domain_set_dev,
> +		.blocking_domain_detach = true,
>   	}
>   };
>   
> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> index 6720dcb437a0..7e6ba6f1218d 100644
> --- a/drivers/iommu/omap-iommu.c
> +++ b/drivers/iommu/omap-iommu.c
> @@ -1732,6 +1732,14 @@ static struct iommu_group *omap_iommu_device_group(struct device *dev)
>   	return group;
>   }
>   
> +static int omap_blocking_domain_set_dev(struct iommu_domain *domain,
> +					struct device *dev)
> +{
> +	omap_iommu_detach_dev(domain, dev);
> +
> +	return 0;
> +}
> +
>   static const struct iommu_ops omap_iommu_ops = {
>   	.domain_alloc	= omap_iommu_domain_alloc,
>   	.probe_device	= omap_iommu_probe_device,
> @@ -1745,6 +1753,10 @@ static const struct iommu_ops omap_iommu_ops = {
>   		.unmap		= omap_iommu_unmap,
>   		.iova_to_phys	= omap_iommu_iova_to_phys,
>   		.free		= omap_iommu_domain_free,
> +	},
> +	.blocking_domain_ops = &(const struct iommu_domain_ops) {
> +		.set_dev	= omap_blocking_domain_set_dev,
> +		.blocking_domain_detach = true,
>   	}
>   };
>   
> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index 0a4196c34179..29c803759a0b 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
> @@ -1185,6 +1185,14 @@ static int rk_iommu_of_xlate(struct device *dev,
>   	return 0;
>   }
>   
> +static int rk_blocking_domain_set_dev(struct iommu_domain *domain,
> +				      struct device *dev)
> +{
> +	rk_iommu_detach_device(domain, dev);
> +
> +	return 0;
> +}
> +
>   static const struct iommu_ops rk_iommu_ops = {
>   	.domain_alloc = rk_iommu_domain_alloc,
>   	.probe_device = rk_iommu_probe_device,
> @@ -1199,6 +1207,10 @@ static const struct iommu_ops rk_iommu_ops = {
>   		.unmap		= rk_iommu_unmap,
>   		.iova_to_phys	= rk_iommu_iova_to_phys,
>   		.free		= rk_iommu_domain_free,
> +	},
> +	.blocking_domain_ops = &(const struct iommu_domain_ops) {
> +		.set_dev	= rk_blocking_domain_set_dev,
> +		.blocking_domain_detach = true,
>   	}
>   };
>   
> diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c
> index cde01c72f573..a03bff562a44 100644
> --- a/drivers/iommu/s390-iommu.c
> +++ b/drivers/iommu/s390-iommu.c
> @@ -360,6 +360,14 @@ void zpci_destroy_iommu(struct zpci_dev *zdev)
>   	iommu_device_sysfs_remove(&zdev->iommu_dev);
>   }
>   
> +static int s390_blocking_domain_set_dev(struct iommu_domain *domain,
> +					struct device *dev)
> +{
> +	s390_iommu_detach_device(domain, dev);
> +
> +	return 0;
> +}
> +
>   static const struct iommu_ops s390_iommu_ops = {
>   	.capable = s390_iommu_capable,
>   	.domain_alloc = s390_domain_alloc,
> @@ -374,6 +382,10 @@ static const struct iommu_ops s390_iommu_ops = {
>   		.unmap		= s390_iommu_unmap,
>   		.iova_to_phys	= s390_iommu_iova_to_phys,
>   		.free		= s390_domain_free,
> +	},
> +	.blocking_domain_ops = &(const struct iommu_domain_ops) {
> +		.set_dev		= s390_blocking_domain_set_dev,
> +		.blocking_domain_detach = true,
>   	}
>   };
>   
> diff --git a/drivers/iommu/sprd-iommu.c b/drivers/iommu/sprd-iommu.c
> index 45b58845f5f8..ab62063cee5b 100644
> --- a/drivers/iommu/sprd-iommu.c
> +++ b/drivers/iommu/sprd-iommu.c
> @@ -413,6 +413,13 @@ static int sprd_iommu_of_xlate(struct device *dev, struct of_phandle_args *args)
>   	return 0;
>   }
>   
> +static int sprd_blocking_domain_set_dev(struct iommu_domain *domain,
> +					struct device *dev)
> +{
> +	sprd_iommu_detach_device(domain, dev);
> +
> +	return 0;
> +}
>   
>   static const struct iommu_ops sprd_iommu_ops = {
>   	.domain_alloc	= sprd_iommu_domain_alloc,
> @@ -431,6 +438,10 @@ static const struct iommu_ops sprd_iommu_ops = {
>   		.iotlb_sync	= sprd_iommu_sync,
>   		.iova_to_phys	= sprd_iommu_iova_to_phys,
>   		.free		= sprd_iommu_domain_free,
> +	},
> +	.blocking_domain_ops = &(const struct iommu_domain_ops) {
> +		.set_dev		= sprd_blocking_domain_set_dev,
> +		.blocking_domain_detach = true,
>   	}
>   };
>   
> diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
> index 041b30463552..a51a55ae0634 100644
> --- a/drivers/iommu/sun50i-iommu.c
> +++ b/drivers/iommu/sun50i-iommu.c
> @@ -758,6 +758,14 @@ static int sun50i_iommu_of_xlate(struct device *dev,
>   	return iommu_fwspec_add_ids(dev, &id, 1);
>   }
>   
> +static int sun50i_blocking_domain_set_dev(struct iommu_domain *domain,
> +					  struct device *dev)
> +{
> +	sun50i_iommu_detach_device(domain, dev);
> +
> +	return 0;
> +}
> +
>   static const struct iommu_ops sun50i_iommu_ops = {
>   	.pgsize_bitmap	= SZ_4K,
>   	.device_group	= sun50i_iommu_device_group,
> @@ -774,6 +782,10 @@ static const struct iommu_ops sun50i_iommu_ops = {
>   		.map		= sun50i_iommu_map,
>   		.unmap		= sun50i_iommu_unmap,
>   		.free		= sun50i_iommu_domain_free,
> +	},
> +	.blocking_domain_ops = &(const struct iommu_domain_ops) {
> +		.set_dev		= sun50i_blocking_domain_set_dev,
> +		.blocking_domain_detach = true,
>   	}
>   };
>   
> diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c
> index f083a9fba4d0..7e2aef7ae72e 100644
> --- a/drivers/iommu/tegra-gart.c
> +++ b/drivers/iommu/tegra-gart.c
> @@ -270,6 +270,14 @@ static void gart_iommu_sync(struct iommu_domain *domain,
>   	gart_iommu_sync_map(domain, gather->start, length);
>   }
>   
> +static int gart_blocking_domain_set_dev(struct iommu_domain *domain,
> +					struct device *dev)
> +{
> +	gart_iommu_detach_dev(domain, dev);
> +
> +	return 0;
> +}
> +
>   static const struct iommu_ops gart_iommu_ops = {
>   	.domain_alloc	= gart_iommu_domain_alloc,
>   	.probe_device	= gart_iommu_probe_device,
> @@ -286,6 +294,10 @@ static const struct iommu_ops gart_iommu_ops = {
>   		.iotlb_sync_map	= gart_iommu_sync_map,
>   		.iotlb_sync	= gart_iommu_sync,
>   		.free		= gart_iommu_domain_free,
> +	},
> +	.blocking_domain_ops = &(const struct iommu_domain_ops) {
> +		.set_dev		= gart_blocking_domain_set_dev,
> +		.blocking_domain_detach = true,
>   	}
>   };
>   
> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> index 36e9c2864e3f..fe2c463db230 100644
> --- a/drivers/iommu/tegra-smmu.c
> +++ b/drivers/iommu/tegra-smmu.c
> @@ -963,6 +963,14 @@ static int tegra_smmu_of_xlate(struct device *dev,
>   	return iommu_fwspec_add_ids(dev, &id, 1);
>   }
>   
> +static int tegra_smmu_blocking_domain_set_dev(struct iommu_domain *domain,
> +					      struct device *dev)
> +{
> +	tegra_smmu_detach_dev(domain, dev);
> +
> +	return 0;
> +}
> +
>   static const struct iommu_ops tegra_smmu_ops = {
>   	.domain_alloc = tegra_smmu_domain_alloc,
>   	.probe_device = tegra_smmu_probe_device,
> @@ -977,6 +985,10 @@ static const struct iommu_ops tegra_smmu_ops = {
>   		.unmap		= tegra_smmu_unmap,
>   		.iova_to_phys	= tegra_smmu_iova_to_phys,
>   		.free		= tegra_smmu_domain_free,
> +	},
> +	.blocking_domain_ops = &(const struct iommu_domain_ops) {
> +		.set_dev		= tegra_smmu_blocking_domain_set_dev,
> +		.blocking_domain_detach = true,
>   	}
>   };
>   
> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> index ce2bd01806d8..5054ebaf9654 100644
> --- a/drivers/iommu/virtio-iommu.c
> +++ b/drivers/iommu/virtio-iommu.c
> @@ -1023,6 +1023,9 @@ static struct iommu_ops viommu_ops = {
>   		.iova_to_phys		= viommu_iova_to_phys,
>   		.iotlb_sync		= viommu_iotlb_sync,
>   		.free			= viommu_domain_free,
> +	},
> +	.blocking_domain_ops = &(const struct iommu_domain_ops) {
> +		.set_dev		= viommu_attach_dev,
>   	}
>   };
>   

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] iommu: Add blocking_domain_ops field in iommu_ops
  2022-05-16  7:27   ` Christoph Hellwig
@ 2022-05-16 13:05     ` Jason Gunthorpe
  0 siblings, 0 replies; 18+ messages in thread
From: Jason Gunthorpe @ 2022-05-16 13:05 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Lu Baolu, Joerg Roedel, Kevin Tian, Ashok Raj, Will Deacon,
	Robin Murphy, Jean-Philippe Brucker, Eric Auger, Liu Yi L,
	Jacob jun Pan, iommu, linux-kernel

On Mon, May 16, 2022 at 12:27:41AM -0700, Christoph Hellwig wrote:
> On Mon, May 16, 2022 at 09:57:56AM +0800, Lu Baolu wrote:
> > Each IOMMU driver must provide a blocking domain ops. If the hardware
> > supports detaching domain from device, setting blocking domain equals
> > detaching the existing domain from the deivce. Otherwise, an UNMANAGED
> > domain without any mapping will be used instead.
> 
> blocking in this case means not allowing any access?  The naming
> sounds a bit odd to me as blocking in the kernel has a specific
> meaning.  Maybe something like noaccess ops might be a better name?

It is because of this:

include/linux/iommu.h: *        IOMMU_DOMAIN_BLOCKED    - All DMA is blocked, can be used to isolate
include/linux/iommu.h:#define IOMMU_DOMAIN_BLOCKED      (0U)

noaccess might be clearer

Jason

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] iommu: Add blocking_domain_ops field in iommu_ops
  2022-05-16 11:22   ` Robin Murphy
@ 2022-05-16 13:43     ` Baolu Lu
  2022-05-16 13:57     ` Jason Gunthorpe
  1 sibling, 0 replies; 18+ messages in thread
From: Baolu Lu @ 2022-05-16 13:43 UTC (permalink / raw)
  To: Robin Murphy, Joerg Roedel, Jason Gunthorpe, Christoph Hellwig,
	Kevin Tian, Ashok Raj, Will Deacon, Jean-Philippe Brucker
  Cc: baolu.lu, Eric Auger, Liu Yi L, Jacob jun Pan, iommu, linux-kernel

Hi Robin,

On 2022/5/16 19:22, Robin Murphy wrote:
> On 2022-05-16 02:57, Lu Baolu wrote:
>> Each IOMMU driver must provide a blocking domain ops. If the hardware
>> supports detaching domain from device, setting blocking domain equals
>> detaching the existing domain from the deivce. Otherwise, an UNMANAGED
>> domain without any mapping will be used instead.
> 
> Unfortunately that's backwards - most of the implementations of 
> .detach_dev are disabling translation entirely, meaning the device ends 
> up effectively in passthrough rather than blocked. Conversely, at least 
> arm-smmu and arm-smmu-v3 could implement IOMMU_DOMAIN_BLOCKED properly 
> with fault-type S2CRs and STEs respectively, it just needs a bit of 
> wiring up.

Thank you for letting me know this.

This means that we need to add an additional UNMANAGED domain for each
iommu group, although it is not used most of the time. If most IOMMU
drivers could implement real dumb blocking domains, this burden may be
reduced.

Best regards,
baolu

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] iommu: Add blocking_domain_ops field in iommu_ops
  2022-05-16 11:22   ` Robin Murphy
  2022-05-16 13:43     ` Baolu Lu
@ 2022-05-16 13:57     ` Jason Gunthorpe
  2022-05-17  2:37       ` Baolu Lu
  1 sibling, 1 reply; 18+ messages in thread
From: Jason Gunthorpe @ 2022-05-16 13:57 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Lu Baolu, Joerg Roedel, Christoph Hellwig, Kevin Tian, Ashok Raj,
	Will Deacon, Jean-Philippe Brucker, Eric Auger, Liu Yi L,
	Jacob jun Pan, iommu, linux-kernel

On Mon, May 16, 2022 at 12:22:08PM +0100, Robin Murphy wrote:
> On 2022-05-16 02:57, Lu Baolu wrote:
> > Each IOMMU driver must provide a blocking domain ops. If the hardware
> > supports detaching domain from device, setting blocking domain equals
> > detaching the existing domain from the deivce. Otherwise, an UNMANAGED
> > domain without any mapping will be used instead.
> 
> Unfortunately that's backwards - most of the implementations of .detach_dev
> are disabling translation entirely, meaning the device ends up effectively
> in passthrough rather than blocked.

Ideally we'd convert the detach_dev of every driver into either
a blocking or identity domain. The trick is knowing which is which..

Guessing going down the list:
 apple dart - blocking, detach_dev calls apple_dart_hw_disable_dma() same as
              IOMMU_DOMAIN_BLOCKED
	      [I wonder if this drive ris wrong in other ways though because
               I dont see a remove_streams in attach_dev]
 exynos - this seems to disable the 'sysmmu' so I'm guessing this is
          identity
 iommu-vmsa - Comment says 'disable mmu translaction' so I'm guessing
              this is idenity
 mkt_v1 - Code looks similar to mkt, which is probably identity.
 rkt - No idea
 sprd - No idea
 sun50i - This driver confusingly treats identity the same as
          unmanaged, seems wrong, no idea.
 amd - Not sure, clear_dte_entry() seems to set translation on but points
       the PTE to 0 ? Based on the spec table 8 I would have expected
       TV to be clear which would be blocking. Maybe a bug??
 arm smmu qcomm - not sure
 intel - blocking

These doesn't support default domains, so detach_dev should return 
back to DMA API ownership, which is either identity or something weird:
 fsl_pamu - identity due to the PPC use of dma direct
 msm
 mkt
 omap
 s390 - platform DMA ops
 terga-gart - Usually something called a GART would be 0 length once
              disabled, guessing blocking?
 tegra-smmu

So, the approach here should be to go driver by driver and convert
detach_dev to either identity, blocking or just delete it entirely,
excluding the above 7 that don't support default domains. And get acks
from the driver owners.

> Conversely, at least arm-smmu and arm-smmu-v3 could implement
> IOMMU_DOMAIN_BLOCKED properly with fault-type S2CRs and STEs
> respectively, it just needs a bit of wiring up.

Given that vfio now uses them it seems worthwhile to do..

Jason

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] iommu: Add blocking_domain_ops field in iommu_ops
  2022-05-16 13:57     ` Jason Gunthorpe
@ 2022-05-17  2:37       ` Baolu Lu
  2022-05-17 12:43         ` Robin Murphy
  2022-05-17 13:08         ` Jason Gunthorpe
  0 siblings, 2 replies; 18+ messages in thread
From: Baolu Lu @ 2022-05-17  2:37 UTC (permalink / raw)
  To: Jason Gunthorpe, Robin Murphy
  Cc: baolu.lu, Joerg Roedel, Christoph Hellwig, Kevin Tian, Ashok Raj,
	Will Deacon, Jean-Philippe Brucker, Eric Auger, Liu Yi L,
	Jacob jun Pan, iommu, linux-kernel

Hi Jason,

On 2022/5/16 21:57, Jason Gunthorpe wrote:
> On Mon, May 16, 2022 at 12:22:08PM +0100, Robin Murphy wrote:
>> On 2022-05-16 02:57, Lu Baolu wrote:
>>> Each IOMMU driver must provide a blocking domain ops. If the hardware
>>> supports detaching domain from device, setting blocking domain equals
>>> detaching the existing domain from the deivce. Otherwise, an UNMANAGED
>>> domain without any mapping will be used instead.
>> Unfortunately that's backwards - most of the implementations of .detach_dev
>> are disabling translation entirely, meaning the device ends up effectively
>> in passthrough rather than blocked.
> Ideally we'd convert the detach_dev of every driver into either
> a blocking or identity domain. The trick is knowing which is which..

I am still a bit puzzled about how the blocking_domain should be used 
when it is extended to support ->set_dev_pasid.

If it's a blocking domain, the IOMMU driver knows that setting the
blocking domain to device pasid means detaching the existing one.

But if it's an identity domain, how could the IOMMU driver choose
between:

  - setting the input domain to the pasid on device; or,
  - detaching the existing domain.

I've ever thought about below solutions:

- Checking the domain types and dispatching them to different
   operations.
- Using different blocking domains for different types of domains.

But both look rough.

> 
> Guessing going down the list:
>   apple dart - blocking, detach_dev calls apple_dart_hw_disable_dma() same as
>                IOMMU_DOMAIN_BLOCKED
> 	      [I wonder if this drive ris wrong in other ways though because
>                 I dont see a remove_streams in attach_dev]
>   exynos - this seems to disable the 'sysmmu' so I'm guessing this is
>            identity
>   iommu-vmsa - Comment says 'disable mmu translaction' so I'm guessing
>                this is idenity
>   mkt_v1 - Code looks similar to mkt, which is probably identity.
>   rkt - No idea
>   sprd - No idea
>   sun50i - This driver confusingly treats identity the same as
>            unmanaged, seems wrong, no idea.
>   amd - Not sure, clear_dte_entry() seems to set translation on but points
>         the PTE to 0 ? Based on the spec table 8 I would have expected
>         TV to be clear which would be blocking. Maybe a bug??
>   arm smmu qcomm - not sure
>   intel - blocking
> 
> These doesn't support default domains, so detach_dev should return
> back to DMA API ownership, which is either identity or something weird:
>   fsl_pamu - identity due to the PPC use of dma direct
>   msm
>   mkt
>   omap
>   s390 - platform DMA ops
>   terga-gart - Usually something called a GART would be 0 length once
>                disabled, guessing blocking?
>   tegra-smmu
> 
> So, the approach here should be to go driver by driver and convert
> detach_dev to either identity, blocking or just delete it entirely,
> excluding the above 7 that don't support default domains. And get acks
> from the driver owners.
> 

Agreed. There seems to be a long way to go. I am wondering if we could
decouple this refactoring from my new SVA API work? We can easily switch
.detach_dev_pasid to using blocking domain later.

Best regards,
baolu

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] iommu: Add blocking_domain_ops field in iommu_ops
  2022-05-17  2:37       ` Baolu Lu
@ 2022-05-17 12:43         ` Robin Murphy
  2022-05-17 13:13           ` Jason Gunthorpe
  2022-05-17 13:08         ` Jason Gunthorpe
  1 sibling, 1 reply; 18+ messages in thread
From: Robin Murphy @ 2022-05-17 12:43 UTC (permalink / raw)
  To: Baolu Lu, Jason Gunthorpe
  Cc: Joerg Roedel, Christoph Hellwig, Kevin Tian, Ashok Raj,
	Will Deacon, Jean-Philippe Brucker, Eric Auger, Liu Yi L,
	Jacob jun Pan, iommu, linux-kernel

On 2022-05-17 03:37, Baolu Lu wrote:
> Hi Jason,
> 
> On 2022/5/16 21:57, Jason Gunthorpe wrote:
>> On Mon, May 16, 2022 at 12:22:08PM +0100, Robin Murphy wrote:
>>> On 2022-05-16 02:57, Lu Baolu wrote:
>>>> Each IOMMU driver must provide a blocking domain ops. If the hardware
>>>> supports detaching domain from device, setting blocking domain equals
>>>> detaching the existing domain from the deivce. Otherwise, an UNMANAGED
>>>> domain without any mapping will be used instead.
>>> Unfortunately that's backwards - most of the implementations of 
>>> .detach_dev
>>> are disabling translation entirely, meaning the device ends up 
>>> effectively
>>> in passthrough rather than blocked.
>> Ideally we'd convert the detach_dev of every driver into either
>> a blocking or identity domain. The trick is knowing which is which..
> 
> I am still a bit puzzled about how the blocking_domain should be used 
> when it is extended to support ->set_dev_pasid.
> 
> If it's a blocking domain, the IOMMU driver knows that setting the
> blocking domain to device pasid means detaching the existing one.
> 
> But if it's an identity domain, how could the IOMMU driver choose
> between:
> 
>   - setting the input domain to the pasid on device; or,
>   - detaching the existing domain.
> 
> I've ever thought about below solutions:
> 
> - Checking the domain types and dispatching them to different
>    operations.
> - Using different blocking domains for different types of domains.
> 
> But both look rough.
> 
>>
>> Guessing going down the list:
>>   apple dart - blocking, detach_dev calls apple_dart_hw_disable_dma() 
>> same as
>>                IOMMU_DOMAIN_BLOCKED
>>           [I wonder if this drive ris wrong in other ways though because
>>                 I dont see a remove_streams in attach_dev]
>>   exynos - this seems to disable the 'sysmmu' so I'm guessing this is
>>            identity
>>   iommu-vmsa - Comment says 'disable mmu translaction' so I'm guessing
>>                this is idenity
>>   mkt_v1 - Code looks similar to mkt, which is probably identity.
>>   rkt - No idea
>>   sprd - No idea
>>   sun50i - This driver confusingly treats identity the same as
>>            unmanaged, seems wrong, no idea.
>>   amd - Not sure, clear_dte_entry() seems to set translation on but 
>> points
>>         the PTE to 0 ? Based on the spec table 8 I would have expected
>>         TV to be clear which would be blocking. Maybe a bug??
>>   arm smmu qcomm - not sure
>>   intel - blocking
>>
>> These doesn't support default domains, so detach_dev should return
>> back to DMA API ownership, which is either identity or something weird:
>>   fsl_pamu - identity due to the PPC use of dma direct
>>   msm
>>   mkt
>>   omap
>>   s390 - platform DMA ops
>>   terga-gart - Usually something called a GART would be 0 length once
>>                disabled, guessing blocking?
>>   tegra-smmu
>>
>> So, the approach here should be to go driver by driver and convert
>> detach_dev to either identity, blocking or just delete it entirely,
>> excluding the above 7 that don't support default domains. And get acks
>> from the driver owners.
>>
> 
> Agreed. There seems to be a long way to go. I am wondering if we could
> decouple this refactoring from my new SVA API work? We can easily switch
> .detach_dev_pasid to using blocking domain later.

FWIW from my point of view I'm happy with having a .detach_dev_pasid op 
meaning implicitly-blocked access for now. On SMMUv3, PASIDs don't mix 
with our current notion of IOMMU_DOMAIN_IDENTITY (nor the potential one 
for IOMMU_DOMAIN_BLOCKED), so giving PASIDs functional symmetry with 
devices would need significantly more work anyway.

Thanks,
Robin.

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] iommu: Add blocking_domain_ops field in iommu_ops
  2022-05-17  2:37       ` Baolu Lu
  2022-05-17 12:43         ` Robin Murphy
@ 2022-05-17 13:08         ` Jason Gunthorpe
  1 sibling, 0 replies; 18+ messages in thread
From: Jason Gunthorpe @ 2022-05-17 13:08 UTC (permalink / raw)
  To: Baolu Lu
  Cc: Robin Murphy, Joerg Roedel, Christoph Hellwig, Kevin Tian,
	Ashok Raj, Will Deacon, Jean-Philippe Brucker, Eric Auger,
	Liu Yi L, Jacob jun Pan, iommu, linux-kernel

On Tue, May 17, 2022 at 10:37:55AM +0800, Baolu Lu wrote:
> Hi Jason,
> 
> On 2022/5/16 21:57, Jason Gunthorpe wrote:
> > On Mon, May 16, 2022 at 12:22:08PM +0100, Robin Murphy wrote:
> > > On 2022-05-16 02:57, Lu Baolu wrote:
> > > > Each IOMMU driver must provide a blocking domain ops. If the hardware
> > > > supports detaching domain from device, setting blocking domain equals
> > > > detaching the existing domain from the deivce. Otherwise, an UNMANAGED
> > > > domain without any mapping will be used instead.
> > > Unfortunately that's backwards - most of the implementations of .detach_dev
> > > are disabling translation entirely, meaning the device ends up effectively
> > > in passthrough rather than blocked.
> > Ideally we'd convert the detach_dev of every driver into either
> > a blocking or identity domain. The trick is knowing which is which..
> 
> I am still a bit puzzled about how the blocking_domain should be used when
> it is extended to support ->set_dev_pasid.
> 
> If it's a blocking domain, the IOMMU driver knows that setting the
> blocking domain to device pasid means detaching the existing one.

> But if it's an identity domain, how could the IOMMU driver choose
> between:

The identity domain would never be used for detaching a PASID. The
reason it is in this explanation is because every detach_dev op should
be deleted - and the code that it was doing can remain in the driver
as either a blocking or identity domain.

> > So, the approach here should be to go driver by driver and convert
> > detach_dev to either identity, blocking or just delete it entirely,
> > excluding the above 7 that don't support default domains. And get acks
> > from the driver owners.
 
> Agreed. There seems to be a long way to go. I am wondering if we could
> decouple this refactoring from my new SVA API work? We can easily switch
> .detach_dev_pasid to using blocking domain later.

Well, PASID has nothing to do with this. PASID enabled drivers would
just need:
 - Must be using default domains and must have a NULL detach_dev op
 - Must provide a blocking_domain
 - Must provide set_dev_pasid for the unmanaged and blocking domains
   it creates

That is it.

Realigning the existing drivers for their base RID support is another
task.

The appeal of this is that it matches how base RID support should look
and doesn't continue the confusing appearance that there is a strictly
paired attach/detach operation.

Any domain can be set ony and RID/PASID at any time.

Drivers intended to be used with VFIO really want a blocking_domain
anyhow for efficiency.

Jason

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] iommu: Add blocking_domain_ops field in iommu_ops
  2022-05-17 12:43         ` Robin Murphy
@ 2022-05-17 13:13           ` Jason Gunthorpe
  2022-05-18  6:43             ` Baolu Lu
  0 siblings, 1 reply; 18+ messages in thread
From: Jason Gunthorpe @ 2022-05-17 13:13 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Baolu Lu, Joerg Roedel, Christoph Hellwig, Kevin Tian, Ashok Raj,
	Will Deacon, Jean-Philippe Brucker, Eric Auger, Liu Yi L,
	Jacob jun Pan, iommu, linux-kernel

On Tue, May 17, 2022 at 01:43:03PM +0100, Robin Murphy wrote:

> FWIW from my point of view I'm happy with having a .detach_dev_pasid op
> meaning implicitly-blocked access for now. 

If this is the path then lets not call it attach/detach
please. 'set_dev_pasid' and 'set_dev_blocking_pasid' are clearer
names.

> On SMMUv3, PASIDs don't mix with our current notion of
> IOMMU_DOMAIN_IDENTITY (nor the potential one for
> IOMMU_DOMAIN_BLOCKED), so giving PASIDs functional symmetry with
> devices would need significantly more work anyway.

There is no extra work in the driver, beyond SMMU having to implement
a blocking domain. The blocking domain's set_dev_pasid op simply is
whatever set_dev_blocking_pasid would have done on the unmanaged
domain.

identity doesn't come into this, identity domains should have a NULL
set_dev_pasid op if the driver can't support using it on a PASID.

IMHO blocking_domain->ops->set_dev_pasid() is just a more logical name
than domain->ops->set_dev_blocking_pasid() - especially since VFIO
would like drivers to implement blocking domain anyhow.

Jason

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] iommu: Add blocking_domain_ops field in iommu_ops
  2022-05-17 13:13           ` Jason Gunthorpe
@ 2022-05-18  6:43             ` Baolu Lu
  0 siblings, 0 replies; 18+ messages in thread
From: Baolu Lu @ 2022-05-18  6:43 UTC (permalink / raw)
  To: Jason Gunthorpe, Robin Murphy
  Cc: baolu.lu, Joerg Roedel, Christoph Hellwig, Kevin Tian, Ashok Raj,
	Will Deacon, Jean-Philippe Brucker, Eric Auger, Liu Yi L,
	Jacob jun Pan, iommu, linux-kernel

On 2022/5/17 21:13, Jason Gunthorpe wrote:
> On Tue, May 17, 2022 at 01:43:03PM +0100, Robin Murphy wrote:
> 
>> FWIW from my point of view I'm happy with having a .detach_dev_pasid op
>> meaning implicitly-blocked access for now.
> 
> If this is the path then lets not call it attach/detach
> please. 'set_dev_pasid' and 'set_dev_blocking_pasid' are clearer
> names.

Sure. And with the blocking domain implemented, the
set_dev_blocking_pasid could be deprecated.

> 
>> On SMMUv3, PASIDs don't mix with our current notion of
>> IOMMU_DOMAIN_IDENTITY (nor the potential one for
>> IOMMU_DOMAIN_BLOCKED), so giving PASIDs functional symmetry with
>> devices would need significantly more work anyway.
> 
> There is no extra work in the driver, beyond SMMU having to implement
> a blocking domain. The blocking domain's set_dev_pasid op simply is
> whatever set_dev_blocking_pasid would have done on the unmanaged
> domain.
> 
> identity doesn't come into this, identity domains should have a NULL
> set_dev_pasid op if the driver can't support using it on a PASID.
> 
> IMHO blocking_domain->ops->set_dev_pasid() is just a more logical name
> than domain->ops->set_dev_blocking_pasid() - especially since VFIO
> would like drivers to implement blocking domain anyhow.

Perhaps implementing blocking domains for intel and smmuv3 iommu drivers
seem to be a more practical start.

Best regards,
baolu

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] iommu: Add blocking_domain_ops field in iommu_ops
  2022-05-16  1:57 ` [PATCH 2/5] iommu: Add blocking_domain_ops field in iommu_ops Lu Baolu
  2022-05-16  7:27   ` Christoph Hellwig
  2022-05-16 11:22   ` Robin Murphy
@ 2022-05-20  8:45   ` Joerg Roedel
  2022-05-20 11:03     ` Baolu Lu
  2 siblings, 1 reply; 18+ messages in thread
From: Joerg Roedel @ 2022-05-20  8:45 UTC (permalink / raw)
  To: Lu Baolu
  Cc: Jason Gunthorpe, Christoph Hellwig, Kevin Tian, Ashok Raj,
	Will Deacon, Robin Murphy, Jean-Philippe Brucker, Eric Auger,
	Liu Yi L, Jacob jun Pan, iommu, linux-kernel

On Mon, May 16, 2022 at 09:57:56AM +0800, Lu Baolu wrote:
>  	const struct iommu_domain_ops *default_domain_ops;
> +	const struct iommu_domain_ops *blocking_domain_ops;

I don't understand why extra domain-ops are needed for this. I think it
would be more straight-forward to implement allocation of
IOMMU_DOMAIN_BLOCKED domains in each driver and let the details be
handled in the set_dev() call-back. The IOMMU driver can make sure DMA
is blocked for a device when it encounters a IOMMU_DOMAIN_BLOCKED
domain.

For IOMMUs that have no explicit way to block DMA could just use an
unmanaged domain with an empty page-table.

Regards,

	Joerg

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] iommu: Add blocking_domain_ops field in iommu_ops
  2022-05-20  8:45   ` Joerg Roedel
@ 2022-05-20 11:03     ` Baolu Lu
  0 siblings, 0 replies; 18+ messages in thread
From: Baolu Lu @ 2022-05-20 11:03 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: baolu.lu, Jason Gunthorpe, Christoph Hellwig, Kevin Tian,
	Ashok Raj, Will Deacon, Robin Murphy, Jean-Philippe Brucker,
	Eric Auger, Liu Yi L, Jacob jun Pan, iommu, linux-kernel

On 2022/5/20 16:45, Joerg Roedel wrote:
> On Mon, May 16, 2022 at 09:57:56AM +0800, Lu Baolu wrote:
>>   	const struct iommu_domain_ops *default_domain_ops;
>> +	const struct iommu_domain_ops *blocking_domain_ops;
> 
> I don't understand why extra domain-ops are needed for this. I think it
> would be more straight-forward to implement allocation of
> IOMMU_DOMAIN_BLOCKED domains in each driver and let the details be
> handled in the set_dev() call-back. The IOMMU driver can make sure DMA
> is blocked for a device when it encounters a IOMMU_DOMAIN_BLOCKED
> domain.
> 
> For IOMMUs that have no explicit way to block DMA could just use an
> unmanaged domain with an empty page-table.

Yes, this is what will go.

Best regards,
baolu


^ permalink raw reply	[flat|nested] 18+ messages in thread
end of thread, other threads:[~2022-05-20 11:03 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-16  1:57 [PATCH 0/5] iommu: Make blocking domain static for group Lu Baolu
2022-05-16  1:57 ` [PATCH 1/5] iommu: Rename attach_dev to set_dev in domain ops Lu Baolu
2022-05-16  1:57 ` [PATCH 2/5] iommu: Add blocking_domain_ops field in iommu_ops Lu Baolu
2022-05-16  7:27   ` Christoph Hellwig
2022-05-16 13:05     ` Jason Gunthorpe
2022-05-16 11:22   ` Robin Murphy
2022-05-16 13:43     ` Baolu Lu
2022-05-16 13:57     ` Jason Gunthorpe
2022-05-17  2:37       ` Baolu Lu
2022-05-17 12:43         ` Robin Murphy
2022-05-17 13:13           ` Jason Gunthorpe
2022-05-18  6:43             ` Baolu Lu
2022-05-17 13:08         ` Jason Gunthorpe
2022-05-20  8:45   ` Joerg Roedel
2022-05-20 11:03     ` Baolu Lu
2022-05-16  1:57 ` [PATCH 3/5] iommu: Make blocking domain static for iommu group Lu Baolu
2022-05-16  1:57 ` [PATCH 4/5] iommu: Use blocking domain for empty domain attaching Lu Baolu
2022-05-16  1:57 ` [PATCH 5/5] iommu: Remove .detach_dev from iommu domain ops Lu Baolu

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