-----邮件原件-----
发件人: 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"