Merged at https://github.com/tianocore/edk2/commit/308e6e0936c43063551babb4a71c46775b1dc01c
Thanks. Regards, Sami Mujawar On 12/03/2024, 16:14, "Sami Mujawar" <sami.muja...@arm.com <mailto:sami.muja...@arm.com>> wrote: Hi Jeshua, Apologies, I somehow missed getting this merged. I will get this in before end of this week. Regards, Sami Mujawar On 12/03/2024, 16:12, "Jeshua Smith" <jesh...@nvidia.com <mailto:jesh...@nvidia.com> <mailto:jesh...@nvidia.com <mailto:jesh...@nvidia.com>>> wrote: Can we get this reviewed/merged? > -----Original Message----- > From: Jeshua Smith <jesh...@nvidia.com <mailto:jesh...@nvidia.com> > <mailto:jesh...@nvidia.com <mailto:jesh...@nvidia.com>>> > Sent: Monday, February 5, 2024 12:01 PM > To: devel@edk2.groups.io <mailto:devel@edk2.groups.io> > <mailto:devel@edk2.groups.io <mailto:devel@edk2.groups.io>> > Cc: ardb+tianoc...@kernel.org <mailto:ardb+tianoc...@kernel.org> > <mailto:ardb+tianoc...@kernel.org <mailto:ardb+tianoc...@kernel.org>>; > quic_llind...@quicinc.com <mailto:quic_llind...@quicinc.com> > <mailto:quic_llind...@quicinc.com <mailto:quic_llind...@quicinc.com>>; > pierre.gond...@arm.com <mailto:pierre.gond...@arm.com> > <mailto:pierre.gond...@arm.com <mailto:pierre.gond...@arm.com>>; > sami.muja...@arm.com <mailto:sami.muja...@arm.com> > <mailto:sami.muja...@arm.com <mailto:sami.muja...@arm.com>>; Jeshua Smith > <jesh...@nvidia.com <mailto:jesh...@nvidia.com> <mailto:jesh...@nvidia.com > <mailto: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 <mailto:jesh...@nvidia.com> > <mailto:jesh...@nvidia.com <mailto:jesh...@nvidia.com>>> > Reviewed-by: Pierre Gondois <pierre.gond...@arm.com > <mailto:pierre.gond...@arm.com> <mailto:pierre.gond...@arm.com > <mailto: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 (#116715): https://edk2.groups.io/g/devel/message/116715 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] -=-=-=-=-=-=-=-=-=-=-=-
