I see the point, thanks a lot for the testing. Best Regards, Hao Wu
From: [email protected] <[email protected]> On Behalf Of Anbazhagan, Baraneedharan via groups.io Sent: Friday, October 21, 2022 6:05 AM To: Wu, Hao A <[email protected]>; Albecki, Mateusz <[email protected]>; [email protected] Cc: Ni, Ray <[email protected]> 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 <[email protected]<mailto:[email protected]>> Sent: Tuesday, October 18, 2022 8:37 PM To: Anbazhagan, Baraneedharan <[email protected]<mailto:[email protected]>>; Albecki, Mateusz <[email protected]<mailto:[email protected]>>; [email protected]<mailto:[email protected]> Cc: Ni, Ray <[email protected]<mailto:[email protected]>> 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 > <[email protected]<mailto:[email protected]>> > Sent: Tuesday, October 18, 2022 11:54 PM > To: [email protected]<mailto:[email protected]> > Cc: Albecki, Mateusz > <[email protected]<mailto:[email protected]>>; Wu, Hao A > <[email protected]<mailto:[email protected]>>; Ni, Ray > <[email protected]<mailto:[email protected]>> > 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 <[email protected]<mailto:[email protected]>> > > Cc: Ray Ni <[email protected]<mailto:[email protected]>> > > > Signed-off-by: Mateusz Albecki > <[email protected]<mailto:[email protected]>> > --- > .../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: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
