From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web10.9538.1652804996362733890 for ; Tue, 17 May 2022 09:29:56 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=jDodFaBA; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 9D135612C8 for ; Tue, 17 May 2022 16:29:55 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 78634C3411B for ; Tue, 17 May 2022 16:29:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1652804994; bh=BULfTBLjoGMgsNtH9FOVhd98P6fbSh/p5hjmYIGaKhU=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=jDodFaBAyPWD19/8zW60mV+BLb1aG4zrH6kFH5IqpMA5raZvZpyDtscHdR/oiYA8V xNe6CHJzf3aI4gy8W6Amj6hI3dMMYehNVsMCp0sWWc47rRjnZlA2cUleMXUY9VNcIn WQR+SQdndywEg8a1akgxVC3qnMQTPt9RHC5JwHmNZswfjDXHpRLKpbnamMlAHFi761 3I81YSfA0R/6g7Ons09QMw1hUOQLtTfUvjirrpdOzlgESE+HQwE9XoXFukDXQpo+pU WWYoP4zNUgxmnCMiu1HmnDKI7MFdc/cn3PfAuUTKlYQGn9PZnpZH9zuyGg30abN58A 92m6Hf5ib0D6Q== Received: by mail-ot1-f44.google.com with SMTP id i25-20020a9d6259000000b00605df9afea7so12523337otk.1 for ; Tue, 17 May 2022 09:29:54 -0700 (PDT) X-Gm-Message-State: AOAM531X2JeoGuwv8LVh1BTnKZ0FzZyoPfBQKNrJtgthhQOJEpR9AgJk cdKA8b9E3/F02KTGxvrvX0w7Jt8bzFR/JDGzSNE= X-Google-Smtp-Source: ABdhPJySdzB/j8EQz6j3BSTjLMI9x5qzNczGqg5Bzm8Kh+uH9/I/puUKTH/OtbzvJsqpF/I/+9fbq3qoEihG6hFU8Ck= X-Received: by 2002:a9d:84f:0:b0:605:e229:3c82 with SMTP id 73-20020a9d084f000000b00605e2293c82mr8667328oty.71.1652804993467; Tue, 17 May 2022 09:29:53 -0700 (PDT) MIME-Version: 1.0 References: <16EFAF988BEBA4A6.18257@groups.io> In-Reply-To: From: "Ard Biesheuvel" Date: Tue, 17 May 2022 18:29:42 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH] OvmfPkg: Make an Ia32/X64 hybrid build work with SEV To: Tom Lendacky Cc: edk2-devel-groups-io , Ard Biesheuvel , Jiewen Yao , Jordan Justen , Gerd Hoffmann , Erdem Aktas , James Bottomley , Michael Roth , Min Xu Content-Type: text/plain; charset="UTF-8" On Tue, 17 May 2022 at 18:26, Tom Lendacky wrote: > > On 5/16/22 15:24, Lendacky, Thomas via groups.io wrote: > > The BaseMemEncryptSevLib functionality was updated to rely on the use of > > the OVMF/SEV workarea to check for SEV guests. However, this area is only > > updated when running the X64 OVMF build, not the hybrid Ia32/X64 build. > > Base SEV support is allowed under the Ia32/X64 build, but it now fails > > to boot as a result of the change. > > > > Update the ResetVector code to check for SEV features when built for > > 32-bit mode, not just 64-bit mode (requiring updates to both the Ia32 > > and Ia32X64 fdf files). > > So this is a regression and it would be great if it could be applied to > the 202205 release. Can folks take a look and make sure it looks safe to > them for applying during hard feature freeze? > > If it's ok to be applied now, is there a particular process for applying > this during hard freeze? > For the change itself: Acked-by: Ard Biesheuvel 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 > > Cc: Jiewen Yao > > Cc: Jordan Justen > > Cc: Gerd Hoffmann > > Cc: Erdem Aktas > > Cc: James Bottomley > > Cc: Michael Roth > > Cc: Min Xu > > Signed-off-by: Tom Lendacky > > --- > > OvmfPkg/OvmfPkgIa32.fdf | 11 +++ > > OvmfPkg/OvmfPkgIa32X64.fdf | 8 +++ > > OvmfPkg/OvmfPkgX64.fdf | 3 +- > > OvmfPkg/ResetVector/Ia32/AmdSev.asm | 4 ++ > > OvmfPkg/ResetVector/Main.asm | 6 ++ > > OvmfPkg/ResetVector/ResetVector.nasmb | 72 ++++++++++---------- > > 6 files changed, 67 insertions(+), 37 deletions(-) > > > > diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf > > index 3ab1755749d4..57d13b7130bc 100644 > > --- a/OvmfPkg/OvmfPkgIa32.fdf > > +++ b/OvmfPkg/OvmfPkgIa32.fdf > > @@ -76,6 +76,9 @@ [FD.MEMFD] > > 0x007000|0x001000 > > gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|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 > > @@ -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"