All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Chan <michael.chan@broadcom.com>
To: davem@davemloft.net
Cc: netdev@vger.kernel.org, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, andrew.gospodarek@broadcom.com,
	David Wei <dw@davidwei.uk>,
	Pavan Chebbi <pavan.chebbi@broadcom.com>,
	Ajit Khaparde <ajit.khaparde@broadcom.com>
Subject: [PATCH net-next 1/7] bnxt_en: Fix and simplify bnxt_get_avail_msix() calls
Date: Tue, 30 Apr 2024 15:44:32 -0700	[thread overview]
Message-ID: <20240430224438.91494-2-michael.chan@broadcom.com> (raw)
In-Reply-To: <20240430224438.91494-1-michael.chan@broadcom.com>

[-- Attachment #1: Type: text/plain, Size: 5036 bytes --]

David reported this issue of the driver not initializing on an older
chip not running in the new resource manager mode (!BNXT_NEW_RM()).

Sample dmesg:

bnxt_en 0000:02:00.0 (unnamed net_device) (uninitialized): Able to reserve only 0 out of 9 requested RX rings
bnxt_en 0000:02:00.0 (unnamed net_device) (uninitialized): Unable to reserve tx rings
bnxt_en 0000:02:00.0 (unnamed net_device) (uninitialized): 2nd rings reservation failed.
bnxt_en 0000:02:00.0 (unnamed net_device) (uninitialized): Not enough rings available.
bnxt_en 0000:02:00.0: probe with driver bnxt_en failed with error -12

This is a regression caused by a recent commit that adds a call to
bnxt_get_avail_msix() before MSIX is initialized:

bnxt_set_dflt_rings()
  __bnxt_reserve_rings()
    bnxt_get_avail_msix()

bnxt_get_avail_msix() returns a negative number if !BNXT_NEW_RM() and
when MSIX is not initialized.  This causes __bnxt_reserve_rings() to
fail in this call sequence and the driver aborts initialization.

Before this commit in 2022:

303432211324 ("bnxt_en: Remove runtime interrupt vector allocation")

MSIX allocation for RoCE was dynamic and bnxt_get_avail_msix() was
used to determine the available run-time MSIX available for RoCE.

Today, bnxt_get_avail_msix() is only used to reserve some available
MSIX ahead of time to be ready when the RoCE driver loads.  It
only needs to be called when BNXT_NEW_RM() is true because older
chips do not require reservations for MSIX.  Simplify
bnxt_get_avail_msix() to only consider the BNXT_NEW_RM() case and
only make this call if BNXT_NEW_RM() is true.

Also change bnxt_get_avail_msix() to static since it is only used
in bnxt.c.

Reported-by: David Wei <dw@davidwei.uk>
Fixes: d630624ebd70 ("bnxt_en: Utilize ulp client resources if RoCE is not registered")
Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
Cc: David Wei <dw@davidwei.uk>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 20 ++++++++------------
 drivers/net/ethernet/broadcom/bnxt/bnxt.h |  1 -
 2 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index be96bb494ae6..0eb880766012 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -7459,6 +7459,8 @@ static bool bnxt_rings_ok(struct bnxt *bp, struct bnxt_hw_rings *hwr)
 	       hwr->stat && (hwr->cp_p5 || !(bp->flags & BNXT_FLAG_CHIP_P5_PLUS));
 }
 
+static int bnxt_get_avail_msix(struct bnxt *bp, int num);
+
 static int __bnxt_reserve_rings(struct bnxt *bp)
 {
 	struct bnxt_hw_rings hwr = {0};
@@ -7471,7 +7473,7 @@ static int __bnxt_reserve_rings(struct bnxt *bp)
 	if (!bnxt_need_reserve_rings(bp))
 		return 0;
 
-	if (!bnxt_ulp_registered(bp->edev)) {
+	if (!bnxt_ulp_registered(bp->edev) && BNXT_NEW_RM(bp)) {
 		ulp_msix = bnxt_get_avail_msix(bp, bp->ulp_num_msix_want);
 		if (!ulp_msix)
 			bnxt_set_ulp_stat_ctxs(bp, 0);
@@ -10474,19 +10476,13 @@ unsigned int bnxt_get_avail_stat_ctxs_for_en(struct bnxt *bp)
 	return bnxt_get_max_func_stat_ctxs(bp) - bnxt_get_func_stat_ctxs(bp);
 }
 
-int bnxt_get_avail_msix(struct bnxt *bp, int num)
+/* Only called if BNXT_NEW_RM() is true to get the available MSIX to
+ * reserve ahead of time before RoCE is registered.
+ */
+static int bnxt_get_avail_msix(struct bnxt *bp, int num)
 {
-	int max_cp = bnxt_get_max_func_cp_rings(bp);
 	int max_irq = bnxt_get_max_func_irqs(bp);
 	int total_req = bp->cp_nr_rings + num;
-	int max_idx, avail_msix;
-
-	max_idx = bp->total_irqs;
-	if (!(bp->flags & BNXT_FLAG_CHIP_P5_PLUS))
-		max_idx = min_t(int, bp->total_irqs, max_cp);
-	avail_msix = max_idx - bp->cp_nr_rings;
-	if (!BNXT_NEW_RM(bp) || avail_msix >= num)
-		return avail_msix;
 
 	if (max_irq < total_req) {
 		num = max_irq - bp->cp_nr_rings;
@@ -10619,7 +10615,7 @@ int bnxt_reserve_rings(struct bnxt *bp, bool irq_re_init)
 	if (!bnxt_need_reserve_rings(bp))
 		return 0;
 
-	if (!bnxt_ulp_registered(bp->edev)) {
+	if (!bnxt_ulp_registered(bp->edev) && BNXT_NEW_RM(bp)) {
 		int ulp_msix = bnxt_get_avail_msix(bp, bp->ulp_num_msix_want);
 
 		if (ulp_msix > bp->ulp_num_msix_want)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index ad57ef051798..0c680032ab66 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -2736,7 +2736,6 @@ unsigned int bnxt_get_max_func_stat_ctxs(struct bnxt *bp);
 unsigned int bnxt_get_avail_stat_ctxs_for_en(struct bnxt *bp);
 unsigned int bnxt_get_max_func_cp_rings(struct bnxt *bp);
 unsigned int bnxt_get_avail_cp_rings_for_en(struct bnxt *bp);
-int bnxt_get_avail_msix(struct bnxt *bp, int num);
 int bnxt_reserve_rings(struct bnxt *bp, bool irq_re_init);
 void bnxt_tx_disable(struct bnxt *bp);
 void bnxt_tx_enable(struct bnxt *bp);
-- 
2.30.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

  reply	other threads:[~2024-04-30 22:46 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-30 22:44 [PATCH net-next 0/7] bnxt_en: Updates for net-next Michael Chan
2024-04-30 22:44 ` Michael Chan [this message]
2024-04-30 23:19   ` [PATCH net-next 1/7] bnxt_en: Fix and simplify bnxt_get_avail_msix() calls Jakub Kicinski
2024-04-30 23:35     ` Michael Chan
2024-04-30 23:44       ` Jakub Kicinski
2024-04-30 22:44 ` [PATCH net-next 2/7] bnxt_en: share NQ ring sw_stats memory with subrings Michael Chan
2024-04-30 22:44 ` [PATCH net-next 3/7] bnxt_en: Don't support offline self test when RoCE driver is loaded Michael Chan
2024-04-30 22:44 ` [PATCH net-next 4/7] bnxt_en: Don't call ULP_STOP/ULP_START during L2 reset Michael Chan
2024-04-30 22:44 ` [PATCH net-next 5/7] bnxt_en: Add a mutex to synchronize ULP operations Michael Chan
2024-04-30 22:44 ` [PATCH net-next 6/7] bnxt_en: Optimize recovery path ULP locking in the driver Michael Chan
2024-04-30 22:44 ` [PATCH net-next 7/7] bnxt_en: Add VF PCI ID for 5760X (P7) chips 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=20240430224438.91494-2-michael.chan@broadcom.com \
    --to=michael.chan@broadcom.com \
    --cc=ajit.khaparde@broadcom.com \
    --cc=andrew.gospodarek@broadcom.com \
    --cc=davem@davemloft.net \
    --cc=dw@davidwei.uk \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=pavan.chebbi@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 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.