Re: [edk2] [PATCH] MdeModulePkg/NvmExpressDxe: Handling return of write to sq and cq db

[Wu, Hao A]

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Wu,
> Hao A
> Sent: Friday, April 21, 2017 9:39 AM
> To: Suman Prakash; edk2-devel@lists.01.org
> Cc: Tian, Feng
> Subject: Re: [edk2] [PATCH] MdeModulePkg/NvmExpressDxe: Handling return
> of write to sq and cq db
> 
> Thanks for the catch. The patch is good to me.
> Reviewed-by: Hao Wu 
> 

Pushed at f6b139bde7e0a39f83ffad30af58136d5b0738a7.

> 
> Best Regards,
> Hao Wu
> 
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Suman Prakash
> > Sent: Thursday, April 20, 2017 6:02 PM
> > To: edk2-devel@lists.01.org
> > Cc: Tian, Feng
> > Subject: [edk2] [PATCH] MdeModulePkg/NvmExpressDxe: Handling return of
> > write to sq and cq db
> >
> > In case of an async command if updating the submission queue tail
> > doorbell fails then the command will not be picked up by device and
> > no completion response will be created. This scenario has to be handled.
> > Also if we create an AsyncRequest element and insert in the async queue,
> > it will never receive a completion so in the timer routine this element
> > won't be freed, resulting in memory leak. Also in case of blocking calls
> > we should capture the status of updating completion queue head doorbell
> > register and return it to caller of PassThru.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Suman Prakash 
> > ---
> >  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 8 ++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> > b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> > index ef3d772..fb80f39 100644
> > --- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> > +++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> > @@ -603,7 +603,7 @@ NvmExpressPassThru (
> >  Private->SqTdbl[QueueId].Sqt ^= 1;
> >}
> >Data = ReadUnaligned32 ((UINT32*)>SqTdbl[QueueId]);
> > -  PciIo->Mem.Write (
> > +  Status = PciIo->Mem.Write (
> > PciIo,
> > EfiPciIoWidthUint32,
> > NVME_BAR,
> > @@ -612,6 +612,10 @@ NvmExpressPassThru (
> > 
> > );
> >
> > +  if (EFI_ERROR (Status)) {
> > +goto EXIT;
> > +  }
> > +
> >//
> >// For non-blocking requests, return directly if the command is placed
> >// in the submission queue.
> > @@ -695,7 +699,7 @@ NvmExpressPassThru (
> >}
> >
> >Data = ReadUnaligned32 ((UINT32*)>CqHdbl[QueueId]);
> > -  PciIo->Mem.Write (
> > +  Status = PciIo->Mem.Write (
> > PciIo,
> > EfiPciIoWidthUint32,
> > NVME_BAR,
> > --
> > 1.9.1
> >
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg/NvmExpressDxe: Handling return of write to sq and cq db

[Wu, Hao A]

Thanks for the catch. The patch is good to me.
Reviewed-by: Hao Wu 


Best Regards,
Hao Wu


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Suman Prakash
> Sent: Thursday, April 20, 2017 6:02 PM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng
> Subject: [edk2] [PATCH] MdeModulePkg/NvmExpressDxe: Handling return of
> write to sq and cq db
> 
> In case of an async command if updating the submission queue tail
> doorbell fails then the command will not be picked up by device and
> no completion response will be created. This scenario has to be handled.
> Also if we create an AsyncRequest element and insert in the async queue,
> it will never receive a completion so in the timer routine this element
> won't be freed, resulting in memory leak. Also in case of blocking calls
> we should capture the status of updating completion queue head doorbell
> register and return it to caller of PassThru.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Suman Prakash 
> ---
>  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> index ef3d772..fb80f39 100644
> --- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> +++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> @@ -603,7 +603,7 @@ NvmExpressPassThru (
>  Private->SqTdbl[QueueId].Sqt ^= 1;
>}
>Data = ReadUnaligned32 ((UINT32*)>SqTdbl[QueueId]);
> -  PciIo->Mem.Write (
> +  Status = PciIo->Mem.Write (
> PciIo,
> EfiPciIoWidthUint32,
> NVME_BAR,
> @@ -612,6 +612,10 @@ NvmExpressPassThru (
> 
> );
> 
> +  if (EFI_ERROR (Status)) {
> +goto EXIT;
> +  }
> +
>//
>// For non-blocking requests, return directly if the command is placed
>// in the submission queue.
> @@ -695,7 +699,7 @@ NvmExpressPassThru (
>}
> 
>Data = ReadUnaligned32 ((UINT32*)>CqHdbl[QueueId]);
> -  PciIo->Mem.Write (
> +  Status = PciIo->Mem.Write (
> PciIo,
> EfiPciIoWidthUint32,
> NVME_BAR,
> --
> 1.9.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel