1. Patchew
  2. EDK2
  3. [edk2-devel] [PATCH 0/8] ShellPkg/AcpiView: Refactor Error Logging
  4. View patch

[edk2-devel] [PATCH 8/8] ShellPkg/AcpiView: Refactor table parsers

Tomas Pilar (tpilar) posted 8 patches 5 years, 7 months ago
There is a newer version of this series
[edk2-devel] [PATCH 8/8] ShellPkg/AcpiView: Refactor table parsers
Posted by Tomas Pilar (tpilar) 5 years, 7 months ago 
The tests for checking specific constraints and checking
for buffer overflows have been simplified to use a standard
set of templates defined in the logging facility.

This regularises some of the error handling and makes
it easier to write more tests like this in the future.

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  |  25 ---
 .../UefiShellAcpiViewCommandLib/AcpiParser.h  |  18 --
 .../Arm/SbbrValidator.c                       |  65 +++---
 .../Parsers/Dbg2/Dbg2Parser.c                 | 118 +++-------
 .../Parsers/Fadt/FadtParser.c                 |  48 ++--
 .../Parsers/Gtdt/GtdtParser.c                 |  84 ++-----
 .../Parsers/Iort/IortParser.c                 | 207 +++++++-----------
 .../Parsers/Madt/MadtParser.c                 |  99 +++------
 .../Parsers/Mcfg/McfgParser.c                 |  11 +-
 .../Parsers/Pptt/PpttParser.c                 | 165 +++-----------
 .../Parsers/Rsdp/RsdpParser.c                 |  42 +---
 .../Parsers/Slit/SlitParser.c                 | 122 ++++-------
 .../Parsers/Spcr/SpcrParser.c                 |  31 +--
 .../Parsers/Srat/SratParser.c                 | 188 +++++-----------
 .../Parsers/Xsdt/XsdtParser.c                 |  92 ++------
 15 files changed, 375 insertions(+), 940 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
index 9bbf724dfdfe..58599442d210 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
@@ -24,31 +24,6 @@ STATIC CONST ACPI_PARSER AcpiHeaderParser[] = {
   PARSE_ACPI_HEADER (&AcpiHdrInfo)
 };
 
-/**
-  This function increments the ACPI table error counter.
-**/
-VOID
-EFIAPI
-IncrementErrorCount (
-  VOID
-  )
-{
-  mTableErrorCount++;
-}
-
-/**
-  This function increments the ACPI table warning counter.
-**/
-VOID
-EFIAPI
-IncrementWarningCount (
-  VOID
-  )
-{
-  mTableWarningCount++;
-}
-
-
 /**
   This function verifies the ACPI table checksum.
 
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
index bd3cdb774fb5..cdae433fef3b 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
@@ -18,24 +18,6 @@
 /// that allows us to process the log options.
 #define RSDP_TABLE_INFO  SIGNATURE_32('R', 'S', 'D', 'P')
 
-/**
-  This function increments the ACPI table error counter.
-**/
-VOID
-EFIAPI
-IncrementErrorCount (
-  VOID
-  );
-
-/**
-  This function increments the ACPI table warning counter.
-**/
-VOID
-EFIAPI
-IncrementWarningCount (
-  VOID
-  );
-
 /**
   This function verifies the ACPI table checksum.
 
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Arm/SbbrValidator.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Arm/SbbrValidator.c
index d3284417fa5f..ba80a5ab3b40 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Arm/SbbrValidator.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Arm/SbbrValidator.c
@@ -18,15 +18,16 @@
 #include <Library/DebugLib.h>
 #include <Library/UefiLib.h>
 #include "AcpiParser.h"
+#include "AcpiViewLog.h"
 #include "Arm/SbbrValidator.h"
 
 /**
   SBBR specification version strings
 **/
-STATIC CONST CHAR8* ArmSbbrVersions[ArmSbbrVersionMax] = {
-  "1.0",     // ArmSbbrVersion_1_0
-  "1.1",     // ArmSbbrVersion_1_1
-  "1.2"      // ArmSbbrVersion_1_2
+STATIC CONST CHAR16* ArmSbbrVersions[ArmSbbrVersionMax] = {
+  L"SBBR-v1.0",     // ArmSbbrVersion_1_0
+  L"SBBR-v1.1",     // ArmSbbrVersion_1_1
+  L"SBBR-v1.2"      // ArmSbbrVersion_1_2
 };
 
 /**
@@ -96,6 +97,16 @@ STATIC ACPI_TABLE_COUNTER ArmSbbrTableCounts[] = {
   {EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGNATURE, 0}
 };
 
+STATIC_ASSERT (
+  ARRAY_SIZE (ArmSbbr10Mandatory) <= ARRAY_SIZE (ArmSbbrTableCounts),
+  "Incompatible mandatory array tables");
+STATIC_ASSERT (
+  ARRAY_SIZE (ArmSbbr11Mandatory) <= ARRAY_SIZE (ArmSbbrTableCounts),
+  "Incompatible mandatory array tables");
+STATIC_ASSERT (
+  ARRAY_SIZE (ArmSbbr12Mandatory) <= ARRAY_SIZE (ArmSbbrTableCounts),
+  "Incompatible mandatory array tables");
+
 /**
   Reset the platform ACPI table instance count for all SBBR-mandatory tables.
 **/
@@ -160,7 +171,6 @@ ArmSbbrReqsValidate (
   UINT32        Table;
   UINT32        Index;
   UINT32        MandatoryTable;
-  CONST UINT8*  SignaturePtr;
   BOOLEAN       IsArmSbbrViolated;
 
   if (Version >= ArmSbbrVersionMax) {
@@ -172,51 +182,30 @@ ArmSbbrReqsValidate (
   // Go through the list of mandatory tables for the input SBBR version
   for (Table = 0; Table < ArmSbbrReqs[Version].TableCount; Table++) {
     MandatoryTable = ArmSbbrReqs[Version].Tables[Table];
-    SignaturePtr = (CONST UINT8*)(UINTN)&MandatoryTable;
 
     // Locate the instance count for the table with the given signature
-    Index = 0;
-    while ((Index < ARRAY_SIZE (ArmSbbrTableCounts)) &&
-           (ArmSbbrTableCounts[Index].Signature != MandatoryTable)) {
-      Index++;
-    }
-
-    if (Index >= ARRAY_SIZE (ArmSbbrTableCounts)) {
-      IncrementErrorCount ();
-      Print (
-        L"\nERROR: SBBR v%a: Mandatory %c%c%c%c table's instance count not " \
-          L"found\n",
-        ArmSbbrVersions[Version],
-        SignaturePtr[0],
-        SignaturePtr[1],
-        SignaturePtr[2],
-        SignaturePtr[3]
-        );
-      return EFI_UNSUPPORTED;
+    for (Index = 0; Index < ARRAY_SIZE (ArmSbbrTableCounts); Index++) {
+      if (ArmSbbrTableCounts[Index].Signature == MandatoryTable) {
+        break;
+      }
     }
 
     if (ArmSbbrTableCounts[Index].Count == 0) {
       IsArmSbbrViolated = TRUE;
-      IncrementErrorCount ();
-      Print (
-        L"\nERROR: SBBR v%a: Mandatory %c%c%c%c table is missing",
+      AcpiError (
+        ACPI_ERROR_CROSS,
+        L"(%a) Mandatory %4a table is missing",
         ArmSbbrVersions[Version],
-        SignaturePtr[0],
-        SignaturePtr[1],
-        SignaturePtr[2],
-        SignaturePtr[3]
-        );
+        MandatoryTable);
     }
   }
 
   if (!IsArmSbbrViolated) {
-    Print (
-      L"\nINFO: SBBR v%a: All mandatory ACPI tables are installed",
-      ArmSbbrVersions[Version]
-      );
+    AcpiLog (
+      ACPI_GOOD,
+      L"(%a): Mandatory ACPI tables present",
+      ArmSbbrVersions[Version]);
   }
 
-  Print (L"\n");
-
   return IsArmSbbrViolated ? EFI_NOT_FOUND : EFI_SUCCESS;
 }
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
index dd69ed6992ba..933ad92312e1 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
@@ -10,6 +10,7 @@
 
 #include <IndustryStandard/DebugPort2Table.h>
 #include <Library/UefiLib.h>
+#include <Library/BaseLib.h>
 #include "AcpiParser.h"
 #include "AcpiTableParser.h"
 #include "AcpiViewLog.h"
@@ -42,17 +43,10 @@ ValidateNameSpaceStrLen (
   IN VOID*  Context
   )
 {
-  UINT16 NameSpaceStrLen;
+  UINT16 NameSpaceStrLen = *(UINT16 *) Ptr;
 
-  NameSpaceStrLen = *(UINT16*)Ptr;
-
-  if (NameSpaceStrLen < 2) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: NamespaceString Length = %d. If no Namespace device exists, " \
-        L"NamespaceString[] must contain a period '.'",
-      NameSpaceStrLen
-      );
+  if (AssertConstraint (L"ACPI", NameSpaceStrLen > 1)) {
+    AcpiInfo (L"With no namespace, NamespaceString[] must be a period '.'");
   }
 }
 
@@ -133,76 +127,51 @@ DumpDbgDeviceInfo (
       (OEMDataOffset == NULL)         ||
       (BaseAddrRegOffset == NULL)     ||
       (AddrSizeOffset == NULL)) {
-    IncrementErrorCount ();
-    Print (
-      L"ERROR: Insufficient Debug Device Information Structure length. " \
-        L"Length = %d.\n",
-      Length
-      );
+    AcpiError (ACPI_ERROR_PARSE, L"Failed to parse DbgDevInfo Structure");
     return;
   }
 
   // GAS
-  Index = 0;
   Offset = *BaseAddrRegOffset;
-  while ((Index++ < *GasCount) &&
-         (Offset < Length)) {
-    PrintFieldName (4, L"BaseAddressRegister");
-    Offset += (UINT16)DumpGasStruct (
-                        Ptr + Offset,
-                        4,
-                        Length - Offset
-                        );
+  for (Index = 0; Index < *GasCount; Index++) {
+    if (AssertMemberIntegrity (Offset, 1, Ptr, Length)) {
+      break;
+    }
+
+    PrintFieldName (4, L"BaseAddressRegister[%d]", Index);
+    Offset += (UINT16)DumpGasStruct (Ptr + Offset, 4, Length - Offset);
   }
 
   // Make sure the array of address sizes corresponding to each GAS fit in the
   // Debug Device Information structure
-  if ((*AddrSizeOffset + (*GasCount * sizeof (UINT32))) > Length) {
-    IncrementErrorCount ();
-    Print (
-      L"ERROR: Invalid GAS count. GasCount = %d. RemainingBufferLength = %d. " \
-        L"Parsing of the Debug Device Information structure aborted.\n",
-      *GasCount,
-      Length - *AddrSizeOffset
-      );
+  if (AssertMemberIntegrity (
+        *AddrSizeOffset, *GasCount * sizeof (UINT32), Ptr, Length)) {
     return;
   }
 
   // Address Size
   Index = 0;
   Offset = *AddrSizeOffset;
-  while ((Index++ < *GasCount) &&
-         (Offset < Length)) {
-    PrintFieldName (4, L"Address Size");
-    Print (L"0x%x\n", *((UINT32*)(Ptr + Offset)));
+  for (Index = 0; Index < *GasCount; Index++) {
+    if (AssertMemberIntegrity (Offset, 1, Ptr, Length)) {
+      break;
+    }
+    PrintFieldName (4, L"Address Size[%d]", Index);
+    AcpiInfo (L"0x%x", ReadUnaligned32 ((CONST UINT32 *)(Ptr + Offset)));
     Offset += sizeof (UINT32);
   }
 
   // NameSpace String
-  Index = 0;
-  Offset = *NameSpaceStringOffset;
   PrintFieldName (4, L"NameSpace String");
-  while ((Index++ < *NameSpaceStringLength) &&
-         (Offset < Length)) {
-    Print (L"%c", *(Ptr + Offset));
-    Offset++;
+  if (!AssertMemberIntegrity (
+        *NameSpaceStringOffset, *NameSpaceStringLength, Ptr, Length)) {
+    AcpiInfo (L"%-.*a", *NameSpaceStringLength - 1, Ptr + *NameSpaceStringOffset);
   }
-  Print (L"\n");
 
   // OEM Data
   if (*OEMDataOffset != 0) {
-    Index = 0;
-    Offset = *OEMDataOffset;
-    PrintFieldName (4, L"OEM Data");
-    while ((Index++ < *OEMDataLength) &&
-           (Offset < Length)) {
-      Print (L"%x ", *(Ptr + Offset));
-      if ((Index & 7) == 0) {
-        Print (L"\n%-*s   ", OUTPUT_FIELD_COLUMN_WIDTH, L"");
-      }
-      Offset++;
-    }
-    Print (L"\n");
+    AcpiInfo (L"OEM Data");
+    DumpRaw (Ptr + *OEMDataOffset, *OEMDataLength);
   }
 }
 
@@ -245,13 +214,8 @@ ParseAcpiDbg2 (
 
   // Check if the values used to control the parsing logic have been
   // successfully read.
-  if ((OffsetDbgDeviceInfo == NULL) ||
-      (NumberDbgDeviceInfo == NULL)) {
-    IncrementErrorCount ();
-    Print (
-      L"ERROR: Insufficient table length. AcpiTableLength = %d\n",
-      AcpiTableLength
-      );
+  if ((OffsetDbgDeviceInfo == NULL) || (NumberDbgDeviceInfo == NULL)) {
+    AcpiError (ACPI_ERROR_PARSE, L"Failed to parse DbgDevInfo array");
     return;
   }
 
@@ -259,7 +223,6 @@ ParseAcpiDbg2 (
   Index = 0;
 
   while (Index++ < *NumberDbgDeviceInfo) {
-
     // Parse the Debug Device Information Structure header to obtain Length
     ParseAcpi (
       FALSE,
@@ -273,31 +236,20 @@ ParseAcpiDbg2 (
     // Check if the values used to control the parsing logic have been
     // successfully read.
     if (DbgDevInfoLen == NULL) {
-      IncrementErrorCount ();
-      Print (
-        L"ERROR: Insufficient remaining table buffer length to read the " \
-          L"Debug Device Information structure's 'Length' field. " \
-          L"RemainingTableBufferLength = %d.\n",
-        AcpiTableLength - Offset
-        );
+      AcpiError (ACPI_ERROR_PARSE, L"Failed to parse DbgDevInfoLen");
       return;
     }
 
     // Validate Debug Device Information Structure length
-    if ((*DbgDevInfoLen == 0) ||
-        ((Offset + (*DbgDevInfoLen)) > AcpiTableLength)) {
-      IncrementErrorCount ();
-      Print (
-        L"ERROR: Invalid Debug Device Information Structure length. " \
-          L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
-        *DbgDevInfoLen,
-        Offset,
-        AcpiTableLength
-        );
+    if (AssertConstraint (L"ACPI", *DbgDevInfoLen > 0)) {
+      return;
+    }
+
+    if (AssertMemberIntegrity (Offset, *DbgDevInfoLen, Ptr, AcpiTableLength)) {
       return;
     }
 
-    DumpDbgDeviceInfo (Ptr + Offset, (*DbgDevInfoLen));
-    Offset += (*DbgDevInfoLen);
+    DumpDbgDeviceInfo (Ptr + Offset, *DbgDevInfoLen);
+    Offset += *DbgDevInfoLen;
   }
 }
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
index 4734864dfdcf..7349784ee067 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
@@ -69,12 +69,8 @@ ValidateFirmwareCtrl (
 )
 {
 #if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
-  if (*(UINT32*)Ptr != 0) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: Firmware Control must be zero for ARM platforms."
-    );
-  }
+  UINT32 FirmwareControl = *(UINT32 *) Ptr;
+  AssertConstraint (L"ARM", FirmwareControl == 0);
 #endif
 }
 
@@ -94,12 +90,8 @@ ValidateXFirmwareCtrl (
 )
 {
 #if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
-  if (*(UINT64*)Ptr != 0) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: X Firmware Control must be zero for ARM platforms."
-    );
-  }
+  UINT32 XFirmwareControl = *(UINT32 *) Ptr;
+  AssertConstraint (L"ARM", XFirmwareControl == 0);
 #endif
 }
 
@@ -119,12 +111,8 @@ ValidateFlags (
 )
 {
 #if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
-  if (((*(UINT32*)Ptr) & HW_REDUCED_ACPI) == 0) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: HW_REDUCED_ACPI flag must be set for ARM platforms."
-    );
-  }
+  UINT32 Flags = *(UINT32 *) Ptr;
+  AssertConstraint (L"ARM", Flags & HW_REDUCED_ACPI);
 #endif
 }
 
@@ -232,15 +220,13 @@ ParseAcpiFadt (
 
   if (Trace) {
     if (FadtMinorRevision != NULL) {
-      Print (L"\nSummary:\n");
+      AcpiInfo (L"Summary:");
       PrintFieldName (2, L"FADT Version");
-      Print (L"%d.%d\n",  *AcpiHdrInfo.Revision, *FadtMinorRevision);
+      AcpiInfo (L"%d.%d", *AcpiHdrInfo.Revision, *FadtMinorRevision);
     }
 
-    if (*GetAcpiXsdtHeaderInfo ()->OemTableId != *AcpiHdrInfo.OemTableId) {
-      IncrementErrorCount ();
-      Print (L"ERROR: OEM Table Id does not match with RSDT/XSDT.\n");
-    }
+    AssertConstraint (
+      L"ACPI", *GetAcpiXsdtHeaderInfo ()->OemTableId == *AcpiHdrInfo.OemTableId);
   }
 
   // If X_FIRMWARE_CTRL is not zero then use X_FIRMWARE_CTRL and ignore
@@ -257,9 +243,9 @@ ParseAcpiFadt (
     if ((Trace) &&
         (Flags != NULL) &&
         ((*Flags & EFI_ACPI_6_3_HW_REDUCED_ACPI) != EFI_ACPI_6_3_HW_REDUCED_ACPI)) {
-      IncrementErrorCount ();
-      Print (L"ERROR: No FACS table found, "
-               L"both X_FIRMWARE_CTRL and FIRMWARE_CTRL are zero.\n");
+      AcpiError (
+        ACPI_ERROR_CROSS,
+        L"No FACS table found, X_FIRMWARE_CTRL and FIRMWARE_CTRL are zero");
     }
   }
 
@@ -283,9 +269,7 @@ ParseAcpiFadt (
 
     Status = GetParser (FacsSignature, &FacsParserProc);
     if (EFI_ERROR (Status)) {
-      Print (
-        L"ERROR: No registered parser found for FACS.\n"
-        );
+      AcpiFatal (L"No registered parser found for FACS");
       return;
     }
 
@@ -309,8 +293,8 @@ ParseAcpiFadt (
       // The DSDT Table is mandatory for ARM systems
       // as the CPU information MUST be presented in
       // the DSDT.
-      IncrementErrorCount ();
-      Print (L"ERROR: Both X_DSDT and DSDT are invalid.\n");
+      AcpiError (
+        ACPI_ERROR_CROSS, L"(ARM) One of X_DSDT or DSDT must be valid!");
     }
 #endif
     return;
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
index d02fc4929d6f..c6b782152c63 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
@@ -13,6 +13,7 @@
 #include "AcpiParser.h"
 #include "AcpiTableParser.h"
 #include "AcpiViewConfig.h"
+#include "AcpiViewLog.h"
 
 // "The number of GT Block Timers must be less than or equal to 8"
 #define GT_BLOCK_TIMER_COUNT_MAX 8
@@ -41,18 +42,8 @@ ValidateGtBlockTimerCount (
   IN VOID*  Context
   )
 {
-  UINT32 BlockTimerCount;
-
-  BlockTimerCount = *(UINT32*)Ptr;
-
-  if (BlockTimerCount > GT_BLOCK_TIMER_COUNT_MAX) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: Timer Count = %d. Max Timer Count is %d.",
-      BlockTimerCount,
-      GT_BLOCK_TIMER_COUNT_MAX
-      );
-  }
+  UINT32 BlockTimerCount = *(UINT32*)Ptr;
+  AssertConstraint (L"ACPI", BlockTimerCount < GT_BLOCK_TIMER_COUNT_MAX);
 }
 
 /**
@@ -70,18 +61,8 @@ ValidateGtFrameNumber (
   IN VOID*  Context
   )
 {
-  UINT8 FrameNumber;
-
-  FrameNumber = *(UINT8*)Ptr;
-
-  if (FrameNumber >= GT_BLOCK_TIMER_COUNT_MAX) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: GT Frame Number = %d. GT Frame Number must be in range 0-%d.",
-      FrameNumber,
-      GT_BLOCK_TIMER_COUNT_MAX - 1
-      );
-  }
+  UINT8 GTFrameNumber = *Ptr;
+  AssertConstraint (L"ACPI", GTFrameNumber < GT_BLOCK_TIMER_COUNT_MAX);
 }
 
 /**
@@ -194,11 +175,7 @@ DumpGTBlock (
   // successfully read.
   if ((GtBlockTimerCount == NULL) ||
       (GtBlockTimerOffset == NULL)) {
-    IncrementErrorCount ();
-    Print (
-      L"ERROR: Insufficient GT Block Structure length. Length = %d.\n",
-      Length
-      );
+    AcpiError (ACPI_ERROR_PARSE, L"Failed to parse GT Block Structure");
     return;
   }
 
@@ -270,7 +247,6 @@ ParseAcpiGtdt (
 {
   UINT32 Index;
   UINT32 Offset;
-  UINT8* TimerPtr;
 
   if (!Trace) {
     return;
@@ -287,17 +263,11 @@ ParseAcpiGtdt (
 
   // Check if the values used to control the parsing logic have been
   // successfully read.
-  if ((GtdtPlatformTimerCount == NULL) ||
-      (GtdtPlatformTimerOffset == NULL)) {
-    IncrementErrorCount ();
-    Print (
-      L"ERROR: Insufficient table length. AcpiTableLength = %d.\n",
-      AcpiTableLength
-      );
+  if ((GtdtPlatformTimerCount == NULL) || (GtdtPlatformTimerOffset == NULL)) {
+    AcpiError (ACPI_ERROR_PARSE, L"Corrupt Platform Timer Table");
     return;
   }
 
-  TimerPtr = Ptr + *GtdtPlatformTimerOffset;
   Offset = *GtdtPlatformTimerOffset;
   Index = 0;
 
@@ -310,55 +280,35 @@ ParseAcpiGtdt (
       FALSE,
       0,
       NULL,
-      TimerPtr,
+      Ptr + Offset,
       AcpiTableLength - Offset,
       PARSER_PARAMS (GtPlatformTimerHeaderParser)
       );
 
     // Check if the values used to control the parsing logic have been
     // successfully read.
-    if ((PlatformTimerType == NULL) ||
-        (PlatformTimerLength == NULL)) {
-      IncrementErrorCount ();
-      Print (
-        L"ERROR: Insufficient remaining table buffer length to read the " \
-          L"Platform Timer Structure header. Length = %d.\n",
-        AcpiTableLength - Offset
-        );
+    if ((PlatformTimerType == NULL) || (PlatformTimerLength == NULL)) {
+      AcpiError (ACPI_ERROR_PARSE, L"Corrupt Platform Timer Structure");
       return;
     }
 
     // Validate Platform Timer Structure length
-    if ((*PlatformTimerLength == 0) ||
-        ((Offset + (*PlatformTimerLength)) > AcpiTableLength)) {
-      IncrementErrorCount ();
-      Print (
-        L"ERROR: Invalid Platform Timer Structure length. " \
-          L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
-        *PlatformTimerLength,
-        Offset,
-        AcpiTableLength
-        );
+    if (AssertMemberIntegrity(Offset, *PlatformTimerLength, Ptr, AcpiTableLength)) {
       return;
     }
 
     switch (*PlatformTimerType) {
       case EFI_ACPI_6_3_GTDT_GT_BLOCK:
-        DumpGTBlock (TimerPtr, *PlatformTimerLength);
+        DumpGTBlock (Ptr + Offset, *PlatformTimerLength);
         break;
       case EFI_ACPI_6_3_GTDT_SBSA_GENERIC_WATCHDOG:
-        DumpWatchdogTimer (TimerPtr, *PlatformTimerLength);
+        DumpWatchdogTimer (Ptr + Offset, *PlatformTimerLength);
         break;
       default:
-        IncrementErrorCount ();
-        Print (
-          L"ERROR: Invalid Platform Timer Type = %d\n",
-          *PlatformTimerType
-          );
-        break;
-    } // switch
+        AcpiError (
+          ACPI_ERROR_VALUE, L"Platform Timer Type %d", *PlatformTimerType);
+      }
 
-    TimerPtr += *PlatformTimerLength;
     Offset += *PlatformTimerLength;
   } // while
 }
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
index 356f355939aa..96227853c6ca 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
@@ -11,6 +11,7 @@
 #include <IndustryStandard/IoRemappingTable.h>
 #include <Library/PrintLib.h>
 #include <Library/UefiLib.h>
+#include <Library/BaseLib.h>
 #include "AcpiParser.h"
 #include "AcpiTableParser.h"
 #include "AcpiViewConfig.h"
@@ -49,10 +50,8 @@ ValidateItsIdMappingCount (
   IN VOID*  Context
   )
 {
-  if (*(UINT32*)Ptr != 0) {
-    IncrementErrorCount ();
-    Print (L"\nERROR: IORT ID Mapping count must be zero.");
-  }
+  UINT32 ItsNodeIdMapping = *(UINT32 *) Ptr;
+  AssertConstraint (L"ACPI", ItsNodeIdMapping == 0);
 }
 
 /**
@@ -71,10 +70,8 @@ ValidatePmcgIdMappingCount (
   IN VOID*  Context
   )
 {
-  if (*(UINT32*)Ptr > 1) {
-    IncrementErrorCount ();
-    Print (L"\nERROR: IORT ID Mapping count must not be greater than 1.");
-  }
+  UINT32 PmcgNodeIdMapping = *(UINT32 *) Ptr;
+  AssertConstraint (L"ACPI", PmcgNodeIdMapping <= 1);
 }
 
 /**
@@ -92,10 +89,8 @@ ValidateItsIdArrayReference (
   IN VOID*  Context
   )
 {
-  if (*(UINT32*)Ptr != 0) {
-    IncrementErrorCount ();
-    Print (L"\nERROR: IORT ID Mapping offset must be zero.");
-  }
+  UINT32 ItsNodeMappingArrayOffset = *(UINT32 *) Ptr;
+  AssertConstraint (L"ACPI", ItsNodeMappingArrayOffset == 0);
 }
 
 /**
@@ -268,28 +263,21 @@ DumpIortNodeIdMappings (
 {
   UINT32 Index;
   UINT32 Offset;
-  CHAR8  Buffer[40];  // Used for AsciiName param of ParseAcpi
 
-  Index = 0;
   Offset = 0;
+  for (Index = 0; Index < MappingCount; Index++) {
+    if (AssertMemberIntegrity(Offset, 1, Ptr, Length)) {
+      return;
+    }
 
-  while ((Index < MappingCount) &&
-         (Offset < Length)) {
-    AsciiSPrint (
-      Buffer,
-      sizeof (Buffer),
-      "ID Mapping [%d]",
-      Index
-      );
+    AcpiLog (ACPI_ITEM, L"    ID Mapping[%d] (+0x%x)", Index, Offset);
     Offset += ParseAcpi (
-                TRUE,
-                4,
-                Buffer,
-                Ptr + Offset,
-                Length - Offset,
-                PARSER_PARAMS (IortNodeIdMappingParser)
-                );
-    Index++;
+      TRUE,
+      4,
+      NULL,
+      Ptr + Offset,
+      Length - Offset,
+      PARSER_PARAMS (IortNodeIdMappingParser));
   }
 }
 
@@ -313,7 +301,6 @@ DumpIortNodeSmmuV1V2 (
 {
   UINT32 Index;
   UINT32 Offset;
-  CHAR8  Buffer[50];  // Used for AsciiName param of ParseAcpi
 
   ParseAcpi (
     TRUE,
@@ -330,56 +317,41 @@ DumpIortNodeSmmuV1V2 (
       (InterruptContextOffset == NULL)  ||
       (PmuInterruptCount == NULL)       ||
       (PmuInterruptOffset == NULL)) {
-    IncrementErrorCount ();
-    Print (
-      L"ERROR: Insufficient SMMUv1/2 node length. Length = %d\n",
-      Length
-      );
+    AcpiError (ACPI_ERROR_PARSE, L"Failed to parse the SMMUv1/2 node");
     return;
   }
 
   Offset = *InterruptContextOffset;
-  Index = 0;
+  for (Index = 0; Index < *InterruptContextCount; Index++) {
+    if (AssertMemberIntegrity(Offset, 1, Ptr, Length)) {
+      break;
+    }
 
-  while ((Index < *InterruptContextCount) &&
-         (Offset < Length)) {
-    AsciiSPrint (
-      Buffer,
-      sizeof (Buffer),
-      "Context Interrupts Array [%d]",
-      Index
-      );
+    AcpiLog (
+      ACPI_ITEM, L"    Context Interrupts Array[%d] (+0x%x)", Index, Offset);
     Offset += ParseAcpi (
-                TRUE,
-                4,
-                Buffer,
-                Ptr + Offset,
-                Length - Offset,
-                PARSER_PARAMS (InterruptArrayParser)
-                );
-    Index++;
+      TRUE,
+      4,
+      NULL,
+      Ptr + Offset,
+      Length - Offset,
+      PARSER_PARAMS (InterruptArrayParser));
   }
 
   Offset = *PmuInterruptOffset;
-  Index = 0;
+  for(Index = 0; Index < *PmuInterruptCount; Index++) {
+    if (AssertMemberIntegrity(Offset, 1, Ptr, Length)){
+      break;
+    }
 
-  while ((Index < *PmuInterruptCount) &&
-         (Offset < Length)) {
-    AsciiSPrint (
-      Buffer,
-      sizeof (Buffer),
-      "PMU Interrupts Array [%d]",
-      Index
-      );
+    AcpiLog (ACPI_ITEM, L"    PMU Interrupts Array[%d] (+0x%x)", Index, Offset);
     Offset += ParseAcpi (
-                TRUE,
-                4,
-                Buffer,
-                Ptr + Offset,
-                Length - Offset,
-                PARSER_PARAMS (InterruptArrayParser)
-                );
-    Index++;
+      TRUE,
+      4,
+      NULL,
+      Ptr + Offset,
+      Length - Offset,
+      PARSER_PARAMS (InterruptArrayParser));
   }
 
   DumpIortNodeIdMappings (
@@ -438,7 +410,6 @@ DumpIortNodeIts (
 {
   UINT32 Offset;
   UINT32 Index;
-  CHAR8  Buffer[80];  // Used for AsciiName param of ParseAcpi
 
   Offset = ParseAcpi (
             TRUE,
@@ -452,32 +423,26 @@ DumpIortNodeIts (
   // Check if the values used to control the parsing logic have been
   // successfully read.
   if (ItsCount == NULL) {
-    IncrementErrorCount ();
-    Print (
-      L"ERROR: Insufficient ITS group length. Length = %d.\n",
-      Length
-      );
+    AcpiError (ACPI_ERROR_PARSE, L"Failed to parse ITS node");
     return;
   }
 
   Index = 0;
 
-  while ((Index < *ItsCount) &&
-         (Offset < Length)) {
-    AsciiSPrint (
-      Buffer,
-      sizeof (Buffer),
-      "GIC ITS Identifier Array [%d]",
-      Index
-      );
+  for (Index = 0; Index < *ItsCount; Index++) {
+    if (AssertMemberIntegrity(Offset, 1, Ptr, Length)) {
+      return;
+    }
+
+    AcpiLog (
+      ACPI_ITEM, L"    GIC ITS Identifier Array[%d] (+0x%x)", Index, Offset);
     Offset += ParseAcpi (
-                TRUE,
-                4,
-                Buffer,
-                Ptr + Offset,
-                Length - Offset,
-                PARSER_PARAMS (ItsIdParser)
-                );
+      TRUE,
+      4,
+      NULL,
+      Ptr + Offset,
+      Length - Offset,
+      PARSER_PARAMS (ItsIdParser));
     Index++;
   }
 
@@ -516,13 +481,10 @@ DumpIortNodeNamedComponent (
 
   // Estimate the Device Name length
   PrintFieldName (2, L"Device Object Name");
-
-  while ((*(Ptr + Offset) != 0) &&
-         (Offset < Length)) {
-    Print (L"%c", *(Ptr + Offset));
-    Offset++;
-  }
-  Print (L"\n");
+  AcpiInfo (
+    L"%.*a",
+    AsciiStrnLenS ((CONST CHAR8 *)Ptr + Offset, Length - Offset),
+    Ptr + Offset);
 
   DumpIortNodeIdMappings (
     Ptr + MappingOffset,
@@ -629,7 +591,6 @@ ParseAcpiIort (
 {
   UINT32 Offset;
   UINT32 Index;
-  UINT8* NodePtr;
 
   if (!Trace) {
     return;
@@ -648,16 +609,11 @@ ParseAcpiIort (
   // successfully read.
   if ((IortNodeCount == NULL) ||
       (IortNodeOffset == NULL)) {
-    IncrementErrorCount ();
-    Print (
-      L"ERROR: Insufficient table length. AcpiTableLength = %d.\n",
-      AcpiTableLength
-      );
+    AcpiError (ACPI_ERROR_PARSE, L"Failed to parse IORT Node.");
     return;
   }
 
   Offset = *IortNodeOffset;
-  NodePtr = Ptr + Offset;
   Index = 0;
 
   // Parse the specified number of IORT nodes or the IORT table buffer length.
@@ -669,7 +625,7 @@ ParseAcpiIort (
       FALSE,
       0,
       "IORT Node Header",
-      NodePtr,
+      Ptr + Offset,
       AcpiTableLength - Offset,
       PARSER_PARAMS (IortNodeHeaderParser)
       );
@@ -680,42 +636,28 @@ ParseAcpiIort (
         (IortNodeLength == NULL)      ||
         (IortIdMappingCount == NULL)  ||
         (IortIdMappingOffset == NULL)) {
-      IncrementErrorCount ();
-      Print (
-        L"ERROR: Insufficient remaining table buffer length to read the " \
-          L"IORT node header. Length = %d.\n",
-        AcpiTableLength - Offset
-        );
+      AcpiError (ACPI_ERROR_PARSE, L"Failed ot parse the IORT node header");
       return;
     }
 
-    // Validate IORT Node length
-    if ((*IortNodeLength == 0) ||
-        ((Offset + (*IortNodeLength)) > AcpiTableLength)) {
-      IncrementErrorCount ();
-      Print (
-        L"ERROR: Invalid IORT Node length. " \
-          L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
-        *IortNodeLength,
-        Offset,
-        AcpiTableLength
-        );
+    // Protect against buffer overrun
+    if (AssertMemberIntegrity (Offset, *IortNodeLength, Ptr, AcpiTableLength)) {
       return;
     }
 
     PrintFieldName (2, L"* Node Offset *");
-    Print (L"0x%x\n", Offset);
+    AcpiInfo (L"0x%x", Offset);
 
     switch (*IortNodeType) {
       case EFI_ACPI_IORT_TYPE_ITS_GROUP:
         DumpIortNodeIts (
-          NodePtr,
+          Ptr + Offset,
           *IortNodeLength
           );
         break;
       case EFI_ACPI_IORT_TYPE_NAMED_COMP:
         DumpIortNodeNamedComponent (
-          NodePtr,
+          Ptr + Offset,
           *IortNodeLength,
           *IortIdMappingCount,
           *IortIdMappingOffset
@@ -723,7 +665,7 @@ ParseAcpiIort (
         break;
       case EFI_ACPI_IORT_TYPE_ROOT_COMPLEX:
         DumpIortNodeRootComplex (
-          NodePtr,
+          Ptr + Offset,
           *IortNodeLength,
           *IortIdMappingCount,
           *IortIdMappingOffset
@@ -731,7 +673,7 @@ ParseAcpiIort (
         break;
       case EFI_ACPI_IORT_TYPE_SMMUv1v2:
         DumpIortNodeSmmuV1V2 (
-          NodePtr,
+          Ptr + Offset,
           *IortNodeLength,
           *IortIdMappingCount,
           *IortIdMappingOffset
@@ -739,7 +681,7 @@ ParseAcpiIort (
         break;
       case EFI_ACPI_IORT_TYPE_SMMUv3:
         DumpIortNodeSmmuV3 (
-          NodePtr,
+          Ptr + Offset,
           *IortNodeLength,
           *IortIdMappingCount,
           *IortIdMappingOffset
@@ -747,7 +689,7 @@ ParseAcpiIort (
         break;
       case EFI_ACPI_IORT_TYPE_PMCG:
         DumpIortNodePmcg (
-          NodePtr,
+          Ptr + Offset,
           *IortNodeLength,
           *IortIdMappingCount,
           *IortIdMappingOffset
@@ -755,11 +697,10 @@ ParseAcpiIort (
         break;
 
       default:
-        IncrementErrorCount ();
-        Print (L"ERROR: Unsupported IORT Node type = %d\n", *IortNodeType);
+        AcpiError (
+          ACPI_ERROR_VALUE, L"Unsupported IORT Node type = %d", *IortNodeType);
     } // switch
 
-    NodePtr += (*IortNodeLength);
     Offset += (*IortNodeLength);
   } // while
 }
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
index 15aa2392b60c..67bbf369ee1a 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
@@ -17,6 +17,7 @@
 #include "AcpiTableParser.h"
 #include "AcpiViewConfig.h"
 #include "MadtParser.h"
+#include "AcpiViewLog.h"
 
 // Local Variables
 STATIC CONST UINT8* MadtInterruptControllerType;
@@ -38,12 +39,8 @@ ValidateGICDSystemVectorBase (
   IN VOID*  Context
 )
 {
-  if (*(UINT32*)Ptr != 0) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: System Vector Base must be zero."
-    );
-  }
+  UINT32 GicdSystemVectorBase = *(UINT32 *) Ptr;
+  AssertConstraint (L"ACPI", GicdSystemVectorBase == 0);
 }
 
 /**
@@ -63,36 +60,20 @@ ValidateSpeOverflowInterrupt (
 {
   UINT16 SpeOverflowInterrupt;
 
-  SpeOverflowInterrupt = *(UINT16*)Ptr;
+  SpeOverflowInterrupt = *(UINT16 *) Ptr;
 
   // SPE not supported by this processor
   if (SpeOverflowInterrupt == 0) {
     return;
   }
 
-  if ((SpeOverflowInterrupt < ARM_PPI_ID_MIN) ||
-      ((SpeOverflowInterrupt > ARM_PPI_ID_MAX) &&
-       (SpeOverflowInterrupt < ARM_PPI_ID_EXTENDED_MIN)) ||
-      (SpeOverflowInterrupt > ARM_PPI_ID_EXTENDED_MAX)) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: SPE Overflow Interrupt ID of %d is not in the allowed PPI ID "
-        L"ranges of %d-%d or %d-%d (for GICv3.1 or later).",
-      SpeOverflowInterrupt,
-      ARM_PPI_ID_MIN,
-      ARM_PPI_ID_MAX,
-      ARM_PPI_ID_EXTENDED_MIN,
-      ARM_PPI_ID_EXTENDED_MAX
-    );
-  } else if (SpeOverflowInterrupt != ARM_PPI_ID_PMBIRQ) {
-    IncrementWarningCount();
-    Print (
-      L"\nWARNING: SPE Overflow Interrupt ID of %d is not compliant with SBSA "
-        L"Level 3 PPI ID assignment: %d.",
-      SpeOverflowInterrupt,
-      ARM_PPI_ID_PMBIRQ
-    );
-  }
+  AssertConstraint (L"ACPI", SpeOverflowInterrupt > ARM_PPI_ID_MIN);
+  AssertConstraint (
+    L"ACPI",
+    (SpeOverflowInterrupt < ARM_PPI_ID_MAX) ||
+      (SpeOverflowInterrupt > ARM_PPI_ID_EXTENDED_MIN));
+  AssertConstraint (L"ACPI", SpeOverflowInterrupt < ARM_PPI_ID_EXTENDED_MAX);
+  WarnConstraint (L"SBSA", SpeOverflowInterrupt == ARM_PPI_ID_PMBIRQ);
 }
 
 /**
@@ -231,7 +212,6 @@ ParseAcpiMadt (
   )
 {
   UINT32 Offset;
-  UINT8* InterruptContollerPtr;
   UINT32 GICDCount;
 
   GICDCount = 0;
@@ -248,7 +228,6 @@ ParseAcpiMadt (
              AcpiTableLength,
              PARSER_PARAMS (MadtParser)
              );
-  InterruptContollerPtr = Ptr + Offset;
 
   while (Offset < AcpiTableLength) {
     // Parse Interrupt Controller Structure to obtain Length.
@@ -256,7 +235,7 @@ ParseAcpiMadt (
       FALSE,
       0,
       NULL,
-      InterruptContollerPtr,
+      Ptr + Offset,
       AcpiTableLength - Offset,
       PARSER_PARAMS (MadtInterruptControllerHeaderParser)
       );
@@ -265,26 +244,14 @@ ParseAcpiMadt (
     // successfully read.
     if ((MadtInterruptControllerType == NULL) ||
         (MadtInterruptControllerLength == NULL)) {
-      IncrementErrorCount ();
-      Print (
-        L"ERROR: Insufficient remaining table buffer length to read the " \
-          L"Interrupt Controller Structure header. Length = %d.\n",
-        AcpiTableLength - Offset
-        );
+      AcpiError (
+        ACPI_ERROR_PARSE,
+        L"Failed to read the Interrupt Controller Structure header");
       return;
     }
 
-    // Validate Interrupt Controller Structure length
-    if ((*MadtInterruptControllerLength == 0) ||
-        ((Offset + (*MadtInterruptControllerLength)) > AcpiTableLength)) {
-      IncrementErrorCount ();
-      Print (
-        L"ERROR: Invalid Interrupt Controller Structure length. " \
-          L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
-        *MadtInterruptControllerLength,
-        Offset,
-        AcpiTableLength
-        );
+    if (AssertMemberIntegrity (
+          Offset, *MadtInterruptControllerLength, Ptr, AcpiTableLength)) {
       return;
     }
 
@@ -294,7 +261,7 @@ ParseAcpiMadt (
           TRUE,
           2,
           "GICC",
-          InterruptContollerPtr,
+          Ptr + Offset,
           *MadtInterruptControllerLength,
           PARSER_PARAMS (GicCParser)
           );
@@ -303,18 +270,16 @@ ParseAcpiMadt (
 
       case EFI_ACPI_6_3_GICD: {
         if (++GICDCount > 1) {
-          IncrementErrorCount ();
-          Print (
-            L"ERROR: Only one GICD must be present,"
-              L" GICDCount = %d\n",
-            GICDCount
-            );
+          AcpiError (
+            ACPI_ERROR_CROSS,
+            L"Only one GICD must be present (now %d)",
+            GICDCount);
         }
         ParseAcpi (
           TRUE,
           2,
           "GICD",
-          InterruptContollerPtr,
+          Ptr + Offset,
           *MadtInterruptControllerLength,
           PARSER_PARAMS (GicDParser)
           );
@@ -326,7 +291,7 @@ ParseAcpiMadt (
           TRUE,
           2,
           "GIC MSI Frame",
-          InterruptContollerPtr,
+          Ptr + Offset,
           *MadtInterruptControllerLength,
           PARSER_PARAMS (GicMSIFrameParser)
           );
@@ -338,7 +303,7 @@ ParseAcpiMadt (
           TRUE,
           2,
           "GICR",
-          InterruptContollerPtr,
+          Ptr + Offset,
           *MadtInterruptControllerLength,
           PARSER_PARAMS (GicRParser)
           );
@@ -350,7 +315,7 @@ ParseAcpiMadt (
           TRUE,
           2,
           "GIC ITS",
-          InterruptContollerPtr,
+          Ptr + Offset,
           *MadtInterruptControllerLength,
           PARSER_PARAMS (GicITSParser)
           );
@@ -358,17 +323,13 @@ ParseAcpiMadt (
       }
 
       default: {
-        IncrementErrorCount ();
-        Print (
-          L"ERROR: Unknown Interrupt Controller Structure,"
-            L" Type = %d, Length = %d\n",
-          *MadtInterruptControllerType,
-          *MadtInterruptControllerLength
-          );
+        AcpiError (
+          ACPI_ERROR_VALUE,
+          L"Interrupt Controller Structure Type = %d",
+          *MadtInterruptControllerType);
       }
     } // switch
 
-    InterruptContollerPtr += *MadtInterruptControllerLength;
     Offset += *MadtInterruptControllerLength;
   } // while
 }
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Mcfg/McfgParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Mcfg/McfgParser.c
index 9da4d60e8497..7a7eaa374acf 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Mcfg/McfgParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Mcfg/McfgParser.c
@@ -12,6 +12,7 @@
 #include <Library/UefiLib.h>
 #include "AcpiParser.h"
 #include "AcpiTableParser.h"
+#include "AcpiViewLog.h"
 
 // Local variables
 STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
@@ -57,8 +58,6 @@ ParseAcpiMcfg (
   )
 {
   UINT32 Offset;
-  UINT32 PciCfgOffset;
-  UINT8* PciCfgSpacePtr;
 
   if (!Trace) {
     return;
@@ -73,18 +72,14 @@ ParseAcpiMcfg (
              PARSER_PARAMS (McfgParser)
              );
 
-  PciCfgSpacePtr = Ptr + Offset;
-
   while (Offset < AcpiTableLength) {
-    PciCfgOffset = ParseAcpi (
+    Offset += ParseAcpi (
                      TRUE,
                      2,
                      "PCI Configuration Space",
-                     PciCfgSpacePtr,
+                     Ptr + Offset,
                      (AcpiTableLength - Offset),
                      PARSER_PARAMS (PciCfgSpaceBaseAddrParser)
                      );
-    PciCfgSpacePtr += PciCfgOffset;
-    Offset += PciCfgOffset;
   }
 }
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
index 97a5203efb5f..3afb4e103ea9 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
@@ -14,6 +14,7 @@
 #include "AcpiParser.h"
 #include "AcpiView.h"
 #include "AcpiViewConfig.h"
+#include "AcpiViewLog.h"
 #include "PpttParser.h"
 #include "AcpiViewLog.h"
 
@@ -39,38 +40,20 @@ ValidateCacheNumberOfSets (
   IN VOID*  Context
   )
 {
-  UINT32 NumberOfSets;
-  NumberOfSets = *(UINT32*)Ptr;
+  UINT32 CacheNumberOfSets = *(UINT32*) Ptr;
 
-  if (NumberOfSets == 0) {
-    IncrementErrorCount ();
-    Print (L"\nERROR: Cache number of sets must be greater than 0");
-    return;
-  }
+  AssertConstraint (L"ACPI", CacheNumberOfSets != 0);
 
 #if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
-  if (NumberOfSets > PPTT_ARM_CCIDX_CACHE_NUMBER_OF_SETS_MAX) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: When ARMv8.3-CCIDX is implemented the maximum cache number of "
-        L"sets must be less than or equal to %d",
-      PPTT_ARM_CCIDX_CACHE_NUMBER_OF_SETS_MAX
-      );
+  if (AssertConstraint (
+        L"ARMv8.3-CCIDX",
+        CacheNumberOfSets < PPTT_ARM_CCIDX_CACHE_NUMBER_OF_SETS_MAX)) {
     return;
   }
 
-  if (NumberOfSets > PPTT_ARM_CACHE_NUMBER_OF_SETS_MAX) {
-    IncrementWarningCount ();
-    Print (
-      L"\nWARNING: Without ARMv8.3-CCIDX, the maximum cache number of sets "
-        L"must be less than or equal to %d. Ignore this message if "
-        L"ARMv8.3-CCIDX is implemented",
-      PPTT_ARM_CACHE_NUMBER_OF_SETS_MAX
-      );
-    return;
-  }
+  WarnConstraint (
+    L"No-ARMv8.3-CCIDX", CacheNumberOfSets < PPTT_ARM_CACHE_NUMBER_OF_SETS_MAX);
 #endif
-
 }
 
 /**
@@ -89,14 +72,8 @@ ValidateCacheAssociativity (
   IN VOID*  Context
   )
 {
-  UINT8 Associativity;
-  Associativity = *(UINT8*)Ptr;
-
-  if (Associativity == 0) {
-    IncrementErrorCount ();
-    Print (L"\nERROR: Cache associativity must be greater than 0");
-    return;
-  }
+  UINT8 CacheAssociativity = *Ptr;
+  AssertConstraint (L"ACPI", CacheAssociativity != 0);
 }
 
 /**
@@ -120,25 +97,14 @@ ValidateCacheLineSize (
   //   LineSize, bits [2:0]
   //     (Log2(Number of bytes in cache line)) - 4.
 
-  UINT16 LineSize;
-  LineSize = *(UINT16*)Ptr;
-
-  if ((LineSize < PPTT_ARM_CACHE_LINE_SIZE_MIN) ||
-      (LineSize > PPTT_ARM_CACHE_LINE_SIZE_MAX)) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: The cache line size must be between %d and %d bytes"
-        L" on ARM Platforms.",
-      PPTT_ARM_CACHE_LINE_SIZE_MIN,
-      PPTT_ARM_CACHE_LINE_SIZE_MAX
-      );
-    return;
-  }
+  UINT16 CacheLineSize = *(UINT16 *) Ptr;
 
-  if ((LineSize & (LineSize - 1)) != 0) {
-    IncrementErrorCount ();
-    Print (L"\nERROR: The cache line size is not a power of 2.");
-  }
+  AssertConstraint (
+    L"ARM",
+    (CacheLineSize >= PPTT_ARM_CACHE_LINE_SIZE_MIN &&
+     CacheLineSize <= PPTT_ARM_CACHE_LINE_SIZE_MAX));
+
+  AssertConstraint (L"ARM", BitFieldCountOnes32 (CacheLineSize, 0, 15) == 1);
 #endif
 }
 
@@ -160,17 +126,8 @@ ValidateCacheAttributes (
   // Reference: Advanced Configuration and Power Interface (ACPI) Specification
   //            Version 6.2 Errata A, September 2017
   // Table 5-153: Cache Type Structure
-  UINT8 Attributes;
-  Attributes = *(UINT8*)Ptr;
-
-  if ((Attributes & 0xE0) != 0) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: Attributes bits [7:5] are reserved and must be zero.",
-      Attributes
-      );
-    return;
-  }
+  UINT8 Attributes = *(UINT8 *) Ptr;
+  AssertConstraint (L"ACPI", BitFieldCountOnes32 (Attributes, 5, 7) == 0);
 }
 
 /**
@@ -255,7 +212,6 @@ DumpProcessorHierarchyNodeStructure (
 {
   UINT32 Offset;
   UINT32 Index;
-  CHAR16 Buffer[OUTPUT_FIELD_COLUMN_WIDTH];
 
   Offset = ParseAcpi (
              TRUE,
@@ -268,48 +224,22 @@ DumpProcessorHierarchyNodeStructure (
 
   // Check if the values used to control the parsing logic have been
   // successfully read.
-  if (NumberOfPrivateResources == NULL) {
-    IncrementErrorCount ();
-    Print (
-      L"ERROR: Insufficient Processor Hierarchy Node length. Length = %d.\n",
-      Length
-      );
-    return;
-  }
-
-  // Make sure the Private Resource array lies inside this structure
-  if (Offset + (*NumberOfPrivateResources * sizeof (UINT32)) > Length) {
-    IncrementErrorCount ();
-    Print (
-      L"ERROR: Invalid Number of Private Resources. " \
-        L"PrivateResourceCount = %d. RemainingBufferLength = %d. " \
-        L"Parsing of this structure aborted.\n",
-      *NumberOfPrivateResources,
-      Length - Offset
-      );
+  if(NumberOfPrivateResources == NULL) {
+    AcpiError (ACPI_ERROR_PARSE, L"Failed to parse processor hierarchy");
     return;
   }
 
-  Index = 0;
-
   // Parse the specified number of private resource references or the Processor
   // Hierarchy Node length. Whichever is minimum.
-  while (Index < *NumberOfPrivateResources) {
-    UnicodeSPrint (
-      Buffer,
-      sizeof (Buffer),
-      L"Private resources [%d]",
-      Index
-      );
+  for (Index = 0; Index < *NumberOfPrivateResources; Index++) {
+    if (AssertMemberIntegrity (Offset, sizeof (UINT32), Ptr, Length)) {
+      return;
+    }
 
-    PrintFieldName (4, Buffer);
-    Print (
-      L"0x%x\n",
-      *((UINT32*)(Ptr + Offset))
-      );
+    PrintFieldName (4, L"Private resources [%d]", Index);
+    AcpiInfo (L"0x%x", *(UINT32 *) (Ptr + Offset));
 
     Offset += sizeof (UINT32);
-    Index++;
   }
 }
 
@@ -386,7 +316,6 @@ ParseAcpiPptt (
   )
 {
   UINT32 Offset;
-  UINT8* ProcessorTopologyStructurePtr;
 
   if (!Trace) {
     return;
@@ -401,15 +330,13 @@ ParseAcpiPptt (
              PARSER_PARAMS (PpttParser)
              );
 
-  ProcessorTopologyStructurePtr = Ptr + Offset;
-
   while (Offset < AcpiTableLength) {
     // Parse Processor Hierarchy Node Structure to obtain Type and Length.
     ParseAcpi (
       FALSE,
       0,
       NULL,
-      ProcessorTopologyStructurePtr,
+      Ptr + Offset,
       AcpiTableLength - Offset,
       PARSER_PARAMS (ProcessorTopologyStructureHeaderParser)
       );
@@ -418,62 +345,42 @@ ParseAcpiPptt (
     // successfully read.
     if ((ProcessorTopologyStructureType == NULL) ||
         (ProcessorTopologyStructureLength == NULL)) {
-      IncrementErrorCount ();
-      Print (
-        L"ERROR: Insufficient remaining table buffer length to read the " \
-          L"processor topology structure header. Length = %d.\n",
-        AcpiTableLength - Offset
-        );
+      AcpiError (ACPI_ERROR_PARSE, L"Failed to parse processor topology");
       return;
     }
 
     // Validate Processor Topology Structure length
-    if ((*ProcessorTopologyStructureLength == 0) ||
-        ((Offset + (*ProcessorTopologyStructureLength)) > AcpiTableLength)) {
-      IncrementErrorCount ();
-      Print (
-        L"ERROR: Invalid Processor Topology Structure length. " \
-          L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
-        *ProcessorTopologyStructureLength,
-        Offset,
-        AcpiTableLength
-        );
+    if (AssertMemberIntegrity (
+          Offset, *ProcessorTopologyStructureLength, Ptr, AcpiTableLength)) {
       return;
     }
 
     PrintFieldName (2, L"* Structure Offset *");
-    Print (L"0x%x\n", Offset);
+    AcpiInfo (L"0x%x", Offset);
 
     switch (*ProcessorTopologyStructureType) {
       case EFI_ACPI_6_2_PPTT_TYPE_PROCESSOR:
         DumpProcessorHierarchyNodeStructure (
-          ProcessorTopologyStructurePtr,
+          Ptr + Offset,
           *ProcessorTopologyStructureLength
           );
         break;
       case EFI_ACPI_6_2_PPTT_TYPE_CACHE:
         DumpCacheTypeStructure (
-          ProcessorTopologyStructurePtr,
+          Ptr + Offset,
           *ProcessorTopologyStructureLength
           );
         break;
       case EFI_ACPI_6_2_PPTT_TYPE_ID:
         DumpIDStructure (
-          ProcessorTopologyStructurePtr,
+          Ptr + Offset,
           *ProcessorTopologyStructureLength
           );
         break;
       default:
-        IncrementErrorCount ();
-        Print (
-          L"ERROR: Unknown processor topology structure:"
-            L" Type = %d, Length = %d\n",
-          *ProcessorTopologyStructureType,
-          *ProcessorTopologyStructureLength
-          );
+        AcpiError (ACPI_ERROR_VALUE, L"Unknown processor topology structure");
     }
 
-    ProcessorTopologyStructurePtr += *ProcessorTopologyStructureLength;
     Offset += *ProcessorTopologyStructureLength;
   } // while
 }
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c
index f4a8732a7db7..b71b59d86ee1 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c
@@ -11,6 +11,7 @@
 #include <Library/UefiLib.h>
 #include "AcpiParser.h"
 #include "AcpiTableParser.h"
+#include "AcpiViewLog.h"
 
 // Local Variables
 STATIC CONST UINT64* XsdtAddress;
@@ -36,17 +37,8 @@ ValidateRsdtAddress (
   // Root System Description Pointer (RSDP), ACPI ? 5.2.5.
   //   - Within the RSDP, the RsdtAddress field must be null (zero) and the
   //     XsdtAddresss MUST be a valid, non-null, 64-bit value.
-  UINT32 RsdtAddr;
-
-  RsdtAddr = *(UINT32*)Ptr;
-
-  if (RsdtAddr != 0) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: Rsdt Address = 0x%p. This must be NULL on ARM Platforms.",
-      RsdtAddr
-      );
-  }
+  UINT32 RsdtAddr = *(UINT32 *) Ptr;
+  AssertConstraint (L"ARM", RsdtAddr == 0);
 #endif
 }
 
@@ -71,17 +63,8 @@ ValidateXsdtAddress (
   // Root System Description Pointer (RSDP), ACPI ? 5.2.5.
   //   - Within the RSDP, the RsdtAddress field must be null (zero) and the
   //     XsdtAddresss MUST be a valid, non-null, 64-bit value.
-  UINT64 XsdtAddr;
-
-  XsdtAddr = *(UINT64*)Ptr;
-
-  if (XsdtAddr == 0) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: Xsdt Address = 0x%p. This must not be NULL on ARM Platforms.",
-      XsdtAddr
-      );
-  }
+  UINT64 XsdtAddr = *(UINT64 *) Ptr;
+  AssertConstraint (L"ARM", XsdtAddr != 0);
 #endif
 }
 
@@ -141,12 +124,7 @@ ParseAcpiRsdp (
   // Check if the values used to control the parsing logic have been
   // successfully read.
   if (XsdtAddress == NULL) {
-    IncrementErrorCount ();
-    Print (
-      L"ERROR: Insufficient table length. AcpiTableLength = %d." \
-        L"RSDP parsing aborted.\n",
-      AcpiTableLength
-      );
+    AcpiError (ACPI_ERROR_PARSE, L"Failed to parse the RSDP table");
     return;
   }
 
@@ -154,11 +132,11 @@ ParseAcpiRsdp (
   // and does not parse the RSDT table. Platforms provide the
   // RSDT to enable compatibility with ACPI 1.0 operating systems.
   // Therefore the RSDT should not be used on ARM platforms.
-  if ((*XsdtAddress) == 0) {
-    IncrementErrorCount ();
-    Print (L"ERROR: XSDT Pointer is not set. RSDP parsing aborted.\n");
+  if (*XsdtAddress == 0) {
+    AcpiError (
+      ACPI_ERROR_VALUE, L"XSDT Pointer is not set. RSDP parsing aborted.");
     return;
   }
 
-  ProcessAcpiTable ((UINT8*)(UINTN)(*XsdtAddress));
+  ProcessAcpiTable ((VOID *) *XsdtAddress);
 }
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
index cedfc8a71849..2cd051e72502 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
@@ -28,11 +28,6 @@ STATIC CONST ACPI_PARSER SlitParser[] = {
    (VOID**)&SlitSystemLocalityCount, NULL, NULL}
 };
 
-/**
-  Macro to get the value of a System Locality
-**/
-#define SLIT_ELEMENT(Ptr, i, j) *(Ptr + (i * LocalityCount) + j)
-
 /**
   This function parses the ACPI SLIT table.
   When trace is enabled this function parses the SLIT table and
@@ -58,11 +53,11 @@ ParseAcpiSlit (
   )
 {
   UINT32 Offset;
-  UINT32 Count;
-  UINT32 Index;
+  UINT32 Index1;
+  UINT32 Index2;
   UINT32 LocalityCount;
-  UINT8* LocalityPtr;
-  CHAR16 Buffer[80];  // Used for AsciiName param of ParseAcpi
+  CHAR16 Buffer[256];
+  UINTN  StrLen;
 
   if (!Trace) {
     return;
@@ -80,11 +75,7 @@ ParseAcpiSlit (
   // Check if the values used to control the parsing logic have been
   // successfully read.
   if (SlitSystemLocalityCount == NULL) {
-    IncrementErrorCount ();
-    Print (
-      L"ERROR: Insufficient table length. AcpiTableLength = %d.\n",
-      AcpiTableLength
-      );
+    AcpiError (ACPI_ERROR_PARSE, L"Failed to parse the SLIT table");
     return;
   }
 
@@ -100,89 +91,64 @@ ParseAcpiSlit (
                 = 65535
                 = MAX_UINT16
   */
-  if (*SlitSystemLocalityCount > MAX_UINT16) {
-    IncrementErrorCount ();
-    Print (
-      L"ERROR: The Number of System Localities provided can't be represented " \
-        L"in the SLIT table. SlitSystemLocalityCount = %ld. " \
-        L"MaxLocalityCountAllowed = %d.\n",
-      *SlitSystemLocalityCount,
-      MAX_UINT16
-      );
+  if (AssertConstraint (L"ACPI", *SlitSystemLocalityCount <= MAX_UINT16)) {
     return;
   }
 
-  LocalityCount = (UINT32)*SlitSystemLocalityCount;
+  LocalityCount = (UINT32) *SlitSystemLocalityCount;
 
   // Make sure system localities fit in the table buffer provided
-  if (Offset + (LocalityCount * LocalityCount) > AcpiTableLength) {
-    IncrementErrorCount ();
-    Print (
-      L"ERROR: Invalid Number of System Localities. " \
-        L"SlitSystemLocalityCount = %ld. AcpiTableLength = %d.\n",
-      *SlitSystemLocalityCount,
-      AcpiTableLength
-      );
+  if (AssertMemberIntegrity (
+        Offset, (LocalityCount * LocalityCount), Ptr, AcpiTableLength)) {
     return;
   }
 
-  LocalityPtr = Ptr + Offset;
-
   // We only print the Localities if the count is less than 16
   // If the locality count is more than 16 then refer to the
   // raw data dump.
   if (LocalityCount < 16) {
-    UnicodeSPrint (
-      Buffer,
-      sizeof (Buffer),
-      L"Entry[0x%lx][0x%lx]",
-      LocalityCount,
-      LocalityCount
-      );
-    PrintFieldName (0, Buffer);
-    Print (L"\n");
-    Print (L"       ");
-    for (Index = 0; Index < LocalityCount; Index++) {
-      Print (L" (%3d) ", Index);
+    PrintFieldName (0, L"Entry[0x%lx][0x%lx]", LocalityCount, LocalityCount);
+    AcpiInfo (L"");
+    UnicodeSPrint (Buffer, sizeof (Buffer), L"       ");
+    for (Index1 = 0; Index1 < LocalityCount; Index1++) {
+      StrLen = StrnLenS (Buffer, sizeof (Buffer));
+      UnicodeSPrint (
+        Buffer + StrLen, sizeof (Buffer) - StrLen, L" (%3d) ", Index1);
     }
-    Print (L"\n");
-    for (Count = 0; Count< LocalityCount; Count++) {
-      Print (L" (%3d) ", Count);
-      for (Index = 0; Index < LocalityCount; Index++) {
-        Print (L"  %3d  ", SLIT_ELEMENT (LocalityPtr, Count, Index));
+    AcpiInfo (L"%s", Buffer);
+    for (Index1 = 0; Index1 < LocalityCount; Index1++) {
+      UnicodeSPrint (Buffer, sizeof (Buffer), L" (%3d) ", Index1);
+      for (Index2 = 0; Index2 < LocalityCount; Index2++) {
+        StrLen = StrnLenS (Buffer, sizeof (Buffer));
+        UnicodeSPrint (
+          Buffer + StrLen,
+          sizeof (Buffer) - StrLen,
+          L"  %3d  ",
+          *(Ptr + Offset + Index1 * LocalityCount + Index2));
       }
-      Print (L"\n");
+      AcpiInfo (L"%s", Buffer);
     }
   }
 
   // Validate
-  for (Count = 0; Count < LocalityCount; Count++) {
-    for (Index = 0; Index < LocalityCount; Index++) {
-      // Element[x][x] must be equal to 10
-      if ((Count == Index) && (SLIT_ELEMENT (LocalityPtr, Count,Index) != 10)) {
-        IncrementErrorCount ();
-        Print (
-          L"ERROR: Diagonal Element[0x%lx][0x%lx] (%3d)."
-            L" Normalized Value is not 10\n",
-          Count,
-          Index,
-          SLIT_ELEMENT (LocalityPtr, Count, Index)
-          );
-      }
+  for (Index1 = 0; Index1 < LocalityCount; Index1++) {
+    // Element[x][x] must be equal to 10
+    if (*(Ptr + Offset + Index1 * LocalityCount + Index2) != 10) {
+      AcpiError (
+        ACPI_ERROR_VALUE, L"SLIT Element[%d][%d] != 10", Index1, Index1);
+    }
+    for (Index2 = 0; Index2 < Index1; Index2++) {
       // Element[i][j] must be equal to Element[j][i]
-      if (SLIT_ELEMENT (LocalityPtr, Count, Index) !=
-          SLIT_ELEMENT (LocalityPtr, Index, Count)) {
-        IncrementErrorCount ();
-        Print (
-          L"ERROR: Relative distances for Element[0x%lx][0x%lx] (%3d) and \n"
-           L"Element[0x%lx][0x%lx] (%3d) do not match.\n",
-          Count,
-          Index,
-          SLIT_ELEMENT (LocalityPtr, Count, Index),
-          Index,
-          Count,
-          SLIT_ELEMENT (LocalityPtr, Index, Count)
-          );
+      if (
+        *(Ptr + Offset + Index1 * LocalityCount + Index2) !=
+        *(Ptr + Offset + Index2 * LocalityCount + Index1)) {
+        AcpiError (
+          ACPI_ERROR_VALUE,
+          L"SLIT Element[%d][%d] != SLIT Element[%d][%d]",
+          Index1,
+          Index2,
+          Index2,
+          Index1);
       }
     }
   }
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Spcr/SpcrParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Spcr/SpcrParser.c
index 3b06b05dee8c..bc3c12e720f2 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Spcr/SpcrParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Spcr/SpcrParser.c
@@ -14,6 +14,7 @@
 #include <Library/UefiLib.h>
 #include "AcpiParser.h"
 #include "AcpiTableParser.h"
+#include "AcpiViewLog.h"
 
 // Local variables
 STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
@@ -34,18 +35,11 @@ ValidateInterruptType (
   )
 {
 #if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
-  UINT8 InterruptType;
-
-  InterruptType = *Ptr;
-
-  if (InterruptType !=
-        EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERRUPT_TYPE_GIC) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: InterruptType = %d. This must be 8 on ARM Platforms",
-      InterruptType
-      );
-  }
+  UINT8 InterruptType = *Ptr;
+  AssertConstraint (
+    L"ARM",
+    InterruptType ==
+      EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERRUPT_TYPE_GIC);
 #endif
 }
 
@@ -65,17 +59,8 @@ ValidateIrq (
   )
 {
 #if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
-  UINT8 Irq;
-
-  Irq = *Ptr;
-
-  if (Irq != 0) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: Irq = %d. This must be zero on ARM Platforms\n",
-      Irq
-      );
-  }
+  UINT8 Irq = *Ptr;
+  AssertConstraint (L"ARM", Irq == 0);
 #endif
 }
 
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
index 568a0400bf07..eafc7e7942a3 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
@@ -37,10 +37,8 @@ ValidateSratReserved (
   IN VOID*  Context
   )
 {
-  if (*(UINT32*)Ptr != 1) {
-    IncrementErrorCount ();
-    Print (L"\nERROR: Reserved should be 1 for backward compatibility.\n");
-  }
+  UINT32 Reserved = *(UINT32 *) Ptr;
+  AssertConstraint (L"Backwards-Compatibility", Reserved == 1);
 }
 
 /**
@@ -59,18 +57,8 @@ ValidateSratDeviceHandleType (
   IN VOID*  Context
   )
 {
-  UINT8   DeviceHandleType;
-
-  DeviceHandleType = *Ptr;
-
-  if (DeviceHandleType > EFI_ACPI_6_3_PCI_DEVICE_HANDLE) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: Invalid Device Handle Type: %d. Must be between 0 and %d.",
-      DeviceHandleType,
-      EFI_ACPI_6_3_PCI_DEVICE_HANDLE
-      );
-  }
+  UINT8   DeviceHandleType = *Ptr;
+  AssertConstraint (L"ACPI", DeviceHandleType < EFI_ACPI_6_3_PCI_DEVICE_HANDLE);
 }
 
 /**
@@ -87,9 +75,9 @@ DumpSratPciBdfNumber (
   IN UINT8*        Ptr
   )
 {
-  CHAR16 Buffer[OUTPUT_FIELD_COLUMN_WIDTH];
-
-  Print (L"\n");
+  UINT16 Bus;
+  UINT16 Device;
+  UINT16 Function;
 
   /*
     The PCI BDF Number subfields are printed in the order specified in the ACPI
@@ -102,43 +90,13 @@ DumpSratPciBdfNumber (
     +-----+------+------+
   */
 
+  Bus = BitFieldRead16(*(UINT16 *) Ptr, 0, 7);
+  Device = BitFieldRead16(*(UINT16 *) Ptr, 8, 10);
+  Function = BitFieldRead16(*(UINT16 *) Ptr, 11, 15);
+
   // Print PCI Bus Number (Bits 7:0 of Byte 2)
-  UnicodeSPrint (
-    Buffer,
-    sizeof (Buffer),
-    L"PCI Bus Number"
-    );
-  PrintFieldName (4, Buffer);
-  Print (
-    L"0x%x\n",
-    *Ptr
-    );
-
-  Ptr++;
-
-  // Print PCI Device Number (Bits 7:3 of Byte 3)
-  UnicodeSPrint (
-    Buffer,
-    sizeof (Buffer),
-    L"PCI Device Number"
-    );
-  PrintFieldName (4, Buffer);
-  Print (
-    L"0x%x\n",
-    (*Ptr & (BIT7 | BIT6 | BIT5 | BIT4 | BIT3)) >> 3
-    );
-
-  // PCI Function Number (Bits 2:0 of Byte 3)
-  UnicodeSPrint (
-    Buffer,
-    sizeof (Buffer),
-    L"PCI Function Number"
-    );
-  PrintFieldName (4, Buffer);
-  Print (
-    L"0x%x\n",
-    *Ptr & (BIT2 | BIT1 | BIT0)
-    );
+  PrintFieldName (4, L"PCI Bus:Device.Function");
+  AcpiInfo (L"%4X:%2X.%d", Bus, Device, Function);
 }
 
 /**
@@ -176,8 +134,7 @@ DumpSratDeviceHandle (
  )
 {
   if (SratDeviceHandleType == NULL) {
-    IncrementErrorCount ();
-    Print (L"\nERROR: Device Handle Type read incorrectly.\n");
+    AcpiError (ACPI_ERROR_PARSE, L"Failed to parse Device Handle");
     return;
   }
 
@@ -222,7 +179,7 @@ DumpSratApicProximity (
 
   ProximityDomain = Ptr[0] | (Ptr[1] << 8) | (Ptr[2] << 16);
 
-  Print (Format, ProximityDomain);
+  AcpiInfo ((CHAR16 *)Format, ProximityDomain);
 }
 
 /**
@@ -360,7 +317,6 @@ ParseAcpiSrat (
   )
 {
   UINT32 Offset;
-  UINT8* ResourcePtr;
   UINT32 GicCAffinityIndex;
   UINT32 GicITSAffinityIndex;
   UINT32 GenericInitiatorAffinityIndex;
@@ -389,155 +345,113 @@ ParseAcpiSrat (
              PARSER_PARAMS (SratParser)
              );
 
-  ResourcePtr = Ptr + Offset;
-
   while (Offset < AcpiTableLength) {
     ParseAcpi (
       FALSE,
       0,
       NULL,
-      ResourcePtr,
+      Ptr + Offset,
       AcpiTableLength - Offset,
       PARSER_PARAMS (SratResourceAllocationParser)
       );
 
     // Check if the values used to control the parsing logic have been
     // successfully read.
-    if ((SratRAType == NULL) ||
-        (SratRALength == NULL)) {
-      IncrementErrorCount ();
-      Print (
-        L"ERROR: Insufficient remaining table buffer length to read the " \
-          L"Static Resource Allocation structure header. Length = %d.\n",
-        AcpiTableLength - Offset
-        );
+    if ((SratRAType == NULL) || (SratRALength == NULL)) {
+      AcpiError (ACPI_ERROR_PARSE, L"Failed to parse SRAT header");
       return;
     }
 
     // Validate Static Resource Allocation Structure length
-    if ((*SratRALength == 0) ||
-        ((Offset + (*SratRALength)) > AcpiTableLength)) {
-      IncrementErrorCount ();
-      Print (
-        L"ERROR: Invalid Static Resource Allocation Structure length. " \
-          L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
-        *SratRALength,
-        Offset,
-        AcpiTableLength
-        );
+    if (AssertMemberIntegrity(Offset, *SratRALength, Ptr, AcpiTableLength)) {
       return;
     }
 
     switch (*SratRAType) {
       case EFI_ACPI_6_3_GICC_AFFINITY:
-        AsciiSPrint (
-          Buffer,
-          sizeof (Buffer),
-          "GICC Affinity Structure [%d]",
-          GicCAffinityIndex++
-          );
+        AcpiLog (
+          ACPI_ITEM, L"GICC Affinity Structure [%d]", GicCAffinityIndex++);
         ParseAcpi (
           TRUE,
           2,
           Buffer,
-          ResourcePtr,
+          Ptr + Offset,
           *SratRALength,
-          PARSER_PARAMS (SratGicCAffinityParser)
-          );
+          PARSER_PARAMS (SratGicCAffinityParser));
         break;
 
       case EFI_ACPI_6_3_GIC_ITS_AFFINITY:
-        AsciiSPrint (
-          Buffer,
-          sizeof (Buffer),
-          "GIC ITS Affinity Structure [%d]",
-          GicITSAffinityIndex++
-        );
+        AcpiLog (
+          ACPI_ITEM, L"GIC ITS Affinity Structure [%d]", GicITSAffinityIndex++);
         ParseAcpi (
           TRUE,
           2,
           Buffer,
-          ResourcePtr,
+          Ptr + Offset,
           *SratRALength,
-          PARSER_PARAMS (SratGicITSAffinityParser)
-          );
+          PARSER_PARAMS (SratGicITSAffinityParser));
         break;
 
       case EFI_ACPI_6_3_GENERIC_INITIATOR_AFFINITY:
-        AsciiSPrint (
-          Buffer,
-          sizeof (Buffer),
-          "Generic Initiator Affinity Structure [%d]",
-          GenericInitiatorAffinityIndex++
-        );
+        AcpiLog (
+          ACPI_ITEM,
+          L"Generic Initiator Affinity Structure [%d]",
+          GenericInitiatorAffinityIndex++);
         ParseAcpi (
           TRUE,
           2,
           Buffer,
-          ResourcePtr,
+          Ptr + Offset,
           *SratRALength,
-          PARSER_PARAMS (SratGenericInitiatorAffinityParser)
-        );
+          PARSER_PARAMS (SratGenericInitiatorAffinityParser));
         break;
 
       case EFI_ACPI_6_3_MEMORY_AFFINITY:
-        AsciiSPrint (
-          Buffer,
-          sizeof (Buffer),
-          "Memory Affinity Structure [%d]",
-          MemoryAffinityIndex++
-          );
+        AcpiLog (
+          ACPI_ITEM, L"Memory Affinity Structure [%d]", MemoryAffinityIndex++);
         ParseAcpi (
           TRUE,
           2,
           Buffer,
-          ResourcePtr,
+          Ptr + Offset,
           *SratRALength,
-          PARSER_PARAMS (SratMemAffinityParser)
-          );
+          PARSER_PARAMS (SratMemAffinityParser));
         break;
 
       case EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC_SAPIC_AFFINITY:
-        AsciiSPrint (
-          Buffer,
-          sizeof (Buffer),
-          "APIC/SAPIC Affinity Structure [%d]",
-          ApicSapicAffinityIndex++
-          );
+        AcpiLog (
+          ACPI_ITEM,
+          L"APIC/SAPIC Affinity Structure [%d]",
+          ApicSapicAffinityIndex++);
         ParseAcpi (
           TRUE,
           2,
           Buffer,
-          ResourcePtr,
+          Ptr + Offset,
           *SratRALength,
-          PARSER_PARAMS (SratApciSapicAffinityParser)
-          );
+          PARSER_PARAMS (SratApciSapicAffinityParser));
         break;
 
       case EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_AFFINITY:
-        AsciiSPrint (
-          Buffer,
-          sizeof (Buffer),
-          "X2APIC Affinity Structure [%d]",
-          X2ApicAffinityIndex++
-          );
+        AcpiLog (
+          ACPI_ITEM, L"X2APIC Affinity Structure [%d]", X2ApicAffinityIndex++);
         ParseAcpi (
           TRUE,
           2,
           Buffer,
-          ResourcePtr,
+          Ptr + Offset,
           *SratRALength,
-          PARSER_PARAMS (SratX2ApciAffinityParser)
-          );
+          PARSER_PARAMS (SratX2ApciAffinityParser));
         break;
 
       default:
-        IncrementErrorCount ();
-        Print (L"ERROR: Unknown SRAT Affinity type = 0x%x\n", *SratRAType);
+        AcpiError (
+          ACPI_ERROR_VALUE,
+          L"Unknown SRAT Affinity type = 0x%x\n",
+          *SratRAType);
         break;
     }
 
-    ResourcePtr += (*SratRALength);
     Offset += (*SratRALength);
   }
 }
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Xsdt/XsdtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Xsdt/XsdtParser.c
index 771c4f322b8e..564e231a627a 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Xsdt/XsdtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Xsdt/XsdtParser.c
@@ -55,87 +55,43 @@ ParseAcpiXsdt (
   IN UINT8   AcpiTableRevision
   )
 {
-  UINT32        Offset;
   UINT32        TableOffset;
-  UINT64*       TablePointer;
+  UINT64**      TablePointer;
   UINTN         EntryIndex;
-  CHAR16        Buffer[32];
 
-  Offset = ParseAcpi (
-             Trace,
-             0,
-             "XSDT",
-             Ptr,
-             AcpiTableLength,
-             PARSER_PARAMS (XsdtParser)
-             );
-
-  TableOffset = Offset;
+  TableOffset = ParseAcpi (
+    Trace, 0, "XSDT", Ptr, AcpiTableLength, PARSER_PARAMS (XsdtParser));
 
+  EntryIndex = 0;
   if (Trace) {
-    EntryIndex = 0;
-    TablePointer = (UINT64*)(Ptr + TableOffset);
-    while (Offset < AcpiTableLength) {
+    for (TablePointer = (UINT64 **)(Ptr + TableOffset);
+         (UINT8 *) TablePointer < Ptr + AcpiTableLength;
+         TablePointer++) {
+
       CONST UINT32* Signature;
       CONST UINT32* Length;
       CONST UINT8*  Revision;
 
-      if ((UINT64*)(UINTN)(*TablePointer) != NULL) {
-        UINT8*      SignaturePtr;
-
-        ParseAcpiHeader (
-          (UINT8*)(UINTN)(*TablePointer),
-          &Signature,
-          &Length,
-          &Revision
-          );
-
-        SignaturePtr = (UINT8*)Signature;
-
-        UnicodeSPrint (
-          Buffer,
-          sizeof (Buffer),
-          L"Entry[%d] - %c%c%c%c",
-          EntryIndex++,
-          SignaturePtr[0],
-          SignaturePtr[1],
-          SignaturePtr[2],
-          SignaturePtr[3]
-          );
+      if (*TablePointer != NULL) {
+        ParseAcpiHeader (*TablePointer, &Signature, &Length, &Revision);
+        PrintFieldName (2, L"Entry[%d] - %.4a", EntryIndex++, Signature);
+        AcpiInfo (L"0x%lx", *TablePointer);
       } else {
-        UnicodeSPrint (
-          Buffer,
-          sizeof (Buffer),
-          L"Entry[%d]",
-          EntryIndex++
-          );
-      }
-
-      PrintFieldName (2, Buffer);
-      Print (L"0x%lx\n", *TablePointer);
-
-      // Validate the table pointers are not NULL
-      if ((UINT64*)(UINTN)(*TablePointer) == NULL) {
-        IncrementErrorCount ();
-        Print (
-          L"ERROR: Invalid table entry at 0x%lx, table address is 0x%lx\n",
-          TablePointer,
-          *TablePointer
-          );
+        PrintFieldName (2, L"Entry[%d]", EntryIndex++);
+        AcpiInfo (L"NULL");
+        AcpiError (ACPI_ERROR_VALUE, L"Invalid table entry");
       }
-      Offset += sizeof (UINT64);
-      TablePointer++;
-    } // while
+    }
   }
 
   // Process the tables
-  Offset = TableOffset;
-  TablePointer = (UINT64*)(Ptr + TableOffset);
-  while (Offset < AcpiTableLength) {
-    if ((UINT64*)(UINTN)(*TablePointer) != NULL) {
-      ProcessAcpiTable ((UINT8*)(UINTN)(*TablePointer));
+  for (TablePointer = (UINT64 **)(Ptr + TableOffset);
+       (UINT8 *) TablePointer < Ptr + AcpiTableLength;
+       TablePointer++) {
+
+    if (*TablePointer != NULL) {
+      ProcessAcpiTable (*TablePointer);
     }
-    Offset += sizeof (UINT64);
-    TablePointer++;
-  } // while
+
+  }
 }
-- 
2.24.1.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61806): https://edk2.groups.io/g/devel/message/61806
Mute This Topic: https://groups.io/mt/75193744/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 8/8] ShellPkg/AcpiView: Refactor table parsers
Posted by Gao, Zhichao 5 years, 7 months ago 
It is highly unrecommended to initializes the local variable at its declaration like below. I didn't find the rule in the CCS 2.1 spec. But most of the edk2 code never do like this. There are serval places like below, I just point out one example.
UINT16 NameSpaceStrLen = *(UINT16 *) Ptr;

For function ParseAcpiRsdp:
ProcessAcpiTable ((VOID *) *XsdtAddress);
Causing a warning and build failure with IA32 arch. I think the correct statement should be:
ProcessAcpiTable ((VOID *)(UINTN)*XsdtAddress);

Thanks,
Zhichao

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Tomas Pilar
> (tpilar)
> Sent: Monday, June 29, 2020 11:20 PM
> To: devel@edk2.groups.io
> Cc: Sami.Mujawar@arm.com; nd@arm.com; Ni, Ray <ray.ni@intel.com>; Gao,
> Zhichao <zhichao.gao@intel.com>
> Subject: [edk2-devel] [PATCH 8/8] ShellPkg/AcpiView: Refactor table parsers
> 
> The tests for checking specific constraints and checking
> for buffer overflows have been simplified to use a standard
> set of templates defined in the logging facility.
> 
> This regularises some of the error handling and makes
> it easier to write more tests like this in the future.
> 
> 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  |  25 ---
>  .../UefiShellAcpiViewCommandLib/AcpiParser.h  |  18 --
>  .../Arm/SbbrValidator.c                       |  65 +++---
>  .../Parsers/Dbg2/Dbg2Parser.c                 | 118 +++-------
>  .../Parsers/Fadt/FadtParser.c                 |  48 ++--
>  .../Parsers/Gtdt/GtdtParser.c                 |  84 ++-----
>  .../Parsers/Iort/IortParser.c                 | 207 +++++++-----------
>  .../Parsers/Madt/MadtParser.c                 |  99 +++------
>  .../Parsers/Mcfg/McfgParser.c                 |  11 +-
>  .../Parsers/Pptt/PpttParser.c                 | 165 +++-----------
>  .../Parsers/Rsdp/RsdpParser.c                 |  42 +---
>  .../Parsers/Slit/SlitParser.c                 | 122 ++++-------
>  .../Parsers/Spcr/SpcrParser.c                 |  31 +--
>  .../Parsers/Srat/SratParser.c                 | 188 +++++-----------
>  .../Parsers/Xsdt/XsdtParser.c                 |  92 ++------
>  15 files changed, 375 insertions(+), 940 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> index 9bbf724dfdfe..58599442d210 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> @@ -24,31 +24,6 @@ STATIC CONST ACPI_PARSER AcpiHeaderParser[] = {
>    PARSE_ACPI_HEADER (&AcpiHdrInfo)
>  };
> 
> -/**
> -  This function increments the ACPI table error counter.
> -**/
> -VOID
> -EFIAPI
> -IncrementErrorCount (
> -  VOID
> -  )
> -{
> -  mTableErrorCount++;
> -}
> -
> -/**
> -  This function increments the ACPI table warning counter.
> -**/
> -VOID
> -EFIAPI
> -IncrementWarningCount (
> -  VOID
> -  )
> -{
> -  mTableWarningCount++;
> -}
> -
> -
>  /**
>    This function verifies the ACPI table checksum.
> 
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> index bd3cdb774fb5..cdae433fef3b 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> @@ -18,24 +18,6 @@
>  /// that allows us to process the log options.
>  #define RSDP_TABLE_INFO  SIGNATURE_32('R', 'S', 'D', 'P')
> 
> -/**
> -  This function increments the ACPI table error counter.
> -**/
> -VOID
> -EFIAPI
> -IncrementErrorCount (
> -  VOID
> -  );
> -
> -/**
> -  This function increments the ACPI table warning counter.
> -**/
> -VOID
> -EFIAPI
> -IncrementWarningCount (
> -  VOID
> -  );
> -
>  /**
>    This function verifies the ACPI table checksum.
> 
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Arm/SbbrValidator.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Arm/SbbrValidator.c
> index d3284417fa5f..ba80a5ab3b40 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Arm/SbbrValidator.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Arm/SbbrValidator.c
> @@ -18,15 +18,16 @@
>  #include <Library/DebugLib.h>
>  #include <Library/UefiLib.h>
>  #include "AcpiParser.h"
> +#include "AcpiViewLog.h"
>  #include "Arm/SbbrValidator.h"
> 
>  /**
>    SBBR specification version strings
>  **/
> -STATIC CONST CHAR8* ArmSbbrVersions[ArmSbbrVersionMax] = {
> -  "1.0",     // ArmSbbrVersion_1_0
> -  "1.1",     // ArmSbbrVersion_1_1
> -  "1.2"      // ArmSbbrVersion_1_2
> +STATIC CONST CHAR16* ArmSbbrVersions[ArmSbbrVersionMax] = {
> +  L"SBBR-v1.0",     // ArmSbbrVersion_1_0
> +  L"SBBR-v1.1",     // ArmSbbrVersion_1_1
> +  L"SBBR-v1.2"      // ArmSbbrVersion_1_2
>  };
> 
>  /**
> @@ -96,6 +97,16 @@ STATIC ACPI_TABLE_COUNTER ArmSbbrTableCounts[] = {
> 
> {EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGN
> ATURE, 0}
>  };
> 
> +STATIC_ASSERT (
> +  ARRAY_SIZE (ArmSbbr10Mandatory) <= ARRAY_SIZE (ArmSbbrTableCounts),
> +  "Incompatible mandatory array tables");
> +STATIC_ASSERT (
> +  ARRAY_SIZE (ArmSbbr11Mandatory) <= ARRAY_SIZE (ArmSbbrTableCounts),
> +  "Incompatible mandatory array tables");
> +STATIC_ASSERT (
> +  ARRAY_SIZE (ArmSbbr12Mandatory) <= ARRAY_SIZE (ArmSbbrTableCounts),
> +  "Incompatible mandatory array tables");
> +
>  /**
>    Reset the platform ACPI table instance count for all SBBR-mandatory tables.
>  **/
> @@ -160,7 +171,6 @@ ArmSbbrReqsValidate (
>    UINT32        Table;
>    UINT32        Index;
>    UINT32        MandatoryTable;
> -  CONST UINT8*  SignaturePtr;
>    BOOLEAN       IsArmSbbrViolated;
> 
>    if (Version >= ArmSbbrVersionMax) {
> @@ -172,51 +182,30 @@ ArmSbbrReqsValidate (
>    // Go through the list of mandatory tables for the input SBBR version
>    for (Table = 0; Table < ArmSbbrReqs[Version].TableCount; Table++) {
>      MandatoryTable = ArmSbbrReqs[Version].Tables[Table];
> -    SignaturePtr = (CONST UINT8*)(UINTN)&MandatoryTable;
> 
>      // Locate the instance count for the table with the given signature
> -    Index = 0;
> -    while ((Index < ARRAY_SIZE (ArmSbbrTableCounts)) &&
> -           (ArmSbbrTableCounts[Index].Signature != MandatoryTable)) {
> -      Index++;
> -    }
> -
> -    if (Index >= ARRAY_SIZE (ArmSbbrTableCounts)) {
> -      IncrementErrorCount ();
> -      Print (
> -        L"\nERROR: SBBR v%a: Mandatory %c%c%c%c table's instance count not " \
> -          L"found\n",
> -        ArmSbbrVersions[Version],
> -        SignaturePtr[0],
> -        SignaturePtr[1],
> -        SignaturePtr[2],
> -        SignaturePtr[3]
> -        );
> -      return EFI_UNSUPPORTED;
> +    for (Index = 0; Index < ARRAY_SIZE (ArmSbbrTableCounts); Index++) {
> +      if (ArmSbbrTableCounts[Index].Signature == MandatoryTable) {
> +        break;
> +      }
>      }
> 
>      if (ArmSbbrTableCounts[Index].Count == 0) {
>        IsArmSbbrViolated = TRUE;
> -      IncrementErrorCount ();
> -      Print (
> -        L"\nERROR: SBBR v%a: Mandatory %c%c%c%c table is missing",
> +      AcpiError (
> +        ACPI_ERROR_CROSS,
> +        L"(%a) Mandatory %4a table is missing",
>          ArmSbbrVersions[Version],
> -        SignaturePtr[0],
> -        SignaturePtr[1],
> -        SignaturePtr[2],
> -        SignaturePtr[3]
> -        );
> +        MandatoryTable);
>      }
>    }
> 
>    if (!IsArmSbbrViolated) {
> -    Print (
> -      L"\nINFO: SBBR v%a: All mandatory ACPI tables are installed",
> -      ArmSbbrVersions[Version]
> -      );
> +    AcpiLog (
> +      ACPI_GOOD,
> +      L"(%a): Mandatory ACPI tables present",
> +      ArmSbbrVersions[Version]);
>    }
> 
> -  Print (L"\n");
> -
>    return IsArmSbbrViolated ? EFI_NOT_FOUND : EFI_SUCCESS;
>  }
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
> index dd69ed6992ba..933ad92312e1 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
> @@ -10,6 +10,7 @@
> 
>  #include <IndustryStandard/DebugPort2Table.h>
>  #include <Library/UefiLib.h>
> +#include <Library/BaseLib.h>
>  #include "AcpiParser.h"
>  #include "AcpiTableParser.h"
>  #include "AcpiViewLog.h"
> @@ -42,17 +43,10 @@ ValidateNameSpaceStrLen (
>    IN VOID*  Context
>    )
>  {
> -  UINT16 NameSpaceStrLen;
> +  UINT16 NameSpaceStrLen = *(UINT16 *) Ptr;
> 
> -  NameSpaceStrLen = *(UINT16*)Ptr;
> -
> -  if (NameSpaceStrLen < 2) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"\nERROR: NamespaceString Length = %d. If no Namespace device exists, "
> \
> -        L"NamespaceString[] must contain a period '.'",
> -      NameSpaceStrLen
> -      );
> +  if (AssertConstraint (L"ACPI", NameSpaceStrLen > 1)) {
> +    AcpiInfo (L"With no namespace, NamespaceString[] must be a period '.'");
>    }
>  }
> 
> @@ -133,76 +127,51 @@ DumpDbgDeviceInfo (
>        (OEMDataOffset == NULL)         ||
>        (BaseAddrRegOffset == NULL)     ||
>        (AddrSizeOffset == NULL)) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"ERROR: Insufficient Debug Device Information Structure length. " \
> -        L"Length = %d.\n",
> -      Length
> -      );
> +    AcpiError (ACPI_ERROR_PARSE, L"Failed to parse DbgDevInfo Structure");
>      return;
>    }
> 
>    // GAS
> -  Index = 0;
>    Offset = *BaseAddrRegOffset;
> -  while ((Index++ < *GasCount) &&
> -         (Offset < Length)) {
> -    PrintFieldName (4, L"BaseAddressRegister");
> -    Offset += (UINT16)DumpGasStruct (
> -                        Ptr + Offset,
> -                        4,
> -                        Length - Offset
> -                        );
> +  for (Index = 0; Index < *GasCount; Index++) {
> +    if (AssertMemberIntegrity (Offset, 1, Ptr, Length)) {
> +      break;
> +    }
> +
> +    PrintFieldName (4, L"BaseAddressRegister[%d]", Index);
> +    Offset += (UINT16)DumpGasStruct (Ptr + Offset, 4, Length - Offset);
>    }
> 
>    // Make sure the array of address sizes corresponding to each GAS fit in the
>    // Debug Device Information structure
> -  if ((*AddrSizeOffset + (*GasCount * sizeof (UINT32))) > Length) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"ERROR: Invalid GAS count. GasCount = %d. RemainingBufferLength = %d. "
> \
> -        L"Parsing of the Debug Device Information structure aborted.\n",
> -      *GasCount,
> -      Length - *AddrSizeOffset
> -      );
> +  if (AssertMemberIntegrity (
> +        *AddrSizeOffset, *GasCount * sizeof (UINT32), Ptr, Length)) {
>      return;
>    }
> 
>    // Address Size
>    Index = 0;
>    Offset = *AddrSizeOffset;
> -  while ((Index++ < *GasCount) &&
> -         (Offset < Length)) {
> -    PrintFieldName (4, L"Address Size");
> -    Print (L"0x%x\n", *((UINT32*)(Ptr + Offset)));
> +  for (Index = 0; Index < *GasCount; Index++) {
> +    if (AssertMemberIntegrity (Offset, 1, Ptr, Length)) {
> +      break;
> +    }
> +    PrintFieldName (4, L"Address Size[%d]", Index);
> +    AcpiInfo (L"0x%x", ReadUnaligned32 ((CONST UINT32 *)(Ptr + Offset)));
>      Offset += sizeof (UINT32);
>    }
> 
>    // NameSpace String
> -  Index = 0;
> -  Offset = *NameSpaceStringOffset;
>    PrintFieldName (4, L"NameSpace String");
> -  while ((Index++ < *NameSpaceStringLength) &&
> -         (Offset < Length)) {
> -    Print (L"%c", *(Ptr + Offset));
> -    Offset++;
> +  if (!AssertMemberIntegrity (
> +        *NameSpaceStringOffset, *NameSpaceStringLength, Ptr, Length)) {
> +    AcpiInfo (L"%-.*a", *NameSpaceStringLength - 1, Ptr +
> *NameSpaceStringOffset);
>    }
> -  Print (L"\n");
> 
>    // OEM Data
>    if (*OEMDataOffset != 0) {
> -    Index = 0;
> -    Offset = *OEMDataOffset;
> -    PrintFieldName (4, L"OEM Data");
> -    while ((Index++ < *OEMDataLength) &&
> -           (Offset < Length)) {
> -      Print (L"%x ", *(Ptr + Offset));
> -      if ((Index & 7) == 0) {
> -        Print (L"\n%-*s   ", OUTPUT_FIELD_COLUMN_WIDTH, L"");
> -      }
> -      Offset++;
> -    }
> -    Print (L"\n");
> +    AcpiInfo (L"OEM Data");
> +    DumpRaw (Ptr + *OEMDataOffset, *OEMDataLength);
>    }
>  }
> 
> @@ -245,13 +214,8 @@ ParseAcpiDbg2 (
> 
>    // Check if the values used to control the parsing logic have been
>    // successfully read.
> -  if ((OffsetDbgDeviceInfo == NULL) ||
> -      (NumberDbgDeviceInfo == NULL)) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"ERROR: Insufficient table length. AcpiTableLength = %d\n",
> -      AcpiTableLength
> -      );
> +  if ((OffsetDbgDeviceInfo == NULL) || (NumberDbgDeviceInfo == NULL)) {
> +    AcpiError (ACPI_ERROR_PARSE, L"Failed to parse DbgDevInfo array");
>      return;
>    }
> 
> @@ -259,7 +223,6 @@ ParseAcpiDbg2 (
>    Index = 0;
> 
>    while (Index++ < *NumberDbgDeviceInfo) {
> -
>      // Parse the Debug Device Information Structure header to obtain Length
>      ParseAcpi (
>        FALSE,
> @@ -273,31 +236,20 @@ ParseAcpiDbg2 (
>      // Check if the values used to control the parsing logic have been
>      // successfully read.
>      if (DbgDevInfoLen == NULL) {
> -      IncrementErrorCount ();
> -      Print (
> -        L"ERROR: Insufficient remaining table buffer length to read the " \
> -          L"Debug Device Information structure's 'Length' field. " \
> -          L"RemainingTableBufferLength = %d.\n",
> -        AcpiTableLength - Offset
> -        );
> +      AcpiError (ACPI_ERROR_PARSE, L"Failed to parse DbgDevInfoLen");
>        return;
>      }
> 
>      // Validate Debug Device Information Structure length
> -    if ((*DbgDevInfoLen == 0) ||
> -        ((Offset + (*DbgDevInfoLen)) > AcpiTableLength)) {
> -      IncrementErrorCount ();
> -      Print (
> -        L"ERROR: Invalid Debug Device Information Structure length. " \
> -          L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
> -        *DbgDevInfoLen,
> -        Offset,
> -        AcpiTableLength
> -        );
> +    if (AssertConstraint (L"ACPI", *DbgDevInfoLen > 0)) {
> +      return;
> +    }
> +
> +    if (AssertMemberIntegrity (Offset, *DbgDevInfoLen, Ptr, AcpiTableLength)) {
>        return;
>      }
> 
> -    DumpDbgDeviceInfo (Ptr + Offset, (*DbgDevInfoLen));
> -    Offset += (*DbgDevInfoLen);
> +    DumpDbgDeviceInfo (Ptr + Offset, *DbgDevInfoLen);
> +    Offset += *DbgDevInfoLen;
>    }
>  }
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
> index 4734864dfdcf..7349784ee067 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
> @@ -69,12 +69,8 @@ ValidateFirmwareCtrl (
>  )
>  {
>  #if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
> -  if (*(UINT32*)Ptr != 0) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"\nERROR: Firmware Control must be zero for ARM platforms."
> -    );
> -  }
> +  UINT32 FirmwareControl = *(UINT32 *) Ptr;
> +  AssertConstraint (L"ARM", FirmwareControl == 0);
>  #endif
>  }
> 
> @@ -94,12 +90,8 @@ ValidateXFirmwareCtrl (
>  )
>  {
>  #if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
> -  if (*(UINT64*)Ptr != 0) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"\nERROR: X Firmware Control must be zero for ARM platforms."
> -    );
> -  }
> +  UINT32 XFirmwareControl = *(UINT32 *) Ptr;
> +  AssertConstraint (L"ARM", XFirmwareControl == 0);
>  #endif
>  }
> 
> @@ -119,12 +111,8 @@ ValidateFlags (
>  )
>  {
>  #if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
> -  if (((*(UINT32*)Ptr) & HW_REDUCED_ACPI) == 0) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"\nERROR: HW_REDUCED_ACPI flag must be set for ARM platforms."
> -    );
> -  }
> +  UINT32 Flags = *(UINT32 *) Ptr;
> +  AssertConstraint (L"ARM", Flags & HW_REDUCED_ACPI);
>  #endif
>  }
> 
> @@ -232,15 +220,13 @@ ParseAcpiFadt (
> 
>    if (Trace) {
>      if (FadtMinorRevision != NULL) {
> -      Print (L"\nSummary:\n");
> +      AcpiInfo (L"Summary:");
>        PrintFieldName (2, L"FADT Version");
> -      Print (L"%d.%d\n",  *AcpiHdrInfo.Revision, *FadtMinorRevision);
> +      AcpiInfo (L"%d.%d", *AcpiHdrInfo.Revision, *FadtMinorRevision);
>      }
> 
> -    if (*GetAcpiXsdtHeaderInfo ()->OemTableId != *AcpiHdrInfo.OemTableId) {
> -      IncrementErrorCount ();
> -      Print (L"ERROR: OEM Table Id does not match with RSDT/XSDT.\n");
> -    }
> +    AssertConstraint (
> +      L"ACPI", *GetAcpiXsdtHeaderInfo ()->OemTableId ==
> *AcpiHdrInfo.OemTableId);
>    }
> 
>    // If X_FIRMWARE_CTRL is not zero then use X_FIRMWARE_CTRL and ignore
> @@ -257,9 +243,9 @@ ParseAcpiFadt (
>      if ((Trace) &&
>          (Flags != NULL) &&
>          ((*Flags & EFI_ACPI_6_3_HW_REDUCED_ACPI) !=
> EFI_ACPI_6_3_HW_REDUCED_ACPI)) {
> -      IncrementErrorCount ();
> -      Print (L"ERROR: No FACS table found, "
> -               L"both X_FIRMWARE_CTRL and FIRMWARE_CTRL are zero.\n");
> +      AcpiError (
> +        ACPI_ERROR_CROSS,
> +        L"No FACS table found, X_FIRMWARE_CTRL and FIRMWARE_CTRL are
> zero");
>      }
>    }
> 
> @@ -283,9 +269,7 @@ ParseAcpiFadt (
> 
>      Status = GetParser (FacsSignature, &FacsParserProc);
>      if (EFI_ERROR (Status)) {
> -      Print (
> -        L"ERROR: No registered parser found for FACS.\n"
> -        );
> +      AcpiFatal (L"No registered parser found for FACS");
>        return;
>      }
> 
> @@ -309,8 +293,8 @@ ParseAcpiFadt (
>        // The DSDT Table is mandatory for ARM systems
>        // as the CPU information MUST be presented in
>        // the DSDT.
> -      IncrementErrorCount ();
> -      Print (L"ERROR: Both X_DSDT and DSDT are invalid.\n");
> +      AcpiError (
> +        ACPI_ERROR_CROSS, L"(ARM) One of X_DSDT or DSDT must be valid!");
>      }
>  #endif
>      return;
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> index d02fc4929d6f..c6b782152c63 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> @@ -13,6 +13,7 @@
>  #include "AcpiParser.h"
>  #include "AcpiTableParser.h"
>  #include "AcpiViewConfig.h"
> +#include "AcpiViewLog.h"
> 
>  // "The number of GT Block Timers must be less than or equal to 8"
>  #define GT_BLOCK_TIMER_COUNT_MAX 8
> @@ -41,18 +42,8 @@ ValidateGtBlockTimerCount (
>    IN VOID*  Context
>    )
>  {
> -  UINT32 BlockTimerCount;
> -
> -  BlockTimerCount = *(UINT32*)Ptr;
> -
> -  if (BlockTimerCount > GT_BLOCK_TIMER_COUNT_MAX) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"\nERROR: Timer Count = %d. Max Timer Count is %d.",
> -      BlockTimerCount,
> -      GT_BLOCK_TIMER_COUNT_MAX
> -      );
> -  }
> +  UINT32 BlockTimerCount = *(UINT32*)Ptr;
> +  AssertConstraint (L"ACPI", BlockTimerCount <
> GT_BLOCK_TIMER_COUNT_MAX);
>  }
> 
>  /**
> @@ -70,18 +61,8 @@ ValidateGtFrameNumber (
>    IN VOID*  Context
>    )
>  {
> -  UINT8 FrameNumber;
> -
> -  FrameNumber = *(UINT8*)Ptr;
> -
> -  if (FrameNumber >= GT_BLOCK_TIMER_COUNT_MAX) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"\nERROR: GT Frame Number = %d. GT Frame Number must be in range 0-
> %d.",
> -      FrameNumber,
> -      GT_BLOCK_TIMER_COUNT_MAX - 1
> -      );
> -  }
> +  UINT8 GTFrameNumber = *Ptr;
> +  AssertConstraint (L"ACPI", GTFrameNumber <
> GT_BLOCK_TIMER_COUNT_MAX);
>  }
> 
>  /**
> @@ -194,11 +175,7 @@ DumpGTBlock (
>    // successfully read.
>    if ((GtBlockTimerCount == NULL) ||
>        (GtBlockTimerOffset == NULL)) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"ERROR: Insufficient GT Block Structure length. Length = %d.\n",
> -      Length
> -      );
> +    AcpiError (ACPI_ERROR_PARSE, L"Failed to parse GT Block Structure");
>      return;
>    }
> 
> @@ -270,7 +247,6 @@ ParseAcpiGtdt (
>  {
>    UINT32 Index;
>    UINT32 Offset;
> -  UINT8* TimerPtr;
> 
>    if (!Trace) {
>      return;
> @@ -287,17 +263,11 @@ ParseAcpiGtdt (
> 
>    // Check if the values used to control the parsing logic have been
>    // successfully read.
> -  if ((GtdtPlatformTimerCount == NULL) ||
> -      (GtdtPlatformTimerOffset == NULL)) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"ERROR: Insufficient table length. AcpiTableLength = %d.\n",
> -      AcpiTableLength
> -      );
> +  if ((GtdtPlatformTimerCount == NULL) || (GtdtPlatformTimerOffset == NULL)) {
> +    AcpiError (ACPI_ERROR_PARSE, L"Corrupt Platform Timer Table");
>      return;
>    }
> 
> -  TimerPtr = Ptr + *GtdtPlatformTimerOffset;
>    Offset = *GtdtPlatformTimerOffset;
>    Index = 0;
> 
> @@ -310,55 +280,35 @@ ParseAcpiGtdt (
>        FALSE,
>        0,
>        NULL,
> -      TimerPtr,
> +      Ptr + Offset,
>        AcpiTableLength - Offset,
>        PARSER_PARAMS (GtPlatformTimerHeaderParser)
>        );
> 
>      // Check if the values used to control the parsing logic have been
>      // successfully read.
> -    if ((PlatformTimerType == NULL) ||
> -        (PlatformTimerLength == NULL)) {
> -      IncrementErrorCount ();
> -      Print (
> -        L"ERROR: Insufficient remaining table buffer length to read the " \
> -          L"Platform Timer Structure header. Length = %d.\n",
> -        AcpiTableLength - Offset
> -        );
> +    if ((PlatformTimerType == NULL) || (PlatformTimerLength == NULL)) {
> +      AcpiError (ACPI_ERROR_PARSE, L"Corrupt Platform Timer Structure");
>        return;
>      }
> 
>      // Validate Platform Timer Structure length
> -    if ((*PlatformTimerLength == 0) ||
> -        ((Offset + (*PlatformTimerLength)) > AcpiTableLength)) {
> -      IncrementErrorCount ();
> -      Print (
> -        L"ERROR: Invalid Platform Timer Structure length. " \
> -          L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
> -        *PlatformTimerLength,
> -        Offset,
> -        AcpiTableLength
> -        );
> +    if (AssertMemberIntegrity(Offset, *PlatformTimerLength, Ptr,
> AcpiTableLength)) {
>        return;
>      }
> 
>      switch (*PlatformTimerType) {
>        case EFI_ACPI_6_3_GTDT_GT_BLOCK:
> -        DumpGTBlock (TimerPtr, *PlatformTimerLength);
> +        DumpGTBlock (Ptr + Offset, *PlatformTimerLength);
>          break;
>        case EFI_ACPI_6_3_GTDT_SBSA_GENERIC_WATCHDOG:
> -        DumpWatchdogTimer (TimerPtr, *PlatformTimerLength);
> +        DumpWatchdogTimer (Ptr + Offset, *PlatformTimerLength);
>          break;
>        default:
> -        IncrementErrorCount ();
> -        Print (
> -          L"ERROR: Invalid Platform Timer Type = %d\n",
> -          *PlatformTimerType
> -          );
> -        break;
> -    } // switch
> +        AcpiError (
> +          ACPI_ERROR_VALUE, L"Platform Timer Type %d", *PlatformTimerType);
> +      }
> 
> -    TimerPtr += *PlatformTimerLength;
>      Offset += *PlatformTimerLength;
>    } // while
>  }
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
> index 356f355939aa..96227853c6ca 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
> @@ -11,6 +11,7 @@
>  #include <IndustryStandard/IoRemappingTable.h>
>  #include <Library/PrintLib.h>
>  #include <Library/UefiLib.h>
> +#include <Library/BaseLib.h>
>  #include "AcpiParser.h"
>  #include "AcpiTableParser.h"
>  #include "AcpiViewConfig.h"
> @@ -49,10 +50,8 @@ ValidateItsIdMappingCount (
>    IN VOID*  Context
>    )
>  {
> -  if (*(UINT32*)Ptr != 0) {
> -    IncrementErrorCount ();
> -    Print (L"\nERROR: IORT ID Mapping count must be zero.");
> -  }
> +  UINT32 ItsNodeIdMapping = *(UINT32 *) Ptr;
> +  AssertConstraint (L"ACPI", ItsNodeIdMapping == 0);
>  }
> 
>  /**
> @@ -71,10 +70,8 @@ ValidatePmcgIdMappingCount (
>    IN VOID*  Context
>    )
>  {
> -  if (*(UINT32*)Ptr > 1) {
> -    IncrementErrorCount ();
> -    Print (L"\nERROR: IORT ID Mapping count must not be greater than 1.");
> -  }
> +  UINT32 PmcgNodeIdMapping = *(UINT32 *) Ptr;
> +  AssertConstraint (L"ACPI", PmcgNodeIdMapping <= 1);
>  }
> 
>  /**
> @@ -92,10 +89,8 @@ ValidateItsIdArrayReference (
>    IN VOID*  Context
>    )
>  {
> -  if (*(UINT32*)Ptr != 0) {
> -    IncrementErrorCount ();
> -    Print (L"\nERROR: IORT ID Mapping offset must be zero.");
> -  }
> +  UINT32 ItsNodeMappingArrayOffset = *(UINT32 *) Ptr;
> +  AssertConstraint (L"ACPI", ItsNodeMappingArrayOffset == 0);
>  }
> 
>  /**
> @@ -268,28 +263,21 @@ DumpIortNodeIdMappings (
>  {
>    UINT32 Index;
>    UINT32 Offset;
> -  CHAR8  Buffer[40];  // Used for AsciiName param of ParseAcpi
> 
> -  Index = 0;
>    Offset = 0;
> +  for (Index = 0; Index < MappingCount; Index++) {
> +    if (AssertMemberIntegrity(Offset, 1, Ptr, Length)) {
> +      return;
> +    }
> 
> -  while ((Index < MappingCount) &&
> -         (Offset < Length)) {
> -    AsciiSPrint (
> -      Buffer,
> -      sizeof (Buffer),
> -      "ID Mapping [%d]",
> -      Index
> -      );
> +    AcpiLog (ACPI_ITEM, L"    ID Mapping[%d] (+0x%x)", Index, Offset);
>      Offset += ParseAcpi (
> -                TRUE,
> -                4,
> -                Buffer,
> -                Ptr + Offset,
> -                Length - Offset,
> -                PARSER_PARAMS (IortNodeIdMappingParser)
> -                );
> -    Index++;
> +      TRUE,
> +      4,
> +      NULL,
> +      Ptr + Offset,
> +      Length - Offset,
> +      PARSER_PARAMS (IortNodeIdMappingParser));
>    }
>  }
> 
> @@ -313,7 +301,6 @@ DumpIortNodeSmmuV1V2 (
>  {
>    UINT32 Index;
>    UINT32 Offset;
> -  CHAR8  Buffer[50];  // Used for AsciiName param of ParseAcpi
> 
>    ParseAcpi (
>      TRUE,
> @@ -330,56 +317,41 @@ DumpIortNodeSmmuV1V2 (
>        (InterruptContextOffset == NULL)  ||
>        (PmuInterruptCount == NULL)       ||
>        (PmuInterruptOffset == NULL)) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"ERROR: Insufficient SMMUv1/2 node length. Length = %d\n",
> -      Length
> -      );
> +    AcpiError (ACPI_ERROR_PARSE, L"Failed to parse the SMMUv1/2 node");
>      return;
>    }
> 
>    Offset = *InterruptContextOffset;
> -  Index = 0;
> +  for (Index = 0; Index < *InterruptContextCount; Index++) {
> +    if (AssertMemberIntegrity(Offset, 1, Ptr, Length)) {
> +      break;
> +    }
> 
> -  while ((Index < *InterruptContextCount) &&
> -         (Offset < Length)) {
> -    AsciiSPrint (
> -      Buffer,
> -      sizeof (Buffer),
> -      "Context Interrupts Array [%d]",
> -      Index
> -      );
> +    AcpiLog (
> +      ACPI_ITEM, L"    Context Interrupts Array[%d] (+0x%x)", Index, Offset);
>      Offset += ParseAcpi (
> -                TRUE,
> -                4,
> -                Buffer,
> -                Ptr + Offset,
> -                Length - Offset,
> -                PARSER_PARAMS (InterruptArrayParser)
> -                );
> -    Index++;
> +      TRUE,
> +      4,
> +      NULL,
> +      Ptr + Offset,
> +      Length - Offset,
> +      PARSER_PARAMS (InterruptArrayParser));
>    }
> 
>    Offset = *PmuInterruptOffset;
> -  Index = 0;
> +  for(Index = 0; Index < *PmuInterruptCount; Index++) {
> +    if (AssertMemberIntegrity(Offset, 1, Ptr, Length)){
> +      break;
> +    }
> 
> -  while ((Index < *PmuInterruptCount) &&
> -         (Offset < Length)) {
> -    AsciiSPrint (
> -      Buffer,
> -      sizeof (Buffer),
> -      "PMU Interrupts Array [%d]",
> -      Index
> -      );
> +    AcpiLog (ACPI_ITEM, L"    PMU Interrupts Array[%d] (+0x%x)", Index, Offset);
>      Offset += ParseAcpi (
> -                TRUE,
> -                4,
> -                Buffer,
> -                Ptr + Offset,
> -                Length - Offset,
> -                PARSER_PARAMS (InterruptArrayParser)
> -                );
> -    Index++;
> +      TRUE,
> +      4,
> +      NULL,
> +      Ptr + Offset,
> +      Length - Offset,
> +      PARSER_PARAMS (InterruptArrayParser));
>    }
> 
>    DumpIortNodeIdMappings (
> @@ -438,7 +410,6 @@ DumpIortNodeIts (
>  {
>    UINT32 Offset;
>    UINT32 Index;
> -  CHAR8  Buffer[80];  // Used for AsciiName param of ParseAcpi
> 
>    Offset = ParseAcpi (
>              TRUE,
> @@ -452,32 +423,26 @@ DumpIortNodeIts (
>    // Check if the values used to control the parsing logic have been
>    // successfully read.
>    if (ItsCount == NULL) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"ERROR: Insufficient ITS group length. Length = %d.\n",
> -      Length
> -      );
> +    AcpiError (ACPI_ERROR_PARSE, L"Failed to parse ITS node");
>      return;
>    }
> 
>    Index = 0;
> 
> -  while ((Index < *ItsCount) &&
> -         (Offset < Length)) {
> -    AsciiSPrint (
> -      Buffer,
> -      sizeof (Buffer),
> -      "GIC ITS Identifier Array [%d]",
> -      Index
> -      );
> +  for (Index = 0; Index < *ItsCount; Index++) {
> +    if (AssertMemberIntegrity(Offset, 1, Ptr, Length)) {
> +      return;
> +    }
> +
> +    AcpiLog (
> +      ACPI_ITEM, L"    GIC ITS Identifier Array[%d] (+0x%x)", Index, Offset);
>      Offset += ParseAcpi (
> -                TRUE,
> -                4,
> -                Buffer,
> -                Ptr + Offset,
> -                Length - Offset,
> -                PARSER_PARAMS (ItsIdParser)
> -                );
> +      TRUE,
> +      4,
> +      NULL,
> +      Ptr + Offset,
> +      Length - Offset,
> +      PARSER_PARAMS (ItsIdParser));
>      Index++;
>    }
> 
> @@ -516,13 +481,10 @@ DumpIortNodeNamedComponent (
> 
>    // Estimate the Device Name length
>    PrintFieldName (2, L"Device Object Name");
> -
> -  while ((*(Ptr + Offset) != 0) &&
> -         (Offset < Length)) {
> -    Print (L"%c", *(Ptr + Offset));
> -    Offset++;
> -  }
> -  Print (L"\n");
> +  AcpiInfo (
> +    L"%.*a",
> +    AsciiStrnLenS ((CONST CHAR8 *)Ptr + Offset, Length - Offset),
> +    Ptr + Offset);
> 
>    DumpIortNodeIdMappings (
>      Ptr + MappingOffset,
> @@ -629,7 +591,6 @@ ParseAcpiIort (
>  {
>    UINT32 Offset;
>    UINT32 Index;
> -  UINT8* NodePtr;
> 
>    if (!Trace) {
>      return;
> @@ -648,16 +609,11 @@ ParseAcpiIort (
>    // successfully read.
>    if ((IortNodeCount == NULL) ||
>        (IortNodeOffset == NULL)) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"ERROR: Insufficient table length. AcpiTableLength = %d.\n",
> -      AcpiTableLength
> -      );
> +    AcpiError (ACPI_ERROR_PARSE, L"Failed to parse IORT Node.");
>      return;
>    }
> 
>    Offset = *IortNodeOffset;
> -  NodePtr = Ptr + Offset;
>    Index = 0;
> 
>    // Parse the specified number of IORT nodes or the IORT table buffer length.
> @@ -669,7 +625,7 @@ ParseAcpiIort (
>        FALSE,
>        0,
>        "IORT Node Header",
> -      NodePtr,
> +      Ptr + Offset,
>        AcpiTableLength - Offset,
>        PARSER_PARAMS (IortNodeHeaderParser)
>        );
> @@ -680,42 +636,28 @@ ParseAcpiIort (
>          (IortNodeLength == NULL)      ||
>          (IortIdMappingCount == NULL)  ||
>          (IortIdMappingOffset == NULL)) {
> -      IncrementErrorCount ();
> -      Print (
> -        L"ERROR: Insufficient remaining table buffer length to read the " \
> -          L"IORT node header. Length = %d.\n",
> -        AcpiTableLength - Offset
> -        );
> +      AcpiError (ACPI_ERROR_PARSE, L"Failed ot parse the IORT node header");
>        return;
>      }
> 
> -    // Validate IORT Node length
> -    if ((*IortNodeLength == 0) ||
> -        ((Offset + (*IortNodeLength)) > AcpiTableLength)) {
> -      IncrementErrorCount ();
> -      Print (
> -        L"ERROR: Invalid IORT Node length. " \
> -          L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
> -        *IortNodeLength,
> -        Offset,
> -        AcpiTableLength
> -        );
> +    // Protect against buffer overrun
> +    if (AssertMemberIntegrity (Offset, *IortNodeLength, Ptr, AcpiTableLength)) {
>        return;
>      }
> 
>      PrintFieldName (2, L"* Node Offset *");
> -    Print (L"0x%x\n", Offset);
> +    AcpiInfo (L"0x%x", Offset);
> 
>      switch (*IortNodeType) {
>        case EFI_ACPI_IORT_TYPE_ITS_GROUP:
>          DumpIortNodeIts (
> -          NodePtr,
> +          Ptr + Offset,
>            *IortNodeLength
>            );
>          break;
>        case EFI_ACPI_IORT_TYPE_NAMED_COMP:
>          DumpIortNodeNamedComponent (
> -          NodePtr,
> +          Ptr + Offset,
>            *IortNodeLength,
>            *IortIdMappingCount,
>            *IortIdMappingOffset
> @@ -723,7 +665,7 @@ ParseAcpiIort (
>          break;
>        case EFI_ACPI_IORT_TYPE_ROOT_COMPLEX:
>          DumpIortNodeRootComplex (
> -          NodePtr,
> +          Ptr + Offset,
>            *IortNodeLength,
>            *IortIdMappingCount,
>            *IortIdMappingOffset
> @@ -731,7 +673,7 @@ ParseAcpiIort (
>          break;
>        case EFI_ACPI_IORT_TYPE_SMMUv1v2:
>          DumpIortNodeSmmuV1V2 (
> -          NodePtr,
> +          Ptr + Offset,
>            *IortNodeLength,
>            *IortIdMappingCount,
>            *IortIdMappingOffset
> @@ -739,7 +681,7 @@ ParseAcpiIort (
>          break;
>        case EFI_ACPI_IORT_TYPE_SMMUv3:
>          DumpIortNodeSmmuV3 (
> -          NodePtr,
> +          Ptr + Offset,
>            *IortNodeLength,
>            *IortIdMappingCount,
>            *IortIdMappingOffset
> @@ -747,7 +689,7 @@ ParseAcpiIort (
>          break;
>        case EFI_ACPI_IORT_TYPE_PMCG:
>          DumpIortNodePmcg (
> -          NodePtr,
> +          Ptr + Offset,
>            *IortNodeLength,
>            *IortIdMappingCount,
>            *IortIdMappingOffset
> @@ -755,11 +697,10 @@ ParseAcpiIort (
>          break;
> 
>        default:
> -        IncrementErrorCount ();
> -        Print (L"ERROR: Unsupported IORT Node type = %d\n", *IortNodeType);
> +        AcpiError (
> +          ACPI_ERROR_VALUE, L"Unsupported IORT Node type = %d",
> *IortNodeType);
>      } // switch
> 
> -    NodePtr += (*IortNodeLength);
>      Offset += (*IortNodeLength);
>    } // while
>  }
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
> index 15aa2392b60c..67bbf369ee1a 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
> @@ -17,6 +17,7 @@
>  #include "AcpiTableParser.h"
>  #include "AcpiViewConfig.h"
>  #include "MadtParser.h"
> +#include "AcpiViewLog.h"
> 
>  // Local Variables
>  STATIC CONST UINT8* MadtInterruptControllerType;
> @@ -38,12 +39,8 @@ ValidateGICDSystemVectorBase (
>    IN VOID*  Context
>  )
>  {
> -  if (*(UINT32*)Ptr != 0) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"\nERROR: System Vector Base must be zero."
> -    );
> -  }
> +  UINT32 GicdSystemVectorBase = *(UINT32 *) Ptr;
> +  AssertConstraint (L"ACPI", GicdSystemVectorBase == 0);
>  }
> 
>  /**
> @@ -63,36 +60,20 @@ ValidateSpeOverflowInterrupt (
>  {
>    UINT16 SpeOverflowInterrupt;
> 
> -  SpeOverflowInterrupt = *(UINT16*)Ptr;
> +  SpeOverflowInterrupt = *(UINT16 *) Ptr;
> 
>    // SPE not supported by this processor
>    if (SpeOverflowInterrupt == 0) {
>      return;
>    }
> 
> -  if ((SpeOverflowInterrupt < ARM_PPI_ID_MIN) ||
> -      ((SpeOverflowInterrupt > ARM_PPI_ID_MAX) &&
> -       (SpeOverflowInterrupt < ARM_PPI_ID_EXTENDED_MIN)) ||
> -      (SpeOverflowInterrupt > ARM_PPI_ID_EXTENDED_MAX)) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"\nERROR: SPE Overflow Interrupt ID of %d is not in the allowed PPI ID "
> -        L"ranges of %d-%d or %d-%d (for GICv3.1 or later).",
> -      SpeOverflowInterrupt,
> -      ARM_PPI_ID_MIN,
> -      ARM_PPI_ID_MAX,
> -      ARM_PPI_ID_EXTENDED_MIN,
> -      ARM_PPI_ID_EXTENDED_MAX
> -    );
> -  } else if (SpeOverflowInterrupt != ARM_PPI_ID_PMBIRQ) {
> -    IncrementWarningCount();
> -    Print (
> -      L"\nWARNING: SPE Overflow Interrupt ID of %d is not compliant with SBSA "
> -        L"Level 3 PPI ID assignment: %d.",
> -      SpeOverflowInterrupt,
> -      ARM_PPI_ID_PMBIRQ
> -    );
> -  }
> +  AssertConstraint (L"ACPI", SpeOverflowInterrupt > ARM_PPI_ID_MIN);
> +  AssertConstraint (
> +    L"ACPI",
> +    (SpeOverflowInterrupt < ARM_PPI_ID_MAX) ||
> +      (SpeOverflowInterrupt > ARM_PPI_ID_EXTENDED_MIN));
> +  AssertConstraint (L"ACPI", SpeOverflowInterrupt <
> ARM_PPI_ID_EXTENDED_MAX);
> +  WarnConstraint (L"SBSA", SpeOverflowInterrupt == ARM_PPI_ID_PMBIRQ);
>  }
> 
>  /**
> @@ -231,7 +212,6 @@ ParseAcpiMadt (
>    )
>  {
>    UINT32 Offset;
> -  UINT8* InterruptContollerPtr;
>    UINT32 GICDCount;
> 
>    GICDCount = 0;
> @@ -248,7 +228,6 @@ ParseAcpiMadt (
>               AcpiTableLength,
>               PARSER_PARAMS (MadtParser)
>               );
> -  InterruptContollerPtr = Ptr + Offset;
> 
>    while (Offset < AcpiTableLength) {
>      // Parse Interrupt Controller Structure to obtain Length.
> @@ -256,7 +235,7 @@ ParseAcpiMadt (
>        FALSE,
>        0,
>        NULL,
> -      InterruptContollerPtr,
> +      Ptr + Offset,
>        AcpiTableLength - Offset,
>        PARSER_PARAMS (MadtInterruptControllerHeaderParser)
>        );
> @@ -265,26 +244,14 @@ ParseAcpiMadt (
>      // successfully read.
>      if ((MadtInterruptControllerType == NULL) ||
>          (MadtInterruptControllerLength == NULL)) {
> -      IncrementErrorCount ();
> -      Print (
> -        L"ERROR: Insufficient remaining table buffer length to read the " \
> -          L"Interrupt Controller Structure header. Length = %d.\n",
> -        AcpiTableLength - Offset
> -        );
> +      AcpiError (
> +        ACPI_ERROR_PARSE,
> +        L"Failed to read the Interrupt Controller Structure header");
>        return;
>      }
> 
> -    // Validate Interrupt Controller Structure length
> -    if ((*MadtInterruptControllerLength == 0) ||
> -        ((Offset + (*MadtInterruptControllerLength)) > AcpiTableLength)) {
> -      IncrementErrorCount ();
> -      Print (
> -        L"ERROR: Invalid Interrupt Controller Structure length. " \
> -          L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
> -        *MadtInterruptControllerLength,
> -        Offset,
> -        AcpiTableLength
> -        );
> +    if (AssertMemberIntegrity (
> +          Offset, *MadtInterruptControllerLength, Ptr, AcpiTableLength)) {
>        return;
>      }
> 
> @@ -294,7 +261,7 @@ ParseAcpiMadt (
>            TRUE,
>            2,
>            "GICC",
> -          InterruptContollerPtr,
> +          Ptr + Offset,
>            *MadtInterruptControllerLength,
>            PARSER_PARAMS (GicCParser)
>            );
> @@ -303,18 +270,16 @@ ParseAcpiMadt (
> 
>        case EFI_ACPI_6_3_GICD: {
>          if (++GICDCount > 1) {
> -          IncrementErrorCount ();
> -          Print (
> -            L"ERROR: Only one GICD must be present,"
> -              L" GICDCount = %d\n",
> -            GICDCount
> -            );
> +          AcpiError (
> +            ACPI_ERROR_CROSS,
> +            L"Only one GICD must be present (now %d)",
> +            GICDCount);
>          }
>          ParseAcpi (
>            TRUE,
>            2,
>            "GICD",
> -          InterruptContollerPtr,
> +          Ptr + Offset,
>            *MadtInterruptControllerLength,
>            PARSER_PARAMS (GicDParser)
>            );
> @@ -326,7 +291,7 @@ ParseAcpiMadt (
>            TRUE,
>            2,
>            "GIC MSI Frame",
> -          InterruptContollerPtr,
> +          Ptr + Offset,
>            *MadtInterruptControllerLength,
>            PARSER_PARAMS (GicMSIFrameParser)
>            );
> @@ -338,7 +303,7 @@ ParseAcpiMadt (
>            TRUE,
>            2,
>            "GICR",
> -          InterruptContollerPtr,
> +          Ptr + Offset,
>            *MadtInterruptControllerLength,
>            PARSER_PARAMS (GicRParser)
>            );
> @@ -350,7 +315,7 @@ ParseAcpiMadt (
>            TRUE,
>            2,
>            "GIC ITS",
> -          InterruptContollerPtr,
> +          Ptr + Offset,
>            *MadtInterruptControllerLength,
>            PARSER_PARAMS (GicITSParser)
>            );
> @@ -358,17 +323,13 @@ ParseAcpiMadt (
>        }
> 
>        default: {
> -        IncrementErrorCount ();
> -        Print (
> -          L"ERROR: Unknown Interrupt Controller Structure,"
> -            L" Type = %d, Length = %d\n",
> -          *MadtInterruptControllerType,
> -          *MadtInterruptControllerLength
> -          );
> +        AcpiError (
> +          ACPI_ERROR_VALUE,
> +          L"Interrupt Controller Structure Type = %d",
> +          *MadtInterruptControllerType);
>        }
>      } // switch
> 
> -    InterruptContollerPtr += *MadtInterruptControllerLength;
>      Offset += *MadtInterruptControllerLength;
>    } // while
>  }
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Mcfg/McfgParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Mcfg/McfgParser.c
> index 9da4d60e8497..7a7eaa374acf 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Mcfg/McfgParser.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Mcfg/McfgParser.c
> @@ -12,6 +12,7 @@
>  #include <Library/UefiLib.h>
>  #include "AcpiParser.h"
>  #include "AcpiTableParser.h"
> +#include "AcpiViewLog.h"
> 
>  // Local variables
>  STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
> @@ -57,8 +58,6 @@ ParseAcpiMcfg (
>    )
>  {
>    UINT32 Offset;
> -  UINT32 PciCfgOffset;
> -  UINT8* PciCfgSpacePtr;
> 
>    if (!Trace) {
>      return;
> @@ -73,18 +72,14 @@ ParseAcpiMcfg (
>               PARSER_PARAMS (McfgParser)
>               );
> 
> -  PciCfgSpacePtr = Ptr + Offset;
> -
>    while (Offset < AcpiTableLength) {
> -    PciCfgOffset = ParseAcpi (
> +    Offset += ParseAcpi (
>                       TRUE,
>                       2,
>                       "PCI Configuration Space",
> -                     PciCfgSpacePtr,
> +                     Ptr + Offset,
>                       (AcpiTableLength - Offset),
>                       PARSER_PARAMS (PciCfgSpaceBaseAddrParser)
>                       );
> -    PciCfgSpacePtr += PciCfgOffset;
> -    Offset += PciCfgOffset;
>    }
>  }
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
> index 97a5203efb5f..3afb4e103ea9 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
> @@ -14,6 +14,7 @@
>  #include "AcpiParser.h"
>  #include "AcpiView.h"
>  #include "AcpiViewConfig.h"
> +#include "AcpiViewLog.h"
>  #include "PpttParser.h"
>  #include "AcpiViewLog.h"
> 
> @@ -39,38 +40,20 @@ ValidateCacheNumberOfSets (
>    IN VOID*  Context
>    )
>  {
> -  UINT32 NumberOfSets;
> -  NumberOfSets = *(UINT32*)Ptr;
> +  UINT32 CacheNumberOfSets = *(UINT32*) Ptr;
> 
> -  if (NumberOfSets == 0) {
> -    IncrementErrorCount ();
> -    Print (L"\nERROR: Cache number of sets must be greater than 0");
> -    return;
> -  }
> +  AssertConstraint (L"ACPI", CacheNumberOfSets != 0);
> 
>  #if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
> -  if (NumberOfSets > PPTT_ARM_CCIDX_CACHE_NUMBER_OF_SETS_MAX) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"\nERROR: When ARMv8.3-CCIDX is implemented the maximum cache
> number of "
> -        L"sets must be less than or equal to %d",
> -      PPTT_ARM_CCIDX_CACHE_NUMBER_OF_SETS_MAX
> -      );
> +  if (AssertConstraint (
> +        L"ARMv8.3-CCIDX",
> +        CacheNumberOfSets <
> PPTT_ARM_CCIDX_CACHE_NUMBER_OF_SETS_MAX)) {
>      return;
>    }
> 
> -  if (NumberOfSets > PPTT_ARM_CACHE_NUMBER_OF_SETS_MAX) {
> -    IncrementWarningCount ();
> -    Print (
> -      L"\nWARNING: Without ARMv8.3-CCIDX, the maximum cache number of
> sets "
> -        L"must be less than or equal to %d. Ignore this message if "
> -        L"ARMv8.3-CCIDX is implemented",
> -      PPTT_ARM_CACHE_NUMBER_OF_SETS_MAX
> -      );
> -    return;
> -  }
> +  WarnConstraint (
> +    L"No-ARMv8.3-CCIDX", CacheNumberOfSets <
> PPTT_ARM_CACHE_NUMBER_OF_SETS_MAX);
>  #endif
> -
>  }
> 
>  /**
> @@ -89,14 +72,8 @@ ValidateCacheAssociativity (
>    IN VOID*  Context
>    )
>  {
> -  UINT8 Associativity;
> -  Associativity = *(UINT8*)Ptr;
> -
> -  if (Associativity == 0) {
> -    IncrementErrorCount ();
> -    Print (L"\nERROR: Cache associativity must be greater than 0");
> -    return;
> -  }
> +  UINT8 CacheAssociativity = *Ptr;
> +  AssertConstraint (L"ACPI", CacheAssociativity != 0);
>  }
> 
>  /**
> @@ -120,25 +97,14 @@ ValidateCacheLineSize (
>    //   LineSize, bits [2:0]
>    //     (Log2(Number of bytes in cache line)) - 4.
> 
> -  UINT16 LineSize;
> -  LineSize = *(UINT16*)Ptr;
> -
> -  if ((LineSize < PPTT_ARM_CACHE_LINE_SIZE_MIN) ||
> -      (LineSize > PPTT_ARM_CACHE_LINE_SIZE_MAX)) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"\nERROR: The cache line size must be between %d and %d bytes"
> -        L" on ARM Platforms.",
> -      PPTT_ARM_CACHE_LINE_SIZE_MIN,
> -      PPTT_ARM_CACHE_LINE_SIZE_MAX
> -      );
> -    return;
> -  }
> +  UINT16 CacheLineSize = *(UINT16 *) Ptr;
> 
> -  if ((LineSize & (LineSize - 1)) != 0) {
> -    IncrementErrorCount ();
> -    Print (L"\nERROR: The cache line size is not a power of 2.");
> -  }
> +  AssertConstraint (
> +    L"ARM",
> +    (CacheLineSize >= PPTT_ARM_CACHE_LINE_SIZE_MIN &&
> +     CacheLineSize <= PPTT_ARM_CACHE_LINE_SIZE_MAX));
> +
> +  AssertConstraint (L"ARM", BitFieldCountOnes32 (CacheLineSize, 0, 15) == 1);
>  #endif
>  }
> 
> @@ -160,17 +126,8 @@ ValidateCacheAttributes (
>    // Reference: Advanced Configuration and Power Interface (ACPI) Specification
>    //            Version 6.2 Errata A, September 2017
>    // Table 5-153: Cache Type Structure
> -  UINT8 Attributes;
> -  Attributes = *(UINT8*)Ptr;
> -
> -  if ((Attributes & 0xE0) != 0) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"\nERROR: Attributes bits [7:5] are reserved and must be zero.",
> -      Attributes
> -      );
> -    return;
> -  }
> +  UINT8 Attributes = *(UINT8 *) Ptr;
> +  AssertConstraint (L"ACPI", BitFieldCountOnes32 (Attributes, 5, 7) == 0);
>  }
> 
>  /**
> @@ -255,7 +212,6 @@ DumpProcessorHierarchyNodeStructure (
>  {
>    UINT32 Offset;
>    UINT32 Index;
> -  CHAR16 Buffer[OUTPUT_FIELD_COLUMN_WIDTH];
> 
>    Offset = ParseAcpi (
>               TRUE,
> @@ -268,48 +224,22 @@ DumpProcessorHierarchyNodeStructure (
> 
>    // Check if the values used to control the parsing logic have been
>    // successfully read.
> -  if (NumberOfPrivateResources == NULL) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"ERROR: Insufficient Processor Hierarchy Node length. Length = %d.\n",
> -      Length
> -      );
> -    return;
> -  }
> -
> -  // Make sure the Private Resource array lies inside this structure
> -  if (Offset + (*NumberOfPrivateResources * sizeof (UINT32)) > Length) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"ERROR: Invalid Number of Private Resources. " \
> -        L"PrivateResourceCount = %d. RemainingBufferLength = %d. " \
> -        L"Parsing of this structure aborted.\n",
> -      *NumberOfPrivateResources,
> -      Length - Offset
> -      );
> +  if(NumberOfPrivateResources == NULL) {
> +    AcpiError (ACPI_ERROR_PARSE, L"Failed to parse processor hierarchy");
>      return;
>    }
> 
> -  Index = 0;
> -
>    // Parse the specified number of private resource references or the Processor
>    // Hierarchy Node length. Whichever is minimum.
> -  while (Index < *NumberOfPrivateResources) {
> -    UnicodeSPrint (
> -      Buffer,
> -      sizeof (Buffer),
> -      L"Private resources [%d]",
> -      Index
> -      );
> +  for (Index = 0; Index < *NumberOfPrivateResources; Index++) {
> +    if (AssertMemberIntegrity (Offset, sizeof (UINT32), Ptr, Length)) {
> +      return;
> +    }
> 
> -    PrintFieldName (4, Buffer);
> -    Print (
> -      L"0x%x\n",
> -      *((UINT32*)(Ptr + Offset))
> -      );
> +    PrintFieldName (4, L"Private resources [%d]", Index);
> +    AcpiInfo (L"0x%x", *(UINT32 *) (Ptr + Offset));
> 
>      Offset += sizeof (UINT32);
> -    Index++;
>    }
>  }
> 
> @@ -386,7 +316,6 @@ ParseAcpiPptt (
>    )
>  {
>    UINT32 Offset;
> -  UINT8* ProcessorTopologyStructurePtr;
> 
>    if (!Trace) {
>      return;
> @@ -401,15 +330,13 @@ ParseAcpiPptt (
>               PARSER_PARAMS (PpttParser)
>               );
> 
> -  ProcessorTopologyStructurePtr = Ptr + Offset;
> -
>    while (Offset < AcpiTableLength) {
>      // Parse Processor Hierarchy Node Structure to obtain Type and Length.
>      ParseAcpi (
>        FALSE,
>        0,
>        NULL,
> -      ProcessorTopologyStructurePtr,
> +      Ptr + Offset,
>        AcpiTableLength - Offset,
>        PARSER_PARAMS (ProcessorTopologyStructureHeaderParser)
>        );
> @@ -418,62 +345,42 @@ ParseAcpiPptt (
>      // successfully read.
>      if ((ProcessorTopologyStructureType == NULL) ||
>          (ProcessorTopologyStructureLength == NULL)) {
> -      IncrementErrorCount ();
> -      Print (
> -        L"ERROR: Insufficient remaining table buffer length to read the " \
> -          L"processor topology structure header. Length = %d.\n",
> -        AcpiTableLength - Offset
> -        );
> +      AcpiError (ACPI_ERROR_PARSE, L"Failed to parse processor topology");
>        return;
>      }
> 
>      // Validate Processor Topology Structure length
> -    if ((*ProcessorTopologyStructureLength == 0) ||
> -        ((Offset + (*ProcessorTopologyStructureLength)) > AcpiTableLength)) {
> -      IncrementErrorCount ();
> -      Print (
> -        L"ERROR: Invalid Processor Topology Structure length. " \
> -          L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
> -        *ProcessorTopologyStructureLength,
> -        Offset,
> -        AcpiTableLength
> -        );
> +    if (AssertMemberIntegrity (
> +          Offset, *ProcessorTopologyStructureLength, Ptr, AcpiTableLength)) {
>        return;
>      }
> 
>      PrintFieldName (2, L"* Structure Offset *");
> -    Print (L"0x%x\n", Offset);
> +    AcpiInfo (L"0x%x", Offset);
> 
>      switch (*ProcessorTopologyStructureType) {
>        case EFI_ACPI_6_2_PPTT_TYPE_PROCESSOR:
>          DumpProcessorHierarchyNodeStructure (
> -          ProcessorTopologyStructurePtr,
> +          Ptr + Offset,
>            *ProcessorTopologyStructureLength
>            );
>          break;
>        case EFI_ACPI_6_2_PPTT_TYPE_CACHE:
>          DumpCacheTypeStructure (
> -          ProcessorTopologyStructurePtr,
> +          Ptr + Offset,
>            *ProcessorTopologyStructureLength
>            );
>          break;
>        case EFI_ACPI_6_2_PPTT_TYPE_ID:
>          DumpIDStructure (
> -          ProcessorTopologyStructurePtr,
> +          Ptr + Offset,
>            *ProcessorTopologyStructureLength
>            );
>          break;
>        default:
> -        IncrementErrorCount ();
> -        Print (
> -          L"ERROR: Unknown processor topology structure:"
> -            L" Type = %d, Length = %d\n",
> -          *ProcessorTopologyStructureType,
> -          *ProcessorTopologyStructureLength
> -          );
> +        AcpiError (ACPI_ERROR_VALUE, L"Unknown processor topology structure");
>      }
> 
> -    ProcessorTopologyStructurePtr += *ProcessorTopologyStructureLength;
>      Offset += *ProcessorTopologyStructureLength;
>    } // while
>  }
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c
> index f4a8732a7db7..b71b59d86ee1 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c
> @@ -11,6 +11,7 @@
>  #include <Library/UefiLib.h>
>  #include "AcpiParser.h"
>  #include "AcpiTableParser.h"
> +#include "AcpiViewLog.h"
> 
>  // Local Variables
>  STATIC CONST UINT64* XsdtAddress;
> @@ -36,17 +37,8 @@ ValidateRsdtAddress (
>    // Root System Description Pointer (RSDP), ACPI ? 5.2.5.
>    //   - Within the RSDP, the RsdtAddress field must be null (zero) and the
>    //     XsdtAddresss MUST be a valid, non-null, 64-bit value.
> -  UINT32 RsdtAddr;
> -
> -  RsdtAddr = *(UINT32*)Ptr;
> -
> -  if (RsdtAddr != 0) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"\nERROR: Rsdt Address = 0x%p. This must be NULL on ARM Platforms.",
> -      RsdtAddr
> -      );
> -  }
> +  UINT32 RsdtAddr = *(UINT32 *) Ptr;
> +  AssertConstraint (L"ARM", RsdtAddr == 0);
>  #endif
>  }
> 
> @@ -71,17 +63,8 @@ ValidateXsdtAddress (
>    // Root System Description Pointer (RSDP), ACPI ? 5.2.5.
>    //   - Within the RSDP, the RsdtAddress field must be null (zero) and the
>    //     XsdtAddresss MUST be a valid, non-null, 64-bit value.
> -  UINT64 XsdtAddr;
> -
> -  XsdtAddr = *(UINT64*)Ptr;
> -
> -  if (XsdtAddr == 0) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"\nERROR: Xsdt Address = 0x%p. This must not be NULL on ARM Platforms.",
> -      XsdtAddr
> -      );
> -  }
> +  UINT64 XsdtAddr = *(UINT64 *) Ptr;
> +  AssertConstraint (L"ARM", XsdtAddr != 0);
>  #endif
>  }
> 
> @@ -141,12 +124,7 @@ ParseAcpiRsdp (
>    // Check if the values used to control the parsing logic have been
>    // successfully read.
>    if (XsdtAddress == NULL) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"ERROR: Insufficient table length. AcpiTableLength = %d." \
> -        L"RSDP parsing aborted.\n",
> -      AcpiTableLength
> -      );
> +    AcpiError (ACPI_ERROR_PARSE, L"Failed to parse the RSDP table");
>      return;
>    }
> 
> @@ -154,11 +132,11 @@ ParseAcpiRsdp (
>    // and does not parse the RSDT table. Platforms provide the
>    // RSDT to enable compatibility with ACPI 1.0 operating systems.
>    // Therefore the RSDT should not be used on ARM platforms.
> -  if ((*XsdtAddress) == 0) {
> -    IncrementErrorCount ();
> -    Print (L"ERROR: XSDT Pointer is not set. RSDP parsing aborted.\n");
> +  if (*XsdtAddress == 0) {
> +    AcpiError (
> +      ACPI_ERROR_VALUE, L"XSDT Pointer is not set. RSDP parsing aborted.");
>      return;
>    }
> 
> -  ProcessAcpiTable ((UINT8*)(UINTN)(*XsdtAddress));
> +  ProcessAcpiTable ((VOID *) *XsdtAddress);
>  }
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
> index cedfc8a71849..2cd051e72502 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
> @@ -28,11 +28,6 @@ STATIC CONST ACPI_PARSER SlitParser[] = {
>     (VOID**)&SlitSystemLocalityCount, NULL, NULL}
>  };
> 
> -/**
> -  Macro to get the value of a System Locality
> -**/
> -#define SLIT_ELEMENT(Ptr, i, j) *(Ptr + (i * LocalityCount) + j)
> -
>  /**
>    This function parses the ACPI SLIT table.
>    When trace is enabled this function parses the SLIT table and
> @@ -58,11 +53,11 @@ ParseAcpiSlit (
>    )
>  {
>    UINT32 Offset;
> -  UINT32 Count;
> -  UINT32 Index;
> +  UINT32 Index1;
> +  UINT32 Index2;
>    UINT32 LocalityCount;
> -  UINT8* LocalityPtr;
> -  CHAR16 Buffer[80];  // Used for AsciiName param of ParseAcpi
> +  CHAR16 Buffer[256];
> +  UINTN  StrLen;
> 
>    if (!Trace) {
>      return;
> @@ -80,11 +75,7 @@ ParseAcpiSlit (
>    // Check if the values used to control the parsing logic have been
>    // successfully read.
>    if (SlitSystemLocalityCount == NULL) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"ERROR: Insufficient table length. AcpiTableLength = %d.\n",
> -      AcpiTableLength
> -      );
> +    AcpiError (ACPI_ERROR_PARSE, L"Failed to parse the SLIT table");
>      return;
>    }
> 
> @@ -100,89 +91,64 @@ ParseAcpiSlit (
>                  = 65535
>                  = MAX_UINT16
>    */
> -  if (*SlitSystemLocalityCount > MAX_UINT16) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"ERROR: The Number of System Localities provided can't be represented " \
> -        L"in the SLIT table. SlitSystemLocalityCount = %ld. " \
> -        L"MaxLocalityCountAllowed = %d.\n",
> -      *SlitSystemLocalityCount,
> -      MAX_UINT16
> -      );
> +  if (AssertConstraint (L"ACPI", *SlitSystemLocalityCount <= MAX_UINT16)) {
>      return;
>    }
> 
> -  LocalityCount = (UINT32)*SlitSystemLocalityCount;
> +  LocalityCount = (UINT32) *SlitSystemLocalityCount;
> 
>    // Make sure system localities fit in the table buffer provided
> -  if (Offset + (LocalityCount * LocalityCount) > AcpiTableLength) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"ERROR: Invalid Number of System Localities. " \
> -        L"SlitSystemLocalityCount = %ld. AcpiTableLength = %d.\n",
> -      *SlitSystemLocalityCount,
> -      AcpiTableLength
> -      );
> +  if (AssertMemberIntegrity (
> +        Offset, (LocalityCount * LocalityCount), Ptr, AcpiTableLength)) {
>      return;
>    }
> 
> -  LocalityPtr = Ptr + Offset;
> -
>    // We only print the Localities if the count is less than 16
>    // If the locality count is more than 16 then refer to the
>    // raw data dump.
>    if (LocalityCount < 16) {
> -    UnicodeSPrint (
> -      Buffer,
> -      sizeof (Buffer),
> -      L"Entry[0x%lx][0x%lx]",
> -      LocalityCount,
> -      LocalityCount
> -      );
> -    PrintFieldName (0, Buffer);
> -    Print (L"\n");
> -    Print (L"       ");
> -    for (Index = 0; Index < LocalityCount; Index++) {
> -      Print (L" (%3d) ", Index);
> +    PrintFieldName (0, L"Entry[0x%lx][0x%lx]", LocalityCount, LocalityCount);
> +    AcpiInfo (L"");
> +    UnicodeSPrint (Buffer, sizeof (Buffer), L"       ");
> +    for (Index1 = 0; Index1 < LocalityCount; Index1++) {
> +      StrLen = StrnLenS (Buffer, sizeof (Buffer));
> +      UnicodeSPrint (
> +        Buffer + StrLen, sizeof (Buffer) - StrLen, L" (%3d) ", Index1);
>      }
> -    Print (L"\n");
> -    for (Count = 0; Count< LocalityCount; Count++) {
> -      Print (L" (%3d) ", Count);
> -      for (Index = 0; Index < LocalityCount; Index++) {
> -        Print (L"  %3d  ", SLIT_ELEMENT (LocalityPtr, Count, Index));
> +    AcpiInfo (L"%s", Buffer);
> +    for (Index1 = 0; Index1 < LocalityCount; Index1++) {
> +      UnicodeSPrint (Buffer, sizeof (Buffer), L" (%3d) ", Index1);
> +      for (Index2 = 0; Index2 < LocalityCount; Index2++) {
> +        StrLen = StrnLenS (Buffer, sizeof (Buffer));
> +        UnicodeSPrint (
> +          Buffer + StrLen,
> +          sizeof (Buffer) - StrLen,
> +          L"  %3d  ",
> +          *(Ptr + Offset + Index1 * LocalityCount + Index2));
>        }
> -      Print (L"\n");
> +      AcpiInfo (L"%s", Buffer);
>      }
>    }
> 
>    // Validate
> -  for (Count = 0; Count < LocalityCount; Count++) {
> -    for (Index = 0; Index < LocalityCount; Index++) {
> -      // Element[x][x] must be equal to 10
> -      if ((Count == Index) && (SLIT_ELEMENT (LocalityPtr, Count,Index) != 10)) {
> -        IncrementErrorCount ();
> -        Print (
> -          L"ERROR: Diagonal Element[0x%lx][0x%lx] (%3d)."
> -            L" Normalized Value is not 10\n",
> -          Count,
> -          Index,
> -          SLIT_ELEMENT (LocalityPtr, Count, Index)
> -          );
> -      }
> +  for (Index1 = 0; Index1 < LocalityCount; Index1++) {
> +    // Element[x][x] must be equal to 10
> +    if (*(Ptr + Offset + Index1 * LocalityCount + Index2) != 10) {
> +      AcpiError (
> +        ACPI_ERROR_VALUE, L"SLIT Element[%d][%d] != 10", Index1, Index1);
> +    }
> +    for (Index2 = 0; Index2 < Index1; Index2++) {
>        // Element[i][j] must be equal to Element[j][i]
> -      if (SLIT_ELEMENT (LocalityPtr, Count, Index) !=
> -          SLIT_ELEMENT (LocalityPtr, Index, Count)) {
> -        IncrementErrorCount ();
> -        Print (
> -          L"ERROR: Relative distances for Element[0x%lx][0x%lx] (%3d) and \n"
> -           L"Element[0x%lx][0x%lx] (%3d) do not match.\n",
> -          Count,
> -          Index,
> -          SLIT_ELEMENT (LocalityPtr, Count, Index),
> -          Index,
> -          Count,
> -          SLIT_ELEMENT (LocalityPtr, Index, Count)
> -          );
> +      if (
> +        *(Ptr + Offset + Index1 * LocalityCount + Index2) !=
> +        *(Ptr + Offset + Index2 * LocalityCount + Index1)) {
> +        AcpiError (
> +          ACPI_ERROR_VALUE,
> +          L"SLIT Element[%d][%d] != SLIT Element[%d][%d]",
> +          Index1,
> +          Index2,
> +          Index2,
> +          Index1);
>        }
>      }
>    }
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Spcr/SpcrParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Spcr/SpcrParser.c
> index 3b06b05dee8c..bc3c12e720f2 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Spcr/SpcrParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Spcr/SpcrParser.c
> @@ -14,6 +14,7 @@
>  #include <Library/UefiLib.h>
>  #include "AcpiParser.h"
>  #include "AcpiTableParser.h"
> +#include "AcpiViewLog.h"
> 
>  // Local variables
>  STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
> @@ -34,18 +35,11 @@ ValidateInterruptType (
>    )
>  {
>  #if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
> -  UINT8 InterruptType;
> -
> -  InterruptType = *Ptr;
> -
> -  if (InterruptType !=
> -
> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERRUPT_TYPE_GI
> C) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"\nERROR: InterruptType = %d. This must be 8 on ARM Platforms",
> -      InterruptType
> -      );
> -  }
> +  UINT8 InterruptType = *Ptr;
> +  AssertConstraint (
> +    L"ARM",
> +    InterruptType ==
> +
> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERRUPT_TYPE_GI
> C);
>  #endif
>  }
> 
> @@ -65,17 +59,8 @@ ValidateIrq (
>    )
>  {
>  #if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
> -  UINT8 Irq;
> -
> -  Irq = *Ptr;
> -
> -  if (Irq != 0) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"\nERROR: Irq = %d. This must be zero on ARM Platforms\n",
> -      Irq
> -      );
> -  }
> +  UINT8 Irq = *Ptr;
> +  AssertConstraint (L"ARM", Irq == 0);
>  #endif
>  }
> 
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
> index 568a0400bf07..eafc7e7942a3 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
> @@ -37,10 +37,8 @@ ValidateSratReserved (
>    IN VOID*  Context
>    )
>  {
> -  if (*(UINT32*)Ptr != 1) {
> -    IncrementErrorCount ();
> -    Print (L"\nERROR: Reserved should be 1 for backward compatibility.\n");
> -  }
> +  UINT32 Reserved = *(UINT32 *) Ptr;
> +  AssertConstraint (L"Backwards-Compatibility", Reserved == 1);
>  }
> 
>  /**
> @@ -59,18 +57,8 @@ ValidateSratDeviceHandleType (
>    IN VOID*  Context
>    )
>  {
> -  UINT8   DeviceHandleType;
> -
> -  DeviceHandleType = *Ptr;
> -
> -  if (DeviceHandleType > EFI_ACPI_6_3_PCI_DEVICE_HANDLE) {
> -    IncrementErrorCount ();
> -    Print (
> -      L"\nERROR: Invalid Device Handle Type: %d. Must be between 0 and %d.",
> -      DeviceHandleType,
> -      EFI_ACPI_6_3_PCI_DEVICE_HANDLE
> -      );
> -  }
> +  UINT8   DeviceHandleType = *Ptr;
> +  AssertConstraint (L"ACPI", DeviceHandleType <
> EFI_ACPI_6_3_PCI_DEVICE_HANDLE);
>  }
> 
>  /**
> @@ -87,9 +75,9 @@ DumpSratPciBdfNumber (
>    IN UINT8*        Ptr
>    )
>  {
> -  CHAR16 Buffer[OUTPUT_FIELD_COLUMN_WIDTH];
> -
> -  Print (L"\n");
> +  UINT16 Bus;
> +  UINT16 Device;
> +  UINT16 Function;
> 
>    /*
>      The PCI BDF Number subfields are printed in the order specified in the ACPI
> @@ -102,43 +90,13 @@ DumpSratPciBdfNumber (
>      +-----+------+------+
>    */
> 
> +  Bus = BitFieldRead16(*(UINT16 *) Ptr, 0, 7);
> +  Device = BitFieldRead16(*(UINT16 *) Ptr, 8, 10);
> +  Function = BitFieldRead16(*(UINT16 *) Ptr, 11, 15);
> +
>    // Print PCI Bus Number (Bits 7:0 of Byte 2)
> -  UnicodeSPrint (
> -    Buffer,
> -    sizeof (Buffer),
> -    L"PCI Bus Number"
> -    );
> -  PrintFieldName (4, Buffer);
> -  Print (
> -    L"0x%x\n",
> -    *Ptr
> -    );
> -
> -  Ptr++;
> -
> -  // Print PCI Device Number (Bits 7:3 of Byte 3)
> -  UnicodeSPrint (
> -    Buffer,
> -    sizeof (Buffer),
> -    L"PCI Device Number"
> -    );
> -  PrintFieldName (4, Buffer);
> -  Print (
> -    L"0x%x\n",
> -    (*Ptr & (BIT7 | BIT6 | BIT5 | BIT4 | BIT3)) >> 3
> -    );
> -
> -  // PCI Function Number (Bits 2:0 of Byte 3)
> -  UnicodeSPrint (
> -    Buffer,
> -    sizeof (Buffer),
> -    L"PCI Function Number"
> -    );
> -  PrintFieldName (4, Buffer);
> -  Print (
> -    L"0x%x\n",
> -    *Ptr & (BIT2 | BIT1 | BIT0)
> -    );
> +  PrintFieldName (4, L"PCI Bus:Device.Function");
> +  AcpiInfo (L"%4X:%2X.%d", Bus, Device, Function);
>  }
> 
>  /**
> @@ -176,8 +134,7 @@ DumpSratDeviceHandle (
>   )
>  {
>    if (SratDeviceHandleType == NULL) {
> -    IncrementErrorCount ();
> -    Print (L"\nERROR: Device Handle Type read incorrectly.\n");
> +    AcpiError (ACPI_ERROR_PARSE, L"Failed to parse Device Handle");
>      return;
>    }
> 
> @@ -222,7 +179,7 @@ DumpSratApicProximity (
> 
>    ProximityDomain = Ptr[0] | (Ptr[1] << 8) | (Ptr[2] << 16);
> 
> -  Print (Format, ProximityDomain);
> +  AcpiInfo ((CHAR16 *)Format, ProximityDomain);
>  }
> 
>  /**
> @@ -360,7 +317,6 @@ ParseAcpiSrat (
>    )
>  {
>    UINT32 Offset;
> -  UINT8* ResourcePtr;
>    UINT32 GicCAffinityIndex;
>    UINT32 GicITSAffinityIndex;
>    UINT32 GenericInitiatorAffinityIndex;
> @@ -389,155 +345,113 @@ ParseAcpiSrat (
>               PARSER_PARAMS (SratParser)
>               );
> 
> -  ResourcePtr = Ptr + Offset;
> -
>    while (Offset < AcpiTableLength) {
>      ParseAcpi (
>        FALSE,
>        0,
>        NULL,
> -      ResourcePtr,
> +      Ptr + Offset,
>        AcpiTableLength - Offset,
>        PARSER_PARAMS (SratResourceAllocationParser)
>        );
> 
>      // Check if the values used to control the parsing logic have been
>      // successfully read.
> -    if ((SratRAType == NULL) ||
> -        (SratRALength == NULL)) {
> -      IncrementErrorCount ();
> -      Print (
> -        L"ERROR: Insufficient remaining table buffer length to read the " \
> -          L"Static Resource Allocation structure header. Length = %d.\n",
> -        AcpiTableLength - Offset
> -        );
> +    if ((SratRAType == NULL) || (SratRALength == NULL)) {
> +      AcpiError (ACPI_ERROR_PARSE, L"Failed to parse SRAT header");
>        return;
>      }
> 
>      // Validate Static Resource Allocation Structure length
> -    if ((*SratRALength == 0) ||
> -        ((Offset + (*SratRALength)) > AcpiTableLength)) {
> -      IncrementErrorCount ();
> -      Print (
> -        L"ERROR: Invalid Static Resource Allocation Structure length. " \
> -          L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
> -        *SratRALength,
> -        Offset,
> -        AcpiTableLength
> -        );
> +    if (AssertMemberIntegrity(Offset, *SratRALength, Ptr, AcpiTableLength)) {
>        return;
>      }
> 
>      switch (*SratRAType) {
>        case EFI_ACPI_6_3_GICC_AFFINITY:
> -        AsciiSPrint (
> -          Buffer,
> -          sizeof (Buffer),
> -          "GICC Affinity Structure [%d]",
> -          GicCAffinityIndex++
> -          );
> +        AcpiLog (
> +          ACPI_ITEM, L"GICC Affinity Structure [%d]", GicCAffinityIndex++);
>          ParseAcpi (
>            TRUE,
>            2,
>            Buffer,
> -          ResourcePtr,
> +          Ptr + Offset,
>            *SratRALength,
> -          PARSER_PARAMS (SratGicCAffinityParser)
> -          );
> +          PARSER_PARAMS (SratGicCAffinityParser));
>          break;
> 
>        case EFI_ACPI_6_3_GIC_ITS_AFFINITY:
> -        AsciiSPrint (
> -          Buffer,
> -          sizeof (Buffer),
> -          "GIC ITS Affinity Structure [%d]",
> -          GicITSAffinityIndex++
> -        );
> +        AcpiLog (
> +          ACPI_ITEM, L"GIC ITS Affinity Structure [%d]", GicITSAffinityIndex++);
>          ParseAcpi (
>            TRUE,
>            2,
>            Buffer,
> -          ResourcePtr,
> +          Ptr + Offset,
>            *SratRALength,
> -          PARSER_PARAMS (SratGicITSAffinityParser)
> -          );
> +          PARSER_PARAMS (SratGicITSAffinityParser));
>          break;
> 
>        case EFI_ACPI_6_3_GENERIC_INITIATOR_AFFINITY:
> -        AsciiSPrint (
> -          Buffer,
> -          sizeof (Buffer),
> -          "Generic Initiator Affinity Structure [%d]",
> -          GenericInitiatorAffinityIndex++
> -        );
> +        AcpiLog (
> +          ACPI_ITEM,
> +          L"Generic Initiator Affinity Structure [%d]",
> +          GenericInitiatorAffinityIndex++);
>          ParseAcpi (
>            TRUE,
>            2,
>            Buffer,
> -          ResourcePtr,
> +          Ptr + Offset,
>            *SratRALength,
> -          PARSER_PARAMS (SratGenericInitiatorAffinityParser)
> -        );
> +          PARSER_PARAMS (SratGenericInitiatorAffinityParser));
>          break;
> 
>        case EFI_ACPI_6_3_MEMORY_AFFINITY:
> -        AsciiSPrint (
> -          Buffer,
> -          sizeof (Buffer),
> -          "Memory Affinity Structure [%d]",
> -          MemoryAffinityIndex++
> -          );
> +        AcpiLog (
> +          ACPI_ITEM, L"Memory Affinity Structure [%d]", MemoryAffinityIndex++);
>          ParseAcpi (
>            TRUE,
>            2,
>            Buffer,
> -          ResourcePtr,
> +          Ptr + Offset,
>            *SratRALength,
> -          PARSER_PARAMS (SratMemAffinityParser)
> -          );
> +          PARSER_PARAMS (SratMemAffinityParser));
>          break;
> 
>        case EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC_SAPIC_AFFINITY:
> -        AsciiSPrint (
> -          Buffer,
> -          sizeof (Buffer),
> -          "APIC/SAPIC Affinity Structure [%d]",
> -          ApicSapicAffinityIndex++
> -          );
> +        AcpiLog (
> +          ACPI_ITEM,
> +          L"APIC/SAPIC Affinity Structure [%d]",
> +          ApicSapicAffinityIndex++);
>          ParseAcpi (
>            TRUE,
>            2,
>            Buffer,
> -          ResourcePtr,
> +          Ptr + Offset,
>            *SratRALength,
> -          PARSER_PARAMS (SratApciSapicAffinityParser)
> -          );
> +          PARSER_PARAMS (SratApciSapicAffinityParser));
>          break;
> 
>        case EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_AFFINITY:
> -        AsciiSPrint (
> -          Buffer,
> -          sizeof (Buffer),
> -          "X2APIC Affinity Structure [%d]",
> -          X2ApicAffinityIndex++
> -          );
> +        AcpiLog (
> +          ACPI_ITEM, L"X2APIC Affinity Structure [%d]", X2ApicAffinityIndex++);
>          ParseAcpi (
>            TRUE,
>            2,
>            Buffer,
> -          ResourcePtr,
> +          Ptr + Offset,
>            *SratRALength,
> -          PARSER_PARAMS (SratX2ApciAffinityParser)
> -          );
> +          PARSER_PARAMS (SratX2ApciAffinityParser));
>          break;
> 
>        default:
> -        IncrementErrorCount ();
> -        Print (L"ERROR: Unknown SRAT Affinity type = 0x%x\n", *SratRAType);
> +        AcpiError (
> +          ACPI_ERROR_VALUE,
> +          L"Unknown SRAT Affinity type = 0x%x\n",
> +          *SratRAType);
>          break;
>      }
> 
> -    ResourcePtr += (*SratRALength);
>      Offset += (*SratRALength);
>    }
>  }
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Xsdt/XsdtParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Xsdt/XsdtParser.c
> index 771c4f322b8e..564e231a627a 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Xsdt/XsdtParser.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Xsdt/XsdtParser.c
> @@ -55,87 +55,43 @@ ParseAcpiXsdt (
>    IN UINT8   AcpiTableRevision
>    )
>  {
> -  UINT32        Offset;
>    UINT32        TableOffset;
> -  UINT64*       TablePointer;
> +  UINT64**      TablePointer;
>    UINTN         EntryIndex;
> -  CHAR16        Buffer[32];
> 
> -  Offset = ParseAcpi (
> -             Trace,
> -             0,
> -             "XSDT",
> -             Ptr,
> -             AcpiTableLength,
> -             PARSER_PARAMS (XsdtParser)
> -             );
> -
> -  TableOffset = Offset;
> +  TableOffset = ParseAcpi (
> +    Trace, 0, "XSDT", Ptr, AcpiTableLength, PARSER_PARAMS (XsdtParser));
> 
> +  EntryIndex = 0;
>    if (Trace) {
> -    EntryIndex = 0;
> -    TablePointer = (UINT64*)(Ptr + TableOffset);
> -    while (Offset < AcpiTableLength) {
> +    for (TablePointer = (UINT64 **)(Ptr + TableOffset);
> +         (UINT8 *) TablePointer < Ptr + AcpiTableLength;
> +         TablePointer++) {
> +
>        CONST UINT32* Signature;
>        CONST UINT32* Length;
>        CONST UINT8*  Revision;
> 
> -      if ((UINT64*)(UINTN)(*TablePointer) != NULL) {
> -        UINT8*      SignaturePtr;
> -
> -        ParseAcpiHeader (
> -          (UINT8*)(UINTN)(*TablePointer),
> -          &Signature,
> -          &Length,
> -          &Revision
> -          );
> -
> -        SignaturePtr = (UINT8*)Signature;
> -
> -        UnicodeSPrint (
> -          Buffer,
> -          sizeof (Buffer),
> -          L"Entry[%d] - %c%c%c%c",
> -          EntryIndex++,
> -          SignaturePtr[0],
> -          SignaturePtr[1],
> -          SignaturePtr[2],
> -          SignaturePtr[3]
> -          );
> +      if (*TablePointer != NULL) {
> +        ParseAcpiHeader (*TablePointer, &Signature, &Length, &Revision);
> +        PrintFieldName (2, L"Entry[%d] - %.4a", EntryIndex++, Signature);
> +        AcpiInfo (L"0x%lx", *TablePointer);
>        } else {
> -        UnicodeSPrint (
> -          Buffer,
> -          sizeof (Buffer),
> -          L"Entry[%d]",
> -          EntryIndex++
> -          );
> -      }
> -
> -      PrintFieldName (2, Buffer);
> -      Print (L"0x%lx\n", *TablePointer);
> -
> -      // Validate the table pointers are not NULL
> -      if ((UINT64*)(UINTN)(*TablePointer) == NULL) {
> -        IncrementErrorCount ();
> -        Print (
> -          L"ERROR: Invalid table entry at 0x%lx, table address is 0x%lx\n",
> -          TablePointer,
> -          *TablePointer
> -          );
> +        PrintFieldName (2, L"Entry[%d]", EntryIndex++);
> +        AcpiInfo (L"NULL");
> +        AcpiError (ACPI_ERROR_VALUE, L"Invalid table entry");
>        }
> -      Offset += sizeof (UINT64);
> -      TablePointer++;
> -    } // while
> +    }
>    }
> 
>    // Process the tables
> -  Offset = TableOffset;
> -  TablePointer = (UINT64*)(Ptr + TableOffset);
> -  while (Offset < AcpiTableLength) {
> -    if ((UINT64*)(UINTN)(*TablePointer) != NULL) {
> -      ProcessAcpiTable ((UINT8*)(UINTN)(*TablePointer));
> +  for (TablePointer = (UINT64 **)(Ptr + TableOffset);
> +       (UINT8 *) TablePointer < Ptr + AcpiTableLength;
> +       TablePointer++) {
> +
> +    if (*TablePointer != NULL) {
> +      ProcessAcpiTable (*TablePointer);
>      }
> -    Offset += sizeof (UINT64);
> -    TablePointer++;
> -  } // while
> +
> +  }
>  }
> --
> 2.24.1.windows.2
> 
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62333): https://edk2.groups.io/g/devel/message/62333
Mute This Topic: https://groups.io/mt/75193744/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.