MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 5 +++++ 1 file changed, 5 insertions(+)
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.guo@linaro.org>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@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
Reviewed-by: Hao Wu <hao.a.wu@intel.com>
Best Regards,
Hao Wu
> -----Original Message-----
> From: Heyi Guo [mailto:heyi.guo@linaro.org]
> Sent: Monday, December 04, 2017 10:28 AM
> To: linaro-uefi@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.guo@linaro.org>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@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
Reviewed-by: Star Zeng <star.zeng@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.guo@linaro.org>; linaro-uefi@lists.linaro.org; edk2-devel@lists.01.org
Cc: Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
Subject: RE: [PATCH v2] MdeModulePkg/NvmExpressDxe: fix error status override
Reviewed-by: Hao Wu <hao.a.wu@intel.com>
Best Regards,
Hao Wu
> -----Original Message-----
> From: Heyi Guo [mailto:heyi.guo@linaro.org]
> Sent: Monday, December 04, 2017 10:28 AM
> To: linaro-uefi@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.guo@linaro.org>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@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
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-uefi@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.zeng@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.guo@linaro.org>; linaro-uefi@lists.linaro.org; edk2-
> devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni,
> Ruiyu <ruiyu.ni@intel.com>
> Subject: RE: [PATCH v2] MdeModulePkg/NvmExpressDxe: fix error status
> override
>
> Reviewed-by: Hao Wu <hao.a.wu@intel.com>
>
> Best Regards,
> Hao Wu
>
>
> > -----Original Message-----
> > From: Heyi Guo [mailto:heyi.guo@linaro.org]
> > Sent: Monday, December 04, 2017 10:28 AM
> > To: linaro-uefi@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.guo@linaro.org>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Hao Wu <hao.a.wu@intel.com>
> > Cc: Ruiyu Ni <ruiyu.ni@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
Thanks Hao :)
在 12/5/2017 8:29 AM, Wu, Hao A 写道:
> 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-uefi@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.zeng@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.guo@linaro.org>; linaro-uefi@lists.linaro.org; edk2-
>> devel@lists.01.org
>> Cc: Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni,
>> Ruiyu <ruiyu.ni@intel.com>
>> Subject: RE: [PATCH v2] MdeModulePkg/NvmExpressDxe: fix error status
>> override
>>
>> Reviewed-by: Hao Wu <hao.a.wu@intel.com>
>>
>> Best Regards,
>> Hao Wu
>>
>>
>>> -----Original Message-----
>>> From: Heyi Guo [mailto:heyi.guo@linaro.org]
>>> Sent: Monday, December 04, 2017 10:28 AM
>>> To: linaro-uefi@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.guo@linaro.org>
>>> Cc: Star Zeng <star.zeng@intel.com>
>>> Cc: Eric Dong <eric.dong@intel.com>
>>> Cc: Hao Wu <hao.a.wu@intel.com>
>>> Cc: Ruiyu Ni <ruiyu.ni@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
© 2016 - 2024 Red Hat, Inc.