public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Dong, Eric" <eric.dong@intel.com>
To: "Ni, Ray" <ray.ni@intel.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: Laszlo Ersek <lersek@redhat.com>,
	"Kumar, Rahul1" <rahul1.kumar@intel.com>
Subject: Re: [PATCH 4/4] UefiCpuPkg/MpInitLib: Consume MicrocodeLib to remove duplicated code
Date: Thu, 8 Apr 2021 14:24:09 +0000	[thread overview]
Message-ID: <CY4PR11MB1272F39090D397A9F6322A0AFE749@CY4PR11MB1272.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20210402055807.858-5-ray.ni@intel.com>

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

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Friday, April 2, 2021 1:58 PM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>
Subject: [PATCH 4/4] UefiCpuPkg/MpInitLib: Consume MicrocodeLib to remove duplicated code

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   1 +
 UefiCpuPkg/Library/MpInitLib/Microcode.c      | 484 ++++--------------
 UefiCpuPkg/Library/MpInitLib/MpLib.h          |   1 +
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   1 +
 4 files changed, 96 insertions(+), 391 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
index 860a9750e2..d34419c2a5 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
@@ -52,6 +52,7 @@ [LibraryClasses]
   SynchronizationLib

   PcdLib

   VmgExitLib

+  MicrocodeLib

 

 [Protocols]

   gEfiTimerArchProtocolGuid                     ## SOMETIMES_CONSUMES

diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index 297c2abcd1..105a9f84bf 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -1,70 +1,16 @@
 /** @file

   Implementation of loading microcode on processors.

 

-  Copyright (c) 2015 - 2020, Intel Corporation. All rights reserved.<BR>

+  Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>

   SPDX-License-Identifier: BSD-2-Clause-Patent

 

 **/

 

 #include "MpLib.h"

 

-/**

-  Get microcode update signature of currently loaded microcode update.

-

-  @return  Microcode signature.

-**/

-UINT32

-GetCurrentMicrocodeSignature (

-  VOID

-  )

-{

-  MSR_IA32_BIOS_SIGN_ID_REGISTER   BiosSignIdMsr;

-

-  AsmWriteMsr64 (MSR_IA32_BIOS_SIGN_ID, 0);

-  AsmCpuid (CPUID_VERSION_INFO, NULL, NULL, NULL, NULL);

-  BiosSignIdMsr.Uint64 = AsmReadMsr64 (MSR_IA32_BIOS_SIGN_ID);

-  return BiosSignIdMsr.Bits.MicrocodeUpdateSignature;

-}

-

 /**

   Detect whether specified processor can find matching microcode patch and load it.

 

-  Microcode Payload as the following format:

-  +----------------------------------------+------------------+

-  |          CPU_MICROCODE_HEADER          |                  |

-  +----------------------------------------+  CheckSum Part1  |

-  |            Microcode Binary            |                  |

-  +----------------------------------------+------------------+

-  |  CPU_MICROCODE_EXTENDED_TABLE_HEADER   |                  |

-  +----------------------------------------+  CheckSum Part2  |

-  |      CPU_MICROCODE_EXTENDED_TABLE      |                  |

-  |                   ...                  |                  |

-  +----------------------------------------+------------------+

-

-  There may by multiple CPU_MICROCODE_EXTENDED_TABLE in this format.

-  The count of CPU_MICROCODE_EXTENDED_TABLE is indicated by ExtendedSignatureCount

-  of CPU_MICROCODE_EXTENDED_TABLE_HEADER structure.

-

-  When we are trying to verify the CheckSum32 with extended table.

-  We should use the fields of exnteded table to replace the corresponding

-  fields in CPU_MICROCODE_HEADER structure, and recalculate the

-  CheckSum32 with CPU_MICROCODE_HEADER + Microcode Binary. We named

-  it as CheckSum Part3.

-

-  The CheckSum Part2 is used to verify the CPU_MICROCODE_EXTENDED_TABLE_HEADER

-  and CPU_MICROCODE_EXTENDED_TABLE parts. We should make sure CheckSum Part2

-  is correct before we are going to verify each CPU_MICROCODE_EXTENDED_TABLE.

-

-  Only ProcessorSignature, ProcessorFlag and CheckSum are different between

-  CheckSum Part1 and CheckSum Part3. To avoid multiple computing CheckSum Part3.

-  Save an in-complete CheckSum32 from CheckSum Part1 for common parts.

-  When we are going to calculate CheckSum32, just should use the corresponding part

-  of the ProcessorSignature, ProcessorFlag and CheckSum with in-complete CheckSum32.

-

-  Notes: CheckSum32 is not a strong verification.

-         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]  ProcessorNumber  The handle number of the processor. The range is

                                from 0 to the total number of logical processors

@@ -76,26 +22,13 @@ MicrocodeDetect (
   IN UINTN                   ProcessorNumber

   )

 {

-  UINT32                                  ExtendedTableLength;

-  UINT32                                  ExtendedTableCount;

-  CPU_MICROCODE_EXTENDED_TABLE            *ExtendedTable;

-  CPU_MICROCODE_EXTENDED_TABLE_HEADER     *ExtendedTableHeader;

-  CPU_MICROCODE_HEADER                    *MicrocodeEntryPoint;

+  CPU_MICROCODE_HEADER                    *Microcode;

   UINTN                                   MicrocodeEnd;

-  UINTN                                   Index;

-  UINT8                                   PlatformId;

-  CPUID_VERSION_INFO_EAX                  Eax;

-  CPU_AP_DATA                             *CpuData;

-  UINT32                                  CurrentRevision;

+  CPU_AP_DATA                             *BspData;

   UINT32                                  LatestRevision;

-  UINTN                                   TotalSize;

-  UINT32                                  CheckSum32;

-  UINT32                                  InCompleteCheckSum32;

-  BOOLEAN                                 CorrectMicrocode;

-  VOID                                    *MicrocodeData;

-  MSR_IA32_PLATFORM_ID_REGISTER           PlatformIdMsr;

+  CPU_MICROCODE_HEADER                    *LatestMicrocode;

   UINT32                                  ThreadId;

-  BOOLEAN                                 IsBspCallIn;

+  EDKII_PEI_MICROCODE_CPU_ID              MicrocodeCpuId;

 

   if (CpuMpData->MicrocodePatchRegionSize == 0) {

     //

@@ -104,9 +37,6 @@ MicrocodeDetect (
     return;

   }

 

-  CurrentRevision = GetCurrentMicrocodeSignature ();

-  IsBspCallIn     = (ProcessorNumber == (UINTN)CpuMpData->BspNumber) ? TRUE : FALSE;

-

   GetProcessorLocationByApicId (GetInitialApicId (), NULL, NULL, &ThreadId);

   if (ThreadId != 0) {

     //

@@ -115,156 +45,35 @@ MicrocodeDetect (
     return;

   }

 

-  ExtendedTableLength = 0;

-  //

-  // Here data of CPUID leafs have not been collected into context buffer, so

-  // GetProcessorCpuid() cannot be used here to retrieve CPUID data.

-  //

-  AsmCpuid (CPUID_VERSION_INFO, &Eax.Uint32, NULL, NULL, NULL);

+  GetProcessorMicrocodeCpuId (&MicrocodeCpuId);

 

-  //

-  // The index of platform information resides in bits 50:52 of MSR IA32_PLATFORM_ID

-  //

-  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 (ProcessorNumber != (UINTN) CpuMpData->BspNumber) {

     //

-    // Get the CPU data for BSP

+    // Direct use microcode of BSP if AP is the same as BSP.

+    // Assume BSP calls this routine() before AP.

     //

-    CpuData = &(CpuMpData->CpuData[CpuMpData->BspNumber]);

-    if ((CpuData->ProcessorSignature == Eax.Uint32) &&

-        (CpuData->PlatformId == PlatformId) &&

-        (CpuData->MicrocodeEntryAddr != 0)) {

-      MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *)(UINTN) CpuData->MicrocodeEntryAddr;

-      MicrocodeData       = (VOID *) (MicrocodeEntryPoint + 1);

-      LatestRevision      = MicrocodeEntryPoint->UpdateRevision;

-      goto Done;

+    BspData = &(CpuMpData->CpuData[CpuMpData->BspNumber]);

+    if ((BspData->ProcessorSignature == MicrocodeCpuId.ProcessorSignature) &&

+        (BspData->PlatformId == MicrocodeCpuId.PlatformId) &&

+        (BspData->MicrocodeEntryAddr != 0)) {

+      LatestMicrocode = (CPU_MICROCODE_HEADER *)(UINTN) BspData->MicrocodeEntryAddr;

+      LatestRevision  = LatestMicrocode->UpdateRevision;

+      goto LoadMicrocode;

     }

   }

 

-  LatestRevision = 0;

-  MicrocodeData  = NULL;

-  MicrocodeEnd = (UINTN) (CpuMpData->MicrocodePatchAddress + CpuMpData->MicrocodePatchRegionSize);

-  MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (UINTN) CpuMpData->MicrocodePatchAddress;

+  //

+  // BSP or AP which is different from BSP runs here

+  // Use 0 as the starting revision to search for microcode because MicrocodePatchInfo HOB needs

+  // the latest microcode location even it's loaded to the processor.

+  //

+  LatestRevision  = 0;

+  LatestMicrocode = NULL;

+  Microcode       = (CPU_MICROCODE_HEADER *) (UINTN) CpuMpData->MicrocodePatchAddress;

+  MicrocodeEnd    = (UINTN) Microcode + (UINTN) CpuMpData->MicrocodePatchRegionSize;

 

   do {

-    //

-    // Check if the microcode is for the Cpu and the version is newer

-    // and the update can be processed on the platform

-    //

-    CorrectMicrocode = FALSE;

-

-    if (MicrocodeEntryPoint->DataSize == 0) {

-      TotalSize = sizeof (CPU_MICROCODE_HEADER) + 2000;

-    } else {

-      TotalSize = sizeof (CPU_MICROCODE_HEADER) + MicrocodeEntryPoint->DataSize;

-    }

-

-    ///

-    /// 0x0       MicrocodeBegin  MicrocodeEntry  MicrocodeEnd      0xffffffff

-    /// |--------------|---------------|---------------|---------------|

-    ///                                 valid TotalSize

-    /// TotalSize is only valid between 0 and (MicrocodeEnd - MicrocodeEntry).

-    /// And it should be aligned with 4 bytes.

-    /// If the TotalSize is invalid, skip 1KB to check next entry.

-    ///

-    if ( (UINTN)MicrocodeEntryPoint > (MAX_ADDRESS - TotalSize) ||

-         ((UINTN)MicrocodeEntryPoint + TotalSize) > MicrocodeEnd ||

-         (TotalSize & 0x3) != 0

-       ) {

-      MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) MicrocodeEntryPoint) + SIZE_1KB);

-      continue;

-    }

-

-    //

-    // Save an in-complete CheckSum32 from CheckSum Part1 for common parts.

-    //

-    InCompleteCheckSum32 = CalculateSum32 (

-                             (UINT32 *) MicrocodeEntryPoint,

-                             TotalSize

-                             );

-    InCompleteCheckSum32 -= MicrocodeEntryPoint->ProcessorSignature.Uint32;

-    InCompleteCheckSum32 -= MicrocodeEntryPoint->ProcessorFlags;

-    InCompleteCheckSum32 -= MicrocodeEntryPoint->Checksum;

-

-    if (MicrocodeEntryPoint->HeaderVersion == 0x1) {

-      //

-      // It is the microcode header. It is not the padding data between microcode patches

-      // because the padding data should not include 0x00000001 and it should be the repeated

-      // byte format (like 0xXYXYXYXY....).

-      //

-      if (MicrocodeEntryPoint->ProcessorSignature.Uint32 == Eax.Uint32 &&

-          MicrocodeEntryPoint->UpdateRevision > LatestRevision &&

-          (MicrocodeEntryPoint->ProcessorFlags & (1 << PlatformId))

-          ) {

-        //

-        // Calculate CheckSum Part1.

-        //

-        CheckSum32 = InCompleteCheckSum32;

-        CheckSum32 += MicrocodeEntryPoint->ProcessorSignature.Uint32;

-        CheckSum32 += MicrocodeEntryPoint->ProcessorFlags;

-        CheckSum32 += MicrocodeEntryPoint->Checksum;

-        if (CheckSum32 == 0) {

-          CorrectMicrocode = TRUE;

-        }

-      } else if ((MicrocodeEntryPoint->DataSize != 0) &&

-                 (MicrocodeEntryPoint->UpdateRevision > LatestRevision)) {

-        ExtendedTableLength = MicrocodeEntryPoint->TotalSize - (MicrocodeEntryPoint->DataSize +

-                                sizeof (CPU_MICROCODE_HEADER));

-        if (ExtendedTableLength != 0) {

-          //

-          // Extended Table exist, check if the CPU in support list

-          //

-          ExtendedTableHeader = (CPU_MICROCODE_EXTENDED_TABLE_HEADER *) ((UINT8 *) (MicrocodeEntryPoint)

-                                  + MicrocodeEntryPoint->DataSize + sizeof (CPU_MICROCODE_HEADER));

-          //

-          // Calculate Extended Checksum

-          //

-          if ((ExtendedTableLength % 4) == 0) {

-            //

-            // Calculate CheckSum Part2.

-            //

-            CheckSum32 = CalculateSum32 ((UINT32 *) ExtendedTableHeader, ExtendedTableLength);

-            if (CheckSum32 == 0) {

-              //

-              // Checksum correct

-              //

-              ExtendedTableCount = ExtendedTableHeader->ExtendedSignatureCount;

-              ExtendedTable      = (CPU_MICROCODE_EXTENDED_TABLE *) (ExtendedTableHeader + 1);

-              for (Index = 0; Index < ExtendedTableCount; Index ++) {

-                //

-                // Calculate CheckSum Part3.

-                //

-                CheckSum32 = InCompleteCheckSum32;

-                CheckSum32 += ExtendedTable->ProcessorSignature.Uint32;

-                CheckSum32 += ExtendedTable->ProcessorFlag;

-                CheckSum32 += ExtendedTable->Checksum;

-                if (CheckSum32 == 0) {

-                  //

-                  // Verify Header

-                  //

-                  if ((ExtendedTable->ProcessorSignature.Uint32 == Eax.Uint32) &&

-                      (ExtendedTable->ProcessorFlag & (1 << PlatformId)) ) {

-                    //

-                    // Find one

-                    //

-                    CorrectMicrocode = TRUE;

-                    break;

-                  }

-                }

-                ExtendedTable ++;

-              }

-            }

-          }

-        }

-      }

-    } else {

+    if (!IsValidMicrocode (Microcode, MicrocodeEnd - (UINTN) Microcode, LatestRevision, &MicrocodeCpuId, 1, TRUE)) {

       //

       // It is the padding data between the microcode patches for microcode patches alignment.

       // Because the microcode patch is the multiple of 1-KByte, the padding data should not

@@ -272,156 +81,40 @@ MicrocodeDetect (
       // alignment value should be larger than 1-KByte. We could skip SIZE_1KB padding data to

       // find the next possible microcode patch header.

       //

-      MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) MicrocodeEntryPoint) + SIZE_1KB);

+      Microcode = (CPU_MICROCODE_HEADER *) ((UINTN) Microcode + SIZE_1KB);

       continue;

     }

-    //

-    // Get the next patch.

-    //

-    if (MicrocodeEntryPoint->DataSize == 0) {

-      TotalSize = 2048;

-    } else {

-      TotalSize = MicrocodeEntryPoint->TotalSize;

-    }

+    LatestMicrocode = Microcode;

+    LatestRevision  = LatestMicrocode->UpdateRevision;

 

-    if (CorrectMicrocode) {

-      LatestRevision = MicrocodeEntryPoint->UpdateRevision;

-      MicrocodeData = (VOID *) ((UINTN) MicrocodeEntryPoint + sizeof (CPU_MICROCODE_HEADER));

-    }

-

-    MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) MicrocodeEntryPoint) + TotalSize);

-  } while (((UINTN) MicrocodeEntryPoint < MicrocodeEnd));

+    Microcode = (CPU_MICROCODE_HEADER *) (((UINTN) Microcode) + GetMicrocodeLength (Microcode));

+  } while ((UINTN) Microcode < MicrocodeEnd);

 

-Done:

+LoadMicrocode:

   if (LatestRevision != 0) {

     //

-    // Save the detected microcode patch entry address (including the

-    // microcode patch header) for each processor.

+    // Save the detected microcode patch entry address (including the microcode

+    // patch header) for each processor even it's the same as the loaded one.

     // It will be used when building the microcode patch cache HOB.

     //

-    CpuMpData->CpuData[ProcessorNumber].MicrocodeEntryAddr =

-      (UINTN) MicrocodeData -  sizeof (CPU_MICROCODE_HEADER);

+    CpuMpData->CpuData[ProcessorNumber].MicrocodeEntryAddr = (UINTN) LatestMicrocode;

   }

 

-  if (LatestRevision > CurrentRevision) {

+  if (LatestRevision > GetProcessorMicrocodeSignature ()) {

     //

     // BIOS only authenticate updates that contain a numerically larger revision

     // than the currently loaded revision, where Current Signature < New Update

     // Revision. A processor with no loaded update is considered to have a

     // revision equal to zero.

     //

-    ASSERT (MicrocodeData != NULL);

-    AsmWriteMsr64 (

-        MSR_IA32_BIOS_UPDT_TRIG,

-        (UINT64) (UINTN) MicrocodeData

-        );

-  }

-  CpuMpData->CpuData[ProcessorNumber].MicrocodeRevision = GetCurrentMicrocodeSignature ();

-}

-

-/**

-  Determine if a microcode patch matchs the specific processor signature and flag.

-

-  @param[in]  CpuMpData             The pointer to CPU MP Data structure.

-  @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 CPU_MP_DATA                 *CpuMpData,

-  IN UINT32                      ProcessorSignature,

-  IN UINT32                      ProcessorFlags

-  )

-{

-  UINTN          Index;

-  CPU_AP_DATA    *CpuData;

-

-  for (Index = 0; Index < CpuMpData->CpuCount; Index++) {

-    CpuData = &CpuMpData->CpuData[Index];

-    if ((ProcessorSignature == CpuData->ProcessorSignature) &&

-        (ProcessorFlags & (1 << CpuData->PlatformId)) != 0) {

-      return TRUE;

-    }

+    LoadMicrocode (LatestMicrocode);

   }

-

-  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]  CpuMpData             The pointer to CPU MP Data structure.

-  @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 CPU_MP_DATA                 *CpuMpData,

-  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;

-

   //

-  // Check the 'ProcessorSignature' and 'ProcessorFlags' in microcode patch header.

+  // It's possible that the microcode fails to load. Just capture the CPU microcode revision after loading.

   //

-  NeedLoad = IsProcessorMatchedMicrocodePatch (

-               CpuMpData,

-               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 (

-                   CpuMpData,

-                   ExtendedTable->ProcessorSignature.Uint32,

-                   ExtendedTable->ProcessorFlag

-                   );

-      if (NeedLoad) {

-        break;

-      }

-      ExtendedTable ++;

-    }

-  }

-

-  return NeedLoad;

+  CpuMpData->CpuData[ProcessorNumber].MicrocodeRevision = GetProcessorMicrocodeSignature ();

 }

 

-

 /**

   Actual worker function that shadows the required microcode patches into memory.

 

@@ -491,14 +184,16 @@ ShadowMicrocodePatchByPcd (
   IN OUT CPU_MP_DATA             *CpuMpData

   )

 {

+  UINTN                                  Index;

   CPU_MICROCODE_HEADER                   *MicrocodeEntryPoint;

   UINTN                                  MicrocodeEnd;

-  UINTN                                  DataSize;

   UINTN                                  TotalSize;

   MICROCODE_PATCH_INFO                   *PatchInfoBuffer;

   UINTN                                  MaxPatchNumber;

   UINTN                                  PatchCount;

   UINTN                                  TotalLoadSize;

+  EDKII_PEI_MICROCODE_CPU_ID             *MicrocodeCpuIds;

+  BOOLEAN                                Valid;

 

   //

   // Initialize the microcode patch related fields in CpuMpData as the values

@@ -526,12 +221,34 @@ ShadowMicrocodePatchByPcd (
     return;

   }

 

+  MicrocodeCpuIds = AllocatePages (

+                      EFI_SIZE_TO_PAGES (CpuMpData->CpuCount * sizeof (EDKII_PEI_MICROCODE_CPU_ID))

+                      );

+  if (MicrocodeCpuIds == NULL) {

+    FreePool (PatchInfoBuffer);

+    return;

+  }

+

+  for (Index = 0; Index < CpuMpData->CpuCount; Index++) {

+    MicrocodeCpuIds[Index].PlatformId         = CpuMpData->CpuData[Index].PlatformId;

+    MicrocodeCpuIds[Index].ProcessorSignature = CpuMpData->CpuData[Index].ProcessorSignature;

+  }

+

   //

   // Process the header of each microcode patch within the region.

   // The purpose is to decide which microcode patch(es) will be loaded into memory.

+  // Microcode checksum is not verified because it's slow when performing on flash.

   //

   do {

-    if (MicrocodeEntryPoint->HeaderVersion != 0x1) {

+    Valid = IsValidMicrocode (

+              MicrocodeEntryPoint,

+              MicrocodeEnd - (UINTN) MicrocodeEntryPoint,

+              0,

+              MicrocodeCpuIds,

+              CpuMpData->CpuCount,

+              FALSE

+              );

+    if (!Valid) {

       //

       // Padding data between the microcode patches, skip 1KB to check next entry.

       //

@@ -539,59 +256,44 @@ ShadowMicrocodePatchByPcd (
       continue;

     }

 

-    DataSize  = MicrocodeEntryPoint->DataSize;

-    TotalSize = (DataSize == 0) ? 2048 : MicrocodeEntryPoint->TotalSize;

-    if ( (UINTN)MicrocodeEntryPoint > (MAX_ADDRESS - TotalSize) ||

-         ((UINTN)MicrocodeEntryPoint + TotalSize) > MicrocodeEnd ||

-         (DataSize & 0x3) != 0 ||

-         (TotalSize & (SIZE_1KB - 1)) != 0 ||

-         TotalSize < DataSize

-       ) {

+    PatchCount++;

+    if (PatchCount > MaxPatchNumber) {

       //

-      // Not a valid microcode header, skip 1KB to check next entry.

+      // Current 'PatchInfoBuffer' cannot hold the information, double the size

+      // and allocate a new buffer.

       //

-      MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) MicrocodeEntryPoint) + SIZE_1KB);

-      continue;

-    }

-

-    if (IsMicrocodePatchNeedLoad (CpuMpData, MicrocodeEntryPoint)) {

-      PatchCount++;

-      if (PatchCount > MaxPatchNumber) {

+      if (MaxPatchNumber > MAX_UINTN / 2 / sizeof (MICROCODE_PATCH_INFO)) {

         //

-        // Current 'PatchInfoBuffer' cannot hold the information, double the size

-        // and allocate a new buffer.

+        // Overflow check for MaxPatchNumber

         //

-        if (MaxPatchNumber > MAX_UINTN / 2 / sizeof (MICROCODE_PATCH_INFO)) {

-          //

-          // Overflow check for MaxPatchNumber

-          //

-          goto OnExit;

-        }

-

-        PatchInfoBuffer = ReallocatePool (

-                            MaxPatchNumber * sizeof (MICROCODE_PATCH_INFO),

-                            2 * MaxPatchNumber * sizeof (MICROCODE_PATCH_INFO),

-                            PatchInfoBuffer

-                            );

-        if (PatchInfoBuffer == NULL) {

-          goto OnExit;

-        }

-        MaxPatchNumber = MaxPatchNumber * 2;

+        goto OnExit;

       }

 

-      //

-      // Store the information of this microcode patch

-      //

-      PatchInfoBuffer[PatchCount - 1].Address = (UINTN) MicrocodeEntryPoint;

-      PatchInfoBuffer[PatchCount - 1].Size    = TotalSize;

-      TotalLoadSize += TotalSize;

+      PatchInfoBuffer = ReallocatePool (

+                          MaxPatchNumber * sizeof (MICROCODE_PATCH_INFO),

+                          2 * MaxPatchNumber * sizeof (MICROCODE_PATCH_INFO),

+                          PatchInfoBuffer

+                          );

+      if (PatchInfoBuffer == NULL) {

+        goto OnExit;

+      }

+      MaxPatchNumber = MaxPatchNumber * 2;

     }

 

+    TotalSize = GetMicrocodeLength (MicrocodeEntryPoint);

+

+    //

+    // Store the information of this microcode patch

+    //

+    PatchInfoBuffer[PatchCount - 1].Address = (UINTN) MicrocodeEntryPoint;

+    PatchInfoBuffer[PatchCount - 1].Size    = TotalSize;

+    TotalLoadSize += TotalSize;

+

     //

     // Process the next microcode patch

     //

-    MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) MicrocodeEntryPoint) + TotalSize);

-  } while (((UINTN) MicrocodeEntryPoint < MicrocodeEnd));

+    MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) ((UINTN) MicrocodeEntryPoint + TotalSize);

+  } while ((UINTN) MicrocodeEntryPoint < MicrocodeEnd);

 

   if (PatchCount != 0) {

     DEBUG ((

@@ -607,7 +309,7 @@ OnExit:
   if (PatchInfoBuffer != NULL) {

     FreePool (PatchInfoBuffer);

   }

-  return;

+  FreePages (MicrocodeCpuIds, EFI_SIZE_TO_PAGES (CpuMpData->CpuCount * sizeof (EDKII_PEI_MICROCODE_CPU_ID)));

 }

 

 /**

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 66f9eb2304..e88a5355c9 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -32,6 +32,7 @@
 #include <Library/MtrrLib.h>

 #include <Library/HobLib.h>

 #include <Library/PcdLib.h>

+#include <Library/MicrocodeLib.h>

 

 #include <Guid/MicrocodePatchHob.h>

 

diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
index 49b0ffe8be..36fcb96b58 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
@@ -51,6 +51,7 @@ [LibraryClasses]
   PeiServicesLib

   PcdLib

   VmgExitLib

+  MicrocodeLib

 

 [Pcd]

   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber        ## CONSUMES

-- 
2.27.0.windows.1


  parent reply	other threads:[~2021-04-08 14:24 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-02  5:58 [PATCH 0/4] UefiCpuPkg: Add MicrocodeLib for loading microcode Ni, Ray
2021-04-02  5:58 ` [PATCH 1/4] " Ni, Ray
2021-04-07 13:05   ` [edk2-devel] " Laszlo Ersek
2021-04-08  2:19   ` Dong, Eric
2021-04-02  5:58 ` [PATCH 2/4] OvmfPkg: Add MicrocodeLib in DSC files Ni, Ray
2021-04-07 13:05   ` [edk2-devel] " Laszlo Ersek
2021-04-02  5:58 ` [PATCH 3/4] UefiPayloadPkg/UefiPayloadPkg.dsc: Consume MicrocodeLib Ni, Ray
2021-04-08  1:56   ` Ma, Maurice
2021-04-02  5:58 ` [PATCH 4/4] UefiCpuPkg/MpInitLib: Consume MicrocodeLib to remove duplicated code Ni, Ray
2021-04-07 13:08   ` [edk2-devel] " Laszlo Ersek
2021-04-08 14:24   ` Dong, Eric [this message]
2021-04-06 12:03 ` [edk2-devel] [PATCH 0/4] UefiCpuPkg: Add MicrocodeLib for loading microcode Laszlo Ersek
2021-04-07  2:43   ` Ni, Ray
2021-04-07 13:04     ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CY4PR11MB1272F39090D397A9F6322A0AFE749@CY4PR11MB1272.namprd11.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox