[PATCH v5 0/2] ufs: fix the abort path of device management commands in MCQ mode

[PATCH v5 0/2] ufs: fix the abort path of device management commands in MCQ mode

[Kiwoong Kim]

This series fixes how a timed-out device management command (query, NOP)
is aborted in MCQ mode.

Patch 1 makes ufshcd_try_to_abort_task() skip the task management path
for device management commands and clear them with ufshcd_clear_cmd()
directly, as the single doorbell path in ufshcd_abort() already does.
Since the conversion to scsi_get_internal_cmd(), ufshcd_mcq_abort() sends
these commands to the QUERY TASK / ABORT TASK path, which is meant for
SCSI I/O commands.

Patch 2 makes ufshcd_mcq_sq_cleanup() wait for the OCS of a device
management command to be updated after SQ cleanup. UFSHCI does not
specify the order between the OCS update and the SQ_CUS update. Without
the wait, a late completion of the cleaned-up query can race with the
retry of that query, which leaves the host with two query contexts and
raises IS.UTPES.

Patch 2 depends on patch 1. Without patch 1, ufshcd_mcq_sq_cleanup() is
reached for a device management command only if the ABORT TASK TMF
succeeds.

Changes since v4:
- Read the OCS in ufshcd_mcq_sq_cleanup() with READ_ONCE() directly and
  drop the cq_lock and the ufshcd_mcq_read_ocs() helper, per review. (Bart)
- Report an error when the OCS poll times out. (Bart)

Link: https://lore.kernel.org/linux-scsi/cover.1793598099.git.kwmad.kim@samsung.com/

Changes since v3:
- Split the v3 patch into two patches: the abort path fix and
  the OCS poll after SQ cleanup (Bean Huo).
- Explain in patch 1 that the problem is only reachable since the
  scsi_get_internal_cmd() conversion and add the corresponding Fixes tag,
  per review.
- Add a Fixes tag to patch 2. (Bean Huo)
- Fix the step numbering of the timeline in the patch 2 commit message.

Link: https://lore.kernel.org/linux-scsi/20261030111045.982094-1-kwmad.kim@samsung.com/

Changes since v2:
- Use read_poll_timeout() for the OCS poll in ufshcd_mcq_sq_cleanup().
  The open-coded loop did not re-read the OCS after the timeout, so it
  could return a false -ETIMEDOUT if the OCS was updated while sleeping
  in the last iteration (Sashiko).

Link: https://lore.kernel.org/linux-scsi/20261030093645.790159-1-kwmad.kim@samsung.com/

Changes since v1:
- Limit the OCS poll in ufshcd_mcq_sq_cleanup() to device management
  commands. ufshcd_compl_one_cqe() copies the OCS from the CQE into the
  UTRD only for them, so for SCSI commands the poll always ran for
  MCQ_POLL_US and then failed with -ETIMEDOUT (Sashiko).
- Return -ETIMEDOUT only if OCS is still OCS_INVALID_COMMAND_STATUS
  after the poll. OCS_ABORTED, which the host controller reports for
  the cleaned-up request, is no longer treated as a cleanup failure
  (Sashiko).
- Update the commit message to match.

Link: https://lore.kernel.org/linux-scsi/20261030083311.602909-1-kwmad.kim@samsung.com/

Kiwoong Kim (2):
  ufs: core: skip ABORT TASK for device management commands
  ufs: mcq: poll OCS after sq cleanup for device management commands

 drivers/ufs/core/ufs-mcq.c | 18 ++++++++++++++++++
 drivers/ufs/core/ufshcd.c  |  4 ++++
 2 files changed, 22 insertions(+)

-- 
2.43.0

[PATCH v5 1/2] ufs: core: skip ABORT TASK for device management commands

[Kiwoong Kim]

In MCQ mode, ufshcd_mcq_abort() falls through to ufshcd_try_to_abort_task()
for all timed-out commands. However, device management commands (query, NOP)
use reserved requests and are not SCSI I/O commands. The ABORT TASK task
management function is designed for SCSI I/O UPIUs and is not applicable to
device management UPIUs.

This is only reachable since device management commands were converted to
scsi_get_internal_cmd(); before that, a timed-out query was handled by
ufshcd_wait_for_dev_cmd() -> ufshcd_clear_cmd() and never reached
ufshcd_try_to_abort_task().

Fix this by checking ufshcd_is_scsi_cmd() in ufshcd_try_to_abort_task(). When
the request is a reserved (device management) command, skip the TM path and
call ufshcd_clear_cmd() directly instead.

Fixes: 08b12cda6c44 ("scsi: ufs: core: Switch to scsi_get_internal_cmd()")
Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
Reviewed-by: Bean Huo <beanhuo@micron.com>
Reported-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index bcc2512bf34b..7fc80926eeec 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8046,6 +8046,9 @@ int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
 
 	lrbp = scsi_cmd_priv(cmd);
 
+	if (!ufshcd_is_scsi_cmd(cmd))
+		goto clear_cmd;
+
 	for (poll_cnt = 100; poll_cnt; poll_cnt--) {
 		err = ufshcd_issue_tm_cmd(hba, lrbp->lun, tag, UFS_QUERY_TASK,
 					  &resp);
@@ -8091,6 +8094,7 @@ int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
 		return err;
 	}
 
+clear_cmd:
 	err = ufshcd_clear_cmd(hba, tag);
 	if (err)
 		dev_err(hba->dev, "%s: Failed clearing cmd at tag %d, err %d\n",
-- 
2.43.0

[PATCH v5 2/2] ufs: mcq: poll OCS after sq cleanup for device management commands

[Kiwoong Kim]

Extend ufshcd_mcq_sq_cleanup() to poll the OCS field in the UTRD of a
device management command after a successful SQ_CUS poll. UFSHCI does not
specify the order between the OCS update and the SQ_CUS update, so OCS may
still be stale when cleanup returns. Without this poll, a late response to
a timed-out query can arrive while a retry is already in flight, leaving
the host with two query contexts and raising IS.UTPES.

The poll is limited to device management commands because
ufshcd_compl_one_cqe() copies the OCS from the CQE into the UTRD only for
them. For SCSI commands, the OCS in the UTRD stays
OCS_INVALID_COMMAND_STATUS, so polling it would always time out.

The poll waits until OCS is no longer OCS_INVALID_COMMAND_STATUS or
MCQ_POLL_US elapses. -ETIMEDOUT is returned only if OCS is still
OCS_INVALID_COMMAND_STATUS after the poll. Any other value, such as
OCS_ABORTED reported by the host controller for the cleaned-up request,
means the cleanup has completed.

Here is the timeline of an actual case for this that happened with kernel 6.18.
1. The host sent a query but the device didn't respond with it for a long time.

[ 7384.156142] [4:  kworker/u40:5:10671] exynos-ufs 38300000.ufs: ufshcd_mcq_sq_cleanup, hwq 0: cleanup return code (RTC) 0
[ 7384.156154] [4:  kworker/u40:5:10671] exynos-ufs 38300000.ufs: ufshcd_query_flag: Sending flag query for idn 14 failed, err = -11

2. So after the query was timed out, the driver did SQ cleanup.
Then the host updated CQ update and raised IS.CQES,
Unfortunately, complete() was called the driver's ISR afterward.

3. The driver started to retried the query. In the meantime, it set OCS to 0xF.
Then the driver didn't wait for completion due to the previous complete() call,
and just checked OCS.

[ 7384.156163] [4:  kworker/u40:5:10671] exynos-ufs 38300000.ufs: ufshcd_query_flag: Sending flag query for idn 14 failed, err = 15

4. The driver retried the query again, but the device didn't return anything,
so the query was also timed-out.

[ 7385.660108] [3:  kworker/u40:5:10671] exynos-ufs 38300000.ufs: ufshcd_query_flag: Sending flag query for idn 14 failed, err = -11

5. A few seconds later, the device returned the response of the 1st query,
but since the host was holding two query contexts and couldn't handle it,
it raised IS.UTPES.

Fixes: 8d7290348992 ("scsi: ufs: mcq: Add supporting functions for MCQ abort")
Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
---
 drivers/ufs/core/ufs-mcq.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 8106d55f4041..100295d02c62 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -609,6 +609,24 @@ int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag)
 			 __func__, id,
 			 FIELD_GET(SQ_ICU_ERR_CODE_MASK, readl(reg)));
 
+	/*
+	 * For device management commands, there is no order guaranteed between
+	 * OCS update and CUS update, which is why we need to wait here. SCSI
+	 * commands are excluded because the OCS in their UTRD is not updated
+	 * from the CQE.
+	 */
+	if (!err && blk_mq_is_reserved_rq(rq)) {
+		enum utp_ocs ocs;
+
+		err = read_poll_timeout(READ_ONCE, ocs,
+					ocs != OCS_INVALID_COMMAND_STATUS, 20,
+					MCQ_POLL_US, false,
+					lrbp->utr_descriptor_ptr->header.ocs);
+		if (err)
+			dev_err(hba->dev, "%s: OCS not updated. hwq=%d, tag=%d err=%d\n",
+				__func__, id, task_tag, err);
+	}
+
 	if (ufshcd_mcq_sq_start(hba, hwq))
 		err = -ETIMEDOUT;
 
-- 
2.43.0

Re: [PATCH v5 2/2] ufs: mcq: poll OCS after sq cleanup for device management commands

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ufs: mcq: race between OCS poll in ufshcd_mcq_sq_cleanup() and CQE completion
--

commit ed08af75bc0d7555962a67f5dd8315bda77ee7a4
Author: Kiwoong Kim <kwmad.kim@samsung.com>

ufs: mcq: poll OCS after sq cleanup for device management commands

This commit extends ufshcd_mcq_sq_cleanup() to poll the UTRD header.ocs
field for reserved (device management) requests after SQ_CUS is set, so
that cleanup waits for the OCS update before returning.

> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
> index 25e392f8ede2b..8857775586311 100644
> --- a/drivers/ufs/core/ufs-mcq.c
> +++ b/drivers/ufs/core/ufs-mcq.c
> @@ -613,6 +613,24 @@ int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag)
>  			 __func__, id,
>  			 FIELD_GET(SQ_ICU_ERR_CODE_MASK, readl(reg)));
>  
> +	/*
> +	 * For device management commands, there is no order guaranteed between
> +	 * OCS update and CUS update, which is why we need to wait here. SCSI
> +	 * commands are excluded because the OCS in their UTRD is not updated
> +	 * from the CQE.
> +	 */
> +	if (!err && blk_mq_is_reserved_rq(rq)) {
> +		enum utp_ocs ocs;
> +
> +		err = read_poll_timeout(READ_ONCE, ocs,
> +					ocs != OCS_INVALID_COMMAND_STATUS, 20,
> +					MCQ_POLL_US, false,
> +					lrbp->utr_descriptor_ptr->header.ocs);
> +		if (err)
> +			dev_err(hba->dev, "%s: OCS not updated. hwq=%d, tag=%d err=%d\n",
> +				__func__, id, task_tag, err);
> +	}

[Severity: High]
Can polling lrbp->utr_descriptor_ptr->header.ocs without synchronizing
with hwq->cq_lock race with ufshcd_compl_one_cqe()?

In MCQ mode, the UFS controller writes the CQE to hwq->cqe_base_addr
rather than updating lrbp->utr_descriptor_ptr->header.ocs in host memory.
The UTRD header.ocs field is updated in software by the CQ completion
handler under hwq->cq_lock:

drivers/ufs/core/ufshcd.c:ufshcd_compl_one_cqe() {
	...
	if (blk_mq_is_reserved_rq(scsi_cmd_to_rq(cmd))) {
		if (cqe) {
			ocs = cqe->overall_status & MASK_OCS;
			lrbp->utr_descriptor_ptr->header.ocs = ocs;
		} else {
			ocs = lrbp->utr_descriptor_ptr->header.ocs;
		}
		ufshcd_add_query_upiu_trace(
			hba,
			ocs == OCS_SUCCESS ? UFS_QUERY_COMP : UFS_QUERY_ERR,
			(struct utp_upiu_req *)lrbp->ucd_rsp_ptr);
		cmd->result = 0;
	}
	/* Do not touch lrbp after scsi_done() has been called. */
	scsi_done(cmd);
}

Because ufshcd_compl_one_cqe() writes lrbp->utr_descriptor_ptr->header.ocs
before tracing lrbp->ucd_rsp_ptr, setting cmd->result = 0, and calling
scsi_done(cmd), ufshcd_mcq_sq_cleanup() can observe the updated ocs and
return 0 to scmd_eh_abort_handler() while ufshcd_compl_one_cqe() is still
running on another CPU.

When scmd_eh_abort_handler() receives SUCCESS from the abort path:

drivers/scsi/scsi_error.c:scmd_eh_abort_handler() {
	...
	set_host_byte(scmd, DID_TIME_OUT);
	...
	scsi_finish_command(scmd);
}

Two races can then occur against the in-progress ufshcd_compl_one_cqe():

1. If ufshcd_compl_one_cqe() executes cmd->result = 0 after
   scmd_eh_abort_handler() calls set_host_byte(scmd, DID_TIME_OUT), the
   timeout status in cmd->result is overwritten with 0 before
   scsi_io_completion() inspects it, causing blk_execute_rq() in
   ufshcd_issue_dev_cmd() to see BLK_STS_OK instead of a timeout.

2. If scsi_finish_command() completes the request first,
   ufshcd_exec_dev_cmd() releases the reserved command via
   ufshcd_put_dev_mgmt_cmd() and a retry loop such as
   ufshcd_query_flag_retry() can immediately reallocate the single
   reserved request and submit the retry via blk_execute_rq(), which
   clears SCMD_STATE_COMPLETE in scsi_prepare_cmd(). When the delayed
   ufshcd_compl_one_cqe() from the first command then reaches
   scsi_done(cmd), it can prematurely complete the newly submitted retry
   command while it is still in flight in hardware.

Should ufshcd_mcq_sq_cleanup() synchronize with hwq->cq_lock (or poll CQEs
under hwq->cq_lock) so that ufshcd_compl_one_cqe() has completely finished
before cleanup returns?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104094914.180788-1-kwmad.kim@samsung.com?part=2

Re: [PATCH v5 2/2] ufs: mcq: poll OCS after sq cleanup for device management commands

[Bean Huo]

On Wed, 2026-11-04 at 18:49 +0900, Kiwoong Kim wrote:
> Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
> ---
>  drivers/ufs/core/ufs-mcq.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
> index 8106d55f4041..100295d02c62 100644
> --- a/drivers/ufs/core/ufs-mcq.c
> +++ b/drivers/ufs/core/ufs-mcq.c
> @@ -609,6 +609,24 @@ int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int
> task_tag)
>                          __func__, id,
>                          FIELD_GET(SQ_ICU_ERR_CODE_MASK, readl(reg)));
>  
> +       /*
> +        * For device management commands, there is no order guaranteed
> between
> +        * OCS update and CUS update, which is why we need to wait here. SCSI
> +        * commands are excluded because the OCS in their UTRD is not updated
> +        * from the CQE.
> +        */
> +       if (!err && blk_mq_is_reserved_rq(rq)) {
> +               enum utp_ocs ocs;
> +
> +               err = read_poll_timeout(READ_ONCE, ocs,

Bart, 

READ_ONCE() is enough to read OCS, but the lock also is to make sure   
ufshcd_compl_one_cqe() has finished?  the window is very small, and the lock
only costs something on this error handling path. 

tell me if I missed something.

Kind regards, 
Bean

Re: [PATCH v5 2/2] ufs: mcq: poll OCS after sq cleanup for device management commands

[Bart Van Assche]

On 11/4/26 12:17 PM, Bean Huo wrote:
> READ_ONCE() is enough to read OCS, but the lock also is to make sure
> ufshcd_compl_one_cqe() has finished?  the window is very small, and the lock
> only costs something on this error handling path.

I overlooked that lrbp->utr_descriptor_ptr->header.ocs is what is being
polled instead of the OCS field in the CQE. Kiwoong, can you please
restore the code that obtains hwq->cq_lock?

Thanks,

Bart.

RE: [PATCH v5 2/2] ufs: mcq: poll OCS after sq cleanup for device management commands

[Kiwoong Kim]

> > READ_ONCE() is enough to read OCS, but the lock also is to make sure
> > ufshcd_compl_one_cqe() has finished?  the window is very small, and
> > the lock only costs something on this error handling path.
> 
> I overlooked that lrbp->utr_descriptor_ptr->header.ocs is what is being
> polled instead of the OCS field in the CQE. Kiwoong, can you please
> restore the code that obtains hwq->cq_lock?
> 
> Thanks,
> 
> Bart.

Got it.

Thanks.

Re: [PATCH v5 2/2] ufs: mcq: poll OCS after sq cleanup for device management commands

[Bean Huo]

On Wed, 2026-11-04 at 18:49 +0900, Kiwoong Kim wrote:
> Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
> ---
>  drivers/ufs/core/ufs-mcq.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
> index 8106d55f4041..100295d02c62 100644
> --- a/drivers/ufs/core/ufs-mcq.c
> +++ b/drivers/ufs/core/ufs-mcq.c
> @@ -609,6 +609,24 @@ int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int
> task_tag)
>                          __func__, id,
>                          FIELD_GET(SQ_ICU_ERR_CODE_MASK, readl(reg)));
>  
> +       /*
> +        * For device management commands, there is no order guaranteed
> between
> +        * OCS update and CUS update, which is why we need to wait here. SCSI
> +        * commands are excluded because the OCS in their UTRD is not updated
> +        * from the CQE.
> +        */
> +       if (!err && blk_mq_is_reserved_rq(rq)) {
> +               enum utp_ocs ocs;
> +
> +               err = read_poll_timeout(READ_ONCE, ocs,

Bart, 

READ_ONCE() is enough to read OCS, but the lock also is to make sure   
ufshcd_compl_one_cqe() has finished?  the window is very small, and the lock
only costs something on this error handling path. 

tell me if I missed something.

Kind regards, 
Bean