Message ID | 20201214192935.895174-1-mario.limonciello@dell.com |
---|---|
Headers | show |
Series | Improve s0ix flows for systems i219LM | expand |
Hi, On 12/14/20 8:29 PM, Mario Limonciello wrote: > commit e086ba2fccda ("e1000e: disable s0ix entry and exit flows for ME systems") > disabled s0ix flows for systems that have various incarnations of the > i219-LM ethernet controller. This was done because of some regressions > caused by an earlier > commit 632fbd5eb5b0e ("e1000e: fix S0ix flows for cable connected case") > with i219-LM controller. > > Per discussion with Intel architecture team this direction should be changed and > allow S0ix flows to be used by default. This patch series includes directional > changes for their conclusions in https://lkml.org/lkml/2020/12/13/15. > > Changes from v4 to v5: > - If setting S0ix to enabled in ethtool examine the hardware generation. > If running on hardware older than Cannon Point return an error. > - Increase ULP timeout to 2.5 seconds, but show a warning after 1 second. Thank you. I've given v5 a test on a Lenovo X1 Carbon 8th gen (AMT capable) and things work fine there with v5: Tested-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > Changes from v3 to v4: > - Drop patch 1 for proper s0i3.2 entry, it was separated and is now merged in kernel > - Add patch to only run S0ix flows if shutdown succeeded which was suggested in > thread > - Adjust series for guidance from https://lkml.org/lkml/2020/12/13/15 > * Revert i219-LM disallow-list. > * Drop all patches for systems tested by Dell in an allow list > * Increase ULP timeout to 1000ms > Changes from v2 to v3: > - Correct some grammar and spelling issues caught by Bjorn H. > * s/s0ix/S0ix/ in all commit messages > * Fix a typo in commit message > * Fix capitalization of proper nouns > - Add more pre-release systems that pass > - Re-order the series to add systems only at the end of the series > - Add Fixes tag to a patch in series. > > Changes from v1 to v2: > - Directly incorporate Vitaly's dependency patch in the series > - Split out s0ix code into it's own file > - Adjust from DMI matching to PCI subsystem vendor ID/device matching > - Remove module parameter and sysfs, use ethtool flag instead. > - Export s0ix flag to ethtool private flags > - Include more people and lists directly in this submission chain. > > > Mario Limonciello (4): > e1000e: Only run S0ix flows if shutdown succeeded > e1000e: bump up timeout to wait when ME un-configures ULP mode > Revert "e1000e: disable s0ix entry and exit flows for ME systems" > e1000e: Export S0ix flags to ethtool > > drivers/net/ethernet/intel/e1000e/e1000.h | 1 + > drivers/net/ethernet/intel/e1000e/ethtool.c | 46 ++++++++++++++++ > drivers/net/ethernet/intel/e1000e/ich8lan.c | 16 ++++-- > drivers/net/ethernet/intel/e1000e/netdev.c | 59 ++++----------------- > 4 files changed, 70 insertions(+), 52 deletions(-) > > -- > 2.25.1 >
Hi, On 12/14/20 8:29 PM, Mario Limonciello wrote: > Per guidance from Intel ethernet architecture team, it may take > up to 1 second for unconfiguring ULP mode. > > However in practice this seems to be taking up to 2 seconds on > some Lenovo machines. Detect scenarios that take more than 1 second > but less than 2.5 seconds and emit a warning on resume for those > scenarios. > > Suggested-by: Aaron Ma <aaron.ma@canonical.com> > Suggested-by: Sasha Netfin <sasha.neftin@intel.com> > Suggested-by: Hans de Goede <hdegoede@redhat.com> > CC: Mark Pearson <markpearson@lenovo.com> > Fixes: f15bb6dde738cc8fa0 ("e1000e: Add support for S0ix") > BugLink: https://bugs.launchpad.net/bugs/1865570 > Link: https://patchwork.ozlabs.org/project/intel-wired-lan/patch/20200323191639.48826-1-aaron.ma@canonical.com/ > Link: https://lkml.org/lkml/2020/12/13/15 > Link: https://lkml.org/lkml/2020/12/14/708 > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com> Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > --- > drivers/net/ethernet/intel/e1000e/ich8lan.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c > index 9aa6fad8ed47..fdf23d20c954 100644 > --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c > +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c > @@ -1240,6 +1240,9 @@ static s32 e1000_disable_ulp_lpt_lp(struct e1000_hw *hw, bool force) > return 0; > > if (er32(FWSM) & E1000_ICH_FWSM_FW_VALID) { > + struct e1000_adapter *adapter = hw->adapter; > + bool firmware_bug = false; > + > if (force) { > /* Request ME un-configure ULP mode in the PHY */ > mac_reg = er32(H2ME); > @@ -1248,16 +1251,23 @@ static s32 e1000_disable_ulp_lpt_lp(struct e1000_hw *hw, bool force) > ew32(H2ME, mac_reg); > } > > - /* Poll up to 300msec for ME to clear ULP_CFG_DONE. */ > + /* Poll up to 2.5 seconds for ME to clear ULP_CFG_DONE. > + * If this takes more than 1 second, show a warning indicating a firmware > + * bug */ > while (er32(FWSM) & E1000_FWSM_ULP_CFG_DONE) { > - if (i++ == 30) { > + if (i++ == 250) { > ret_val = -E1000_ERR_PHY; > goto out; > } > + if (i > 100 && !firmware_bug) > + firmware_bug = true; > > usleep_range(10000, 11000); > } > - e_dbg("ULP_CONFIG_DONE cleared after %dmsec\n", i * 10); > + if (firmware_bug) > + e_warn("ULP_CONFIG_DONE took %dmsec. This is a firmware bug\n", i * 10); > + else > + e_dbg("ULP_CONFIG_DONE cleared after %dmsec\n", i * 10); > > if (force) { > mac_reg = er32(H2ME); >
Hi, On 12/14/20 8:29 PM, Mario Limonciello wrote: > commit e086ba2fccda ("e1000e: disable s0ix entry and exit flows for ME systems") > disabled s0ix flows for systems that have various incarnations of the > i219-LM ethernet controller. This changed caused power consumption regressions > on the following shipping Dell Comet Lake based laptops: > * Latitude 5310 > * Latitude 5410 > * Latitude 5410 > * Latitude 5510 > * Precision 3550 > * Latitude 5411 > * Latitude 5511 > * Precision 3551 > * Precision 7550 > * Precision 7750 > > This commit was introduced because of some regressions on certain Thinkpad > laptops. This comment was potentially caused by an earlier > commit 632fbd5eb5b0e ("e1000e: fix S0ix flows for cable connected case"). > or it was possibly caused by a system not meeting platform architectural > requirements for low power consumption. Other changes made in the driver > with extended timeouts are expected to make the driver more impervious to > platform firmware behavior. > > Fixes: e086ba2fccda ("e1000e: disable s0ix entry and exit flows for ME systems") > Reviewed-by: Alexander Duyck <alexander.duyck@gmail.com> > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com> Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > --- > drivers/net/ethernet/intel/e1000e/netdev.c | 45 +--------------------- > 1 file changed, 2 insertions(+), 43 deletions(-) > > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c > index 6588f5d4a2be..b9800ba2006c 100644 > --- a/drivers/net/ethernet/intel/e1000e/netdev.c > +++ b/drivers/net/ethernet/intel/e1000e/netdev.c > @@ -103,45 +103,6 @@ static const struct e1000_reg_info e1000_reg_info_tbl[] = { > {0, NULL} > }; > > -struct e1000e_me_supported { > - u16 device_id; /* supported device ID */ > -}; > - > -static const struct e1000e_me_supported me_supported[] = { > - {E1000_DEV_ID_PCH_LPT_I217_LM}, > - {E1000_DEV_ID_PCH_LPTLP_I218_LM}, > - {E1000_DEV_ID_PCH_I218_LM2}, > - {E1000_DEV_ID_PCH_I218_LM3}, > - {E1000_DEV_ID_PCH_SPT_I219_LM}, > - {E1000_DEV_ID_PCH_SPT_I219_LM2}, > - {E1000_DEV_ID_PCH_LBG_I219_LM3}, > - {E1000_DEV_ID_PCH_SPT_I219_LM4}, > - {E1000_DEV_ID_PCH_SPT_I219_LM5}, > - {E1000_DEV_ID_PCH_CNP_I219_LM6}, > - {E1000_DEV_ID_PCH_CNP_I219_LM7}, > - {E1000_DEV_ID_PCH_ICP_I219_LM8}, > - {E1000_DEV_ID_PCH_ICP_I219_LM9}, > - {E1000_DEV_ID_PCH_CMP_I219_LM10}, > - {E1000_DEV_ID_PCH_CMP_I219_LM11}, > - {E1000_DEV_ID_PCH_CMP_I219_LM12}, > - {E1000_DEV_ID_PCH_TGP_I219_LM13}, > - {E1000_DEV_ID_PCH_TGP_I219_LM14}, > - {E1000_DEV_ID_PCH_TGP_I219_LM15}, > - {0} > -}; > - > -static bool e1000e_check_me(u16 device_id) > -{ > - struct e1000e_me_supported *id; > - > - for (id = (struct e1000e_me_supported *)me_supported; > - id->device_id; id++) > - if (device_id == id->device_id) > - return true; > - > - return false; > -} > - > /** > * __ew32_prepare - prepare to write to MAC CSR register on certain parts > * @hw: pointer to the HW structure > @@ -6974,8 +6935,7 @@ static __maybe_unused int e1000e_pm_suspend(struct device *dev) > e1000e_pm_thaw(dev); > } else { > /* Introduce S0ix implementation */ > - if (hw->mac.type >= e1000_pch_cnp && > - !e1000e_check_me(hw->adapter->pdev->device)) > + if (hw->mac.type >= e1000_pch_cnp) > e1000e_s0ix_entry_flow(adapter); > } > > @@ -6991,8 +6951,7 @@ static __maybe_unused int e1000e_pm_resume(struct device *dev) > int rc; > > /* Introduce S0ix implementation */ > - if (hw->mac.type >= e1000_pch_cnp && > - !e1000e_check_me(hw->adapter->pdev->device)) > + if (hw->mac.type >= e1000_pch_cnp) > e1000e_s0ix_exit_flow(adapter); > > rc = __e1000_resume(pdev); >
Hi, On 12/14/20 8:29 PM, Mario Limonciello wrote: > This flag can be used by an end user to disable S0ix flows on a > buggy system or by an OEM for development purposes. > > If you need this flag to be persisted across reboots, it's suggested > to use a udev rule to call adjust it until the kernel could have your > configuration in a disallow list. > > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com> Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > --- > drivers/net/ethernet/intel/e1000e/e1000.h | 1 + > drivers/net/ethernet/intel/e1000e/ethtool.c | 46 +++++++++++++++++++++ > drivers/net/ethernet/intel/e1000e/netdev.c | 9 ++-- > 3 files changed, 52 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h > index ba7a0f8f6937..5b2143f4b1f8 100644 > --- a/drivers/net/ethernet/intel/e1000e/e1000.h > +++ b/drivers/net/ethernet/intel/e1000e/e1000.h > @@ -436,6 +436,7 @@ s32 e1000e_get_base_timinca(struct e1000_adapter *adapter, u32 *timinca); > #define FLAG2_DFLT_CRC_STRIPPING BIT(12) > #define FLAG2_CHECK_RX_HWTSTAMP BIT(13) > #define FLAG2_CHECK_SYSTIM_OVERFLOW BIT(14) > +#define FLAG2_ENABLE_S0IX_FLOWS BIT(15) > > #define E1000_RX_DESC_PS(R, i) \ > (&(((union e1000_rx_desc_packet_split *)((R).desc))[i])) > diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c > index 03215b0aee4b..06442e6bef73 100644 > --- a/drivers/net/ethernet/intel/e1000e/ethtool.c > +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c > @@ -23,6 +23,13 @@ struct e1000_stats { > int stat_offset; > }; > > +static const char e1000e_priv_flags_strings[][ETH_GSTRING_LEN] = { > +#define E1000E_PRIV_FLAGS_S0IX_ENABLED BIT(0) > + "s0ix-enabled", > +}; > + > +#define E1000E_PRIV_FLAGS_STR_LEN ARRAY_SIZE(e1000e_priv_flags_strings) > + > #define E1000_STAT(str, m) { \ > .stat_string = str, \ > .type = E1000_STATS, \ > @@ -1776,6 +1783,8 @@ static int e1000e_get_sset_count(struct net_device __always_unused *netdev, > return E1000_TEST_LEN; > case ETH_SS_STATS: > return E1000_STATS_LEN; > + case ETH_SS_PRIV_FLAGS: > + return E1000E_PRIV_FLAGS_STR_LEN; > default: > return -EOPNOTSUPP; > } > @@ -2097,6 +2106,10 @@ static void e1000_get_strings(struct net_device __always_unused *netdev, > p += ETH_GSTRING_LEN; > } > break; > + case ETH_SS_PRIV_FLAGS: > + memcpy(data, e1000e_priv_flags_strings, > + E1000E_PRIV_FLAGS_STR_LEN * ETH_GSTRING_LEN); > + break; > } > } > > @@ -2305,6 +2318,37 @@ static int e1000e_get_ts_info(struct net_device *netdev, > return 0; > } > > +static u32 e1000e_get_priv_flags(struct net_device *netdev) > +{ > + struct e1000_adapter *adapter = netdev_priv(netdev); > + u32 priv_flags = 0; > + > + if (adapter->flags2 & FLAG2_ENABLE_S0IX_FLOWS) > + priv_flags |= E1000E_PRIV_FLAGS_S0IX_ENABLED; > + > + return priv_flags; > +} > + > +static int e1000e_set_priv_flags(struct net_device *netdev, u32 priv_flags) > +{ > + struct e1000_adapter *adapter = netdev_priv(netdev); > + unsigned int flags2 = adapter->flags2; > + > + flags2 &= ~FLAG2_ENABLE_S0IX_FLOWS; > + if (priv_flags & E1000E_PRIV_FLAGS_S0IX_ENABLED) { > + struct e1000_hw *hw = &adapter->hw; > + > + if (hw->mac.type < e1000_pch_cnp) > + return -EINVAL; > + flags2 |= FLAG2_ENABLE_S0IX_FLOWS; > + } > + > + if (flags2 != adapter->flags2) > + adapter->flags2 = flags2; > + > + return 0; > +} > + > static const struct ethtool_ops e1000_ethtool_ops = { > .supported_coalesce_params = ETHTOOL_COALESCE_RX_USECS, > .get_drvinfo = e1000_get_drvinfo, > @@ -2336,6 +2380,8 @@ static const struct ethtool_ops e1000_ethtool_ops = { > .set_eee = e1000e_set_eee, > .get_link_ksettings = e1000_get_link_ksettings, > .set_link_ksettings = e1000_set_link_ksettings, > + .get_priv_flags = e1000e_get_priv_flags, > + .set_priv_flags = e1000e_set_priv_flags, > }; > > void e1000e_set_ethtool_ops(struct net_device *netdev) > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c > index b9800ba2006c..e9b82c209c2d 100644 > --- a/drivers/net/ethernet/intel/e1000e/netdev.c > +++ b/drivers/net/ethernet/intel/e1000e/netdev.c > @@ -6923,7 +6923,6 @@ static __maybe_unused int e1000e_pm_suspend(struct device *dev) > struct net_device *netdev = pci_get_drvdata(to_pci_dev(dev)); > struct e1000_adapter *adapter = netdev_priv(netdev); > struct pci_dev *pdev = to_pci_dev(dev); > - struct e1000_hw *hw = &adapter->hw; > int rc; > > e1000e_flush_lpic(pdev); > @@ -6935,7 +6934,7 @@ static __maybe_unused int e1000e_pm_suspend(struct device *dev) > e1000e_pm_thaw(dev); > } else { > /* Introduce S0ix implementation */ > - if (hw->mac.type >= e1000_pch_cnp) > + if (adapter->flags2 & FLAG2_ENABLE_S0IX_FLOWS) > e1000e_s0ix_entry_flow(adapter); > } > > @@ -6947,11 +6946,10 @@ static __maybe_unused int e1000e_pm_resume(struct device *dev) > struct net_device *netdev = pci_get_drvdata(to_pci_dev(dev)); > struct e1000_adapter *adapter = netdev_priv(netdev); > struct pci_dev *pdev = to_pci_dev(dev); > - struct e1000_hw *hw = &adapter->hw; > int rc; > > /* Introduce S0ix implementation */ > - if (hw->mac.type >= e1000_pch_cnp) > + if (adapter->flags2 & FLAG2_ENABLE_S0IX_FLOWS) > e1000e_s0ix_exit_flow(adapter); > > rc = __e1000_resume(pdev); > @@ -7615,6 +7613,9 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > if (!(adapter->flags & FLAG_HAS_AMT)) > e1000e_get_hw_control(adapter); > > + if (hw->mac.type >= e1000_pch_cnp) > + adapter->flags2 |= FLAG2_ENABLE_S0IX_FLOWS; > + > strlcpy(netdev->name, "eth%d", sizeof(netdev->name)); > err = register_netdev(netdev); > if (err) >
> -----Original Message----- > From: Limonciello, Mario <Mario_Limonciello@Dell.com> > Sent: Tuesday, December 15, 2020 3:30 AM > To: Jeff Kirsher; Tony Nguyen; intel-wired-lan@lists.osuosl.org > Cc: linux-kernel@vger.kernel.org; Netdev; Alexander Duyck; Jakub Kicinski; > Sasha Netfin; Aaron Brown; Stefan Assmann; David Miller; > darcari@redhat.com; Shen, Yijun; Yuan, Perry; > anthony.wong@canonical.com; Hans de Goede; Limonciello, Mario; Aaron > Ma; Mark Pearson > Subject: [PATCH v5 2/4] e1000e: bump up timeout to wait when ME un- > configures ULP mode > > Per guidance from Intel ethernet architecture team, it may take up to 1 > second for unconfiguring ULP mode. > > However in practice this seems to be taking up to 2 seconds on some Lenovo > machines. Detect scenarios that take more than 1 second but less than 2.5 > seconds and emit a warning on resume for those scenarios. > > Suggested-by: Aaron Ma <aaron.ma@canonical.com> > Suggested-by: Sasha Netfin <sasha.neftin@intel.com> > Suggested-by: Hans de Goede <hdegoede@redhat.com> > CC: Mark Pearson <markpearson@lenovo.com> > Fixes: f15bb6dde738cc8fa0 ("e1000e: Add support for S0ix") > BugLink: https://bugs.launchpad.net/bugs/1865570 > Link: https://patchwork.ozlabs.org/project/intel-wired- > lan/patch/20200323191639.48826-1-aaron.ma@canonical.com/ > Link: https://lkml.org/lkml/2020/12/13/15 > Link: https://lkml.org/lkml/2020/12/14/708 > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com> Verified this series patches with Dell Systems. Tested-By: Yijun Shen <Yijun.shen@dell.com> > --- > drivers/net/ethernet/intel/e1000e/ich8lan.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c > b/drivers/net/ethernet/intel/e1000e/ich8lan.c > index 9aa6fad8ed47..fdf23d20c954 100644 > --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c > +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c > @@ -1240,6 +1240,9 @@ static s32 e1000_disable_ulp_lpt_lp(struct > e1000_hw *hw, bool force) > return 0; > > if (er32(FWSM) & E1000_ICH_FWSM_FW_VALID) { > + struct e1000_adapter *adapter = hw->adapter; > + bool firmware_bug = false; > + > if (force) { > /* Request ME un-configure ULP mode in the PHY */ > mac_reg = er32(H2ME); > @@ -1248,16 +1251,23 @@ static s32 e1000_disable_ulp_lpt_lp(struct > e1000_hw *hw, bool force) > ew32(H2ME, mac_reg); > } > > - /* Poll up to 300msec for ME to clear ULP_CFG_DONE. */ > + /* Poll up to 2.5 seconds for ME to clear ULP_CFG_DONE. > + * If this takes more than 1 second, show a warning indicating > a firmware > + * bug */ > while (er32(FWSM) & E1000_FWSM_ULP_CFG_DONE) { > - if (i++ == 30) { > + if (i++ == 250) { > ret_val = -E1000_ERR_PHY; > goto out; > } > + if (i > 100 && !firmware_bug) > + firmware_bug = true; > > usleep_range(10000, 11000); > } > - e_dbg("ULP_CONFIG_DONE cleared after %dmsec\n", i * 10); > + if (firmware_bug) > + e_warn("ULP_CONFIG_DONE took %dmsec. This is a > firmware bug\n", i * 10); > + else > + e_dbg("ULP_CONFIG_DONE cleared after %dmsec\n", > i * 10); > > if (force) { > mac_reg = er32(H2ME); > -- > 2.25.1
> -----Original Message----- > From: Limonciello, Mario <Mario_Limonciello@Dell.com> > Sent: Tuesday, December 15, 2020 3:30 AM > To: Jeff Kirsher; Tony Nguyen; intel-wired-lan@lists.osuosl.org > Cc: linux-kernel@vger.kernel.org; Netdev; Alexander Duyck; Jakub Kicinski; > Sasha Netfin; Aaron Brown; Stefan Assmann; David Miller; > darcari@redhat.com; Shen, Yijun; Yuan, Perry; > anthony.wong@canonical.com; Hans de Goede; Limonciello, Mario > Subject: [PATCH v5 3/4] Revert "e1000e: disable s0ix entry and exit flows for > ME systems" > > commit e086ba2fccda ("e1000e: disable s0ix entry and exit flows for ME > systems") disabled s0ix flows for systems that have various incarnations of > the i219-LM ethernet controller. This changed caused power consumption > regressions on the following shipping Dell Comet Lake based laptops: > * Latitude 5310 > * Latitude 5410 > * Latitude 5410 > * Latitude 5510 > * Precision 3550 > * Latitude 5411 > * Latitude 5511 > * Precision 3551 > * Precision 7550 > * Precision 7750 > > This commit was introduced because of some regressions on certain > Thinkpad laptops. This comment was potentially caused by an earlier > commit 632fbd5eb5b0e ("e1000e: fix S0ix flows for cable connected case"). > or it was possibly caused by a system not meeting platform architectural > requirements for low power consumption. Other changes made in the driver > with extended timeouts are expected to make the driver more impervious to > platform firmware behavior. > > Fixes: e086ba2fccda ("e1000e: disable s0ix entry and exit flows for ME > systems") > Reviewed-by: Alexander Duyck <alexander.duyck@gmail.com> > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com> Verified this series patches with Dell Systems. Tested-By: Yijun Shen <Yijun.shen@dell.com> > --- > drivers/net/ethernet/intel/e1000e/netdev.c | 45 +--------------------- > 1 file changed, 2 insertions(+), 43 deletions(-) > > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c > b/drivers/net/ethernet/intel/e1000e/netdev.c > index 6588f5d4a2be..b9800ba2006c 100644 > --- a/drivers/net/ethernet/intel/e1000e/netdev.c > +++ b/drivers/net/ethernet/intel/e1000e/netdev.c > @@ -103,45 +103,6 @@ static const struct e1000_reg_info > e1000_reg_info_tbl[] = { > {0, NULL} > }; > > -struct e1000e_me_supported { > - u16 device_id; /* supported device ID */ > -}; > - > -static const struct e1000e_me_supported me_supported[] = { > - {E1000_DEV_ID_PCH_LPT_I217_LM}, > - {E1000_DEV_ID_PCH_LPTLP_I218_LM}, > - {E1000_DEV_ID_PCH_I218_LM2}, > - {E1000_DEV_ID_PCH_I218_LM3}, > - {E1000_DEV_ID_PCH_SPT_I219_LM}, > - {E1000_DEV_ID_PCH_SPT_I219_LM2}, > - {E1000_DEV_ID_PCH_LBG_I219_LM3}, > - {E1000_DEV_ID_PCH_SPT_I219_LM4}, > - {E1000_DEV_ID_PCH_SPT_I219_LM5}, > - {E1000_DEV_ID_PCH_CNP_I219_LM6}, > - {E1000_DEV_ID_PCH_CNP_I219_LM7}, > - {E1000_DEV_ID_PCH_ICP_I219_LM8}, > - {E1000_DEV_ID_PCH_ICP_I219_LM9}, > - {E1000_DEV_ID_PCH_CMP_I219_LM10}, > - {E1000_DEV_ID_PCH_CMP_I219_LM11}, > - {E1000_DEV_ID_PCH_CMP_I219_LM12}, > - {E1000_DEV_ID_PCH_TGP_I219_LM13}, > - {E1000_DEV_ID_PCH_TGP_I219_LM14}, > - {E1000_DEV_ID_PCH_TGP_I219_LM15}, > - {0} > -}; > - > -static bool e1000e_check_me(u16 device_id) -{ > - struct e1000e_me_supported *id; > - > - for (id = (struct e1000e_me_supported *)me_supported; > - id->device_id; id++) > - if (device_id == id->device_id) > - return true; > - > - return false; > -} > - > /** > * __ew32_prepare - prepare to write to MAC CSR register on certain parts > * @hw: pointer to the HW structure > @@ -6974,8 +6935,7 @@ static __maybe_unused int > e1000e_pm_suspend(struct device *dev) > e1000e_pm_thaw(dev); > } else { > /* Introduce S0ix implementation */ > - if (hw->mac.type >= e1000_pch_cnp && > - !e1000e_check_me(hw->adapter->pdev->device)) > + if (hw->mac.type >= e1000_pch_cnp) > e1000e_s0ix_entry_flow(adapter); > } > > @@ -6991,8 +6951,7 @@ static __maybe_unused int > e1000e_pm_resume(struct device *dev) > int rc; > > /* Introduce S0ix implementation */ > - if (hw->mac.type >= e1000_pch_cnp && > - !e1000e_check_me(hw->adapter->pdev->device)) > + if (hw->mac.type >= e1000_pch_cnp) > e1000e_s0ix_exit_flow(adapter); > > rc = __e1000_resume(pdev); > -- > 2.25.1
> -----Original Message----- > From: Limonciello, Mario <Mario_Limonciello@Dell.com> > Sent: Tuesday, December 15, 2020 3:30 AM > To: Jeff Kirsher; Tony Nguyen; intel-wired-lan@lists.osuosl.org > Cc: linux-kernel@vger.kernel.org; Netdev; Alexander Duyck; Jakub Kicinski; > Sasha Netfin; Aaron Brown; Stefan Assmann; David Miller; > darcari@redhat.com; Shen, Yijun; Yuan, Perry; > anthony.wong@canonical.com; Hans de Goede; Limonciello, Mario > Subject: [PATCH v5 4/4] e1000e: Export S0ix flags to ethtool > > This flag can be used by an end user to disable S0ix flows on a buggy system > or by an OEM for development purposes. > > If you need this flag to be persisted across reboots, it's suggested to use a > udev rule to call adjust it until the kernel could have your configuration in a > disallow list. > > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com> Verified this series patches with Dell Systems. Tested-By: Yijun Shen <Yijun.shen@dell.com> > --- > drivers/net/ethernet/intel/e1000e/e1000.h | 1 + > drivers/net/ethernet/intel/e1000e/ethtool.c | 46 +++++++++++++++++++++ > drivers/net/ethernet/intel/e1000e/netdev.c | 9 ++-- > 3 files changed, 52 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h > b/drivers/net/ethernet/intel/e1000e/e1000.h > index ba7a0f8f6937..5b2143f4b1f8 100644 > --- a/drivers/net/ethernet/intel/e1000e/e1000.h > +++ b/drivers/net/ethernet/intel/e1000e/e1000.h > @@ -436,6 +436,7 @@ s32 e1000e_get_base_timinca(struct e1000_adapter > *adapter, u32 *timinca); > #define FLAG2_DFLT_CRC_STRIPPING BIT(12) > #define FLAG2_CHECK_RX_HWTSTAMP BIT(13) > #define FLAG2_CHECK_SYSTIM_OVERFLOW BIT(14) > +#define FLAG2_ENABLE_S0IX_FLOWS BIT(15) > > #define E1000_RX_DESC_PS(R, i) \ > (&(((union e1000_rx_desc_packet_split *)((R).desc))[i])) diff --git > a/drivers/net/ethernet/intel/e1000e/ethtool.c > b/drivers/net/ethernet/intel/e1000e/ethtool.c > index 03215b0aee4b..06442e6bef73 100644 > --- a/drivers/net/ethernet/intel/e1000e/ethtool.c > +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c > @@ -23,6 +23,13 @@ struct e1000_stats { > int stat_offset; > }; > > +static const char e1000e_priv_flags_strings[][ETH_GSTRING_LEN] = { > +#define E1000E_PRIV_FLAGS_S0IX_ENABLED BIT(0) > + "s0ix-enabled", > +}; > + > +#define E1000E_PRIV_FLAGS_STR_LEN > ARRAY_SIZE(e1000e_priv_flags_strings) > + > #define E1000_STAT(str, m) { \ > .stat_string = str, \ > .type = E1000_STATS, \ > @@ -1776,6 +1783,8 @@ static int e1000e_get_sset_count(struct net_device > __always_unused *netdev, > return E1000_TEST_LEN; > case ETH_SS_STATS: > return E1000_STATS_LEN; > + case ETH_SS_PRIV_FLAGS: > + return E1000E_PRIV_FLAGS_STR_LEN; > default: > return -EOPNOTSUPP; > } > @@ -2097,6 +2106,10 @@ static void e1000_get_strings(struct net_device > __always_unused *netdev, > p += ETH_GSTRING_LEN; > } > break; > + case ETH_SS_PRIV_FLAGS: > + memcpy(data, e1000e_priv_flags_strings, > + E1000E_PRIV_FLAGS_STR_LEN * ETH_GSTRING_LEN); > + break; > } > } > > @@ -2305,6 +2318,37 @@ static int e1000e_get_ts_info(struct net_device > *netdev, > return 0; > } > > +static u32 e1000e_get_priv_flags(struct net_device *netdev) { > + struct e1000_adapter *adapter = netdev_priv(netdev); > + u32 priv_flags = 0; > + > + if (adapter->flags2 & FLAG2_ENABLE_S0IX_FLOWS) > + priv_flags |= E1000E_PRIV_FLAGS_S0IX_ENABLED; > + > + return priv_flags; > +} > + > +static int e1000e_set_priv_flags(struct net_device *netdev, u32 > +priv_flags) { > + struct e1000_adapter *adapter = netdev_priv(netdev); > + unsigned int flags2 = adapter->flags2; > + > + flags2 &= ~FLAG2_ENABLE_S0IX_FLOWS; > + if (priv_flags & E1000E_PRIV_FLAGS_S0IX_ENABLED) { > + struct e1000_hw *hw = &adapter->hw; > + > + if (hw->mac.type < e1000_pch_cnp) > + return -EINVAL; > + flags2 |= FLAG2_ENABLE_S0IX_FLOWS; > + } > + > + if (flags2 != adapter->flags2) > + adapter->flags2 = flags2; > + > + return 0; > +} > + > static const struct ethtool_ops e1000_ethtool_ops = { > .supported_coalesce_params = ETHTOOL_COALESCE_RX_USECS, > .get_drvinfo = e1000_get_drvinfo, > @@ -2336,6 +2380,8 @@ static const struct ethtool_ops e1000_ethtool_ops > = { > .set_eee = e1000e_set_eee, > .get_link_ksettings = e1000_get_link_ksettings, > .set_link_ksettings = e1000_set_link_ksettings, > + .get_priv_flags = e1000e_get_priv_flags, > + .set_priv_flags = e1000e_set_priv_flags, > }; > > void e1000e_set_ethtool_ops(struct net_device *netdev) diff --git > a/drivers/net/ethernet/intel/e1000e/netdev.c > b/drivers/net/ethernet/intel/e1000e/netdev.c > index b9800ba2006c..e9b82c209c2d 100644 > --- a/drivers/net/ethernet/intel/e1000e/netdev.c > +++ b/drivers/net/ethernet/intel/e1000e/netdev.c > @@ -6923,7 +6923,6 @@ static __maybe_unused int > e1000e_pm_suspend(struct device *dev) > struct net_device *netdev = pci_get_drvdata(to_pci_dev(dev)); > struct e1000_adapter *adapter = netdev_priv(netdev); > struct pci_dev *pdev = to_pci_dev(dev); > - struct e1000_hw *hw = &adapter->hw; > int rc; > > e1000e_flush_lpic(pdev); > @@ -6935,7 +6934,7 @@ static __maybe_unused int > e1000e_pm_suspend(struct device *dev) > e1000e_pm_thaw(dev); > } else { > /* Introduce S0ix implementation */ > - if (hw->mac.type >= e1000_pch_cnp) > + if (adapter->flags2 & FLAG2_ENABLE_S0IX_FLOWS) > e1000e_s0ix_entry_flow(adapter); > } > > @@ -6947,11 +6946,10 @@ static __maybe_unused int > e1000e_pm_resume(struct device *dev) > struct net_device *netdev = pci_get_drvdata(to_pci_dev(dev)); > struct e1000_adapter *adapter = netdev_priv(netdev); > struct pci_dev *pdev = to_pci_dev(dev); > - struct e1000_hw *hw = &adapter->hw; > int rc; > > /* Introduce S0ix implementation */ > - if (hw->mac.type >= e1000_pch_cnp) > + if (adapter->flags2 & FLAG2_ENABLE_S0IX_FLOWS) > e1000e_s0ix_exit_flow(adapter); > > rc = __e1000_resume(pdev); > @@ -7615,6 +7613,9 @@ static int e1000_probe(struct pci_dev *pdev, const > struct pci_device_id *ent) > if (!(adapter->flags & FLAG_HAS_AMT)) > e1000e_get_hw_control(adapter); > > + if (hw->mac.type >= e1000_pch_cnp) > + adapter->flags2 |= FLAG2_ENABLE_S0IX_FLOWS; > + > strlcpy(netdev->name, "eth%d", sizeof(netdev->name)); > err = register_netdev(netdev); > if (err) > -- > 2.25.1