Re: [edk2-devel] [PATCH] DynamicTablesPkg: Allow for specified CPU names
Merged as 05da2d24b08b..5fb3f5723a1e Thanks. Regards, Sami Mujawar -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#97557): https://edk2.groups.io/g/devel/message/97557 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] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] DynamicTablesPkg: Allow for specified CPU names
Hi Jeff, I think we can go ahead with this patch for now. We can revisit this with any modifications should we have any other requirement that needs addressing. Reviewed-by: Sami Mujawar Regards, Sami Mujawar -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#97555): https://edk2.groups.io/g/devel/message/97555 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] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] DynamicTablesPkg: Allow for specified CPU names
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
---
.../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_TOKENLpiToken;
+ /// 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
+ BOOLEANOverrideNameUidEnabled;
+ /// 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] GeneratorThe 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] CpuNameValue used to generate the node name.
@param [in] ProcHierarchyNodeInfo CM_ARM_PROC_HIERARCHY_INFO describing
the Cpu.
@@ -643,7 +643,7 @@ CreateAmlCpuFromProcHierarchy (
INACPI_CPU_TOPOLOGY_GENERATOR *Generator,
IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
INAML_NODE_HANDLE ParentNode,
- INUINT32CpuIndex,
+ INUINT32CpuName,
INCM_ARM_PROC_HIERARCHY_INFO
*ProcHierarchyNodeInfo
)
{
@@ -668,7 +668
Re: [edk2-devel] [PATCH] DynamicTablesPkg: Allow for specified CPU names
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
> 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
> 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
>
> ---
>
> .../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_TOKENLpiToken;
>
> + /// 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
>
> + BOOLEANOverrideNameUidEnabled;
>
> + /// 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] GeneratorThe 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] CpuNameValue used to generate the node name.
>
>
