.../CpuCommonFeaturesLib/CpuCommonFeatures.h | 15 +++++ .../CpuCommonFeaturesLib.c | 2 +- .../Library/CpuCommonFeaturesLib/Ppin.c | 65 +++++++++++++++---- 3 files changed, 70 insertions(+), 12 deletions(-)
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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#43653): https://edk2.groups.io/g/devel/message/43653
Mute This Topic: https://groups.io/mt/32440401/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43673): https://edk2.groups.io/g/devel/message/43673 Mute This Topic: https://groups.io/mt/32440401/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#43768): https://edk2.groups.io/g/devel/message/43768
Mute This Topic: https://groups.io/mt/32440401/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#43769): https://edk2.groups.io/g/devel/message/43769
Mute This Topic: https://groups.io/mt/32440401/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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
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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#43770): https://edk2.groups.io/g/devel/message/43770
Mute This Topic: https://groups.io/mt/32440401/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.