Tom: This patch fixes the regression issue. So, I am OK to merge it for this stable tag.
Thanks Liming > -----邮件原件----- > 发件人: Tom Lendacky <thomas.lenda...@amd.com> > 发送时间: 2022年5月20日 6:02 > 收件人: Ard Biesheuvel <a...@kernel.org>; Liming Gao > <gaolim...@byosoft.com.cn> > 抄送: edk2-devel-groups-io <devel@edk2.groups.io>; Ard Biesheuvel > <ardb+tianoc...@kernel.org>; Jiewen Yao <jiewen....@intel.com>; Jordan > Justen <jordan.l.jus...@intel.com>; Gerd Hoffmann <kra...@redhat.com>; > Erdem Aktas <erdemak...@google.com>; James Bottomley > <j...@linux.ibm.com>; Michael Roth <michael.r...@amd.com>; Min Xu > <min.m...@intel.com> > 主题: 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.lenda...@amd.com> > 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 <a...@kernel.org> > > > > 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+tianoc...@kernel.org> > >>> Cc: Jiewen Yao <jiewen....@intel.com> > >>> Cc: Jordan Justen <jordan.l.jus...@intel.com> > >>> Cc: Gerd Hoffmann <kra...@redhat.com> > >>> Cc: Erdem Aktas <erdemak...@google.com> > >>> Cc: James Bottomley <j...@linux.ibm.com> > >>> Cc: Michael Roth <michael.r...@amd.com> > >>> Cc: Min Xu <min.m...@intel.com> > >>> Signed-off-by: Tom Lendacky <thomas.lenda...@amd.com> > >>> --- > >>> 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" -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#89911): https://edk2.groups.io/g/devel/message/89911 Mute This Topic: https://groups.io/mt/91222320/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
