From 6cc55c969b7ce8d85e09a636693d4126c3676c11 Mon Sep 17 00:00:00 2001 From: Maurizio Lombardi Date: Tue, 14 Feb 2023 15:15:56 +0100 Subject: [PATCH 01/20] scsi: target: iscsi: Fix an error message in iscsi_check_key() The first half of the error message is printed by pr_err(), the second half is printed by pr_debug(). The user will therefore see only the first part of the message and will miss some useful information. Link: https://lore.kernel.org/r/20230214141556.762047-1-mlombard@redhat.com Signed-off-by: Maurizio Lombardi Reviewed-by: Mike Christie Signed-off-by: Martin K. Petersen --- drivers/target/iscsi/iscsi_target_parameters.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/target/iscsi/iscsi_target_parameters.c b/drivers/target/iscsi/iscsi_target_parameters.c index 2317fb077db0..557516c642c3 100644 --- a/drivers/target/iscsi/iscsi_target_parameters.c +++ b/drivers/target/iscsi/iscsi_target_parameters.c @@ -1262,18 +1262,20 @@ static struct iscsi_param *iscsi_check_key( return param; if (!(param->phase & phase)) { - pr_err("Key \"%s\" may not be negotiated during ", - param->name); + char *phase_name; + switch (phase) { case PHASE_SECURITY: - pr_debug("Security phase.\n"); + phase_name = "Security"; break; case PHASE_OPERATIONAL: - pr_debug("Operational phase.\n"); + phase_name = "Operational"; break; default: - pr_debug("Unknown phase.\n"); + phase_name = "Unknown"; } + pr_err("Key \"%s\" may not be negotiated during %s phase.\n", + param->name, phase_name); return NULL; } From 877b03795fcf29ff2e2351f7e574ecc9b9c51732 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Wed, 8 Feb 2023 16:20:14 +0100 Subject: [PATCH 02/20] scsi: qla2xxx: Add option to disable FC2 Target support Commit 44c57f205876 ("scsi: qla2xxx: Changes to support FCP2 Target") added support for FC2 Targets. Unfortunately, there are older setups which break with this new feature enabled. Allow to disable it via module option. Link: https://lore.kernel.org/r/20230208152014.109214-1-dwagner@suse.de Signed-off-by: Daniel Wagner Reviewed-by: Himanshu Madhani Signed-off-by: Martin K. Petersen --- drivers/scsi/qla2xxx/qla_gbl.h | 1 + drivers/scsi/qla2xxx/qla_init.c | 3 ++- drivers/scsi/qla2xxx/qla_os.c | 10 +++++++++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h index 9142df876c73..9aba07c7bde4 100644 --- a/drivers/scsi/qla2xxx/qla_gbl.h +++ b/drivers/scsi/qla2xxx/qla_gbl.h @@ -192,6 +192,7 @@ extern int ql2xsecenable; extern int ql2xenforce_iocb_limit; extern int ql2xabts_wait_nvme; extern u32 ql2xnvme_queues; +extern int ql2xfc2target; extern int qla2x00_loop_reset(scsi_qla_host_t *); extern void qla2x00_abort_all_cmds(scsi_qla_host_t *, int); diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index 1dbc1496ebed..ec0423ec6681 100644 --- a/drivers/scsi/qla2xxx/qla_init.c +++ b/drivers/scsi/qla2xxx/qla_init.c @@ -1840,7 +1840,8 @@ void qla2x00_handle_rscn(scsi_qla_host_t *vha, struct event_arg *ea) case RSCN_PORT_ADDR: fcport = qla2x00_find_fcport_by_nportid(vha, &ea->id, 1); if (fcport) { - if (fcport->flags & FCF_FCP2_DEVICE && + if (ql2xfc2target && + fcport->flags & FCF_FCP2_DEVICE && atomic_read(&fcport->state) == FCS_ONLINE) { ql_dbg(ql_dbg_disc, vha, 0x2115, "Delaying session delete for FCP2 portid=%06x %8phC ", diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c index 545167627e48..80c4ee9df2a4 100644 --- a/drivers/scsi/qla2xxx/qla_os.c +++ b/drivers/scsi/qla2xxx/qla_os.c @@ -360,6 +360,13 @@ MODULE_PARM_DESC(ql2xnvme_queues, "1 - Minimum number of queues supported\n" "8 - Default value"); +int ql2xfc2target = 1; +module_param(ql2xfc2target, int, 0444); +MODULE_PARM_DESC(qla2xfc2target, + "Enables FC2 Target support. " + "0 - FC2 Target support is disabled. " + "1 - FC2 Target support is enabled (default)."); + static struct scsi_transport_template *qla2xxx_transport_template = NULL; struct scsi_transport_template *qla2xxx_transport_vport_template = NULL; @@ -4085,7 +4092,8 @@ qla2x00_mark_all_devices_lost(scsi_qla_host_t *vha) "Mark all dev lost\n"); list_for_each_entry(fcport, &vha->vp_fcports, list) { - if (fcport->loop_id != FC_NO_LOOP_ID && + if (ql2xfc2target && + fcport->loop_id != FC_NO_LOOP_ID && (fcport->flags & FCF_FCP2_DEVICE) && fcport->port_type == FCT_TARGET && !qla2x00_reset_active(vha)) { From 06d1a90de60208054cca15ef200138cfdbb642a9 Mon Sep 17 00:00:00 2001 From: Kang Chen Date: Mon, 27 Feb 2023 11:10:30 +0800 Subject: [PATCH 03/20] scsi: hisi_sas: Check devm_add_action() return value In case devm_add_action() fails, check it in the caller of interrupt_preinit_v3_hw(). Link: https://lore.kernel.org/r/20230227031030.893324-1-void0red@gmail.com Signed-off-by: Kang Chen Acked-by: Xiang Chen Signed-off-by: Martin K. Petersen --- drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c index 0c3fcb807806..a63279f55d09 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c @@ -2495,8 +2495,7 @@ static int interrupt_preinit_v3_hw(struct hisi_hba *hisi_hba) hisi_hba->cq_nvecs = vectors - BASE_VECTORS_V3_HW; shost->nr_hw_queues = hisi_hba->cq_nvecs; - devm_add_action(&pdev->dev, hisi_sas_v3_free_vectors, pdev); - return 0; + return devm_add_action(&pdev->dev, hisi_sas_v3_free_vectors, pdev); } static int interrupt_init_v3_hw(struct hisi_hba *hisi_hba) From 2ebe16155dc8bd4e602cad5b5f65458d2eaa1a75 Mon Sep 17 00:00:00 2001 From: Adrien Thierry Date: Mon, 20 Feb 2023 09:07:40 -0500 Subject: [PATCH 04/20] scsi: ufs: core: Add soft dependency on governor_simpleondemand The ufshcd driver uses simpleondemand governor for devfreq. Add it to the list of ufshcd softdeps to allow userspace initramfs tools like dracut to automatically pull the governor module into the initramfs together with UFS drivers. Link: https://lore.kernel.org/r/20230220140740.14379-1-athierry@redhat.com Signed-off-by: Adrien Thierry Signed-off-by: Martin K. Petersen --- drivers/ufs/core/ufshcd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 172d25fef740..05eac965ee27 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -10512,4 +10512,5 @@ module_exit(ufshcd_core_exit); MODULE_AUTHOR("Santosh Yaragnavi "); MODULE_AUTHOR("Vinayak Holikatti "); MODULE_DESCRIPTION("Generic UFS host controller driver Core"); +MODULE_SOFTDEP("pre: governor_simpleondemand"); MODULE_LICENSE("GPL"); From fa8d32721a07d1349d963832400e97ebb515aa96 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Mon, 27 Feb 2023 13:07:26 +0300 Subject: [PATCH 05/20] scsi: ufs: ufs-qcom: Remove impossible check The "dev_req_params" pointer points to inside the middle of a struct so it can't be NULL. Removing this impossible condition is nice because now we don't need to consider the correct error code for that situation. Link: https://lore.kernel.org/r/Y/yA3niWUcGYgBU8@kili Fixes: f06fcc7155dc ("scsi: ufs-qcom: add QUniPro hardware support and power optimizations") Signed-off-by: Dan Carpenter Signed-off-by: Martin K. Petersen --- drivers/ufs/host/ufs-qcom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c index 34fc453f3eb1..797230ad3939 100644 --- a/drivers/ufs/host/ufs-qcom.c +++ b/drivers/ufs/host/ufs-qcom.c @@ -1177,7 +1177,7 @@ static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba, err = ufs_qcom_clk_scale_down_post_change(hba); - if (err || !dev_req_params) { + if (err) { ufshcd_uic_hibern8_exit(hba); return err; } From c8be073bd2bced20f58e65276281fc116758c7cb Mon Sep 17 00:00:00 2001 From: Asutosh Das Date: Wed, 1 Mar 2023 17:41:06 -0800 Subject: [PATCH 06/20] scsi: ufs: mcq: qcom: Fix passing zero to PTR_ERR Fix an error case in ufs_qcom_mcq_config_resource(), where the return value is set to 0 before passing it to PTR_ERR. This led to Smatch warning: drivers/ufs/host/ufs-qcom.c:1455 ufs_qcom_mcq_config_resource() warn: passing zero to 'PTR_ERR' Fixes: c263b4ef737e ("scsi: ufs: core: mcq: Configure resource regions") Reported-by: Dan Carpenter Signed-off-by: Asutosh Das Link: https://lore.kernel.org/r/94ca99b327af634799ce5f25d0112c28cd00970d.1677721072.git.quic_asutoshd@quicinc.com Reviewed-by: Bjorn Andersson Reviewed-by: Manivannan Sadhasivam Signed-off-by: Martin K. Petersen --- drivers/ufs/host/ufs-qcom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c index 797230ad3939..2781739a66e0 100644 --- a/drivers/ufs/host/ufs-qcom.c +++ b/drivers/ufs/host/ufs-qcom.c @@ -1451,8 +1451,8 @@ static int ufs_qcom_mcq_config_resource(struct ufs_hba *hba) if (IS_ERR(res->base)) { dev_err(hba->dev, "Failed to map res %s, err=%d\n", res->name, (int)PTR_ERR(res->base)); - res->base = NULL; ret = PTR_ERR(res->base); + res->base = NULL; return ret; } } From c9507eab9fa7af7ab4c80856eeafd1481ff16f7c Mon Sep 17 00:00:00 2001 From: Asutosh Das Date: Wed, 1 Mar 2023 17:42:56 -0800 Subject: [PATCH 07/20] scsi: ufs: mcq: qcom: Clean the return path of ufs_qcom_mcq_config_resource() Smatch static checker reported: drivers/ufs/host/ufs-qcom.c:1469 ufs_qcom_mcq_config_resource() info: returning a literal zero is cleaner Fix the above warning by returning in place instead of a jump to a label. Also remove the usage of devm_kfree() as it's unnecessary in this function. Fixes: c263b4ef737e ("scsi: ufs: core: mcq: Configure resource regions") Reported-by: Dan Carpenter Signed-off-by: Asutosh Das Link: https://lore.kernel.org/r/3ebd2582af74b81ef7b57149f57c6a3bf0963953.1677721229.git.quic_asutoshd@quicinc.com Reviewed-by: Bjorn Andersson Reviewed-by: Manivannan Sadhasivam Signed-off-by: Martin K. Petersen --- drivers/ufs/host/ufs-qcom.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c index 2781739a66e0..a02cd866e2f8 100644 --- a/drivers/ufs/host/ufs-qcom.c +++ b/drivers/ufs/host/ufs-qcom.c @@ -1466,7 +1466,7 @@ static int ufs_qcom_mcq_config_resource(struct ufs_hba *hba) /* Explicitly allocate MCQ resource from ufs_mem */ res_mcq = devm_kzalloc(hba->dev, sizeof(*res_mcq), GFP_KERNEL); if (!res_mcq) - return ret; + return -ENOMEM; res_mcq->start = res_mem->start + MCQ_SQATTR_OFFSET(hba->mcq_capabilities); @@ -1478,7 +1478,7 @@ static int ufs_qcom_mcq_config_resource(struct ufs_hba *hba) if (ret) { dev_err(hba->dev, "Failed to insert MCQ resource, err=%d\n", ret); - goto insert_res_err; + return ret; } res->base = devm_ioremap_resource(hba->dev, res_mcq); @@ -1495,8 +1495,6 @@ out: ioremap_err: res->base = NULL; remove_resource(res_mcq); -insert_res_err: - devm_kfree(hba->dev, res_mcq); return ret; } From 312320b0e0ec21249a17645683fe5304d796aec1 Mon Sep 17 00:00:00 2001 From: Justin Tee Date: Mon, 27 Feb 2023 20:43:36 -0800 Subject: [PATCH 08/20] scsi: lpfc: Check kzalloc() in lpfc_sli4_cgn_params_read() If kzalloc() fails in lpfc_sli4_cgn_params_read(), then we rely on lpfc_read_object()'s routine to NULL check pdata. Currently, an early return error is thrown from lpfc_read_object() to protect us from NULL ptr dereference, but the errno code is -ENODEV. Change the errno code to a more appropriate -ENOMEM. Reported-by: Kang Chen Link: https://lore.kernel.org/all/20230226102338.3362585-1-void0red@gmail.com Signed-off-by: Justin Tee Link: https://lore.kernel.org/r/20230228044336.5195-1-justintee8345@gmail.com Signed-off-by: Martin K. Petersen --- drivers/scsi/lpfc/lpfc_init.c | 2 ++ drivers/scsi/lpfc/lpfc_sli.c | 4 ---- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c index 61958a24a43d..60e4ac2b8a6b 100644 --- a/drivers/scsi/lpfc/lpfc_init.c +++ b/drivers/scsi/lpfc/lpfc_init.c @@ -7291,6 +7291,8 @@ lpfc_sli4_cgn_params_read(struct lpfc_hba *phba) /* Find out if the FW has a new set of congestion parameters. */ len = sizeof(struct lpfc_cgn_param); pdata = kzalloc(len, GFP_KERNEL); + if (!pdata) + return -ENOMEM; ret = lpfc_read_object(phba, (char *)LPFC_PORT_CFG_NAME, pdata, len); diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c index c5b69f313af3..c5ad46fdde81 100644 --- a/drivers/scsi/lpfc/lpfc_sli.c +++ b/drivers/scsi/lpfc/lpfc_sli.c @@ -22109,10 +22109,6 @@ lpfc_read_object(struct lpfc_hba *phba, char *rdobject, uint32_t *datap, struct lpfc_dmabuf *pcmd; u32 rd_object_name[LPFC_MBX_OBJECT_NAME_LEN_DW] = {0}; - /* sanity check on queue memory */ - if (!datap) - return -ENODEV; - mbox = mempool_alloc(phba->mbox_mem_pool, GFP_KERNEL); if (!mbox) return -ENOMEM; From 2850b23e9f9ae3696e472d2883ea1b43aafa884e Mon Sep 17 00:00:00 2001 From: Jakob Koschel Date: Wed, 1 Mar 2023 18:19:14 +0100 Subject: [PATCH 09/20] scsi: lpfc: Avoid usage of list iterator variable after loop If the &epd_pool->list is empty when executing lpfc_get_io_buf_from_expedite_pool() the function would return an invalid pointer. Even in the case if the list is guaranteed to be populated, the iterator variable should not be used after the loop to be more robust for future changes. Linus proposed to avoid any use of the list iterator variable after the loop, in the attempt to move the list iterator variable declaration into the macro to avoid any potential misuse after the loop [1]. Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1] Signed-off-by: Jakob Koschel Link: https://lore.kernel.org/r/20230301-scsi-lpfc-avoid-list-iterator-after-loop-v1-1-325578ae7561@gmail.com Reviewed-by: Justin Tee Signed-off-by: Martin K. Petersen --- drivers/scsi/lpfc/lpfc_sli.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c index c5ad46fdde81..cf630aa6734e 100644 --- a/drivers/scsi/lpfc/lpfc_sli.c +++ b/drivers/scsi/lpfc/lpfc_sli.c @@ -21899,20 +21899,20 @@ lpfc_get_io_buf_from_private_pool(struct lpfc_hba *phba, static struct lpfc_io_buf * lpfc_get_io_buf_from_expedite_pool(struct lpfc_hba *phba) { - struct lpfc_io_buf *lpfc_ncmd; + struct lpfc_io_buf *lpfc_ncmd = NULL, *iter; struct lpfc_io_buf *lpfc_ncmd_next; unsigned long iflag; struct lpfc_epd_pool *epd_pool; epd_pool = &phba->epd_pool; - lpfc_ncmd = NULL; spin_lock_irqsave(&epd_pool->lock, iflag); if (epd_pool->count > 0) { - list_for_each_entry_safe(lpfc_ncmd, lpfc_ncmd_next, + list_for_each_entry_safe(iter, lpfc_ncmd_next, &epd_pool->list, list) { - list_del(&lpfc_ncmd->list); + list_del(&iter->list); epd_pool->count--; + lpfc_ncmd = iter; break; } } From 02ca7da2919ada525fb424640205110e24646b50 Mon Sep 17 00:00:00 2001 From: Ranjan Kumar Date: Tue, 28 Feb 2023 06:08:30 -0800 Subject: [PATCH 10/20] scsi: mpi3mr: ioctl timeout when disabling/enabling interrupt As part of Task Management handling, the driver will disable and enable the MSIx index zero which belongs to the Admin reply queue. During this transition the driver loses some interrupts and this leads to Admin request and ioctl timeouts. After enabling the interrupts, poll the Admin reply queue to avoid timeouts. Signed-off-by: Ranjan Kumar Signed-off-by: Sreekanth Reddy Link: https://lore.kernel.org/r/20230228140835.4075-2-ranjan.kumar@broadcom.com Signed-off-by: Martin K. Petersen --- 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(-) diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h index 23de2603e71f..40f238fa80cc 100644 --- a/drivers/scsi/mpi3mr/mpi3mr.h +++ b/drivers/scsi/mpi3mr/mpi3mr.h @@ -902,6 +902,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 @@ -1055,6 +1056,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; @@ -1390,4 +1392,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*/ diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c index 758f7ca9e0ee..4254e46a20f1 100644 --- a/drivers/scsi/mpi3mr/mpi3mr_fw.c +++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c @@ -415,7 +415,7 @@ out: 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; } @@ -2605,6 +2611,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, @@ -4155,6 +4162,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)); diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c index 6eaeba41072c..a794cc8a1c0b 100644 --- a/drivers/scsi/mpi3mr/mpi3mr_os.c +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c @@ -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: From 5b06a7169c59ce0c77ef8b9c82aa07c478f82aac Mon Sep 17 00:00:00 2001 From: Ranjan Kumar Date: Tue, 28 Feb 2023 06:08:31 -0800 Subject: [PATCH 11/20] scsi: mpi3mr: Driver unload crashes host when enhanced logging is enabled Prevent driver from trying to dereference a NULL pointer in a debug print while removing a device during driver unload. Signed-off-by: Ranjan Kumar Signed-off-by: Sreekanth Reddy Link: https://lore.kernel.org/r/20230228140835.4075-3-ranjan.kumar@broadcom.com Signed-off-by: Martin K. Petersen --- drivers/scsi/mpi3mr/mpi3mr_transport.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/mpi3mr/mpi3mr_transport.c b/drivers/scsi/mpi3mr/mpi3mr_transport.c index 3b61815979da..b795a325534d 100644 --- a/drivers/scsi/mpi3mr/mpi3mr_transport.c +++ b/drivers/scsi/mpi3mr/mpi3mr_transport.c @@ -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) From 0a319f1629495d27879b7ebf6eee62b8cf6e4c37 Mon Sep 17 00:00:00 2001 From: Ranjan Kumar Date: Tue, 28 Feb 2023 06:08:32 -0800 Subject: [PATCH 12/20] scsi: mpi3mr: Wait for diagnostic save during controller init If a controller reset operation is triggered to recover the controller from a fault state, then wait for the snapdump to be saved in the firmware region before proceeding to reset the controller. Signed-off-by: Ranjan Kumar Signed-off-by: Sreekanth Reddy Link: https://lore.kernel.org/r/20230228140835.4075-4-ranjan.kumar@broadcom.com Signed-off-by: Martin K. Petersen --- drivers/scsi/mpi3mr/mpi3mr_fw.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c index 4254e46a20f1..fa903a70baac 100644 --- a/drivers/scsi/mpi3mr/mpi3mr_fw.c +++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c @@ -1198,7 +1198,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; @@ -1252,6 +1252,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, From ba8a9ba41fbde250fd8b0ed1e5dad0dc9318df46 Mon Sep 17 00:00:00 2001 From: Ranjan Kumar Date: Tue, 28 Feb 2023 06:08:33 -0800 Subject: [PATCH 13/20] scsi: mpi3mr: Return proper values for failures in firmware init path Return proper non-zero return values for all the cases when the controller initialization and re-initialization fails. Signed-off-by: Ranjan Kumar Signed-off-by: Sreekanth Reddy Link: https://lore.kernel.org/r/20230228140835.4075-5-ranjan.kumar@broadcom.com Signed-off-by: Martin K. Petersen --- drivers/scsi/mpi3mr/mpi3mr_fw.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c index fa903a70baac..29acf6111db3 100644 --- a/drivers/scsi/mpi3mr/mpi3mr_fw.c +++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c @@ -3840,8 +3840,10 @@ retry_init: 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; @@ -3903,8 +3905,10 @@ retry_init: 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); @@ -3924,6 +3928,7 @@ out_failed: 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, @@ -4036,6 +4041,7 @@ retry_init: 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; } @@ -4102,6 +4108,7 @@ out_failed: 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"); From 4f297e856a7b5da2f2c66a12e739666e23943560 Mon Sep 17 00:00:00 2001 From: Ranjan Kumar Date: Tue, 28 Feb 2023 06:08:34 -0800 Subject: [PATCH 14/20] scsi: mpi3mr: NVMe command size greater than 8K fails A wrong variable is checked while populating PRP entries in the PRP page and this results in failure. No PRP entries in the PRP page were successfully created and any NVMe Encapsulated commands with PRP of size greater than 8K failed. Signed-off-by: Ranjan Kumar Signed-off-by: Sreekanth Reddy Link: https://lore.kernel.org/r/20230228140835.4075-6-ranjan.kumar@broadcom.com Signed-off-by: Martin K. Petersen --- drivers/scsi/mpi3mr/mpi3mr_app.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c index bff637702397..d10c6afb7f9c 100644 --- a/drivers/scsi/mpi3mr/mpi3mr_app.c +++ b/drivers/scsi/mpi3mr/mpi3mr_app.c @@ -886,7 +886,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__); @@ -895,7 +895,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; } /* From 8e45183978d64699df639e795235433a60f35047 Mon Sep 17 00:00:00 2001 From: Ranjan Kumar Date: Tue, 28 Feb 2023 06:08:35 -0800 Subject: [PATCH 15/20] scsi: mpi3mr: Bad drive in topology results kernel crash When the SAS Transport Layer support is enabled and a device exposed to the OS by the driver fails INQUIRY commands, 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, the freed memory is accessed and that leads to memory corruption and OS crash. Signed-off-by: Ranjan Kumar Signed-off-by: Sreekanth Reddy Link: https://lore.kernel.org/r/20230228140835.4075-7-ranjan.kumar@broadcom.com Signed-off-by: Martin K. Petersen --- drivers/scsi/mpi3mr/mpi3mr_transport.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/mpi3mr/mpi3mr_transport.c b/drivers/scsi/mpi3mr/mpi3mr_transport.c index b795a325534d..be25f242fa79 100644 --- a/drivers/scsi/mpi3mr/mpi3mr_transport.c +++ b/drivers/scsi/mpi3mr/mpi3mr_transport.c @@ -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); From bfa659177dcba48cf13f2bd88c1972f12a60bf1c Mon Sep 17 00:00:00 2001 From: Chandrakanth Patil Date: Thu, 2 Mar 2023 16:23:40 +0530 Subject: [PATCH 16/20] scsi: megaraid_sas: Update max supported LD IDs to 240 The firmware only supports Logical Disk IDs up to 240 and LD ID 255 (0xFF) is reserved for deleted LDs. However, in some cases, firmware was assigning LD ID 254 (0xFE) to deleted LDs and this was causing the driver to mark the wrong disk as deleted. This in turn caused the wrong disk device to be taken offline by the SCSI midlayer. To address this issue, limit the LD ID range from 255 to 240. This ensures the deleted LD ID is properly identified and removed by the driver without accidently deleting any valid LDs. Fixes: ae6874ba4b43 ("scsi: megaraid_sas: Early detection of VD deletion through RaidMap update") Reported-by: Martin K. Petersen Signed-off-by: Chandrakanth Patil Signed-off-by: Sumit Saxena Link: https://lore.kernel.org/r/20230302105342.34933-2-chandrakanth.patil@broadcom.com Signed-off-by: Martin K. Petersen --- drivers/scsi/megaraid/megaraid_sas.h | 2 ++ drivers/scsi/megaraid/megaraid_sas_fp.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/megaraid/megaraid_sas.h b/drivers/scsi/megaraid/megaraid_sas.h index 4919ea54b827..2ef9d41fc6f4 100644 --- a/drivers/scsi/megaraid/megaraid_sas.h +++ b/drivers/scsi/megaraid/megaraid_sas.h @@ -1519,6 +1519,8 @@ struct megasas_ctrl_info { #define MEGASAS_MAX_LD_IDS (MEGASAS_MAX_LD_CHANNELS * \ MEGASAS_MAX_DEV_PER_CHANNEL) +#define MEGASAS_MAX_SUPPORTED_LD_IDS 240 + #define MEGASAS_MAX_SECTORS (2*1024) #define MEGASAS_MAX_SECTORS_IEEE (2*128) #define MEGASAS_DBG_LVL 1 diff --git a/drivers/scsi/megaraid/megaraid_sas_fp.c b/drivers/scsi/megaraid/megaraid_sas_fp.c index da1cad1ee123..4463a538102a 100644 --- a/drivers/scsi/megaraid/megaraid_sas_fp.c +++ b/drivers/scsi/megaraid/megaraid_sas_fp.c @@ -358,7 +358,7 @@ u8 MR_ValidateMapInfo(struct megasas_instance *instance, u64 map_id) ld = MR_TargetIdToLdGet(i, drv_map); /* For non existing VDs, iterate to next VD*/ - if (ld >= (MAX_LOGICAL_DRIVES_EXT - 1)) + if (ld >= MEGASAS_MAX_SUPPORTED_LD_IDS) continue; raid = MR_LdRaidGet(ld, drv_map); From 9bcb1d5a3d10f4d15da2174f8eaec942602f681f Mon Sep 17 00:00:00 2001 From: Chandrakanth Patil Date: Thu, 2 Mar 2023 16:23:41 +0530 Subject: [PATCH 17/20] scsi: megaraid_sas: Add crash dump mode capability bit in MFI capabilities In kdump kernel mode, the driver works in reduced functionality mode with some features disabled such as reduced MSI-X count and RDPQ disabled, etc. However, the firmware is not aware of this mode in some cases, which results in undefined behavior. To address this, the driver informs the firmware about the kdump mode through MPI capabilities bit during driver initialization. This allows firmware to adjust its behavior accordingly. Signed-off-by: Chandrakanth Patil Signed-off-by: Sumit Saxena Link: https://lore.kernel.org/r/20230302105342.34933-3-chandrakanth.patil@broadcom.com Signed-off-by: Martin K. Petersen --- drivers/scsi/megaraid/megaraid_sas.h | 6 ++++-- drivers/scsi/megaraid/megaraid_sas_fusion.c | 3 +++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/megaraid/megaraid_sas.h b/drivers/scsi/megaraid/megaraid_sas.h index 2ef9d41fc6f4..c2bace65105b 100644 --- a/drivers/scsi/megaraid/megaraid_sas.h +++ b/drivers/scsi/megaraid/megaraid_sas.h @@ -1760,7 +1760,8 @@ union megasas_sgl_frame { typedef union _MFI_CAPABILITIES { struct { #if defined(__BIG_ENDIAN_BITFIELD) - u32 reserved:16; + u32 reserved:15; + u32 support_memdump:1; u32 support_fw_exposed_dev_list:1; u32 support_nvme_passthru:1; u32 support_64bit_mode:1; @@ -1794,7 +1795,8 @@ typedef union _MFI_CAPABILITIES { u32 support_64bit_mode:1; u32 support_nvme_passthru:1; u32 support_fw_exposed_dev_list:1; - u32 reserved:16; + u32 support_memdump:1; + u32 reserved:15; #endif } mfi_capabilities; __le32 reg; diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c b/drivers/scsi/megaraid/megaraid_sas_fusion.c index 6597e118c805..84c9a55a5794 100644 --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c @@ -1201,6 +1201,9 @@ megasas_ioc_init_fusion(struct megasas_instance *instance) drv_ops->mfi_capabilities.support_nvme_passthru = 1; drv_ops->mfi_capabilities.support_fw_exposed_dev_list = 1; + if (reset_devices) + drv_ops->mfi_capabilities.support_memdump = 1; + if (instance->consistent_mask_64bit) drv_ops->mfi_capabilities.support_64bit_mode = 1; From a2033f9f9d785972513bca207ed4a48bffc75f8f Mon Sep 17 00:00:00 2001 From: Chandrakanth Patil Date: Thu, 2 Mar 2023 16:23:42 +0530 Subject: [PATCH 18/20] scsi: megaraid_sas: Driver version update to 07.725.01.00-rc1 Update driver version. Signed-off-by: Chandrakanth Patil Link: https://lore.kernel.org/r/20230302105342.34933-4-chandrakanth.patil@broadcom.com Signed-off-by: Martin K. Petersen --- drivers/scsi/megaraid/megaraid_sas.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/megaraid/megaraid_sas.h b/drivers/scsi/megaraid/megaraid_sas.h index c2bace65105b..63bac3684c19 100644 --- a/drivers/scsi/megaraid/megaraid_sas.h +++ b/drivers/scsi/megaraid/megaraid_sas.h @@ -23,8 +23,8 @@ /* * MegaRAID SAS Driver meta data */ -#define MEGASAS_VERSION "07.719.03.00-rc1" -#define MEGASAS_RELDATE "Sep 29, 2021" +#define MEGASAS_VERSION "07.725.01.00-rc1" +#define MEGASAS_RELDATE "Mar 2, 2023" #define MEGASAS_MSIX_NAME_LEN 32 From 11d9874c4204a785f43d899a1ab12f9dc8d9de3e Mon Sep 17 00:00:00 2001 From: Michael Kelley Date: Mon, 27 Feb 2023 08:48:34 -0800 Subject: [PATCH 19/20] scsi: storvsc: Handle BlockSize change in Hyper-V VHD/VHDX file Hyper-V uses a VHD or VHDX file on the host as the underlying storage for a virtual disk. The VHD/VHDX file format is a sparse format where real disk space on the host is assigned in chunks that the VHD/VHDX file format calls the BlockSize. This BlockSize is not to be confused with the 512-byte (or 4096-byte) sector size of the underlying storage device. The default block size for a new VHD/VHDX file is 32 Mbytes. When a guest VM touches any disk space within a 32 Mbyte chunk of the VHD/VHDX file, Hyper-V allocates 32 Mbytes of real disk space for that section of the VHD/VHDX. Similarly, if a discard operation is done that covers an entire 32 Mbyte chunk, Hyper-V will free the real disk space for that portion of the VHD/VHDX. This BlockSize is surfaced in Linux as the "discard_granularity" in /sys/block/sd/queue, which makes sense. Hyper-V also has differencing disks that can overlay a VHD/VHDX file to capture changes to the VHD/VHDX while preserving the original VHD/VHDX. One example of this differencing functionality is for VM snapshots. When a snapshot is created, a differencing disk is created. If the snapshot is rolled back, Hyper-V can just delete the differencing disk, and the VM will see the original disk contents at the time the snapshot was taken. Differencing disks are used in other scenarios as well. The BlockSize for a differencing disk defaults to 2 Mbytes, not 32 Mbytes. The smaller default is used because changes to differencing disks are typically scattered all over, and Hyper-V doesn't want to allocate 32 Mbytes of real disk space for a stray write here or there. The smaller BlockSize provides more efficient use of real disk space. When a differencing disk is added to a VHD/VHDX, Hyper-V reports UNIT_ATTENTION with a sense code indicating "Operating parameters have changed", because the value of discard_granularity should be changed to 2 Mbytes. When the differencing disk is removed, discard_granularity should be changed back to 32 Mbytes. However, current code simply reports a message from scsi_report_sense() and the value of /sys/block/sd/queue/discard_granularity is not updated. The message isn't very actionable by a sysadmin. Fix this by having the storvsc driver check for the sense code indicating that the underly VHD/VHDX block size has changed, and do a rescan of the device to pick up the new discard_granularity. With this change the entire transition to/from differencing disks is handled automatically and transparently, with no confusing messages being output. Link: https://lore.kernel.org/r/1677516514-86060-1-git-send-email-mikelley@microsoft.com Signed-off-by: Michael Kelley Signed-off-by: Martin K. Petersen --- drivers/scsi/storvsc_drv.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index 33f568b7f54d..d9ce379c4d2e 100644 --- a/drivers/scsi/storvsc_drv.c +++ b/drivers/scsi/storvsc_drv.c @@ -987,6 +987,22 @@ static void storvsc_handle_error(struct vmscsi_request *vm_srb, goto do_work; } + /* + * Check for "Operating parameters have changed" + * due to Hyper-V changing the VHD/VHDX BlockSize + * when adding/removing a differencing disk. This + * causes discard_granularity to change, so do a + * rescan to pick up the new granularity. We don't + * want scsi_report_sense() to output a message + * that a sysadmin wouldn't know what to do with. + */ + if ((asc == 0x3f) && (ascq != 0x03) && + (ascq != 0x0e)) { + process_err_fn = storvsc_device_scan; + set_host_byte(scmnd, DID_REQUEUE); + goto do_work; + } + /* * Otherwise, let upper layer deal with the * error when sense message is present From 288b3271d920c9ba949c3bab0f749f4cecc70e09 Mon Sep 17 00:00:00 2001 From: Shin'ichiro Kawasaki Date: Mon, 6 Mar 2023 15:30:24 +0900 Subject: [PATCH 20/20] scsi: sd: Fix wrong zone_write_granularity value during revalidate When the sd driver revalidates host-managed SMR disks, it calls disk_set_zoned() which changes the zone_write_granularity attribute value to the logical block size regardless of the device type. After that, the sd driver overwrites the value in sd_zbc_read_zone() with the physical block size, since ZBC/ZAC requires this for host-managed disks. Between the calls to disk_set_zoned() and sd_zbc_read_zone(), there exists a window where the attribute shows the logical block size as the zone_write_granularity value, which is wrong for host-managed disks. The duration of the window is from 20ms to 200ms, depending on report zone command execution time. To avoid the wrong zone_write_granularity value between disk_set_zoned() and sd_zbc_read_zone(), modify the value not in sd_zbc_read_zone() but just after disk_set_zoned() call. Fixes: a805a4fa4fa3 ("block: introduce zone_write_granularity limit") Signed-off-by: Shin'ichiro Kawasaki Link: https://lore.kernel.org/r/20230306063024.3376959-1-shinichiro.kawasaki@wdc.com Reviewed-by: Damien Le Moal Reviewed-by: Johannes Thumshirn Reviewed-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/scsi/sd.c | 7 ++++++- drivers/scsi/sd_zbc.c | 8 -------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 4f28dd617eca..4bb87043e6db 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -2988,8 +2988,13 @@ static void sd_read_block_characteristics(struct scsi_disk *sdkp) } if (sdkp->device->type == TYPE_ZBC) { - /* Host-managed */ + /* + * Host-managed: Per ZBC and ZAC specifications, writes in + * sequential write required zones of host-managed devices must + * be aligned to the device physical block size. + */ disk_set_zoned(sdkp->disk, BLK_ZONED_HM); + blk_queue_zone_write_granularity(q, sdkp->physical_block_size); } else { sdkp->zoned = zoned; if (sdkp->zoned == 1) { diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c index 6b3a02d4406c..22801c24ea19 100644 --- a/drivers/scsi/sd_zbc.c +++ b/drivers/scsi/sd_zbc.c @@ -965,14 +965,6 @@ int sd_zbc_read_zones(struct scsi_disk *sdkp, u8 buf[SD_BUF_SIZE]) disk_set_max_active_zones(disk, 0); nr_zones = round_up(sdkp->capacity, zone_blocks) >> ilog2(zone_blocks); - /* - * Per ZBC and ZAC specifications, writes in sequential write required - * zones of host-managed devices must be aligned to the device physical - * block size. - */ - if (blk_queue_zoned_model(q) == BLK_ZONED_HM) - blk_queue_zone_write_granularity(q, sdkp->physical_block_size); - sdkp->early_zone_info.nr_zones = nr_zones; sdkp->early_zone_info.zone_blocks = zone_blocks;