From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web12.6830.1617800937982910915 for ; Wed, 07 Apr 2021 06:08:58 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=izvZ0jkN; spf=pass (domain: redhat.com, ip: 170.10.133.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1617800937; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=msEUpdiuK3Vbrn8UFOQNpnEnDJeMF9dEJ8YmPolnOeI=; b=izvZ0jkNZPZG0//DCiEVl4qM7jlUwDckDZfPP/N4x90XwiA1jbGEG97iZ8y8GVI7gOWtdM oLmk+6YrQh1wV/4IaY7Bj4GVM89uKa8JQDw5DX8ubFp75kI7tWkGMl+H0r5NS98OmsOC5T sIuYIILXMfbo6+pWbtrDEsWnarkPu/g= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-410-UK0Oko1nPNuuwuXTu7bKCA-1; Wed, 07 Apr 2021 09:08:54 -0400 X-MC-Unique: UK0Oko1nPNuuwuXTu7bKCA-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 581681922961; Wed, 7 Apr 2021 13:08:53 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-38.ams2.redhat.com [10.36.112.38]) by smtp.corp.redhat.com (Postfix) with ESMTP id BD5E75C1C4; Wed, 7 Apr 2021 13:08:51 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH 4/4] UefiCpuPkg/MpInitLib: Consume MicrocodeLib to remove duplicated code To: devel@edk2.groups.io, ray.ni@intel.com Cc: Eric Dong , Rahul Kumar References: <20210402055807.858-1-ray.ni@intel.com> <20210402055807.858-5-ray.ni@intel.com> From: "Laszlo Ersek" Message-ID: Date: Wed, 7 Apr 2021 15:08:50 +0200 MIME-Version: 1.0 In-Reply-To: <20210402055807.858-5-ray.ni@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 04/02/21 07:58, Ni, Ray wrote: > Signed-off-by: Ray Ni > Cc: Eric Dong > Cc: Laszlo Ersek > Cc: Rahul Kumar > --- > 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 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.
> + Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.
> 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 > #include > #include > +#include > > #include > > 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 >