[PATCH v6 1/6] OvmfPkg/BaseMemEncryptLib: Detect SEV live migration feature.

[PATCH v6 1/6] OvmfPkg/BaseMemEncryptLib: Detect SEV live migration feature.

[Ashish Kalra]

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

Add support to check if we are running inside KVM HVM and
KVM HVM supports SEV Live Migration feature.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 OvmfPkg/Include/Library/MemEncryptSevLib.h                            | 27 ++++++++++
 OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c    | 39 +++++++++++++++
 OvmfPkg/Library/BaseMemEncryptSevLib/PeiDxeMemEncryptSevLibInternal.c | 52 ++++++++++++++++++++
 OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c    | 39 +++++++++++++++
 OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c    | 18 +++++++
 5 files changed, 175 insertions(+)

diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h b/OvmfPkg/Include/Library/MemEncryptSevLib.h
index 76d06c206c..59f694fb8a 100644
--- a/OvmfPkg/Include/Library/MemEncryptSevLib.h
+++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
@@ -90,6 +90,18 @@ MemEncryptSevIsEnabled (
   VOID
   );
 
+/**
+  Returns a boolean to indicate whether SEV live migration is enabled.
+
+  @retval TRUE           SEV live migration is enabled
+  @retval FALSE          SEV live migration is not enabled
+**/
+BOOLEAN
+EFIAPI
+MemEncryptSevLiveMigrationIsEnabled (
+  VOID
+  );
+
 /**
   This function clears memory encryption bit for the memory region specified by
   BaseAddress and NumPages from the current page table context.
@@ -222,4 +234,19 @@ MemEncryptSevClearMmioPageEncMask (
   IN UINTN                    NumPages
   );
 
+#define KVM_FEATURE_MIGRATION_CONTROL   BIT17
+
+/**
+  Figures out if we are running inside KVM HVM and
+  KVM HVM supports SEV Live Migration feature.
+
+  @retval TRUE           SEV live migration is supported.
+  @retval FALSE          SEV live migration is not supported.
+**/
+BOOLEAN
+EFIAPI
+KvmDetectSevLiveMigrationFeature(
+  VOID
+  );
+
 #endif // _MEM_ENCRYPT_SEV_LIB_H_
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c
index 2816f859a0..ead754cd7b 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c
@@ -20,6 +20,8 @@
 STATIC BOOLEAN mSevStatus = FALSE;
 STATIC BOOLEAN mSevEsStatus = FALSE;
 STATIC BOOLEAN mSevStatusChecked = FALSE;
+STATIC BOOLEAN mSevLiveMigrationStatus = FALSE;
+STATIC BOOLEAN mSevLiveMigrationStatusChecked = FALSE;
 
 STATIC UINT64  mSevEncryptionMask = 0;
 STATIC BOOLEAN mSevEncryptionMaskSaved = FALSE;
@@ -87,6 +89,24 @@ InternalMemEncryptSevStatus (
   mSevStatusChecked = TRUE;
 }
 
+/**
+  Figures out if we are running inside KVM HVM and
+  KVM HVM supports SEV Live Migration feature.
+**/
+STATIC
+VOID
+EFIAPI
+InternalDetectSevLiveMigrationFeature(
+  VOID
+  )
+{
+  if (KvmDetectSevLiveMigrationFeature()) {
+        mSevLiveMigrationStatus = TRUE;
+  }
+
+  mSevLiveMigrationStatusChecked = TRUE;
+}
+
 /**
   Returns a boolean to indicate whether SEV-ES is enabled.
 
@@ -125,6 +145,25 @@ MemEncryptSevIsEnabled (
   return mSevStatus;
 }
 
+/**
+  Returns a boolean to indicate whether SEV live migration is enabled.
+
+  @retval TRUE           SEV live migration is enabled
+  @retval FALSE          SEV live migration is not enabled
+**/
+BOOLEAN
+EFIAPI
+MemEncryptSevLiveMigrationIsEnabled (
+  VOID
+  )
+{
+  if (!mSevLiveMigrationStatusChecked) {
+    InternalDetectSevLiveMigrationFeature ();
+  }
+
+  return mSevLiveMigrationStatus;
+}
+
 /**
   Returns the SEV encryption mask.
 
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiDxeMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiDxeMemEncryptSevLibInternal.c
index b4a9f464e2..d7fc973134 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiDxeMemEncryptSevLibInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiDxeMemEncryptSevLibInternal.c
@@ -61,3 +61,55 @@ MemEncryptSevLocateInitialSmramSaveStateMapPages (
 
   return RETURN_SUCCESS;
 }
+
+/**
+  Figures out if we are running inside KVM HVM and
+  KVM HVM supports SEV Live Migration feature.
+
+  @retval TRUE           SEV live migration is supported.
+  @retval FALSE          SEV live migration is not supported.
+**/
+BOOLEAN
+EFIAPI
+KvmDetectSevLiveMigrationFeature(
+  VOID
+  )
+{
+  CHAR8 Signature[13];
+  UINT32 mKvmLeaf;
+  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 (Signature, "KVMKVMKVM") == 0) {
+      DEBUG ((
+        DEBUG_INFO,
+        "%a: KVM Detected, signature = %a\n",
+        __FUNCTION__,
+        Signature
+        ));
+
+      RegEax = mKvmLeaf + 1;
+      RegEcx = 0;
+      AsmCpuid (mKvmLeaf + 1, &RegEax, &RegEbx, &RegEcx, &RegEdx);
+      if ((RegEax & KVM_FEATURE_MIGRATION_CONTROL) != 0) {
+        DEBUG ((
+          DEBUG_INFO,
+          "%a: SEV Live Migration feature supported\n",
+          __FUNCTION__
+          ));
+
+        return TRUE;
+      }
+    }
+  }
+
+  return FALSE;
+}
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
index e2fd109d12..9db6c2ef71 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
@@ -20,6 +20,8 @@
 STATIC BOOLEAN mSevStatus = FALSE;
 STATIC BOOLEAN mSevEsStatus = FALSE;
 STATIC BOOLEAN mSevStatusChecked = FALSE;
+STATIC BOOLEAN mSevLiveMigrationStatus = FALSE;
+STATIC BOOLEAN mSevLiveMigrationStatusChecked = FALSE;
 
 STATIC UINT64  mSevEncryptionMask = 0;
 STATIC BOOLEAN mSevEncryptionMaskSaved = FALSE;
@@ -87,6 +89,24 @@ InternalMemEncryptSevStatus (
   mSevStatusChecked = TRUE;
 }
 
+/**
+  Figures out if we are running inside KVM HVM and
+  KVM HVM supports SEV Live Migration feature.
+**/
+STATIC
+VOID
+EFIAPI
+InternalDetectSevLiveMigrationFeature(
+  VOID
+  )
+{
+  if (KvmDetectSevLiveMigrationFeature()) {
+    mSevLiveMigrationStatus = TRUE;
+  }
+
+  mSevLiveMigrationStatusChecked = TRUE;
+}
+
 /**
   Returns a boolean to indicate whether SEV-ES is enabled.
 
@@ -125,6 +145,25 @@ MemEncryptSevIsEnabled (
   return mSevStatus;
 }
 
+/**
+  Returns a boolean to indicate whether SEV live migration is enabled.
+
+  @retval TRUE           SEV live migration is enabled
+  @retval FALSE          SEV live migration is not enabled
+**/
+BOOLEAN
+EFIAPI
+MemEncryptSevLiveMigrationIsEnabled (
+  VOID
+  )
+{
+  if (!mSevLiveMigrationStatusChecked) {
+    InternalDetectSevLiveMigrationFeature ();
+  }
+
+  return mSevLiveMigrationStatus;
+}
+
 /**
   Returns the SEV encryption mask.
 
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
index 56d8f3f318..d9f7befcd2 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
@@ -100,6 +100,24 @@ MemEncryptSevIsEnabled (
   return Msr.Bits.SevBit ? TRUE : FALSE;
 }
 
+/**
+  Returns a boolean to indicate whether SEV live migration is enabled.
+
+  @retval TRUE           SEV live migration is enabled
+  @retval FALSE          SEV live migration is not enabled
+**/
+BOOLEAN
+EFIAPI
+MemEncryptSevLiveMigrationIsEnabled (
+  VOID
+  )
+{
+  //
+  // Not used in SEC phase.
+  //
+  return FALSE;
+}
+
 /**
   Returns the SEV encryption mask.
 
-- 
2.17.1

[PATCH v6 2/6] OvmfPkg/BaseMemEncryptLib: Hypercall API for page encryption state change

[Ashish Kalra]

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

Add API to issue hypercall on page encryption state change.

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.

This hypercall is used to notify hypervisor when the page's
encryption state changes.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 OvmfPkg/Include/Library/MemEncryptSevLib.h                         | 43 +++++++++++++
 OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf       |  1 +
 OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c       | 27 +++++++++
 OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf       |  1 +
 OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c | 20 ++++++
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/AsmHelperStub.nasm        | 33 ++++++++++
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c        | 64 ++++++++++++++++++++
 7 files changed, 189 insertions(+)

diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h b/OvmfPkg/Include/Library/MemEncryptSevLib.h
index 59f694fb8a..56cc7bb958 100644
--- a/OvmfPkg/Include/Library/MemEncryptSevLib.h
+++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
@@ -249,4 +249,47 @@ KvmDetectSevLiveMigrationFeature(
   VOID
   );
 
+/**
+ This hypercall is used to notify hypervisor when the page's encryption
+ state changes.
+
+ @param[in]   PhysicalAddress       The physical address that is the start address
+                                    of a memory region.
+ @param[in]   Pages                 Number of pages in memory region.
+ @param[in]   IsEncrypted           Encrypted or Decrypted.
+
+ @retval RETURN_SUCCESS             Hypercall returned success.
+ @retval RETURN_UNSUPPORTED         Hypercall not supported.
+ @retval RETURN_NO_MAPPING          Hypercall returned error.
+**/
+RETURN_STATUS
+EFIAPI
+SetMemoryEncDecHypercall3 (
+  IN  UINTN     PhysicalAddress,
+  IN  UINTN     Pages,
+  IN  BOOLEAN   IsEncrypted
+  );
+
+#define KVM_HC_MAP_GPA_RANGE   12
+#define KVM_MAP_GPA_RANGE_PAGE_SZ_4K    0
+#define KVM_MAP_GPA_RANGE_PAGE_SZ_2M    BIT0
+#define KVM_MAP_GPA_RANGE_PAGE_SZ_1G    BIT1
+#define KVM_MAP_GPA_RANGE_ENC_STAT(n)   ((n) << 4)
+#define KVM_MAP_GPA_RANGE_ENCRYPTED     KVM_MAP_GPA_RANGE_ENC_STAT(1)
+#define KVM_MAP_GPA_RANGE_DECRYPTED     KVM_MAP_GPA_RANGE_ENC_STAT(0)
+
+/**
+  Interface exposed by the ASM implementation of the core hypercall
+
+  @retval Hypercall returned status.
+**/
+UINTN
+EFIAPI
+SetMemoryEncDecHypercall3AsmStub (
+  IN  UINTN  HypercallNum,
+  IN  UINTN  PhysicalAddress,
+  IN  UINTN  Pages,
+  IN  UINTN  Attributes
+  );
+
 #endif // _MEM_ENCRYPT_SEV_LIB_H_
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
index f2e162d680..0c28afadee 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
@@ -38,6 +38,7 @@
   X64/PeiDxeVirtualMemory.c
   X64/VirtualMemory.c
   X64/VirtualMemory.h
+  X64/AsmHelperStub.nasm
 
 [Sources.IA32]
   Ia32/MemEncryptSevLib.c
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
index be260e0d10..516d639489 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
@@ -136,3 +136,30 @@ MemEncryptSevClearMmioPageEncMask (
   //
   return RETURN_UNSUPPORTED;
 }
+
+/**
+ This hyercall is used to notify hypervisor when the page's encryption
+ state changes.
+
+ @param[in]   PhysicalAddress       The physical address that is the start address
+                                    of a memory region.
+ @param[in]   Pages                 Number of Pages in the memory region.
+ @param[in]   IsEncrypted           Encrypted or Decrypted.
+
+ @retval RETURN_SUCCESS             Hypercall returned success.
+ @retval RETURN_UNSUPPORTED         Hypercall not supported.
+ @retval RETURN_NO_MAPPING          Hypercall returned error.
+**/
+RETURN_STATUS
+EFIAPI
+SetMemoryEncDecHypercall3 (
+  IN  UINTN     PhysicalAddress,
+  IN  UINTN     Pages,
+  IN  BOOLEAN   IsEncrypted
+  )
+{
+  //
+  // Memory encryption bit is not accessible in 32-bit mode
+  //
+  return RETURN_UNSUPPORTED;
+}
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
index 03a78c32df..3233ca7979 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
@@ -38,6 +38,7 @@
   X64/PeiDxeVirtualMemory.c
   X64/VirtualMemory.c
   X64/VirtualMemory.h
+  X64/AsmHelperStub.nasm
 
 [Sources.IA32]
   Ia32/MemEncryptSevLib.c
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
index d9f7befcd2..ebb1c39319 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
@@ -118,6 +118,26 @@ MemEncryptSevLiveMigrationIsEnabled (
   return FALSE;
 }
 
+/**
+  Interface exposed by the ASM implementation of the core hypercall
+
+  @retval Hypercall returned status.
+**/
+UINTN
+EFIAPI
+SetMemoryEncDecHypercall3AsmStub (
+  IN  UINTN  HypercallNum,
+  IN  UINTN  PhysicalAddress,
+  IN  UINTN  Pages,
+  IN  UINTN  Attributes
+  )
+{
+  //
+  // Not used in SEC phase.
+  //
+  return RETURN_UNSUPPORTED;
+}
+
 /**
   Returns the SEV encryption mask.
 
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/AsmHelperStub.nasm b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/AsmHelperStub.nasm
new file mode 100644
index 0000000000..0ec35dd9b6
--- /dev/null
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/AsmHelperStub.nasm
@@ -0,0 +1,33 @@
+/** @file
+
+  ASM helper stub to invoke hypercall
+
+  Copyright (c) 2021, AMD Incorporated. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+DEFAULT REL
+SECTION .text
+
+; UINTN
+; EFIAPI
+; SetMemoryEncDecHypercall3AsmStub (
+;   IN UINTN HypercallNum,
+;   IN UINTN Arg1,
+;   IN UINTN Arg2,
+;   IN UINTN Arg3
+;   );
+global ASM_PFX(SetMemoryEncDecHypercall3AsmStub)
+ASM_PFX(SetMemoryEncDecHypercall3AsmStub):
+  ; UEFI calling conventions require RBX to
+  ; be nonvolatile/callee-saved.
+  push rbx
+  mov rax, rcx    ; Copy HypercallNumber to rax
+  mov rbx, rdx    ; Copy Arg1 to the register expected by KVM
+  mov rcx, r8     ; Copy Arg2 to register expected by KVM
+  mov rdx, r9     ; Copy Arg3 to register expected by KVM
+  vmmcall         ; Call VMMCALL
+  pop rbx
+  ret
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
index a57e8fd37f..fa679c9fc9 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
@@ -143,3 +143,67 @@ MemEncryptSevClearMmioPageEncMask (
            );
 
 }
+
+/**
+ This hyercall is used to notify hypervisor when the page's encryption
+ state changes.
+
+ @param[in]   PhysicalAddress       The physical address that is the start address
+                                    of a memory region.
+ @param[in]   Pages                 Number of Pages in the memory region.
+ @param[in]   IsEncrypted           Encrypted or Decrypted.
+
+ @retval RETURN_SUCCESS             Hypercall returned success.
+ @retval RETURN_UNSUPPORTED         Hypercall not supported.
+ @retval RETURN_NO_MAPPING          Hypercall returned error.
+**/
+RETURN_STATUS
+EFIAPI
+SetMemoryEncDecHypercall3 (
+  IN  UINTN     PhysicalAddress,
+  IN  UINTN     Pages,
+  IN  BOOLEAN   IsEncrypted
+  )
+{
+  RETURN_STATUS Ret;
+  UINTN Error;
+  UINTN EncryptMask;
+
+  Ret = RETURN_UNSUPPORTED;
+
+  if (MemEncryptSevLiveMigrationIsEnabled ()) {
+    Ret = RETURN_SUCCESS;
+    //
+    // The encryption bit is set/clear on the smallest page size, hence
+    // use the 4k page size in MAP_GPA_RANGE hypercall below.
+    //
+    // Also, when the GCD map is being walked and the c-bit being cleared
+    // from MMIO and NonExistent memory spaces, the physical address
+    // range being passed may not be page-aligned and adding an assert
+    // here prevents booting. Hence, rounding it down when calling
+    // SetMemoryEncDecHypercall3AsmStub below.
+    //
+
+    EncryptMask = IsEncrypted ? KVM_MAP_GPA_RANGE_ENCRYPTED :
+        KVM_MAP_GPA_RANGE_DECRYPTED;
+
+    Error = SetMemoryEncDecHypercall3AsmStub (
+              KVM_HC_MAP_GPA_RANGE,
+              PhysicalAddress & ~EFI_PAGE_MASK,
+              Pages,
+              KVM_MAP_GPA_RANGE_PAGE_SZ_4K | EncryptMask
+              );
+
+    if (Error != 0) {
+      DEBUG ((DEBUG_ERROR,
+              "SetMemoryEncDecHypercall3 failed, Phys = %Lx, Pages = %Ld, Err = %Ld\n",
+              PhysicalAddress,
+              Pages,
+              (INT64)Error));
+
+      Ret = RETURN_NO_MAPPING;
+    }
+  }
+
+  return Ret;
+}
-- 
2.17.1

[PATCH v6 3/6] OvmfPkg/BaseMemEncryptLib: Invoke page encryption state change hypercall

[Ashish Kalra]

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

Invoke the hypercall API to notify hypervisor when the page's
encryption state changes.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
index c696745f9d..f562e16fc2 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
@@ -585,6 +585,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 +639,9 @@ SetMemoryEncDec (
 
   Status = EFI_SUCCESS;
 
+  Size = Length;
+  CBitChanged = FALSE;
+  OrigPhysicalAddress = PhysicalAddress;
   while (Length != 0)
   {
     //
@@ -695,6 +701,7 @@ SetMemoryEncDec (
           ));
         PhysicalAddress += BIT30;
         Length -= BIT30;
+        CBitChanged = TRUE;
       } else {
         //
         // We must split the page
@@ -749,6 +756,7 @@ SetMemoryEncDec (
           SetOrClearCBit (&PageDirectory2MEntry->Uint64, Mode);
           PhysicalAddress += BIT21;
           Length -= BIT21;
+          CBitChanged = TRUE;
         } else {
           //
           // We must split up this page into 4K pages
@@ -791,6 +799,7 @@ SetMemoryEncDec (
         SetOrClearCBit (&PageTableEntry->Uint64, Mode);
         PhysicalAddress += EFI_PAGE_SIZE;
         Length -= EFI_PAGE_SIZE;
+        CBitChanged = TRUE;
       }
     }
   }
@@ -808,6 +817,17 @@ SetMemoryEncDec (
   //
   CpuFlushTlb();
 
+  //
+  // Notify Hypervisor on C-bit status
+  //
+  if (CBitChanged) {
+    Status = SetMemoryEncDecHypercall3 (
+               OrigPhysicalAddress,
+               EFI_SIZE_TO_PAGES(Size),
+               (Mode == SetCBit) ? TRUE : FALSE
+               );
+  }
+
 Done:
   //
   // Restore page table write protection, if any.
-- 
2.17.1

[PATCH v6 4/6] OvmfPkg/VmgExitLib: Encryption state change hypercall support in VC handler

[Ashish Kalra]

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

Make the #VC handler aware of the page encryption state
change hypercall by adding support to check KVM_HC_MAP_GPA_RANGE
hypercall and add the additional register values used by
hypercall in the GHCB.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
index 41b0c8cc53..2d06343e92 100644
--- a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
+++ b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
@@ -1171,6 +1171,19 @@ VmmCallExit (
   Ghcb->SaveArea.Cpl = (UINT8) (Regs->Cs & 0x3);
   VmgSetOffsetValid (Ghcb, GhcbCpl);
 
+  if (Regs->Rax == KVM_HC_MAP_GPA_RANGE) {
+    //
+    // KVM_HC_MAP_GPA_RANGE hypercall requires these
+    // extra registers.
+    //
+    Ghcb->SaveArea.Rbx = Regs->Rbx;
+    VmgSetOffsetValid (Ghcb, GhcbRbx);
+    Ghcb->SaveArea.Rcx = Regs->Rcx;
+    VmgSetOffsetValid (Ghcb, GhcbRcx);
+    Ghcb->SaveArea.Rdx = Regs->Rdx;
+    VmgSetOffsetValid (Ghcb, GhcbRdx);
+  }
+
   Status = VmgExit (Ghcb, SVM_EXIT_VMMCALL, 0, 0);
   if (Status != 0) {
     return Status;
-- 
2.17.1

[PATCH v6 5/6] OvmfPkg/PlatformPei: Mark SEC GHCB page as unencrypted via hypercall

[Ashish Kalra]

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

Mark the SEC GHCB page (that is mapped as unencrypted in
ResetVector code) in the hypervisor's guest page encryption
state tracking.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 OvmfPkg/PlatformPei/AmdSev.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index a8bf610022..1d38056ec0 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -52,6 +52,17 @@ AmdSevEsInitialize (
   PcdStatus = PcdSetBoolS (PcdSevEsIsEnabled, TRUE);
   ASSERT_RETURN_ERROR (PcdStatus);
 
+  //
+  // The SEC Ghcb setup during reset-vector needs to be marked as
+  // decrypted in the hypervisor's guest page encryption state
+  // tracking.
+  //
+  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.17.1

[PATCH v6 6/6] OvmfPkg/AmdSevDxe: Add support for SEV live migration.

[Ashish Kalra]

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

Check for SEV live migration feature support, if detected
setup a new UEFI enviroment variable to indicate OVMF
support for SEV live migration.

The new runtime UEFI environment variable is set via the
notification function registered for the
EFI_END_OF_DXE_EVENT_GROUP_GUID event in AmdSevDxe driver.

AmdSevDxe module is an apriori driver so it gets loaded between PEI
and DXE phases and the SetVariable call will fail at the driver's
entry point as the Variable DXE module is still not loaded yet.
So we need to wait for an event notification which is signaled
after the Variable DXE module is loaded, hence, using the
EndOfDxe event notification to make this call.

Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 OvmfPkg/AmdSevDxe/AmdSevDxe.c              | 64 ++++++++++++++++++++
 OvmfPkg/AmdSevDxe/AmdSevDxe.inf            |  4 ++
 OvmfPkg/Include/Guid/AmdSevMemEncryptLib.h | 20 ++++++
 OvmfPkg/OvmfPkg.dec                        |  1 +
 4 files changed, 89 insertions(+)

diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index c66c4e9b92..bfad71b9c6 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -15,10 +15,47 @@
 #include <Library/BaseMemoryLib.h>
 #include <Library/DebugLib.h>
 #include <Library/DxeServicesTableLib.h>
+#include <Library/UefiRuntimeServicesTableLib.h>
+#include <Library/UefiBootServicesTableLib.h>
 #include <Library/MemEncryptSevLib.h>
 #include <Library/MemoryAllocationLib.h>
+#include <Guid/AmdSevMemEncryptLib.h>
+#include <Guid/EventGroup.h>
 #include <Library/PcdLib.h>
 
+STATIC
+VOID
+EFIAPI
+AmdSevDxeOnEndOfDxe (
+  IN EFI_EVENT Event,
+  IN VOID      *EventToSignal
+  )
+{
+  EFI_STATUS Status;
+  BOOLEAN SevLiveMigrationEnabled;
+
+  SevLiveMigrationEnabled = MemEncryptSevLiveMigrationIsEnabled();
+
+  if (SevLiveMigrationEnabled) {
+    Status = gRT->SetVariable (
+               L"SevLiveMigrationEnabled",
+               &gAmdSevMemEncryptGuid,
+               EFI_VARIABLE_NON_VOLATILE |
+               EFI_VARIABLE_BOOTSERVICE_ACCESS |
+               EFI_VARIABLE_RUNTIME_ACCESS,
+               sizeof SevLiveMigrationEnabled,
+               &SevLiveMigrationEnabled
+               );
+
+    DEBUG ((
+      DEBUG_INFO,
+      "%a: Setting SevLiveMigrationEnabled variable, status = %lx\n",
+      __FUNCTION__,
+      Status
+      ));
+  }
+}
+
 EFI_STATUS
 EFIAPI
 AmdSevDxeEntryPoint (
@@ -30,6 +67,7 @@ AmdSevDxeEntryPoint (
   EFI_GCD_MEMORY_SPACE_DESCRIPTOR  *AllDescMap;
   UINTN                            NumEntries;
   UINTN                            Index;
+  EFI_EVENT                        Event;
 
   //
   // Do nothing when SEV is not enabled
@@ -130,5 +168,31 @@ AmdSevDxeEntryPoint (
     }
   }
 
+  //
+  // AmdSevDxe module is an apriori driver so it gets loaded between PEI
+  // and DXE phases and the SetVariable call will fail at the driver's
+  // entry point as the Variable DXE module is still not loaded yet.
+  // So we need to wait for an event notification which is signaled
+  // after the Variable DXE module is loaded, hence, using the
+  // EndOfDxe event notification to make this call.
+  //
+  // Register EFI_END_OF_DXE_EVENT_GROUP_GUID event.
+  // The notification function sets the runtime variable indicating OVMF
+  // support for SEV live migration.
+  //
+  Status = gBS->CreateEventEx (
+                  EVT_NOTIFY_SIGNAL,
+                  TPL_CALLBACK,
+                  AmdSevDxeOnEndOfDxe,
+                  NULL,
+                  &gEfiEndOfDxeEventGroupGuid,
+                  &Event
+                  );
+
+  if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_INFO, "%a: CreateEventEx(): %r\n",
+        __FUNCTION__, Status));
+  }
+
   return EFI_SUCCESS;
 }
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
index 0676fcc5b6..2ad1fb8632 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
@@ -45,3 +45,7 @@
 
 [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
+
+[Guids]
+  gAmdSevMemEncryptGuid
+  gEfiEndOfDxeEventGroupGuid         ## CONSUMES   ## Event
diff --git a/OvmfPkg/Include/Guid/AmdSevMemEncryptLib.h b/OvmfPkg/Include/Guid/AmdSevMemEncryptLib.h
new file mode 100644
index 0000000000..8ab283860b
--- /dev/null
+++ b/OvmfPkg/Include/Guid/AmdSevMemEncryptLib.h
@@ -0,0 +1,20 @@
+/** @file
+
+  AMD Memory Encryption GUID, define a new GUID for defining
+  new UEFI environment variables assocaiated with SEV Memory Encryption.
+
+  Copyright (c) 2021, AMD Inc. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef __AMD_SEV_MEMENCRYPT_LIB_H__
+#define __AMD_SEV_MEMENCRYPT_LIB_H__
+
+#define AMD_SEV_MEMENCRYPT_GUID \
+{0x0cf29b71, 0x9e51, 0x433a, {0xa3, 0xb7, 0x81, 0xf3, 0xab, 0x16, 0xb8, 0x75}}
+
+extern EFI_GUID gAmdSevMemEncryptGuid;
+
+#endif
diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 2ab27f0c73..3978852557 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -125,6 +125,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}}
+  gAmdSevMemEncryptGuid                 = {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
-- 
2.17.1

Re: [PATCH v6 1/6] OvmfPkg/BaseMemEncryptLib: Detect SEV live migration feature.

[Lendacky, Thomas]

On 8/2/21 7:31 AM, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> Add support to check if we are running inside KVM HVM and
> KVM HVM supports SEV Live Migration feature.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>  OvmfPkg/Include/Library/MemEncryptSevLib.h                            | 27 ++++++++++
>  OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c    | 39 +++++++++++++++
>  OvmfPkg/Library/BaseMemEncryptSevLib/PeiDxeMemEncryptSevLibInternal.c | 52 ++++++++++++++++++++
>  OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c    | 39 +++++++++++++++
>  OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c    | 18 +++++++
>  5 files changed, 175 insertions(+)
> 
> diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h b/OvmfPkg/Include/Library/MemEncryptSevLib.h
> index 76d06c206c..59f694fb8a 100644
> --- a/OvmfPkg/Include/Library/MemEncryptSevLib.h
> +++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
> @@ -90,6 +90,18 @@ MemEncryptSevIsEnabled (
>    VOID
>    );
>  
> +/**
> +  Returns a boolean to indicate whether SEV live migration is enabled.
> +
> +  @retval TRUE           SEV live migration is enabled
> +  @retval FALSE          SEV live migration is not enabled
> +**/
> +BOOLEAN
> +EFIAPI
> +MemEncryptSevLiveMigrationIsEnabled (
> +  VOID
> +  );
> +
>  /**
>    This function clears memory encryption bit for the memory region specified by
>    BaseAddress and NumPages from the current page table context.
> @@ -222,4 +234,19 @@ MemEncryptSevClearMmioPageEncMask (
>    IN UINTN                    NumPages
>    );
>  
> +#define KVM_FEATURE_MIGRATION_CONTROL   BIT17
> +
> +/**
> +  Figures out if we are running inside KVM HVM and
> +  KVM HVM supports SEV Live Migration feature.
> +
> +  @retval TRUE           SEV live migration is supported.
> +  @retval FALSE          SEV live migration is not supported.
> +**/
> +BOOLEAN
> +EFIAPI
> +KvmDetectSevLiveMigrationFeature(
> +  VOID
> +  );
> +

I don't think KvmDetectSevLiveMigrationFeature() should be in
OvmfPkg/Include/Library/MemEncryptSevLib.h since it isn't called except as
a helper by InternalDetectSevLiveMigrationFeature(). You should probably
create a new PeiDxeMemEncryptSevLibInternal.h header file for that
function that lives in OvmfPkg/Library/BaseMemEncryptSevLib.

>  #endif // _MEM_ENCRYPT_SEV_LIB_H_
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c
> index 2816f859a0..ead754cd7b 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c
> @@ -20,6 +20,8 @@
>  STATIC BOOLEAN mSevStatus = FALSE;
>  STATIC BOOLEAN mSevEsStatus = FALSE;
>  STATIC BOOLEAN mSevStatusChecked = FALSE;
> +STATIC BOOLEAN mSevLiveMigrationStatus = FALSE;
> +STATIC BOOLEAN mSevLiveMigrationStatusChecked = FALSE;
>  
>  STATIC UINT64  mSevEncryptionMask = 0;
>  STATIC BOOLEAN mSevEncryptionMaskSaved = FALSE;
> @@ -87,6 +89,24 @@ InternalMemEncryptSevStatus (
>    mSevStatusChecked = TRUE;
>  }
>  
> +/**
> +  Figures out if we are running inside KVM HVM and
> +  KVM HVM supports SEV Live Migration feature.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +InternalDetectSevLiveMigrationFeature(
> +  VOID
> +  )
> +{
> +  if (KvmDetectSevLiveMigrationFeature()) {

Add a space before the "()"

> +        mSevLiveMigrationStatus = TRUE;
> +  }
> +
> +  mSevLiveMigrationStatusChecked = TRUE;
> +}
> +
>  /**
>    Returns a boolean to indicate whether SEV-ES is enabled.
>  
> @@ -125,6 +145,25 @@ MemEncryptSevIsEnabled (
>    return mSevStatus;
>  }
>  
> +/**
> +  Returns a boolean to indicate whether SEV live migration is enabled.
> +
> +  @retval TRUE           SEV live migration is enabled
> +  @retval FALSE          SEV live migration is not enabled
> +**/
> +BOOLEAN
> +EFIAPI
> +MemEncryptSevLiveMigrationIsEnabled (
> +  VOID
> +  )
> +{
> +  if (!mSevLiveMigrationStatusChecked) {
> +    InternalDetectSevLiveMigrationFeature ();
> +  }
> +
> +  return mSevLiveMigrationStatus;
> +}
> +
>  /**
>    Returns the SEV encryption mask.
>  
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiDxeMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiDxeMemEncryptSevLibInternal.c
> index b4a9f464e2..d7fc973134 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiDxeMemEncryptSevLibInternal.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiDxeMemEncryptSevLibInternal.c
> @@ -61,3 +61,55 @@ MemEncryptSevLocateInitialSmramSaveStateMapPages (
>  
>    return RETURN_SUCCESS;
>  }
> +
> +/**
> +  Figures out if we are running inside KVM HVM and
> +  KVM HVM supports SEV Live Migration feature.
> +
> +  @retval TRUE           SEV live migration is supported.
> +  @retval FALSE          SEV live migration is not supported.
> +**/
> +BOOLEAN
> +EFIAPI
> +KvmDetectSevLiveMigrationFeature(

Add a space before the "("

> +  VOID
> +  )
> +{
> +  CHAR8 Signature[13];
> +  UINT32 mKvmLeaf;
> +  UINT32 RegEax, RegEbx, RegEcx, RegEdx;

Coding style requires these to be four separate declarations.

> +
> +  Signature[12] = '\0';
> +  for (mKvmLeaf = 0x40000000; mKvmLeaf < 0x40010000; mKvmLeaf += 0x100) {

I still really don't understand the need for the CPUID loop. KVM only ever
programs CPUID function 0x40000000, right?

> +    AsmCpuid (
> +      mKvmLeaf,
> +      NULL,
> +      (UINT32 *) &Signature[0],
> +      (UINT32 *) &Signature[4],
> +      (UINT32 *) &Signature[8]);
> +
> +    if (AsciiStrCmp (Signature, "KVMKVMKVM") == 0) {
> +      DEBUG ((
> +        DEBUG_INFO,
> +        "%a: KVM Detected, signature = %a\n",
> +        __FUNCTION__,
> +        Signature
> +        ));
> +
> +      RegEax = mKvmLeaf + 1;
> +      RegEcx = 0;
> +      AsmCpuid (mKvmLeaf + 1, &RegEax, &RegEbx, &RegEcx, &RegEdx);
> +      if ((RegEax & KVM_FEATURE_MIGRATION_CONTROL) != 0) {
> +        DEBUG ((
> +          DEBUG_INFO,
> +          "%a: SEV Live Migration feature supported\n",
> +          __FUNCTION__
> +          ));
> +
> +        return TRUE;
> +      }
> +    }
> +  }
> +
> +  return FALSE;
> +}
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
> index e2fd109d12..9db6c2ef71 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
> @@ -20,6 +20,8 @@
>  STATIC BOOLEAN mSevStatus = FALSE;
>  STATIC BOOLEAN mSevEsStatus = FALSE;
>  STATIC BOOLEAN mSevStatusChecked = FALSE;
> +STATIC BOOLEAN mSevLiveMigrationStatus = FALSE;
> +STATIC BOOLEAN mSevLiveMigrationStatusChecked = FALSE;
>  
>  STATIC UINT64  mSevEncryptionMask = 0;
>  STATIC BOOLEAN mSevEncryptionMaskSaved = FALSE;
> @@ -87,6 +89,24 @@ InternalMemEncryptSevStatus (
>    mSevStatusChecked = TRUE;
>  }
>  
> +/**
> +  Figures out if we are running inside KVM HVM and
> +  KVM HVM supports SEV Live Migration feature.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +InternalDetectSevLiveMigrationFeature(

Add a space before "("

> +  VOID
> +  )
> +{
> +  if (KvmDetectSevLiveMigrationFeature()) {

Add a space before "()"

Thanks,
Tom

> +    mSevLiveMigrationStatus = TRUE;
> +  }
> +
> +  mSevLiveMigrationStatusChecked = TRUE;
> +}
> +
>  /**
>    Returns a boolean to indicate whether SEV-ES is enabled.
>  
> @@ -125,6 +145,25 @@ MemEncryptSevIsEnabled (
>    return mSevStatus;
>  }
>  
> +/**
> +  Returns a boolean to indicate whether SEV live migration is enabled.
> +
> +  @retval TRUE           SEV live migration is enabled
> +  @retval FALSE          SEV live migration is not enabled
> +**/
> +BOOLEAN
> +EFIAPI
> +MemEncryptSevLiveMigrationIsEnabled (
> +  VOID
> +  )
> +{
> +  if (!mSevLiveMigrationStatusChecked) {
> +    InternalDetectSevLiveMigrationFeature ();
> +  }
> +
> +  return mSevLiveMigrationStatus;
> +}
> +
>  /**
>    Returns the SEV encryption mask.
>  
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
> index 56d8f3f318..d9f7befcd2 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
> @@ -100,6 +100,24 @@ MemEncryptSevIsEnabled (
>    return Msr.Bits.SevBit ? TRUE : FALSE;
>  }
>  
> +/**
> +  Returns a boolean to indicate whether SEV live migration is enabled.
> +
> +  @retval TRUE           SEV live migration is enabled
> +  @retval FALSE          SEV live migration is not enabled
> +**/
> +BOOLEAN
> +EFIAPI
> +MemEncryptSevLiveMigrationIsEnabled (
> +  VOID
> +  )
> +{
> +  //
> +  // Not used in SEC phase.
> +  //
> +  return FALSE;
> +}
> +
>  /**
>    Returns the SEV encryption mask.
>  
> 

Re: [PATCH v6 2/6] OvmfPkg/BaseMemEncryptLib: Hypercall API for page encryption state change

[Lendacky, Thomas]

On 8/2/21 7:31 AM, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> Add API to issue hypercall on page encryption state change.
> 
> 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.
> 
> This hypercall is used to notify hypervisor when the page's
> encryption state changes.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>  OvmfPkg/Include/Library/MemEncryptSevLib.h                         | 43 +++++++++++++
>  OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf       |  1 +
>  OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c       | 27 +++++++++
>  OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf       |  1 +
>  OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c | 20 ++++++
>  OvmfPkg/Library/BaseMemEncryptSevLib/X64/AsmHelperStub.nasm        | 33 ++++++++++
>  OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c        | 64 ++++++++++++++++++++
>  7 files changed, 189 insertions(+)
> 
> diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h b/OvmfPkg/Include/Library/MemEncryptSevLib.h
> index 59f694fb8a..56cc7bb958 100644
> --- a/OvmfPkg/Include/Library/MemEncryptSevLib.h
> +++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
> @@ -249,4 +249,47 @@ KvmDetectSevLiveMigrationFeature(
>    VOID
>    );
>  
> +/**
> + This hypercall is used to notify hypervisor when the page's encryption
> + state changes.
> +
> + @param[in]   PhysicalAddress       The physical address that is the start address
> +                                    of a memory region.
> + @param[in]   Pages                 Number of pages in memory region.
> + @param[in]   IsEncrypted           Encrypted or Decrypted.
> +
> + @retval RETURN_SUCCESS             Hypercall returned success.
> + @retval RETURN_UNSUPPORTED         Hypercall not supported.
> + @retval RETURN_NO_MAPPING          Hypercall returned error.
> +**/
> +RETURN_STATUS
> +EFIAPI
> +SetMemoryEncDecHypercall3 (
> +  IN  UINTN     PhysicalAddress,
> +  IN  UINTN     Pages,
> +  IN  BOOLEAN   IsEncrypted
> +  );
> +
> +#define KVM_HC_MAP_GPA_RANGE   12
> +#define KVM_MAP_GPA_RANGE_PAGE_SZ_4K    0
> +#define KVM_MAP_GPA_RANGE_PAGE_SZ_2M    BIT0
> +#define KVM_MAP_GPA_RANGE_PAGE_SZ_1G    BIT1
> +#define KVM_MAP_GPA_RANGE_ENC_STAT(n)   ((n) << 4)

s/STAT/STATE/ ?

> +#define KVM_MAP_GPA_RANGE_ENCRYPTED     KVM_MAP_GPA_RANGE_ENC_STAT(1)
> +#define KVM_MAP_GPA_RANGE_DECRYPTED     KVM_MAP_GPA_RANGE_ENC_STAT(0)
> +
> +/**
> +  Interface exposed by the ASM implementation of the core hypercall

Need to put the function parameters in the comment here.

> +
> +  @retval Hypercall returned status.
> +**/
> +UINTN
> +EFIAPI
> +SetMemoryEncDecHypercall3AsmStub (
> +  IN  UINTN  HypercallNum,
> +  IN  UINTN  PhysicalAddress,
> +  IN  UINTN  Pages,
> +  IN  UINTN  Attributes
> +  );
> +
>  #endif // _MEM_ENCRYPT_SEV_LIB_H_
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
> index f2e162d680..0c28afadee 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
> @@ -38,6 +38,7 @@
>    X64/PeiDxeVirtualMemory.c
>    X64/VirtualMemory.c
>    X64/VirtualMemory.h
> +  X64/AsmHelperStub.nasm
>  
>  [Sources.IA32]
>    Ia32/MemEncryptSevLib.c
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
> index be260e0d10..516d639489 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
> @@ -136,3 +136,30 @@ MemEncryptSevClearMmioPageEncMask (
>    //
>    return RETURN_UNSUPPORTED;
>  }
> +
> +/**
> + This hyercall is used to notify hypervisor when the page's encryption
> + state changes.
> +
> + @param[in]   PhysicalAddress       The physical address that is the start address
> +                                    of a memory region.
> + @param[in]   Pages                 Number of Pages in the memory region.
> + @param[in]   IsEncrypted           Encrypted or Decrypted.
> +
> + @retval RETURN_SUCCESS             Hypercall returned success.
> + @retval RETURN_UNSUPPORTED         Hypercall not supported.
> + @retval RETURN_NO_MAPPING          Hypercall returned error.
> +**/
> +RETURN_STATUS
> +EFIAPI
> +SetMemoryEncDecHypercall3 (
> +  IN  UINTN     PhysicalAddress,
> +  IN  UINTN     Pages,
> +  IN  BOOLEAN   IsEncrypted
> +  )
> +{
> +  //
> +  // Memory encryption bit is not accessible in 32-bit mode
> +  //
> +  return RETURN_UNSUPPORTED;
> +}
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
> index 03a78c32df..3233ca7979 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
> @@ -38,6 +38,7 @@
>    X64/PeiDxeVirtualMemory.c
>    X64/VirtualMemory.c
>    X64/VirtualMemory.h
> +  X64/AsmHelperStub.nasm
>  
>  [Sources.IA32]
>    Ia32/MemEncryptSevLib.c
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
> index d9f7befcd2..ebb1c39319 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
> @@ -118,6 +118,26 @@ MemEncryptSevLiveMigrationIsEnabled (
>    return FALSE;
>  }
>  
> +/**
> +  Interface exposed by the ASM implementation of the core hypercall
> +
> +  @retval Hypercall returned status.
> +**/
> +UINTN
> +EFIAPI
> +SetMemoryEncDecHypercall3AsmStub (
> +  IN  UINTN  HypercallNum,
> +  IN  UINTN  PhysicalAddress,
> +  IN  UINTN  Pages,
> +  IN  UINTN  Attributes
> +  )
> +{
> +  //
> +  // Not used in SEC phase.
> +  //
> +  return RETURN_UNSUPPORTED;
> +}
> +
>  /**
>    Returns the SEV encryption mask.
>  
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/AsmHelperStub.nasm b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/AsmHelperStub.nasm
> new file mode 100644
> index 0000000000..0ec35dd9b6
> --- /dev/null
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/AsmHelperStub.nasm
> @@ -0,0 +1,33 @@
> +/** @file
> +
> +  ASM helper stub to invoke hypercall
> +
> +  Copyright (c) 2021, AMD Incorporated. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +DEFAULT REL
> +SECTION .text
> +
> +; UINTN
> +; EFIAPI
> +; SetMemoryEncDecHypercall3AsmStub (
> +;   IN UINTN HypercallNum,
> +;   IN UINTN Arg1,
> +;   IN UINTN Arg2,
> +;   IN UINTN Arg3
> +;   );
> +global ASM_PFX(SetMemoryEncDecHypercall3AsmStub)
> +ASM_PFX(SetMemoryEncDecHypercall3AsmStub):
> +  ; UEFI calling conventions require RBX to
> +  ; be nonvolatile/callee-saved.
> +  push rbx
> +  mov rax, rcx    ; Copy HypercallNumber to rax
> +  mov rbx, rdx    ; Copy Arg1 to the register expected by KVM
> +  mov rcx, r8     ; Copy Arg2 to register expected by KVM
> +  mov rdx, r9     ; Copy Arg3 to register expected by KVM
> +  vmmcall         ; Call VMMCALL
> +  pop rbx
> +  ret
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
> index a57e8fd37f..fa679c9fc9 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
> @@ -143,3 +143,67 @@ MemEncryptSevClearMmioPageEncMask (
>             );
>  
>  }
> +
> +/**
> + This hyercall is used to notify hypervisor when the page's encryption
> + state changes.
> +
> + @param[in]   PhysicalAddress       The physical address that is the start address
> +                                    of a memory region.
> + @param[in]   Pages                 Number of Pages in the memory region.
> + @param[in]   IsEncrypted           Encrypted or Decrypted.
> +
> + @retval RETURN_SUCCESS             Hypercall returned success.
> + @retval RETURN_UNSUPPORTED         Hypercall not supported.
> + @retval RETURN_NO_MAPPING          Hypercall returned error.
> +**/
> +RETURN_STATUS
> +EFIAPI
> +SetMemoryEncDecHypercall3 (
> +  IN  UINTN     PhysicalAddress,
> +  IN  UINTN     Pages,
> +  IN  BOOLEAN   IsEncrypted
> +  )
> +{
> +  RETURN_STATUS Ret;
> +  UINTN Error;
> +  UINTN EncryptMask;
> +
> +  Ret = RETURN_UNSUPPORTED;
> +
> +  if (MemEncryptSevLiveMigrationIsEnabled ()) {
> +    Ret = RETURN_SUCCESS;
> +    //
> +    // The encryption bit is set/clear on the smallest page size, hence
> +    // use the 4k page size in MAP_GPA_RANGE hypercall below.
> +    //
> +    // Also, when the GCD map is being walked and the c-bit being cleared
> +    // from MMIO and NonExistent memory spaces, the physical address
> +    // range being passed may not be page-aligned and adding an assert
> +    // here prevents booting. Hence, rounding it down when calling
> +    // SetMemoryEncDecHypercall3AsmStub below.
> +    //
> +
> +    EncryptMask = IsEncrypted ? KVM_MAP_GPA_RANGE_ENCRYPTED :
> +        KVM_MAP_GPA_RANGE_DECRYPTED;

Just a nit, but EncryptMask is a bit confusing because is sounds like the
encryption mask used by SEV, but it's really the page encryption state as
defined by the hypercall, maybe call it EncryptionState or EncryptState?

> +
> +    Error = SetMemoryEncDecHypercall3AsmStub (
> +              KVM_HC_MAP_GPA_RANGE,
> +              PhysicalAddress & ~EFI_PAGE_MASK,
> +              Pages,
> +              KVM_MAP_GPA_RANGE_PAGE_SZ_4K | EncryptMask
> +              );
> +
> +    if (Error != 0) {
> +      DEBUG ((DEBUG_ERROR,
> +              "SetMemoryEncDecHypercall3 failed, Phys = %Lx, Pages = %Ld, Err = %Ld\n",

I don't believe the "L" is needed for "Phys" and "Pages" since those are
UINTN variables.

> +              PhysicalAddress,
> +              Pages,
> +              (INT64)Error));

Indentation needs to be two spaces past the "DEBUG" function call.

Thanks,
Tom

> +
> +      Ret = RETURN_NO_MAPPING;
> +    }
> +  }
> +
> +  return Ret;
> +}
> 

Re: [PATCH v6 6/6] OvmfPkg/AmdSevDxe: Add support for SEV live migration.

[Lendacky, Thomas]

On 8/2/21 7:33 AM, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> Check for SEV live migration feature support, if detected
> setup a new UEFI enviroment variable to indicate OVMF
> support for SEV live migration.
> 
> The new runtime UEFI environment variable is set via the
> notification function registered for the
> EFI_END_OF_DXE_EVENT_GROUP_GUID event in AmdSevDxe driver.
> 
> AmdSevDxe module is an apriori driver so it gets loaded between PEI
> and DXE phases and the SetVariable call will fail at the driver's
> entry point as the Variable DXE module is still not loaded yet.
> So we need to wait for an event notification which is signaled
> after the Variable DXE module is loaded, hence, using the
> EndOfDxe event notification to make this call.
> 
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>  OvmfPkg/AmdSevDxe/AmdSevDxe.c              | 64 ++++++++++++++++++++
>  OvmfPkg/AmdSevDxe/AmdSevDxe.inf            |  4 ++
>  OvmfPkg/Include/Guid/AmdSevMemEncryptLib.h | 20 ++++++
>  OvmfPkg/OvmfPkg.dec                        |  1 +
>  4 files changed, 89 insertions(+)
> 
> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> index c66c4e9b92..bfad71b9c6 100644
> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> @@ -15,10 +15,47 @@
>  #include <Library/BaseMemoryLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/DxeServicesTableLib.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
>  #include <Library/MemEncryptSevLib.h>
>  #include <Library/MemoryAllocationLib.h>
> +#include <Guid/AmdSevMemEncryptLib.h>
> +#include <Guid/EventGroup.h>
>  #include <Library/PcdLib.h>
>  
> +STATIC
> +VOID
> +EFIAPI
> +AmdSevDxeOnEndOfDxe (
> +  IN EFI_EVENT Event,
> +  IN VOID      *EventToSignal
> +  )
> +{
> +  EFI_STATUS Status;
> +  BOOLEAN SevLiveMigrationEnabled;
> +
> +  SevLiveMigrationEnabled = MemEncryptSevLiveMigrationIsEnabled();
> +
> +  if (SevLiveMigrationEnabled) {
> +    Status = gRT->SetVariable (
> +               L"SevLiveMigrationEnabled",
> +               &gAmdSevMemEncryptGuid,
> +               EFI_VARIABLE_NON_VOLATILE |
> +               EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +               EFI_VARIABLE_RUNTIME_ACCESS,
> +               sizeof SevLiveMigrationEnabled,
> +               &SevLiveMigrationEnabled
> +               );
> +
> +    DEBUG ((
> +      DEBUG_INFO,
> +      "%a: Setting SevLiveMigrationEnabled variable, status = %lx\n",
> +      __FUNCTION__,
> +      Status
> +      ));
> +  }
> +}
> +
>  EFI_STATUS
>  EFIAPI
>  AmdSevDxeEntryPoint (
> @@ -30,6 +67,7 @@ AmdSevDxeEntryPoint (
>    EFI_GCD_MEMORY_SPACE_DESCRIPTOR  *AllDescMap;
>    UINTN                            NumEntries;
>    UINTN                            Index;
> +  EFI_EVENT                        Event;
>  
>    //
>    // Do nothing when SEV is not enabled
> @@ -130,5 +168,31 @@ AmdSevDxeEntryPoint (
>      }
>    }
>  
> +  //
> +  // AmdSevDxe module is an apriori driver so it gets loaded between PEI
> +  // and DXE phases and the SetVariable call will fail at the driver's
> +  // entry point as the Variable DXE module is still not loaded yet.
> +  // So we need to wait for an event notification which is signaled
> +  // after the Variable DXE module is loaded, hence, using the
> +  // EndOfDxe event notification to make this call.
> +  //
> +  // Register EFI_END_OF_DXE_EVENT_GROUP_GUID event.
> +  // The notification function sets the runtime variable indicating OVMF
> +  // support for SEV live migration.
> +  //
> +  Status = gBS->CreateEventEx (
> +                  EVT_NOTIFY_SIGNAL,
> +                  TPL_CALLBACK,
> +                  AmdSevDxeOnEndOfDxe,
> +                  NULL,
> +                  &gEfiEndOfDxeEventGroupGuid,
> +                  &Event
> +                  );
> +
> +  if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_INFO, "%a: CreateEventEx(): %r\n",

DEBUG_ERROR?

> +        __FUNCTION__, Status));

Should there be an "ASSERT_EFI_ERROR (Status)" after the DEBUG call?

Thanks,
Tom

> +  }
> +
>    return EFI_SUCCESS;
>  }
> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
> index 0676fcc5b6..2ad1fb8632 100644
> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
> @@ -45,3 +45,7 @@
>  
>  [Pcd]
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
> +
> +[Guids]
> +  gAmdSevMemEncryptGuid
> +  gEfiEndOfDxeEventGroupGuid         ## CONSUMES   ## Event
> diff --git a/OvmfPkg/Include/Guid/AmdSevMemEncryptLib.h b/OvmfPkg/Include/Guid/AmdSevMemEncryptLib.h
> new file mode 100644
> index 0000000000..8ab283860b
> --- /dev/null
> +++ b/OvmfPkg/Include/Guid/AmdSevMemEncryptLib.h
> @@ -0,0 +1,20 @@
> +/** @file
> +
> +  AMD Memory Encryption GUID, define a new GUID for defining
> +  new UEFI environment variables assocaiated with SEV Memory Encryption.
> +
> +  Copyright (c) 2021, AMD Inc. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef __AMD_SEV_MEMENCRYPT_LIB_H__
> +#define __AMD_SEV_MEMENCRYPT_LIB_H__
> +
> +#define AMD_SEV_MEMENCRYPT_GUID \
> +{0x0cf29b71, 0x9e51, 0x433a, {0xa3, 0xb7, 0x81, 0xf3, 0xab, 0x16, 0xb8, 0x75}}
> +
> +extern EFI_GUID gAmdSevMemEncryptGuid;
> +
> +#endif
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index 2ab27f0c73..3978852557 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -125,6 +125,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}}
> +  gAmdSevMemEncryptGuid                 = {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
> 

Re: [PATCH v6 1/6] OvmfPkg/BaseMemEncryptLib: Detect SEV live migration feature.

[Ashish Kalra]

Hello Tom,

On Mon, Aug 09, 2021 at 08:41:27AM -0500, Tom Lendacky wrote:
> On 8/2/21 7:31 AM, Ashish Kalra wrote:
> > +
> > +  Signature[12] = '\0';
> > +  for (mKvmLeaf = 0x40000000; mKvmLeaf < 0x40010000; mKvmLeaf += 0x100) {
> 
> I still really don't understand the need for the CPUID loop. KVM only ever
> programs CPUID function 0x40000000, right?
> 

Yes KVM only programs CPUID function 0x40000000, as do other hypervisors
like Hyper-V. Also mentioned that leaf 0x40000000 is the Hypervisor
CPUID leaf range and vendor ID signature in MSFT Hypervisor Interface
document.

But looking at linux kernel code for the same functionality : 

static inline uint32_t hypervisor_cpuid_base(const char *sig, uint32_t leaves)
{
        uint32_t base, eax, signature[3];

        for (base = 0x40000000; base < 0x40010000; base += 0x100) {
                cpuid(base, &eax, &signature[0], &signature[1], &signature[2]);

                if (!memcmp(sig, signature, 12) 
		...
		...


And the Xen detection code in OVMF: 

  for (mXenLeaf = 0x40000000; mXenLeaf < 0x40010000; mXenLeaf += 0x100) {
    AsmCpuid (mXenLeaf,
              NULL,
              (UINT32 *) &Signature[0],
              (UINT32 *) &Signature[4],
              (UINT32 *) &Signature[8]);

    if (!AsciiStrCmp ((CHAR8 *) Signature, "XenVMMXenVMM")) {
      return TRUE;

The above functions are doing a loop-test.

The kernel patch also mentions about the loop-test :
https://lore.kernel.org/kvm/51FF1E26.6010707@redhat.com/t/

This patch introduce hypervisor_cpuid_base() which loop test the hypervisor
existence function until the signature match and check the number of leaves if
required. This could be used by Xen/KVM guest to detect the existence of
hypervisor.

The above patches/functions don't have any additonal documentation for
why are they doing the loop-test ? 

I don't want to miss any functionality, hence i am reusing the same
loop-test code.

Thanks,
Ashish

> > +    AsmCpuid (
> > +      mKvmLeaf,
> > +      NULL,
> > +      (UINT32 *) &Signature[0],
> > +      (UINT32 *) &Signature[4],
> > +      (UINT32 *) &Signature[8]);
> > +
> > +    if (AsciiStrCmp (Signature, "KVMKVMKVM") == 0) {
> > +      DEBUG ((
> > +        DEBUG_INFO,
> > +        "%a: KVM Detected, signature = %a\n",
> > +        __FUNCTION__,
> > +        Signature
> > +        ));
> > +
> > +      RegEax = mKvmLeaf + 1;
> > +      RegEcx = 0;
> > +      AsmCpuid (mKvmLeaf + 1, &RegEax, &RegEbx, &RegEcx, &RegEdx);
> > +      if ((RegEax & KVM_FEATURE_MIGRATION_CONTROL) != 0) {
> > +        DEBUG ((
> > +          DEBUG_INFO,
> > +          "%a: SEV Live Migration feature supported\n",
> > +          __FUNCTION__
> > +          ));
> > +
> > +        return TRUE;
> > +      }
> > +    }
> > +  }
> > +
> > +  return FALSE;
> > +}

Re: [edk2-devel] [PATCH v6 1/6] OvmfPkg/BaseMemEncryptLib: Detect SEV live migration feature.

[Gerd Hoffmann]

  Hi,

> > I still really don't understand the need for the CPUID loop. KVM only ever
> > programs CPUID function 0x40000000, right?

Nope.  When you enable hyper-v emulation features you'll go find the kvm
cpuid @ 0x40000000 and the hyper-v cpuid @ 0x40000100 (or the other way
around, not sure).

take care,
  Gerd

Re: [PATCH v6 6/6] OvmfPkg/AmdSevDxe: Add support for SEV live migration.

[Ashish Kalra]

Hello Tom,

On Mon, Aug 09, 2021 at 09:29:29AM -0500, Tom Lendacky wrote:
> On 8/2/21 7:33 AM, Ashish Kalra wrote:
> > From: Ashish Kalra <ashish.kalra@amd.com>
> > 
> > Check for SEV live migration feature support, if detected
> > setup a new UEFI enviroment variable to indicate OVMF
> > support for SEV live migration.
> > 
> > The new runtime UEFI environment variable is set via the
> > notification function registered for the
> > EFI_END_OF_DXE_EVENT_GROUP_GUID event in AmdSevDxe driver.
> > 
> > AmdSevDxe module is an apriori driver so it gets loaded between PEI
> > and DXE phases and the SetVariable call will fail at the driver's
> > entry point as the Variable DXE module is still not loaded yet.
> > So we need to wait for an event notification which is signaled
> > after the Variable DXE module is loaded, hence, using the
> > EndOfDxe event notification to make this call.
> > 
> > Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> > ---
> >  OvmfPkg/AmdSevDxe/AmdSevDxe.c              | 64 ++++++++++++++++++++
> >  OvmfPkg/AmdSevDxe/AmdSevDxe.inf            |  4 ++
> >  OvmfPkg/Include/Guid/AmdSevMemEncryptLib.h | 20 ++++++
> >  OvmfPkg/OvmfPkg.dec                        |  1 +
> >  4 files changed, 89 insertions(+)
> > 
> > diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > index c66c4e9b92..bfad71b9c6 100644
> > --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > @@ -15,10 +15,47 @@
> >  #include <Library/BaseMemoryLib.h>
> >  #include <Library/DebugLib.h>
> >  #include <Library/DxeServicesTableLib.h>
> > +#include <Library/UefiRuntimeServicesTableLib.h>
> > +#include <Library/UefiBootServicesTableLib.h>
> >  #include <Library/MemEncryptSevLib.h>
> >  #include <Library/MemoryAllocationLib.h>
> > +#include <Guid/AmdSevMemEncryptLib.h>
> > +#include <Guid/EventGroup.h>
> >  #include <Library/PcdLib.h>
> >  
> > +STATIC
> > +VOID
> > +EFIAPI
> > +AmdSevDxeOnEndOfDxe (
> > +  IN EFI_EVENT Event,
> > +  IN VOID      *EventToSignal
> > +  )
> > +{
> > +  EFI_STATUS Status;
> > +  BOOLEAN SevLiveMigrationEnabled;
> > +
> > +  SevLiveMigrationEnabled = MemEncryptSevLiveMigrationIsEnabled();
> > +
> > +  if (SevLiveMigrationEnabled) {
> > +    Status = gRT->SetVariable (
> > +               L"SevLiveMigrationEnabled",
> > +               &gAmdSevMemEncryptGuid,
> > +               EFI_VARIABLE_NON_VOLATILE |
> > +               EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > +               EFI_VARIABLE_RUNTIME_ACCESS,
> > +               sizeof SevLiveMigrationEnabled,
> > +               &SevLiveMigrationEnabled
> > +               );
> > +
> > +    DEBUG ((
> > +      DEBUG_INFO,
> > +      "%a: Setting SevLiveMigrationEnabled variable, status = %lx\n",
> > +      __FUNCTION__,
> > +      Status
> > +      ));
> > +  }
> > +}
> > +
> >  EFI_STATUS
> >  EFIAPI
> >  AmdSevDxeEntryPoint (
> > @@ -30,6 +67,7 @@ AmdSevDxeEntryPoint (
> >    EFI_GCD_MEMORY_SPACE_DESCRIPTOR  *AllDescMap;
> >    UINTN                            NumEntries;
> >    UINTN                            Index;
> > +  EFI_EVENT                        Event;
> >  
> >    //
> >    // Do nothing when SEV is not enabled
> > @@ -130,5 +168,31 @@ AmdSevDxeEntryPoint (
> >      }
> >    }
> >  
> > +  //
> > +  // AmdSevDxe module is an apriori driver so it gets loaded between PEI
> > +  // and DXE phases and the SetVariable call will fail at the driver's
> > +  // entry point as the Variable DXE module is still not loaded yet.
> > +  // So we need to wait for an event notification which is signaled
> > +  // after the Variable DXE module is loaded, hence, using the
> > +  // EndOfDxe event notification to make this call.
> > +  //
> > +  // Register EFI_END_OF_DXE_EVENT_GROUP_GUID event.
> > +  // The notification function sets the runtime variable indicating OVMF
> > +  // support for SEV live migration.
> > +  //
> > +  Status = gBS->CreateEventEx (
> > +                  EVT_NOTIFY_SIGNAL,
> > +                  TPL_CALLBACK,
> > +                  AmdSevDxeOnEndOfDxe,
> > +                  NULL,
> > +                  &gEfiEndOfDxeEventGroupGuid,
> > +                  &Event
> > +                  );
> > +
> > +  if (EFI_ERROR (Status)) {
> > +      DEBUG ((DEBUG_INFO, "%a: CreateEventEx(): %r\n",
> 
> DEBUG_ERROR?
> 
> > +        __FUNCTION__, Status));
> 
> Should there be an "ASSERT_EFI_ERROR (Status)" after the DEBUG call?
> 

I don't think we should do an assert here and abort booting, failure
here will simply disable live migration support but i don't think that
it is such a fatal error that we should abort booting because of that.

OTOH, if there is a failure when doing page encryption status hypercalls
then aborting boot makes sense as guest page encryption status tracking
may be critical for multiple guest operations and not only live
migration.

Thanks,
Ashish

Re: [edk2-devel] [PATCH v6 1/6] OvmfPkg/BaseMemEncryptLib: Detect SEV live migration feature.

[Lendacky, Thomas]

On 8/10/21 1:05 AM, Gerd Hoffmann wrote:
>   Hi,
> 
>>> I still really don't understand the need for the CPUID loop. KVM only ever
>>> programs CPUID function 0x40000000, right?
> 
> Nope.  When you enable hyper-v emulation features you'll go find the kvm
> cpuid @ 0x40000000 and the hyper-v cpuid @ 0x40000100 (or the other way
> around, not sure).

Ah, thanks. I just saw the comment above get_out_of_range_cpuid_entry() in
arch/x86/kvm/cpuid.c where HyperV would get the 0x40000000-0x400000ff
range and KVM would then get the 0x40000100-0x400001ff range (basically
each hypervisor class gets their own 0x100 range).

Thanks,
Tom

> 
> take care,
>   Gerd
> 

Re: [PATCH v6 6/6] OvmfPkg/AmdSevDxe: Add support for SEV live migration.

[Lendacky, Thomas]

On 8/10/21 6:13 AM, Ashish Kalra wrote:
> Hello Tom,
> 
> On Mon, Aug 09, 2021 at 09:29:29AM -0500, Tom Lendacky wrote:
>> On 8/2/21 7:33 AM, Ashish Kalra wrote:
>>
>> Should there be an "ASSERT_EFI_ERROR (Status)" after the DEBUG call?
>>
> 
> I don't think we should do an assert here and abort booting, failure
> here will simply disable live migration support but i don't think that
> it is such a fatal error that we should abort booting because of that.
> 
> OTOH, if there is a failure when doing page encryption status hypercalls
> then aborting boot makes sense as guest page encryption status tracking
> may be critical for multiple guest operations and not only live
> migration.

Makes sense.

Thanks,
Tom

> 
> Thanks,
> Ashish
>