[edk2-devel] [PATCH 8/8] ShellPkg/AcpiView: Refactor table parsers
Gao, Zhichao
zhichao.gao at intel.com
Fri Jul 10 06:42:28 UTC 2020
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 at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Tomas Pilar
> (tpilar)
> Sent: Monday, June 29, 2020 11:20 PM
> To: devel at edk2.groups.io
> Cc: Sami.Mujawar at arm.com; nd at arm.com; Ni, Ray <ray.ni at intel.com>; Gao,
> Zhichao <zhichao.gao at 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 at intel.com>
> Cc: Zhichao Gao <zhichao.gao at intel.com>
> Signed-off-by: Tomas Pilar <tomas.pilar at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list