public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] MdeModulePkg/NvmExpressDxe: Handling return of write to sq and cq db
       [not found] <CGME20170420100209epcas5p1839e6395a1399a22ab8f705c52e65eeb@epcas5p1.samsung.com>
@ 2017-04-20 10:01 ` Suman Prakash
  2017-04-21  1:38   ` Wu, Hao A
  0 siblings, 1 reply; 3+ messages in thread
From: Suman Prakash @ 2017-04-20 10:01 UTC (permalink / raw)
  To: edk2-devel; +Cc: feng.tian, Suman Prakash

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 <suman.p@samsung.com>
---
 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*)&Private->SqTdbl[QueueId]);
-  PciIo->Mem.Write (
+  Status = PciIo->Mem.Write (
                PciIo,
                EfiPciIoWidthUint32,
                NVME_BAR,
@@ -612,6 +612,10 @@ NvmExpressPassThru (
                &Data
                );
 
+  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*)&Private->CqHdbl[QueueId]);
-  PciIo->Mem.Write (
+  Status = PciIo->Mem.Write (
                PciIo,
                EfiPciIoWidthUint32,
                NVME_BAR,
-- 
1.9.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] MdeModulePkg/NvmExpressDxe: Handling return of write to sq and cq db
  2017-04-20 10:01 ` [PATCH] MdeModulePkg/NvmExpressDxe: Handling return of write to sq and cq db Suman Prakash
@ 2017-04-21  1:38   ` Wu, Hao A
  2017-04-24  1:07     ` Wu, Hao A
  0 siblings, 1 reply; 3+ messages in thread
From: Wu, Hao A @ 2017-04-21  1:38 UTC (permalink / raw)
  To: Suman Prakash, edk2-devel@lists.01.org; +Cc: Tian, Feng

Thanks for the catch. The patch is good to me.
Reviewed-by: Hao Wu <hao.a.wu@intel.com>


Best Regards,
Hao Wu


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@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 <suman.p@samsung.com>
> ---
>  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*)&Private->SqTdbl[QueueId]);
> -  PciIo->Mem.Write (
> +  Status = PciIo->Mem.Write (
>                 PciIo,
>                 EfiPciIoWidthUint32,
>                 NVME_BAR,
> @@ -612,6 +612,10 @@ NvmExpressPassThru (
>                 &Data
>                 );
> 
> +  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*)&Private->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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] MdeModulePkg/NvmExpressDxe: Handling return of write to sq and cq db
  2017-04-21  1:38   ` Wu, Hao A
@ 2017-04-24  1:07     ` Wu, Hao A
  0 siblings, 0 replies; 3+ messages in thread
From: Wu, Hao A @ 2017-04-24  1:07 UTC (permalink / raw)
  To: Suman Prakash, edk2-devel@lists.01.org; +Cc: Tian, Feng

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@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 <hao.a.wu@intel.com>
> 

Pushed at f6b139bde7e0a39f83ffad30af58136d5b0738a7.

> 
> Best Regards,
> Hao Wu
> 
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@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 <suman.p@samsung.com>
> > ---
> >  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*)&Private->SqTdbl[QueueId]);
> > -  PciIo->Mem.Write (
> > +  Status = PciIo->Mem.Write (
> >                 PciIo,
> >                 EfiPciIoWidthUint32,
> >                 NVME_BAR,
> > @@ -612,6 +612,10 @@ NvmExpressPassThru (
> >                 &Data
> >                 );
> >
> > +  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*)&Private->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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-04-24  1:07 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CGME20170420100209epcas5p1839e6395a1399a22ab8f705c52e65eeb@epcas5p1.samsung.com>
2017-04-20 10:01 ` [PATCH] MdeModulePkg/NvmExpressDxe: Handling return of write to sq and cq db Suman Prakash
2017-04-21  1:38   ` Wu, Hao A
2017-04-24  1:07     ` Wu, Hao A

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox