[PATCH net v3 0/3] bnxt_en: PCIe FLR/AER fixes

[PATCH net v3 0/3] bnxt_en: PCIe FLR/AER fixes

[Michael Chan]

This patchset is split from the earlier 9-patch series to limit the
fixes to the PCIe FLR/AER issues only (patches 7 to 9).  The other
fixes (patches 1 to 6) will be sent to net-next separately.

The 1st patch is unchanged since v1.  It refactors a bnxt_clear_bars()
helper needed by patch 2 and 3.  Patch 2 fixes an issue during driver
init.  in the kdump kernel on a Dell system with a smart PCIe switch
upstream from the NIC.  Patch 3 fixes an issue in the AER code path for
non-fatal errors found by a partner.

These patches have been regression tested on Broadcom 5750X and 5760X
NICs running 237.x.x.x production FW.

v3:
Only include the 3 PCIe FLR/AER fixes.
Updated to address some more Sashiko comments.

v2:
Updated to address some Sashiko comments.

https://lore.kernel.org/netdev/20260928041712.3467803-1-michael.chan@broadcom.com/

v1:
https://lore.kernel.org/netdev/20260831024342.2161156-1-michael.chan@broadcom.com/

Michael Chan (2):
  bnxt_en: Add bnxt_clear_bars() helper
  bnxt_en: Fix driver init in kdump kernel

Pavan Chebbi (1):
  bnxt_en: Re-write the BARs following any type of PCIe errors

 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 164 ++++++++++++++--------
 drivers/net/ethernet/broadcom/bnxt/bnxt.h |   1 -
 2 files changed, 108 insertions(+), 57 deletions(-)

-- 
2.51.0

[PATCH net v3 1/3] bnxt_en: Add bnxt_clear_bars() helper

[Michael Chan]

In bnxt_io_slot_reset(), we clear the 6 BAR registers.  Add a helper
function to do that.  The helper will be used again in the next patch.

Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Joe Damato <joe@dama.to>
Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index d7728d0c5b6e..8ce8a82d3453 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -17111,6 +17111,14 @@ void bnxt_print_device_info(struct bnxt *bp)
 	pcie_print_link_status(bp->pdev);
 }
 
+static void bnxt_clear_bars(struct pci_dev *pdev)
+{
+	int off;
+
+	for (off = PCI_BASE_ADDRESS_0; off <= PCI_BASE_ADDRESS_5; off += 4)
+		pci_write_config_dword(pdev, off, 0);
+}
+
 static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct bnxt_hw_resc *hw_resc;
@@ -17598,7 +17606,6 @@ static pci_ers_result_t bnxt_io_slot_reset(struct pci_dev *pdev)
 	struct bnxt *bp = netdev_priv(netdev);
 	int retry = 0;
 	int err = 0;
-	int off;
 
 	netdev_info(bp->dev, "PCI Slot Reset\n");
 
@@ -17627,11 +17634,8 @@ static pci_ers_result_t bnxt_io_slot_reset(struct pci_dev *pdev)
 		 * write the BARs to 0 to force restore, in case of fatal error.
 		 */
 		if (test_and_clear_bit(BNXT_STATE_PCI_CHANNEL_IO_FROZEN,
-				       &bp->state)) {
-			for (off = PCI_BASE_ADDRESS_0;
-			     off <= PCI_BASE_ADDRESS_5; off += 4)
-				pci_write_config_dword(bp->pdev, off, 0);
-		}
+				       &bp->state))
+			bnxt_clear_bars(pdev);
 		pci_restore_state(pdev);
 
 		bnxt_inv_fw_health_reg(bp);
-- 
2.51.0

[PATCH net v3 2/3] bnxt_en: Fix driver init in kdump kernel

[Michael Chan]

Fix and strengthen the FLR sequence when initializing in the kdump
kernel.  If the NIC is behind a PCIe switch in synthetic (smart)
mode, the switch may need to see that the BARs have been initialized
before it will pass mem read/write TLPs to the NIC.

On a Dell system with a PEX89144 PCIe switch, echo c > /proc/sysrq-trigger
will trigger fatal AER without this patch:

bnxt_en 0000:67:00.0: enabling device (0000 -> 0002)
[Hardware Error]: Hardware error from APEI Generic Hardware Error Source: 5
[Hardware Error]: event severity: recoverable
[Hardware Error]:  Error 0, type: fatal
[Hardware Error]:   section_type: PCIe error
[Hardware Error]:   port_type: 5, upstream switch port
...

Add a new bnxt_kdump_reset() to do the expanded FLR sequence in the
kdump kernel.  We now disable bus master and memory, save the PCI
state, do the FLR, clear the BARs, and restore the PCI state.  The
BARs have to be cleared to ensure that they get re-initialized and
visible to the PCIe switch.

Since it is the kdump kernel, we make every effort to continue in
the best possible way even if pci_save_state() or pcie_flr() returns
error.  After FLR, we poll for an additional 5 seconds before
aborting in case the device is not properly returning CRS.  This is
similar to the 5-second wait in bnxt_io_slot_reset().

Fixes: 8743db4a9acf ("bnxt_en: Issue PCIe FLR in kdump kernel to cleanup pending DMAs.")
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
v3:
Poll for 5 seconds after FLR and abort if config space is not responding.

v2:
Disable device before pci_save_state() and pcie_flr() and check for
errors. 

https://lore.kernel.org/netdev/20260928041712.3467803-9-michael.chan@broadcom.com/

v1:
https://lore.kernel.org/netdev/20260831024342.2161156-4-michael.chan@broadcom.com/
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 45 ++++++++++++++++++++---
 1 file changed, 40 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 8ce8a82d3453..9ea7e172787e 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -17119,6 +17119,43 @@ static void bnxt_clear_bars(struct pci_dev *pdev)
 		pci_write_config_dword(pdev, off, 0);
 }
 
+/* Clear any pending DMA transactions from crash kernel while loading driver in
+ * capture kernel.
+ */
+static int bnxt_kdump_reset(struct pci_dev *pdev)
+{
+	int rc, i;
+	u16 cmd;
+
+	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
+	cmd &= ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY);
+	pci_write_config_word(pdev, PCI_COMMAND, cmd);
+
+	if (pci_save_state(pdev))
+		dev_warn(&pdev->dev, "Failed to save PCI state, PCI restore may be incomplete\n");
+
+	rc = pcie_flr(pdev);
+	if (rc)
+		dev_warn(&pdev->dev, "pcie_flr() failed (rc: %d), trying to continue\n",
+			 rc);
+
+	/* In case device is not returning CRS, wait 5 seconds longer */
+	for (i = 0; i < 50; i++) {
+		pci_read_config_word(pdev, PCI_COMMAND, &cmd);
+		if (!PCI_POSSIBLE_ERROR(cmd))
+			break;
+		msleep(100);
+	}
+	if (PCI_POSSIBLE_ERROR(cmd)) {
+		dev_err(&pdev->dev, "PCI config space inaccessible after FLR, aborting\n");
+		return -ENODEV;
+	}
+
+	bnxt_clear_bars(pdev);
+	pci_restore_state(pdev);
+	return 0;
+}
+
 static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct bnxt_hw_resc *hw_resc;
@@ -17134,12 +17171,10 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 		return -ENODEV;
 	}
 
-	/* Clear any pending DMA transactions from crash kernel
-	 * while loading driver in capture kernel.
-	 */
 	if (is_kdump_kernel()) {
-		pci_clear_master(pdev);
-		pcie_flr(pdev);
+		rc = bnxt_kdump_reset(pdev);
+		if (rc)
+			return rc;
 	}
 
 	max_irqs = bnxt_get_max_irq(pdev);
-- 
2.51.0

Re: [PATCH net v3 2/3] bnxt_en: Fix driver init in kdump kernel

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] bnxt_kdump_reset() (drivers/net/ethernet/broadcom/bnxt/bnxt.c)
  checks that config space is reachable only after the FLR, never before…

--

bnxt_en: Fix driver init in kdump kernel

This commit adds bnxt_kdump_reset() and calls it from bnxt_init_one() in
the kdump kernel. It disables bus master and memory decode, saves PCI
state, issues an FLR, polls config space for up to 5 more seconds, clears
the BARs and restores the saved PCI state, so a PCIe switch in synthetic
mode sees the BARs reprogrammed.

> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 8ce8a82d34530..9ea7e172787e2 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -17119,6 +17119,43 @@ static void bnxt_clear_bars(struct pci_dev *pdev)
>  		pci_write_config_dword(pdev, off, 0);
>  }
>  
> +/* Clear any pending DMA transactions from crash kernel while loading driver in
> + * capture kernel.
> + */
> +static int bnxt_kdump_reset(struct pci_dev *pdev)
> +{
> +	int rc, i;
> +	u16 cmd;
> +
> +	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> +	cmd &= ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY);
> +	pci_write_config_word(pdev, PCI_COMMAND, cmd);
> +
> +	if (pci_save_state(pdev))
> +		dev_warn(&pdev->dev, "Failed to save PCI state, PCI restore may be incomplete\n");
> +
> +	rc = pcie_flr(pdev);
> +	if (rc)
> +		dev_warn(&pdev->dev, "pcie_flr() failed (rc: %d), trying to continue\n",
> +			 rc);
> +
> +	/* In case device is not returning CRS, wait 5 seconds longer */
> +	for (i = 0; i < 50; i++) {
> +		pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> +		if (!PCI_POSSIBLE_ERROR(cmd))
> +			break;
> +		msleep(100);
> +	}
> +	if (PCI_POSSIBLE_ERROR(cmd)) {
> +		dev_err(&pdev->dev, "PCI config space inaccessible after FLR, aborting\n");
> +		return -ENODEV;
> +	}
> +
> +	bnxt_clear_bars(pdev);
> +	pci_restore_state(pdev);
> +	return 0;

[Severity: Medium]
What happens in bnxt_kdump_reset() if the function is not answering config
reads when the kdump kernel probes it? One example is a chip reset that is
still running.

The first PCI_COMMAND read at the top of the function is not checked with
PCI_POSSIBLE_ERROR(). In that case cmd would be 0xffff, and the masked
value gets written back. pci_save_state() also does not check what it
reads:

drivers/pci/pci.c:pci_save_state() {
	for (i = 0; i < 16; i++) {
		pci_read_config_dword(dev, i * 4, &dev->saved_config_space[i]);
		...
	}
	dev->state_saved = true;
	...
}

So it returns 0 with an all-ones snapshot, and the "Failed to save PCI
state" warning does not fire.

pcie_flr() would then fail its pci_dev_config_accessible() check. It
returns -ENOTTY without doing the reset, and the code carries on.

If the device starts responding during the new 5 second poll,
bnxt_clear_bars() zeroes BAR0-5. pci_restore_state() then writes back
every saved dword that differs from the live value, plus the PCIe control
registers:

  - BAR0-5 and the ROM BAR get 0xffffffff instead of the assigned
    addresses
  - PCI_COMMAND becomes 0xffff, which enables memory/IO decode and bus
    master on a function that was never FLR'd
  - BIST gets 0xff
  - pci_restore_pcie_state() writes DEVCTL as 0xffff, which includes
    PCI_EXP_DEVCTL_BCR_FLR

bnxt_kdump_reset() still returns 0 in this case. bnxt_init_one() then goes
on to bnxt_init_board() and maps BARs the device no longer decodes.

Could this cause the same UR/AER errors behind the PCIe switch that this
patch is meant to fix?

Would it make sense to check PCI_POSSIBLE_ERROR(cmd) after the first read,
or to validate the saved header before restoring it? The code could then
abort or skip the save/restore.

The later patch in this series, "bnxt_en: Re-write the BARs following any
type of PCIe errors", does not change bnxt_kdump_reset(). So this seems to
remain at the end of the series.

> +}
> +

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261005204246.3822563-1-michael.chan%40broadcom.com

Re: [PATCH net v3 2/3] bnxt_en: Fix driver init in kdump kernel

[Michael Chan]

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

On Wed, Oct 7, 2026 at 1:44 PM <netdev-bot+sashiko@kernel.org> wrote:

> > +static int bnxt_kdump_reset(struct pci_dev *pdev)
> > +{
> > +     int rc, i;
> > +     u16 cmd;
> > +
> > +     pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> > +     cmd &= ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY);
> > +     pci_write_config_word(pdev, PCI_COMMAND, cmd);
> > +
> > +     if (pci_save_state(pdev))
> > +             dev_warn(&pdev->dev, "Failed to save PCI state, PCI restore may be incomplete\n");
> > +
> > +     rc = pcie_flr(pdev);
> > +     if (rc)
> > +             dev_warn(&pdev->dev, "pcie_flr() failed (rc: %d), trying to continue\n",
> > +                      rc);
> > +
> > +     /* In case device is not returning CRS, wait 5 seconds longer */
> > +     for (i = 0; i < 50; i++) {
> > +             pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> > +             if (!PCI_POSSIBLE_ERROR(cmd))
> > +                     break;
> > +             msleep(100);
> > +     }
> > +     if (PCI_POSSIBLE_ERROR(cmd)) {
> > +             dev_err(&pdev->dev, "PCI config space inaccessible after FLR, aborting\n");
> > +             return -ENODEV;
> > +     }
> > +
> > +     bnxt_clear_bars(pdev);
> > +     pci_restore_state(pdev);
> > +     return 0;
>
> [Severity: Medium]
> What happens in bnxt_kdump_reset() if the function is not answering config
> reads when the kdump kernel probes it? One example is a chip reset that is
> still running.
>
> The first PCI_COMMAND read at the top of the function is not checked with
> PCI_POSSIBLE_ERROR().

At this point, the kdump kernel has just re-enumerated our device and
successfully read at least the PCI vendor and device IDs.  We really
don't expect any problems at this point, so we don't check
PCI_POSSIBLE_ERROR().

On the hand, after FLR, it is prudent to check PCI_POSSIBLE_ERROR()
and that's why it is only done after calling pcie_flr().

So I am pushing back on this.  Thanks.

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

[PATCH net v3 3/3] bnxt_en: Re-write the BARs following any type of PCIe errors

[Michael Chan]

From: Pavan Chebbi <pavan.chebbi@broadcom.com>

Currently the driver zeroes the BARs only when fatal PCIe errors
are reported so that pci_restore_state() restores it.  However
firmware handles both fatal and non-fatal errors the same way when
it sees the slot reset resulting from the PCI_ERS_RESULT_NEED_RESET
return code from the driver.  This means that we must re-write the
BARs post recovery even during non-fatal errors.  Otherwise we will
see that every MMIO access returns all-ones and the firmware appears
dead.

Zero-out the BARs during PCIe error recovery regardless of type of
PCIe error, and make the wait after the hot reset unconditional.

Disable memory decode and bus mastering before rewriting the BARs so
the device doesn't decode a half-updated address, bailing out if
config space is still inaccessible.  Defer pci_enable_device() until
after the BAR rewrite and restore, so the device isn't re-enabled
while its BARs are still being rewritten, then re-enable the device
and re-assert bus mastering.  Guard the same Command register cleanup
on the re-enable failure path against an inaccessible device.

Skip re-enabling the device in bnxt_io_slot_reset() if it is already
enabled, so enable_cnt does not go unbalanced.  A concurrent
bnxt_fw_reset_task() can also be re-enabling the same device in its
ENABLE_DEV state, so guard that call the same way.

Fixes: f75d9a0aa967 ("bnxt_en: Re-write PCI BARs after PCI fatal error.")
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
v3:
Check that PCI is disabled before enabling it in bnxt_fw_reset_task().


v2:
Disable device before rewriting the BARs.
Improve error checking.

https://lore.kernel.org/netdev/20260928041712.3467803-10-michael.chan@broadcom.com/

v1: https://lore.kernel.org/netdev/20260831024342.2161156-5-michael.chan@broadcom.com/
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 107 ++++++++++++----------
 drivers/net/ethernet/broadcom/bnxt/bnxt.h |   1 -
 2 files changed, 60 insertions(+), 48 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 9ea7e172787e..5bd817479d64 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -15547,7 +15547,7 @@ static void bnxt_fw_reset_task(struct work_struct *work)
 		if (test_and_clear_bit(BNXT_STATE_FW_ACTIVATE_RESET, &bp->state) &&
 		    !test_bit(BNXT_STATE_FW_ACTIVATE, &bp->state))
 			bnxt_dl_remote_reload(bp);
-		if (pci_enable_device(bp->pdev)) {
+		if (!pci_is_enabled(bp->pdev) && pci_enable_device(bp->pdev)) {
 			netdev_err(bp->dev, "Cannot re-enable PCI device\n");
 			rc = -ENODEV;
 			goto fw_reset_abort;
@@ -17608,10 +17608,8 @@ static pci_ers_result_t bnxt_io_error_detected(struct pci_dev *pdev,
 	 * so we disable bus master to prevent any potential bad DMAs before
 	 * freeing kernel memory.
 	 */
-	if (state == pci_channel_io_frozen) {
-		set_bit(BNXT_STATE_PCI_CHANNEL_IO_FROZEN, &bp->state);
+	if (state == pci_channel_io_frozen)
 		bnxt_fw_fatal_close(bp);
-	}
 
 	if (netif_running(netdev))
 		__bnxt_close_nic(bp, true, true);
@@ -17641,65 +17639,80 @@ static pci_ers_result_t bnxt_io_slot_reset(struct pci_dev *pdev)
 	struct bnxt *bp = netdev_priv(netdev);
 	int retry = 0;
 	int err = 0;
+	u16 cmd;
 
 	netdev_info(bp->dev, "PCI Slot Reset\n");
 
-	if (test_bit(BNXT_STATE_PCI_CHANNEL_IO_FROZEN, &bp->state)) {
-		/* After DPC, the chip should return CRS when the vendor ID
-		 * config register is read until it is ready.  On all chips,
-		 * this is not happening reliably so add a 5-second delay as a
-		 * workaround.
-		 */
-		msleep(5000);
-	}
+	/* After a PCIe hot reset, the chip should return CRS when the
+	 * vendor ID config register is read until it is ready.  On all
+	 * chips, this is not happening reliably so add a 5-second delay
+	 * as a workaround.
+	 */
+	msleep(5000);
 
 	netdev_lock(netdev);
 
-	if (pci_enable_device(pdev)) {
+	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
+	if (PCI_POSSIBLE_ERROR(cmd)) {
 		dev_err(&pdev->dev,
-			"Cannot re-enable PCI device after reset.\n");
-	} else {
-		pci_set_master(pdev);
-		/* Upon fatal error, our device internal logic that latches to
-		 * BAR value is getting reset and will restore only upon
-		 * rewriting the BARs.
-		 *
-		 * As pci_restore_state() does not re-write the BARs if the
-		 * value is same as saved value earlier, driver needs to
-		 * write the BARs to 0 to force restore, in case of fatal error.
-		 */
-		if (test_and_clear_bit(BNXT_STATE_PCI_CHANNEL_IO_FROZEN,
-				       &bp->state))
-			bnxt_clear_bars(pdev);
-		pci_restore_state(pdev);
+			"PCI config space inaccessible after reset\n");
+		goto reset_exit;
+	}
 
-		bnxt_inv_fw_health_reg(bp);
-		bnxt_try_map_fw_health_reg(bp);
+	/* Upon PCIe error, our device internal logic that latches to
+	 * BAR value is getting reset and will restore only upon
+	 * rewriting the BARs.
+	 *
+	 * As pci_restore_state() does not re-write the BARs if the
+	 * value is same as saved value earlier, driver needs to
+	 * write the BARs to 0 to force restore.
+	 */
+	pci_clear_master(pdev);
+	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
+	cmd &= ~PCI_COMMAND_MEMORY;
+	pci_write_config_word(pdev, PCI_COMMAND, cmd);
 
-		/* In some PCIe AER scenarios, firmware may take up to
-		 * 10 seconds to become ready in the worst case.
-		 */
-		do {
-			err = bnxt_try_recover_fw(bp);
-			if (!err)
-				break;
-			retry++;
-		} while (retry < BNXT_FW_SLOT_RESET_RETRY);
+	bnxt_clear_bars(pdev);
+	pci_restore_state(pdev);
 
-		if (err) {
-			dev_err(&pdev->dev, "Firmware not ready\n");
-			goto reset_exit;
+	if (!pci_is_enabled(pdev) && pci_enable_device(pdev)) {
+		dev_err(&pdev->dev,
+			"Cannot re-enable PCI device after reset.\n");
+		pci_read_config_word(pdev, PCI_COMMAND, &cmd);
+		if (!PCI_POSSIBLE_ERROR(cmd)) {
+			cmd &= ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY);
+			pci_write_config_word(pdev, PCI_COMMAND, cmd);
 		}
+		goto reset_exit;
+	}
+	pci_set_master(pdev);
 
-		err = bnxt_hwrm_func_reset(bp);
+	bnxt_inv_fw_health_reg(bp);
+	bnxt_try_map_fw_health_reg(bp);
+
+	/* In some PCIe AER scenarios, firmware may take up to
+	 * 10 seconds to become ready in the worst case.
+	 */
+	do {
+		err = bnxt_try_recover_fw(bp);
 		if (!err)
-			result = PCI_ERS_RESULT_RECOVERED;
+			break;
+		retry++;
+	} while (retry < BNXT_FW_SLOT_RESET_RETRY);
 
-		/* IRQ will be initialized later in bnxt_io_resume */
-		bnxt_ulp_irq_stop(bp);
-		bnxt_clear_int_mode(bp);
+	if (err) {
+		dev_err(&pdev->dev, "Firmware not ready\n");
+		goto reset_exit;
 	}
 
+	err = bnxt_hwrm_func_reset(bp);
+	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);
+
 reset_exit:
 	clear_bit(BNXT_STATE_IN_FW_RESET, &bp->state);
 	bnxt_clear_reservations(bp, true);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index c673b2ce4a0d..41d05efd6307 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -2468,7 +2468,6 @@ struct bnxt {
 #define BNXT_STATE_ABORT_ERR	5
 #define BNXT_STATE_FW_FATAL_COND	6
 #define BNXT_STATE_DRV_REGISTERED	7
-#define BNXT_STATE_PCI_CHANNEL_IO_FROZEN	8
 #define BNXT_STATE_NAPI_DISABLED	9
 #define BNXT_STATE_FW_ACTIVATE		11
 #define BNXT_STATE_RECOVER		12
-- 
2.51.0

Re: [PATCH net v3 3/3] bnxt_en: Re-write the BARs following any type of PCIe errors

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 2 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The new guard `if (!pci_is_enabled(pdev) &&
  pci_enable_device(pdev))` is a non-atomic check-then-act.
- [Medium] In bnxt_io_slot_reset()
  (drivers/net/ethernet/broadcom/bnxt/bnxt.c), the slot-reset handler…
- [Medium] The commit says it disables memory decode and bus mastering
  before rewriting the BARs 'so the device doesn't decode a half-updated…

--

bnxt_en: Re-write the BARs following any type of PCIe errors

With this patch, bnxt_io_slot_reset() zeroes and restores the BARs after
every kind of PCIe error, not only fatal ones, and the 5 second wait after
the reset is now unconditional. Memory decode and bus mastering are turned
off while the BARs are rewritten, and pci_enable_device() is deferred until
after the restore. pci_enable_device() in both bnxt_io_slot_reset() and
bnxt_fw_reset_task() is now guarded by pci_is_enabled().

> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 9ea7e172787e2..5bd817479d642 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -15547,7 +15547,7 @@ static void bnxt_fw_reset_task(struct work_struct *work)
>  		if (test_and_clear_bit(BNXT_STATE_FW_ACTIVATE_RESET, &bp->state) &&
>  		    !test_bit(BNXT_STATE_FW_ACTIVATE, &bp->state))
>  			bnxt_dl_remote_reload(bp);
> -		if (pci_enable_device(bp->pdev)) {
> +		if (!pci_is_enabled(bp->pdev) && pci_enable_device(bp->pdev)) {

[Severity: High]

The commit message says bnxt_fw_reset_task() may be re-enabling the device
at the same time as bnxt_io_slot_reset(), and that this guard keeps
enable_cnt balanced. Is the check-then-enable atomic with respect to
bnxt_io_slot_reset()?

In the ENABLE_DEV state, bnxt_fw_reset_task() holds neither netdev_lock
nor any other lock that bnxt_io_slot_reset() takes. So both contexts can
see pci_is_enabled() == 0 before either one reaches the increment:

drivers/pci/pci.c:pci_enable_device_flags() {
    ...
	pci_update_current_state(dev, dev->current_state);

	if (atomic_inc_return(&dev->enable_cnt) > 1)
		return 0;		/* already enabled */
    ...
}

pci_update_current_state() does a config read before the increment, so
the window is wider than it looks.

If both callers get into pci_enable_device(), enable_cnt ends up at 2.
bnxt_io_error_detected(), bnxt_fw_reset_close() and bnxt_cleanup_pci()
each drop a single reference, behind a pci_is_enabled() check:

	if (pci_is_enabled(bp->pdev))
		pci_disable_device(bp->pdev);

drivers/pci/pci.c:pci_disable_device() {
    ...
	if (atomic_dec_return(&dev->enable_cnt) != 0)
		return;
    ...
}

In that case do_pci_disable_device() never runs. Can bus mastering then
stay on while bnxt_free_ctx_mem() and bnxt_free_mem() free DMA memory? Can
the device also stay enabled after unbind?

One way to reach this: bnxt_io_error_detected() returns DISCONNECT because
BNXT_STATE_IN_FW_RESET is already set. A sibling function then votes
NEED_RESET, merge_result() upgrades the result, and slot_reset is still
delivered to this PF while fw_reset_task is active.

Do the two recovery flows need real serialization instead? Some options:
take netdev_lock around ENABLE_DEV, cancel or flush fw_reset_task, or skip
slot_reset recovery when error_detected aborted.

[ ... ]

> @@ -17608,10 +17608,8 @@ static pci_ers_result_t bnxt_io_error_detected(struct pci_dev *pdev,
>  	 * so we disable bus master to prevent any potential bad DMAs before
>  	 * freeing kernel memory.
>  	 */
> -	if (state == pci_channel_io_frozen) {
> -		set_bit(BNXT_STATE_PCI_CHANNEL_IO_FROZEN, &bp->state);
> +	if (state == pci_channel_io_frozen)
>  		bnxt_fw_fatal_close(bp);
> -	}

[Severity: Medium]

BNXT_STATE_PCI_CHANNEL_IO_FROZEN was only set on the path where
bnxt_io_error_detected() actually quiesced the device. Once it is gone,
how does bnxt_io_slot_reset() know whether error_detected took ownership?

Earlier in bnxt_io_error_detected(), this happens when a firmware reset is
already in progress:

	if (test_and_set_bit(BNXT_STATE_IN_FW_RESET, &bp->state)) {
		netdev_err(bp->dev, "Firmware reset already in progress\n");
		abort = true;
	}

	if (abort || state == pci_channel_io_perm_failure) {
		netdev_unlock(netdev);
		return PCI_ERS_RESULT_DISCONNECT;
	}

That path returns without calling __bnxt_close_nic(),
bnxt_fw_fatal_close() or pci_disable_device().

Suppose a sibling function under the same bridge, such as the other PF of
a dual-port adapter, votes NEED_RESET. merge_result() then upgrades the
result:

drivers/pci/pcie/err.c:merge_result() {
    ...
	case PCI_ERS_RESULT_DISCONNECT:
		if (new == PCI_ERS_RESULT_NEED_RESET)
			orig = PCI_ERS_RESULT_NEED_RESET;
    ...
}

report_slot_reset() then calls bnxt_io_slot_reset() on this PF anyway.

bnxt_io_slot_reset() now always clears master and memory decode, zeroes
the BARs and calls pci_restore_state(). The device is still enabled, so
the new pci_is_enabled() guard skips the enable and execution continues.
bnxt_try_recover_fw(), bnxt_hwrm_func_reset(), bnxt_ulp_irq_stop() and
bnxt_clear_int_mode() all run, and BNXT_STATE_IN_FW_RESET is cleared,
while bnxt_fw_reset_task() still owns the device.

Before this patch the BARs were not rewritten on this path. Going by the
commit message, MMIO returned all-ones and recovery stopped at "Firmware
not ready".

Now it can return PCI_ERS_RESULT_RECOVERED, which leads to
bnxt_io_resume()->bnxt_open(). Meanwhile the OPENING state in
bnxt_fw_reset_task() may also call bnxt_open(). It can also take rtnl
there and then block on the netdev_lock that slot_reset holds.

Should error_detected record whether it quiesced the device, so that
slot_reset can bail out in the abort case (or cancel or flush
fw_reset_task)?

[ ... ]

> @@ -17641,65 +17639,80 @@ static pci_ers_result_t bnxt_io_slot_reset(struct pci_dev *pdev)

[ ... ]

> +	/* Upon PCIe error, our device internal logic that latches to
> +	 * BAR value is getting reset and will restore only upon
> +	 * rewriting the BARs.
> +	 *
> +	 * As pci_restore_state() does not re-write the BARs if the
> +	 * value is same as saved value earlier, driver needs to
> +	 * write the BARs to 0 to force restore.
> +	 */
> +	pci_clear_master(pdev);
> +	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> +	cmd &= ~PCI_COMMAND_MEMORY;
> +	pci_write_config_word(pdev, PCI_COMMAND, cmd);
>  

[ ... ]

> +	bnxt_clear_bars(pdev);
> +	pci_restore_state(pdev);

[Severity: Medium]

According to the commit message, decode and bus mastering are turned off
here "so the device doesn't decode a half-updated address". The commit
message also expects bnxt_fw_reset_task() to run at the same time. Does
the guarantee still hold in that case?

The ENABLE_DEV state in bnxt_fw_reset_task() shares no lock with
bnxt_io_slot_reset(), and it does this:

		if (!pci_is_enabled(bp->pdev) && pci_enable_device(bp->pdev)) {
			...
		}
		pci_set_master(bp->pdev);
		bp->fw_reset_state = BNXT_FW_RESET_STATE_POLL_FW;
		fallthrough;

pci_enable_device() can set PCI_COMMAND_MEMORY, and pci_set_master() sets
PCI_COMMAND_MASTER.

Suppose that lands after the PCI_COMMAND_MEMORY clear above but before
pci_restore_state() finishes. Wouldn't the function then decode memory
with the BARs zeroed by bnxt_clear_bars()? It would keep decoding while
the BARs are rewritten one dword at a time:

drivers/pci/pci.c:pci_restore_config_space() {
    ...
		pci_restore_config_space_range(pdev, 10, 15, false);
		/* Restore BARs before the command register. */
		pci_restore_config_space_range(pdev, 4, 9, false);
		pci_restore_config_space_range(pdev, 0, 3, false);
    ...
}

For a 64-bit BAR, the upper dword could be correct while the lower dword
is still zero, with bus master enabled.

The reverse can also happen. The POLL_FW_DOWN and POLL_FW states read
BAR0 through bnxt_fw_health_readl() and bnxt_hwrm_poll() without
netdev_lock. If those reads land while slot_reset has decode turned off,
would they get Unsupported Request completions and all-ones data that the
reset state machine reads as firmware status?

The new pci_is_enabled() checks only change enable_cnt bookkeeping. They
don't order the Command register and BAR writes between the two paths.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261005204246.3822563-1-michael.chan%40broadcom.com

Re: [PATCH net v3 0/3] bnxt_en: PCIe FLR/AER fixes

[Jacob Keller]

On 10/5/2026 1:42 PM, Michael Chan wrote:
> This patchset is split from the earlier 9-patch series to limit the
> fixes to the PCIe FLR/AER issues only (patches 7 to 9).  The other
> fixes (patches 1 to 6) will be sent to net-next separately.
> 
> The 1st patch is unchanged since v1.  It refactors a bnxt_clear_bars()
> helper needed by patch 2 and 3.  Patch 2 fixes an issue during driver
> init.  in the kdump kernel on a Dell system with a smart PCIe switch
> upstream from the NIC.  Patch 3 fixes an issue in the AER code path for
> non-fatal errors found by a partner.
> 
> These patches have been regression tested on Broadcom 5750X and 5760X
> NICs running 237.x.x.x production FW.
> 
> v3:
> Only include the 3 PCIe FLR/AER fixes.
> Updated to address some more Sashiko comments.
> 

The unconditional sleeps are a bit annoying, but given what you
described (and that they preexist this series and are simply expanded to
cover more cases) it makes sense.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

> v2:
> Updated to address some Sashiko comments.
> 
> https://lore.kernel.org/netdev/20260928041712.3467803-1-michael.chan@broadcom.com/
> 
> v1:
> https://lore.kernel.org/netdev/20260831024342.2161156-1-michael.chan@broadcom.com/
> 
> Michael Chan (2):
>   bnxt_en: Add bnxt_clear_bars() helper
>   bnxt_en: Fix driver init in kdump kernel
> 
> Pavan Chebbi (1):
>   bnxt_en: Re-write the BARs following any type of PCIe errors
> 
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c | 164 ++++++++++++++--------
>  drivers/net/ethernet/broadcom/bnxt/bnxt.h |   1 -
>  2 files changed, 108 insertions(+), 57 deletions(-)
> 

Re: [PATCH net v3 0/3] bnxt_en: PCIe FLR/AER fixes

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon,  5 Oct 2026 13:42:43 -0700 you wrote:
> This patchset is split from the earlier 9-patch series to limit the
> fixes to the PCIe FLR/AER issues only (patches 7 to 9).  The other
> fixes (patches 1 to 6) will be sent to net-next separately.
> 
> The 1st patch is unchanged since v1.  It refactors a bnxt_clear_bars()
> helper needed by patch 2 and 3.  Patch 2 fixes an issue during driver
> init.  in the kdump kernel on a Dell system with a smart PCIe switch
> upstream from the NIC.  Patch 3 fixes an issue in the AER code path for
> non-fatal errors found by a partner.
> 
> [...]

Here is the summary with links:
  - [net,v3,1/3] bnxt_en: Add bnxt_clear_bars() helper
    https://git.kernel.org/netdev/net/c/8f5d59b9cfa6
  - [net,v3,2/3] bnxt_en: Fix driver init in kdump kernel
    https://git.kernel.org/netdev/net/c/386f887c8290
  - [net,v3,3/3] bnxt_en: Re-write the BARs following any type of PCIe errors
    https://git.kernel.org/netdev/net/c/2e52da27096d

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html