Message-ID: <0ee48f4a-2341-4967-aca5-3fe6b4cd5fe2@samsung.com>
Date: Tue, 9 Dec 2025 13:00:55 +0100
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: Manivannan Sadhasivam <mani@...nel.org>
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 09.12.2025 12:15, Manivannan Sadhasivam wrote:
> 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?

After the above command, system suspend/resume works again.

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

Powered by blists - more mailing lists