public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Wu, Hao A" <hao.a.wu@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"Wang, Jian J" <jian.j.wang@intel.com>,
	"Ni, Ray" <ray.ni@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Subject: Re: [PATCH v1] MdeModulePkg/NvmExpressDxe: Fix wrong queue size for async IO queues
Date: Fri, 15 Nov 2019 02:51:38 +0000	[thread overview]
Message-ID: <B80AF82E9BFB8E4FBD8C89DA810C6A093C95BFC2@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <20191114050141.18352-1-hao.a.wu@intel.com>

Hello Ray and Jian,

I plan to push this bug fix before the upcoming stable tag, could you help to
review this patch? Thanks in advance.

Best Regards,
Hao Wu


> -----Original Message-----
> From: Wu, Hao A
> Sent: Thursday, November 14, 2019 1:02 PM
> To: devel@edk2.groups.io
> Cc: Sean Brogan; Wang, Jian J; Ni, Ray; Wu, Hao A
> Subject: [PATCH v1] MdeModulePkg/NvmExpressDxe: Fix wrong queue size for
> async IO queues
> 
> From: Sean Brogan <sean.brogan@microsoft.com>
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2118
> 
> When a packet is queued/completed for the asynchronous IO queue, the logic
> to roll over to the front of the queue doesn't account for actual size of
> the IO Submission/Completion queue.
> 
> This causes a device to hang due to doorbell being outside of visible
> queue. An example would be if an NVMe drive only supported a queue size of
> 128 while the driver supports 256.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Sean Brogan <sean.brogan@microsoft.com>
> Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
> ---
>  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c         | 2 +-
>  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 6 ++++--
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
> b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
> index 3bde96bc95..62886d5c91 100644
> --- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
> +++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
> @@ -672,7 +672,7 @@ ProcessAsyncTaskList (
>      }
> 
>      Private->CqHdbl[QueueId].Cqh++;
> -    if (Private->CqHdbl[QueueId].Cqh > NVME_ASYNC_CCQ_SIZE) {
> +    if (Private->CqHdbl[QueueId].Cqh > MIN (NVME_ASYNC_CCQ_SIZE, Private-
> >Cap.Mqes)) {
>        Private->CqHdbl[QueueId].Cqh = 0;
>        Private->Pt[QueueId] ^= 1;
>      }
> diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> index 8e72137946..e9357b1239 100644
> --- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> +++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> @@ -452,6 +452,7 @@ NvmExpressPassThru (
>    NVME_SQ                        *Sq;
>    NVME_CQ                        *Cq;
>    UINT16                         QueueId;
> +  UINT16                         QueueSize;
>    UINT32                         Bytes;
>    UINT16                         Offset;
>    EFI_EVENT                      TimerEvent;
> @@ -540,6 +541,7 @@ NvmExpressPassThru (
>    Prp         = NULL;
>    TimerEvent  = NULL;
>    Status      = EFI_SUCCESS;
> +  QueueSize   = MIN (NVME_ASYNC_CSQ_SIZE, Private->Cap.Mqes) + 1;
> 
>    if (Packet->QueueType == NVME_ADMIN_QUEUE) {
>      QueueId = 0;
> @@ -552,7 +554,7 @@ NvmExpressPassThru (
>        //
>        // Submission queue full check.
>        //
> -      if ((Private->SqTdbl[QueueId].Sqt + 1) % (NVME_ASYNC_CSQ_SIZE + 1) ==
> +      if ((Private->SqTdbl[QueueId].Sqt + 1) % QueueSize ==
>            Private->AsyncSqHead) {
>          return EFI_NOT_READY;
>        }
> @@ -701,7 +703,7 @@ NvmExpressPassThru (
>    //
>    if ((Event != NULL) && (QueueId != 0)) {
>      Private->SqTdbl[QueueId].Sqt =
> -      (Private->SqTdbl[QueueId].Sqt + 1) % (NVME_ASYNC_CSQ_SIZE + 1);
> +      (Private->SqTdbl[QueueId].Sqt + 1) % QueueSize;
>    } else {
>      Private->SqTdbl[QueueId].Sqt ^= 1;
>    }
> --
> 2.12.0.windows.1


  reply	other threads:[~2019-11-15  2:51 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-14  5:01 [PATCH v1] MdeModulePkg/NvmExpressDxe: Fix wrong queue size for async IO queues Wu, Hao A
2019-11-15  2:51 ` Wu, Hao A [this message]
2019-11-15  3:28 ` Ni, Ray
2019-11-18  1:05   ` Wu, Hao A

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=B80AF82E9BFB8E4FBD8C89DA810C6A093C95BFC2@SHSMSX104.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox