[edk2-devel] [PATCH v5 4/4] OvmfPkg/AmdSevDxe: Add support for SEV live migration.
Dov Murik
dovmurik at linux.ibm.com
Mon Jul 19 07:31:10 UTC 2021
Ashish,
On 08/07/2021 17:09, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra at amd.com>
>
> Check for SEV live migration feature support, if detected
> setup a new UEFI enviroment variable to indicate OVMF
> support for SEV live migration.
>
> The new runtime UEFI environment variable is set via the
> notification function registered for the
> EFI_END_OF_DXE_EVENT_GROUP_GUID event in AmdSevDxe driver.
>
Why is this indirect notification needed? Why not simply call
gRT->SetVariable in AmdSevDxeEntryPoint (instead of calling CreateEventEx)?
If this is needed, please add a clarification (in the commit message and
before the CreateEventEx call).
> Signed-off-by: Ashish Kalra <ashish.kalra at amd.com>
> ---
> OvmfPkg/AmdSevDxe/AmdSevDxe.c | 59 ++++++++++++++++++++
> OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 4 ++
> OvmfPkg/Include/Guid/MemEncryptLib.h | 20 +++++++
> OvmfPkg/OvmfPkg.dec | 1 +
> 4 files changed, 84 insertions(+)
>
> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> index c66c4e9b92..45adf3249c 100644
> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> @@ -15,10 +15,49 @@
> #include <Library/BaseMemoryLib.h>
> #include <Library/DebugLib.h>
> #include <Library/DxeServicesTableLib.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> #include <Library/MemEncryptSevLib.h>
> #include <Library/MemoryAllocationLib.h>
> +#include <Guid/MemEncryptLib.h>
> +#include <Guid/EventGroup.h>
> #include <Library/PcdLib.h>
>
> +STATIC
> +VOID
> +EFIAPI
> +AmdSevDxeOnEndOfDxe (
> + IN EFI_EVENT Event,
> + IN VOID *EventToSignal
> + )
> +{
> + EFI_STATUS Status;
> + BOOLEAN SevLiveMigrationEnabled;
> +
> + SevLiveMigrationEnabled = MemEncryptSevLiveMigrationIsEnabled();
> +
> + if (SevLiveMigrationEnabled) {
> + Status = gRT->SetVariable (
> + L"SevLiveMigrationEnabled",
> + &gMemEncryptGuid,
> + EFI_VARIABLE_NON_VOLATILE |
> + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> + EFI_VARIABLE_RUNTIME_ACCESS,
> + sizeof (BOOLEAN),
Should be:
sizeof SevLiveMigrationEnabled,
> + &SevLiveMigrationEnabled
> + );
> +
> + DEBUG ((
> + DEBUG_INFO,
> + "%a: Setting SevLiveMigrationEnabled variable, status = %lx\n",
> + __FUNCTION__,
> + Status
> + ));
> + }
> +
> + DEBUG ((DEBUG_VERBOSE, "%a\n", __FUNCTION__));
Remove debug print.
> +}
> +
> EFI_STATUS
> EFIAPI
> AmdSevDxeEntryPoint (
> @@ -30,6 +69,7 @@ AmdSevDxeEntryPoint (
> EFI_GCD_MEMORY_SPACE_DESCRIPTOR *AllDescMap;
> UINTN NumEntries;
> UINTN Index;
> + EFI_EVENT Event;
>
> //
> // Do nothing when SEV is not enabled
> @@ -130,5 +170,24 @@ AmdSevDxeEntryPoint (
> }
> }
>
> + //
> + // Register EFI_END_OF_DXE_EVENT_GROUP_GUID event.
> + // The notification function sets the runtime variable indicating OVMF
> + // support for SEV live migration.
> + //
> + Status = gBS->CreateEventEx (
> + EVT_NOTIFY_SIGNAL,
> + TPL_CALLBACK,
> + AmdSevDxeOnEndOfDxe,
> + NULL,
> + &gEfiEndOfDxeEventGroupGuid,
> + &Event
> + );
> +
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_INFO, "%a: CreateEventEx(): %r\n",
> + __FUNCTION__, Status));
> + }
> +
> return EFI_SUCCESS;
> }
> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
> index 0676fcc5b6..f4e40ff412 100644
> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
> @@ -45,3 +45,7 @@
>
> [Pcd]
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
> +
> +[Guids]
> + gMemEncryptGuid
> + gEfiEndOfDxeEventGroupGuid ## CONSUMES ## Event
> diff --git a/OvmfPkg/Include/Guid/MemEncryptLib.h b/OvmfPkg/Include/Guid/MemEncryptLib.h
> new file mode 100644
> index 0000000000..4c046ba439
> --- /dev/null
> +++ b/OvmfPkg/Include/Guid/MemEncryptLib.h
Should the filename, GUID #define name, and global var name include
"AMD" or "SEV" in them? (and similarly in the corresponding Linux patch)
Or: maybe the new "SevLiveMigrationEnabled" variable can be set in the
confidential computing GUID? (not sure what are the guidelines for
creating or reusing GUIDs).
> @@ -0,0 +1,20 @@
> +/** @file
> +
> + AMD Memory Encryption GUID, define a new GUID for defining
> + new UEFI enviroment variables assocaiated with SEV Memory Encryption.
typos: environment, associated
> +
> + Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef __MEMENCRYPT_LIB_H__
> +#define __MEMENCRYPT_LIB_H__
> +
> +#define MEMENCRYPT_GUID \
> +{0x0cf29b71, 0x9e51, 0x433a, {0xa3, 0xb7, 0x81, 0xf3, 0xab, 0x16, 0xb8, 0x75}}
> +
> +extern EFI_GUID gMemEncryptGuid;
> +
> +#endif
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index 6ae733f6e3..e452dc8494 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -122,6 +122,7 @@
> gQemuKernelLoaderFsMediaGuid = {0x1428f772, 0xb64a, 0x441e, {0xb8, 0xc3, 0x9e, 0xbd, 0xd7, 0xf8, 0x93, 0xc7}}
> gGrubFileGuid = {0xb5ae312c, 0xbc8a, 0x43b1, {0x9c, 0x62, 0xeb, 0xb8, 0x26, 0xdd, 0x5d, 0x07}}
> gConfidentialComputingSecretGuid = {0xadf956ad, 0xe98c, 0x484c, {0xae, 0x11, 0xb5, 0x1c, 0x7d, 0x33, 0x64, 0x47}}
> + gMemEncryptGuid = {0x0cf29b71, 0x9e51, 0x433a, {0xa3, 0xb7, 0x81, 0xf3, 0xab, 0x16, 0xb8, 0x75}}
>
> [Ppis]
> # PPI whose presence in the PPI database signals that the TPM base address
>
-Dov
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77872): https://edk2.groups.io/g/devel/message/77872
Mute This Topic: https://groups.io/mt/84068379/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