I see the point, thanks a lot for the testing. Best Regards, Hao Wu
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Anbazhagan, Baraneedharan via groups.io Sent: Friday, October 21, 2022 6:05 AM To: Wu, Hao A <hao.a...@intel.com>; Albecki, Mateusz <mateusz.albe...@intel.com>; devel@edk2.groups.io Cc: Ni, Ray <ray...@intel.com> Subject: Re: [edk2-devel] [PATCH 1/1] MdeModulePkg/Ata: Fix command status reporting Below patch didn't resolve https://bugzilla.tianocore.org/show_bug.cgi?id=4011 since AhciRecoverPortError return success on incorrect drive password and continues with additional retries before returning to caller. From: Wu, Hao A <hao.a...@intel.com<mailto:hao.a...@intel.com>> Sent: Tuesday, October 18, 2022 8:37 PM To: Anbazhagan, Baraneedharan <anbazha...@hp.com<mailto:anbazha...@hp.com>>; Albecki, Mateusz <mateusz.albe...@intel.com<mailto:mateusz.albe...@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> Cc: Ni, Ray <ray...@intel.com<mailto:ray...@intel.com>> Subject: RE: [PATCH 1/1] MdeModulePkg/Ata: Fix command status reporting CAUTION: External Email Hello Baraneedharan Anbazhagan, Could you help to check if this patch can resolve the issue https://bugzilla.tianocore.org/show_bug.cgi?id=4011 when switching back to: "#define AHCI_COMMAND_RETRIES 5"? This change can be accessed for integration at: https://github.com/tianocore/edk2/commit/f2eb27e00ed55cca03964f13a2839e41e2c1f61f.patch Best Regards, Hao Wu > -----Original Message----- > From: Albecki, Mateusz > <mateusz.albe...@intel.com<mailto:mateusz.albe...@intel.com>> > Sent: Tuesday, October 18, 2022 11:54 PM > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> > Cc: Albecki, Mateusz > <mateusz.albe...@intel.com<mailto:mateusz.albe...@intel.com>>; Wu, Hao A > <hao.a...@intel.com<mailto:hao.a...@intel.com>>; Ni, Ray > <ray...@intel.com<mailto:ray...@intel.com>> > Subject: [PATCH 1/1] MdeModulePkg/Ata: Fix command status reporting > > Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=4016 > > AtaAtapiPassThru driver was reporting recovery status on failed command > packets which led to incorrect flows in upper layers and to SCT tests > fails. This commit will change the logic to report command status. > > > Cc: Hao A Wu <hao.a...@intel.com<mailto:hao.a...@intel.com>> > > Cc: Ray Ni <ray...@intel.com<mailto:ray...@intel.com>> > > > Signed-off-by: Mateusz Albecki > <mateusz.albe...@intel.com<mailto:mateusz.albe...@intel.com>> > --- > .../Bus/Ata/AtaAtapiPassThru/AhciMode.c | 19 +++++++++++-------- > 1 file changed, 11 insertions(+), 8 deletions(-) > > diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c > b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c > index a240be940d..06c4a3e052 100644 > --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c > +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c > @@ -949,6 +949,7 @@ AhciPioTransfer ( > EFI_AHCI_COMMAND_LIST CmdList; > > UINT32 PrdCount; > > UINT32 Retry; > > + EFI_STATUS RecoveryStatus; > > > > if (Read) { > > Flag = EfiPciIoOperationBusMasterWrite; > > @@ -1026,8 +1027,8 @@ AhciPioTransfer ( > > > if (Status == EFI_DEVICE_ERROR) { > > DEBUG ((DEBUG_ERROR, "PIO command failed at retry %d\n", Retry)); > > - Status = AhciRecoverPortError (PciIo, Port); > > - if (EFI_ERROR (Status)) { > > + RecoveryStatus = AhciRecoverPortError (PciIo, Port); > > + if (EFI_ERROR (RecoveryStatus)) { > > break; > > } > > } else { > > @@ -1122,6 +1123,7 @@ AhciDmaTransfer ( > EFI_PCI_IO_PROTOCOL *PciIo; > > EFI_TPL OldTpl; > > UINT32 Retry; > > + EFI_STATUS RecoveryStatus; > > > > Map = NULL; > > PciIo = Instance->PciIo; > > @@ -1220,8 +1222,8 @@ AhciDmaTransfer ( > Status = AhciWaitUntilFisReceived (PciIo, Port, Timeout, SataFisD2H); > > if (Status == EFI_DEVICE_ERROR) { > > DEBUG ((DEBUG_ERROR, "DMA command failed at retry: %d\n", Retry)); > > - Status = AhciRecoverPortError (PciIo, Port); > > - if (EFI_ERROR (Status)) { > > + RecoveryStatus = AhciRecoverPortError (PciIo, Port); > > + if (EFI_ERROR (RecoveryStatus)) { > > break; > > } > > } else { > > @@ -1261,14 +1263,14 @@ AhciDmaTransfer ( > Status = AhciCheckFisReceived (PciIo, Port, SataFisD2H); > > if (Status == EFI_DEVICE_ERROR) { > > DEBUG ((DEBUG_ERROR, "DMA command failed at retry: %d\n", Task- > >RetryTimes)); > > - Status = AhciRecoverPortError (PciIo, Port); > > + RecoveryStatus = AhciRecoverPortError (PciIo, Port); > > // > > // If recovery passed mark the Task as not started and change the status > > // to EFI_NOT_READY. This will make the higher level call this function > again > > // and on next call the command will be re-issued due to IsStart being > FALSE. > > // This also makes the next condition decrement the RetryTimes. > > // > > - if (Status == EFI_SUCCESS) { > > + if (RecoveryStatus == EFI_SUCCESS) { > > Task->IsStart = FALSE; > > Status = EFI_NOT_READY; > > } > > @@ -1375,6 +1377,7 @@ AhciNonDataTransfer ( > EFI_AHCI_COMMAND_FIS CFis; > > EFI_AHCI_COMMAND_LIST CmdList; > > UINT32 Retry; > > + EFI_STATUS RecoveryStatus; > > > > // > > // Package read needed > > @@ -1415,8 +1418,8 @@ AhciNonDataTransfer ( > Status = AhciWaitUntilFisReceived (PciIo, Port, Timeout, SataFisD2H); > > if (Status == EFI_DEVICE_ERROR) { > > DEBUG ((DEBUG_ERROR, "Non data transfer failed at retry %d\n", Retry)); > > - Status = AhciRecoverPortError (PciIo, Port); > > - if (EFI_ERROR (Status)) { > > + RecoveryStatus = AhciRecoverPortError (PciIo, Port); > > + if (EFI_ERROR (RecoveryStatus)) { > > break; > > } > > } else { > > -- > 2.28.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#95455): https://edk2.groups.io/g/devel/message/95455 Mute This Topic: https://groups.io/mt/94411389/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
