Re: [Patch v2 2/3] UefiCpuPkg/MpInitLib: Use BSP uCode for APs if possible.

[Laszlo Ersek <lersek@redhat.com>]

On 07/12/18 12:49, Eric Dong wrote:
> Search uCode costs much time, if AP has same processor type
> with BSP, AP can use BSP saved uCode info to get better performance.
> 
> This change enables this solution.
> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
>  UefiCpuPkg/Library/MpInitLib/Microcode.c | 34 +++++++++++++++++++++++++++++---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c     |  4 ++--
>  UefiCpuPkg/Library/MpInitLib/MpLib.h     | 11 +++++++++--
>  3 files changed, 42 insertions(+), 7 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> index e47f9f4f8f..351975e2a2 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> @@ -35,11 +35,13 @@ GetCurrentMicrocodeSignature (
>  /**
>    Detect whether specified processor can find matching microcode patch and load it.
>  
> -  @param[in]  CpuMpData  The pointer to CPU MP Data structure.
> +  @param[in]  CpuMpData    The pointer to CPU MP Data structure.
> +  @param[in]  IsBspCallIn  Indicate whether the caller is BSP or not.
>  **/
>  VOID
>  MicrocodeDetect (
> -  IN CPU_MP_DATA             *CpuMpData
> +  IN CPU_MP_DATA             *CpuMpData,
> +  IN BOOLEAN                 IsBspCallIn
>    )
>  {
>    UINT32                                  ExtendedTableLength;
> @@ -58,6 +60,7 @@ MicrocodeDetect (
>    BOOLEAN                                 CorrectMicrocode;
>    VOID                                    *MicrocodeData;
>    MSR_IA32_PLATFORM_ID_REGISTER           PlatformIdMsr;
> +  UINT32                                  ProcessorFlags;
>  
>    if (CpuMpData->MicrocodePatchRegionSize == 0) {
>      //
> @@ -67,7 +70,7 @@ MicrocodeDetect (
>    }
>  
>    CurrentRevision = GetCurrentMicrocodeSignature ();
> -  if (CurrentRevision != 0) {
> +  if (CurrentRevision != 0 && !IsBspCallIn) {
>      //
>      // Skip loading microcode if it has been loaded successfully
>      //
> @@ -87,6 +90,19 @@ MicrocodeDetect (
>    PlatformIdMsr.Uint64 = AsmReadMsr64 (MSR_IA32_PLATFORM_ID);
>    PlatformId = (UINT8) PlatformIdMsr.Bits.PlatformId;
>  
> +  //
> +  // Check whether AP has same processor with BSP.
> +  // If yes, direct use microcode info saved by BSP.
> +  //
> +  if (!IsBspCallIn) {
> +    if ((CpuMpData->ProcessorSignature == Eax.Uint32) &&
> +        (CpuMpData->ProcessorFlags & (1 << PlatformId)) != 0) {
> +        MicrocodeData = (VOID *)(UINTN) CpuMpData->MicrocodeDataAddress;
> +        LatestRevision = CpuMpData->MicrocodeRevision;
> +        goto Done;
> +    }
> +  }
> +
>    LatestRevision = 0;
>    MicrocodeData  = NULL;
>    MicrocodeEnd = (UINTN) (CpuMpData->MicrocodePatchAddress + CpuMpData->MicrocodePatchRegionSize);
> @@ -117,6 +133,7 @@ MicrocodeDetect (
>          }
>          if (CheckSum32 == 0) {
>            CorrectMicrocode = TRUE;
> +          ProcessorFlags = MicrocodeEntryPoint->ProcessorFlags;
>          }
>        } else if ((MicrocodeEntryPoint->DataSize != 0) &&
>                   (MicrocodeEntryPoint->UpdateRevision > LatestRevision)) {
> @@ -151,6 +168,7 @@ MicrocodeDetect (
>                      // Find one
>                      //
>                      CorrectMicrocode = TRUE;
> +                    ProcessorFlags = ExtendedTable->ProcessorFlag;
>                      break;
>                    }
>                  }
> @@ -188,6 +206,7 @@ MicrocodeDetect (
>      MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) MicrocodeEntryPoint) + TotalSize);
>    } while (((UINTN) MicrocodeEntryPoint < MicrocodeEnd));
>  
> +Done:
>    if (LatestRevision > CurrentRevision) {
>      //
>      // BIOS only authenticate updates that contain a numerically larger revision
> @@ -211,4 +230,13 @@ MicrocodeDetect (
>        ReleaseSpinLock(&CpuMpData->MpLock);
>      }
>    }
> +
> +  if (IsBspCallIn && (LatestRevision != 0)) {
> +    CpuMpData->ProcessorSignature = Eax.Uint32;
> +    CpuMpData->ProcessorFlags = ProcessorFlags;
> +    CpuMpData->MicrocodeDataAddress = (UINTN) MicrocodeData;
> +    CpuMpData->MicrocodeRevision = LatestRevision;
> +    DEBUG ((DEBUG_INFO, "BSP Microcode:: signature [0x%08x], ProcessorFlags [0x%08x], \
> +       MicroData [0x%08x], Revision [0x%08x]\n", Eax.Uint32, ProcessorFlags, (UINTN) MicrocodeData, LatestRevision));
> +  }
>  }
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index c3cd6d7d51..3a7bdfa00b 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -410,7 +410,7 @@ ApInitializeSync (
>    //
>    // Load microcode on AP
>    //
> -  MicrocodeDetect (CpuMpData);
> +  MicrocodeDetect (CpuMpData, FALSE);
>    //
>    // Sync BSP's MTRR table to AP
>    //
> @@ -1659,7 +1659,7 @@ MpInitLibInitialize (
>    //
>    // Load Microcode on BSP
>    //
> -  MicrocodeDetect (CpuMpData);
> +  MicrocodeDetect (CpuMpData, TRUE);
>    //
>    // Store BSP's MTRR setting
>    //
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index 9aedb52636..6958080ac1 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -245,6 +245,11 @@ struct _CPU_MP_DATA {
>    BOOLEAN                        TimerInterruptState;
>    UINT64                         MicrocodePatchAddress;
>    UINT64                         MicrocodePatchRegionSize;
> +
> +  UINT32                         ProcessorSignature;
> +  UINT32                         ProcessorFlags;
> +  UINT64                         MicrocodeDataAddress;
> +  UINT32                         MicrocodeRevision;
>  };
>  
>  extern EFI_GUID mCpuInitMpLibHobGuid;
> @@ -546,11 +551,13 @@ 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]  CpuMpData    The pointer to CPU MP Data structure.
> +  @param[in]  IsBspCallIn  Indicate whether the caller is BSP or not.
>  **/
>  VOID
>  MicrocodeDetect (
> -  IN CPU_MP_DATA             *CpuMpData
> +  IN CPU_MP_DATA             *CpuMpData,
> +  IN BOOLEAN                 IsBspCallIn
>    );
>  
>  /**
> 

Acked-by: Laszlo Ersek <lersek@redhat.com>