Re: [PATCH 3/3] IntelSiliconPkg/ShadowMicrocodePei: Consume MicrocodeLib

["Siyuan, Fu" <siyuan.fu@intel.com>]

Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>

> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: 2021年4月2日 14:00
> To: devel@edk2.groups.io
> Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Fu, Siyuan
> <siyuan.fu@intel.com>
> Subject: [PATCH 3/3] IntelSiliconPkg/ShadowMicrocodePei: Consume
> MicrocodeLib
> 
> Signed-off-by: Ray Ni <ray.ni@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> ---
>  .../ShadowMicrocode/ShadowMicrocodePei.c      | 155 ++----------------
>  .../ShadowMicrocode/ShadowMicrocodePei.inf    |   3 +-
>  2 files changed, 13 insertions(+), 145 deletions(-)
> 
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodePe
> i.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodePe
> i.c
> index 98a7aed697..4e4b69a0ca 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodePe
> i.c
> +++
> b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodePe
> i.c
> @@ -1,7 +1,7 @@
>  /** @file
> 
>    FIT based microcode shadow PEIM.
> 
> 
> 
> -Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> 
> +Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
>  **/
> 
> @@ -13,6 +13,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include <Library/DebugLib.h>
> 
>  #include <Library/BaseMemoryLib.h>
> 
>  #include <Library/MemoryAllocationLib.h>
> 
> +#include <Library/MicrocodeLib.h>
> 
>  #include <IndustryStandard/FirmwareInterfaceTable.h>
> 
>  #include <Register/Intel/Microcode.h>
> 
>  #include <Register/Intel/Cpuid.h>
> 
> @@ -70,118 +71,6 @@ EFI_PEI_PPI_DESCRIPTOR
> mPeiShadowMicrocodePpiList[] = {
>    }
> 
>  };
> 
> 
> 
> -/**
> 
> -  Determine if a microcode patch matchs the specific processor signature and
> flag.
> 
> -
> 
> -  @param[in]  CpuIdCount            Number of elements in MicrocodeCpuId array.
> 
> -  @param[in]  MicrocodeCpuId        A pointer to an array of
> EDKII_PEI_MICROCODE_CPU_ID
> 
> -                                    structures.
> 
> -  @param[in]  ProcessorSignature    The processor signature field value
> 
> -                                    supported by a microcode patch.
> 
> -  @param[in]  ProcessorFlags        The prcessor flags field value supported by
> 
> -                                    a microcode patch.
> 
> -
> 
> -  @retval TRUE     The specified microcode patch will be loaded.
> 
> -  @retval FALSE    The specified microcode patch will not be loaded.
> 
> -**/
> 
> -BOOLEAN
> 
> -IsProcessorMatchedMicrocodePatch (
> 
> -  IN  UINTN                           CpuIdCount,
> 
> -  IN  EDKII_PEI_MICROCODE_CPU_ID      *MicrocodeCpuId,
> 
> -  IN UINT32                           ProcessorSignature,
> 
> -  IN UINT32                           ProcessorFlags
> 
> -  )
> 
> -{
> 
> -  UINTN          Index;
> 
> -
> 
> -  for (Index = 0; Index < CpuIdCount; Index++) {
> 
> -    if ((ProcessorSignature == MicrocodeCpuId[Index].ProcessorSignature) &&
> 
> -        (ProcessorFlags & (1 << MicrocodeCpuId[Index].PlatformId)) != 0) {
> 
> -      return TRUE;
> 
> -    }
> 
> -  }
> 
> -
> 
> -  return FALSE;
> 
> -}
> 
> -
> 
> -/**
> 
> -  Check the 'ProcessorSignature' and 'ProcessorFlags' of the microcode
> 
> -  patch header with the CPUID and PlatformID of the processors within
> 
> -  system to decide if it will be copied into memory.
> 
> -
> 
> -  @param[in]  CpuIdCount            Number of elements in MicrocodeCpuId array.
> 
> -  @param[in]  MicrocodeCpuId        A pointer to an array of
> EDKII_PEI_MICROCODE_CPU_ID
> 
> -                                    structures.
> 
> -  @param[in]  MicrocodeEntryPoint   The pointer to the microcode patch
> header.
> 
> -
> 
> -  @retval TRUE     The specified microcode patch need to be loaded.
> 
> -  @retval FALSE    The specified microcode patch dosen't need to be loaded.
> 
> -**/
> 
> -BOOLEAN
> 
> -IsMicrocodePatchNeedLoad (
> 
> -  IN  UINTN                         CpuIdCount,
> 
> -  IN  EDKII_PEI_MICROCODE_CPU_ID    *MicrocodeCpuId,
> 
> -  CPU_MICROCODE_HEADER              *MicrocodeEntryPoint
> 
> -  )
> 
> -{
> 
> -  BOOLEAN                                NeedLoad;
> 
> -  UINTN                                  DataSize;
> 
> -  UINTN                                  TotalSize;
> 
> -  CPU_MICROCODE_EXTENDED_TABLE_HEADER    *ExtendedTableHeader;
> 
> -  UINT32                                 ExtendedTableCount;
> 
> -  CPU_MICROCODE_EXTENDED_TABLE           *ExtendedTable;
> 
> -  UINTN                                  Index;
> 
> -
> 
> -  if (FeaturePcdGet (PcdShadowAllMicrocode)) {
> 
> -    return TRUE;
> 
> -  }
> 
> -
> 
> -  //
> 
> -  // Check the 'ProcessorSignature' and 'ProcessorFlags' in microcode patch
> header.
> 
> -  //
> 
> -  NeedLoad = IsProcessorMatchedMicrocodePatch (
> 
> -               CpuIdCount,
> 
> -               MicrocodeCpuId,
> 
> -               MicrocodeEntryPoint->ProcessorSignature.Uint32,
> 
> -               MicrocodeEntryPoint->ProcessorFlags
> 
> -               );
> 
> -
> 
> -  //
> 
> -  // If the Extended Signature Table exists, check if the processor is in the
> 
> -  // support list
> 
> -  //
> 
> -  DataSize  = MicrocodeEntryPoint->DataSize;
> 
> -  TotalSize = (DataSize == 0) ? 2048 : MicrocodeEntryPoint->TotalSize;
> 
> -  if ((!NeedLoad) && (DataSize != 0) &&
> 
> -      (TotalSize - DataSize > sizeof (CPU_MICROCODE_HEADER) +
> 
> -                              sizeof (CPU_MICROCODE_EXTENDED_TABLE_HEADER))) {
> 
> -    ExtendedTableHeader = (CPU_MICROCODE_EXTENDED_TABLE_HEADER *)
> ((UINT8 *) (MicrocodeEntryPoint)
> 
> -                            + DataSize + sizeof (CPU_MICROCODE_HEADER));
> 
> -    ExtendedTableCount  = ExtendedTableHeader->ExtendedSignatureCount;
> 
> -    ExtendedTable       = (CPU_MICROCODE_EXTENDED_TABLE *)
> (ExtendedTableHeader + 1);
> 
> -
> 
> -    for (Index = 0; Index < ExtendedTableCount; Index ++) {
> 
> -      //
> 
> -      // Check the 'ProcessorSignature' and 'ProcessorFlag' of the Extended
> 
> -      // Signature Table entry with the CPUID and PlatformID of the processors
> 
> -      // within system to decide if it will be copied into memory
> 
> -      //
> 
> -      NeedLoad = IsProcessorMatchedMicrocodePatch (
> 
> -                   CpuIdCount,
> 
> -                   MicrocodeCpuId,
> 
> -                   ExtendedTable->ProcessorSignature.Uint32,
> 
> -                   ExtendedTable->ProcessorFlag
> 
> -                   );
> 
> -      if (NeedLoad) {
> 
> -        break;
> 
> -      }
> 
> -      ExtendedTable ++;
> 
> -    }
> 
> -  }
> 
> -
> 
> -  return NeedLoad;
> 
> -}
> 
> -
> 
>  /**
> 
>    Actual worker function that shadows the required microcode patches into
> memory.
> 
> 
> 
> @@ -439,6 +328,11 @@ ShadowMicrocode (
>      return EFI_OUT_OF_RESOURCES;
> 
>    }
> 
> 
> 
> +  if (FeaturePcdGet (PcdShadowAllMicrocode)) {
> 
> +    MicrocodeCpuId = NULL;
> 
> +    CpuIdCount     = 0;
> 
> +  }
> 
> +
> 
>    //
> 
>    // Fill up microcode patch info buffer according to FIT table.
> 
>    //
> 
> @@ -447,37 +341,10 @@ ShadowMicrocode (
>    for (Index = 0; Index < EntryNum; Index++) {
> 
>      if (FitEntry[Index].Type == FIT_TYPE_01_MICROCODE) {
> 
>        MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (UINTN)
> FitEntry[Index].Address;
> 
> -
> 
> -      if (*(UINT32 *) MicrocodeEntryPoint == 0xFFFFFFFF) {
> 
> -        //
> 
> -        // An empty slot for reserved microcode update, skip to check next entry.
> 
> -        //
> 
> -        continue;
> 
> -      }
> 
> -
> 
> -      if (MicrocodeEntryPoint->HeaderVersion != 0x1) {
> 
> -        //
> 
> -        // Not a valid microcode header, skip to check next entry.
> 
> -        //
> 
> -        continue;
> 
> -      }
> 
> -
> 
> -      DataSize  = MicrocodeEntryPoint->DataSize;
> 
> -      TotalSize = (DataSize == 0) ? 2048 : MicrocodeEntryPoint->TotalSize;
> 
> -      if ( (UINTN)MicrocodeEntryPoint > (MAX_ADDRESS - TotalSize) ||
> 
> -           (DataSize & 0x3) != 0 ||
> 
> -           (TotalSize & (SIZE_1KB - 1)) != 0 ||
> 
> -           TotalSize < DataSize
> 
> -        ) {
> 
> -        //
> 
> -        // Not a valid microcode header, skip to check next entry.
> 
> -        //
> 
> -        continue;
> 
> -      }
> 
> -
> 
> -      if (IsMicrocodePatchNeedLoad (CpuIdCount, MicrocodeCpuId,
> MicrocodeEntryPoint)) {
> 
> -        PatchInfoBuffer[PatchCount].Address     = (UINTN) MicrocodeEntryPoint;
> 
> -        PatchInfoBuffer[PatchCount].Size        = TotalSize;
> 
> +      TotalSize = GetMicrocodeLength (MicrocodeEntryPoint);
> 
> +      if (IsValidMicrocode (MicrocodeEntryPoint, TotalSize, MicrocodeCpuId,
> CpuIdCount, FALSE)) {
> 
> +        PatchInfoBuffer[PatchCount].Address = (UINTN) MicrocodeEntryPoint;
> 
> +        PatchInfoBuffer[PatchCount].Size    = TotalSize;
> 
>          TotalLoadSize += TotalSize;
> 
>          PatchCount++;
> 
>        }
> 
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodePe
> i.inf
> b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodePe
> i.inf
> index 581780add8..5ee225297d 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodePe
> i.inf
> +++
> b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodePe
> i.inf
> @@ -1,7 +1,7 @@
>  ### @file
> 
>  # FIT based microcode shadow PEIM.
> 
>  #
> 
> -# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> 
> +# Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>
> 
>  #
> 
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  #
> 
> @@ -25,6 +25,7 @@
>    BaseMemoryLib
> 
>    HobLib
> 
>    PeiServicesLib
> 
> +  MicrocodeLib
> 
> 
> 
>  [Packages]
> 
>    MdePkg/MdePkg.dec
> 
> --
> 2.27.0.windows.1