Re: [edk2-devel] [PATCH v5 4/6] UefiCpuPkg/MpInitLib: Produce EDKII microcode patch HOB

["Dong, Eric" <eric.dong@intel.com>]

Reviewed-by: Eric Dong <eric.dong@intel.com>

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Wu, Hao A
> Sent: Tuesday, December 31, 2019 8:49 AM
> 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>; Zeng, Star
> <star.zeng@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Kinney, Michael
> D <michael.d.kinney@intel.com>
> Subject: [edk2-devel] [PATCH v5 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.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Star Zeng <star.zeng@intel.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/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 330fd99623..4162b4a8dc 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 @@ -295,6
> +299,16 @@ MicrocodeDetect (
>    } while (((UINTN) MicrocodeEntryPoint < MicrocodeEnd));
> 
>  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);  }
> +
>    if (LatestRevision > CurrentRevision) {
>      //
>      // BIOS only authenticate updates that contain a numerically larger revision
> 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
> 
> 
>