[edk2-devel] [PATCH 4/4] MdeModulePkg/AtaAtapiPassThru: Trace ATA packets
Wu, Hao A
hao.a.wu at intel.com
Wed Nov 4 05:33:41 UTC 2020
> -----Original Message-----
> From: Albecki, Mateusz <mateusz.albecki at intel.com>
> Sent: Tuesday, November 3, 2020 9:24 PM
> To: devel at edk2.groups.io
> Cc: Albecki, Mateusz <mateusz.albecki at intel.com>; Ni, Ray
> <ray.ni at intel.com>; Wu, Hao A <hao.a.wu at intel.com>
> Subject: [PATCH 4/4] MdeModulePkg/AtaAtapiPassThru: Trace ATA packets
>
> From: Albecki <mateusz.albecki at intel.com>
>
> This simplify ATA driver debugging all ATA packets will be printed to debug
> port on DEBUG_VERBOSE level along with the packet execution status.
> Additionally failed packets and the failed packet execution status will be
> printed on DEBUG_ERROR level.
A couple of minor comments:
1. Could you help to correct the typos for "commmand" to "command"?
2. For the below debug messages added in AhciPioTransfer(), AhciDmaTransfer() and AhciNonDataTransfer():
DEBUG ((DEBUG_VERBOSE, "Starting commmand: \n"));
DEBUG ((DEBUG_ERROR, "Failed to execute command:\n"));
Could you help to add the transfer type information. Like for non-blocking DMA transfer:
DEBUG ((DEBUG_VERBOSE, "Starting command for non-blocking DMA transfer: \n"));
DEBUG ((DEBUG_ERROR, "Failed to execute command for non-blocking DMA transfer:\n"));
This might be helpful to identify these messages with their corresponding transfer request.
Best Regards,
Hao Wu
>
> Signed-off-by: Mateusz Albecki <mateusz.albecki at intel.com>
>
> Cc: Ray Ni <ray.ni at intel.com>
> Cc: Hao A Wu <hao.a.wu at intel.com>
>
> ---
> .../Bus/Ata/AtaAtapiPassThru/AhciMode.c | 94 +++++++++++++++++++
> 1 file changed, 94 insertions(+)
>
> diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> index 4fe7e4b1dc..3a7a6eb018 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> @@ -846,6 +846,54 @@ AhciWaitUntilFisReceived (
> return EFI_TIMEOUT;
> }
>
> +/**
> + Prints contents of the ATA command block into the debug port.
> +
> + @param[in] AtaCommandBlock AtaCommandBlock to print.
> + @param[in] DebugLevel Debug level on which to print.
> +**/
> +VOID
> +AhciPrintCommandBlock (
> + IN EFI_ATA_COMMAND_BLOCK *AtaCommandBlock,
> + IN UINT32 DebugLevel
> + )
> +{
> + DEBUG ((DebugLevel, "ATA COMMAND BLOCK:\n"));
> + DEBUG ((DebugLevel, "AtaCommand: %d\n",
> +AtaCommandBlock->AtaCommand));
> + DEBUG ((DebugLevel, "AtaFeatures: %X\n",
> +AtaCommandBlock->AtaFeatures));
> + DEBUG ((DebugLevel, "AtaSectorNumber: %d\n",
> +AtaCommandBlock->AtaSectorNumber));
> + DEBUG ((DebugLevel, "AtaCylinderLow: %X\n",
> +AtaCommandBlock->AtaCylinderHigh));
> + DEBUG ((DebugLevel, "AtaCylinderHigh: %X\n",
> +AtaCommandBlock->AtaCylinderHigh));
> + DEBUG ((DebugLevel, "AtaDeviceHead: %d\n",
> +AtaCommandBlock->AtaDeviceHead));
> + DEBUG ((DebugLevel, "AtaSectorNumberExp: %d\n",
> +AtaCommandBlock->AtaSectorNumberExp));
> + DEBUG ((DebugLevel, "AtaCylinderLowExp: %X\n",
> +AtaCommandBlock->AtaCylinderLowExp));
> + DEBUG ((DebugLevel, "AtaCylinderHighExp: %X\n",
> +AtaCommandBlock->AtaCylinderHighExp));
> + DEBUG ((DebugLevel, "AtaFeaturesExp: %X\n",
> +AtaCommandBlock->AtaFeaturesExp));
> + DEBUG ((DebugLevel, "AtaSectorCount: %d\n",
> +AtaCommandBlock->AtaSectorCount));
> + DEBUG ((DebugLevel, "AtaSectorCountExp: %d\n",
> +AtaCommandBlock->AtaSectorCountExp));
> +}
> +
> +/**
> + Prints contents of the ATA status block into the debug port.
> +
> + @param[in] AtaStatusBlock AtaStatusBlock to print.
> + @param[in] DebugLevel Debug level on which to print.
> +**/
> +VOID
> +AhciPrintStatusBlock (
> + IN EFI_ATA_STATUS_BLOCK *AtaStatusBlock,
> + IN UINT32 DebugLevel
> + )
> +{
> + //
> + // Only print status and error since we have all of the rest printed
> +as
> + // a part of command block print.
> + //
> + DEBUG ((DebugLevel, "ATA STATUS BLOCK:\n"));
> + DEBUG ((DebugLevel, "AtaStatus: %d\n", AtaStatusBlock->AtaStatus));
> + DEBUG ((DebugLevel, "AtaError: %d\n", AtaStatusBlock->AtaError)); }
> +
> /**
> Start a PIO data transfer on specific port.
>
> @@ -947,6 +995,8 @@ AhciPioTransfer (
> DataCount
> );
>
> + DEBUG ((DEBUG_VERBOSE, "Starting commmand: \n"));
> + AhciPrintCommandBlock (AtaCommandBlock, DEBUG_VERBOSE);
> Status = AhciStartCommand (
> PciIo,
> Port,
> @@ -1000,6 +1050,19 @@ AhciPioTransfer (
> );
>
> AhciDumpPortStatus (PciIo, AhciRegisters, Port, AtaStatusBlock);
> +
> + if (Status == EFI_DEVICE_ERROR) {
> + DEBUG ((DEBUG_ERROR, "Failed to execute command:\n"));
> + //
> + // Repeat command block here to make sure it is printed on
> + // device error debug level.
> + //
> + AhciPrintCommandBlock (AtaCommandBlock, DEBUG_ERROR);
> + AhciPrintStatusBlock (AtaStatusBlock, DEBUG_ERROR); } else {
> + AhciPrintStatusBlock (AtaStatusBlock, DEBUG_VERBOSE); }
> +
> return Status;
> }
>
> @@ -1132,6 +1195,8 @@ AhciDmaTransfer (
> DataCount
> );
>
> + DEBUG ((DEBUG_VERBOSE, "Starting commmand: \n"));
> + AhciPrintCommandBlock (AtaCommandBlock, DEBUG_VERBOSE);
> Status = AhciStartCommand (
> PciIo,
> Port,
> @@ -1168,6 +1233,8 @@ AhciDmaTransfer (
> DataCount
> );
>
> + DEBUG ((DEBUG_VERBOSE, "Starting commmand: \n"));
> + AhciPrintCommandBlock (AtaCommandBlock, DEBUG_VERBOSE);
> Status = AhciStartCommand (
> PciIo,
> Port,
> @@ -1238,6 +1305,19 @@ AhciDmaTransfer (
> }
>
> AhciDumpPortStatus (PciIo, AhciRegisters, Port, AtaStatusBlock);
> +
> + if (Status == EFI_DEVICE_ERROR) {
> + DEBUG ((DEBUG_ERROR, "Failed to execute command:\n"));
> + //
> + // Repeat command block here to make sure it is printed on
> + // device error debug level.
> + //
> + AhciPrintCommandBlock (AtaCommandBlock, DEBUG_ERROR);
> + AhciPrintStatusBlock (AtaStatusBlock, DEBUG_ERROR); } else {
> + AhciPrintStatusBlock (AtaStatusBlock, DEBUG_VERBOSE); }
> +
> return Status;
> }
>
> @@ -1307,6 +1387,8 @@ AhciNonDataTransfer (
> 0
> );
>
> + DEBUG ((DEBUG_VERBOSE, "Starting commmand: \n"));
> + AhciPrintCommandBlock (AtaCommandBlock, DEBUG_VERBOSE);
> Status = AhciStartCommand (
> PciIo,
> Port,
> @@ -1343,6 +1425,18 @@ AhciNonDataTransfer (
>
> AhciDumpPortStatus (PciIo, AhciRegisters, Port, AtaStatusBlock);
>
> + if (Status == EFI_DEVICE_ERROR) {
> + DEBUG ((DEBUG_ERROR, "Failed to execute command:\n"));
> + //
> + // Repeat command block here to make sure it is printed on
> + // device error debug level.
> + //
> + AhciPrintCommandBlock (AtaCommandBlock, DEBUG_ERROR);
> + AhciPrintStatusBlock (AtaStatusBlock, DEBUG_ERROR); } else {
> + AhciPrintStatusBlock (AtaStatusBlock, DEBUG_VERBOSE); }
> +
> return Status;
> }
>
> --
> 2.28.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#66962): https://edk2.groups.io/g/devel/message/66962
Mute This Topic: https://groups.io/mt/78005852/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list