public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Jeff Brasen" <jbrasen@nvidia.com>
To: <devel@edk2.groups.io>
Cc: <ardb+tianocore@kernel.org>, <Sami.Mujawar@arm.com>,
	<Alexei.Fedorov@arm.com>, Jeff Brasen <jbrasen@nvidia.com>
Subject: [PATCH v2] DynamicTablesPkg: Fix nested processor containers
Date: Thu, 18 Aug 2022 10:47:55 -0600	[thread overview]
Message-ID: <b585176a4bdf11ad28fc8fc68f3ca6d0e515b6d0.1660841195.git.jbrasen@nvidia.com> (raw)

Current code will generate duplicate UID if there are nested processor
containers in the topology. For example if there is a
socket/cluster/core layout.

Change references to processor container from cluster to be more
accurate on what is being created.

Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
---
 .../SsdtCpuTopologyGenerator.c                | 94 ++++++++++---------
 1 file changed, 50 insertions(+), 44 deletions(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
index 3266d8dd98..8561f48e1f 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
@@ -587,10 +587,10 @@ CreateAmlCpuFromProcHierarchy (
   return Status;
 }
 
-/** Create a Cluster in the AML namespace.
+/** Create a Processor Container in the AML namespace.
 
   Any CM_ARM_PROC_HIERARCHY_INFO object with the following flags is
-  assumed to be a cluster:
+  assumed to be a processor container:
    - EFI_ACPI_6_3_PPTT_PACKAGE_NOT_PHYSICAL
    - EFI_ACPI_6_3_PPTT_PROCESSOR_ID_INVALID
    - EFI_ACPI_6_3_PPTT_NODE_IS_NOT_LEAF
@@ -605,13 +605,13 @@ CreateAmlCpuFromProcHierarchy (
   @param [in]  Generator              The SSDT Cpu Topology generator.
   @param [in]  CfgMgrProtocol         Pointer to the Configuration Manager
                                       Protocol Interface.
-  @param [in]  ParentNode             Parent node to attach the Cluster
-                                      node to.
+  @param [in]  ParentNode             Parent node to attach the processor
+                                      container node to.
   @param [in]  ProcHierarchyNodeInfo  CM_ARM_PROC_HIERARCHY_INFO object used
                                       to create the node.
-  @param [in]  ClusterIndex           Index used to generate the node name.
-  @param [out] ClusterNodePtr         If success, contains the created Cluster
-                                      node.
+  @param [in]  ProcContainerIndex     Index used to generate the node name.
+  @param [out] ProcContainerNodePtr   If success, contains the created processor
+                                      container node.
 
   @retval EFI_SUCCESS             Success.
   @retval EFI_INVALID_PARAMETER   Invalid parameter.
@@ -620,43 +620,43 @@ CreateAmlCpuFromProcHierarchy (
 STATIC
 EFI_STATUS
 EFIAPI
-CreateAmlCluster (
+CreateAmlProcessorContainer (
   IN        ACPI_CPU_TOPOLOGY_GENERATOR                   *Generator,
   IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  CfgMgrProtocol,
   IN        AML_NODE_HANDLE                               ParentNode,
   IN        CM_ARM_PROC_HIERARCHY_INFO                    *ProcHierarchyNodeInfo,
-  IN        UINT32                                        ClusterIndex,
-  OUT       AML_OBJECT_NODE_HANDLE                        *ClusterNodePtr
+  IN        UINT32                                        ProcContainerIndex,
+  OUT       AML_OBJECT_NODE_HANDLE                        *ProcContainerNodePtr
   )
 {
   EFI_STATUS              Status;
-  AML_OBJECT_NODE_HANDLE  ClusterNode;
-  CHAR8                   AslNameCluster[AML_NAME_SEG_SIZE + 1];
+  AML_OBJECT_NODE_HANDLE  ProcContainerNode;
+  CHAR8                   AslNameProcContainer[AML_NAME_SEG_SIZE + 1];
 
   ASSERT (Generator != NULL);
   ASSERT (CfgMgrProtocol != NULL);
   ASSERT (ParentNode != NULL);
   ASSERT (ProcHierarchyNodeInfo != NULL);
-  ASSERT (ClusterNodePtr != NULL);
+  ASSERT (ProcContainerNodePtr != NULL);
 
-  Status = WriteAslName ('C', ClusterIndex, AslNameCluster);
+  Status = WriteAslName ('C', ProcContainerIndex, AslNameProcContainer);
   if (EFI_ERROR (Status)) {
     ASSERT (0);
     return Status;
   }
 
-  Status = AmlCodeGenDevice (AslNameCluster, ParentNode, &ClusterNode);
+  Status = AmlCodeGenDevice (AslNameProcContainer, ParentNode, &ProcContainerNode);
   if (EFI_ERROR (Status)) {
     ASSERT (0);
     return Status;
   }
 
-  // Use the ClusterIndex for the _UID value as there is no AcpiProcessorUid
+  // Use the ProcContainerIndex for the _UID value as there is no AcpiProcessorUid
   // and EFI_ACPI_6_3_PPTT_PROCESSOR_ID_INVALID is set for non-Cpus.
   Status = AmlCodeGenNameInteger (
              "_UID",
-             ClusterIndex,
-             ClusterNode,
+             ProcContainerIndex,
+             ProcContainerNode,
              NULL
              );
   if (EFI_ERROR (Status)) {
@@ -667,7 +667,7 @@ CreateAmlCluster (
   Status = AmlCodeGenNameString (
              "_HID",
              ACPI_HID_PROCESSOR_CONTAINER_DEVICE,
-             ClusterNode,
+             ProcContainerNode,
              NULL
              );
   if (EFI_ERROR (Status)) {
@@ -681,7 +681,7 @@ CreateAmlCluster (
     Status = CreateAmlLpiMethod (
                Generator,
                ProcHierarchyNodeInfo,
-               ClusterNode
+               ProcContainerNode
                );
     if (EFI_ERROR (Status)) {
       ASSERT (0);
@@ -689,23 +689,25 @@ CreateAmlCluster (
     }
   }
 
-  *ClusterNodePtr = ClusterNode;
+  *ProcContainerNodePtr = ProcContainerNode;
 
   return Status;
 }
 
 /** Create an AML representation of the Cpu topology.
 
-  A cluster is by extension any non-leave device in the cpu topology.
+  A processor container is by extension any non-leave device in the cpu topology.
 
-  @param [in] Generator          The SSDT Cpu Topology generator.
-  @param [in] CfgMgrProtocol     Pointer to the Configuration Manager
-                                 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] Generator               The SSDT Cpu Topology generator.
+  @param [in] CfgMgrProtocol          Pointer to the Configuration Manager
+                                      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
+                                      index to be used as UID.
 
   @retval EFI_SUCCESS             Success.
   @retval EFI_INVALID_PARAMETER   Invalid parameter.
@@ -718,14 +720,14 @@ CreateAmlCpuTopologyTree (
   IN        ACPI_CPU_TOPOLOGY_GENERATOR                   *Generator,
   IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  CfgMgrProtocol,
   IN        CM_OBJECT_TOKEN                               NodeToken,
-  IN        AML_NODE_HANDLE                               ParentNode
+  IN        AML_NODE_HANDLE                               ParentNode,
+  IN OUT    UINT32                                        *ProcContainerIndex
   )
 {
   EFI_STATUS              Status;
   UINT32                  Index;
   UINT32                  CpuIndex;
-  UINT32                  ClusterIndex;
-  AML_OBJECT_NODE_HANDLE  ClusterNode;
+  AML_OBJECT_NODE_HANDLE  ProcContainerNode;
 
   ASSERT (Generator != NULL);
   ASSERT (Generator->ProcNodeList != NULL);
@@ -733,9 +735,9 @@ CreateAmlCpuTopologyTree (
   ASSERT (CfgMgrProtocol != NULL);
   ASSERT (NodeToken != CM_NULL_TOKEN);
   ASSERT (ParentNode != NULL);
+  ASSERT (ProcContainerIndex != NULL);
 
-  CpuIndex     = 0;
-  ClusterIndex = 0;
+  CpuIndex = 0;
 
   for (Index = 0; Index < Generator->ProcNodeCount; Index++) {
     // Find the children of the CM_ARM_PROC_HIERARCHY_INFO
@@ -770,7 +772,7 @@ CreateAmlCpuTopologyTree (
 
         CpuIndex++;
       } else {
-        // If this is not a Cpu, then this is a cluster.
+        // If this is not a Cpu, then this is a processor container.
 
         // Acpi processor Id for clusters is not handled.
         if ((Generator->ProcNodeList[Index].Flags & PPTT_PROCESSOR_MASK) !=
@@ -785,13 +787,13 @@ CreateAmlCpuTopologyTree (
           return EFI_INVALID_PARAMETER;
         }
 
-        Status = CreateAmlCluster (
+        Status = CreateAmlProcessorContainer (
                    Generator,
                    CfgMgrProtocol,
                    ParentNode,
                    &Generator->ProcNodeList[Index],
-                   ClusterIndex,
-                   &ClusterNode
+                   *ProcContainerIndex,
+                   &ProcContainerNode
                    );
         if (EFI_ERROR (Status)) {
           ASSERT (0);
@@ -799,8 +801,8 @@ CreateAmlCpuTopologyTree (
         }
 
         // Nodes must have a unique name in the ASL namespace.
-        // Reset the Cpu index whenever we create a new Cluster.
-        ClusterIndex++;
+        // Reset the Cpu index whenever we create a new processor container.
+        (*ProcContainerIndex)++;
         CpuIndex = 0;
 
         // Recursively continue creating an AML tree.
@@ -808,7 +810,8 @@ CreateAmlCpuTopologyTree (
                    Generator,
                    CfgMgrProtocol,
                    Generator->ProcNodeList[Index].Token,
-                   ClusterNode
+                   ProcContainerNode,
+                   ProcContainerIndex
                    );
         if (EFI_ERROR (Status)) {
           ASSERT (0);
@@ -845,6 +848,7 @@ CreateTopologyFromProcHierarchy (
   EFI_STATUS  Status;
   UINT32      Index;
   UINT32      TopLevelProcNodeIndex;
+  UINT32      ProcContainerIndex;
 
   ASSERT (Generator != NULL);
   ASSERT (Generator->ProcNodeCount != 0);
@@ -853,6 +857,7 @@ CreateTopologyFromProcHierarchy (
   ASSERT (ScopeNode != NULL);
 
   TopLevelProcNodeIndex = MAX_UINT32;
+  ProcContainerIndex    = 0;
 
   Status = TokenTableInitialize (Generator, Generator->ProcNodeCount);
   if (EFI_ERROR (Status)) {
@@ -887,7 +892,8 @@ CreateTopologyFromProcHierarchy (
              Generator,
              CfgMgrProtocol,
              Generator->ProcNodeList[TopLevelProcNodeIndex].Token,
-             ScopeNode
+             ScopeNode,
+             &ProcContainerIndex
              );
   if (EFI_ERROR (Status)) {
     ASSERT (0);
@@ -908,7 +914,7 @@ exit_handler:
 /** Create the processor hierarchy AML tree from CM_ARM_GICC_INFO
     CM objects.
 
-  A cluster is by extension any non-leave device in the cpu topology.
+  A processor container is by extension any non-leave device in the cpu topology.
 
   @param [in] Generator        The SSDT Cpu Topology generator.
   @param [in] CfgMgrProtocol   Pointer to the Configuration Manager
-- 
2.25.1


             reply	other threads:[~2022-08-18 16:48 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-18 16:47 Jeff Brasen [this message]
2022-08-18 17:17 ` [PATCH v2] DynamicTablesPkg: Fix nested processor containers Sami Mujawar
2022-09-01 14:22   ` [edk2-devel] " Sami Mujawar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b585176a4bdf11ad28fc8fc68f3ca6d0e515b6d0.1660841195.git.jbrasen@nvidia.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox