Re: [edk2-devel] [PATCH v4 10/10] DynamicTablesPkg: Remove check for _CPC field

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

Hi Pierre,

I think we should not relax the checks as this may lead to a production platform ignoring the mandatory fields.
Maybe we need to define a pre-processor e.g. DEVELOPMENT_PLATFORM_RELAXATIONS and add additional warnings to say that the platform is not spec compliant.

Regards,

Sami Mujawar


On 12/12/2023, 09:30, "Pierre Gondois" <pierre.gondois@arm.com <mailto:pierre.gondois@arm.com>> wrote:


When generating _CPC objects, some fields are mandatory by spec [1].
Some fields cannot be supported by a the Juno platform, which is used
to test the _CPC generation. Therefore, don't prevent the generation
of _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


Also warn if one of the following optional fields is missing:
- HighestPerformance[Buffer|Integer]
- NominalPerformance[Buffer|Integer]


[1] Cf. non-optional fields in:
ACPI 6.5, s8.4.6.1 '_CPC (Continuous Performance Control)'


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


diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index 2b370320d244..cc258127b62e 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -3531,6 +3531,22 @@ AmlCreateCpcNode (
return EFI_INVALID_PARAMETER;


}






+ // The following fields are theoretically mandatory, but not supported


+ // by some platforms. Just warn if they are missing:


+ // - PerformanceLimitedRegister


+ // - ReferencePerformanceCounterRegister


+ // - DeliveredPerformanceCounterRegister


+ if (IsNullGenericAddress (&CpcInfo->PerformanceLimitedRegister) ||


+ IsNullGenericAddress (&CpcInfo->ReferencePerformanceCounterRegister) ||


+ IsNullGenericAddress (&CpcInfo->DeliveredPerformanceCounterRegister))


+ {


+ DEBUG ((


+ DEBUG_WARN,


+ "Missing PerformanceLimited|ReferencePerformanceCounter|"


+ "DeliveredPerformanceCounter field in _CPC object\n"


+ ));


+ }


+


if ((IsNullGenericAddress (&CpcInfo->HighestPerformanceBuffer) &&


(CpcInfo->HighestPerformanceInteger == 0)) ||


(IsNullGenericAddress (&CpcInfo->NominalPerformanceBuffer) &&


@@ -3539,13 +3555,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 (#114344): https://edk2.groups.io/g/devel/message/114344
Mute This Topic: https://groups.io/mt/103127056/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-