[Intel-wired-lan] [PATCH iwl-next v1 1/1] e1000e: Introduce private flag to disable K1

[Intel-wired-lan] [PATCH iwl-next v1 1/1] e1000e: Introduce private flag to disable K1

[Vitaly Lifshits]

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.

Link: https://lore.kernel.org/intel-wired-lan/CAMqyJG3LVqfgqMcTxeaPur_Jq0oQH7GgdxRuVtRX_6TTH2mX5Q@mail.gmail.com/
Link: https://lore.kernel.org/intel-wired-lan/20250626153544.1853d106@onyx.my.domain/
Link: https://lore.kernel.org/intel-wired-lan/Z_z9EjcKtwHCQcZR@mail-itl/

Signed-off-by: Vitaly Lifshits <vitaly.lifshits@intel.com>
---
 drivers/net/ethernet/intel/e1000e/e1000.h   |  1 +
 drivers/net/ethernet/intel/e1000e/ethtool.c | 29 ++++++++++++++--
 drivers/net/ethernet/intel/e1000e/hw.h      |  1 +
 drivers/net/ethernet/intel/e1000e/ich8lan.c | 38 +++++++++++----------
 drivers/net/ethernet/intel/e1000e/ich8lan.h |  2 ++
 drivers/net/ethernet/intel/e1000e/netdev.c  |  5 +++
 6 files changed, 55 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h
index 018e61aea787..aa08f397988e 100644
--- a/drivers/net/ethernet/intel/e1000e/e1000.h
+++ b/drivers/net/ethernet/intel/e1000e/e1000.h
@@ -461,6 +461,7 @@ s32 e1000e_get_base_timinca(struct e1000_adapter *adapter, u32 *timinca);
 #define FLAG2_CHECK_RX_HWTSTAMP           BIT(13)
 #define FLAG2_CHECK_SYSTIM_OVERFLOW       BIT(14)
 #define FLAG2_ENABLE_S0IX_FLOWS           BIT(15)
+#define FLAG2_DISABLE_K1		   BIT(16)
 
 #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 06482ad50508..30b8db3a7804 100644
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c
+++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
@@ -26,6 +26,8 @@ struct e1000_stats {
 static const char e1000e_priv_flags_strings[][ETH_GSTRING_LEN] = {
 #define E1000E_PRIV_FLAGS_S0IX_ENABLED	BIT(0)
 	"s0ix-enabled",
+#define E1000E_PRIV_FLAGS_DISABLE_K1	BIT(1)
+	"disable-k1",
 };
 
 #define E1000E_PRIV_FLAGS_STR_LEN ARRAY_SIZE(e1000e_priv_flags_strings)
@@ -2297,23 +2299,44 @@ static u32 e1000e_get_priv_flags(struct net_device *netdev)
 	if (adapter->flags2 & FLAG2_ENABLE_S0IX_FLOWS)
 		priv_flags |= E1000E_PRIV_FLAGS_S0IX_ENABLED;
 
+	if (adapter->flags2 & FLAG2_DISABLE_K1)
+		priv_flags |= E1000E_PRIV_FLAGS_DISABLE_K1;
+
 	return priv_flags;
 }
 
 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) {
+		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;
 
diff --git a/drivers/net/ethernet/intel/e1000e/hw.h b/drivers/net/ethernet/intel/e1000e/hw.h
index fc8ed38aa095..655872c99a9b 100644
--- a/drivers/net/ethernet/intel/e1000e/hw.h
+++ b/drivers/net/ethernet/intel/e1000e/hw.h
@@ -709,6 +709,7 @@ struct e1000_dev_spec_ich8lan {
 	struct e1000_shadow_ram shadow_ram[E1000_ICH8_SHADOW_RAM_WORDS];
 	bool nvm_k1_enabled;
 	bool eee_disable;
+	bool disable_k1;
 	u16 eee_lp_ability;
 	enum e1000_ulp_state ulp_state;
 };
diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index df4e7d781cb1..3112316014f8 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -286,21 +286,23 @@ static void e1000_toggle_lanphypc_pch_lpt(struct e1000_hw *hw)
 }
 
 /**
- * e1000_reconfigure_k1_exit_timeout - reconfigure K1 exit timeout to
- * align to MTP and later platform requirements.
+ * e1000_reconfigure_k1_params - reconfigure Kumeran K1 parameters.
  * @hw: pointer to the HW structure
  *
  * Context: PHY semaphore must be held by caller.
  * Return: 0 on success, negative on failure
  */
-static s32 e1000_reconfigure_k1_exit_timeout(struct e1000_hw *hw)
+s32 e1000_reconfigure_k1_params(struct e1000_hw *hw)
 {
 	u16 phy_timeout;
 	u32 fextnvm12;
 	s32 ret_val;
 
-	if (hw->mac.type < e1000_pch_mtp)
+	if (hw->mac.type < e1000_pch_mtp) {
+		if (hw->dev_spec.ich8lan.disable_k1)
+			return e1000_configure_k1_ich8lan(hw, false);
 		return 0;
+	}
 
 	/* Change Kumeran K1 power down state from P0s to P1 */
 	fextnvm12 = er32(FEXTNVM12);
@@ -310,6 +312,8 @@ static s32 e1000_reconfigure_k1_exit_timeout(struct e1000_hw *hw)
 
 	/* Wait for the interface the settle */
 	usleep_range(1000, 1100);
+	if (hw->dev_spec.ich8lan.disable_k1)
+		return e1000_configure_k1_ich8lan(hw, false);
 
 	/* Change K1 exit timeout */
 	ret_val = e1e_rphy_locked(hw, I217_PHY_TIMEOUTS_REG,
@@ -373,8 +377,8 @@ static s32 e1000_init_phy_workarounds_pchlan(struct e1000_hw *hw)
 		/* At this point the PHY might be inaccessible so don't
 		 * propagate the failure
 		 */
-		if (e1000_reconfigure_k1_exit_timeout(hw))
-			e_dbg("Failed to reconfigure K1 exit timeout\n");
+		if (e1000_reconfigure_k1_params(hw))
+			e_dbg("Failed to reconfigure K1 parameters\n");
 
 		fallthrough;
 	case e1000_pch_lpt:
@@ -473,10 +477,10 @@ static s32 e1000_init_phy_workarounds_pchlan(struct e1000_hw *hw)
 		if (hw->mac.type >= e1000_pch_mtp) {
 			ret_val = hw->phy.ops.acquire(hw);
 			if (ret_val) {
-				e_err("Failed to reconfigure K1 exit timeout\n");
+				e_err("Failed to reconfigure K1 parameters\n");
 				goto out;
 			}
-			ret_val = e1000_reconfigure_k1_exit_timeout(hw);
+			ret_val = e1000_reconfigure_k1_params(hw);
 			hw->phy.ops.release(hw);
 		}
 	}
@@ -4948,17 +4952,15 @@ static s32 e1000_init_hw_ich8lan(struct e1000_hw *hw)
 	u16 i;
 
 	e1000_initialize_hw_bits_ich8lan(hw);
-	if (hw->mac.type >= e1000_pch_mtp) {
-		ret_val = hw->phy.ops.acquire(hw);
-		if (ret_val)
-			return ret_val;
+	ret_val = hw->phy.ops.acquire(hw);
+	if (ret_val)
+		return ret_val;
 
-		ret_val = e1000_reconfigure_k1_exit_timeout(hw);
-		hw->phy.ops.release(hw);
-		if (ret_val) {
-			e_dbg("Error failed to reconfigure K1 exit timeout\n");
-			return ret_val;
-		}
+	ret_val = e1000_reconfigure_k1_params(hw);
+	hw->phy.ops.release(hw);
+	if (ret_val) {
+		e_dbg("Error failed to reconfigure K1 parameters\n");
+		return ret_val;
 	}
 
 	/* Initialize identification LED */
diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.h b/drivers/net/ethernet/intel/e1000e/ich8lan.h
index 5feb589a9b5f..97ceb6fa763b 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.h
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.h
@@ -310,4 +310,6 @@ s32 e1000_read_emi_reg_locked(struct e1000_hw *hw, u16 addr, u16 *data);
 s32 e1000_write_emi_reg_locked(struct e1000_hw *hw, u16 addr, u16 data);
 s32 e1000_set_eee_pchlan(struct e1000_hw *hw);
 s32 e1000_enable_ulp_lpt_lp(struct e1000_hw *hw, bool to_sx);
+s32 e1000_reconfigure_k1_params(struct e1000_hw *hw);
+
 #endif /* _E1000E_ICH8LAN_H_ */
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 201322dac233..44c774639c35 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -5267,6 +5267,11 @@ static void e1000_watchdog_task(struct work_struct *work)
 						  &adapter->link_duplex);
 			e1000_print_link_info(adapter);
 
+			if (adapter->flags2 & FLAG2_DISABLE_K1) {
+				adapter->hw.dev_spec.ich8lan.disable_k1 = true;
+				e1000_reconfigure_k1_params(&adapter->hw);
+			}
+
 			/* check if SmartSpeed worked */
 			e1000e_check_downshift(hw);
 			if (phy->speed_downgraded)
-- 
2.34.1

Re: [Intel-wired-lan] [PATCH iwl-next v1 1/1] e1000e: Introduce private flag to disable K1

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Vitaly Lifshits
> Sent: Tuesday, August 19, 2025 2:43 PM
> To: intel-wired-lan@osuosl.org
> Cc: Wu, Ricky <en-wei.wu@canonical.com>; timo.teras@iki.fi;
> marmarek@invisiblethingslab.com; Lifshits, Vitaly
> <vitaly.lifshits@intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next v1 1/1] e1000e: Introduce
> private flag to disable K1
> 
> 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.
> 
> Link: https://lore.kernel.org/intel-wired-
> lan/CAMqyJG3LVqfgqMcTxeaPur_Jq0oQH7GgdxRuVtRX_6TTH2mX5Q@mail.gmail.com
> /
> Link: https://lore.kernel.org/intel-wired-
> lan/20250626153544.1853d106@onyx.my.domain/
> Link: https://lore.kernel.org/intel-wired-lan/Z_z9EjcKtwHCQcZR@mail-
> itl/
> 
> Signed-off-by: Vitaly Lifshits <vitaly.lifshits@intel.com>
> ---
>  drivers/net/ethernet/intel/e1000e/e1000.h   |  1 +
>  drivers/net/ethernet/intel/e1000e/ethtool.c | 29 ++++++++++++++--
>  drivers/net/ethernet/intel/e1000e/hw.h      |  1 +
>  drivers/net/ethernet/intel/e1000e/ich8lan.c | 38 +++++++++++---------
> -
>  drivers/net/ethernet/intel/e1000e/ich8lan.h |  2 ++
>  drivers/net/ethernet/intel/e1000e/netdev.c  |  5 +++
>  6 files changed, 55 insertions(+), 21 deletions(-)
> 

...

> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -5267,6 +5267,11 @@ static void e1000_watchdog_task(struct
> work_struct *work)
>  						  &adapter->link_duplex);
>  			e1000_print_link_info(adapter);
> 
> +			if (adapter->flags2 & FLAG2_DISABLE_K1) {
> +				adapter->hw.dev_spec.ich8lan.disable_k1 =
> true;
> +				e1000_reconfigure_k1_params(&adapter->hw);
> +			}
> +
You changed the function header comment to require the PHY sem. 
Good - but. You already correctly hold the lock in other call sites (e.g., init paths).
Are you sure watchdog still consistent? 

>  			/* check if SmartSpeed worked */
>  			e1000e_check_downshift(hw);
>  			if (phy->speed_downgraded)
> --
> 2.34.1

Re: [Intel-wired-lan] [PATCH iwl-next v1 1/1] e1000e: Introduce private flag to disable K1

[Lifshits, Vitaly]

On 8/19/2025 5:19 PM, Loktionov, Aleksandr wrote:

> 
> ...
> 
>> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
>> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
>> @@ -5267,6 +5267,11 @@ static void e1000_watchdog_task(struct
>> work_struct *work)
>>   						  &adapter->link_duplex);
>>   			e1000_print_link_info(adapter);
>>
>> +			if (adapter->flags2 & FLAG2_DISABLE_K1) {
>> +				adapter->hw.dev_spec.ich8lan.disable_k1 =
>> true;
>> +				e1000_reconfigure_k1_params(&adapter->hw);
>> +			}
>> +
> You changed the function header comment to require the PHY sem.
> Good - but. You already correctly hold the lock in other call sites (e.g., init paths).
> Are you sure watchdog still consistent?

Hi Aleksandr,

Thank you for noticing it.
Yes, you are correct, a semaphore should be taken here, I will fix it in v2.

> 
>>   			/* check if SmartSpeed worked */
>>   			e1000e_check_downshift(hw);
>>   			if (phy->speed_downgraded)
>> --
>> 2.34.1
> 

Re: [Intel-wired-lan] [PATCH iwl-next v1 1/1] e1000e: Introduce private flag to disable K1

[Timo Teras]

On Tue, 19 Aug 2025 15:43:26 +0300
Vitaly Lifshits <vitaly.lifshits@intel.com> 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.

Thanks for adding this!

However, as a user, I find it inconvenient if the default setting
results in a subtly broken system on a device I just from a store.

Since this affects devices from multiple large vendors, would it be
possible to add some kind of quirk mechanism to automatically enable
this on known "bad" systems. Perhaps something based on the DMI or other
system specific information. Could something like this be implemented?

At least in my use case I have multiple e1000e using laptops on the
same link partner working, and only one broken device for which I
reported this issue. So at least on my experience the issue relates to
specific system primarily (perhaps also requiring a specific link
partner for the issue to show up).

Thanks!

> Link:
> https://lore.kernel.org/intel-wired-lan/CAMqyJG3LVqfgqMcTxeaPur_Jq0oQH7GgdxRuVtRX_6TTH2mX5Q@mail.gmail.com/
> Link:
> https://lore.kernel.org/intel-wired-lan/20250626153544.1853d106@onyx.my.domain/
> Link:
> https://lore.kernel.org/intel-wired-lan/Z_z9EjcKtwHCQcZR@mail-itl/
> 
> Signed-off-by: Vitaly Lifshits <vitaly.lifshits@intel.com>

Re: [Intel-wired-lan] [PATCH iwl-next v1 1/1] e1000e: Introduce private flag to disable K1

[Lifshits, Vitaly]

On 8/19/2025 8:10 PM, Timo Teras wrote:
> On Tue, 19 Aug 2025 15:43:26 +0300
> Vitaly Lifshits <vitaly.lifshits@intel.com> 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.
> 
> Thanks for adding this!
> 
> However, as a user, I find it inconvenient if the default setting
> results in a subtly broken system on a device I just from a store.
> 
> Since this affects devices from multiple large vendors, would it be
> possible to add some kind of quirk mechanism to automatically enable
> this on known "bad" systems. Perhaps something based on the DMI or other
> system specific information. Could something like this be implemented?
> 
> At least in my use case I have multiple e1000e using laptops on the
> same link partner working, and only one broken device for which I
> reported this issue. So at least on my experience the issue relates to
> specific system primarily (perhaps also requiring a specific link
> partner for the issue to show up).
> 
> Thanks!

Hi Timo,

Unfortunately, there is no visible configuration that allows the driver 
to reliably identify problematic systems.
If in the future we find such data, then we can improve the workaround 
and make it automatic.

At present, the user-controlled interface is the best we have.

Re: [Intel-wired-lan] [PATCH iwl-next v1 1/1] e1000e: Introduce private flag to disable K1

[Timo Teras]

On Wed, 20 Aug 2025 09:43:38 +0300
"Lifshits, Vitaly" <vitaly.lifshits@intel.com> wrote:

> On 8/19/2025 8:10 PM, Timo Teras wrote:
> > On Tue, 19 Aug 2025 15:43:26 +0300
> > Vitaly Lifshits <vitaly.lifshits@intel.com> 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.  
> > 
> > Thanks for adding this!
> > 
> > However, as a user, I find it inconvenient if the default setting
> > results in a subtly broken system on a device I just from a store.
> > 
> > Since this affects devices from multiple large vendors, would it be
> > possible to add some kind of quirk mechanism to automatically enable
> > this on known "bad" systems. Perhaps something based on the DMI or
> > other system specific information. Could something like this be
> > implemented?
> > 
> > At least in my use case I have multiple e1000e using laptops on the
> > same link partner working, and only one broken device for which I
> > reported this issue. So at least on my experience the issue relates
> > to specific system primarily (perhaps also requiring a specific link
> > partner for the issue to show up).
> 
> Unfortunately, there is no visible configuration that allows the
> driver to reliably identify problematic systems.
> If in the future we find such data, then we can improve the
> workaround and make it automatic.
> 
> At present, the user-controlled interface is the best we have.

Could you look at:
 - drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
 - drivers/soundwire/dmi-quirks.c

These use dmi_first_match() to match the DMI information of the system
and then apply quirks based on the matching per-system data.

Having similar mechanism in e1000e should be possible, right?

I am happy to provide the needed DMI information from my system if this
works out.

Timo

Re: [Intel-wired-lan] [PATCH iwl-next v1 1/1] e1000e: Introduce private flag to disable K1

[Lifshits, Vitaly]

On 8/20/2025 9:57 AM, Timo Teras wrote:

>>>
>>> Thanks for adding this!
>>>
>>> However, as a user, I find it inconvenient if the default setting
>>> results in a subtly broken system on a device I just from a store.
>>>
>>> Since this affects devices from multiple large vendors, would it be
>>> possible to add some kind of quirk mechanism to automatically enable
>>> this on known "bad" systems. Perhaps something based on the DMI or
>>> other system specific information. Could something like this be
>>> implemented?
>>>
>>> At least in my use case I have multiple e1000e using laptops on the
>>> same link partner working, and only one broken device for which I
>>> reported this issue. So at least on my experience the issue relates
>>> to specific system primarily (perhaps also requiring a specific link
>>> partner for the issue to show up).
>>
>> Unfortunately, there is no visible configuration that allows the
>> driver to reliably identify problematic systems.
>> If in the future we find such data, then we can improve the
>> workaround and make it automatic.
>>
>> At present, the user-controlled interface is the best we have.
> 
> Could you look at:
>   - drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
>   - drivers/soundwire/dmi-quirks.c
> 
> These use dmi_first_match() to match the DMI information of the system
> and then apply quirks based on the matching per-system data.
> 
> Having similar mechanism in e1000e should be possible, right?
> 
> I am happy to provide the needed DMI information from my system if this
> works out.
> 
> Timo

Hi Timo,

At the moment, we have no clear knowledge as to which systems may be 
affected, and what common characteristics they share.
We are working with vendors to try to narrow it down.
You are most welcome to share DMI information from your system. It can 
help with further investigation.

However, maintaining a DMI quirk for every single system for which an 
issue has been reported is not feasible. Trying to deduce a pattern from 
a handful of data points can lead to it being too broad or too narrow.
Furthermore, it may set up expectations of updating the quirk every time 
another user comes and says 'your default setting does not work for me'. 
This can quickly escalate out of control, and generally seems like the 
wrong approach.

Ultimately, vendors are best positioned to manage this, as they know 
which of their systems require this parameter. If a list were to be 
maintained, I’d suggest something similar to what Mario proposed for 
Dell platforms a few years ago for a different issue:
https://patchwork.ozlabs.org/project/netdev/patch/20201202161748.128938-4-mario.limonciello@dell.com/

For now, I prefer not to delay the current patch, acknowledging that 
finding a better solution may take time.

Re: [Intel-wired-lan] [PATCH iwl-next v1 1/1] e1000e: Introduce private flag to disable K1

[Timo Teras]

On Wed, 20 Aug 2025 15:38:12 +0300
"Lifshits, Vitaly" <vitaly.lifshits@intel.com> wrote:

> On 8/20/2025 9:57 AM, Timo Teras wrote:
> 
> >>>
> >>> Thanks for adding this!
> >>>
> >>> However, as a user, I find it inconvenient if the default setting
> >>> results in a subtly broken system on a device I just from a store.
> >>>
> >>> Since this affects devices from multiple large vendors, would it
> >>> be possible to add some kind of quirk mechanism to automatically
> >>> enable this on known "bad" systems. Perhaps something based on
> >>> the DMI or other system specific information. Could something
> >>> like this be implemented?
> >>>
> >>> At least in my use case I have multiple e1000e using laptops on
> >>> the same link partner working, and only one broken device for
> >>> which I reported this issue. So at least on my experience the
> >>> issue relates to specific system primarily (perhaps also
> >>> requiring a specific link partner for the issue to show up).  
> >>
> >> Unfortunately, there is no visible configuration that allows the
> >> driver to reliably identify problematic systems.
> >> If in the future we find such data, then we can improve the
> >> workaround and make it automatic.
> >>
> >> At present, the user-controlled interface is the best we have.  
> > 
> > Could you look at:
> >   - drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
> >   - drivers/soundwire/dmi-quirks.c
> > 
> > These use dmi_first_match() to match the DMI information of the
> > system and then apply quirks based on the matching per-system data.
> > 
> > Having similar mechanism in e1000e should be possible, right?
> > 
> > I am happy to provide the needed DMI information from my system if
> > this works out.
> > 
> > Timo  
> 
> Hi Timo,
> 
> At the moment, we have no clear knowledge as to which systems may be 
> affected, and what common characteristics they share.
> We are working with vendors to try to narrow it down.
> You are most welcome to share DMI information from your system. It
> can help with further investigation.
> 
> However, maintaining a DMI quirk for every single system for which an 
> issue has been reported is not feasible. Trying to deduce a pattern
> from a handful of data points can lead to it being too broad or too
> narrow. Furthermore, it may set up expectations of updating the quirk
> every time another user comes and says 'your default setting does not
> work for me'. This can quickly escalate out of control, and generally
> seems like the wrong approach.
> 
> Ultimately, vendors are best positioned to manage this, as they know 
> which of their systems require this parameter. If a list were to be 
> maintained, I’d suggest something similar to what Mario proposed for 
> Dell platforms a few years ago for a different issue:
> https://patchwork.ozlabs.org/project/netdev/patch/20201202161748.128938-4-mario.limonciello@dell.com/
> 
> For now, I prefer not to delay the current patch, acknowledging that 
> finding a better solution may take time.

Thank you for the continued investigation on the issue!

But I find this commit to not fix the reported regression. Nothing
changes without additional admin/user changes. Things used to work and
the added/modified K1 support thing is causing a regression.

Ubuntu has already reverted the offending patch due to complaints in
some flavors:
 https://patchwork.ozlabs.org/project/ubuntu-kernel/patch/20250805071341.41797-2-en-wei.wu@canonical.com/
 https://bugs.launchpad.net/bugs/2115393
 https://www.mail-archive.com/kernel-packages@lists.launchpad.net/msg551129.html

This is what I ended up also doing as it reliably fixes things on every
model I have, and has not caused any of them to have any other issues
(including packet loss).

At least mainstream Dell Pro and HP Zbook laptops have been reported to
be broken. See:
 https://lists.openwall.net/netdev/2025/07/01/57
 https://lists.osuosl.org/pipermail/intel-wired-lan/Week-of-Mon-20250623/048860.html

This seems to be the same issue:
 https://bugzilla.kernel.org/show_bug.cgi?id=218642

So some questions at this point:

If the added K1 configuration does not work and causes regressions,
could it be reverted and added back when a k1 configuration change that
can determine the affected systems is ready?

Could you explain the commit "e1000e: change k1 configuration on MTP
and later platforms" more? What does it fix? My understanding it is
"minor packet loss that may affect some machines"?

How many machines / what kind of scenario is affected? Is it fixing a
more serious issue than the regression it is causing?
The regression is completely defunct ethernet after unplugging cable.

My understanding is that the K1 change affects only power consumption.
Is this right? How much is the consumption difference? Would it rather
make sense to disable K1 by default on the potentially affected mac/phy
versions until a good common denominator is found?

On the other hand, do you think that asking to have a list of the few
currently known affected machines (until a simpler common denominator
can be found) too unreasonable? If the list seems to grow much, it
would be an indication that the default setting is wrong and changing
the defaults might be a good idea.

Timo

Re: [Intel-wired-lan] [PATCH iwl-next v1 1/1] e1000e: Introduce private flag to disable K1

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 6672 bytes --]

On Wed, Aug 20, 2025 at 04:26:14PM +0300, Timo Teras wrote:
> On Wed, 20 Aug 2025 15:38:12 +0300
> "Lifshits, Vitaly" <vitaly.lifshits@intel.com> wrote:
> 
> > On 8/20/2025 9:57 AM, Timo Teras wrote:
> > 
> > >>>
> > >>> Thanks for adding this!
> > >>>
> > >>> However, as a user, I find it inconvenient if the default setting
> > >>> results in a subtly broken system on a device I just from a store.
> > >>>
> > >>> Since this affects devices from multiple large vendors, would it
> > >>> be possible to add some kind of quirk mechanism to automatically
> > >>> enable this on known "bad" systems. Perhaps something based on
> > >>> the DMI or other system specific information. Could something
> > >>> like this be implemented?
> > >>>
> > >>> At least in my use case I have multiple e1000e using laptops on
> > >>> the same link partner working, and only one broken device for
> > >>> which I reported this issue. So at least on my experience the
> > >>> issue relates to specific system primarily (perhaps also
> > >>> requiring a specific link partner for the issue to show up).  
> > >>
> > >> Unfortunately, there is no visible configuration that allows the
> > >> driver to reliably identify problematic systems.
> > >> If in the future we find such data, then we can improve the
> > >> workaround and make it automatic.
> > >>
> > >> At present, the user-controlled interface is the best we have.  
> > > 
> > > Could you look at:
> > >   - drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
> > >   - drivers/soundwire/dmi-quirks.c
> > > 
> > > These use dmi_first_match() to match the DMI information of the
> > > system and then apply quirks based on the matching per-system data.
> > > 
> > > Having similar mechanism in e1000e should be possible, right?
> > > 
> > > I am happy to provide the needed DMI information from my system if
> > > this works out.
> > > 
> > > Timo  
> > 
> > Hi Timo,
> > 
> > At the moment, we have no clear knowledge as to which systems may be 
> > affected, and what common characteristics they share.
> > We are working with vendors to try to narrow it down.
> > You are most welcome to share DMI information from your system. It
> > can help with further investigation.
> > 
> > However, maintaining a DMI quirk for every single system for which an 
> > issue has been reported is not feasible. Trying to deduce a pattern
> > from a handful of data points can lead to it being too broad or too
> > narrow. Furthermore, it may set up expectations of updating the quirk
> > every time another user comes and says 'your default setting does not
> > work for me'. This can quickly escalate out of control, and generally
> > seems like the wrong approach.
> > 
> > Ultimately, vendors are best positioned to manage this, as they know 
> > which of their systems require this parameter. If a list were to be 
> > maintained, I’d suggest something similar to what Mario proposed for 
> > Dell platforms a few years ago for a different issue:
> > https://patchwork.ozlabs.org/project/netdev/patch/20201202161748.128938-4-mario.limonciello@dell.com/
> > 
> > For now, I prefer not to delay the current patch, acknowledging that 
> > finding a better solution may take time.
> 
> Thank you for the continued investigation on the issue!
> 
> But I find this commit to not fix the reported regression. Nothing
> changes without additional admin/user changes. Things used to work and
> the added/modified K1 support thing is causing a regression.
> 
> Ubuntu has already reverted the offending patch due to complaints in
> some flavors:
>  https://patchwork.ozlabs.org/project/ubuntu-kernel/patch/20250805071341.41797-2-en-wei.wu@canonical.com/
>  https://bugs.launchpad.net/bugs/2115393
>  https://www.mail-archive.com/kernel-packages@lists.launchpad.net/msg551129.html

Qubes OS also has this change reverted in default kernel, for the same
reason:
https://github.com/QubesOS/qubes-issues/issues/9896
https://github.com/QubesOS/qubes-linux-kernel/commit/4fb8c96dd7bd73dda00a89d026b6ebefff939a67

We've got several reports of the regression caused by the "e1000e:
change k1 configuration on MTP and later platforms", and _none_
complains after reverting it. And we do have many users on MTL or newer.

> This is what I ended up also doing as it reliably fixes things on every
> model I have, and has not caused any of them to have any other issues
> (including packet loss).
> 
> At least mainstream Dell Pro and HP Zbook laptops have been reported to
> be broken. See:
>  https://lists.openwall.net/netdev/2025/07/01/57
>  https://lists.osuosl.org/pipermail/intel-wired-lan/Week-of-Mon-20250623/048860.html
> 
> This seems to be the same issue:
>  https://bugzilla.kernel.org/show_bug.cgi?id=218642
> 
> So some questions at this point:
> 
> If the added K1 configuration does not work and causes regressions,
> could it be reverted and added back when a k1 configuration change that
> can determine the affected systems is ready?
> 
> Could you explain the commit "e1000e: change k1 configuration on MTP
> and later platforms" more? What does it fix? My understanding it is
> "minor packet loss that may affect some machines"?
> 
> How many machines / what kind of scenario is affected? Is it fixing a
> more serious issue than the regression it is causing?
> The regression is completely defunct ethernet after unplugging cable.
> 
> My understanding is that the K1 change affects only power consumption.
> Is this right? How much is the consumption difference? Would it rather
> make sense to disable K1 by default on the potentially affected mac/phy
> versions until a good common denominator is found?

Given the severity of the regression, I'd suggest something like the
above. Have functional configuration by default, and have an option to
potentially improve power consumption. Once criteria when it can be
safely enabled by default are figured out, then it's fine to apply the
improvement by default. But I'd rather have users with functional
ethernet, than slight power (or performance?) improvement at the cost of
completely breaking it for others...

> On the other hand, do you think that asking to have a list of the few
> currently known affected machines (until a simpler common denominator
> can be found) too unreasonable? If the list seems to grow much, it
> would be an indication that the default setting is wrong and changing
> the defaults might be a good idea.

Let me know what info you'd need for such list.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Intel-wired-lan] [PATCH iwl-next v1 1/1] e1000e: Introduce private flag to disable K1

[Lifshits, Vitaly]

On 8/20/2025 4:51 PM, Marek Marczykowski-Górecki wrote:
> On Wed, Aug 20, 2025 at 04:26:14PM +0300, Timo Teras wrote:
>> On Wed, 20 Aug 2025 15:38:12 +0300
>> "Lifshits, Vitaly" <vitaly.lifshits@intel.com> wrote:
>>
>>> On 8/20/2025 9:57 AM, Timo Teras wrote:
>>>
>>>>>>
>>>>>> Thanks for adding this!
>>>>>>
>>>>>> However, as a user, I find it inconvenient if the default setting
>>>>>> results in a subtly broken system on a device I just from a store.
>>>>>>
>>>>>> Since this affects devices from multiple large vendors, would it
>>>>>> be possible to add some kind of quirk mechanism to automatically
>>>>>> enable this on known "bad" systems. Perhaps something based on
>>>>>> the DMI or other system specific information. Could something
>>>>>> like this be implemented?
>>>>>>
>>>>>> At least in my use case I have multiple e1000e using laptops on
>>>>>> the same link partner working, and only one broken device for
>>>>>> which I reported this issue. So at least on my experience the
>>>>>> issue relates to specific system primarily (perhaps also
>>>>>> requiring a specific link partner for the issue to show up).
>>>>>
>>>>> Unfortunately, there is no visible configuration that allows the
>>>>> driver to reliably identify problematic systems.
>>>>> If in the future we find such data, then we can improve the
>>>>> workaround and make it automatic.
>>>>>
>>>>> At present, the user-controlled interface is the best we have.
>>>>
>>>> Could you look at:
>>>>    - drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
>>>>    - drivers/soundwire/dmi-quirks.c
>>>>
>>>> These use dmi_first_match() to match the DMI information of the
>>>> system and then apply quirks based on the matching per-system data.
>>>>
>>>> Having similar mechanism in e1000e should be possible, right?
>>>>
>>>> I am happy to provide the needed DMI information from my system if
>>>> this works out.
>>>>
>>>> Timo
>>>
>>> Hi Timo,
>>>
>>> At the moment, we have no clear knowledge as to which systems may be
>>> affected, and what common characteristics they share.
>>> We are working with vendors to try to narrow it down.
>>> You are most welcome to share DMI information from your system. It
>>> can help with further investigation.
>>>
>>> However, maintaining a DMI quirk for every single system for which an
>>> issue has been reported is not feasible. Trying to deduce a pattern
>>> from a handful of data points can lead to it being too broad or too
>>> narrow. Furthermore, it may set up expectations of updating the quirk
>>> every time another user comes and says 'your default setting does not
>>> work for me'. This can quickly escalate out of control, and generally
>>> seems like the wrong approach.
>>>
>>> Ultimately, vendors are best positioned to manage this, as they know
>>> which of their systems require this parameter. If a list were to be
>>> maintained, I’d suggest something similar to what Mario proposed for
>>> Dell platforms a few years ago for a different issue:
>>> https://patchwork.ozlabs.org/project/netdev/patch/20201202161748.128938-4-mario.limonciello@dell.com/
>>>
>>> For now, I prefer not to delay the current patch, acknowledging that
>>> finding a better solution may take time.
>>
>> Thank you for the continued investigation on the issue!
>>
>> But I find this commit to not fix the reported regression. Nothing
>> changes without additional admin/user changes. Things used to work and
>> the added/modified K1 support thing is causing a regression.
>>
>> Ubuntu has already reverted the offending patch due to complaints in
>> some flavors:
>>   https://patchwork.ozlabs.org/project/ubuntu-kernel/patch/20250805071341.41797-2-en-wei.wu@canonical.com/
>>   https://bugs.launchpad.net/bugs/2115393
>>   https://www.mail-archive.com/kernel-packages@lists.launchpad.net/msg551129.html
> 
> Qubes OS also has this change reverted in default kernel, for the same
> reason:
> https://github.com/QubesOS/qubes-issues/issues/9896
> https://github.com/QubesOS/qubes-linux-kernel/commit/4fb8c96dd7bd73dda00a89d026b6ebefff939a67
> 
> We've got several reports of the regression caused by the "e1000e:
> change k1 configuration on MTP and later platforms", and _none_
> complains after reverting it. And we do have many users on MTL or newer.
> 
>> This is what I ended up also doing as it reliably fixes things on every
>> model I have, and has not caused any of them to have any other issues
>> (including packet loss).
>>
>> At least mainstream Dell Pro and HP Zbook laptops have been reported to
>> be broken. See:
>>   https://lists.openwall.net/netdev/2025/07/01/57
>>   https://lists.osuosl.org/pipermail/intel-wired-lan/Week-of-Mon-20250623/048860.html
>>
>> This seems to be the same issue:
>>   https://bugzilla.kernel.org/show_bug.cgi?id=218642
>>
>> So some questions at this point:
>>
>> If the added K1 configuration does not work and causes regressions,
>> could it be reverted and added back when a k1 configuration change that
>> can determine the affected systems is ready?
>>
>> Could you explain the commit "e1000e: change k1 configuration on MTP
>> and later platforms" more? What does it fix? My understanding it is
>> "minor packet loss that may affect some machines"?
>>
>> How many machines / what kind of scenario is affected? Is it fixing a
>> more serious issue than the regression it is causing?
>> The regression is completely defunct ethernet after unplugging cable.
>>
>> My understanding is that the K1 change affects only power consumption.
>> Is this right? How much is the consumption difference? Would it rather
>> make sense to disable K1 by default on the potentially affected mac/phy
>> versions until a good common denominator is found?
> 
> Given the severity of the regression, I'd suggest something like the
> above. Have functional configuration by default, and have an option to
> potentially improve power consumption. Once criteria when it can be
> safely enabled by default are figured out, then it's fine to apply the
> improvement by default. But I'd rather have users with functional
> ethernet, than slight power (or performance?) improvement at the cost of
> completely breaking it for others...
> 
>> On the other hand, do you think that asking to have a list of the few
>> currently known affected machines (until a simpler common denominator
>> can be found) too unreasonable? If the list seems to grow much, it
>> would be an indication that the default setting is wrong and changing
>> the defaults might be a good idea.
> 
> Let me know what info you'd need for such list.
> 


Hi,

Thanks for your input — I think your points are valid.

As I mentioned earlier, fully reverting the patch "e1000e: change K1 
configuration on MTP and later platforms" is not advisable.
In addition to a possibility of packet corruption, it also increases the 
risk of PHY access failures, which can lead to the driver sporadically 
failing probe() or resume() flows.

However, surveying the breadth of the systems suffering regressions with 
this patch, it seems that indeed a safer approach is to have the 
"disable K1" flag default as TRUE for the MTP and later.

This approach ensures:
No impact on legacy platforms.
Affected platforms will be protected both from the PHY access failures 
resolved by the previous patch as well as the packet loss issues 
introduced by it, only at the cost of a somewhat increased power 
consumption.

I will send a V2 with this change.