* [PATCH v3 7/8] ShellPkg/AcpiView: Refactor AcpiView
2020-07-14 17:45 [PATCH v3 0/8] ShellPkg/AcpiView: Refactor Error Logging Tomas Pilar (tpilar)
` (5 preceding siblings ...)
2020-07-14 17:45 ` [PATCH v3 6/8] ShellPkg/AcpiView: Refactor dump helpers Tomas Pilar (tpilar)
@ 2020-07-14 17:45 ` Tomas Pilar (tpilar)
2020-07-14 17:45 ` [PATCH v3 8/8] ShellPkg/AcpiView: Refactor table parsers Tomas Pilar (tpilar)
7 siblings, 0 replies; 9+ messages in thread
From: Tomas Pilar (tpilar) @ 2020-07-14 17:45 UTC (permalink / raw)
To: devel; +Cc: nd, Sami.Mujawar, Ray Ni, Zhichao Gao
Refactor logging using the AcpiViewLog facility.
Trim some of the source to more elegant state.
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Tomas Pilar <tomas.pilar@arm.com>
---
.../UefiShellAcpiViewCommandLib/AcpiParser.c | 225 ++++++++----------
.../UefiShellAcpiViewCommandLib/AcpiParser.h | 6 +-
.../AcpiTableParser.c | 52 ++--
.../AcpiTableParser.h | 2 +-
.../UefiShellAcpiViewCommandLib/AcpiView.c | 187 +++++----------
.../FieldFormatHelper.h | 106 +--------
6 files changed, 188 insertions(+), 390 deletions(-)
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
index 65108e25ff96..16b8d1f80bc2 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
@@ -8,6 +8,9 @@
#include <Uefi.h>
#include <Library/UefiLib.h>
#include <Library/UefiBootServicesTableLib.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/PrintLib.h>
#include "AcpiParser.h"
#include "AcpiView.h"
#include "AcpiViewConfig.h"
@@ -46,6 +49,7 @@ IncrementWarningCount (
mTableWarningCount++;
}
+
/**
This function verifies the ACPI table checksum.
@@ -69,12 +73,7 @@ VerifyChecksum (
{
UINTN ByteCount;
UINT8 Checksum;
- UINTN OriginalAttribute;
- //
- // set local variables to suppress incorrect compiler/analyzer warnings
- //
- OriginalAttribute = 0;
ByteCount = 0;
Checksum = 0;
@@ -84,29 +83,10 @@ VerifyChecksum (
}
if (Log) {
- OriginalAttribute = gST->ConOut->Mode->Attribute;
if (Checksum == 0) {
- if (mConfig.ColourHighlighting) {
- gST->ConOut->SetAttribute (
- gST->ConOut,
- EFI_TEXT_ATTR (EFI_GREEN,
- ((OriginalAttribute&(BIT4|BIT5|BIT6))>>4))
- );
- }
- Print (L"Table Checksum : OK\n\n");
+ AcpiLog (ACPI_GOOD, L"Table Checksum : OK\n");
} else {
- IncrementErrorCount ();
- if (mConfig.ColourHighlighting) {
- gST->ConOut->SetAttribute (
- gST->ConOut,
- EFI_TEXT_ATTR (EFI_RED,
- ((OriginalAttribute&(BIT4|BIT5|BIT6))>>4))
- );
- }
- Print (L"Table Checksum : FAILED (0x%X)\n\n", Checksum);
- }
- if (mConfig.ColourHighlighting) {
- gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute);
+ AcpiError (ACPI_ERROR_CSUM, L"Table Checksum (0x%X != 0)\n", Checksum);
}
}
@@ -127,53 +107,101 @@ DumpRaw (
)
{
UINTN ByteCount;
- UINTN PartLineChars;
- UINTN AsciiBufferIndex;
CHAR8 AsciiBuffer[17];
+ CHAR8 HexBuffer[128];
+ CHAR8 *HexCursor;
ByteCount = 0;
- AsciiBufferIndex = 0;
- Print (L"Address : 0x%p\n", Ptr);
- Print (L"Length : %d\n", Length);
+ AcpiInfo (L"Address : 0x%p", Ptr);
+ AcpiInfo (L"Length : %d\n", Length);
while (ByteCount < Length) {
- if ((ByteCount & 0x0F) == 0) {
- AsciiBuffer[AsciiBufferIndex] = '\0';
- Print (L" %a\n%08X : ", AsciiBuffer, ByteCount);
- AsciiBufferIndex = 0;
- } else if ((ByteCount & 0x07) == 0) {
- Print (L"- ");
+
+ // Reset ascii and hex strings
+ if (ByteCount % 16 == 0) {
+ HexCursor = HexBuffer;
+ ZeroMem (AsciiBuffer, sizeof(AsciiBuffer));
+ ZeroMem (HexBuffer, sizeof(HexBuffer));
+ } else if (ByteCount % 8 == 0) {
+ HexCursor += AsciiSPrint (HexCursor, sizeof(HexBuffer), "- ");
}
+ // Add hex couplet to hex buffer
+ HexCursor += AsciiSPrint (HexCursor, sizeof(HexBuffer), "%02X ", *Ptr);
+
+ // Add ascii letter to the ascii buffer
+ AsciiBuffer[ByteCount & 0xF] = '.';
if ((*Ptr >= ' ') && (*Ptr < 0x7F)) {
- AsciiBuffer[AsciiBufferIndex++] = *Ptr;
- } else {
- AsciiBuffer[AsciiBufferIndex++] = '.';
+ AsciiBuffer[ByteCount & 0xF] = *Ptr;
}
- Print (L"%02X ", *Ptr++);
+ // Print line with fixed width hex part
+ if (ByteCount % 16 == 15) {
+ AcpiInfo (L"%08X : %-.*a %a", ByteCount + 1, 46, HexBuffer, AsciiBuffer);
+ }
ByteCount++;
+ Ptr++;
}
- // Justify the final line using spaces before printing
- // the ASCII data.
- PartLineChars = (Length & 0x0F);
- if (PartLineChars != 0) {
- PartLineChars = 48 - (PartLineChars * 3);
- if ((Length & 0x0F) <= 8) {
- PartLineChars += 2;
- }
- while (PartLineChars > 0) {
- Print (L" ");
- PartLineChars--;
+ // Print the last line
+ if (ByteCount % 16 != 15) {
+ AcpiInfo (
+ L"%08X : %-*a %.*a",
+ (ByteCount + 16) & ~0xF,
+ 46,
+ HexBuffer,
+ ByteCount & 0xF,
+ AsciiBuffer);
}
+}
+
+/**
+ Prints an arbitrary variable to screen using a given parser.
+ Also calls the internal validator if it exists.
+
+ @param[in] Parser The parser to use to print to screen
+ @param[in] Prt Pointer to variable that should be printed
+**/
+STATIC
+VOID
+EFIAPI
+DumpAndValidate(
+ IN CONST ACPI_PARSER* Parser,
+ IN VOID* Ptr
+ )
+{
+ // if there is a Formatter function let the function handle
+ // the printing else if a Format is specified in the table use
+ // the Format for printing
+ PrintFieldName (2, Parser->NameStr);
+ if (Parser->PrintFormatter != NULL) {
+ Parser->PrintFormatter(Parser->Format, Ptr);
+ } else if (Parser->Format != NULL) {
+ switch (Parser->Length) {
+ case 1:
+ AcpiInfo (Parser->Format, *(UINT8 *)Ptr);
+ break;
+ case 2:
+ AcpiInfo (Parser->Format, ReadUnaligned16 ((CONST UINT16 *)Ptr));
+ break;
+ case 4:
+ AcpiInfo (Parser->Format, ReadUnaligned32 ((CONST UINT32 *)Ptr));
+ break;
+ case 8:
+ AcpiInfo (Parser->Format, ReadUnaligned64 ((CONST UINT64 *)Ptr));
+ break;
+ default:
+ AcpiLog (ACPI_BAD, L"<Parse Error>");
+ } // switch
}
- // Print ASCII data for the final line.
- AsciiBuffer[AsciiBufferIndex] = '\0';
- Print (L" %a\n\n", AsciiBuffer);
+ // Validating only makes sense if we are tracing
+ // the parsed table entries, to report by table name.
+ if (mConfig.ConsistencyCheck && (Parser->FieldValidator != NULL)) {
+ Parser->FieldValidator(Ptr, Parser->Context);
+ }
}
/**
@@ -216,39 +244,25 @@ ParseAcpi (
{
UINT32 Index;
UINT32 Offset;
- UINTN OriginalAttribute;
- //
- // set local variables to suppress incorrect compiler/analyzer warnings
- //
- OriginalAttribute = 0;
- Offset = 0;
+ if (Length == 0) {
+ AcpiLog (
+ ACPI_WARN,
+ L"Will not parse zero-length buffer <%a>=%p",
+ AsciiName ? AsciiName : "Unknown Item",
+ Ptr);
+ return 0;
+ }
// Increment the Indent
gIndent += Indent;
if (Trace && (AsciiName != NULL)){
-
- if (mConfig.ColourHighlighting) {
- OriginalAttribute = gST->ConOut->Mode->Attribute;
- gST->ConOut->SetAttribute (
- gST->ConOut,
- EFI_TEXT_ATTR(EFI_YELLOW,
- ((OriginalAttribute&(BIT4|BIT5|BIT6))>>4))
- );
- }
- Print (
- L"%*a%-*a :\n",
- gIndent,
- "",
- (OUTPUT_FIELD_COLUMN_WIDTH - gIndent),
- AsciiName
- );
- if (mConfig.ColourHighlighting) {
- gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute);
- }
+ AcpiLog (
+ ACPI_ITEM, L"%*.a%a", gIndent, "", AsciiName);
}
+ Offset = 0;
for (Index = 0; Index < ParserItems; Index++) {
if ((Offset + Parser[Index].Length) > Length) {
@@ -263,10 +277,8 @@ ParseAcpi (
}
if (mConfig.ConsistencyCheck && (Offset != Parser[Index].Offset)) {
- IncrementErrorCount ();
- Print (
- L"\nERROR: %a: Offset Mismatch for %s\n"
- L"CurrentOffset = %d FieldOffset = %d\n",
+ AcpiError (ACPI_ERROR_PARSE,
+ L"%a: Offset Mismatch for %s (%d != %d)",
AsciiName,
Parser[Index].NameStr,
Offset,
@@ -275,48 +287,13 @@ ParseAcpi (
}
if (Trace) {
- // if there is a Formatter function let the function handle
- // the printing else if a Format is specified in the table use
- // the Format for printing
- PrintFieldName (2, Parser[Index].NameStr);
- if (Parser[Index].PrintFormatter != NULL) {
- Parser[Index].PrintFormatter (Parser[Index].Format, Ptr);
- } else if (Parser[Index].Format != NULL) {
- switch (Parser[Index].Length) {
- case 1:
- DumpUint8 (Parser[Index].Format, Ptr);
- break;
- case 2:
- DumpUint16 (Parser[Index].Format, Ptr);
- break;
- case 4:
- DumpUint32 (Parser[Index].Format, Ptr);
- break;
- case 8:
- DumpUint64 (Parser[Index].Format, Ptr);
- break;
- default:
- Print (
- L"\nERROR: %a: CANNOT PARSE THIS FIELD, Field Length = %d\n",
- AsciiName,
- Parser[Index].Length
- );
- } // switch
-
- // Validating only makes sense if we are tracing
- // the parsed table entries, to report by table name.
- if (mConfig.ConsistencyCheck && (Parser[Index].FieldValidator != NULL)) {
- Parser[Index].FieldValidator (Ptr, Parser[Index].Context);
- }
- }
- Print (L"\n");
- } // if (Trace)
+ DumpAndValidate (&Parser[Index], &Ptr[Offset]);
+ }
if (Parser[Index].ItemPtr != NULL) {
- *Parser[Index].ItemPtr = (VOID*)Ptr;
+ *Parser[Index].ItemPtr = Ptr + Offset;
}
- Ptr += Parser[Index].Length;
Offset += Parser[Index].Length;
} // for
@@ -355,7 +332,7 @@ DumpGasStruct (
IN UINT32 Length
)
{
- Print (L"\n");
+ AcpiInfo(L"");
return ParseAcpi (
TRUE,
Indent,
@@ -421,7 +398,7 @@ DumpAcpiHeader (
UINT32
EFIAPI
ParseAcpiHeader (
- IN UINT8* Ptr,
+ IN VOID* Ptr,
OUT CONST UINT32** Signature,
OUT CONST UINT32** Length,
OUT CONST UINT8** Revision
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
index 54ce44132055..bd3cdb774fb5 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
@@ -18,10 +18,6 @@
/// that allows us to process the log options.
#define RSDP_TABLE_INFO SIGNATURE_32('R', 'S', 'D', 'P')
-// Publicly accessible error and warning counters.
-extern UINT32 mTableErrorCount;
-extern UINT32 mTableWarningCount;
-
/**
This function increments the ACPI table error counter.
**/
@@ -310,7 +306,7 @@ DumpAcpiHeader (
UINT32
EFIAPI
ParseAcpiHeader (
- IN UINT8* Ptr,
+ IN VOID* Ptr,
OUT CONST UINT32** Signature,
OUT CONST UINT32** Length,
OUT CONST UINT8** Revision
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
index 526cb8cb7cad..49acb3d03da1 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
@@ -18,6 +18,7 @@
#include "AcpiTableParser.h"
#include "AcpiView.h"
#include "AcpiViewConfig.h"
+#include "AcpiViewLog.h"
#if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
#include "Arm/SbbrValidator.h"
@@ -179,61 +180,43 @@ GetParser (
VOID
EFIAPI
ProcessAcpiTable (
- IN UINT8* Ptr
+ IN VOID* Ptr
)
{
EFI_STATUS Status;
BOOLEAN Trace;
- CONST UINT32* AcpiTableSignature;
- CONST UINT32* AcpiTableLength;
- CONST UINT8* AcpiTableRevision;
- CONST UINT8* SignaturePtr;
+ CONST UINT32* Signature;
+ CONST UINT32* Length;
+ CONST UINT8* Revision;
PARSE_ACPI_TABLE_PROC ParserProc;
- ParseAcpiHeader (
- Ptr,
- &AcpiTableSignature,
- &AcpiTableLength,
- &AcpiTableRevision
- );
+ ParseAcpiHeader (Ptr, &Signature, &Length, &Revision);
- Trace = ProcessTableReportOptions (
- *AcpiTableSignature,
- Ptr,
- *AcpiTableLength
- );
+ Trace = ProcessTableReportOptions (*Signature, Ptr, *Length);
if (Trace) {
- DumpRaw (Ptr, *AcpiTableLength);
+ DumpRaw (Ptr, *Length);
// Do not process the ACPI table any further if the table length read
// is invalid. The ACPI table should at least contain the table header.
- if (*AcpiTableLength < sizeof (EFI_ACPI_DESCRIPTION_HEADER)) {
- SignaturePtr = (CONST UINT8*)AcpiTableSignature;
- IncrementErrorCount ();
- Print (
- L"ERROR: Invalid %c%c%c%c table length. Length = %d\n",
- SignaturePtr[0],
- SignaturePtr[1],
- SignaturePtr[2],
- SignaturePtr[3],
- *AcpiTableLength
- );
+ if (*Length < sizeof (EFI_ACPI_DESCRIPTION_HEADER)) {
+ AcpiError (
+ ACPI_ERROR_LENGTH, L"Table %4a invalid length %d", Signature, *Length);
return;
}
if (mConfig.ConsistencyCheck) {
- VerifyChecksum (TRUE, Ptr, *AcpiTableLength);
+ VerifyChecksum (TRUE, Ptr, *Length);
}
}
#if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
if (mConfig.MandatoryTableValidate) {
- ArmSbbrIncrementTableCount (*AcpiTableSignature);
+ ArmSbbrIncrementTableCount (*Signature);
}
#endif
- Status = GetParser (*AcpiTableSignature, &ParserProc);
+ Status = GetParser (*Signature, &ParserProc);
if (EFI_ERROR (Status)) {
// No registered parser found, do default handling.
if (Trace) {
@@ -242,10 +225,5 @@ ProcessAcpiTable (
return;
}
- ParserProc (
- Trace,
- Ptr,
- *AcpiTableLength,
- *AcpiTableRevision
- );
+ ParserProc (Trace, Ptr, *Length, *Revision);
}
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.h
index 4f92596b90a6..e2afeda2379c 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.h
@@ -101,7 +101,7 @@ DeregisterParser (
VOID
EFIAPI
ProcessAcpiTable (
- IN UINT8* Ptr
+ IN VOID* Ptr
);
/**
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
index 16f498b8c328..a34af68f7c23 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
@@ -10,6 +10,8 @@
- Arm Server Base Boot Requirements 1.2, September 2019
**/
+#include <Guid/Acpi.h>
+
#include <Library/PrintLib.h>
#include <Library/UefiLib.h>
#include <Library/ShellLib.h>
@@ -22,6 +24,7 @@
#include "AcpiTableParser.h"
#include "AcpiView.h"
#include "AcpiViewConfig.h"
+#include "AcpiViewLog.h"
#if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
#include "Arm/SbbrValidator.h"
@@ -57,7 +60,7 @@ DumpAcpiTableToFile (
mBinTableCount++
);
- Print (L"Dumping ACPI table to : %s ... ", FileNameBuffer);
+ AcpiInfo (L"Dumping ACPI table to : %s ... ", FileNameBuffer);
TransferBytes = ShellDumpBufferToFile (FileNameBuffer, Ptr, Length);
return (Length == TransferBytes);
@@ -79,15 +82,7 @@ ProcessTableReportOptions (
IN CONST UINT32 Length
)
{
- UINTN OriginalAttribute;
- UINT8 *SignaturePtr;
BOOLEAN Log;
-
- //
- // set local variables to suppress incorrect compiler/analyzer warnings
- //
- OriginalAttribute = 0;
- SignaturePtr = (UINT8*)(UINTN)&Signature;
Log = FALSE;
switch (mConfig.ReportType) {
@@ -102,27 +97,9 @@ ProcessTableReportOptions (
break;
case ReportTableList:
if (mTableCount == 0) {
- if (mConfig.ColourHighlighting) {
- OriginalAttribute = gST->ConOut->Mode->Attribute;
- gST->ConOut->SetAttribute (
- gST->ConOut,
- EFI_TEXT_ATTR(EFI_CYAN,
- ((OriginalAttribute&(BIT4|BIT5|BIT6))>>4))
- );
- }
- Print (L"\nInstalled Table(s):\n");
- if (mConfig.ColourHighlighting) {
- gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute);
- }
+ AcpiLog (ACPI_HIGHLIGHT, L"\nInstalled Table(s):");
}
- Print (
- L"\t%4d. %c%c%c%c\n",
- ++mTableCount,
- SignaturePtr[0],
- SignaturePtr[1],
- SignaturePtr[2],
- SignaturePtr[3]
- );
+ AcpiInfo (L"\t%4d. %.*a", ++mTableCount, 4, &Signature);
break;
case ReportDumpBinFile:
if (Signature == mSelectedAcpiTable.Type) {
@@ -137,31 +114,16 @@ ProcessTableReportOptions (
} // switch
if (Log) {
- if (mConfig.ColourHighlighting) {
- OriginalAttribute = gST->ConOut->Mode->Attribute;
- gST->ConOut->SetAttribute (
- gST->ConOut,
- EFI_TEXT_ATTR(EFI_LIGHTBLUE,
- ((OriginalAttribute&(BIT4|BIT5|BIT6))>>4))
- );
- }
- Print (
- L"\n\n --------------- %c%c%c%c Table --------------- \n\n",
- SignaturePtr[0],
- SignaturePtr[1],
- SignaturePtr[2],
- SignaturePtr[3]
- );
- if (mConfig.ColourHighlighting) {
- gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute);
- }
+ AcpiLog (
+ ACPI_HIGHLIGHT,
+ L"\n --------------- %.*a Table --------------- \n",
+ 4,
+ &Signature);
}
return Log;
}
-
-
/**
This function iterates the configuration table entries in the
system table, retrieves the RSDP pointer and starts parsing the ACPI tables.
@@ -181,17 +143,12 @@ AcpiView (
EFI_STATUS Status;
UINTN Index;
EFI_CONFIGURATION_TABLE* EfiConfigurationTable;
- BOOLEAN FoundAcpiTable;
- UINTN OriginalAttribute;
- UINTN PrintAttribute;
UINT8* RsdpPtr;
UINT32 RsdpLength;
UINT8 RsdpRevision;
PARSE_ACPI_TABLE_PROC RsdpParserProc;
BOOLEAN Trace;
- OriginalAttribute = 0;
-
// Reset Table counts
mTableCount = 0;
mBinTableCount = 0;
@@ -201,107 +158,77 @@ AcpiView (
mTableWarningCount = 0;
// Search the table for an entry that matches the ACPI Table Guid
- FoundAcpiTable = FALSE;
+ EfiConfigurationTable = NULL;
for (Index = 0; Index < SystemTable->NumberOfTableEntries; Index++) {
if (CompareGuid (&gEfiAcpiTableGuid,
&(SystemTable->ConfigurationTable[Index].VendorGuid))) {
EfiConfigurationTable = &SystemTable->ConfigurationTable[Index];
- FoundAcpiTable = TRUE;
break;
}
}
- if (FoundAcpiTable) {
- RsdpPtr = (UINT8*)EfiConfigurationTable->VendorTable;
+ if (!EfiConfigurationTable) {
+ AcpiFatal (L"No ACPI Table Guid in System Configuration Table.");
+ return EFI_NOT_FOUND;
+ }
- // The RSDP revision is 1 byte starting at offset 15
- RsdpRevision = *(RsdpPtr + RSDP_REVISION_OFFSET);
+ RsdpPtr = (UINT8 *)EfiConfigurationTable->VendorTable;
- if (RsdpRevision < 2) {
- Print (
- L"ERROR: RSDP version less than 2 is not supported.\n"
- );
- return EFI_UNSUPPORTED;
- }
+ // The RSDP revision is 1 byte starting at offset 15
+ RsdpRevision = *(RsdpPtr + RSDP_REVISION_OFFSET);
-#if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
- if (mConfig.MandatoryTableValidate) {
- ArmSbbrResetTableCounts ();
- }
-#endif
+ if (RsdpRevision < 2) {
+ AcpiFatal (L"RSDP version less than 2 is not supported.");
+ return EFI_UNSUPPORTED;
+ }
- // The RSDP length is 4 bytes starting at offset 20
- RsdpLength = *(UINT32*)(RsdpPtr + RSDP_LENGTH_OFFSET);
+#if defined(MDE_CPU_ARM) || defined(MDE_CPU_AARCH64)
+ if (mConfig.MandatoryTableValidate) {
+ ArmSbbrResetTableCounts();
+ }
+#endif
- Trace = ProcessTableReportOptions (RSDP_TABLE_INFO, RsdpPtr, RsdpLength);
+ // The RSDP length is 4 bytes starting at offset 20
+ RsdpLength = *(UINT32 *)(RsdpPtr + RSDP_LENGTH_OFFSET);
- Status = GetParser (RSDP_TABLE_INFO, &RsdpParserProc);
- if (EFI_ERROR (Status)) {
- Print (
- L"ERROR: No registered parser found for RSDP.\n"
- );
- return Status;
- }
+ Trace = ProcessTableReportOptions(RSDP_TABLE_INFO, RsdpPtr, RsdpLength);
- RsdpParserProc (
- Trace,
- RsdpPtr,
- RsdpLength,
- RsdpRevision
- );
-
- } else {
- IncrementErrorCount ();
- Print (
- L"ERROR: Failed to find ACPI Table Guid in System Configuration Table.\n"
- );
- return EFI_NOT_FOUND;
+ Status = GetParser(RSDP_TABLE_INFO, &RsdpParserProc);
+ if (EFI_ERROR(Status)) {
+ AcpiFatal (L"No registered parser found for RSDP.");
+ return Status;
}
+ RsdpParserProc(Trace, RsdpPtr, RsdpLength, RsdpRevision);
+
#if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
if (mConfig.MandatoryTableValidate) {
ArmSbbrReqsValidate ((ARM_SBBR_VERSION) mConfig.MandatoryTableSpec);
}
#endif
- if (ReportTableList != mConfig.ReportType) {
- if (((ReportSelected == mConfig.ReportType) ||
- (ReportDumpBinFile == mConfig.ReportType)) &&
- (!mSelectedAcpiTable.Found)) {
- Print (L"\nRequested ACPI Table not found.\n");
- } else if (mConfig.ConsistencyCheck &&
- (ReportDumpBinFile != mConfig.ReportType)) {
- OriginalAttribute = gST->ConOut->Mode->Attribute;
-
- Print (L"\nTable Statistics:\n");
-
- if (mConfig.ColourHighlighting) {
- PrintAttribute = ((mTableErrorCount) > 0) ?
- EFI_TEXT_ATTR (
- EFI_RED,
- ((OriginalAttribute&(BIT4|BIT5|BIT6))>>4)
- ) :
- OriginalAttribute;
- gST->ConOut->SetAttribute (gST->ConOut, PrintAttribute);
- }
- Print (L"\t%d Error(s)\n", mTableErrorCount);
-
- if (mConfig.ColourHighlighting) {
- PrintAttribute = (mTableWarningCount > 0) ?
- EFI_TEXT_ATTR (
- EFI_RED,
- ((OriginalAttribute&(BIT4|BIT5|BIT6))>>4)
- ) :
- OriginalAttribute;
-
- gST->ConOut->SetAttribute (gST->ConOut, PrintAttribute);
- }
- Print (L"\t%d Warning(s)\n", mTableWarningCount);
+ if (mConfig.ReportType == ReportSelected ||
+ mConfig.ReportType == ReportDumpBinFile) {
+ if (!mSelectedAcpiTable.Found) {
+ AcpiFatal (L"Requested ACPI Table not found.");
+ return EFI_SUCCESS;
+ }
+ }
- if (mConfig.ColourHighlighting) {
- gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute);
- }
+ if (mConfig.ConsistencyCheck) {
+ if (mConfig.ReportType == ReportSelected ||
+ mConfig.ReportType == ReportAll) {
+ AcpiInfo (L"Table Statistics:");
+ AcpiLog (
+ mTableErrorCount ? ACPI_BAD : ACPI_GOOD,
+ L"\t%d Error(s)",
+ mTableErrorCount);
+ AcpiLog (
+ mTableWarningCount ? ACPI_BAD : ACPI_GOOD,
+ L"\t%d Warning(s)\n",
+ mTableWarningCount);
}
}
+
return EFI_SUCCESS;
}
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/FieldFormatHelper.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/FieldFormatHelper.h
index 25c70652806c..b0072b68844c 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/FieldFormatHelper.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/FieldFormatHelper.h
@@ -8,90 +8,10 @@
#ifndef FIELD_FORMAT_HELPER_H_
#define FIELD_FORMAT_HELPER_H_
-#include <Library/UefiLib.h>
-#include <Uefi.h>
-
-/**
- This function traces 1 byte of data as specified in the format string.
-
- @param [in] Format The format string for tracing the data.
- @param [in] Ptr Pointer to the start of the buffer.
-**/
-static
-inline
-VOID
-EFIAPI
-DumpUint8 (
- IN CONST CHAR16* Format,
- IN UINT8* Ptr
- )
-{
- Print (Format, *Ptr);
-}
-
-/**
- This function traces 2 bytes of data as specified in the format string.
-
- @param [in] Format The format string for tracing the data.
- @param [in] Ptr Pointer to the start of the buffer.
-**/
-static
-inline
-VOID
-EFIAPI
-DumpUint16 (
- IN CONST CHAR16* Format,
- IN UINT8* Ptr
- )
-{
- Print (Format, *(UINT16*)Ptr);
-}
-
-/**
- This function traces 4 bytes of data as specified in the format string.
-
- @param [in] Format The format string for tracing the data.
- @param [in] Ptr Pointer to the start of the buffer.
-**/
-static
-inline
-VOID
-EFIAPI
-DumpUint32 (
- IN CONST CHAR16* Format,
- IN UINT8* Ptr
- )
-{
- Print (Format, *(UINT32*)Ptr);
-}
-
-/**
- This function traces 8 bytes of data as specified by the format string.
+#define INLINE inline
- @param [in] Format The format string for tracing the data.
- @param [in] Ptr Pointer to the start of the buffer.
-**/
-static
-inline
-VOID
-EFIAPI
-DumpUint64 (
- IN CONST CHAR16* Format,
- IN UINT8* Ptr
- )
-{
- // Some fields are not aligned and this causes alignment faults
- // on ARM platforms if the compiler generates LDRD instructions.
- // Perform word access so that LDRD instructions are not generated.
- UINT64 Val;
-
- Val = *(UINT32*)(Ptr + sizeof (UINT32));
-
- Val = LShiftU64(Val,32);
- Val |= (UINT64)*(UINT32*)Ptr;
-
- Print (Format, Val);
-}
+#include <Uefi.h>
+#include "AcpiViewLog.h"
/**
This function traces 3 characters which can be optionally
@@ -111,8 +31,8 @@ Dump3Chars (
IN UINT8* Ptr
)
{
- Print (
- (Format != NULL) ? Format : (CONST CHAR16*) L"%c%c%c",
+ AcpiInfo (
+ (Format != NULL) ? Format : L"%c%c%c",
Ptr[0],
Ptr[1],
Ptr[2]
@@ -137,8 +57,8 @@ Dump4Chars (
IN UINT8* Ptr
)
{
- Print (
- (Format != NULL) ? Format : (CONST CHAR16*) L"%c%c%c%c",
+ AcpiInfo(
+ (Format != NULL) ? Format : L"%c%c%c%c",
Ptr[0],
Ptr[1],
Ptr[2],
@@ -164,8 +84,8 @@ Dump6Chars (
IN UINT8* Ptr
)
{
- Print (
- (Format != NULL) ? Format : (CONST CHAR16*) L"%c%c%c%c%c%c",
+ AcpiInfo(
+ (Format != NULL) ? Format : L"%c%c%c%c%c%c",
Ptr[0],
Ptr[1],
Ptr[2],
@@ -193,8 +113,8 @@ Dump8Chars (
IN UINT8* Ptr
)
{
- Print (
- (Format != NULL) ? Format : (CONST CHAR16*) L"%c%c%c%c%c%c%c%c",
+ AcpiInfo(
+ (Format != NULL) ? Format : L"%c%c%c%c%c%c%c%c",
Ptr[0],
Ptr[1],
Ptr[2],
@@ -224,8 +144,8 @@ Dump12Chars (
IN UINT8* Ptr
)
{
- Print (
- (Format != NULL) ? Format : (CONST CHAR16*) L"%c%c%c%c%c%c%c%c%c%c%c%c",
+ AcpiInfo(
+ (Format != NULL) ? Format : L"%c%c%c%c%c%c%c%c%c%c%c%c",
Ptr[0],
Ptr[1],
Ptr[2],
--
2.24.1.windows.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v3 8/8] ShellPkg/AcpiView: Refactor table parsers
2020-07-14 17:45 [PATCH v3 0/8] ShellPkg/AcpiView: Refactor Error Logging Tomas Pilar (tpilar)
` (6 preceding siblings ...)
2020-07-14 17:45 ` [PATCH v3 7/8] ShellPkg/AcpiView: Refactor AcpiView Tomas Pilar (tpilar)
@ 2020-07-14 17:45 ` Tomas Pilar (tpilar)
7 siblings, 0 replies; 9+ messages in thread
From: Tomas Pilar (tpilar) @ 2020-07-14 17:45 UTC (permalink / raw)
To: devel; +Cc: nd, Sami.Mujawar, Ray Ni, Zhichao Gao
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 | 119 ++++-------
.../Parsers/Fadt/FadtParser.c | 54 ++---
.../Parsers/Gtdt/GtdtParser.c | 80 ++-----
.../Parsers/Iort/IortParser.c | 197 +++++++-----------
.../Parsers/Madt/MadtParser.c | 101 +++------
.../Parsers/Mcfg/McfgParser.c | 11 +-
.../Parsers/Pptt/PpttParser.c | 165 ++++-----------
.../Parsers/Rsdp/RsdpParser.c | 38 +---
.../Parsers/Slit/SlitParser.c | 124 +++++------
.../Parsers/Spcr/SpcrParser.c | 23 +-
.../Parsers/Srat/SratParser.c | 188 +++++------------
.../Parsers/Xsdt/XsdtParser.c | 96 +++------
15 files changed, 390 insertions(+), 914 deletions(-)
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
index 16b8d1f80bc2..1c8664910515 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
@@ -25,31 +25,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..f4c465a4a617 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,50 @@ 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 +213,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 +222,6 @@ ParseAcpiDbg2 (
Index = 0;
while (Index++ < *NumberDbgDeviceInfo) {
-
// Parse the Debug Device Information Structure header to obtain Length
ParseAcpi (
FALSE,
@@ -273,31 +235,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..473495651370 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
@@ -69,12 +69,10 @@ 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;
+
+ FirmwareControl = *(UINT32 *) Ptr;
+ AssertConstraint (L"ARM", FirmwareControl == 0);
#endif
}
@@ -94,12 +92,10 @@ 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;
+
+ XFirmwareControl = *(UINT32 *) Ptr;
+ AssertConstraint (L"ARM", XFirmwareControl == 0);
#endif
}
@@ -119,12 +115,10 @@ 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;
+
+ Flags = *(UINT32 *) Ptr;
+ AssertConstraint (L"ARM", Flags & HW_REDUCED_ACPI);
#endif
}
@@ -232,15 +226,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 +249,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 +275,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 +299,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..46d4f59f4386 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
@@ -44,15 +45,7 @@ ValidateGtBlockTimerCount (
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
- );
- }
+ AssertConstraint (L"ACPI", BlockTimerCount <= GT_BLOCK_TIMER_COUNT_MAX);
}
/**
@@ -70,18 +63,10 @@ ValidateGtFrameNumber (
IN VOID* Context
)
{
- UINT8 FrameNumber;
+ UINT8 GTFrameNumber;
- 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
- );
- }
+ GTFrameNumber = *Ptr;
+ AssertConstraint (L"ACPI", GTFrameNumber < GT_BLOCK_TIMER_COUNT_MAX);
}
/**
@@ -194,11 +179,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 +251,6 @@ ParseAcpiGtdt (
{
UINT32 Index;
UINT32 Offset;
- UINT8* TimerPtr;
if (!Trace) {
return;
@@ -287,17 +267,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 +284,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..c886d70e0914 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,10 @@ ValidateItsIdMappingCount (
IN VOID* Context
)
{
- if (*(UINT32*)Ptr != 0) {
- IncrementErrorCount ();
- Print (L"\nERROR: IORT ID Mapping count must be zero.");
- }
+ UINT32 ItsNodeIdMapping;
+
+ ItsNodeIdMapping = *(UINT32 *) Ptr;
+ AssertConstraint (L"ACPI", ItsNodeIdMapping == 0);
}
/**
@@ -71,10 +72,10 @@ ValidatePmcgIdMappingCount (
IN VOID* Context
)
{
- if (*(UINT32*)Ptr > 1) {
- IncrementErrorCount ();
- Print (L"\nERROR: IORT ID Mapping count must not be greater than 1.");
- }
+ UINT32 PmcgNodeIdMapping;
+
+ PmcgNodeIdMapping = *(UINT32 *) Ptr;
+ AssertConstraint (L"ACPI", PmcgNodeIdMapping <= 1);
}
/**
@@ -92,10 +93,10 @@ ValidateItsIdArrayReference (
IN VOID* Context
)
{
- if (*(UINT32*)Ptr != 0) {
- IncrementErrorCount ();
- Print (L"\nERROR: IORT ID Mapping offset must be zero.");
- }
+ UINT32 ItsNodeMappingArrayOffset;
+
+ ItsNodeMappingArrayOffset = *(UINT32 *) Ptr;
+ AssertConstraint (L"ACPI", ItsNodeMappingArrayOffset == 0);
}
/**
@@ -268,28 +269,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,
+ NULL,
Ptr + Offset,
Length - Offset,
- PARSER_PARAMS (IortNodeIdMappingParser)
- );
- Index++;
+ PARSER_PARAMS (IortNodeIdMappingParser));
}
}
@@ -313,7 +307,6 @@ DumpIortNodeSmmuV1V2 (
{
UINT32 Index;
UINT32 Offset;
- CHAR8 Buffer[50]; // Used for AsciiName param of ParseAcpi
ParseAcpi (
TRUE,
@@ -330,56 +323,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,
+ NULL,
Ptr + Offset,
Length - Offset,
- PARSER_PARAMS (InterruptArrayParser)
- );
- Index++;
+ 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 +416,6 @@ DumpIortNodeIts (
{
UINT32 Offset;
UINT32 Index;
- CHAR8 Buffer[80]; // Used for AsciiName param of ParseAcpi
Offset = ParseAcpi (
TRUE,
@@ -452,32 +429,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 +487,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 +597,6 @@ ParseAcpiIort (
{
UINT32 Offset;
UINT32 Index;
- UINT8* NodePtr;
if (!Trace) {
return;
@@ -648,16 +615,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 +631,7 @@ ParseAcpiIort (
FALSE,
0,
"IORT Node Header",
- NodePtr,
+ Ptr + Offset,
AcpiTableLength - Offset,
PARSER_PARAMS (IortNodeHeaderParser)
);
@@ -680,42 +642,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 to 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 +671,7 @@ ParseAcpiIort (
break;
case EFI_ACPI_IORT_TYPE_ROOT_COMPLEX:
DumpIortNodeRootComplex (
- NodePtr,
+ Ptr + Offset,
*IortNodeLength,
*IortIdMappingCount,
*IortIdMappingOffset
@@ -731,7 +679,7 @@ ParseAcpiIort (
break;
case EFI_ACPI_IORT_TYPE_SMMUv1v2:
DumpIortNodeSmmuV1V2 (
- NodePtr,
+ Ptr + Offset,
*IortNodeLength,
*IortIdMappingCount,
*IortIdMappingOffset
@@ -739,7 +687,7 @@ ParseAcpiIort (
break;
case EFI_ACPI_IORT_TYPE_SMMUv3:
DumpIortNodeSmmuV3 (
- NodePtr,
+ Ptr + Offset,
*IortNodeLength,
*IortIdMappingCount,
*IortIdMappingOffset
@@ -747,7 +695,7 @@ ParseAcpiIort (
break;
case EFI_ACPI_IORT_TYPE_PMCG:
DumpIortNodePmcg (
- NodePtr,
+ Ptr + Offset,
*IortNodeLength,
*IortIdMappingCount,
*IortIdMappingOffset
@@ -755,11 +703,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..83a987942ff2 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,10 @@ ValidateGICDSystemVectorBase (
IN VOID* Context
)
{
- if (*(UINT32*)Ptr != 0) {
- IncrementErrorCount ();
- Print (
- L"\nERROR: System Vector Base must be zero."
- );
- }
+ UINT32 GicdSystemVectorBase;
+
+ GicdSystemVectorBase = *(UINT32 *) Ptr;
+ AssertConstraint (L"ACPI", GicdSystemVectorBase == 0);
}
/**
@@ -63,36 +62,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 +214,6 @@ ParseAcpiMadt (
)
{
UINT32 Offset;
- UINT8* InterruptContollerPtr;
UINT32 GICDCount;
GICDCount = 0;
@@ -248,7 +230,6 @@ ParseAcpiMadt (
AcpiTableLength,
PARSER_PARAMS (MadtParser)
);
- InterruptContollerPtr = Ptr + Offset;
while (Offset < AcpiTableLength) {
// Parse Interrupt Controller Structure to obtain Length.
@@ -256,7 +237,7 @@ ParseAcpiMadt (
FALSE,
0,
NULL,
- InterruptContollerPtr,
+ Ptr + Offset,
AcpiTableLength - Offset,
PARSER_PARAMS (MadtInterruptControllerHeaderParser)
);
@@ -265,26 +246,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 +263,7 @@ ParseAcpiMadt (
TRUE,
2,
"GICC",
- InterruptContollerPtr,
+ Ptr + Offset,
*MadtInterruptControllerLength,
PARSER_PARAMS (GicCParser)
);
@@ -303,18 +272,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 +293,7 @@ ParseAcpiMadt (
TRUE,
2,
"GIC MSI Frame",
- InterruptContollerPtr,
+ Ptr + Offset,
*MadtInterruptControllerLength,
PARSER_PARAMS (GicMSIFrameParser)
);
@@ -338,7 +305,7 @@ ParseAcpiMadt (
TRUE,
2,
"GICR",
- InterruptContollerPtr,
+ Ptr + Offset,
*MadtInterruptControllerLength,
PARSER_PARAMS (GicRParser)
);
@@ -350,7 +317,7 @@ ParseAcpiMadt (
TRUE,
2,
"GIC ITS",
- InterruptContollerPtr,
+ Ptr + Offset,
*MadtInterruptControllerLength,
PARSER_PARAMS (GicITSParser)
);
@@ -358,17 +325,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..a7f5597ab139 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,21 @@ ValidateCacheNumberOfSets (
IN VOID* Context
)
{
- UINT32 NumberOfSets;
- NumberOfSets = *(UINT32*)Ptr;
+ UINT32 CacheNumberOfSets;
- if (NumberOfSets == 0) {
- IncrementErrorCount ();
- Print (L"\nERROR: Cache number of sets must be greater than 0");
- return;
- }
+ CacheNumberOfSets = *(UINT32*) Ptr;
+ 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 +73,10 @@ ValidateCacheAssociativity (
IN VOID* Context
)
{
- UINT8 Associativity;
- Associativity = *(UINT8*)Ptr;
+ UINT8 CacheAssociativity;
- if (Associativity == 0) {
- IncrementErrorCount ();
- Print (L"\nERROR: Cache associativity must be greater than 0");
- return;
- }
+ CacheAssociativity = *Ptr;
+ AssertConstraint (L"ACPI", CacheAssociativity != 0);
}
/**
@@ -120,25 +100,15 @@ 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;
- if ((LineSize & (LineSize - 1)) != 0) {
- IncrementErrorCount ();
- Print (L"\nERROR: The cache line size is not a power of 2.");
- }
+ CacheLineSize = *(UINT16 *) Ptr;
+ 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
}
@@ -161,16 +131,9 @@ ValidateCacheAttributes (
// 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;
- }
+ Attributes = *(UINT8 *) Ptr;
+ AssertConstraint (L"ACPI", BitFieldCountOnes32 (Attributes, 5, 7) == 0);
}
/**
@@ -255,7 +218,6 @@ DumpProcessorHierarchyNodeStructure (
{
UINT32 Offset;
UINT32 Index;
- CHAR16 Buffer[OUTPUT_FIELD_COLUMN_WIDTH];
Offset = ParseAcpi (
TRUE,
@@ -268,48 +230,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 +322,6 @@ ParseAcpiPptt (
)
{
UINT32 Offset;
- UINT8* ProcessorTopologyStructurePtr;
if (!Trace) {
return;
@@ -401,15 +336,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 +351,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..16dbab118795 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;
@@ -38,15 +39,8 @@ ValidateRsdtAddress (
// 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
- );
- }
+ RsdtAddr = *(UINT32 *) Ptr;
+ AssertConstraint (L"ARM", RsdtAddr == 0);
#endif
}
@@ -73,15 +67,8 @@ ValidateXsdtAddress (
// 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
- );
- }
+ XsdtAddr = *(UINT64 *) Ptr;
+ AssertConstraint (L"ARM", XsdtAddr != 0);
#endif
}
@@ -141,12 +128,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 +136,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 *)(UINTN) *XsdtAddress);
}
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
index cedfc8a71849..5f3207f5827c 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
@@ -29,9 +29,11 @@ STATIC CONST ACPI_PARSER SlitParser[] = {
};
/**
- Macro to get the value of a System Locality
+ Macro to get the value of a System Locality, simple
+ 2D variable size array item retrieval
**/
-#define SLIT_ELEMENT(Ptr, i, j) *(Ptr + (i * LocalityCount) + j)
+#define SLIT_ELEMENT(ArrayPtr, RowSize, Row, Column) \
+ *((ArrayPtr) + ((Row) * (RowSize)) + (Column))
/**
This function parses the ACPI SLIT table.
@@ -58,11 +60,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 +82,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 +98,65 @@ 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 ",
+ SLIT_ELEMENT(Ptr + Offset, LocalityCount, Index1, 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 (SLIT_ELEMENT(Ptr + Offset, LocalityCount, Index1, Index1) != 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 (
+ SLIT_ELEMENT(Ptr + Offset, LocalityCount, Index1, Index2) !=
+ SLIT_ELEMENT(Ptr + Offset, LocalityCount, Index2, 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..fa611d4b37c2 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;
@@ -37,15 +38,10 @@ ValidateInterruptType (
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
- );
- }
+ AssertConstraint (
+ L"ARM",
+ InterruptType ==
+ EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERRUPT_TYPE_GIC);
#endif
}
@@ -68,14 +64,7 @@ ValidateIrq (
UINT8 Irq;
Irq = *Ptr;
-
- if (Irq != 0) {
- IncrementErrorCount ();
- Print (
- L"\nERROR: Irq = %d. This must be zero on ARM Platforms\n",
- Irq
- );
- }
+ 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..71e6b61eeeac 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
@@ -37,10 +37,10 @@ ValidateSratReserved (
IN VOID* Context
)
{
- if (*(UINT32*)Ptr != 1) {
- IncrementErrorCount ();
- Print (L"\nERROR: Reserved should be 1 for backward compatibility.\n");
- }
+ UINT32 Reserved;
+
+ Reserved = *(UINT32 *) Ptr;
+ AssertConstraint (L"Backwards-Compatibility", Reserved == 1);
}
/**
@@ -59,18 +59,10 @@ ValidateSratDeviceHandleType (
IN VOID* Context
)
{
- UINT8 DeviceHandleType;
+ 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
- );
- }
+ AssertConstraint (L"ACPI", DeviceHandleType <= EFI_ACPI_6_3_PCI_DEVICE_HANDLE);
}
/**
@@ -87,9 +79,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 +94,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 +138,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 +183,7 @@ DumpSratApicProximity (
ProximityDomain = Ptr[0] | (Ptr[1] << 8) | (Ptr[2] << 16);
- Print (Format, ProximityDomain);
+ AcpiInfo ((CHAR16 *)Format, ProximityDomain);
}
/**
@@ -360,7 +321,6 @@ ParseAcpiSrat (
)
{
UINT32 Offset;
- UINT8* ResourcePtr;
UINT32 GicCAffinityIndex;
UINT32 GicITSAffinityIndex;
UINT32 GenericInitiatorAffinityIndex;
@@ -389,155 +349,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..77031183782d 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Xsdt/XsdtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Xsdt/XsdtParser.c
@@ -29,9 +29,7 @@ STATIC CONST ACPI_PARSER XsdtParser[] = {
**/
CONST ACPI_DESCRIPTION_HEADER_INFO *
EFIAPI
-GetAcpiXsdtHeaderInfo (
- VOID
-)
+GetAcpiXsdtHeaderInfo (VOID)
{
return &AcpiHdrInfo;
}
@@ -55,87 +53,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
^ permalink raw reply related [flat|nested] 9+ messages in thread