Re: [edk2-devel] [PATCH v3 11/15] OvmfPkg/MemEncryptSevLib: Make the MemEncryptSevLib available for SEC

["Laszlo Ersek" <lersek@redhat.com>]

On 01/07/21 19:48, Lendacky, Thomas wrote:
> From: Tom Lendacky <thomas.lendacky@amd.com>
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3108
> 
> In preparation for a new interface to be added to the MemEncryptSevLib
> library that will be used in SEC, create an SEC version of the library.
> 
> This requires the creation of SEC specific files.
> 
> Some of the current MemEncryptSevLib functions perform memory allocations
> which cannot be performed in SEC, so these interfaces will return an error
> during SEC. Also, the current MemEncryptSevLib library uses some static
> variables to optimize access to variables, which cannot be used in SEC.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Acked-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf                                          |   4 +-
>  OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf                                          |   4 +-
>  OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf                                          |  50 +++++++
>  OvmfPkg/Library/BaseMemEncryptSevLib/{MemEncryptSevLibInternal.c => PeiDxeMemEncryptSevLibInternal.c} |   0
>  OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c                                    | 155 ++++++++++++++++++++
>  OvmfPkg/Library/BaseMemEncryptSevLib/X64/{VirtualMemory.c => PeiDxeVirtualMemory.c}                   |   0
>  OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecVirtualMemory.c                                           |  80 ++++++++++
>  7 files changed, 289 insertions(+), 4 deletions(-)
> 
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
> index 837db0876184..4480e4cc7c89 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
> @@ -31,11 +31,11 @@ [Packages]
>  
>  [Sources]
>    DxeMemEncryptSevLibInternal.c
> -  MemEncryptSevLibInternal.c
> +  PeiDxeMemEncryptSevLibInternal.c
>  
>  [Sources.X64]
>    X64/MemEncryptSevLib.c
> -  X64/VirtualMemory.c
> +  X64/PeiDxeVirtualMemory.c
>    X64/VirtualMemory.h
>  
>  [Sources.IA32]
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
> index c3cd046cb630..0697f1dab502 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
> @@ -30,12 +30,12 @@ [Packages]
>    UefiCpuPkg/UefiCpuPkg.dec
>  
>  [Sources]
> -  MemEncryptSevLibInternal.c
> +  PeiDxeMemEncryptSevLibInternal.c
>    PeiMemEncryptSevLibInternal.c

Yes, this is where I expected a need to resolve a conflict due to the
earlier sorting order correction; that's why I preferred a v3 from you :)

Thanks
Laszlo

>  
>  [Sources.X64]
>    X64/MemEncryptSevLib.c
> -  X64/VirtualMemory.c
> +  X64/PeiDxeVirtualMemory.c
>    X64/VirtualMemory.h
>  
>  [Sources.IA32]
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
> new file mode 100644
> index 000000000000..7cd0111fe47b
> --- /dev/null
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
> @@ -0,0 +1,50 @@
> +## @file
> +#  Library provides the helper functions for SEV guest
> +#
> +# Copyright (c) 2020 Advanced Micro Devices. All rights reserved.<BR>
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 1.25
> +  BASE_NAME                      = SecMemEncryptSevLib
> +  FILE_GUID                      = 046388b4-430e-4e61-88f6-51ea21db2632
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = MemEncryptSevLib|SEC
> +
> +#
> +# The following information is for reference only and not required by the build
> +# tools.
> +#
> +# VALID_ARCHITECTURES           = IA32 X64
> +#
> +
> +[Packages]
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  OvmfPkg/OvmfPkg.dec
> +  UefiCpuPkg/UefiCpuPkg.dec
> +
> +[Sources]
> +  SecMemEncryptSevLibInternal.c
> +
> +[Sources.X64]
> +  X64/MemEncryptSevLib.c
> +  X64/SecVirtualMemory.c
> +  X64/VirtualMemory.h
> +
> +[Sources.IA32]
> +  Ia32/MemEncryptSevLib.c
> +
> +[LibraryClasses]
> +  BaseLib
> +  CpuLib
> +  DebugLib
> +  PcdLib
> +
> +[FixedPcd]
> +  gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/MemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiDxeMemEncryptSevLibInternal.c
> similarity index 100%
> rename from OvmfPkg/Library/BaseMemEncryptSevLib/MemEncryptSevLibInternal.c
> rename to OvmfPkg/Library/BaseMemEncryptSevLib/PeiDxeMemEncryptSevLibInternal.c
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
> new file mode 100644
> index 000000000000..56d8f3f3183f
> --- /dev/null
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
> @@ -0,0 +1,155 @@
> +/** @file
> +
> +  Secure Encrypted Virtualization (SEV) library helper function
> +
> +  Copyright (c) 2020, Advanced Micro Devices, Inc. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MemEncryptSevLib.h>
> +#include <Library/PcdLib.h>
> +#include <Register/Amd/Cpuid.h>
> +#include <Register/Amd/Msr.h>
> +#include <Register/Cpuid.h>
> +#include <Uefi/UefiBaseType.h>
> +
> +/**
> +  Reads and sets the status of SEV features.
> +
> +  **/
> +STATIC
> +UINT32
> +EFIAPI
> +InternalMemEncryptSevStatus (
> +  VOID
> +  )
> +{
> +  UINT32                            RegEax;
> +  CPUID_MEMORY_ENCRYPTION_INFO_EAX  Eax;
> +  BOOLEAN                           ReadSevMsr;
> +  SEC_SEV_ES_WORK_AREA              *SevEsWorkArea;
> +
> +  ReadSevMsr = FALSE;
> +
> +  SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32 (PcdSevEsWorkAreaBase);
> +  if (SevEsWorkArea != NULL && SevEsWorkArea->EncryptionMask != 0) {
> +    //
> +    // The MSR has been read before, so it is safe to read it again and avoid
> +    // having to validate the CPUID information.
> +    //
> +    ReadSevMsr = TRUE;
> +  } else {
> +    //
> +    // Check if memory encryption leaf exist
> +    //
> +    AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
> +    if (RegEax >= CPUID_MEMORY_ENCRYPTION_INFO) {
> +      //
> +      // CPUID Fn8000_001F[EAX] Bit 1 (Sev supported)
> +      //
> +      AsmCpuid (CPUID_MEMORY_ENCRYPTION_INFO, &Eax.Uint32, NULL, NULL, NULL);
> +
> +      if (Eax.Bits.SevBit) {
> +        ReadSevMsr = TRUE;
> +      }
> +    }
> +  }
> +
> +  return ReadSevMsr ? AsmReadMsr32 (MSR_SEV_STATUS) : 0;
> +}
> +
> +/**
> +  Returns a boolean to indicate whether SEV-ES is enabled.
> +
> +  @retval TRUE           SEV-ES is enabled
> +  @retval FALSE          SEV-ES is not enabled
> +**/
> +BOOLEAN
> +EFIAPI
> +MemEncryptSevEsIsEnabled (
> +  VOID
> +  )
> +{
> +  MSR_SEV_STATUS_REGISTER           Msr;
> +
> +  Msr.Uint32 = InternalMemEncryptSevStatus ();
> +
> +  return Msr.Bits.SevEsBit ? TRUE : FALSE;
> +}
> +
> +/**
> +  Returns a boolean to indicate whether SEV is enabled.
> +
> +  @retval TRUE           SEV is enabled
> +  @retval FALSE          SEV is not enabled
> +**/
> +BOOLEAN
> +EFIAPI
> +MemEncryptSevIsEnabled (
> +  VOID
> +  )
> +{
> +  MSR_SEV_STATUS_REGISTER           Msr;
> +
> +  Msr.Uint32 = InternalMemEncryptSevStatus ();
> +
> +  return Msr.Bits.SevBit ? TRUE : FALSE;
> +}
> +
> +/**
> +  Returns the SEV encryption mask.
> +
> +  @return  The SEV pagtable encryption mask
> +**/
> +UINT64
> +EFIAPI
> +MemEncryptSevGetEncryptionMask (
> +  VOID
> +  )
> +{
> +  CPUID_MEMORY_ENCRYPTION_INFO_EBX  Ebx;
> +  SEC_SEV_ES_WORK_AREA              *SevEsWorkArea;
> +  UINT64                            EncryptionMask;
> +
> +  SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32 (PcdSevEsWorkAreaBase);
> +  if (SevEsWorkArea != NULL) {
> +    EncryptionMask = SevEsWorkArea->EncryptionMask;
> +  } else {
> +    //
> +    // CPUID Fn8000_001F[EBX] Bit 0:5 (memory encryption bit position)
> +    //
> +    AsmCpuid (CPUID_MEMORY_ENCRYPTION_INFO, NULL, &Ebx.Uint32, NULL, NULL);
> +    EncryptionMask = LShiftU64 (1, Ebx.Bits.PtePosBits);
> +  }
> +
> +  return EncryptionMask;
> +}
> +
> +/**
> +  Locate the page range that covers the initial (pre-SMBASE-relocation) SMRAM
> +  Save State Map.
> +
> +  @param[out] BaseAddress     The base address of the lowest-address page that
> +                              covers the initial SMRAM Save State Map.
> +
> +  @param[out] NumberOfPages   The number of pages in the page range that covers
> +                              the initial SMRAM Save State Map.
> +
> +  @retval RETURN_SUCCESS      BaseAddress and NumberOfPages have been set on
> +                              output.
> +
> +  @retval RETURN_UNSUPPORTED  SMM is unavailable.
> +**/
> +RETURN_STATUS
> +EFIAPI
> +MemEncryptSevLocateInitialSmramSaveStateMapPages (
> +  OUT UINTN *BaseAddress,
> +  OUT UINTN *NumberOfPages
> +  )
> +{
> +  return RETURN_UNSUPPORTED;
> +}
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> similarity index 100%
> rename from OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c
> rename to OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecVirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecVirtualMemory.c
> new file mode 100644
> index 000000000000..5c337ea0b820
> --- /dev/null
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecVirtualMemory.c
> @@ -0,0 +1,80 @@
> +/** @file
> +
> +  Virtual Memory Management Services to set or clear the memory encryption bit
> +
> +  Copyright (c) 2020, AMD Incorporated. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Library/CpuLib.h>
> +#include <Library/MemEncryptSevLib.h>
> +
> +#include "VirtualMemory.h"
> +
> +/**
> +  This function clears memory encryption bit for the memory region specified by
> +  PhysicalAddress and Length from the current page table context.
> +
> +  @param[in]  Cr3BaseAddress          Cr3 Base Address (if zero then use
> +                                      current CR3)
> +  @param[in]  PhysicalAddress         The physical address that is the start
> +                                      address of a memory region.
> +  @param[in]  Length                  The length of memory region
> +  @param[in]  Flush                   Flush the caches before applying the
> +                                      encryption mask
> +
> +  @retval RETURN_SUCCESS              The attributes were cleared for the
> +                                      memory region.
> +  @retval RETURN_INVALID_PARAMETER    Number of pages is zero.
> +  @retval RETURN_UNSUPPORTED          Clearing the memory encyrption attribute
> +                                      is not supported
> +**/
> +RETURN_STATUS
> +EFIAPI
> +InternalMemEncryptSevSetMemoryDecrypted (
> +  IN  PHYSICAL_ADDRESS        Cr3BaseAddress,
> +  IN  PHYSICAL_ADDRESS        PhysicalAddress,
> +  IN  UINTN                   Length,
> +  IN  BOOLEAN                 Flush
> +  )
> +{
> +  //
> +  // This function is not available during SEC.
> +  //
> +  return RETURN_UNSUPPORTED;
> +}
> +
> +/**
> +  This function sets memory encryption bit for the memory region specified by
> +  PhysicalAddress and Length from the current page table context.
> +
> +  @param[in]  Cr3BaseAddress          Cr3 Base Address (if zero then use
> +                                      current CR3)
> +  @param[in]  PhysicalAddress         The physical address that is the start
> +                                      address of a memory region.
> +  @param[in]  Length                  The length of memory region
> +  @param[in]  Flush                   Flush the caches before applying the
> +                                      encryption mask
> +
> +  @retval RETURN_SUCCESS              The attributes were set for the memory
> +                                      region.
> +  @retval RETURN_INVALID_PARAMETER    Number of pages is zero.
> +  @retval RETURN_UNSUPPORTED          Setting the memory encyrption attribute
> +                                      is not supported
> +**/
> +RETURN_STATUS
> +EFIAPI
> +InternalMemEncryptSevSetMemoryEncrypted (
> +  IN  PHYSICAL_ADDRESS        Cr3BaseAddress,
> +  IN  PHYSICAL_ADDRESS        PhysicalAddress,
> +  IN  UINTN                   Length,
> +  IN  BOOLEAN                 Flush
> +  )
> +{
> +  //
> +  // This function is not available during SEC.
> +  //
> +  return RETURN_UNSUPPORTED;
> +}
>