Re: [PATCH v2 02/13] scsi: qla2xxx: Update few AENs for 29xx handling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] scsi: qla2xxx: missing IS_QLA29XX() in qla24xx and mpi RAM dump helpers
- [Medium] scsi: qla2xxx: unloaded fwdt[1] template breaks 29xx MPI dump in qla2x00_async_event()
--

commit a55d0fe512b854062fc6be4342178f19e1073032
Author: Nilesh Javali <njavali@marvell.com>

scsi: qla2xxx: Update few AENs for 29xx handling

This commit updates qla2x00_async_event() so that QLA29XX adapters follow the
same handling as QLA27XX and QLA28XX adapters for several asynchronous events,
including triggering MPI and RISC firmware dumps on MBA_SYSTEM_ERR and
MBA_IDC_AEN.

> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
> index 63b70d4abbf02..7bae26d4404c2 100644
> --- a/drivers/scsi/qla2xxx/qla_isr.c
> +++ b/drivers/scsi/qla2xxx/qla_isr.c
> @@ -1443,7 +1443,7 @@ qla2x00_async_event(scsi_qla_host_t *vha, struct rsp_que *rsp, uint16_t *mb)
>  		vha->hw_err_cnt++;
>  
>  		if (IS_QLA81XX(ha) || IS_QLA83XX(ha) ||
> -		    IS_QLA27XX(ha) || IS_QLA28XX(ha)) {
> +		    IS_QLA27XX(ha) || IS_QLA28XX(ha) || IS_QLA29XX(ha)) {
>  			u16 m[4];
>  
>  			m[0] = rd_reg_word(&reg24->mailbox4);
> @@ -1459,7 +1459,7 @@ qla2x00_async_event(scsi_qla_host_t *vha, struct rsp_que *rsp, uint16_t *mb)
>  			    "ISP System Error - mbx1=%xh mbx2=%xh mbx3=%xh.\n ",
>  			    mb[1], mb[2], mb[3]);
>  
> -		if ((IS_QLA27XX(ha) || IS_QLA28XX(ha)) &&
> +		if ((IS_QLA27XX(ha) || IS_QLA28XX(ha) || IS_QLA29XX(ha)) &&
>  		    rd_reg_word(&reg24->mailbox7) & BIT_8)
>  			ha->isp_ops->mpi_fw_dump(vha, 1);

[Severity: Medium]
Can ha->isp_ops->mpi_fw_dump(vha, 1) succeed on QLA29XX under the default
flash firmware load path (ql2xfwloadbin == 0)?

When qla29xx_load_risc() loads firmware from flash, it calls
qla29xx_load_fw_template(), which hardcodes templates = 1 and only populates
ha->fwdt[0].template:

drivers/scsi/qla2xxx/qla_init.c:qla29xx_load_fw_template() {
    struct qla_hw_data *ha = vha->hw;
    struct fwdt *fwdt = ha->fwdt;
    uint templates = 1;
    ...
}

Because ha->fwdt[1].template remains NULL, calling qla27xx_mpi_fwdump() via
ha->isp_ops->mpi_fw_dump(vha, 1) during MBA_SYSTEM_ERR (when mailbox7 BIT_8 is
set) or MBA_IDC_AEN will hit the missing template check and bail out:

drivers/scsi/qla2xxx/qla_tmpl.c:qla27xx_mpi_fwdump() {
    ...
    struct fwdt *fwdt = &ha->fwdt[1];
    ...
    if (!fwdt->template) {
        ql_log(ql_log_warn, vha, 0x02f6,
               "-> fwdt1 no template\n");
        goto bailout;
    }
    ...
}

In addition, when template entries execute for QLA29XX,
qla27xx_fwdt_entry_t263() in drivers/scsi/qla2xxx/qla_tmpl.c copies request
and response queues using sizeof(*req->ring) and sizeof(*rsp->ring) (64 bytes)
rather than qla_req_entry_size(vha->hw) and qla_rsp_entry_size(vha->hw) (128
bytes on QLA29XX):

drivers/scsi/qla2xxx/qla_tmpl.c:qla27xx_fwdt_entry_t263() {
    ...
    qla27xx_insertbuf(req ? req->ring : NULL,
        length * sizeof(*req->ring), buf, len);
    ...
    qla27xx_insertbuf(rsp ? rsp->ring : NULL,
        length * sizeof(*rsp->ring), buf, len);
    ...
}

Should qla29xx_load_fw_template() load the secondary MPI template into
ha->fwdt[1] and should qla27xx_fwdt_entry_t263() use the dynamic entry size
helpers so that QLA29XX dumps are not aborted or truncated?

[ ... ]

> @@ -1998,7 +1999,7 @@ qla2x00_async_event(scsi_qla_host_t *vha, struct rsp_que *rsp, uint16_t *mb)
>  		break;
>  
>  	case MBA_IDC_AEN:
> -		if (IS_QLA27XX(ha) || IS_QLA28XX(ha)) {
> +		if (IS_QLA27XX(ha) || IS_QLA28XX(ha) || IS_QLA29XX(ha)) {
>  			vha->hw_err_cnt++;
>  			qla27xx_handle_8200_aen(vha, mb);

[Severity: Medium]
When MBA_SYSTEM_ERR or MBA_IDC_AEN (via qla27xx_handle_8200_aen()) triggers
qla27xx_mpi_fwdump() or qla27xx_fwdump() on QLA29XX, does executing template
entries T272 (ENTRY_TYPE_RDREMRAM) and T262 (ENTRY_TYPE_RD_RAM) byte-swap all
dumped RAM words?

qla27xx_execute_fwdt_template() dispatches T272 to qla27xx_dump_mpi_ram() and
T262 to qla24xx_dump_ram(). Both helpers in drivers/scsi/qla2xxx/qla_dbg.c
check only IS_QLA27XX(ha) || IS_QLA28XX(ha) to skip swab32():

drivers/scsi/qla2xxx/qla_dbg.c:qla27xx_dump_mpi_ram() {
    ...
    ram[i + j] =
        (IS_QLA27XX(ha) || IS_QLA28XX(ha)) ?
        chunk[j] : swab32(chunk[j]);
    ...
}

drivers/scsi/qla2xxx/qla_dbg.c:qla24xx_dump_ram() {
    ...
    ram[i + j] = (__force __be32)
        ((IS_QLA27XX(ha) || IS_QLA28XX(ha)) ?
         chunk[j] : swab32(chunk[j]));
    ...
}

Should both of these checks also include IS_QLA29XX(ha) so that MPI and RISC
RAM words in QLA29XX template dumps are not byte-swapped?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023101600.1431179-1-njavali@marvell.com?part=2