Re: [PATCH 0/7] iommu/vt-d: Make intel-iommu.h private

2022-05-15 Thread Christoph Hellwig
On Sat, May 14, 2022 at 09:43:15AM +0800, Lu Baolu wrote:
> Hi folks,
> 
> The include/linux/intel-iommu.h should be private to the Intel IOMMU
> driver. Other drivers or components should interact with the IOMMU
> drivers through the kAPIs provided by the iommu core.
> 
> This series cleanups all includes of intel-iommu.h outside of the Intel
> IOMMU driver and move this header from include/linux to
> drivers/iommu/intel/.
> 
> No functional changes intended. Please help to review and suggest.

Thanks, this was long overdue!

The series looks good to me:

Reviewed-by: Christoph Hellwig 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v6 03/12] iommu: Add attach/detach_dev_pasid domain ops

2022-05-15 Thread Baolu Lu

On 2022/5/12 19:51, Jason Gunthorpe wrote:

On Thu, May 12, 2022 at 08:00:59AM +0100, Jean-Philippe Brucker wrote:


It is not "missing" it is just renamed to blocking_domain->ops->set_dev_pasid()

The implementation of that function would be identical to
detach_dev_pasid.


   attach(dev, pasid, sva_domain)
   detach(dev, pasid, sva_domain)

versus

   set_dev_pasid(dev, pasid, sva_domain)
   set_dev_pasid(dev, pasid, blocking)

we loose the information of the domain previously attached, and the SMMU
driver has to retrieve it to find the ASID corresponding to the mm.


It would be easy to have the old domain be an input as well - the core
code knows it.


Thanks a lot for all suggestions. I have posted a follow-up series for
this:

https://lore.kernel.org/linux-iommu/20220516015759.2952771-1-baolu...@linux.intel.com/

Let's discuss this there.

Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 5/5] iommu: Remove .detach_dev from iommu domain ops

2022-05-15 Thread Lu Baolu
The .detach_dev callback is not used anymore. Reomve it to avoid dead
code.

Signed-off-by: Lu Baolu 
---
 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= 

[PATCH 2/5] iommu: Add blocking_domain_ops field in iommu_ops

2022-05-15 Thread 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 
---
 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 = {

[PATCH 4/5] iommu: Use blocking domain for empty domain attaching

2022-05-15 Thread 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 
---
 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

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 3/5] iommu: Make blocking domain static for iommu group

2022-05-15 Thread 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 
---
 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(>mutex);
+   ret = iommu_group_alloc_blocking_domain(group, dev);
+   mutex_unlock(>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(>devices))
+   iommu_group_free_blocking_domain(group, dev);
mutex_unlock(>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(>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 

[PATCH 1/5] iommu: Rename attach_dev to set_dev in domain ops

2022-05-15 Thread 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 
Signed-off-by: Lu Baolu 
---
 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 = {
   

[PATCH 0/5] iommu: Make blocking domain static for group

2022-05-15 Thread 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.gd49...@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

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v5 2/5] iommu: Implement of_iommu_get_resv_regions()

2022-05-15 Thread Janne Grunau
On 2022-05-12 21:00:49 +0200, Thierry Reding wrote:
> From: Thierry Reding 
> 
> This is an implementation that IOMMU drivers can use to obtain reserved
> memory regions from a device tree node. It uses the reserved-memory DT
> bindings to find the regions associated with a given device. If these
> regions are marked accordingly, identity mappings will be created for
> them in the IOMMU domain that the devices will be attached to.
> 
> Signed-off-by: Thierry Reding 
> ---
> Changes in v5:
> - update for new "iommu-addresses" device tree bindings
> 
> Changes in v4:
> - fix build failure on !CONFIG_OF_ADDRESS
> 
> Changes in v3:
> - change "active" property to identity mapping flag that is part of the
>   memory region specifier (as defined by #memory-region-cells) to allow
>   per-reference flags to be used
> 
> Changes in v2:
> - use "active" property to determine whether direct mappings are needed
> 
>  drivers/iommu/of_iommu.c | 90 
>  include/linux/of_iommu.h |  8 
>  2 files changed, 98 insertions(+)
> 
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 5696314ae69e..9e341b5e307f 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -11,12 +11,15 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
>  
> +#include 
> +
>  #define NO_IOMMU 1
>  
>  static int of_iommu_xlate(struct device *dev,
> @@ -172,3 +175,90 @@ const struct iommu_ops *of_iommu_configure(struct device 
> *dev,
>  
>   return ops;
>  }
> +
> +/**
> + * of_iommu_get_resv_regions - reserved region driver helper for device tree
> + * @dev: device for which to get reserved regions
> + * @list: reserved region list
> + *
> + * IOMMU drivers can use this to implement their .get_resv_regions() callback
> + * for memory regions attached to a device tree node. See the reserved-memory
> + * device tree bindings on how to use these:
> + *
> + *   Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> + */
> +void of_iommu_get_resv_regions(struct device *dev, struct list_head *list)
> +{
> +#if IS_ENABLED(CONFIG_OF_ADDRESS)
> + struct of_phandle_iterator it;
> + int err;
> +
> + of_for_each_phandle(, err, dev->of_node, "memory-region", NULL, 0) {
> + struct iommu_resv_region *region;
> + struct resource res;
> + const __be32 *maps;
> + int size;

Adding 'if (!of_device_is_available(it.node)) continue;' here would help 
backwards compatibility. My plan was to add the reserved regions with 
"iommu-addresses" with all zero adresses and sizes with status = 
"disabled" to the devicetree. A bootloader update is required to fill 
those.

> +
> + memset(, 0, sizeof(res));
> +
> + /*
> +  * The "reg" property is optional and can be omitted by 
> reserved-memory regions
> +  * that represent reservations in the IOVA space, which are 
> regions that should
> +  * not be mapped.
> +  */
> + if (of_find_property(it.node, "reg", NULL)) {
> + err = of_address_to_resource(it.node, 0, );
> + if (err < 0) {
> + dev_err(dev, "failed to parse memory region 
> %pOF: %d\n",
> + it.node, err);
> + continue;
> + }
> + }
> +
> + maps = of_get_property(it.node, "iommu-addresses", );
> + if (maps) {
> + const __be32 *end = maps + size / sizeof(__be32);
> + struct device_node *np;
> + unsigned int index = 0;
> + u32 phandle;
> + int na, ns;
> +
> + while (maps < end) {
> + phys_addr_t start, end;
> + size_t length;
> +
> + phandle = be32_to_cpup(maps++);
> + np = of_find_node_by_phandle(phandle);
> + na = of_n_addr_cells(np);
> + ns = of_n_size_cells(np);
> +
> + start = of_translate_dma_address(np, maps);
> + length = of_read_number(maps + na, ns);

alternatively we could handle mappings/reservations with length 0 as 
error and skip them.

> + end = start + length - 1;
> +
> + if (np == dev->of_node) {
> + int prot = IOMMU_READ | IOMMU_WRITE;
> + enum iommu_resv_type type;
> +
> + /*
> +  * IOMMU regions without an associated 
> physical region
> +  * cannot be mapped and are simply 
> 

Re: [PATCH v5 1/5] dt-bindings: reserved-memory: Document iommu-addresses

2022-05-15 Thread Janne Grunau
On 2022-05-12 21:00:48 +0200, Thierry Reding wrote:
> From: Thierry Reding 
> 
> This adds the "iommu-addresses" property to reserved-memory nodes, which
> allow describing the interaction of memory regions with IOMMUs. Two use-
> cases are supported:
> 
>   1. Static mappings can be described by pairing the "iommu-addresses"
>  property with a "reg" property. This is mostly useful for adopting
>  firmware-allocated buffers via identity mappings. One common use-
>  case where this is required is if early firmware or bootloaders
>  have set up a bootsplash framebuffer that a display controller is
>  actively scanning out from during the operating system boot
>  process.
> 
>   2. If an "iommu-addresses" property exists without a "reg" property,
>  the reserved-memory node describes an IOVA reservation. Such memory
>  regions are excluded from the IOVA space available to operating
>  system drivers and can be used for regions that must not be used to
>  map arbitrary buffers.
> 
> Each mapping or reservation is tied to a specific device via a phandle
> to the device's device tree node. This allows a reserved-memory region
> to be reused across multiple devices.
> 
> Signed-off-by: Thierry Reding 
> ---
>  .../reserved-memory/reserved-memory.txt   |  1 -
>  .../reserved-memory/reserved-memory.yaml  | 62 +++
>  include/dt-bindings/reserved-memory.h |  8 +++
>  3 files changed, 70 insertions(+), 1 deletion(-)
>  delete mode 100644 
> Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
>  create mode 100644 include/dt-bindings/reserved-memory.h
> 
> diff --git 
> a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt 
> b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> deleted file mode 100644
> index 1810701a8509..
> --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> +++ /dev/null
> @@ -1 +0,0 @@
> -This file has been moved to reserved-memory.yaml.
> diff --git 
> a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml 
> b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
> index 7a0744052ff6..3a769aa66e1c 100644
> --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
> +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
> @@ -52,6 +52,30 @@ properties:
>Address and Length pairs. Specifies regions of memory that are
>acceptable to allocate from.
>  
> +  iommu-addresses:
> +$ref: /schemas/types.yaml#/definitions/phandle-array
> +description: >
> +  A list of phandle and specifier pairs that describe static IO virtual
> +  address space mappings and carveouts associated with a given reserved
> +  memory region. The phandle in the first cell refers to the device for
> +  which the mapping or carveout is to be created.
> +
> +  The specifier consists of an address/size pair and denotes the IO
> +  virtual address range of the region for the given device. The exact
> +  format depends on the values of the "#address-cells" and "#size-cells"
> +  properties of the device referenced via the phandle.
> +
> +  When used in combination with a "reg" property, an IOVA mapping is to
> +  be established for this memory region. One example where this can be
> +  useful is to create an identity mapping for physical memory that the
> +  firmware has configured some hardware to access (such as a bootsplash
> +  framebuffer).
> +
> +  If no "reg" property is specified, the "iommu-addresses" property
> +  defines carveout regions in the IOVA space for the given device. This
> +  can be useful if a certain memory region should not be mapped through
> +  the IOMMU.
> +
>no-map:
>  type: boolean
>  description: >
> @@ -97,4 +121,42 @@ oneOf:
>  
>  additionalProperties: true
>  
> +examples:
> +  - |
> +reserved-memory {
> +  #address-cells = <2>;
> +  #size-cells = <2>;
> +  ranges;
> +
> +  adsp: reservation-adsp {
> +/*
> + * Restrict IOVA mappings for ADSP buffers to the 512 MiB region
> + * from 0x4000 - 0x5fff. Anything outside is reserved by
> + * the ADSP for I/O memory and private memory allocations.
> + */
> +iommu-addresses = <0x0 0x 0x00 0x4000>,
> +  <0x0 0x6000 0xff 0xa000>;

This misses the device's phandle. One could argue it's not necessary for 
reservations but it will complicate the parsing code and the current 
parsing code is not prepared for it.

> +  };
> +
> +  fb: framebuffer@9000 {
> +reg = <0x0 0x9000 0x0 0x0080>;
> +iommu-addresses = < 0x0 0x9000 0x0 0x0080>;
> +  };
> +};
> +
> +bus@0 {
> +  #address-cells = <2>;
> +  #size-cells = <2>;
> +
> +  adsp@299 {
> +   

Re: [PATCH v5 0/5] iommu: Support mappings/reservations in reserved-memory regions

2022-05-15 Thread Janne Grunau
Hej,

I'm working on the display controller for Apple silicon SoCs and will 
add some comments with support for it in mind.

added as...@lists.linux.dev to CC for the Apple silicon related aspects

On 2022-05-12 21:00:47 +0200, Thierry Reding wrote:
> 
> this is another attempt at solving the problem of passing IOMMU
> configuration via device tree. It has significantly evolved since the
> last attempt, based on the discussion that followed. The discussion can
> be found here:
> 
>   
> https://lore.kernel.org/all/20210423163234.3651547-1-thierry.red...@gmail.com/
> 
> Rather than using a memory-region specifier, this new version introduces
> a new "iommu-addresses" property for the reserved-memory regions
> themselves.

If experimented with both proposed bindings for dcp and I think this 
binding is easer to understand and to work with.

> These are used to describe either a static mapping or
> reservation that should be created for a given device. If both "reg" and
> "iommu-addresses" properties are given, a mapping will be created
> (typically this would be an identity mapping)

dcp on Apple silicon will not use identity mappings. The IOMMU supports 
identity mapping but the pre-configured mappings setup by Apple's system 
firmware will not work with identity mapping. It maps multiple regions 
which are incompatible with a linear identity mapping. In addition the 
embbeded aarch64 micro controllers used in the display subsystem appears 
to use a heap mapped at low IOVA space starting at 0.

> whereas if only an "iommu-addresses" property is specified, a 
> reservation for the specified range will be installed.
> 
> An example is included in the DT bindings, but here is an extract of
> what I've used to test this:
> 
>   reserved-memory {
>   #address-cells = <2>;
>   #size-cells = <2>;
>   ranges;
> 
>   /*
>* Creates an identity mapping for the framebuffer that
>* the firmware has setup to scan out a bootsplash from.
>*/
>   fb: framebuffer@92cb2000 {
>   reg = <0x0 0x92cb2000 0x0 0x0080>;
>   iommu-addresses = < 0x0 0x92cb2000 0x0 0x0080>;
>   };

The binding supports mapping the same region to multiple devices. The 
code supports that and it will be used on Apple silicon. Not necessary 
to extend and complicate the example for I wanted to mention it 
explicitly.

> 
>   /*
>* Creates a reservation in the IOVA space to prevent
>* any buffers from being mapped to that region. Note
>* that on Tegra the range is actually quite different
>* from this, but it would conflict with the display
>* driver that I tested this against, so this is just
>* a dummy region for testing.
>*/
>   adsp: reservation-adsp {
>   iommu-addresses = < 0x0 0x9000 0x0 0x0001>;
>   };
>   };
> 
>   host1x@5000 {
>   dc@5420 {
>   memory-region = <>, <>;
>   };
>   };
> 
> This is abbreviated a little to focus on the essentials. Note also that
> the ADSP reservation is not actually used on this device and the driver
> for this doesn't exist yet, but I wanted to include this variant for
> testing, because we'll want to use these bindings for the reservation
> use-case as well at some point.
> 
> Adding Alyssa and Janne who have in the past tried to make these
> bindings work on Apple M1. Also adding Sameer from the Tegra audio team
> to look at the ADSP reservation and double-check that this is suitable
> for our needs.

The binding itself is sufficient for the needs of the display subsystem 
on Apple silicon. The device tree parsing code for reserved regions is 
of limited use in it's current form. We will have either to extend or 
duplicate it to retrieve the non-identity mappings. That's our problem 
to solve.

Janne
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu