Re: [PATCH v1 12/13] DynamicTablesPkg: Add CM_ARM_LPI_INFO object

["Sami Mujawar" <sami.mujawar@arm.com>]

Hi Pierre,

Thank you for this patch.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar


On 23/06/2021 12:40 PM, Pierre.Gondois@arm.com wrote:
> From: Pierre Gondois <Pierre.Gondois@arm.com>
>
> Introduce the CM_ARM_LPI_INFO CmObj in the ArmNameSpaceObjects.
> This allows to describe LPI state information, as described in
> ACPI 6.4, s8.4.4.3 "_LPI (Low Power Idle States)".
>
> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
> ---
>   .../Include/ArmNameSpaceObjects.h             | 68 +++++++++++++++++++
>   .../ConfigurationManagerObjectParser.c        | 35 ++++++++++
>   2 files changed, 103 insertions(+)
>
> diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> index 19dcae13b219..9c49091def59 100644
> --- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> +++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> @@ -58,6 +58,7 @@ typedef enum ArmObjectID {
>     EArmObjGenericInitiatorAffinityInfo, ///< 34 - Generic Initiator Affinity
>     EArmObjSerialPortInfo,               ///< 35 - Generic Serial Port Info
>     EArmObjCmn600Info,                   ///< 36 - CMN-600 Info
> +  EArmObjLpiInfo,                      ///< 37 - Lpi Info
>     EArmObjMax
>   } EARM_OBJECT_ID;
>   
> @@ -711,6 +712,10 @@ typedef struct CmArmProcHierarchyInfo {
>     /// the NoOfPrivateResources is 0, in which case it is recommended to set
>     /// this field to CM_NULL_TOKEN.
>     CM_OBJECT_TOKEN   PrivateResourcesArrayToken;
> +  /// Optional field: Reference Token for the Lpi state of this processor.
> +  /// Token identifying a CM_ARM_OBJ_REF structure, itself referencing
> +  /// CM_ARM_LPI_INFO objects.
> +  CM_OBJECT_TOKEN   LpiToken;
>   } CM_ARM_PROC_HIERARCHY_INFO;
>   
>   /** A structure that describes the Cache Type Structure (Type 1) in PPTT
> @@ -878,6 +883,69 @@ typedef struct CmArmCmn600Info {
>     CM_ARM_EXTENDED_INTERRUPT  DtcInterrupt[4];
>   } CM_ARM_CMN_600_INFO;
>   
> +/** A structure that describes the Lpi information.
> +
> +  The Low Power Idle states are described in DSDT/SSDT and associated
> +  to cpus/clusters in the cpu topology.
> +
> +  ID: EArmObjLpiInfo
> +*/
> +typedef struct CmArmLpiInfo {
> +  /** Minimum Residency. Time in microseconds after which a
> +      state becomes more energy efficient than any shallower state.
> +  */
> +  UINT32                                  MinResidency;
> +
> +  /** Worst case time in microseconds from a wake interrupt
> +      being asserted to the return to a running state
> +  */
> +  UINT32                                  WorstCaseWakeLatency;
> +
> +  /** Flags.
> +  */
> +  UINT32                                  Flags;
> +
> +  /** Architecture specific context loss flags.
> +  */
> +  UINT32                                  ArchFlags;
> +
> +  /** Residency counter frequency in cycles-per-second (Hz).
> +  */
> +  UINT32                                  ResCntFreq;
> +
> +  /** Every shallower power state in the parent is also enabled.
> +  */
> +  UINT32                                  EnableParentState;
> +
> +  /** The EntryMethod _LPI field can be described as an integer
> +      or in a Register resource data descriptor.
> +
> +  If IsInteger is TRUE, the IntegerEntryMethod field is used.
> +  If IsInteger is FALSE, the RegisterEntryMethod field is used.
> +  */
> +  BOOLEAN                                 IsInteger;
> +
> +  /** EntryMethod described as an Integer.
> +  */
> +  UINT64                                  IntegerEntryMethod;
> +
> +  /** EntryMethod described as a EFI_ACPI_GENERIC_REGISTER_DESCRIPTOR.
> +  */
> +  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  RegisterEntryMethod;
> +
> +  /** Residency counter register.
> +  */
> +  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  ResidencyCounterRegister;
> +
> +  /** Usage counter register.
> +  */
> +  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  UsageCounterRegister;
> +
> +  /** String representing the Lpi state
> +  */
> +  CHAR8                                   StateName[16];
> +} CM_ARM_LPI_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 654ead6878e6..da5f5846edd9 100644
> --- a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
> +++ b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
> @@ -368,6 +368,39 @@ STATIC CONST CM_OBJ_PARSER CmArmCmn600InfoParser[] = {
>     {"DtcFlags[3]", 4, "0x%x", NULL}
>   };
>   
> +/** A parser for the EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE structure.
> +*/
> +STATIC CONST CM_OBJ_PARSER AcpiGenericAddressParser[] = {
> +  {"AddressSpaceId", 1, "%d", NULL},
> +  {"RegisterBitWidth", 1, "%d", NULL},
> +  {"RegisterBitOffset", 1, "%d", NULL},
> +  {"AccessSize", 1, "%d", NULL},
> +  {"Address", 8, "0x%llx", NULL},
> +};
> +
> +/** A parser for EArmObjLpiInfo.
> +*/
> +STATIC CONST CM_OBJ_PARSER CmArmLpiInfoParser[] = {
> +  {"MinResidency", 4, "0x%llx", NULL},
> +  {"WorstCaseWakeLatency", 4, "0x%llx", NULL},
> +  {"Flags", 4, "0x%llx", NULL},
> +  {"ArchFlags", 4, "0x%llx", NULL},
> +  {"ResCntFreq", 4, "0x%llx", NULL},
> +  {"EnableParentState", 4, "0x%llx", NULL},
> +  {"IsInteger", 1, "%d", NULL},
> +  {"IntegerEntryMethod", 8, "0x%llx", NULL},
> +  {"RegisterEntryMethod", sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
> +    NULL, NULL, AcpiGenericAddressParser,
> +    ARRAY_SIZE (AcpiGenericAddressParser)},
> +  {"ResidencyCounterRegister", sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
> +    NULL, NULL, AcpiGenericAddressParser,
> +    ARRAY_SIZE (AcpiGenericAddressParser)},
> +  {"UsageCounterRegister", sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
> +    NULL, NULL, AcpiGenericAddressParser,
> +    ARRAY_SIZE (AcpiGenericAddressParser)},
> +  {"StateName", 16, "0x%a", NULL},
> +};
> +
>   /** A parser for Arm namespace objects.
>   */
>   STATIC CONST CM_OBJ_PARSER_ARRAY ArmNamespaceObjectParser[] = {
> @@ -440,6 +473,8 @@ STATIC CONST CM_OBJ_PARSER_ARRAY ArmNamespaceObjectParser[] = {
>       ARRAY_SIZE (CmArmSerialPortInfoParser)},
>     {"EArmObjCmn600Info", CmArmCmn600InfoParser,
>       ARRAY_SIZE (CmArmCmn600InfoParser)},
> +  {"EArmObjLpiInfo", CmArmLpiInfoParser,
> +      ARRAY_SIZE (CmArmLpiInfoParser)},
>     {"EArmObjMax", NULL, 0},
>   };
>