There is no functional changes, only extract DumpMicrocodeRevision since
only in PEI BSP will detect, apply microcode, and APs will sync
microcode.
Cc: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
---
UefiCpuPkg/Library/MpInitLib/Microcode.c | 91 +++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------
UefiCpuPkg/Library/MpInitLib/MpLib.c | 31 ++-----------------------------
UefiCpuPkg/Library/MpInitLib/MpLib.h | 31 ++++++++++---------------------
3 files changed, 57 insertions(+), 96 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index 11720560af..c0ca85543a 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -322,65 +322,64 @@ OnExit:
}
/**
- Shadow the required microcode patches data into memory.
+ Dump the microcode revision for each core.
- @param[in, out] CpuMpData The pointer to CPU MP Data structure.
-**/
+ @param[in] CpuMpData Pointer to CPU MP Data
+ **/
VOID
-ShadowMicrocodeUpdatePatch (
- IN OUT CPU_MP_DATA *CpuMpData
+DumpMicrocodeRevision (
+ IN CPU_MP_DATA *CpuMpData
)
{
- EFI_STATUS Status;
+ UINT32 ThreadId;
+ UINT32 ExpectedMicrocodeRevision;
+ CPU_INFO_IN_HOB *CpuInfoInHob;
+ UINTN Index;
- Status = PlatformShadowMicrocode (CpuMpData);
- if (EFI_ERROR (Status)) {
- ShadowMicrocodePatchByPcd (CpuMpData);
+ //
+ // Dump the microcode revision for each core.
+ //
+ DEBUG_CODE_BEGIN ();
+ CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
+ for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
+ GetProcessorLocationByApicId (CpuInfoInHob[Index].InitialApicId, NULL, NULL, &ThreadId);
+ if (ThreadId == 0) {
+ //
+ // MicrocodeDetect() loads microcode in first thread of each core, so,
+ // CpuMpData->CpuData[Index].MicrocodeEntryAddr is initialized only for first thread of each core.
+ //
+ ExpectedMicrocodeRevision = 0;
+ if (CpuMpData->CpuData[Index].MicrocodeEntryAddr != 0) {
+ ExpectedMicrocodeRevision = ((CPU_MICROCODE_HEADER *)(UINTN)CpuMpData->CpuData[Index].MicrocodeEntryAddr)->UpdateRevision;
+ }
+
+ DEBUG ((
+ DEBUG_INFO,
+ "CPU[%04d]: Microcode revision = %08x, expected = %08x\n",
+ Index,
+ CpuMpData->CpuData[Index].MicrocodeRevision,
+ ExpectedMicrocodeRevision
+ ));
+ }
}
+
+ DEBUG_CODE_END ();
}
/**
- Get the cached microcode patch base address and size from the microcode patch
- information cache HOB.
-
- @param[out] Address Base address of the microcode patches data.
- It will be updated if the microcode patch
- information cache HOB is found.
- @param[out] RegionSize Size of the microcode patches data.
- It will be updated if the microcode patch
- information cache HOB is found.
-
- @retval TRUE The microcode patch information cache HOB is found.
- @retval FALSE The microcode patch information cache HOB is not found.
+ Shadow the required microcode patches data into memory.
+ @param[in, out] CpuMpData The pointer to CPU MP Data structure.
**/
-BOOLEAN
-GetMicrocodePatchInfoFromHob (
- UINT64 *Address,
- UINT64 *RegionSize
+VOID
+ShadowMicrocodeUpdatePatch (
+ IN OUT CPU_MP_DATA *CpuMpData
)
{
- EFI_HOB_GUID_TYPE *GuidHob;
- EDKII_MICROCODE_PATCH_HOB *MicrocodePathHob;
+ EFI_STATUS Status;
- GuidHob = GetFirstGuidHob (&gEdkiiMicrocodePatchHobGuid);
- if (GuidHob == NULL) {
- DEBUG ((DEBUG_INFO, "%a: Microcode patch cache HOB is not found.\n", __func__));
- return FALSE;
+ Status = PlatformShadowMicrocode (CpuMpData);
+ if (EFI_ERROR (Status)) {
+ ShadowMicrocodePatchByPcd (CpuMpData);
}
-
- MicrocodePathHob = GET_GUID_HOB_DATA (GuidHob);
-
- *Address = MicrocodePathHob->MicrocodePatchAddress;
- *RegionSize = MicrocodePathHob->MicrocodePatchRegionSize;
-
- DEBUG ((
- DEBUG_INFO,
- "%a: MicrocodeBase = 0x%lx, MicrocodeSize = 0x%lx\n",
- __func__,
- *Address,
- *RegionSize
- ));
-
- return TRUE;
}
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 538095d3bb..2fd96bf0bc 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -2282,37 +2282,10 @@ MpInitLibInitialize (
}
}
- //
- // Dump the microcode revision for each core.
- //
- DEBUG_CODE_BEGIN ();
- UINT32 ThreadId;
- UINT32 ExpectedMicrocodeRevision;
-
- CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
- for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
- GetProcessorLocationByApicId (CpuInfoInHob[Index].InitialApicId, NULL, NULL, &ThreadId);
- if (ThreadId == 0) {
- //
- // MicrocodeDetect() loads microcode in first thread of each core, so,
- // CpuMpData->CpuData[Index].MicrocodeEntryAddr is initialized only for first thread of each core.
- //
- ExpectedMicrocodeRevision = 0;
- if (CpuMpData->CpuData[Index].MicrocodeEntryAddr != 0) {
- ExpectedMicrocodeRevision = ((CPU_MICROCODE_HEADER *)(UINTN)CpuMpData->CpuData[Index].MicrocodeEntryAddr)->UpdateRevision;
- }
-
- DEBUG ((
- DEBUG_INFO,
- "CPU[%04d]: Microcode revision = %08x, expected = %08x\n",
- Index,
- CpuMpData->CpuData[Index].MicrocodeRevision,
- ExpectedMicrocodeRevision
- ));
- }
+ if (MpHandOff == NULL) {
+ DumpMicrocodeRevision (CpuMpData);
}
- DEBUG_CODE_END ();
//
// Initialize global data for MP support
//
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 763db4963d..fd302e6845 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -735,34 +735,23 @@ MicrocodeDetect (
);
/**
- Shadow the required microcode patches data into memory.
+ Dump the microcode revision for each core.
- @param[in, out] CpuMpData The pointer to CPU MP Data structure.
-**/
+ @param[in] CpuMpData Pointer to CPU MP Data
+ **/
VOID
-ShadowMicrocodeUpdatePatch (
- IN OUT CPU_MP_DATA *CpuMpData
+DumpMicrocodeRevision (
+ IN CPU_MP_DATA *CpuMpData
);
/**
- Get the cached microcode patch base address and size from the microcode patch
- information cache HOB.
-
- @param[out] Address Base address of the microcode patches data.
- It will be updated if the microcode patch
- information cache HOB is found.
- @param[out] RegionSize Size of the microcode patches data.
- It will be updated if the microcode patch
- information cache HOB is found.
-
- @retval TRUE The microcode patch information cache HOB is found.
- @retval FALSE The microcode patch information cache HOB is not found.
+ Shadow the required microcode patches data into memory.
+ @param[in, out] CpuMpData The pointer to CPU MP Data structure.
**/
-BOOLEAN
-GetMicrocodePatchInfoFromHob (
- UINT64 *Address,
- UINT64 *RegionSize
+VOID
+ShadowMicrocodeUpdatePatch (
+ IN OUT CPU_MP_DATA *CpuMpData
);
/**
--
2.39.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111597): https://edk2.groups.io/g/devel/message/111597
Mute This Topic: https://groups.io/mt/102744601/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-> VOID
> -ShadowMicrocodeUpdatePatch (
> - IN OUT CPU_MP_DATA *CpuMpData
> +DumpMicrocodeRevision (
> + IN CPU_MP_DATA *CpuMpData
> )
> {
> - EFI_STATUS Status;
> + UINT32 ThreadId;
> + UINT32 ExpectedMicrocodeRevision;
> + CPU_INFO_IN_HOB *CpuInfoInHob;
> + UINTN Index;
When DEBUG_CODE is disabled, above local variable declarations will
cause build failure as the compiler only sees the local variable declarations
but cannot see any code that references them.
So, can you remove DEBUG_CODE_BEGIN/END() from DumpMicrocodeRevision()?
Instead, you call it using following way:
DEBUG_CODE (
DumpMicrocodeRevision ();
);
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111638): https://edk2.groups.io/g/devel/message/111638
Mute This Topic: https://groups.io/mt/102744601/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi Ray,
In v4, DEBUG_CODE_BEGIN/END() from DumpMicrocodeRevision() is removed, and calling is updated as:
DEBUG_CODE (
DumpMicrocodeRevision ();
);
Thanks a lot for the feedback!
Regards,
Yunahao
-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Thursday, November 23, 2023 9:24 AM
To: Xie, Yuanhao <yuanhao.xie@intel.com>; devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>; Laszlo Ersek <lersek@redhat.com>
Subject: RE: [Patch V3 3/3] UefiCpuPkg/MpInitLib: Extract Dump Microcode Revision as function.
> VOID
> -ShadowMicrocodeUpdatePatch (
> - IN OUT CPU_MP_DATA *CpuMpData
> +DumpMicrocodeRevision (
> + IN CPU_MP_DATA *CpuMpData
> )
> {
> - EFI_STATUS Status;
> + UINT32 ThreadId;
> + UINT32 ExpectedMicrocodeRevision;
> + CPU_INFO_IN_HOB *CpuInfoInHob;
> + UINTN Index;
When DEBUG_CODE is disabled, above local variable declarations will cause build failure as the compiler only sees the local variable declarations but cannot see any code that references them.
So, can you remove DEBUG_CODE_BEGIN/END() from DumpMicrocodeRevision()?
Instead, you call it using following way:
DEBUG_CODE (
DumpMicrocodeRevision ();
);
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111691): https://edk2.groups.io/g/devel/message/111691
Mute This Topic: https://groups.io/mt/102744601/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.