[edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Always get CPUID & PlatformID in MicrocodeDetect()
Wu, Hao A
hao.a.wu at intel.com
Thu Feb 6 01:39:30 UTC 2020
Thanks Siyuan and Eric,
The patch has been pushed via commit a9e3458ba7.
Best Regards,
Hao Wu
> -----Original Message-----
> From: Dong, Eric
> Sent: Tuesday, February 04, 2020 9:47 PM
> To: devel at edk2.groups.io; Wu, Hao A
> Cc: Ni, Ray; Laszlo Ersek; Fu, Siyuan; Kinney, Michael D
> Subject: RE: [edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Always get
> CPUID & PlatformID in MicrocodeDetect()
>
> Reviewed-by: Eric Dong <eric.dong at intel.com>
>
> -----Original Message-----
> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Wu, Hao
> A
> Sent: Monday, February 3, 2020 8:35 AM
> 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 (#53836): https://edk2.groups.io/g/devel/message/53836
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