On Thu, 21 Aug 2025 20:12:29 +0300 Vitaly Lifshits <[email protected]> wrote:
> The K1 state reduces power consumption on ICH family network > controllers during idle periods, similarly to L1 state on PCI Express > NICs. Therefore, it is recommended and enabled by default. > However, on some systems it has been observed to have adverse side > effects, such as packet loss. It has been established through debug > that the problem may be due to firmware misconfiguration of specific > systems, interoperability with certain link partners, or marginal > electrical conditions of specific units. > > These problems typically cannot be fixed in the field, and generic > workarounds to resolve the side effects on all systems, while keeping > K1 enabled, were found infeasible. > Therefore, add the option for users to globally disable K1 idle state > on the adapter. > > Additionally, disable K1 by default for MTL and later platforms, due > to issues reported with the current configuration. Thanks for this! See below for one potential issue. > > Link: > https://lore.kernel.org/intel-wired-lan/camqyjg3lvqfgqmctxeapur_jq0oqh7ggdxruvtrx_6tth2m...@mail.gmail.com/ > Link: > https://lore.kernel.org/intel-wired-lan/[email protected]/ > Link: > https://lore.kernel.org/intel-wired-lan/Z_z9EjcKtwHCQcZR@mail-itl/ > Link: https://github.com/QubesOS/qubes-issues/issues/9896 Link: > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2115393 > > Signed-off-by: Vitaly Lifshits <[email protected]> > --- > static int e1000e_set_priv_flags(struct net_device *netdev, u32 > priv_flags) { > struct e1000_adapter *adapter = netdev_priv(netdev); > + struct e1000_hw *hw = &adapter->hw; > unsigned int flags2 = adapter->flags2; > > - flags2 &= ~FLAG2_ENABLE_S0IX_FLOWS; > + flags2 &= ~(FLAG2_ENABLE_S0IX_FLOWS | FLAG2_DISABLE_K1); > 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 (priv_flags & E1000E_PRIV_FLAGS_DISABLE_K1) { This is just checking if 'disable-k1' flag is set. But if I understand correctly the reset should be done if the 'disable-k1' flag state has changed? Currently this would cause unnecessary reset if 's0ix-enabled' was changed when 'disable-k1' was already set and not changed. Also it incorrect would not trigger reset if 'disable-k1' was set, and adjusted to be not set. Or did I miss something? > + if (hw->mac.type < e1000_ich8lan) > + return -EINVAL; > + flags2 |= FLAG2_DISABLE_K1; > + while (test_and_set_bit(__E1000_RESETTING, > + &adapter->state)) > + usleep_range(1000, 2000); > + > + /* reset the link to apply the changes */ > + if (netif_running(adapter->netdev)) { > + e1000e_down(adapter, true); > + e1000e_up(adapter); > + } else { > + e1000e_reset(adapter); > + } > + > + clear_bit(__E1000_RESETTING, &adapter->state); > + } > + > if (flags2 != adapter->flags2) > adapter->flags2 = flags2; >
