Re: [PATCH v6 38/56] scsi: qla2xxx: Add LS4 pass-through IOCB handling for 29xx series

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The 29xx extended NVMe LS reject path truncates 9-bit virtual port indices (vp_idx >= 256) to 8 bits, potentially causing the reject IOCB to be sent on the wrong vport.

Pre-existing issues:
- [High] The function qla_nvme_ls_reject_iocb() modifies the hardware request queue and a shared DMA buffer without holding the required queue locks, causing queue corruption.
--

--- Patch [38]: [PATCH v6 38/56] scsi: qla2xxx: Add LS4 pass-through IOCB handling for 29xx series ---
commit f75839a47ccdbe873640f70512142d6ebee38618
Author: Manish Rangankar <mrangankar@marvell.com>
Date:   Thu Jul 23 05:03:55 2026 +0000

    scsi: qla2xxx: Add LS4 pass-through IOCB handling for 29xx series
    
    Extend the LS4 pass-through IOCB handling to support the 128-byte
    pt_ls4_request_ext layout used by 29xx series adapters. The extension
    grows inline DSD capacity from 2 to 5 entries.

> diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
> index 8fa980db6ce4e..0038b6274d444 100644
> --- a/drivers/scsi/qla2xxx/qla_nvme.c
> +++ b/drivers/scsi/qla2xxx/qla_nvme.c
> @@ -1197,37 +1197,57 @@ static void qla_nvme_fc_format_rjt(void *buf, u8 ls_cmd, u8 reason,

[ ... ]

> +	if (IS_QLA29XX(ha)) {
> +		struct pt_ls4_request_ext *ext = lsrjt_iocb;
> +
> +		ext->vp_index = cpu_to_le16(a->vp_idx);

[Severity: High]
Does this truncate 9-bit virtual port indices (vp_idx >= 256)?

The struct pt_ls4_request_ext widens vp_index to a 16-bit integer, but the 
intermediate struct qla_nvme_lsrjt_pt_arg still defines vp_idx as a u8. 

Assigning a->vp_idx to ext->vp_index will drop the highest bit for any index
over 255. Will this cause the reject IOCB to be sent on the wrong vport if
an initiator sends an invalid request to a high-numbered vport?

> +		ext->tx_byte_count = cpu_to_le32(a->tx_byte_count);
> +		ext->rx_byte_count = 0;

[ ... ]

>  static int
>  qla_nvme_ls_reject_iocb(struct scsi_qla_host *vha, struct qla_qpair *qp,
>  			struct qla_nvme_lsrjt_pt_arg *a, bool is_xchg_terminate)
>  {
> -	struct pt_ls4_request *lsrjt_iocb;
> +	void *lsrjt_iocb;
>  
>  	lsrjt_iocb = __qla2x00_alloc_iocbs(qp, NULL);

[Severity: High]
This is a pre-existing issue, but does calling __qla2x00_alloc_iocbs() here
modify the hardware request queue safely?

__qla2x00_alloc_iocbs() typically requires the hardware queue lock 
(qp_lock_ptr) to be held to safely manipulate ring buffer pointers. 
However, qla_nvme_ls_reject_iocb() is called from qla2xxx_process_purls_pkt()
in a workqueue context without holding the hardware_lock or qp_lock.

Additionally, it can be called from qla24xx_process_nvme_ls4_unsol() in
interrupt context holding hardware_lock, but not qp_lock. Can this concurrent
lockless access cause queue corruption or garbled NVMe reject payloads?

>  	if (!lsrjt_iocb) {
>  		ql_log(ql_log_warn, vha, 0x210e,
>  		       "qla2x00_alloc_iocbs failed.\n");

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