[PATCH] UefiCpuPkg CpuCommonFeaturesLib: Enhance Ppin code

["Zeng, Star" <star.zeng@intel.com>]

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1961
Enhance Ppin code to enable and unlock for TRUE State,
and disable and lock for FALSE State.
Note: enable and lock could not be set both.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Chandana Kumar <chandana.c.kumar@intel.com>
Cc: Kevin Li <kevin.y.li@intel.com>
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 .../CpuCommonFeaturesLib/CpuCommonFeatures.h  | 15 +++++
 .../CpuCommonFeaturesLib.c                    |  2 +-
 .../Library/CpuCommonFeaturesLib/Ppin.c       | 65 +++++++++++++++----
 3 files changed, 70 insertions(+), 12 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeatures.h b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeatures.h
index 9e784e916a85..8406c6c1619f 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeatures.h
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeatures.h
@@ -863,6 +863,21 @@ FeatureControlGetConfigData (
   IN UINTN               NumberOfProcessors
   );
 
+/**
+  Prepares for the data used by CPU feature detection and initialization.
+
+  @param[in]  NumberOfProcessors  The number of CPUs in the platform.
+
+  @return  Pointer to a buffer of CPU related configuration data.
+
+  @note This service could be called by BSP only.
+**/
+VOID *
+EFIAPI
+PpinGetConfigData (
+  IN UINTN               NumberOfProcessors
+  );
+
 /**
   Detects if Protected Processor Inventory Number feature supported on current
   processor.
diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c
index 7cc692efb649..fd43b8d66290 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c
@@ -203,7 +203,7 @@ CpuCommonFeaturesLibConstructor (
   if (IsCpuFeatureSupported (CPU_FEATURE_PPIN)) {
     Status = RegisterCpuFeature (
                "PPIN",
-               NULL,
+               PpinGetConfigData,
                PpinSupport,
                PpinInitialize,
                CPU_FEATURE_PPIN,
diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/Ppin.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/Ppin.c
index e8a4de8dcf60..8067cf44d015 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/Ppin.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/Ppin.c
@@ -8,6 +8,28 @@
 
 #include "CpuCommonFeatures.h"
 
+/**
+  Prepares for the data used by CPU feature detection and initialization.
+
+  @param[in]  NumberOfProcessors  The number of CPUs in the platform.
+
+  @return  Pointer to a buffer of CPU related configuration data.
+
+  @note This service could be called by BSP only.
+**/
+VOID *
+EFIAPI
+PpinGetConfigData (
+  IN UINTN               NumberOfProcessors
+  )
+{
+  VOID          *ConfigData;
+
+  ConfigData = AllocateZeroPool (sizeof (MSR_IVY_BRIDGE_PPIN_CTL_REGISTER) * NumberOfProcessors);
+  ASSERT (ConfigData != NULL);
+  return ConfigData;
+}
+
 /**
   Detects if Protected Processor Inventory Number feature supported on current
   processor.
@@ -34,6 +56,7 @@ PpinSupport (
   )
 {
   MSR_IVY_BRIDGE_PLATFORM_INFO_1_REGISTER    PlatformInfo;
+  MSR_IVY_BRIDGE_PPIN_CTL_REGISTER           *MsrPpinCtrl;
 
   if ((CpuInfo->DisplayFamily == 0x06) &&
       ((CpuInfo->DisplayModel == 0x3E) ||      // Xeon E5 V2
@@ -47,7 +70,12 @@ PpinSupport (
     // Check whether platform support this feature.
     //
     PlatformInfo.Uint64 = AsmReadMsr64 (MSR_IVY_BRIDGE_PLATFORM_INFO_1);
-    return (PlatformInfo.Bits.PPIN_CAP != 0);
+    if (PlatformInfo.Bits.PPIN_CAP != 0) {
+      MsrPpinCtrl = (MSR_IVY_BRIDGE_PPIN_CTL_REGISTER *) ConfigData;
+      ASSERT (MsrPpinCtrl != NULL);
+      MsrPpinCtrl[ProcessorNumber].Uint64 = AsmReadMsr64 (MSR_IVY_BRIDGE_PPIN_CTL);
+      return TRUE;
+    }
   }
 
   return FALSE;
@@ -73,6 +101,7 @@ PpinSupport (
   @retval RETURN_DEVICE_ERROR  Device can't change state because it has been
                                locked.
 
+  @note This service could be called by BSP only.
 **/
 RETURN_STATUS
 EFIAPI
@@ -83,16 +112,18 @@ PpinInitialize (
   IN BOOLEAN                           State
   )
 {
-  MSR_IVY_BRIDGE_PPIN_CTL_REGISTER     MsrPpinCtrl;
+  MSR_IVY_BRIDGE_PPIN_CTL_REGISTER     *MsrPpinCtrl;
+
+  MsrPpinCtrl = (MSR_IVY_BRIDGE_PPIN_CTL_REGISTER *) ConfigData;
+  ASSERT (MsrPpinCtrl != NULL);
 
   //
-  // Check whether device already lock this register.
-  // If already locked, just base on the request state and
+  // Check whether processor already lock this register.
+  // If already locked, just based on the request state and
   // the current state to return the status.
   //
-  MsrPpinCtrl.Uint64 = AsmReadMsr64 (MSR_IVY_BRIDGE_PPIN_CTL);
-  if (MsrPpinCtrl.Bits.LockOut != 0) {
-    return MsrPpinCtrl.Bits.Enable_PPIN == State ? RETURN_SUCCESS : RETURN_DEVICE_ERROR;
+  if (MsrPpinCtrl[ProcessorNumber].Bits.LockOut != 0) {
+    return MsrPpinCtrl[ProcessorNumber].Bits.Enable_PPIN == State ? RETURN_SUCCESS : RETURN_DEVICE_ERROR;
   }
 
   //
@@ -106,13 +137,25 @@ PpinInitialize (
     return RETURN_SUCCESS;
   }
 
-  CPU_REGISTER_TABLE_WRITE_FIELD (
+  if (State) {
+    //
+    // Enable and Unlock.
+    //
+    MsrPpinCtrl[ProcessorNumber].Bits.Enable_PPIN = 1;
+    MsrPpinCtrl[ProcessorNumber].Bits.LockOut = 0;
+  } else {
+    //
+    // Disable and Lock.
+    //
+    MsrPpinCtrl[ProcessorNumber].Bits.Enable_PPIN = 0;
+    MsrPpinCtrl[ProcessorNumber].Bits.LockOut = 1;
+  }
+
+  CPU_REGISTER_TABLE_WRITE64 (
     ProcessorNumber,
     Msr,
     MSR_IVY_BRIDGE_PPIN_CTL,
-    MSR_IVY_BRIDGE_PPIN_CTL_REGISTER,
-    Bits.Enable_PPIN,
-    (State) ? 1 : 0
+    MsrPpinCtrl[ProcessorNumber].Uint64
     );
 
   return RETURN_SUCCESS;
-- 
2.21.0.windows.1