Netdev List
 help / color / mirror / Atom feed
From: Manjunath Patil <manjunath.b.patil@oracle.com>
To: saeedm@nvidia.com
Cc: leonro@nvidia.com, tariqt@nvidia.com, mbloch@nvidia.com,
	netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
	Manjunath Patil <manjunath.b.patil@oracle.com>
Subject: [PATCH net] net/mlx5: retain command mailboxes after timeout
Date: Fri,  2 Oct 2026 20:56:49 +0000	[thread overview]
Message-ID: <20261002205649.2029588-1-manjunath.b.patil@oracle.com> (raw)

Firmware can access command DMA mailboxes after the driver times out.
The command entry already keeps a reference for a possible firmware
completion, which keeps its slot allocated, but the blocking caller
and async callback can still free their input and output mailboxes on
timeout. A late firmware access can therefore hit reallocated DMA
pool memory.

Give the entry ownership of the mailboxes on timeout and free them when
its final reference is dropped. If timeout claims PENDING_COMP first,
retain the firmware-event reference while a real completion remains
possible; a late completion drops it.

The real EQ handler may instead claim PENDING_COMP before the blocking
timeout handler runs and still be reading the mailboxes. Set
RETAIN_MSGS before forcing timeout completion in the blocking path.
The EQ handler holds its entry reference through its last mailbox
access and done notification, so the caller can return on timeout
without freeing memory beneath it.

Serialize completion claims and firmware-reference handoffs under
alloc_lock. The existing reset/error flush marks synthetic
completions with MLX5_TRIGGERED_CMD_COMP and treats them as terminal
for firmware ownership; only such a triggered completion drops a
reference retained by an earlier timeout. Normal command EQ teardown
and its slot-drain policy are unchanged.

Fixes: e126ba97dba9 ("mlx5: Add driver for Mellanox Connect-IB adapters")
Assisted-by: Codex:gpt-5
Signed-off-by: Manjunath Patil <manjunath.b.patil@oracle.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 127 ++++++++++++++----
 include/linux/mlx5/driver.h                   |   1 +
 2 files changed, 103 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
index 84583dc5eb1c..22508b26972d 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
@@ -142,8 +142,19 @@ cmd_alloc_ent(struct mlx5_cmd *cmd, struct mlx5_cmd_msg *in,
 	return ent;
 }
 
+static void free_msg(struct mlx5_core_dev *dev, struct mlx5_cmd_msg *msg);
+static void mlx5_free_cmd_msg(struct mlx5_core_dev *dev,
+			      struct mlx5_cmd_msg *msg);
+
 static void cmd_free_ent(struct mlx5_cmd_work_ent *ent)
 {
+	if (test_bit(MLX5_CMD_ENT_STATE_RETAIN_MSGS, &ent->state)) {
+		struct mlx5_core_dev *dev = container_of(ent->cmd,
+							  struct mlx5_core_dev, cmd);
+
+		mlx5_free_cmd_msg(dev, ent->out);
+		free_msg(dev, ent->in);
+	}
 	kfree(ent);
 }
 
@@ -958,10 +969,6 @@ static void cb_timeout_handler(struct work_struct *work)
 	cmd_ent_put(ent); /* for the cmd_ent_get() took on schedule delayed work */
 }
 
-static void free_msg(struct mlx5_core_dev *dev, struct mlx5_cmd_msg *msg);
-static void mlx5_free_cmd_msg(struct mlx5_core_dev *dev,
-			      struct mlx5_cmd_msg *msg);
-
 static bool opcode_allowed(struct mlx5_cmd *cmd, u16 opcode)
 {
 	if (cmd->allowed_opcode == CMD_ALLOWED_OPCODE_ALL)
@@ -1163,6 +1170,10 @@ static void wait_func_handle_exec_timeout(struct mlx5_core_dev *dev,
 		       mlx5_command_str(ent->op), ent->op);
 
 	ent->ret = -ETIMEDOUT;
+	/* The real handler may have claimed the completion but still be using
+	 * the mailboxes. Keep them with the entry until its last reference.
+	 */
+	set_bit(MLX5_CMD_ENT_STATE_RETAIN_MSGS, &ent->state);
 	mlx5_cmd_comp_handler(dev, 1ULL << ent->idx, true);
 }
 
@@ -1260,7 +1271,7 @@ static int mlx5_cmd_invoke(struct mlx5_core_dev *dev, struct mlx5_cmd_msg *in,
 			   struct mlx5_cmd_msg *out, void *uout, int uout_size,
 			   mlx5_cmd_cbk_t callback,
 			   void *context, int page_queue,
-			   u8 token, bool force_polling)
+			   u8 token, bool force_polling, bool *retain_msgs)
 {
 	struct mlx5_cmd *cmd = &dev->cmd;
 	struct mlx5_cmd_work_ent *ent;
@@ -1313,6 +1324,7 @@ static int mlx5_cmd_invoke(struct mlx5_core_dev *dev, struct mlx5_cmd_msg *in,
 		return 0; /* mlx5_cmd_comp_handler() will put(ent) */
 
 	err = wait_func(dev, ent);
+	*retain_msgs = test_bit(MLX5_CMD_ENT_STATE_RETAIN_MSGS, &ent->state);
 	if (err == -ETIMEDOUT || err == -ECANCELED || err == -EBUSY)
 		goto out_free;
 
@@ -1732,6 +1744,66 @@ static void free_msg(struct mlx5_core_dev *dev, struct mlx5_cmd_msg *msg)
 	}
 }
 
+/*
+ * cmd_work_handler() takes an entry reference for the firmware event and
+ * sets PENDING_COMP before ringing the command doorbell. Firmware can still
+ * use the input and output DMA mailboxes after the caller times out.
+ *
+ * Claim PENDING_COMP under alloc_lock so exactly one handler makes the
+ * mailbox and firmware-reference decisions:
+ *
+ * - A timeout that wins retains the mailboxes in the entry. If the command
+ *   interface is still up and the opcode is allowed, firmware can still
+ *   generate a real completion, so TIMEDOUT retains the firmware-event ref.
+ * - A real completion that wins consumes its firmware-event ref normally.
+ * - A real completion that loses is the late event after a timeout. It drops
+ *   the ref retained by TIMEDOUT; RETAIN_MSGS stays set until entry teardown.
+ * - A reset completion cannot be followed by a real firmware event. It drops
+ *   a ref only when TIMEDOUT says the timeout retained one.
+ *
+ * A blocking timeout retains the mailboxes even when a real completion has
+ * claimed PENDING_COMP. That handler keeps its firmware-event reference until
+ * it finishes using the mailboxes. Keeping the reference transitions under
+ * alloc_lock prevents timeout, firmware, and reset from consuming the same
+ * reference.
+ */
+static bool mlx5_cmd_claim_completion(struct mlx5_core_dev *dev,
+				      struct mlx5_cmd_work_ent *ent, u64 vec,
+				      bool forced, bool *drop_fw_ref)
+{
+	struct mlx5_cmd *cmd = &dev->cmd;
+	unsigned long flags;
+	bool timed_out = forced && ent->ret == -ETIMEDOUT;
+	bool pending;
+
+	*drop_fw_ref = false;
+	spin_lock_irqsave(&cmd->alloc_lock, flags);
+	pending = test_and_clear_bit(MLX5_CMD_ENT_STATE_PENDING_COMP,
+				     &ent->state);
+	if (pending) {
+		if (timed_out)
+			set_bit(MLX5_CMD_ENT_STATE_RETAIN_MSGS, &ent->state);
+
+		if (timed_out && !mlx5_cmd_is_down(dev) &&
+		    opcode_allowed(cmd, ent->op)) {
+			set_bit(MLX5_CMD_ENT_STATE_TIMEDOUT, &ent->state);
+		} else {
+			clear_bit(MLX5_CMD_ENT_STATE_TIMEDOUT, &ent->state);
+			*drop_fw_ref = true;
+		}
+	} else if (!forced) {
+		clear_bit(MLX5_CMD_ENT_STATE_TIMEDOUT, &ent->state);
+		*drop_fw_ref = true;
+	} else if (vec & MLX5_TRIGGERED_CMD_COMP) {
+		/* Reset cannot receive a late firmware completion. */
+		*drop_fw_ref = test_and_clear_bit(MLX5_CMD_ENT_STATE_TIMEDOUT,
+						  &ent->state);
+	}
+	spin_unlock_irqrestore(&cmd->alloc_lock, flags);
+
+	return pending;
+}
+
 static void mlx5_cmd_comp_handler(struct mlx5_core_dev *dev, u64 vec, bool forced)
 {
 	struct mlx5_cmd *cmd = &dev->cmd;
@@ -1744,38 +1816,33 @@ static void mlx5_cmd_comp_handler(struct mlx5_core_dev *dev, u64 vec, bool force
 	struct mlx5_cmd_stats *stats;
 	unsigned long flags;
 	unsigned long vector;
+	bool pending;
+	bool drop_fw_ref;
 
 	/* there can be at most 32 command queues */
 	vector = vec & 0xffffffff;
 	for (i = 0; i < (1 << cmd->vars.log_sz); i++) {
 		if (test_bit(i, &vector)) {
 			ent = cmd->ent_arr[i];
-
-			if (forced && ent->ret == -ETIMEDOUT)
-				set_bit(MLX5_CMD_ENT_STATE_TIMEDOUT,
-					&ent->state);
-			else if (!forced) /* real FW completion */
-				clear_bit(MLX5_CMD_ENT_STATE_TIMEDOUT,
-					  &ent->state);
+			pending = mlx5_cmd_claim_completion(dev, ent, vec, forced,
+							    &drop_fw_ref);
 
 			/* if we already completed the command, ignore it */
-			if (!test_and_clear_bit(MLX5_CMD_ENT_STATE_PENDING_COMP,
-						&ent->state)) {
-				/* only real completion can free the cmd slot */
+			if (!pending) {
 				if (!forced) {
-					mlx5_core_err(dev, "Command completion arrived after timeout (entry idx = %d).\n",
+					mlx5_core_err(dev,
+						      "Command completion arrived after timeout (entry idx = %d).\n",
 						      ent->idx);
-					cmd_ent_put(ent);
 				}
+				if (drop_fw_ref)
+					cmd_ent_put(ent);
 				continue;
 			}
 
 			if (ent->callback && cancel_delayed_work(&ent->cb_timeout_work))
 				cmd_ent_put(ent); /* timeout work was canceled */
 
-			if (!forced || /* Real FW completion */
-			     mlx5_cmd_is_down(dev) || /* No real FW completion is expected */
-			     !opcode_allowed(cmd, ent->op))
+			if (drop_fw_ref && ent->callback)
 				cmd_ent_put(ent);
 
 			ent->ts2 = ktime_get_ns();
@@ -1816,17 +1883,23 @@ static void mlx5_cmd_comp_handler(struct mlx5_core_dev *dev, u64 vec, bool force
 								 ent->out,
 								 ent->uout_size);
 
-				mlx5_free_cmd_msg(dev, ent->out);
-				free_msg(dev, ent->in);
+				if (!test_bit(MLX5_CMD_ENT_STATE_RETAIN_MSGS,
+					      &ent->state)) {
+					mlx5_free_cmd_msg(dev, ent->out);
+					free_msg(dev, ent->in);
+				}
 
 				/* final consumer is done, release ent */
 				cmd_ent_put(ent);
 				callback(err, context);
 			} else {
-				/* release wait_func() so mlx5_cmd_invoke()
-				 * can make the final ent_put()
+				/* No mailbox accesses after done. If the caller timed out,
+				 * the firmware reference keeps ent and its mailboxes alive
+				 * until this handler has finished.
 				 */
 				complete(&ent->done);
+				if (drop_fw_ref)
+					cmd_ent_put(ent);
 			}
 		}
 	}
@@ -1965,6 +2038,7 @@ static int cmd_exec(struct mlx5_core_dev *dev, void *in, int in_size, void *out,
 	gfp_t gfp;
 	u8 token;
 	int err;
+	bool retain_msgs = false;
 
 	if (mlx5_cmd_is_down(dev) || !opcode_allowed(&dev->cmd, opcode))
 		return -ENXIO;
@@ -2008,9 +2082,12 @@ static int cmd_exec(struct mlx5_core_dev *dev, void *in, int in_size, void *out,
 	}
 
 	err = mlx5_cmd_invoke(dev, inb, outb, out, out_size, callback, context,
-			      pages_queue, token, force_polling);
+			      pages_queue, token, force_polling, &retain_msgs);
 	if (callback && !err)
 		return 0;
+	/* The entry releases retained DMA mailboxes with its final reference. */
+	if (retain_msgs)
+		goto out_up;
 
 	if (err > 0) /* Failed in FW, command didn't execute */
 		err = deliv_status_to_err(err);
diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
index 83d0a83bbfbc..99cabe13e6c1 100644
--- a/include/linux/mlx5/driver.h
+++ b/include/linux/mlx5/driver.h
@@ -829,6 +829,7 @@ typedef void (*mlx5_cmd_cbk_t)(int status, void *context);
 enum {
 	MLX5_CMD_ENT_STATE_PENDING_COMP,
 	MLX5_CMD_ENT_STATE_TIMEDOUT,
+	MLX5_CMD_ENT_STATE_RETAIN_MSGS,
 };
 
 struct mlx5_cmd_work_ent {
-- 
2.52.0


             reply	other threads:[~2026-10-02 20:56 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-02 20:56 Manjunath Patil [this message]
2026-10-02 20:58 ` [PATCH net] net/mlx5: retain command mailboxes after timeout netdev-bot+sinfo
2026-10-02 21:33   ` manjunath.b.patil
2026-10-03 21:18 ` netdev-bot+sashiko
2026-10-05 17:23   ` manjunath.b.patil
2026-10-06 13:01 ` Moshe Shemesh
2026-10-06 19:42   ` manjunath.b.patil

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=20261002205649.2029588-1-manjunath.b.patil@oracle.com \
    --to=manjunath.b.patil@oracle.com \
    --cc=leonro@nvidia.com \
    --cc=linux-rdma@vger.kernel.org \
    --cc=mbloch@nvidia.com \
    --cc=netdev@vger.kernel.org \
    --cc=saeedm@nvidia.com \
    --cc=tariqt@nvidia.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox