Re: [PATCH v6 32/56] scsi: qla2xxx: Enhance task management IOCB handling for 29xx series

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] qla24xx_abort_iocb() and qla24xx_abort_command() fail to use the extended 29xx layout (abort_entry_24xx_ext) for ABORT_IOCB_TYPE commands.
- [High] qla_nvme_abort_set_option() writes to incorrect struct offsets for 29xx adapters.
--

--- Patch [32]: [PATCH v6 32/56] scsi: qla2xxx: Enhance task management IOCB handling for 29xx series ---
commit f1cddf95aa4847713d2ce2a2f2a175d85c061018
Author: Nilesh Javali <njavali@marvell.com>
Subject: scsi: qla2xxx: Enhance task management IOCB handling for 29xx series

This commit updates qla24xx_tm_iocb() and __qla24xx_issue_tmf() to support
the extended task management structure for 29xx adapters. It introduces a
common view for the IOCB header and branches on IS_QLA29XX() for diverging
port_id and vp_index assignments.

> diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
> index 4dcacee61e77e..c21d7f418f471 100644
> --- a/drivers/scsi/qla2xxx/qla_iocb.c
> +++ b/drivers/scsi/qla2xxx/qla_iocb.c
[ ... ]
> @@ -2736,26 +2736,39 @@ qla24xx_tm_iocb(srb_t *sp, struct tsk_mgmt_entry *tsk)
>  	struct qla_hw_data *ha = vha->hw;
>  	struct srb_iocb *iocb = &sp->u.iocb_cmd;
>  	struct req_que *req = sp->qpair->req;
> +	struct tsk_mgmt_entry *tsk;
>  
>  	flags = iocb->u.tmf.flags;
>  	lun = iocb->u.tmf.lun;
>  
> +	/*
> +	 * tsk_mgmt_entry_ext overlays tsk_mgmt_entry through control_flags
> +	 * (offsets 0-27 are byte-identical), so the common header writes
> +	 * go through one struct tsk_mgmt_entry * view.  The ext layout
> +	 * has no port_id and uses a wider __le16 vp_index at a different
> +	 * offset, so port_id / vp_index assignments diverge per stride.
> +	 */
> +	tsk = pkt;
>  	tsk->entry_type = TSK_MGMT_IOCB_TYPE;
>  	tsk->entry_count = 1;
>  	tsk->handle = make_handle(req->id, tsk->handle);
>  	tsk->nport_handle = cpu_to_le16(fcport->loop_id);
>  	tsk->timeout = cpu_to_le16(ha->r_a_tov / 10 * 2);
>  	tsk->control_flags = cpu_to_le32(flags);
> -	tsk->port_id[0] = fcport->d_id.b.al_pa;
> -	tsk->port_id[1] = fcport->d_id.b.area;
> -	tsk->port_id[2] = fcport->d_id.b.domain;
> -	tsk->vp_index = fcport->vha->vp_idx;
> +	if (IS_QLA29XX(ha)) {
> +		((struct tsk_mgmt_entry_ext *)pkt)->vp_index =
> +		    cpu_to_le16(fcport->vha->vp_idx);
> +	} else {
> +		tsk->port_id[0] = fcport->d_id.b.al_pa;
> +		tsk->port_id[1] = fcport->d_id.b.area;
> +		tsk->port_id[2] = fcport->d_id.b.domain;
> +		tsk->vp_index = fcport->vha->vp_idx;
> +	}

[Severity: High]
Does this same IS_QLA29XX check need to be applied to abort IOCB handling
as well?

In qla24xx_abort_iocb() and qla24xx_abort_command(), for ABORT_IOCB_TYPE
commands, vp_index is written at offset 51. Without an IS_QLA29XX check, it
misses the required offset 18 for the extended 29xx layout
(abort_entry_24xx_ext).

When an asynchronous abort is issued on a 29xx adapter, this could result in
29xx firmware reading a zeroed vp_index and an invalid port ID, potentially
leading to abort failures and host resets.

[Severity: High]
Similarly, does qla_nvme_abort_set_option() need to be updated to write to
the correct struct offsets for 29xx adapters?

It currently writes the retry count to offset 56 (drv.abts_rty_cnt) instead
of the 29xx-expected offset 24. If NVMe command timeouts trigger this path,
the aborts could miss their driver-specified retry counts and response
timeouts on 29xx adapters, corrupting other fields in the 128-byte IOCB.

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