diff mbox series

[v1] e1000e: disable s0ix entry and exit flows for ME systems

Message ID 20200507171406.6883-1-vitaly.lifshits@intel.com
State Accepted
Delegated to: Jeff Kirsher
Headers show
Series [v1] e1000e: disable s0ix entry and exit flows for ME systems | expand

Commit Message

Vitaly Lifshits May 7, 2020, 5:14 p.m. UTC
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.

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

Signed-off-by: Vitaly Lifshits <vitaly.lifshits@intel.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 45 ++++++++++++++++++++++++++++--
 1 file changed, 43 insertions(+), 2 deletions(-)

Comments

Paul Menzel May 7, 2020, 10:37 p.m. UTC | #1
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@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
Brown, Aaron F May 21, 2020, 7:23 a.m. UTC | #2
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Vitaly
> Lifshits
> Sent: Thursday, May 7, 2020 10:14 AM
> To: intel-wired-lan@lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH v1] e1000e: disable s0ix entry and exit flows
> for ME systems
> 
> 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.
> 
> This was done by creating a struct of all devices that comes with ME
> and by checking if the current device has ME.
> 
> Signed-off-by: Vitaly Lifshits <vitaly.lifshits@intel.com>
> ---
>  drivers/net/ethernet/intel/e1000e/netdev.c | 45
> ++++++++++++++++++++++++++++--
>  1 file changed, 43 insertions(+), 2 deletions(-)
> 
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
diff mbox series

Patch

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);