Can we get this reviewed/merged?
> -----Original Message-----
> From: Jeshua Smith <jesh...@nvidia.com>
> Sent: Monday, February 5, 2024 12:01 PM
> To: devel@edk2.groups.io
> Cc: ardb+tianoc...@kernel.org; quic_llind...@quicinc.com;
> pierre.gond...@arm.com; sami.muja...@arm.com; Jeshua Smith
> <jesh...@nvidia.com>
> Subject: [PATCH v2] DynamicTablesPkg/SSDT: Require Package node in
> hierarchy
>
> 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 <jesh...@nvidia.com>
> Reviewed-by: Pierre Gondois <pierre.gond...@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/SsdtCpu
> TopologyGenerator.c
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpu
> TopologyGenerator.c
> index 9e3efb49e6..40ed10eae6 100644
> ---
> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpu
> TopologyGenerator.c
> +++
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCp
> +++ uTopologyGenerator.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 (#116689): https://edk2.groups.io/g/devel/message/116689
Mute This Topic: https://groups.io/mt/104183075/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
