Re: [PATCH v2] PCI: Disable ATS via quirk before notifying IOMMU drivers

[Baolu Lu <baolu.lu@linux.intel.com>]

On 3/28/26 05:16, David Matlack wrote:
> Ensure that PCI devices with ATS disabled via quirk have it disabled
> before IOMMU drivers are notified about the device rather than after.
> Fix this by converting the existing quirks from final to early fixups
> and changing the quirk logic to set a new no_ats bit in struct pci_dev
> that prevents pci_dev.ats_cap from ever gettting set.
> 
> This change ensures that pci_ats_supported() takes quirks into account
> during iommu_ops.probe_device(), when IOMMU drivers are first notified
> about devices. It also ensures that pci_ats_supported() returns the same
> value when the device is released in iommu_ops.release_device().
> 
> Notably, the Intel IOMMU driver uses pci_ats_supported() in
> probe/release to determine whether to add/remove a device from a data
> structure, which easily leads to a use-after-free without this fix.

Can you please shed more light on the above issue? In my investigation,
iommu_ops.probe_device() is always called after the no_ats quirk,
regardless of whether this patch is applied.

The diff of the changes I made for testing is as follows:

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 442271a1b92a..c024964ac53b 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -3271,6 +3271,8 @@ static struct iommu_device 
*intel_iommu_probe_device(struct device *dev)
                                 info->pfsid = pci_dev_id(pci_physfn(pdev));
                         info->ats_qdep = pci_ats_queue_depth(pdev);
                 }
+               pci_info(pdev, "ATS %s\n", info->ats_supported ?
+                        "supported" : "not supported");
                 if (sm_supported(iommu)) {
                         if (pasid_supported(iommu)) {
                                 int features = pci_pasid_features(pdev);
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 48946cca4be7..c63616d108b7 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -5714,6 +5714,8 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 
0x1457, quirk_intel_e2000_no_ats);
  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1459, 
quirk_intel_e2000_no_ats);
  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x145a, 
quirk_intel_e2000_no_ats);
  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x145c, 
quirk_intel_e2000_no_ats);
+
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0b25, quirk_no_ats);
  #endif /* CONFIG_PCI_ATS */

  /* Freescale PCIe doesn't support MSI in RC mode */


The related kernel messages are shown below:

# dmesg | grep "0000:00:01.0"
[   15.834944] pci 0000:00:01.0: [8086:0b25] type 00 class 0x088000 PCIe 
Root Complex Integrated Endpoint
[   15.836382] pci 0000:00:01.0: BAR 0 [mem 
0x1e0fff980000-0x1e0fff99ffff 64bit pref]
[   15.836655] pci 0000:00:01.0: BAR 2 [mem 
0x1e0fff900000-0x1e0fff93ffff 64bit pref]
[   15.837904] pci 0000:00:01.0: calling 
quirk_igfx_skip_te_disable+0x0/0xe0 @ 1
[   15.838614] pci 0000:00:01.0: quirk_igfx_skip_te_disable+0x0/0xe0 
took 0 usecs
[   21.205177] pci 0000:00:01.0: calling  quirk_no_ats+0x0/0x40 @ 1
[   21.206767] pci 0000:00:01.0: disabling ATS
[   21.207916] pci 0000:00:01.0: quirk_no_ats+0x0/0x40 took 1122 usecs
[   21.305357] pci 0000:00:01.0: DMAR: ATS not supported
[   21.306925] pci 0000:00:01.0: Adding to iommu group 4
[   42.564912] idxd 0000:00:01.0: Intel(R) Accelerator Device (v200)
[   42.568653] probe of 0000:00:01.0 returned 0 after 87413 usecs


Anything I missed?

Thanks,
baolu

> 
> This change also makes disabling ATS via quirk behave the same way as
> the pci=noats command line option, in that pci_ats_init() bails
> immediately and never intializes pci_dev.ats_cap.
> 
> Fixes: a18615b1cfc0 ("PCI: Disable ATS for specific Intel IPU E2000 devices")
> Closes:https://lore.kernel.org/linux-iommu/aYUQ_HkDJU9kjsUl@google.com/
> Signed-off-by: David Matlack<dmatlack@google.com>
> ---
> v2:
>   - Update the commit message with reasons why this is being fixed in the
>     PCI core, rather than applying a point fix to the Intel IOMMU driver
>     (Andy)
>   - Condense the pci_ats_disabled() and dev->no_ats checks into a single
>     line in pci_ats_init()
>   - Reorder the no_ats bitfield to be after ats_stu since there is likely
>     u8-sized gap there for alignment purposes
> 
> v1:https://lore.kernel.org/linux-pci/20260223184017.688212-1- 
> dmatlack@google.com/
> 
> Cc: Raghavendra Rao Ananta<rananta@google.com>
> Cc: David Woodhouse<dwmw2@infradead.org>
> Cc: Lu Baolu<baolu.lu@linux.intel.com>
> Cc: Andy Shevchenko<andriy.shevchenko@linux.intel.com>
> 
>   drivers/pci/ats.c    |  2 +-
>   drivers/pci/quirks.c | 50 ++++++++++++++++++++++----------------------
>   include/linux/pci.h  |  1 +
>   3 files changed, 27 insertions(+), 26 deletions(-)