* [edk2-devel] [PATCH v2] DynamicTablesPkg/SSDT: Require Package node in hierarchy
@ 2024-02-05 19:00 Jeshua Smith via groups.io
2024-03-12 16:11 ` Jeshua Smith via groups.io
0 siblings, 1 reply; 4+ messages in thread
From: Jeshua Smith via groups.io @ 2024-02-05 19:00 UTC (permalink / raw)
To: devel
Cc: ardb+tianocore, quic_llindhol, pierre.gondois, Sami.Mujawar,
Jeshua Smith
The code was incorrectly assuming that root nodes had to be physical
package nodes and vice versa. This is not always true, so update the
check to simply require exactly one package node somewhere in the
hierarchy.
Signed-off-by: Jeshua Smith <jeshuas@nvidia.com>
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
---
Note: This is a complete replacement for [PATCH] DynamicTablesPkg/SSDT: Remove incorrect root node check
Version 2: added documentation for the PackageNodeSeen parameter
.../SsdtCpuTopologyGenerator.c | 32 +++++++++++++------
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
index 9e3efb49e6..40ed10eae6 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
@@ -1072,6 +1072,7 @@ CreateAmlProcessorContainer (
@param [in] IsLeaf The ProcNode is a leaf.
@param [in] NodeToken NodeToken of the ProcNode.
@param [in] ParentNodeToken Parent NodeToken of the ProcNode.
+ @param [in] PackageNodeSeen A parent of the ProcNode has the physical package flag set.
@retval EFI_SUCCESS Success.
@retval EFI_INVALID_PARAMETER Invalid parameter.
@@ -1083,23 +1084,24 @@ CheckProcNode (
UINT32 NodeFlags,
BOOLEAN IsLeaf,
CM_OBJECT_TOKEN NodeToken,
- CM_OBJECT_TOKEN ParentNodeToken
+ CM_OBJECT_TOKEN ParentNodeToken,
+ BOOLEAN PackageNodeSeen
)
{
BOOLEAN InvalidFlags;
BOOLEAN HasPhysicalPackageBit;
- BOOLEAN IsTopLevelNode;
HasPhysicalPackageBit = (NodeFlags & EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL) ==
EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL;
- IsTopLevelNode = (ParentNodeToken == CM_NULL_TOKEN);
- // A top-level node is a Physical Package and conversely.
- InvalidFlags = HasPhysicalPackageBit ^ IsTopLevelNode;
+ // Only one Physical Package flag is allowed in the hierarchy
+ InvalidFlags = HasPhysicalPackageBit && PackageNodeSeen;
// Check Leaf specific flags.
if (IsLeaf) {
InvalidFlags |= ((NodeFlags & PPTT_LEAF_MASK) != PPTT_LEAF_MASK);
+ // Must have Physical Package flag somewhere in the hierarchy
+ InvalidFlags |= !(HasPhysicalPackageBit || PackageNodeSeen);
} else {
InvalidFlags |= ((NodeFlags & PPTT_LEAF_MASK) != 0);
}
@@ -1130,6 +1132,7 @@ CheckProcNode (
node to.
@param [in,out] ProcContainerIndex Pointer to the current processor container
index to be used as UID.
+ @param [in] PackageNodeSeen A parent of the ProcNode has the physical package flag set.
@retval EFI_SUCCESS Success.
@retval EFI_INVALID_PARAMETER Invalid parameter.
@@ -1143,7 +1146,8 @@ CreateAmlCpuTopologyTree (
IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
IN CM_OBJECT_TOKEN NodeToken,
IN AML_NODE_HANDLE ParentNode,
- IN OUT UINT32 *ProcContainerIndex
+ IN OUT UINT32 *ProcContainerIndex,
+ IN BOOLEAN PackageNodeSeen
)
{
EFI_STATUS Status;
@@ -1153,6 +1157,7 @@ CreateAmlCpuTopologyTree (
AML_OBJECT_NODE_HANDLE ProcContainerNode;
UINT32 Uid;
UINT16 Name;
+ BOOLEAN HasPhysicalPackageBit;
ASSERT (Generator != NULL);
ASSERT (Generator->ProcNodeList != NULL);
@@ -1175,7 +1180,8 @@ CreateAmlCpuTopologyTree (
Generator->ProcNodeList[Index].Flags,
TRUE,
Generator->ProcNodeList[Index].Token,
- NodeToken
+ NodeToken,
+ PackageNodeSeen
);
if (EFI_ERROR (Status)) {
ASSERT (0);
@@ -1208,7 +1214,8 @@ CreateAmlCpuTopologyTree (
Generator->ProcNodeList[Index].Flags,
FALSE,
Generator->ProcNodeList[Index].Token,
- NodeToken
+ NodeToken,
+ PackageNodeSeen
);
if (EFI_ERROR (Status)) {
ASSERT (0);
@@ -1249,13 +1256,17 @@ CreateAmlCpuTopologyTree (
ProcContainerName++;
}
+ HasPhysicalPackageBit = (Generator->ProcNodeList[Index].Flags & EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL) ==
+ EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL;
+
// Recursively continue creating an AML tree.
Status = CreateAmlCpuTopologyTree (
Generator,
CfgMgrProtocol,
Generator->ProcNodeList[Index].Token,
ProcContainerNode,
- ProcContainerIndex
+ ProcContainerIndex,
+ (PackageNodeSeen || HasPhysicalPackageBit)
);
if (EFI_ERROR (Status)) {
ASSERT (0);
@@ -1311,7 +1322,8 @@ CreateTopologyFromProcHierarchy (
CfgMgrProtocol,
CM_NULL_TOKEN,
ScopeNode,
- &ProcContainerIndex
+ &ProcContainerIndex,
+ FALSE
);
if (EFI_ERROR (Status)) {
ASSERT (0);
--
2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115126): https://edk2.groups.io/g/devel/message/115126
Mute This Topic: https://groups.io/mt/104183075/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH v2] DynamicTablesPkg/SSDT: Require Package node in hierarchy
2024-02-05 19:00 [edk2-devel] [PATCH v2] DynamicTablesPkg/SSDT: Require Package node in hierarchy Jeshua Smith via groups.io
@ 2024-03-12 16:11 ` Jeshua Smith via groups.io
2024-03-12 16:14 ` Sami Mujawar
0 siblings, 1 reply; 4+ messages in thread
From: Jeshua Smith via groups.io @ 2024-03-12 16:11 UTC (permalink / raw)
To: devel@edk2.groups.io
Cc: ardb+tianocore@kernel.org, quic_llindhol@quicinc.com,
pierre.gondois@arm.com, Sami.Mujawar@arm.com
Can we get this reviewed/merged?
> -----Original Message-----
> From: Jeshua Smith <jeshuas@nvidia.com>
> Sent: Monday, February 5, 2024 12:01 PM
> To: devel@edk2.groups.io
> Cc: ardb+tianocore@kernel.org; quic_llindhol@quicinc.com;
> pierre.gondois@arm.com; Sami.Mujawar@arm.com; Jeshua Smith
> <jeshuas@nvidia.com>
> Subject: [PATCH v2] DynamicTablesPkg/SSDT: Require Package node in
> hierarchy
>
> The code was incorrectly assuming that root nodes had to be physical package
> nodes and vice versa. This is not always true, so update the check to simply
> require exactly one package node somewhere in the hierarchy.
>
> Signed-off-by: Jeshua Smith <jeshuas@nvidia.com>
> Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
> Note: This is a complete replacement for [PATCH] DynamicTablesPkg/SSDT:
> Remove incorrect root node check
>
> Version 2: added documentation for the PackageNodeSeen parameter
>
> .../SsdtCpuTopologyGenerator.c | 32 +++++++++++++------
> 1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git
> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpu
> TopologyGenerator.c
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpu
> TopologyGenerator.c
> index 9e3efb49e6..40ed10eae6 100644
> ---
> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpu
> TopologyGenerator.c
> +++
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCp
> +++ uTopologyGenerator.c
> @@ -1072,6 +1072,7 @@ CreateAmlProcessorContainer (
> @param [in] IsLeaf The ProcNode is a leaf.
> @param [in] NodeToken NodeToken of the ProcNode.
> @param [in] ParentNodeToken Parent NodeToken of the ProcNode.
> + @param [in] PackageNodeSeen A parent of the ProcNode has the physical
> package flag set.
>
> @retval EFI_SUCCESS Success.
> @retval EFI_INVALID_PARAMETER Invalid parameter.
> @@ -1083,23 +1084,24 @@ CheckProcNode (
> UINT32 NodeFlags,
> BOOLEAN IsLeaf,
> CM_OBJECT_TOKEN NodeToken,
> - CM_OBJECT_TOKEN ParentNodeToken
> + CM_OBJECT_TOKEN ParentNodeToken,
> + BOOLEAN PackageNodeSeen
> )
> {
> BOOLEAN InvalidFlags;
> BOOLEAN HasPhysicalPackageBit;
> - BOOLEAN IsTopLevelNode;
>
> HasPhysicalPackageBit = (NodeFlags &
> EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL) ==
> EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL;
> - IsTopLevelNode = (ParentNodeToken == CM_NULL_TOKEN);
>
> - // A top-level node is a Physical Package and conversely.
> - InvalidFlags = HasPhysicalPackageBit ^ IsTopLevelNode;
> + // Only one Physical Package flag is allowed in the hierarchy
> + InvalidFlags = HasPhysicalPackageBit && PackageNodeSeen;
>
> // Check Leaf specific flags.
> if (IsLeaf) {
> InvalidFlags |= ((NodeFlags & PPTT_LEAF_MASK) != PPTT_LEAF_MASK);
> + // Must have Physical Package flag somewhere in the hierarchy
> + InvalidFlags |= !(HasPhysicalPackageBit || PackageNodeSeen);
> } else {
> InvalidFlags |= ((NodeFlags & PPTT_LEAF_MASK) != 0);
> }
> @@ -1130,6 +1132,7 @@ CheckProcNode (
> node to.
> @param [in,out] ProcContainerIndex Pointer to the current processor
> container
> index to be used as UID.
> + @param [in] PackageNodeSeen A parent of the ProcNode has the
> physical package flag set.
>
> @retval EFI_SUCCESS Success.
> @retval EFI_INVALID_PARAMETER Invalid parameter.
> @@ -1143,7 +1146,8 @@ CreateAmlCpuTopologyTree (
> IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST
> CfgMgrProtocol,
> IN CM_OBJECT_TOKEN NodeToken,
> IN AML_NODE_HANDLE ParentNode,
> - IN OUT UINT32 *ProcContainerIndex
> + IN OUT UINT32 *ProcContainerIndex,
> + IN BOOLEAN PackageNodeSeen
> )
> {
> EFI_STATUS Status;
> @@ -1153,6 +1157,7 @@ CreateAmlCpuTopologyTree (
> AML_OBJECT_NODE_HANDLE ProcContainerNode;
> UINT32 Uid;
> UINT16 Name;
> + BOOLEAN HasPhysicalPackageBit;
>
> ASSERT (Generator != NULL);
> ASSERT (Generator->ProcNodeList != NULL); @@ -1175,7 +1180,8 @@
> CreateAmlCpuTopologyTree (
> Generator->ProcNodeList[Index].Flags,
> TRUE,
> Generator->ProcNodeList[Index].Token,
> - NodeToken
> + NodeToken,
> + PackageNodeSeen
> );
> if (EFI_ERROR (Status)) {
> ASSERT (0);
> @@ -1208,7 +1214,8 @@ CreateAmlCpuTopologyTree (
> Generator->ProcNodeList[Index].Flags,
> FALSE,
> Generator->ProcNodeList[Index].Token,
> - NodeToken
> + NodeToken,
> + PackageNodeSeen
> );
> if (EFI_ERROR (Status)) {
> ASSERT (0);
> @@ -1249,13 +1256,17 @@ CreateAmlCpuTopologyTree (
> ProcContainerName++;
> }
>
> + HasPhysicalPackageBit = (Generator->ProcNodeList[Index].Flags &
> EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL) ==
> + EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL;
> +
> // Recursively continue creating an AML tree.
> Status = CreateAmlCpuTopologyTree (
> Generator,
> CfgMgrProtocol,
> Generator->ProcNodeList[Index].Token,
> ProcContainerNode,
> - ProcContainerIndex
> + ProcContainerIndex,
> + (PackageNodeSeen || HasPhysicalPackageBit)
> );
> if (EFI_ERROR (Status)) {
> ASSERT (0);
> @@ -1311,7 +1322,8 @@ CreateTopologyFromProcHierarchy (
> CfgMgrProtocol,
> CM_NULL_TOKEN,
> ScopeNode,
> - &ProcContainerIndex
> + &ProcContainerIndex,
> + FALSE
> );
> if (EFI_ERROR (Status)) {
> ASSERT (0);
> --
> 2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116689): https://edk2.groups.io/g/devel/message/116689
Mute This Topic: https://groups.io/mt/104183075/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH v2] DynamicTablesPkg/SSDT: Require Package node in hierarchy
2024-03-12 16:11 ` Jeshua Smith via groups.io
@ 2024-03-12 16:14 ` Sami Mujawar
2024-03-13 14:23 ` Sami Mujawar
0 siblings, 1 reply; 4+ messages in thread
From: Sami Mujawar @ 2024-03-12 16:14 UTC (permalink / raw)
To: Jeshua Smith, devel@edk2.groups.io
Cc: ardb+tianocore@kernel.org, quic_llindhol@quicinc.com,
Pierre Gondois, nd
Hi Jeshua,
Apologies, I somehow missed getting this merged.
I will get this in before end of this week.
Regards,
Sami Mujawar
On 12/03/2024, 16:12, "Jeshua Smith" <jeshuas@nvidia.com <mailto:jeshuas@nvidia.com>> wrote:
Can we get this reviewed/merged?
> -----Original Message-----
> From: Jeshua Smith <jeshuas@nvidia.com <mailto:jeshuas@nvidia.com>>
> Sent: Monday, February 5, 2024 12:01 PM
> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> Cc: ardb+tianocore@kernel.org <mailto:ardb+tianocore@kernel.org>; quic_llindhol@quicinc.com <mailto:quic_llindhol@quicinc.com>;
> pierre.gondois@arm.com <mailto:pierre.gondois@arm.com>; Sami.Mujawar@arm.com <mailto:Sami.Mujawar@arm.com>; Jeshua Smith
> <jeshuas@nvidia.com <mailto:jeshuas@nvidia.com>>
> Subject: [PATCH v2] DynamicTablesPkg/SSDT: Require Package node in
> hierarchy
>
> The code was incorrectly assuming that root nodes had to be physical package
> nodes and vice versa. This is not always true, so update the check to simply
> require exactly one package node somewhere in the hierarchy.
>
> Signed-off-by: Jeshua Smith <jeshuas@nvidia.com <mailto:jeshuas@nvidia.com>>
> Reviewed-by: Pierre Gondois <pierre.gondois@arm.com <mailto:pierre.gondois@arm.com>>
> ---
> Note: This is a complete replacement for [PATCH] DynamicTablesPkg/SSDT:
> Remove incorrect root node check
>
> Version 2: added documentation for the PackageNodeSeen parameter
>
> .../SsdtCpuTopologyGenerator.c | 32 +++++++++++++------
> 1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git
> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpu
> TopologyGenerator.c
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpu
> TopologyGenerator.c
> index 9e3efb49e6..40ed10eae6 100644
> ---
> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpu
> TopologyGenerator.c
> +++
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCp
> +++ uTopologyGenerator.c
> @@ -1072,6 +1072,7 @@ CreateAmlProcessorContainer (
> @param [in] IsLeaf The ProcNode is a leaf.
> @param [in] NodeToken NodeToken of the ProcNode.
> @param [in] ParentNodeToken Parent NodeToken of the ProcNode.
> + @param [in] PackageNodeSeen A parent of the ProcNode has the physical
> package flag set.
>
> @retval EFI_SUCCESS Success.
> @retval EFI_INVALID_PARAMETER Invalid parameter.
> @@ -1083,23 +1084,24 @@ CheckProcNode (
> UINT32 NodeFlags,
> BOOLEAN IsLeaf,
> CM_OBJECT_TOKEN NodeToken,
> - CM_OBJECT_TOKEN ParentNodeToken
> + CM_OBJECT_TOKEN ParentNodeToken,
> + BOOLEAN PackageNodeSeen
> )
> {
> BOOLEAN InvalidFlags;
> BOOLEAN HasPhysicalPackageBit;
> - BOOLEAN IsTopLevelNode;
>
> HasPhysicalPackageBit = (NodeFlags &
> EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL) ==
> EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL;
> - IsTopLevelNode = (ParentNodeToken == CM_NULL_TOKEN);
>
> - // A top-level node is a Physical Package and conversely.
> - InvalidFlags = HasPhysicalPackageBit ^ IsTopLevelNode;
> + // Only one Physical Package flag is allowed in the hierarchy
> + InvalidFlags = HasPhysicalPackageBit && PackageNodeSeen;
>
> // Check Leaf specific flags.
> if (IsLeaf) {
> InvalidFlags |= ((NodeFlags & PPTT_LEAF_MASK) != PPTT_LEAF_MASK);
> + // Must have Physical Package flag somewhere in the hierarchy
> + InvalidFlags |= !(HasPhysicalPackageBit || PackageNodeSeen);
> } else {
> InvalidFlags |= ((NodeFlags & PPTT_LEAF_MASK) != 0);
> }
> @@ -1130,6 +1132,7 @@ CheckProcNode (
> node to.
> @param [in,out] ProcContainerIndex Pointer to the current processor
> container
> index to be used as UID.
> + @param [in] PackageNodeSeen A parent of the ProcNode has the
> physical package flag set.
>
> @retval EFI_SUCCESS Success.
> @retval EFI_INVALID_PARAMETER Invalid parameter.
> @@ -1143,7 +1146,8 @@ CreateAmlCpuTopologyTree (
> IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST
> CfgMgrProtocol,
> IN CM_OBJECT_TOKEN NodeToken,
> IN AML_NODE_HANDLE ParentNode,
> - IN OUT UINT32 *ProcContainerIndex
> + IN OUT UINT32 *ProcContainerIndex,
> + IN BOOLEAN PackageNodeSeen
> )
> {
> EFI_STATUS Status;
> @@ -1153,6 +1157,7 @@ CreateAmlCpuTopologyTree (
> AML_OBJECT_NODE_HANDLE ProcContainerNode;
> UINT32 Uid;
> UINT16 Name;
> + BOOLEAN HasPhysicalPackageBit;
>
> ASSERT (Generator != NULL);
> ASSERT (Generator->ProcNodeList != NULL); @@ -1175,7 +1180,8 @@
> CreateAmlCpuTopologyTree (
> Generator->ProcNodeList[Index].Flags,
> TRUE,
> Generator->ProcNodeList[Index].Token,
> - NodeToken
> + NodeToken,
> + PackageNodeSeen
> );
> if (EFI_ERROR (Status)) {
> ASSERT (0);
> @@ -1208,7 +1214,8 @@ CreateAmlCpuTopologyTree (
> Generator->ProcNodeList[Index].Flags,
> FALSE,
> Generator->ProcNodeList[Index].Token,
> - NodeToken
> + NodeToken,
> + PackageNodeSeen
> );
> if (EFI_ERROR (Status)) {
> ASSERT (0);
> @@ -1249,13 +1256,17 @@ CreateAmlCpuTopologyTree (
> ProcContainerName++;
> }
>
> + HasPhysicalPackageBit = (Generator->ProcNodeList[Index].Flags &
> EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL) ==
> + EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL;
> +
> // Recursively continue creating an AML tree.
> Status = CreateAmlCpuTopologyTree (
> Generator,
> CfgMgrProtocol,
> Generator->ProcNodeList[Index].Token,
> ProcContainerNode,
> - ProcContainerIndex
> + ProcContainerIndex,
> + (PackageNodeSeen || HasPhysicalPackageBit)
> );
> if (EFI_ERROR (Status)) {
> ASSERT (0);
> @@ -1311,7 +1322,8 @@ CreateTopologyFromProcHierarchy (
> CfgMgrProtocol,
> CM_NULL_TOKEN,
> ScopeNode,
> - &ProcContainerIndex
> + &ProcContainerIndex,
> + FALSE
> );
> if (EFI_ERROR (Status)) {
> ASSERT (0);
> --
> 2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116690): https://edk2.groups.io/g/devel/message/116690
Mute This Topic: https://groups.io/mt/104183075/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH v2] DynamicTablesPkg/SSDT: Require Package node in hierarchy
2024-03-12 16:14 ` Sami Mujawar
@ 2024-03-13 14:23 ` Sami Mujawar
0 siblings, 0 replies; 4+ messages in thread
From: Sami Mujawar @ 2024-03-13 14:23 UTC (permalink / raw)
To: Jeshua Smith, devel@edk2.groups.io
Cc: ardb+tianocore@kernel.org, quic_llindhol@quicinc.com,
Pierre Gondois, nd
Merged at https://github.com/tianocore/edk2/commit/308e6e0936c43063551babb4a71c46775b1dc01c
Thanks.
Regards,
Sami Mujawar
On 12/03/2024, 16:14, "Sami Mujawar" <Sami.Mujawar@arm.com <mailto:Sami.Mujawar@arm.com>> wrote:
Hi Jeshua,
Apologies, I somehow missed getting this merged.
I will get this in before end of this week.
Regards,
Sami Mujawar
On 12/03/2024, 16:12, "Jeshua Smith" <jeshuas@nvidia.com <mailto:jeshuas@nvidia.com> <mailto:jeshuas@nvidia.com <mailto:jeshuas@nvidia.com>>> wrote:
Can we get this reviewed/merged?
> -----Original Message-----
> From: Jeshua Smith <jeshuas@nvidia.com <mailto:jeshuas@nvidia.com> <mailto:jeshuas@nvidia.com <mailto:jeshuas@nvidia.com>>>
> Sent: Monday, February 5, 2024 12:01 PM
> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io> <mailto:devel@edk2.groups.io <mailto:devel@edk2.groups.io>>
> Cc: ardb+tianocore@kernel.org <mailto:ardb+tianocore@kernel.org> <mailto:ardb+tianocore@kernel.org <mailto:ardb+tianocore@kernel.org>>; quic_llindhol@quicinc.com <mailto:quic_llindhol@quicinc.com> <mailto:quic_llindhol@quicinc.com <mailto:quic_llindhol@quicinc.com>>;
> pierre.gondois@arm.com <mailto:pierre.gondois@arm.com> <mailto:pierre.gondois@arm.com <mailto:pierre.gondois@arm.com>>; Sami.Mujawar@arm.com <mailto:Sami.Mujawar@arm.com> <mailto:Sami.Mujawar@arm.com <mailto:Sami.Mujawar@arm.com>>; Jeshua Smith
> <jeshuas@nvidia.com <mailto:jeshuas@nvidia.com> <mailto:jeshuas@nvidia.com <mailto:jeshuas@nvidia.com>>>
> Subject: [PATCH v2] DynamicTablesPkg/SSDT: Require Package node in
> hierarchy
>
> The code was incorrectly assuming that root nodes had to be physical package
> nodes and vice versa. This is not always true, so update the check to simply
> require exactly one package node somewhere in the hierarchy.
>
> Signed-off-by: Jeshua Smith <jeshuas@nvidia.com <mailto:jeshuas@nvidia.com> <mailto:jeshuas@nvidia.com <mailto:jeshuas@nvidia.com>>>
> Reviewed-by: Pierre Gondois <pierre.gondois@arm.com <mailto:pierre.gondois@arm.com> <mailto:pierre.gondois@arm.com <mailto:pierre.gondois@arm.com>>>
> ---
> Note: This is a complete replacement for [PATCH] DynamicTablesPkg/SSDT:
> Remove incorrect root node check
>
> Version 2: added documentation for the PackageNodeSeen parameter
>
> .../SsdtCpuTopologyGenerator.c | 32 +++++++++++++------
> 1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git
> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpu
> TopologyGenerator.c
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpu
> TopologyGenerator.c
> index 9e3efb49e6..40ed10eae6 100644
> ---
> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpu
> TopologyGenerator.c
> +++
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCp
> +++ uTopologyGenerator.c
> @@ -1072,6 +1072,7 @@ CreateAmlProcessorContainer (
> @param [in] IsLeaf The ProcNode is a leaf.
> @param [in] NodeToken NodeToken of the ProcNode.
> @param [in] ParentNodeToken Parent NodeToken of the ProcNode.
> + @param [in] PackageNodeSeen A parent of the ProcNode has the physical
> package flag set.
>
> @retval EFI_SUCCESS Success.
> @retval EFI_INVALID_PARAMETER Invalid parameter.
> @@ -1083,23 +1084,24 @@ CheckProcNode (
> UINT32 NodeFlags,
> BOOLEAN IsLeaf,
> CM_OBJECT_TOKEN NodeToken,
> - CM_OBJECT_TOKEN ParentNodeToken
> + CM_OBJECT_TOKEN ParentNodeToken,
> + BOOLEAN PackageNodeSeen
> )
> {
> BOOLEAN InvalidFlags;
> BOOLEAN HasPhysicalPackageBit;
> - BOOLEAN IsTopLevelNode;
>
> HasPhysicalPackageBit = (NodeFlags &
> EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL) ==
> EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL;
> - IsTopLevelNode = (ParentNodeToken == CM_NULL_TOKEN);
>
> - // A top-level node is a Physical Package and conversely.
> - InvalidFlags = HasPhysicalPackageBit ^ IsTopLevelNode;
> + // Only one Physical Package flag is allowed in the hierarchy
> + InvalidFlags = HasPhysicalPackageBit && PackageNodeSeen;
>
> // Check Leaf specific flags.
> if (IsLeaf) {
> InvalidFlags |= ((NodeFlags & PPTT_LEAF_MASK) != PPTT_LEAF_MASK);
> + // Must have Physical Package flag somewhere in the hierarchy
> + InvalidFlags |= !(HasPhysicalPackageBit || PackageNodeSeen);
> } else {
> InvalidFlags |= ((NodeFlags & PPTT_LEAF_MASK) != 0);
> }
> @@ -1130,6 +1132,7 @@ CheckProcNode (
> node to.
> @param [in,out] ProcContainerIndex Pointer to the current processor
> container
> index to be used as UID.
> + @param [in] PackageNodeSeen A parent of the ProcNode has the
> physical package flag set.
>
> @retval EFI_SUCCESS Success.
> @retval EFI_INVALID_PARAMETER Invalid parameter.
> @@ -1143,7 +1146,8 @@ CreateAmlCpuTopologyTree (
> IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST
> CfgMgrProtocol,
> IN CM_OBJECT_TOKEN NodeToken,
> IN AML_NODE_HANDLE ParentNode,
> - IN OUT UINT32 *ProcContainerIndex
> + IN OUT UINT32 *ProcContainerIndex,
> + IN BOOLEAN PackageNodeSeen
> )
> {
> EFI_STATUS Status;
> @@ -1153,6 +1157,7 @@ CreateAmlCpuTopologyTree (
> AML_OBJECT_NODE_HANDLE ProcContainerNode;
> UINT32 Uid;
> UINT16 Name;
> + BOOLEAN HasPhysicalPackageBit;
>
> ASSERT (Generator != NULL);
> ASSERT (Generator->ProcNodeList != NULL); @@ -1175,7 +1180,8 @@
> CreateAmlCpuTopologyTree (
> Generator->ProcNodeList[Index].Flags,
> TRUE,
> Generator->ProcNodeList[Index].Token,
> - NodeToken
> + NodeToken,
> + PackageNodeSeen
> );
> if (EFI_ERROR (Status)) {
> ASSERT (0);
> @@ -1208,7 +1214,8 @@ CreateAmlCpuTopologyTree (
> Generator->ProcNodeList[Index].Flags,
> FALSE,
> Generator->ProcNodeList[Index].Token,
> - NodeToken
> + NodeToken,
> + PackageNodeSeen
> );
> if (EFI_ERROR (Status)) {
> ASSERT (0);
> @@ -1249,13 +1256,17 @@ CreateAmlCpuTopologyTree (
> ProcContainerName++;
> }
>
> + HasPhysicalPackageBit = (Generator->ProcNodeList[Index].Flags &
> EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL) ==
> + EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL;
> +
> // Recursively continue creating an AML tree.
> Status = CreateAmlCpuTopologyTree (
> Generator,
> CfgMgrProtocol,
> Generator->ProcNodeList[Index].Token,
> ProcContainerNode,
> - ProcContainerIndex
> + ProcContainerIndex,
> + (PackageNodeSeen || HasPhysicalPackageBit)
> );
> if (EFI_ERROR (Status)) {
> ASSERT (0);
> @@ -1311,7 +1322,8 @@ CreateTopologyFromProcHierarchy (
> CfgMgrProtocol,
> CM_NULL_TOKEN,
> ScopeNode,
> - &ProcContainerIndex
> + &ProcContainerIndex,
> + FALSE
> );
> if (EFI_ERROR (Status)) {
> ASSERT (0);
> --
> 2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116715): https://edk2.groups.io/g/devel/message/116715
Mute This Topic: https://groups.io/mt/104183075/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-03-13 14:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-05 19:00 [edk2-devel] [PATCH v2] DynamicTablesPkg/SSDT: Require Package node in hierarchy Jeshua Smith via groups.io
2024-03-12 16:11 ` Jeshua Smith via groups.io
2024-03-12 16:14 ` Sami Mujawar
2024-03-13 14:23 ` Sami Mujawar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox