Re: [PATCH v2 12/14] qla2xxx: Capture FW dump on MPI heartbeat stop event

[Laurence Oberman]

On Thu, 2019-09-12 at 11:09 -0700, Himanshu Madhani wrote:
> From: Quinn Tran <qut...@marvell.com>
> 
> For MPI heartbeat stop Async Event, this patch would capture
> MPI FW dump and chip reset. FW will tell which function to
> capture FW dump for.
> 
> Signed-off-by: Quinn Tran <qut...@marvell.com>
> Signed-off-by: Himanshu Madhani <hmadh...@marvell.com>
> ---
>  drivers/scsi/qla2xxx/qla_attr.c |  4 +++-
>  drivers/scsi/qla2xxx/qla_isr.c  | 31 ++++++++++++++++++++++++++-----
>  drivers/scsi/qla2xxx/qla_tmpl.c |  4 +++-
>  3 files changed, 32 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_attr.c
> b/drivers/scsi/qla2xxx/qla_attr.c
> index 8b3015361428..d6b585d303b7 100644
> --- a/drivers/scsi/qla2xxx/qla_attr.c
> +++ b/drivers/scsi/qla2xxx/qla_attr.c
> @@ -102,8 +102,10 @@ qla2x00_sysfs_write_fw_dump(struct file *filp,
> struct kobject *kobj,
>                       qla8044_idc_lock(ha);
>                       qla82xx_set_reset_owner(vha);
>                       qla8044_idc_unlock(ha);
> -             } else
> +             } else {
> +                     ha->fw_dump_mpi = 1;
>                       qla2x00_system_error(vha);
> +             }
>               break;
>       case 4:
>               if (IS_P3P_TYPE(ha)) {
> diff --git a/drivers/scsi/qla2xxx/qla_isr.c
> b/drivers/scsi/qla2xxx/qla_isr.c
> index 4c26630c1c3e..30fd2d355f3a 100644
> --- a/drivers/scsi/qla2xxx/qla_isr.c
> +++ b/drivers/scsi/qla2xxx/qla_isr.c
> @@ -1227,11 +1227,32 @@ qla2x00_async_event(scsi_qla_host_t *vha,
> struct rsp_que *rsp, uint16_t *mb)
>               break;
>  
>       case MBA_IDC_AEN:
> -             mb[4] = RD_REG_WORD(&reg24->mailbox4);
> -             mb[5] = RD_REG_WORD(&reg24->mailbox5);
> -             mb[6] = RD_REG_WORD(&reg24->mailbox6);
> -             mb[7] = RD_REG_WORD(&reg24->mailbox7);
> -             qla83xx_handle_8200_aen(vha, mb);
> +             if (IS_QLA27XX(ha) || IS_QLA28XX(ha)) {
> +                     ha->flags.fw_init_done = 0;
> +                     ql_log(ql_log_warn, vha, 0xffff,
> +                         "MPI Heartbeat stop. Chip reset needed.
> MB0[%xh] MB1[%xh] MB2[%xh] MB3[%xh]\n",
> +                         mb[0], mb[1], mb[2], mb[3]);
> +
> +                     if ((mb[1] & BIT_8) ||
> +                         (mb[2] & BIT_8)) {
> +                             ql_log(ql_log_warn, vha, 0xd013,
> +                                 "MPI Heartbeat stop. FW dump
> needed\n");
> +                             ha->fw_dump_mpi = 1;
> +                             ha->isp_ops->fw_dump(vha, 1);
> +                     }
> +                     set_bit(ISP_ABORT_NEEDED, &vha->dpc_flags);
> +                     qla2xxx_wake_dpc(vha);
> +             } else if (IS_QLA83XX(ha)) {
> +                     mb[4] = RD_REG_WORD(&reg24->mailbox4);
> +                     mb[5] = RD_REG_WORD(&reg24->mailbox5);
> +                     mb[6] = RD_REG_WORD(&reg24->mailbox6);
> +                     mb[7] = RD_REG_WORD(&reg24->mailbox7);
> +                     qla83xx_handle_8200_aen(vha, mb);
> +             } else {
> +                     ql_dbg(ql_dbg_async, vha, 0x5052,
> +                         "skip Heartbeat processing mb0-3=[0x%04x]
> [0x%04x] [0x%04x] [0x%04x]\n",
> +                         mb[0], mb[1], mb[2], mb[3]);
> +             }
>               break;
>  
>       case MBA_DPORT_DIAGNOSTICS:
> diff --git a/drivers/scsi/qla2xxx/qla_tmpl.c
> b/drivers/scsi/qla2xxx/qla_tmpl.c
> index b948d94c8b3c..5b0c057def2b 100644
> --- a/drivers/scsi/qla2xxx/qla_tmpl.c
> +++ b/drivers/scsi/qla2xxx/qla_tmpl.c
> @@ -1017,8 +1017,9 @@ qla27xx_fwdump(scsi_qla_host_t *vha, int
> hardware_locked)
>               uint j;
>               ulong len;
>               void *buf = vha->hw->fw_dump;
> +             uint count = vha->hw->fw_dump_mpi ? 2 : 1;
>  
> -             for (j = 0; j < 2; j++, fwdt++, buf += len) {
> +             for (j = 0; j < count; j++, fwdt++, buf += len) {
>                       ql_log(ql_log_warn, vha, 0xd011,
>                           "-> fwdt%u running...\n", j);
>                       if (!fwdt->template) {
> @@ -1046,6 +1047,7 @@ qla27xx_fwdump(scsi_qla_host_t *vha, int
> hardware_locked)
>       }
>  
>  bailout:
> +     vha->hw->fw_dump_mpi = 0;
>  #ifndef __CHECKER__
>       if (!hardware_locked)
>               spin_unlock_irqrestore(&vha->hw->hardware_lock, flags);

Being aware of this issue I reviewed it for sanity and code. I cant
speak to functions that interract with the firmware though.
Looks good otherwise

Reviewed-by: Laurence Oberman <lober...@redhat.com>