OvmfPkg/AmdSevDxe: decrypt the pages of the initial SMRAM save state map

Based on the following patch from Brijesh Singh <brijesh.singh@amd.com>:

  [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 <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>

2 files changed, 59 insertions, 0 deletions

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 <Library/BaseLib.h>

+#include <Library/BaseMemoryLib.h>

 #include <Library/DebugLib.h>

 #include <Library/DxeServicesTableLib.h>

 #include <Library/MemEncryptSevLib.h>

 #include <Library/MemoryAllocationLib.h>

+#include <Library/PcdLib.h>

 

 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