Thread (67 messages) 67 messages, 2 authors, 2023-08-16

Re: [PATCH v6 02/25] iommu: Add IOMMU_DOMAIN_PLATFORM

From: Baolu Lu <baolu.lu@linux.intel.com>
Date: 2023-08-12 01:36:49
Also in: linux-arm-kernel, linux-arm-msm, linux-iommu, linux-mediatek, linux-rockchip, linux-s390, linux-samsung-soc, linux-sunxi, linux-tegra

On 2023/8/3 8:07, Jason Gunthorpe wrote:
quoted hunk ↗ jump to hunk
This is used when the iommu driver is taking control of the dma_ops,
currently only on S390 and power spapr. It is designed to preserve the
original ops->detach_dev() semantic that these S390 was built around.

Provide an opaque domain type and a 'default_domain' ops value that allows
the driver to trivially force any single domain as the default domain.

Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
  drivers/iommu/iommu.c | 14 +++++++++++++-
  include/linux/iommu.h |  6 ++++++
  2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 5e3cdc9f3a9e78..c64365169b678d 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1705,6 +1705,17 @@ iommu_group_alloc_default_domain(struct iommu_group *group, int req_type)
  
  	lockdep_assert_held(&group->mutex);
  
+	/*
+	 * Allow legacy drivers to specify the domain that will be the default
+	 * domain. This should always be either an IDENTITY or PLATFORM domain.
+	 * Do not use in new drivers.
+	 */
+	if (bus->iommu_ops->default_domain) {
+		if (req_type)
+			return ERR_PTR(-EINVAL);
+		return bus->iommu_ops->default_domain;
+	}
+
  	if (req_type)
  		return __iommu_group_alloc_default_domain(bus, group, req_type);
  
@@ -1967,7 +1978,8 @@ void iommu_domain_free(struct iommu_domain *domain)
  	if (domain->type == IOMMU_DOMAIN_SVA)
  		mmdrop(domain->mm);
  	iommu_put_dma_cookie(domain);
-	domain->ops->free(domain);
+	if (domain->ops->free)
+		domain->ops->free(domain);
  }
  EXPORT_SYMBOL_GPL(iommu_domain_free);
  
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index e05c93b6c37fba..87aebba474e093 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -64,6 +64,7 @@ struct iommu_domain_geometry {
  #define __IOMMU_DOMAIN_DMA_FQ	(1U << 3)  /* DMA-API uses flush queue    */
  
  #define __IOMMU_DOMAIN_SVA	(1U << 4)  /* Shared process address space */
+#define __IOMMU_DOMAIN_PLATFORM	(1U << 5)
  
  #define IOMMU_DOMAIN_ALLOC_FLAGS ~__IOMMU_DOMAIN_DMA_FQ
  /*
@@ -81,6 +82,8 @@ struct iommu_domain_geometry {
   *				  invalidation.
   *	IOMMU_DOMAIN_SVA	- DMA addresses are shared process addresses
   *				  represented by mm_struct's.
+ *	IOMMU_DOMAIN_PLATFORM	- Legacy domain for drivers that do their own
+ *				  dma_api stuff. Do not use in new drivers.
   */
  #define IOMMU_DOMAIN_BLOCKED	(0U)
  #define IOMMU_DOMAIN_IDENTITY	(__IOMMU_DOMAIN_PT)
@@ -91,6 +94,7 @@ struct iommu_domain_geometry {
  				 __IOMMU_DOMAIN_DMA_API |	\
  				 __IOMMU_DOMAIN_DMA_FQ)
  #define IOMMU_DOMAIN_SVA	(__IOMMU_DOMAIN_SVA)
+#define IOMMU_DOMAIN_PLATFORM	(__IOMMU_DOMAIN_PLATFORM)
  
  struct iommu_domain {
  	unsigned type;
@@ -256,6 +260,7 @@ struct iommu_iotlb_gather {
   * @owner: Driver module providing these ops
   * @identity_domain: An always available, always attachable identity
   *                   translation.
+ * @default_domain: If not NULL this will always be set as the default domain.
   */
  struct iommu_ops {
  	bool (*capable)(struct device *dev, enum iommu_cap);
@@ -290,6 +295,7 @@ struct iommu_ops {
  	unsigned long pgsize_bitmap;
  	struct module *owner;
  	struct iommu_domain *identity_domain;
+	struct iommu_domain *default_domain;
I am imaging whether we can merge above two pointers into a single one.
It is either an IDENTITY or PLATFORM domain and the core will choose it
as the default domain of a group if iommu_group_alloc_default_domain()
fails to allocate one through the iommu dev_ops.

Those iommu drivers that could result in a NULL default domain could
provide such domain and guarantee that this domain is always usable for
devices.

Probably we could give it a more meaningful name? For example,
supplemental_domain or rescue_domain?

I am not sure whether this can address the NULL-default-domain issues
of all drivers this series tries to address. So just for discussion
purpose.

Best regards,
baolu
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help