This patchset contains critical Bug fixes
Subsystems (2)
cover This patchset contains critical Bug fixes
This patchset contains critical Bug fixes
Ranjan Kumar (6):
mpi3mr: IOCTL timeout when disable/enable Interpt
mpi3mr: Driver unload crash host when enhanced logging is enabled
mpi3mr: Wait for diagnostic save during controller init
mpi3mr: appropriate return values for failures in firmware init path
mpi3mr: NVMe commands size greater than 8K fails
mpi3mr: Bad drive in topology results kernel crash
drivers/scsi/mpi3mr/mpi3mr.h | 3 ++ drivers/scsi/mpi3mr/mpi3mr_app.c | 4 +-- drivers/scsi/mpi3mr/mpi3mr_fw.c | 42 +++++++++++++++++++++++--- drivers/scsi/mpi3mr/mpi3mr_os.c | 1 + drivers/scsi/mpi3mr/mpi3mr_transport.c | 15 +++++---- 5 files changed, 52 insertions(+), 13 deletions(-)
Signature · 1 line
2.31.1
PATCH 1/6 mpi3mr: IOCTL timeout when disable/enable Interpt
As part of Task Management handling, the driver will disable and enable the MSIx index zero which belongs to the Admin reply queue. And while enabling the interrupts driver loses some interrupts and it leads to Admin requests such as IOCTL timeout. So, after enabling the interrupts, poll the Admin reply queue to avoid timeouts.
--- drivers/scsi/mpi3mr/mpi3mr.h | 3 +++ drivers/scsi/mpi3mr/mpi3mr_fw.c | 12 ++++++++++-- drivers/scsi/mpi3mr/mpi3mr_os.c | 1 + 3 files changed, 14 insertions(+), 2 deletions(-)
drivers/scsi/mpi3mr/mpi3mr.h +3 -0
@@ -903,6 +903,7 @@ struct scmd_priv {* @admin_reply_ephase:Admin reply queue expected phase * @admin_reply_base: Admin reply queue base virtual address * @admin_reply_dma: Admin reply queue base dma address +* @admin_reply_q_in_use: Queue is handled by poll/ISR * @ready_timeout: Controller ready timeout * @intr_info: Interrupt cookie pointer * @intr_info_count: Number of interrupt cookies
@@ -1059,6 +1060,7 @@ struct mpi3mr_ioc {u8 admin_reply_ephase;void *admin_reply_base;dma_addr_t admin_reply_dma; +atomic_t admin_reply_q_in_use;u32 ready_timeout;
@@ -1397,4 +1399,5 @@ void mpi3mr_add_event_wait_for_device_refresh(struct mpi3mr_ioc *mrioc);void mpi3mr_flush_drv_cmds (struct mpi3mr_ioc *mrioc);void mpi3mr_flush_cmds_for_unrecovered_controller (struct mpi3mr_ioc *mrioc);void mpi3mr_free_enclosure_list (struct mpi3mr_ioc *mrioc); +int mpi3mr_process_admin_reply_q (struct mpi3mr_ioc *mrioc);#endif /*MPI3MR_H_INCLUDED*/
drivers/scsi/mpi3mr/mpi3mr_fw.c +10 -2
@@ -415,7 +415,7 @@ static void mpi3mr_process_admin_reply_desc(struct mpi3mr_ioc *mrioc, le64_to_cpu(scsi_reply->sense_data_buffer_address)); } -static int mpi3mr_process_admin_reply_q (struct mpi3mr_ioc *mrioc) +int mpi3mr_process_admin_reply_q (struct mpi3mr_ioc *mrioc) {u32 exp_phase = mrioc->admin_reply_ephase;u32 admin_reply_ci = mrioc->admin_reply_ci;
@@ -423,12 +423,17 @@ static int mpi3mr_process_admin_reply_q(struct mpi3mr_ioc *mrioc)u64 reply_dma =0 ;struct mpi3_default_reply_descriptor *reply_desc; +if (!atomic_add_unless (&mrioc->admin_reply_q_in_use,1 ,1 )) +return 0 ; + reply_desc = (struct mpi3_default_reply_descriptor *)mrioc->admin_reply_base + admin_reply_ci;if ((le16_to_cpu (reply_desc->reply_flags) & -MPI3_REPLY_DESCRIPT_FLAGS_PHASE_MASK ) != exp_phase) +MPI3_REPLY_DESCRIPT_FLAGS_PHASE_MASK ) != exp_phase) { +atomic_dec (&mrioc->admin_reply_q_in_use);return 0 ; + } do { if (mrioc->unrecoverable)
@@ -454,6 +459,7 @@ static int mpi3mr_process_admin_reply_q(struct mpi3mr_ioc *mrioc)writel (admin_reply_ci, &mrioc->sysif_regs->admin_reply_queue_ci); mrioc->admin_reply_ci = admin_reply_ci; mrioc->admin_reply_ephase = exp_phase; +atomic_dec (&mrioc->admin_reply_q_in_use);return num_admin_replies; }
@@ -2608,6 +2614,7 @@ static int mpi3mr_setup_admin_qpair(struct mpi3mr_ioc *mrioc) mrioc->admin_reply_ci =0 ; mrioc->admin_reply_ephase =1 ; mrioc->admin_reply_base =NULL ; +atomic_set (&mrioc->admin_reply_q_in_use,0 );if (!mrioc->admin_req_base) { mrioc->admin_req_base = dma_alloc_coherent(&mrioc->pdev->dev,
@@ -4171,6 +4178,7 @@ void mpi3mr_memset_buffers(struct mpi3mr_ioc *mrioc)memset (mrioc->admin_req_base,0 , mrioc->admin_req_q_sz);if (mrioc->admin_reply_base)memset (mrioc->admin_reply_base,0 , mrioc->admin_reply_q_sz); +atomic_set (&mrioc->admin_reply_q_in_use,0 );if (mrioc->init_cmds.reply) {memset (mrioc->init_cmds.reply,0 ,sizeof (*mrioc->init_cmds.reply));
drivers/scsi/mpi3mr/mpi3mr_os.c +1 -0
@@ -3720,6 +3720,7 @@ int mpi3mr_issue_tm(struct mpi3mr_ioc *mrioc, u8 tm_type,mpi3mr_poll_pend_io_completions (mrioc);mpi3mr_ioc_enable_intr (mrioc);mpi3mr_poll_pend_io_completions (mrioc); +mpi3mr_process_admin_reply_q (mrioc); }switch (tm_type) {case MPI3_SCSITASKMGMT_TASKTYPE_TARGET_RESET :
Signature · 1 line
2.31.1
PATCH 2/6 mpi3mr: Driver unload crash host when enhanced logging is enabled
In reply to Ranjan KumarDriver was trying to access null pointer in a debug print while removing a device during driver unload
--- drivers/scsi/mpi3mr/mpi3mr_transport.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/scsi/mpi3mr/mpi3mr_transport.c +2 -1
@@ -1552,7 +1552,8 @@ static void mpi3mr_sas_port_remove(struct mpi3mr_ioc *mrioc, u64 sas_address,list_for_each_entry_safe (mr_sas_phy, next_phy, &mr_sas_port->phy_list, port_siblings) { -if ((mrioc->logging_level &MPI3_DEBUG_TRANSPORT_INFO )) +if ((!mrioc->stop_drv_processing) && + (mrioc->logging_level &MPI3_DEBUG_TRANSPORT_INFO )) dev_info(&mr_sas_port->port->dev, "remove: sas_address(0x%016llx), phy(%d)\n", (unsigned long long)
Signature · 1 line
2.31.1
PATCH 3/6 mpi3mr: Wait for diagnostic save during controller init
In reply to Ranjan KumarIf the controller reset operation is triggered to recover the controller from the fault state then wait for the snapdump to be saved in the firmware region before proceeding for resetting the controller.
--- drivers/scsi/mpi3mr/mpi3mr_fw.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
drivers/scsi/mpi3mr/mpi3mr_fw.c +18 -1
@@ -1201,7 +1201,7 @@ mpi3mr_revalidate_factsdata(struct mpi3mr_ioc *mrioc)*/ static int mpi3mr_bring_ioc_ready (struct mpi3mr_ioc *mrioc) { -u32 ioc_config, ioc_status, timeout; +u32 ioc_config, ioc_status, timeout, host_diagnostic;int retval =0 ;enum mpi3mr_iocstate ioc_state;u64 base_info;
@@ -1255,6 +1255,23 @@ static int mpi3mr_bring_ioc_ready(struct mpi3mr_ioc *mrioc) retval, mpi3mr_iocstate_name(ioc_state)); }if (ioc_state !=MRIOC_STATE_RESET ) { +if (ioc_state ==MRIOC_STATE_FAULT ) { + timeout =MPI3_SYSIF_DIAG_SAVE_TIMEOUT *10 ; +mpi3mr_print_fault_info (mrioc); +do { + host_diagnostic = +readl (&mrioc->sysif_regs->host_diagnostic); +if (!(host_diagnostic & +MPI3_SYSIF_HOST_DIAG_SAVE_IN_PROGRESS )) +break ; +if (!pci_device_is_present (mrioc->pdev)) { + mrioc->unrecoverable =1 ; +ioc_err (mrioc,"controller is not present at the bringup\n" ); + goto out_device_not_present; + } +msleep (100 ); + }while (--timeout); + }mpi3mr_print_fault_info (mrioc);ioc_info (mrioc,"issuing soft reset to bring to reset state\n" ); retval = mpi3mr_issue_reset(mrioc,
Signature · 1 line
2.31.1
PATCH 4/6 mpi3mr: appropriate return values for failures in firmware init path
In reply to Ranjan KumarThe driver is modified to return a proper non-zero return value for all the cases when the controller initialization and re-initialization fails.
--- drivers/scsi/mpi3mr/mpi3mr_fw.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
drivers/scsi/mpi3mr/mpi3mr_fw.c +9 -2
@@ -3856,8 +3856,10 @@ int mpi3mr_init_ioc(struct mpi3mr_ioc *mrioc)dprint_init (mrioc,"allocating config page buffers\n" ); mrioc->cfg_page =dma_alloc_coherent (&mrioc->pdev->dev,MPI3MR_DEFAULT_CFG_PAGE_SZ , &mrioc->cfg_page_dma,GFP_KERNEL ); -if (!mrioc->cfg_page) +if (!mrioc->cfg_page) { + retval =-1 ; goto out_failed_noretry; + } mrioc->cfg_page_sz =MPI3MR_DEFAULT_CFG_PAGE_SZ ;
@@ -3919,8 +3921,10 @@ int mpi3mr_init_ioc(struct mpi3mr_ioc *mrioc)dprint_init (mrioc,"allocating memory for throttle groups\n" ); sz =sizeof (struct mpi3mr_throttle_group_info ); mrioc->throttle_groups =kcalloc (mrioc->num_io_throttle_group, sz,GFP_KERNEL ); -if (!mrioc->throttle_groups) +if (!mrioc->throttle_groups) { + retval =-1 ; goto out_failed_noretry; + } } retval =mpi3mr_enable_events (mrioc);
@@ -3940,6 +3944,7 @@ int mpi3mr_init_ioc(struct mpi3mr_ioc *mrioc)mpi3mr_memset_buffers (mrioc); goto retry_init; } + retval =-1 ; out_failed_noretry:ioc_err (mrioc,"controller initialization failed\n" ); mpi3mr_issue_reset(mrioc, MPI3_SYSIF_HOST_DIAG_RESET_ACTION_DIAG_FAULT,
@@ -4052,6 +4057,7 @@ int mpi3mr_reinit_ioc(struct mpi3mr_ioc *mrioc, u8 is_resume)ioc_err (mrioc,"cannot create minimum number of operational queues expected:%d created:%d\n" , mrioc->shost->nr_hw_queues, mrioc->num_op_reply_q); + retval =-1 ; goto out_failed_noretry; }
@@ -4118,6 +4124,7 @@ int mpi3mr_reinit_ioc(struct mpi3mr_ioc *mrioc, u8 is_resume)mpi3mr_memset_buffers (mrioc); goto retry_init; } + retval =-1 ; out_failed_noretry:ioc_err (mrioc,"controller %s is failed\n" , (is_resume)?"resume" :"re-initialization" );
Signature · 1 line
2.31.1
PATCH 5/6 mpi3mr: NVMe commands size greater than 8K fails
In reply to Ranjan KumarA wrong variable is checked while populating PRP entries in the PRP page and that results in failure .Hence no PRP entries in the PRP page were successfully created and any NVMe Encapsulated commands with PRP of size greater than 8K failed
--- drivers/scsi/mpi3mr/mpi3mr_app.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/scsi/mpi3mr/mpi3mr_app.c +2 -2
@@ -894,7 +894,7 @@ static int mpi3mr_build_nvme_prp(struct mpi3mr_ioc *mrioc,* each time through the loop. */ *prp_entry =cpu_to_le64 (dma_addr); -if (*prp1_entry & sgemod_mask) { +if (*prp_entry & sgemod_mask) {dprint_bsg_err (mrioc,"%s: PRP address collides with SGE modifier\n" , __func__);
@@ -903,7 +903,7 @@ static int mpi3mr_build_nvme_prp(struct mpi3mr_ioc *mrioc, *prp_entry &= ~sgemod_mask; *prp_entry |= sgemod_val; prp_entry++; - prp_entry_dma++; + prp_entry_dma += prp_size; }/*
Signature · 1 line
2.31.1
PATCH 6/6 mpi3mr: Bad drive in topology results kernel crash
In reply to Ranjan KumarWhen the SAS Transport Layer support is enabled and when a device exposed to the OS by the driver failed Inquiry commands then the driver frees up the memory allocated for an internal HBA port data structure. However, in some places, the reference to the freed memory is not cleared.When the firmware sends the Device Info change event for the same device again,then the freed memory is accessed and that leads to memory corruption and OS crash.
--- drivers/scsi/mpi3mr/mpi3mr_transport.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
drivers/scsi/mpi3mr/mpi3mr_transport.c +7 -5
@@ -2358,15 +2358,16 @@ int mpi3mr_report_tgtdev_to_sas_transport(struct mpi3mr_ioc *mrioc, tgtdev->host_exposed =1 ;if (!mpi3mr_sas_port_add (mrioc, tgtdev->dev_handle, sas_address_parent, hba_port)) { - tgtdev->host_exposed =0 ; retval =-1 ; - }else if ((!tgtdev->starget)) { -if (!mrioc->is_driver_loading) + }else if ((!tgtdev->starget) && (!mrioc->is_driver_loading)) {mpi3mr_sas_port_remove (mrioc, sas_address, sas_address_parent, hba_port); - tgtdev->host_exposed =0 ; retval =-1 ; } +if (retval) { + tgtdev->dev_spec.sas_sata_inf.hba_port =NULL ; + tgtdev->host_exposed =0 ; + }return retval; }
@@ -2395,6 +2396,7 @@ void mpi3mr_remove_tgtdev_from_sas_transport(struct mpi3mr_ioc *mrioc,mpi3mr_sas_port_remove (mrioc, sas_address, sas_address_parent, hba_port); tgtdev->host_exposed =0 ; + tgtdev->dev_spec.sas_sata_inf.hba_port =NULL ; }/**
@@ -2451,7 +2453,7 @@ static u8 mpi3mr_get_port_id_by_rphy(struct mpi3mr_ioc *mrioc, struct sas_rphy * tgtdev =__mpi3mr_get_tgtdev_by_addr_and_rphy (mrioc, rphy->identify.sas_address, rphy); -if (tgtdev) { +if (tgtdev && tgtdev->dev_spec.sas_sata_inf.hba_port) { port_id = tgtdev->dev_spec.sas_sata_inf.hba_port->port_id;mpi3mr_tgtdev_put (tgtdev);
Signature · 1 line
2.31.1
Martin K. Petersen
AppliedQuoted 11 lines · Ranjan Kumar
This patchset contains critical Bug fixes
Ranjan Kumar (6):
mpi3mr: IOCTL timeout when disable/enable Interpt
mpi3mr: Driver unload crash host when enhanced logging is enabled
mpi3mr: Wait for diagnostic save during controller init
mpi3mr: appropriate return values for failures in firmware init path
mpi3mr: NVMe commands size greater than 8K fails
mpi3mr: Bad drive in topology results kernel crash[...]
Applied to 6.3/scsi-fixes, thanks!
[1/6] mpi3mr: IOCTL timeout when disable/enable Interpt
https://git.kernel.org/mkp/scsi/c/02ca7da2919a [2/6] mpi3mr: Driver unload crash host when enhanced logging is enabled https://git.kernel.org/mkp/scsi/c/5b06a7169c59 [3/6] mpi3mr: Wait for diagnostic save during controller init https://git.kernel.org/mkp/scsi/c/0a319f162949 [4/6] mpi3mr: appropriate return values for failures in firmware init path https://git.kernel.org/mkp/scsi/c/ba8a9ba41fbd [5/6] mpi3mr: NVMe commands size greater than 8K fails https://git.kernel.org/mkp/scsi/c/4f297e856a7b [6/6] mpi3mr: Bad drive in topology results kernel crash https://git.kernel.org/mkp/scsi/c/8e45183978d6
Signature · 1 line
Martin K. Petersen Oracle Linux Engineering
Hi,
Quoted 12 lines · Martin K. Petersen
Quoted 10 lines · Ranjan Kumar
This patchset contains critical Bug fixes
Ranjan Kumar (6):
mpi3mr: IOCTL timeout when disable/enable Interpt
mpi3mr: Driver unload crash host when enhanced logging is enabled
mpi3mr: Wait for diagnostic save during controller init
mpi3mr: appropriate return values for failures in firmware init path
mpi3mr: NVMe commands size greater than 8K fails
mpi3mr: Bad drive in topology results kernel crash
Quoted 3 lines
Quoted 1 line
[...]
Applied to 6.3/scsi-fixes, thanks!
Will those be backported as well as needed to at least 6.1.y where impacted? It was noticed that the patches do not contain Fixes tags and no CC to stable@ so they might not be picked automatically for stable series.
Regards,
Salvatore
Salvatore,
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Ranjan Kumar (6):
mpi3mr: IOCTL timeout when disable/enable Interpt
mpi3mr: Driver unload crash host when enhanced logging is enabled
mpi3mr: Wait for diagnostic save during controller init
mpi3mr: appropriate return values for failures in firmware init path
mpi3mr: NVMe commands size greater than 8K fails
mpi3mr: Bad drive in topology results kernel crash
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
[...]
Applied to 6.3/scsi-fixes, thanks!
Will those be backported as well as needed to at least 6.1.y where impacted? It was noticed that the patches do not contain Fixes tags and no CC to stable@ so they might not be picked automatically for stable series.
That's up to Broadcom. I rely on driver maintainers to tag the commits they wish to see in stable.
Signature · 1 line
Martin K. Petersen Oracle Linux Engineering