Re: [PATCH v2 0/4] PCI: Fix ACS enablement for Root Ports in OF platforms

[Marek Szyprowski <m.szyprowski@samsung.com>]

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@samsung.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.


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