Skip to content

Commit

Permalink
UefiCpuPkg/MpInitLib: avoid printing debug messages in AP
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-by: Eric Dong <[email protected]>
Acked-by: Laszlo Ersek <[email protected]>
Cc: Rahul Kumar <[email protected]>
  • Loading branch information
niruiyu committed Mar 17, 2021
1 parent e4ff377 commit 6d39bd0
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 10 deletions.
11 changes: 1 addition & 10 deletions UefiCpuPkg/Library/MpInitLib/Microcode.c
Original file line number Diff line number Diff line change
Expand Up @@ -315,17 +315,8 @@ MicrocodeDetect (
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 ();
}

/**
Expand Down
25 changes: 25 additions & 0 deletions UefiCpuPkg/Library/MpInitLib/MpLib.c
Original file line number Diff line number Diff line change
Expand Up @@ -2135,6 +2135,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
//
Expand Down
1 change: 1 addition & 0 deletions UefiCpuPkg/Library/MpInitLib/MpLib.h
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ typedef struct {
UINT32 ProcessorSignature;
UINT8 PlatformId;
UINT64 MicrocodeEntryAddr;
UINT32 MicrocodeRevision;
} CPU_AP_DATA;

//
Expand Down

0 comments on commit 6d39bd0

Please sign in to comment.