[edk2-devel] [PATCH v6 1/1] DynamicTablesPkg: Exempt some _CPC field from checks

[edk2-devel] [PATCH v6 1/1] DynamicTablesPkg: Exempt some _CPC field from checks

[PierreGondois]

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, rely on the
PcdDevelopmentPlatformRelaxations Pcd to either:
- warn about the missing fields and and let the OS handle the
  missing information
- consider the missing fields as an error

_CPC fields that are exempted from checks when the Pcd is set:
- PerformanceLimitedRegister
- ReferencePerformanceCounterRegister
- DeliveredPerformanceCounterRegister

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

Notes:
    v6:
    Do not warn if one of the following fields is missing:
    - HighestPerformanceBuffer
    - NominalPerformanceBuffer
    An error code should just be returned.

 .../Library/Common/AmlLib/AmlLib.inf          |  3 ++
 .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 28 ++++++++++++++++---
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf b/DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
index 6d0aa6ff934c..f269691864d4 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
+++ b/DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
@@ -71,6 +71,9 @@ [LibraryClasses]
   BaseLib
   MemoryAllocationLib
 
+[FixedPcd]
+  gEdkiiDynamicTablesPkgTokenSpaceGuid.PcdDevelopmentPlatformRelaxations
+
 [BuildOptions]
   *_*_*_CC_FLAGS = -DAML_HANDLE
 
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index 6f3f46e3b1ed..22c2d598d0d8 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -3532,6 +3532,29 @@ AmlCreateCpcNode (
     return EFI_INVALID_PARAMETER;
   }
 
+  /// The following fields are theoretically mandatory, but not supported
+  /// by some platforms.
+  /// - PerformanceLimitedRegister
+  /// - ReferencePerformanceCounterRegister
+  /// - DeliveredPerformanceCounterRegister
+  /// Warn if BIT0 in PcdDevelopmentPlatformRelaxations is set, otherwise
+  /// return an error.
+  if (IsNullGenericAddress (&CpcInfo->PerformanceLimitedRegister) ||
+      IsNullGenericAddress (&CpcInfo->ReferencePerformanceCounterRegister) ||
+      IsNullGenericAddress (&CpcInfo->DeliveredPerformanceCounterRegister))
+  {
+    if ((PcdGet64 (PcdDevelopmentPlatformRelaxations) & BIT0) != 0) {
+      DEBUG ((
+        DEBUG_WARN,
+        "Missing PerformanceLimited|ReferencePerformanceCounter|"
+        "DeliveredPerformanceCounter field in _CPC object\n"
+        ));
+    } else {
+      ASSERT (0);
+      return EFI_INVALID_PARAMETER;
+    }
+  }
+
   if ((IsNullGenericAddress (&CpcInfo->HighestPerformanceBuffer) &&
        (CpcInfo->HighestPerformanceInteger == 0)) ||
       (IsNullGenericAddress (&CpcInfo->NominalPerformanceBuffer) &&
@@ -3540,10 +3563,7 @@ 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;
-- 
2.25.1



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

Re: [edk2-devel] [PATCH v6 1/1] DynamicTablesPkg: Exempt some _CPC field from checks

[PierreGondois]

Hello Sami,
Instead of re-sending the whole serie, only this patch was updated/re-submitted.
The v5 of this patch is at:
   https://edk2.groups.io/g/devel/message/114421

Regards,
Pierre

On 1/29/24 13:09, Pierre Gondois 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, rely on the
> PcdDevelopmentPlatformRelaxations Pcd to either:
> - warn about the missing fields and and let the OS handle the
>    missing information
> - consider the missing fields as an error
> 
> _CPC fields that are exempted from checks when the Pcd is set:
> - PerformanceLimitedRegister
> - ReferencePerformanceCounterRegister
> - DeliveredPerformanceCounterRegister
> 
> [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>
> ---
> 
> Notes:
>      v6:
>      Do not warn if one of the following fields is missing:
>      - HighestPerformanceBuffer
>      - NominalPerformanceBuffer
>      An error code should just be returned.
> 
>   .../Library/Common/AmlLib/AmlLib.inf          |  3 ++
>   .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 28 ++++++++++++++++---
>   2 files changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf b/DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
> index 6d0aa6ff934c..f269691864d4 100644
> --- a/DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
> +++ b/DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
> @@ -71,6 +71,9 @@ [LibraryClasses]
>     BaseLib
>     MemoryAllocationLib
>   
> +[FixedPcd]
> +  gEdkiiDynamicTablesPkgTokenSpaceGuid.PcdDevelopmentPlatformRelaxations
> +
>   [BuildOptions]
>     *_*_*_CC_FLAGS = -DAML_HANDLE
>   
> diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> index 6f3f46e3b1ed..22c2d598d0d8 100644
> --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> @@ -3532,6 +3532,29 @@ AmlCreateCpcNode (
>       return EFI_INVALID_PARAMETER;
>     }
>   
> +  /// The following fields are theoretically mandatory, but not supported
> +  /// by some platforms.
> +  /// - PerformanceLimitedRegister
> +  /// - ReferencePerformanceCounterRegister
> +  /// - DeliveredPerformanceCounterRegister
> +  /// Warn if BIT0 in PcdDevelopmentPlatformRelaxations is set, otherwise
> +  /// return an error.
> +  if (IsNullGenericAddress (&CpcInfo->PerformanceLimitedRegister) ||
> +      IsNullGenericAddress (&CpcInfo->ReferencePerformanceCounterRegister) ||
> +      IsNullGenericAddress (&CpcInfo->DeliveredPerformanceCounterRegister))
> +  {
> +    if ((PcdGet64 (PcdDevelopmentPlatformRelaxations) & BIT0) != 0) {
> +      DEBUG ((
> +        DEBUG_WARN,
> +        "Missing PerformanceLimited|ReferencePerformanceCounter|"
> +        "DeliveredPerformanceCounter field in _CPC object\n"
> +        ));
> +    } else {
> +      ASSERT (0);
> +      return EFI_INVALID_PARAMETER;
> +    }
> +  }
> +
>     if ((IsNullGenericAddress (&CpcInfo->HighestPerformanceBuffer) &&
>          (CpcInfo->HighestPerformanceInteger == 0)) ||
>         (IsNullGenericAddress (&CpcInfo->NominalPerformanceBuffer) &&
> @@ -3540,10 +3563,7 @@ 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;


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

Re: [edk2-devel] [PATCH v6 1/1] DynamicTablesPkg: Exempt some _CPC field from checks

[Sami Mujawar]

Hi Pierre,

Thank you for this patch.

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

Regards,

Sami Mujawar

On 29/01/2024, 12:09, "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, rely on the
PcdDevelopmentPlatformRelaxations Pcd to either:
- warn about the missing fields and and let the OS handle the
missing information
- consider the missing fields as an error


_CPC fields that are exempted from checks when the Pcd is set:
- PerformanceLimitedRegister
- ReferencePerformanceCounterRegister
- DeliveredPerformanceCounterRegister


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


Notes:
v6:
Do not warn if one of the following fields is missing:
- HighestPerformanceBuffer
- NominalPerformanceBuffer
An error code should just be returned.


.../Library/Common/AmlLib/AmlLib.inf | 3 ++
.../Common/AmlLib/CodeGen/AmlCodeGen.c | 28 ++++++++++++++++---
2 files changed, 27 insertions(+), 4 deletions(-)


diff --git a/DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf b/DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
index 6d0aa6ff934c..f269691864d4 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
+++ b/DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
@@ -71,6 +71,9 @@ [LibraryClasses]
BaseLib


MemoryAllocationLib






+[FixedPcd]


+ gEdkiiDynamicTablesPkgTokenSpaceGuid.PcdDevelopmentPlatformRelaxations


+


[BuildOptions]


*_*_*_CC_FLAGS = -DAML_HANDLE






diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index 6f3f46e3b1ed..22c2d598d0d8 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -3532,6 +3532,29 @@ AmlCreateCpcNode (
return EFI_INVALID_PARAMETER;


}






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


+ /// by some platforms.


+ /// - PerformanceLimitedRegister


+ /// - ReferencePerformanceCounterRegister


+ /// - DeliveredPerformanceCounterRegister


+ /// Warn if BIT0 in PcdDevelopmentPlatformRelaxations is set, otherwise


+ /// return an error.


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


+ IsNullGenericAddress (&CpcInfo->ReferencePerformanceCounterRegister) ||


+ IsNullGenericAddress (&CpcInfo->DeliveredPerformanceCounterRegister))


+ {


+ if ((PcdGet64 (PcdDevelopmentPlatformRelaxations) & BIT0) != 0) {


+ DEBUG ((


+ DEBUG_WARN,


+ "Missing PerformanceLimited|ReferencePerformanceCounter|"


+ "DeliveredPerformanceCounter field in _CPC object\n"


+ ));


+ } else {


+ ASSERT (0);


+ return EFI_INVALID_PARAMETER;


+ }


+ }


+


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


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


(IsNullGenericAddress (&CpcInfo->NominalPerformanceBuffer) &&


@@ -3540,10 +3563,7 @@ 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;


-- 
2.25.1







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