On 8/2/21 7:31 AM, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.ka...@amd.com>
>
> Add API to issue hypercall on page encryption state change.
>
> By default all the SEV guest memory regions are considered encrypted,
> if a guest changes the encryption attribute of the page (e.g mark a
> page as decrypted) then notify hypervisor. Hypervisor will need to
> track the unencrypted pages. The information will be used during
> guest live migration, guest page migration and guest debugging.
>
> This hypercall is used to notify hypervisor when the page's
> encryption state changes.
>
> Cc: Jordan Justen <jordan.l.jus...@intel.com>
> Cc: Ard Biesheuvel <ard.biesheu...@arm.com>
> Signed-off-by: Brijesh Singh <brijesh.si...@amd.com>
> Signed-off-by: Ashish Kalra <ashish.ka...@amd.com>
> ---
> OvmfPkg/Include/Library/MemEncryptSevLib.h | 43
> +++++++++++++
> OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf | 1 +
> OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c | 27
> +++++++++
> OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf | 1 +
> OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c | 20
> ++++++
> OvmfPkg/Library/BaseMemEncryptSevLib/X64/AsmHelperStub.nasm | 33
> ++++++++++
> OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c | 64
> ++++++++++++++++++++
> 7 files changed, 189 insertions(+)
>
> diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h
> b/OvmfPkg/Include/Library/MemEncryptSevLib.h
> index 59f694fb8a..56cc7bb958 100644
> --- a/OvmfPkg/Include/Library/MemEncryptSevLib.h
> +++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
> @@ -249,4 +249,47 @@ KvmDetectSevLiveMigrationFeature(
> VOID
> );
>
> +/**
> + This hypercall is used to notify hypervisor when the page's encryption
> + state changes.
> +
> + @param[in] PhysicalAddress The physical address that is the start
> address
> + of a memory region.
> + @param[in] Pages Number of pages in memory region.
> + @param[in] IsEncrypted Encrypted or Decrypted.
> +
> + @retval RETURN_SUCCESS Hypercall returned success.
> + @retval RETURN_UNSUPPORTED Hypercall not supported.
> + @retval RETURN_NO_MAPPING Hypercall returned error.
> +**/
> +RETURN_STATUS
> +EFIAPI
> +SetMemoryEncDecHypercall3 (
> + IN UINTN PhysicalAddress,
> + IN UINTN Pages,
> + IN BOOLEAN IsEncrypted
> + );
> +
> +#define KVM_HC_MAP_GPA_RANGE 12
> +#define KVM_MAP_GPA_RANGE_PAGE_SZ_4K 0
> +#define KVM_MAP_GPA_RANGE_PAGE_SZ_2M BIT0
> +#define KVM_MAP_GPA_RANGE_PAGE_SZ_1G BIT1
> +#define KVM_MAP_GPA_RANGE_ENC_STAT(n) ((n) << 4)
s/STAT/STATE/ ?
> +#define KVM_MAP_GPA_RANGE_ENCRYPTED KVM_MAP_GPA_RANGE_ENC_STAT(1)
> +#define KVM_MAP_GPA_RANGE_DECRYPTED KVM_MAP_GPA_RANGE_ENC_STAT(0)
> +
> +/**
> + Interface exposed by the ASM implementation of the core hypercall
Need to put the function parameters in the comment here.
> +
> + @retval Hypercall returned status.
> +**/
> +UINTN
> +EFIAPI
> +SetMemoryEncDecHypercall3AsmStub (
> + IN UINTN HypercallNum,
> + IN UINTN PhysicalAddress,
> + IN UINTN Pages,
> + IN UINTN Attributes
> + );
> +
> #endif // _MEM_ENCRYPT_SEV_LIB_H_
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
> b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
> index f2e162d680..0c28afadee 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
> @@ -38,6 +38,7 @@
> X64/PeiDxeVirtualMemory.c
> X64/VirtualMemory.c
> X64/VirtualMemory.h
> + X64/AsmHelperStub.nasm
>
> [Sources.IA32]
> Ia32/MemEncryptSevLib.c
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
> b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
> index be260e0d10..516d639489 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
> @@ -136,3 +136,30 @@ MemEncryptSevClearMmioPageEncMask (
> //
> return RETURN_UNSUPPORTED;
> }
> +
> +/**
> + This hyercall is used to notify hypervisor when the page's encryption
> + state changes.
> +
> + @param[in] PhysicalAddress The physical address that is the start
> address
> + of a memory region.
> + @param[in] Pages Number of Pages in the memory region.
> + @param[in] IsEncrypted Encrypted or Decrypted.
> +
> + @retval RETURN_SUCCESS Hypercall returned success.
> + @retval RETURN_UNSUPPORTED Hypercall not supported.
> + @retval RETURN_NO_MAPPING Hypercall returned error.
> +**/
> +RETURN_STATUS
> +EFIAPI
> +SetMemoryEncDecHypercall3 (
> + IN UINTN PhysicalAddress,
> + IN UINTN Pages,
> + IN BOOLEAN IsEncrypted
> + )
> +{
> + //
> + // Memory encryption bit is not accessible in 32-bit mode
> + //
> + return RETURN_UNSUPPORTED;
> +}
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
> b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
> index 03a78c32df..3233ca7979 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
> @@ -38,6 +38,7 @@
> X64/PeiDxeVirtualMemory.c
> X64/VirtualMemory.c
> X64/VirtualMemory.h
> + X64/AsmHelperStub.nasm
>
> [Sources.IA32]
> Ia32/MemEncryptSevLib.c
> diff --git
> a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
> b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
> index d9f7befcd2..ebb1c39319 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
> @@ -118,6 +118,26 @@ MemEncryptSevLiveMigrationIsEnabled (
> return FALSE;
> }
>
> +/**
> + Interface exposed by the ASM implementation of the core hypercall
> +
> + @retval Hypercall returned status.
> +**/
> +UINTN
> +EFIAPI
> +SetMemoryEncDecHypercall3AsmStub (
> + IN UINTN HypercallNum,
> + IN UINTN PhysicalAddress,
> + IN UINTN Pages,
> + IN UINTN Attributes
> + )
> +{
> + //
> + // Not used in SEC phase.
> + //
> + return RETURN_UNSUPPORTED;
> +}
> +
> /**
> Returns the SEV encryption mask.
>
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/AsmHelperStub.nasm
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/AsmHelperStub.nasm
> new file mode 100644
> index 0000000000..0ec35dd9b6
> --- /dev/null
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/AsmHelperStub.nasm
> @@ -0,0 +1,33 @@
> +/** @file
> +
> + ASM helper stub to invoke hypercall
> +
> + Copyright (c) 2021, AMD Incorporated. All rights reserved.<BR>
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +DEFAULT REL
> +SECTION .text
> +
> +; UINTN
> +; EFIAPI
> +; SetMemoryEncDecHypercall3AsmStub (
> +; IN UINTN HypercallNum,
> +; IN UINTN Arg1,
> +; IN UINTN Arg2,
> +; IN UINTN Arg3
> +; );
> +global ASM_PFX(SetMemoryEncDecHypercall3AsmStub)
> +ASM_PFX(SetMemoryEncDecHypercall3AsmStub):
> + ; UEFI calling conventions require RBX to
> + ; be nonvolatile/callee-saved.
> + push rbx
> + mov rax, rcx ; Copy HypercallNumber to rax
> + mov rbx, rdx ; Copy Arg1 to the register expected by KVM
> + mov rcx, r8 ; Copy Arg2 to register expected by KVM
> + mov rdx, r9 ; Copy Arg3 to register expected by KVM
> + vmmcall ; Call VMMCALL
> + pop rbx
> + ret
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
> index a57e8fd37f..fa679c9fc9 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
> @@ -143,3 +143,67 @@ MemEncryptSevClearMmioPageEncMask (
> );
>
> }
> +
> +/**
> + This hyercall is used to notify hypervisor when the page's encryption
> + state changes.
> +
> + @param[in] PhysicalAddress The physical address that is the start
> address
> + of a memory region.
> + @param[in] Pages Number of Pages in the memory region.
> + @param[in] IsEncrypted Encrypted or Decrypted.
> +
> + @retval RETURN_SUCCESS Hypercall returned success.
> + @retval RETURN_UNSUPPORTED Hypercall not supported.
> + @retval RETURN_NO_MAPPING Hypercall returned error.
> +**/
> +RETURN_STATUS
> +EFIAPI
> +SetMemoryEncDecHypercall3 (
> + IN UINTN PhysicalAddress,
> + IN UINTN Pages,
> + IN BOOLEAN IsEncrypted
> + )
> +{
> + RETURN_STATUS Ret;
> + UINTN Error;
> + UINTN EncryptMask;
> +
> + Ret = RETURN_UNSUPPORTED;
> +
> + if (MemEncryptSevLiveMigrationIsEnabled ()) {
> + Ret = RETURN_SUCCESS;
> + //
> + // The encryption bit is set/clear on the smallest page size, hence
> + // use the 4k page size in MAP_GPA_RANGE hypercall below.
> + //
> + // Also, when the GCD map is being walked and the c-bit being cleared
> + // from MMIO and NonExistent memory spaces, the physical address
> + // range being passed may not be page-aligned and adding an assert
> + // here prevents booting. Hence, rounding it down when calling
> + // SetMemoryEncDecHypercall3AsmStub below.
> + //
> +
> + EncryptMask = IsEncrypted ? KVM_MAP_GPA_RANGE_ENCRYPTED :
> + KVM_MAP_GPA_RANGE_DECRYPTED;
Just a nit, but EncryptMask is a bit confusing because is sounds like the
encryption mask used by SEV, but it's really the page encryption state as
defined by the hypercall, maybe call it EncryptionState or EncryptState?
> +
> + Error = SetMemoryEncDecHypercall3AsmStub (
> + KVM_HC_MAP_GPA_RANGE,
> + PhysicalAddress & ~EFI_PAGE_MASK,
> + Pages,
> + KVM_MAP_GPA_RANGE_PAGE_SZ_4K | EncryptMask
> + );
> +
> + if (Error != 0) {
> + DEBUG ((DEBUG_ERROR,
> + "SetMemoryEncDecHypercall3 failed, Phys = %Lx, Pages = %Ld,
> Err = %Ld\n",
I don't believe the "L" is needed for "Phys" and "Pages" since those are
UINTN variables.
> + PhysicalAddress,
> + Pages,
> + (INT64)Error));
Indentation needs to be two spaces past the "DEBUG" function call.
Thanks,
Tom
> +
> + Ret = RETURN_NO_MAPPING;
> + }
> + }
> +
> + return Ret;
> +}
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78962): https://edk2.groups.io/g/devel/message/78962
Mute This Topic: https://groups.io/mt/84609836/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
