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

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

On 8/24/2026 5:53 PM, 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>

One minor nit. Otherwise patch looks good.

Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>



> ---
>  drivers/iommu/amd/iommu.c | 117 +++++++++++++++++++-------------------
>  1 file changed, 60 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 4c31294fabc5..e1f9fbb63837 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)


.../...

>  static struct iommu_device *amd_iommu_probe_device(struct device *dev)
>  {
>  	struct iommu_device *iommu_dev;
>  	struct amd_iommu *iommu;
>  	struct iommu_dev_data *dev_data;
>  	int ret;
> +	u16 devid;
>  
> -	if (!check_device(dev))
> -		return ERR_PTR(-ENODEV);
> -
> -	iommu = rlookup_amd_iommu(dev);
> -	if (!iommu)
> -		return ERR_PTR(-ENODEV);
> -
> -	/* Not registered yet? */
> -	if (!iommu->iommu.ops)
> +	if (!iommu_lookup_device(dev, &iommu, &devid))
>  		return ERR_PTR(-ENODEV);
>  
>  	if (dev_iommu_priv_get(dev))
>  		return &iommu->iommu;
>  
> -	ret = iommu_init_device(iommu, dev);
> -	if (ret) {
> +	dev_data = iommu_init_device(iommu, dev, devid);
> +	if (IS_ERR(dev_data)) {
> +		ret = PTR_ERR(dev_data);

Instead of this, you can change below iommu_Dev assignment directly.

-Vasant

>  		dev_err(dev, "Failed to initialize - trying to proceed anyway\n");
>  		iommu_dev = ERR_PTR(ret);
>  		iommu_ignore_device(iommu, dev);
>  		goto out_err;
>  	}
>  
> -	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.
> +	 * 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.
>  	 */
> -	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)));
> -	}
> -
>  	if (amd_iommu_pgtable == PD_MODE_NONE) {
>  		pr_warn_once("%s: DMA translation not supported by iommu.\n",
>  			     __func__);
> @@ -2527,16 +2540,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;
>  }