From: Michael Chan <michael.chan@broadcom.com> Date: 2021-03-22 07:09:45
This series contains some improvements for error recovery. The main
changes are:
1. Keep better track of the health register mappings with the
"status_reliable" flag.
2. Don't wait for firmware responses if firmware is not healthy.
3. Better retry logic of the first firmware message.
4. Set the proper flag early to let the RDMA driver know that firmware
reset has been detected.
Edwin Peer (1):
bnxt_en: don't fake firmware response success when PCI is disabled
Michael Chan (3):
bnxt_en: Improve the status_reliable flag in bp->fw_health.
bnxt_en: Set BNXT_STATE_FW_RESET_DET flag earlier for the RDMA driver.
bnxt_en: Enhance retry of the first message to the firmware.
Pavan Chebbi (1):
bnxt_en: Improve wait for firmware commands completion
Scott Branden (1):
bnxt_en: check return value of bnxt_hwrm_func_resc_qcaps
Vasundhara Volam (1):
bnxt_en: Remove the read of BNXT_FW_RESET_INPROG_REG after firmware
reset.
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 108 +++++++++++++++-------
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 9 ++
2 files changed, 82 insertions(+), 35 deletions(-)
--
2.18.1
From: Michael Chan <michael.chan@broadcom.com> Date: 2021-03-22 07:09:45
From: Pavan Chebbi <pavan.chebbi@broadcom.com>
In situations where FW has crashed, the bnxt_hwrm_do_send_msg() call
will have to wait until timeout for each firmware message. This
generally takes about half a second for each firmware message. If we
try to unload the driver n this state, the unload sequence will take
a long time to complete.
Improve this by checking the health register if it is available and
abort the wait for the firmware response if the register shows that
firmware is not healthy. The very first message HWRM_VER_GET is
excluded from this check because that message is used to poll for
firmware to come out of reset during error recovery.
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 | 28 +++++++++++++++++++----
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 5 ++++
2 files changed, 29 insertions(+), 4 deletions(-)
@@ -4500,12 +4500,15 @@ static int bnxt_hwrm_do_send_msg(struct bnxt *bp, void *msg, u32 msg_len,if(test_bit(BNXT_STATE_FW_FATAL_COND,&bp->state))return-EBUSY;/* on first few passes, just barely sleep */-if(i<HWRM_SHORT_TIMEOUT_COUNTER)+if(i<HWRM_SHORT_TIMEOUT_COUNTER){usleep_range(HWRM_SHORT_MIN_TIMEOUT,HWRM_SHORT_MAX_TIMEOUT);-else+}else{+if(HWRM_WAIT_MUST_ABORT(bp,req))+break;usleep_range(HWRM_MIN_TIMEOUT,HWRM_MAX_TIMEOUT);+}}if(bp->hwrm_intr_seq_id!=(u16)~seq_id){
@@ -4530,15 +4533,19 @@ static int bnxt_hwrm_do_send_msg(struct bnxt *bp, void *msg, u32 msg_len,if(len)break;/* on first few passes, just barely sleep */-if(i<HWRM_SHORT_TIMEOUT_COUNTER)+if(i<HWRM_SHORT_TIMEOUT_COUNTER){usleep_range(HWRM_SHORT_MIN_TIMEOUT,HWRM_SHORT_MAX_TIMEOUT);-else+}else{+if(HWRM_WAIT_MUST_ABORT(bp,req))+gototimeout_abort;usleep_range(HWRM_MIN_TIMEOUT,HWRM_MAX_TIMEOUT);+}}if(i>=tmo_count){+timeout_abort:if(!silent)netdev_err(bp->dev,"Error (timeout: %d) msg {0x%x 0x%x} len:%d\n",HWRM_TOTAL_TIMEOUT(i),
From: Michael Chan <michael.chan@broadcom.com> Date: 2021-03-22 07:09:45
In order to read the firmware health status, we first need to determine
the register location and then the register may need to be mapped.
There are 2 code paths to do this. The first one is done early as a
best effort attempt by the function bnxt_try_map_fw_health_reg(). The
second one is done later in the function bnxt_map_fw_health_regs()
after establishing communications with the firmware. We currently
only set fw_health->status_reliable if we can successfully set up the
health register in the first code path.
Improve the scheme by setting the fw_health->status_reliable flag if
either (or both) code paths can successfully set up the health
register. This flag is relied upon during run-time when we need to
check the health status. So this will make it work better.
During ifdown, if the health register is mapped, we need to invalidate
the health register mapping because a potential fw reset will reset
the mapping. Similarly, we need to do the same after firmware reset
during recovery. We'll remap it during ifup.
Reviewed-by: Edwin Peer <redacted>
Reviewed-by: Vasundhara Volam <redacted>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 31 +++++++++++++++++++----
1 file changed, 26 insertions(+), 5 deletions(-)
@@ -7592,6 +7606,7 @@ static int bnxt_map_fw_health_regs(struct bnxt *bp)u32reg_base=0xffffffff;inti;+bp->fw_health->status_reliable=false;/* Only pre-map the monitoring GRC registers using window 3 */for(i=0;i<4;i++){u32reg=fw_health->regs[i];
@@ -7604,6 +7619,7 @@ static int bnxt_map_fw_health_regs(struct bnxt *bp)return-ERANGE;fw_health->mapped_regs[i]=BNXT_FW_HEALTH_WIN_OFF(reg);}+bp->fw_health->status_reliable=true;if(reg_base==0xffffffff)return0;
@@ -9556,13 +9572,17 @@ static int bnxt_hwrm_if_change(struct bnxt *bp, bool up)if(rc)returnrc;-if(!up)+if(!up){+bnxt_inv_fw_health_reg(bp);return0;+}if(flags&FUNC_DRV_IF_CHANGE_RESP_FLAGS_RESC_CHANGE)resc_reinit=true;if(flags&FUNC_DRV_IF_CHANGE_RESP_FLAGS_HOT_FW_RESET_DONE)fw_reset=true;+elseif(bp->fw_health&&!bp->fw_health->status_reliable)+bnxt_try_map_fw_health_reg(bp);if(test_bit(BNXT_STATE_IN_FW_RESET,&bp->state)&&!fw_reset){netdev_err(bp->dev,"RESET_DONE not set during FW reset.\n");
From: Michael Chan <michael.chan@broadcom.com> Date: 2021-03-22 07:09:46
From: Edwin Peer <redacted>
The original intent here is to allow commands during reset to succeed
without error when the device is disabled, to ensure that cleanup
completes normally during NIC close, where firmware is not necessarily
expected to respond.
The problem with faking success during reset's PCI disablement is that
unrelated ULP commands will also see inadvertent success during reset
when failure would otherwise be appropriate. It is better to return
a different error result such that reset related code can detect
this unique condition and ignore as appropriate.
Note, the pci_disable_device() when firmware is fatally wounded in
bnxt_fw_reset_close() does not need to be addressed, as subsequent
commands are already expected to fail due to the BNXT_NO_FW_ACCESS()
check in bnxt_hwrm_do_send_msg().
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Signed-off-by: Edwin Peer <redacted>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Michael Chan <michael.chan@broadcom.com> Date: 2021-03-22 07:09:46
Two enhancements:
1. Read the health status first before sending the first
HWRM_VER_GET message to firmware instead of the other way around.
This guarantees we got the accurate health status before we attempt
to send the message.
2. We currently only retry sending the first HWRM_VER_GET message to
the firmware if the firmware is in the process of booting. If the
firmware is in error state and is doing core dump for example, the
driver should also retry if the health register has the RECOVERING
flag set. This flag indicates the firmware will undergo recovery
soon. Modify the retry logic to retry for this case as well.
Reviewed-by: Edwin Peer <redacted>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 5 +++--
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 4 ++++
2 files changed, 7 insertions(+), 2 deletions(-)
From: Michael Chan <michael.chan@broadcom.com> Date: 2021-03-22 07:09:46
During ifup, if the driver detects that firmware has gone through a
reset, it will go through a re-probe sequence. If the RDMA driver is
loaded, the re-probe sequence includes calling the RDMA driver to stop.
We need to set the BNXT_STATE_FW_RESET_DET flag earlier so that it is
visible to the RDMA driver. The RDMA driver's stop sequence is
different if firmware has gone through a reset.
Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Reviewed-by: P B S Naresh Kumar <redacted>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
From: Michael Chan <michael.chan@broadcom.com> Date: 2021-03-22 07:09:46
From: Scott Branden <scott.branden@broadcom.com>
Check return value of call to bnxt_hwrm_func_resc_qcaps in
bnxt_hwrm_if_change and return failure on error.
Reviewed-by: Edwin Peer <redacted>
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
From: Michael Chan <michael.chan@broadcom.com> Date: 2021-03-22 07:09:46
From: Vasundhara Volam <redacted>
Once the chip goes through reset, the register mapping may be lost
and any read of the mapped health registers may return garbage value
until the registers are mapped again in the init path.
Reading BNXT_FW_RESET_INPROG_REG after firmware reset will likely
return garbage value due to the above reason. Reading this register
is for information purpose only so remove it.
Reviewed-by: Edwin Peer <redacted>
Signed-off-by: Vasundhara Volam <redacted>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 31 ++++++++---------------
1 file changed, 11 insertions(+), 20 deletions(-)
Hello:
This series was applied to netdev/net-next.git (refs/heads/master):
On Mon, 22 Mar 2021 03:08:38 -0400 you wrote:
This series contains some improvements for error recovery. The main
changes are:
1. Keep better track of the health register mappings with the
"status_reliable" flag.
2. Don't wait for firmware responses if firmware is not healthy.
[...]