Re: [PATCH v5 09/56] scsi: qla2xxx: Add 128-byte IOCB definitions for 29xx

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Hardware-mapped structures use C bitfields on `__le16` bitwise types, leading to memory layout bugs on big-endian architectures and violating Sparse checks.
--

--- Patch [9]: [PATCH v5 09/56] scsi: qla2xxx: Add 128-byte IOCB definitions for 29xx ---
commit 16ec74f0c14d7afa47bfe50e3dd3cef172201ab7
Author: Anil Gurumurthy <agurumurthy@marvell.com>

scsi: qla2xxx: Add 128-byte IOCB definitions for 29xx

The 29xx series uses 128-byte IOCBs instead of the 64-byte IOCBs
used by earlier adapters.  Add a new header (qla_fw29.h) with the
extended IOCB structure definitions that match the 29xx firmware
interface.

> diff --git a/drivers/scsi/qla2xxx/qla_fw29.h b/drivers/scsi/qla2xxx/qla_fw29.h
> new file mode 100644
> index 0000000000000..efe1c60bee81d
> --- /dev/null
> +++ b/drivers/scsi/qla2xxx/qla_fw29.h
> @@ -0,0 +1,686 @@
[ ... ]
> +struct els_entry_24xx_ext {
[ ... ]
> +	__le16	tx_dsd_count;
> +
> +	__le16	vp_index : 9;		/* VP Index 9bits */
> +	__le16	reserved_1_sof : 3;
> +	__le16	sof_type : 4;
> +
> +	__le32	rx_xchg_address;	/* Receive exchange address. */

[Severity: High]
Does this bitfield usage on a restricted bitwise type (__le16) break memory
layout on big-endian hosts?

The C standard leaves bitfield memory layout up to the compiler. On big-endian
architectures, bitfields are typically allocated from MSB to LSB rather than
LSB to MSB. 

Could the firmware read or write corrupted VP indices and SOF types on
big-endian hosts because the bytes won't map to the hardware's expected
layout?

Sparse also explicitly rejects bitfields on restricted bitwise types,
producing "degrading to integer" errors.

Could this use a plain __le16 field and apply bitmasks manually via
le16_to_cpu() and bitwise operations instead, similar to how it is handled
in struct cmd_type_6_ext?

[ ... ]
> +struct els_sts_entry_24xx_ext {
[ ... ]
> +	__le16	reserved_1;
> +
> +	__le16	vp_index : 9;		/* VP Index 9bits */
> +	__le16	reserved_1_sof : 3;
> +	__le16	sof_type : 4;
> +
> +	__le32	rx_xchg_address;	/* Receive exchange address. */

[Severity: High]
Will this bitfield layout also cause endianness mapping issues with the
hardware on big-endian architectures?

[ ... ]
> +struct abts_entry_24xx_ext {
[ ... ]
> +	__le16	control_flags;		/* type 0x55 only */
> +	__le16	vp_idx : 9;		/* VP index 9 bits */
> +	__le16	reserved_1_sof : 3;
> +	__le16	sof_type : 4;		/* sof_type is upper nibble */
> +
> +	__le32	rx_xch_addr;

[Severity: High]
Could this bitfield definition on __le16 similarly break hardware mapping
on big-endian hosts?

[ ... ]
> +struct vp_rpt_id_entry_24xx_ext {
[ ... ]
> +	uint8_t vp_acquired;
> +	uint8_t vp_setup;
> +	__le16	vp_idx : 9;		/* VP Index 9bits */
> +	__le16	vp_status : 7;		/* VP Status 7bits */
> +
> +	uint8_t port_id[3];

[Severity: High]
Is this C bitfield definition on __le16 also resulting in incorrect memory
layout when accessed on big-endian architectures?

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