public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Tomas Pilar (tpilar)" <Tomas.Pilar@arm.com>
To: <devel@edk2.groups.io>
Cc: <Sami.Mujawar@arm.com>, <nd@arm.com>, Ray Ni <ray.ni@intel.com>,
	"Zhichao Gao" <zhichao.gao@intel.com>
Subject: [PATCH 7/8] ShellPkg/AcpiView: Refactor AcpiView
Date: Mon, 29 Jun 2020 16:20:07 +0100	[thread overview]
Message-ID: <20200629152008.685-8-Tomas.Pilar@arm.com> (raw)
In-Reply-To: <20200629152008.685-1-Tomas.Pilar@arm.com>

Refactor logging using the AcpiViewLog facility.
Trim some of the source to more elegant state.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Tomas Pilar <tomas.pilar@arm.com>
---
 .../UefiShellAcpiViewCommandLib/AcpiParser.c  | 224 ++++++++----------
 .../UefiShellAcpiViewCommandLib/AcpiParser.h  |   6 +-
 .../AcpiTableParser.c                         |  52 ++--
 .../AcpiTableParser.h                         |   2 +-
 .../UefiShellAcpiViewCommandLib/AcpiView.c    | 187 +++++----------
 .../FieldFormatHelper.h                       | 106 +--------
 6 files changed, 187 insertions(+), 390 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
index 65108e25ff96..9bbf724dfdfe 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
@@ -8,6 +8,8 @@
 #include <Uefi.h>
 #include <Library/UefiLib.h>
 #include <Library/UefiBootServicesTableLib.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
 #include "AcpiParser.h"
 #include "AcpiView.h"
 #include "AcpiViewConfig.h"
@@ -46,6 +48,7 @@ IncrementWarningCount (
   mTableWarningCount++;
 }
 
+
 /**
   This function verifies the ACPI table checksum.
 
@@ -69,12 +72,7 @@ VerifyChecksum (
 {
   UINTN ByteCount;
   UINT8 Checksum;
-  UINTN OriginalAttribute;
 
-  //
-  // set local variables to suppress incorrect compiler/analyzer warnings
-  //
-  OriginalAttribute = 0;
   ByteCount = 0;
   Checksum = 0;
 
@@ -84,29 +82,10 @@ VerifyChecksum (
   }
 
   if (Log) {
-    OriginalAttribute = gST->ConOut->Mode->Attribute;
     if (Checksum == 0) {
-      if (mConfig.ColourHighlighting) {
-        gST->ConOut->SetAttribute (
-                       gST->ConOut,
-                       EFI_TEXT_ATTR (EFI_GREEN,
-                         ((OriginalAttribute&(BIT4|BIT5|BIT6))>>4))
-                       );
-      }
-      Print (L"Table Checksum : OK\n\n");
+      AcpiLog (ACPI_GOOD, L"Table Checksum : OK\n");
     } else {
-      IncrementErrorCount ();
-      if (mConfig.ColourHighlighting) {
-        gST->ConOut->SetAttribute (
-                       gST->ConOut,
-                       EFI_TEXT_ATTR (EFI_RED,
-                         ((OriginalAttribute&(BIT4|BIT5|BIT6))>>4))
-                       );
-      }
-      Print (L"Table Checksum : FAILED (0x%X)\n\n", Checksum);
-    }
-    if (mConfig.ColourHighlighting) {
-      gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute);
+      AcpiError (ACPI_ERROR_CSUM, L"Table Checksum (0x%X != 0)\n", Checksum);
     }
   }
 
@@ -127,53 +106,101 @@ DumpRaw (
   )
 {
   UINTN ByteCount;
-  UINTN PartLineChars;
-  UINTN AsciiBufferIndex;
   CHAR8 AsciiBuffer[17];
+  CHAR8 HexBuffer[128];
+  CHAR8 *HexCursor;
 
   ByteCount = 0;
-  AsciiBufferIndex = 0;
 
-  Print (L"Address  : 0x%p\n", Ptr);
-  Print (L"Length   : %d\n", Length);
+  AcpiInfo (L"Address  : 0x%p", Ptr);
+  AcpiInfo (L"Length   : %d\n", Length);
 
   while (ByteCount < Length) {
-    if ((ByteCount & 0x0F) == 0) {
-      AsciiBuffer[AsciiBufferIndex] = '\0';
-      Print (L"  %a\n%08X : ", AsciiBuffer, ByteCount);
-      AsciiBufferIndex = 0;
-    } else if ((ByteCount & 0x07) == 0) {
-      Print (L"- ");
+
+    // Reset ascii and hex strings
+    if (ByteCount % 16 == 0) {
+      HexCursor = HexBuffer;
+      ZeroMem (AsciiBuffer, sizeof(AsciiBuffer));
+      ZeroMem (HexBuffer, sizeof(HexBuffer));
+    } else if (ByteCount % 8 == 0) {
+      HexCursor += AsciiSPrint (HexCursor, sizeof(HexBuffer), "- ");
     }
 
+    // Add hex couplet to hex buffer
+    HexCursor += AsciiSPrint (HexCursor, sizeof(HexBuffer), "%02X ", *Ptr);
+
+    // Add ascii letter to the ascii buffer
+    AsciiBuffer[ByteCount & 0xF] = '.';
     if ((*Ptr >= ' ') && (*Ptr < 0x7F)) {
-      AsciiBuffer[AsciiBufferIndex++] = *Ptr;
-    } else {
-      AsciiBuffer[AsciiBufferIndex++] = '.';
+      AsciiBuffer[ByteCount & 0xF] = *Ptr;
     }
 
-    Print (L"%02X ", *Ptr++);
+    // Print line with fixed width hex part
+    if (ByteCount % 16 == 15) {
+      AcpiInfo (L"%08X : %-.*a %a", ByteCount + 1, 46, HexBuffer, AsciiBuffer);
+    }
 
     ByteCount++;
+    Ptr++;
   }
 
-  // Justify the final line using spaces before printing
-  // the ASCII data.
-  PartLineChars = (Length & 0x0F);
-  if (PartLineChars != 0) {
-    PartLineChars = 48 - (PartLineChars * 3);
-    if ((Length & 0x0F) <= 8) {
-      PartLineChars += 2;
-    }
-    while (PartLineChars > 0) {
-      Print (L" ");
-      PartLineChars--;
+    // Print the last line
+    if (ByteCount % 16 != 15) {
+      AcpiInfo (
+        L"%08X : %-*a %.*a",
+        (ByteCount + 16) & ~0xF,
+        46,
+        HexBuffer,
+        ByteCount & 0xF,
+        AsciiBuffer);
     }
+}
+
+/**
+  Prints an arbitrary variable to screen using a given parser.
+  Also calls the internal validator if it exists.
+
+  @param[in] Parser The parser to use to print to screen
+  @param[in] Prt    Pointer to variable that should be printed
+**/
+STATIC
+VOID
+EFIAPI
+DumpAndValidate(
+  IN CONST ACPI_PARSER* Parser,
+  IN VOID* Ptr
+  )
+{
+  // if there is a Formatter function let the function handle
+  // the printing else if a Format is specified in the table use
+  // the Format for printing
+  PrintFieldName (2, Parser->NameStr);
+  if (Parser->PrintFormatter != NULL) {
+    Parser->PrintFormatter(Parser->Format, Ptr);
+  } else if (Parser->Format != NULL) {
+    switch (Parser->Length) {
+    case 1:
+      AcpiInfo (Parser->Format, *(UINT8 *)Ptr);
+      break;
+    case 2:
+      AcpiInfo (Parser->Format, ReadUnaligned16 ((CONST UINT16 *)Ptr));
+      break;
+    case 4:
+      AcpiInfo (Parser->Format, ReadUnaligned32 ((CONST UINT32 *)Ptr));
+      break;
+    case 8:
+      AcpiInfo (Parser->Format, ReadUnaligned64 ((CONST UINT64 *)Ptr));
+      break;
+    default:
+      AcpiLog (ACPI_BAD, L"<Parse Error>");
+    } // switch
   }
 
-  // Print ASCII data for the final line.
-  AsciiBuffer[AsciiBufferIndex] = '\0';
-  Print (L"  %a\n\n", AsciiBuffer);
+  // Validating only makes sense if we are tracing
+  // the parsed table entries, to report by table name.
+  if (mConfig.ConsistencyCheck && (Parser->FieldValidator != NULL)) {
+    Parser->FieldValidator(Ptr, Parser->Context);
+  }
 }
 
 /**
@@ -216,39 +243,25 @@ ParseAcpi (
 {
   UINT32  Index;
   UINT32  Offset;
-  UINTN   OriginalAttribute;
 
-  //
-  // set local variables to suppress incorrect compiler/analyzer warnings
-  //
-  OriginalAttribute = 0;
-  Offset = 0;
+  if (Length == 0) {
+    AcpiLog (
+      ACPI_WARN,
+      L"Will not parse zero-length buffer <%a>=%p",
+      AsciiName ? AsciiName : "Unknown Item",
+      Ptr);
+    return 0;
+  }
 
   // Increment the Indent
   gIndent += Indent;
 
   if (Trace && (AsciiName != NULL)){
-
-    if (mConfig.ColourHighlighting) {
-      OriginalAttribute = gST->ConOut->Mode->Attribute;
-      gST->ConOut->SetAttribute (
-                     gST->ConOut,
-                     EFI_TEXT_ATTR(EFI_YELLOW,
-                       ((OriginalAttribute&(BIT4|BIT5|BIT6))>>4))
-                     );
-    }
-    Print (
-      L"%*a%-*a :\n",
-      gIndent,
-      "",
-      (OUTPUT_FIELD_COLUMN_WIDTH - gIndent),
-      AsciiName
-      );
-    if (mConfig.ColourHighlighting) {
-      gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute);
-    }
+    AcpiLog (
+      ACPI_ITEM, L"%*.a%a", gIndent, "", AsciiName);
   }
 
+  Offset = 0;
   for (Index = 0; Index < ParserItems; Index++) {
     if ((Offset + Parser[Index].Length) > Length) {
 
@@ -263,10 +276,8 @@ ParseAcpi (
     }
 
     if (mConfig.ConsistencyCheck && (Offset != Parser[Index].Offset)) {
-      IncrementErrorCount ();
-      Print (
-        L"\nERROR: %a: Offset Mismatch for %s\n"
-          L"CurrentOffset = %d FieldOffset = %d\n",
+      AcpiError (ACPI_ERROR_PARSE,
+        L"%a: Offset Mismatch for %s (%d != %d)",
         AsciiName,
         Parser[Index].NameStr,
         Offset,
@@ -275,48 +286,13 @@ ParseAcpi (
     }
 
     if (Trace) {
-      // if there is a Formatter function let the function handle
-      // the printing else if a Format is specified in the table use
-      // the Format for printing
-      PrintFieldName (2, Parser[Index].NameStr);
-      if (Parser[Index].PrintFormatter != NULL) {
-        Parser[Index].PrintFormatter (Parser[Index].Format, Ptr);
-      } else if (Parser[Index].Format != NULL) {
-        switch (Parser[Index].Length) {
-          case 1:
-            DumpUint8 (Parser[Index].Format, Ptr);
-            break;
-          case 2:
-            DumpUint16 (Parser[Index].Format, Ptr);
-            break;
-          case 4:
-            DumpUint32 (Parser[Index].Format, Ptr);
-            break;
-          case 8:
-            DumpUint64 (Parser[Index].Format, Ptr);
-            break;
-          default:
-            Print (
-              L"\nERROR: %a: CANNOT PARSE THIS FIELD, Field Length = %d\n",
-              AsciiName,
-              Parser[Index].Length
-              );
-        } // switch
-
-        // Validating only makes sense if we are tracing
-        // the parsed table entries, to report by table name.
-        if (mConfig.ConsistencyCheck && (Parser[Index].FieldValidator != NULL)) {
-          Parser[Index].FieldValidator (Ptr, Parser[Index].Context);
-        }
-      }
-      Print (L"\n");
-    } // if (Trace)
+      DumpAndValidate (&Parser[Index], &Ptr[Offset]);
+    }
 
     if (Parser[Index].ItemPtr != NULL) {
-      *Parser[Index].ItemPtr = (VOID*)Ptr;
+      *Parser[Index].ItemPtr = Ptr + Offset;
     }
 
-    Ptr += Parser[Index].Length;
     Offset += Parser[Index].Length;
   } // for
 
@@ -355,7 +331,7 @@ DumpGasStruct (
   IN UINT32        Length
   )
 {
-  Print (L"\n");
+  AcpiInfo(L"");
   return ParseAcpi (
            TRUE,
            Indent,
@@ -421,7 +397,7 @@ DumpAcpiHeader (
 UINT32
 EFIAPI
 ParseAcpiHeader (
-  IN  UINT8*         Ptr,
+  IN  VOID*         Ptr,
   OUT CONST UINT32** Signature,
   OUT CONST UINT32** Length,
   OUT CONST UINT8**  Revision
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
index 54ce44132055..bd3cdb774fb5 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
@@ -18,10 +18,6 @@
 /// that allows us to process the log options.
 #define RSDP_TABLE_INFO  SIGNATURE_32('R', 'S', 'D', 'P')
 
-// Publicly accessible error and warning counters.
-extern UINT32   mTableErrorCount;
-extern UINT32   mTableWarningCount;
-
 /**
   This function increments the ACPI table error counter.
 **/
@@ -310,7 +306,7 @@ DumpAcpiHeader (
 UINT32
 EFIAPI
 ParseAcpiHeader (
-  IN  UINT8*         Ptr,
+  IN  VOID*          Ptr,
   OUT CONST UINT32** Signature,
   OUT CONST UINT32** Length,
   OUT CONST UINT8**  Revision
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
index 526cb8cb7cad..49acb3d03da1 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
@@ -18,6 +18,7 @@
 #include "AcpiTableParser.h"
 #include "AcpiView.h"
 #include "AcpiViewConfig.h"
+#include "AcpiViewLog.h"
 
 #if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
 #include "Arm/SbbrValidator.h"
@@ -179,61 +180,43 @@ GetParser (
 VOID
 EFIAPI
 ProcessAcpiTable (
-  IN UINT8* Ptr
+  IN VOID* Ptr
   )
 {
   EFI_STATUS    Status;
   BOOLEAN       Trace;
-  CONST UINT32* AcpiTableSignature;
-  CONST UINT32* AcpiTableLength;
-  CONST UINT8*  AcpiTableRevision;
-  CONST UINT8*  SignaturePtr;
+  CONST UINT32* Signature;
+  CONST UINT32* Length;
+  CONST UINT8*  Revision;
   PARSE_ACPI_TABLE_PROC ParserProc;
 
-  ParseAcpiHeader (
-    Ptr,
-    &AcpiTableSignature,
-    &AcpiTableLength,
-    &AcpiTableRevision
-    );
+  ParseAcpiHeader (Ptr, &Signature, &Length, &Revision);
 
-  Trace = ProcessTableReportOptions (
-            *AcpiTableSignature,
-            Ptr,
-            *AcpiTableLength
-            );
+  Trace = ProcessTableReportOptions (*Signature, Ptr, *Length);
 
   if (Trace) {
-    DumpRaw (Ptr, *AcpiTableLength);
+    DumpRaw (Ptr, *Length);
 
     // Do not process the ACPI table any further if the table length read
     // is invalid. The ACPI table should at least contain the table header.
-    if (*AcpiTableLength < sizeof (EFI_ACPI_DESCRIPTION_HEADER)) {
-      SignaturePtr = (CONST UINT8*)AcpiTableSignature;
-      IncrementErrorCount ();
-      Print (
-        L"ERROR: Invalid %c%c%c%c table length. Length = %d\n",
-        SignaturePtr[0],
-        SignaturePtr[1],
-        SignaturePtr[2],
-        SignaturePtr[3],
-        *AcpiTableLength
-        );
+    if (*Length < sizeof (EFI_ACPI_DESCRIPTION_HEADER)) {
+      AcpiError (
+        ACPI_ERROR_LENGTH, L"Table %4a invalid length %d", Signature, *Length);
       return;
     }
 
     if (mConfig.ConsistencyCheck) {
-      VerifyChecksum (TRUE, Ptr, *AcpiTableLength);
+      VerifyChecksum (TRUE, Ptr, *Length);
     }
   }
 
 #if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
   if (mConfig.MandatoryTableValidate) {
-    ArmSbbrIncrementTableCount (*AcpiTableSignature);
+    ArmSbbrIncrementTableCount (*Signature);
   }
 #endif
 
-  Status = GetParser (*AcpiTableSignature, &ParserProc);
+  Status = GetParser (*Signature, &ParserProc);
   if (EFI_ERROR (Status)) {
     // No registered parser found, do default handling.
     if (Trace) {
@@ -242,10 +225,5 @@ ProcessAcpiTable (
     return;
   }
 
-  ParserProc (
-    Trace,
-    Ptr,
-    *AcpiTableLength,
-    *AcpiTableRevision
-    );
+  ParserProc (Trace, Ptr, *Length, *Revision);
 }
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.h
index 4f92596b90a6..e2afeda2379c 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.h
@@ -101,7 +101,7 @@ DeregisterParser (
 VOID
 EFIAPI
 ProcessAcpiTable (
-  IN UINT8* Ptr
+  IN VOID* Ptr
   );
 
 /**
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
index e25e0712948b..64caff6ab0fe 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
@@ -10,6 +10,8 @@
     - Arm Server Base Boot Requirements 1.2, September 2019
 **/
 
+#include <Guid/Acpi.h>
+
 #include <Library/PrintLib.h>
 #include <Library/UefiLib.h>
 #include <Library/ShellLib.h>
@@ -22,6 +24,7 @@
 #include "AcpiTableParser.h"
 #include "AcpiView.h"
 #include "AcpiViewConfig.h"
+#include "AcpiViewLog.h"
 
 #if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
 #include "Arm/SbbrValidator.h"
@@ -59,7 +62,7 @@ DumpAcpiTableToFile (
     mBinTableCount++
     );
 
-  Print (L"Dumping ACPI table to : %s ... ", FileNameBuffer);
+  AcpiInfo (L"Dumping ACPI table to : %s ... ", FileNameBuffer);
 
   TransferBytes = ShellDumpBufferToFile (FileNameBuffer, Ptr, Length);
   return (Length == TransferBytes);
@@ -81,15 +84,7 @@ ProcessTableReportOptions (
   IN CONST UINT32  Length
   )
 {
-  UINTN               OriginalAttribute;
-  UINT8               *SignaturePtr;
   BOOLEAN             Log;
-
-  //
-  // set local variables to suppress incorrect compiler/analyzer warnings
-  //
-  OriginalAttribute = 0;
-  SignaturePtr = (UINT8*)(UINTN)&Signature;
   Log = FALSE;
 
   switch (mConfig.ReportType) {
@@ -104,27 +99,9 @@ ProcessTableReportOptions (
       break;
     case ReportTableList:
       if (mTableCount == 0) {
-        if (mConfig.ColourHighlighting) {
-          OriginalAttribute = gST->ConOut->Mode->Attribute;
-          gST->ConOut->SetAttribute (
-                         gST->ConOut,
-                         EFI_TEXT_ATTR(EFI_CYAN,
-                           ((OriginalAttribute&(BIT4|BIT5|BIT6))>>4))
-                         );
-        }
-        Print (L"\nInstalled Table(s):\n");
-        if (mConfig.ColourHighlighting) {
-          gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute);
-        }
+        AcpiLog (ACPI_HIGHLIGHT, L"\nInstalled Table(s):");
       }
-      Print (
-        L"\t%4d. %c%c%c%c\n",
-        ++mTableCount,
-        SignaturePtr[0],
-        SignaturePtr[1],
-        SignaturePtr[2],
-        SignaturePtr[3]
-        );
+      AcpiInfo (L"\t%4d. %.*a", ++mTableCount, 4, &Signature);
       break;
     case ReportDumpBinFile:
       if (Signature == mSelectedAcpiTable.Type) {
@@ -139,31 +116,16 @@ ProcessTableReportOptions (
   } // switch
 
   if (Log) {
-    if (mConfig.ColourHighlighting) {
-      OriginalAttribute = gST->ConOut->Mode->Attribute;
-      gST->ConOut->SetAttribute (
-                     gST->ConOut,
-                     EFI_TEXT_ATTR(EFI_LIGHTBLUE,
-                       ((OriginalAttribute&(BIT4|BIT5|BIT6))>>4))
-                     );
-    }
-    Print (
-      L"\n\n --------------- %c%c%c%c Table --------------- \n\n",
-      SignaturePtr[0],
-      SignaturePtr[1],
-      SignaturePtr[2],
-      SignaturePtr[3]
-      );
-    if (mConfig.ColourHighlighting) {
-      gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute);
-    }
+    AcpiLog (
+      ACPI_HIGHLIGHT,
+      L"\n --------------- %.*a Table --------------- \n",
+      4,
+      &Signature);
   }
 
   return Log;
 }
 
-
-
 /**
   This function iterates the configuration table entries in the
   system table, retrieves the RSDP pointer and starts parsing the ACPI tables.
@@ -183,17 +145,12 @@ AcpiView (
   EFI_STATUS               Status;
   UINTN                    Index;
   EFI_CONFIGURATION_TABLE* EfiConfigurationTable;
-  BOOLEAN                  FoundAcpiTable;
-  UINTN                    OriginalAttribute;
-  UINTN                    PrintAttribute;
   UINT8*                   RsdpPtr;
   UINT32                   RsdpLength;
   UINT8                    RsdpRevision;
   PARSE_ACPI_TABLE_PROC    RsdpParserProc;
   BOOLEAN                  Trace;
 
-  OriginalAttribute = 0;
-
   // Reset Table counts
   mTableCount = 0;
   mBinTableCount = 0;
@@ -203,107 +160,77 @@ AcpiView (
   mTableWarningCount = 0;
 
   // Search the table for an entry that matches the ACPI Table Guid
-  FoundAcpiTable = FALSE;
+  EfiConfigurationTable = NULL;
   for (Index = 0; Index < SystemTable->NumberOfTableEntries; Index++) {
     if (CompareGuid (&gEfiAcpiTableGuid,
           &(SystemTable->ConfigurationTable[Index].VendorGuid))) {
       EfiConfigurationTable = &SystemTable->ConfigurationTable[Index];
-      FoundAcpiTable = TRUE;
       break;
     }
   }
 
-  if (FoundAcpiTable) {
-    RsdpPtr = (UINT8*)EfiConfigurationTable->VendorTable;
+  if (!EfiConfigurationTable) {
+    AcpiFatal (L"No ACPI Table Guid in System Configuration Table.");
+    return EFI_NOT_FOUND;
+  }
 
-    // The RSDP revision is 1 byte starting at offset 15
-    RsdpRevision = *(RsdpPtr + RSDP_REVISION_OFFSET);
+  RsdpPtr = (UINT8 *)EfiConfigurationTable->VendorTable;
 
-    if (RsdpRevision < 2) {
-      Print (
-        L"ERROR: RSDP version less than 2 is not supported.\n"
-        );
-      return EFI_UNSUPPORTED;
-    }
+  // The RSDP revision is 1 byte starting at offset 15
+  RsdpRevision = *(RsdpPtr + RSDP_REVISION_OFFSET);
 
-#if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
-    if (mConfig.MandatoryTableValidate) {
-      ArmSbbrResetTableCounts ();
-    }
-#endif
+  if (RsdpRevision < 2) {
+    AcpiFatal (L"RSDP version less than 2 is not supported.");
+    return EFI_UNSUPPORTED;
+  }
 
-    // The RSDP length is 4 bytes starting at offset 20
-    RsdpLength = *(UINT32*)(RsdpPtr + RSDP_LENGTH_OFFSET);
+#if defined(MDE_CPU_ARM) || defined(MDE_CPU_AARCH64)
+  if (mConfig.MandatoryTableValidate) {
+    ArmSbbrResetTableCounts();
+  }
+#endif
 
-    Trace = ProcessTableReportOptions (RSDP_TABLE_INFO, RsdpPtr, RsdpLength);
+  // The RSDP length is 4 bytes starting at offset 20
+  RsdpLength = *(UINT32 *)(RsdpPtr + RSDP_LENGTH_OFFSET);
 
-    Status = GetParser (RSDP_TABLE_INFO, &RsdpParserProc);
-    if (EFI_ERROR (Status)) {
-      Print (
-        L"ERROR: No registered parser found for RSDP.\n"
-        );
-      return Status;
-    }
+  Trace = ProcessTableReportOptions(RSDP_TABLE_INFO, RsdpPtr, RsdpLength);
 
-    RsdpParserProc (
-      Trace,
-      RsdpPtr,
-      RsdpLength,
-      RsdpRevision
-      );
-
-  } else {
-    IncrementErrorCount ();
-    Print (
-      L"ERROR: Failed to find ACPI Table Guid in System Configuration Table.\n"
-      );
-    return EFI_NOT_FOUND;
+  Status = GetParser(RSDP_TABLE_INFO, &RsdpParserProc);
+  if (EFI_ERROR(Status)) {
+    AcpiFatal (L"No registered parser found for RSDP.");
+    return Status;
   }
 
+  RsdpParserProc(Trace, RsdpPtr, RsdpLength, RsdpRevision);
+
 #if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
   if (mConfig.MandatoryTableValidate) {
     ArmSbbrReqsValidate ((ARM_SBBR_VERSION) mConfig.MandatoryTableSpec);
   }
 #endif
 
-  if (ReportTableList != mConfig.ReportType) {
-    if (((ReportSelected == mConfig.ReportType)  ||
-         (ReportDumpBinFile == mConfig.ReportType)) &&
-        (!mSelectedAcpiTable.Found)) {
-      Print (L"\nRequested ACPI Table not found.\n");
-    } else if (mConfig.ConsistencyCheck &&
-               (ReportDumpBinFile != mConfig.ReportType)) {
-      OriginalAttribute = gST->ConOut->Mode->Attribute;
-
-      Print (L"\nTable Statistics:\n");
-
-      if (mConfig.ColourHighlighting) {
-        PrintAttribute = ((mTableErrorCount) > 0) ?
-                            EFI_TEXT_ATTR (
-                              EFI_RED,
-                              ((OriginalAttribute&(BIT4|BIT5|BIT6))>>4)
-                              ) :
-                            OriginalAttribute;
-        gST->ConOut->SetAttribute (gST->ConOut, PrintAttribute);
-      }
-      Print (L"\t%d Error(s)\n", mTableErrorCount);
-
-      if (mConfig.ColourHighlighting) {
-        PrintAttribute = (mTableWarningCount > 0) ?
-                            EFI_TEXT_ATTR (
-                              EFI_RED,
-                              ((OriginalAttribute&(BIT4|BIT5|BIT6))>>4)
-                              ) :
-                            OriginalAttribute;
-
-        gST->ConOut->SetAttribute (gST->ConOut, PrintAttribute);
-      }
-      Print (L"\t%d Warning(s)\n", mTableWarningCount);
+  if (mConfig.ReportType == ReportSelected ||
+      mConfig.ReportType == ReportDumpBinFile) {
+    if (!mSelectedAcpiTable.Found) {
+      AcpiFatal (L"Requested ACPI Table not found.");
+      return EFI_SUCCESS;
+    }
+  }
 
-      if (mConfig.ColourHighlighting) {
-        gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute);
-      }
+  if (mConfig.ConsistencyCheck) {
+    if (mConfig.ReportType == ReportSelected ||
+        mConfig.ReportType == ReportAll) {
+      AcpiInfo (L"Table Statistics:");
+      AcpiLog (
+        mTableErrorCount ? ACPI_BAD : ACPI_GOOD,
+        L"\t%d Error(s)",
+        mTableErrorCount);
+      AcpiLog (
+        mTableWarningCount ? ACPI_BAD : ACPI_GOOD,
+        L"\t%d Warning(s)\n",
+        mTableWarningCount);
     }
   }
+
   return EFI_SUCCESS;
 }
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/FieldFormatHelper.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/FieldFormatHelper.h
index 25c70652806c..b0072b68844c 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/FieldFormatHelper.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/FieldFormatHelper.h
@@ -8,90 +8,10 @@
 #ifndef FIELD_FORMAT_HELPER_H_
 #define FIELD_FORMAT_HELPER_H_
 
-#include <Library/UefiLib.h>
-#include <Uefi.h>
-
-/**
-  This function traces 1 byte of data as specified in the format string.
-
-  @param [in] Format  The format string for tracing the data.
-  @param [in] Ptr     Pointer to the start of the buffer.
-**/
-static
-inline
-VOID
-EFIAPI
-DumpUint8 (
-  IN CONST CHAR16* Format,
-  IN UINT8*        Ptr
-  )
-{
-  Print (Format, *Ptr);
-}
-
-/**
-  This function traces 2 bytes of data as specified in the format string.
-
-  @param [in] Format  The format string for tracing the data.
-  @param [in] Ptr     Pointer to the start of the buffer.
-**/
-static
-inline
-VOID
-EFIAPI
-DumpUint16 (
-  IN CONST CHAR16* Format,
-  IN UINT8*        Ptr
-  )
-{
-  Print (Format, *(UINT16*)Ptr);
-}
-
-/**
-  This function traces 4 bytes of data as specified in the format string.
-
-  @param [in] Format  The format string for tracing the data.
-  @param [in] Ptr     Pointer to the start of the buffer.
-**/
-static
-inline
-VOID
-EFIAPI
-DumpUint32 (
-  IN CONST CHAR16* Format,
-  IN UINT8*        Ptr
-  )
-{
-  Print (Format, *(UINT32*)Ptr);
-}
-
-/**
-  This function traces 8 bytes of data as specified by the format string.
+#define INLINE inline
 
-  @param [in] Format  The format string for tracing the data.
-  @param [in] Ptr     Pointer to the start of the buffer.
-**/
-static
-inline
-VOID
-EFIAPI
-DumpUint64 (
-  IN CONST CHAR16* Format,
-  IN UINT8*        Ptr
-  )
-{
-  // Some fields are not aligned and this causes alignment faults
-  // on ARM platforms if the compiler generates LDRD instructions.
-  // Perform word access so that LDRD instructions are not generated.
-  UINT64 Val;
-
-  Val = *(UINT32*)(Ptr + sizeof (UINT32));
-
-  Val = LShiftU64(Val,32);
-  Val |= (UINT64)*(UINT32*)Ptr;
-
-  Print (Format, Val);
-}
+#include <Uefi.h>
+#include "AcpiViewLog.h"
 
 /**
   This function traces 3 characters which can be optionally
@@ -111,8 +31,8 @@ Dump3Chars (
   IN UINT8*        Ptr
   )
 {
-  Print (
-    (Format != NULL) ? Format : (CONST CHAR16*) L"%c%c%c",
+  AcpiInfo (
+    (Format != NULL) ? Format : L"%c%c%c",
     Ptr[0],
     Ptr[1],
     Ptr[2]
@@ -137,8 +57,8 @@ Dump4Chars (
   IN UINT8*        Ptr
   )
 {
-  Print (
-    (Format != NULL) ? Format : (CONST CHAR16*) L"%c%c%c%c",
+  AcpiInfo(
+    (Format != NULL) ? Format : L"%c%c%c%c",
     Ptr[0],
     Ptr[1],
     Ptr[2],
@@ -164,8 +84,8 @@ Dump6Chars (
   IN UINT8*        Ptr
   )
 {
-  Print (
-    (Format != NULL) ? Format : (CONST CHAR16*) L"%c%c%c%c%c%c",
+  AcpiInfo(
+    (Format != NULL) ? Format : L"%c%c%c%c%c%c",
     Ptr[0],
     Ptr[1],
     Ptr[2],
@@ -193,8 +113,8 @@ Dump8Chars (
   IN UINT8*        Ptr
   )
 {
-  Print (
-    (Format != NULL) ? Format : (CONST CHAR16*) L"%c%c%c%c%c%c%c%c",
+  AcpiInfo(
+    (Format != NULL) ? Format : L"%c%c%c%c%c%c%c%c",
     Ptr[0],
     Ptr[1],
     Ptr[2],
@@ -224,8 +144,8 @@ Dump12Chars (
   IN       UINT8*  Ptr
   )
 {
-  Print (
-    (Format != NULL) ? Format : (CONST CHAR16*) L"%c%c%c%c%c%c%c%c%c%c%c%c",
+  AcpiInfo(
+    (Format != NULL) ? Format : L"%c%c%c%c%c%c%c%c%c%c%c%c",
     Ptr[0],
     Ptr[1],
     Ptr[2],
-- 
2.24.1.windows.2



  parent reply	other threads:[~2020-06-29 15:20 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-29 15:20 [PATCH 0/8] ShellPkg/AcpiView: Refactor Error Logging Tomas Pilar (tpilar)
2020-06-29 15:20 ` [PATCH 1/8] ShellPkg/AcpiView: Extract configuration struct Tomas Pilar (tpilar)
2020-07-10  6:15   ` Gao, Zhichao
2020-06-29 15:20 ` [PATCH 2/8] ShellPkg/AcpiView: Declutter error counters Tomas Pilar (tpilar)
2020-06-29 15:20 ` [PATCH 3/8] ShellPkg/AcpiView: Modify error message Tomas Pilar (tpilar)
2020-06-29 15:20 ` [PATCH 4/8] ShellPkg/AcpiView: Create a logging facility Tomas Pilar (tpilar)
2020-07-10  6:26   ` Gao, Zhichao
2020-06-29 15:20 ` [PATCH 5/8] ShellPkg/AcpiView: Refactor PrintFieldName Tomas Pilar (tpilar)
2020-06-29 15:20 ` [PATCH 6/8] ShellPkg/AcpiView: Refactor dump helpers Tomas Pilar (tpilar)
2020-06-29 15:20 ` Tomas Pilar (tpilar) [this message]
2020-06-29 15:20 ` [PATCH 8/8] ShellPkg/AcpiView: Refactor table parsers Tomas Pilar (tpilar)
2020-07-10  6:42   ` [edk2-devel] " Gao, Zhichao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200629152008.685-8-Tomas.Pilar@arm.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox