[PATCH v2 1/2] OvmfPkg/AmdSevDxe: Clear the C-bit from SMM Saved State

[Brijesh Singh <brijesh.singh@amd.com>]

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.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 OvmfPkg/AmdSevDxe/AmdSevDxe.inf                         |  4 +++
 OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf |  1 +
 OvmfPkg/AmdSevDxe/AmdSevDxe.c                           | 35 ++++++++++++++++++++
 OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c   | 21 ++++++++++++
 4 files changed, 61 insertions(+)

diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
index 41635a57a454..162ed98a2fbe 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
@@ -29,6 +29,7 @@ [Packages]
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
   OvmfPkg/OvmfPkg.dec
+  UefiCpuPkg/UefiCpuPkg.dec
 
 [LibraryClasses]
   BaseLib
@@ -41,3 +42,6 @@ [LibraryClasses]
 
 [Depex]
   TRUE
+
+[FeaturePcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
index 31edf3a9c1fd..ba564abb787b 100644
--- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
+++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
@@ -36,3 +36,4 @@ [LibraryClasses]
   PcdLib
   DebugLib
   SmmServicesTableLib
+  MemEncryptSevLib
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index e472096320ea..5803e8655049 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -25,6 +25,8 @@
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/DxeServicesTableLib.h>
 #include <Library/MemEncryptSevLib.h>
+#include <Register/SmramSaveStateMap.h>
+#include <Register/QemuSmramSaveStateMap.h>
 
 EFI_STATUS
 EFIAPI
@@ -71,5 +73,38 @@ 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)) {
+    EFI_PHYSICAL_ADDRESS  SmmSavedStateAreaAddress;
+
+    SmmSavedStateAreaAddress = SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET;
+
+    Status = MemEncryptSevClearPageEncMask (
+               0,
+               SmmSavedStateAreaAddress,
+               EFI_SIZE_TO_PAGES (sizeof(QEMU_SMRAM_SAVE_STATE_MAP)),
+               FALSE
+               );
+    ASSERT_EFI_ERROR (Status);
+  }
+
   return EFI_SUCCESS;
 }
diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
index a307f64c9c61..946294701c62 100644
--- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
+++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
@@ -20,6 +20,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Library/MemoryAllocationLib.h>
 #include <Library/SmmServicesTableLib.h>
 #include <Library/DebugLib.h>
+#include <Library/MemEncryptSevLib.h>
 #include <Register/QemuSmramSaveStateMap.h>
 
 //
@@ -183,6 +184,26 @@ SmmCpuFeaturesSmmRelocationComplete (
   VOID
   )
 {
+  EFI_STATUS Status;
+  EFI_PHYSICAL_ADDRESS  SmmSavedStateAreaAddress;
+
+  //
+  // When SEV is enabled, the SMM SavedState is mapped with C=0
+  // (See OvmfPkg/AmdSevDxe/AmdSevDxe.c). Now the SMBASE is relocated hence we
+  // remap the address with C=1.
+  //
+  if (!MemEncryptSevIsEnabled ()) {
+    return;
+  }
+
+  SmmSavedStateAreaAddress = SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET;
+  Status = MemEncryptSevSetPageEncMask (
+             0,
+             SmmSavedStateAreaAddress,
+             EFI_SIZE_TO_PAGES (sizeof(QEMU_SMRAM_SAVE_STATE_MAP)),
+             FALSE
+             );
+  ASSERT_EFI_ERROR (Status);
 }
 
 /**
-- 
2.14.3