Commit 0fef6bb7 authored by Stanley Chu's avatar Stanley Chu Committed by Martin K. Petersen
Browse files

scsi: ufs: core: mcq: Fix the incorrect OCS value for the device command 



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: 57b1c0ef ("scsi: ufs: core: mcq: Add support to allocate multiple queues")
Suggested-by: default avatarCan Guo <quic_cang@quicinc.com>
Signed-off-by: default avatarStanley Chu <stanley.chu@mediatek.com>
Link: https://lore.kernel.org/r/20230610021553.1213-2-powen.kao@mediatek.com


Tested-by: default avatarPo-Wen Kao <powen.kao@mediatek.com>
Reviewed-by: default avatarBart Van Assche <bvanassche@acm.org>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
parent e246514a

drivers/ufs/core/ufshcd.c

+7 −3
Original line number Original line Diff line number Diff line
@@ -3086,7 +3086,7 @@ static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba, 
		 * not trigger any race conditions.

		 * not trigger any race conditions.

		 */

		 */

		hba->dev_cmd.complete = NULL;

		hba->dev_cmd.complete = NULL;

		err = ufshcd_get_tr_ocs(lrbp, hba->dev_cmd.cqe);

		err = ufshcd_get_tr_ocs(lrbp, NULL);

		if (!err)

		if (!err)

			err = ufshcd_dev_cmd_completion(hba, lrbp);

			err = ufshcd_dev_cmd_completion(hba, lrbp);

	} else {

	} else {
@@ -3182,7 +3182,6 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba, 
		goto out;

		goto out;





	hba->dev_cmd.complete = &wait;

	hba->dev_cmd.complete = &wait;

	hba->dev_cmd.cqe = NULL;





	ufshcd_add_query_upiu_trace(hba, UFS_QUERY_SEND, lrbp->ucd_req_ptr);

	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 ufshcd_lrb *lrbp;

	struct scsi_cmnd *cmd;

	struct scsi_cmnd *cmd;

	enum utp_ocs ocs;





	lrbp = &hba->lrb[task_tag];

	lrbp = &hba->lrb[task_tag];

	lrbp->compl_time_stamp = ktime_get();

	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 ||

	} else if (lrbp->command_type == UTP_CMD_TYPE_DEV_MANAGE ||

		   lrbp->command_type == UTP_CMD_TYPE_UFS_STORAGE) {

		   lrbp->command_type == UTP_CMD_TYPE_UFS_STORAGE) {

		if (hba->dev_cmd.complete) {

		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);

			complete(hba->dev_cmd.complete);

			ufshcd_clk_scaling_update_busy(hba);

			ufshcd_clk_scaling_update_busy(hba);

		}

		}

include/ufs/ufshcd.h

+0 −1
Original line number Original line Diff line number Diff line
@@ -225,7 +225,6 @@ struct ufs_dev_cmd { 
	struct mutex lock;

	struct mutex lock;

	struct completion *complete;

	struct completion *complete;

	struct ufs_query query;

	struct ufs_query query;

	struct cq_entry *cqe;

};

};





/**

/**