Re: [PATCH v2 01/10] mpi3mr: Skip device shutdown during unload per controller configuration

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Pre-existing endianness bug when reading struct mpi3_driver_page1 fields in mpi3mr_alloc_diag_bufs().
--

--- Patch [1]: [PATCH v2 01/10] mpi3mr: Skip device shutdown during unload per controller configuration ---
commit 06e862cedaadd19cd270a091a6097fdfe30c2c71
Author: Ranjan Kumar <ranjan.kumar@broadcom.com>

mpi3mr: Skip device shutdown during unload per controller configuration

This patch allows the controller to skip device shutdown requests during
driver unload if configured in Driver Page 1. It caches this setting during
IOC initialization and properly handles endianness for the flag evaluation
to prevent failures on big-endian architectures.

Closes: https://sashiko.dev/#/patchset/20260626114109.43685-1-ranjan.kumar@broadcom.com?part=1

> diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
> index 31b19ed1528e5..59241038f689d 100644
> --- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
> +++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
[ ... ]
> @@ -4107,26 +4107,35 @@ static int mpi3mr_repost_diag_bufs(struct mpi3mr_ioc *mrioc)
[ ... ]
> +	mrioc->skip_dev_shutdown_on_unload =
> +		(le32_to_cpu(driver_pg1.flags) &
> +		 MPI3_DRIVER1_FLAGS_DEVICE_SHUTDOWN_ON_UNLOAD_DISABLE) ? 1 : 0;
>  }

[Severity: High]
This is a pre-existing issue, but since this patch corrects the endianness
for driver_pg1.flags, should similar endianness conversions be applied to
the buffer size fields read from driver_pg1 in mpi3mr_alloc_diag_bufs()?

Looking at drivers/scsi/mpi3mr/mpi3mr_app.c:mpi3mr_alloc_diag_bufs():

    trace_size = driver_pg1.host_diag_trace_max_size * 1024;
    trace_dec_size = driver_pg1.host_diag_trace_decrement_size * 1024;
    trace_min_size = driver_pg1.host_diag_trace_min_size * 1024;
    fw_size = driver_pg1.host_diag_fw_max_size * 1024;

These 16-bit little-endian fields are accessed without le16_to_cpu(), which
can result in wildly incorrect buffer size calculations on big-endian
architectures. For example, a 4KB size (0x0004) would be interpreted as
0x0400 (1024), causing memory allocation of 1MB instead of 4KB.

Are there plans to fix these missing conversions in a subsequent patch?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708183305.244485-1-ranjan.kumar@broadcom.com?part=1