1 file changed, 14 insertions(+), 8 deletions(-)
Current code will generate duplicate UID if there are nested clusters
in the topology.
Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
---
.../SsdtCpuTopologyGenerator.c | 22 ++++++++++++-------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
index 3266d8dd98..9295117f1f 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.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 (#92487): https://edk2.groups.io/g/devel/message/92487
Mute This Topic: https://groups.io/mt/93067950/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-On Tue, 16 Aug 2022 at 22:46, Jeff Brasen <jbrasen@nvidia.com> wrote:
>
> Current code will generate duplicate UID if there are nested clusters
> in the topology.
>
What is a nested cluster?
> Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
> ---
> .../SsdtCpuTopologyGenerator.c | 22 ++++++++++++-------
> 1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
> index 3266d8dd98..9295117f1f 100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.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 (#92502): https://edk2.groups.io/g/devel/message/92502
Mute This Topic: https://groups.io/mt/93067950/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
> -----Original Message-----
> From: Ard Biesheuvel <ardb@kernel.org>
> Sent: Wednesday, August 17, 2022 1:01 AM
> To: Jeff Brasen <jbrasen@nvidia.com>
> Cc: devel@edk2.groups.io; quic_llindhol@quicinc.com;
> ardb+tianocore@kernel.org; Sami.Mujawar@arm.com;
> Alexei.Fedorov@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 <jbrasen@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 <jbrasen@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On Wed, 17 Aug 2022 at 18:14, Jeff Brasen <jbrasen@nvidia.com> wrote: > > > > > -----Original Message----- > > From: Ard Biesheuvel <ardb@kernel.org> > > Sent: Wednesday, August 17, 2022 1:01 AM > > To: Jeff Brasen <jbrasen@nvidia.com> > > Cc: devel@edk2.groups.io; quic_llindhol@quicinc.com; > > ardb+tianocore@kernel.org; Sami.Mujawar@arm.com; > > Alexei.Fedorov@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 <jbrasen@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? > Works for me, but Sami has the final call as he maintains this package. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#92546): https://edk2.groups.io/g/devel/message/92546 Mute This Topic: https://groups.io/mt/93067950/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi Jeff, Thank you for this patch. On 18/08/2022 11:22 am, Ard Biesheuvel wrote: > On Wed, 17 Aug 2022 at 18:14, Jeff Brasen <jbrasen@nvidia.com> wrote: >> >> >>> -----Original Message----- >>> From: Ard Biesheuvel <ardb@kernel.org> >>> Sent: Wednesday, August 17, 2022 1:01 AM >>> To: Jeff Brasen <jbrasen@nvidia.com> >>> Cc: devel@edk2.groups.io; quic_llindhol@quicinc.com; >>> ardb+tianocore@kernel.org; Sami.Mujawar@arm.com; >>> Alexei.Fedorov@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 <jbrasen@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? >> > Works for me, but Sami has the final call as he maintains this package. [SAMI] CreateAmlProcessorContainer is more appropriate. Please change CreateAmlCluster to CreateAmlProcessorContainer. Regards, Sami Mujawar -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#92550): https://edk2.groups.io/g/devel/message/92550 Mute This Topic: https://groups.io/mt/93067950/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.