Netdev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] e1000e: bump up timeout to wait when ME un-configure ULP mode
@ 2020-03-23 19:16 Aaron Ma
  2020-03-25  4:17 ` Kai-Heng Feng
  2020-04-02 12:31 ` Hans de Goede
  0 siblings, 2 replies; 16+ messages in thread
From: Aaron Ma @ 2020-03-23 19:16 UTC (permalink / raw)
  To: jeffrey.t.kirsher, aaron.ma, davem, intel-wired-lan, netdev,
	linux-kernel, sasha.neftin

ME takes 2+ seconds to un-configure ULP mode done after resume
from s2idle on some ThinkPad laptops.
Without enough wait, reset and re-init will fail with error.

Fixes: f15bb6dde738cc8fa0 ("e1000e: Add support for S0ix")
BugLink: https://bugs.launchpad.net/bugs/1865570
Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
---
 drivers/net/ethernet/intel/e1000e/ich8lan.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index b4135c50e905..147b15a2f8b3 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -1240,9 +1240,9 @@ 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.5sec for ME to clear ULP_CFG_DONE. */
 		while (er32(FWSM) & E1000_FWSM_ULP_CFG_DONE) {
-			if (i++ == 30) {
+			if (i++ == 250) {
 				ret_val = -E1000_ERR_PHY;
 				goto out;
 			}
-- 
2.17.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] e1000e: bump up timeout to wait when ME un-configure ULP mode
  2020-03-23 19:16 [PATCH] e1000e: bump up timeout to wait when ME un-configure ULP mode Aaron Ma
@ 2020-03-25  4:17 ` Kai-Heng Feng
  2020-03-25  6:36   ` Neftin, Sasha
  2020-04-02 12:31 ` Hans de Goede
  1 sibling, 1 reply; 16+ messages in thread
From: Kai-Heng Feng @ 2020-03-25  4:17 UTC (permalink / raw)
  To: Aaron Ma
  Cc: Kirsher, Jeffrey T, David Miller,
	moderated list:INTEL ETHERNET DRIVERS,
	open list:NETWORKING DRIVERS, open list, Neftin, Sasha

Hi Aaron,

> On Mar 24, 2020, at 03:16, Aaron Ma <aaron.ma@canonical.com> wrote:
> 
> ME takes 2+ seconds to un-configure ULP mode done after resume
> from s2idle on some ThinkPad laptops.
> Without enough wait, reset and re-init will fail with error.

Thanks, this patch solves the issue. We can drop the DMI quirk in favor of this patch.

> 
> Fixes: f15bb6dde738cc8fa0 ("e1000e: Add support for S0ix")
> BugLink: https://bugs.launchpad.net/bugs/1865570
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> ---
> drivers/net/ethernet/intel/e1000e/ich8lan.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c
> index b4135c50e905..147b15a2f8b3 100644
> --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
> +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
> @@ -1240,9 +1240,9 @@ 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.5sec for ME to clear ULP_CFG_DONE. */
> 		while (er32(FWSM) & E1000_FWSM_ULP_CFG_DONE) {
> -			if (i++ == 30) {
> +			if (i++ == 250) {
> 				ret_val = -E1000_ERR_PHY;
> 				goto out;
> 			}

The return value was not caught by the caller, so the error ends up unnoticed.
Maybe let the caller check the return value of e1000_disable_ulp_lpt_lp()?

Kai-Heng

> -- 
> 2.17.1
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] e1000e: bump up timeout to wait when ME un-configure ULP mode
  2020-03-25  4:17 ` Kai-Heng Feng
@ 2020-03-25  6:36   ` Neftin, Sasha
  2020-03-25  6:39     ` Kai-Heng Feng
  2020-03-25  6:43     ` Aaron Ma
  0 siblings, 2 replies; 16+ messages in thread
From: Neftin, Sasha @ 2020-03-25  6:36 UTC (permalink / raw)
  To: Kai-Heng Feng, Aaron Ma
  Cc: Kirsher, Jeffrey T, David Miller,
	moderated list:INTEL ETHERNET DRIVERS,
	open list:NETWORKING DRIVERS, open list, Lifshits, Vitaly,
	rex.tsai

On 3/25/2020 06:17, Kai-Heng Feng wrote:
> Hi Aaron,
> 
>> On Mar 24, 2020, at 03:16, Aaron Ma <aaron.ma@canonical.com> wrote:
>>
>> ME takes 2+ seconds to un-configure ULP mode done after resume
>> from s2idle on some ThinkPad laptops.
>> Without enough wait, reset and re-init will fail with error.
> 
> Thanks, this patch solves the issue. We can drop the DMI quirk in favor of this patch.
> 
>>
>> Fixes: f15bb6dde738cc8fa0 ("e1000e: Add support for S0ix")
>> BugLink: https://bugs.launchpad.net/bugs/1865570
>> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
>> ---
>> drivers/net/ethernet/intel/e1000e/ich8lan.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>> index b4135c50e905..147b15a2f8b3 100644
>> --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
>> +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>> @@ -1240,9 +1240,9 @@ 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.5sec for ME to clear ULP_CFG_DONE. */
>> 		while (er32(FWSM) & E1000_FWSM_ULP_CFG_DONE) {
>> -			if (i++ == 30) {
>> +			if (i++ == 250) {
>> 				ret_val = -E1000_ERR_PHY;
>> 				goto out;
>> 			}
> 
> The return value was not caught by the caller, so the error ends up unnoticed.
> Maybe let the caller check the return value of e1000_disable_ulp_lpt_lp()?
> 
> Kai-Heng
Hello Kai-Heng and Aaron,
I a bit confused. In our previous conversation you told ME not running. 
let me shimming in. Increasing delay won't be solve the problem and just 
mask it. We need to understand why ME take too much time. What is 
problem with this specific system?
So, basically no ME system should works for you.

Meanwhile I prefer keep DMI quirk.
Thanks,
Sasha
> 
>> -- 
>> 2.17.1
>>
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] e1000e: bump up timeout to wait when ME un-configure ULP mode
  2020-03-25  6:36   ` Neftin, Sasha
@ 2020-03-25  6:39     ` Kai-Heng Feng
  2020-03-25  6:42       ` Tsai, Rex
  2020-03-25  6:43     ` Aaron Ma
  1 sibling, 1 reply; 16+ messages in thread
From: Kai-Heng Feng @ 2020-03-25  6:39 UTC (permalink / raw)
  To: Neftin, Sasha
  Cc: Aaron Ma, Kirsher, Jeffrey T, David Miller,
	moderated list:INTEL ETHERNET DRIVERS,
	open list:NETWORKING DRIVERS, open list, Lifshits, Vitaly,
	rex.tsai

Hi Sasha,

> On Mar 25, 2020, at 14:36, Neftin, Sasha <sasha.neftin@intel.com> wrote:
> 
> On 3/25/2020 06:17, Kai-Heng Feng wrote:
>> Hi Aaron,
>>> On Mar 24, 2020, at 03:16, Aaron Ma <aaron.ma@canonical.com> wrote:
>>> 
>>> ME takes 2+ seconds to un-configure ULP mode done after resume
>>> from s2idle on some ThinkPad laptops.
>>> Without enough wait, reset and re-init will fail with error.
>> Thanks, this patch solves the issue. We can drop the DMI quirk in favor of this patch.
>>> 
>>> Fixes: f15bb6dde738cc8fa0 ("e1000e: Add support for S0ix")
>>> BugLink: https://bugs.launchpad.net/bugs/1865570
>>> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
>>> ---
>>> drivers/net/ethernet/intel/e1000e/ich8lan.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>> index b4135c50e905..147b15a2f8b3 100644
>>> --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>> +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>> @@ -1240,9 +1240,9 @@ 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.5sec for ME to clear ULP_CFG_DONE. */
>>> 		while (er32(FWSM) & E1000_FWSM_ULP_CFG_DONE) {
>>> -			if (i++ == 30) {
>>> +			if (i++ == 250) {
>>> 				ret_val = -E1000_ERR_PHY;
>>> 				goto out;
>>> 			}
>> The return value was not caught by the caller, so the error ends up unnoticed.
>> Maybe let the caller check the return value of e1000_disable_ulp_lpt_lp()?
>> Kai-Heng
> Hello Kai-Heng and Aaron,
> I a bit confused. In our previous conversation you told ME not running.

Yes I can confirm Intel AMT is disabled BIOS menu. I think Intel AMT is ME in this context?

How do I check if it's really disabled under Linux?

Kai-Heng

> let me shimming in. Increasing delay won't be solve the problem and just mask it. We need to understand why ME take too much time. What is problem with this specific system?
> So, basically no ME system should works for you.
> 
> Meanwhile I prefer keep DMI quirk.
> Thanks,
> Sasha
>>> -- 
>>> 2.17.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH] e1000e: bump up timeout to wait when ME un-configure ULP mode
  2020-03-25  6:39     ` Kai-Heng Feng
@ 2020-03-25  6:42       ` Tsai, Rex
  2020-03-25  6:49         ` Aaron Ma
  0 siblings, 1 reply; 16+ messages in thread
From: Tsai, Rex @ 2020-03-25  6:42 UTC (permalink / raw)
  To: Kai-Heng Feng, Neftin, Sasha
  Cc: Aaron Ma, Kirsher, Jeffrey T, David Miller,
	moderated list:INTEL ETHERNET DRIVERS,
	open list:NETWORKING DRIVERS, open list, Lifshits, Vitaly

Hello Kai-Heng,
If you are using vPro system, ME LAN driver is always alive and you have no way to disable it until you build new BIOS. Is this also for Lenovo system?

Rex Tsai | Intel Client LAN Engineer | +1 (503) 264-0517

-----Original Message-----
From: Kai-Heng Feng <kai.heng.feng@canonical.com> 
Sent: Tuesday, March 24, 2020 11:40 PM
To: Neftin, Sasha <sasha.neftin@intel.com>
Cc: Aaron Ma <aaron.ma@canonical.com>; Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; David Miller <davem@davemloft.net>; moderated list:INTEL ETHERNET DRIVERS <intel-wired-lan@lists.osuosl.org>; open list:NETWORKING DRIVERS <netdev@vger.kernel.org>; open list <linux-kernel@vger.kernel.org>; Lifshits, Vitaly <vitaly.lifshits@intel.com>; Tsai, Rex <rex.tsai@intel.com>
Subject: Re: [PATCH] e1000e: bump up timeout to wait when ME un-configure ULP mode

Hi Sasha,

> On Mar 25, 2020, at 14:36, Neftin, Sasha <sasha.neftin@intel.com> wrote:
> 
> On 3/25/2020 06:17, Kai-Heng Feng wrote:
>> Hi Aaron,
>>> On Mar 24, 2020, at 03:16, Aaron Ma <aaron.ma@canonical.com> wrote:
>>> 
>>> ME takes 2+ seconds to un-configure ULP mode done after resume from 
>>> s2idle on some ThinkPad laptops.
>>> Without enough wait, reset and re-init will fail with error.
>> Thanks, this patch solves the issue. We can drop the DMI quirk in favor of this patch.
>>> 
>>> Fixes: f15bb6dde738cc8fa0 ("e1000e: Add support for S0ix")
>>> BugLink: https://bugs.launchpad.net/bugs/1865570
>>> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
>>> ---
>>> drivers/net/ethernet/intel/e1000e/ich8lan.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c 
>>> b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>> index b4135c50e905..147b15a2f8b3 100644
>>> --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>> +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>> @@ -1240,9 +1240,9 @@ 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.5sec for ME to clear ULP_CFG_DONE. */
>>> 		while (er32(FWSM) & E1000_FWSM_ULP_CFG_DONE) {
>>> -			if (i++ == 30) {
>>> +			if (i++ == 250) {
>>> 				ret_val = -E1000_ERR_PHY;
>>> 				goto out;
>>> 			}
>> The return value was not caught by the caller, so the error ends up unnoticed.
>> Maybe let the caller check the return value of e1000_disable_ulp_lpt_lp()?
>> Kai-Heng
> Hello Kai-Heng and Aaron,
> I a bit confused. In our previous conversation you told ME not running.

Yes I can confirm Intel AMT is disabled BIOS menu. I think Intel AMT is ME in this context?

How do I check if it's really disabled under Linux?

Kai-Heng

> let me shimming in. Increasing delay won't be solve the problem and just mask it. We need to understand why ME take too much time. What is problem with this specific system?
> So, basically no ME system should works for you.
> 
> Meanwhile I prefer keep DMI quirk.
> Thanks,
> Sasha
>>> --
>>> 2.17.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] e1000e: bump up timeout to wait when ME un-configure ULP mode
  2020-03-25  6:36   ` Neftin, Sasha
  2020-03-25  6:39     ` Kai-Heng Feng
@ 2020-03-25  6:43     ` Aaron Ma
  2020-03-25 13:58       ` Neftin, Sasha
  1 sibling, 1 reply; 16+ messages in thread
From: Aaron Ma @ 2020-03-25  6:43 UTC (permalink / raw)
  To: Neftin, Sasha, Kai-Heng Feng
  Cc: Kirsher, Jeffrey T, David Miller,
	moderated list:INTEL ETHERNET DRIVERS,
	open list:NETWORKING DRIVERS, open list, Lifshits, Vitaly,
	rex.tsai



On 3/25/20 2:36 PM, Neftin, Sasha wrote:
> On 3/25/2020 06:17, Kai-Heng Feng wrote:
>> Hi Aaron,
>>
>>> On Mar 24, 2020, at 03:16, Aaron Ma <aaron.ma@canonical.com> wrote:
>>>
>>> ME takes 2+ seconds to un-configure ULP mode done after resume
>>> from s2idle on some ThinkPad laptops.
>>> Without enough wait, reset and re-init will fail with error.
>>
>> Thanks, this patch solves the issue. We can drop the DMI quirk in
>> favor of this patch.
>>
>>>
>>> Fixes: f15bb6dde738cc8fa0 ("e1000e: Add support for S0ix")
>>> BugLink: https://bugs.launchpad.net/bugs/1865570
>>> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
>>> ---
>>> drivers/net/ethernet/intel/e1000e/ich8lan.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>> b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>> index b4135c50e905..147b15a2f8b3 100644
>>> --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>> +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>> @@ -1240,9 +1240,9 @@ 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.5sec for ME to clear ULP_CFG_DONE. */
>>>         while (er32(FWSM) & E1000_FWSM_ULP_CFG_DONE) {
>>> -            if (i++ == 30) {
>>> +            if (i++ == 250) {
>>>                 ret_val = -E1000_ERR_PHY;
>>>                 goto out;
>>>             }
>>
>> The return value was not caught by the caller, so the error ends up
>> unnoticed.
>> Maybe let the caller check the return value of
>> e1000_disable_ulp_lpt_lp()?
>>
>> Kai-Heng
> Hello Kai-Heng and Aaron,
> I a bit confused. In our previous conversation you told ME not running.
> let me shimming in. Increasing delay won't be solve the problem and just
> mask it. We need to understand why ME take too much time. What is
> problem with this specific system?
> So, basically no ME system should works for you.

Some laptops ME work that's why only reproduce issue on some laptops.
In this issue i219 is controlled by ME.

Quirk is only for 1 model type. But issue is reproduced by more models.
So it won't work.

Regard,
Aaron

> 
> Meanwhile I prefer keep DMI quirk.
> Thanks,
> Sasha
>>
>>> -- 
>>> 2.17.1
>>>
>>
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] e1000e: bump up timeout to wait when ME un-configure ULP mode
  2020-03-25  6:42       ` Tsai, Rex
@ 2020-03-25  6:49         ` Aaron Ma
  0 siblings, 0 replies; 16+ messages in thread
From: Aaron Ma @ 2020-03-25  6:49 UTC (permalink / raw)
  To: Tsai, Rex, Kai-Heng Feng, Neftin, Sasha
  Cc: Kirsher, Jeffrey T, David Miller,
	moderated list:INTEL ETHERNET DRIVERS,
	open list:NETWORKING DRIVERS, open list, Lifshits, Vitaly

On 3/25/20 2:42 PM, Tsai, Rex wrote:
> Hello Kai-Heng,
> If you are using vPro system, ME LAN driver is always alive and you have no way to disable it until you build new BIOS. Is this also for Lenovo system?

Right, some new models from Lenovo.

Regards,
Aaron

> 
> Rex Tsai | Intel Client LAN Engineer | +1 (503) 264-0517
> 
> -----Original Message-----
> From: Kai-Heng Feng <kai.heng.feng@canonical.com> 
> Sent: Tuesday, March 24, 2020 11:40 PM
> To: Neftin, Sasha <sasha.neftin@intel.com>
> Cc: Aaron Ma <aaron.ma@canonical.com>; Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; David Miller <davem@davemloft.net>; moderated list:INTEL ETHERNET DRIVERS <intel-wired-lan@lists.osuosl.org>; open list:NETWORKING DRIVERS <netdev@vger.kernel.org>; open list <linux-kernel@vger.kernel.org>; Lifshits, Vitaly <vitaly.lifshits@intel.com>; Tsai, Rex <rex.tsai@intel.com>
> Subject: Re: [PATCH] e1000e: bump up timeout to wait when ME un-configure ULP mode
> 
> Hi Sasha,
> 
>> On Mar 25, 2020, at 14:36, Neftin, Sasha <sasha.neftin@intel.com> wrote:
>>
>> On 3/25/2020 06:17, Kai-Heng Feng wrote:
>>> Hi Aaron,
>>>> On Mar 24, 2020, at 03:16, Aaron Ma <aaron.ma@canonical.com> wrote:
>>>>
>>>> ME takes 2+ seconds to un-configure ULP mode done after resume from 
>>>> s2idle on some ThinkPad laptops.
>>>> Without enough wait, reset and re-init will fail with error.
>>> Thanks, this patch solves the issue. We can drop the DMI quirk in favor of this patch.
>>>>
>>>> Fixes: f15bb6dde738cc8fa0 ("e1000e: Add support for S0ix")
>>>> BugLink: https://bugs.launchpad.net/bugs/1865570
>>>> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
>>>> ---
>>>> drivers/net/ethernet/intel/e1000e/ich8lan.c | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c 
>>>> b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>>> index b4135c50e905..147b15a2f8b3 100644
>>>> --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>>> +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>>> @@ -1240,9 +1240,9 @@ 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.5sec for ME to clear ULP_CFG_DONE. */
>>>> 		while (er32(FWSM) & E1000_FWSM_ULP_CFG_DONE) {
>>>> -			if (i++ == 30) {
>>>> +			if (i++ == 250) {
>>>> 				ret_val = -E1000_ERR_PHY;
>>>> 				goto out;
>>>> 			}
>>> The return value was not caught by the caller, so the error ends up unnoticed.
>>> Maybe let the caller check the return value of e1000_disable_ulp_lpt_lp()?
>>> Kai-Heng
>> Hello Kai-Heng and Aaron,
>> I a bit confused. In our previous conversation you told ME not running.
> 
> Yes I can confirm Intel AMT is disabled BIOS menu. I think Intel AMT is ME in this context?
> 
> How do I check if it's really disabled under Linux?
> 
> Kai-Heng
> 
>> let me shimming in. Increasing delay won't be solve the problem and just mask it. We need to understand why ME take too much time. What is problem with this specific system?
>> So, basically no ME system should works for you.
>>
>> Meanwhile I prefer keep DMI quirk.
>> Thanks,
>> Sasha
>>>> --
>>>> 2.17.1
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] e1000e: bump up timeout to wait when ME un-configure ULP mode
  2020-03-25  6:43     ` Aaron Ma
@ 2020-03-25 13:58       ` Neftin, Sasha
  2020-03-25 14:07         ` Aaron Ma
  2020-03-25 15:49         ` [Intel-wired-lan] " Paul Menzel
  0 siblings, 2 replies; 16+ messages in thread
From: Neftin, Sasha @ 2020-03-25 13:58 UTC (permalink / raw)
  To: Aaron Ma, Kai-Heng Feng
  Cc: Kirsher, Jeffrey T, David Miller,
	moderated list:INTEL ETHERNET DRIVERS,
	open list:NETWORKING DRIVERS, open list, Lifshits, Vitaly,
	rex.tsai

On 3/25/2020 08:43, Aaron Ma wrote:
> 
> 
> On 3/25/20 2:36 PM, Neftin, Sasha wrote:
>> On 3/25/2020 06:17, Kai-Heng Feng wrote:
>>> Hi Aaron,
>>>
>>>> On Mar 24, 2020, at 03:16, Aaron Ma <aaron.ma@canonical.com> wrote:
>>>>
>>>> ME takes 2+ seconds to un-configure ULP mode done after resume
>>>> from s2idle on some ThinkPad laptops.
>>>> Without enough wait, reset and re-init will fail with error.
>>>
>>> Thanks, this patch solves the issue. We can drop the DMI quirk in
>>> favor of this patch.
>>>
>>>>
>>>> Fixes: f15bb6dde738cc8fa0 ("e1000e: Add support for S0ix")
>>>> BugLink: https://bugs.launchpad.net/bugs/1865570
>>>> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
>>>> ---
>>>> drivers/net/ethernet/intel/e1000e/ich8lan.c | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>>> b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>>> index b4135c50e905..147b15a2f8b3 100644
>>>> --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>>> +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>>> @@ -1240,9 +1240,9 @@ 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.5sec for ME to clear ULP_CFG_DONE. */
>>>>          while (er32(FWSM) & E1000_FWSM_ULP_CFG_DONE) {
>>>> -            if (i++ == 30) {
>>>> +            if (i++ == 250) {
>>>>                  ret_val = -E1000_ERR_PHY;
>>>>                  goto out;
>>>>              }
>>>
>>> The return value was not caught by the caller, so the error ends up
>>> unnoticed.
>>> Maybe let the caller check the return value of
>>> e1000_disable_ulp_lpt_lp()?
>>>
>>> Kai-Heng
>> Hello Kai-Heng and Aaron,
>> I a bit confused. In our previous conversation you told ME not running.
>> let me shimming in. Increasing delay won't be solve the problem and just
>> mask it. We need to understand why ME take too much time. What is
>> problem with this specific system?
>> So, basically no ME system should works for you.
> 
> Some laptops ME work that's why only reproduce issue on some laptops.
> In this issue i219 is controlled by ME.
> 
Who can explain - why ME required too much time on this system?
Probably need work with ME/BIOS vendor and understand it.
Delay will just mask the real problem - we need to find root cause.
> Quirk is only for 1 model type. But issue is reproduced by more models.
> So it won't work.
> 
> Regard,
> Aaron
> 
>>
>> Meanwhile I prefer keep DMI quirk.
>> Thanks,
>> Sasha
>>>
>>>> -- 
>>>> 2.17.1
>>>>
>>>
>>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] e1000e: bump up timeout to wait when ME un-configure ULP mode
  2020-03-25 13:58       ` Neftin, Sasha
@ 2020-03-25 14:07         ` Aaron Ma
  2020-03-25 15:49         ` [Intel-wired-lan] " Paul Menzel
  1 sibling, 0 replies; 16+ messages in thread
From: Aaron Ma @ 2020-03-25 14:07 UTC (permalink / raw)
  To: Neftin, Sasha, Kai-Heng Feng
  Cc: Kirsher, Jeffrey T, David Miller,
	moderated list:INTEL ETHERNET DRIVERS,
	open list:NETWORKING DRIVERS, open list, Lifshits, Vitaly,
	rex.tsai

On 3/25/20 9:58 PM, Neftin, Sasha wrote:
> On 3/25/2020 08:43, Aaron Ma wrote:
>>
>>
>> On 3/25/20 2:36 PM, Neftin, Sasha wrote:
>>> On 3/25/2020 06:17, Kai-Heng Feng wrote:
>>>> Hi Aaron,
>>>>
>>>>> On Mar 24, 2020, at 03:16, Aaron Ma <aaron.ma@canonical.com> wrote:
>>>>>
>>>>> ME takes 2+ seconds to un-configure ULP mode done after resume
>>>>> from s2idle on some ThinkPad laptops.
>>>>> Without enough wait, reset and re-init will fail with error.
>>>>
>>>> Thanks, this patch solves the issue. We can drop the DMI quirk in
>>>> favor of this patch.
>>>>
>>>>>
>>>>> Fixes: f15bb6dde738cc8fa0 ("e1000e: Add support for S0ix")
>>>>> BugLink: https://bugs.launchpad.net/bugs/1865570
>>>>> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
>>>>> ---
>>>>> drivers/net/ethernet/intel/e1000e/ich8lan.c | 4 ++--
>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>>>> b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>>>> index b4135c50e905..147b15a2f8b3 100644
>>>>> --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>>>> +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>>>> @@ -1240,9 +1240,9 @@ 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.5sec for ME to clear ULP_CFG_DONE. */
>>>>>          while (er32(FWSM) & E1000_FWSM_ULP_CFG_DONE) {
>>>>> -            if (i++ == 30) {
>>>>> +            if (i++ == 250) {
>>>>>                  ret_val = -E1000_ERR_PHY;
>>>>>                  goto out;
>>>>>              }
>>>>
>>>> The return value was not caught by the caller, so the error ends up
>>>> unnoticed.
>>>> Maybe let the caller check the return value of
>>>> e1000_disable_ulp_lpt_lp()?
>>>>
>>>> Kai-Heng
>>> Hello Kai-Heng and Aaron,
>>> I a bit confused. In our previous conversation you told ME not running.
>>> let me shimming in. Increasing delay won't be solve the problem and just
>>> mask it. We need to understand why ME take too much time. What is
>>> problem with this specific system?
>>> So, basically no ME system should works for you.
>>
>> Some laptops ME work that's why only reproduce issue on some laptops.
>> In this issue i219 is controlled by ME.
>>
> Who can explain - why ME required too much time on this system?
> Probably need work with ME/BIOS vendor and understand it.
> Delay will just mask the real problem - we need to find root cause.

IMHO, it should be ME check the link status when link disconnected,
that's why Poll up to 5 seconds for Cable Disconnected indication when
enable ULP.

The reason is same for both disable/enable ULP mode.

I agree to ask ME to check it too.

Regards,
Aaron

>> Quirk is only for 1 model type. But issue is reproduced by more models.
>> So it won't work.
>>
>> Regard,
>> Aaron
>>
>>>
>>> Meanwhile I prefer keep DMI quirk.
>>> Thanks,
>>> Sasha
>>>>
>>>>> -- 
>>>>> 2.17.1
>>>>>
>>>>
>>>
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Intel-wired-lan] [PATCH] e1000e: bump up timeout to wait when ME un-configure ULP mode
  2020-03-25 13:58       ` Neftin, Sasha
  2020-03-25 14:07         ` Aaron Ma
@ 2020-03-25 15:49         ` " Paul Menzel
  2020-03-26 11:29           ` Kai-Heng Feng
  1 sibling, 1 reply; 16+ messages in thread
From: Paul Menzel @ 2020-03-25 15:49 UTC (permalink / raw)
  To: Sasha Neftin, Aaron Ma, Kai-Heng Feng
  Cc: netdev, linux-kernel, intel-wired-lan, David Miller, Rex Tsai

Dear Linux folks,


Am 25.03.20 um 14:58 schrieb Neftin, Sasha:
> On 3/25/2020 08:43, Aaron Ma wrote:

>> On 3/25/20 2:36 PM, Neftin, Sasha wrote:
>>> On 3/25/2020 06:17, Kai-Heng Feng wrote:

>>>>> On Mar 24, 2020, at 03:16, Aaron Ma <aaron.ma@canonical.com> wrote:
>>>>>
>>>>> ME takes 2+ seconds to un-configure ULP mode done after resume
>>>>> from s2idle on some ThinkPad laptops.
>>>>> Without enough wait, reset and re-init will fail with error.
>>>>
>>>> Thanks, this patch solves the issue. We can drop the DMI quirk in
>>>> favor of this patch.
>>>>
>>>>> Fixes: f15bb6dde738cc8fa0 ("e1000e: Add support for S0ix")
>>>>> BugLink: https://bugs.launchpad.net/bugs/1865570
>>>>> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
>>>>> ---
>>>>> drivers/net/ethernet/intel/e1000e/ich8lan.c | 4 ++--
>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>>>> b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>>>> index b4135c50e905..147b15a2f8b3 100644
>>>>> --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>>>> +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>>>> @@ -1240,9 +1240,9 @@ 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.5sec for ME to clear ULP_CFG_DONE. */
>>>>>          while (er32(FWSM) & E1000_FWSM_ULP_CFG_DONE) {
>>>>> -            if (i++ == 30) {
>>>>> +            if (i++ == 250) {
>>>>>                  ret_val = -E1000_ERR_PHY;
>>>>>                  goto out;
>>>>>              }
>>>>
>>>> The return value was not caught by the caller, so the error ends up
>>>> unnoticed.
>>>> Maybe let the caller check the return value of
>>>> e1000_disable_ulp_lpt_lp()?

>>> I a bit confused. In our previous conversation you told ME not running.
>>> let me shimming in. Increasing delay won't be solve the problem and just
>>> mask it. We need to understand why ME take too much time. What is
>>> problem with this specific system?
>>> So, basically no ME system should works for you.
>>
>> Some laptops ME work that's why only reproduce issue on some laptops.
>> In this issue i219 is controlled by ME.
>
> Who can explain - why ME required too much time on this system?
> Probably need work with ME/BIOS vendor and understand it.
> Delay will just mask the real problem - we need to find root cause.
>> Quirk is only for 1 model type. But issue is reproduced by more models.
>> So it won't work.

(Where is Aaron’s reply? It wasn’t delivered to me yet.)

As this is clearly a regression, please revert the commit for now, and 
then find a way to correctly implement S0ix support. Linux’ regression 
policy demands that as no fix has been found since v5.5-rc1. Changing 
Intel ME settings, even if it would work around the issue, is not an 
acceptable solution. Delaying the resume time is also not a solution.

Regarding Intel Management Engine, only Intel knows what it does and 
what the error is, as the ME firmware is proprietary and closed.

Lastly, there is no way to fully disable the Intel Management Engine. 
The HAP stuff claims to stop the Intel ME execution, but nobody knows, 
if it’s successful.

Aaron, Kai-Hang, can you send the revert?


Kind regards,

Paul



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Intel-wired-lan] [PATCH] e1000e: bump up timeout to wait when ME un-configure ULP mode
  2020-03-25 15:49         ` [Intel-wired-lan] " Paul Menzel
@ 2020-03-26 11:29           ` Kai-Heng Feng
  2020-03-26 11:41             ` Paul Menzel
  2020-03-28 10:55             ` David Laight
  0 siblings, 2 replies; 16+ messages in thread
From: Kai-Heng Feng @ 2020-03-26 11:29 UTC (permalink / raw)
  To: Paul Menzel
  Cc: Sasha Neftin, Aaron Ma, netdev, linux-kernel, intel-wired-lan,
	David Miller, Rex Tsai

Hi Paul,

> On Mar 25, 2020, at 23:49, Paul Menzel <pmenzel@molgen.mpg.de> wrote:
> 
> Dear Linux folks,
> 
> 
> Am 25.03.20 um 14:58 schrieb Neftin, Sasha:
>> On 3/25/2020 08:43, Aaron Ma wrote:
> 
>>> On 3/25/20 2:36 PM, Neftin, Sasha wrote:
>>>> On 3/25/2020 06:17, Kai-Heng Feng wrote:
> 
>>>>>> On Mar 24, 2020, at 03:16, Aaron Ma <aaron.ma@canonical.com> wrote:
>>>>>> 
>>>>>> ME takes 2+ seconds to un-configure ULP mode done after resume
>>>>>> from s2idle on some ThinkPad laptops.
>>>>>> Without enough wait, reset and re-init will fail with error.
>>>>> 
>>>>> Thanks, this patch solves the issue. We can drop the DMI quirk in
>>>>> favor of this patch.
>>>>> 
>>>>>> Fixes: f15bb6dde738cc8fa0 ("e1000e: Add support for S0ix")
>>>>>> BugLink: https://bugs.launchpad.net/bugs/1865570
>>>>>> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
>>>>>> ---
>>>>>> drivers/net/ethernet/intel/e1000e/ich8lan.c | 4 ++--
>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>> 
>>>>>> diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>>>>> b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>>>>> index b4135c50e905..147b15a2f8b3 100644
>>>>>> --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>>>>> +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>>>>> @@ -1240,9 +1240,9 @@ 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.5sec for ME to clear ULP_CFG_DONE. */
>>>>>>          while (er32(FWSM) & E1000_FWSM_ULP_CFG_DONE) {
>>>>>> -            if (i++ == 30) {
>>>>>> +            if (i++ == 250) {
>>>>>>                  ret_val = -E1000_ERR_PHY;
>>>>>>                  goto out;
>>>>>>              }
>>>>> 
>>>>> The return value was not caught by the caller, so the error ends up
>>>>> unnoticed.
>>>>> Maybe let the caller check the return value of
>>>>> e1000_disable_ulp_lpt_lp()?
> 
>>>> I a bit confused. In our previous conversation you told ME not running.
>>>> let me shimming in. Increasing delay won't be solve the problem and just
>>>> mask it. We need to understand why ME take too much time. What is
>>>> problem with this specific system?
>>>> So, basically no ME system should works for you.
>>> 
>>> Some laptops ME work that's why only reproduce issue on some laptops.
>>> In this issue i219 is controlled by ME.
>> 
>> Who can explain - why ME required too much time on this system?
>> Probably need work with ME/BIOS vendor and understand it.
>> Delay will just mask the real problem - we need to find root cause.
>>> Quirk is only for 1 model type. But issue is reproduced by more models.
>>> So it won't work.
> 
> (Where is Aaron’s reply? It wasn’t delivered to me yet.)
> 
> As this is clearly a regression, please revert the commit for now, and then find a way to correctly implement S0ix support. Linux’ regression policy demands that as no fix has been found since v5.5-rc1. Changing Intel ME settings, even if it would work around the issue, is not an acceptable solution. Delaying the resume time is also not a solution.

The s0ix patch can reduce power consumption, finally makes S2idle an acceptable sleep method.
So I'd say it's a fix, albeit a regression was introduced.

> 
> Regarding Intel Management Engine, only Intel knows what it does and what the error is, as the ME firmware is proprietary and closed.
> 
> Lastly, there is no way to fully disable the Intel Management Engine. The HAP stuff claims to stop the Intel ME execution, but nobody knows, if it’s successful.
> 
> Aaron, Kai-Hang, can you send the revert?

I consider that as an important fix for s2idle, I don't think reverting is appropriate.

Kai-Heng

> 
> 
> Kind regards,
> 
> Paul
> 
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Intel-wired-lan] [PATCH] e1000e: bump up timeout to wait when ME un-configure ULP mode
  2020-03-26 11:29           ` Kai-Heng Feng
@ 2020-03-26 11:41             ` Paul Menzel
  2020-03-28 10:55             ` David Laight
  1 sibling, 0 replies; 16+ messages in thread
From: Paul Menzel @ 2020-03-26 11:41 UTC (permalink / raw)
  To: Kai-Heng Feng
  Cc: netdev, linux-kernel, intel-wired-lan, David Miller, Rex Tsai

Dear Kai-Heng,


Am 26.03.20 um 12:29 schrieb Kai-Heng Feng:

>> On Mar 25, 2020, at 23:49, Paul Menzel <pmenzel@molgen.mpg.de> wrote:

>> Am 25.03.20 um 14:58 schrieb Neftin, Sasha:
>>> On 3/25/2020 08:43, Aaron Ma wrote:
>>
>>>> On 3/25/20 2:36 PM, Neftin, Sasha wrote:
>>>>> On 3/25/2020 06:17, Kai-Heng Feng wrote:
>>
>>>>>>> On Mar 24, 2020, at 03:16, Aaron Ma <aaron.ma@canonical.com> wrote:
>>>>>>>
>>>>>>> ME takes 2+ seconds to un-configure ULP mode done after resume
>>>>>>> from s2idle on some ThinkPad laptops.
>>>>>>> Without enough wait, reset and re-init will fail with error.
>>>>>>
>>>>>> Thanks, this patch solves the issue. We can drop the DMI quirk in
>>>>>> favor of this patch.
>>>>>>
>>>>>>> Fixes: f15bb6dde738cc8fa0 ("e1000e: Add support for S0ix")
>>>>>>> BugLink: https://bugs.launchpad.net/bugs/1865570
>>>>>>> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
>>>>>>> ---
>>>>>>> drivers/net/ethernet/intel/e1000e/ich8lan.c | 4 ++--
>>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>>>>>> b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>>>>>> index b4135c50e905..147b15a2f8b3 100644
>>>>>>> --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>>>>>> +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>>>>>>> @@ -1240,9 +1240,9 @@ 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.5sec for ME to clear ULP_CFG_DONE. */
>>>>>>>           while (er32(FWSM) & E1000_FWSM_ULP_CFG_DONE) {
>>>>>>> -            if (i++ == 30) {
>>>>>>> +            if (i++ == 250) {
>>>>>>>                   ret_val = -E1000_ERR_PHY;
>>>>>>>                   goto out;
>>>>>>>               }
>>>>>>
>>>>>> The return value was not caught by the caller, so the error ends up
>>>>>> unnoticed.
>>>>>> Maybe let the caller check the return value of
>>>>>> e1000_disable_ulp_lpt_lp()?
>>
>>>>> I a bit confused. In our previous conversation you told ME not running.
>>>>> let me shimming in. Increasing delay won't be solve the problem and just
>>>>> mask it. We need to understand why ME take too much time. What is
>>>>> problem with this specific system?
>>>>> So, basically no ME system should works for you.
>>>>
>>>> Some laptops ME work that's why only reproduce issue on some laptops.
>>>> In this issue i219 is controlled by ME.
>>>
>>> Who can explain - why ME required too much time on this system?
>>> Probably need work with ME/BIOS vendor and understand it.
>>> Delay will just mask the real problem - we need to find root cause.
>>>> Quirk is only for 1 model type. But issue is reproduced by more models.
>>>> So it won't work.
>>
>> (Where is Aaron’s reply? It wasn’t delivered to me yet.)
>>
>> As this is clearly a regression, please revert the commit for now,
>> and then find a way to correctly implement S0ix support. Linux’
>> regression policy demands that as no fix has been found since
>> v5.5-rc1. Changing Intel ME settings, even if it would work around
>> the issue, is not an acceptable solution. Delaying the resume time
>> is also not a solution.
> 
> The s0ix patch can reduce power consumption, finally makes S2idle an
> acceptable sleep method. So I'd say it's a fix, albeit a regression
> was introduced.
> 
>> Regarding Intel Management Engine, only Intel knows what it does
>> and what the error is, as the ME firmware is proprietary and
>> closed.
>> 
>> Lastly, there is no way to fully disable the Intel Management
>> Engine. The HAP stuff claims to stop the Intel ME execution, but
>> nobody knows, if it’s successful.
>> 
>> Aaron, Kai-Hang, can you send the revert?
> 
> I consider that as an important fix for s2idle, I don't think
> reverting is appropriate.

If there is a fix with no other regression, I agree. But there has not 
been one, so please revert. It doesn’t matter if the commit introducing 
the regression fixes something else. It gets too complicated to decide, 
which regression is worth it or not. The Linux kernel guarantees its 
users, they can update any time without breaking user space (in this 
case suspend/resume).  Please read Linus’ several messages about that. 
His message [1] exactly addresses your arguments.

> Yeah, HELL NO!
> 
> Guess what? You're wrong. YOU ARE MISSING THE #1 KERNEL RULE.
> 
> We do not regress, and we do not regress exactly because your are 100% wrong.
> 
> And the reason you state for your opinion is in fact exactly *WHY* you
> are wrong.
> 
> Your "good reasons" are pure and utter garbage.
> 
> The whole point of "we do not regress" is so that people can upgrade
> the kernel and never have to worry about it.
> 
>> Kernel had a bug which has been fixed
> 
> That is *ENTIRELY* immaterial.
> 
> Guys, whether something was buggy or not DOES NOT MATTER.

So, please (also Intel developers), please adhere to this rule, and 
revert the commit.


Kind regards,

Paul


[1]: https://lkml.org/lkml/2018/8/3/621

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [Intel-wired-lan] [PATCH] e1000e: bump up timeout to wait when ME un-configure ULP mode
  2020-03-26 11:29           ` Kai-Heng Feng
  2020-03-26 11:41             ` Paul Menzel
@ 2020-03-28 10:55             ` David Laight
  1 sibling, 0 replies; 16+ messages in thread
From: David Laight @ 2020-03-28 10:55 UTC (permalink / raw)
  To: 'Kai-Heng Feng', Paul Menzel
  Cc: Sasha Neftin, Aaron Ma, netdev, linux-kernel, intel-wired-lan,
	David Miller, Rex Tsai

From: Kai-Heng Feng
> Sent: 26 March 2020 11:30
...
> > Regarding Intel Management Engine, only Intel knows what it does and what the error is, as the ME
> firmware is proprietary and closed.
> >
> > Lastly, there is no way to fully disable the Intel Management Engine.
> > The HAP stuff claims to stop
> > the Intel ME execution, but nobody knows, if it’s successful.

This isn't the only 'bug' caused by the ME logic.

Some systems occasionally spin for many multiples of 50us on any
write to any MAC register - eg to indicate there is a packet to tx.

I really don't understand WTF this ME is playing at on an unmanaged
desktop system - if it receives or sends a packet it is most likely
to be some kind of security attack.
I'm not even sure it needs access during the boot sequence.
Maybe there are some features to get the console output over
ethernet - but they have to be enabled in the BIOS.

We have some small server boards (for 1U systems) that have a
separate ethernet interface for (I think) the ME code.
Better - except you plug a cable in and wonder why is doesn't work.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Intel-wired-lan] [PATCH] e1000e: bump up timeout to wait when ME un-configure ULP mode
  2020-03-23 19:16 [PATCH] e1000e: bump up timeout to wait when ME un-configure ULP mode Aaron Ma
  2020-03-25  4:17 ` Kai-Heng Feng
@ 2020-04-02 12:31 ` Hans de Goede
  2020-04-03  3:15   ` Aaron Ma
  1 sibling, 1 reply; 16+ messages in thread
From: Hans de Goede @ 2020-04-02 12:31 UTC (permalink / raw)
  To: Aaron Ma, jeffrey.t.kirsher, davem, intel-wired-lan, netdev,
	linux-kernel, sasha.neftin

Hi,

On 3/23/20 8:16 PM, Aaron Ma wrote:
> ME takes 2+ seconds to un-configure ULP mode done after resume
> from s2idle on some ThinkPad laptops.
> Without enough wait, reset and re-init will fail with error.
> 
> Fixes: f15bb6dde738cc8fa0 ("e1000e: Add support for S0ix")
> BugLink: https://bugs.launchpad.net/bugs/1865570
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>

I have been testing this bug because this is being reported against
Fedora 32 too:

https://bugzilla.redhat.com/show_bug.cgi?id=1816621

I can confirm that this patch fixes the problem of both
a X1 7th gen as a X1 8th gen no longer suspending after
a suspend resume cycle.

Not only does it fix that, before this patch the kernel
would regularly log the following error on these laptops
independent of suspend/resume activity:

e1000e 0000:00:1f.6 enp0s31f6: Hardware Error

These messages are now also gone. So it seems that the timeout
is really just too short.

I can agree that it would be good to better understand this;
and/or to get the ME firmware fixed to not take so long.

But in my experience when dealing with e.g. embedded-controller
in various laptops sometimes the firmware of these devives
simply just takes a long time for certain things.

This fix fixes a real problem, on a popular model laptop
and since it just extends a timeout it is a pretty harmless
(no chance of regressions) fix. As such since there seems
to be no other solution in sight, can we please move forward
with this fix for now ?

Regards,

Hans





> ---
>   drivers/net/ethernet/intel/e1000e/ich8lan.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c
> index b4135c50e905..147b15a2f8b3 100644
> --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
> +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
> @@ -1240,9 +1240,9 @@ 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.5sec for ME to clear ULP_CFG_DONE. */
>   		while (er32(FWSM) & E1000_FWSM_ULP_CFG_DONE) {
> -			if (i++ == 30) {
> +			if (i++ == 250) {
>   				ret_val = -E1000_ERR_PHY;
>   				goto out;
>   			}
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Intel-wired-lan] [PATCH] e1000e: bump up timeout to wait when ME un-configure ULP mode
  2020-04-02 12:31 ` Hans de Goede
@ 2020-04-03  3:15   ` Aaron Ma
  2020-04-03  7:37     ` Paul Menzel
  0 siblings, 1 reply; 16+ messages in thread
From: Aaron Ma @ 2020-04-03  3:15 UTC (permalink / raw)
  To: Hans de Goede, jeffrey.t.kirsher, davem, intel-wired-lan, netdev,
	linux-kernel, sasha.neftin

Hi Jeffrey:

I have received the email that you apply this patch to next-queue branch
dev-queue.

But after this branch is rebased to v5.6, I can't find it.

Will you apply again?

Thanks,
Aaron

On 4/2/20 8:31 PM, Hans de Goede wrote:
> 
> This fix fixes a real problem, on a popular model laptop
> and since it just extends a timeout it is a pretty harmless
> (no chance of regressions) fix. As such since there seems
> to be no other solution in sight, can we please move forward
> with this fix for now ?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Intel-wired-lan] [PATCH] e1000e: bump up timeout to wait when ME un-configure ULP mode
  2020-04-03  3:15   ` Aaron Ma
@ 2020-04-03  7:37     ` Paul Menzel
  0 siblings, 0 replies; 16+ messages in thread
From: Paul Menzel @ 2020-04-03  7:37 UTC (permalink / raw)
  To: Aaron Ma, Hans de Goede, Jeff Kirsher, davem, intel-wired-lan,
	netdev, linux-kernel, sasha.neftin

Dear Linux folks,


Am 03.04.20 um 05:15 schrieb Aaron Ma:

> I have received the email that you apply this patch to next-queue branch
> dev-queue.
> 
> But after this branch is rebased to v5.6, I can't find it.
> 
> Will you apply again?

I really urge you to write more elaborate commit messages.

The exact error is not listed. The known regressed devices are not 
listed, so possible testers cannot easily test or affected people cannot 
find it, when searching the Linux git history.

How did you find out, that ME takes more than two seconds?

Also, it’s not clear, what effect increasing the timeout has. Does the 
whole resume process take longer or just getting the Ethernet device 
back up?

Lastly, in case of the timeout, an error message should be printed, so 
people with even more broken ME devices, get a clue.

Without this information, how can anybody know, if this is the right fix 
and distributions, if it should be backported?


Kind regards,

Paul

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, back to index

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-23 19:16 [PATCH] e1000e: bump up timeout to wait when ME un-configure ULP mode Aaron Ma
2020-03-25  4:17 ` Kai-Heng Feng
2020-03-25  6:36   ` Neftin, Sasha
2020-03-25  6:39     ` Kai-Heng Feng
2020-03-25  6:42       ` Tsai, Rex
2020-03-25  6:49         ` Aaron Ma
2020-03-25  6:43     ` Aaron Ma
2020-03-25 13:58       ` Neftin, Sasha
2020-03-25 14:07         ` Aaron Ma
2020-03-25 15:49         ` [Intel-wired-lan] " Paul Menzel
2020-03-26 11:29           ` Kai-Heng Feng
2020-03-26 11:41             ` Paul Menzel
2020-03-28 10:55             ` David Laight
2020-04-02 12:31 ` Hans de Goede
2020-04-03  3:15   ` Aaron Ma
2020-04-03  7:37     ` Paul Menzel

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org
	public-inbox-index netdev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git