* [PATCH v3 0/7] Support ACPI 6.4 PPTT changes
@ 2021-12-08 16:06 Chris Jones
2021-12-08 16:06 ` [PATCH v3 1/7] MdePkg: Add missing Cache ID (in)valid define Chris Jones
` (8 more replies)
0 siblings, 9 replies; 12+ messages in thread
From: Chris Jones @ 2021-12-08 16:06 UTC (permalink / raw)
To: devel
Cc: michael.d.kinney, gaoliming, zhiguang.liu, ray.ni, zhichao.gao,
Alexei.Fedorov, Sami.Mujawar, nd
Bugzilla: 3697 (https://bugzilla.tianocore.org/show_bug.cgi?id=3697)
This patch series updates the Acpiview PPTT parser and DynamicTablesPkg
PPTT generator to support ACPI 6.4. This consists of two main changes:
- The addition of the 'Cache ID' field.
- The removal of the PPTT ID (type 2) structure.
In addition add two 'Cache ID' defines and remove the type 2 PPTT
structure from Acpi64.h as these changes were missing when Acpi64.h was
introduced.
Changes since v2:
1. Added mantis ID for PPTT type 2 structure removal on relevant commit
messages. No change to code.
2. Applied uncrustify to all patches. No other changes to code.
Changes since v1:
1. Patch 3/7 ShellPkg:
- Changed PPTT ID error message to say "removed" instead of "deprecated".
2. Patch 4/7 ShellPkg:
- Fixed a bug where 'CacheFlags' and 'CacheId' were only set after the
validation function had finished. Instead set them inside the
validation function using the first 'Ptr' parameter.
3. Patch 7/7 DynamicTablesPkg:
- Make IsCacheIdUnique() return BOOLEAN instead of EFI_STATUS.
- Added a missing space to the definition of 'IsCacheIdUnique'.
The changes can be seen at: https://github.com/chris-jones-arm/edk2/tree/1632_64_acpi_cache_id_v3
Chris Jones (7):
MdePkg: Add missing Cache ID (in)valid define
MdePkg: Remove PPTT ID type structure
ShellPkg: Update Acpiview PPTT parser to ACPI 6.4
ShellPkg: Add Cache ID to PPTT parser
DynamicTablesPkg: Remove PPTT ID structure from ACPI 6.4 generator
DynamicTablesPkg: Update PPTT generator to ACPI 6.4
DynamicTablesPkg: Add CacheId to PPTT generator
.../Include/ArmNameSpaceObjects.h | 27 +-
.../Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c | 296 +++++++-----------
.../Acpi/Arm/AcpiPpttLibArm/PpttGenerator.h | 4 -
MdePkg/Include/IndustryStandard/Acpi64.h | 18 +-
.../Parsers/Pptt/PpttParser.c | 214 +++++++++----
.../UefiShellAcpiViewCommandLib.c | 2 +-
6 files changed, 272 insertions(+), 289 deletions(-)
--
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 1/7] MdePkg: Add missing Cache ID (in)valid define
2021-12-08 16:06 [PATCH v3 0/7] Support ACPI 6.4 PPTT changes Chris Jones
@ 2021-12-08 16:06 ` Chris Jones
2021-12-08 16:06 ` [PATCH v3 2/7] MdePkg: Remove PPTT ID type structure Chris Jones
` (7 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Chris Jones @ 2021-12-08 16:06 UTC (permalink / raw)
To: devel
Cc: michael.d.kinney, gaoliming, zhiguang.liu, ray.ni, zhichao.gao,
Alexei.Fedorov, Sami.Mujawar, nd
Bugzilla: 3697 (https://bugzilla.tianocore.org/show_bug.cgi?id=3697)
Add Cache ID valid/invalid defines to Acpi64.h which were not initially
added when the CacheIdValid field was added to
EFI_ACPI_6_4_PPTT_STRUCTURE_CACHE_FLAGS.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>
---
MdePkg/Include/IndustryStandard/Acpi64.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/MdePkg/Include/IndustryStandard/Acpi64.h b/MdePkg/Include/IndustryStandard/Acpi64.h
index 9cdd35b563fa8a181084f4f1b243793ae467ad6a..ff3cb441c215550f7cc635ced0f58e00d6510163 100644
--- a/MdePkg/Include/IndustryStandard/Acpi64.h
+++ b/MdePkg/Include/IndustryStandard/Acpi64.h
@@ -2680,6 +2680,8 @@ typedef struct {
#define EFI_ACPI_6_4_PPTT_WRITE_POLICY_VALID 0x1
#define EFI_ACPI_6_4_PPTT_LINE_SIZE_INVALID 0x0
#define EFI_ACPI_6_4_PPTT_LINE_SIZE_VALID 0x1
+#define EFI_ACPI_6_4_PPTT_CACHE_ID_INVALID 0x0
+#define EFI_ACPI_6_4_PPTT_CACHE_ID_VALID 0x1
///
/// Cache Type Structure flags
--
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 2/7] MdePkg: Remove PPTT ID type structure
2021-12-08 16:06 [PATCH v3 0/7] Support ACPI 6.4 PPTT changes Chris Jones
2021-12-08 16:06 ` [PATCH v3 1/7] MdePkg: Add missing Cache ID (in)valid define Chris Jones
@ 2021-12-08 16:06 ` Chris Jones
2021-12-08 16:06 ` [PATCH v3 3/7] ShellPkg: Update Acpiview PPTT parser to ACPI 6.4 Chris Jones
` (6 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Chris Jones @ 2021-12-08 16:06 UTC (permalink / raw)
To: devel
Cc: michael.d.kinney, gaoliming, zhiguang.liu, ray.ni, zhichao.gao,
Alexei.Fedorov, Sami.Mujawar, nd
Bugzilla: 3697 (https://bugzilla.tianocore.org/show_bug.cgi?id=3697)
The ACPI 6.3A specification deprecated the PPTT ID type structure (type
2) therefore remove it from Acpi64.h.
Mantis ID: 2072 (https://mantis.uefi.org/mantis/view.php?id=2072)
Signed-off-by: Chris Jones <christopher.jones@arm.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
---
MdePkg/Include/IndustryStandard/Acpi64.h | 16 ----------------
1 file changed, 16 deletions(-)
diff --git a/MdePkg/Include/IndustryStandard/Acpi64.h b/MdePkg/Include/IndustryStandard/Acpi64.h
index ff3cb441c215550f7cc635ced0f58e00d6510163..a79b7f2eaae16b47c17a89a16c6d67fcaaad4cbf 100644
--- a/MdePkg/Include/IndustryStandard/Acpi64.h
+++ b/MdePkg/Include/IndustryStandard/Acpi64.h
@@ -2613,7 +2613,6 @@ typedef struct {
///
#define EFI_ACPI_6_4_PPTT_TYPE_PROCESSOR 0x00
#define EFI_ACPI_6_4_PPTT_TYPE_CACHE 0x01
-#define EFI_ACPI_6_4_PPTT_TYPE_ID 0x02
///
/// PPTT Structure Header
@@ -2737,21 +2736,6 @@ typedef struct {
UINT32 CacheId;
} EFI_ACPI_6_4_PPTT_STRUCTURE_CACHE;
-///
-/// ID structure
-///
-typedef struct {
- UINT8 Type;
- UINT8 Length;
- UINT8 Reserved[2];
- UINT32 VendorId;
- UINT64 Level1Id;
- UINT64 Level2Id;
- UINT16 MajorRev;
- UINT16 MinorRev;
- UINT16 SpinRev;
-} EFI_ACPI_6_4_PPTT_STRUCTURE_ID;
-
///
/// Platform Health Assessment Table (PHAT) Format
///
--
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 3/7] ShellPkg: Update Acpiview PPTT parser to ACPI 6.4
2021-12-08 16:06 [PATCH v3 0/7] Support ACPI 6.4 PPTT changes Chris Jones
2021-12-08 16:06 ` [PATCH v3 1/7] MdePkg: Add missing Cache ID (in)valid define Chris Jones
2021-12-08 16:06 ` [PATCH v3 2/7] MdePkg: Remove PPTT ID type structure Chris Jones
@ 2021-12-08 16:06 ` Chris Jones
2021-12-08 16:06 ` [PATCH v3 4/7] ShellPkg: Add Cache ID to PPTT parser Chris Jones
` (5 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Chris Jones @ 2021-12-08 16:06 UTC (permalink / raw)
To: devel
Cc: michael.d.kinney, gaoliming, zhiguang.liu, ray.ni, zhichao.gao,
Alexei.Fedorov, Sami.Mujawar, nd
Bugzilla: 3697 (https://bugzilla.tianocore.org/show_bug.cgi?id=3697)
Update the Acpiview PPTT parser to use Acpi64.h. As part of the changes,
remove support for parsing PPTT type 2 ID structure.
Mantis ID for removing PPTT type 2 structure:
2072 (https://mantis.uefi.org/mantis/view.php?id=2072)
Signed-off-by: Chris Jones <christopher.jones@arm.com>
Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>
---
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c | 61 ++++----------------
ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c | 2 +-
2 files changed, 12 insertions(+), 51 deletions(-)
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
index f47b92f8c55ba5a479571a0bea378c75dfde13ed..7be249819e70dee9547c3fecc0763f473aa9ce92 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
@@ -1,11 +1,11 @@
/** @file
PPTT table parser
- Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.
+ Copyright (c) 2019 - 2021, ARM Limited. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
@par Reference(s):
- - ACPI 6.3 Specification - January 2019
+ - ACPI 6.4 Specification - January 2021
- ARM Architecture Reference Manual ARMv8 (D.a)
**/
@@ -161,8 +161,8 @@ ValidateCacheAttributes (
)
{
// Reference: Advanced Configuration and Power Interface (ACPI) Specification
- // Version 6.2 Errata A, September 2017
- // Table 5-153: Cache Type Structure
+ // Version 6.4, January 2021
+ // Table 5-140: Cache Type Structure
UINT8 Attributes;
Attributes = *(UINT8 *)Ptr;
@@ -227,22 +227,6 @@ STATIC CONST ACPI_PARSER CacheTypeStructureParser[] = {
{ L"Line size", 2, 22, L"%d", NULL, NULL, ValidateCacheLineSize, NULL }
};
-/**
- An ACPI_PARSER array describing the ID Type Structure - Type 2.
-**/
-STATIC CONST ACPI_PARSER IdStructureParser[] = {
- { L"Type", 1, 0, L"0x%x", NULL, NULL, NULL, NULL },
- { L"Length", 1, 1, L"%d", NULL, NULL, NULL, NULL },
- { L"Reserved", 2, 2, L"0x%x", NULL, NULL, NULL, NULL },
-
- { L"VENDOR_ID", 4, 4, NULL, Dump4Chars, NULL, NULL, NULL },
- { L"LEVEL_1_ID", 8, 8, L"0x%x", NULL, NULL, NULL, NULL },
- { L"LEVEL_2_ID", 8, 16, L"0x%x", NULL, NULL, NULL, NULL },
- { L"MAJOR_REV", 2, 24, L"0x%x", NULL, NULL, NULL, NULL },
- { L"MINOR_REV", 2, 26, L"0x%x", NULL, NULL, NULL, NULL },
- { L"SPIN_REV", 2, 28, L"0x%x", NULL, NULL, NULL, NULL },
-};
-
/**
This function parses the Processor Hierarchy Node Structure (Type 0).
@@ -340,29 +324,6 @@ DumpCacheTypeStructure (
);
}
-/**
- This function parses the ID Structure (Type 2).
-
- @param [in] Ptr Pointer to the start of the ID Structure data.
- @param [in] Length Length of the ID Structure.
-**/
-STATIC
-VOID
-DumpIDStructure (
- IN UINT8 *Ptr,
- IN UINT8 Length
- )
-{
- ParseAcpi (
- TRUE,
- 2,
- "ID Structure",
- Ptr,
- Length,
- PARSER_PARAMS (IdStructureParser)
- );
-}
-
/**
This function parses the ACPI PPTT table.
When trace is enabled this function parses the PPTT table and
@@ -371,7 +332,6 @@ DumpIDStructure (
This function parses the following processor topology structures:
- Processor hierarchy node structure (Type 0)
- Cache Type Structure (Type 1)
- - ID structure (Type 2)
This function also performs validation of the ACPI table fields.
@@ -451,22 +411,23 @@ ParseAcpiPptt (
Print (L"0x%x\n", Offset);
switch (*ProcessorTopologyStructureType) {
- case EFI_ACPI_6_2_PPTT_TYPE_PROCESSOR:
+ case EFI_ACPI_6_4_PPTT_TYPE_PROCESSOR:
DumpProcessorHierarchyNodeStructure (
ProcessorTopologyStructurePtr,
*ProcessorTopologyStructureLength
);
break;
- case EFI_ACPI_6_2_PPTT_TYPE_CACHE:
+ case EFI_ACPI_6_4_PPTT_TYPE_CACHE:
DumpCacheTypeStructure (
ProcessorTopologyStructurePtr,
*ProcessorTopologyStructureLength
);
break;
- case EFI_ACPI_6_2_PPTT_TYPE_ID:
- DumpIDStructure (
- ProcessorTopologyStructurePtr,
- *ProcessorTopologyStructureLength
+ case EFI_ACPI_6_3_PPTT_TYPE_ID:
+ IncrementErrorCount ();
+ Print (
+ L"ERROR: PPTT Type 2 - Processor ID has been removed and must not be"
+ L"used.\n"
);
break;
default:
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
index 0ad7bf4c845f3c065764680c01eaa27796b09226..09bdddb56e5bd70dec44bdbdcba88373f93daa66 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
@@ -62,7 +62,7 @@ ACPI_TABLE_PARSER ParserList[] = {
ParseAcpiMcfg },
{ EFI_ACPI_6_4_PLATFORM_COMMUNICATIONS_CHANNEL_TABLE_SIGNATURE,
ParseAcpiPcct },
- { EFI_ACPI_6_2_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGNATURE,
+ { EFI_ACPI_6_4_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGNATURE,
ParseAcpiPptt },
{ RSDP_TABLE_INFO, ParseAcpiRsdp },
{ EFI_ACPI_6_2_SYSTEM_LOCALITY_INFORMATION_TABLE_SIGNATURE, ParseAcpiSlit },
--
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 4/7] ShellPkg: Add Cache ID to PPTT parser
2021-12-08 16:06 [PATCH v3 0/7] Support ACPI 6.4 PPTT changes Chris Jones
` (2 preceding siblings ...)
2021-12-08 16:06 ` [PATCH v3 3/7] ShellPkg: Update Acpiview PPTT parser to ACPI 6.4 Chris Jones
@ 2021-12-08 16:06 ` Chris Jones
2021-12-08 16:06 ` [PATCH v3 5/7] DynamicTablesPkg: Remove PPTT ID structure from ACPI 6.4 generator Chris Jones
` (4 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Chris Jones @ 2021-12-08 16:06 UTC (permalink / raw)
To: devel
Cc: michael.d.kinney, gaoliming, zhiguang.liu, ray.ni, zhichao.gao,
Alexei.Fedorov, Sami.Mujawar, nd
Bugzilla: 3697 (https://bugzilla.tianocore.org/show_bug.cgi?id=3697)
Update the Acpiview PPTT parser with the Cache ID field and relevant
validations as defined in tables 5.140 and 5.141 of the ACPI 6.4
specification.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>
---
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c | 153 ++++++++++++++++++--
1 file changed, 139 insertions(+), 14 deletions(-)
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
index 7be249819e70dee9547c3fecc0763f473aa9ce92..8b12809f8a155b0ee0a4ad5c63d620b48d97f4b0 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
@@ -17,10 +17,94 @@
#include "PpttParser.h"
// Local variables
-STATIC CONST UINT8 *ProcessorTopologyStructureType;
-STATIC CONST UINT8 *ProcessorTopologyStructureLength;
-STATIC CONST UINT32 *NumberOfPrivateResources;
-STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
+STATIC CONST UINT8 *ProcessorTopologyStructureType;
+STATIC CONST UINT8 *ProcessorTopologyStructureLength;
+STATIC CONST UINT32 *NumberOfPrivateResources;
+STATIC CONST EFI_ACPI_6_4_PPTT_STRUCTURE_CACHE_FLAGS *CacheFlags;
+STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
+
+/**
+ Increment the error count and print an error that a required flag is missing.
+
+ @param [in] FlagName Name of the missing flag.
+**/
+STATIC
+VOID
+EFIAPI
+LogCacheFlagError (
+ IN CONST CHAR16 *FlagName
+ )
+{
+ IncrementErrorCount ();
+ Print (
+ L"\nERROR: On Arm based systems, all cache properties must be"
+ L"provided in the cache type structure."
+ L"Missing '%s' flag.",
+ *FlagName
+ );
+}
+
+/**
+ This function validates the Cache Type Structure (Type 1) Cache Flags 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
+ValidateCacheFlags (
+ IN UINT8 *Ptr,
+ IN VOID *Context
+ )
+{
+ #if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
+ CacheFlags = (EFI_ACPI_6_4_PPTT_STRUCTURE_CACHE_FLAGS *)Ptr;
+
+ if (CacheFlags == NULL) {
+ IncrementErrorCount ();
+ Print (L"\nERROR: Cache Structure Flags were not successfully read.");
+ return;
+ }
+
+ if (CacheFlags->SizePropertyValid == EFI_ACPI_6_4_PPTT_CACHE_SIZE_INVALID) {
+ LogCacheFlagError (L"Size Property Valid");
+ }
+
+ if (CacheFlags->NumberOfSetsValid == EFI_ACPI_6_4_PPTT_NUMBER_OF_SETS_INVALID) {
+ LogCacheFlagError (L"Number Of Sets Valid");
+ }
+
+ if (CacheFlags->AssociativityValid == EFI_ACPI_6_4_PPTT_ASSOCIATIVITY_INVALID) {
+ LogCacheFlagError (L"Associativity Valid");
+ }
+
+ if (CacheFlags->AllocationTypeValid == EFI_ACPI_6_4_PPTT_ALLOCATION_TYPE_INVALID) {
+ LogCacheFlagError (L"Allocation Type Valid");
+ }
+
+ if (CacheFlags->CacheTypeValid == EFI_ACPI_6_4_PPTT_CACHE_TYPE_INVALID) {
+ LogCacheFlagError (L"Cache Type Valid");
+ }
+
+ if (CacheFlags->WritePolicyValid == EFI_ACPI_6_4_PPTT_WRITE_POLICY_INVALID) {
+ LogCacheFlagError (L"Write Policy Valid");
+ }
+
+ if (CacheFlags->LineSizeValid == EFI_ACPI_6_4_PPTT_LINE_SIZE_INVALID) {
+ LogCacheFlagError (L"Line Size Valid");
+ }
+
+ // Cache ID was only introduced in revision 3
+ if (*(AcpiHdrInfo.Revision) >= 3) {
+ if (CacheFlags->CacheIdValid == EFI_ACPI_6_4_PPTT_CACHE_ID_INVALID) {
+ LogCacheFlagError (L"Cache Id Valid");
+ }
+ }
+
+ #endif
+}
/**
This function validates the Cache Type Structure (Type 1) 'Number of sets'
@@ -145,6 +229,45 @@ ValidateCacheLineSize (
#endif
}
+/**
+ This function validates the Cache Type Structure (Type 1) Cache ID 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
+ValidateCacheId (
+ IN UINT8 *Ptr,
+ IN VOID *Context
+ )
+{
+ UINT32 CacheId;
+
+ CacheId = *(UINT32 *)Ptr;
+
+ // Cache ID was only introduced in revision 3
+ if (*(AcpiHdrInfo.Revision) < 3) {
+ return;
+ }
+
+ if (CacheFlags == NULL) {
+ IncrementErrorCount ();
+ Print (L"\nERROR: Cache Structure Flags were not successfully read.");
+ return;
+ }
+
+ if (CacheFlags->CacheIdValid == EFI_ACPI_6_4_PPTT_CACHE_ID_VALID) {
+ if (CacheId == 0) {
+ IncrementErrorCount ();
+ Print (L"\nERROR: 0 is not a valid Cache ID.");
+ return;
+ }
+ }
+}
+
/**
This function validates the Cache Type Structure (Type 1) Attributes field.
@@ -214,17 +337,19 @@ STATIC CONST ACPI_PARSER ProcessorHierarchyNodeStructureParser[] = {
An ACPI_PARSER array describing the Cache Type Structure - Type 1.
**/
STATIC CONST ACPI_PARSER CacheTypeStructureParser[] = {
- { L"Type", 1, 0, L"0x%x", NULL, NULL, NULL, NULL },
- { L"Length", 1, 1, L"%d", NULL, NULL, NULL, NULL },
- { L"Reserved", 2, 2, L"0x%x", NULL, NULL, NULL, NULL },
+ { L"Type", 1, 0, L"0x%x", NULL, NULL, NULL, NULL },
+ { L"Length", 1, 1, L"%d", NULL, NULL, NULL, NULL },
+ { L"Reserved", 2, 2, L"0x%x", NULL, NULL, NULL, NULL },
- { L"Flags", 4, 4, L"0x%x", NULL, NULL, NULL, NULL },
- { L"Next Level of Cache", 4, 8, L"0x%x", NULL, NULL, NULL, NULL },
- { L"Size", 4, 12, L"0x%x", NULL, NULL, NULL, NULL },
- { L"Number of sets", 4, 16, L"%d", NULL, NULL, ValidateCacheNumberOfSets, NULL },
- { L"Associativity", 1, 20, L"%d", NULL, NULL, ValidateCacheAssociativity, NULL },
- { L"Attributes", 1, 21, L"0x%x", NULL, NULL, ValidateCacheAttributes, NULL },
- { L"Line size", 2, 22, L"%d", NULL, NULL, ValidateCacheLineSize, NULL }
+ { L"Flags", 4, 4, L"0x%x", NULL, (VOID **)&CacheFlags, ValidateCacheFlags,
+ NULL },
+ { L"Next Level of Cache", 4, 8, L"0x%x", NULL, NULL, NULL, NULL },
+ { L"Size", 4, 12, L"0x%x", NULL, NULL, NULL, NULL },
+ { L"Number of sets", 4, 16, L"%d", NULL, NULL, ValidateCacheNumberOfSets, NULL },
+ { L"Associativity", 1, 20, L"%d", NULL, NULL, ValidateCacheAssociativity, NULL },
+ { L"Attributes", 1, 21, L"0x%x", NULL, NULL, ValidateCacheAttributes, NULL },
+ { L"Line size", 2, 22, L"%d", NULL, NULL, ValidateCacheLineSize, NULL },
+ { L"Cache ID", 4, 24, L"%d", NULL, NULL, ValidateCacheId, NULL }
};
/**
--
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 5/7] DynamicTablesPkg: Remove PPTT ID structure from ACPI 6.4 generator
2021-12-08 16:06 [PATCH v3 0/7] Support ACPI 6.4 PPTT changes Chris Jones
` (3 preceding siblings ...)
2021-12-08 16:06 ` [PATCH v3 4/7] ShellPkg: Add Cache ID to PPTT parser Chris Jones
@ 2021-12-08 16:06 ` Chris Jones
2021-12-08 16:06 ` [PATCH v3 6/7] DynamicTablesPkg: Update PPTT generator to ACPI 6.4 Chris Jones
` (3 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Chris Jones @ 2021-12-08 16:06 UTC (permalink / raw)
To: devel
Cc: michael.d.kinney, gaoliming, zhiguang.liu, ray.ni, zhichao.gao,
Alexei.Fedorov, Sami.Mujawar, nd
Bugzilla: 3697 (https://bugzilla.tianocore.org/show_bug.cgi?id=3697)
ACPI 6.3A deprecated PPTT ID (type 2) structure which was subsequently
removed in ACPI 6.4. Therefore remove support for generating PPTT ID
structures.
Mantis ID for removing PPTT type 2 structure:
2072 (https://mantis.uefi.org/mantis/view.php?id=2072)
Signed-off-by: Chris Jones <christopher.jones@arm.com>
---
DynamicTablesPkg/Include/ArmNameSpaceObjects.h | 23 +--
DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c | 154 +-------------------
DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.h | 4 -
3 files changed, 3 insertions(+), 178 deletions(-)
diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
index 22b37edfabd1f6a1d1a3015f4a0249c28489367c..3246e8884723ac85340bf880a3859800726be3c1 100644
--- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
@@ -50,7 +50,7 @@ typedef enum ArmObjectID {
EArmObjSmmuInterruptArray, ///< 26 - SMMU Interrupt Array
EArmObjProcHierarchyInfo, ///< 27 - Processor Hierarchy Info
EArmObjCacheInfo, ///< 28 - Cache Info
- EArmObjProcNodeIdInfo, ///< 29 - Processor Node ID Info
+ EArmObjReserved29, ///< 29 - Reserved
EArmObjCmRef, ///< 30 - CM Object Reference
EArmObjMemoryAffinityInfo, ///< 31 - Memory Affinity Info
EArmObjDeviceHandleAcpi, ///< 32 - Device Handle Acpi
@@ -747,27 +747,6 @@ typedef struct CmArmCacheInfo {
UINT16 LineSize;
} CM_ARM_CACHE_INFO;
-/** A structure that describes the ID Structure (Type 2) in PPTT
-
- ID: EArmObjProcNodeIdInfo
-*/
-typedef struct CmArmProcNodeIdInfo {
- /// A unique token used to identify this object
- CM_OBJECT_TOKEN Token;
- // Vendor ID (as described in ACPI ID registry)
- UINT32 VendorId;
- /// First level unique node ID
- UINT64 Level1Id;
- /// Second level unique node ID
- UINT64 Level2Id;
- /// Major revision of the node
- UINT16 MajorRev;
- /// Minor revision of the node
- UINT16 MinorRev;
- /// Spin revision of the node
- UINT16 SpinRev;
-} CM_ARM_PROC_NODE_ID_INFO;
-
/** A structure that describes a reference to another Configuration Manager
object.
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
index f78bbed7c1beee4211752bba94ce531a84e84f10..58ad338d339f7f2cf506df961f016edb160f0edc 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
@@ -34,7 +34,6 @@
The following Configuration Manager Object(s) are used by this Generator:
- EArmObjProcHierarchyInfo (REQUIRED)
- EArmObjCacheInfo
- - EArmObjProcNodeIdInfo
- EArmObjCmRef
- EArmObjGicCInfo (REQUIRED)
*/
@@ -59,16 +58,6 @@ GET_OBJECT_LIST (
CM_ARM_CACHE_INFO
);
-/**
- This macro expands to a function that retrieves the ID information for
- Processor Hierarchy Nodes from the Configuration Manager.
-*/
-GET_OBJECT_LIST (
- EObjNameSpaceArm,
- EArmObjProcNodeIdInfo,
- CM_ARM_PROC_NODE_ID_INFO
- );
-
/**
This macro expands to a function that retrieves the cross-CM-object-
reference information from the Configuration Manager.
@@ -131,15 +120,6 @@ GET_SIZE_OF_PPTT_STRUCTS (
CM_ARM_CACHE_INFO
);
-/** This macro expands to a function that retrieves the amount of memory
- required to store the ID Structures (Type 2) and updates the Node Indexer.
-*/
-GET_SIZE_OF_PPTT_STRUCTS (
- IdStructs,
- sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_ID),
- CM_ARM_PROC_NODE_ID_INFO
- );
-
/**
Search the Node Indexer and return the indexed PPTT node with the given
Token.
@@ -373,8 +353,8 @@ AddPrivateResources (
}
// The Node indexer has the Processor hierarchy nodes at the begining
- // followed by the cache structs and Id structs. Therefore we can
- // skip the Processor hierarchy nodes in the node indexer search.
+ // followed by the cache structs. Therefore we can skip the Processor
+ // hierarchy nodes in the node indexer search.
Status = GetPpttNodeReferencedByToken (
Generator->CacheStructIndexedList,
(Generator->ProcTopologyStructCount -
@@ -969,71 +949,6 @@ AddCacheTypeStructures (
return EFI_SUCCESS;
}
-/**
- Update the ID Type Structure (Type 2) information.
-
- This function populates the ID Type Structures with information from
- the Configuration Manager and and adds this information to the PPTT table.
-
- @param [in] Generator Pointer to the PPTT Generator.
- @param [in] CfgMgrProtocol Pointer to the Configuration Manager
- Protocol Interface.
- @param [in] Pptt Pointer to PPTT table structure.
- @param [in] NodesStartOffset Offset from the start of PPTT table to the
- start of ID Type Structures.
-
- @retval EFI_SUCCESS Structures updated successfully.
- @retval EFI_INVALID_PARAMETER A parameter is invalid.
- @retval EFI_NOT_FOUND A required object was not found.
-**/
-STATIC
-EFI_STATUS
-AddIdTypeStructures (
- IN CONST ACPI_PPTT_GENERATOR *CONST Generator,
- IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
- IN CONST EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_HEADER *Pptt,
- IN CONST UINT32 NodesStartOffset
- )
-{
- EFI_ACPI_6_3_PPTT_STRUCTURE_ID *IdStruct;
- CM_ARM_PROC_NODE_ID_INFO *ProcIdInfoNode;
- PPTT_NODE_INDEXER *IdStructIterator;
- UINT32 NodeCount;
-
- ASSERT (
- (Generator != NULL) &&
- (CfgMgrProtocol != NULL) &&
- (Pptt != NULL)
- );
-
- IdStruct = (EFI_ACPI_6_3_PPTT_STRUCTURE_ID *)((UINT8 *)Pptt + NodesStartOffset);
-
- IdStructIterator = Generator->IdStructIndexedList;
- NodeCount = Generator->IdStructCount;
- while (NodeCount-- != 0) {
- ProcIdInfoNode = (CM_ARM_PROC_NODE_ID_INFO *)IdStructIterator->Object;
-
- // Populate the node
- IdStruct->Type = EFI_ACPI_6_3_PPTT_TYPE_ID;
- IdStruct->Length = sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_ID);
- IdStruct->Reserved[0] = EFI_ACPI_RESERVED_BYTE;
- IdStruct->Reserved[1] = EFI_ACPI_RESERVED_BYTE;
- IdStruct->VendorId = ProcIdInfoNode->VendorId;
- IdStruct->Level1Id = ProcIdInfoNode->Level1Id;
- IdStruct->Level2Id = ProcIdInfoNode->Level2Id;
- IdStruct->MajorRev = ProcIdInfoNode->MajorRev;
- IdStruct->MinorRev = ProcIdInfoNode->MinorRev;
- IdStruct->SpinRev = ProcIdInfoNode->SpinRev;
-
- // Next ID Type Structure
- IdStruct = (EFI_ACPI_6_3_PPTT_STRUCTURE_ID *)((UINT8 *)IdStruct +
- IdStruct->Length);
- IdStructIterator++;
- } // ID Type Structure
-
- return EFI_SUCCESS;
-}
-
/**
Construct the PPTT ACPI table.
@@ -1072,15 +987,12 @@ BuildPpttTable (
UINT32 ProcTopologyStructCount;
UINT32 ProcHierarchyNodeCount;
UINT32 CacheStructCount;
- UINT32 IdStructCount;
UINT32 ProcHierarchyNodeOffset;
UINT32 CacheStructOffset;
- UINT32 IdStructOffset;
CM_ARM_PROC_HIERARCHY_INFO *ProcHierarchyNodeList;
CM_ARM_CACHE_INFO *CacheStructList;
- CM_ARM_PROC_NODE_ID_INFO *IdStructList;
ACPI_PPTT_GENERATOR *Generator;
@@ -1155,27 +1067,6 @@ BuildPpttTable (
ProcTopologyStructCount += CacheStructCount;
Generator->CacheStructCount = CacheStructCount;
- // Get the processor hierarchy node ID info and update the processor topology
- // structure count with ID Structures (Type 2)
- Status = GetEArmObjProcNodeIdInfo (
- CfgMgrProtocol,
- CM_NULL_TOKEN,
- &IdStructList,
- &IdStructCount
- );
- if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: PPTT: Failed to get processor hierarchy node ID info. " \
- "Status = %r\n",
- Status
- ));
- goto error_handler;
- }
-
- ProcTopologyStructCount += IdStructCount;
- Generator->IdStructCount = IdStructCount;
-
// Allocate Node Indexer array
NodeIndexer = (PPTT_NODE_INDEXER *)AllocateZeroPool (
sizeof (PPTT_NODE_INDEXER) *
@@ -1241,27 +1132,6 @@ BuildPpttTable (
));
}
- // Include the size of ID Type Structures and index them
- if (Generator->IdStructCount != 0) {
- IdStructOffset = TableSize;
- Generator->IdStructIndexedList = NodeIndexer;
- TableSize += GetSizeofIdStructs (
- IdStructOffset,
- IdStructList,
- Generator->IdStructCount,
- &NodeIndexer
- );
- DEBUG ((
- DEBUG_INFO,
- " IdStructCount = %d\n" \
- " IdStructOffset = 0x%x\n" \
- " IdStructIndexedList = 0x%p\n",
- Generator->IdStructCount,
- IdStructOffset,
- Generator->IdStructIndexedList
- ));
- }
-
DEBUG ((
DEBUG_INFO,
"INFO: PPTT:\n" \
@@ -1347,24 +1217,6 @@ BuildPpttTable (
}
}
- // Add ID Type Structures (Type 2) to the generated table
- if (Generator->IdStructCount != 0) {
- Status = AddIdTypeStructures (
- Generator,
- CfgMgrProtocol,
- Pptt,
- IdStructOffset
- );
- if (EFI_ERROR (Status)) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: PPTT: Failed to add ID Type Structures. Status = %r\n",
- Status
- ));
- goto error_handler;
- }
- }
-
// Validate CM object cross-references in PPTT
Status = DetectCyclesInTopology (Generator);
if (EFI_ERROR (Status)) {
@@ -1488,8 +1340,6 @@ ACPI_PPTT_GENERATOR PpttGenerator = {
0,
// Count of Cache Structures
0,
- // Count of Id Structures
- 0,
// Pointer to PPTT Node Indexer
NULL
};
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.h b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.h
index f587e67e9f8bf51511c6187ba0384e1cb60661d5..15b0a9871c3d555a62b79595c317e6597f6ead05 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.h
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.h
@@ -171,8 +171,6 @@ typedef struct AcpiPpttGenerator {
UINT32 ProcHierarchyNodeCount;
/// Count of Cache Structures
UINT32 CacheStructCount;
- /// Count of Id Structures
- UINT32 IdStructCount;
/// List of indexed CM objects for PPTT generation
PPTT_NODE_INDEXER *NodeIndexer;
/// Pointer to the start of Processor Hierarchy nodes in
@@ -180,8 +178,6 @@ typedef struct AcpiPpttGenerator {
PPTT_NODE_INDEXER *ProcHierarchyNodeIndexedList;
/// Pointer to the start of Cache Structures in the Node Indexer array
PPTT_NODE_INDEXER *CacheStructIndexedList;
- /// Pointer to the start of Id Structures in the Node Indexer array
- PPTT_NODE_INDEXER *IdStructIndexedList;
} ACPI_PPTT_GENERATOR;
#pragma pack()
--
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 6/7] DynamicTablesPkg: Update PPTT generator to ACPI 6.4
2021-12-08 16:06 [PATCH v3 0/7] Support ACPI 6.4 PPTT changes Chris Jones
` (4 preceding siblings ...)
2021-12-08 16:06 ` [PATCH v3 5/7] DynamicTablesPkg: Remove PPTT ID structure from ACPI 6.4 generator Chris Jones
@ 2021-12-08 16:06 ` Chris Jones
2021-12-08 16:06 ` [PATCH v3 7/7] DynamicTablesPkg: Add CacheId to PPTT generator Chris Jones
` (2 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Chris Jones @ 2021-12-08 16:06 UTC (permalink / raw)
To: devel
Cc: michael.d.kinney, gaoliming, zhiguang.liu, ray.ni, zhichao.gao,
Alexei.Fedorov, Sami.Mujawar, nd
Bugzilla: 3697 (https://bugzilla.tianocore.org/show_bug.cgi?id=3697)
Update the PPTT generator to use Acpi64.h.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
---
DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c | 44 ++++++++++----------
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
index 58ad338d339f7f2cf506df961f016edb160f0edc..3d416ca78ec16a1929ede87abbe4f8f4464ef0cf 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
@@ -1,11 +1,11 @@
/** @file
PPTT Table Generator
- Copyright (c) 2019, ARM Limited. All rights reserved.
+ Copyright (c) 2021, ARM Limited. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
@par Reference(s):
- - ACPI 6.3 Specification, January 2019
+ - ACPI 6.4 Specification, January 2021
@par Glossary:
- Cm or CM - Configuration Manager
@@ -96,7 +96,7 @@ GetProcHierarchyNodeSize (
ASSERT (Node != NULL);
// <size of Processor Hierarchy Node> + <size of Private Resources array>
- return sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR) +
+ return sizeof (EFI_ACPI_6_4_PPTT_STRUCTURE_PROCESSOR) +
(Node->NoOfPrivateResources * sizeof (UINT32));
}
@@ -116,7 +116,7 @@ GET_SIZE_OF_PPTT_STRUCTS (
*/
GET_SIZE_OF_PPTT_STRUCTS (
CacheTypeStructs,
- sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE),
+ sizeof (EFI_ACPI_6_4_PPTT_STRUCTURE_CACHE),
CM_ARM_CACHE_INFO
);
@@ -468,12 +468,12 @@ EFI_STATUS
AddProcHierarchyNodes (
IN CONST ACPI_PPTT_GENERATOR *CONST Generator,
IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
- IN CONST EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_HEADER *Pptt,
+ IN CONST EFI_ACPI_6_4_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_HEADER *Pptt,
IN CONST UINT32 NodesStartOffset
)
{
EFI_STATUS Status;
- EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR *ProcStruct;
+ EFI_ACPI_6_4_PPTT_STRUCTURE_PROCESSOR *ProcStruct;
UINT32 *PrivateResources;
BOOLEAN IsGicCTokenDuplicated;
@@ -494,7 +494,7 @@ AddProcHierarchyNodes (
(Pptt != NULL)
);
- ProcStruct = (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR *)((UINT8 *)Pptt +
+ ProcStruct = (EFI_ACPI_6_4_PPTT_STRUCTURE_PROCESSOR *)((UINT8 *)Pptt +
NodesStartOffset);
ProcNodeIterator = Generator->ProcHierarchyNodeIndexedList;
@@ -538,7 +538,7 @@ AddProcHierarchyNodes (
}
// Populate the node header
- ProcStruct->Type = EFI_ACPI_6_3_PPTT_TYPE_PROCESSOR;
+ ProcStruct->Type = EFI_ACPI_6_4_PPTT_TYPE_PROCESSOR;
ProcStruct->Length = (UINT8)Length;
ProcStruct->Reserved[0] = EFI_ACPI_RESERVED_BYTE;
ProcStruct->Reserved[1] = EFI_ACPI_RESERVED_BYTE;
@@ -661,7 +661,7 @@ AddProcHierarchyNodes (
ProcStruct->NumberOfPrivateResources = ProcInfoNode->NoOfPrivateResources;
PrivateResources = (UINT32 *)((UINT8 *)ProcStruct +
- sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR));
+ sizeof (EFI_ACPI_6_4_PPTT_STRUCTURE_PROCESSOR));
if (ProcStruct->NumberOfPrivateResources != 0) {
// Populate the private resources array
@@ -684,7 +684,7 @@ AddProcHierarchyNodes (
}
// Next Processor Hierarchy Node
- ProcStruct = (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR *)((UINT8 *)ProcStruct +
+ ProcStruct = (EFI_ACPI_6_4_PPTT_STRUCTURE_PROCESSOR *)((UINT8 *)ProcStruct +
ProcStruct->Length);
ProcNodeIterator++;
} // Processor Hierarchy Node
@@ -748,12 +748,12 @@ EFI_STATUS
AddCacheTypeStructures (
IN CONST ACPI_PPTT_GENERATOR *CONST Generator,
IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
- IN CONST EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_HEADER *Pptt,
+ IN CONST EFI_ACPI_6_4_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_HEADER *Pptt,
IN CONST UINT32 NodesStartOffset
)
{
EFI_STATUS Status;
- EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE *CacheStruct;
+ EFI_ACPI_6_4_PPTT_STRUCTURE_CACHE *CacheStruct;
PPTT_NODE_INDEXER *PpttNodeFound;
CM_ARM_CACHE_INFO *CacheInfoNode;
PPTT_NODE_INDEXER *CacheNodeIterator;
@@ -765,7 +765,7 @@ AddCacheTypeStructures (
(Pptt != NULL)
);
- CacheStruct = (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE *)((UINT8 *)Pptt +
+ CacheStruct = (EFI_ACPI_6_4_PPTT_STRUCTURE_CACHE *)((UINT8 *)Pptt +
NodesStartOffset);
CacheNodeIterator = Generator->CacheStructIndexedList;
@@ -775,13 +775,13 @@ AddCacheTypeStructures (
CacheInfoNode = (CM_ARM_CACHE_INFO *)CacheNodeIterator->Object;
// Populate the node header
- CacheStruct->Type = EFI_ACPI_6_3_PPTT_TYPE_CACHE;
- CacheStruct->Length = sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE);
+ CacheStruct->Type = EFI_ACPI_6_4_PPTT_TYPE_CACHE;
+ CacheStruct->Length = sizeof (EFI_ACPI_6_4_PPTT_STRUCTURE_CACHE);
CacheStruct->Reserved[0] = EFI_ACPI_RESERVED_BYTE;
CacheStruct->Reserved[1] = EFI_ACPI_RESERVED_BYTE;
// "On Arm-based systems, all cache properties must be provided in the
- // table." (ACPI 6.3, Section 5.2.29.2)
+ // table." (ACPI 6.4, Section 5.2.29.2)
CacheStruct->Flags.SizePropertyValid = 1;
CacheStruct->Flags.NumberOfSetsValid = 1;
CacheStruct->Flags.AssociativityValid = 1;
@@ -941,7 +941,7 @@ AddCacheTypeStructures (
CacheStruct->LineSize = CacheInfoNode->LineSize;
// Next Cache Type Structure
- CacheStruct = (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE *)((UINT8 *)CacheStruct +
+ CacheStruct = (EFI_ACPI_6_4_PPTT_STRUCTURE_CACHE *)((UINT8 *)CacheStruct +
CacheStruct->Length);
CacheNodeIterator++;
} // Cache Type Structure
@@ -999,7 +999,7 @@ BuildPpttTable (
// Pointer to the Node Indexer array
PPTT_NODE_INDEXER *NodeIndexer;
- EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_HEADER *Pptt;
+ EFI_ACPI_6_4_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_HEADER *Pptt;
ASSERT (
(This != NULL) &&
@@ -1087,7 +1087,7 @@ BuildPpttTable (
Generator->NodeIndexer = NodeIndexer;
// Calculate the size of the PPTT table
- TableSize = sizeof (EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_HEADER);
+ TableSize = sizeof (EFI_ACPI_6_4_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_HEADER);
// Include the size of Processor Hierarchy Nodes and index them
if (Generator->ProcHierarchyNodeCount != 0) {
@@ -1155,7 +1155,7 @@ BuildPpttTable (
goto error_handler;
}
- Pptt = (EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_HEADER *)*Table;
+ Pptt = (EFI_ACPI_6_4_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_HEADER *)*Table;
DEBUG ((
DEBUG_INFO,
@@ -1312,9 +1312,9 @@ ACPI_PPTT_GENERATOR PpttGenerator = {
// Generator Description
L"ACPI.STD.PPTT.GENERATOR",
// ACPI Table Signature
- EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGNATURE,
+ EFI_ACPI_6_4_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGNATURE,
// ACPI Table Revision supported by this Generator
- EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_REVISION,
+ EFI_ACPI_6_4_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_REVISION,
// Minimum supported ACPI Table Revision
EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_REVISION,
// Creator ID
--
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 7/7] DynamicTablesPkg: Add CacheId to PPTT generator
2021-12-08 16:06 [PATCH v3 0/7] Support ACPI 6.4 PPTT changes Chris Jones
` (5 preceding siblings ...)
2021-12-08 16:06 ` [PATCH v3 6/7] DynamicTablesPkg: Update PPTT generator to ACPI 6.4 Chris Jones
@ 2021-12-08 16:06 ` Chris Jones
2021-12-09 13:19 ` Sami Mujawar
2021-12-09 17:00 ` [PATCH v3 0/7] Support ACPI 6.4 PPTT changes Sami Mujawar
[not found] ` <16BF24D08620153C.20064@groups.io>
8 siblings, 1 reply; 12+ messages in thread
From: Chris Jones @ 2021-12-08 16:06 UTC (permalink / raw)
To: devel
Cc: michael.d.kinney, gaoliming, zhiguang.liu, ray.ni, zhichao.gao,
Alexei.Fedorov, Sami.Mujawar, nd
Bugzilla: 3697 (https://bugzilla.tianocore.org/show_bug.cgi?id=3697)
Update the PPTT generator with the CacheId field as defined in table
5.140 of the ACPI 6.4 specification.
Also add validations to ensure that the cache id generated is unique.
Signed-off-by: Chris Jones <christopher.jones@arm.com>
---
DynamicTablesPkg/Include/ArmNameSpaceObjects.h | 4 +-
DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c | 102 ++++++++++++++++++--
2 files changed, 96 insertions(+), 10 deletions(-)
diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
index 3246e8884723ac85340bf880a3859800726be3c1..6ea03fca487b96577b8fd8105bc3d22047ff5697 100644
--- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
@@ -741,10 +741,12 @@ typedef struct CmArmCacheInfo {
/// PPTT_ARM_CCIDX_CACHE_ASSOCIATIVITY_MAX. Therfore this field
/// is 32-bit wide.
UINT32 Associativity;
- /// Cache attributes (ACPI 6.3 - January 2019, PPTT, Table 5-156)
+ /// Cache attributes (ACPI 6.4 - January 2021, PPTT, Table 5.140)
UINT8 Attributes;
/// Line size in bytes
UINT16 LineSize;
+ /// Unique ID for the cache
+ UINT32 CacheId;
} CM_ARM_CACHE_INFO;
/** A structure that describes a reference to another Configuration Manager
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
index 3d416ca78ec16a1929ede87abbe4f8f4464ef0cf..6b74572ea2dd8478f14d013e6cb7394216e45d8d 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
@@ -726,6 +726,35 @@ AddProcHierarchyNodes (
return Status;
}
+/**
+ Test whether CacheId is unique among the CacheIdList.
+
+ @param [in] CacheId Cache ID to check.
+ @param [in] CacheIdList List of already existing cache IDs.
+ @param [in] CacheIdListSize Size of CacheIdList.
+
+ @retval TRUE CacheId does not exist in CacheIdList.
+ @retval FALSE CacheId already exists in CacheIdList.
+**/
+STATIC
+BOOLEAN
+IsCacheIdUnique (
+ IN CONST UINT32 CacheId,
+ IN CONST UINT32 *CacheIdList,
+ IN CONST UINT32 CacheIdListSize
+ )
+{
+ UINT32 Index;
+
+ for (Index = 0; Index < CacheIdListSize; Index++) {
+ if (CacheIdList[Index] == CacheId) {
+ return FALSE;
+ }
+ }
+
+ return TRUE;
+}
+
/**
Update the Cache Type Structure (Type 1) information.
@@ -738,10 +767,12 @@ AddProcHierarchyNodes (
@param [in] Pptt Pointer to PPTT table structure.
@param [in] NodesStartOffset Offset from the start of PPTT table to the
start of Cache Type Structures.
+ @param [in] Revision Revision of the PPTT table being requested.
@retval EFI_SUCCESS Structures updated successfully.
@retval EFI_INVALID_PARAMETER A parameter is invalid.
@retval EFI_NOT_FOUND A required object was not found.
+ @retval EFI_OUT_OF_RESOURCES Out of resources.
**/
STATIC
EFI_STATUS
@@ -749,7 +780,8 @@ AddCacheTypeStructures (
IN CONST ACPI_PPTT_GENERATOR *CONST Generator,
IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
IN CONST EFI_ACPI_6_4_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_HEADER *Pptt,
- IN CONST UINT32 NodesStartOffset
+ IN CONST UINT32 NodesStartOffset,
+ IN CONST UINT32 Revision
)
{
EFI_STATUS Status;
@@ -758,6 +790,9 @@ AddCacheTypeStructures (
CM_ARM_CACHE_INFO *CacheInfoNode;
PPTT_NODE_INDEXER *CacheNodeIterator;
UINT32 NodeCount;
+ BOOLEAN CacheIdUnique;
+ UINT32 TotalNodeCount;
+ UINT32 *FoundCacheIds;
ASSERT (
(Generator != NULL) &&
@@ -770,6 +805,13 @@ AddCacheTypeStructures (
CacheNodeIterator = Generator->CacheStructIndexedList;
NodeCount = Generator->CacheStructCount;
+ TotalNodeCount = NodeCount;
+
+ FoundCacheIds = AllocateZeroPool (TotalNodeCount * sizeof (*FoundCacheIds));
+ if (FoundCacheIds == NULL) {
+ DEBUG ((DEBUG_ERROR, "ERROR: PPTT: Failed to allocate resources.\n"));
+ return EFI_OUT_OF_RESOURCES;
+ }
while (NodeCount-- != 0) {
CacheInfoNode = (CM_ARM_CACHE_INFO *)CacheNodeIterator->Object;
@@ -789,6 +831,7 @@ AddCacheTypeStructures (
CacheStruct->Flags.CacheTypeValid = 1;
CacheStruct->Flags.WritePolicyValid = 1;
CacheStruct->Flags.LineSizeValid = 1;
+ CacheStruct->Flags.CacheIdValid = 1;
CacheStruct->Flags.Reserved = 0;
// Populate the reference to the next level of cache
@@ -811,7 +854,7 @@ AddCacheTypeStructures (
CacheInfoNode->Token,
Status
));
- return Status;
+ goto cleanup;
}
// Update Cache Structure with the offset for the next level of cache
@@ -835,7 +878,7 @@ AddCacheTypeStructures (
CacheInfoNode->NumberOfSets,
Status
));
- return Status;
+ goto cleanup;
}
if (CacheInfoNode->NumberOfSets > PPTT_ARM_CACHE_NUMBER_OF_SETS_MAX) {
@@ -862,7 +905,7 @@ AddCacheTypeStructures (
CacheInfoNode->Associativity,
Status
));
- return Status;
+ goto cleanup;
}
// Validate the Associativity field based on the architecture specification
@@ -881,7 +924,7 @@ AddCacheTypeStructures (
CacheInfoNode->Associativity,
Status
));
- return Status;
+ goto cleanup;
}
if (CacheInfoNode->Associativity > PPTT_ARM_CACHE_ASSOCIATIVITY_MAX) {
@@ -923,7 +966,7 @@ AddCacheTypeStructures (
CacheInfoNode->LineSize,
Status
));
- return Status;
+ goto cleanup;
}
if ((CacheInfoNode->LineSize & (CacheInfoNode->LineSize - 1)) != 0) {
@@ -935,18 +978,58 @@ AddCacheTypeStructures (
CacheInfoNode->LineSize,
Status
));
- return Status;
+ goto cleanup;
}
CacheStruct->LineSize = CacheInfoNode->LineSize;
+ if (Revision >= 3) {
+ // Validate and populate cache id
+ if (CacheInfoNode->CacheId == 0) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: PPTT: The cache id cannot be zero. Status = %r\n",
+ Status
+ ));
+ goto cleanup;
+ }
+
+ CacheIdUnique = IsCacheIdUnique (
+ CacheInfoNode->CacheId,
+ FoundCacheIds,
+ TotalNodeCount
+ );
+ if (!CacheIdUnique) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: PPTT: The cache id is not unique. " \
+ "CacheId = %d. Status = %r\n",
+ CacheInfoNode->CacheId,
+ Status
+ ));
+ goto cleanup;
+ }
+
+ // Store the cache id so we can check future cache ids for uniqueness
+ FoundCacheIds[NodeCount] = CacheInfoNode->CacheId;
+
+ CacheStruct->CacheId = CacheInfoNode->CacheId;
+ }
+
// Next Cache Type Structure
CacheStruct = (EFI_ACPI_6_4_PPTT_STRUCTURE_CACHE *)((UINT8 *)CacheStruct +
CacheStruct->Length);
CacheNodeIterator++;
} // Cache Type Structure
- return EFI_SUCCESS;
+ Status = EFI_SUCCESS;
+
+cleanup:
+ FreePool (FoundCacheIds);
+
+ return Status;
}
/**
@@ -1205,7 +1288,8 @@ BuildPpttTable (
Generator,
CfgMgrProtocol,
Pptt,
- CacheStructOffset
+ CacheStructOffset,
+ AcpiTableInfo->AcpiTableRevision
);
if (EFI_ERROR (Status)) {
DEBUG ((
--
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v3 7/7] DynamicTablesPkg: Add CacheId to PPTT generator
2021-12-08 16:06 ` [PATCH v3 7/7] DynamicTablesPkg: Add CacheId to PPTT generator Chris Jones
@ 2021-12-09 13:19 ` Sami Mujawar
2021-12-09 13:42 ` Chris Jones
0 siblings, 1 reply; 12+ messages in thread
From: Sami Mujawar @ 2021-12-09 13:19 UTC (permalink / raw)
To: Chris Jones, devel
Cc: michael.d.kinney, gaoliming, zhiguang.liu, ray.ni, zhichao.gao,
Alexei.Fedorov, nd
Hi Chris,
Thank you for this patch.
Please see my feedback below inline marked [SAMI].
Regards,
Sami Mujawar
On 08/12/2021 04:06 PM, Chris Jones wrote:
> Bugzilla: 3697 (https://bugzilla.tianocore.org/show_bug.cgi?id=3697)
>
> Update the PPTT generator with the CacheId field as defined in table
> 5.140 of the ACPI 6.4 specification.
>
> Also add validations to ensure that the cache id generated is unique.
>
> Signed-off-by: Chris Jones <christopher.jones@arm.com>
> ---
> DynamicTablesPkg/Include/ArmNameSpaceObjects.h | 4 +-
> DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c | 102 ++++++++++++++++++--
> 2 files changed, 96 insertions(+), 10 deletions(-)
>
> diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> index 3246e8884723ac85340bf880a3859800726be3c1..6ea03fca487b96577b8fd8105bc3d22047ff5697 100644
> --- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> +++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> @@ -741,10 +741,12 @@ typedef struct CmArmCacheInfo {
> /// PPTT_ARM_CCIDX_CACHE_ASSOCIATIVITY_MAX. Therfore this field
> /// is 32-bit wide.
> UINT32 Associativity;
> - /// Cache attributes (ACPI 6.3 - January 2019, PPTT, Table 5-156)
> + /// Cache attributes (ACPI 6.4 - January 2021, PPTT, Table 5.140)
> UINT8 Attributes;
> /// Line size in bytes
> UINT16 LineSize;
> + /// Unique ID for the cache
> + UINT32 CacheId;
> } CM_ARM_CACHE_INFO;
>
> /** A structure that describes a reference to another Configuration Manager
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
> index 3d416ca78ec16a1929ede87abbe4f8f4464ef0cf..6b74572ea2dd8478f14d013e6cb7394216e45d8d 100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
> @@ -726,6 +726,35 @@ AddProcHierarchyNodes (
> return Status;
> }
>
> +/**
> + Test whether CacheId is unique among the CacheIdList.
> +
> + @param [in] CacheId Cache ID to check.
> + @param [in] CacheIdList List of already existing cache IDs.
> + @param [in] CacheIdListSize Size of CacheIdList.
> +
> + @retval TRUE CacheId does not exist in CacheIdList.
> + @retval FALSE CacheId already exists in CacheIdList.
> +**/
> +STATIC
> +BOOLEAN
> +IsCacheIdUnique (
> + IN CONST UINT32 CacheId,
> + IN CONST UINT32 *CacheIdList,
> + IN CONST UINT32 CacheIdListSize
> + )
> +{
> + UINT32 Index;
> +
> + for (Index = 0; Index < CacheIdListSize; Index++) {
> + if (CacheIdList[Index] == CacheId) {
> + return FALSE;
> + }
> + }
> +
> + return TRUE;
> +}
> +
> /**
> Update the Cache Type Structure (Type 1) information.
>
> @@ -738,10 +767,12 @@ AddProcHierarchyNodes (
> @param [in] Pptt Pointer to PPTT table structure.
> @param [in] NodesStartOffset Offset from the start of PPTT table to the
> start of Cache Type Structures.
> + @param [in] Revision Revision of the PPTT table being requested.
>
> @retval EFI_SUCCESS Structures updated successfully.
> @retval EFI_INVALID_PARAMETER A parameter is invalid.
> @retval EFI_NOT_FOUND A required object was not found.
> + @retval EFI_OUT_OF_RESOURCES Out of resources.
> **/
> STATIC
> EFI_STATUS
> @@ -749,7 +780,8 @@ AddCacheTypeStructures (
> IN CONST ACPI_PPTT_GENERATOR *CONST Generator,
> IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
> IN CONST EFI_ACPI_6_4_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_HEADER *Pptt,
> - IN CONST UINT32 NodesStartOffset
> + IN CONST UINT32 NodesStartOffset,
> + IN CONST UINT32 Revision
> )
> {
> EFI_STATUS Status;
> @@ -758,6 +790,9 @@ AddCacheTypeStructures (
> CM_ARM_CACHE_INFO *CacheInfoNode;
> PPTT_NODE_INDEXER *CacheNodeIterator;
> UINT32 NodeCount;
> + BOOLEAN CacheIdUnique;
> + UINT32 TotalNodeCount;
[SAMI] I think we could do slight optimisation by doing the following below:
(a) change TotalNode Count to NodeIndex.
> + UINT32 *FoundCacheIds;
>
> ASSERT (
> (Generator != NULL) &&
> @@ -770,6 +805,13 @@ AddCacheTypeStructures (
>
> CacheNodeIterator = Generator->CacheStructIndexedList;
> NodeCount = Generator->CacheStructCount;
> + TotalNodeCount = NodeCount;
> +
> + FoundCacheIds = AllocateZeroPool (TotalNodeCount * sizeof (*FoundCacheIds));
[SAMI] (b) Change TotalNodeCount to NodeCount.
> + if (FoundCacheIds == NULL) {
> + DEBUG ((DEBUG_ERROR, "ERROR: PPTT: Failed to allocate resources.\n"));
> + return EFI_OUT_OF_RESOURCES;
> + }
>
> while (NodeCount-- != 0) {
[SAMI] (c) Replace while loop to a for loop where NodeIndes = 0 to
NodeCount.
> CacheInfoNode = (CM_ARM_CACHE_INFO *)CacheNodeIterator->Object;
> @@ -789,6 +831,7 @@ AddCacheTypeStructures (
> CacheStruct->Flags.CacheTypeValid = 1;
> CacheStruct->Flags.WritePolicyValid = 1;
> CacheStruct->Flags.LineSizeValid = 1;
> + CacheStruct->Flags.CacheIdValid = 1;
> CacheStruct->Flags.Reserved = 0;
>
> // Populate the reference to the next level of cache
> @@ -811,7 +854,7 @@ AddCacheTypeStructures (
> CacheInfoNode->Token,
> Status
> ));
> - return Status;
> + goto cleanup;
> }
>
> // Update Cache Structure with the offset for the next level of cache
> @@ -835,7 +878,7 @@ AddCacheTypeStructures (
> CacheInfoNode->NumberOfSets,
> Status
> ));
> - return Status;
> + goto cleanup;
> }
>
> if (CacheInfoNode->NumberOfSets > PPTT_ARM_CACHE_NUMBER_OF_SETS_MAX) {
> @@ -862,7 +905,7 @@ AddCacheTypeStructures (
> CacheInfoNode->Associativity,
> Status
> ));
> - return Status;
> + goto cleanup;
> }
>
> // Validate the Associativity field based on the architecture specification
> @@ -881,7 +924,7 @@ AddCacheTypeStructures (
> CacheInfoNode->Associativity,
> Status
> ));
> - return Status;
> + goto cleanup;
> }
>
> if (CacheInfoNode->Associativity > PPTT_ARM_CACHE_ASSOCIATIVITY_MAX) {
> @@ -923,7 +966,7 @@ AddCacheTypeStructures (
> CacheInfoNode->LineSize,
> Status
> ));
> - return Status;
> + goto cleanup;
> }
>
> if ((CacheInfoNode->LineSize & (CacheInfoNode->LineSize - 1)) != 0) {
> @@ -935,18 +978,58 @@ AddCacheTypeStructures (
> CacheInfoNode->LineSize,
> Status
> ));
> - return Status;
> + goto cleanup;
> }
>
> CacheStruct->LineSize = CacheInfoNode->LineSize;
>
> + if (Revision >= 3) {
> + // Validate and populate cache id
> + if (CacheInfoNode->CacheId == 0) {
> + Status = EFI_INVALID_PARAMETER;
> + DEBUG ((
> + DEBUG_ERROR,
> + "ERROR: PPTT: The cache id cannot be zero. Status = %r\n",
> + Status
> + ));
> + goto cleanup;
> + }
> +
> + CacheIdUnique = IsCacheIdUnique (
> + CacheInfoNode->CacheId,
> + FoundCacheIds,
> + TotalNodeCount
[SAMI] (d) Replace TotalNodeCount with NodeIndex in call above.
By doing (a) to (d) and (e) below we can reduce the
number of iterations in IsCacheIdUnique ().
If you agree I will make the adjustments before pushing
this patch series.
[/SAMI]
> + );
> + if (!CacheIdUnique) {
> + Status = EFI_INVALID_PARAMETER;
> + DEBUG ((
> + DEBUG_ERROR,
> + "ERROR: PPTT: The cache id is not unique. " \
> + "CacheId = %d. Status = %r\n",
> + CacheInfoNode->CacheId,
> + Status
> + ));
> + goto cleanup;
> + }
> +
> + // Store the cache id so we can check future cache ids for uniqueness
> + FoundCacheIds[NodeCount] = CacheInfoNode->CacheId;
[SAMI] (e) Replace NodeCount with NodeIndex above.
> +
> + CacheStruct->CacheId = CacheInfoNode->CacheId;
> + }
> +
> // Next Cache Type Structure
> CacheStruct = (EFI_ACPI_6_4_PPTT_STRUCTURE_CACHE *)((UINT8 *)CacheStruct +
> CacheStruct->Length);
> CacheNodeIterator++;
> } // Cache Type Structure
>
> - return EFI_SUCCESS;
> + Status = EFI_SUCCESS;
> +
> +cleanup:
> + FreePool (FoundCacheIds);
> +
> + return Status;
> }
>
> /**
> @@ -1205,7 +1288,8 @@ BuildPpttTable (
> Generator,
> CfgMgrProtocol,
> Pptt,
> - CacheStructOffset
> + CacheStructOffset,
> + AcpiTableInfo->AcpiTableRevision
> );
> if (EFI_ERROR (Status)) {
> DEBUG ((
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 7/7] DynamicTablesPkg: Add CacheId to PPTT generator
2021-12-09 13:19 ` Sami Mujawar
@ 2021-12-09 13:42 ` Chris Jones
0 siblings, 0 replies; 12+ messages in thread
From: Chris Jones @ 2021-12-09 13:42 UTC (permalink / raw)
To: Sami Mujawar, devel@edk2.groups.io
Cc: michael.d.kinney@intel.com, gaoliming@byosoft.com.cn,
zhiguang.liu@intel.com, ray.ni@intel.com, zhichao.gao@intel.com,
Alexei Fedorov, nd
[-- Attachment #1: Type: text/plain, Size: 10535 bytes --]
Hi Sami,
The optimisation looks good to me and I would be glad if you added it to the patch.
Thanks,
Chris
________________________________
From: Sami Mujawar <Sami.Mujawar@arm.com>
Sent: Thursday, December 9, 2021 1:19 PM
To: Christopher Jones <Christopher.Jones@arm.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: michael.d.kinney@intel.com <michael.d.kinney@intel.com>; gaoliming@byosoft.com.cn <gaoliming@byosoft.com.cn>; zhiguang.liu@intel.com <zhiguang.liu@intel.com>; ray.ni@intel.com <ray.ni@intel.com>; zhichao.gao@intel.com <zhichao.gao@intel.com>; Alexei Fedorov <Alexei.Fedorov@arm.com>; nd <nd@arm.com>
Subject: Re: [PATCH v3 7/7] DynamicTablesPkg: Add CacheId to PPTT generator
Hi Chris,
Thank you for this patch.
Please see my feedback below inline marked [SAMI].
Regards,
Sami Mujawar
On 08/12/2021 04:06 PM, Chris Jones wrote:
> Bugzilla: 3697 (https://bugzilla.tianocore.org/show_bug.cgi?id=3697)
>
> Update the PPTT generator with the CacheId field as defined in table
> 5.140 of the ACPI 6.4 specification.
>
> Also add validations to ensure that the cache id generated is unique.
>
> Signed-off-by: Chris Jones <christopher.jones@arm.com>
> ---
> DynamicTablesPkg/Include/ArmNameSpaceObjects.h | 4 +-
> DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c | 102 ++++++++++++++++++--
> 2 files changed, 96 insertions(+), 10 deletions(-)
>
> diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> index 3246e8884723ac85340bf880a3859800726be3c1..6ea03fca487b96577b8fd8105bc3d22047ff5697 100644
> --- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> +++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> @@ -741,10 +741,12 @@ typedef struct CmArmCacheInfo {
> /// PPTT_ARM_CCIDX_CACHE_ASSOCIATIVITY_MAX. Therfore this field
> /// is 32-bit wide.
> UINT32 Associativity;
> - /// Cache attributes (ACPI 6.3 - January 2019, PPTT, Table 5-156)
> + /// Cache attributes (ACPI 6.4 - January 2021, PPTT, Table 5.140)
> UINT8 Attributes;
> /// Line size in bytes
> UINT16 LineSize;
> + /// Unique ID for the cache
> + UINT32 CacheId;
> } CM_ARM_CACHE_INFO;
>
> /** A structure that describes a reference to another Configuration Manager
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
> index 3d416ca78ec16a1929ede87abbe4f8f4464ef0cf..6b74572ea2dd8478f14d013e6cb7394216e45d8d 100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
> @@ -726,6 +726,35 @@ AddProcHierarchyNodes (
> return Status;
> }
>
> +/**
> + Test whether CacheId is unique among the CacheIdList.
> +
> + @param [in] CacheId Cache ID to check.
> + @param [in] CacheIdList List of already existing cache IDs.
> + @param [in] CacheIdListSize Size of CacheIdList.
> +
> + @retval TRUE CacheId does not exist in CacheIdList.
> + @retval FALSE CacheId already exists in CacheIdList.
> +**/
> +STATIC
> +BOOLEAN
> +IsCacheIdUnique (
> + IN CONST UINT32 CacheId,
> + IN CONST UINT32 *CacheIdList,
> + IN CONST UINT32 CacheIdListSize
> + )
> +{
> + UINT32 Index;
> +
> + for (Index = 0; Index < CacheIdListSize; Index++) {
> + if (CacheIdList[Index] == CacheId) {
> + return FALSE;
> + }
> + }
> +
> + return TRUE;
> +}
> +
> /**
> Update the Cache Type Structure (Type 1) information.
>
> @@ -738,10 +767,12 @@ AddProcHierarchyNodes (
> @param [in] Pptt Pointer to PPTT table structure.
> @param [in] NodesStartOffset Offset from the start of PPTT table to the
> start of Cache Type Structures.
> + @param [in] Revision Revision of the PPTT table being requested.
>
> @retval EFI_SUCCESS Structures updated successfully.
> @retval EFI_INVALID_PARAMETER A parameter is invalid.
> @retval EFI_NOT_FOUND A required object was not found.
> + @retval EFI_OUT_OF_RESOURCES Out of resources.
> **/
> STATIC
> EFI_STATUS
> @@ -749,7 +780,8 @@ AddCacheTypeStructures (
> IN CONST ACPI_PPTT_GENERATOR *CONST Generator,
> IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
> IN CONST EFI_ACPI_6_4_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_HEADER *Pptt,
> - IN CONST UINT32 NodesStartOffset
> + IN CONST UINT32 NodesStartOffset,
> + IN CONST UINT32 Revision
> )
> {
> EFI_STATUS Status;
> @@ -758,6 +790,9 @@ AddCacheTypeStructures (
> CM_ARM_CACHE_INFO *CacheInfoNode;
> PPTT_NODE_INDEXER *CacheNodeIterator;
> UINT32 NodeCount;
> + BOOLEAN CacheIdUnique;
> + UINT32 TotalNodeCount;
[SAMI] I think we could do slight optimisation by doing the following below:
(a) change TotalNode Count to NodeIndex.
> + UINT32 *FoundCacheIds;
>
> ASSERT (
> (Generator != NULL) &&
> @@ -770,6 +805,13 @@ AddCacheTypeStructures (
>
> CacheNodeIterator = Generator->CacheStructIndexedList;
> NodeCount = Generator->CacheStructCount;
> + TotalNodeCount = NodeCount;
> +
> + FoundCacheIds = AllocateZeroPool (TotalNodeCount * sizeof (*FoundCacheIds));
[SAMI] (b) Change TotalNodeCount to NodeCount.
> + if (FoundCacheIds == NULL) {
> + DEBUG ((DEBUG_ERROR, "ERROR: PPTT: Failed to allocate resources.\n"));
> + return EFI_OUT_OF_RESOURCES;
> + }
>
> while (NodeCount-- != 0) {
[SAMI] (c) Replace while loop to a for loop where NodeIndes = 0 to
NodeCount.
> CacheInfoNode = (CM_ARM_CACHE_INFO *)CacheNodeIterator->Object;
> @@ -789,6 +831,7 @@ AddCacheTypeStructures (
> CacheStruct->Flags.CacheTypeValid = 1;
> CacheStruct->Flags.WritePolicyValid = 1;
> CacheStruct->Flags.LineSizeValid = 1;
> + CacheStruct->Flags.CacheIdValid = 1;
> CacheStruct->Flags.Reserved = 0;
>
> // Populate the reference to the next level of cache
> @@ -811,7 +854,7 @@ AddCacheTypeStructures (
> CacheInfoNode->Token,
> Status
> ));
> - return Status;
> + goto cleanup;
> }
>
> // Update Cache Structure with the offset for the next level of cache
> @@ -835,7 +878,7 @@ AddCacheTypeStructures (
> CacheInfoNode->NumberOfSets,
> Status
> ));
> - return Status;
> + goto cleanup;
> }
>
> if (CacheInfoNode->NumberOfSets > PPTT_ARM_CACHE_NUMBER_OF_SETS_MAX) {
> @@ -862,7 +905,7 @@ AddCacheTypeStructures (
> CacheInfoNode->Associativity,
> Status
> ));
> - return Status;
> + goto cleanup;
> }
>
> // Validate the Associativity field based on the architecture specification
> @@ -881,7 +924,7 @@ AddCacheTypeStructures (
> CacheInfoNode->Associativity,
> Status
> ));
> - return Status;
> + goto cleanup;
> }
>
> if (CacheInfoNode->Associativity > PPTT_ARM_CACHE_ASSOCIATIVITY_MAX) {
> @@ -923,7 +966,7 @@ AddCacheTypeStructures (
> CacheInfoNode->LineSize,
> Status
> ));
> - return Status;
> + goto cleanup;
> }
>
> if ((CacheInfoNode->LineSize & (CacheInfoNode->LineSize - 1)) != 0) {
> @@ -935,18 +978,58 @@ AddCacheTypeStructures (
> CacheInfoNode->LineSize,
> Status
> ));
> - return Status;
> + goto cleanup;
> }
>
> CacheStruct->LineSize = CacheInfoNode->LineSize;
>
> + if (Revision >= 3) {
> + // Validate and populate cache id
> + if (CacheInfoNode->CacheId == 0) {
> + Status = EFI_INVALID_PARAMETER;
> + DEBUG ((
> + DEBUG_ERROR,
> + "ERROR: PPTT: The cache id cannot be zero. Status = %r\n",
> + Status
> + ));
> + goto cleanup;
> + }
> +
> + CacheIdUnique = IsCacheIdUnique (
> + CacheInfoNode->CacheId,
> + FoundCacheIds,
> + TotalNodeCount
[SAMI] (d) Replace TotalNodeCount with NodeIndex in call above.
By doing (a) to (d) and (e) below we can reduce the
number of iterations in IsCacheIdUnique ().
If you agree I will make the adjustments before pushing
this patch series.
[/SAMI]
> + );
> + if (!CacheIdUnique) {
> + Status = EFI_INVALID_PARAMETER;
> + DEBUG ((
> + DEBUG_ERROR,
> + "ERROR: PPTT: The cache id is not unique. " \
> + "CacheId = %d. Status = %r\n",
> + CacheInfoNode->CacheId,
> + Status
> + ));
> + goto cleanup;
> + }
> +
> + // Store the cache id so we can check future cache ids for uniqueness
> + FoundCacheIds[NodeCount] = CacheInfoNode->CacheId;
[SAMI] (e) Replace NodeCount with NodeIndex above.
> +
> + CacheStruct->CacheId = CacheInfoNode->CacheId;
> + }
> +
> // Next Cache Type Structure
> CacheStruct = (EFI_ACPI_6_4_PPTT_STRUCTURE_CACHE *)((UINT8 *)CacheStruct +
> CacheStruct->Length);
> CacheNodeIterator++;
> } // Cache Type Structure
>
> - return EFI_SUCCESS;
> + Status = EFI_SUCCESS;
> +
> +cleanup:
> + FreePool (FoundCacheIds);
> +
> + return Status;
> }
>
> /**
> @@ -1205,7 +1288,8 @@ BuildPpttTable (
> Generator,
> CfgMgrProtocol,
> Pptt,
> - CacheStructOffset
> + CacheStructOffset,
> + AcpiTableInfo->AcpiTableRevision
> );
> if (EFI_ERROR (Status)) {
> DEBUG ((
[-- Attachment #2: Type: text/html, Size: 22906 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 0/7] Support ACPI 6.4 PPTT changes
2021-12-08 16:06 [PATCH v3 0/7] Support ACPI 6.4 PPTT changes Chris Jones
` (6 preceding siblings ...)
2021-12-08 16:06 ` [PATCH v3 7/7] DynamicTablesPkg: Add CacheId to PPTT generator Chris Jones
@ 2021-12-09 17:00 ` Sami Mujawar
[not found] ` <16BF24D08620153C.20064@groups.io>
8 siblings, 0 replies; 12+ messages in thread
From: Sami Mujawar @ 2021-12-09 17:00 UTC (permalink / raw)
To: Chris Jones, devel
Cc: michael.d.kinney, gaoliming, zhiguang.liu, ray.ni, zhichao.gao,
Alexei.Fedorov, nd
Hi Chris,
Thank you for this patch series.
Other than my feedback for optimisation in patch 7/7 (which I will
address before merging), this patch series looks good to me.
For this series,
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Regards,
Sami Mujawar
On 08/12/2021 04:06 PM, Chris Jones wrote:
> Bugzilla: 3697 (https://bugzilla.tianocore.org/show_bug.cgi?id=3697)
>
> This patch series updates the Acpiview PPTT parser and DynamicTablesPkg
> PPTT generator to support ACPI 6.4. This consists of two main changes:
> - The addition of the 'Cache ID' field.
> - The removal of the PPTT ID (type 2) structure.
>
> In addition add two 'Cache ID' defines and remove the type 2 PPTT
> structure from Acpi64.h as these changes were missing when Acpi64.h was
> introduced.
>
> Changes since v2:
> 1. Added mantis ID for PPTT type 2 structure removal on relevant commit
> messages. No change to code.
> 2. Applied uncrustify to all patches. No other changes to code.
>
> Changes since v1:
> 1. Patch 3/7 ShellPkg:
> - Changed PPTT ID error message to say "removed" instead of "deprecated".
> 2. Patch 4/7 ShellPkg:
> - Fixed a bug where 'CacheFlags' and 'CacheId' were only set after the
> validation function had finished. Instead set them inside the
> validation function using the first 'Ptr' parameter.
> 3. Patch 7/7 DynamicTablesPkg:
> - Make IsCacheIdUnique() return BOOLEAN instead of EFI_STATUS.
> - Added a missing space to the definition of 'IsCacheIdUnique'.
>
> The changes can be seen at: https://github.com/chris-jones-arm/edk2/tree/1632_64_acpi_cache_id_v3
>
>
> Chris Jones (7):
> MdePkg: Add missing Cache ID (in)valid define
> MdePkg: Remove PPTT ID type structure
> ShellPkg: Update Acpiview PPTT parser to ACPI 6.4
> ShellPkg: Add Cache ID to PPTT parser
> DynamicTablesPkg: Remove PPTT ID structure from ACPI 6.4 generator
> DynamicTablesPkg: Update PPTT generator to ACPI 6.4
> DynamicTablesPkg: Add CacheId to PPTT generator
>
> .../Include/ArmNameSpaceObjects.h | 27 +-
> .../Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c | 296 +++++++-----------
> .../Acpi/Arm/AcpiPpttLibArm/PpttGenerator.h | 4 -
> MdePkg/Include/IndustryStandard/Acpi64.h | 18 +-
> .../Parsers/Pptt/PpttParser.c | 214 +++++++++----
> .../UefiShellAcpiViewCommandLib.c | 2 +-
> 6 files changed, 272 insertions(+), 289 deletions(-)
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH v3 0/7] Support ACPI 6.4 PPTT changes
[not found] ` <16BF24D08620153C.20064@groups.io>
@ 2021-12-10 20:10 ` Sami Mujawar
0 siblings, 0 replies; 12+ messages in thread
From: Sami Mujawar @ 2021-12-10 20:10 UTC (permalink / raw)
To: devel, Chris Jones
Cc: michael.d.kinney, gaoliming, zhiguang.liu, ray.ni, zhichao.gao,
Alexei.Fedorov, nd
Merged as 0f1d7477c0a8..e81a81e5846e
Regards,
Sami Mujawar
On 09/12/2021 05:00 PM, Sami Mujawar via groups.io wrote:
> Hi Chris,
>
> Thank you for this patch series.
>
> Other than my feedback for optimisation in patch 7/7 (which I will
> address before merging), this patch series looks good to me.
>
> For this series,
>
> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
>
> Regards,
>
> Sami Mujawar
>
> On 08/12/2021 04:06 PM, Chris Jones wrote:
>> Bugzilla: 3697 (https://bugzilla.tianocore.org/show_bug.cgi?id=3697)
>>
>> This patch series updates the Acpiview PPTT parser and DynamicTablesPkg
>> PPTT generator to support ACPI 6.4. This consists of two main changes:
>> - The addition of the 'Cache ID' field.
>> - The removal of the PPTT ID (type 2) structure.
>>
>> In addition add two 'Cache ID' defines and remove the type 2 PPTT
>> structure from Acpi64.h as these changes were missing when Acpi64.h was
>> introduced.
>>
>> Changes since v2:
>> 1. Added mantis ID for PPTT type 2 structure removal on relevant commit
>> messages. No change to code.
>> 2. Applied uncrustify to all patches. No other changes to code.
>>
>> Changes since v1:
>> 1. Patch 3/7 ShellPkg:
>> - Changed PPTT ID error message to say "removed" instead of
>> "deprecated".
>> 2. Patch 4/7 ShellPkg:
>> - Fixed a bug where 'CacheFlags' and 'CacheId' were only set
>> after the
>> validation function had finished. Instead set them inside the
>> validation function using the first 'Ptr' parameter.
>> 3. Patch 7/7 DynamicTablesPkg:
>> - Make IsCacheIdUnique() return BOOLEAN instead of EFI_STATUS.
>> - Added a missing space to the definition of 'IsCacheIdUnique'.
>>
>> The changes can be seen at:
>> https://github.com/chris-jones-arm/edk2/tree/1632_64_acpi_cache_id_v3
>>
>>
>> Chris Jones (7):
>> MdePkg: Add missing Cache ID (in)valid define
>> MdePkg: Remove PPTT ID type structure
>> ShellPkg: Update Acpiview PPTT parser to ACPI 6.4
>> ShellPkg: Add Cache ID to PPTT parser
>> DynamicTablesPkg: Remove PPTT ID structure from ACPI 6.4 generator
>> DynamicTablesPkg: Update PPTT generator to ACPI 6.4
>> DynamicTablesPkg: Add CacheId to PPTT generator
>>
>> .../Include/ArmNameSpaceObjects.h | 27 +-
>> .../Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c | 296 +++++++-----------
>> .../Acpi/Arm/AcpiPpttLibArm/PpttGenerator.h | 4 -
>> MdePkg/Include/IndustryStandard/Acpi64.h | 18 +-
>> .../Parsers/Pptt/PpttParser.c | 214 +++++++++----
>> .../UefiShellAcpiViewCommandLib.c | 2 +-
>> 6 files changed, 272 insertions(+), 289 deletions(-)
>>
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-12-10 20:11 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-12-08 16:06 [PATCH v3 0/7] Support ACPI 6.4 PPTT changes Chris Jones
2021-12-08 16:06 ` [PATCH v3 1/7] MdePkg: Add missing Cache ID (in)valid define Chris Jones
2021-12-08 16:06 ` [PATCH v3 2/7] MdePkg: Remove PPTT ID type structure Chris Jones
2021-12-08 16:06 ` [PATCH v3 3/7] ShellPkg: Update Acpiview PPTT parser to ACPI 6.4 Chris Jones
2021-12-08 16:06 ` [PATCH v3 4/7] ShellPkg: Add Cache ID to PPTT parser Chris Jones
2021-12-08 16:06 ` [PATCH v3 5/7] DynamicTablesPkg: Remove PPTT ID structure from ACPI 6.4 generator Chris Jones
2021-12-08 16:06 ` [PATCH v3 6/7] DynamicTablesPkg: Update PPTT generator to ACPI 6.4 Chris Jones
2021-12-08 16:06 ` [PATCH v3 7/7] DynamicTablesPkg: Add CacheId to PPTT generator Chris Jones
2021-12-09 13:19 ` Sami Mujawar
2021-12-09 13:42 ` Chris Jones
2021-12-09 17:00 ` [PATCH v3 0/7] Support ACPI 6.4 PPTT changes Sami Mujawar
[not found] ` <16BF24D08620153C.20064@groups.io>
2021-12-10 20:10 ` [edk2-devel] " Sami Mujawar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox