<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="EN-US" link="blue" vlink="purple" style="word-wrap:break-word"> <div class="WordSection1"> I see the point, thanks a lot for the testing.<o:p></o:p> <o:p> </o:p> <div> Best Regards,<o:p></o:p> Hao Wu<o:p></o:p> </div> <o:p> </o:p> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm"> 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.wu@intel.com>; Albecki, Mateusz <mateusz.albecki@intel.com>; devel@edk2.groups.io Cc: Ni, Ray <ray.ni@intel.com> Subject: Re: [edk2-devel] [PATCH 1/1] MdeModulePkg/Ata: Fix command status reporting<o:p></o:p> </div> </div> <o:p> </o:p> Below patch didn’t resolve <a href="https://bugzilla.tianocore.org/show_bug.cgi?id=4011"> https://bugzilla.tianocore.org/show_bug.cgi?id=4011</a> since AhciRecoverPortError return success on incorrect drive password and continues with additional retries before returning to caller.<o:p></o:p> <o:p> </o:p> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm"> From: Wu, Hao A <<a href="mailto:hao.a.wu@intel.com">hao.a.wu@intel.com</a>> Sent: Tuesday, October 18, 2022 8:37 PM To: Anbazhagan, Baraneedharan <<a href="mailto:anbazhagan@hp.com">anbazhagan@hp.com</a>>; Albecki, Mateusz <<a href="mailto:mateusz.albecki@intel.com">mateusz.albecki@intel.com</a>>; <a href="mailto:devel@edk2.groups.io">devel@edk2.groups.io</a> Cc: Ni, Ray <<a href="mailto:ray.ni@intel.com">ray.ni@intel.com</a>> Subject: RE: [PATCH 1/1] MdeModulePkg/Ata: Fix command status reporting<o:p></o:p> </div> </div> <o:p> </o:p> CAUTION: External Email <o:p></o:p> <div> Hello Baraneedharan Anbazhagan, Could you help to check if this patch can resolve the issue <a href="https://bugzilla.tianocore.org/show_bug.cgi?id=4011"> https://bugzilla.tianocore.org/show_bug.cgi?id=4011</a> when switching back to: "#define AHCI_COMMAND_RETRIES 5"? This change can be accessed for integration at: <a href="https://github.com/tianocore/edk2/commit/f2eb27e00ed55cca03964f13a2839e41e2c1f61f.patch"> https://github.com/tianocore/edk2/commit/f2eb27e00ed55cca03964f13a2839e41e2c1f61f.patch</a> Best Regards, Hao Wu > -----Original Message----- > From: Albecki, Mateusz <<a href="mailto:mateusz.albecki@intel.com">mateusz.albecki@intel.com</a>> > Sent: Tuesday, October 18, 2022 11:54 PM > To: <a href="mailto:devel@edk2.groups.io">devel@edk2.groups.io</a> > Cc: Albecki, Mateusz <<a href="mailto:mateusz.albecki@intel.com">mateusz.albecki@intel.com</a>>; Wu, Hao A > <<a href="mailto:hao.a.wu@intel.com">hao.a.wu@intel.com</a>>; Ni, Ray <<a href="mailto:ray.ni@intel.com">ray.ni@intel.com</a>> > Subject: [PATCH 1/1] MdeModulePkg/Ata: Fix command status reporting > > Bugzilla: <a href="https://bugzilla.tianocore.org/show_bug.cgi?id=4016">https://bugzilla.tianocore.org/show_bug.cgi?id=4016</a> > > 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 <<a href="mailto:hao.a.wu@intel.com">hao.a.wu@intel.com</a>> > > Cc: Ray Ni <<a href="mailto:ray.ni@intel.com">ray.ni@intel.com</a>> > > > Signed-off-by: Mateusz Albecki <<a href="mailto:mateusz.albecki@intel.com">mateusz.albecki@intel.com</a>> > --- > .../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<o:p></o:p> </div> <div> </o:p> </div> </div> </body> </html> <div width="1" style="color:white;clear:both">_._,_._,_</div> <hr> Groups.io Links: You receive all messages sent to this group. <a target="_blank" href="https://edk2.groups.io/g/devel/message/95455">View/Reply Online (#95455)</a> | | <a target="_blank" href="https://groups.io/mt/94411389/1813853">Mute This Topic</a> | <a href="https://edk2.groups.io/g/devel/post">New Topic</a> <a href="https://edk2.groups.io/g/devel/editsub/1813853">Your Subscription</a> | <a href="mailto:devel+owner@edk2.groups.io">Contact Group Owner</a> | <a href="https://edk2.groups.io/g/devel/unsub">Unsubscribe</a> [edk2-devel-archive@redhat.com] <div width="1" style="color:white;clear:both">_._,_._,_</div>