Our current callback puts the APs in wait-for-SIPI state, by sending them
an INIT IPI. This works fine as long as the APs are not woken between
ExitBootServices() and the time the runtime OS boots the APs.
However, if a platform includes PiSmmCpuDxeSmm, and bases the runtime UEFI
services (like the variable services) on SMM and SMIs, then two problem
scenarios can be experienced:
(a) On virtual platforms, the APs can be (spuriously?) woken by SMIs,
between ExitBootServices() and the runtime OS's AP boot; for example
by variable service calls. In this case the APs have no code to
continue executing after the RSM instruction, which causes them to
re-set.
(b) On physical platforms, the APs might not wake from the wait-for-SIPI
state at all, causing performance (or functional) problems with
PiSmmCpuDxeSmm, dependent on the BSP/AP sync mode configured therein.
Implement the following logic instead:
- If there is no AP, there's no need for an ExitBootServices() callback.
- Otherwise, reboot all APs with a minimal real-mode stub. Each AP shall
bump a shared counter atomically at first, then spiral into an infinite
HLT loop. This occurs regardless of what each AP has been doing at that
moment.
- APs can wake from HLT due to an SMI easily.
- The BSP in the ExitBootServices() callback will wait until all APs have
moved on to the stub.
- Since we exit boot services time now with the original MP services
startup code abandoned by all APs, we can even allocate that startup
code in Boot Services Code type memory; only the new stub has to be
AcpiNVS.
Cc: Chen Fan <[email protected]>
Cc: Jeff Fan <[email protected]>
Cc: Jordan Justen <[email protected]>
Cc: Michael Kinney <[email protected]>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <[email protected]>
---
Notes:
So this would be option (2) from
<http://thread.gmane.org/gmane.comp.bios.edk2.devel/3357/focus=3403>.
With this patch in place, the AP reboots I had seen on TCG are gone.
UefiCpuPkg/CpuDxe/ApStartup.c | 2 +-
UefiCpuPkg/CpuDxe/CpuMp.c | 84 +++++++++++++++++++++++++++++++++++--------
2 files changed, 71 insertions(+), 15 deletions(-)
diff --git a/UefiCpuPkg/CpuDxe/ApStartup.c b/UefiCpuPkg/CpuDxe/ApStartup.c
index 78fb26f..b8d736f 100644
--- a/UefiCpuPkg/CpuDxe/ApStartup.c
+++ b/UefiCpuPkg/CpuDxe/ApStartup.c
@@ -382,7 +382,7 @@ PrepareAPStartupCode (
StartAddress = BASE_1MB;
Status = gBS->AllocatePages (
AllocateMaxAddress,
- EfiACPIMemoryNVS,
+ EfiBootServicesCode,
EFI_SIZE_TO_PAGES (sizeof (*StartupCode)),
&StartAddress
);
diff --git a/UefiCpuPkg/CpuDxe/CpuMp.c b/UefiCpuPkg/CpuDxe/CpuMp.c
index 04c2f1f..98976a1 100644
--- a/UefiCpuPkg/CpuDxe/CpuMp.c
+++ b/UefiCpuPkg/CpuDxe/CpuMp.c
@@ -30,6 +30,33 @@ VOID *mApStackStart = 0;
volatile BOOLEAN mAPsAlreadyInitFinished = FALSE;
volatile BOOLEAN mStopCheckAllAPsStatus = TRUE;
+#pragma pack (1)
+typedef struct {
+ UINT8 MovAx;
+ UINT16 AxValue;
+ UINT8 MovDsAx[2];
+ UINT8 LockedWordCounterIncrement[6];
+
+ UINT8 Cli;
+ UINT8 Hlt;
+ UINT8 JmpToCli[2];
+
+ UINT16 WordCounter;
+} CPU_AP_HLT_LOOP;
+#pragma pack ()
+
+STATIC CONST CPU_AP_HLT_LOOP mCpuApHltLoopTemplate = {
+ 0xB8, 0x1234, // mov ax,0x1234
+ { 0x8E, 0xD8 }, // mov ds,ax
+ { 0xF0, 0x3E, 0xFF, 0x06, 0x0F, 0x00 }, // lock inc word [ds:0xf]
+
+ 0xFA, // cli @ offset 0xb
+ 0xF4, // hlt
+ { 0xEB, 0xFC }, // jmp short 0xb
+
+ 0x0000 // WordCounter @ offset 0xf
+};
+
EFI_MP_SERVICES_PROTOCOL mMpServicesTemplate = {
GetNumberOfProcessors,
GetProcessorInfo,
@@ -1659,11 +1686,31 @@ ExitBootServicesCallback (
IN VOID *Context
)
{
+ CPU_AP_HLT_LOOP *CpuApHltLoop;
+ UINT16 ApCount;
+
+ CpuApHltLoop = Context;
+ CopyMem (CpuApHltLoop, &mCpuApHltLoopTemplate, sizeof mCpuApHltLoopTemplate);
+
//
- // Avoid APs access invalid buff datas which allocated by BootServices,
- // so we send INIT IPI to APs to let them wait for SIPI state.
+ // Point DS via AX to the page where the routine has been allocated.
//
- SendInitIpiAllExcludingSelf ();
+ CpuApHltLoop->AxValue = (UINT16)((UINTN)Context >> 4);
+ DEBUG ((EFI_D_VERBOSE, "%a: %a: HLT loop routine at %04x:%04x\n",
+ gEfiCallerBaseName, __FUNCTION__, CpuApHltLoop->AxValue, 0));
+
+ //
+ // Force all APs to execute the HLT loop. Wait until they all enter the code.
+ //
+ SendInitSipiSipiAllExcludingSelf ((UINT32)(UINTN)Context);
+
+ ApCount = (UINT16)(mMpSystemData.NumberOfProcessors - 1);
+ while (InterlockedCompareExchange16 (
+ &CpuApHltLoop->WordCounter,
+ ApCount,
+ ApCount) != ApCount) {
+ CpuPause();
+ }
}
/**
@@ -1691,9 +1738,10 @@ InitializeMpSupport (
VOID
)
{
- EFI_STATUS Status;
- MTRR_SETTINGS MtrrSettings;
- UINTN Timeout;
+ EFI_STATUS Status;
+ MTRR_SETTINGS MtrrSettings;
+ UINTN Timeout;
+ EFI_PHYSICAL_ADDRESS CpuApHltLoopAddress;
gMaxLogicalProcessorNumber = (UINTN) PcdGet32
(PcdCpuMaxLogicalProcessorNumber);
if (gMaxLogicalProcessorNumber < 1) {
@@ -1795,12 +1843,20 @@ InitializeMpSupport (
}
}
- Status = gBS->CreateEvent (
- EVT_SIGNAL_EXIT_BOOT_SERVICES,
- TPL_CALLBACK,
- ExitBootServicesCallback,
- NULL,
- &mExitBootServicesEvent
- );
- ASSERT_EFI_ERROR (Status);
+ if (mMpSystemData.NumberOfProcessors > 1) {
+ CpuApHltLoopAddress = BASE_1MB - 1;
+ Status = gBS->AllocatePages (AllocateMaxAddress, EfiACPIMemoryNVS,
+ EFI_SIZE_TO_PAGES (sizeof (CPU_AP_HLT_LOOP)),
+ &CpuApHltLoopAddress);
+ ASSERT_EFI_ERROR (Status);
+
+ Status = gBS->CreateEvent (
+ EVT_SIGNAL_EXIT_BOOT_SERVICES,
+ TPL_CALLBACK,
+ ExitBootServicesCallback,
+ (VOID *)(UINTN)CpuApHltLoopAddress,
+ &mExitBootServicesEvent
+ );
+ ASSERT_EFI_ERROR (Status);
+ }
}
--
1.8.3.1
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
