From 18f463edbaa911b6e2c32a3e783bf6c2c9997512 Mon Sep 17 00:00:00 2001 From: Pierre Gondois Date: Thu, 9 Mar 2023 16:32:49 +0100 Subject: 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 Signed-off-by: Pierre Gondois Reviewed-by: Jeff Brasen Reviewed-by: Sami Mujawar --- .../SsdtCpuTopologyGenerator.c | 131 +++++++++++++-------- .../SsdtCpuTopologyGenerator.h | 4 + 2 files changed, 84 insertions(+), 51 deletions(-) diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c index c24da8ec71..6fb131b664 100644 --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c +++ b/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) != - PPTT_CPU_PROCESSOR_MASK) - { - DEBUG (( - DEBUG_ERROR, - "ERROR: SSDT-CPU-TOPOLOGY: Invalid flags for cpu: 0x%x.\n", - Generator->ProcNodeList[Index].Flags - )); + Status = CheckProcNode ( + Generator->ProcNodeList[Index].Flags, + TRUE, + Generator->ProcNodeList[Index].Token, + NodeToken + ); + 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. - if ((Generator->ProcNodeList[Index].Flags & PPTT_PROCESSOR_MASK) != - PPTT_CLUSTER_PROCESSOR_MASK) - { - DEBUG (( - DEBUG_ERROR, - "ERROR: SSDT-CPU-TOPOLOGY: Invalid flags for cluster: 0x%x.\n", - Generator->ProcNodeList[Index].Flags - )); + Status = CheckProcNode ( + Generator->ProcNodeList[Index].Flags, + FALSE, + Generator->ProcNodeList[Index].Token, + NodeToken + ); + if (EFI_ERROR (Status)) { 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 ); diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.h b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.h index f174d9c2e2..48e4455490 100644 --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.h +++ b/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. */ -- cgit