* [edk2-devel] [PATCH v1] UefiCpuPkg/Library/MpInitLib: fix SEV-ES AP bootinng failure
@ 2024-07-26 15:58 Wencheng Yang
0 siblings, 0 replies; 3+ messages in thread
From: Wencheng Yang @ 2024-07-26 15:58 UTC (permalink / raw)
To: devel; +Cc: yuanhao.xie, brijesh.singh, min.m.xu, michael.kubacki,
Wencheng Yang
According to SEV-ES Guest-Hypervisor Communication Block Standardization
section 4.3 SMP Booting, the subsequent reset requires the AP enters
Reset Hold state either by AP Reset Hold NAE event or
AP Reset Hold Request MSR Protocol.
If the AP is not in AP Reset Hold state, it may miss subsequent
INIT-SIPI, as the INIT-SIPI process depends on GHCB page in kernel,
which is only mapped in VMGEXIT interception.
To ensure all APs are in AP Reset Hold state, we add a var in
AP specific data structure, set the var before the AP going to
AP Reset Hold state. Subsequent INIT-SIPI should check the var of each
AP before issuing INIT-SIPI signal.
Cc: Yuanhao Xie <yuanhao.xie@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Wencheng Yang <east.moutain.yang@gmail.com>
---
UefiCpuPkg/Library/MpInitLib/AmdSev.c | 11 ++++++-
UefiCpuPkg/Library/MpInitLib/MpHandOff.h | 1 +
UefiCpuPkg/Library/MpInitLib/MpLib.c | 39 ++++++++++++++++++++++--
UefiCpuPkg/Library/MpInitLib/MpLib.h | 4 ++-
UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 2 ++
5 files changed, 52 insertions(+), 5 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/AmdSev.c b/UefiCpuPkg/Library/MpInitLib/AmdSev.c
index d34f9513e0..e5d5ecb181 100644
--- a/UefiCpuPkg/Library/MpInitLib/AmdSev.c
+++ b/UefiCpuPkg/Library/MpInitLib/AmdSev.c
@@ -194,16 +194,19 @@ SetSevEsJumpTable (
**/
VOID
SevEsPlaceApHlt (
- CPU_MP_DATA *CpuMpData
+ CPU_MP_DATA *CpuMpData,
+ UINT32 ProcessorNumber
)
{
MSR_SEV_ES_GHCB_REGISTER Msr;
GHCB *Ghcb;
UINT64 Status;
BOOLEAN DoDecrement;
+ BOOLEAN EnterHltLoop;
BOOLEAN InterruptState;
DoDecrement = (BOOLEAN)(CpuMpData->InitFlag == ApInitConfig);
+ EnterHltLoop = FALSE;
while (TRUE) {
Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
@@ -221,7 +224,13 @@ SevEsPlaceApHlt (
InterlockedDecrement ((UINT32 *)&CpuMpData->MpCpuExchangeInfo->NumApsExecuting);
}
+ if (!EnterHltLoop) {
+ EnterHltLoop = TRUE;
+ CpuMpData->CpuData[ProcessorNumber].SevEsApEnterHltLoopAfterWakeup = 1;
+ }
+
Status = CcExitVmgExit (Ghcb, SVM_EXIT_AP_RESET_HOLD, 0, 0);
+
if ((Status == 0) && (Ghcb->SaveArea.SwExitInfo2 != 0)) {
CcExitVmgDone (Ghcb, InterruptState);
break;
diff --git a/UefiCpuPkg/Library/MpInitLib/MpHandOff.h b/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
index ae93b7e3d7..50e290f5b3 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
@@ -37,6 +37,7 @@ extern EFI_GUID mMpHandOffConfigGuid;
typedef struct {
UINT32 ApicId;
UINT32 Health;
+ UINT32 SevEsApEnterHltLoopAfterWakeup;
UINT64 StartupSignalAddress;
UINT64 StartupProcedureAddress;
} PROCESSOR_HAND_OFF;
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 1951922912..59f0d87f9d 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -660,13 +660,14 @@ InitializeApData (
**/
VOID
PlaceAPInHltLoop (
- IN CPU_MP_DATA *CpuMpData
+ IN CPU_MP_DATA *CpuMpData,
+ IN UINTN ProcessorNumber
)
{
while (TRUE) {
DisableInterrupts ();
if (CpuMpData->UseSevEsAPMethod) {
- SevEsPlaceApHlt (CpuMpData);
+ SevEsPlaceApHlt (CpuMpData, ProcessorNumber);
} else {
CpuSleep ();
}
@@ -762,6 +763,9 @@ ApWakeupFunction (
while (TRUE) {
if (CpuMpData->InitFlag == ApInitConfig) {
ProcessorNumber = ApIndex;
+ if (CpuMpData->UseSevEsAPMethod) {
+ CpuMpData->CpuData[ProcessorNumber].SevEsApEnterHltLoopAfterWakeup = 0;
+ }
//
// This is first time AP wakeup, get BIST information from AP stack
//
@@ -782,6 +786,9 @@ ApWakeupFunction (
// Execute AP function if AP is ready
//
GetProcessorNumber (CpuMpData, &ProcessorNumber);
+ if (CpuMpData->UseSevEsAPMethod) {
+ CpuMpData->CpuData[ProcessorNumber].SevEsApEnterHltLoopAfterWakeup = 0;
+ }
//
// Clear AP start-up signal when AP waken up
//
@@ -903,7 +910,7 @@ ApWakeupFunction (
// Place AP is specified loop mode
//
if (CpuMpData->ApLoopMode == ApInHltLoop) {
- PlaceAPInHltLoop (CpuMpData);
+ PlaceAPInHltLoop (CpuMpData, ProcessorNumber);
//
// Never run here
//
@@ -993,6 +1000,20 @@ GetApResetVectorSize (
*SizeAbove1Mb = AddressMap->RendezvousFunnelSize - AddressMap->ModeTransitionOffset;
}
}
+/**
+ Wait for SEV-ES AP enter in HLT-LOOP.^M
+
+ @param[in] SevEsApInHltLoop Pointer to SevEsApInHltLoop^M
+**/
+VOID
+WaitSevEsApEnterHltLoopAfterWakeup (
+ IN volatile UINT32 *SevEsApEnterHltLoop
+ )
+{
+ while (*(UINT32*)SevEsApEnterHltLoop == 0) {
+ CpuPause ();
+ }
+}
/**
This function will fill the exchange info structure.
@@ -1324,6 +1345,17 @@ WakeUpAP (
//
SendStartupIpiAllExcludingSelf ((UINT32)ExchangeInfo->BufferStart);
} else {
+ //
+ // Subsequent INIT-SIPI-SIPI
+ //
+ if (CpuMpData->SevEsIsEnabled && (CpuMpData->InitFlag != ApInitConfig)) {
+ for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
+ CpuData = &CpuMpData->CpuData[Index];
+ if (Index != CpuMpData->BspNumber) {
+ WaitSevEsApEnterHltLoopAfterWakeup(&CpuData->SevEsApEnterHltLoopAfterWakeup);
+ }
+ }
+ }
SendInitSipiSipiAllExcludingSelf ((UINT32)ExchangeInfo->BufferStart);
}
}
@@ -2270,6 +2302,7 @@ MpInitLibInitialize (
InitializeSpinLock (&CpuMpData->CpuData[Index].ApLock);
CpuMpData->CpuData[Index].CpuHealthy = (MpHandOff->Info[HobIndex].Health == 0) ? TRUE : FALSE;
CpuMpData->CpuData[Index].ApFunction = 0;
+ CpuMpData->CpuData[Index].SevEsApEnterHltLoopAfterWakeup = MpHandOff->Info[HobIndex].SevEsApEnterHltLoopAfterWakeup;
CpuInfoInHob[Index].InitialApicId = MpHandOff->Info[HobIndex].ApicId;
CpuInfoInHob[Index].ApTopOfStack = CpuMpData->Buffer + (Index + 1) * CpuMpData->CpuApStackSize;
CpuInfoInHob[Index].ApicId = MpHandOff->Info[HobIndex].ApicId;
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 88b31fecca..c33a7bf658 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -146,6 +146,7 @@ typedef enum {
typedef struct {
SPIN_LOCK ApLock;
volatile UINT32 *StartupApSignal;
+ volatile UINT32 SevEsApEnterHltLoopAfterWakeup;
volatile UINTN ApFunction;
volatile UINTN ApFunctionArgument;
BOOLEAN CpuHealthy;
@@ -862,7 +863,8 @@ SetSevEsJumpTable (
**/
VOID
SevEsPlaceApHlt (
- CPU_MP_DATA *CpuMpData
+ CPU_MP_DATA *CpuMpData,
+ UINT32 ProcessorNumber
);
/**
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
index 16a858d542..b393ce00be 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
@@ -164,6 +164,8 @@ SaveCpuMpData (
if (CpuMpData->ApLoopMode != ApInHltLoop) {
MpHandOff->Info[Index-HobBase].StartupSignalAddress = (UINT64)(UINTN)CpuMpData->CpuData[Index].StartupApSignal;
MpHandOff->Info[Index-HobBase].StartupProcedureAddress = (UINT64)(UINTN)&CpuMpData->CpuData[Index].ApFunction;
+ } else {
+ MpHandOff->Info[Index-HobBase].SevEsApEnterHltLoopAfterWakeup = CpuMpData->CpuData[Index].SevEsApEnterHltLoopAfterWakeup;
}
}
--
2.39.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120127): https://edk2.groups.io/g/devel/message/120127
Mute This Topic: https://groups.io/mt/107630732/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 3+ messages in thread* [edk2-devel] [PATCH v1] UefiCpuPkg/Library/MpInitLib: fix SEV-ES AP bootinng failure
@ 2024-07-31 10:22 Wencheng Yang
2024-08-01 14:55 ` Lendacky, Thomas via groups.io
0 siblings, 1 reply; 3+ messages in thread
From: Wencheng Yang @ 2024-07-31 10:22 UTC (permalink / raw)
To: devel; +Cc: yuanhao.xie, brijesh.singh, min.m.xu, michael.kubacki,
Wencheng Yang
According to SEV-ES Guest-Hypervisor Communication Block Standardization
section 4.3 SMP Booting, the subsequent reset requires the AP enters
Reset Hold state either by AP Reset Hold NAE event or
AP Reset Hold Request MSR Protocol.
If the AP is not in AP Reset Hold state, it may miss subsequent
INIT-SIPI, as the INIT-SIPI process depends on GHCB page in kernel,
which is only mapped in VMGEXIT interception.
To ensure all APs are in AP Reset Hold state, we add a var in
AP specific data structure, set the var before the AP going to
AP Reset Hold state. Subsequent INIT-SIPI should check the var of each
AP before issuing INIT-SIPI signal.
Cc: Yuanhao Xie <yuanhao.xie@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Wencheng Yang <east.moutain.yang@gmail.com>
---
UefiCpuPkg/Library/MpInitLib/AmdSev.c | 11 ++++++-
UefiCpuPkg/Library/MpInitLib/MpHandOff.h | 1 +
UefiCpuPkg/Library/MpInitLib/MpLib.c | 39 ++++++++++++++++++++++--
UefiCpuPkg/Library/MpInitLib/MpLib.h | 4 ++-
UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 2 ++
5 files changed, 52 insertions(+), 5 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/AmdSev.c b/UefiCpuPkg/Library/MpInitLib/AmdSev.c
index d34f9513e0..e5d5ecb181 100644
--- a/UefiCpuPkg/Library/MpInitLib/AmdSev.c
+++ b/UefiCpuPkg/Library/MpInitLib/AmdSev.c
@@ -194,16 +194,19 @@ SetSevEsJumpTable (
**/
VOID
SevEsPlaceApHlt (
- CPU_MP_DATA *CpuMpData
+ CPU_MP_DATA *CpuMpData,
+ UINT32 ProcessorNumber
)
{
MSR_SEV_ES_GHCB_REGISTER Msr;
GHCB *Ghcb;
UINT64 Status;
BOOLEAN DoDecrement;
+ BOOLEAN EnterHltLoop;
BOOLEAN InterruptState;
DoDecrement = (BOOLEAN)(CpuMpData->InitFlag == ApInitConfig);
+ EnterHltLoop = FALSE;
while (TRUE) {
Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
@@ -221,7 +224,13 @@ SevEsPlaceApHlt (
InterlockedDecrement ((UINT32 *)&CpuMpData->MpCpuExchangeInfo->NumApsExecuting);
}
+ if (!EnterHltLoop) {
+ EnterHltLoop = TRUE;
+ CpuMpData->CpuData[ProcessorNumber].SevEsApEnterHltLoopAfterWakeup = 1;
+ }
+
Status = CcExitVmgExit (Ghcb, SVM_EXIT_AP_RESET_HOLD, 0, 0);
+
if ((Status == 0) && (Ghcb->SaveArea.SwExitInfo2 != 0)) {
CcExitVmgDone (Ghcb, InterruptState);
break;
diff --git a/UefiCpuPkg/Library/MpInitLib/MpHandOff.h b/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
index ae93b7e3d7..50e290f5b3 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
@@ -37,6 +37,7 @@ extern EFI_GUID mMpHandOffConfigGuid;
typedef struct {
UINT32 ApicId;
UINT32 Health;
+ UINT32 SevEsApEnterHltLoopAfterWakeup;
UINT64 StartupSignalAddress;
UINT64 StartupProcedureAddress;
} PROCESSOR_HAND_OFF;
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 1951922912..59f0d87f9d 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -660,13 +660,14 @@ InitializeApData (
**/
VOID
PlaceAPInHltLoop (
- IN CPU_MP_DATA *CpuMpData
+ IN CPU_MP_DATA *CpuMpData,
+ IN UINTN ProcessorNumber
)
{
while (TRUE) {
DisableInterrupts ();
if (CpuMpData->UseSevEsAPMethod) {
- SevEsPlaceApHlt (CpuMpData);
+ SevEsPlaceApHlt (CpuMpData, ProcessorNumber);
} else {
CpuSleep ();
}
@@ -762,6 +763,9 @@ ApWakeupFunction (
while (TRUE) {
if (CpuMpData->InitFlag == ApInitConfig) {
ProcessorNumber = ApIndex;
+ if (CpuMpData->UseSevEsAPMethod) {
+ CpuMpData->CpuData[ProcessorNumber].SevEsApEnterHltLoopAfterWakeup = 0;
+ }
//
// This is first time AP wakeup, get BIST information from AP stack
//
@@ -782,6 +786,9 @@ ApWakeupFunction (
// Execute AP function if AP is ready
//
GetProcessorNumber (CpuMpData, &ProcessorNumber);
+ if (CpuMpData->UseSevEsAPMethod) {
+ CpuMpData->CpuData[ProcessorNumber].SevEsApEnterHltLoopAfterWakeup = 0;
+ }
//
// Clear AP start-up signal when AP waken up
//
@@ -903,7 +910,7 @@ ApWakeupFunction (
// Place AP is specified loop mode
//
if (CpuMpData->ApLoopMode == ApInHltLoop) {
- PlaceAPInHltLoop (CpuMpData);
+ PlaceAPInHltLoop (CpuMpData, ProcessorNumber);
//
// Never run here
//
@@ -993,6 +1000,20 @@ GetApResetVectorSize (
*SizeAbove1Mb = AddressMap->RendezvousFunnelSize - AddressMap->ModeTransitionOffset;
}
}
+/**
+ Wait for SEV-ES AP enter in HLT-LOOP.^M
+
+ @param[in] SevEsApInHltLoop Pointer to SevEsApInHltLoop^M
+**/
+VOID
+WaitSevEsApEnterHltLoopAfterWakeup (
+ IN volatile UINT32 *SevEsApEnterHltLoop
+ )
+{
+ while (*(UINT32*)SevEsApEnterHltLoop == 0) {
+ CpuPause ();
+ }
+}
/**
This function will fill the exchange info structure.
@@ -1324,6 +1345,17 @@ WakeUpAP (
//
SendStartupIpiAllExcludingSelf ((UINT32)ExchangeInfo->BufferStart);
} else {
+ //
+ // Subsequent INIT-SIPI-SIPI
+ //
+ if (CpuMpData->SevEsIsEnabled && (CpuMpData->InitFlag != ApInitConfig)) {
+ for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
+ CpuData = &CpuMpData->CpuData[Index];
+ if (Index != CpuMpData->BspNumber) {
+ WaitSevEsApEnterHltLoopAfterWakeup(&CpuData->SevEsApEnterHltLoopAfterWakeup);
+ }
+ }
+ }
SendInitSipiSipiAllExcludingSelf ((UINT32)ExchangeInfo->BufferStart);
}
}
@@ -2270,6 +2302,7 @@ MpInitLibInitialize (
InitializeSpinLock (&CpuMpData->CpuData[Index].ApLock);
CpuMpData->CpuData[Index].CpuHealthy = (MpHandOff->Info[HobIndex].Health == 0) ? TRUE : FALSE;
CpuMpData->CpuData[Index].ApFunction = 0;
+ CpuMpData->CpuData[Index].SevEsApEnterHltLoopAfterWakeup = MpHandOff->Info[HobIndex].SevEsApEnterHltLoopAfterWakeup;
CpuInfoInHob[Index].InitialApicId = MpHandOff->Info[HobIndex].ApicId;
CpuInfoInHob[Index].ApTopOfStack = CpuMpData->Buffer + (Index + 1) * CpuMpData->CpuApStackSize;
CpuInfoInHob[Index].ApicId = MpHandOff->Info[HobIndex].ApicId;
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 88b31fecca..c33a7bf658 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -146,6 +146,7 @@ typedef enum {
typedef struct {
SPIN_LOCK ApLock;
volatile UINT32 *StartupApSignal;
+ volatile UINT32 SevEsApEnterHltLoopAfterWakeup;
volatile UINTN ApFunction;
volatile UINTN ApFunctionArgument;
BOOLEAN CpuHealthy;
@@ -862,7 +863,8 @@ SetSevEsJumpTable (
**/
VOID
SevEsPlaceApHlt (
- CPU_MP_DATA *CpuMpData
+ CPU_MP_DATA *CpuMpData,
+ UINT32 ProcessorNumber
);
/**
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
index 16a858d542..b393ce00be 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
@@ -164,6 +164,8 @@ SaveCpuMpData (
if (CpuMpData->ApLoopMode != ApInHltLoop) {
MpHandOff->Info[Index-HobBase].StartupSignalAddress = (UINT64)(UINTN)CpuMpData->CpuData[Index].StartupApSignal;
MpHandOff->Info[Index-HobBase].StartupProcedureAddress = (UINT64)(UINTN)&CpuMpData->CpuData[Index].ApFunction;
+ } else {
+ MpHandOff->Info[Index-HobBase].SevEsApEnterHltLoopAfterWakeup = CpuMpData->CpuData[Index].SevEsApEnterHltLoopAfterWakeup;
}
}
--
2.39.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120138): https://edk2.groups.io/g/devel/message/120138
Mute This Topic: https://groups.io/mt/107630732/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [edk2-devel] [PATCH v1] UefiCpuPkg/Library/MpInitLib: fix SEV-ES AP bootinng failure
2024-07-31 10:22 Wencheng Yang
@ 2024-08-01 14:55 ` Lendacky, Thomas via groups.io
0 siblings, 0 replies; 3+ messages in thread
From: Lendacky, Thomas via groups.io @ 2024-08-01 14:55 UTC (permalink / raw)
To: devel, east.moutain.yang
Cc: yuanhao.xie, brijesh.singh, min.m.xu, michael.kubacki
On 7/31/24 05:22, Wencheng Yang via groups.io wrote:
> According to SEV-ES Guest-Hypervisor Communication Block Standardization
> section 4.3 SMP Booting, the subsequent reset requires the AP enters
> Reset Hold state either by AP Reset Hold NAE event or
> AP Reset Hold Request MSR Protocol.
>
> If the AP is not in AP Reset Hold state, it may miss subsequent
> INIT-SIPI, as the INIT-SIPI process depends on GHCB page in kernel,
> which is only mapped in VMGEXIT interception.
>
> To ensure all APs are in AP Reset Hold state, we add a var in
> AP specific data structure, set the var before the AP going to
> AP Reset Hold state. Subsequent INIT-SIPI should check the var of each
> AP before issuing INIT-SIPI signal.
Were you actually seeing an issue here?
The moving of InterlockedDecrement() was for the same reason as this
patch. I assume this was discovered when the CpuMpData->InitFlag wasn't
set to ApInitConfig, which skips the InterlockedDecrement()? What if the
InterlockedIncrement() was delayed like InterlockedDecrement(), would
that also fix the issue? If so, that would remove the changes to, and
related to, the CPU_AP_DATA struct.
If not, then using this method can remove the changes around the
InterlockedDecrement() in the ApWakeupFunction() and SevEsPlaceApHlt()
and just rely on the EnteredHlt flag/
I just saw this and haven't had a chance to test it, yet. I'm very busy
at the moment but will try to get to it very soon.
>
> Cc: Yuanhao Xie <yuanhao.xie@intel.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Min Xu <min.m.xu@intel.com>
> Cc: Michael Kubacki <michael.kubacki@microsoft.com>
>
> Signed-off-by: Wencheng Yang <east.moutain.yang@gmail.com>
> ---
> UefiCpuPkg/Library/MpInitLib/AmdSev.c | 11 ++++++-
> UefiCpuPkg/Library/MpInitLib/MpHandOff.h | 1 +
> UefiCpuPkg/Library/MpInitLib/MpLib.c | 39 ++++++++++++++++++++++--
> UefiCpuPkg/Library/MpInitLib/MpLib.h | 4 ++-
> UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 2 ++
> 5 files changed, 52 insertions(+), 5 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/AmdSev.c b/UefiCpuPkg/Library/MpInitLib/AmdSev.c
> index d34f9513e0..e5d5ecb181 100644
> --- a/UefiCpuPkg/Library/MpInitLib/AmdSev.c
> +++ b/UefiCpuPkg/Library/MpInitLib/AmdSev.c
> @@ -194,16 +194,19 @@ SetSevEsJumpTable (
> **/
> VOID
> SevEsPlaceApHlt (
> - CPU_MP_DATA *CpuMpData
> + CPU_MP_DATA *CpuMpData,
> + UINT32 ProcessorNumber
The alignment is off here (and in other places below). You'll need to
run this through uncrustify so that it doesn't fail CI.
> )
> {
> MSR_SEV_ES_GHCB_REGISTER Msr;
> GHCB *Ghcb;
> UINT64 Status;
> BOOLEAN DoDecrement;
> + BOOLEAN EnterHltLoop;
Maybe a better name is EnteredHlt.
> BOOLEAN InterruptState;
>
> DoDecrement = (BOOLEAN)(CpuMpData->InitFlag == ApInitConfig);
> + EnterHltLoop = FALSE;
>
> while (TRUE) {
> Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
> @@ -221,7 +224,13 @@ SevEsPlaceApHlt (
> InterlockedDecrement ((UINT32 *)&CpuMpData->MpCpuExchangeInfo->NumApsExecuting);
> }
>
> + if (!EnterHltLoop) {
> + EnterHltLoop = TRUE;
> + CpuMpData->CpuData[ProcessorNumber].SevEsApEnterHltLoopAfterWakeup = 1;
> + }
> +
> Status = CcExitVmgExit (Ghcb, SVM_EXIT_AP_RESET_HOLD, 0, 0);
> +
Remove the added blank line.
> if ((Status == 0) && (Ghcb->SaveArea.SwExitInfo2 != 0)) {
> CcExitVmgDone (Ghcb, InterruptState);
> break;
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpHandOff.h b/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
> index ae93b7e3d7..50e290f5b3 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
> @@ -37,6 +37,7 @@ extern EFI_GUID mMpHandOffConfigGuid;
> typedef struct {
> UINT32 ApicId;
> UINT32 Health;
> + UINT32 SevEsApEnterHltLoopAfterWakeup;
SevEsEnteredHlt should be enough, right?
> UINT64 StartupSignalAddress;
> UINT64 StartupProcedureAddress;
> } PROCESSOR_HAND_OFF;
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 1951922912..59f0d87f9d 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -660,13 +660,14 @@ InitializeApData (
> **/
> VOID
> PlaceAPInHltLoop (
> - IN CPU_MP_DATA *CpuMpData
> + IN CPU_MP_DATA *CpuMpData,
> + IN UINTN ProcessorNumber
> )
> {
> while (TRUE) {
> DisableInterrupts ();
> if (CpuMpData->UseSevEsAPMethod) {
> - SevEsPlaceApHlt (CpuMpData);
> + SevEsPlaceApHlt (CpuMpData, ProcessorNumber);
> } else {
> CpuSleep ();
> }
> @@ -762,6 +763,9 @@ ApWakeupFunction (
> while (TRUE) {
> if (CpuMpData->InitFlag == ApInitConfig) {
> ProcessorNumber = ApIndex;
> + if (CpuMpData->UseSevEsAPMethod) {
> + CpuMpData->CpuData[ProcessorNumber].SevEsApEnterHltLoopAfterWakeup = 0;
> + }
> //
> // This is first time AP wakeup, get BIST information from AP stack
> //
> @@ -782,6 +786,9 @@ ApWakeupFunction (
> // Execute AP function if AP is ready
> //
> GetProcessorNumber (CpuMpData, &ProcessorNumber);
> + if (CpuMpData->UseSevEsAPMethod) {
> + CpuMpData->CpuData[ProcessorNumber].SevEsApEnterHltLoopAfterWakeup = 0;
> + }
> //
> // Clear AP start-up signal when AP waken up
> //
> @@ -903,7 +910,7 @@ ApWakeupFunction (
> // Place AP is specified loop mode
> //
> if (CpuMpData->ApLoopMode == ApInHltLoop) {
> - PlaceAPInHltLoop (CpuMpData);
> + PlaceAPInHltLoop (CpuMpData, ProcessorNumber);
> //
> // Never run here
> //
> @@ -993,6 +1000,20 @@ GetApResetVectorSize (
> *SizeAbove1Mb = AddressMap->RendezvousFunnelSize - AddressMap->ModeTransitionOffset;
> }
> }
> +/**
> + Wait for SEV-ES AP enter in HLT-LOOP.^M
> +
> + @param[in] SevEsApInHltLoop Pointer to SevEsApInHltLoop^M
> +**/
> +VOID
> +WaitSevEsApEnterHltLoopAfterWakeup (
> + IN volatile UINT32 *SevEsApEnterHltLoop
> + )
> +{
> + while (*(UINT32*)SevEsApEnterHltLoop == 0) {
> + CpuPause ();
> + }
> +}
>
> /**
> This function will fill the exchange info structure.
> @@ -1324,6 +1345,17 @@ WakeUpAP (
> //
> SendStartupIpiAllExcludingSelf ((UINT32)ExchangeInfo->BufferStart);
> } else {
> + //
> + // Subsequent INIT-SIPI-SIPI
> + //
> + if (CpuMpData->SevEsIsEnabled && (CpuMpData->InitFlag != ApInitConfig)) {
> + for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
> + CpuData = &CpuMpData->CpuData[Index];
> + if (Index != CpuMpData->BspNumber) {
> + WaitSevEsApEnterHltLoopAfterWakeup(&CpuData->SevEsApEnterHltLoopAfterWakeup);
> + }
> + }
> + }
> SendInitSipiSipiAllExcludingSelf ((UINT32)ExchangeInfo->BufferStart);
> }
> }
> @@ -2270,6 +2302,7 @@ MpInitLibInitialize (
> InitializeSpinLock (&CpuMpData->CpuData[Index].ApLock);
> CpuMpData->CpuData[Index].CpuHealthy = (MpHandOff->Info[HobIndex].Health == 0) ? TRUE : FALSE;
> CpuMpData->CpuData[Index].ApFunction = 0;
> + CpuMpData->CpuData[Index].SevEsApEnterHltLoopAfterWakeup = MpHandOff->Info[HobIndex].SevEsApEnterHltLoopAfterWakeup;
> CpuInfoInHob[Index].InitialApicId = MpHandOff->Info[HobIndex].ApicId;
> CpuInfoInHob[Index].ApTopOfStack = CpuMpData->Buffer + (Index + 1) * CpuMpData->CpuApStackSize;
> CpuInfoInHob[Index].ApicId = MpHandOff->Info[HobIndex].ApicId;
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index 88b31fecca..c33a7bf658 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -146,6 +146,7 @@ typedef enum {
> typedef struct {
> SPIN_LOCK ApLock;
> volatile UINT32 *StartupApSignal;
> + volatile UINT32 SevEsApEnterHltLoopAfterWakeup;
Similar to above, SevEsEnteredHlt.
Thanks,
Tom
> volatile UINTN ApFunction;
> volatile UINTN ApFunctionArgument;
> BOOLEAN CpuHealthy;
> @@ -862,7 +863,8 @@ SetSevEsJumpTable (
> **/
> VOID
> SevEsPlaceApHlt (
> - CPU_MP_DATA *CpuMpData
> + CPU_MP_DATA *CpuMpData,
> + UINT32 ProcessorNumber
> );
>
> /**
> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> index 16a858d542..b393ce00be 100644
> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> @@ -164,6 +164,8 @@ SaveCpuMpData (
> if (CpuMpData->ApLoopMode != ApInHltLoop) {
> MpHandOff->Info[Index-HobBase].StartupSignalAddress = (UINT64)(UINTN)CpuMpData->CpuData[Index].StartupApSignal;
> MpHandOff->Info[Index-HobBase].StartupProcedureAddress = (UINT64)(UINTN)&CpuMpData->CpuData[Index].ApFunction;
> + } else {
> + MpHandOff->Info[Index-HobBase].SevEsApEnterHltLoopAfterWakeup = CpuMpData->CpuData[Index].SevEsApEnterHltLoopAfterWakeup;
> }
> }
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120179): https://edk2.groups.io/g/devel/message/120179
Mute This Topic: https://groups.io/mt/107630732/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-08-01 14:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-26 15:58 [edk2-devel] [PATCH v1] UefiCpuPkg/Library/MpInitLib: fix SEV-ES AP bootinng failure Wencheng Yang
-- strict thread matches above, loose matches on Subject: below --
2024-07-31 10:22 Wencheng Yang
2024-08-01 14:55 ` Lendacky, Thomas via groups.io
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox