Hi Tom,
On 09/19/19 21:52, Lendacky, Thomas wrote:
> From: Tom Lendacky <thomas.lenda...@amd.com>
>
> 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 <jordan.l.jus...@intel.com>
> Cc: Laszlo Ersek <ler...@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
> Signed-off-by: Tom Lendacky <thomas.lenda...@amd.com>
> ---
> 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 <PiPei.h>
> #include <Register/Amd/Cpuid.h>
> #include <Register/Cpuid.h>
> +#include <Register/Amd/Msr.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/MemoryAllocationLib.h>
>
> #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);
> }
>
> /**
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#48080): https://edk2.groups.io/g/devel/message/48080
Mute This Topic: https://groups.io/mt/34203543/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
