public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Jeshua Smith via groups.io" <jeshuas=nvidia.com@groups.io>
To: <devel@edk2.groups.io>
Cc: <ardb+tianocore@kernel.org>, <quic_llindhol@quicinc.com>,
	<pierre.gondois@arm.com>, <Sami.Mujawar@arm.com>,
	Jeshua Smith <jeshuas@nvidia.com>
Subject: [edk2-devel] [PATCH v2] DynamicTablesPkg/SSDT: Require Package node in hierarchy
Date: Mon, 5 Feb 2024 11:00:43 -0800	[thread overview]
Message-ID: <6666e0a5b84cef6d13a6c082e9fdf052c8618a62.1707152171.git.jeshuas@nvidia.com> (raw)

The code was incorrectly assuming that root nodes had to be physical
package nodes and vice versa. This is not always true, so update the
check to simply require exactly one package node somewhere in the
hierarchy.

Signed-off-by: Jeshua Smith <jeshuas@nvidia.com>
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
---
Note: This is a complete replacement for [PATCH] DynamicTablesPkg/SSDT: Remove incorrect root node check

Version 2: added documentation for the PackageNodeSeen parameter

 .../SsdtCpuTopologyGenerator.c                | 32 +++++++++++++------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
index 9e3efb49e6..40ed10eae6 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
@@ -1072,6 +1072,7 @@ CreateAmlProcessorContainer (
   @param [in]  IsLeaf           The ProcNode is a leaf.
   @param [in]  NodeToken        NodeToken of the ProcNode.
   @param [in]  ParentNodeToken  Parent NodeToken of the ProcNode.
+  @param [in]  PackageNodeSeen  A parent of the ProcNode has the physical package flag set.
 
   @retval EFI_SUCCESS             Success.
   @retval EFI_INVALID_PARAMETER   Invalid parameter.
@@ -1083,23 +1084,24 @@ CheckProcNode (
   UINT32           NodeFlags,
   BOOLEAN          IsLeaf,
   CM_OBJECT_TOKEN  NodeToken,
-  CM_OBJECT_TOKEN  ParentNodeToken
+  CM_OBJECT_TOKEN  ParentNodeToken,
+  BOOLEAN          PackageNodeSeen
   )
 {
   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;
+  // Only one Physical Package flag is allowed in the hierarchy
+  InvalidFlags = HasPhysicalPackageBit && PackageNodeSeen;
 
   // Check Leaf specific flags.
   if (IsLeaf) {
     InvalidFlags |= ((NodeFlags & PPTT_LEAF_MASK) != PPTT_LEAF_MASK);
+    // Must have Physical Package flag somewhere in the hierarchy
+    InvalidFlags |= !(HasPhysicalPackageBit || PackageNodeSeen);
   } else {
     InvalidFlags |= ((NodeFlags & PPTT_LEAF_MASK) != 0);
   }
@@ -1130,6 +1132,7 @@ CheckProcNode (
                                       node to.
   @param [in,out] ProcContainerIndex  Pointer to the current processor container
                                       index to be used as UID.
+  @param [in]  PackageNodeSeen        A parent of the ProcNode has the physical package flag set.
 
   @retval EFI_SUCCESS             Success.
   @retval EFI_INVALID_PARAMETER   Invalid parameter.
@@ -1143,7 +1146,8 @@ CreateAmlCpuTopologyTree (
   IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  CfgMgrProtocol,
   IN        CM_OBJECT_TOKEN                               NodeToken,
   IN        AML_NODE_HANDLE                               ParentNode,
-  IN OUT    UINT32                                        *ProcContainerIndex
+  IN OUT    UINT32                                        *ProcContainerIndex,
+  IN        BOOLEAN                                       PackageNodeSeen
   )
 {
   EFI_STATUS              Status;
@@ -1153,6 +1157,7 @@ CreateAmlCpuTopologyTree (
   AML_OBJECT_NODE_HANDLE  ProcContainerNode;
   UINT32                  Uid;
   UINT16                  Name;
+  BOOLEAN                 HasPhysicalPackageBit;
 
   ASSERT (Generator != NULL);
   ASSERT (Generator->ProcNodeList != NULL);
@@ -1175,7 +1180,8 @@ CreateAmlCpuTopologyTree (
                    Generator->ProcNodeList[Index].Flags,
                    TRUE,
                    Generator->ProcNodeList[Index].Token,
-                   NodeToken
+                   NodeToken,
+                   PackageNodeSeen
                    );
         if (EFI_ERROR (Status)) {
           ASSERT (0);
@@ -1208,7 +1214,8 @@ CreateAmlCpuTopologyTree (
                    Generator->ProcNodeList[Index].Flags,
                    FALSE,
                    Generator->ProcNodeList[Index].Token,
-                   NodeToken
+                   NodeToken,
+                   PackageNodeSeen
                    );
         if (EFI_ERROR (Status)) {
           ASSERT (0);
@@ -1249,13 +1256,17 @@ CreateAmlCpuTopologyTree (
           ProcContainerName++;
         }
 
+        HasPhysicalPackageBit = (Generator->ProcNodeList[Index].Flags & EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL) ==
+                                EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL;
+
         // Recursively continue creating an AML tree.
         Status = CreateAmlCpuTopologyTree (
                    Generator,
                    CfgMgrProtocol,
                    Generator->ProcNodeList[Index].Token,
                    ProcContainerNode,
-                   ProcContainerIndex
+                   ProcContainerIndex,
+                   (PackageNodeSeen || HasPhysicalPackageBit)
                    );
         if (EFI_ERROR (Status)) {
           ASSERT (0);
@@ -1311,7 +1322,8 @@ CreateTopologyFromProcHierarchy (
              CfgMgrProtocol,
              CM_NULL_TOKEN,
              ScopeNode,
-             &ProcContainerIndex
+             &ProcContainerIndex,
+             FALSE
              );
   if (EFI_ERROR (Status)) {
     ASSERT (0);
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115126): https://edk2.groups.io/g/devel/message/115126
Mute This Topic: https://groups.io/mt/104183075/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



             reply	other threads:[~2024-02-05 19:01 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-05 19:00 Jeshua Smith via groups.io [this message]
2024-03-12 16:11 ` [edk2-devel] [PATCH v2] DynamicTablesPkg/SSDT: Require Package node in hierarchy Jeshua Smith via groups.io
2024-03-12 16:14   ` Sami Mujawar
2024-03-13 14:23     ` 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=6666e0a5b84cef6d13a6c082e9fdf052c8618a62.1707152171.git.jeshuas@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