Re: [PATCH] PCI: Add PCI quirk to disable L0s ASPM state for RTL8125 2.5GbE Controller

["hans.zhang" <hans.zhang@cixtech.com>]

On 2025/3/6 06:20, Bjorn Helgaas wrote:
> [Some people who received this message don't often get email from helgaas@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> EXTERNAL EMAIL
> 
> [+cc r8169 maintainers, since upstream r8169 claims device 0x8125]
> 
> On Wed, Mar 05, 2025 at 02:30:35PM +0800, hans.zhang@cixtech.com wrote:
>> From: Hans Zhang <hans.zhang@cixtech.com>
>>
>> This patch is intended to disable L0s ASPM link state for RTL8125 2.5GbE
>> Controller due to the fact that it is possible to corrupt TX data when
>> coming back out of L0s on some systems. This quirk uses the ASPM api to
>> prevent the ASPM subsystem from re-enabling the L0s state.
> 
> Sounds like this should be a documented erratum.  Realtek folks?  Or
> maybe an erratum on the other end of the link, which looks like a CIX
> Root Port:
> 
>    https://admin.pci-ids.ucw.cz/read/PC/1f6c/0001
> 
> If it's a CIX Root Port defect, it could affect devices other than
> RTL8125.
> 
>> And it causes the following AER errors:
>>    pcieport 0003:30:00.0: AER: Multiple Corrected error received: 0003:31:00.0
>>    pcieport 0003:30:00.0: PCIe Bus Error: severity=Corrected, type=Data Link Layer, (Transmitter ID)
>>    pcieport 0003:30:00.0:   device [1f6c:0001] error status/mask=00001000/0000e000
>>    pcieport 0003:30:00.0:    [12] Timeout
>>    r8125 0003:31:00.0: PCIe Bus Error: severity=Corrected, type=Data Link Layer, (Transmitter ID)
>>    r8125 0003:31:00.0:   device [10ec:8125] error status/mask=00001000/0000e000
>>    r8125 0003:31:00.0:    [12] Timeout
>>    r8125 0003:31:00.0: AER:   Error of this Agent is reported first
> 
> Looks like a driver name of "r8125", but I don't see that upstream.
> Is this an out-of-tree driver?

I'm terribly sorry. In the r8169 driver, I see the setting for pulling 
the disable L0s. Please discard this patch.

When I enabled ASPM and insmod r8169.ko, it worked fine.

Previously, we used a separate RTL8125 driver from RELTEAK. Compared 
with upstream, it has more functions and is more complete, but their 
drivers do not disable L0s, so this patch is mentioned.

Best regards,
Hans

drivers/net/ethernet/realtek/r8169_main.c

static void rtl_task(struct work_struct *work)
{
	struct rtl8169_private *tp =
		container_of(work, struct rtl8169_private, wk.work);
	int ret;

	if (test_and_clear_bit(RTL_FLAG_TASK_TX_TIMEOUT, tp->wk.flags)) {
		/* if chip isn't accessible, reset bus to revive it */
		if (RTL_R32(tp, TxConfig) == ~0) {
			ret = pci_reset_bus(tp->pci_dev);
			if (ret < 0) {
				netdev_err(tp->dev, "Can't reset secondary PCI bus, detach NIC\n");
				netif_device_detach(tp->dev);
				return;
			}
		}

		/* ASPM compatibility issues are a typical reason for tx timeouts */
		ret = pci_disable_link_state(tp->pci_dev, PCIE_LINK_STATE_L1 |
							  PCIE_LINK_STATE_L0S);
		if (!ret)
			netdev_warn_once(tp->dev, "ASPM disabled on Tx timeout\n");
		goto reset;
	}

	if (test_and_clear_bit(RTL_FLAG_TASK_RESET_PENDING, tp->wk.flags)) {
reset:
		rtl_reset_work(tp);
		netif_wake_queue(tp->dev);
	}
}


> 
>> And the RTL8125 website does not say that it supports L0s. It only supports
>> L1 and L1ss.
>>
>> RTL8125 website: https://www.realtek.com/Product/Index?id=3962
> 
> I don't think it matters what the web site says.  Apparently the
> device advertises L0s support via Link Capabilities.
> 
>> Signed-off-by: Hans Zhang <hans.zhang@cixtech.com>
>> Reviewed-by: Peter Chen <peter.chen@cixtech.com>
>> ---
>>   drivers/pci/quirks.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>> index 82b21e34c545..5f69bb5ee3ff 100644
>> --- a/drivers/pci/quirks.c
>> +++ b/drivers/pci/quirks.c
>> @@ -2514,6 +2514,12 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x10f1, quirk_disable_aspm_l0s);
>>   DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x10f4, quirk_disable_aspm_l0s);
>>   DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1508, quirk_disable_aspm_l0s);
>>
>> +/*
>> + * The RTL8125 may experience data corruption issues when transitioning out
>> + * of L0S. To prevent this we need to disable L0S on the PCIe link.
>> + */
>> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_REALTEK, 0x8125, quirk_disable_aspm_l0s);
>> +
>>   static void quirk_disable_aspm_l0s_l1(struct pci_dev *dev)
>>   {
>>        pci_info(dev, "Disabling ASPM L0s/L1\n");
>>
>> base-commit: 99fa936e8e4f117d62f229003c9799686f74cebc
>> --
>> 2.47.1
>>