| Message ID | 20250305063035.415717-1-hans.zhang@cixtech.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI: Add PCI quirk to disable L0s ASPM state for RTL8125 2.5GbE Controller | expand |
[+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? > 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 >
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 >>
On 2025/3/6 06:20, Bjorn Helgaas wrote: > 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 Hi Bjorn, Name: CIX P1 CD8180 PCI Express Root Port 0000:90:00.0 PCI bridge [0604]: Device [1f6c:0001] 0001:60:00.0 PCI bridge [0604]: Device [1f6c:0001] 0002:00:00.0 PCI bridge [0604]: Device [1f6c:0001] 0003:30:00.0 PCI bridge [0604]: Device [1f6c:0001] This URL does not appear right, how should be changed, is it you? Or can you tell me who I should call to change it? The correct answer is: 0000:90:00.0 PCI bridge [0604]: Device [1f6c:0001] 0001:C0:00.0 PCI bridge [0604]: Device [1f6c:0001] 0002:60:00.0 PCI bridge [0604]: Device [1f6c:0001] 0003:30:00.0 PCI bridge [0604]: Device [1f6c:0001] 0004:00:00.0 PCI bridge [0604]: Device [1f6c:0001] The domain might be random, so whichever controller probes first, it's assigned first. The URL currently shows the BDF with one controller missing. That's the order in which we're going to controller probe. Best regards, Hans
On Thu, Mar 06, 2025 at 11:32:04AM +0800, hans.zhang wrote: > On 2025/3/6 06:20, Bjorn Helgaas wrote: > > 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 > > Name: CIX P1 CD8180 PCI Express Root Port > > 0000:90:00.0 PCI bridge [0604]: Device [1f6c:0001] > 0001:60:00.0 PCI bridge [0604]: Device [1f6c:0001] > 0002:00:00.0 PCI bridge [0604]: Device [1f6c:0001] > 0003:30:00.0 PCI bridge [0604]: Device [1f6c:0001] > > > This URL does not appear right, how should be changed, is it you? Or can you > tell me who I should call to change it? > > The correct answer is: > 0000:90:00.0 PCI bridge [0604]: Device [1f6c:0001] > 0001:C0:00.0 PCI bridge [0604]: Device [1f6c:0001] > 0002:60:00.0 PCI bridge [0604]: Device [1f6c:0001] > 0003:30:00.0 PCI bridge [0604]: Device [1f6c:0001] > 0004:00:00.0 PCI bridge [0604]: Device [1f6c:0001] This part of the web page is just commentary. In this case it's just an example of what devices might be on some system. It's not a requirement that all systems have this many devices or devices at these addresses. The only important parts are the Vendor ID, Device ID, and the name ("CIX P1 CD8180 PCI Express Root Port"). If those are correct, no need to do anything. > The domain might be random, so whichever controller probes first, it's > assigned first. The URL currently shows the BDF with one controller missing. > That's the order in which we're going to controller probe. > > Best regards, > Hans > >
On 05.03.2025 23:20, Bjorn Helgaas wrote: > [+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? > Yes, this refers to Realtek's out-of-tree r8125 driver. As stated by Hans, with the r8169 in-tree driver the issue doesn't occur. >> 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 >>
On 2025/3/7 00:28, 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 > > On Thu, Mar 06, 2025 at 11:32:04AM +0800, hans.zhang wrote: >> On 2025/3/6 06:20, Bjorn Helgaas wrote: >>> 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 >> >> Name: CIX P1 CD8180 PCI Express Root Port >> >> 0000:90:00.0 PCI bridge [0604]: Device [1f6c:0001] >> 0001:60:00.0 PCI bridge [0604]: Device [1f6c:0001] >> 0002:00:00.0 PCI bridge [0604]: Device [1f6c:0001] >> 0003:30:00.0 PCI bridge [0604]: Device [1f6c:0001] >> >> >> This URL does not appear right, how should be changed, is it you? Or can you >> tell me who I should call to change it? >> >> The correct answer is: >> 0000:90:00.0 PCI bridge [0604]: Device [1f6c:0001] >> 0001:C0:00.0 PCI bridge [0604]: Device [1f6c:0001] >> 0002:60:00.0 PCI bridge [0604]: Device [1f6c:0001] >> 0003:30:00.0 PCI bridge [0604]: Device [1f6c:0001] >> 0004:00:00.0 PCI bridge [0604]: Device [1f6c:0001] > > This part of the web page is just commentary. In this case it's just > an example of what devices might be on some system. It's not a > requirement that all systems have this many devices or devices at > these addresses. > > The only important parts are the Vendor ID, Device ID, and the name > ("CIX P1 CD8180 PCI Express Root Port"). If those are correct, no > need to do anything. I see. Thank you very much Bjorn. Best regards, Hans
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");