From: "Ashish Kalra" <ashish.kalra@amd.com>
To: Tom Lendacky <thomas.lendacky@amd.com>
Cc: devel@edk2.groups.io, dovmurik@linux.vnet.ibm.com,
brijesh.singh@amd.com, tobin@ibm.com, jejb@linux.ibm.com,
jordan.l.justen@intel.com, ard.biesheuvel@arm.com,
erdemaktas@google.com, jiewen.yao@intel.com, min.m.xu@intel.com
Subject: Re: [PATCH v6 6/6] OvmfPkg/AmdSevDxe: Add support for SEV live migration.
Date: Tue, 10 Aug 2021 11:13:49 +0000 [thread overview]
Message-ID: <20210810111349.GA9132@ashkalra_ubuntu_server> (raw)
In-Reply-To: <4f2224f2-de60-03a6-333e-163f31fe1c1a@amd.com>
Hello Tom,
On Mon, Aug 09, 2021 at 09:29:29AM -0500, Tom Lendacky wrote:
> On 8/2/21 7:33 AM, Ashish Kalra wrote:
> > From: Ashish Kalra <ashish.kalra@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.
> >
> > AmdSevDxe module is an apriori driver so it gets loaded between PEI
> > and DXE phases and the SetVariable call will fail at the driver's
> > entry point as the Variable DXE module is still not loaded yet.
> > So we need to wait for an event notification which is signaled
> > after the Variable DXE module is loaded, hence, using the
> > EndOfDxe event notification to make this call.
> >
> > Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> > ---
> > OvmfPkg/AmdSevDxe/AmdSevDxe.c | 64 ++++++++++++++++++++
> > OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 4 ++
> > OvmfPkg/Include/Guid/AmdSevMemEncryptLib.h | 20 ++++++
> > OvmfPkg/OvmfPkg.dec | 1 +
> > 4 files changed, 89 insertions(+)
> >
> > diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > index c66c4e9b92..bfad71b9c6 100644
> > --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > @@ -15,10 +15,47 @@
> > #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/AmdSevMemEncryptLib.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",
> > + &gAmdSevMemEncryptGuid,
> > + EFI_VARIABLE_NON_VOLATILE |
> > + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > + EFI_VARIABLE_RUNTIME_ACCESS,
> > + sizeof SevLiveMigrationEnabled,
> > + &SevLiveMigrationEnabled
> > + );
> > +
> > + DEBUG ((
> > + DEBUG_INFO,
> > + "%a: Setting SevLiveMigrationEnabled variable, status = %lx\n",
> > + __FUNCTION__,
> > + Status
> > + ));
> > + }
> > +}
> > +
> > EFI_STATUS
> > EFIAPI
> > AmdSevDxeEntryPoint (
> > @@ -30,6 +67,7 @@ AmdSevDxeEntryPoint (
> > EFI_GCD_MEMORY_SPACE_DESCRIPTOR *AllDescMap;
> > UINTN NumEntries;
> > UINTN Index;
> > + EFI_EVENT Event;
> >
> > //
> > // Do nothing when SEV is not enabled
> > @@ -130,5 +168,31 @@ AmdSevDxeEntryPoint (
> > }
> > }
> >
> > + //
> > + // AmdSevDxe module is an apriori driver so it gets loaded between PEI
> > + // and DXE phases and the SetVariable call will fail at the driver's
> > + // entry point as the Variable DXE module is still not loaded yet.
> > + // So we need to wait for an event notification which is signaled
> > + // after the Variable DXE module is loaded, hence, using the
> > + // EndOfDxe event notification to make this call.
> > + //
> > + // 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",
>
> DEBUG_ERROR?
>
> > + __FUNCTION__, Status));
>
> Should there be an "ASSERT_EFI_ERROR (Status)" after the DEBUG call?
>
I don't think we should do an assert here and abort booting, failure
here will simply disable live migration support but i don't think that
it is such a fatal error that we should abort booting because of that.
OTOH, if there is a failure when doing page encryption status hypercalls
then aborting boot makes sense as guest page encryption status tracking
may be critical for multiple guest operations and not only live
migration.
Thanks,
Ashish
next prev parent reply other threads:[~2021-08-10 11:13 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <cover.1627906232.git.ashish.kalra@amd.com>
2021-08-02 12:31 ` [PATCH v6 1/6] OvmfPkg/BaseMemEncryptLib: Detect SEV live migration feature Ashish Kalra
2021-08-09 13:41 ` Lendacky, Thomas
2021-08-09 14:37 ` Ashish Kalra
2021-08-10 6:05 ` [edk2-devel] " Gerd Hoffmann
2021-08-10 13:04 ` Lendacky, Thomas
2021-08-02 12:31 ` [PATCH v6 2/6] OvmfPkg/BaseMemEncryptLib: Hypercall API for page encryption state change Ashish Kalra
2021-08-09 14:19 ` Lendacky, Thomas
2021-08-02 12:32 ` [PATCH v6 3/6] OvmfPkg/BaseMemEncryptLib: Invoke page encryption state change hypercall Ashish Kalra
2021-08-02 12:32 ` [PATCH v6 4/6] OvmfPkg/VmgExitLib: Encryption state change hypercall support in VC handler Ashish Kalra
2021-08-02 12:33 ` [PATCH v6 5/6] OvmfPkg/PlatformPei: Mark SEC GHCB page as unencrypted via hypercall Ashish Kalra
2021-08-02 12:33 ` [PATCH v6 6/6] OvmfPkg/AmdSevDxe: Add support for SEV live migration Ashish Kalra
2021-08-09 14:29 ` Lendacky, Thomas
2021-08-10 11:13 ` Ashish Kalra [this message]
2021-08-10 13:06 ` Lendacky, Thomas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210810111349.GA9132@ashkalra_ubuntu_server \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox