All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tony Nguyen <anthony.l.nguyen@intel.com>
To: intel-wired-lan@osuosl.org
Subject: [Intel-wired-lan] [PATCH S40 05/15] ice: Fix removing driver while bare-metal VFs pass traffic
Date: Thu, 27 Feb 2020 10:14:55 -0800	[thread overview]
Message-ID: <20200227181505.61720-5-anthony.l.nguyen@intel.com> (raw)
In-Reply-To: <20200227181505.61720-1-anthony.l.nguyen@intel.com>

From: Brett Creeley <brett.creeley@intel.com>

Currently, if there are bare-metal VFs passing traffic and the ice
driver is removed, there is a possibility of VFs triggering a Tx timeout
right before iavf_remove(). This is causing iavf_close() to not be
called because there is a check in the beginning of iavf_remove() that
bails out early if (adapter->state < IAVF_DOWN_PENDING). This makes it
so some resources do not get cleaned up. Specifically, free_irq()
is never called for data interrupts, which results in the following line
of code to trigger:

pci_disable_msix()
	free_msi_irqs()
		...
		BUG_ON(irq_has_action(entry->irq + i));
		...

To prevent the Tx timeout from occurring on the VF during driver unload
for ice and the iavf there are a few changes that are needed.

[1] Don't disable all active VF Tx/Rx queues prior to calling
pci_disable_sriov.

[2] Call ice_free_vfs() before disabling the service task.

[3] Disable VF resets when the ice driver is being unloaded by setting
the pf->state flag __ICE_VF_RESETS_DISABLED.

Changing [1] and [2] allow each VF driver's remove flow to successfully
send VIRTCHNL requests, which includes queue disable. This prevents
unexpected Tx timeouts because the PF driver is no longer forcefully
disabling queues.

Due to [1] and [2] there is a possibility that the PF driver will get a
VFLR or reset request over VIRTCHNL from a VF during PF driver unload.
Prevent that by doing [3].

Signed-off-by: Brett Creeley <brett.creeley@intel.com>
---
 drivers/net/ethernet/intel/ice/ice.h          |  1 +
 drivers/net/ethernet/intel/ice/ice_main.c     | 19 +++++++++++++++----
 .../net/ethernet/intel/ice/ice_virtchnl_pf.c  | 19 +++++++++++++------
 3 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index bdd6d8a732f5..b32d9ed35b21 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -220,6 +220,7 @@ enum ice_state {
 	__ICE_SERVICE_DIS,
 	__ICE_OICR_INTR_DIS,		/* Global OICR interrupt disabled */
 	__ICE_MDD_VF_PRINT_PENDING,	/* set when MDD event handle */
+	__ICE_VF_RESETS_DISABLED,	/* disable resets during ice_remove */
 	__ICE_STATE_NBITS		/* must be last */
 };
 
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 933f41455628..13ac6073a75d 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -2060,8 +2060,16 @@ static irqreturn_t ice_misc_intr(int __always_unused irq, void *data)
 		set_bit(__ICE_MDD_EVENT_PENDING, pf->state);
 	}
 	if (oicr & PFINT_OICR_VFLR_M) {
-		ena_mask &= ~PFINT_OICR_VFLR_M;
-		set_bit(__ICE_VFLR_EVENT_PENDING, pf->state);
+		/* disable any further VFLR event notifications */
+		if (test_bit(__ICE_VF_RESETS_DISABLED, pf->state)) {
+			u32 reg = rd32(hw, PFINT_OICR_ENA);
+
+			reg &= ~PFINT_OICR_VFLR_M;
+			wr32(hw, PFINT_OICR_ENA, reg);
+		} else {
+			ena_mask &= ~PFINT_OICR_VFLR_M;
+			set_bit(__ICE_VFLR_EVENT_PENDING, pf->state);
+		}
 	}
 
 	if (oicr & PFINT_OICR_GRST_M) {
@@ -3457,6 +3465,11 @@ static void ice_remove(struct pci_dev *pdev)
 		msleep(100);
 	}
 
+	if (test_bit(ICE_FLAG_SRIOV_ENA, pf->flags)) {
+		set_bit(__ICE_VF_RESETS_DISABLED, pf->state);
+		ice_free_vfs(pf);
+	}
+
 	ice_service_task_stop(pf);
 	if (ice_is_peer_ena(pf)) {
 		reason = IIDC_REASON_INTERFACE_DOWN;
@@ -3464,8 +3477,6 @@ static void ice_remove(struct pci_dev *pdev)
 	}
 	set_bit(__ICE_DOWN, pf->state);
 
-	if (test_bit(ICE_FLAG_SRIOV_ENA, pf->flags))
-		ice_free_vfs(pf);
 	ice_vsi_release_all(pf);
 	if (ice_is_peer_ena(pf)) {
 		ice_for_each_peer(pf, NULL, ice_unreg_peer_device);
diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c b/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c
index 48791fc387ba..5fb457479060 100644
--- a/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c
@@ -300,11 +300,6 @@ void ice_free_vfs(struct ice_pf *pf)
 	while (test_and_set_bit(__ICE_VF_DIS, pf->state))
 		usleep_range(1000, 2000);
 
-	/* Avoid wait time by stopping all VFs at the same time */
-	ice_for_each_vf(pf, i)
-		if (test_bit(ICE_VF_STATE_QS_ENA, pf->vf[i].vf_states))
-			ice_dis_vf_qs(&pf->vf[i]);
-
 	/* Disable IOV before freeing resources. This lets any VF drivers
 	 * running in the host get themselves cleaned up before we yank
 	 * the carpet out from underneath their feet.
@@ -314,6 +309,11 @@ void ice_free_vfs(struct ice_pf *pf)
 	else
 		dev_warn(dev, "VFs are assigned - not disabling SR-IOV\n");
 
+	/* Avoid wait time by stopping all VFs at the same time */
+	ice_for_each_vf(pf, i)
+		if (test_bit(ICE_VF_STATE_QS_ENA, pf->vf[i].vf_states))
+			ice_dis_vf_qs(&pf->vf[i]);
+
 	tmp = pf->num_alloc_vfs;
 	pf->num_qps_per_vf = 0;
 	pf->num_alloc_vfs = 0;
@@ -1155,7 +1155,8 @@ static bool ice_is_vf_disabled(struct ice_vf *vf)
  * @vf: pointer to the VF structure
  * @is_vflr: true if VFLR was issued, false if not
  *
- * Returns true if the VF is reset, false otherwise.
+ * Returns true if the VF is currently in reset, resets successfully, or resets
+ * are disabled and false otherwise.
  */
 bool ice_reset_vf(struct ice_vf *vf, bool is_vflr)
 {
@@ -1170,6 +1171,12 @@ bool ice_reset_vf(struct ice_vf *vf, bool is_vflr)
 
 	dev = ice_pf_to_dev(pf);
 
+	if (test_bit(__ICE_VF_RESETS_DISABLED, pf->state)) {
+		dev_dbg(dev, "Trying to reset VF %d, but all VF resets are disabled\n",
+			vf->vf_id);
+		return true;
+	}
+
 	if (ice_is_vf_disabled(vf)) {
 		dev_dbg(dev, "VF is already disabled, there is no need for resetting it, telling VM, all is fine %d\n",
 			vf->vf_id);
-- 
2.20.1


  parent reply	other threads:[~2020-02-27 18:14 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-27 18:14 [Intel-wired-lan] [PATCH S40 01/15] iavf: Enable support for up to 16 queues Tony Nguyen
2020-02-27 18:14 ` [Intel-wired-lan] [PATCH S40 02/15] ice: allow bigger VFs Tony Nguyen
2020-03-03 19:04   ` Bowers, AndrewX
2020-02-27 18:14 ` [Intel-wired-lan] [PATCH S40 03/15] ice: Improve clarity of prints and variables Tony Nguyen
2020-03-03 19:04   ` Bowers, AndrewX
2020-02-27 18:14 ` [Intel-wired-lan] [PATCH S40 04/15] ice: Add support for tunnel offloads Tony Nguyen
2020-03-03 19:05   ` Bowers, AndrewX
2020-02-27 18:14 ` Tony Nguyen [this message]
2020-03-03 19:05   ` [Intel-wired-lan] [PATCH S40 05/15] ice: Fix removing driver while bare-metal VFs pass traffic Bowers, AndrewX
2020-02-27 18:14 ` [Intel-wired-lan] [PATCH S40 06/15] ice: Display Link detected via Ethtool in safe mode Tony Nguyen
2020-03-03 19:06   ` Bowers, AndrewX
2020-02-27 18:14 ` [Intel-wired-lan] [PATCH S40 07/15] ice: Fix corner case when switching from IEEE to CEE Tony Nguyen
2020-03-03 19:06   ` Bowers, AndrewX
2020-02-27 18:14 ` [Intel-wired-lan] [PATCH S40 08/15] ice: renegotiate link after FW DCB on Tony Nguyen
2020-03-03 19:07   ` Bowers, AndrewX
2020-02-27 18:14 ` [Intel-wired-lan] [PATCH S40 09/15] ice: Correct setting VLAN pruning Tony Nguyen
2020-03-03 19:07   ` Bowers, AndrewX
2020-02-27 18:15 ` [Intel-wired-lan] [PATCH S40 10/15] ice: Increase mailbox receive queue length to maximum Tony Nguyen
2020-03-03 19:07   ` Bowers, AndrewX
2020-02-27 18:15 ` [Intel-wired-lan] [PATCH S40 11/15] ice: fix use of deprecated strlcpy() Tony Nguyen
2020-03-03 19:08   ` Bowers, AndrewX
2020-02-27 18:15 ` [Intel-wired-lan] [PATCH S40 12/15] ice: Fix format specifier Tony Nguyen
2020-03-03 19:09   ` Bowers, AndrewX
2020-02-27 18:15 ` [Intel-wired-lan] [PATCH S40 13/15] ice: Use EOPNOTSUPP instead of ENOTSUPP Tony Nguyen
2020-03-03 19:09   ` Bowers, AndrewX
2020-02-27 18:15 ` [Intel-wired-lan] [PATCH S40 14/15] ice: use variable name more descriptive than type Tony Nguyen
2020-03-03 19:10   ` Bowers, AndrewX
2020-02-27 18:15 ` [Intel-wired-lan] [PATCH S40 15/15] ice: fix incorrect size description of ice_get_nvm_version Tony Nguyen
2020-03-03 19:10   ` Bowers, AndrewX
2020-03-03 19:11 ` [Intel-wired-lan] [PATCH S40 01/15] iavf: Enable support for up to 16 queues Bowers, AndrewX

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200227181505.61720-5-anthony.l.nguyen@intel.com \
    --to=anthony.l.nguyen@intel.com \
    --cc=intel-wired-lan@osuosl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.