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]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to