Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
From:   "hoyoung seo" <hy50.seo@samsung.com>
To:     "'Bart Van Assche'" <bvanassche@acm.org>,
        <linux-scsi@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <alim.akhtar@samsung.com>, <avri.altman@wdc.com>,
        <jejb@linux.ibm.com>, <martin.petersen@oracle.com>,
        <beanhuo@micron.com>, <kwangwon.min@samsung.com>,
        <kwmad.kim@samsung.com>, <sh425.lee@samsung.com>,
        <sc.suh@samsung.com>, <quic_nguyenb@quicinc.com>,
        <cpgs@samsung.com>
In-Reply-To: <e1ed8776-d8ad-49f2-bf8f-2759256e33e9@acm.org>
Subject: RE: [PATCH v1] scsi: ufs: core: Process abort completed command in
 MCQ mode
Date:   Thu, 2 Nov 2023 13:07:31 +0900
Message-ID: <000001da0d42$1a1c8410$4e558c30$@samsung.com>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Thread-Index: AQI/NadRec19V/jm3RVDFrtg0RqXhwIoHnBGAuFKLQmvc6/hoA==
Content-Language: ko
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 102P
DLP-Filter: Pass
References: <CGME20231101084246epcas2p32ae15219878d1c31e7d8a14c22489519@epcas2p3.samsung.com>
        <20231101084504.79087-1-hy50.seo@samsung.com>
        <e1ed8776-d8ad-49f2-bf8f-2759256e33e9@acm.org>
Precedence: bulk

> -----Original Message-----
> From: Bart Van Assche <bvanassche=40acm.org>
> Sent: Thursday, November 2, 2023 1:39 AM
> To: SEO HOYOUNG <hy50.seo=40samsung.com>; linux-scsi=40vger.kernel.org; l=
inux-
> kernel=40vger.kernel.org; alim.akhtar=40samsung.com; avri.altman=40wdc.co=
m;
> jejb=40linux.ibm.com; martin.petersen=40oracle.com; beanhuo=40micron.com;
> kwangwon.min=40samsung.com; kwmad.kim=40samsung.com; sh425.lee=40samsung.=
com;
> sc.suh=40samsung.com; quic_nguyenb=40quicinc.com; cpgs=40samsung.com
> Subject: Re: =5BPATCH v1=5D scsi: ufs: core: Process abort completed comm=
and
> in MCQ mode
>=20
> On 11/1/23 01:45, SEO HOYOUNG wrote:
> > In MCQ mode, the case where OCS is updated to aborted is as follows
> >   1. when abort processing is completed
> >   2. When a duplicate command occurs
>=20
> What is a =22duplicate command=22? The UFSHCI driver guarantees that each=
 SCSI
> command has a unique tag.
>=20
> > In case of 1 situation, cmd should be re-request.
>=20
> It should be resubmitted by the SCSI error handler. The UFSHCI driver doe=
s
> not have to request this explicitly. See also the code at the end of
> scmd_eh_abort_handler().
>=20
> >   	case OCS_ABORTED:
> > -		result =7C=3D DID_ABORT << 16;
> > +		if (cqe)
> > +			eec =3D le32_to_cpu(cqe->status) & MASK_EEC;
> > +
> > +		if (is_mcq_enabled(hba) && =21eec)
> > +			result =7C=3D DID_REQUEUE << 16;
> > +		else
> > +			result =7C=3D DID_ABORT << 16;
> >   		break;
>=20
> I don't think this change is necessary. Additionally, introducing
> different behavior for MCQ compared to legacy mode in this code path is
> suspicious. Why should how commands are queued affect how aborts are
> processed?
>=20
> Thanks,
>=20
> Bart.

when the ufs host receives any error, the ufs driver executes the error-han=
der.
If the error-hendler attempts re-init, it must abort and organize unprocess=
ed
 requests.
The above operation is the same for both MCQ/legacy mode.
However, in the MCQ mode, if b or c is included in the following specs,=20
the OCS is updated to aborted, which is different from the legacy mode.

B. If the command is in the Submission Queue and not issued to the device y=
et,=20
the host controller will mark the command to be skipped in the Submission Q=
ueue.
The host controller will post to the Completion Queue to update the OCS fie=
ld=20
with =E2=80=98ABORTED=E2=80=99.=0D=0AC.=20If=20the=20command=20is=20issued=
=20to=20the=20device=20already=20but=20there=20is=20no=20response=20yet=20=
=0D=0Afrom=20the=20device,=20the=20host=20software=20driver=20issue=20the=
=20Abort=20task=20management=20function=0D=0A=20to=20the=20device=20for=20t=
hat=20command.=0D=0AThen=20the=20host=20driver=20set=20SQRTCy.ICU=20as=20=
=E2=80=981=E2=80=99=20to=20initiate=20the=20clean=20up=20the=20hardware=20=
=0D=0Aresources.=20The=20host=20controller=20will=20post=20to=20the=20Compl=
etion=20Queue=20to=20update=20the=20OCS=0D=0A=20field=20with=20=E2=80=98ABO=
RTED=E2=80=99.=0D=0A=0D=0AUnlike=20legacy=20mode,=20this=20phenomenon=20cau=
ses=20unintended=20behavior.=20(As=20shown=20in=20the=20log=20below)=0D=0A=
=0D=0A=5B1:=20=20kworker/u20:2:23157=5D=20ufshcd_try_to_abort_task:=20cmd=
=20pending=20in=20the=20device.=20tag=20=3D=209=0D=0A=5B3:=20=20kworker/u20=
:2:23157=5D=20Aborting=20tag=209=20/=20CDB=200x2a=20succeeded=0D=0A=5B4:=20=
=20=20=20=20=20swapper/4:=20=20=20=200=5D=20sd=200:0:0:0:=20=5Bsda=5D=20tag=
=239=20UNKNOWN(0x2003)=20Result:=20hostbyte=3D0x05=20driverbyte=3DDRIVER_OK=
=20cmd_age=3D0s=20//=20DID_ABORT=0D=0A=5B4:=20=20=20=20=20=20swapper/4:=20=
=20=20=200=5D=20sd=200:0:0:0:=20=5Bsda=5D=20tag=239=20CDB:=20opcode=3D0x2a=
=202a=2000=2000=20d3=2002=2000=2000=2001=2000=2000=0D=0A=5B4:=20=20=20=20=
=20=20swapper/4:=20=20=20=200=5D=20I/O=20error,=20dev=20sda,=20sector=20110=
628864=20op=200x1:(WRITE)=20flags=200x800=20phys_seg=20256=20prio=20class=
=202=0D=0A=0D=0A=0D=0AFor=20commands=20that=20have=20completed=20the=20abor=
t=20operation=20in=20MCQ=20mode,=0D=0Asince=20OCS=20has=20been=20updated=20=
to=20aborted,=20it=20seems=20that=20it=20will=20be=20retransmitted=20only=
=0D=0A=20when=20it=20is=20made=20to=20REQUEUE.=0D=0A=0D=0AThanks.=0D=0A=0D=
=0ASEO.=0D=0A=0D=0A