Re: [RFC PATCH v1 4/4] ufs: mcq: Added ufshcd_mcq_abort()

Bao D. Nguyen
2023-03-09T05:34:00Z

On 3/8/2023 7:10 PM, Stanley Chu wrote:
> Hi Bao,
>
> On Wed, Mar 8, 2023 at 12:10 PM Bao D. Nguyen <quic_nguyenb@quicinc.com> wrote:
>> Add ufshcd_mcq_abort() to support ufs abort in mcq mode.
>>
>> Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
>> ---
>> drivers/ufs/core/ufs-mcq.c | 76 ++++++++++++++++++++++++++++++++++++++++++
>> drivers/ufs/core/ufshcd-priv.h | 5 ++-
>> drivers/ufs/core/ufshcd.c | 11 ++++--
>> 3 files changed, 88 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
>> index e27d8eb..6c65cd5 100644
>> --- a/drivers/ufs/core/ufs-mcq.c
>> +++ b/drivers/ufs/core/ufs-mcq.c
>> @@ -646,3 +646,79 @@ static bool ufshcd_mcq_cqe_search(struct ufs_hba *hba,
>> spin_unlock(&hwq->cq_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;
>> + }
>> +
>> + if (ufshcd_mcq_cqe_search(hba, hwq, tag)) {
>> + dev_err(hba->dev, "%s: cmd found in cq. hwq=%d, tag=%d\n",
>> + __func__, hwq->id, tag);
>> + /*
>> + * The command should not be 'stuck' in the CQ for such a long time.
>> + * Is interrupt missing? Process the CQEs here. If the interrupt is
>> + * invoked at a later time, the CQ will be empty because the CQEs
>> + * are already processed here.
>> + */
>> + ufshcd_mcq_poll_cqe_lock(hba, hwq);
>> + err = SUCCESS;
>> + 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;
>> +}
>> diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
>> index 0527926..d883c03 100644
>> --- a/drivers/ufs/core/ufshcd-priv.h
>> +++ b/drivers/ufs/core/ufshcd-priv.h
>> @@ -77,7 +77,10 @@ unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba,
>> struct ufs_hw_queue *hwq);
>>
>> int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag, int *result);
>> -
>> +int ufshcd_mcq_abort(struct scsi_cmnd *cmd);
>> +int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag);
>> +void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
>> + struct ufshcd_lrb *lrbp);
>> #define UFSHCD_MCQ_IO_QUEUE_OFFSET 1
>> #define SD_ASCII_STD true
>> #define SD_RAW false
>> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
>> index 408c16c..e06399e 100644
>> --- a/drivers/ufs/core/ufshcd.c
>> +++ b/drivers/ufs/core/ufshcd.c
>> @@ -302,7 +302,6 @@ static int ufshcd_setup_hba_vreg(struct ufs_hba *hba, bool on);
>> static int ufshcd_setup_vreg(struct ufs_hba *hba, bool on);
>> static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
>> struct ufs_vreg *vreg);
>> -static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag);
>> static void ufshcd_wb_toggle_buf_flush_during_h8(struct ufs_hba *hba,
>> bool enable);
>> static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba);
>> @@ -5414,7 +5413,7 @@ static irqreturn_t ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
>> }
>>
>> /* Release the resources allocated for processing a SCSI command. */
>> -static void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
>> +void ufshcd_release_scsi_cmd(struct ufs_hba *hba,
>> struct ufshcd_lrb *lrbp)
>> {
>> struct scsi_cmnd *cmd = lrbp->cmd;
>> @@ -7340,7 +7339,7 @@ static void ufshcd_set_req_abort_skip(struct ufs_hba *hba, unsigned long bitmap)
>> *
>> * Returns zero on success, non-zero on failure
>> */
>> -static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
>> +int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
>> {
>> struct ufshcd_lrb *lrbp = &hba->lrb[tag];
>> int err = 0;
>> @@ -7500,6 +7499,12 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
>> goto release;
>> }
>>
>> + if (is_mcq_enabled(hba)) {
>> + /* MCQ mode. Branch off to handle abort for mcq mode */
>> + err = ufshcd_mcq_abort(cmd);
>> + goto release;
>> + }
>> +
>> /* Skip task abort in case previous aborts failed and report failure */
>> if (lrbp->req_abort_skip) {
>> dev_err(hba->dev, "%s: skipping abort\n", __func__);
>> --
>> 2.7.4
>>
> It seems that ufshcd_abort_all() also needs an error handling flow for MCQ.

Hi Stanley, thank you for the reviews.

I am not able to find the function ufshcd_abort_all() in the drivers/ufs
directory. Can you please clarify where this function is located? Thanks.

>
> Thanks,
> Stanley