Add Secure Encrypted Virtualization (SEV) helper library. The library provides
the routines to set or clear memory encryption bit for a given memory region
and functions to check whether SEV is enabled.
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
OvmfPkg/Include/Library/MemEncryptSevLib.h | 69 +++++
.../BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf | 46 +++
.../BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c | 124 ++++++++
.../BaseMemEncryptSevLib/X64/MemEncryptSevLib.c | 120 ++++++++
.../BaseMemEncryptSevLib/X64/VirtualMemory.c | 304 ++++++++++++++++++++
.../BaseMemEncryptSevLib/X64/VirtualMemory.h | 158 ++++++++++
OvmfPkg/OvmfPkgIa32.dsc | 1
OvmfPkg/OvmfPkgIa32X64.dsc | 1
OvmfPkg/OvmfPkgX64.dsc | 1
9 files changed, 824 insertions(+)
create mode 100644 OvmfPkg/Include/Library/MemEncryptSevLib.h
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h b/OvmfPkg/Include/Library/MemEncryptSevLib.h
new file mode 100644
index 0000000..a9e9356
--- /dev/null
+++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
@@ -0,0 +1,69 @@
+/** @file
+
+ Define Secure Encrypted Virtualization (SEV) base library helper function
+
+ Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
+
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD License
+ which accompanies this distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef __MEM_ENCRYPT_SEV_LIB_H_
+#define __MEM_ENCRYPT_SEV_LIB_H_
+
+#include <Base.h>
+
+/**
+ Returns a boolean to indicate whether SEV is enabled
+
+ @retval TRUE When SEV is active
+ @retval FALSE When SEV is not enabled
+ **/
+BOOLEAN
+EFIAPI
+MemEncryptSevIsEnabled (
+ VOID
+ );
+
+/**
+ This function clears memory encryption bit for the memory region specified by BaseAddress and
+ Number of pages from the current page table context.
+
+ @param[in] BaseAddress The physical address that is the start address of a memory region.
+ @param[in] NumberOfPages The number of pages from start memory region.
+
+ @retval RETURN_SUCCESS The attributes were cleared for the memory region.
+ @retval RETURN_INVALID_PARAMETER Number of pages is zero.
+ @retval RETURN_UNSUPPORTED Clearing memory encryption attribute is not supported
+ **/
+RETURN_STATUS
+EFIAPI
+MemEncryptSevClearPageEncMask (
+ IN PHYSICAL_ADDRESS BaseAddress,
+ IN UINT32 NumberOfPages
+ );
+
+/**
+ This function sets memory encryption bit for the memory region specified by BaseAddress and
+ Number of pages from the current page table context.
+
+ @param[in] BaseAddress The physical address that is the start address of a memory region.
+ @param[in] NumberOfPages The number of pages from start memory region.
+
+ @retval RETURN_SUCCESS The attributes were set for the memory region.
+ @retval RETURN_INVALID_PARAMETER Number of pages is zero.
+ @retval RETURN_UNSUPPORTED Clearing memory encryption attribute is not supported
+ **/
+RETURN_STATUS
+EFIAPI
+MemEncryptSevSetPageEncMask (
+ IN PHYSICAL_ADDRESS BaseAddress,
+ IN UINT32 NumberOfPages
+ );
+#endif // __MEM_ENCRYPT_SEV_LIB_H_
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
new file mode 100644
index 0000000..c23261f
--- /dev/null
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
@@ -0,0 +1,46 @@
+## @file
+#
+# Copyright (c) 2017 Advanced Micro Devices. All rights reserved.<BR>
+#
+# This program and the accompanying materials
+# are licensed and made available under the terms and conditions of the BSD License
+# which accompanies this distribution. The full text of the license may be found at
+# http://opensource.org/licenses/bsd-license.php
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = MemEncryptSevLib
+ FILE_GUID = c1594631-3888-4be4-949f-9c630dbc842b
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = MemEncryptSevLib|PEIM DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_DRIVER
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+# VALID_ARCHITECTURES = IA32 X64
+#
+
+[Packages]
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ OvmfPkg/OvmfPkg.dec
+ UefiCpuPkg/UefiCpuPkg.dec
+
+[Sources.X64]
+ X64/MemEncryptSevLib.c
+ X64/VirtualMemory.c
+
+[Sources.IA32]
+ Ia32/MemEncryptSevLib.c
+
+[LibraryClasses]
+ BaseLib
+ DebugLib
+ MemoryAllocationLib
+ CacheMaintenanceLib
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
new file mode 100644
index 0000000..70fdd2e
--- /dev/null
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
@@ -0,0 +1,124 @@
+/** @file
+
+ Secure Encrypted Virtualization (SEV) library helper function
+
+ Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
+
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD License
+ which accompanies this distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include "Uefi.h"
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Register/Cpuid.h>
+#include <Register/AmdSevMap.h>
+#include <Library/MemEncryptSevLib.h>
+
+STATIC BOOLEAN SevStatus = FALSE;
+STATIC BOOLEAN SevStatusChecked = FALSE;
+
+/**
+
+ Returns a boolean to indicate whether SEV is enabled
+
+ @retval TRUE When SEV is active
+ @retval FALSE When SEV is not enabled
+ **/
+BOOLEAN
+EFIAPI
+MemEncryptSevIsEnabled (
+ VOID
+ )
+{
+ UINT32 RegEax;
+ MSR_SEV_STATUS_REGISTER Msr;
+ CPUID_MEMORY_ENCRYPTION_INFO_EAX Eax;
+
+ //
+ // If Status is already checked then return it
+ //
+ if (SevStatusChecked) {
+ return SevStatus;
+ }
+
+ //
+ // 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) {
+ //
+ // Check MSR_0xC0010131 Bit 0 (Sev is Enabled)
+ //
+ Msr.Uint32 = AsmReadMsr32 (MSR_SEV_STATUS);
+ if (Msr.Bits.SevBit) {
+ return TRUE;
+ }
+ }
+ }
+
+ SevStatusChecked = TRUE;
+
+ return FALSE;
+}
+
+/**
+ This function clears memory encryption bit for the memory region specified by BaseAddress and
+ Number of pages from the current page table context.
+
+ @param[in] BaseAddress The physical address that is the start address of a memory region.
+ @param[in] NumberOfPages The number of pages from start memory region.
+
+ @retval RETURN_SUCCESS The attributes were cleared 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
+MemEncryptSevClearPageEncMask (
+ IN PHYSICAL_ADDRESS BaseAddress,
+ IN UINT32 NumberOfPages
+ )
+{
+ //
+ // Memory encryption bit is not accessible in 32-bit mode
+ //
+ return RETURN_UNSUPPORTED;
+}
+
+/**
+ This function sets memory encryption bit for the memory region specified by BaseAddress and
+ Number of pages from the current page table context.
+
+ @param[in] BaseAddress The physical address that is the start address of a memory region.
+ @param[in] NumberOfPages The number of pages from start memory region.
+
+ @retval RETURN_SUCCESS The attributes were cleared 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
+MemEncryptSevSetPageEncMask (
+ IN PHYSICAL_ADDRESS BaseAddress,
+ IN UINT32 NumberOfPages
+ )
+{
+ //
+ // Memory encryption bit is not accessible in 32-bit mode
+ //
+ return RETURN_UNSUPPORTED;
+}
+
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
new file mode 100644
index 0000000..098acf2
--- /dev/null
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
@@ -0,0 +1,120 @@
+/** @file
+
+ Secure Encrypted Virtualization (SEV) library helper function
+
+ Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
+
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD License
+ which accompanies this distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include "Uefi.h"
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Register/Cpuid.h>
+#include <Register/AmdSevMap.h>
+#include <Library/MemEncryptSevLib.h>
+
+#include "VirtualMemory.h"
+
+STATIC BOOLEAN SevStatus = FALSE;
+STATIC BOOLEAN SevStatusChecked = FALSE;
+
+/**
+ Returns a boolean to indicate whether SEV is enabled
+
+ @retval TRUE When SEV is active
+ @retval FALSE When SEV is not enabled
+ **/
+BOOLEAN
+EFIAPI
+MemEncryptSevIsEnabled (
+ VOID
+ )
+{
+ UINT32 RegEax;
+ MSR_SEV_STATUS_REGISTER Msr;
+ CPUID_MEMORY_ENCRYPTION_INFO_EAX Eax;
+
+ //
+ // If Status is already checked then return it
+ //
+ if (SevStatusChecked) {
+ return SevStatus;
+ }
+
+ //
+ // 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) {
+ //
+ // Check MSR_0xC0010131 Bit 0 (Sev Enabled)
+ //
+ Msr.Uint32 = AsmReadMsr32 (MSR_SEV_STATUS);
+ if (Msr.Bits.SevBit) {
+ return TRUE;
+ }
+ }
+ }
+
+ SevStatusChecked = TRUE;
+
+ return FALSE;
+}
+
+/**
+
+ This function clears memory encryption bit for the memory region specified by BaseAddress and
+ Number of pages from the current page table context.
+
+ @param[in] BaseAddress The physical address that is the start address of a memory region.
+ @param[in] NumberOfPages The number of pages from start memory region.
+
+ @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 encryption attribute is not supported
+ **/
+RETURN_STATUS
+EFIAPI
+MemEncryptSevClearPageEncMask (
+ IN PHYSICAL_ADDRESS BaseAddress,
+ IN UINT32 NumPages
+ )
+{
+ return Set_Memory_Decrypted (BaseAddress, NumPages * EFI_PAGE_SIZE);
+}
+
+/**
+
+ This function clears memory encryption bit for the memory region specified by BaseAddress and
+ Number of pages from the current page table context.
+
+ @param[in] BaseAddress The physical address that is the start address of a memory region.
+ @param[in] NumberOfPages The number of pages from start memory region.
+
+ @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 encryption attribute is not supported
+ **/
+RETURN_STATUS
+EFIAPI
+MemEncryptSevSetPageEncMask (
+ IN PHYSICAL_ADDRESS BaseAddress,
+ IN UINT32 NumPages
+ )
+{
+ return Set_Memory_Encrypted (BaseAddress, NumPages * EFI_PAGE_SIZE);
+}
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c
new file mode 100644
index 0000000..7acebf3
--- /dev/null
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c
@@ -0,0 +1,304 @@
+/** @file
+
+ Virtual Memory Management Services to set or clear the memory encryption bit
+
+ References:
+ 1) IA-32 Intel(R) Architecture Software Developer's Manual Volume 1:Basic Architecture, Intel
+ 2) IA-32 Intel(R) Architecture Software Developer's Manual Volume 2:Instruction Set Reference, Intel
+ 3) IA-32 Intel(R) Architecture Software Developer's Manual Volume 3:System Programmer's Guide, Intel
+
+Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
+
+This program and the accompanying materials
+are licensed and made available under the terms and conditions of the BSD License
+which accompanies this distribution. The full text of the license may be found at
+http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include "VirtualMemory.h"
+
+#include <Register/AmdSevMap.h>
+
+STATIC UINT64 AddressEncMask;
+
+typedef enum {
+ SetCBit,
+ ClearCBit
+} MAP_RANGE_MODE;
+
+/**
+ Split 2M page to 4K.
+
+ @param[in] PhysicalAddress Start physical address the 2M page covered.
+ @param[in, out] PageEntry2M Pointer to 2M page entry.
+ @param[in] StackBase Stack base address.
+ @param[in] StackSize Stack size.
+
+**/
+STATIC
+VOID
+Split2MPageTo4K (
+ IN EFI_PHYSICAL_ADDRESS PhysicalAddress,
+ IN OUT UINT64 *PageEntry2M,
+ IN EFI_PHYSICAL_ADDRESS StackBase,
+ IN UINTN StackSize
+ )
+{
+ EFI_PHYSICAL_ADDRESS PhysicalAddress4K;
+ UINTN IndexOfPageTableEntries;
+ PAGE_TABLE_4K_ENTRY *PageTableEntry, *PageTableEntry1;
+
+ PageTableEntry = AllocatePages(1);
+
+ PageTableEntry1 = PageTableEntry;
+
+ ASSERT (PageTableEntry != NULL);
+ ASSERT (*PageEntry2M & AddressEncMask);
+
+ PhysicalAddress4K = PhysicalAddress;
+ for (IndexOfPageTableEntries = 0; IndexOfPageTableEntries < 512; IndexOfPageTableEntries++, PageTableEntry++, PhysicalAddress4K += SIZE_4KB) {
+ //
+ // Fill in the Page Table entries
+ //
+ PageTableEntry->Uint64 = (UINT64) PhysicalAddress4K | AddressEncMask;
+ PageTableEntry->Bits.ReadWrite = 1;
+ PageTableEntry->Bits.Present = 1;
+ if ((PhysicalAddress4K >= StackBase) && (PhysicalAddress4K < StackBase + StackSize)) {
+ //
+ // Set Nx bit for stack.
+ //
+ PageTableEntry->Bits.Nx = 1;
+ }
+ }
+
+ //
+ // Fill in 2M page entry.
+ //
+ *PageEntry2M = (UINT64) (UINTN) PageTableEntry1 | IA32_PG_P | IA32_PG_RW | AddressEncMask;
+}
+
+/**
+ Split 1G page to 2M.
+
+ @param[in] PhysicalAddress Start physical address the 1G page covered.
+ @param[in, out] PageEntry1G Pointer to 1G page entry.
+ @param[in] StackBase Stack base address.
+ @param[in] StackSize Stack size.
+
+**/
+STATIC
+VOID
+Split1GPageTo2M (
+ IN EFI_PHYSICAL_ADDRESS PhysicalAddress,
+ IN OUT UINT64 *PageEntry1G,
+ IN EFI_PHYSICAL_ADDRESS StackBase,
+ IN UINTN StackSize
+ )
+{
+ EFI_PHYSICAL_ADDRESS PhysicalAddress2M;
+ UINTN IndexOfPageDirectoryEntries;
+ PAGE_TABLE_ENTRY *PageDirectoryEntry;
+
+ PageDirectoryEntry = AllocatePages(1);
+
+ ASSERT (PageDirectoryEntry != NULL);
+ ASSERT (*PageEntry1G & AddressEncMask);
+ //
+ // Fill in 1G page entry.
+ //
+ *PageEntry1G = (UINT64) (UINTN) PageDirectoryEntry | IA32_PG_P | IA32_PG_RW | AddressEncMask;
+
+ PhysicalAddress2M = PhysicalAddress;
+ for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512; IndexOfPageDirectoryEntries++, PageDirectoryEntry++, PhysicalAddress2M += SIZE_2MB) {
+ if ((PhysicalAddress2M < StackBase + StackSize) && ((PhysicalAddress2M + SIZE_2MB) > StackBase)) {
+ //
+ // Need to split this 2M page that covers stack range.
+ //
+ Split2MPageTo4K (PhysicalAddress2M, (UINT64 *) PageDirectoryEntry, StackBase, StackSize);
+ } else {
+ //
+ // Fill in the Page Directory entries
+ //
+ PageDirectoryEntry->Uint64 = (UINT64) PhysicalAddress2M | AddressEncMask;
+ PageDirectoryEntry->Bits.ReadWrite = 1;
+ PageDirectoryEntry->Bits.Present = 1;
+ PageDirectoryEntry->Bits.MustBe1 = 1;
+ }
+ }
+}
+
+
+STATIC VOID
+SetOrClearCBit(
+ IN UINT64* PageTablePointer,
+ IN MAP_RANGE_MODE Mode
+ )
+{
+ if (Mode == SetCBit) {
+ *PageTablePointer |= AddressEncMask;
+ } else {
+ *PageTablePointer &= ~AddressEncMask;
+ }
+
+}
+
+STATIC
+UINT64
+GetMemEncryptionAddressMask (
+ VOID
+ )
+{
+ UINT64 MeMask;
+ CPUID_MEMORY_ENCRYPTION_INFO_EBX Ebx;
+
+ //
+ // CPUID Fn8000_001F[EBX] Bit 0:5 (memory encryption bit position)
+ //
+ AsmCpuid (CPUID_MEMORY_ENCRYPTION_INFO, NULL, &Ebx.Uint32, NULL, NULL);
+ MeMask = LShiftU64 (1, Ebx.Bits.PtePosBits);
+
+ return MeMask & PAGING_1G_ADDRESS_MASK_64;
+}
+
+STATIC
+EFI_STATUS
+EFIAPI
+Set_Memory_Enc_Dec (
+ IN EFI_PHYSICAL_ADDRESS PhysicalAddress,
+ IN UINT64 Length,
+ IN MAP_RANGE_MODE Mode
+ )
+{
+ PAGE_MAP_AND_DIRECTORY_POINTER *PageMapLevel4Entry;
+ PAGE_MAP_AND_DIRECTORY_POINTER *PageUpperDirectoryPointerEntry;
+ PAGE_MAP_AND_DIRECTORY_POINTER *PageDirectoryPointerEntry;
+ PAGE_TABLE_1G_ENTRY *PageDirectory1GEntry;
+ PAGE_TABLE_ENTRY *PageDirectory2MEntry;
+ PAGE_TABLE_4K_ENTRY *PageTableEntry;
+ UINT64 PgTableMask;
+
+ AddressEncMask = GetMemEncryptionAddressMask ();
+
+ if (!AddressEncMask) {
+ return RETURN_ACCESS_DENIED;
+ }
+
+ PgTableMask = AddressEncMask | EFI_PAGE_MASK;
+
+ DEBUG ((EFI_D_VERBOSE, "Set memory range 0x%#Lx+0x%x (%s)\n", PhysicalAddress, Length,
+ Mode == ClearCBit ? "unencrypted" : "encrypted"));
+
+ if (Length == 0) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
+ // We are going to change the memory encryption attribute from C=0 -> C=1 or vice versa
+ // Flush the caches to ensure that data is written into memory with correct C-bit
+ //
+ WriteBackInvalidateDataCacheRange((VOID*) PhysicalAddress, Length);
+
+ while (Length)
+ {
+
+ PageMapLevel4Entry = (VOID*) (AsmReadCr3() & ~PgTableMask);
+ PageMapLevel4Entry += PML4_OFFSET(PhysicalAddress);
+ if (!PageMapLevel4Entry->Bits.Present) {
+ DEBUG((DEBUG_WARN, "ERROR bad PML4 for %lx\n", PhysicalAddress));
+ return EFI_NO_MAPPING;
+ }
+
+ PageDirectory1GEntry = (VOID*) (PageMapLevel4Entry->Bits.PageTableBaseAddress<<12 & ~PgTableMask);
+ PageDirectory1GEntry += PDP_OFFSET(PhysicalAddress);
+ if (!PageDirectory1GEntry->Bits.Present) {
+ DEBUG((DEBUG_WARN, "ERROR bad PDPE for %lx\n", PhysicalAddress));
+ return EFI_NO_MAPPING;
+ }
+
+ // If the MustBe1 bit is not 1, it's not actually a 1GB entry
+ if (PageDirectory1GEntry->Bits.MustBe1) {
+ // Valid 1GB page
+ // If we have at least 1GB to go, we can just update this entry
+ if (!(PhysicalAddress & ((1<<30) - 1)) && Length >= (1<<30)) {
+ SetOrClearCBit(&PageDirectory1GEntry->Uint64, Mode);
+ DEBUG((DEBUG_VERBOSE, "Updated 1GB entry for %lx\n", PhysicalAddress));
+ PhysicalAddress += 1<<30;
+ Length -= 1<<30;
+ } else {
+ // We must split the page
+ DEBUG((DEBUG_VERBOSE, "Spliting 1GB page\n"));
+ Split1GPageTo2M(((UINT64)PageDirectory1GEntry->Bits.PageTableBaseAddress)<<30, (UINT64*) PageDirectory1GEntry, 0, 0);
+ continue;
+ }
+ } else {
+ // Actually a PDP
+ PageUpperDirectoryPointerEntry = (PAGE_MAP_AND_DIRECTORY_POINTER*) PageDirectory1GEntry;
+ PageDirectory2MEntry = (VOID*) (PageUpperDirectoryPointerEntry->Bits.PageTableBaseAddress<<12 & ~PgTableMask);
+ PageDirectory2MEntry += PDE_OFFSET(PhysicalAddress);
+ if (!PageDirectory2MEntry->Bits.Present) {
+ DEBUG((DEBUG_WARN, "ERROR bad PDE for %lx\n", PhysicalAddress));
+ return EFI_NO_MAPPING;
+ }
+ // If the MustBe1 bit is not a 1, it's not a 2MB entry
+ if (PageDirectory2MEntry->Bits.MustBe1) {
+ // Valid 2MB page
+ // If we have at least 2MB left to go, we can just update this entry
+ if (!(PhysicalAddress & ((1<<21)-1)) && Length >= (1<<21)) {
+ SetOrClearCBit(&PageDirectory2MEntry->Uint64, Mode);
+ DEBUG((DEBUG_VERBOSE, "Updated 2MB entry for %lx\n", PhysicalAddress));
+ PhysicalAddress += 1<<21;
+ Length -= 1<<21;
+ } else {
+ // We must split up this page into 4K pages
+ DEBUG((DEBUG_VERBOSE, "Spliting 2MB page at %lx\n", PhysicalAddress));
+ Split2MPageTo4K(((UINT64)PageDirectory2MEntry->Bits.PageTableBaseAddress) << 21, (UINT64*) PageDirectory2MEntry, 0, 0);
+ continue;
+ }
+ } else {
+ PageDirectoryPointerEntry = (PAGE_MAP_AND_DIRECTORY_POINTER*) PageDirectory2MEntry;
+ PageTableEntry = (VOID*) (PageDirectoryPointerEntry->Bits.PageTableBaseAddress<<12 & ~PgTableMask);
+ PageTableEntry += PTE_OFFSET(PhysicalAddress);
+ if (!PageTableEntry->Bits.Present) {
+ DEBUG((DEBUG_WARN, "ERROR bad PTE for %lx\n", PhysicalAddress));
+ return EFI_NO_MAPPING;
+ }
+ SetOrClearCBit(&PageTableEntry->Uint64, Mode);
+ DEBUG((DEBUG_VERBOSE, "Updated 4KB entry for %lx\n", PhysicalAddress));
+ PhysicalAddress += EFI_PAGE_SIZE;
+ Length -= EFI_PAGE_SIZE;
+ }
+ }
+ }
+
+ //
+ // Flush TLB
+ //
+ AsmWriteCr3(AsmReadCr3());
+ return EFI_SUCCESS;
+}
+
+EFI_STATUS
+EFIAPI
+Set_Memory_Decrypted (
+ IN EFI_PHYSICAL_ADDRESS PhysicalAddress,
+ IN UINT64 Length
+ )
+{
+ return Set_Memory_Enc_Dec (PhysicalAddress, Length, ClearCBit);
+}
+
+EFI_STATUS
+EFIAPI
+Set_Memory_Encrypted (
+ IN EFI_PHYSICAL_ADDRESS PhysicalAddress,
+ IN UINT64 Length
+ )
+{
+ return Set_Memory_Enc_Dec (PhysicalAddress, Length, SetCBit);
+}
+
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
new file mode 100644
index 0000000..a556211
--- /dev/null
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
@@ -0,0 +1,158 @@
+/** @file
+
+ Virtual Memory Management Services to set or clear the memory encryption bit
+
+ References:
+ 1) IA-32 Intel(R) Architecture Software Developer's Manual Volume 1:Basic Architecture, Intel
+ 2) IA-32 Intel(R) Architecture Software Developer's Manual Volume 2:Instruction Set Reference, Intel
+ 3) IA-32 Intel(R) Architecture Software Developer's Manual Volume 3:System Programmer's Guide, Intel
+
+Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
+
+This program and the accompanying materials
+are licensed and made available under the terms and conditions of the BSD License
+which accompanies this distribution. The full text of the license may be found at
+http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef __VIRTUAL_MEMORY__
+#define __VIRTUAL_MEMORY__
+
+#include <Uefi.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+//#include <Library/UefiBootServicesTableLib.h>
+//#include <Library/UefiRuntimeServicesTableLib.h>
+
+#include <Library/CacheMaintenanceLib.h>
+#define SYS_CODE64_SEL 0x38
+
+#pragma pack(1)
+
+//
+// Page-Map Level-4 Offset (PML4) and
+// Page-Directory-Pointer Offset (PDPE) entries 4K & 2MB
+//
+
+typedef union {
+ struct {
+ UINT64 Present:1; // 0 = Not present in memory, 1 = Present in memory
+ UINT64 ReadWrite:1; // 0 = Read-Only, 1= Read/Write
+ UINT64 UserSupervisor:1; // 0 = Supervisor, 1=User
+ UINT64 WriteThrough:1; // 0 = Write-Back caching, 1=Write-Through caching
+ UINT64 CacheDisabled:1; // 0 = Cached, 1=Non-Cached
+ UINT64 Accessed:1; // 0 = Not accessed, 1 = Accessed (set by CPU)
+ UINT64 Reserved:1; // Reserved
+ UINT64 MustBeZero:2; // Must Be Zero
+ UINT64 Available:3; // Available for use by system software
+ UINT64 PageTableBaseAddress:40; // Page Table Base Address
+ UINT64 AvabilableHigh:11; // Available for use by system software
+ UINT64 Nx:1; // No Execute bit
+ } Bits;
+ UINT64 Uint64;
+} PAGE_MAP_AND_DIRECTORY_POINTER;
+
+//
+// Page Table Entry 4KB
+//
+typedef union {
+ struct {
+ UINT64 Present:1; // 0 = Not present in memory, 1 = Present in memory
+ UINT64 ReadWrite:1; // 0 = Read-Only, 1= Read/Write
+ UINT64 UserSupervisor:1; // 0 = Supervisor, 1=User
+ UINT64 WriteThrough:1; // 0 = Write-Back caching, 1=Write-Through caching
+ UINT64 CacheDisabled:1; // 0 = Cached, 1=Non-Cached
+ UINT64 Accessed:1; // 0 = Not accessed, 1 = Accessed (set by CPU)
+ UINT64 Dirty:1; // 0 = Not Dirty, 1 = written by processor on access to page
+ UINT64 PAT:1; //
+ UINT64 Global:1; // 0 = Not global page, 1 = global page TLB not cleared on CR3 write
+ UINT64 Available:3; // Available for use by system software
+ UINT64 PageTableBaseAddress:40; // Page Table Base Address
+ UINT64 AvabilableHigh:11; // Available for use by system software
+ UINT64 Nx:1; // 0 = Execute Code, 1 = No Code Execution
+ } Bits;
+ UINT64 Uint64;
+} PAGE_TABLE_4K_ENTRY;
+
+//
+// Page Table Entry 2MB
+//
+typedef union {
+ struct {
+ UINT64 Present:1; // 0 = Not present in memory, 1 = Present in memory
+ UINT64 ReadWrite:1; // 0 = Read-Only, 1= Read/Write
+ UINT64 UserSupervisor:1; // 0 = Supervisor, 1=User
+ UINT64 WriteThrough:1; // 0 = Write-Back caching, 1=Write-Through caching
+ UINT64 CacheDisabled:1; // 0 = Cached, 1=Non-Cached
+ UINT64 Accessed:1; // 0 = Not accessed, 1 = Accessed (set by CPU)
+ UINT64 Dirty:1; // 0 = Not Dirty, 1 = written by processor on access to page
+ UINT64 MustBe1:1; // Must be 1
+ UINT64 Global:1; // 0 = Not global page, 1 = global page TLB not cleared on CR3 write
+ UINT64 Available:3; // Available for use by system software
+ UINT64 PAT:1; //
+ UINT64 MustBeZero:8; // Must be zero;
+ UINT64 PageTableBaseAddress:31; // Page Table Base Address
+ UINT64 AvabilableHigh:11; // Available for use by system software
+ UINT64 Nx:1; // 0 = Execute Code, 1 = No Code Execution
+ } Bits;
+ UINT64 Uint64;
+} PAGE_TABLE_ENTRY;
+
+//
+// Page Table Entry 1GB
+//
+typedef union {
+ struct {
+ UINT64 Present:1; // 0 = Not present in memory, 1 = Present in memory
+ UINT64 ReadWrite:1; // 0 = Read-Only, 1= Read/Write
+ UINT64 UserSupervisor:1; // 0 = Supervisor, 1=User
+ UINT64 WriteThrough:1; // 0 = Write-Back caching, 1=Write-Through caching
+ UINT64 CacheDisabled:1; // 0 = Cached, 1=Non-Cached
+ UINT64 Accessed:1; // 0 = Not accessed, 1 = Accessed (set by CPU)
+ UINT64 Dirty:1; // 0 = Not Dirty, 1 = written by processor on access to page
+ UINT64 MustBe1:1; // Must be 1
+ UINT64 Global:1; // 0 = Not global page, 1 = global page TLB not cleared on CR3 write
+ UINT64 Available:3; // Available for use by system software
+ UINT64 PAT:1; //
+ UINT64 MustBeZero:17; // Must be zero;
+ UINT64 PageTableBaseAddress:22; // Page Table Base Address
+ UINT64 AvabilableHigh:11; // Available for use by system software
+ UINT64 Nx:1; // 0 = Execute Code, 1 = No Code Execution
+ } Bits;
+ UINT64 Uint64;
+} PAGE_TABLE_1G_ENTRY;
+
+#pragma pack()
+
+#define IA32_PG_P BIT0
+#define IA32_PG_RW BIT1
+
+#define PAGETABLE_ENTRY_MASK ((1UL << 9) - 1)
+#define PML4_OFFSET(x) ( (x >> 39) & PAGETABLE_ENTRY_MASK)
+#define PDP_OFFSET(x) ( (x >> 30) & PAGETABLE_ENTRY_MASK)
+#define PDE_OFFSET(x) ( (x >> 21) & PAGETABLE_ENTRY_MASK)
+#define PTE_OFFSET(x) ( (x >> 12) & PAGETABLE_ENTRY_MASK)
+#define PAGING_1G_ADDRESS_MASK_64 0x000FFFFFC0000000ull
+
+EFI_STATUS
+EFIAPI
+Set_Memory_Decrypted (
+ IN EFI_PHYSICAL_ADDRESS PhysicalAddress,
+ IN UINT64 Length
+ );
+
+EFI_STATUS
+EFIAPI
+Set_Memory_Encrypted (
+ IN EFI_PHYSICAL_ADDRESS PhysicalAddress,
+ IN UINT64 Length
+ );
+
+#endif
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 769251d..c2821b7 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -126,6 +126,7 @@
QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
+ MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
!if $(SMM_REQUIRE) == FALSE
LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
!endif
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 3874c35..1dd8064 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -131,6 +131,7 @@
QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
+ MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
!if $(SMM_REQUIRE) == FALSE
LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
!endif
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index fe7f086..06bee32 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -131,6 +131,7 @@
QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
+ MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
!if $(SMM_REQUIRE) == FALSE
LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
!endif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 03/21/17 22:13, Brijesh Singh wrote:
> Add Secure Encrypted Virtualization (SEV) helper library. The library provides
> the routines to set or clear memory encryption bit for a given memory region
> and functions to check whether SEV is enabled.
Please rewrap this commit message to 74 characters.
>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
> OvmfPkg/Include/Library/MemEncryptSevLib.h | 69 +++++
> .../BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf | 46 +++
> .../BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c | 124 ++++++++
> .../BaseMemEncryptSevLib/X64/MemEncryptSevLib.c | 120 ++++++++
> .../BaseMemEncryptSevLib/X64/VirtualMemory.c | 304 ++++++++++++++++++++
> .../BaseMemEncryptSevLib/X64/VirtualMemory.h | 158 ++++++++++
> OvmfPkg/OvmfPkgIa32.dsc | 1
> OvmfPkg/OvmfPkgIa32X64.dsc | 1
> OvmfPkg/OvmfPkgX64.dsc | 1
Please format patches as described under
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-23
In particular, the options
--stat=1000 \
--stat-graph-width=20 \
will ensure that git does not use the "..." abbreviation in the
diffstat, while also keeping the plus/minus stats reasonably narrow.
> 9 files changed, 824 insertions(+)
> create mode 100644 OvmfPkg/Include/Library/MemEncryptSevLib.h
> create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
> create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
> create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
> create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c
> create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
>
> diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h b/OvmfPkg/Include/Library/MemEncryptSevLib.h
> new file mode 100644
> index 0000000..a9e9356
> --- /dev/null
> +++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
> @@ -0,0 +1,69 @@
> +/** @file
> +
> + Define Secure Encrypted Virtualization (SEV) base library helper function
> +
> + Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> +
> + This program and the accompanying materials
> + are licensed and made available under the terms and conditions of the BSD License
> + which accompanies this distribution. The full text of the license may be found at
Overlong line. No line should be wider than 79 chars.
> + http://opensource.org/licenses/bsd-license.php
> +
> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#ifndef __MEM_ENCRYPT_SEV_LIB_H_
> +#define __MEM_ENCRYPT_SEV_LIB_H_
If you start the guard macro name with two underscores, then please also
end it with two underscores.
> +
> +#include <Base.h>
> +
> +/**
> + Returns a boolean to indicate whether SEV is enabled
> +
> + @retval TRUE When SEV is active
> + @retval FALSE When SEV is not enabled
> + **/
> +BOOLEAN
> +EFIAPI
> +MemEncryptSevIsEnabled (
> + VOID
> + );
Would it make sense to call this library function in PlatformPei, rather
than add a separate SevIsEnabled() function to it (in patch #3)? The
implementations look nearly identical.
The client module type restrictions under LIBRARY_CLASS (below) look
fine for that; all of the permitted module types support writeable
global variables in OVMF.
> +
> +/**
> + This function clears memory encryption bit for the memory region specified by BaseAddress and
> + Number of pages from the current page table context.
> +
> + @param[in] BaseAddress The physical address that is the start address of a memory region.
> + @param[in] NumberOfPages The number of pages from start memory region.
> +
> + @retval RETURN_SUCCESS The attributes were cleared for the memory region.
> + @retval RETURN_INVALID_PARAMETER Number of pages is zero.
> + @retval RETURN_UNSUPPORTED Clearing memory encryption attribute is not supported
> + **/
These lines as well are too wide.
> +RETURN_STATUS
> +EFIAPI
> +MemEncryptSevClearPageEncMask (
> + IN PHYSICAL_ADDRESS BaseAddress,
> + IN UINT32 NumberOfPages
> + );
> +
> +/**
> + This function sets memory encryption bit for the memory region specified by BaseAddress and
> + Number of pages from the current page table context.
> +
> + @param[in] BaseAddress The physical address that is the start address of a memory region.
> + @param[in] NumberOfPages The number of pages from start memory region.
> +
> + @retval RETURN_SUCCESS The attributes were set for the memory region.
> + @retval RETURN_INVALID_PARAMETER Number of pages is zero.
> + @retval RETURN_UNSUPPORTED Clearing memory encryption attribute is not supported
> + **/
> +RETURN_STATUS
> +EFIAPI
> +MemEncryptSevSetPageEncMask (
> + IN PHYSICAL_ADDRESS BaseAddress,
> + IN UINT32 NumberOfPages
> + );
> +#endif // __MEM_ENCRYPT_SEV_LIB_H_
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
> new file mode 100644
> index 0000000..c23261f
> --- /dev/null
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
> @@ -0,0 +1,46 @@
> +## @file
Please add one or two sentences under @file about the purpose of this
library instance.
> +#
> +# Copyright (c) 2017 Advanced Micro Devices. All rights reserved.<BR>
> +#
> +# This program and the accompanying materials
> +# are licensed and made available under the terms and conditions of the BSD License
> +# which accompanies this distribution. The full text of the license may be found at
> +# http://opensource.org/licenses/bsd-license.php
> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +#
> +##
Please rewrap to 79 chars.
(I think I won't ask for that explicitly in any other locations; please
just ensure all new files conform to that.)
> +
> +[Defines]
> + INF_VERSION = 0x00010005
Please use "1.25" here, that's the most recent release of the INF spec
(to my knowledge).
> + BASE_NAME = MemEncryptSevLib
> + FILE_GUID = c1594631-3888-4be4-949f-9c630dbc842b
> + MODULE_TYPE = BASE
> + VERSION_STRING = 1.0
> + LIBRARY_CLASS = MemEncryptSevLib|PEIM DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_DRIVER
> +
> +#
> +# The following information is for reference only and not required by the build tools.
> +#
> +# VALID_ARCHITECTURES = IA32 X64
> +#
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> + MdeModulePkg/MdeModulePkg.dec
> + OvmfPkg/OvmfPkg.dec
> + UefiCpuPkg/UefiCpuPkg.dec
> +
> +[Sources.X64]
> + X64/MemEncryptSevLib.c
> + X64/VirtualMemory.c
> +
> +[Sources.IA32]
> + Ia32/MemEncryptSevLib.c
> +
> +[LibraryClasses]
> + BaseLib
> + DebugLib
> + MemoryAllocationLib
> + CacheMaintenanceLib
Please sort the entries in each section alphabetically.
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
> new file mode 100644
> index 0000000..70fdd2e
> --- /dev/null
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
> @@ -0,0 +1,124 @@
> +/** @file
> +
> + Secure Encrypted Virtualization (SEV) library helper function
> +
> + Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> +
> + This program and the accompanying materials
> + are licensed and made available under the terms and conditions of the BSD License
> + which accompanies this distribution. The full text of the license may be found at
> + http://opensource.org/licenses/bsd-license.php
> +
> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include "Uefi.h"
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Register/Cpuid.h>
> +#include <Register/AmdSevMap.h>
> +#include <Library/MemEncryptSevLib.h>
> +
> +STATIC BOOLEAN SevStatus = FALSE;
> +STATIC BOOLEAN SevStatusChecked = FALSE;
These variables should be called mSevStatus and mSevStatusChecked.
> +
> +/**
> +
> + Returns a boolean to indicate whether SEV is enabled
> +
> + @retval TRUE When SEV is active
> + @retval FALSE When SEV is not enabled
> + **/
> +BOOLEAN
> +EFIAPI
> +MemEncryptSevIsEnabled (
> + VOID
> + )
> +{
> + UINT32 RegEax;
> + MSR_SEV_STATUS_REGISTER Msr;
> + CPUID_MEMORY_ENCRYPTION_INFO_EAX Eax;
Please align the variable names.
> +
> + //
> + // If Status is already checked then return it
> + //
> + if (SevStatusChecked) {
> + return SevStatus;
> + }
> +
> + //
> + // 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) {
> + //
> + // Check MSR_0xC0010131 Bit 0 (Sev is Enabled)
> + //
> + Msr.Uint32 = AsmReadMsr32 (MSR_SEV_STATUS);
> + if (Msr.Bits.SevBit) {
You forgot to set mSevStatusChecked to TRUE here.
> + return TRUE;
> + }
> + }
> + }
> +
> + SevStatusChecked = TRUE;
> +
> + return FALSE;
> +}
> +
> +/**
> + This function clears memory encryption bit for the memory region specified by BaseAddress and
> + Number of pages from the current page table context.
> +
> + @param[in] BaseAddress The physical address that is the start address of a memory region.
> + @param[in] NumberOfPages The number of pages from start memory region.
> +
> + @retval RETURN_SUCCESS The attributes were cleared 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
> +MemEncryptSevClearPageEncMask (
> + IN PHYSICAL_ADDRESS BaseAddress,
> + IN UINT32 NumberOfPages
> + )
> +{
> + //
> + // Memory encryption bit is not accessible in 32-bit mode
> + //
> + return RETURN_UNSUPPORTED;
> +}
> +
> +/**
> + This function sets memory encryption bit for the memory region specified by BaseAddress and
> + Number of pages from the current page table context.
> +
> + @param[in] BaseAddress The physical address that is the start address of a memory region.
> + @param[in] NumberOfPages The number of pages from start memory region.
> +
> + @retval RETURN_SUCCESS The attributes were cleared 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
> +MemEncryptSevSetPageEncMask (
> + IN PHYSICAL_ADDRESS BaseAddress,
> + IN UINT32 NumberOfPages
> + )
> +{
> + //
> + // Memory encryption bit is not accessible in 32-bit mode
> + //
> + return RETURN_UNSUPPORTED;
> +}
> +
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
> new file mode 100644
> index 0000000..098acf2
> --- /dev/null
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
> @@ -0,0 +1,120 @@
> +/** @file
> +
> + Secure Encrypted Virtualization (SEV) library helper function
> +
> + Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> +
> + This program and the accompanying materials
> + are licensed and made available under the terms and conditions of the BSD License
> + which accompanies this distribution. The full text of the license may be found at
> + http://opensource.org/licenses/bsd-license.php
> +
> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include "Uefi.h"
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Register/Cpuid.h>
> +#include <Register/AmdSevMap.h>
> +#include <Library/MemEncryptSevLib.h>
> +
> +#include "VirtualMemory.h"
> +
> +STATIC BOOLEAN SevStatus = FALSE;
> +STATIC BOOLEAN SevStatusChecked = FALSE;
> +
> +/**
> + Returns a boolean to indicate whether SEV is enabled
> +
> + @retval TRUE When SEV is active
> + @retval FALSE When SEV is not enabled
> + **/
> +BOOLEAN
> +EFIAPI
> +MemEncryptSevIsEnabled (
> + VOID
> + )
> +{
> + UINT32 RegEax;
> + MSR_SEV_STATUS_REGISTER Msr;
> + CPUID_MEMORY_ENCRYPTION_INFO_EAX Eax;
> +
> + //
> + // If Status is already checked then return it
> + //
> + if (SevStatusChecked) {
> + return SevStatus;
> + }
> +
> + //
> + // 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) {
> + //
> + // Check MSR_0xC0010131 Bit 0 (Sev Enabled)
> + //
> + Msr.Uint32 = AsmReadMsr32 (MSR_SEV_STATUS);
> + if (Msr.Bits.SevBit) {
> + return TRUE;
> + }
> + }
> + }
> +
> + SevStatusChecked = TRUE;
> +
> + return FALSE;
> +}
In fact, I suggest that:
(a) you introduce two new files, called
MemEncryptSevLibInternal.h
MemEncryptSevLibInternal.c
(b) add the .c file to both [Sources.X64] and [Sources.IA32],
(c) rename MemEncryptSevIsEnabled() to InternalMemEncryptSevIsEnabled,
and declare it in the new header file,
(d) move MemEncryptSevIsEnabled, SevStatus and SevStatusChecked to the
new .c file.
> +
> +/**
> +
> + This function clears memory encryption bit for the memory region specified by BaseAddress and
> + Number of pages from the current page table context.
> +
> + @param[in] BaseAddress The physical address that is the start address of a memory region.
> + @param[in] NumberOfPages The number of pages from start memory region.
> +
> + @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 encryption attribute is not supported
> + **/
> +RETURN_STATUS
> +EFIAPI
> +MemEncryptSevClearPageEncMask (
> + IN PHYSICAL_ADDRESS BaseAddress,
> + IN UINT32 NumPages
> + )
> +{
> + return Set_Memory_Decrypted (BaseAddress, NumPages * EFI_PAGE_SIZE);
> +}
"Set_Memory_Decrypted" is not idiomatic edk2, please call it
SetMemoryDecrypted,
Also, the second argument should be written as
EFI_PAGES_TO_SIZE (NumPages)
... However, for that, you should please switch the type of "NumPages"
to UINTN. Is that functionally okay?
> +
> +/**
> +
> + This function clears memory encryption bit for the memory region specified by BaseAddress and
> + Number of pages from the current page table context.
> +
> + @param[in] BaseAddress The physical address that is the start address of a memory region.
> + @param[in] NumberOfPages The number of pages from start memory region.
> +
> + @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 encryption attribute is not supported
> + **/
> +RETURN_STATUS
> +EFIAPI
> +MemEncryptSevSetPageEncMask (
> + IN PHYSICAL_ADDRESS BaseAddress,
> + IN UINT32 NumPages
> + )
> +{
> + return Set_Memory_Encrypted (BaseAddress, NumPages * EFI_PAGE_SIZE);
> +}
Same comments here.
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c
> new file mode 100644
> index 0000000..7acebf3
> --- /dev/null
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c
> @@ -0,0 +1,304 @@
> +/** @file
> +
> + Virtual Memory Management Services to set or clear the memory encryption bit
> +
> + References:
> + 1) IA-32 Intel(R) Architecture Software Developer's Manual Volume 1:Basic Architecture, Intel
> + 2) IA-32 Intel(R) Architecture Software Developer's Manual Volume 2:Instruction Set Reference, Intel
> + 3) IA-32 Intel(R) Architecture Software Developer's Manual Volume 3:System Programmer's Guide, Intel
> +
> +Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> +
> +This program and the accompanying materials
> +are licensed and made available under the terms and conditions of the BSD License
> +which accompanies this distribution. The full text of the license may be found at
> +http://opensource.org/licenses/bsd-license.php
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include "VirtualMemory.h"
> +
> +#include <Register/AmdSevMap.h>
Please put the <> includes first, and the "" includes second.
> +
> +STATIC UINT64 AddressEncMask;
Should be called "mAddressEncMask".
> +
> +typedef enum {
> + SetCBit,
> + ClearCBit
> +} MAP_RANGE_MODE;
> +
> +/**
> + Split 2M page to 4K.
> +
> + @param[in] PhysicalAddress Start physical address the 2M page covered.
> + @param[in, out] PageEntry2M Pointer to 2M page entry.
> + @param[in] StackBase Stack base address.
> + @param[in] StackSize Stack size.
> +
> +**/
> +STATIC
> +VOID
> +Split2MPageTo4K (
> + IN EFI_PHYSICAL_ADDRESS PhysicalAddress,
Your use of PHYSICAL_ADDRESS <-> EFI_PHYSICAL_ADDRESS is inconsistent.
Since this is a BASE type library, I suggest to stick with
PHYSICAL_ADDRESS. Can you please check all occurrences of
EFI_PHYSICAL_ADDRESS?
> + IN OUT UINT64 *PageEntry2M,
IN and OUT columns should be visually separated across all parameters.
> + IN EFI_PHYSICAL_ADDRESS StackBase,
> + IN UINTN StackSize
> + )
> +{
> + EFI_PHYSICAL_ADDRESS PhysicalAddress4K;
> + UINTN IndexOfPageTableEntries;
> + PAGE_TABLE_4K_ENTRY *PageTableEntry, *PageTableEntry1;
> +
> + PageTableEntry = AllocatePages(1);
> +
> + PageTableEntry1 = PageTableEntry;
> +
> + ASSERT (PageTableEntry != NULL);
> + ASSERT (*PageEntry2M & AddressEncMask);
> +
> + PhysicalAddress4K = PhysicalAddress;
> + for (IndexOfPageTableEntries = 0; IndexOfPageTableEntries < 512; IndexOfPageTableEntries++, PageTableEntry++, PhysicalAddress4K += SIZE_4KB) {
> + //
> + // Fill in the Page Table entries
> + //
> + PageTableEntry->Uint64 = (UINT64) PhysicalAddress4K | AddressEncMask;
> + PageTableEntry->Bits.ReadWrite = 1;
> + PageTableEntry->Bits.Present = 1;
> + if ((PhysicalAddress4K >= StackBase) && (PhysicalAddress4K < StackBase + StackSize)) {
> + //
> + // Set Nx bit for stack.
> + //
> + PageTableEntry->Bits.Nx = 1;
> + }
> + }
> +
> + //
> + // Fill in 2M page entry.
> + //
> + *PageEntry2M = (UINT64) (UINTN) PageTableEntry1 | IA32_PG_P | IA32_PG_RW | AddressEncMask;
> +}
Side question: is this function copied from some other place in edk2? If
so, I suggest to add a comment (in the code or in the commit message)
where it comes from.
Also, shouldn't you call CpuFlushTlb() somewhere, when messing with page
tables? (Hm, maybe not; a given virtual address will continue to map to
the same physical address.)
> +
> +/**
> + Split 1G page to 2M.
> +
> + @param[in] PhysicalAddress Start physical address the 1G page covered.
> + @param[in, out] PageEntry1G Pointer to 1G page entry.
> + @param[in] StackBase Stack base address.
> + @param[in] StackSize Stack size.
> +
> +**/
> +STATIC
> +VOID
> +Split1GPageTo2M (
> + IN EFI_PHYSICAL_ADDRESS PhysicalAddress,
> + IN OUT UINT64 *PageEntry1G,
> + IN EFI_PHYSICAL_ADDRESS StackBase,
> + IN UINTN StackSize
> + )
> +{
> + EFI_PHYSICAL_ADDRESS PhysicalAddress2M;
> + UINTN IndexOfPageDirectoryEntries;
> + PAGE_TABLE_ENTRY *PageDirectoryEntry;
> +
> + PageDirectoryEntry = AllocatePages(1);
> +
> + ASSERT (PageDirectoryEntry != NULL);
> + ASSERT (*PageEntry1G & AddressEncMask);
> + //
> + // Fill in 1G page entry.
> + //
> + *PageEntry1G = (UINT64) (UINTN) PageDirectoryEntry | IA32_PG_P | IA32_PG_RW | AddressEncMask;
> +
> + PhysicalAddress2M = PhysicalAddress;
> + for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512; IndexOfPageDirectoryEntries++, PageDirectoryEntry++, PhysicalAddress2M += SIZE_2MB) {
> + if ((PhysicalAddress2M < StackBase + StackSize) && ((PhysicalAddress2M + SIZE_2MB) > StackBase)) {
> + //
> + // Need to split this 2M page that covers stack range.
> + //
> + Split2MPageTo4K (PhysicalAddress2M, (UINT64 *) PageDirectoryEntry, StackBase, StackSize);
> + } else {
> + //
> + // Fill in the Page Directory entries
> + //
> + PageDirectoryEntry->Uint64 = (UINT64) PhysicalAddress2M | AddressEncMask;
> + PageDirectoryEntry->Bits.ReadWrite = 1;
> + PageDirectoryEntry->Bits.Present = 1;
> + PageDirectoryEntry->Bits.MustBe1 = 1;
> + }
> + }
> +}
> +
> +
> +STATIC VOID
> +SetOrClearCBit(
> + IN UINT64* PageTablePointer,
> + IN MAP_RANGE_MODE Mode
Non-idiomatic formatting, plus "IN" is actually incorrect for
PageTablePointer, as it is both read and written. This is what you need:
IN OUT UINT64 *PageTablePointer,
IN MAP_RANGE_MODE Mode
> + )
> +{
> + if (Mode == SetCBit) {
> + *PageTablePointer |= AddressEncMask;
> + } else {
> + *PageTablePointer &= ~AddressEncMask;
> + }
> +
> +}
> +
> +STATIC
> +UINT64
> +GetMemEncryptionAddressMask (
> + VOID
> + )
> +{
> + UINT64 MeMask;
> + CPUID_MEMORY_ENCRYPTION_INFO_EBX Ebx;
variable name alignment
> +
> + //
> + // CPUID Fn8000_001F[EBX] Bit 0:5 (memory encryption bit position)
> + //
> + AsmCpuid (CPUID_MEMORY_ENCRYPTION_INFO, NULL, &Ebx.Uint32, NULL, NULL);
> + MeMask = LShiftU64 (1, Ebx.Bits.PtePosBits);
> +
> + return MeMask & PAGING_1G_ADDRESS_MASK_64;
> +}
Can you perhaps export this function and call it from PlatformPei as
well? Just asking.
> +
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +Set_Memory_Enc_Dec (
Please use camel case without underscores.
Also, this function looks like the "workhorse" of the library. Can you
add a leading comment block to it?
> + IN EFI_PHYSICAL_ADDRESS PhysicalAddress,
> + IN UINT64 Length,
> + IN MAP_RANGE_MODE Mode
> + )
> +{
> + PAGE_MAP_AND_DIRECTORY_POINTER *PageMapLevel4Entry;
> + PAGE_MAP_AND_DIRECTORY_POINTER *PageUpperDirectoryPointerEntry;
> + PAGE_MAP_AND_DIRECTORY_POINTER *PageDirectoryPointerEntry;
> + PAGE_TABLE_1G_ENTRY *PageDirectory1GEntry;
> + PAGE_TABLE_ENTRY *PageDirectory2MEntry;
> + PAGE_TABLE_4K_ENTRY *PageTableEntry;
> + UINT64 PgTableMask;
> +
> + AddressEncMask = GetMemEncryptionAddressMask ();
You unconditionally rewrite the static variable here, and then you rely
on it in the other helper functions.
What do you think about adding some internal caching to
GetMemEncryptionAddressMask(), and then calling
GetMemEncryptionAddressMask() explicitly everywhere the mask is needed?
> +
> + if (!AddressEncMask) {
> + return RETURN_ACCESS_DENIED;
> + }
> +
> + PgTableMask = AddressEncMask | EFI_PAGE_MASK;
> +
> + DEBUG ((EFI_D_VERBOSE, "Set memory range 0x%#Lx+0x%x (%s)\n", PhysicalAddress, Length,
> + Mode == ClearCBit ? "unencrypted" : "encrypted"));
Please use DEBUG_VERBOSE.
Also, %x is not right for printing Length (UINT64).
> +
> + if (Length == 0) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + //
> + // We are going to change the memory encryption attribute from C=0 -> C=1 or vice versa
> + // Flush the caches to ensure that data is written into memory with correct C-bit
> + //
> + WriteBackInvalidateDataCacheRange((VOID*) PhysicalAddress, Length);
Please convert pointers to (UINTN) first.
> +
> + while (Length)
> + {
> +
> + PageMapLevel4Entry = (VOID*) (AsmReadCr3() & ~PgTableMask);
> + PageMapLevel4Entry += PML4_OFFSET(PhysicalAddress);
> + if (!PageMapLevel4Entry->Bits.Present) {
> + DEBUG((DEBUG_WARN, "ERROR bad PML4 for %lx\n", PhysicalAddress));
missing space after DEBUG
> + return EFI_NO_MAPPING;
> + }
> +
> + PageDirectory1GEntry = (VOID*) (PageMapLevel4Entry->Bits.PageTableBaseAddress<<12 & ~PgTableMask);
missing spaces around <<
also, to clarify the binding precedence of << over &, please use
parentheses.
> + PageDirectory1GEntry += PDP_OFFSET(PhysicalAddress);
> + if (!PageDirectory1GEntry->Bits.Present) {
> + DEBUG((DEBUG_WARN, "ERROR bad PDPE for %lx\n", PhysicalAddress));
missing space after DEBUG... please check the rest of the occurrences
> + return EFI_NO_MAPPING;
> + }
> +
> + // If the MustBe1 bit is not 1, it's not actually a 1GB entry
Comment style is
//
// comment
//
please check the rest
> + if (PageDirectory1GEntry->Bits.MustBe1) {
> + // Valid 1GB page
> + // If we have at least 1GB to go, we can just update this entry
> + if (!(PhysicalAddress & ((1<<30) - 1)) && Length >= (1<<30)) {
For constants like the above, please consider using the macro BIT30
> + SetOrClearCBit(&PageDirectory1GEntry->Uint64, Mode);
missing space after SetOrClearCBit
> + DEBUG((DEBUG_VERBOSE, "Updated 1GB entry for %lx\n", PhysicalAddress));
> + PhysicalAddress += 1<<30;
> + Length -= 1<<30;
> + } else {
> + // We must split the page
> + DEBUG((DEBUG_VERBOSE, "Spliting 1GB page\n"));
> + Split1GPageTo2M(((UINT64)PageDirectory1GEntry->Bits.PageTableBaseAddress)<<30, (UINT64*) PageDirectory1GEntry, 0, 0);
> + continue;
> + }
> + } else {
> + // Actually a PDP
> + PageUpperDirectoryPointerEntry = (PAGE_MAP_AND_DIRECTORY_POINTER*) PageDirectory1GEntry;
> + PageDirectory2MEntry = (VOID*) (PageUpperDirectoryPointerEntry->Bits.PageTableBaseAddress<<12 & ~PgTableMask);
> + PageDirectory2MEntry += PDE_OFFSET(PhysicalAddress);
> + if (!PageDirectory2MEntry->Bits.Present) {
> + DEBUG((DEBUG_WARN, "ERROR bad PDE for %lx\n", PhysicalAddress));
> + return EFI_NO_MAPPING;
> + }
> + // If the MustBe1 bit is not a 1, it's not a 2MB entry
> + if (PageDirectory2MEntry->Bits.MustBe1) {
> + // Valid 2MB page
> + // If we have at least 2MB left to go, we can just update this entry
> + if (!(PhysicalAddress & ((1<<21)-1)) && Length >= (1<<21)) {
> + SetOrClearCBit(&PageDirectory2MEntry->Uint64, Mode);
> + DEBUG((DEBUG_VERBOSE, "Updated 2MB entry for %lx\n", PhysicalAddress));
> + PhysicalAddress += 1<<21;
> + Length -= 1<<21;
> + } else {
> + // We must split up this page into 4K pages
> + DEBUG((DEBUG_VERBOSE, "Spliting 2MB page at %lx\n", PhysicalAddress));
> + Split2MPageTo4K(((UINT64)PageDirectory2MEntry->Bits.PageTableBaseAddress) << 21, (UINT64*) PageDirectory2MEntry, 0, 0);
> + continue;
> + }
> + } else {
> + PageDirectoryPointerEntry = (PAGE_MAP_AND_DIRECTORY_POINTER*) PageDirectory2MEntry;
> + PageTableEntry = (VOID*) (PageDirectoryPointerEntry->Bits.PageTableBaseAddress<<12 & ~PgTableMask);
> + PageTableEntry += PTE_OFFSET(PhysicalAddress);
> + if (!PageTableEntry->Bits.Present) {
> + DEBUG((DEBUG_WARN, "ERROR bad PTE for %lx\n", PhysicalAddress));
> + return EFI_NO_MAPPING;
> + }
> + SetOrClearCBit(&PageTableEntry->Uint64, Mode);
> + DEBUG((DEBUG_VERBOSE, "Updated 4KB entry for %lx\n", PhysicalAddress));
> + PhysicalAddress += EFI_PAGE_SIZE;
> + Length -= EFI_PAGE_SIZE;
> + }
> + }
> + }
So, I didn't do any functional validation for the above, just coding style.
> +
> + //
> + // Flush TLB
> + //
> + AsmWriteCr3(AsmReadCr3());
Please just call CpuFlushTlb().
> + return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +Set_Memory_Decrypted (
> + IN EFI_PHYSICAL_ADDRESS PhysicalAddress,
> + IN UINT64 Length
> + )
> +{
> + return Set_Memory_Enc_Dec (PhysicalAddress, Length, ClearCBit);
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +Set_Memory_Encrypted (
> + IN EFI_PHYSICAL_ADDRESS PhysicalAddress,
> + IN UINT64 Length
> + )
> +{
> + return Set_Memory_Enc_Dec (PhysicalAddress, Length, SetCBit);
> +}
> +
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
Can we upstream the contents of this file to MdePkg somehow? My first
idea would be Include/IndustryStandard.
However, the following types are already in
"MdePkg/Include/Library/BaseLib.h":
///
/// IA32 and x64 Specific Functions.
/// Byte packed structure for 16-bit Real Mode EFLAGS.
///
IA32_FLAGS16, IA32_EFLAGS32, IA32_CR0, IA32_CR4,
IA32_SEGMENT_DESCRIPTOR, IA32_DESCRIPTOR, IA32_IDT_GATE_DESCRIPTOR, ...
and a bunch of others.
I'm asking because now we have:
BaseTools/Source/C/GenPage/VirtualMemory.h
DuetPkg/DxeIpl/Ia32/VirtualMemory.h
DuetPkg/DxeIpl/X64/VirtualMemory.h
MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h
OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
> new file mode 100644
> index 0000000..a556211
> --- /dev/null
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
> @@ -0,0 +1,158 @@
> +/** @file
> +
> + Virtual Memory Management Services to set or clear the memory encryption bit
> +
> + References:
> + 1) IA-32 Intel(R) Architecture Software Developer's Manual Volume 1:Basic Architecture, Intel
> + 2) IA-32 Intel(R) Architecture Software Developer's Manual Volume 2:Instruction Set Reference, Intel
> + 3) IA-32 Intel(R) Architecture Software Developer's Manual Volume 3:System Programmer's Guide, Intel
> +
> +Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> +
> +This program and the accompanying materials
> +are licensed and made available under the terms and conditions of the BSD License
> +which accompanies this distribution. The full text of the license may be found at
> +http://opensource.org/licenses/bsd-license.php
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#ifndef __VIRTUAL_MEMORY__
> +#define __VIRTUAL_MEMORY__
> +
> +#include <Uefi.h>
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +//#include <Library/UefiBootServicesTableLib.h>
> +//#include <Library/UefiRuntimeServicesTableLib.h>
> +
> +#include <Library/CacheMaintenanceLib.h>
> +#define SYS_CODE64_SEL 0x38
> +
> +#pragma pack(1)
> +
> +//
> +// Page-Map Level-4 Offset (PML4) and
> +// Page-Directory-Pointer Offset (PDPE) entries 4K & 2MB
> +//
> +
> +typedef union {
> + struct {
> + UINT64 Present:1; // 0 = Not present in memory, 1 = Present in memory
> + UINT64 ReadWrite:1; // 0 = Read-Only, 1= Read/Write
> + UINT64 UserSupervisor:1; // 0 = Supervisor, 1=User
> + UINT64 WriteThrough:1; // 0 = Write-Back caching, 1=Write-Through caching
> + UINT64 CacheDisabled:1; // 0 = Cached, 1=Non-Cached
> + UINT64 Accessed:1; // 0 = Not accessed, 1 = Accessed (set by CPU)
> + UINT64 Reserved:1; // Reserved
> + UINT64 MustBeZero:2; // Must Be Zero
> + UINT64 Available:3; // Available for use by system software
> + UINT64 PageTableBaseAddress:40; // Page Table Base Address
> + UINT64 AvabilableHigh:11; // Available for use by system software
> + UINT64 Nx:1; // No Execute bit
> + } Bits;
> + UINT64 Uint64;
> +} PAGE_MAP_AND_DIRECTORY_POINTER;
> +
> +//
> +// Page Table Entry 4KB
> +//
> +typedef union {
> + struct {
> + UINT64 Present:1; // 0 = Not present in memory, 1 = Present in memory
> + UINT64 ReadWrite:1; // 0 = Read-Only, 1= Read/Write
> + UINT64 UserSupervisor:1; // 0 = Supervisor, 1=User
> + UINT64 WriteThrough:1; // 0 = Write-Back caching, 1=Write-Through caching
> + UINT64 CacheDisabled:1; // 0 = Cached, 1=Non-Cached
> + UINT64 Accessed:1; // 0 = Not accessed, 1 = Accessed (set by CPU)
> + UINT64 Dirty:1; // 0 = Not Dirty, 1 = written by processor on access to page
> + UINT64 PAT:1; //
> + UINT64 Global:1; // 0 = Not global page, 1 = global page TLB not cleared on CR3 write
> + UINT64 Available:3; // Available for use by system software
> + UINT64 PageTableBaseAddress:40; // Page Table Base Address
> + UINT64 AvabilableHigh:11; // Available for use by system software
> + UINT64 Nx:1; // 0 = Execute Code, 1 = No Code Execution
> + } Bits;
> + UINT64 Uint64;
> +} PAGE_TABLE_4K_ENTRY;
> +
> +//
> +// Page Table Entry 2MB
> +//
> +typedef union {
> + struct {
> + UINT64 Present:1; // 0 = Not present in memory, 1 = Present in memory
> + UINT64 ReadWrite:1; // 0 = Read-Only, 1= Read/Write
> + UINT64 UserSupervisor:1; // 0 = Supervisor, 1=User
> + UINT64 WriteThrough:1; // 0 = Write-Back caching, 1=Write-Through caching
> + UINT64 CacheDisabled:1; // 0 = Cached, 1=Non-Cached
> + UINT64 Accessed:1; // 0 = Not accessed, 1 = Accessed (set by CPU)
> + UINT64 Dirty:1; // 0 = Not Dirty, 1 = written by processor on access to page
> + UINT64 MustBe1:1; // Must be 1
> + UINT64 Global:1; // 0 = Not global page, 1 = global page TLB not cleared on CR3 write
> + UINT64 Available:3; // Available for use by system software
> + UINT64 PAT:1; //
> + UINT64 MustBeZero:8; // Must be zero;
> + UINT64 PageTableBaseAddress:31; // Page Table Base Address
> + UINT64 AvabilableHigh:11; // Available for use by system software
> + UINT64 Nx:1; // 0 = Execute Code, 1 = No Code Execution
> + } Bits;
> + UINT64 Uint64;
> +} PAGE_TABLE_ENTRY;
> +
> +//
> +// Page Table Entry 1GB
> +//
> +typedef union {
> + struct {
> + UINT64 Present:1; // 0 = Not present in memory, 1 = Present in memory
> + UINT64 ReadWrite:1; // 0 = Read-Only, 1= Read/Write
> + UINT64 UserSupervisor:1; // 0 = Supervisor, 1=User
> + UINT64 WriteThrough:1; // 0 = Write-Back caching, 1=Write-Through caching
> + UINT64 CacheDisabled:1; // 0 = Cached, 1=Non-Cached
> + UINT64 Accessed:1; // 0 = Not accessed, 1 = Accessed (set by CPU)
> + UINT64 Dirty:1; // 0 = Not Dirty, 1 = written by processor on access to page
> + UINT64 MustBe1:1; // Must be 1
> + UINT64 Global:1; // 0 = Not global page, 1 = global page TLB not cleared on CR3 write
> + UINT64 Available:3; // Available for use by system software
> + UINT64 PAT:1; //
> + UINT64 MustBeZero:17; // Must be zero;
> + UINT64 PageTableBaseAddress:22; // Page Table Base Address
> + UINT64 AvabilableHigh:11; // Available for use by system software
> + UINT64 Nx:1; // 0 = Execute Code, 1 = No Code Execution
> + } Bits;
> + UINT64 Uint64;
> +} PAGE_TABLE_1G_ENTRY;
> +
> +#pragma pack()
> +
> +#define IA32_PG_P BIT0
> +#define IA32_PG_RW BIT1
> +
> +#define PAGETABLE_ENTRY_MASK ((1UL << 9) - 1)
> +#define PML4_OFFSET(x) ( (x >> 39) & PAGETABLE_ENTRY_MASK)
> +#define PDP_OFFSET(x) ( (x >> 30) & PAGETABLE_ENTRY_MASK)
> +#define PDE_OFFSET(x) ( (x >> 21) & PAGETABLE_ENTRY_MASK)
> +#define PTE_OFFSET(x) ( (x >> 12) & PAGETABLE_ENTRY_MASK)
> +#define PAGING_1G_ADDRESS_MASK_64 0x000FFFFFC0000000ull
> +
> +EFI_STATUS
> +EFIAPI
> +Set_Memory_Decrypted (
> + IN EFI_PHYSICAL_ADDRESS PhysicalAddress,
> + IN UINT64 Length
> + );
> +
> +EFI_STATUS
> +EFIAPI
> +Set_Memory_Encrypted (
> + IN EFI_PHYSICAL_ADDRESS PhysicalAddress,
> + IN UINT64 Length
> + );
> +
> +#endif
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 769251d..c2821b7 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -126,6 +126,7 @@
> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
> VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
> LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
> + MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
> !if $(SMM_REQUIRE) == FALSE
> LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
> !endif
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 3874c35..1dd8064 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -131,6 +131,7 @@
> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
> VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
> LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
> + MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
> !if $(SMM_REQUIRE) == FALSE
> LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
> !endif
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index fe7f086..06bee32 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -131,6 +131,7 @@
> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
> VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
> LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
> + MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
> !if $(SMM_REQUIRE) == FALSE
> LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
> !endif
>
Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 03/27/17 11:19, Laszlo Ersek wrote: > On 03/21/17 22:13, Brijesh Singh wrote: >> + Returns a boolean to indicate whether SEV is enabled >> + >> + @retval TRUE When SEV is active >> + @retval FALSE When SEV is not enabled >> + **/ >> +BOOLEAN >> +EFIAPI >> +MemEncryptSevIsEnabled ( >> + VOID >> + ); > > Would it make sense to call this library function in PlatformPei, rather > than add a separate SevIsEnabled() function to it (in patch #3)? The > implementations look nearly identical. I realize that earlier I seemingly suggested the opposite: http://mid.mail-archive.com/dd9436dc-415c-9fab-081c-39dd2cd71fd5@redhat.com http://mid.mail-archive.com/9193d837-6a78-b1c4-42c0-427fbc1f2364@redhat.com However, at that time, my understanding was that this library would only be used in PlatformPei (hence the single user wouldn't justify the new library instance). Now it seems that there are going to be several client modules that check on SEV enablement. Is that right? Thanks Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Mon, Mar 27, 2017 at 5:07 AM, Laszlo Ersek <lersek@redhat.com> wrote: > On 03/27/17 11:19, Laszlo Ersek wrote: > > On 03/21/17 22:13, Brijesh Singh wrote: > > >> + Returns a boolean to indicate whether SEV is enabled > >> + > >> + @retval TRUE When SEV is active > >> + @retval FALSE When SEV is not enabled > >> + **/ > >> +BOOLEAN > >> +EFIAPI > >> +MemEncryptSevIsEnabled ( > >> + VOID > >> + ); > > > > Would it make sense to call this library function in PlatformPei, rather > > than add a separate SevIsEnabled() function to it (in patch #3)? The > > implementations look nearly identical. > > I realize that earlier I seemingly suggested the opposite: > > http://mid.mail-archive.com/dd9436dc-415c-9fab-081c- > 39dd2cd71fd5@redhat.com > > http://mid.mail-archive.com/9193d837-6a78-b1c4-42c0- > 427fbc1f2364@redhat.com > > However, at that time, my understanding was that this library would only > be used in PlatformPei (hence the single user wouldn't justify the new > library instance). Now it seems that there are going to be several > client modules that check on SEV enablement. Is that right? > > Yes, I do expect several client module link against this library to check whether the SEV is enabled. Are you okay if we link MemEncryptSevLib in PlatformPei and make use of MemEncryptSevIsEnabled() routine instead of having a local copy ? I was not sure which way to go hence I still have PlatformPei and QemuFwCfgPei using the local implementation of the same functions. My personal perference would be to link with MemEncryptSevLib instead of having local function. But as always I am open to suggestions. Thanks Brijesh _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 03/27/17 20:44, Brijesh Singh wrote: > On Mon, Mar 27, 2017 at 5:07 AM, Laszlo Ersek <lersek@redhat.com> wrote: > >> On 03/27/17 11:19, Laszlo Ersek wrote: >>> On 03/21/17 22:13, Brijesh Singh wrote: >> >>>> + Returns a boolean to indicate whether SEV is enabled >>>> + >>>> + @retval TRUE When SEV is active >>>> + @retval FALSE When SEV is not enabled >>>> + **/ >>>> +BOOLEAN >>>> +EFIAPI >>>> +MemEncryptSevIsEnabled ( >>>> + VOID >>>> + ); >>> >>> Would it make sense to call this library function in PlatformPei, rather >>> than add a separate SevIsEnabled() function to it (in patch #3)? The >>> implementations look nearly identical. >> >> I realize that earlier I seemingly suggested the opposite: >> >> http://mid.mail-archive.com/dd9436dc-415c-9fab-081c- >> 39dd2cd71fd5@redhat.com >> >> http://mid.mail-archive.com/9193d837-6a78-b1c4-42c0- >> 427fbc1f2364@redhat.com >> >> However, at that time, my understanding was that this library would only >> be used in PlatformPei (hence the single user wouldn't justify the new >> library instance). Now it seems that there are going to be several >> client modules that check on SEV enablement. Is that right? >> >> > > Yes, I do expect several client module link against this library to check > whether the SEV is enabled. > Are you okay if we link MemEncryptSevLib in PlatformPei and make use of > MemEncryptSevIsEnabled() > routine instead of having a local copy ? I was not sure which way to go > hence I still have PlatformPei > and QemuFwCfgPei using the local implementation of the same functions. My > personal perference would > be to link with MemEncryptSevLib instead of having local function. But as > always I am open to suggestions. I think the library function should be used (caching the CPUID detection results) whenever we have writeable memory (PEI and onwards). Thanks Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.