| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20251202-pci_acs-v2-3-5d2759a71489@oss.qualcomm.com>
Date: Tue, 02 Dec 2025 19:52:50 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@....qualcomm.com>
To: Bjorn Helgaas <bhelgaas@...gle.com>
Cc: linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
iommu@...ts.linux.dev, Naresh Kamboju <naresh.kamboju@...aro.org>,
Pavankumar Kondeti <quic_pkondeti@...cinc.com>,
Xingang Wang <wangxingang5@...wei.com>,
Marek Szyprowski <m.szyprowski@...sung.com>,
Robin Murphy <robin.murphy@....com>, Jason Gunthorpe <jgg@...pe.ca>,
Manivannan Sadhasivam <mani@...nel.org>,
Manivannan Sadhasivam <manivannan.sadhasivam@....qualcomm.com>
Subject: [PATCH v2 3/4] PCI: Disable ACS SV capability for the broken IDT
switches
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@....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.1
Powered by blists - more mailing lists