Some IDT switches behave erratically when ACS Source Validation is enabled.
For example, they incorrectly flag an ACS Source Validation error on
completions for config read requests even though PCIe r4.0, sec 6.12.1.1,
says that completions are never affected by ACS Source Validation.
Even though IDT suggests working around this issue by issuing a config
write before the first config read, so that the device caches the bus and
device number. But it would still be fragile since the device could loose
the IDs after the reset and any further access may trigger ACS SV
violation.
Hence, to properly fix the issue, the respective capability needs to be
disabled. Since the ACS Capabilities are RO values, and are cached in the
'pci_dev::acs_capabilities' field, add a new field for broken caps, set it
in quirks and use it to remove the broken capabilities in pci_acs_init().
This will allow pci_enable_acs() helper to disable the relevant ACS ctrls.
It should be noted that the quirk should be of the fixup_header level, so
that it gets called before pci_acs_init().
With this, the previous workaround can now be safely removed.
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
drivers/pci/pci.c | 1 +
drivers/pci/pci.h | 1 -
drivers/pci/probe.c | 12 -----------
drivers/pci/quirks.c | 61 ++++++++++++----------------------------------------
include/linux/pci.h | 1 +
5 files changed, 16 insertions(+), 60 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 4eb5b487c982..6ed35affea06 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3681,6 +3681,7 @@ void pci_acs_init(struct pci_dev *dev)
return;
pci_read_config_word(dev, pos + PCI_ACS_CAP, &dev->acs_capabilities);
+ dev->acs_capabilities &= ~dev->acs_broken_cap;
}
void pci_rebar_init(struct pci_dev *pdev)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 972b28fc5455..56ba7d60d658 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -430,7 +430,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,
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 9cd032dff31e..6f8142cf9487 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2517,18 +2517,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 b9c252aa6fe0..a5956726a49f 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -5778,59 +5778,26 @@ DECLARE_PCI_FIXUP_CLASS_RESUME_EARLY(PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID,
PCI_BASE_CLASS_DISPLAY, 16, quirk_nvidia_hda);
/*
- * Some IDT switches incorrectly flag an ACS Source Validation error on
- * completions for config read requests even though PCIe r4.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:
+ * Some IDT switches behave erratically when ACS Source Validation is enabled.
+ * For example, they incorrectly flag an ACS Source Validation error on
+ * completions for config read requests even though PCIe r4.0, sec 6.12.1.1,
+ * says that completions are never affected by ACS Source Validation.
*
- * Item #36 - Downstream port applies ACS Source Validation to Completions
- * Section 6.12.1.1 of the PCI Express Base Specification 3.1 states that
- * completions are never affected by ACS Source Validation. However,
- * completions received by a downstream port of the PCIe switch from a
- * device that has not yet captured a PCIe bus number are incorrectly
- * dropped by ACS Source Validation by the switch downstream port.
+ * Even though IDT suggests working around this issue by issuing a config write
+ * before the first config read, so that the switch caches the bus and device
+ * number, it would still be fragile since the device could loose the IDs after
+ * the reset.
*
- * 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.
- *
- * 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.
+ * Hence, a reliable fix would be to assume that these switches don't support
+ * ACS SV.
*/
-int pci_idt_bus_quirk(struct pci_bus *bus, int devfn, u32 *l, int timeout)
+static void pci_disable_acs_sv(struct pci_dev *dev)
{
- 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);
- }
+ pci_info(dev, "Disabling broken ACS SV\n");
- 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;
+ dev->acs_broken_cap |= PCI_ACS_SV;
}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_IDT, 0x80b5, pci_disable_acs_sv);
/*
* Microsemi Switchtec NTB uses devfn proxy IDs to move TLPs between
diff --git a/include/linux/pci.h b/include/linux/pci.h
index c6ee1dfdb0fb..246c0ca34308 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -544,6 +544,7 @@ struct pci_dev {
#endif
u16 acs_cap; /* ACS Capability offset */
u16 acs_capabilities; /* ACS Capabilities */
+ u16 acs_broken_cap; /* Broken ACS Capabilities */
u8 supported_speeds; /* Supported Link Speeds Vector */
phys_addr_t rom; /* Physical address if not from BAR */
size_t romlen; /* Length if not from BAR */
--
2.48.1On Tue, Dec 02, 2025 at 07:52:50PM +0530, Manivannan Sadhasivam wrote:
> @@ -544,6 +544,7 @@ struct pci_dev {
> #endif
> u16 acs_cap; /* ACS Capability offset */
> u16 acs_capabilities; /* ACS Capabilities */
> + u16 acs_broken_cap; /* Broken ACS Capabilities */
Why do we need this? Have the quirk function accep tthe
acs_capabilities from the register and return the value to program
into struct pci_dev ?
Jason
On Tue, Dec 02, 2025 at 03:15:33PM -0400, Jason Gunthorpe wrote:
> On Tue, Dec 02, 2025 at 07:52:50PM +0530, Manivannan Sadhasivam wrote:
> > @@ -544,6 +544,7 @@ struct pci_dev {
> > #endif
> > u16 acs_cap; /* ACS Capability offset */
> > u16 acs_capabilities; /* ACS Capabilities */
> > + u16 acs_broken_cap; /* Broken ACS Capabilities */
>
> Why do we need this? Have the quirk function accep tthe
> acs_capabilities from the register and return the value to program
> into struct pci_dev ?
>
We dont have any quirk levels between pci_acs_init() and pci_acs_enable() that
will allow us to modify pci_dev::acs_capabilities in the quirk function. Hence,
I came up with one more member to pass the broken caps.
- Mani
--
மணிவண்ணன் சதாசிவம்
On Tue, Dec 09, 2025 at 08:20:39PM +0900, Manivannan Sadhasivam wrote:
> On Tue, Dec 02, 2025 at 03:15:33PM -0400, Jason Gunthorpe wrote:
> > On Tue, Dec 02, 2025 at 07:52:50PM +0530, Manivannan Sadhasivam wrote:
> > > @@ -544,6 +544,7 @@ struct pci_dev {
> > > #endif
> > > u16 acs_cap; /* ACS Capability offset */
> > > u16 acs_capabilities; /* ACS Capabilities */
> > > + u16 acs_broken_cap; /* Broken ACS Capabilities */
> >
> > Why do we need this? Have the quirk function accep tthe
> > acs_capabilities from the register and return the value to program
> > into struct pci_dev ?
> >
>
> We dont have any quirk levels between pci_acs_init() and pci_acs_enable() that
> will allow us to modify pci_dev::acs_capabilities in the quirk function. Hence,
> I came up with one more member to pass the broken caps.
Call the quirk function directly from the ACS path? We have things
like that already for ACS?
Jason
© 2016 - 2026 Red Hat, Inc.