From 5e2e5647b9fba569b7ba5ede0a77d06ae3c16504 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 1 Mar 2018 22:05:55 +0100 Subject: OvmfPkg/AmdSevDxe: decrypt the pages of the initial SMRAM save state map Based on the following patch from Brijesh Singh : [PATCH v2 1/2] OvmfPkg/AmdSevDxe: Clear the C-bit from SMM Saved State http://mid.mail-archive.com/20180228161415.28723-2-brijesh.singh@amd.com https://lists.01.org/pipermail/edk2-devel/2018-February/022016.html Original commit message from Brijesh: > When OVMF is built with SMM, SMMSaved State area (SMM_DEFAULT_SMBASE + > SMRAM_SAVE_STATE_MAP_OFFSET) contains data which need to be accessed by > both guest and hypervisor. Since the data need to be accessed by both > hence we must map the SMMSaved State area as unencrypted (i.e C-bit > cleared). > > This patch clears the SavedStateArea address before SMBASE relocation. > Currently, we do not clear the SavedStateArea address after SMBASE is > relocated due to the following reasons: > > 1) Guest BIOS never access the relocated SavedStateArea. > > 2) The C-bit works on page-aligned address, but the SavedStateArea > address is not a page-aligned. Theoretically, we could roundup the > address and clear the C-bit of aligned address but looking carefully we > found that some portion of the page contains code -- which will causes a > bigger issue for the SEV guest. When SEV is enabled, all the code must > be encrypted otherwise hardware will cause trap. Changes by Laszlo: - separate AmdSevDxe bits from SmmCpuFeaturesLib bits; - spell out PcdLib dependency with #include and in LibraryClasses; - replace (SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET) calculation with call to new MemEncryptSevLocateInitialSmramSaveStateMapPages() function; - consequently, pass page-aligned BaseAddress to MemEncryptSevClearPageEncMask(); - zero the pages before clearing the C-bit; - pass Flush=TRUE to MemEncryptSevClearPageEncMask(); - harden the treatment of MemEncryptSevClearPageEncMask() failure. Cc: Ard Biesheuvel Cc: Brijesh Singh Cc: Jordan Justen Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek Tested-by: Brijesh Singh Reviewed-by: Brijesh Singh --- OvmfPkg/AmdSevDxe/AmdSevDxe.c | 53 +++++++++++++++++++++++++++++++++++++++++ OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 6 +++++ 2 files changed, 59 insertions(+) (limited to 'OvmfPkg/AmdSevDxe') diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c index 8f02d0627e..c697580ad5 100644 --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c @@ -16,10 +16,13 @@ **/ +#include +#include #include #include #include #include +#include EFI_STATUS EFIAPI @@ -68,5 +71,55 @@ AmdSevDxeEntryPoint ( FreePool (AllDescMap); } + // + // When SMM is enabled, clear the C-bit from SMM Saved State Area + // + // NOTES: The SavedStateArea address cleared here is before SMBASE + // relocation. Currently, we do not clear the SavedStateArea address after + // SMBASE is relocated due to the following reasons: + // + // 1) Guest BIOS never access the relocated SavedStateArea. + // + // 2) The C-bit works on page-aligned address, but the SavedStateArea + // address is not a page-aligned. Theoretically, we could roundup the address + // and clear the C-bit of aligned address but looking carefully we found + // that some portion of the page contains code -- which will causes a bigger + // issues for SEV guest. When SEV is enabled, all the code must be encrypted + // otherwise hardware will cause trap. + // + // We restore the C-bit for this SMM Saved State Area after SMBASE relocation + // is completed (See OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c). + // + if (FeaturePcdGet (PcdSmmSmramRequire)) { + UINTN MapPagesBase; + UINTN MapPagesCount; + + Status = MemEncryptSevLocateInitialSmramSaveStateMapPages ( + &MapPagesBase, + &MapPagesCount + ); + ASSERT_EFI_ERROR (Status); + + // + // Although these pages were set aside (i.e., allocated) by PlatformPei, we + // could be after a warm reboot from the OS. Don't leak any stale OS data + // to the hypervisor. + // + ZeroMem ((VOID *)MapPagesBase, EFI_PAGES_TO_SIZE (MapPagesCount)); + + Status = MemEncryptSevClearPageEncMask ( + 0, // Cr3BaseAddress -- use current CR3 + MapPagesBase, // BaseAddress + MapPagesCount, // NumPages + TRUE // Flush + ); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "%a: MemEncryptSevClearPageEncMask(): %r\n", + __FUNCTION__, Status)); + ASSERT (FALSE); + CpuDeadLoop (); + } + } + return EFI_SUCCESS; } diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf index 3aff7e2920..b7e7da002d 100644 --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf @@ -32,11 +32,17 @@ OvmfPkg/OvmfPkg.dec [LibraryClasses] + BaseLib + BaseMemoryLib DebugLib DxeServicesTableLib MemEncryptSevLib MemoryAllocationLib + PcdLib UefiDriverEntryPoint [Depex] TRUE + +[FeaturePcd] + gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire -- cgit v1.2.3