| Message ID | 20201214192935.895174-5-mario.limonciello@dell.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Improve s0ix flows for systems i219LM | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 119 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
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 > 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
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)
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> --- 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(-)