[PATCH v3 3/3] iommu: Drop sw_msi from iommu_domain

Nicolin Chen posted 3 patches 11 months, 1 week ago
There is a newer version of this series
[PATCH v3 3/3] iommu: Drop sw_msi from iommu_domain
Posted by Nicolin Chen 11 months, 1 week ago
There are only two sw_msi implementations in the entire system, thus it's
not very necessary to have an sw_msi pointer.

Instead, check domain->private_data_owner to call the two implementations
directly from the core code.

Suggested-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/dma-iommu.h            |  9 +++++++++
 include/linux/iommu.h                | 15 ---------------
 drivers/iommu/dma-iommu.c            | 14 ++------------
 drivers/iommu/iommu.c                | 17 +++++++++++++++--
 drivers/iommu/iommufd/hw_pagetable.c |  3 ---
 5 files changed, 26 insertions(+), 32 deletions(-)

diff --git a/drivers/iommu/dma-iommu.h b/drivers/iommu/dma-iommu.h
index 9cca11806e5d..eca201c1f963 100644
--- a/drivers/iommu/dma-iommu.h
+++ b/drivers/iommu/dma-iommu.h
@@ -19,6 +19,9 @@ int iommu_dma_init_fq(struct iommu_domain *domain);
 
 void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list);
 
+int iommu_dma_sw_msi(struct iommu_domain *domain, struct msi_desc *desc,
+		     phys_addr_t msi_addr);
+
 extern bool iommu_dma_forcedac;
 
 #else /* CONFIG_IOMMU_DMA */
@@ -49,5 +52,11 @@ static inline void iommu_dma_get_resv_regions(struct device *dev, struct list_he
 {
 }
 
+static inline int iommu_dma_sw_msi(struct iommu_domain *domain,
+				   struct msi_desc *desc, phys_addr_t msi_addr)
+{
+	return -ENODEV;
+}
+
 #endif	/* CONFIG_IOMMU_DMA */
 #endif	/* __DMA_IOMMU_H */
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 06cc14e9993d..e01c855ae8a7 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -229,11 +229,6 @@ struct iommu_domain {
 	struct iommu_domain_geometry geometry;
 	int (*iopf_handler)(struct iopf_group *group);
 
-#if IS_ENABLED(CONFIG_IRQ_MSI_IOMMU)
-	int (*sw_msi)(struct iommu_domain *domain, struct msi_desc *desc,
-		      phys_addr_t msi_addr);
-#endif
-
 	union { /* cookie */
 		struct iommu_dma_cookie *iova_cookie;
 		struct iommu_dma_msi_cookie *msi_cookie;
@@ -254,16 +249,6 @@ struct iommu_domain {
 	};
 };
 
-static inline void iommu_domain_set_sw_msi(
-	struct iommu_domain *domain,
-	int (*sw_msi)(struct iommu_domain *domain, struct msi_desc *desc,
-		      phys_addr_t msi_addr))
-{
-#if IS_ENABLED(CONFIG_IRQ_MSI_IOMMU)
-	domain->sw_msi = sw_msi;
-#endif
-}
-
 static inline bool iommu_is_dma_domain(struct iommu_domain *domain)
 {
 	return domain->type & __IOMMU_DOMAIN_DMA_API;
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index bc9bb9cb70c8..96e9f8d0a4f6 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -94,9 +94,6 @@ static int __init iommu_dma_forcedac_setup(char *str)
 }
 early_param("iommu.forcedac", iommu_dma_forcedac_setup);
 
-static int iommu_dma_sw_msi(struct iommu_domain *domain, struct msi_desc *desc,
-			    phys_addr_t msi_addr);
-
 /* Number of entries per flush queue */
 #define IOVA_DEFAULT_FQ_SIZE	256
 #define IOVA_SINGLE_FQ_SIZE	32768
@@ -377,7 +374,6 @@ int iommu_get_dma_cookie(struct iommu_domain *domain)
 
 	mutex_init(&cookie->mutex);
 	INIT_LIST_HEAD(&cookie->msi_page_list);
-	iommu_domain_set_sw_msi(domain, iommu_dma_sw_msi);
 	domain->cookie_type = IOMMU_COOKIE_DMA_IOVA;
 	domain->iova_cookie = cookie;
 	return 0;
@@ -411,7 +407,6 @@ int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
 
 	cookie->msi_iova = base;
 	INIT_LIST_HEAD(&cookie->msi_page_list);
-	iommu_domain_set_sw_msi(domain, iommu_dma_sw_msi);
 	domain->cookie_type = IOMMU_COOKIE_DMA_MSI;
 	domain->msi_cookie = cookie;
 	return 0;
@@ -427,11 +422,6 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
 	struct iommu_dma_cookie *cookie = domain->iova_cookie;
 	struct iommu_dma_msi_page *msi, *tmp;
 
-#if IS_ENABLED(CONFIG_IRQ_MSI_IOMMU)
-	if (domain->sw_msi != iommu_dma_sw_msi)
-		return;
-#endif
-
 	if (cookie->iovad.granule) {
 		iommu_dma_free_fq(cookie);
 		put_iova_domain(&cookie->iovad);
@@ -1825,8 +1815,8 @@ static struct iommu_dma_msi_page *iommu_dma_get_msi_page(struct device *dev,
 	return NULL;
 }
 
-static int iommu_dma_sw_msi(struct iommu_domain *domain, struct msi_desc *desc,
-			    phys_addr_t msi_addr)
+int iommu_dma_sw_msi(struct iommu_domain *domain, struct msi_desc *desc,
+		     phys_addr_t msi_addr)
 {
 	struct device *dev = msi_desc_to_dev(desc);
 	const struct iommu_dma_msi_page *msi_page;
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 8b9423340221..c1c0656f41ef 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -18,6 +18,7 @@
 #include <linux/errno.h>
 #include <linux/host1x_context_bus.h>
 #include <linux/iommu.h>
+#include <linux/iommufd.h>
 #include <linux/idr.h>
 #include <linux/err.h>
 #include <linux/pci.h>
@@ -3649,8 +3650,20 @@ int iommu_dma_prepare_msi(struct msi_desc *desc, phys_addr_t msi_addr)
 		return 0;
 
 	mutex_lock(&group->mutex);
-	if (group->domain && group->domain->sw_msi)
-		ret = group->domain->sw_msi(group->domain, desc, msi_addr);
+	if (group->domain) {
+		switch (group->domain->cookie_type) {
+		case IOMMU_COOKIE_DMA_MSI:
+		case IOMMU_COOKIE_DMA_IOVA:
+			ret = iommu_dma_sw_msi(group->domain, desc, msi_addr);
+			break;
+		case IOMMU_COOKIE_IOMMUFD:
+			ret = iommufd_sw_msi(group->domain, desc, msi_addr);
+			break;
+		default:
+			ret = -EOPNOTSUPP;
+			break;
+		}
+	}
 	mutex_unlock(&group->mutex);
 	return ret;
 }
diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c
index 227514030655..98aecf904902 100644
--- a/drivers/iommu/iommufd/hw_pagetable.c
+++ b/drivers/iommu/iommufd/hw_pagetable.c
@@ -156,7 +156,6 @@ iommufd_hwpt_paging_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
 			goto out_abort;
 		}
 	}
-	iommu_domain_set_sw_msi(hwpt->domain, iommufd_sw_msi);
 
 	/*
 	 * Set the coherency mode before we do iopt_table_add_domain() as some
@@ -252,7 +251,6 @@ iommufd_hwpt_nested_alloc(struct iommufd_ctx *ictx,
 		goto out_abort;
 	}
 	hwpt->domain->owner = ops;
-	iommu_domain_set_sw_msi(hwpt->domain, iommufd_sw_msi);
 
 	if (WARN_ON_ONCE(hwpt->domain->type != IOMMU_DOMAIN_NESTED)) {
 		rc = -EINVAL;
@@ -309,7 +307,6 @@ iommufd_viommu_alloc_hwpt_nested(struct iommufd_viommu *viommu, u32 flags,
 		goto out_abort;
 	}
 	hwpt->domain->owner = viommu->iommu_dev->ops;
-	iommu_domain_set_sw_msi(hwpt->domain, iommufd_sw_msi);
 
 	if (WARN_ON_ONCE(hwpt->domain->type != IOMMU_DOMAIN_NESTED)) {
 		rc = -EINVAL;
-- 
2.43.0
RE: [PATCH v3 3/3] iommu: Drop sw_msi from iommu_domain
Posted by Tian, Kevin 11 months, 1 week ago
> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: Tuesday, March 4, 2025 4:53 AM
> 
> There are only two sw_msi implementations in the entire system, thus it's
> not very necessary to have an sw_msi pointer.
> 
> Instead, check domain->private_data_owner to call the two implementations
> directly from the core code.

s/private_data_owner/cookie_type/

> 
> Suggested-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Re: [PATCH v3 3/3] iommu: Drop sw_msi from iommu_domain
Posted by Robin Murphy 11 months, 1 week ago
On 03/03/2025 8:52 pm, Nicolin Chen wrote:
> There are only two sw_msi implementations in the entire system, thus it's
> not very necessary to have an sw_msi pointer.
> 
> Instead, check domain->private_data_owner to call the two implementations
> directly from the core code.

FWIW I still wish this was squashed into the pending patches so we're 
not adding stuff we immediately remove again, but oh well, what's done 
is done.

Reviewed-by: Robin Murphy <robin.murphy@arm.com>

(assuming this isn't also queued already given that once again I've had 
the audacity to not look at my inbox until after 2PM the day after it 
was posted...)

> Suggested-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> ---
>   drivers/iommu/dma-iommu.h            |  9 +++++++++
>   include/linux/iommu.h                | 15 ---------------
>   drivers/iommu/dma-iommu.c            | 14 ++------------
>   drivers/iommu/iommu.c                | 17 +++++++++++++++--
>   drivers/iommu/iommufd/hw_pagetable.c |  3 ---
>   5 files changed, 26 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/iommu/dma-iommu.h b/drivers/iommu/dma-iommu.h
> index 9cca11806e5d..eca201c1f963 100644
> --- a/drivers/iommu/dma-iommu.h
> +++ b/drivers/iommu/dma-iommu.h
> @@ -19,6 +19,9 @@ int iommu_dma_init_fq(struct iommu_domain *domain);
>   
>   void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list);
>   
> +int iommu_dma_sw_msi(struct iommu_domain *domain, struct msi_desc *desc,
> +		     phys_addr_t msi_addr);
> +
>   extern bool iommu_dma_forcedac;
>   
>   #else /* CONFIG_IOMMU_DMA */
> @@ -49,5 +52,11 @@ static inline void iommu_dma_get_resv_regions(struct device *dev, struct list_he
>   {
>   }
>   
> +static inline int iommu_dma_sw_msi(struct iommu_domain *domain,
> +				   struct msi_desc *desc, phys_addr_t msi_addr)
> +{
> +	return -ENODEV;
> +}
> +
>   #endif	/* CONFIG_IOMMU_DMA */
>   #endif	/* __DMA_IOMMU_H */
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 06cc14e9993d..e01c855ae8a7 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -229,11 +229,6 @@ struct iommu_domain {
>   	struct iommu_domain_geometry geometry;
>   	int (*iopf_handler)(struct iopf_group *group);
>   
> -#if IS_ENABLED(CONFIG_IRQ_MSI_IOMMU)
> -	int (*sw_msi)(struct iommu_domain *domain, struct msi_desc *desc,
> -		      phys_addr_t msi_addr);
> -#endif
> -
>   	union { /* cookie */
>   		struct iommu_dma_cookie *iova_cookie;
>   		struct iommu_dma_msi_cookie *msi_cookie;
> @@ -254,16 +249,6 @@ struct iommu_domain {
>   	};
>   };
>   
> -static inline void iommu_domain_set_sw_msi(
> -	struct iommu_domain *domain,
> -	int (*sw_msi)(struct iommu_domain *domain, struct msi_desc *desc,
> -		      phys_addr_t msi_addr))
> -{
> -#if IS_ENABLED(CONFIG_IRQ_MSI_IOMMU)
> -	domain->sw_msi = sw_msi;
> -#endif
> -}
> -
>   static inline bool iommu_is_dma_domain(struct iommu_domain *domain)
>   {
>   	return domain->type & __IOMMU_DOMAIN_DMA_API;
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index bc9bb9cb70c8..96e9f8d0a4f6 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -94,9 +94,6 @@ static int __init iommu_dma_forcedac_setup(char *str)
>   }
>   early_param("iommu.forcedac", iommu_dma_forcedac_setup);
>   
> -static int iommu_dma_sw_msi(struct iommu_domain *domain, struct msi_desc *desc,
> -			    phys_addr_t msi_addr);
> -
>   /* Number of entries per flush queue */
>   #define IOVA_DEFAULT_FQ_SIZE	256
>   #define IOVA_SINGLE_FQ_SIZE	32768
> @@ -377,7 +374,6 @@ int iommu_get_dma_cookie(struct iommu_domain *domain)
>   
>   	mutex_init(&cookie->mutex);
>   	INIT_LIST_HEAD(&cookie->msi_page_list);
> -	iommu_domain_set_sw_msi(domain, iommu_dma_sw_msi);
>   	domain->cookie_type = IOMMU_COOKIE_DMA_IOVA;
>   	domain->iova_cookie = cookie;
>   	return 0;
> @@ -411,7 +407,6 @@ int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
>   
>   	cookie->msi_iova = base;
>   	INIT_LIST_HEAD(&cookie->msi_page_list);
> -	iommu_domain_set_sw_msi(domain, iommu_dma_sw_msi);
>   	domain->cookie_type = IOMMU_COOKIE_DMA_MSI;
>   	domain->msi_cookie = cookie;
>   	return 0;
> @@ -427,11 +422,6 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
>   	struct iommu_dma_cookie *cookie = domain->iova_cookie;
>   	struct iommu_dma_msi_page *msi, *tmp;
>   
> -#if IS_ENABLED(CONFIG_IRQ_MSI_IOMMU)
> -	if (domain->sw_msi != iommu_dma_sw_msi)
> -		return;
> -#endif
> -
>   	if (cookie->iovad.granule) {
>   		iommu_dma_free_fq(cookie);
>   		put_iova_domain(&cookie->iovad);
> @@ -1825,8 +1815,8 @@ static struct iommu_dma_msi_page *iommu_dma_get_msi_page(struct device *dev,
>   	return NULL;
>   }
>   
> -static int iommu_dma_sw_msi(struct iommu_domain *domain, struct msi_desc *desc,
> -			    phys_addr_t msi_addr)
> +int iommu_dma_sw_msi(struct iommu_domain *domain, struct msi_desc *desc,
> +		     phys_addr_t msi_addr)
>   {
>   	struct device *dev = msi_desc_to_dev(desc);
>   	const struct iommu_dma_msi_page *msi_page;
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 8b9423340221..c1c0656f41ef 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -18,6 +18,7 @@
>   #include <linux/errno.h>
>   #include <linux/host1x_context_bus.h>
>   #include <linux/iommu.h>
> +#include <linux/iommufd.h>
>   #include <linux/idr.h>
>   #include <linux/err.h>
>   #include <linux/pci.h>
> @@ -3649,8 +3650,20 @@ int iommu_dma_prepare_msi(struct msi_desc *desc, phys_addr_t msi_addr)
>   		return 0;
>   
>   	mutex_lock(&group->mutex);
> -	if (group->domain && group->domain->sw_msi)
> -		ret = group->domain->sw_msi(group->domain, desc, msi_addr);
> +	if (group->domain) {
> +		switch (group->domain->cookie_type) {
> +		case IOMMU_COOKIE_DMA_MSI:
> +		case IOMMU_COOKIE_DMA_IOVA:
> +			ret = iommu_dma_sw_msi(group->domain, desc, msi_addr);
> +			break;
> +		case IOMMU_COOKIE_IOMMUFD:
> +			ret = iommufd_sw_msi(group->domain, desc, msi_addr);
> +			break;
> +		default:
> +			ret = -EOPNOTSUPP;
> +			break;
> +		}
> +	}
>   	mutex_unlock(&group->mutex);
>   	return ret;
>   }
> diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c
> index 227514030655..98aecf904902 100644
> --- a/drivers/iommu/iommufd/hw_pagetable.c
> +++ b/drivers/iommu/iommufd/hw_pagetable.c
> @@ -156,7 +156,6 @@ iommufd_hwpt_paging_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
>   			goto out_abort;
>   		}
>   	}
> -	iommu_domain_set_sw_msi(hwpt->domain, iommufd_sw_msi);
>   
>   	/*
>   	 * Set the coherency mode before we do iopt_table_add_domain() as some
> @@ -252,7 +251,6 @@ iommufd_hwpt_nested_alloc(struct iommufd_ctx *ictx,
>   		goto out_abort;
>   	}
>   	hwpt->domain->owner = ops;
> -	iommu_domain_set_sw_msi(hwpt->domain, iommufd_sw_msi);
>   
>   	if (WARN_ON_ONCE(hwpt->domain->type != IOMMU_DOMAIN_NESTED)) {
>   		rc = -EINVAL;
> @@ -309,7 +307,6 @@ iommufd_viommu_alloc_hwpt_nested(struct iommufd_viommu *viommu, u32 flags,
>   		goto out_abort;
>   	}
>   	hwpt->domain->owner = viommu->iommu_dev->ops;
> -	iommu_domain_set_sw_msi(hwpt->domain, iommufd_sw_msi);
>   
>   	if (WARN_ON_ONCE(hwpt->domain->type != IOMMU_DOMAIN_NESTED)) {
>   		rc = -EINVAL;