[PATCH] UefiCpuPkg CpuCommFeaturesLib: Fix ASSERT if LMCE is supported

[PATCH] UefiCpuPkg CpuCommFeaturesLib: Fix ASSERT if LMCE is supported

[Zeng, Star]

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1829

There will be ASSERT if LMCE is supported as below.
DXE_ASSERT!: [CpuFeaturesDxe]
  XXX\UefiCpuPkg\Library\CpuCommonFeaturesLib\MachineCheck.c (342):
    ConfigData != ((void *) 0)

The code should get Config Data and FeatureControlGetConfigData
could be used.

BTW: A typo in LmceInitialize is also fixed.

Change-Id: I32b63ba649fc2977e155181a6263009e359742ed
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ruiyu Ni <ruiyu.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>
---
 UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c | 2 +-
 UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c
index 738b57dc87f9..9ddc6ce9d476 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c
@@ -214,7 +214,7 @@ CpuCommonFeaturesLibConstructor (
   if (IsCpuFeatureSupported (CPU_FEATURE_LMCE)) {
     Status = RegisterCpuFeature (
                "LMCE",
-               NULL,
+               FeatureControlGetConfigData,
                LmceSupport,
                LmceInitialize,
                CPU_FEATURE_LMCE,
diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c
index 9ee559130080..2528e0044ecb 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c
@@ -322,7 +322,7 @@ LmceInitialize (
   MSR_IA32_FEATURE_CONTROL_REGISTER    *MsrRegister;
 
   //
-  // The scope of FastStrings bit in the MSR_IA32_MISC_ENABLE is core for below processor type, only program
+  // The scope of LcmeOn bit in the MSR_IA32_MISC_ENABLE is core for below processor type, only program
   // MSR_IA32_MISC_ENABLE for thread 0 in each core.
   //
   if (IS_SILVERMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) ||
-- 
2.21.0.windows.1

Re: [PATCH] UefiCpuPkg CpuCommFeaturesLib: Fix ASSERT if LMCE is supported

[Laszlo Ersek]

On 05/22/19 05:26, Star Zeng wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1829
> 
> There will be ASSERT if LMCE is supported as below.
> DXE_ASSERT!: [CpuFeaturesDxe]
>   XXX\UefiCpuPkg\Library\CpuCommonFeaturesLib\MachineCheck.c (342):
>     ConfigData != ((void *) 0)
> 
> The code should get Config Data and FeatureControlGetConfigData
> could be used.
> 
> BTW: A typo in LmceInitialize is also fixed.
> 
> Change-Id: I32b63ba649fc2977e155181a6263009e359742ed
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ruiyu Ni <ruiyu.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>
> ---
>  UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c | 2 +-
>  UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c         | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

(1) Please drop the "Change-Id" line from the commit message.

(2) Please update Ray's name and email address in the commit message.
(See Maintainers.txt.)

(3) This looks like a clear bugfix to me, so I'm fine if it's pushed,
after Eric or Ray ACK it.

(4) It would be really nice if you could identify in either the commit
message or the bugzilla the commit that introduced the issue.

If this is a regression fix, then I also suggest adding the Keyword
"regression" to TianoCore#1829.

Thanks
Laszlo


> diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c
> index 738b57dc87f9..9ddc6ce9d476 100644
> --- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c
> +++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c
> @@ -214,7 +214,7 @@ CpuCommonFeaturesLibConstructor (
>    if (IsCpuFeatureSupported (CPU_FEATURE_LMCE)) {
>      Status = RegisterCpuFeature (
>                 "LMCE",
> -               NULL,
> +               FeatureControlGetConfigData,
>                 LmceSupport,
>                 LmceInitialize,
>                 CPU_FEATURE_LMCE,
> diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c
> index 9ee559130080..2528e0044ecb 100644
> --- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c
> +++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c
> @@ -322,7 +322,7 @@ LmceInitialize (
>    MSR_IA32_FEATURE_CONTROL_REGISTER    *MsrRegister;
>  
>    //
> -  // The scope of FastStrings bit in the MSR_IA32_MISC_ENABLE is core for below processor type, only program
> +  // The scope of LcmeOn bit in the MSR_IA32_MISC_ENABLE is core for below processor type, only program
>    // MSR_IA32_MISC_ENABLE for thread 0 in each core.
>    //
>    if (IS_SILVERMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) ||
>