Re: [PATCH v5 3/5] iommu: Add iommu_driver_get_domain_for_dev() helper

[Baolu Lu <baolu.lu@linux.intel.com>]

On 11/11/25 13:12, Nicolin Chen wrote:
> There is a need to stage a resetting PCI device to temporally the blocked
> domain and then attach back to its previously attached domain after reset.
> 
> This can be simply done by keeping the "previously attached domain" in the
> iommu_group->domain pointer while adding an iommu_group->resetting_domain,
> which gives troubles to IOMMU drivers using the iommu_get_domain_for_dev()
> for a device's physical domain in order to program IOMMU hardware.
> 
> And in such for-driver use cases, the iommu_group->mutex must be held, so
> it doesn't fit in external callers that don't hold the iommu_group->mutex.
> 
> Introduce a new iommu_driver_get_domain_for_dev() helper, exclusively for
> driver use cases that hold the iommu_group->mutex, to separate from those
> external use cases.
> 
> Add a lockdep_assert_not_held to the existing iommu_get_domain_for_dev()
> and highlight that in a kdoc.
> 
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> ---
>   include/linux/iommu.h                       |  1 +
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  5 ++--
>   drivers/iommu/iommu.c                       | 28 +++++++++++++++++++++
>   3 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 801b2bd9e8d49..a42a2d1d7a0b7 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -910,6 +910,7 @@ extern int iommu_attach_device(struct iommu_domain *domain,
>   extern void iommu_detach_device(struct iommu_domain *domain,
>   				struct device *dev);
>   extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
> +struct iommu_domain *iommu_driver_get_domain_for_dev(struct device *dev);
>   extern struct iommu_domain *iommu_get_dma_domain(struct device *dev);
>   extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
>   		     phys_addr_t paddr, size_t size, int prot, gfp_t gfp);
> 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 a33fbd12a0dd9..412d1a9b31275 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -3125,7 +3125,8 @@ int arm_smmu_set_pasid(struct arm_smmu_master *master,
>   		       struct arm_smmu_domain *smmu_domain, ioasid_t pasid,
>   		       struct arm_smmu_cd *cd, struct iommu_domain *old)
>   {
> -	struct iommu_domain *sid_domain = iommu_get_domain_for_dev(master->dev);
> +	struct iommu_domain *sid_domain =
> +		iommu_driver_get_domain_for_dev(master->dev);
>   	struct arm_smmu_attach_state state = {
>   		.master = master,
>   		.ssid = pasid,
> @@ -3191,7 +3192,7 @@ static int arm_smmu_blocking_set_dev_pasid(struct iommu_domain *new_domain,
>   	 */
>   	if (!arm_smmu_ssids_in_use(&master->cd_table)) {
>   		struct iommu_domain *sid_domain =
> -			iommu_get_domain_for_dev(master->dev);
> +			iommu_driver_get_domain_for_dev(master->dev);
>   
>   		if (sid_domain->type == IOMMU_DOMAIN_IDENTITY ||
>   		    sid_domain->type == IOMMU_DOMAIN_BLOCKED)
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 1e322f87b1710..1f4d6ca0937bc 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -2217,6 +2217,15 @@ void iommu_detach_device(struct iommu_domain *domain, struct device *dev)
>   }
>   EXPORT_SYMBOL_GPL(iommu_detach_device);
>   
> +/**
> + * iommu_get_domain_for_dev() - Return the DMA API domain pointer
> + * @dev - Device to query
> + *
> + * This function can be called within a driver bound to dev. The returned
> + * pointer is valid for the lifetime of the bound driver.
> + *
> + * It should not be called by drivers with driver_managed_dma = true.

"driver_managed_dma != true" means the driver will use the default
domain allocated by the iommu core during iommu probe. The iommu core
ensures that this domain stored at group->domain will not be changed
during the driver's whole lifecycle. That's reasonable.

How about making some code to enforce this requirement? Something like
below ...

> + */
>   struct iommu_domain *iommu_get_domain_for_dev(struct device *dev)
>   {
>   	/* Caller must be a probed driver on dev */
> @@ -2225,10 +2234,29 @@ struct iommu_domain *iommu_get_domain_for_dev(struct device *dev)
>   	if (!group)
>   		return NULL;
>   
> +	lockdep_assert_not_held(&group->mutex);

...
	if (WARN_ON(!dev->driver || !group->owner_cnt || group->owner))
		return NULL;

> +
>   	return group->domain;
>   }
>   EXPORT_SYMBOL_GPL(iommu_get_domain_for_dev);
>   
> +/**
> + * iommu_driver_get_domain_for_dev() - Return the driver-level domain pointer
> + * @dev - Device to query
> + *
> + * This function can be called by an iommu driver that wants to get the physical
> + * domain within an iommu callback function where group->mutex is held.
> + */
> +struct iommu_domain *iommu_driver_get_domain_for_dev(struct device *dev)
> +{
> +	struct iommu_group *group = dev->iommu_group;
> +
> +	lockdep_assert_held(&group->mutex);
> +
> +	return group->domain;
> +}
> +EXPORT_SYMBOL_GPL(iommu_driver_get_domain_for_dev);
> +
>   /*
>    * For IOMMU_DOMAIN_DMA implementations which already provide their own
>    * guarantees that the group and its default domain are valid and correct.

Others look good to me.

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