| 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, 28 Jan 2022 11:43:35 +0000
From: Hau <hau@...ltek.com>
To: Heiner Kallweit <hkallweit1@...il.com>,
Jakub Kicinski <kuba@...nel.org>,
David Miller <davem@...emloft.net>,
nic_swsd <nic_swsd@...ltek.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"grundler@...omium.org" <grundler@...omium.org>
Subject: RE: [PATCH net-next] r8169: add rtl_disable_exit_l1()
> On 28.01.2022 09:09, Hau wrote:
> >
> >
> >> -----Original Message-----
> >> From: Heiner Kallweit [mailto:hkallweit1@...il.com]
> >> Sent: Friday, January 28, 2022 6:15 AM
> >> To: Jakub Kicinski <kuba@...nel.org>; David Miller
> >> <davem@...emloft.net>; nic_swsd <nic_swsd@...ltek.com>
> >> Cc: netdev@...r.kernel.org; Hau <hau@...ltek.com>
> >> Subject: [PATCH net-next] r8169: add rtl_disable_exit_l1()
> >>
> >> Add rtl_disable_exit_l1() for ensuring that the chip doesn't
> >> inadvertently exit ASPM L1 when being in a low-power mode.
> >> The new function is called from rtl_prepare_power_down() which has to
> >> be moved in the code to avoid a forward declaration.
> >>
> >> According to Realtek OCP register 0xc0ac shadows ERI register 0xd4 on
> >> RTL8168 versions from RTL8168g. This allows to simplify the code a little.
> >>
> >> Suggested-by: Chun-Hao Lin <hau@...ltek.com>
> >> Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>
> >> ---
> >> drivers/net/ethernet/realtek/r8169_main.c | 65
> >> ++++++++++++++---------
> >> 1 file changed, 39 insertions(+), 26 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/realtek/r8169_main.c
> >> b/drivers/net/ethernet/realtek/r8169_main.c
> >> index 3c3d1506b..104ebc0fb 100644
> >> --- a/drivers/net/ethernet/realtek/r8169_main.c
> >> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> >> @@ -2231,28 +2231,6 @@ static int rtl_set_mac_address(struct
> >> net_device *dev, void *p)
> >> return 0;
> >> }
> >>
> >> -static void rtl_wol_enable_rx(struct rtl8169_private *tp) -{
> >> - if (tp->mac_version >= RTL_GIGA_MAC_VER_25)
> >> - RTL_W32(tp, RxConfig, RTL_R32(tp, RxConfig) |
> >> - AcceptBroadcast | AcceptMulticast | AcceptMyPhys);
> >> -}
> >> -
> >> -static void rtl_prepare_power_down(struct rtl8169_private *tp) -{
> >> - if (tp->dash_type != RTL_DASH_NONE)
> >> - return;
> >> -
> >> - if (tp->mac_version == RTL_GIGA_MAC_VER_32 ||
> >> - tp->mac_version == RTL_GIGA_MAC_VER_33)
> >> - rtl_ephy_write(tp, 0x19, 0xff64);
> >> -
> >> - if (device_may_wakeup(tp_to_dev(tp))) {
> >> - phy_speed_down(tp->phydev, false);
> >> - rtl_wol_enable_rx(tp);
> >> - }
> >> -}
> >> -
> >> static void rtl_init_rxcfg(struct rtl8169_private *tp) {
> >> switch (tp->mac_version) {
> >> @@ -2667,10 +2645,7 @@ static void rtl_enable_exit_l1(struct
> >> rtl8169_private *tp)
> >> case RTL_GIGA_MAC_VER_37 ... RTL_GIGA_MAC_VER_38:
> >> rtl_eri_set_bits(tp, 0xd4, 0x0c00);
> >> break;
> >> - case RTL_GIGA_MAC_VER_40 ... RTL_GIGA_MAC_VER_53:
> >> - rtl_eri_set_bits(tp, 0xd4, 0x1f80);
> >> - break;
> >> - case RTL_GIGA_MAC_VER_60 ... RTL_GIGA_MAC_VER_63:
> >> + case RTL_GIGA_MAC_VER_40 ... RTL_GIGA_MAC_VER_63:
> >> r8168_mac_ocp_modify(tp, 0xc0ac, 0, 0x1f80);
> >> break;
> >> default:
> >> @@ -2678,6 +2653,20 @@ static void rtl_enable_exit_l1(struct
> >> rtl8169_private *tp)
> >> }
> >> }
> >>
> >> +static void rtl_disable_exit_l1(struct rtl8169_private *tp) {
> >> + switch (tp->mac_version) {
> >> + case RTL_GIGA_MAC_VER_34 ... RTL_GIGA_MAC_VER_38:
> >> + rtl_eri_clear_bits(tp, 0xd4, 0x1f00);
> >> + break;
> >> + case RTL_GIGA_MAC_VER_40 ... RTL_GIGA_MAC_VER_63:
> >> + r8168_mac_ocp_modify(tp, 0xc0ac, 0x1f80, 0);
> >> + break;
> >> + default:
> >> + break;
> >> + }
> >> +}
> >> +
> >> static void rtl_hw_aspm_clkreq_enable(struct rtl8169_private *tp,
> >> bool
> >> enable) {
> >> /* Don't enable ASPM in the chip if OS can't control ASPM */ @@ -
> >> 4689,6 +4678,30 @@ static int r8169_phy_connect(struct rtl8169_private
> *tp)
> >> return 0;
> >> }
> >>
> >> +static void rtl_wol_enable_rx(struct rtl8169_private *tp) {
> >> + if (tp->mac_version >= RTL_GIGA_MAC_VER_25)
> >> + RTL_W32(tp, RxConfig, RTL_R32(tp, RxConfig) |
> >> + AcceptBroadcast | AcceptMulticast | AcceptMyPhys); }
> >> +
> >> +static void rtl_prepare_power_down(struct rtl8169_private *tp) {
> >> + if (tp->dash_type != RTL_DASH_NONE)
> >> + return;
> >> +
> >> + if (tp->mac_version == RTL_GIGA_MAC_VER_32 ||
> >> + tp->mac_version == RTL_GIGA_MAC_VER_33)
> >> + rtl_ephy_write(tp, 0x19, 0xff64);
> >> +
> >> + if (device_may_wakeup(tp_to_dev(tp))) {
> >> + phy_speed_down(tp->phydev, false);
> >> + rtl_wol_enable_rx(tp);
> >> + } else {
> >> + rtl_disable_exit_l1(tp);
> >> + }
> >> +}
> >> +
> > Hi Heiner,
> >
> Hi Hau,
>
> > rtl_disable_exit_l1(tp) can be called before device enter D3 state. I think
> you don’t need to check wol status.
> > You may update the code link following.
>
> my thought was that if DASH or WoL are enabled then we might miss
> something on the bus if not waking from L1 in time.
> You think this shouldn't be a problem?
>
I think it should not be a problem.
If driver does not call rtl_disable_exit_l1() when wol is enabled, then hardware will not go to L1.
> By the way, because I'm no DASH expert:
> Should we go to D3 at all if DASH is enabled? Will it still work?
>
For hardware that support dash, dash will work in D3 state.
Before D3 state, driver will give control back to dash firmware by call rtl8168_driver_stop().
> >
> > static void rtl_prepare_power_down(struct rtl8169_private *tp) {
> > rtl_disable_exit_l1(tp);
> >
> > if (tp->dash_type != RTL_DASH_NONE)
> > return;
> >
> > if (tp->mac_version == RTL_GIGA_MAC_VER_32 ||
> > tp->mac_version == RTL_GIGA_MAC_VER_33)
> > rtl_ephy_write(tp, 0x19, 0xff64);
> >
> > if (device_may_wakeup(tp_to_dev(tp))) {
> > phy_speed_down(tp->phydev, false);
> > rtl_wol_enable_rx(tp);
> > }
> > }
> >
> > Thanks.
> >> static void rtl8169_down(struct rtl8169_private *tp) {
> >> /* Clear all task flags */
> >> --
> >> 2.35.0
> >>
> >> ------Please consider the environment before printing this e-mail.
Powered by blists - more mailing lists