[Intel-wired-lan] [PATCH v1] e1000e: disable s0ix entry and exit flows for ME systems

[Paul Menzel]

Dear Vitaly,


Am 07.05.20 um 19:14 schrieb Vitaly Lifshits:
> Since ME systems do not support SLP_S0 in S0ix state, and S0ix entry
> and exit flows may cause errors on them it is best to avoid using
> e1000e_s0ix_entry_flow and e1000e_s0ix_exit_flow functions.

Can you please elaborate? I am unable to parse the sentence. Do you mean 
the SLP_S0# signal on a mainboard? What are “ME systems”? Do you mean AMT?

Where is that limitation you state documented?

Also please mention, that there are problem reports. (But wasn’t it only 
for some devices?)

> This was done by creating a struct of all devices that comes with ME
> and by checking if the current device has ME.

Please list the bug reports and discussions. Also, please CC the 
affected people.

> Signed-off-by: Vitaly Lifshits <vitaly.lifshits at intel.com>
> ---
>   drivers/net/ethernet/intel/e1000e/netdev.c | 45 ++++++++++++++++++++++++++++--
>   1 file changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index e0b074820b47..57af6b81d79d 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -107,6 +107,45 @@ static const struct e1000_reg_info e1000_reg_info_tbl[] = {
>   	{0, NULL}
>   };
>   
> +struct e1000e_me_supported {
> +	u16 device_id;		/* supported device ID */
> +};
> +
> +static const struct e1000e_me_supported me_supported[] = {
> +	{E1000_DEV_ID_PCH_LPT_I217_LM},
> +	{E1000_DEV_ID_PCH_LPTLP_I218_LM},
> +	{E1000_DEV_ID_PCH_I218_LM2},
> +	{E1000_DEV_ID_PCH_I218_LM3},
> +	{E1000_DEV_ID_PCH_SPT_I219_LM},
> +	{E1000_DEV_ID_PCH_SPT_I219_LM2},
> +	{E1000_DEV_ID_PCH_LBG_I219_LM3},
> +	{E1000_DEV_ID_PCH_SPT_I219_LM4},
> +	{E1000_DEV_ID_PCH_SPT_I219_LM5},
> +	{E1000_DEV_ID_PCH_CNP_I219_LM6},
> +	{E1000_DEV_ID_PCH_CNP_I219_LM7},
> +	{E1000_DEV_ID_PCH_ICP_I219_LM8},
> +	{E1000_DEV_ID_PCH_ICP_I219_LM9},
> +	{E1000_DEV_ID_PCH_CMP_I219_LM10},
> +	{E1000_DEV_ID_PCH_CMP_I219_LM11},
> +	{E1000_DEV_ID_PCH_CMP_I219_LM12},
> +	{E1000_DEV_ID_PCH_TGP_I219_LM13},
> +	{E1000_DEV_ID_PCH_TGP_I219_LM14},
> +	{E1000_DEV_ID_PCH_TGP_I219_LM15},
> +	{0}
> +};
> +
> +static bool e1000e_check_me(u16 device_id)
> +{
> +	struct e1000e_me_supported *id;
> +
> +	for (id = (struct e1000e_me_supported *)me_supported;
> +	     id->device_id; id++)
> +		if (device_id == id->device_id)
> +			return true;
> +
> +	return false;
> +}
> +
>   /**
>    * __ew32_prepare - prepare to write to MAC CSR register on certain parts
>    * @hw: pointer to the HW structure
> @@ -6912,7 +6951,8 @@ static int e1000e_pm_suspend(struct device *dev)
>   		e1000e_pm_thaw(dev);
>   
>   	/* Introduce S0ix implementation */
> -	if (hw->mac.type >= e1000_pch_cnp)
> +	if (hw->mac.type >= e1000_pch_cnp &&
> +	    !e1000e_check_me(hw->adapter->pdev->device))
>   		e1000e_s0ix_entry_flow(adapter);
>   
>   	return rc;
> @@ -6927,7 +6967,8 @@ static int e1000e_pm_resume(struct device *dev)
>   	int rc;
>   
>   	/* Introduce S0ix implementation */
> -	if (hw->mac.type >= e1000_pch_cnp)
> +	if (hw->mac.type >= e1000_pch_cnp &&
> +	    !e1000e_check_me(hw->adapter->pdev->device))
>   		e1000e_s0ix_exit_flow(adapter);
>   
>   	rc = __e1000_resume(pdev);
> 


Kind regards,

Paul