BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4654
Currently, the first time an AP is started for an SEV-SNP guest, it relies on the VMSA as set by the hypervisor. If the list of APIC IDs has been retrieved, this is not necessary. Instead, use the SEV-SNP AP Create protocol to start the AP for the first time and thereafter using the VMPL at which the BSP is running. Signed-off-by: Tom Lendacky <thomas.lenda...@amd.com> --- UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 1 + UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 3 +- UefiCpuPkg/Library/MpInitLib/MpLib.h | 13 ++++ UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c | 19 +++++ UefiCpuPkg/Library/MpInitLib/MpLib.c | 7 +- UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c | 79 +++++++++++++++++++- 6 files changed, 116 insertions(+), 6 deletions(-) diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf index 55e46d4a1fad..1ec50481f0d4 100644 --- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf @@ -82,6 +82,7 @@ [Pcd] gUefiCpuPkgTokenSpaceGuid.PcdGhcbHypervisorFeatures ## CONSUMES gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase ## SOMETIMES_CONSUMES gUefiCpuPkgTokenSpaceGuid.PcdFirstTimeWakeUpAPsBySipi ## CONSUMES + gUefiCpuPkgTokenSpaceGuid.PcdSevSnpApicIds ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase ## CONSUMES gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr ## CONSUMES diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf index bc3d716aa951..f0af07d3bdfb 100644 --- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf @@ -66,7 +66,8 @@ [Pcd] gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate ## SOMETIMES_CONSUMES gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase ## SOMETIMES_CONSUMES gUefiCpuPkgTokenSpaceGuid.PcdGhcbHypervisorFeatures ## CONSUMES - gUefiCpuPkgTokenSpaceGuid.PcdFirstTimeWakeUpAPsBySipi ## CONSUMES + gUefiCpuPkgTokenSpaceGuid.PcdFirstTimeWakeUpAPsBySipi ## CONSUMES + gUefiCpuPkgTokenSpaceGuid.PcdSevSnpApicIds ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase ## CONSUMES gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr ## CONSUMES diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h index 6e2137cb17cd..f1a5fa98d425 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h @@ -897,6 +897,19 @@ SevSnpCreateAP ( IN INTN ProcessorNumber ); +/** + Determine if the SEV-SNP AP Create protocol should be used. + + @param[in] CpuMpData Pointer to CPU MP Data + + @retval TRUE Use SEV-SNP AP Create protocol + @retval FALSE Do not use SEV-SNP AP Create protocol +**/ +BOOLEAN +SevSnpUseCreateAP ( + IN CPU_MP_DATA *CpuMpData + ); + /** Get pointer to CPU MP Data structure from GUIDed HOB. diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c b/UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c index a2b8a5b3f516..f9f24bee09de 100644 --- a/UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c @@ -48,3 +48,22 @@ SevSnpCreateAP ( // ASSERT (FALSE); } + +/** + Determine if the SEV-SNP AP Create protocol should be used. + + @param[in] CpuMpData Pointer to CPU MP Data + + @retval TRUE Use SEV-SNP AP Create protocol + @retval FALSE Do not use SEV-SNP AP Create protocol +**/ +BOOLEAN +SevSnpUseCreateAP ( + IN CPU_MP_DATA *CpuMpData + ) +{ + // + // SEV-SNP is not supported on 32-bit build. + // + return FALSE; +} diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c index cdfb570e61a0..5e017bcf9018 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c @@ -1302,9 +1302,10 @@ WakeUpAP ( // // Wakeup all APs // Must use the INIT-SIPI-SIPI method for initial configuration in - // order to obtain the APIC ID. + // order to obtain the APIC ID if not an SEV-SNP guest and the + // list of APIC IDs is not available. // - if (CpuMpData->SevSnpIsEnabled && (CpuMpData->InitFlag != ApInitConfig)) { + if (SevSnpUseCreateAP (CpuMpData)) { SevSnpCreateAP (CpuMpData, -1); } else { if ((CpuMpData->InitFlag == ApInitConfig) && FixedPcdGetBool (PcdFirstTimeWakeUpAPsBySipi)) { @@ -1414,7 +1415,7 @@ WakeUpAP ( SetSevEsJumpTable (ExchangeInfo->BufferStart); } - if (CpuMpData->SevSnpIsEnabled && (CpuMpData->InitFlag != ApInitConfig)) { + if (SevSnpUseCreateAP (CpuMpData)) { SevSnpCreateAP (CpuMpData, (INTN)ProcessorNumber); } else { SendInitSipiSipi ( diff --git a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c index db9a37fbbd19..6186a8d71521 100644 --- a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c +++ b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c @@ -263,17 +263,63 @@ SevSnpCreateAP ( CPU_INFO_IN_HOB *CpuInfoInHob; CPU_AP_DATA *CpuData; UINTN Index; + UINTN MaxIndex; UINT32 ApicId; + GHCB_APIC_IDS *GhcbApicIds; ASSERT (CpuMpData->MpCpuExchangeInfo->BufferStart < 0x100000); CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob; if (ProcessorNumber < 0) { - for (Index = 0; Index < CpuMpData->CpuCount; Index++) { + GhcbApicIds = (GHCB_APIC_IDS *)(UINTN)PcdGet64 (PcdSevSnpApicIds); + + if (CpuMpData->InitFlag == ApInitConfig) { + // + // APs have not been started, so CpuCount is not "known" yet. + // Use the retrieved APIC IDs to start the APs and fill out the + // MpLib CPU information properly. + // + ASSERT (GhcbApicIds != NULL); + if (GhcbApicIds == NULL) { + return; + } + + MaxIndex = MIN (GhcbApicIds->NumEntries, PcdGet32 (PcdCpuMaxLogicalProcessorNumber)); + } else { + // + // APs have been previously started. + // + MaxIndex = CpuMpData->CpuCount; + } + + for (Index = 0; Index < MaxIndex; Index++) { if (Index != CpuMpData->BspNumber) { CpuData = &CpuMpData->CpuData[Index]; - ApicId = CpuInfoInHob[Index].ApicId, + + if (CpuMpData->InitFlag == ApInitConfig) { + // + // CodeQL doesn't understand that a check for NULL was already done + // above, so check again. + // + if (GhcbApicIds == NULL) { + return; + } + + ApicId = GhcbApicIds->ApicIds[Index]; + + // + // For the first boot, use the BSP register information. + // + CopyMem ( + &CpuData->VolatileRegisters, + &CpuMpData->CpuData[0].VolatileRegisters, + sizeof (CpuData->VolatileRegisters) + ); + } else { + ApicId = CpuInfoInHob[Index].ApicId; + } + SevSnpCreateSaveArea (CpuMpData, CpuData, ApicId); } } @@ -284,3 +330,32 @@ SevSnpCreateAP ( SevSnpCreateSaveArea (CpuMpData, CpuData, ApicId); } } + +/** + Determine if the SEV-SNP AP Create protocol should be used. + + @param[in] CpuMpData Pointer to CPU MP Data + + @retval TRUE Use SEV-SNP AP Create protocol + @retval FALSE Do not use SEV-SNP AP Create protocol +**/ +BOOLEAN +SevSnpUseCreateAP ( + IN CPU_MP_DATA *CpuMpData + ) +{ + // + // The AP Create protocol is used for an SEV-SNP guest if + // - The initial configuration has been performed already or + // - PcdSevSnpApicIds is non-zero. + // + if (!CpuMpData->SevSnpIsEnabled) { + return FALSE; + } + + if ((CpuMpData->InitFlag == ApInitConfig) && (PcdGet64 (PcdSevSnpApicIds) == 0)) { + return FALSE; + } + + return TRUE; +} -- 2.42.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114639): https://edk2.groups.io/g/devel/message/114639 Mute This Topic: https://groups.io/mt/103986469/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-