lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 13 Aug 2021 18:11:06 +0800 From: Kai-Heng Feng <kai.heng.feng@...onical.com> To: Heiner Kallweit <hkallweit1@...il.com> Cc: nic_swsd <nic_swsd@...ltek.com>, "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, "open list:8169 10/100/1000 GIGABIT ETHERNET DRIVER" <netdev@...r.kernel.org>, open list <linux-kernel@...r.kernel.org> Subject: Re: [PATCH v2 2/2] r8169: Enable ASPM for selected NICs On Fri, Aug 13, 2021 at 3:39 AM Heiner Kallweit <hkallweit1@...il.com> wrote: > > On 12.08.2021 17:53, Kai-Heng Feng wrote: > > The latest vendor driver enables ASPM for more recent r8168 NICs, do the > > same here to match the behavior. > > > > In addition, pci_disable_link_state() is only used for RTL8168D/8111D in > > vendor driver, also match that. > > > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@...onical.com> > > --- > > v2: > > - No change > > > > drivers/net/ethernet/realtek/r8169_main.c | 34 +++++++++++++++++------ > > 1 file changed, 26 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c > > index 7ab2e841dc69..caa29e72a21a 100644 > > --- a/drivers/net/ethernet/realtek/r8169_main.c > > +++ b/drivers/net/ethernet/realtek/r8169_main.c > > @@ -623,7 +623,7 @@ struct rtl8169_private { > > } wk; > > > > unsigned supports_gmii:1; > > - unsigned aspm_manageable:1; > > + unsigned aspm_supported:1; > > unsigned aspm_enabled:1; > > struct delayed_work aspm_toggle; > > struct mutex aspm_mutex; > > @@ -2667,8 +2667,11 @@ static void rtl_pcie_state_l2l3_disable(struct rtl8169_private *tp) > > > > static void rtl_hw_aspm_clkreq_enable(struct rtl8169_private *tp, bool enable) > > { > > + if (!tp->aspm_supported) > > + return; > > + > > /* Don't enable ASPM in the chip if OS can't control ASPM */ > > - if (enable && tp->aspm_manageable) { > > + if (enable) { > > RTL_W8(tp, Config5, RTL_R8(tp, Config5) | ASPM_en); > > RTL_W8(tp, Config2, RTL_R8(tp, Config2) | ClkReqEn); > > } else { > > @@ -5284,6 +5287,21 @@ static void rtl_init_mac_address(struct rtl8169_private *tp) > > rtl_rar_set(tp, mac_addr); > > } > > > > +static int rtl_hw_aspm_supported(struct rtl8169_private *tp) > > +{ > > + switch (tp->mac_version) { > > + case RTL_GIGA_MAC_VER_32 ... RTL_GIGA_MAC_VER_36: > > + case RTL_GIGA_MAC_VER_38: > > + case RTL_GIGA_MAC_VER_40 ... RTL_GIGA_MAC_VER_42: > > + case RTL_GIGA_MAC_VER_44 ... RTL_GIGA_MAC_VER_46: > > + case RTL_GIGA_MAC_VER_49 ... RTL_GIGA_MAC_VER_63: > > This shouldn't be needed because ASPM support is announced the > standard PCI way. Max a blacklist should be needed if there are > chip versions that announce ASPM support whilst in reality they > do not support it (or support is completely broken). So can we also remove aspm_manageable since blacklist will be used? Kai-Heng > > > + return 1; > > + > > + default: > > + return 0; > > + } > > +} > > + > > static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) > > { > > struct rtl8169_private *tp; > > @@ -5315,12 +5333,12 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) > > if (rc) > > return rc; > > > > - /* Disable ASPM completely as that cause random device stop working > > - * problems as well as full system hangs for some PCIe devices users. > > - */ > > - rc = pci_disable_link_state(pdev, PCIE_LINK_STATE_L0S | > > - PCIE_LINK_STATE_L1); > > - tp->aspm_manageable = !rc; > > + if (tp->mac_version == RTL_GIGA_MAC_VER_25) > > + pci_disable_link_state(pdev, PCIE_LINK_STATE_L0S | > > + PCIE_LINK_STATE_L1 | > > + PCIE_LINK_STATE_CLKPM); > > + > > + tp->aspm_supported = rtl_hw_aspm_supported(tp); > > > > /* enable device (incl. PCI PM wakeup and hotplug setup) */ > > rc = pcim_enable_device(pdev); > > >
Powered by blists - more mailing lists