[PATCH 2/2] UefiCpuPkg/MpInitLib: resend the Start-up IPI when an AP misses it #VSTOR-146639

["Denis V. Lunev" <den@openvz.org>]

From: Denis V. Lunev <den@openvz.org>

WaitApWakeup spins until the AP clears its wakeup signal, with no
timeout and no way out. If the AP never consumes the Start-up IPI, the
BSP spins forever and the boot is dead.

A hypervisor can lose that IPI. Under KVM, INIT is latched while a
vCPU is in SMM but the Start-up IPI is discarded outright, so an AP
that takes an SMI just before the BSP broadcasts INIT-SIPI-SIPI is
left in wait-for-SIPI with nothing left to start it. The window is the
few instructions between an AP releasing the BSP at the end of the SMM
rendezvous and its own RSM, which a scheduled vCPU can be preempted
inside of. KVM holds that delivering the SIPI is software's job, so
the firmware has to cope.

Resend rather than spin. Address the AP individually so that APs
already running the procedure are not disturbed, and pass the reset
vector only on the paths that sent a Start-up IPI in the first place,
so the SEV-SNP and hand-off paths keep waiting as before. A Start-up
IPI is ignored unless the target is in wait-for-SIPI, so resending
costs nothing when the AP is merely slow.

The failure this fixes looks like a guest that never boots: firmware
starts the boot manager, paints nothing further, and one CPU spins
while the other APs sit idle.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Cc: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 85 +++++++++++++++++++++++++---
 UefiCpuPkg/Library/MpInitLib/MpLib.h |  2 +
 2 files changed, 80 insertions(+), 7 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 426d998b9a..87e8f824b6 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -942,16 +942,63 @@ DxeApEntryPoint (
   ApWakeupFunction (CpuMpData, ProcessorNumber);
 }
 
+/**
+  Calculate timeout value and return the current performance counter value.
+
+  @param[in]  TimeoutInMicroseconds  The timeout value in microseconds.
+  @param[out] CurrentTime            Returns the current value of the performance counter.
+
+  @return Expected timestamp counter for timeout, or 0 if TimeoutInMicroseconds is 0.
+**/
+UINT64
+CalculateTimeout (
+  IN  UINTN   TimeoutInMicroseconds,
+  OUT UINT64  *CurrentTime
+  );
+
+/**
+  Checks whether timeout expires.
+
+  @param[in, out]  PreviousTime   On input, the requested time. On output, the elapsed time.
+  @param[in]       TotalTime      The total amount of elapsed time.
+  @param[in]       Timeout        The timeout ticks, 0 means infinity.
+
+  @retval TRUE   A timeout occurred.
+  @retval FALSE  No timeout occurred.
+**/
+BOOLEAN
+CheckTimeout (
+  IN OUT UINT64  *PreviousTime,
+  IN     UINT64  *TotalTime,
+  IN     UINT64  Timeout
+  );
+
 /**
   Wait for AP wakeup and write AP start-up signal till AP is waken up.
 
   @param[in] ApStartupSignalBuffer  Pointer to AP wakeup signal
+  @param[in] ApicId                 Local APIC ID of the AP being woken
+  @param[in] StartupRoutine         AP reset vector, or 0 if the AP was not
+                                    woken by INIT-SIPI-SIPI and no Start-up
+                                    IPI may be resent
 **/
 VOID
 WaitApWakeup (
-  IN volatile UINT32  *ApStartupSignalBuffer
+  IN volatile UINT32  *ApStartupSignalBuffer,
+  IN UINT32           ApicId,
+  IN UINT32           StartupRoutine
   )
 {
+  UINT64  CurrentTime;
+  UINT64  TotalTime;
+  UINT64  Timeout;
+
+  Timeout = CalculateTimeout (
+              (StartupRoutine != 0) ? AP_SIPI_RESEND_TIMEOUT_US : 0,
+              &CurrentTime
+              );
+  TotalTime = 0;
+
   //
   // If AP is waken up, StartupApSignal should be cleared.
   // Otherwise, write StartupApSignal again till AP waken up.
@@ -962,6 +1009,12 @@ WaitApWakeup (
            WAKEUP_AP_SIGNAL
            ) != 0)
   {
+    if (CheckTimeout (&CurrentTime, &TotalTime, Timeout)) {
+      SendStartupIpi (ApicId, StartupRoutine);
+      Timeout   = CalculateTimeout (AP_SIPI_RESEND_TIMEOUT_US, &CurrentTime);
+      TotalTime = 0;
+    }
+
     CpuPause ();
   }
 }
@@ -1256,8 +1309,10 @@ WakeUpAP (
   CPU_AP_DATA                    *CpuData;
   BOOLEAN                        ResetVectorRequired;
   CPU_INFO_IN_HOB                *CpuInfoInHob;
+  UINT32                         SipiVector;
 
   CpuMpData->FinishedCount = 0;
+  SipiVector               = 0;
   ResetVectorRequired      = FALSE;
 
   if (CpuMpData->WakeUpByInitSipiSipi ||
@@ -1320,13 +1375,14 @@ WakeUpAP (
       if (CanUseSevSnpCreateAP (CpuMpData)) {
         SevSnpCreateAP (CpuMpData, -1);
       } else {
+        SipiVector = (UINT32)ExchangeInfo->BufferStart;
         if ((CpuMpData->InitFlag == ApInitConfig) && FixedPcdGetBool (PcdFirstTimeWakeUpAPsBySipi)) {
           //
           // SIPI can be used for the first time wake up after reset to reduce boot time.
           //
-          SendStartupIpiAllExcludingSelf ((UINT32)ExchangeInfo->BufferStart);
+          SendStartupIpiAllExcludingSelf (SipiVector);
         } else {
-          SendInitSipiSipiAllExcludingSelf ((UINT32)ExchangeInfo->BufferStart);
+          SendInitSipiSipiAllExcludingSelf (SipiVector);
         }
       }
     }
@@ -1398,10 +1454,15 @@ WakeUpAP (
       //
       // Wait all APs waken up if this is not the 1st broadcast of SIPI
       //
+      CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
       for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
         CpuData = &CpuMpData->CpuData[Index];
         if (Index != CpuMpData->BspNumber) {
-          WaitApWakeup (CpuData->StartupApSignal);
+          WaitApWakeup (
+            CpuData->StartupApSignal,
+            CpuInfoInHob[Index].ApicId,
+            SipiVector
+            );
         }
       }
     }
@@ -1430,9 +1491,10 @@ WakeUpAP (
       if (CanUseSevSnpCreateAP (CpuMpData)) {
         SevSnpCreateAP (CpuMpData, (INTN)ProcessorNumber);
       } else {
+        SipiVector = (UINT32)ExchangeInfo->BufferStart;
         SendInitSipiSipi (
           CpuInfoInHob[ProcessorNumber].ApicId,
-          (UINT32)ExchangeInfo->BufferStart
+          SipiVector
           );
       }
     }
@@ -1440,7 +1502,12 @@ WakeUpAP (
     //
     // Wait specified AP waken up
     //
-    WaitApWakeup (CpuData->StartupApSignal);
+    CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
+    WaitApWakeup (
+      CpuData->StartupApSignal,
+      CpuInfoInHob[ProcessorNumber].ApicId,
+      SipiVector
+      );
   }
 
   if (ResetVectorRequired) {
@@ -2017,7 +2084,11 @@ SwitchApContext (
   {
     for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
       if (MpHandOff->ProcessorIndex + Index != BspNumber) {
-        WaitApWakeup ((UINT32 *)(UINTN)(MpHandOff->Info[Index].StartupSignalAddress));
+        WaitApWakeup (
+          (UINT32 *)(UINTN)(MpHandOff->Info[Index].StartupSignalAddress),
+          0,
+          0
+          );
       }
     }
   }
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 5d10516cd8..94bda7f1ea 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -44,6 +44,8 @@
 #include "MpHandOff.h"
 
 #define WAKEUP_AP_SIGNAL  SIGNATURE_32 ('S', 'T', 'A', 'P')
+
+#define AP_SIPI_RESEND_TIMEOUT_US  10000
 //
 // To trigger the start-up signal, BSP writes the specified
 // StartupSignalValue to the StartupSignalAddress of each processor.
-- 
2.53.0