[PATCH v2 0/3] UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit.

[PATCH v2 0/3] UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit.

[Ray Ni]

Ray Ni (3):
  UefiCpuPkg/RegisterCpuFeaturesLib: Delete CPU_FEATURE_[BEFORE|AFTER]
  UefiCpuPkg/RegisterCpuFeaturesLib: Rename [Before|After]FeatureBitMask
  UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit.

 .../Include/Library/RegisterCpuFeaturesLib.h  | 49 +++++++++---
 .../CpuCommonFeaturesLib.c                    |  6 +-
 .../CpuFeaturesInitialize.c                   | 74 +++++++++++++++++++
 .../RegisterCpuFeatures.h                     |  4 +-
 .../RegisterCpuFeaturesLib.c                  | 74 ++++++++++---------
 5 files changed, 158 insertions(+), 49 deletions(-)

-- 
2.21.0.windows.1

[PATCH v2 1/3] UefiCpuPkg/RegisterCpuFeaturesLib: Delete CPU_FEATURE_[BEFORE|AFTER]

[Ray Ni]

From: Ray Ni <ray.ni@intel.com>

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

Commit b3c71b472dff2c02f0cc38d7a1959cfb2ba8420d supported MSR setting
in different scopes. It added below macro:
 CPU_FEATURE_THREAD_BEFORE
 CPU_FEATURE_THREAD_AFTER
 CPU_FEATURE_CORE_BEFORE
 CPU_FEATURE_CORE_AFTER
 CPU_FEATURE_PACKAGE_BEFORE
 CPU_FEATURE_PACKAGE_AFTER

And it re-interpreted CPU_FEATURE_BEFORE as CPU_FEATURE_THREAD_BEFORE
and CPU_FEATURE_AFTER as CPU_FEATURE_THREAD_AFTER.

This patch retires CPU_FEATURE_BEFORE and CPU_FEATURE_AFTER
completely.

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
---
 UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h | 13 ++-----------
 .../CpuCommonFeaturesLib/CpuCommonFeaturesLib.c     |  6 +++---
 .../RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c | 10 +++++-----
 3 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
index f370373d63..d075606cdb 100644
--- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
+++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
@@ -69,17 +69,8 @@
 
 #define CPU_FEATURE_BEFORE_ALL                      BIT23
 #define CPU_FEATURE_AFTER_ALL                       BIT24
-//
-// CPU_FEATURE_BEFORE and CPU_FEATURE_AFTER only mean Thread scope
-// before and Thread scope after.
-// It will be replace with CPU_FEATURE_THREAD_BEFORE and
-// CPU_FEATURE_THREAD_AFTER, and should not be used anymore.
-//
-#define CPU_FEATURE_BEFORE                          BIT25
-#define CPU_FEATURE_AFTER                           BIT26
-
-#define CPU_FEATURE_THREAD_BEFORE                   CPU_FEATURE_BEFORE
-#define CPU_FEATURE_THREAD_AFTER                    CPU_FEATURE_AFTER
+#define CPU_FEATURE_THREAD_BEFORE                   BIT25
+#define CPU_FEATURE_THREAD_AFTER                    BIT26
 #define CPU_FEATURE_CORE_BEFORE                     BIT27
 #define CPU_FEATURE_CORE_AFTER                      BIT28
 #define CPU_FEATURE_PACKAGE_BEFORE                  BIT29
diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c
index 3ebd9392a9..d1fe14f519 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c
@@ -95,7 +95,7 @@ CpuCommonFeaturesLibConstructor (
                SmxSupport,
                SmxInitialize,
                CPU_FEATURE_SMX,
-               CPU_FEATURE_LOCK_FEATURE_CONTROL_REGISTER | CPU_FEATURE_BEFORE,
+               CPU_FEATURE_LOCK_FEATURE_CONTROL_REGISTER | CPU_FEATURE_THREAD_BEFORE,
                CPU_FEATURE_END
                );
     ASSERT_EFI_ERROR (Status);
@@ -107,7 +107,7 @@ CpuCommonFeaturesLibConstructor (
                VmxSupport,
                VmxInitialize,
                CPU_FEATURE_VMX,
-               CPU_FEATURE_LOCK_FEATURE_CONTROL_REGISTER | CPU_FEATURE_BEFORE,
+               CPU_FEATURE_LOCK_FEATURE_CONTROL_REGISTER | CPU_FEATURE_THREAD_BEFORE,
                CPU_FEATURE_END
                );
     ASSERT_EFI_ERROR (Status);
@@ -207,7 +207,7 @@ CpuCommonFeaturesLibConstructor (
                LmceSupport,
                LmceInitialize,
                CPU_FEATURE_LMCE,
-               CPU_FEATURE_LOCK_FEATURE_CONTROL_REGISTER | CPU_FEATURE_BEFORE,
+               CPU_FEATURE_LOCK_FEATURE_CONTROL_REGISTER | CPU_FEATURE_THREAD_BEFORE,
                CPU_FEATURE_END
                );
     ASSERT_EFI_ERROR (Status);
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
index 58910b8891..1f953832b9 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
@@ -858,16 +858,16 @@ RegisterCpuFeature (
                     != (CPU_FEATURE_CORE_BEFORE | CPU_FEATURE_CORE_AFTER));
     ASSERT ((Feature & (CPU_FEATURE_PACKAGE_BEFORE | CPU_FEATURE_PACKAGE_AFTER))
                     != (CPU_FEATURE_PACKAGE_BEFORE | CPU_FEATURE_PACKAGE_AFTER));
-    if (Feature < CPU_FEATURE_BEFORE) {
+    if (Feature < CPU_FEATURE_THREAD_BEFORE) {
       BeforeAll = ((Feature & CPU_FEATURE_BEFORE_ALL) != 0) ? TRUE : FALSE;
       AfterAll  = ((Feature & CPU_FEATURE_AFTER_ALL) != 0) ? TRUE : FALSE;
       Feature  &= ~(CPU_FEATURE_BEFORE_ALL | CPU_FEATURE_AFTER_ALL);
       ASSERT (FeatureMask == NULL);
       SetCpuFeaturesBitMask (&FeatureMask, Feature, CpuFeaturesData->BitMaskSize);
-    } else if ((Feature & CPU_FEATURE_BEFORE) != 0) {
-      SetCpuFeaturesBitMask (&BeforeFeatureBitMask, Feature & ~CPU_FEATURE_BEFORE, CpuFeaturesData->BitMaskSize);
-    } else if ((Feature & CPU_FEATURE_AFTER) != 0) {
-      SetCpuFeaturesBitMask (&AfterFeatureBitMask, Feature & ~CPU_FEATURE_AFTER, CpuFeaturesData->BitMaskSize);
+    } else if ((Feature & CPU_FEATURE_THREAD_BEFORE) != 0) {
+      SetCpuFeaturesBitMask (&BeforeFeatureBitMask, Feature & ~CPU_FEATURE_THREAD_BEFORE, CpuFeaturesData->BitMaskSize);
+    } else if ((Feature & CPU_FEATURE_THREAD_AFTER) != 0) {
+      SetCpuFeaturesBitMask (&AfterFeatureBitMask, Feature & ~CPU_FEATURE_THREAD_AFTER, CpuFeaturesData->BitMaskSize);
     } else if ((Feature & CPU_FEATURE_CORE_BEFORE) != 0) {
       SetCpuFeaturesBitMask (&CoreBeforeFeatureBitMask, Feature & ~CPU_FEATURE_CORE_BEFORE, CpuFeaturesData->BitMaskSize);
     } else if ((Feature & CPU_FEATURE_CORE_AFTER) != 0) {
-- 
2.21.0.windows.1

[PATCH v2 2/3] UefiCpuPkg/RegisterCpuFeaturesLib: Rename [Before|After]FeatureBitMask

[Ray Ni]

From: Ray Ni <ray.ni@intel.com>

The patch doesn't have any functionality impact.

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
---
 .../RegisterCpuFeatures.h                     |  4 +-
 .../RegisterCpuFeaturesLib.c                  | 68 +++++++++++--------
 2 files changed, 40 insertions(+), 32 deletions(-)

diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
index 7c48b0a645..4780f792d9 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
@@ -43,8 +43,8 @@ typedef struct {
   CPU_FEATURE_GET_CONFIG_DATA  GetConfigDataFunc;
   CPU_FEATURE_SUPPORT          SupportFunc;
   CPU_FEATURE_INITIALIZE       InitializeFunc;
-  UINT8                        *BeforeFeatureBitMask;
-  UINT8                        *AfterFeatureBitMask;
+  UINT8                        *ThreadBeforeFeatureBitMask;
+  UINT8                        *ThreadAfterFeatureBitMask;
   UINT8                        *CoreBeforeFeatureBitMask;
   UINT8                        *CoreAfterFeatureBitMask;
   UINT8                        *PackageBeforeFeatureBitMask;
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
index 1f953832b9..3d18b5916f 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
@@ -194,8 +194,8 @@ DetectFeatureScope (
       return CoreDepType;
     }
 
-    if ((CpuFeature->BeforeFeatureBitMask != NULL) &&
-        IsBitMaskMatchCheck (NextCpuFeatureMask, CpuFeature->BeforeFeatureBitMask)) {
+    if ((CpuFeature->ThreadBeforeFeatureBitMask != NULL) &&
+        IsBitMaskMatchCheck (NextCpuFeatureMask, CpuFeature->ThreadBeforeFeatureBitMask)) {
       return ThreadDepType;
     }
 
@@ -212,8 +212,8 @@ DetectFeatureScope (
     return CoreDepType;
   }
 
-  if ((CpuFeature->AfterFeatureBitMask != NULL) &&
-      IsBitMaskMatchCheck (NextCpuFeatureMask, CpuFeature->AfterFeatureBitMask)) {
+  if ((CpuFeature->ThreadAfterFeatureBitMask != NULL) &&
+      IsBitMaskMatchCheck (NextCpuFeatureMask, CpuFeature->ThreadAfterFeatureBitMask)) {
     return ThreadDepType;
   }
 
@@ -247,8 +247,8 @@ DetectNoneNeighborhoodFeatureScope (
       return CoreDepType;
     }
 
-    if ((CpuFeature->BeforeFeatureBitMask != NULL) &&
-        FindSpecifyFeature(FeatureList, &CpuFeature->Link, FALSE, CpuFeature->BeforeFeatureBitMask)) {
+    if ((CpuFeature->ThreadBeforeFeatureBitMask != NULL) &&
+        FindSpecifyFeature(FeatureList, &CpuFeature->Link, FALSE, CpuFeature->ThreadBeforeFeatureBitMask)) {
       return ThreadDepType;
     }
 
@@ -265,8 +265,8 @@ DetectNoneNeighborhoodFeatureScope (
     return CoreDepType;
   }
 
-  if ((CpuFeature->AfterFeatureBitMask != NULL) &&
-      FindSpecifyFeature(FeatureList, &CpuFeature->Link, TRUE, CpuFeature->AfterFeatureBitMask)) {
+  if ((CpuFeature->ThreadAfterFeatureBitMask != NULL) &&
+      FindSpecifyFeature(FeatureList, &CpuFeature->Link, TRUE, CpuFeature->ThreadAfterFeatureBitMask)) {
     return ThreadDepType;
   }
 
@@ -561,15 +561,15 @@ CheckCpuFeaturesDependency (
       }
     }
 
-    if (CpuFeature->BeforeFeatureBitMask != NULL) {
-      Swapped = InsertToBeforeEntry (FeatureList, CurrentEntry, CpuFeature->BeforeFeatureBitMask);
+    if (CpuFeature->ThreadBeforeFeatureBitMask != NULL) {
+      Swapped = InsertToBeforeEntry (FeatureList, CurrentEntry, CpuFeature->ThreadBeforeFeatureBitMask);
       if (Swapped) {
         continue;
       }
     }
 
-    if (CpuFeature->AfterFeatureBitMask != NULL) {
-      Swapped = InsertToAfterEntry (FeatureList, CurrentEntry, CpuFeature->AfterFeatureBitMask);
+    if (CpuFeature->ThreadAfterFeatureBitMask != NULL) {
+      Swapped = InsertToAfterEntry (FeatureList, CurrentEntry, CpuFeature->ThreadAfterFeatureBitMask);
       if (Swapped) {
         continue;
       }
@@ -676,17 +676,17 @@ RegisterCpuFeatureWorker (
       ASSERT_EFI_ERROR (Status);
       FreePool (CpuFeature->FeatureName);
     }
-    if (CpuFeature->BeforeFeatureBitMask != NULL) {
-      if (CpuFeatureEntry->BeforeFeatureBitMask != NULL) {
-        FreePool (CpuFeatureEntry->BeforeFeatureBitMask);
+    if (CpuFeature->ThreadBeforeFeatureBitMask != NULL) {
+      if (CpuFeatureEntry->ThreadBeforeFeatureBitMask != NULL) {
+        FreePool (CpuFeatureEntry->ThreadBeforeFeatureBitMask);
       }
-      CpuFeatureEntry->BeforeFeatureBitMask = CpuFeature->BeforeFeatureBitMask;
+      CpuFeatureEntry->ThreadBeforeFeatureBitMask = CpuFeature->ThreadBeforeFeatureBitMask;
     }
-    if (CpuFeature->AfterFeatureBitMask != NULL) {
-      if (CpuFeatureEntry->AfterFeatureBitMask != NULL) {
-        FreePool (CpuFeatureEntry->AfterFeatureBitMask);
+    if (CpuFeature->ThreadAfterFeatureBitMask != NULL) {
+      if (CpuFeatureEntry->ThreadAfterFeatureBitMask != NULL) {
+        FreePool (CpuFeatureEntry->ThreadAfterFeatureBitMask);
       }
-      CpuFeatureEntry->AfterFeatureBitMask = CpuFeature->AfterFeatureBitMask;
+      CpuFeatureEntry->ThreadAfterFeatureBitMask = CpuFeature->ThreadAfterFeatureBitMask;
     }
     if (CpuFeature->CoreBeforeFeatureBitMask != NULL) {
       if (CpuFeatureEntry->CoreBeforeFeatureBitMask != NULL) {
@@ -815,8 +815,8 @@ RegisterCpuFeature (
   UINT32                     Feature;
   CPU_FEATURES_ENTRY         *CpuFeature;
   UINT8                      *FeatureMask;
-  UINT8                      *BeforeFeatureBitMask;
-  UINT8                      *AfterFeatureBitMask;
+  UINT8                      *ThreadBeforeFeatureBitMask;
+  UINT8                      *ThreadAfterFeatureBitMask;
   UINT8                      *CoreBeforeFeatureBitMask;
   UINT8                      *CoreAfterFeatureBitMask;
   UINT8                      *PackageBeforeFeatureBitMask;
@@ -826,8 +826,8 @@ RegisterCpuFeature (
   CPU_FEATURES_DATA          *CpuFeaturesData;
 
   FeatureMask                 = NULL;
-  BeforeFeatureBitMask        = NULL;
-  AfterFeatureBitMask         = NULL;
+  ThreadBeforeFeatureBitMask  = NULL;
+  ThreadAfterFeatureBitMask   = NULL;
   CoreBeforeFeatureBitMask    = NULL;
   CoreAfterFeatureBitMask     = NULL;
   PackageBeforeFeatureBitMask = NULL;
@@ -850,10 +850,18 @@ RegisterCpuFeature (
   VA_START (Marker, InitializeFunc);
   Feature = VA_ARG (Marker, UINT32);
   while (Feature != CPU_FEATURE_END) {
-    ASSERT ((Feature & (CPU_FEATURE_BEFORE | CPU_FEATURE_AFTER))
-                    != (CPU_FEATURE_BEFORE | CPU_FEATURE_AFTER));
+    //
+    // It's invalid to require a feature is before AND after all other features.
+    //
     ASSERT ((Feature & (CPU_FEATURE_BEFORE_ALL | CPU_FEATURE_AFTER_ALL))
                     != (CPU_FEATURE_BEFORE_ALL | CPU_FEATURE_AFTER_ALL));
+
+    //
+    // It's invalid to require feature A is before AND after before feature B,
+    // either in thread level, core level or package level.
+    //
+    ASSERT ((Feature & (CPU_FEATURE_THREAD_BEFORE | CPU_FEATURE_THREAD_AFTER))
+                    != (CPU_FEATURE_THREAD_BEFORE | CPU_FEATURE_THREAD_AFTER));
     ASSERT ((Feature & (CPU_FEATURE_CORE_BEFORE | CPU_FEATURE_CORE_AFTER))
                     != (CPU_FEATURE_CORE_BEFORE | CPU_FEATURE_CORE_AFTER));
     ASSERT ((Feature & (CPU_FEATURE_PACKAGE_BEFORE | CPU_FEATURE_PACKAGE_AFTER))
@@ -865,9 +873,9 @@ RegisterCpuFeature (
       ASSERT (FeatureMask == NULL);
       SetCpuFeaturesBitMask (&FeatureMask, Feature, CpuFeaturesData->BitMaskSize);
     } else if ((Feature & CPU_FEATURE_THREAD_BEFORE) != 0) {
-      SetCpuFeaturesBitMask (&BeforeFeatureBitMask, Feature & ~CPU_FEATURE_THREAD_BEFORE, CpuFeaturesData->BitMaskSize);
+      SetCpuFeaturesBitMask (&ThreadBeforeFeatureBitMask, Feature & ~CPU_FEATURE_THREAD_BEFORE, CpuFeaturesData->BitMaskSize);
     } else if ((Feature & CPU_FEATURE_THREAD_AFTER) != 0) {
-      SetCpuFeaturesBitMask (&AfterFeatureBitMask, Feature & ~CPU_FEATURE_THREAD_AFTER, CpuFeaturesData->BitMaskSize);
+      SetCpuFeaturesBitMask (&ThreadAfterFeatureBitMask, Feature & ~CPU_FEATURE_THREAD_AFTER, CpuFeaturesData->BitMaskSize);
     } else if ((Feature & CPU_FEATURE_CORE_BEFORE) != 0) {
       SetCpuFeaturesBitMask (&CoreBeforeFeatureBitMask, Feature & ~CPU_FEATURE_CORE_BEFORE, CpuFeaturesData->BitMaskSize);
     } else if ((Feature & CPU_FEATURE_CORE_AFTER) != 0) {
@@ -885,8 +893,8 @@ RegisterCpuFeature (
   ASSERT (CpuFeature != NULL);
   CpuFeature->Signature                   = CPU_FEATURE_ENTRY_SIGNATURE;
   CpuFeature->FeatureMask                 = FeatureMask;
-  CpuFeature->BeforeFeatureBitMask        = BeforeFeatureBitMask;
-  CpuFeature->AfterFeatureBitMask         = AfterFeatureBitMask;
+  CpuFeature->ThreadBeforeFeatureBitMask  = ThreadBeforeFeatureBitMask;
+  CpuFeature->ThreadAfterFeatureBitMask   = ThreadAfterFeatureBitMask;
   CpuFeature->CoreBeforeFeatureBitMask    = CoreBeforeFeatureBitMask;
   CpuFeature->CoreAfterFeatureBitMask     = CoreAfterFeatureBitMask;
   CpuFeature->PackageBeforeFeatureBitMask = PackageBeforeFeatureBitMask;
-- 
2.21.0.windows.1

[PATCH v2 3/3] UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit.

[Ray Ni]

From: Ray Ni <ray.ni@intel.com>

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

The flow of CPU feature initialization logic is:
1. BSP calls GetConfigDataFunc() for each thread/AP;
2. Each thread/AP calls SupportFunc() to detect its own capability;
3. BSP calls InitializeFunc() for each thread/AP.

There is a design gap in step #3. For a package scope feature that only
requires one thread of each package does the initialization operation,
what InitializeFunc() currently does is to do the initialization
operation only CPU physical location Core# is 0.
But in certain platform, Core#0 might be disabled in hardware level
which results the certain package scope feature isn't initialized at
all.

The patch adds a new field Fist to indicate the CPU's location in
its parent scope.
First.Package is set for all APs/threads under first package;
First.Core is set for all APs/threads under first core of each
package;
First.Thread is set for the AP/thread of each core.

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
---
 .../Include/Library/RegisterCpuFeaturesLib.h  | 36 +++++++++
 .../CpuFeaturesInitialize.c                   | 74 +++++++++++++++++++
 2 files changed, 110 insertions(+)

diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
index d075606cdb..7114c8ce89 100644
--- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
+++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
@@ -78,6 +78,37 @@
 #define CPU_FEATURE_END                             MAX_UINT32
 /// @}
 
+///
+/// The bit field to indicate whether the processor is the first in its parent scope.
+///
+typedef struct {
+  //
+  // Set to 1 when current processor is the first thread in the core it resides in.
+  //
+  UINT32 Thread   : 1;
+  //
+  // Set to 1 when current processor is a thread of the first core in the module it resides in.
+  //
+  UINT32 Core     : 1;
+  //
+  // Set to 1 when current processor is a thread of the first module in the tile it resides in.
+  //
+  UINT32 Module   : 1;
+  //
+  // Set to 1 when current processor is a thread of the first tile in the die it resides in.
+  //
+  UINT32 Tile     : 1;
+  //
+  // Set to 1 when current processor is a thread of the first die in the package it resides in.
+  //
+  UINT32 Die      : 1;
+  //
+  // Set to 1 when current processor is a thread of the first package in the system.
+  //
+  UINT32 Package  : 1;
+  UINT32 Reserved : 26;
+} REGISTER_CPU_FEATURE_FIRST_PROCESSOR;
+
 ///
 /// CPU Information passed into the SupportFunc and InitializeFunc of the
 /// RegisterCpuFeature() library function.  This structure contains information
@@ -88,6 +119,11 @@ typedef struct {
   /// The package that the CPU resides
   ///
   EFI_PROCESSOR_INFORMATION            ProcessorInfo;
+
+  ///
+  /// The bit flag indicating whether the CPU is the first Thread/Core/Module/Tile/Die/Package in its parent scope.
+  ///
+  REGISTER_CPU_FEATURE_FIRST_PROCESSOR First;
   ///
   /// The Display Family of the CPU computed from CPUID leaf CPUID_VERSION_INFO
   ///
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index 0a4fcff033..23076fd453 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -105,6 +105,9 @@ CpuInitDataInitialize (
   EFI_CPU_PHYSICAL_LOCATION            *Location;
   BOOLEAN                              *CoresVisited;
   UINTN                                Index;
+  UINT32                               PackageIndex;
+  UINT32                               CoreIndex;
+  UINT32                               First;
   ACPI_CPU_DATA                        *AcpiCpuData;
   CPU_STATUS_INFORMATION               *CpuStatus;
   UINT32                               *ValidCoreCountPerPackage;
@@ -234,6 +237,77 @@ CpuInitDataInitialize (
   ASSERT (CpuFeaturesData->CpuFlags.CoreSemaphoreCount != NULL);
   CpuFeaturesData->CpuFlags.PackageSemaphoreCount = AllocateZeroPool (sizeof (UINT32) * CpuStatus->PackageCount * CpuStatus->MaxCoreCount * CpuStatus->MaxThreadCount);
   ASSERT (CpuFeaturesData->CpuFlags.PackageSemaphoreCount != NULL);
+
+  //
+  // Initialize CpuFeaturesData->InitOrder[].CpuInfo.First
+  //
+
+  //
+  // Set First.Package for each thread belonging to the first package.
+  //
+  First = MAX_UINT32;
+  for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+    Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+    First = MIN (Location->Package, First);
+  }
+  for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+    Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+    if (Location->Package == First) {
+      CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Package = 1;
+    }
+  }
+
+  //
+  // Set First.Die/Tile/Module for each thread assuming:
+  //  single Die under each package, single Tile under each Die, single Module under each Tile
+  //
+  for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+    CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Die = 1;
+    CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Tile = 1;
+    CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Module = 1;
+  }
+
+  for (PackageIndex = 0; PackageIndex < CpuStatus->PackageCount; PackageIndex++) {
+    //
+    // Set First.Core for each thread in the first core of each package.
+    //
+    First = MAX_UINT32;
+    for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+      Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+      if (Location->Package == PackageIndex) {
+        First = MIN (Location->Core, First);
+      }
+    }
+
+    for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+      Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+      if (Location->Package == PackageIndex && Location->Core == First) {
+        CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Core = 1;
+      }
+    }
+  }
+
+  for (PackageIndex = 0; PackageIndex < CpuStatus->PackageCount; PackageIndex++) {
+    for (CoreIndex = 0; CoreIndex < CpuStatus->MaxCoreCount; CoreIndex++) {
+      //
+      // Set First.Thread for the first thread of each core.
+      //
+      First = MAX_UINT32;
+      for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+        Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+        if (Location->Package == PackageIndex && Location->Core == CoreIndex) {
+          First = MIN (Location->Thread, First);
+        }
+      }
+
+      for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+        Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+        if (Location->Package == PackageIndex && Location->Core == CoreIndex && Location->Thread == First) {
+          CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Thread = 1;
+        }
+      }
+    }
+  }
 }
 
 /**
-- 
2.21.0.windows.1

Re: [PATCH v2 1/3] UefiCpuPkg/RegisterCpuFeaturesLib: Delete CPU_FEATURE_[BEFORE|AFTER]

[Zeng, Star]

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

-----Original Message-----
From: Ray Ni [mailto:niruiyu@users.noreply.github.com] 
Sent: Tuesday, November 26, 2019 2:16 PM
To: devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: [PATCH v2 1/3] UefiCpuPkg/RegisterCpuFeaturesLib: Delete CPU_FEATURE_[BEFORE|AFTER]

From: Ray Ni <ray.ni@intel.com>

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

Commit b3c71b472dff2c02f0cc38d7a1959cfb2ba8420d supported MSR setting in different scopes. It added below macro:
 CPU_FEATURE_THREAD_BEFORE
 CPU_FEATURE_THREAD_AFTER
 CPU_FEATURE_CORE_BEFORE
 CPU_FEATURE_CORE_AFTER
 CPU_FEATURE_PACKAGE_BEFORE
 CPU_FEATURE_PACKAGE_AFTER

And it re-interpreted CPU_FEATURE_BEFORE as CPU_FEATURE_THREAD_BEFORE and CPU_FEATURE_AFTER as CPU_FEATURE_THREAD_AFTER.

This patch retires CPU_FEATURE_BEFORE and CPU_FEATURE_AFTER completely.

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
---
 UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h | 13 ++-----------
 .../CpuCommonFeaturesLib/CpuCommonFeaturesLib.c     |  6 +++---
 .../RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c | 10 +++++-----
 3 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
index f370373d63..d075606cdb 100644
--- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
+++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
@@ -69,17 +69,8 @@
 
 #define CPU_FEATURE_BEFORE_ALL                      BIT23
 #define CPU_FEATURE_AFTER_ALL                       BIT24
-//
-// CPU_FEATURE_BEFORE and CPU_FEATURE_AFTER only mean Thread scope -// before and Thread scope after.
-// It will be replace with CPU_FEATURE_THREAD_BEFORE and -// CPU_FEATURE_THREAD_AFTER, and should not be used anymore.
-//
-#define CPU_FEATURE_BEFORE                          BIT25
-#define CPU_FEATURE_AFTER                           BIT26
-
-#define CPU_FEATURE_THREAD_BEFORE                   CPU_FEATURE_BEFORE
-#define CPU_FEATURE_THREAD_AFTER                    CPU_FEATURE_AFTER
+#define CPU_FEATURE_THREAD_BEFORE                   BIT25
+#define CPU_FEATURE_THREAD_AFTER                    BIT26
 #define CPU_FEATURE_CORE_BEFORE                     BIT27
 #define CPU_FEATURE_CORE_AFTER                      BIT28
 #define CPU_FEATURE_PACKAGE_BEFORE                  BIT29
diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c
index 3ebd9392a9..d1fe14f519 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c
@@ -95,7 +95,7 @@ CpuCommonFeaturesLibConstructor (
                SmxSupport,
                SmxInitialize,
                CPU_FEATURE_SMX,
-               CPU_FEATURE_LOCK_FEATURE_CONTROL_REGISTER | CPU_FEATURE_BEFORE,
+               CPU_FEATURE_LOCK_FEATURE_CONTROL_REGISTER | 
+ CPU_FEATURE_THREAD_BEFORE,
                CPU_FEATURE_END
                );
     ASSERT_EFI_ERROR (Status);
@@ -107,7 +107,7 @@ CpuCommonFeaturesLibConstructor (
                VmxSupport,
                VmxInitialize,
                CPU_FEATURE_VMX,
-               CPU_FEATURE_LOCK_FEATURE_CONTROL_REGISTER | CPU_FEATURE_BEFORE,
+               CPU_FEATURE_LOCK_FEATURE_CONTROL_REGISTER | 
+ CPU_FEATURE_THREAD_BEFORE,
                CPU_FEATURE_END
                );
     ASSERT_EFI_ERROR (Status);
@@ -207,7 +207,7 @@ CpuCommonFeaturesLibConstructor (
                LmceSupport,
                LmceInitialize,
                CPU_FEATURE_LMCE,
-               CPU_FEATURE_LOCK_FEATURE_CONTROL_REGISTER | CPU_FEATURE_BEFORE,
+               CPU_FEATURE_LOCK_FEATURE_CONTROL_REGISTER | 
+ CPU_FEATURE_THREAD_BEFORE,
                CPU_FEATURE_END
                );
     ASSERT_EFI_ERROR (Status);
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
index 58910b8891..1f953832b9 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
@@ -858,16 +858,16 @@ RegisterCpuFeature (
                     != (CPU_FEATURE_CORE_BEFORE | CPU_FEATURE_CORE_AFTER));
     ASSERT ((Feature & (CPU_FEATURE_PACKAGE_BEFORE | CPU_FEATURE_PACKAGE_AFTER))
                     != (CPU_FEATURE_PACKAGE_BEFORE | CPU_FEATURE_PACKAGE_AFTER));
-    if (Feature < CPU_FEATURE_BEFORE) {
+    if (Feature < CPU_FEATURE_THREAD_BEFORE) {
       BeforeAll = ((Feature & CPU_FEATURE_BEFORE_ALL) != 0) ? TRUE : FALSE;
       AfterAll  = ((Feature & CPU_FEATURE_AFTER_ALL) != 0) ? TRUE : FALSE;
       Feature  &= ~(CPU_FEATURE_BEFORE_ALL | CPU_FEATURE_AFTER_ALL);
       ASSERT (FeatureMask == NULL);
       SetCpuFeaturesBitMask (&FeatureMask, Feature, CpuFeaturesData->BitMaskSize);
-    } else if ((Feature & CPU_FEATURE_BEFORE) != 0) {
-      SetCpuFeaturesBitMask (&BeforeFeatureBitMask, Feature & ~CPU_FEATURE_BEFORE, CpuFeaturesData->BitMaskSize);
-    } else if ((Feature & CPU_FEATURE_AFTER) != 0) {
-      SetCpuFeaturesBitMask (&AfterFeatureBitMask, Feature & ~CPU_FEATURE_AFTER, CpuFeaturesData->BitMaskSize);
+    } else if ((Feature & CPU_FEATURE_THREAD_BEFORE) != 0) {
+      SetCpuFeaturesBitMask (&BeforeFeatureBitMask, Feature & ~CPU_FEATURE_THREAD_BEFORE, CpuFeaturesData->BitMaskSize);
+    } else if ((Feature & CPU_FEATURE_THREAD_AFTER) != 0) {
+      SetCpuFeaturesBitMask (&AfterFeatureBitMask, Feature & 
+ ~CPU_FEATURE_THREAD_AFTER, CpuFeaturesData->BitMaskSize);
     } else if ((Feature & CPU_FEATURE_CORE_BEFORE) != 0) {
       SetCpuFeaturesBitMask (&CoreBeforeFeatureBitMask, Feature & ~CPU_FEATURE_CORE_BEFORE, CpuFeaturesData->BitMaskSize);
     } else if ((Feature & CPU_FEATURE_CORE_AFTER) != 0) {
--
2.21.0.windows.1

Re: [edk2-devel] [PATCH v2 2/3] UefiCpuPkg/RegisterCpuFeaturesLib: Rename [Before|After]FeatureBitMask

[Zeng, Star]

Minor comment added.

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Ray Ni
> Sent: Tuesday, November 26, 2019 2:16 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Dong, Eric <eric.dong@intel.com>
> Subject: [edk2-devel] [PATCH v2 2/3] UefiCpuPkg/RegisterCpuFeaturesLib:
> Rename [Before|After]FeatureBitMask
> 
> From: Ray Ni <ray.ni@intel.com>
> 
> The patch doesn't have any functionality impact.
> 
> Signed-off-by: Ray Ni <ray.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> ---
>  .../RegisterCpuFeatures.h                     |  4 +-
>  .../RegisterCpuFeaturesLib.c                  | 68 +++++++++++--------
>  2 files changed, 40 insertions(+), 32 deletions(-)
> 
> diff --git

[Trimmed]

> 
>    FeatureMask                 = NULL;
> -  BeforeFeatureBitMask        = NULL;
> -  AfterFeatureBitMask         = NULL;
> +  ThreadBeforeFeatureBitMask  = NULL;
> +  ThreadAfterFeatureBitMask   = NULL;
>    CoreBeforeFeatureBitMask    = NULL;
>    CoreAfterFeatureBitMask     = NULL;
>    PackageBeforeFeatureBitMask = NULL;
> @@ -850,10 +850,18 @@ RegisterCpuFeature (
>    VA_START (Marker, InitializeFunc);
>    Feature = VA_ARG (Marker, UINT32);
>    while (Feature != CPU_FEATURE_END) {
> -    ASSERT ((Feature & (CPU_FEATURE_BEFORE | CPU_FEATURE_AFTER))
> -                    != (CPU_FEATURE_BEFORE | CPU_FEATURE_AFTER));
> +    //
> +    // It's invalid to require a feature is before AND after all other features.
> +    //
>      ASSERT ((Feature & (CPU_FEATURE_BEFORE_ALL |
> CPU_FEATURE_AFTER_ALL))
>                      != (CPU_FEATURE_BEFORE_ALL | CPU_FEATURE_AFTER_ALL));
> +
> +    //
> +    // It's invalid to require feature A is before AND after before feature B,

"after before" should be just "after", right?

With it corrected, Reviewed-by: Star Zeng <star.zeng@intel.com>

Thanks,
Star

> +    // either in thread level, core level or package level.
> +    //
> +    ASSERT ((Feature & (CPU_FEATURE_THREAD_BEFORE |
> CPU_FEATURE_THREAD_AFTER))
> +                    != (CPU_FEATURE_THREAD_BEFORE |
> CPU_FEATURE_THREAD_AFTER));
>      ASSERT ((Feature & (CPU_FEATURE_CORE_BEFORE |
> CPU_FEATURE_CORE_AFTER))
>                      != (CPU_FEATURE_CORE_BEFORE | CPU_FEATURE_CORE_AFTER));
>      ASSERT ((Feature & (CPU_FEATURE_PACKAGE_BEFORE |
> CPU_FEATURE_PACKAGE_AFTER))
> @@ -865,9 +873,9 @@ RegisterCpuFeature (
>        ASSERT (FeatureMask == NULL);
>        SetCpuFeaturesBitMask (&FeatureMask, Feature, CpuFeaturesData-
> >BitMaskSize);
>      } else if ((Feature & CPU_FEATURE_THREAD_BEFORE) != 0) {
> -      SetCpuFeaturesBitMask (&BeforeFeatureBitMask, Feature &
> ~CPU_FEATURE_THREAD_BEFORE, CpuFeaturesData->BitMaskSize);
> +      SetCpuFeaturesBitMask (&ThreadBeforeFeatureBitMask, Feature &
> ~CPU_FEATURE_THREAD_BEFORE, CpuFeaturesData->BitMaskSize);
>      } else if ((Feature & CPU_FEATURE_THREAD_AFTER) != 0) {
> -      SetCpuFeaturesBitMask (&AfterFeatureBitMask, Feature &
> ~CPU_FEATURE_THREAD_AFTER, CpuFeaturesData->BitMaskSize);
> +      SetCpuFeaturesBitMask (&ThreadAfterFeatureBitMask, Feature &
> ~CPU_FEATURE_THREAD_AFTER, CpuFeaturesData->BitMaskSize);
>      } else if ((Feature & CPU_FEATURE_CORE_BEFORE) != 0) {
>        SetCpuFeaturesBitMask (&CoreBeforeFeatureBitMask, Feature &
> ~CPU_FEATURE_CORE_BEFORE, CpuFeaturesData->BitMaskSize);
>      } else if ((Feature & CPU_FEATURE_CORE_AFTER) != 0) {
> @@ -885,8 +893,8 @@ RegisterCpuFeature (
>    ASSERT (CpuFeature != NULL);
>    CpuFeature->Signature                   = CPU_FEATURE_ENTRY_SIGNATURE;
>    CpuFeature->FeatureMask                 = FeatureMask;
> -  CpuFeature->BeforeFeatureBitMask        = BeforeFeatureBitMask;
> -  CpuFeature->AfterFeatureBitMask         = AfterFeatureBitMask;
> +  CpuFeature->ThreadBeforeFeatureBitMask  =
> ThreadBeforeFeatureBitMask;
> +  CpuFeature->ThreadAfterFeatureBitMask   = ThreadAfterFeatureBitMask;
>    CpuFeature->CoreBeforeFeatureBitMask    = CoreBeforeFeatureBitMask;
>    CpuFeature->CoreAfterFeatureBitMask     = CoreAfterFeatureBitMask;
>    CpuFeature->PackageBeforeFeatureBitMask =
> PackageBeforeFeatureBitMask;
> --
> 2.21.0.windows.1
> 
> 
> 

Re: [PATCH v2 3/3] UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit.

[Zeng, Star]

Some feedback added.

> -----Original Message-----
> From: Ray Ni [mailto:niruiyu@users.noreply.github.com]
> Sent: Tuesday, November 26, 2019 2:16 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: [PATCH v2 3/3] UefiCpuPkg/CpuFeature: Introduce First to indicate
> 1st unit.
> 
> From: Ray Ni <ray.ni@intel.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1584
> 
> The flow of CPU feature initialization logic is:
> 1. BSP calls GetConfigDataFunc() for each thread/AP;
> 2. Each thread/AP calls SupportFunc() to detect its own capability;
> 3. BSP calls InitializeFunc() for each thread/AP.
> 
> There is a design gap in step #3. For a package scope feature that only
> requires one thread of each package does the initialization operation,
> what InitializeFunc() currently does is to do the initialization
> operation only CPU physical location Core# is 0.
> But in certain platform, Core#0 might be disabled in hardware level
> which results the certain package scope feature isn't initialized at
> all.

Need some patches to update individual InitializeFunc() for features.
These patches can be a separated patch series.

> 
> The patch adds a new field Fist to indicate the CPU's location in

"Firt" should be "First".

> its parent scope.
> First.Package is set for all APs/threads under first package;
> First.Core is set for all APs/threads under first core of each
> package;
> First.Thread is set for the AP/thread of each core.
> 
> Signed-off-by: Ray Ni <ray.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  .../Include/Library/RegisterCpuFeaturesLib.h  | 36 +++++++++
>  .../CpuFeaturesInitialize.c                   | 74 +++++++++++++++++++
>  2 files changed, 110 insertions(+)
> 
> diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> index d075606cdb..7114c8ce89 100644
> --- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> +++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> @@ -78,6 +78,37 @@
>  #define CPU_FEATURE_END                             MAX_UINT32
>  /// @}
> 
> +///
> +/// The bit field to indicate whether the processor is the first in its parent
> scope.
> +///
> +typedef struct {
> +  //
> +  // Set to 1 when current processor is the first thread in the core it resides
> in.
> +  //
> +  UINT32 Thread   : 1;
> +  //
> +  // Set to 1 when current processor is a thread of the first core in the
> module it resides in.
> +  //
> +  UINT32 Core     : 1;
> +  //
> +  // Set to 1 when current processor is a thread of the first module in the tile
> it resides in.
> +  //
> +  UINT32 Module   : 1;
> +  //
> +  // Set to 1 when current processor is a thread of the first tile in the die it
> resides in.
> +  //
> +  UINT32 Tile     : 1;
> +  //
> +  // Set to 1 when current processor is a thread of the first die in the package
> it resides in.
> +  //
> +  UINT32 Die      : 1;
> +  //
> +  // Set to 1 when current processor is a thread of the first package in the
> system.
> +  //
> +  UINT32 Package  : 1;
> +  UINT32 Reserved : 26;
> +} REGISTER_CPU_FEATURE_FIRST_PROCESSOR;
> +
>  ///
>  /// CPU Information passed into the SupportFunc and InitializeFunc of the
>  /// RegisterCpuFeature() library function.  This structure contains
> information
> @@ -88,6 +119,11 @@ typedef struct {
>    /// The package that the CPU resides
>    ///
>    EFI_PROCESSOR_INFORMATION            ProcessorInfo;
> +
> +  ///
> +  /// The bit flag indicating whether the CPU is the first
> Thread/Core/Module/Tile/Die/Package in its parent scope.
> +  ///
> +  REGISTER_CPU_FEATURE_FIRST_PROCESSOR First;
>    ///
>    /// The Display Family of the CPU computed from CPUID leaf
> CPUID_VERSION_INFO
>    ///
> diff --git
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> index 0a4fcff033..23076fd453 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> @@ -105,6 +105,9 @@ CpuInitDataInitialize (
>    EFI_CPU_PHYSICAL_LOCATION            *Location;
>    BOOLEAN                              *CoresVisited;
>    UINTN                                Index;
> +  UINT32                               PackageIndex;
> +  UINT32                               CoreIndex;
> +  UINT32                               First;
>    ACPI_CPU_DATA                        *AcpiCpuData;
>    CPU_STATUS_INFORMATION               *CpuStatus;
>    UINT32                               *ValidCoreCountPerPackage;
> @@ -234,6 +237,77 @@ CpuInitDataInitialize (
>    ASSERT (CpuFeaturesData->CpuFlags.CoreSemaphoreCount != NULL);
>    CpuFeaturesData->CpuFlags.PackageSemaphoreCount = AllocateZeroPool
> (sizeof (UINT32) * CpuStatus->PackageCount * CpuStatus->MaxCoreCount *
> CpuStatus->MaxThreadCount);
>    ASSERT (CpuFeaturesData->CpuFlags.PackageSemaphoreCount != NULL);
> +
> +  //
> +  // Initialize CpuFeaturesData->InitOrder[].CpuInfo.First
> +  //
> +
> +  //
> +  // Set First.Package for each thread belonging to the first package.
> +  //
> +  First = MAX_UINT32;
> +  for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> ProcessorNumber++) {
> +    Location = &CpuFeaturesData-
> >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
> +    First = MIN (Location->Package, First);
> +  }
> +  for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> ProcessorNumber++) {
> +    Location = &CpuFeaturesData-
> >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
> +    if (Location->Package == First) {
> +      CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Package =
> 1;
> +    }
> +  }
> +
> +  //
> +  // Set First.Die/Tile/Module for each thread assuming:
> +  //  single Die under each package, single Tile under each Die, single Module
> under each Tile

This assumption needs to be addressed in this or a separated patch series.

> +  //
> +  for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> ProcessorNumber++) {
> +    CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Die = 1;
> +    CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Tile = 1;
> +    CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Module =
> 1;
> +  }
> +
> +  for (PackageIndex = 0; PackageIndex < CpuStatus->PackageCount;
> PackageIndex++) {
> +    //
> +    // Set First.Core for each thread in the first core of each package.
> +    //
> +    First = MAX_UINT32;
> +    for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> ProcessorNumber++) {
> +      Location = &CpuFeaturesData-
> >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
> +      if (Location->Package == PackageIndex) {

Here the code is assuming Location->Package starts from 0 and consecutive.

> +        First = MIN (Location->Core, First);
> +      }
> +    }
> +
> +    for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> ProcessorNumber++) {
> +      Location = &CpuFeaturesData-
> >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
> +      if (Location->Package == PackageIndex && Location->Core == First) {
> +        CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Core = 1;
> +      }
> +    }
> +  }
> +
> +  for (PackageIndex = 0; PackageIndex < CpuStatus->PackageCount;
> PackageIndex++) {
> +    for (CoreIndex = 0; CoreIndex < CpuStatus->MaxCoreCount;
> CoreIndex++) {
> +      //
> +      // Set First.Thread for the first thread of each core.
> +      //
> +      First = MAX_UINT32;
> +      for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> ProcessorNumber++) {
> +        Location = &CpuFeaturesData-
> >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
> +        if (Location->Package == PackageIndex && Location->Core ==
> CoreIndex) {

Here the code is assuming Location->Package and Location->Core start from 0 and consecutive.
We could not have this assumption, this patch is to resolve this assumption.


Thanks,
Star

> +          First = MIN (Location->Thread, First);
> +        }
> +      }
> +
> +      for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> ProcessorNumber++) {
> +        Location = &CpuFeaturesData-
> >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
> +        if (Location->Package == PackageIndex && Location->Core ==
> CoreIndex && Location->Thread == First) {
> +          CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Thread
> = 1;
> +        }
> +      }
> +    }
> +  }
>  }
> 
>  /**
> --
> 2.21.0.windows.1

Re: [PATCH v2 3/3] UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit.

[Ni, Ray]

> 
> Need some patches to update individual InitializeFunc() for features.
> These patches can be a separated patch series.
> 
Yes.

> >
> > The patch adds a new field Fist to indicate the CPU's location in
> 
> "Firt" should be "First".
Will fix the typo in next version of patch or pushing.

> > +  //
> > +  // Set First.Die/Tile/Module for each thread assuming:
> > +  //  single Die under each package, single Tile under each Die, single
> Module
> > under each Tile
> 
> This assumption needs to be addressed in this or a separated patch series.

The assumption will be fixed after the below changes are merged to trunk.
https://github.com/tianocore/edk2-staging/tree/cpu/6-level


> > +  for (PackageIndex = 0; PackageIndex < CpuStatus->PackageCount;
> > PackageIndex++) {
> > +    //
> > +    // Set First.Core for each thread in the first core of each package.
> > +    //
> > +    First = MAX_UINT32;
> > +    for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> > ProcessorNumber++) {
> > +      Location = &CpuFeaturesData-
> > >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
> > +      if (Location->Package == PackageIndex) {
> 
> Here the code is assuming Location->Package starts from 0 and consecutive.

CpuStatus->PackageCount is assigned in CpuInitDataInitialize():
>  CpuStatus->PackageCount    = Package + 1;
>  CpuStatus->MaxCoreCount    = Core + 1;
>  CpuStatus->MaxThreadCount  = Thread + 1;
So PackageCount actually is the value of max package ID + 1.
With that, the code change isn't assuming Location->Package starts from 0 and consecutive.

> 
> Here the code is assuming Location->Package and Location->Core start from
> 0 and consecutive.
> We could not have this assumption, this patch is to resolve this assumption.
Similarly, The code change above isn't assuming Location->Core starts from 0 and consecutive.

> 
> 
> Thanks,
> Star
> 

Re: [PATCH v2 3/3] UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit.

[Zeng, Star]

Got the point.

With the typo fixed, Reviewed-by: Star Zeng <star.zeng@intel.com>

> -----Original Message-----
> From: Ni, Ray
> Sent: Thursday, January 2, 2020 11:15 AM
> To: Zeng, Star <star.zeng@intel.com>; Ray Ni
> <niruiyu@users.noreply.github.com>; devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [PATCH v2 3/3] UefiCpuPkg/CpuFeature: Introduce First to indicate
> 1st unit.
> 
> >
> > Need some patches to update individual InitializeFunc() for features.
> > These patches can be a separated patch series.
> >
> Yes.
> 
> > >
> > > The patch adds a new field Fist to indicate the CPU's location in
> >
> > "Firt" should be "First".
> Will fix the typo in next version of patch or pushing.
> 
> > > +  //
> > > +  // Set First.Die/Tile/Module for each thread assuming:
> > > +  //  single Die under each package, single Tile under each Die, single
> > Module
> > > under each Tile
> >
> > This assumption needs to be addressed in this or a separated patch series.
> 
> The assumption will be fixed after the below changes are merged to trunk.
> https://github.com/tianocore/edk2-staging/tree/cpu/6-level
> 
> 
> > > +  for (PackageIndex = 0; PackageIndex < CpuStatus->PackageCount;
> > > PackageIndex++) {
> > > +    //
> > > +    // Set First.Core for each thread in the first core of each package.
> > > +    //
> > > +    First = MAX_UINT32;
> > > +    for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> > > ProcessorNumber++) {
> > > +      Location = &CpuFeaturesData-
> > > >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
> > > +      if (Location->Package == PackageIndex) {
> >
> > Here the code is assuming Location->Package starts from 0 and consecutive.
> 
> CpuStatus->PackageCount is assigned in CpuInitDataInitialize():
> >  CpuStatus->PackageCount    = Package + 1;
> >  CpuStatus->MaxCoreCount    = Core + 1;
> >  CpuStatus->MaxThreadCount  = Thread + 1;
> So PackageCount actually is the value of max package ID + 1.
> With that, the code change isn't assuming Location->Package starts from 0 and
> consecutive.
> 
> >
> > Here the code is assuming Location->Package and Location->Core start from
> > 0 and consecutive.
> > We could not have this assumption, this patch is to resolve this assumption.
> Similarly, The code change above isn't assuming Location->Core starts from 0
> and consecutive.
> 
> >
> >
> > Thanks,
> > Star
> >

Re: [PATCH v2 3/3] UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit.

[Dong, Eric]

Reviewed-by: Eric Dong <eric.dong@intel.com>

-----Original Message-----
From: Ray Ni <niruiyu@users.noreply.github.com> 
Sent: Tuesday, November 26, 2019 2:16 PM
To: devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: [PATCH v2 3/3] UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit.

From: Ray Ni <ray.ni@intel.com>

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

The flow of CPU feature initialization logic is:
1. BSP calls GetConfigDataFunc() for each thread/AP;
2. Each thread/AP calls SupportFunc() to detect its own capability;
3. BSP calls InitializeFunc() for each thread/AP.

There is a design gap in step #3. For a package scope feature that only
requires one thread of each package does the initialization operation,
what InitializeFunc() currently does is to do the initialization
operation only CPU physical location Core# is 0.
But in certain platform, Core#0 might be disabled in hardware level
which results the certain package scope feature isn't initialized at
all.

The patch adds a new field Fist to indicate the CPU's location in
its parent scope.
First.Package is set for all APs/threads under first package;
First.Core is set for all APs/threads under first core of each
package;
First.Thread is set for the AP/thread of each core.

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
---
 .../Include/Library/RegisterCpuFeaturesLib.h  | 36 +++++++++
 .../CpuFeaturesInitialize.c                   | 74 +++++++++++++++++++
 2 files changed, 110 insertions(+)

diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
index d075606cdb..7114c8ce89 100644
--- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
+++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
@@ -78,6 +78,37 @@
 #define CPU_FEATURE_END                             MAX_UINT32
 /// @}
 
+///
+/// The bit field to indicate whether the processor is the first in its parent scope.
+///
+typedef struct {
+  //
+  // Set to 1 when current processor is the first thread in the core it resides in.
+  //
+  UINT32 Thread   : 1;
+  //
+  // Set to 1 when current processor is a thread of the first core in the module it resides in.
+  //
+  UINT32 Core     : 1;
+  //
+  // Set to 1 when current processor is a thread of the first module in the tile it resides in.
+  //
+  UINT32 Module   : 1;
+  //
+  // Set to 1 when current processor is a thread of the first tile in the die it resides in.
+  //
+  UINT32 Tile     : 1;
+  //
+  // Set to 1 when current processor is a thread of the first die in the package it resides in.
+  //
+  UINT32 Die      : 1;
+  //
+  // Set to 1 when current processor is a thread of the first package in the system.
+  //
+  UINT32 Package  : 1;
+  UINT32 Reserved : 26;
+} REGISTER_CPU_FEATURE_FIRST_PROCESSOR;
+
 ///
 /// CPU Information passed into the SupportFunc and InitializeFunc of the
 /// RegisterCpuFeature() library function.  This structure contains information
@@ -88,6 +119,11 @@ typedef struct {
   /// The package that the CPU resides
   ///
   EFI_PROCESSOR_INFORMATION            ProcessorInfo;
+
+  ///
+  /// The bit flag indicating whether the CPU is the first Thread/Core/Module/Tile/Die/Package in its parent scope.
+  ///
+  REGISTER_CPU_FEATURE_FIRST_PROCESSOR First;
   ///
   /// The Display Family of the CPU computed from CPUID leaf CPUID_VERSION_INFO
   ///
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index 0a4fcff033..23076fd453 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -105,6 +105,9 @@ CpuInitDataInitialize (
   EFI_CPU_PHYSICAL_LOCATION            *Location;
   BOOLEAN                              *CoresVisited;
   UINTN                                Index;
+  UINT32                               PackageIndex;
+  UINT32                               CoreIndex;
+  UINT32                               First;
   ACPI_CPU_DATA                        *AcpiCpuData;
   CPU_STATUS_INFORMATION               *CpuStatus;
   UINT32                               *ValidCoreCountPerPackage;
@@ -234,6 +237,77 @@ CpuInitDataInitialize (
   ASSERT (CpuFeaturesData->CpuFlags.CoreSemaphoreCount != NULL);
   CpuFeaturesData->CpuFlags.PackageSemaphoreCount = AllocateZeroPool (sizeof (UINT32) * CpuStatus->PackageCount * CpuStatus->MaxCoreCount * CpuStatus->MaxThreadCount);
   ASSERT (CpuFeaturesData->CpuFlags.PackageSemaphoreCount != NULL);
+
+  //
+  // Initialize CpuFeaturesData->InitOrder[].CpuInfo.First
+  //
+
+  //
+  // Set First.Package for each thread belonging to the first package.
+  //
+  First = MAX_UINT32;
+  for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+    Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+    First = MIN (Location->Package, First);
+  }
+  for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+    Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+    if (Location->Package == First) {
+      CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Package = 1;
+    }
+  }
+
+  //
+  // Set First.Die/Tile/Module for each thread assuming:
+  //  single Die under each package, single Tile under each Die, single Module under each Tile
+  //
+  for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+    CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Die = 1;
+    CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Tile = 1;
+    CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Module = 1;
+  }
+
+  for (PackageIndex = 0; PackageIndex < CpuStatus->PackageCount; PackageIndex++) {
+    //
+    // Set First.Core for each thread in the first core of each package.
+    //
+    First = MAX_UINT32;
+    for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+      Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+      if (Location->Package == PackageIndex) {
+        First = MIN (Location->Core, First);
+      }
+    }
+
+    for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+      Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+      if (Location->Package == PackageIndex && Location->Core == First) {
+        CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Core = 1;
+      }
+    }
+  }
+
+  for (PackageIndex = 0; PackageIndex < CpuStatus->PackageCount; PackageIndex++) {
+    for (CoreIndex = 0; CoreIndex < CpuStatus->MaxCoreCount; CoreIndex++) {
+      //
+      // Set First.Thread for the first thread of each core.
+      //
+      First = MAX_UINT32;
+      for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+        Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+        if (Location->Package == PackageIndex && Location->Core == CoreIndex) {
+          First = MIN (Location->Thread, First);
+        }
+      }
+
+      for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
+        Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;
+        if (Location->Package == PackageIndex && Location->Core == CoreIndex && Location->Thread == First) {
+          CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Thread = 1;
+        }
+      }
+    }
+  }
 }
 
 /**
-- 
2.21.0.windows.1

Re: [edk2-devel] [PATCH v2 0/3] UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit.

[Dong, Eric]

For the serial: Reviewed-by: Eric Dong <eric.dong@intel.com>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ray Ni
Sent: Tuesday, November 26, 2019 2:16 PM
To: devel@edk2.groups.io
Subject: [edk2-devel] [PATCH v2 0/3] UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit.

Ray Ni (3):
  UefiCpuPkg/RegisterCpuFeaturesLib: Delete CPU_FEATURE_[BEFORE|AFTER]
  UefiCpuPkg/RegisterCpuFeaturesLib: Rename [Before|After]FeatureBitMask
  UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit.

 .../Include/Library/RegisterCpuFeaturesLib.h  | 49 +++++++++---
 .../CpuCommonFeaturesLib.c                    |  6 +-
 .../CpuFeaturesInitialize.c                   | 74 +++++++++++++++++++
 .../RegisterCpuFeatures.h                     |  4 +-
 .../RegisterCpuFeaturesLib.c                  | 74 ++++++++++---------
 5 files changed, 158 insertions(+), 49 deletions(-)

-- 
2.21.0.windows.1

Re: [edk2-devel] [PATCH v2 0/3] UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit.

[Ni, Ray]

Laszlo,
I forgot to add you to CC list.
Do you have any comments on the patch set?

Thanks,
Ray

> -----Original Message-----
> From: Dong, Eric <eric.dong@intel.com>
> Sent: Thursday, February 13, 2020 4:04 PM
> To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>
> Subject: RE: [edk2-devel] [PATCH v2 0/3] UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit.
> 
> For the serial: Reviewed-by: Eric Dong <eric.dong@intel.com>
> 
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ray Ni
> Sent: Tuesday, November 26, 2019 2:16 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH v2 0/3] UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit.
> 
> Ray Ni (3):
>   UefiCpuPkg/RegisterCpuFeaturesLib: Delete CPU_FEATURE_[BEFORE|AFTER]
>   UefiCpuPkg/RegisterCpuFeaturesLib: Rename [Before|After]FeatureBitMask
>   UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit.
> 
>  .../Include/Library/RegisterCpuFeaturesLib.h  | 49 +++++++++---
>  .../CpuCommonFeaturesLib.c                    |  6 +-
>  .../CpuFeaturesInitialize.c                   | 74 +++++++++++++++++++
>  .../RegisterCpuFeatures.h                     |  4 +-
>  .../RegisterCpuFeaturesLib.c                  | 74 ++++++++++---------
>  5 files changed, 158 insertions(+), 49 deletions(-)
> 
> --
> 2.21.0.windows.1
> 
> 
> 

Re: [edk2-devel] [PATCH v2 0/3] UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit.

[Laszlo Ersek]

Hi Ray,

On 02/13/20 15:15, Ni, Ray wrote:
> Laszlo,
> I forgot to add you to CC list.
> Do you have any comments on the patch set?

Thanks for the ping -- no comments for now; please go ahead. I'm super
swamped with todos and OVMF doesn't use RegisterCpuFeaturesLib or
CpuCommonFeaturesLib, so I prefer to skip this set now.

Thanks
Laszlo

> 
> Thanks,
> Ray
> 
>> -----Original Message-----
>> From: Dong, Eric <eric.dong@intel.com>
>> Sent: Thursday, February 13, 2020 4:04 PM
>> To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>
>> Subject: RE: [edk2-devel] [PATCH v2 0/3] UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit.
>>
>> For the serial: Reviewed-by: Eric Dong <eric.dong@intel.com>
>>
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ray Ni
>> Sent: Tuesday, November 26, 2019 2:16 PM
>> To: devel@edk2.groups.io
>> Subject: [edk2-devel] [PATCH v2 0/3] UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit.
>>
>> Ray Ni (3):
>>   UefiCpuPkg/RegisterCpuFeaturesLib: Delete CPU_FEATURE_[BEFORE|AFTER]
>>   UefiCpuPkg/RegisterCpuFeaturesLib: Rename [Before|After]FeatureBitMask
>>   UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit.
>>
>>  .../Include/Library/RegisterCpuFeaturesLib.h  | 49 +++++++++---
>>  .../CpuCommonFeaturesLib.c                    |  6 +-
>>  .../CpuFeaturesInitialize.c                   | 74 +++++++++++++++++++
>>  .../RegisterCpuFeatures.h                     |  4 +-
>>  .../RegisterCpuFeaturesLib.c                  | 74 ++++++++++---------
>>  5 files changed, 158 insertions(+), 49 deletions(-)
>>
>> --
>> 2.21.0.windows.1
>>
>>
>> 
>