public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/3] DynamicTablesPkg: _CPC support
@ 2022-09-07 15:15 Jeff Brasen
  2022-09-07 15:15 ` [PATCH 1/3] DynamicTablesPkg: Add CM_ARM_CPC_INFO object Jeff Brasen
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Jeff Brasen @ 2022-09-07 15:15 UTC (permalink / raw)
  To: devel; +Cc: ardb+tianocore, Sami.Mujawar, Alexei.Fedorov, Jeff Brasen

Add generator for creating the _CPC object for CPU nodes.

If viewing this review by a pull request is helpful one exists here:
https://github.com/NVIDIA/edk2/pull/12

Jeff Brasen (3):
  DynamicTablesPkg: Add CM_ARM_CPC_INFO object
  DynamicTablesPkg: AML Code generation to add _CPC entries
  DynamicTablesPkg: SSDT CPU _CPC generator

 .../Include/ArmNameSpaceObjects.h             | 146 ++++-
 .../Include/Library/AmlLib/AmlLib.h           | 155 +++++
 .../SsdtCpuTopologyGenerator.c                | 211 ++++++-
 .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 549 ++++++++++++++++++
 .../ConfigurationManagerObjectParser.c        |  79 +++
 5 files changed, 1118 insertions(+), 22 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] DynamicTablesPkg: Add CM_ARM_CPC_INFO object
  2022-09-07 15:15 [PATCH 0/3] DynamicTablesPkg: _CPC support Jeff Brasen
@ 2022-09-07 15:15 ` Jeff Brasen
  2022-09-12 10:34   ` [edk2-devel] " PierreGondois
  2022-09-07 15:15 ` [PATCH 2/3] DynamicTablesPkg: AML Code generation to add _CPC entries Jeff Brasen
  2022-09-07 15:15 ` [PATCH 3/3] DynamicTablesPkg: SSDT CPU _CPC generator Jeff Brasen
  2 siblings, 1 reply; 7+ messages in thread
From: Jeff Brasen @ 2022-09-07 15:15 UTC (permalink / raw)
  To: devel; +Cc: ardb+tianocore, Sami.Mujawar, Alexei.Fedorov, Jeff Brasen

Introduce the CM_ARM_CPC_INFO CmObj in the ArmNameSpaceObjects.
This allows to describe CPC information, as described in ACPI 6.4,
s8.4.7.1 "_CPC (Continuous Performance Control)".

Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
---
 .../Include/ArmNameSpaceObjects.h             | 146 ++++++++++++++++--
 .../ConfigurationManagerObjectParser.c        |  79 ++++++++++
 2 files changed, 208 insertions(+), 17 deletions(-)

diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
index 102e0f96be..4d3f9ae534 100644
--- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
@@ -63,6 +63,7 @@ typedef enum ArmObjectID {
   EArmObjPciInterruptMapInfo,          ///< 39 - Pci Interrupt Map Info
   EArmObjRmr,                          ///< 40 - Reserved Memory Range Node
   EArmObjMemoryRangeDescriptor,        ///< 41 - Memory Range Descriptor
+  EArmObjCpcInfo,                      ///< 42 - Continuous Performance Control Info
   EArmObjMax
 } EARM_OBJECT_ID;
 
@@ -97,99 +98,105 @@ typedef struct CmArmPowerManagementProfileInfo {
 */
 typedef struct CmArmGicCInfo {
   /// The GIC CPU Interface number.
-  UINT32    CPUInterfaceNumber;
+  UINT32             CPUInterfaceNumber;
 
   /** The ACPI Processor UID. This must match the
       _UID of the CPU Device object information described
       in the DSDT/SSDT for the CPU.
   */
-  UINT32    AcpiProcessorUid;
+  UINT32             AcpiProcessorUid;
 
   /** The flags field as described by the GICC structure
       in the ACPI Specification.
   */
-  UINT32    Flags;
+  UINT32             Flags;
 
   /** The parking protocol version field as described by
     the GICC structure in the ACPI Specification.
   */
-  UINT32    ParkingProtocolVersion;
+  UINT32             ParkingProtocolVersion;
 
   /** The Performance Interrupt field as described by
       the GICC structure in the ACPI Specification.
   */
-  UINT32    PerformanceInterruptGsiv;
+  UINT32             PerformanceInterruptGsiv;
 
   /** The CPU Parked address field as described by
       the GICC structure in the ACPI Specification.
   */
-  UINT64    ParkedAddress;
+  UINT64             ParkedAddress;
 
   /** The base address for the GIC CPU Interface
       as described by the GICC structure in the
       ACPI Specification.
   */
-  UINT64    PhysicalBaseAddress;
+  UINT64             PhysicalBaseAddress;
 
   /** The base address for GICV interface
       as described by the GICC structure in the
       ACPI Specification.
   */
-  UINT64    GICV;
+  UINT64             GICV;
 
   /** The base address for GICH interface
       as described by the GICC structure in the
       ACPI Specification.
   */
-  UINT64    GICH;
+  UINT64             GICH;
 
   /** The GICV maintenance interrupt
       as described by the GICC structure in the
       ACPI Specification.
   */
-  UINT32    VGICMaintenanceInterrupt;
+  UINT32             VGICMaintenanceInterrupt;
 
   /** The base address for GICR interface
       as described by the GICC structure in the
       ACPI Specification.
   */
-  UINT64    GICRBaseAddress;
+  UINT64             GICRBaseAddress;
 
   /** The MPIDR for the CPU
       as described by the GICC structure in the
       ACPI Specification.
   */
-  UINT64    MPIDR;
+  UINT64             MPIDR;
 
   /** The Processor Power Efficiency class
       as described by the GICC structure in the
       ACPI Specification.
   */
-  UINT8     ProcessorPowerEfficiencyClass;
+  UINT8              ProcessorPowerEfficiencyClass;
 
   /** Statistical Profiling Extension buffer overflow GSIV. Zero if
       unsupported by this processor. This field was introduced in
       ACPI 6.3 (MADT revision 5) and is therefore ignored when
       generating MADT revision 4 or lower.
   */
-  UINT16    SpeOverflowInterrupt;
+  UINT16             SpeOverflowInterrupt;
 
   /** The proximity domain to which the logical processor belongs.
       This field is used to populate the GICC affinity structure
       in the SRAT table.
   */
-  UINT32    ProximityDomain;
+  UINT32             ProximityDomain;
 
   /** The clock domain to which the logical processor belongs.
       This field is used to populate the GICC affinity structure
       in the SRAT table.
   */
-  UINT32    ClockDomain;
+  UINT32             ClockDomain;
 
   /** The GICC Affinity flags field as described by the GICC Affinity structure
       in the SRAT table.
   */
-  UINT32    AffinityFlags;
+  UINT32             AffinityFlags;
+
+  /** Optional field: Reference Token for the Cpc info of this processor.
+      Token identifying a CM_ARM_OBJ_REF structure, itself referencing
+      CM_ARM_CPC_INFO objects.
+  */
+  CM_OBJECT_TOKEN    CpcToken;
 } CM_ARM_GICC_INFO;
 
 /** A structure that describes the
@@ -1070,6 +1077,111 @@ typedef struct CmArmRmrDescriptor {
   UINT64    Length;
 } CM_ARM_MEMORY_RANGE_DESCRIPTOR;
 
+/** A structure that describes the Cpc information.
+
+  Continuous Performance Control is described in DSDT/SSDT and associated
+  to cpus/clusters in the cpu topology.
+
+  Unsupported Optional registers should be encoded with NULL resource
+  Register {(SystemMemory, 0, 0, 0, 0)}
+
+  For values that support Integer or Buffer, integer will be used
+  if buffer is NULL resource.
+  If resource is not NULL then Integer must be 0
+
+  Cf. ACPI 6.4, s8.4.7.1 _CPC (Continuous Performance Control)
+
+  ID: EArmObjCpcInfo
+*/
+typedef struct CmArmCpcInfo {
+  /// Indicates the highest level of performance the processor
+  /// is theoretically capable of achieving.
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE    HighestPerformanceBuffer;
+  UINT32                                    HighestPerformanceInteger;
+
+  /// Indicates the highest sustained performance level of the processor.
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE    NominalPerformanceBuffer;
+  UINT32                                    NominalPerformanceInteger;
+
+  /// Indicates the lowest performance level of the processor with non-linear power savings.
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE    LowestNonlinearPerformanceBuffer;
+  UINT32                                    LowestNonlinearPerformanceInteger;
+
+  /// Indicates the lowest performance level of the processor..
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE    LowestPerformanceBuffer;
+  UINT32                                    LowestPerformanceInteger;
+
+  /// Guaranteed Performance Register Buffer.
+  /// Optional
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE    GuaranteedPerformanceRegister;
+
+  /// Desired Performance Register Buffer.
+  /// Optional
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE    DesiredPerformanceRegister;
+
+  /// Minimum Performance Register Buffer.
+  /// Optional
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE    MinimumPerformanceRegister;
+
+  /// Maximum Performance Register Buffer.
+  /// Optional
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE    MaximumPerformanceRegister;
+
+  /// Performance Reduction Tolerance Register.
+  /// Optional
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE    PerformanceReductionToleranceRegister;
+
+  /// Time Window Register.
+  /// Optional
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE    TimeWindowRegister;
+
+  /// Counter Wraparound Time
+  /// Optional
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE    CounterWraparoundTimeBuffer;
+  UINT32                                    CounterWraparoundTimeInteger;
+
+  /// Reference Performance Counter Register
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE    ReferencePerformanceCounterRegister;
+
+  /// Delivered Performance Counter Register
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE    DeliveredPerformanceCounterRegister;
+
+  /// Performance Limited Register
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE    PerformanceLimitedRegister;
+
+  /// CPPC EnableRegister
+  /// Optional
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE    CPPCEnableRegister;
+
+  /// Autonomous Selection Enable
+  /// Optional
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE    AutonomousSelectionEnableBuffer;
+  UINT32                                    AutonomousSelectionEnableInteger;
+
+  /// AutonomousActivity-WindowRegister
+  /// Optional
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE    AutonomousActivityWindowRegister;
+
+  /// EnergyPerformance-PreferenceRegister
+  /// Optional
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE    EnergyPerformancePreferenceRegister;
+
+  /// Reference Performance
+  /// Optional
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE    ReferencePerformanceBuffer;
+  UINT32                                    ReferencePerformanceInteger;
+
+  /// Lowest Frequency
+  /// Optional
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE    LowestFrequencyBuffer;
+  UINT32                                    LowestFrequencyInteger;
+
+  /// Nominal Frequency
+  /// Optional
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE    NominalFrequencyBuffer;
+  UINT32                                    NominalFrequencyInteger;
+} CM_ARM_CPC_INFO;
+
 #pragma pack()
 
 #endif // ARM_NAMESPACE_OBJECTS_H_
diff --git a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
index c1b21d24a4..e2c608443b 100644
--- a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
+++ b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
@@ -423,6 +423,83 @@ STATIC CONST CM_OBJ_PARSER  CmPciInterruptMapInfoParser[] = {
     ARRAY_SIZE (CmArmGenericInterruptParser) },
 };
 
+/** A parser for EArmObjCpcInfo.
+*/
+STATIC CONST CM_OBJ_PARSER  CmArmCpcInfoParser[] = {
+  { "HighestPerformanceBuffer",              sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
+    NULL, NULL, AcpiGenericAddressParser,
+    ARRAY_SIZE (AcpiGenericAddressParser) },
+  { "HighestPerformanceInteger",             4,                                              "0x%llx",  NULL },
+  { "NominalPerformanceBuffer",              sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
+    NULL, NULL, AcpiGenericAddressParser,
+    ARRAY_SIZE (AcpiGenericAddressParser) },
+  { "NominalPerformanceInteger",             4,                                              "0x%llx",  NULL },
+  { "LowestNonlinearPerformanceBuffer",      sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
+    NULL, NULL, AcpiGenericAddressParser,
+    ARRAY_SIZE (AcpiGenericAddressParser) },
+  { "LowestNonlinearPerformanceInteger",     4,                                              "0x%llx",  NULL },
+  { "LowestPerformanceBuffer",               sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
+    NULL, NULL, AcpiGenericAddressParser,
+    ARRAY_SIZE (AcpiGenericAddressParser) },
+  { "LowestPerformanceInteger",              4,                                              "0x%llx",  NULL },
+  { "GuaranteedPerformanceRegister",         sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
+    NULL, NULL, AcpiGenericAddressParser,
+    ARRAY_SIZE (AcpiGenericAddressParser) },
+  { "DesiredPerformanceRegister",            sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
+    NULL, NULL, AcpiGenericAddressParser,
+    ARRAY_SIZE (AcpiGenericAddressParser) },
+  { "MinimumPerformanceRegister",            sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
+    NULL, NULL, AcpiGenericAddressParser,
+    ARRAY_SIZE (AcpiGenericAddressParser) },
+  { "MaximumPerformanceRegister",            sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
+    NULL, NULL, AcpiGenericAddressParser,
+    ARRAY_SIZE (AcpiGenericAddressParser) },
+  { "PerformanceReductionToleranceRegister", sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
+    NULL, NULL, AcpiGenericAddressParser,
+    ARRAY_SIZE (AcpiGenericAddressParser) },
+  { "TimeWindowRegister",                    sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
+    NULL, NULL, AcpiGenericAddressParser,
+    ARRAY_SIZE (AcpiGenericAddressParser) },
+  { "CounterWraparoundTimeBuffer",           sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
+    NULL, NULL, AcpiGenericAddressParser,
+    ARRAY_SIZE (AcpiGenericAddressParser) },
+  { "CounterWraparoundTimeInteger",          4,                                              "0x%llx",  NULL },
+  { "ReferencePerformanceCounterRegister",   sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
+    NULL, NULL, AcpiGenericAddressParser,
+    ARRAY_SIZE (AcpiGenericAddressParser) },
+  { "DeliveredPerformanceCounterRegister",   sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
+    NULL, NULL, AcpiGenericAddressParser,
+    ARRAY_SIZE (AcpiGenericAddressParser) },
+  { "PerformanceLimitedRegister",            sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
+    NULL, NULL, AcpiGenericAddressParser,
+    ARRAY_SIZE (AcpiGenericAddressParser) },
+  { "CPPCEnableRegister",                    sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
+    NULL, NULL, AcpiGenericAddressParser,
+    ARRAY_SIZE (AcpiGenericAddressParser) },
+  { "AutonomousSelectionEnableBuffer",       sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
+    NULL, NULL, AcpiGenericAddressParser,
+    ARRAY_SIZE (AcpiGenericAddressParser) },
+  { "AutonomousSelectionEnableInteger",      4,                                              "0x%llx",  NULL },
+  { "AutonomousActivityWindowRegister",      sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
+    NULL, NULL, AcpiGenericAddressParser,
+    ARRAY_SIZE (AcpiGenericAddressParser) },
+  { "EnergyPerformancePreferenceRegister",   sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
+    NULL, NULL, AcpiGenericAddressParser,
+    ARRAY_SIZE (AcpiGenericAddressParser) },
+  { "ReferencePerformanceBuffer",            sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
+    NULL, NULL, AcpiGenericAddressParser,
+    ARRAY_SIZE (AcpiGenericAddressParser) },
+  { "ReferencePerformanceInteger",           4,                                              "0x%llx",  NULL },
+  { "LowestFrequencyBuffer",                 sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
+    NULL, NULL, AcpiGenericAddressParser,
+    ARRAY_SIZE (AcpiGenericAddressParser) },
+  { "LowestFrequencyInteger",                4,                                              "0x%llx",  NULL },
+  { "NominalFrequencyBuffer",                sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
+    NULL, NULL, AcpiGenericAddressParser,
+    ARRAY_SIZE (AcpiGenericAddressParser) },
+  { "NominalFrequencyInteger",               4,                                              "0x%llx",  NULL },
+};
+
 /** A parser for Arm namespace objects.
 */
 STATIC CONST CM_OBJ_PARSER_ARRAY  ArmNamespaceObjectParser[] = {
@@ -501,6 +578,8 @@ STATIC CONST CM_OBJ_PARSER_ARRAY  ArmNamespaceObjectParser[] = {
     ARRAY_SIZE (CmArmPciAddressMapInfoParser) },
   { "EArmObjPciInterruptMapInfo",          CmPciInterruptMapInfoParser,
     ARRAY_SIZE (CmPciInterruptMapInfoParser) },
+  { "EArmObjCpcInfo",                      CmArmCpcInfoParser,
+    ARRAY_SIZE (CmArmCpcInfoParser) },
   { "EArmObjMax",                          NULL,                                  0                                },
 };
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/3] DynamicTablesPkg: AML Code generation to add _CPC entries
  2022-09-07 15:15 [PATCH 0/3] DynamicTablesPkg: _CPC support Jeff Brasen
  2022-09-07 15:15 ` [PATCH 1/3] DynamicTablesPkg: Add CM_ARM_CPC_INFO object Jeff Brasen
@ 2022-09-07 15:15 ` Jeff Brasen
  2022-09-12 10:36   ` [edk2-devel] " PierreGondois
  2022-09-07 15:15 ` [PATCH 3/3] DynamicTablesPkg: SSDT CPU _CPC generator Jeff Brasen
  2 siblings, 1 reply; 7+ messages in thread
From: Jeff Brasen @ 2022-09-07 15:15 UTC (permalink / raw)
  To: devel; +Cc: ardb+tianocore, Sami.Mujawar, Alexei.Fedorov, Jeff Brasen

_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 <jbrasen@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
--- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
+++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
@@ -1336,6 +1336,161 @@ AmlAddNameIntegerPackage (
   IN AML_OBJECT_NODE_HANDLE  PackageNode
   );
 
+/** 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
+  );
+
 // DEPRECATED APIS
 #ifndef DISABLE_NEW_DEPRECATED_INTERFACES
 
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;
+
+  Status = AmlCodeGenResourceTemplate (&ResourceTemplateNode);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    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;
+
+  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;
+    }
+
+    Status = AmlVarListAddTail (
+               (AML_NODE_HANDLE)PackageNode,
+               (AML_NODE_HANDLE)IntegerNode
+               );
+    if (EFI_ERROR (Status)) {
+      ASSERT (0);
+      goto error_handler;
+    }
+
+    IntegerNode = NULL;
+  }
+
+  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)))
+  {
+    ASSERT (0);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  CpcNode    = NULL;
+  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;
+  }
+
+  Status = AmlAddRegisterOrIntegerToPackage (
+             HighestPerformanceBuffer,
+             HighestPerformanceInteger,
+             CpcPackage
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlAddRegisterOrIntegerToPackage (
+             NominalPerformanceBuffer,
+             NominalPerformanceInteger,
+             CpcPackage
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlAddRegisterOrIntegerToPackage (
+             LowestNonlinearPerformanceBuffer,
+             LowestNonlinearPerformanceInteger,
+             CpcPackage
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlAddRegisterOrIntegerToPackage (
+             LowestPerformanceBuffer,
+             LowestPerformanceInteger,
+             CpcPackage
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlAddRegisterToPackage (GuaranteedPerformanceRegister, CpcPackage);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlAddRegisterToPackage (DesiredPerformanceRegister, CpcPackage);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlAddRegisterToPackage (MinimumPerformanceRegister, CpcPackage);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlAddRegisterToPackage (MaximumPerformanceRegister, CpcPackage);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlAddRegisterToPackage (PerformanceReductionToleranceRegister, CpcPackage);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlAddRegisterToPackage (TimeWindowRegister, CpcPackage);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlAddRegisterOrIntegerToPackage (
+             CounterWraparoundTimeBuffer,
+             CounterWraparoundTimeInteger,
+             CpcPackage
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlAddRegisterToPackage (ReferencePerformanceCounterRegister, CpcPackage);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlAddRegisterToPackage (DeliveredPerformanceCounterRegister, CpcPackage);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlAddRegisterToPackage (PerformanceLimitedRegister, CpcPackage);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlAddRegisterToPackage (CPPCEnableRegister, CpcPackage);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlAddRegisterOrIntegerToPackage (
+             AutonomousSelectionEnableBuffer,
+             AutonomousSelectionEnableInteger,
+             CpcPackage
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlAddRegisterToPackage (AutonomousActivityWindowRegister, CpcPackage);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlAddRegisterToPackage (EnergyPerformancePreferenceRegister, CpcPackage);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlAddRegisterOrIntegerToPackage (
+             ReferencePerformanceBuffer,
+             ReferencePerformanceInteger,
+             CpcPackage
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlAddRegisterOrIntegerToPackage (
+             LowestFrequencyBuffer,
+             LowestFrequencyInteger,
+             CpcPackage
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlAddRegisterOrIntegerToPackage (
+             NominalFrequencyBuffer,
+             NominalFrequencyInteger,
+             CpcPackage
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  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;
+  }
+
+  return Status;
+
+error_handler:
+  if (CpcNode != NULL) {
+    AmlDeleteTree ((AML_NODE_HANDLE)CpcNode);
+  }
+
+  return Status;
+}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/3] DynamicTablesPkg: SSDT CPU _CPC generator
  2022-09-07 15:15 [PATCH 0/3] DynamicTablesPkg: _CPC support Jeff Brasen
  2022-09-07 15:15 ` [PATCH 1/3] DynamicTablesPkg: Add CM_ARM_CPC_INFO object Jeff Brasen
  2022-09-07 15:15 ` [PATCH 2/3] DynamicTablesPkg: AML Code generation to add _CPC entries Jeff Brasen
@ 2022-09-07 15:15 ` Jeff Brasen
  2022-09-12 10:37   ` [edk2-devel] " PierreGondois
  2 siblings, 1 reply; 7+ messages in thread
From: Jeff Brasen @ 2022-09-07 15:15 UTC (permalink / raw)
  To: devel; +Cc: ardb+tianocore, Sami.Mujawar, Alexei.Fedorov, Jeff Brasen

Add code to use a token attached to GICC to generate _CPC object on cpus.

Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
---
 .../SsdtCpuTopologyGenerator.c                | 211 +++++++++++++++++-
 1 file changed, 206 insertions(+), 5 deletions(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
index 8561f48e1f..ba1f1bd436 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
@@ -76,6 +76,16 @@ GET_OBJECT_LIST (
   CM_ARM_LPI_INFO
   );
 
+/**
+  This macro expands to a function that retrieves the CPC
+  information from the Configuration Manager.
+*/
+GET_OBJECT_LIST (
+  EObjNameSpaceArm,
+  EArmObjCpcInfo,
+  CM_ARM_CPC_INFO
+  );
+
 /** Initialize the TokenTable.
 
   One entry should be allocated for each CM_ARM_PROC_HIERARCHY_INFO
@@ -229,6 +239,182 @@ WriteAslName (
   return EFI_SUCCESS;
 }
 
+/** Utility function to check if generic address points to NULL
+
+  @param [in]  Address  Pointer to the Generic address
+
+  @retval TRUE          Address is system memory with an Address of 0.
+  @retval FALSE         Address does not point to NULL.
+**/
+STATIC
+BOOLEAN
+EFIAPI
+IsNullGenericAddress (
+  IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  *Address
+  )
+{
+  if ((Address == NULL) ||
+      ((Address->AddressSpaceId == EFI_ACPI_6_3_SYSTEM_MEMORY) &&
+       (Address->Address == 0x0)))
+  {
+    return TRUE;
+  }
+
+  return FALSE;
+}
+
+/** Create and add an _CPC Node to Cpu Node.
+
+  For instance, transform an AML node from:
+  Device (C002)
+  {
+      Name (_UID, 2)
+      Name (_HID, "ACPI0007")
+  }
+
+  To:
+  Device (C002)
+  {
+      Name (_UID, 2)
+      Name (_HID, "ACPI0007")
+      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)
+      })
+  }
+
+  @param [in]  Generator              The SSDT Cpu Topology generator.
+  @param [in]  CfgMgrProtocol         Pointer to the Configuration Manager
+                                      Protocol Interface.
+  @param [in]  ProcHierarchyNodeInfo  CM_ARM_PROC_HIERARCHY_INFO describing
+                                      the Cpu.
+  @param [in]  Node                   CPU Node to which the _CPC node is
+                                      attached.
+
+  @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
+CreateAmlCpcNode (
+  IN  ACPI_CPU_TOPOLOGY_GENERATOR                         *Generator,
+  IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  CfgMgrProtocol,
+  IN    CM_ARM_GICC_INFO                                  *GicCInfo,
+  IN  AML_OBJECT_NODE_HANDLE                              *Node
+  )
+{
+  EFI_STATUS       Status;
+  CM_ARM_CPC_INFO  *CpcInfo;
+
+  Status = GetEArmObjCpcInfo (
+             CfgMgrProtocol,
+             GicCInfo->CpcToken,
+             &CpcInfo,
+             NULL
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    return Status;
+  }
+
+  Status = AmlCreateCpcNode (
+             IsNullGenericAddress (&CpcInfo->HighestPerformanceBuffer) ?
+             NULL :
+             &CpcInfo->HighestPerformanceBuffer,
+             CpcInfo->HighestPerformanceInteger,
+             IsNullGenericAddress (&CpcInfo->NominalPerformanceBuffer) ?
+             NULL :
+             &CpcInfo->NominalPerformanceBuffer,
+             CpcInfo->NominalPerformanceInteger,
+             IsNullGenericAddress (&CpcInfo->LowestNonlinearPerformanceBuffer) ?
+             NULL :
+             &CpcInfo->LowestNonlinearPerformanceBuffer,
+             CpcInfo->LowestNonlinearPerformanceInteger,
+             IsNullGenericAddress (&CpcInfo->LowestPerformanceBuffer) ?
+             NULL :
+             &CpcInfo->LowestPerformanceBuffer,
+             CpcInfo->LowestPerformanceInteger,
+             IsNullGenericAddress (&CpcInfo->GuaranteedPerformanceRegister) ?
+             NULL :
+             &CpcInfo->GuaranteedPerformanceRegister,
+             IsNullGenericAddress (&CpcInfo->DesiredPerformanceRegister) ?
+             NULL :
+             &CpcInfo->DesiredPerformanceRegister,
+             IsNullGenericAddress (&CpcInfo->MinimumPerformanceRegister) ?
+             NULL :
+             &CpcInfo->MinimumPerformanceRegister,
+             IsNullGenericAddress (&CpcInfo->MaximumPerformanceRegister) ?
+             NULL :
+             &CpcInfo->MaximumPerformanceRegister,
+             IsNullGenericAddress (&CpcInfo->PerformanceReductionToleranceRegister) ?
+             NULL :
+             &CpcInfo->PerformanceReductionToleranceRegister,
+             IsNullGenericAddress (&CpcInfo->TimeWindowRegister) ?
+             NULL :
+             &CpcInfo->TimeWindowRegister,
+             IsNullGenericAddress (&CpcInfo->CounterWraparoundTimeBuffer) ?
+             NULL :
+             &CpcInfo->CounterWraparoundTimeBuffer,
+             CpcInfo->CounterWraparoundTimeInteger,
+             &CpcInfo->ReferencePerformanceCounterRegister,
+             &CpcInfo->DeliveredPerformanceCounterRegister,
+             &CpcInfo->PerformanceLimitedRegister,
+             IsNullGenericAddress (&CpcInfo->CPPCEnableRegister) ?
+             NULL :
+             &CpcInfo->CPPCEnableRegister,
+             IsNullGenericAddress (&CpcInfo->AutonomousSelectionEnableBuffer) ?
+             NULL :
+             &CpcInfo->AutonomousSelectionEnableBuffer,
+             CpcInfo->AutonomousSelectionEnableInteger,
+             IsNullGenericAddress (&CpcInfo->AutonomousActivityWindowRegister) ?
+             NULL :
+             &CpcInfo->AutonomousActivityWindowRegister,
+             IsNullGenericAddress (&CpcInfo->EnergyPerformancePreferenceRegister) ?
+             NULL :
+             &CpcInfo->EnergyPerformancePreferenceRegister,
+             IsNullGenericAddress (&CpcInfo->ReferencePerformanceBuffer) ?
+             NULL :
+             &CpcInfo->ReferencePerformanceBuffer,
+             CpcInfo->ReferencePerformanceInteger,
+             IsNullGenericAddress (&CpcInfo->LowestFrequencyBuffer) ?
+             NULL :
+             &CpcInfo->LowestFrequencyBuffer,
+             CpcInfo->LowestFrequencyInteger,
+             IsNullGenericAddress (&CpcInfo->NominalFrequencyBuffer) ?
+             NULL :
+             &CpcInfo->NominalFrequencyBuffer,
+             CpcInfo->NominalFrequencyInteger,
+             Node,
+             NULL
+             );
+  ASSERT_EFI_ERROR (Status);
+  return Status;
+}
+
 /** Create and add an _LPI method to Cpu/Cluster Node.
 
   For instance, transform an AML node from:
@@ -584,6 +770,13 @@ CreateAmlCpuFromProcHierarchy (
     ASSERT_EFI_ERROR (Status);
   }
 
+  // If a CPC info is associated with the
+  // GicCinfo, create an _CPC method returning them.
+  if (GicCInfo->CpcToken != CM_NULL_TOKEN) {
+    Status = CreateAmlCpcNode (Generator, CfgMgrProtocol, GicCInfo, CpuNode);
+    ASSERT_EFI_ERROR (Status);
+  }
+
   return Status;
 }
 
@@ -934,10 +1127,11 @@ CreateTopologyFromGicC (
   IN        AML_OBJECT_NODE_HANDLE                        ScopeNode
   )
 {
-  EFI_STATUS        Status;
-  CM_ARM_GICC_INFO  *GicCInfo;
-  UINT32            GicCInfoCount;
-  UINT32            Index;
+  EFI_STATUS              Status;
+  CM_ARM_GICC_INFO        *GicCInfo;
+  UINT32                  GicCInfoCount;
+  UINT32                  Index;
+  AML_OBJECT_NODE_HANDLE  CpuNode;
 
   ASSERT (Generator != NULL);
   ASSERT (CfgMgrProtocol != NULL);
@@ -961,12 +1155,19 @@ CreateTopologyFromGicC (
                ScopeNode,
                &GicCInfo[Index],
                Index,
-               NULL
+               &CpuNode
                );
     if (EFI_ERROR (Status)) {
       ASSERT (0);
       break;
     }
+
+    // If a CPC info is associated with the
+    // GicCinfo, create an _CPC method returning them.
+    if (GicCInfo->CpcToken != CM_NULL_TOKEN) {
+      Status = CreateAmlCpcNode (Generator, CfgMgrProtocol, &GicCInfo[Index], CpuNode);
+      ASSERT_EFI_ERROR (Status);
+    }
   } // for
 
   return Status;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [edk2-devel] [PATCH 1/3] DynamicTablesPkg: Add CM_ARM_CPC_INFO object
  2022-09-07 15:15 ` [PATCH 1/3] DynamicTablesPkg: Add CM_ARM_CPC_INFO object Jeff Brasen
@ 2022-09-12 10:34   ` PierreGondois
  0 siblings, 0 replies; 7+ messages in thread
From: PierreGondois @ 2022-09-12 10:34 UTC (permalink / raw)
  To: devel, jbrasen; +Cc: ardb+tianocore, Sami.Mujawar, Alexei.Fedorov

Hello Jeff,
Please find some remarks inline:

On 9/7/22 17:15, Jeff Brasen via groups.io wrote:
> Introduce the CM_ARM_CPC_INFO CmObj in the ArmNameSpaceObjects.
> 
> This allows to describe CPC information, as described in ACPI 6.4,
> 
> s8.4.7.1 "_CPC (Continuous Performance Control)".
> 
> 
> 
> Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
> 
> ---
> 
>   .../Include/ArmNameSpaceObjects.h             | 146 ++++++++++++++++--
> 
>   .../ConfigurationManagerObjectParser.c        |  79 ++++++++++
> 
>   2 files changed, 208 insertions(+), 17 deletions(-)
> 
> 
> 
> diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> 
> index 102e0f96be..4d3f9ae534 100644
> 
> --- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> 
> +++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> 
> @@ -63,6 +63,7 @@ typedef enum ArmObjectID {
> 
>     EArmObjPciInterruptMapInfo,          ///< 39 - Pci Interrupt Map Info
> 
>     EArmObjRmr,                          ///< 40 - Reserved Memory Range Node
> 
>     EArmObjMemoryRangeDescriptor,        ///< 41 - Memory Range Descriptor
> 
> +  EArmObjCpcInfo,                      ///< 42 - Continuous Performance Control Info
> 
>     EArmObjMax
> 
>   } EARM_OBJECT_ID;
> 
>   

[snip]

> 
> +/** A structure that describes the Cpc information.
> 
> +
> 
> +  Continuous Performance Control is described in DSDT/SSDT and associated
> 
> +  to cpus/clusters in the cpu topology.
> 
> +
> 
> +  Unsupported Optional registers should be encoded with NULL resource
> 
> +  Register {(SystemMemory, 0, 0, 0, 0)}
> 
> +
> 
> +  For values that support Integer or Buffer, integer will be used
> 
> +  if buffer is NULL resource.
> 
> +  If resource is not NULL then Integer must be 0
> 
> +
> 
> +  Cf. ACPI 6.4, s8.4.7.1 _CPC (Continuous Performance Control)
> 
> +
> 
> +  ID: EArmObjCpcInfo
> 
> +*/
> 
> +typedef struct CmArmCpcInfo {

I think it would be good to have the CPC Revision entry here aswell.
The NumEntries can be inferred from the Revision.

> 
> +  /// Indicates the highest level of performance the processor
> 
> +  /// is theoretically capable of achieving.
> 
> +  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE    HighestPerformanceBuffer;

I think we can use ACPI 6.4 structs (it should be the same).
This comment can be applied to all the *6_3* objects.

> 
> +  UINT32                                    HighestPerformanceInteger;
> 
> +
> 

[snip]

> 
> @@ -501,6 +578,8 @@ STATIC CONST CM_OBJ_PARSER_ARRAY  ArmNamespaceObjectParser[] = {
> 
>       ARRAY_SIZE (CmArmPciAddressMapInfoParser) },
> 
>     { "EArmObjPciInterruptMapInfo",          CmPciInterruptMapInfoParser,
> 
>       ARRAY_SIZE (CmPciInterruptMapInfoParser) },
> 

(for Sami)
Not related to this patchset, but the following parsers are missing:
-EArmObjRmr
-EArmObjMemoryRangeDescriptor

> +  { "EArmObjCpcInfo",                      CmArmCpcInfoParser,
> 
> +    ARRAY_SIZE (CmArmCpcInfoParser) },
> 
>     { "EArmObjMax",                          NULL,                                  0                                },
> 
>   };
> 
>   
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [edk2-devel] [PATCH 2/3] DynamicTablesPkg: AML Code generation to add _CPC entries
  2022-09-07 15:15 ` [PATCH 2/3] DynamicTablesPkg: AML Code generation to add _CPC entries Jeff Brasen
@ 2022-09-12 10:36   ` PierreGondois
  0 siblings, 0 replies; 7+ messages in thread
From: PierreGondois @ 2022-09-12 10:36 UTC (permalink / raw)
  To: devel, jbrasen; +Cc: ardb+tianocore, Sami.Mujawar, Alexei.Fedorov

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 <jbrasen@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;
> 
> +}
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [edk2-devel] [PATCH 3/3] DynamicTablesPkg: SSDT CPU _CPC generator
  2022-09-07 15:15 ` [PATCH 3/3] DynamicTablesPkg: SSDT CPU _CPC generator Jeff Brasen
@ 2022-09-12 10:37   ` PierreGondois
  0 siblings, 0 replies; 7+ messages in thread
From: PierreGondois @ 2022-09-12 10:37 UTC (permalink / raw)
  To: devel, jbrasen; +Cc: ardb+tianocore, Sami.Mujawar, Alexei.Fedorov

Hello Jeff,
Please find some remarks inline:

On 9/7/22 17:15, Jeff Brasen via groups.io wrote:
> Add code to use a token attached to GICC to generate _CPC object on cpus.
> 
> 
> 
> Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
> 
> ---
> 
>   .../SsdtCpuTopologyGenerator.c                | 211 +++++++++++++++++-
> 
>   1 file changed, 206 insertions(+), 5 deletions(-)
> 
> 
> 
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
> 
> index 8561f48e1f..ba1f1bd436 100644
> 
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
> 
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
> 

[snip]

> 
> @@ -584,6 +770,13 @@ CreateAmlCpuFromProcHierarchy (
> 
>       ASSERT_EFI_ERROR (Status);
> 

I think a 'return Status;' needs to be added here.

>     }
> 
>   
> 
> +  // If a CPC info is associated with the
> 
> +  // GicCinfo, create an _CPC method returning them.
> 
> +  if (GicCInfo->CpcToken != CM_NULL_TOKEN) {
> 
> +    Status = CreateAmlCpcNode (Generator, CfgMgrProtocol, GicCInfo, CpuNode);
> 
> +    ASSERT_EFI_ERROR (Status);
> 
> +  }
> 
> +
> 
>     return Status;
> 
>   }
> 
>   
> 
> @@ -934,10 +1127,11 @@ CreateTopologyFromGicC (
> 
>     IN        AML_OBJECT_NODE_HANDLE                        ScopeNode
> 
>     )
> 
>   {
> 
> -  EFI_STATUS        Status;
> 
> -  CM_ARM_GICC_INFO  *GicCInfo;
> 
> -  UINT32            GicCInfoCount;
> 
> -  UINT32            Index;
> 
> +  EFI_STATUS              Status;
> 
> +  CM_ARM_GICC_INFO        *GicCInfo;
> 
> +  UINT32                  GicCInfoCount;
> 
> +  UINT32                  Index;
> 
> +  AML_OBJECT_NODE_HANDLE  CpuNode;
> 
>   
> 
>     ASSERT (Generator != NULL);
> 
>     ASSERT (CfgMgrProtocol != NULL);
> 
> @@ -961,12 +1155,19 @@ CreateTopologyFromGicC (
> 
>                  ScopeNode,
> 
>                  &GicCInfo[Index],
> 
>                  Index,
> 
> -               NULL
> 
> +               &CpuNode
> 
>                  );
> 
>       if (EFI_ERROR (Status)) {
> 
>         ASSERT (0);
> 
>         break;
> 
>       }
> 
> +
> 
> +    // If a CPC info is associated with the
> 
> +    // GicCinfo, create an _CPC method returning them.
> 
> +    if (GicCInfo->CpcToken != CM_NULL_TOKEN) {
> 
> +      Status = CreateAmlCpcNode (Generator, CfgMgrProtocol, &GicCInfo[Index], CpuNode);
> 
> +      ASSERT_EFI_ERROR (Status);

Shouln't we consider this as an error and break ? As:
if (EFI_ERROR (Status)) {
   ASSERT_EFI_ERROR (Status);
   break;
}

> 
> +    }
> 
>     } // for
> 
>   
> 
>     return Status;
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-09-12 10:38 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-07 15:15 [PATCH 0/3] DynamicTablesPkg: _CPC support Jeff Brasen
2022-09-07 15:15 ` [PATCH 1/3] DynamicTablesPkg: Add CM_ARM_CPC_INFO object Jeff Brasen
2022-09-12 10:34   ` [edk2-devel] " PierreGondois
2022-09-07 15:15 ` [PATCH 2/3] DynamicTablesPkg: AML Code generation to add _CPC entries Jeff Brasen
2022-09-12 10:36   ` [edk2-devel] " PierreGondois
2022-09-07 15:15 ` [PATCH 3/3] DynamicTablesPkg: SSDT CPU _CPC generator Jeff Brasen
2022-09-12 10:37   ` [edk2-devel] " PierreGondois

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox