Reviewed-by: Eric Dong <eric.d...@intel.com> > -----Original Message----- > From: Wu, Hao A > Sent: Tuesday, December 31, 2019 8:49 AM > To: devel@edk2.groups.io > Cc: Wu, Hao A <hao.a...@intel.com>; Dong, Eric <eric.d...@intel.com>; Ni, > Ray <ray...@intel.com>; Laszlo Ersek <ler...@redhat.com>; Zeng, Star > <star.z...@intel.com>; Fu, Siyuan <siyuan...@intel.com>; Kinney, Michael > D <michael.d.kin...@intel.com> > Subject: [PATCH v5 2/6] UefiCpuPkg/MpInitLib: Reduce the size when > loading microcode patches > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2429 > > This commit will attempt to reduce the copy size when loading the microcode > patches data from flash into memory. > > Such optimization is done by a pre-process of the microcode patch headers > (on flash). A microcode patch will be loaded into memory only when the > below 3 criteria are met: > > A. With a microcode patch header (which means the data is not padding data > between microcode patches); > B. The 'ProcessorSignature' & 'ProcessorFlags' fields in the header match > at least one processor within system; C. If the Extended Signature Table > exists in a microcode patch, the > 'ProcessorSignature' & 'ProcessorFlag' fields in the table entries > match at least one processor within system. > > Criterion B and C will require all the processors to be woken up once to > collect their CPUID and Platform ID information. Hence, this commit will > move the copy, detect and apply of microcode patch on BSP and APs after all > the processors have been woken up. > > Cc: Eric Dong <eric.d...@intel.com> > Cc: Ray Ni <ray...@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Cc: Star Zeng <star.z...@intel.com> > Cc: Siyuan Fu <siyuan...@intel.com> > Cc: Michael D Kinney <michael.d.kin...@intel.com> > Signed-off-by: Hao A Wu <hao.a...@intel.com> > --- > UefiCpuPkg/Library/MpInitLib/MpLib.h | 24 ++ > UefiCpuPkg/Library/MpInitLib/Microcode.c | 288 ++++++++++++++++++++ > UefiCpuPkg/Library/MpInitLib/MpLib.c | 90 ++---- > 3 files changed, 340 insertions(+), 62 deletions(-) > > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h > b/UefiCpuPkg/Library/MpInitLib/MpLib.h > index 4440dc2701..56b0df664a 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h > @@ -44,6 +44,20 @@ > #define CPU_SWITCH_STATE_LOADED 2 > > // > +// Default maximum number of entries to store the microcode patches > +information // #define DEFAULT_MAX_MICROCODE_PATCH_NUM 8 > + > +// > +// Data structure for microcode patch information // typedef struct { > + UINTN Address; > + UINTN Size; > + UINTN AlignedSize; > +} MICROCODE_PATCH_INFO; > + > +// > // CPU exchange information for switch BSP // typedef struct { @@ -576,6 > +590,16 @@ MicrocodeDetect ( > ); > > /** > + Load the required microcode patches data into memory. > + > + @param[in, out] CpuMpData The pointer to CPU MP Data structure. > +**/ > +VOID > +LoadMicrocodePatch ( > + IN OUT CPU_MP_DATA *CpuMpData > + ); > + > +/** > Detect whether Mwait-monitor feature is supported. > > @retval TRUE Mwait-monitor feature is supported. > diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c > b/UefiCpuPkg/Library/MpInitLib/Microcode.c > index 199b1f23ce..330fd99623 100644 > --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c > +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c > @@ -331,3 +331,291 @@ Done: > MicroData [0x%08x], Revision [0x%08x]\n", Eax.Uint32, ProcessorFlags, > (UINTN) MicrocodeData, LatestRevision)); > } > } > + > +/** > + Determine if a microcode patch will be loaded into memory. > + > + @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 > +IsMicrocodePatchNeedLoad ( > + 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; > + } > + } > + > + return FALSE; > +} > + > +/** > + Actual worker function that loads the required microcode patches into > memory. > + > + @param[in, out] CpuMpData The pointer to CPU MP Data structure. > + @param[in] Patches The pointer to an array of information on > + the microcode patches that will be loaded > + into memory. > + @param[in] PatchCount The number of microcode patches that will > + be loaded into memory. > + @param[in] TotalLoadSize The total size of all the microcode > patches > + to be loaded. > +**/ > +VOID > +LoadMicrocodePatchWorker ( > + IN OUT CPU_MP_DATA *CpuMpData, > + IN MICROCODE_PATCH_INFO *Patches, > + IN UINTN PatchCount, > + IN UINTN TotalLoadSize > + ) > +{ > + UINTN Index; > + VOID *MicrocodePatchInRam; > + UINT8 *Walker; > + > + ASSERT ((Patches != NULL) && (PatchCount != 0)); > + > + MicrocodePatchInRam = AllocatePages (EFI_SIZE_TO_PAGES > + (TotalLoadSize)); if (MicrocodePatchInRam == NULL) { > + return; > + } > + > + // > + // Load all the required microcode patches into memory // for > + (Walker = MicrocodePatchInRam, Index = 0; Index < PatchCount; Index++) > { > + CopyMem ( > + Walker, > + (VOID *) Patches[Index].Address, > + Patches[Index].Size > + ); > + > + // > + // Zero-fill the padding area > + // Please note that AlignedSize will be no less than Size > + // > + ZeroMem ( > + Walker + Patches[Index].Size, > + Patches[Index].AlignedSize - Patches[Index].Size > + ); > + > + Walker += Patches[Index].AlignedSize; } > + > + // > + // Update the microcode patch related fields in CpuMpData // > + CpuMpData->MicrocodePatchAddress = (UINTN) MicrocodePatchInRam; > + CpuMpData->MicrocodePatchRegionSize = TotalLoadSize; > + > + DEBUG (( > + DEBUG_INFO, > + "%a: Required microcode patches have been loaded at 0x%lx, with size > 0x%lx.\n", > + __FUNCTION__, CpuMpData->MicrocodePatchAddress, CpuMpData- > >MicrocodePatchRegionSize > + )); > + > + return; > +} > + > +/** > + Load the required microcode patches data into memory. > + > + @param[in, out] CpuMpData The pointer to CPU MP Data structure. > +**/ > +VOID > +LoadMicrocodePatch ( > + IN OUT CPU_MP_DATA *CpuMpData > + ) > +{ > + CPU_MICROCODE_HEADER *MicrocodeEntryPoint; > + UINTN MicrocodeEnd; > + UINTN DataSize; > + UINTN TotalSize; > + CPU_MICROCODE_EXTENDED_TABLE_HEADER *ExtendedTableHeader; > + UINT32 ExtendedTableCount; > + CPU_MICROCODE_EXTENDED_TABLE *ExtendedTable; > + MICROCODE_PATCH_INFO *PatchInfoBuffer; > + UINTN MaxPatchNumber; > + UINTN PatchCount; > + UINTN TotalLoadSize; > + UINTN Index; > + BOOLEAN NeedLoad; > + > + // > + // Initialize the microcode patch related fields in CpuMpData as the > + values // specified by the PCD pair. If the microcode patches are > + loaded into memory, // these fields will be updated. > + // > + CpuMpData->MicrocodePatchAddress = PcdGet64 > (PcdCpuMicrocodePatchAddress); > + CpuMpData->MicrocodePatchRegionSize = PcdGet64 > + (PcdCpuMicrocodePatchRegionSize); > + > + MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (UINTN) > CpuMpData->MicrocodePatchAddress; > + MicrocodeEnd = (UINTN) MicrocodeEntryPoint + > + (UINTN) CpuMpData->MicrocodePatchRegionSize; > + if ((MicrocodeEntryPoint == NULL) || ((UINTN) MicrocodeEntryPoint == > MicrocodeEnd)) { > + // > + // There is no microcode patches > + // > + return; > + } > + > + PatchCount = 0; > + MaxPatchNumber = DEFAULT_MAX_MICROCODE_PATCH_NUM; > + TotalLoadSize = 0; > + PatchInfoBuffer = AllocatePool (MaxPatchNumber * sizeof > + (MICROCODE_PATCH_INFO)); if (PatchInfoBuffer == NULL) { > + return; > + } > + > + // > + // Process the header of each microcode patch within the region. > + // The purpose is to decide which microcode patch(es) will be loaded into > memory. > + // > + do { > + if (MicrocodeEntryPoint->HeaderVersion != 0x1) { > + // > + // Padding data between the microcode patches, skip 1KB to check next > entry. > + // > + MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) > MicrocodeEntryPoint) + SIZE_1KB); > + 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 > + ) { > + // > + // Not a valid microcode header, skip 1KB to check next entry. > + // > + MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) > MicrocodeEntryPoint) + SIZE_1KB); > + continue; > + } > + > + // > + // 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 > + // > + NeedLoad = IsMicrocodePatchNeedLoad ( > + CpuMpData, > + MicrocodeEntryPoint->ProcessorSignature.Uint32, > + MicrocodeEntryPoint->ProcessorFlags > + ); > + > + // > + // If the Extended Signature Table exists, check if the processor is in > the > + // support list > + // > + 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 ++) { > + // > + // Avoid access content beyond MicrocodeEnd > + // > + if ((UINTN) ExtendedTable > MicrocodeEnd - sizeof > (CPU_MICROCODE_EXTENDED_TABLE)) { > + break; > + } > + > + // > + // 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 = IsMicrocodePatchNeedLoad ( > + CpuMpData, > + ExtendedTable->ProcessorSignature.Uint32, > + ExtendedTable->ProcessorFlag > + ); > + if (NeedLoad) { > + break; > + } > + ExtendedTable ++; > + } > + } > + > + if (NeedLoad) { > + PatchCount++; > + if (PatchCount > MaxPatchNumber) { > + // > + // Current 'PatchInfoBuffer' cannot hold the information, double the > size > + // and allocate a new buffer. > + // > + 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; > + } > + > + // > + // Store the information of this microcode patch > + // > + if (TotalSize > ALIGN_VALUE (TotalSize, SIZE_1KB) || > + ALIGN_VALUE (TotalSize, SIZE_1KB) > MAX_UINTN - TotalLoadSize) { > + goto OnExit; > + } > + PatchInfoBuffer[PatchCount - 1].Address = (UINTN) > MicrocodeEntryPoint; > + PatchInfoBuffer[PatchCount - 1].Size = TotalSize; > + PatchInfoBuffer[PatchCount - 1].AlignedSize = ALIGN_VALUE (TotalSize, > SIZE_1KB); > + TotalLoadSize += PatchInfoBuffer[PatchCount - 1].AlignedSize; > + } > + > + // > + // Process the next microcode patch > + // > + MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) > + MicrocodeEntryPoint) + TotalSize); } while (((UINTN) > + MicrocodeEntryPoint < MicrocodeEnd)); > + > + if (PatchCount != 0) { > + DEBUG (( > + DEBUG_INFO, > + "%a: 0x%x microcode patches will be loaded into memory, with size > 0x%x.\n", > + __FUNCTION__, PatchCount, TotalLoadSize > + )); > + > + LoadMicrocodePatchWorker (CpuMpData, PatchInfoBuffer, PatchCount, > + TotalLoadSize); } > + > +OnExit: > + if (PatchInfoBuffer != NULL) { > + FreePool (PatchInfoBuffer); > + } > + return; > +} > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c > b/UefiCpuPkg/Library/MpInitLib/MpLib.c > index d5077e080e..c72bf3c9ee 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c > @@ -628,10 +628,6 @@ ApWakeupFunction ( > ApTopOfStack = CpuMpData->Buffer + (ProcessorNumber + 1) * > CpuMpData->CpuApStackSize; > BistData = *(UINT32 *) ((UINTN) ApTopOfStack - sizeof (UINTN)); > // > - // Do some AP initialize sync > - // > - ApInitializeSync (CpuMpData); > - // > // CpuMpData->CpuData[0].VolatileRegisters is initialized based on BSP > environment, > // to initialize AP in InitConfig path. > // NOTE: IDTR.BASE stored in CpuMpData->CpuData[0].VolatileRegisters > points to a different IDT shared by all APs. > @@ -1615,7 +1611,6 @@ MpInitLibInitialize ( > UINTN ApResetVectorSize; > UINTN BackupBufferAddr; > UINTN ApIdtBase; > - VOID *MicrocodePatchInRam; > > OldCpuMpData = GetCpuMpDataFromGuidedHob (); > if (OldCpuMpData == NULL) { > @@ -1683,39 +1678,7 @@ MpInitLibInitialize ( > CpuMpData->SwitchBspFlag = FALSE; > CpuMpData->CpuData = (CPU_AP_DATA *) (CpuMpData + 1); > CpuMpData->CpuInfoInHob = (UINT64) (UINTN) (CpuMpData->CpuData > + MaxLogicalProcessorNumber); > - if (OldCpuMpData == NULL) { > - CpuMpData->MicrocodePatchRegionSize = PcdGet64 > (PcdCpuMicrocodePatchRegionSize); > - // > - // If platform has more than one CPU, relocate microcode to memory to > reduce > - // loading microcode time. > - // > - MicrocodePatchInRam = NULL; > - if (MaxLogicalProcessorNumber > 1) { > - MicrocodePatchInRam = AllocatePages ( > - EFI_SIZE_TO_PAGES ( > - (UINTN)CpuMpData->MicrocodePatchRegionSize > - ) > - ); > - } > - if (MicrocodePatchInRam == NULL) { > - // > - // there is only one processor, or no microcode patch is available, or > - // memory allocation failed > - // > - CpuMpData->MicrocodePatchAddress = PcdGet64 > (PcdCpuMicrocodePatchAddress); > - } else { > - // > - // there are multiple processors, and a microcode patch is available, > and > - // memory allocation succeeded > - // > - CopyMem ( > - MicrocodePatchInRam, > - (VOID *)(UINTN)PcdGet64 (PcdCpuMicrocodePatchAddress), > - (UINTN)CpuMpData->MicrocodePatchRegionSize > - ); > - CpuMpData->MicrocodePatchAddress = (UINTN)MicrocodePatchInRam; > - } > - }else { > + if (OldCpuMpData != NULL) { > CpuMpData->MicrocodePatchRegionSize = OldCpuMpData- > >MicrocodePatchRegionSize; > CpuMpData->MicrocodePatchAddress = OldCpuMpData- > >MicrocodePatchAddress; > } > @@ -1762,14 +1725,6 @@ MpInitLibInitialize ( > (UINT32 *)(MonitorBuffer + MonitorFilterSize * Index); > } > // > - // Load Microcode on BSP > - // > - MicrocodeDetect (CpuMpData, TRUE); > - // > - // Store BSP's MTRR setting > - // > - MtrrGetAllMtrrs (&CpuMpData->MtrrTable); > - // > // Enable the local APIC for Virtual Wire Mode. > // > ProgramVirtualWireMode (); > @@ -1781,6 +1736,11 @@ MpInitLibInitialize ( > // > CollectProcessorCount (CpuMpData); > } > + > + // > + // Load required microcode patches data into memory > + // > + LoadMicrocodePatch (CpuMpData); > } else { > // > // APs have been wakeup before, just get the CPU Information @@ - > 1788,7 +1748,6 @@ MpInitLibInitialize ( > // > CpuMpData->CpuCount = OldCpuMpData->CpuCount; > CpuMpData->BspNumber = OldCpuMpData->BspNumber; > - CpuMpData->InitFlag = ApInitReconfig; > CpuMpData->CpuInfoInHob = OldCpuMpData->CpuInfoInHob; > CpuInfoInHob = (CPU_INFO_IN_HOB *) (UINTN) CpuMpData- > >CpuInfoInHob; > for (Index = 0; Index < CpuMpData->CpuCount; Index++) { @@ -1797,21 > +1756,28 @@ MpInitLibInitialize ( > CpuMpData->CpuData[Index].ApFunction = 0; > CopyMem (&CpuMpData->CpuData[Index].VolatileRegisters, > &VolatileRegisters, sizeof (CPU_VOLATILE_REGISTERS)); > } > - if (MaxLogicalProcessorNumber > 1) { > - // > - // Wakeup APs to do some AP initialize sync > - // > - WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData, TRUE); > - // > - // Wait for all APs finished initialization > - // > - while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) { > - CpuPause (); > - } > - CpuMpData->InitFlag = ApInitDone; > - for (Index = 0; Index < CpuMpData->CpuCount; Index++) { > - SetApState (&CpuMpData->CpuData[Index], CpuStateIdle); > - } > + } > + > + // > + // Detect and apply Microcode on BSP > + // > + MicrocodeDetect (CpuMpData, TRUE); > + // > + // Store BSP's MTRR setting > + // > + MtrrGetAllMtrrs (&CpuMpData->MtrrTable); > + > + // > + // Wakeup APs to do some AP initialize sync (Microcode & MTRR) // > + if (CpuMpData->CpuCount > 1) { > + WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData, TRUE); > + while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) { > + CpuPause (); > + } > + > + for (Index = 0; Index < CpuMpData->CpuCount; Index++) { > + SetApState (&CpuMpData->CpuData[Index], CpuStateIdle); > } > } > > -- > 2.12.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#52621): https://edk2.groups.io/g/devel/message/52621 Mute This Topic: https://groups.io/mt/69341884/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
