Reviewed-by: Ray Ni <ray...@intel.com> > -----Original Message----- > From: Wu, Hao A <hao.a...@intel.com> > Sent: Thursday, February 6, 2020 1:24 PM > To: devel@edk2.groups.io > Cc: Wu, Hao A <hao.a...@intel.com>; Kubacki, Michael A > <michael.a.kuba...@intel.com>; Kinney, Michael D > <michael.d.kin...@intel.com>; Dong, Eric <eric.d...@intel.com>; Ni, Ray > <ray...@intel.com>; Laszlo Ersek <ler...@redhat.com> > Subject: [PATCH v1 2/2] UefiCpuPkg/MpInitLib: Not pass microcode info > between archs in CPU_MP_DATA > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2465 > > Commit 89164babec: > UefiCpuPkg/MpInitLib: don't shadow the microcode patch twice. > > attempted to use 'MicrocodePatchRegionSize' and 'MicrocodePatchAddress' > fields to avoid loading the microcode patches data into memory again in > the DXE phase. > > However, the CPU_MP_DATA structure has members with type 'UINTN' or > pointer before the microcode patch related fields. This may cause issues > when PEI and DXE are of different archs (e.g. PEI - IA32, DXE - x64), > since the microcode patch related fields will have different offsets in > the CPU_MP_DATA structure. > > Commit 88bd066166: > UefiCpuPkg/MpInitLib: Relocate microcode patch fields in CPU_MP_DATA > > tried to resolve the above-mentioned issue by relocating the fields > 'MicrocodePatchRegionSize' and 'MicrocodePatchAddress' before members with > different size between different archs. But it failed to take the case of > pre-built binaries (e.g. FSP) into consideration. > > Binaries can be built when the code base had a different version of the > CPU_MP_DATA structure definition. This may cause issues when accessing > these microcode patch related fields, since their offsets are different > (between PEI phase in the binaries and DXE phase in current code > implementation). > > This commit will use the newly introduced EDKII microcode patch HOB > instead for the DXE phase to get the information of the loaded microcode > patches data done in the PEI phase. And the 'MicrocodePatchRegionSize' and > 'MicrocodePatchAddress' fields in CPU_MP_DATA will not be used to pass > information between phases. > > For pre-built binaries, they can be classified into 3 types with regard to > the time when they are being built: > > A. Before commit 89164babec > (In other words, 'MicrocodePatchRegionSize' and 'MicrocodePatchAddress' > were not being used to skip microcode load in DXE) > > For this case, the EDKII microcode patch HOB will not be produced. This > commit will load the microcode patches data again in DXE. Such behavior is > the same with the code base back then. > > B. After commit 89164babec, before commit e1ed55738e > (In other words, 'MicrocodePatchRegionSize' and 'MicrocodePatchAddress' > being used to skip microcode load in DXE, but failed to work properly > between differnt archs.) > > For this case, the EDKII microcode patch HOB will not be produced as well. > This commit will also load the microcode patches data again in DXE. > > But since commit 89164babec failed to keep the detection and application > of microcode patches working properly in DXE after skipping the load, we > fall back to the origin behavior (that is to load the microcode patches > data again in DXE). > > C. After commit e1ed55738e > (In other words, EDKII microcode patch HOB will be produced.) > > For this case, it will have the same behavior with the BIOS built from > the current source codes. > > Cc: Michael Kubacki <michael.a.kuba...@intel.com> > Cc: Michael D Kinney <michael.d.kin...@intel.com> > Cc: Eric Dong <eric.d...@intel.com> > Cc: Ray Ni <ray...@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Signed-off-by: Hao A Wu <hao.a...@intel.com> > --- > UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 3 +- > UefiCpuPkg/Library/MpInitLib/MpLib.h | 23 +++++++++++ > UefiCpuPkg/Library/MpInitLib/Microcode.c | 43 ++++++++++++++++++++ > UefiCpuPkg/Library/MpInitLib/MpLib.c | 20 +++++---- > UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 3 +- > 5 files changed, 80 insertions(+), 12 deletions(-) > > diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > index bf5d18d521..9e6cce0895 100644 > --- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > @@ -1,7 +1,7 @@ > ## @file > # MP Initialize Library instance for DXE driver. > # > -# Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR> > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > ## > @@ -58,6 +58,7 @@ [Protocols] > [Guids] > gEfiEventExitBootServicesGuid ## CONSUMES ## Event > gEfiEventLegacyBootGuid ## SOMETIMES_CONSUMES ## > Event > + gEdkiiMicrocodePatchHobGuid ## SOMETIMES_CONSUMES ## > HOB > > [Pcd] > gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ## > CONSUMES > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h > b/UefiCpuPkg/Library/MpInitLib/MpLib.h > index d7e20f0b74..a6eab5f3d7 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h > @@ -29,6 +29,8 @@ > #include <Library/MtrrLib.h> > #include <Library/HobLib.h> > > +#include <Guid/MicrocodePatchHob.h> > + > #include <IndustryStandard/FirmwareInterfaceTable.h> > > > @@ -600,6 +602,27 @@ ShadowMicrocodeUpdatePatch ( > ); > > /** > + Get the cached microcode patch base address and size from the microcode > patch > + information cache HOB. > + > + @param[out] Address Base address of the microcode patches data. > + It will be updated if the microcode patch > + information cache HOB is found. > + @param[out] RegionSize Size of the microcode patches data. > + It will be updated if the microcode patch > + information cache HOB is found. > + > + @retval TRUE The microcode patch information cache HOB is found. > + @retval FALSE The microcode patch information cache HOB is not found. > + > +**/ > +BOOLEAN > +GetMicrocodePatchInfoFromHob ( > + UINT64 *Address, > + UINT64 *RegionSize > + ); > + > +/** > Detect whether Mwait-monitor feature is supported. > > @retval TRUE Mwait-monitor feature is supported. > diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c > b/UefiCpuPkg/Library/MpInitLib/Microcode.c > index 247f835b09..67e214d463 100644 > --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c > +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c > @@ -739,3 +739,46 @@ ShadowMicrocodeUpdatePatch ( > ShadowMicrocodePatchByPcd (CpuMpData); > } > } > + > +/** > + Get the cached microcode patch base address and size from the microcode > patch > + information cache HOB. > + > + @param[out] Address Base address of the microcode patches data. > + It will be updated if the microcode patch > + information cache HOB is found. > + @param[out] RegionSize Size of the microcode patches data. > + It will be updated if the microcode patch > + information cache HOB is found. > + > + @retval TRUE The microcode patch information cache HOB is found. > + @retval FALSE The microcode patch information cache HOB is not found. > + > +**/ > +BOOLEAN > +GetMicrocodePatchInfoFromHob ( > + UINT64 *Address, > + UINT64 *RegionSize > + ) > +{ > + EFI_HOB_GUID_TYPE *GuidHob; > + EDKII_MICROCODE_PATCH_HOB *MicrocodePathHob; > + > + GuidHob = GetFirstGuidHob (&gEdkiiMicrocodePatchHobGuid); > + if (GuidHob == NULL) { > + DEBUG((DEBUG_INFO, "%a: Microcode patch cache HOB is not found.\n", > __FUNCTION__)); > + return FALSE; > + } > + > + MicrocodePathHob = GET_GUID_HOB_DATA (GuidHob); > + > + *Address = MicrocodePathHob->MicrocodePatchAddress; > + *RegionSize = MicrocodePathHob->MicrocodePatchRegionSize; > + > + DEBUG(( > + DEBUG_INFO, "%a: MicrocodeBase = 0x%lx, MicrocodeSize = 0x%lx\n", > + __FUNCTION__, *Address, *RegionSize > + )); > + > + return TRUE; > +} > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c > b/UefiCpuPkg/Library/MpInitLib/MpLib.c > index 855d37ba3e..d0fbc17ce5 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c > @@ -1682,10 +1682,6 @@ MpInitLibInitialize ( > CpuMpData->SwitchBspFlag = FALSE; > CpuMpData->CpuData = (CPU_AP_DATA *) (CpuMpData + 1); > CpuMpData->CpuInfoInHob = (UINT64) (UINTN) (CpuMpData->CpuData + > MaxLogicalProcessorNumber); > - if (OldCpuMpData != NULL) { > - CpuMpData->MicrocodePatchRegionSize = OldCpuMpData- > >MicrocodePatchRegionSize; > - CpuMpData->MicrocodePatchAddress = OldCpuMpData- > >MicrocodePatchAddress; > - } > InitializeSpinLock(&CpuMpData->MpLock); > > // > @@ -1740,11 +1736,6 @@ MpInitLibInitialize ( > // > CollectProcessorCount (CpuMpData); > } > - > - // > - // Load required microcode patches data into memory > - // > - ShadowMicrocodeUpdatePatch (CpuMpData); > } else { > // > // APs have been wakeup before, just get the CPU Information > @@ -1762,6 +1753,17 @@ MpInitLibInitialize ( > } > } > > + if (!GetMicrocodePatchInfoFromHob ( > + &CpuMpData->MicrocodePatchAddress, > + &CpuMpData->MicrocodePatchRegionSize > + )) { > + // > + // The microcode patch information cache HOB does not exist, which means > + // the microcode patches data has not been loaded into memory yet > + // > + ShadowMicrocodeUpdatePatch (CpuMpData); > + } > + > // > // Detect and apply Microcode on BSP > // > diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c > b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c > index 06e3f5d0d3..6ecbed39ec 100644 > --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c > @@ -1,7 +1,7 @@ > /** @file > MP initialize support functions for PEI phase. > > - Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR> > + Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -9,7 +9,6 @@ > #include "MpLib.h" > #include <Library/PeiServicesLib.h> > #include <Guid/S3SmmInitDone.h> > -#include <Guid/MicrocodePatchHob.h> > > /** > S3 SMM Init Done notification function. > -- > 2.12.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#54171): https://edk2.groups.io/g/devel/message/54171 Mute This Topic: https://groups.io/mt/71015996/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
