[PATCH v6 6/6] iommu/amd: Fail probe on ATS configuration failure

[Ankit Soni]

On Fri, May 29, 2026 at 11:12:08AM +0000, Pranjal Shrivastava wrote:
> Update the AMD IOMMU driver to handle ATS configuration and enablement
> more strictly. Specifically, update the device probe to fail if
> pci_prepare_ats() returns an error. This ensures that any ATS-capable
> master reaching the attach phase is guaranteed to have a valid config.
> 
> Additionally, update pdev_enable_cap_ats() to WARN_ON() if pci_enable_ats
> fails. Since earlier checks in the probe phase preclude config-related
> failures, any failure during hardware enablement is considered a kernel
> bug.
> 
> Fix a pre-existing Use-After-Free risk by ensuring iommu_ignore_device()
> is called on all probe failures after iommu_init_device().
> 
> Signed-off-by: Pranjal Shrivastava <praan at google.com>
> ---
>  drivers/iommu/amd/iommu.c | 30 ++++++++++++++++++++++++------
>  1 file changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 84cad43dc188..b74c4504bee3 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -570,10 +570,16 @@ static inline int pdev_enable_cap_ats(struct pci_dev *pdev)
>  	if (amd_iommu_iotlb_sup &&
>  	    (dev_data->flags & AMD_IOMMU_DEVICE_FLAG_ATS_SUP)) {
>  		ret = pci_enable_ats(pdev, PAGE_SHIFT);
> -		if (!ret) {
> -			dev_data->ats_enabled = 1;
> -			dev_data->ats_qdep    = pci_ats_queue_depth(pdev);
> -		}
> +		/*
> +		 * pci_enable_ats() should not fail here because earlier checks
> +		 * have already verified support and configuration.
> +		 */
> +		if (WARN_ON(ret))
> +			return ret;
> +
> +		dev_data->ats_enabled = 1;
> +		dev_data->ats_qdep    = pci_ats_queue_depth(pdev);
> +		ret = 0;
>  	}
>  
>  	return ret;
> @@ -2502,10 +2508,22 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)
>  	else
>  		dev_data->max_irqs = MAX_IRQS_PER_TABLE_512;
>  
> -	if (dev_is_pci(dev))
> -		pci_prepare_ats(to_pci_dev(dev), PAGE_SHIFT);
> +	if (dev_is_pci(dev)) {
> +		struct pci_dev *pdev = to_pci_dev(dev);
> +
> +		if (pci_ats_supported(pdev)) {
> +			ret = pci_prepare_ats(pdev, PAGE_SHIFT);
> +			if (ret) {
> +				iommu_dev = ERR_PTR(ret);
> +				goto out_err;
> +			}
> +		}
> +	}
>  
>  out_err:
> +	if (IS_ERR(iommu_dev))
> +		iommu_ignore_device(iommu, dev);
> +
>  	return iommu_dev;
>  }
>  

Hi,
This regresses IRQ remapping in the PD_MODE_NONE branch. By design
rlookup_table[devid] must stay valid for IR - init.c:2257 documents
this: "Do not return an error to enable IRQ remapping ...". Pre-patch
the PD_MODE_NONE branch returned ERR_PTR(-ENODEV) without nulling
rlookup, precisely so irq_remapping_alloc() / __rlookup_amd_iommu()
keep working; this unconditional cleanup violates that.
The new pci_prepare_ats() failure path has the same shape:
amd_iommu_set_pci_msi_domain() ran earlier and parented dev->msi_domain
on iommu->ir_domain, but on this new out_err that's not unwound. So
nulling rlookup_table[devid] makes irq_remapping_alloc() return -EINVAL
on the first MSI alloc for the device. Sashiko also flagged this in [1];

Also if iommu_init_device() branch fails, iommu_ignore_device() will be
called twice.

[1] https://lore.kernel.org/r/20260529153216.2AD1E1F00899@smtp.kernel.org

-Ankit

> -- 
> 2.54.0.823.g6e5bcc1fc9-goog
>