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,
lersek@redhat.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 v5 1/4] OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall
Date: Mon, 19 Jul 2021 20:24:16 +0000 [thread overview]
Message-ID: <20210719202416.GA24481@ashkalra_ubuntu_server> (raw)
In-Reply-To: <8a307c55-337d-b93f-b77c-a33846b8f965@amd.com>
Hello Tom,
On Fri, Jul 16, 2021 at 09:11:23AM -0500, Tom Lendacky wrote:
> On 7/8/21 9:07 AM, Ashish Kalra wrote:
> > From: Ashish Kalra <ashish.kalra@amd.com>
> >
>
> The patch subject is a bit confusing. Something more like "Add API to
> issue hypercall on page encryption state change" or similar, since this is
> issued for changes to shared and private, not just shared.
>
> > 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.
>
> This is a large patch. It looks like this should be split into a few patches.
> - one patch for the MemEncryptSevLiveMigrationIsEnabled() API
> - one patch for the SetMemoryEncDecHypercall3() API
> - one patch to make use of the SetMemoryEncDecHypercall3() API.
>
Ok.
> >
> > 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/Include/Library/MemEncryptSevLib.h | 69 ++++++++++++++++++++
> > OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf | 1 +
> > OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c | 39 +++++++++++
> > OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c | 27 ++++++++
> > OvmfPkg/Library/BaseMemEncryptSevLib/PeiDxeMemEncryptSevLibInternal.c | 51 +++++++++++++++
> > OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf | 1 +
> > OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c | 39 +++++++++++
> > OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c | 38 +++++++++++
> > OvmfPkg/Library/BaseMemEncryptSevLib/X64/AsmHelperStub.nasm | 33 ++++++++++
> > OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c | 54 +++++++++++++++
> > OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c | 22 ++++++-
> > 11 files changed, 373 insertions(+), 1 deletion(-)
> >
> > diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h b/OvmfPkg/Include/Library/MemEncryptSevLib.h
> > index 76d06c206c..c2b2a99a08 100644
> > --- a/OvmfPkg/Include/Library/MemEncryptSevLib.h
> > +++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
> > @@ -90,6 +90,18 @@ MemEncryptSevIsEnabled (
> > VOID
> > );
> >
> > +/**
> > + Returns a boolean to indicate whether SEV live migration is enabled.
> > +
> > + @retval TRUE SEV live migration is enabled
> > + @retval FALSE SEV live migration is not enabled
> > +**/
> > +BOOLEAN
> > +EFIAPI
> > +MemEncryptSevLiveMigrationIsEnabled (
> > + VOID
> > + );
> > +
> > /**
> > This function clears memory encryption bit for the memory region specified by
> > BaseAddress and NumPages from the current page table context.
> > @@ -222,4 +234,61 @@ MemEncryptSevClearMmioPageEncMask (
> > IN UINTN NumPages
> > );
> >
> > +/**
> > + 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. The PhysicalAddress is
> > + expected to be PAGE_SIZE aligned.
> > + @param[in] Pages Number of pages in memory region.
> > + @param[in] Status Encrypted(1) or Decrypted(0).
> > +
> > +@retval RETURN_SUCCESS Hypercall returned success.
>
> It looks like RETURN_UNSUPPORTED is also possible.
>
> > +**/
> > +RETURN_STATUS
> > +EFIAPI
> > +SetMemoryEncDecHypercall3 (
> > + IN UINTN PhysicalAddress,
> > + IN UINTN Pages,
> > + IN UINTN Status
> > + );
> > +
> > +#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)
> > +#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)
>
> You define these but don't use them (and you should).
>
Used later in another patch.
> > +
> > +#define KVM_FEATURE_MIGRATION_CONTROL BIT17
> > +
> > +/**
> > + Figures out if we are running inside KVM HVM and
> > + KVM HVM supports SEV Live Migration feature.
> > +
> > + @retval TRUE SEV live migration is supported.
> > + @retval FALSE SEV live migration is not supported.
> > +**/
> > +BOOLEAN
> > +EFIAPI
> > +KvmDetectSevLiveMigrationFeature(
> > + VOID
> > + );
> > +
> > +/**
> > + 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
> > + );
> > +
> > #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/DxeMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c
> > index 2816f859a0..ead754cd7b 100644
> > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c
> > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c
> > @@ -20,6 +20,8 @@
> > STATIC BOOLEAN mSevStatus = FALSE;
> > STATIC BOOLEAN mSevEsStatus = FALSE;
> > STATIC BOOLEAN mSevStatusChecked = FALSE;
> > +STATIC BOOLEAN mSevLiveMigrationStatus = FALSE;
> > +STATIC BOOLEAN mSevLiveMigrationStatusChecked = FALSE;
> >
> > STATIC UINT64 mSevEncryptionMask = 0;
> > STATIC BOOLEAN mSevEncryptionMaskSaved = FALSE;
> > @@ -87,6 +89,24 @@ InternalMemEncryptSevStatus (
> > mSevStatusChecked = TRUE;
> > }
> >
> > +/**
> > + Figures out if we are running inside KVM HVM and
> > + KVM HVM supports SEV Live Migration feature.
> > +**/
> > +STATIC
> > +VOID
> > +EFIAPI
> > +InternalDetectSevLiveMigrationFeature(
> > + VOID
> > + )
> > +{
> > + if (KvmDetectSevLiveMigrationFeature()) {
> > + mSevLiveMigrationStatus = TRUE;
> > + }
> > +
> > + mSevLiveMigrationStatusChecked = TRUE;
> > +}
> > +
> > /**
> > Returns a boolean to indicate whether SEV-ES is enabled.
> >
> > @@ -125,6 +145,25 @@ MemEncryptSevIsEnabled (
> > return mSevStatus;
> > }
> >
> > +/**
> > + Returns a boolean to indicate whether SEV live migration is enabled.
> > +
> > + @retval TRUE SEV live migration is enabled
> > + @retval FALSE SEV live migration is not enabled
> > +**/
> > +BOOLEAN
> > +EFIAPI
> > +MemEncryptSevLiveMigrationIsEnabled (
> > + VOID
> > + )
> > +{
> > + if (!mSevLiveMigrationStatusChecked) {
> > + InternalDetectSevLiveMigrationFeature ();
> > + }
> > +
> > + return mSevLiveMigrationStatus;
> > +}
> > +
> > /**
> > Returns the SEV encryption mask.
> >
> > diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
> > index be260e0d10..62392309fe 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. The physical address is
> > + expected to be PAGE_SIZE aligned.
> > + @param[in] Pages Number of Pages in the memory region.
> > + @param[in] Status Encrypted(1) or Decrypted(0).
> > +
> > +@retval RETURN_SUCCESS Hypercall returned success.
> > +**/
> > +RETURN_STATUS
> > +EFIAPI
> > +SetMemoryEncDecHypercall3 (
> > + IN UINTN PhysicalAddress,
> > + IN UINTN Pages,
> > + IN UINTN Status
> > + )
> > +{
> > + //
> > + // Memory encryption bit is not accessible in 32-bit mode
> > + //
> > + return RETURN_UNSUPPORTED;
> > +}
> > +
> > diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiDxeMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiDxeMemEncryptSevLibInternal.c
> > index b4a9f464e2..0c9f7e17ba 100644
> > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiDxeMemEncryptSevLibInternal.c
> > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiDxeMemEncryptSevLibInternal.c
> > @@ -61,3 +61,54 @@ MemEncryptSevLocateInitialSmramSaveStateMapPages (
> >
> > return RETURN_SUCCESS;
> > }
> > +
> > +/**
> > + Figures out if we are running inside KVM HVM and
> > + KVM HVM supports SEV Live Migration feature.
> > +
> > + @retval TRUE SEV live migration is supported.
> > + @retval FALSE SEV live migration is not supported.
> > +**/
> > +BOOLEAN
> > +EFIAPI
> > +KvmDetectSevLiveMigrationFeature(
> > + VOID
> > + )
> > +{
> > + CHAR8 Signature[13];
> > + UINT32 mKvmLeaf;
> > + UINT32 RegEax, RegEbx, RegEcx, RegEdx;
> > +
> > + Signature[12] = '\0';
> > + for (mKvmLeaf = 0x40000000; mKvmLeaf < 0x40010000; mKvmLeaf += 0x100) {
> > + AsmCpuid (mKvmLeaf,
> > + NULL,
> > + (UINT32 *) &Signature[0],
> > + (UINT32 *) &Signature[4],
> > + (UINT32 *) &Signature[8]);
> > +
> > + if (AsciiStrCmp ((CHAR8 *) Signature, "KVMKVMKVM\0\0\0") == 0) {
> > + DEBUG ((
> > + DEBUG_INFO,
> > + "%a: KVM Detected, signature = %s\n",
> > + __FUNCTION__,
> > + Signature
> > + ));
> > +
> > + RegEax = mKvmLeaf + 1;
> > + RegEcx = 0;
> > + AsmCpuid (mKvmLeaf + 1, &RegEax, &RegEbx, &RegEcx, &RegEdx);
> > + if ((RegEax & KVM_FEATURE_MIGRATION_CONTROL) != 0) {
> > + DEBUG ((
> > + DEBUG_INFO,
> > + "%a: Live Migration feature supported\n",
> > + __FUNCTION__
> > + ));
> > +
> > + return TRUE;
> > + }
> > + }
> > + }
> > +
> > + return FALSE;
> > +}
> > 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/PeiMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
> > index e2fd109d12..9db6c2ef71 100644
> > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
> > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
> > @@ -20,6 +20,8 @@
> > STATIC BOOLEAN mSevStatus = FALSE;
> > STATIC BOOLEAN mSevEsStatus = FALSE;
> > STATIC BOOLEAN mSevStatusChecked = FALSE;
> > +STATIC BOOLEAN mSevLiveMigrationStatus = FALSE;
> > +STATIC BOOLEAN mSevLiveMigrationStatusChecked = FALSE;
> >
> > STATIC UINT64 mSevEncryptionMask = 0;
> > STATIC BOOLEAN mSevEncryptionMaskSaved = FALSE;
> > @@ -87,6 +89,24 @@ InternalMemEncryptSevStatus (
> > mSevStatusChecked = TRUE;
> > }
> >
> > +/**
> > + Figures out if we are running inside KVM HVM and
> > + KVM HVM supports SEV Live Migration feature.
> > +**/
> > +STATIC
> > +VOID
> > +EFIAPI
> > +InternalDetectSevLiveMigrationFeature(
> > + VOID
> > + )
> > +{
> > + if (KvmDetectSevLiveMigrationFeature()) {
> > + mSevLiveMigrationStatus = TRUE;
> > + }
> > +
> > + mSevLiveMigrationStatusChecked = TRUE;
> > +}
> > +
> > /**
> > Returns a boolean to indicate whether SEV-ES is enabled.
> >
> > @@ -125,6 +145,25 @@ MemEncryptSevIsEnabled (
> > return mSevStatus;
> > }
> >
> > +/**
> > + Returns a boolean to indicate whether SEV live migration is enabled.
> > +
> > + @retval TRUE SEV live migration is enabled
> > + @retval FALSE SEV live migration is not enabled
> > +**/
> > +BOOLEAN
> > +EFIAPI
> > +MemEncryptSevLiveMigrationIsEnabled (
> > + VOID
> > + )
> > +{
> > + if (!mSevLiveMigrationStatusChecked) {
> > + InternalDetectSevLiveMigrationFeature ();
> > + }
> > +
> > + return mSevLiveMigrationStatus;
> > +}
> > +
> > /**
> > Returns the SEV encryption mask.
> >
> > diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
> > index 56d8f3f318..b926c7b786 100644
> > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
> > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
> > @@ -100,6 +100,44 @@ MemEncryptSevIsEnabled (
> > return Msr.Bits.SevBit ? TRUE : 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 a boolean to indicate whether SEV live migration is enabled.
> > +
> > + @retval TRUE SEV live migration is enabled
> > + @retval FALSE SEV live migration is not enabled
> > +**/
> > +BOOLEAN
> > +EFIAPI
> > +MemEncryptSevLiveMigrationIsEnabled (
> > + VOID
> > + )
> > +{
> > + //
> > + // Not used in SEC phase.
> > + //
> > + return FALSE;
> > +}
> > +
> > /**
> > 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..c7c11f77f1
> > --- /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 Arg2 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..57447e69dc 100644
> > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
> > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
> > @@ -143,3 +143,57 @@ 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. The physical address is
> > + expected to be PAGE_SIZE aligned.
> > + @param[in] Pages Number of Pages in the memory region.
> > + @param[in] Status Encrypted(1) or Decrypted(0).
> > +
> > +@retval RETURN_SUCCESS Hypercall returned success.
>
> I see RETURN_NO_MAPPING also, so you'll need to update the retvals everywhere.
>
Yes.
> > +**/
> > +RETURN_STATUS
> > +EFIAPI
> > +SetMemoryEncDecHypercall3 (
> > + IN UINTN PhysicalAddress,
> > + IN UINTN Pages,
> > + IN UINTN Status
> > + )
> > +{
> > + RETURN_STATUS Ret;
> > + INTN Error;
>
> Should be UINTN.
>
> > +
> > + Ret = RETURN_UNSUPPORTED;
> > +
> > + if (MemEncryptSevLiveMigrationIsEnabled ()) {
> > + Ret = EFI_SUCCESS;
>
> RETURN_SUCCESS since Ret is type RETURN_STATUS.
Ok.
> > + //
> > + // The encryption bit is set/clear on the smallest page size, hence
> > + // use the 4k page size in MAP_GPA_RANGE hypercall below.
> > + // Also, the hypercall expects the guest physical address to be
> > + // page-aligned.
> > + //
> > + Error = SetMemoryEncDecHypercall3AsmStub (
> > + KVM_HC_MAP_GPA_RANGE,
> > + (PhysicalAddress & (~(EFI_PAGE_SIZE-1))),
> > + Pages,
> > + KVM_MAP_GPA_RANGE_PAGE_SZ_4K | KVM_MAP_GPA_RANGE_ENC_STAT(Status)
>
> Status is UINTN, but is passed from an enum variable. If for any reason
> that enum should change in the future, this may break. So you should fixup
> your call to explicitly pass 0 or 1 and then you can safely use that value
> here.
>
> Maybe add an "ASSERT (Status == 0 || Status == 1)" to catch bad input values.
>
Ok.
> > + );
> > +
> > + if (Error != 0) {
> > + DEBUG ((DEBUG_ERROR,
> > + "SetMemoryEncDecHypercall3 failed, Phys = %Lx, Pages = %Ld, Err = %Ld\n",
> > + PhysicalAddress,
> > + Pages,
> > + (INT64)Error));
> > +
> > + Ret = RETURN_NO_MAPPING;
> > + }
> > + }
> > +
> > + return Ret;
> > +}
> > diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> > index c696745f9d..0b1588a4c1 100644
> > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> > @@ -536,7 +536,6 @@ EnableReadOnlyPageWriteProtect (
> > AsmWriteCr0 (AsmReadCr0() | BIT16);
> > }
> >
> > -
> > /**
> > This function either sets or clears memory encryption bit for the memory
> > region specified by PhysicalAddress and Length from the current page table
> > @@ -585,6 +584,9 @@ SetMemoryEncDec (
> > UINT64 AddressEncMask;
> > BOOLEAN IsWpEnabled;
> > RETURN_STATUS Status;
> > + UINTN Size;
> > + BOOLEAN CBitChanged;
> > + PHYSICAL_ADDRESS OrigPhysicalAddress;
> >
> > //
> > // Set PageMapLevel4Entry to suppress incorrect compiler/analyzer warnings.
> > @@ -636,6 +638,10 @@ SetMemoryEncDec (
> >
> > Status = EFI_SUCCESS;
> >
> > + Size = Length;
> > + CBitChanged = FALSE;
> > + OrigPhysicalAddress = PhysicalAddress;
> > +
> > while (Length != 0)
> > {
> > //
> > @@ -695,6 +701,7 @@ SetMemoryEncDec (
> > ));
> > PhysicalAddress += BIT30;
> > Length -= BIT30;
> > + CBitChanged = TRUE;
> > } else {
> > //
> > // We must split the page
> > @@ -749,6 +756,7 @@ SetMemoryEncDec (
> > SetOrClearCBit (&PageDirectory2MEntry->Uint64, Mode);
> > PhysicalAddress += BIT21;
> > Length -= BIT21;
> > + CBitChanged = TRUE;
> > } else {
> > //
> > // We must split up this page into 4K pages
> > @@ -791,6 +799,7 @@ SetMemoryEncDec (
> > SetOrClearCBit (&PageTableEntry->Uint64, Mode);
> > PhysicalAddress += EFI_PAGE_SIZE;
> > Length -= EFI_PAGE_SIZE;
> > + CBitChanged = TRUE;
> > }
> > }
> > }
> > @@ -808,6 +817,17 @@ SetMemoryEncDec (
> > //
> > CpuFlushTlb();
> >
> > + //
> > + // Notify Hypervisor on C-bit status
> > + //
> > + if (CBitChanged) {
> > + Status = SetMemoryEncDecHypercall3 (
> > + OrigPhysicalAddress,
> > + EFI_SIZE_TO_PAGES(Size),
> > + !Mode
>
> "Mode" is a MAP_RANGE_MODE enum that is local to this file. So you need to
> either move this to a common header file so you can use it with
> SetMemoryEncDecHypercall3() or set a 0 or 1 based on Mode and pass that.
>
>
Ok.
Thanks,
Ashish
> > + );
> > + }
> > +
> > Done:
> > //
> > // Restore page table write protection, if any.
> >
next prev parent reply other threads:[~2021-07-19 20:24 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-08 14:07 [PATCH v5 0/4] SEV Live Migration support for OVMF Ashish Kalra
2021-07-08 14:07 ` [PATCH v5 1/4] OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall Ashish Kalra
2021-07-15 20:58 ` Dov Murik
2021-07-16 12:29 ` Ashish Kalra
2021-07-19 8:04 ` Dov Murik
2021-07-19 15:30 ` Ashish Kalra
2021-07-16 14:11 ` Lendacky, Thomas
2021-07-19 20:24 ` Ashish Kalra [this message]
2021-07-08 14:08 ` [PATCH v5 2/4] OvmfPkg/VmgExitLib: Add support for hypercalls with SEV-ES Ashish Kalra
2021-07-16 14:16 ` Lendacky, Thomas
2021-07-19 20:25 ` Ashish Kalra
2021-07-08 14:08 ` [PATCH v5 3/4] OvmfPkg/PlatformPei: Mark SEC GHCB page as unencrypted via hypercall Ashish Kalra
2021-07-16 14:22 ` Lendacky, Thomas
2021-07-19 20:27 ` Ashish Kalra
2021-07-08 14:09 ` [PATCH v5 4/4] OvmfPkg/AmdSevDxe: Add support for SEV live migration Ashish Kalra
2021-07-19 7:31 ` Dov Murik
2021-07-19 15:22 ` Ashish Kalra
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=20210719202416.GA24481@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