From: "Laszlo Ersek" <lersek@redhat.com>
To: devel@edk2.groups.io, ray.ni@intel.com
Cc: Eric Dong <eric.dong@intel.com>, Rahul Kumar <rahul1.kumar@intel.com>
Subject: Re: [edk2-devel] [PATCH 4/4] UefiCpuPkg/MpInitLib: Consume MicrocodeLib to remove duplicated code
Date: Wed, 7 Apr 2021 15:08:50 +0200 [thread overview]
Message-ID: <a7daef80-cfe6-fa5e-1a84-492ab7d9dbbb@redhat.com> (raw)
In-Reply-To: <20210402055807.858-5-ray.ni@intel.com>
On 04/02/21 07:58, Ni, Ray wrote:
> 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(-)
With the BZ ref added:
Acked-by: Laszlo Ersek <lersek@redhat.com>
One observation: this patch will affect all platforms (in edk2-platforms
too, for example), that consume "PeiMpInitLib.inf" or
"DxeMpInitLib.inf". The MicrocodeLib class is new (from patch#1), so all
affected platform DSCs will need new resolutions (patches similar to
patch#2).
Thanks,
Laszlo
>
> 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
>
next prev parent reply other threads:[~2021-04-07 13:08 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 ` Laszlo Ersek [this message]
2021-04-08 14:24 ` Dong, Eric
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=a7daef80-cfe6-fa5e-1a84-492ab7d9dbbb@redhat.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