[PATCH v6 2/2] ufs: core: requeue aborted request

[Bart Van Assche]

On 9/11/24 2:56 AM, peter.wang at mediatek.com wrote:
> ufshcd_abort_all forcibly aborts all on-going commands and the host
> controller will automatically fill in the OCS field of the corresponding
> response with OCS_ABORTED based on different working modes.
> 
> MCQ mode: aborts a command using SQ cleanup, The host controller
> will post a Completion Queue entry with OCS = ABORTED.
> 
> SDB mode: aborts a command using UTRLCLR. Task Management response
> which means a Transfer Request was aborted.

I think this is incorrect. The UFSHCI specification does not require a
host controller to set the OCS field if a SCSI command is aborted by the
ABORT TMF nor if its resources are freed by writing into the UTRLCLR
register.

> @@ -5404,7 +5405,10 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
>   		}
>   		break;
>   	case OCS_ABORTED:
> -		result |= DID_ABORT << 16;
> +		if (lrbp->abort_initiated_by_eh)
> +			result |= DID_REQUEUE << 16;
> +		else
> +			result |= DID_ABORT << 16;
>   		break;

Is the above change necessary? ufshcd_abort_one() aborts commands by
submitting an ABORT TMF. Hence, ufshcd_transfer_rsp_status() won't be
called if aborting the command succeeds.

> @@ -7561,6 +7551,21 @@ int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
>   		goto out;
>   	}
>   
> +	/*
> +	 * When the host software receives a "FUNCTION COMPLETE", set flag
> +	 * to requeue command after receive response with OCS_ABORTED
> +	 * SDB mode: UTRLCLR Task Management response which means a Transfer
> +	 *           Request was aborted.
> +	 * MCQ mode: Host will post to CQ with OCS_ABORTED after SQ cleanup
> +	 *
> +	 * This flag is set because error handler ufshcd_abort_all forcibly
> +	 * aborts all commands, and the host controller will automatically
> +	 * fill in the OCS field of the corresponding response with OCS_ABORTED.
> +	 * Therefore, upon receiving this response, it needs to be requeued.
> +	 */
> +	if (!err && ufshcd_eh_in_progress(hba))
> +		lrbp->abort_initiated_by_eh = true;
> +
>   	err = ufshcd_clear_cmd(hba, tag);
>   	if (err)
>   		dev_err(hba->dev, "%s: Failed clearing cmd at tag %d, err %d\n",

The above change will cause lrbp->abort_initiated_by_eh to be set not
only if the UFS error handler decides to abort a command but also if the
SCSI core decides to abort a command. I think this is wrong.

Is this patch 2/2 necessary or is patch 1/2 perhaps sufficient?

Thanks,

Bart.