* Re: [edk2-devel] [PATCH] OvmfPkg: Make an Ia32/X64 hybrid build work with SEV [not found] <16EFAF988BEBA4A6.18257@groups.io> @ 2022-05-17 16:26 ` Lendacky, Thomas 2022-05-17 16:29 ` Ard Biesheuvel 0 siblings, 1 reply; 5+ messages in thread From: Lendacky, Thomas @ 2022-05-17 16:26 UTC (permalink / raw) To: devel Cc: Ard Biesheuvel, Jiewen Yao, Jordan Justen, Gerd Hoffmann, Erdem Aktas, James Bottomley, Michael Roth, Min Xu 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? Thanks, Tom > > Fixes: f1d1c337e7c0575da7fd248b2dd9cffc755940df > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org> > Cc: Jiewen Yao <jiewen.yao@intel.com> > Cc: Jordan Justen <jordan.l.justen@intel.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Erdem Aktas <erdemaktas@google.com> > Cc: James Bottomley <jejb@linux.ibm.com> > Cc: Michael Roth <michael.roth@amd.com> > Cc: Min Xu <min.m.xu@intel.com> > Signed-off-by: Tom Lendacky <thomas.lendacky@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|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize > > +0x008000|0x001000 > +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize > + > 0x010000|0x010000 > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize > > @@ -87,6 +90,14 @@ [FD.MEMFD] > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTokenSpaceGuid.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.PcdOvmfConfidentialComputingWorkAreaHeader > +SET gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize - gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeader > +########################################################################################## > + > ################################################################################ > > [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|gUefiOvmfPkgTokenSpaceGuid.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.PcdOvmfConfidentialComputingWorkAreaHeader > +SET gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize - gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeader > +########################################################################################## > + > ################################################################################ > > [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.PcdOvmfConfidentialComputingWorkAreaHeader > SET gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize - gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeader > 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" ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH] OvmfPkg: Make an Ia32/X64 hybrid build work with SEV 2022-05-17 16:26 ` [edk2-devel] [PATCH] OvmfPkg: Make an Ia32/X64 hybrid build work with SEV Lendacky, Thomas @ 2022-05-17 16:29 ` Ard Biesheuvel 2022-05-19 22:02 ` Lendacky, Thomas 0 siblings, 1 reply; 5+ messages in thread From: Ard Biesheuvel @ 2022-05-17 16:29 UTC (permalink / raw) To: Tom Lendacky Cc: edk2-devel-groups-io, Ard Biesheuvel, Jiewen Yao, Jordan Justen, Gerd Hoffmann, Erdem Aktas, James Bottomley, Michael Roth, Min Xu On Tue, 17 May 2022 at 18:26, Tom Lendacky <thomas.lendacky@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 <ardb@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+tianocore@kernel.org> > > Cc: Jiewen Yao <jiewen.yao@intel.com> > > Cc: Jordan Justen <jordan.l.justen@intel.com> > > Cc: Gerd Hoffmann <kraxel@redhat.com> > > Cc: Erdem Aktas <erdemaktas@google.com> > > Cc: James Bottomley <jejb@linux.ibm.com> > > Cc: Michael Roth <michael.roth@amd.com> > > Cc: Min Xu <min.m.xu@intel.com> > > Signed-off-by: Tom Lendacky <thomas.lendacky@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|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize > > > > +0x008000|0x001000 > > +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize > > + > > 0x010000|0x010000 > > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize > > > > @@ -87,6 +90,14 @@ [FD.MEMFD] > > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTokenSpaceGuid.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.PcdOvmfConfidentialComputingWorkAreaHeader > > +SET gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize - gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeader > > +########################################################################################## > > + > > ################################################################################ > > > > [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|gUefiOvmfPkgTokenSpaceGuid.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.PcdOvmfConfidentialComputingWorkAreaHeader > > +SET gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize - gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeader > > +########################################################################################## > > + > > ################################################################################ > > > > [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.PcdOvmfConfidentialComputingWorkAreaHeader > > SET gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize - gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeader > > 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" ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH] OvmfPkg: Make an Ia32/X64 hybrid build work with SEV 2022-05-17 16:29 ` Ard Biesheuvel @ 2022-05-19 22:02 ` Lendacky, Thomas 2022-05-20 1:43 ` 回复: " gaoliming 0 siblings, 1 reply; 5+ messages in thread From: Lendacky, Thomas @ 2022-05-19 22:02 UTC (permalink / raw) To: Ard Biesheuvel, Liming Gao Cc: edk2-devel-groups-io, Ard Biesheuvel, Jiewen Yao, Jordan Justen, Gerd Hoffmann, Erdem Aktas, James Bottomley, Michael Roth, Min Xu 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@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 <ardb@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+tianocore@kernel.org> >>> Cc: Jiewen Yao <jiewen.yao@intel.com> >>> Cc: Jordan Justen <jordan.l.justen@intel.com> >>> Cc: Gerd Hoffmann <kraxel@redhat.com> >>> Cc: Erdem Aktas <erdemaktas@google.com> >>> Cc: James Bottomley <jejb@linux.ibm.com> >>> Cc: Michael Roth <michael.roth@amd.com> >>> Cc: Min Xu <min.m.xu@intel.com> >>> Signed-off-by: Tom Lendacky <thomas.lendacky@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|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize >>> >>> +0x008000|0x001000 >>> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize >>> + >>> 0x010000|0x010000 >>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize >>> >>> @@ -87,6 +90,14 @@ [FD.MEMFD] >>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTokenSpaceGuid.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.PcdOvmfConfidentialComputingWorkAreaHeader >>> +SET gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize - gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeader >>> +########################################################################################## >>> + >>> ################################################################################ >>> >>> [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|gUefiOvmfPkgTokenSpaceGuid.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.PcdOvmfConfidentialComputingWorkAreaHeader >>> +SET gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize - gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeader >>> +########################################################################################## >>> + >>> ################################################################################ >>> >>> [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.PcdOvmfConfidentialComputingWorkAreaHeader >>> SET gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize - gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeader >>> 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" ^ permalink raw reply [flat|nested] 5+ messages in thread
* 回复: [edk2-devel] [PATCH] OvmfPkg: Make an Ia32/X64 hybrid build work with SEV 2022-05-19 22:02 ` Lendacky, Thomas @ 2022-05-20 1:43 ` gaoliming 2022-05-20 6:36 ` Ard Biesheuvel 0 siblings, 1 reply; 5+ messages in thread From: gaoliming @ 2022-05-20 1:43 UTC (permalink / raw) To: 'Tom Lendacky', 'Ard Biesheuvel' Cc: 'edk2-devel-groups-io', 'Ard Biesheuvel', 'Jiewen Yao', 'Jordan Justen', 'Gerd Hoffmann', 'Erdem Aktas', 'James Bottomley', 'Michael Roth', 'Min Xu' Tom: This patch fixes the regression issue. So, I am OK to merge it for this stable tag. Thanks Liming > -----邮件原件----- > 发件人: Tom Lendacky <thomas.lendacky@amd.com> > 发送时间: 2022年5月20日 6:02 > 收件人: Ard Biesheuvel <ardb@kernel.org>; Liming Gao > <gaoliming@byosoft.com.cn> > 抄送: edk2-devel-groups-io <devel@edk2.groups.io>; Ard Biesheuvel > <ardb+tianocore@kernel.org>; Jiewen Yao <jiewen.yao@intel.com>; Jordan > Justen <jordan.l.justen@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; > Erdem Aktas <erdemaktas@google.com>; James Bottomley > <jejb@linux.ibm.com>; Michael Roth <michael.roth@amd.com>; Min Xu > <min.m.xu@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.lendacky@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 <ardb@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+tianocore@kernel.org> > >>> Cc: Jiewen Yao <jiewen.yao@intel.com> > >>> Cc: Jordan Justen <jordan.l.justen@intel.com> > >>> Cc: Gerd Hoffmann <kraxel@redhat.com> > >>> Cc: Erdem Aktas <erdemaktas@google.com> > >>> Cc: James Bottomley <jejb@linux.ibm.com> > >>> Cc: Michael Roth <michael.roth@amd.com> > >>> Cc: Min Xu <min.m.xu@intel.com> > >>> Signed-off-by: Tom Lendacky <thomas.lendacky@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" ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH] OvmfPkg: Make an Ia32/X64 hybrid build work with SEV 2022-05-20 1:43 ` 回复: " gaoliming @ 2022-05-20 6:36 ` Ard Biesheuvel 0 siblings, 0 replies; 5+ messages in thread From: Ard Biesheuvel @ 2022-05-20 6:36 UTC (permalink / raw) To: edk2-devel-groups-io, Liming Gao (Byosoft address) Cc: Tom Lendacky, Ard Biesheuvel, Jiewen Yao, Jordan Justen, Gerd Hoffmann, Erdem Aktas, James Bottomley, Michael Roth, Min Xu On Fri, 20 May 2022 at 03:44, gaoliming <gaoliming@byosoft.com.cn> 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@amd.com> > > 发送时间: 2022年5月20日 6:02 > > 收件人: Ard Biesheuvel <ardb@kernel.org>; Liming Gao > > <gaoliming@byosoft.com.cn> > > 抄送: edk2-devel-groups-io <devel@edk2.groups.io>; Ard Biesheuvel > > <ardb+tianocore@kernel.org>; Jiewen Yao <jiewen.yao@intel.com>; Jordan > > Justen <jordan.l.justen@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; > > Erdem Aktas <erdemaktas@google.com>; James Bottomley > > <jejb@linux.ibm.com>; Michael Roth <michael.roth@amd.com>; Min Xu > > <min.m.xu@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.lendacky@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 <ardb@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+tianocore@kernel.org> > > >>> Cc: Jiewen Yao <jiewen.yao@intel.com> > > >>> Cc: Jordan Justen <jordan.l.justen@intel.com> > > >>> Cc: Gerd Hoffmann <kraxel@redhat.com> > > >>> Cc: Erdem Aktas <erdemaktas@google.com> > > >>> Cc: James Bottomley <jejb@linux.ibm.com> > > >>> Cc: Michael Roth <michael.roth@amd.com> > > >>> Cc: Min Xu <min.m.xu@intel.com> > > >>> Signed-off-by: Tom Lendacky <thomas.lendacky@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" > > > > > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-20 6:36 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <16EFAF988BEBA4A6.18257@groups.io>
2022-05-17 16:26 ` [edk2-devel] [PATCH] OvmfPkg: Make an Ia32/X64 hybrid build work with SEV Lendacky, Thomas
2022-05-17 16:29 ` Ard Biesheuvel
2022-05-19 22:02 ` Lendacky, Thomas
2022-05-20 1:43 ` 回复: " gaoliming
2022-05-20 6:36 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox