[edk2-devel] [PATCH v4 1/4] OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls.
Brijesh Singh via groups.io
brijesh.singh=amd.com at groups.io
Tue Jun 22 19:47:47 UTC 2021
On 6/21/2021 8:56 AM, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra at 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 at intel.com>
> Cc: Laszlo Ersek <lersek at redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel at arm.com>
>
Remove this newline.
> Signed-off-by: Ashish Kalra <ashish.kalra at 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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#76859): https://edk2.groups.io/g/devel/message/76859
Mute This Topic: https://groups.io/mt/83688875/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list