[PATCH v1 0/2] Support ACPI 6.4 in Acpiview PCCT parser

[PATCH v1 0/2] Support ACPI 6.4 in Acpiview PCCT parser

[Chris Jones]

Bugzilla: 3563 (https://bugzilla.tianocore.org/show_bug.cgi?id=3563)

This patch series adds support for ACPI 6.4 to the Acpiview PCCT parser
by using Acpi64.h and adding parsing, validation and printing support
for the new 'Type 5' subspace structure.

The changes can be seen at: https://github.com/chris-jones-arm/edk2/tree/1825_64_acpi_pcct_update_v1

Chris Jones (2):
  ShellPkg: Update Acpiview PCCT parser to ACPI 6.4
  ShellPkg: Add Type 5 PCC Subspace Structure parser

 .../Parsers/Pcct/PcctParser.c                 | 104 +++++++++++++-----
 .../UefiShellAcpiViewCommandLib.c             |   4 +-
 2 files changed, 80 insertions(+), 28 deletions(-)

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

[PATCH v1 1/2] ShellPkg: Update Acpiview PCCT parser to ACPI 6.4

[Chris Jones]

Bugzilla: 3563 (https://bugzilla.tianocore.org/show_bug.cgi?id=3563)

Update the Acpiview PCCT parser to use Acpi64.h.

Signed-off-by: Chris Jones <christopher.jones@arm.com>
---
 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pcct/PcctParser.c     | 46 ++++++++++----------
 ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c |  4 +-
 2 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pcct/PcctParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pcct/PcctParser.c
index ca553d5953f05651d8df4c95d1e39a73eded5129..7d3a2dd10dc928265bad3711e929d96c1ed40b1d 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pcct/PcctParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pcct/PcctParser.c
@@ -1,11 +1,11 @@
 /** @file
   PCCT table parser
 
-  Copyright (c) 2020, Arm Limited.
+  Copyright (c) 2021, Arm Limited.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @par Reference(s):
-    - ACPI 6.3 Specification - January 2019
+    - ACPI 6.4 Specification - January 2021
 **/
 
 #include <Library/BaseMemoryLib.h>
@@ -93,9 +93,9 @@ ValidatePccType0Gas (
 {
   switch (*(UINT8*)Ptr) {
 #if !(defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64))
-    case EFI_ACPI_6_3_SYSTEM_IO:
+    case EFI_ACPI_6_4_SYSTEM_IO:
 #endif //if not (defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64))
-    case EFI_ACPI_6_3_SYSTEM_MEMORY:
+    case EFI_ACPI_6_4_SYSTEM_MEMORY:
       return;
     default:
       IncrementErrorCount ();
@@ -120,10 +120,10 @@ ValidatePccGas (
 {
   switch (*(UINT8*)Ptr) {
 #if !(defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64))
-    case EFI_ACPI_6_3_SYSTEM_IO:
+    case EFI_ACPI_6_4_SYSTEM_IO:
 #endif //if not (defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64))
-    case EFI_ACPI_6_3_FUNCTIONAL_FIXED_HARDWARE:
-    case EFI_ACPI_6_3_SYSTEM_MEMORY:
+    case EFI_ACPI_6_4_FUNCTIONAL_FIXED_HARDWARE:
+    case EFI_ACPI_6_4_SYSTEM_MEMORY:
       return;
     default:
       IncrementErrorCount ();
@@ -148,10 +148,10 @@ ValidatePccDoorbellGas (
 {
   // For slave subspaces this field is optional, if not present the field
   // should just contain zeros.
-  if (*PccSubspaceType == EFI_ACPI_6_3_PCCT_SUBSPACE_TYPE_4_EXTENDED_PCC) {
+  if (*PccSubspaceType == EFI_ACPI_6_4_PCCT_SUBSPACE_TYPE_4_EXTENDED_PCC) {
     if (IsZeroBuffer (
           Ptr,
-          sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE)
+          sizeof (EFI_ACPI_6_4_GENERIC_ADDRESS_STRUCTURE)
           )) {
       return;
     }
@@ -180,14 +180,14 @@ ValidatePccIntAckGas (
   // edge driven the register may be omitted. A value of 0x0 on all
   // 12 bytes of the GAS structure indicates the register is not
   // present.
-  if (((*PccGlobalFlags & EFI_ACPI_6_3_PCCT_FLAGS_PLATFORM_INTERRUPT) !=
-       EFI_ACPI_6_3_PCCT_FLAGS_PLATFORM_INTERRUPT) ||
+  if (((*PccGlobalFlags & EFI_ACPI_6_4_PCCT_FLAGS_PLATFORM_INTERRUPT) !=
+       EFI_ACPI_6_4_PCCT_FLAGS_PLATFORM_INTERRUPT) ||
       ((*ExtendedPccSubspaceInterruptFlags &
-       EFI_ACPI_6_3_PCCT_SUBSPACE_PLATFORM_INTERRUPT_FLAGS_MODE) ==
-       EFI_ACPI_6_3_PCCT_SUBSPACE_PLATFORM_INTERRUPT_FLAGS_MODE)) {
+       EFI_ACPI_6_4_PCCT_SUBSPACE_PLATFORM_INTERRUPT_FLAGS_MODE) ==
+       EFI_ACPI_6_4_PCCT_SUBSPACE_PLATFORM_INTERRUPT_FLAGS_MODE)) {
     if (IsZeroBuffer (
           Ptr,
-          sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE)
+          sizeof (EFI_ACPI_6_4_GENERIC_ADDRESS_STRUCTURE)
           )) {
       return;
     }
@@ -212,7 +212,7 @@ ValidatePccErrStatusGas (
   )
 {
   // This field is ignored by the OSPM on slave channels.
-  if (*PccSubspaceType == EFI_ACPI_6_3_PCCT_SUBSPACE_TYPE_4_EXTENDED_PCC) {
+  if (*PccSubspaceType == EFI_ACPI_6_4_PCCT_SUBSPACE_TYPE_4_EXTENDED_PCC) {
     return;
   }
 
@@ -236,9 +236,9 @@ ValidatePlatInterrupt (
 {
   // If a slave subspace is present in the PCCT, then the global Platform
   // Interrupt flag must be set to 1.
-  if ((*PccSubspaceType == EFI_ACPI_6_3_PCCT_SUBSPACE_TYPE_4_EXTENDED_PCC) &&
-      ((*PccGlobalFlags & EFI_ACPI_6_3_PCCT_FLAGS_PLATFORM_INTERRUPT) !=
-        EFI_ACPI_6_3_PCCT_FLAGS_PLATFORM_INTERRUPT)) {
+  if ((*PccSubspaceType == EFI_ACPI_6_4_PCCT_SUBSPACE_TYPE_4_EXTENDED_PCC) &&
+      ((*PccGlobalFlags & EFI_ACPI_6_4_PCCT_FLAGS_PLATFORM_INTERRUPT) !=
+        EFI_ACPI_6_4_PCCT_FLAGS_PLATFORM_INTERRUPT)) {
     IncrementErrorCount ();
     Print (
       L"\nError: Global Platform interrupt flag must be set to 1" \
@@ -563,31 +563,31 @@ ParseAcpiPcct (
     }
 
     switch (*PccSubspaceType) {
-      case EFI_ACPI_6_3_PCCT_SUBSPACE_TYPE_GENERIC:
+      case EFI_ACPI_6_4_PCCT_SUBSPACE_TYPE_GENERIC:
         DumpPccSubspaceType0 (
           PccSubspacePtr,
           *PccSubspaceLength
           );
         break;
-      case EFI_ACPI_6_3_PCCT_SUBSPACE_TYPE_1_HW_REDUCED_COMMUNICATIONS:
+      case EFI_ACPI_6_4_PCCT_SUBSPACE_TYPE_1_HW_REDUCED_COMMUNICATIONS:
         DumpPccSubspaceType1 (
           PccSubspacePtr,
           *PccSubspaceLength
           );
         break;
-      case EFI_ACPI_6_3_PCCT_SUBSPACE_TYPE_2_HW_REDUCED_COMMUNICATIONS:
+      case EFI_ACPI_6_4_PCCT_SUBSPACE_TYPE_2_HW_REDUCED_COMMUNICATIONS:
         DumpPccSubspaceType2 (
           PccSubspacePtr,
           *PccSubspaceLength
           );
         break;
-      case EFI_ACPI_6_3_PCCT_SUBSPACE_TYPE_3_EXTENDED_PCC:
+      case EFI_ACPI_6_4_PCCT_SUBSPACE_TYPE_3_EXTENDED_PCC:
         DumpPccSubspaceType3 (
           PccSubspacePtr,
           *PccSubspaceLength
           );
         break;
-      case EFI_ACPI_6_3_PCCT_SUBSPACE_TYPE_4_EXTENDED_PCC:
+      case EFI_ACPI_6_4_PCCT_SUBSPACE_TYPE_4_EXTENDED_PCC:
         DumpPccSubspaceType4 (
           PccSubspacePtr,
           *PccSubspaceLength
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
index b30ed3fc8597b229dd15b6ad4f2aab2e3d0ca583..c81e5220272ae94448203ee02ea5caacb606842c 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
@@ -1,7 +1,7 @@
 /** @file
   Main file for 'acpiview' Shell command function.
 
-  Copyright (c) 2016 - 2020, Arm Limited. All rights reserved.<BR>
+  Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 
@@ -60,7 +60,7 @@ ACPI_TABLE_PARSER ParserList[] = {
   {EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE, ParseAcpiMadt},
   {EFI_ACPI_6_2_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE,
    ParseAcpiMcfg},
-  {EFI_ACPI_6_2_PLATFORM_COMMUNICATIONS_CHANNEL_TABLE_SIGNATURE,
+  {EFI_ACPI_6_4_PLATFORM_COMMUNICATIONS_CHANNEL_TABLE_SIGNATURE,
    ParseAcpiPcct},
   {EFI_ACPI_6_2_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGNATURE,
    ParseAcpiPptt},
-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

[PATCH v1 2/2] ShellPkg: Add Type 5 PCC Subspace Structure parser

[Chris Jones]

Bugzilla: 3563 (https://bugzilla.tianocore.org/show_bug.cgi?id=3563)

Update the Acpiview PCCT parser with the HW Registers based
Communications Subspace Structure (Type 5) as defined in Section 14.1.7
of the ACPI 6.4 specification.

Signed-off-by: Chris Jones <christopher.jones@arm.com>
---
 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pcct/PcctParser.c | 58 +++++++++++++++++++-
 1 file changed, 55 insertions(+), 3 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pcct/PcctParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pcct/PcctParser.c
index 7d3a2dd10dc928265bad3711e929d96c1ed40b1d..6beee70b23a3b32992efa0edac48fe236591b9f5 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pcct/PcctParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pcct/PcctParser.c
@@ -77,7 +77,7 @@ ValidateRangeLength8 (
 }
 
 /**
-  This function validates address space for type 0 structure.
+  This function validates address space for Memory/IO GAS.
 
   @param [in] Ptr     Pointer to the start of the field data.
   @param [in] Context Pointer to context specific information e.g. this
@@ -86,7 +86,7 @@ ValidateRangeLength8 (
 STATIC
 VOID
 EFIAPI
-ValidatePccType0Gas (
+ValidatePccMemoryIoGas (
   IN UINT8* Ptr,
   IN VOID*  Context
   )
@@ -274,7 +274,7 @@ STATIC CONST ACPI_PARSER PccSubspaceType0Parser[] = {
   {L"Base Address", 8, 8, L"0x%lx", NULL, NULL, NULL, NULL},
   {L"Memory Range Length", 8, 16, L"0x%lx", NULL, NULL, ValidateRangeLength8,
     NULL},
-  {L"Doorbell Register", 12, 24, NULL, DumpGas, NULL, ValidatePccType0Gas,
+  {L"Doorbell Register", 12, 24, NULL, DumpGas, NULL, ValidatePccMemoryIoGas,
     NULL},
   {L"Doorbell Preserve", 8, 36, L"0x%lx", NULL, NULL, NULL, NULL},
   {L"Doorbell Write", 8, 44, L"0x%lx", NULL, NULL, NULL, NULL},
@@ -367,6 +367,29 @@ STATIC CONST ACPI_PARSER PccSubspaceType3Parser[] = {
   {L"Error Status Mask", 8, 156, L"0x%lx", NULL, NULL, NULL, NULL},
 };
 
+/**
+  An ACPI_PARSER array describing the HW Registers based Communications
+  Subspace Structure - Type 5
+*/
+STATIC CONST ACPI_PARSER PccSubspaceType5Parser[] = {
+  PCC_SUBSPACE_HEADER (),
+  {L"Version", 2, 2, L"0x%x", NULL, NULL, NULL, NULL},
+  {L"Base Address", 8, 4, L"0x%lx", NULL, NULL, NULL, NULL},
+  {L"Shared Memory Range Length", 8, 12, L"0x%lx", NULL, NULL, NULL, NULL},
+  {L"Doorbell Register", 12, 20, NULL, DumpGas, NULL,
+    ValidatePccMemoryIoGas, NULL},
+  {L"Doorbell Preserve", 8, 32, L"0x%lx", NULL, NULL, NULL, NULL},
+  {L"Doorbell Write", 8, 40, L"0x%lx", NULL, NULL, NULL, NULL},
+  {L"Command Complete Check Register", 12, 48, NULL, DumpGas, NULL,
+    ValidatePccMemoryIoGas, NULL},
+  {L"Command Complete Check Mask", 8, 60, L"0x%lx", NULL, NULL, NULL, NULL},
+  {L"Error Status Register", 12, 68, NULL, DumpGas, NULL,
+    ValidatePccMemoryIoGas, NULL},
+  {L"Error Status Mask", 8, 80, L"0x%lx", NULL, NULL, NULL, NULL},
+  {L"Nominal Latency", 4, 88, L"0x%x", NULL, NULL, NULL, NULL},
+  {L"Minimum Request Turnaround Time", 4, 92, L"0x%x", NULL, NULL, NULL, NULL}
+};
+
 /**
   This function parses the PCC Subspace type 0.
 
@@ -482,6 +505,29 @@ DumpPccSubspaceType4 (
     );
 }
 
+/**
+  This function parses the PCC Subspace type 5.
+
+  @param [in] Ptr     Pointer to the start of the Subspace Structure.
+  @param [in] Length  Length of the Subspace Structure.
+**/
+STATIC
+VOID
+DumpPccSubspaceType5 (
+  IN UINT8* Ptr,
+  IN UINT8  Length
+  )
+{
+  ParseAcpi (
+    TRUE,
+    2,
+    "Subspace Type 5",
+    Ptr,
+    Length,
+    PARSER_PARAMS (PccSubspaceType5Parser)
+    );
+}
+
 /**
   This function parses the ACPI PCCT table including its sub-structures
   of type 0 through 4.
@@ -593,6 +639,12 @@ ParseAcpiPcct (
           *PccSubspaceLength
           );
         break;
+      case EFI_ACPI_6_4_PCCT_SUBSPACE_TYPE_5_HW_REGISTERS_COMMUNICATIONS:
+        DumpPccSubspaceType5 (
+          PccSubspacePtr,
+          *PccSubspaceLength
+          );
+        break;
       default:
         IncrementErrorCount ();
         Print (
-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

Re: [PATCH v1 0/2] Support ACPI 6.4 in Acpiview PCCT parser

[Sami Mujawar]

For this series.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 13/08/2021, 18:02, "Chris Jones" <Christopher.Jones@arm.com> wrote:

    Bugzilla: 3563 (https://bugzilla.tianocore.org/show_bug.cgi?id=3563)

    This patch series adds support for ACPI 6.4 to the Acpiview PCCT parser
    by using Acpi64.h and adding parsing, validation and printing support
    for the new 'Type 5' subspace structure.

    The changes can be seen at: https://github.com/chris-jones-arm/edk2/tree/1825_64_acpi_pcct_update_v1

    Chris Jones (2):
      ShellPkg: Update Acpiview PCCT parser to ACPI 6.4
      ShellPkg: Add Type 5 PCC Subspace Structure parser

     .../Parsers/Pcct/PcctParser.c                 | 104 +++++++++++++-----
     .../UefiShellAcpiViewCommandLib.c             |   4 +-
     2 files changed, 80 insertions(+), 28 deletions(-)

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

Re: [PATCH v1 0/2] Support ACPI 6.4 in Acpiview PCCT parser

[Chris Jones]

[-- Attachment #1: Type: text/plain, Size: 1660 bytes --]

I believe this patch series is just waiting for approval from the ShellPkg maintainers. As this has been waiting for several weeks, please could this get reviewed soon.


Thanks,
Chris
________________________________
From: Sami Mujawar <Sami.Mujawar@arm.com>
Sent: Wednesday, August 18, 2021 11:32 AM
To: Christopher Jones <Christopher.Jones@arm.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: ray.ni@intel.com <ray.ni@intel.com>; zhichao.gao@intel.com <zhichao.gao@intel.com>; Ben Adderson <Ben.Adderson@arm.com>; Akanksha Jain <Akanksha.Jain2@arm.com>; Matteo Carlini <Matteo.Carlini@arm.com>; nd <nd@arm.com>
Subject: Re: [PATCH v1 0/2] Support ACPI 6.4 in Acpiview PCCT parser

For this series.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 13/08/2021, 18:02, "Chris Jones" <Christopher.Jones@arm.com> wrote:

    Bugzilla: 3563 (https://bugzilla.tianocore.org/show_bug.cgi?id=3563)

    This patch series adds support for ACPI 6.4 to the Acpiview PCCT parser
    by using Acpi64.h and adding parsing, validation and printing support
    for the new 'Type 5' subspace structure.

    The changes can be seen at: https://github.com/chris-jones-arm/edk2/tree/1825_64_acpi_pcct_update_v1

    Chris Jones (2):
      ShellPkg: Update Acpiview PCCT parser to ACPI 6.4
      ShellPkg: Add Type 5 PCC Subspace Structure parser

     .../Parsers/Pcct/PcctParser.c                 | 104 +++++++++++++-----
     .../UefiShellAcpiViewCommandLib.c             |   4 +-
     2 files changed, 80 insertions(+), 28 deletions(-)

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



[-- Attachment #2: Type: text/html, Size: 3626 bytes --]

Re: [PATCH v1 0/2] Support ACPI 6.4 in Acpiview PCCT parser

[Gao, Zhichao]

For this patch set, Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

Thanks,
Zhichao

> -----Original Message-----
> From: Chris Jones <christopher.jones@arm.com>
> Sent: Saturday, August 14, 2021 1:03 AM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>;
> Sami.Mujawar@arm.com; Ben.Adderson@arm.com;
> Akanksha.Jain2@arm.com; Matteo.Carlini@arm.com; nd@arm.com
> Subject: [PATCH v1 0/2] Support ACPI 6.4 in Acpiview PCCT parser
> 
> Bugzilla: 3563 (https://bugzilla.tianocore.org/show_bug.cgi?id=3563)
> 
> This patch series adds support for ACPI 6.4 to the Acpiview PCCT parser by
> using Acpi64.h and adding parsing, validation and printing support for the
> new 'Type 5' subspace structure.
> 
> The changes can be seen at: https://github.com/chris-jones-
> arm/edk2/tree/1825_64_acpi_pcct_update_v1
> 
> Chris Jones (2):
>   ShellPkg: Update Acpiview PCCT parser to ACPI 6.4
>   ShellPkg: Add Type 5 PCC Subspace Structure parser
> 
>  .../Parsers/Pcct/PcctParser.c                 | 104 +++++++++++++-----
>  .../UefiShellAcpiViewCommandLib.c             |   4 +-
>  2 files changed, 80 insertions(+), 28 deletions(-)
> 
> --
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")