public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch v3 0/2] UefiCpuPkg/RegisterCpuFeaturesLib: Avoid AP calls PeiServices table
@ 2019-07-17  1:58 Dong, Eric
  2019-07-17  1:58 ` [Patch v3 1/2] " Dong, Eric
  2019-07-17  1:59 ` [Patch v3 2/2] UefiCpuPkg/RegisterCpuFeaturesLib: avoid use dynamic PCD Dong, Eric
  0 siblings, 2 replies; 5+ messages in thread
From: Dong, Eric @ 2019-07-17  1:58 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Laszlo Ersek, Chandana Kumar, Star Zeng

V3 Changes:
1. Remove IsCpuFeatureMatch function.

V2 Changes:
1. Revert IsBitMaskMatchCheck change which is not correct.
2. refine some variable name.

v1 changes:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1972

AP calls CollectProcessorData() to collect processor info.
CollectProcessorData function finally calls PcdGetSize function to get DynamicPCD PcdCpuFeaturesSetting value. PcdGetSize will use gBS which caused ASSERT. 
This patch serial fixes the issue and enhances the related code to avoid later report this issue again.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Chandana Kumar <chandana.c.kumar@intel.com>
Cc: Star Zeng <star.zeng@intel.com>


Dong, Eric (1):
  UefiCpuPkg/RegisterCpuFeaturesLib: avoid use dynamic PCD.

Eric Dong (1):
  UefiCpuPkg/RegisterCpuFeaturesLib: Avoid AP calls PeiServices table.

 .../CpuFeaturesInitialize.c                   |  77 +++++++------
 .../RegisterCpuFeatures.h                     |  10 +-
 .../RegisterCpuFeaturesLib.c                  | 102 +++++++-----------
 3 files changed, 87 insertions(+), 102 deletions(-)

-- 
2.21.0.windows.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Patch v3 1/2] UefiCpuPkg/RegisterCpuFeaturesLib: Avoid AP calls PeiServices table.
  2019-07-17  1:58 [Patch v3 0/2] UefiCpuPkg/RegisterCpuFeaturesLib: Avoid AP calls PeiServices table Dong, Eric
@ 2019-07-17  1:58 ` Dong, Eric
  2019-07-17  1:59 ` [Patch v3 2/2] UefiCpuPkg/RegisterCpuFeaturesLib: avoid use dynamic PCD Dong, Eric
  1 sibling, 0 replies; 5+ messages in thread
From: Dong, Eric @ 2019-07-17  1:58 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Laszlo Ersek, Chandana Kumar, Star Zeng

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1972

AP calls CollectProcessorData() to collect processor info.
CollectProcessorData function finally calls PcdGetSize function to
get DynamicPCD PcdCpuFeaturesSetting value. PcdGetSize will use
PeiServices table which caused below assert info:
Processor Info: Package: 1, MaxCore : 4, MaxThread: 1
Package: 0, Valid Core : 4
ASSERT [CpuFeaturesPei] c:\projects\jsl\jsl_v1193\Edk2\MdePkg\Library
\PeiServicesTablePointerLibIdt\PeiServicesTablePointer.c(48):
PeiServices != ((void *) 0)

This change uses saved global pcd size instead of calls PcdGetSize to
fix this issue.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Chandana Kumar <chandana.c.kumar@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
---
 .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c  | 13 ++++++++-----
 .../RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c |  5 +++++
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index 1746f4f07f..1e25ba8b32 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -246,19 +246,20 @@ CpuInitDataInitialize (
 
   @param[in]  SupportedFeatureMask  The pointer to CPU feature bits mask buffer
   @param[in]  OrFeatureBitMask      The feature bit mask to do OR operation
+  @param[in]  BitMaskSize           The CPU feature bits mask buffer size.
+
 **/
 VOID
 SupportedMaskOr (
   IN UINT8               *SupportedFeatureMask,
-  IN UINT8               *OrFeatureBitMask
+  IN UINT8               *OrFeatureBitMask,
+  IN UINT32              BitMaskSize
   )
 {
   UINTN                  Index;
-  UINTN                  BitMaskSize;
   UINT8                  *Data1;
   UINT8                  *Data2;
 
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
   Data1 = SupportedFeatureMask;
   Data2 = OrFeatureBitMask;
   for (Index = 0; Index < BitMaskSize; Index++) {
@@ -384,12 +385,14 @@ CollectProcessorData (
       //
       SupportedMaskOr (
         CpuFeaturesData->InitOrder[ProcessorNumber].FeaturesSupportedMask,
-        CpuFeature->FeatureMask
+        CpuFeature->FeatureMask,
+        CpuFeaturesData->BitMaskSize
         );
     } else if (CpuFeature->SupportFunc (ProcessorNumber, CpuInfo, CpuFeature->ConfigData)) {
       SupportedMaskOr (
         CpuFeaturesData->InitOrder[ProcessorNumber].FeaturesSupportedMask,
-        CpuFeature->FeatureMask
+        CpuFeature->FeatureMask,
+        CpuFeaturesData->BitMaskSize
         );
     }
     Entry = Entry->ForwardLink;
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
index fa0f0b41e2..36aabd7267 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
@@ -658,6 +658,11 @@ RegisterCpuFeatureWorker (
     InitializeListHead (&CpuFeaturesData->FeatureList);
     InitializeSpinLock (&CpuFeaturesData->CpuFlags.MemoryMappedLock);
     InitializeSpinLock (&CpuFeaturesData->CpuFlags.ConsoleLogLock);
+    //
+    // Driver has assumption that these three PCD should has same buffer size.
+    //
+    ASSERT (PcdGetSize (PcdCpuFeaturesSetting) == PcdGetSize (PcdCpuFeaturesCapability));
+    ASSERT (PcdGetSize (PcdCpuFeaturesSetting) == PcdGetSize (PcdCpuFeaturesSupport));
     CpuFeaturesData->BitMaskSize = (UINT32) BitMaskSize;
   }
   ASSERT (CpuFeaturesData->BitMaskSize == BitMaskSize);
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [Patch v3 2/2] UefiCpuPkg/RegisterCpuFeaturesLib: avoid use dynamic PCD.
  2019-07-17  1:58 [Patch v3 0/2] UefiCpuPkg/RegisterCpuFeaturesLib: Avoid AP calls PeiServices table Dong, Eric
  2019-07-17  1:58 ` [Patch v3 1/2] " Dong, Eric
@ 2019-07-17  1:59 ` Dong, Eric
  2019-07-17  3:40   ` Zeng, Star
  1 sibling, 1 reply; 5+ messages in thread
From: Dong, Eric @ 2019-07-17  1:59 UTC (permalink / raw)
  To: devel; +Cc: Dong, Eric, Ray Ni, Laszlo Ersek, Chandana Kumar, Star Zeng

From: "Dong, Eric" <eric.dong@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1972

Function in this library may be used by APs. Assert will be trig if AP
uses dynamic pcd.
This patch enhance the current code, remove the unnecessary usage of
dynamic PCD. This change try to avoid report this issue again later.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Chandana Kumar <chandana.c.kumar@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
 .../CpuFeaturesInitialize.c                   |  64 +++++------
 .../RegisterCpuFeatures.h                     |  10 +-
 .../RegisterCpuFeaturesLib.c                  | 107 +++++++-----------
 3 files changed, 79 insertions(+), 102 deletions(-)

diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index 1e25ba8b32..33752c1a9f 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -15,20 +15,16 @@ CHAR16 *mRegisterTypeStr[] = {L"MSR", L"CR", L"MMIO", L"CACHE", L"SEMAP", L"INVA
   Worker function to save PcdCpuFeaturesCapability.
 
   @param[in]  SupportedFeatureMask  The pointer to CPU feature bits mask buffer
-  @param[in]  FeatureMaskSize       CPU feature bits mask buffer size.
+  @param[in]  BitMaskSize           CPU feature bits mask buffer size.
 
 **/
 VOID
 SetCapabilityPcd (
   IN UINT8               *SupportedFeatureMask,
-  IN UINT32              FeatureMaskSize
+  IN UINTN               BitMaskSize
   )
 {
   EFI_STATUS             Status;
-  UINTN                  BitMaskSize;
-
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesCapability);
-  ASSERT (FeatureMaskSize == BitMaskSize);
 
   Status = PcdSetPtrS (PcdCpuFeaturesCapability, &BitMaskSize, SupportedFeatureMask);
   ASSERT_EFI_ERROR (Status);
@@ -38,16 +34,16 @@ SetCapabilityPcd (
   Worker function to save PcdCpuFeaturesSetting.
 
   @param[in]  SupportedFeatureMask  The pointer to CPU feature bits mask buffer
+  @param[in]  BitMaskSize           CPU feature bits mask buffer size.
 **/
 VOID
 SetSettingPcd (
-  IN UINT8               *SupportedFeatureMask
+  IN UINT8               *SupportedFeatureMask,
+  IN UINTN               BitMaskSize
   )
 {
   EFI_STATUS             Status;
-  UINTN                  BitMaskSize;
 
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
   Status = PcdSetPtrS (PcdCpuFeaturesSetting, &BitMaskSize, SupportedFeatureMask);
   ASSERT_EFI_ERROR (Status);
 }
@@ -272,19 +268,20 @@ SupportedMaskOr (
 
   @param[in]  SupportedFeatureMask  The pointer to CPU feature bits mask buffer
   @param[in]  AndFeatureBitMask     The feature bit mask to do AND operation
+  @param[in]  BitMaskSize           CPU feature bits mask buffer size.
+
 **/
 VOID
 SupportedMaskAnd (
   IN       UINT8               *SupportedFeatureMask,
-  IN CONST UINT8               *AndFeatureBitMask
+  IN CONST UINT8               *AndFeatureBitMask,
+  IN       UINT32              BitMaskSize
   )
 {
   UINTN                  Index;
-  UINTN                  BitMaskSize;
   UINT8                  *Data1;
   CONST UINT8            *Data2;
 
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
   Data1 = SupportedFeatureMask;
   Data2 = AndFeatureBitMask;
   for (Index = 0; Index < BitMaskSize; Index++) {
@@ -297,19 +294,19 @@ SupportedMaskAnd (
 
   @param[in]  SupportedFeatureMask  The pointer to CPU feature bits mask buffer
   @param[in]  AndFeatureBitMask     The feature bit mask to do XOR operation
+  @param[in]  BitMaskSize           CPU feature bits mask buffer size.
 **/
 VOID
 SupportedMaskCleanBit (
   IN UINT8               *SupportedFeatureMask,
-  IN UINT8               *AndFeatureBitMask
+  IN UINT8               *AndFeatureBitMask,
+  IN UINT32              BitMaskSize
   )
 {
   UINTN                  Index;
-  UINTN                  BitMaskSize;
   UINT8                  *Data1;
   UINT8                  *Data2;
 
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
   Data1 = SupportedFeatureMask;
   Data2 = AndFeatureBitMask;
   for (Index = 0; Index < BitMaskSize; Index++) {
@@ -323,6 +320,7 @@ SupportedMaskCleanBit (
 
   @param[in]  SupportedFeatureMask   The pointer to CPU feature bits mask buffer
   @param[in]  ComparedFeatureBitMask The feature bit mask to be compared
+  @param[in]  BitMaskSize            CPU feature bits mask buffer size.
 
   @retval TRUE   The ComparedFeatureBitMask is set in CPU feature supported bits
                  mask buffer.
@@ -332,16 +330,14 @@ SupportedMaskCleanBit (
 BOOLEAN
 IsBitMaskMatch (
   IN UINT8               *SupportedFeatureMask,
-  IN UINT8               *ComparedFeatureBitMask
+  IN UINT8               *ComparedFeatureBitMask,
+  IN UINT32              BitMaskSize
   )
 {
   UINTN                  Index;
-  UINTN                  BitMaskSize;
   UINT8                  *Data1;
   UINT8                  *Data2;
 
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
-
   Data1 = SupportedFeatureMask;
   Data2 = ComparedFeatureBitMask;
   for (Index = 0; Index < BitMaskSize; Index++) {
@@ -557,14 +553,14 @@ AnalysisProcessorFeatures (
     //
     // Calculate the last capability on all processors
     //
-    SupportedMaskAnd (CpuFeaturesData->CapabilityPcd, CpuInitOrder->FeaturesSupportedMask);
+    SupportedMaskAnd (CpuFeaturesData->CapabilityPcd, CpuInitOrder->FeaturesSupportedMask, CpuFeaturesData->BitMaskSize);
   }
   //
   // Calculate the last setting
   //
   CpuFeaturesData->SettingPcd = AllocateCopyPool (CpuFeaturesData->BitMaskSize, CpuFeaturesData->CapabilityPcd);
   ASSERT (CpuFeaturesData->SettingPcd != NULL);
-  SupportedMaskAnd (CpuFeaturesData->SettingPcd, PcdGetPtr (PcdCpuFeaturesSetting));
+  SupportedMaskAnd (CpuFeaturesData->SettingPcd, PcdGetPtr (PcdCpuFeaturesSetting), CpuFeaturesData->BitMaskSize);
 
   //
   // Dump the last CPU feature list
@@ -574,8 +570,8 @@ AnalysisProcessorFeatures (
     Entry = GetFirstNode (&CpuFeaturesData->FeatureList);
     while (!IsNull (&CpuFeaturesData->FeatureList, Entry)) {
       CpuFeature = CPU_FEATURE_ENTRY_FROM_LINK (Entry);
-      if (IsBitMaskMatch (CpuFeature->FeatureMask, CpuFeaturesData->CapabilityPcd)) {
-        if (IsBitMaskMatch (CpuFeature->FeatureMask, CpuFeaturesData->SettingPcd)) {
+      if (IsBitMaskMatch (CpuFeature->FeatureMask, CpuFeaturesData->CapabilityPcd, CpuFeaturesData->BitMaskSize)) {
+        if (IsBitMaskMatch (CpuFeature->FeatureMask, CpuFeaturesData->SettingPcd, CpuFeaturesData->BitMaskSize)) {
           DEBUG ((DEBUG_INFO, "[Enable   ] "));
         } else {
           DEBUG ((DEBUG_INFO, "[Disable  ] "));
@@ -583,22 +579,22 @@ AnalysisProcessorFeatures (
       } else {
         DEBUG ((DEBUG_INFO, "[Unsupport] "));
       }
-      DumpCpuFeature (CpuFeature);
+      DumpCpuFeature (CpuFeature, CpuFeaturesData->BitMaskSize);
       Entry = Entry->ForwardLink;
     }
     DEBUG ((DEBUG_INFO, "PcdCpuFeaturesCapability:\n"));
-    DumpCpuFeatureMask (CpuFeaturesData->CapabilityPcd);
+    DumpCpuFeatureMask (CpuFeaturesData->CapabilityPcd, CpuFeaturesData->BitMaskSize);
     DEBUG ((DEBUG_INFO, "Origin PcdCpuFeaturesSetting:\n"));
-    DumpCpuFeatureMask (PcdGetPtr (PcdCpuFeaturesSetting));
+    DumpCpuFeatureMask (PcdGetPtr (PcdCpuFeaturesSetting), CpuFeaturesData->BitMaskSize);
     DEBUG ((DEBUG_INFO, "Final PcdCpuFeaturesSetting:\n"));
-    DumpCpuFeatureMask (CpuFeaturesData->SettingPcd);
+    DumpCpuFeatureMask (CpuFeaturesData->SettingPcd, CpuFeaturesData->BitMaskSize);
   );
 
   //
   // Save PCDs and display CPU PCDs
   //
   SetCapabilityPcd (CpuFeaturesData->CapabilityPcd, CpuFeaturesData->BitMaskSize);
-  SetSettingPcd (CpuFeaturesData->SettingPcd);
+  SetSettingPcd (CpuFeaturesData->SettingPcd, CpuFeaturesData->BitMaskSize);
 
   for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
     CpuInitOrder = &CpuFeaturesData->InitOrder[ProcessorNumber];
@@ -608,7 +604,7 @@ AnalysisProcessorFeatures (
       // Insert each feature into processor's order list
       //
       CpuFeature = CPU_FEATURE_ENTRY_FROM_LINK (Entry);
-      if (IsBitMaskMatch (CpuFeature->FeatureMask, CpuFeaturesData->CapabilityPcd)) {
+      if (IsBitMaskMatch (CpuFeature->FeatureMask, CpuFeaturesData->CapabilityPcd, CpuFeaturesData->BitMaskSize)) {
         CpuFeatureInOrder = AllocateCopyPool (sizeof (CPU_FEATURES_ENTRY), CpuFeature);
         ASSERT (CpuFeatureInOrder != NULL);
         InsertTailList (&CpuInitOrder->OrderList, &CpuFeatureInOrder->Link);
@@ -624,18 +620,18 @@ AnalysisProcessorFeatures (
       CpuFeatureInOrder = CPU_FEATURE_ENTRY_FROM_LINK (Entry);
 
       Success = FALSE;
-      if (IsBitMaskMatch (CpuFeatureInOrder->FeatureMask, CpuFeaturesData->SettingPcd)) {
+      if (IsBitMaskMatch (CpuFeatureInOrder->FeatureMask, CpuFeaturesData->SettingPcd, CpuFeaturesData->BitMaskSize)) {
         Status = CpuFeatureInOrder->InitializeFunc (ProcessorNumber, CpuInfo, CpuFeatureInOrder->ConfigData, TRUE);
         if (EFI_ERROR (Status)) {
           //
           // Clean the CpuFeatureInOrder->FeatureMask in setting PCD.
           //
-          SupportedMaskCleanBit (CpuFeaturesData->SettingPcd, CpuFeatureInOrder->FeatureMask);
+          SupportedMaskCleanBit (CpuFeaturesData->SettingPcd, CpuFeatureInOrder->FeatureMask, CpuFeaturesData->BitMaskSize);
           if (CpuFeatureInOrder->FeatureName != NULL) {
             DEBUG ((DEBUG_WARN, "Warning :: Failed to enable Feature: Name = %a.\n", CpuFeatureInOrder->FeatureName));
           } else {
             DEBUG ((DEBUG_WARN, "Warning :: Failed to enable Feature: Mask = "));
-            DumpCpuFeatureMask (CpuFeatureInOrder->FeatureMask);
+            DumpCpuFeatureMask (CpuFeatureInOrder->FeatureMask, CpuFeaturesData->BitMaskSize);
           }
         } else {
           Success = TRUE;
@@ -647,7 +643,7 @@ AnalysisProcessorFeatures (
             DEBUG ((DEBUG_WARN, "Warning :: Failed to disable Feature: Name = %a.\n", CpuFeatureInOrder->FeatureName));
           } else {
             DEBUG ((DEBUG_WARN, "Warning :: Failed to disable Feature: Mask = "));
-            DumpCpuFeatureMask (CpuFeatureInOrder->FeatureMask);
+            DumpCpuFeatureMask (CpuFeatureInOrder->FeatureMask, CpuFeaturesData->BitMaskSize);
           }
         } else {
           Success = TRUE;
@@ -699,7 +695,7 @@ AnalysisProcessorFeatures (
     // again during initialize the features.
     //
     DEBUG ((DEBUG_INFO, "Dump final value for PcdCpuFeaturesSetting:\n"));
-    DumpCpuFeatureMask (CpuFeaturesData->SettingPcd);
+    DumpCpuFeatureMask (CpuFeaturesData->SettingPcd, CpuFeaturesData->BitMaskSize);
 
     //
     // Dump the RegisterTable
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
index 5c546ee153..a18f926641 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
@@ -180,20 +180,26 @@ SwitchNewBsp (
   Function that uses DEBUG() macros to display the contents of a a CPU feature bit mask.
 
   @param[in]  FeatureMask  A pointer to the CPU feature bit mask.
+  @param[in]  BitMaskSize  CPU feature bits mask buffer size.
+
 **/
 VOID
 DumpCpuFeatureMask (
-  IN UINT8               *FeatureMask
+  IN UINT8               *FeatureMask,
+  IN UINT32              BitMaskSize
   );
 
 /**
   Dump CPU feature name or CPU feature bit mask.
 
   @param[in]  CpuFeature   Pointer to CPU_FEATURES_ENTRY
+  @param[in]  BitMaskSize  CPU feature bits mask buffer size.
+
 **/
 VOID
 DumpCpuFeature (
-  IN CPU_FEATURES_ENTRY  *CpuFeature
+  IN CPU_FEATURES_ENTRY  *CpuFeature,
+  IN UINT32              BitMaskSize
   );
 
 /**
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
index 36aabd7267..493566de5d 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
@@ -8,46 +8,22 @@
 
 #include "RegisterCpuFeatures.h"
 
-/**
-  Checks if two CPU feature bit masks are equal.
-
-  @param[in]  FirstFeatureMask  The first input CPU feature bit mask
-  @param[in]  SecondFeatureMask The second input CPU feature bit mask
-
-  @retval TRUE  Two CPU feature bit masks are equal.
-  @retval FALSE Two CPU feature bit masks are not equal.
-**/
-BOOLEAN
-IsCpuFeatureMatch (
-  IN UINT8               *FirstFeatureMask,
-  IN UINT8               *SecondFeatureMask
-  )
-{
-  UINTN                 BitMaskSize;
-
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
-  if (CompareMem (FirstFeatureMask, SecondFeatureMask, BitMaskSize) == 0) {
-    return TRUE;
-  } else {
-    return FALSE;
-  }
-}
-
 /**
   Function that uses DEBUG() macros to display the contents of a a CPU feature bit mask.
 
   @param[in]  FeatureMask  A pointer to the CPU feature bit mask.
+  @param[in]  BitMaskSize  CPU feature bits mask buffer size.
+
 **/
 VOID
 DumpCpuFeatureMask (
-  IN UINT8               *FeatureMask
+  IN UINT8               *FeatureMask,
+  IN UINT32              BitMaskSize
   )
 {
   UINTN                  Index;
   UINT8                  *Data8;
-  UINTN                  BitMaskSize;
 
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
   Data8       = (UINT8 *) FeatureMask;
   for (Index = 0; Index < BitMaskSize; Index++) {
     DEBUG ((DEBUG_INFO, " %02x ", *Data8++));
@@ -59,10 +35,13 @@ DumpCpuFeatureMask (
   Dump CPU feature name or CPU feature bit mask.
 
   @param[in]  CpuFeature   Pointer to CPU_FEATURES_ENTRY
+  @param[in]  BitMaskSize  CPU feature bits mask buffer size.
+
 **/
 VOID
 DumpCpuFeature (
-  IN CPU_FEATURES_ENTRY  *CpuFeature
+  IN CPU_FEATURES_ENTRY  *CpuFeature,
+  IN UINT32              BitMaskSize
   )
 {
 
@@ -70,7 +49,7 @@ DumpCpuFeature (
     DEBUG ((DEBUG_INFO, "FeatureName: %a\n", CpuFeature->FeatureName));
   } else {
     DEBUG ((DEBUG_INFO, "FeatureMask = "));
-    DumpCpuFeatureMask (CpuFeature->FeatureMask);
+    DumpCpuFeatureMask (CpuFeature->FeatureMask, BitMaskSize);
   }
 }
 
@@ -89,16 +68,16 @@ IsBitMaskMatchCheck (
   IN UINT8        *DependentBitMask
   )
 {
-  UINTN      Index;
-  UINTN      BitMaskSize;
-  UINT8      *Data1;
-  UINT8      *Data2;
+  UINTN              Index;
+  UINT8              *Data1;
+  UINT8              *Data2;
+  CPU_FEATURES_DATA  *CpuFeaturesData;
 
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
+  CpuFeaturesData = GetCpuFeaturesData ();
 
   Data1 = FeatureMask;
   Data2 = DependentBitMask;
-  for (Index = 0; Index < BitMaskSize; Index++) {
+  for (Index = 0; Index < CpuFeaturesData->BitMaskSize; Index++) {
     if (((*(Data1++)) & (*(Data2++))) != 0) {
       return TRUE;
     }
@@ -631,6 +610,7 @@ CheckCpuFeaturesDependency (
 /**
   Worker function to register CPU Feature.
 
+  @param[in]  CpuFeaturesData       Pointer to CPU feature data structure.
   @param[in]  CpuFeature            Pointer to CPU feature entry
 
   @retval  RETURN_SUCCESS           The CPU feature was successfully registered.
@@ -642,37 +622,21 @@ CheckCpuFeaturesDependency (
 **/
 RETURN_STATUS
 RegisterCpuFeatureWorker (
+  IN CPU_FEATURES_DATA       *CpuFeaturesData,
   IN CPU_FEATURES_ENTRY      *CpuFeature
   )
 {
   EFI_STATUS                 Status;
-  CPU_FEATURES_DATA          *CpuFeaturesData;
   CPU_FEATURES_ENTRY         *CpuFeatureEntry;
   LIST_ENTRY                 *Entry;
-  UINTN                      BitMaskSize;
   BOOLEAN                    FeatureExist;
 
-  BitMaskSize     = PcdGetSize (PcdCpuFeaturesSetting);
-  CpuFeaturesData = GetCpuFeaturesData ();
-  if (CpuFeaturesData->FeaturesCount == 0) {
-    InitializeListHead (&CpuFeaturesData->FeatureList);
-    InitializeSpinLock (&CpuFeaturesData->CpuFlags.MemoryMappedLock);
-    InitializeSpinLock (&CpuFeaturesData->CpuFlags.ConsoleLogLock);
-    //
-    // Driver has assumption that these three PCD should has same buffer size.
-    //
-    ASSERT (PcdGetSize (PcdCpuFeaturesSetting) == PcdGetSize (PcdCpuFeaturesCapability));
-    ASSERT (PcdGetSize (PcdCpuFeaturesSetting) == PcdGetSize (PcdCpuFeaturesSupport));
-    CpuFeaturesData->BitMaskSize = (UINT32) BitMaskSize;
-  }
-  ASSERT (CpuFeaturesData->BitMaskSize == BitMaskSize);
-
   FeatureExist = FALSE;
   CpuFeatureEntry = NULL;
   Entry = GetFirstNode (&CpuFeaturesData->FeatureList);
   while (!IsNull (&CpuFeaturesData->FeatureList, Entry)) {
     CpuFeatureEntry = CPU_FEATURE_ENTRY_FROM_LINK (Entry);
-    if (IsCpuFeatureMatch (CpuFeature->FeatureMask, CpuFeatureEntry->FeatureMask)) {
+    if (CompareMem (CpuFeature->FeatureMask, CpuFeatureEntry->FeatureMask, CpuFeaturesData->BitMaskSize) == 0) {
       //
       // If this feature already registered
       //
@@ -684,12 +648,12 @@ RegisterCpuFeatureWorker (
 
   if (!FeatureExist) {
     DEBUG ((DEBUG_INFO, "[NEW] "));
-    DumpCpuFeature (CpuFeature);
+    DumpCpuFeature (CpuFeature, CpuFeaturesData->BitMaskSize);
     InsertTailList (&CpuFeaturesData->FeatureList, &CpuFeature->Link);
     CpuFeaturesData->FeaturesCount++;
   } else {
     DEBUG ((DEBUG_INFO, "[OVERRIDE] "));
-    DumpCpuFeature (CpuFeature);
+    DumpCpuFeature (CpuFeature, CpuFeaturesData->BitMaskSize);
     ASSERT (CpuFeatureEntry != NULL);
     //
     // Overwrite original parameters of CPU feature
@@ -849,7 +813,6 @@ RegisterCpuFeature (
   EFI_STATUS                 Status;
   VA_LIST                    Marker;
   UINT32                     Feature;
-  UINTN                      BitMaskSize;
   CPU_FEATURES_ENTRY         *CpuFeature;
   UINT8                      *FeatureMask;
   UINT8                      *BeforeFeatureBitMask;
@@ -860,6 +823,7 @@ RegisterCpuFeature (
   UINT8                      *PackageAfterFeatureBitMask;
   BOOLEAN                    BeforeAll;
   BOOLEAN                    AfterAll;
+  CPU_FEATURES_DATA          *CpuFeaturesData;
 
   FeatureMask                 = NULL;
   BeforeFeatureBitMask        = NULL;
@@ -871,7 +835,18 @@ RegisterCpuFeature (
   BeforeAll            = FALSE;
   AfterAll             = FALSE;
 
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
+  CpuFeaturesData = GetCpuFeaturesData ();
+  if (CpuFeaturesData->FeaturesCount == 0) {
+    InitializeListHead (&CpuFeaturesData->FeatureList);
+    InitializeSpinLock (&CpuFeaturesData->CpuFlags.MemoryMappedLock);
+    InitializeSpinLock (&CpuFeaturesData->CpuFlags.ConsoleLogLock);
+    //
+    // Code assumes below three PCDs have PCD same buffer size.
+    //
+    ASSERT (PcdGetSize (PcdCpuFeaturesSetting) == PcdGetSize (PcdCpuFeaturesCapability));
+    ASSERT (PcdGetSize (PcdCpuFeaturesSetting) == PcdGetSize (PcdCpuFeaturesSupport));
+    CpuFeaturesData->BitMaskSize = (UINT32) PcdGetSize (PcdCpuFeaturesSetting);
+  }
 
   VA_START (Marker, InitializeFunc);
   Feature = VA_ARG (Marker, UINT32);
@@ -889,19 +864,19 @@ RegisterCpuFeature (
       AfterAll  = ((Feature & CPU_FEATURE_AFTER_ALL) != 0) ? TRUE : FALSE;
       Feature  &= ~(CPU_FEATURE_BEFORE_ALL | CPU_FEATURE_AFTER_ALL);
       ASSERT (FeatureMask == NULL);
-      SetCpuFeaturesBitMask (&FeatureMask, Feature, BitMaskSize);
+      SetCpuFeaturesBitMask (&FeatureMask, Feature, CpuFeaturesData->BitMaskSize);
     } else if ((Feature & CPU_FEATURE_BEFORE) != 0) {
-      SetCpuFeaturesBitMask (&BeforeFeatureBitMask, Feature & ~CPU_FEATURE_BEFORE, BitMaskSize);
+      SetCpuFeaturesBitMask (&BeforeFeatureBitMask, Feature & ~CPU_FEATURE_BEFORE, CpuFeaturesData->BitMaskSize);
     } else if ((Feature & CPU_FEATURE_AFTER) != 0) {
-      SetCpuFeaturesBitMask (&AfterFeatureBitMask, Feature & ~CPU_FEATURE_AFTER, BitMaskSize);
+      SetCpuFeaturesBitMask (&AfterFeatureBitMask, Feature & ~CPU_FEATURE_AFTER, CpuFeaturesData->BitMaskSize);
     } else if ((Feature & CPU_FEATURE_CORE_BEFORE) != 0) {
-      SetCpuFeaturesBitMask (&CoreBeforeFeatureBitMask, Feature & ~CPU_FEATURE_CORE_BEFORE, BitMaskSize);
+      SetCpuFeaturesBitMask (&CoreBeforeFeatureBitMask, Feature & ~CPU_FEATURE_CORE_BEFORE, CpuFeaturesData->BitMaskSize);
     } else if ((Feature & CPU_FEATURE_CORE_AFTER) != 0) {
-      SetCpuFeaturesBitMask (&CoreAfterFeatureBitMask, Feature & ~CPU_FEATURE_CORE_AFTER, BitMaskSize);
+      SetCpuFeaturesBitMask (&CoreAfterFeatureBitMask, Feature & ~CPU_FEATURE_CORE_AFTER, CpuFeaturesData->BitMaskSize);
     } else if ((Feature & CPU_FEATURE_PACKAGE_BEFORE) != 0) {
-      SetCpuFeaturesBitMask (&PackageBeforeFeatureBitMask, Feature & ~CPU_FEATURE_PACKAGE_BEFORE, BitMaskSize);
+      SetCpuFeaturesBitMask (&PackageBeforeFeatureBitMask, Feature & ~CPU_FEATURE_PACKAGE_BEFORE, CpuFeaturesData->BitMaskSize);
     } else if ((Feature & CPU_FEATURE_PACKAGE_AFTER) != 0) {
-      SetCpuFeaturesBitMask (&PackageAfterFeatureBitMask, Feature & ~CPU_FEATURE_PACKAGE_AFTER, BitMaskSize);
+      SetCpuFeaturesBitMask (&PackageAfterFeatureBitMask, Feature & ~CPU_FEATURE_PACKAGE_AFTER, CpuFeaturesData->BitMaskSize);
     }
     Feature = VA_ARG (Marker, UINT32);
   }
@@ -929,7 +904,7 @@ RegisterCpuFeature (
     ASSERT_EFI_ERROR (Status);
   }
 
-  Status = RegisterCpuFeatureWorker (CpuFeature);
+  Status = RegisterCpuFeatureWorker (CpuFeaturesData, CpuFeature);
   ASSERT_EFI_ERROR (Status);
 
   return RETURN_SUCCESS;
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Patch v3 2/2] UefiCpuPkg/RegisterCpuFeaturesLib: avoid use dynamic PCD.
  2019-07-17  1:59 ` [Patch v3 2/2] UefiCpuPkg/RegisterCpuFeaturesLib: avoid use dynamic PCD Dong, Eric
@ 2019-07-17  3:40   ` Zeng, Star
  2019-07-17  3:57     ` Ni, Ray
  0 siblings, 1 reply; 5+ messages in thread
From: Zeng, Star @ 2019-07-17  3:40 UTC (permalink / raw)
  To: Dong, Eric, devel@edk2.groups.io
  Cc: Ni, Ray, Laszlo Ersek, Kumar, Chandana C, Zeng, Star

Reviewed-by: Star Zeng <star.zeng@intel.com>

> -----Original Message-----
> From: Dong, Eric
> Sent: Wednesday, July 17, 2019 9:59 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo
> Ersek <lersek@redhat.com>; Kumar, Chandana C
> <chandana.c.kumar@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [Patch v3 2/2] UefiCpuPkg/RegisterCpuFeaturesLib: avoid use
> dynamic PCD.
> 
> From: "Dong, Eric" <eric.dong@intel.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1972
> 
> Function in this library may be used by APs. Assert will be trig if AP uses
> dynamic pcd.
> This patch enhance the current code, remove the unnecessary usage of
> dynamic PCD. This change try to avoid report this issue again later.
> 
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Chandana Kumar <chandana.c.kumar@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
>  .../CpuFeaturesInitialize.c                   |  64 +++++------
>  .../RegisterCpuFeatures.h                     |  10 +-
>  .../RegisterCpuFeaturesLib.c                  | 107 +++++++-----------
>  3 files changed, 79 insertions(+), 102 deletions(-)
> 
> diff --git
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> index 1e25ba8b32..33752c1a9f 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> @@ -15,20 +15,16 @@ CHAR16 *mRegisterTypeStr[] = {L"MSR", L"CR",
> L"MMIO", L"CACHE", L"SEMAP", L"INVA
>    Worker function to save PcdCpuFeaturesCapability.
> 
>    @param[in]  SupportedFeatureMask  The pointer to CPU feature bits mask
> buffer
> -  @param[in]  FeatureMaskSize       CPU feature bits mask buffer size.
> +  @param[in]  BitMaskSize           CPU feature bits mask buffer size.
> 
>  **/
>  VOID
>  SetCapabilityPcd (
>    IN UINT8               *SupportedFeatureMask,
> -  IN UINT32              FeatureMaskSize
> +  IN UINTN               BitMaskSize
>    )
>  {
>    EFI_STATUS             Status;
> -  UINTN                  BitMaskSize;
> -
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesCapability);
> -  ASSERT (FeatureMaskSize == BitMaskSize);
> 
>    Status = PcdSetPtrS (PcdCpuFeaturesCapability, &BitMaskSize,
> SupportedFeatureMask);
>    ASSERT_EFI_ERROR (Status);
> @@ -38,16 +34,16 @@ SetCapabilityPcd (
>    Worker function to save PcdCpuFeaturesSetting.
> 
>    @param[in]  SupportedFeatureMask  The pointer to CPU feature bits mask
> buffer
> +  @param[in]  BitMaskSize           CPU feature bits mask buffer size.
>  **/
>  VOID
>  SetSettingPcd (
> -  IN UINT8               *SupportedFeatureMask
> +  IN UINT8               *SupportedFeatureMask,
> +  IN UINTN               BitMaskSize
>    )
>  {
>    EFI_STATUS             Status;
> -  UINTN                  BitMaskSize;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
>    Status = PcdSetPtrS (PcdCpuFeaturesSetting, &BitMaskSize,
> SupportedFeatureMask);
>    ASSERT_EFI_ERROR (Status);
>  }
> @@ -272,19 +268,20 @@ SupportedMaskOr (
> 
>    @param[in]  SupportedFeatureMask  The pointer to CPU feature bits mask
> buffer
>    @param[in]  AndFeatureBitMask     The feature bit mask to do AND
> operation
> +  @param[in]  BitMaskSize           CPU feature bits mask buffer size.
> +
>  **/
>  VOID
>  SupportedMaskAnd (
>    IN       UINT8               *SupportedFeatureMask,
> -  IN CONST UINT8               *AndFeatureBitMask
> +  IN CONST UINT8               *AndFeatureBitMask,
> +  IN       UINT32              BitMaskSize
>    )
>  {
>    UINTN                  Index;
> -  UINTN                  BitMaskSize;
>    UINT8                  *Data1;
>    CONST UINT8            *Data2;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
>    Data1 = SupportedFeatureMask;
>    Data2 = AndFeatureBitMask;
>    for (Index = 0; Index < BitMaskSize; Index++) { @@ -297,19 +294,19 @@
> SupportedMaskAnd (
> 
>    @param[in]  SupportedFeatureMask  The pointer to CPU feature bits mask
> buffer
>    @param[in]  AndFeatureBitMask     The feature bit mask to do XOR
> operation
> +  @param[in]  BitMaskSize           CPU feature bits mask buffer size.
>  **/
>  VOID
>  SupportedMaskCleanBit (
>    IN UINT8               *SupportedFeatureMask,
> -  IN UINT8               *AndFeatureBitMask
> +  IN UINT8               *AndFeatureBitMask,
> +  IN UINT32              BitMaskSize
>    )
>  {
>    UINTN                  Index;
> -  UINTN                  BitMaskSize;
>    UINT8                  *Data1;
>    UINT8                  *Data2;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
>    Data1 = SupportedFeatureMask;
>    Data2 = AndFeatureBitMask;
>    for (Index = 0; Index < BitMaskSize; Index++) { @@ -323,6 +320,7 @@
> SupportedMaskCleanBit (
> 
>    @param[in]  SupportedFeatureMask   The pointer to CPU feature bits mask
> buffer
>    @param[in]  ComparedFeatureBitMask The feature bit mask to be
> compared
> +  @param[in]  BitMaskSize            CPU feature bits mask buffer size.
> 
>    @retval TRUE   The ComparedFeatureBitMask is set in CPU feature
> supported bits
>                   mask buffer.
> @@ -332,16 +330,14 @@ SupportedMaskCleanBit (  BOOLEAN
> IsBitMaskMatch (
>    IN UINT8               *SupportedFeatureMask,
> -  IN UINT8               *ComparedFeatureBitMask
> +  IN UINT8               *ComparedFeatureBitMask,
> +  IN UINT32              BitMaskSize
>    )
>  {
>    UINTN                  Index;
> -  UINTN                  BitMaskSize;
>    UINT8                  *Data1;
>    UINT8                  *Data2;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
> -
>    Data1 = SupportedFeatureMask;
>    Data2 = ComparedFeatureBitMask;
>    for (Index = 0; Index < BitMaskSize; Index++) { @@ -557,14 +553,14 @@
> AnalysisProcessorFeatures (
>      //
>      // Calculate the last capability on all processors
>      //
> -    SupportedMaskAnd (CpuFeaturesData->CapabilityPcd, CpuInitOrder-
> >FeaturesSupportedMask);
> +    SupportedMaskAnd (CpuFeaturesData->CapabilityPcd,
> + CpuInitOrder->FeaturesSupportedMask, CpuFeaturesData->BitMaskSize);
>    }
>    //
>    // Calculate the last setting
>    //
>    CpuFeaturesData->SettingPcd = AllocateCopyPool (CpuFeaturesData-
> >BitMaskSize, CpuFeaturesData->CapabilityPcd);
>    ASSERT (CpuFeaturesData->SettingPcd != NULL);
> -  SupportedMaskAnd (CpuFeaturesData->SettingPcd, PcdGetPtr
> (PcdCpuFeaturesSetting));
> +  SupportedMaskAnd (CpuFeaturesData->SettingPcd, PcdGetPtr
> + (PcdCpuFeaturesSetting), CpuFeaturesData->BitMaskSize);
> 
>    //
>    // Dump the last CPU feature list
> @@ -574,8 +570,8 @@ AnalysisProcessorFeatures (
>      Entry = GetFirstNode (&CpuFeaturesData->FeatureList);
>      while (!IsNull (&CpuFeaturesData->FeatureList, Entry)) {
>        CpuFeature = CPU_FEATURE_ENTRY_FROM_LINK (Entry);
> -      if (IsBitMaskMatch (CpuFeature->FeatureMask, CpuFeaturesData-
> >CapabilityPcd)) {
> -        if (IsBitMaskMatch (CpuFeature->FeatureMask, CpuFeaturesData-
> >SettingPcd)) {
> +      if (IsBitMaskMatch (CpuFeature->FeatureMask, CpuFeaturesData-
> >CapabilityPcd, CpuFeaturesData->BitMaskSize)) {
> +        if (IsBitMaskMatch (CpuFeature->FeatureMask,
> + CpuFeaturesData->SettingPcd, CpuFeaturesData->BitMaskSize)) {
>            DEBUG ((DEBUG_INFO, "[Enable   ] "));
>          } else {
>            DEBUG ((DEBUG_INFO, "[Disable  ] ")); @@ -583,22 +579,22 @@
> AnalysisProcessorFeatures (
>        } else {
>          DEBUG ((DEBUG_INFO, "[Unsupport] "));
>        }
> -      DumpCpuFeature (CpuFeature);
> +      DumpCpuFeature (CpuFeature, CpuFeaturesData->BitMaskSize);
>        Entry = Entry->ForwardLink;
>      }
>      DEBUG ((DEBUG_INFO, "PcdCpuFeaturesCapability:\n"));
> -    DumpCpuFeatureMask (CpuFeaturesData->CapabilityPcd);
> +    DumpCpuFeatureMask (CpuFeaturesData->CapabilityPcd,
> + CpuFeaturesData->BitMaskSize);
>      DEBUG ((DEBUG_INFO, "Origin PcdCpuFeaturesSetting:\n"));
> -    DumpCpuFeatureMask (PcdGetPtr (PcdCpuFeaturesSetting));
> +    DumpCpuFeatureMask (PcdGetPtr (PcdCpuFeaturesSetting),
> + CpuFeaturesData->BitMaskSize);
>      DEBUG ((DEBUG_INFO, "Final PcdCpuFeaturesSetting:\n"));
> -    DumpCpuFeatureMask (CpuFeaturesData->SettingPcd);
> +    DumpCpuFeatureMask (CpuFeaturesData->SettingPcd,
> + CpuFeaturesData->BitMaskSize);
>    );
> 
>    //
>    // Save PCDs and display CPU PCDs
>    //
>    SetCapabilityPcd (CpuFeaturesData->CapabilityPcd, CpuFeaturesData-
> >BitMaskSize);
> -  SetSettingPcd (CpuFeaturesData->SettingPcd);
> +  SetSettingPcd (CpuFeaturesData->SettingPcd,
> + CpuFeaturesData->BitMaskSize);
> 
>    for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> ProcessorNumber++) {
>      CpuInitOrder = &CpuFeaturesData->InitOrder[ProcessorNumber];
> @@ -608,7 +604,7 @@ AnalysisProcessorFeatures (
>        // Insert each feature into processor's order list
>        //
>        CpuFeature = CPU_FEATURE_ENTRY_FROM_LINK (Entry);
> -      if (IsBitMaskMatch (CpuFeature->FeatureMask, CpuFeaturesData-
> >CapabilityPcd)) {
> +      if (IsBitMaskMatch (CpuFeature->FeatureMask,
> + CpuFeaturesData->CapabilityPcd, CpuFeaturesData->BitMaskSize)) {
>          CpuFeatureInOrder = AllocateCopyPool (sizeof (CPU_FEATURES_ENTRY),
> CpuFeature);
>          ASSERT (CpuFeatureInOrder != NULL);
>          InsertTailList (&CpuInitOrder->OrderList, &CpuFeatureInOrder->Link);
> @@ -624,18 +620,18 @@ AnalysisProcessorFeatures (
>        CpuFeatureInOrder = CPU_FEATURE_ENTRY_FROM_LINK (Entry);
> 
>        Success = FALSE;
> -      if (IsBitMaskMatch (CpuFeatureInOrder->FeatureMask,
> CpuFeaturesData->SettingPcd)) {
> +      if (IsBitMaskMatch (CpuFeatureInOrder->FeatureMask,
> + CpuFeaturesData->SettingPcd, CpuFeaturesData->BitMaskSize)) {
>          Status = CpuFeatureInOrder->InitializeFunc (ProcessorNumber, CpuInfo,
> CpuFeatureInOrder->ConfigData, TRUE);
>          if (EFI_ERROR (Status)) {
>            //
>            // Clean the CpuFeatureInOrder->FeatureMask in setting PCD.
>            //
> -          SupportedMaskCleanBit (CpuFeaturesData->SettingPcd,
> CpuFeatureInOrder->FeatureMask);
> +          SupportedMaskCleanBit (CpuFeaturesData->SettingPcd,
> + CpuFeatureInOrder->FeatureMask, CpuFeaturesData->BitMaskSize);
>            if (CpuFeatureInOrder->FeatureName != NULL) {
>              DEBUG ((DEBUG_WARN, "Warning :: Failed to enable Feature: Name
> = %a.\n", CpuFeatureInOrder->FeatureName));
>            } else {
>              DEBUG ((DEBUG_WARN, "Warning :: Failed to enable Feature: Mask =
> "));
> -            DumpCpuFeatureMask (CpuFeatureInOrder->FeatureMask);
> +            DumpCpuFeatureMask (CpuFeatureInOrder->FeatureMask,
> + CpuFeaturesData->BitMaskSize);
>            }
>          } else {
>            Success = TRUE;
> @@ -647,7 +643,7 @@ AnalysisProcessorFeatures (
>              DEBUG ((DEBUG_WARN, "Warning :: Failed to disable Feature: Name
> = %a.\n", CpuFeatureInOrder->FeatureName));
>            } else {
>              DEBUG ((DEBUG_WARN, "Warning :: Failed to disable Feature: Mask =
> "));
> -            DumpCpuFeatureMask (CpuFeatureInOrder->FeatureMask);
> +            DumpCpuFeatureMask (CpuFeatureInOrder->FeatureMask,
> + CpuFeaturesData->BitMaskSize);
>            }
>          } else {
>            Success = TRUE;
> @@ -699,7 +695,7 @@ AnalysisProcessorFeatures (
>      // again during initialize the features.
>      //
>      DEBUG ((DEBUG_INFO, "Dump final value for
> PcdCpuFeaturesSetting:\n"));
> -    DumpCpuFeatureMask (CpuFeaturesData->SettingPcd);
> +    DumpCpuFeatureMask (CpuFeaturesData->SettingPcd,
> + CpuFeaturesData->BitMaskSize);
> 
>      //
>      // Dump the RegisterTable
> diff --git
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
> index 5c546ee153..a18f926641 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
> @@ -180,20 +180,26 @@ SwitchNewBsp (
>    Function that uses DEBUG() macros to display the contents of a a CPU
> feature bit mask.
> 
>    @param[in]  FeatureMask  A pointer to the CPU feature bit mask.
> +  @param[in]  BitMaskSize  CPU feature bits mask buffer size.
> +
>  **/
>  VOID
>  DumpCpuFeatureMask (
> -  IN UINT8               *FeatureMask
> +  IN UINT8               *FeatureMask,
> +  IN UINT32              BitMaskSize
>    );
> 
>  /**
>    Dump CPU feature name or CPU feature bit mask.
> 
>    @param[in]  CpuFeature   Pointer to CPU_FEATURES_ENTRY
> +  @param[in]  BitMaskSize  CPU feature bits mask buffer size.
> +
>  **/
>  VOID
>  DumpCpuFeature (
> -  IN CPU_FEATURES_ENTRY  *CpuFeature
> +  IN CPU_FEATURES_ENTRY  *CpuFeature,
> +  IN UINT32              BitMaskSize
>    );
> 
>  /**
> diff --git
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> index 36aabd7267..493566de5d 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> +++
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> @@ -8,46 +8,22 @@
> 
>  #include "RegisterCpuFeatures.h"
> 
> -/**
> -  Checks if two CPU feature bit masks are equal.
> -
> -  @param[in]  FirstFeatureMask  The first input CPU feature bit mask
> -  @param[in]  SecondFeatureMask The second input CPU feature bit mask
> -
> -  @retval TRUE  Two CPU feature bit masks are equal.
> -  @retval FALSE Two CPU feature bit masks are not equal.
> -**/
> -BOOLEAN
> -IsCpuFeatureMatch (
> -  IN UINT8               *FirstFeatureMask,
> -  IN UINT8               *SecondFeatureMask
> -  )
> -{
> -  UINTN                 BitMaskSize;
> -
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
> -  if (CompareMem (FirstFeatureMask, SecondFeatureMask, BitMaskSize) ==
> 0) {
> -    return TRUE;
> -  } else {
> -    return FALSE;
> -  }
> -}
> -
>  /**
>    Function that uses DEBUG() macros to display the contents of a a CPU
> feature bit mask.
> 
>    @param[in]  FeatureMask  A pointer to the CPU feature bit mask.
> +  @param[in]  BitMaskSize  CPU feature bits mask buffer size.
> +
>  **/
>  VOID
>  DumpCpuFeatureMask (
> -  IN UINT8               *FeatureMask
> +  IN UINT8               *FeatureMask,
> +  IN UINT32              BitMaskSize
>    )
>  {
>    UINTN                  Index;
>    UINT8                  *Data8;
> -  UINTN                  BitMaskSize;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
>    Data8       = (UINT8 *) FeatureMask;
>    for (Index = 0; Index < BitMaskSize; Index++) {
>      DEBUG ((DEBUG_INFO, " %02x ", *Data8++)); @@ -59,10 +35,13 @@
> DumpCpuFeatureMask (
>    Dump CPU feature name or CPU feature bit mask.
> 
>    @param[in]  CpuFeature   Pointer to CPU_FEATURES_ENTRY
> +  @param[in]  BitMaskSize  CPU feature bits mask buffer size.
> +
>  **/
>  VOID
>  DumpCpuFeature (
> -  IN CPU_FEATURES_ENTRY  *CpuFeature
> +  IN CPU_FEATURES_ENTRY  *CpuFeature,
> +  IN UINT32              BitMaskSize
>    )
>  {
> 
> @@ -70,7 +49,7 @@ DumpCpuFeature (
>      DEBUG ((DEBUG_INFO, "FeatureName: %a\n", CpuFeature-
> >FeatureName));
>    } else {
>      DEBUG ((DEBUG_INFO, "FeatureMask = "));
> -    DumpCpuFeatureMask (CpuFeature->FeatureMask);
> +    DumpCpuFeatureMask (CpuFeature->FeatureMask, BitMaskSize);
>    }
>  }
> 
> @@ -89,16 +68,16 @@ IsBitMaskMatchCheck (
>    IN UINT8        *DependentBitMask
>    )
>  {
> -  UINTN      Index;
> -  UINTN      BitMaskSize;
> -  UINT8      *Data1;
> -  UINT8      *Data2;
> +  UINTN              Index;
> +  UINT8              *Data1;
> +  UINT8              *Data2;
> +  CPU_FEATURES_DATA  *CpuFeaturesData;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
> +  CpuFeaturesData = GetCpuFeaturesData ();
> 
>    Data1 = FeatureMask;
>    Data2 = DependentBitMask;
> -  for (Index = 0; Index < BitMaskSize; Index++) {
> +  for (Index = 0; Index < CpuFeaturesData->BitMaskSize; Index++) {
>      if (((*(Data1++)) & (*(Data2++))) != 0) {
>        return TRUE;
>      }
> @@ -631,6 +610,7 @@ CheckCpuFeaturesDependency (
>  /**
>    Worker function to register CPU Feature.
> 
> +  @param[in]  CpuFeaturesData       Pointer to CPU feature data structure.
>    @param[in]  CpuFeature            Pointer to CPU feature entry
> 
>    @retval  RETURN_SUCCESS           The CPU feature was successfully
> registered.
> @@ -642,37 +622,21 @@ CheckCpuFeaturesDependency (  **/
> RETURN_STATUS  RegisterCpuFeatureWorker (
> +  IN CPU_FEATURES_DATA       *CpuFeaturesData,
>    IN CPU_FEATURES_ENTRY      *CpuFeature
>    )
>  {
>    EFI_STATUS                 Status;
> -  CPU_FEATURES_DATA          *CpuFeaturesData;
>    CPU_FEATURES_ENTRY         *CpuFeatureEntry;
>    LIST_ENTRY                 *Entry;
> -  UINTN                      BitMaskSize;
>    BOOLEAN                    FeatureExist;
> 
> -  BitMaskSize     = PcdGetSize (PcdCpuFeaturesSetting);
> -  CpuFeaturesData = GetCpuFeaturesData ();
> -  if (CpuFeaturesData->FeaturesCount == 0) {
> -    InitializeListHead (&CpuFeaturesData->FeatureList);
> -    InitializeSpinLock (&CpuFeaturesData->CpuFlags.MemoryMappedLock);
> -    InitializeSpinLock (&CpuFeaturesData->CpuFlags.ConsoleLogLock);
> -    //
> -    // Driver has assumption that these three PCD should has same buffer size.
> -    //
> -    ASSERT (PcdGetSize (PcdCpuFeaturesSetting) == PcdGetSize
> (PcdCpuFeaturesCapability));
> -    ASSERT (PcdGetSize (PcdCpuFeaturesSetting) == PcdGetSize
> (PcdCpuFeaturesSupport));
> -    CpuFeaturesData->BitMaskSize = (UINT32) BitMaskSize;
> -  }
> -  ASSERT (CpuFeaturesData->BitMaskSize == BitMaskSize);
> -
>    FeatureExist = FALSE;
>    CpuFeatureEntry = NULL;
>    Entry = GetFirstNode (&CpuFeaturesData->FeatureList);
>    while (!IsNull (&CpuFeaturesData->FeatureList, Entry)) {
>      CpuFeatureEntry = CPU_FEATURE_ENTRY_FROM_LINK (Entry);
> -    if (IsCpuFeatureMatch (CpuFeature->FeatureMask, CpuFeatureEntry-
> >FeatureMask)) {
> +    if (CompareMem (CpuFeature->FeatureMask,
> + CpuFeatureEntry->FeatureMask, CpuFeaturesData->BitMaskSize) == 0) {
>        //
>        // If this feature already registered
>        //
> @@ -684,12 +648,12 @@ RegisterCpuFeatureWorker (
> 
>    if (!FeatureExist) {
>      DEBUG ((DEBUG_INFO, "[NEW] "));
> -    DumpCpuFeature (CpuFeature);
> +    DumpCpuFeature (CpuFeature, CpuFeaturesData->BitMaskSize);
>      InsertTailList (&CpuFeaturesData->FeatureList, &CpuFeature->Link);
>      CpuFeaturesData->FeaturesCount++;
>    } else {
>      DEBUG ((DEBUG_INFO, "[OVERRIDE] "));
> -    DumpCpuFeature (CpuFeature);
> +    DumpCpuFeature (CpuFeature, CpuFeaturesData->BitMaskSize);
>      ASSERT (CpuFeatureEntry != NULL);
>      //
>      // Overwrite original parameters of CPU feature @@ -849,7 +813,6 @@
> RegisterCpuFeature (
>    EFI_STATUS                 Status;
>    VA_LIST                    Marker;
>    UINT32                     Feature;
> -  UINTN                      BitMaskSize;
>    CPU_FEATURES_ENTRY         *CpuFeature;
>    UINT8                      *FeatureMask;
>    UINT8                      *BeforeFeatureBitMask;
> @@ -860,6 +823,7 @@ RegisterCpuFeature (
>    UINT8                      *PackageAfterFeatureBitMask;
>    BOOLEAN                    BeforeAll;
>    BOOLEAN                    AfterAll;
> +  CPU_FEATURES_DATA          *CpuFeaturesData;
> 
>    FeatureMask                 = NULL;
>    BeforeFeatureBitMask        = NULL;
> @@ -871,7 +835,18 @@ RegisterCpuFeature (
>    BeforeAll            = FALSE;
>    AfterAll             = FALSE;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
> +  CpuFeaturesData = GetCpuFeaturesData ();  if
> + (CpuFeaturesData->FeaturesCount == 0) {
> +    InitializeListHead (&CpuFeaturesData->FeatureList);
> +    InitializeSpinLock (&CpuFeaturesData->CpuFlags.MemoryMappedLock);
> +    InitializeSpinLock (&CpuFeaturesData->CpuFlags.ConsoleLogLock);
> +    //
> +    // Code assumes below three PCDs have PCD same buffer size.
> +    //
> +    ASSERT (PcdGetSize (PcdCpuFeaturesSetting) == PcdGetSize
> (PcdCpuFeaturesCapability));
> +    ASSERT (PcdGetSize (PcdCpuFeaturesSetting) == PcdGetSize
> (PcdCpuFeaturesSupport));
> +    CpuFeaturesData->BitMaskSize = (UINT32) PcdGetSize
> + (PcdCpuFeaturesSetting);  }
> 
>    VA_START (Marker, InitializeFunc);
>    Feature = VA_ARG (Marker, UINT32);
> @@ -889,19 +864,19 @@ RegisterCpuFeature (
>        AfterAll  = ((Feature & CPU_FEATURE_AFTER_ALL) != 0) ? TRUE : FALSE;
>        Feature  &= ~(CPU_FEATURE_BEFORE_ALL | CPU_FEATURE_AFTER_ALL);
>        ASSERT (FeatureMask == NULL);
> -      SetCpuFeaturesBitMask (&FeatureMask, Feature, BitMaskSize);
> +      SetCpuFeaturesBitMask (&FeatureMask, Feature,
> + CpuFeaturesData->BitMaskSize);
>      } else if ((Feature & CPU_FEATURE_BEFORE) != 0) {
> -      SetCpuFeaturesBitMask (&BeforeFeatureBitMask, Feature &
> ~CPU_FEATURE_BEFORE, BitMaskSize);
> +      SetCpuFeaturesBitMask (&BeforeFeatureBitMask, Feature &
> + ~CPU_FEATURE_BEFORE, CpuFeaturesData->BitMaskSize);
>      } else if ((Feature & CPU_FEATURE_AFTER) != 0) {
> -      SetCpuFeaturesBitMask (&AfterFeatureBitMask, Feature &
> ~CPU_FEATURE_AFTER, BitMaskSize);
> +      SetCpuFeaturesBitMask (&AfterFeatureBitMask, Feature &
> + ~CPU_FEATURE_AFTER, CpuFeaturesData->BitMaskSize);
>      } else if ((Feature & CPU_FEATURE_CORE_BEFORE) != 0) {
> -      SetCpuFeaturesBitMask (&CoreBeforeFeatureBitMask, Feature &
> ~CPU_FEATURE_CORE_BEFORE, BitMaskSize);
> +      SetCpuFeaturesBitMask (&CoreBeforeFeatureBitMask, Feature &
> + ~CPU_FEATURE_CORE_BEFORE, CpuFeaturesData->BitMaskSize);
>      } else if ((Feature & CPU_FEATURE_CORE_AFTER) != 0) {
> -      SetCpuFeaturesBitMask (&CoreAfterFeatureBitMask, Feature &
> ~CPU_FEATURE_CORE_AFTER, BitMaskSize);
> +      SetCpuFeaturesBitMask (&CoreAfterFeatureBitMask, Feature &
> + ~CPU_FEATURE_CORE_AFTER, CpuFeaturesData->BitMaskSize);
>      } else if ((Feature & CPU_FEATURE_PACKAGE_BEFORE) != 0) {
> -      SetCpuFeaturesBitMask (&PackageBeforeFeatureBitMask, Feature &
> ~CPU_FEATURE_PACKAGE_BEFORE, BitMaskSize);
> +      SetCpuFeaturesBitMask (&PackageBeforeFeatureBitMask, Feature &
> + ~CPU_FEATURE_PACKAGE_BEFORE, CpuFeaturesData->BitMaskSize);
>      } else if ((Feature & CPU_FEATURE_PACKAGE_AFTER) != 0) {
> -      SetCpuFeaturesBitMask (&PackageAfterFeatureBitMask, Feature &
> ~CPU_FEATURE_PACKAGE_AFTER, BitMaskSize);
> +      SetCpuFeaturesBitMask (&PackageAfterFeatureBitMask, Feature &
> + ~CPU_FEATURE_PACKAGE_AFTER, CpuFeaturesData->BitMaskSize);
>      }
>      Feature = VA_ARG (Marker, UINT32);
>    }
> @@ -929,7 +904,7 @@ RegisterCpuFeature (
>      ASSERT_EFI_ERROR (Status);
>    }
> 
> -  Status = RegisterCpuFeatureWorker (CpuFeature);
> +  Status = RegisterCpuFeatureWorker (CpuFeaturesData, CpuFeature);
>    ASSERT_EFI_ERROR (Status);
> 
>    return RETURN_SUCCESS;
> --
> 2.21.0.windows.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Patch v3 2/2] UefiCpuPkg/RegisterCpuFeaturesLib: avoid use dynamic PCD.
  2019-07-17  3:40   ` Zeng, Star
@ 2019-07-17  3:57     ` Ni, Ray
  0 siblings, 0 replies; 5+ messages in thread
From: Ni, Ray @ 2019-07-17  3:57 UTC (permalink / raw)
  To: Zeng, Star, Dong, Eric, devel@edk2.groups.io
  Cc: Laszlo Ersek, Kumar, Chandana C

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Zeng, Star
> Sent: Wednesday, July 17, 2019 11:40 AM
> To: Dong, Eric <eric.dong@intel.com>; devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kumar,
> Chandana C <chandana.c.kumar@intel.com>; Zeng, Star
> <star.zeng@intel.com>
> Subject: RE: [Patch v3 2/2] UefiCpuPkg/RegisterCpuFeaturesLib: avoid use
> dynamic PCD.
> 
> Reviewed-by: Star Zeng <star.zeng@intel.com>
> 
> > -----Original Message-----
> > From: Dong, Eric
> > Sent: Wednesday, July 17, 2019 9:59 AM
> > To: devel@edk2.groups.io
> > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>;
> > Laszlo Ersek <lersek@redhat.com>; Kumar, Chandana C
> > <chandana.c.kumar@intel.com>; Zeng, Star <star.zeng@intel.com>
> > Subject: [Patch v3 2/2] UefiCpuPkg/RegisterCpuFeaturesLib: avoid use
> > dynamic PCD.
> >
> > From: "Dong, Eric" <eric.dong@intel.com>
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1972
> >
> > Function in this library may be used by APs. Assert will be trig if AP
> > uses dynamic pcd.
> > This patch enhance the current code, remove the unnecessary usage of
> > dynamic PCD. This change try to avoid report this issue again later.
> >
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Chandana Kumar <chandana.c.kumar@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Signed-off-by: Eric Dong <eric.dong@intel.com>
> > ---
> >  .../CpuFeaturesInitialize.c                   |  64 +++++------
> >  .../RegisterCpuFeatures.h                     |  10 +-
> >  .../RegisterCpuFeaturesLib.c                  | 107 +++++++-----------
> >  3 files changed, 79 insertions(+), 102 deletions(-)
> >
> > diff --git
> > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> > b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> > index 1e25ba8b32..33752c1a9f 100644
> > ---
> > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> > +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.
> > +++ c
> > @@ -15,20 +15,16 @@ CHAR16 *mRegisterTypeStr[] = {L"MSR", L"CR",
> > L"MMIO", L"CACHE", L"SEMAP", L"INVA
> >    Worker function to save PcdCpuFeaturesCapability.
> >
> >    @param[in]  SupportedFeatureMask  The pointer to CPU feature bits
> > mask buffer
> > -  @param[in]  FeatureMaskSize       CPU feature bits mask buffer size.
> > +  @param[in]  BitMaskSize           CPU feature bits mask buffer size.
> >
> >  **/
> >  VOID
> >  SetCapabilityPcd (
> >    IN UINT8               *SupportedFeatureMask,
> > -  IN UINT32              FeatureMaskSize
> > +  IN UINTN               BitMaskSize
> >    )
> >  {
> >    EFI_STATUS             Status;
> > -  UINTN                  BitMaskSize;
> > -
> > -  BitMaskSize = PcdGetSize (PcdCpuFeaturesCapability);
> > -  ASSERT (FeatureMaskSize == BitMaskSize);
> >
> >    Status = PcdSetPtrS (PcdCpuFeaturesCapability, &BitMaskSize,
> > SupportedFeatureMask);
> >    ASSERT_EFI_ERROR (Status);
> > @@ -38,16 +34,16 @@ SetCapabilityPcd (
> >    Worker function to save PcdCpuFeaturesSetting.
> >
> >    @param[in]  SupportedFeatureMask  The pointer to CPU feature bits
> > mask buffer
> > +  @param[in]  BitMaskSize           CPU feature bits mask buffer size.
> >  **/
> >  VOID
> >  SetSettingPcd (
> > -  IN UINT8               *SupportedFeatureMask
> > +  IN UINT8               *SupportedFeatureMask,
> > +  IN UINTN               BitMaskSize
> >    )
> >  {
> >    EFI_STATUS             Status;
> > -  UINTN                  BitMaskSize;
> >
> > -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
> >    Status = PcdSetPtrS (PcdCpuFeaturesSetting, &BitMaskSize,
> > SupportedFeatureMask);
> >    ASSERT_EFI_ERROR (Status);
> >  }
> > @@ -272,19 +268,20 @@ SupportedMaskOr (
> >
> >    @param[in]  SupportedFeatureMask  The pointer to CPU feature bits
> > mask buffer
> >    @param[in]  AndFeatureBitMask     The feature bit mask to do AND
> > operation
> > +  @param[in]  BitMaskSize           CPU feature bits mask buffer size.
> > +
> >  **/
> >  VOID
> >  SupportedMaskAnd (
> >    IN       UINT8               *SupportedFeatureMask,
> > -  IN CONST UINT8               *AndFeatureBitMask
> > +  IN CONST UINT8               *AndFeatureBitMask,
> > +  IN       UINT32              BitMaskSize
> >    )
> >  {
> >    UINTN                  Index;
> > -  UINTN                  BitMaskSize;
> >    UINT8                  *Data1;
> >    CONST UINT8            *Data2;
> >
> > -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
> >    Data1 = SupportedFeatureMask;
> >    Data2 = AndFeatureBitMask;
> >    for (Index = 0; Index < BitMaskSize; Index++) { @@ -297,19 +294,19
> > @@ SupportedMaskAnd (
> >
> >    @param[in]  SupportedFeatureMask  The pointer to CPU feature bits
> > mask buffer
> >    @param[in]  AndFeatureBitMask     The feature bit mask to do XOR
> > operation
> > +  @param[in]  BitMaskSize           CPU feature bits mask buffer size.
> >  **/
> >  VOID
> >  SupportedMaskCleanBit (
> >    IN UINT8               *SupportedFeatureMask,
> > -  IN UINT8               *AndFeatureBitMask
> > +  IN UINT8               *AndFeatureBitMask,
> > +  IN UINT32              BitMaskSize
> >    )
> >  {
> >    UINTN                  Index;
> > -  UINTN                  BitMaskSize;
> >    UINT8                  *Data1;
> >    UINT8                  *Data2;
> >
> > -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
> >    Data1 = SupportedFeatureMask;
> >    Data2 = AndFeatureBitMask;
> >    for (Index = 0; Index < BitMaskSize; Index++) { @@ -323,6 +320,7 @@
> > SupportedMaskCleanBit (
> >
> >    @param[in]  SupportedFeatureMask   The pointer to CPU feature bits
> mask
> > buffer
> >    @param[in]  ComparedFeatureBitMask The feature bit mask to be
> > compared
> > +  @param[in]  BitMaskSize            CPU feature bits mask buffer size.
> >
> >    @retval TRUE   The ComparedFeatureBitMask is set in CPU feature
> > supported bits
> >                   mask buffer.
> > @@ -332,16 +330,14 @@ SupportedMaskCleanBit (  BOOLEAN
> IsBitMaskMatch
> > (
> >    IN UINT8               *SupportedFeatureMask,
> > -  IN UINT8               *ComparedFeatureBitMask
> > +  IN UINT8               *ComparedFeatureBitMask,
> > +  IN UINT32              BitMaskSize
> >    )
> >  {
> >    UINTN                  Index;
> > -  UINTN                  BitMaskSize;
> >    UINT8                  *Data1;
> >    UINT8                  *Data2;
> >
> > -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
> > -
> >    Data1 = SupportedFeatureMask;
> >    Data2 = ComparedFeatureBitMask;
> >    for (Index = 0; Index < BitMaskSize; Index++) { @@ -557,14 +553,14
> > @@ AnalysisProcessorFeatures (
> >      //
> >      // Calculate the last capability on all processors
> >      //
> > -    SupportedMaskAnd (CpuFeaturesData->CapabilityPcd, CpuInitOrder-
> > >FeaturesSupportedMask);
> > +    SupportedMaskAnd (CpuFeaturesData->CapabilityPcd,
> > + CpuInitOrder->FeaturesSupportedMask, CpuFeaturesData-
> >BitMaskSize);
> >    }
> >    //
> >    // Calculate the last setting
> >    //
> >    CpuFeaturesData->SettingPcd = AllocateCopyPool (CpuFeaturesData-
> > >BitMaskSize, CpuFeaturesData->CapabilityPcd);
> >    ASSERT (CpuFeaturesData->SettingPcd != NULL);
> > -  SupportedMaskAnd (CpuFeaturesData->SettingPcd, PcdGetPtr
> > (PcdCpuFeaturesSetting));
> > +  SupportedMaskAnd (CpuFeaturesData->SettingPcd, PcdGetPtr
> > + (PcdCpuFeaturesSetting), CpuFeaturesData->BitMaskSize);
> >
> >    //
> >    // Dump the last CPU feature list
> > @@ -574,8 +570,8 @@ AnalysisProcessorFeatures (
> >      Entry = GetFirstNode (&CpuFeaturesData->FeatureList);
> >      while (!IsNull (&CpuFeaturesData->FeatureList, Entry)) {
> >        CpuFeature = CPU_FEATURE_ENTRY_FROM_LINK (Entry);
> > -      if (IsBitMaskMatch (CpuFeature->FeatureMask, CpuFeaturesData-
> > >CapabilityPcd)) {
> > -        if (IsBitMaskMatch (CpuFeature->FeatureMask, CpuFeaturesData-
> > >SettingPcd)) {
> > +      if (IsBitMaskMatch (CpuFeature->FeatureMask, CpuFeaturesData-
> > >CapabilityPcd, CpuFeaturesData->BitMaskSize)) {
> > +        if (IsBitMaskMatch (CpuFeature->FeatureMask,
> > + CpuFeaturesData->SettingPcd, CpuFeaturesData->BitMaskSize)) {
> >            DEBUG ((DEBUG_INFO, "[Enable   ] "));
> >          } else {
> >            DEBUG ((DEBUG_INFO, "[Disable  ] ")); @@ -583,22 +579,22 @@
> > AnalysisProcessorFeatures (
> >        } else {
> >          DEBUG ((DEBUG_INFO, "[Unsupport] "));
> >        }
> > -      DumpCpuFeature (CpuFeature);
> > +      DumpCpuFeature (CpuFeature, CpuFeaturesData->BitMaskSize);
> >        Entry = Entry->ForwardLink;
> >      }
> >      DEBUG ((DEBUG_INFO, "PcdCpuFeaturesCapability:\n"));
> > -    DumpCpuFeatureMask (CpuFeaturesData->CapabilityPcd);
> > +    DumpCpuFeatureMask (CpuFeaturesData->CapabilityPcd,
> > + CpuFeaturesData->BitMaskSize);
> >      DEBUG ((DEBUG_INFO, "Origin PcdCpuFeaturesSetting:\n"));
> > -    DumpCpuFeatureMask (PcdGetPtr (PcdCpuFeaturesSetting));
> > +    DumpCpuFeatureMask (PcdGetPtr (PcdCpuFeaturesSetting),
> > + CpuFeaturesData->BitMaskSize);
> >      DEBUG ((DEBUG_INFO, "Final PcdCpuFeaturesSetting:\n"));
> > -    DumpCpuFeatureMask (CpuFeaturesData->SettingPcd);
> > +    DumpCpuFeatureMask (CpuFeaturesData->SettingPcd,
> > + CpuFeaturesData->BitMaskSize);
> >    );
> >
> >    //
> >    // Save PCDs and display CPU PCDs
> >    //
> >    SetCapabilityPcd (CpuFeaturesData->CapabilityPcd, CpuFeaturesData-
> > >BitMaskSize);
> > -  SetSettingPcd (CpuFeaturesData->SettingPcd);
> > +  SetSettingPcd (CpuFeaturesData->SettingPcd,
> > + CpuFeaturesData->BitMaskSize);
> >
> >    for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> > ProcessorNumber++) {
> >      CpuInitOrder = &CpuFeaturesData->InitOrder[ProcessorNumber];
> > @@ -608,7 +604,7 @@ AnalysisProcessorFeatures (
> >        // Insert each feature into processor's order list
> >        //
> >        CpuFeature = CPU_FEATURE_ENTRY_FROM_LINK (Entry);
> > -      if (IsBitMaskMatch (CpuFeature->FeatureMask, CpuFeaturesData-
> > >CapabilityPcd)) {
> > +      if (IsBitMaskMatch (CpuFeature->FeatureMask,
> > + CpuFeaturesData->CapabilityPcd, CpuFeaturesData->BitMaskSize)) {
> >          CpuFeatureInOrder = AllocateCopyPool (sizeof
> > (CPU_FEATURES_ENTRY), CpuFeature);
> >          ASSERT (CpuFeatureInOrder != NULL);
> >          InsertTailList (&CpuInitOrder->OrderList,
> > &CpuFeatureInOrder->Link); @@ -624,18 +620,18 @@
> AnalysisProcessorFeatures (
> >        CpuFeatureInOrder = CPU_FEATURE_ENTRY_FROM_LINK (Entry);
> >
> >        Success = FALSE;
> > -      if (IsBitMaskMatch (CpuFeatureInOrder->FeatureMask,
> > CpuFeaturesData->SettingPcd)) {
> > +      if (IsBitMaskMatch (CpuFeatureInOrder->FeatureMask,
> > + CpuFeaturesData->SettingPcd, CpuFeaturesData->BitMaskSize)) {
> >          Status = CpuFeatureInOrder->InitializeFunc (ProcessorNumber,
> > CpuInfo,
> > CpuFeatureInOrder->ConfigData, TRUE);
> >          if (EFI_ERROR (Status)) {
> >            //
> >            // Clean the CpuFeatureInOrder->FeatureMask in setting PCD.
> >            //
> > -          SupportedMaskCleanBit (CpuFeaturesData->SettingPcd,
> > CpuFeatureInOrder->FeatureMask);
> > +          SupportedMaskCleanBit (CpuFeaturesData->SettingPcd,
> > + CpuFeatureInOrder->FeatureMask, CpuFeaturesData->BitMaskSize);
> >            if (CpuFeatureInOrder->FeatureName != NULL) {
> >              DEBUG ((DEBUG_WARN, "Warning :: Failed to enable Feature:
> > Name = %a.\n", CpuFeatureInOrder->FeatureName));
> >            } else {
> >              DEBUG ((DEBUG_WARN, "Warning :: Failed to enable Feature:
> > Mask = "));
> > -            DumpCpuFeatureMask (CpuFeatureInOrder->FeatureMask);
> > +            DumpCpuFeatureMask (CpuFeatureInOrder->FeatureMask,
> > + CpuFeaturesData->BitMaskSize);
> >            }
> >          } else {
> >            Success = TRUE;
> > @@ -647,7 +643,7 @@ AnalysisProcessorFeatures (
> >              DEBUG ((DEBUG_WARN, "Warning :: Failed to disable
> > Feature: Name = %a.\n", CpuFeatureInOrder->FeatureName));
> >            } else {
> >              DEBUG ((DEBUG_WARN, "Warning :: Failed to disable
> > Feature: Mask = "));
> > -            DumpCpuFeatureMask (CpuFeatureInOrder->FeatureMask);
> > +            DumpCpuFeatureMask (CpuFeatureInOrder->FeatureMask,
> > + CpuFeaturesData->BitMaskSize);
> >            }
> >          } else {
> >            Success = TRUE;
> > @@ -699,7 +695,7 @@ AnalysisProcessorFeatures (
> >      // again during initialize the features.
> >      //
> >      DEBUG ((DEBUG_INFO, "Dump final value for
> > PcdCpuFeaturesSetting:\n"));
> > -    DumpCpuFeatureMask (CpuFeaturesData->SettingPcd);
> > +    DumpCpuFeatureMask (CpuFeaturesData->SettingPcd,
> > + CpuFeaturesData->BitMaskSize);
> >
> >      //
> >      // Dump the RegisterTable
> > diff --git
> > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
> > b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
> > index 5c546ee153..a18f926641 100644
> > --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
> > +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
> > @@ -180,20 +180,26 @@ SwitchNewBsp (
> >    Function that uses DEBUG() macros to display the contents of a a
> > CPU feature bit mask.
> >
> >    @param[in]  FeatureMask  A pointer to the CPU feature bit mask.
> > +  @param[in]  BitMaskSize  CPU feature bits mask buffer size.
> > +
> >  **/
> >  VOID
> >  DumpCpuFeatureMask (
> > -  IN UINT8               *FeatureMask
> > +  IN UINT8               *FeatureMask,
> > +  IN UINT32              BitMaskSize
> >    );
> >
> >  /**
> >    Dump CPU feature name or CPU feature bit mask.
> >
> >    @param[in]  CpuFeature   Pointer to CPU_FEATURES_ENTRY
> > +  @param[in]  BitMaskSize  CPU feature bits mask buffer size.
> > +
> >  **/
> >  VOID
> >  DumpCpuFeature (
> > -  IN CPU_FEATURES_ENTRY  *CpuFeature
> > +  IN CPU_FEATURES_ENTRY  *CpuFeature,
> > +  IN UINT32              BitMaskSize
> >    );
> >
> >  /**
> > diff --git
> > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> > b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> > index 36aabd7267..493566de5d 100644
> > ---
> > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> > +++
> > b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> > @@ -8,46 +8,22 @@
> >
> >  #include "RegisterCpuFeatures.h"
> >
> > -/**
> > -  Checks if two CPU feature bit masks are equal.
> > -
> > -  @param[in]  FirstFeatureMask  The first input CPU feature bit mask
> > -  @param[in]  SecondFeatureMask The second input CPU feature bit mask
> > -
> > -  @retval TRUE  Two CPU feature bit masks are equal.
> > -  @retval FALSE Two CPU feature bit masks are not equal.
> > -**/
> > -BOOLEAN
> > -IsCpuFeatureMatch (
> > -  IN UINT8               *FirstFeatureMask,
> > -  IN UINT8               *SecondFeatureMask
> > -  )
> > -{
> > -  UINTN                 BitMaskSize;
> > -
> > -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
> > -  if (CompareMem (FirstFeatureMask, SecondFeatureMask, BitMaskSize)
> > ==
> > 0) {
> > -    return TRUE;
> > -  } else {
> > -    return FALSE;
> > -  }
> > -}
> > -
> >  /**
> >    Function that uses DEBUG() macros to display the contents of a a
> > CPU feature bit mask.
> >
> >    @param[in]  FeatureMask  A pointer to the CPU feature bit mask.
> > +  @param[in]  BitMaskSize  CPU feature bits mask buffer size.
> > +
> >  **/
> >  VOID
> >  DumpCpuFeatureMask (
> > -  IN UINT8               *FeatureMask
> > +  IN UINT8               *FeatureMask,
> > +  IN UINT32              BitMaskSize
> >    )
> >  {
> >    UINTN                  Index;
> >    UINT8                  *Data8;
> > -  UINTN                  BitMaskSize;
> >
> > -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
> >    Data8       = (UINT8 *) FeatureMask;
> >    for (Index = 0; Index < BitMaskSize; Index++) {
> >      DEBUG ((DEBUG_INFO, " %02x ", *Data8++)); @@ -59,10 +35,13 @@
> > DumpCpuFeatureMask (
> >    Dump CPU feature name or CPU feature bit mask.
> >
> >    @param[in]  CpuFeature   Pointer to CPU_FEATURES_ENTRY
> > +  @param[in]  BitMaskSize  CPU feature bits mask buffer size.
> > +
> >  **/
> >  VOID
> >  DumpCpuFeature (
> > -  IN CPU_FEATURES_ENTRY  *CpuFeature
> > +  IN CPU_FEATURES_ENTRY  *CpuFeature,
> > +  IN UINT32              BitMaskSize
> >    )
> >  {
> >
> > @@ -70,7 +49,7 @@ DumpCpuFeature (
> >      DEBUG ((DEBUG_INFO, "FeatureName: %a\n", CpuFeature-
> > >FeatureName));
> >    } else {
> >      DEBUG ((DEBUG_INFO, "FeatureMask = "));
> > -    DumpCpuFeatureMask (CpuFeature->FeatureMask);
> > +    DumpCpuFeatureMask (CpuFeature->FeatureMask, BitMaskSize);
> >    }
> >  }
> >
> > @@ -89,16 +68,16 @@ IsBitMaskMatchCheck (
> >    IN UINT8        *DependentBitMask
> >    )
> >  {
> > -  UINTN      Index;
> > -  UINTN      BitMaskSize;
> > -  UINT8      *Data1;
> > -  UINT8      *Data2;
> > +  UINTN              Index;
> > +  UINT8              *Data1;
> > +  UINT8              *Data2;
> > +  CPU_FEATURES_DATA  *CpuFeaturesData;
> >
> > -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
> > +  CpuFeaturesData = GetCpuFeaturesData ();
> >
> >    Data1 = FeatureMask;
> >    Data2 = DependentBitMask;
> > -  for (Index = 0; Index < BitMaskSize; Index++) {
> > +  for (Index = 0; Index < CpuFeaturesData->BitMaskSize; Index++) {
> >      if (((*(Data1++)) & (*(Data2++))) != 0) {
> >        return TRUE;
> >      }
> > @@ -631,6 +610,7 @@ CheckCpuFeaturesDependency (
> >  /**
> >    Worker function to register CPU Feature.
> >
> > +  @param[in]  CpuFeaturesData       Pointer to CPU feature data structure.
> >    @param[in]  CpuFeature            Pointer to CPU feature entry
> >
> >    @retval  RETURN_SUCCESS           The CPU feature was successfully
> > registered.
> > @@ -642,37 +622,21 @@ CheckCpuFeaturesDependency (  **/
> RETURN_STATUS
> > RegisterCpuFeatureWorker (
> > +  IN CPU_FEATURES_DATA       *CpuFeaturesData,
> >    IN CPU_FEATURES_ENTRY      *CpuFeature
> >    )
> >  {
> >    EFI_STATUS                 Status;
> > -  CPU_FEATURES_DATA          *CpuFeaturesData;
> >    CPU_FEATURES_ENTRY         *CpuFeatureEntry;
> >    LIST_ENTRY                 *Entry;
> > -  UINTN                      BitMaskSize;
> >    BOOLEAN                    FeatureExist;
> >
> > -  BitMaskSize     = PcdGetSize (PcdCpuFeaturesSetting);
> > -  CpuFeaturesData = GetCpuFeaturesData ();
> > -  if (CpuFeaturesData->FeaturesCount == 0) {
> > -    InitializeListHead (&CpuFeaturesData->FeatureList);
> > -    InitializeSpinLock (&CpuFeaturesData->CpuFlags.MemoryMappedLock);
> > -    InitializeSpinLock (&CpuFeaturesData->CpuFlags.ConsoleLogLock);
> > -    //
> > -    // Driver has assumption that these three PCD should has same buffer
> size.
> > -    //
> > -    ASSERT (PcdGetSize (PcdCpuFeaturesSetting) == PcdGetSize
> > (PcdCpuFeaturesCapability));
> > -    ASSERT (PcdGetSize (PcdCpuFeaturesSetting) == PcdGetSize
> > (PcdCpuFeaturesSupport));
> > -    CpuFeaturesData->BitMaskSize = (UINT32) BitMaskSize;
> > -  }
> > -  ASSERT (CpuFeaturesData->BitMaskSize == BitMaskSize);
> > -
> >    FeatureExist = FALSE;
> >    CpuFeatureEntry = NULL;
> >    Entry = GetFirstNode (&CpuFeaturesData->FeatureList);
> >    while (!IsNull (&CpuFeaturesData->FeatureList, Entry)) {
> >      CpuFeatureEntry = CPU_FEATURE_ENTRY_FROM_LINK (Entry);
> > -    if (IsCpuFeatureMatch (CpuFeature->FeatureMask, CpuFeatureEntry-
> > >FeatureMask)) {
> > +    if (CompareMem (CpuFeature->FeatureMask,
> > + CpuFeatureEntry->FeatureMask, CpuFeaturesData->BitMaskSize) == 0) {
> >        //
> >        // If this feature already registered
> >        //
> > @@ -684,12 +648,12 @@ RegisterCpuFeatureWorker (
> >
> >    if (!FeatureExist) {
> >      DEBUG ((DEBUG_INFO, "[NEW] "));
> > -    DumpCpuFeature (CpuFeature);
> > +    DumpCpuFeature (CpuFeature, CpuFeaturesData->BitMaskSize);
> >      InsertTailList (&CpuFeaturesData->FeatureList, &CpuFeature->Link);
> >      CpuFeaturesData->FeaturesCount++;
> >    } else {
> >      DEBUG ((DEBUG_INFO, "[OVERRIDE] "));
> > -    DumpCpuFeature (CpuFeature);
> > +    DumpCpuFeature (CpuFeature, CpuFeaturesData->BitMaskSize);
> >      ASSERT (CpuFeatureEntry != NULL);
> >      //
> >      // Overwrite original parameters of CPU feature @@ -849,7 +813,6
> > @@ RegisterCpuFeature (
> >    EFI_STATUS                 Status;
> >    VA_LIST                    Marker;
> >    UINT32                     Feature;
> > -  UINTN                      BitMaskSize;
> >    CPU_FEATURES_ENTRY         *CpuFeature;
> >    UINT8                      *FeatureMask;
> >    UINT8                      *BeforeFeatureBitMask;
> > @@ -860,6 +823,7 @@ RegisterCpuFeature (
> >    UINT8                      *PackageAfterFeatureBitMask;
> >    BOOLEAN                    BeforeAll;
> >    BOOLEAN                    AfterAll;
> > +  CPU_FEATURES_DATA          *CpuFeaturesData;
> >
> >    FeatureMask                 = NULL;
> >    BeforeFeatureBitMask        = NULL;
> > @@ -871,7 +835,18 @@ RegisterCpuFeature (
> >    BeforeAll            = FALSE;
> >    AfterAll             = FALSE;
> >
> > -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
> > +  CpuFeaturesData = GetCpuFeaturesData ();  if
> > + (CpuFeaturesData->FeaturesCount == 0) {
> > +    InitializeListHead (&CpuFeaturesData->FeatureList);
> > +    InitializeSpinLock (&CpuFeaturesData->CpuFlags.MemoryMappedLock);
> > +    InitializeSpinLock (&CpuFeaturesData->CpuFlags.ConsoleLogLock);
> > +    //
> > +    // Code assumes below three PCDs have PCD same buffer size.
> > +    //
> > +    ASSERT (PcdGetSize (PcdCpuFeaturesSetting) == PcdGetSize
> > (PcdCpuFeaturesCapability));
> > +    ASSERT (PcdGetSize (PcdCpuFeaturesSetting) == PcdGetSize
> > (PcdCpuFeaturesSupport));
> > +    CpuFeaturesData->BitMaskSize = (UINT32) PcdGetSize
> > + (PcdCpuFeaturesSetting);  }
> >
> >    VA_START (Marker, InitializeFunc);
> >    Feature = VA_ARG (Marker, UINT32);
> > @@ -889,19 +864,19 @@ RegisterCpuFeature (
> >        AfterAll  = ((Feature & CPU_FEATURE_AFTER_ALL) != 0) ? TRUE : FALSE;
> >        Feature  &= ~(CPU_FEATURE_BEFORE_ALL |
> CPU_FEATURE_AFTER_ALL);
> >        ASSERT (FeatureMask == NULL);
> > -      SetCpuFeaturesBitMask (&FeatureMask, Feature, BitMaskSize);
> > +      SetCpuFeaturesBitMask (&FeatureMask, Feature,
> > + CpuFeaturesData->BitMaskSize);
> >      } else if ((Feature & CPU_FEATURE_BEFORE) != 0) {
> > -      SetCpuFeaturesBitMask (&BeforeFeatureBitMask, Feature &
> > ~CPU_FEATURE_BEFORE, BitMaskSize);
> > +      SetCpuFeaturesBitMask (&BeforeFeatureBitMask, Feature &
> > + ~CPU_FEATURE_BEFORE, CpuFeaturesData->BitMaskSize);
> >      } else if ((Feature & CPU_FEATURE_AFTER) != 0) {
> > -      SetCpuFeaturesBitMask (&AfterFeatureBitMask, Feature &
> > ~CPU_FEATURE_AFTER, BitMaskSize);
> > +      SetCpuFeaturesBitMask (&AfterFeatureBitMask, Feature &
> > + ~CPU_FEATURE_AFTER, CpuFeaturesData->BitMaskSize);
> >      } else if ((Feature & CPU_FEATURE_CORE_BEFORE) != 0) {
> > -      SetCpuFeaturesBitMask (&CoreBeforeFeatureBitMask, Feature &
> > ~CPU_FEATURE_CORE_BEFORE, BitMaskSize);
> > +      SetCpuFeaturesBitMask (&CoreBeforeFeatureBitMask, Feature &
> > + ~CPU_FEATURE_CORE_BEFORE, CpuFeaturesData->BitMaskSize);
> >      } else if ((Feature & CPU_FEATURE_CORE_AFTER) != 0) {
> > -      SetCpuFeaturesBitMask (&CoreAfterFeatureBitMask, Feature &
> > ~CPU_FEATURE_CORE_AFTER, BitMaskSize);
> > +      SetCpuFeaturesBitMask (&CoreAfterFeatureBitMask, Feature &
> > + ~CPU_FEATURE_CORE_AFTER, CpuFeaturesData->BitMaskSize);
> >      } else if ((Feature & CPU_FEATURE_PACKAGE_BEFORE) != 0) {
> > -      SetCpuFeaturesBitMask (&PackageBeforeFeatureBitMask, Feature &
> > ~CPU_FEATURE_PACKAGE_BEFORE, BitMaskSize);
> > +      SetCpuFeaturesBitMask (&PackageBeforeFeatureBitMask, Feature &
> > + ~CPU_FEATURE_PACKAGE_BEFORE, CpuFeaturesData->BitMaskSize);
> >      } else if ((Feature & CPU_FEATURE_PACKAGE_AFTER) != 0) {
> > -      SetCpuFeaturesBitMask (&PackageAfterFeatureBitMask, Feature &
> > ~CPU_FEATURE_PACKAGE_AFTER, BitMaskSize);
> > +      SetCpuFeaturesBitMask (&PackageAfterFeatureBitMask, Feature &
> > + ~CPU_FEATURE_PACKAGE_AFTER, CpuFeaturesData->BitMaskSize);
> >      }
> >      Feature = VA_ARG (Marker, UINT32);
> >    }
> > @@ -929,7 +904,7 @@ RegisterCpuFeature (
> >      ASSERT_EFI_ERROR (Status);
> >    }
> >
> > -  Status = RegisterCpuFeatureWorker (CpuFeature);
> > +  Status = RegisterCpuFeatureWorker (CpuFeaturesData, CpuFeature);
> >    ASSERT_EFI_ERROR (Status);
> >
> >    return RETURN_SUCCESS;
> > --
> > 2.21.0.windows.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-07-17  3:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-07-17  1:58 [Patch v3 0/2] UefiCpuPkg/RegisterCpuFeaturesLib: Avoid AP calls PeiServices table Dong, Eric
2019-07-17  1:58 ` [Patch v3 1/2] " Dong, Eric
2019-07-17  1:59 ` [Patch v3 2/2] UefiCpuPkg/RegisterCpuFeaturesLib: avoid use dynamic PCD Dong, Eric
2019-07-17  3:40   ` Zeng, Star
2019-07-17  3:57     ` Ni, Ray

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox