Message-ID: <4clyxcy5pwqaz6uguxnjei4hnxsree6k2uz5upro7khuvklfyo@nc5ebeicuqw4>
Date: Tue, 9 Dec 2025 20:15:52 +0900
From: Manivannan Sadhasivam <mani@...nel.org>
To: Marek Szyprowski <m.szyprowski@...sung.com>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@....qualcomm.com>, 
	Bjorn Helgaas <bhelgaas@...gle.com>, 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>, 
	Robin Murphy <robin.murphy@....com>, Jason Gunthorpe <jgg@...pe.ca>
Subject: Re: [PATCH v2 0/4] PCI: Fix ACS enablement for Root Ports in OF
 platforms

On Tue, Dec 09, 2025 at 09:28:38AM +0100, Marek Szyprowski wrote:
> On 09.12.2025 08:31, Marek Szyprowski wrote:
> > On 04.12.2025 14:13, Marek Szyprowski wrote:
> >> On 03.12.2025 13:04, Marek Szyprowski wrote:
> >>> On 02.12.2025 15:22, Manivannan Sadhasivam wrote:
> >>>> This series fixes the long standing issue with ACS in OF platforms. 
> >>>> There are
> >>>> two fixes in this series, both fixing independent issues on their 
> >>>> own, but both
> >>>> are needed to properly enable ACS on OF platforms.
> >>>>
> >>>> Issue(s) background
> >>>> ===================
> >>>>
> >>>> Back in 2021, 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 
> >>>> was 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.
> >>>>
> >>>> Solution
> >>>> ========
> >>>>
> >>>> In September, I submitted a series [6] to fix both issues. For the 
> >>>> IDT issue,
> >>>> I reused the existing quirk in the PCI core which does a dummy 
> >>>> config write
> >>>> before issuing the first config read to the device. And for the ACS 
> >>>> enablement
> >>>> issue, I just resubmitted the original patch from Xingang which called
> >>>> pci_request_acs() from devm_of_pci_bridge_init().
> >>>>
> >>>> But during the review of the series, several comments were received 
> >>>> and they
> >>>> required the series to be reworked completely. Hence, in this 
> >>>> version, I've
> >>>> incorported the comments as below:
> >>>>
> >>>> 1. For the ACS enablement issue, I've moved the pci_enable_acs() 
> >>>> call from
> >>>> pci_acs_init() to pci_dma_configure().
> >>>>
> >>>> 2. For the IDT issue, I've cached the ACS capabilities (RO) in 
> >>>> 'pci_dev',
> >>>> collected the broken capability for the IDT switches in the quirk 
> >>>> and used it to
> >>>> disable the capability in the cache. This also allowed me to get 
> >>>> rid of the
> >>>> earlier workaround for the switch.
> >>>>
> >>>> [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@huawei.com
> >>>> [3] 
> >>>> https://lore.kernel.org/all/01314d70-41e6-70f9-e496-84091948701a@samsung.com
> >>>> [4] 
> >>>> https://lore.kernel.org/all/CADYN=9JWU3CMLzMEcD5MSQGnaLyDRSKc5SofBFHUax6YuTRaJA@mail.gmail.com
> >>>> [5] 
> >>>> https://lore.kernel.org/linux-pci/20241107-pci_acs_fix-v1-1-185a2462a571@quicinc.com
> >>>> [6] 
> >>>> https://lore.kernel.org/linux-pci/20250910-pci-acs-v1-0-fe9adb65ad7d@oss.qualcomm.com
> >>>>
> >>> Thanks for this patchset! I've tested it on my ARM Juno R1 and it 
> >>> looks that it almost works fine. This patchset even fixed some 
> >>> issues with PCI devices probe, as I again see SATA and GBit ethernet 
> >>> devices, which were missing since Linux v6.14 (it looks that 
> >>> I've also missed this in my tests).
> >>>
> >>> # lspci
> >>> 00:00.0 PCI bridge: PLDA PCI Express Core Reference Design (rev 01)
> >>> 01:00.0 PCI bridge: Integrated Device Technology, Inc. [IDT] Device 
> >>> 8090 (rev 02)
> >>> 02:01.0 PCI bridge: Integrated Device Technology, Inc. [IDT] Device 
> >>> 8090 (rev 02)
> >>> 02:02.0 PCI bridge: Integrated Device Technology, Inc. [IDT] Device 
> >>> 8090 (rev 02)
> >>> 02:03.0 PCI bridge: Integrated Device Technology, Inc. [IDT] Device 
> >>> 8090 (rev 02)
> >>> 02:0c.0 PCI bridge: Integrated Device Technology, Inc. [IDT] Device 
> >>> 8090 (rev 02)
> >>> 02:10.0 PCI bridge: Integrated Device Technology, Inc. [IDT] Device 
> >>> 8090 (rev 02)
> >>> 02:1f.0 PCI bridge: Integrated Device Technology, Inc. [IDT] Device 
> >>> 8090 (rev 02)
> >>> 03:00.0 Mass storage controller: Silicon Image, Inc. SiI 3132 Serial 
> >>> ATA Raid II Controller (rev 01)
> >>> 08:00.0 Ethernet controller: Marvell Technology Group Ltd. 88E8057 
> >>> PCI-E Gigabit Ethernet Controller
> >>>
> >>> However there is also a regression. After applying this patchset 
> >>> system suspend/resume stopped working. This is probably related to 
> >>> this message:
> >>>
> >>> pcieport 0000:02:1f.0: Unable to change power state from D0 to 
> >>> D3hot, device inaccessible
> >>>
> >>> which appears after calling 'rtcwake -s10 -mmem'. This might not be 
> >>> related to this patchset, so I probably need to apply it on older 
> >>> kernel releases and check.
> >>
> >>
> >> Just one more information - I've applied this patchset on top of 
> >> v6.16 and it works perfectly on ARM Juno R1. SATA and GBit ethernet 
> >> are visible again and system suspend/resume works too, so the issue 
> >> with the latter on top of v6.18 seems not to be directly related to 
> >> $subject patchset. I will try to bisect this issue when I have some 
> >> spare time.
> >>
> >> Feel free to add:
> >>
> >> Tested-by: Marek Szyprowski <m.szyprowski@...sung.com>
> >
> >
> > I spent some time analyzing this regression on Juno R1 and found that:
> >
> > 1. SATA and GBit Ethernet stopped working after commit bcb81ac6ae3c 
> > ("iommu: Get DT/ACPI parsing into the proper probe path") merged to 
> > v6.15-rc1.
> >
> > 2. With $subject patch applied to enable SATA & GBit ethernet again, 
> > system suspend/resume stopped working after commit f3ac2ff14834 
> > ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree 
> > platforms") merged to v6.18-rc1.
> >

Yes, this was expected as if you don't disable ACS, it will cause issues in
detecting the devices.

> > If I got it right, according to the latter commit message, some quirks 
> > have to be added to fix the suspend/resume issue. Unfortunately I have 
> > no idea if this is the Juno R1 or the given PCI devices specific issue.
> 
> 
> And one more note, commit df5192d9bb0e ("PCI/ASPM: Enable only L0s and 
> L1 for devicetree platforms") doesn't fix the suspend/resume issue 
> either (with $subject patchset applied on top of it).
> 

Interesting. Can you do:

echo performance > /sys/module/pcie_aspm/parameters/policy

and then suspend?

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Powered by blists - more mailing lists