* [PATCH 1/5] UefiCpuPkg: Create MpHandOff to transfer info from PEI to DXE.
2023-06-12 13:37 [PATCH 0/5] Eliminate the second INIT-SIPI-SIPI sequence Yuanhao Xie
@ 2023-06-12 13:37 ` Yuanhao Xie
2023-06-20 14:08 ` [edk2-devel] " Gerd Hoffmann
2023-06-12 13:37 ` [PATCH 2/5] UefiCpuPkg: Refactor the logic for placing APs in HltLoop Yuanhao Xie
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Yuanhao Xie @ 2023-06-12 13:37 UTC (permalink / raw)
To: devel
Cc: Gerd Hoffmann, Eric Dong, Ray Ni, Rahul Kumar, Tom Lendacky,
Yuanhao Xie
The purpose of the Hob characterized by mCpuInitMpLibHobGuid was
twofold: To transfer information from PEI to DXE and acted as a global
variable for PEI. However, during the DXE phase,only a few fields
from the CPU_MP_DATA which collected in PEI phase were needed.
To enhance this process, a new Hob was specifically created to transfer
information to the DXE phase. This new Hob contained only the essential
fields required for reuse in DXE. Instead of directly including the
BspNumber in MpHandOff, the DXE phase introduced the use of
GetBspNumber() to collect the BspNumber from ApicID and CpuCount.
The addition of MP_HAND_OFF_SIGNAL serves the sole purpose of awakening
the APs and transitioning their context from PEI to DXE. The
WaitLoopExecutionMode field indicates whether the bit mode of PEI
matches that of DXE. Both of these fields are filled only when the
ApLoopMode is not ApInHltLoop. In the case of ApLoopMode, it remains
necessary to wake up the APs using the init-sipi-sipi sequence.
The function GetMpHandOffHob() was added to the collected MpHandOff in
the DXE phase. The CpuMpData in the DXE phase was updated by gathering
information from MpHandOff.
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
---
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 1 +
UefiCpuPkg/Library/MpInitLib/MpHandOff.h | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
UefiCpuPkg/Library/MpInitLib/MpLib.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------
UefiCpuPkg/Library/MpInitLib/MpLib.h | 2 ++
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 1 +
UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 32 +++++++++++++++++++++++++++++++-
6 files changed, 186 insertions(+), 14 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
index 67ebf7254c..7d45d3ad4d 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
@@ -38,6 +38,7 @@
MpLib.c
MpLib.h
Microcode.c
+ MpHandOff.h
[Packages]
MdePkg/MdePkg.dec
diff --git a/UefiCpuPkg/Library/MpInitLib/MpHandOff.h b/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
new file mode 100644
index 0000000000..65c97d865c
--- /dev/null
+++ b/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
@@ -0,0 +1,64 @@
+/** @file
+ Defines the HOB GUID used to describe the MSEG memory region allocated in PEI.
+ Copyright (c) 2015 - 2023, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef MP_HANDOFF_H_
+#define MP_HANDOFF_H_
+//
+// To trigger the start-up signal, BSP writes the specified
+// StartupSignalValue to the StartupSignalAddress of each processor.
+// This address is monitored by the APs, and as soon as they receive
+// the value that matches the MP_HAND_OFF_SIGNAL, they will wake up
+// and switch the context from PEI to DXE phase.
+//
+#define MP_HAND_OFF_SIGNAL SIGNATURE_32 ('M', 'P', 'H', 'O')
+
+#define MP_HANDOFF_GUID \
+ { \
+ 0x11e2bd88, 0xed38, 0x4abd, {0xa3, 0x99, 0x21, 0xf2, 0x5f, 0xd0, 0x7a, 0x60 } \
+ }
+extern EFI_GUID mMpHandOffGuid;
+//
+// The information required to transfer from the PEI phase to the
+// DXE phase is contained within the MP_HAND_OFF and PROCESSOR_HAND_OFF.
+// If the SizeOfPointer (WaitLoopExecutionMode) of both phases are equal,
+// and the APs is not in halt mode,
+// then the APs can be awakened by triggering the start-up
+// signal, rather than using INIT-SIPI-SIPI.
+// To trigger the start-up signal, BSP writes the specified
+// StartupSignalValue to the StartupSignalAddress of each processor.
+// This address is monitored by the APs.
+//
+typedef struct {
+ UINT32 ApicId;
+ UINT32 Health;
+ UINT64 StartupSignalAddress;
+ UINT64 StartupProcedureAddress;
+} PROCESSOR_HAND_OFF;
+
+typedef struct {
+ //
+ // The ProcessorIndex indicates the range of processors. If it is set to 0, it signifies
+ // processors from 0 to CpuCount - 1. Multiple instances in the HOB list describe
+ // processors from ProcessorIndex to ProcessorIndex + CpuCount - 1.
+ //
+ UINT32 ProcessorIndex;
+ UINT32 CpuCount;
+ UINT32 WaitLoopExecutionMode;
+ UINT32 StartupSignalValue;
+ PROCESSOR_HAND_OFF Info[];
+} MP_HAND_OFF;
+
+/**
+ Get pointer to MP_HAND_OFF GUIDed HOB.
+
+ @return The pointer to MP_HAND_OFF structure.
+**/
+MP_HAND_OFF *
+GetMpHandOffHob (
+ VOID
+ );
+
+#endif
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index f1f2840714..c55150e7b6 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -14,6 +14,7 @@
#include <Register/Amd/Ghcb.h>
EFI_GUID mCpuInitMpLibHobGuid = CPU_INIT_MP_LIB_HOB_GUID;
+EFI_GUID mMpHandOffGuid = MP_HANDOFF_GUID;
/**
Save the volatile registers required to be restored following INIT IPI.
@@ -1784,6 +1785,54 @@ CheckAllAPs (
return EFI_NOT_READY;
}
+/**
+ This function Get BspNumber.
+
+ @param[in] MpHandOff Pointer to MpHandOff
+ @return BspNumber
+**/
+UINT32
+GetBspNumber (
+ IN CONST MP_HAND_OFF *MpHandOff
+ )
+{
+ UINT32 ApicId;
+ UINT32 BspNumber;
+ UINT32 Index;
+
+ //
+ // Get the processor number for the BSP
+ //
+ BspNumber = MAX_UINT32;
+ ApicId = GetInitialApicId ();
+ for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
+ if (MpHandOff->Info[Index].ApicId == ApicId) {
+ BspNumber = Index;
+ }
+ }
+
+ ASSERT (BspNumber != MAX_UINT32);
+
+ return BspNumber;
+}
+
+/**
+ Get pointer to CPU MP Data structure from GUIDed HOB.
+
+ @param[in] CpuMpData The pointer to CPU MP Data structure.
+**/
+VOID
+AmdSevUpdateCpuMpData (
+ IN CPU_MP_DATA *CpuMpData
+ )
+{
+ CPU_MP_DATA *OldCpuMpData;
+
+ OldCpuMpData = GetCpuMpDataFromGuidedHob ();
+
+ OldCpuMpData->NewCpuMpData = CpuMpData;
+}
+
/**
MP Initialize Library initialization.
@@ -1803,7 +1852,7 @@ MpInitLibInitialize (
VOID
)
{
- CPU_MP_DATA *OldCpuMpData;
+ MP_HAND_OFF *MpHandOff;
CPU_INFO_IN_HOB *CpuInfoInHob;
UINT32 MaxLogicalProcessorNumber;
UINT32 ApStackSize;
@@ -1822,11 +1871,11 @@ MpInitLibInitialize (
UINTN BackupBufferAddr;
UINTN ApIdtBase;
- OldCpuMpData = GetCpuMpDataFromGuidedHob ();
- if (OldCpuMpData == NULL) {
+ MpHandOff = GetMpHandOffHob ();
+ if (MpHandOff == NULL) {
MaxLogicalProcessorNumber = PcdGet32 (PcdCpuMaxLogicalProcessorNumber);
} else {
- MaxLogicalProcessorNumber = OldCpuMpData->CpuCount;
+ MaxLogicalProcessorNumber = MpHandOff->CpuCount;
}
ASSERT (MaxLogicalProcessorNumber != 0);
@@ -1970,7 +2019,7 @@ MpInitLibInitialize (
//
ProgramVirtualWireMode ();
- if (OldCpuMpData == NULL) {
+ if (MpHandOff == NULL) {
if (MaxLogicalProcessorNumber > 1) {
//
// Wakeup all APs and calculate the processor count in system
@@ -1982,15 +2031,18 @@ MpInitLibInitialize (
// APs have been wakeup before, just get the CPU Information
// from HOB
//
- OldCpuMpData->NewCpuMpData = CpuMpData;
- CpuMpData->CpuCount = OldCpuMpData->CpuCount;
- CpuMpData->BspNumber = OldCpuMpData->BspNumber;
- CpuMpData->CpuInfoInHob = OldCpuMpData->CpuInfoInHob;
- CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
+ AmdSevUpdateCpuMpData (CpuMpData);
+ CpuMpData->CpuCount = MpHandOff->CpuCount;
+ CpuMpData->BspNumber = GetBspNumber (MpHandOff);
+ CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
InitializeSpinLock (&CpuMpData->CpuData[Index].ApLock);
- CpuMpData->CpuData[Index].CpuHealthy = (CpuInfoInHob[Index].Health == 0) ? TRUE : FALSE;
+ CpuMpData->CpuData[Index].CpuHealthy = (MpHandOff->Info[Index].Health == 0) ? TRUE : FALSE;
CpuMpData->CpuData[Index].ApFunction = 0;
+ CpuInfoInHob[Index].InitialApicId = MpHandOff->Info[Index].ApicId;
+ CpuInfoInHob[Index].ApTopOfStack = CpuMpData->Buffer + (Index + 1) * CpuMpData->CpuApStackSize;
+ CpuInfoInHob[Index].ApicId = MpHandOff->Info[Index].ApicId;
+ CpuInfoInHob[Index].Health = MpHandOff->Info[Index].Health;
}
}
@@ -2019,7 +2071,7 @@ MpInitLibInitialize (
// Wakeup APs to do some AP initialize sync (Microcode & MTRR)
//
if (CpuMpData->CpuCount > 1) {
- if (OldCpuMpData != NULL) {
+ if (MpHandOff != NULL) {
//
// Only needs to use this flag for DXE phase to update the wake up
// buffer. Wakeup buffer allocated in PEI phase is no longer valid
@@ -2036,7 +2088,7 @@ MpInitLibInitialize (
CpuPause ();
}
- if (OldCpuMpData != NULL) {
+ if (MpHandOff != NULL) {
CpuMpData->InitFlag = ApInitDone;
}
@@ -2763,6 +2815,28 @@ StartupThisAPWorker (
return Status;
}
+/**
+ Get pointer to MP_HAND_OFF GUIDed HOB.
+
+ @return The pointer to MP_HAND_OFF structure.
+**/
+MP_HAND_OFF *
+GetMpHandOffHob (
+ VOID
+ )
+{
+ EFI_HOB_GUID_TYPE *GuidHob;
+ MP_HAND_OFF *MpHandOff;
+
+ MpHandOff = NULL;
+ GuidHob = GetFirstGuidHob (&mMpHandOffGuid);
+ if (GuidHob != NULL) {
+ MpHandOff = (MP_HAND_OFF *)GET_GUID_HOB_DATA (GuidHob);
+ }
+
+ return MpHandOff;
+}
+
/**
Get pointer to CPU MP Data structure from GUIDed HOB.
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index b694c7b40f..9bb70415f3 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -40,6 +40,8 @@
#include <Guid/MicrocodePatchHob.h>
+#include "MpHandOff.h"
+
#define WAKEUP_AP_SIGNAL SIGNATURE_32 ('S', 'T', 'A', 'P')
#define CPU_INIT_MP_LIB_HOB_GUID \
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
index 78eded96bf..1aacb9b4cf 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
@@ -36,6 +36,7 @@
MpLib.c
MpLib.h
Microcode.c
+ MpHandOff.h
[Packages]
MdePkg/MdePkg.dec
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
index 013f89b197..f80e00edcf 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
@@ -126,7 +126,37 @@ SaveCpuMpData (
IN CPU_MP_DATA *CpuMpData
)
{
- UINT64 Data64;
+ UINT64 Data64;
+ UINTN Index;
+ CPU_INFO_IN_HOB *CpuInfoInHob;
+ MP_HAND_OFF *MpHandOff;
+ UINTN MpHandOffSize;
+
+ //
+ // When APs are in a state that can be waken up by a store operation to a memory address,
+ // report the MP_HAND_OFF data for DXE to use.
+ //
+ CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
+ MpHandOffSize = sizeof (MP_HAND_OFF) + sizeof (PROCESSOR_HAND_OFF) * CpuMpData->CpuCount;
+ MpHandOff = (MP_HAND_OFF *)BuildGuidHob (&mMpHandOffGuid, MpHandOffSize);
+ ASSERT (MpHandOff != NULL);
+ ZeroMem (MpHandOff, MpHandOffSize);
+ MpHandOff->ProcessorIndex = 0;
+
+ MpHandOff->CpuCount = CpuMpData->CpuCount;
+ if (CpuMpData->ApLoopMode != ApInHltLoop) {
+ MpHandOff->StartupSignalValue = MP_HAND_OFF_SIGNAL;
+ MpHandOff->WaitLoopExecutionMode = sizeof (VOID *);
+ }
+
+ for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
+ MpHandOff->Info[Index].ApicId = CpuInfoInHob[Index].ApicId;
+ MpHandOff->Info[Index].Health = CpuInfoInHob[Index].Health;
+ if (CpuMpData->ApLoopMode != ApInHltLoop) {
+ MpHandOff->Info[Index].StartupSignalAddress = (UINT64)(UINTN)CpuMpData->CpuData[Index].StartupApSignal;
+ MpHandOff->Info[Index].StartupProcedureAddress = (UINT64)(UINTN)&CpuMpData->CpuData[Index].ApFunction;
+ }
+ }
//
// Build location of CPU MP DATA buffer in HOB
--
2.36.1.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [edk2-devel] [PATCH 1/5] UefiCpuPkg: Create MpHandOff to transfer info from PEI to DXE.
2023-06-12 13:37 ` [PATCH 1/5] UefiCpuPkg: Create MpHandOff to transfer info from PEI to DXE Yuanhao Xie
@ 2023-06-20 14:08 ` Gerd Hoffmann
2023-06-26 1:50 ` Yuanhao Xie
0 siblings, 1 reply; 10+ messages in thread
From: Gerd Hoffmann @ 2023-06-20 14:08 UTC (permalink / raw)
To: devel, yuanhao.xie; +Cc: Eric Dong, Ray Ni, Rahul Kumar, Tom Lendacky
Hi,
> +typedef struct {
> + UINT32 ApicId;
> + UINT32 Health;
> + UINT64 StartupSignalAddress;
> + UINT64 StartupProcedureAddress;
> +} PROCESSOR_HAND_OFF;
> +
> +typedef struct {
> + //
> + // The ProcessorIndex indicates the range of processors. If it is set to 0, it signifies
> + // processors from 0 to CpuCount - 1. Multiple instances in the HOB list describe
> + // processors from ProcessorIndex to ProcessorIndex + CpuCount - 1.
> + //
> + UINT32 ProcessorIndex;
> + UINT32 CpuCount;
> + UINT32 WaitLoopExecutionMode;
> + UINT32 StartupSignalValue;
> + PROCESSOR_HAND_OFF Info[];
> +} MP_HAND_OFF;
This works for ~2700 processors, with larger numbers you hit the
hob size limit. Not a good idea IMHO, we already have systems
that big.
take care,
Gerd
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [edk2-devel] [PATCH 1/5] UefiCpuPkg: Create MpHandOff to transfer info from PEI to DXE.
2023-06-20 14:08 ` [edk2-devel] " Gerd Hoffmann
@ 2023-06-26 1:50 ` Yuanhao Xie
0 siblings, 0 replies; 10+ messages in thread
From: Yuanhao Xie @ 2023-06-26 1:50 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: devel@edk2.groups.io
Hi Gerd,
I have updated is in V2.
Please have a check.
Thanks
Yuanhao
-----Original Message-----
From: Gerd Hoffmann <kraxel@redhat.com>
Sent: Tuesday, June 20, 2023 10:09 PM
To: devel@edk2.groups.io; Xie, Yuanhao <yuanhao.xie@intel.com>
Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>
Subject: Re: [edk2-devel] [PATCH 1/5] UefiCpuPkg: Create MpHandOff to transfer info from PEI to DXE.
Hi,
> +typedef struct {
> + UINT32 ApicId;
> + UINT32 Health;
> + UINT64 StartupSignalAddress;
> + UINT64 StartupProcedureAddress;
> +} PROCESSOR_HAND_OFF;
> +
> +typedef struct {
> + //
> + // The ProcessorIndex indicates the range of processors. If it is
> +set to 0, it signifies
> + // processors from 0 to CpuCount - 1. Multiple instances in the HOB
> +list describe
> + // processors from ProcessorIndex to ProcessorIndex + CpuCount - 1.
> + //
> + UINT32 ProcessorIndex;
> + UINT32 CpuCount;
> + UINT32 WaitLoopExecutionMode;
> + UINT32 StartupSignalValue;
> + PROCESSOR_HAND_OFF Info[];
> +} MP_HAND_OFF;
This works for ~2700 processors, with larger numbers you hit the hob size limit. Not a good idea IMHO, we already have systems that big.
take care,
Gerd
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/5] UefiCpuPkg: Refactor the logic for placing APs in HltLoop.
2023-06-12 13:37 [PATCH 0/5] Eliminate the second INIT-SIPI-SIPI sequence Yuanhao Xie
2023-06-12 13:37 ` [PATCH 1/5] UefiCpuPkg: Create MpHandOff to transfer info from PEI to DXE Yuanhao Xie
@ 2023-06-12 13:37 ` Yuanhao Xie
2023-06-12 13:37 ` [PATCH 3/5] UefiCpuPkg: Refactor the logic for placing APs in Mwait/Runloop Yuanhao Xie
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Yuanhao Xie @ 2023-06-12 13:37 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar, Yuanhao Xie
Refactor the logic for placing APs in HltLoop into a separate function.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
---
UefiCpuPkg/Library/MpInitLib/MpLib.c | 35 ++++++++++++++++++++++++-----------
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index c55150e7b6..4e517a9b19 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -637,6 +637,28 @@ InitializeApData (
SetApState (&CpuMpData->CpuData[ProcessorNumber], CpuStateIdle);
}
+/**
+ This function place APs in Halt loop.
+
+ @param[in] CpuMpData Pointer to CPU MP Data
+**/
+VOID
+PlaceAPInHltLoop (
+ IN CPU_MP_DATA *CpuMpData
+ )
+{
+ while (TRUE) {
+ DisableInterrupts ();
+ if (CpuMpData->UseSevEsAPMethod) {
+ SevEsPlaceApHlt (CpuMpData);
+ } else {
+ CpuSleep ();
+ }
+
+ CpuPause ();
+ }
+}
+
/**
This function will be called from AP reset code if BSP uses WakeUpAP.
@@ -813,19 +835,10 @@ ApWakeupFunction (
// Place AP is specified loop mode
//
if (CpuMpData->ApLoopMode == ApInHltLoop) {
+ PlaceAPInHltLoop (CpuMpData);
//
- // Place AP in HLT-loop
+ // Never run here
//
- while (TRUE) {
- DisableInterrupts ();
- if (CpuMpData->UseSevEsAPMethod) {
- SevEsPlaceApHlt (CpuMpData);
- } else {
- CpuSleep ();
- }
-
- CpuPause ();
- }
}
while (TRUE) {
--
2.36.1.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/5] UefiCpuPkg: Refactor the logic for placing APs in Mwait/Runloop.
2023-06-12 13:37 [PATCH 0/5] Eliminate the second INIT-SIPI-SIPI sequence Yuanhao Xie
2023-06-12 13:37 ` [PATCH 1/5] UefiCpuPkg: Create MpHandOff to transfer info from PEI to DXE Yuanhao Xie
2023-06-12 13:37 ` [PATCH 2/5] UefiCpuPkg: Refactor the logic for placing APs in HltLoop Yuanhao Xie
@ 2023-06-12 13:37 ` Yuanhao Xie
2023-06-20 14:10 ` [edk2-devel] " Gerd Hoffmann
2023-06-12 13:37 ` [PATCH 4/5] UefiCpuPkg: ApWakeupFunction directly use CpuMpData Yuanhao Xie
2023-06-12 13:37 ` [PATCH 5/5] UefiCpuPkg: Eliminate the second INIT-SIPI-SIPI sequence Yuanhao Xie
4 siblings, 1 reply; 10+ messages in thread
From: Yuanhao Xie @ 2023-06-12 13:37 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar, Yuanhao Xie
Refactor the logic for placing APs in
Mwait/Runloop into a separate function.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
---
UefiCpuPkg/Library/MpInitLib/MpLib.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------
1 file changed, 50 insertions(+), 33 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 4e517a9b19..8aecd29a94 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -659,6 +659,54 @@ PlaceAPInHltLoop (
}
}
+/**
+ This function place APs in Mwait or Run loop.
+
+ @param[in] ApLoopMode Ap Loop Mode
+ @param[in] ApStartupSignalBuffer Pointer to Ap Startup Signal Buffer
+ @param[in] ApTargetCState Ap Target CState
+**/
+VOID
+PlaceAPInMwaitLoopOrRunLoop (
+ IN UINT8 ApLoopMode,
+ IN volatile UINT32 *ApStartupSignalBuffer,
+ IN UINT8 ApTargetCState
+ )
+{
+ while (TRUE) {
+ DisableInterrupts ();
+ if (ApLoopMode == ApInMwaitLoop) {
+ //
+ // Place AP in MWAIT-loop
+ //
+ AsmMonitor ((UINTN)ApStartupSignalBuffer, 0, 0);
+ if ((*ApStartupSignalBuffer != WAKEUP_AP_SIGNAL) && (*ApStartupSignalBuffer != MP_HAND_OFF_SIGNAL)) {
+ //
+ // Check AP start-up signal again.
+ // If AP start-up signal is not set, place AP into
+ // the specified C-state
+ //
+ AsmMwait (ApTargetCState << 4, 0);
+ }
+ } else if (ApLoopMode == ApInRunLoop) {
+ //
+ // Place AP in Run-loop
+ //
+ CpuPause ();
+ } else {
+ ASSERT (FALSE);
+ }
+
+ //
+ // If AP start-up signal is written, AP is waken up
+ // otherwise place AP in loop again
+ //
+ if ((*ApStartupSignalBuffer == WAKEUP_AP_SIGNAL) || (*ApStartupSignalBuffer == MP_HAND_OFF_SIGNAL)) {
+ break;
+ }
+ }
+}
+
/**
This function will be called from AP reset code if BSP uses WakeUpAP.
@@ -839,39 +887,8 @@ ApWakeupFunction (
//
// Never run here
//
- }
-
- while (TRUE) {
- DisableInterrupts ();
- if (CpuMpData->ApLoopMode == ApInMwaitLoop) {
- //
- // Place AP in MWAIT-loop
- //
- AsmMonitor ((UINTN)ApStartupSignalBuffer, 0, 0);
- if (*ApStartupSignalBuffer != WAKEUP_AP_SIGNAL) {
- //
- // Check AP start-up signal again.
- // If AP start-up signal is not set, place AP into
- // the specified C-state
- //
- AsmMwait (CpuMpData->ApTargetCState << 4, 0);
- }
- } else if (CpuMpData->ApLoopMode == ApInRunLoop) {
- //
- // Place AP in Run-loop
- //
- CpuPause ();
- } else {
- ASSERT (FALSE);
- }
-
- //
- // If AP start-up signal is written, AP is waken up
- // otherwise place AP in loop again
- //
- if (*ApStartupSignalBuffer == WAKEUP_AP_SIGNAL) {
- break;
- }
+ } else {
+ PlaceAPInMwaitLoopOrRunLoop (CpuMpData->ApLoopMode, ApStartupSignalBuffer, CpuMpData->ApTargetCState);
}
}
}
--
2.36.1.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [edk2-devel] [PATCH 3/5] UefiCpuPkg: Refactor the logic for placing APs in Mwait/Runloop.
2023-06-12 13:37 ` [PATCH 3/5] UefiCpuPkg: Refactor the logic for placing APs in Mwait/Runloop Yuanhao Xie
@ 2023-06-20 14:10 ` Gerd Hoffmann
2023-06-26 1:45 ` Yuanhao Xie
0 siblings, 1 reply; 10+ messages in thread
From: Gerd Hoffmann @ 2023-06-20 14:10 UTC (permalink / raw)
To: devel, yuanhao.xie; +Cc: Eric Dong, Ray Ni, Rahul Kumar
On Mon, Jun 12, 2023 at 09:37:18PM +0800, Yuanhao Xie wrote:
> Refactor the logic for placing APs in
> Mwait/Runloop into a separate function.
> + // If AP start-up signal is written, AP is waken up
> + // otherwise place AP in loop again
> + //
> + if ((*ApStartupSignalBuffer == WAKEUP_AP_SIGNAL) || (*ApStartupSignalBuffer == MP_HAND_OFF_SIGNAL)) {
> + break;
> + }
> - // If AP start-up signal is written, AP is waken up
> - // otherwise place AP in loop again
> - //
> - if (*ApStartupSignalBuffer == WAKEUP_AP_SIGNAL) {
> - break;
> - }
Nope. This isn't refactoring. Please split the patch into two, one
doing the refactoring without functional change, and one adding the code
for MP_HAND_OFF_SIGNAL.
take care,
Gerd
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [edk2-devel] [PATCH 3/5] UefiCpuPkg: Refactor the logic for placing APs in Mwait/Runloop.
2023-06-20 14:10 ` [edk2-devel] " Gerd Hoffmann
@ 2023-06-26 1:45 ` Yuanhao Xie
0 siblings, 0 replies; 10+ messages in thread
From: Yuanhao Xie @ 2023-06-26 1:45 UTC (permalink / raw)
To: kraxel@redhat.com; +Cc: devel@edk2.groups.io
Hi Gerd,
Thanks for the feedback.
I have updated in V2.
Please have a check.
Yuanhao
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd Hoffmann
Sent: Tuesday, June 20, 2023 10:11 PM
To: devel@edk2.groups.io; Xie, Yuanhao <yuanhao.xie@intel.com>
Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>
Subject: Re: [edk2-devel] [PATCH 3/5] UefiCpuPkg: Refactor the logic for placing APs in Mwait/Runloop.
On Mon, Jun 12, 2023 at 09:37:18PM +0800, Yuanhao Xie wrote:
> Refactor the logic for placing APs in
> Mwait/Runloop into a separate function.
> + // If AP start-up signal is written, AP is waken up
> + // otherwise place AP in loop again
> + //
> + if ((*ApStartupSignalBuffer == WAKEUP_AP_SIGNAL) || (*ApStartupSignalBuffer == MP_HAND_OFF_SIGNAL)) {
> + break;
> + }
> - // If AP start-up signal is written, AP is waken up
> - // otherwise place AP in loop again
> - //
> - if (*ApStartupSignalBuffer == WAKEUP_AP_SIGNAL) {
> - break;
> - }
Nope. This isn't refactoring. Please split the patch into two, one doing the refactoring without functional change, and one adding the code for MP_HAND_OFF_SIGNAL.
take care,
Gerd
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/5] UefiCpuPkg: ApWakeupFunction directly use CpuMpData.
2023-06-12 13:37 [PATCH 0/5] Eliminate the second INIT-SIPI-SIPI sequence Yuanhao Xie
` (2 preceding siblings ...)
2023-06-12 13:37 ` [PATCH 3/5] UefiCpuPkg: Refactor the logic for placing APs in Mwait/Runloop Yuanhao Xie
@ 2023-06-12 13:37 ` Yuanhao Xie
2023-06-12 13:37 ` [PATCH 5/5] UefiCpuPkg: Eliminate the second INIT-SIPI-SIPI sequence Yuanhao Xie
4 siblings, 0 replies; 10+ messages in thread
From: Yuanhao Xie @ 2023-06-12 13:37 UTC (permalink / raw)
To: devel
Cc: Gerd Hoffmann, Eric Dong, Ray Ni, Rahul Kumar, Tom Lendacky,
Yuanhao Xie
CpuMpData can be passed directly to the ApWakeUpFunction instead of
through MP_CPU_EXCHANGE_INFO. This modification is made
in preparation for eliminating the requirement of a second
INIT-SIPI-SIPI sequence in the DXE phase.
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
---
UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm | 4 ++--
UefiCpuPkg/Library/MpInitLib/MpLib.c | 12 +++---------
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 3 +--
3 files changed, 6 insertions(+), 13 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
index 59db4081d6..d117f09ef5 100644
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
@@ -197,8 +197,8 @@ CProcedureInvoke:
push ebx ; Push ApIndex
mov eax, esi
- add eax, MP_CPU_EXCHANGE_INFO_OFFSET
- push eax ; push address of exchange info data buffer
+ add eax, MP_CPU_EXCHANGE_INFO_FIELD (CpuMpData)
+ push dword [eax] ; push address of CpuMpData
mov edi, esi
add edi, MP_CPU_EXCHANGE_INFO_FIELD (CFunction)
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 8aecd29a94..e56f2455dc 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -710,17 +710,16 @@ PlaceAPInMwaitLoopOrRunLoop (
/**
This function will be called from AP reset code if BSP uses WakeUpAP.
- @param[in] ExchangeInfo Pointer to the MP exchange info buffer
+ @param[in] CpuMpData Pointer to CPU MP Data
@param[in] ApIndex Number of current executing AP
**/
VOID
EFIAPI
ApWakeupFunction (
- IN MP_CPU_EXCHANGE_INFO *ExchangeInfo,
- IN UINTN ApIndex
+ IN CPU_MP_DATA *CpuMpData,
+ IN UINTN ApIndex
)
{
- CPU_MP_DATA *CpuMpData;
UINTN ProcessorNumber;
EFI_AP_PROCEDURE Procedure;
VOID *Parameter;
@@ -731,11 +730,6 @@ ApWakeupFunction (
UINTN CurrentApicMode;
AP_STACK_DATA *ApStackData;
- //
- // AP finished assembly code and begin to execute C code
- //
- CpuMpData = ExchangeInfo->CpuMpData;
-
//
// AP's local APIC settings will be lost after received INIT IPI
// We need to re-initialize them at here
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index 5bcdf7726b..40e80ffab4 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -259,8 +259,7 @@ CProcedureInvoke:
add rsp, 20h
mov edx, ebx ; edx is ApIndex
- mov ecx, esi
- add ecx, MP_CPU_EXCHANGE_INFO_OFFSET ; rcx is address of exchange info data buffer
+ mov rcx, qword [esi + MP_CPU_EXCHANGE_INFO_FIELD (CpuMpData)]
mov edi, esi
add edi, MP_CPU_EXCHANGE_INFO_FIELD (CFunction)
--
2.36.1.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 5/5] UefiCpuPkg: Eliminate the second INIT-SIPI-SIPI sequence.
2023-06-12 13:37 [PATCH 0/5] Eliminate the second INIT-SIPI-SIPI sequence Yuanhao Xie
` (3 preceding siblings ...)
2023-06-12 13:37 ` [PATCH 4/5] UefiCpuPkg: ApWakeupFunction directly use CpuMpData Yuanhao Xie
@ 2023-06-12 13:37 ` Yuanhao Xie
4 siblings, 0 replies; 10+ messages in thread
From: Yuanhao Xie @ 2023-06-12 13:37 UTC (permalink / raw)
To: devel
Cc: Gerd Hoffmann, Eric Dong, Ray Ni, Rahul Kumar, Tom Lendacky,
Yuanhao Xie
When both the PEI and DXE phases operate in the same execution
mode(32-bit/64-bit), the BSP send a special start-up signal during
the DXE phase to awaken the Application APs.
To eliminate the need for the INIT-SIPI-SIPI sequence at the beginning
of the DXE phase, the BSP call the SwitchApContext function to trigger
the special start-up signal. By writing the specified
StartupSignalValue to the designated StartupSignalAddress, the BSP
wakes up the APs from mwait mode. Once the APs receive the
MP_HAND_OFF_SIGNAL value, they are awakened and proceed to execute the
SwitchContextPerAp procedure. They enter another while loop,
transitioning their context from the PEI phase to the DXE phase.
The original state transitions for an AP during the procedure are as
follows:
Idle ----> Ready ----> Busy ----> Idle
[BSP] [AP] [AP]
Instead of init-sipi-sipi sequence, we make use of a
start-up signal to awaken the APs and transfer their context from
PEI to DXE. Consequently, APs, rather than the BSP, to set their state
to CpuStateReady.
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
---
UefiCpuPkg/Library/MpInitLib/MpLib.c | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
UefiCpuPkg/Library/MpInitLib/MpLib.h | 8 ++++++++
2 files changed, 138 insertions(+)
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index e56f2455dc..325710b442 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -729,6 +729,7 @@ ApWakeupFunction (
UINT64 ApTopOfStack;
UINTN CurrentApicMode;
AP_STACK_DATA *ApStackData;
+ UINT32 OriginalValue;
//
// AP's local APIC settings will be lost after received INIT IPI
@@ -769,6 +770,15 @@ ApWakeupFunction (
// Clear AP start-up signal when AP waken up
//
ApStartupSignalBuffer = CpuMpData->CpuData[ProcessorNumber].StartupApSignal;
+ OriginalValue = InterlockedCompareExchange32 (
+ (UINT32 *)ApStartupSignalBuffer,
+ MP_HAND_OFF_SIGNAL,
+ 0
+ );
+ if (OriginalValue == MP_HAND_OFF_SIGNAL) {
+ SetApState (&CpuMpData->CpuData[ProcessorNumber], CpuStateReady);
+ }
+
InterlockedCompareExchange32 (
(UINT32 *)ApStartupSignalBuffer,
WAKEUP_AP_SIGNAL,
@@ -887,6 +897,31 @@ ApWakeupFunction (
}
}
+/**
+ This function serves as the entry point for APs when
+ they are awakened by the stores in the memory address
+ indicated by the MP_HANDOFF_INFO structure.
+ @param[in] CpuMpData Pointer to PEI CPU MP Data
+**/
+VOID
+EFIAPI
+DxeApEntryPoint (
+ CPU_MP_DATA *CpuMpData
+ )
+{
+ UINTN ProcessorNumber;
+
+ GetProcessorNumber (CpuMpData, &ProcessorNumber);
+ InterlockedIncrement ((UINT32 *)&CpuMpData->FinishedCount);
+ RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters, FALSE);
+ PlaceAPInMwaitLoopOrRunLoop (
+ CpuMpData->ApLoopMode,
+ CpuMpData->CpuData[ProcessorNumber].StartupApSignal,
+ CpuMpData->ApTargetCState
+ );
+ ApWakeupFunction (CpuMpData, ProcessorNumber);
+}
+
/**
Wait for AP wakeup and write AP start-up signal till AP is waken up.
@@ -1857,6 +1892,68 @@ AmdSevUpdateCpuMpData (
OldCpuMpData->NewCpuMpData = CpuMpData;
}
+/**
+ Switch Context for each AP.
+**/
+VOID
+EFIAPI
+SwitchContextPerAp (
+ VOID
+ )
+{
+ UINTN ProcessorNumber;
+ CPU_MP_DATA *CpuMpData;
+ CPU_INFO_IN_HOB *CpuInfoInHob;
+
+ CpuMpData = GetCpuMpData ();
+ CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
+ GetProcessorNumber (CpuMpData, &ProcessorNumber);
+
+ SwitchStack (
+ (SWITCH_STACK_ENTRY_POINT)(UINTN)DxeApEntryPoint,
+ (VOID *)(UINTN)CpuMpData,
+ NULL,
+ (VOID *)((UINTN)CpuInfoInHob[ProcessorNumber].ApTopOfStack)
+ );
+}
+
+/**
+ This function is intended to be invoked by the BSP in order
+ to wake up the AP. The BSP accomplishes this by triggering a
+ start-up signal, which in turn causes any APs that are
+ currently in a loop on the PEI-prepared memory to awaken and
+ begin running the procedure called SwitchContextPerAp.
+ This procedure allows the AP to switch to another section of
+ memory and continue its loop there.
+ @param[in] MpHandOff Pointer to MP hand-off data structure.
+**/
+VOID
+SwitchApContext (
+ IN MP_HAND_OFF *MpHandOff
+ )
+{
+ UINTN Index;
+ UINT32 BspNumber;
+
+ BspNumber = GetBspNumber (MpHandOff);
+
+ for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
+ if (Index != BspNumber) {
+ *(UINTN *)(UINTN)MpHandOff->Info[Index].StartupProcedureAddress = (UINTN)SwitchContextPerAp;
+ *(UINT32 *)(UINTN)MpHandOff->Info[Index].StartupSignalAddress = MpHandOff->StartupSignalValue;
+ }
+ }
+
+ //
+ // Wait all APs waken up if this is not the 1st broadcast of SIPI
+ //
+ for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
+ if (Index != BspNumber) {
+ WaitApWakeup ((UINT32 *)(UINTN)(MpHandOff->Info[Index].StartupSignalAddress));
+ }
+ }
+}
+
/**
MP Initialize Library initialization.
@@ -2068,6 +2165,39 @@ MpInitLibInitialize (
CpuInfoInHob[Index].ApicId = MpHandOff->Info[Index].ApicId;
CpuInfoInHob[Index].Health = MpHandOff->Info[Index].Health;
}
+
+ DEBUG ((DEBUG_INFO, "MpHandOff->WaitLoopExecutionMode: %04d, sizeof (VOID *): %04d\n", MpHandOff->WaitLoopExecutionMode, sizeof (VOID *)));
+ if (MpHandOff->WaitLoopExecutionMode == sizeof (VOID *)) {
+ //
+ // In scenarios where both the PEI and DXE phases run in the same
+ // execution mode (32bit or 64bit), the BSP triggers
+ // a start-up signal during the DXE phase to wake up the APs. This causes any
+ // APs that are currently in a loop on the memory prepared during the PEI
+ // phase to awaken and run the SwitchContextPerAp procedure. This procedure
+ // enables the APs to switch to a different memory section and continue their
+ // looping process there.
+ //
+ CpuMpData->FinishedCount = 0;
+ CpuMpData->InitFlag = ApInitDone;
+ SaveCpuMpData (CpuMpData);
+ SwitchApContext (MpHandOff);
+ ASSERT (CpuMpData->FinishedCount == (CpuMpData->CpuCount - 1));
+
+ //
+ // Set Apstate as Idle, otherwise Aps cannot be waken-up again.
+ // If any enabled AP is not idle, return EFI_NOT_READY during waken-up.
+ //
+ for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
+ SetApState (&CpuMpData->CpuData[Index], CpuStateIdle);
+ }
+
+ //
+ // Initialize global data for MP support
+ //
+ InitMpGlobalData (CpuMpData);
+
+ return EFI_SUCCESS;
+ }
}
if (!GetMicrocodePatchInfoFromHob (
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 9bb70415f3..2e7d1a0af5 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -467,6 +467,14 @@ GetWakeupBuffer (
IN UINTN WakeupBufferSize
);
+/**
+ Switch Context for each AP.
+**/
+VOID
+SwitchApContext (
+ IN MP_HAND_OFF *MpHandOff
+ );
+
/**
Get available EfiBootServicesCode memory below 4GB by specified size.
--
2.36.1.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread