[PATCH 0/2] Wake up APs by a single SIPI.

[PATCH 0/2] Wake up APs by a single SIPI.

[Yuanhao Xie]

After a power-up or reset, all application processors (APs) are in a 
state where they are waiting for sipi. In this state, the bootstrap 
processor (BSP) send a single SIPI Inter-Processor Interrupt 
(IPI) to all the APs.

Yuanhao Xie (2):
  UefiCpuPkg: Add SendStartupIpiAllExcludingSelf
  UefiCpuPkg: After reset, wake up APs by SIPI.

 UefiCpuPkg/Include/Library/LocalApicLib.h     | 17 +++++++-
 .../Library/BaseXApicLib/BaseXApicLib.c       | 43 +++++++++++++------
 .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c   | 43 +++++++++++++------
 UefiCpuPkg/Library/MpInitLib/MpLib.c          |  9 +++-
 4 files changed, 84 insertions(+), 28 deletions(-)

-- 
2.36.1.windows.1

[PATCH 1/2] UefiCpuPkg: Add SendStartupIpiAllExcludingSelf

[Yuanhao Xie]

Add new API SendStartupIpiAllExcludingSelf(), and modify
SendInitSipiSipiAllExcludingSelf() by let it call the new API.

Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Sean Rhodes <sean@starlabs.systems>
Cc: James Lu <james.lu@intel.com>
Cc: Gua Guo <gua.guo@intel.com>
Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
---
 UefiCpuPkg/Include/Library/LocalApicLib.h     | 17 +++++++-
 .../Library/BaseXApicLib/BaseXApicLib.c       | 43 +++++++++++++------
 .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c   | 43 +++++++++++++------
 3 files changed, 76 insertions(+), 27 deletions(-)

diff --git a/UefiCpuPkg/Include/Library/LocalApicLib.h b/UefiCpuPkg/Include/Library/LocalApicLib.h
index b55d88b0f5..d7c2ad3f70 100644
--- a/UefiCpuPkg/Include/Library/LocalApicLib.h
+++ b/UefiCpuPkg/Include/Library/LocalApicLib.h
@@ -4,7 +4,7 @@
   Local APIC library assumes local APIC is enabled. It does not
   handles cases where local APIC is disabled.
 
-  Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2010 - 2023, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -185,6 +185,21 @@ SendInitIpiAllExcludingSelf (
   VOID
   );
 
+/**
+  Send a Start-up IPI to all processors excluding self.
+  This function returns after the IPI has been accepted by the target processors.
+  if StartupRoutine >= 1M, then ASSERT.
+  if StartupRoutine is not multiple of 4K, then ASSERT.
+  @param  StartupRoutine  Points to a start-up routine which is below 1M physical
+                          address and 4K aligned.
+**/
+
+VOID
+EFIAPI
+SendStartupIpiAllExcludingSelf (
+  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 008b8a070b..d56c6275cc 100644
--- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
+++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
@@ -3,7 +3,7 @@
 
   This local APIC library instance supports xAPIC mode only.
 
-  Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2010 - 2023, Intel Corporation. All rights reserved.<BR>
   Copyright (c) 2017 - 2020, AMD Inc. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -497,6 +497,33 @@ SendInitIpiAllExcludingSelf (
   SendIpi (IcrLow.Uint32, 0);
 }
 
+/**
+  Send a Start-up IPI to all processors excluding self.
+  This function returns after the IPI has been accepted by the target processors.
+  if StartupRoutine >= 1M, then ASSERT.
+  if StartupRoutine is not multiple of 4K, then ASSERT.
+  @param  StartupRoutine  Points to a start-up routine which is below 1M physical
+                          address and 4K aligned.
+**/
+VOID
+EFIAPI
+SendStartupIpiAllExcludingSelf (
+  IN UINT32  StartupRoutine
+  )
+{
+  LOCAL_APIC_ICR_LOW  IcrLow;
+
+  ASSERT (StartupRoutine < 0x100000);
+  ASSERT ((StartupRoutine & 0xfff) == 0);
+
+  IcrLow.Uint32                    = 0;
+  IcrLow.Bits.Vector               = (StartupRoutine >> 12);
+  IcrLow.Bits.DeliveryMode         = LOCAL_APIC_DELIVERY_MODE_STARTUP;
+  IcrLow.Bits.Level                = 1;
+  IcrLow.Bits.DestinationShorthand = LOCAL_APIC_DESTINATION_SHORTHAND_ALL_EXCLUDING_SELF;
+  SendIpi (IcrLow.Uint32, 0);
+}
+
 /**
   Send an INIT-Start-up-Start-up IPI sequence to a specified target processor.
 
@@ -551,22 +578,12 @@ SendInitSipiSipiAllExcludingSelf (
   IN UINT32  StartupRoutine
   )
 {
-  LOCAL_APIC_ICR_LOW  IcrLow;
-
-  ASSERT (StartupRoutine < 0x100000);
-  ASSERT ((StartupRoutine & 0xfff) == 0);
-
   SendInitIpiAllExcludingSelf ();
   MicroSecondDelay (PcdGet32 (PcdCpuInitIpiDelayInMicroSeconds));
-  IcrLow.Uint32                    = 0;
-  IcrLow.Bits.Vector               = (StartupRoutine >> 12);
-  IcrLow.Bits.DeliveryMode         = LOCAL_APIC_DELIVERY_MODE_STARTUP;
-  IcrLow.Bits.Level                = 1;
-  IcrLow.Bits.DestinationShorthand = LOCAL_APIC_DESTINATION_SHORTHAND_ALL_EXCLUDING_SELF;
-  SendIpi (IcrLow.Uint32, 0);
+  SendStartupIpiAllExcludingSelf (StartupRoutine);
   if (!StandardSignatureIsAuthenticAMD ()) {
     MicroSecondDelay (200);
-    SendIpi (IcrLow.Uint32, 0);
+    SendStartupIpiAllExcludingSelf (StartupRoutine);
   }
 }
 
diff --git a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
index 0ba0499631..aa4eb11181 100644
--- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
+++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
@@ -4,7 +4,7 @@
   This local APIC library instance supports x2APIC capable processors
   which have xAPIC and x2APIC modes.
 
-  Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2010 - 2023, Intel Corporation. All rights reserved.<BR>
   Copyright (c) 2017 - 2020, AMD Inc. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -736,6 +736,33 @@ SendInitIpiAllExcludingSelf (
   SendIpi (IcrLow.Uint32, 0);
 }
 
+/**
+  Send a Start-up IPI to all processors excluding self.
+  This function returns after the IPI has been accepted by the target processors.
+  if StartupRoutine >= 1M, then ASSERT.
+  if StartupRoutine is not multiple of 4K, then ASSERT.
+  @param  StartupRoutine  Points to a start-up routine which is below 1M physical
+                          address and 4K aligned.
+**/
+VOID
+EFIAPI
+SendStartupIpiAllExcludingSelf (
+  IN UINT32  StartupRoutine
+  )
+{
+  LOCAL_APIC_ICR_LOW  IcrLow;
+
+  ASSERT (StartupRoutine < 0x100000);
+  ASSERT ((StartupRoutine & 0xfff) == 0);
+
+  IcrLow.Uint32                    = 0;
+  IcrLow.Bits.Vector               = (StartupRoutine >> 12);
+  IcrLow.Bits.DeliveryMode         = LOCAL_APIC_DELIVERY_MODE_STARTUP;
+  IcrLow.Bits.Level                = 1;
+  IcrLow.Bits.DestinationShorthand = LOCAL_APIC_DESTINATION_SHORTHAND_ALL_EXCLUDING_SELF;
+  SendIpi (IcrLow.Uint32, 0);
+}
+
 /**
   Send an INIT-Start-up-Start-up IPI sequence to a specified target processor.
 
@@ -790,22 +817,12 @@ SendInitSipiSipiAllExcludingSelf (
   IN UINT32  StartupRoutine
   )
 {
-  LOCAL_APIC_ICR_LOW  IcrLow;
-
-  ASSERT (StartupRoutine < 0x100000);
-  ASSERT ((StartupRoutine & 0xfff) == 0);
-
   SendInitIpiAllExcludingSelf ();
   MicroSecondDelay (PcdGet32 (PcdCpuInitIpiDelayInMicroSeconds));
-  IcrLow.Uint32                    = 0;
-  IcrLow.Bits.Vector               = (StartupRoutine >> 12);
-  IcrLow.Bits.DeliveryMode         = LOCAL_APIC_DELIVERY_MODE_STARTUP;
-  IcrLow.Bits.Level                = 1;
-  IcrLow.Bits.DestinationShorthand = LOCAL_APIC_DESTINATION_SHORTHAND_ALL_EXCLUDING_SELF;
-  SendIpi (IcrLow.Uint32, 0);
+  SendStartupIpiAllExcludingSelf (StartupRoutine);
   if (!StandardSignatureIsAuthenticAMD ()) {
     MicroSecondDelay (200);
-    SendIpi (IcrLow.Uint32, 0);
+    SendStartupIpiAllExcludingSelf (StartupRoutine);
   }
 }
 
-- 
2.36.1.windows.1

[PATCH 2/2] UefiCpuPkg: After reset, wake up APs by SIPI.

[Yuanhao Xie]

Following a power-up or RESET, since all APs are already in the
“wait-for-SIPI state,” the BSP can broadcast just a single SIPI IPI to
the APs to wake them up and initialize them.

Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Sean Rhodes <sean@starlabs.systems>
Cc: James Lu <james.lu@intel.com>
Cc: Gua Guo <gua.guo@intel.com>
Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index e5dc852ed9..a540b3fe3d 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1233,7 +1233,14 @@ WakeUpAP (
       if (CpuMpData->SevSnpIsEnabled && (CpuMpData->InitFlag != ApInitConfig)) {
         SevSnpCreateAP (CpuMpData, -1);
       } else {
-        SendInitSipiSipiAllExcludingSelf ((UINT32)ExchangeInfo->BufferStart);
+        if (CpuMpData->InitFlag == ApInitConfig) {
+          //
+          // SIPI can be used for the first time wake up after reset to reduce boot time.
+          //
+          SendStartupIpiAllExcludingSelf ((UINT32)ExchangeInfo->BufferStart);
+        } else {
+          SendInitSipiSipiAllExcludingSelf ((UINT32)ExchangeInfo->BufferStart);
+        }
       }
     }
 
-- 
2.36.1.windows.1

Re: [edk2-devel] [PATCH 2/2] UefiCpuPkg: After reset, wake up APs by SIPI.

[Gerd Hoffmann]

On Mon, Mar 27, 2023 at 11:17:11AM +0800, Yuanhao Xie wrote:
> Following a power-up or RESET, since all APs are already in the
> “wait-for-SIPI state,” the BSP can broadcast just a single SIPI IPI to
> the APs to wake them up and initialize them.
> 
> Cc: Guo Dong <guo.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Sean Rhodes <sean@starlabs.systems>
> Cc: James Lu <james.lu@intel.com>
> Cc: Gua Guo <gua.guo@intel.com>
> Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>

This patch breaks OVMF (just boot the firmware in qemu image with '-smp 2').

take care,
  Gerd

Re: [edk2-devel] [PATCH 2/2] UefiCpuPkg: After reset, wake up APs by SIPI.

[Yuanhao Xie]

Hi Gerd,

Could you please provide me more information, like where does it hang, ad what build and boot options you use

I used this image https://www.kraxel.org/repos/images/fedora-30-efi-systemd-i686.qcow2.xz, and extracted the kernel from second partition (VFAT) of "/a29755367bf242efb072cfdd1a70134f/5.3.12-200.fc30.i686/linux" .

Build and boot with:
build \
    -a IA32 \
    -b NOOPT \
    -p OvmfPkg/OvmfPkgIa32.dsc \
    -t GCC5 \
    -D SECURE_BOOT_ENABLE \
    -D SMM_REQUIRE \
    -D NETWORK_TLS_ENABLE \
    -D NETWORK_IP6_ENABLE \
    -D NETWORK_HTTP_BOOT_ENABLE \
    -D TPM2_ENABLE \
    --pcd=gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel=0x8040004F

qemu-system-i386 \
    -cpu coreduo,-nx \
    -machine q35,smm=on,accel=kvm \
    -m 5120 \
    -smp 2 \
    -global driver=cfi.pflash01,property=secure,value=on \
    -drive if=pflash,format=raw,unit=0,file=OVMF_CODE.fd,readonly=on \
    -drive if=pflash,format=raw,unit=1,file=OVMF_VARS.fd,snapshot=on \
    -debugcon file:debug3.log \
    -global isa-debugcon.iobase=0x402 \
    -chardev stdio,signal=off,mux=on,id=char0 \
    -mon chardev=char0,mode=readline \
    -serial chardev:char0 \
    -kernel linux \
    -append "ignore_loglevel console=ttyS0"


The end of log are as following:

	PixelBlueGreenRedReserved8BitPerColor
	ConvertPages: range 400000 - 141CFFF covers multiple entries
	SmmInstallProtocolInterface: 296EB418-C4C8-4E05-AB59-39E8AF56F00A 0
	CpuDxe: 5-Level Paging = 0
	MpInitChangeApLoopCallback() done!
	SetUefiImageMemoryAttributes - 0x000000007ED47000 - 0x000000000000A000 (0x0000000000000008)
	SetUefiImageMemoryAttributes - 0x000000007ED3E000 - 0x0000000000009000 (0x0000000000000008)
	SetUefiImageMemoryAttributes - 0x000000007ED33000 - 0x000000000000B000 (0x0000000000000008)
	SetUefiImageMemoryAttributes - 0x000000007ED2A000 - 0x0000000000009000 (0x0000000000000008)
	SetUefiImageMemoryAttributes - 0x000000007ED1F000 - 0x000000000000B000 (0x0000000000000008)
	SetUefiImageMemoryAttributes - 0x000000007ED11000 - 0x000000000000E000 (0x0000000000000008)
	SetUefiImageMemoryAttributes - 0x000000007ED04000 - 0x000000000000D000 (0x0000000000000008)
	SetUefiImageMemoryAttributes - 0x000000007ECF8000 - 0x000000000000C000 (0x0000000000000008)
	SetUefiImageMemoryAttributes - 0x000000007ECF1000 - 0x0000000000007000 (0x0000000000000008)
	SetUefiImageMemoryAttributes - 0x000000007ECE9000 - 0x0000000000008000 (0x0000000000000008)
	ValidateSetVariable - Variable (4424AC57-BE4B-47DD-9E97-ED50F09F92A9:DUMMY) returning Success.

Thanks
Yuanhao
-----Original Message-----
From: Gerd Hoffmann <kraxel@redhat.com> 
Sent: Monday, March 27, 2023 7:03 PM
To: devel@edk2.groups.io; Xie, Yuanhao <yuanhao.xie@intel.com>
Cc: Dong, Guo <guo.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Rhodes, Sean <sean@starlabs.systems>; Lu, James <james.lu@intel.com>; Guo, Gua <gua.guo@intel.com>
Subject: Re: [edk2-devel] [PATCH 2/2] UefiCpuPkg: After reset, wake up APs by SIPI.

On Mon, Mar 27, 2023 at 11:17:11AM +0800, Yuanhao Xie wrote:
> Following a power-up or RESET, since all APs are already in the 
> “wait-for-SIPI state,” the BSP can broadcast just a single SIPI IPI to 
> the APs to wake them up and initialize them.
> 
> Cc: Guo Dong <guo.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Sean Rhodes <sean@starlabs.systems>
> Cc: James Lu <james.lu@intel.com>
> Cc: Gua Guo <gua.guo@intel.com>
> Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>

This patch breaks OVMF (just boot the firmware in qemu image with '-smp 2').

take care,
  Gerd

Re: [edk2-devel] [PATCH 2/2] UefiCpuPkg: After reset, wake up APs by SIPI.

[Gerd Hoffmann]

On Wed, Mar 29, 2023 at 08:50:24AM +0000, Xie, Yuanhao wrote:
> Hi Gerd,
> 
> Could you please provide me more information, like where does it hang, ad what build and boot options you use

qemu-system-x86_64 -display none -accel kvm -smp 2 -M q35 \
	-bios Build/OvmfX64/DEBUG_GCC5/FV/OVMF.fd \
	-chardev stdio,id=fw -device isa-debugcon,iobase=0x402,chardev=fw

Last log lines are ...

	AP Loop Mode is 1
	AP Vector: non-16-bit = 7EA7000/447
	WakeupBufferStart = 9F000, WakeupBufferSize = 1000
	AP Vector: 16-bit = 9F000/39, ExchangeInfo = 9F039/A4
	CpuMpPei: 5-Level Paging = 0

... and here it hangs.

>     -p OvmfPkg/OvmfPkgIa32.dsc \

Using OvmfPkgX64.dsc here.  Tried using Ia32 instead, hangs too,
at the same place.

take care,
  Gerd