[PATCH v3] ufs: mcq: fix dev cmd abort path and add OCS poll after sq cleanup

[PATCH v3] ufs: mcq: fix dev cmd abort path and add OCS poll after sq cleanup

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

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.

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

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

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

Signed-off-by: Kiwoong Kim <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/sashiko-outbox-190791@kernel.org/T/#t

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/sashiko-outbox-190714@kernel.org/T/#t
---
 drivers/ufs/core/ufs-mcq.c | 21 +++++++++++++++++++++
 drivers/ufs/core/ufshcd.c  |  4 ++++
 2 files changed, 25 insertions(+)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 8106d55f4041..7c2aa0e9466a 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -550,6 +550,13 @@ static int ufshcd_mcq_sq_start(struct ufs_hba *hba, struct ufs_hw_queue *hwq)
 	return err;
 }
 
+static enum utp_ocs ufshcd_mcq_read_ocs(struct ufs_hw_queue *hwq,
+					struct ufshcd_lrb *lrbp)
+{
+	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 +616,20 @@ 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_read_ocs, ocs,
+					ocs != OCS_INVALID_COMMAND_STATUS, 20,
+					MCQ_POLL_US, false, hwq, lrbp);
+	}
+
 	if (ufshcd_mcq_sq_start(hba, hwq))
 		err = -ETIMEDOUT;
 
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

Re: [PATCH v3] ufs: mcq: fix dev cmd abort path and add OCS poll after sq cleanup

[Bean Huo]

Hi Kiwoong, 

I think this should be two patches, each needs different fix tag:
 
On Fri, 2026-10-30 at 20:10 +0900, Kiwoong Kim wrote:
> 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 true only since 08b12cda6c44 ("scsi: ufs: core: Switch to
scsi_get_internal_cmd()"), v6.19. Before that, a timed-out query went through
ufshcd_wait_for_dev_cmd() -> ufshcd_clear_cmd() and never reached
ufshcd_try_to_abort_task(). you need to add fix tag for it.

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


> Additionally, 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.
> 
> --

I didn't understand why here there is "--", this shoudl be "---"?

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


this is another patch, and OCS poll patch needs a fixes tag too, for the MCQ
abort support. 


> 
> [ 7384.156163] [4:  kworker/u40:5:10671] exynos-ufs 38300000.ufs:
> ufshcd_query_flag: Sending flag query for idn 14 failed, err = 15
> 
> 5. 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
> 
> 6. 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.
> 
> Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
> 
> 

Kind regards, 
Bean

RE: [PATCH v3] ufs: mcq: fix dev cmd abort path and add OCS poll after sq cleanup

[Kiwoong Kim]

> I think this should be two patches, each needs different fix tag:
Got it.

> > --
> 
> I didn't understand why here there is "--", this shoudl be "---"?
I just tried to separate two contents, but it seems like I made you confused. Let me remove this.


> this is another patch, and OCS poll patch needs a fixes tag too, for the
> MCQ abort support.
Got it.

> Kind regards,
> Bean

Thank you for your comments.


Kiwoong
BR.