* [PATCH v5 0/6] Microcode related optimizations
@ 2019-12-31 0:49 Wu, Hao A
2019-12-31 0:49 ` [PATCH v5 1/6] UefiCpuPkg/MpInitLib: Collect processors' CPUID & Platform ID info Wu, Hao A
` (6 more replies)
0 siblings, 7 replies; 14+ messages in thread
From: Wu, Hao A @ 2019-12-31 0:49 UTC (permalink / raw)
To: devel
Cc: Hao A Wu, Eric Dong, Ray Ni, Laszlo Ersek, Star Zeng, Siyuan Fu,
Michael D Kinney
Series is also available at:
https://github.com/hwu25/edk2/tree/mpinitlib_opt_v5
V5 changes:
A. For patch 2, address a typo to resolve enlarging the microcode patch
information buffer too early when it is not full;
B. For patch 4, relocate the logic of storing detected microcode patch
before the application of microcode patch.
V4 history:
A. For patch 2, keep the calling sequence of functions:
MicrocodeDetect() and MtrrGetAllMtrrs() unchanged for BSP.
B. For patch 2, in function LoadMicrocodePatch(), add the missing logic
that handles the Extended Signature Table of a microcode patch.
V3 history:
For patch 3, correct license information for newly added header file.
V2 history:
A. For patch 2, rename function parameters and variables to better reflect
their usage;
B. For patch 2, remove unneeded check in LoadMicrocodePatchWorker();
C. For patch 3, rename a couple of fields in the HOB structure;
D. For patch 3, update the 'ProcessorSpecificPatchOffset' field to point
to the microcode patch header instead of microcode patch data;
E. Add a new patch 5 to address an issue that certain fields in the
CPU_MP_DATA structure cannot be passed from PEI phase to DXE phase;
F. Add a new patch 6 to remove the redundant microcode related fields in
the CPU_MP_DATA structure.
V1 history:
This series will introduce a couple of optimizations to the MpInitLib with
regard to microcode:
A. Reduce the copy size when loading the microcode patches data from flash
into memory;
B. Produce a HOB to contain microcode patches information for subsequent
consumers of the microcode patches during the boot flow.
Uni-test done for the series:
A. System boot to OS/Shell successfully on a real platform;
B. Add debug message to verify the same microcode patch is applied to each
processor before and after the series.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Hao A Wu (6):
UefiCpuPkg/MpInitLib: Collect processors' CPUID & Platform ID info
UefiCpuPkg/MpInitLib: Reduce the size when loading microcode patches
UefiCpuPkg: Add definitions for EDKII microcode patch HOB
UefiCpuPkg/MpInitLib: Produce EDKII microcode patch HOB
UefiCpuPkg/MpInitLib: Relocate microcode patch fields in CPU_MP_DATA
UefiCpuPkg/MpInitLib: Remove redundant microcode fields in CPU_MP_DATA
UefiCpuPkg/UefiCpuPkg.dec | 3 +
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 1 +
UefiCpuPkg/Include/Guid/MicrocodePatchHob.h | 44 +++
UefiCpuPkg/Library/MpInitLib/MpLib.h | 59 +++-
UefiCpuPkg/Library/MpInitLib/Microcode.c | 351 ++++++++++++++++++--
UefiCpuPkg/Library/MpInitLib/MpLib.c | 110 +++---
UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 55 +++
7 files changed, 513 insertions(+), 110 deletions(-)
create mode 100644 UefiCpuPkg/Include/Guid/MicrocodePatchHob.h
--
2.12.0.windows.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v5 1/6] UefiCpuPkg/MpInitLib: Collect processors' CPUID & Platform ID info
2019-12-31 0:49 [PATCH v5 0/6] Microcode related optimizations Wu, Hao A
@ 2019-12-31 0:49 ` Wu, Hao A
2019-12-31 0:49 ` [PATCH v5 2/6] UefiCpuPkg/MpInitLib: Reduce the size when loading microcode patches Wu, Hao A
` (5 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Wu, Hao A @ 2019-12-31 0:49 UTC (permalink / raw)
To: devel
Cc: Hao A Wu, Eric Dong, Ray Ni, Laszlo Ersek, Star Zeng, Siyuan Fu,
Michael D Kinney
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2429
This commit will collect the CPUID and Platform ID information for each
processor within system. They will be stored in the CPU_AP_DATA structure.
These information will be used in the next commit to decide whether a
microcode patch will be loaded into memory.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
---
UefiCpuPkg/Library/MpInitLib/MpLib.h | 2 ++
UefiCpuPkg/Library/MpInitLib/MpLib.c | 14 +++++++++++++-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 8fa07b12c5..4440dc2701 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -122,6 +122,8 @@ typedef struct {
UINT64 CurrentTime;
UINT64 TotalTime;
EFI_EVENT WaitEvent;
+ UINT32 ProcessorSignature;
+ UINT8 PlatformId;
} CPU_AP_DATA;
//
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index d32adf0780..d5077e080e 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -548,7 +548,8 @@ InitializeApData (
IN UINT64 ApTopOfStack
)
{
- CPU_INFO_IN_HOB *CpuInfoInHob;
+ CPU_INFO_IN_HOB *CpuInfoInHob;
+ MSR_IA32_PLATFORM_ID_REGISTER PlatformIdMsr;
CpuInfoInHob = (CPU_INFO_IN_HOB *) (UINTN) CpuMpData->CpuInfoInHob;
CpuInfoInHob[ProcessorNumber].InitialApicId = GetInitialApicId ();
@@ -559,6 +560,17 @@ InitializeApData (
CpuMpData->CpuData[ProcessorNumber].Waiting = FALSE;
CpuMpData->CpuData[ProcessorNumber].CpuHealthy = (BistData == 0) ? TRUE : FALSE;
+ PlatformIdMsr.Uint64 = AsmReadMsr64 (MSR_IA32_PLATFORM_ID);
+ CpuMpData->CpuData[ProcessorNumber].PlatformId = (UINT8) PlatformIdMsr.Bits.PlatformId;
+
+ AsmCpuid (
+ CPUID_VERSION_INFO,
+ &CpuMpData->CpuData[ProcessorNumber].ProcessorSignature,
+ NULL,
+ NULL,
+ NULL
+ );
+
InitializeSpinLock(&CpuMpData->CpuData[ProcessorNumber].ApLock);
SetApState (&CpuMpData->CpuData[ProcessorNumber], CpuStateIdle);
}
--
2.12.0.windows.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v5 2/6] UefiCpuPkg/MpInitLib: Reduce the size when loading microcode patches
2019-12-31 0:49 [PATCH v5 0/6] Microcode related optimizations Wu, Hao A
2019-12-31 0:49 ` [PATCH v5 1/6] UefiCpuPkg/MpInitLib: Collect processors' CPUID & Platform ID info Wu, Hao A
@ 2019-12-31 0:49 ` Wu, Hao A
2019-12-31 1:17 ` Dong, Eric
2019-12-31 0:49 ` [PATCH v5 3/6] UefiCpuPkg: Add definitions for EDKII microcode patch HOB Wu, Hao A
` (4 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Wu, Hao A @ 2019-12-31 0:49 UTC (permalink / raw)
To: devel
Cc: Hao A Wu, Eric Dong, Ray Ni, Laszlo Ersek, Star Zeng, Siyuan Fu,
Michael D Kinney
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.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Hao A Wu <hao.a.wu@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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v5 3/6] UefiCpuPkg: Add definitions for EDKII microcode patch HOB
2019-12-31 0:49 [PATCH v5 0/6] Microcode related optimizations Wu, Hao A
2019-12-31 0:49 ` [PATCH v5 1/6] UefiCpuPkg/MpInitLib: Collect processors' CPUID & Platform ID info Wu, Hao A
2019-12-31 0:49 ` [PATCH v5 2/6] UefiCpuPkg/MpInitLib: Reduce the size when loading microcode patches Wu, Hao A
@ 2019-12-31 0:49 ` Wu, Hao A
2019-12-31 0:49 ` [PATCH v5 4/6] UefiCpuPkg/MpInitLib: Produce " Wu, Hao A
` (3 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Wu, Hao A @ 2019-12-31 0:49 UTC (permalink / raw)
To: devel
Cc: Hao A Wu, Eric Dong, Ray Ni, Laszlo Ersek, Star Zeng, Siyuan Fu,
Michael D Kinney
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2430
This commit will add the definitions for EDKII microcode patch HOB.
The intention of adding this HOB is to provide a scheme to store the below
information:
A. The base address and size of the microcode patches that are being
loaded (from flash) into memory;
B. The information of detected microcode patch for each processor within
the system.
The producer of the HOB will be the UefiCpuPkg/MpInitLib (where the load,
detect and apply of the microcode happen). The consumer of the HOB can be
modules that want to detect/apply the microcode patch by themselves again
later during the boot flow.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
---
UefiCpuPkg/UefiCpuPkg.dec | 3 ++
UefiCpuPkg/Include/Guid/MicrocodePatchHob.h | 44 ++++++++++++++++++++
2 files changed, 47 insertions(+)
diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index 797f948631..45b267ac61 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -63,6 +63,9 @@ [Guids]
## Include/Guid/CpuFeaturesInitDone.h
gEdkiiCpuFeaturesInitDoneGuid = { 0xc77c3a41, 0x61ab, 0x4143, { 0x98, 0x3e, 0x33, 0x39, 0x28, 0x6, 0x28, 0xe5 }}
+ ## Include/Guid/MicrocodePatchHob.h
+ gEdkiiMicrocodePatchHobGuid = { 0xd178f11d, 0x8716, 0x418e, { 0xa1, 0x31, 0x96, 0x7d, 0x2a, 0xc4, 0x28, 0x43 }}
+
[Protocols]
## Include/Protocol/SmmCpuService.h
gEfiSmmCpuServiceProtocolGuid = { 0x1d202cab, 0xc8ab, 0x4d5c, { 0x94, 0xf7, 0x3c, 0xfc, 0xc0, 0xd3, 0xd3, 0x35 }}
diff --git a/UefiCpuPkg/Include/Guid/MicrocodePatchHob.h b/UefiCpuPkg/Include/Guid/MicrocodePatchHob.h
new file mode 100644
index 0000000000..2d307fbffb
--- /dev/null
+++ b/UefiCpuPkg/Include/Guid/MicrocodePatchHob.h
@@ -0,0 +1,44 @@
+/** @file
+ The microcode patch HOB is used to store the information of:
+ A. Base address and size of the loaded microcode patches data;
+ B. Detected microcode patch for each processor within system.
+
+ Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _MICROCODE_PATCH_HOB_H_
+#define _MICROCODE_PATCH_HOB_H_
+
+extern EFI_GUID gEdkiiMicrocodePatchHobGuid;
+
+//
+// The EDKII microcode patch HOB will be produced by MpInitLib and it can be
+// consumed by modules that want to detect/apply microcode patches.
+//
+typedef struct {
+ //
+ // The base address of the microcode patches data after being loaded into
+ // memory.
+ //
+ UINT64 MicrocodePatchAddress;
+ //
+ // The total size of the loaded microcode patches.
+ //
+ UINT64 MicrocodePatchRegionSize;
+ //
+ // The number of processors within the system.
+ //
+ UINT32 ProcessorCount;
+ //
+ // An array with 'ProcessorCount' elements that stores the offset (with
+ // regard to 'MicrocodePatchAddress') of the detected microcode patch
+ // (including the CPU_MICROCODE_HEADER data structure) for each processor.
+ // If no microcode patch is detected for certain processor, the relating
+ // element will be set to MAX_UINT64.
+ //
+ UINT64 ProcessorSpecificPatchOffset[0];
+} EDKII_MICROCODE_PATCH_HOB;
+
+#endif
--
2.12.0.windows.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v5 4/6] UefiCpuPkg/MpInitLib: Produce EDKII microcode patch HOB
2019-12-31 0:49 [PATCH v5 0/6] Microcode related optimizations Wu, Hao A
` (2 preceding siblings ...)
2019-12-31 0:49 ` [PATCH v5 3/6] UefiCpuPkg: Add definitions for EDKII microcode patch HOB Wu, Hao A
@ 2019-12-31 0:49 ` Wu, Hao A
2019-12-31 1:17 ` [edk2-devel] " Dong, Eric
2019-12-31 0:49 ` [PATCH v5 5/6] UefiCpuPkg/MpInitLib: Relocate microcode patch fields in CPU_MP_DATA Wu, Hao A
` (2 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Wu, Hao A @ 2019-12-31 0:49 UTC (permalink / raw)
To: devel
Cc: Hao A Wu, Eric Dong, Ray Ni, Laszlo Ersek, Star Zeng, Siyuan Fu,
Michael D Kinney
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2430
This commit will update the MpInitLib to:
A. Collect the base address and size information after microcode patches
being loaded into memory;
B. Collect the detected microcode patch for each processor within system;
C. Based on the collected information, produce the EDKII microcode patch
HOB.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
---
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 1 +
UefiCpuPkg/Library/MpInitLib/MpLib.h | 24 +++++++--
UefiCpuPkg/Library/MpInitLib/Microcode.c | 20 +++++--
UefiCpuPkg/Library/MpInitLib/MpLib.c | 8 ++-
UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 55 ++++++++++++++++++++
5 files changed, 100 insertions(+), 8 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
index 1538185ef9..326703cc9a 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
@@ -63,3 +63,4 @@ [Pcd]
[Guids]
gEdkiiS3SmmInitDoneGuid
+ gEdkiiMicrocodePatchHobGuid
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 56b0df664a..885656900c 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -138,6 +138,7 @@ typedef struct {
EFI_EVENT WaitEvent;
UINT32 ProcessorSignature;
UINT8 PlatformId;
+ UINT64 MicrocodeEntryAddr;
} CPU_AP_DATA;
//
@@ -580,13 +581,15 @@ CheckAndUpdateApsStatus (
/**
Detect whether specified processor can find matching microcode patch and load it.
- @param[in] CpuMpData The pointer to CPU MP Data structure.
- @param[in] IsBspCallIn Indicate whether the caller is BSP or not.
+ @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
+ minus 1.
**/
VOID
MicrocodeDetect (
IN CPU_MP_DATA *CpuMpData,
- IN BOOLEAN IsBspCallIn
+ IN UINTN ProcessorNumber
);
/**
@@ -619,5 +622,20 @@ EnableDebugAgent (
VOID
);
+/**
+ Find the current Processor number by APIC ID.
+
+ @param[in] CpuMpData Pointer to PEI CPU MP Data
+ @param[out] ProcessorNumber Return the pocessor number found
+
+ @retval EFI_SUCCESS ProcessorNumber is found and returned.
+ @retval EFI_NOT_FOUND ProcessorNumber is not found.
+**/
+EFI_STATUS
+GetProcessorNumber (
+ IN CPU_MP_DATA *CpuMpData,
+ OUT UINTN *ProcessorNumber
+ );
+
#endif
diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index 330fd99623..4162b4a8dc 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -65,13 +65,15 @@ GetCurrentMicrocodeSignature (
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] IsBspCallIn Indicate whether the caller is BSP or not.
+ @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
+ minus 1.
**/
VOID
MicrocodeDetect (
IN CPU_MP_DATA *CpuMpData,
- IN BOOLEAN IsBspCallIn
+ IN UINTN ProcessorNumber
)
{
UINT32 ExtendedTableLength;
@@ -93,6 +95,7 @@ MicrocodeDetect (
MSR_IA32_PLATFORM_ID_REGISTER PlatformIdMsr;
UINT32 ProcessorFlags;
UINT32 ThreadId;
+ BOOLEAN IsBspCallIn;
//
// set ProcessorFlags to suppress incorrect compiler/analyzer warnings
@@ -107,6 +110,7 @@ MicrocodeDetect (
}
CurrentRevision = GetCurrentMicrocodeSignature ();
+ IsBspCallIn = (ProcessorNumber == (UINTN)CpuMpData->BspNumber) ? TRUE : FALSE;
if (CurrentRevision != 0 && !IsBspCallIn) {
//
// Skip loading microcode if it has been loaded successfully
@@ -295,6 +299,16 @@ MicrocodeDetect (
} while (((UINTN) MicrocodeEntryPoint < MicrocodeEnd));
Done:
+ if (LatestRevision != 0) {
+ //
+ // Save the detected microcode patch entry address (including the
+ // microcode patch header) for each processor.
+ // It will be used when building the microcode patch cache HOB.
+ //
+ CpuMpData->CpuData[ProcessorNumber].MicrocodeEntryAddr =
+ (UINTN) MicrocodeData - sizeof (CPU_MICROCODE_HEADER);
+ }
+
if (LatestRevision > CurrentRevision) {
//
// BIOS only authenticate updates that contain a numerically larger revision
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index c72bf3c9ee..e611a8ca40 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -399,12 +399,16 @@ ApInitializeSync (
)
{
CPU_MP_DATA *CpuMpData;
+ UINTN ProcessorNumber;
+ EFI_STATUS Status;
CpuMpData = (CPU_MP_DATA *) Buffer;
+ Status = GetProcessorNumber (CpuMpData, &ProcessorNumber);
+ ASSERT_EFI_ERROR (Status);
//
// Load microcode on AP
//
- MicrocodeDetect (CpuMpData, FALSE);
+ MicrocodeDetect (CpuMpData, ProcessorNumber);
//
// Sync BSP's MTRR table to AP
//
@@ -1761,7 +1765,7 @@ MpInitLibInitialize (
//
// Detect and apply Microcode on BSP
//
- MicrocodeDetect (CpuMpData, TRUE);
+ MicrocodeDetect (CpuMpData, CpuMpData->BspNumber);
//
// Store BSP's MTRR setting
//
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
index 3999603c3e..06e3f5d0d3 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
@@ -9,6 +9,7 @@
#include "MpLib.h"
#include <Library/PeiServicesLib.h>
#include <Guid/S3SmmInitDone.h>
+#include <Guid/MicrocodePatchHob.h>
/**
S3 SMM Init Done notification function.
@@ -291,6 +292,59 @@ CheckAndUpdateApsStatus (
}
/**
+ Build the microcode patch HOB that contains the base address and size of the
+ microcode patch stored in the memory.
+
+ @param[in] CpuMpData Pointer to the CPU_MP_DATA structure.
+
+**/
+VOID
+BuildMicrocodeCacheHob (
+ IN CPU_MP_DATA *CpuMpData
+ )
+{
+ EDKII_MICROCODE_PATCH_HOB *MicrocodeHob;
+ UINTN HobDataLength;
+ UINT32 Index;
+
+ HobDataLength = sizeof (EDKII_MICROCODE_PATCH_HOB) +
+ sizeof (UINT64) * CpuMpData->CpuCount;
+
+ MicrocodeHob = AllocatePool (HobDataLength);
+ if (MicrocodeHob == NULL) {
+ ASSERT (FALSE);
+ return;
+ }
+
+ //
+ // Store the information of the memory region that holds the microcode patches.
+ //
+ MicrocodeHob->MicrocodePatchAddress = CpuMpData->MicrocodePatchAddress;
+ MicrocodeHob->MicrocodePatchRegionSize = CpuMpData->MicrocodePatchRegionSize;
+
+ //
+ // Store the detected microcode patch for each processor as well.
+ //
+ MicrocodeHob->ProcessorCount = CpuMpData->CpuCount;
+ for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
+ if (CpuMpData->CpuData[Index].MicrocodeEntryAddr != 0) {
+ MicrocodeHob->ProcessorSpecificPatchOffset[Index] =
+ CpuMpData->CpuData[Index].MicrocodeEntryAddr - CpuMpData->MicrocodePatchAddress;
+ } else {
+ MicrocodeHob->ProcessorSpecificPatchOffset[Index] = MAX_UINT64;
+ }
+ }
+
+ BuildGuidDataHob (
+ &gEdkiiMicrocodePatchHobGuid,
+ MicrocodeHob,
+ HobDataLength
+ );
+
+ return;
+}
+
+/**
Initialize global data for MP support.
@param[in] CpuMpData The pointer to CPU MP Data structure.
@@ -302,6 +356,7 @@ InitMpGlobalData (
{
EFI_STATUS Status;
+ BuildMicrocodeCacheHob (CpuMpData);
SaveCpuMpData (CpuMpData);
///
--
2.12.0.windows.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v5 5/6] UefiCpuPkg/MpInitLib: Relocate microcode patch fields in CPU_MP_DATA
2019-12-31 0:49 [PATCH v5 0/6] Microcode related optimizations Wu, Hao A
` (3 preceding siblings ...)
2019-12-31 0:49 ` [PATCH v5 4/6] UefiCpuPkg/MpInitLib: Produce " Wu, Hao A
@ 2019-12-31 0:49 ` Wu, Hao A
2019-12-31 0:49 ` [PATCH v5 6/6] UefiCpuPkg/MpInitLib: Remove redundant microcode " Wu, Hao A
2020-01-02 2:39 ` [edk2-devel] [PATCH v5 0/6] Microcode related optimizations Ni, Ray
6 siblings, 0 replies; 14+ messages in thread
From: Wu, Hao A @ 2019-12-31 0:49 UTC (permalink / raw)
To: devel
Cc: Hao A Wu, Eric Dong, Ray Ni, Laszlo Ersek, Star Zeng, Siyuan Fu,
Michael D Kinney
The below 2 microcode patch related fields in structure CPU_MP_DATA:
UINT64 MicrocodePatchAddress;
UINT64 MicrocodePatchRegionSize;
They will be passed from PEI phase and be reused DXE phase.
Previously, these 2 fields were placed after some fields with type
'UINTN', this will lead to different field offset in different
architecture for them.
This commit will move them before the fields with different size in
different architecture to ensure they can be properly used in DXE phase.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
---
UefiCpuPkg/Library/MpInitLib/MpLib.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 885656900c..5f50e79744 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -217,6 +217,8 @@ struct _CPU_MP_DATA {
UINT64 CpuInfoInHob;
UINT32 CpuCount;
UINT32 BspNumber;
+ UINT64 MicrocodePatchAddress;
+ UINT64 MicrocodePatchRegionSize;
//
// The above fields data will be passed from PEI to DXE
// Please make sure the fields offset same in the different
@@ -260,8 +262,6 @@ struct _CPU_MP_DATA {
UINT8 Vector;
BOOLEAN PeriodicMode;
BOOLEAN TimerInterruptState;
- UINT64 MicrocodePatchAddress;
- UINT64 MicrocodePatchRegionSize;
UINT32 ProcessorSignature;
UINT32 ProcessorFlags;
--
2.12.0.windows.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v5 6/6] UefiCpuPkg/MpInitLib: Remove redundant microcode fields in CPU_MP_DATA
2019-12-31 0:49 [PATCH v5 0/6] Microcode related optimizations Wu, Hao A
` (4 preceding siblings ...)
2019-12-31 0:49 ` [PATCH v5 5/6] UefiCpuPkg/MpInitLib: Relocate microcode patch fields in CPU_MP_DATA Wu, Hao A
@ 2019-12-31 0:49 ` Wu, Hao A
2020-01-02 2:39 ` [edk2-devel] [PATCH v5 0/6] Microcode related optimizations Ni, Ray
6 siblings, 0 replies; 14+ messages in thread
From: Wu, Hao A @ 2019-12-31 0:49 UTC (permalink / raw)
To: devel
Cc: Hao A Wu, Eric Dong, Ray Ni, Laszlo Ersek, Star Zeng, Siyuan Fu,
Michael D Kinney
Previous commits have introduced below fields in structure CPU_AP_DATA:
UINT32 ProcessorSignature;
UINT8 PlatformId;
UINT64 MicrocodeEntryAddr;
which store the information of:
A. CPUID
B. Platform ID
C. Detected microcode patch entry address (including the microcode patch
header)
for each processor within system.
Therefore, the below fields in structure CPU_MP_DATA:
UINT32 ProcessorSignature;
UINT32 ProcessorFlags;
UINT64 MicrocodeDataAddress;
UINT32 MicrocodeRevision;
which store the BSP's information of:
A. CPUID
B. Platform ID
C. The address and revision of detected microcode patch
are redundant and can be removed.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
---
UefiCpuPkg/Library/MpInitLib/MpLib.h | 5 --
UefiCpuPkg/Library/MpInitLib/Microcode.c | 51 ++++++--------------
2 files changed, 14 insertions(+), 42 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 5f50e79744..6609c958ce 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -263,11 +263,6 @@ struct _CPU_MP_DATA {
BOOLEAN PeriodicMode;
BOOLEAN TimerInterruptState;
- UINT32 ProcessorSignature;
- UINT32 ProcessorFlags;
- UINT64 MicrocodeDataAddress;
- UINT32 MicrocodeRevision;
-
//
// Whether need to use Init-Sipi-Sipi to wake up the APs.
// Two cases need to set this value to TRUE. One is in HLT
diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index 4162b4a8dc..3da5bfb9cf 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -85,6 +85,7 @@ MicrocodeDetect (
UINTN Index;
UINT8 PlatformId;
CPUID_VERSION_INFO_EAX Eax;
+ CPU_AP_DATA *CpuData;
UINT32 CurrentRevision;
UINT32 LatestRevision;
UINTN TotalSize;
@@ -92,16 +93,9 @@ MicrocodeDetect (
UINT32 InCompleteCheckSum32;
BOOLEAN CorrectMicrocode;
VOID *MicrocodeData;
- MSR_IA32_PLATFORM_ID_REGISTER PlatformIdMsr;
- UINT32 ProcessorFlags;
UINT32 ThreadId;
BOOLEAN IsBspCallIn;
- //
- // set ProcessorFlags to suppress incorrect compiler/analyzer warnings
- //
- ProcessorFlags = 0;
-
if (CpuMpData->MicrocodePatchRegionSize == 0) {
//
// There is no microcode patches
@@ -127,28 +121,25 @@ MicrocodeDetect (
}
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);
-
- //
- // 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;
+ Eax.Uint32 = CpuMpData->CpuData[ProcessorNumber].ProcessorSignature;
+ PlatformId = CpuMpData->CpuData[ProcessorNumber].PlatformId;
//
// Check whether AP has same processor with BSP.
// If yes, direct use microcode info saved by BSP.
//
if (!IsBspCallIn) {
- if ((CpuMpData->ProcessorSignature == Eax.Uint32) &&
- (CpuMpData->ProcessorFlags & (1 << PlatformId)) != 0) {
- MicrocodeData = (VOID *)(UINTN) CpuMpData->MicrocodeDataAddress;
- LatestRevision = CpuMpData->MicrocodeRevision;
- goto Done;
+ //
+ // Get the CPU data for BSP
+ //
+ 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;
}
}
@@ -216,7 +207,6 @@ MicrocodeDetect (
CheckSum32 += MicrocodeEntryPoint->Checksum;
if (CheckSum32 == 0) {
CorrectMicrocode = TRUE;
- ProcessorFlags = MicrocodeEntryPoint->ProcessorFlags;
}
} else if ((MicrocodeEntryPoint->DataSize != 0) &&
(MicrocodeEntryPoint->UpdateRevision > LatestRevision)) {
@@ -260,7 +250,6 @@ MicrocodeDetect (
// Find one
//
CorrectMicrocode = TRUE;
- ProcessorFlags = ExtendedTable->ProcessorFlag;
break;
}
}
@@ -332,18 +321,6 @@ Done:
ReleaseSpinLock(&CpuMpData->MpLock);
}
}
-
- if (IsBspCallIn && (LatestRevision != 0)) {
- //
- // Save BSP processor info and microcode info for later AP use.
- //
- CpuMpData->ProcessorSignature = Eax.Uint32;
- CpuMpData->ProcessorFlags = ProcessorFlags;
- CpuMpData->MicrocodeDataAddress = (UINTN) MicrocodeData;
- CpuMpData->MicrocodeRevision = LatestRevision;
- DEBUG ((DEBUG_INFO, "BSP Microcode:: signature [0x%08x], ProcessorFlags [0x%08x], \
- MicroData [0x%08x], Revision [0x%08x]\n", Eax.Uint32, ProcessorFlags, (UINTN) MicrocodeData, LatestRevision));
- }
}
/**
--
2.12.0.windows.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [PATCH v5 4/6] UefiCpuPkg/MpInitLib: Produce EDKII microcode patch HOB
2019-12-31 0:49 ` [PATCH v5 4/6] UefiCpuPkg/MpInitLib: Produce " Wu, Hao A
@ 2019-12-31 1:17 ` Dong, Eric
0 siblings, 0 replies; 14+ messages in thread
From: Dong, Eric @ 2019-12-31 1:17 UTC (permalink / raw)
To: devel@edk2.groups.io, Wu, Hao A
Cc: Ni, Ray, Laszlo Ersek, Zeng, Star, Fu, Siyuan, Kinney, Michael D
Reviewed-by: Eric Dong <eric.dong@intel.com>
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Wu, Hao A
> Sent: Tuesday, December 31, 2019 8:49 AM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni,
> Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Zeng, Star
> <star.zeng@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Kinney, Michael
> D <michael.d.kinney@intel.com>
> Subject: [edk2-devel] [PATCH v5 4/6] UefiCpuPkg/MpInitLib: Produce EDKII
> microcode patch HOB
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2430
>
> This commit will update the MpInitLib to:
>
> A. Collect the base address and size information after microcode patches
> being loaded into memory;
> B. Collect the detected microcode patch for each processor within system; C.
> Based on the collected information, produce the EDKII microcode patch
> HOB.
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
> ---
> UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 1 +
> UefiCpuPkg/Library/MpInitLib/MpLib.h | 24 +++++++--
> UefiCpuPkg/Library/MpInitLib/Microcode.c | 20 +++++--
> UefiCpuPkg/Library/MpInitLib/MpLib.c | 8 ++-
> UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 55 ++++++++++++++++++++
> 5 files changed, 100 insertions(+), 8 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> index 1538185ef9..326703cc9a 100644
> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> @@ -63,3 +63,4 @@ [Pcd]
>
> [Guids]
> gEdkiiS3SmmInitDoneGuid
> + gEdkiiMicrocodePatchHobGuid
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index 56b0df664a..885656900c 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -138,6 +138,7 @@ typedef struct {
> EFI_EVENT WaitEvent;
> UINT32 ProcessorSignature;
> UINT8 PlatformId;
> + UINT64 MicrocodeEntryAddr;
> } CPU_AP_DATA;
>
> //
> @@ -580,13 +581,15 @@ CheckAndUpdateApsStatus (
> /**
> Detect whether specified processor can find matching microcode patch and
> load it.
>
> - @param[in] CpuMpData The pointer to CPU MP Data structure.
> - @param[in] IsBspCallIn Indicate whether the caller is BSP or not.
> + @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
> + minus 1.
> **/
> VOID
> MicrocodeDetect (
> IN CPU_MP_DATA *CpuMpData,
> - IN BOOLEAN IsBspCallIn
> + IN UINTN ProcessorNumber
> );
>
> /**
> @@ -619,5 +622,20 @@ EnableDebugAgent (
> VOID
> );
>
> +/**
> + Find the current Processor number by APIC ID.
> +
> + @param[in] CpuMpData Pointer to PEI CPU MP Data
> + @param[out] ProcessorNumber Return the pocessor number found
> +
> + @retval EFI_SUCCESS ProcessorNumber is found and returned.
> + @retval EFI_NOT_FOUND ProcessorNumber is not found.
> +**/
> +EFI_STATUS
> +GetProcessorNumber (
> + IN CPU_MP_DATA *CpuMpData,
> + OUT UINTN *ProcessorNumber
> + );
> +
> #endif
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> index 330fd99623..4162b4a8dc 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> @@ -65,13 +65,15 @@ GetCurrentMicrocodeSignature (
> 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] IsBspCallIn Indicate whether the caller is BSP or not.
> + @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
> + minus 1.
> **/
> VOID
> MicrocodeDetect (
> IN CPU_MP_DATA *CpuMpData,
> - IN BOOLEAN IsBspCallIn
> + IN UINTN ProcessorNumber
> )
> {
> UINT32 ExtendedTableLength;
> @@ -93,6 +95,7 @@ MicrocodeDetect (
> MSR_IA32_PLATFORM_ID_REGISTER PlatformIdMsr;
> UINT32 ProcessorFlags;
> UINT32 ThreadId;
> + BOOLEAN IsBspCallIn;
>
> //
> // set ProcessorFlags to suppress incorrect compiler/analyzer warnings @@
> -107,6 +110,7 @@ MicrocodeDetect (
> }
>
> CurrentRevision = GetCurrentMicrocodeSignature ();
> + IsBspCallIn = (ProcessorNumber == (UINTN)CpuMpData->BspNumber) ?
> TRUE : FALSE;
> if (CurrentRevision != 0 && !IsBspCallIn) {
> //
> // Skip loading microcode if it has been loaded successfully @@ -295,6
> +299,16 @@ MicrocodeDetect (
> } while (((UINTN) MicrocodeEntryPoint < MicrocodeEnd));
>
> Done:
> + if (LatestRevision != 0) {
> + //
> + // Save the detected microcode patch entry address (including the
> + // microcode patch header) for each processor.
> + // It will be used when building the microcode patch cache HOB.
> + //
> + CpuMpData->CpuData[ProcessorNumber].MicrocodeEntryAddr =
> + (UINTN) MicrocodeData - sizeof (CPU_MICROCODE_HEADER); }
> +
> if (LatestRevision > CurrentRevision) {
> //
> // BIOS only authenticate updates that contain a numerically larger revision
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index c72bf3c9ee..e611a8ca40 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -399,12 +399,16 @@ ApInitializeSync (
> )
> {
> CPU_MP_DATA *CpuMpData;
> + UINTN ProcessorNumber;
> + EFI_STATUS Status;
>
> CpuMpData = (CPU_MP_DATA *) Buffer;
> + Status = GetProcessorNumber (CpuMpData, &ProcessorNumber);
> + ASSERT_EFI_ERROR (Status);
> //
> // Load microcode on AP
> //
> - MicrocodeDetect (CpuMpData, FALSE);
> + MicrocodeDetect (CpuMpData, ProcessorNumber);
> //
> // Sync BSP's MTRR table to AP
> //
> @@ -1761,7 +1765,7 @@ MpInitLibInitialize (
> //
> // Detect and apply Microcode on BSP
> //
> - MicrocodeDetect (CpuMpData, TRUE);
> + MicrocodeDetect (CpuMpData, CpuMpData->BspNumber);
> //
> // Store BSP's MTRR setting
> //
> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> index 3999603c3e..06e3f5d0d3 100644
> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> @@ -9,6 +9,7 @@
> #include "MpLib.h"
> #include <Library/PeiServicesLib.h>
> #include <Guid/S3SmmInitDone.h>
> +#include <Guid/MicrocodePatchHob.h>
>
> /**
> S3 SMM Init Done notification function.
> @@ -291,6 +292,59 @@ CheckAndUpdateApsStatus ( }
>
> /**
> + Build the microcode patch HOB that contains the base address and size
> + of the microcode patch stored in the memory.
> +
> + @param[in] CpuMpData Pointer to the CPU_MP_DATA structure.
> +
> +**/
> +VOID
> +BuildMicrocodeCacheHob (
> + IN CPU_MP_DATA *CpuMpData
> + )
> +{
> + EDKII_MICROCODE_PATCH_HOB *MicrocodeHob;
> + UINTN HobDataLength;
> + UINT32 Index;
> +
> + HobDataLength = sizeof (EDKII_MICROCODE_PATCH_HOB) +
> + sizeof (UINT64) * CpuMpData->CpuCount;
> +
> + MicrocodeHob = AllocatePool (HobDataLength); if (MicrocodeHob ==
> + NULL) {
> + ASSERT (FALSE);
> + return;
> + }
> +
> + //
> + // Store the information of the memory region that holds the microcode
> patches.
> + //
> + MicrocodeHob->MicrocodePatchAddress = CpuMpData-
> >MicrocodePatchAddress;
> + MicrocodeHob->MicrocodePatchRegionSize =
> + CpuMpData->MicrocodePatchRegionSize;
> +
> + //
> + // Store the detected microcode patch for each processor as well.
> + //
> + MicrocodeHob->ProcessorCount = CpuMpData->CpuCount; for (Index = 0;
> + Index < CpuMpData->CpuCount; Index++) {
> + if (CpuMpData->CpuData[Index].MicrocodeEntryAddr != 0) {
> + MicrocodeHob->ProcessorSpecificPatchOffset[Index] =
> + CpuMpData->CpuData[Index].MicrocodeEntryAddr - CpuMpData-
> >MicrocodePatchAddress;
> + } else {
> + MicrocodeHob->ProcessorSpecificPatchOffset[Index] = MAX_UINT64;
> + }
> + }
> +
> + BuildGuidDataHob (
> + &gEdkiiMicrocodePatchHobGuid,
> + MicrocodeHob,
> + HobDataLength
> + );
> +
> + return;
> +}
> +
> +/**
> Initialize global data for MP support.
>
> @param[in] CpuMpData The pointer to CPU MP Data structure.
> @@ -302,6 +356,7 @@ InitMpGlobalData (
> {
> EFI_STATUS Status;
>
> + BuildMicrocodeCacheHob (CpuMpData);
> SaveCpuMpData (CpuMpData);
>
> ///
> --
> 2.12.0.windows.1
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 2/6] UefiCpuPkg/MpInitLib: Reduce the size when loading microcode patches
2019-12-31 0:49 ` [PATCH v5 2/6] UefiCpuPkg/MpInitLib: Reduce the size when loading microcode patches Wu, Hao A
@ 2019-12-31 1:17 ` Dong, Eric
0 siblings, 0 replies; 14+ messages in thread
From: Dong, Eric @ 2019-12-31 1:17 UTC (permalink / raw)
To: Wu, Hao A, devel@edk2.groups.io
Cc: Ni, Ray, Laszlo Ersek, Zeng, Star, Fu, Siyuan, Kinney, Michael D
Reviewed-by: Eric Dong <eric.dong@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.wu@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni,
> Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Zeng, Star
> <star.zeng@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Kinney, Michael
> D <michael.d.kinney@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.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Signed-off-by: Hao A Wu <hao.a.wu@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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [PATCH v5 0/6] Microcode related optimizations
2019-12-31 0:49 [PATCH v5 0/6] Microcode related optimizations Wu, Hao A
` (5 preceding siblings ...)
2019-12-31 0:49 ` [PATCH v5 6/6] UefiCpuPkg/MpInitLib: Remove redundant microcode " Wu, Hao A
@ 2020-01-02 2:39 ` Ni, Ray
2020-01-02 3:12 ` Wu, Hao A
6 siblings, 1 reply; 14+ messages in thread
From: Ni, Ray @ 2020-01-02 2:39 UTC (permalink / raw)
To: devel@edk2.groups.io, Wu, Hao A
Cc: Dong, Eric, Laszlo Ersek, Zeng, Star, Fu, Siyuan,
Kinney, Michael D
Reviewed-by: Ray Ni <ray.ni@intel.com>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Hao
> A
> Sent: Tuesday, December 31, 2019 8:49 AM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni,
> Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Zeng, Star
> <star.zeng@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Kinney, Michael
> D <michael.d.kinney@intel.com>
> Subject: [edk2-devel] [PATCH v5 0/6] Microcode related optimizations
>
> Series is also available at:
> https://github.com/hwu25/edk2/tree/mpinitlib_opt_v5
>
> V5 changes:
> A. For patch 2, address a typo to resolve enlarging the microcode patch
> information buffer too early when it is not full;
> B. For patch 4, relocate the logic of storing detected microcode patch
> before the application of microcode patch.
>
> V4 history:
> A. For patch 2, keep the calling sequence of functions:
> MicrocodeDetect() and MtrrGetAllMtrrs() unchanged for BSP.
> B. For patch 2, in function LoadMicrocodePatch(), add the missing logic
> that handles the Extended Signature Table of a microcode patch.
>
>
> V3 history:
> For patch 3, correct license information for newly added header file.
>
> V2 history:
> A. For patch 2, rename function parameters and variables to better reflect
> their usage;
> B. For patch 2, remove unneeded check in LoadMicrocodePatchWorker();
> C. For patch 3, rename a couple of fields in the HOB structure;
> D. For patch 3, update the 'ProcessorSpecificPatchOffset' field to point
> to the microcode patch header instead of microcode patch data;
> E. Add a new patch 5 to address an issue that certain fields in the
> CPU_MP_DATA structure cannot be passed from PEI phase to DXE phase;
> F. Add a new patch 6 to remove the redundant microcode related fields in
> the CPU_MP_DATA structure.
>
> V1 history:
> This series will introduce a couple of optimizations to the MpInitLib with
> regard to microcode:
>
> A. Reduce the copy size when loading the microcode patches data from flash
> into memory;
> B. Produce a HOB to contain microcode patches information for subsequent
> consumers of the microcode patches during the boot flow.
>
> Uni-test done for the series:
> A. System boot to OS/Shell successfully on a real platform;
> B. Add debug message to verify the same microcode patch is applied to each
> processor before and after the series.
>
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>
> Hao A Wu (6):
> UefiCpuPkg/MpInitLib: Collect processors' CPUID & Platform ID info
> UefiCpuPkg/MpInitLib: Reduce the size when loading microcode patches
> UefiCpuPkg: Add definitions for EDKII microcode patch HOB
> UefiCpuPkg/MpInitLib: Produce EDKII microcode patch HOB
> UefiCpuPkg/MpInitLib: Relocate microcode patch fields in CPU_MP_DATA
> UefiCpuPkg/MpInitLib: Remove redundant microcode fields in
> CPU_MP_DATA
>
> UefiCpuPkg/UefiCpuPkg.dec | 3 +
> UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 1 +
> UefiCpuPkg/Include/Guid/MicrocodePatchHob.h | 44 +++
> UefiCpuPkg/Library/MpInitLib/MpLib.h | 59 +++-
> UefiCpuPkg/Library/MpInitLib/Microcode.c | 351 ++++++++++++++++++-
> -
> UefiCpuPkg/Library/MpInitLib/MpLib.c | 110 +++---
> UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 55 +++
> 7 files changed, 513 insertions(+), 110 deletions(-)
> create mode 100644 UefiCpuPkg/Include/Guid/MicrocodePatchHob.h
>
> --
> 2.12.0.windows.1
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [PATCH v5 0/6] Microcode related optimizations
2020-01-02 2:39 ` [edk2-devel] [PATCH v5 0/6] Microcode related optimizations Ni, Ray
@ 2020-01-02 3:12 ` Wu, Hao A
2020-01-02 15:07 ` Laszlo Ersek
0 siblings, 1 reply; 14+ messages in thread
From: Wu, Hao A @ 2020-01-02 3:12 UTC (permalink / raw)
To: Ni, Ray, Dong, Eric, Laszlo Ersek, devel@edk2.groups.io
Cc: Zeng, Star, Fu, Siyuan, Kinney, Michael D
Ray and Eric, thanks a lot for the review.
Hello Laszlo,
It seems that you are out of office, I plan to push the series without your
comments first. If you have feedbacks/comments with regard to the series, I
will follow up for the potential refine/revert of the series.
Best Regards,
Hao Wu
> -----Original Message-----
> From: Ni, Ray
> Sent: Thursday, January 02, 2020 10:40 AM
> To: devel@edk2.groups.io; Wu, Hao A
> Cc: Dong, Eric; Laszlo Ersek; Zeng, Star; Fu, Siyuan; Kinney, Michael D
> Subject: RE: [edk2-devel] [PATCH v5 0/6] Microcode related optimizations
>
> Reviewed-by: Ray Ni <ray.ni@intel.com>
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu,
> Hao
> > A
> > Sent: Tuesday, December 31, 2019 8:49 AM
> > To: devel@edk2.groups.io
> > Cc: Wu, Hao A <hao.a.wu@intel.com>; Dong, Eric <eric.dong@intel.com>;
> Ni,
> > Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Zeng, Star
> > <star.zeng@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Kinney,
> Michael
> > D <michael.d.kinney@intel.com>
> > Subject: [edk2-devel] [PATCH v5 0/6] Microcode related optimizations
> >
> > Series is also available at:
> > https://github.com/hwu25/edk2/tree/mpinitlib_opt_v5
> >
> > V5 changes:
> > A. For patch 2, address a typo to resolve enlarging the microcode patch
> > information buffer too early when it is not full;
> > B. For patch 4, relocate the logic of storing detected microcode patch
> > before the application of microcode patch.
> >
> > V4 history:
> > A. For patch 2, keep the calling sequence of functions:
> > MicrocodeDetect() and MtrrGetAllMtrrs() unchanged for BSP.
> > B. For patch 2, in function LoadMicrocodePatch(), add the missing logic
> > that handles the Extended Signature Table of a microcode patch.
> >
> >
> > V3 history:
> > For patch 3, correct license information for newly added header file.
> >
> > V2 history:
> > A. For patch 2, rename function parameters and variables to better reflect
> > their usage;
> > B. For patch 2, remove unneeded check in LoadMicrocodePatchWorker();
> > C. For patch 3, rename a couple of fields in the HOB structure;
> > D. For patch 3, update the 'ProcessorSpecificPatchOffset' field to point
> > to the microcode patch header instead of microcode patch data;
> > E. Add a new patch 5 to address an issue that certain fields in the
> > CPU_MP_DATA structure cannot be passed from PEI phase to DXE phase;
> > F. Add a new patch 6 to remove the redundant microcode related fields in
> > the CPU_MP_DATA structure.
> >
> > V1 history:
> > This series will introduce a couple of optimizations to the MpInitLib with
> > regard to microcode:
> >
> > A. Reduce the copy size when loading the microcode patches data from
> flash
> > into memory;
> > B. Produce a HOB to contain microcode patches information for
> subsequent
> > consumers of the microcode patches during the boot flow.
> >
> > Uni-test done for the series:
> > A. System boot to OS/Shell successfully on a real platform;
> > B. Add debug message to verify the same microcode patch is applied to
> each
> > processor before and after the series.
> >
> >
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Siyuan Fu <siyuan.fu@intel.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> >
> > Hao A Wu (6):
> > UefiCpuPkg/MpInitLib: Collect processors' CPUID & Platform ID info
> > UefiCpuPkg/MpInitLib: Reduce the size when loading microcode patches
> > UefiCpuPkg: Add definitions for EDKII microcode patch HOB
> > UefiCpuPkg/MpInitLib: Produce EDKII microcode patch HOB
> > UefiCpuPkg/MpInitLib: Relocate microcode patch fields in CPU_MP_DATA
> > UefiCpuPkg/MpInitLib: Remove redundant microcode fields in
> > CPU_MP_DATA
> >
> > UefiCpuPkg/UefiCpuPkg.dec | 3 +
> > UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 1 +
> > UefiCpuPkg/Include/Guid/MicrocodePatchHob.h | 44 +++
> > UefiCpuPkg/Library/MpInitLib/MpLib.h | 59 +++-
> > UefiCpuPkg/Library/MpInitLib/Microcode.c | 351
> ++++++++++++++++++-
> > -
> > UefiCpuPkg/Library/MpInitLib/MpLib.c | 110 +++---
> > UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 55 +++
> > 7 files changed, 513 insertions(+), 110 deletions(-)
> > create mode 100644 UefiCpuPkg/Include/Guid/MicrocodePatchHob.h
> >
> > --
> > 2.12.0.windows.1
> >
> >
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [PATCH v5 0/6] Microcode related optimizations
2020-01-02 3:12 ` Wu, Hao A
@ 2020-01-02 15:07 ` Laszlo Ersek
2020-01-03 20:09 ` Laszlo Ersek
0 siblings, 1 reply; 14+ messages in thread
From: Laszlo Ersek @ 2020-01-02 15:07 UTC (permalink / raw)
To: Wu, Hao A, Ni, Ray, Dong, Eric, devel@edk2.groups.io
Cc: Zeng, Star, Fu, Siyuan, Kinney, Michael D
On 01/02/20 04:12, Wu, Hao A wrote:
> Ray and Eric, thanks a lot for the review.
>
> Hello Laszlo,
> It seems that you are out of office, I plan to push the series without your
> comments first. If you have feedbacks/comments with regard to the series, I
> will follow up for the potential refine/revert of the series.
Just returned today. I'm now working to regain control of my mailbox.
The above procedure works fine for me; in fact I would have suggested it
myself. In case I encounter a regression with OVMF, I'll report it.
Thank you!
Laszlo
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [PATCH v5 0/6] Microcode related optimizations
2020-01-02 15:07 ` Laszlo Ersek
@ 2020-01-03 20:09 ` Laszlo Ersek
2020-01-06 1:36 ` Wu, Hao A
0 siblings, 1 reply; 14+ messages in thread
From: Laszlo Ersek @ 2020-01-03 20:09 UTC (permalink / raw)
To: Wu, Hao A, Ni, Ray, Dong, Eric, devel@edk2.groups.io
Cc: Zeng, Star, Fu, Siyuan, Kinney, Michael D
Hello Hao,
On 01/02/20 16:07, Laszlo Ersek wrote:
> On 01/02/20 04:12, Wu, Hao A wrote:
>> Ray and Eric, thanks a lot for the review.
>>
>> Hello Laszlo,
>> It seems that you are out of office, I plan to push the series without your
>> comments first. If you have feedbacks/comments with regard to the series, I
>> will follow up for the potential refine/revert of the series.
>
> Just returned today. I'm now working to regain control of my mailbox.
>
> The above procedure works fine for me; in fact I would have suggested it
> myself. In case I encounter a regression with OVMF, I'll report it.
I've now run some regression tests [*], with OVMF built at commit
b948a496150f ("UefiCpuPkg/PiSmmCpuDxeSmm: Pre-allocate PROCEDURE_TOKEN
buffer", 2020-01-02). At that stage, the tree contains your present
patch series too (which ends at commit fd30b0070773).
[*] Including, but a bit more than,
<https://github.com/tianocore/tianocore.github.io/wiki/Testing-SMM-with-QEMU,-KVM-and-libvirt#tests-to-perform-in-the-installed-guest-fedora-26-guest>.
Basically my usual Linux guest tests.
I haven't noticed any regressions.
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [PATCH v5 0/6] Microcode related optimizations
2020-01-03 20:09 ` Laszlo Ersek
@ 2020-01-06 1:36 ` Wu, Hao A
0 siblings, 0 replies; 14+ messages in thread
From: Wu, Hao A @ 2020-01-06 1:36 UTC (permalink / raw)
To: devel@edk2.groups.io, lersek@redhat.com, Ni, Ray, Dong, Eric
Cc: Zeng, Star, Fu, Siyuan, Kinney, Michael D
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Saturday, January 04, 2020 4:09 AM
> To: Wu, Hao A; Ni, Ray; Dong, Eric; devel@edk2.groups.io
> Cc: Zeng, Star; Fu, Siyuan; Kinney, Michael D
> Subject: Re: [edk2-devel] [PATCH v5 0/6] Microcode related optimizations
>
> Hello Hao,
>
> On 01/02/20 16:07, Laszlo Ersek wrote:
> > On 01/02/20 04:12, Wu, Hao A wrote:
> >> Ray and Eric, thanks a lot for the review.
> >>
> >> Hello Laszlo,
> >> It seems that you are out of office, I plan to push the series without your
> >> comments first. If you have feedbacks/comments with regard to the
> series, I
> >> will follow up for the potential refine/revert of the series.
> >
> > Just returned today. I'm now working to regain control of my mailbox.
> >
> > The above procedure works fine for me; in fact I would have suggested it
> > myself. In case I encounter a regression with OVMF, I'll report it.
>
> I've now run some regression tests [*], with OVMF built at commit
> b948a496150f ("UefiCpuPkg/PiSmmCpuDxeSmm: Pre-allocate
> PROCEDURE_TOKEN
> buffer", 2020-01-02). At that stage, the tree contains your present
> patch series too (which ends at commit fd30b0070773).
>
> [*] Including, but a bit more than,
> <https://github.com/tianocore/tianocore.github.io/wiki/Testing-SMM-with-
> QEMU,-KVM-and-libvirt#tests-to-perform-in-the-installed-guest-fedora-26-
> guest>.
> Basically my usual Linux guest tests.
>
>
> I haven't noticed any regressions.
Got it, thanks a lot for the testing effort.
Best Regards,
Hao Wu
>
> Thanks!
> Laszlo
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-01-06 1:36 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-12-31 0:49 [PATCH v5 0/6] Microcode related optimizations Wu, Hao A
2019-12-31 0:49 ` [PATCH v5 1/6] UefiCpuPkg/MpInitLib: Collect processors' CPUID & Platform ID info Wu, Hao A
2019-12-31 0:49 ` [PATCH v5 2/6] UefiCpuPkg/MpInitLib: Reduce the size when loading microcode patches Wu, Hao A
2019-12-31 1:17 ` Dong, Eric
2019-12-31 0:49 ` [PATCH v5 3/6] UefiCpuPkg: Add definitions for EDKII microcode patch HOB Wu, Hao A
2019-12-31 0:49 ` [PATCH v5 4/6] UefiCpuPkg/MpInitLib: Produce " Wu, Hao A
2019-12-31 1:17 ` [edk2-devel] " Dong, Eric
2019-12-31 0:49 ` [PATCH v5 5/6] UefiCpuPkg/MpInitLib: Relocate microcode patch fields in CPU_MP_DATA Wu, Hao A
2019-12-31 0:49 ` [PATCH v5 6/6] UefiCpuPkg/MpInitLib: Remove redundant microcode " Wu, Hao A
2020-01-02 2:39 ` [edk2-devel] [PATCH v5 0/6] Microcode related optimizations Ni, Ray
2020-01-02 3:12 ` Wu, Hao A
2020-01-02 15:07 ` Laszlo Ersek
2020-01-03 20:09 ` Laszlo Ersek
2020-01-06 1:36 ` Wu, Hao A
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox