On Fri, 20 May 2022 at 03:44, gaoliming <gaoliming@...> wrote: Tom: This patch fixes the regression issue. So, I am OK to merge it for this stable tag.
Merged as #2910 Thanks all, -----邮件原件----- 发件人: Tom Lendacky <thomas.lendacky@...> 发送时间: 2022年5月20日 6:02 收件人: Ard Biesheuvel <ardb@...>; Liming Gao <gaoliming@...> 抄送: edk2-devel-groups-io <devel@edk2.groups.io>; Ard Biesheuvel <ardb+tianocore@...>; Jiewen Yao <jiewen.yao@...>; Jordan Justen <jordan.l.justen@...>; Gerd Hoffmann <kraxel@...>; Erdem Aktas <erdemaktas@...>; James Bottomley <jejb@...>; Michael Roth <michael.roth@...>; Min Xu <min.m.xu@...> 主题: Re: [edk2-devel] [PATCH] OvmfPkg: Make an Ia32/X64 hybrid build work with SEV
Explicitly adding Liming to the To: line for visibility.
Thanks, Tom
On 5/17/22 11:29, Ard Biesheuvel wrote:
On Tue, 17 May 2022 at 18:26, Tom Lendacky <thomas.lendacky@...> wrote:
On 5/16/22 15:24, Lendacky, Thomas via groups.io wrote:
The BaseMemEncryptSevLib functionality was updated to rely on the use
of
the OVMF/SEV workarea to check for SEV guests. However, this area is
only
updated when running the X64 OVMF build, not the hybrid Ia32/X64
build.
Base SEV support is allowed under the Ia32/X64 build, but it now fails to boot as a result of the change.
Update the ResetVector code to check for SEV features when built for 32-bit mode, not just 64-bit mode (requiring updates to both the Ia32 and Ia32X64 fdf files). So this is a regression and it would be great if it could be applied to the 202205 release. Can folks take a look and make sure it looks safe to them for applying during hard feature freeze?
If it's ok to be applied now, is there a particular process for applying this during hard freeze?
For the change itself:
Acked-by: Ard Biesheuvel <ardb@...>
and I am fine with taking this during hard freeze, but I'll defer to Liming to make the final call.
Fixes: f1d1c337e7c0575da7fd248b2dd9cffc755940df Cc: Ard Biesheuvel <ardb+tianocore@...> Cc: Jiewen Yao <jiewen.yao@...> Cc: Jordan Justen <jordan.l.justen@...> Cc: Gerd Hoffmann <kraxel@...> Cc: Erdem Aktas <erdemaktas@...> Cc: James Bottomley <jejb@...> Cc: Michael Roth <michael.roth@...> Cc: Min Xu <min.m.xu@...> Signed-off-by: Tom Lendacky <thomas.lendacky@...> --- OvmfPkg/OvmfPkgIa32.fdf | 11 +++ OvmfPkg/OvmfPkgIa32X64.fdf | 8 +++ OvmfPkg/OvmfPkgX64.fdf | 3 +- OvmfPkg/ResetVector/Ia32/AmdSev.asm | 4 ++ OvmfPkg/ResetVector/Main.asm | 6 ++ OvmfPkg/ResetVector/ResetVector.nasmb | 72 ++++++++++---------- 6 files changed, 67 insertions(+), 37 deletions(-)
diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf index 3ab1755749d4..57d13b7130bc 100644 --- a/OvmfPkg/OvmfPkgIa32.fdf +++ b/OvmfPkg/OvmfPkgIa32.fdf @@ -76,6 +76,9 @@ [FD.MEMFD] 0x007000|0x001000
gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOv mfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
+0x008000|0x001000
+gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase|gUefiOvmfPkgToken SpaceGuid.PcdOvmfWorkAreaSize
+ 0x010000|0x010000
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgT okenSpaceGuid.PcdOvmfSecPeiTempRamSize
@@ -87,6 +90,14 @@ [FD.MEMFD]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgToken SpaceGuid.PcdOvmfDxeMemFvSize
FV = DXEFV
+############################################################# #############################
+# Set the SEV-ES specific work area PCDs (used for all forms of SEV since
the
+# the SEV STATUS MSR is now saved in the work area) +# +SET gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase =
$(MEMFD_BASE_ADDRESS) + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHea der
+SET gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize =
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize - gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHea der
+############################################################# #############################
+
############################################################## ##################
[FV.SECFV] diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf
b/OvmfPkg/OvmfPkgIa32X64.fdf
index e1638fa6ea38..ccde366887a9 100644 --- a/OvmfPkg/OvmfPkgIa32X64.fdf +++ b/OvmfPkg/OvmfPkgIa32X64.fdf @@ -90,6 +90,14 @@ [FD.MEMFD]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgToken SpaceGuid.PcdOvmfDxeMemFvSize
FV = DXEFV
+############################################################# #############################
+# Set the SEV-ES specific work area PCDs (used for all forms of SEV since
the
+# the SEV STATUS MSR is now saved in the work area) +# +SET gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase =
$(MEMFD_BASE_ADDRESS) + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHea der
+SET gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize =
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize - gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHea der
+############################################################# #############################
+
############################################################## ##################
[FV.SECFV] diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf index aa9a83032d9b..438806fba8f1 100644 --- a/OvmfPkg/OvmfPkgX64.fdf +++ b/OvmfPkg/OvmfPkgX64.fdf @@ -106,7 +106,8 @@ [FD.MEMFD] FV = DXEFV
############################################################## ############################
-# Set the SEV-ES specific work area PCDs +# Set the SEV-ES specific work area PCDs (used for all forms of SEV since
the
+# the SEV STATUS MSR is now saved in the work area) # SET gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase =
$(MEMFD_BASE_ADDRESS) + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHea der
SET gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize =
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize - gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHea der
diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
index 864d68385342..9350b0406833 100644 --- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm +++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm @@ -150,6 +150,8 @@ BITS 32 SevEsUnexpectedRespTerminate: TerminateVmgExit TERM_UNEXPECTED_RESP_CODE
+%ifdef ARCH_X64 + ; If SEV-ES is enabled then initialize and make the GHCB page shared SevClearPageEncMaskForGhcbPage: ; Check if SEV is enabled @@ -209,6 +211,8 @@ GetSevCBitMaskAbove31: GetSevCBitMaskAbove31Exit: OneTimeCallRet GetSevCBitMaskAbove31
+%endif + ; Check if Secure Encrypted Virtualization (SEV) features are enabled. ; ; Register usage is tight in this routine, so multiple calls for the diff --git a/OvmfPkg/ResetVector/Main.asm
b/OvmfPkg/ResetVector/Main.asm
index 5cfc0b5c72b1..46cfa87c4c0a 100644 --- a/OvmfPkg/ResetVector/Main.asm +++ b/OvmfPkg/ResetVector/Main.asm @@ -75,6 +75,12 @@ SearchBfv:
%ifdef ARCH_IA32
+ ; + ; SEV support can be built and run using the Ia32/X64 split
environment.
+ ; Set the OVMF/SEV work area as appropriate. + ; + OneTimeCall CheckSevFeatures + ; ; Restore initial EAX value into the EAX register ; diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb
b/OvmfPkg/ResetVector/ResetVector.nasmb
index 9421f4818907..94fbb0a87b37 100644 --- a/OvmfPkg/ResetVector/ResetVector.nasmb +++ b/OvmfPkg/ResetVector/ResetVector.nasmb @@ -47,7 +47,36 @@ %include "Ia32/SearchForBfvBase.asm" %include "Ia32/SearchForSecEntry.asm"
-%define WORK_AREA_GUEST_TYPE (FixedPcdGet32
(PcdOvmfWorkAreaBase))
+%define WORK_AREA_GUEST_TYPE (FixedPcdGet32
(PcdOvmfWorkAreaBase))
+%define PT_ADDR(Offset) (FixedPcdGet32
(PcdOvmfSecPageTablesBase) + (Offset))
+ +%define GHCB_PT_ADDR (FixedPcdGet32
(PcdOvmfSecGhcbPageTableBase))
+%define GHCB_BASE (FixedPcdGet32
(PcdOvmfSecGhcbBase))
+%define GHCB_SIZE (FixedPcdGet32
(PcdOvmfSecGhcbSize))
+%define SEV_ES_WORK_AREA (FixedPcdGet32
(PcdSevEsWorkAreaBase))
+%define SEV_ES_WORK_AREA_SIZE 25 +%define SEV_ES_WORK_AREA_STATUS_MSR (FixedPcdGet32
(PcdSevEsWorkAreaBase))
+%define SEV_ES_WORK_AREA_RDRAND (FixedPcdGet32
(PcdSevEsWorkAreaBase) + 8)
+%define SEV_ES_WORK_AREA_ENC_MASK (FixedPcdGet32
(PcdSevEsWorkAreaBase) + 16)
+%define SEV_ES_WORK_AREA_RECEIVED_VC (FixedPcdGet32
(PcdSevEsWorkAreaBase) + 24)
+%define SEV_ES_VC_TOP_OF_STACK (FixedPcdGet32
(PcdOvmfSecPeiTempRamBase) + FixedPcdGet32 (PcdOvmfSecPeiTempRamSize))
+%define SEV_SNP_SECRETS_BASE (FixedPcdGet32
(PcdOvmfSnpSecretsBase))
+%define SEV_SNP_SECRETS_SIZE (FixedPcdGet32
(PcdOvmfSnpSecretsSize))
+%define CPUID_BASE (FixedPcdGet32
(PcdOvmfCpuidBase))
+%define CPUID_SIZE (FixedPcdGet32
(PcdOvmfCpuidSize))
+%define SNP_SEC_MEM_BASE_DESC_1 (FixedPcdGet32
(PcdOvmfSecPageTablesBase))
+%define SNP_SEC_MEM_SIZE_DESC_1 (FixedPcdGet32
(PcdOvmfSecGhcbBase) - SNP_SEC_MEM_BASE_DESC_1)
+; +; The PcdOvmfSecGhcbBase reserves two GHCB pages. The first page is
used
+; as GHCB shared page and second is used for bookkeeping to support
the
+; nested GHCB in SEC phase. The bookkeeping page is mapped private.
The VMM
+; does not need to validate the shared page but it need to validate the +; bookkeeping page. +; +%define SNP_SEC_MEM_BASE_DESC_2 (GHCB_BASE + 0x1000) +%define SNP_SEC_MEM_SIZE_DESC_2
(SEV_SNP_SECRETS_BASE - SNP_SEC_MEM_BASE_DESC_2)
+%define SNP_SEC_MEM_BASE_DESC_3 (CPUID_BASE +
CPUID_SIZE)
+%define SNP_SEC_MEM_SIZE_DESC_3 (FixedPcdGet32
(PcdOvmfPeiMemFvBase) - SNP_SEC_MEM_BASE_DESC_3)
%ifdef ARCH_X64 #include <AutoGen.h> @@ -94,43 +123,14 @@ %define TDX_WORK_AREA_PGTBL_READY (FixedPcdGet32
(PcdOvmfWorkAreaBase) + 4)
%define TDX_WORK_AREA_GPAW (FixedPcdGet32
(PcdOvmfWorkAreaBase) + 8)
- %define PT_ADDR(Offset) (FixedPcdGet32
(PcdOvmfSecPageTablesBase) + (Offset))
+ %include "X64/IntelTdxMetadata.asm" + %include "Ia32/Flat32ToFlat64.asm" + %include "Ia32/PageTables64.asm" + %include "Ia32/IntelTdx.asm" + %include "X64/OvmfSevMetadata.asm" +%endif
- %define GHCB_PT_ADDR (FixedPcdGet32
(PcdOvmfSecGhcbPageTableBase))
- %define GHCB_BASE (FixedPcdGet32 (PcdOvmfSecGhcbBase)) - %define GHCB_SIZE (FixedPcdGet32 (PcdOvmfSecGhcbSize)) - %define SEV_ES_WORK_AREA (FixedPcdGet32
(PcdSevEsWorkAreaBase))
- %define SEV_ES_WORK_AREA_SIZE 25 - %define SEV_ES_WORK_AREA_STATUS_MSR (FixedPcdGet32
(PcdSevEsWorkAreaBase))
- %define SEV_ES_WORK_AREA_RDRAND (FixedPcdGet32
(PcdSevEsWorkAreaBase) + 8)
- %define SEV_ES_WORK_AREA_ENC_MASK (FixedPcdGet32
(PcdSevEsWorkAreaBase) + 16)
- %define SEV_ES_WORK_AREA_RECEIVED_VC (FixedPcdGet32
(PcdSevEsWorkAreaBase) + 24)
- %define SEV_ES_VC_TOP_OF_STACK (FixedPcdGet32
(PcdOvmfSecPeiTempRamBase) + FixedPcdGet32 (PcdOvmfSecPeiTempRamSize))
- %define SEV_SNP_SECRETS_BASE (FixedPcdGet32
(PcdOvmfSnpSecretsBase))
- %define SEV_SNP_SECRETS_SIZE (FixedPcdGet32
(PcdOvmfSnpSecretsSize))
- %define CPUID_BASE (FixedPcdGet32 (PcdOvmfCpuidBase)) - %define CPUID_SIZE (FixedPcdGet32 (PcdOvmfCpuidSize)) - %define SNP_SEC_MEM_BASE_DESC_1 (FixedPcdGet32
(PcdOvmfSecPageTablesBase))
- %define SNP_SEC_MEM_SIZE_DESC_1 (FixedPcdGet32
(PcdOvmfSecGhcbBase) - SNP_SEC_MEM_BASE_DESC_1)
- ; - ; The PcdOvmfSecGhcbBase reserves two GHCB pages. The first page
is used
- ; as GHCB shared page and second is used for bookkeeping to support
the
- ; nested GHCB in SEC phase. The bookkeeping page is mapped private.
The VMM
- ; does not need to validate the shared page but it need to validate the - ; bookkeeping page. - ; - %define SNP_SEC_MEM_BASE_DESC_2 (GHCB_BASE + 0x1000) - %define SNP_SEC_MEM_SIZE_DESC_2 (SEV_SNP_SECRETS_BASE -
SNP_SEC_MEM_BASE_DESC_2)
- %define SNP_SEC_MEM_BASE_DESC_3 (CPUID_BASE + CPUID_SIZE) - %define SNP_SEC_MEM_SIZE_DESC_3 (FixedPcdGet32
(PcdOvmfPeiMemFvBase) - SNP_SEC_MEM_BASE_DESC_3)
- -%include "X64/IntelTdxMetadata.asm" -%include "Ia32/Flat32ToFlat64.asm" %include "Ia32/AmdSev.asm" -%include "Ia32/PageTables64.asm" -%include "Ia32/IntelTdx.asm" -%include "X64/OvmfSevMetadata.asm" -%endif
%include "Ia16/Real16ToFlat32.asm" %include "Ia16/Init16.asm"
|