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.wu@intel.com>
Sent: Tuesday, October 18, 2022 8:37 PM
To: Anbazhagan, Baraneedharan <anbazhagan@hp.com>; Albecki, Mateusz <mateusz.albecki@intel.com>; devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@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.albecki@intel.com>
> Sent: Tuesday, October 18, 2022 11:54 PM
> To: devel@edk2.groups.io
> Cc: Albecki, Mateusz <mateusz.albecki@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; Ni, Ray <ray.ni@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.wu@intel.com>
>
> Cc: Ray Ni <ray.ni@intel.com>
>
>
> Signed-off-by: Mateusz Albecki <mateusz.albecki@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