Re: [PATCH v1] MdeModulePkg/NvmExpressDxe: Fix wrong queue size for async IO queues

["Ni, Ray" <ray.ni@intel.com>]

Acked-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Wu, Hao A <hao.a.wu@intel.com>
> Sent: Thursday, November 14, 2019 1:02 PM
> To: devel@edk2.groups.io
> Cc: Sean Brogan <sean.brogan@microsoft.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Ni, Ray <ray.ni@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>
> 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