* Re: [edk2-devel] [PATCH] UefiCpuPkg CpuCommonFeaturesLib: Enhance Ppin code
[not found] ` <0C09AFA07DD0434D9E2A0C6AEB048310403662D5@shsmsx102.ccr.corp.intel.com>
@ 2019-07-16 2:12 ` Ni, Ray
0 siblings, 0 replies; only message in thread
From: Ni, Ray @ 2019-07-16 2:12 UTC (permalink / raw)
To: Zeng, Star; +Cc: devel@edk2.groups.io
Reviewed-by: Ray Ni <Ray.ni@intel.com>
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Zeng, Star
> Sent: Friday, July 12, 2019 6:13 PM
> To: devel@edk2.groups.io
> Cc: Zeng, Star <star.zeng@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar,
> Chandana C <chandana.c.kumar@intel.com>; Li, Kevin Y
> <kevin.y.li@intel.com>
> Subject: [edk2-devel] [PATCH] UefiCpuPkg CpuCommonFeaturesLib:
> Enhance Ppin code
>
> 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
>
>
>
^ permalink raw reply [flat|nested] only message in thread