> -----Original Message-----
> From: Dong, Eric
> Sent: Monday, December 30, 2019 2:18 PM
> To: Wu, Hao A; devel@edk2.groups.io
> Cc: Ni, Ray; Laszlo Ersek; Zeng, Star; Fu, Siyuan; Kinney, Michael D
> Subject: RE: [PATCH v4 4/6] UefiCpuPkg/MpInitLib: Produce EDKII microcode
> patch HOB
>
>
>
> > -----Original Message-----
> > From: Wu, Hao A
> > Sent: Friday, December 27, 2019 3:32 PM
> > To: devel@edk2.groups.io
> > Cc: Wu, Hao A <hao.a...@intel.com>; Dong, Eric <eric.d...@intel.com>; Ni,
> > Ray <ray...@intel.com>; Laszlo Ersek <ler...@redhat.com>; Zeng, Star
> > <star.z...@intel.com>; Fu, Siyuan <siyuan...@intel.com>; Kinney, Michael
> > D <michael.d.kin...@intel.com>
> > Subject: [PATCH v4 4/6] UefiCpuPkg/MpInitLib: Produce EDKII microcode
> > patch HOB
> >
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2430
> >
> > This commit will update the MpInitLib to:
> >
> > A. Collect the base address and size information after microcode patches
> > being loaded into memory;
> > B. Collect the detected microcode patch for each processor within system; C.
> > Based on the collected information, produce the EDKII microcode patch
> > HOB.
> >
> > Cc: Eric Dong <eric.d...@intel.com>
> > Cc: Ray Ni <ray...@intel.com>
> > Cc: Laszlo Ersek <ler...@redhat.com>
> > Cc: Star Zeng <star.z...@intel.com>
> > Cc: Siyuan Fu <siyuan...@intel.com>
> > Cc: Michael D Kinney <michael.d.kin...@intel.com>
> > Signed-off-by: Hao A Wu <hao.a...@intel.com>
> > ---
> > UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 1 +
> > UefiCpuPkg/Library/MpInitLib/MpLib.h | 24 +++++++--
> > UefiCpuPkg/Library/MpInitLib/Microcode.c | 20 +++++--
> > UefiCpuPkg/Library/MpInitLib/MpLib.c | 8 ++-
> > UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 55 ++++++++++++++++++++
> > 5 files changed, 100 insertions(+), 8 deletions(-)
> >
> > diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> > b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> > index 1538185ef9..326703cc9a 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> > +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> > @@ -63,3 +63,4 @@ [Pcd]
> >
> > [Guids]
> > gEdkiiS3SmmInitDoneGuid
> > + gEdkiiMicrocodePatchHobGuid
> > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> > b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> > index 56b0df664a..885656900c 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> > @@ -138,6 +138,7 @@ typedef struct {
> > EFI_EVENT WaitEvent;
> > UINT32 ProcessorSignature;
> > UINT8 PlatformId;
> > + UINT64 MicrocodeEntryAddr;
> > } CPU_AP_DATA;
> >
> > //
> > @@ -580,13 +581,15 @@ CheckAndUpdateApsStatus (
> > /**
> > Detect whether specified processor can find matching microcode patch and
> > load it.
> >
> > - @param[in] CpuMpData The pointer to CPU MP Data structure.
> > - @param[in] IsBspCallIn Indicate whether the caller is BSP or not.
> > + @param[in] CpuMpData The pointer to CPU MP Data structure.
> > + @param[in] ProcessorNumber The handle number of the processor. The
> > range is
> > + from 0 to the total number of logical
> > processors
> > + minus 1.
> > **/
> > VOID
> > MicrocodeDetect (
> > IN CPU_MP_DATA *CpuMpData,
> > - IN BOOLEAN IsBspCallIn
> > + IN UINTN ProcessorNumber
> > );
> >
> > /**
> > @@ -619,5 +622,20 @@ EnableDebugAgent (
> > VOID
> > );
> >
> > +/**
> > + Find the current Processor number by APIC ID.
> > +
> > + @param[in] CpuMpData Pointer to PEI CPU MP Data
> > + @param[out] ProcessorNumber Return the pocessor number found
> > +
> > + @retval EFI_SUCCESS ProcessorNumber is found and returned.
> > + @retval EFI_NOT_FOUND ProcessorNumber is not found.
> > +**/
> > +EFI_STATUS
> > +GetProcessorNumber (
> > + IN CPU_MP_DATA *CpuMpData,
> > + OUT UINTN *ProcessorNumber
> > + );
> > +
> > #endif
> >
> > diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > index 62a104742a..74a34c48fa 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > @@ -65,13 +65,15 @@ GetCurrentMicrocodeSignature (
> > It does not guarantee that the data has not been modified.
> > CPU has its own mechanism to verify Microcode Binary part.
> >
> > - @param[in] CpuMpData The pointer to CPU MP Data structure.
> > - @param[in] IsBspCallIn Indicate whether the caller is BSP or not.
> > + @param[in] CpuMpData The pointer to CPU MP Data structure.
> > + @param[in] ProcessorNumber The handle number of the processor. The
> > range is
> > + from 0 to the total number of logical
> > processors
> > + minus 1.
> > **/
> > VOID
> > MicrocodeDetect (
> > IN CPU_MP_DATA *CpuMpData,
> > - IN BOOLEAN IsBspCallIn
> > + IN UINTN ProcessorNumber
> > )
> > {
> > UINT32 ExtendedTableLength;
> > @@ -93,6 +95,7 @@ MicrocodeDetect (
> > MSR_IA32_PLATFORM_ID_REGISTER PlatformIdMsr;
> > UINT32 ProcessorFlags;
> > UINT32 ThreadId;
> > + BOOLEAN IsBspCallIn;
> >
> > //
> > // set ProcessorFlags to suppress incorrect compiler/analyzer warnings @@
> > -107,6 +110,7 @@ MicrocodeDetect (
> > }
> >
> > CurrentRevision = GetCurrentMicrocodeSignature ();
> > + IsBspCallIn = (ProcessorNumber == (UINTN)CpuMpData->BspNumber) ?
> > TRUE : FALSE;
> > if (CurrentRevision != 0 && !IsBspCallIn) {
> > //
> > // Skip loading microcode if it has been loaded successfully @@ -319,6
> > +323,16 @@ Done:
> > }
> > }
> >
> > + if (LatestRevision != 0) {
> > + //
> > + // Save the detected microcode patch entry address (including the
> > + // microcode patch header) for each processor.
> > + // It will be used when building the microcode patch cache HOB.
> > + //
> > + CpuMpData->CpuData[ProcessorNumber].MicrocodeEntryAddr =
> > + (UINTN) MicrocodeData - sizeof (CPU_MICROCODE_HEADER); }
> > +
>
> [[Eric]] Not an code error but for easy to understand the logic, propose to
> move above section code to right after "Done:".
Agree.
I will move this chunk to the suggested location.
Best Regards,
Hao Wu
>
> Thanks,
> Eric
>
> > if (IsBspCallIn && (LatestRevision != 0)) {
> > //
> > // Save BSP processor info and microcode info for later AP use.
> > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > index c72bf3c9ee..e611a8ca40 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > @@ -399,12 +399,16 @@ ApInitializeSync (
> > )
> > {
> > CPU_MP_DATA *CpuMpData;
> > + UINTN ProcessorNumber;
> > + EFI_STATUS Status;
> >
> > CpuMpData = (CPU_MP_DATA *) Buffer;
> > + Status = GetProcessorNumber (CpuMpData, &ProcessorNumber);
> > + ASSERT_EFI_ERROR (Status);
> > //
> > // Load microcode on AP
> > //
> > - MicrocodeDetect (CpuMpData, FALSE);
> > + MicrocodeDetect (CpuMpData, ProcessorNumber);
> > //
> > // Sync BSP's MTRR table to AP
> > //
> > @@ -1761,7 +1765,7 @@ MpInitLibInitialize (
> > //
> > // Detect and apply Microcode on BSP
> > //
> > - MicrocodeDetect (CpuMpData, TRUE);
> > + MicrocodeDetect (CpuMpData, CpuMpData->BspNumber);
> > //
> > // Store BSP's MTRR setting
> > //
> > diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> > b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> > index 3999603c3e..06e3f5d0d3 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> > +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> > @@ -9,6 +9,7 @@
> > #include "MpLib.h"
> > #include <Library/PeiServicesLib.h>
> > #include <Guid/S3SmmInitDone.h>
> > +#include <Guid/MicrocodePatchHob.h>
> >
> > /**
> > S3 SMM Init Done notification function.
> > @@ -291,6 +292,59 @@ CheckAndUpdateApsStatus ( }
> >
> > /**
> > + Build the microcode patch HOB that contains the base address and size
> > + of the microcode patch stored in the memory.
> > +
> > + @param[in] CpuMpData Pointer to the CPU_MP_DATA structure.
> > +
> > +**/
> > +VOID
> > +BuildMicrocodeCacheHob (
> > + IN CPU_MP_DATA *CpuMpData
> > + )
> > +{
> > + EDKII_MICROCODE_PATCH_HOB *MicrocodeHob;
> > + UINTN HobDataLength;
> > + UINT32 Index;
> > +
> > + HobDataLength = sizeof (EDKII_MICROCODE_PATCH_HOB) +
> > + sizeof (UINT64) * CpuMpData->CpuCount;
> > +
> > + MicrocodeHob = AllocatePool (HobDataLength); if (MicrocodeHob ==
> > + NULL) {
> > + ASSERT (FALSE);
> > + return;
> > + }
> > +
> > + //
> > + // Store the information of the memory region that holds the microcode
> > patches.
> > + //
> > + MicrocodeHob->MicrocodePatchAddress = CpuMpData-
> > >MicrocodePatchAddress;
> > + MicrocodeHob->MicrocodePatchRegionSize =
> > + CpuMpData->MicrocodePatchRegionSize;
> > +
> > + //
> > + // Store the detected microcode patch for each processor as well.
> > + //
> > + MicrocodeHob->ProcessorCount = CpuMpData->CpuCount; for (Index = 0;
> > + Index < CpuMpData->CpuCount; Index++) {
> > + if (CpuMpData->CpuData[Index].MicrocodeEntryAddr != 0) {
> > + MicrocodeHob->ProcessorSpecificPatchOffset[Index] =
> > + CpuMpData->CpuData[Index].MicrocodeEntryAddr - CpuMpData-
> > >MicrocodePatchAddress;
> > + } else {
> > + MicrocodeHob->ProcessorSpecificPatchOffset[Index] = MAX_UINT64;
> > + }
> > + }
> > +
> > + BuildGuidDataHob (
> > + &gEdkiiMicrocodePatchHobGuid,
> > + MicrocodeHob,
> > + HobDataLength
> > + );
> > +
> > + return;
> > +}
> > +
> > +/**
> > Initialize global data for MP support.
> >
> > @param[in] CpuMpData The pointer to CPU MP Data structure.
> > @@ -302,6 +356,7 @@ InitMpGlobalData (
> > {
> > EFI_STATUS Status;
> >
> > + BuildMicrocodeCacheHob (CpuMpData);
> > SaveCpuMpData (CpuMpData);
> >
> > ///
> > --
> > 2.12.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#52607): https://edk2.groups.io/g/devel/message/52607
Mute This Topic: https://groups.io/mt/69283207/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
