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

Kind regards, 
Bean