* [PATCH] UefiCpuPkg CpuCommonFeaturesLib: Enhance Ppin code @ 2019-07-12 10:12 Zeng, Star 2019-07-12 22:12 ` Laszlo Ersek 2019-07-16 7:24 ` Dong, Eric 0 siblings, 2 replies; 5+ messages in thread From: Zeng, Star @ 2019-07-12 10:12 UTC (permalink / raw) To: devel; +Cc: Star Zeng, Laszlo Ersek, Eric Dong, Ray Ni, Chandana Kumar, Kevin Li 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 related [flat|nested] 5+ messages in thread
* Re: [PATCH] UefiCpuPkg CpuCommonFeaturesLib: Enhance Ppin code 2019-07-12 10:12 [PATCH] UefiCpuPkg CpuCommonFeaturesLib: Enhance Ppin code Zeng, Star @ 2019-07-12 22:12 ` Laszlo Ersek 2019-07-16 7:24 ` Dong, Eric 1 sibling, 0 replies; 5+ messages in thread From: Laszlo Ersek @ 2019-07-12 22:12 UTC (permalink / raw) To: Star Zeng, devel; +Cc: Eric Dong, Ray Ni, Chandana Kumar, Kevin Li On 07/12/19 12:12, Star Zeng wrote: > 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(-) I'll skip this patch. Thanks Laszlo ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] UefiCpuPkg CpuCommonFeaturesLib: Enhance Ppin code 2019-07-12 10:12 [PATCH] UefiCpuPkg CpuCommonFeaturesLib: Enhance Ppin code Zeng, Star 2019-07-12 22:12 ` Laszlo Ersek @ 2019-07-16 7:24 ` Dong, Eric 2019-07-16 8:00 ` Zeng, Star 1 sibling, 1 reply; 5+ messages in thread From: Dong, Eric @ 2019-07-16 7:24 UTC (permalink / raw) To: Zeng, Star, devel@edk2.groups.io Cc: Laszlo Ersek, Ni, Ray, Kumar, Chandana C, Li, Kevin Y Hi Star, Suggest to add some code comments for the behavior, detail see the inline comments. with these comments, Reviewed-by: Eric Dong <eric.dong@intel.com> Thanks, Eric > -----Original Message----- > From: 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: [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 { 1. I suggest to add some comments about why unlock & enable need to set at the same time. > + // > + // Disable and Lock. > + // > + MsrPpinCtrl[ProcessorNumber].Bits.Enable_PPIN = 0; > + MsrPpinCtrl[ProcessorNumber].Bits.LockOut = 1; } 2. Same as comments 1. Thanks, Eric > + > + 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] 5+ messages in thread
* Re: [PATCH] UefiCpuPkg CpuCommonFeaturesLib: Enhance Ppin code 2019-07-16 7:24 ` Dong, Eric @ 2019-07-16 8:00 ` Zeng, Star 2019-07-16 8:05 ` Dong, Eric 0 siblings, 1 reply; 5+ messages in thread From: Zeng, Star @ 2019-07-16 8:00 UTC (permalink / raw) To: Dong, Eric, devel@edk2.groups.io Cc: Laszlo Ersek, Ni, Ray, Kumar, Chandana C, Li, Kevin Y, Zeng, Star [-- Attachment #1: Type: text/plain, Size: 8082 bytes --] Eric, Thanks for the comments. Attach the updated patch, and you may help push it if it is ok. Thanks, Star > -----Original Message----- > From: Dong, Eric > Sent: Tuesday, July 16, 2019 3:24 PM > To: Zeng, Star <star.zeng@intel.com>; devel@edk2.groups.io > Cc: Laszlo Ersek <lersek@redhat.com>; Ni, Ray <ray.ni@intel.com>; Kumar, > Chandana C <chandana.c.kumar@intel.com>; Li, Kevin Y > <kevin.y.li@intel.com> > Subject: RE: [PATCH] UefiCpuPkg CpuCommonFeaturesLib: Enhance Ppin > code > > Hi Star, > > Suggest to add some code comments for the behavior, detail see the inline > comments. > with these comments, Reviewed-by: Eric Dong <eric.dong@intel.com> > > Thanks, > Eric > > -----Original Message----- > > From: 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: [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 { > > 1. I suggest to add some comments about why unlock & enable need to set > at the same time. > > > > + // > > + // Disable and Lock. > > + // > > + MsrPpinCtrl[ProcessorNumber].Bits.Enable_PPIN = 0; > > + MsrPpinCtrl[ProcessorNumber].Bits.LockOut = 1; } > > > 2. Same as comments 1. > > Thanks, > Eric > > + > > + 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 [-- Attachment #2: 0001-UefiCpuPkg-CpuCommonFeaturesLib-Enhance-Ppin-code.patch --] [-- Type: application/octet-stream, Size: 6623 bytes --] From c18a666bce999efedc3be0d0e42e9980e36ecb0f Mon Sep 17 00:00:00 2001 From: Star Zeng <star.zeng@intel.com> Date: Fri, 5 Jul 2019 13:14:33 +0800 Subject: [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. According to SDM, once Enable_PPIN is set, attempt to write 1 to LockOut will cause #GP, and writing 1 to LockOut is permitted only if Enable_PPIN is clear. 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> Reviewed-by: Ray Ni <ray.ni@intel.com> Reviewed-by: Eric Dong <eric.dong@intel.com> --- .../CpuCommonFeaturesLib/CpuCommonFeatures.h | 15 +++++ .../CpuCommonFeaturesLib.c | 2 +- .../Library/CpuCommonFeaturesLib/Ppin.c | 67 ++++++++++++++++--- 3 files changed, 72 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..8450c7ea3eaf 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,27 @@ PpinInitialize ( return RETURN_SUCCESS; } - CPU_REGISTER_TABLE_WRITE_FIELD ( + if (State) { + // + // Enable and Unlock. + // According to SDM, once Enable_PPIN is set, attempt to write 1 to LockOut will cause #GP. + // + MsrPpinCtrl[ProcessorNumber].Bits.Enable_PPIN = 1; + MsrPpinCtrl[ProcessorNumber].Bits.LockOut = 0; + } else { + // + // Disable and Lock. + // According to SDM, writing 1 to LockOut is permitted only if Enable_PPIN is clear. + // + 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 related [flat|nested] 5+ messages in thread
* Re: [PATCH] UefiCpuPkg CpuCommonFeaturesLib: Enhance Ppin code 2019-07-16 8:00 ` Zeng, Star @ 2019-07-16 8:05 ` Dong, Eric 0 siblings, 0 replies; 5+ messages in thread From: Dong, Eric @ 2019-07-16 8:05 UTC (permalink / raw) To: Zeng, Star, devel@edk2.groups.io Cc: Laszlo Ersek, Ni, Ray, Kumar, Chandana C, Li, Kevin Y Done SHA-1: 84a459472075d94963463bffaa5dc6eee47f14c3 * 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. According to SDM, once Enable_PPIN is set, attempt to write 1 to LockOut will cause #GP, and writing 1 to LockOut is permitted only if Enable_PPIN is clear. 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> Reviewed-by: Ray Ni <ray.ni@intel.com> Reviewed-by: Eric Dong <eric.dong@intel.com> > -----Original Message----- > From: Zeng, Star > Sent: Tuesday, July 16, 2019 4:00 PM > To: Dong, Eric <eric.dong@intel.com>; devel@edk2.groups.io > Cc: Laszlo Ersek <lersek@redhat.com>; Ni, Ray <ray.ni@intel.com>; Kumar, > Chandana C <chandana.c.kumar@intel.com>; Li, Kevin Y > <kevin.y.li@intel.com>; Zeng, Star <star.zeng@intel.com> > Subject: RE: [PATCH] UefiCpuPkg CpuCommonFeaturesLib: Enhance Ppin > code > > Eric, > > Thanks for the comments. > Attach the updated patch, and you may help push it if it is ok. > > > Thanks, > Star > > > -----Original Message----- > > From: Dong, Eric > > Sent: Tuesday, July 16, 2019 3:24 PM > > To: Zeng, Star <star.zeng@intel.com>; devel@edk2.groups.io > > Cc: Laszlo Ersek <lersek@redhat.com>; Ni, Ray <ray.ni@intel.com>; > > Kumar, Chandana C <chandana.c.kumar@intel.com>; Li, Kevin Y > > <kevin.y.li@intel.com> > > Subject: RE: [PATCH] UefiCpuPkg CpuCommonFeaturesLib: Enhance Ppin > > code > > > > Hi Star, > > > > Suggest to add some code comments for the behavior, detail see the > > inline comments. > > with these comments, Reviewed-by: Eric Dong <eric.dong@intel.com> > > > > Thanks, > > Eric > > > -----Original Message----- > > > From: 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: [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 { > > > > 1. I suggest to add some comments about why unlock & enable need to > > set at the same time. > > > > > > > + // > > > + // Disable and Lock. > > > + // > > > + MsrPpinCtrl[ProcessorNumber].Bits.Enable_PPIN = 0; > > > + MsrPpinCtrl[ProcessorNumber].Bits.LockOut = 1; } > > > > > > 2. Same as comments 1. > > > > Thanks, > > Eric > > > + > > > + 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] 5+ messages in thread
end of thread, other threads:[~2019-07-16 8:05 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-07-12 10:12 [PATCH] UefiCpuPkg CpuCommonFeaturesLib: Enhance Ppin code Zeng, Star 2019-07-12 22:12 ` Laszlo Ersek 2019-07-16 7:24 ` Dong, Eric 2019-07-16 8:00 ` Zeng, Star 2019-07-16 8:05 ` Dong, Eric
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox