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] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGaU9a9QkCnN-1CvHGB78vXaARB0uWMZCeAYr7qRcUE9effGYQ@mail.gmail.com>
Date:   Thu, 9 Mar 2023 14:21:13 +0800
From:   Stanley Chu <chu.stanley@...il.com>
To:     "Bao D. Nguyen" <quic_nguyenb@...cinc.com>
Cc:     quic_asutoshd@...cinc.com, quic_cang@...cinc.com,
        bvanassche@....org, mani@...nel.org, stanley.chu@...iatek.com,
        adrian.hunter@...el.com, beanhuo@...ron.com, avri.altman@....com,
        martin.petersen@...cle.com, linux-scsi@...r.kernel.org,
        Alim Akhtar <alim.akhtar@...sung.com>,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        Arthur Simchaev <Arthur.Simchaev@....com>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH v1 4/4] ufs: mcq: Added ufshcd_mcq_abort()

Hi Bao,

On Thu, Mar 9, 2023 at 1:31 PM Bao D. Nguyen <quic_nguyenb@...cinc.com> wrote:
>
> 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@...cinc.com> wrote:
> >> Add ufshcd_mcq_abort() to support ufs abort in mcq mode.
> >>
> >> Signed-off-by: Bao D. Nguyen <quic_nguyenb@...cinc.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.

The function ufshcd_abort_all() was introduced in the following link:
https://android.googlesource.com/kernel/common/+/b817e6ffbad7a1a0a5ca5bb7d4020823c3f4d9d0

Can you confirm if these patches are based on the latest kernel
6.3-rc1 or the latest linux-next tree?

Thanks,
Stanley

>
> >
> > Thanks,
> > Stanley
>
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ