From: "PierreGondois" <pierre.gondois@arm.com>
To: devel@edk2.groups.io, jbrasen@nvidia.com
Cc: Sami.Mujawar@arm.com, Alexei.Fedorov@arm.com,
quic_llindhol@quicinc.com, ardb+tianocore@kernel.org
Subject: Re: [edk2-devel] [PATCH] DynamicTablesPkg: Allow for specified CPU names
Date: Wed, 9 Nov 2022 13:58:07 +0100 [thread overview]
Message-ID: <14750ed9-124f-5441-b378-d34a8e6f09a2@arm.com> (raw)
In-Reply-To: <0fd42dc1fd90f28521a3a1e47934fa7d45bcec6b.1667836523.git.jbrasen@nvidia.com>
Hello Jeff,
On 11/7/22 16:57, Jeff Brasen via groups.io wrote:
> 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 CM_ARM_PROC_HIERARCHY_INFO struct has a token that can be used
for referencing. If there were generators for the _PSL method and the
APMT table, this would be possible to avoid handwriting CPUs' Name/Uid
and use the ones generated in SsdtCpuTopologyGenerator.c somehow.
This would take more time to do. Otherwise the code itself looks good
to me,
Regards,
Pierre
>
>
>
> 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 <jbrasen@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/SsdtCpuTopologyGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
>
> index d06c7615fb..92fa904103 100644
>
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
>
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.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/ConfigurationManagerObjectParser.c b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
>
> index 5a01ed0fb8..5e4b88e8cc 100644
>
> --- a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
>
> +++ b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.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.
>
prev parent reply other threads:[~2022-11-09 12:58 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-07 15:57 [PATCH] DynamicTablesPkg: Allow for specified CPU names Jeff Brasen
2022-11-07 16:06 ` Jeff Brasen
2022-12-19 11:24 ` [edk2-devel] " Sami Mujawar
2022-12-19 12:56 ` Sami Mujawar
2022-11-09 12:58 ` PierreGondois [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=14750ed9-124f-5441-b378-d34a8e6f09a2@arm.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox