Netdev List
 help / color / mirror / Atom feed
From: Joe Damato <joe@dama.to>
To: netdev@vger.kernel.org, Michael Chan <michael.chan@broadcom.com>,
	Pavan Chebbi <pavan.chebbi@broadcom.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Prashant Sreedharan <prashant@broadcom.com>
Cc: horms@kernel.org, linux-kernel@vger.kernel.org, Joe Damato <joe@dama.to>
Subject: [RFC net v2 2/3] bnxt_en: check HWRM response if completion never arrives
Date: Tue, 22 Sep 2026 11:24:02 -0700	[thread overview]
Message-ID: <20260922182405.1290749-3-joe@dama.to> (raw)
In-Reply-To: <20260922182405.1290749-1-joe@dama.to>

When a command is sent over a completion ring, __hwrm_send() waits for
NAPI to consume the completion and gives up if it never arrives, without
looking at the response.

If a completion is not posted within the timeout, check the response
before giving up. If resp_len is set, the sequence id matches, and the
valid byte appears then the firmware completed the command and only the
notification was lost. Fall through to the normal error_code handling in
that case.

Log the response state on both paths so there is more data when this
rare event occurs.

Fixes: 74608fc98d28 ("bnxt_en: Ring free response from close path should use completion ring")
Signed-off-by: Joe Damato <joe@dama.to>
---
 .../net/ethernet/broadcom/bnxt/bnxt_hwrm.c    | 75 ++++++++++++++-----
 1 file changed, 57 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_hwrm.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_hwrm.c
index 5bfabdca7d0e..c494abb71c51 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_hwrm.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_hwrm.c
@@ -456,6 +456,30 @@ static bool hwrm_wait_must_abort(struct bnxt *bp, u32 req_type, u32 *fw_status)
 	return *fw_status && !BNXT_FW_IS_HEALTHY(*fw_status);
 }
 
+/* Wait for the firmware to set the valid byte at the end of the response.
+ * Returns the number of usec spent waiting; a return of
+ * HWRM_VALID_BIT_DELAY_USEC or more means the byte never appeared.
+ */
+static int hwrm_wait_for_valid(u8 *valid)
+{
+	int j;
+
+	for (j = 0; j < HWRM_VALID_BIT_DELAY_USEC; ) {
+		/* make sure we read from updated DMA memory */
+		dma_rmb();
+		if (*valid)
+			break;
+		if (j < 10) {
+			udelay(1);
+			j++;
+		} else {
+			usleep_range(20, 30);
+			j += 20;
+		}
+	}
+	return j;
+}
+
 static int __hwrm_send(struct bnxt *bp, struct bnxt_hwrm_ctx *ctx)
 {
 	u32 doorbell_offset = BNXT_GRCPF_REG_CHIMP_COMM_TRIGGER;
@@ -582,12 +606,39 @@ static int __hwrm_send(struct bnxt *bp, struct bnxt_hwrm_ctx *ctx)
 		}
 
 		if (READ_ONCE(token->state) != BNXT_HWRM_COMPLETE) {
-			hwrm_err(bp, ctx, "Resp cmpl intr err msg: 0x%x\n",
-				 req_type);
-			goto exit;
+			bool completed = false;
+			u8 valid_byte = 0;
+
+			/* The completion ring entry was not delivered for
+			 * some reason. It might be possible that the command
+			 * was carried out even without a completion being
+			 * posted. Check the response before giving up and log
+			 * the state.
+			 */
+			len = le16_to_cpu(READ_ONCE(ctx->resp->resp_len));
+			if (len &&
+			    READ_ONCE(ctx->resp->seq_id) == ctx->req->seq_id) {
+				valid = (u8 *)ctx->resp + len - 1;
+				completed = hwrm_wait_for_valid(valid) <
+					    HWRM_VALID_BIT_DELAY_USEC;
+				valid_byte = *valid;
+			}
+			if (!completed) {
+				hwrm_err(bp, ctx,
+					 "Resp cmpl intr err msg: 0x%x len:%d valid:0x%x seq:0x%x/0x%x\n",
+					 req_type, len, valid_byte,
+					 le16_to_cpu(READ_ONCE(ctx->resp->seq_id)),
+					 le16_to_cpu(ctx->req->seq_id));
+				goto exit;
+			}
+			netdev_warn(bp->dev,
+				    "Resp cmpl intr not delivered, msg: 0x%x completed anyway (len:%d valid:0x%x err:0x%x)\n",
+				    req_type, len, valid_byte,
+				    le16_to_cpu(ctx->resp->error_code));
+		} else {
+			len = le16_to_cpu(READ_ONCE(ctx->resp->resp_len));
+			valid = ((u8 *)ctx->resp) + len - 1;
 		}
-		len = le16_to_cpu(READ_ONCE(ctx->resp->resp_len));
-		valid = ((u8 *)ctx->resp) + len - 1;
 	} else {
 		__le16 seen_out_of_seq = ctx->req->seq_id; /* will never see */
 		int j;
@@ -647,19 +698,7 @@ static int __hwrm_send(struct bnxt *bp, struct bnxt_hwrm_ctx *ctx)
 
 		/* Last byte of resp contains valid bit */
 		valid = ((u8 *)ctx->resp) + len - 1;
-		for (j = 0; j < HWRM_VALID_BIT_DELAY_USEC; ) {
-			/* make sure we read from updated DMA memory */
-			dma_rmb();
-			if (*valid)
-				break;
-			if (j < 10) {
-				udelay(1);
-				j++;
-			} else {
-				usleep_range(20, 30);
-				j += 20;
-			}
-		}
+		j = hwrm_wait_for_valid(valid);
 
 		if (j >= HWRM_VALID_BIT_DELAY_USEC) {
 			hwrm_err(bp, ctx, "Error (timeout: %u) msg {0x%x 0x%x} len:%d v:%d\n",
-- 
2.53.0-Meta


  parent reply	other threads:[~2026-09-22 18:24 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-22 18:24 [RFC net v2 0/3] bnxt_en: Make RING FREE more robust Joe Damato
2026-09-22 18:24 ` [RFC net v2 1/3] bnxt_en: return the RING_FREE status to callers Joe Damato
2026-09-22 18:24 ` Joe Damato [this message]
2026-09-23  4:14   ` [RFC net v2 2/3] bnxt_en: check HWRM response if completion never arrives Michael Chan
2026-09-22 18:24 ` [RFC net v2 3/3] bnxt_en: stop DMA before releasing rings the firmware did not free Joe Damato
2026-09-23  4:43   ` Michael Chan

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=20260922182405.1290749-3-joe@dama.to \
    --to=joe@dama.to \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michael.chan@broadcom.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=pavan.chebbi@broadcom.com \
    --cc=prashant@broadcom.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