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

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

[Ashish Kalra]

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

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

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

BaseMemEncryptSevLib invokes hypercalls via this new hypercall library.

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

Changes since v2:
 - GHCB_BASE setup during reset-vector as decrypted is marked explicitly
   in the hypervisor page encryption bitmap after setting the 
   PcdSevEsIsEnabled PCD.

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

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

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

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

-- 
2.17.1

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

[Ashish Kalra]

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

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

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

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

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

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

[Dov Murik]

On 04/12/2020 2:03, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> Add SEV and SEV-ES hypercall abstraction library to support SEV Page
> encryption/deceryption status hypercalls for SEV and SEV-ES guests.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> 
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>   OvmfPkg/Include/Library/MemEncryptHypercallLib.h                  |  37 +++++++
>   OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.c   | 105 ++++++++++++++++++++
>   OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf |  39 ++++++++
>   OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm     |  39 ++++++++
>   OvmfPkg/OvmfPkgX64.dsc                                            |   1 +
>   5 files changed, 221 insertions(+)
> 
> diff --git a/OvmfPkg/Include/Library/MemEncryptHypercallLib.h b/OvmfPkg/Include/Library/MemEncryptHypercallLib.h
> new file mode 100644
> index 0000000000..cd46a7f2b3
> --- /dev/null
> +++ b/OvmfPkg/Include/Library/MemEncryptHypercallLib.h
> @@ -0,0 +1,37 @@
> +/** @file
> +
> +  Define Secure Encrypted Virtualization (SEV) hypercall library.
> +
> +  Copyright (c) 2020, AMD Incorporated. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef _MEM_ENCRYPT_HYPERCALL_LIB_H_
> +#define _MEM_ENCRYPT_HYPERCALL_LIB_H_
> +
> +#include <Base.h>
> +
> +#define SEV_PAGE_ENC_HYPERCALL  12
> +
> +/**
> + This hyercall is used to notify hypervisor when a page is marked as
> + 'decrypted' (i.e C-bit removed).
> +
> + @param[in]   PhysicalAddress       The physical address that is the start address
> +                                    of a memory region.
> + @param[in]   Length                The length of memory region
> + @param[in]   Mode                  SetCBit or ClearCBit
> +
> +**/
> +
> +VOID
> +EFIAPI
> +SetMemoryEncDecHypercall3 (
> +  IN  UINTN     PhysicalAddress,
> +  IN  UINTN     Length,
> +  IN  UINTN     Mode
> +  );

(1) I'm not sure why the "3" is needed at the end of the function name.

(2) The second argument is called 'Length' here and 'Pages' in the .c 
file. Which name is correct? If the semantics of the hypercall deals 
only with whole pages, maybe it'll be better to modify the address to 
GFN and length to NumberOfPages.  This way you don't have to do deal 
with non-page-aligned addresses or lengths.  Of course this may reflect 
on changes needed in the hypercall implementation itself in KVM.

(3) Mode: is this actually a boolean, or the enum which can be 
SetCBit(=0) or ClearCBit(=1)? Maybe a clearer argument would be "BOOL 
Encrypted" or "BOOL NewCBitValue"?  Of course this has to match 
semantics of this argument in KVM.


> +
> +#endif
> diff --git a/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.c b/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.c
> new file mode 100644
> index 0000000000..f1136b7d36
> --- /dev/null
> +++ b/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.c
> @@ -0,0 +1,105 @@
> +/** @file
> +
> +  Secure Encrypted Virtualization (SEV) hypercall helper library
> +
> +  Copyright (c) 2020, AMD Incorporated. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Uefi.h>
> +#include <Uefi/UefiBaseType.h>
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/VmgExitLib.h>
> +#include <Register/Amd/Ghcb.h>
> +#include <Register/Amd/Msr.h>
> +#include <Library/MemEncryptSevLib.h>
> +#include <Library/MemEncryptHypercallLib.h>
> +
> +//
> +// Interface exposed by the ASM implementation of the core hypercall
> +//
> +//
> +
> +VOID
> +EFIAPI
> +SetMemoryEncDecHypercall3AsmStub (
> +  IN  UINTN  HypercallNum,
> +  IN  UINTN  PhysicalAddress,
> +  IN  UINTN  Length,
> +  IN  UINTN  Mode
> +  );
> +
> +/**
> + This function returns the current CPU privilege level, implemented
> + in ASM helper stub.
> +
> +**/
> +
> +UINT8
> +EFIAPI
> +GetCurrentCpuPrivilegeLevel (
> +  VOID
> +  );
> +
> +STATIC
> +VOID
> +GhcbSetRegValid (
> +  IN OUT GHCB       *Ghcb,
> +  IN GHCB_REGISTER  Reg
> +  )
> +{
> +  UINT32  RegIndex;
> +  UINT32  RegBit;
> +
> +  RegIndex = Reg / 8;
> +  RegBit   = Reg & 0x07;
> +
> +  Ghcb->SaveArea.ValidBitmap[RegIndex] |= (1 << RegBit);
> +}
> +
> +VOID
> +EFIAPI
> +SetMemoryEncDecHypercall3 (
> +  IN PHYSICAL_ADDRESS PhysicalAddress,
> +  IN UINTN            Pages,
> +  IN UINTN            Mode
> +  )
> +{
> +  if (MemEncryptSevEsIsEnabled ()) {
> +    MSR_SEV_ES_GHCB_REGISTER  Msr;
> +    GHCB                      *Ghcb;
> +    BOOLEAN                   InterruptState;
> +    UINT64                    Status;
> +
> +    Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
> +    Ghcb = Msr.Ghcb;
> +
> +    VmgInit (Ghcb, &InterruptState);
> +
> +    Ghcb->SaveArea.Rax = SEV_PAGE_ENC_HYPERCALL;
> +    GhcbSetRegValid (Ghcb, GhcbRax);
> +    Ghcb->SaveArea.Rbx = PhysicalAddress;
> +    GhcbSetRegValid (Ghcb, GhcbRbx);
> +    Ghcb->SaveArea.Rcx = Pages;
> +    GhcbSetRegValid (Ghcb, GhcbRcx);
> +    Ghcb->SaveArea.Rdx = Mode;
> +    GhcbSetRegValid (Ghcb, GhcbRdx);
> +    Ghcb->SaveArea.Cpl = GetCurrentCpuPrivilegeLevel();
> +    GhcbSetRegValid (Ghcb, GhcbCpl);
> +
> +    Status = VmgExit (Ghcb, SVM_EXIT_VMMCALL, 0, 0);
> +    if (Status) {
> +      DEBUG ((DEBUG_ERROR, "SVM_EXIT_VMMCALL failed %lx\n", Status));
> +    }
> +    VmgDone (Ghcb, InterruptState);
> +  } else {
> +    SetMemoryEncDecHypercall3AsmStub (
> +      SEV_PAGE_ENC_HYPERCALL,
> +      PhysicalAddress,
> +      Pages,
> +      Mode);
> +  }
> +}
> diff --git a/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf b/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
> new file mode 100644
> index 0000000000..1936fe5b37
> --- /dev/null
> +++ b/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
> @@ -0,0 +1,39 @@
> +## @file
> +#  Library provides the hypervisor helper functions for SEV guest
> +#
> +# Copyright (c) 2020 Advanced Micro Devices. All rights reserved.<BR>
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 1.25
> +  BASE_NAME                      = MemEncryptHypercallLib
> +  FILE_GUID                      = 86f2501e-f128-45f3-91c4-3cff31656ca8
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = MemEncryptHypercallLib|SEC PEI_CORE PEIM DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_DRIVER
> +
> +#
> +# The following information is for reference only and not required by the build
> +# tools.
> +#
> +# VALID_ARCHITECTURES           = IA32 X64
> +#
> +
> +[Packages]
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  UefiCpuPkg/UefiCpuPkg.dec
> +  OvmfPkg/OvmfPkg.dec
> +
> +[Sources.X64]
> +  MemEncryptHypercallLib.c
> +  X64/AsmHelperStub.nasm
> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> +  VmgExitLib
> diff --git a/OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm b/OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm
> new file mode 100644
> index 0000000000..5d8a7aa85a
> --- /dev/null
> +++ b/OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm
> @@ -0,0 +1,39 @@
> +DEFAULT REL
> +SECTION .text
> +
> +; VOID
> +; EFIAPI
> +; SetMemoryEncDecHypercall3AsmStub (
> +;   IN UINT HypercallNum,
> +;   IN INTN Arg1,
> +;   IN INTN Arg2,
> +;   IN INTN Arg3
> +;   );

The name of the function hints that this has something to do with memory 
enc/dec, but actually this is a generic vmmcall-based hypercall.  In 
your corresponding linux patch 
<https://lore.kernel.org/kvm/b6bc54ed6c8ae4444f3acf1ed4386010783ad386.1606782580.git.ashish.kalra@amd.com/> 
you called it kvm_sev_hypercall3, so I assume something like that would 
be good here too.  Also, to support future hypercalls, allow it to 
return an INTN (value of rax), even though that is not used for the 
current hypercall in question.


> +global ASM_PFX(SetMemoryEncDecHypercall3AsmStub)
> +ASM_PFX(SetMemoryEncDecHypercall3AsmStub):
> +  ; UEFI calling conventions require RBX to
> +  ; be nonvolatile/callee-saved.
> +  push rbx
> +  ; Copy HypercallNumber to rax
> +  mov rax, rcx
> +  ; Copy Arg1 to the register expected by KVM
> +  mov rbx, rdx
> +  ; Copy Arg2 to register expected by KVM
> +  mov rcx, r8
> +  ; Copy Arg2 to register expected by KVM
> +  mov rdx, r9
> +  ; Call VMMCALL
> +  vmmcall
> +  pop rbx
> +  ret
> +
> +; UINT8
> +; EFIAPI
> +; GetCurrentCpuPrivilegeLevel (
> +;   VOID
> +;   );
> +global ASM_PFX(GetCurrentCpuPrivilegeLevel)
> +ASM_PFX(GetCurrentCpuPrivilegeLevel):
> +  mov  ax, cs
> +  and  al, 0x3
> +  ret

I think GetCurrentCpuPrivilegeLevel can be implemented in C as:

   return AsmReadCS() & 0x3;




> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index e59ae05b73..97c31c7586 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -174,6 +174,7 @@
>     VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
>     LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
>     MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
> +  MemEncryptHypercallLib|OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
>   !if $(SMM_REQUIRE) == FALSE
>     LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
>   !endif
> 


-Dov

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

[Ashish Kalra]

Hello Dov,

On Sun, Dec 06, 2020 at 02:43:45PM +0200, Dov Murik wrote:
> 
> 
> On 04/12/2020 2:03, Ashish Kalra wrote:
> > From: Ashish Kalra <ashish.kalra@amd.com>
> > 
> > Add SEV and SEV-ES hypercall abstraction library to support SEV Page
> > encryption/deceryption status hypercalls for SEV and SEV-ES guests.
> > 
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> > 
> > Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> > ---
> >   OvmfPkg/Include/Library/MemEncryptHypercallLib.h                  |  37 +++++++
> >   OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.c   | 105 ++++++++++++++++++++
> >   OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf |  39 ++++++++
> >   OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm     |  39 ++++++++
> >   OvmfPkg/OvmfPkgX64.dsc                                            |   1 +
> >   5 files changed, 221 insertions(+)
> > 
> > diff --git a/OvmfPkg/Include/Library/MemEncryptHypercallLib.h b/OvmfPkg/Include/Library/MemEncryptHypercallLib.h
> > new file mode 100644
> > index 0000000000..cd46a7f2b3
> > --- /dev/null
> > +++ b/OvmfPkg/Include/Library/MemEncryptHypercallLib.h
> > @@ -0,0 +1,37 @@
> > +/** @file
> > +
> > +  Define Secure Encrypted Virtualization (SEV) hypercall library.
> > +
> > +  Copyright (c) 2020, AMD Incorporated. All rights reserved.<BR>
> > +
> > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#ifndef _MEM_ENCRYPT_HYPERCALL_LIB_H_
> > +#define _MEM_ENCRYPT_HYPERCALL_LIB_H_
> > +
> > +#include <Base.h>
> > +
> > +#define SEV_PAGE_ENC_HYPERCALL  12
> > +
> > +/**
> > + This hyercall is used to notify hypervisor when a page is marked as
> > + 'decrypted' (i.e C-bit removed).
> > +
> > + @param[in]   PhysicalAddress       The physical address that is the start address
> > +                                    of a memory region.
> > + @param[in]   Length                The length of memory region
> > + @param[in]   Mode                  SetCBit or ClearCBit
> > +
> > +**/
> > +
> > +VOID
> > +EFIAPI
> > +SetMemoryEncDecHypercall3 (
> > +  IN  UINTN     PhysicalAddress,
> > +  IN  UINTN     Length,
> > +  IN  UINTN     Mode
> > +  );
> 
> (1) I'm not sure why the "3" is needed at the end of the function name.
> 

As you mentioned below, this is analogous to our kernel interface, i.e.,
kvm_sev_hypercall3() and the XenHypercallLib also uses functional
interfaces such as XenHypercall2(). Though i think this functional
interface can be at the lowest level, at a higher level something like
SetMemoryEncDecHypercall() should be better.

> (2) The second argument is called 'Length' here and 'Pages' in the .c file.
> Which name is correct? If the semantics of the hypercall deals only with
> whole pages, maybe it'll be better to modify the address to GFN and length
> to NumberOfPages.  This way you don't have to do deal with non-page-aligned
> addresses or lengths.  Of course this may reflect on changes needed in the
> hypercall implementation itself in KVM.
> 
> (3) Mode: is this actually a boolean, or the enum which can be SetCBit(=0)
> or ClearCBit(=1)? Maybe a clearer argument would be "BOOL Encrypted" or
> "BOOL NewCBitValue"?  Of course this has to match semantics of this argument
> in KVM.

I will fix this.
> 
> 
> > +
> > +#endif
> > diff --git a/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.c b/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.c
> > new file mode 100644
> > index 0000000000..f1136b7d36
> > --- /dev/null
> > +++ b/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.c
> > @@ -0,0 +1,105 @@
> > +/** @file
> > +
> > +  Secure Encrypted Virtualization (SEV) hypercall helper library
> > +
> > +  Copyright (c) 2020, AMD Incorporated. All rights reserved.<BR>
> > +
> > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#include <Uefi.h>
> > +#include <Uefi/UefiBaseType.h>
> > +#include <Library/BaseLib.h>
> > +#include <Library/DebugLib.h>
> > +#include <Library/VmgExitLib.h>
> > +#include <Register/Amd/Ghcb.h>
> > +#include <Register/Amd/Msr.h>
> > +#include <Library/MemEncryptSevLib.h>
> > +#include <Library/MemEncryptHypercallLib.h>
> > +
> > +//
> > +// Interface exposed by the ASM implementation of the core hypercall
> > +//
> > +//
> > +
> > +VOID
> > +EFIAPI
> > +SetMemoryEncDecHypercall3AsmStub (
> > +  IN  UINTN  HypercallNum,
> > +  IN  UINTN  PhysicalAddress,
> > +  IN  UINTN  Length,
> > +  IN  UINTN  Mode
> > +  );
> > +
> > +/**
> > + This function returns the current CPU privilege level, implemented
> > + in ASM helper stub.
> > +
> > +**/
> > +
> > +UINT8
> > +EFIAPI
> > +GetCurrentCpuPrivilegeLevel (
> > +  VOID
> > +  );
> > +
> > +STATIC
> > +VOID
> > +GhcbSetRegValid (
> > +  IN OUT GHCB       *Ghcb,
> > +  IN GHCB_REGISTER  Reg
> > +  )
> > +{
> > +  UINT32  RegIndex;
> > +  UINT32  RegBit;
> > +
> > +  RegIndex = Reg / 8;
> > +  RegBit   = Reg & 0x07;
> > +
> > +  Ghcb->SaveArea.ValidBitmap[RegIndex] |= (1 << RegBit);
> > +}
> > +
> > +VOID
> > +EFIAPI
> > +SetMemoryEncDecHypercall3 (
> > +  IN PHYSICAL_ADDRESS PhysicalAddress,
> > +  IN UINTN            Pages,
> > +  IN UINTN            Mode
> > +  )
> > +{
> > +  if (MemEncryptSevEsIsEnabled ()) {
> > +    MSR_SEV_ES_GHCB_REGISTER  Msr;
> > +    GHCB                      *Ghcb;
> > +    BOOLEAN                   InterruptState;
> > +    UINT64                    Status;
> > +
> > +    Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
> > +    Ghcb = Msr.Ghcb;
> > +
> > +    VmgInit (Ghcb, &InterruptState);
> > +
> > +    Ghcb->SaveArea.Rax = SEV_PAGE_ENC_HYPERCALL;
> > +    GhcbSetRegValid (Ghcb, GhcbRax);
> > +    Ghcb->SaveArea.Rbx = PhysicalAddress;
> > +    GhcbSetRegValid (Ghcb, GhcbRbx);
> > +    Ghcb->SaveArea.Rcx = Pages;
> > +    GhcbSetRegValid (Ghcb, GhcbRcx);
> > +    Ghcb->SaveArea.Rdx = Mode;
> > +    GhcbSetRegValid (Ghcb, GhcbRdx);
> > +    Ghcb->SaveArea.Cpl = GetCurrentCpuPrivilegeLevel();
> > +    GhcbSetRegValid (Ghcb, GhcbCpl);
> > +
> > +    Status = VmgExit (Ghcb, SVM_EXIT_VMMCALL, 0, 0);
> > +    if (Status) {
> > +      DEBUG ((DEBUG_ERROR, "SVM_EXIT_VMMCALL failed %lx\n", Status));
> > +    }
> > +    VmgDone (Ghcb, InterruptState);
> > +  } else {
> > +    SetMemoryEncDecHypercall3AsmStub (
> > +      SEV_PAGE_ENC_HYPERCALL,
> > +      PhysicalAddress,
> > +      Pages,
> > +      Mode);
> > +  }
> > +}
> > diff --git a/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf b/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
> > new file mode 100644
> > index 0000000000..1936fe5b37
> > --- /dev/null
> > +++ b/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
> > @@ -0,0 +1,39 @@
> > +## @file
> > +#  Library provides the hypervisor helper functions for SEV guest
> > +#
> > +# Copyright (c) 2020 Advanced Micro Devices. All rights reserved.<BR>
> > +#
> > +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> > +#
> > +#
> > +##
> > +
> > +[Defines]
> > +  INF_VERSION                    = 1.25
> > +  BASE_NAME                      = MemEncryptHypercallLib
> > +  FILE_GUID                      = 86f2501e-f128-45f3-91c4-3cff31656ca8
> > +  MODULE_TYPE                    = BASE
> > +  VERSION_STRING                 = 1.0
> > +  LIBRARY_CLASS                  = MemEncryptHypercallLib|SEC PEI_CORE PEIM DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_DRIVER
> > +
> > +#
> > +# The following information is for reference only and not required by the build
> > +# tools.
> > +#
> > +# VALID_ARCHITECTURES           = IA32 X64
> > +#
> > +
> > +[Packages]
> > +  MdeModulePkg/MdeModulePkg.dec
> > +  MdePkg/MdePkg.dec
> > +  UefiCpuPkg/UefiCpuPkg.dec
> > +  OvmfPkg/OvmfPkg.dec
> > +
> > +[Sources.X64]
> > +  MemEncryptHypercallLib.c
> > +  X64/AsmHelperStub.nasm
> > +
> > +[LibraryClasses]
> > +  BaseLib
> > +  DebugLib
> > +  VmgExitLib
> > diff --git a/OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm b/OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm
> > new file mode 100644
> > index 0000000000..5d8a7aa85a
> > --- /dev/null
> > +++ b/OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm
> > @@ -0,0 +1,39 @@
> > +DEFAULT REL
> > +SECTION .text
> > +
> > +; VOID
> > +; EFIAPI
> > +; SetMemoryEncDecHypercall3AsmStub (
> > +;   IN UINT HypercallNum,
> > +;   IN INTN Arg1,
> > +;   IN INTN Arg2,
> > +;   IN INTN Arg3
> > +;   );
> 
> The name of the function hints that this has something to do with memory
> enc/dec, but actually this is a generic vmmcall-based hypercall.  In your
> corresponding linux patch <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fkvm%2Fb6bc54ed6c8ae4444f3acf1ed4386010783ad386.1606782580.git.ashish.kalra%40amd.com%2F&amp;data=04%7C01%7CAshish.Kalra%40amd.com%7C4123cf72f32946b8e75a08d899e498be%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637428554380675577%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=xUydJsFm7hI04UBbucGpSVD2X%2FIZtwLAMbW%2Bm5RHAuo%3D&amp;reserved=0>
> you called it kvm_sev_hypercall3, so I assume something like that would be
> good here too.  Also, to support future hypercalls, allow it to return an
> INTN (value of rax), even though that is not used for the current hypercall
> in question.
> 
> 
Yes, i will add a return value to the hypercall.

> > +global ASM_PFX(SetMemoryEncDecHypercall3AsmStub)
> > +ASM_PFX(SetMemoryEncDecHypercall3AsmStub):
> > +  ; UEFI calling conventions require RBX to
> > +  ; be nonvolatile/callee-saved.
> > +  push rbx
> > +  ; Copy HypercallNumber to rax
> > +  mov rax, rcx
> > +  ; Copy Arg1 to the register expected by KVM
> > +  mov rbx, rdx
> > +  ; Copy Arg2 to register expected by KVM
> > +  mov rcx, r8
> > +  ; Copy Arg2 to register expected by KVM
> > +  mov rdx, r9
> > +  ; Call VMMCALL
> > +  vmmcall
> > +  pop rbx
> > +  ret
> > +
> > +; UINT8
> > +; EFIAPI
> > +; GetCurrentCpuPrivilegeLevel (
> > +;   VOID
> > +;   );
> > +global ASM_PFX(GetCurrentCpuPrivilegeLevel)
> > +ASM_PFX(GetCurrentCpuPrivilegeLevel):
> > +  mov  ax, cs
> > +  and  al, 0x3
> > +  ret
> 
> I think GetCurrentCpuPrivilegeLevel can be implemented in C as:
> 
>   return AsmReadCS() & 0x3;
> 

Ok. 
> 
> 
> > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> > index e59ae05b73..97c31c7586 100644
> > --- a/OvmfPkg/OvmfPkgX64.dsc
> > +++ b/OvmfPkg/OvmfPkgX64.dsc
> > @@ -174,6 +174,7 @@
> >     VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
> >     LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
> >     MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
> > +  MemEncryptHypercallLib|OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
> >   !if $(SMM_REQUIRE) == FALSE
> >     LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
> >   !endif
> > 
> 
> 
Thanks,
Ashish

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

[Ashish Kalra]

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

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

Invoke hypercall via the new hypercall library.

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

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

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

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

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

[Ashish Kalra]

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

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

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

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

diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index 4a515a4847..da9470db7f 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -15,6 +15,7 @@
 #include <Library/HobLib.h>
 #include <Library/MemEncryptSevLib.h>
 #include <Library/MemoryAllocationLib.h>
+#include <Library/MemEncryptHypercallLib.h>
 #include <Library/PcdLib.h>
 #include <PiPei.h>
 #include <Register/Amd/Cpuid.h>
@@ -49,6 +50,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.17.1

Re: [edk2-devel] [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF.

[Laszlo Ersek]

On 12/04/20 01:03, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> By default all the SEV guest memory regions are considered encrypted,
> if a guest changes the encryption attribute of the page (e.g mark a
> page as decrypted) then notify hypervisor. Hypervisor will need to
> track the unencrypted pages. The information will be used during
> guest live migration, guest page migration and guest debugging.
> 
> The patch-set also adds a new SEV and SEV-ES hypercall abstraction
> library to support SEV Page encryption/decryption status hypercalls
> for SEV and SEV-ES guests.
> 
> BaseMemEncryptSevLib invokes hypercalls via this new hypercall library.
> 
> A branch containing these patches is available here:
> https://github.com/ashkalra/edk2/tree/sev_page_encryption_bitmap_v3
> 
> Changes since v2:
>  - GHCB_BASE setup during reset-vector as decrypted is marked explicitly
>    in the hypervisor page encryption bitmap after setting the 
>    PcdSevEsIsEnabled PCD.
> 
> Changes since v1:
>  - Mark GHCB_BASE setup during reset-vector as decrypted explicitly in
>    the hypervisor page encryption bitmap.
>  - Resending the series with correct shallow threading.
> 
> Ashish Kalra (2):
>   OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls.
>   OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap.
> 
> Brijesh Singh (1):
>   OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall
> 
>  .../Include/Library/MemEncryptHypercallLib.h  |  37 ++++++
>  .../BaseMemEncryptSevLib.inf                  |   1 +
>  .../BaseMemEncryptSevLib/X64/VirtualMemory.c  |  18 +++
>  .../MemEncryptHypercallLib.c                  | 105 ++++++++++++++++++
>  .../MemEncryptHypercallLib.inf                |  39 +++++++
>  .../X64/AsmHelperStub.nasm                    |  39 +++++++
>  OvmfPkg/OvmfPkgX64.dsc                        |   1 +
>  OvmfPkg/PlatformPei/AmdSev.c                  |  10 ++
>  8 files changed, 250 insertions(+)
>  create mode 100644 OvmfPkg/Include/Library/MemEncryptHypercallLib.h
>  create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.c
>  create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
>  create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm
> 

I'll need some time to get to this series.

I'm fairly certain though, from a quick skim, that this series breaks
all DSC files under OvmfPkg except X64. Please fix that.

Thanks
Laszlo

Re: [edk2-devel] [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF.

[Ashish Kalra]

On Fri, Dec 04, 2020 at 04:50:05AM +0100, Laszlo Ersek wrote:
> On 12/04/20 01:03, Ashish Kalra wrote:
> > From: Ashish Kalra <ashish.kalra@amd.com>
> > 
> > By default all the SEV guest memory regions are considered encrypted,
> > if a guest changes the encryption attribute of the page (e.g mark a
> > page as decrypted) then notify hypervisor. Hypervisor will need to
> > track the unencrypted pages. The information will be used during
> > guest live migration, guest page migration and guest debugging.
> > 
> > The patch-set also adds a new SEV and SEV-ES hypercall abstraction
> > library to support SEV Page encryption/decryption status hypercalls
> > for SEV and SEV-ES guests.
> > 
> > BaseMemEncryptSevLib invokes hypercalls via this new hypercall library.
> > 
> > A branch containing these patches is available here:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fashkalra%2Fedk2%2Ftree%2Fsev_page_encryption_bitmap_v3&amp;data=04%7C01%7Cashish.kalra%40amd.com%7Cbc3c88f21f1d40b322b408d89807b5c8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637426506192800828%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=VZzP2MVJSECgMhOyuCCASw58g74BiCVAH9JW8hZG3Tw%3D&amp;reserved=0
> > 
> > Changes since v2:
> >  - GHCB_BASE setup during reset-vector as decrypted is marked explicitly
> >    in the hypervisor page encryption bitmap after setting the 
> >    PcdSevEsIsEnabled PCD.
> > 
> > Changes since v1:
> >  - Mark GHCB_BASE setup during reset-vector as decrypted explicitly in
> >    the hypervisor page encryption bitmap.
> >  - Resending the series with correct shallow threading.
> > 
> > Ashish Kalra (2):
> >   OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls.
> >   OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap.
> > 
> > Brijesh Singh (1):
> >   OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall
> > 
> >  .../Include/Library/MemEncryptHypercallLib.h  |  37 ++++++
> >  .../BaseMemEncryptSevLib.inf                  |   1 +
> >  .../BaseMemEncryptSevLib/X64/VirtualMemory.c  |  18 +++
> >  .../MemEncryptHypercallLib.c                  | 105 ++++++++++++++++++
> >  .../MemEncryptHypercallLib.inf                |  39 +++++++
> >  .../X64/AsmHelperStub.nasm                    |  39 +++++++
> >  OvmfPkg/OvmfPkgX64.dsc                        |   1 +
> >  OvmfPkg/PlatformPei/AmdSev.c                  |  10 ++
> >  8 files changed, 250 insertions(+)
> >  create mode 100644 OvmfPkg/Include/Library/MemEncryptHypercallLib.h
> >  create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.c
> >  create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
> >  create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm
> > 
> 
> I'll need some time to get to this series.
> 
> I'm fairly certain though, from a quick skim, that this series breaks
> all DSC files under OvmfPkg except X64. Please fix that.
> 
> 

Ok thanks Laszlo, i will fix this.

Ashish

Re: [edk2-devel] [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF.

[Laszlo Ersek]

On 12/04/20 09:10, Ashish Kalra wrote:
> On Fri, Dec 04, 2020 at 04:50:05AM +0100, Laszlo Ersek wrote:
>> On 12/04/20 01:03, Ashish Kalra wrote:
>>> From: Ashish Kalra <ashish.kalra@amd.com>
>>>
>>> By default all the SEV guest memory regions are considered encrypted,
>>> if a guest changes the encryption attribute of the page (e.g mark a
>>> page as decrypted) then notify hypervisor. Hypervisor will need to
>>> track the unencrypted pages. The information will be used during
>>> guest live migration, guest page migration and guest debugging.
>>>
>>> The patch-set also adds a new SEV and SEV-ES hypercall abstraction
>>> library to support SEV Page encryption/decryption status hypercalls
>>> for SEV and SEV-ES guests.
>>>
>>> BaseMemEncryptSevLib invokes hypercalls via this new hypercall library.
>>>
>>> A branch containing these patches is available here:
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fashkalra%2Fedk2%2Ftree%2Fsev_page_encryption_bitmap_v3&amp;data=04%7C01%7Cashish.kalra%40amd.com%7Cbc3c88f21f1d40b322b408d89807b5c8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637426506192800828%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=VZzP2MVJSECgMhOyuCCASw58g74BiCVAH9JW8hZG3Tw%3D&amp;reserved=0
>>>
>>> Changes since v2:
>>>  - GHCB_BASE setup during reset-vector as decrypted is marked explicitly
>>>    in the hypervisor page encryption bitmap after setting the 
>>>    PcdSevEsIsEnabled PCD.
>>>
>>> Changes since v1:
>>>  - Mark GHCB_BASE setup during reset-vector as decrypted explicitly in
>>>    the hypervisor page encryption bitmap.
>>>  - Resending the series with correct shallow threading.
>>>
>>> Ashish Kalra (2):
>>>   OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls.
>>>   OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap.
>>>
>>> Brijesh Singh (1):
>>>   OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall
>>>
>>>  .../Include/Library/MemEncryptHypercallLib.h  |  37 ++++++
>>>  .../BaseMemEncryptSevLib.inf                  |   1 +
>>>  .../BaseMemEncryptSevLib/X64/VirtualMemory.c  |  18 +++
>>>  .../MemEncryptHypercallLib.c                  | 105 ++++++++++++++++++
>>>  .../MemEncryptHypercallLib.inf                |  39 +++++++
>>>  .../X64/AsmHelperStub.nasm                    |  39 +++++++
>>>  OvmfPkg/OvmfPkgX64.dsc                        |   1 +
>>>  OvmfPkg/PlatformPei/AmdSev.c                  |  10 ++
>>>  8 files changed, 250 insertions(+)
>>>  create mode 100644 OvmfPkg/Include/Library/MemEncryptHypercallLib.h
>>>  create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.c
>>>  create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
>>>  create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm
>>>
>>
>> I'll need some time to get to this series.
>>
>> I'm fairly certain though, from a quick skim, that this series breaks
>> all DSC files under OvmfPkg except X64. Please fix that.
>>
>>
> 
> Ok thanks Laszlo, i will fix this.

Thanks.

I can see a new comment for the series from Dov Murik, and I think
that's awesome. I'd welcome if there were lively exchanges around OVMF
patch sets. I'm selfish of course: I'd like to delegate reviews.

So, on this patch set, I notice it does not add the new
(MemEncryptHypercallLib-related) files to Maintainers.txt, namely
section "OvmfPkg: SEV-related modules".

Please include such a patch in v4 -- if Tom and Brijesh agree, I'd like
to put the new lib explicitly under their reviewership.

Also, I plan to review this series (v4, at this point) only for
formalities. I'd like to receive an R-b from Tom or Brijesh [*], and
another from Dov or a colleague at IBM, for this series; those together
should suffice for merging the library.

[*] Brijesh seems to be the original author of patch#2, so maybe Tom is
a better-poised reviewer for this.

Thanks
Laszlo

Re: [edk2-devel] [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF.

[Brijesh Singh]

On 12/7/20 8:44 PM, Laszlo Ersek wrote:
> On 12/04/20 09:10, Ashish Kalra wrote:
>> On Fri, Dec 04, 2020 at 04:50:05AM +0100, Laszlo Ersek wrote:
>>> On 12/04/20 01:03, Ashish Kalra wrote:
>>>> From: Ashish Kalra <ashish.kalra@amd.com>
>>>>
>>>> By default all the SEV guest memory regions are considered encrypted,
>>>> if a guest changes the encryption attribute of the page (e.g mark a
>>>> page as decrypted) then notify hypervisor. Hypervisor will need to
>>>> track the unencrypted pages. The information will be used during
>>>> guest live migration, guest page migration and guest debugging.
>>>>
>>>> The patch-set also adds a new SEV and SEV-ES hypercall abstraction
>>>> library to support SEV Page encryption/decryption status hypercalls
>>>> for SEV and SEV-ES guests.
>>>>
>>>> BaseMemEncryptSevLib invokes hypercalls via this new hypercall library.
>>>>
>>>> A branch containing these patches is available here:
>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fashkalra%2Fedk2%2Ftree%2Fsev_page_encryption_bitmap_v3&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7C13b084db30e246f25b3f08d89b233f99%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637429922982198583%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=wuYFXFyBtwZWSWOCb3OYK8I7MDFAxId%2BC63fsa0XcjQ%3D&amp;reserved=0
>>>>
>>>> Changes since v2:
>>>>  - GHCB_BASE setup during reset-vector as decrypted is marked explicitly
>>>>    in the hypervisor page encryption bitmap after setting the 
>>>>    PcdSevEsIsEnabled PCD.
>>>>
>>>> Changes since v1:
>>>>  - Mark GHCB_BASE setup during reset-vector as decrypted explicitly in
>>>>    the hypervisor page encryption bitmap.
>>>>  - Resending the series with correct shallow threading.
>>>>
>>>> Ashish Kalra (2):
>>>>   OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls.
>>>>   OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap.
>>>>
>>>> Brijesh Singh (1):
>>>>   OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall
>>>>
>>>>  .../Include/Library/MemEncryptHypercallLib.h  |  37 ++++++
>>>>  .../BaseMemEncryptSevLib.inf                  |   1 +
>>>>  .../BaseMemEncryptSevLib/X64/VirtualMemory.c  |  18 +++
>>>>  .../MemEncryptHypercallLib.c                  | 105 ++++++++++++++++++
>>>>  .../MemEncryptHypercallLib.inf                |  39 +++++++
>>>>  .../X64/AsmHelperStub.nasm                    |  39 +++++++
>>>>  OvmfPkg/OvmfPkgX64.dsc                        |   1 +
>>>>  OvmfPkg/PlatformPei/AmdSev.c                  |  10 ++
>>>>  8 files changed, 250 insertions(+)
>>>>  create mode 100644 OvmfPkg/Include/Library/MemEncryptHypercallLib.h
>>>>  create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.c
>>>>  create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
>>>>  create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm
>>>>
>>> I'll need some time to get to this series.
>>>
>>> I'm fairly certain though, from a quick skim, that this series breaks
>>> all DSC files under OvmfPkg except X64. Please fix that.
>>>
>>>
>> Ok thanks Laszlo, i will fix this.
> Thanks.
>
> I can see a new comment for the series from Dov Murik, and I think
> that's awesome. I'd welcome if there were lively exchanges around OVMF
> patch sets. I'm selfish of course: I'd like to delegate reviews.
>
> So, on this patch set, I notice it does not add the new
> (MemEncryptHypercallLib-related) files to Maintainers.txt, namely
> section "OvmfPkg: SEV-related modules".
>
> Please include such a patch in v4 -- if Tom and Brijesh agree, I'd like
> to put the new lib explicitly under their reviewership.


I am okay with the ownership.


> Also, I plan to review this series (v4, at this point) only for
> formalities. I'd like to receive an R-b from Tom or Brijesh [*], and
> another from Dov or a colleague at IBM, for this series; those together
> should suffice for merging the library.


Since this patch has dependency on HV feature, so I was going to review
this patch after I see some confirmation coming from KVM upstream on the
hypervcall approach. It appears that Sean may have some other ideas, so
lets wait to hear those before we consider this patch.


>
> [*] Brijesh seems to be the original author of patch#2, so maybe Tom is
> a better-poised reviewer for this.
>
> Thanks
> Laszlo
>

Re: [edk2-devel] [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF.

[Lendacky, Thomas]

On 12/7/20 8:44 PM, Laszlo Ersek wrote:
> On 12/04/20 09:10, Ashish Kalra wrote:
>> On Fri, Dec 04, 2020 at 04:50:05AM +0100, Laszlo Ersek wrote:
>>> On 12/04/20 01:03, Ashish Kalra wrote:
>>>> From: Ashish Kalra <ashish.kalra@amd.com>
>>>>
>>>> By default all the SEV guest memory regions are considered encrypted,
>>>> if a guest changes the encryption attribute of the page (e.g mark a
>>>> page as decrypted) then notify hypervisor. Hypervisor will need to
>>>> track the unencrypted pages. The information will be used during
>>>> guest live migration, guest page migration and guest debugging.
>>>>
>>>> The patch-set also adds a new SEV and SEV-ES hypercall abstraction
>>>> library to support SEV Page encryption/decryption status hypercalls
>>>> for SEV and SEV-ES guests.
>>>>
>>>> BaseMemEncryptSevLib invokes hypercalls via this new hypercall library.
>>>>
>>>> A branch containing these patches is available here:
>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fashkalra%2Fedk2%2Ftree%2Fsev_page_encryption_bitmap_v3&amp;data=04%7C01%7Cthomas.lendacky%40amd.com%7Caa286d7e06864008110008d89b233ebc%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637429922982193672%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=EjrGD2LNlji8ualk8KClh%2BhqJa5Fm0UzlmPc4%2FQvb2g%3D&amp;reserved=0
>>>>
>>>> Changes since v2:
>>>>  - GHCB_BASE setup during reset-vector as decrypted is marked explicitly
>>>>    in the hypervisor page encryption bitmap after setting the 
>>>>    PcdSevEsIsEnabled PCD.
>>>>
>>>> Changes since v1:
>>>>  - Mark GHCB_BASE setup during reset-vector as decrypted explicitly in
>>>>    the hypervisor page encryption bitmap.
>>>>  - Resending the series with correct shallow threading.
>>>>
>>>> Ashish Kalra (2):
>>>>   OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls.
>>>>   OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap.
>>>>
>>>> Brijesh Singh (1):
>>>>   OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall
>>>>
>>>>  .../Include/Library/MemEncryptHypercallLib.h  |  37 ++++++
>>>>  .../BaseMemEncryptSevLib.inf                  |   1 +
>>>>  .../BaseMemEncryptSevLib/X64/VirtualMemory.c  |  18 +++
>>>>  .../MemEncryptHypercallLib.c                  | 105 ++++++++++++++++++
>>>>  .../MemEncryptHypercallLib.inf                |  39 +++++++
>>>>  .../X64/AsmHelperStub.nasm                    |  39 +++++++
>>>>  OvmfPkg/OvmfPkgX64.dsc                        |   1 +
>>>>  OvmfPkg/PlatformPei/AmdSev.c                  |  10 ++
>>>>  8 files changed, 250 insertions(+)
>>>>  create mode 100644 OvmfPkg/Include/Library/MemEncryptHypercallLib.h
>>>>  create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.c
>>>>  create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
>>>>  create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm
>>>>
>>>
>>> I'll need some time to get to this series.
>>>
>>> I'm fairly certain though, from a quick skim, that this series breaks
>>> all DSC files under OvmfPkg except X64. Please fix that.
>>>
>>>
>>
>> Ok thanks Laszlo, i will fix this.
> 
> Thanks.
> 
> I can see a new comment for the series from Dov Murik, and I think
> that's awesome. I'd welcome if there were lively exchanges around OVMF
> patch sets. I'm selfish of course: I'd like to delegate reviews.
> 
> So, on this patch set, I notice it does not add the new
> (MemEncryptHypercallLib-related) files to Maintainers.txt, namely
> section "OvmfPkg: SEV-related modules".
> 
> Please include such a patch in v4 -- if Tom and Brijesh agree, I'd like
> to put the new lib explicitly under their reviewership.

Yes, no issues with that.

> 
> Also, I plan to review this series (v4, at this point) only for
> formalities. I'd like to receive an R-b from Tom or Brijesh [*], and
> another from Dov or a colleague at IBM, for this series; those together
> should suffice for merging the library.
> 
> [*] Brijesh seems to be the original author of patch#2, so maybe Tom is
> a better-poised reviewer for this.

Will do. I know a new version is coming as well as discussion about the
hypercall in general, so lets see where that goes.

Thanks,
Tom

> 
> Thanks
> Laszlo
> 

Re: [edk2-devel] [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF.

[Laszlo Ersek]

On 12/08/20 15:57, Lendacky, Thomas wrote:
> On 12/7/20 8:44 PM, Laszlo Ersek wrote:
>> On 12/04/20 09:10, Ashish Kalra wrote:
>>> On Fri, Dec 04, 2020 at 04:50:05AM +0100, Laszlo Ersek wrote:
>>>> On 12/04/20 01:03, Ashish Kalra wrote:
>>>>> From: Ashish Kalra <ashish.kalra@amd.com>
>>>>>
>>>>> By default all the SEV guest memory regions are considered encrypted,
>>>>> if a guest changes the encryption attribute of the page (e.g mark a
>>>>> page as decrypted) then notify hypervisor. Hypervisor will need to
>>>>> track the unencrypted pages. The information will be used during
>>>>> guest live migration, guest page migration and guest debugging.
>>>>>
>>>>> The patch-set also adds a new SEV and SEV-ES hypercall abstraction
>>>>> library to support SEV Page encryption/decryption status hypercalls
>>>>> for SEV and SEV-ES guests.
>>>>>
>>>>> BaseMemEncryptSevLib invokes hypercalls via this new hypercall library.
>>>>>
>>>>> A branch containing these patches is available here:
>>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fashkalra%2Fedk2%2Ftree%2Fsev_page_encryption_bitmap_v3&amp;data=04%7C01%7Cthomas.lendacky%40amd.com%7Caa286d7e06864008110008d89b233ebc%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637429922982193672%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=EjrGD2LNlji8ualk8KClh%2BhqJa5Fm0UzlmPc4%2FQvb2g%3D&amp;reserved=0
>>>>>
>>>>> Changes since v2:
>>>>>  - GHCB_BASE setup during reset-vector as decrypted is marked explicitly
>>>>>    in the hypervisor page encryption bitmap after setting the 
>>>>>    PcdSevEsIsEnabled PCD.
>>>>>
>>>>> Changes since v1:
>>>>>  - Mark GHCB_BASE setup during reset-vector as decrypted explicitly in
>>>>>    the hypervisor page encryption bitmap.
>>>>>  - Resending the series with correct shallow threading.
>>>>>
>>>>> Ashish Kalra (2):
>>>>>   OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls.
>>>>>   OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap.
>>>>>
>>>>> Brijesh Singh (1):
>>>>>   OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall
>>>>>
>>>>>  .../Include/Library/MemEncryptHypercallLib.h  |  37 ++++++
>>>>>  .../BaseMemEncryptSevLib.inf                  |   1 +
>>>>>  .../BaseMemEncryptSevLib/X64/VirtualMemory.c  |  18 +++
>>>>>  .../MemEncryptHypercallLib.c                  | 105 ++++++++++++++++++
>>>>>  .../MemEncryptHypercallLib.inf                |  39 +++++++
>>>>>  .../X64/AsmHelperStub.nasm                    |  39 +++++++
>>>>>  OvmfPkg/OvmfPkgX64.dsc                        |   1 +
>>>>>  OvmfPkg/PlatformPei/AmdSev.c                  |  10 ++
>>>>>  8 files changed, 250 insertions(+)
>>>>>  create mode 100644 OvmfPkg/Include/Library/MemEncryptHypercallLib.h
>>>>>  create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.c
>>>>>  create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
>>>>>  create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm
>>>>>
>>>>
>>>> I'll need some time to get to this series.
>>>>
>>>> I'm fairly certain though, from a quick skim, that this series breaks
>>>> all DSC files under OvmfPkg except X64. Please fix that.
>>>>
>>>>
>>>
>>> Ok thanks Laszlo, i will fix this.
>>
>> Thanks.
>>
>> I can see a new comment for the series from Dov Murik, and I think
>> that's awesome. I'd welcome if there were lively exchanges around OVMF
>> patch sets. I'm selfish of course: I'd like to delegate reviews.
>>
>> So, on this patch set, I notice it does not add the new
>> (MemEncryptHypercallLib-related) files to Maintainers.txt, namely
>> section "OvmfPkg: SEV-related modules".
>>
>> Please include such a patch in v4 -- if Tom and Brijesh agree, I'd like
>> to put the new lib explicitly under their reviewership.
> 
> Yes, no issues with that.

Thank you guys!
Laszlo

> 
>>
>> Also, I plan to review this series (v4, at this point) only for
>> formalities. I'd like to receive an R-b from Tom or Brijesh [*], and
>> another from Dov or a colleague at IBM, for this series; those together
>> should suffice for merging the library.
>>
>> [*] Brijesh seems to be the original author of patch#2, so maybe Tom is
>> a better-poised reviewer for this.
> 
> Will do. I know a new version is coming as well as discussion about the
> hypercall in general, so lets see where that goes.
> 
> Thanks,
> Tom
> 
>>
>> Thanks
>> Laszlo
>>
> 
> 
> 
> 
>