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

2023-11-02 Thread PierreGondois 




On 10/26/23 13:05, Leif Lindholm wrote:

On Wed, Oct 25, 2023 at 13:25:40 +0200, pierre.gond...@arm.com wrote:

From: Pierre Gondois 

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 
---
  .../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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

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

2023-10-26 Thread Leif Lindholm 
On Wed, Oct 25, 2023 at 13:25:40 +0200, pierre.gond...@arm.com wrote:
> From: Pierre Gondois 
> 
> 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 
> ---
>  .../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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

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

2023-10-25 Thread PierreGondois 
From: Pierre Gondois 

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 
---
 .../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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-