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

2023-11-10 Thread 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 

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 

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

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

2023-11-09 Thread 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 
> > > > 
> > > > 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 

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 
> > > > ---
> > > >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 1

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

2023-11-09 Thread 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 

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 
---
   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 ..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.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef ARM_SCMI_INFO_LIB_H_
+#define ARM_SCMI_INFO_LIB_H_
+
+#include 
+
+/** Populate a AML_CPC_INFO object based on SCMI information.
+
+  @param[in]  DomainIdIdentifier 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_ERRORDevice error.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_TIMEOUT

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

2023-11-02 Thread 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 

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 
---
  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 ..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.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef ARM_SCMI_INFO_LIB_H_
+#define ARM_SCMI_INFO_LIB_H_
+
+#include 
+
+/** Populate a AML_CPC_INFO object based on SCMI information.
+
+  @param[in]  DomainIdIdentifier 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_ERRORDevice error.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_TIMEOUT Time out.
+  @retval EFI_UNSUPPORTED Unsupported.
+**/
+EFI_STATUS
+EFIAPI
+ArmScmiInfoGetFastChannel (
+  IN  UINT32DomainId,
+  OUT AML_CPC_INFO  *CpcInfo
+  );
+
+#endif // ARM_SCMI_INFO_LIB_H_
diff --git a/DynamicTablesPkg/Library/ArmScmiInfoLib/ArmScmiInfoLib.c 
b/DynamicTablesPkg/Library/ArmScmiInfoLib/Arm

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

2023-10-26 Thread Leif Lindholm 
On Wed, Oct 25, 2023 at 13:25:39 +0200, PierreGondois wrote:
> From: Pierre Gondois 
> 
> 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 
> ---
>  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 ..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.
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#ifndef ARM_SCMI_INFO_LIB_H_
> +#define ARM_SCMI_INFO_LIB_H_
> +
> +#include 
> +
> +/** Populate a AML_CPC_INFO object based on SCMI information.
> +
> +  @param[in]  DomainIdIdentifier 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_ERRORDevice error.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +  @retval EFI_TIMEOUT Time out.
> +  @retval EFI_UNSUPPORTED Unsupported.
> +**/
> +EFI_STATUS
> +EFIAPI
> +ArmScmiInfoGetFastChannel (
> +  IN  UINT32DomainId,
> +  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 ..c23bff63bb6f
> --- /dev/null
> +++ b/DynamicTablesPkg/Library/ArmScmiInfoLib/Ar

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

2023-10-25 Thread PierreGondois 
From: Pierre Gondois 

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 
---
 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 ..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.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef ARM_SCMI_INFO_LIB_H_
+#define ARM_SCMI_INFO_LIB_H_
+
+#include 
+
+/** Populate a AML_CPC_INFO object based on SCMI information.
+
+  @param[in]  DomainIdIdentifier 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_ERRORDevice error.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_TIMEOUT Time out.
+  @retval EFI_UNSUPPORTED Unsupported.
+**/
+EFI_STATUS
+EFIAPI
+ArmScmiInfoGetFastChannel (
+  IN  UINT32DomainId,
+  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 ..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.
+
+  Arm Functional Fixed Hardware Specification:
+  - https://developer.arm.com/documentation/den0048/latest/
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/** Arm FFH registers
+
+  Cf. Arm Functional Fixed Hardware Specification
+  s3.2 Performance management and Collaborative Processor Performance Control
+*/
+#define ARM_FFH