netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net 0/8] bnxt_en: Misc. bug fixes
@ 2025-04-28 22:58 Michael Chan
  2025-04-28 22:58 ` [PATCH net 1/8] bnxt_en: Fix error handling path in bnxt_init_chip() Michael Chan
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Michael Chan @ 2025-04-28 22:58 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, kuba, pabeni, andrew+netdev, pavan.chebbi,
	andrew.gospodarek

This series fixes a bug in the driver initialization path, MSIX
setup sequencing issue in the FW error and AER paths, a missing
skb_mark_for_recycle() in the VLAN error path, some ethtool coredump
fixes, an ethtool selftest fix, and an ethtool register dump byte order
fix.

Kalesh AP (1):
  bnxt_en: Fix ethtool selftest output in one of the failure cases

Kashyap Desai (2):
  bnxt_en: call pci_alloc_irq_vectors() after bnxt_reserve_rings()
  bnxt_en: delay pci_alloc_irq_vectors() in the AER path

Michael Chan (1):
  bnxt_en: Fix ethtool -d byte order for 32-bit values

Shravya KN (1):
  bnxt_en: Fix error handling path in bnxt_init_chip()

Shruti Parab (2):
  bnxt_en: Fix coredump logic to free allocated buffer
  bnxt_en: Fix out-of-bound memcpy() during ethtool -w

Somnath Kotur (1):
  bnxt_en: Add missing skb_mark_for_recycle() in bnxt_rx_vlan()

 drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 28 +++++++------
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |  1 +
 .../ethernet/broadcom/bnxt/bnxt_coredump.c    | 20 +++++++---
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 40 +++++++++++++++----
 4 files changed, 65 insertions(+), 24 deletions(-)

-- 
2.30.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH net 1/8] bnxt_en: Fix error handling path in bnxt_init_chip()
  2025-04-28 22:58 [PATCH net 0/8] bnxt_en: Misc. bug fixes Michael Chan
@ 2025-04-28 22:58 ` Michael Chan
  2025-04-28 22:58 ` [PATCH net 2/8] bnxt_en: Fix ethtool selftest output in one of the failure cases Michael Chan
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Michael Chan @ 2025-04-28 22:58 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, kuba, pabeni, andrew+netdev, pavan.chebbi,
	andrew.gospodarek, Shravya KN, Somnath Kotur

From: Shravya KN <shravya.k-n@broadcom.com>

WARN_ON() is triggered in __flush_work() if bnxt_init_chip() fails
because we call cancel_work_sync() on dim work that has not been
initialized.

WARNING: CPU: 37 PID: 5223 at kernel/workqueue.c:4201 __flush_work.isra.0+0x212/0x230

The driver relies on the BNXT_STATE_NAPI_DISABLED bit to check if dim
work has already been cancelled.  But in the bnxt_open() path,
BNXT_STATE_NAPI_DISABLED is not set and this causes the error
path to think that it needs to cancel the uninitalized dim work.
Fix it by setting BNXT_STATE_NAPI_DISABLED during initialization.
The bit will be cleared when we enable NAPI and initialize dim work.

Fixes: 40452969a506 ("bnxt_en: Fix DIM shutdown")
Suggested-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Signed-off-by: Shravya KN <shravya.k-n@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 2c8e2c19d854..c4bccc683597 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -11602,6 +11602,9 @@ static void bnxt_init_napi(struct bnxt *bp)
 		poll_fn = bnxt_poll_p5;
 	else if (BNXT_CHIP_TYPE_NITRO_A0(bp))
 		cp_nr_rings--;
+
+	set_bit(BNXT_STATE_NAPI_DISABLED, &bp->state);
+
 	for (i = 0; i < cp_nr_rings; i++) {
 		bnapi = bp->bnapi[i];
 		netif_napi_add_config_locked(bp->dev, &bnapi->napi, poll_fn,
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net 2/8] bnxt_en: Fix ethtool selftest output in one of the failure cases
  2025-04-28 22:58 [PATCH net 0/8] bnxt_en: Misc. bug fixes Michael Chan
  2025-04-28 22:58 ` [PATCH net 1/8] bnxt_en: Fix error handling path in bnxt_init_chip() Michael Chan
@ 2025-04-28 22:58 ` Michael Chan
  2025-04-28 22:58 ` [PATCH net 3/8] bnxt_en: Add missing skb_mark_for_recycle() in bnxt_rx_vlan() Michael Chan
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Michael Chan @ 2025-04-28 22:58 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, kuba, pabeni, andrew+netdev, pavan.chebbi,
	andrew.gospodarek, Kalesh AP, Somnath Kotur

From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>

When RDMA driver is loaded, running offline self test is not
supported and driver returns failure early. But it is not clearing
the input buffer and hence the application prints some junk
characters for individual test results.

Fix it by clearing the buffer before returning.

Fixes: 895621f1c816 ("bnxt_en: Don't support offline self test when RoCE driver is loaded")
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 48dd5922e4dd..7be37976f3e4 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -4991,6 +4991,7 @@ static void bnxt_self_test(struct net_device *dev, struct ethtool_test *etest,
 	if (!bp->num_tests || !BNXT_PF(bp))
 		return;
 
+	memset(buf, 0, sizeof(u64) * bp->num_tests);
 	if (etest->flags & ETH_TEST_FL_OFFLINE &&
 	    bnxt_ulp_registered(bp->edev)) {
 		etest->flags |= ETH_TEST_FL_FAILED;
@@ -4998,7 +4999,6 @@ static void bnxt_self_test(struct net_device *dev, struct ethtool_test *etest,
 		return;
 	}
 
-	memset(buf, 0, sizeof(u64) * bp->num_tests);
 	if (!netif_running(dev)) {
 		etest->flags |= ETH_TEST_FL_FAILED;
 		return;
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net 3/8] bnxt_en: Add missing skb_mark_for_recycle() in bnxt_rx_vlan()
  2025-04-28 22:58 [PATCH net 0/8] bnxt_en: Misc. bug fixes Michael Chan
  2025-04-28 22:58 ` [PATCH net 1/8] bnxt_en: Fix error handling path in bnxt_init_chip() Michael Chan
  2025-04-28 22:58 ` [PATCH net 2/8] bnxt_en: Fix ethtool selftest output in one of the failure cases Michael Chan
@ 2025-04-28 22:58 ` Michael Chan
  2025-04-28 22:58 ` [PATCH net 4/8] bnxt_en: call pci_alloc_irq_vectors() after bnxt_reserve_rings() Michael Chan
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Michael Chan @ 2025-04-28 22:58 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, kuba, pabeni, andrew+netdev, pavan.chebbi,
	andrew.gospodarek, Somnath Kotur, Kalesh AP

From: Somnath Kotur <somnath.kotur@broadcom.com>

If bnxt_rx_vlan() fails because the VLAN protocol ID is invalid,
the SKB is freed but we're missing the call to recycle it.  This
may cause the warning:

"page_pool_release_retry() stalled pool shutdown"

Add the missing skb_mark_for_recycle() in bnxt_rx_vlan().

Fixes: 86b05508f775 ("bnxt_en: Use the unified RX page pool buffers for XDP and non-XDP")
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index c4bccc683597..cfc9ccab39bf 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -2015,6 +2015,7 @@ static struct sk_buff *bnxt_rx_vlan(struct sk_buff *skb, u8 cmp_type,
 	}
 	return skb;
 vlan_err:
+	skb_mark_for_recycle(skb);
 	dev_kfree_skb(skb);
 	return NULL;
 }
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net 4/8] bnxt_en: call pci_alloc_irq_vectors() after bnxt_reserve_rings()
  2025-04-28 22:58 [PATCH net 0/8] bnxt_en: Misc. bug fixes Michael Chan
                   ` (2 preceding siblings ...)
  2025-04-28 22:58 ` [PATCH net 3/8] bnxt_en: Add missing skb_mark_for_recycle() in bnxt_rx_vlan() Michael Chan
@ 2025-04-28 22:58 ` Michael Chan
  2025-04-28 22:59 ` [PATCH net 5/8] bnxt_en: delay pci_alloc_irq_vectors() in the AER path Michael Chan
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Michael Chan @ 2025-04-28 22:58 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, kuba, pabeni, andrew+netdev, pavan.chebbi,
	andrew.gospodarek, Kashyap Desai

From: Kashyap Desai <kashyap.desai@broadcom.com>

On some architectures (e.g. ARM), calling pci_alloc_irq_vectors()
will immediately cause the MSIX table to be written.  This will not
work if we haven't called bnxt_reserve_rings() to properly map
the MSIX table to the MSIX vectors reserved by FW.

Fix the FW error recovery path to delay the bnxt_init_int_mode() ->
pci_alloc_irq_vectors() call by removing it from bnxt_hwrm_if_change().
bnxt_request_irq() later in the code path will call it and by then the
MSIX table is properly mapped.

Fixes: 4343838ca5eb ("bnxt_en: Replace deprecated PCI MSIX APIs")
Suggested-by: Michael Chan <michael.chan@broadcom.com>
Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index cfc9ccab39bf..3b2ea36f25a2 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -12399,13 +12399,8 @@ static int bnxt_hwrm_if_change(struct bnxt *bp, bool up)
 				set_bit(BNXT_STATE_ABORT_ERR, &bp->state);
 				return rc;
 			}
+			/* IRQ will be initialized later in bnxt_request_irq()*/
 			bnxt_clear_int_mode(bp);
-			rc = bnxt_init_int_mode(bp);
-			if (rc) {
-				clear_bit(BNXT_STATE_FW_RESET_DET, &bp->state);
-				netdev_err(bp->dev, "init int mode failed\n");
-				return rc;
-			}
 		}
 		rc = bnxt_cancel_reservations(bp, fw_reset);
 	}
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net 5/8] bnxt_en: delay pci_alloc_irq_vectors() in the AER path
  2025-04-28 22:58 [PATCH net 0/8] bnxt_en: Misc. bug fixes Michael Chan
                   ` (3 preceding siblings ...)
  2025-04-28 22:58 ` [PATCH net 4/8] bnxt_en: call pci_alloc_irq_vectors() after bnxt_reserve_rings() Michael Chan
@ 2025-04-28 22:59 ` Michael Chan
  2025-04-28 22:59 ` [PATCH net 6/8] bnxt_en: Fix coredump logic to free allocated buffer Michael Chan
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Michael Chan @ 2025-04-28 22:59 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, kuba, pabeni, andrew+netdev, pavan.chebbi,
	andrew.gospodarek, Kashyap Desai

From: Kashyap Desai <kashyap.desai@broadcom.com>

This patch is similar to the last patch to delay the
pci_alloc_irq_vectors() call in the AER path until after calling
bnxt_reserve_rings().  bnxt_reserve_rings() needs to properly map
the MSIX table first before we call pci_alloc_irq_vectors() which
may immediately write to the MSIX table in some architectures.

Move the bnxt_init_int_mode() call from bnxt_io_slot_reset() to
bnxt_io_resume() after calling bnxt_reserve_rings().

With this change, the AER path may call bnxt_open() ->
bnxt_hwrm_if_change() with bp->irq_tbl set to NULL.  bp->irq_tbl is
cleared when we call bnxt_clear_int_mode() in bnxt_io_slot_reset().
So we cannot use !bp->irq_tbl to detect aborted FW reset.  Add a
new BNXT_FW_RESET_STATE_ABORT to detect aborted FW reset in
bnxt_hwrm_if_change().

Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 17 +++++++++++------
 drivers/net/ethernet/broadcom/bnxt/bnxt.h |  1 +
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 3b2ea36f25a2..78e496b0ec26 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -12325,12 +12325,15 @@ static int bnxt_hwrm_if_change(struct bnxt *bp, bool up)
 {
 	struct hwrm_func_drv_if_change_output *resp;
 	struct hwrm_func_drv_if_change_input *req;
-	bool fw_reset = !bp->irq_tbl;
 	bool resc_reinit = false;
 	bool caps_change = false;
 	int rc, retry = 0;
+	bool fw_reset;
 	u32 flags = 0;
 
+	fw_reset = (bp->fw_reset_state == BNXT_FW_RESET_STATE_ABORT);
+	bp->fw_reset_state = 0;
+
 	if (!(bp->fw_cap & BNXT_FW_CAP_IF_CHANGE))
 		return 0;
 
@@ -14833,7 +14836,7 @@ static void bnxt_fw_reset_abort(struct bnxt *bp, int rc)
 	clear_bit(BNXT_STATE_IN_FW_RESET, &bp->state);
 	if (bp->fw_reset_state != BNXT_FW_RESET_STATE_POLL_VF)
 		bnxt_dl_health_fw_status_update(bp, false);
-	bp->fw_reset_state = 0;
+	bp->fw_reset_state = BNXT_FW_RESET_STATE_ABORT;
 	netif_close(bp->dev);
 }
 
@@ -16931,10 +16934,9 @@ static pci_ers_result_t bnxt_io_slot_reset(struct pci_dev *pdev)
 		if (!err)
 			result = PCI_ERS_RESULT_RECOVERED;
 
+		/* IRQ will be initialized later in bnxt_io_resume */
 		bnxt_ulp_irq_stop(bp);
 		bnxt_clear_int_mode(bp);
-		err = bnxt_init_int_mode(bp);
-		bnxt_ulp_irq_restart(bp, err);
 	}
 
 reset_exit:
@@ -16963,10 +16965,13 @@ static void bnxt_io_resume(struct pci_dev *pdev)
 
 	err = bnxt_hwrm_func_qcaps(bp);
 	if (!err) {
-		if (netif_running(netdev))
+		if (netif_running(netdev)) {
 			err = bnxt_open(netdev);
-		else
+		} else {
 			err = bnxt_reserve_rings(bp, true);
+			if (!err)
+				err = bnxt_init_int_mode(bp);
+		}
 	}
 
 	if (!err)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 21726cf56586..bc8b3b7e915d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -2614,6 +2614,7 @@ struct bnxt {
 #define BNXT_FW_RESET_STATE_POLL_FW	4
 #define BNXT_FW_RESET_STATE_OPENING	5
 #define BNXT_FW_RESET_STATE_POLL_FW_DOWN	6
+#define BNXT_FW_RESET_STATE_ABORT	7
 
 	u16			fw_reset_min_dsecs;
 #define BNXT_DFLT_FW_RST_MIN_DSECS	20
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net 6/8] bnxt_en: Fix coredump logic to free allocated buffer
  2025-04-28 22:58 [PATCH net 0/8] bnxt_en: Misc. bug fixes Michael Chan
                   ` (4 preceding siblings ...)
  2025-04-28 22:59 ` [PATCH net 5/8] bnxt_en: delay pci_alloc_irq_vectors() in the AER path Michael Chan
@ 2025-04-28 22:59 ` Michael Chan
  2025-04-28 22:59 ` [PATCH net 7/8] bnxt_en: Fix out-of-bound memcpy() during ethtool -w Michael Chan
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Michael Chan @ 2025-04-28 22:59 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, kuba, pabeni, andrew+netdev, pavan.chebbi,
	andrew.gospodarek, Shruti Parab, Kalesh AP

From: Shruti Parab <shruti.parab@broadcom.com>

When handling HWRM_DBG_COREDUMP_LIST FW command in
bnxt_hwrm_dbg_dma_data(), the allocated buffer info->dest_buf is
not freed in the error path.  In the normal path, info->dest_buf
is assigned to coredump->data and it will eventually be freed after
the coredump is collected.

Free info->dest_buf immediately inside bnxt_hwrm_dbg_dma_data() in
the error path.

Fixes: c74751f4c392 ("bnxt_en: Return error if FW returns more data than dump length")
Reported-by: Michael Chan <michael.chan@broadcom.com>
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Shruti Parab <shruti.parab@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_coredump.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_coredump.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_coredump.c
index 5576e7cf8463..9a74793648f6 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_coredump.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_coredump.c
@@ -116,6 +116,11 @@ static int bnxt_hwrm_dbg_dma_data(struct bnxt *bp, void *msg,
 				memcpy(info->dest_buf + off, dma_buf, len);
 			} else {
 				rc = -ENOBUFS;
+				if (cmn_req->req_type ==
+				    cpu_to_le16(HWRM_DBG_COREDUMP_LIST)) {
+					kfree(info->dest_buf);
+					info->dest_buf = NULL;
+				}
 				break;
 			}
 		}
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net 7/8] bnxt_en: Fix out-of-bound memcpy() during ethtool -w
  2025-04-28 22:58 [PATCH net 0/8] bnxt_en: Misc. bug fixes Michael Chan
                   ` (5 preceding siblings ...)
  2025-04-28 22:59 ` [PATCH net 6/8] bnxt_en: Fix coredump logic to free allocated buffer Michael Chan
@ 2025-04-28 22:59 ` Michael Chan
  2025-04-28 22:59 ` [PATCH net 8/8] bnxt_en: Fix ethtool -d byte order for 32-bit values Michael Chan
  2025-05-01 13:57 ` [PATCH net 0/8] bnxt_en: Misc. bug fixes Jakub Kicinski
  8 siblings, 0 replies; 10+ messages in thread
From: Michael Chan @ 2025-04-28 22:59 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, kuba, pabeni, andrew+netdev, pavan.chebbi,
	andrew.gospodarek, Shruti Parab, Kalesh AP

From: Shruti Parab <shruti.parab@broadcom.com>

When retrieving the FW coredump using ethtool, it can sometimes cause
memory corruption:

BUG: KFENCE: memory corruption in __bnxt_get_coredump+0x3ef/0x670 [bnxt_en]
Corrupted memory at 0x000000008f0f30e8 [ ! ! ! ! ! ! ! ! ! ! ! ! ! ! ! ! ] (in kfence-#45):
__bnxt_get_coredump+0x3ef/0x670 [bnxt_en]
ethtool_get_dump_data+0xdc/0x1a0
__dev_ethtool+0xa1e/0x1af0
dev_ethtool+0xa8/0x170
dev_ioctl+0x1b5/0x580
sock_do_ioctl+0xab/0xf0
sock_ioctl+0x1ce/0x2e0
__x64_sys_ioctl+0x87/0xc0
do_syscall_64+0x5c/0xf0
entry_SYSCALL_64_after_hwframe+0x78/0x80

...

This happens when copying the coredump segment list in
bnxt_hwrm_dbg_dma_data() with the HWRM_DBG_COREDUMP_LIST FW command.
The info->dest_buf buffer is allocated based on the number of coredump
segments returned by the FW.  The segment list is then DMA'ed by
the FW and the length of the DMA is returned by FW.  The driver then
copies this DMA'ed segment list to info->dest_buf.

In some cases, this DMA length may exceed the info->dest_buf length
and cause the above BUG condition.  Fix it by capping the copy
length to not exceed the length of info->dest_buf.  The extra
DMA data contains no useful information.

This code path is shared for the HWRM_DBG_COREDUMP_LIST and the
HWRM_DBG_COREDUMP_RETRIEVE FW commands.  The buffering is different
for these 2 FW commands.  To simplify the logic, we need to move
the line to adjust the buffer length for HWRM_DBG_COREDUMP_RETRIEVE
up, so that the new check to cap the copy length will work for both
commands.

Fixes: c74751f4c392 ("bnxt_en: Return error if FW returns more data than dump length")
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Shruti Parab <shruti.parab@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 .../net/ethernet/broadcom/bnxt/bnxt_coredump.c    | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_coredump.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_coredump.c
index 9a74793648f6..a000d3f630bd 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_coredump.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_coredump.c
@@ -110,10 +110,19 @@ static int bnxt_hwrm_dbg_dma_data(struct bnxt *bp, void *msg,
 			}
 		}
 
+		if (cmn_req->req_type ==
+				cpu_to_le16(HWRM_DBG_COREDUMP_RETRIEVE))
+			info->dest_buf_size += len;
+
 		if (info->dest_buf) {
 			if ((info->seg_start + off + len) <=
 			    BNXT_COREDUMP_BUF_LEN(info->buf_len)) {
-				memcpy(info->dest_buf + off, dma_buf, len);
+				u16 copylen = min_t(u16, len,
+						    info->dest_buf_size - off);
+
+				memcpy(info->dest_buf + off, dma_buf, copylen);
+				if (copylen < len)
+					break;
 			} else {
 				rc = -ENOBUFS;
 				if (cmn_req->req_type ==
@@ -125,10 +134,6 @@ static int bnxt_hwrm_dbg_dma_data(struct bnxt *bp, void *msg,
 			}
 		}
 
-		if (cmn_req->req_type ==
-				cpu_to_le16(HWRM_DBG_COREDUMP_RETRIEVE))
-			info->dest_buf_size += len;
-
 		if (!(cmn_resp->flags & HWRM_DBG_CMN_FLAGS_MORE))
 			break;
 
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net 8/8] bnxt_en: Fix ethtool -d byte order for 32-bit values
  2025-04-28 22:58 [PATCH net 0/8] bnxt_en: Misc. bug fixes Michael Chan
                   ` (6 preceding siblings ...)
  2025-04-28 22:59 ` [PATCH net 7/8] bnxt_en: Fix out-of-bound memcpy() during ethtool -w Michael Chan
@ 2025-04-28 22:59 ` Michael Chan
  2025-05-01 13:57 ` [PATCH net 0/8] bnxt_en: Misc. bug fixes Jakub Kicinski
  8 siblings, 0 replies; 10+ messages in thread
From: Michael Chan @ 2025-04-28 22:59 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, kuba, pabeni, andrew+netdev, pavan.chebbi,
	andrew.gospodarek, Shruti Parab, Kalesh AP

For version 1 register dump that includes the PCIe stats, the existing
code incorrectly assumes that all PCIe stats are 64-bit values.  Fix it
by using an array containing the starting and ending index of the 32-bit
values.  The loop in bnxt_get_regs() will use the array to do proper
endian swap for the 32-bit values.

Fixes: b5d600b027eb ("bnxt_en: Add support for 'ethtool -d'")
Reviewed-by: Shruti Parab <shruti.parab@broadcom.com>
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 38 ++++++++++++++++---
 1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 7be37976f3e4..f5d490bf997e 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -2062,6 +2062,17 @@ static int bnxt_get_regs_len(struct net_device *dev)
 	return reg_len;
 }
 
+#define BNXT_PCIE_32B_ENTRY(start, end)			\
+	 { offsetof(struct pcie_ctx_hw_stats, start),	\
+	   offsetof(struct pcie_ctx_hw_stats, end) }
+
+static const struct {
+	u16 start;
+	u16 end;
+} bnxt_pcie_32b_entries[] = {
+	BNXT_PCIE_32B_ENTRY(pcie_ltssm_histogram[0], pcie_ltssm_histogram[3]),
+};
+
 static void bnxt_get_regs(struct net_device *dev, struct ethtool_regs *regs,
 			  void *_p)
 {
@@ -2094,12 +2105,27 @@ static void bnxt_get_regs(struct net_device *dev, struct ethtool_regs *regs,
 	req->pcie_stat_host_addr = cpu_to_le64(hw_pcie_stats_addr);
 	rc = hwrm_req_send(bp, req);
 	if (!rc) {
-		__le64 *src = (__le64 *)hw_pcie_stats;
-		u64 *dst = (u64 *)(_p + BNXT_PXP_REG_LEN);
-		int i;
-
-		for (i = 0; i < sizeof(*hw_pcie_stats) / sizeof(__le64); i++)
-			dst[i] = le64_to_cpu(src[i]);
+		u8 *dst = (u8 *)(_p + BNXT_PXP_REG_LEN);
+		u8 *src = (u8 *)hw_pcie_stats;
+		int i, j;
+
+		for (i = 0, j = 0; i < sizeof(*hw_pcie_stats); ) {
+			if (i >= bnxt_pcie_32b_entries[j].start &&
+			    i <= bnxt_pcie_32b_entries[j].end) {
+				u32 *dst32 = (u32 *)(dst + i);
+
+				*dst32 = le32_to_cpu(*(__le32 *)(src + i));
+				i += 4;
+				if (i > bnxt_pcie_32b_entries[j].end &&
+				    j < ARRAY_SIZE(bnxt_pcie_32b_entries) - 1)
+					j++;
+			} else {
+				u64 *dst64 = (u64 *)(dst + i);
+
+				*dst64 = le64_to_cpu(*(__le64 *)(src + i));
+				i += 8;
+			}
+		}
 	}
 	hwrm_req_drop(bp, req);
 }
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH net 0/8] bnxt_en: Misc. bug fixes
  2025-04-28 22:58 [PATCH net 0/8] bnxt_en: Misc. bug fixes Michael Chan
                   ` (7 preceding siblings ...)
  2025-04-28 22:59 ` [PATCH net 8/8] bnxt_en: Fix ethtool -d byte order for 32-bit values Michael Chan
@ 2025-05-01 13:57 ` Jakub Kicinski
  8 siblings, 0 replies; 10+ messages in thread
From: Jakub Kicinski @ 2025-05-01 13:57 UTC (permalink / raw)
  To: Michael Chan
  Cc: davem, netdev, edumazet, pabeni, andrew+netdev, pavan.chebbi,
	andrew.gospodarek

On Mon, 28 Apr 2025 15:58:55 -0700 Michael Chan wrote:
> This series fixes a bug in the driver initialization path, MSIX
> setup sequencing issue in the FW error and AER paths, a missing
> skb_mark_for_recycle() in the VLAN error path, some ethtool coredump
> fixes, an ethtool selftest fix, and an ethtool register dump byte order
> fix.

Applied by Dave last night, thanks!

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-05-01 13:57 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-28 22:58 [PATCH net 0/8] bnxt_en: Misc. bug fixes Michael Chan
2025-04-28 22:58 ` [PATCH net 1/8] bnxt_en: Fix error handling path in bnxt_init_chip() Michael Chan
2025-04-28 22:58 ` [PATCH net 2/8] bnxt_en: Fix ethtool selftest output in one of the failure cases Michael Chan
2025-04-28 22:58 ` [PATCH net 3/8] bnxt_en: Add missing skb_mark_for_recycle() in bnxt_rx_vlan() Michael Chan
2025-04-28 22:58 ` [PATCH net 4/8] bnxt_en: call pci_alloc_irq_vectors() after bnxt_reserve_rings() Michael Chan
2025-04-28 22:59 ` [PATCH net 5/8] bnxt_en: delay pci_alloc_irq_vectors() in the AER path Michael Chan
2025-04-28 22:59 ` [PATCH net 6/8] bnxt_en: Fix coredump logic to free allocated buffer Michael Chan
2025-04-28 22:59 ` [PATCH net 7/8] bnxt_en: Fix out-of-bound memcpy() during ethtool -w Michael Chan
2025-04-28 22:59 ` [PATCH net 8/8] bnxt_en: Fix ethtool -d byte order for 32-bit values Michael Chan
2025-05-01 13:57 ` [PATCH net 0/8] bnxt_en: Misc. bug fixes Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).