enic: match mailbox replies to request numbers

The version-1 VF mailbox protocol identifies every message with a message
number, and a reply or acknowledgment echoes the number of the message it
answers.  ENIC instead generates a new number for outgoing replies and
accepts a VF reply by message type alone.

If a request times out, a delayed reply can therefore satisfy a subsequent
request of the same type and cause the VF to consume the result of the old
request.

Allow replies to reuse the initiating message number. Make the in-tree PF
handlers and the VF link-state acknowledgment echo that number. Record the
expected reply type and message number on the VF, and require both values
to match before accepting a reply.

Protect expected-reply state with a lock so reply acceptance and timeout
invalidation cannot race. Keep message numbers monotonic across an admin-
channel reopen so a delayed reply from an earlier channel generation cannot
match a new request.

Reply-number echo is part of the established version-1 protocol, so this
remains compatible with deployed V2-capable PF implementations that already
echo msg_num.

Fixes: 72b65c94058e ("enic: add MBOX VF handlers for capability, register and link state")
Signed-off-by: Satish Kharat <satishkh@cisco.com>
Link: https://patch.msgid.link/20260830-b4-enic-v2-mbox-fixes-net-v1-2-23adf9bfd426@cisco.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

3 files changed, 174 insertions, 124 deletions

diff --git a/drivers/net/ethernet/cisco/enic/enic.h b/drivers/net/ethernet/cisco/enic/enic.h
index 2822fbfb6474..7a509a056990 100644
--- a/drivers/net/ethernet/cisco/enic/enic.h
+++ b/drivers/net/ethernet/cisco/enic/enic.h
@@ -329,15 +329,14 @@ struct enic {
 	/* MBOX protocol state — mbox_lock serializes admin WQ sends */
 	struct mutex mbox_lock;
 	u64 mbox_msg_num;
-	/* MBOX request-reply state.  mbox_expected_reply is written and
-	 * cleared by the process-context request helpers (capability/register/
-	 * unregister) and only read by the admin_msg_work receive handlers, so
-	 * it is annotated with READ_ONCE()/WRITE_ONCE() rather than locked:
-	 * only one request is in flight at a time (requesters run under RTNL or
-	 * single-threaded probe/remove), so each request is serialized and its
-	 * reply completes mbox_comp before the next request is issued.
+	/* MBOX request-reply state.  Existing request callers allow only one
+	 * request in flight.  The state lock arbitrates reply acceptance against
+	 * timeout invalidation, while mbox_comp publishes the accepted result to
+	 * the requester.
 	 */
 	struct completion mbox_comp;
+	spinlock_t mbox_state_lock;	/* protects expected reply state */
+	u64 mbox_expected_msg_num;
 	u8 mbox_expected_reply;
 	bool mbox_initialized;
 
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index 48d16ef18c49..14d1637a4342 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -2206,9 +2206,10 @@ static void enic_admin_chan_reopen(struct enic *enic)
 {
 	int err;
 
-	/* Install the MBOX receive handler and reset the sequence number
-	 * before opening the channel, so the handler is in place before the
-	 * admin interrupt is unmasked and no early completion is dropped.
+	/* Install the MBOX receive handler and clear pending reply state before
+	 * opening the channel, so the handler is in place before the admin
+	 * interrupt is unmasked and no early completion is dropped.  Keep the
+	 * sequence number monotonic across channel generations.
 	 */
 	enic_mbox_init(enic);
 
@@ -2220,7 +2221,7 @@ static void enic_admin_chan_reopen(struct enic *enic)
 	 * registration over a dead channel.
 	 */
 	if (enic_is_sriov_vf_v2(enic))
-		enic->vf_registered = false;
+		WRITE_ONCE(enic->vf_registered, false);
 
 	err = enic_admin_channel_open(enic);
 	if (err) {
@@ -3349,7 +3350,7 @@ static int enic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 err_out_admin_close:
 	if (enic_is_sriov_vf_v2(enic)) {
-		if (enic->vf_registered) {
+		if (READ_ONCE(enic->vf_registered)) {
 			int unreg_err = enic_mbox_vf_unregister(enic);
 
 			if (unreg_err)
@@ -3402,7 +3403,7 @@ static void enic_remove(struct pci_dev *pdev)
 		 * touching a netdev that is being torn down.
 		 */
 		if (enic_is_sriov_vf_v2(enic)) {
-			if (enic->vf_registered) {
+			if (READ_ONCE(enic->vf_registered)) {
 				int unreg_err = enic_mbox_vf_unregister(enic);
 
 				if (unreg_err)
diff --git a/drivers/net/ethernet/cisco/enic/enic_mbox.c b/drivers/net/ethernet/cisco/enic/enic_mbox.c
index ad79d3951f3d..5c93ca49552a 100644
--- a/drivers/net/ethernet/cisco/enic/enic_mbox.c
+++ b/drivers/net/ethernet/cisco/enic/enic_mbox.c
@@ -18,22 +18,25 @@
 #define ENIC_MBOX_POLL_TIMEOUT_US	5000000
 #define ENIC_MBOX_POLL_INTERVAL_US	100
 
-static void enic_mbox_fill_hdr(struct enic *enic, struct enic_mbox_hdr *hdr,
-			       u8 msg_type, u16 dst_vnic_id, u16 msg_len)
+static void enic_mbox_fill_hdr(struct enic_mbox_hdr *hdr, u8 msg_type,
+			       u16 dst_vnic_id, u16 msg_len, u64 msg_num)
 {
 	memset(hdr, 0, sizeof(*hdr));
 	hdr->dst_vnic_id = cpu_to_le16(dst_vnic_id);
 	hdr->msg_type = msg_type;
 	hdr->msg_len = cpu_to_le16(msg_len);
-	hdr->msg_num = cpu_to_le64(++enic->mbox_msg_num);
+	hdr->msg_num = cpu_to_le64(msg_num);
 }
 
-int enic_mbox_send_msg(struct enic *enic, u8 msg_type, u16 dst_vnic_id,
-		       void *payload, u16 payload_len)
+static int enic_mbox_send_msg_id(struct enic *enic, u8 msg_type,
+				 u16 dst_vnic_id, void *payload,
+				 u16 payload_len, u64 msg_num, bool reuse_msg_num,
+				 u8 expected_reply)
 {
 	size_t total_len = sizeof(struct enic_mbox_hdr) + payload_len;
 	struct vnic_wq *wq = &enic->admin_wq;
 	struct wq_enet_desc *desc;
+	bool reply_expected = false;
 	unsigned long timeout;
 	dma_addr_t dma_addr;
 	u16 vlan_tag;
@@ -68,7 +71,21 @@ int enic_mbox_send_msg(struct enic *enic, u8 msg_type, u16 dst_vnic_id,
 		goto unlock;
 	}
 
-	enic_mbox_fill_hdr(enic, buf, msg_type, dst_vnic_id, total_len);
+	/* Replies reuse the initiating message number.  Requests and
+	 * notifications allocate a new one.
+	 */
+	if (!reuse_msg_num)
+		msg_num = ++enic->mbox_msg_num;
+	if (expected_reply) {
+		reinit_completion(&enic->mbox_comp);
+		spin_lock_bh(&enic->mbox_state_lock);
+		enic->mbox_expected_reply = expected_reply;
+		enic->mbox_expected_msg_num = msg_num;
+		spin_unlock_bh(&enic->mbox_state_lock);
+		reply_expected = true;
+	}
+
+	enic_mbox_fill_hdr(buf, msg_type, dst_vnic_id, total_len, msg_num);
 	if (payload_len) {
 		void *dst = buf + sizeof(struct enic_mbox_hdr);
 
@@ -139,18 +156,66 @@ int enic_mbox_send_msg(struct enic *enic, u8 msg_type, u16 dst_vnic_id,
 		   "MBOX send msg_type %u dst %u vlan %u err %d\n",
 		   msg_type, dst_vnic_id, vlan_tag, err);
 unlock:
+	if (err && reply_expected) {
+		spin_lock_bh(&enic->mbox_state_lock);
+		if (enic->mbox_expected_reply == expected_reply &&
+		    enic->mbox_expected_msg_num == msg_num) {
+			enic->mbox_expected_reply = 0;
+			enic->mbox_expected_msg_num = 0;
+		}
+		spin_unlock_bh(&enic->mbox_state_lock);
+	}
 	mutex_unlock(&enic->mbox_lock);
 	return err;
 }
 
+int enic_mbox_send_msg(struct enic *enic, u8 msg_type, u16 dst_vnic_id,
+		       void *payload, u16 payload_len)
+{
+	return enic_mbox_send_msg_id(enic, msg_type, dst_vnic_id, payload,
+				     payload_len, 0, false, 0);
+}
+
+static int enic_mbox_send_reply(struct enic *enic, u8 msg_type,
+				u16 dst_vnic_id, void *payload, u16 payload_len,
+				u64 msg_num)
+{
+	return enic_mbox_send_msg_id(enic, msg_type, dst_vnic_id, payload,
+				     payload_len, msg_num, true, 0);
+}
+
+static int enic_mbox_vf_send_request(struct enic *enic, u8 request_type,
+				     u8 expected_reply, void *payload,
+				     u16 payload_len)
+{
+	return enic_mbox_send_msg_id(enic, request_type, ENIC_MBOX_DST_PF,
+				     payload, payload_len, 0, false,
+				     expected_reply);
+}
+
 static int enic_mbox_wait_reply(struct enic *enic, unsigned long timeout_ms)
 {
 	unsigned long left;
+	int err = 0;
 
 	left = wait_for_completion_timeout(&enic->mbox_comp,
 					   msecs_to_jiffies(timeout_ms));
+	if (left)
+		return 0;
+
+	/* Invalidate a request that the handler has not already accepted.  A
+	 * delayed reply cannot match a later request because message numbers are
+	 * monotonic across channel reopen.
+	 */
+	spin_lock_bh(&enic->mbox_state_lock);
+	if (enic->mbox_expected_reply) {
+		enic->mbox_expected_reply = 0;
+		enic->mbox_expected_msg_num = 0;
+		err = -ETIMEDOUT;
+	}
+	spin_unlock_bh(&enic->mbox_state_lock);
 
-	return left ? 0 : -ETIMEDOUT;
+	return err;
 }
 
 int enic_mbox_send_link_state(struct enic *enic, u16 vf_id, u32 link_state)
@@ -178,8 +243,8 @@ static int enic_mbox_pf_handle_capability(struct enic *enic, void *msg,
 	reply.reply.ret_major = cpu_to_le16(0);
 	reply.version = cpu_to_le32(ENIC_MBOX_CAP_VERSION_1);
 
-	return enic_mbox_send_msg(enic, ENIC_MBOX_VF_CAPABILITY_REPLY, vf_id,
-				  &reply, sizeof(reply));
+	return enic_mbox_send_reply(enic, ENIC_MBOX_VF_CAPABILITY_REPLY, vf_id,
+				    &reply, sizeof(reply), msg_num);
 }
 
 static int enic_mbox_pf_handle_register(struct enic *enic, void *msg,
@@ -208,8 +273,8 @@ static int enic_mbox_pf_handle_register(struct enic *enic, void *msg,
 	}
 
 	reply.reply.ret_major = cpu_to_le16(0);
-	err = enic_mbox_send_msg(enic, ENIC_MBOX_VF_REGISTER_REPLY, vf_id,
-				 &reply, sizeof(reply));
+	err = enic_mbox_send_reply(enic, ENIC_MBOX_VF_REGISTER_REPLY, vf_id,
+				   &reply, sizeof(reply), msg_num);
 	if (err)
 		return err;
 
@@ -253,8 +318,8 @@ static int enic_mbox_pf_handle_unregister(struct enic *enic, void *msg,
 	enic->vf_state[vf_id].registered = false;
 
 	reply.reply.ret_major = cpu_to_le16(0);
-	err = enic_mbox_send_msg(enic, ENIC_MBOX_VF_UNREGISTER_REPLY, vf_id,
-				 &reply, sizeof(reply));
+	err = enic_mbox_send_reply(enic, ENIC_MBOX_VF_UNREGISTER_REPLY, vf_id,
+				   &reply, sizeof(reply), msg_num);
 
 	if (net_ratelimit())
 		netdev_info(enic->netdev,
@@ -324,75 +389,57 @@ static void enic_mbox_pf_process_msg(struct enic *enic,
 			    hdr->msg_type, vf_id, err);
 }
 
-static void enic_mbox_vf_handle_capability_reply(struct enic *enic,
-						 void *payload)
+static void enic_mbox_vf_handle_reply(struct enic *enic, u8 reply_type,
+				      void *payload, u64 msg_num)
 {
-	struct enic_mbox_vf_capability_reply_msg *reply = payload;
-
-	if (READ_ONCE(enic->mbox_expected_reply) != ENIC_MBOX_VF_CAPABILITY_REPLY) {
+	struct enic_mbox_generic_reply *reply = payload;
+	u16 ret_major = le16_to_cpu(reply->ret_major);
+	u64 expected_msg_num;
+	u8 expected_type;
+
+	spin_lock_bh(&enic->mbox_state_lock);
+	expected_type = enic->mbox_expected_reply;
+	expected_msg_num = enic->mbox_expected_msg_num;
+	if (expected_type != reply_type || expected_msg_num != msg_num) {
+		spin_unlock_bh(&enic->mbox_state_lock);
 		netdev_warn(enic->netdev,
-			    "MBOX: stale capability reply (expected %u), drop\n",
-			    READ_ONCE(enic->mbox_expected_reply));
+			    "MBOX: stale reply %u/%llu (expected %u/%llu), drop\n",
+			    reply_type, (unsigned long long)msg_num,
+			    expected_type, (unsigned long long)expected_msg_num);
 		return;
 	}
 
-	if (le16_to_cpu(reply->reply.ret_major) == 0)
-		enic->pf_cap_version = le32_to_cpu(reply->version);
-	else
-		netdev_warn(enic->netdev,
-			    "MBOX: PF rejected capability request: %u/%u\n",
-			    le16_to_cpu(reply->reply.ret_major),
-			    le16_to_cpu(reply->reply.ret_minor));
-	complete(&enic->mbox_comp);
-}
-
-static void enic_mbox_vf_handle_register_reply(struct enic *enic,
-					       void *payload)
-{
-	struct enic_mbox_vf_register_reply_msg *reply = payload;
-
-	if (READ_ONCE(enic->mbox_expected_reply) != ENIC_MBOX_VF_REGISTER_REPLY) {
-		netdev_warn(enic->netdev,
-			    "MBOX: stale register reply (expected %u), drop\n",
-			    READ_ONCE(enic->mbox_expected_reply));
-		return;
-	}
+	if (!ret_major) {
+		switch (reply_type) {
+		case ENIC_MBOX_VF_CAPABILITY_REPLY: {
+			struct enic_mbox_vf_capability_reply_msg *cap = payload;
 
-	if (le16_to_cpu(reply->reply.ret_major)) {
-		netdev_warn(enic->netdev,
-			    "MBOX: VF register rejected by PF: %u/%u\n",
-			    le16_to_cpu(reply->reply.ret_major),
-			    le16_to_cpu(reply->reply.ret_minor));
-	} else {
-		enic->vf_registered = true;
+			WRITE_ONCE(enic->pf_cap_version,
+				   le32_to_cpu(cap->version));
+			break;
+		}
+		case ENIC_MBOX_VF_REGISTER_REPLY:
+			WRITE_ONCE(enic->vf_registered, true);
+			break;
+		case ENIC_MBOX_VF_UNREGISTER_REPLY:
+			WRITE_ONCE(enic->vf_registered, false);
+			break;
+		}
 	}
+	enic->mbox_expected_reply = 0;
+	enic->mbox_expected_msg_num = 0;
 	complete(&enic->mbox_comp);
-}
-
-static void enic_mbox_vf_handle_unregister_reply(struct enic *enic,
-						 void *payload)
-{
-	struct enic_mbox_vf_register_reply_msg *reply = payload;
+	spin_unlock_bh(&enic->mbox_state_lock);
 
-	if (READ_ONCE(enic->mbox_expected_reply) != ENIC_MBOX_VF_UNREGISTER_REPLY) {
+	if (ret_major)
 		netdev_warn(enic->netdev,
-			    "MBOX: stale unregister reply (expected %u), drop\n",
-			    READ_ONCE(enic->mbox_expected_reply));
-		return;
-	}
-
-	if (le16_to_cpu(reply->reply.ret_major)) {
-		netdev_warn(enic->netdev,
-			    "MBOX: VF unregister rejected by PF: %u/%u\n",
-			    le16_to_cpu(reply->reply.ret_major),
-			    le16_to_cpu(reply->reply.ret_minor));
-	} else {
-		enic->vf_registered = false;
-	}
-	complete(&enic->mbox_comp);
+			    "MBOX: PF rejected reply type %u: %u/%u\n",
+			    reply_type, ret_major,
+			    le16_to_cpu(reply->ret_minor));
 }
 
-static void enic_mbox_vf_handle_link_state(struct enic *enic, void *payload)
+static void enic_mbox_vf_handle_link_state(struct enic *enic, void *payload,
+					   u64 msg_num)
 {
 	struct enic_mbox_pf_link_state_notif_msg *notif = payload;
 	struct enic_mbox_pf_link_state_ack_msg ack = {};
@@ -423,8 +470,8 @@ static void enic_mbox_vf_handle_link_state(struct enic *enic, void *payload)
 	}
 	spin_unlock_bh(&enic->vf_link_state_lock);
 
-	err = enic_mbox_send_msg(enic, ENIC_MBOX_PF_LINK_STATE_ACK,
-				 ENIC_MBOX_DST_PF, &ack, sizeof(ack));
+	err = enic_mbox_send_reply(enic, ENIC_MBOX_PF_LINK_STATE_ACK,
+				   ENIC_MBOX_DST_PF, &ack, sizeof(ack), msg_num);
 	if (err && net_ratelimit())
 		netdev_warn(enic->netdev,
 			    "MBOX: failed to send link state ACK: %d\n", err);
@@ -468,6 +515,8 @@ static void enic_mbox_vf_process_msg(struct enic *enic,
 				     struct enic_mbox_hdr *hdr, void *payload,
 				     u16 payload_len)
 {
+	u64 msg_num = le64_to_cpu(hdr->msg_num);
+
 	switch (hdr->msg_type) {
 	case ENIC_MBOX_VF_CAPABILITY_REPLY: {
 		size_t exp = sizeof(struct enic_mbox_vf_capability_reply_msg);
@@ -475,7 +524,7 @@ static void enic_mbox_vf_process_msg(struct enic *enic,
 		if (!enic_mbox_vf_payload_ok(enic, hdr->msg_type,
 					     payload_len, exp))
 			return;
-		enic_mbox_vf_handle_capability_reply(enic, payload);
+		enic_mbox_vf_handle_reply(enic, hdr->msg_type, payload, msg_num);
 		break;
 	}
 	case ENIC_MBOX_VF_REGISTER_REPLY: {
@@ -484,7 +533,7 @@ static void enic_mbox_vf_process_msg(struct enic *enic,
 		if (!enic_mbox_vf_payload_ok(enic, hdr->msg_type,
 					     payload_len, exp))
 			return;
-		enic_mbox_vf_handle_register_reply(enic, payload);
+		enic_mbox_vf_handle_reply(enic, hdr->msg_type, payload, msg_num);
 		break;
 	}
 	case ENIC_MBOX_VF_UNREGISTER_REPLY: {
@@ -493,7 +542,7 @@ static void enic_mbox_vf_process_msg(struct enic *enic,
 		if (!enic_mbox_vf_payload_ok(enic, hdr->msg_type,
 					     payload_len, exp))
 			return;
-		enic_mbox_vf_handle_unregister_reply(enic, payload);
+		enic_mbox_vf_handle_reply(enic, hdr->msg_type, payload, msg_num);
 		break;
 	}
 	case ENIC_MBOX_PF_LINK_STATE_NOTIF: {
@@ -502,7 +551,7 @@ static void enic_mbox_vf_process_msg(struct enic *enic,
 		if (!enic_mbox_vf_payload_ok(enic, hdr->msg_type,
 					     payload_len, exp))
 			return;
-		enic_mbox_vf_handle_link_state(enic, payload);
+		enic_mbox_vf_handle_link_state(enic, payload, msg_num);
 		break;
 	}
 	default:
@@ -571,32 +620,31 @@ static void enic_mbox_recv_handler(struct enic *enic, void *buf,
 int enic_mbox_vf_capability_check(struct enic *enic)
 {
 	struct enic_mbox_vf_capability_msg req = {};
+	u32 version;
 	int err;
 
-	enic->pf_cap_version = 0;
-	reinit_completion(&enic->mbox_comp);
-	WRITE_ONCE(enic->mbox_expected_reply, ENIC_MBOX_VF_CAPABILITY_REPLY);
+	WRITE_ONCE(enic->pf_cap_version, 0);
 	req.version = cpu_to_le32(ENIC_MBOX_CAP_VERSION_1);
 
-	err = enic_mbox_send_msg(enic, ENIC_MBOX_VF_CAPABILITY_REQUEST,
-				 ENIC_MBOX_DST_PF, &req, sizeof(req));
-	if (err) {
-		WRITE_ONCE(enic->mbox_expected_reply, 0);
+	err = enic_mbox_vf_send_request(enic,
+					ENIC_MBOX_VF_CAPABILITY_REQUEST,
+					ENIC_MBOX_VF_CAPABILITY_REPLY,
+					&req, sizeof(req));
+	if (err)
 		return err;
-	}
 
 	err = enic_mbox_wait_reply(enic, 3000);
-	WRITE_ONCE(enic->mbox_expected_reply, 0);
+	version = READ_ONCE(enic->pf_cap_version);
 	if (err) {
 		netdev_warn(enic->netdev,
 			    "MBOX: no capability reply from PF\n");
 		return err;
 	}
 
-	if (enic->pf_cap_version < ENIC_MBOX_CAP_VERSION_1) {
+	if (version < ENIC_MBOX_CAP_VERSION_1) {
 		netdev_warn(enic->netdev,
 			    "MBOX: PF rejected capability request or reported unsupported version %u\n",
-			    enic->pf_cap_version);
+			    version);
 		return -EOPNOTSUPP;
 	}
 
@@ -605,28 +653,25 @@ int enic_mbox_vf_capability_check(struct enic *enic)
 
 int enic_mbox_vf_register(struct enic *enic)
 {
+	bool registered;
 	int err;
 
-	enic->vf_registered = false;
-	reinit_completion(&enic->mbox_comp);
-	WRITE_ONCE(enic->mbox_expected_reply, ENIC_MBOX_VF_REGISTER_REPLY);
+	WRITE_ONCE(enic->vf_registered, false);
 
-	err = enic_mbox_send_msg(enic, ENIC_MBOX_VF_REGISTER_REQUEST,
-				 ENIC_MBOX_DST_PF, NULL, 0);
-	if (err) {
-		WRITE_ONCE(enic->mbox_expected_reply, 0);
+	err = enic_mbox_vf_send_request(enic, ENIC_MBOX_VF_REGISTER_REQUEST,
+					ENIC_MBOX_VF_REGISTER_REPLY, NULL, 0);
+	if (err)
 		return err;
-	}
 
 	err = enic_mbox_wait_reply(enic, 3000);
-	WRITE_ONCE(enic->mbox_expected_reply, 0);
+	registered = READ_ONCE(enic->vf_registered);
 	if (err) {
 		netdev_warn(enic->netdev,
 			    "MBOX: VF registration with PF timed out\n");
 		return err;
 	}
 
-	if (!enic->vf_registered)
+	if (!registered)
 		return -ENODEV;
 
 	return 0;
@@ -634,43 +679,48 @@ int enic_mbox_vf_register(struct enic *enic)
 
 int enic_mbox_vf_unregister(struct enic *enic)
 {
+	bool registered;
 	int err;
 
-	if (!enic->vf_registered)
+	if (!READ_ONCE(enic->vf_registered))
 		return 0;
 
-	reinit_completion(&enic->mbox_comp);
-	WRITE_ONCE(enic->mbox_expected_reply, ENIC_MBOX_VF_UNREGISTER_REPLY);
-
-	err = enic_mbox_send_msg(enic, ENIC_MBOX_VF_UNREGISTER_REQUEST,
-				 ENIC_MBOX_DST_PF, NULL, 0);
-	if (err) {
-		WRITE_ONCE(enic->mbox_expected_reply, 0);
+	err = enic_mbox_vf_send_request(enic,
+					ENIC_MBOX_VF_UNREGISTER_REQUEST,
+					ENIC_MBOX_VF_UNREGISTER_REPLY,
+					NULL, 0);
+	if (err)
 		return err;
-	}
 
 	err = enic_mbox_wait_reply(enic, 3000);
-	WRITE_ONCE(enic->mbox_expected_reply, 0);
+	registered = READ_ONCE(enic->vf_registered);
 	if (err)
 		return err;
-	if (enic->vf_registered)
+	if (registered)
 		return -EACCES;
 	return 0;
 }
 
 void enic_mbox_init(struct enic *enic)
 {
-	/* mbox_lock and mbox_comp must be initialized exactly once per
+	bool reinit = enic->mbox_initialized;
+
+	/* MBOX locks and mbox_comp must be initialized exactly once per
 	 * device lifetime; the PF sriov_configure path can re-enter this
 	 * on each enable cycle where these primitives are already set up.
 	 */
-	if (!enic->mbox_initialized) {
+	if (!reinit) {
 		mutex_init(&enic->mbox_lock);
 		init_completion(&enic->mbox_comp);
+		spin_lock_init(&enic->mbox_state_lock);
+		enic->mbox_msg_num = 0;
 		enic->mbox_initialized = true;
 	} else {
 		reinit_completion(&enic->mbox_comp);
 	}
-	enic->mbox_msg_num = 0;
+	spin_lock_bh(&enic->mbox_state_lock);
+	enic->mbox_expected_reply = 0;
+	enic->mbox_expected_msg_num = 0;
+	spin_unlock_bh(&enic->mbox_state_lock);
 	enic->admin_rq_handler = enic_mbox_recv_handler;
 }