linux-kernel - Re: [RFC PATCH v2 3/3] ufs: mcq: Added ufshcd_mcq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <08a21d08-9067-caa5-94f9-3264efabea37@quicinc.com>
Date:   Wed, 15 Mar 2023 00:46:08 -0700
From:   "Bao D. Nguyen" <quic_nguyenb@...cinc.com>
To:     hoyoung seo <hy50.seo@...sung.com>
CC:     <Arthur.Simchaev@....com>, <adrian.hunter@...el.com>,
        <alim.akhtar@...sung.com>, <avri.altman@....com>,
        <beanhuo@...ron.com>, <bvanassche@....org>, <jejb@...ux.ibm.com>,
        <linux-kernel@...r.kernel.org>, <linux-scsi@...r.kernel.org>,
        <mani@...nel.org>, <martin.petersen@...cle.com>,
        <quic_asutoshd@...cinc.com>, <quic_cang@...cinc.com>
Subject: Re: [RFC PATCH v2 3/3] ufs: mcq: Added ufshcd_mcq_abort()

On 3/14/2023 6:45 PM, hoyoung seo wrote:
> 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
> @@ -610,3 +610,65 @@ static bool ufshcd_mcq_sqe_search(struct ufs_hba *hba,
>   	spin_unlock(&hwq->sq_lock);
>   	return ret;
>   }
> +
> +/**
> + * ufshcd_mcq_abort - Abort the command in MCQ.
> + * @cmd - The command to be aborted.
> + *
> + * Returns SUCCESS or FAILED error codes  */ int
> +ufshcd_mcq_abort(struct scsi_cmnd *cmd) {
> +	struct Scsi_Host *host = cmd->device->host;
> +	struct ufs_hba *hba = shost_priv(host);
> +	int tag = scsi_cmd_to_rq(cmd)->tag;
> +	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
> +	struct ufs_hw_queue *hwq;
> +	int err = FAILED;
> +
> +	if (!lrbp->cmd) {
> +		dev_err(hba->dev,
> +			"%s: skip abort. cmd at tag %d already completed.\n",
> +			__func__, tag);
> +		goto out;
> +	}
> +
> +	/* Skip task abort in case previous aborts failed and report failure */
> +	if (lrbp->req_abort_skip) {
> +		dev_err(hba->dev, "%s: skip abort. tag %d failed earlier\n",
> +			__func__, tag);
> +		goto out;
> +	}
> +
> +	hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(cmd));
> +
> +	if (ufshcd_mcq_sqe_search(hba, hwq, tag)) {
> +		/*
> +		 * Failure. The command should not be "stuck" in SQ for
> +		 * a long time which resulted in command being aborted.
> +		 */
> +		dev_err(hba->dev, "%s: cmd found in sq. hwq=%d, tag=%d\n",
> +				__func__, hwq->id, tag);
> +		/* Set the Command Type to 0xF per the spec */
> +		ufshcd_mcq_nullify_cmd(hba, hwq);
> +		goto out;
> +	}
> +
> +	/*
> +	 * 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)) {
> +		dev_err(hba->dev, "%s: device abort failed %d\n", __func__, err);
> +		lrbp->req_abort_skip = true;
> +		goto out;
> +	}
> +
> +	err = SUCCESS;
> +	if (lrbp->cmd)
> +		ufshcd_release_scsi_cmd(hba, lrbp);
> +
> +out:
> +	return err;
> +}
>
> Hi, Bao.
>
> Looking at the 4.0spec, it stops SQ before abort processing and checks whether SQRTSy.STS is 1. Shouldn't this be added?
> And need to check SQRTSy.STS bit as '0' after re-start SQ
Hi Seo, we do ufshcd_mcq_sq_stop() to stop the SQ before the clean up 
operation and restart the SQ with ufshcd_mcq_sq_start() after the clean 
up is done.

We should also stop the SQ before the ufshcd_mcq_sqe_search() and 
restart the SQ after the search is done. I will add that logic.

Thanks!

>
> Below is the original text of the UFS 4.0 spec
>
> 1. The device driver set SQRTCy.STOP as ‘1’. The host controller will stop the fetching command from the Submission Queue, and set the SQRTS.STS bit as ‘1’ to indicate that SQ is stopped
>
> 2. Depending on the status of command which is requested to be aborted, A. If the command is already completed, then go to step 3.
> B. If the command is in the Submission Queue and not issued to the device yet, the host controller will mark the command to be skipped in the Submission Queue. The host controller will post to the Completion Queue to update the OCS field with ‘ABORTED’.
> C. If the command is issued to the device already but there is no response yet from the device, the host software driver issue the Abort task management function to the device for that command. Then the host driver set SQRTCy.ICU as ‘1’ to initiate the clean up the hardware resources. The host controller will post to the Completion Queue to update the OCS field with ‘ABORTED’. If host software driver receive the ‘task not found’ as the response of the associated task management function, then go to 3.
>
> 3. SW set SQRTCy. STOP as ‘0’, host controller set the SQRTSy.STS bit as ‘0’, then resume fetching the command from Submission Queue.
>
> Thanks.
> BRS SEO.
>
>