Commit f37c4eac authored by Aleksandr Loktionov's avatar Aleksandr Loktionov Committed by Tony Nguyen

i40e: fix vf may be used uninitialized in this function warning

To fix the regression introduced by commit 52424f97, which causes
servers hang in very hard to reproduce conditions with resets races.
Using two sources for the information is the root cause.
In this function before the fix bumping v didn't mean bumping vf
pointer. But the code used this variables interchangeably, so stale vf
could point to different/not intended vf.

Remove redundant "v" variable and iterate via single VF pointer across
whole function instead to guarantee VF pointer validity.

Fixes: 52424f97 ("i40e: Fix VF hang when reset is triggered on another VF")
Signed-off-by: default avatarAleksandr Loktionov <aleksandr.loktionov@intel.com>
Reviewed-by: default avatarArkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Reviewed-by: default avatarPrzemek Kitszel <przemyslaw.kitszel@intel.com>
Reviewed-by: default avatarPaul Menzel <pmenzel@molgen.mpg.de>
Tested-by: default avatarRafal Romanowski <rafal.romanowski@intel.com>
Signed-off-by: default avatarTony Nguyen <anthony.l.nguyen@intel.com>
parent eb58c598
...@@ -1624,8 +1624,8 @@ bool i40e_reset_all_vfs(struct i40e_pf *pf, bool flr) ...@@ -1624,8 +1624,8 @@ bool i40e_reset_all_vfs(struct i40e_pf *pf, bool flr)
{ {
struct i40e_hw *hw = &pf->hw; struct i40e_hw *hw = &pf->hw;
struct i40e_vf *vf; struct i40e_vf *vf;
int i, v;
u32 reg; u32 reg;
int i;
/* If we don't have any VFs, then there is nothing to reset */ /* If we don't have any VFs, then there is nothing to reset */
if (!pf->num_alloc_vfs) if (!pf->num_alloc_vfs)
...@@ -1636,11 +1636,10 @@ bool i40e_reset_all_vfs(struct i40e_pf *pf, bool flr) ...@@ -1636,11 +1636,10 @@ bool i40e_reset_all_vfs(struct i40e_pf *pf, bool flr)
return false; return false;
/* Begin reset on all VFs at once */ /* Begin reset on all VFs at once */
for (v = 0; v < pf->num_alloc_vfs; v++) { for (vf = &pf->vf[0]; vf < &pf->vf[pf->num_alloc_vfs]; ++vf) {
vf = &pf->vf[v];
/* If VF is being reset no need to trigger reset again */ /* If VF is being reset no need to trigger reset again */
if (!test_bit(I40E_VF_STATE_RESETTING, &vf->vf_states)) if (!test_bit(I40E_VF_STATE_RESETTING, &vf->vf_states))
i40e_trigger_vf_reset(&pf->vf[v], flr); i40e_trigger_vf_reset(vf, flr);
} }
/* HW requires some time to make sure it can flush the FIFO for a VF /* HW requires some time to make sure it can flush the FIFO for a VF
...@@ -1649,14 +1648,13 @@ bool i40e_reset_all_vfs(struct i40e_pf *pf, bool flr) ...@@ -1649,14 +1648,13 @@ bool i40e_reset_all_vfs(struct i40e_pf *pf, bool flr)
* the VFs using a simple iterator that increments once that VF has * the VFs using a simple iterator that increments once that VF has
* finished resetting. * finished resetting.
*/ */
for (i = 0, v = 0; i < 10 && v < pf->num_alloc_vfs; i++) { for (i = 0, vf = &pf->vf[0]; i < 10 && vf < &pf->vf[pf->num_alloc_vfs]; ++i) {
usleep_range(10000, 20000); usleep_range(10000, 20000);
/* Check each VF in sequence, beginning with the VF to fail /* Check each VF in sequence, beginning with the VF to fail
* the previous check. * the previous check.
*/ */
while (v < pf->num_alloc_vfs) { while (vf < &pf->vf[pf->num_alloc_vfs]) {
vf = &pf->vf[v];
if (!test_bit(I40E_VF_STATE_RESETTING, &vf->vf_states)) { if (!test_bit(I40E_VF_STATE_RESETTING, &vf->vf_states)) {
reg = rd32(hw, I40E_VPGEN_VFRSTAT(vf->vf_id)); reg = rd32(hw, I40E_VPGEN_VFRSTAT(vf->vf_id));
if (!(reg & I40E_VPGEN_VFRSTAT_VFRD_MASK)) if (!(reg & I40E_VPGEN_VFRSTAT_VFRD_MASK))
...@@ -1666,7 +1664,7 @@ bool i40e_reset_all_vfs(struct i40e_pf *pf, bool flr) ...@@ -1666,7 +1664,7 @@ bool i40e_reset_all_vfs(struct i40e_pf *pf, bool flr)
/* If the current VF has finished resetting, move on /* If the current VF has finished resetting, move on
* to the next VF in sequence. * to the next VF in sequence.
*/ */
v++; ++vf;
} }
} }
...@@ -1676,39 +1674,39 @@ bool i40e_reset_all_vfs(struct i40e_pf *pf, bool flr) ...@@ -1676,39 +1674,39 @@ bool i40e_reset_all_vfs(struct i40e_pf *pf, bool flr)
/* Display a warning if at least one VF didn't manage to reset in /* Display a warning if at least one VF didn't manage to reset in
* time, but continue on with the operation. * time, but continue on with the operation.
*/ */
if (v < pf->num_alloc_vfs) if (vf < &pf->vf[pf->num_alloc_vfs])
dev_err(&pf->pdev->dev, "VF reset check timeout on VF %d\n", dev_err(&pf->pdev->dev, "VF reset check timeout on VF %d\n",
pf->vf[v].vf_id); vf->vf_id);
usleep_range(10000, 20000); usleep_range(10000, 20000);
/* Begin disabling all the rings associated with VFs, but do not wait /* Begin disabling all the rings associated with VFs, but do not wait
* between each VF. * between each VF.
*/ */
for (v = 0; v < pf->num_alloc_vfs; v++) { for (vf = &pf->vf[0]; vf < &pf->vf[pf->num_alloc_vfs]; ++vf) {
/* On initial reset, we don't have any queues to disable */ /* On initial reset, we don't have any queues to disable */
if (pf->vf[v].lan_vsi_idx == 0) if (vf->lan_vsi_idx == 0)
continue; continue;
/* If VF is reset in another thread just continue */ /* If VF is reset in another thread just continue */
if (test_bit(I40E_VF_STATE_RESETTING, &vf->vf_states)) if (test_bit(I40E_VF_STATE_RESETTING, &vf->vf_states))
continue; continue;
i40e_vsi_stop_rings_no_wait(pf->vsi[pf->vf[v].lan_vsi_idx]); i40e_vsi_stop_rings_no_wait(pf->vsi[vf->lan_vsi_idx]);
} }
/* Now that we've notified HW to disable all of the VF rings, wait /* Now that we've notified HW to disable all of the VF rings, wait
* until they finish. * until they finish.
*/ */
for (v = 0; v < pf->num_alloc_vfs; v++) { for (vf = &pf->vf[0]; vf < &pf->vf[pf->num_alloc_vfs]; ++vf) {
/* On initial reset, we don't have any queues to disable */ /* On initial reset, we don't have any queues to disable */
if (pf->vf[v].lan_vsi_idx == 0) if (vf->lan_vsi_idx == 0)
continue; continue;
/* If VF is reset in another thread just continue */ /* If VF is reset in another thread just continue */
if (test_bit(I40E_VF_STATE_RESETTING, &vf->vf_states)) if (test_bit(I40E_VF_STATE_RESETTING, &vf->vf_states))
continue; continue;
i40e_vsi_wait_queues_disabled(pf->vsi[pf->vf[v].lan_vsi_idx]); i40e_vsi_wait_queues_disabled(pf->vsi[vf->lan_vsi_idx]);
} }
/* Hw may need up to 50ms to finish disabling the RX queues. We /* Hw may need up to 50ms to finish disabling the RX queues. We
...@@ -1717,12 +1715,12 @@ bool i40e_reset_all_vfs(struct i40e_pf *pf, bool flr) ...@@ -1717,12 +1715,12 @@ bool i40e_reset_all_vfs(struct i40e_pf *pf, bool flr)
mdelay(50); mdelay(50);
/* Finish the reset on each VF */ /* Finish the reset on each VF */
for (v = 0; v < pf->num_alloc_vfs; v++) { for (vf = &pf->vf[0]; vf < &pf->vf[pf->num_alloc_vfs]; ++vf) {
/* If VF is reset in another thread just continue */ /* If VF is reset in another thread just continue */
if (test_bit(I40E_VF_STATE_RESETTING, &vf->vf_states)) if (test_bit(I40E_VF_STATE_RESETTING, &vf->vf_states))
continue; continue;
i40e_cleanup_reset_vf(&pf->vf[v]); i40e_cleanup_reset_vf(vf);
} }
i40e_flush(hw); i40e_flush(hw);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment