| 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
| ||
|
Message-ID: <34924fb0423c44c4ce39cf897bb8b2956e6ea66b.camel@intel.com>
Date: Mon, 20 May 2024 02:03:34 +0000
From: "Zhang, Rui" <rui.zhang@...el.com>
To: "naamax.meir@...ux.intel.com" <naamax.meir@...ux.intel.com>,
"hui.wang@...onical.com" <hui.wang@...onical.com>, "davem@...emloft.net"
<davem@...emloft.net>, "Ruinskiy, Dima" <dima.ruinskiy@...el.com>, "Lifshits,
Vitaly" <vitaly.lifshits@...el.com>, "netdev@...r.kernel.org"
<netdev@...r.kernel.org>, "kuba@...nel.org" <kuba@...nel.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>
Subject: Re: [Intel-wired-lan] [iwl-net][PATCH v3] e1000e: move force SMBUS
near the end of enable_ulp function
On Fri, 2024-05-17 at 21:50 +0800, Hui Wang wrote:
> The commit 861e8086029e ("e1000e: move force SMBUS from enable ulp
> function to avoid PHY loss issue") introduces a regression on
> PCH_MTP_I219_LM18 (PCIID: 0x8086550A). Without the referred commit,
> the
> ethernet works well after suspend and resume, but after applying the
> commit, the ethernet couldn't work anymore after the resume and the
> dmesg shows that the NIC link changes to 10Mbps (1000Mbps
> originally):
>
> [ 43.305084] e1000e 0000:00:1f.6 enp0s31f6: NIC Link is Up 10
> Mbps Full Duplex, Flow Control: Rx/Tx
>
> Without the commit, the force SMBUS code will not be executed if
> "return 0" or "goto out" is executed in the enable_ulp(), and in my
> case, the "goto out" is executed since FWSM_FW_VALID is set. But
> after
> applying the commit, the force SMBUS code will be ran
> unconditionally.
>
> Here move the force SMBUS code back to enable_ulp() and put it
> immediately ahead of hw->phy.ops.release(hw), this could allow the
> longest settling time as possible for interface in this function and
> doesn't change the original code logic.
>
> The issue was found on a Lenovo laptop with the ethernet hw as below:
> 00:1f.6 Ethernet controller [0200]: Intel Corporation Device
> [8086:550a]
> (rev 20).
>
> And this patch is verified (cable plug and unplug, system suspend
> and resume) on Lenovo laptops with ethernet hw: [8086:550a],
> [8086:550b], [8086:15bb], [8086:15be], [8086:1a1f], [8086:1a1c] and
> [8086:0dc7].
>
> Fixes: 861e8086029e ("e1000e: move force SMBUS from enable ulp
> function to avoid PHY loss issue")
> Signed-off-by: Hui Wang <hui.wang@...onical.com>
> Acked-by: Vitaly Lifshits <vitaly.lifshits@...el.com>
> Tested-by: Naama Meir <naamax.meir@...ux.intel.com>
> Reviewed-by: Simon Horman <horms@...nel.org>
> Reviewed-by: Paul Menzel <pmenzel@...gen.mpg.de>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@...el.com>
This indeed fixes another freeze regression on an Intel Broadwell NUC,
as reported in below thread,
https://lore.kernel.org/intel-wired-lan/202405150942.f9b873b1-oliver.sang@intel.com/
Tested-by: Zhang Rui <rui.zhang@...el.com>
thanks,
rui
> ---
> In the v3:
> addressed Paul's comment about commit header,
> - Change CH_MTP_I219_LM18 to PCH_MTP_I219_LM18
> - Change Link to link
> - Add a blank line and four spaces indent for [ 43.305084] e1000e
> 0000:00:1f.6
> - Change immediate to immediately
> - Add system info about reproduced the issue and verified the fix
>
> drivers/net/ethernet/intel/e1000e/ich8lan.c | 22
> +++++++++++++++++++++
> drivers/net/ethernet/intel/e1000e/netdev.c | 18 -----------------
> 2 files changed, 22 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c
> b/drivers/net/ethernet/intel/e1000e/ich8lan.c
> index f9e94be36e97..2e98a2a0bead 100644
> --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
> +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
> @@ -1225,6 +1225,28 @@ 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 3692fce20195..cc8c531ec3df 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -6623,7 +6623,6 @@ 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)
> @@ -6697,23 +6696,6 @@ 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
Powered by blists - more mailing lists