Re: [PATCH v2 1/5] iommu/amd: Refactor device probe and capability initialization

[Vasant Hegde <vasant.hegde@amd.com>]

Pranjal,


On 8/14/2026 7:26 AM, Pranjal Shrivastava wrote:
> Restructure the device probe path to improve readability and prepare for
> cleaner error handling. Refactor check_device() into iommu_lookup_device
> to explicitly validate and return the amd_iommu ptr & devid. Refactor
> iommu_init_device() to return the allocated dev_data. Consolidate all
> PCI cap inits (MSI domains, PASID, ATS) into a new helper:
> iommu_init_device_caps().
> 
> Suggested-by: Vasant Hegde <vasant.hegde@amd.com>
> Signed-off-by: Pranjal Shrivastava <praan@google.com>
> ---
>  drivers/iommu/amd/iommu.c | 114 +++++++++++++++++++-------------------
>  1 file changed, 56 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 4c31294fabc5..9b8ad131ba79 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -675,7 +675,8 @@ static void pdev_disable_caps(struct pci_dev *pdev)


.../...

> -	amd_iommu_set_pci_msi_domain(dev, iommu);
> +	iommu_init_device_caps(dev_data, dev, iommu);
>  	iommu_dev = &iommu->iommu;
>  
> -	/*
> -	 * If IOMMU and device supports PASID then it will contain max
> -	 * supported PASIDs, else it will be zero.
> -	 */
> -	dev_data = dev_iommu_priv_get(dev);
> -	if (amd_iommu_pasid_supported() && dev_is_pci(dev) &&
> -	    pdev_pasid_supported(dev_data)) {
> -		dev_data->max_pasids = min_t(u32, iommu->iommu.max_pasids,
> -					     pci_max_pasids(to_pci_dev(dev)));
> -	}
> -

Can you add something like below comment so that we know why we are not calling
iommu_ignore_device in error path.

+        * When DMA translation is unavailable return error so the iommu core
+        * won't attempt domain attach for this device. But interrupt-remap
+        * is still supported. Hence do not ignore the device.

-Vasant

>  	if (amd_iommu_pgtable == PD_MODE_NONE) {
>  		pr_warn_once("%s: DMA translation not supported by iommu.\n",
>  			     __func__);
> @@ -2527,16 +2535,6 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)
>  		goto out_err;
>  	}
>  
> -	iommu_completion_wait(iommu);
> -
> -	if (FEATURE_NUM_INT_REMAP_SUP_2K(amd_iommu_efr2))
> -		dev_data->max_irqs = MAX_IRQS_PER_TABLE_2K;
> -	else
> -		dev_data->max_irqs = MAX_IRQS_PER_TABLE_512;
> -
> -	if (dev_is_pci(dev))
> -		pci_prepare_ats(to_pci_dev(dev), PAGE_SHIFT);
> -
>  out_err:
>  	return iommu_dev;
>  }