Message-ID: <26a93564-fcdd-4e90-b28d-8cc84cedeaa8@samsung.com>
Date: Tue, 9 Dec 2025 09:28:38 +0100
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: Manivannan Sadhasivam <manivannan.sadhasivam@....qualcomm.com>, 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>, Robin Murphy <robin.murphy@....com>, Jason
	Gunthorpe <jgg@...pe.ca>, Manivannan Sadhasivam <mani@...nel.org>
Subject: Re: [PATCH v2 0/4] PCI: Fix ACS enablement for Root Ports in OF
 platforms

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.
>
> 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).


Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Powered by blists - more mailing lists