From: Po-Wen Kao <powen.kao@mediatek.com> To: <linux-scsi@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>, <linux-mediatek@lists.infradead.org>, Alim Akhtar <alim.akhtar@samsung.com>, Avri Altman <avri.altman@wdc.com>, Bart Van Assche <bvanassche@acm.org>, "James E.J. Bottomley" <jejb@linux.ibm.com>, "Martin K. Petersen" <martin.petersen@oracle.com>, Matthias Brugger <matthias.bgg@gmail.com>, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> Cc: <wsd_upstream@mediatek.com>, <peter.wang@mediatek.com>, <stanley.chu@mediatek.com>, <powen.kao@mediatek.com>, <alice.chao@mediatek.com>, <naomi.chu@mediatek.com>, <chun-hung.wu@mediatek.com>, <cc.chou@mediatek.com>, <eddie.huang@mediatek.com> Subject: [PATCH v2 1/2] scsi: ufs: mcq: Fix the incorrect OCS value for the device command Date: Sat, 10 Jun 2023 10:15:51 +0800 [thread overview] Message-ID: <20230610021553.1213-2-powen.kao@mediatek.com> (raw) In-Reply-To: <20230610021553.1213-1-powen.kao@mediatek.com> From: Stanley Chu <stanley.chu@mediatek.com> In MCQ mode, when a device command uses a hardware queue shared with other commands, a race condition may occur in the following scenario: 1. A device command is completed in CQx with CQE entry "e". 2. The interrupt handler copies the "cqe" pointer to "hba->dev_cmd.cqe" and completes "hba->dev_cmd.complete". 3. The "ufshcd_wait_for_dev_cmd()" function is awakened and retrieves the OCS value from "hba->dev_cmd.cqe". However, there is a possibility that the CQE entry "e" will be overwritten by newly completed commands in CQx, resulting in an incorrect OCS value being received by "ufshcd_wait_for_dev_cmd()". To avoid this race condition, the OCS value should be immediately copied to the struct "lrb" of the device command. Then "ufshcd_wait_for_dev_cmd()" can retrieve the OCS value from the struct "lrb". Fixes: b5167638ec82 ("scsi: ufs: core: mcq: Add support to allocate multiple queues") Suggested-by: Can Guo <quic_cang@quicinc.com> Signed-off-by: Stanley Chu <stanley.chu@mediatek.com> Tested-by: Po-Wen Kao <powen.kao@mediatek.com> --- drivers/ufs/core/ufshcd.c | 10 +++++++--- include/ufs/ufshcd.h | 1 - 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 5da62248ebc4..593790fa4837 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -3086,7 +3086,7 @@ static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba, * not trigger any race conditions. */ hba->dev_cmd.complete = NULL; - err = ufshcd_get_tr_ocs(lrbp, hba->dev_cmd.cqe); + err = ufshcd_get_tr_ocs(lrbp, NULL); if (!err) err = ufshcd_dev_cmd_completion(hba, lrbp); } else { @@ -3182,7 +3182,6 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba, goto out; hba->dev_cmd.complete = &wait; - hba->dev_cmd.cqe = NULL; ufshcd_add_query_upiu_trace(hba, UFS_QUERY_SEND, lrbp->ucd_req_ptr); @@ -5431,6 +5430,7 @@ void ufshcd_compl_one_cqe(struct ufs_hba *hba, int task_tag, { struct ufshcd_lrb *lrbp; struct scsi_cmnd *cmd; + enum utp_ocs ocs; lrbp = &hba->lrb[task_tag]; lrbp->compl_time_stamp = ktime_get(); @@ -5446,7 +5446,11 @@ void ufshcd_compl_one_cqe(struct ufs_hba *hba, int task_tag, } else if (lrbp->command_type == UTP_CMD_TYPE_DEV_MANAGE || lrbp->command_type == UTP_CMD_TYPE_UFS_STORAGE) { if (hba->dev_cmd.complete) { - hba->dev_cmd.cqe = cqe; + if (cqe) { + ocs = le32_to_cpu(cqe->status) & MASK_OCS; + lrbp->utr_descriptor_ptr->header.dword_2 = + cpu_to_le32(ocs); + } complete(hba->dev_cmd.complete); ufshcd_clk_scaling_update_busy(hba); } diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h index 9b2d1859f885..602615e6d1bf 100644 --- a/include/ufs/ufshcd.h +++ b/include/ufs/ufshcd.h @@ -225,7 +225,6 @@ struct ufs_dev_cmd { struct mutex lock; struct completion *complete; struct ufs_query query; - struct cq_entry *cqe; }; /** -- 2.18.0
WARNING: multiple messages have this Message-ID (diff)
From: Po-Wen Kao <powen.kao@mediatek.com> To: <linux-scsi@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>, <linux-mediatek@lists.infradead.org>, Alim Akhtar <alim.akhtar@samsung.com>, Avri Altman <avri.altman@wdc.com>, Bart Van Assche <bvanassche@acm.org>, "James E.J. Bottomley" <jejb@linux.ibm.com>, "Martin K. Petersen" <martin.petersen@oracle.com>, Matthias Brugger <matthias.bgg@gmail.com>, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> Cc: <wsd_upstream@mediatek.com>, <peter.wang@mediatek.com>, <stanley.chu@mediatek.com>, <powen.kao@mediatek.com>, <alice.chao@mediatek.com>, <naomi.chu@mediatek.com>, <chun-hung.wu@mediatek.com>, <cc.chou@mediatek.com>, <eddie.huang@mediatek.com> Subject: [PATCH v2 1/2] scsi: ufs: mcq: Fix the incorrect OCS value for the device command Date: Sat, 10 Jun 2023 10:15:51 +0800 [thread overview] Message-ID: <20230610021553.1213-2-powen.kao@mediatek.com> (raw) In-Reply-To: <20230610021553.1213-1-powen.kao@mediatek.com> From: Stanley Chu <stanley.chu@mediatek.com> In MCQ mode, when a device command uses a hardware queue shared with other commands, a race condition may occur in the following scenario: 1. A device command is completed in CQx with CQE entry "e". 2. The interrupt handler copies the "cqe" pointer to "hba->dev_cmd.cqe" and completes "hba->dev_cmd.complete". 3. The "ufshcd_wait_for_dev_cmd()" function is awakened and retrieves the OCS value from "hba->dev_cmd.cqe". However, there is a possibility that the CQE entry "e" will be overwritten by newly completed commands in CQx, resulting in an incorrect OCS value being received by "ufshcd_wait_for_dev_cmd()". To avoid this race condition, the OCS value should be immediately copied to the struct "lrb" of the device command. Then "ufshcd_wait_for_dev_cmd()" can retrieve the OCS value from the struct "lrb". Fixes: b5167638ec82 ("scsi: ufs: core: mcq: Add support to allocate multiple queues") Suggested-by: Can Guo <quic_cang@quicinc.com> Signed-off-by: Stanley Chu <stanley.chu@mediatek.com> Tested-by: Po-Wen Kao <powen.kao@mediatek.com> --- drivers/ufs/core/ufshcd.c | 10 +++++++--- include/ufs/ufshcd.h | 1 - 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 5da62248ebc4..593790fa4837 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -3086,7 +3086,7 @@ static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba, * not trigger any race conditions. */ hba->dev_cmd.complete = NULL; - err = ufshcd_get_tr_ocs(lrbp, hba->dev_cmd.cqe); + err = ufshcd_get_tr_ocs(lrbp, NULL); if (!err) err = ufshcd_dev_cmd_completion(hba, lrbp); } else { @@ -3182,7 +3182,6 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba, goto out; hba->dev_cmd.complete = &wait; - hba->dev_cmd.cqe = NULL; ufshcd_add_query_upiu_trace(hba, UFS_QUERY_SEND, lrbp->ucd_req_ptr); @@ -5431,6 +5430,7 @@ void ufshcd_compl_one_cqe(struct ufs_hba *hba, int task_tag, { struct ufshcd_lrb *lrbp; struct scsi_cmnd *cmd; + enum utp_ocs ocs; lrbp = &hba->lrb[task_tag]; lrbp->compl_time_stamp = ktime_get(); @@ -5446,7 +5446,11 @@ void ufshcd_compl_one_cqe(struct ufs_hba *hba, int task_tag, } else if (lrbp->command_type == UTP_CMD_TYPE_DEV_MANAGE || lrbp->command_type == UTP_CMD_TYPE_UFS_STORAGE) { if (hba->dev_cmd.complete) { - hba->dev_cmd.cqe = cqe; + if (cqe) { + ocs = le32_to_cpu(cqe->status) & MASK_OCS; + lrbp->utr_descriptor_ptr->header.dword_2 = + cpu_to_le32(ocs); + } complete(hba->dev_cmd.complete); ufshcd_clk_scaling_update_busy(hba); } diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h index 9b2d1859f885..602615e6d1bf 100644 --- a/include/ufs/ufshcd.h +++ b/include/ufs/ufshcd.h @@ -225,7 +225,6 @@ struct ufs_dev_cmd { struct mutex lock; struct completion *complete; struct ufs_query query; - struct cq_entry *cqe; }; /** -- 2.18.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-06-10 2:16 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-06-10 2:15 [PATCH v2 0/2] ufs: mcq: Share first hwq for dev comamnd and IO request Po-Wen Kao 2023-06-10 2:15 ` Po-Wen Kao 2023-06-10 2:15 ` Po-Wen Kao [this message] 2023-06-10 2:15 ` [PATCH v2 1/2] scsi: ufs: mcq: Fix the incorrect OCS value for the device command Po-Wen Kao 2023-06-11 13:59 ` Bart Van Assche 2023-06-11 14:49 ` Stanley Chu 2023-06-11 15:19 ` Bart Van Assche 2023-06-10 2:15 ` [PATCH v2 2/2] scsi: ufs: core: Remove dedicated hwq for dev command Po-Wen Kao 2023-06-10 2:15 ` Po-Wen Kao 2023-06-10 3:00 ` Stanley Chu 2023-06-10 3:00 ` Stanley Chu 2023-06-11 14:04 ` Bart Van Assche 2023-06-15 1:38 ` [PATCH v2 0/2] ufs: mcq: Share first hwq for dev comamnd and IO request Martin K. Petersen 2023-06-15 1:38 ` Martin K. Petersen 2023-06-22 1:26 ` Martin K. Petersen 2023-06-22 1:26 ` Martin K. Petersen
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20230610021553.1213-2-powen.kao@mediatek.com \ --to=powen.kao@mediatek.com \ --cc=alice.chao@mediatek.com \ --cc=alim.akhtar@samsung.com \ --cc=angelogioacchino.delregno@collabora.com \ --cc=avri.altman@wdc.com \ --cc=bvanassche@acm.org \ --cc=cc.chou@mediatek.com \ --cc=chun-hung.wu@mediatek.com \ --cc=eddie.huang@mediatek.com \ --cc=jejb@linux.ibm.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mediatek@lists.infradead.org \ --cc=linux-scsi@vger.kernel.org \ --cc=martin.petersen@oracle.com \ --cc=matthias.bgg@gmail.com \ --cc=naomi.chu@mediatek.com \ --cc=peter.wang@mediatek.com \ --cc=stanley.chu@mediatek.com \ --cc=wsd_upstream@mediatek.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.