Pushed at 9a77210b43ef34af52ea7285fadc0ce5779306fe.

Best Regards,
Hao Wu


> -----Original Message-----
> From: Zeng, Star
> Sent: Monday, December 04, 2017 1:54 PM
> To: Wu, Hao A; Heyi Guo; linaro-u...@lists.linaro.org; edk2-devel@lists.01.org
> Cc: Dong, Eric; Ni, Ruiyu; Zeng, Star
> Subject: RE: [PATCH v2] MdeModulePkg/NvmExpressDxe: fix error status
> override
> 
> Reviewed-by: Star Zeng <star.z...@intel.com>
> 
> Hao, please help push the patch if no other comments received.
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: Wu, Hao A
> Sent: Monday, December 4, 2017 10:47 AM
> To: Heyi Guo <heyi....@linaro.org>; linaro-u...@lists.linaro.org; edk2-
> de...@lists.01.org
> Cc: Zeng, Star <star.z...@intel.com>; Dong, Eric <eric.d...@intel.com>; Ni,
> Ruiyu <ruiyu...@intel.com>
> Subject: RE: [PATCH v2] MdeModulePkg/NvmExpressDxe: fix error status
> override
> 
> Reviewed-by: Hao Wu <hao.a...@intel.com>
> 
> Best Regards,
> Hao Wu
> 
> 
> > -----Original Message-----
> > From: Heyi Guo [mailto:heyi....@linaro.org]
> > Sent: Monday, December 04, 2017 10:28 AM
> > To: linaro-u...@lists.linaro.org; edk2-devel@lists.01.org
> > Cc: Heyi Guo; Zeng, Star; Dong, Eric; Wu, Hao A; Ni, Ruiyu
> > Subject: [PATCH v2] MdeModulePkg/NvmExpressDxe: fix error status
> > override
> >
> > Commit f6b139b added return status handling to PciIo->Mem.Write.
> > However, the second status handling will override EFI_DEVICE_ERROR
> > returned in this branch:
> >
> >   //
> >   // Check the NVMe cmd execution result
> >   //
> >   if (Status != EFI_TIMEOUT) {
> >     if ((Cq->Sct == 0) && (Cq->Sc == 0)) {
> >       Status = EFI_SUCCESS;
> >     } else {
> >       Status = EFI_DEVICE_ERROR;
> >                ^^^^^^^^^^^^^^^^
> >
> > Since PciIo->Mem.Write will probably return SUCCESS, it causes
> > NvmExpressPassThru to return SUCCESS even when DEVICE_ERROR occurs.
> > Callers of NvmExpressPassThru will then continue executing which may
> > cause further unexpected results, e.g. DiscoverAllNamespaces couldn't
> > break out the loop.
> >
> > So we save previous status before calling PciIo->Mem.Write and restore
> > the previous one if it already contains error.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Heyi Guo <heyi....@linaro.org>
> > Cc: Star Zeng <star.z...@intel.com>
> > Cc: Eric Dong <eric.d...@intel.com>
> > Cc: Hao Wu <hao.a...@intel.com>
> > Cc: Ruiyu Ni <ruiyu...@intel.com>
> > ---
> >  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> > b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> > index c33038f..7356c1d 100644
> > --- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> > +++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> > @@ -453,6 +453,7 @@ NvmExpressPassThru (  {
> >    NVME_CONTROLLER_PRIVATE_DATA   *Private;
> >    EFI_STATUS                     Status;
> > +  EFI_STATUS                     PreviousStatus;
> >    EFI_PCI_IO_PROTOCOL            *PciIo;
> >    NVME_SQ                        *Sq;
> >    NVME_CQ                        *Cq;
> > @@ -831,6 +832,7 @@ NvmExpressPassThru (
> >    }
> >
> >    Data = ReadUnaligned32 ((UINT32*)&Private->CqHdbl[QueueId]);
> > +  PreviousStatus = Status;
> >    Status = PciIo->Mem.Write (
> >                 PciIo,
> >                 EfiPciIoWidthUint32,
> > @@ -839,6 +841,9 @@ NvmExpressPassThru (
> >                 1,
> >                 &Data
> >                 );
> > +  // The return status of PciIo->Mem.Write should not override  //
> > + previous status if previous status contains error.
> > +  Status = EFI_ERROR (PreviousStatus) ? PreviousStatus : Status;
> >
> >    //
> >    // For now, the code does not support the non-blocking feature for
> > admin queue.
> > --
> > 2.7.2.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to