[edk2-devel] [PATCH 08/12] OvmfPkg/MemEncryptSevLib: Make the MemEncryptSevLib available for SEC
Laszlo Ersek
lersek at redhat.com
Tue Jan 5 09:40:51 UTC 2021
On 12/15/20 21:51, Lendacky, Thomas wrote:
> From: Tom Lendacky <thomas.lendacky at 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 at intel.com>
> Cc: Laszlo Ersek <lersek at redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel at arm.com>
> Cc: Brijesh Singh <brijesh.singh at amd.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky at amd.com>
> ---
> .../DxeBaseMemEncryptSevLib.inf | 2 +-
> .../PeiBaseMemEncryptSevLib.inf | 2 +-
> .../SecBaseMemEncryptSevLib.inf | 54 ++++++++
> .../SecMemEncryptSevLibInternal.c | 130 ++++++++++++++++++
> ...{VirtualMemory.c => PeiDxeVirtualMemory.c} | 12 +-
> .../X64/SecVirtualMemory.c | 80 +++++++++++
> 6 files changed, 272 insertions(+), 8 deletions(-)
> create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/SecBaseMemEncryptSevLib.inf
> create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
> rename OvmfPkg/Library/BaseMemEncryptSevLib/X64/{VirtualMemory.c => PeiDxeVirtualMemory.c} (95%)
> create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecVirtualMemory.c
(1) /s/SecBase/Sec/ (in filenames and in filename references; the
BASE_NAME is OK)
>
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeBaseMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeBaseMemEncryptSevLib.inf
> index 2be6ca1fa737..390f2d60677f 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeBaseMemEncryptSevLib.inf
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeBaseMemEncryptSevLib.inf
> @@ -33,7 +33,7 @@ [Sources.X64]
> DxeMemEncryptSevLibInternal.c
> MemEncryptSevLibInternal.c
> X64/MemEncryptSevLib.c
> - X64/VirtualMemory.c
> + X64/PeiDxeVirtualMemory.c
> X64/VirtualMemory.h
>
> [Sources.IA32]
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiBaseMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiBaseMemEncryptSevLib.inf
> index 7bdf8cb5210d..cb973fdeb868 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiBaseMemEncryptSevLib.inf
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiBaseMemEncryptSevLib.inf
> @@ -33,7 +33,7 @@ [Sources.X64]
> PeiMemEncryptSevLibInternal.c
> MemEncryptSevLibInternal.c
> X64/MemEncryptSevLib.c
> - X64/VirtualMemory.c
> + X64/PeiDxeVirtualMemory.c
> X64/VirtualMemory.h
>
> [Sources.IA32]
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/SecBaseMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/SecBaseMemEncryptSevLib.inf
> new file mode 100644
> index 000000000000..b26f739d69fd
> --- /dev/null
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SecBaseMemEncryptSevLib.inf
> @@ -0,0 +1,54 @@
> +## @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.X64]
> + SecMemEncryptSevLibInternal.c
> + MemEncryptSevLibInternal.c
> + X64/MemEncryptSevLib.c
> + X64/SecVirtualMemory.c
> + X64/VirtualMemory.h
> +
> +[Sources.IA32]
> + SecMemEncryptSevLibInternal.c
> + MemEncryptSevLibInternal.c
> + Ia32/MemEncryptSevLib.c
> +
> +[LibraryClasses]
> + BaseLib
> + CpuLib
> + DebugLib
> + PcdLib
> +
> +[FeaturePcd]
> + gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
> +
(2) This PCD does not look useful for the new library instance (at least
at this stage).
> +[FixedPcd]
> + gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
> new file mode 100644
> index 000000000000..30d2ebe1d6e9
> --- /dev/null
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
> @@ -0,0 +1,130 @@
> +/** @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;
> +}
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> similarity index 95%
> rename from OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c
> rename to OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> index 6422bc53bd5d..3a5bab657bd7 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> @@ -192,7 +192,8 @@ Split2MPageTo4K (
> {
> PHYSICAL_ADDRESS PhysicalAddress4K;
> UINTN IndexOfPageTableEntries;
> - PAGE_TABLE_4K_ENTRY *PageTableEntry, *PageTableEntry1;
> + PAGE_TABLE_4K_ENTRY *PageTableEntry;
> + PAGE_TABLE_4K_ENTRY *PageTableEntry1;
> UINT64 AddressEncMask;
>
> PageTableEntry = AllocatePageTableMemory(1);
> @@ -472,7 +473,7 @@ Split1GPageTo2M (
> /**
> Set or Clear the memory encryption bit
>
> - @param[in] PagetablePoint Page table entry pointer (PTE).
> + @param[in, out] PageTablePointer Page table entry pointer (PTE).
> @param[in] Mode Set or Clear encryption bit
>
> **/
> @@ -562,7 +563,6 @@ EnableReadOnlyPageWriteProtect (
> @retval RETURN_UNSUPPORTED Setting the memory encyrption attribute
> is not supported
> **/
> -
> STATIC
> RETURN_STATUS
> EFIAPI
> @@ -635,7 +635,7 @@ SetMemoryEncDec (
>
> Status = EFI_SUCCESS;
>
> - while (Length)
> + while (Length != 0)
> {
> //
> // If Cr3BaseAddress is not specified then read the current CR3
> @@ -683,7 +683,7 @@ SetMemoryEncDec (
> // Valid 1GB page
> // If we have at least 1GB to go, we can just update this entry
> //
> - if (!(PhysicalAddress & (BIT30 - 1)) && Length >= BIT30) {
> + if ((PhysicalAddress & (BIT30 - 1)) == 0 && Length >= BIT30) {
> SetOrClearCBit(&PageDirectory1GEntry->Uint64, Mode);
> DEBUG ((
> DEBUG_VERBOSE,
> @@ -744,7 +744,7 @@ SetMemoryEncDec (
> // Valid 2MB page
> // If we have at least 2MB left to go, we can just update this entry
> //
> - if (!(PhysicalAddress & (BIT21-1)) && Length >= BIT21) {
> + if ((PhysicalAddress & (BIT21-1)) == 0 && Length >= BIT21) {
> SetOrClearCBit (&PageDirectory2MEntry->Uint64, Mode);
> PhysicalAddress += BIT21;
> Length -= BIT21;
(3) The style fixes in this file seem unrelated to the subject. Please
split them to a different patch.
(Were they motivated by ECC?)
Looks OK otherwise.
Thanks!
Laszlo
> 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;
> +}
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69686): https://edk2.groups.io/g/devel/message/69686
Mute This Topic: https://groups.io/mt/78986172/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