From: "Ard Biesheuvel" <ardb@kernel.org>
To: Tom Lendacky <thomas.lendacky@amd.com>
Cc: 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>
Subject: Re: [edk2-devel] [PATCH] OvmfPkg: Make an Ia32/X64 hybrid build work with SEV
Date: Tue, 17 May 2022 18:29:42 +0200 [thread overview]
Message-ID: <CAMj1kXESwrhiHvejt+jt9AYQ8Us_3yfnrR_-V4RkYOecthaOag@mail.gmail.com> (raw)
In-Reply-To: <cb419378-b8f4-a3f6-e9fd-607d675a2d86@amd.com>
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"
next prev parent reply other threads:[~2022-05-17 16:29 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
[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 [this message]
2022-05-19 22:02 ` Lendacky, Thomas
2022-05-20 1:43 ` 回复: " gaoliming
2022-05-20 6:36 ` Ard Biesheuvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAMj1kXESwrhiHvejt+jt9AYQ8Us_3yfnrR_-V4RkYOecthaOag@mail.gmail.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox