This was the simplest approach that I had to solve this issue, but not sure if
it would be better to do something smarter. A couple other ideas I had where:
- Specify the affinity level that CPUs use and use the levels above that for
the containers.
- Have a new object that has the container levels and affinity mapping. This
wouldn't support unbalanced layouts.
- I can't think of a good way to auto detect the cpu level affinity level but
that would be nice.
-Jeff
> -----Original Message-----
> From: Jeff Brasen <jbra...@nvidia.com>
> Sent: Monday, November 7, 2022 8:57 AM
> To: devel@edk2.groups.io
> Cc: sami.muja...@arm.com; alexei.fedo...@arm.com;
> pierre.gond...@arm.com; quic_llind...@quicinc.com;
> ardb+tianoc...@kernel.org; Jeff Brasen <jbra...@nvidia.com>
> Subject: [PATCH] DynamicTablesPkg: Allow for specified CPU names
>
> Allow object to specify the name of processor and processor container
>
> nodes and the UID of processor containers.
>
>
>
> This allows these to be more accurately referenced from other tables.
>
> For example for the _PSL method or the UID in the APMT table.
>
>
>
> The UID and Name for processor container may be different as if the
>
> intention is to set names as the corresponding affinity level the UID
>
> may need to be different if there are multiple levels of containers.
>
>
>
> Signed-off-by: Jeff Brasen <jbra...@nvidia.com>
>
> ---
>
> .../Include/ArmNameSpaceObjects.h | 11 +++++
>
> .../SsdtCpuTopologyGenerator.c | 40 ++++++++++++++-----
>
> .../ConfigurationManagerObjectParser.c | 3 ++
>
> 3 files changed, 43 insertions(+), 11 deletions(-)
>
>
>
> diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
>
> index 6aafd41a2e..19098609de 100644
>
> --- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
>
> +++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
>
> @@ -768,6 +768,17 @@ typedef struct CmArmProcHierarchyInfo {
>
> /// Token identifying a CM_ARM_OBJ_REF structure, itself referencing
>
> /// CM_ARM_LPI_INFO objects.
>
> CM_OBJECT_TOKEN LpiToken;
>
> + /// Set to TRUE if UID should override index for name and _UID
>
> + /// for processor container nodes and name of processors.
>
> + /// This should be consistently set for containers or processors to avoid
>
> + /// duplicate values
>
> + BOOLEAN OverrideNameUidEnabled;
>
> + /// If OverrideNameUidEnabled is TRUE then this value will be used for
> name of
>
> + /// processors and processor containers.
>
> + UINT16 OverrideName;
>
> + /// If OverrideNameUidEnabled is TRUE then this value will be used for
>
> + /// the UID of processor containers.
>
> + UINT32 OverrideUid;
>
> } CM_ARM_PROC_HIERARCHY_INFO;
>
>
>
> /** A structure that describes the Cache Type Structure (Type 1) in PPTT
>
> diff --git
> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCp
> uTopologyGenerator.c
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCp
> uTopologyGenerator.c
>
> index d06c7615fb..92fa904103 100644
>
> ---
> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCp
> uTopologyGenerator.c
>
> +++
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCp
> uTopologyGenerator.c
>
> @@ -553,7 +553,7 @@ GenerateLpiStates (
>
> @param [in] Generator The SSDT Cpu Topology generator.
>
> @param [in] ParentNode Parent node to attach the Cpu node to.
>
> @param [in] GicCInfo CM_ARM_GICC_INFO object used to create the
> node.
>
> - @param [in] CpuIndex Index used to generate the node name.
>
> + @param [in] CpuName Value used to generate the node name.
>
> @param [out] CpuNodePtr If not NULL, return the created Cpu node.
>
>
>
> @retval EFI_SUCCESS Success.
>
> @@ -567,7 +567,7 @@ CreateAmlCpu (
>
> IN ACPI_CPU_TOPOLOGY_GENERATOR *Generator,
>
> IN AML_NODE_HANDLE ParentNode,
>
> IN CM_ARM_GICC_INFO *GicCInfo,
>
> - IN UINT32 CpuIndex,
>
> + IN UINT32 CpuName,
>
> OUT AML_OBJECT_NODE_HANDLE *CpuNodePtr OPTIONAL
>
> )
>
> {
>
> @@ -579,7 +579,7 @@ CreateAmlCpu (
>
> ASSERT (ParentNode != NULL);
>
> ASSERT (GicCInfo != NULL);
>
>
>
> - Status = WriteAslName ('C', CpuIndex, AslName);
>
> + Status = WriteAslName ('C', CpuName, AslName);
>
> if (EFI_ERROR (Status)) {
>
> ASSERT (0);
>
> return Status;
>
> @@ -628,7 +628,7 @@ CreateAmlCpu (
>
> @param [in] CfgMgrProtocol Pointer to the Configuration Manager
>
> Protocol Interface.
>
> @param [in] ParentNode Parent node to attach the Cpu node to.
>
> - @param [in] CpuIndex Index used to generate the node name.
>
> + @param [in] CpuName Value used to generate the node name.
>
> @param [in] ProcHierarchyNodeInfo CM_ARM_PROC_HIERARCHY_INFO
> describing
>
> the Cpu.
>
>
>
> @@ -643,7 +643,7 @@ CreateAmlCpuFromProcHierarchy (
>
> IN ACPI_CPU_TOPOLOGY_GENERATOR *Generator,
>
> IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST
> CfgMgrProtocol,
>
> IN AML_NODE_HANDLE ParentNode,
>
> - IN UINT32 CpuIndex,
>
> + IN UINT32 CpuName,
>
> IN CM_ARM_PROC_HIERARCHY_INFO
> *ProcHierarchyNodeInfo
>
> )
>
> {
>
> @@ -668,7 +668,7 @@ CreateAmlCpuFromProcHierarchy (
>
> return Status;
>
> }
>
>
>
> - Status = CreateAmlCpu (Generator, ParentNode, GicCInfo, CpuIndex,
> &CpuNode);
>
> + Status = CreateAmlCpu (Generator, ParentNode, GicCInfo, CpuName,
> &CpuNode);
>
> if (EFI_ERROR (Status)) {
>
> ASSERT (0);
>
> return Status;
>
> @@ -735,7 +735,8 @@ CreateAmlProcessorContainer (
>
> IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST
> CfgMgrProtocol,
>
> IN AML_NODE_HANDLE ParentNode,
>
> IN CM_ARM_PROC_HIERARCHY_INFO
> *ProcHierarchyNodeInfo,
>
> - IN UINT32 ProcContainerIndex,
>
> + IN UINT16 ProcContainerName,
>
> + IN UINT32 ProcContainerUid,
>
> OUT AML_OBJECT_NODE_HANDLE
> *ProcContainerNodePtr
>
> )
>
> {
>
> @@ -749,7 +750,7 @@ CreateAmlProcessorContainer (
>
> ASSERT (ProcHierarchyNodeInfo != NULL);
>
> ASSERT (ProcContainerNodePtr != NULL);
>
>
>
> - Status = WriteAslName ('C', ProcContainerIndex, AslNameProcContainer);
>
> + Status = WriteAslName ('C', ProcContainerName, AslNameProcContainer);
>
> if (EFI_ERROR (Status)) {
>
> ASSERT (0);
>
> return Status;
>
> @@ -765,7 +766,7 @@ CreateAmlProcessorContainer (
>
> // and EFI_ACPI_6_3_PPTT_PROCESSOR_ID_INVALID is set for non-Cpus.
>
> Status = AmlCodeGenNameInteger (
>
> "_UID",
>
> - ProcContainerIndex,
>
> + ProcContainerUid,
>
> ProcContainerNode,
>
> NULL
>
> );
>
> @@ -838,6 +839,8 @@ CreateAmlCpuTopologyTree (
>
> UINT32 Index;
>
> UINT32 CpuIndex;
>
> AML_OBJECT_NODE_HANDLE ProcContainerNode;
>
> + UINT32 Uid;
>
> + UINT16 Name;
>
>
>
> ASSERT (Generator != NULL);
>
> ASSERT (Generator->ProcNodeList != NULL);
>
> @@ -868,11 +871,17 @@ CreateAmlCpuTopologyTree (
>
> return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> + if (Generator->ProcNodeList[Index].OverrideNameUidEnabled) {
>
> + Name = Generator->ProcNodeList[Index].OverrideName;
>
> + } else {
>
> + Name = CpuIndex;
>
> + }
>
> +
>
> Status = CreateAmlCpuFromProcHierarchy (
>
> Generator,
>
> CfgMgrProtocol,
>
> ParentNode,
>
> - CpuIndex,
>
> + Name,
>
> &Generator->ProcNodeList[Index]
>
> );
>
> if (EFI_ERROR (Status)) {
>
> @@ -897,12 +906,21 @@ CreateAmlCpuTopologyTree (
>
> return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> + if (Generator->ProcNodeList[Index].OverrideNameUidEnabled) {
>
> + Name = Generator->ProcNodeList[Index].OverrideName;
>
> + Uid = Generator->ProcNodeList[Index].OverrideUid;
>
> + } else {
>
> + Name = *ProcContainerIndex;
>
> + Uid = *ProcContainerIndex;
>
> + }
>
> +
>
> Status = CreateAmlProcessorContainer (
>
> Generator,
>
> CfgMgrProtocol,
>
> ParentNode,
>
> &Generator->ProcNodeList[Index],
>
> - *ProcContainerIndex,
>
> + Name,
>
> + Uid,
>
> &ProcContainerNode
>
> );
>
> if (EFI_ERROR (Status)) {
>
> diff --git
> a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManag
> erObjectParser.c
> b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManag
> erObjectParser.c
>
> index 5a01ed0fb8..5e4b88e8cc 100644
>
> ---
> a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManag
> erObjectParser.c
>
> +++
> b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManag
> erObjectParser.c
>
> @@ -305,6 +305,9 @@ STATIC CONST CM_OBJ_PARSER
> CmArmProcHierarchyInfoParser[] = {
>
> { "NoOfPrivateResources", 4, "0x%x", NULL },
>
> { "PrivateResourcesArrayToken", sizeof (CM_OBJECT_TOKEN), "0x%p",
> NULL },
>
> { "LpiToken", sizeof (CM_OBJECT_TOKEN), "0x%p", NULL },
>
> + { "OverrideNameUidEnabled", 1, "%d", NULL },
>
> + { "OverrideName", 2, "0x%x", NULL },
>
> + { "OverrideUid", 4, "0x%x", NULL }
>
> };
>
>
>
> /** A parser for EArmObjCacheInfo.
>
> --
>
> 2.25.1
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96037): https://edk2.groups.io/g/devel/message/96037
Mute This Topic: https://groups.io/mt/94869322/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
