Hello Jeff,
Please find some remarks inline:
On 9/7/22 17:15, Jeff Brasen via groups.io wrote:
_CPC entries can describe CPU performance information.
The object is described in ACPI 6.4 s8.4.7.1.
"_CPC (Continuous Performance Control)".
Add AmlCreateCpcNode() helper function to add _CPC entries to an
existing CPU object.
Signed-off-by: Jeff Brasen <jbra...@nvidia.com>
---
.../Include/Library/AmlLib/AmlLib.h | 155 +++++
.../Common/AmlLib/CodeGen/AmlCodeGen.c | 549 ++++++++++++++++++
2 files changed, 704 insertions(+)
diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
index 39968660f2..65f6cd7583 100644
[snip]
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index 5fb39d077b..dfc3015baf 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -2850,3 +2850,552 @@ error_handler:
return Status;
}
+
+/** Adds a register to the package
+
+ @ingroup CodeGenApis
+
+ @param [in] Register If provided, register that will be added to
package.
+ otherwise NULL register will be added
+ @param [in] PackageNode Package to add value to
+
+ @retval EFI_SUCCESS The function completed successfully.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+ @retval EFI_OUT_OF_RESOURCES Failed to allocate memory.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+AmlAddRegisterToPackage (
+ IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE *Register OPTIONAL,
+ IN AML_OBJECT_NODE_HANDLE PackageNode
+ )
+{
+ EFI_STATUS Status;
+ AML_DATA_NODE_HANDLE RdNode;
+ AML_OBJECT_NODE_HANDLE ResourceTemplateNode;
+
+ RdNode = NULL;
+ ResourceTemplateNode = NULL;
I don't think it would be necessary to set ResourceTemplateNode
if the 'goto error_handler;' just above was replaced by a 'return Status;'.
+
+ Status = AmlCodeGenResourceTemplate (&ResourceTemplateNode);
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
I know this is not consistent in the file, but would it be possible
to use:
ASSERT_EFI_ERROR (Status);
when a Status is available ? This comment can be applied to other places.
For this specifc error handling, I think we can just return without
going to the error hangler.
+ goto error_handler;
+ }
+
+ if (Register != NULL) {
+ Status = AmlCodeGenRdRegister (
+ Register->AddressSpaceId,
+ Register->RegisterBitWidth,
+ Register->RegisterBitOffset,
+ Register->Address,
+ Register->AccessSize,
+ NULL,
+ &RdNode
+ );
+ } else {
+ Status = AmlCodeGenRdRegister (
+ EFI_ACPI_6_4_SYSTEM_MEMORY,
+ 0,
+ 0,
+ 0,
+ 0,
+ NULL,
+ &RdNode
+ );
+ }
+
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler;
+ }
+
+ Status = AmlAppendRdNode (ResourceTemplateNode, RdNode);
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler;
+ }
+
+ RdNode = NULL;
+
+ Status = AmlVarListAddTail (
+ (AML_NODE_HANDLE)PackageNode,
+ (AML_NODE_HANDLE)ResourceTemplateNode
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler;
+ }
+
+ ResourceTemplateNode = NULL;
I don't think it's necessary.
+
+ return Status;
+
+error_handler:
+ if (RdNode != NULL) {
+ AmlDeleteTree ((AML_NODE_HANDLE)RdNode);
+ }
+
+ if (ResourceTemplateNode != NULL) {
+ AmlDeleteTree ((AML_NODE_HANDLE)ResourceTemplateNode);
+ }
+
+ return Status;
+}
+
+/** Adds an integer or register to the package
+
+ @ingroup CodeGenApis
+
+ @param [in] Register If provided, register that will be added to package
+ @param [in] Integer If Register is NULL, integer that will be added to
the package
+ @param [in] PackageNode Package to add value to
+
+ @retval EFI_SUCCESS The function completed successfully.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+ @retval EFI_OUT_OF_RESOURCES Failed to allocate memory.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+AmlAddRegisterOrIntegerToPackage (
+ IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE *Register OPTIONAL,
+ IN UINT32 Integer,
+ IN AML_OBJECT_NODE_HANDLE PackageNode
+ )
+{
+ EFI_STATUS Status;
+ AML_OBJECT_NODE_HANDLE IntegerNode;
+
+ IntegerNode = NULL;
+
+ if (Register != NULL) {
+ Status = AmlAddRegisterToPackage (Register, PackageNode);
+ } else {
+ Status = AmlCodeGenInteger (Integer, &IntegerNode);
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler;
I think this could just be a 'return Status;'
+ }
+
+ Status = AmlVarListAddTail (
+ (AML_NODE_HANDLE)PackageNode,
+ (AML_NODE_HANDLE)IntegerNode
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler;
+ }
This should be out of the 'else' statement to assert if
AmlAddRegisterToPackage() fails, as:
if () {
Status = AmlAddRegisterToPackage ();
} else {
...
Status = AmlVarListAddTail ();
}
if (EFI_ERROR (Status)) {
}
+
+ IntegerNode = NULL;
I don't think it's necessary.
+ }
+
+ return Status;
+
+error_handler:
+ if (IntegerNode != NULL) {
+ AmlDeleteTree ((AML_NODE_HANDLE)IntegerNode);
+ }
+
+ return Status;
+}
+
+/** Create a _CPC node.
+
+ Creates and optionally adds the following node
+ Name(_CPC, Package()
+ {
+ NumEntries, // Integer
+ Revision, // Integer
+ HighestPerformance, // Integer or Buffer (Resource
Descriptor)
+ NominalPerformance, // Integer or Buffer (Resource
Descriptor)
+ LowestNonlinearPerformance, // Integer or Buffer (Resource
Descriptor)
+ LowestPerformance, // Integer or Buffer (Resource
Descriptor)
+ GuaranteedPerformanceRegister, // Buffer (Resource Descriptor)
+ DesiredPerformanceRegister , // Buffer (Resource Descriptor)
+ MinimumPerformanceRegister , // Buffer (Resource Descriptor)
+ MaximumPerformanceRegister , // Buffer (Resource Descriptor)
+ PerformanceReductionToleranceRegister, // Buffer (Resource Descriptor)
+ TimeWindowRegister, // Buffer (Resource Descriptor)
+ CounterWraparoundTime, // Integer or Buffer (Resource
Descriptor)
+ ReferencePerformanceCounterRegister, // Buffer (Resource Descriptor)
+ DeliveredPerformanceCounterRegister, // Buffer (Resource Descriptor)
+ PerformanceLimitedRegister, // Buffer (Resource Descriptor)
+ CPPCEnableRegister // Buffer (Resource Descriptor)
+ AutonomousSelectionEnable, // Integer or Buffer (Resource
Descriptor)
+ AutonomousActivityWindowRegister, // Buffer (Resource Descriptor)
+ EnergyPerformancePreferenceRegister, // Buffer (Resource Descriptor)
+ ReferencePerformance // Integer or Buffer (Resource
Descriptor)
+ LowestFrequency, // Integer or Buffer (Resource
Descriptor)
+ NominalFrequency // Integer or Buffer (Resource
Descriptor)
+ })
+
+ If resource buffer is NULL then integer will be used.
+
+ Cf. ACPI 6.4, s8.4.7.1 _CPC (Continuous Performance Control)
+
+ @ingroup CodeGenApis
+
+ @param [in] HighestPerformanceBuffer If provided, buffer that
indicates the highest level
+ of performance the
processor.
+ @param [in] HighestPerformanceInteger Indicates the highest
level of performance the processor,
+ used if buffer is NULL.
+ @param [in] NominalPerformanceBuffer If provided buffer that
indicates the highest sustained
+ performance level of the
processor.
+ @param [in] NominalPerformanceInteger Indicates the highest
sustained performance level
+ of the processor, used
if buffer is NULL.
+ @param [in] LowestNonlinearPerformanceBuffer If provided, buffer that
indicates the lowest performance level
+ of the processor with
non-linear power savings.
+ @param [in] LowestNonlinearPerformanceInteger Indicates the lowest
performance level of the processor with
+ non-linear power
savings, used if buffer is NULL.
+ @param [in] LowestPerformanceBuffer If provided, buffer that
indicates the
+ lowest performance level
of the processor.
+ @param [in] LowestPerformanceInteger Indicates the lowest
performance level of the processor,
+ used if buffer is NULL.
+ @param [in] GuaranteedPerformanceRegister If provided, Guaranteed
Performance Register Buffer.
+ @param [in] DesiredPerformanceRegister If provided, Desired
Performance Register Buffer.
+ @param [in] MinimumPerformanceRegister If provided, Minimum
Performance Register Buffer.
+ @param [in] MaximumPerformanceRegister If provided, Maximum
Performance Register Buffer.
+ @param [in] PerformanceReductionToleranceRegister If provided, Performance
Reduction Tolerance Register.
+ @param [in] TimeWindowRegister If provided, Time Window
Register.
+ @param [in] CounterWraparoundTimeBuffer If provided, Counter
Wraparound Time buffer.
+ @param [in] CounterWraparoundTimeInteger Counter Wraparound Time,
used if buffer is NULL.
+ @param [in] ReferencePerformanceCounterRegister Reference Performance
Counter Register.
+ @param [in] DeliveredPerformanceCounterRegister Delivered Performance
Counter Register.
+ @param [in] PerformanceLimitedRegister Performance Limited
Register.
+ @param [in] CPPCEnableRegister If provided, CPPC
EnableRegister.
+ @param [in] AutonomousSelectionEnableBuffer If provided, Autonomous
Selection Enable buffer.
+ @param [in] AutonomousSelectionEnableInteger Autonomous Selection
Enable, used if buffer is NULL.
+ @param [in] AutonomousActivityWindowRegister If provided,
AutonomousActivity-WindowRegister.
+ @param [in] EnergyPerformancePreferenceRegister If provided,
EnergyPerformance-PreferenceRegister.
+ @param [in] ReferencePerformanceBuffer If provided, Reference
Performance buffer.
+ @param [in] ReferencePerformanceInteger Reference Performance,
used if buffer is NULL.
+ @param [in] LowestFrequencyBuffer If provided, Lowest
Frequency buffer.
+ @param [in] LowestFrequencyInteger Lowest Frequency, used
if buffer is NULL.
+ @param [in] NominalFrequencyBuffer If provided,
NominalFrequencyBuffer buffer.
+ @param [in] NominalFrequencyInteger NominalFrequencyBuffer,
used if buffer is NULL.
+ @param [in] ParentNode If provided, set
ParentNode as the parent
+ of the node created.
+ @param [out] NewCpcNode If success and provided,
contains the created node.
+
+ @retval EFI_SUCCESS The function completed successfully.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+ @retval EFI_OUT_OF_RESOURCES Failed to allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+AmlCreateCpcNode (
+ /// Indicates the highest level of performance the processor
+ /// is theoretically capable of achieving.
+ IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE *HighestPerformanceBuffer
OPTIONAL,
+ IN UINT32 HighestPerformanceInteger,
+ /// Indicates the highest sustained performance level of the processor.
+ IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE *NominalPerformanceBuffer
OPTIONAL,
+ IN UINT32 NominalPerformanceInteger,
+ /// Indicates the lowest performance level of the processor with non-linear
power savings.
+ IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE *LowestNonlinearPerformanceBuffer
OPTIONAL,
+ IN UINT32 LowestNonlinearPerformanceInteger,
+ /// Indicates the lowest performance level of the processor..
+ IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE *LowestPerformanceBuffer OPTIONAL,
+ IN UINT32 LowestPerformanceInteger,
+ /// Guaranteed Performance Register Buffer.
+ /// Optional
+ IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE *GuaranteedPerformanceRegister
OPTIONAL,
+ /// Desired Performance Register Buffer.
+ /// Optional
+ IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE *DesiredPerformanceRegister
OPTIONAL,
+ /// Minimum Performance Register Buffer.
+ /// Optional
+ IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE *MinimumPerformanceRegister
OPTIONAL,
+ /// Maximum Performance Register Buffer.
+ /// Optional
+ IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE *MaximumPerformanceRegister
OPTIONAL,
+ /// Performance Reduction Tolerance Register.
+ /// Optional
+ IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE
*PerformanceReductionToleranceRegister OPTIONAL,
+ /// Time Window Register.
+ /// Optional
+ IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE *TimeWindowRegister OPTIONAL,
+ /// Counter Wraparound Time
+ /// Optional
+ IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE *CounterWraparoundTimeBuffer
OPTIONAL,
+ IN UINT32 CounterWraparoundTimeInteger,
+ /// Reference Performance Counter Register
+ IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE
*ReferencePerformanceCounterRegister,
+ /// Delivered Performance Counter Register
+ IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE
*DeliveredPerformanceCounterRegister,
+ /// Performance Limited Register
+ IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE *PerformanceLimitedRegister,
+ /// CPPC EnableRegister
+ /// Optional
+ IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE *CPPCEnableRegister OPTIONAL,
+ /// Autonomous Selection Enable
+ /// Optional
+ IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE *AutonomousSelectionEnableBuffer
OPTIONAL,
+ IN UINT32 AutonomousSelectionEnableInteger,
+ /// AutonomousActivity-WindowRegister
+ /// Optional
+ IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE *AutonomousActivityWindowRegister
OPTIONAL,
+ /// EnergyPerformance-PreferenceRegister
+ /// Optional
+ IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE
*EnergyPerformancePreferenceRegister OPTIONAL,
+ /// Reference Performance
+ /// Optional
+ IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE *ReferencePerformanceBuffer
OPTIONAL,
+ IN UINT32 ReferencePerformanceInteger,
+ /// Lowest Frequency
+ /// Optional
+ IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE *LowestFrequencyBuffer OPTIONAL,
+ IN UINT32 LowestFrequencyInteger,
+ /// Nominal Frequency
+ /// Optional
+ IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE *NominalFrequencyBuffer OPTIONAL,
+ IN UINT32 NominalFrequencyInteger,
+ IN AML_NODE_HANDLE ParentNode OPTIONAL,
+ OUT AML_OBJECT_NODE_HANDLE *NewCpcNode OPTIONAL
+ )
+{
+ EFI_STATUS Status;
+ AML_OBJECT_NODE_HANDLE CpcNode;
+ AML_OBJECT_NODE_HANDLE CpcPackage;
+
+ if ((ReferencePerformanceCounterRegister == NULL) ||
+ (PerformanceLimitedRegister == NULL) ||
+ ((ParentNode == NULL) && (NewCpcNode == NULL)))
Aren't other fields mandatory like the Nominal Performance ?
I believe there should be other checks like:
((NominalPerformanceBuffer == NULL) && (NominalFrequencyInteger == 0))
+ {
+ ASSERT (0);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ CpcNode = NULL;
I think CpcNode shouldn't need to be set if the above call to
AmlCodeGenNamePackage() was handling the error with a
'return Status' instead of a 'goto error_handler'
+ CpcPackage = NULL;
+
+ Status = AmlCodeGenNamePackage ("_CPC", NULL, &CpcNode);
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler;
+ }
+
+ // Get the Package object node of the _CPC node,
+ // which is the 2nd fixed argument (i.e. index 1).
+ CpcPackage = (AML_OBJECT_NODE_HANDLE)AmlGetFixedArgument (
+ CpcNode,
+ EAmlParseIndexTerm1
+ );
+ if ((CpcPackage == NULL) ||
+ (AmlGetNodeType ((AML_NODE_HANDLE)CpcPackage) != EAmlNodeObject) ||
+ (!AmlNodeHasOpCode (CpcPackage, AML_PACKAGE_OP, 0)))
+ {
+ ASSERT (0);
+ goto error_handler;
+ }
+
+ // NumEntries 23 per ACPI 6.4 specification
+ Status = AmlAddRegisterOrIntegerToPackage (
+ NULL,
+ 23,
+ CpcPackage
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler;
+ }
+
+ // Revision 3 per ACPI 6.4 specification
+ Status = AmlAddRegisterOrIntegerToPackage (
+ NULL,
+ 3,
+ CpcPackage
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler;
+ }
If a Revision field is added to the CM_ARM_CPC_INFO object,
the generation of NumEntries and Revision will have to be modified.
+
+ Status = AmlAddRegisterOrIntegerToPackage (
+ HighestPerformanceBuffer,
+ HighestPerformanceInteger,
+ CpcPackage
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler;
+ }
+
[snip]
+
+ if (ParentNode != NULL) {
+ Status = AmlVarListAddTail (
+ (AML_NODE_HANDLE)ParentNode,
+ (AML_NODE_HANDLE)CpcNode
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler;
+ }
+ }
+
+ if (NewCpcNode != NULL) {
+ *NewCpcNode = CpcNode;
+ }
I think the handling of ParentNode and NewCpcNode can be done
with LinkNode()
+
+ return Status;
+
+error_handler:
+ if (CpcNode != NULL) {
+ AmlDeleteTree ((AML_NODE_HANDLE)CpcNode);
+ }
+
+ return Status;
+}
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#93647): https://edk2.groups.io/g/devel/message/93647
Mute This Topic: https://groups.io/mt/93527136/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
