From: Pavan Chebbi 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. Fixes: f75d9a0aa967 ("bnxt_en: Re-write PCI BARs after PCI fatal error.") Reviewed-by: Kalesh AP Reviewed-by: Scott Branden Signed-off-by: Pavan Chebbi Signed-off-by: Michael Chan --- v2: Disable device before rewriting the BARs. Improve error checking. v1: https://lore.kernel.org/netdev/20260831024342.2161156-5-michael.chan@broadcom.com/ --- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 105 ++++++++++++---------- drivers/net/ethernet/broadcom/bnxt/bnxt.h | 1 - 2 files changed, 59 insertions(+), 47 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index e4530b091d3b..810219d9cae2 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c @@ -17710,10 +17710,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); @@ -17743,65 +17741,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 a757d8258f71..75428a585577 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