Re: [edk2-devel] [PATCH v6 3/3] Platform/ARM: Juno: Generate _CPC objects for JunoR2

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

Hi Pierre,

Thank you for this patch.

I have some minor suggestions, that I will fix before merging.
With that,

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

Regards,

Sami Mujawar

On 29/01/2024, 12:08, "Pierre.Gondois@arm.com <mailto:Pierre.Gondois@arm.com>" <Pierre.Gondois@arm.com <mailto:Pierre.Gondois@arm.com>> wrote:


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


The SsdtCpuTopologyGenerator can generate _CPC objects.
This is done by querying the SCP for the relevant performance
state information through SCMI. CM_ARM_CPC_INFO are then populated
and used to generate _CPC objects in the Ssdt Cpu topology.


Use the DynamicTablesScmiInfoLib and add the handling to generate
_CPC information.


Note that using _CPC is only possible if SCP is correctly tuned
to advertise performance levels on an abstract and unified scale.
A basic check is done to prevent the _CPC generation otherwise.


Perf level values used for testing:
- little CPUs OPPs: [181, 322, 383] * 1000
- big CPUs OPPs: [512, 833, 1024] * 1000


Also make use of the newly added PcdDevelopmentPlatformRelaxations
and add a new ENABLE_CPC build parameter ('-D ENABLE_CPC') to enable
CPC generation.


Signed-off-by: Pierre Gondois <pierre.gondois@arm.com <mailto:pierre.gondois@arm.com>>
---
Platform/ARM/JunoPkg/ArmJuno.dsc | 11 +
.../ConfigurationManager.c | 252 +++++++++++++++++-
.../ConfigurationManager.h | 7 +
.../ConfigurationManagerDxe.inf | 3 +
4 files changed, 270 insertions(+), 3 deletions(-)


diff --git a/Platform/ARM/JunoPkg/ArmJuno.dsc b/Platform/ARM/JunoPkg/ArmJuno.dsc
index 1ea0aba2655f..7fe796a53433 100644
--- a/Platform/ARM/JunoPkg/ArmJuno.dsc
+++ b/Platform/ARM/JunoPkg/ArmJuno.dsc
@@ -83,6 +83,9 @@ [BuildOptions]
!ifdef DYNAMIC_TABLES_FRAMEWORK
*_*_*_PLATFORM_FLAGS = -DDYNAMIC_TABLES_FRAMEWORK
!endif
+!ifdef ENABLE_CPC
+ *_*_*_PLATFORM_FLAGS = -DENABLE_CPC
+!endif


################################################################################
#
@@ -203,6 +206,14 @@ [PcdsFixedAtBuild.common]
#
gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions|0x20


+!ifdef ENABLE_CPC
+ #
+ # Allow some relaxation on some specific points for the platforms that desire it.
+ # BIT0: Allow the absence of some registers in the _CPC object.
+ #
+ gEdkiiDynamicTablesPkgTokenSpaceGuid.PcdDevelopmentPlatformRelaxations|0x1
+!endif
+
[PcdsPatchableInModule]
# Console Resolution (Full HD)
gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|1920
diff --git a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
index efc4c0ddf87c..636645ed2821 100644
--- a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
+++ b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
@@ -14,6 +14,7 @@
#include <IndustryStandard/MemoryMappedConfigurationSpaceAccessTable.h>
#include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
#include <Library/ArmLib.h>
+#include <Library/DynamicTablesScmiInfoLib.h>
#include <Library/DebugLib.h>
#include <Library/IoLib.h>
#include <Library/PcdLib.h>
@@ -901,6 +902,173 @@ HandleCmObjectSearchPlatformRepo (
return Status;
}


+/** Clear Cpc information.
+
+ If populating _CPC information fails, remove GicC tokens pointing
+ to Cpc CmObj to avoid creating corrupted _CPC objects.
+
+ @param [in] PlatformRepo Platfom Info repository.
+
+ @retval EFI_SUCCESS Success.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+ClearCpcInfo (
+ EDKII_PLATFORM_REPOSITORY_INFO *PlatformRepo
+ )
+{
+ CM_ARM_GICC_INFO *GicCInfo;
+
+ GicCInfo = (CM_ARM_GICC_INFO*)&PlatformRepo->GicCInfo;
+
+ GicCInfo[0].CpcToken = CM_NULL_TOKEN;
+ GicCInfo[1].CpcToken = CM_NULL_TOKEN;
+ GicCInfo[2].CpcToken = CM_NULL_TOKEN;
+ GicCInfo[3].CpcToken = CM_NULL_TOKEN;
+ GicCInfo[4].CpcToken = CM_NULL_TOKEN;
+ GicCInfo[5].CpcToken = CM_NULL_TOKEN;
+
+ return EFI_SUCCESS;
+}
+
+/** Use the SCMI protocol to populate CPC objects dynamically.
+
+ @param [in] PlatformRepo Platfom Info repository.
+ @param [in] DomainId Id of the DVFS domain to probe.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_UNSUPPORTED Not supported.
+ @retval !(EFI_SUCCESS) An error occured.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+PopulateCpcInfo (
+ EDKII_PLATFORM_REPOSITORY_INFO *PlatformRepo,
+ IN UINT32 DomainId
+ )
+{
+ EFI_STATUS Status;
+ CM_ARM_GICC_INFO *GicCInfo;
+ AML_CPC_INFO *CpcInfo;
+
+ if ((PlatformRepo == NULL) ||
+ ((DomainId != PSD_BIG_DOMAIN_ID) &&
+ (DomainId != PSD_LITTLE_DOMAIN_ID))) {
+ Status = EFI_INVALID_PARAMETER;
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+ }
+
+ CpcInfo = &PlatformRepo->CpcInfo[DomainId];
+ GicCInfo = (CM_ARM_GICC_INFO*)&PlatformRepo->GicCInfo;
+
+ Status = DynamicTablesScmiInfoGetFastChannel (
+ PlatformRepo->PsdInfo[DomainId].Domain,
+ CpcInfo
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ /* CPPC must advertise performances on a 'continuous, abstract, unit-less
+ performance scale', i.e. CPU performances on an asymmetric platform
+ nust be represented on a unified scale.
+ CPU performance values are obtained from SCP through SCMI and advertised
+ to the OS via the _CPC objects. SCP currently maps performance requests
+ to frequency requests.
+ Thus, SCP must be modified to advertise (and correctly handle)
+ performance values on a unified scale.
+
+ Check that SCP is using a unified scale by checking that the advertised
+ lowest/nominal frequencies are not the default ones.
+ */
+ if (((DomainId == PSD_BIG_DOMAIN_ID) &&
+ (CpcInfo->LowestPerformanceInteger == 600000000) &&
+ (CpcInfo->NominalPerformanceInteger == 1000000000)) ||
+ ((DomainId == PSD_LITTLE_DOMAIN_ID) &&
+ (CpcInfo->LowestPerformanceInteger == 450000000) &&
+ (CpcInfo->NominalPerformanceInteger == 800000000))) {
+ return EFI_UNSUPPORTED;
+ }
+
+ // Juno R2's lowest/nominal frequencies.
+ // Nominal frequency != Highest frequency.
+ if (DomainId == PSD_BIG_DOMAIN_ID) {
+ CpcInfo->LowestFrequencyInteger = 600;
+ CpcInfo->NominalFrequencyInteger = 1000;
+ } else {
+ CpcInfo->LowestFrequencyInteger = 450;
+ CpcInfo->NominalFrequencyInteger = 800;
+ }
+
+ // The mapping Psd -> CPUs is available here.
+ if (DomainId == PSD_BIG_DOMAIN_ID) {
+ GicCInfo[0].CpcToken = (CM_OBJECT_TOKEN)CpcInfo;
+ GicCInfo[1].CpcToken = (CM_OBJECT_TOKEN)CpcInfo;
+ } else {
+ GicCInfo[2].CpcToken = (CM_OBJECT_TOKEN)CpcInfo;
+ GicCInfo[3].CpcToken = (CM_OBJECT_TOKEN)CpcInfo;
+ GicCInfo[4].CpcToken = (CM_OBJECT_TOKEN)CpcInfo;
+ GicCInfo[5].CpcToken = (CM_OBJECT_TOKEN)CpcInfo;
+ }
+
+ /*
+ Arm advises to use FFH to the following registers which uses AMU counters:
+ - ReferencePerformanceCounterRegister
+ - DeliveredPerformanceCounterRegister
+ Cf. Arm Functional Fixed Hardware Specification
+ s3.2 Performance management and Collaborative Processor Performance Control
+
+ AMU is not supported by the Juno, so clear these registers.
+ */
+ CpcInfo->ReferencePerformanceCounterRegister.AddressSpaceId = EFI_ACPI_6_5_SYSTEM_MEMORY;
+ CpcInfo->ReferencePerformanceCounterRegister.RegisterBitWidth = 0;
+ CpcInfo->ReferencePerformanceCounterRegister.RegisterBitOffset = 0;
+ CpcInfo->ReferencePerformanceCounterRegister.AccessSize = 0;
+ CpcInfo->ReferencePerformanceCounterRegister.Address = 0;
+
+ CpcInfo->DeliveredPerformanceCounterRegister.AddressSpaceId = EFI_ACPI_6_5_SYSTEM_MEMORY;
+ CpcInfo->DeliveredPerformanceCounterRegister.RegisterBitWidth = 0;
+ CpcInfo->DeliveredPerformanceCounterRegister.RegisterBitOffset = 0;
+ CpcInfo->DeliveredPerformanceCounterRegister.AccessSize = 0;
+ CpcInfo->DeliveredPerformanceCounterRegister.Address = 0;
+
+ return Status;
+}
+
+/** Iterate over the PSD Domains and try to populate the Cpc objects.
[SAMI] Documentation for parameters is missing. I will add that before merging.
+**/
+STATIC
+VOID
+EFIAPI
+PopulateCpcObjects (
+ EDKII_PLATFORM_REPOSITORY_INFO * PlatformRepo
+ )
+{
+ EFI_STATUS Status;
+ UINT32 Index;
+ BOOLEAN CpcFailed;
+
+ CpcFailed = FALSE;
+ for (Index = 0; Index < PSD_DOMAIN_COUNT; Index++) {
+ Status = PopulateCpcInfo (PlatformRepo, Index);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_WARN, "WARN: Could not populate _CPC.\n"));
+ CpcFailed = TRUE;
+ break;
+ }
+ }
+
+ if (CpcFailed) {
+ // _CPC information is not mandatory and SCP might not support some
+ // SCMI requests. Failing should not prevent from booting.
+ ClearCpcInfo (PlatformRepo);
+ }
+}
+
/** Initialize the platform configuration repository.


@param [in] This Pointer to the Configuration Manager Protocol.
@@ -920,6 +1088,23 @@ InitializePlatformRepository (


GetJunoRevision (PlatformRepo->JunoRevision);
DEBUG ((DEBUG_INFO, "Juno Rev = 0x%x\n", PlatformRepo->JunoRevision));
+
+ ///
+ /// 1.
+ /// _CPC was only tested on Juno R2, so only enable support for this version.
+ ///
+ /// 2.
+ /// Some _CPC registers cannot be populated for the Juno:
+ /// - PerformanceLimitedRegister
+ /// - ReferencePerformanceCounterRegister
+ /// - DeliveredPerformanceCounterRegister
+ /// Only build _CPC objects if relaxation regarding these registers
+ /// is allowed.
+ if ((PlatformRepo->JunoRevision == JUNO_REVISION_R2) &&
+ (PcdGet64(PcdDevelopmentPlatformRelaxations) & BIT0)) {
+ PopulateCpcObjects (PlatformRepo);
+ }
+
return EFI_SUCCESS;
}


@@ -1212,6 +1397,55 @@ GetPsdInfo (
return EFI_NOT_FOUND;
}


+/** Return Cpc Info.
+
+ @param [in] This Pointer to the Configuration Manager Protocol.
+ @param [in] CmObjectId The Object ID of the CM object requested
+ @param [in] SearchToken A unique token for identifying the requested
+ CM_ARM_PCI_INTERRUPT_MAP_INFO object.
+ @param [in, out] CmObject Pointer to the Configuration Manager Object
+ descriptor describing the requested Object.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_NOT_FOUND The required object information is not found.
+**/
+EFI_STATUS
+EFIAPI
+GetCpcInfo (
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST This,
+ IN CONST CM_OBJECT_ID CmObjectId,
+ IN CONST CM_OBJECT_TOKEN SearchToken,
+ IN OUT CM_OBJ_DESCRIPTOR * CONST CmObject
+ )
+{
+ EDKII_PLATFORM_REPOSITORY_INFO * PlatformRepo;
+ UINT32 TotalObjCount;
+ UINT32 ObjIndex;
+
+ if ((This == NULL) || (CmObject == NULL)) {
+ ASSERT (This != NULL);
+ ASSERT (CmObject != NULL);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ PlatformRepo = This->PlatRepoInfo;
+
+ TotalObjCount = ARRAY_SIZE (PlatformRepo->CpcInfo);
+
+ for (ObjIndex = 0; ObjIndex < TotalObjCount; ObjIndex++) {
+ if (SearchToken == (CM_OBJECT_TOKEN)&PlatformRepo->CpcInfo[ObjIndex]) {
+ CmObject->ObjectId = CmObjectId;
+ CmObject->Size = sizeof (PlatformRepo->CpcInfo[ObjIndex]);
+ CmObject->Data = (VOID*)&PlatformRepo->CpcInfo[ObjIndex];
+ CmObject->Count = 1;
+ return EFI_SUCCESS;
+ }
+ }
+
+ return EFI_NOT_FOUND;
+}
+
/** Return a list of Configuration Manager object references pointed to by the
given input token.


@@ -1633,6 +1867,19 @@ GetArmNameSpaceObject (
);
break;


+ case EArmObjCpcInfo:
+ Status = HandleCmObjectRefByToken (
+ This,
+ CmObjectId,
+ PlatformRepo->CpcInfo,
+ sizeof (PlatformRepo->CpcInfo),
+ ARRAY_SIZE (PlatformRepo->CpcInfo),
+ Token,
+ GetCpcInfo,
+ CmObject
+ );
+ break;
+
default: {
Status = EFI_NOT_FOUND;
DEBUG ((
@@ -1821,7 +2068,7 @@ ConfigurationManagerDxeInitialize (
" Status = %r\n",
Status
));
- goto error_handler;
+ return Status;
[SAMI] This change not related to this patch. I will drop this before merging.
}


Status = InitializePlatformRepository (
@@ -1836,6 +2083,5 @@ ConfigurationManagerDxeInitialize (
));
}


-error_handler:
- return Status;
+ return EFI_SUCCESS;
[SAMI] This change not related to this patch. Also, we should be returning the proper error code instead of EFI_SUCESS. I will drop this before merging. 
}
diff --git a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h
index e58e9cbecb23..78452295a180 100644
--- a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h
+++ b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h
@@ -303,6 +303,13 @@ typedef struct PlatformRepositoryInfo {
// Power domains
CM_ARM_PSD_INFO PsdInfo[PSD_DOMAIN_COUNT];


+ //
+ // Dynamically populated fields from here.
+ //
+
+ // Cpc info (1 for each PSD domain)
+ CM_ARM_CPC_INFO CpcInfo[PSD_DOMAIN_COUNT];
+
/// Juno Board Revision
UINT32 JunoRevision;
} EDKII_PLATFORM_REPOSITORY_INFO;
diff --git a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
index 91bffe8d5d82..dea475375688 100644
--- a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
+++ b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
@@ -35,6 +35,7 @@ [Packages]


[LibraryClasses]
ArmPlatformLib
+ DynamicTablesScmiInfoLib
PrintLib
UefiBootServicesTableLib
UefiDriverEntryPoint
@@ -76,6 +77,8 @@ [FixedPcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate


+ gEdkiiDynamicTablesPkgTokenSpaceGuid.PcdDevelopmentPlatformRelaxations
+
[Pcd]


[Depex]
-- 
2.25.1







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