Re: Experimental fix for MSI-X issue on r8169

[Jian-Hong Pan <jian-hong@endlessm.com>]

2018-08-20 4:34 GMT+08:00 Heiner Kallweit <hkallweit1@gmail.com>:
> The three of you reported an MSI-X-related error when the system
> resumes from suspend. This has been fixed for now by disabling MSI-X
> on certain chip versions. However more versions may be affected.
>
> I checked with Realtek and they confirmed that on certain chip
> versions a MSIX-related value in PCI config space is reset when
> resuming from S3.
>
> I would appreciate if you could test the following experimental patch
> and whether warning "MSIX address lost, re-configuring" appears in
> your dmesg output after resume from suspend.
>
> Thanks a lot for your efforts.

Tested with the experiment patch on ASUS X441UAR.

This is the information before suspend:

dev@endless:~$ dmesg | grep r8169
[   10.279565] libphy: r8169: probed
[   10.279947] r8169 0000:02:00.0 eth0: RTL8106e, 0c:9d:92:32:67:b4,
XID 44900000, IRQ 127
[   10.445952] r8169 0000:02:00.0 enp2s0: renamed from eth0
[   15.676229] Generic PHY r8169-200:00: attached PHY driver [Generic
PHY] (mii_bus:phy_addr=r8169-200:00, irq=IGNORE)
[   17.455392] r8169 0000:02:00.0 enp2s0: Link is Up - 100Mbps/Full -
flow control off

dev@endless:~$ ip addr show enp2s0
4: enp2s0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast
state UP group default qlen 1000
    link/ether 0c:9d:92:32:67:b4 brd ff:ff:ff:ff:ff:ff
    inet 10.100.13.152/24 brd 10.100.13.255 scope global noprefixroute
dynamic enp2s0
       valid_lft 86347sec preferred_lft 86347sec
    inet6 fe80::2873:a2a9:6ca1:c79d/64 scope link noprefixroute
       valid_lft forever preferred_lft forever

This is the information after resume:

dev@endless:~$ dmesg | grep r8169
[   10.279565] libphy: r8169: probed
[   10.279947] r8169 0000:02:00.0 eth0: RTL8106e, 0c:9d:92:32:67:b4,
XID 44900000, IRQ 127
[   10.445952] r8169 0000:02:00.0 enp2s0: renamed from eth0
[   15.676229] Generic PHY r8169-200:00: attached PHY driver [Generic
PHY] (mii_bus:phy_addr=r8169-200:00, irq=IGNORE)
[   17.455392] r8169 0000:02:00.0 enp2s0: Link is Up - 100Mbps/Full -
flow control off
[   95.594265] r8169 0000:02:00.0 enp2s0: Link is Down
[   96.242074] Generic PHY r8169-200:00: attached PHY driver [Generic
PHY] (mii_bus:phy_addr=r8169-200:00, irq=IGNORE)

dev@endless:~$ ip addr show enp2s0
4: enp2s0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc
pfifo_fast state DOWN group default qlen 1000
    link/ether 0c:9d:92:32:67:b4 brd ff:ff:ff:ff:ff:ff

There is no "MSIX address lost, re-configuring" in dmesg.
The ethernet interface is still down after resume.

This is the ethernet controller in detail:

02:00.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd.
RTL8101/2/6E PCI Express Fast/Gigabit Ethernet controller [10ec:8136]
(rev 07)
Subsystem: ASUSTeK Computer Inc. RTL810xE PCI Express Fast Ethernet
controller [1043:200f]
Flags: bus master, fast devsel, latency 0, IRQ 16
I/O ports at e000 [size=256]
Memory at ef100000 (64-bit, non-prefetchable) [size=4K]
Memory at e0000000 (64-bit, prefetchable) [size=16K]
Capabilities: [40] Power Management version 3
Capabilities: [50] MSI: Enable- Count=1/1 Maskable- 64bit+
Capabilities: [70] Express Endpoint, MSI 01
Capabilities: [b0] MSI-X: Enable+ Count=4 Masked-
Capabilities: [d0] Vital Product Data
Capabilities: [100] Advanced Error Reporting
Capabilities: [140] Virtual Channel
Capabilities: [160] Device Serial Number 01-00-00-00-36-4c-e0-00
Capabilities: [170] Latency Tolerance Reporting
Kernel driver in use: r8169
Kernel modules: r8169

Regards,
Jian-Hong Pan

>
> ---
>  drivers/net/ethernet/realtek/r8169.c | 27 ++++++++++++++++++++++-----
>  1 file changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index 0d9c38318..56b4bdff9 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -690,6 +690,8 @@ struct rtl8169_private {
>         struct rtl8169_counters *counters;
>         struct rtl8169_tc_offsets tc_offset;
>         u32 saved_wolopts;
> +       u32 saved_msix_addr_lo;
> +       u32 saved_msix_addr_hi;
>
>         struct rtl_fw {
>                 const struct firmware *fw;
> @@ -6876,6 +6878,19 @@ static int rtl8169_resume(struct device *device)
>  {
>         struct pci_dev *pdev = to_pci_dev(device);
>         struct net_device *dev = pci_get_drvdata(pdev);
> +       struct rtl8169_private *tp = netdev_priv(dev);
> +       u32 val;
> +
> +       /* Some chip versions loose these values when resuming */
> +       if (pdev->msix_enabled) {
> +               pci_read_config_dword(pdev, PCI_BASE_ADDRESS_4, &val);
> +               if (!val)
> +                       dev_warn(device, "MSIX address lost, re-configuring\n");
> +               pci_write_config_dword(pdev, PCI_BASE_ADDRESS_4,
> +                                      tp->saved_msix_addr_lo);
> +               pci_write_config_dword(pdev, PCI_BASE_ADDRESS_5,
> +                                      tp->saved_msix_addr_hi);
> +       }
>
>         if (netif_running(dev))
>                 __rtl8169_resume(dev);
> @@ -7076,11 +7091,6 @@ static int rtl_alloc_irq(struct rtl8169_private *tp)
>                 RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~MSIEnable);
>                 RTL_W8(tp, Cfg9346, Cfg9346_Lock);
>                 flags = PCI_IRQ_LEGACY;
> -       } else if (tp->mac_version == RTL_GIGA_MAC_VER_40) {
> -               /* This version was reported to have issues with resume
> -                * from suspend when using MSI-X
> -                */
> -               flags = PCI_IRQ_LEGACY | PCI_IRQ_MSI;
>         } else {
>                 flags = PCI_IRQ_ALL_TYPES;
>         }
> @@ -7355,6 +7365,13 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>                 return rc;
>         }
>
> +       if (pdev->msix_enabled) {
> +               pci_read_config_dword(pdev, PCI_BASE_ADDRESS_4,
> +                                     &tp->saved_msix_addr_lo);
> +               pci_read_config_dword(pdev, PCI_BASE_ADDRESS_5,
> +                                     &tp->saved_msix_addr_hi);
> +       }
> +
>         tp->saved_wolopts = __rtl8169_get_wol(tp);
>
>         mutex_init(&tp->wk.mutex);
> --
> 2.18.0
>