All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-wired-lan] [PATCH net v1] i40e: Fix for failed to init adminq while VF reset
@ 2020-12-07 11:42 Mateusz Palczewski
  2020-12-07 13:32 ` Paul Menzel
  2020-12-07 19:18   ` kernel test robot
  0 siblings, 2 replies; 6+ messages in thread
From: Mateusz Palczewski @ 2020-12-07 11:42 UTC (permalink / raw)
  To: intel-wired-lan

Fix for failed to init adminq: -53 while VF is resetting via MAC
address changing procedure.
Added sync module to avoid reading deadbeef value in reinit adminq
during software reset.
Without this patch it is possible to trigger VF reset procedure
during reinit adminq. This resulted in an incorrect reading of
value from the AQP registers and generated the -53 error.

Fixes: 9dc2e417383815"(i40e: split some code in i40e_reset_vf into helpers")
Signed-off-by: Grzegorz Szczurek <grzegorzx.szczurek@intel.com>
Signed-off-by: Mateusz Palczewski <mateusz.palczewski@intel.com>
Reviewed-by: Jaroslaw Gawin <jaroslawx.gawin@intel.com>
Reviewed-by: Slawomir Laba <slawomirx.laba@intel.com>
Reviewed-by: Karen Sornek <karen.sornek@intel.com>
Reviewed-by: Grzegorz Siwik <grzegorz.siwik@intel.com>
Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
---
 .../ethernet/intel/i40e/i40e_virtchnl_pf.c    | 46 ++++++++++++++++++-
 .../ethernet/intel/i40e/i40e_virtchnl_pf.h    |  2 +
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 21ee564..afccbbc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -1297,6 +1297,32 @@ static i40e_status i40e_config_vf_promiscuous_mode(struct i40e_vf *vf,
 	return aq_ret;
 }
 
+/**
+* i40e_sync_vfr_reset
+* @hw: pointer to hw struct
+* @vf_id: VF identifier
+*
+* Before trigger hardware reset, we need to know if no other process has
+* reserved the hardware for any reset operations. This check is done by
+* examining the status of the ADMINQ bit in VF interrupt register.
+**/
+static int i40e_sync_vfr_reset(struct i40e_hw *hw, int vf_id)
+{
+	u32 reg;
+	int i;
+
+	for (i = 0; i < I40E_VFR_WAIT_COUNT; i++) {
+		reg = rd32(hw, I40E_VFINT_ICR0_ENA(vf_id)) &
+			   I40E_VFINT_ICR0_ADMINQ_MASK;
+		if (reg)
+			return 0;
+
+		usleep_range(10, 200);
+	}
+
+	return -EAGAIN;
+}
+
 /**
  * i40e_trigger_vf_reset
  * @vf: pointer to the VF structure
@@ -1311,9 +1337,11 @@ static void i40e_trigger_vf_reset(struct i40e_vf *vf, bool flr)
 	struct i40e_pf *pf = vf->pf;
 	struct i40e_hw *hw = &pf->hw;
 	u32 reg, reg_idx, bit_idx;
+	bool vf_active;
+	u32 radq;
 
 	/* warn the VF */
-	clear_bit(I40E_VF_STATE_ACTIVE, &vf->vf_states);
+	vf_active = test_and_clear_bit(I40E_VF_STATE_ACTIVE, &vf->vf_states);
 
 	/* Disable VF's configuration API during reset. The flag is re-enabled
 	 * in i40e_alloc_vf_res(), when it's safe again to access VF's VSI.
@@ -1327,7 +1355,19 @@ static void i40e_trigger_vf_reset(struct i40e_vf *vf, bool flr)
 	 * just need to clean up, so don't hit the VFRTRIG register.
 	 */
 	if (!flr) {
-		/* reset VF using VPGEN_VFRTRIG reg */
+		/* Sync VFR reset before trigger next one */
+		radq = rd32(hw, I40E_VFINT_ICR0_ENA(vf->vf_id)) &
+			    I40E_VFINT_ICR0_ADMINQ_MASK;
+		if (vf_active && !radq)
+			/* waiting for finish reset by virtual driver */
+			if (i40e_sync_vfr_reset(hw, vf->vf_id))
+				dev_info(&pf->pdev->dev,
+					 "Reset VF %d never finished\n",
+					 vf->vf_id);
+
+		/* Reset VF using VPGEN_VFRTRIG reg. It is also setting
+		 * in progress state in rstat1 register.
+		 */
 		reg = rd32(hw, I40E_VPGEN_VFRTRIG(vf->vf_id));
 		reg |= I40E_VPGEN_VFRTRIG_VFSWR_MASK;
 		wr32(hw, I40E_VPGEN_VFRTRIG(vf->vf_id), reg);
@@ -1457,6 +1497,7 @@ bool i40e_reset_vf(struct i40e_vf *vf, bool flr)
 	i40e_cleanup_reset_vf(vf);
 
 	i40e_flush(hw);
+	usleep_range(20000, 40000);
 	clear_bit(__I40E_VF_DISABLE, pf->state);
 
 	return true;
@@ -1561,6 +1602,7 @@ bool i40e_reset_all_vfs(struct i40e_pf *pf, bool flr)
 		i40e_cleanup_reset_vf(&pf->vf[v]);
 
 	i40e_flush(hw);
+	usleep_range(20000, 40000);
 	clear_bit(__I40E_VF_DISABLE, pf->state);
 
 	return true;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
index 5491215..fb4f5db 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
@@ -18,6 +18,8 @@
 
 #define I40E_MAX_VF_PROMISC_FLAGS	3
 
+#define I40E_VFR_WAIT_COUNT		100
+
 /* Various queue ctrls */
 enum i40e_queue_ctrl {
 	I40E_QUEUE_CTRL_UNKNOWN = 0,
-- 
2.17.1

---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Sowackiego 173 | 80-298 Gdask | Sd Rejonowy Gdask Pnoc | VII Wydzia Gospodarczy Krajowego Rejestru Sdowego - KRS 101882 | NIP 957-07-52-316 | Kapita zakadowy 200.000 PLN.
Ta wiadomo wraz z zacznikami jest przeznaczona dla okrelonego adresata i moe zawiera informacje poufne. W razie przypadkowego otrzymania tej wiadomoci, prosimy o powiadomienie nadawcy oraz trwae jej usunicie; jakiekolwiek przegldanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.
 


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

* [Intel-wired-lan] [PATCH net v1] i40e: Fix for failed to init adminq while VF reset
  2020-12-07 11:42 [Intel-wired-lan] [PATCH net v1] i40e: Fix for failed to init adminq while VF reset Mateusz Palczewski
@ 2020-12-07 13:32 ` Paul Menzel
  2020-12-07 19:18   ` kernel test robot
  1 sibling, 0 replies; 6+ messages in thread
From: Paul Menzel @ 2020-12-07 13:32 UTC (permalink / raw)
  To: intel-wired-lan

Dear Mateusz,


Am 07.12.20 um 12:42 schrieb Mateusz Palczewski:
> Fix for failed to init adminq: -53 while VF is resetting via MAC
> address changing procedure.
> Added sync module to avoid reading deadbeef value in reinit adminq
> during software reset.
> Without this patch it is possible to trigger VF reset procedure
> during reinit adminq. This resulted in an incorrect reading of
> value from the AQP registers and generated the -53 error.

Thank you for fixing this issue. (Blank lines between paragraphs would 
make them better readable.)

How can the problem be reproduced? Do you have a recipe do cause this?

I have some nits below, but maybe you also can use the summary below for 
the git commit message summary, so it?s a statement with a verb in 
imperative mood.

 > Fix init adminq failure while VF reset

You are adding several delays below. What is the effect on that on 
overall timings of the driver?


> Fixes: 9dc2e417383815"(i40e: split some code in i40e_reset_vf into helpers")
> Signed-off-by: Grzegorz Szczurek <grzegorzx.szczurek@intel.com>
> Signed-off-by: Mateusz Palczewski <mateusz.palczewski@intel.com>
> Reviewed-by: Jaroslaw Gawin <jaroslawx.gawin@intel.com>
> Reviewed-by: Slawomir Laba <slawomirx.laba@intel.com>
> Reviewed-by: Karen Sornek <karen.sornek@intel.com>
> Reviewed-by: Grzegorz Siwik <grzegorz.siwik@intel.com>
> Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---
>   .../ethernet/intel/i40e/i40e_virtchnl_pf.c    | 46 ++++++++++++++++++-
>   .../ethernet/intel/i40e/i40e_virtchnl_pf.h    |  2 +
>   2 files changed, 46 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> index 21ee564..afccbbc 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> @@ -1297,6 +1297,32 @@ static i40e_status i40e_config_vf_promiscuous_mode(struct i40e_vf *vf,
>   	return aq_ret;
>   }
>   
> +/**
> +* i40e_sync_vfr_reset
> +* @hw: pointer to hw struct
> +* @vf_id: VF identifier
> +*
> +* Before trigger hardware reset, we need to know if no other process has

Small typo:

 > Before trigger*ing a* hardware reset, ?

> +* reserved the hardware for any reset operations. This check is done by
> +* examining the status of the ADMINQ bit in VF interrupt register.

Please add how much time this delays.

> +**/
> +static int i40e_sync_vfr_reset(struct i40e_hw *hw, int vf_id)
> +{
> +	u32 reg;
> +	int i;
> +
> +	for (i = 0; i < I40E_VFR_WAIT_COUNT; i++) {
> +		reg = rd32(hw, I40E_VFINT_ICR0_ENA(vf_id)) &
> +			   I40E_VFINT_ICR0_ADMINQ_MASK;
> +		if (reg)
> +			return 0;

The variable `reg` does not seem to be needed.

> +
> +		usleep_range(10, 200);
> +	}

Should an error be logged in case of failure?

> +
> +	return -EAGAIN;
> +}
> +
>   /**
>    * i40e_trigger_vf_reset
>    * @vf: pointer to the VF structure
> @@ -1311,9 +1337,11 @@ static void i40e_trigger_vf_reset(struct i40e_vf *vf, bool flr)
>   	struct i40e_pf *pf = vf->pf;
>   	struct i40e_hw *hw = &pf->hw;
>   	u32 reg, reg_idx, bit_idx;
> +	bool vf_active;
> +	u32 radq;
>   
>   	/* warn the VF */
> -	clear_bit(I40E_VF_STATE_ACTIVE, &vf->vf_states);
> +	vf_active = test_and_clear_bit(I40E_VF_STATE_ACTIVE, &vf->vf_states);
>   
>   	/* Disable VF's configuration API during reset. The flag is re-enabled
>   	 * in i40e_alloc_vf_res(), when it's safe again to access VF's VSI.
> @@ -1327,7 +1355,19 @@ static void i40e_trigger_vf_reset(struct i40e_vf *vf, bool flr)
>   	 * just need to clean up, so don't hit the VFRTRIG register.
>   	 */
>   	if (!flr) {
> -		/* reset VF using VPGEN_VFRTRIG reg */
> +		/* Sync VFR reset before trigger next one */

triggering

> +		radq = rd32(hw, I40E_VFINT_ICR0_ENA(vf->vf_id)) &
> +			    I40E_VFINT_ICR0_ADMINQ_MASK;
> +		if (vf_active && !radq)
> +			/* waiting for finish reset by virtual driver */

waiting for virtual driver to finish reset

> +			if (i40e_sync_vfr_reset(hw, vf->vf_id))
> +				dev_info(&pf->pdev->dev,
> +					 "Reset VF %d never finished\n",

? after X ms.

Also, please add the consequences of that failure. What does it mean for 
the user, that the reset was unsuccessful?

> +					 vf->vf_id);
> +
> +		/* Reset VF using VPGEN_VFRTRIG reg. It is also setting
> +		 * in progress state in rstat1 register.
> +		 */
>   		reg = rd32(hw, I40E_VPGEN_VFRTRIG(vf->vf_id));
>   		reg |= I40E_VPGEN_VFRTRIG_VFSWR_MASK;
>   		wr32(hw, I40E_VPGEN_VFRTRIG(vf->vf_id), reg);
> @@ -1457,6 +1497,7 @@ bool i40e_reset_vf(struct i40e_vf *vf, bool flr)
>   	i40e_cleanup_reset_vf(vf);
>   
>   	i40e_flush(hw);
> +	usleep_range(20000, 40000);

This seems unrelated to the commit?

>   	clear_bit(__I40E_VF_DISABLE, pf->state);
>   
>   	return true;
> @@ -1561,6 +1602,7 @@ bool i40e_reset_all_vfs(struct i40e_pf *pf, bool flr)
>   		i40e_cleanup_reset_vf(&pf->vf[v]);
>   
>   	i40e_flush(hw);
> +	usleep_range(20000, 40000);

Ditto.

>   	clear_bit(__I40E_VF_DISABLE, pf->state);
>   
>   	return true;
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
> index 5491215..fb4f5db 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
> @@ -18,6 +18,8 @@
>   
>   #define I40E_MAX_VF_PROMISC_FLAGS	3
>   
> +#define I40E_VFR_WAIT_COUNT		100

Why is 100 chosen? The commit message does not say.

> +
>   /* Various queue ctrls */
>   enum i40e_queue_ctrl {
>   	I40E_QUEUE_CTRL_UNKNOWN = 0,
> 


Kind regards,

Paul

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

* [Intel-wired-lan] [PATCH net v1] i40e: Fix for failed to init adminq while VF reset
  2020-12-07 11:42 [Intel-wired-lan] [PATCH net v1] i40e: Fix for failed to init adminq while VF reset Mateusz Palczewski
@ 2020-12-07 19:18   ` kernel test robot
  2020-12-07 19:18   ` kernel test robot
  1 sibling, 0 replies; 6+ messages in thread
From: kernel test robot @ 2020-12-07 19:18 UTC (permalink / raw)
  To: intel-wired-lan

Hi Mateusz,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net/master]

url:    https://github.com/0day-ci/linux/commits/Mateusz-Palczewski/i40e-Fix-for-failed-to-init-adminq-while-VF-reset/20201207-194429
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 10c678bd0a035ac2c64a9b26b222f20556227a53
config: alpha-randconfig-r034-20201207 (attached as .config)
compiler: alpha-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/3473ae8b20a6b744f90aa0479ad4d66567f8d832
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Mateusz-Palczewski/i40e-Fix-for-failed-to-init-adminq-while-VF-reset/20201207-194429
        git checkout 3473ae8b20a6b744f90aa0479ad4d66567f8d832
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=alpha 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from drivers/net/ethernet/intel/i40e/i40e_type.h:8,
                    from drivers/net/ethernet/intel/i40e/i40e.h:40,
                    from drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c:4:
   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c: In function 'i40e_sync_vfr_reset':
>> drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c:1315:18: error: implicit declaration of function 'I40E_VFINT_ICR0_ENA'; did you mean 'I40E_PFINT_ICR0_ENA'? [-Werror=implicit-function-declaration]
    1315 |   reg = rd32(hw, I40E_VFINT_ICR0_ENA(vf_id)) &
         |                  ^~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/intel/i40e/i40e_osdep.h:27:45: note: in definition of macro 'rd32'
      27 | #define rd32(a, reg)  readl((a)->hw_addr + (reg))
         |                                             ^~~
>> drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c:1316:7: error: 'I40E_VFINT_ICR0_ADMINQ_MASK' undeclared (first use in this function); did you mean 'I40E_PFINT_ICR0_ADMINQ_MASK'?
    1316 |       I40E_VFINT_ICR0_ADMINQ_MASK;
         |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~
         |       I40E_PFINT_ICR0_ADMINQ_MASK
   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c:1316:7: note: each undeclared identifier is reported only once for each function it appears in
   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c: In function 'i40e_trigger_vf_reset':
   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c:1360:8: error: 'I40E_VFINT_ICR0_ADMINQ_MASK' undeclared (first use in this function); did you mean 'I40E_PFINT_ICR0_ADMINQ_MASK'?
    1360 |        I40E_VFINT_ICR0_ADMINQ_MASK;
         |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~
         |        I40E_PFINT_ICR0_ADMINQ_MASK
   cc1: some warnings being treated as errors

vim +1315 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c

  1299	
  1300	/**
  1301	* i40e_sync_vfr_reset
  1302	* @hw: pointer to hw struct
  1303	* @vf_id: VF identifier
  1304	*
  1305	* Before trigger hardware reset, we need to know if no other process has
  1306	* reserved the hardware for any reset operations. This check is done by
  1307	* examining the status of the ADMINQ bit in VF interrupt register.
  1308	**/
  1309	static int i40e_sync_vfr_reset(struct i40e_hw *hw, int vf_id)
  1310	{
  1311		u32 reg;
  1312		int i;
  1313	
  1314		for (i = 0; i < I40E_VFR_WAIT_COUNT; i++) {
> 1315			reg = rd32(hw, I40E_VFINT_ICR0_ENA(vf_id)) &
> 1316				   I40E_VFINT_ICR0_ADMINQ_MASK;
  1317			if (reg)
  1318				return 0;
  1319	
  1320			usleep_range(10, 200);
  1321		}
  1322	
  1323		return -EAGAIN;
  1324	}
  1325	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all at lists.01.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 28591 bytes
Desc: not available
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20201208/5cd5ec94/attachment-0001.bin>

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

* Re: [Intel-wired-lan] [PATCH net v1] i40e: Fix for failed to init adminq while VF reset
@ 2020-12-07 19:18   ` kernel test robot
  0 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2020-12-07 19:18 UTC (permalink / raw)
  To: kbuild-all

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

Hi Mateusz,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net/master]

url:    https://github.com/0day-ci/linux/commits/Mateusz-Palczewski/i40e-Fix-for-failed-to-init-adminq-while-VF-reset/20201207-194429
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 10c678bd0a035ac2c64a9b26b222f20556227a53
config: alpha-randconfig-r034-20201207 (attached as .config)
compiler: alpha-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/3473ae8b20a6b744f90aa0479ad4d66567f8d832
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Mateusz-Palczewski/i40e-Fix-for-failed-to-init-adminq-while-VF-reset/20201207-194429
        git checkout 3473ae8b20a6b744f90aa0479ad4d66567f8d832
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=alpha 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from drivers/net/ethernet/intel/i40e/i40e_type.h:8,
                    from drivers/net/ethernet/intel/i40e/i40e.h:40,
                    from drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c:4:
   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c: In function 'i40e_sync_vfr_reset':
>> drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c:1315:18: error: implicit declaration of function 'I40E_VFINT_ICR0_ENA'; did you mean 'I40E_PFINT_ICR0_ENA'? [-Werror=implicit-function-declaration]
    1315 |   reg = rd32(hw, I40E_VFINT_ICR0_ENA(vf_id)) &
         |                  ^~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/intel/i40e/i40e_osdep.h:27:45: note: in definition of macro 'rd32'
      27 | #define rd32(a, reg)  readl((a)->hw_addr + (reg))
         |                                             ^~~
>> drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c:1316:7: error: 'I40E_VFINT_ICR0_ADMINQ_MASK' undeclared (first use in this function); did you mean 'I40E_PFINT_ICR0_ADMINQ_MASK'?
    1316 |       I40E_VFINT_ICR0_ADMINQ_MASK;
         |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~
         |       I40E_PFINT_ICR0_ADMINQ_MASK
   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c:1316:7: note: each undeclared identifier is reported only once for each function it appears in
   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c: In function 'i40e_trigger_vf_reset':
   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c:1360:8: error: 'I40E_VFINT_ICR0_ADMINQ_MASK' undeclared (first use in this function); did you mean 'I40E_PFINT_ICR0_ADMINQ_MASK'?
    1360 |        I40E_VFINT_ICR0_ADMINQ_MASK;
         |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~
         |        I40E_PFINT_ICR0_ADMINQ_MASK
   cc1: some warnings being treated as errors

vim +1315 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c

  1299	
  1300	/**
  1301	* i40e_sync_vfr_reset
  1302	* @hw: pointer to hw struct
  1303	* @vf_id: VF identifier
  1304	*
  1305	* Before trigger hardware reset, we need to know if no other process has
  1306	* reserved the hardware for any reset operations. This check is done by
  1307	* examining the status of the ADMINQ bit in VF interrupt register.
  1308	**/
  1309	static int i40e_sync_vfr_reset(struct i40e_hw *hw, int vf_id)
  1310	{
  1311		u32 reg;
  1312		int i;
  1313	
  1314		for (i = 0; i < I40E_VFR_WAIT_COUNT; i++) {
> 1315			reg = rd32(hw, I40E_VFINT_ICR0_ENA(vf_id)) &
> 1316				   I40E_VFINT_ICR0_ADMINQ_MASK;
  1317			if (reg)
  1318				return 0;
  1319	
  1320			usleep_range(10, 200);
  1321		}
  1322	
  1323		return -EAGAIN;
  1324	}
  1325	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 28591 bytes --]

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

* [Intel-wired-lan] [PATCH net v1] i40e: Fix for failed to init adminq while VF reset
  2021-11-30  7:32 Sornek, Karen
@ 2021-11-30 21:42 ` Nguyen, Anthony L
  0 siblings, 0 replies; 6+ messages in thread
From: Nguyen, Anthony L @ 2021-11-30 21:42 UTC (permalink / raw)
  To: intel-wired-lan

On Tue, 2021-11-30 at 08:32 +0100, Sornek, Karen wrote:
> From: Karen Sornek <karen.sornek@intel.com>
> 
> Fix for failed to init adminq: -53 while VF is resetting via MAC
> address changing procedure.
> Added sync module to avoid reading deadbeef value in reinit adminq
> during software reset.
> Without this patch it is possible to trigger VF reset procedure
> during reinit adminq. This resulted in an incorrect reading of
> value from the AQP registers and generated the -53 error.
> 

If this is for net, it needs a Fixes:

> Signed-off-by: Grzegorz Szczurek <grzegorzx.szczurek@intel.com>
> Signed-off-by: Karen Sornek <karen.sornek@intel.com>

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

* [Intel-wired-lan] [PATCH net v1] i40e: Fix for failed to init adminq while VF reset
@ 2021-11-30  7:32 Sornek, Karen
  2021-11-30 21:42 ` Nguyen, Anthony L
  0 siblings, 1 reply; 6+ messages in thread
From: Sornek, Karen @ 2021-11-30  7:32 UTC (permalink / raw)
  To: intel-wired-lan

From: Karen Sornek <karen.sornek@intel.com>

Fix for failed to init adminq: -53 while VF is resetting via MAC
address changing procedure.
Added sync module to avoid reading deadbeef value in reinit adminq
during software reset.
Without this patch it is possible to trigger VF reset procedure
during reinit adminq. This resulted in an incorrect reading of
value from the AQP registers and generated the -53 error.

Signed-off-by: Grzegorz Szczurek <grzegorzx.szczurek@intel.com>
Signed-off-by: Karen Sornek <karen.sornek@intel.com>
---
 .../net/ethernet/intel/i40e/i40e_register.h   |  3 ++
 .../ethernet/intel/i40e/i40e_virtchnl_pf.c    | 44 ++++++++++++++++++-
 .../ethernet/intel/i40e/i40e_virtchnl_pf.h    |  1 +
 3 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_register.h b/drivers/net/ethernet/intel/i40e/i40e_register.h
index 8d0588a27a05..1908eed4fa5e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_register.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_register.h
@@ -413,6 +413,9 @@
 #define I40E_VFINT_DYN_CTLN(_INTVF) (0x00024800 + ((_INTVF) * 4)) /* _i=0...511 */ /* Reset: VFR */
 #define I40E_VFINT_DYN_CTLN_CLEARPBA_SHIFT 1
 #define I40E_VFINT_DYN_CTLN_CLEARPBA_MASK I40E_MASK(0x1, I40E_VFINT_DYN_CTLN_CLEARPBA_SHIFT)
+#define I40E_VFINT_ICR0_ADMINQ_SHIFT 30
+#define I40E_VFINT_ICR0_ADMINQ_MASK I40E_MASK(0x1, I40E_VFINT_ICR0_ADMINQ_SHIFT)
+#define I40E_VFINT_ICR0_ENA(_VF) (0x0002C000 + ((_VF) * 4)) /* _i=0...127 */ /* Reset: CORER */
 #define I40E_VPINT_AEQCTL(_VF) (0x0002B800 + ((_VF) * 4)) /* _i=0...127 */ /* Reset: CORER */
 #define I40E_VPINT_AEQCTL_MSIX_INDX_SHIFT 0
 #define I40E_VPINT_AEQCTL_ITR_INDX_SHIFT 11
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 3efc6926d308..d4c6914d2347 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -1379,6 +1379,32 @@ static i40e_status i40e_config_vf_promiscuous_mode(struct i40e_vf *vf,
 	return aq_ret;
 }
 
+/**
+ * i40e_sync_vfr_reset
+ * @hw: pointer to hw struct
+ * @vf_id: VF identifier
+ *
+ * Before trigger hardware reset, we need to know if no other process has
+ * reserved the hardware for any reset operations. This check is done by
+ * examining the status of the RSTAT1 register used to signal the reset.
+ **/
+static int i40e_sync_vfr_reset(struct i40e_hw *hw, int vf_id)
+{
+	u32 reg;
+	int i;
+
+	for (i = 0; i < I40E_VFR_WAIT_COUNT; i++) {
+		reg = rd32(hw, I40E_VFINT_ICR0_ENA(vf_id)) &
+			   I40E_VFINT_ICR0_ADMINQ_MASK;
+		if (reg)
+			return 0;
+
+		usleep_range(100, 200);
+	}
+
+	return -EAGAIN;
+}
+
 /**
  * i40e_trigger_vf_reset
  * @vf: pointer to the VF structure
@@ -1393,9 +1419,11 @@ static void i40e_trigger_vf_reset(struct i40e_vf *vf, bool flr)
 	struct i40e_pf *pf = vf->pf;
 	struct i40e_hw *hw = &pf->hw;
 	u32 reg, reg_idx, bit_idx;
+	bool vf_active;
+	u32 radq;
 
 	/* warn the VF */
-	clear_bit(I40E_VF_STATE_ACTIVE, &vf->vf_states);
+	vf_active = test_and_clear_bit(I40E_VF_STATE_ACTIVE, &vf->vf_states);
 
 	/* Disable VF's configuration API during reset. The flag is re-enabled
 	 * in i40e_alloc_vf_res(), when it's safe again to access VF's VSI.
@@ -1409,7 +1437,19 @@ static void i40e_trigger_vf_reset(struct i40e_vf *vf, bool flr)
 	 * just need to clean up, so don't hit the VFRTRIG register.
 	 */
 	if (!flr) {
-		/* reset VF using VPGEN_VFRTRIG reg */
+		/* Sync VFR reset before trigger next one */
+		radq = rd32(hw, I40E_VFINT_ICR0_ENA(vf->vf_id)) &
+			    I40E_VFINT_ICR0_ADMINQ_MASK;
+		if (vf_active && !radq)
+			/* waiting for finish reset by virtual driver */
+			if (i40e_sync_vfr_reset(hw, vf->vf_id))
+				dev_info(&pf->pdev->dev,
+					 "Reset VF %d never finished\n",
+				vf->vf_id);
+
+		/* Reset VF using VPGEN_VFRTRIG reg. It is also setting
+		 * in progress state in rstat1 register.
+		 */
 		reg = rd32(hw, I40E_VPGEN_VFRTRIG(vf->vf_id));
 		reg |= I40E_VPGEN_VFRTRIG_VFSWR_MASK;
 		wr32(hw, I40E_VPGEN_VFRTRIG(vf->vf_id), reg);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
index 6aa35c8c9091..8135bd6a1c0a 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
@@ -19,6 +19,7 @@
 #define I40E_MAX_VF_PROMISC_FLAGS	3
 
 #define I40E_VF_STATE_WAIT_COUNT	20
+#define I40E_VFR_WAIT_COUNT		100
 #define I40E_VF_RESET_TIME_MIN		30000000	/* time in nsec */
 
 /* Various queue ctrls */
-- 
2.27.0


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

end of thread, other threads:[~2021-11-30 21:42 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-07 11:42 [Intel-wired-lan] [PATCH net v1] i40e: Fix for failed to init adminq while VF reset Mateusz Palczewski
2020-12-07 13:32 ` Paul Menzel
2020-12-07 19:18 ` kernel test robot
2020-12-07 19:18   ` kernel test robot
2021-11-30  7:32 Sornek, Karen
2021-11-30 21:42 ` Nguyen, Anthony L

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.