Re: [PATCH v5 6/7] iommu/vt-d: Fail probe on ATS configuration failure

[Pranjal Shrivastava <praan@google.com>]

On Fri, May 29, 2026 at 02:39:26PM +0800, Baolu Lu wrote:
> On 5/29/26 04:23, Pranjal Shrivastava wrote:
> > Update the Intel VT-d 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 iommu_enable_pci_ats() to WARN() 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.
> > 
> > Signed-off-by: Pranjal Shrivastava <praan@google.com>
> > ---
> >   drivers/iommu/intel/iommu.c | 15 ++++++++++++---
> >   1 file changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> > index ed6d3a0203f5..f13da16717fe 100644
> > --- a/drivers/iommu/intel/iommu.c
> > +++ b/drivers/iommu/intel/iommu.c
> > @@ -876,8 +876,14 @@ static void iommu_enable_pci_ats(struct device_domain_info *info)
> >   	if (!pci_ats_page_aligned(pdev))
> >   		return;
> > -	if (!pci_enable_ats(pdev, VTD_PAGE_SHIFT))
> > -		info->ats_enabled = 1;
> > +	/*
> > +	 * pci_enable_ats() should not fail here because earlier checks
> > +	 * have already verified support and configuration.
> > +	 */
> > +	if (WARN_ON(pci_enable_ats(pdev, VTD_PAGE_SHIFT)))
> > +		return;
> > +
> > +	info->ats_enabled = 1;
> >   }
> >   static void iommu_disable_pci_ats(struct device_domain_info *info)
> > @@ -3292,7 +3298,10 @@ static struct iommu_device *intel_iommu_probe_device(struct device *dev)
> >   	dev_iommu_priv_set(dev, info);
> >   	if (pdev && pci_ats_supported(pdev)) {
> > -		pci_prepare_ats(pdev, VTD_PAGE_SHIFT);
> > +		ret = pci_prepare_ats(pdev, VTD_PAGE_SHIFT);
> > +		if (ret)
> > +			goto free;
> > +
> >   		ret = device_rbtree_insert(iommu, info);
> >   		if (ret)
> >   			goto free;
> 
> Sashiko made a valuable review comment, and I believe it applies here as
> well:
> 
> [Severity: High]
> Since ATS is an optional performance optimization, does failing the
> IOMMU probe when pci_prepare_ats() fails break backward compatibility?
> 
> This completely prevents devices with buggy ATS capabilities (or VF/PF
> STU mismatches) from attaching to the IOMMU.
> 
> Could this disable DMA translation entirely for hardware that would
> otherwise work correctly without ATS?

Ack. We explored the "not failing probe" strategy in v3, but following
discussion with Jason, we moved toward this "Fail Hard" policy [1]

The rationale is that we ONLY enable ATS IFF ats_is_supported().
If a function claims ATS support via pci_ats_supported() but then fails
in pci_prepare_ats() (e.g., due to an STU mismatch between PF and VF),
it indicates a bug or hardware inconsistency.

Thanks,
Praan

[1] https://lore.kernel.org/all/20260519145947.GK7702@ziepe.ca/