From: "Wu, Hao A" <hao.a.wu@intel.com>
To: "Fu, Siyuan" <siyuan.fu@intel.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Dong, Eric" <eric.dong@intel.com>, "Ni, Ray" <ray.ni@intel.com>,
"Laszlo Ersek" <lersek@redhat.com>,
"Kinney, Michael D" <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Always get CPUID & PlatformID in MicrocodeDetect()
Date: Mon, 3 Feb 2020 01:02:48 +0000 [thread overview]
Message-ID: <B80AF82E9BFB8E4FBD8C89DA810C6A093C9A2601@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <B1FF2E9001CE9041BD10B825821D5BC58B965073@SHSMSX103.ccr.corp.intel.com>
> -----Original Message-----
> From: Fu, Siyuan
> Sent: Monday, February 03, 2020 8:55 AM
> To: devel@edk2.groups.io; Wu, Hao A
> Cc: Dong, Eric; Ni, Ray; Laszlo Ersek; Kinney, Michael D
> Subject: RE: [edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Always get CPUID
> & PlatformID in MicrocodeDetect()
>
> Hi, Hao
>
> If CPUID and PlatformID will always be collected in MicrocodeDetect(), is the
> change in 999463 redundant now? Should we remove that code?
I think the changes are still needed.
The CPUID and PlatformID information stored in the 'CPU_AP_DATA' structure are
needed during the pre-process of the microcode patch headers for deciding
whether the microcode patch data should be loaded into memory.
Best Regards,
Hao Wu
>
> Best Regards
> Siyuan
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Hao
> > A
> > Sent: 2020年2月3日 8:35
> > To: devel@edk2.groups.io
> > Cc: Wu, Hao A <hao.a.wu@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni,
> > Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Fu, Siyuan
> > <siyuan.fu@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> > Subject: [edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Always get CPUID &
> > PlatformID in MicrocodeDetect()
> >
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2498
> >
> > Commit fd30b00707 updated the logic in function MicrocodeDetect() that
> > will directly use the CPUID and PlatformID information from the 'CpuData'
> > field in the CPU_MP_DATA structure, instead of collecting these
> > information for each processor via AsmCpuid() and AsmReadMsr64() calls
> > respectively.
> >
> > At that moment, this approach worked fine for APs. Since:
> > a) When the APs are waken up for the 1st time (1st MpInitLibInitialize()
> > entry at PEI phase), the function InitializeApData() will be called for
> > each AP and the CPUID and PlatformID information will be collected.
> >
> > b) During the 2nd entry of MpInitLibInitialize() at DXE phase, when the
> > APs are waken up again, the function InitializeApData() will not be
> > called, which means the CPUID and PlatformID information will not be
> > collected. However, the below logics in MicrocodeDetect() function:
> >
> > CurrentRevision = GetCurrentMicrocodeSignature ();
> > IsBspCallIn = (ProcessorNumber == (UINTN)CpuMpData->BspNumber) ?
> > TRUE : FALSE;
> > if (CurrentRevision != 0 && !IsBspCallIn) {
> > //
> > // Skip loading microcode if it has been loaded successfully
> > //
> > return;
> > }
> >
> > will ensure that the microcode detection and application will be
> > skipped due to the fact that such process has already been done in the
> > PEI phase.
> >
> > But after commit 396e791059, which removes the above skip loading logic,
> > the CPUID and PlatformID information on APs will be used upon the 2nd
> > entry of the MpInitLibInitialize(). But since the CPUID and PlatformID
> > information has not been collected, it will bring issue to the microcode
> > detection process.
> >
> > This commit will update the logic in MicrocodeDetect() back to always
> > collecting the CPUID and PlatformID information explicitly.
> >
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Siyuan Fu <siyuan.fu@intel.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
> > ---
> > UefiCpuPkg/Library/MpInitLib/Microcode.c | 15 +++++++++++++--
> > 1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > index b6675b9a60..67e214d463 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > @@ -93,6 +93,7 @@ MicrocodeDetect (
> > UINT32 InCompleteCheckSum32;
> > BOOLEAN CorrectMicrocode;
> > VOID *MicrocodeData;
> > + MSR_IA32_PLATFORM_ID_REGISTER PlatformIdMsr;
> > UINT32 ThreadId;
> > BOOLEAN IsBspCallIn;
> >
> > @@ -115,8 +116,18 @@ MicrocodeDetect (
> > }
> >
> > ExtendedTableLength = 0;
> > - Eax.Uint32 = CpuMpData->CpuData[ProcessorNumber].ProcessorSignature;
> > - PlatformId = CpuMpData->CpuData[ProcessorNumber].PlatformId;
> > + //
> > + // Here data of CPUID leafs have not been collected into context buffer,
> so
> > + // GetProcessorCpuid() cannot be used here to retrieve CPUID data.
> > + //
> > + AsmCpuid (CPUID_VERSION_INFO, &Eax.Uint32, NULL, NULL, NULL);
> > +
> > + //
> > + // The index of platform information resides in bits 50:52 of MSR
> > IA32_PLATFORM_ID
> > + //
> > + PlatformIdMsr.Uint64 = AsmReadMsr64 (MSR_IA32_PLATFORM_ID);
> > + PlatformId = (UINT8) PlatformIdMsr.Bits.PlatformId;
> > +
> >
> > //
> > // Check whether AP has same processor with BSP.
> > --
> > 2.12.0.windows.1
> >
> >
> >
next prev parent reply other threads:[~2020-02-03 1:02 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-03 0:34 [PATCH v1] UefiCpuPkg/MpInitLib: Always get CPUID & PlatformID in MicrocodeDetect() Wu, Hao A
2020-02-03 0:55 ` [edk2-devel] " Siyuan, Fu
2020-02-03 1:02 ` Wu, Hao A [this message]
2020-02-03 1:04 ` Siyuan, Fu
2020-02-03 9:09 ` Laszlo Ersek
2020-02-04 13:46 ` [edk2-devel] " Dong, Eric
2020-02-06 1:39 ` Wu, Hao A
2020-02-11 11:06 ` Ni, Ray
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=B80AF82E9BFB8E4FBD8C89DA810C6A093C9A2601@SHSMSX104.ccr.corp.intel.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