MIME-Version: 1.0
References: <cover.1678247309.git.quic_nguyenb@quicinc.com> <c7fcbb70f0e74d225c1a09f107ba1058270739be.1678247309.git.quic_nguyenb@quicinc.com>
In-Reply-To: <c7fcbb70f0e74d225c1a09f107ba1058270739be.1678247309.git.quic_nguyenb@quicinc.com>
From:   Stanley Chu <chu.stanley@gmail.com>
Date:   Thu, 9 Mar 2023 11:10:15 +0800
Message-ID: <CAGaU9a_CkxHU-k519TL2hfLZSM6Hfoo6sAELmJ+6Vy2X2ZCOrw@mail.gmail.com>
Subject: Re: [RFC PATCH v1 4/4] ufs: mcq: Added ufshcd_mcq_abort()
To:     "Bao D. Nguyen" <quic_nguyenb@quicinc.com>
Cc:     quic_asutoshd@quicinc.com, quic_cang@quicinc.com,
        bvanassche@acm.org, mani@kernel.org, stanley.chu@mediatek.com,
        adrian.hunter@intel.com, beanhuo@micron.com, avri.altman@wdc.com,
        martin.petersen@oracle.com, linux-scsi@vger.kernel.org,
        Alim Akhtar <alim.akhtar@samsung.com>,
        "James E.J. Bottomley" <jejb@linux.ibm.com>,
        Arthur Simchaev <Arthur.Simchaev@wdc.com>,
        open list <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Bao,

On Wed, Mar 8, 2023 at 12:10=E2=80=AFPM 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 *hb=
a,
>         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 =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[tag];
> +       struct ufs_hw_queue *hwq;
> +       int err =3D 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 fail=
ure */
> +       if (lrbp->req_abort_skip) {
> +               dev_err(hba->dev, "%s: skip abort. tag %d failed earlier\=
n",
> +                       __func__, tag);
> +               goto out;
> +       }
> +
> +       hwq =3D 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=3D%d, tag=3D%=
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=3D%d, tag=3D%=
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 in=
terrupt 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 =3D 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 =3D true;
> +               goto out;
> +       }
> +
> +       err =3D 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-pri=
v.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 =3D lrbp->cmd;
> @@ -7340,7 +7339,7 @@ static void ufshcd_set_req_abort_skip(struct ufs_hb=
a *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 =3D &hba->lrb[tag];
>         int err =3D 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 =3D ufshcd_mcq_abort(cmd);
> +               goto release;
> +       }
> +
>         /* Skip task abort in case previous aborts failed and report fail=
ure */
>         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.

Thanks,
Stanley