From:   "hoyoung seo" <hy50.seo@samsung.com>
To:     <quic_nguyenb@quicinc.com>
Cc:     <Arthur.Simchaev@wdc.com>, <adrian.hunter@intel.com>,
        <alim.akhtar@samsung.com>, <avri.altman@wdc.com>,
        <beanhuo@micron.com>, <bvanassche@acm.org>, <jejb@linux.ibm.com>,
        <linux-kernel@vger.kernel.org>, <linux-scsi@vger.kernel.org>,
        <mani@kernel.org>, <martin.petersen@oracle.com>,
        <quic_asutoshd@quicinc.com>, <quic_cang@quicinc.com>
Subject: Re: [RFC PATCH v2 3/3] ufs: mcq: Added ufshcd_mcq_abort()
Date:   Wed, 15 Mar 2023 10:45:41 +0900
Message-ID: <036101d956df$d9afe7a0$8d0fb6e0$@samsung.com>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Thread-Index: AdlW37LX9J9N3Z9WQoWloGX4SC/Ojw==
Content-Language: ko
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 102P
DLP-Filter: Pass
References: <CGME20230315014541epcas2p3fdbf4ea817fbc87f9ba230e389914286@epcas2p3.samsung.com>
Precedence: bulk

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c index =
4c33c1a..8d0019e 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
=40=40 -610,3 +610,65 =40=40 static bool ufshcd_mcq_sqe_search(struct ufs_h=
ba *hba,
 	spin_unlock(&hwq->sq_lock);
 	return ret;
 =7D
+
+/**
+ * ufshcd_mcq_abort - Abort the command in MCQ.
+ * =40cmd - The command to be aborted.
+ *
+ * Returns SUCCESS or FAILED error codes  */ int=20
+ufshcd_mcq_abort(struct scsi_cmnd *cmd) =7B
+	struct Scsi_Host *host =3D cmd->device->host;
+	struct ufs_hba *hba =3D shost_priv(host);
+	int tag =3D scsi_cmd_to_rq(cmd)->tag;
+	struct ufshcd_lrb *lrbp =3D &hba->lrb=5Btag=5D;
+	struct ufs_hw_queue *hwq;
+	int err =3D FAILED;
+
+	if (=21lrbp->cmd) =7B
+		dev_err(hba->dev,
+			=22%s: skip abort. cmd at tag %d already completed.=5Cn=22,
+			__func__, tag);
+		goto out;
+	=7D
+
+	/* Skip task abort in case previous aborts failed and report failure */
+	if (lrbp->req_abort_skip) =7B
+		dev_err(hba->dev, =22%s: skip abort. tag %d failed earlier=5Cn=22,
+			__func__, tag);
+		goto out;
+	=7D
+
+	hwq =3D ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(cmd));
+
+	if (ufshcd_mcq_sqe_search(hba, hwq, tag)) =7B
+		/*
+		 * Failure. The command should not be =22stuck=22 in SQ for
+		 * a long time which resulted in command being aborted.
+		 */
+		dev_err(hba->dev, =22%s: cmd found in sq. hwq=3D%d, tag=3D%d=5Cn=22,
+				__func__, hwq->id, tag);
+		/* Set the Command Type to 0xF per the spec */
+		ufshcd_mcq_nullify_cmd(hba, hwq);
+		goto out;
+	=7D
+
+	/*
+	 * The command is not in the Submission Queue, and it is not
+	 * in the Completion Queue either. Query the device to see if
+	 * the command is being processed in the device.
+	 */
+	if (ufshcd_try_to_abort_task(hba, tag)) =7B
+		dev_err(hba->dev, =22%s: device abort failed %d=5Cn=22, __func__, err);
+		lrbp->req_abort_skip =3D true;
+		goto out;
+	=7D
+
+	err =3D SUCCESS;
+	if (lrbp->cmd)
+		ufshcd_release_scsi_cmd(hba, lrbp);
+
+out:
+	return err;
+=7D

Hi, Bao.

Looking at the 4.0spec, it stops SQ before abort processing and checks whet=
her SQRTSy.STS is 1. Shouldn't this be added?
And need to check SQRTSy.STS bit as '0' after re-start SQ

Below is the original text of the UFS 4.0 spec

1. The device driver set SQRTCy.STOP as =E2=80=981=E2=80=99.=20The=20host=
=20controller=20will=20stop=20the=20fetching=20command=20from=20the=20Submi=
ssion=20Queue,=20and=20set=20the=20SQRTS.STS=20bit=20as=20=E2=80=981=E2=80=
=99=20to=20indicate=20that=20SQ=20is=20stopped=0D=0A=0D=0A2.=20Depending=20=
on=20the=20status=20of=20command=20which=20is=20requested=20to=20be=20abort=
ed,=20A.=20If=20the=20command=20is=20already=20completed,=20then=20go=20to=
=20step=203.=0D=0AB.=20If=20the=20command=20is=20in=20the=20Submission=20Qu=
eue=20and=20not=20issued=20to=20the=20device=20yet,=20the=20host=20controll=
er=20will=20mark=20the=20command=20to=20be=20skipped=20in=20the=20Submissio=
n=20Queue.=20The=20host=20controller=20will=20post=20to=20the=20Completion=
=20Queue=20to=20update=20the=20OCS=20field=20with=20=E2=80=98ABORTED=E2=80=
=99.=0D=0AC.=20If=20the=20command=20is=20issued=20to=20the=20device=20alrea=
dy=20but=20there=20is=20no=20response=20yet=20from=20the=20device,=20the=20=
host=20software=20driver=20issue=20the=20Abort=20task=20management=20functi=
on=20to=20the=20device=20for=20that=20command.=20Then=20the=20host=20driver=
=20set=20SQRTCy.ICU=20as=20=E2=80=981=E2=80=99=20to=20initiate=20the=20clea=
n=20up=20the=20hardware=20resources.=20The=20host=20controller=20will=20pos=
t=20to=20the=20Completion=20Queue=20to=20update=20the=20OCS=20field=20with=
=20=E2=80=98ABORTED=E2=80=99.=20If=20host=20software=20driver=20receive=20t=
he=20=E2=80=98task=20not=20found=E2=80=99=20as=20the=20response=20of=20the=
=20associated=20task=20management=20function,=20then=20go=20to=203.=0D=0A=
=0D=0A3.=20SW=20set=20SQRTCy.=20STOP=20as=20=E2=80=980=E2=80=99,=20host=20c=
ontroller=20set=20the=20SQRTSy.STS=20bit=20as=20=E2=80=980=E2=80=99,=20then=
=20resume=20fetching=20the=20command=20from=20Submission=20Queue.=0D=0A=0D=
=0AThanks.=0D=0ABRS=20SEO.=0D=0A=0D=0A