* [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF.
@ 2020-12-04 0:03 Ashish Kalra
2020-12-04 0:03 ` [PATCH v3 1/3] OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls Ashish Kalra
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Ashish Kalra @ 2020-12-04 0:03 UTC (permalink / raw)
To: devel
Cc: dovmurik, brijesh.singh, tobin, Jon.Grimm, Thomas.Lendacky, jejb,
frankeh, dgilbert, lersek, jordan.l.justen, ard.biesheuvel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 1/3] OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls.
2020-12-04 0:03 [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF Ashish Kalra
@ 2020-12-04 0:03 ` Ashish Kalra
2020-12-06 12:43 ` Dov Murik
2020-12-04 0:03 ` [PATCH v3 2/3] OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall Ashish Kalra
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Ashish Kalra @ 2020-12-04 0:03 UTC (permalink / raw)
To: devel
Cc: dovmurik, brijesh.singh, tobin, Jon.Grimm, Thomas.Lendacky, jejb,
frankeh, dgilbert, lersek, jordan.l.justen, ard.biesheuvel
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 2/3] OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall
2020-12-04 0:03 [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF Ashish Kalra
2020-12-04 0:03 ` [PATCH v3 1/3] OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls Ashish Kalra
@ 2020-12-04 0:03 ` Ashish Kalra
2020-12-04 0:03 ` [PATCH v3 3/3] OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap Ashish Kalra
2020-12-04 3:50 ` [edk2-devel] [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF Laszlo Ersek
3 siblings, 0 replies; 12+ messages in thread
From: Ashish Kalra @ 2020-12-04 0:03 UTC (permalink / raw)
To: devel
Cc: dovmurik, brijesh.singh, tobin, Jon.Grimm, Thomas.Lendacky, jejb,
frankeh, dgilbert, lersek, jordan.l.justen, ard.biesheuvel
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 3/3] OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap.
2020-12-04 0:03 [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF Ashish Kalra
2020-12-04 0:03 ` [PATCH v3 1/3] OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls Ashish Kalra
2020-12-04 0:03 ` [PATCH v3 2/3] OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall Ashish Kalra
@ 2020-12-04 0:03 ` Ashish Kalra
2020-12-04 3:50 ` [edk2-devel] [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF Laszlo Ersek
3 siblings, 0 replies; 12+ messages in thread
From: Ashish Kalra @ 2020-12-04 0:03 UTC (permalink / raw)
To: devel
Cc: dovmurik, brijesh.singh, tobin, Jon.Grimm, Thomas.Lendacky, jejb,
frankeh, dgilbert, lersek, jordan.l.justen, ard.biesheuvel
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF.
2020-12-04 0:03 [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF Ashish Kalra
` (2 preceding siblings ...)
2020-12-04 0:03 ` [PATCH v3 3/3] OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap Ashish Kalra
@ 2020-12-04 3:50 ` Laszlo Ersek
2020-12-04 8:10 ` Ashish Kalra
3 siblings, 1 reply; 12+ messages in thread
From: Laszlo Ersek @ 2020-12-04 3:50 UTC (permalink / raw)
To: devel, ashish.kalra
Cc: dovmurik, brijesh.singh, tobin, Jon.Grimm, Thomas.Lendacky, jejb,
frankeh, dgilbert, jordan.l.justen, ard.biesheuvel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF.
2020-12-04 3:50 ` [edk2-devel] [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF Laszlo Ersek
@ 2020-12-04 8:10 ` Ashish Kalra
2020-12-08 2:44 ` Laszlo Ersek
0 siblings, 1 reply; 12+ messages in thread
From: Ashish Kalra @ 2020-12-04 8:10 UTC (permalink / raw)
To: Laszlo Ersek
Cc: devel, dovmurik, brijesh.singh, tobin, Jon.Grimm, Thomas.Lendacky,
jejb, frankeh, dgilbert, jordan.l.justen, ard.biesheuvel
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&data=04%7C01%7Cashish.kalra%40amd.com%7Cbc3c88f21f1d40b322b408d89807b5c8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637426506192800828%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=VZzP2MVJSECgMhOyuCCASw58g74BiCVAH9JW8hZG3Tw%3D&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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/3] OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls.
2020-12-04 0:03 ` [PATCH v3 1/3] OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls Ashish Kalra
@ 2020-12-06 12:43 ` Dov Murik
2020-12-08 14:23 ` Ashish Kalra
0 siblings, 1 reply; 12+ messages in thread
From: Dov Murik @ 2020-12-06 12:43 UTC (permalink / raw)
To: Ashish Kalra, devel
Cc: brijesh.singh, tobin, Jon.Grimm, Thomas.Lendacky, jejb, frankeh,
dgilbert, lersek, jordan.l.justen, ard.biesheuvel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF.
2020-12-04 8:10 ` Ashish Kalra
@ 2020-12-08 2:44 ` Laszlo Ersek
2020-12-08 4:44 ` Brijesh Singh
2020-12-08 14:57 ` Lendacky, Thomas
0 siblings, 2 replies; 12+ messages in thread
From: Laszlo Ersek @ 2020-12-08 2:44 UTC (permalink / raw)
To: devel, ashish.kalra
Cc: dovmurik, brijesh.singh, tobin, Jon.Grimm, Thomas.Lendacky, jejb,
frankeh, dgilbert, jordan.l.justen, ard.biesheuvel
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&data=04%7C01%7Cashish.kalra%40amd.com%7Cbc3c88f21f1d40b322b408d89807b5c8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637426506192800828%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=VZzP2MVJSECgMhOyuCCASw58g74BiCVAH9JW8hZG3Tw%3D&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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF.
2020-12-08 2:44 ` Laszlo Ersek
@ 2020-12-08 4:44 ` Brijesh Singh
2020-12-08 14:57 ` Lendacky, Thomas
1 sibling, 0 replies; 12+ messages in thread
From: Brijesh Singh @ 2020-12-08 4:44 UTC (permalink / raw)
To: Laszlo Ersek, devel, ashish.kalra
Cc: brijesh.singh, dovmurik, tobin, Jon.Grimm, Thomas.Lendacky, jejb,
frankeh, dgilbert, jordan.l.justen, ard.biesheuvel
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&data=04%7C01%7Cbrijesh.singh%40amd.com%7C13b084db30e246f25b3f08d89b233f99%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637429922982198583%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=wuYFXFyBtwZWSWOCb3OYK8I7MDFAxId%2BC63fsa0XcjQ%3D&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
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/3] OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls.
2020-12-06 12:43 ` Dov Murik
@ 2020-12-08 14:23 ` Ashish Kalra
0 siblings, 0 replies; 12+ messages in thread
From: Ashish Kalra @ 2020-12-08 14:23 UTC (permalink / raw)
To: Dov Murik
Cc: devel, brijesh.singh, tobin, Jon.Grimm, Thomas.Lendacky, jejb,
frankeh, dgilbert, lersek, jordan.l.justen, ard.biesheuvel
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&data=04%7C01%7CAshish.Kalra%40amd.com%7C4123cf72f32946b8e75a08d899e498be%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637428554380675577%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=xUydJsFm7hI04UBbucGpSVD2X%2FIZtwLAMbW%2Bm5RHAuo%3D&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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF.
2020-12-08 2:44 ` Laszlo Ersek
2020-12-08 4:44 ` Brijesh Singh
@ 2020-12-08 14:57 ` Lendacky, Thomas
2020-12-10 7:53 ` Laszlo Ersek
1 sibling, 1 reply; 12+ messages in thread
From: Lendacky, Thomas @ 2020-12-08 14:57 UTC (permalink / raw)
To: Laszlo Ersek, devel, ashish.kalra
Cc: dovmurik, brijesh.singh, tobin, Jon.Grimm, jejb, frankeh,
dgilbert, jordan.l.justen, ard.biesheuvel
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&data=04%7C01%7Cthomas.lendacky%40amd.com%7Caa286d7e06864008110008d89b233ebc%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637429922982193672%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=EjrGD2LNlji8ualk8KClh%2BhqJa5Fm0UzlmPc4%2FQvb2g%3D&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
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF.
2020-12-08 14:57 ` Lendacky, Thomas
@ 2020-12-10 7:53 ` Laszlo Ersek
0 siblings, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2020-12-10 7:53 UTC (permalink / raw)
To: devel, thomas.lendacky, ashish.kalra
Cc: dovmurik, brijesh.singh, tobin, Jon.Grimm, jejb, frankeh,
dgilbert, jordan.l.justen, ard.biesheuvel
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&data=04%7C01%7Cthomas.lendacky%40amd.com%7Caa286d7e06864008110008d89b233ebc%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637429922982193672%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=EjrGD2LNlji8ualk8KClh%2BhqJa5Fm0UzlmPc4%2FQvb2g%3D&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
>>
>
>
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-12-10 7:54 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-04 0:03 [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF Ashish Kalra
2020-12-04 0:03 ` [PATCH v3 1/3] OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls Ashish Kalra
2020-12-06 12:43 ` Dov Murik
2020-12-08 14:23 ` Ashish Kalra
2020-12-04 0:03 ` [PATCH v3 2/3] OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall Ashish Kalra
2020-12-04 0:03 ` [PATCH v3 3/3] OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap Ashish Kalra
2020-12-04 3:50 ` [edk2-devel] [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF Laszlo Ersek
2020-12-04 8:10 ` Ashish Kalra
2020-12-08 2:44 ` Laszlo Ersek
2020-12-08 4:44 ` Brijesh Singh
2020-12-08 14:57 ` Lendacky, Thomas
2020-12-10 7:53 ` Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox