[edk2-devel] [PATCH v5 1/4] OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall

Ashish Kalra via groups.io ashish.kalra=amd.com at groups.io
Mon Jul 19 20:24:16 UTC 2021


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 at 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 at intel.com>
> > Cc: Laszlo Ersek <lersek at redhat.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel at arm.com>
> > Signed-off-by: Brijesh Singh <brijesh.singh at amd.com>
> > Signed-off-by: Ashish Kalra <ashish.kalra at 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).
> > +
> > + at 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).
> > +
> > + at 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).
> > +
> > + at 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.
> > 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77925): https://edk2.groups.io/g/devel/message/77925
Mute This Topic: https://groups.io/mt/84068331/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