Hello Jeff, Just 2 minors comments. Maybe Sami will have more. Otherwise, for the 3 patches: Reviewed-by: Pierre Gondois <pierre.gond...@arm.com>
Regards, Pierre On 9/20/22 00:01, Jeff Brasen 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 | 54 ++ .../Common/AmlLib/CodeGen/AmlCodeGen.c | 476 ++++++++++++++++++ 2 files changed, 530 insertions(+) diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h index 39968660f2..ebaccba811 100644 --- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h +++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
[snip]
+ +/** 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_4_GENERIC_ADDRESS_STRUCTURE *Register OPTIONAL, + IN UINT32 Integer, + IN AML_OBJECT_NODE_HANDLE PackageNode + ) +{ + EFI_STATUS Status; + AML_OBJECT_NODE_HANDLE IntegerNode; + + IntegerNode = NULL; +
I think that with IsNullGenericAddress(), the first (Register != NULL) is not necessary.
+ if ((Register != NULL) && !IsNullGenericAddress (Register)) { + Status = AmlAddRegisterToPackage (Register, PackageNode); + } else { + Status = AmlCodeGenInteger (Integer, &IntegerNode); + if (EFI_ERROR (Status)) { + ASSERT_EFI_ERROR (Status); + return Status; + } + + Status = AmlVarListAddTail ( + (AML_NODE_HANDLE)PackageNode, + (AML_NODE_HANDLE)IntegerNode + ); + } + + if (EFI_ERROR (Status)) { + ASSERT_EFI_ERROR (Status); + 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] CpcInfo CpcInfo object + @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 ( + IN AML_CPC_INFO *CpcInfo, + 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; + UINT32 NumberOfEntries; + + if ((CpcInfo == NULL) || + ((ParentNode == NULL) && (NewCpcNode == NULL))) + { + ASSERT (0); + return EFI_INVALID_PARAMETER; + } + + // Revision 3 per ACPI 6.4 specification + if (CpcInfo->Revision == 3) { + // NumEntries 23 per ACPI 6.4 specification + NumberOfEntries = 23; + } else { + ASSERT (0); + return EFI_INVALID_PARAMETER; + } +
It seems CpcInfo is already checked earlier. It seems the checks below could be part of the other checks above (where ParentNode is checked).
+ if ((CpcInfo == NULL) || + (IsNullGenericAddress (&CpcInfo->HighestPerformanceBuffer) && + (CpcInfo->HighestPerformanceInteger == 0)) || + (IsNullGenericAddress (&CpcInfo->NominalPerformanceBuffer) && + (CpcInfo->NominalPerformanceInteger == 0)) || + (IsNullGenericAddress (&CpcInfo->LowestNonlinearPerformanceBuffer) && + (CpcInfo->LowestNonlinearPerformanceInteger == 0)) || + (IsNullGenericAddress (&CpcInfo->LowestPerformanceBuffer) && + (CpcInfo->LowestPerformanceInteger == 0)) || + IsNullGenericAddress (&CpcInfo->DesiredPerformanceRegister) || + IsNullGenericAddress (&CpcInfo->ReferencePerformanceCounterRegister) || + IsNullGenericAddress (&CpcInfo->DeliveredPerformanceCounterRegister) || + IsNullGenericAddress (&CpcInfo->PerformanceLimitedRegister)) + { + ASSERT (0); + return EFI_INVALID_PARAMETER; + } + + CpcPackage = NULL; + + Status = AmlCodeGenNamePackage ("_CPC", NULL, &CpcNode); + if (EFI_ERROR (Status)) { + ASSERT_EFI_ERROR (Status); + return Status; + } +
[snip] -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#94001): https://edk2.groups.io/g/devel/message/94001 Mute This Topic: https://groups.io/mt/93792018/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-