| 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: Mon, 10 Jun 2024 14:14:26 +0000
From: "Zhang, Rui" <rui.zhang@...el.com>
To: "pmenzel@...gen.mpg.de" <pmenzel@...gen.mpg.de>, "hui.wang@...onical.com"
<hui.wang@...onical.com>, "davem@...emloft.net" <davem@...emloft.net>,
"naamax.meir@...ux.intel.com" <naamax.meir@...ux.intel.com>, "Ruinskiy, Dima"
<dima.ruinskiy@...el.com>, "Lifshits, Vitaly" <vitaly.lifshits@...el.com>,
"kuba@...nel.org" <kuba@...nel.org>, "netdev@...r.kernel.org"
<netdev@...r.kernel.org>, "intel-wired-lan@...ts.osuosl.org"
<intel-wired-lan@...ts.osuosl.org>, "Nguyen, Anthony L"
<anthony.l.nguyen@...el.com>, "Neftin, Sasha" <sasha.neftin@...el.com>,
"edumazet@...gle.com" <edumazet@...gle.com>, "pabeni@...hat.com"
<pabeni@...hat.com>
CC: "Keller, Jacob E" <jacob.e.keller@...el.com>, "horms@...nel.org"
<horms@...nel.org>, "Brandt, Todd E" <todd.e.brandt@...el.com>,
"dmummenschanz@....de" <dmummenschanz@....de>, "regressions@...ts.linux.dev"
<regressions@...ts.linux.dev>
Subject: Re: [iwl-net][PATCH] Revert "e1000e: move force SMBUS near the end of
enable_ulp function"
On Mon, 2024-06-10 at 08:36 +0200, Paul Menzel wrote:
> Dear Hui,
>
>
> Thank you for your patch.
>
>
> Am 10.06.24 um 03:32 schrieb Hui Wang:
> > This reverts commit bfd546a552e140b0a4c8a21527c39d6d21addb28
> >
> > Commit bfd546a552e1 ("e1000e: move force SMBUS near the end of
> > enable_ulp function") introduces system suspend failure on some
> > ethernet cards, at the moment, the pciid of the affected ethernet
> > cards include [8086:15b8] and [8086:15bc].
> >
> > About the regression the commit bfd546a552e1 ("e1000e: move force
>
> … regression introduced by commit …
>
> > SMBUS near the end of enable_ulp function") tried to fix, looks
> > like
> > it is not trivial to fix, we need to find a better way to resolve
> > it.
>
> Please send a revert for commit 861e8086029e (e1000e: move force
> SMBUS
> from enable ulp function to avoid PHY loss issue), present since
> Linux
> v6.9-rc3 and not containing enough information in the commit
> messsage,
> so we have a proper baseline. (That’s also why I originally suggested
> to
> split it into two commits (revert + your change).)
>
> > Reported-by: Todd Brandt <todd.e.brandt@...el.com>
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218940
> > Reported-by: Dieter Mummenschanz <dmummenschanz@....de>
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218936
> > Signed-off-by: Hui Wang <hui.wang@...onical.com>
> > ---
> > drivers/net/ethernet/intel/e1000e/ich8lan.c | 22 ----------------
> > -----
> > drivers/net/ethernet/intel/e1000e/netdev.c | 18
> > +++++++++++++++++
> > 2 files changed, 18 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c
> > b/drivers/net/ethernet/intel/e1000e/ich8lan.c
> > index 2e98a2a0bead..f9e94be36e97 100644
> > --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
> > +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
> > @@ -1225,28 +1225,6 @@ s32 e1000_enable_ulp_lpt_lp(struct e1000_hw
> > *hw, bool to_sx)
> > }
> >
> > release:
> > - /* Switching PHY interface always returns MDI error
> > - * so disable retry mechanism to avoid wasting time
> > - */
> > - e1000e_disable_phy_retry(hw);
> > -
> > - /* Force SMBus mode in PHY */
> > - ret_val = e1000_read_phy_reg_hv_locked(hw, CV_SMB_CTRL,
> > &phy_reg);
> > - if (ret_val) {
> > - e1000e_enable_phy_retry(hw);
> > - hw->phy.ops.release(hw);
> > - goto out;
> > - }
> > - phy_reg |= CV_SMB_CTRL_FORCE_SMBUS;
> > - e1000_write_phy_reg_hv_locked(hw, CV_SMB_CTRL, phy_reg);
> > -
> > - e1000e_enable_phy_retry(hw);
> > -
> > - /* Force SMBus mode in MAC */
> > - mac_reg = er32(CTRL_EXT);
> > - mac_reg |= E1000_CTRL_EXT_FORCE_SMBUS;
> > - ew32(CTRL_EXT, mac_reg);
> > -
> > hw->phy.ops.release(hw);
> > out:
> > if (ret_val)
> > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c
> > b/drivers/net/ethernet/intel/e1000e/netdev.c
> > index da5c59daf8ba..220d62fca55d 100644
> > --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> > +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> > @@ -6623,6 +6623,7 @@ static int __e1000_shutdown(struct pci_dev
> > *pdev, bool runtime)
> > struct e1000_hw *hw = &adapter->hw;
> > u32 ctrl, ctrl_ext, rctl, status, wufc;
> > int retval = 0;
> > + u16 smb_ctrl;
> >
> > /* Runtime suspend should only enable wakeup for link
> > changes */
> > if (runtime)
> > @@ -6696,6 +6697,23 @@ static int __e1000_shutdown(struct pci_dev
> > *pdev, bool runtime)
> > if (retval)
> > return retval;
> > }
> > +
> > + /* Force SMBUS to allow WOL */
> > + /* Switching PHY interface always returns MDI error
> > + * so disable retry mechanism to avoid wasting time
> > + */
> > + e1000e_disable_phy_retry(hw);
> > +
> > + e1e_rphy(hw, CV_SMB_CTRL, &smb_ctrl);
> > + smb_ctrl |= CV_SMB_CTRL_FORCE_SMBUS;
> > + e1e_wphy(hw, CV_SMB_CTRL, smb_ctrl);
> > +
> > + e1000e_enable_phy_retry(hw);
> > +
> > + /* Force SMBus mode in MAC */
> > + ctrl_ext = er32(CTRL_EXT);
> > + ctrl_ext |= E1000_CTRL_EXT_FORCE_SMBUS;
> > + ew32(CTRL_EXT, ctrl_ext);
> > }
> >
> > /* Ensure that the appropriate bits are set in LPI_CTRL
>
> Naama also added Tested-by lines two both commits in question. Could
> Intel’s test coverage please extended to the problem at hand?
>
> Acked-by: Paul Menzel <pmenzel@...gen.mpg.de>
Plus that,
1. Todd and I can test with upstream + this patch to confirm that
a. the regression for Todd is gone.
b. the s2idle failure for me is back
2. I can test with upstream + this patch + revert of commit
861e8086029e (e1000e: move force SMBUS from enable ulp function to
avoid PHY loss issue) to confirm s2idle is working again.
thanks,
rui
>
>
> Kind regards,
>
> Paul
Powered by blists - more mailing lists