1 file changed, 12 insertions(+), 31 deletions(-)
In SSDT CPU topology generator allow for multiple top level physical
nodes as would be seen with a multi-socket system. This will create a
top level processor container for all systems.
Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
---
.../SsdtCpuTopologyGenerator.c | 43 ++++++-------------
1 file changed, 12 insertions(+), 31 deletions(-)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
index c24da8ec71..46b757e0b2 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
@@ -814,7 +814,8 @@ CreateAmlProcessorContainer (
Protocol Interface.
@param [in] NodeToken Token of the CM_ARM_PROC_HIERARCHY_INFO
currently handled.
- Cannot be CM_NULL_TOKEN.
+ CM_NULL_TOKEN if top level container
+ should be created.
@param [in] ParentNode Parent node to attach the created
node to.
@param [in,out] ProcContainerIndex Pointer to the current processor container
@@ -841,12 +842,12 @@ CreateAmlCpuTopologyTree (
AML_OBJECT_NODE_HANDLE ProcContainerNode;
UINT32 Uid;
UINT16 Name;
+ UINT32 NodeFlags;
ASSERT (Generator != NULL);
ASSERT (Generator->ProcNodeList != NULL);
ASSERT (Generator->ProcNodeCount != 0);
ASSERT (CfgMgrProtocol != NULL);
- ASSERT (NodeToken != CM_NULL_TOKEN);
ASSERT (ParentNode != NULL);
ASSERT (ProcContainerIndex != NULL);
@@ -893,8 +894,14 @@ CreateAmlCpuTopologyTree (
} else {
// If this is not a Cpu, then this is a processor container.
+ NodeFlags = Generator->ProcNodeList[Index].Flags;
+ // Allow physical property for top level nodes
+ if (NodeToken == CM_NULL_TOKEN) {
+ NodeFlags &= ~EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL;
+ }
+
// Acpi processor Id for clusters is not handled.
- if ((Generator->ProcNodeList[Index].Flags & PPTT_PROCESSOR_MASK) !=
+ if ((NodeFlags & PPTT_PROCESSOR_MASK) !=
PPTT_CLUSTER_PROCESSOR_MASK)
{
DEBUG ((
@@ -974,8 +981,6 @@ CreateTopologyFromProcHierarchy (
)
{
EFI_STATUS Status;
- UINT32 Index;
- UINT32 TopLevelProcNodeIndex;
UINT32 ProcContainerIndex;
ASSERT (Generator != NULL);
@@ -984,8 +989,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 +997,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
);
--
2.25.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99615): https://edk2.groups.io/g/devel/message/99615
Mute This Topic: https://groups.io/mt/96729031/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hello Jeff, Thanks for the v2. Also cf the first discussion at: https://edk2.groups.io/g/devel/topic/96680589#99612 - I think it would be good to extract a function that does all the checks as there are many possibilities for the flags/parent combinations. - I think it would also be nice to reset the index of ProcContainers for each new level (i.e. not to have the same incrementing index for clusters/packages) I created a branch based on your work at: https://github.com/pierregondois/edk2/tree/pg/top_level_pnode_Wip Regards, Pierre On 2/3/23 19:10, Jeff Brasen wrote: > In SSDT CPU topology generator allow for multiple top level physical > nodes as would be seen with a multi-socket system. This will create a > top level processor container for all systems. > > Signed-off-by: Jeff Brasen <jbrasen@nvidia.com> > --- > .../SsdtCpuTopologyGenerator.c | 43 ++++++------------- > 1 file changed, 12 insertions(+), 31 deletions(-) > > diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c > index c24da8ec71..46b757e0b2 100644 > --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c > +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c > @@ -814,7 +814,8 @@ CreateAmlProcessorContainer ( > Protocol Interface. > @param [in] NodeToken Token of the CM_ARM_PROC_HIERARCHY_INFO > currently handled. > - Cannot be CM_NULL_TOKEN. > + CM_NULL_TOKEN if top level container > + should be created. > @param [in] ParentNode Parent node to attach the created > node to. > @param [in,out] ProcContainerIndex Pointer to the current processor container > @@ -841,12 +842,12 @@ CreateAmlCpuTopologyTree ( > AML_OBJECT_NODE_HANDLE ProcContainerNode; > UINT32 Uid; > UINT16 Name; > + UINT32 NodeFlags; > > ASSERT (Generator != NULL); > ASSERT (Generator->ProcNodeList != NULL); > ASSERT (Generator->ProcNodeCount != 0); > ASSERT (CfgMgrProtocol != NULL); > - ASSERT (NodeToken != CM_NULL_TOKEN); > ASSERT (ParentNode != NULL); > ASSERT (ProcContainerIndex != NULL); > > @@ -893,8 +894,14 @@ CreateAmlCpuTopologyTree ( > } else { > // If this is not a Cpu, then this is a processor container. > > + NodeFlags = Generator->ProcNodeList[Index].Flags; > + // Allow physical property for top level nodes > + if (NodeToken == CM_NULL_TOKEN) { > + NodeFlags &= ~EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL; > + } > + I think that if (NodeToken == CM_NULL_TOKEN) and doesn't have the Physical Package flag, no error will be triggered even though this is not a valid configuration. > // Acpi processor Id for clusters is not handled. > - if ((Generator->ProcNodeList[Index].Flags & PPTT_PROCESSOR_MASK) != > + if ((NodeFlags & PPTT_PROCESSOR_MASK) != > PPTT_CLUSTER_PROCESSOR_MASK) > { > DEBUG (( > @@ -974,8 +981,6 @@ CreateTopologyFromProcHierarchy ( > ) > { > EFI_STATUS Status; > - UINT32 Index; > - UINT32 TopLevelProcNodeIndex; > UINT32 ProcContainerIndex; > > ASSERT (Generator != NULL); > @@ -984,8 +989,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 +997,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 > ); -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#99655): https://edk2.groups.io/g/devel/message/99655 Mute This Topic: https://groups.io/mt/96729031/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
The changes on your branch seem pretty good to me > -----Original Message----- > From: Pierre Gondois <pierre.gondois@arm.com> > Sent: Monday, February 6, 2023 2:28 AM > To: Jeff Brasen <jbrasen@nvidia.com>; devel@edk2.groups.io > Cc: Sami.Mujawar@arm.com; Alexei.Fedorov@arm.com; > quic_llindhol@quicinc.com; ardb+tianocore@kernel.org > Subject: Re: [PATCH v2] DynamicTablesPkg: Allow multiple top level physical > nodes > > External email: Use caution opening links or attachments > > > Hello Jeff, > Thanks for the v2. Also cf the first discussion at: > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk > 2.groups.io%2Fg%2Fdevel%2Ftopic%2F96680589%2399612&data=05%7C01% > 7Cjbrasen%40nvidia.com%7Ccee1a4886a754ba2d28508db08246448%7C43083 > d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638112724625353615%7CUnkn > own%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik > 1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=ur6vlVCBpt%2BQdid > 3xJKglx3trDZb4kxajkAWFXsr920%3D&reserved=0 > - I think it would be good to extract a function that does all the checks as > there are many possibilities for the flags/parent combinations. > - I think it would also be nice to reset the index of ProcContainers for each > new level (i.e. not to have the same incrementing index for > clusters/packages) > > I created a branch based on your work at: > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith > ub.com%2Fpierregondois%2Fedk2%2Ftree%2Fpg%2Ftop_level_pnode_Wip > &data=05%7C01%7Cjbrasen%40nvidia.com%7Ccee1a4886a754ba2d28508db0 > 8246448%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C63811272462 > 5353615%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV > 2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Ydj > RbbboKTyVmi2rr2bvu0ARx9uey5mLYtWikbkP7Ek%3D&reserved=0 > > Regards, > Pierre > > On 2/3/23 19:10, Jeff Brasen wrote: > > In SSDT CPU topology generator allow for multiple top level physical > > nodes as would be seen with a multi-socket system. This will create a > > top level processor container for all systems. > > > > Signed-off-by: Jeff Brasen <jbrasen@nvidia.com> > > --- > > .../SsdtCpuTopologyGenerator.c | 43 ++++++------------- > > 1 file changed, 12 insertions(+), 31 deletions(-) > > > > diff --git > > > a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCp > uT > > opologyGenerator.c > > > b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCp > uT > > opologyGenerator.c > > index c24da8ec71..46b757e0b2 100644 > > --- > > > a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCp > uT > > opologyGenerator.c > > +++ > b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/Ssdt > > +++ CpuTopologyGenerator.c > > @@ -814,7 +814,8 @@ CreateAmlProcessorContainer ( > > Protocol Interface. > > @param [in] NodeToken Token of the > CM_ARM_PROC_HIERARCHY_INFO > > currently handled. > > - Cannot be CM_NULL_TOKEN. > > + CM_NULL_TOKEN if top level container > > + should be created. > > @param [in] ParentNode Parent node to attach the created > > node to. > > @param [in,out] ProcContainerIndex Pointer to the current > > processor container @@ -841,12 +842,12 @@ CreateAmlCpuTopologyTree > ( > > AML_OBJECT_NODE_HANDLE ProcContainerNode; > > UINT32 Uid; > > UINT16 Name; > > + UINT32 NodeFlags; > > > > ASSERT (Generator != NULL); > > ASSERT (Generator->ProcNodeList != NULL); > > ASSERT (Generator->ProcNodeCount != 0); > > ASSERT (CfgMgrProtocol != NULL); > > - ASSERT (NodeToken != CM_NULL_TOKEN); > > ASSERT (ParentNode != NULL); > > ASSERT (ProcContainerIndex != NULL); > > > > @@ -893,8 +894,14 @@ CreateAmlCpuTopologyTree ( > > } else { > > // If this is not a Cpu, then this is a processor container. > > > > + NodeFlags = Generator->ProcNodeList[Index].Flags; > > + // Allow physical property for top level nodes > > + if (NodeToken == CM_NULL_TOKEN) { > > + NodeFlags &= ~EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL; > > + } > > + > > I think that if (NodeToken == CM_NULL_TOKEN) and doesn't have the > Physical Package flag, no error will be triggered even though this is not a valid > configuration. > > > // Acpi processor Id for clusters is not handled. > > - if ((Generator->ProcNodeList[Index].Flags & > PPTT_PROCESSOR_MASK) != > > + if ((NodeFlags & PPTT_PROCESSOR_MASK) != > > PPTT_CLUSTER_PROCESSOR_MASK) > > { > > DEBUG (( > > @@ -974,8 +981,6 @@ CreateTopologyFromProcHierarchy ( > > ) > > { > > EFI_STATUS Status; > > - UINT32 Index; > > - UINT32 TopLevelProcNodeIndex; > > UINT32 ProcContainerIndex; > > > > ASSERT (Generator != NULL); > > @@ -984,8 +989,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 +997,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 > > ); -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100115): https://edk2.groups.io/g/devel/message/100115 Mute This Topic: https://groups.io/mt/96729031/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.