[RFC net v3 3/3] bnxt_en: stop DMA before releasing rings the firmware did not free
From: Joe Damato <hidden>
Date: 2026-09-23 21:07:59
Also in:
lkml
Subsystem:
broadcom bnxt_en 50 gigabit ethernet driver, networking drivers, the rest · Maintainers:
Michael Chan, Pavan Chebbi, Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
When HWRM_RING_FREE is not answered, bnxt_hwrm_ring_free() clears
fw_ring_id and __bnxt_close_nic() goes on to call bnxt_free_mem(), which
unmaps the ring memory and the RX buffers that the FW may still be
using.
This is reachable in production. On a BCM57504 the first sign is the TX
watchdog; the close that follows times out a subset of its RING_FREEs and
the driver releases those rings anyway:
05:30:12 NETDEV WATCHDOG: transmit queue 0 timed out 6073 ms
05:30:12 Resp cmpl intr err msg: 0x51 x20
05:30:12 hwrm_ring_free type 1 failed x12
05:30:12 hwrm_ring_free type 2 failed x8
05:30:12 AMD-Vi: IO_PAGE_FAULT x3
Count the rings the firmware did not free and report that to the caller
so it can decide what to do. bnxt_hwrm_resource_free() still frees the
remaining firmware resources before returning the error, so the shutdown
can send every message it needs to before the device is stopped.
The device stays unusable until a firmware reset.
Fixes: 74608fc98d28 ("bnxt_en: Ring free response from close path should use completion ring")
Signed-off-by: Joe Damato <redacted>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 48 +++++++++++++++++------
1 file changed, 35 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index a7f6facca7b4..332fb374db5e 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c@@ -7754,21 +7754,25 @@ static void bnxt_clear_one_cp_ring(struct bnxt *bp, struct bnxt_cp_ring_info *cp memset(cpr->cp_desc_ring[i], 0, size); } -static void bnxt_hwrm_ring_free(struct bnxt *bp, bool close_path) +static int bnxt_hwrm_ring_free(struct bnxt *bp, bool close_path) { + int stuck = 0; u32 type; int i; if (!bp->bnapi) - return; + return 0; for (i = 0; i < bp->tx_nr_rings; i++) - bnxt_hwrm_tx_ring_free(bp, &bp->tx_ring[i], close_path); + if (bnxt_hwrm_tx_ring_free(bp, &bp->tx_ring[i], close_path)) + stuck++; bnxt_cancel_dim(bp); for (i = 0; i < bp->rx_nr_rings; i++) { - bnxt_hwrm_rx_ring_free(bp, &bp->rx_ring[i], close_path); - bnxt_hwrm_rx_agg_ring_free(bp, &bp->rx_ring[i], close_path); + if (bnxt_hwrm_rx_ring_free(bp, &bp->rx_ring[i], close_path)) + stuck++; + if (bnxt_hwrm_rx_agg_ring_free(bp, &bp->rx_ring[i], close_path)) + stuck++; } /* The completion rings are about to be freed. After that the
@@ -7798,6 +7802,19 @@ static void bnxt_hwrm_ring_free(struct bnxt *bp, bool close_path) bp->grp_info[i].cp_fw_ring_id = INVALID_HW_RING_ID; } } + + if (!stuck) + return 0; + + netdev_err(bp->dev, "Firmware did not free %d ring(s)\n", stuck); + return -EIO; +} + +static void bnxt_stop_dma(struct bnxt *bp) +{ + netdev_err(bp->dev, + "Disabling DMA before releasing ring memory, a firmware reset is required\n"); + pci_clear_master(bp->pdev); } static int __bnxt_trim_rings(struct bnxt *bp, int *rx, int *tx, int max,
@@ -10832,16 +10849,19 @@ static void bnxt_clear_vnic(struct bnxt *bp) bnxt_hwrm_vnic_ctx_free(bp); } -static void bnxt_hwrm_resource_free(struct bnxt *bp, bool close_path, - bool irq_re_init) +static int bnxt_hwrm_resource_free(struct bnxt *bp, bool close_path, + bool irq_re_init) { + int rc; + bnxt_clear_vnic(bp); - bnxt_hwrm_ring_free(bp, close_path); + rc = bnxt_hwrm_ring_free(bp, close_path); bnxt_hwrm_ring_grp_free(bp); if (irq_re_init) { bnxt_hwrm_stat_ctx_free(bp); bnxt_hwrm_free_tunnel_ports(bp); } + return rc; } static int bnxt_hwrm_set_br_mode(struct bnxt *bp, u16 br_mode)
@@ -11363,15 +11383,15 @@ static int bnxt_init_chip(struct bnxt *bp, bool irq_re_init) return 0; err_out: - bnxt_hwrm_resource_free(bp, 0, true); + if (bnxt_hwrm_resource_free(bp, 0, true)) + bnxt_stop_dma(bp); return rc; } static int bnxt_shutdown_nic(struct bnxt *bp, bool irq_re_init) { - bnxt_hwrm_resource_free(bp, 1, irq_re_init); - return 0; + return bnxt_hwrm_resource_free(bp, 1, irq_re_init); } static int bnxt_init_nic(struct bnxt *bp, bool irq_re_init)
@@ -13422,7 +13442,8 @@ int bnxt_half_open_nic(struct bnxt *bp) */ void bnxt_half_close_nic(struct bnxt *bp) { - bnxt_hwrm_resource_free(bp, false, true); + if (bnxt_hwrm_resource_free(bp, false, true)) + bnxt_stop_dma(bp); bnxt_del_napi(bp); bnxt_free_skbs(bp); bnxt_free_mem(bp, true);
@@ -13501,7 +13522,8 @@ static void __bnxt_close_nic(struct bnxt *bp, bool irq_re_init, if (BNXT_SUPPORTS_MULTI_RSS_CTX(bp)) bnxt_clear_rss_ctxs(bp); /* Flush rings and disable interrupts */ - bnxt_shutdown_nic(bp, irq_re_init); + if (bnxt_shutdown_nic(bp, irq_re_init)) + bnxt_stop_dma(bp); /* TODO CHIMP_FW: Link/PHY related cleanup if (link_re_init) */
--
2.53.0-Meta