[edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Always get CPUID & PlatformID in MicrocodeDetect()
Siyuan, Fu
siyuan.fu at intel.com
Mon Feb 3 01:04:21 UTC 2020
Got it.
Reviewed-by: Siyuan Fu <siyuan.fu at intel.com>
> -----Original Message-----
> From: Wu, Hao A <hao.a.wu at intel.com>
> Sent: 2020年2月3日 9:03
> To: Fu, Siyuan <siyuan.fu at intel.com>; devel at edk2.groups.io
> Cc: Dong, Eric <eric.dong at intel.com>; Ni, Ray <ray.ni at intel.com>; Laszlo
> Ersek <lersek at redhat.com>; Kinney, Michael D
> <michael.d.kinney at intel.com>
> Subject: RE: [edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Always get
> CPUID & PlatformID in MicrocodeDetect()
>
> > -----Original Message-----
> > From: Fu, Siyuan
> > Sent: Monday, February 03, 2020 8:55 AM
> > To: devel at 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 at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Wu,
> Hao
> > > A
> > > Sent: 2020年2月3日 8:35
> > > To: devel at edk2.groups.io
> > > Cc: Wu, Hao A <hao.a.wu at intel.com>; Dong, Eric <eric.dong at intel.com>;
> Ni,
> > > Ray <ray.ni at intel.com>; Laszlo Ersek <lersek at redhat.com>; Fu, Siyuan
> > > <siyuan.fu at intel.com>; Kinney, Michael D <michael.d.kinney at 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 at intel.com>
> > > Cc: Ray Ni <ray.ni at intel.com>
> > > Cc: Laszlo Ersek <lersek at redhat.com>
> > > Cc: Siyuan Fu <siyuan.fu at intel.com>
> > > Cc: Michael D Kinney <michael.d.kinney at intel.com>
> > > Signed-off-by: Hao A Wu <hao.a.wu at 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
> > >
> > >
> > >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#53637): https://edk2.groups.io/g/devel/message/53637
Mute This Topic: https://groups.io/mt/70934372/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list