[edk2-devel] [PATCH v2 00/11] DynamicTablesPkg: Enable _PSD/_CPC generation using SCMI

[edk2-devel] [PATCH v2 00/11] DynamicTablesPkg: Enable _PSD/_CPC generation using SCMI

[PierreGondois]

From: Pierre Gondois <pierre.gondois@arm.com>

v1:
- https://edk2.groups.io/g/devel/message/104115
v2:
- Rebase patches on latest version.

The SCMI performance protocol allows to query the Operating Performance
Points (OPPs) available for each performance domain. Each OPP has a
specific frequency/power consumption/performance.

On Device Tree (DT) based platforms, the SCMI protocol is directly
available from the OS.
On ACPI based platforms, the _PSD objects allows to identify CPUs
belonging
to the same performance domain. CPUs belonging to the same performance
domain have the same frequency. The _CPC object allows to
describe/control the performance level/frequency of a CPU (i.e. its
frequency).

This patchset:
- Add support for 'DescribeFastchannel' SCMI performance protocol
- Add a object to the DynamicTablesPkg to describe the PSD information
- Allows to generate _PSD objects in the SsdtCpuTopologyGenerator
- Add a ArmScmiInfoLib library populating DynamicTablesPkg CPC objects,
  relying on the SCMI protocol to fetch the relevant information

A patchset for Juno-r2 platforms will be submitted to demonstrate
this functionality.

Pierre Gondois (11):
  ArmPkg/ArmScmiDxe: Rename PERFORMANCE_PROTOCOL_VERSION
  ArmPkg/ArmScmiDxe: Add PERFORMANCE_DESCRIBE_FASTCHANNEL support
  MdePkg/Acpi64: Add _PSD/_CPC/State Coordination Types macros
  DynamicTablesPkg: Use new CPC revision macro
  DynamicTablesPkg: Rename AmlCpcInfo.h to AcpiObjects.h
  DynamicTablesPkg: Add CM_ARM_PSD_INFO object
  DynamicTablesPkg: Add PsdToken field to CM_ARM_GICC_INFO object
  DynamicTablesPkg: Add AmlCreatePsdNode() to generate _PSD
  DynamicTablesPkg: Generate _PSD in SsdtCpuTopologyGenerator
  DynamicTablesPkg: Add ArmScmiInfoLib
  DynamicTablesPkg: Remove check for _CPC field

 .../ArmScmiDxe/ScmiPerformanceProtocol.c      |  80 ++++-
 ArmPkg/Include/Library/ArmLib.h               |   1 +
 .../Protocol/ArmScmiPerformanceProtocol.h     | 101 +++++-
 DynamicTablesPkg/DynamicTables.dsc.inc        |   1 +
 DynamicTablesPkg/DynamicTablesPkg.dec         |   3 +
 DynamicTablesPkg/DynamicTablesPkg.dsc         |   1 +
 .../Include/{AmlCpcInfo.h => AcpiObjects.h}   |  20 ++
 .../Include/ArmNameSpaceObjects.h             |  19 +-
 .../Include/Library/AmlLib/AmlLib.h           |  37 ++-
 .../Include/Library/ArmScmiInfoLib.h          |  33 ++
 .../SsdtCpuTopologyGenerator.c                |  98 +++++-
 .../Library/ArmScmiInfoLib/ArmScmiInfoLib.c   | 294 ++++++++++++++++++
 .../Library/ArmScmiInfoLib/ArmScmiInfoLib.inf |  31 ++
 .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 211 ++++++++++++-
 .../ConfigurationManagerObjectParser.c        |  14 +-
 MdePkg/Include/IndustryStandard/Acpi64.h      |  25 +-
 16 files changed, 936 insertions(+), 33 deletions(-)
 rename DynamicTablesPkg/Include/{AmlCpcInfo.h => AcpiObjects.h} (88%)
 create mode 100644 DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h
 create mode 100644 DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c
 create mode 100644 DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf

-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110036): https://edk2.groups.io/g/devel/message/110036
Mute This Topic: https://groups.io/mt/102175809/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 01/11] ArmPkg/ArmScmiDxe: Rename PERFORMANCE_PROTOCOL_VERSION

[PierreGondois]

From: Pierre Gondois <pierre.gondois@arm.com>

Rename PERFORMANCE_PROTOCOL_VERSION to reflect the different
versions of the protocol. The macro is neither used in edk2 nor
in edk2-platforms.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 ArmPkg/Include/Library/ArmLib.h                     |  1 +
 .../Include/Protocol/ArmScmiPerformanceProtocol.h   | 13 ++++++++-----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
index 0169dbc1092c..7b2b2238fed9 100644
--- a/ArmPkg/Include/Library/ArmLib.h
+++ b/ArmPkg/Include/Library/ArmLib.h
@@ -780,6 +780,7 @@ EFIAPI
 ArmHasVhe (
   VOID
   );
+
 #endif // MDE_CPU_AARCH64
 
 #ifdef MDE_CPU_ARM
diff --git a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
index 7e548e4765c2..8e8e05d5a5f6 100644
--- a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
+++ b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
@@ -1,12 +1,12 @@
 /** @file
 
-  Copyright (c) 2017-2021, Arm Limited. All rights reserved.
+  Copyright (c) 2017-2023, Arm Limited. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
-  System Control and Management Interface V1.0
-    http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/
-    DEN0056A_System_Control_and_Management_Interface.pdf
+  System Control and Management Interface, latest version:
+  - https://developer.arm.com/documentation/den0056/latest/
+
 **/
 
 #ifndef ARM_SCMI_PERFORMANCE_PROTOCOL_H_
@@ -14,7 +14,10 @@
 
 #include <Protocol/ArmScmi.h>
 
-#define PERFORMANCE_PROTOCOL_VERSION  0x10000
+/// Arm Scmi performance protocol versions.
+#define PERFORMANCE_PROTOCOL_VERSION_V1  0x10000
+#define PERFORMANCE_PROTOCOL_VERSION_V2  0x20000
+#define PERFORMANCE_PROTOCOL_VERSION_V3  0x30000
 
 #define ARM_SCMI_PERFORMANCE_PROTOCOL_GUID  { \
   0x9b8ba84, 0x3dd3, 0x49a6, {0xa0, 0x5a, 0x31, 0x34, 0xa5, 0xf0, 0x7b, 0xad} \
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110037): https://edk2.groups.io/g/devel/message/110037
Mute This Topic: https://groups.io/mt/102175810/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 02/11] ArmPkg/ArmScmiDxe: Add PERFORMANCE_DESCRIBE_FASTCHANNEL support

[PierreGondois]

From: Pierre Gondois <pierre.gondois@arm.com>

The PERFORMANCE_DESCRIBE_FASTCHANNEL Scmi command is available
since SCMI v2.0 and allows to query information about the supported
fast-channels of the Scmi performance protocol.
Add support for this command.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 .../ArmScmiDxe/ScmiPerformanceProtocol.c      | 80 +++++++++++++++--
 .../Protocol/ArmScmiPerformanceProtocol.h     | 90 ++++++++++++++++---
 2 files changed, 155 insertions(+), 15 deletions(-)

diff --git a/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c b/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
index 0f89808fbdf9..1d87339209fd 100644
--- a/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
+++ b/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
@@ -1,12 +1,12 @@
 /** @file
 
-  Copyright (c) 2017-2021, Arm Limited. All rights reserved.<BR>
+  Copyright (c) 2017-2023, Arm Limited. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
-  System Control and Management Interface V1.0
-    http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/
-    DEN0056A_System_Control_and_Management_Interface.pdf
+  System Control and Management Interface, latest version:
+  - https://developer.arm.com/documentation/den0056/latest/
+
 **/
 
 #include <Library/BaseMemoryLib.h>
@@ -416,6 +416,75 @@ PerformanceLevelGet (
   return EFI_SUCCESS;
 }
 
+/** Discover the attributes of the FastChannel for the specified
+    performance domain and the specified message.
+
+  @param[in]  This        A Pointer to SCMI_PERFORMANCE_PROTOCOL Instance.
+  @param[in]  DomainId    Identifier for the performance domain.
+  @param[in]  MessageId   Message Id of the FastChannel to discover.
+                          Must be one of:
+                           - PERFORMANCE_LIMITS_SET
+                           - PERFORMANCE_LIMITS_GET
+                           - PERFORMANCE_LEVEL_SET
+                           - PERFORMANCE_LEVEL_GET
+  @param[out] FastChannel If success, contains the FastChannel description.
+
+  @retval EFI_SUCCESS             Performance level got successfully.
+  @retval EFI_DEVICE_ERROR        SCP returns an SCMI error.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_TIMEOUT             Time out.
+  @retval EFI_UNSUPPORTED         Unsupported.
+**/
+EFI_STATUS
+DescribeFastchannel (
+  IN  SCMI_PERFORMANCE_PROTOCOL     *This,
+  IN  UINT32                        DomainId,
+  IN  SCMI_MESSAGE_ID_PERFORMANCE   MessageId,
+  OUT SCMI_PERFORMANCE_FASTCHANNEL  *FastChannel
+  )
+{
+  EFI_STATUS    Status;
+  SCMI_COMMAND  Cmd;
+  UINT32        PayloadLength;
+  UINT32        *ReturnValues;
+  UINT32        *MessageParams;
+
+  if ((This == NULL)  ||
+      (FastChannel == NULL))
+  {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  Status = ScmiCommandGetPayload (&MessageParams);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  *MessageParams++ = DomainId;
+  *MessageParams   = MessageId;
+
+  Cmd.ProtocolId = ScmiProtocolIdPerformance;
+  Cmd.MessageId  = ScmiMessageIdPerformanceDescribeFastchannel;
+  PayloadLength  = sizeof (DomainId) + sizeof (MessageId);
+
+  Status = ScmiCommandExecute (
+             &Cmd,
+             &PayloadLength,
+             &ReturnValues
+             );
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  CopyMem (
+    FastChannel,
+    ReturnValues,
+    sizeof (SCMI_PERFORMANCE_FASTCHANNEL)
+    );
+
+  return Status;
+}
+
 // Instance of the SCMI performance management protocol.
 STATIC CONST SCMI_PERFORMANCE_PROTOCOL  PerformanceProtocol = {
   PerformanceGetVersion,
@@ -425,7 +494,8 @@ STATIC CONST SCMI_PERFORMANCE_PROTOCOL  PerformanceProtocol = {
   PerformanceLimitsSet,
   PerformanceLimitsGet,
   PerformanceLevelSet,
-  PerformanceLevelGet
+  PerformanceLevelGet,
+  DescribeFastchannel,
 };
 
 /** Initialize performance management protocol and install on a given Handle.
diff --git a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
index 8e8e05d5a5f6..088182945a06 100644
--- a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
+++ b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
@@ -14,7 +14,7 @@
 
 #include <Protocol/ArmScmi.h>
 
-/// Arm Scmi performance protocol versions.
+/// Arm SCMI performance protocol versions.
 #define PERFORMANCE_PROTOCOL_VERSION_V1  0x10000
 #define PERFORMANCE_PROTOCOL_VERSION_V2  0x20000
 #define PERFORMANCE_PROTOCOL_VERSION_V3  0x30000
@@ -79,6 +79,56 @@ typedef struct {
   UINT32    RangeMin;
 } SCMI_PERFORMANCE_LIMITS;
 
+/// Doorbell Support bit.
+#define SCMI_PERF_FC_ATTRIB_HAS_DOORBELL  BIT0
+
+/// Performance protocol describe fastchannel
+typedef struct {
+  /// Attributes.
+  UINT32    Attributes;
+
+  /// Rate limit.
+  UINT32    RateLimit;
+
+  /// Lower 32 bits of the FastChannel address.
+  UINT32    ChanAddrLow;
+
+  /// Higher 32 bits of the FastChannel address.
+  UINT32    ChanAddrHigh;
+
+  /// Size of the FastChannel in bytes.
+  UINT32    ChanSize;
+
+  /// Lower 32 bits of the doorbell address.
+  UINT32    DoorbellAddrLow;
+
+  /// Higher 32 bits of the doorbell address.
+  UINT32    DoorbellAddrHigh;
+
+  /// Mask of lower 32 bits to set when writing to the doorbell register.
+  UINT32    DoorbellSetMaskLow;
+
+  /// Mask of higher 32 bits to set when writing to the doorbell register.
+  UINT32    DoorbellSetMaskHigh;
+
+  /// Mask of lower 32 bits to preserve when writing to the doorbell register.
+  UINT32    DoorbellPreserveMaskLow;
+
+  /// Mask of higher 32 bits to preserve when writing to the doorbell register.
+  UINT32    DoorbellPreserveMaskHigh;
+} SCMI_PERFORMANCE_FASTCHANNEL;
+
+/// SCMI Message Ids for the Performance Protocol.
+typedef enum {
+  ScmiMessageIdPerformanceDomainAttributes    = 0x3,
+  ScmiMessageIdPerformanceDescribeLevels      = 0x4,
+  ScmiMessageIdPerformanceLimitsSet           = 0x5,
+  ScmiMessageIdPerformanceLimitsGet           = 0x6,
+  ScmiMessageIdPerformanceLevelSet            = 0x7,
+  ScmiMessageIdPerformanceLevelGet            = 0x8,
+  ScmiMessageIdPerformanceDescribeFastchannel = 0xB,
+} SCMI_MESSAGE_ID_PERFORMANCE;
+
 #pragma pack()
 
 /** Return version of the performance management protocol supported by SCP.
@@ -238,6 +288,34 @@ EFI_STATUS
   OUT UINT32                    *Level
   );
 
+/** Discover the attributes of the FastChannel for the specified
+    performance domain and the specified message.
+
+  @param[in]  This        A Pointer to SCMI_PERFORMANCE_PROTOCOL Instance.
+  @param[in]  DomainId    Identifier for the performance domain.
+  @param[in]  MessageId   Message Id of the FastChannel to discover.
+                          Must be one of:
+                           - PERFORMANCE_LIMITS_SET
+                           - PERFORMANCE_LIMITS_GET
+                           - PERFORMANCE_LEVEL_SET
+                           - PERFORMANCE_LEVEL_GET
+  @param[out] FastChannel If success, contains the FastChannel description.
+
+  @retval EFI_SUCCESS             Performance level got successfully.
+  @retval EFI_DEVICE_ERROR        SCP returns an SCMI error.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_TIMEOUT             Time out.
+  @retval EFI_UNSUPPORTED         Unsupported.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *SCMI_PERFORMANCE_DESCRIBE_FASTCHANNEL)(
+  IN  SCMI_PERFORMANCE_PROTOCOL       *This,
+  IN  UINT32                          DomainId,
+  IN  SCMI_MESSAGE_ID_PERFORMANCE     MessageId,
+  OUT SCMI_PERFORMANCE_FASTCHANNEL    *FastChannel
+  );
+
 typedef struct _SCMI_PERFORMANCE_PROTOCOL {
   SCMI_PERFORMANCE_GET_VERSION              GetVersion;
   SCMI_PERFORMANCE_GET_ATTRIBUTES           GetProtocolAttributes;
@@ -247,15 +325,7 @@ typedef struct _SCMI_PERFORMANCE_PROTOCOL {
   SCMI_PERFORMANCE_LIMITS_GET               LimitsGet;
   SCMI_PERFORMANCE_LEVEL_SET                LevelSet;
   SCMI_PERFORMANCE_LEVEL_GET                LevelGet;
+  SCMI_PERFORMANCE_DESCRIBE_FASTCHANNEL     DescribeFastchannel;
 } SCMI_PERFORMANCE_PROTOCOL;
 
-typedef enum {
-  ScmiMessageIdPerformanceDomainAttributes = 0x3,
-  ScmiMessageIdPerformanceDescribeLevels   = 0x4,
-  ScmiMessageIdPerformanceLimitsSet        = 0x5,
-  ScmiMessageIdPerformanceLimitsGet        = 0x6,
-  ScmiMessageIdPerformanceLevelSet         = 0x7,
-  ScmiMessageIdPerformanceLevelGet         = 0x8,
-} SCMI_MESSAGE_ID_PERFORMANCE;
-
 #endif /* ARM_SCMI_PERFORMANCE_PROTOCOL_H_ */
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110038): https://edk2.groups.io/g/devel/message/110038
Mute This Topic: https://groups.io/mt/102175812/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 03/11] MdePkg/Acpi64: Add _PSD/_CPC/State Coordination Types macros

[PierreGondois]

From: Pierre Gondois <pierre.gondois@arm.com>

Add macros for:
- _PSD version
- _CPC version
- C-state/T-state/P-state Coordination Types

These objects were present in previous ACPI specification version,
but are only added to the latest availbable version (6.4).

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 MdePkg/Include/IndustryStandard/Acpi64.h | 25 +++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/IndustryStandard/Acpi64.h b/MdePkg/Include/IndustryStandard/Acpi64.h
index 575ca0430c13..e3c128d7548d 100644
--- a/MdePkg/Include/IndustryStandard/Acpi64.h
+++ b/MdePkg/Include/IndustryStandard/Acpi64.h
@@ -2,7 +2,7 @@
   ACPI 6.4 definitions from the ACPI Specification Revision 6.4 Jan, 2021.
 
   Copyright (c) 2017 - 2022, Intel Corporation. All rights reserved.<BR>
-  Copyright (c) 2019 - 2021, ARM Ltd. All rights reserved.<BR>
+  Copyright (c) 2019 - 2023, Arm Ltd. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
@@ -17,6 +17,29 @@
 //
 #pragma pack(1)
 
+///
+/// C-state/T-state/P-state Coordination Types
+/// Cf. s8.3 Power, Performance, and Throttling State Dependencies
+///
+#define EFI_ACPI_6_4_AML_STATE_COORD_TYPE_SW_ALL  0xFC
+#define EFI_ACPI_6_4_AML_STATE_COORD_TYPE_SW_ANY  0xFD
+#define EFI_ACPI_6_4_AML_STATE_COORD_TYPE_HW_ALL  0xFE
+
+///
+/// _PSD Revision
+/// Cf. s8.4.6.5 _PSD (P-State Dependency)
+///
+#define EFI_ACPI_6_4_AML_PSD_REVISION_V0  0
+
+///
+/// _CPC Revision
+/// Cf. s8.4.7.1 _CPC (Continuous Performance Control)
+///
+#define EFI_ACPI_6_4_AML_CPC_REVISION_V0  0
+#define EFI_ACPI_6_4_AML_CPC_REVISION_V1  1
+#define EFI_ACPI_6_4_AML_CPC_REVISION_V2  2
+#define EFI_ACPI_6_4_AML_CPC_REVISION_V3  3
+
 ///
 /// ACPI 6.4 Generic Address Space definition
 ///
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110039): https://edk2.groups.io/g/devel/message/110039
Mute This Topic: https://groups.io/mt/102175813/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 04/11] DynamicTablesPkg: Use new CPC revision macro

[PierreGondois]

From: Pierre Gondois <pierre.gondois@arm.com>

Make use of the newly added CPC revision macro.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index a6db34fb970f..c90f23c53f0e 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -3523,7 +3523,7 @@ AmlCreateCpcNode (
   }
 
   // Revision 3 per ACPI 6.4 specification
-  if (CpcInfo->Revision == 3) {
+  if (CpcInfo->Revision == EFI_ACPI_6_4_AML_CPC_REVISION_V3) {
     // NumEntries 23 per ACPI 6.4 specification
     NumberOfEntries = 23;
   } else {
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110040): https://edk2.groups.io/g/devel/message/110040
Mute This Topic: https://groups.io/mt/102175814/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 05/11] DynamicTablesPkg: Rename AmlCpcInfo.h to AcpiObjects.h

[PierreGondois]

From: Pierre Gondois <pierre.gondois@arm.com>

The DynamicTables framework uses the AmlLib to generate some
Aml objects. It is done by using structured known by both
frameworks, e.g. the AML_CPC_INFO/CM_ARM_CPC_INFO structures.

To prepare adding similar structures (e.g. representing _PSD
information), rename AmlCpcInfo.h to AcpiObjects.h. This new
file will contain all the structures used by the AmlLib and
the DynamicTables framework.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 DynamicTablesPkg/Include/{AmlCpcInfo.h => AcpiObjects.h}    | 0
 DynamicTablesPkg/Include/ArmNameSpaceObjects.h              | 2 +-
 DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h            | 2 +-
 DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c | 2 +-
 4 files changed, 3 insertions(+), 3 deletions(-)
 rename DynamicTablesPkg/Include/{AmlCpcInfo.h => AcpiObjects.h} (100%)

diff --git a/DynamicTablesPkg/Include/AmlCpcInfo.h b/DynamicTablesPkg/Include/AcpiObjects.h
similarity index 100%
rename from DynamicTablesPkg/Include/AmlCpcInfo.h
rename to DynamicTablesPkg/Include/AcpiObjects.h
diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
index 19098609de4b..8199882f69fe 100644
--- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
@@ -13,7 +13,7 @@
 #ifndef ARM_NAMESPACE_OBJECTS_H_
 #define ARM_NAMESPACE_OBJECTS_H_
 
-#include <AmlCpcInfo.h>
+#include <AcpiObjects.h>
 #include <StandardNameSpaceObjects.h>
 
 #pragma pack(1)
diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
index 71e8539b306c..be78c00b6109 100644
--- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
+++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
@@ -36,7 +36,7 @@
   @}
 */
 
-#include <AmlCpcInfo.h>
+#include <AcpiObjects.h>
 #include <IndustryStandard/Acpi.h>
 
 #ifndef AML_HANDLE
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index c90f23c53f0e..f63d9fee8c11 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -11,7 +11,7 @@
 #include <AcpiTableGenerator.h>
 
 #include <AmlCoreInterface.h>
-#include <AmlCpcInfo.h>
+#include <AcpiObjects.h>
 #include <AmlEncoding/Aml.h>
 #include <Api/AmlApiHelper.h>
 #include <CodeGen/AmlResourceDataCodeGen.h>
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110041): https://edk2.groups.io/g/devel/message/110041
Mute This Topic: https://groups.io/mt/102175815/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 06/11] DynamicTablesPkg: Add CM_ARM_PSD_INFO object

[PierreGondois]

From: Pierre Gondois <pierre.gondois@arm.com>

Add an object describing _PSD information, cf. ACPI 6.4,
s8.4.5.5 _PSD (P-State Dependency).
Also add the corresponding CmObjParser.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 DynamicTablesPkg/Include/AcpiObjects.h        | 20 +++++++++++++++++++
 .../Include/ArmNameSpaceObjects.h             | 12 ++++++++++-
 .../ConfigurationManagerObjectParser.c        | 11 ++++++++++
 3 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/DynamicTablesPkg/Include/AcpiObjects.h b/DynamicTablesPkg/Include/AcpiObjects.h
index 8981c229544a..d3c23d8d4f2b 100644
--- a/DynamicTablesPkg/Include/AcpiObjects.h
+++ b/DynamicTablesPkg/Include/AcpiObjects.h
@@ -1,6 +1,7 @@
 /** @file
 
   Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES. All rights reserved.<BR>
+  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
@@ -119,6 +120,25 @@ typedef struct AmlCpcInfo {
   UINT32                                    NominalFrequencyInteger;
 } AML_CPC_INFO;
 
+/** A structure that describes a
+    P-State Dependency (PSD) Info.
+
+  Cf. ACPI 6.4, s8.4.5.5 _PSD (P-State Dependency).
+*/
+typedef struct AmlPsdInfo {
+  /// Revision.
+  UINT8     Revision;
+
+  /// Domain Id.
+  UINT32    Domain;
+
+  /// Coordination type.
+  UINT32    CoordType;
+
+  /// Number of processors belonging to the Domain.
+  UINT32    NumProc;
+} AML_PSD_INFO;
+
 #pragma pack()
 
 #endif //AML_CPC_INFO_H_
diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
index 8199882f69fe..ddd17fa45b1e 100644
--- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
@@ -1,6 +1,6 @@
 /** @file
 
-  Copyright (c) 2017 - 2022, Arm Limited. All rights reserved.<BR>
+  Copyright (c) 2017 - 2023, Arm Limited. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -71,6 +71,7 @@ typedef enum ArmObjectID {
   EArmObjPccSubspaceType3Info,                                 ///< 46 - Pcc Subspace Type 3 Info
   EArmObjPccSubspaceType4Info,                                 ///< 47 - Pcc Subspace Type 4 Info
   EArmObjPccSubspaceType5Info,                                 ///< 48 - Pcc Subspace Type 5 Info
+  EArmObjPsdInfo,                                              ///< 49 - P-State Dependency (PSD) Info
   EArmObjMax
 } EARM_OBJECT_ID;
 
@@ -1297,6 +1298,15 @@ typedef struct CmArmPccSubspaceType5Info {
   PCC_MAILBOX_REGISTER_INFO    ErrorStatusReg;
 } CM_ARM_PCC_SUBSPACE_TYPE5_INFO;
 
+/** A structure that describes a
+    P-State Dependency (PSD) Info.
+
+    Cf. ACPI 6.4, s8.4.5.5 _PSD (P-State Dependency).
+
+    ID: EArmObjPsdInfo
+*/
+typedef AML_PSD_INFO CM_ARM_PSD_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 22b8fdb90606..b3ee12da8c4f 100644
--- a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
+++ b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
@@ -661,6 +661,15 @@ STATIC CONST CM_OBJ_PARSER  CmArmPccSubspaceType5InfoParser[] = {
     ARRAY_SIZE (CmArmMailboxRegisterInfoParser) },
 };
 
+/** A parser for EArmObjPsdInfo.
+*/
+STATIC CONST CM_OBJ_PARSER  CmArmPsdInfoParser[] = {
+  { "Revision",  1, "0x%llx", NULL },
+  { "DomainId",  4, "0x%x",   NULL },
+  { "CoordType", 4, "0x%x",   NULL },
+  { "NumProc",   4, "0x%x",   NULL },
+};
+
 /** A parser for Arm namespace objects.
 */
 STATIC CONST CM_OBJ_PARSER_ARRAY  ArmNamespaceObjectParser[] = {
@@ -757,6 +766,8 @@ STATIC CONST CM_OBJ_PARSER_ARRAY  ArmNamespaceObjectParser[] = {
     ARRAY_SIZE (CmArmPccSubspaceType34InfoParser) },
   { "EArmObjPccSubspaceType5Info",         CmArmPccSubspaceType5InfoParser,
     ARRAY_SIZE (CmArmPccSubspaceType5InfoParser) },
+  { "EArmObjCpcInfo",                      CmArmPsdInfoParser,
+    ARRAY_SIZE (CmArmPsdInfoParser) },
   { "EArmObjMax",                          NULL,                                  0                                },
 };
 
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110042): https://edk2.groups.io/g/devel/message/110042
Mute This Topic: https://groups.io/mt/102175816/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 07/11] DynamicTablesPkg: Add PsdToken field to CM_ARM_GICC_INFO object

[PierreGondois]

From: Pierre Gondois <pierre.gondois@arm.com>

The _PSD object (cf. ACPI 6.4, s8.4.5.5 _PSD (P-State Dependency)
allows to describe CPU's power state dependencies. Add a PsdToken
field to the CM_ARM_GICC_INFO object so that interdependent CPUs
can reference the same CM_ARM_PSD_INFO object.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 DynamicTablesPkg/Include/ArmNameSpaceObjects.h               | 5 +++++
 .../Common/TableHelperLib/ConfigurationManagerObjectParser.c | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
index ddd17fa45b1e..2a0ebe24bd04 100644
--- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
@@ -204,6 +204,11 @@ typedef struct CmArmGicCInfo {
       i.e. a token referencing a CM_ARM_CPC_INFO object.
   */
   CM_OBJECT_TOKEN    CpcToken;
+
+  /** Optional field: Reference Token for the Psd info of this processor.
+      i.e. a token referencing a CM_ARM_PSD_INFO object.
+  */
+  CM_OBJECT_TOKEN    PsdToken;
 } CM_ARM_GICC_INFO;
 
 /** A structure that describes the
diff --git a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
index b3ee12da8c4f..a9f5c95c1039 100644
--- a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
+++ b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
@@ -83,7 +83,8 @@ STATIC CONST CM_OBJ_PARSER  CmArmGicCInfoParser[] = {
   { "ProximityDomain",               4,                        "0x%x",   NULL },
   { "ClockDomain",                   4,                        "0x%x",   NULL },
   { "AffinityFlags",                 4,                        "0x%x",   NULL },
-  { "CpcToken",                      sizeof (CM_OBJECT_TOKEN), "0x%p",   NULL }
+  { "CpcToken",                      sizeof (CM_OBJECT_TOKEN), "0x%p",   NULL },
+  { "PsdToken",                      sizeof (CM_OBJECT_TOKEN), "0x%p",   NULL },
 };
 
 /** A parser for EArmObjGicDInfo.
@@ -766,7 +767,7 @@ STATIC CONST CM_OBJ_PARSER_ARRAY  ArmNamespaceObjectParser[] = {
     ARRAY_SIZE (CmArmPccSubspaceType34InfoParser) },
   { "EArmObjPccSubspaceType5Info",         CmArmPccSubspaceType5InfoParser,
     ARRAY_SIZE (CmArmPccSubspaceType5InfoParser) },
-  { "EArmObjCpcInfo",                      CmArmPsdInfoParser,
+  { "EArmObjPsdInfo",                      CmArmPsdInfoParser,
     ARRAY_SIZE (CmArmPsdInfoParser) },
   { "EArmObjMax",                          NULL,                                  0                                },
 };
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110043): https://edk2.groups.io/g/devel/message/110043
Mute This Topic: https://groups.io/mt/102175817/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 08/11] DynamicTablesPkg: Add AmlCreatePsdNode() to generate _PSD

[PierreGondois]

From: Pierre Gondois <pierre.gondois@arm.com>

Add AmlCreatePsdNode() to the AmlLib to generate _PSD objects.
_PSD objects allow to describe 'performance control, P-state
or CPPC, logical processor dependency', Cf. ACPI 6.4,
s8.4.5.5 _PSD (P-State Dependency).

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 .../Include/Library/AmlLib/AmlLib.h           |  35 +++-
 .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 188 +++++++++++++++++-
 2 files changed, 221 insertions(+), 2 deletions(-)

diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
index be78c00b6109..8145ab70fa69 100644
--- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
+++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
@@ -1,7 +1,7 @@
 /** @file
   AML Lib.
 
-  Copyright (c) 2019 - 2021, Arm Limited. All rights reserved.<BR>
+  Copyright (c) 2019 - 2023, Arm Limited. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
@@ -1628,4 +1628,37 @@ AmlAddNameStringToNamedPackage (
   IN AML_OBJECT_NODE_HANDLE  NamedNode
   );
 
+/** Create a _PSD node.
+
+  Creates and optionally adds the following node
+   Name(_PSD, Package()
+   {
+    NumEntries,  // Integer
+    Revision,    // Integer
+    Domain,      // Integer
+    CoordType,   // Integer
+    NumProc,     // Integer
+  })
+
+  Cf. ACPI 6.4, s8.4.6.5 _PSD (P-State Dependency)
+
+  @ingroup CodeGenApis
+
+  @param [in]  PsdInfo      PsdInfo object
+  @param [in]  ParentNode   If provided, set ParentNode as the parent
+                            of the node created.
+  @param [out] NewPsdNode   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
+AmlCreatePsdNode (
+  IN  AML_PSD_INFO            *PsdInfo,
+  IN  AML_NODE_HANDLE         ParentNode    OPTIONAL,
+  OUT AML_OBJECT_NODE_HANDLE  *NewPsdNode   OPTIONAL
+  );
+
 #endif // AML_LIB_H_
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index f63d9fee8c11..f350083b148c 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -1,7 +1,7 @@
 /** @file
   AML Code Generation.
 
-  Copyright (c) 2020 - 2022, Arm Limited. All rights reserved.<BR>
+  Copyright (c) 2020 - 2023, Arm Limited. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
@@ -3849,3 +3849,189 @@ exit_handler:
 
   return Status;
 }
+
+/** Create a _PSD node.
+
+  Creates and optionally adds the following node
+   Name(_PSD, Package()
+   {
+    NumEntries,  // Integer
+    Revision,    // Integer
+    Domain,      // Integer
+    CoordType,   // Integer
+    NumProc,     // Integer
+  })
+
+  Cf. ACPI 6.4, s8.4.6.5 _PSD (P-State Dependency)
+
+  @ingroup CodeGenApis
+
+  @param [in]  PsdInfo      PsdInfo object
+  @param [in]  ParentNode   If provided, set ParentNode as the parent
+                            of the node created.
+  @param [out] NewPsdNode   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
+AmlCreatePsdNode (
+  IN  AML_PSD_INFO            *PsdInfo,
+  IN  AML_NODE_HANDLE         ParentNode    OPTIONAL,
+  OUT AML_OBJECT_NODE_HANDLE  *NewPsdNode   OPTIONAL
+  )
+{
+  EFI_STATUS              Status;
+  AML_OBJECT_NODE_HANDLE  PsdNode;
+  AML_OBJECT_NODE_HANDLE  PsdPackage;
+  AML_OBJECT_NODE_HANDLE  IntegerNode;
+  UINT32                  NumberOfEntries;
+
+  if ((PsdInfo == NULL) ||
+      ((ParentNode == NULL) && (NewPsdNode == NULL)))
+  {
+    Status = EFI_INVALID_PARAMETER;
+    ASSERT_EFI_ERROR (Status);
+    return Status;
+  }
+
+  // Revision 3 per ACPI 6.4 specification
+  if (PsdInfo->Revision == EFI_ACPI_6_4_AML_PSD_REVISION_V0) {
+    // NumEntries 5 per ACPI 6.4 specification
+    NumberOfEntries = 5;
+  } else {
+    Status = EFI_INVALID_PARAMETER;
+    ASSERT_EFI_ERROR (Status);
+    return Status;
+  }
+
+  if (((PsdInfo->CoordType != EFI_ACPI_6_4_AML_STATE_COORD_TYPE_SW_ALL) &&
+       (PsdInfo->CoordType != EFI_ACPI_6_4_AML_STATE_COORD_TYPE_SW_ANY) &&
+       (PsdInfo->CoordType != EFI_ACPI_6_4_AML_STATE_COORD_TYPE_HW_ALL)) ||
+      (PsdInfo->NumProc == 0))
+  {
+    Status = EFI_INVALID_PARAMETER;
+    ASSERT_EFI_ERROR (Status);
+    return Status;
+  }
+
+  Status = AmlCodeGenNamePackage ("_PSD", NULL, &PsdNode);
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    return Status;
+  }
+
+  // Get the Package object node of the _PSD node,
+  // which is the 2nd fixed argument (i.e. index 1).
+  PsdPackage = (AML_OBJECT_NODE_HANDLE)AmlGetFixedArgument (
+                                         PsdNode,
+                                         EAmlParseIndexTerm1
+                                         );
+  if ((PsdPackage == NULL)                                              ||
+      (AmlGetNodeType ((AML_NODE_HANDLE)PsdPackage) != EAmlNodeObject)  ||
+      (!AmlNodeHasOpCode (PsdPackage, AML_PACKAGE_OP, 0)))
+  {
+    Status = EFI_INVALID_PARAMETER;
+    ASSERT_EFI_ERROR (Status);
+    goto error_handler;
+  }
+
+  // NumEntries
+  Status = AmlCodeGenInteger (NumberOfEntries, &IntegerNode);
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    return Status;
+  }
+
+  Status = AmlVarListAddTail (
+             (AML_NODE_HANDLE)PsdPackage,
+             (AML_NODE_HANDLE)IntegerNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    FreePool (IntegerNode);
+    return Status;
+  }
+
+  // Revision
+  Status = AmlCodeGenInteger (PsdInfo->Revision, &IntegerNode);
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    return Status;
+  }
+
+  Status = AmlVarListAddTail (
+             (AML_NODE_HANDLE)PsdPackage,
+             (AML_NODE_HANDLE)IntegerNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    FreePool (IntegerNode);
+    return Status;
+  }
+
+  // Domain
+  Status = AmlCodeGenInteger (PsdInfo->Domain, &IntegerNode);
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    return Status;
+  }
+
+  Status = AmlVarListAddTail (
+             (AML_NODE_HANDLE)PsdPackage,
+             (AML_NODE_HANDLE)IntegerNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    FreePool (IntegerNode);
+    return Status;
+  }
+
+  // CoordType
+  Status = AmlCodeGenInteger (PsdInfo->CoordType, &IntegerNode);
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    return Status;
+  }
+
+  Status = AmlVarListAddTail (
+             (AML_NODE_HANDLE)PsdPackage,
+             (AML_NODE_HANDLE)IntegerNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    FreePool (IntegerNode);
+    return Status;
+  }
+
+  // Num Processors
+  Status = AmlCodeGenInteger (PsdInfo->NumProc, &IntegerNode);
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    return Status;
+  }
+
+  Status = AmlVarListAddTail (
+             (AML_NODE_HANDLE)PsdPackage,
+             (AML_NODE_HANDLE)IntegerNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    FreePool (IntegerNode);
+    return Status;
+  }
+
+  Status = LinkNode (PsdNode, ParentNode, NewPsdNode);
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    goto error_handler;
+  }
+
+  return Status;
+
+error_handler:
+  AmlDeleteTree ((AML_NODE_HANDLE)PsdNode);
+  return Status;
+}
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110044): https://edk2.groups.io/g/devel/message/110044
Mute This Topic: https://groups.io/mt/102175818/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 09/11] DynamicTablesPkg: Generate _PSD in SsdtCpuTopologyGenerator

[PierreGondois]

From: Pierre Gondois <pierre.gondois@arm.com>

Make use of the newly added AmlCreatePsdNode() to generate
_PSD objects.

_PSD objects allow to describe 'performance control, P-state
or CPPC, logical processor dependency', Cf. ACPI 6.4,
s8.4.5.5 _PSD (P-State Dependency).

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 .../SsdtCpuTopologyGenerator.c                | 98 ++++++++++++++++++-
 1 file changed, 97 insertions(+), 1 deletion(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
index 6fb131b66482..9cebf57e8a46 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
@@ -1,7 +1,7 @@
 /** @file
   SSDT Cpu Topology Table Generator.
 
-  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+  Copyright (c) 2021 - 2023, Arm Limited. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @par Reference(s):
@@ -35,6 +35,7 @@ Requirements:
   - EArmObjProcHierarchyInfo (OPTIONAL) along with
   - EArmObjCmRef (OPTIONAL)
   - EArmObjLpiInfo (OPTIONAL)
+  - EArmObjPsdInfo (OPTIONAL)
 */
 
 /** This macro expands to a function that retrieves the GIC
@@ -86,6 +87,16 @@ GET_OBJECT_LIST (
   CM_ARM_CPC_INFO
   );
 
+/**
+  This macro expands to a function that retrieves the PSD
+  information from the Configuration Manager.
+*/
+GET_OBJECT_LIST (
+  EObjNameSpaceArm,
+  EArmObjPsdInfo,
+  CM_ARM_PSD_INFO
+  );
+
 /** Initialize the TokenTable.
 
   One entry should be allocated for each CM_ARM_PROC_HIERARCHY_INFO
@@ -239,6 +250,75 @@ WriteAslName (
   return EFI_SUCCESS;
 }
 
+/** Create and add an _PSD 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 (_PSD, Package()
+      {
+        NumEntries,      // Integer
+        Revision,        // Integer
+        Domain,          // Integer
+        CoordType,       // Integer
+        NumProcessors,   // Integer
+      })
+  }
+
+  @param [in]  Generator              The SSDT Cpu Topology generator.
+  @param [in]  CfgMgrProtocol         Pointer to the Configuration Manager
+                                      Protocol Interface.
+  @param [in]  GicCInfo               Pointer to the CM_ARM_GICC_INFO object
+                                      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
+CreateAmlPsdNode (
+  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_PSD_INFO  *PsdInfo;
+
+  Status = GetEArmObjPsdInfo (
+             CfgMgrProtocol,
+             GicCInfo->PsdToken,
+             &PsdInfo,
+             NULL
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    return Status;
+  }
+
+  Status = AmlCreatePsdNode (
+             PsdInfo,
+             Node,
+             NULL
+             );
+  ASSERT_EFI_ERROR (Status);
+  return Status;
+}
+
 /** Create and add an _CPC Node to Cpu Node.
 
   For instance, transform an AML node from:
@@ -684,6 +764,14 @@ CreateAmlCpuFromProcHierarchy (
     }
   }
 
+  if (GicCInfo->PsdToken != CM_NULL_TOKEN) {
+    Status = CreateAmlPsdNode (Generator, CfgMgrProtocol, GicCInfo, CpuNode);
+    if (EFI_ERROR (Status)) {
+      ASSERT_EFI_ERROR (Status);
+      return Status;
+    }
+  }
+
   // If a CPC info is associated with the
   // GicCinfo, create an _CPC method returning them.
   if (GicCInfo->CpcToken != CM_NULL_TOKEN) {
@@ -1126,6 +1214,14 @@ CreateTopologyFromGicC (
       break;
     }
 
+    if (GicCInfo->PsdToken != CM_NULL_TOKEN) {
+      Status = CreateAmlPsdNode (Generator, CfgMgrProtocol, GicCInfo, CpuNode);
+      if (EFI_ERROR (Status)) {
+        ASSERT_EFI_ERROR (Status);
+        return Status;
+      }
+    }
+
     // If a CPC info is associated with the
     // GicCinfo, create an _CPC method returning them.
     if (GicCInfo->CpcToken != CM_NULL_TOKEN) {
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110045): https://edk2.groups.io/g/devel/message/110045
Mute This Topic: https://groups.io/mt/102175820/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 10/11] DynamicTablesPkg: Add ArmScmiInfoLib

[PierreGondois]

From: Pierre Gondois <pierre.gondois@arm.com>

The SCP holds some power information that could be advertised
through the _CPC object. The communication with the SCP is done
through SCMI protocols (c.f. ArmScmiDxe).

Use the SCMI protocols to query information and feed it to
the DynamicTablesPkg.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 DynamicTablesPkg/DynamicTables.dsc.inc        |   1 +
 DynamicTablesPkg/DynamicTablesPkg.dec         |   3 +
 DynamicTablesPkg/DynamicTablesPkg.dsc         |   1 +
 .../Include/Library/ArmScmiInfoLib.h          |  33 ++
 .../Library/ArmScmiInfoLib/ArmScmiInfoLib.c   | 294 ++++++++++++++++++
 .../Library/ArmScmiInfoLib/ArmScmiInfoLib.inf |  31 ++
 6 files changed, 363 insertions(+)
 create mode 100644 DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h
 create mode 100644 DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c
 create mode 100644 DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf

diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc b/DynamicTablesPkg/DynamicTables.dsc.inc
index 9d4312c4e87d..be40ebc4b472 100644
--- a/DynamicTablesPkg/DynamicTables.dsc.inc
+++ b/DynamicTablesPkg/DynamicTables.dsc.inc
@@ -15,6 +15,7 @@ [BuildOptions]
 [LibraryClasses.common]
   AcpiHelperLib|DynamicTablesPkg/Library/Common/AcpiHelperLib/AcpiHelperLib.inf
   AmlLib|DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
+  ArmScmiInfoLib|DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
   SsdtPcieSupportLib|DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
   SsdtSerialPortFixupLib|DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.inf
   TableHelperLib|DynamicTablesPkg/Library/Common/TableHelperLib/TableHelperLib.inf
diff --git a/DynamicTablesPkg/DynamicTablesPkg.dec b/DynamicTablesPkg/DynamicTablesPkg.dec
index cfbcbb9569f1..26498e5fec53 100644
--- a/DynamicTablesPkg/DynamicTablesPkg.dec
+++ b/DynamicTablesPkg/DynamicTablesPkg.dec
@@ -42,6 +42,9 @@ [LibraryClasses]
   ##  @libraryclass  Defines a set of SMBIOS string helper methods.
   SmbiosStringTableLib|Include/Library/SmbiosStringTableLib.h
 
+  ##  @libraryclass  Defines a set of APIs to populate CmObj using SCMI.
+  ArmScmiInfoLib|Include/Library/ArmScmiInfoLib.h
+
 [Protocols]
   # Configuration Manager Protocol GUID
   gEdkiiConfigurationManagerProtocolGuid = { 0xd85a4835, 0x5a82, 0x4894, { 0xac, 0x2, 0x70, 0x6f, 0x43, 0xd5, 0x97, 0x8e } }
diff --git a/DynamicTablesPkg/DynamicTablesPkg.dsc b/DynamicTablesPkg/DynamicTablesPkg.dsc
index bd5084a9008f..6ea86c9efdb0 100644
--- a/DynamicTablesPkg/DynamicTablesPkg.dsc
+++ b/DynamicTablesPkg/DynamicTablesPkg.dsc
@@ -39,6 +39,7 @@ [LibraryClasses.ARM, LibraryClasses.AARCH64]
   PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
 
 [Components.common]
+  DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
   DynamicTablesPkg/Library/Common/AcpiHelperLib/AcpiHelperLib.inf
   DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
   DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
diff --git a/DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h b/DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h
new file mode 100644
index 000000000000..8d3fb31df13c
--- /dev/null
+++ b/DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h
@@ -0,0 +1,33 @@
+/** @file
+  Arm SCMI Info Library.
+
+  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef ARM_SCMI_INFO_LIB_H_
+#define ARM_SCMI_INFO_LIB_H_
+
+#include <ConfigurationManagerObject.h>
+
+/** Populate a AML_CPC_INFO object based on SCMI information.
+
+  @param[in]  DomainId    Identifier for the performance domain.
+  @param[out] CpcInfo     If success, this structure was populated from
+                          information queried to the SCP.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_DEVICE_ERROR        Device error.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_TIMEOUT             Time out.
+  @retval EFI_UNSUPPORTED         Unsupported.
+**/
+EFI_STATUS
+EFIAPI
+ArmScmiInfoGetFastChannel (
+  IN  UINT32        DomainId,
+  OUT AML_CPC_INFO  *CpcInfo
+  );
+
+#endif // ARM_SCMI_INFO_LIB_H_
diff --git a/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c
new file mode 100644
index 000000000000..c23bff63bb6f
--- /dev/null
+++ b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c
@@ -0,0 +1,294 @@
+/** @file
+  Arm SCMI Info Library.
+
+  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.<BR>
+
+  Arm Functional Fixed Hardware Specification:
+  - https://developer.arm.com/documentation/den0048/latest/
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Library/AcpiLib.h>
+#include <Library/ArmScmiInfoLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Protocol/ArmScmi.h>
+#include <Protocol/ArmScmiPerformanceProtocol.h>
+
+/** Arm FFH registers
+
+  Cf. Arm Functional Fixed Hardware Specification
+  s3.2 Performance management and Collaborative Processor Performance Control
+*/
+#define ARM_FFH_DELIVERED_PERF_COUNTER_REGISTER  0x0
+#define ARM_FFH_REFERENCE_PERF_COUNTER_REGISTER  0x1
+
+/// Arm SCMI performance protocol.
+STATIC SCMI_PERFORMANCE_PROTOCOL  *ScmiPerfProtocol;
+
+/** Arm SCMI Info Library constructor.
+
+  @param  ImageHandle   Image of the loaded driver.
+  @param  SystemTable   Pointer to the System Table.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_DEVICE_ERROR        Device error.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_NOT_FOUND           Not Found
+  @retval EFI_TIMEOUT             Timeout.
+  @retval EFI_UNSUPPORTED         Unsupported.
+**/
+EFI_STATUS
+EFIAPI
+ArmScmiInfoLibConstructor (
+  IN  EFI_HANDLE        ImageHandle,
+  IN  EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS  Status;
+  UINT32      Version;
+
+  Status = gBS->LocateProtocol (
+                  &gArmScmiPerformanceProtocolGuid,
+                  NULL,
+                  (VOID **)&ScmiPerfProtocol
+                  );
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  Status = ScmiPerfProtocol->GetVersion (ScmiPerfProtocol, &Version);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  // FastChannels were added in SCMI v2.0 spec.
+  if (Version < PERFORMANCE_PROTOCOL_VERSION_V2) {
+    DEBUG ((DEBUG_ERROR, "ArmScmiInfoLib requires SCMI version > 2.0\n"));
+    return EFI_UNSUPPORTED;
+  }
+
+  return Status;
+}
+
+/** Get the OPPs/performance states of a power domain.
+
+  This function is a wrapper around the SCMI PERFORMANCE_DESCRIBE_LEVELS
+  command. The list of discrete performance states is returned in a buffer
+  that must be freed by the caller.
+
+  @param[in]  DomainId        Identifier for the performance domain.
+  @param[out] LevelArray      If success, pointer to the list of list of
+                              performance state. This memory must be freed by
+                              the caller.
+  @param[out] LevelArrayCount If success, contains the number of states in
+                              LevelArray.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_DEVICE_ERROR        Device error.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_TIMEOUT             Time out.
+  @retval EFI_UNSUPPORTED         Unsupported.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+ArmScmiInfoDescribeLevels (
+  IN  UINT32                  DomainId,
+  OUT SCMI_PERFORMANCE_LEVEL  **LevelArray,
+  OUT UINT32                  *LevelArrayCount
+  )
+{
+  EFI_STATUS              Status;
+  SCMI_PERFORMANCE_LEVEL  *Array;
+  UINT32                  Count;
+  UINT32                  Size;
+
+  if ((ScmiPerfProtocol == NULL)  ||
+      (LevelArray == NULL)  ||
+      (LevelArrayCount == NULL))
+  {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  // First call to get the number of levels.
+  Size   = 0;
+  Status = ScmiPerfProtocol->DescribeLevels (
+                               ScmiPerfProtocol,
+                               DomainId,
+                               &Count,
+                               &Size,
+                               NULL
+                               );
+  if (Status != EFI_BUFFER_TOO_SMALL) {
+    // EFI_SUCCESS is not a valid option.
+    if (Status == EFI_SUCCESS) {
+      return EFI_INVALID_PARAMETER;
+    } else {
+      return Status;
+    }
+  }
+
+  Array = AllocateZeroPool (Size);
+  if (Array == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  // Second call to get the descriptions of the levels.
+  Status = ScmiPerfProtocol->DescribeLevels (
+                               ScmiPerfProtocol,
+                               DomainId,
+                               &Count,
+                               &Size,
+                               Array
+                               );
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  *LevelArray      = Array;
+  *LevelArrayCount = Count;
+
+  return Status;
+}
+
+/** Populate a AML_CPC_INFO object based on SCMI information.
+
+  @param[in]  DomainId    Identifier for the performance domain.
+  @param[out] CpcInfo     If success, this structure was populated from
+                          information queried to the SCP.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_DEVICE_ERROR        Device error.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_TIMEOUT             Time out.
+  @retval EFI_UNSUPPORTED         Unsupported.
+**/
+EFI_STATUS
+EFIAPI
+ArmScmiInfoGetFastChannel (
+  IN  UINT32        DomainId,
+  OUT AML_CPC_INFO  *CpcInfo
+  )
+{
+  EFI_STATUS                          Status;
+  SCMI_PERFORMANCE_FASTCHANNEL        FcLevelGet;
+  SCMI_PERFORMANCE_FASTCHANNEL        FcLimitsSet;
+  SCMI_PERFORMANCE_DOMAIN_ATTRIBUTES  DomainAttributes;
+
+  SCMI_PERFORMANCE_LEVEL  *LevelArray;
+  UINT32                  LevelCount;
+
+  UINT64  FcLevelGetAddr;
+  UINT64  FcLimitsMaxSetAddr;
+  UINT64  FcLimitsMinSetAddr;
+
+  if ((ScmiPerfProtocol == NULL)  ||
+      (CpcInfo == NULL))
+  {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  Status = ScmiPerfProtocol->DescribeFastchannel (
+                               ScmiPerfProtocol,
+                               DomainId,
+                               ScmiMessageIdPerformanceLevelSet,
+                               &FcLevelGet
+                               );
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  Status = ScmiPerfProtocol->DescribeFastchannel (
+                               ScmiPerfProtocol,
+                               DomainId,
+                               ScmiMessageIdPerformanceLimitsSet,
+                               &FcLimitsSet
+                               );
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  Status = ScmiPerfProtocol->GetDomainAttributes (
+                               ScmiPerfProtocol,
+                               DomainId,
+                               &DomainAttributes
+                               );
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  Status = ArmScmiInfoDescribeLevels (DomainId, &LevelArray, &LevelCount);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  /* Do some safety checks.
+     Only support FastChannels (and not doorbells) as this is
+     the only mechanism supported by SCP.
+     FcLimits[Get|Set] require 2 UINT32 values (max, then min) and
+     FcLimits[Get|Set] require 1 UINT32 value (level).
+  */
+  if ((FcLevelGet.ChanSize != sizeof (UINT32))  ||
+      ((FcLevelGet.Attributes & SCMI_PERF_FC_ATTRIB_HAS_DOORBELL) ==
+       SCMI_PERF_FC_ATTRIB_HAS_DOORBELL) ||
+      (FcLimitsSet.ChanSize != 2 * sizeof (UINT32)) ||
+      ((FcLimitsSet.Attributes & SCMI_PERF_FC_ATTRIB_HAS_DOORBELL) ==
+       SCMI_PERF_FC_ATTRIB_HAS_DOORBELL))
+  {
+    Status = EFI_INVALID_PARAMETER;
+    goto exit_handler;
+  }
+
+  FcLevelGetAddr = ((UINT64)FcLevelGet.ChanAddrHigh << 32) |
+                   FcLevelGet.ChanAddrLow;
+  FcLimitsMaxSetAddr = ((UINT64)FcLimitsSet.ChanAddrHigh << 32) |
+                       FcLimitsSet.ChanAddrLow;
+  FcLimitsMinSetAddr = FcLimitsMaxSetAddr + 0x4;
+
+  CpcInfo->Revision                          = EFI_ACPI_6_4_AML_CPC_REVISION_V3;
+  CpcInfo->HighestPerformanceInteger         = LevelArray[LevelCount - 1].Level;
+  CpcInfo->NominalPerformanceInteger         = DomainAttributes.SustainedPerfLevel;
+  CpcInfo->LowestNonlinearPerformanceInteger = LevelArray[0].Level;
+  CpcInfo->LowestPerformanceInteger          = LevelArray[0].Level;
+
+  CpcInfo->DesiredPerformanceRegister.AddressSpaceId    = EFI_ACPI_6_4_SYSTEM_MEMORY;
+  CpcInfo->DesiredPerformanceRegister.RegisterBitWidth  = 32;
+  CpcInfo->DesiredPerformanceRegister.RegisterBitOffset = 0;
+  CpcInfo->DesiredPerformanceRegister.AccessSize        = EFI_ACPI_6_4_DWORD;
+  CpcInfo->DesiredPerformanceRegister.Address           = FcLevelGetAddr;
+
+  CpcInfo->MinimumPerformanceRegister.AddressSpaceId    = EFI_ACPI_6_4_SYSTEM_MEMORY;
+  CpcInfo->MinimumPerformanceRegister.RegisterBitWidth  = 32;
+  CpcInfo->MinimumPerformanceRegister.RegisterBitOffset = 0;
+  CpcInfo->MinimumPerformanceRegister.AccessSize        = EFI_ACPI_6_4_DWORD;
+  CpcInfo->MinimumPerformanceRegister.Address           = FcLimitsMinSetAddr;
+
+  CpcInfo->MaximumPerformanceRegister.AddressSpaceId    = EFI_ACPI_6_4_SYSTEM_MEMORY;
+  CpcInfo->MaximumPerformanceRegister.RegisterBitWidth  = 32;
+  CpcInfo->MaximumPerformanceRegister.RegisterBitOffset = 0;
+  CpcInfo->MaximumPerformanceRegister.AccessSize        = EFI_ACPI_6_4_DWORD;
+  CpcInfo->MaximumPerformanceRegister.Address           = FcLimitsMaxSetAddr;
+
+  CpcInfo->ReferencePerformanceCounterRegister.AddressSpaceId    = EFI_ACPI_6_4_FUNCTIONAL_FIXED_HARDWARE;
+  CpcInfo->ReferencePerformanceCounterRegister.RegisterBitWidth  = 0x40;
+  CpcInfo->ReferencePerformanceCounterRegister.RegisterBitOffset = 0;
+  CpcInfo->ReferencePerformanceCounterRegister.AccessSize        = ARM_FFH_REFERENCE_PERF_COUNTER_REGISTER;
+  CpcInfo->ReferencePerformanceCounterRegister.Address           = 0x4;
+
+  CpcInfo->DeliveredPerformanceCounterRegister.AddressSpaceId    = EFI_ACPI_6_4_FUNCTIONAL_FIXED_HARDWARE;
+  CpcInfo->DeliveredPerformanceCounterRegister.RegisterBitWidth  = 0x40;
+  CpcInfo->DeliveredPerformanceCounterRegister.RegisterBitOffset = 0;
+  CpcInfo->DeliveredPerformanceCounterRegister.AccessSize        = ARM_FFH_DELIVERED_PERF_COUNTER_REGISTER;
+  CpcInfo->DeliveredPerformanceCounterRegister.Address           = 0x4;
+
+  // SCMI should advertise performance values on a unified scale. So frequency
+  // values are not available. LowestFrequencyInteger and
+  // NominalFrequencyInteger are populated in the ConfigurationManager.
+
+exit_handler:
+  FreePool (LevelArray);
+  return Status;
+}
diff --git a/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
new file mode 100644
index 000000000000..aad3f0fa7b83
--- /dev/null
+++ b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
@@ -0,0 +1,31 @@
+## @file
+#  Arm SCMI Info Library.
+#
+#  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+  INF_VERSION    = 0x0001001B
+  BASE_NAME      = ArmScmiInfoLib
+  FILE_GUID      = 1A7CDB04-9FFC-40DA-A87C-A5ACADAF8136
+  VERSION_STRING = 1.0
+  MODULE_TYPE    = DXE_DRIVER
+  LIBRARY_CLASS  = ArmScmiInfoLib
+  CONSTRUCTOR    = ArmScmiInfoLibConstructor
+
+[Sources]
+  ArmScmiInfoLib.c
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  DynamicTablesPkg/DynamicTablesPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdePkg/MdePkg.dec
+
+[Protocols]
+  gArmScmiPerformanceProtocolGuid   ## CONSUMES
+
+[Depex]
+  gArmScmiPerformanceProtocolGuid
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110046): https://edk2.groups.io/g/devel/message/110046
Mute This Topic: https://groups.io/mt/102175821/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 11/11] DynamicTablesPkg: Remove check for _CPC field

[PierreGondois]

From: Pierre Gondois <pierre.gondois@arm.com>

When generating _CPC objects, some fields are mandatory.
Some fields cannot be supported by a the Juno platform, which is used
to test the _CPC generation. Therefore, don't prevent from generating
_CPC objects if the fields below are missing, and let the OS handle
the missing information.

_CPC fields that are exempted from checks:
- PerformanceLimitedRegister
- ReferencePerformanceCounterRegister
- DeliveredPerformanceCounterRegister

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index f350083b148c..423e64f12606 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -3531,6 +3531,11 @@ AmlCreateCpcNode (
     return EFI_INVALID_PARAMETER;
   }
 
+  // The following fields are theoretically mandatory, but not supported
+  // by some platforms. Don't check them:
+  // - PerformanceLimitedRegister
+  // - ReferencePerformanceCounterRegister
+  // - DeliveredPerformanceCounterRegister
   if ((IsNullGenericAddress (&CpcInfo->HighestPerformanceBuffer) &&
        (CpcInfo->HighestPerformanceInteger == 0)) ||
       (IsNullGenericAddress (&CpcInfo->NominalPerformanceBuffer) &&
@@ -3539,13 +3544,19 @@ AmlCreateCpcNode (
        (CpcInfo->LowestNonlinearPerformanceInteger == 0)) ||
       (IsNullGenericAddress (&CpcInfo->LowestPerformanceBuffer) &&
        (CpcInfo->LowestPerformanceInteger == 0)) ||
-      IsNullGenericAddress (&CpcInfo->DesiredPerformanceRegister) ||
-      IsNullGenericAddress (&CpcInfo->ReferencePerformanceCounterRegister) ||
-      IsNullGenericAddress (&CpcInfo->DeliveredPerformanceCounterRegister) ||
-      IsNullGenericAddress (&CpcInfo->PerformanceLimitedRegister))
+      IsNullGenericAddress (&CpcInfo->DesiredPerformanceRegister))
   {
     ASSERT (0);
     return EFI_INVALID_PARAMETER;
+  } else if ((IsNullGenericAddress (&CpcInfo->HighestPerformanceBuffer) &&
+              (CpcInfo->HighestPerformanceInteger == 0)) ||
+             (IsNullGenericAddress (&CpcInfo->NominalPerformanceBuffer) &&
+              (CpcInfo->NominalPerformanceInteger == 0)))
+  {
+    DEBUG ((
+      DEBUG_WARN,
+      "Missing Reference|Delivered performance field in _CPC object\n"
+      ));
   }
 
   CpcPackage = NULL;
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110047): https://edk2.groups.io/g/devel/message/110047
Mute This Topic: https://groups.io/mt/102175822/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 01/11] ArmPkg/ArmScmiDxe: Rename PERFORMANCE_PROTOCOL_VERSION

[Leif Lindholm]

On Wed, Oct 25, 2023 at 13:25:30 +0200, pierre.gondois@arm.com wrote:
> From: Pierre Gondois <pierre.gondois@arm.com>
> 
> Rename PERFORMANCE_PROTOCOL_VERSION to reflect the different
> versions of the protocol. The macro is neither used in edk2 nor
> in edk2-platforms.

OK, so slight nitpick, but mainly because it parses a bit weirdly...
*Will* it be used after this series is merged, or is this an update
for completeness?

> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>  ArmPkg/Include/Library/ArmLib.h                     |  1 +
>  .../Include/Protocol/ArmScmiPerformanceProtocol.h   | 13 ++++++++-----
>  2 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
> index 0169dbc1092c..7b2b2238fed9 100644
> --- a/ArmPkg/Include/Library/ArmLib.h
> +++ b/ArmPkg/Include/Library/ArmLib.h
> @@ -780,6 +780,7 @@ EFIAPI
>  ArmHasVhe (
>    VOID
>    );
> +
>  #endif // MDE_CPU_AARCH64
>  
>  #ifdef MDE_CPU_ARM
> diff --git a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
> index 7e548e4765c2..8e8e05d5a5f6 100644
> --- a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
> +++ b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
> @@ -1,12 +1,12 @@
>  /** @file
>  
> -  Copyright (c) 2017-2021, Arm Limited. All rights reserved.
> +  Copyright (c) 2017-2023, Arm Limited. All rights reserved.
>  
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>  
> -  System Control and Management Interface V1.0
> -    http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/
> -    DEN0056A_System_Control_and_Management_Interface.pdf
> +  System Control and Management Interface, latest version:

I see this as a pattern throughout the series.
But this statement will at some point become untrue; this
implementation is written against a specific version. I think this
version shold be reflected in the comment. (And that applies
throughout the series.)

> +  - https://developer.arm.com/documentation/den0056/latest/

But I think the above is the most useful link.

/
    Leif

> +
>  **/
>  
>  #ifndef ARM_SCMI_PERFORMANCE_PROTOCOL_H_
> @@ -14,7 +14,10 @@
>  
>  #include <Protocol/ArmScmi.h>
>  
> -#define PERFORMANCE_PROTOCOL_VERSION  0x10000
> +/// Arm Scmi performance protocol versions.
> +#define PERFORMANCE_PROTOCOL_VERSION_V1  0x10000
> +#define PERFORMANCE_PROTOCOL_VERSION_V2  0x20000
> +#define PERFORMANCE_PROTOCOL_VERSION_V3  0x30000
>  
>  #define ARM_SCMI_PERFORMANCE_PROTOCOL_GUID  { \
>    0x9b8ba84, 0x3dd3, 0x49a6, {0xa0, 0x5a, 0x31, 0x34, 0xa5, 0xf0, 0x7b, 0xad} \
> -- 
> 2.25.1
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110088): https://edk2.groups.io/g/devel/message/110088
Mute This Topic: https://groups.io/mt/102175810/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 02/11] ArmPkg/ArmScmiDxe: Add PERFORMANCE_DESCRIBE_FASTCHANNEL support

[Leif Lindholm]

On Wed, Oct 25, 2023 at 13:25:31 +0200, PierreGondois wrote:
> From: Pierre Gondois <pierre.gondois@arm.com>

Sidebar: I think if you correct your name in your git config, the
above tag will drop out.

> The PERFORMANCE_DESCRIBE_FASTCHANNEL Scmi command is available
> since SCMI v2.0 and allows to query information about the supported
> fast-channels of the Scmi performance protocol.
> Add support for this command.
> 
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>  .../ArmScmiDxe/ScmiPerformanceProtocol.c      | 80 +++++++++++++++--
>  .../Protocol/ArmScmiPerformanceProtocol.h     | 90 ++++++++++++++++---
>  2 files changed, 155 insertions(+), 15 deletions(-)
> 
> diff --git a/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c b/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
> index 0f89808fbdf9..1d87339209fd 100644
> --- a/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
> +++ b/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
> @@ -1,12 +1,12 @@
>  /** @file
>  
> -  Copyright (c) 2017-2021, Arm Limited. All rights reserved.<BR>
> +  Copyright (c) 2017-2023, Arm Limited. All rights reserved.<BR>
>  
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>  
> -  System Control and Management Interface V1.0
> -    http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/
> -    DEN0056A_System_Control_and_Management_Interface.pdf
> +  System Control and Management Interface, latest version:
> +  - https://developer.arm.com/documentation/den0056/latest/
> +
>  **/
>  
>  #include <Library/BaseMemoryLib.h>
> @@ -416,6 +416,75 @@ PerformanceLevelGet (
>    return EFI_SUCCESS;
>  }
>  
> +/** Discover the attributes of the FastChannel for the specified
> +    performance domain and the specified message.
> +
> +  @param[in]  This        A Pointer to SCMI_PERFORMANCE_PROTOCOL Instance.
> +  @param[in]  DomainId    Identifier for the performance domain.
> +  @param[in]  MessageId   Message Id of the FastChannel to discover.
> +                          Must be one of:
> +                           - PERFORMANCE_LIMITS_SET
> +                           - PERFORMANCE_LIMITS_GET
> +                           - PERFORMANCE_LEVEL_SET
> +                           - PERFORMANCE_LEVEL_GET
> +  @param[out] FastChannel If success, contains the FastChannel description.
> +
> +  @retval EFI_SUCCESS             Performance level got successfully.
> +  @retval EFI_DEVICE_ERROR        SCP returns an SCMI error.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +  @retval EFI_TIMEOUT             Time out.
> +  @retval EFI_UNSUPPORTED         Unsupported.
> +**/
> +EFI_STATUS
> +DescribeFastchannel (
> +  IN  SCMI_PERFORMANCE_PROTOCOL     *This,
> +  IN  UINT32                        DomainId,
> +  IN  SCMI_MESSAGE_ID_PERFORMANCE   MessageId,
> +  OUT SCMI_PERFORMANCE_FASTCHANNEL  *FastChannel
> +  )
> +{
> +  EFI_STATUS    Status;
> +  SCMI_COMMAND  Cmd;
> +  UINT32        PayloadLength;
> +  UINT32        *ReturnValues;
> +  UINT32        *MessageParams;
> +
> +  if ((This == NULL)  ||
> +      (FastChannel == NULL))
> +  {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Status = ScmiCommandGetPayload (&MessageParams);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  *MessageParams++ = DomainId;
> +  *MessageParams   = MessageId;
> +
> +  Cmd.ProtocolId = ScmiProtocolIdPerformance;
> +  Cmd.MessageId  = ScmiMessageIdPerformanceDescribeFastchannel;
> +  PayloadLength  = sizeof (DomainId) + sizeof (MessageId);
> +
> +  Status = ScmiCommandExecute (
> +             &Cmd,
> +             &PayloadLength,
> +             &ReturnValues
> +             );
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  CopyMem (
> +    FastChannel,
> +    ReturnValues,
> +    sizeof (SCMI_PERFORMANCE_FASTCHANNEL)
> +    );
> +
> +  return Status;
> +}
> +
>  // Instance of the SCMI performance management protocol.
>  STATIC CONST SCMI_PERFORMANCE_PROTOCOL  PerformanceProtocol = {
>    PerformanceGetVersion,
> @@ -425,7 +494,8 @@ STATIC CONST SCMI_PERFORMANCE_PROTOCOL  PerformanceProtocol = {
>    PerformanceLimitsSet,
>    PerformanceLimitsGet,
>    PerformanceLevelSet,
> -  PerformanceLevelGet
> +  PerformanceLevelGet,
> +  DescribeFastchannel,
>  };
>  
>  /** Initialize performance management protocol and install on a given Handle.
> diff --git a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
> index 8e8e05d5a5f6..088182945a06 100644
> --- a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
> +++ b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
> @@ -14,7 +14,7 @@
>  
>  #include <Protocol/ArmScmi.h>
>  
> -/// Arm Scmi performance protocol versions.
> +/// Arm SCMI performance protocol versions.
>  #define PERFORMANCE_PROTOCOL_VERSION_V1  0x10000
>  #define PERFORMANCE_PROTOCOL_VERSION_V2  0x20000
>  #define PERFORMANCE_PROTOCOL_VERSION_V3  0x30000
> @@ -79,6 +79,56 @@ typedef struct {
>    UINT32    RangeMin;
>  } SCMI_PERFORMANCE_LIMITS;
>  
> +/// Doorbell Support bit.
> +#define SCMI_PERF_FC_ATTRIB_HAS_DOORBELL  BIT0
> +
> +/// Performance protocol describe fastchannel
> +typedef struct {
> +  /// Attributes.
> +  UINT32    Attributes;
> +
> +  /// Rate limit.
> +  UINT32    RateLimit;
> +
> +  /// Lower 32 bits of the FastChannel address.
> +  UINT32    ChanAddrLow;
> +
> +  /// Higher 32 bits of the FastChannel address.
> +  UINT32    ChanAddrHigh;
> +
> +  /// Size of the FastChannel in bytes.
> +  UINT32    ChanSize;
> +
> +  /// Lower 32 bits of the doorbell address.
> +  UINT32    DoorbellAddrLow;
> +
> +  /// Higher 32 bits of the doorbell address.
> +  UINT32    DoorbellAddrHigh;
> +
> +  /// Mask of lower 32 bits to set when writing to the doorbell register.
> +  UINT32    DoorbellSetMaskLow;
> +
> +  /// Mask of higher 32 bits to set when writing to the doorbell register.
> +  UINT32    DoorbellSetMaskHigh;
> +
> +  /// Mask of lower 32 bits to preserve when writing to the doorbell register.
> +  UINT32    DoorbellPreserveMaskLow;
> +
> +  /// Mask of higher 32 bits to preserve when writing to the doorbell register.
> +  UINT32    DoorbellPreserveMaskHigh;
> +} SCMI_PERFORMANCE_FASTCHANNEL;
> +
> +/// SCMI Message Ids for the Performance Protocol.
> +typedef enum {
> +  ScmiMessageIdPerformanceDomainAttributes    = 0x3,
> +  ScmiMessageIdPerformanceDescribeLevels      = 0x4,
> +  ScmiMessageIdPerformanceLimitsSet           = 0x5,
> +  ScmiMessageIdPerformanceLimitsGet           = 0x6,
> +  ScmiMessageIdPerformanceLevelSet            = 0x7,
> +  ScmiMessageIdPerformanceLevelGet            = 0x8,
> +  ScmiMessageIdPerformanceDescribeFastchannel = 0xB,
> +} SCMI_MESSAGE_ID_PERFORMANCE;

This struct appears to move in the code at the same time as having an
entry added to it. This seems superficially unmotivated.
However it is also moved to inside the pack(1) block, which makes no
sense for an enum.

/
    Leif

> +
>  #pragma pack()
>  
>  /** Return version of the performance management protocol supported by SCP.
> @@ -238,6 +288,34 @@ EFI_STATUS
>    OUT UINT32                    *Level
>    );
>  
> +/** Discover the attributes of the FastChannel for the specified
> +    performance domain and the specified message.
> +
> +  @param[in]  This        A Pointer to SCMI_PERFORMANCE_PROTOCOL Instance.
> +  @param[in]  DomainId    Identifier for the performance domain.
> +  @param[in]  MessageId   Message Id of the FastChannel to discover.
> +                          Must be one of:
> +                           - PERFORMANCE_LIMITS_SET
> +                           - PERFORMANCE_LIMITS_GET
> +                           - PERFORMANCE_LEVEL_SET
> +                           - PERFORMANCE_LEVEL_GET
> +  @param[out] FastChannel If success, contains the FastChannel description.
> +
> +  @retval EFI_SUCCESS             Performance level got successfully.
> +  @retval EFI_DEVICE_ERROR        SCP returns an SCMI error.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +  @retval EFI_TIMEOUT             Time out.
> +  @retval EFI_UNSUPPORTED         Unsupported.
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *SCMI_PERFORMANCE_DESCRIBE_FASTCHANNEL)(
> +  IN  SCMI_PERFORMANCE_PROTOCOL       *This,
> +  IN  UINT32                          DomainId,
> +  IN  SCMI_MESSAGE_ID_PERFORMANCE     MessageId,
> +  OUT SCMI_PERFORMANCE_FASTCHANNEL    *FastChannel
> +  );
> +
>  typedef struct _SCMI_PERFORMANCE_PROTOCOL {
>    SCMI_PERFORMANCE_GET_VERSION              GetVersion;
>    SCMI_PERFORMANCE_GET_ATTRIBUTES           GetProtocolAttributes;
> @@ -247,15 +325,7 @@ typedef struct _SCMI_PERFORMANCE_PROTOCOL {
>    SCMI_PERFORMANCE_LIMITS_GET               LimitsGet;
>    SCMI_PERFORMANCE_LEVEL_SET                LevelSet;
>    SCMI_PERFORMANCE_LEVEL_GET                LevelGet;
> +  SCMI_PERFORMANCE_DESCRIBE_FASTCHANNEL     DescribeFastchannel;
>  } SCMI_PERFORMANCE_PROTOCOL;
>  
> -typedef enum {
> -  ScmiMessageIdPerformanceDomainAttributes = 0x3,
> -  ScmiMessageIdPerformanceDescribeLevels   = 0x4,
> -  ScmiMessageIdPerformanceLimitsSet        = 0x5,
> -  ScmiMessageIdPerformanceLimitsGet        = 0x6,
> -  ScmiMessageIdPerformanceLevelSet         = 0x7,
> -  ScmiMessageIdPerformanceLevelGet         = 0x8,
> -} SCMI_MESSAGE_ID_PERFORMANCE;
> -
>  #endif /* ARM_SCMI_PERFORMANCE_PROTOCOL_H_ */
> -- 
> 2.25.1
> 
> 
> 
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110089): https://edk2.groups.io/g/devel/message/110089
Mute This Topic: https://groups.io/mt/102175812/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 07/11] DynamicTablesPkg: Add PsdToken field to CM_ARM_GICC_INFO object

[Leif Lindholm]

On Wed, Oct 25, 2023 at 13:25:36 +0200, pierre.gondois@arm.com wrote:
> From: Pierre Gondois <pierre.gondois@arm.com>
> 
> The _PSD object (cf. ACPI 6.4, s8.4.5.5 _PSD (P-State Dependency)
> allows to describe CPU's power state dependencies. Add a PsdToken
> field to the CM_ARM_GICC_INFO object so that interdependent CPUs
> can reference the same CM_ARM_PSD_INFO object.
> 
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>  DynamicTablesPkg/Include/ArmNameSpaceObjects.h               | 5 +++++
>  .../Common/TableHelperLib/ConfigurationManagerObjectParser.c | 5 +++--
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> index ddd17fa45b1e..2a0ebe24bd04 100644
> --- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> +++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> @@ -204,6 +204,11 @@ typedef struct CmArmGicCInfo {
>        i.e. a token referencing a CM_ARM_CPC_INFO object.
>    */
>    CM_OBJECT_TOKEN    CpcToken;
> +
> +  /** Optional field: Reference Token for the Psd info of this processor.
> +      i.e. a token referencing a CM_ARM_PSD_INFO object.
> +  */
> +  CM_OBJECT_TOKEN    PsdToken;
>  } CM_ARM_GICC_INFO;
>  
>  /** A structure that describes the
> diff --git a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
> index b3ee12da8c4f..a9f5c95c1039 100644
> --- a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
> +++ b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
> @@ -83,7 +83,8 @@ STATIC CONST CM_OBJ_PARSER  CmArmGicCInfoParser[] = {
>    { "ProximityDomain",               4,                        "0x%x",   NULL },
>    { "ClockDomain",                   4,                        "0x%x",   NULL },
>    { "AffinityFlags",                 4,                        "0x%x",   NULL },
> -  { "CpcToken",                      sizeof (CM_OBJECT_TOKEN), "0x%p",   NULL }
> +  { "CpcToken",                      sizeof (CM_OBJECT_TOKEN), "0x%p",   NULL },
> +  { "PsdToken",                      sizeof (CM_OBJECT_TOKEN), "0x%p",   NULL },
>  };
>  
>  /** A parser for EArmObjGicDInfo.
> @@ -766,7 +767,7 @@ STATIC CONST CM_OBJ_PARSER_ARRAY  ArmNamespaceObjectParser[] = {
>      ARRAY_SIZE (CmArmPccSubspaceType34InfoParser) },
>    { "EArmObjPccSubspaceType5Info",         CmArmPccSubspaceType5InfoParser,
>      ARRAY_SIZE (CmArmPccSubspaceType5InfoParser) },
> -  { "EArmObjCpcInfo",                      CmArmPsdInfoParser,
> +  { "EArmObjPsdInfo",                      CmArmPsdInfoParser,

Can you add something to the commit message about this bit?

/
    Leif

>      ARRAY_SIZE (CmArmPsdInfoParser) },
>    { "EArmObjMax",                          NULL,                                  0                                },
>  };
> -- 
> 2.25.1
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110090): https://edk2.groups.io/g/devel/message/110090
Mute This Topic: https://groups.io/mt/102175817/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 10/11] DynamicTablesPkg: Add ArmScmiInfoLib

[Leif Lindholm]

On Wed, Oct 25, 2023 at 13:25:39 +0200, PierreGondois wrote:
> From: Pierre Gondois <pierre.gondois@arm.com>
> 
> The SCP holds some power information that could be advertised
> through the _CPC object. The communication with the SCP is done
> through SCMI protocols (c.f. ArmScmiDxe).
> 
> Use the SCMI protocols to query information and feed it to
> the DynamicTablesPkg.

Couple of questions:
With a generic name like ArmScmiInfoLib, does is belong in ArmPkg or
MdeModulePkg?

Or if it's more tightly integrated with DynamicTablesPkg (not
blatantly obvious from a quick skim below), should that be reflected
by the library name?

/
    Leif

> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>  DynamicTablesPkg/DynamicTables.dsc.inc        |   1 +
>  DynamicTablesPkg/DynamicTablesPkg.dec         |   3 +
>  DynamicTablesPkg/DynamicTablesPkg.dsc         |   1 +
>  .../Include/Library/ArmScmiInfoLib.h          |  33 ++
>  .../Library/ArmScmiInfoLib/ArmScmiInfoLib.c   | 294 ++++++++++++++++++
>  .../Library/ArmScmiInfoLib/ArmScmiInfoLib.inf |  31 ++
>  6 files changed, 363 insertions(+)
>  create mode 100644 DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h
>  create mode 100644 DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c
>  create mode 100644 DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
> 
> diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc b/DynamicTablesPkg/DynamicTables.dsc.inc
> index 9d4312c4e87d..be40ebc4b472 100644
> --- a/DynamicTablesPkg/DynamicTables.dsc.inc
> +++ b/DynamicTablesPkg/DynamicTables.dsc.inc
> @@ -15,6 +15,7 @@ [BuildOptions]
>  [LibraryClasses.common]
>    AcpiHelperLib|DynamicTablesPkg/Library/Common/AcpiHelperLib/AcpiHelperLib.inf
>    AmlLib|DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
> +  ArmScmiInfoLib|DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
>    SsdtPcieSupportLib|DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
>    SsdtSerialPortFixupLib|DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.inf
>    TableHelperLib|DynamicTablesPkg/Library/Common/TableHelperLib/TableHelperLib.inf
> diff --git a/DynamicTablesPkg/DynamicTablesPkg.dec b/DynamicTablesPkg/DynamicTablesPkg.dec
> index cfbcbb9569f1..26498e5fec53 100644
> --- a/DynamicTablesPkg/DynamicTablesPkg.dec
> +++ b/DynamicTablesPkg/DynamicTablesPkg.dec
> @@ -42,6 +42,9 @@ [LibraryClasses]
>    ##  @libraryclass  Defines a set of SMBIOS string helper methods.
>    SmbiosStringTableLib|Include/Library/SmbiosStringTableLib.h
>  
> +  ##  @libraryclass  Defines a set of APIs to populate CmObj using SCMI.
> +  ArmScmiInfoLib|Include/Library/ArmScmiInfoLib.h
> +
>  [Protocols]
>    # Configuration Manager Protocol GUID
>    gEdkiiConfigurationManagerProtocolGuid = { 0xd85a4835, 0x5a82, 0x4894, { 0xac, 0x2, 0x70, 0x6f, 0x43, 0xd5, 0x97, 0x8e } }
> diff --git a/DynamicTablesPkg/DynamicTablesPkg.dsc b/DynamicTablesPkg/DynamicTablesPkg.dsc
> index bd5084a9008f..6ea86c9efdb0 100644
> --- a/DynamicTablesPkg/DynamicTablesPkg.dsc
> +++ b/DynamicTablesPkg/DynamicTablesPkg.dsc
> @@ -39,6 +39,7 @@ [LibraryClasses.ARM, LibraryClasses.AARCH64]
>    PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
>  
>  [Components.common]
> +  DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
>    DynamicTablesPkg/Library/Common/AcpiHelperLib/AcpiHelperLib.inf
>    DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
>    DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
> diff --git a/DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h b/DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h
> new file mode 100644
> index 000000000000..8d3fb31df13c
> --- /dev/null
> +++ b/DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h
> @@ -0,0 +1,33 @@
> +/** @file
> +  Arm SCMI Info Library.
> +
> +  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#ifndef ARM_SCMI_INFO_LIB_H_
> +#define ARM_SCMI_INFO_LIB_H_
> +
> +#include <ConfigurationManagerObject.h>
> +
> +/** Populate a AML_CPC_INFO object based on SCMI information.
> +
> +  @param[in]  DomainId    Identifier for the performance domain.
> +  @param[out] CpcInfo     If success, this structure was populated from
> +                          information queried to the SCP.
> +
> +  @retval EFI_SUCCESS             Success.
> +  @retval EFI_DEVICE_ERROR        Device error.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +  @retval EFI_TIMEOUT             Time out.
> +  @retval EFI_UNSUPPORTED         Unsupported.
> +**/
> +EFI_STATUS
> +EFIAPI
> +ArmScmiInfoGetFastChannel (
> +  IN  UINT32        DomainId,
> +  OUT AML_CPC_INFO  *CpcInfo
> +  );
> +
> +#endif // ARM_SCMI_INFO_LIB_H_
> diff --git a/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c
> new file mode 100644
> index 000000000000..c23bff63bb6f
> --- /dev/null
> +++ b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c
> @@ -0,0 +1,294 @@
> +/** @file
> +  Arm SCMI Info Library.
> +
> +  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.<BR>
> +
> +  Arm Functional Fixed Hardware Specification:
> +  - https://developer.arm.com/documentation/den0048/latest/
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include <Library/AcpiLib.h>
> +#include <Library/ArmScmiInfoLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Protocol/ArmScmi.h>
> +#include <Protocol/ArmScmiPerformanceProtocol.h>
> +
> +/** Arm FFH registers
> +
> +  Cf. Arm Functional Fixed Hardware Specification
> +  s3.2 Performance management and Collaborative Processor Performance Control
> +*/
> +#define ARM_FFH_DELIVERED_PERF_COUNTER_REGISTER  0x0
> +#define ARM_FFH_REFERENCE_PERF_COUNTER_REGISTER  0x1
> +
> +/// Arm SCMI performance protocol.
> +STATIC SCMI_PERFORMANCE_PROTOCOL  *ScmiPerfProtocol;
> +
> +/** Arm SCMI Info Library constructor.
> +
> +  @param  ImageHandle   Image of the loaded driver.
> +  @param  SystemTable   Pointer to the System Table.
> +
> +  @retval EFI_SUCCESS             Success.
> +  @retval EFI_DEVICE_ERROR        Device error.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +  @retval EFI_NOT_FOUND           Not Found
> +  @retval EFI_TIMEOUT             Timeout.
> +  @retval EFI_UNSUPPORTED         Unsupported.
> +**/
> +EFI_STATUS
> +EFIAPI
> +ArmScmiInfoLibConstructor (
> +  IN  EFI_HANDLE        ImageHandle,
> +  IN  EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  EFI_STATUS  Status;
> +  UINT32      Version;
> +
> +  Status = gBS->LocateProtocol (
> +                  &gArmScmiPerformanceProtocolGuid,
> +                  NULL,
> +                  (VOID **)&ScmiPerfProtocol
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Status = ScmiPerfProtocol->GetVersion (ScmiPerfProtocol, &Version);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  // FastChannels were added in SCMI v2.0 spec.
> +  if (Version < PERFORMANCE_PROTOCOL_VERSION_V2) {
> +    DEBUG ((DEBUG_ERROR, "ArmScmiInfoLib requires SCMI version > 2.0\n"));
> +    return EFI_UNSUPPORTED;
> +  }
> +
> +  return Status;
> +}
> +
> +/** Get the OPPs/performance states of a power domain.
> +
> +  This function is a wrapper around the SCMI PERFORMANCE_DESCRIBE_LEVELS
> +  command. The list of discrete performance states is returned in a buffer
> +  that must be freed by the caller.
> +
> +  @param[in]  DomainId        Identifier for the performance domain.
> +  @param[out] LevelArray      If success, pointer to the list of list of
> +                              performance state. This memory must be freed by
> +                              the caller.
> +  @param[out] LevelArrayCount If success, contains the number of states in
> +                              LevelArray.
> +
> +  @retval EFI_SUCCESS             Success.
> +  @retval EFI_DEVICE_ERROR        Device error.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +  @retval EFI_TIMEOUT             Time out.
> +  @retval EFI_UNSUPPORTED         Unsupported.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +ArmScmiInfoDescribeLevels (
> +  IN  UINT32                  DomainId,
> +  OUT SCMI_PERFORMANCE_LEVEL  **LevelArray,
> +  OUT UINT32                  *LevelArrayCount
> +  )
> +{
> +  EFI_STATUS              Status;
> +  SCMI_PERFORMANCE_LEVEL  *Array;
> +  UINT32                  Count;
> +  UINT32                  Size;
> +
> +  if ((ScmiPerfProtocol == NULL)  ||
> +      (LevelArray == NULL)  ||
> +      (LevelArrayCount == NULL))
> +  {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  // First call to get the number of levels.
> +  Size   = 0;
> +  Status = ScmiPerfProtocol->DescribeLevels (
> +                               ScmiPerfProtocol,
> +                               DomainId,
> +                               &Count,
> +                               &Size,
> +                               NULL
> +                               );
> +  if (Status != EFI_BUFFER_TOO_SMALL) {
> +    // EFI_SUCCESS is not a valid option.
> +    if (Status == EFI_SUCCESS) {
> +      return EFI_INVALID_PARAMETER;
> +    } else {
> +      return Status;
> +    }
> +  }
> +
> +  Array = AllocateZeroPool (Size);
> +  if (Array == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  // Second call to get the descriptions of the levels.
> +  Status = ScmiPerfProtocol->DescribeLevels (
> +                               ScmiPerfProtocol,
> +                               DomainId,
> +                               &Count,
> +                               &Size,
> +                               Array
> +                               );
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  *LevelArray      = Array;
> +  *LevelArrayCount = Count;
> +
> +  return Status;
> +}
> +
> +/** Populate a AML_CPC_INFO object based on SCMI information.
> +
> +  @param[in]  DomainId    Identifier for the performance domain.
> +  @param[out] CpcInfo     If success, this structure was populated from
> +                          information queried to the SCP.
> +
> +  @retval EFI_SUCCESS             Success.
> +  @retval EFI_DEVICE_ERROR        Device error.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +  @retval EFI_TIMEOUT             Time out.
> +  @retval EFI_UNSUPPORTED         Unsupported.
> +**/
> +EFI_STATUS
> +EFIAPI
> +ArmScmiInfoGetFastChannel (
> +  IN  UINT32        DomainId,
> +  OUT AML_CPC_INFO  *CpcInfo
> +  )
> +{
> +  EFI_STATUS                          Status;
> +  SCMI_PERFORMANCE_FASTCHANNEL        FcLevelGet;
> +  SCMI_PERFORMANCE_FASTCHANNEL        FcLimitsSet;
> +  SCMI_PERFORMANCE_DOMAIN_ATTRIBUTES  DomainAttributes;
> +
> +  SCMI_PERFORMANCE_LEVEL  *LevelArray;
> +  UINT32                  LevelCount;
> +
> +  UINT64  FcLevelGetAddr;
> +  UINT64  FcLimitsMaxSetAddr;
> +  UINT64  FcLimitsMinSetAddr;
> +
> +  if ((ScmiPerfProtocol == NULL)  ||
> +      (CpcInfo == NULL))
> +  {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Status = ScmiPerfProtocol->DescribeFastchannel (
> +                               ScmiPerfProtocol,
> +                               DomainId,
> +                               ScmiMessageIdPerformanceLevelSet,
> +                               &FcLevelGet
> +                               );
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Status = ScmiPerfProtocol->DescribeFastchannel (
> +                               ScmiPerfProtocol,
> +                               DomainId,
> +                               ScmiMessageIdPerformanceLimitsSet,
> +                               &FcLimitsSet
> +                               );
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Status = ScmiPerfProtocol->GetDomainAttributes (
> +                               ScmiPerfProtocol,
> +                               DomainId,
> +                               &DomainAttributes
> +                               );
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Status = ArmScmiInfoDescribeLevels (DomainId, &LevelArray, &LevelCount);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  /* Do some safety checks.
> +     Only support FastChannels (and not doorbells) as this is
> +     the only mechanism supported by SCP.
> +     FcLimits[Get|Set] require 2 UINT32 values (max, then min) and
> +     FcLimits[Get|Set] require 1 UINT32 value (level).
> +  */
> +  if ((FcLevelGet.ChanSize != sizeof (UINT32))  ||
> +      ((FcLevelGet.Attributes & SCMI_PERF_FC_ATTRIB_HAS_DOORBELL) ==
> +       SCMI_PERF_FC_ATTRIB_HAS_DOORBELL) ||
> +      (FcLimitsSet.ChanSize != 2 * sizeof (UINT32)) ||
> +      ((FcLimitsSet.Attributes & SCMI_PERF_FC_ATTRIB_HAS_DOORBELL) ==
> +       SCMI_PERF_FC_ATTRIB_HAS_DOORBELL))
> +  {
> +    Status = EFI_INVALID_PARAMETER;
> +    goto exit_handler;
> +  }
> +
> +  FcLevelGetAddr = ((UINT64)FcLevelGet.ChanAddrHigh << 32) |
> +                   FcLevelGet.ChanAddrLow;
> +  FcLimitsMaxSetAddr = ((UINT64)FcLimitsSet.ChanAddrHigh << 32) |
> +                       FcLimitsSet.ChanAddrLow;
> +  FcLimitsMinSetAddr = FcLimitsMaxSetAddr + 0x4;
> +
> +  CpcInfo->Revision                          = EFI_ACPI_6_4_AML_CPC_REVISION_V3;
> +  CpcInfo->HighestPerformanceInteger         = LevelArray[LevelCount - 1].Level;
> +  CpcInfo->NominalPerformanceInteger         = DomainAttributes.SustainedPerfLevel;
> +  CpcInfo->LowestNonlinearPerformanceInteger = LevelArray[0].Level;
> +  CpcInfo->LowestPerformanceInteger          = LevelArray[0].Level;
> +
> +  CpcInfo->DesiredPerformanceRegister.AddressSpaceId    = EFI_ACPI_6_4_SYSTEM_MEMORY;
> +  CpcInfo->DesiredPerformanceRegister.RegisterBitWidth  = 32;
> +  CpcInfo->DesiredPerformanceRegister.RegisterBitOffset = 0;
> +  CpcInfo->DesiredPerformanceRegister.AccessSize        = EFI_ACPI_6_4_DWORD;
> +  CpcInfo->DesiredPerformanceRegister.Address           = FcLevelGetAddr;
> +
> +  CpcInfo->MinimumPerformanceRegister.AddressSpaceId    = EFI_ACPI_6_4_SYSTEM_MEMORY;
> +  CpcInfo->MinimumPerformanceRegister.RegisterBitWidth  = 32;
> +  CpcInfo->MinimumPerformanceRegister.RegisterBitOffset = 0;
> +  CpcInfo->MinimumPerformanceRegister.AccessSize        = EFI_ACPI_6_4_DWORD;
> +  CpcInfo->MinimumPerformanceRegister.Address           = FcLimitsMinSetAddr;
> +
> +  CpcInfo->MaximumPerformanceRegister.AddressSpaceId    = EFI_ACPI_6_4_SYSTEM_MEMORY;
> +  CpcInfo->MaximumPerformanceRegister.RegisterBitWidth  = 32;
> +  CpcInfo->MaximumPerformanceRegister.RegisterBitOffset = 0;
> +  CpcInfo->MaximumPerformanceRegister.AccessSize        = EFI_ACPI_6_4_DWORD;
> +  CpcInfo->MaximumPerformanceRegister.Address           = FcLimitsMaxSetAddr;
> +
> +  CpcInfo->ReferencePerformanceCounterRegister.AddressSpaceId    = EFI_ACPI_6_4_FUNCTIONAL_FIXED_HARDWARE;
> +  CpcInfo->ReferencePerformanceCounterRegister.RegisterBitWidth  = 0x40;
> +  CpcInfo->ReferencePerformanceCounterRegister.RegisterBitOffset = 0;
> +  CpcInfo->ReferencePerformanceCounterRegister.AccessSize        = ARM_FFH_REFERENCE_PERF_COUNTER_REGISTER;
> +  CpcInfo->ReferencePerformanceCounterRegister.Address           = 0x4;
> +
> +  CpcInfo->DeliveredPerformanceCounterRegister.AddressSpaceId    = EFI_ACPI_6_4_FUNCTIONAL_FIXED_HARDWARE;
> +  CpcInfo->DeliveredPerformanceCounterRegister.RegisterBitWidth  = 0x40;
> +  CpcInfo->DeliveredPerformanceCounterRegister.RegisterBitOffset = 0;
> +  CpcInfo->DeliveredPerformanceCounterRegister.AccessSize        = ARM_FFH_DELIVERED_PERF_COUNTER_REGISTER;
> +  CpcInfo->DeliveredPerformanceCounterRegister.Address           = 0x4;
> +
> +  // SCMI should advertise performance values on a unified scale. So frequency
> +  // values are not available. LowestFrequencyInteger and
> +  // NominalFrequencyInteger are populated in the ConfigurationManager.
> +
> +exit_handler:
> +  FreePool (LevelArray);
> +  return Status;
> +}
> diff --git a/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
> new file mode 100644
> index 000000000000..aad3f0fa7b83
> --- /dev/null
> +++ b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
> @@ -0,0 +1,31 @@
> +## @file
> +#  Arm SCMI Info Library.
> +#
> +#  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +##
> +
> +[Defines]
> +  INF_VERSION    = 0x0001001B
> +  BASE_NAME      = ArmScmiInfoLib
> +  FILE_GUID      = 1A7CDB04-9FFC-40DA-A87C-A5ACADAF8136
> +  VERSION_STRING = 1.0
> +  MODULE_TYPE    = DXE_DRIVER
> +  LIBRARY_CLASS  = ArmScmiInfoLib
> +  CONSTRUCTOR    = ArmScmiInfoLibConstructor
> +
> +[Sources]
> +  ArmScmiInfoLib.c
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  DynamicTablesPkg/DynamicTablesPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +
> +[Protocols]
> +  gArmScmiPerformanceProtocolGuid   ## CONSUMES
> +
> +[Depex]
> +  gArmScmiPerformanceProtocolGuid
> -- 
> 2.25.1
> 
> 
> 
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110091): https://edk2.groups.io/g/devel/message/110091
Mute This Topic: https://groups.io/mt/102175821/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 11/11] DynamicTablesPkg: Remove check for _CPC field

[Leif Lindholm]

On Wed, Oct 25, 2023 at 13:25:40 +0200, pierre.gondois@arm.com wrote:
> From: Pierre Gondois <pierre.gondois@arm.com>
> 
> When generating _CPC objects, some fields are mandatory.

Mandatory by spec or mandatory by current API?
If the former, could we either warn or add a Pcd to enable the more
lenient behaviour?

/
    Leif

> Some fields cannot be supported by a the Juno platform, which is used
> to test the _CPC generation. Therefore, don't prevent from generating
> _CPC objects if the fields below are missing, and let the OS handle
> the missing information.
> 
> _CPC fields that are exempted from checks:
> - PerformanceLimitedRegister
> - ReferencePerformanceCounterRegister
> - DeliveredPerformanceCounterRegister
> 
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>  .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> index f350083b148c..423e64f12606 100644
> --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> @@ -3531,6 +3531,11 @@ AmlCreateCpcNode (
>      return EFI_INVALID_PARAMETER;
>    }
>  
> +  // The following fields are theoretically mandatory, but not supported
> +  // by some platforms. Don't check them:
> +  // - PerformanceLimitedRegister
> +  // - ReferencePerformanceCounterRegister
> +  // - DeliveredPerformanceCounterRegister
>    if ((IsNullGenericAddress (&CpcInfo->HighestPerformanceBuffer) &&
>         (CpcInfo->HighestPerformanceInteger == 0)) ||
>        (IsNullGenericAddress (&CpcInfo->NominalPerformanceBuffer) &&
> @@ -3539,13 +3544,19 @@ AmlCreateCpcNode (
>         (CpcInfo->LowestNonlinearPerformanceInteger == 0)) ||
>        (IsNullGenericAddress (&CpcInfo->LowestPerformanceBuffer) &&
>         (CpcInfo->LowestPerformanceInteger == 0)) ||
> -      IsNullGenericAddress (&CpcInfo->DesiredPerformanceRegister) ||
> -      IsNullGenericAddress (&CpcInfo->ReferencePerformanceCounterRegister) ||
> -      IsNullGenericAddress (&CpcInfo->DeliveredPerformanceCounterRegister) ||
> -      IsNullGenericAddress (&CpcInfo->PerformanceLimitedRegister))
> +      IsNullGenericAddress (&CpcInfo->DesiredPerformanceRegister))
>    {
>      ASSERT (0);
>      return EFI_INVALID_PARAMETER;
> +  } else if ((IsNullGenericAddress (&CpcInfo->HighestPerformanceBuffer) &&
> +              (CpcInfo->HighestPerformanceInteger == 0)) ||
> +             (IsNullGenericAddress (&CpcInfo->NominalPerformanceBuffer) &&
> +              (CpcInfo->NominalPerformanceInteger == 0)))
> +  {
> +    DEBUG ((
> +      DEBUG_WARN,
> +      "Missing Reference|Delivered performance field in _CPC object\n"
> +      ));
>    }
>  
>    CpcPackage = NULL;
> -- 
> 2.25.1
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110092): https://edk2.groups.io/g/devel/message/110092
Mute This Topic: https://groups.io/mt/102175822/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 02/11] ArmPkg/ArmScmiDxe: Add PERFORMANCE_DESCRIBE_FASTCHANNEL support

[PierreGondois]

On 10/26/23 12:37, Leif Lindholm wrote:
> On Wed, Oct 25, 2023 at 13:25:31 +0200, PierreGondois wrote:
>> From: Pierre Gondois <pierre.gondois@arm.com>
> 
> Sidebar: I think if you correct your name in your git config, the
> above tag will drop out.

Ok I will check.

> 
>> The PERFORMANCE_DESCRIBE_FASTCHANNEL Scmi command is available
>> since SCMI v2.0 and allows to query information about the supported
>> fast-channels of the Scmi performance protocol.
>> Add support for this command.
>>
>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>> ---
>>   .../ArmScmiDxe/ScmiPerformanceProtocol.c      | 80 +++++++++++++++--
>>   .../Protocol/ArmScmiPerformanceProtocol.h     | 90 ++++++++++++++++---
>>   2 files changed, 155 insertions(+), 15 deletions(-)
>>
>> diff --git a/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c b/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
>> index 0f89808fbdf9..1d87339209fd 100644
>> --- a/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
>> +++ b/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
>> @@ -1,12 +1,12 @@
>>   /** @file
>>   
>> -  Copyright (c) 2017-2021, Arm Limited. All rights reserved.<BR>
>> +  Copyright (c) 2017-2023, Arm Limited. All rights reserved.<BR>
>>   
>>     SPDX-License-Identifier: BSD-2-Clause-Patent
>>   
>> -  System Control and Management Interface V1.0
>> -    http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/
>> -    DEN0056A_System_Control_and_Management_Interface.pdf
>> +  System Control and Management Interface, latest version:
>> +  - https://developer.arm.com/documentation/den0056/latest/
>> +
>>   **/
>>   
>>   #include <Library/BaseMemoryLib.h>
>> @@ -416,6 +416,75 @@ PerformanceLevelGet (
>>     return EFI_SUCCESS;
>>   }
>>   
>> +/** Discover the attributes of the FastChannel for the specified
>> +    performance domain and the specified message.
>> +
>> +  @param[in]  This        A Pointer to SCMI_PERFORMANCE_PROTOCOL Instance.
>> +  @param[in]  DomainId    Identifier for the performance domain.
>> +  @param[in]  MessageId   Message Id of the FastChannel to discover.
>> +                          Must be one of:
>> +                           - PERFORMANCE_LIMITS_SET
>> +                           - PERFORMANCE_LIMITS_GET
>> +                           - PERFORMANCE_LEVEL_SET
>> +                           - PERFORMANCE_LEVEL_GET
>> +  @param[out] FastChannel If success, contains the FastChannel description.
>> +
>> +  @retval EFI_SUCCESS             Performance level got successfully.
>> +  @retval EFI_DEVICE_ERROR        SCP returns an SCMI error.
>> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>> +  @retval EFI_TIMEOUT             Time out.
>> +  @retval EFI_UNSUPPORTED         Unsupported.
>> +**/
>> +EFI_STATUS
>> +DescribeFastchannel (
>> +  IN  SCMI_PERFORMANCE_PROTOCOL     *This,
>> +  IN  UINT32                        DomainId,
>> +  IN  SCMI_MESSAGE_ID_PERFORMANCE   MessageId,
>> +  OUT SCMI_PERFORMANCE_FASTCHANNEL  *FastChannel
>> +  )
>> +{
>> +  EFI_STATUS    Status;
>> +  SCMI_COMMAND  Cmd;
>> +  UINT32        PayloadLength;
>> +  UINT32        *ReturnValues;
>> +  UINT32        *MessageParams;
>> +
>> +  if ((This == NULL)  ||
>> +      (FastChannel == NULL))
>> +  {
>> +    return EFI_INVALID_PARAMETER;
>> +  }
>> +
>> +  Status = ScmiCommandGetPayload (&MessageParams);
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  *MessageParams++ = DomainId;
>> +  *MessageParams   = MessageId;
>> +
>> +  Cmd.ProtocolId = ScmiProtocolIdPerformance;
>> +  Cmd.MessageId  = ScmiMessageIdPerformanceDescribeFastchannel;
>> +  PayloadLength  = sizeof (DomainId) + sizeof (MessageId);
>> +
>> +  Status = ScmiCommandExecute (
>> +             &Cmd,
>> +             &PayloadLength,
>> +             &ReturnValues
>> +             );
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  CopyMem (
>> +    FastChannel,
>> +    ReturnValues,
>> +    sizeof (SCMI_PERFORMANCE_FASTCHANNEL)
>> +    );
>> +
>> +  return Status;
>> +}
>> +
>>   // Instance of the SCMI performance management protocol.
>>   STATIC CONST SCMI_PERFORMANCE_PROTOCOL  PerformanceProtocol = {
>>     PerformanceGetVersion,
>> @@ -425,7 +494,8 @@ STATIC CONST SCMI_PERFORMANCE_PROTOCOL  PerformanceProtocol = {
>>     PerformanceLimitsSet,
>>     PerformanceLimitsGet,
>>     PerformanceLevelSet,
>> -  PerformanceLevelGet
>> +  PerformanceLevelGet,
>> +  DescribeFastchannel,
>>   };
>>   
>>   /** Initialize performance management protocol and install on a given Handle.
>> diff --git a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
>> index 8e8e05d5a5f6..088182945a06 100644
>> --- a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
>> +++ b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
>> @@ -14,7 +14,7 @@
>>   
>>   #include <Protocol/ArmScmi.h>
>>   
>> -/// Arm Scmi performance protocol versions.
>> +/// Arm SCMI performance protocol versions.
>>   #define PERFORMANCE_PROTOCOL_VERSION_V1  0x10000
>>   #define PERFORMANCE_PROTOCOL_VERSION_V2  0x20000
>>   #define PERFORMANCE_PROTOCOL_VERSION_V3  0x30000
>> @@ -79,6 +79,56 @@ typedef struct {
>>     UINT32    RangeMin;
>>   } SCMI_PERFORMANCE_LIMITS;
>>   
>> +/// Doorbell Support bit.
>> +#define SCMI_PERF_FC_ATTRIB_HAS_DOORBELL  BIT0
>> +
>> +/// Performance protocol describe fastchannel
>> +typedef struct {
>> +  /// Attributes.
>> +  UINT32    Attributes;
>> +
>> +  /// Rate limit.
>> +  UINT32    RateLimit;
>> +
>> +  /// Lower 32 bits of the FastChannel address.
>> +  UINT32    ChanAddrLow;
>> +
>> +  /// Higher 32 bits of the FastChannel address.
>> +  UINT32    ChanAddrHigh;
>> +
>> +  /// Size of the FastChannel in bytes.
>> +  UINT32    ChanSize;
>> +
>> +  /// Lower 32 bits of the doorbell address.
>> +  UINT32    DoorbellAddrLow;
>> +
>> +  /// Higher 32 bits of the doorbell address.
>> +  UINT32    DoorbellAddrHigh;
>> +
>> +  /// Mask of lower 32 bits to set when writing to the doorbell register.
>> +  UINT32    DoorbellSetMaskLow;
>> +
>> +  /// Mask of higher 32 bits to set when writing to the doorbell register.
>> +  UINT32    DoorbellSetMaskHigh;
>> +
>> +  /// Mask of lower 32 bits to preserve when writing to the doorbell register.
>> +  UINT32    DoorbellPreserveMaskLow;
>> +
>> +  /// Mask of higher 32 bits to preserve when writing to the doorbell register.
>> +  UINT32    DoorbellPreserveMaskHigh;
>> +} SCMI_PERFORMANCE_FASTCHANNEL;
>> +
>> +/// SCMI Message Ids for the Performance Protocol.
>> +typedef enum {
>> +  ScmiMessageIdPerformanceDomainAttributes    = 0x3,
>> +  ScmiMessageIdPerformanceDescribeLevels      = 0x4,
>> +  ScmiMessageIdPerformanceLimitsSet           = 0x5,
>> +  ScmiMessageIdPerformanceLimitsGet           = 0x6,
>> +  ScmiMessageIdPerformanceLevelSet            = 0x7,
>> +  ScmiMessageIdPerformanceLevelGet            = 0x8,
>> +  ScmiMessageIdPerformanceDescribeFastchannel = 0xB,
>> +} SCMI_MESSAGE_ID_PERFORMANCE;
> 
> This struct appears to move in the code at the same time as having an
> entry added to it. This seems superficially unmotivated.
> However it is also moved to inside the pack(1) block, which makes no
> sense for an enum.

Yes right, I will move it out of the pack(1) section.
The reason to move the SCMI_MESSAGE_ID_PERFORMANCE definition up in the file
was that the SCMI_PERFORMANCE_DESCRIBE_FASTCHANNEL interface added in this
patch takes a SCMI_MESSAGE_ID_PERFORMANCE parameter as an argument, so the
enum needs to be defined before.
I can add a comment about this in the commit message.

Regards,
Pierre

> 
> /
>      Leif
> 
>> +
>>   #pragma pack()
>>   
>>   /** Return version of the performance management protocol supported by SCP.
>> @@ -238,6 +288,34 @@ EFI_STATUS
>>     OUT UINT32                    *Level
>>     );
>>   
>> +/** Discover the attributes of the FastChannel for the specified
>> +    performance domain and the specified message.
>> +
>> +  @param[in]  This        A Pointer to SCMI_PERFORMANCE_PROTOCOL Instance.
>> +  @param[in]  DomainId    Identifier for the performance domain.
>> +  @param[in]  MessageId   Message Id of the FastChannel to discover.
>> +                          Must be one of:
>> +                           - PERFORMANCE_LIMITS_SET
>> +                           - PERFORMANCE_LIMITS_GET
>> +                           - PERFORMANCE_LEVEL_SET
>> +                           - PERFORMANCE_LEVEL_GET
>> +  @param[out] FastChannel If success, contains the FastChannel description.
>> +
>> +  @retval EFI_SUCCESS             Performance level got successfully.
>> +  @retval EFI_DEVICE_ERROR        SCP returns an SCMI error.
>> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>> +  @retval EFI_TIMEOUT             Time out.
>> +  @retval EFI_UNSUPPORTED         Unsupported.
>> +**/
>> +typedef
>> +EFI_STATUS
>> +(EFIAPI *SCMI_PERFORMANCE_DESCRIBE_FASTCHANNEL)(
>> +  IN  SCMI_PERFORMANCE_PROTOCOL       *This,
>> +  IN  UINT32                          DomainId,
>> +  IN  SCMI_MESSAGE_ID_PERFORMANCE     MessageId,
>> +  OUT SCMI_PERFORMANCE_FASTCHANNEL    *FastChannel
>> +  );
>> +
>>   typedef struct _SCMI_PERFORMANCE_PROTOCOL {
>>     SCMI_PERFORMANCE_GET_VERSION              GetVersion;
>>     SCMI_PERFORMANCE_GET_ATTRIBUTES           GetProtocolAttributes;
>> @@ -247,15 +325,7 @@ typedef struct _SCMI_PERFORMANCE_PROTOCOL {
>>     SCMI_PERFORMANCE_LIMITS_GET               LimitsGet;
>>     SCMI_PERFORMANCE_LEVEL_SET                LevelSet;
>>     SCMI_PERFORMANCE_LEVEL_GET                LevelGet;
>> +  SCMI_PERFORMANCE_DESCRIBE_FASTCHANNEL     DescribeFastchannel;
>>   } SCMI_PERFORMANCE_PROTOCOL;
>>   
>> -typedef enum {
>> -  ScmiMessageIdPerformanceDomainAttributes = 0x3,
>> -  ScmiMessageIdPerformanceDescribeLevels   = 0x4,
>> -  ScmiMessageIdPerformanceLimitsSet        = 0x5,
>> -  ScmiMessageIdPerformanceLimitsGet        = 0x6,
>> -  ScmiMessageIdPerformanceLevelSet         = 0x7,
>> -  ScmiMessageIdPerformanceLevelGet         = 0x8,
>> -} SCMI_MESSAGE_ID_PERFORMANCE;
>> -
>>   #endif /* ARM_SCMI_PERFORMANCE_PROTOCOL_H_ */
>> -- 
>> 2.25.1
>>
>>
>>
>> 
>>
>>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110513): https://edk2.groups.io/g/devel/message/110513
Mute This Topic: https://groups.io/mt/102175812/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 10/11] DynamicTablesPkg: Add ArmScmiInfoLib

[PierreGondois]

On 10/26/23 13:03, Leif Lindholm wrote:
> On Wed, Oct 25, 2023 at 13:25:39 +0200, PierreGondois wrote:
>> From: Pierre Gondois <pierre.gondois@arm.com>
>>
>> The SCP holds some power information that could be advertised
>> through the _CPC object. The communication with the SCP is done
>> through SCMI protocols (c.f. ArmScmiDxe).
>>
>> Use the SCMI protocols to query information and feed it to
>> the DynamicTablesPkg.
> 
> Couple of questions:
> With a generic name like ArmScmiInfoLib, does is belong in ArmPkg or
> MdeModulePkg?
> 
> Or if it's more tightly integrated with DynamicTablesPkg (not
> blatantly obvious from a quick skim below), should that be reflected
> by the library name?

The library is tight to the DynamicTablesPkg as it produces DynamicTablesPkg
specific objects. It uses the SCMI interface to fetch information from the SCP.
The ScmiProtocol resides in the ArmPkg, thus the name.
Does the name 'ScmiInfoLib' sound better ?

Regards,
Pierre


> 
> /
>      Leif
> 
>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>> ---
>>   DynamicTablesPkg/DynamicTables.dsc.inc        |   1 +
>>   DynamicTablesPkg/DynamicTablesPkg.dec         |   3 +
>>   DynamicTablesPkg/DynamicTablesPkg.dsc         |   1 +
>>   .../Include/Library/ArmScmiInfoLib.h          |  33 ++
>>   .../Library/ArmScmiInfoLib/ArmScmiInfoLib.c   | 294 ++++++++++++++++++
>>   .../Library/ArmScmiInfoLib/ArmScmiInfoLib.inf |  31 ++
>>   6 files changed, 363 insertions(+)
>>   create mode 100644 DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h
>>   create mode 100644 DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c
>>   create mode 100644 DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
>>
>> diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc b/DynamicTablesPkg/DynamicTables.dsc.inc
>> index 9d4312c4e87d..be40ebc4b472 100644
>> --- a/DynamicTablesPkg/DynamicTables.dsc.inc
>> +++ b/DynamicTablesPkg/DynamicTables.dsc.inc
>> @@ -15,6 +15,7 @@ [BuildOptions]
>>   [LibraryClasses.common]
>>     AcpiHelperLib|DynamicTablesPkg/Library/Common/AcpiHelperLib/AcpiHelperLib.inf
>>     AmlLib|DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
>> +  ArmScmiInfoLib|DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
>>     SsdtPcieSupportLib|DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
>>     SsdtSerialPortFixupLib|DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.inf
>>     TableHelperLib|DynamicTablesPkg/Library/Common/TableHelperLib/TableHelperLib.inf
>> diff --git a/DynamicTablesPkg/DynamicTablesPkg.dec b/DynamicTablesPkg/DynamicTablesPkg.dec
>> index cfbcbb9569f1..26498e5fec53 100644
>> --- a/DynamicTablesPkg/DynamicTablesPkg.dec
>> +++ b/DynamicTablesPkg/DynamicTablesPkg.dec
>> @@ -42,6 +42,9 @@ [LibraryClasses]
>>     ##  @libraryclass  Defines a set of SMBIOS string helper methods.
>>     SmbiosStringTableLib|Include/Library/SmbiosStringTableLib.h
>>   
>> +  ##  @libraryclass  Defines a set of APIs to populate CmObj using SCMI.
>> +  ArmScmiInfoLib|Include/Library/ArmScmiInfoLib.h
>> +
>>   [Protocols]
>>     # Configuration Manager Protocol GUID
>>     gEdkiiConfigurationManagerProtocolGuid = { 0xd85a4835, 0x5a82, 0x4894, { 0xac, 0x2, 0x70, 0x6f, 0x43, 0xd5, 0x97, 0x8e } }
>> diff --git a/DynamicTablesPkg/DynamicTablesPkg.dsc b/DynamicTablesPkg/DynamicTablesPkg.dsc
>> index bd5084a9008f..6ea86c9efdb0 100644
>> --- a/DynamicTablesPkg/DynamicTablesPkg.dsc
>> +++ b/DynamicTablesPkg/DynamicTablesPkg.dsc
>> @@ -39,6 +39,7 @@ [LibraryClasses.ARM, LibraryClasses.AARCH64]
>>     PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
>>   
>>   [Components.common]
>> +  DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
>>     DynamicTablesPkg/Library/Common/AcpiHelperLib/AcpiHelperLib.inf
>>     DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
>>     DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
>> diff --git a/DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h b/DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h
>> new file mode 100644
>> index 000000000000..8d3fb31df13c
>> --- /dev/null
>> +++ b/DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h
>> @@ -0,0 +1,33 @@
>> +/** @file
>> +  Arm SCMI Info Library.
>> +
>> +  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.<BR>
>> +
>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +**/
>> +
>> +#ifndef ARM_SCMI_INFO_LIB_H_
>> +#define ARM_SCMI_INFO_LIB_H_
>> +
>> +#include <ConfigurationManagerObject.h>
>> +
>> +/** Populate a AML_CPC_INFO object based on SCMI information.
>> +
>> +  @param[in]  DomainId    Identifier for the performance domain.
>> +  @param[out] CpcInfo     If success, this structure was populated from
>> +                          information queried to the SCP.
>> +
>> +  @retval EFI_SUCCESS             Success.
>> +  @retval EFI_DEVICE_ERROR        Device error.
>> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>> +  @retval EFI_TIMEOUT             Time out.
>> +  @retval EFI_UNSUPPORTED         Unsupported.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +ArmScmiInfoGetFastChannel (
>> +  IN  UINT32        DomainId,
>> +  OUT AML_CPC_INFO  *CpcInfo
>> +  );
>> +
>> +#endif // ARM_SCMI_INFO_LIB_H_
>> diff --git a/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c
>> new file mode 100644
>> index 000000000000..c23bff63bb6f
>> --- /dev/null
>> +++ b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c
>> @@ -0,0 +1,294 @@
>> +/** @file
>> +  Arm SCMI Info Library.
>> +
>> +  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.<BR>
>> +
>> +  Arm Functional Fixed Hardware Specification:
>> +  - https://developer.arm.com/documentation/den0048/latest/
>> +
>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +**/
>> +
>> +#include <Library/AcpiLib.h>
>> +#include <Library/ArmScmiInfoLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/MemoryAllocationLib.h>
>> +#include <Library/UefiBootServicesTableLib.h>
>> +#include <Protocol/ArmScmi.h>
>> +#include <Protocol/ArmScmiPerformanceProtocol.h>
>> +
>> +/** Arm FFH registers
>> +
>> +  Cf. Arm Functional Fixed Hardware Specification
>> +  s3.2 Performance management and Collaborative Processor Performance Control
>> +*/
>> +#define ARM_FFH_DELIVERED_PERF_COUNTER_REGISTER  0x0
>> +#define ARM_FFH_REFERENCE_PERF_COUNTER_REGISTER  0x1
>> +
>> +/// Arm SCMI performance protocol.
>> +STATIC SCMI_PERFORMANCE_PROTOCOL  *ScmiPerfProtocol;
>> +
>> +/** Arm SCMI Info Library constructor.
>> +
>> +  @param  ImageHandle   Image of the loaded driver.
>> +  @param  SystemTable   Pointer to the System Table.
>> +
>> +  @retval EFI_SUCCESS             Success.
>> +  @retval EFI_DEVICE_ERROR        Device error.
>> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>> +  @retval EFI_NOT_FOUND           Not Found
>> +  @retval EFI_TIMEOUT             Timeout.
>> +  @retval EFI_UNSUPPORTED         Unsupported.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +ArmScmiInfoLibConstructor (
>> +  IN  EFI_HANDLE        ImageHandle,
>> +  IN  EFI_SYSTEM_TABLE  *SystemTable
>> +  )
>> +{
>> +  EFI_STATUS  Status;
>> +  UINT32      Version;
>> +
>> +  Status = gBS->LocateProtocol (
>> +                  &gArmScmiPerformanceProtocolGuid,
>> +                  NULL,
>> +                  (VOID **)&ScmiPerfProtocol
>> +                  );
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  Status = ScmiPerfProtocol->GetVersion (ScmiPerfProtocol, &Version);
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  // FastChannels were added in SCMI v2.0 spec.
>> +  if (Version < PERFORMANCE_PROTOCOL_VERSION_V2) {
>> +    DEBUG ((DEBUG_ERROR, "ArmScmiInfoLib requires SCMI version > 2.0\n"));
>> +    return EFI_UNSUPPORTED;
>> +  }
>> +
>> +  return Status;
>> +}
>> +
>> +/** Get the OPPs/performance states of a power domain.
>> +
>> +  This function is a wrapper around the SCMI PERFORMANCE_DESCRIBE_LEVELS
>> +  command. The list of discrete performance states is returned in a buffer
>> +  that must be freed by the caller.
>> +
>> +  @param[in]  DomainId        Identifier for the performance domain.
>> +  @param[out] LevelArray      If success, pointer to the list of list of
>> +                              performance state. This memory must be freed by
>> +                              the caller.
>> +  @param[out] LevelArrayCount If success, contains the number of states in
>> +                              LevelArray.
>> +
>> +  @retval EFI_SUCCESS             Success.
>> +  @retval EFI_DEVICE_ERROR        Device error.
>> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>> +  @retval EFI_TIMEOUT             Time out.
>> +  @retval EFI_UNSUPPORTED         Unsupported.
>> +**/
>> +STATIC
>> +EFI_STATUS
>> +EFIAPI
>> +ArmScmiInfoDescribeLevels (
>> +  IN  UINT32                  DomainId,
>> +  OUT SCMI_PERFORMANCE_LEVEL  **LevelArray,
>> +  OUT UINT32                  *LevelArrayCount
>> +  )
>> +{
>> +  EFI_STATUS              Status;
>> +  SCMI_PERFORMANCE_LEVEL  *Array;
>> +  UINT32                  Count;
>> +  UINT32                  Size;
>> +
>> +  if ((ScmiPerfProtocol == NULL)  ||
>> +      (LevelArray == NULL)  ||
>> +      (LevelArrayCount == NULL))
>> +  {
>> +    return EFI_INVALID_PARAMETER;
>> +  }
>> +
>> +  // First call to get the number of levels.
>> +  Size   = 0;
>> +  Status = ScmiPerfProtocol->DescribeLevels (
>> +                               ScmiPerfProtocol,
>> +                               DomainId,
>> +                               &Count,
>> +                               &Size,
>> +                               NULL
>> +                               );
>> +  if (Status != EFI_BUFFER_TOO_SMALL) {
>> +    // EFI_SUCCESS is not a valid option.
>> +    if (Status == EFI_SUCCESS) {
>> +      return EFI_INVALID_PARAMETER;
>> +    } else {
>> +      return Status;
>> +    }
>> +  }
>> +
>> +  Array = AllocateZeroPool (Size);
>> +  if (Array == NULL) {
>> +    return EFI_OUT_OF_RESOURCES;
>> +  }
>> +
>> +  // Second call to get the descriptions of the levels.
>> +  Status = ScmiPerfProtocol->DescribeLevels (
>> +                               ScmiPerfProtocol,
>> +                               DomainId,
>> +                               &Count,
>> +                               &Size,
>> +                               Array
>> +                               );
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  *LevelArray      = Array;
>> +  *LevelArrayCount = Count;
>> +
>> +  return Status;
>> +}
>> +
>> +/** Populate a AML_CPC_INFO object based on SCMI information.
>> +
>> +  @param[in]  DomainId    Identifier for the performance domain.
>> +  @param[out] CpcInfo     If success, this structure was populated from
>> +                          information queried to the SCP.
>> +
>> +  @retval EFI_SUCCESS             Success.
>> +  @retval EFI_DEVICE_ERROR        Device error.
>> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>> +  @retval EFI_TIMEOUT             Time out.
>> +  @retval EFI_UNSUPPORTED         Unsupported.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +ArmScmiInfoGetFastChannel (
>> +  IN  UINT32        DomainId,
>> +  OUT AML_CPC_INFO  *CpcInfo
>> +  )
>> +{
>> +  EFI_STATUS                          Status;
>> +  SCMI_PERFORMANCE_FASTCHANNEL        FcLevelGet;
>> +  SCMI_PERFORMANCE_FASTCHANNEL        FcLimitsSet;
>> +  SCMI_PERFORMANCE_DOMAIN_ATTRIBUTES  DomainAttributes;
>> +
>> +  SCMI_PERFORMANCE_LEVEL  *LevelArray;
>> +  UINT32                  LevelCount;
>> +
>> +  UINT64  FcLevelGetAddr;
>> +  UINT64  FcLimitsMaxSetAddr;
>> +  UINT64  FcLimitsMinSetAddr;
>> +
>> +  if ((ScmiPerfProtocol == NULL)  ||
>> +      (CpcInfo == NULL))
>> +  {
>> +    return EFI_INVALID_PARAMETER;
>> +  }
>> +
>> +  Status = ScmiPerfProtocol->DescribeFastchannel (
>> +                               ScmiPerfProtocol,
>> +                               DomainId,
>> +                               ScmiMessageIdPerformanceLevelSet,
>> +                               &FcLevelGet
>> +                               );
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  Status = ScmiPerfProtocol->DescribeFastchannel (
>> +                               ScmiPerfProtocol,
>> +                               DomainId,
>> +                               ScmiMessageIdPerformanceLimitsSet,
>> +                               &FcLimitsSet
>> +                               );
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  Status = ScmiPerfProtocol->GetDomainAttributes (
>> +                               ScmiPerfProtocol,
>> +                               DomainId,
>> +                               &DomainAttributes
>> +                               );
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  Status = ArmScmiInfoDescribeLevels (DomainId, &LevelArray, &LevelCount);
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  /* Do some safety checks.
>> +     Only support FastChannels (and not doorbells) as this is
>> +     the only mechanism supported by SCP.
>> +     FcLimits[Get|Set] require 2 UINT32 values (max, then min) and
>> +     FcLimits[Get|Set] require 1 UINT32 value (level).
>> +  */
>> +  if ((FcLevelGet.ChanSize != sizeof (UINT32))  ||
>> +      ((FcLevelGet.Attributes & SCMI_PERF_FC_ATTRIB_HAS_DOORBELL) ==
>> +       SCMI_PERF_FC_ATTRIB_HAS_DOORBELL) ||
>> +      (FcLimitsSet.ChanSize != 2 * sizeof (UINT32)) ||
>> +      ((FcLimitsSet.Attributes & SCMI_PERF_FC_ATTRIB_HAS_DOORBELL) ==
>> +       SCMI_PERF_FC_ATTRIB_HAS_DOORBELL))
>> +  {
>> +    Status = EFI_INVALID_PARAMETER;
>> +    goto exit_handler;
>> +  }
>> +
>> +  FcLevelGetAddr = ((UINT64)FcLevelGet.ChanAddrHigh << 32) |
>> +                   FcLevelGet.ChanAddrLow;
>> +  FcLimitsMaxSetAddr = ((UINT64)FcLimitsSet.ChanAddrHigh << 32) |
>> +                       FcLimitsSet.ChanAddrLow;
>> +  FcLimitsMinSetAddr = FcLimitsMaxSetAddr + 0x4;
>> +
>> +  CpcInfo->Revision                          = EFI_ACPI_6_4_AML_CPC_REVISION_V3;
>> +  CpcInfo->HighestPerformanceInteger         = LevelArray[LevelCount - 1].Level;
>> +  CpcInfo->NominalPerformanceInteger         = DomainAttributes.SustainedPerfLevel;
>> +  CpcInfo->LowestNonlinearPerformanceInteger = LevelArray[0].Level;
>> +  CpcInfo->LowestPerformanceInteger          = LevelArray[0].Level;
>> +
>> +  CpcInfo->DesiredPerformanceRegister.AddressSpaceId    = EFI_ACPI_6_4_SYSTEM_MEMORY;
>> +  CpcInfo->DesiredPerformanceRegister.RegisterBitWidth  = 32;
>> +  CpcInfo->DesiredPerformanceRegister.RegisterBitOffset = 0;
>> +  CpcInfo->DesiredPerformanceRegister.AccessSize        = EFI_ACPI_6_4_DWORD;
>> +  CpcInfo->DesiredPerformanceRegister.Address           = FcLevelGetAddr;
>> +
>> +  CpcInfo->MinimumPerformanceRegister.AddressSpaceId    = EFI_ACPI_6_4_SYSTEM_MEMORY;
>> +  CpcInfo->MinimumPerformanceRegister.RegisterBitWidth  = 32;
>> +  CpcInfo->MinimumPerformanceRegister.RegisterBitOffset = 0;
>> +  CpcInfo->MinimumPerformanceRegister.AccessSize        = EFI_ACPI_6_4_DWORD;
>> +  CpcInfo->MinimumPerformanceRegister.Address           = FcLimitsMinSetAddr;
>> +
>> +  CpcInfo->MaximumPerformanceRegister.AddressSpaceId    = EFI_ACPI_6_4_SYSTEM_MEMORY;
>> +  CpcInfo->MaximumPerformanceRegister.RegisterBitWidth  = 32;
>> +  CpcInfo->MaximumPerformanceRegister.RegisterBitOffset = 0;
>> +  CpcInfo->MaximumPerformanceRegister.AccessSize        = EFI_ACPI_6_4_DWORD;
>> +  CpcInfo->MaximumPerformanceRegister.Address           = FcLimitsMaxSetAddr;
>> +
>> +  CpcInfo->ReferencePerformanceCounterRegister.AddressSpaceId    = EFI_ACPI_6_4_FUNCTIONAL_FIXED_HARDWARE;
>> +  CpcInfo->ReferencePerformanceCounterRegister.RegisterBitWidth  = 0x40;
>> +  CpcInfo->ReferencePerformanceCounterRegister.RegisterBitOffset = 0;
>> +  CpcInfo->ReferencePerformanceCounterRegister.AccessSize        = ARM_FFH_REFERENCE_PERF_COUNTER_REGISTER;
>> +  CpcInfo->ReferencePerformanceCounterRegister.Address           = 0x4;
>> +
>> +  CpcInfo->DeliveredPerformanceCounterRegister.AddressSpaceId    = EFI_ACPI_6_4_FUNCTIONAL_FIXED_HARDWARE;
>> +  CpcInfo->DeliveredPerformanceCounterRegister.RegisterBitWidth  = 0x40;
>> +  CpcInfo->DeliveredPerformanceCounterRegister.RegisterBitOffset = 0;
>> +  CpcInfo->DeliveredPerformanceCounterRegister.AccessSize        = ARM_FFH_DELIVERED_PERF_COUNTER_REGISTER;
>> +  CpcInfo->DeliveredPerformanceCounterRegister.Address           = 0x4;
>> +
>> +  // SCMI should advertise performance values on a unified scale. So frequency
>> +  // values are not available. LowestFrequencyInteger and
>> +  // NominalFrequencyInteger are populated in the ConfigurationManager.
>> +
>> +exit_handler:
>> +  FreePool (LevelArray);
>> +  return Status;
>> +}
>> diff --git a/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
>> new file mode 100644
>> index 000000000000..aad3f0fa7b83
>> --- /dev/null
>> +++ b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
>> @@ -0,0 +1,31 @@
>> +## @file
>> +#  Arm SCMI Info Library.
>> +#
>> +#  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.
>> +#
>> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +##
>> +
>> +[Defines]
>> +  INF_VERSION    = 0x0001001B
>> +  BASE_NAME      = ArmScmiInfoLib
>> +  FILE_GUID      = 1A7CDB04-9FFC-40DA-A87C-A5ACADAF8136
>> +  VERSION_STRING = 1.0
>> +  MODULE_TYPE    = DXE_DRIVER
>> +  LIBRARY_CLASS  = ArmScmiInfoLib
>> +  CONSTRUCTOR    = ArmScmiInfoLibConstructor
>> +
>> +[Sources]
>> +  ArmScmiInfoLib.c
>> +
>> +[Packages]
>> +  ArmPkg/ArmPkg.dec
>> +  DynamicTablesPkg/DynamicTablesPkg.dec
>> +  EmbeddedPkg/EmbeddedPkg.dec
>> +  MdePkg/MdePkg.dec
>> +
>> +[Protocols]
>> +  gArmScmiPerformanceProtocolGuid   ## CONSUMES
>> +
>> +[Depex]
>> +  gArmScmiPerformanceProtocolGuid
>> -- 
>> 2.25.1
>>
>>
>>
>> 
>>
>>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110514): https://edk2.groups.io/g/devel/message/110514
Mute This Topic: https://groups.io/mt/102175821/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 01/11] ArmPkg/ArmScmiDxe: Rename PERFORMANCE_PROTOCOL_VERSION

[PierreGondois]

Hello Leif,
Thanks for the review,

On 10/26/23 12:05, Leif Lindholm wrote:
> On Wed, Oct 25, 2023 at 13:25:30 +0200, pierre.gondois@arm.com wrote:
>> From: Pierre Gondois <pierre.gondois@arm.com>
>>
>> Rename PERFORMANCE_PROTOCOL_VERSION to reflect the different
>> versions of the protocol. The macro is neither used in edk2 nor
>> in edk2-platforms.
> 
> OK, so slight nitpick, but mainly because it parses a bit weirdly...
> *Will* it be used after this series is merged, or is this an update
> for completeness?

The 'fast channels' were added in the v2.0 SCMI specification. This patch-set
relies on this feature, so it is checked in:
    [PATCH v2 10/11] DynamicTablesPkg: Add ArmScmiInfoLib
that the underlying SCP is at least at this version.

```
    // FastChannels were added in SCMI v2.0 spec.
    if (Version < PERFORMANCE_PROTOCOL_VERSION_V2) {
      DEBUG ((DEBUG_ERROR, "ArmScmiInfoLib requires SCMI version > 2.0\n"));
      return EFI_UNSUPPORTED;
    }
```

> 
>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>> ---
>>   ArmPkg/Include/Library/ArmLib.h                     |  1 +
>>   .../Include/Protocol/ArmScmiPerformanceProtocol.h   | 13 ++++++++-----
>>   2 files changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
>> index 0169dbc1092c..7b2b2238fed9 100644
>> --- a/ArmPkg/Include/Library/ArmLib.h
>> +++ b/ArmPkg/Include/Library/ArmLib.h
>> @@ -780,6 +780,7 @@ EFIAPI
>>   ArmHasVhe (
>>     VOID
>>     );
>> +
>>   #endif // MDE_CPU_AARCH64
>>   
>>   #ifdef MDE_CPU_ARM
>> diff --git a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
>> index 7e548e4765c2..8e8e05d5a5f6 100644
>> --- a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
>> +++ b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
>> @@ -1,12 +1,12 @@
>>   /** @file
>>   
>> -  Copyright (c) 2017-2021, Arm Limited. All rights reserved.
>> +  Copyright (c) 2017-2023, Arm Limited. All rights reserved.
>>   
>>     SPDX-License-Identifier: BSD-2-Clause-Patent
>>   
>> -  System Control and Management Interface V1.0
>> -    http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/
>> -    DEN0056A_System_Control_and_Management_Interface.pdf
>> +  System Control and Management Interface, latest version:
> 
> I see this as a pattern throughout the series.
> But this statement will at some point become untrue; this
> implementation is written against a specific version. I think this
> version shold be reflected in the comment. (And that applies
> throughout the series.)
> 
>> +  - https://developer.arm.com/documentation/den0056/latest/
> 
> But I think the above is the most useful link.

I am not sure I understand completely. Do you mean that the SCMI
structures/interfaces defined in:
    ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
and that were written against the SCMI v1.0 specification should
not be used as such for other SCMI specification version ?
I.e. the same process as for the AcpiXX.h files
(MdePkg/Include/IndustryStandard/Acpi65.h) should be used ?

Or do you mean that the _CPC object generation implies that the
SCP should comply to the v2.0 version at least and this should be
reflected in the commit messages ?

Regards,
Pierre

> 
> /
>      Leif
> 
>> +
>>   **/
>>   
>>   #ifndef ARM_SCMI_PERFORMANCE_PROTOCOL_H_
>> @@ -14,7 +14,10 @@
>>   
>>   #include <Protocol/ArmScmi.h>
>>   
>> -#define PERFORMANCE_PROTOCOL_VERSION  0x10000
>> +/// Arm Scmi performance protocol versions.
>> +#define PERFORMANCE_PROTOCOL_VERSION_V1  0x10000
>> +#define PERFORMANCE_PROTOCOL_VERSION_V2  0x20000
>> +#define PERFORMANCE_PROTOCOL_VERSION_V3  0x30000
>>   
>>   #define ARM_SCMI_PERFORMANCE_PROTOCOL_GUID  { \
>>     0x9b8ba84, 0x3dd3, 0x49a6, {0xa0, 0x5a, 0x31, 0x34, 0xa5, 0xf0, 0x7b, 0xad} \
>> -- 
>> 2.25.1
>>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110515): https://edk2.groups.io/g/devel/message/110515
Mute This Topic: https://groups.io/mt/102175810/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 07/11] DynamicTablesPkg: Add PsdToken field to CM_ARM_GICC_INFO object

[PierreGondois]

On 10/26/23 12:45, Leif Lindholm wrote:
> On Wed, Oct 25, 2023 at 13:25:36 +0200, pierre.gondois@arm.com wrote:
>> From: Pierre Gondois <pierre.gondois@arm.com>
>>
>> The _PSD object (cf. ACPI 6.4, s8.4.5.5 _PSD (P-State Dependency)
>> allows to describe CPU's power state dependencies. Add a PsdToken
>> field to the CM_ARM_GICC_INFO object so that interdependent CPUs
>> can reference the same CM_ARM_PSD_INFO object.
>>
>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>> ---
>>   DynamicTablesPkg/Include/ArmNameSpaceObjects.h               | 5 +++++
>>   .../Common/TableHelperLib/ConfigurationManagerObjectParser.c | 5 +++--
>>   2 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
>> index ddd17fa45b1e..2a0ebe24bd04 100644
>> --- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
>> +++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
>> @@ -204,6 +204,11 @@ typedef struct CmArmGicCInfo {
>>         i.e. a token referencing a CM_ARM_CPC_INFO object.
>>     */
>>     CM_OBJECT_TOKEN    CpcToken;
>> +
>> +  /** Optional field: Reference Token for the Psd info of this processor.
>> +      i.e. a token referencing a CM_ARM_PSD_INFO object.
>> +  */
>> +  CM_OBJECT_TOKEN    PsdToken;
>>   } CM_ARM_GICC_INFO;
>>   
>>   /** A structure that describes the
>> diff --git a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
>> index b3ee12da8c4f..a9f5c95c1039 100644
>> --- a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
>> +++ b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
>> @@ -83,7 +83,8 @@ STATIC CONST CM_OBJ_PARSER  CmArmGicCInfoParser[] = {
>>     { "ProximityDomain",               4,                        "0x%x",   NULL },
>>     { "ClockDomain",                   4,                        "0x%x",   NULL },
>>     { "AffinityFlags",                 4,                        "0x%x",   NULL },
>> -  { "CpcToken",                      sizeof (CM_OBJECT_TOKEN), "0x%p",   NULL }
>> +  { "CpcToken",                      sizeof (CM_OBJECT_TOKEN), "0x%p",   NULL },
>> +  { "PsdToken",                      sizeof (CM_OBJECT_TOKEN), "0x%p",   NULL },
>>   };
>>   
>>   /** A parser for EArmObjGicDInfo.
>> @@ -766,7 +767,7 @@ STATIC CONST CM_OBJ_PARSER_ARRAY  ArmNamespaceObjectParser[] = {
>>       ARRAY_SIZE (CmArmPccSubspaceType34InfoParser) },
>>     { "EArmObjPccSubspaceType5Info",         CmArmPccSubspaceType5InfoParser,
>>       ARRAY_SIZE (CmArmPccSubspaceType5InfoParser) },
>> -  { "EArmObjCpcInfo",                      CmArmPsdInfoParser,
>> +  { "EArmObjPsdInfo",                      CmArmPsdInfoParser,
> 
> Can you add something to the commit message about this bit?

This is actually a naming mistake in:
    [PATCH v1 06/11] DynamicTablesPkg: Add CM_ARM_PSD_INFO object
I will correct it it there, the change in this patch should be removed.

Regards,
Pierre

> 
> /
>      Leif
> 
>>       ARRAY_SIZE (CmArmPsdInfoParser) },
>>     { "EArmObjMax",                          NULL,                                  0                                },
>>   };
>> -- 
>> 2.25.1
>>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110516): https://edk2.groups.io/g/devel/message/110516
Mute This Topic: https://groups.io/mt/102175817/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 11/11] DynamicTablesPkg: Remove check for _CPC field

[PierreGondois]

On 10/26/23 13:05, Leif Lindholm wrote:
> On Wed, Oct 25, 2023 at 13:25:40 +0200, pierre.gondois@arm.com wrote:
>> From: Pierre Gondois <pierre.gondois@arm.com>
>>
>> When generating _CPC objects, some fields are mandatory.
> 
> Mandatory by spec or mandatory by current API?
> If the former, could we either warn or add a Pcd to enable the more
> lenient behaviour?

They are mandatory by spec. These are actually the fields that are not
described as optional in:
    8.4.6.1 _CPC (Continuous Performance Control)
E.g.: 'Time Window Register': 'Optional. If supported, contains ...'

I will add a warning to signal them.

Regards,
Pierre

> 
> /
>      Leif
> 
>> Some fields cannot be supported by a the Juno platform, which is used
>> to test the _CPC generation. Therefore, don't prevent from generating
>> _CPC objects if the fields below are missing, and let the OS handle
>> the missing information.
>>
>> _CPC fields that are exempted from checks:
>> - PerformanceLimitedRegister
>> - ReferencePerformanceCounterRegister
>> - DeliveredPerformanceCounterRegister
>>
>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>> ---
>>   .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 19 +++++++++++++++----
>>   1 file changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
>> index f350083b148c..423e64f12606 100644
>> --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
>> +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
>> @@ -3531,6 +3531,11 @@ AmlCreateCpcNode (
>>       return EFI_INVALID_PARAMETER;
>>     }
>>   
>> +  // The following fields are theoretically mandatory, but not supported
>> +  // by some platforms. Don't check them:
>> +  // - PerformanceLimitedRegister
>> +  // - ReferencePerformanceCounterRegister
>> +  // - DeliveredPerformanceCounterRegister
>>     if ((IsNullGenericAddress (&CpcInfo->HighestPerformanceBuffer) &&
>>          (CpcInfo->HighestPerformanceInteger == 0)) ||
>>         (IsNullGenericAddress (&CpcInfo->NominalPerformanceBuffer) &&
>> @@ -3539,13 +3544,19 @@ AmlCreateCpcNode (
>>          (CpcInfo->LowestNonlinearPerformanceInteger == 0)) ||
>>         (IsNullGenericAddress (&CpcInfo->LowestPerformanceBuffer) &&
>>          (CpcInfo->LowestPerformanceInteger == 0)) ||
>> -      IsNullGenericAddress (&CpcInfo->DesiredPerformanceRegister) ||
>> -      IsNullGenericAddress (&CpcInfo->ReferencePerformanceCounterRegister) ||
>> -      IsNullGenericAddress (&CpcInfo->DeliveredPerformanceCounterRegister) ||
>> -      IsNullGenericAddress (&CpcInfo->PerformanceLimitedRegister))
>> +      IsNullGenericAddress (&CpcInfo->DesiredPerformanceRegister))
>>     {
>>       ASSERT (0);
>>       return EFI_INVALID_PARAMETER;
>> +  } else if ((IsNullGenericAddress (&CpcInfo->HighestPerformanceBuffer) &&
>> +              (CpcInfo->HighestPerformanceInteger == 0)) ||
>> +             (IsNullGenericAddress (&CpcInfo->NominalPerformanceBuffer) &&
>> +              (CpcInfo->NominalPerformanceInteger == 0)))
>> +  {
>> +    DEBUG ((
>> +      DEBUG_WARN,
>> +      "Missing Reference|Delivered performance field in _CPC object\n"
>> +      ));
>>     }
>>   
>>     CpcPackage = NULL;
>> -- 
>> 2.25.1
>>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110517): https://edk2.groups.io/g/devel/message/110517
Mute This Topic: https://groups.io/mt/102175822/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 02/11] ArmPkg/ArmScmiDxe: Add PERFORMANCE_DESCRIBE_FASTCHANNEL support

[Leif Lindholm]

On 2023-11-02 10:19, PierreGondois wrote:

>>> +/// SCMI Message Ids for the Performance Protocol.
>>> +typedef enum {
>>> +  ScmiMessageIdPerformanceDomainAttributes    = 0x3,
>>> +  ScmiMessageIdPerformanceDescribeLevels      = 0x4,
>>> +  ScmiMessageIdPerformanceLimitsSet           = 0x5,
>>> +  ScmiMessageIdPerformanceLimitsGet           = 0x6,
>>> +  ScmiMessageIdPerformanceLevelSet            = 0x7,
>>> +  ScmiMessageIdPerformanceLevelGet            = 0x8,
>>> +  ScmiMessageIdPerformanceDescribeFastchannel = 0xB,
>>> +} SCMI_MESSAGE_ID_PERFORMANCE;
>>
>> This struct appears to move in the code at the same time as having an
>> entry added to it. This seems superficially unmotivated.
>> However it is also moved to inside the pack(1) block, which makes no
>> sense for an enum.
> 
> Yes right, I will move it out of the pack(1) section.

Thx, with that change:
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>

> The reason to move the SCMI_MESSAGE_ID_PERFORMANCE definition up in the 
> file
> was that the SCMI_PERFORMANCE_DESCRIBE_FASTCHANNEL interface added in this
> patch takes a SCMI_MESSAGE_ID_PERFORMANCE parameter as an argument, so the
> enum needs to be defined before.
> I can add a comment about this in the commit message.

It doesn't really need to be in the commit message, but it's the kind of 
thing that can be helpful to point out below ---.

Regards,

Leif



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110527): https://edk2.groups.io/g/devel/message/110527
Mute This Topic: https://groups.io/mt/102175812/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 10/11] DynamicTablesPkg: Add ArmScmiInfoLib

[PierreGondois]

Hello Leif,

On 11/2/23 11:20, Pierre Gondois wrote:
> 
> 
> On 10/26/23 13:03, Leif Lindholm wrote:
>> On Wed, Oct 25, 2023 at 13:25:39 +0200, PierreGondois wrote:
>>> From: Pierre Gondois <pierre.gondois@arm.com>
>>>
>>> The SCP holds some power information that could be advertised
>>> through the _CPC object. The communication with the SCP is done
>>> through SCMI protocols (c.f. ArmScmiDxe).
>>>
>>> Use the SCMI protocols to query information and feed it to
>>> the DynamicTablesPkg.
>>
>> Couple of questions:
>> With a generic name like ArmScmiInfoLib, does is belong in ArmPkg or
>> MdeModulePkg?
>>
>> Or if it's more tightly integrated with DynamicTablesPkg (not
>> blatantly obvious from a quick skim below), should that be reflected
>> by the library name?
> 
> The library is tight to the DynamicTablesPkg as it produces DynamicTablesPkg
> specific objects. It uses the SCMI interface to fetch information from the SCP.
> The ScmiProtocol resides in the ArmPkg, thus the name.
> Does the name 'ScmiInfoLib' sound better ?

Should I keep ArmScmiInfoLib / ScmiInfoLib ?

Also I'm not sure if there is a change required to:
   [PATCH v2 01/11] ArmPkg/ArmScmiDxe: Rename PERFORMANCE_PROTOCOL_VERSION
regarding the spec. reference, cf. https://edk2.groups.io/g/devel/message/110515

Regards
Pierre

> 
>>
>> /
>>       Leif
>>
>>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>>> ---
>>>    DynamicTablesPkg/DynamicTables.dsc.inc        |   1 +
>>>    DynamicTablesPkg/DynamicTablesPkg.dec         |   3 +
>>>    DynamicTablesPkg/DynamicTablesPkg.dsc         |   1 +
>>>    .../Include/Library/ArmScmiInfoLib.h          |  33 ++
>>>    .../Library/ArmScmiInfoLib/ArmScmiInfoLib.c   | 294 ++++++++++++++++++
>>>    .../Library/ArmScmiInfoLib/ArmScmiInfoLib.inf |  31 ++
>>>    6 files changed, 363 insertions(+)
>>>    create mode 100644 DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h
>>>    create mode 100644 DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c
>>>    create mode 100644 DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
>>>
>>> diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc b/DynamicTablesPkg/DynamicTables.dsc.inc
>>> index 9d4312c4e87d..be40ebc4b472 100644
>>> --- a/DynamicTablesPkg/DynamicTables.dsc.inc
>>> +++ b/DynamicTablesPkg/DynamicTables.dsc.inc
>>> @@ -15,6 +15,7 @@ [BuildOptions]
>>>    [LibraryClasses.common]
>>>      AcpiHelperLib|DynamicTablesPkg/Library/Common/AcpiHelperLib/AcpiHelperLib.inf
>>>      AmlLib|DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
>>> +  ArmScmiInfoLib|DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
>>>      SsdtPcieSupportLib|DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
>>>      SsdtSerialPortFixupLib|DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.inf
>>>      TableHelperLib|DynamicTablesPkg/Library/Common/TableHelperLib/TableHelperLib.inf
>>> diff --git a/DynamicTablesPkg/DynamicTablesPkg.dec b/DynamicTablesPkg/DynamicTablesPkg.dec
>>> index cfbcbb9569f1..26498e5fec53 100644
>>> --- a/DynamicTablesPkg/DynamicTablesPkg.dec
>>> +++ b/DynamicTablesPkg/DynamicTablesPkg.dec
>>> @@ -42,6 +42,9 @@ [LibraryClasses]
>>>      ##  @libraryclass  Defines a set of SMBIOS string helper methods.
>>>      SmbiosStringTableLib|Include/Library/SmbiosStringTableLib.h
>>>    
>>> +  ##  @libraryclass  Defines a set of APIs to populate CmObj using SCMI.
>>> +  ArmScmiInfoLib|Include/Library/ArmScmiInfoLib.h
>>> +
>>>    [Protocols]
>>>      # Configuration Manager Protocol GUID
>>>      gEdkiiConfigurationManagerProtocolGuid = { 0xd85a4835, 0x5a82, 0x4894, { 0xac, 0x2, 0x70, 0x6f, 0x43, 0xd5, 0x97, 0x8e } }
>>> diff --git a/DynamicTablesPkg/DynamicTablesPkg.dsc b/DynamicTablesPkg/DynamicTablesPkg.dsc
>>> index bd5084a9008f..6ea86c9efdb0 100644
>>> --- a/DynamicTablesPkg/DynamicTablesPkg.dsc
>>> +++ b/DynamicTablesPkg/DynamicTablesPkg.dsc
>>> @@ -39,6 +39,7 @@ [LibraryClasses.ARM, LibraryClasses.AARCH64]
>>>      PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
>>>    
>>>    [Components.common]
>>> +  DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
>>>      DynamicTablesPkg/Library/Common/AcpiHelperLib/AcpiHelperLib.inf
>>>      DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
>>>      DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
>>> diff --git a/DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h b/DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h
>>> new file mode 100644
>>> index 000000000000..8d3fb31df13c
>>> --- /dev/null
>>> +++ b/DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h
>>> @@ -0,0 +1,33 @@
>>> +/** @file
>>> +  Arm SCMI Info Library.
>>> +
>>> +  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.<BR>
>>> +
>>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>>> +**/
>>> +
>>> +#ifndef ARM_SCMI_INFO_LIB_H_
>>> +#define ARM_SCMI_INFO_LIB_H_
>>> +
>>> +#include <ConfigurationManagerObject.h>
>>> +
>>> +/** Populate a AML_CPC_INFO object based on SCMI information.
>>> +
>>> +  @param[in]  DomainId    Identifier for the performance domain.
>>> +  @param[out] CpcInfo     If success, this structure was populated from
>>> +                          information queried to the SCP.
>>> +
>>> +  @retval EFI_SUCCESS             Success.
>>> +  @retval EFI_DEVICE_ERROR        Device error.
>>> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>>> +  @retval EFI_TIMEOUT             Time out.
>>> +  @retval EFI_UNSUPPORTED         Unsupported.
>>> +**/
>>> +EFI_STATUS
>>> +EFIAPI
>>> +ArmScmiInfoGetFastChannel (
>>> +  IN  UINT32        DomainId,
>>> +  OUT AML_CPC_INFO  *CpcInfo
>>> +  );
>>> +
>>> +#endif // ARM_SCMI_INFO_LIB_H_
>>> diff --git a/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c
>>> new file mode 100644
>>> index 000000000000..c23bff63bb6f
>>> --- /dev/null
>>> +++ b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c
>>> @@ -0,0 +1,294 @@
>>> +/** @file
>>> +  Arm SCMI Info Library.
>>> +
>>> +  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.<BR>
>>> +
>>> +  Arm Functional Fixed Hardware Specification:
>>> +  - https://developer.arm.com/documentation/den0048/latest/
>>> +
>>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>>> +**/
>>> +
>>> +#include <Library/AcpiLib.h>
>>> +#include <Library/ArmScmiInfoLib.h>
>>> +#include <Library/DebugLib.h>
>>> +#include <Library/MemoryAllocationLib.h>
>>> +#include <Library/UefiBootServicesTableLib.h>
>>> +#include <Protocol/ArmScmi.h>
>>> +#include <Protocol/ArmScmiPerformanceProtocol.h>
>>> +
>>> +/** Arm FFH registers
>>> +
>>> +  Cf. Arm Functional Fixed Hardware Specification
>>> +  s3.2 Performance management and Collaborative Processor Performance Control
>>> +*/
>>> +#define ARM_FFH_DELIVERED_PERF_COUNTER_REGISTER  0x0
>>> +#define ARM_FFH_REFERENCE_PERF_COUNTER_REGISTER  0x1
>>> +
>>> +/// Arm SCMI performance protocol.
>>> +STATIC SCMI_PERFORMANCE_PROTOCOL  *ScmiPerfProtocol;
>>> +
>>> +/** Arm SCMI Info Library constructor.
>>> +
>>> +  @param  ImageHandle   Image of the loaded driver.
>>> +  @param  SystemTable   Pointer to the System Table.
>>> +
>>> +  @retval EFI_SUCCESS             Success.
>>> +  @retval EFI_DEVICE_ERROR        Device error.
>>> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>>> +  @retval EFI_NOT_FOUND           Not Found
>>> +  @retval EFI_TIMEOUT             Timeout.
>>> +  @retval EFI_UNSUPPORTED         Unsupported.
>>> +**/
>>> +EFI_STATUS
>>> +EFIAPI
>>> +ArmScmiInfoLibConstructor (
>>> +  IN  EFI_HANDLE        ImageHandle,
>>> +  IN  EFI_SYSTEM_TABLE  *SystemTable
>>> +  )
>>> +{
>>> +  EFI_STATUS  Status;
>>> +  UINT32      Version;
>>> +
>>> +  Status = gBS->LocateProtocol (
>>> +                  &gArmScmiPerformanceProtocolGuid,
>>> +                  NULL,
>>> +                  (VOID **)&ScmiPerfProtocol
>>> +                  );
>>> +  if (EFI_ERROR (Status)) {
>>> +    return Status;
>>> +  }
>>> +
>>> +  Status = ScmiPerfProtocol->GetVersion (ScmiPerfProtocol, &Version);
>>> +  if (EFI_ERROR (Status)) {
>>> +    return Status;
>>> +  }
>>> +
>>> +  // FastChannels were added in SCMI v2.0 spec.
>>> +  if (Version < PERFORMANCE_PROTOCOL_VERSION_V2) {
>>> +    DEBUG ((DEBUG_ERROR, "ArmScmiInfoLib requires SCMI version > 2.0\n"));
>>> +    return EFI_UNSUPPORTED;
>>> +  }
>>> +
>>> +  return Status;
>>> +}
>>> +
>>> +/** Get the OPPs/performance states of a power domain.
>>> +
>>> +  This function is a wrapper around the SCMI PERFORMANCE_DESCRIBE_LEVELS
>>> +  command. The list of discrete performance states is returned in a buffer
>>> +  that must be freed by the caller.
>>> +
>>> +  @param[in]  DomainId        Identifier for the performance domain.
>>> +  @param[out] LevelArray      If success, pointer to the list of list of
>>> +                              performance state. This memory must be freed by
>>> +                              the caller.
>>> +  @param[out] LevelArrayCount If success, contains the number of states in
>>> +                              LevelArray.
>>> +
>>> +  @retval EFI_SUCCESS             Success.
>>> +  @retval EFI_DEVICE_ERROR        Device error.
>>> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>>> +  @retval EFI_TIMEOUT             Time out.
>>> +  @retval EFI_UNSUPPORTED         Unsupported.
>>> +**/
>>> +STATIC
>>> +EFI_STATUS
>>> +EFIAPI
>>> +ArmScmiInfoDescribeLevels (
>>> +  IN  UINT32                  DomainId,
>>> +  OUT SCMI_PERFORMANCE_LEVEL  **LevelArray,
>>> +  OUT UINT32                  *LevelArrayCount
>>> +  )
>>> +{
>>> +  EFI_STATUS              Status;
>>> +  SCMI_PERFORMANCE_LEVEL  *Array;
>>> +  UINT32                  Count;
>>> +  UINT32                  Size;
>>> +
>>> +  if ((ScmiPerfProtocol == NULL)  ||
>>> +      (LevelArray == NULL)  ||
>>> +      (LevelArrayCount == NULL))
>>> +  {
>>> +    return EFI_INVALID_PARAMETER;
>>> +  }
>>> +
>>> +  // First call to get the number of levels.
>>> +  Size   = 0;
>>> +  Status = ScmiPerfProtocol->DescribeLevels (
>>> +                               ScmiPerfProtocol,
>>> +                               DomainId,
>>> +                               &Count,
>>> +                               &Size,
>>> +                               NULL
>>> +                               );
>>> +  if (Status != EFI_BUFFER_TOO_SMALL) {
>>> +    // EFI_SUCCESS is not a valid option.
>>> +    if (Status == EFI_SUCCESS) {
>>> +      return EFI_INVALID_PARAMETER;
>>> +    } else {
>>> +      return Status;
>>> +    }
>>> +  }
>>> +
>>> +  Array = AllocateZeroPool (Size);
>>> +  if (Array == NULL) {
>>> +    return EFI_OUT_OF_RESOURCES;
>>> +  }
>>> +
>>> +  // Second call to get the descriptions of the levels.
>>> +  Status = ScmiPerfProtocol->DescribeLevels (
>>> +                               ScmiPerfProtocol,
>>> +                               DomainId,
>>> +                               &Count,
>>> +                               &Size,
>>> +                               Array
>>> +                               );
>>> +  if (EFI_ERROR (Status)) {
>>> +    return Status;
>>> +  }
>>> +
>>> +  *LevelArray      = Array;
>>> +  *LevelArrayCount = Count;
>>> +
>>> +  return Status;
>>> +}
>>> +
>>> +/** Populate a AML_CPC_INFO object based on SCMI information.
>>> +
>>> +  @param[in]  DomainId    Identifier for the performance domain.
>>> +  @param[out] CpcInfo     If success, this structure was populated from
>>> +                          information queried to the SCP.
>>> +
>>> +  @retval EFI_SUCCESS             Success.
>>> +  @retval EFI_DEVICE_ERROR        Device error.
>>> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>>> +  @retval EFI_TIMEOUT             Time out.
>>> +  @retval EFI_UNSUPPORTED         Unsupported.
>>> +**/
>>> +EFI_STATUS
>>> +EFIAPI
>>> +ArmScmiInfoGetFastChannel (
>>> +  IN  UINT32        DomainId,
>>> +  OUT AML_CPC_INFO  *CpcInfo
>>> +  )
>>> +{
>>> +  EFI_STATUS                          Status;
>>> +  SCMI_PERFORMANCE_FASTCHANNEL        FcLevelGet;
>>> +  SCMI_PERFORMANCE_FASTCHANNEL        FcLimitsSet;
>>> +  SCMI_PERFORMANCE_DOMAIN_ATTRIBUTES  DomainAttributes;
>>> +
>>> +  SCMI_PERFORMANCE_LEVEL  *LevelArray;
>>> +  UINT32                  LevelCount;
>>> +
>>> +  UINT64  FcLevelGetAddr;
>>> +  UINT64  FcLimitsMaxSetAddr;
>>> +  UINT64  FcLimitsMinSetAddr;
>>> +
>>> +  if ((ScmiPerfProtocol == NULL)  ||
>>> +      (CpcInfo == NULL))
>>> +  {
>>> +    return EFI_INVALID_PARAMETER;
>>> +  }
>>> +
>>> +  Status = ScmiPerfProtocol->DescribeFastchannel (
>>> +                               ScmiPerfProtocol,
>>> +                               DomainId,
>>> +                               ScmiMessageIdPerformanceLevelSet,
>>> +                               &FcLevelGet
>>> +                               );
>>> +  if (EFI_ERROR (Status)) {
>>> +    return Status;
>>> +  }
>>> +
>>> +  Status = ScmiPerfProtocol->DescribeFastchannel (
>>> +                               ScmiPerfProtocol,
>>> +                               DomainId,
>>> +                               ScmiMessageIdPerformanceLimitsSet,
>>> +                               &FcLimitsSet
>>> +                               );
>>> +  if (EFI_ERROR (Status)) {
>>> +    return Status;
>>> +  }
>>> +
>>> +  Status = ScmiPerfProtocol->GetDomainAttributes (
>>> +                               ScmiPerfProtocol,
>>> +                               DomainId,
>>> +                               &DomainAttributes
>>> +                               );
>>> +  if (EFI_ERROR (Status)) {
>>> +    return Status;
>>> +  }
>>> +
>>> +  Status = ArmScmiInfoDescribeLevels (DomainId, &LevelArray, &LevelCount);
>>> +  if (EFI_ERROR (Status)) {
>>> +    return Status;
>>> +  }
>>> +
>>> +  /* Do some safety checks.
>>> +     Only support FastChannels (and not doorbells) as this is
>>> +     the only mechanism supported by SCP.
>>> +     FcLimits[Get|Set] require 2 UINT32 values (max, then min) and
>>> +     FcLimits[Get|Set] require 1 UINT32 value (level).
>>> +  */
>>> +  if ((FcLevelGet.ChanSize != sizeof (UINT32))  ||
>>> +      ((FcLevelGet.Attributes & SCMI_PERF_FC_ATTRIB_HAS_DOORBELL) ==
>>> +       SCMI_PERF_FC_ATTRIB_HAS_DOORBELL) ||
>>> +      (FcLimitsSet.ChanSize != 2 * sizeof (UINT32)) ||
>>> +      ((FcLimitsSet.Attributes & SCMI_PERF_FC_ATTRIB_HAS_DOORBELL) ==
>>> +       SCMI_PERF_FC_ATTRIB_HAS_DOORBELL))
>>> +  {
>>> +    Status = EFI_INVALID_PARAMETER;
>>> +    goto exit_handler;
>>> +  }
>>> +
>>> +  FcLevelGetAddr = ((UINT64)FcLevelGet.ChanAddrHigh << 32) |
>>> +                   FcLevelGet.ChanAddrLow;
>>> +  FcLimitsMaxSetAddr = ((UINT64)FcLimitsSet.ChanAddrHigh << 32) |
>>> +                       FcLimitsSet.ChanAddrLow;
>>> +  FcLimitsMinSetAddr = FcLimitsMaxSetAddr + 0x4;
>>> +
>>> +  CpcInfo->Revision                          = EFI_ACPI_6_4_AML_CPC_REVISION_V3;
>>> +  CpcInfo->HighestPerformanceInteger         = LevelArray[LevelCount - 1].Level;
>>> +  CpcInfo->NominalPerformanceInteger         = DomainAttributes.SustainedPerfLevel;
>>> +  CpcInfo->LowestNonlinearPerformanceInteger = LevelArray[0].Level;
>>> +  CpcInfo->LowestPerformanceInteger          = LevelArray[0].Level;
>>> +
>>> +  CpcInfo->DesiredPerformanceRegister.AddressSpaceId    = EFI_ACPI_6_4_SYSTEM_MEMORY;
>>> +  CpcInfo->DesiredPerformanceRegister.RegisterBitWidth  = 32;
>>> +  CpcInfo->DesiredPerformanceRegister.RegisterBitOffset = 0;
>>> +  CpcInfo->DesiredPerformanceRegister.AccessSize        = EFI_ACPI_6_4_DWORD;
>>> +  CpcInfo->DesiredPerformanceRegister.Address           = FcLevelGetAddr;
>>> +
>>> +  CpcInfo->MinimumPerformanceRegister.AddressSpaceId    = EFI_ACPI_6_4_SYSTEM_MEMORY;
>>> +  CpcInfo->MinimumPerformanceRegister.RegisterBitWidth  = 32;
>>> +  CpcInfo->MinimumPerformanceRegister.RegisterBitOffset = 0;
>>> +  CpcInfo->MinimumPerformanceRegister.AccessSize        = EFI_ACPI_6_4_DWORD;
>>> +  CpcInfo->MinimumPerformanceRegister.Address           = FcLimitsMinSetAddr;
>>> +
>>> +  CpcInfo->MaximumPerformanceRegister.AddressSpaceId    = EFI_ACPI_6_4_SYSTEM_MEMORY;
>>> +  CpcInfo->MaximumPerformanceRegister.RegisterBitWidth  = 32;
>>> +  CpcInfo->MaximumPerformanceRegister.RegisterBitOffset = 0;
>>> +  CpcInfo->MaximumPerformanceRegister.AccessSize        = EFI_ACPI_6_4_DWORD;
>>> +  CpcInfo->MaximumPerformanceRegister.Address           = FcLimitsMaxSetAddr;
>>> +
>>> +  CpcInfo->ReferencePerformanceCounterRegister.AddressSpaceId    = EFI_ACPI_6_4_FUNCTIONAL_FIXED_HARDWARE;
>>> +  CpcInfo->ReferencePerformanceCounterRegister.RegisterBitWidth  = 0x40;
>>> +  CpcInfo->ReferencePerformanceCounterRegister.RegisterBitOffset = 0;
>>> +  CpcInfo->ReferencePerformanceCounterRegister.AccessSize        = ARM_FFH_REFERENCE_PERF_COUNTER_REGISTER;
>>> +  CpcInfo->ReferencePerformanceCounterRegister.Address           = 0x4;
>>> +
>>> +  CpcInfo->DeliveredPerformanceCounterRegister.AddressSpaceId    = EFI_ACPI_6_4_FUNCTIONAL_FIXED_HARDWARE;
>>> +  CpcInfo->DeliveredPerformanceCounterRegister.RegisterBitWidth  = 0x40;
>>> +  CpcInfo->DeliveredPerformanceCounterRegister.RegisterBitOffset = 0;
>>> +  CpcInfo->DeliveredPerformanceCounterRegister.AccessSize        = ARM_FFH_DELIVERED_PERF_COUNTER_REGISTER;
>>> +  CpcInfo->DeliveredPerformanceCounterRegister.Address           = 0x4;
>>> +
>>> +  // SCMI should advertise performance values on a unified scale. So frequency
>>> +  // values are not available. LowestFrequencyInteger and
>>> +  // NominalFrequencyInteger are populated in the ConfigurationManager.
>>> +
>>> +exit_handler:
>>> +  FreePool (LevelArray);
>>> +  return Status;
>>> +}
>>> diff --git a/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
>>> new file mode 100644
>>> index 000000000000..aad3f0fa7b83
>>> --- /dev/null
>>> +++ b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
>>> @@ -0,0 +1,31 @@
>>> +## @file
>>> +#  Arm SCMI Info Library.
>>> +#
>>> +#  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.
>>> +#
>>> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
>>> +##
>>> +
>>> +[Defines]
>>> +  INF_VERSION    = 0x0001001B
>>> +  BASE_NAME      = ArmScmiInfoLib
>>> +  FILE_GUID      = 1A7CDB04-9FFC-40DA-A87C-A5ACADAF8136
>>> +  VERSION_STRING = 1.0
>>> +  MODULE_TYPE    = DXE_DRIVER
>>> +  LIBRARY_CLASS  = ArmScmiInfoLib
>>> +  CONSTRUCTOR    = ArmScmiInfoLibConstructor
>>> +
>>> +[Sources]
>>> +  ArmScmiInfoLib.c
>>> +
>>> +[Packages]
>>> +  ArmPkg/ArmPkg.dec
>>> +  DynamicTablesPkg/DynamicTablesPkg.dec
>>> +  EmbeddedPkg/EmbeddedPkg.dec
>>> +  MdePkg/MdePkg.dec
>>> +
>>> +[Protocols]
>>> +  gArmScmiPerformanceProtocolGuid   ## CONSUMES
>>> +
>>> +[Depex]
>>> +  gArmScmiPerformanceProtocolGuid
>>> -- 
>>> 2.25.1
>>>
>>>
>>>
>>> 
>>>
>>>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110962): https://edk2.groups.io/g/devel/message/110962
Mute This Topic: https://groups.io/mt/102175821/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 10/11] DynamicTablesPkg: Add ArmScmiInfoLib

[Leif Lindholm]

On Thu, Nov 09, 2023 at 10:58:58 +0100, Pierre Gondois wrote:
> Hello Leif,
> 
> On 11/2/23 11:20, Pierre Gondois wrote:
> > 
> > 
> > On 10/26/23 13:03, Leif Lindholm wrote:
> > > On Wed, Oct 25, 2023 at 13:25:39 +0200, PierreGondois wrote:
> > > > From: Pierre Gondois <pierre.gondois@arm.com>
> > > > 
> > > > The SCP holds some power information that could be advertised
> > > > through the _CPC object. The communication with the SCP is done
> > > > through SCMI protocols (c.f. ArmScmiDxe).
> > > > 
> > > > Use the SCMI protocols to query information and feed it to
> > > > the DynamicTablesPkg.
> > > 
> > > Couple of questions:
> > > With a generic name like ArmScmiInfoLib, does is belong in ArmPkg or
> > > MdeModulePkg?
> > > 
> > > Or if it's more tightly integrated with DynamicTablesPkg (not
> > > blatantly obvious from a quick skim below), should that be reflected
> > > by the library name?
> > 
> > The library is tight to the DynamicTablesPkg as it produces DynamicTablesPkg
> > specific objects. It uses the SCMI interface to fetch information from the SCP.
> > The ScmiProtocol resides in the ArmPkg, thus the name.
> > Does the name 'ScmiInfoLib' sound better ?
> 
> Should I keep ArmScmiInfoLib / ScmiInfoLib ?

I know it gets tedious with the long names, but if it's fully
integrated with DynamicTablesPkg, then it's kind of important that the
name does not imply it's generic; i.e., the name should be DynamicTable*.

If it helps clarify my thinking, my litmus test for this is I
don't want to have to go searching through code to determine whether

#include <Library/ArmScmi*.h>

imports an Arm-specification header or a package-local helper.

> Also I'm not sure if there is a change required to:
>   [PATCH v2 01/11] ArmPkg/ArmScmiDxe: Rename PERFORMANCE_PROTOCOL_VERSION
> regarding the spec. reference, cf. https://edk2.groups.io/g/devel/message/110515

I'm not sure I follow? A change in what way?

I agree the name is too generic. I didn't flag it because it replaces
a non-ideal thing with something that isn't any worse, and I'm trying
to cut down on asking people to fix existing quirks when adding new
features.

If you're happy to rename it while you're at it, I'm happy to take it.

Regards,

Leif

> Regards
> Pierre
> 
> > 
> > > 
> > > /
> > >       Leif
> > > 
> > > > Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> > > > ---
> > > >    DynamicTablesPkg/DynamicTables.dsc.inc        |   1 +
> > > >    DynamicTablesPkg/DynamicTablesPkg.dec         |   3 +
> > > >    DynamicTablesPkg/DynamicTablesPkg.dsc         |   1 +
> > > >    .../Include/Library/ArmScmiInfoLib.h          |  33 ++
> > > >    .../Library/ArmScmiInfoLib/ArmScmiInfoLib.c   | 294 ++++++++++++++++++
> > > >    .../Library/ArmScmiInfoLib/ArmScmiInfoLib.inf |  31 ++
> > > >    6 files changed, 363 insertions(+)
> > > >    create mode 100644 DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h
> > > >    create mode 100644 DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c
> > > >    create mode 100644 DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
> > > > 
> > > > diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc b/DynamicTablesPkg/DynamicTables.dsc.inc
> > > > index 9d4312c4e87d..be40ebc4b472 100644
> > > > --- a/DynamicTablesPkg/DynamicTables.dsc.inc
> > > > +++ b/DynamicTablesPkg/DynamicTables.dsc.inc
> > > > @@ -15,6 +15,7 @@ [BuildOptions]
> > > >    [LibraryClasses.common]
> > > >      AcpiHelperLib|DynamicTablesPkg/Library/Common/AcpiHelperLib/AcpiHelperLib.inf
> > > >      AmlLib|DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
> > > > +  ArmScmiInfoLib|DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
> > > >      SsdtPcieSupportLib|DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
> > > >      SsdtSerialPortFixupLib|DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.inf
> > > >      TableHelperLib|DynamicTablesPkg/Library/Common/TableHelperLib/TableHelperLib.inf
> > > > diff --git a/DynamicTablesPkg/DynamicTablesPkg.dec b/DynamicTablesPkg/DynamicTablesPkg.dec
> > > > index cfbcbb9569f1..26498e5fec53 100644
> > > > --- a/DynamicTablesPkg/DynamicTablesPkg.dec
> > > > +++ b/DynamicTablesPkg/DynamicTablesPkg.dec
> > > > @@ -42,6 +42,9 @@ [LibraryClasses]
> > > >      ##  @libraryclass  Defines a set of SMBIOS string helper methods.
> > > >      SmbiosStringTableLib|Include/Library/SmbiosStringTableLib.h
> > > > +  ##  @libraryclass  Defines a set of APIs to populate CmObj using SCMI.
> > > > +  ArmScmiInfoLib|Include/Library/ArmScmiInfoLib.h
> > > > +
> > > >    [Protocols]
> > > >      # Configuration Manager Protocol GUID
> > > >      gEdkiiConfigurationManagerProtocolGuid = { 0xd85a4835, 0x5a82, 0x4894, { 0xac, 0x2, 0x70, 0x6f, 0x43, 0xd5, 0x97, 0x8e } }
> > > > diff --git a/DynamicTablesPkg/DynamicTablesPkg.dsc b/DynamicTablesPkg/DynamicTablesPkg.dsc
> > > > index bd5084a9008f..6ea86c9efdb0 100644
> > > > --- a/DynamicTablesPkg/DynamicTablesPkg.dsc
> > > > +++ b/DynamicTablesPkg/DynamicTablesPkg.dsc
> > > > @@ -39,6 +39,7 @@ [LibraryClasses.ARM, LibraryClasses.AARCH64]
> > > >      PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
> > > >    [Components.common]
> > > > +  DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
> > > >      DynamicTablesPkg/Library/Common/AcpiHelperLib/AcpiHelperLib.inf
> > > >      DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
> > > >      DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
> > > > diff --git a/DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h b/DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h
> > > > new file mode 100644
> > > > index 000000000000..8d3fb31df13c
> > > > --- /dev/null
> > > > +++ b/DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h
> > > > @@ -0,0 +1,33 @@
> > > > +/** @file
> > > > +  Arm SCMI Info Library.
> > > > +
> > > > +  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.<BR>
> > > > +
> > > > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > +**/
> > > > +
> > > > +#ifndef ARM_SCMI_INFO_LIB_H_
> > > > +#define ARM_SCMI_INFO_LIB_H_
> > > > +
> > > > +#include <ConfigurationManagerObject.h>
> > > > +
> > > > +/** Populate a AML_CPC_INFO object based on SCMI information.
> > > > +
> > > > +  @param[in]  DomainId    Identifier for the performance domain.
> > > > +  @param[out] CpcInfo     If success, this structure was populated from
> > > > +                          information queried to the SCP.
> > > > +
> > > > +  @retval EFI_SUCCESS             Success.
> > > > +  @retval EFI_DEVICE_ERROR        Device error.
> > > > +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> > > > +  @retval EFI_TIMEOUT             Time out.
> > > > +  @retval EFI_UNSUPPORTED         Unsupported.
> > > > +**/
> > > > +EFI_STATUS
> > > > +EFIAPI
> > > > +ArmScmiInfoGetFastChannel (
> > > > +  IN  UINT32        DomainId,
> > > > +  OUT AML_CPC_INFO  *CpcInfo
> > > > +  );
> > > > +
> > > > +#endif // ARM_SCMI_INFO_LIB_H_
> > > > diff --git a/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c
> > > > new file mode 100644
> > > > index 000000000000..c23bff63bb6f
> > > > --- /dev/null
> > > > +++ b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c
> > > > @@ -0,0 +1,294 @@
> > > > +/** @file
> > > > +  Arm SCMI Info Library.
> > > > +
> > > > +  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.<BR>
> > > > +
> > > > +  Arm Functional Fixed Hardware Specification:
> > > > +  - https://developer.arm.com/documentation/den0048/latest/
> > > > +
> > > > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > +**/
> > > > +
> > > > +#include <Library/AcpiLib.h>
> > > > +#include <Library/ArmScmiInfoLib.h>
> > > > +#include <Library/DebugLib.h>
> > > > +#include <Library/MemoryAllocationLib.h>
> > > > +#include <Library/UefiBootServicesTableLib.h>
> > > > +#include <Protocol/ArmScmi.h>
> > > > +#include <Protocol/ArmScmiPerformanceProtocol.h>
> > > > +
> > > > +/** Arm FFH registers
> > > > +
> > > > +  Cf. Arm Functional Fixed Hardware Specification
> > > > +  s3.2 Performance management and Collaborative Processor Performance Control
> > > > +*/
> > > > +#define ARM_FFH_DELIVERED_PERF_COUNTER_REGISTER  0x0
> > > > +#define ARM_FFH_REFERENCE_PERF_COUNTER_REGISTER  0x1
> > > > +
> > > > +/// Arm SCMI performance protocol.
> > > > +STATIC SCMI_PERFORMANCE_PROTOCOL  *ScmiPerfProtocol;
> > > > +
> > > > +/** Arm SCMI Info Library constructor.
> > > > +
> > > > +  @param  ImageHandle   Image of the loaded driver.
> > > > +  @param  SystemTable   Pointer to the System Table.
> > > > +
> > > > +  @retval EFI_SUCCESS             Success.
> > > > +  @retval EFI_DEVICE_ERROR        Device error.
> > > > +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> > > > +  @retval EFI_NOT_FOUND           Not Found
> > > > +  @retval EFI_TIMEOUT             Timeout.
> > > > +  @retval EFI_UNSUPPORTED         Unsupported.
> > > > +**/
> > > > +EFI_STATUS
> > > > +EFIAPI
> > > > +ArmScmiInfoLibConstructor (
> > > > +  IN  EFI_HANDLE        ImageHandle,
> > > > +  IN  EFI_SYSTEM_TABLE  *SystemTable
> > > > +  )
> > > > +{
> > > > +  EFI_STATUS  Status;
> > > > +  UINT32      Version;
> > > > +
> > > > +  Status = gBS->LocateProtocol (
> > > > +                  &gArmScmiPerformanceProtocolGuid,
> > > > +                  NULL,
> > > > +                  (VOID **)&ScmiPerfProtocol
> > > > +                  );
> > > > +  if (EFI_ERROR (Status)) {
> > > > +    return Status;
> > > > +  }
> > > > +
> > > > +  Status = ScmiPerfProtocol->GetVersion (ScmiPerfProtocol, &Version);
> > > > +  if (EFI_ERROR (Status)) {
> > > > +    return Status;
> > > > +  }
> > > > +
> > > > +  // FastChannels were added in SCMI v2.0 spec.
> > > > +  if (Version < PERFORMANCE_PROTOCOL_VERSION_V2) {
> > > > +    DEBUG ((DEBUG_ERROR, "ArmScmiInfoLib requires SCMI version > 2.0\n"));
> > > > +    return EFI_UNSUPPORTED;
> > > > +  }
> > > > +
> > > > +  return Status;
> > > > +}
> > > > +
> > > > +/** Get the OPPs/performance states of a power domain.
> > > > +
> > > > +  This function is a wrapper around the SCMI PERFORMANCE_DESCRIBE_LEVELS
> > > > +  command. The list of discrete performance states is returned in a buffer
> > > > +  that must be freed by the caller.
> > > > +
> > > > +  @param[in]  DomainId        Identifier for the performance domain.
> > > > +  @param[out] LevelArray      If success, pointer to the list of list of
> > > > +                              performance state. This memory must be freed by
> > > > +                              the caller.
> > > > +  @param[out] LevelArrayCount If success, contains the number of states in
> > > > +                              LevelArray.
> > > > +
> > > > +  @retval EFI_SUCCESS             Success.
> > > > +  @retval EFI_DEVICE_ERROR        Device error.
> > > > +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> > > > +  @retval EFI_TIMEOUT             Time out.
> > > > +  @retval EFI_UNSUPPORTED         Unsupported.
> > > > +**/
> > > > +STATIC
> > > > +EFI_STATUS
> > > > +EFIAPI
> > > > +ArmScmiInfoDescribeLevels (
> > > > +  IN  UINT32                  DomainId,
> > > > +  OUT SCMI_PERFORMANCE_LEVEL  **LevelArray,
> > > > +  OUT UINT32                  *LevelArrayCount
> > > > +  )
> > > > +{
> > > > +  EFI_STATUS              Status;
> > > > +  SCMI_PERFORMANCE_LEVEL  *Array;
> > > > +  UINT32                  Count;
> > > > +  UINT32                  Size;
> > > > +
> > > > +  if ((ScmiPerfProtocol == NULL)  ||
> > > > +      (LevelArray == NULL)  ||
> > > > +      (LevelArrayCount == NULL))
> > > > +  {
> > > > +    return EFI_INVALID_PARAMETER;
> > > > +  }
> > > > +
> > > > +  // First call to get the number of levels.
> > > > +  Size   = 0;
> > > > +  Status = ScmiPerfProtocol->DescribeLevels (
> > > > +                               ScmiPerfProtocol,
> > > > +                               DomainId,
> > > > +                               &Count,
> > > > +                               &Size,
> > > > +                               NULL
> > > > +                               );
> > > > +  if (Status != EFI_BUFFER_TOO_SMALL) {
> > > > +    // EFI_SUCCESS is not a valid option.
> > > > +    if (Status == EFI_SUCCESS) {
> > > > +      return EFI_INVALID_PARAMETER;
> > > > +    } else {
> > > > +      return Status;
> > > > +    }
> > > > +  }
> > > > +
> > > > +  Array = AllocateZeroPool (Size);
> > > > +  if (Array == NULL) {
> > > > +    return EFI_OUT_OF_RESOURCES;
> > > > +  }
> > > > +
> > > > +  // Second call to get the descriptions of the levels.
> > > > +  Status = ScmiPerfProtocol->DescribeLevels (
> > > > +                               ScmiPerfProtocol,
> > > > +                               DomainId,
> > > > +                               &Count,
> > > > +                               &Size,
> > > > +                               Array
> > > > +                               );
> > > > +  if (EFI_ERROR (Status)) {
> > > > +    return Status;
> > > > +  }
> > > > +
> > > > +  *LevelArray      = Array;
> > > > +  *LevelArrayCount = Count;
> > > > +
> > > > +  return Status;
> > > > +}
> > > > +
> > > > +/** Populate a AML_CPC_INFO object based on SCMI information.
> > > > +
> > > > +  @param[in]  DomainId    Identifier for the performance domain.
> > > > +  @param[out] CpcInfo     If success, this structure was populated from
> > > > +                          information queried to the SCP.
> > > > +
> > > > +  @retval EFI_SUCCESS             Success.
> > > > +  @retval EFI_DEVICE_ERROR        Device error.
> > > > +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> > > > +  @retval EFI_TIMEOUT             Time out.
> > > > +  @retval EFI_UNSUPPORTED         Unsupported.
> > > > +**/
> > > > +EFI_STATUS
> > > > +EFIAPI
> > > > +ArmScmiInfoGetFastChannel (
> > > > +  IN  UINT32        DomainId,
> > > > +  OUT AML_CPC_INFO  *CpcInfo
> > > > +  )
> > > > +{
> > > > +  EFI_STATUS                          Status;
> > > > +  SCMI_PERFORMANCE_FASTCHANNEL        FcLevelGet;
> > > > +  SCMI_PERFORMANCE_FASTCHANNEL        FcLimitsSet;
> > > > +  SCMI_PERFORMANCE_DOMAIN_ATTRIBUTES  DomainAttributes;
> > > > +
> > > > +  SCMI_PERFORMANCE_LEVEL  *LevelArray;
> > > > +  UINT32                  LevelCount;
> > > > +
> > > > +  UINT64  FcLevelGetAddr;
> > > > +  UINT64  FcLimitsMaxSetAddr;
> > > > +  UINT64  FcLimitsMinSetAddr;
> > > > +
> > > > +  if ((ScmiPerfProtocol == NULL)  ||
> > > > +      (CpcInfo == NULL))
> > > > +  {
> > > > +    return EFI_INVALID_PARAMETER;
> > > > +  }
> > > > +
> > > > +  Status = ScmiPerfProtocol->DescribeFastchannel (
> > > > +                               ScmiPerfProtocol,
> > > > +                               DomainId,
> > > > +                               ScmiMessageIdPerformanceLevelSet,
> > > > +                               &FcLevelGet
> > > > +                               );
> > > > +  if (EFI_ERROR (Status)) {
> > > > +    return Status;
> > > > +  }
> > > > +
> > > > +  Status = ScmiPerfProtocol->DescribeFastchannel (
> > > > +                               ScmiPerfProtocol,
> > > > +                               DomainId,
> > > > +                               ScmiMessageIdPerformanceLimitsSet,
> > > > +                               &FcLimitsSet
> > > > +                               );
> > > > +  if (EFI_ERROR (Status)) {
> > > > +    return Status;
> > > > +  }
> > > > +
> > > > +  Status = ScmiPerfProtocol->GetDomainAttributes (
> > > > +                               ScmiPerfProtocol,
> > > > +                               DomainId,
> > > > +                               &DomainAttributes
> > > > +                               );
> > > > +  if (EFI_ERROR (Status)) {
> > > > +    return Status;
> > > > +  }
> > > > +
> > > > +  Status = ArmScmiInfoDescribeLevels (DomainId, &LevelArray, &LevelCount);
> > > > +  if (EFI_ERROR (Status)) {
> > > > +    return Status;
> > > > +  }
> > > > +
> > > > +  /* Do some safety checks.
> > > > +     Only support FastChannels (and not doorbells) as this is
> > > > +     the only mechanism supported by SCP.
> > > > +     FcLimits[Get|Set] require 2 UINT32 values (max, then min) and
> > > > +     FcLimits[Get|Set] require 1 UINT32 value (level).
> > > > +  */
> > > > +  if ((FcLevelGet.ChanSize != sizeof (UINT32))  ||
> > > > +      ((FcLevelGet.Attributes & SCMI_PERF_FC_ATTRIB_HAS_DOORBELL) ==
> > > > +       SCMI_PERF_FC_ATTRIB_HAS_DOORBELL) ||
> > > > +      (FcLimitsSet.ChanSize != 2 * sizeof (UINT32)) ||
> > > > +      ((FcLimitsSet.Attributes & SCMI_PERF_FC_ATTRIB_HAS_DOORBELL) ==
> > > > +       SCMI_PERF_FC_ATTRIB_HAS_DOORBELL))
> > > > +  {
> > > > +    Status = EFI_INVALID_PARAMETER;
> > > > +    goto exit_handler;
> > > > +  }
> > > > +
> > > > +  FcLevelGetAddr = ((UINT64)FcLevelGet.ChanAddrHigh << 32) |
> > > > +                   FcLevelGet.ChanAddrLow;
> > > > +  FcLimitsMaxSetAddr = ((UINT64)FcLimitsSet.ChanAddrHigh << 32) |
> > > > +                       FcLimitsSet.ChanAddrLow;
> > > > +  FcLimitsMinSetAddr = FcLimitsMaxSetAddr + 0x4;
> > > > +
> > > > +  CpcInfo->Revision                          = EFI_ACPI_6_4_AML_CPC_REVISION_V3;
> > > > +  CpcInfo->HighestPerformanceInteger         = LevelArray[LevelCount - 1].Level;
> > > > +  CpcInfo->NominalPerformanceInteger         = DomainAttributes.SustainedPerfLevel;
> > > > +  CpcInfo->LowestNonlinearPerformanceInteger = LevelArray[0].Level;
> > > > +  CpcInfo->LowestPerformanceInteger          = LevelArray[0].Level;
> > > > +
> > > > +  CpcInfo->DesiredPerformanceRegister.AddressSpaceId    = EFI_ACPI_6_4_SYSTEM_MEMORY;
> > > > +  CpcInfo->DesiredPerformanceRegister.RegisterBitWidth  = 32;
> > > > +  CpcInfo->DesiredPerformanceRegister.RegisterBitOffset = 0;
> > > > +  CpcInfo->DesiredPerformanceRegister.AccessSize        = EFI_ACPI_6_4_DWORD;
> > > > +  CpcInfo->DesiredPerformanceRegister.Address           = FcLevelGetAddr;
> > > > +
> > > > +  CpcInfo->MinimumPerformanceRegister.AddressSpaceId    = EFI_ACPI_6_4_SYSTEM_MEMORY;
> > > > +  CpcInfo->MinimumPerformanceRegister.RegisterBitWidth  = 32;
> > > > +  CpcInfo->MinimumPerformanceRegister.RegisterBitOffset = 0;
> > > > +  CpcInfo->MinimumPerformanceRegister.AccessSize        = EFI_ACPI_6_4_DWORD;
> > > > +  CpcInfo->MinimumPerformanceRegister.Address           = FcLimitsMinSetAddr;
> > > > +
> > > > +  CpcInfo->MaximumPerformanceRegister.AddressSpaceId    = EFI_ACPI_6_4_SYSTEM_MEMORY;
> > > > +  CpcInfo->MaximumPerformanceRegister.RegisterBitWidth  = 32;
> > > > +  CpcInfo->MaximumPerformanceRegister.RegisterBitOffset = 0;
> > > > +  CpcInfo->MaximumPerformanceRegister.AccessSize        = EFI_ACPI_6_4_DWORD;
> > > > +  CpcInfo->MaximumPerformanceRegister.Address           = FcLimitsMaxSetAddr;
> > > > +
> > > > +  CpcInfo->ReferencePerformanceCounterRegister.AddressSpaceId    = EFI_ACPI_6_4_FUNCTIONAL_FIXED_HARDWARE;
> > > > +  CpcInfo->ReferencePerformanceCounterRegister.RegisterBitWidth  = 0x40;
> > > > +  CpcInfo->ReferencePerformanceCounterRegister.RegisterBitOffset = 0;
> > > > +  CpcInfo->ReferencePerformanceCounterRegister.AccessSize        = ARM_FFH_REFERENCE_PERF_COUNTER_REGISTER;
> > > > +  CpcInfo->ReferencePerformanceCounterRegister.Address           = 0x4;
> > > > +
> > > > +  CpcInfo->DeliveredPerformanceCounterRegister.AddressSpaceId    = EFI_ACPI_6_4_FUNCTIONAL_FIXED_HARDWARE;
> > > > +  CpcInfo->DeliveredPerformanceCounterRegister.RegisterBitWidth  = 0x40;
> > > > +  CpcInfo->DeliveredPerformanceCounterRegister.RegisterBitOffset = 0;
> > > > +  CpcInfo->DeliveredPerformanceCounterRegister.AccessSize        = ARM_FFH_DELIVERED_PERF_COUNTER_REGISTER;
> > > > +  CpcInfo->DeliveredPerformanceCounterRegister.Address           = 0x4;
> > > > +
> > > > +  // SCMI should advertise performance values on a unified scale. So frequency
> > > > +  // values are not available. LowestFrequencyInteger and
> > > > +  // NominalFrequencyInteger are populated in the ConfigurationManager.
> > > > +
> > > > +exit_handler:
> > > > +  FreePool (LevelArray);
> > > > +  return Status;
> > > > +}
> > > > diff --git a/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
> > > > new file mode 100644
> > > > index 000000000000..aad3f0fa7b83
> > > > --- /dev/null
> > > > +++ b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
> > > > @@ -0,0 +1,31 @@
> > > > +## @file
> > > > +#  Arm SCMI Info Library.
> > > > +#
> > > > +#  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.
> > > > +#
> > > > +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > +##
> > > > +
> > > > +[Defines]
> > > > +  INF_VERSION    = 0x0001001B
> > > > +  BASE_NAME      = ArmScmiInfoLib
> > > > +  FILE_GUID      = 1A7CDB04-9FFC-40DA-A87C-A5ACADAF8136
> > > > +  VERSION_STRING = 1.0
> > > > +  MODULE_TYPE    = DXE_DRIVER
> > > > +  LIBRARY_CLASS  = ArmScmiInfoLib
> > > > +  CONSTRUCTOR    = ArmScmiInfoLibConstructor
> > > > +
> > > > +[Sources]
> > > > +  ArmScmiInfoLib.c
> > > > +
> > > > +[Packages]
> > > > +  ArmPkg/ArmPkg.dec
> > > > +  DynamicTablesPkg/DynamicTablesPkg.dec
> > > > +  EmbeddedPkg/EmbeddedPkg.dec
> > > > +  MdePkg/MdePkg.dec
> > > > +
> > > > +[Protocols]
> > > > +  gArmScmiPerformanceProtocolGuid   ## CONSUMES
> > > > +
> > > > +[Depex]
> > > > +  gArmScmiPerformanceProtocolGuid
> > > > -- 
> > > > 2.25.1
> > > > 
> > > > 
> > > > 
> > > > 
> > > > 
> > > > 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110966): https://edk2.groups.io/g/devel/message/110966
Mute This Topic: https://groups.io/mt/102175821/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 10/11] DynamicTablesPkg: Add ArmScmiInfoLib

[PierreGondois]

On 11/9/23 12:26, Leif Lindholm wrote:
> On Thu, Nov 09, 2023 at 10:58:58 +0100, Pierre Gondois wrote:
>> Hello Leif,
>>
>> On 11/2/23 11:20, Pierre Gondois wrote:
>>>
>>>
>>> On 10/26/23 13:03, Leif Lindholm wrote:
>>>> On Wed, Oct 25, 2023 at 13:25:39 +0200, PierreGondois wrote:
>>>>> From: Pierre Gondois <pierre.gondois@arm.com>
>>>>>
>>>>> The SCP holds some power information that could be advertised
>>>>> through the _CPC object. The communication with the SCP is done
>>>>> through SCMI protocols (c.f. ArmScmiDxe).
>>>>>
>>>>> Use the SCMI protocols to query information and feed it to
>>>>> the DynamicTablesPkg.
>>>>
>>>> Couple of questions:
>>>> With a generic name like ArmScmiInfoLib, does is belong in ArmPkg or
>>>> MdeModulePkg?
>>>>
>>>> Or if it's more tightly integrated with DynamicTablesPkg (not
>>>> blatantly obvious from a quick skim below), should that be reflected
>>>> by the library name?
>>>
>>> The library is tight to the DynamicTablesPkg as it produces DynamicTablesPkg
>>> specific objects. It uses the SCMI interface to fetch information from the SCP.
>>> The ScmiProtocol resides in the ArmPkg, thus the name.
>>> Does the name 'ScmiInfoLib' sound better ?
>>
>> Should I keep ArmScmiInfoLib / ScmiInfoLib ?
> 
> I know it gets tedious with the long names, but if it's fully
> integrated with DynamicTablesPkg, then it's kind of important that the
> name does not imply it's generic; i.e., the name should be DynamicTable*.
> 
> If it helps clarify my thinking, my litmus test for this is I
> don't want to have to go searching through code to determine whether
> 
> #include <Library/ArmScmi*.h>
> 
> imports an Arm-specification header or a package-local helper.

Ok right, I'll rename it DynamicTablesScmiInfoLib then.

> 
>> Also I'm not sure if there is a change required to:
>>    [PATCH v2 01/11] ArmPkg/ArmScmiDxe: Rename PERFORMANCE_PROTOCOL_VERSION
>> regarding the spec. reference, cf. https://edk2.groups.io/g/devel/message/110515
> 
> I'm not sure I follow? A change in what way?

I was referring to your point about the fact some features might
be written against a specific SCMI spec. version, cf. the related thread.

> 
> I agree the name is too generic. I didn't flag it because it replaces
> a non-ideal thing with something that isn't any worse, and I'm trying
> to cut down on asking people to fix existing quirks when adding new
> features.
> 
> If you're happy to rename it while you're at it, I'm happy to take it.
> 
> Regards,
> 
> Leif
> 
>> Regards
>> Pierre
>>
>>>
>>>>
>>>> /
>>>>        Leif
>>>>
>>>>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>>>>> ---
>>>>>     DynamicTablesPkg/DynamicTables.dsc.inc        |   1 +
>>>>>     DynamicTablesPkg/DynamicTablesPkg.dec         |   3 +
>>>>>     DynamicTablesPkg/DynamicTablesPkg.dsc         |   1 +
>>>>>     .../Include/Library/ArmScmiInfoLib.h          |  33 ++
>>>>>     .../Library/ArmScmiInfoLib/ArmScmiInfoLib.c   | 294 ++++++++++++++++++
>>>>>     .../Library/ArmScmiInfoLib/ArmScmiInfoLib.inf |  31 ++
>>>>>     6 files changed, 363 insertions(+)
>>>>>     create mode 100644 DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h
>>>>>     create mode 100644 DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c
>>>>>     create mode 100644 DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
>>>>>
>>>>> diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc b/DynamicTablesPkg/DynamicTables.dsc.inc
>>>>> index 9d4312c4e87d..be40ebc4b472 100644
>>>>> --- a/DynamicTablesPkg/DynamicTables.dsc.inc
>>>>> +++ b/DynamicTablesPkg/DynamicTables.dsc.inc
>>>>> @@ -15,6 +15,7 @@ [BuildOptions]
>>>>>     [LibraryClasses.common]
>>>>>       AcpiHelperLib|DynamicTablesPkg/Library/Common/AcpiHelperLib/AcpiHelperLib.inf
>>>>>       AmlLib|DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
>>>>> +  ArmScmiInfoLib|DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
>>>>>       SsdtPcieSupportLib|DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
>>>>>       SsdtSerialPortFixupLib|DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.inf
>>>>>       TableHelperLib|DynamicTablesPkg/Library/Common/TableHelperLib/TableHelperLib.inf
>>>>> diff --git a/DynamicTablesPkg/DynamicTablesPkg.dec b/DynamicTablesPkg/DynamicTablesPkg.dec
>>>>> index cfbcbb9569f1..26498e5fec53 100644
>>>>> --- a/DynamicTablesPkg/DynamicTablesPkg.dec
>>>>> +++ b/DynamicTablesPkg/DynamicTablesPkg.dec
>>>>> @@ -42,6 +42,9 @@ [LibraryClasses]
>>>>>       ##  @libraryclass  Defines a set of SMBIOS string helper methods.
>>>>>       SmbiosStringTableLib|Include/Library/SmbiosStringTableLib.h
>>>>> +  ##  @libraryclass  Defines a set of APIs to populate CmObj using SCMI.
>>>>> +  ArmScmiInfoLib|Include/Library/ArmScmiInfoLib.h
>>>>> +
>>>>>     [Protocols]
>>>>>       # Configuration Manager Protocol GUID
>>>>>       gEdkiiConfigurationManagerProtocolGuid = { 0xd85a4835, 0x5a82, 0x4894, { 0xac, 0x2, 0x70, 0x6f, 0x43, 0xd5, 0x97, 0x8e } }
>>>>> diff --git a/DynamicTablesPkg/DynamicTablesPkg.dsc b/DynamicTablesPkg/DynamicTablesPkg.dsc
>>>>> index bd5084a9008f..6ea86c9efdb0 100644
>>>>> --- a/DynamicTablesPkg/DynamicTablesPkg.dsc
>>>>> +++ b/DynamicTablesPkg/DynamicTablesPkg.dsc
>>>>> @@ -39,6 +39,7 @@ [LibraryClasses.ARM, LibraryClasses.AARCH64]
>>>>>       PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
>>>>>     [Components.common]
>>>>> +  DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
>>>>>       DynamicTablesPkg/Library/Common/AcpiHelperLib/AcpiHelperLib.inf
>>>>>       DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
>>>>>       DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
>>>>> diff --git a/DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h b/DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h
>>>>> new file mode 100644
>>>>> index 000000000000..8d3fb31df13c
>>>>> --- /dev/null
>>>>> +++ b/DynamicTablesPkg/Include/Library/ArmScmiInfoLib.h
>>>>> @@ -0,0 +1,33 @@
>>>>> +/** @file
>>>>> +  Arm SCMI Info Library.
>>>>> +
>>>>> +  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.<BR>
>>>>> +
>>>>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>>>>> +**/
>>>>> +
>>>>> +#ifndef ARM_SCMI_INFO_LIB_H_
>>>>> +#define ARM_SCMI_INFO_LIB_H_
>>>>> +
>>>>> +#include <ConfigurationManagerObject.h>
>>>>> +
>>>>> +/** Populate a AML_CPC_INFO object based on SCMI information.
>>>>> +
>>>>> +  @param[in]  DomainId    Identifier for the performance domain.
>>>>> +  @param[out] CpcInfo     If success, this structure was populated from
>>>>> +                          information queried to the SCP.
>>>>> +
>>>>> +  @retval EFI_SUCCESS             Success.
>>>>> +  @retval EFI_DEVICE_ERROR        Device error.
>>>>> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>>>>> +  @retval EFI_TIMEOUT             Time out.
>>>>> +  @retval EFI_UNSUPPORTED         Unsupported.
>>>>> +**/
>>>>> +EFI_STATUS
>>>>> +EFIAPI
>>>>> +ArmScmiInfoGetFastChannel (
>>>>> +  IN  UINT32        DomainId,
>>>>> +  OUT AML_CPC_INFO  *CpcInfo
>>>>> +  );
>>>>> +
>>>>> +#endif // ARM_SCMI_INFO_LIB_H_
>>>>> diff --git a/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c
>>>>> new file mode 100644
>>>>> index 000000000000..c23bff63bb6f
>>>>> --- /dev/null
>>>>> +++ b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c
>>>>> @@ -0,0 +1,294 @@
>>>>> +/** @file
>>>>> +  Arm SCMI Info Library.
>>>>> +
>>>>> +  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.<BR>
>>>>> +
>>>>> +  Arm Functional Fixed Hardware Specification:
>>>>> +  - https://developer.arm.com/documentation/den0048/latest/
>>>>> +
>>>>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>>>>> +**/
>>>>> +
>>>>> +#include <Library/AcpiLib.h>
>>>>> +#include <Library/ArmScmiInfoLib.h>
>>>>> +#include <Library/DebugLib.h>
>>>>> +#include <Library/MemoryAllocationLib.h>
>>>>> +#include <Library/UefiBootServicesTableLib.h>
>>>>> +#include <Protocol/ArmScmi.h>
>>>>> +#include <Protocol/ArmScmiPerformanceProtocol.h>
>>>>> +
>>>>> +/** Arm FFH registers
>>>>> +
>>>>> +  Cf. Arm Functional Fixed Hardware Specification
>>>>> +  s3.2 Performance management and Collaborative Processor Performance Control
>>>>> +*/
>>>>> +#define ARM_FFH_DELIVERED_PERF_COUNTER_REGISTER  0x0
>>>>> +#define ARM_FFH_REFERENCE_PERF_COUNTER_REGISTER  0x1
>>>>> +
>>>>> +/// Arm SCMI performance protocol.
>>>>> +STATIC SCMI_PERFORMANCE_PROTOCOL  *ScmiPerfProtocol;
>>>>> +
>>>>> +/** Arm SCMI Info Library constructor.
>>>>> +
>>>>> +  @param  ImageHandle   Image of the loaded driver.
>>>>> +  @param  SystemTable   Pointer to the System Table.
>>>>> +
>>>>> +  @retval EFI_SUCCESS             Success.
>>>>> +  @retval EFI_DEVICE_ERROR        Device error.
>>>>> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>>>>> +  @retval EFI_NOT_FOUND           Not Found
>>>>> +  @retval EFI_TIMEOUT             Timeout.
>>>>> +  @retval EFI_UNSUPPORTED         Unsupported.
>>>>> +**/
>>>>> +EFI_STATUS
>>>>> +EFIAPI
>>>>> +ArmScmiInfoLibConstructor (
>>>>> +  IN  EFI_HANDLE        ImageHandle,
>>>>> +  IN  EFI_SYSTEM_TABLE  *SystemTable
>>>>> +  )
>>>>> +{
>>>>> +  EFI_STATUS  Status;
>>>>> +  UINT32      Version;
>>>>> +
>>>>> +  Status = gBS->LocateProtocol (
>>>>> +                  &gArmScmiPerformanceProtocolGuid,
>>>>> +                  NULL,
>>>>> +                  (VOID **)&ScmiPerfProtocol
>>>>> +                  );
>>>>> +  if (EFI_ERROR (Status)) {
>>>>> +    return Status;
>>>>> +  }
>>>>> +
>>>>> +  Status = ScmiPerfProtocol->GetVersion (ScmiPerfProtocol, &Version);
>>>>> +  if (EFI_ERROR (Status)) {
>>>>> +    return Status;
>>>>> +  }
>>>>> +
>>>>> +  // FastChannels were added in SCMI v2.0 spec.
>>>>> +  if (Version < PERFORMANCE_PROTOCOL_VERSION_V2) {
>>>>> +    DEBUG ((DEBUG_ERROR, "ArmScmiInfoLib requires SCMI version > 2.0\n"));
>>>>> +    return EFI_UNSUPPORTED;
>>>>> +  }
>>>>> +
>>>>> +  return Status;
>>>>> +}
>>>>> +
>>>>> +/** Get the OPPs/performance states of a power domain.
>>>>> +
>>>>> +  This function is a wrapper around the SCMI PERFORMANCE_DESCRIBE_LEVELS
>>>>> +  command. The list of discrete performance states is returned in a buffer
>>>>> +  that must be freed by the caller.
>>>>> +
>>>>> +  @param[in]  DomainId        Identifier for the performance domain.
>>>>> +  @param[out] LevelArray      If success, pointer to the list of list of
>>>>> +                              performance state. This memory must be freed by
>>>>> +                              the caller.
>>>>> +  @param[out] LevelArrayCount If success, contains the number of states in
>>>>> +                              LevelArray.
>>>>> +
>>>>> +  @retval EFI_SUCCESS             Success.
>>>>> +  @retval EFI_DEVICE_ERROR        Device error.
>>>>> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>>>>> +  @retval EFI_TIMEOUT             Time out.
>>>>> +  @retval EFI_UNSUPPORTED         Unsupported.
>>>>> +**/
>>>>> +STATIC
>>>>> +EFI_STATUS
>>>>> +EFIAPI
>>>>> +ArmScmiInfoDescribeLevels (
>>>>> +  IN  UINT32                  DomainId,
>>>>> +  OUT SCMI_PERFORMANCE_LEVEL  **LevelArray,
>>>>> +  OUT UINT32                  *LevelArrayCount
>>>>> +  )
>>>>> +{
>>>>> +  EFI_STATUS              Status;
>>>>> +  SCMI_PERFORMANCE_LEVEL  *Array;
>>>>> +  UINT32                  Count;
>>>>> +  UINT32                  Size;
>>>>> +
>>>>> +  if ((ScmiPerfProtocol == NULL)  ||
>>>>> +      (LevelArray == NULL)  ||
>>>>> +      (LevelArrayCount == NULL))
>>>>> +  {
>>>>> +    return EFI_INVALID_PARAMETER;
>>>>> +  }
>>>>> +
>>>>> +  // First call to get the number of levels.
>>>>> +  Size   = 0;
>>>>> +  Status = ScmiPerfProtocol->DescribeLevels (
>>>>> +                               ScmiPerfProtocol,
>>>>> +                               DomainId,
>>>>> +                               &Count,
>>>>> +                               &Size,
>>>>> +                               NULL
>>>>> +                               );
>>>>> +  if (Status != EFI_BUFFER_TOO_SMALL) {
>>>>> +    // EFI_SUCCESS is not a valid option.
>>>>> +    if (Status == EFI_SUCCESS) {
>>>>> +      return EFI_INVALID_PARAMETER;
>>>>> +    } else {
>>>>> +      return Status;
>>>>> +    }
>>>>> +  }
>>>>> +
>>>>> +  Array = AllocateZeroPool (Size);
>>>>> +  if (Array == NULL) {
>>>>> +    return EFI_OUT_OF_RESOURCES;
>>>>> +  }
>>>>> +
>>>>> +  // Second call to get the descriptions of the levels.
>>>>> +  Status = ScmiPerfProtocol->DescribeLevels (
>>>>> +                               ScmiPerfProtocol,
>>>>> +                               DomainId,
>>>>> +                               &Count,
>>>>> +                               &Size,
>>>>> +                               Array
>>>>> +                               );
>>>>> +  if (EFI_ERROR (Status)) {
>>>>> +    return Status;
>>>>> +  }
>>>>> +
>>>>> +  *LevelArray      = Array;
>>>>> +  *LevelArrayCount = Count;
>>>>> +
>>>>> +  return Status;
>>>>> +}
>>>>> +
>>>>> +/** Populate a AML_CPC_INFO object based on SCMI information.
>>>>> +
>>>>> +  @param[in]  DomainId    Identifier for the performance domain.
>>>>> +  @param[out] CpcInfo     If success, this structure was populated from
>>>>> +                          information queried to the SCP.
>>>>> +
>>>>> +  @retval EFI_SUCCESS             Success.
>>>>> +  @retval EFI_DEVICE_ERROR        Device error.
>>>>> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>>>>> +  @retval EFI_TIMEOUT             Time out.
>>>>> +  @retval EFI_UNSUPPORTED         Unsupported.
>>>>> +**/
>>>>> +EFI_STATUS
>>>>> +EFIAPI
>>>>> +ArmScmiInfoGetFastChannel (
>>>>> +  IN  UINT32        DomainId,
>>>>> +  OUT AML_CPC_INFO  *CpcInfo
>>>>> +  )
>>>>> +{
>>>>> +  EFI_STATUS                          Status;
>>>>> +  SCMI_PERFORMANCE_FASTCHANNEL        FcLevelGet;
>>>>> +  SCMI_PERFORMANCE_FASTCHANNEL        FcLimitsSet;
>>>>> +  SCMI_PERFORMANCE_DOMAIN_ATTRIBUTES  DomainAttributes;
>>>>> +
>>>>> +  SCMI_PERFORMANCE_LEVEL  *LevelArray;
>>>>> +  UINT32                  LevelCount;
>>>>> +
>>>>> +  UINT64  FcLevelGetAddr;
>>>>> +  UINT64  FcLimitsMaxSetAddr;
>>>>> +  UINT64  FcLimitsMinSetAddr;
>>>>> +
>>>>> +  if ((ScmiPerfProtocol == NULL)  ||
>>>>> +      (CpcInfo == NULL))
>>>>> +  {
>>>>> +    return EFI_INVALID_PARAMETER;
>>>>> +  }
>>>>> +
>>>>> +  Status = ScmiPerfProtocol->DescribeFastchannel (
>>>>> +                               ScmiPerfProtocol,
>>>>> +                               DomainId,
>>>>> +                               ScmiMessageIdPerformanceLevelSet,
>>>>> +                               &FcLevelGet
>>>>> +                               );
>>>>> +  if (EFI_ERROR (Status)) {
>>>>> +    return Status;
>>>>> +  }
>>>>> +
>>>>> +  Status = ScmiPerfProtocol->DescribeFastchannel (
>>>>> +                               ScmiPerfProtocol,
>>>>> +                               DomainId,
>>>>> +                               ScmiMessageIdPerformanceLimitsSet,
>>>>> +                               &FcLimitsSet
>>>>> +                               );
>>>>> +  if (EFI_ERROR (Status)) {
>>>>> +    return Status;
>>>>> +  }
>>>>> +
>>>>> +  Status = ScmiPerfProtocol->GetDomainAttributes (
>>>>> +                               ScmiPerfProtocol,
>>>>> +                               DomainId,
>>>>> +                               &DomainAttributes
>>>>> +                               );
>>>>> +  if (EFI_ERROR (Status)) {
>>>>> +    return Status;
>>>>> +  }
>>>>> +
>>>>> +  Status = ArmScmiInfoDescribeLevels (DomainId, &LevelArray, &LevelCount);
>>>>> +  if (EFI_ERROR (Status)) {
>>>>> +    return Status;
>>>>> +  }
>>>>> +
>>>>> +  /* Do some safety checks.
>>>>> +     Only support FastChannels (and not doorbells) as this is
>>>>> +     the only mechanism supported by SCP.
>>>>> +     FcLimits[Get|Set] require 2 UINT32 values (max, then min) and
>>>>> +     FcLimits[Get|Set] require 1 UINT32 value (level).
>>>>> +  */
>>>>> +  if ((FcLevelGet.ChanSize != sizeof (UINT32))  ||
>>>>> +      ((FcLevelGet.Attributes & SCMI_PERF_FC_ATTRIB_HAS_DOORBELL) ==
>>>>> +       SCMI_PERF_FC_ATTRIB_HAS_DOORBELL) ||
>>>>> +      (FcLimitsSet.ChanSize != 2 * sizeof (UINT32)) ||
>>>>> +      ((FcLimitsSet.Attributes & SCMI_PERF_FC_ATTRIB_HAS_DOORBELL) ==
>>>>> +       SCMI_PERF_FC_ATTRIB_HAS_DOORBELL))
>>>>> +  {
>>>>> +    Status = EFI_INVALID_PARAMETER;
>>>>> +    goto exit_handler;
>>>>> +  }
>>>>> +
>>>>> +  FcLevelGetAddr = ((UINT64)FcLevelGet.ChanAddrHigh << 32) |
>>>>> +                   FcLevelGet.ChanAddrLow;
>>>>> +  FcLimitsMaxSetAddr = ((UINT64)FcLimitsSet.ChanAddrHigh << 32) |
>>>>> +                       FcLimitsSet.ChanAddrLow;
>>>>> +  FcLimitsMinSetAddr = FcLimitsMaxSetAddr + 0x4;
>>>>> +
>>>>> +  CpcInfo->Revision                          = EFI_ACPI_6_4_AML_CPC_REVISION_V3;
>>>>> +  CpcInfo->HighestPerformanceInteger         = LevelArray[LevelCount - 1].Level;
>>>>> +  CpcInfo->NominalPerformanceInteger         = DomainAttributes.SustainedPerfLevel;
>>>>> +  CpcInfo->LowestNonlinearPerformanceInteger = LevelArray[0].Level;
>>>>> +  CpcInfo->LowestPerformanceInteger          = LevelArray[0].Level;
>>>>> +
>>>>> +  CpcInfo->DesiredPerformanceRegister.AddressSpaceId    = EFI_ACPI_6_4_SYSTEM_MEMORY;
>>>>> +  CpcInfo->DesiredPerformanceRegister.RegisterBitWidth  = 32;
>>>>> +  CpcInfo->DesiredPerformanceRegister.RegisterBitOffset = 0;
>>>>> +  CpcInfo->DesiredPerformanceRegister.AccessSize        = EFI_ACPI_6_4_DWORD;
>>>>> +  CpcInfo->DesiredPerformanceRegister.Address           = FcLevelGetAddr;
>>>>> +
>>>>> +  CpcInfo->MinimumPerformanceRegister.AddressSpaceId    = EFI_ACPI_6_4_SYSTEM_MEMORY;
>>>>> +  CpcInfo->MinimumPerformanceRegister.RegisterBitWidth  = 32;
>>>>> +  CpcInfo->MinimumPerformanceRegister.RegisterBitOffset = 0;
>>>>> +  CpcInfo->MinimumPerformanceRegister.AccessSize        = EFI_ACPI_6_4_DWORD;
>>>>> +  CpcInfo->MinimumPerformanceRegister.Address           = FcLimitsMinSetAddr;
>>>>> +
>>>>> +  CpcInfo->MaximumPerformanceRegister.AddressSpaceId    = EFI_ACPI_6_4_SYSTEM_MEMORY;
>>>>> +  CpcInfo->MaximumPerformanceRegister.RegisterBitWidth  = 32;
>>>>> +  CpcInfo->MaximumPerformanceRegister.RegisterBitOffset = 0;
>>>>> +  CpcInfo->MaximumPerformanceRegister.AccessSize        = EFI_ACPI_6_4_DWORD;
>>>>> +  CpcInfo->MaximumPerformanceRegister.Address           = FcLimitsMaxSetAddr;
>>>>> +
>>>>> +  CpcInfo->ReferencePerformanceCounterRegister.AddressSpaceId    = EFI_ACPI_6_4_FUNCTIONAL_FIXED_HARDWARE;
>>>>> +  CpcInfo->ReferencePerformanceCounterRegister.RegisterBitWidth  = 0x40;
>>>>> +  CpcInfo->ReferencePerformanceCounterRegister.RegisterBitOffset = 0;
>>>>> +  CpcInfo->ReferencePerformanceCounterRegister.AccessSize        = ARM_FFH_REFERENCE_PERF_COUNTER_REGISTER;
>>>>> +  CpcInfo->ReferencePerformanceCounterRegister.Address           = 0x4;
>>>>> +
>>>>> +  CpcInfo->DeliveredPerformanceCounterRegister.AddressSpaceId    = EFI_ACPI_6_4_FUNCTIONAL_FIXED_HARDWARE;
>>>>> +  CpcInfo->DeliveredPerformanceCounterRegister.RegisterBitWidth  = 0x40;
>>>>> +  CpcInfo->DeliveredPerformanceCounterRegister.RegisterBitOffset = 0;
>>>>> +  CpcInfo->DeliveredPerformanceCounterRegister.AccessSize        = ARM_FFH_DELIVERED_PERF_COUNTER_REGISTER;
>>>>> +  CpcInfo->DeliveredPerformanceCounterRegister.Address           = 0x4;
>>>>> +
>>>>> +  // SCMI should advertise performance values on a unified scale. So frequency
>>>>> +  // values are not available. LowestFrequencyInteger and
>>>>> +  // NominalFrequencyInteger are populated in the ConfigurationManager.
>>>>> +
>>>>> +exit_handler:
>>>>> +  FreePool (LevelArray);
>>>>> +  return Status;
>>>>> +}
>>>>> diff --git a/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
>>>>> new file mode 100644
>>>>> index 000000000000..aad3f0fa7b83
>>>>> --- /dev/null
>>>>> +++ b/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.inf
>>>>> @@ -0,0 +1,31 @@
>>>>> +## @file
>>>>> +#  Arm SCMI Info Library.
>>>>> +#
>>>>> +#  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.
>>>>> +#
>>>>> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
>>>>> +##
>>>>> +
>>>>> +[Defines]
>>>>> +  INF_VERSION    = 0x0001001B
>>>>> +  BASE_NAME      = ArmScmiInfoLib
>>>>> +  FILE_GUID      = 1A7CDB04-9FFC-40DA-A87C-A5ACADAF8136
>>>>> +  VERSION_STRING = 1.0
>>>>> +  MODULE_TYPE    = DXE_DRIVER
>>>>> +  LIBRARY_CLASS  = ArmScmiInfoLib
>>>>> +  CONSTRUCTOR    = ArmScmiInfoLibConstructor
>>>>> +
>>>>> +[Sources]
>>>>> +  ArmScmiInfoLib.c
>>>>> +
>>>>> +[Packages]
>>>>> +  ArmPkg/ArmPkg.dec
>>>>> +  DynamicTablesPkg/DynamicTablesPkg.dec
>>>>> +  EmbeddedPkg/EmbeddedPkg.dec
>>>>> +  MdePkg/MdePkg.dec
>>>>> +
>>>>> +[Protocols]
>>>>> +  gArmScmiPerformanceProtocolGuid   ## CONSUMES
>>>>> +
>>>>> +[Depex]
>>>>> +  gArmScmiPerformanceProtocolGuid
>>>>> -- 
>>>>> 2.25.1
>>>>>
>>>>>
>>>>>
>>>>> 
>>>>>
>>>>>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111024): https://edk2.groups.io/g/devel/message/111024
Mute This Topic: https://groups.io/mt/102175821/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 01/11] ArmPkg/ArmScmiDxe: Rename PERFORMANCE_PROTOCOL_VERSION

[PierreGondois]

Hello Leif,

On 11/2/23 11:20, Pierre Gondois wrote:
> Hello Leif,
> Thanks for the review,
> 
> On 10/26/23 12:05, Leif Lindholm wrote:
>> On Wed, Oct 25, 2023 at 13:25:30 +0200, pierre.gondois@arm.com wrote:
>>> From: Pierre Gondois <pierre.gondois@arm.com>
>>>
>>> Rename PERFORMANCE_PROTOCOL_VERSION to reflect the different
>>> versions of the protocol. The macro is neither used in edk2 nor
>>> in edk2-platforms.
>>
>> OK, so slight nitpick, but mainly because it parses a bit weirdly...
>> *Will* it be used after this series is merged, or is this an update
>> for completeness?
> 
> The 'fast channels' were added in the v2.0 SCMI specification. This patch-set
> relies on this feature, so it is checked in:
>      [PATCH v2 10/11] DynamicTablesPkg: Add ArmScmiInfoLib
> that the underlying SCP is at least at this version.
> 
> ```
>      // FastChannels were added in SCMI v2.0 spec.
>      if (Version < PERFORMANCE_PROTOCOL_VERSION_V2) {
>        DEBUG ((DEBUG_ERROR, "ArmScmiInfoLib requires SCMI version > 2.0\n"));
>        return EFI_UNSUPPORTED;
>      }
> ```
> 
>>
>>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>>> ---
>>>    ArmPkg/Include/Library/ArmLib.h                     |  1 +
>>>    .../Include/Protocol/ArmScmiPerformanceProtocol.h   | 13 ++++++++-----
>>>    2 files changed, 9 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
>>> index 0169dbc1092c..7b2b2238fed9 100644
>>> --- a/ArmPkg/Include/Library/ArmLib.h
>>> +++ b/ArmPkg/Include/Library/ArmLib.h
>>> @@ -780,6 +780,7 @@ EFIAPI
>>>    ArmHasVhe (
>>>      VOID
>>>      );
>>> +
>>>    #endif // MDE_CPU_AARCH64
>>>    
>>>    #ifdef MDE_CPU_ARM
>>> diff --git a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
>>> index 7e548e4765c2..8e8e05d5a5f6 100644
>>> --- a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
>>> +++ b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
>>> @@ -1,12 +1,12 @@
>>>    /** @file
>>>    
>>> -  Copyright (c) 2017-2021, Arm Limited. All rights reserved.
>>> +  Copyright (c) 2017-2023, Arm Limited. All rights reserved.
>>>    
>>>      SPDX-License-Identifier: BSD-2-Clause-Patent
>>>    
>>> -  System Control and Management Interface V1.0
>>> -    http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/
>>> -    DEN0056A_System_Control_and_Management_Interface.pdf
>>> +  System Control and Management Interface, latest version:
>>
>> I see this as a pattern throughout the series.
>> But this statement will at some point become untrue; this
>> implementation is written against a specific version. I think this
>> version shold be reflected in the comment. (And that applies
>> throughout the series.)
>>
>>> +  - https://developer.arm.com/documentation/den0056/latest/
>>
>> But I think the above is the most useful link.

I was referring to this point I'm not sure I understood.

Regards,
Pierre

> 
> I am not sure I understand completely. Do you mean that the SCMI
> structures/interfaces defined in:
>      ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
> and that were written against the SCMI v1.0 specification should
> not be used as such for other SCMI specification version ?
> I.e. the same process as for the AcpiXX.h files
> (MdePkg/Include/IndustryStandard/Acpi65.h) should be used ?
> 
> Or do you mean that the _CPC object generation implies that the
> SCP should comply to the v2.0 version at least and this should be
> reflected in the commit messages ?
> 
> Regards,
> Pierre
> 
>>
>> /
>>       Leif
>>
>>> +
>>>    **/
>>>    
>>>    #ifndef ARM_SCMI_PERFORMANCE_PROTOCOL_H_
>>> @@ -14,7 +14,10 @@
>>>    
>>>    #include <Protocol/ArmScmi.h>
>>>    
>>> -#define PERFORMANCE_PROTOCOL_VERSION  0x10000
>>> +/// Arm Scmi performance protocol versions.
>>> +#define PERFORMANCE_PROTOCOL_VERSION_V1  0x10000
>>> +#define PERFORMANCE_PROTOCOL_VERSION_V2  0x20000
>>> +#define PERFORMANCE_PROTOCOL_VERSION_V3  0x30000
>>>    
>>>    #define ARM_SCMI_PERFORMANCE_PROTOCOL_GUID  { \
>>>      0x9b8ba84, 0x3dd3, 0x49a6, {0xa0, 0x5a, 0x31, 0x34, 0xa5, 0xf0, 0x7b, 0xad} \
>>> -- 
>>> 2.25.1
>>>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111025): https://edk2.groups.io/g/devel/message/111025
Mute This Topic: https://groups.io/mt/102175810/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 01/11] ArmPkg/ArmScmiDxe: Rename PERFORMANCE_PROTOCOL_VERSION

[Leif Lindholm]

On 2023-11-10 09:11, Pierre Gondois wrote:
> Hello Leif,
> 
> On 11/2/23 11:20, Pierre Gondois wrote:
>> Hello Leif,
>> Thanks for the review,
>>
>> On 10/26/23 12:05, Leif Lindholm wrote:
>>> On Wed, Oct 25, 2023 at 13:25:30 +0200, pierre.gondois@arm.com wrote:
>>>> From: Pierre Gondois <pierre.gondois@arm.com>
>>>>
>>>> Rename PERFORMANCE_PROTOCOL_VERSION to reflect the different
>>>> versions of the protocol. The macro is neither used in edk2 nor
>>>> in edk2-platforms.
>>>
>>> OK, so slight nitpick, but mainly because it parses a bit weirdly...
>>> *Will* it be used after this series is merged, or is this an update
>>> for completeness?
>>
>> The 'fast channels' were added in the v2.0 SCMI specification. This 
>> patch-set
>> relies on this feature, so it is checked in:
>>      [PATCH v2 10/11] DynamicTablesPkg: Add ArmScmiInfoLib
>> that the underlying SCP is at least at this version.
>>
>> ```
>>      // FastChannels were added in SCMI v2.0 spec.
>>      if (Version < PERFORMANCE_PROTOCOL_VERSION_V2) {
>>        DEBUG ((DEBUG_ERROR, "ArmScmiInfoLib requires SCMI version > 
>> 2.0\n"));
>>        return EFI_UNSUPPORTED;
>>      }
>> ```
>>
>>>
>>>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>>>> ---
>>>>    ArmPkg/Include/Library/ArmLib.h                     |  1 +
>>>>    .../Include/Protocol/ArmScmiPerformanceProtocol.h   | 13 
>>>> ++++++++-----
>>>>    2 files changed, 9 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/ArmPkg/Include/Library/ArmLib.h 
>>>> b/ArmPkg/Include/Library/ArmLib.h
>>>> index 0169dbc1092c..7b2b2238fed9 100644
>>>> --- a/ArmPkg/Include/Library/ArmLib.h
>>>> +++ b/ArmPkg/Include/Library/ArmLib.h
>>>> @@ -780,6 +780,7 @@ EFIAPI
>>>>    ArmHasVhe (
>>>>      VOID
>>>>      );
>>>> +
>>>>    #endif // MDE_CPU_AARCH64
>>>>    #ifdef MDE_CPU_ARM
>>>> diff --git a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h 
>>>> b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
>>>> index 7e548e4765c2..8e8e05d5a5f6 100644
>>>> --- a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
>>>> +++ b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
>>>> @@ -1,12 +1,12 @@
>>>>    /** @file
>>>> -  Copyright (c) 2017-2021, Arm Limited. All rights reserved.
>>>> +  Copyright (c) 2017-2023, Arm Limited. All rights reserved.
>>>>      SPDX-License-Identifier: BSD-2-Clause-Patent
>>>> -  System Control and Management Interface V1.0
>>>> -    http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/
>>>> -    DEN0056A_System_Control_and_Management_Interface.pdf
>>>> +  System Control and Management Interface, latest version:
>>>
>>> I see this as a pattern throughout the series.
>>> But this statement will at some point become untrue; this
>>> implementation is written against a specific version. I think this
>>> version shold be reflected in the comment. (And that applies
>>> throughout the series.)
>>>
>>>> +  - https://developer.arm.com/documentation/den0056/latest/
>>>
>>> But I think the above is the most useful link.
> 
> I was referring to this point I'm not sure I understood.

Oh, right.

I was referring to "latest version" being a moving target.
If I go to https://developer.arm.com/documentation/den0056/latest/, that 
currently means "version 3.2". At some point in the future, that number 
will change, but this code won't automatically get updated.

So I'd prefer something like
"System Control and Management Interface v3.2,
Latest version of the specification can be downloaded from
https://developer.arm.com/documentation/den0056/latest/"

If that makes more sense?

Regards,

Leif

> Regards,
> Pierre
> 
>>
>> I am not sure I understand completely. Do you mean that the SCMI
>> structures/interfaces defined in:
>>      ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
>> and that were written against the SCMI v1.0 specification should
>> not be used as such for other SCMI specification version ?
>> I.e. the same process as for the AcpiXX.h files
>> (MdePkg/Include/IndustryStandard/Acpi65.h) should be used ?
>>
>> Or do you mean that the _CPC object generation implies that the
>> SCP should comply to the v2.0 version at least and this should be
>> reflected in the commit messages ?
>>
>> Regards,
>> Pierre
>>
>>>
>>> /
>>>       Leif
>>>
>>>> +
>>>>    **/
>>>>    #ifndef ARM_SCMI_PERFORMANCE_PROTOCOL_H_
>>>> @@ -14,7 +14,10 @@
>>>>    #include <Protocol/ArmScmi.h>
>>>> -#define PERFORMANCE_PROTOCOL_VERSION  0x10000
>>>> +/// Arm Scmi performance protocol versions.
>>>> +#define PERFORMANCE_PROTOCOL_VERSION_V1  0x10000
>>>> +#define PERFORMANCE_PROTOCOL_VERSION_V2  0x20000
>>>> +#define PERFORMANCE_PROTOCOL_VERSION_V3  0x30000
>>>>    #define ARM_SCMI_PERFORMANCE_PROTOCOL_GUID  { \
>>>>      0x9b8ba84, 0x3dd3, 0x49a6, {0xa0, 0x5a, 0x31, 0x34, 0xa5, 0xf0, 
>>>> 0x7b, 0xad} \
>>>> -- 
>>>> 2.25.1
>>>>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111038): https://edk2.groups.io/g/devel/message/111038
Mute This Topic: https://groups.io/mt/102175810/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-