Greetings:

This series makes a failed TX HWRM_RING_FREE recoverable instead of silently
releasing ring memory the FW might still own to address a use after free I
noticed on a production system.

I'm posting this as an RFC because:
  - I am not sure if patch 2 is correct. Maybe Broadcom can let me know ?
  - I am not sure if this is a Fixes or not. I guess if it was always like
    this then this is net-next material?

The core issue is that bnxt_hwrm_tx_ring_free discards the return value of
hwrm_ring_free_send_msg and sets fw_ring_id = INVALID_HW_RING_ID
unconditionally. When a TX RING_FREE sent over a completion ring times out,
the driver logs:

  hwrm_ring_free type 1 failed. rc:fffffff0 err:0
  Resp cmpl intr err msg: 0x51

and then __bnxt_close_nic() calls bnxt_free_mem(), unmapping ring memory that
the FW was never told to release.

bnxt_queue_stop mentions something about this in a comment:

  "HWRM_RING_FREE completion is handled in NAPI to guarantee no more DMA
   on that ring after seeing the completion."

But... if the completion ring is dead, the completion never arrives.

This is reachable on production systems today:

  - TX completions stop
  - netdev watchdog fires
  - reset closes the device
  - every RING_FREE routed through that dead completion ring times out
  - ring memory freed by the driver but still in use by the FW

The result on an IOMMU host is IO_PAGE_FAULT or DMAR fault against freed
pages.

I tried to test the code in patch 2 on a BCM57504 with FW 235.1.208.0/pkg
235.1.208.0.

I hacked something together to inject a failure to test the reset paths on my
device. It seems like HWRM_RING_RESET ring_type=TX is accepted by thte FW and
the polled RING_FREE also succeeds, but in my testing the TX ring was idle. I
never tested a reset against a ring with descriptors in flight. Which leads me
to my questions.....

1. Does HWRM_RING_RESET with ring_type=TX cause the FW to abandon work already
outstanding on that ring and stop DMA ? If not .... then this code is wrong :(
and maybe see question (3) below.

2. Is a TX ring reset supposed to post a completion ring entry? In my testing
it seemed like the FW may have written success into the response DMA buffer
but never posted the entry, so the request times out with -EBUSY even though
it succeeded. Is that intentional? If so, maybe only polled transport mode
works on this FW?

3. Maybe the TX reset isn't necessary at all? Maybe instead the code should
retry the RING_FREE over polled transport and that's good enough? This depends
on the answer to question (1) above, but I guess it would simplify the code if
a polled RING_FREE is enough?

Thanks,
Joe

Joe Damato (2):
  bnxt_en: return status from bnxt_hwrm_tx_ring_free
  bnxt_en: recover a failed TX RING_FREE with a ring reset

 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 55 ++++++++++++++++++++---
 1 file changed, 49 insertions(+), 6 deletions(-)


base-commit: 5ccdfb2c3203207deb17e7c5b0db8c7f475639a7
-- 
2.53.0-Meta

On Thu, Sep 17, 2026 at 4:32 PM Joe Damato <joe@dama.to> wrote:

> But... if the completion ring is dead, the completion never arrives.
>
> This is reachable on production systems today:
>
>   - TX completions stop
>   - netdev watchdog fires
>   - reset closes the device
>   - every RING_FREE routed through that dead completion ring times out
>   - ring memory freed by the driver but still in use by the FW
>
> The result on an IOMMU host is IO_PAGE_FAULT or DMAR fault against freed
> pages.

If the FW did not receive the HWRM_RING_FREE command from the driver,
there is no guarantee that DMA will stop.  I agree that the driver is
not very robust in handling this in the close path.  We just continue
after the timeout hoping that the FW actually received it but couldn't
respond.

In contrast, the error recovery path for fatal errors is a little more
robust.  For example, in bnxt_fw_fatal_close(), we call
pci_disable_device() to stop DMA from the device.

We probably should do something similar in the close path when
HWRM_RING_FREE is not responding at all.  Maybe even FLR.

>
> I tried to test the code in patch 2 on a BCM57504 with FW 235.1.208.0/pkg
> 235.1.208.0.
>
> I hacked something together to inject a failure to test the reset paths on my
> device. It seems like HWRM_RING_RESET ring_type=TX is accepted by thte FW and
> the polled RING_FREE also succeeds, but in my testing the TX ring was idle. I
> never tested a reset against a ring with descriptors in flight. Which leads me
> to my questions.....
>
> 1. Does HWRM_RING_RESET with ring_type=TX cause the FW to abandon work already
> outstanding on that ring and stop DMA ? If not .... then this code is wrong :(
> and maybe see question (3) below.

HWRM_RING_RESET is not supported on BCM5750X and BCM5760X.  Even if
the FW accepts it, it probably does nothing internally.

>
> 3. Maybe the TX reset isn't necessary at all? Maybe instead the code should
> retry the RING_FREE over polled transport and that's good enough? This depends
> on the answer to question (1) above, but I guess it would simplify the code if
> a polled RING_FREE is enough?
>

If interrupt is not working and the HWRM_RING_FREE doesn't complete
using the interrupt path, it will timeout using the same FW polling
timeout.  In other words, if FW received it but just couldn't post the
completion on the completion ring and interrupt, the wait should be
long enough for it to complete.   One additional thing we can do that
we're currently not doing is to check for a valid response length and
check the valid bit anyway on interrupt timeout.  If these look valid,
the FW has accepted the HWRM_RING_FREE but just couldn't interrupt.
If these are not valid, sending a second HWRM_RING_FREE should yield
the same results.

Thanks for looking into this.