public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2 0/7] Support ACPI 6.4 PPTT changes
@ 2021-11-03 15:41 Chris Jones
  2021-11-03 15:41 ` [PATCH v2 1/7] MdePkg: Add missing Cache ID (in)valid define Chris Jones
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Chris Jones @ 2021-11-03 15:41 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 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_v2

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   | 297 +++++++-----------
 .../Acpi/Arm/AcpiPpttLibArm/PpttGenerator.h   |   4 -
 MdePkg/Include/IndustryStandard/Acpi64.h      |  18 +-
 .../Parsers/Pptt/PpttParser.c                 | 181 ++++++++---
 .../UefiShellAcpiViewCommandLib.c             |   2 +-
 6 files changed, 251 insertions(+), 278 deletions(-)

-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/7] MdePkg: Add missing Cache ID (in)valid define
  2021-11-03 15:41 [PATCH v2 0/7] Support ACPI 6.4 PPTT changes Chris Jones
@ 2021-11-03 15:41 ` Chris Jones
  2021-11-09  1:19   ` [edk2-devel] " Gao, Zhichao
  2021-11-03 15:41 ` [PATCH v2 2/7] MdePkg: Remove PPTT ID type structure Chris Jones
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Chris Jones @ 2021-11-03 15:41 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>
---
 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 3a91302f8c0e71d4951d27aac35322073219c836..625504ba77b4000c0a0b540299d2e0e42c768400 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] 11+ messages in thread

* [PATCH v2 2/7] MdePkg: Remove PPTT ID type structure
  2021-11-03 15:41 [PATCH v2 0/7] Support ACPI 6.4 PPTT changes Chris Jones
  2021-11-03 15:41 ` [PATCH v2 1/7] MdePkg: Add missing Cache ID (in)valid define Chris Jones
@ 2021-11-03 15:41 ` Chris Jones
  2021-11-03 15:41 ` [PATCH v2 3/7] ShellPkg: Update Acpiview PPTT parser to ACPI 6.4 Chris Jones
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Chris Jones @ 2021-11-03 15:41 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>
---
 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 625504ba77b4000c0a0b540299d2e0e42c768400..bc1522722424156f0381e848c2617760f2d31aba 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] 11+ messages in thread

* [PATCH v2 3/7] ShellPkg: Update Acpiview PPTT parser to ACPI 6.4
  2021-11-03 15:41 [PATCH v2 0/7] Support ACPI 6.4 PPTT changes Chris Jones
  2021-11-03 15:41 ` [PATCH v2 1/7] MdePkg: Add missing Cache ID (in)valid define Chris Jones
  2021-11-03 15:41 ` [PATCH v2 2/7] MdePkg: Remove PPTT ID type structure Chris Jones
@ 2021-11-03 15:41 ` Chris Jones
  2021-11-09  1:20   ` Gao, Zhichao
  2021-11-03 15:41 ` [PATCH v2 4/7] ShellPkg: Add Cache ID to PPTT parser Chris Jones
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Chris Jones @ 2021-11-03 15:41 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.

Signed-off-by: Chris Jones <christopher.jones@arm.com>
---

Notes:
    v2:
    - Changed PPTT ID error message to say "removed" instead of "deprecated".

 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 acd2b81bb3258c7322aa10d2c0e0d842d89e358b..bb840a0dbab240d371aa58d323e61f47fa8d1587 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)
 **/
 
@@ -157,8 +157,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;
 
@@ -222,22 +222,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).
 
@@ -335,29 +319,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
@@ -366,7 +327,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.
 
@@ -444,22 +404,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 d725cad14c5d018e2004eb8e33c845aa9c719429..ab9e6c619d70df4f79d782416037d7bef62c92d5 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] 11+ messages in thread

* [PATCH v2 4/7] ShellPkg: Add Cache ID to PPTT parser
  2021-11-03 15:41 [PATCH v2 0/7] Support ACPI 6.4 PPTT changes Chris Jones
                   ` (2 preceding siblings ...)
  2021-11-03 15:41 ` [PATCH v2 3/7] ShellPkg: Update Acpiview PPTT parser to ACPI 6.4 Chris Jones
@ 2021-11-03 15:41 ` Chris Jones
  2021-11-09  1:20   ` [edk2-devel] " Gao, Zhichao
  2021-11-03 15:41 ` [PATCH v2 5/7] DynamicTablesPkg: Remove PPTT ID structure from ACPI 6.4 generator Chris Jones
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Chris Jones @ 2021-11-03 15:41 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>
---

Notes:
    v2:
    - 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.

 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c | 120 +++++++++++++++++++-
 1 file changed, 118 insertions(+), 2 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
index bb840a0dbab240d371aa58d323e61f47fa8d1587..3f93038ce1d83c005ae3d6a43e11f309440ad6fa 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
@@ -20,8 +20,84 @@
 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'
   field.
@@ -141,6 +217,44 @@ 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.
 
@@ -213,13 +327,15 @@ STATIC CONST ACPI_PARSER CacheTypeStructureParser[] = {
   {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"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"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] 11+ messages in thread

* [PATCH v2 5/7] DynamicTablesPkg: Remove PPTT ID structure from ACPI 6.4 generator
  2021-11-03 15:41 [PATCH v2 0/7] Support ACPI 6.4 PPTT changes Chris Jones
                   ` (3 preceding siblings ...)
  2021-11-03 15:41 ` [PATCH v2 4/7] ShellPkg: Add Cache ID to PPTT parser Chris Jones
@ 2021-11-03 15:41 ` Chris Jones
  2021-11-03 15:41 ` [PATCH v2 6/7] DynamicTablesPkg: Update PPTT generator to ACPI 6.4 Chris Jones
  2021-11-03 15:41 ` [PATCH v2 7/7] DynamicTablesPkg: Add CacheId to PPTT generator Chris Jones
  6 siblings, 0 replies; 11+ messages in thread
From: Chris Jones @ 2021-11-03 15:41 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.

Signed-off-by: Chris Jones <christopher.jones@arm.com>
---
 DynamicTablesPkg/Include/ArmNameSpaceObjects.h                   |  23 +--
 DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c | 155 +-------------------
 DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.h |   4 -
 3 files changed, 3 insertions(+), 179 deletions(-)

diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
index f19c9c70666970bb70b6aa09f064bb10a9a67112..6bc5ab6b2b28424c1afddc26cc89a54b81941aeb 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
@@ -746,27 +746,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 d70fc59e754e7d348965b8c3739822a9f1c4b7e6..65d1661c0ec47a4d1631c0dcac7bcb16dbc619e0 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.
@@ -372,8 +352,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 -
@@ -964,72 +944,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.
 
@@ -1068,15 +982,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;
 
@@ -1150,27 +1061,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) *
@@ -1237,27 +1127,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" \
@@ -1343,24 +1212,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)) {
@@ -1484,8 +1335,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 0a14da502d595e27d87262b1bac681318f1d9ced..59ddd71ac4ef57d748e143afaa6ac0befb4900e0 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] 11+ messages in thread

* [PATCH v2 6/7] DynamicTablesPkg: Update PPTT generator to ACPI 6.4
  2021-11-03 15:41 [PATCH v2 0/7] Support ACPI 6.4 PPTT changes Chris Jones
                   ` (4 preceding siblings ...)
  2021-11-03 15:41 ` [PATCH v2 5/7] DynamicTablesPkg: Remove PPTT ID structure from ACPI 6.4 generator Chris Jones
@ 2021-11-03 15:41 ` Chris Jones
  2021-11-03 15:41 ` [PATCH v2 7/7] DynamicTablesPkg: Add CacheId to PPTT generator Chris Jones
  6 siblings, 0 replies; 11+ messages in thread
From: Chris Jones @ 2021-11-03 15:41 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 65d1661c0ec47a4d1631c0dcac7bcb16dbc619e0..18f093998db57ea4698953ed06a9826df559c1b7 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
   );
 
@@ -466,12 +466,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;
 
@@ -492,7 +492,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;
@@ -536,7 +536,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;
@@ -657,7 +657,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
@@ -680,7 +680,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
@@ -744,12 +744,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;
@@ -761,7 +761,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;
@@ -771,13 +771,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;
@@ -936,7 +936,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
@@ -994,7 +994,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) &&
@@ -1081,7 +1081,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) {
@@ -1150,7 +1150,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,
@@ -1307,9 +1307,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] 11+ messages in thread

* [PATCH v2 7/7] DynamicTablesPkg: Add CacheId to PPTT generator
  2021-11-03 15:41 [PATCH v2 0/7] Support ACPI 6.4 PPTT changes Chris Jones
                   ` (5 preceding siblings ...)
  2021-11-03 15:41 ` [PATCH v2 6/7] DynamicTablesPkg: Update PPTT generator to ACPI 6.4 Chris Jones
@ 2021-11-03 15:41 ` Chris Jones
  6 siblings, 0 replies; 11+ messages in thread
From: Chris Jones @ 2021-11-03 15:41 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>
---

Notes:
    v2:
    - Make IsCacheIdUnique() return BOOLEAN instead of EFI_STATUS.
    - Added a missing space to the definition of 'IsCacheIdUnique'.

 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 6bc5ab6b2b28424c1afddc26cc89a54b81941aeb..6e27158d720947efb22a350a7e5ffcfdd3005361 100644
--- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
@@ -740,10 +740,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 18f093998db57ea4698953ed06a9826df559c1b7..b74ab0b157b89aed576d6d100fadbf28182f80fa 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
@@ -722,6 +722,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.
 
@@ -734,10 +763,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
@@ -745,7 +776,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;
@@ -754,6 +786,9 @@ AddCacheTypeStructures (
   CM_ARM_CACHE_INFO                   * CacheInfoNode;
   PPTT_NODE_INDEXER                   * CacheNodeIterator;
   UINT32                                NodeCount;
+  BOOLEAN                               CacheIdUnique;
+  UINT32                                TotalNodeCount;
+  UINT32                              * FoundCacheIds;
 
   ASSERT (
     (Generator != NULL) &&
@@ -766,6 +801,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;
@@ -785,6 +827,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
@@ -807,7 +850,7 @@ AddCacheTypeStructures (
           CacheInfoNode->Token,
           Status
           ));
-        return Status;
+        goto cleanup;
       }
 
       // Update Cache Structure with the offset for the next level of cache
@@ -831,7 +874,7 @@ AddCacheTypeStructures (
         CacheInfoNode->NumberOfSets,
         Status
         ));
-      return Status;
+      goto cleanup;
     }
 
     if (CacheInfoNode->NumberOfSets > PPTT_ARM_CACHE_NUMBER_OF_SETS_MAX) {
@@ -858,7 +901,7 @@ AddCacheTypeStructures (
         CacheInfoNode->Associativity,
         Status
         ));
-      return Status;
+      goto cleanup;
     }
 
     // Validate the Associativity field based on the architecture specification
@@ -877,7 +920,7 @@ AddCacheTypeStructures (
         CacheInfoNode->Associativity,
         Status
         ));
-      return Status;
+      goto cleanup;
     }
 
     if (CacheInfoNode->Associativity > PPTT_ARM_CACHE_ASSOCIATIVITY_MAX) {
@@ -918,7 +961,7 @@ AddCacheTypeStructures (
         CacheInfoNode->LineSize,
         Status
         ));
-      return Status;
+      goto cleanup;
     }
 
     if ((CacheInfoNode->LineSize & (CacheInfoNode->LineSize - 1)) != 0) {
@@ -930,18 +973,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;
 }
 
 /**
@@ -1200,7 +1283,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] 11+ messages in thread

* Re: [edk2-devel] [PATCH v2 1/7] MdePkg: Add missing Cache ID (in)valid define
  2021-11-03 15:41 ` [PATCH v2 1/7] MdePkg: Add missing Cache ID (in)valid define Chris Jones
@ 2021-11-09  1:19   ` Gao, Zhichao
  0 siblings, 0 replies; 11+ messages in thread
From: Gao, Zhichao @ 2021-11-09  1:19 UTC (permalink / raw)
  To: devel@edk2.groups.io, christopher.jones@arm.com
  Cc: Kinney, Michael D, gaoliming@byosoft.com.cn, Liu, Zhiguang,
	Ni, Ray, Alexei.Fedorov@arm.com, Sami.Mujawar@arm.com, nd@arm.com

Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

Thanks,
Zhichao

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chris
> Jones
> Sent: Wednesday, November 3, 2021 11:41 PM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> gaoliming@byosoft.com.cn; Liu, Zhiguang <zhiguang.liu@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>;
> Alexei.Fedorov@arm.com; Sami.Mujawar@arm.com; nd@arm.com
> Subject: [edk2-devel] [PATCH v2 1/7] MdePkg: Add missing Cache ID (in)valid
> define
> 
> 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>
> ---
>  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
> 3a91302f8c0e71d4951d27aac35322073219c836..625504ba77b4000c0a0b540299
> d2e0e42c768400 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	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/7] ShellPkg: Update Acpiview PPTT parser to ACPI 6.4
  2021-11-03 15:41 ` [PATCH v2 3/7] ShellPkg: Update Acpiview PPTT parser to ACPI 6.4 Chris Jones
@ 2021-11-09  1:20   ` Gao, Zhichao
  0 siblings, 0 replies; 11+ messages in thread
From: Gao, Zhichao @ 2021-11-09  1:20 UTC (permalink / raw)
  To: Chris Jones, devel@edk2.groups.io
  Cc: Kinney, Michael D, gaoliming@byosoft.com.cn, Liu, Zhiguang,
	Ni, Ray, Alexei.Fedorov@arm.com, Sami.Mujawar@arm.com, nd@arm.com

Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

Thanks,
Zhichao

> -----Original Message-----
> From: Chris Jones <christopher.jones@arm.com>
> Sent: Wednesday, November 3, 2021 11:41 PM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> gaoliming@byosoft.com.cn; Liu, Zhiguang <zhiguang.liu@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>;
> Alexei.Fedorov@arm.com; Sami.Mujawar@arm.com; nd@arm.com
> Subject: [PATCH v2 3/7] ShellPkg: Update Acpiview PPTT parser to ACPI 6.4
> 
> 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.
> 
> Signed-off-by: Chris Jones <christopher.jones@arm.com>
> ---
> 
> Notes:
>     v2:
>     - Changed PPTT ID error message to say "removed" instead of
> "deprecated".
> 
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
> | 61 ++++----------------
> 
> ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComman
> dLib.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
> acd2b81bb3258c7322aa10d2c0e0d842d89e358b..bb840a0dbab240d371aa58d3
> 23e61f47fa8d1587 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.
> c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttPars
> +++ er.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)  **/
> 
> @@ -157,8 +157,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;
> 
> @@ -222,22 +222,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).
> 
> @@ -335,29 +319,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 @@ -366,7
> +327,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.
> 
> @@ -444,22 +404,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/UefiShellAcpiViewComm
> andLib.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.c
> index
> d725cad14c5d018e2004eb8e33c845aa9c719429..ab9e6c619d70df4f79d782416
> 037d7bef62c92d5 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> +++ andLib.c
> @@ -62,7 +62,7 @@ ACPI_TABLE_PARSER ParserList[] = {
>     ParseAcpiMcfg},
> 
> {EFI_ACPI_6_4_PLATFORM_COMMUNICATIONS_CHANNEL_TABLE_SIGNAT
> URE,
>     ParseAcpiPcct},
> -
> {EFI_ACPI_6_2_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_S
> IGNATURE,
> +
> +
> {EFI_ACPI_6_4_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_S
> IGNATURE,
>     ParseAcpiPptt},
>    {RSDP_TABLE_INFO, ParseAcpiRsdp},
>    {EFI_ACPI_6_2_SYSTEM_LOCALITY_INFORMATION_TABLE_SIGNATURE,
> ParseAcpiSlit},
> --
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [edk2-devel] [PATCH v2 4/7] ShellPkg: Add Cache ID to PPTT parser
  2021-11-03 15:41 ` [PATCH v2 4/7] ShellPkg: Add Cache ID to PPTT parser Chris Jones
@ 2021-11-09  1:20   ` Gao, Zhichao
  0 siblings, 0 replies; 11+ messages in thread
From: Gao, Zhichao @ 2021-11-09  1:20 UTC (permalink / raw)
  To: devel@edk2.groups.io, christopher.jones@arm.com
  Cc: Kinney, Michael D, gaoliming@byosoft.com.cn, Liu, Zhiguang,
	Ni, Ray, Alexei.Fedorov@arm.com, Sami.Mujawar@arm.com, nd@arm.com

Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

Thanks,
Zhichao

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chris
> Jones
> Sent: Wednesday, November 3, 2021 11:41 PM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> gaoliming@byosoft.com.cn; Liu, Zhiguang <zhiguang.liu@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>;
> Alexei.Fedorov@arm.com; Sami.Mujawar@arm.com; nd@arm.com
> Subject: [edk2-devel] [PATCH v2 4/7] ShellPkg: Add Cache ID to PPTT parser
> 
> 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>
> ---
> 
> Notes:
>     v2:
>     - 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.
> 
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
> | 120 +++++++++++++++++++-
>  1 file changed, 118 insertions(+), 2 deletions(-)
> 
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.
> c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.
> c
> index
> bb840a0dbab240d371aa58d323e61f47fa8d1587..3f93038ce1d83c005ae3d6a43
> e11f309440ad6fa 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.
> c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttPars
> +++ er.c
> @@ -20,8 +20,84 @@
>  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'
>    field.
> @@ -141,6 +217,44 @@ 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.
> 
> @@ -213,13 +327,15 @@ STATIC CONST ACPI_PARSER
> CacheTypeStructureParser[] = {
>    {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"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"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	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-11-09  1:20 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-11-03 15:41 [PATCH v2 0/7] Support ACPI 6.4 PPTT changes Chris Jones
2021-11-03 15:41 ` [PATCH v2 1/7] MdePkg: Add missing Cache ID (in)valid define Chris Jones
2021-11-09  1:19   ` [edk2-devel] " Gao, Zhichao
2021-11-03 15:41 ` [PATCH v2 2/7] MdePkg: Remove PPTT ID type structure Chris Jones
2021-11-03 15:41 ` [PATCH v2 3/7] ShellPkg: Update Acpiview PPTT parser to ACPI 6.4 Chris Jones
2021-11-09  1:20   ` Gao, Zhichao
2021-11-03 15:41 ` [PATCH v2 4/7] ShellPkg: Add Cache ID to PPTT parser Chris Jones
2021-11-09  1:20   ` [edk2-devel] " Gao, Zhichao
2021-11-03 15:41 ` [PATCH v2 5/7] DynamicTablesPkg: Remove PPTT ID structure from ACPI 6.4 generator Chris Jones
2021-11-03 15:41 ` [PATCH v2 6/7] DynamicTablesPkg: Update PPTT generator to ACPI 6.4 Chris Jones
2021-11-03 15:41 ` [PATCH v2 7/7] DynamicTablesPkg: Add CacheId to PPTT generator Chris Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox