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

[Laurence Oberman <loberman@xxxxxxxxxx>]

On Thu, 2019-09-12 at 11:09 -0700, Himanshu Madhani wrote:
> From: Quinn Tran <qutran@xxxxxxxxxxx>
> 
> 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 <qutran@xxxxxxxxxxx>
> Signed-off-by: Himanshu Madhani <hmadhani@xxxxxxxxxxx>
> ---
>  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 <loberman@xxxxxxxxxx>