From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Thu, 26 Sep 2019 01:00:36 -0700 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id CF742308FFB1; Thu, 26 Sep 2019 08:00:35 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-49.rdu2.redhat.com [10.10.120.49]) by smtp.corp.redhat.com (Postfix) with ESMTP id 77E516012E; Thu, 26 Sep 2019 08:00:33 +0000 (UTC) Subject: Re: [edk2-devel] [RFC PATCH v2 08/44] OvmfPkg: Create GHCB pages for use during Pei and Dxe phase To: devel@edk2.groups.io, thomas.lendacky@amd.com Cc: Jordan Justen , Ard Biesheuvel , Michael D Kinney , Liming Gao , Eric Dong , Ray Ni , "Singh, Brijesh" References: <9799d415f652618c8a960cdb0040918185588652.1568922728.git.thomas.lendacky@amd.com> From: "Laszlo Ersek" Message-ID: Date: Thu, 26 Sep 2019 10:00:32 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <9799d415f652618c8a960cdb0040918185588652.1568922728.git.thomas.lendacky@amd.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.49]); Thu, 26 Sep 2019 08:00:35 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Hi Tom, On 09/19/19 21:52, Lendacky, Thomas wrote: > From: Tom Lendacky > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198 > > Allocate memory for the GHCB pages during SEV initialization for use > during Pei and Dxe phases. The GHCB page(s) must be shared pages, so > clear the encryption mask from the current page table entries. Upon > successful allocation, set the GHCB PCDs (PcdGhcbBase and PcdGhcbSize). skimming this patch and the next two ones for OvmfPkg (#10, #11), I'm a bit lost. I'm missing a parallel between the "early X64 page tables" and the GHCB-related pages. The former are set up (in X64 OVMF) in SEC, and are used throughout PEI until the DXE IPL builds new ones for the DXE phase. The latter also *seemed* to be set up in SEC, and I thought they'd be used throughout PEI -- I assumed the next place we'd need to massage GHCB pages would be similarly in the DXE IPL, or thereabouts. However, in this patch, we seem to allocate new pages for GHCB, and the commit message implies they are supposed to be used during PEI. That diverges from how long the "early X64 page tables" are used. I guess this difference could be justified, especially because we do MP stuff in PEI. (And we need separate GHCB stuff per VCPU -- in SEC we only consider the BSP.) But then, the question becomes: what exactly do we need the GHCB page allocated in SEC for? From the blurb, it seems that the GHCB allows the guest to selectively (actively) share information with the hypervisor -- such as (parts of?) the register file, which the hypervisor cannot directly access, for a SEV-ES guest. But, we never seem to place such information at PcdOvmfSecGhcbBase (aka GHCB_BASE) in SEC. We program the GHCB's base address, and then we clear the GHCB, but that seems to be it. Do we write anything non-zero to that block, ever? Thanks Laszlo > Cc: Jordan Justen > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Signed-off-by: Tom Lendacky > --- > OvmfPkg/OvmfPkgIa32.dsc | 2 ++ > OvmfPkg/OvmfPkgIa32X64.dsc | 2 ++ > OvmfPkg/OvmfPkgX64.dsc | 2 ++ > OvmfPkg/PlatformPei/PlatformPei.inf | 2 ++ > OvmfPkg/PlatformPei/AmdSev.c | 36 ++++++++++++++++++++++++++++- > 5 files changed, 43 insertions(+), 1 deletion(-) > > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc > index 0ce5c01722ef..4369cf6d55e5 100644 > --- a/OvmfPkg/OvmfPkgIa32.dsc > +++ b/OvmfPkg/OvmfPkgIa32.dsc > @@ -560,6 +560,8 @@ [PcdsDynamicDefault] > > # Set SEV-ES defaults > gEfiMdeModulePkgTokenSpaceGuid.PcdSevEsActive|0 > + gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase|0 > + gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbSize|0 > > !if $(SMM_REQUIRE) == TRUE > gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes|8 > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc > index e7455e35a55d..a74f5028068e 100644 > --- a/OvmfPkg/OvmfPkgIa32X64.dsc > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc > @@ -572,6 +572,8 @@ [PcdsDynamicDefault] > > # Set SEV-ES defaults > gEfiMdeModulePkgTokenSpaceGuid.PcdSevEsActive|0 > + gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase|0 > + gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbSize|0 > > !if $(SMM_REQUIRE) == TRUE > gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes|8 > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > index 0b8305cd10a2..fd714d386e75 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -571,6 +571,8 @@ [PcdsDynamicDefault] > > # Set SEV-ES defaults > gEfiMdeModulePkgTokenSpaceGuid.PcdSevEsActive|0 > + gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase|0 > + gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbSize|0 > > !if $(SMM_REQUIRE) == TRUE > gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes|8 > diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf > index a9e424a6012a..62abc99f4622 100644 > --- a/OvmfPkg/PlatformPei/PlatformPei.inf > +++ b/OvmfPkg/PlatformPei/PlatformPei.inf > @@ -105,6 +105,8 @@ [Pcd] > gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds > gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize > gEfiMdeModulePkgTokenSpaceGuid.PcdSevEsActive > + gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase > + gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbSize > > [FixedPcd] > gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress > diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c > index 7ae2f26a2ba7..30c0e4af7252 100644 > --- a/OvmfPkg/PlatformPei/AmdSev.c > +++ b/OvmfPkg/PlatformPei/AmdSev.c > @@ -16,6 +16,9 @@ > #include > #include > #include > +#include > +#include > +#include > > #include "Platform.h" > > @@ -30,7 +33,10 @@ AmdSevEsInitialize ( > VOID > ) > { > - RETURN_STATUS PcdStatus; > + VOID *GhcbBase; > + PHYSICAL_ADDRESS GhcbBasePa; > + UINTN GhcbPageCount; > + RETURN_STATUS PcdStatus, DecryptStatus; > > if (!MemEncryptSevEsIsEnabled ()) { > return; > @@ -38,6 +44,34 @@ AmdSevEsInitialize ( > > PcdStatus = PcdSetBoolS (PcdSevEsActive, 1); > ASSERT_RETURN_ERROR (PcdStatus); > + > + // > + // Allocate GHCB pages. > + // > + GhcbPageCount = mMaxCpuCount; > + GhcbBase = AllocatePages (GhcbPageCount); > + ASSERT (GhcbBase); > + > + GhcbBasePa = (PHYSICAL_ADDRESS)(UINTN) GhcbBase; > + > + DecryptStatus = MemEncryptSevClearPageEncMask ( > + 0, > + GhcbBasePa, > + GhcbPageCount, > + TRUE > + ); > + ASSERT_RETURN_ERROR (DecryptStatus); > + > + SetMem (GhcbBase, GhcbPageCount * SIZE_4KB, 0); > + > + PcdStatus = PcdSet64S (PcdGhcbBase, (UINT64)GhcbBasePa); > + ASSERT_RETURN_ERROR (PcdStatus); > + PcdStatus = PcdSet64S (PcdGhcbSize, (UINT64)EFI_PAGES_TO_SIZE (GhcbPageCount)); > + ASSERT_RETURN_ERROR (PcdStatus); > + > + DEBUG ((DEBUG_INFO, "SEV-ES is enabled, %u GHCB pages allocated starting at 0x%lx\n", GhcbPageCount, GhcbBase)); > + > + AsmWriteMsr64 (MSR_SEV_ES_GHCB, (UINT64)GhcbBasePa); > } > > /** >