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

["Wu, Hao A" <hao.a.wu@intel.com>]

> -----Original Message-----
> From: Ni, Ray
> Sent: Friday, November 15, 2019 11:28 AM
> To: Wu, Hao A; devel@edk2.groups.io
> Cc: Sean Brogan; Wang, Jian J
> Subject: RE: [PATCH v1] MdeModulePkg/NvmExpressDxe: Fix wrong queue size
> for async IO queues
> 
> Acked-by: Ray Ni <ray.ni@intel.com>


Hello all,

I plan to push this bugfix to catch the upcoming stable tag. If there is
concern on the patch, please help to raise. Thanks.

Best Regards,
Hao Wu


> 
> > -----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