Re: [PATCH 01/12] iommu/vt-d: Use iommu_get_domain_for_dev() in debugfs

[Jason Gunthorpe]

On Fri, May 27, 2022 at 02:30:08PM +0800, Lu Baolu wrote:
> Retrieve the attached domain for a device through the generic interface
> exposed by the iommu core. This also makes device_domain_lock static.
> 
> Signed-off-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
>  drivers/iommu/intel/iommu.h   |  1 -
>  drivers/iommu/intel/debugfs.c | 20 ++++++++------------
>  drivers/iommu/intel/iommu.c   |  2 +-
>  3 files changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
> index a22adfbdf870..8a6d64d726c0 100644
> +++ b/drivers/iommu/intel/iommu.h
> @@ -480,7 +480,6 @@ enum {
>  #define VTD_FLAG_SVM_CAPABLE		(1 << 2)
>  
>  extern int intel_iommu_sm;
> -extern spinlock_t device_domain_lock;
>  
>  #define sm_supported(iommu)	(intel_iommu_sm && ecap_smts((iommu)->ecap))
>  #define pasid_supported(iommu)	(sm_supported(iommu) &&			\
> diff --git a/drivers/iommu/intel/debugfs.c b/drivers/iommu/intel/debugfs.c
> index d927ef10641b..eea8727aa7bc 100644
> +++ b/drivers/iommu/intel/debugfs.c
> @@ -344,19 +344,21 @@ static void pgtable_walk_level(struct seq_file *m, struct dma_pte *pde,
>  
>  static int show_device_domain_translation(struct device *dev, void *data)
>  {
> -	struct device_domain_info *info = dev_iommu_priv_get(dev);
> -	struct dmar_domain *domain = info->domain;
> +	struct dmar_domain *dmar_domain;
> +	struct iommu_domain *domain;
>  	struct seq_file *m = data;
>  	u64 path[6] = { 0 };
>  
> +	domain = iommu_get_domain_for_dev(dev);
>  	if (!domain)
>  		return 0;

The iommu_get_domain_for_dev() API should be called something like
'iommu_get_dma_api_domain()' and clearly documented that it is safe to
call only so long as a DMA API using driver is attached to the device,
which is most of the current callers.

This use in random sysfs inside the iommu driver is not OK because it
doesn't have any locking protecting domain from concurrent free.

Jason