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

[PATCH v7 0/2] scsi: 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. ufshcd_mcq_abort() sends these commands to
the QUERY TASK / ABORT TASK path, which is meant for SCSI I/O commands.
With this change, the SDB path in ufshcd_abort() no longer needs
its own branch for reserved requests, so that branch is removed.

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 complete the
retry of that query too early, which leaves the host controller with two
query contexts and raises IS.UTPES. Each iteration of the wait reaps the
CQ directly, so the wait does not depend on interrupt delivery.

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 v6:
- Patch 1: Remove the reserved-request branch from ufshcd_abort(), since
  ufshcd_try_to_abort_task() now handles device management commands. (Bart)
- Patch 1: Update the kernel-doc of ufshcd_try_to_abort_task() and add
  a Closes: tag for the report. (Bart)
- Patch 2: Reap the CQ with ufshcd_mcq_poll_cqe_lock() in each iteration
  of the OCS poll, as ufshcd_clear_lu_cmds() does after
  ufshcd_clear_cmd(). This removes the dependency on interrupt delivery.
  The OCS is still read under hwq->cq_lock, because the interrupt handler
  can reap the CQE between the reap and the read. Rename
  ufshcd_mcq_read_ocs() to ufshcd_mcq_poll_ocs(). (Bart)
- Patch 2: Rework the commit message. Explain how the problem applies to
  the current code and how it affects the retry, quote UFSHCI on the CQE
  posted after a successful cleanup, and explain the CQ reaping.
- Add the "scsi:" prefix to the subjects.

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

Changes since v5:
- Read the OCS under hwq->cq_lock again, as in v4. Without the lock,
  ufshcd_mcq_sq_cleanup() can observe the OCS update and return while
  ufshcd_compl_one_cqe() has not yet called scsi_done(). A retry that
  reuses the reserved request can then be completed by that pending
  scsi_done() call (Sashiko).
- Explain the need for cq_lock in a comment and in the patch 2 commit
  message.

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

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):
  scsi: ufs: core: skip ABORT TASK for device management commands
  scsi: ufs: mcq: poll OCS after sq cleanup for device management
    commands

 drivers/ufs/core/ufs-mcq.c | 34 ++++++++++++++++++++++++++++++++++
 drivers/ufs/core/ufshcd.c  | 23 ++++++++++++++---------
 2 files changed, 48 insertions(+), 9 deletions(-)

-- 
2.43.0

[PATCH v7 1/2] scsi: 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().
For a reserved (device management) command, skip the TM path and call
ufshcd_clear_cmd() directly. ufshcd_abort() now calls
ufshcd_try_to_abort_task() for every request,
so its separate reserved-request branch is removed.

Fixes: 08b12cda6c44 ("scsi: ufs: core: Switch to scsi_get_internal_cmd()")
Reported-by: Bart Van Assche <bvanassche@acm.org>
Closes: https://lore.kernel.org/r/8446dc27-9518-4b0c-b9e9-116458210616@acm.org
Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
Reviewed-by: Bean Huo <beanhuo@micron.com>
---
 drivers/ufs/core/ufshcd.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index bcc2512bf34b..b9aa0b692fa5 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8025,11 +8025,15 @@ static void ufshcd_set_req_abort_skip(struct ufs_hba *hba, unsigned long bitmap)
  * @hba: Pointer to adapter instance
  * @tag: Tag of the task to be aborted
  *
- * Abort the pending command in device by sending UFS_ABORT_TASK task management
- * command, and in host controller by clearing the door-bell register. There can
- * be race between controller sending the command to the device while abort is
- * issued. To avoid that, first issue UFS_QUERY_TASK to check if the command is
- * really issued and then try to abort it.
+ * Abort the pending SCSI command in device by sending UFS_ABORT_TASK task
+ * management command, and in host controller by clearing the door-bell
+ * register. There can be race between controller sending the command to the
+ * device while abort is issued. To avoid that, first issue UFS_QUERY_TASK to
+ * check if the command is really issued and then try to abort it.
+ *
+ * Device management commands are not SCSI commands, so no task management
+ * function is issued for them; only the command is cleared in the host
+ * controller.
  *
  * Return: zero on success, non-zero on failure.
  */
@@ -8046,6 +8050,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 +8098,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",
@@ -8194,10 +8202,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 		goto release;
 	}
 
-	if (blk_mq_is_reserved_rq(rq))
-		err = ufshcd_clear_cmd(hba, tag);
-	else
-		err = ufshcd_try_to_abort_task(hba, tag);
+	err = ufshcd_try_to_abort_task(hba, tag);
 	if (err) {
 		dev_err(hba->dev, "%s: failed with err %d\n", __func__, err);
 		ufshcd_set_req_abort_skip(hba, hba->outstanding_reqs);
-- 
2.43.0

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

[Bart Van Assche]

On 11/5/26 8:12 PM, Kiwoong Kim wrote:
 > Reported-by: Bart Van Assche <bvanassche@acm.org>
 > Closes: 
https://lore.kernel.org/r/8446dc27-9518-4b0c-b9e9-116458210616@acm.org

That Closes: link points at my review of v6 instead of the original
report. Please fix this if you post a v8. Otherwise:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

[PATCH v7 2/2] scsi: 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 controller with two query contexts and raising IS.UTPES.

Here is the timeline of an actual case I found with kernel 6.18:

1. The driver sent a query but the device didn't respond to 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 timed out, the driver did SQ cleanup. Then
   the host controller updated the CQ tail and raised IS.CQES.
   Unfortunately, complete() was called by the driver's ISR afterward.

3. The driver started to retry 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 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 first query, but since the host controller was holding
   two query contexts and couldn't handle it, it raised IS.UTPES.

The same problem exists in the current code. If the query times out, the
SCSI error handler calls ufshcd_abort() -> ufshcd_mcq_abort() ->
ufshcd_mcq_sq_cleanup(), the request is finished, and the retry reuses the
same reserved tag, struct scsi_cmnd and devman UCD. A late CQE for the
first query then maps to the same tag and completes the retry by calling
scsi_done() too early. The retry then returns the OCS of the first query
and, if that OCS is OCS_SUCCESS, its response as well. Hence
ufshcd_mcq_sq_cleanup() must not return until the CQE of the cleaned-up
request has been processed.

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.
The UFSHCI specification says "The host controller shall free up
any resources associated to the nexus, and shall post
a Completion Queue entry with OCS = ABORTED if clean up successful."

Each poll iteration reaps the CQ with ufshcd_mcq_poll_cqe_lock() before
reading the OCS, as ufshcd_clear_lu_cmds() does after ufshcd_clear_cmd().
This removes the dependency on interrupt delivery. Otherwise the OCS is
only updated once the interrupt handler processes the CQE, and a delayed
or lost interrupt would make the poll time out and escalate to a host
reset although the request has already been cleaned up.

The OCS is still read under hwq->cq_lock because the interrupt handler may
reap the CQE between the reap and the read. ufshcd_compl_one_cqe() updates
the OCS under that lock before it sets cmd->result and calls scsi_done().
Without the lock, cleanup could observe the new OCS and return while the
completion is still running. The abort could then finish the command and
a retry could reuse the same reserved request before the pending
scsi_done() call, which would then complete the retry prematurely.

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 | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 8106d55f4041..889f0b78034c 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -550,6 +550,23 @@ static int ufshcd_mcq_sq_start(struct ufs_hba *hba, struct ufs_hw_queue *hwq)
 	return err;
 }
 
+/*
+ * Reap the CQ directly so that progress does not depend on interrupt
+ * delivery. ufshcd_compl_one_cqe() updates the OCS under cq_lock before
+ * calling scsi_done(). Holding cq_lock while reading the OCS makes sure that
+ * the completion has finished once the updated OCS is observed, even if the
+ * CQE was reaped by the interrupt handler.
+ */
+static enum utp_ocs ufshcd_mcq_poll_ocs(struct ufs_hba *hba,
+					struct ufs_hw_queue *hwq,
+					struct ufshcd_lrb *lrbp)
+{
+	ufshcd_mcq_poll_cqe_lock(hba, hwq);
+
+	guard(spinlock_irqsave)(&hwq->cq_lock);
+	return READ_ONCE(lrbp->utr_descriptor_ptr->header.ocs);
+}
+
 /**
  * ufshcd_mcq_sq_cleanup - Clean up submission queue resources
  * associated with the pending command.
@@ -609,6 +626,23 @@ 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(ufshcd_mcq_poll_ocs, ocs,
+					ocs != OCS_INVALID_COMMAND_STATUS, 20,
+					MCQ_POLL_US, false, hba, hwq, lrbp);
+		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 v7 2/2] scsi: ufs: mcq: poll OCS after sq cleanup for device management commands

[Bart Van Assche]

On 11/5/26 8:12 PM, Kiwoong Kim wrote:
 > +static enum utp_ocs ufshcd_mcq_poll_ocs(struct ufs_hba *hba,
 > +					struct ufs_hw_queue *hwq,
 > +					struct ufshcd_lrb *lrbp)
 > +{
 > +	ufshcd_mcq_poll_cqe_lock(hba, hwq);
 > +
 > +	guard(spinlock_irqsave)(&hwq->cq_lock);
 > +	return READ_ONCE(lrbp->utr_descriptor_ptr->header.ocs);
 > +}

As mentioned before, READ_ONCE() is not necessary since the OCS is read
with cq_lock held.

 > means the cleanup has completed.
 > The UFSHCI specification says "The host controller shall free up

Please insert a blank line before "The UFSHCI specification says" and
mention the version and section number of the UFSHCI specification.

With the above addressed, feel free to add:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

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

[Bean Huo]

On Fri, 2026-11-06 at 13:12 +0900, Kiwoong Kim wrote:
> Fixes: 8d7290348992 ("scsi: ufs: mcq: Add supporting functions for MCQ abort")
> Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>

Reviewed-by: Bean Huo <beanhuo@micron.com>