[PATCH v2 0/3] SEV Page Encryption Bitmap support for OVMF.

[PATCH v2 0/3] SEV Page Encryption Bitmap support for OVMF.

[ashish.kalra]

From: Ashish Kalra <ashish.kalra@amd.com>

By default all the SEV guest memory regions are considered encrypted,
if a guest changes the encryption attribute of the page (e.g mark a
page as decrypted) then notify hypervisor. Hypervisor will need to
track the unencrypted pages. The information will be used during
guest live migration, guest page migration and guest debugging.

The patch-set also adds a new SEV and SEV-ES hypercall abstraction
library to support SEV Page encryption/decryption status hypercalls
for SEV and SEV-ES guests.

BaseMemEncryptSevLib invokes hypercalls via this new hypercall library.

A branch containing these patches is available here:
https://github.com/ashkalra/edk2/tree/sev_page_encryption_bitmap_v2

Changes since v1:
 - Mark GHCB_BASE setup during reset-vector as decrypted explicitly in
   the hypervisor page encryption bitmap.
 - Resending the series with correct shallow threading.

Ashish Kalra (2):
  OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls.
  OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encryption bitmap.

Brijesh Singh (1):
  OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall

 .../Include/Library/MemEncryptHypercallLib.h  |  37 ++++++
 .../BaseMemEncryptSevLib.inf                  |   1 +
 .../BaseMemEncryptSevLib/X64/VirtualMemory.c  |  18 +++
 .../MemEncryptHypercallLib.c                  | 105 ++++++++++++++++++
 .../MemEncryptHypercallLib.inf                |  39 +++++++
 .../X64/AsmHelperStub.nasm                    |  39 +++++++
 OvmfPkg/OvmfPkgX64.dsc                        |   1 +
 OvmfPkg/PlatformPei/AmdSev.c                  |  10 ++
 8 files changed, 250 insertions(+)
 create mode 100644 OvmfPkg/Include/Library/MemEncryptHypercallLib.h
 create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.c
 create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
 create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm

-- 
2.17.1

[PATCH v2 1/3] OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls.

[Ashish Kalra]

From: Ashish Kalra <ashish.kalra@amd.com>

Add SEV and SEV-ES hypercall abstraction library to support SEV Page
encryption/deceryption status hypercalls for SEV and SEV-ES guests.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>

Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 OvmfPkg/Include/Library/MemEncryptHypercallLib.h                  |  37 +++++++
 OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.c   | 105 ++++++++++++++++++++
 OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf |  39 ++++++++
 OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm     |  39 ++++++++
 OvmfPkg/OvmfPkgX64.dsc                                            |   1 +
 5 files changed, 221 insertions(+)

diff --git a/OvmfPkg/Include/Library/MemEncryptHypercallLib.h b/OvmfPkg/Include/Library/MemEncryptHypercallLib.h
new file mode 100644
index 0000000000..cd46a7f2b3
--- /dev/null
+++ b/OvmfPkg/Include/Library/MemEncryptHypercallLib.h
@@ -0,0 +1,37 @@
+/** @file
+
+  Define Secure Encrypted Virtualization (SEV) hypercall library.
+
+  Copyright (c) 2020, AMD Incorporated. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _MEM_ENCRYPT_HYPERCALL_LIB_H_
+#define _MEM_ENCRYPT_HYPERCALL_LIB_H_
+
+#include <Base.h>
+
+#define SEV_PAGE_ENC_HYPERCALL  12
+
+/**
+ This hyercall is used to notify hypervisor when a page is marked as
+ 'decrypted' (i.e C-bit removed).
+
+ @param[in]   PhysicalAddress       The physical address that is the start address
+                                    of a memory region.
+ @param[in]   Length                The length of memory region
+ @param[in]   Mode                  SetCBit or ClearCBit
+
+**/
+
+VOID
+EFIAPI
+SetMemoryEncDecHypercall3 (
+  IN  UINTN     PhysicalAddress,
+  IN  UINTN     Length,
+  IN  UINTN     Mode
+  );
+
+#endif
diff --git a/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.c b/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.c
new file mode 100644
index 0000000000..f1136b7d36
--- /dev/null
+++ b/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.c
@@ -0,0 +1,105 @@
+/** @file
+
+  Secure Encrypted Virtualization (SEV) hypercall helper library
+
+  Copyright (c) 2020, AMD Incorporated. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Uefi.h>
+#include <Uefi/UefiBaseType.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/VmgExitLib.h>
+#include <Register/Amd/Ghcb.h>
+#include <Register/Amd/Msr.h>
+#include <Library/MemEncryptSevLib.h>
+#include <Library/MemEncryptHypercallLib.h>
+
+//
+// Interface exposed by the ASM implementation of the core hypercall
+//
+//
+
+VOID
+EFIAPI
+SetMemoryEncDecHypercall3AsmStub (
+  IN  UINTN  HypercallNum,
+  IN  UINTN  PhysicalAddress,
+  IN  UINTN  Length,
+  IN  UINTN  Mode
+  );
+
+/**
+ This function returns the current CPU privilege level, implemented
+ in ASM helper stub.
+
+**/
+
+UINT8
+EFIAPI
+GetCurrentCpuPrivilegeLevel (
+  VOID
+  );
+
+STATIC
+VOID
+GhcbSetRegValid (
+  IN OUT GHCB       *Ghcb,
+  IN GHCB_REGISTER  Reg
+  )
+{
+  UINT32  RegIndex;
+  UINT32  RegBit;
+
+  RegIndex = Reg / 8;
+  RegBit   = Reg & 0x07;
+
+  Ghcb->SaveArea.ValidBitmap[RegIndex] |= (1 << RegBit);
+}
+
+VOID
+EFIAPI
+SetMemoryEncDecHypercall3 (
+  IN PHYSICAL_ADDRESS PhysicalAddress,
+  IN UINTN            Pages,
+  IN UINTN            Mode
+  )
+{
+  if (MemEncryptSevEsIsEnabled ()) {
+    MSR_SEV_ES_GHCB_REGISTER  Msr;
+    GHCB                      *Ghcb;
+    BOOLEAN                   InterruptState;
+    UINT64                    Status;
+
+    Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
+    Ghcb = Msr.Ghcb;
+
+    VmgInit (Ghcb, &InterruptState);
+
+    Ghcb->SaveArea.Rax = SEV_PAGE_ENC_HYPERCALL;
+    GhcbSetRegValid (Ghcb, GhcbRax);
+    Ghcb->SaveArea.Rbx = PhysicalAddress;
+    GhcbSetRegValid (Ghcb, GhcbRbx);
+    Ghcb->SaveArea.Rcx = Pages;
+    GhcbSetRegValid (Ghcb, GhcbRcx);
+    Ghcb->SaveArea.Rdx = Mode;
+    GhcbSetRegValid (Ghcb, GhcbRdx);
+    Ghcb->SaveArea.Cpl = GetCurrentCpuPrivilegeLevel();
+    GhcbSetRegValid (Ghcb, GhcbCpl);
+
+    Status = VmgExit (Ghcb, SVM_EXIT_VMMCALL, 0, 0);
+    if (Status) {
+      DEBUG ((DEBUG_ERROR, "SVM_EXIT_VMMCALL failed %lx\n", Status));
+    }
+    VmgDone (Ghcb, InterruptState);
+  } else {
+    SetMemoryEncDecHypercall3AsmStub (
+      SEV_PAGE_ENC_HYPERCALL,
+      PhysicalAddress,
+      Pages,
+      Mode);
+  }
+}
diff --git a/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf b/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
new file mode 100644
index 0000000000..1936fe5b37
--- /dev/null
+++ b/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
@@ -0,0 +1,39 @@
+## @file
+#  Library provides the hypervisor helper functions for SEV guest
+#
+# Copyright (c) 2020 Advanced Micro Devices. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#
+##
+
+[Defines]
+  INF_VERSION                    = 1.25
+  BASE_NAME                      = MemEncryptHypercallLib
+  FILE_GUID                      = 86f2501e-f128-45f3-91c4-3cff31656ca8
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = MemEncryptHypercallLib|SEC PEI_CORE PEIM DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_DRIVER
+
+#
+# The following information is for reference only and not required by the build
+# tools.
+#
+# VALID_ARCHITECTURES           = IA32 X64
+#
+
+[Packages]
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  UefiCpuPkg/UefiCpuPkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[Sources.X64]
+  MemEncryptHypercallLib.c
+  X64/AsmHelperStub.nasm
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  VmgExitLib
diff --git a/OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm b/OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm
new file mode 100644
index 0000000000..5d8a7aa85a
--- /dev/null
+++ b/OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm
@@ -0,0 +1,39 @@
+DEFAULT REL
+SECTION .text
+
+; VOID
+; EFIAPI
+; SetMemoryEncDecHypercall3AsmStub (
+;   IN UINT HypercallNum,
+;   IN INTN Arg1,
+;   IN INTN Arg2,
+;   IN INTN Arg3
+;   );
+global ASM_PFX(SetMemoryEncDecHypercall3AsmStub)
+ASM_PFX(SetMemoryEncDecHypercall3AsmStub):
+  ; UEFI calling conventions require RBX to
+  ; be nonvolatile/callee-saved.
+  push rbx
+  ; Copy HypercallNumber to rax
+  mov rax, rcx
+  ; Copy Arg1 to the register expected by KVM
+  mov rbx, rdx
+  ; Copy Arg2 to register expected by KVM
+  mov rcx, r8
+  ; Copy Arg2 to register expected by KVM
+  mov rdx, r9
+  ; Call VMMCALL
+  vmmcall
+  pop rbx
+  ret
+
+; UINT8
+; EFIAPI
+; GetCurrentCpuPrivilegeLevel (
+;   VOID
+;   );
+global ASM_PFX(GetCurrentCpuPrivilegeLevel)
+ASM_PFX(GetCurrentCpuPrivilegeLevel):
+  mov  ax, cs
+  and  al, 0x3
+  ret
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index e59ae05b73..97c31c7586 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -174,6 +174,7 @@
   VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
   LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
   MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
+  MemEncryptHypercallLib|OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
 !if $(SMM_REQUIRE) == FALSE
   LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
 !endif
-- 
2.17.1

[PATCH v2 2/3] OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall

[Ashish Kalra]

From: Brijesh Singh <brijesh.singh@amd.com>

By default all the SEV guest memory regions are considered encrypted,
if a guest changes the encryption attribute of the page (e.g mark a
page as decrypted) then notify hypervisor. Hypervisor will need to
track the unencrypted pages. The information will be used during
guest live migration, guest page migration and guest debugging.

Invoke hypercall via the new hypercall library.

This hypercall is used to notify hypervisor when a page is marked as
'decrypted' (i.e C-bit removed).

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf |  1 +
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c      | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
index 7c44d09528..95ee707918 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
@@ -46,6 +46,7 @@
   DebugLib
   MemoryAllocationLib
   PcdLib
+  MemEncryptHypercallLib
 
 [FeaturePcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c
index 5e110c84ff..1e670b6200 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c
@@ -14,6 +14,7 @@
 #include <Library/CpuLib.h>
 #include <Register/Amd/Cpuid.h>
 #include <Register/Cpuid.h>
+#include <Library/MemEncryptHypercallLib.h>
 
 #include "VirtualMemory.h"
 
@@ -589,6 +590,9 @@ SetMemoryEncDec (
   UINT64                         AddressEncMask;
   BOOLEAN                        IsWpEnabled;
   RETURN_STATUS                  Status;
+  UINTN                          Size;
+  BOOLEAN                        CBitChanged;
+  PHYSICAL_ADDRESS               OrigPhysicalAddress;
 
   //
   // Set PageMapLevel4Entry to suppress incorrect compiler/analyzer warnings.
@@ -640,6 +644,10 @@ SetMemoryEncDec (
 
   Status = EFI_SUCCESS;
 
+  Size = Length;
+  CBitChanged = FALSE;
+  OrigPhysicalAddress = PhysicalAddress;
+
   while (Length)
   {
     //
@@ -699,6 +707,7 @@ SetMemoryEncDec (
           ));
         PhysicalAddress += BIT30;
         Length -= BIT30;
+        CBitChanged = TRUE;
       } else {
         //
         // We must split the page
@@ -753,6 +762,7 @@ SetMemoryEncDec (
           SetOrClearCBit (&PageDirectory2MEntry->Uint64, Mode);
           PhysicalAddress += BIT21;
           Length -= BIT21;
+          CBitChanged = TRUE;
         } else {
           //
           // We must split up this page into 4K pages
@@ -795,6 +805,7 @@ SetMemoryEncDec (
         SetOrClearCBit (&PageTableEntry->Uint64, Mode);
         PhysicalAddress += EFI_PAGE_SIZE;
         Length -= EFI_PAGE_SIZE;
+        CBitChanged = TRUE;
       }
     }
   }
@@ -812,6 +823,13 @@ SetMemoryEncDec (
   //
   CpuFlushTlb();
 
+  //
+  // Notify Hypervisor on C-bit status
+  //
+  if (CBitChanged) {
+    SetMemoryEncDecHypercall3 (OrigPhysicalAddress, EFI_SIZE_TO_PAGES(Size), !Mode);
+  }
+
 Done:
   //
   // Restore page table write protection, if any.
-- 
2.17.1

[PATCH v2 3/3] OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encryption bitmap.

[Ashish Kalra]

From: Ashish Kalra <ashish.kalra@amd.com>

Mark the SEC GHCB page that is mapped as unencrypted in
ResetVector code in the hypervisor page encryption bitmap.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>

Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 OvmfPkg/PlatformPei/AmdSev.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index 4a515a4847..456d32be84 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -15,6 +15,7 @@
 #include <Library/HobLib.h>
 #include <Library/MemEncryptSevLib.h>
 #include <Library/MemoryAllocationLib.h>
+#include <Library/MemEncryptHypercallLib.h>
 #include <Library/PcdLib.h>
 #include <PiPei.h>
 #include <Register/Amd/Cpuid.h>
@@ -77,6 +78,15 @@ AmdSevEsInitialize (
 
   ZeroMem (GhcbBase, EFI_PAGES_TO_SIZE (GhcbPageCount));
 
+  //
+  // GHCB_BASE setup during reset-vector needs to be marked as
+  // decrypted in the hypervisor page encryption bitmap.
+  //
+  SetMemoryEncDecHypercall3 (FixedPcdGet32 (PcdOvmfSecGhcbBase),
+    EFI_SIZE_TO_PAGES(FixedPcdGet32 (PcdOvmfSecGhcbSize)),
+    FALSE
+    );
+
   PcdStatus = PcdSet64S (PcdGhcbBase, GhcbBasePa);
   ASSERT_RETURN_ERROR (PcdStatus);
   PcdStatus = PcdSet64S (PcdGhcbSize, EFI_PAGES_TO_SIZE (GhcbPageCount));
-- 
2.17.1

Re: [PATCH v2 3/3] OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encryption bitmap.

[Lendacky, Thomas]

On 12/3/20 4:27 PM, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> Mark the SEC GHCB page that is mapped as unencrypted in
> ResetVector code in the hypervisor page encryption bitmap.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> 
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>   OvmfPkg/PlatformPei/AmdSev.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
> index 4a515a4847..456d32be84 100644
> --- a/OvmfPkg/PlatformPei/AmdSev.c
> +++ b/OvmfPkg/PlatformPei/AmdSev.c
> @@ -15,6 +15,7 @@
>   #include <Library/HobLib.h>
>   #include <Library/MemEncryptSevLib.h>
>   #include <Library/MemoryAllocationLib.h>
> +#include <Library/MemEncryptHypercallLib.h>
>   #include <Library/PcdLib.h>
>   #include <PiPei.h>
>   #include <Register/Amd/Cpuid.h>
> @@ -77,6 +78,15 @@ AmdSevEsInitialize (
>   
>     ZeroMem (GhcbBase, EFI_PAGES_TO_SIZE (GhcbPageCount));
>   
> +  //
> +  // GHCB_BASE setup during reset-vector needs to be marked as
> +  // decrypted in the hypervisor page encryption bitmap.
> +  //
> +  SetMemoryEncDecHypercall3 (FixedPcdGet32 (PcdOvmfSecGhcbBase),
> +    EFI_SIZE_TO_PAGES(FixedPcdGet32 (PcdOvmfSecGhcbSize)),
> +    FALSE
> +    );
> +

This seems like an odd place to add this. Why not near the top of the 
function just after setting the PcdSevEsIsEnabled PCD, rather than in the 
middle of setting up the new GHCB pages.

Thanks,
Tom

>     PcdStatus = PcdSet64S (PcdGhcbBase, GhcbBasePa);
>     ASSERT_RETURN_ERROR (PcdStatus);
>     PcdStatus = PcdSet64S (PcdGhcbSize, EFI_PAGES_TO_SIZE (GhcbPageCount));
>