Re: [edk2] [Patch v2 3/4] UefiCpuPkg/RegisterCpuFeaturesLib: Simplify PcdCpuFeaturesSupport.

2019-04-02 Thread Ni, Ray
Reviewed-by: Ray Ni 

> -Original Message-
> From: edk2-devel  On Behalf Of Eric Dong
> Sent: Friday, March 1, 2019 1:40 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [Patch v2 3/4] UefiCpuPkg/RegisterCpuFeaturesLib: Simplify
> PcdCpuFeaturesSupport.
> 
> PcdCpuFeaturesSupport used to specify the platform policy about what CPU
> features this platform supports. This PCD will be used in 
> IsCpuFeatureSupported
> only.
> 
> Now RegisterCpuFeaturesLib use this PCD as an template to Get the pcd size.
> Update the code logic to replace it with PcdCpuFeaturesSetting.
> 
> BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1375
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 43 
> +-
>  .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h   |  1 -
>  .../RegisterCpuFeaturesLib.c   | 10 ++---
>  3 files changed, 22 insertions(+), 32 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> index d877caff74..c82f848b97 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> @@ -245,11 +245,6 @@ CpuInitDataInitialize (
>ASSERT (CpuFeaturesData->CpuFlags.CoreSemaphoreCount != NULL);
>CpuFeaturesData->CpuFlags.PackageSemaphoreCount = AllocateZeroPool
> (sizeof (UINT32) * CpuStatus->PackageCount * CpuStatus->MaxCoreCount *
> CpuStatus->MaxThreadCount);
>ASSERT (CpuFeaturesData->CpuFlags.PackageSemaphoreCount != NULL);
> -
> -  //
> -  // Get support and configuration PCDs
> -  //
> -  CpuFeaturesData->SupportPcd   = GetSupportPcd ();
>  }
> 
>  /**
> @@ -269,7 +264,7 @@ SupportedMaskOr (
>UINT8  *Data1;
>UINT8  *Data2;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport);
> +  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
>Data1 = SupportedFeatureMask;
>Data2 = OrFeatureBitMask;
>for (Index = 0; Index < BitMaskSize; Index++) { @@ -294,7 +289,7 @@
> SupportedMaskAnd (
>UINT8  *Data1;
>UINT8  *Data2;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport);
> +  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
>Data1 = SupportedFeatureMask;
>Data2 = AndFeatureBitMask;
>for (Index = 0; Index < BitMaskSize; Index++) { @@ -319,7 +314,7 @@
> SupportedMaskCleanBit (
>UINT8  *Data1;
>UINT8  *Data2;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport);
> +  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
>Data1 = SupportedFeatureMask;
>Data2 = AndFeatureBitMask;
>for (Index = 0; Index < BitMaskSize; Index++) { @@ -350,7 +345,7 @@
> IsBitMaskMatch (
>UINT8  *Data1;
>UINT8  *Data2;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport);
> +  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
> 
>Data1 = SupportedFeatureMask;
>Data2 = ComparedFeatureBitMask;
> @@ -389,21 +384,19 @@ CollectProcessorData (
>Entry = GetFirstNode (>FeatureList);
>while (!IsNull (>FeatureList, Entry)) {
>  CpuFeature = CPU_FEATURE_ENTRY_FROM_LINK (Entry);
> -if (IsBitMaskMatch (CpuFeaturesData->SupportPcd, CpuFeature-
> >FeatureMask)) {
> -  if (CpuFeature->SupportFunc == NULL) {
> -//
> -// If SupportFunc is NULL, then the feature is supported.
> -//
> -SupportedMaskOr (
> -  CpuFeaturesData->InitOrder[ProcessorNumber].FeaturesSupportedMask,
> -  CpuFeature->FeatureMask
> -  );
> -  } else if (CpuFeature->SupportFunc (ProcessorNumber, CpuInfo, 
> CpuFeature-
> >ConfigData)) {
> -SupportedMaskOr (
> -  CpuFeaturesData->InitOrder[ProcessorNumber].FeaturesSupportedMask,
> -  CpuFeature->FeatureMask
> -  );
> -  }
> +if (CpuFeature->SupportFunc == NULL) {
> +  //
> +  // If SupportFunc is NULL, then the feature is supported.
> +  //
> +  SupportedMaskOr (
> +CpuFeaturesData->InitOrder[ProcessorNumber].FeaturesSupportedMask,
> +CpuFeature->FeatureMask
> +);
> +} else if (CpuFeature->SupportFunc (ProcessorNumber, CpuInfo, CpuFeature-
> >ConfigData)) {
> +  SupportedMaskOr (
> +CpuFeaturesData->InitOrder[ProcessorNumber].FeaturesSupportedMask,
> +CpuFeature

[edk2] [Patch v2 3/4] UefiCpuPkg/RegisterCpuFeaturesLib: Simplify PcdCpuFeaturesSupport.

2019-02-28 Thread Eric Dong
PcdCpuFeaturesSupport used to specify the platform policy about
what CPU features this platform supports. This PCD will be used
in IsCpuFeatureSupported only.

Now RegisterCpuFeaturesLib use this PCD as an template to Get the
pcd size. Update the code logic to replace it with
PcdCpuFeaturesSetting.

BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1375

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 43 +-
 .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h   |  1 -
 .../RegisterCpuFeaturesLib.c   | 10 ++---
 3 files changed, 22 insertions(+), 32 deletions(-)

diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c 
b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index d877caff74..c82f848b97 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -245,11 +245,6 @@ CpuInitDataInitialize (
   ASSERT (CpuFeaturesData->CpuFlags.CoreSemaphoreCount != NULL);
   CpuFeaturesData->CpuFlags.PackageSemaphoreCount = AllocateZeroPool (sizeof 
(UINT32) * CpuStatus->PackageCount * CpuStatus->MaxCoreCount * 
CpuStatus->MaxThreadCount);
   ASSERT (CpuFeaturesData->CpuFlags.PackageSemaphoreCount != NULL);
-
-  //
-  // Get support and configuration PCDs
-  //
-  CpuFeaturesData->SupportPcd   = GetSupportPcd ();
 }
 
 /**
@@ -269,7 +264,7 @@ SupportedMaskOr (
   UINT8  *Data1;
   UINT8  *Data2;
 
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport);
+  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
   Data1 = SupportedFeatureMask;
   Data2 = OrFeatureBitMask;
   for (Index = 0; Index < BitMaskSize; Index++) {
@@ -294,7 +289,7 @@ SupportedMaskAnd (
   UINT8  *Data1;
   UINT8  *Data2;
 
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport);
+  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
   Data1 = SupportedFeatureMask;
   Data2 = AndFeatureBitMask;
   for (Index = 0; Index < BitMaskSize; Index++) {
@@ -319,7 +314,7 @@ SupportedMaskCleanBit (
   UINT8  *Data1;
   UINT8  *Data2;
 
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport);
+  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
   Data1 = SupportedFeatureMask;
   Data2 = AndFeatureBitMask;
   for (Index = 0; Index < BitMaskSize; Index++) {
@@ -350,7 +345,7 @@ IsBitMaskMatch (
   UINT8  *Data1;
   UINT8  *Data2;
 
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport);
+  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
 
   Data1 = SupportedFeatureMask;
   Data2 = ComparedFeatureBitMask;
@@ -389,21 +384,19 @@ CollectProcessorData (
   Entry = GetFirstNode (>FeatureList);
   while (!IsNull (>FeatureList, Entry)) {
 CpuFeature = CPU_FEATURE_ENTRY_FROM_LINK (Entry);
-if (IsBitMaskMatch (CpuFeaturesData->SupportPcd, CpuFeature->FeatureMask)) 
{
-  if (CpuFeature->SupportFunc == NULL) {
-//
-// If SupportFunc is NULL, then the feature is supported.
-//
-SupportedMaskOr (
-  CpuFeaturesData->InitOrder[ProcessorNumber].FeaturesSupportedMask,
-  CpuFeature->FeatureMask
-  );
-  } else if (CpuFeature->SupportFunc (ProcessorNumber, CpuInfo, 
CpuFeature->ConfigData)) {
-SupportedMaskOr (
-  CpuFeaturesData->InitOrder[ProcessorNumber].FeaturesSupportedMask,
-  CpuFeature->FeatureMask
-  );
-  }
+if (CpuFeature->SupportFunc == NULL) {
+  //
+  // If SupportFunc is NULL, then the feature is supported.
+  //
+  SupportedMaskOr (
+CpuFeaturesData->InitOrder[ProcessorNumber].FeaturesSupportedMask,
+CpuFeature->FeatureMask
+);
+} else if (CpuFeature->SupportFunc (ProcessorNumber, CpuInfo, 
CpuFeature->ConfigData)) {
+  SupportedMaskOr (
+CpuFeaturesData->InitOrder[ProcessorNumber].FeaturesSupportedMask,
+CpuFeature->FeatureMask
+);
 }
 Entry = Entry->ForwardLink;
   }
@@ -596,8 +589,6 @@ AnalysisProcessorFeatures (
   DumpCpuFeature (CpuFeature);
   Entry = Entry->ForwardLink;
 }
-DEBUG ((DEBUG_INFO, "PcdCpuFeaturesSupport:\n"));
-DumpCpuFeatureMask (CpuFeaturesData->SupportPcd);
 DEBUG ((DEBUG_INFO, "PcdCpuFeaturesCapability:\n"));
 DumpCpuFeatureMask (CpuFeaturesData->CapabilityPcd);
 DEBUG ((DEBUG_INFO, "Origin PcdCpuFeaturesSetting:\n"));
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h 
b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
index 3e0a342fd1..836ed3549c 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
@@ -81,7 +81,6 @@ typedef struct {
   LIST_ENTRY   FeatureList;
 
   CPU_FEATURES_INIT_ORDER