[PATCH 0/2] UefiCpuPkg/MpInitLib: recover from a lost Start-up IPI #VSTOR-146639

[PATCH 0/2] UefiCpuPkg/MpInitLib: recover from a lost Start-up IPI #VSTOR-146639

[Denis V. Lunev]

WaitApWakeup() spins until an AP clears its wakeup signal, with no timeout
and no way out. A hypervisor can lose the Start-up IPI: KVM latches INIT
while the target vCPU is in SMM but discards the SIPI outright. The AP is
left in wait-for-SIPI, the BSP spins forever, and the guest never boots.

Analysis of the live hung guest is in the bug.

Patch 1 exports SendStartupIpi() from LocalApicLib, so one AP can be poked
without an INIT that would reset an AP already running the procedure.
Patch 2 makes WaitApWakeup() resend rather than spin.

Verified by dropping Start-up IPIs in KVM with a kprobe on
kvm_apic_send_ipi(); both firmware binaries are unmodified. Killing one
INIT-SIPI-SIPI sequence hangs the current firmware at all 8 drop points
tried, leaving the APs at CR0=0x00000010. The patched firmware boots at
all 8, issuing exactly one directed Start-up IPI per stranded AP. Without
injection, 80 boots patched against stock show no regression.

Submitted upstream as https://github.com/tianocore/edk2/pull/13164

Denis V. Lunev (2):
  UefiCpuPkg: add SendStartupIpi to LocalApicLib #VSTOR-146639
  UefiCpuPkg/MpInitLib: resend the Start-up IPI when an AP misses it
    #VSTOR-146639

 UefiCpuPkg/Include/Library/LocalApicLib.h     | 19 +++++
 .../Library/BaseXApicLib/BaseXApicLib.c       | 32 +++++--
 .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c   | 32 +++++--
 UefiCpuPkg/Library/MpInitLib/MpLib.c          | 85 +++++++++++++++++--
 UefiCpuPkg/Library/MpInitLib/MpLib.h          |  2 +
 5 files changed, 153 insertions(+), 17 deletions(-)

-- 
2.53.0

[PATCH 1/2] UefiCpuPkg: add SendStartupIpi to LocalApicLib #VSTOR-146639

[Denis V. Lunev]

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

The library can send a Start-up IPI to all processors excluding self,
or an INIT-SIPI-SIPI sequence to one processor, but it cannot send a
bare Start-up IPI to one processor. SendInitSipiSipi builds that ICR
inline instead.

Export it, and build SendInitSipiSipi on top of it. A caller that has
to poke a single AP now has a way to do so without an INIT, which
would reset an AP that had already started.

No functional change.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Cc: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 UefiCpuPkg/Include/Library/LocalApicLib.h     | 19 +++++++++++
 .../Library/BaseXApicLib/BaseXApicLib.c       | 32 ++++++++++++++++---
 .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c   | 32 ++++++++++++++++---
 3 files changed, 73 insertions(+), 10 deletions(-)

diff --git a/UefiCpuPkg/Include/Library/LocalApicLib.h b/UefiCpuPkg/Include/Library/LocalApicLib.h
index d7c2ad3f70..6c55ed75cc 100644
--- a/UefiCpuPkg/Include/Library/LocalApicLib.h
+++ b/UefiCpuPkg/Include/Library/LocalApicLib.h
@@ -200,6 +200,25 @@ SendStartupIpiAllExcludingSelf (
   IN UINT32  StartupRoutine
   );
 
+/**
+  Send a Start-up IPI to a specified target processor.
+
+  This function returns after the IPI has been accepted by the target processor.
+
+  if StartupRoutine >= 1M, then ASSERT.
+  if StartupRoutine is not multiple of 4K, then ASSERT.
+
+  @param  ApicId          Specify the local APIC ID of the target processor.
+  @param  StartupRoutine  Points to a start-up routine which is below 1M physical
+                          address and 4K aligned.
+**/
+VOID
+EFIAPI
+SendStartupIpi (
+  IN UINT32  ApicId,
+  IN UINT32  StartupRoutine
+  );
+
 /**
   Send an INIT-Start-up-Start-up IPI sequence to a specified target processor.
 
diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
index 9767e3b48e..2f2d31333a 100644
--- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
+++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
@@ -525,7 +525,7 @@ SendStartupIpiAllExcludingSelf (
 }
 
 /**
-  Send an INIT-Start-up-Start-up IPI sequence to a specified target processor.
+  Send a Start-up IPI to a specified target processor.
 
   This function returns after the IPI has been accepted by the target processor.
 
@@ -538,7 +538,7 @@ SendStartupIpiAllExcludingSelf (
 **/
 VOID
 EFIAPI
-SendInitSipiSipi (
+SendStartupIpi (
   IN UINT32  ApicId,
   IN UINT32  StartupRoutine
   )
@@ -548,16 +548,38 @@ SendInitSipiSipi (
   ASSERT (StartupRoutine < 0x100000);
   ASSERT ((StartupRoutine & 0xfff) == 0);
 
-  SendInitIpi (ApicId);
-  MicroSecondDelay (PcdGet32 (PcdCpuInitIpiDelayInMicroSeconds));
   IcrLow.Uint32            = 0;
   IcrLow.Bits.Vector       = (StartupRoutine >> 12);
   IcrLow.Bits.DeliveryMode = LOCAL_APIC_DELIVERY_MODE_STARTUP;
   IcrLow.Bits.Level        = 1;
   SendIpi (IcrLow.Uint32, ApicId);
+}
+
+/**
+  Send an INIT-Start-up-Start-up IPI sequence to a specified target processor.
+
+  This function returns after the IPI has been accepted by the target processor.
+
+  if StartupRoutine >= 1M, then ASSERT.
+  if StartupRoutine is not multiple of 4K, then ASSERT.
+
+  @param  ApicId          Specify the local APIC ID of the target processor.
+  @param  StartupRoutine  Points to a start-up routine which is below 1M physical
+                          address and 4K aligned.
+**/
+VOID
+EFIAPI
+SendInitSipiSipi (
+  IN UINT32  ApicId,
+  IN UINT32  StartupRoutine
+  )
+{
+  SendInitIpi (ApicId);
+  MicroSecondDelay (PcdGet32 (PcdCpuInitIpiDelayInMicroSeconds));
+  SendStartupIpi (ApicId, StartupRoutine);
   if (!StandardSignatureIsAuthenticAMD ()) {
     MicroSecondDelay (200);
-    SendIpi (IcrLow.Uint32, ApicId);
+    SendStartupIpi (ApicId, StartupRoutine);
   }
 }
 
diff --git a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
index 7fa997c882..d4db452a80 100644
--- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
+++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
@@ -764,7 +764,7 @@ SendStartupIpiAllExcludingSelf (
 }
 
 /**
-  Send an INIT-Start-up-Start-up IPI sequence to a specified target processor.
+  Send a Start-up IPI to a specified target processor.
 
   This function returns after the IPI has been accepted by the target processor.
 
@@ -777,7 +777,7 @@ SendStartupIpiAllExcludingSelf (
 **/
 VOID
 EFIAPI
-SendInitSipiSipi (
+SendStartupIpi (
   IN UINT32  ApicId,
   IN UINT32  StartupRoutine
   )
@@ -787,16 +787,38 @@ SendInitSipiSipi (
   ASSERT (StartupRoutine < 0x100000);
   ASSERT ((StartupRoutine & 0xfff) == 0);
 
-  SendInitIpi (ApicId);
-  MicroSecondDelay (PcdGet32 (PcdCpuInitIpiDelayInMicroSeconds));
   IcrLow.Uint32            = 0;
   IcrLow.Bits.Vector       = (StartupRoutine >> 12);
   IcrLow.Bits.DeliveryMode = LOCAL_APIC_DELIVERY_MODE_STARTUP;
   IcrLow.Bits.Level        = 1;
   SendIpi (IcrLow.Uint32, ApicId);
+}
+
+/**
+  Send an INIT-Start-up-Start-up IPI sequence to a specified target processor.
+
+  This function returns after the IPI has been accepted by the target processor.
+
+  if StartupRoutine >= 1M, then ASSERT.
+  if StartupRoutine is not multiple of 4K, then ASSERT.
+
+  @param  ApicId          Specify the local APIC ID of the target processor.
+  @param  StartupRoutine  Points to a start-up routine which is below 1M physical
+                          address and 4K aligned.
+**/
+VOID
+EFIAPI
+SendInitSipiSipi (
+  IN UINT32  ApicId,
+  IN UINT32  StartupRoutine
+  )
+{
+  SendInitIpi (ApicId);
+  MicroSecondDelay (PcdGet32 (PcdCpuInitIpiDelayInMicroSeconds));
+  SendStartupIpi (ApicId, StartupRoutine);
   if (!StandardSignatureIsAuthenticAMD ()) {
     MicroSecondDelay (200);
-    SendIpi (IcrLow.Uint32, ApicId);
+    SendStartupIpi (ApicId, StartupRoutine);
   }
 }
 
-- 
2.53.0

Re: [PATCH 1/2] UefiCpuPkg: add SendStartupIpi to LocalApicLib #VSTOR-146639

[Andrey Drobyshev]

Ack

On 9/21/26 6:44 PM, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
> 
> The library can send a Start-up IPI to all processors excluding self,
> or an INIT-SIPI-SIPI sequence to one processor, but it cannot send a
> bare Start-up IPI to one processor. SendInitSipiSipi builds that ICR
> inline instead.
> 
> Export it, and build SendInitSipiSipi on top of it. A caller that has
> to poke a single AP now has a way to do so without an INIT, which
> would reset an AP that had already started.
> 
> No functional change.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
>  UefiCpuPkg/Include/Library/LocalApicLib.h     | 19 +++++++++++
>  .../Library/BaseXApicLib/BaseXApicLib.c       | 32 ++++++++++++++++---
>  .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c   | 32 ++++++++++++++++---
>  3 files changed, 73 insertions(+), 10 deletions(-)
> 
> diff --git a/UefiCpuPkg/Include/Library/LocalApicLib.h b/UefiCpuPkg/Include/Library/LocalApicLib.h
> index d7c2ad3f70..6c55ed75cc 100644
> --- a/UefiCpuPkg/Include/Library/LocalApicLib.h
> +++ b/UefiCpuPkg/Include/Library/LocalApicLib.h
> @@ -200,6 +200,25 @@ SendStartupIpiAllExcludingSelf (
>    IN UINT32  StartupRoutine
>    );
>  
> +/**
> +  Send a Start-up IPI to a specified target processor.
> +
> +  This function returns after the IPI has been accepted by the target processor.
> +
> +  if StartupRoutine >= 1M, then ASSERT.
> +  if StartupRoutine is not multiple of 4K, then ASSERT.
> +
> +  @param  ApicId          Specify the local APIC ID of the target processor.
> +  @param  StartupRoutine  Points to a start-up routine which is below 1M physical
> +                          address and 4K aligned.
> +**/
> +VOID
> +EFIAPI
> +SendStartupIpi (
> +  IN UINT32  ApicId,
> +  IN UINT32  StartupRoutine
> +  );
> +
>  /**
>    Send an INIT-Start-up-Start-up IPI sequence to a specified target processor.
>  
> diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> index 9767e3b48e..2f2d31333a 100644
> --- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> +++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> @@ -525,7 +525,7 @@ SendStartupIpiAllExcludingSelf (
>  }
>  
>  /**
> -  Send an INIT-Start-up-Start-up IPI sequence to a specified target processor.
> +  Send a Start-up IPI to a specified target processor.
>  
>    This function returns after the IPI has been accepted by the target processor.
>  
> @@ -538,7 +538,7 @@ SendStartupIpiAllExcludingSelf (
>  **/
>  VOID
>  EFIAPI
> -SendInitSipiSipi (
> +SendStartupIpi (
>    IN UINT32  ApicId,
>    IN UINT32  StartupRoutine
>    )
> @@ -548,16 +548,38 @@ SendInitSipiSipi (
>    ASSERT (StartupRoutine < 0x100000);
>    ASSERT ((StartupRoutine & 0xfff) == 0);
>  
> -  SendInitIpi (ApicId);
> -  MicroSecondDelay (PcdGet32 (PcdCpuInitIpiDelayInMicroSeconds));
>    IcrLow.Uint32            = 0;
>    IcrLow.Bits.Vector       = (StartupRoutine >> 12);
>    IcrLow.Bits.DeliveryMode = LOCAL_APIC_DELIVERY_MODE_STARTUP;
>    IcrLow.Bits.Level        = 1;
>    SendIpi (IcrLow.Uint32, ApicId);
> +}
> +
> +/**
> +  Send an INIT-Start-up-Start-up IPI sequence to a specified target processor.
> +
> +  This function returns after the IPI has been accepted by the target processor.
> +
> +  if StartupRoutine >= 1M, then ASSERT.
> +  if StartupRoutine is not multiple of 4K, then ASSERT.
> +
> +  @param  ApicId          Specify the local APIC ID of the target processor.
> +  @param  StartupRoutine  Points to a start-up routine which is below 1M physical
> +                          address and 4K aligned.
> +**/
> +VOID
> +EFIAPI
> +SendInitSipiSipi (
> +  IN UINT32  ApicId,
> +  IN UINT32  StartupRoutine
> +  )
> +{
> +  SendInitIpi (ApicId);
> +  MicroSecondDelay (PcdGet32 (PcdCpuInitIpiDelayInMicroSeconds));
> +  SendStartupIpi (ApicId, StartupRoutine);
>    if (!StandardSignatureIsAuthenticAMD ()) {
>      MicroSecondDelay (200);
> -    SendIpi (IcrLow.Uint32, ApicId);
> +    SendStartupIpi (ApicId, StartupRoutine);
>    }
>  }
>  
> diff --git a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> index 7fa997c882..d4db452a80 100644
> --- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> +++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> @@ -764,7 +764,7 @@ SendStartupIpiAllExcludingSelf (
>  }
>  
>  /**
> -  Send an INIT-Start-up-Start-up IPI sequence to a specified target processor.
> +  Send a Start-up IPI to a specified target processor.
>  
>    This function returns after the IPI has been accepted by the target processor.
>  
> @@ -777,7 +777,7 @@ SendStartupIpiAllExcludingSelf (
>  **/
>  VOID
>  EFIAPI
> -SendInitSipiSipi (
> +SendStartupIpi (
>    IN UINT32  ApicId,
>    IN UINT32  StartupRoutine
>    )
> @@ -787,16 +787,38 @@ SendInitSipiSipi (
>    ASSERT (StartupRoutine < 0x100000);
>    ASSERT ((StartupRoutine & 0xfff) == 0);
>  
> -  SendInitIpi (ApicId);
> -  MicroSecondDelay (PcdGet32 (PcdCpuInitIpiDelayInMicroSeconds));
>    IcrLow.Uint32            = 0;
>    IcrLow.Bits.Vector       = (StartupRoutine >> 12);
>    IcrLow.Bits.DeliveryMode = LOCAL_APIC_DELIVERY_MODE_STARTUP;
>    IcrLow.Bits.Level        = 1;
>    SendIpi (IcrLow.Uint32, ApicId);
> +}
> +
> +/**
> +  Send an INIT-Start-up-Start-up IPI sequence to a specified target processor.
> +
> +  This function returns after the IPI has been accepted by the target processor.
> +
> +  if StartupRoutine >= 1M, then ASSERT.
> +  if StartupRoutine is not multiple of 4K, then ASSERT.
> +
> +  @param  ApicId          Specify the local APIC ID of the target processor.
> +  @param  StartupRoutine  Points to a start-up routine which is below 1M physical
> +                          address and 4K aligned.
> +**/
> +VOID
> +EFIAPI
> +SendInitSipiSipi (
> +  IN UINT32  ApicId,
> +  IN UINT32  StartupRoutine
> +  )
> +{
> +  SendInitIpi (ApicId);
> +  MicroSecondDelay (PcdGet32 (PcdCpuInitIpiDelayInMicroSeconds));
> +  SendStartupIpi (ApicId, StartupRoutine);
>    if (!StandardSignatureIsAuthenticAMD ()) {
>      MicroSecondDelay (200);
> -    SendIpi (IcrLow.Uint32, ApicId);
> +    SendStartupIpi (ApicId, StartupRoutine);
>    }
>  }
>  

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

[Denis V. Lunev]

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

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

[Andrey Drobyshev]

Ack
Overall LGTM, see some minor nits below

On 9/21/26 6:44 PM, Denis V. Lunev wrote:
> 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
> +  );
> +

These functions are only used here in this translation unit, yet they
aren't declared static.  Should we wither declare them static and move
their definitions above (without the need for forward declarations), or
just declare them in MpLib.h?

>  /**
>    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;
> +    }

This re-send of IPI should happen rarely.  Wouldn't we wanna know when
it does happen?  How about adding some debug info when IPI is re-issued?

Andrey

> +
>      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.

Re: [PATCH 0/2] UefiCpuPkg/MpInitLib: recover from a lost Start-up IPI #VSTOR-146639

[Andrey Drobyshev]

On 9/21/26 6:44 PM, Denis V. Lunev wrote:
> WaitApWakeup() spins until an AP clears its wakeup signal, with no timeout
> and no way out. A hypervisor can lose the Start-up IPI: KVM latches INIT
> while the target vCPU is in SMM but discards the SIPI outright. The AP is
> left in wait-for-SIPI, the BSP spins forever, and the guest never boots.
> 
> Analysis of the live hung guest is in the bug.
> 
> Patch 1 exports SendStartupIpi() from LocalApicLib, so one AP can be poked
> without an INIT that would reset an AP already running the procedure.
> Patch 2 makes WaitApWakeup() resend rather than spin.
> 
> Verified by dropping Start-up IPIs in KVM with a kprobe on
> kvm_apic_send_ipi(); both firmware binaries are unmodified. Killing one
> INIT-SIPI-SIPI sequence hangs the current firmware at all 8 drop points
> tried, leaving the APs at CR0=0x00000010. The patched firmware boots at
> all 8, issuing exactly one directed Start-up IPI per stranded AP. Without
> injection, 80 boots patched against stock show no regression.
> 
> Submitted upstream as https://github.com/tianocore/edk2/pull/13164
> 
> Denis V. Lunev (2):
>   UefiCpuPkg: add SendStartupIpi to LocalApicLib #VSTOR-146639
>   UefiCpuPkg/MpInitLib: resend the Start-up IPI when an AP misses it
>     #VSTOR-146639
> 
>  UefiCpuPkg/Include/Library/LocalApicLib.h     | 19 +++++
>  .../Library/BaseXApicLib/BaseXApicLib.c       | 32 +++++--
>  .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c   | 32 +++++--
>  UefiCpuPkg/Library/MpInitLib/MpLib.c          | 85 +++++++++++++++++--
>  UefiCpuPkg/Library/MpInitLib/MpLib.h          |  2 +
>  5 files changed, 153 insertions(+), 17 deletions(-)


Please put the component name in the subject, we have multiple
components in development in svt-core@.  Same for the branch name.  We
have FC for hci-8.0 now - should this series go into 8.0, or 8.1, or both?

Andrey

Re: [PATCH 0/2] UefiCpuPkg/MpInitLib: recover from a lost Start-up IPI #VSTOR-146639

[Denis V. Lunev]

On 9/23/26 15:30, Andrey Drobyshev wrote:
> On 9/21/26 6:44 PM, Denis V. Lunev wrote:
>> WaitApWakeup() spins until an AP clears its wakeup signal, with no timeout
>> and no way out. A hypervisor can lose the Start-up IPI: KVM latches INIT
>> while the target vCPU is in SMM but discards the SIPI outright. The AP is
>> left in wait-for-SIPI, the BSP spins forever, and the guest never boots.
>>
>> Analysis of the live hung guest is in the bug.
>>
>> Patch 1 exports SendStartupIpi() from LocalApicLib, so one AP can be poked
>> without an INIT that would reset an AP already running the procedure.
>> Patch 2 makes WaitApWakeup() resend rather than spin.
>>
>> Verified by dropping Start-up IPIs in KVM with a kprobe on
>> kvm_apic_send_ipi(); both firmware binaries are unmodified. Killing one
>> INIT-SIPI-SIPI sequence hangs the current firmware at all 8 drop points
>> tried, leaving the APs at CR0=0x00000010. The patched firmware boots at
>> all 8, issuing exactly one directed Start-up IPI per stranded AP. Without
>> injection, 80 boots patched against stock show no regression.
>>
>> Submitted upstream as https://github.com/tianocore/edk2/pull/13164
>>
>> Denis V. Lunev (2):
>>   UefiCpuPkg: add SendStartupIpi to LocalApicLib #VSTOR-146639
>>   UefiCpuPkg/MpInitLib: resend the Start-up IPI when an AP misses it
>>     #VSTOR-146639
>>
>>  UefiCpuPkg/Include/Library/LocalApicLib.h     | 19 +++++
>>  .../Library/BaseXApicLib/BaseXApicLib.c       | 32 +++++--
>>  .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c   | 32 +++++--
>>  UefiCpuPkg/Library/MpInitLib/MpLib.c          | 85 +++++++++++++++++--
>>  UefiCpuPkg/Library/MpInitLib/MpLib.h          |  2 +
>>  5 files changed, 153 insertions(+), 17 deletions(-)
>
> Please put the component name in the subject, we have multiple
> components in development in svt-core@.  Same for the branch name.  We
> have FC for hci-8.0 now - should this series go into 8.0, or 8.1, or both?
>
> Andrey
both