[PATCH v4 1/4] 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>
---
Maintainers.txt | 2 +
OvmfPkg/Include/Library/MemEncryptHypercallLib.h | 43 ++++++++
OvmfPkg/Library/MemEncryptHypercallLib/Ia32/MemEncryptHypercallLib.c | 37 +++++++
OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf | 42 ++++++++
OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm | 28 ++++++
OvmfPkg/Library/MemEncryptHypercallLib/X64/MemEncryptHypercallLib.c | 105 ++++++++++++++++++++
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
OvmfPkg/OvmfXen.dsc | 1 +
10 files changed, 261 insertions(+)

diff --git a/Maintainers.txt b/Maintainers.txt
index ea54e0b7e9..8ecc8464ba 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -449,8 +449,10 @@ F: OvmfPkg/AmdSev/
F: OvmfPkg/AmdSevDxe/
F: OvmfPkg/Include/Guid/ConfidentialComputingSecret.h
F: OvmfPkg/Include/Library/MemEncryptSevLib.h
+F: OvmfPkg/Include/Library/MemEncryptHypercallLib.h
F: OvmfPkg/IoMmuDxe/AmdSevIoMmu.*
F: OvmfPkg/Library/BaseMemEncryptSevLib/
+F: OvmfPkg/Library/MemEncryptHypercallLib/
F: OvmfPkg/Library/PlatformBootManagerLibGrub/
F: OvmfPkg/Library/VmgExitLib/
F: OvmfPkg/PlatformPei/AmdSev.c
diff --git a/OvmfPkg/Include/Library/MemEncryptHypercallLib.h b/OvmfPkg/Include/Library/MemEncryptHypercallLib.h
new file mode 100644
index 0000000000..b241a189b6
--- /dev/null
+++ b/OvmfPkg/Include/Library/MemEncryptHypercallLib.h
@@ -0,0 +1,43 @@
+/** @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 KVM_HC_MAP_GPA_RANGE 12
+#define KVM_MAP_GPA_RANGE_PAGE_SZ_4K 0
+#define KVM_MAP_GPA_RANGE_PAGE_SZ_2M (1 << 0)
+#define KVM_MAP_GPA_RANGE_PAGE_SZ_1G (1 << 1)
+#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)
+
+/**
+ 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/Ia32/MemEncryptHypercallLib.c b/OvmfPkg/Library/MemEncryptHypercallLib/Ia32/MemEncryptHypercallLib.c
new file mode 100644
index 0000000000..2e73d47ee6
--- /dev/null
+++ b/OvmfPkg/Library/MemEncryptHypercallLib/Ia32/MemEncryptHypercallLib.c
@@ -0,0 +1,37 @@
+/** @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>
+
+/**
+ 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 PHYSICAL_ADDRESS PhysicalAddress,
+ IN UINTN Pages,
+ IN UINTN Mode
+ )
+{
+ //
+ // Memory encryption bit is not accessible in 32-bit mode
+ //
+}
diff --git a/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf b/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
new file mode 100644
index 0000000000..a77d58a7e6
--- /dev/null
+++ b/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
@@ -0,0 +1,42 @@
+## @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
+
+#
+# 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]
+ X64/MemEncryptHypercallLib.c
+ X64/AsmHelperStub.nasm
+
+[Sources.IA32]
+ Ia32/MemEncryptHypercallLib.c
+
+[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..f29b96f9b0
--- /dev/null
+++ b/OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm
@@ -0,0 +1,28 @@
+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
diff --git a/OvmfPkg/Library/MemEncryptHypercallLib/X64/MemEncryptHypercallLib.c b/OvmfPkg/Library/MemEncryptHypercallLib/X64/MemEncryptHypercallLib.c
new file mode 100644
index 0000000000..1c09ea012b
--- /dev/null
+++ b/OvmfPkg/Library/MemEncryptHypercallLib/X64/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
+ );
+
+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);
+}
+
+/**
+ 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 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 = KVM_HC_MAP_GPA_RANGE;
+ 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 = AsmReadCs() & 0x3;
+ 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 (
+ KVM_HC_MAP_GPA_RANGE,
+ PhysicalAddress,
+ Pages,
+ Mode
+ );
+ }
+}
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index f53efeae79..36f1d82ce7 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -176,6 +176,7 @@
VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+ MemEncryptHypercallLib|OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
!if $(SMM_REQUIRE) == FALSE
LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
!endif
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index b3662e17f2..2a743688b4 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -180,6 +180,7 @@
VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+ MemEncryptHypercallLib|OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
!if $(SMM_REQUIRE) == FALSE
LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
!endif
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 0a237a9058..eb9da51a15 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -180,6 +180,7 @@
VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+ MemEncryptHypercallLib|OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
!if $(SMM_REQUIRE) == FALSE
LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
!endif
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 3c1ca6bfd4..de0c052832 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -167,6 +167,7 @@
QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+ MemEncryptHypercallLib|OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
--
2.17.1


Brijesh Singh
 

On 6/21/2021 8:56 AM, 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>
Remove this newline.

Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
Maintainers.txt | 2 +
OvmfPkg/Include/Library/MemEncryptHypercallLib.h | 43 ++++++++
OvmfPkg/Library/MemEncryptHypercallLib/Ia32/MemEncryptHypercallLib.c | 37 +++++++
OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf | 42 ++++++++
OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm | 28 ++++++
OvmfPkg/Library/MemEncryptHypercallLib/X64/MemEncryptHypercallLib.c | 105 ++++++++++++++++++++
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
OvmfPkg/OvmfXen.dsc | 1 +
10 files changed, 261 insertions(+)

diff --git a/Maintainers.txt b/Maintainers.txt
index ea54e0b7e9..8ecc8464ba 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -449,8 +449,10 @@ F: OvmfPkg/AmdSev/
F: OvmfPkg/AmdSevDxe/
F: OvmfPkg/Include/Guid/ConfidentialComputingSecret.h
F: OvmfPkg/Include/Library/MemEncryptSevLib.h
+F: OvmfPkg/Include/Library/MemEncryptHypercallLib.h
F: OvmfPkg/IoMmuDxe/AmdSevIoMmu.*
F: OvmfPkg/Library/BaseMemEncryptSevLib/
+F: OvmfPkg/Library/MemEncryptHypercallLib/
F: OvmfPkg/Library/PlatformBootManagerLibGrub/
F: OvmfPkg/Library/VmgExitLib/
F: OvmfPkg/PlatformPei/AmdSev.c
diff --git a/OvmfPkg/Include/Library/MemEncryptHypercallLib.h b/OvmfPkg/Include/Library/MemEncryptHypercallLib.h
new file mode 100644
index 0000000000..b241a189b6
--- /dev/null
+++ b/OvmfPkg/Include/Library/MemEncryptHypercallLib.h
@@ -0,0 +1,43 @@
+/** @file
+
+ Define Secure Encrypted Virtualization (SEV) hypercall library.
+
+ Copyright (c) 2020, AMD Incorporated. All rights reserved.<BR>
^^^^
2021

+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _MEM_ENCRYPT_HYPERCALL_LIB_H_
+#define _MEM_ENCRYPT_HYPERCALL_LIB_H_
+
+#include <Base.h>
+
+#define KVM_HC_MAP_GPA_RANGE 12
+#define KVM_MAP_GPA_RANGE_PAGE_SZ_4K 0
+#define KVM_MAP_GPA_RANGE_PAGE_SZ_2M (1 << 0)
+#define KVM_MAP_GPA_RANGE_PAGE_SZ_1G (1 << 1)
+#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)
+
+/**
+ This hyercall is used to notify hypervisor when a page is marked as
+ 'decrypted' (i.e C-bit removed).
+
Looking at the function signature it seems this routine is used for both set
and clear. Please update the comment accordingly.


+ @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/Ia32/MemEncryptHypercallLib.c b/OvmfPkg/Library/MemEncryptHypercallLib/Ia32/MemEncryptHypercallLib.c
new file mode 100644
index 0000000000..2e73d47ee6
--- /dev/null
+++ b/OvmfPkg/Library/MemEncryptHypercallLib/Ia32/MemEncryptHypercallLib.c
@@ -0,0 +1,37 @@
+/** @file
+
+ Secure Encrypted Virtualization (SEV) hypercall helper library
+
+ Copyright (c) 2020, AMD Incorporated. All rights reserved.<BR>
^^^^
2021

+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Uefi.h>
+#include <Uefi/UefiBaseType.h>
+#include <Library/BaseLib.h>
+
+/**
+ 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 PHYSICAL_ADDRESS PhysicalAddress,
+ IN UINTN Pages,
+ IN UINTN Mode
+ )
+{
+ //
+ // Memory encryption bit is not accessible in 32-bit mode
+ //
+}
diff --git a/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf b/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
new file mode 100644
index 0000000000..a77d58a7e6
--- /dev/null
+++ b/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
@@ -0,0 +1,42 @@
+## @file
+# Library provides the hypervisor helper functions for SEV guest
+#
+# Copyright (c) 2020 Advanced Micro Devices. All rights reserved.<BR>
^^^^
2021
+#
+# 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
+
+#
+# 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]
+ X64/MemEncryptHypercallLib.c
+ X64/AsmHelperStub.nasm
+
+[Sources.IA32]
+ Ia32/MemEncryptHypercallLib.c
+
+[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..f29b96f9b0
--- /dev/null
+++ b/OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm
@@ -0,0 +1,28 @@
+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
diff --git a/OvmfPkg/Library/MemEncryptHypercallLib/X64/MemEncryptHypercallLib.c b/OvmfPkg/Library/MemEncryptHypercallLib/X64/MemEncryptHypercallLib.c
new file mode 100644
index 0000000000..1c09ea012b
--- /dev/null
+++ b/OvmfPkg/Library/MemEncryptHypercallLib/X64/MemEncryptHypercallLib.c
@@ -0,0 +1,105 @@
+/** @file
+
+ Secure Encrypted Virtualization (SEV) hypercall helper library
+
+ Copyright (c) 2020, AMD Incorporated. All rights reserved.<BR>
^^^^
2021
+
+ 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
+ );
+
The function signature does not match with documented signature. Fix the
SetMemoryEncDecHypercall3AsmStub() documented in AsmHelperStub.nasm to use
UINTN.


+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);
+}
+
This looks similar to VmgSetOffsetValid().

+/**
+ This hyercall is used to notify hypervisor when a page is marked as
+ 'decrypted' (i.e C-bit removed).
+Please update the comment.
+ @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 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 = KVM_HC_MAP_GPA_RANGE;
+ 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 = AsmReadCs() & 0x3;
+ GhcbSetRegValid (Ghcb, GhcbCpl);
+
+ Status = VmgExit (Ghcb, SVM_EXIT_VMMCALL, 0, 0);
+ if (Status) {
+ DEBUG ((DEBUG_ERROR, "SVM_EXIT_VMMCALL failed %lx\n", Status));
You need to issue an SEV-ES guest termination vmexit followed by a deadloop to ensure
that boot does not proceed.

You probably also need to check for the RAX register for the return code.


+ }
+ VmgDone (Ghcb, InterruptState);
+ } else {
+ SetMemoryEncDecHypercall3AsmStub (
+ KVM_HC_MAP_GPA_RANGE,
+ PhysicalAddress,
+ Pages,
+ Mode
+ );
How do you know whether the hyperviosr supports the Live migration ? In other
words, is it safe to call the HC without knowing if HV supports the feature ?

Also, what will happen if we pass a bogus GPA. Does the HC return an error ?
Same as SEV-ES block, you probably need to check the RAX register for the
return code. On failure, cause an assert() and terminate the boot.


+ }
+}
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index f53efeae79..36f1d82ce7 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -176,6 +176,7 @@
VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+ MemEncryptHypercallLib|OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
!if $(SMM_REQUIRE) == FALSE
LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
!endif
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index b3662e17f2..2a743688b4 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -180,6 +180,7 @@
VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+ MemEncryptHypercallLib|OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
!if $(SMM_REQUIRE) == FALSE
LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
!endif
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 0a237a9058..eb9da51a15 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -180,6 +180,7 @@
VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+ MemEncryptHypercallLib|OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
!if $(SMM_REQUIRE) == FALSE
LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
!endif
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 3c1ca6bfd4..de0c052832 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -167,6 +167,7 @@
QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+ MemEncryptHypercallLib|OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
Update the AmdSev.dsc to include this library.


-Brijesh


Brijesh Singh
 

On 6/22/2021 2:47 PM, Brijesh Singh wrote:


On 6/21/2021 8:56 AM, 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>
Remove this newline.

Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
Maintainers.txt | 2 +
OvmfPkg/Include/Library/MemEncryptHypercallLib.h | 43 ++++++++
OvmfPkg/Library/MemEncryptHypercallLib/Ia32/MemEncryptHypercallLib.c | 37 +++++++
OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf | 42 ++++++++
OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm | 28 ++++++
OvmfPkg/Library/MemEncryptHypercallLib/X64/MemEncryptHypercallLib.c | 105 ++++++++++++++++++++
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
OvmfPkg/OvmfXen.dsc | 1 +
10 files changed, 261 insertions(+)

diff --git a/Maintainers.txt b/Maintainers.txt
index ea54e0b7e9..8ecc8464ba 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -449,8 +449,10 @@ F: OvmfPkg/AmdSev/
F: OvmfPkg/AmdSevDxe/
F: OvmfPkg/Include/Guid/ConfidentialComputingSecret.h
F: OvmfPkg/Include/Library/MemEncryptSevLib.h
+F: OvmfPkg/Include/Library/MemEncryptHypercallLib.h
F: OvmfPkg/IoMmuDxe/AmdSevIoMmu.*
F: OvmfPkg/Library/BaseMemEncryptSevLib/
+F: OvmfPkg/Library/MemEncryptHypercallLib/
F: OvmfPkg/Library/PlatformBootManagerLibGrub/
F: OvmfPkg/Library/VmgExitLib/
F: OvmfPkg/PlatformPei/AmdSev.c
diff --git a/OvmfPkg/Include/Library/MemEncryptHypercallLib.h b/OvmfPkg/Include/Library/MemEncryptHypercallLib.h
new file mode 100644
index 0000000000..b241a189b6
--- /dev/null
+++ b/OvmfPkg/Include/Library/MemEncryptHypercallLib.h
@@ -0,0 +1,43 @@
+/** @file
+
+ Define Secure Encrypted Virtualization (SEV) hypercall library.
+
+ Copyright (c) 2020, AMD Incorporated. All rights reserved.<BR>
^^^^
2021

+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _MEM_ENCRYPT_HYPERCALL_LIB_H_
+#define _MEM_ENCRYPT_HYPERCALL_LIB_H_
+
+#include <Base.h>
+
+#define KVM_HC_MAP_GPA_RANGE 12
+#define KVM_MAP_GPA_RANGE_PAGE_SZ_4K 0
+#define KVM_MAP_GPA_RANGE_PAGE_SZ_2M (1 << 0)
+#define KVM_MAP_GPA_RANGE_PAGE_SZ_1G (1 << 1)
+#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)
+
+/**
+ This hyercall is used to notify hypervisor when a page is marked as
+ 'decrypted' (i.e C-bit removed).
+
Looking at the function signature it seems this routine is used for both set
and clear. Please update the comment accordingly.


+ @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/Ia32/MemEncryptHypercallLib.c b/OvmfPkg/Library/MemEncryptHypercallLib/Ia32/MemEncryptHypercallLib.c
new file mode 100644
index 0000000000..2e73d47ee6
--- /dev/null
+++ b/OvmfPkg/Library/MemEncryptHypercallLib/Ia32/MemEncryptHypercallLib.c
@@ -0,0 +1,37 @@
+/** @file
+
+ Secure Encrypted Virtualization (SEV) hypercall helper library
+
+ Copyright (c) 2020, AMD Incorporated. All rights reserved.<BR>
^^^^
2021

+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Uefi.h>
+#include <Uefi/UefiBaseType.h>
+#include <Library/BaseLib.h>
+
+/**
+ 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 PHYSICAL_ADDRESS PhysicalAddress,
+ IN UINTN Pages,
+ IN UINTN Mode
+ )
+{
+ //
+ // Memory encryption bit is not accessible in 32-bit mode
+ //
+}
diff --git a/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf b/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
new file mode 100644
index 0000000000..a77d58a7e6
--- /dev/null
+++ b/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
@@ -0,0 +1,42 @@
+## @file
+# Library provides the hypervisor helper functions for SEV guest
+#
+# Copyright (c) 2020 Advanced Micro Devices. All rights reserved.<BR>
^^^^
2021
+#
+# 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
+
+#
+# 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]
+ X64/MemEncryptHypercallLib.c
+ X64/AsmHelperStub.nasm
+
+[Sources.IA32]
+ Ia32/MemEncryptHypercallLib.c
+
+[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..f29b96f9b0
--- /dev/null
+++ b/OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm
@@ -0,0 +1,28 @@
+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
diff --git a/OvmfPkg/Library/MemEncryptHypercallLib/X64/MemEncryptHypercallLib.c b/OvmfPkg/Library/MemEncryptHypercallLib/X64/MemEncryptHypercallLib.c
new file mode 100644
index 0000000000..1c09ea012b
--- /dev/null
+++ b/OvmfPkg/Library/MemEncryptHypercallLib/X64/MemEncryptHypercallLib.c
@@ -0,0 +1,105 @@
+/** @file
+
+ Secure Encrypted Virtualization (SEV) hypercall helper library
+
+ Copyright (c) 2020, AMD Incorporated. All rights reserved.<BR>
^^^^
2021
+
+ 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
+ );
+
The function signature does not match with documented signature. Fix the
SetMemoryEncDecHypercall3AsmStub() documented in AsmHelperStub.nasm to use
UINTN.


+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);
+}
+
This looks similar to VmgSetOffsetValid().

+/**
+ This hyercall is used to notify hypervisor when a page is marked as
+ 'decrypted' (i.e C-bit removed).
+Please update the comment.
+ @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 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 = KVM_HC_MAP_GPA_RANGE;
+ 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 = AsmReadCs() & 0x3;
+ GhcbSetRegValid (Ghcb, GhcbCpl);
+
+ Status = VmgExit (Ghcb, SVM_EXIT_VMMCALL, 0, 0);
+ if (Status) {
+ DEBUG ((DEBUG_ERROR, "SVM_EXIT_VMMCALL failed %lx\n", Status));
You need to issue an SEV-ES guest termination vmexit followed by a deadloop to ensure
that boot does not proceed.

You probably also need to check for the RAX register for the return code.


+ }
+ VmgDone (Ghcb, InterruptState);
+ } else {
+ SetMemoryEncDecHypercall3AsmStub (
+ KVM_HC_MAP_GPA_RANGE,
+ PhysicalAddress,
+ Pages,
+ Mode
+ );
How do you know whether the hyperviosr supports the Live migration ? In other
words, is it safe to call the HC without knowing if HV supports the feature ?
I see that in patch#4 you are adding a support to query the HV features flag
to determine whether HV supports the Live migration. You probably need to
pull that function into this library and and check the feature flag before
invoking HC.

This library can also export the function for others to use. You can cache
the value on the first call (similar to how BaseMemEncryptSevLib does for
the MemEncryptSevIsEnabled).


Also, what will happen if we pass a bogus GPA. Does the HC return an error ?
Same as SEV-ES block, you probably need to check the RAX register for the
return code. On failure, cause an assert() and terminate the boot.


+ }
+}
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index f53efeae79..36f1d82ce7 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -176,6 +176,7 @@
VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+ MemEncryptHypercallLib|OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
!if $(SMM_REQUIRE) == FALSE
LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
!endif
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index b3662e17f2..2a743688b4 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -180,6 +180,7 @@
VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+ MemEncryptHypercallLib|OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
!if $(SMM_REQUIRE) == FALSE
LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
!endif
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 0a237a9058..eb9da51a15 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -180,6 +180,7 @@
VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+ MemEncryptHypercallLib|OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
!if $(SMM_REQUIRE) == FALSE
LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
!endif
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 3c1ca6bfd4..de0c052832 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -167,6 +167,7 @@
QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+ MemEncryptHypercallLib|OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
Update the AmdSev.dsc to include this library.


-Brijesh


Lendacky, Thomas
 

On 6/21/21 8:56 AM, 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.
Does this have to be a new library? It's just a single function and so I
would think it could live in the BaseMemEncryptSevLib library where the
change to the c-bit is being done anyway.


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>
---
Maintainers.txt | 2 +
OvmfPkg/Include/Library/MemEncryptHypercallLib.h | 43 ++++++++
OvmfPkg/Library/MemEncryptHypercallLib/Ia32/MemEncryptHypercallLib.c | 37 +++++++
OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf | 42 ++++++++
OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm | 28 ++++++
OvmfPkg/Library/MemEncryptHypercallLib/X64/MemEncryptHypercallLib.c | 105 ++++++++++++++++++++
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
OvmfPkg/OvmfXen.dsc | 1 +
10 files changed, 261 insertions(+)

diff --git a/Maintainers.txt b/Maintainers.txt
index ea54e0b7e9..8ecc8464ba 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -449,8 +449,10 @@ F: OvmfPkg/AmdSev/
F: OvmfPkg/AmdSevDxe/
F: OvmfPkg/Include/Guid/ConfidentialComputingSecret.h
F: OvmfPkg/Include/Library/MemEncryptSevLib.h
+F: OvmfPkg/Include/Library/MemEncryptHypercallLib.h
F: OvmfPkg/IoMmuDxe/AmdSevIoMmu.*
F: OvmfPkg/Library/BaseMemEncryptSevLib/
+F: OvmfPkg/Library/MemEncryptHypercallLib/
F: OvmfPkg/Library/PlatformBootManagerLibGrub/
F: OvmfPkg/Library/VmgExitLib/
F: OvmfPkg/PlatformPei/AmdSev.c
diff --git a/OvmfPkg/Include/Library/MemEncryptHypercallLib.h b/OvmfPkg/Include/Library/MemEncryptHypercallLib.h
new file mode 100644
index 0000000000..b241a189b6
--- /dev/null
+++ b/OvmfPkg/Include/Library/MemEncryptHypercallLib.h
@@ -0,0 +1,43 @@
+/** @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 KVM_HC_MAP_GPA_RANGE 12
+#define KVM_MAP_GPA_RANGE_PAGE_SZ_4K 0
+#define KVM_MAP_GPA_RANGE_PAGE_SZ_2M (1 << 0)
Use the BIT0 define, e.g.:
#define KVM_MAP_GPA_RANGE_PAGE_SZ_2M BIT0

+#define KVM_MAP_GPA_RANGE_PAGE_SZ_1G (1 << 1)
BIT1

Also, where are these used? Are they supposed to be part of the "Mode"
parameter, in which case the comment for Mode below is incorrect.

+#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)
+
+/**
+ 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/Ia32/MemEncryptHypercallLib.c b/OvmfPkg/Library/MemEncryptHypercallLib/Ia32/MemEncryptHypercallLib.c
new file mode 100644
index 0000000000..2e73d47ee6
--- /dev/null
+++ b/OvmfPkg/Library/MemEncryptHypercallLib/Ia32/MemEncryptHypercallLib.c
@@ -0,0 +1,37 @@
+/** @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>
+
+/**
+ 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
This isn't actually SetCBit or ClearCBit based on how I see it used below.

+
+**/
+
+VOID
+EFIAPI
+SetMemoryEncDecHypercall3 (
+ IN PHYSICAL_ADDRESS PhysicalAddress,
+ IN UINTN Pages,
This doesn't match the comment above.

+ IN UINTN Mode
+ )
+{
+ //
+ // Memory encryption bit is not accessible in 32-bit mode
+ //
+}
diff --git a/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf b/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
new file mode 100644
index 0000000000..a77d58a7e6
--- /dev/null
+++ b/OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
@@ -0,0 +1,42 @@
+## @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
+
+#
+# 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]
+ X64/MemEncryptHypercallLib.c
+ X64/AsmHelperStub.nasm
+
+[Sources.IA32]
+ Ia32/MemEncryptHypercallLib.c
+
+[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..f29b96f9b0
--- /dev/null
+++ b/OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm
@@ -0,0 +1,28 @@
+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
Placing these comments on the same line will make this more readable, e.g.:

mov rax, rcx ; Copy HypercallNum 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
diff --git a/OvmfPkg/Library/MemEncryptHypercallLib/X64/MemEncryptHypercallLib.c b/OvmfPkg/Library/MemEncryptHypercallLib/X64/MemEncryptHypercallLib.c
new file mode 100644
index 0000000000..1c09ea012b
--- /dev/null
+++ b/OvmfPkg/Library/MemEncryptHypercallLib/X64/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
+ );
+
+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);
+}
As Brijesh noted, you should be using the function defined in VmgExitLib.h.

+
+/**
+ 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 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 = KVM_HC_MAP_GPA_RANGE;
+ 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 = AsmReadCs() & 0x3;
+ 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 (
+ KVM_HC_MAP_GPA_RANGE,
+ PhysicalAddress,
+ Pages,
+ Mode
+ );
+ }
+}
You could just issue the VMMCALL and, for SEV-ES, let the VC handler take
care of this. You would just have to add some smarts to the VC handler to
compare the hypercall number and add the additional register values. You
could probably get rid of a level of function calls that way. Thoughts?

Thanks,
Tom

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index f53efeae79..36f1d82ce7 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -176,6 +176,7 @@
VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+ MemEncryptHypercallLib|OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
!if $(SMM_REQUIRE) == FALSE
LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
!endif
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index b3662e17f2..2a743688b4 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -180,6 +180,7 @@
VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+ MemEncryptHypercallLib|OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
!if $(SMM_REQUIRE) == FALSE
LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
!endif
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 0a237a9058..eb9da51a15 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -180,6 +180,7 @@
VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+ MemEncryptHypercallLib|OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
!if $(SMM_REQUIRE) == FALSE
LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
!endif
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 3c1ca6bfd4..de0c052832 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -167,6 +167,7 @@
QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+ MemEncryptHypercallLib|OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf


Ashish Kalra
 

Hello Tom,

On Tue, Jun 22, 2021 at 05:47:48PM -0500, Tom Lendacky wrote:
...
+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 = KVM_HC_MAP_GPA_RANGE;
+ 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 = AsmReadCs() & 0x3;
+ 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 (
+ KVM_HC_MAP_GPA_RANGE,
+ PhysicalAddress,
+ Pages,
+ Mode
+ );
+ }
+}
You could just issue the VMMCALL and, for SEV-ES, let the VC handler take
care of this. You would just have to add some smarts to the VC handler to
compare the hypercall number and add the additional register values. You
could probably get rid of a level of function calls that way. Thoughts?
IIRC, we have already discussed this internally.

Letting the VC handler do it was making it too complicated to add hooks
inside the VmgExitLib, and corresponding updation of MdePkg and UefiCpuPkg
(as described in the email thread below), and at that time
Brijesh had suggested the use of this alternative VmgExit() approach.

Email thread copied below :

...
Well, I does not mean that you should literally use VMMCALL instruction instead you use its corresponding VMGEXIT number.
Something like this:

Status = VmgExit (Ghcb, SVM_EXIT_VMMCALL, 0, 0);

This way, a #VC will not be kicked in and there is no need to hook anything inside the VmgExitLib. Maybe Tom can correct me if that is not acceptable.

-Brijesh

I am not able to follow your OVMF patches, could you provide a very high level overview of what exactly you are trying to achieve? Its possible that I am missing something fundamental but why do we care of Hypercall inside the bare metal pkg (e.g MdePkg, UefiCpuPkg)? Why we are needing a Hob etc ? IMO, a Hypercall implementation should be straight forward like what I did the SEV live migration. In case of ES, all you need to use the Ghcb instance to save the register values (rax, rbx, rcx etc) then invoke vmmcall instruction.
I need to do this hypercall validation and setup as part of VC# exception's VMMCALL handling,
i.e, in the VmgExitLib code. I need to use a HOB to store/cache hypercalls invoked during SEC
and PEI phase and flush them later at DXE IPL phase. As VmgExitLib code is invoked in SEC and
PEI phases and references MdePkg and UefiCpuPkg, i need to add HypercallLib references in UefiCpuPkg.

Also, i need to verify if Hypercall library interfaces are being invoked during SEC and/or PEI phase,
currently i do it by checking PcdOvmfSecGhcbBase and for accessing that i had to add reference to
it in MdePkg/MdePkg.dec and UefiCpuPkg/UefiCpuPkg.dec, if i can check that the Hypercall library
interfaces are being invoked in SEC/PEI phase using some other mechanism then i can drop this
reference to PcdOvmfSecGhcbBase.
...


Thanks,
Ashish


Brijesh Singh
 

On 6/22/2021 6:20 PM, Ashish Kalra wrote:
Hello Tom,

On Tue, Jun 22, 2021 at 05:47:48PM -0500, Tom Lendacky wrote:
...
+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 = KVM_HC_MAP_GPA_RANGE;
+ 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 = AsmReadCs() & 0x3;
+ 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 (
+ KVM_HC_MAP_GPA_RANGE,
+ PhysicalAddress,
+ Pages,
+ Mode
+ );
+ }
+}
You could just issue the VMMCALL and, for SEV-ES, let the VC handler take
care of this. You would just have to add some smarts to the VC handler to
compare the hypercall number and add the additional register values. You
could probably get rid of a level of function calls that way. Thoughts?
IIRC, we have already discussed this internally.

Letting the VC handler do it was making it too complicated to add hooks
inside the VmgExitLib, and corresponding updation of MdePkg and UefiCpuPkg
(as described in the email thread below), and at that time
Brijesh had suggested the use of this alternative VmgExit() approach.
A lot has changed in the OVMF code since you last submitted the patch.
IIRC, in ES wip patches Tom was implementing the VC handling library
in the EDK2 core. But in the final version, all the VC handling is
done in the OVMF. The EDK2 core provides a Null library that get
override from the OVMF. In other words, you no longer need to
touch the MdePkg and UefiCpuPkg etc for this change.

I agree with Tom that it would be nice if we can add smarts
in the VC handler. My previous comment was mainly focuses around
how you can avoid touching the EDK2 core.


Email thread copied below :

...
Well, I does not mean that you should literally use VMMCALL instruction instead you use its corresponding VMGEXIT number.
Something like this:

Status = VmgExit (Ghcb, SVM_EXIT_VMMCALL, 0, 0);

This way, a #VC will not be kicked in and there is no need to hook anything inside the VmgExitLib. Maybe Tom can correct me if that is not acceptable.

-Brijesh

I am not able to follow your OVMF patches, could you provide a very high level overview of what exactly you are trying to achieve? Its possible that I am missing something fundamental but why do we care of Hypercall inside the bare metal pkg (e.g MdePkg, UefiCpuPkg)? Why we are needing a Hob etc ? IMO, a Hypercall implementation should be straight forward like what I did the SEV live migration. In case of ES, all you need to use the Ghcb instance to save the register values (rax, rbx, rcx etc) then invoke vmmcall instruction.
I need to do this hypercall validation and setup as part of VC# exception's VMMCALL handling,
i.e, in the VmgExitLib code. I need to use a HOB to store/cache hypercalls invoked during SEC
and PEI phase and flush them later at DXE IPL phase. As VmgExitLib code is invoked in SEC and
PEI phases and references MdePkg and UefiCpuPkg, i need to add HypercallLib references in UefiCpuPkg.

Also, i need to verify if Hypercall library interfaces are being invoked during SEC and/or PEI phase,
currently i do it by checking PcdOvmfSecGhcbBase and for accessing that i had to add reference to
it in MdePkg/MdePkg.dec and UefiCpuPkg/UefiCpuPkg.dec, if i can check that the Hypercall library
interfaces are being invoked in SEC/PEI phase using some other mechanism then i can drop this
reference to PcdOvmfSecGhcbBase.
...


Thanks,
Ashish


Ashish Kalra
 

Hello Tom,

On Tue, Jun 22, 2021 at 05:47:48PM -0500, Tom Lendacky wrote:
On 6/21/21 8:56 AM, 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.
Does this have to be a new library? It's just a single function and so I
would think it could live in the BaseMemEncryptSevLib library where the
change to the c-bit is being done anyway.
Actually i like this approach, instead of adding a new library.

Again, IIRC we had discussed this internally and then decided to
create a new hypercall library similar to Xen, with reference to
the email copied below :

...
Xen has support under Ovmf for a Xen hypercall library
(OvmfPkg/Library/XenHypercallLib). Take a look at that and maybe create
something similar for KVM.
...

Thanks,
Ashish


Ashish Kalra
 

Hello Tom, Brijesh,

Thanks for your comments and feedback on this patch-set.

With reference to those, i will remove the new library and
add the hypercall function inside BaseMemEncryptSevLib library.

Also i will let the hypercall handling for SEV-ES be done
as part of #VC exception handling in VmgExitLib library.

Thanks,
Ashish

On Wed, Jun 23, 2021 at 01:47:47AM +0000, Ashish Kalra wrote:
Hello Tom,

On Tue, Jun 22, 2021 at 05:47:48PM -0500, Tom Lendacky wrote:
On 6/21/21 8:56 AM, 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.
Does this have to be a new library? It's just a single function and so I
would think it could live in the BaseMemEncryptSevLib library where the
change to the c-bit is being done anyway.
Actually i like this approach, instead of adding a new library.

Again, IIRC we had discussed this internally and then decided to
create a new hypercall library similar to Xen, with reference to
the email copied below :

...
Xen has support under Ovmf for a Xen hypercall library
(OvmfPkg/Library/XenHypercallLib). Take a look at that and maybe create
something similar for KVM.
...

Thanks,
Ashish