From: "Laszlo Ersek" <lersek@redhat.com>
To: devel@edk2.groups.io, star.zeng@intel.com
Cc: Eric Dong <eric.dong@intel.com>, Ray Ni <ray.ni@intel.com>,
Chandana Kumar <chandana.c.kumar@intel.com>,
Kevin Li <kevin.y.li@intel.com>
Subject: Re: [edk2-devel] [PATCH V2] UefiCpuPkg CpuCommFeaturesLib: Fix ASSERT if LMCE is supported
Date: Wed, 22 May 2019 12:32:24 +0200 [thread overview]
Message-ID: <b79c167c-e444-2d10-88ab-a31d94da687c@redhat.com> (raw)
In-Reply-To: <20190522101751.89952-1-star.zeng@intel.com>
On 05/22/19 12:17, Zeng, Star 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.
>
> This issue is there since the code was added at the commit below.
>
> Revision: 3d6275c1137c9633ce24e31522b71105367bd6a0
> Date: 2017/8/4 8:46:41
> UefiCpuPkg CpuCommonFeaturesLib: Enable LMCE feature.
>
> The commits below are also related to move the code.
>
> Revision: 023387144299741d727521b425ef443438aecc1f
> Date: 2017/9/1 10:12:38
> UefiCpuPkg/Lmce.c Remove useless file.
>
> Revision: 306a5bcc6b0170d28b0db10bd359817bb4b1db9f
> Date: 2017/8/17 11:40:38
> UefiCpuPkg/CpuCommonFeaturesLib: Merge machine check code to same file.
>
> So, the code should not be tested at all on a platform
> that supports LMCE.
>
> BTW: A typo in LmceInitialize is also fixed.
> The typo is introduced by the commit below.
>
> Revision: d28daaddb3e732468e930a809d3d3943a5de9558
> Date: 2018/10/17 9:24:05
> UefiCpuPkg/CpuCommonFeaturesLib: Register MSR base on scope Info.
>
> 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>
>
> Notes:
> v2: Based on Laszlo's great feedback.
> - Update Ray's name and email address to match Maintainers.txt.
> - Remove "Change-Id" line that was added unintended.
> - Add more code history.
>
> ---
[*]
> UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c | 2 +-
> UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
Thanks for the update. Just one more quick note:
- if you want to include the v2 changes in the commit message, then
please describe them above your Signed-off-by (in the normal commit
message body).
- If you don't want the v2 notes to be present in the final commit
message, then please add them between the "---" mark, and the diffstat.
I marked the location above with [*]. Then "git am" will automatically
ignore the notes.
No need to resubmit now, it's just a hint for the future.
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) ||
>
next prev parent reply other threads:[~2019-05-22 10:32 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-22 10:17 [PATCH V2] UefiCpuPkg CpuCommFeaturesLib: Fix ASSERT if LMCE is supported Zeng, Star
2019-05-22 10:32 ` Laszlo Ersek [this message]
2019-05-23 1:31 ` [edk2-devel] " Zeng, Star
2019-05-23 23:34 ` Dong, Eric
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b79c167c-e444-2d10-88ab-a31d94da687c@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox