| Message ID | 20201214153450.874339-5-mario.limonciello@dell.com (mailing list archive) |
|---|---|
| State | Superseded |
| 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, 113 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 4:34 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> > --- > drivers/net/ethernet/intel/e1000e/e1000.h | 1 + > drivers/net/ethernet/intel/e1000e/ethtool.c | 40 +++++++++++++++++++++ > drivers/net/ethernet/intel/e1000e/netdev.c | 9 ++--- > 3 files changed, 46 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..eb683949ebfe 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,31 @@ 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) > + flags2 |= FLAG2_ENABLE_S0IX_FLOWS; > + if (flags2 != adapter->flags2) > + adapter->flags2 = flags2; This will allow ethtool to enable the s0ix code on hw which does not support this. I believe that this needs a if (hw->mac.type >= e1000_pch_cnp) Check to avoid this scenario. And probably return -EINVAL when a user tries to enable this on hw where it is not supported. Regards, Hans > + > + return 0; > +} > + > static const struct ethtool_ops e1000_ethtool_ops = { > .supported_coalesce_params = ETHTOOL_COALESCE_RX_USECS, > .get_drvinfo = e1000_get_drvinfo, > @@ -2336,6 +2374,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) >
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..eb683949ebfe 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,31 @@ 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) + 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 +2374,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 | 40 +++++++++++++++++++++ drivers/net/ethernet/intel/e1000e/netdev.c | 9 ++--- 3 files changed, 46 insertions(+), 4 deletions(-)