* [PATCH v2 01/10] mpi3mr: Skip device shutdown during unload per controller configuration
2026-07-08 18:32 [PATCH v2 00/10] mpi3mr: Few Enhancements and minor fixes Ranjan Kumar
@ 2026-07-08 18:32 ` Ranjan Kumar
2026-07-08 18:53 ` sashiko-bot
2026-07-08 18:32 ` [PATCH v2 02/10] mpi3mr: Update MPI Headers to revision 41 Ranjan Kumar
` (8 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Ranjan Kumar @ 2026-07-08 18:32 UTC (permalink / raw)
To: linux-scsi, martin.petersen
Cc: sathya.prakash, chandrakanth.patil, vishakhavc, ipylypiv,
Ranjan Kumar, Sashiko
The controller may be configured through Driver Page 1 to suppress
device shutdown requests during driver unload. Cache this setting and
skip the device shutdown request during IOC shutdown when unloading
the driver.
Additionally, ensure the driver_pg1.flags field is properly converted
from little-endian to CPU endianness using le32_to_cpu() before
evaluating the shutdown disable flag to prevent failures on big-endian
architectures.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260626114109.43685-1-ranjan.kumar@broadcom.com?part=1
Signed-off-by: Chandrakanth Patil <chandrakanth.patil@broadcom.com>
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
drivers/scsi/mpi3mr/mpi3mr.h | 3 +++
drivers/scsi/mpi3mr/mpi3mr_fw.c | 35 ++++++++++++++++++++++++---------
drivers/scsi/mpi3mr/mpi3mr_os.c | 2 ++
3 files changed, 31 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h
index c25525fe0671..39096004c60a 100644
--- a/drivers/scsi/mpi3mr/mpi3mr.h
+++ b/drivers/scsi/mpi3mr/mpi3mr.h
@@ -1410,6 +1410,9 @@ struct mpi3mr_ioc {
struct dma_pool *trace_buf_pool;
struct segments *trace_buf;
u8 invalid_io_comp;
+ bool is_unload;
+ bool skip_dev_shutdown_on_unload;
+
};
diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
index 31b19ed1528e..59241038f689 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)
}
/**
- * mpi3mr_read_tsu_interval - Update time stamp interval
+ * mpi3mr_read_driver_page1 - Read Driver Page 1 parameters
* @mrioc: Adapter instance reference
*
- * Update time stamp interval if its defined in driver page 1,
- * otherwise use default value.
+ * Reads and caches Driver Page 1 parameters such as
+ * timestamp update interval and driver behavior flags.
*
* Return: Nothing
*/
static void
-mpi3mr_read_tsu_interval(struct mpi3mr_ioc *mrioc)
+mpi3mr_read_driver_page1(struct mpi3mr_ioc *mrioc)
{
struct mpi3_driver_page1 driver_pg1;
u16 pg_sz = sizeof(driver_pg1);
int retval = 0;
mrioc->ts_update_interval = MPI3MR_TSUPDATE_INTERVAL;
+ mrioc->skip_dev_shutdown_on_unload = 0;
retval = mpi3mr_cfg_get_driver_pg1(mrioc, &driver_pg1, pg_sz);
- if (!retval && driver_pg1.time_stamp_update)
+
+ if (retval)
+ return;
+
+ if (driver_pg1.time_stamp_update)
mrioc->ts_update_interval = (driver_pg1.time_stamp_update * 60);
+
+ mrioc->skip_dev_shutdown_on_unload =
+ (le32_to_cpu(driver_pg1.flags) &
+ MPI3_DRIVER1_FLAGS_DEVICE_SHUTDOWN_ON_UNLOAD_DISABLE) ? 1 : 0;
}
/**
@@ -4432,7 +4441,7 @@ int mpi3mr_init_ioc(struct mpi3mr_ioc *mrioc)
goto out_failed_noretry;
}
- mpi3mr_read_tsu_interval(mrioc);
+ mpi3mr_read_driver_page1(mrioc);
mpi3mr_print_ioc_info(mrioc);
dprint_init(mrioc, "allocating host diag buffers\n");
@@ -4604,7 +4613,7 @@ int mpi3mr_reinit_ioc(struct mpi3mr_ioc *mrioc, u8 is_resume)
goto out_failed_noretry;
}
- mpi3mr_read_tsu_interval(mrioc);
+ mpi3mr_read_driver_page1(mrioc);
mpi3mr_print_ioc_info(mrioc);
if (is_resume) {
@@ -5089,8 +5098,16 @@ static void mpi3mr_issue_ioc_shutdown(struct mpi3mr_ioc *mrioc)
return;
}
- shutdown_action = MPI3_SYSIF_IOC_CONFIG_SHUTDOWN_NORMAL |
- MPI3_SYSIF_IOC_CONFIG_DEVICE_SHUTDOWN_SEND_REQ;
+ shutdown_action = MPI3_SYSIF_IOC_CONFIG_SHUTDOWN_NORMAL;
+
+ if (!(mrioc->is_unload && mrioc->skip_dev_shutdown_on_unload))
+ shutdown_action |=
+ MPI3_SYSIF_IOC_CONFIG_DEVICE_SHUTDOWN_SEND_REQ;
+ else
+ ioc_info(mrioc,
+ "The shutdown request is issued without the device shutdown bit set\n"
+ "as indicated by the controller configuration\n");
+
ioc_config = readl(&mrioc->sysif_regs->ioc_configuration);
ioc_config |= shutdown_action;
diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
index 402d1f35d214..d2a20f2721db 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_os.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
@@ -5665,6 +5665,8 @@ static void mpi3mr_remove(struct pci_dev *pdev)
return;
mrioc = shost_priv(shost);
+ mrioc->is_unload = true;
+
while (mrioc->reset_in_progress || mrioc->is_driver_loading)
ssleep(1);
--
2.47.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v2 01/10] mpi3mr: Skip device shutdown during unload per controller configuration
2026-07-08 18:32 ` [PATCH v2 01/10] mpi3mr: Skip device shutdown during unload per controller configuration Ranjan Kumar
@ 2026-07-08 18:53 ` sashiko-bot
0 siblings, 0 replies; 17+ messages in thread
From: sashiko-bot @ 2026-07-08 18:53 UTC (permalink / raw)
To: Ranjan Kumar; +Cc: linux-scsi
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 02/10] mpi3mr: Update MPI Headers to revision 41
2026-07-08 18:32 [PATCH v2 00/10] mpi3mr: Few Enhancements and minor fixes Ranjan Kumar
2026-07-08 18:32 ` [PATCH v2 01/10] mpi3mr: Skip device shutdown during unload per controller configuration Ranjan Kumar
@ 2026-07-08 18:32 ` Ranjan Kumar
2026-07-08 18:32 ` [PATCH v2 03/10] mpi3mr: Add early timestamp synchronization after driver load Ranjan Kumar
` (7 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Ranjan Kumar @ 2026-07-08 18:32 UTC (permalink / raw)
To: linux-scsi, martin.petersen
Cc: sathya.prakash, chandrakanth.patil, vishakhavc, ipylypiv,
Ranjan Kumar
Update MPI Headers to revision 41
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
drivers/scsi/mpi3mr/mpi/mpi30_cnfg.h | 77 +++++++++++++++++++++--
drivers/scsi/mpi3mr/mpi/mpi30_image.h | 7 ++-
drivers/scsi/mpi3mr/mpi/mpi30_ioc.h | 15 +++--
drivers/scsi/mpi3mr/mpi/mpi30_transport.h | 2 +-
4 files changed, 87 insertions(+), 14 deletions(-)
diff --git a/drivers/scsi/mpi3mr/mpi/mpi30_cnfg.h b/drivers/scsi/mpi3mr/mpi/mpi30_cnfg.h
index 33dd303c97bb..7cf16a5c15b7 100644
--- a/drivers/scsi/mpi3mr/mpi/mpi30_cnfg.h
+++ b/drivers/scsi/mpi3mr/mpi/mpi30_cnfg.h
@@ -72,6 +72,12 @@
#define MPI3_SECURITY_PGAD_SLOT_GROUP_SHIFT (8)
#define MPI3_SECURITY_PGAD_SLOT_MASK (0x000000ff)
#define MPI3_INSTANCE_PGAD_INSTANCE_MASK (0x0000ffff)
+#define MPI3_INSTANCE_PGAD_INSTANCE_SHIFT (0)
+#define MPI3_INTERFACE_PGAD_INTERFACE_MASK (0x0000000f)
+#define MPI3_INTERFACE_PGAD_INTERFACE_SHIFT (0)
+#define MPI3_INTERFACE_PGAD_INTERFACE_MPI (0)
+#define MPI3_INTERFACE_PGAD_INTERFACE_NVME_VD (1)
+#define MPI3_INTERFACE_PGAD_INTERFACE_NVME_PD (2)
struct mpi3_config_request {
__le16 host_tag;
u8 ioc_use_only02;
@@ -492,10 +498,31 @@ struct mpi3_man10_istwi_ctrlr_entry {
};
#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_BUS_SPEED_MASK (0x000c)
-#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_BUS_SPEED_100K (0x0000)
-#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_BUS_SPEED_400K (0x0004)
+#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_BUS_SPEED_100_KHZ (0x0000)
+#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_BUS_SPEED_400_KHZ (0x0004)
#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_TARGET_ENABLED (0x0002)
#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_INITIATOR_ENABLED (0x0001)
+#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_I2C_GLITCH_FLTR_MASK (0xc000)
+#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_I2C_GLITCH_FLTR_SHIFT (14)
+#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_I2C_GLITCH_FLTR_50_NS (0x0000)
+#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_I2C_GLITCH_FLTR_10_NS (0x4000)
+#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_I2C_GLITCH_FLTR_5_NS (0x8000)
+#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_I2C_GLITCH_FLTR_0_NS (0xc000)
+#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_BUS_TYPE_MASK (0x3000)
+#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_BUS_TYPE_SHIFT (12)
+#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_BUS_TYPE_I2C (0x0000)
+#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_BUS_TYPE_I3C (0x1000)
+#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_BUS_TYPE_AUTO (0x2000)
+#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_I3C_MAX_DATA_RATE_MASK (0x0e00)
+#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_I3C_MAX_DATA_RATE_SHIFT (9)
+#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_I3C_MAX_DATA_RATE_12_5_MHZ (0x0000)
+#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_I3C_MAX_DATA_RATE_8_MHZ (0x0200)
+#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_I3C_MAX_DATA_RATE_6_MHZ (0x0400)
+#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_I3C_MAX_DATA_RATE_4_MHZ (0x0600)
+#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_I3C_MAX_DATA_RATE_2_MHZ (0x0800)
+#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_BUS_SPEED_MASK (0x000c)
+#define MPI3_MAN10_ISTWI_CTRLR_FLAGS_BUS_SPEED_SHIFT (0)
+
#ifndef MPI3_MAN10_ISTWI_CTRLR_MAX
#define MPI3_MAN10_ISTWI_CTRLR_MAX (1)
#endif
@@ -1027,6 +1054,16 @@ struct mpi3_io_unit_page5 {
#define MPI3_IOUNIT5_DEVICE_SHUTDOWN_SWITCH_ATTACHED (0x02)
#define MPI3_IOUNIT5_DEVICE_SHUTDOWN_DIRECT_AND_EXPANDER (0x03)
#define MPI3_IOUNIT5_DEVICE_SHUTDOWN_DIRECT_AND_SWITCH (0x03)
+#define MPI3_IOUNIT5_DEVICE_SHUTDOWN_HDD_SPINDOWN_MASK (0xc000)
+#define MPI3_IOUNIT5_DEVICE_SHUTDOWN_HDD_SPINDOWN_NONE (0x0000)
+#define MPI3_IOUNIT5_DEVICE_SHUTDOWN_HDD_SPINDOWN_ALL (0x4000)
+#define MPI3_IOUNIT5_DEVICE_SHUTDOWN_HDD_SPINDOWN_FILTERED (0x8000)
+#define MPI3_IOUNIT5_DEVICE_SHUTDOWN_HDD_SPINDOWN_RESERVED (0xc000)
+#define MPI3_IOUNIT5_DEVICE_SHUTDOWN_SYNC_CACHE_MASK (0x3000)
+#define MPI3_IOUNIT5_DEVICE_SHUTDOWN_SYNC_CACHE_ALL (0x0000)
+#define MPI3_IOUNIT5_DEVICE_SHUTDOWN_SYNC_CACHE_FILTERED (0x1000)
+#define MPI3_IOUNIT5_DEVICE_SHUTDOWN_SYNC_CACHE_NONE (0x2000)
+#define MPI3_IOUNIT5_DEVICE_SHUTDOWN_SYNC_CACHE_RESERVED (0x3000)
#define MPI3_IOUNIT5_DEVICE_SHUTDOWN_SATA_HDD_MASK (0x0300)
#define MPI3_IOUNIT5_DEVICE_SHUTDOWN_SATA_HDD_SHIFT (8)
#define MPI3_IOUNIT5_DEVICE_SHUTDOWN_SAS_HDD_MASK (0x00c0)
@@ -1069,7 +1106,8 @@ struct mpi3_io_unit_page8 {
struct mpi3_config_page_header header;
u8 sb_mode;
u8 sb_state;
- __le16 reserved0a;
+ u8 flags;
+ u8 reserved0b;
u8 num_slots;
u8 slots_available;
u8 current_key_encryption_algo;
@@ -1088,6 +1126,8 @@ struct mpi3_io_unit_page8 {
#define MPI3_IOUNIT8_SBSTATE_SVN_UPDATE_PENDING (0x04)
#define MPI3_IOUNIT8_SBSTATE_KEY_UPDATE_PENDING (0x02)
#define MPI3_IOUNIT8_SBSTATE_SECURE_BOOT_ENABLED (0x01)
+#define MPI3_IOUNIT8_FLAGS_FWQR_CAPABLE (0x80)
+#define MPI3_IOUNIT8_FLAGS_FWQR_SECURED (0x40)
#define MPI3_IOUNIT8_SBMODE_CURRENT_KEY_IOUNIT17 (0x10)
#define MPI3_IOUNIT8_SBMODE_HARD_SECURE_RECERTIFIED (0x08)
struct mpi3_io_unit_page9 {
@@ -1174,10 +1214,16 @@ struct mpi3_io_unit_page12 {
#define MPI3_IOUNIT12_FLAGS_NUMPASSES_32 (0x00000200)
#define MPI3_IOUNIT12_FLAGS_NUMPASSES_64 (0x00000300)
#define MPI3_IOUNIT12_FLAGS_PASSPERIOD_MASK (0x00000003)
+#define MPI3_IOUNIT12_FLAGS_PASSPERIOD_SHIFT (0)
#define MPI3_IOUNIT12_FLAGS_PASSPERIOD_DISABLED (0x00000000)
#define MPI3_IOUNIT12_FLAGS_PASSPERIOD_500US (0x00000001)
#define MPI3_IOUNIT12_FLAGS_PASSPERIOD_1MS (0x00000002)
#define MPI3_IOUNIT12_FLAGS_PASSPERIOD_2MS (0x00000003)
+#define MPI3_IOUNIT12_FLAGS_INTERFACE_MASK (0x0000000c)
+#define MPI3_IOUNIT12_FLAGS_INTERFACE_SHIFT (2)
+#define MPI3_IOUNIT12_FLAGS_INTERFACE_MPI (0x00000000)
+#define MPI3_IOUNIT12_FLAGS_INTERFACE_NVME_VD (0x00000004)
+#define MPI3_IOUNIT12_FLAGS_INTERFACE_NVME_PD (0x00000008)
#ifndef MPI3_IOUNIT13_FUNC_MAX
#define MPI3_IOUNIT13_FUNC_MAX (1)
#endif
@@ -1238,6 +1284,7 @@ struct mpi3_io_unit_page15 {
#define MPI3_IOUNIT15_PAGEVERSION (0x00)
#define MPI3_IOUNIT15_FLAGS_EPRINIT_INITREQUIRED (0x04)
#define MPI3_IOUNIT15_FLAGS_EPRSUPPORT_MASK (0x03)
+#define MPI3_IOUNIT15_FLAGS_EPRSUPPORT_SHIFT (0)
#define MPI3_IOUNIT15_FLAGS_EPRSUPPORT_NOT_SUPPORTED (0x00)
#define MPI3_IOUNIT15_FLAGS_EPRSUPPORT_WITHOUT_POWER_BRAKE_GPIO (0x01)
#define MPI3_IOUNIT15_FLAGS_EPRSUPPORT_WITH_POWER_BRAKE_GPIO (0x02)
@@ -1255,6 +1302,9 @@ struct mpi3_io_unit_page17 {
__le32 current_key[];
};
#define MPI3_IOUNIT17_PAGEVERSION (0x00)
+#define MPI3_IOUNIT17_FLAGS_KEYROOT_MASK (0x01)
+#define MPI3_IOUNIT17_FLAGS_KEYROOT_HW (0x00)
+#define MPI3_IOUNIT17_FLAGS_KEYROOT_FW (0x01)
struct mpi3_io_unit_page18 {
struct mpi3_config_page_header header;
u8 flags;
@@ -1640,11 +1690,28 @@ struct mpi3_security_page3 {
};
#define MPI3_SECURITY3_PAGEVERSION (0x00)
-#define MPI3_SECURITY3_FLAGS_TYPE_MASK (0x0f)
+#define MPI3_SECURITY3_FLAGS_TYPE_MASK (0x1f)
#define MPI3_SECURITY3_FLAGS_TYPE_SHIFT (0)
#define MPI3_SECURITY3_FLAGS_TYPE_NOT_VALID (0)
#define MPI3_SECURITY3_FLAGS_TYPE_MLDSA_PRIVATE (1)
#define MPI3_SECURITY3_FLAGS_TYPE_MLDSA_PUBLIC (2)
+union mpi3_security_digest {
+ __le32 dword[16];
+ __le16 word[32];
+ u8 byte[64];
+};
+struct mpi3_security_page4 {
+ struct mpi3_config_page_header header;
+ __le32 reserved08[2];
+ union mpi3_security_mac mac;
+ union mpi3_security_nonce nonce;
+ u8 num_digests;
+ u8 hash_algorithm;
+ __le16 reserved92;
+ __le32 reserved94[3];
+ union mpi3_security_digest digest[];
+};
+#define MPI3_SECURITY4_PAGEVERSION (0x00)
struct mpi3_security_page10 {
struct mpi3_config_page_header header;
__le32 reserved08[2];
@@ -2074,7 +2141,7 @@ struct mpi3_sas_phy3_phy_event_config {
#define MPI3_SASPHY3_EVENT_CODE_LCCONN_TIME (0xd5)
#define MPI3_SASPHY3_EVENT_CODE_SSP_TX_START_TRANSMIT (0xd6)
#define MPI3_SASPHY3_EVENT_CODE_SATA_TX_START (0xd7)
-#define MPI3_SASPHY3_EVENT_CODE_SMP_TX_START_TRANSMT (0xd8)
+#define MPI3_SASPHY3_EVENT_CODE_SMP_TX_START_TRANSMIT (0xd8)
#define MPI3_SASPHY3_EVENT_CODE_TX_SMP_BREAK_CONN (0xd9)
#define MPI3_SASPHY3_EVENT_CODE_SSP_RX_START_RECEIVE (0xda)
#define MPI3_SASPHY3_EVENT_CODE_SATA_RX_START_RECEIVE (0xdb)
diff --git a/drivers/scsi/mpi3mr/mpi/mpi30_image.h b/drivers/scsi/mpi3mr/mpi/mpi30_image.h
index 62ddf094d46c..5fa09fa79358 100644
--- a/drivers/scsi/mpi3mr/mpi/mpi30_image.h
+++ b/drivers/scsi/mpi3mr/mpi/mpi30_image.h
@@ -18,7 +18,7 @@ struct mpi3_hash_exclusion_format {
__le32 size;
};
-#define MPI3_IMAGE_HASH_EXCUSION_NUM (4)
+#define MPI3_IMAGE_HASH_EXCLUSION_NUM (4)
struct mpi3_component_image_header {
__le32 signature0;
__le32 load_address;
@@ -42,7 +42,7 @@ struct mpi3_component_image_header {
union mpi3_version_union rmc_interface_version;
union mpi3_version_union etp_interface_version;
struct mpi3_comp_image_version component_image_version;
- struct mpi3_hash_exclusion_format hash_exclusion[MPI3_IMAGE_HASH_EXCUSION_NUM];
+ struct mpi3_hash_exclusion_format hash_exclusion[MPI3_IMAGE_HASH_EXCLUSION_NUM];
__le32 next_image_header_offset;
union mpi3_version_union security_version;
__le32 reserved84[31];
@@ -347,7 +347,8 @@ struct mpi3_encrypted_hash_entry {
struct mpi3_encrypted_hash_data {
u8 image_version;
u8 num_hash;
- __le16 reserved02;
+ u8 fw_num_hash;
+ u8 reserved03;
__le32 reserved04;
struct mpi3_encrypted_hash_entry encrypted_hash_entry[MPI3_ENCRYPTED_HASH_ENTRY_MAX];
};
diff --git a/drivers/scsi/mpi3mr/mpi/mpi30_ioc.h b/drivers/scsi/mpi3mr/mpi/mpi30_ioc.h
index 68efa0d51345..aa42fba7f930 100644
--- a/drivers/scsi/mpi3mr/mpi/mpi30_ioc.h
+++ b/drivers/scsi/mpi3mr/mpi/mpi30_ioc.h
@@ -428,10 +428,10 @@ struct mpi3_event_data_sas_discovery {
#define MPI3_EVENT_SAS_DISC_FLAGS_IN_PROGRESS (0x01)
#define MPI3_EVENT_SAS_DISC_RC_STARTED (0x01)
#define MPI3_EVENT_SAS_DISC_RC_COMPLETED (0x02)
-#define MPI3_SAS_DISC_STATUS_MAX_ENCLOSURES_EXCEED (0x80000000)
-#define MPI3_SAS_DISC_STATUS_MAX_EXPANDERS_EXCEED (0x40000000)
-#define MPI3_SAS_DISC_STATUS_MAX_DEVICES_EXCEED (0x20000000)
-#define MPI3_SAS_DISC_STATUS_MAX_TOPO_PHYS_EXCEED (0x10000000)
+#define MPI3_SAS_DISC_STATUS_MAX_ENCLOSURES_EXCEEDED (0x80000000)
+#define MPI3_SAS_DISC_STATUS_MAX_EXPANDERS_EXCEEDED (0x40000000)
+#define MPI3_SAS_DISC_STATUS_MAX_DEVICES_EXCEEDED (0x20000000)
+#define MPI3_SAS_DISC_STATUS_MAX_TOPO_PHYS_EXCEEDED (0x10000000)
#define MPI3_SAS_DISC_STATUS_INVALID_CEI (0x00010000)
#define MPI3_SAS_DISC_STATUS_FECEI_MISMATCH (0x00008000)
#define MPI3_SAS_DISC_STATUS_MULTIPLE_DEVICES_IN_SLOT (0x00004000)
@@ -965,7 +965,7 @@ struct mpi3_ci_download_reply {
u8 flags;
u8 cache_dirty;
u8 pending_count;
- u8 reserved13;
+ u8 additional_flags;
};
#define MPI3_CI_DOWNLOAD_FLAGS_DOWNLOAD_IN_PROGRESS (0x80)
@@ -979,6 +979,11 @@ struct mpi3_ci_download_reply {
#define MPI3_CI_DOWNLOAD_FLAGS_ACTIVATION_STATUS_ONLINE_PENDING (0x04)
#define MPI3_CI_DOWNLOAD_FLAGS_ACTIVATION_STATUS_OFFLINE_PENDING (0x06)
#define MPI3_CI_DOWNLOAD_FLAGS_COMPATIBLE (0x01)
+#define MPI3_CI_DOWNLOAD_ADDITIONALFLAGS_REDUNDANCYRESTORATION_MASK (0x03)
+#define MPI3_CI_DOWNLOAD_ADDITIONALFLAGS_REDUNDANCYRESTORATION_SHIFT (0)
+#define MPI3_CI_DOWNLOAD_ADDITIONALFLAGS_REDUNDANCYRESTORATION_NONE (0x00)
+#define MPI3_CI_DOWNLOAD_ADDITIONALFLAGS_REDUNDANCYRESTORATION_PRIMARY (0x01)
+#define MPI3_CI_DOWNLOAD_ADDITIONALFLAGS_REDUNDANCYRESTORATION_SECONDARY (0x02)
struct mpi3_ci_upload_request {
__le16 host_tag;
u8 ioc_use_only02;
diff --git a/drivers/scsi/mpi3mr/mpi/mpi30_transport.h b/drivers/scsi/mpi3mr/mpi/mpi30_transport.h
index 290a1f5c2924..794ecc778945 100644
--- a/drivers/scsi/mpi3mr/mpi/mpi30_transport.h
+++ b/drivers/scsi/mpi3mr/mpi/mpi30_transport.h
@@ -18,7 +18,7 @@ union mpi3_version_union {
#define MPI3_VERSION_MAJOR (3)
#define MPI3_VERSION_MINOR (0)
-#define MPI3_VERSION_UNIT (39)
+#define MPI3_VERSION_UNIT (41)
#define MPI3_VERSION_DEV (0)
#define MPI3_DEVHANDLE_INVALID (0xffff)
struct mpi3_sysif_oper_queue_indexes {
--
2.47.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 03/10] mpi3mr: Add early timestamp synchronization after driver load
2026-07-08 18:32 [PATCH v2 00/10] mpi3mr: Few Enhancements and minor fixes Ranjan Kumar
2026-07-08 18:32 ` [PATCH v2 01/10] mpi3mr: Skip device shutdown during unload per controller configuration Ranjan Kumar
2026-07-08 18:32 ` [PATCH v2 02/10] mpi3mr: Update MPI Headers to revision 41 Ranjan Kumar
@ 2026-07-08 18:32 ` Ranjan Kumar
2026-07-08 18:32 ` [PATCH v2 04/10] mpi3mr: Fix NVMe page size caching for non-operational devices Ranjan Kumar
` (6 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Ranjan Kumar @ 2026-07-08 18:32 UTC (permalink / raw)
To: linux-scsi, martin.petersen
Cc: sathya.prakash, chandrakanth.patil, vishakhavc, ipylypiv,
Ranjan Kumar
When the driver is loaded from initramfs, the controller timestamp may
be initialized before the system clock has been synchronized. As a
result, the controller can operate with a stale timestamp until the
first periodic synchronization occurs.
Currently, the first controller timestamp synchronization occurs only
after the configured ts_update_interval expires (15 minutes by default).
Add an early timestamp synchronization 60 seconds after driver load,
followed by the existing periodic synchronization interval.
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
drivers/scsi/mpi3mr/mpi3mr.h | 3 +++
drivers/scsi/mpi3mr/mpi3mr_fw.c | 25 +++++++++++++++++++------
2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h
index 39096004c60a..1f2f0951b560 100644
--- a/drivers/scsi/mpi3mr/mpi3mr.h
+++ b/drivers/scsi/mpi3mr/mpi3mr.h
@@ -125,6 +125,7 @@ extern atomic64_t event_counter;
#define MPI3MR_RESETTM_TIMEOUT 60
#define MPI3MR_RESET_HOST_IOWAIT_TIMEOUT 5
#define MPI3MR_TSUPDATE_INTERVAL 900
+#define MPI3MR_EARLY_TSUPDATE_SECONDS 60
#define MPI3MR_DEFAULT_SHUTDOWN_TIME 120
#define MPI3MR_RAID_ERRREC_RESET_TIMEOUT 180
#define MPI3MR_PREPARE_FOR_RESET_TIMEOUT 180
@@ -1118,6 +1119,7 @@ struct scmd_priv {
* @evtack_cmds_bitmap: Event Ack bitmap
* @delayed_evtack_cmds_list: Delayed event acknowledgment list
* @ts_update_counter: Timestamp update counter
+ * @early_ts_sync_done: Early (1 min) timestamp sync completed after load
* @ts_update_interval: Timestamp update interval
* @reset_in_progress: Reset in progress flag
* @unrecoverable: Controller unrecoverable flag
@@ -1318,6 +1320,7 @@ struct mpi3mr_ioc {
struct list_head delayed_evtack_cmds_list;
u16 ts_update_counter;
+ u8 early_ts_sync_done;
u16 ts_update_interval;
u8 reset_in_progress;
u8 unrecoverable;
diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
index 59241038f689..434b66f7b502 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
@@ -2870,8 +2870,9 @@ static int mpi3mr_print_pkg_ver(struct mpi3mr_ioc *mrioc)
* @work: work struct
*
* Watch dog work periodically executed (1 second interval) to
- * monitor firmware fault and to issue periodic timer sync to
- * the firmware.
+ * monitor firmware fault and perform timestamp synchronization
+ * to firmware, with an early sync 1 minute after load followed
+ * by periodic updates at ts_update_interval seconds (default 15 minutes).
*
* Return: Nothing.
*/
@@ -2917,11 +2918,23 @@ static void mpi3mr_watchdog_work(struct work_struct *work)
}
if (!(mrioc->facts.ioc_capabilities &
- MPI3_IOCFACTS_CAPABILITY_NON_SUPERVISOR_IOC) &&
- (mrioc->ts_update_counter++ >= mrioc->ts_update_interval)) {
+ MPI3_IOCFACTS_CAPABILITY_NON_SUPERVISOR_IOC)) {
+ if (!mrioc->early_ts_sync_done) {
+ /*
+ * Send time sync 1 min after load
+ */
+ if (mrioc->ts_update_counter++ >=
+ MPI3MR_EARLY_TSUPDATE_SECONDS) {
+ mrioc->early_ts_sync_done = 1;
+ mrioc->ts_update_counter = 0;
+ mpi3mr_sync_timestamp(mrioc);
+ }
+ } else if (mrioc->ts_update_counter++ >=
+ mrioc->ts_update_interval) {
+ mrioc->ts_update_counter = 0;
+ mpi3mr_sync_timestamp(mrioc);
+ }
- mrioc->ts_update_counter = 0;
- mpi3mr_sync_timestamp(mrioc);
}
if ((mrioc->prepare_for_reset) &&
--
2.47.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 04/10] mpi3mr: Fix NVMe page size caching for non-operational devices
2026-07-08 18:32 [PATCH v2 00/10] mpi3mr: Few Enhancements and minor fixes Ranjan Kumar
` (2 preceding siblings ...)
2026-07-08 18:32 ` [PATCH v2 03/10] mpi3mr: Add early timestamp synchronization after driver load Ranjan Kumar
@ 2026-07-08 18:32 ` Ranjan Kumar
2026-07-08 19:18 ` sashiko-bot
2026-07-08 18:33 ` [PATCH v2 05/10] mpi3mr: Fix performance regression caused by extended IRQ poll sleep Ranjan Kumar
` (5 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Ranjan Kumar @ 2026-07-08 18:32 UTC (permalink / raw)
To: linux-scsi, martin.petersen
Cc: sathya.prakash, chandrakanth.patil, vishakhavc, ipylypiv,
Ranjan Kumar, Sashiko
For NVMe devices reported with an error access status,
the cached PCIe page size may remain unset during device discovery.
This causes management IOCTL validation to fail, preventing requests
from reaching firmware and resulting in an incorrect error
being returned to user space.
Populate the page size attribute irrespective of device access status
so that management IOCTLs are processed by firmware and the appropriate
device-specific error is reported.
Additionally, add bounds checking for the firmware-provided page_size.
If the device is in an error state, the firmware might return invalid
data. Unvalidated values could lead to undefined behavior or kernel
panics during later bitwise shift operations. Fall back to a default
page size of 4096 bytes (shift exponent 12) if the value is invalid.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260626114109.43685-1-ranjan.kumar@broadcom.com?part=4
Signed-off-by: Chandrakanth Patil <chandrakanth.patil@broadcom.com>
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
drivers/scsi/mpi3mr/mpi3mr_os.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
index d2a20f2721db..df7365d19b44 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_os.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
@@ -1354,12 +1354,14 @@ static void mpi3mr_update_tgtdev(struct mpi3mr_ioc *mrioc,
tgtdev->dev_spec.pcie_inf.capb =
le32_to_cpu(pcieinf->capabilities);
tgtdev->dev_spec.pcie_inf.mdts = MPI3MR_DEFAULT_MDTS;
- /* 2^12 = 4096 */
- tgtdev->dev_spec.pcie_inf.pgsz = 12;
+ /* Validate firmware page size to prevent undefined shift behavior */
+ if (pcieinf->page_size > 0 && pcieinf->page_size < 31)
+ tgtdev->dev_spec.pcie_inf.pgsz = pcieinf->page_size;
+ else
+ tgtdev->dev_spec.pcie_inf.pgsz = 12; /* Default to 4096 (2^12) */
if (dev_pg0->access_status == MPI3_DEVICE0_ASTATUS_NO_ERRORS) {
tgtdev->dev_spec.pcie_inf.mdts =
le32_to_cpu(pcieinf->maximum_data_transfer_size);
- tgtdev->dev_spec.pcie_inf.pgsz = pcieinf->page_size;
tgtdev->dev_spec.pcie_inf.reset_to =
max_t(u8, pcieinf->controller_reset_to,
MPI3MR_INTADMCMD_TIMEOUT);
--
2.47.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v2 04/10] mpi3mr: Fix NVMe page size caching for non-operational devices
2026-07-08 18:32 ` [PATCH v2 04/10] mpi3mr: Fix NVMe page size caching for non-operational devices Ranjan Kumar
@ 2026-07-08 19:18 ` sashiko-bot
0 siblings, 0 replies; 17+ messages in thread
From: sashiko-bot @ 2026-07-08 19:18 UTC (permalink / raw)
To: Ranjan Kumar; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The `reset_to` and `abort_to` fields are left uninitialized for PCIe devices in error states, causing task management commands to time out instantly and trigger unintended full controller resets.
--
--- Patch [4]: [PATCH v2 04/10] mpi3mr: Fix NVMe page size caching for non-operational devices ---
commit a648ff0d4d5627aa485822e3033940c480028902
Author: Ranjan Kumar <ranjan.kumar@broadcom.com>
mpi3mr: Fix NVMe page size caching for non-operational devices
This patch populates the PCIe page size attribute for NVMe devices regardless
of their access status, ensuring management IOCTLs can reach the firmware.
It also adds bounds checking for the firmware-provided page size to prevent
undefined shift behavior, falling back to a default size if invalid.
Closes: https://sashiko.dev/#/patchset/20260626114109.43685-1-ranjan.kumar@broadcom.com?part=4
> diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
> index d2a20f2721dbf..df7365d19b44a 100644
> --- a/drivers/scsi/mpi3mr/mpi3mr_os.c
> +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
> @@ -1354,12 +1354,14 @@ static void mpi3mr_update_tgtdev(struct mpi3mr_ioc *mrioc,
> tgtdev->dev_spec.pcie_inf.capb =
> le32_to_cpu(pcieinf->capabilities);
> tgtdev->dev_spec.pcie_inf.mdts = MPI3MR_DEFAULT_MDTS;
> - /* 2^12 = 4096 */
> - tgtdev->dev_spec.pcie_inf.pgsz = 12;
> + /* Validate firmware page size to prevent undefined shift behavior */
> + if (pcieinf->page_size > 0 && pcieinf->page_size < 31)
> + tgtdev->dev_spec.pcie_inf.pgsz = pcieinf->page_size;
> + else
> + tgtdev->dev_spec.pcie_inf.pgsz = 12; /* Default to 4096 (2^12) */
> if (dev_pg0->access_status == MPI3_DEVICE0_ASTATUS_NO_ERRORS) {
> tgtdev->dev_spec.pcie_inf.mdts =
> le32_to_cpu(pcieinf->maximum_data_transfer_size);
> - tgtdev->dev_spec.pcie_inf.pgsz = pcieinf->page_size;
> tgtdev->dev_spec.pcie_inf.reset_to =
> max_t(u8, pcieinf->controller_reset_to,
> MPI3MR_INTADMCMD_TIMEOUT);
[Severity: High]
This is a pre-existing issue, but does exposing the device to IOCTLs by
setting the page size here unintentionally turn a device error into a full
controller reset?
When an NVMe device is in an error state (e.g., NEEDS_INITIALIZATION),
access_status != NO_ERRORS, so reset_to and abort_to are left uninitialized (0).
With this patch populating pgsz for these devices, management IOCTLs can pass
the driver's PRP validation and reach the firmware. If the IOCTL times out
because the device is in an error state, the driver attempts recovery:
drivers/scsi/mpi3mr/mpi3mr_os.c:mpi3mr_issue_tm() {
...
if (tgtdev) {
if (tgtdev->dev_type == MPI3_DEVICE_DEVFORM_PCIE)
timeout = cmd_priv ? tgtdev->dev_spec.pcie_inf.abort_to
: tgtdev->dev_spec.pcie_inf.reset_to;
...
}
Since reset_to and abort_to are 0, wait_for_completion_timeout() fails
instantly, causing the driver to falsely detect a TM timeout and
unconditionally execute mpi3mr_soft_reset_handler(), resetting the entire
controller.
Should reset_to and abort_to also be initialized with defaults even when the
device is in an error state to prevent routine monitoring tools from triggering
an HBA reset?
> tgtdev->dev_spec.pcie_inf.abort_to =
> max_t(u8, pcieinf->nvme_abort_to,
> MPI3MR_INTADMCMD_TIMEOUT);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708183305.244485-1-ranjan.kumar@broadcom.com?part=4
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 05/10] mpi3mr: Fix performance regression caused by extended IRQ poll sleep
2026-07-08 18:32 [PATCH v2 00/10] mpi3mr: Few Enhancements and minor fixes Ranjan Kumar
` (3 preceding siblings ...)
2026-07-08 18:32 ` [PATCH v2 04/10] mpi3mr: Fix NVMe page size caching for non-operational devices Ranjan Kumar
@ 2026-07-08 18:33 ` Ranjan Kumar
2026-07-08 19:30 ` sashiko-bot
2026-07-08 18:33 ` [PATCH v2 06/10] mpi3mr: Fix memory leak on operational queue creation failure Ranjan Kumar
` (4 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Ranjan Kumar @ 2026-07-08 18:33 UTC (permalink / raw)
To: linux-scsi, martin.petersen
Cc: sathya.prakash, chandrakanth.patil, vishakhavc, ipylypiv,
Ranjan Kumar, Sashiko
Commit 24d7071d9645 ("scsi: mpi3mr: A performance fix") increased the
threaded IRQ poll sleep range from 2-20 us to 20-21 us to work around a
timer slack issue.
On kernels unaffected by the timer slack issue, the longer sleep interval
reduces reply queue processing efficiency and causes an approximately 7%
throughput regression on NVMe direct-attached RAID10 configurations.
Restore the IRQ poll sleep range to 2-20 us to recover the lost
throughput.
Additionally, add missing dma_rmb() memory barriers in the admin and
operational reply queue processing loops. This ensures that the descriptor
payload is only read after the phase bit check is complete, preventing
weakly ordered architectures from speculatively processing stale data.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260626114109.43685-1-ranjan.kumar@broadcom.com?part=5
Signed-off-by: Chandrakanth Patil <chandrakanth.patil@broadcom.com>
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
drivers/scsi/mpi3mr/mpi3mr.h | 2 +-
drivers/scsi/mpi3mr/mpi3mr_fw.c | 27 ++++++++++++++++++++++++++-
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h
index 1f2f0951b560..1d11d7c69536 100644
--- a/drivers/scsi/mpi3mr/mpi3mr.h
+++ b/drivers/scsi/mpi3mr/mpi3mr.h
@@ -178,7 +178,7 @@ extern atomic64_t event_counter;
#define MPI3MR_DEFAULT_SDEV_QD 32
/* Definitions for Threaded IRQ poll*/
-#define MPI3MR_IRQ_POLL_SLEEP 20
+#define MPI3MR_IRQ_POLL_SLEEP 2
#define MPI3MR_IRQ_POLL_TRIGGER_IOCOUNT 8
/* Definitions for the controller security status*/
diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
index 434b66f7b502..2f787fa36ffd 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
@@ -473,6 +473,12 @@ int mpi3mr_process_admin_reply_q(struct mpi3mr_ioc *mrioc)
return 0;
}
+ /*
+ * Ensure that the descriptor payload is read only after
+ * the phase bit check is complete.
+ */
+ dma_rmb();
+
do {
if (mrioc->unrecoverable || mrioc->io_admin_reset_sync)
break;
@@ -493,6 +499,13 @@ int mpi3mr_process_admin_reply_q(struct mpi3mr_ioc *mrioc)
if ((le16_to_cpu(reply_desc->reply_flags) &
MPI3_REPLY_DESCRIPT_FLAGS_PHASE_MASK) != exp_phase)
break;
+
+ /*
+ * Ensure that the descriptor payload is read only after
+ * the phase bit check is complete.
+ */
+ dma_rmb();
+
if (threshold_comps == MPI3MR_THRESHOLD_REPLY_COUNT) {
writel(admin_reply_ci,
&mrioc->sysif_regs->admin_reply_queue_ci);
@@ -568,6 +581,12 @@ int mpi3mr_process_op_reply_q(struct mpi3mr_ioc *mrioc,
return 0;
}
+ /*
+ * Ensure that the descriptor payload is read only after
+ * the phase bit check is complete.
+ */
+ dma_rmb();
+
do {
if (mrioc->unrecoverable || mrioc->io_admin_reset_sync)
break;
@@ -594,6 +613,12 @@ int mpi3mr_process_op_reply_q(struct mpi3mr_ioc *mrioc,
if ((le16_to_cpu(reply_desc->reply_flags) &
MPI3_REPLY_DESCRIPT_FLAGS_PHASE_MASK) != exp_phase)
break;
+
+ /*
+ * Ensure that the descriptor payload is read only after
+ * the phase bit check is complete.
+ */
+ dma_rmb();
#ifndef CONFIG_PREEMPT_RT
/*
* Exit completion loop to avoid CPU lockup
@@ -744,7 +769,7 @@ static irqreturn_t mpi3mr_isr_poll(int irq, void *privdata)
mpi3mr_process_op_reply_q(mrioc,
intr_info->op_reply_q);
- usleep_range(MPI3MR_IRQ_POLL_SLEEP, MPI3MR_IRQ_POLL_SLEEP + 1);
+ usleep_range(MPI3MR_IRQ_POLL_SLEEP, 10 * MPI3MR_IRQ_POLL_SLEEP);
} while (atomic_read(&intr_info->op_reply_q->pend_ios) &&
(num_op_reply < mrioc->max_host_ios));
--
2.47.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v2 05/10] mpi3mr: Fix performance regression caused by extended IRQ poll sleep
2026-07-08 18:33 ` [PATCH v2 05/10] mpi3mr: Fix performance regression caused by extended IRQ poll sleep Ranjan Kumar
@ 2026-07-08 19:30 ` sashiko-bot
0 siblings, 0 replies; 17+ messages in thread
From: sashiko-bot @ 2026-07-08 19:30 UTC (permalink / raw)
To: Ranjan Kumar; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [Critical] The driver extracts `request_queue_id` from the hardware-populated descriptor and uses it as an array index without bounds checking, leading to out-of-bounds memory corruption.
- [High] A TOCTOU race in `mpi3mr_process_op_reply_q` can cause operational reply descriptors to be missed, leading to IO stalls and timeouts.
- [High] The threaded IRQ polling routine `mpi3mr_isr_poll` enters an unbounded busy-sleep loop waiting for slow hardware IOs to complete, causing severe CPU consumption.
- [High] An invalid `reply_frame_address` provided in a DMA reply descriptor triggers a direct kernel panic.
--
--- Patch [5]: [PATCH v2 05/10] mpi3mr: Fix performance regression caused by extended IRQ poll sleep ---
commit 6c88e5d170a116451ea90d4d4dc8870ab7372f57
Author: Ranjan Kumar <ranjan.kumar@broadcom.com>
mpi3mr: Fix performance regression caused by extended IRQ poll sleep
This commit reverts a previous change to the threaded IRQ poll sleep
interval, restoring it to 2-20 us to fix a 7% throughput regression
on NVMe direct-attached RAID10 configurations. It also adds missing
dma_rmb() barriers in the admin and operational reply queue loops
to ensure phase bit checks complete before payloads are read.
Closes: https://sashiko.dev/#/patchset/20260626114109.43685-1-ranjan.kumar@broadcom.com?part=5
> diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
> index 434b66f7b502d..2f787fa36ffdb 100644
> --- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
> +++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
[ ... ]
> @@ -568,6 +581,12 @@ int mpi3mr_process_op_reply_q(struct mpi3mr_ioc *mrioc,
> return 0;
> }
>
> + /*
> + * Ensure that the descriptor payload is read only after
> + * the phase bit check is complete.
> + */
> + dma_rmb();
> +
> do {
> if (mrioc->unrecoverable || mrioc->io_admin_reset_sync)
> break;
[Severity: Critical]
This is a pre-existing issue, but does this code bounds check the queue ID
before using it as an array index?
In mpi3mr_process_op_reply_q(), the driver computes req_q_idx from
device-supplied DMA memory:
req_q_idx = le16_to_cpu(reply_desc->request_queue_id) - 1;
op_req_q = &mrioc->req_qinfo[req_q_idx];
WRITE_ONCE(op_req_q->ci, le16_to_cpu(reply_desc->request_queue_ci));
If the hardware provides an invalid queue ID (such as 0, underflowing to
65535, or a value larger than mrioc->num_op_req_q), could this lead to an
out-of-bounds memory write to the host kernel heap?
[Severity: High]
This isn't a bug introduced by this patch, but does
mpi3mr_process_op_reply_desc() safely handle malformed DMA reply addresses?
If a malicious or faulty PCIe device sends an invalid reply_frame_address,
mpi3mr_get_reply_virt_addr() will return NULL, triggering a panic:
scsi_reply = mpi3mr_get_reply_virt_addr(mrioc,
*reply_dma);
if (!scsi_reply) {
panic("%s: scsi_reply is NULL, this shouldn't happen\n",
mrioc->name);
goto out;
}
Could a compromised device or hypervisor repeatedly send crafted DMA frames
containing invalid addresses to intentionally crash the host system?
[ ... ]
> @@ -594,6 +613,12 @@ int mpi3mr_process_op_reply_q(struct mpi3mr_ioc *mrioc,
> if ((le16_to_cpu(reply_desc->reply_flags) &
> MPI3_REPLY_DESCRIPT_FLAGS_PHASE_MASK) != exp_phase)
> break;
> +
> + /*
> + * Ensure that the descriptor payload is read only after
> + * the phase bit check is complete.
> + */
> + dma_rmb();
[Severity: High]
This is a pre-existing issue, but is there a TOCTOU race condition when
finishing descriptor processing in mpi3mr_process_op_reply_q()?
If the active thread breaks out of the loop after checking the phase bit,
but before it executes:
atomic_dec(&op_reply_q->in_use);
And the hardware writes a new descriptor and triggers an MSI-X interrupt in
that tiny window, the interrupt handler will fail the atomic_add_unless()
check:
if (!atomic_add_unless(&op_reply_q->in_use, 1, 1))
return 0;
Will this permanently drop the interrupt without re-checking for newly
arrived descriptors, potentially stalling the queue until another unrelated
command completes?
[ ... ]
> @@ -744,7 +769,7 @@ static irqreturn_t mpi3mr_isr_poll(int irq, void *privdata)
> mpi3mr_process_op_reply_q(mrioc,
> intr_info->op_reply_q);
>
> - usleep_range(MPI3MR_IRQ_POLL_SLEEP, MPI3MR_IRQ_POLL_SLEEP + 1);
> + usleep_range(MPI3MR_IRQ_POLL_SLEEP, 10 * MPI3MR_IRQ_POLL_SLEEP);
>
> } while (atomic_read(&intr_info->op_reply_q->pend_ios) &&
> (num_op_reply < mrioc->max_host_ios));
[Severity: High]
This isn't a bug introduced by this patch, but does this threaded IRQ polling
loop busy-wait unboundedly for slow hardware IOs?
When pend_ios exceeds 8, mpi3mr_isr_poll is awoken. The thread loops and
continuously executes usleep_range():
do {
...
usleep_range(MPI3MR_IRQ_POLL_SLEEP, 10 * MPI3MR_IRQ_POLL_SLEEP);
} while (atomic_read(&intr_info->op_reply_q->pend_ios) &&
(num_op_reply < mrioc->max_host_ios));
If multiple slow IOs are pending, will this keep the CPU occupied with
thousands of context switches, rather than going to sleep and waiting for
the actual hardware interrupt when the async IO finishes?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708183305.244485-1-ranjan.kumar@broadcom.com?part=5
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 06/10] mpi3mr: Fix memory leak on operational queue creation failure
2026-07-08 18:32 [PATCH v2 00/10] mpi3mr: Few Enhancements and minor fixes Ranjan Kumar
` (4 preceding siblings ...)
2026-07-08 18:33 ` [PATCH v2 05/10] mpi3mr: Fix performance regression caused by extended IRQ poll sleep Ranjan Kumar
@ 2026-07-08 18:33 ` Ranjan Kumar
2026-07-08 19:48 ` sashiko-bot
2026-07-08 18:33 ` [PATCH v2 07/10] mpi3mr: Fix firmware event reference leak during cleanup Ranjan Kumar
` (3 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Ranjan Kumar @ 2026-07-08 18:33 UTC (permalink / raw)
To: linux-scsi, martin.petersen
Cc: sathya.prakash, chandrakanth.patil, vishakhavc, ipylypiv,
Ranjan Kumar, Sashiko
When operational queue creation fails after one or more queues have
been created, the error path frees the queue information arrays but
does not release the DMA memory segments associated with the created
queues, resulting in a memory leak.
Fix the memory leak by ensuring that partially allocated segments are
freed immediately if a queue fails to create.
Additionally, harden the error handling path to fix two potential
kernel panics:
1. Prevent a NULL pointer dereference by ensuring the queue information
arrays are successfully allocated before attempting to free their
associated segments.
2. Ensure the operational queue counts are reset to zero on failure,
preventing a deferred kernel panic during driver cleanup if queue
creation fails during a controller reset.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260626114109.43685-1-ranjan.kumar@broadcom.com?part=6
Signed-off-by: Chandrakanth Patil <chandrakanth.patil@broadcom.com>
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
drivers/scsi/mpi3mr/mpi3mr_fw.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
index 2f787fa36ffd..4b9fff176040 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
@@ -2481,7 +2481,7 @@ static int mpi3mr_create_op_req_q(struct mpi3mr_ioc *mrioc, u16 idx,
static int mpi3mr_create_op_queues(struct mpi3mr_ioc *mrioc)
{
int retval = 0;
- u16 num_queues = 0, i = 0, msix_count_op_q = 1;
+ u16 num_queues = 0, i = 0, j = 0, msix_count_op_q = 1;
u32 ioc_status;
enum mpi3mr_iocstate ioc_state;
@@ -2533,6 +2533,13 @@ static int mpi3mr_create_op_queues(struct mpi3mr_ioc *mrioc)
}
}
+ if (i < num_queues) {
+ for (j = i; j < num_queues; j++) {
+ mpi3mr_free_op_req_q_segments(mrioc, j);
+ mpi3mr_free_op_reply_q_segments(mrioc, j);
+ }
+ }
+
if (i == 0) {
/* Not even one queue is created successfully*/
retval = -1;
@@ -2554,11 +2561,18 @@ static int mpi3mr_create_op_queues(struct mpi3mr_ioc *mrioc)
return retval;
out_failed:
- kfree(mrioc->req_qinfo);
- mrioc->req_qinfo = NULL;
-
+ if (mrioc->req_qinfo) {
+ for (j = 0; j < i; j++) {
+ mpi3mr_free_op_req_q_segments(mrioc, j);
+ mpi3mr_free_op_reply_q_segments(mrioc, j);
+ }
+ kfree(mrioc->req_qinfo);
+ mrioc->req_qinfo = NULL;
+ }
+ mrioc->num_op_req_q = 0;
kfree(mrioc->op_reply_qinfo);
mrioc->op_reply_qinfo = NULL;
+ mrioc->num_op_reply_q = 0;
return retval;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v2 06/10] mpi3mr: Fix memory leak on operational queue creation failure
2026-07-08 18:33 ` [PATCH v2 06/10] mpi3mr: Fix memory leak on operational queue creation failure Ranjan Kumar
@ 2026-07-08 19:48 ` sashiko-bot
0 siblings, 0 replies; 17+ messages in thread
From: sashiko-bot @ 2026-07-08 19:48 UTC (permalink / raw)
To: Ranjan Kumar; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] The patch introduces a newly triggerable NULL pointer dereference if a spurious interrupt fires on an IRQ vector associated with a queue that was downgraded or freed.
Pre-existing issues:
- [High] The `out_failed` error path leaves dangling pointers in `mrioc->intr_info[*].op_reply_q`, leading to a Use-After-Free in the interrupt handler.
--
--- Patch [6]: [PATCH v2 06/10] mpi3mr: Fix memory leak on operational queue creation failure ---
commit 442fad52bcdebc4eb17a6ed94630805034267b41
Author: Ranjan Kumar <ranjan.kumar@broadcom.com>
Subject: mpi3mr: Fix memory leak on operational queue creation failure
This commit fixes a memory leak where partially allocated DMA memory segments
were not freed if operational queue creation failed. It also hardens the
error handling path by checking for NULL pointers before freeing segments
and resetting operational queue counts to prevent deferred panics.
Closes: https://sashiko.dev/#/patchset/20260626114109.43685-1-ranjan.kumar@broadcom.com?part=6
> diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
> --- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
> +++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
[ ... ]
> @@ -2533,6 +2533,13 @@ static int mpi3mr_create_op_queues(struct mpi3mr_ioc *mrioc)
> }
> }
>
> + if (i < num_queues) {
> + for (j = i; j < num_queues; j++) {
> + mpi3mr_free_op_req_q_segments(mrioc, j);
> + mpi3mr_free_op_reply_q_segments(mrioc, j);
> + }
> + }
> +
[Severity: High]
Does this introduce a NULL pointer dereference if a spurious interrupt fires
on an IRQ vector associated with a downgraded queue?
If operational queue creation partially fails during a controller reset
(e.g., i < num_queues), this loop explicitly sets
op_reply_qinfo[j].q_segments = NULL via mpi3mr_free_op_reply_q_segments().
Since i > 0, the function succeeds and the controller downgrades to fewer
queues.
However, the driver appears to leave mrioc->intr_info[j].op_reply_q pointing
to these unreached queues. If a spurious interrupt triggers on one of the
unreached vectors, the handler would do this:
mpi3mr_isr_primary() {
if (intr_info->op_reply_q)
mpi3mr_process_op_reply_q(mrioc, intr_info->op_reply_q)
}
And inside mpi3mr_process_op_reply_q():
mpi3mr_process_op_reply_q() {
struct segments *segments = op_reply_q->q_segments;
...
reply_desc = (struct mpi3_default_reply_descriptor *)segments[segment_ix].segment + ...
}
Could this unconditionally dereference the newly nullified q_segments array?
> if (i == 0) {
> /* Not even one queue is created successfully*/
> retval = -1;
[ ... ]
> out_failed:
> - kfree(mrioc->req_qinfo);
> - mrioc->req_qinfo = NULL;
> -
> + if (mrioc->req_qinfo) {
> + for (j = 0; j < i; j++) {
> + mpi3mr_free_op_req_q_segments(mrioc, j);
> + mpi3mr_free_op_reply_q_segments(mrioc, j);
> + }
> + kfree(mrioc->req_qinfo);
> + mrioc->req_qinfo = NULL;
> + }
> + mrioc->num_op_req_q = 0;
> kfree(mrioc->op_reply_qinfo);
> mrioc->op_reply_qinfo = NULL;
> + mrioc->num_op_reply_q = 0;
>
> return retval;
> }
[Severity: High]
This is a pre-existing issue, but does this error path leave dangling pointers
that could cause a Use-After-Free?
If mpi3mr_create_op_queues completely fails during a controller reset
(e.g., i == 0), the code jumps here and frees the mrioc->op_reply_qinfo array.
However, it seems to leave pointers to this array intact inside
mrioc->intr_info[*].op_reply_q. Because these pointers were preserved from
the initial driver load, and interrupts are re-enabled earlier in the reset
sequence, any hardware or spurious interrupt could trigger the primary handler:
mpi3mr_isr_primary() {
if (intr_info->op_reply_q)
num_op_reply = mpi3mr_process_op_reply_q(mrioc, intr_info->op_reply_q);
}
Could this dereference the freed op_reply_qinfo memory? Should the references
in intr_info be cleared when this array is freed?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708183305.244485-1-ranjan.kumar@broadcom.com?part=6
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 07/10] mpi3mr: Fix firmware event reference leak during cleanup
2026-07-08 18:32 [PATCH v2 00/10] mpi3mr: Few Enhancements and minor fixes Ranjan Kumar
` (5 preceding siblings ...)
2026-07-08 18:33 ` [PATCH v2 06/10] mpi3mr: Fix memory leak on operational queue creation failure Ranjan Kumar
@ 2026-07-08 18:33 ` Ranjan Kumar
2026-07-08 19:59 ` sashiko-bot
2026-07-08 18:33 ` [PATCH v2 08/10] mpi3mr: Fix SAS port allocation and registration error handling Ranjan Kumar
` (2 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Ranjan Kumar @ 2026-07-08 18:33 UTC (permalink / raw)
To: linux-scsi, martin.petersen
Cc: sathya.prakash, chandrakanth.patil, vishakhavc, ipylypiv,
Ranjan Kumar, Sashiko
During firmware event cleanup, when an event is currently executing or
pending at the SCSI mid-layer, the driver sets a discard flag and exits
the cleanup routine early. This early exit skips the normal cancel path,
resulting in the firmware event reference count not being decremented,
leading to a reference leak.
Additionally, the cleanup routine performed a lockless read of
mrioc->current_event. This created a Time-of-Check to Time-of-Use
(TOCTOU) race condition where the firmware event worker thread could
free the event before the cleanup routine finished accessing it,
potentially leading to a Use-After-Free panic.
Fix these issues by:
1. Safely acquiring a reference to current_event under the fwevt_lock
to prevent the TOCTOU race and ensure the event is not freed
prematurely.
2. Releasing the acquired firmware event reference before returning
from both the early-exit path and the normal cancel path.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260626114109.43685-1-ranjan.kumar@broadcom.com?part=7
Signed-off-by: Chandrakanth Patil <chandrakanth.patil@broadcom.com>
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
drivers/scsi/mpi3mr/mpi3mr_os.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
index df7365d19b44..273512ab25ba 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_os.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
@@ -379,6 +379,7 @@ static void mpi3mr_cancel_work(struct mpi3mr_fwevt *fwevt)
void mpi3mr_cleanup_fwevt_list(struct mpi3mr_ioc *mrioc)
{
struct mpi3mr_fwevt *fwevt = NULL;
+ unsigned long flags;
if ((list_empty(&mrioc->fwevt_list) && !mrioc->current_event) ||
!mrioc->fwevt_worker_thread)
@@ -387,8 +388,17 @@ void mpi3mr_cleanup_fwevt_list(struct mpi3mr_ioc *mrioc)
while ((fwevt = mpi3mr_dequeue_fwevt(mrioc)))
mpi3mr_cancel_work(fwevt);
- if (mrioc->current_event) {
- fwevt = mrioc->current_event;
+ /*
+ * Safely read current_event under lock to prevent TOCTOU race
+ * with the firmware event worker thread.
+ */
+ spin_lock_irqsave(&mrioc->fwevt_lock, flags);
+ fwevt = mrioc->current_event;
+ if (fwevt)
+ mpi3mr_fwevt_get(fwevt);
+ spin_unlock_irqrestore(&mrioc->fwevt_lock, flags);
+
+ if (fwevt) {
/*
* Don't call cancel_work_sync() API for the
* fwevt work if the controller reset is
@@ -399,10 +409,12 @@ void mpi3mr_cleanup_fwevt_list(struct mpi3mr_ioc *mrioc)
*/
if (current_work() == &fwevt->work || fwevt->pending_at_sml) {
fwevt->discard = 1;
+ mpi3mr_fwevt_put(fwevt);
return;
}
mpi3mr_cancel_work(fwevt);
+ mpi3mr_fwevt_put(fwevt);
}
}
--
2.47.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v2 07/10] mpi3mr: Fix firmware event reference leak during cleanup
2026-07-08 18:33 ` [PATCH v2 07/10] mpi3mr: Fix firmware event reference leak during cleanup Ranjan Kumar
@ 2026-07-08 19:59 ` sashiko-bot
0 siblings, 0 replies; 17+ messages in thread
From: sashiko-bot @ 2026-07-08 19:59 UTC (permalink / raw)
To: Ranjan Kumar; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] The lock-based TOCTOU fix is incomplete and fails to prevent a Use-After-Free because the worker thread modifies `current_event` locklessly.
- [High] A Use-After-Free write exists during event cleanup because `mpi3mr_dequeue_fwevt()` drops the event's reference before it is passed to `mpi3mr_cancel_work()`.
- [High] A race window during event dispatch allows `mpi3mr_cleanup_fwevt_list()` to return early and bypass cancelling an active firmware event.
--
--- Patch [7]: [PATCH v2 07/10] mpi3mr: Fix firmware event reference leak during cleanup ---
commit 594f8db6a0f63b3b0acbdd43108522c6a9ed9f0b
Author: Ranjan Kumar <ranjan.kumar@broadcom.com>
mpi3mr: Fix firmware event reference leak during cleanup
This commit addresses a firmware event reference leak in the cleanup routine
by properly releasing the reference before returning from the early exit and
normal cancel paths. It also adds a lock around the read of the current
event to prevent a race condition with the firmware event worker thread.
Closes: https://sashiko.dev/#/patchset/20260626114109.43685-1-ranjan.kumar@broadcom.com?part=7
> diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
> index df7365d19b44a..273512ab25ba0 100644
> --- a/drivers/scsi/mpi3mr/mpi3mr_os.c
> +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
> @@ -379,6 +379,7 @@ static void mpi3mr_cancel_work(struct mpi3mr_fwevt *fwevt)
> void mpi3mr_cleanup_fwevt_list(struct mpi3mr_ioc *mrioc)
> {
> struct mpi3mr_fwevt *fwevt = NULL;
> + unsigned long flags;
>
> if ((list_empty(&mrioc->fwevt_list) && !mrioc->current_event) ||
> !mrioc->fwevt_worker_thread)
> return;
[Severity: High]
This is a pre-existing issue, but does this early exit condition allow a race
where we bypass cancelling an active firmware event?
During event dispatch, the event is removed from the list and assigned to
current_event sequentially without atomicity:
drivers/scsi/mpi3mr/mpi3mr_os.c:mpi3mr_fwevt_bh() {
...
mpi3mr_fwevt_del_from_list(mrioc, fwevt);
mrioc->current_event = fwevt;
...
}
This creates a race window where the list is empty, but current_event is
still NULL. If mpi3mr_cleanup_fwevt_list() executes during this window,
could its lockless check evaluate to true and return early without waiting
for the work to complete?
>
> while ((fwevt = mpi3mr_dequeue_fwevt(mrioc)))
> mpi3mr_cancel_work(fwevt);
[Severity: High]
This is a pre-existing issue, but does passing this pointer to
mpi3mr_cancel_work() result in a use-after-free?
The mpi3mr_dequeue_fwevt() function drops the list reference before returning
the pointer:
drivers/scsi/mpi3mr/mpi3mr_os.c:mpi3mr_dequeue_fwevt() {
...
list_del_init(&fwevt->list);
mpi3mr_fwevt_put(fwevt);
...
}
When we pass this unreferenced pointer to mpi3mr_cancel_work(), it calls
cancel_work_sync(&fwevt->work). If the worker thread finishes executing
concurrently and drops its remaining references, the structure could be freed.
Would cancel_work_sync() then write to freed memory when it calls
clear_work_data() on the embedded work structure?
>
> - if (mrioc->current_event) {
> - fwevt = mrioc->current_event;
> + /*
> + * Safely read current_event under lock to prevent TOCTOU race
> + * with the firmware event worker thread.
> + */
> + spin_lock_irqsave(&mrioc->fwevt_lock, flags);
> + fwevt = mrioc->current_event;
> + if (fwevt)
> + mpi3mr_fwevt_get(fwevt);
> + spin_unlock_irqrestore(&mrioc->fwevt_lock, flags);
[Severity: High]
This is a pre-existing issue, but does the lock-based fix still leave
a use-after-free window because the worker thread modifies current_event
locklessly?
The read-side acquires the lock here, but the worker thread locklessly clears
current_event and drops the final reference:
drivers/scsi/mpi3mr/mpi3mr_os.c:mpi3mr_fwevt_bh() {
...
mpi3mr_fwevt_put(fwevt);
mrioc->current_event = NULL;
...
}
If mpi3mr_cleanup_fwevt_list() reads a non-NULL current_event, but gets
preempted before calling mpi3mr_fwevt_get(), could the worker thread
concurrently clear the pointer and free the memory? When the cleanup routine
resumes, it would call mpi3mr_fwevt_get() on a freed structure.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708183305.244485-1-ranjan.kumar@broadcom.com?part=7
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 08/10] mpi3mr: Fix SAS port allocation and registration error handling
2026-07-08 18:32 [PATCH v2 00/10] mpi3mr: Few Enhancements and minor fixes Ranjan Kumar
` (6 preceding siblings ...)
2026-07-08 18:33 ` [PATCH v2 07/10] mpi3mr: Fix firmware event reference leak during cleanup Ranjan Kumar
@ 2026-07-08 18:33 ` Ranjan Kumar
2026-07-08 20:12 ` sashiko-bot
2026-07-08 18:33 ` [PATCH v2 09/10] mpi3mr: Fix SAS PHY cleanup in host addition error paths Ranjan Kumar
2026-07-08 18:33 ` [PATCH v2 10/10] mpi3mr: Driver version update to 8.18.0.8.50 Ranjan Kumar
9 siblings, 1 reply; 17+ messages in thread
From: Ranjan Kumar @ 2026-07-08 18:33 UTC (permalink / raw)
To: linux-scsi, martin.petersen
Cc: sathya.prakash, chandrakanth.patil, vishakhavc, ipylypiv,
Ranjan Kumar, Sashiko
During SAS port creation, the driver does not verify successful port
allocation before attempting registration, which can lead to a NULL
pointer dereference. Additionally, if registration fails, the allocated
port is not freed, resulting in a memory leak.
Fix this by adding a NULL check after allocation and freeing the port
when registration fails.
Additionally, fix similar missing NULL checks for rphy allocations,
handle sas_rphy_add() failures properly, and ensure the target device
reference is released in the error path to prevent resource leaks.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260626114109.43685-1-ranjan.kumar@broadcom.com?part=8
Signed-off-by: Chandrakanth Patil <chandrakanth.patil@broadcom.com>
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
drivers/scsi/mpi3mr/mpi3mr_transport.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/scsi/mpi3mr/mpi3mr_transport.c b/drivers/scsi/mpi3mr/mpi3mr_transport.c
index 240f67a8e2e3..b51edbb921eb 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_transport.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_transport.c
@@ -1428,9 +1428,15 @@ static struct mpi3mr_sas_port *mpi3mr_sas_port_add(struct mpi3mr_ioc *mrioc,
}
port = sas_port_alloc_num(mr_sas_node->parent_dev);
+ if (!port) {
+ ioc_err(mrioc, "failure at %s:%d/%s() (sas_port_alloc)!\n",
+ __FILE__, __LINE__, __func__);
+ goto out_fail;
+ }
if ((sas_port_add(port))) {
ioc_err(mrioc, "failure at %s:%d/%s()!\n",
__FILE__, __LINE__, __func__);
+ sas_port_free(port);
goto out_fail;
}
@@ -1450,10 +1456,22 @@ static struct mpi3mr_sas_port *mpi3mr_sas_port_add(struct mpi3mr_ioc *mrioc,
mr_sas_port->port = port;
if (mr_sas_port->remote_identify.device_type == SAS_END_DEVICE) {
rphy = sas_end_device_alloc(port);
+ if (!rphy) {
+ ioc_err(mrioc, "failure at %s:%d/%s() (sas_end_device_alloc)!\n",
+ __FILE__, __LINE__, __func__);
+ sas_port_delete(port);
+ goto out_fail;
+ }
tgtdev->dev_spec.sas_sata_inf.rphy = rphy;
} else {
rphy = sas_expander_alloc(port,
mr_sas_port->remote_identify.device_type);
+ if (!rphy) {
+ ioc_err(mrioc, "failure at %s:%d/%s() (sas_expander_alloc)!\n",
+ __FILE__, __LINE__, __func__);
+ sas_port_delete(port);
+ goto out_fail;
+ }
}
rphy->identify = mr_sas_port->remote_identify;
@@ -1463,6 +1481,9 @@ static struct mpi3mr_sas_port *mpi3mr_sas_port_add(struct mpi3mr_ioc *mrioc,
if ((sas_rphy_add(rphy))) {
ioc_err(mrioc, "failure at %s:%d/%s()!\n",
__FILE__, __LINE__, __func__);
+ sas_port_delete(port);
+ sas_rphy_free(rphy);
+ goto out_fail;
}
if (mr_sas_port->remote_identify.device_type == SAS_END_DEVICE) {
tgtdev->dev_spec.sas_sata_inf.pend_sas_rphy_add = 0;
@@ -1498,6 +1519,9 @@ static struct mpi3mr_sas_port *mpi3mr_sas_port_add(struct mpi3mr_ioc *mrioc,
return mr_sas_port;
out_fail:
+ if (tgtdev)
+ mpi3mr_tgtdev_put(tgtdev);
+
list_for_each_entry_safe(mr_sas_phy, next, &mr_sas_port->phy_list,
port_siblings)
list_del(&mr_sas_phy->port_siblings);
--
2.47.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v2 08/10] mpi3mr: Fix SAS port allocation and registration error handling
2026-07-08 18:33 ` [PATCH v2 08/10] mpi3mr: Fix SAS port allocation and registration error handling Ranjan Kumar
@ 2026-07-08 20:12 ` sashiko-bot
0 siblings, 0 replies; 17+ messages in thread
From: sashiko-bot @ 2026-07-08 20:12 UTC (permalink / raw)
To: Ranjan Kumar; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Double free of `rphy` on `sas_rphy_add` failure path.
--
--- Patch [8]: [PATCH v2 08/10] mpi3mr: Fix SAS port allocation and registration error handling ---
commit e4e396e6a24625afa6491df657cf719648b0c2a2
Author: Ranjan Kumar <ranjan.kumar@broadcom.com>
mpi3mr: Fix SAS port allocation and registration error handling
This commit adds NULL checks for SAS port and rphy allocations to prevent
potential NULL pointer dereferences. It also introduces cleanup logic on
the error paths to prevent memory leaks when registration fails.
Closes: https://sashiko.dev/#/patchset/20260626114109.43685-1-ranjan.kumar@broadcom.com?part=8
> diff --git a/drivers/scsi/mpi3mr/mpi3mr_transport.c b/drivers/scsi/mpi3mr/mpi3mr_transport.c
> index 240f67a8e2e3b..b51edbb921ebb 100644
> --- a/drivers/scsi/mpi3mr/mpi3mr_transport.c
> +++ b/drivers/scsi/mpi3mr/mpi3mr_transport.c
[ ... ]
> @@ -1463,6 +1481,9 @@ static struct mpi3mr_sas_port *mpi3mr_sas_port_add(struct mpi3mr_ioc *mrioc,
> if ((sas_rphy_add(rphy))) {
> ioc_err(mrioc, "failure at %s:%d/%s()!\n",
> __FILE__, __LINE__, __func__);
> + sas_port_delete(port);
> + sas_rphy_free(rphy);
[Severity: High]
Does this code introduce a double free? When sas_rphy_add() fails, port->rphy
still points to the newly allocated rphy object.
Calling sas_port_delete(port) automatically invokes sas_rphy_delete(port->rphy):
drivers/scsi/scsi_transport_sas.c:sas_port_delete() {
...
if (port->rphy) {
sas_rphy_delete(port->rphy);
port->rphy = NULL;
}
...
}
Since sas_rphy_delete() will free the rphy, executing sas_rphy_free(rphy)
immediately afterward appears to operate on an already-freed pointer.
> + goto out_fail;
> }
> if (mr_sas_port->remote_identify.device_type == SAS_END_DEVICE) {
> tgtdev->dev_spec.sas_sata_inf.pend_sas_rphy_add = 0;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708183305.244485-1-ranjan.kumar@broadcom.com?part=8
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 09/10] mpi3mr: Fix SAS PHY cleanup in host addition error paths
2026-07-08 18:32 [PATCH v2 00/10] mpi3mr: Few Enhancements and minor fixes Ranjan Kumar
` (7 preceding siblings ...)
2026-07-08 18:33 ` [PATCH v2 08/10] mpi3mr: Fix SAS port allocation and registration error handling Ranjan Kumar
@ 2026-07-08 18:33 ` Ranjan Kumar
2026-07-08 18:33 ` [PATCH v2 10/10] mpi3mr: Driver version update to 8.18.0.8.50 Ranjan Kumar
9 siblings, 0 replies; 17+ messages in thread
From: Ranjan Kumar @ 2026-07-08 18:33 UTC (permalink / raw)
To: linux-scsi, martin.petersen
Cc: sathya.prakash, chandrakanth.patil, vishakhavc, ipylypiv,
Ranjan Kumar, Sashiko
When adding a SAS host, the driver allocates a PHY array and subsequently
creates individual SAS PHYs. If a later step fails, the error path exits
without cleaning up previously allocated resources, resulting in leaks of
both the PHY array and any registered SAS PHYs.
Additionally, the return value of mpi3mr_add_host_phy() was being ignored.
If it failed, mr_sas_phy->phy would be left as NULL, which could later
lead to a NULL pointer dereference in mpi3mr_sas_port_add() when the
attached device triggers a device addition event.
Add a dedicated cleanup path that deletes any successfully created SAS
PHYs and frees the PHY array before returning from initialization
failure paths. Also, check the return value of mpi3mr_add_host_phy()
and jump to the cleanup path on failure.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260626114109.43685-1-ranjan.kumar@broadcom.com?part=9
Signed-off-by: Chandrakanth Patil <chandrakanth.patil@broadcom.com>
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
drivers/scsi/mpi3mr/mpi3mr_transport.c | 33 ++++++++++++++++++--------
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/mpi3mr/mpi3mr_transport.c b/drivers/scsi/mpi3mr/mpi3mr_transport.c
index b51edbb921eb..a8b79da25d38 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_transport.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_transport.c
@@ -1216,13 +1216,14 @@ void mpi3mr_sas_host_add(struct mpi3mr_ioc *mrioc)
}
num_phys = sas_io_unit_pg0->num_phys;
kfree(sas_io_unit_pg0);
+ sas_io_unit_pg0 = NULL;
mrioc->sas_hba.host_node = 1;
INIT_LIST_HEAD(&mrioc->sas_hba.sas_port_list);
mrioc->sas_hba.parent_dev = &mrioc->shost->shost_gendev;
mrioc->sas_hba.phy = kzalloc_objs(struct mpi3mr_sas_phy, num_phys);
if (!mrioc->sas_hba.phy)
- return;
+ goto out;
mrioc->sas_hba.num_phys = num_phys;
@@ -1230,12 +1231,12 @@ void mpi3mr_sas_host_add(struct mpi3mr_ioc *mrioc)
(num_phys * sizeof(struct mpi3_sas_io_unit0_phy_data));
sas_io_unit_pg0 = kzalloc(sz, GFP_KERNEL);
if (!sas_io_unit_pg0)
- return;
+ goto out_free_phy;
if (mpi3mr_cfg_get_sas_io_unit_pg0(mrioc, sas_io_unit_pg0, sz)) {
ioc_err(mrioc, "failure at %s:%d/%s()!\n",
__FILE__, __LINE__, __func__);
- goto out;
+ goto out_free_phy;
}
mrioc->sas_hba.handle = 0;
@@ -1249,12 +1250,12 @@ void mpi3mr_sas_host_add(struct mpi3mr_ioc *mrioc)
MPI3_SAS_PHY_PGAD_FORM_PHY_NUMBER, i)) {
ioc_err(mrioc, "failure at %s:%d/%s()!\n",
__FILE__, __LINE__, __func__);
- goto out;
+ goto out_free_phy;
}
if (ioc_status != MPI3_IOCSTATUS_SUCCESS) {
ioc_err(mrioc, "failure at %s:%d/%s()!\n",
__FILE__, __LINE__, __func__);
- goto out;
+ goto out_free_phy;
}
if (!mrioc->sas_hba.handle)
@@ -1264,26 +1265,27 @@ void mpi3mr_sas_host_add(struct mpi3mr_ioc *mrioc)
if (!(mpi3mr_get_hba_port_by_id(mrioc, port_id)))
if (!mpi3mr_alloc_hba_port(mrioc, port_id))
- goto out;
+ goto out_free_phy;
mrioc->sas_hba.phy[i].handle = mrioc->sas_hba.handle;
mrioc->sas_hba.phy[i].phy_id = i;
mrioc->sas_hba.phy[i].hba_port =
mpi3mr_get_hba_port_by_id(mrioc, port_id);
- mpi3mr_add_host_phy(mrioc, &mrioc->sas_hba.phy[i],
- phy_pg0, mrioc->sas_hba.parent_dev);
+ if (mpi3mr_add_host_phy(mrioc, &mrioc->sas_hba.phy[i],
+ phy_pg0, mrioc->sas_hba.parent_dev))
+ goto out_free_phy;
}
if ((mpi3mr_cfg_get_dev_pg0(mrioc, &ioc_status, &dev_pg0,
sizeof(dev_pg0), MPI3_DEVICE_PGAD_FORM_HANDLE,
mrioc->sas_hba.handle))) {
ioc_err(mrioc, "%s: device page0 read failed\n", __func__);
- goto out;
+ goto out_free_phy;
}
if (ioc_status != MPI3_IOCSTATUS_SUCCESS) {
ioc_err(mrioc, "device page read failed for handle(0x%04x), with ioc_status(0x%04x) failure at %s:%d/%s()!\n",
mrioc->sas_hba.handle, ioc_status, __FILE__, __LINE__,
__func__);
- goto out;
+ goto out_free_phy;
}
mrioc->sas_hba.enclosure_handle =
le16_to_cpu(dev_pg0.enclosure_handle);
@@ -1306,6 +1308,17 @@ void mpi3mr_sas_host_add(struct mpi3mr_ioc *mrioc)
le64_to_cpu(encl_pg0.enclosure_logical_id);
}
+ goto out;
+
+out_free_phy:
+ for (i = 0; i < mrioc->sas_hba.num_phys; i++) {
+ if (mrioc->sas_hba.phy[i].phy)
+ sas_phy_delete(mrioc->sas_hba.phy[i].phy);
+ }
+ kfree(mrioc->sas_hba.phy);
+ mrioc->sas_hba.phy = NULL;
+ mrioc->sas_hba.num_phys = 0;
+
out:
kfree(sas_io_unit_pg0);
}
--
2.47.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 10/10] mpi3mr: Driver version update to 8.18.0.8.50
2026-07-08 18:32 [PATCH v2 00/10] mpi3mr: Few Enhancements and minor fixes Ranjan Kumar
` (8 preceding siblings ...)
2026-07-08 18:33 ` [PATCH v2 09/10] mpi3mr: Fix SAS PHY cleanup in host addition error paths Ranjan Kumar
@ 2026-07-08 18:33 ` Ranjan Kumar
9 siblings, 0 replies; 17+ messages in thread
From: Ranjan Kumar @ 2026-07-08 18:33 UTC (permalink / raw)
To: linux-scsi, martin.petersen
Cc: sathya.prakash, chandrakanth.patil, vishakhavc, ipylypiv,
Ranjan Kumar
Update driver version to 8.18.0.8.50
Signed-off-by: Ranjan Kumar <ranjan.kumar@broadcom.com>
---
drivers/scsi/mpi3mr/mpi3mr.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h
index 1d11d7c69536..c6bbd6b33cfe 100644
--- a/drivers/scsi/mpi3mr/mpi3mr.h
+++ b/drivers/scsi/mpi3mr/mpi3mr.h
@@ -56,8 +56,8 @@ extern struct list_head mrioc_list;
extern int prot_mask;
extern atomic64_t event_counter;
-#define MPI3MR_DRIVER_VERSION "8.17.0.3.50"
-#define MPI3MR_DRIVER_RELDATE "09-January-2026"
+#define MPI3MR_DRIVER_VERSION "8.18.0.8.50"
+#define MPI3MR_DRIVER_RELDATE "26-June-2026"
#define MPI3MR_DRIVER_NAME "mpi3mr"
#define MPI3MR_DRIVER_LICENSE "GPL"
--
2.47.3
^ permalink raw reply related [flat|nested] 17+ messages in thread