[RFC PATCH 00/14] Firmware Support for Fast Live Migration for AMD SEV

[RFC PATCH 00/14] Firmware Support for Fast Live Migration for AMD SEV

[Tobin Feldman-Fitzthum]

This is a demonstration of fast migration for encrypted virtual machines
using a Migration Handler that lives in OVMF. This demo uses AMD SEV,
but the ideas may generalize to other confidential computing platforms.
With AMD SEV, guest memory is encrypted and the hypervisor cannot access
or move it. This makes migration tricky. In this demo, we show how the
HV can ask a Migration Handler (MH) in the firmware for an encrypted
page. The MH encrypts the page with a transport key prior to releasing
it to the HV. The target machine also runs an MH that decrypts the page
once it is passed in by the target HV. These patches are not ready for
production, but the are a full end-to-end solution that facilitates a
fast live migration between two SEV VMs.

Corresponding patches for QEMU have been posted my colleague Dov Murik
on qemu-devel. Our approach needs little kernel support, requiring only
one hypercall that the guest can use to mark a page as encrypted or
shared. This series includes updated patches from Ashish Kalra and
Brijesh Singh that allow OVMF to use this hypercall. 

The MH runs continuously in the guest, waiting for communication from
the HV. The HV starts an additional vCPU for the MH but does not expose
it to the guest OS via ACPI. We use the MpService to start the MH. The
MpService is only available at runtime and processes that are started by
it are usually cleaned up on ExitBootServices. Since we need the MH to
run continuously, we had to make some modifications. Ideally a feature
could be added to the MpService to allow for the starting of
long-running processes. Besides migration, this could support other
background processes that need to operate within the encryption
boundary. For now, we have included a handful of patches that modify the
MpService to allow the MH to keep running after ExitBootServices. These
are temporary. 

Ashish Kalra (2):
  OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap.
  OvmfPkg/PlatformDxe: Add support for SEV live migration.

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

Dov Murik (1):
  OvmfPkg/AmdSev: Build page table for migration handler

Tobin Feldman-Fitzthum (10):
  OvmfPkg/AmdSev: Base for Confidential Migration Handler
  OvmfPkg/PlatfomPei: Set Confidential Migration PCD
  OvmfPkg/AmdSev: Setup Migration Handler Mailbox
  OvmfPkg/AmdSev: MH support for mailbox protocol
  UefiCpuPkg/MpInitLib: temp removal of MpLib cleanup
  UefiCpuPkg/MpInitLib: Allocate MP buffer as runtime memory
  UefiCpuPkg/CpuExceptionHandlerLib: Exception handling as runtime
    memory
  OvmfPkg/AmdSev: Don't overwrite mailbox or pagetables
  OvmfPkg/AmdSev: Don't overwrite MH stack
  OvmfPkg/AmdSev: MH page encryption POC

 OvmfPkg/OvmfPkg.dec                           |  11 +
 OvmfPkg/AmdSev/AmdSevX64.dsc                  |   2 +
 OvmfPkg/AmdSev/AmdSevX64.fdf                  |  13 +-
 .../ConfidentialMigrationDxe.inf              |  45 +++
 .../ConfidentialMigrationPei.inf              |  35 ++
 .../DxeMemEncryptSevLib.inf                   |   1 +
 .../PeiMemEncryptSevLib.inf                   |   1 +
 OvmfPkg/PlatformDxe/Platform.inf              |   2 +
 OvmfPkg/PlatformPei/PlatformPei.inf           |   2 +
 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   2 +
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   2 +
 OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h  | 235 +++++++++++++
 .../ConfidentialMigration/VirtualMemory.h     | 177 ++++++++++
 OvmfPkg/Include/Guid/MemEncryptLib.h          |  16 +
 OvmfPkg/PlatformDxe/PlatformConfig.h          |   5 +
 .../ConfidentialMigrationDxe.c                | 325 ++++++++++++++++++
 .../ConfidentialMigrationPei.c                |  25 ++
 .../X64/PeiDxeVirtualMemory.c                 |  18 +
 OvmfPkg/PlatformDxe/AmdSev.c                  |  99 ++++++
 OvmfPkg/PlatformDxe/Platform.c                |   6 +
 OvmfPkg/PlatformPei/AmdSev.c                  |  10 +
 OvmfPkg/PlatformPei/Platform.c                |  10 +
 .../CpuExceptionHandlerLib/DxeException.c     |   8 +-
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c       |  21 +-
 UefiCpuPkg/Library/MpInitLib/MpLib.c          |   7 +-
 25 files changed, 1061 insertions(+), 17 deletions(-)
 create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
 create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.inf
 create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h
 create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h
 create mode 100644 OvmfPkg/Include/Guid/MemEncryptLib.h
 create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
 create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.c
 create mode 100644 OvmfPkg/PlatformDxe/AmdSev.c

-- 
2.20.1

[RFC PATCH 01/14] OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall

[Tobin Feldman-Fitzthum]

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>
---
 .../DxeMemEncryptSevLib.inf                    |  1 +
 .../PeiMemEncryptSevLib.inf                    |  1 +
 .../X64/PeiDxeVirtualMemory.c                  | 18 ++++++++++++++++++
 3 files changed, 20 insertions(+)

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
index f2e162d680..aefcd7c0f7 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
@@ -49,6 +49,7 @@
   DebugLib
   MemoryAllocationLib
   PcdLib
+  MemEncryptHypercallLib
 
 [FeaturePcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
index 03a78c32df..7503f56a0b 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
@@ -49,6 +49,7 @@
   DebugLib
   MemoryAllocationLib
   PcdLib
+  MemEncryptHypercallLib
 
 [FeaturePcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
index d3455e812b..98a1d2e3a8 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
@@ -15,6 +15,7 @@
 #include <Library/MemEncryptSevLib.h>
 #include <Register/Amd/Cpuid.h>
 #include <Register/Cpuid.h>
+#include <Library/MemEncryptHypercallLib.h>
 
 #include "VirtualMemory.h"
 
@@ -585,6 +586,9 @@ SetMemoryEncDec (
   UINT64                         AddressEncMask;
   BOOLEAN                        IsWpEnabled;
   RETURN_STATUS                  Status;
+  UINTN                          Size;
+  BOOLEAN                        CBitChanged;
+  PHYSICAL_ADDRESS               OrigPhysicalAddress;
 
   //
   // Set PageMapLevel4Entry to suppress incorrect compiler/analyzer warnings.
@@ -636,6 +640,10 @@ SetMemoryEncDec (
 
   Status = EFI_SUCCESS;
 
+  Size = Length;
+  CBitChanged = FALSE;
+  OrigPhysicalAddress = PhysicalAddress;
+
   while (Length != 0)
   {
     //
@@ -695,6 +703,7 @@ SetMemoryEncDec (
           ));
         PhysicalAddress += BIT30;
         Length -= BIT30;
+        CBitChanged = TRUE;
       } else {
         //
         // We must split the page
@@ -749,6 +758,7 @@ SetMemoryEncDec (
           SetOrClearCBit (&PageDirectory2MEntry->Uint64, Mode);
           PhysicalAddress += BIT21;
           Length -= BIT21;
+          CBitChanged = TRUE;
         } else {
           //
           // We must split up this page into 4K pages
@@ -791,6 +801,7 @@ SetMemoryEncDec (
         SetOrClearCBit (&PageTableEntry->Uint64, Mode);
         PhysicalAddress += EFI_PAGE_SIZE;
         Length -= EFI_PAGE_SIZE;
+        CBitChanged = TRUE;
       }
     }
   }
@@ -808,6 +819,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.20.1

[RFC PATCH 02/14] OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap.

[Tobin Feldman-Fitzthum]

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 dddffdebda..c72eeb37c5 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/Msr.h>
@@ -52,6 +53,15 @@ AmdSevEsInitialize (
   PcdStatus = PcdSetBoolS (PcdSevEsIsEnabled, TRUE);
   ASSERT_RETURN_ERROR (PcdStatus);
 
+  //
+  // 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
+    );
+
   //
   // Allocate GHCB and per-CPU variable pages.
   //   Since the pages must survive across the UEFI to OS transition
-- 
2.20.1

[RFC PATCH 03/14] OvmfPkg/PlatformDxe: Add support for SEV live migration.

[Tobin Feldman-Fitzthum]

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

Detect for KVM hypervisor and check for SEV live migration
feature support via KVM_FEATURE_CPUID, if detected setup a new
UEFI enviroment variable to indicate OVMF support for SEV
live migration.

Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 OvmfPkg/OvmfPkg.dec                  |  1 +
 OvmfPkg/PlatformDxe/Platform.inf     |  2 +
 OvmfPkg/Include/Guid/MemEncryptLib.h | 16 +++++
 OvmfPkg/PlatformDxe/PlatformConfig.h |  5 ++
 OvmfPkg/PlatformDxe/AmdSev.c         | 99 ++++++++++++++++++++++++++++
 OvmfPkg/PlatformDxe/Platform.c       |  6 ++
 6 files changed, 129 insertions(+)
 create mode 100644 OvmfPkg/Include/Guid/MemEncryptLib.h
 create mode 100644 OvmfPkg/PlatformDxe/AmdSev.c

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 4348bb45c6..4450d78b91 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -122,6 +122,7 @@
   gQemuKernelLoaderFsMediaGuid          = {0x1428f772, 0xb64a, 0x441e, {0xb8, 0xc3, 0x9e, 0xbd, 0xd7, 0xf8, 0x93, 0xc7}}
   gGrubFileGuid                         = {0xb5ae312c, 0xbc8a, 0x43b1, {0x9c, 0x62, 0xeb, 0xb8, 0x26, 0xdd, 0x5d, 0x07}}
   gConfidentialComputingSecretGuid      = {0xadf956ad, 0xe98c, 0x484c, {0xae, 0x11, 0xb5, 0x1c, 0x7d, 0x33, 0x64, 0x47}}
+  gMemEncryptGuid                       = {0x0cf29b71, 0x9e51, 0x433a, {0xa3, 0xb7, 0x81, 0xf3, 0xab, 0x16, 0xb8, 0x75}}
 
 [Ppis]
   # PPI whose presence in the PPI database signals that the TPM base address
diff --git a/OvmfPkg/PlatformDxe/Platform.inf b/OvmfPkg/PlatformDxe/Platform.inf
index 14727c1220..2896f0a1d1 100644
--- a/OvmfPkg/PlatformDxe/Platform.inf
+++ b/OvmfPkg/PlatformDxe/Platform.inf
@@ -24,6 +24,7 @@
   PlatformConfig.c
   PlatformConfig.h
   PlatformForms.vfr
+  AmdSev.c
 
 [Packages]
   MdePkg/MdePkg.dec
@@ -56,6 +57,7 @@
 [Guids]
   gEfiIfrTianoGuid
   gOvmfPlatformConfigGuid
+  gMemEncryptGuid
 
 [Depex]
   gEfiHiiConfigRoutingProtocolGuid  AND
diff --git a/OvmfPkg/Include/Guid/MemEncryptLib.h b/OvmfPkg/Include/Guid/MemEncryptLib.h
new file mode 100644
index 0000000000..8264a647af
--- /dev/null
+++ b/OvmfPkg/Include/Guid/MemEncryptLib.h
@@ -0,0 +1,16 @@
+/** @file
+  AMD Memory Encryption GUID, define a new GUID for defining
+  new UEFI enviroment variables assocaiated with SEV Memory Encryption.
+  Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef __MEMENCRYPT_LIB_H__
+#define __MEMENCRYPT_LIB_H__
+
+#define MEMENCRYPT_GUID \
+{0x0cf29b71, 0x9e51, 0x433a, {0xa3, 0xb7, 0x81, 0xf3, 0xab, 0x16, 0xb8, 0x75}}
+
+extern EFI_GUID gMemEncryptGuid;
+
+#endif
diff --git a/OvmfPkg/PlatformDxe/PlatformConfig.h b/OvmfPkg/PlatformDxe/PlatformConfig.h
index 716514da21..4f662aafa4 100644
--- a/OvmfPkg/PlatformDxe/PlatformConfig.h
+++ b/OvmfPkg/PlatformDxe/PlatformConfig.h
@@ -44,6 +44,11 @@ PlatformConfigLoad (
   OUT UINT64          *OptionalElements
   );
 
+VOID
+AmdSevSetConfig(
+  VOID
+  );
+
 //
 // Feature flags for OptionalElements.
 //
diff --git a/OvmfPkg/PlatformDxe/AmdSev.c b/OvmfPkg/PlatformDxe/AmdSev.c
new file mode 100644
index 0000000000..1f804984b7
--- /dev/null
+++ b/OvmfPkg/PlatformDxe/AmdSev.c
@@ -0,0 +1,99 @@
+/**@file
+  Detect KVM hypervisor support for SEV live migration and if
+  detected, setup a new UEFI enviroment variable indicating
+  OVMF support for SEV live migration.
+  Copyright (c) 2020, Advanced Micro Devices. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+//
+// The package level header files this module uses
+//
+
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiRuntimeServicesTableLib.h>
+#include <Guid/MemEncryptLib.h>
+
+/**
+  Figures out if we are running inside KVM HVM and
+  KVM HVM supports SEV Live Migration feature.
+  @retval TRUE   KVM was detected and Live Migration supported
+  @retval FALSE  KVM was not detected or Live Migration not supported
+**/
+BOOLEAN
+KvmDetectSevLiveMigrationFeature(
+  VOID
+  )
+{
+  UINT8 Signature[13];
+  UINT32 mKvmLeaf = 0;
+  UINT32 RegEax, RegEbx, RegEcx, RegEdx;
+
+  Signature[12] = '\0';
+  for (mKvmLeaf = 0x40000000; mKvmLeaf < 0x40010000; mKvmLeaf += 0x100) {
+    AsmCpuid (mKvmLeaf,
+              NULL,
+              (UINT32 *) &Signature[0],
+              (UINT32 *) &Signature[4],
+              (UINT32 *) &Signature[8]);
+
+    if (!AsciiStrCmp ((CHAR8 *) Signature, "KVMKVMKVM\0\0\0")) {
+   DEBUG ((
+    DEBUG_ERROR,
+    "%a: KVM Detected, signature = %s\n",
+    __FUNCTION__,
+    Signature
+    ));
+
+    RegEax = 0x40000001;
+    RegEcx = 0;
+      AsmCpuid (0x40000001, &RegEax, &RegEbx, &RegEcx, &RegEdx);
+      if (RegEax & (1 << 14)) {
+     DEBUG ((
+    DEBUG_ERROR,
+    "%a: Live Migration feature supported\n",
+    __FUNCTION__
+    ));
+    return TRUE;
+     }
+    }
+  }
+
+  return FALSE;
+}
+
+/**
+  Function checks if SEV Live Migration support is available, if present then it sets
+  a UEFI enviroment variable to be queried later using Runtime services.
+  **/
+VOID
+AmdSevSetConfig(
+  VOID
+  )
+{
+  EFI_STATUS Status;
+  BOOLEAN SevLiveMigrationEnabled;
+
+  SevLiveMigrationEnabled = KvmDetectSevLiveMigrationFeature();
+
+  if (SevLiveMigrationEnabled) {
+   Status = gRT->SetVariable (
+    L"SevLiveMigrationEnabled",
+                &gMemEncryptGuid,
+    EFI_VARIABLE_NON_VOLATILE |
+                EFI_VARIABLE_BOOTSERVICE_ACCESS |
+          EFI_VARIABLE_RUNTIME_ACCESS,
+                sizeof (BOOLEAN),
+                &SevLiveMigrationEnabled
+               );
+
+   DEBUG ((
+    DEBUG_ERROR,
+    "%a: Setting SevLiveMigrationEnabled variable, status = %lx\n",
+    __FUNCTION__,
+    Status
+    ));
+  }
+}
diff --git a/OvmfPkg/PlatformDxe/Platform.c b/OvmfPkg/PlatformDxe/Platform.c
index f2e51960ce..9a19b9f6b1 100644
--- a/OvmfPkg/PlatformDxe/Platform.c
+++ b/OvmfPkg/PlatformDxe/Platform.c
@@ -763,6 +763,12 @@ PlatformInit (
 {
   EFI_STATUS Status;
 
+  //
+  // Set Amd Sev configuation
+  //
+  AmdSevSetConfig();
+
+
   ExecutePlatformConfig ();
 
   mConfigAccess.ExtractConfig = &ExtractConfig;
-- 
2.20.1

[RFC PATCH 04/14] OvmfPkg/AmdSev: Base for Confidential Migration Handler

[Tobin Feldman-Fitzthum]

Base enablement of DXE driver that supports confidential migration.

Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
---
 OvmfPkg/OvmfPkg.dec                           |  5 ++
 OvmfPkg/AmdSev/AmdSevX64.dsc                  |  1 +
 OvmfPkg/AmdSev/AmdSevX64.fdf                  |  1 +
 .../ConfidentialMigrationDxe.inf              | 39 +++++++++
 .../ConfidentialMigrationDxe.c                | 83 +++++++++++++++++++
 5 files changed, 129 insertions(+)
 create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
 create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 4450d78b91..402c3b61fa 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -324,6 +324,11 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId|0|UINT16|0x1b
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE|BOOLEAN|0x21
 
+  ## Set via FW_CFG to enable confidentialmigration as source or target.
+  #
+  gUefiOvmfPkgTokenSpaceGuid.PcdIsConfidentialMigrationTarget|FALSE|BOOLEAN|0x46
+  gUefiOvmfPkgTokenSpaceGuid.PcdStartConfidentialMigrationHandler|FALSE|BOOLEAN|0x47
+
   ## The IO port aperture shared by all PCI root bridges.
   #
   gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase|0x0|UINT64|0x22
diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index ca21fd6e5f..fa68143663 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -787,6 +787,7 @@
 !endif
   OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf
   OvmfPkg/AmdSev/Grub/Grub.inf
+  OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
 !if $(BUILD_SHELL) == TRUE
   ShellPkg/Application/Shell/Shell.inf {
     <LibraryClasses>
diff --git a/OvmfPkg/AmdSev/AmdSevX64.fdf b/OvmfPkg/AmdSev/AmdSevX64.fdf
index c0098502aa..6ef6dc89f2 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.fdf
+++ b/OvmfPkg/AmdSev/AmdSevX64.fdf
@@ -273,6 +273,7 @@ INF  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
 INF  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
 !endif
 INF OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf
+INF OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
 INF  OvmfPkg/AmdSev/Grub/Grub.inf
 !if $(BUILD_SHELL) == TRUE
 INF  ShellPkg/Application/Shell/Shell.inf
diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
new file mode 100644
index 0000000000..a4906a2451
--- /dev/null
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
@@ -0,0 +1,39 @@
+## @file
+#
+#  Copyright (C) 2021 IBM Corporation.
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = ConfidentialMigration
+  FILE_GUID                      = 5c2978f4-f175-434b-9e6c-9b03bd7e346f
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = LaunchMigrationHandler
+
+[Sources]
+  ConfidentialMigrationDxe.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+  UefiCpuPkg/UefiCpuPkg.dec
+
+[LibraryClasses]
+  MemoryAllocationLib
+  DebugLib
+  UefiBootServicesTableLib
+  MpInitLib
+  UefiDriverEntryPoint
+
+[Protocols]
+  gEfiMpServiceProtocolGuid
+
+[Pcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdIsConfidentialMigrationTarget
+  gUefiOvmfPkgTokenSpaceGuid.PcdStartConfidentialMigrationHandler
+
+[Depex]
+  gEfiMpServiceProtocolGuid
diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
new file mode 100644
index 0000000000..6d9fe7043b
--- /dev/null
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
@@ -0,0 +1,83 @@
+/** @file
+  In-guest support for confidential migration
+
+  Copyright (C) 2021 IBM Coporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+#include <Library/UefiLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiRuntimeServicesTableLib.h>
+#include <Library/DebugLib.h>
+#include <Protocol/MpService.h>
+
+UINTN MigrationHandlerCpuIndex;
+
+VOID
+EFIAPI
+MigrationHandlerMain (
+  IN OUT VOID *Buffer
+  )
+{
+  DebugPrint (DEBUG_INFO,"MIGRATION Handler Started\n");
+}
+
+EFI_STATUS
+EFIAPI
+LaunchMigrationHandler (
+  IN EFI_HANDLE           ImageHandle,
+  IN EFI_SYSTEM_TABLE     *SystemTable
+  )
+{
+  EFI_MP_SERVICES_PROTOCOL      *MpProto;
+  EFI_PROCESSOR_INFORMATION     Tcb;
+  EFI_STATUS                    Status;
+  UINTN                         NumProc;
+  UINTN                         NumEnabled;
+
+  gST = SystemTable;
+  gBS = gST->BootServices;
+  gRT = gST->RuntimeServices;
+
+  Status = EFI_NOT_STARTED;
+
+  if (!PcdGetBool(PcdStartConfidentialMigrationHandler)) {
+    return 0;
+  }
+
+  //
+  // Use the MP Service protocol to start Migration Handler on AP
+  //
+  gBS->LocateProtocol (&gEfiMpServiceProtocolGuid, NULL, (void**)&MpProto);
+  MpProto->GetNumberOfProcessors (MpProto, &NumProc, &NumEnabled);
+  if (NumProc < 2) {
+    DebugPrint (DEBUG_ERROR,"Only one vCPU enabled. Please start an extra for the MH.\n");
+    return 0;
+  }
+
+  MigrationHandlerCpuIndex = NumProc - 1;
+
+  EFI_EVENT Event;
+  MpProto->GetProcessorInfo (MpProto, MigrationHandlerCpuIndex, &Tcb);
+  if (Tcb.StatusFlag != 7) {
+    gBS->CreateEvent (EVT_TIMER, TPL_NOTIFY, NULL, NULL, &Event);
+
+    Status = MpProto->StartupThisAP(MpProto, MigrationHandlerMain, 1, Event,
+                                    0, MpProto, NULL);
+  }
+  if (Status != EFI_SUCCESS) {
+    DebugPrint (DEBUG_ERROR,"Failed to start Migration Handler\n");
+    return 0;
+  }
+
+  //
+  // If we are the target, wait for incoming migration. Otherwise,
+  // procede with the boot.
+  //
+  if (PcdGetBool(PcdIsConfidentialMigrationTarget)) {
+    DebugPrint (DEBUG_INFO,"Waiting for incoming confidential migration.\n");
+    DisableInterrupts ();
+    CpuDeadLoop ();
+  }
+
+  return 0;
+}
-- 
2.20.1

[RFC PATCH 05/14] OvmfPkg/PlatfomPei: Set Confidential Migration PCD

[Tobin Feldman-Fitzthum]

Confidential Migration relies on two boolean PCDs set from FW_CFG

Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
---
 OvmfPkg/PlatformPei/PlatformPei.inf |  2 ++
 OvmfPkg/PlatformPei/Platform.c      | 10 ++++++++++
 2 files changed, 12 insertions(+)

diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 6ef77ba7bb..66e6fcfa4f 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -92,6 +92,8 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecompressionScratchEnd
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase
+  gUefiOvmfPkgTokenSpaceGuid.PcdStartConfidentialMigrationHandler
+  gUefiOvmfPkgTokenSpaceGuid.PcdIsConfidentialMigrationTarget
   gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 96468701e3..5926c8d414 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -275,6 +275,15 @@ NoexecDxeInitialization (
   UPDATE_BOOLEAN_PCD_FROM_FW_CFG (PcdSetNxForStack);
 }
 
+VOID
+ConfidentialMigrationInitialization (
+  VOID
+  )
+{
+  UPDATE_BOOLEAN_PCD_FROM_FW_CFG (PcdStartConfidentialMigrationHandler);
+  UPDATE_BOOLEAN_PCD_FROM_FW_CFG (PcdIsConfidentialMigrationTarget);
+}
+
 VOID
 PciExBarInitialization (
   VOID
@@ -752,6 +761,7 @@ InitializePlatform (
 
   InstallClearCacheCallback ();
   AmdSevInitialize ();
+  ConfidentialMigrationInitialization ();
   MiscInitialization ();
   InstallFeatureControlCallback ();
 
-- 
2.20.1

[RFC PATCH 06/14] OvmfPkg/AmdSev: Setup Migration Handler Mailbox

[Tobin Feldman-Fitzthum]

The migration handler communicates with the hypervisor using a
special mailbox, a page of shared memory where pending commands
can be written. Another shared page is used to pass the incoming
or outgoing guest memory pages. These pages are set aside in MEMFD,
which this patch expands, and reserved as runtime memory in
ConfidentialMigrationPei, which this patch introduces.

Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
---
 OvmfPkg/OvmfPkg.dec                           |  5 +++
 OvmfPkg/AmdSev/AmdSevX64.dsc                  |  1 +
 OvmfPkg/AmdSev/AmdSevX64.fdf                  | 12 ++++---
 .../ConfidentialMigrationPei.inf              | 35 +++++++++++++++++++
 .../ConfidentialMigrationPei.c                | 25 +++++++++++++
 5 files changed, 74 insertions(+), 4 deletions(-)
 create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.inf
 create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.c

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 402c3b61fa..5c55e3c7c9 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -318,6 +318,11 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase|0x0|UINT32|0x42
   gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretSize|0x0|UINT32|0x43
 
+  ## Area used by the confidential migration handler to communicate with
+  # the hypervisor.
+  gUefiOvmfPkgTokenSpaceGuid.PcdConfidentialMigrationMailboxBase|0x0|UINT32|0x48
+  gUefiOvmfPkgTokenSpaceGuid.PcdConfidentialMigrationMailboxSize|0x0|UINT32|0x49
+
 [PcdsDynamic, PcdsDynamicEx]
   gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN|0x10
diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index fa68143663..4f748a0015 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -620,6 +620,7 @@
   UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
   UefiCpuPkg/CpuMpPei/CpuMpPei.inf
   OvmfPkg/AmdSev/SecretPei/SecretPei.inf
+  OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.inf
 
 !if $(TPM_ENABLE) == TRUE
   OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
diff --git a/OvmfPkg/AmdSev/AmdSevX64.fdf b/OvmfPkg/AmdSev/AmdSevX64.fdf
index 6ef6dc89f2..94468f2ca0 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.fdf
+++ b/OvmfPkg/AmdSev/AmdSevX64.fdf
@@ -36,10 +36,10 @@ FV = SECFV
 
 [FD.MEMFD]
 BaseAddress   = $(MEMFD_BASE_ADDRESS)
-Size          = 0xD00000
+Size          = 0xE00000
 ErasePolarity = 1
 BlockSize     = 0x10000
-NumBlocks     = 0xD0
+NumBlocks     = 0xE0
 
 0x000000|0x006000
 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
@@ -68,11 +68,14 @@ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase|gUefiOvmfPkgTokenSpaceGuid.P
 0x010000|0x010000
 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize
 
-0x020000|0x0E0000
+0x020000|0x003000
+gUefiOvmfPkgTokenSpaceGuid.PcdConfidentialMigrationMailboxBase|gUefiOvmfPkgTokenSpaceGuid.PcdConfidentialMigrationMailboxSize
+
+0x120000|0x0E0000
 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
 FV = PEIFV
 
-0x100000|0xC00000
+0x200000|0xC00000
 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
 FV = DXEFV
 
@@ -145,6 +148,7 @@ INF  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
 INF  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
 INF  UefiCpuPkg/CpuMpPei/CpuMpPei.inf
 INF  OvmfPkg/AmdSev/SecretPei/SecretPei.inf
+INF  OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.inf
 
 !if $(TPM_ENABLE) == TRUE
 INF  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.inf b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.inf
new file mode 100644
index 0000000000..918cf22abd
--- /dev/null
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.inf
@@ -0,0 +1,35 @@
+## @file
+#  PEI support for confidential migration.
+#
+#  Copyright (C) 2021 IBM Corporation.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = ConfidentialMigration
+  FILE_GUID                      = a747792e-71a1-4c24-84a9-a76a0a279878
+  MODULE_TYPE                    = PEIM
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = InitializeConfidentialMigrationPei
+
+[Sources]
+  ConfidentialMigrationPei.c
+
+[Packages]
+  OvmfPkg/OvmfPkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  HobLib
+  PeimEntryPoint
+  PcdLib
+
+[FixedPcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdConfidentialMigrationMailboxBase
+  gUefiOvmfPkgTokenSpaceGuid.PcdConfidentialMigrationMailboxSize
+
+[Depex]
+  TRUE
diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.c b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.c
new file mode 100644
index 0000000000..ce304bc07b
--- /dev/null
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.c
@@ -0,0 +1,25 @@
+/** @file
+  Reserve memory for confidential migration handler.
+
+  Copyright (C) 2020 IBM Corporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+#include <PiPei.h>
+#include <Library/HobLib.h>
+#include <Library/PcdLib.h>
+
+EFI_STATUS
+EFIAPI
+InitializeConfidentialMigrationPei (
+  IN       EFI_PEI_FILE_HANDLE  FileHandle,
+  IN CONST EFI_PEI_SERVICES     **PeiServices
+  )
+{
+  BuildMemoryAllocationHob (
+    PcdGet32 (PcdConfidentialMigrationMailboxBase),
+    PcdGet32 (PcdConfidentialMigrationMailboxSize),
+    EfiRuntimeServicesData
+    );
+
+  return EFI_SUCCESS;
+}
-- 
2.20.1

[RFC PATCH 07/14] OvmfPkg/AmdSev: MH support for mailbox protocol

[Tobin Feldman-Fitzthum]

The migration handler communicates with the hypervisor
via a shared mailbox page. The MH can perform four functions
at the behest of the HV: init, save page, restore page, and
reset.

Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
---
 .../ConfidentialMigrationDxe.inf              |  1 +
 .../ConfidentialMigrationDxe.c                | 78 +++++++++++++++++++
 2 files changed, 79 insertions(+)

diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
index a4906a2451..49457d5d17 100644
--- a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
@@ -34,6 +34,7 @@
 [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdIsConfidentialMigrationTarget
   gUefiOvmfPkgTokenSpaceGuid.PcdStartConfidentialMigrationHandler
+  gUefiOvmfPkgTokenSpaceGuid.PcdConfidentialMigrationMailboxBase
 
 [Depex]
   gEfiMpServiceProtocolGuid
diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
index 6d9fe7043b..8402fcc4fa 100644
--- a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
@@ -9,16 +9,94 @@
 #include <Library/UefiRuntimeServicesTableLib.h>
 #include <Library/DebugLib.h>
 #include <Protocol/MpService.h>
+#include <Library/BaseMemoryLib.h>
 
+//
+// Functions implemented by the migration handler
+//
+#define MH_FUNC_INIT          0
+#define MH_FUNC_SAVE_PAGE     1
+#define MH_FUNC_RESTORE_PAGE  2
+#define MH_FUNC_RESET         3
+
+//
+// Return codes for MH functions
+//
+#define MH_SUCCESS            0
+#define MH_INVALID_FUNC     (-1)
+#define MH_AUTH_ERR         (-2)
+
+//
+// Index of CPU that MH runs on.
+//
 UINTN MigrationHandlerCpuIndex;
 
+//
+// Mailbox for communication with hypervisor
+//
+typedef volatile struct {
+  UINT64       nr;
+  UINT64       gpa;
+  UINT32       do_prefetch;
+  UINT32       ret;
+  UINT32       go;
+  UINT32       done;
+} MH_COMMAND_PARAMETERS;
+
+
 VOID
 EFIAPI
 MigrationHandlerMain (
   IN OUT VOID *Buffer
   )
 {
+  UINT64                       params_base;
+  MH_COMMAND_PARAMETERS        *params;
+  VOID                         *page_va;
+
   DebugPrint (DEBUG_INFO,"MIGRATION Handler Started\n");
+
+  params_base = PcdGet32 (PcdConfidentialMigrationMailboxBase);
+  params = (VOID *)params_base;
+  page_va = (VOID *)params_base + 0x1000;
+
+  DisableInterrupts();
+  params->go = 0;
+
+  while (1) {
+    while (!params->go) {
+      CpuPause();
+    }
+    params->done = 0;
+
+    switch (params->nr) {
+    case MH_FUNC_INIT:
+      params->ret = MH_SUCCESS;
+      break;
+
+    case MH_FUNC_SAVE_PAGE:
+      CopyMem(page_va, (VOID *)params->gpa, 4096);
+      params->ret = MH_SUCCESS;
+      break;
+
+    case MH_FUNC_RESTORE_PAGE:
+      CopyMem((VOID *)params->gpa, page_va, 4096);
+      params->ret = MH_SUCCESS;
+      break;
+
+    case MH_FUNC_RESET:
+      params->ret = MH_SUCCESS;
+      break;
+
+    default:
+      params->ret = MH_INVALID_FUNC;
+      break;
+    }
+
+    params->go = 0;
+    params->done = 1;
+
+  }
 }
 
 EFI_STATUS
-- 
2.20.1

[RFC PATCH 08/14] UefiCpuPkg/MpInitLib: temp removal of MpLib cleanup

[Tobin Feldman-Fitzthum]

The Migration Handdler is started using the Mp Service, which
is only designed to function during boot time. The MH needs
to run continuously. In the abscence of a generalized persitent
Mp Service, temporary alterations were made to keep the MH running.

Here, we skip registering the ExitBootServices callback that
would normally clean up the APs. Obviously this is not suitable
for production, as it does not generalize for multiple APs
(it leaves all APs untouched rather than just the MH) and it
introduces a weird dependency where the MpLib needs an
OVMF PCD.

Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
---
 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |  2 ++
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c       | 21 ++++++++++++-------
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
index 1771575c69..71cc968de8 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
@@ -39,6 +39,7 @@
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
+  OvmfPkg/OvmfPkg.dec
 
 [LibraryClasses]
   BaseLib
@@ -76,3 +77,4 @@
   gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase                       ## SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard                      ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase                           ## CONSUMES
+  gUefiOvmfPkgTokenSpaceGuid.PcdStartConfidentialMigrationHandler
diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 7839c24976..7d59ec4a92 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -569,14 +569,19 @@ InitMpGlobalData (
                   );
   ASSERT_EFI_ERROR (Status);
 
-  Status = gBS->CreateEvent (
-                  EVT_SIGNAL_EXIT_BOOT_SERVICES,
-                  TPL_CALLBACK,
-                  MpInitChangeApLoopCallback,
-                  NULL,
-                  &mMpInitExitBootServicesEvent
-                  );
-  ASSERT_EFI_ERROR (Status);
+  //
+  // Workaround for persistent processes .
+  //
+  if (!PcdGetBool (PcdStartConfidentialMigrationHandler)) {
+    Status = gBS->CreateEvent (
+                    EVT_SIGNAL_EXIT_BOOT_SERVICES,
+                    TPL_CALLBACK,
+                    MpInitChangeApLoopCallback,
+                    NULL,
+                    &mMpInitExitBootServicesEvent
+                    );
+    ASSERT_EFI_ERROR (Status);
+  }
 
   Status = gBS->CreateEventEx (
                   EVT_NOTIFY_SIGNAL,
-- 
2.20.1

[RFC PATCH 09/14] UefiCpuPkg/MpInitLib: Allocate MP buffer as runtime memory

[Tobin Feldman-Fitzthum]

Another temporary change to support the persistence of the MH.
The Mp buffer needs to be allocated as runtime memory or it
may be overwritten by the OS.

Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
---
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 2 ++
 UefiCpuPkg/Library/MpInitLib/MpLib.c          | 7 ++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
index 34abf25d43..0b26cf6aaf 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
@@ -39,6 +39,7 @@
   MdePkg/MdePkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
   MdeModulePkg/MdeModulePkg.dec
+  OvmfPkg/OvmfPkg.dec
 
 [LibraryClasses]
   BaseLib
@@ -65,6 +66,7 @@
   gUefiCpuPkgTokenSpaceGuid.PcdSevEsIsEnabled                      ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase                   ## SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase                       ## CONSUMES
+  gUefiOvmfPkgTokenSpaceGuid.PcdStartConfidentialMigrationHandler
 
 [Ppis]
   gEdkiiPeiShadowMicrocodePpiGuid        ## SOMETIMES_CONSUMES
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 2568986d8c..0ca2858ca3 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1974,7 +1974,12 @@ MpInitLibInitialize (
   BufferSize += VolatileRegisters.Idtr.Limit + 1;
   BufferSize += sizeof (CPU_MP_DATA);
   BufferSize += (sizeof (CPU_AP_DATA) + sizeof (CPU_INFO_IN_HOB))* MaxLogicalProcessorNumber;
-  MpBuffer    = AllocatePages (EFI_SIZE_TO_PAGES (BufferSize));
+  if (PcdGetBool (PcdStartConfidentialMigrationHandler)) {
+    MpBuffer    = AllocateRuntimePages (EFI_SIZE_TO_PAGES (BufferSize));
+  }
+  else {
+    MpBuffer    = AllocatePages (EFI_SIZE_TO_PAGES (BufferSize));
+  }
   ASSERT (MpBuffer != NULL);
   ZeroMem (MpBuffer, BufferSize);
   Buffer = (UINTN) MpBuffer;
-- 
2.20.1

[RFC PATCH 10/14] UefiCpuPkg/CpuExceptionHandlerLib: Exception handling as runtime memory

[Tobin Feldman-Fitzthum]

Reserve IDT and other exception-related memory as runtime so
it won't be overwritten by the OS while the MH is running.

Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
---
 UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
index fd59f09ecd..35610f8cf5 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
@@ -102,7 +102,7 @@ InitializeCpuInterruptHandlers (
   EFI_CPU_INTERRUPT_HANDLER          *ExternalInterruptHandler;
 
   Status = gBS->AllocatePool (
-                  EfiBootServicesCode,
+                  EfiRuntimeServicesCode,
                   sizeof (RESERVED_VECTORS_DATA) * CPU_INTERRUPT_NUM,
                   (VOID **)&ReservedVectors
                   );
@@ -116,7 +116,7 @@ InitializeCpuInterruptHandlers (
     }
   }
 
-  ExternalInterruptHandler = AllocateZeroPool (sizeof (EFI_CPU_INTERRUPT_HANDLER) * CPU_INTERRUPT_NUM);
+  ExternalInterruptHandler = AllocateRuntimeZeroPool (sizeof (EFI_CPU_INTERRUPT_HANDLER) * CPU_INTERRUPT_NUM);
   ASSERT (ExternalInterruptHandler != NULL);
 
   //
@@ -130,7 +130,7 @@ InitializeCpuInterruptHandlers (
   //
   // Create Interrupt Descriptor Table and Copy the old IDT table in
   //
-  IdtTable = AllocateZeroPool (sizeof (IA32_IDT_GATE_DESCRIPTOR) * CPU_INTERRUPT_NUM);
+  IdtTable = AllocateRuntimeZeroPool (sizeof (IA32_IDT_GATE_DESCRIPTOR) * CPU_INTERRUPT_NUM);
   ASSERT (IdtTable != NULL);
   CopyMem (IdtTable, (VOID *)IdtDescriptor.Base, sizeof (IA32_IDT_GATE_DESCRIPTOR) * IdtEntryCount);
 
@@ -138,7 +138,7 @@ InitializeCpuInterruptHandlers (
   ASSERT (TemplateMap.ExceptionStubHeaderSize <= HOOKAFTER_STUB_SIZE);
 
   Status = gBS->AllocatePool (
-                  EfiBootServicesCode,
+                  EfiRuntimeServicesCode,
                   TemplateMap.ExceptionStubHeaderSize * CPU_INTERRUPT_NUM,
                   (VOID **)&InterruptEntryCode
                   );
-- 
2.20.1

[RFC PATCH 11/14] OvmfPkg/AmdSev: Build page table for migration handler

[Tobin Feldman-Fitzthum]

From: Dov Murik <dovmurik@linux.ibm.com>

The migration handler builds its own page tables and switches
to them. The MH pagetables are reserved as runtime memory.

When the hypervisor asks the MH to import/export a page, the HV
writes the guest physical address of the page in question to the
mailbox. The MH uses an identity mapping so that it can read/write
whatever GPA is requested by the HV. The hypervisor only asks the
MH to import/export encrypted pages. Thus, the C-Bit can be set
for every page in the identity map.

The MH also needs to read shared pages, such as the mailbox.
These are mapped at an offset. The offset must be added to
the physical address before it can be resolved.

Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
Signed-off-by: Dov Murik <dovmurik@linux.vnet.ibm.com>
---
 .../ConfidentialMigrationDxe.inf              |   1 +
 .../ConfidentialMigration/VirtualMemory.h     | 177 ++++++++++++++++++
 .../ConfidentialMigrationDxe.c                |  88 ++++++++-
 3 files changed, 265 insertions(+), 1 deletion(-)
 create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h

diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
index 49457d5d17..8dadfd1d13 100644
--- a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
@@ -15,6 +15,7 @@
 
 [Sources]
   ConfidentialMigrationDxe.c
+  VirtualMemory.h
 
 [Packages]
   MdePkg/MdePkg.dec
diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h b/OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h
new file mode 100644
index 0000000000..c50cb64c63
--- /dev/null
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h
@@ -0,0 +1,177 @@
+/** @file
+  Virtual Memory Management Services to set or clear the memory encryption bit
+  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+  Code is derived from OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
+
+**/
+
+#ifndef __VIRTUAL_MEMORY__
+#define __VIRTUAL_MEMORY__
+
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/CacheMaintenanceLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Uefi.h>
+
+#define SYS_CODE64_SEL 0x38
+
+#pragma pack(1)
+
+//
+// Page-Map Level-4 Offset (PML4) and
+// Page-Directory-Pointer Offset (PDPE) entries 4K & 2MB
+//
+
+typedef union {
+  struct {
+    UINT64  Present:1;                // 0 = Not present in memory,
+                                      //   1 = Present in memory
+    UINT64  ReadWrite:1;              // 0 = Read-Only, 1= Read/Write
+    UINT64  UserSupervisor:1;         // 0 = Supervisor, 1=User
+    UINT64  WriteThrough:1;           // 0 = Write-Back caching,
+                                      //   1 = Write-Through caching
+    UINT64  CacheDisabled:1;          // 0 = Cached, 1=Non-Cached
+    UINT64  Accessed:1;               // 0 = Not accessed,
+                                      //   1 = Accessed (set by CPU)
+    UINT64  Reserved:1;               // Reserved
+    UINT64  MustBeZero:2;             // Must Be Zero
+    UINT64  Available:3;              // Available for use by system software
+    UINT64  PageTableBaseAddress:40;  // Page Table Base Address
+    UINT64  AvabilableHigh:11;        // Available for use by system software
+    UINT64  Nx:1;                     // No Execute bit
+  } Bits;
+  UINT64    Uint64;
+} PAGE_MAP_AND_DIRECTORY_POINTER;
+
+//
+// Page Table Entry 4KB
+//
+typedef union {
+  struct {
+    UINT64  Present:1;                // 0 = Not present in memory,
+                                      //   1 = Present in memory
+    UINT64  ReadWrite:1;              // 0 = Read-Only, 1= Read/Write
+    UINT64  UserSupervisor:1;         // 0 = Supervisor, 1=User
+    UINT64  WriteThrough:1;           // 0 = Write-Back caching,
+                                      //   1 = Write-Through caching
+    UINT64  CacheDisabled:1;          // 0 = Cached, 1=Non-Cached
+    UINT64  Accessed:1;               // 0 = Not accessed,
+                                      //   1 = Accessed (set by CPU)
+    UINT64  Dirty:1;                  // 0 = Not Dirty, 1 = written by
+                                      //   processor on access to page
+    UINT64  PAT:1;                    //
+    UINT64  Global:1;                 // 0 = Not global page, 1 = global page
+                                      //   TLB not cleared on CR3 write
+    UINT64  Available:3;              // Available for use by system software
+    UINT64  PageTableBaseAddress:40;  // Page Table Base Address
+    UINT64  AvabilableHigh:11;        // Available for use by system software
+    UINT64  Nx:1;                     // 0 = Execute Code,
+                                      //   1 = No Code Execution
+  } Bits;
+  UINT64    Uint64;
+} PAGE_TABLE_4K_ENTRY;
+
+//
+// Page Table Entry 2MB
+//
+typedef union {
+  struct {
+    UINT64  Present:1;                // 0 = Not present in memory,
+                                      //   1 = Present in memory
+    UINT64  ReadWrite:1;              // 0 = Read-Only, 1= Read/Write
+    UINT64  UserSupervisor:1;         // 0 = Supervisor, 1=User
+    UINT64  WriteThrough:1;           // 0 = Write-Back caching,
+                                      //   1=Write-Through caching
+    UINT64  CacheDisabled:1;          // 0 = Cached, 1=Non-Cached
+    UINT64  Accessed:1;               // 0 = Not accessed,
+                                      //   1 = Accessed (set by CPU)
+    UINT64  Dirty:1;                  // 0 = Not Dirty, 1 = written by
+                                      //   processor on access to page
+    UINT64  MustBe1:1;                // Must be 1
+    UINT64  Global:1;                 // 0 = Not global page, 1 = global page
+                                      //   TLB not cleared on CR3 write
+    UINT64  Available:3;              // Available for use by system software
+    UINT64  PAT:1;                    //
+    UINT64  MustBeZero:8;             // Must be zero;
+    UINT64  PageTableBaseAddress:31;  // Page Table Base Address
+    UINT64  AvabilableHigh:11;        // Available for use by system software
+    UINT64  Nx:1;                     // 0 = Execute Code,
+                                      //   1 = No Code Execution
+  } Bits;
+  UINT64    Uint64;
+} PAGE_TABLE_ENTRY;
+
+//
+// Page Table Entry 1GB
+//
+typedef union {
+  struct {
+    UINT64  Present:1;                // 0 = Not present in memory,
+                                      //   1 = Present in memory
+    UINT64  ReadWrite:1;              // 0 = Read-Only, 1= Read/Write
+    UINT64  UserSupervisor:1;         // 0 = Supervisor, 1=User
+    UINT64  WriteThrough:1;           // 0 = Write-Back caching,
+                                      //   1 = Write-Through caching
+    UINT64  CacheDisabled:1;          // 0 = Cached, 1=Non-Cached
+    UINT64  Accessed:1;               // 0 = Not accessed,
+                                      //   1 = Accessed (set by CPU)
+    UINT64  Dirty:1;                  // 0 = Not Dirty, 1 = written by
+                                      //   processor on access to page
+    UINT64  MustBe1:1;                // Must be 1
+    UINT64  Global:1;                 // 0 = Not global page, 1 = global page
+                                      //   TLB not cleared on CR3 write
+    UINT64  Available:3;              // Available for use by system software
+    UINT64  PAT:1;                    //
+    UINT64  MustBeZero:17;            // Must be zero;
+    UINT64  PageTableBaseAddress:22;  // Page Table Base Address
+    UINT64  AvabilableHigh:11;        // Available for use by system software
+    UINT64  Nx:1;                     // 0 = Execute Code,
+                                      //   1 = No Code Execution
+  } Bits;
+  UINT64    Uint64;
+} PAGE_TABLE_1G_ENTRY;
+
+#pragma pack()
+
+#define IA32_PG_P                   BIT0
+#define IA32_PG_RW                  BIT1
+#define IA32_PG_PS                  BIT7
+
+#define PAGING_PAE_INDEX_MASK       0x1FF
+
+#define PAGING_4K_ADDRESS_MASK_64   0x000FFFFFFFFFF000ull
+#define PAGING_2M_ADDRESS_MASK_64   0x000FFFFFFFE00000ull
+#define PAGING_1G_ADDRESS_MASK_64   0x000FFFFFC0000000ull
+
+#define PAGING_L1_ADDRESS_SHIFT     12
+#define PAGING_L2_ADDRESS_SHIFT     21
+#define PAGING_L3_ADDRESS_SHIFT     30
+#define PAGING_L4_ADDRESS_SHIFT     39
+
+#define PAGING_PML4E_NUMBER         4
+
+#define PAGETABLE_ENTRY_MASK        ((1UL << 9) - 1)
+#define PML4_OFFSET(x)              ( (x >> 39) & PAGETABLE_ENTRY_MASK)
+#define PDP_OFFSET(x)               ( (x >> 30) & PAGETABLE_ENTRY_MASK)
+#define PDE_OFFSET(x)               ( (x >> 21) & PAGETABLE_ENTRY_MASK)
+#define PTE_OFFSET(x)               ( (x >> 12) & PAGETABLE_ENTRY_MASK)
+#define PAGING_1G_ADDRESS_MASK_64   0x000FFFFFC0000000ull
+
+#define PAGE_TABLE_POOL_ALIGNMENT   BASE_2MB
+#define PAGE_TABLE_POOL_UNIT_SIZE   SIZE_2MB
+#define PAGE_TABLE_POOL_UNIT_PAGES  \
+  EFI_SIZE_TO_PAGES (PAGE_TABLE_POOL_UNIT_SIZE)
+#define PAGE_TABLE_POOL_ALIGN_MASK  \
+  (~(EFI_PHYSICAL_ADDRESS)(PAGE_TABLE_POOL_ALIGNMENT - 1))
+
+typedef struct {
+  VOID            *NextPool;
+  UINTN           Offset;
+  UINTN           FreePages;
+} PAGE_TABLE_POOL;
+
+#endif
diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
index 8402fcc4fa..3df3b09732 100644
--- a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
@@ -11,6 +11,7 @@
 #include <Protocol/MpService.h>
 #include <Library/BaseMemoryLib.h>
 
+#include "VirtualMemory.h"
 //
 // Functions implemented by the migration handler
 //
@@ -43,6 +44,83 @@ typedef volatile struct {
   UINT32       done;
 } MH_COMMAND_PARAMETERS;
 
+//
+// Addresses for MH page table.
+//
+STATIC PAGE_TABLE_POOL   *mPageTablePool = NULL;
+STATIC PHYSICAL_ADDRESS  mMigrationHelperPageTables = 0;
+
+//
+// Offset for non-cbit mapping.
+//
+#define UNENC_VIRT_ADDR_BASE    0xffffff8000000000ULL
+
+
+/**
+  Allocates and fills in custom page tables for Migration Handler.
+  The MH must be able to write to any encrypted page. Thus, it
+  uses an identity map where the C-bit is set for every page. The
+  HV should never ask the MH to import/export a shared page. The
+  MH must also be able to read some shared pages. The first 1GB
+  of memory is mapped at offset UNENC_VIRT_ADDR_BASE.
+
+**/
+VOID
+PrepareMigrationHandlerPageTables (
+  VOID
+  )
+{
+  UINTN                            PoolPages;
+  VOID                             *Buffer;
+  VOID                             *Start;
+  PAGE_MAP_AND_DIRECTORY_POINTER   *PageMapLevel4Entry;
+  PAGE_TABLE_1G_ENTRY              *PageDirectory1GEntry;
+  PAGE_TABLE_1G_ENTRY              *Unenc1GEntry;
+  UINT64                           AddressEncMask;
+
+  PoolPages = 1 + 10;
+  Buffer = AllocateAlignedRuntimePages (PoolPages, PAGE_TABLE_POOL_ALIGNMENT);
+  mPageTablePool = Buffer;
+  mPageTablePool->NextPool = mPageTablePool;
+  mPageTablePool->FreePages  = PoolPages - 1;
+  mPageTablePool->Offset = EFI_PAGES_TO_SIZE (1);
+
+  Start = (UINT8 *)mPageTablePool + mPageTablePool->Offset;
+  ZeroMem(Start, mPageTablePool->FreePages * EFI_PAGE_SIZE);
+
+  AddressEncMask = 1ULL << 47;
+
+  PageMapLevel4Entry = Start;
+  PageDirectory1GEntry = (PAGE_TABLE_1G_ENTRY*)((UINT8*)Start + EFI_PAGE_SIZE);
+  Unenc1GEntry = (PAGE_TABLE_1G_ENTRY*)((UINT8*)Start + 2 * EFI_PAGE_SIZE);
+
+  PageMapLevel4Entry = Start;
+  PageMapLevel4Entry += PML4_OFFSET(0x0ULL);
+  PageMapLevel4Entry->Uint64 = (UINT64)PageDirectory1GEntry | AddressEncMask | 0x23;
+
+  PageMapLevel4Entry = Start;
+  PageMapLevel4Entry += PML4_OFFSET(UNENC_VIRT_ADDR_BASE); // should be 511
+  PageMapLevel4Entry->Uint64 = (UINT64)Unenc1GEntry | AddressEncMask | 0x23;
+
+  UINT64 PageAddr = 0;
+  for (int i = 0; i < 512; i++, PageAddr += SIZE_1GB) {
+    PAGE_TABLE_1G_ENTRY *e = PageDirectory1GEntry + i;
+    e->Uint64 = PageAddr | AddressEncMask | 0xe3; // 1GB page
+  }
+
+  UINT64 UnencPageAddr = 0;
+  Unenc1GEntry->Uint64 = UnencPageAddr | 0xe3; // 1GB page unencrypted
+
+  mMigrationHelperPageTables = (UINT64)Start | AddressEncMask;
+}
+
+VOID
+SwitchToMigrationHelperPageTables(VOID)
+{
+  AsmWriteCr3(mMigrationHelperPageTables);
+}
+
+
 
 VOID
 EFIAPI
@@ -56,7 +134,12 @@ MigrationHandlerMain (
 
   DebugPrint (DEBUG_INFO,"MIGRATION Handler Started\n");
 
-  params_base = PcdGet32 (PcdConfidentialMigrationMailboxBase);
+  SwitchToMigrationHelperPageTables();
+
+  //
+  // Shared pages must be offset by UNENC_VIRT_ADDR_BASE.
+  //
+  params_base = PcdGet32 (PcdConfidentialMigrationMailboxBase) + UNENC_VIRT_ADDR_BASE;
   params = (VOID *)params_base;
   page_va = (VOID *)params_base + 0x1000;
 
@@ -134,6 +217,8 @@ LaunchMigrationHandler (
 
   MigrationHandlerCpuIndex = NumProc - 1;
 
+  PrepareMigrationHandlerPageTables();
+
   EFI_EVENT Event;
   MpProto->GetProcessorInfo (MpProto, MigrationHandlerCpuIndex, &Tcb);
   if (Tcb.StatusFlag != 7) {
@@ -154,6 +239,7 @@ LaunchMigrationHandler (
   if (PcdGetBool(PcdIsConfidentialMigrationTarget)) {
     DebugPrint (DEBUG_INFO,"Waiting for incoming confidential migration.\n");
     DisableInterrupts ();
+    SwitchToMigrationHelperPageTables();
     CpuDeadLoop ();
   }
 
-- 
2.20.1

[RFC PATCH 12/14] OvmfPkg/AmdSev: Don't overwrite mailbox or pagetables

[Tobin Feldman-Fitzthum]

While restoring pages, the MH should avoid overwriting its
pagetables or the mailbox it uses to communicate with the HV.

Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
---
 .../ConfidentialMigrationDxe.c                | 22 +++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
index 3df3b09732..f609e16f8d 100644
--- a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
@@ -128,6 +128,10 @@ MigrationHandlerMain (
   IN OUT VOID *Buffer
   )
 {
+  UINT64                       mailbox_start;
+  UINT64                       mailbox_end;
+  UINT64                       pagetable_start;
+  UINT64                       pagetable_end;
   UINT64                       params_base;
   MH_COMMAND_PARAMETERS        *params;
   VOID                         *page_va;
@@ -139,10 +143,16 @@ MigrationHandlerMain (
   //
   // Shared pages must be offset by UNENC_VIRT_ADDR_BASE.
   //
-  params_base = PcdGet32 (PcdConfidentialMigrationMailboxBase) + UNENC_VIRT_ADDR_BASE;
+  mailbox_start = PcdGet32 (PcdConfidentialMigrationMailboxBase);
+  params_base = mailbox_start + UNENC_VIRT_ADDR_BASE;
   params = (VOID *)params_base;
   page_va = (VOID *)params_base + 0x1000;
 
+  mailbox_end = mailbox_start + 2 * EFI_PAGE_SIZE;
+
+  pagetable_start = mMigrationHelperPageTables;
+  pagetable_end = pagetable_start + 11 * EFI_PAGE_SIZE;
+
   DisableInterrupts();
   params->go = 0;
 
@@ -163,7 +173,15 @@ MigrationHandlerMain (
       break;
 
     case MH_FUNC_RESTORE_PAGE:
-      CopyMem((VOID *)params->gpa, page_va, 4096);
+      //
+      // Don't import a page that covers the mailbox or pagetables.
+      //
+      if ((params->gpa >= mailbox_start && params->gpa < mailbox_end) ||
+          (params->gpa >= pagetable_start && params->gpa < pagetable_end)) {
+      }
+      else {
+        CopyMem((VOID *)params->gpa, page_va, 4096);
+      }
       params->ret = MH_SUCCESS;
       break;
 
-- 
2.20.1

[RFC PATCH 13/14] OvmfPkg/AmdSev: Don't overwrite MH stack

[Tobin Feldman-Fitzthum]

When restoring pages, the Migration Handler shoudl avoid overwriting
its own stack.

Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
---
 .../ConfidentialMigrationDxe.inf              |   2 +
 OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h  | 235 ++++++++++++++++++
 .../ConfidentialMigrationDxe.c                |  30 ++-
 3 files changed, 266 insertions(+), 1 deletion(-)
 create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h

diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
index 8dadfd1d13..2816952863 100644
--- a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
@@ -16,6 +16,7 @@
 [Sources]
   ConfidentialMigrationDxe.c
   VirtualMemory.h
+  MpLib.h
 
 [Packages]
   MdePkg/MdePkg.dec
@@ -36,6 +37,7 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdIsConfidentialMigrationTarget
   gUefiOvmfPkgTokenSpaceGuid.PcdStartConfidentialMigrationHandler
   gUefiOvmfPkgTokenSpaceGuid.PcdConfidentialMigrationMailboxBase
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize
 
 [Depex]
   gEfiMpServiceProtocolGuid
diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h b/OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h
new file mode 100644
index 0000000000..5007e25243
--- /dev/null
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h
@@ -0,0 +1,235 @@
+/** @file
+  Common header file for MP Initialize Library.
+  -- adapted from UefiCpuPkg/Library/MpInitLib/MpLib.h
+  Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef _MP_LIB_H_
+#define _MP_LIB_H_
+
+#include <PiPei.h>
+
+#include <Library/MpInitLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/CpuLib.h>
+#include <Library/HobLib.h>
+#include <Library/SynchronizationLib.h>
+#include <Library/MtrrLib.h>
+
+#define CPU_INIT_MP_LIB_HOB_GUID \
+  { \
+    0x58eb6a19, 0x3699, 0x4c68, { 0xa8, 0x36, 0xda, 0xcd, 0x8e, 0xdc, 0xad, 0x4a } \
+  }
+
+
+//
+// CPU exchange information for switch BSP
+//
+typedef struct {
+  UINT8             State;        // offset 0
+  UINTN             StackPointer; // offset 4 / 8
+  IA32_DESCRIPTOR   Gdtr;         // offset 8 / 16
+  IA32_DESCRIPTOR   Idtr;         // offset 14 / 26
+} CPU_EXCHANGE_ROLE_INFO;
+
+//
+// AP initialization state during APs wakeup
+//
+typedef enum {
+  ApInitConfig   = 1,
+  ApInitReconfig = 2,
+  ApInitDone     = 3
+} AP_INIT_STATE;
+
+//
+// AP state
+//
+// The state transitions for an AP when it process a procedure are:
+//  Idle ----> Ready ----> Busy ----> Idle
+//       [BSP]       [AP]       [AP]
+//
+typedef enum {
+  CpuStateIdle,
+  CpuStateReady,
+  CpuStateBusy,
+  CpuStateFinished,
+  CpuStateDisabled
+} CPU_STATE;
+
+//
+// CPU volatile registers around INIT-SIPI-SIPI
+//
+typedef struct {
+  UINTN                          Cr0;
+  UINTN                          Cr3;
+  UINTN                          Cr4;
+  UINTN                          Dr0;
+  UINTN                          Dr1;
+  UINTN                          Dr2;
+  UINTN                          Dr3;
+  UINTN                          Dr6;
+  UINTN                          Dr7;
+  IA32_DESCRIPTOR                Gdtr;
+  IA32_DESCRIPTOR                Idtr;
+  UINT16                         Tr;
+} CPU_VOLATILE_REGISTERS;
+
+//
+// AP related data
+//
+typedef struct {
+  SPIN_LOCK                      ApLock;
+  volatile UINT32                *StartupApSignal;
+  volatile UINTN                 ApFunction;
+  volatile UINTN                 ApFunctionArgument;
+  BOOLEAN                        CpuHealthy;
+  volatile CPU_STATE             State;
+  CPU_VOLATILE_REGISTERS         VolatileRegisters;
+  BOOLEAN                        Waiting;
+  BOOLEAN                        *Finished;
+  UINT64                         ExpectedTime;
+  UINT64                         CurrentTime;
+  UINT64                         TotalTime;
+  EFI_EVENT                      WaitEvent;
+  UINT32                         ProcessorSignature;
+  UINT8                          PlatformId;
+  UINT64                         MicrocodeEntryAddr;
+} CPU_AP_DATA;
+
+//
+// Basic CPU information saved in Guided HOB.
+// Because the contents will be shard between PEI and DXE,
+// we need to make sure the each fields offset same in different
+// architecture.
+//
+#pragma pack (1)
+typedef struct {
+  UINT32                         InitialApicId;
+  UINT32                         ApicId;
+  UINT32                         Health;
+  UINT64                         ApTopOfStack;
+} CPU_INFO_IN_HOB;
+#pragma pack ()
+
+//
+// AP reset code information including code address and size,
+// this structure will be shared be C code and assembly code.
+// It is natural aligned by design.
+//
+typedef struct {
+  UINT8             *RendezvousFunnelAddress;
+  UINTN             ModeEntryOffset;
+  UINTN             RendezvousFunnelSize;
+  UINT8             *RelocateApLoopFuncAddress;
+  UINTN             RelocateApLoopFuncSize;
+  UINTN             ModeTransitionOffset;
+} MP_ASSEMBLY_ADDRESS_MAP;
+
+typedef struct _CPU_MP_DATA  CPU_MP_DATA;
+
+#pragma pack(1)
+
+//
+// MP CPU exchange information for AP reset code
+// This structure is required to be packed because fixed field offsets
+// into this structure are used in assembly code in this module
+//
+typedef struct {
+  UINTN                 Lock;
+  UINTN                 StackStart;
+  UINTN                 StackSize;
+  UINTN                 CFunction;
+  IA32_DESCRIPTOR       GdtrProfile;
+  IA32_DESCRIPTOR       IdtrProfile;
+  UINTN                 BufferStart;
+  UINTN                 ModeOffset;
+  UINTN                 ApIndex;
+  UINTN                 CodeSegment;
+  UINTN                 DataSegment;
+  UINTN                 EnableExecuteDisable;
+  UINTN                 Cr3;
+  UINTN                 InitFlag;
+  CPU_INFO_IN_HOB       *CpuInfo;
+  UINTN                 NumApsExecuting;
+  CPU_MP_DATA           *CpuMpData;
+  UINTN                 InitializeFloatingPointUnitsAddress;
+  UINT32                ModeTransitionMemory;
+  UINT16                ModeTransitionSegment;
+  UINT32                ModeHighMemory;
+  UINT16                ModeHighSegment;
+  //
+  // Enable5LevelPaging indicates whether 5-level paging is enabled in long mode.
+  //
+  BOOLEAN               Enable5LevelPaging;
+} MP_CPU_EXCHANGE_INFO;
+
+#pragma pack()
+
+//
+// CPU MP Data save in memory
+//
+struct _CPU_MP_DATA {
+  UINT64                         CpuInfoInHob;
+  UINT32                         CpuCount;
+  UINT32                         BspNumber;
+  //
+  // The above fields data will be passed from PEI to DXE
+  // Please make sure the fields offset same in the different
+  // architecture.
+  //
+  SPIN_LOCK                      MpLock;
+  UINTN                          Buffer;
+  UINTN                          CpuApStackSize;
+  MP_ASSEMBLY_ADDRESS_MAP        AddressMap;
+  UINTN                          WakeupBuffer;
+  UINTN                          WakeupBufferHigh;
+  UINTN                          BackupBuffer;
+  UINTN                          BackupBufferSize;
+
+  volatile UINT32                FinishedCount;
+  UINT32                         RunningCount;
+  BOOLEAN                        SingleThread;
+  EFI_AP_PROCEDURE               Procedure;
+  VOID                           *ProcArguments;
+  BOOLEAN                        *Finished;
+  UINT64                         ExpectedTime;
+  UINT64                         CurrentTime;
+  UINT64                         TotalTime;
+  EFI_EVENT                      WaitEvent;
+  UINTN                          **FailedCpuList;
+
+  AP_INIT_STATE                  InitFlag;
+  BOOLEAN                        SwitchBspFlag;
+  UINTN                          NewBspNumber;
+  CPU_EXCHANGE_ROLE_INFO         BSPInfo;
+  CPU_EXCHANGE_ROLE_INFO         APInfo;
+  MTRR_SETTINGS                  MtrrTable;
+  UINT8                          ApLoopMode;
+  UINT8                          ApTargetCState;
+  UINT16                         PmCodeSegment;
+  CPU_AP_DATA                    *CpuData;
+  volatile MP_CPU_EXCHANGE_INFO  *MpCpuExchangeInfo;
+
+  UINT32                         CurrentTimerCount;
+  UINTN                          DivideValue;
+  UINT8                          Vector;
+  BOOLEAN                        PeriodicMode;
+  BOOLEAN                        TimerInterruptState;
+  UINT64                         MicrocodePatchAddress;
+  UINT64                         MicrocodePatchRegionSize;
+
+  //
+  // Whether need to use Init-Sipi-Sipi to wake up the APs.
+  // Two cases need to set this value to TRUE. One is in HLT
+  // loop mode, the other is resume from S3 which loop mode
+  // will be hardcode change to HLT mode by PiSmmCpuDxeSmm
+  // driver.
+  //
+  BOOLEAN                        WakeUpByInitSipiSipi;
+};
+
+extern EFI_GUID mCpuInitMpLibHobGuid;
+
+#endif
diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
index f609e16f8d..42b99be552 100644
--- a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
@@ -12,6 +12,8 @@
 #include <Library/BaseMemoryLib.h>
 
 #include "VirtualMemory.h"
+#include "MpLib.h"
+
 //
 // Functions implemented by the migration handler
 //
@@ -114,6 +116,7 @@ PrepareMigrationHandlerPageTables (
   mMigrationHelperPageTables = (UINT64)Start | AddressEncMask;
 }
 
+
 VOID
 SwitchToMigrationHelperPageTables(VOID)
 {
@@ -121,6 +124,25 @@ SwitchToMigrationHelperPageTables(VOID)
 }
 
 
+UINT64
+GetMHTopOfStack()
+{
+  EFI_HOB_GUID_TYPE       *GuidHob;
+  VOID                    *DataInHob;
+  CPU_MP_DATA             *CpuMpData;
+  CPU_INFO_IN_HOB         *CpuInfoInHob;
+
+  GuidHob = GetFirstGuidHob (&mCpuInitMpLibHobGuid);
+  ASSERT(GuidHob != NULL);
+
+  DataInHob = GET_GUID_HOB_DATA (GuidHob);
+  CpuMpData = (CPU_MP_DATA *) (*(UINTN *) DataInHob);
+  CpuInfoInHob = (CPU_INFO_IN_HOB *) (UINTN) CpuMpData->CpuInfoInHob;
+
+  return CpuInfoInHob[MigrationHandlerCpuIndex].ApTopOfStack;
+
+}
+
 
 VOID
 EFIAPI
@@ -132,6 +154,8 @@ MigrationHandlerMain (
   UINT64                       mailbox_end;
   UINT64                       pagetable_start;
   UINT64                       pagetable_end;
+  UINT64                       stack_start;
+  UINT64                       stack_end;
   UINT64                       params_base;
   MH_COMMAND_PARAMETERS        *params;
   VOID                         *page_va;
@@ -153,6 +177,9 @@ MigrationHandlerMain (
   pagetable_start = mMigrationHelperPageTables;
   pagetable_end = pagetable_start + 11 * EFI_PAGE_SIZE;
 
+  stack_end = GetMHTopOfStack();
+  stack_start = stack_end - PcdGet32(PcdCpuApStackSize);
+
   DisableInterrupts();
   params->go = 0;
 
@@ -177,7 +204,8 @@ MigrationHandlerMain (
       // Don't import a page that covers the mailbox or pagetables.
       //
       if ((params->gpa >= mailbox_start && params->gpa < mailbox_end) ||
-          (params->gpa >= pagetable_start && params->gpa < pagetable_end)) {
+          (params->gpa >= pagetable_start && params->gpa < pagetable_end) ||
+          (params->gpa >= stack_start && params->gpa < stack_end)) {
       }
       else {
         CopyMem((VOID *)params->gpa, page_va, 4096);
-- 
2.20.1

[RFC PATCH 14/14] OvmfPkg/AmdSev: MH page encryption POC

[Tobin Feldman-Fitzthum]

This code is for demonstration purposes only. It is not secure or
robust. The purpose is to show where encryption will be incorporated
and to get a sense of the performance impact of adding encryption.

We plan to use AES-GCM to encrypt the pages as a stream. This will
also allow us to verify the GPA as part of the AAD, ensuring that
a malicious hypervisor hasn't exchanged pages in-flight.

Currently the CryptoPkg and the BaseCryptLib do not expose AES-GCM,
despite it being included in OpensslLib. Thus, we use CBC here.

Key sharing is out of scope for this part of the RFC. We assume that
the source and destination MH share a key. This will probably be
implemented via inject-launch-secret in the future. For now, we
hardcode a key, but this is strictly temporary.

We have had trouble using RandomBytes() in the MH when SEV is
enabled. Thus the IV is hardcoded. Again, this is temporary.

The HV and the MH will exchange information pertaining to encryption,
like the IV, via an additional header on the shared mailbox page.

This patch does not do any safety checks or handle encrypt/decrypt
failures. Again, this is only here to show where encryption will
go and generally how the MH on the source and target can share
pages without exposing guest memory to the HV.

Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
---
 .../ConfidentialMigrationDxe.inf              |  2 ++
 .../ConfidentialMigrationDxe.c                | 36 +++++++++++++++++--
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
index 2816952863..ae074a8b07 100644
--- a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
@@ -22,6 +22,7 @@
   MdePkg/MdePkg.dec
   OvmfPkg/OvmfPkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
+  CryptoPkg/CryptoPkg.dec
 
 [LibraryClasses]
   MemoryAllocationLib
@@ -29,6 +30,7 @@
   UefiBootServicesTableLib
   MpInitLib
   UefiDriverEntryPoint
+  BaseCryptLib
 
 [Protocols]
   gEfiMpServiceProtocolGuid
diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
index 42b99be552..a9cb490561 100644
--- a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
@@ -10,6 +10,7 @@
 #include <Library/DebugLib.h>
 #include <Protocol/MpService.h>
 #include <Library/BaseMemoryLib.h>
+#include <Library/BaseCryptLib.h>
 
 #include "VirtualMemory.h"
 #include "MpLib.h"
@@ -46,6 +47,14 @@ typedef volatile struct {
   UINT32       done;
 } MH_COMMAND_PARAMETERS;
 
+//
+// Additional header for encryption support.
+//
+struct page_hdr {
+  UINT8        IV[16];
+  UINT8        tag[16];
+};
+
 //
 // Addresses for MH page table.
 //
@@ -57,6 +66,20 @@ STATIC PHYSICAL_ADDRESS  mMigrationHelperPageTables = 0;
 //
 #define UNENC_VIRT_ADDR_BASE    0xffffff8000000000ULL
 
+//
+// Key shared between source and target MH (temporary)
+//
+GLOBAL_REMOVE_IF_UNREFERENCED CONST UINT8 cipher_key[] = {
+  0xc2, 0x86, 0x69, 0x6d, 0x88, 0x7c, 0x9a, 0xa0, 0x61, 0x1b, 0xbb, 0x3e, 0x20, 0x25, 0xa4, 0x5a
+ };
+
+//
+// IV for CBC cipher (temporary). We are having trouble with
+// calling RandomBytes from inside the Migration Handler
+//
+GLOBAL_REMOVE_IF_UNREFERENCED UINT8 Ivec[] = {
+  0xfe, 0xdc, 0xba, 0x98, 0x76, 0x54, 0x32, 0x10
+};
 
 /**
   Allocates and fills in custom page tables for Migration Handler.
@@ -159,6 +182,8 @@ MigrationHandlerMain (
   UINT64                       params_base;
   MH_COMMAND_PARAMETERS        *params;
   VOID                         *page_va;
+  VOID                         *cipher_ctx;
+  INTN                         ctx_size;
 
   DebugPrint (DEBUG_INFO,"MIGRATION Handler Started\n");
 
@@ -171,6 +196,7 @@ MigrationHandlerMain (
   params_base = mailbox_start + UNENC_VIRT_ADDR_BASE;
   params = (VOID *)params_base;
   page_va = (VOID *)params_base + 0x1000;
+  //struct page_hdr *hdr_va = (void *) params_base + 0x800;
 
   mailbox_end = mailbox_start + 2 * EFI_PAGE_SIZE;
 
@@ -180,6 +206,9 @@ MigrationHandlerMain (
   stack_end = GetMHTopOfStack();
   stack_start = stack_end - PcdGet32(PcdCpuApStackSize);
 
+  ctx_size = AesGetContextSize ();
+  cipher_ctx = AllocateRuntimePool (ctx_size);
+
   DisableInterrupts();
   params->go = 0;
 
@@ -195,7 +224,9 @@ MigrationHandlerMain (
       break;
 
     case MH_FUNC_SAVE_PAGE:
-      CopyMem(page_va, (VOID *)params->gpa, 4096);
+      AesInit (cipher_ctx, cipher_key, 128);
+      AesCbcEncrypt(cipher_ctx, (VOID *)params->gpa, 4096, Ivec, page_va);
+
       params->ret = MH_SUCCESS;
       break;
 
@@ -208,7 +239,8 @@ MigrationHandlerMain (
           (params->gpa >= stack_start && params->gpa < stack_end)) {
       }
       else {
-        CopyMem((VOID *)params->gpa, page_va, 4096);
+        AesInit (cipher_ctx, cipher_key, 128);
+        AesCbcDecrypt (cipher_ctx, page_va, 4096, Ivec, (VOID *)params->gpa);
       }
       params->ret = MH_SUCCESS;
       break;
-- 
2.20.1

Re: [RFC PATCH 02/14] OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap.

[Ashish Kalra]

Hello Tobin,

Just a high level question, why is this patch included in this
patch series, i don't think you are supporting SEV-ES platform 
migration in this patch-set ?

Thanks,
Ashish

On Tue, Mar 02, 2021 at 03:48:27PM -0500, Tobin Feldman-Fitzthum 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 dddffdebda..c72eeb37c5 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/Msr.h>
> @@ -52,6 +53,15 @@ AmdSevEsInitialize (
>    PcdStatus = PcdSetBoolS (PcdSevEsIsEnabled, TRUE);
>    ASSERT_RETURN_ERROR (PcdStatus);
>  
> +  //
> +  // 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
> +    );
> +
>    //
>    // Allocate GHCB and per-CPU variable pages.
>    //   Since the pages must survive across the UEFI to OS transition
> -- 
> 2.20.1
> 

Re: [edk2-devel] [RFC PATCH 02/14] OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap.

[Tobin Feldman-Fitzthum]

> Hello Tobin,
>
> Just a high level question, why is this patch included in this
> patch series, i don't think you are supporting SEV-ES platform
> migration in this patch-set ?

You are correct that we don't support migration for SEV-ES machines, 
although our approach can potentially be adapted for SEV-ES. I was on 
the fence about including this patch, because we don't strictly need it 
for migration. I'm not sure if the SEC GHCB would be significant even if 
we did support SEV-ES migration. Ultimately it seemed like a good idea 
because the SEV firmware build does otherwise support SEV-ES. Since I 
was introducing the hypercall in an environment where SEV-ES can be 
enabled, it seemed reasonable to include. Syncing page encryption status 
hypothetically has uses beyond migration.

Note that I am not adding full support for the hypercall in OVMF, which 
might be a good idea at some point.

-Tobin

> Thanks,
> Ashish
>
> On Tue, Mar 02, 2021 at 03:48:27PM -0500, Tobin Feldman-Fitzthum 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 dddffdebda..c72eeb37c5 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/Msr.h>
>> @@ -52,6 +53,15 @@ AmdSevEsInitialize (
>>     PcdStatus = PcdSetBoolS (PcdSevEsIsEnabled, TRUE);
>>     ASSERT_RETURN_ERROR (PcdStatus);
>>   
>> +  //
>> +  // 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
>> +    );
>> +
>>     //
>>     // Allocate GHCB and per-CPU variable pages.
>>     //   Since the pages must survive across the UEFI to OS transition
>> -- 
>> 2.20.1
>>
>
> 
>
>

Re: [edk2-devel] [RFC PATCH 02/14] OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap.

[Ashish Kalra]

On Wed, Mar 03, 2021 at 09:56:00AM -0500, Tobin Feldman-Fitzthum wrote:
> 
> > Hello Tobin,
> > 
> > Just a high level question, why is this patch included in this
> > patch series, i don't think you are supporting SEV-ES platform
> > migration in this patch-set ?
> 
> You are correct that we don't support migration for SEV-ES machines,
> although our approach can potentially be adapted for SEV-ES. I was on the
> fence about including this patch, because we don't strictly need it for
> migration. I'm not sure if the SEC GHCB would be significant even if we did
> support SEV-ES migration. Ultimately it seemed like a good idea because the
> SEV firmware build does otherwise support SEV-ES. Since I was introducing
> the hypercall in an environment where SEV-ES can be enabled, it seemed
> reasonable to include. Syncing page encryption status hypothetically has
> uses beyond migration.
> 
> Note that I am not adding full support for the hypercall in OVMF, which
> might be a good idea at some point.
> 

Yes, i don't see the assembler code stub for the hypercall in the OVMF 
patches, so i was wondering the same.

Thanks,
Ashish

Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live Migration for AMD SEV

[Laszlo Ersek]

Hi Tobin,

On 03/02/21 21:48, Tobin Feldman-Fitzthum wrote:
> This is a demonstration of fast migration for encrypted virtual machines
> using a Migration Handler that lives in OVMF. This demo uses AMD SEV,
> but the ideas may generalize to other confidential computing platforms.
> With AMD SEV, guest memory is encrypted and the hypervisor cannot access
> or move it. This makes migration tricky. In this demo, we show how the
> HV can ask a Migration Handler (MH) in the firmware for an encrypted
> page. The MH encrypts the page with a transport key prior to releasing
> it to the HV. The target machine also runs an MH that decrypts the page
> once it is passed in by the target HV. These patches are not ready for
> production, but the are a full end-to-end solution that facilitates a
> fast live migration between two SEV VMs.
> 
> Corresponding patches for QEMU have been posted my colleague Dov Murik
> on qemu-devel. Our approach needs little kernel support, requiring only
> one hypercall that the guest can use to mark a page as encrypted or
> shared. This series includes updated patches from Ashish Kalra and
> Brijesh Singh that allow OVMF to use this hypercall. 
> 
> The MH runs continuously in the guest, waiting for communication from
> the HV. The HV starts an additional vCPU for the MH but does not expose
> it to the guest OS via ACPI. We use the MpService to start the MH. The
> MpService is only available at runtime and processes that are started by
> it are usually cleaned up on ExitBootServices. Since we need the MH to
> run continuously, we had to make some modifications. Ideally a feature
> could be added to the MpService to allow for the starting of
> long-running processes. Besides migration, this could support other
> background processes that need to operate within the encryption
> boundary. For now, we have included a handful of patches that modify the
> MpService to allow the MH to keep running after ExitBootServices. These
> are temporary.

I plan to do a lightweight review for this series. (My understanding is
that it's an RFC and not actually being proposed for merging.)

Regarding the MH's availability at runtime -- does that necessarily
require the isolation of an AP? Because in the current approach,
allowing the MP Services to survive into OS runtime (in some form or
another) seems critical, and I don't think it's going to fly.

I agree that the UefiCpuPkg patches have been well separated from the
rest of the series, but I'm somewhat doubtful the "firmware-initiated
background process" idea will be accepted. Have you investigated
exposing a new "runtime service" (a function pointer) via the UEFI
Configuration table, and calling that (perhaps periodically?) from the
guest kernel? It would be a form of polling I guess. Or maybe, poll the
mailbox directly in the kernel, and call the new firmware runtime
service when there's an actual command to process.

(You do spell out "little kernel support", and I'm not sure if that's a
technical benefit, or a political / community benefit.)

I'm quite uncomfortable with an attempt to hide a CPU from the OS via
ACPI. The OS has other ways to learn (for example, a boot loader could
use the MP services itself, stash the information, and hand it to the OS
kernel -- this would minimally allow for detecting an inconsistency in
the OS). What about "all-but-self" IPIs too -- the kernel might think
all the processors it's poking like that were under its control.

Also, as far as I can tell from patch #7, the AP seems to be
busy-looping (with a CpuPause() added in), for the entire lifetime of
the OS. Do I understand right? If so -- is it a temporary trait as well?

Sorry if my questions are "premature", in the sense that I could get my
own answers as well if I actually read the patches in detail -- however,
I wouldn't like to do that at once, because then I'll be distracted by
many style issues and other "trivial" stuff. Examples for the latter:

- patch#1 calls SetMemoryEncDecHypercall3(), but there is no such
function in edk2, so minimally it's a patch ordering bug in the series,

- in patch#1, there's minimally one whitespace error (no whitespace
right after "EFI_SIZE_TO_PAGES")

- in patch#1, the alphabetical ordering in the [LibraryClasses] section,
and in the matching #include directives, gets broken,

- I'd prefer if the "SevLiveMigrationEnabled" UEFI variable were set in
ConfidentialMigrationDxe, rather than PlatformDxe (patch #3), or at
least another AMD SEV related DXE driver (OvmfPkg/AmdSevDxe etc).

- any particular reasonf or making the UEFI variable non-volatile? I
don't think it should survive any particular boot of the guest.

- Why do we need a variable in the first place?

etc etc

Thanks!
Laszlo




> 
> Ashish Kalra (2):
>   OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap.
>   OvmfPkg/PlatformDxe: Add support for SEV live migration.
> 
> Brijesh Singh (1):
>   OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall
> 
> Dov Murik (1):
>   OvmfPkg/AmdSev: Build page table for migration handler
> 
> Tobin Feldman-Fitzthum (10):
>   OvmfPkg/AmdSev: Base for Confidential Migration Handler
>   OvmfPkg/PlatfomPei: Set Confidential Migration PCD
>   OvmfPkg/AmdSev: Setup Migration Handler Mailbox
>   OvmfPkg/AmdSev: MH support for mailbox protocol
>   UefiCpuPkg/MpInitLib: temp removal of MpLib cleanup
>   UefiCpuPkg/MpInitLib: Allocate MP buffer as runtime memory
>   UefiCpuPkg/CpuExceptionHandlerLib: Exception handling as runtime
>     memory
>   OvmfPkg/AmdSev: Don't overwrite mailbox or pagetables
>   OvmfPkg/AmdSev: Don't overwrite MH stack
>   OvmfPkg/AmdSev: MH page encryption POC
> 
>  OvmfPkg/OvmfPkg.dec                           |  11 +
>  OvmfPkg/AmdSev/AmdSevX64.dsc                  |   2 +
>  OvmfPkg/AmdSev/AmdSevX64.fdf                  |  13 +-
>  .../ConfidentialMigrationDxe.inf              |  45 +++
>  .../ConfidentialMigrationPei.inf              |  35 ++
>  .../DxeMemEncryptSevLib.inf                   |   1 +
>  .../PeiMemEncryptSevLib.inf                   |   1 +
>  OvmfPkg/PlatformDxe/Platform.inf              |   2 +
>  OvmfPkg/PlatformPei/PlatformPei.inf           |   2 +
>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   2 +
>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   2 +
>  OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h  | 235 +++++++++++++
>  .../ConfidentialMigration/VirtualMemory.h     | 177 ++++++++++
>  OvmfPkg/Include/Guid/MemEncryptLib.h          |  16 +
>  OvmfPkg/PlatformDxe/PlatformConfig.h          |   5 +
>  .../ConfidentialMigrationDxe.c                | 325 ++++++++++++++++++
>  .../ConfidentialMigrationPei.c                |  25 ++
>  .../X64/PeiDxeVirtualMemory.c                 |  18 +
>  OvmfPkg/PlatformDxe/AmdSev.c                  |  99 ++++++
>  OvmfPkg/PlatformDxe/Platform.c                |   6 +
>  OvmfPkg/PlatformPei/AmdSev.c                  |  10 +
>  OvmfPkg/PlatformPei/Platform.c                |  10 +
>  .../CpuExceptionHandlerLib/DxeException.c     |   8 +-
>  UefiCpuPkg/Library/MpInitLib/DxeMpLib.c       |  21 +-
>  UefiCpuPkg/Library/MpInitLib/MpLib.c          |   7 +-
>  25 files changed, 1061 insertions(+), 17 deletions(-)
>  create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
>  create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.inf
>  create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h
>  create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h
>  create mode 100644 OvmfPkg/Include/Guid/MemEncryptLib.h
>  create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
>  create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.c
>  create mode 100644 OvmfPkg/PlatformDxe/AmdSev.c
> 

Re: [RFC PATCH 11/14] OvmfPkg/AmdSev: Build page table for migration handler

[Ashish Kalra]

On Tue, Mar 02, 2021 at 03:48:36PM -0500, Tobin Feldman-Fitzthum wrote:
> From: Dov Murik <dovmurik@linux.ibm.com>
> 
> The migration handler builds its own page tables and switches
> to them. The MH pagetables are reserved as runtime memory.
> 
> When the hypervisor asks the MH to import/export a page, the HV
> writes the guest physical address of the page in question to the
> mailbox. The MH uses an identity mapping so that it can read/write
> whatever GPA is requested by the HV. The hypervisor only asks the
> MH to import/export encrypted pages. Thus, the C-Bit can be set
> for every page in the identity map.
> 
> The MH also needs to read shared pages, such as the mailbox.
> These are mapped at an offset. The offset must be added to
> the physical address before it can be resolved.
> 
> Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
> Signed-off-by: Dov Murik <dovmurik@linux.vnet.ibm.com>
> ---
>  .../ConfidentialMigrationDxe.inf              |   1 +
>  .../ConfidentialMigration/VirtualMemory.h     | 177 ++++++++++++++++++
>  .../ConfidentialMigrationDxe.c                |  88 ++++++++-
>  3 files changed, 265 insertions(+), 1 deletion(-)
>  create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h
> 
> diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
> index 49457d5d17..8dadfd1d13 100644
> --- a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
> +++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
> @@ -15,6 +15,7 @@
>  
>  [Sources]
>    ConfidentialMigrationDxe.c
> +  VirtualMemory.h
>  
>  [Packages]
>    MdePkg/MdePkg.dec
> diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h b/OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h
> new file mode 100644
> index 0000000000..c50cb64c63
> --- /dev/null
> +++ b/OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h
> @@ -0,0 +1,177 @@
> +/** @file
> +  Virtual Memory Management Services to set or clear the memory encryption bit
> +  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +  Code is derived from OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
> +
> +**/
> +
> +#ifndef __VIRTUAL_MEMORY__
> +#define __VIRTUAL_MEMORY__
> +
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/CacheMaintenanceLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Uefi.h>
> +
> +#define SYS_CODE64_SEL 0x38
> +
> +#pragma pack(1)
> +
> +//
> +// Page-Map Level-4 Offset (PML4) and
> +// Page-Directory-Pointer Offset (PDPE) entries 4K & 2MB
> +//
> +
> +typedef union {
> +  struct {
> +    UINT64  Present:1;                // 0 = Not present in memory,
> +                                      //   1 = Present in memory
> +    UINT64  ReadWrite:1;              // 0 = Read-Only, 1= Read/Write
> +    UINT64  UserSupervisor:1;         // 0 = Supervisor, 1=User
> +    UINT64  WriteThrough:1;           // 0 = Write-Back caching,
> +                                      //   1 = Write-Through caching
> +    UINT64  CacheDisabled:1;          // 0 = Cached, 1=Non-Cached
> +    UINT64  Accessed:1;               // 0 = Not accessed,
> +                                      //   1 = Accessed (set by CPU)
> +    UINT64  Reserved:1;               // Reserved
> +    UINT64  MustBeZero:2;             // Must Be Zero
> +    UINT64  Available:3;              // Available for use by system software
> +    UINT64  PageTableBaseAddress:40;  // Page Table Base Address
> +    UINT64  AvabilableHigh:11;        // Available for use by system software
> +    UINT64  Nx:1;                     // No Execute bit
> +  } Bits;
> +  UINT64    Uint64;
> +} PAGE_MAP_AND_DIRECTORY_POINTER;
> +
> +//
> +// Page Table Entry 4KB
> +//
> +typedef union {
> +  struct {
> +    UINT64  Present:1;                // 0 = Not present in memory,
> +                                      //   1 = Present in memory
> +    UINT64  ReadWrite:1;              // 0 = Read-Only, 1= Read/Write
> +    UINT64  UserSupervisor:1;         // 0 = Supervisor, 1=User
> +    UINT64  WriteThrough:1;           // 0 = Write-Back caching,
> +                                      //   1 = Write-Through caching
> +    UINT64  CacheDisabled:1;          // 0 = Cached, 1=Non-Cached
> +    UINT64  Accessed:1;               // 0 = Not accessed,
> +                                      //   1 = Accessed (set by CPU)
> +    UINT64  Dirty:1;                  // 0 = Not Dirty, 1 = written by
> +                                      //   processor on access to page
> +    UINT64  PAT:1;                    //
> +    UINT64  Global:1;                 // 0 = Not global page, 1 = global page
> +                                      //   TLB not cleared on CR3 write
> +    UINT64  Available:3;              // Available for use by system software
> +    UINT64  PageTableBaseAddress:40;  // Page Table Base Address
> +    UINT64  AvabilableHigh:11;        // Available for use by system software
> +    UINT64  Nx:1;                     // 0 = Execute Code,
> +                                      //   1 = No Code Execution
> +  } Bits;
> +  UINT64    Uint64;
> +} PAGE_TABLE_4K_ENTRY;
> +
> +//
> +// Page Table Entry 2MB
> +//
> +typedef union {
> +  struct {
> +    UINT64  Present:1;                // 0 = Not present in memory,
> +                                      //   1 = Present in memory
> +    UINT64  ReadWrite:1;              // 0 = Read-Only, 1= Read/Write
> +    UINT64  UserSupervisor:1;         // 0 = Supervisor, 1=User
> +    UINT64  WriteThrough:1;           // 0 = Write-Back caching,
> +                                      //   1=Write-Through caching
> +    UINT64  CacheDisabled:1;          // 0 = Cached, 1=Non-Cached
> +    UINT64  Accessed:1;               // 0 = Not accessed,
> +                                      //   1 = Accessed (set by CPU)
> +    UINT64  Dirty:1;                  // 0 = Not Dirty, 1 = written by
> +                                      //   processor on access to page
> +    UINT64  MustBe1:1;                // Must be 1
> +    UINT64  Global:1;                 // 0 = Not global page, 1 = global page
> +                                      //   TLB not cleared on CR3 write
> +    UINT64  Available:3;              // Available for use by system software
> +    UINT64  PAT:1;                    //
> +    UINT64  MustBeZero:8;             // Must be zero;
> +    UINT64  PageTableBaseAddress:31;  // Page Table Base Address
> +    UINT64  AvabilableHigh:11;        // Available for use by system software
> +    UINT64  Nx:1;                     // 0 = Execute Code,
> +                                      //   1 = No Code Execution
> +  } Bits;
> +  UINT64    Uint64;
> +} PAGE_TABLE_ENTRY;
> +
> +//
> +// Page Table Entry 1GB
> +//
> +typedef union {
> +  struct {
> +    UINT64  Present:1;                // 0 = Not present in memory,
> +                                      //   1 = Present in memory
> +    UINT64  ReadWrite:1;              // 0 = Read-Only, 1= Read/Write
> +    UINT64  UserSupervisor:1;         // 0 = Supervisor, 1=User
> +    UINT64  WriteThrough:1;           // 0 = Write-Back caching,
> +                                      //   1 = Write-Through caching
> +    UINT64  CacheDisabled:1;          // 0 = Cached, 1=Non-Cached
> +    UINT64  Accessed:1;               // 0 = Not accessed,
> +                                      //   1 = Accessed (set by CPU)
> +    UINT64  Dirty:1;                  // 0 = Not Dirty, 1 = written by
> +                                      //   processor on access to page
> +    UINT64  MustBe1:1;                // Must be 1
> +    UINT64  Global:1;                 // 0 = Not global page, 1 = global page
> +                                      //   TLB not cleared on CR3 write
> +    UINT64  Available:3;              // Available for use by system software
> +    UINT64  PAT:1;                    //
> +    UINT64  MustBeZero:17;            // Must be zero;
> +    UINT64  PageTableBaseAddress:22;  // Page Table Base Address
> +    UINT64  AvabilableHigh:11;        // Available for use by system software
> +    UINT64  Nx:1;                     // 0 = Execute Code,
> +                                      //   1 = No Code Execution
> +  } Bits;
> +  UINT64    Uint64;
> +} PAGE_TABLE_1G_ENTRY;
> +
> +#pragma pack()
> +
> +#define IA32_PG_P                   BIT0
> +#define IA32_PG_RW                  BIT1
> +#define IA32_PG_PS                  BIT7
> +
> +#define PAGING_PAE_INDEX_MASK       0x1FF
> +
> +#define PAGING_4K_ADDRESS_MASK_64   0x000FFFFFFFFFF000ull
> +#define PAGING_2M_ADDRESS_MASK_64   0x000FFFFFFFE00000ull
> +#define PAGING_1G_ADDRESS_MASK_64   0x000FFFFFC0000000ull
> +
> +#define PAGING_L1_ADDRESS_SHIFT     12
> +#define PAGING_L2_ADDRESS_SHIFT     21
> +#define PAGING_L3_ADDRESS_SHIFT     30
> +#define PAGING_L4_ADDRESS_SHIFT     39
> +
> +#define PAGING_PML4E_NUMBER         4
> +
> +#define PAGETABLE_ENTRY_MASK        ((1UL << 9) - 1)
> +#define PML4_OFFSET(x)              ( (x >> 39) & PAGETABLE_ENTRY_MASK)
> +#define PDP_OFFSET(x)               ( (x >> 30) & PAGETABLE_ENTRY_MASK)
> +#define PDE_OFFSET(x)               ( (x >> 21) & PAGETABLE_ENTRY_MASK)
> +#define PTE_OFFSET(x)               ( (x >> 12) & PAGETABLE_ENTRY_MASK)
> +#define PAGING_1G_ADDRESS_MASK_64   0x000FFFFFC0000000ull
> +
> +#define PAGE_TABLE_POOL_ALIGNMENT   BASE_2MB
> +#define PAGE_TABLE_POOL_UNIT_SIZE   SIZE_2MB
> +#define PAGE_TABLE_POOL_UNIT_PAGES  \
> +  EFI_SIZE_TO_PAGES (PAGE_TABLE_POOL_UNIT_SIZE)
> +#define PAGE_TABLE_POOL_ALIGN_MASK  \
> +  (~(EFI_PHYSICAL_ADDRESS)(PAGE_TABLE_POOL_ALIGNMENT - 1))
> +
> +typedef struct {
> +  VOID            *NextPool;
> +  UINTN           Offset;
> +  UINTN           FreePages;
> +} PAGE_TABLE_POOL;
> +
> +#endif
> diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
> index 8402fcc4fa..3df3b09732 100644
> --- a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
> +++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
> @@ -11,6 +11,7 @@
>  #include <Protocol/MpService.h>
>  #include <Library/BaseMemoryLib.h>
>  
> +#include "VirtualMemory.h"
>  //
>  // Functions implemented by the migration handler
>  //
> @@ -43,6 +44,83 @@ typedef volatile struct {
>    UINT32       done;
>  } MH_COMMAND_PARAMETERS;
>  
> +//
> +// Addresses for MH page table.
> +//
> +STATIC PAGE_TABLE_POOL   *mPageTablePool = NULL;
> +STATIC PHYSICAL_ADDRESS  mMigrationHelperPageTables = 0;
> +
> +//
> +// Offset for non-cbit mapping.
> +//
> +#define UNENC_VIRT_ADDR_BASE    0xffffff8000000000ULL
> +
> +
> +/**
> +  Allocates and fills in custom page tables for Migration Handler.
> +  The MH must be able to write to any encrypted page. Thus, it
> +  uses an identity map where the C-bit is set for every page. The
> +  HV should never ask the MH to import/export a shared page. The
> +  MH must also be able to read some shared pages. The first 1GB
> +  of memory is mapped at offset UNENC_VIRT_ADDR_BASE.
> +
> +**/
> +VOID
> +PrepareMigrationHandlerPageTables (
> +  VOID
> +  )
> +{
> +  UINTN                            PoolPages;
> +  VOID                             *Buffer;
> +  VOID                             *Start;
> +  PAGE_MAP_AND_DIRECTORY_POINTER   *PageMapLevel4Entry;
> +  PAGE_TABLE_1G_ENTRY              *PageDirectory1GEntry;
> +  PAGE_TABLE_1G_ENTRY              *Unenc1GEntry;
> +  UINT64                           AddressEncMask;
> +
> +  PoolPages = 1 + 10;
> +  Buffer = AllocateAlignedRuntimePages (PoolPages, PAGE_TABLE_POOL_ALIGNMENT);
> +  mPageTablePool = Buffer;
> +  mPageTablePool->NextPool = mPageTablePool;
> +  mPageTablePool->FreePages  = PoolPages - 1;
> +  mPageTablePool->Offset = EFI_PAGES_TO_SIZE (1);
> +
> +  Start = (UINT8 *)mPageTablePool + mPageTablePool->Offset;
> +  ZeroMem(Start, mPageTablePool->FreePages * EFI_PAGE_SIZE);
> +
> +  AddressEncMask = 1ULL << 47;
> +

Preferably getting the encryption bit location from SEV CPUID
information.

> +  PageMapLevel4Entry = Start;
> +  PageDirectory1GEntry = (PAGE_TABLE_1G_ENTRY*)((UINT8*)Start + EFI_PAGE_SIZE);
> +  Unenc1GEntry = (PAGE_TABLE_1G_ENTRY*)((UINT8*)Start + 2 * EFI_PAGE_SIZE);
> +
> +  PageMapLevel4Entry = Start;
> +  PageMapLevel4Entry += PML4_OFFSET(0x0ULL);
> +  PageMapLevel4Entry->Uint64 = (UINT64)PageDirectory1GEntry | AddressEncMask | 0x23;
> +
> +  PageMapLevel4Entry = Start;
> +  PageMapLevel4Entry += PML4_OFFSET(UNENC_VIRT_ADDR_BASE); // should be 511
> +  PageMapLevel4Entry->Uint64 = (UINT64)Unenc1GEntry | AddressEncMask | 0x23;
> +
> +  UINT64 PageAddr = 0;
> +  for (int i = 0; i < 512; i++, PageAddr += SIZE_1GB) {
> +    PAGE_TABLE_1G_ENTRY *e = PageDirectory1GEntry + i;
> +    e->Uint64 = PageAddr | AddressEncMask | 0xe3; // 1GB page
> +  }
> +

Changing encryption attributes of a page requires to flush it from
the caches, you may need to do a clflush here.

Thanks,
Ashish

> +  UINT64 UnencPageAddr = 0;
> +  Unenc1GEntry->Uint64 = UnencPageAddr | 0xe3; // 1GB page unencrypted
> +
> +  mMigrationHelperPageTables = (UINT64)Start | AddressEncMask;
> +}
> +
> +VOID
> +SwitchToMigrationHelperPageTables(VOID)
> +{
> +  AsmWriteCr3(mMigrationHelperPageTables);
> +}
> +
> +
>  
>  VOID
>  EFIAPI
> @@ -56,7 +134,12 @@ MigrationHandlerMain (
>  
>    DebugPrint (DEBUG_INFO,"MIGRATION Handler Started\n");
>  
> -  params_base = PcdGet32 (PcdConfidentialMigrationMailboxBase);
> +  SwitchToMigrationHelperPageTables();
> +
> +  //
> +  // Shared pages must be offset by UNENC_VIRT_ADDR_BASE.
> +  //
> +  params_base = PcdGet32 (PcdConfidentialMigrationMailboxBase) + UNENC_VIRT_ADDR_BASE;
>    params = (VOID *)params_base;
>    page_va = (VOID *)params_base + 0x1000;
>  
> @@ -134,6 +217,8 @@ LaunchMigrationHandler (
>  
>    MigrationHandlerCpuIndex = NumProc - 1;
>  
> +  PrepareMigrationHandlerPageTables();
> +
>    EFI_EVENT Event;
>    MpProto->GetProcessorInfo (MpProto, MigrationHandlerCpuIndex, &Tcb);
>    if (Tcb.StatusFlag != 7) {
> @@ -154,6 +239,7 @@ LaunchMigrationHandler (
>    if (PcdGetBool(PcdIsConfidentialMigrationTarget)) {
>      DebugPrint (DEBUG_INFO,"Waiting for incoming confidential migration.\n");
>      DisableInterrupts ();
> +    SwitchToMigrationHelperPageTables();
>      CpuDeadLoop ();
>    }
>  
> -- 
> 2.20.1
> 

Re: [RFC PATCH 03/14] OvmfPkg/PlatformDxe: Add support for SEV live migration.

[Ashish Kalra]

Hello Tobin,

You don't need this patch for MH support, this patch is only required
for (SEV) slow migration support.

Thanks,
Ashish

On Tue, Mar 02, 2021 at 03:48:28PM -0500, Tobin Feldman-Fitzthum wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> Detect for KVM hypervisor and check for SEV live migration
> feature support via KVM_FEATURE_CPUID, if detected setup a new
> UEFI enviroment variable to indicate OVMF support for SEV
> live migration.
> 
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>  OvmfPkg/OvmfPkg.dec                  |  1 +
>  OvmfPkg/PlatformDxe/Platform.inf     |  2 +
>  OvmfPkg/Include/Guid/MemEncryptLib.h | 16 +++++
>  OvmfPkg/PlatformDxe/PlatformConfig.h |  5 ++
>  OvmfPkg/PlatformDxe/AmdSev.c         | 99 ++++++++++++++++++++++++++++
>  OvmfPkg/PlatformDxe/Platform.c       |  6 ++
>  6 files changed, 129 insertions(+)
>  create mode 100644 OvmfPkg/Include/Guid/MemEncryptLib.h
>  create mode 100644 OvmfPkg/PlatformDxe/AmdSev.c
> 
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index 4348bb45c6..4450d78b91 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -122,6 +122,7 @@
>    gQemuKernelLoaderFsMediaGuid          = {0x1428f772, 0xb64a, 0x441e, {0xb8, 0xc3, 0x9e, 0xbd, 0xd7, 0xf8, 0x93, 0xc7}}
>    gGrubFileGuid                         = {0xb5ae312c, 0xbc8a, 0x43b1, {0x9c, 0x62, 0xeb, 0xb8, 0x26, 0xdd, 0x5d, 0x07}}
>    gConfidentialComputingSecretGuid      = {0xadf956ad, 0xe98c, 0x484c, {0xae, 0x11, 0xb5, 0x1c, 0x7d, 0x33, 0x64, 0x47}}
> +  gMemEncryptGuid                       = {0x0cf29b71, 0x9e51, 0x433a, {0xa3, 0xb7, 0x81, 0xf3, 0xab, 0x16, 0xb8, 0x75}}
>  
>  [Ppis]
>    # PPI whose presence in the PPI database signals that the TPM base address
> diff --git a/OvmfPkg/PlatformDxe/Platform.inf b/OvmfPkg/PlatformDxe/Platform.inf
> index 14727c1220..2896f0a1d1 100644
> --- a/OvmfPkg/PlatformDxe/Platform.inf
> +++ b/OvmfPkg/PlatformDxe/Platform.inf
> @@ -24,6 +24,7 @@
>    PlatformConfig.c
>    PlatformConfig.h
>    PlatformForms.vfr
> +  AmdSev.c
>  
>  [Packages]
>    MdePkg/MdePkg.dec
> @@ -56,6 +57,7 @@
>  [Guids]
>    gEfiIfrTianoGuid
>    gOvmfPlatformConfigGuid
> +  gMemEncryptGuid
>  
>  [Depex]
>    gEfiHiiConfigRoutingProtocolGuid  AND
> diff --git a/OvmfPkg/Include/Guid/MemEncryptLib.h b/OvmfPkg/Include/Guid/MemEncryptLib.h
> new file mode 100644
> index 0000000000..8264a647af
> --- /dev/null
> +++ b/OvmfPkg/Include/Guid/MemEncryptLib.h
> @@ -0,0 +1,16 @@
> +/** @file
> +  AMD Memory Encryption GUID, define a new GUID for defining
> +  new UEFI enviroment variables assocaiated with SEV Memory Encryption.
> +  Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#ifndef __MEMENCRYPT_LIB_H__
> +#define __MEMENCRYPT_LIB_H__
> +
> +#define MEMENCRYPT_GUID \
> +{0x0cf29b71, 0x9e51, 0x433a, {0xa3, 0xb7, 0x81, 0xf3, 0xab, 0x16, 0xb8, 0x75}}
> +
> +extern EFI_GUID gMemEncryptGuid;
> +
> +#endif
> diff --git a/OvmfPkg/PlatformDxe/PlatformConfig.h b/OvmfPkg/PlatformDxe/PlatformConfig.h
> index 716514da21..4f662aafa4 100644
> --- a/OvmfPkg/PlatformDxe/PlatformConfig.h
> +++ b/OvmfPkg/PlatformDxe/PlatformConfig.h
> @@ -44,6 +44,11 @@ PlatformConfigLoad (
>    OUT UINT64          *OptionalElements
>    );
>  
> +VOID
> +AmdSevSetConfig(
> +  VOID
> +  );
> +
>  //
>  // Feature flags for OptionalElements.
>  //
> diff --git a/OvmfPkg/PlatformDxe/AmdSev.c b/OvmfPkg/PlatformDxe/AmdSev.c
> new file mode 100644
> index 0000000000..1f804984b7
> --- /dev/null
> +++ b/OvmfPkg/PlatformDxe/AmdSev.c
> @@ -0,0 +1,99 @@
> +/**@file
> +  Detect KVM hypervisor support for SEV live migration and if
> +  detected, setup a new UEFI enviroment variable indicating
> +  OVMF support for SEV live migration.
> +  Copyright (c) 2020, Advanced Micro Devices. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +//
> +// The package level header files this module uses
> +//
> +
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>
> +#include <Guid/MemEncryptLib.h>
> +
> +/**
> +  Figures out if we are running inside KVM HVM and
> +  KVM HVM supports SEV Live Migration feature.
> +  @retval TRUE   KVM was detected and Live Migration supported
> +  @retval FALSE  KVM was not detected or Live Migration not supported
> +**/
> +BOOLEAN
> +KvmDetectSevLiveMigrationFeature(
> +  VOID
> +  )
> +{
> +  UINT8 Signature[13];
> +  UINT32 mKvmLeaf = 0;
> +  UINT32 RegEax, RegEbx, RegEcx, RegEdx;
> +
> +  Signature[12] = '\0';
> +  for (mKvmLeaf = 0x40000000; mKvmLeaf < 0x40010000; mKvmLeaf += 0x100) {
> +    AsmCpuid (mKvmLeaf,
> +              NULL,
> +              (UINT32 *) &Signature[0],
> +              (UINT32 *) &Signature[4],
> +              (UINT32 *) &Signature[8]);
> +
> +    if (!AsciiStrCmp ((CHAR8 *) Signature, "KVMKVMKVM\0\0\0")) {
> +   DEBUG ((
> +    DEBUG_ERROR,
> +    "%a: KVM Detected, signature = %s\n",
> +    __FUNCTION__,
> +    Signature
> +    ));
> +
> +    RegEax = 0x40000001;
> +    RegEcx = 0;
> +      AsmCpuid (0x40000001, &RegEax, &RegEbx, &RegEcx, &RegEdx);
> +      if (RegEax & (1 << 14)) {
> +     DEBUG ((
> +    DEBUG_ERROR,
> +    "%a: Live Migration feature supported\n",
> +    __FUNCTION__
> +    ));
> +    return TRUE;
> +     }
> +    }
> +  }
> +
> +  return FALSE;
> +}
> +
> +/**
> +  Function checks if SEV Live Migration support is available, if present then it sets
> +  a UEFI enviroment variable to be queried later using Runtime services.
> +  **/
> +VOID
> +AmdSevSetConfig(
> +  VOID
> +  )
> +{
> +  EFI_STATUS Status;
> +  BOOLEAN SevLiveMigrationEnabled;
> +
> +  SevLiveMigrationEnabled = KvmDetectSevLiveMigrationFeature();
> +
> +  if (SevLiveMigrationEnabled) {
> +   Status = gRT->SetVariable (
> +    L"SevLiveMigrationEnabled",
> +                &gMemEncryptGuid,
> +    EFI_VARIABLE_NON_VOLATILE |
> +                EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +          EFI_VARIABLE_RUNTIME_ACCESS,
> +                sizeof (BOOLEAN),
> +                &SevLiveMigrationEnabled
> +               );
> +
> +   DEBUG ((
> +    DEBUG_ERROR,
> +    "%a: Setting SevLiveMigrationEnabled variable, status = %lx\n",
> +    __FUNCTION__,
> +    Status
> +    ));
> +  }
> +}
> diff --git a/OvmfPkg/PlatformDxe/Platform.c b/OvmfPkg/PlatformDxe/Platform.c
> index f2e51960ce..9a19b9f6b1 100644
> --- a/OvmfPkg/PlatformDxe/Platform.c
> +++ b/OvmfPkg/PlatformDxe/Platform.c
> @@ -763,6 +763,12 @@ PlatformInit (
>  {
>    EFI_STATUS Status;
>  
> +  //
> +  // Set Amd Sev configuation
> +  //
> +  AmdSevSetConfig();
> +
> +
>    ExecutePlatformConfig ();
>  
>    mConfigAccess.ExtractConfig = &ExtractConfig;
> -- 
> 2.20.1
> 

Re: [RFC PATCH 03/14] OvmfPkg/PlatformDxe: Add support for SEV live migration.

[Tobin Feldman-Fitzthum]

On 3/3/21 11:41 AM, Ashish Kalra wrote:
> Hello Tobin,
>
> You don't need this patch for MH support, this patch is only required
> for (SEV) slow migration support.

If the SevLiveMigrationEnabled variable is not set, the bitmap sync does 
not work correctly (bitmap all zeros), at least for the version of the 
kernel we have been using. Since the bitmap will be replaced, this might 
not be necessary in the future but it is for our setup at the moment.

-Tobin

>
> Thanks,
> Ashish
>
> On Tue, Mar 02, 2021 at 03:48:28PM -0500, Tobin Feldman-Fitzthum wrote:
>> From: Ashish Kalra <ashish.kalra@amd.com>
>>
>> Detect for KVM hypervisor and check for SEV live migration
>> feature support via KVM_FEATURE_CPUID, if detected setup a new
>> UEFI enviroment variable to indicate OVMF support for SEV
>> live migration.
>>
>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>> ---
>>   OvmfPkg/OvmfPkg.dec                  |  1 +
>>   OvmfPkg/PlatformDxe/Platform.inf     |  2 +
>>   OvmfPkg/Include/Guid/MemEncryptLib.h | 16 +++++
>>   OvmfPkg/PlatformDxe/PlatformConfig.h |  5 ++
>>   OvmfPkg/PlatformDxe/AmdSev.c         | 99 ++++++++++++++++++++++++++++
>>   OvmfPkg/PlatformDxe/Platform.c       |  6 ++
>>   6 files changed, 129 insertions(+)
>>   create mode 100644 OvmfPkg/Include/Guid/MemEncryptLib.h
>>   create mode 100644 OvmfPkg/PlatformDxe/AmdSev.c
>>
>> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
>> index 4348bb45c6..4450d78b91 100644
>> --- a/OvmfPkg/OvmfPkg.dec
>> +++ b/OvmfPkg/OvmfPkg.dec
>> @@ -122,6 +122,7 @@
>>     gQemuKernelLoaderFsMediaGuid          = {0x1428f772, 0xb64a, 0x441e, {0xb8, 0xc3, 0x9e, 0xbd, 0xd7, 0xf8, 0x93, 0xc7}}
>>     gGrubFileGuid                         = {0xb5ae312c, 0xbc8a, 0x43b1, {0x9c, 0x62, 0xeb, 0xb8, 0x26, 0xdd, 0x5d, 0x07}}
>>     gConfidentialComputingSecretGuid      = {0xadf956ad, 0xe98c, 0x484c, {0xae, 0x11, 0xb5, 0x1c, 0x7d, 0x33, 0x64, 0x47}}
>> +  gMemEncryptGuid                       = {0x0cf29b71, 0x9e51, 0x433a, {0xa3, 0xb7, 0x81, 0xf3, 0xab, 0x16, 0xb8, 0x75}}
>>   
>>   [Ppis]
>>     # PPI whose presence in the PPI database signals that the TPM base address
>> diff --git a/OvmfPkg/PlatformDxe/Platform.inf b/OvmfPkg/PlatformDxe/Platform.inf
>> index 14727c1220..2896f0a1d1 100644
>> --- a/OvmfPkg/PlatformDxe/Platform.inf
>> +++ b/OvmfPkg/PlatformDxe/Platform.inf
>> @@ -24,6 +24,7 @@
>>     PlatformConfig.c
>>     PlatformConfig.h
>>     PlatformForms.vfr
>> +  AmdSev.c
>>   
>>   [Packages]
>>     MdePkg/MdePkg.dec
>> @@ -56,6 +57,7 @@
>>   [Guids]
>>     gEfiIfrTianoGuid
>>     gOvmfPlatformConfigGuid
>> +  gMemEncryptGuid
>>   
>>   [Depex]
>>     gEfiHiiConfigRoutingProtocolGuid  AND
>> diff --git a/OvmfPkg/Include/Guid/MemEncryptLib.h b/OvmfPkg/Include/Guid/MemEncryptLib.h
>> new file mode 100644
>> index 0000000000..8264a647af
>> --- /dev/null
>> +++ b/OvmfPkg/Include/Guid/MemEncryptLib.h
>> @@ -0,0 +1,16 @@
>> +/** @file
>> +  AMD Memory Encryption GUID, define a new GUID for defining
>> +  new UEFI enviroment variables assocaiated with SEV Memory Encryption.
>> +  Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +**/
>> +
>> +#ifndef __MEMENCRYPT_LIB_H__
>> +#define __MEMENCRYPT_LIB_H__
>> +
>> +#define MEMENCRYPT_GUID \
>> +{0x0cf29b71, 0x9e51, 0x433a, {0xa3, 0xb7, 0x81, 0xf3, 0xab, 0x16, 0xb8, 0x75}}
>> +
>> +extern EFI_GUID gMemEncryptGuid;
>> +
>> +#endif
>> diff --git a/OvmfPkg/PlatformDxe/PlatformConfig.h b/OvmfPkg/PlatformDxe/PlatformConfig.h
>> index 716514da21..4f662aafa4 100644
>> --- a/OvmfPkg/PlatformDxe/PlatformConfig.h
>> +++ b/OvmfPkg/PlatformDxe/PlatformConfig.h
>> @@ -44,6 +44,11 @@ PlatformConfigLoad (
>>     OUT UINT64          *OptionalElements
>>     );
>>   
>> +VOID
>> +AmdSevSetConfig(
>> +  VOID
>> +  );
>> +
>>   //
>>   // Feature flags for OptionalElements.
>>   //
>> diff --git a/OvmfPkg/PlatformDxe/AmdSev.c b/OvmfPkg/PlatformDxe/AmdSev.c
>> new file mode 100644
>> index 0000000000..1f804984b7
>> --- /dev/null
>> +++ b/OvmfPkg/PlatformDxe/AmdSev.c
>> @@ -0,0 +1,99 @@
>> +/**@file
>> +  Detect KVM hypervisor support for SEV live migration and if
>> +  detected, setup a new UEFI enviroment variable indicating
>> +  OVMF support for SEV live migration.
>> +  Copyright (c) 2020, Advanced Micro Devices. All rights reserved.<BR>
>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +**/
>> +//
>> +// The package level header files this module uses
>> +//
>> +
>> +#include <Library/BaseLib.h>
>> +#include <Library/BaseMemoryLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/UefiBootServicesTableLib.h>
>> +#include <Library/UefiRuntimeServicesTableLib.h>
>> +#include <Guid/MemEncryptLib.h>
>> +
>> +/**
>> +  Figures out if we are running inside KVM HVM and
>> +  KVM HVM supports SEV Live Migration feature.
>> +  @retval TRUE   KVM was detected and Live Migration supported
>> +  @retval FALSE  KVM was not detected or Live Migration not supported
>> +**/
>> +BOOLEAN
>> +KvmDetectSevLiveMigrationFeature(
>> +  VOID
>> +  )
>> +{
>> +  UINT8 Signature[13];
>> +  UINT32 mKvmLeaf = 0;
>> +  UINT32 RegEax, RegEbx, RegEcx, RegEdx;
>> +
>> +  Signature[12] = '\0';
>> +  for (mKvmLeaf = 0x40000000; mKvmLeaf < 0x40010000; mKvmLeaf += 0x100) {
>> +    AsmCpuid (mKvmLeaf,
>> +              NULL,
>> +              (UINT32 *) &Signature[0],
>> +              (UINT32 *) &Signature[4],
>> +              (UINT32 *) &Signature[8]);
>> +
>> +    if (!AsciiStrCmp ((CHAR8 *) Signature, "KVMKVMKVM\0\0\0")) {
>> +   DEBUG ((
>> +    DEBUG_ERROR,
>> +    "%a: KVM Detected, signature = %s\n",
>> +    __FUNCTION__,
>> +    Signature
>> +    ));
>> +
>> +    RegEax = 0x40000001;
>> +    RegEcx = 0;
>> +      AsmCpuid (0x40000001, &RegEax, &RegEbx, &RegEcx, &RegEdx);
>> +      if (RegEax & (1 << 14)) {
>> +     DEBUG ((
>> +    DEBUG_ERROR,
>> +    "%a: Live Migration feature supported\n",
>> +    __FUNCTION__
>> +    ));
>> +    return TRUE;
>> +     }
>> +    }
>> +  }
>> +
>> +  return FALSE;
>> +}
>> +
>> +/**
>> +  Function checks if SEV Live Migration support is available, if present then it sets
>> +  a UEFI enviroment variable to be queried later using Runtime services.
>> +  **/
>> +VOID
>> +AmdSevSetConfig(
>> +  VOID
>> +  )
>> +{
>> +  EFI_STATUS Status;
>> +  BOOLEAN SevLiveMigrationEnabled;
>> +
>> +  SevLiveMigrationEnabled = KvmDetectSevLiveMigrationFeature();
>> +
>> +  if (SevLiveMigrationEnabled) {
>> +   Status = gRT->SetVariable (
>> +    L"SevLiveMigrationEnabled",
>> +                &gMemEncryptGuid,
>> +    EFI_VARIABLE_NON_VOLATILE |
>> +                EFI_VARIABLE_BOOTSERVICE_ACCESS |
>> +          EFI_VARIABLE_RUNTIME_ACCESS,
>> +                sizeof (BOOLEAN),
>> +                &SevLiveMigrationEnabled
>> +               );
>> +
>> +   DEBUG ((
>> +    DEBUG_ERROR,
>> +    "%a: Setting SevLiveMigrationEnabled variable, status = %lx\n",
>> +    __FUNCTION__,
>> +    Status
>> +    ));
>> +  }
>> +}
>> diff --git a/OvmfPkg/PlatformDxe/Platform.c b/OvmfPkg/PlatformDxe/Platform.c
>> index f2e51960ce..9a19b9f6b1 100644
>> --- a/OvmfPkg/PlatformDxe/Platform.c
>> +++ b/OvmfPkg/PlatformDxe/Platform.c
>> @@ -763,6 +763,12 @@ PlatformInit (
>>   {
>>     EFI_STATUS Status;
>>   
>> +  //
>> +  // Set Amd Sev configuation
>> +  //
>> +  AmdSevSetConfig();
>> +
>> +
>>     ExecutePlatformConfig ();
>>   
>>     mConfigAccess.ExtractConfig = &ExtractConfig;
>> -- 
>> 2.20.1
>>

Re: [RFC PATCH 03/14] OvmfPkg/PlatformDxe: Add support for SEV live migration.

[Ashish Kalra]

On Wed, Mar 03, 2021 at 11:47:51AM -0500, Tobin Feldman-Fitzthum wrote:
> 
> On 3/3/21 11:41 AM, Ashish Kalra wrote:
> > Hello Tobin,
> > 
> > You don't need this patch for MH support, this patch is only required
> > for (SEV) slow migration support.
> 
> If the SevLiveMigrationEnabled variable is not set, the bitmap sync does not
> work correctly (bitmap all zeros), at least for the version of the kernel we
> have been using. Since the bitmap will be replaced, this might not be
> necessary in the future but it is for our setup at the moment.
> 

Ok, i remember that GET_PAGE_ENCRYPTION_BITMAP ioctl will not work
correctly.

Thanks,
Ashish

> > 
> > On Tue, Mar 02, 2021 at 03:48:28PM -0500, Tobin Feldman-Fitzthum wrote:
> > > From: Ashish Kalra <ashish.kalra@amd.com>
> > > 
> > > Detect for KVM hypervisor and check for SEV live migration
> > > feature support via KVM_FEATURE_CPUID, if detected setup a new
> > > UEFI enviroment variable to indicate OVMF support for SEV
> > > live migration.
> > > 
> > > Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> > > ---
> > >   OvmfPkg/OvmfPkg.dec                  |  1 +
> > >   OvmfPkg/PlatformDxe/Platform.inf     |  2 +
> > >   OvmfPkg/Include/Guid/MemEncryptLib.h | 16 +++++
> > >   OvmfPkg/PlatformDxe/PlatformConfig.h |  5 ++
> > >   OvmfPkg/PlatformDxe/AmdSev.c         | 99 ++++++++++++++++++++++++++++
> > >   OvmfPkg/PlatformDxe/Platform.c       |  6 ++
> > >   6 files changed, 129 insertions(+)
> > >   create mode 100644 OvmfPkg/Include/Guid/MemEncryptLib.h
> > >   create mode 100644 OvmfPkg/PlatformDxe/AmdSev.c
> > > 
> > > diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> > > index 4348bb45c6..4450d78b91 100644
> > > --- a/OvmfPkg/OvmfPkg.dec
> > > +++ b/OvmfPkg/OvmfPkg.dec
> > > @@ -122,6 +122,7 @@
> > >     gQemuKernelLoaderFsMediaGuid          = {0x1428f772, 0xb64a, 0x441e, {0xb8, 0xc3, 0x9e, 0xbd, 0xd7, 0xf8, 0x93, 0xc7}}
> > >     gGrubFileGuid                         = {0xb5ae312c, 0xbc8a, 0x43b1, {0x9c, 0x62, 0xeb, 0xb8, 0x26, 0xdd, 0x5d, 0x07}}
> > >     gConfidentialComputingSecretGuid      = {0xadf956ad, 0xe98c, 0x484c, {0xae, 0x11, 0xb5, 0x1c, 0x7d, 0x33, 0x64, 0x47}}
> > > +  gMemEncryptGuid                       = {0x0cf29b71, 0x9e51, 0x433a, {0xa3, 0xb7, 0x81, 0xf3, 0xab, 0x16, 0xb8, 0x75}}
> > >   [Ppis]
> > >     # PPI whose presence in the PPI database signals that the TPM base address
> > > diff --git a/OvmfPkg/PlatformDxe/Platform.inf b/OvmfPkg/PlatformDxe/Platform.inf
> > > index 14727c1220..2896f0a1d1 100644
> > > --- a/OvmfPkg/PlatformDxe/Platform.inf
> > > +++ b/OvmfPkg/PlatformDxe/Platform.inf
> > > @@ -24,6 +24,7 @@
> > >     PlatformConfig.c
> > >     PlatformConfig.h
> > >     PlatformForms.vfr
> > > +  AmdSev.c
> > >   [Packages]
> > >     MdePkg/MdePkg.dec
> > > @@ -56,6 +57,7 @@
> > >   [Guids]
> > >     gEfiIfrTianoGuid
> > >     gOvmfPlatformConfigGuid
> > > +  gMemEncryptGuid
> > >   [Depex]
> > >     gEfiHiiConfigRoutingProtocolGuid  AND
> > > diff --git a/OvmfPkg/Include/Guid/MemEncryptLib.h b/OvmfPkg/Include/Guid/MemEncryptLib.h
> > > new file mode 100644
> > > index 0000000000..8264a647af
> > > --- /dev/null
> > > +++ b/OvmfPkg/Include/Guid/MemEncryptLib.h
> > > @@ -0,0 +1,16 @@
> > > +/** @file
> > > +  AMD Memory Encryption GUID, define a new GUID for defining
> > > +  new UEFI enviroment variables assocaiated with SEV Memory Encryption.
> > > +  Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
> > > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > > +**/
> > > +
> > > +#ifndef __MEMENCRYPT_LIB_H__
> > > +#define __MEMENCRYPT_LIB_H__
> > > +
> > > +#define MEMENCRYPT_GUID \
> > > +{0x0cf29b71, 0x9e51, 0x433a, {0xa3, 0xb7, 0x81, 0xf3, 0xab, 0x16, 0xb8, 0x75}}
> > > +
> > > +extern EFI_GUID gMemEncryptGuid;
> > > +
> > > +#endif
> > > diff --git a/OvmfPkg/PlatformDxe/PlatformConfig.h b/OvmfPkg/PlatformDxe/PlatformConfig.h
> > > index 716514da21..4f662aafa4 100644
> > > --- a/OvmfPkg/PlatformDxe/PlatformConfig.h
> > > +++ b/OvmfPkg/PlatformDxe/PlatformConfig.h
> > > @@ -44,6 +44,11 @@ PlatformConfigLoad (
> > >     OUT UINT64          *OptionalElements
> > >     );
> > > +VOID
> > > +AmdSevSetConfig(
> > > +  VOID
> > > +  );
> > > +
> > >   //
> > >   // Feature flags for OptionalElements.
> > >   //
> > > diff --git a/OvmfPkg/PlatformDxe/AmdSev.c b/OvmfPkg/PlatformDxe/AmdSev.c
> > > new file mode 100644
> > > index 0000000000..1f804984b7
> > > --- /dev/null
> > > +++ b/OvmfPkg/PlatformDxe/AmdSev.c
> > > @@ -0,0 +1,99 @@
> > > +/**@file
> > > +  Detect KVM hypervisor support for SEV live migration and if
> > > +  detected, setup a new UEFI enviroment variable indicating
> > > +  OVMF support for SEV live migration.
> > > +  Copyright (c) 2020, Advanced Micro Devices. All rights reserved.<BR>
> > > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > > +**/
> > > +//
> > > +// The package level header files this module uses
> > > +//
> > > +
> > > +#include <Library/BaseLib.h>
> > > +#include <Library/BaseMemoryLib.h>
> > > +#include <Library/DebugLib.h>
> > > +#include <Library/UefiBootServicesTableLib.h>
> > > +#include <Library/UefiRuntimeServicesTableLib.h>
> > > +#include <Guid/MemEncryptLib.h>
> > > +
> > > +/**
> > > +  Figures out if we are running inside KVM HVM and
> > > +  KVM HVM supports SEV Live Migration feature.
> > > +  @retval TRUE   KVM was detected and Live Migration supported
> > > +  @retval FALSE  KVM was not detected or Live Migration not supported
> > > +**/
> > > +BOOLEAN
> > > +KvmDetectSevLiveMigrationFeature(
> > > +  VOID
> > > +  )
> > > +{
> > > +  UINT8 Signature[13];
> > > +  UINT32 mKvmLeaf = 0;
> > > +  UINT32 RegEax, RegEbx, RegEcx, RegEdx;
> > > +
> > > +  Signature[12] = '\0';
> > > +  for (mKvmLeaf = 0x40000000; mKvmLeaf < 0x40010000; mKvmLeaf += 0x100) {
> > > +    AsmCpuid (mKvmLeaf,
> > > +              NULL,
> > > +              (UINT32 *) &Signature[0],
> > > +              (UINT32 *) &Signature[4],
> > > +              (UINT32 *) &Signature[8]);
> > > +
> > > +    if (!AsciiStrCmp ((CHAR8 *) Signature, "KVMKVMKVM\0\0\0")) {
> > > +   DEBUG ((
> > > +    DEBUG_ERROR,
> > > +    "%a: KVM Detected, signature = %s\n",
> > > +    __FUNCTION__,
> > > +    Signature
> > > +    ));
> > > +
> > > +    RegEax = 0x40000001;
> > > +    RegEcx = 0;
> > > +      AsmCpuid (0x40000001, &RegEax, &RegEbx, &RegEcx, &RegEdx);
> > > +      if (RegEax & (1 << 14)) {
> > > +     DEBUG ((
> > > +    DEBUG_ERROR,
> > > +    "%a: Live Migration feature supported\n",
> > > +    __FUNCTION__
> > > +    ));
> > > +    return TRUE;
> > > +     }
> > > +    }
> > > +  }
> > > +
> > > +  return FALSE;
> > > +}
> > > +
> > > +/**
> > > +  Function checks if SEV Live Migration support is available, if present then it sets
> > > +  a UEFI enviroment variable to be queried later using Runtime services.
> > > +  **/
> > > +VOID
> > > +AmdSevSetConfig(
> > > +  VOID
> > > +  )
> > > +{
> > > +  EFI_STATUS Status;
> > > +  BOOLEAN SevLiveMigrationEnabled;
> > > +
> > > +  SevLiveMigrationEnabled = KvmDetectSevLiveMigrationFeature();
> > > +
> > > +  if (SevLiveMigrationEnabled) {
> > > +   Status = gRT->SetVariable (
> > > +    L"SevLiveMigrationEnabled",
> > > +                &gMemEncryptGuid,
> > > +    EFI_VARIABLE_NON_VOLATILE |
> > > +                EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > > +          EFI_VARIABLE_RUNTIME_ACCESS,
> > > +                sizeof (BOOLEAN),
> > > +                &SevLiveMigrationEnabled
> > > +               );
> > > +
> > > +   DEBUG ((
> > > +    DEBUG_ERROR,
> > > +    "%a: Setting SevLiveMigrationEnabled variable, status = %lx\n",
> > > +    __FUNCTION__,
> > > +    Status
> > > +    ));
> > > +  }
> > > +}
> > > diff --git a/OvmfPkg/PlatformDxe/Platform.c b/OvmfPkg/PlatformDxe/Platform.c
> > > index f2e51960ce..9a19b9f6b1 100644
> > > --- a/OvmfPkg/PlatformDxe/Platform.c
> > > +++ b/OvmfPkg/PlatformDxe/Platform.c
> > > @@ -763,6 +763,12 @@ PlatformInit (
> > >   {
> > >     EFI_STATUS Status;
> > > +  //
> > > +  // Set Amd Sev configuation
> > > +  //
> > > +  AmdSevSetConfig();
> > > +
> > > +
> > >     ExecutePlatformConfig ();
> > >     mConfigAccess.ExtractConfig = &ExtractConfig;
> > > -- 
> > > 2.20.1
> > > 

Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live Migration for AMD SEV

[Tobin Feldman-Fitzthum]

> Hi Tobin,
>
> On 03/02/21 21:48, Tobin Feldman-Fitzthum wrote:
>> This is a demonstration of fast migration for encrypted virtual machines
>> using a Migration Handler that lives in OVMF. This demo uses AMD SEV,
>> but the ideas may generalize to other confidential computing platforms.
>> With AMD SEV, guest memory is encrypted and the hypervisor cannot access
>> or move it. This makes migration tricky. In this demo, we show how the
>> HV can ask a Migration Handler (MH) in the firmware for an encrypted
>> page. The MH encrypts the page with a transport key prior to releasing
>> it to the HV. The target machine also runs an MH that decrypts the page
>> once it is passed in by the target HV. These patches are not ready for
>> production, but the are a full end-to-end solution that facilitates a
>> fast live migration between two SEV VMs.
>>
>> Corresponding patches for QEMU have been posted my colleague Dov Murik
>> on qemu-devel. Our approach needs little kernel support, requiring only
>> one hypercall that the guest can use to mark a page as encrypted or
>> shared. This series includes updated patches from Ashish Kalra and
>> Brijesh Singh that allow OVMF to use this hypercall.
>>
>> The MH runs continuously in the guest, waiting for communication from
>> the HV. The HV starts an additional vCPU for the MH but does not expose
>> it to the guest OS via ACPI. We use the MpService to start the MH. The
>> MpService is only available at runtime and processes that are started by
>> it are usually cleaned up on ExitBootServices. Since we need the MH to
>> run continuously, we had to make some modifications. Ideally a feature
>> could be added to the MpService to allow for the starting of
>> long-running processes. Besides migration, this could support other
>> background processes that need to operate within the encryption
>> boundary. For now, we have included a handful of patches that modify the
>> MpService to allow the MH to keep running after ExitBootServices. These
>> are temporary.
> I plan to do a lightweight review for this series. (My understanding is
> that it's an RFC and not actually being proposed for merging.)
>
> Regarding the MH's availability at runtime -- does that necessarily
> require the isolation of an AP? Because in the current approach,
> allowing the MP Services to survive into OS runtime (in some form or
> another) seems critical, and I don't think it's going to fly.
>
> I agree that the UefiCpuPkg patches have been well separated from the
> rest of the series, but I'm somewhat doubtful the "firmware-initiated
> background process" idea will be accepted. Have you investigated
> exposing a new "runtime service" (a function pointer) via the UEFI
> Configuration table, and calling that (perhaps periodically?) from the
> guest kernel? It would be a form of polling I guess. Or maybe, poll the
> mailbox directly in the kernel, and call the new firmware runtime
> service when there's an actual command to process.
Continuous runtime availability for the MH is almost certainly the most 
controversial part of this proposal, which is why I put it in the cover 
letter and why it's good to discuss.
> (You do spell out "little kernel support", and I'm not sure if that's a
> technical benefit, or a political / community benefit.)

As you allude to, minimal kernel support is really one of the main 
things that shapes our approach. This is partly a political and 
practical benefit, but there are also technical benefits. Having the MH 
in firmware likely leads to higher availability. It can be accessed when 
the OS is unreachable, perhaps during boot or when the OS is hung. There 
are also potential portability advantages although we do currently 
require support for one hypercall. The cost of implementing this 
hypercall is low.

Generally speaking, our task is to find a home for functionality that 
was traditionally provided by the hypervisor, but that needs to be 
inside the trust domain, but that isn't really part of a guest. A 
meta-goal of this project is to figure out the best way to do this.

>
> I'm quite uncomfortable with an attempt to hide a CPU from the OS via
> ACPI. The OS has other ways to learn (for example, a boot loader could
> use the MP services itself, stash the information, and hand it to the OS
> kernel -- this would minimally allow for detecting an inconsistency in
> the OS). What about "all-but-self" IPIs too -- the kernel might think
> all the processors it's poking like that were under its control.

This might be the second most controversial piece. Here's a question: if 
we could successfully hide the MH vCPU from the OS, would it still make 
you uncomfortable? In other words, is the worry that there might be some 
inconsistency or more generally that there is something hidden from the 
OS? One thing to think about is that the guest owner should generally be 
aware that there is a migration handler running. The way I see it, a 
guest owner of an SEV VM would need to opt-in to migration and should 
then expect that there is an MH running even if they aren't able to see 
it. Of course we need to be certain that the MH isn't going to break the OS.

> Also, as far as I can tell from patch #7, the AP seems to be
> busy-looping (with a CpuPause() added in), for the entire lifetime of
> the OS. Do I understand right? If so -- is it a temporary trait as well?

In our approach the MH continuously checks for commands from the 
hypervisor. There are potentially ways to optimize this, such as having 
the hypervisor de-schedule the MH vCPU while not migrating. You could 
potentially shut down down the MH on the target after receiving the 
MH_RESET command (when the migration finishes), but what if you want to 
migrate that VM somewhere else?

>
> Sorry if my questions are "premature", in the sense that I could get my
> own answers as well if I actually read the patches in detail -- however,
> I wouldn't like to do that at once, because then I'll be distracted by
> many style issues and other "trivial" stuff. Examples for the latter:

Not premature at all. I think you hit the nail on the head with 
everything you raised.

-Tobin

>
> - patch#1 calls SetMemoryEncDecHypercall3(), but there is no such
> function in edk2, so minimally it's a patch ordering bug in the series,
>
> - in patch#1, there's minimally one whitespace error (no whitespace
> right after "EFI_SIZE_TO_PAGES")
>
> - in patch#1, the alphabetical ordering in the [LibraryClasses] section,
> and in the matching #include directives, gets broken,
>
> - I'd prefer if the "SevLiveMigrationEnabled" UEFI variable were set in
> ConfidentialMigrationDxe, rather than PlatformDxe (patch #3), or at
> least another AMD SEV related DXE driver (OvmfPkg/AmdSevDxe etc).
>
> - any particular reasonf or making the UEFI variable non-volatile? I
> don't think it should survive any particular boot of the guest.
>
> - Why do we need a variable in the first place?
>
> etc etc
>
> Thanks!
> Laszlo
>
>
>
>
>> Ashish Kalra (2):
>>    OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap.
>>    OvmfPkg/PlatformDxe: Add support for SEV live migration.
>>
>> Brijesh Singh (1):
>>    OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall
>>
>> Dov Murik (1):
>>    OvmfPkg/AmdSev: Build page table for migration handler
>>
>> Tobin Feldman-Fitzthum (10):
>>    OvmfPkg/AmdSev: Base for Confidential Migration Handler
>>    OvmfPkg/PlatfomPei: Set Confidential Migration PCD
>>    OvmfPkg/AmdSev: Setup Migration Handler Mailbox
>>    OvmfPkg/AmdSev: MH support for mailbox protocol
>>    UefiCpuPkg/MpInitLib: temp removal of MpLib cleanup
>>    UefiCpuPkg/MpInitLib: Allocate MP buffer as runtime memory
>>    UefiCpuPkg/CpuExceptionHandlerLib: Exception handling as runtime
>>      memory
>>    OvmfPkg/AmdSev: Don't overwrite mailbox or pagetables
>>    OvmfPkg/AmdSev: Don't overwrite MH stack
>>    OvmfPkg/AmdSev: MH page encryption POC
>>
>>   OvmfPkg/OvmfPkg.dec                           |  11 +
>>   OvmfPkg/AmdSev/AmdSevX64.dsc                  |   2 +
>>   OvmfPkg/AmdSev/AmdSevX64.fdf                  |  13 +-
>>   .../ConfidentialMigrationDxe.inf              |  45 +++
>>   .../ConfidentialMigrationPei.inf              |  35 ++
>>   .../DxeMemEncryptSevLib.inf                   |   1 +
>>   .../PeiMemEncryptSevLib.inf                   |   1 +
>>   OvmfPkg/PlatformDxe/Platform.inf              |   2 +
>>   OvmfPkg/PlatformPei/PlatformPei.inf           |   2 +
>>   UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   2 +
>>   UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   2 +
>>   OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h  | 235 +++++++++++++
>>   .../ConfidentialMigration/VirtualMemory.h     | 177 ++++++++++
>>   OvmfPkg/Include/Guid/MemEncryptLib.h          |  16 +
>>   OvmfPkg/PlatformDxe/PlatformConfig.h          |   5 +
>>   .../ConfidentialMigrationDxe.c                | 325 ++++++++++++++++++
>>   .../ConfidentialMigrationPei.c                |  25 ++
>>   .../X64/PeiDxeVirtualMemory.c                 |  18 +
>>   OvmfPkg/PlatformDxe/AmdSev.c                  |  99 ++++++
>>   OvmfPkg/PlatformDxe/Platform.c                |   6 +
>>   OvmfPkg/PlatformPei/AmdSev.c                  |  10 +
>>   OvmfPkg/PlatformPei/Platform.c                |  10 +
>>   .../CpuExceptionHandlerLib/DxeException.c     |   8 +-
>>   UefiCpuPkg/Library/MpInitLib/DxeMpLib.c       |  21 +-
>>   UefiCpuPkg/Library/MpInitLib/MpLib.c          |   7 +-
>>   25 files changed, 1061 insertions(+), 17 deletions(-)
>>   create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
>>   create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.inf
>>   create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h
>>   create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h
>>   create mode 100644 OvmfPkg/Include/Guid/MemEncryptLib.h
>>   create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
>>   create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.c
>>   create mode 100644 OvmfPkg/PlatformDxe/AmdSev.c
>>

Re: [RFC PATCH 11/14] OvmfPkg/AmdSev: Build page table for migration handler

[Dov Murik]

On 03/03/2021 18:32, Ashish Kalra wrote:
> On Tue, Mar 02, 2021 at 03:48:36PM -0500, Tobin Feldman-Fitzthum wrote:
>> From: Dov Murik <dovmurik@linux.ibm.com>
>>
>> The migration handler builds its own page tables and switches
>> to them. The MH pagetables are reserved as runtime memory.
>>
>> When the hypervisor asks the MH to import/export a page, the HV
>> writes the guest physical address of the page in question to the
>> mailbox. The MH uses an identity mapping so that it can read/write
>> whatever GPA is requested by the HV. The hypervisor only asks the
>> MH to import/export encrypted pages. Thus, the C-Bit can be set
>> for every page in the identity map.
>>
>> The MH also needs to read shared pages, such as the mailbox.
>> These are mapped at an offset. The offset must be added to
>> the physical address before it can be resolved.
>>
>> Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
>> Signed-off-by: Dov Murik <dovmurik@linux.vnet.ibm.com>
>> ---
>>  .../ConfidentialMigrationDxe.inf              |   1 +
>>  .../ConfidentialMigration/VirtualMemory.h     | 177 ++++++++++++++++++
>>  .../ConfidentialMigrationDxe.c                |  88 ++++++++-
>>  3 files changed, 265 insertions(+), 1 deletion(-)
>>  create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h
>>
>> diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
>> index 49457d5d17..8dadfd1d13 100644
>> --- a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
>> +++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
>> @@ -15,6 +15,7 @@
>>  
>>  [Sources]
>>    ConfidentialMigrationDxe.c
>> +  VirtualMemory.h
>>  
>>  [Packages]
>>    MdePkg/MdePkg.dec
>> diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h b/OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h
>> new file mode 100644
>> index 0000000000..c50cb64c63
>> --- /dev/null
>> +++ b/OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h
>> @@ -0,0 +1,177 @@
>> +/** @file
>> +  Virtual Memory Management Services to set or clear the memory encryption bit
>> +  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>> +  Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +  Code is derived from OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
>> +
>> +**/
>> +
>> +#ifndef __VIRTUAL_MEMORY__
>> +#define __VIRTUAL_MEMORY__
>> +
>> +#include <Library/BaseLib.h>
>> +#include <Library/BaseMemoryLib.h>
>> +#include <Library/CacheMaintenanceLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/MemoryAllocationLib.h>
>> +#include <Uefi.h>
>> +
>> +#define SYS_CODE64_SEL 0x38
>> +
>> +#pragma pack(1)
>> +
>> +//
>> +// Page-Map Level-4 Offset (PML4) and
>> +// Page-Directory-Pointer Offset (PDPE) entries 4K & 2MB
>> +//
>> +
>> +typedef union {
>> +  struct {
>> +    UINT64  Present:1;                // 0 = Not present in memory,
>> +                                      //   1 = Present in memory
>> +    UINT64  ReadWrite:1;              // 0 = Read-Only, 1= Read/Write
>> +    UINT64  UserSupervisor:1;         // 0 = Supervisor, 1=User
>> +    UINT64  WriteThrough:1;           // 0 = Write-Back caching,
>> +                                      //   1 = Write-Through caching
>> +    UINT64  CacheDisabled:1;          // 0 = Cached, 1=Non-Cached
>> +    UINT64  Accessed:1;               // 0 = Not accessed,
>> +                                      //   1 = Accessed (set by CPU)
>> +    UINT64  Reserved:1;               // Reserved
>> +    UINT64  MustBeZero:2;             // Must Be Zero
>> +    UINT64  Available:3;              // Available for use by system software
>> +    UINT64  PageTableBaseAddress:40;  // Page Table Base Address
>> +    UINT64  AvabilableHigh:11;        // Available for use by system software
>> +    UINT64  Nx:1;                     // No Execute bit
>> +  } Bits;
>> +  UINT64    Uint64;
>> +} PAGE_MAP_AND_DIRECTORY_POINTER;
>> +
>> +//
>> +// Page Table Entry 4KB
>> +//
>> +typedef union {
>> +  struct {
>> +    UINT64  Present:1;                // 0 = Not present in memory,
>> +                                      //   1 = Present in memory
>> +    UINT64  ReadWrite:1;              // 0 = Read-Only, 1= Read/Write
>> +    UINT64  UserSupervisor:1;         // 0 = Supervisor, 1=User
>> +    UINT64  WriteThrough:1;           // 0 = Write-Back caching,
>> +                                      //   1 = Write-Through caching
>> +    UINT64  CacheDisabled:1;          // 0 = Cached, 1=Non-Cached
>> +    UINT64  Accessed:1;               // 0 = Not accessed,
>> +                                      //   1 = Accessed (set by CPU)
>> +    UINT64  Dirty:1;                  // 0 = Not Dirty, 1 = written by
>> +                                      //   processor on access to page
>> +    UINT64  PAT:1;                    //
>> +    UINT64  Global:1;                 // 0 = Not global page, 1 = global page
>> +                                      //   TLB not cleared on CR3 write
>> +    UINT64  Available:3;              // Available for use by system software
>> +    UINT64  PageTableBaseAddress:40;  // Page Table Base Address
>> +    UINT64  AvabilableHigh:11;        // Available for use by system software
>> +    UINT64  Nx:1;                     // 0 = Execute Code,
>> +                                      //   1 = No Code Execution
>> +  } Bits;
>> +  UINT64    Uint64;
>> +} PAGE_TABLE_4K_ENTRY;
>> +
>> +//
>> +// Page Table Entry 2MB
>> +//
>> +typedef union {
>> +  struct {
>> +    UINT64  Present:1;                // 0 = Not present in memory,
>> +                                      //   1 = Present in memory
>> +    UINT64  ReadWrite:1;              // 0 = Read-Only, 1= Read/Write
>> +    UINT64  UserSupervisor:1;         // 0 = Supervisor, 1=User
>> +    UINT64  WriteThrough:1;           // 0 = Write-Back caching,
>> +                                      //   1=Write-Through caching
>> +    UINT64  CacheDisabled:1;          // 0 = Cached, 1=Non-Cached
>> +    UINT64  Accessed:1;               // 0 = Not accessed,
>> +                                      //   1 = Accessed (set by CPU)
>> +    UINT64  Dirty:1;                  // 0 = Not Dirty, 1 = written by
>> +                                      //   processor on access to page
>> +    UINT64  MustBe1:1;                // Must be 1
>> +    UINT64  Global:1;                 // 0 = Not global page, 1 = global page
>> +                                      //   TLB not cleared on CR3 write
>> +    UINT64  Available:3;              // Available for use by system software
>> +    UINT64  PAT:1;                    //
>> +    UINT64  MustBeZero:8;             // Must be zero;
>> +    UINT64  PageTableBaseAddress:31;  // Page Table Base Address
>> +    UINT64  AvabilableHigh:11;        // Available for use by system software
>> +    UINT64  Nx:1;                     // 0 = Execute Code,
>> +                                      //   1 = No Code Execution
>> +  } Bits;
>> +  UINT64    Uint64;
>> +} PAGE_TABLE_ENTRY;
>> +
>> +//
>> +// Page Table Entry 1GB
>> +//
>> +typedef union {
>> +  struct {
>> +    UINT64  Present:1;                // 0 = Not present in memory,
>> +                                      //   1 = Present in memory
>> +    UINT64  ReadWrite:1;              // 0 = Read-Only, 1= Read/Write
>> +    UINT64  UserSupervisor:1;         // 0 = Supervisor, 1=User
>> +    UINT64  WriteThrough:1;           // 0 = Write-Back caching,
>> +                                      //   1 = Write-Through caching
>> +    UINT64  CacheDisabled:1;          // 0 = Cached, 1=Non-Cached
>> +    UINT64  Accessed:1;               // 0 = Not accessed,
>> +                                      //   1 = Accessed (set by CPU)
>> +    UINT64  Dirty:1;                  // 0 = Not Dirty, 1 = written by
>> +                                      //   processor on access to page
>> +    UINT64  MustBe1:1;                // Must be 1
>> +    UINT64  Global:1;                 // 0 = Not global page, 1 = global page
>> +                                      //   TLB not cleared on CR3 write
>> +    UINT64  Available:3;              // Available for use by system software
>> +    UINT64  PAT:1;                    //
>> +    UINT64  MustBeZero:17;            // Must be zero;
>> +    UINT64  PageTableBaseAddress:22;  // Page Table Base Address
>> +    UINT64  AvabilableHigh:11;        // Available for use by system software
>> +    UINT64  Nx:1;                     // 0 = Execute Code,
>> +                                      //   1 = No Code Execution
>> +  } Bits;
>> +  UINT64    Uint64;
>> +} PAGE_TABLE_1G_ENTRY;
>> +
>> +#pragma pack()
>> +
>> +#define IA32_PG_P                   BIT0
>> +#define IA32_PG_RW                  BIT1
>> +#define IA32_PG_PS                  BIT7
>> +
>> +#define PAGING_PAE_INDEX_MASK       0x1FF
>> +
>> +#define PAGING_4K_ADDRESS_MASK_64   0x000FFFFFFFFFF000ull
>> +#define PAGING_2M_ADDRESS_MASK_64   0x000FFFFFFFE00000ull
>> +#define PAGING_1G_ADDRESS_MASK_64   0x000FFFFFC0000000ull
>> +
>> +#define PAGING_L1_ADDRESS_SHIFT     12
>> +#define PAGING_L2_ADDRESS_SHIFT     21
>> +#define PAGING_L3_ADDRESS_SHIFT     30
>> +#define PAGING_L4_ADDRESS_SHIFT     39
>> +
>> +#define PAGING_PML4E_NUMBER         4
>> +
>> +#define PAGETABLE_ENTRY_MASK        ((1UL << 9) - 1)
>> +#define PML4_OFFSET(x)              ( (x >> 39) & PAGETABLE_ENTRY_MASK)
>> +#define PDP_OFFSET(x)               ( (x >> 30) & PAGETABLE_ENTRY_MASK)
>> +#define PDE_OFFSET(x)               ( (x >> 21) & PAGETABLE_ENTRY_MASK)
>> +#define PTE_OFFSET(x)               ( (x >> 12) & PAGETABLE_ENTRY_MASK)
>> +#define PAGING_1G_ADDRESS_MASK_64   0x000FFFFFC0000000ull
>> +
>> +#define PAGE_TABLE_POOL_ALIGNMENT   BASE_2MB
>> +#define PAGE_TABLE_POOL_UNIT_SIZE   SIZE_2MB
>> +#define PAGE_TABLE_POOL_UNIT_PAGES  \
>> +  EFI_SIZE_TO_PAGES (PAGE_TABLE_POOL_UNIT_SIZE)
>> +#define PAGE_TABLE_POOL_ALIGN_MASK  \
>> +  (~(EFI_PHYSICAL_ADDRESS)(PAGE_TABLE_POOL_ALIGNMENT - 1))
>> +
>> +typedef struct {
>> +  VOID            *NextPool;
>> +  UINTN           Offset;
>> +  UINTN           FreePages;
>> +} PAGE_TABLE_POOL;
>> +
>> +#endif
>> diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
>> index 8402fcc4fa..3df3b09732 100644
>> --- a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
>> +++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
>> @@ -11,6 +11,7 @@
>>  #include <Protocol/MpService.h>
>>  #include <Library/BaseMemoryLib.h>
>>  
>> +#include "VirtualMemory.h"
>>  //
>>  // Functions implemented by the migration handler
>>  //
>> @@ -43,6 +44,83 @@ typedef volatile struct {
>>    UINT32       done;
>>  } MH_COMMAND_PARAMETERS;
>>  
>> +//
>> +// Addresses for MH page table.
>> +//
>> +STATIC PAGE_TABLE_POOL   *mPageTablePool = NULL;
>> +STATIC PHYSICAL_ADDRESS  mMigrationHelperPageTables = 0;
>> +
>> +//
>> +// Offset for non-cbit mapping.
>> +//
>> +#define UNENC_VIRT_ADDR_BASE    0xffffff8000000000ULL
>> +
>> +
>> +/**
>> +  Allocates and fills in custom page tables for Migration Handler.
>> +  The MH must be able to write to any encrypted page. Thus, it
>> +  uses an identity map where the C-bit is set for every page. The
>> +  HV should never ask the MH to import/export a shared page. The
>> +  MH must also be able to read some shared pages. The first 1GB
>> +  of memory is mapped at offset UNENC_VIRT_ADDR_BASE.
>> +
>> +**/
>> +VOID
>> +PrepareMigrationHandlerPageTables (
>> +  VOID
>> +  )
>> +{
>> +  UINTN                            PoolPages;
>> +  VOID                             *Buffer;
>> +  VOID                             *Start;
>> +  PAGE_MAP_AND_DIRECTORY_POINTER   *PageMapLevel4Entry;
>> +  PAGE_TABLE_1G_ENTRY              *PageDirectory1GEntry;
>> +  PAGE_TABLE_1G_ENTRY              *Unenc1GEntry;
>> +  UINT64                           AddressEncMask;
>> +
>> +  PoolPages = 1 + 10;
>> +  Buffer = AllocateAlignedRuntimePages (PoolPages, PAGE_TABLE_POOL_ALIGNMENT);
>> +  mPageTablePool = Buffer;
>> +  mPageTablePool->NextPool = mPageTablePool;
>> +  mPageTablePool->FreePages  = PoolPages - 1;
>> +  mPageTablePool->Offset = EFI_PAGES_TO_SIZE (1);
>> +
>> +  Start = (UINT8 *)mPageTablePool + mPageTablePool->Offset;
>> +  ZeroMem(Start, mPageTablePool->FreePages * EFI_PAGE_SIZE);
>> +
>> +  AddressEncMask = 1ULL << 47;
>> +
> 
> Preferably getting the encryption bit location from SEV CPUID
> information.

You're right; I'll fix.


> 
>> +  PageMapLevel4Entry = Start;
>> +  PageDirectory1GEntry = (PAGE_TABLE_1G_ENTRY*)((UINT8*)Start + EFI_PAGE_SIZE);
>> +  Unenc1GEntry = (PAGE_TABLE_1G_ENTRY*)((UINT8*)Start + 2 * EFI_PAGE_SIZE);
>> +
>> +  PageMapLevel4Entry = Start;
>> +  PageMapLevel4Entry += PML4_OFFSET(0x0ULL);
>> +  PageMapLevel4Entry->Uint64 = (UINT64)PageDirectory1GEntry | AddressEncMask | 0x23;
>> +
>> +  PageMapLevel4Entry = Start;
>> +  PageMapLevel4Entry += PML4_OFFSET(UNENC_VIRT_ADDR_BASE); // should be 511
>> +  PageMapLevel4Entry->Uint64 = (UINT64)Unenc1GEntry | AddressEncMask | 0x23;
>> +
>> +  UINT64 PageAddr = 0;
>> +  for (int i = 0; i < 512; i++, PageAddr += SIZE_1GB) {
>> +    PAGE_TABLE_1G_ENTRY *e = PageDirectory1GEntry + i;
>> +    e->Uint64 = PageAddr | AddressEncMask | 0xe3; // 1GB page
>> +  }
>> +
> 
> Changing encryption attributes of a page requires to flush it from
> the caches, you may need to do a clflush here.

Thanks for pointing that out.  I think I didn't see that in the code that
builds the page tables for encrypted memory (which inspired most of this
function), but maybe I missed it.


Thanks,
Dov


> 
> Thanks,
> Ashish
> 
>> +  UINT64 UnencPageAddr = 0;
>> +  Unenc1GEntry->Uint64 = UnencPageAddr | 0xe3; // 1GB page unencrypted
>> +
>> +  mMigrationHelperPageTables = (UINT64)Start | AddressEncMask;
>> +}
>> +
>> +VOID
>> +SwitchToMigrationHelperPageTables(VOID)
>> +{
>> +  AsmWriteCr3(mMigrationHelperPageTables);
>> +}
>> +
>> +
>>  
>>  VOID
>>  EFIAPI
>> @@ -56,7 +134,12 @@ MigrationHandlerMain (
>>  
>>    DebugPrint (DEBUG_INFO,"MIGRATION Handler Started\n");
>>  
>> -  params_base = PcdGet32 (PcdConfidentialMigrationMailboxBase);
>> +  SwitchToMigrationHelperPageTables();
>> +
>> +  //
>> +  // Shared pages must be offset by UNENC_VIRT_ADDR_BASE.
>> +  //
>> +  params_base = PcdGet32 (PcdConfidentialMigrationMailboxBase) + UNENC_VIRT_ADDR_BASE;
>>    params = (VOID *)params_base;
>>    page_va = (VOID *)params_base + 0x1000;
>>  
>> @@ -134,6 +217,8 @@ LaunchMigrationHandler (
>>  
>>    MigrationHandlerCpuIndex = NumProc - 1;
>>  
>> +  PrepareMigrationHandlerPageTables();
>> +
>>    EFI_EVENT Event;
>>    MpProto->GetProcessorInfo (MpProto, MigrationHandlerCpuIndex, &Tcb);
>>    if (Tcb.StatusFlag != 7) {
>> @@ -154,6 +239,7 @@ LaunchMigrationHandler (
>>    if (PcdGetBool(PcdIsConfidentialMigrationTarget)) {
>>      DebugPrint (DEBUG_INFO,"Waiting for incoming confidential migration.\n");
>>      DisableInterrupts ();
>> +    SwitchToMigrationHelperPageTables();
>>      CpuDeadLoop ();
>>    }
>>  
>> -- 
>> 2.20.1
>>

Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live Migration for AMD SEV

[Yao, Jiewen]

Hi Tobin
Thanks for your patch.
You may that Intel is working on TDX for the same live migration feature.

Please give me some time (about 1 work week) to digest and evaluate the patch and impact.
Then I will provide feedback.

Thank you
Yao Jiewen

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Tobin
> Feldman-Fitzthum
> Sent: Wednesday, March 3, 2021 4:48 AM
> To: devel@edk2.groups.io
> Cc: Dov Murik <dovmurik@linux.vnet.ibm.com>; Tobin Feldman-Fitzthum
> <tobin@ibm.com>; Tobin Feldman-Fitzthum <tobin@linux.ibm.com>; James
> Bottomley <jejb@linux.ibm.com>; Hubertus Franke <frankeh@us.ibm.com>;
> Brijesh Singh <brijesh.singh@amd.com>; Ashish Kalra <ashish.kalra@amd.com>;
> Jon Grimm <jon.grimm@amd.com>; Tom Lendacky
> <thomas.lendacky@amd.com>
> Subject: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live
> Migration for AMD SEV
> 
> This is a demonstration of fast migration for encrypted virtual machines
> using a Migration Handler that lives in OVMF. This demo uses AMD SEV,
> but the ideas may generalize to other confidential computing platforms.
> With AMD SEV, guest memory is encrypted and the hypervisor cannot access
> or move it. This makes migration tricky. In this demo, we show how the
> HV can ask a Migration Handler (MH) in the firmware for an encrypted
> page. The MH encrypts the page with a transport key prior to releasing
> it to the HV. The target machine also runs an MH that decrypts the page
> once it is passed in by the target HV. These patches are not ready for
> production, but the are a full end-to-end solution that facilitates a
> fast live migration between two SEV VMs.
> 
> Corresponding patches for QEMU have been posted my colleague Dov Murik
> on qemu-devel. Our approach needs little kernel support, requiring only
> one hypercall that the guest can use to mark a page as encrypted or
> shared. This series includes updated patches from Ashish Kalra and
> Brijesh Singh that allow OVMF to use this hypercall.
> 
> The MH runs continuously in the guest, waiting for communication from
> the HV. The HV starts an additional vCPU for the MH but does not expose
> it to the guest OS via ACPI. We use the MpService to start the MH. The
> MpService is only available at runtime and processes that are started by
> it are usually cleaned up on ExitBootServices. Since we need the MH to
> run continuously, we had to make some modifications. Ideally a feature
> could be added to the MpService to allow for the starting of
> long-running processes. Besides migration, this could support other
> background processes that need to operate within the encryption
> boundary. For now, we have included a handful of patches that modify the
> MpService to allow the MH to keep running after ExitBootServices. These
> are temporary.
> 
> Ashish Kalra (2):
>   OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap.
>   OvmfPkg/PlatformDxe: Add support for SEV live migration.
> 
> Brijesh Singh (1):
>   OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall
> 
> Dov Murik (1):
>   OvmfPkg/AmdSev: Build page table for migration handler
> 
> Tobin Feldman-Fitzthum (10):
>   OvmfPkg/AmdSev: Base for Confidential Migration Handler
>   OvmfPkg/PlatfomPei: Set Confidential Migration PCD
>   OvmfPkg/AmdSev: Setup Migration Handler Mailbox
>   OvmfPkg/AmdSev: MH support for mailbox protocol
>   UefiCpuPkg/MpInitLib: temp removal of MpLib cleanup
>   UefiCpuPkg/MpInitLib: Allocate MP buffer as runtime memory
>   UefiCpuPkg/CpuExceptionHandlerLib: Exception handling as runtime
>     memory
>   OvmfPkg/AmdSev: Don't overwrite mailbox or pagetables
>   OvmfPkg/AmdSev: Don't overwrite MH stack
>   OvmfPkg/AmdSev: MH page encryption POC
> 
>  OvmfPkg/OvmfPkg.dec                           |  11 +
>  OvmfPkg/AmdSev/AmdSevX64.dsc                  |   2 +
>  OvmfPkg/AmdSev/AmdSevX64.fdf                  |  13 +-
>  .../ConfidentialMigrationDxe.inf              |  45 +++
>  .../ConfidentialMigrationPei.inf              |  35 ++
>  .../DxeMemEncryptSevLib.inf                   |   1 +
>  .../PeiMemEncryptSevLib.inf                   |   1 +
>  OvmfPkg/PlatformDxe/Platform.inf              |   2 +
>  OvmfPkg/PlatformPei/PlatformPei.inf           |   2 +
>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   2 +
>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   2 +
>  OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h  | 235 +++++++++++++
>  .../ConfidentialMigration/VirtualMemory.h     | 177 ++++++++++
>  OvmfPkg/Include/Guid/MemEncryptLib.h          |  16 +
>  OvmfPkg/PlatformDxe/PlatformConfig.h          |   5 +
>  .../ConfidentialMigrationDxe.c                | 325 ++++++++++++++++++
>  .../ConfidentialMigrationPei.c                |  25 ++
>  .../X64/PeiDxeVirtualMemory.c                 |  18 +
>  OvmfPkg/PlatformDxe/AmdSev.c                  |  99 ++++++
>  OvmfPkg/PlatformDxe/Platform.c                |   6 +
>  OvmfPkg/PlatformPei/AmdSev.c                  |  10 +
>  OvmfPkg/PlatformPei/Platform.c                |  10 +
>  .../CpuExceptionHandlerLib/DxeException.c     |   8 +-
>  UefiCpuPkg/Library/MpInitLib/DxeMpLib.c       |  21 +-
>  UefiCpuPkg/Library/MpInitLib/MpLib.c          |   7 +-
>  25 files changed, 1061 insertions(+), 17 deletions(-)
>  create mode 100644
> OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
>  create mode 100644
> OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.inf
>  create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h
>  create mode 100644
> OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h
>  create mode 100644 OvmfPkg/Include/Guid/MemEncryptLib.h
>  create mode 100644
> OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
>  create mode 100644
> OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.c
>  create mode 100644 OvmfPkg/PlatformDxe/AmdSev.c
> 
> --
> 2.20.1
> 
> 
> 
> 
> 

Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live Migration for AMD SEV

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 965 bytes --]

Hi Tobin,

as mentioned in the reply to the QEMU patches posted by Tobin, I think the firmware helper approach is very good, but there are some disadvantages in the idea of auxiliary vCPUs. These are especially true in the VMM, where it's much nicer to have a separate VM that goes through a specialized run loop; however, even in the firmware level there are some complications (as you pointed out) in letting MpService workers run after ExitBootServices.

My idea would be that the firmware would start the VM as usual using the same launch data; then, the firmware would detect it was running as a migration helper VM during the SEC or PEI phases (for example via the GHCB or some other unencrypted communication area), and divert execution to the migration helper instead of proceeding to the next boot phase. This would be somewhat similar in spirit to how edk2 performs S3 resume, if my memory serves correctly.

What do you think?

Thanks,

Paolo

[-- Attachment #2: Type: text/html, Size: 1005 bytes --]

Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live Migration for AMD SEV

[Laszlo Ersek]

On 03/03/21 19:25, Tobin Feldman-Fitzthum wrote:
>> Laszlo wrote:

>> I'm quite uncomfortable with an attempt to hide a CPU from the OS via
>> ACPI. The OS has other ways to learn (for example, a boot loader could
>> use the MP services itself, stash the information, and hand it to the OS
>> kernel -- this would minimally allow for detecting an inconsistency in
>> the OS). What about "all-but-self" IPIs too -- the kernel might think
>> all the processors it's poking like that were under its control.
> 
> This might be the second most controversial piece. Here's a question: if
> we could successfully hide the MH vCPU from the OS, would it still make
> you uncomfortable? In other words, is the worry that there might be some
> inconsistency or more generally that there is something hidden from the
> OS?

(1) My personal concern is the consistency aspect. In *some* parts of
the firmware, we'd rely on the hidden CPU to behave as a "logical
execution unit" (because we want it to run the MH), but in other parts
of the firmware, we'd expect it to be hidden. (Consider what
EFI_MP_SERVICES_PROTOCOL.StartupAllAPs() should do while the MH is
running!) And then the CPU should be hidden from the OS completely, even
if the OS doesn't rely on ACPI, but massages LAPIC stuff that is
architecturally specified.

In other words, we'd have to treat this processor as a "service
processor", outside of the "normal" (?) processor domain -- basically
what the PSP is right now. I don't have the slightest idea how physical
firmware deals with service processors in general. I'm really scared of
the many possible corner cases (CPU hot(un)plug, NUMA proximity, ...)

(2) I expect kernel developers to have concerns about a firmware-level
"background job" at OS runtime. SMM does something similar (periodic or
otherwise hardware-initiated async SMIs etc), and kernel developers
already dislike those (latency spikes, messing with hardware state...).


> One thing to think about is that the guest owner should generally be
> aware that there is a migration handler running. The way I see it, a
> guest owner of an SEV VM would need to opt-in to migration and should
> then expect that there is an MH running even if they aren't able to see
> it. Of course we need to be certain that the MH isn't going to break the
> OS.

I didn't think of the guest owner, but the developers that work on
(possibly unrelated parts of) the guest kernel.


> 
>> Also, as far as I can tell from patch #7, the AP seems to be
>> busy-looping (with a CpuPause() added in), for the entire lifetime of
>> the OS. Do I understand right? If so -- is it a temporary trait as well?
> 
> In our approach the MH continuously checks for commands from the
> hypervisor. There are potentially ways to optimize this, such as having
> the hypervisor de-schedule the MH vCPU while not migrating. You could
> potentially shut down down the MH on the target after receiving the
> MH_RESET command (when the migration finishes), but what if you want to
> migrate that VM somewhere else?

I have no idea.

In the current world, de-scheduling a particular VCPU for extended
periods of time is a bad idea (stolen time goes up, ticks get lost, ...)
So I guess this would depend on how well you could "hide" the service
processor from the guest kernel.


I'd really like if we could rely on an established "service processor"
methodology, in the guest. Physical platform vendors have used service
processors for ages, the firmwares on those platforms (on the main
boards) do manage the service processors, and the service processors are
hidden from the OS too (beyond specified access methods, if any).

My understanding (or assumption) is that such a service processor is
primarily a separate entity (you cannot talk to them "unintentionally",
for example with an All-But-Self IPI), and that it's reachable only with
specific access methods. I think the AMD PSP itself might follow this
approach (AIUI it's an aarch64 CPU on an otherwise Intel/AMD arch platform).

I'd like us to benefit from a crystallized "service processor"
abstraction, if possible. I apologize that I'm this vague -- I've never
seen such firmware code that deals with a service processor, I just
assume it exists.

Thanks
Laszlo


> 
>>
>> Sorry if my questions are "premature", in the sense that I could get my
>> own answers as well if I actually read the patches in detail -- however,
>> I wouldn't like to do that at once, because then I'll be distracted by
>> many style issues and other "trivial" stuff. Examples for the latter:
> 
> Not premature at all. I think you hit the nail on the head with
> everything you raised.
> 
> -Tobin
> 
>>
>> - patch#1 calls SetMemoryEncDecHypercall3(), but there is no such
>> function in edk2, so minimally it's a patch ordering bug in the series,
>>
>> - in patch#1, there's minimally one whitespace error (no whitespace
>> right after "EFI_SIZE_TO_PAGES")
>>
>> - in patch#1, the alphabetical ordering in the [LibraryClasses] section,
>> and in the matching #include directives, gets broken,
>>
>> - I'd prefer if the "SevLiveMigrationEnabled" UEFI variable were set in
>> ConfidentialMigrationDxe, rather than PlatformDxe (patch #3), or at
>> least another AMD SEV related DXE driver (OvmfPkg/AmdSevDxe etc).
>>
>> - any particular reasonf or making the UEFI variable non-volatile? I
>> don't think it should survive any particular boot of the guest.
>>
>> - Why do we need a variable in the first place?
>>
>> etc etc
>>
>> Thanks!
>> Laszlo
>>
>>
>>
>>
>>> Ashish Kalra (2):
>>>    OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion
>>> bitmap.
>>>    OvmfPkg/PlatformDxe: Add support for SEV live migration.
>>>
>>> Brijesh Singh (1):
>>>    OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall
>>>
>>> Dov Murik (1):
>>>    OvmfPkg/AmdSev: Build page table for migration handler
>>>
>>> Tobin Feldman-Fitzthum (10):
>>>    OvmfPkg/AmdSev: Base for Confidential Migration Handler
>>>    OvmfPkg/PlatfomPei: Set Confidential Migration PCD
>>>    OvmfPkg/AmdSev: Setup Migration Handler Mailbox
>>>    OvmfPkg/AmdSev: MH support for mailbox protocol
>>>    UefiCpuPkg/MpInitLib: temp removal of MpLib cleanup
>>>    UefiCpuPkg/MpInitLib: Allocate MP buffer as runtime memory
>>>    UefiCpuPkg/CpuExceptionHandlerLib: Exception handling as runtime
>>>      memory
>>>    OvmfPkg/AmdSev: Don't overwrite mailbox or pagetables
>>>    OvmfPkg/AmdSev: Don't overwrite MH stack
>>>    OvmfPkg/AmdSev: MH page encryption POC
>>>
>>>   OvmfPkg/OvmfPkg.dec                           |  11 +
>>>   OvmfPkg/AmdSev/AmdSevX64.dsc                  |   2 +
>>>   OvmfPkg/AmdSev/AmdSevX64.fdf                  |  13 +-
>>>   .../ConfidentialMigrationDxe.inf              |  45 +++
>>>   .../ConfidentialMigrationPei.inf              |  35 ++
>>>   .../DxeMemEncryptSevLib.inf                   |   1 +
>>>   .../PeiMemEncryptSevLib.inf                   |   1 +
>>>   OvmfPkg/PlatformDxe/Platform.inf              |   2 +
>>>   OvmfPkg/PlatformPei/PlatformPei.inf           |   2 +
>>>   UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   2 +
>>>   UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   2 +
>>>   OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h  | 235 +++++++++++++
>>>   .../ConfidentialMigration/VirtualMemory.h     | 177 ++++++++++
>>>   OvmfPkg/Include/Guid/MemEncryptLib.h          |  16 +
>>>   OvmfPkg/PlatformDxe/PlatformConfig.h          |   5 +
>>>   .../ConfidentialMigrationDxe.c                | 325 ++++++++++++++++++
>>>   .../ConfidentialMigrationPei.c                |  25 ++
>>>   .../X64/PeiDxeVirtualMemory.c                 |  18 +
>>>   OvmfPkg/PlatformDxe/AmdSev.c                  |  99 ++++++
>>>   OvmfPkg/PlatformDxe/Platform.c                |   6 +
>>>   OvmfPkg/PlatformPei/AmdSev.c                  |  10 +
>>>   OvmfPkg/PlatformPei/Platform.c                |  10 +
>>>   .../CpuExceptionHandlerLib/DxeException.c     |   8 +-
>>>   UefiCpuPkg/Library/MpInitLib/DxeMpLib.c       |  21 +-
>>>   UefiCpuPkg/Library/MpInitLib/MpLib.c          |   7 +-
>>>   25 files changed, 1061 insertions(+), 17 deletions(-)
>>>   create mode 100644
>>> OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
>>>   create mode 100644
>>> OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.inf
>>>   create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h
>>>   create mode 100644
>>> OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h
>>>   create mode 100644 OvmfPkg/Include/Guid/MemEncryptLib.h
>>>   create mode 100644
>>> OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
>>>   create mode 100644
>>> OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.c
>>>   create mode 100644 OvmfPkg/PlatformDxe/AmdSev.c
>>>
> 

Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live Migration for AMD SEV

[Laszlo Ersek]

On 03/04/21 10:21, Paolo Bonzini wrote:
> Hi Tobin,
> 
> as mentioned in the reply to the QEMU patches posted by Tobin, I
> think the firmware helper approach is very good, but there are some
> disadvantages in the idea of auxiliary vCPUs. These are especially
> true in the VMM, where it's much nicer to have a separate VM that
> goes through a specialized run loop; however, even in the firmware
> level there are some complications (as you pointed out) in letting
> MpService workers run after ExitBootServices.
> 
> My idea would be that the firmware would start the VM as usual using
> the same launch data; then, the firmware would detect it was running
> as a migration helper VM during the SEC or PEI phases (for example
> via the GHCB or some other unencrypted communication area), and
> divert execution to the migration helper instead of proceeding to the
> next boot phase. This would be somewhat similar in spirit to how edk2
> performs S3 resume, if my memory serves correctly.

Very cool. You'd basically warm-reboot the virtual machine into a new
boot mode (cf. BOOT_WITH_FULL_CONFIGURATION vs. BOOT_ON_S3_RESUME in
OvmfPkg/PlatformPei).

To me that's much more attractive than a "background job".

The S3 parallel is great. What I'm missing is:

- Is it possible to warm-reboot an SEV VM? (I vaguely recall that it's
not possible for SEV-ES at least.) Because, that's how we'd transfer
control to the early parts of the firmware again, IIUC your idea, while
preserving the memory contents.

- Who would initiate this process? S3 suspend is guest-initiated. (Not
that we couldn't use the guest agent, if needed.)

(In case the idea is really about a separate VM, and not about rebooting
the already running VM, then I don't understand -- how would a separate
VM access the guest RAM that needs to be migrated?)

NB in the X64 PEI phase of OVMF, only the first 4GB of RAM is mapped, so
the migration handler would have to build its own page table under this
approach too.

Thanks!
Laszlo

Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live Migration for AMD SEV

[Laszlo Ersek]

On 03/04/21 21:45, Laszlo Ersek wrote:
> On 03/04/21 10:21, Paolo Bonzini wrote:
>> Hi Tobin,
>>
>> as mentioned in the reply to the QEMU patches posted by Tobin, I
>> think the firmware helper approach is very good, but there are some
>> disadvantages in the idea of auxiliary vCPUs. These are especially
>> true in the VMM, where it's much nicer to have a separate VM that
>> goes through a specialized run loop; however, even in the firmware
>> level there are some complications (as you pointed out) in letting
>> MpService workers run after ExitBootServices.
>>
>> My idea would be that the firmware would start the VM as usual using
>> the same launch data; then, the firmware would detect it was running
>> as a migration helper VM during the SEC or PEI phases (for example
>> via the GHCB or some other unencrypted communication area), and
>> divert execution to the migration helper instead of proceeding to the
>> next boot phase. This would be somewhat similar in spirit to how edk2
>> performs S3 resume, if my memory serves correctly.
> 
> Very cool. You'd basically warm-reboot the virtual machine into a new
> boot mode (cf. BOOT_WITH_FULL_CONFIGURATION vs. BOOT_ON_S3_RESUME in
> OvmfPkg/PlatformPei).
> 
> To me that's much more attractive than a "background job".
> 
> The S3 parallel is great. What I'm missing is:
> 
> - Is it possible to warm-reboot an SEV VM? (I vaguely recall that it's
> not possible for SEV-ES at least.) Because, that's how we'd transfer
> control to the early parts of the firmware again, IIUC your idea, while
> preserving the memory contents.
> 
> - Who would initiate this process? S3 suspend is guest-initiated. (Not
> that we couldn't use the guest agent, if needed.)
> 
> (In case the idea is really about a separate VM, and not about rebooting
> the already running VM, then I don't understand -- how would a separate
> VM access the guest RAM that needs to be migrated?)

Sorry -- I've just caught up with the QEMU thread. Your message there:

  https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg01220.html

says:

  Patches were posted recently to the KVM mailing list to create
  secondary VMs sharing the encryption context (ASID) with a primary VM

I did think of VMs sharing memory, but the goal of SEV seemed to be to
prevent exactly that, so I didn't think that was possible. I stand
corrected, and yes, this way I understand -- and welcome -- a completely
separate VM snooping the migration subject VM's memory.

My question would be then whether the migration helper VM would run on
its own memory, and just read out the other VM's memory -- or the MH VM
would run somewhere inside the original VM's memory (which sounds a lot
riskier). But your message explains that too:

  The main advantage would be that the migration VM would not have to
  share the address space with the primary VM

This sounds ideal; it should allow for a completely independent firmware
platform -- we wouldn't even have to call it "OVMF", and it might not
even have to contain the DXE Core and later-phase components. (Of course
if it's more convenient to keep the stuff in OVMF, that works too.)

(For some unsolicited personal information, now I feel less bad about
this idea never occurring to me -- I never knew about the KVM patch set
that would enable encryption context sharing. (TBH I thought that was
prevented, by design, in the SEV hardware...))


A workflow request to Tobin and Dov -- when posting closely interfacing
QEMU and edk2 series, it's best to cross-post both series to both lists,
and to CC everybody on everything. Feel free to use subject prefixes
like [qemu PATCH] and [edk2 PATCH] for clarity. It's been difficult for
me to follow both discussions (it doesn't help that I've been CC'd on
neither).

Thanks!
Laszlo

> 
> NB in the X64 PEI phase of OVMF, only the first 4GB of RAM is mapped, so
> the migration handler would have to build its own page table under this
> approach too.
> 
> Thanks!
> Laszlo
> 

Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live Migration for AMD SEV

[Paolo Bonzini]

On 04/03/21 21:45, Laszlo Ersek wrote:
> On 03/04/21 10:21, Paolo Bonzini wrote:
>> Hi Tobin,
>>
>> as mentioned in the reply to the QEMU patches posted by Tobin, I
>> think the firmware helper approach is very good, but there are some
>> disadvantages in the idea of auxiliary vCPUs. These are especially
>> true in the VMM, where it's much nicer to have a separate VM that
>> goes through a specialized run loop; however, even in the firmware
>> level there are some complications (as you pointed out) in letting
>> MpService workers run after ExitBootServices.
>>
>> My idea would be that the firmware would start the VM as usual using
>> the same launch data; then, the firmware would detect it was running
>> as a migration helper VM during the SEC or PEI phases (for example
>> via the GHCB or some other unencrypted communication area), and
>> divert execution to the migration helper instead of proceeding to the
>> next boot phase. This would be somewhat similar in spirit to how edk2
>> performs S3 resume, if my memory serves correctly.
> 
> Very cool. You'd basically warm-reboot the virtual machine into a new
> boot mode (cf. BOOT_WITH_FULL_CONFIGURATION vs. BOOT_ON_S3_RESUME in
> OvmfPkg/PlatformPei).
> 
> To me that's much more attractive than a "background job".
> 
> The S3 parallel is great. What I'm missing is:
> 
> - Is it possible to warm-reboot an SEV VM? (I vaguely recall that it's
> not possible for SEV-ES at least.) Because, that's how we'd transfer
> control to the early parts of the firmware again, IIUC your idea, while
> preserving the memory contents.

It's not exactly a warm reboot.  It's two VMs booted at the same time, 
with exactly the same contents as far as encrypted RAM goes, but 
different unencrypted RAM.  The difference makes one VM boot regularly 
and the other end up in the migration helper.  The migration helper can 
be entirely contained in PEI, or it can even be its own OS, stored as a 
flat binary in the firmware.  Whatever is easier.

The divergence would happen much earlier than S3 though.  It would have 
to happen before the APs are brought up, for example, and essentially 
before the first fw_cfg access if (as is likely) the migration helper VM 
does not have fw_cfg at all.  That's why I brought up the possibility of 
diverging as soon as SEC.

> - Who would initiate this process? S3 suspend is guest-initiated. (Not
> that we couldn't use the guest agent, if needed.)
> 
> (In case the idea is really about a separate VM, and not about rebooting
> the already running VM, then I don't understand -- how would a separate
> VM access the guest RAM that needs to be migrated?)

Answering the other message:

> (For some unsolicited personal information, now I feel less bad about
> this idea never occurring to me -- I never knew about the KVM patch set
> that would enable encryption context sharing. (TBH I thought that was
> prevented, by design, in the SEV hardware...))

As far as the SEV hardware is concerned, a "VM" is defined by the ASID.

The VM would be separate at the KVM level, but it would share the ASID 
(and thus the guest RAM) with the primary VM.  So as far as the SEV 
hardware and the processor are concerned, the separate VM would be just 
one more VMCB that runs with that ASID.  Only KVM knows that they are 
backed by different file descriptors etc.

In fact, another advantage is that it would be much easier to scale the 
migration helper to multiple vCPUs.  This is probably also a case for 
diverging much earlier than PEI, because a multi-processor migration 
helper running in PEI or DXE would require ACPI tables and a lot of 
infrastructure that is probably undesirable.

Paolo

Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live Migration for AMD SEV

[Ashish Kalra]

On Wed, Mar 03, 2021 at 01:25:40PM -0500, Tobin Feldman-Fitzthum wrote:
> 
> > Hi Tobin,
> > 
> > On 03/02/21 21:48, Tobin Feldman-Fitzthum wrote:
> > > This is a demonstration of fast migration for encrypted virtual machines
> > > using a Migration Handler that lives in OVMF. This demo uses AMD SEV,
> > > but the ideas may generalize to other confidential computing platforms.
> > > With AMD SEV, guest memory is encrypted and the hypervisor cannot access
> > > or move it. This makes migration tricky. In this demo, we show how the
> > > HV can ask a Migration Handler (MH) in the firmware for an encrypted
> > > page. The MH encrypts the page with a transport key prior to releasing
> > > it to the HV. The target machine also runs an MH that decrypts the page
> > > once it is passed in by the target HV. These patches are not ready for
> > > production, but the are a full end-to-end solution that facilitates a
> > > fast live migration between two SEV VMs.
> > > 
> > > Corresponding patches for QEMU have been posted my colleague Dov Murik
> > > on qemu-devel. Our approach needs little kernel support, requiring only
> > > one hypercall that the guest can use to mark a page as encrypted or
> > > shared. This series includes updated patches from Ashish Kalra and
> > > Brijesh Singh that allow OVMF to use this hypercall.
> > > 
> > > The MH runs continuously in the guest, waiting for communication from
> > > the HV. The HV starts an additional vCPU for the MH but does not expose
> > > it to the guest OS via ACPI. We use the MpService to start the MH. The
> > > MpService is only available at runtime and processes that are started by
> > > it are usually cleaned up on ExitBootServices. Since we need the MH to
> > > run continuously, we had to make some modifications. Ideally a feature
> > > could be added to the MpService to allow for the starting of
> > > long-running processes. Besides migration, this could support other
> > > background processes that need to operate within the encryption
> > > boundary. For now, we have included a handful of patches that modify the
> > > MpService to allow the MH to keep running after ExitBootServices. These
> > > are temporary.
> > I plan to do a lightweight review for this series. (My understanding is
> > that it's an RFC and not actually being proposed for merging.)
> > 
> > Regarding the MH's availability at runtime -- does that necessarily
> > require the isolation of an AP? Because in the current approach,
> > allowing the MP Services to survive into OS runtime (in some form or
> > another) seems critical, and I don't think it's going to fly.
> > 
> > I agree that the UefiCpuPkg patches have been well separated from the
> > rest of the series, but I'm somewhat doubtful the "firmware-initiated
> > background process" idea will be accepted. Have you investigated
> > exposing a new "runtime service" (a function pointer) via the UEFI
> > Configuration table, and calling that (perhaps periodically?) from the
> > guest kernel? It would be a form of polling I guess. Or maybe, poll the
> > mailbox directly in the kernel, and call the new firmware runtime
> > service when there's an actual command to process.
> Continuous runtime availability for the MH is almost certainly the most
> controversial part of this proposal, which is why I put it in the cover
> letter and why it's good to discuss.
> > (You do spell out "little kernel support", and I'm not sure if that's a
> > technical benefit, or a political / community benefit.)
> 
> As you allude to, minimal kernel support is really one of the main things
> that shapes our approach. This is partly a political and practical benefit,
> but there are also technical benefits. Having the MH in firmware likely
> leads to higher availability. It can be accessed when the OS is unreachable,
> perhaps during boot or when the OS is hung. There are also potential
> portability advantages although we do currently require support for one
> hypercall. The cost of implementing this hypercall is low.
> 
> Generally speaking, our task is to find a home for functionality that was
> traditionally provided by the hypervisor, but that needs to be inside the
> trust domain, but that isn't really part of a guest. A meta-goal of this
> project is to figure out the best way to do this.
> 
> > 
> > I'm quite uncomfortable with an attempt to hide a CPU from the OS via
> > ACPI. The OS has other ways to learn (for example, a boot loader could
> > use the MP services itself, stash the information, and hand it to the OS
> > kernel -- this would minimally allow for detecting an inconsistency in
> > the OS). What about "all-but-self" IPIs too -- the kernel might think
> > all the processors it's poking like that were under its control.
> 
> This might be the second most controversial piece. Here's a question: if we
> could successfully hide the MH vCPU from the OS, would it still make you
> uncomfortable? In other words, is the worry that there might be some
> inconsistency or more generally that there is something hidden from the OS?
> One thing to think about is that the guest owner should generally be aware
> that there is a migration handler running. The way I see it, a guest owner
> of an SEV VM would need to opt-in to migration and should then expect that
> there is an MH running even if they aren't able to see it. Of course we need
> to be certain that the MH isn't going to break the OS.
> 
> > Also, as far as I can tell from patch #7, the AP seems to be
> > busy-looping (with a CpuPause() added in), for the entire lifetime of
> > the OS. Do I understand right? If so -- is it a temporary trait as well?
> 
> In our approach the MH continuously checks for commands from the hypervisor.
> There are potentially ways to optimize this, such as having the hypervisor
> de-schedule the MH vCPU while not migrating. You could potentially shut down
> down the MH on the target after receiving the MH_RESET command (when the
> migration finishes), but what if you want to migrate that VM somewhere else?
> 

I think another approach can be considered here, why not implement MH
vCPU(s) as hot-plugged vCPU(s), basically hot-plug a new vCPU when migration
is started and hot unplug the vCPU when migration is completed, then we
won't need a vCPU running (and potentially consuming cycles) forever and
busy-looping with CpuPause(). 

Thanks,
Ashish

Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live Migration for AMD SEV

[Ashish Kalra]

On Fri, Mar 05, 2021 at 10:44:23AM +0000, Ashish Kalra wrote:
> On Wed, Mar 03, 2021 at 01:25:40PM -0500, Tobin Feldman-Fitzthum wrote:
> > 
> > > Hi Tobin,
> > > 
> > > On 03/02/21 21:48, Tobin Feldman-Fitzthum wrote:
> > > > This is a demonstration of fast migration for encrypted virtual machines
> > > > using a Migration Handler that lives in OVMF. This demo uses AMD SEV,
> > > > but the ideas may generalize to other confidential computing platforms.
> > > > With AMD SEV, guest memory is encrypted and the hypervisor cannot access
> > > > or move it. This makes migration tricky. In this demo, we show how the
> > > > HV can ask a Migration Handler (MH) in the firmware for an encrypted
> > > > page. The MH encrypts the page with a transport key prior to releasing
> > > > it to the HV. The target machine also runs an MH that decrypts the page
> > > > once it is passed in by the target HV. These patches are not ready for
> > > > production, but the are a full end-to-end solution that facilitates a
> > > > fast live migration between two SEV VMs.
> > > > 
> > > > Corresponding patches for QEMU have been posted my colleague Dov Murik
> > > > on qemu-devel. Our approach needs little kernel support, requiring only
> > > > one hypercall that the guest can use to mark a page as encrypted or
> > > > shared. This series includes updated patches from Ashish Kalra and
> > > > Brijesh Singh that allow OVMF to use this hypercall.
> > > > 
> > > > The MH runs continuously in the guest, waiting for communication from
> > > > the HV. The HV starts an additional vCPU for the MH but does not expose
> > > > it to the guest OS via ACPI. We use the MpService to start the MH. The
> > > > MpService is only available at runtime and processes that are started by
> > > > it are usually cleaned up on ExitBootServices. Since we need the MH to
> > > > run continuously, we had to make some modifications. Ideally a feature
> > > > could be added to the MpService to allow for the starting of
> > > > long-running processes. Besides migration, this could support other
> > > > background processes that need to operate within the encryption
> > > > boundary. For now, we have included a handful of patches that modify the
> > > > MpService to allow the MH to keep running after ExitBootServices. These
> > > > are temporary.
> > > I plan to do a lightweight review for this series. (My understanding is
> > > that it's an RFC and not actually being proposed for merging.)
> > > 
> > > Regarding the MH's availability at runtime -- does that necessarily
> > > require the isolation of an AP? Because in the current approach,
> > > allowing the MP Services to survive into OS runtime (in some form or
> > > another) seems critical, and I don't think it's going to fly.
> > > 
> > > I agree that the UefiCpuPkg patches have been well separated from the
> > > rest of the series, but I'm somewhat doubtful the "firmware-initiated
> > > background process" idea will be accepted. Have you investigated
> > > exposing a new "runtime service" (a function pointer) via the UEFI
> > > Configuration table, and calling that (perhaps periodically?) from the
> > > guest kernel? It would be a form of polling I guess. Or maybe, poll the
> > > mailbox directly in the kernel, and call the new firmware runtime
> > > service when there's an actual command to process.
> > Continuous runtime availability for the MH is almost certainly the most
> > controversial part of this proposal, which is why I put it in the cover
> > letter and why it's good to discuss.
> > > (You do spell out "little kernel support", and I'm not sure if that's a
> > > technical benefit, or a political / community benefit.)
> > 
> > As you allude to, minimal kernel support is really one of the main things
> > that shapes our approach. This is partly a political and practical benefit,
> > but there are also technical benefits. Having the MH in firmware likely
> > leads to higher availability. It can be accessed when the OS is unreachable,
> > perhaps during boot or when the OS is hung. There are also potential
> > portability advantages although we do currently require support for one
> > hypercall. The cost of implementing this hypercall is low.
> > 
> > Generally speaking, our task is to find a home for functionality that was
> > traditionally provided by the hypervisor, but that needs to be inside the
> > trust domain, but that isn't really part of a guest. A meta-goal of this
> > project is to figure out the best way to do this.
> > 
> > > 
> > > I'm quite uncomfortable with an attempt to hide a CPU from the OS via
> > > ACPI. The OS has other ways to learn (for example, a boot loader could
> > > use the MP services itself, stash the information, and hand it to the OS
> > > kernel -- this would minimally allow for detecting an inconsistency in
> > > the OS). What about "all-but-self" IPIs too -- the kernel might think
> > > all the processors it's poking like that were under its control.
> > 
> > This might be the second most controversial piece. Here's a question: if we
> > could successfully hide the MH vCPU from the OS, would it still make you
> > uncomfortable? In other words, is the worry that there might be some
> > inconsistency or more generally that there is something hidden from the OS?
> > One thing to think about is that the guest owner should generally be aware
> > that there is a migration handler running. The way I see it, a guest owner
> > of an SEV VM would need to opt-in to migration and should then expect that
> > there is an MH running even if they aren't able to see it. Of course we need
> > to be certain that the MH isn't going to break the OS.
> > 
> > > Also, as far as I can tell from patch #7, the AP seems to be
> > > busy-looping (with a CpuPause() added in), for the entire lifetime of
> > > the OS. Do I understand right? If so -- is it a temporary trait as well?
> > 
> > In our approach the MH continuously checks for commands from the hypervisor.
> > There are potentially ways to optimize this, such as having the hypervisor
> > de-schedule the MH vCPU while not migrating. You could potentially shut down
> > down the MH on the target after receiving the MH_RESET command (when the
> > migration finishes), but what if you want to migrate that VM somewhere else?
> > 
> 
> I think another approach can be considered here, why not implement MH
> vCPU(s) as hot-plugged vCPU(s), basically hot-plug a new vCPU when migration
> is started and hot unplug the vCPU when migration is completed, then we
> won't need a vCPU running (and potentially consuming cycles) forever and
> busy-looping with CpuPause(). 
> 

After internal discussions, realized that this approach will not work as
vCPU hotplug will not work for SEV-ES, SNP. As the VMSA has to be
encrypted as part of the LAUNCH command, therefore we can't create/add a
new vCPU after LAUNCH has completed.

Thanks,
Ashish

Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live Migration for AMD SEV

[Tobin Feldman-Fitzthum]

> On Fri, Mar 05, 2021 at 10:44:23AM +0000, Ashish Kalra wrote:
>> On Wed, Mar 03, 2021 at 01:25:40PM -0500, Tobin Feldman-Fitzthum wrote:
>>>> Hi Tobin,
>>>>
>>>> On 03/02/21 21:48, Tobin Feldman-Fitzthum wrote:
>>>>> This is a demonstration of fast migration for encrypted virtual machines
>>>>> using a Migration Handler that lives in OVMF. This demo uses AMD SEV,
>>>>> but the ideas may generalize to other confidential computing platforms.
>>>>> With AMD SEV, guest memory is encrypted and the hypervisor cannot access
>>>>> or move it. This makes migration tricky. In this demo, we show how the
>>>>> HV can ask a Migration Handler (MH) in the firmware for an encrypted
>>>>> page. The MH encrypts the page with a transport key prior to releasing
>>>>> it to the HV. The target machine also runs an MH that decrypts the page
>>>>> once it is passed in by the target HV. These patches are not ready for
>>>>> production, but the are a full end-to-end solution that facilitates a
>>>>> fast live migration between two SEV VMs.
>>>>>
>>>>> Corresponding patches for QEMU have been posted my colleague Dov Murik
>>>>> on qemu-devel. Our approach needs little kernel support, requiring only
>>>>> one hypercall that the guest can use to mark a page as encrypted or
>>>>> shared. This series includes updated patches from Ashish Kalra and
>>>>> Brijesh Singh that allow OVMF to use this hypercall.
>>>>>
>>>>> The MH runs continuously in the guest, waiting for communication from
>>>>> the HV. The HV starts an additional vCPU for the MH but does not expose
>>>>> it to the guest OS via ACPI. We use the MpService to start the MH. The
>>>>> MpService is only available at runtime and processes that are started by
>>>>> it are usually cleaned up on ExitBootServices. Since we need the MH to
>>>>> run continuously, we had to make some modifications. Ideally a feature
>>>>> could be added to the MpService to allow for the starting of
>>>>> long-running processes. Besides migration, this could support other
>>>>> background processes that need to operate within the encryption
>>>>> boundary. For now, we have included a handful of patches that modify the
>>>>> MpService to allow the MH to keep running after ExitBootServices. These
>>>>> are temporary.
>>>> I plan to do a lightweight review for this series. (My understanding is
>>>> that it's an RFC and not actually being proposed for merging.)
>>>>
>>>> Regarding the MH's availability at runtime -- does that necessarily
>>>> require the isolation of an AP? Because in the current approach,
>>>> allowing the MP Services to survive into OS runtime (in some form or
>>>> another) seems critical, and I don't think it's going to fly.
>>>>
>>>> I agree that the UefiCpuPkg patches have been well separated from the
>>>> rest of the series, but I'm somewhat doubtful the "firmware-initiated
>>>> background process" idea will be accepted. Have you investigated
>>>> exposing a new "runtime service" (a function pointer) via the UEFI
>>>> Configuration table, and calling that (perhaps periodically?) from the
>>>> guest kernel? It would be a form of polling I guess. Or maybe, poll the
>>>> mailbox directly in the kernel, and call the new firmware runtime
>>>> service when there's an actual command to process.
>>> Continuous runtime availability for the MH is almost certainly the most
>>> controversial part of this proposal, which is why I put it in the cover
>>> letter and why it's good to discuss.
>>>> (You do spell out "little kernel support", and I'm not sure if that's a
>>>> technical benefit, or a political / community benefit.)
>>> As you allude to, minimal kernel support is really one of the main things
>>> that shapes our approach. This is partly a political and practical benefit,
>>> but there are also technical benefits. Having the MH in firmware likely
>>> leads to higher availability. It can be accessed when the OS is unreachable,
>>> perhaps during boot or when the OS is hung. There are also potential
>>> portability advantages although we do currently require support for one
>>> hypercall. The cost of implementing this hypercall is low.
>>>
>>> Generally speaking, our task is to find a home for functionality that was
>>> traditionally provided by the hypervisor, but that needs to be inside the
>>> trust domain, but that isn't really part of a guest. A meta-goal of this
>>> project is to figure out the best way to do this.
>>>
>>>> I'm quite uncomfortable with an attempt to hide a CPU from the OS via
>>>> ACPI. The OS has other ways to learn (for example, a boot loader could
>>>> use the MP services itself, stash the information, and hand it to the OS
>>>> kernel -- this would minimally allow for detecting an inconsistency in
>>>> the OS). What about "all-but-self" IPIs too -- the kernel might think
>>>> all the processors it's poking like that were under its control.
>>> This might be the second most controversial piece. Here's a question: if we
>>> could successfully hide the MH vCPU from the OS, would it still make you
>>> uncomfortable? In other words, is the worry that there might be some
>>> inconsistency or more generally that there is something hidden from the OS?
>>> One thing to think about is that the guest owner should generally be aware
>>> that there is a migration handler running. The way I see it, a guest owner
>>> of an SEV VM would need to opt-in to migration and should then expect that
>>> there is an MH running even if they aren't able to see it. Of course we need
>>> to be certain that the MH isn't going to break the OS.
>>>
>>>> Also, as far as I can tell from patch #7, the AP seems to be
>>>> busy-looping (with a CpuPause() added in), for the entire lifetime of
>>>> the OS. Do I understand right? If so -- is it a temporary trait as well?
>>> In our approach the MH continuously checks for commands from the hypervisor.
>>> There are potentially ways to optimize this, such as having the hypervisor
>>> de-schedule the MH vCPU while not migrating. You could potentially shut down
>>> down the MH on the target after receiving the MH_RESET command (when the
>>> migration finishes), but what if you want to migrate that VM somewhere else?
>>>
>> I think another approach can be considered here, why not implement MH
>> vCPU(s) as hot-plugged vCPU(s), basically hot-plug a new vCPU when migration
>> is started and hot unplug the vCPU when migration is completed, then we
>> won't need a vCPU running (and potentially consuming cycles) forever and
>> busy-looping with CpuPause().
>>
> After internal discussions, realized that this approach will not work as
> vCPU hotplug will not work for SEV-ES, SNP. As the VMSA has to be
> encrypted as part of the LAUNCH command, therefore we can't create/add a
> new vCPU after LAUNCH has completed.
>
> Thanks,
> Ashish

Hm yeah we talked about hotplug a bit. It was never clear how it would 
square with OVMF.

-Tobin

Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live Migration for AMD SEV

[Yao, Jiewen]

Hi
We discuss the patch internally. We do see PROs and CONs with this approach.
The advantage is that it is very simple. In-VM migration can save lots of effort on security context restore.
On the other hand, we feel not so comfortable to reserve a dedicate CPU to achieve that. Similar to the feedback in the community.

Using Hot-Plug is not a solution for Intel TDX as well. It is unsupported now.

I like the idea to diverge the migration boot mode v.s. normal boot mode in SEC phase.
We must be very carefully handle this migration boot mode, to avoid any touching on system memory.
Intel TDX Virtual Firmware skips the PEI phase directly. If we choose this approach, SEC-based migration is our preference.

Besides this patch, we would like to understand a full picture.
1) How the key is passed from source VM to destination?
I saw you mentions: "Key sharing is out of scope for this part of the RFC."
"This will probably be implemented via inject-launch-secret in the future"

Does that mean two PSP will sync with each other and negotiate the key, after the Migration Agent (MA) checks the policy?

2) How the attestation is supported?
I read the whitepaper https://www.amd.com/system/files/TechDocs/SEV-SNP-strengthening-vm-isolation-with-integrity-protection-and-more.pdf.
It seems SEV and SEV-ES only support attestation during launch, I don't believe this migration feature will impact the attestation report. Am I right?
SEV-SNP supports more flexible attestation, does it include any information about the new migrated content?


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
> Sent: Thursday, March 4, 2021 9:49 AM
> To: devel@edk2.groups.io; tobin@linux.ibm.com
> Cc: Dov Murik <dovmurik@linux.vnet.ibm.com>; Tobin Feldman-Fitzthum
> <tobin@ibm.com>; James Bottomley <jejb@linux.ibm.com>; Hubertus Franke
> <frankeh@us.ibm.com>; Brijesh Singh <brijesh.singh@amd.com>; Ashish Kalra
> <ashish.kalra@amd.com>; Jon Grimm <jon.grimm@amd.com>; Tom Lendacky
> <thomas.lendacky@amd.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live
> Migration for AMD SEV
> 
> Hi Tobin
> Thanks for your patch.
> You may that Intel is working on TDX for the same live migration feature.
> 
> Please give me some time (about 1 work week) to digest and evaluate the patch
> and impact.
> Then I will provide feedback.
> 
> Thank you
> Yao Jiewen
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Tobin
> > Feldman-Fitzthum
> > Sent: Wednesday, March 3, 2021 4:48 AM
> > To: devel@edk2.groups.io
> > Cc: Dov Murik <dovmurik@linux.vnet.ibm.com>; Tobin Feldman-Fitzthum
> > <tobin@ibm.com>; Tobin Feldman-Fitzthum <tobin@linux.ibm.com>; James
> > Bottomley <jejb@linux.ibm.com>; Hubertus Franke <frankeh@us.ibm.com>;
> > Brijesh Singh <brijesh.singh@amd.com>; Ashish Kalra
> <ashish.kalra@amd.com>;
> > Jon Grimm <jon.grimm@amd.com>; Tom Lendacky
> > <thomas.lendacky@amd.com>
> > Subject: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live
> > Migration for AMD SEV
> >
> > This is a demonstration of fast migration for encrypted virtual machines
> > using a Migration Handler that lives in OVMF. This demo uses AMD SEV,
> > but the ideas may generalize to other confidential computing platforms.
> > With AMD SEV, guest memory is encrypted and the hypervisor cannot access
> > or move it. This makes migration tricky. In this demo, we show how the
> > HV can ask a Migration Handler (MH) in the firmware for an encrypted
> > page. The MH encrypts the page with a transport key prior to releasing
> > it to the HV. The target machine also runs an MH that decrypts the page
> > once it is passed in by the target HV. These patches are not ready for
> > production, but the are a full end-to-end solution that facilitates a
> > fast live migration between two SEV VMs.
> >
> > Corresponding patches for QEMU have been posted my colleague Dov Murik
> > on qemu-devel. Our approach needs little kernel support, requiring only
> > one hypercall that the guest can use to mark a page as encrypted or
> > shared. This series includes updated patches from Ashish Kalra and
> > Brijesh Singh that allow OVMF to use this hypercall.
> >
> > The MH runs continuously in the guest, waiting for communication from
> > the HV. The HV starts an additional vCPU for the MH but does not expose
> > it to the guest OS via ACPI. We use the MpService to start the MH. The
> > MpService is only available at runtime and processes that are started by
> > it are usually cleaned up on ExitBootServices. Since we need the MH to
> > run continuously, we had to make some modifications. Ideally a feature
> > could be added to the MpService to allow for the starting of
> > long-running processes. Besides migration, this could support other
> > background processes that need to operate within the encryption
> > boundary. For now, we have included a handful of patches that modify the
> > MpService to allow the MH to keep running after ExitBootServices. These
> > are temporary.
> >
> > Ashish Kalra (2):
> >   OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap.
> >   OvmfPkg/PlatformDxe: Add support for SEV live migration.
> >
> > Brijesh Singh (1):
> >   OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall
> >
> > Dov Murik (1):
> >   OvmfPkg/AmdSev: Build page table for migration handler
> >
> > Tobin Feldman-Fitzthum (10):
> >   OvmfPkg/AmdSev: Base for Confidential Migration Handler
> >   OvmfPkg/PlatfomPei: Set Confidential Migration PCD
> >   OvmfPkg/AmdSev: Setup Migration Handler Mailbox
> >   OvmfPkg/AmdSev: MH support for mailbox protocol
> >   UefiCpuPkg/MpInitLib: temp removal of MpLib cleanup
> >   UefiCpuPkg/MpInitLib: Allocate MP buffer as runtime memory
> >   UefiCpuPkg/CpuExceptionHandlerLib: Exception handling as runtime
> >     memory
> >   OvmfPkg/AmdSev: Don't overwrite mailbox or pagetables
> >   OvmfPkg/AmdSev: Don't overwrite MH stack
> >   OvmfPkg/AmdSev: MH page encryption POC
> >
> >  OvmfPkg/OvmfPkg.dec                           |  11 +
> >  OvmfPkg/AmdSev/AmdSevX64.dsc                  |   2 +
> >  OvmfPkg/AmdSev/AmdSevX64.fdf                  |  13 +-
> >  .../ConfidentialMigrationDxe.inf              |  45 +++
> >  .../ConfidentialMigrationPei.inf              |  35 ++
> >  .../DxeMemEncryptSevLib.inf                   |   1 +
> >  .../PeiMemEncryptSevLib.inf                   |   1 +
> >  OvmfPkg/PlatformDxe/Platform.inf              |   2 +
> >  OvmfPkg/PlatformPei/PlatformPei.inf           |   2 +
> >  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   2 +
> >  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   2 +
> >  OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h  | 235 +++++++++++++
> >  .../ConfidentialMigration/VirtualMemory.h     | 177 ++++++++++
> >  OvmfPkg/Include/Guid/MemEncryptLib.h          |  16 +
> >  OvmfPkg/PlatformDxe/PlatformConfig.h          |   5 +
> >  .../ConfidentialMigrationDxe.c                | 325 ++++++++++++++++++
> >  .../ConfidentialMigrationPei.c                |  25 ++
> >  .../X64/PeiDxeVirtualMemory.c                 |  18 +
> >  OvmfPkg/PlatformDxe/AmdSev.c                  |  99 ++++++
> >  OvmfPkg/PlatformDxe/Platform.c                |   6 +
> >  OvmfPkg/PlatformPei/AmdSev.c                  |  10 +
> >  OvmfPkg/PlatformPei/Platform.c                |  10 +
> >  .../CpuExceptionHandlerLib/DxeException.c     |   8 +-
> >  UefiCpuPkg/Library/MpInitLib/DxeMpLib.c       |  21 +-
> >  UefiCpuPkg/Library/MpInitLib/MpLib.c          |   7 +-
> >  25 files changed, 1061 insertions(+), 17 deletions(-)
> >  create mode 100644
> > OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
> >  create mode 100644
> > OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.inf
> >  create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h
> >  create mode 100644
> > OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h
> >  create mode 100644 OvmfPkg/Include/Guid/MemEncryptLib.h
> >  create mode 100644
> > OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
> >  create mode 100644
> > OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.c
> >  create mode 100644 OvmfPkg/PlatformDxe/AmdSev.c
> >
> > --
> > 2.20.1
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 

Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live Migration for AMD SEV

[Singh, Brijesh]

[AMD Official Use Only - Internal Distribution Only]

Hi Yao,

In the current proposal the accelerated migration does not involve the PSP. I will let Tobin and Dov comment on how things works in current prototype.

If PSP was involved in the migration, then flow would be like this:

- During the guest creation time two things will happen (both source and destination VMs go through this step)
  a) create a random VM encryption key (VEK) -- the key is used for encrypting the guest pages.
  b) guest owner supplies a session blob to the PSP. The session blob contains transport encryption key (TEK).  The TEK is used to encrypt all the confidential information exchanged between the PSP and the external entities such as a guest owner or another PSP.

During the migration
i) source VMM asks PSP to get a page that can be migrated.
ii) source PSP  encrypt the guest pages using the TEK
iii) source VMM write the encrypted pages on the wire
iv) destination VMM  will call PSP to put the received encrypted page in the guest memory.
v) destination PSP will decrypt the received pages using TEK, then encrypt it using the VEK before copying it to the guest memory.

As you see in the flow, the PSP's never share the keys. The TEK is wrapped in the session blob provided to the PSP on launch.

You are correct that the SEV/SEV-ES does not support querying the attestation report after the guest boot. All the attestation need to be done during the guest creation time.

With SEV-SNP, a guest OS/BIOS can call PSP to get the attestation report. The SEV-SNP, provides a method in which the guest owner can provide an IMI (Initial migration agent) through the launch process. The IMI will be measured separately and stored in IMD (Initial Migration Digest). When source VMM is ready to migrate it will use a PSP command (VM_EXPORT) to export the data from source to destination. The export will contains information about IMD etc. The destination VMM will use the PSP command (ABSORB) to import the incoming data. During the absorb process the destination PSP will check the IMD to ensure that same IMI is used at the source end. I have cut short few details in the email; See the SEV-SNP spec (section migration 4.11) for more.

Thanks
Brijesh

-----Original Message-----
From: Yao, Jiewen <jiewen.yao@intel.com>
Sent: Friday, March 12, 2021 8:32 PM
To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>; tobin@linux.ibm.com
Cc: Dov Murik <dovmurik@linux.vnet.ibm.com>; Tobin Feldman-Fitzthum <tobin@ibm.com>; James Bottomley <jejb@linux.ibm.com>; Hubertus Franke <frankeh@us.ibm.com>; Singh, Brijesh <brijesh.singh@amd.com>; Kalra, Ashish <Ashish.Kalra@amd.com>; Grimm, Jon <Jon.Grimm@amd.com>; Lendacky, Thomas <Thomas.Lendacky@amd.com>
Subject: RE: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live Migration for AMD SEV

Hi
We discuss the patch internally. We do see PROs and CONs with this approach.
The advantage is that it is very simple. In-VM migration can save lots of effort on security context restore.
On the other hand, we feel not so comfortable to reserve a dedicate CPU to achieve that. Similar to the feedback in the community.

Using Hot-Plug is not a solution for Intel TDX as well. It is unsupported now.

I like the idea to diverge the migration boot mode v.s. normal boot mode in SEC phase.
We must be very carefully handle this migration boot mode, to avoid any touching on system memory.
Intel TDX Virtual Firmware skips the PEI phase directly. If we choose this approach, SEC-based migration is our preference.

Besides this patch, we would like to understand a full picture.
1) How the key is passed from source VM to destination?
I saw you mentions: "Key sharing is out of scope for this part of the RFC."
"This will probably be implemented via inject-launch-secret in the future"

Does that mean two PSP will sync with each other and negotiate the key, after the Migration Agent (MA) checks the policy?

2) How the attestation is supported?
I read the whitepaper https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2FSEV-SNP-strengthening-vm-isolation-with-integrity-protection-and-more.pdf&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cb19ccecd6ca946abd0eb08d8e5c84177%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637511995981376795%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=h67VntbdjigZFvhRfP6%2FGYTE9eqrFDqJRojWqG0C25c%3D&amp;reserved=0.
It seems SEV and SEV-ES only support attestation during launch, I don't believe this migration feature will impact the attestation report. Am I right?
SEV-SNP supports more flexible attestation, does it include any information about the new migrated content?


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao,
> Jiewen
> Sent: Thursday, March 4, 2021 9:49 AM
> To: devel@edk2.groups.io; tobin@linux.ibm.com
> Cc: Dov Murik <dovmurik@linux.vnet.ibm.com>; Tobin Feldman-Fitzthum
> <tobin@ibm.com>; James Bottomley <jejb@linux.ibm.com>; Hubertus Franke
> <frankeh@us.ibm.com>; Brijesh Singh <brijesh.singh@amd.com>; Ashish
> Kalra <ashish.kalra@amd.com>; Jon Grimm <jon.grimm@amd.com>; Tom
> Lendacky <thomas.lendacky@amd.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast
> Live Migration for AMD SEV
>
> Hi Tobin
> Thanks for your patch.
> You may that Intel is working on TDX for the same live migration feature.
>
> Please give me some time (about 1 work week) to digest and evaluate
> the patch and impact.
> Then I will provide feedback.
>
> Thank you
> Yao Jiewen
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Tobin
> > Feldman-Fitzthum
> > Sent: Wednesday, March 3, 2021 4:48 AM
> > To: devel@edk2.groups.io
> > Cc: Dov Murik <dovmurik@linux.vnet.ibm.com>; Tobin Feldman-Fitzthum
> > <tobin@ibm.com>; Tobin Feldman-Fitzthum <tobin@linux.ibm.com>; James
> > Bottomley <jejb@linux.ibm.com>; Hubertus Franke
> > <frankeh@us.ibm.com>; Brijesh Singh <brijesh.singh@amd.com>; Ashish
> > Kalra
> <ashish.kalra@amd.com>;
> > Jon Grimm <jon.grimm@amd.com>; Tom Lendacky
> > <thomas.lendacky@amd.com>
> > Subject: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast
> > Live Migration for AMD SEV
> >
> > This is a demonstration of fast migration for encrypted virtual
> > machines using a Migration Handler that lives in OVMF. This demo
> > uses AMD SEV, but the ideas may generalize to other confidential computing platforms.
> > With AMD SEV, guest memory is encrypted and the hypervisor cannot
> > access or move it. This makes migration tricky. In this demo, we
> > show how the HV can ask a Migration Handler (MH) in the firmware for
> > an encrypted page. The MH encrypts the page with a transport key
> > prior to releasing it to the HV. The target machine also runs an MH
> > that decrypts the page once it is passed in by the target HV. These
> > patches are not ready for production, but the are a full end-to-end
> > solution that facilitates a fast live migration between two SEV VMs.
> >
> > Corresponding patches for QEMU have been posted my colleague Dov
> > Murik on qemu-devel. Our approach needs little kernel support,
> > requiring only one hypercall that the guest can use to mark a page
> > as encrypted or shared. This series includes updated patches from
> > Ashish Kalra and Brijesh Singh that allow OVMF to use this hypercall.
> >
> > The MH runs continuously in the guest, waiting for communication
> > from the HV. The HV starts an additional vCPU for the MH but does
> > not expose it to the guest OS via ACPI. We use the MpService to
> > start the MH. The MpService is only available at runtime and
> > processes that are started by it are usually cleaned up on
> > ExitBootServices. Since we need the MH to run continuously, we had
> > to make some modifications. Ideally a feature could be added to the
> > MpService to allow for the starting of long-running processes.
> > Besides migration, this could support other background processes
> > that need to operate within the encryption boundary. For now, we
> > have included a handful of patches that modify the MpService to
> > allow the MH to keep running after ExitBootServices. These are temporary.
> >
> > Ashish Kalra (2):
> >   OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap.
> >   OvmfPkg/PlatformDxe: Add support for SEV live migration.
> >
> > Brijesh Singh (1):
> >   OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall
> >
> > Dov Murik (1):
> >   OvmfPkg/AmdSev: Build page table for migration handler
> >
> > Tobin Feldman-Fitzthum (10):
> >   OvmfPkg/AmdSev: Base for Confidential Migration Handler
> >   OvmfPkg/PlatfomPei: Set Confidential Migration PCD
> >   OvmfPkg/AmdSev: Setup Migration Handler Mailbox
> >   OvmfPkg/AmdSev: MH support for mailbox protocol
> >   UefiCpuPkg/MpInitLib: temp removal of MpLib cleanup
> >   UefiCpuPkg/MpInitLib: Allocate MP buffer as runtime memory
> >   UefiCpuPkg/CpuExceptionHandlerLib: Exception handling as runtime
> >     memory
> >   OvmfPkg/AmdSev: Don't overwrite mailbox or pagetables
> >   OvmfPkg/AmdSev: Don't overwrite MH stack
> >   OvmfPkg/AmdSev: MH page encryption POC
> >
> >  OvmfPkg/OvmfPkg.dec                           |  11 +
> >  OvmfPkg/AmdSev/AmdSevX64.dsc                  |   2 +
> >  OvmfPkg/AmdSev/AmdSevX64.fdf                  |  13 +-
> >  .../ConfidentialMigrationDxe.inf              |  45 +++
> >  .../ConfidentialMigrationPei.inf              |  35 ++
> >  .../DxeMemEncryptSevLib.inf                   |   1 +
> >  .../PeiMemEncryptSevLib.inf                   |   1 +
> >  OvmfPkg/PlatformDxe/Platform.inf              |   2 +
> >  OvmfPkg/PlatformPei/PlatformPei.inf           |   2 +
> >  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   2 +
> >  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   2 +
> >  OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h  | 235 +++++++++++++
> >  .../ConfidentialMigration/VirtualMemory.h     | 177 ++++++++++
> >  OvmfPkg/Include/Guid/MemEncryptLib.h          |  16 +
> >  OvmfPkg/PlatformDxe/PlatformConfig.h          |   5 +
> >  .../ConfidentialMigrationDxe.c                | 325 ++++++++++++++++++
> >  .../ConfidentialMigrationPei.c                |  25 ++
> >  .../X64/PeiDxeVirtualMemory.c                 |  18 +
> >  OvmfPkg/PlatformDxe/AmdSev.c                  |  99 ++++++
> >  OvmfPkg/PlatformDxe/Platform.c                |   6 +
> >  OvmfPkg/PlatformPei/AmdSev.c                  |  10 +
> >  OvmfPkg/PlatformPei/Platform.c                |  10 +
> >  .../CpuExceptionHandlerLib/DxeException.c     |   8 +-
> >  UefiCpuPkg/Library/MpInitLib/DxeMpLib.c       |  21 +-
> >  UefiCpuPkg/Library/MpInitLib/MpLib.c          |   7 +-
> >  25 files changed, 1061 insertions(+), 17 deletions(-)  create mode
> > 100644
> > OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
> >  create mode 100644
> > OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.inf
> >  create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h
> >  create mode 100644
> > OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h
> >  create mode 100644 OvmfPkg/Include/Guid/MemEncryptLib.h
> >  create mode 100644
> > OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
> >  create mode 100644
> > OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.c
> >  create mode 100644 OvmfPkg/PlatformDxe/AmdSev.c
> >
> > --
> > 2.20.1
> >
> >
> >
> >
> >
>
>
>
> 
>

Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live Migration for AMD SEV

[Tobin Feldman-Fitzthum]

On 3/12/21 9:32 PM, Yao, Jiewen wrote:

> Hi
> We discuss the patch internally. We do see PROs and CONs with this approach.
> The advantage is that it is very simple. In-VM migration can save lots of effort on security context restore.
> On the other hand, we feel not so comfortable to reserve a dedicate CPU to achieve that. Similar to the feedback in the community.
>
> Using Hot-Plug is not a solution for Intel TDX as well. It is unsupported now.
>
> I like the idea to diverge the migration boot mode v.s. normal boot mode in SEC phase.
> We must be very carefully handle this migration boot mode, to avoid any touching on system memory.
> Intel TDX Virtual Firmware skips the PEI phase directly. If we choose this approach, SEC-based migration is our preference.
>
> Besides this patch, we would like to understand a full picture.
> 1) How the key is passed from source VM to destination?
> I saw you mentions: "Key sharing is out of scope for this part of the RFC."
> "This will probably be implemented via inject-launch-secret in the future"
>
> Does that mean two PSP will sync with each other and negotiate the key, after the Migration Agent (MA) checks the policy?

The source and destination migration handlers will need to share a key. 
If we only relied on the PSP for migration, we could use the existing 
secure channel between the PSP and the guest owner to transfer the 
pages. Unfortunately the throughput of this approach is far too low. 
Thus, we have some migration handler running on a guest vCPU with a 
transport key shared between the source and the target.

The main mechanism for getting a key to the migration handler is 
inject-launch-secret. Here the guest owner can provide a secret to the 
PSP via a secure channel and the PSP will inject it at some guest 
physical address. You use inject-launch-secret after the launch 
measurement of the guest has been generated to inject the secret 
conditionally. One approach would be to inject the transport key 
directly in the source and the target. This is pretty simple, but might 
have a few drawbacks. The injection has to happen at boot, meaning that 
the source machine would have to be provisioned with a transport key 
before a migration happens and that all migrations from that machine 
would have to use the same transport key. One way around this would be 
to inject asymmetric keys and use them to derive the transport key.

Another approach entirely is to use the PSP to migrate just a few pages, 
which might include a secret set by the source MH that the target MH 
could use to decrypt incoming pages. Using the PSP to migrate pages 
requires some extra kernel support.

For the RFC, we just assume that there is some shared key. We have 
talked some about the various options internally.

> 2) How the attestation is supported?
> I read the whitepaper https://www.amd.com/system/files/TechDocs/SEV-SNP-strengthening-vm-isolation-with-integrity-protection-and-more.pdf.
> It seems SEV and SEV-ES only support attestation during launch, I don't believe this migration feature will impact the attestation report. Am I right?
> SEV-SNP supports more flexible attestation, does it include any information about the new migrated content?

Brijesh already addressed most of this. In our approach the MH is baked 
into the firmware, which can be attested prior to injecting the key. In 
other words there aren't any additional steps to attest the MH and it 
does not change the functionality of any existing attestation mechanisms.

-Tobin

>
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
>> Sent: Thursday, March 4, 2021 9:49 AM
>> To: devel@edk2.groups.io; tobin@linux.ibm.com
>> Cc: Dov Murik <dovmurik@linux.vnet.ibm.com>; Tobin Feldman-Fitzthum
>> <tobin@ibm.com>; James Bottomley <jejb@linux.ibm.com>; Hubertus Franke
>> <frankeh@us.ibm.com>; Brijesh Singh <brijesh.singh@amd.com>; Ashish Kalra
>> <ashish.kalra@amd.com>; Jon Grimm <jon.grimm@amd.com>; Tom Lendacky
>> <thomas.lendacky@amd.com>; Yao, Jiewen <jiewen.yao@intel.com>
>> Subject: Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live
>> Migration for AMD SEV
>>
>> Hi Tobin
>> Thanks for your patch.
>> You may that Intel is working on TDX for the same live migration feature.
>>
>> Please give me some time (about 1 work week) to digest and evaluate the patch
>> and impact.
>> Then I will provide feedback.
>>
>> Thank you
>> Yao Jiewen
>>
>>> -----Original Message-----
>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Tobin
>>> Feldman-Fitzthum
>>> Sent: Wednesday, March 3, 2021 4:48 AM
>>> To: devel@edk2.groups.io
>>> Cc: Dov Murik <dovmurik@linux.vnet.ibm.com>; Tobin Feldman-Fitzthum
>>> <tobin@ibm.com>; Tobin Feldman-Fitzthum <tobin@linux.ibm.com>; James
>>> Bottomley <jejb@linux.ibm.com>; Hubertus Franke <frankeh@us.ibm.com>;
>>> Brijesh Singh <brijesh.singh@amd.com>; Ashish Kalra
>> <ashish.kalra@amd.com>;
>>> Jon Grimm <jon.grimm@amd.com>; Tom Lendacky
>>> <thomas.lendacky@amd.com>
>>> Subject: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live
>>> Migration for AMD SEV
>>>
>>> This is a demonstration of fast migration for encrypted virtual machines
>>> using a Migration Handler that lives in OVMF. This demo uses AMD SEV,
>>> but the ideas may generalize to other confidential computing platforms.
>>> With AMD SEV, guest memory is encrypted and the hypervisor cannot access
>>> or move it. This makes migration tricky. In this demo, we show how the
>>> HV can ask a Migration Handler (MH) in the firmware for an encrypted
>>> page. The MH encrypts the page with a transport key prior to releasing
>>> it to the HV. The target machine also runs an MH that decrypts the page
>>> once it is passed in by the target HV. These patches are not ready for
>>> production, but the are a full end-to-end solution that facilitates a
>>> fast live migration between two SEV VMs.
>>>
>>> Corresponding patches for QEMU have been posted my colleague Dov Murik
>>> on qemu-devel. Our approach needs little kernel support, requiring only
>>> one hypercall that the guest can use to mark a page as encrypted or
>>> shared. This series includes updated patches from Ashish Kalra and
>>> Brijesh Singh that allow OVMF to use this hypercall.
>>>
>>> The MH runs continuously in the guest, waiting for communication from
>>> the HV. The HV starts an additional vCPU for the MH but does not expose
>>> it to the guest OS via ACPI. We use the MpService to start the MH. The
>>> MpService is only available at runtime and processes that are started by
>>> it are usually cleaned up on ExitBootServices. Since we need the MH to
>>> run continuously, we had to make some modifications. Ideally a feature
>>> could be added to the MpService to allow for the starting of
>>> long-running processes. Besides migration, this could support other
>>> background processes that need to operate within the encryption
>>> boundary. For now, we have included a handful of patches that modify the
>>> MpService to allow the MH to keep running after ExitBootServices. These
>>> are temporary.
>>>
>>> Ashish Kalra (2):
>>>    OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap.
>>>    OvmfPkg/PlatformDxe: Add support for SEV live migration.
>>>
>>> Brijesh Singh (1):
>>>    OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall
>>>
>>> Dov Murik (1):
>>>    OvmfPkg/AmdSev: Build page table for migration handler
>>>
>>> Tobin Feldman-Fitzthum (10):
>>>    OvmfPkg/AmdSev: Base for Confidential Migration Handler
>>>    OvmfPkg/PlatfomPei: Set Confidential Migration PCD
>>>    OvmfPkg/AmdSev: Setup Migration Handler Mailbox
>>>    OvmfPkg/AmdSev: MH support for mailbox protocol
>>>    UefiCpuPkg/MpInitLib: temp removal of MpLib cleanup
>>>    UefiCpuPkg/MpInitLib: Allocate MP buffer as runtime memory
>>>    UefiCpuPkg/CpuExceptionHandlerLib: Exception handling as runtime
>>>      memory
>>>    OvmfPkg/AmdSev: Don't overwrite mailbox or pagetables
>>>    OvmfPkg/AmdSev: Don't overwrite MH stack
>>>    OvmfPkg/AmdSev: MH page encryption POC
>>>
>>>   OvmfPkg/OvmfPkg.dec                           |  11 +
>>>   OvmfPkg/AmdSev/AmdSevX64.dsc                  |   2 +
>>>   OvmfPkg/AmdSev/AmdSevX64.fdf                  |  13 +-
>>>   .../ConfidentialMigrationDxe.inf              |  45 +++
>>>   .../ConfidentialMigrationPei.inf              |  35 ++
>>>   .../DxeMemEncryptSevLib.inf                   |   1 +
>>>   .../PeiMemEncryptSevLib.inf                   |   1 +
>>>   OvmfPkg/PlatformDxe/Platform.inf              |   2 +
>>>   OvmfPkg/PlatformPei/PlatformPei.inf           |   2 +
>>>   UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   2 +
>>>   UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   2 +
>>>   OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h  | 235 +++++++++++++
>>>   .../ConfidentialMigration/VirtualMemory.h     | 177 ++++++++++
>>>   OvmfPkg/Include/Guid/MemEncryptLib.h          |  16 +
>>>   OvmfPkg/PlatformDxe/PlatformConfig.h          |   5 +
>>>   .../ConfidentialMigrationDxe.c                | 325 ++++++++++++++++++
>>>   .../ConfidentialMigrationPei.c                |  25 ++
>>>   .../X64/PeiDxeVirtualMemory.c                 |  18 +
>>>   OvmfPkg/PlatformDxe/AmdSev.c                  |  99 ++++++
>>>   OvmfPkg/PlatformDxe/Platform.c                |   6 +
>>>   OvmfPkg/PlatformPei/AmdSev.c                  |  10 +
>>>   OvmfPkg/PlatformPei/Platform.c                |  10 +
>>>   .../CpuExceptionHandlerLib/DxeException.c     |   8 +-
>>>   UefiCpuPkg/Library/MpInitLib/DxeMpLib.c       |  21 +-
>>>   UefiCpuPkg/Library/MpInitLib/MpLib.c          |   7 +-
>>>   25 files changed, 1061 insertions(+), 17 deletions(-)
>>>   create mode 100644
>>> OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
>>>   create mode 100644
>>> OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.inf
>>>   create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h
>>>   create mode 100644
>>> OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h
>>>   create mode 100644 OvmfPkg/Include/Guid/MemEncryptLib.h
>>>   create mode 100644
>>> OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
>>>   create mode 100644
>>> OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.c
>>>   create mode 100644 OvmfPkg/PlatformDxe/AmdSev.c
>>>
>>> --
>>> 2.20.1
>>>
>>>
>>>
>>>
>>>
>>
>>
>> 
>>

Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live Migration for AMD SEV

[Yao, Jiewen]

Thank you very much Tobin and Brijesh.

Yes, I agree that there are multiple ways to pass the transport key from source to destination.
I will wait for your final solution.


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Tobin
> Feldman-Fitzthum
> Sent: Wednesday, March 17, 2021 1:47 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Cc: Dov Murik <dovmurik@linux.vnet.ibm.com>; Tobin Feldman-Fitzthum
> <tobin@ibm.com>; James Bottomley <jejb@linux.ibm.com>; Hubertus Franke
> <frankeh@us.ibm.com>; Brijesh Singh <brijesh.singh@amd.com>; Ashish Kalra
> <ashish.kalra@amd.com>; Jon Grimm <jon.grimm@amd.com>; Tom Lendacky
> <thomas.lendacky@amd.com>
> Subject: Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live
> Migration for AMD SEV
> 
> On 3/12/21 9:32 PM, Yao, Jiewen wrote:
> 
> > Hi
> > We discuss the patch internally. We do see PROs and CONs with this approach.
> > The advantage is that it is very simple. In-VM migration can save lots of effort
> on security context restore.
> > On the other hand, we feel not so comfortable to reserve a dedicate CPU to
> achieve that. Similar to the feedback in the community.
> >
> > Using Hot-Plug is not a solution for Intel TDX as well. It is unsupported now.
> >
> > I like the idea to diverge the migration boot mode v.s. normal boot mode in
> SEC phase.
> > We must be very carefully handle this migration boot mode, to avoid any
> touching on system memory.
> > Intel TDX Virtual Firmware skips the PEI phase directly. If we choose this
> approach, SEC-based migration is our preference.
> >
> > Besides this patch, we would like to understand a full picture.
> > 1) How the key is passed from source VM to destination?
> > I saw you mentions: "Key sharing is out of scope for this part of the RFC."
> > "This will probably be implemented via inject-launch-secret in the future"
> >
> > Does that mean two PSP will sync with each other and negotiate the key, after
> the Migration Agent (MA) checks the policy?
> 
> The source and destination migration handlers will need to share a key.
> If we only relied on the PSP for migration, we could use the existing
> secure channel between the PSP and the guest owner to transfer the
> pages. Unfortunately the throughput of this approach is far too low.
> Thus, we have some migration handler running on a guest vCPU with a
> transport key shared between the source and the target.
> 
> The main mechanism for getting a key to the migration handler is
> inject-launch-secret. Here the guest owner can provide a secret to the
> PSP via a secure channel and the PSP will inject it at some guest
> physical address. You use inject-launch-secret after the launch
> measurement of the guest has been generated to inject the secret
> conditionally. One approach would be to inject the transport key
> directly in the source and the target. This is pretty simple, but might
> have a few drawbacks. The injection has to happen at boot, meaning that
> the source machine would have to be provisioned with a transport key
> before a migration happens and that all migrations from that machine
> would have to use the same transport key. One way around this would be
> to inject asymmetric keys and use them to derive the transport key.
> 
> Another approach entirely is to use the PSP to migrate just a few pages,
> which might include a secret set by the source MH that the target MH
> could use to decrypt incoming pages. Using the PSP to migrate pages
> requires some extra kernel support.
> 
> For the RFC, we just assume that there is some shared key. We have
> talked some about the various options internally.
> 
> > 2) How the attestation is supported?
> > I read the whitepaper https://www.amd.com/system/files/TechDocs/SEV-
> SNP-strengthening-vm-isolation-with-integrity-protection-and-more.pdf.
> > It seems SEV and SEV-ES only support attestation during launch, I don't believe
> this migration feature will impact the attestation report. Am I right?
> > SEV-SNP supports more flexible attestation, does it include any information
> about the new migrated content?
> 
> Brijesh already addressed most of this. In our approach the MH is baked
> into the firmware, which can be attested prior to injecting the key. In
> other words there aren't any additional steps to attest the MH and it
> does not change the functionality of any existing attestation mechanisms.
> 
> -Tobin
> 
> >
> >> -----Original Message-----
> >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao,
> Jiewen
> >> Sent: Thursday, March 4, 2021 9:49 AM
> >> To: devel@edk2.groups.io; tobin@linux.ibm.com
> >> Cc: Dov Murik <dovmurik@linux.vnet.ibm.com>; Tobin Feldman-Fitzthum
> >> <tobin@ibm.com>; James Bottomley <jejb@linux.ibm.com>; Hubertus Franke
> >> <frankeh@us.ibm.com>; Brijesh Singh <brijesh.singh@amd.com>; Ashish
> Kalra
> >> <ashish.kalra@amd.com>; Jon Grimm <jon.grimm@amd.com>; Tom
> Lendacky
> >> <thomas.lendacky@amd.com>; Yao, Jiewen <jiewen.yao@intel.com>
> >> Subject: Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live
> >> Migration for AMD SEV
> >>
> >> Hi Tobin
> >> Thanks for your patch.
> >> You may that Intel is working on TDX for the same live migration feature.
> >>
> >> Please give me some time (about 1 work week) to digest and evaluate the
> patch
> >> and impact.
> >> Then I will provide feedback.
> >>
> >> Thank you
> >> Yao Jiewen
> >>
> >>> -----Original Message-----
> >>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Tobin
> >>> Feldman-Fitzthum
> >>> Sent: Wednesday, March 3, 2021 4:48 AM
> >>> To: devel@edk2.groups.io
> >>> Cc: Dov Murik <dovmurik@linux.vnet.ibm.com>; Tobin Feldman-Fitzthum
> >>> <tobin@ibm.com>; Tobin Feldman-Fitzthum <tobin@linux.ibm.com>; James
> >>> Bottomley <jejb@linux.ibm.com>; Hubertus Franke <frankeh@us.ibm.com>;
> >>> Brijesh Singh <brijesh.singh@amd.com>; Ashish Kalra
> >> <ashish.kalra@amd.com>;
> >>> Jon Grimm <jon.grimm@amd.com>; Tom Lendacky
> >>> <thomas.lendacky@amd.com>
> >>> Subject: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live
> >>> Migration for AMD SEV
> >>>
> >>> This is a demonstration of fast migration for encrypted virtual machines
> >>> using a Migration Handler that lives in OVMF. This demo uses AMD SEV,
> >>> but the ideas may generalize to other confidential computing platforms.
> >>> With AMD SEV, guest memory is encrypted and the hypervisor cannot
> access
> >>> or move it. This makes migration tricky. In this demo, we show how the
> >>> HV can ask a Migration Handler (MH) in the firmware for an encrypted
> >>> page. The MH encrypts the page with a transport key prior to releasing
> >>> it to the HV. The target machine also runs an MH that decrypts the page
> >>> once it is passed in by the target HV. These patches are not ready for
> >>> production, but the are a full end-to-end solution that facilitates a
> >>> fast live migration between two SEV VMs.
> >>>
> >>> Corresponding patches for QEMU have been posted my colleague Dov
> Murik
> >>> on qemu-devel. Our approach needs little kernel support, requiring only
> >>> one hypercall that the guest can use to mark a page as encrypted or
> >>> shared. This series includes updated patches from Ashish Kalra and
> >>> Brijesh Singh that allow OVMF to use this hypercall.
> >>>
> >>> The MH runs continuously in the guest, waiting for communication from
> >>> the HV. The HV starts an additional vCPU for the MH but does not expose
> >>> it to the guest OS via ACPI. We use the MpService to start the MH. The
> >>> MpService is only available at runtime and processes that are started by
> >>> it are usually cleaned up on ExitBootServices. Since we need the MH to
> >>> run continuously, we had to make some modifications. Ideally a feature
> >>> could be added to the MpService to allow for the starting of
> >>> long-running processes. Besides migration, this could support other
> >>> background processes that need to operate within the encryption
> >>> boundary. For now, we have included a handful of patches that modify the
> >>> MpService to allow the MH to keep running after ExitBootServices. These
> >>> are temporary.
> >>>
> >>> Ashish Kalra (2):
> >>>    OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap.
> >>>    OvmfPkg/PlatformDxe: Add support for SEV live migration.
> >>>
> >>> Brijesh Singh (1):
> >>>    OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall
> >>>
> >>> Dov Murik (1):
> >>>    OvmfPkg/AmdSev: Build page table for migration handler
> >>>
> >>> Tobin Feldman-Fitzthum (10):
> >>>    OvmfPkg/AmdSev: Base for Confidential Migration Handler
> >>>    OvmfPkg/PlatfomPei: Set Confidential Migration PCD
> >>>    OvmfPkg/AmdSev: Setup Migration Handler Mailbox
> >>>    OvmfPkg/AmdSev: MH support for mailbox protocol
> >>>    UefiCpuPkg/MpInitLib: temp removal of MpLib cleanup
> >>>    UefiCpuPkg/MpInitLib: Allocate MP buffer as runtime memory
> >>>    UefiCpuPkg/CpuExceptionHandlerLib: Exception handling as runtime
> >>>      memory
> >>>    OvmfPkg/AmdSev: Don't overwrite mailbox or pagetables
> >>>    OvmfPkg/AmdSev: Don't overwrite MH stack
> >>>    OvmfPkg/AmdSev: MH page encryption POC
> >>>
> >>>   OvmfPkg/OvmfPkg.dec                           |  11 +
> >>>   OvmfPkg/AmdSev/AmdSevX64.dsc                  |   2 +
> >>>   OvmfPkg/AmdSev/AmdSevX64.fdf                  |  13 +-
> >>>   .../ConfidentialMigrationDxe.inf              |  45 +++
> >>>   .../ConfidentialMigrationPei.inf              |  35 ++
> >>>   .../DxeMemEncryptSevLib.inf                   |   1 +
> >>>   .../PeiMemEncryptSevLib.inf                   |   1 +
> >>>   OvmfPkg/PlatformDxe/Platform.inf              |   2 +
> >>>   OvmfPkg/PlatformPei/PlatformPei.inf           |   2 +
> >>>   UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   2 +
> >>>   UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   2 +
> >>>   OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h  | 235 +++++++++++++
> >>>   .../ConfidentialMigration/VirtualMemory.h     | 177 ++++++++++
> >>>   OvmfPkg/Include/Guid/MemEncryptLib.h          |  16 +
> >>>   OvmfPkg/PlatformDxe/PlatformConfig.h          |   5 +
> >>>   .../ConfidentialMigrationDxe.c                | 325 ++++++++++++++++++
> >>>   .../ConfidentialMigrationPei.c                |  25 ++
> >>>   .../X64/PeiDxeVirtualMemory.c                 |  18 +
> >>>   OvmfPkg/PlatformDxe/AmdSev.c                  |  99 ++++++
> >>>   OvmfPkg/PlatformDxe/Platform.c                |   6 +
> >>>   OvmfPkg/PlatformPei/AmdSev.c                  |  10 +
> >>>   OvmfPkg/PlatformPei/Platform.c                |  10 +
> >>>   .../CpuExceptionHandlerLib/DxeException.c     |   8 +-
> >>>   UefiCpuPkg/Library/MpInitLib/DxeMpLib.c       |  21 +-
> >>>   UefiCpuPkg/Library/MpInitLib/MpLib.c          |   7 +-
> >>>   25 files changed, 1061 insertions(+), 17 deletions(-)
> >>>   create mode 100644
> >>> OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
> >>>   create mode 100644
> >>> OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.inf
> >>>   create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h
> >>>   create mode 100644
> >>> OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h
> >>>   create mode 100644 OvmfPkg/Include/Guid/MemEncryptLib.h
> >>>   create mode 100644
> >>> OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
> >>>   create mode 100644
> >>> OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.c
> >>>   create mode 100644 OvmfPkg/PlatformDxe/AmdSev.c
> >>>
> >>> --
> >>> 2.20.1
> >>>
> >>>
> >>>
> >>>
> >>>
> >>
> >>
> >>
> >>
> 
> 
> 
>