All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rasesh Mody <rasesh.mody@cavium.com>
To: dev@dpdk.org, ferruh.yigit@intel.com
Cc: Rasesh Mody <rasesh.mody@cavium.com>, Dept-EngDPDKDev@cavium.com
Subject: [PATCH 18/53] net/qede/base: avoid possible race condition
Date: Mon, 18 Sep 2017 18:29:58 -0700	[thread overview]
Message-ID: <1505784633-1171-19-git-send-email-rasesh.mody@cavium.com> (raw)
In-Reply-To: <1505784633-1171-1-git-send-email-rasesh.mody@cavium.com>

There's a possible race in multiple VF scenarios for base driver users
that use the optional APIs ecore_iov_pf_get_and_clear_pending_events,
ecore_iov_pf_add_pending_events. If the client doesn't synchronize the two
calls, it's possible for the PF to clear a VF pending message indication
without ever getting it [as 'get & clear' isn't atomic], leading to VF
timeout on the command.

The solution is to switch into a per-VF indication rather than having a
bitfield for the various VFs with pending events. As part of the solution,
the setting/clearing of the indications is done internally by base driver.
As a result, ecore_iov_pf_add_pending_events is no longer needed and
ecore_iov_pf_get_and_clear_pending_events loses the 'and_clear' from its
name as its now a proper getter.

A VF would be considered 'pending' [I.e., get_pending_events() should
have '1' for it in its bitfield] beginning with the PF's base driver
recognizing a message sent by that VF [in SP_DPC] and ending only when
that VF message is processed.

Signed-off-by: Rasesh Mody <rasesh.mody@cavium.com>
---
 drivers/net/qede/base/ecore_iov_api.h |   16 +++--------
 drivers/net/qede/base/ecore_sriov.c   |   47 ++++++++++++++++++---------------
 drivers/net/qede/base/ecore_sriov.h   |    4 ++-
 3 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/drivers/net/qede/base/ecore_iov_api.h b/drivers/net/qede/base/ecore_iov_api.h
index 4fce6b6..4ec6217 100644
--- a/drivers/net/qede/base/ecore_iov_api.h
+++ b/drivers/net/qede/base/ecore_iov_api.h
@@ -345,21 +345,13 @@ struct ecore_public_vf_info*
 			     u16 vfid, bool b_enabled_only);
 
 /**
- * @brief Set pending events bitmap for given @vfid
- *
- * @param p_hwfn
- * @param vfid
- */
-void ecore_iov_pf_add_pending_events(struct ecore_hwfn *p_hwfn, u8 vfid);
-
-/**
- * @brief Copy pending events bitmap in @events and clear
- *	  original copy of events
+ * @brief fills a bitmask of all VFs which have pending unhandled
+ *        messages.
  *
  * @param p_hwfn
  */
-void ecore_iov_pf_get_and_clear_pending_events(struct ecore_hwfn *p_hwfn,
-					       u64 *events);
+void ecore_iov_pf_get_pending_events(struct ecore_hwfn *p_hwfn,
+				     u64 *events);
 
 /**
  * @brief Copy VF's message to PF's buffer
diff --git a/drivers/net/qede/base/ecore_sriov.c b/drivers/net/qede/base/ecore_sriov.c
index b8e03d0..7ac533e 100644
--- a/drivers/net/qede/base/ecore_sriov.c
+++ b/drivers/net/qede/base/ecore_sriov.c
@@ -3755,8 +3755,7 @@ static enum _ecore_status_t ecore_iov_vf_flr_poll(struct ecore_hwfn *p_hwfn,
 		ack_vfs[vfid / 32] |= (1 << (vfid % 32));
 		p_hwfn->pf_iov_info->pending_flr[rel_vf_id / 64] &=
 		    ~(1ULL << (rel_vf_id % 64));
-		p_hwfn->pf_iov_info->pending_events[rel_vf_id / 64] &=
-		    ~(1ULL << (rel_vf_id % 64));
+		p_vf->vf_mbx.b_pending_msg = false;
 	}
 
 	return rc;
@@ -3886,12 +3885,22 @@ void ecore_iov_process_mbx_req(struct ecore_hwfn *p_hwfn,
 	mbx = &p_vf->vf_mbx;
 
 	/* ecore_iov_process_mbx_request */
-	DP_VERBOSE(p_hwfn,
-		   ECORE_MSG_IOV,
-		   "VF[%02x]: Processing mailbox message\n", p_vf->abs_vf_id);
+#ifndef CONFIG_ECORE_SW_CHANNEL
+	if (!mbx->b_pending_msg) {
+		DP_NOTICE(p_hwfn, true,
+			  "VF[%02x]: Trying to process mailbox message when none is pending\n",
+			  p_vf->abs_vf_id);
+		return;
+	}
+	mbx->b_pending_msg = false;
+#endif
 
 	mbx->first_tlv = mbx->req_virt->first_tlv;
 
+	DP_VERBOSE(p_hwfn, ECORE_MSG_IOV,
+		   "VF[%02x]: Processing mailbox message [type %04x]\n",
+		   p_vf->abs_vf_id, mbx->first_tlv.tl.type);
+
 	OSAL_IOV_VF_MSG_TYPE(p_hwfn,
 			     p_vf->relative_vf_id,
 			     mbx->first_tlv.tl.type);
@@ -4016,26 +4025,20 @@ void ecore_iov_process_mbx_req(struct ecore_hwfn *p_hwfn,
 #endif
 }
 
-void ecore_iov_pf_add_pending_events(struct ecore_hwfn *p_hwfn, u8 vfid)
+void ecore_iov_pf_get_pending_events(struct ecore_hwfn *p_hwfn,
+				     u64 *events)
 {
-	u64 add_bit = 1ULL << (vfid % 64);
+	int i;
 
-	/* TODO - add locking mechanisms [no atomics in ecore, so we can't
-	* add the lock inside the ecore_pf_iov struct].
-	*/
-	p_hwfn->pf_iov_info->pending_events[vfid / 64] |= add_bit;
-}
+	OSAL_MEM_ZERO(events, sizeof(u64) * ECORE_VF_ARRAY_LENGTH);
 
-void ecore_iov_pf_get_and_clear_pending_events(struct ecore_hwfn *p_hwfn,
-					       u64 *events)
-{
-	u64 *p_pending_events = p_hwfn->pf_iov_info->pending_events;
+	ecore_for_each_vf(p_hwfn, i) {
+		struct ecore_vf_info *p_vf;
 
-	/* TODO - Take a lock */
-	OSAL_MEMCPY(events, p_pending_events,
-		    sizeof(u64) * ECORE_VF_ARRAY_LENGTH);
-	OSAL_MEMSET(p_pending_events, 0,
-		    sizeof(u64) * ECORE_VF_ARRAY_LENGTH);
+		p_vf = &p_hwfn->pf_iov_info->vfs_array[i];
+		if (p_vf->vf_mbx.b_pending_msg)
+			events[i / 64] |= 1ULL << (i % 64);
+	}
 }
 
 static struct ecore_vf_info *
@@ -4069,6 +4072,8 @@ static enum _ecore_status_t ecore_sriov_vfpf_msg(struct ecore_hwfn *p_hwfn,
 	 */
 	p_vf->vf_mbx.pending_req = (((u64)vf_msg->hi) << 32) | vf_msg->lo;
 
+	p_vf->vf_mbx.b_pending_msg = true;
+
 	return OSAL_PF_VF_MSG(p_hwfn, p_vf->relative_vf_id);
 }
 
diff --git a/drivers/net/qede/base/ecore_sriov.h b/drivers/net/qede/base/ecore_sriov.h
index d190126..8923730 100644
--- a/drivers/net/qede/base/ecore_sriov.h
+++ b/drivers/net/qede/base/ecore_sriov.h
@@ -45,6 +45,9 @@ struct ecore_iov_vf_mbx {
 	/* Address in VF where a pending message is located */
 	dma_addr_t		pending_req;
 
+	/* Message from VF awaits handling */
+	bool			b_pending_msg;
+
 	u8 *offset;
 
 #ifdef CONFIG_ECORE_SW_CHANNEL
@@ -168,7 +171,6 @@ struct ecore_vf_info {
  */
 struct ecore_pf_iov {
 	struct ecore_vf_info	vfs_array[E4_MAX_NUM_VFS];
-	u64			pending_events[ECORE_VF_ARRAY_LENGTH];
 	u64			pending_flr[ECORE_VF_ARRAY_LENGTH];
 
 #ifndef REMOVE_DBG
-- 
1.7.10.3

  parent reply	other threads:[~2017-09-19  1:31 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-19  1:29 [PATCH 00/53] net/qede/base: update PMD to 2.6.0.1 Rasesh Mody
2017-09-19  1:29 ` [PATCH 01/53] net/qede/base: add NVM config options Rasesh Mody
2017-09-19  1:29 ` [PATCH 02/53] net/qede/base: update management FW supported features Rasesh Mody
2017-09-19  1:29 ` [PATCH 03/53] net/qede/base: use crc32 OSAL macro Rasesh Mody
2017-09-19  1:29 ` [PATCH 04/53] net/qede/base: allocate VF queues before PF Rasesh Mody
2017-09-19  1:29 ` [PATCH 05/53] net/qede/base: convert device type to enum Rasesh Mody
2017-09-19  1:29 ` [PATCH 06/53] net/qede/base: changes for VF queue zone Rasesh Mody
2017-09-19  1:29 ` [PATCH 07/53] net/qede/base: interchangeably use SB between PF and VF Rasesh Mody
2017-09-19  1:29 ` [PATCH 08/53] net/qede/base: add API to configure coalescing for VF queues Rasesh Mody
2017-09-19  1:29 ` [PATCH 09/53] net/qede/base: restrict cache line size register padding Rasesh Mody
2017-09-19  1:29 ` [PATCH 10/53] net/qede/base: fix to use a passed ptt handle Rasesh Mody
2017-09-19  1:29 ` [PATCH 11/53] net/qede/base: add a sanity check Rasesh Mody
2017-09-19  1:29 ` [PATCH 12/53] net/qede/base: add SmartAN support Rasesh Mody
2017-09-19  1:29 ` [PATCH 13/53] net/qede/base: alter driver's force load behavior Rasesh Mody
2017-09-19  1:29 ` [PATCH 14/53] net/qede/base: add mdump sub-commands Rasesh Mody
2017-09-19  1:29 ` [PATCH 15/53] net/qede/base: add EEE support Rasesh Mody
2017-09-19  1:29 ` [PATCH 16/53] net/qede/base: use passed ptt handler Rasesh Mody
2017-09-19  1:29 ` [PATCH 17/53] net/qede/base: prevent re-assertions of parity errors Rasesh Mody
2017-09-19  1:29 ` Rasesh Mody [this message]
2017-09-19  1:29 ` [PATCH 19/53] net/qede/base: revise management FW mbox access scheme Rasesh Mody
2017-09-19  1:30 ` [PATCH 20/53] net/qede/base: remove helper functions/structures Rasesh Mody
2017-09-19  1:30 ` [PATCH 21/53] net/qede/base: initialize resc lock/unlock params Rasesh Mody
2017-09-19  1:30 ` [PATCH 22/53] net/qede/base: rename MFW get/set field defines Rasesh Mody
2017-09-19  1:30 ` [PATCH 23/53] net/qede/base: allow clients to override VF MSI-X table size Rasesh Mody
2017-09-19  1:30 ` [PATCH 24/53] net/qede/base: add API to send STAG config update to FW Rasesh Mody
2017-09-19  1:30 ` [PATCH 25/53] net/qede/base: add support for doorbell overflow recovery Rasesh Mody
2017-09-19  1:30 ` [PATCH 26/53] net/qede/base: block mbox command to unresponsive MFW Rasesh Mody
2017-09-19  1:30 ` [PATCH 27/53] net/qede/base: prevent stop vport assert by malicious VF Rasesh Mody
2017-09-19  1:30 ` [PATCH 28/53] net/qede/base: remove unused parameters Rasesh Mody
2017-09-19  1:30 ` [PATCH 29/53] net/qede/base: fix macros to check chip revision/metal Rasesh Mody
2017-09-20 11:00 ` [PATCH 00/53] net/qede/base: update PMD to 2.6.0.1 Ferruh Yigit

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=1505784633-1171-19-git-send-email-rasesh.mody@cavium.com \
    --to=rasesh.mody@cavium.com \
    --cc=Dept-EngDPDKDev@cavium.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    /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.