DynamicTablesPkg/SsdtCpuTopology: Allow multi-packages topologies

The topology of a platform is represented in ACPI using the PPTT
table. It is possible to append information to CPUs/processor
containers using their associated AML nodes in a SSDT
table.
A platform might have multiple 'physical packages' (or top-level
nodes) in their PPTT topology representation. It can be assumed
from [1] that a 'physical packages' is always a 'top-level node',
and conversely.

The SSDT topology generator doesn't support having multiple top-level
nodes. The top-level node is also not generated in the SSDT topology
representation.
Add support to generate multiple top-level nodes in the SSDT topology
generator and generate an AML node for this top-level node. This will
allow to have matching PPTT and SSDT topology representations. Prior
to this patch, this top-level AML node was not generated.

Also factorize the flag checking in CheckProcNode() and add more
checks.

This patch takes inspiration from the discussion at:
- v1: https://edk2.groups.io/g/devel/message/99410
- v2: https://edk2.groups.io/g/devel/message/99615

[1]
ACPI 6.5, 5.2.30.1 Processor hierarchy node structure (Type 0):
- "Multiple trees may be described, covering for example multiple
  packages. For the root of a tree, the parent pointer should be 0.""
- "Each valid processor must belong to exactly one package. That is,
  the leaf must itself be a physical package or have an ancestor
  marked as a physical package."

Suggested-by: Jeff Brasen <jbrasen@nvidia.com>
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
Reviewed-by: Jeff Brasen <jbrasen@nvidia.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c

@@ -805,6 +805,57 @@ CreateAmlProcessorContainer (
   return Status;
 }
 
+/** Check flags and topology of a ProcNode.
+
+  @param [in]  NodeFlags        Flags of the ProcNode to check.
+  @param [in]  IsLeaf           The ProcNode is a leaf.
+  @param [in]  NodeToken        NodeToken of the ProcNode.
+  @param [in]  ParentNodeToken  Parent NodeToken of the ProcNode.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+CheckProcNode (
+  UINT32           NodeFlags,
+  BOOLEAN          IsLeaf,
+  CM_OBJECT_TOKEN  NodeToken,
+  CM_OBJECT_TOKEN  ParentNodeToken
+  )
+{
+  BOOLEAN  InvalidFlags;
+  BOOLEAN  HasPhysicalPackageBit;
+  BOOLEAN  IsTopLevelNode;
+
+  HasPhysicalPackageBit = (NodeFlags & EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL) ==
+                          EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL;
+  IsTopLevelNode = (ParentNodeToken == CM_NULL_TOKEN);
+
+  // A top-level node is a Physical Package and conversely.
+  InvalidFlags = HasPhysicalPackageBit ^ IsTopLevelNode;
+
+  // Check Leaf specific flags.
+  if (IsLeaf) {
+    InvalidFlags |= ((NodeFlags & PPTT_LEAF_MASK) != PPTT_LEAF_MASK);
+  } else {
+    InvalidFlags |= ((NodeFlags & PPTT_LEAF_MASK) != 0);
+  }
+
+  if (InvalidFlags) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "ERROR: SSDT-CPU-TOPOLOGY: Invalid flags for ProcNode: 0x%p.\n",
+      (VOID *)NodeToken
+      ));
+    ASSERT (0);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  return EFI_SUCCESS;
+}
+
 /** Create an AML representation of the Cpu topology.
 
   A processor container is by extension any non-leave device in the cpu topology.
@@ -814,7 +865,6 @@ CreateAmlProcessorContainer (
                                       Protocol Interface.
   @param [in] NodeToken               Token of the CM_ARM_PROC_HIERARCHY_INFO
                                       currently handled.
-                                      Cannot be CM_NULL_TOKEN.
   @param [in] ParentNode              Parent node to attach the created
                                       node to.
   @param [in,out] ProcContainerIndex  Pointer to the current processor container
@@ -838,6 +888,7 @@ CreateAmlCpuTopologyTree (
   EFI_STATUS              Status;
   UINT32                  Index;
   UINT32                  CpuIndex;
+  UINT32                  ProcContainerName;
   AML_OBJECT_NODE_HANDLE  ProcContainerNode;
   UINT32                  Uid;
   UINT16                  Name;
@@ -846,11 +897,11 @@ CreateAmlCpuTopologyTree (
   ASSERT (Generator->ProcNodeList != NULL);
   ASSERT (Generator->ProcNodeCount != 0);
   ASSERT (CfgMgrProtocol != NULL);
-  ASSERT (NodeToken != CM_NULL_TOKEN);
   ASSERT (ParentNode != NULL);
   ASSERT (ProcContainerIndex != NULL);
 
-  CpuIndex = 0;
+  CpuIndex          = 0;
+  ProcContainerName = 0;
 
   for (Index = 0; Index < Generator->ProcNodeCount; Index++) {
     // Find the children of the CM_ARM_PROC_HIERARCHY_INFO
@@ -859,16 +910,15 @@ CreateAmlCpuTopologyTree (
       // Only Cpus (leaf nodes in this tree) have a GicCToken.
       // Create a Cpu node.
       if (Generator->ProcNodeList[Index].GicCToken != CM_NULL_TOKEN) {
-        if ((Generator->ProcNodeList[Index].Flags & PPTT_PROCESSOR_MASK) !=
+        Status = CheckProcNode (
-            PPTT_CPU_PROCESSOR_MASK)
+                   Generator->ProcNodeList[Index].Flags,
-        {
+                   TRUE,
-          DEBUG ((
+                   Generator->ProcNodeList[Index].Token,
-            DEBUG_ERROR,
+                   NodeToken
-            "ERROR: SSDT-CPU-TOPOLOGY: Invalid flags for cpu: 0x%x.\n",
+                   );
-            Generator->ProcNodeList[Index].Flags
+        if (EFI_ERROR (Status)) {
-            ));
           ASSERT (0);
-          return EFI_INVALID_PARAMETER;
+          return Status;
         }
 
         if (Generator->ProcNodeList[Index].OverrideNameUidEnabled) {
@@ -893,24 +943,22 @@ CreateAmlCpuTopologyTree (
       } else {
         // If this is not a Cpu, then this is a processor container.
 
-        // Acpi processor Id for clusters is not handled.
+        Status = CheckProcNode (
-        if ((Generator->ProcNodeList[Index].Flags & PPTT_PROCESSOR_MASK) !=
+                   Generator->ProcNodeList[Index].Flags,
-            PPTT_CLUSTER_PROCESSOR_MASK)
+                   FALSE,
-        {
+                   Generator->ProcNodeList[Index].Token,
-          DEBUG ((
+                   NodeToken
-            DEBUG_ERROR,
+                   );
-            "ERROR: SSDT-CPU-TOPOLOGY: Invalid flags for cluster: 0x%x.\n",
+        if (EFI_ERROR (Status)) {
-            Generator->ProcNodeList[Index].Flags
-            ));
           ASSERT (0);
-          return EFI_INVALID_PARAMETER;
+          return Status;
         }
 
         if (Generator->ProcNodeList[Index].OverrideNameUidEnabled) {
           Name = Generator->ProcNodeList[Index].OverrideName;
           Uid  = Generator->ProcNodeList[Index].OverrideUid;
         } else {
-          Name = *ProcContainerIndex;
+          Name = ProcContainerName;
           Uid  = *ProcContainerIndex;
         }
 
@@ -933,6 +981,13 @@ CreateAmlCpuTopologyTree (
         (*ProcContainerIndex)++;
         CpuIndex = 0;
 
+        // And reset the cluster name whenever there is a package.
+        if (NodeToken == CM_NULL_TOKEN) {
+          ProcContainerName = 0;
+        } else {
+          ProcContainerName++;
+        }
+
         // Recursively continue creating an AML tree.
         Status = CreateAmlCpuTopologyTree (
                    Generator,
@@ -974,8 +1029,6 @@ CreateTopologyFromProcHierarchy (
   )
 {
   EFI_STATUS  Status;
-  UINT32      Index;
-  UINT32      TopLevelProcNodeIndex;
   UINT32      ProcContainerIndex;
 
   ASSERT (Generator != NULL);
@@ -984,8 +1037,7 @@ CreateTopologyFromProcHierarchy (
   ASSERT (CfgMgrProtocol != NULL);
   ASSERT (ScopeNode != NULL);
 
-  TopLevelProcNodeIndex = MAX_UINT32;
+  ProcContainerIndex = 0;
-  ProcContainerIndex    = 0;
 
   Status = TokenTableInitialize (Generator, Generator->ProcNodeCount);
   if (EFI_ERROR (Status)) {
@@ -993,33 +1045,10 @@ CreateTopologyFromProcHierarchy (
     return Status;
   }
 
-  // It is assumed that there is one unique CM_ARM_PROC_HIERARCHY_INFO
-  // structure with no ParentToken and the EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL
-  // flag set. All other CM_ARM_PROC_HIERARCHY_INFO are non-physical and
-  // have a ParentToken.
-  for (Index = 0; Index < Generator->ProcNodeCount; Index++) {
-    if ((Generator->ProcNodeList[Index].ParentToken == CM_NULL_TOKEN) &&
-        (Generator->ProcNodeList[Index].Flags &
-         EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL))
-    {
-      if (TopLevelProcNodeIndex != MAX_UINT32) {
-        DEBUG ((
-          DEBUG_ERROR,
-          "ERROR: SSDT-CPU-TOPOLOGY: Top level CM_ARM_PROC_HIERARCHY_INFO "
-          "must be unique\n"
-          ));
-        ASSERT (0);
-        goto exit_handler;
-      }
-
-      TopLevelProcNodeIndex = Index;
-    }
-  } // for
-
   Status = CreateAmlCpuTopologyTree (
              Generator,
              CfgMgrProtocol,
-             Generator->ProcNodeList[TopLevelProcNodeIndex].Token,
+             CM_NULL_TOKEN,
              ScopeNode,
              &ProcContainerIndex
              );

DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.h

@@ -34,6 +34,10 @@
           (EFI_ACPI_6_3_PPTT_PROCESSOR_ID_INVALID << 1) |                     \
           (EFI_ACPI_6_3_PPTT_NODE_IS_NOT_LEAF << 3))
 
+// Leaf nodes specific mask.
+#define PPTT_LEAF_MASK  ((EFI_ACPI_6_3_PPTT_PROCESSOR_ID_VALID << 1) |        \
+                         (EFI_ACPI_6_3_PPTT_NODE_IS_LEAF << 3))
+
 /** LPI states are stored in the ASL namespace at '\_SB_.Lxxx',
     with xxx being the node index of the LPI state.
 */