summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRay Ni <ray.ni@intel.com>2021-03-12 19:39:38 +0800
committermergify[bot] <37929162+mergify[bot]@users.noreply.github.com>2021-03-17 12:39:31 +0000
commit030ba3097a6e7d08b99f8a9d19a322f61409c1f6 (patch)
tree70a4ec201e56e9eb1afeaac19124e7892fbcf528
parente4ff3773b711527ff46759759f34ea1cb9ff2a45 (diff)
downloadedk2-030ba3097a6e7d08b99f8a9d19a322f61409c1f6.zip
edk2-030ba3097a6e7d08b99f8a9d19a322f61409c1f6.tar.gz
edk2-030ba3097a6e7d08b99f8a9d19a322f61409c1f6.tar.bz2
UefiCpuPkg/MpInitLib: avoid printing debug messages in AP
MpInitLib contains a function MicrocodeDetect() which is called by all threads as an AP procedure. Today this function contains below code: if (CurrentRevision != LatestRevision) { AcquireSpinLock(&CpuMpData->MpLock); DEBUG (( EFI_D_ERROR, "Updated microcode signature [0x%08x] does not match \ loaded microcode signature [0x%08x]\n", CurrentRevision, LatestRevision )); ReleaseSpinLock(&CpuMpData->MpLock); } When the if-check is passed, the code may call into PEI services: 1. AcquireSpinLock When the PcdSpinTimeout is not 0, TimerLib GetPerformanceCounterProperties() is called. And some of the TimerLib implementations would get the information cached in HOB. But AP procedure cannot call PEI services to retrieve the HOB list. 2. DEBUG Certain DebugLib relies on ReportStatusCode services and the ReportStatusCode PPI is retrieved through the PEI services. DebugLibSerialPort should be used. But when SerialPortLib is implemented to depend on PEI services, even using DebugLibSerialPort can still cause AP calls PEI services resulting hang. It causes a lot of debugging effort on the platform side. There are 2 options to fix the problem: 1. make sure platform DSC chooses the proper DebugLib and set the PcdSpinTimeout to 0. So that AcquireSpinLock and DEBUG don't call PEI services. 2. remove the AcquireSpinLock and DEBUG call from the procedure. Option #2 is preferred because it's not practical to ask every platform DSC to be written properly. Following option #2, there are two sub-options: 2.A. Just remove the if-check. 2.B. Capture the CurrentRevision and ExpectedRevision in the memory for each AP and print them together from BSP. The patch follows option 2.B. Signed-off-by: Ray Ni <ray.ni@intel.com> Reviewed-by: Eric Dong <eric.dong@intel.com> Acked-by: Laszlo Ersek <lersek@redhat.com> Cc: Rahul Kumar <rahul1.kumar@intel.com>
-rw-r--r--UefiCpuPkg/Library/MpInitLib/Microcode.c11
-rw-r--r--UefiCpuPkg/Library/MpInitLib/MpLib.c25
-rw-r--r--UefiCpuPkg/Library/MpInitLib/MpLib.h1
3 files changed, 27 insertions, 10 deletions
diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index 1562959..297c2ab 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -315,17 +315,8 @@ Done:
MSR_IA32_BIOS_UPDT_TRIG,
(UINT64) (UINTN) MicrocodeData
);
- //
- // Get and check new microcode signature
- //
- CurrentRevision = GetCurrentMicrocodeSignature ();
- if (CurrentRevision != LatestRevision) {
- AcquireSpinLock(&CpuMpData->MpLock);
- DEBUG ((EFI_D_ERROR, "Updated microcode signature [0x%08x] does not match \
- loaded microcode signature [0x%08x]\n", CurrentRevision, LatestRevision));
- ReleaseSpinLock(&CpuMpData->MpLock);
- }
}
+ CpuMpData->CpuData[ProcessorNumber].MicrocodeRevision = GetCurrentMicrocodeSignature ();
}
/**
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 5040053..3d94597 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -2136,6 +2136,31 @@ MpInitLibInitialize (
}
//
+ // Dump the microcode revision for each core.
+ //
+ DEBUG_CODE (
+ 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
+ ));
+ }
+ }
+ );
+ //
// Initialize global data for MP support
//
InitMpGlobalData (CpuMpData);
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 0bd6038..66f9eb2 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -144,6 +144,7 @@ typedef struct {
UINT32 ProcessorSignature;
UINT8 PlatformId;
UINT64 MicrocodeEntryAddr;
+ UINT32 MicrocodeRevision;
} CPU_AP_DATA;
//