Hi Zhichao, Thank you for reviewing this patch.
I have incorporated the review comments and will send an updated patch. The HMAT spec issue has also been raised with ASWG. Regards, Sami Mujawar -----Original Message----- From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gao, Zhichao via groups.io Sent: 17 September 2020 03:18 AM To: devel@edk2.groups.io; Sami Mujawar <sami.muja...@arm.com> Cc: Ni, Ray <ray...@intel.com>; Marc Moisson-Franckhauser <marc.moisson-franckhau...@arm.com>; Guillaume Letellier <guillaume.letell...@arm.com>; Matteo Carlini <matteo.carl...@arm.com>; Ben Adderson <ben.adder...@arm.com>; nd <n...@arm.com> Subject: Re: [edk2-devel] [PATCH v2 1/1] ShellPkg/AcpiView: HMAT Parser Hi Sami, Please see below comment. > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sami > Mujawar > Sent: Tuesday, August 25, 2020 7:07 PM > To: devel@edk2.groups.io > Cc: Sami Mujawar <sami.muja...@arm.com>; Ni, Ray <ray...@intel.com>; Gao, > Zhichao <zhichao....@intel.com>; marc.moisson-franckhau...@arm.com; > guillaume.letell...@arm.com; matteo.carl...@arm.com; > ben.adder...@arm.com; n...@arm.com > Subject: [edk2-devel] [PATCH v2 1/1] ShellPkg/AcpiView: HMAT Parser > > From: Marc Moisson-Franckhauser <marc.moisson-franckhau...@arm.com> > > Add a new parser for the Heterogeneous Memory Attribute Table. The parser > also validates some fields for this table. > > The HMAT table is used to describe the memory attributes such as memory side > cache attributes and bandwidth and latency details related to memory proximity > domains. The info in the HMAT table can be used by an operating system for > optimisation. > > Signed-off-by: Marc Moisson-Franckhauser <marc.moisson- > franckhau...@arm.com> > Signed-off-by: Sami Mujawar <sami.muja...@arm.com> > --- > > The changes can be seen at: > https://github.com/samimujawar/edk2/tree/833_hmat_parser_v2 > > Notes: > v2: > - Fixed minor output formatting in DumpCacheAttributes() [SAMI] > > ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h > | 28 +- > ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.h > | 4 > +- > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Hmat/HmatParser.c > | 641 ++++++++++++++++++++ > > ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c > | 3 +- > > ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.i > nf | 3 +- > > ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib. > uni | 3 +- > 6 files changed, 676 insertions(+), 6 deletions(-) > > diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h > index > f81ccac7e118378aa185db4b625e5bcd75f78347..969cc0b371852f01f30c88dc506 > 374a459c9c19e 100644 > --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h > +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h > @@ -1,7 +1,7 @@ > /** @file > Header file for ACPI parser > > - Copyright (c) 2016 - 2020, ARM Limited. All rights reserved. > + Copyright (c) 2016 - 2020, Arm Limited. All rights reserved. > SPDX-License-Identifier: BSD-2-Clause-Patent **/ > > @@ -592,6 +592,32 @@ ParseAcpiGtdt ( > ); > > /** > + This function parses the ACPI HMAT table. > + When trace is enabled this function parses the HMAT table and traces > + the ACPI table fields. > + > + This function parses the following HMAT structures: > + - Memory Proximity Domain Attributes Structure (Type 0) > + - System Locality Latency and Bandwidth Info Structure (Type 1) > + - Memory Side Cache Info structure (Type 2) > + > + This function also performs validation of the ACPI table fields. > + > + @param [in] Trace If TRUE, trace the ACPI fields. > + @param [in] Ptr Pointer to the start of the buffer. > + @param [in] AcpiTableLength Length of the ACPI table. > + @param [in] AcpiTableRevision Revision of the ACPI table. > +**/ > +VOID > +EFIAPI > +ParseAcpiHmat ( > + IN BOOLEAN Trace, > + IN UINT8* Ptr, > + IN UINT32 AcpiTableLength, > + IN UINT8 AcpiTableRevision > + ); > + > +/** > This function parses the ACPI IORT table. > When trace is enabled this function parses the IORT table and > traces the ACPI fields. > diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.h > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.h > index > 4f92596b90a6ee422d8d0959881015ffd3de4da0..f8e8b5979f3be041bbc8d17042 > b2db8e0b73f205 100644 > --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.h > +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.h > @@ -1,7 +1,7 @@ > /** @file > Header file for ACPI table parser > > - Copyright (c) 2016 - 2018, ARM Limited. All rights reserved. > + Copyright (c) 2016 - 2018, Arm Limited. All rights reserved. > SPDX-License-Identifier: BSD-2-Clause-Patent **/ > > @@ -11,7 +11,7 @@ > /** > The maximum number of ACPI table parsers. > */ > -#define MAX_ACPI_TABLE_PARSERS 16 > +#define MAX_ACPI_TABLE_PARSERS 32 > > /** An invalid/NULL signature value. > */ > diff --git > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Hmat/HmatParser.c > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Hmat/HmatParser.c > new file mode 100644 > index > 0000000000000000000000000000000000000000..57b93cca6ba24ed77f9dcd7bf > 2f45ba9f0cb9f26 > --- /dev/null > +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Hmat/HmatPars > +++ er.c > @@ -0,0 +1,641 @@ > +/** @file > + HMAT table parser > + > + Copyright (c) 2020, Arm Limited. > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > + @par Reference(s): > + - ACPI 6.3 Specification - January 2019 > + > + @par Glossary: > + - MPDA - Memory Proximity Domain Attributes > + - SLLBI - System Locality Latency and Bandwidth Information > + - MSCI - Memory Side Cache Information > + - Dom - Domain > +**/ > + > +#include <Library/PrintLib.h> > +#include <Library/BaseLib.h> > +#include <Library/UefiLib.h> > +#include "AcpiParser.h" > +#include "AcpiView.h" > + > +// Maximum Memory Domain matrix print size. > +#define MAX_MEMORY_DOMAIN_TARGET_PRINT_MATRIX 12 > + > +// Local variables > +STATIC CONST UINT16* HmatStructureType; STATIC CONST UINT32* > +HmatStructureLength; > + > +STATIC CONST UINT32* NumberInitiatorProximityDomain; STATIC CONST > +UINT32* NumberTargetProximityDomain; STATIC CONST > +EFI_ACPI_6_3_HMAT_STRUCTURE_SYSTEM_LOCALITY_LATENCY_AND_BAND > WIDTH_INFO_ > +FLAGS* > +SllbiFlags; > + > +STATIC CONST UINT8* SllbiDataType; > +STATIC CONST UINT32* NumberSMBIOSHandles; > + > +STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo; > + > +/** > + Names of System Locality Latency Bandwidth Information (SLLBI) data > +types **/ STATIC CONST CHAR16* SllbiNames[] = { > + L"Access %sLatency%s", > + L"Read %sLatency%s", > + L"Write %sLatency%s", > + L"Access %sBandwidth%s", > + L"Read %sBandwidth%s", > + L"Write %sBandwidth%s" > +}; > + > +/** > + This function validates the Cache Attributes field. > + > + @param [in] Ptr Pointer to the start of the field data. > + @param [in] Context Pointer to context specific information e.g. this > + could be a pointer to the ACPI table header. > +**/ > +STATIC > +VOID > +EFIAPI > +ValidateCacheAttributes ( > + IN UINT8* Ptr, > + IN VOID* Context > + ) > +{ > + > EFI_ACPI_6_3_HMAT_STRUCTURE_MEMORY_SIDE_CACHE_INFO_CACHE_ATTR > IBUTES* > + Attributes; > + > + Attributes = > + > + > (EFI_ACPI_6_3_HMAT_STRUCTURE_MEMORY_SIDE_CACHE_INFO_CACHE_ATT > RIBUTES*) > + Ptr; > + > + if (Attributes->TotalCacheLevels > 0x3) { > + IncrementErrorCount (); > + Print ( > + L"\nERROR: Attributes bits [3:0] have invalid value: 0x%x", > + Attributes->TotalCacheLevels > + ); > + } > + if (Attributes->CacheLevel > 0x3) { > + IncrementErrorCount (); > + Print ( > + L"\nERROR: Attributes bits [7:4] have invalid value: 0x%x", > + Attributes->CacheLevel > + ); > + } > + if (Attributes->CacheAssociativity > 0x2) { > + IncrementErrorCount (); > + Print ( > + L"\nERROR: Attributes bits [11:8] have invalid value: 0x%x", > + Attributes->CacheAssociativity > + ); > + } > + if (Attributes->WritePolicy > 0x2) { > + IncrementErrorCount (); > + Print ( > + L"\nERROR: Attributes bits [15:12] have invalid value: 0x%x", > + Attributes->WritePolicy > + ); > + } > +} > + > +/** > + Dumps the cache attributes field > + > + @param [in] Format Optional format string for tracing the data. > + @param [in] Ptr Pointer to the start of the buffer. > +**/ > +STATIC > +VOID > +EFIAPI > +DumpCacheAttributes ( > + IN CONST CHAR16* Format OPTIONAL, > + IN UINT8* Ptr > + ) > +{ > + > EFI_ACPI_6_3_HMAT_STRUCTURE_MEMORY_SIDE_CACHE_INFO_CACHE_ATTR > IBUTES* > + Attributes; > + > + Attributes = > + > + > (EFI_ACPI_6_3_HMAT_STRUCTURE_MEMORY_SIDE_CACHE_INFO_CACHE_ATT > RIBUTES*) > + Ptr; > + > + Print (L"\n"); > + PrintFieldName (4, L"Total Cache Levels"); > + Print (L"%d\n", Attributes->TotalCacheLevels); > + PrintFieldName (4, L"Cache Level"); > + Print (L"%d\n", Attributes->CacheLevel); > + PrintFieldName (4, L"Cache Associativity"); > + Print (L"%d\n", Attributes->CacheAssociativity); > + PrintFieldName (4, L"Write Policy"); > + Print (L"%d\n", Attributes->WritePolicy); > + PrintFieldName (4, L"Cache Line Size"); > + Print (L"%d\n", Attributes->CacheLineSize); } > + > +/** > + An ACPI_PARSER array describing the ACPI HMAT Table. > +*/ > +STATIC CONST ACPI_PARSER HmatParser[] = { > + PARSE_ACPI_HEADER (&AcpiHdrInfo), > + {L"Reserved", 4, 36, NULL, NULL, NULL, NULL, NULL} }; > + > +/** > + An ACPI_PARSER array describing the HMAT structure header. > +*/ > +STATIC CONST ACPI_PARSER HmatStructureHeaderParser[] = { > + {L"Type", 2, 0, NULL, NULL, (VOID**)&HmatStructureType, NULL, NULL}, > + {L"Reserved", 2, 2, NULL, NULL, NULL, NULL, NULL}, > + {L"Length", 4, 4, NULL, NULL, (VOID**)&HmatStructureLength, NULL, > +NULL} }; > + > +/** > + An ACPI PARSER array describing the Memory Proximity Domain > +Attributes > + Structure - Type 0. > +*/ > +STATIC CONST ACPI_PARSER MemProximityDomainAttributeParser[] = { > + {L"Type", 2, 0, L"0x%x", NULL, NULL, NULL, NULL}, > + {L"Reserved", 2, 2, L"0x%x", NULL, NULL, NULL, NULL}, > + {L"Length", 4, 4, L"%d", NULL, NULL, NULL, NULL}, > + {L"Flags", 2, 8, L"0x%x", NULL, NULL, NULL, NULL}, > + {L"Reserved", 2, 10, L"0x%x", NULL, NULL, NULL, NULL}, > + {L"Proximity Dom for initiator", 4, 12, L"0x%x", NULL, NULL, NULL, > +NULL}, > + {L"Proximity Dom for memory", 4, 16, L"0x%x", NULL, NULL, NULL, > +NULL}, > + {L"Reserved", 4, 20, L"0x%x", NULL, NULL, NULL, NULL}, > + {L"Reserved", 8, 24, L"0x%lx", NULL, NULL, NULL, NULL}, > + {L"Reserved", 8, 32, L"0x%lx", NULL, NULL, NULL, NULL} }; > + > +/** > + An ACPI PARSER array describing the System Locality Latency and > +Bandwidth > + Information Structure - Type 1. > +*/ > +STATIC CONST ACPI_PARSER > +SllbiParser[] = { > + {L"Type", 2, 0, L"0x%x", NULL, NULL, NULL, NULL}, > + {L"Reserved", 2, 2, L"0x%x", NULL, NULL, NULL, NULL}, > + {L"Length", 4, 4, L"%d", NULL, NULL, NULL, NULL}, > + {L"Flags", 1, 8, L"0x%x", NULL, (VOID**)&SllbiFlags, NULL, NULL}, > + {L"Data type", 1, 9, L"0x%x", NULL, (VOID**)&SllbiDataType, NULL, > +NULL}, > + {L"Reserved", 2, 10, L"0x%x", NULL, NULL, NULL, NULL}, > + {L"Initiator Proximity Dom Count", 4, 12, L"%d", NULL, > + (VOID**)&NumberInitiatorProximityDomain, NULL, NULL}, > + {L"Target Proximity Dom Count", 4, 16, L"%d", NULL, > + (VOID**)&NumberTargetProximityDomain, NULL, NULL}, > + {L"Reserved", 4, 20, L"0x%x", NULL, NULL, NULL, NULL}, > + {L"Entry Base Unit", 8, 24, L"0x%lx", NULL, NULL, NULL, NULL} > + // initiator Proximity Domain list ... > + // target Proximity Domain list ... > + // Latency/Bandwidth matrix ... > +}; > + > +/** > + An ACPI PARSER array describing the Memory Side Cache Information > + Structure - Type 2. > +*/ > +STATIC CONST ACPI_PARSER MemSideCacheInfoParser[] = { > + {L"Type", 2, 0, L"0x%x", NULL, NULL, NULL, NULL}, > + {L"Reserved", 2, 2, L"0x%x", NULL, NULL, NULL, NULL}, > + {L"Length", 4, 4, L"%d", NULL, NULL, NULL, NULL}, > + {L"Proximity Dom for memory", 4, 8, L"0x%x", NULL, NULL, NULL, NULL}, > + {L"Reserved", 4, 12, L"0x%x", NULL, NULL, NULL, NULL}, > + {L"Memory Side Cache Size", 8, 16, L"0x%lx", NULL, NULL, NULL, NULL}, > + {L"Cache Attributes", 4, 24, NULL, DumpCacheAttributes, NULL, > + ValidateCacheAttributes, NULL}, > + {L"Reserved", 2, 28, L"0x%x", NULL, NULL, NULL, NULL}, > + {L"SMBIOS Handle Count", 2, 30, L"%d", NULL, > + (VOID**)&NumberSMBIOSHandles, NULL, NULL} > + // SMBIOS handles List ... > +}; > + > +/** > + This function parses the Memory Proximity Domain Attributes > + Structure (Type 0). > + > + @param [in] Ptr Pointer to the start of the Memory Proximity Domain > + Attributes Structure data. > + @param [in] Length Length of the Memory Proximity Domain Attributes > + Structure. > +**/ > +STATIC > +VOID > +DumpMpda ( > + IN UINT8* Ptr, > + IN UINT8 Length > + ) > +{ > + ParseAcpi ( > + TRUE, > + 2, > + "Memory Proximity Domain Attributes Structure", > + Ptr, > + Length, > + PARSER_PARAMS (MemProximityDomainAttributeParser) > + ); > +} > + > +/** > + This function parses the System Locality Latency and Bandwidth > +Information > + Structure (Type 1). > + > + @param [in] Ptr Pointer to the start of the System Locality Latency and > + Bandwidth Information Structure data. > + @param [in] Length Length of the System Locality Latency and Bandwidth > + Information Structure. > +**/ > +STATIC > +VOID > +DumpSllbi ( > + IN UINT8* Ptr, > + IN UINT8 Length > + ) > +{ > + CONST UINT32* InitiatorProximityDomainList; > + CONST UINT32* TargetProximityDomainList; > + CONST UINT16* LatencyBandwidthMatrix; > + UINT32 Offset; > + CHAR16 Buffer[OUTPUT_FIELD_COLUMN_WIDTH]; > + CHAR16 SecondBuffer[OUTPUT_FIELD_COLUMN_WIDTH]; > + UINT32 RequiredTableSize; > + UINT32 Index; > + UINT32 IndexInitiator; > + UINT32 IndexTarget; > + > + Offset = ParseAcpi ( > + TRUE, > + 2, > + "System Locality Latency and Bandwidth Information Structure", > + Ptr, > + Length, > + PARSER_PARAMS (SllbiParser) > + ); > + > + // Check if the values used to control the parsing logic have been > + // successfully read. > + if ((SllbiFlags == NULL) || > + (SllbiDataType == NULL) || > + (NumberInitiatorProximityDomain == NULL) || > + (NumberTargetProximityDomain == NULL)) { > + IncrementErrorCount (); > + Print ( > + L"ERROR: Insufficient remaining table buffer length to read the " \ > + L"SLLBI structure header. Length = %d.\n", > + Length > + ); > + return; > + } > + > + RequiredTableSize = (*NumberInitiatorProximityDomain * sizeof (UINT32)) + > + (*NumberTargetProximityDomain * sizeof (UINT32)) + > + (*NumberInitiatorProximityDomain * > + *NumberTargetProximityDomain * sizeof (UINT16)) + > + Offset; > + > + if (RequiredTableSize > Length) { > + IncrementErrorCount (); > + Print ( > + L"ERROR: Insufficient System Locality Latency and Bandwidth" \ > + L"Information Structure length. TableLength = %d. " \ > + L"RequiredTableLength = %d.\n", > + Length, > + RequiredTableSize > + ); > + return; > + } > + > + InitiatorProximityDomainList = (UINT32*) (Ptr + Offset); > + TargetProximityDomainList = InitiatorProximityDomainList + > + *NumberInitiatorProximityDomain; > + LatencyBandwidthMatrix = (UINT16*) (TargetProximityDomainList + > + *NumberTargetProximityDomain); > + > + // Display each element of the Initiator Proximity Domain list for > + (Index = 0; Index < *NumberInitiatorProximityDomain; Index++) { > + UnicodeSPrint ( > + Buffer, > + sizeof (Buffer), > + L"Initiator Proximity Dom [%d]", > + Index > + ); > + > + PrintFieldName (4, Buffer); > + Print ( > + L"0x%x\n", > + InitiatorProximityDomainList[Index] > + ); > + } > + > + // Display each element of the Target Proximity Domain list for > + (Index = 0; Index < *NumberTargetProximityDomain; Index++) { > + UnicodeSPrint ( > + Buffer, > + sizeof (Buffer), > + L"Target Proximity Dom [%d]", > + Index > + ); > + > + PrintFieldName (4, Buffer); > + Print ( > + L"0x%x\n", > + TargetProximityDomainList[Index] > + ); > + } > + > + // Create base name depending on Data Type in this Structure if > + (*SllbiDataType >= ARRAY_SIZE (SllbiNames)) { > + IncrementErrorCount (); > + Print (L"Error: Unkown Data Type. DataType = 0x%x.\n", *SllbiDataType); > + return; > + } > + StrCpyS (Buffer, sizeof (Buffer), SllbiNames[*SllbiDataType]); > + > + // Adjust base name depending on Memory Hierarchy in this Structure > + switch (SllbiFlags->MemoryHierarchy) { > + case 0: { > + UnicodeSPrint ( > + SecondBuffer, > + sizeof (SecondBuffer), > + Buffer, > + L"", > + L"%s" > + ); > + break; > + } > + > + case 1: > + case 2: > + case 3: { > + UnicodeSPrint ( > + SecondBuffer, > + sizeof (SecondBuffer), > + Buffer, > + L"Hit ", > + L"%s" > + ); > + break; > + } > + > + default: { > + IncrementErrorCount (); > + Print ( > + L"Error: Invalid Memory Hierarchy. MemoryHierarchy = %d.\n", > + SllbiFlags->MemoryHierarchy > + ); > + return; > + } I don't think switch case need the '{' and '}' to indicated the block section. And refer to APCI spec 6.3 table 5-146. It says for memory hierarchy == 1, 2, 3 or 4, the data type is 'hit'. I am not sure if it is a spec mistake in 'Flags' or 'Data Type'. > + } // switch > + > + if (IndexInitiator <= MAX_MEMORY_DOMAIN_TARGET_PRINT_MATRIX) { The IndexInitiator is not initialized before use it. And it should not be the Initiator, it should be the target as the horizontal axis. The IndexInitiator should be *NumberTargetProximityDomain. If I am wrong, please feel free to point out. > + // Display the latency/bandwidth matrix as a matrix > + UnicodeSPrint ( > + Buffer, > + sizeof (Buffer), > + SecondBuffer, > + L"" > + ); > + PrintFieldName (4, Buffer); > + Print (L"\n\n Initiator ->\nTarget\n|\nV\n"); As mentioned above, the Initiator and Target should be exchanged. > + Print (L" |"); > + > + for (IndexTarget = 0; > + IndexTarget < *NumberTargetProximityDomain; > + IndexTarget++) { > + Print (L" %2d", IndexTarget); > + } > + > + Print (L"\n---+"); > + for (IndexTarget = 0; > + IndexTarget < *NumberTargetProximityDomain; > + IndexTarget++) { > + Print (L"-------"); > + } > + Print (L"\n"); > + > + for (IndexInitiator = 0; > + IndexInitiator < *NumberInitiatorProximityDomain; > + IndexInitiator++) { > + Print (L"%2d |", IndexInitiator); > + for (IndexTarget = 0; > + IndexTarget < *NumberTargetProximityDomain; > + IndexTarget++) { > + Print ( > + L" %5d", > + LatencyBandwidthMatrix[IndexInitiator * > + (*NumberTargetProximityDomain + > IndexTarget)] Incorrect index value for the array. It should be "IndexInitiator * (*NumberTargetProximityDomain) + IndexTarget". > + ); > + } // for Target > + Print (L"\n"); > + } // for Initiator > + Print (L"\n"); > + } else { > + // Display the latency/bandwidth matrix as a list > + UnicodeSPrint ( > + Buffer, > + sizeof (Buffer), > + SecondBuffer, > + L" [%d][%d]" > + ); > + for (IndexInitiator = 0; > + IndexInitiator < *NumberInitiatorProximityDomain; > + IndexInitiator++) { > + for (IndexTarget = 0; > + IndexTarget < *NumberTargetProximityDomain; > + IndexTarget++) { > + UnicodeSPrint ( > + Buffer, > + sizeof (Buffer), > + SecondBuffer, > + IndexInitiator, > + IndexTarget > + ); > + > + PrintFieldName (4, Buffer); > + Print ( > + L"%d\n", > + LatencyBandwidthMatrix[IndexInitiator * > + (*NumberTargetProximityDomain + > IndexTarget)] > + ); > + } // for Target > + } // for Initiator > + } > +} > + > +/** > + This function parses the Memory Side Cache Information Structure (Type 2). > + > + @param [in] Ptr Pointer to the start of the Memory Side Cache > Information > + Structure data. > + @param [in] Length Length of the Memory Side Cache Information Structure. > +**/ > +STATIC > +VOID > +DumpMsci ( > + IN UINT8* Ptr, > + IN UINT8 Length > + ) > +{ > + CONST UINT16* SMBIOSHandlesList; > + CHAR16 Buffer[OUTPUT_FIELD_COLUMN_WIDTH]; > + UINT32 Offset; > + UINT16 Index; > + > + Offset = ParseAcpi ( > + TRUE, > + 2, > + "Memory Side Cache Information Structure", > + Ptr, > + Length, > + PARSER_PARAMS (MemSideCacheInfoParser) > + ); > + > + // Check if the values used to control the parsing logic have been > + // successfully read. > + if (NumberSMBIOSHandles == NULL) { > + IncrementErrorCount (); > + Print ( > + L"ERROR: Insufficient remaining table buffer length to read the " \ > + L"MSCI structure header. Length = %d.\n", > + Length > + ); > + return; > + } > + > + if ((*NumberSMBIOSHandles * sizeof (UINT16)) > (Length - Offset)) { > + IncrementErrorCount (); > + Print ( > + L"ERROR: Invalid Number of SMBIOS Handles. SMBIOSHandlesCount = %d." > \ > + L"RemainingBufferLength = %d.\n", > + *NumberSMBIOSHandles, > + Length - Offset > + ); > + return; > + } > + > + SMBIOSHandlesList = (UINT16*) (Ptr + Offset); > + > + for (Index = 0; Index < *NumberSMBIOSHandles; Index++) { > + UnicodeSPrint ( > + Buffer, > + sizeof (Buffer), > + L"SMBIOS Handles [%d]", > + Index > + ); > + > + PrintFieldName (4, Buffer); > + Print ( > + L"0x%x\n", > + SMBIOSHandlesList[Index] > + ); > + } > +} The three dump functions use UINT8 to transfer the length. It should be UINT32 refer to the caller and the usage in the function. > + > +/** > + This function parses the ACPI HMAT table. > + When trace is enabled this function parses the HMAT table and > + traces the ACPI table fields. > + > + This function parses the following HMAT structures: > + - Memory Proximity Domain Attributes Structure (Type 0) > + - System Locality Latency and Bandwidth Info Structure (Type 1) > + - Memory Side Cache Info structure (Type 2) > + > + This function also performs validation of the ACPI table fields. > + > + @param [in] Trace If TRUE, trace the ACPI fields. > + @param [in] Ptr Pointer to the start of the buffer. > + @param [in] AcpiTableLength Length of the ACPI table. > + @param [in] AcpiTableRevision Revision of the ACPI table. > +**/ > +VOID > +EFIAPI > +ParseAcpiHmat ( > + IN BOOLEAN Trace, > + IN UINT8* Ptr, > + IN UINT32 AcpiTableLength, > + IN UINT8 AcpiTableRevision > + ) > +{ > + UINT32 Offset; > + UINT8* HmatStructurePtr; > + > + if (!Trace) { > + return; > + } > + > + Offset = ParseAcpi ( > + Trace, > + 0, > + "HMAT", > + Ptr, > + AcpiTableLength, > + PARSER_PARAMS (HmatParser) > + ); > + > + HmatStructurePtr = Ptr + Offset; > + > + while (Offset < AcpiTableLength) { > + // Parse HMAT Structure Header to obtain Type and Length. > + ParseAcpi ( > + FALSE, > + 0, > + NULL, > + HmatStructurePtr, > + AcpiTableLength - Offset, > + PARSER_PARAMS (HmatStructureHeaderParser) > + ); > + > + // Check if the values used to control the parsing logic have been > + // successfully read. > + if ((HmatStructureType == NULL) || > + (HmatStructureLength == NULL)) { > + IncrementErrorCount (); > + Print ( > + L"ERROR: Insufficient remaining table buffer length to read the " \ > + L"HMAT structure header. Length = %d.\n", > + AcpiTableLength - Offset > + ); > + return; > + } > + > + switch (*HmatStructureType) { > + case > EFI_ACPI_6_3_HMAT_TYPE_MEMORY_PROXIMITY_DOMAIN_ATTRIBUTES: { > + DumpMpda ( > + HmatStructurePtr, > + *HmatStructureLength > + ); > + break; > + } > + > + case > EFI_ACPI_6_3_HMAT_TYPE_SYSTEM_LOCALITY_LATENCY_AND_BANDWIDTH_I > NFO: { > + DumpSllbi ( > + HmatStructurePtr, > + *HmatStructureLength > + ); > + break; > + } > + > + case EFI_ACPI_6_3_HMAT_TYPE_MEMORY_SIDE_CACHE_INFO: { > + DumpMsci ( > + HmatStructurePtr, > + *HmatStructureLength > + ); > + break; > + } > + > + default: { > + IncrementErrorCount (); > + Print ( > + L"ERROR: Unknown HMAT structure:" > + L" Type = %d, Length = %d\n", > + *HmatStructureType, > + *HmatStructureLength > + ); > + } Redundant '{' and '}' for switch case and missing break for 'default'. Please test the patch before send to community. Thanks, Zhichao > + } > + > + HmatStructurePtr += *HmatStructureLength; > + Offset += *HmatStructureLength; > + } // while > +} > diff --git > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLi > b.c > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLi > b.c > index > d2f26ff89f12e596702281c38ab0de3729aa68e4..c9e3054a5c413ba95e6420ce3d > 02f0d954e99d90 100644 > --- > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLi > b.c > +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm > +++ andLib.c > @@ -1,7 +1,7 @@ > /** @file > Main file for 'acpiview' Shell command function. > > - Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.<BR> > + Copyright (c) 2016 - 2020, Arm Limited. All rights reserved.<BR> > SPDX-License-Identifier: BSD-2-Clause-Patent **/ > > @@ -53,6 +53,7 @@ ACPI_TABLE_PARSER ParserList[] = { > {EFI_ACPI_6_3_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE, > ParseAcpiFacs}, > {EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE, ParseAcpiFadt}, > {EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE, > ParseAcpiGtdt}, > + {EFI_ACPI_6_3_HETEROGENEOUS_MEMORY_ATTRIBUTE_TABLE_SIGNATURE, > + ParseAcpiHmat}, > {EFI_ACPI_6_2_IO_REMAPPING_TABLE_SIGNATURE, ParseAcpiIort}, > {EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE, > ParseAcpiMadt}, > > {EFI_ACPI_6_2_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BA > SE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE, > diff --git > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLi > b.inf > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLi > b.inf > index > 91459f9ec632635ee453c5ef46f67445cd9eee0c..e970c4259c143b5b06fb1a759d5 > 819e7cb93e749 100644 > --- > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLi > b.inf > +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm > +++ andLib.inf > @@ -1,7 +1,7 @@ > ## @file > # Provides Shell 'acpiview' command functions # -# Copyright (c) 2016 - > 2020, > ARM Limited. All rights reserved.<BR> > +# Copyright (c) 2016 - 2020, Arm Limited. All rights reserved.<BR> > # > # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -33,6 +33,7 @@ > [Sources.common] > Parsers/Facs/FacsParser.c > Parsers/Fadt/FadtParser.c > Parsers/Gtdt/GtdtParser.c > + Parsers/Hmat/HmatParser.c > Parsers/Iort/IortParser.c > Parsers/Madt/MadtParser.c > Parsers/Madt/MadtParser.h > diff --git > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLi > b.uni > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLi > b.uni > index > 7cd43d0518fd0a23dc547a5cab0d08b62602a113..6b0537b47a751184e8a68a653 > 0aa85eb28ea33ba 100644 > --- > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLi > b.uni > +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm > +++ andLib.uni > @@ -1,6 +1,6 @@ > // /** > // > -// Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.<BR> > +// Copyright (c) 2016 - 2020, Arm Limited. All rights reserved.<BR> > // SPDX-License-Identifier: BSD-2-Clause-Patent // // Module Name: > @@ -84,6 +84,7 @@ > " DSDT - Differentiated System Description Table\r\n" > " FACP - Fixed ACPI Description Table (FADT)\r\n" > " GTDT - Generic Timer Description Table\r\n" > +" HMAT - Heterogeneous Memory Attributes Table\r\n" > " IORT - IO Remapping Table\r\n" > " MCFG - Memory Mapped Config Space Base Address Description > Table\r\n" > " PPTT - Processor Properties Topology Table\r\n" > -- > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#65401): https://edk2.groups.io/g/devel/message/65401 Mute This Topic: https://groups.io/mt/76404316/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
