RE: [net-next 08/15] i40e: acknowledge VFLR when disabling SR-IOV

-----Original Message-----
From: Sergei Shtylyov [mailto:sergei.shtylyov@cogentembedded.com]
Sent: Monday, January 06, 2014 8:43 AM
To: Kirsher, Jeffrey T; Williams, Mitch A
Cc: davem@davemloft.net; netdev@vger.kernel.org; gospo@redhat.com;
sassmann@redhat.com; Brandeburg, Jesse
Subject: Re: [net-next 08/15] i40e: acknowledge VFLR when disabling SR-IOV

On 06.01.2014 16:30, Jeff Kirsher wrote:

quoted

From: Mitch Williams <redacted>

quoted

When SR-IOV is disabled, the (now nonexistent) virtual function
devices undergo a VFLR event. We don't need to handle this event
because the VFs are gone, but we do need to tell the HW that they are
complete. This fixes an issue with a phantom VFLR and broken VFs when
SR-IOV is re-enabled.

quoted

Change-Id: I7580b49ded0158172a85b14661ec212af77000c8
Signed-off-by: Mitch Williams <redacted>
Signed-off-by: Jesse Brandeburg <redacted>
Tested-by: Sibai Li <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
  drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 15 +++++++++++++--
  1 file changed, 13 insertions(+), 2 deletions(-)

quoted

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c

b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c

quoted

index f92404c..e91f9d7 100644

--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c

[...]

quoted

@@ -748,8 +750,17 @@ void i40e_free_vfs(struct i40e_pf *pf)
  	kfree(pf->vf);
  	pf->vf = NULL;

-	if (!i40e_vfs_are_assigned(pf))
+	if (!i40e_vfs_are_assigned(pf)) {
  		pci_disable_sriov(pf->pdev);
+		/* Acknowledge VFLR for all VFS. Without this, VFs will fail to
+		 * work correctly when SR-IOV gets re-enabled.
+		 */
+		for (vf_id = 0; vf_id < tmp; vf_id++) {
+			reg_idx = (hw->func_caps.vf_base_id + vf_id) / 32;
+			bit_idx = (hw->func_caps.vf_base_id + vf_id) % 32;
+			wr32(hw, I40E_GLGEN_VFLRSTAT(reg_idx), (1 << bit_idx));
+		}
+	}
  	else

    } and *else* should be on the same line. And the *else* arm should also
have {} now.

quoted

  		dev_warn(&pf->pdev->dev,
  			 "unable to disable SR-IOV because VFs are assigned.\n");

WBR, Sergei

You are absolutely correct, Sergei, and I apologize for not seeing this before I submitted the patch.

There will be a patch coming from Greg Rose in the next few weeks that will fix this problem in the process of adding a bug fix. Since this is just cosmetic, would it be all right with you if we just wait for Greg's patch to come through?

-Mitch