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>
Signed-off-by: Tom Lendacky <thomas.lendacky@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
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
+
+[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;
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;
+}
--
2.28.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#68908): https://edk2.groups.io/g/devel/message/68908
Mute This Topic: https://groups.io/mt/78986172/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 12/15/20 21:51, 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>
> Signed-off-by: Tom Lendacky <thomas.lendacky@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 1/5/21 3:40 AM, Laszlo Ersek wrote:
> On 12/15/20 21:51, Lendacky, Thomas wrote:
>> From: Tom Lendacky <thomas.lendacky@amd.com>
>>
>> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3108&data=04%7C01%7Cthomas.lendacky%40amd.com%7C1440a9afd7f1450ba93d08d8b15e02a5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637454364641627971%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=P52g0gS3SEhdgkF2qRY6l1J8%2FLjJm1DNR3LLlEmKSBk%3D&reserved=0
>>
>> 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>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@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)
Yup, I'll fix that.
>
>>
>> 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).
The PCD is used in MemEncryptSevLocateInitialSmramSaveStateMapPages() in
the MemEncryptSevLibInternal.c file, which is part of the library. Because
of that, I assumed that it needed to be added even though the function
that uses it isn't called during SEC.
I'll remove it.
>
>> +[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?)
Yes, IIRC (I need to swap everything back in after the holidays :)), the
test pull request was failing until I made these changes. I'll split these
changes out as a pre-patch to this patch.
Thanks,
Tom
>
> 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 (#69710): https://edk2.groups.io/g/devel/message/69710
Mute This Topic: https://groups.io/mt/78986172/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 1/5/21 8:34 AM, Tom Lendacky wrote:
> On 1/5/21 3:40 AM, Laszlo Ersek wrote:
>> On 12/15/20 21:51, Lendacky, Thomas wrote:
>>> From: Tom Lendacky <thomas.lendacky@amd.com>
>>>
>>> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3108&data=04%7C01%7Cthomas.lendacky%40amd.com%7C1440a9afd7f1450ba93d08d8b15e02a5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637454364641627971%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=P52g0gS3SEhdgkF2qRY6l1J8%2FLjJm1DNR3LLlEmKSBk%3D&reserved=0
>>>
>>> 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>
>>> Signed-off-by: Tom Lendacky <thomas.lendacky@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)
>
> Yup, I'll fix that.
>
>>
>>>
>>> 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).
>
> The PCD is used in MemEncryptSevLocateInitialSmramSaveStateMapPages() in
> the MemEncryptSevLibInternal.c file, which is part of the library. Because
> of that, I assumed that it needed to be added even though the function
> that uses it isn't called during SEC.
>
> I'll remove it.
Removing it does cause an error.
If we really don't want to include this PCD, I can create SEC and PEI/DXE
specific versions of the MemEncryptSevLibInternal.c file and just return
RETURN_UNSUPPORTED for the SEC version of
MemEncryptSevLocateInitialSmramSaveStateMapPages().
Alternatively, I can just remove the MemEncryptSevLibInternal.c file from
the build of the SEC library. This should be ok during SEC because there
are no calls to MemEncryptSevLocateInitialSmramSaveStateMapPages(). If,
for some reason a call is added later, then the build will fail, but it
should be obvious why it failed.
Or I can just leave the FeaturePcd section in the SEC inf file.
Thoughts?
Thanks,
Tom
>
>>
>>> +[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?)
>
> Yes, IIRC (I need to swap everything back in after the holidays :)), the
> test pull request was failing until I made these changes. I'll split these
> changes out as a pre-patch to this patch.
>
> Thanks,
> Tom
>
>>
>> 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 (#69718): https://edk2.groups.io/g/devel/message/69718
Mute This Topic: https://groups.io/mt/78986172/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 01/05/21 16:38, Lendacky, Thomas wrote:
> On 1/5/21 8:34 AM, Tom Lendacky wrote:
>> On 1/5/21 3:40 AM, Laszlo Ersek wrote:
>>> On 12/15/20 21:51, Lendacky, Thomas wrote:
>>>> From: Tom Lendacky <thomas.lendacky@amd.com>
>>>>
>>>> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3108&data=04%7C01%7Cthomas.lendacky%40amd.com%7C1440a9afd7f1450ba93d08d8b15e02a5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637454364641627971%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=P52g0gS3SEhdgkF2qRY6l1J8%2FLjJm1DNR3LLlEmKSBk%3D&reserved=0
>>>>
>>>> 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>
>>>> Signed-off-by: Tom Lendacky <thomas.lendacky@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)
>>
>> Yup, I'll fix that.
>>
>>>
>>>>
>>>> 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).
>>
>> The PCD is used in MemEncryptSevLocateInitialSmramSaveStateMapPages() in
>> the MemEncryptSevLibInternal.c file, which is part of the library. Because
>> of that, I assumed that it needed to be added even though the function
>> that uses it isn't called during SEC.
>>
>> I'll remove it.
>
> Removing it does cause an error.
>
> If we really don't want to include this PCD, I can create SEC and PEI/DXE
> specific versions of the MemEncryptSevLibInternal.c file and just return
> RETURN_UNSUPPORTED for the SEC version of
> MemEncryptSevLocateInitialSmramSaveStateMapPages().
This one seems like the best way forward.
Thanks!
Laszlo
>
> Alternatively, I can just remove the MemEncryptSevLibInternal.c file from
> the build of the SEC library. This should be ok during SEC because there
> are no calls to MemEncryptSevLocateInitialSmramSaveStateMapPages(). If,
> for some reason a call is added later, then the build will fail, but it
> should be obvious why it failed.
>
> Or I can just leave the FeaturePcd section in the SEC inf file.
>
> Thoughts?
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69826): https://edk2.groups.io/g/devel/message/69826
Mute This Topic: https://groups.io/mt/78986172/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 01/05/21 15:34, Lendacky, Thomas wrote: > On 1/5/21 3:40 AM, Laszlo Ersek wrote: >> On 12/15/20 21:51, Lendacky, Thomas wrote: >>> 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). > > The PCD is used in MemEncryptSevLocateInitialSmramSaveStateMapPages() in > the MemEncryptSevLibInternal.c file, which is part of the library. Because > of that, I assumed that it needed to be added even though the function > that uses it isn't called during SEC. > > I'll remove it. My apologies, your original thought was correct; please keep the PCD here. Thanks! Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#69825): https://edk2.groups.io/g/devel/message/69825 Mute This Topic: https://groups.io/mt/78986172/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.