PCI: Disable ACS SV for IDT 0x80b5 switch

Some IDT switches incorrectly flag an ACS Source Validation error on
completions for config read requests before they have captured the bus
number from a previous config write, even though PCIe r7.0, sec 6.12.1.1,
says that completions are never affected by ACS Source Validation.

The previous workaround, aa667c6408d2 ("PCI: Workaround IDT switch ACS
Source Validation erratum"), temporarily disabled ACS SV during
enumeration. This was effective but didn't cover the time after switch
reset, when it may lose the captured bus number.

Avoid the issue by preventing use of ACS SV altogether for these switches
by calling pci_disable_broken_acs_cap() from pci_acs_init() and remove the
previous workaround in pci_bus_read_dev_vendor_id().

Removal of ACS SV for these switches means they no longer enforce
everything in REQ_ACS_FLAGS, so downstream devices are not isolated from
each other and the iommu_group may include more devices.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
[bhelgaas: commit log, retain specific erratum details]
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Link: https://patch.msgid.link/20260102-pci_acs-v3-3-72280b94d288@oss.qualcomm.com

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index e4c11efce34ca9a0d65c4f07765744e3ec154323..0ce6a7b560e7f83505eb5b358d8fc438c9c10e36 100644 (file)
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3653,6 +3653,7 @@ void pci_acs_init(struct pci_dev *dev)
                return;
 
        pci_read_config_word(dev, pos + PCI_ACS_CAP, &dev->acs_capabilities);
+       pci_disable_broken_acs_cap(dev);
 }
 
 /**

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 4592ede0ebcc695b0ec50b1df1be9d28db0088f1..5fe5d6e84c95456013a443837ef4576ab98248fe 100644 (file)
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -432,7 +432,6 @@ bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *pl,
                                int rrs_timeout);
 bool pci_bus_generic_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *pl,
                                        int rrs_timeout);
-int pci_idt_bus_quirk(struct pci_bus *bus, int devfn, u32 *pl, int rrs_timeout);
 
 int pci_setup_device(struct pci_dev *dev);
 void __pci_size_stdbars(struct pci_dev *dev, int count,
@@ -944,6 +943,7 @@ void pci_enable_acs(struct pci_dev *dev);
 int pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags);
 int pci_dev_specific_enable_acs(struct pci_dev *dev);
 int pci_dev_specific_disable_acs_redir(struct pci_dev *dev);
+void pci_disable_broken_acs_cap(struct pci_dev *pdev);
 int pcie_failed_link_retrain(struct pci_dev *dev);
 #else
 static inline int pci_dev_specific_acs_enabled(struct pci_dev *dev,
@@ -959,6 +959,7 @@ static inline int pci_dev_specific_disable_acs_redir(struct pci_dev *dev)
 {
        return -ENOTTY;
 }
+static inline void pci_disable_broken_acs_cap(struct pci_dev *dev) { }
 static inline int pcie_failed_link_retrain(struct pci_dev *dev)
 {
        return -ENOTTY;

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 41183aed8f5d94c514e9d1787107999acbba7c4f..c7304ac5afc2deffa7a0190cfda8b3c4653abece 100644 (file)
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2547,18 +2547,6 @@ bool pci_bus_generic_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l,
 bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l,
                                int timeout)
 {
-#ifdef CONFIG_PCI_QUIRKS
-       struct pci_dev *bridge = bus->self;
-
-       /*
-        * Certain IDT switches have an issue where they improperly trigger
-        * ACS Source Validation errors on completions for config reads.
-        */
-       if (bridge && bridge->vendor == PCI_VENDOR_ID_IDT &&
-           bridge->device == 0x80b5)
-               return pci_idt_bus_quirk(bus, devfn, l, timeout);
-#endif
-
        return pci_bus_generic_read_dev_vendor_id(bus, devfn, l, timeout);
 }
 EXPORT_SYMBOL(pci_bus_read_dev_vendor_id);

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 9bd9048af8564b39afc2dffe24c11147fe45ad15..6360c172ff0d9c1e5a44a417e4b82e8e7e394a75 100644 (file)
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -5801,7 +5801,7 @@ DECLARE_PCI_FIXUP_CLASS_RESUME_EARLY(PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID,
 
 /*
  * Some IDT switches incorrectly flag an ACS Source Validation error on
- * completions for config read requests even though PCIe r4.0, sec
+ * completions for config read requests even though PCIe r7.0, sec
  * 6.12.1.1, says that completions are never affected by ACS Source
  * Validation.  Here's the text of IDT 89H32H8G3-YC, erratum #36:
  *
@@ -5814,44 +5814,20 @@ DECLARE_PCI_FIXUP_CLASS_RESUME_EARLY(PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID,
  *
  * The workaround suggested by IDT is to issue a config write to the
  * downstream device before issuing the first config read.  This allows the
- * downstream device to capture its bus and device numbers (see PCIe r4.0,
- * sec 2.2.9), thus avoiding the ACS error on the completion.
+ * downstream device to capture its bus and device numbers (see PCIe r7.0,
+ * sec 2.2.9.1), thus avoiding the ACS error on the completion.
  *
  * However, we don't know when the device is ready to accept the config
- * write, so we do config reads until we receive a non-Config Request Retry
- * Status, then do the config write.
- *
- * To avoid hitting the erratum when doing the config reads, we disable ACS
- * SV around this process.
+ * write, and the issue affects resets of the switch as well as enumeration,
+ * so disable use of ACS SV for these devices altogether.
  */
-int pci_idt_bus_quirk(struct pci_bus *bus, int devfn, u32 *l, int timeout)
+void pci_disable_broken_acs_cap(struct pci_dev *pdev)
 {
-       int pos;
-       u16 ctrl = 0;
-       bool found;
-       struct pci_dev *bridge = bus->self;
-
-       pos = bridge->acs_cap;
-
-       /* Disable ACS SV before initial config reads */
-       if (pos) {
-               pci_read_config_word(bridge, pos + PCI_ACS_CTRL, &ctrl);
-               if (ctrl & PCI_ACS_SV)
-                       pci_write_config_word(bridge, pos + PCI_ACS_CTRL,
-                                             ctrl & ~PCI_ACS_SV);
+       if (pdev->vendor == PCI_VENDOR_ID_IDT &&
+           pdev->device == 0x80b5) {
+               pci_info(pdev, "Disabling broken ACS SV; downstream device isolation reduced\n");
+               pdev->acs_capabilities &= ~PCI_ACS_SV;
        }
-
-       found = pci_bus_generic_read_dev_vendor_id(bus, devfn, l, timeout);
-
-       /* Write Vendor ID (read-only) so the endpoint latches its bus/dev */
-       if (found)
-               pci_bus_write_config_word(bus, devfn, PCI_VENDOR_ID, 0);
-
-       /* Re-enable ACS_SV if it was previously enabled */
-       if (ctrl & PCI_ACS_SV)
-               pci_write_config_word(bridge, pos + PCI_ACS_CTRL, ctrl);
-
-       return found;
 }
 
 /*