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

[Lendacky, Thomas via groups.io]

On 6/21/21 8:57 AM, Ashish Kalra wrote:
> From: Brijesh Singh <brijesh.singh at amd.com>
> 
> 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.
> 
> Invoke hypercall via the new hypercall library.
> 
> This hypercall is used to notify hypervisor when a page is marked as
> 'decrypted' (i.e C-bit removed).
> 
> 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/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf   |  1 +
>  OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf   |  1 +
>  OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c | 22 ++++++++++++++++++++
>  3 files changed, 24 insertions(+)
> 
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
> index f2e162d680..aefcd7c0f7 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
> @@ -49,6 +49,7 @@
>    DebugLib
>    MemoryAllocationLib
>    PcdLib
> +  MemEncryptHypercallLib
>  
>  [FeaturePcd]
>    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
> index 03a78c32df..7503f56a0b 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
> @@ -49,6 +49,7 @@
>    DebugLib
>    MemoryAllocationLib
>    PcdLib
> +  MemEncryptHypercallLib
>  
>  [FeaturePcd]
>    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> index c696745f9d..12b3a9fcfb 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> @@ -15,6 +15,7 @@
>  #include <Library/MemEncryptSevLib.h>
>  #include <Register/Amd/Cpuid.h>
>  #include <Register/Cpuid.h>
> +#include <Library/MemEncryptHypercallLib.h>
>  
>  #include "VirtualMemory.h"
>  
> @@ -585,6 +586,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 +640,10 @@ SetMemoryEncDec (
>  
>    Status = EFI_SUCCESS;
>  
> +  Size = Length;
> +  CBitChanged = FALSE;
> +  OrigPhysicalAddress = PhysicalAddress;
> +
>    while (Length != 0)
>    {
>      //
> @@ -695,6 +703,7 @@ SetMemoryEncDec (
>            ));
>          PhysicalAddress += BIT30;
>          Length -= BIT30;
> +        CBitChanged = TRUE;
>        } else {
>          //
>          // We must split the page
> @@ -749,6 +758,7 @@ SetMemoryEncDec (
>            SetOrClearCBit (&PageDirectory2MEntry->Uint64, Mode);
>            PhysicalAddress += BIT21;
>            Length -= BIT21;
> +          CBitChanged = TRUE;
>          } else {
>            //
>            // We must split up this page into 4K pages
> @@ -791,6 +801,7 @@ SetMemoryEncDec (
>          SetOrClearCBit (&PageTableEntry->Uint64, Mode);
>          PhysicalAddress += EFI_PAGE_SIZE;
>          Length -= EFI_PAGE_SIZE;
> +        CBitChanged = TRUE;
>        }
>      }
>    }
> @@ -808,6 +819,17 @@ SetMemoryEncDec (
>    //
>    CpuFlushTlb();
>  
> +  //
> +  // Notify Hypervisor on C-bit status
> +  //
> +  if (CBitChanged) {
> +    SetMemoryEncDecHypercall3 (
> +      OrigPhysicalAddress,
> +      EFI_SIZE_TO_PAGES(Size),
> +      KVM_MAP_GPA_RANGE_ENC_STAT(!Mode)
> +      );
> +  }

Maybe add a comment here that the "Mode" doesn't care whether the change
was for 4k vs 2mb vs 1gb. It is just tracking the total number of pages
changed (should you just remove those #defines for the page sizes then?)
which can be done by calculating everything at the 4k level.

Thanks,
Tom

> +
>  Done:
>    //
>    // Restore page table write protection, if any.
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#76868): https://edk2.groups.io/g/devel/message/76868
Mute This Topic: https://groups.io/mt/83688883/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-