> -----Original Message-----
> From: Ard Biesheuvel <a...@kernel.org>
> Sent: Wednesday, August 17, 2022 1:01 AM
> To: Jeff Brasen <jbra...@nvidia.com>
> Cc: devel@edk2.groups.io; quic_llind...@quicinc.com;
> ardb+tianoc...@kernel.org; sami.muja...@arm.com;
> alexei.fedo...@arm.com
> Subject: Re: [PATCH] DynamicTablesPkg: Correct cluster index
>
> External email: Use caution opening links or attachments
>
>
> On Tue, 16 Aug 2022 at 22:46, Jeff Brasen <jbra...@nvidia.com> wrote:
> >
> > Current code will generate duplicate UID if there are nested clusters
> > in the topology.
> >
>
> What is a nested cluster?
>
Better terminology would be nested processor containers, in our case we have
socket/cluster/cpu. I'll update the commit message. Should I also change
CreateAmlCluster to CreateAmlProcessorContainer?
> > Signed-off-by: Jeff Brasen <jbra...@nvidia.com>
> > ---
> > .../SsdtCpuTopologyGenerator.c | 22 ++++++++++++-------
> > 1 file changed, 14 insertions(+), 8 deletions(-)
> >
> > diff --git
> >
> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCp
> uT
> > opologyGenerator.c
> >
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCp
> uT
> > opologyGenerator.c
> > index 3266d8dd98..9295117f1f 100644
> > ---
> >
> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCp
> uT
> > opologyGenerator.c
> > +++
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/Ssdt
> > +++ CpuTopologyGenerator.c
> > @@ -706,6 +706,8 @@ CreateAmlCluster (
> > Cannot be CM_NULL_TOKEN.
> > @param [in] ParentNode Parent node to attach the created
> > node to.
> > + @param [in,out] ClusterIndex Pointer to the current cluster index
> > + to be used as UID.
> >
> > @retval EFI_SUCCESS Success.
> > @retval EFI_INVALID_PARAMETER Invalid parameter.
> > @@ -718,13 +720,13 @@ 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 *ClusterIndex
> > )
> > {
> > EFI_STATUS Status;
> > UINT32 Index;
> > UINT32 CpuIndex;
> > - UINT32 ClusterIndex;
> > AML_OBJECT_NODE_HANDLE ClusterNode;
> >
> > ASSERT (Generator != NULL);
> > @@ -733,9 +735,9 @@ CreateAmlCpuTopologyTree (
> > ASSERT (CfgMgrProtocol != NULL);
> > ASSERT (NodeToken != CM_NULL_TOKEN);
> > ASSERT (ParentNode != NULL);
> > + ASSERT (ClusterIndex != NULL);
> >
> > - CpuIndex = 0;
> > - ClusterIndex = 0;
> > + CpuIndex = 0;
> >
> > for (Index = 0; Index < Generator->ProcNodeCount; Index++) {
> > // Find the children of the CM_ARM_PROC_HIERARCHY_INFO @@ -
> 790,7
> > +792,7 @@ CreateAmlCpuTopologyTree (
> > CfgMgrProtocol,
> > ParentNode,
> > &Generator->ProcNodeList[Index],
> > - ClusterIndex,
> > + *ClusterIndex,
> > &ClusterNode
> > );
> > if (EFI_ERROR (Status)) {
> > @@ -800,7 +802,7 @@ CreateAmlCpuTopologyTree (
> >
> > // Nodes must have a unique name in the ASL namespace.
> > // Reset the Cpu index whenever we create a new Cluster.
> > - ClusterIndex++;
> > + (*ClusterIndex)++;
> > CpuIndex = 0;
> >
> > // Recursively continue creating an AML tree.
> > @@ -808,7 +810,8 @@ CreateAmlCpuTopologyTree (
> > Generator,
> > CfgMgrProtocol,
> > Generator->ProcNodeList[Index].Token,
> > - ClusterNode
> > + ClusterNode,
> > + ClusterIndex
> > );
> > if (EFI_ERROR (Status)) {
> > ASSERT (0);
> > @@ -845,6 +848,7 @@ CreateTopologyFromProcHierarchy (
> > EFI_STATUS Status;
> > UINT32 Index;
> > UINT32 TopLevelProcNodeIndex;
> > + UINT32 ClusterIndex;
> >
> > ASSERT (Generator != NULL);
> > ASSERT (Generator->ProcNodeCount != 0); @@ -853,6 +857,7 @@
> > CreateTopologyFromProcHierarchy (
> > ASSERT (ScopeNode != NULL);
> >
> > TopLevelProcNodeIndex = MAX_UINT32;
> > + ClusterIndex = 0;
> >
> > Status = TokenTableInitialize (Generator, Generator->ProcNodeCount);
> > if (EFI_ERROR (Status)) {
> > @@ -887,7 +892,8 @@ CreateTopologyFromProcHierarchy (
> > Generator,
> > CfgMgrProtocol,
> > Generator->ProcNodeList[TopLevelProcNodeIndex].Token,
> > - ScopeNode
> > + ScopeNode,
> > + &ClusterIndex
> > );
> > if (EFI_ERROR (Status)) {
> > ASSERT (0);
> > --
> > 2.25.1
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92526): https://edk2.groups.io/g/devel/message/92526
Mute This Topic: https://groups.io/mt/93067950/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
