Hi,
This series fixes the long standing issue with ACS in DT platforms. There are two fixes in this series, both fixing independent issues on their own, but both are needed to properly enable ACS on DT platforms (well, patch 1 is only needed for Juno board, but that was a blocker for patch 2, more below...).
Issue(s) background ===================
Back in 2024, Xingang Wang first noted a failure in attaching the HiSilicon SEC device to QEMU ARM64 pci-root-port device [1]. He then tracked down the issue to ACS not being enabled for the QEMU Root Port device and he proposed a patch to fix it [2].
Once the patch got applied, people reported PCIe issues with linux-next on the ARM Juno Development boards, where they saw failure in enumerating the endpoint devices [3][4]. So soon, the patch got dropped, but the actual issue with the ARM Juno boards was left behind.
Fast forward to 2024, Pavan resubmitted the same fix [5] for his own usecase, hoping that someone in the community would fix the issue with ARM Juno boards. But the patch was rightly rejected, as a patch that was known to cause issues should not be merged to the kernel. But again, no one investigated the Juno issue and it was left behind again.
Now it ended up in my plate and I managed to track down the issue with the help of Naresh who got access to the Juno boards in LKFT. The Juno issue is with the PCIe switch from Microsemi/IDT, which triggers ACS Source Validation error on Completions received for the Configuration Read Request from a device connected to the downstream port that has not yet captured the PCIe bus number. As per the PCIe spec r6.0 sec 2.2.6.2, "Functions must capture the Bus and Device Numbers supplied with all Type 0 Configuration Write Requests completed by the Function and supply these numbers in the Bus and Device Number fields of the Requester ID for all Requests". So during the first Configuration Read Request issued by the switch downstream port during enumeration (for reading Vendor ID), Bus and Device numbers will be unknown to the device. So it responds to the Read Request with Completion having Bus and Device number as 0. The switch interprets the Completion as an ACS Source Validation error and drops the completion, leading to the failure in detecting the endpoint device. Though the PCIe spec r6.0, sec 6.12.1.1, states that "Completions are never affected by ACS Source Validation". This behavior is in violation of the spec.
This issue was already found and addressed with a quirk for a different device from Microsemi with 'commit, aa667c6408d2 ("PCI: Workaround IDT switch ACS Source Validation erratum")'. Apparently, this issue seems to be documented in the erratum #36 of IDT 89H32H8G3-YC, which is not publicly available.
Solution for Juno issue =======================
To fix this issue, I've extended the quirk to the Device ID of the switch found in Juno R2 boards. I believe the same switch is also present in Juno R1 board as well.
With Patch 1, the Juno R2 boards can now detect the endpoints even with ACS enabled for the Switch downstream ports. Finally, I added patch 2 that properly enables ACS for all the PCI devices on DT platforms.
It should be noted that even without patch 2 which enables ACS for the Root Port, the Juno boards were failing since 'commit, bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper probe path")' as reported in LKFT [6]. I believe, this commit made sure pci_request_acs() gets called before the enumeration of the switch downstream ports. The LKFT team ended up disabling ACS using cmdline param 'pci=config_acs=000000@pci:0:0'. So I added the above mentioned commit as a Fixes tag for patch 1.
Also, to mitigate this issue, one could enumerate all the PCIe devices in bootloader without enabling ACS (as also noted by Robin in the LKFT thread). This will make sure that the endpoint device has a valid bus number when it responds to the first Configuration Read Request from the switch downstream port. So the ACS Source Validation error doesn't get triggered.
Solution for ACS issue ======================
To fix this issue, I've kept the patch from Xingang as is (with rewording of the patch subject/description). This patch moves the pci_request_acs() call to devm_of_pci_bridge_init(), which gets called during the host bridge registration. This makes sure that the 'pci_acs_enable' flag set by pci_request_acs() is getting set before the enumeration of the Root Port device. So now, ACS will be enabled for all ACS capable devices of DT platforms.
[1] https://lore.kernel.org/all/038397a6-57e2-b6fc-6e1c-7c03b7be9d96@huawei.com [2] https://lore.kernel.org/all/1621566204-37456-1-git-send-email-wangxingang5@h... [3] https://lore.kernel.org/all/01314d70-41e6-70f9-e496-84091948701a@samsung.com [4] https://lore.kernel.org/all/CADYN=9JWU3CMLzMEcD5MSQGnaLyDRSKc5SofBFHUax6YuTR... [5] https://lore.kernel.org/linux-pci/20241107-pci_acs_fix-v1-1-185a2462a571@qui... [6] https://lists.linaro.org/archives/list/lkft-triage@lists.linaro.org/message/...
Signed-off-by: Manivannan Sadhasivam manivannan.sadhasivam@oss.qualcomm.com --- Manivannan Sadhasivam (1): PCI: Extend pci_idt_bus_quirk() for IDT switch with Device ID 0x8090
Xingang Wang (1): iommu/of: Call pci_request_acs() before enumerating the Root Port device
drivers/iommu/of_iommu.c | 1 - drivers/pci/of.c | 8 +++++++- drivers/pci/probe.c | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) --- base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585 change-id: 20250910-pci-acs-cb4fa3983a2c
Best regards,
From: Manivannan Sadhasivam manivannan.sadhasivam@oss.qualcomm.com
If ACS is enabled, the IDT switch with Device ID 0x8090 found in ARM Juno R2 development board incorrectly raises an ACS Source Validation error on Completions for Config Read Requests, even though PCIe r6.0, sec 6.12.1.1, says that Completions are never affected by ACS Source Validation.
This behavior is documented in non-public erratum 89H32H8G3-YC and there is already a quirk available to workaround this issue.
Hence, extend the quirk for Device ID 0x8090 to make the switch functional if ACS is enabled.
Note: The commit mentioned in the Fixes tag causes ACS to be enabled before the enumeration of the switch downstream port. So it ended up breaking PCIe on ARM Juno R2 board, which used to work before this commit until someone forcefully enabled ACS with cmdline.
Cc: stable@vger.kernel.org # 6.15 Fixes: bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper probe path") Closes: https://lists.linaro.org/archives/list/lkft-triage@lists.linaro.org/message/... Signed-off-by: Manivannan Sadhasivam manivannan.sadhasivam@oss.qualcomm.com --- drivers/pci/probe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index f41128f91ca76ab014ad669ae84a53032c7c6b6b..2320818bc8e58c61d9ada312dfbd8c0fbfbadc0c 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -2500,7 +2500,7 @@ bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l, * ACS Source Validation errors on completions for config reads. */ if (bridge && bridge->vendor == PCI_VENDOR_ID_IDT && - bridge->device == 0x80b5) + (bridge->device == 0x80b5 || bridge->device == 0x8090)) return pci_idt_bus_quirk(bus, devfn, l, timeout); #endif
From: Xingang Wang wangxingang5@huawei.com
When booting with devicetree, ACS is enabled for all ACS capable PCI devices except the first Root Port enumerated in the system. This is due to calling pci_request_acs() after the enumeration and initialization of the Root Port device. But afterwards, ACS is getting enabled for the rest of the PCI devices, since pci_request_acs() sets the 'pci_acs_enable' flag and the PCI core uses this flag to enable ACS for the rest of the ACS capable devices.
Ideally, pci_request_acs() should only be called if the 'iommu-map' DT property is set for the host bridge device. Hence, call pci_request_acs() from devm_of_pci_bridge_init() if the 'iommu-map' property is present in the host bridge DT node. This aligns with the implementation of the ARM64 ACPI driver (drivers/acpi/arm64/iort.c) as well.
With this change, ACS will be enabled for all the PCI devices including the first Root Port device of the DT platforms.
Cc: stable@vger.kernel.org # 5.6 Fixes: 6bf6c24720d33 ("iommu/of: Request ACS from the PCI core when configuring IOMMU linkage") Signed-off-by: Xingang Wang wangxingang5@huawei.com Signed-off-by: Pavankumar Kondeti quic_pkondeti@quicinc.com [mani: reworded subject, description and comment] Signed-off-by: Manivannan Sadhasivam manivannan.sadhasivam@oss.qualcomm.com --- drivers/iommu/of_iommu.c | 1 - drivers/pci/of.c | 8 +++++++- 2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c index 6b989a62def20ecafd833f00a3a92ce8dca192e0..c31369924944d36a3afd3d4ff08c86fc6daf55de 100644 --- a/drivers/iommu/of_iommu.c +++ b/drivers/iommu/of_iommu.c @@ -141,7 +141,6 @@ int of_iommu_configure(struct device *dev, struct device_node *master_np, .np = master_np, };
- pci_request_acs(); err = pci_for_each_dma_alias(to_pci_dev(dev), of_pci_iommu_init, &info); of_pci_check_device_ats(dev, master_np); diff --git a/drivers/pci/of.c b/drivers/pci/of.c index 3579265f119845637e163d9051437c89662762f8..98c2523f898667b1618c37451d1759959d523da1 100644 --- a/drivers/pci/of.c +++ b/drivers/pci/of.c @@ -638,9 +638,15 @@ static int pci_parse_request_of_pci_ranges(struct device *dev,
int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge) { - if (!dev->of_node) + struct device_node *node = dev->of_node; + + if (!node) return 0;
+ /* Enable ACS if IOMMU mapping is detected for the host bridge */ + if (of_property_read_bool(node, "iommu-map")) + pci_request_acs(); + bridge->swizzle_irq = pci_common_swizzle; bridge->map_irq = of_irq_parse_and_map_pci;
linux-stable-mirror@lists.linaro.org