* [PATCH v3 1/4] MdeModulePkg: Add PCD PcdPteMemoryEncryptionAddressOrMask
2017-02-16 21:02 [PATCH v3 0/4] Add PCD PcdPteMemoryEncryptionAddressOrMask Leo Duran
@ 2017-02-16 21:02 ` Leo Duran
2017-02-16 21:02 ` [PATCH v3 2/4] MdeModulePkg/Universal/CapsulePei: Add support for " Leo Duran
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Leo Duran @ 2017-02-16 21:02 UTC (permalink / raw)
To: edk2-devel; +Cc: Leo Duran, Feng Tian, Star Zeng, Laszlo Ersek, Brijesh Singh
This PCD holds the address mask for page table entries when memory
encryption is enabled on AMD processors supporting the Secure Encrypted
Virtualization (SEV) feature.
Cc: Feng Tian <feng.tian@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Leo Duran <leo.duran@amd.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
---
MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 5 +++-
MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 29 +++++++++++++++++-------
MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h | 6 +++++
MdeModulePkg/MdeModulePkg.dec | 8 +++++++
4 files changed, 39 insertions(+), 9 deletions(-)
diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
index 2bc41be..d62bd9b 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
@@ -6,6 +6,8 @@
# needed to run the DXE Foundation.
#
# 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
@@ -111,7 +113,8 @@
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSupportUefiDecompress ## CONSUMES
[Pcd.IA32,Pcd.X64]
- gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable ## SOMETIMES_CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable ## SOMETIMES_CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask ## CONSUMES
[Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64]
gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack ## SOMETIMES_CONSUMES
diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
index 790f6ab..89b116a 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
@@ -16,6 +16,8 @@
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
@@ -65,20 +67,24 @@ Split2MPageTo4K (
EFI_PHYSICAL_ADDRESS PhysicalAddress4K;
UINTN IndexOfPageTableEntries;
PAGE_TABLE_4K_ENTRY *PageTableEntry;
+ UINT64 AddressSetMask;
PageTableEntry = AllocatePages (1);
ASSERT (PageTableEntry != NULL);
+
+ AddressSetMask = PcdGet64 (PcdPteMemoryEncryptionAddressOrMask);
+
//
// Fill in 2M page entry.
//
- *PageEntry2M = (UINT64) (UINTN) PageTableEntry | IA32_PG_P | IA32_PG_RW;
+ *PageEntry2M = (UINT64)(UINTN)PageTableEntry | (AddressSetMask & PAGING_2M_ADDRESS_MASK_64) | IA32_PG_P | IA32_PG_RW;
PhysicalAddress4K = PhysicalAddress;
for (IndexOfPageTableEntries = 0; IndexOfPageTableEntries < 512; IndexOfPageTableEntries++, PageTableEntry++, PhysicalAddress4K += SIZE_4KB) {
//
// Fill in the Page Table entries
//
- PageTableEntry->Uint64 = (UINT64) PhysicalAddress4K;
+ PageTableEntry->Uint64 = (UINT64)PhysicalAddress4K | (AddressSetMask & PAGING_4K_ADDRESS_MASK_64);
PageTableEntry->Bits.ReadWrite = 1;
PageTableEntry->Bits.Present = 1;
if ((PhysicalAddress4K >= StackBase) && (PhysicalAddress4K < StackBase + StackSize)) {
@@ -110,13 +116,17 @@ Split1GPageTo2M (
EFI_PHYSICAL_ADDRESS PhysicalAddress2M;
UINTN IndexOfPageDirectoryEntries;
PAGE_TABLE_ENTRY *PageDirectoryEntry;
+ UINT64 AddressSetMask;
PageDirectoryEntry = AllocatePages (1);
ASSERT (PageDirectoryEntry != NULL);
+
+ AddressSetMask = PcdGet64 (PcdPteMemoryEncryptionAddressOrMask);
+
//
// Fill in 1G page entry.
//
- *PageEntry1G = (UINT64) (UINTN) PageDirectoryEntry | IA32_PG_P | IA32_PG_RW;
+ *PageEntry1G = (UINT64)(UINTN)PageDirectoryEntry | (AddressSetMask & PAGING_1G_ADDRESS_MASK_64) | IA32_PG_P | IA32_PG_RW;
PhysicalAddress2M = PhysicalAddress;
for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512; IndexOfPageDirectoryEntries++, PageDirectoryEntry++, PhysicalAddress2M += SIZE_2MB) {
@@ -129,7 +139,7 @@ Split1GPageTo2M (
//
// Fill in the Page Directory entries
//
- PageDirectoryEntry->Uint64 = (UINT64) PhysicalAddress2M;
+ PageDirectoryEntry->Uint64 = (UINT64)PhysicalAddress2M | (AddressSetMask & PAGING_2M_ADDRESS_MASK_64);
PageDirectoryEntry->Bits.ReadWrite = 1;
PageDirectoryEntry->Bits.Present = 1;
PageDirectoryEntry->Bits.MustBe1 = 1;
@@ -171,6 +181,7 @@ CreateIdentityMappingPageTables (
VOID *Hob;
BOOLEAN Page1GSupport;
PAGE_TABLE_1G_ENTRY *PageDirectory1GEntry;
+ UINT64 AddressSetMask;
Page1GSupport = FALSE;
if (PcdGetBool(PcdUse1GPageTable)) {
@@ -229,6 +240,8 @@ CreateIdentityMappingPageTables (
BigPageAddress = (UINTN) AllocatePages (TotalPagesNum);
ASSERT (BigPageAddress != 0);
+ AddressSetMask = PcdGet64 (PcdPteMemoryEncryptionAddressOrMask);
+
//
// By architecture only one PageMapLevel4 exists - so lets allocate storage for it.
//
@@ -248,7 +261,7 @@ CreateIdentityMappingPageTables (
//
// Make a PML4 Entry
//
- PageMapLevel4Entry->Uint64 = (UINT64)(UINTN)PageDirectoryPointerEntry;
+ PageMapLevel4Entry->Uint64 = (UINT64)(UINTN)PageDirectoryPointerEntry | (AddressSetMask & PAGING_4K_ADDRESS_MASK_64);
PageMapLevel4Entry->Bits.ReadWrite = 1;
PageMapLevel4Entry->Bits.Present = 1;
@@ -262,7 +275,7 @@ CreateIdentityMappingPageTables (
//
// Fill in the Page Directory entries
//
- PageDirectory1GEntry->Uint64 = (UINT64)PageAddress;
+ PageDirectory1GEntry->Uint64 = (UINT64)PageAddress | (AddressSetMask & PAGING_1G_ADDRESS_MASK_64);
PageDirectory1GEntry->Bits.ReadWrite = 1;
PageDirectory1GEntry->Bits.Present = 1;
PageDirectory1GEntry->Bits.MustBe1 = 1;
@@ -280,7 +293,7 @@ CreateIdentityMappingPageTables (
//
// Fill in a Page Directory Pointer Entries
//
- PageDirectoryPointerEntry->Uint64 = (UINT64)(UINTN)PageDirectoryEntry;
+ PageDirectoryPointerEntry->Uint64 = (UINT64)(UINTN)PageDirectoryEntry | (AddressSetMask & PAGING_4K_ADDRESS_MASK_64);
PageDirectoryPointerEntry->Bits.ReadWrite = 1;
PageDirectoryPointerEntry->Bits.Present = 1;
@@ -294,7 +307,7 @@ CreateIdentityMappingPageTables (
//
// Fill in the Page Directory entries
//
- PageDirectoryEntry->Uint64 = (UINT64)PageAddress;
+ PageDirectoryEntry->Uint64 = (UINT64)PageAddress | (AddressSetMask & PAGING_2M_ADDRESS_MASK_64);
PageDirectoryEntry->Bits.ReadWrite = 1;
PageDirectoryEntry->Bits.Present = 1;
PageDirectoryEntry->Bits.MustBe1 = 1;
diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h
index 20c31f5..2e0691e 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h
@@ -8,6 +8,8 @@
4) AMD64 Architecture Programmer's Manual Volume 2: System Programming
Copyright (c) 2006 - 2015, 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
@@ -23,6 +25,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#define SYS_CODE64_SEL 0x38
+#define PAGING_4K_ADDRESS_MASK_64 0x000FFFFFFFFFF000ull
+#define PAGING_2M_ADDRESS_MASK_64 0x000FFFFFFFE00000ull
+#define PAGING_1G_ADDRESS_MASK_64 0x000FFFFFC0000000ull
+
#pragma pack(1)
typedef union {
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 273cd7e..15119d0 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -6,6 +6,8 @@
# Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.<BR>
# Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
# (C) Copyright 2016 Hewlett Packard Enterprise Development LP<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 that accompanies this distribution.
# The full text of the license may be found at
@@ -1682,6 +1684,12 @@
# @Prompt A list of system FMP ImageTypeId GUIDs
gEfiMdeModulePkgTokenSpaceGuid.PcdSystemFmpCapsuleImageTypeIdGuid|{0x0}|VOID*|0x30001046
+ ## This PCD holds the address mask for page table entries when memory encryption is
+ # enabled on AMD processors supporting the Secure Encrypted Virtualization (SEV) feature.
+ # This mask should be applied when creating 1:1 virtual to physical mapping tables.
+ #
+ gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask|0x0|UINT64|0x30001047
+
[PcdsPatchableInModule]
## Specify memory size with page number for PEI code when
# Loading Module at Fixed Address feature is enabled.
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 2/4] MdeModulePkg/Universal/CapsulePei: Add support for PCD PcdPteMemoryEncryptionAddressOrMask
2017-02-16 21:02 [PATCH v3 0/4] Add PCD PcdPteMemoryEncryptionAddressOrMask Leo Duran
2017-02-16 21:02 ` [PATCH v3 1/4] MdeModulePkg: " Leo Duran
@ 2017-02-16 21:02 ` Leo Duran
2017-02-20 6:04 ` Zeng, Star
2017-02-16 21:02 ` [PATCH v3 3/4] UefiCpuPkg/Universal/Acpi/S3Resume2Pei: " Leo Duran
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Leo Duran @ 2017-02-16 21:02 UTC (permalink / raw)
To: edk2-devel; +Cc: Leo Duran, Feng Tian, Star Zeng, Laszlo Ersek, Brijesh Singh
This PCD holds the address mask for page table entries when memory
encryption is enabled on AMD processors supporting the Secure Encrypted
Virtualization (SEV) feature.
The mask is applied when 4GB tables are created (UefiCapsule.c), and when
the tables are expanded on-demand by page-faults above 4GB's (X64Entry.c).
Cc: Feng Tian <feng.tian@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leo Duran <leo.duran@amd.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
---
MdeModulePkg/Universal/CapsulePei/CapsulePei.inf | 2 ++
| 7 +++++++
MdeModulePkg/Universal/CapsulePei/UefiCapsule.c | 13 ++++++++----
MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c | 23 +++++++++++++++-------
4 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
index d2ca0d0..c54bc21 100644
--- a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
+++ b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
@@ -7,6 +7,7 @@
# buffer overflow, integer overflow.
#
# 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
@@ -76,6 +77,7 @@
[Pcd.IA32]
gEfiMdeModulePkgTokenSpaceGuid.PcdCapsuleCoalesceFile ## SOMETIMES_CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable ## SOMETIMES_CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask ## CONSUMES
[FeaturePcd.IA32]
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode ## CONSUMES
--git a/MdeModulePkg/Universal/CapsulePei/Common/CommonHeader.h b/MdeModulePkg/Universal/CapsulePei/Common/CommonHeader.h
index 7298874..0a9761e 100644
--- a/MdeModulePkg/Universal/CapsulePei/Common/CommonHeader.h
+++ b/MdeModulePkg/Universal/CapsulePei/Common/CommonHeader.h
@@ -2,6 +2,8 @@
Common header file.
Copyright (c) 2011 - 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
@@ -20,6 +22,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
//
#define EXTRA_PAGE_TABLE_PAGES 8
+#define PAGING_4K_ADDRESS_MASK_64 0x000FFFFFFFFFF000ull
+#define PAGING_2M_ADDRESS_MASK_64 0x000FFFFFFFE00000ull
+#define PAGING_1G_ADDRESS_MASK_64 0x000FFFFFC0000000ull
+
//
// This capsule PEIM puts its private data at the start of the
// coalesced capsule. Here's the structure definition.
@@ -60,6 +66,7 @@ typedef struct {
EFI_PHYSICAL_ADDRESS MemoryBase64Ptr;
EFI_PHYSICAL_ADDRESS MemorySize64Ptr;
BOOLEAN Page1GSupport;
+ UINT64 PteMemoryEncryptionAddressOrMask;
} SWITCH_32_TO_64_CONTEXT;
typedef struct {
diff --git a/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c b/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
index 9ac9d22..7c651f6 100644
--- a/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
+++ b/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
@@ -2,6 +2,7 @@
Capsule update PEIM for UEFI2.0
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
@@ -145,6 +146,7 @@ Create4GPageTables (
PAGE_TABLE_ENTRY *PageDirectoryEntry;
UINTN BigPageAddress;
PAGE_TABLE_1G_ENTRY *PageDirectory1GEntry;
+ UINT64 AddressSetMask;
//
// Create 4G page table by default,
@@ -168,6 +170,8 @@ Create4GPageTables (
//
BigPageAddress = (UINTN) PageTablesAddress;
+ AddressSetMask = PcdGet64 (PcdPteMemoryEncryptionAddressOrMask);
+
//
// By architecture only one PageMapLevel4 exists - so lets allocate storage for it.
//
@@ -187,7 +191,7 @@ Create4GPageTables (
//
// Make a PML4 Entry
//
- PageMapLevel4Entry->Uint64 = (UINT64)(UINTN)PageDirectoryPointerEntry;
+ PageMapLevel4Entry->Uint64 = (UINT64)(UINTN)PageDirectoryPointerEntry | (AddressSetMask & PAGING_4K_ADDRESS_MASK_64);
PageMapLevel4Entry->Bits.ReadWrite = 1;
PageMapLevel4Entry->Bits.Present = 1;
@@ -198,7 +202,7 @@ Create4GPageTables (
//
// Fill in the Page Directory entries
//
- PageDirectory1GEntry->Uint64 = (UINT64)PageAddress;
+ PageDirectory1GEntry->Uint64 = (UINT64)PageAddress | (AddressSetMask & PAGING_1G_ADDRESS_MASK_64);
PageDirectory1GEntry->Bits.ReadWrite = 1;
PageDirectory1GEntry->Bits.Present = 1;
PageDirectory1GEntry->Bits.MustBe1 = 1;
@@ -215,7 +219,7 @@ Create4GPageTables (
//
// Fill in a Page Directory Pointer Entries
//
- PageDirectoryPointerEntry->Uint64 = (UINT64)(UINTN)PageDirectoryEntry;
+ PageDirectoryPointerEntry->Uint64 = (UINT64)(UINTN)PageDirectoryEntry | (AddressSetMask & PAGING_4K_ADDRESS_MASK_64);
PageDirectoryPointerEntry->Bits.ReadWrite = 1;
PageDirectoryPointerEntry->Bits.Present = 1;
@@ -223,7 +227,7 @@ Create4GPageTables (
//
// Fill in the Page Directory entries
//
- PageDirectoryEntry->Uint64 = (UINT64)PageAddress;
+ PageDirectoryEntry->Uint64 = (UINT64)PageAddress | (AddressSetMask & PAGING_2M_ADDRESS_MASK_64);
PageDirectoryEntry->Bits.ReadWrite = 1;
PageDirectoryEntry->Bits.Present = 1;
PageDirectoryEntry->Bits.MustBe1 = 1;
@@ -443,6 +447,7 @@ ModeSwitch (
Context.MemoryBase64Ptr = (EFI_PHYSICAL_ADDRESS)(UINTN)&MemoryBase64;
Context.MemorySize64Ptr = (EFI_PHYSICAL_ADDRESS)(UINTN)&MemorySize64;
Context.Page1GSupport = Page1GSupport;
+ Context.PteMemoryEncryptionAddressOrMask = PcdGet64 (PcdPteMemoryEncryptionAddressOrMask);
//
// Prepare data for return back
diff --git a/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c b/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
index 5ad95d2..2197502 100644
--- a/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
+++ b/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
@@ -2,6 +2,8 @@
The X64 entrypoint is used to process capsule in long mode.
Copyright (c) 2011 - 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
@@ -29,6 +31,7 @@ typedef struct _PAGE_FAULT_CONTEXT {
UINT64 PhyMask;
UINTN PageFaultBuffer;
UINTN PageFaultIndex;
+ UINT64 PteMemoryEncryptionAddressOrMask;
//
// Store the uplink information for each page being used.
//
@@ -114,6 +117,7 @@ AcquirePage (
)
{
UINTN Address;
+ UINT64 AddressSetMask;
Address = PageFaultContext->PageFaultBuffer + EFI_PAGES_TO_SIZE (PageFaultContext->PageFaultIndex);
ZeroMem ((VOID *) Address, EFI_PAGES_TO_SIZE (1));
@@ -121,14 +125,16 @@ AcquirePage (
//
// Cut the previous uplink if it exists and wasn't overwritten.
//
- if ((PageFaultContext->PageFaultUplink[PageFaultContext->PageFaultIndex] != NULL) && ((*PageFaultContext->PageFaultUplink[PageFaultContext->PageFaultIndex] & PageFaultContext->PhyMask) == Address)) {
+ if ((PageFaultContext->PageFaultUplink[PageFaultContext->PageFaultIndex] != NULL) &&
+ ((*PageFaultContext->PageFaultUplink[PageFaultContext->PageFaultIndex] & PageFaultContext->PhyMask) == Address)) {
*PageFaultContext->PageFaultUplink[PageFaultContext->PageFaultIndex] = 0;
}
//
// Link & Record the current uplink.
//
- *Uplink = Address | IA32_PG_P | IA32_PG_RW;
+ AddressSetMask = PageFaultContext->PteMemoryEncryptionAddressOrMask;
+ *Uplink = Address | (AddressSetMask & PAGING_4K_ADDRESS_MASK_64) | IA32_PG_P | IA32_PG_RW;
PageFaultContext->PageFaultUplink[PageFaultContext->PageFaultIndex] = Uplink;
PageFaultContext->PageFaultIndex = (PageFaultContext->PageFaultIndex + 1) % EXTRA_PAGE_TABLE_PAGES;
@@ -153,6 +159,7 @@ PageFaultHandler (
UINT64 *PageTable;
UINT64 PFAddress;
UINTN PTIndex;
+ UINT64 AddressSetMask;
//
// Get the IDT Descriptor.
@@ -163,6 +170,7 @@ PageFaultHandler (
//
PageFaultContext = (PAGE_FAULT_CONTEXT *) (UINTN) (Idtr.Base - sizeof (PAGE_FAULT_CONTEXT));
PhyMask = PageFaultContext->PhyMask;
+ AddressSetMask = PageFaultContext->PteMemoryEncryptionAddressOrMask;
PFAddress = AsmReadCr2 ();
DEBUG ((EFI_D_ERROR, "CapsuleX64 - PageFaultHandler: Cr2 - %lx\n", PFAddress));
@@ -179,19 +187,19 @@ PageFaultHandler (
if ((PageTable[PTIndex] & IA32_PG_P) == 0) {
AcquirePage (PageFaultContext, &PageTable[PTIndex]);
}
- PageTable = (UINT64*)(UINTN)(PageTable[PTIndex] & PhyMask);
+ PageTable = (UINT64*)(UINTN)((PageTable[PTIndex] & ~(AddressSetMask & PAGING_4K_ADDRESS_MASK_64)) & PhyMask);
PTIndex = BitFieldRead64 (PFAddress, 30, 38);
// PDPTE
if (PageFaultContext->Page1GSupport) {
- PageTable[PTIndex] = (PFAddress & ~((1ull << 30) - 1)) | IA32_PG_P | IA32_PG_RW | IA32_PG_PS;
+ PageTable[PTIndex] = ((PFAddress | (AddressSetMask & PAGING_1G_ADDRESS_MASK_64)) & ~((1ull << 30) - 1)) | IA32_PG_P | IA32_PG_RW | IA32_PG_PS;
} else {
if ((PageTable[PTIndex] & IA32_PG_P) == 0) {
AcquirePage (PageFaultContext, &PageTable[PTIndex]);
}
- PageTable = (UINT64*)(UINTN)(PageTable[PTIndex] & PhyMask);
+ PageTable = (UINT64*)(UINTN)((PageTable[PTIndex] & ~(AddressSetMask & PAGING_4K_ADDRESS_MASK_64)) & PhyMask);
PTIndex = BitFieldRead64 (PFAddress, 21, 29);
// PD
- PageTable[PTIndex] = (PFAddress & ~((1ull << 21) - 1)) | IA32_PG_P | IA32_PG_RW | IA32_PG_PS;
+ PageTable[PTIndex] = ((PFAddress | (AddressSetMask & PAGING_2M_ADDRESS_MASK_64)) & ~((1ull << 21) - 1)) | IA32_PG_P | IA32_PG_RW | IA32_PG_PS;
}
return NULL;
@@ -244,6 +252,7 @@ _ModuleEntryPoint (
// Hook page fault handler to handle >4G request.
//
PageFaultIdtTable.PageFaultContext.Page1GSupport = EntrypointContext->Page1GSupport;
+ PageFaultIdtTable.PageFaultContext.PteMemoryEncryptionAddressOrMask = EntrypointContext->PteMemoryEncryptionAddressOrMask;
IdtEntry = (IA32_IDT_GATE_DESCRIPTOR *) (X64Idtr.Base + (14 * sizeof (IA32_IDT_GATE_DESCRIPTOR)));
HookPageFaultHandler (IdtEntry, &(PageFaultIdtTable.PageFaultContext));
@@ -298,4 +307,4 @@ _ModuleEntryPoint (
//
ASSERT (FALSE);
return EFI_SUCCESS;
-}
\ No newline at end of file
+}
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/4] MdeModulePkg/Universal/CapsulePei: Add support for PCD PcdPteMemoryEncryptionAddressOrMask
2017-02-16 21:02 ` [PATCH v3 2/4] MdeModulePkg/Universal/CapsulePei: Add support for " Leo Duran
@ 2017-02-20 6:04 ` Zeng, Star
2017-02-21 16:42 ` Duran, Leo
0 siblings, 1 reply; 10+ messages in thread
From: Zeng, Star @ 2017-02-20 6:04 UTC (permalink / raw)
To: Leo Duran, edk2-devel; +Cc: Laszlo Ersek, Feng Tian, Brijesh Singh, star.zeng
Leo,
Comments added inline.
On 2017/2/17 5:02, Leo Duran wrote:
> This PCD holds the address mask for page table entries when memory
> encryption is enabled on AMD processors supporting the Secure Encrypted
> Virtualization (SEV) feature.
>
> The mask is applied when 4GB tables are created (UefiCapsule.c), and when
> the tables are expanded on-demand by page-faults above 4GB's (X64Entry.c).
>
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Leo Duran <leo.duran@amd.com>
> Reviewed-by: Star Zeng <star.zeng@intel.com>
> ---
> MdeModulePkg/Universal/CapsulePei/CapsulePei.inf | 2 ++
> .../Universal/CapsulePei/Common/CommonHeader.h | 7 +++++++
> MdeModulePkg/Universal/CapsulePei/UefiCapsule.c | 13 ++++++++----
> MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c | 23 +++++++++++++++-------
> 4 files changed, 34 insertions(+), 11 deletions(-)
>
[snipped]
> diff --git a/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c b/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> index 5ad95d2..2197502 100644
> --- a/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> +++ b/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> @@ -2,6 +2,8 @@
> The X64 entrypoint is used to process capsule in long mode.
>
> Copyright (c) 2011 - 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
> @@ -29,6 +31,7 @@ typedef struct _PAGE_FAULT_CONTEXT {
> UINT64 PhyMask;
> UINTN PageFaultBuffer;
> UINTN PageFaultIndex;
> + UINT64 PteMemoryEncryptionAddressOrMask;
> //
> // Store the uplink information for each page being used.
> //
> @@ -114,6 +117,7 @@ AcquirePage (
> )
> {
> UINTN Address;
> + UINT64 AddressSetMask;
>
> Address = PageFaultContext->PageFaultBuffer + EFI_PAGES_TO_SIZE (PageFaultContext->PageFaultIndex);
> ZeroMem ((VOID *) Address, EFI_PAGES_TO_SIZE (1));
> @@ -121,14 +125,16 @@ AcquirePage (
> //
> // Cut the previous uplink if it exists and wasn't overwritten.
> //
> - if ((PageFaultContext->PageFaultUplink[PageFaultContext->PageFaultIndex] != NULL) && ((*PageFaultContext->PageFaultUplink[PageFaultContext->PageFaultIndex] & PageFaultContext->PhyMask) == Address)) {
> + if ((PageFaultContext->PageFaultUplink[PageFaultContext->PageFaultIndex] != NULL) &&
> + ((*PageFaultContext->PageFaultUplink[PageFaultContext->PageFaultIndex] & PageFaultContext->PhyMask) == Address)) {
No real change at here except the line feed added.
You were going to update code at here, but forgot to do the real change?
Will you do similar change for [PATCH v3 4/4]
Thanks,
Star
> *PageFaultContext->PageFaultUplink[PageFaultContext->PageFaultIndex] = 0;
> }
>
> //
> // Link & Record the current uplink.
> //
> - *Uplink = Address | IA32_PG_P | IA32_PG_RW;
> + AddressSetMask = PageFaultContext->PteMemoryEncryptionAddressOrMask;
> + *Uplink = Address | (AddressSetMask & PAGING_4K_ADDRESS_MASK_64) | IA32_PG_P | IA32_PG_RW;
> PageFaultContext->PageFaultUplink[PageFaultContext->PageFaultIndex] = Uplink;
>
> PageFaultContext->PageFaultIndex = (PageFaultContext->PageFaultIndex + 1) % EXTRA_PAGE_TABLE_PAGES;
> @@ -153,6 +159,7 @@ PageFaultHandler (
> UINT64 *PageTable;
> UINT64 PFAddress;
> UINTN PTIndex;
> + UINT64 AddressSetMask;
>
> //
> // Get the IDT Descriptor.
> @@ -163,6 +170,7 @@ PageFaultHandler (
> //
> PageFaultContext = (PAGE_FAULT_CONTEXT *) (UINTN) (Idtr.Base - sizeof (PAGE_FAULT_CONTEXT));
> PhyMask = PageFaultContext->PhyMask;
> + AddressSetMask = PageFaultContext->PteMemoryEncryptionAddressOrMask;
>
> PFAddress = AsmReadCr2 ();
> DEBUG ((EFI_D_ERROR, "CapsuleX64 - PageFaultHandler: Cr2 - %lx\n", PFAddress));
> @@ -179,19 +187,19 @@ PageFaultHandler (
> if ((PageTable[PTIndex] & IA32_PG_P) == 0) {
> AcquirePage (PageFaultContext, &PageTable[PTIndex]);
> }
> - PageTable = (UINT64*)(UINTN)(PageTable[PTIndex] & PhyMask);
> + PageTable = (UINT64*)(UINTN)((PageTable[PTIndex] & ~(AddressSetMask & PAGING_4K_ADDRESS_MASK_64)) & PhyMask);
> PTIndex = BitFieldRead64 (PFAddress, 30, 38);
> // PDPTE
> if (PageFaultContext->Page1GSupport) {
> - PageTable[PTIndex] = (PFAddress & ~((1ull << 30) - 1)) | IA32_PG_P | IA32_PG_RW | IA32_PG_PS;
> + PageTable[PTIndex] = ((PFAddress | (AddressSetMask & PAGING_1G_ADDRESS_MASK_64)) & ~((1ull << 30) - 1)) | IA32_PG_P | IA32_PG_RW | IA32_PG_PS;
> } else {
> if ((PageTable[PTIndex] & IA32_PG_P) == 0) {
> AcquirePage (PageFaultContext, &PageTable[PTIndex]);
> }
> - PageTable = (UINT64*)(UINTN)(PageTable[PTIndex] & PhyMask);
> + PageTable = (UINT64*)(UINTN)((PageTable[PTIndex] & ~(AddressSetMask & PAGING_4K_ADDRESS_MASK_64)) & PhyMask);
> PTIndex = BitFieldRead64 (PFAddress, 21, 29);
> // PD
> - PageTable[PTIndex] = (PFAddress & ~((1ull << 21) - 1)) | IA32_PG_P | IA32_PG_RW | IA32_PG_PS;
> + PageTable[PTIndex] = ((PFAddress | (AddressSetMask & PAGING_2M_ADDRESS_MASK_64)) & ~((1ull << 21) - 1)) | IA32_PG_P | IA32_PG_RW | IA32_PG_PS;
> }
>
> return NULL;
> @@ -244,6 +252,7 @@ _ModuleEntryPoint (
> // Hook page fault handler to handle >4G request.
> //
> PageFaultIdtTable.PageFaultContext.Page1GSupport = EntrypointContext->Page1GSupport;
> + PageFaultIdtTable.PageFaultContext.PteMemoryEncryptionAddressOrMask = EntrypointContext->PteMemoryEncryptionAddressOrMask;
> IdtEntry = (IA32_IDT_GATE_DESCRIPTOR *) (X64Idtr.Base + (14 * sizeof (IA32_IDT_GATE_DESCRIPTOR)));
> HookPageFaultHandler (IdtEntry, &(PageFaultIdtTable.PageFaultContext));
>
> @@ -298,4 +307,4 @@ _ModuleEntryPoint (
> //
> ASSERT (FALSE);
> return EFI_SUCCESS;
> -}
> \ No newline at end of file
> +}
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/4] MdeModulePkg/Universal/CapsulePei: Add support for PCD PcdPteMemoryEncryptionAddressOrMask
2017-02-20 6:04 ` Zeng, Star
@ 2017-02-21 16:42 ` Duran, Leo
2017-02-22 1:20 ` Zeng, Star
0 siblings, 1 reply; 10+ messages in thread
From: Duran, Leo @ 2017-02-21 16:42 UTC (permalink / raw)
To: 'Zeng, Star', edk2-devel@ml01.01.org
Cc: Laszlo Ersek, Feng Tian, Singh, Brijesh
Hi Star,
Please double-check the complete [PATCH v3 2/4].
Yes, there is a non-functional change where I did break a 'very long' line into 2 lines as you noted (I can put that back as it was before if so required).
However the intended functional changes are applied in the rest of the patch in lines where I reference 'AddressSetMask'.
As for [PATCH v3 4/4]
The intended functional changes are applied... please confirm, or please let me know what seems to be missing.
Thanks,
Leo.
> -----Original Message-----
> From: Zeng, Star [mailto:star.zeng@intel.com]
> Sent: Monday, February 20, 2017 12:05 AM
> To: Duran, Leo <leo.duran@amd.com>; edk2-devel@ml01.01.org
> Cc: Laszlo Ersek <lersek@redhat.com>; Feng Tian <feng.tian@intel.com>;
> Singh, Brijesh <brijesh.singh@amd.com>; star.zeng@intel.com
> Subject: Re: [edk2] [PATCH v3 2/4] MdeModulePkg/Universal/CapsulePei:
> Add support for PCD PcdPteMemoryEncryptionAddressOrMask
>
> Leo,
>
> Comments added inline.
>
> On 2017/2/17 5:02, Leo Duran wrote:
> > This PCD holds the address mask for page table entries when memory
> > encryption is enabled on AMD processors supporting the Secure
> > Encrypted Virtualization (SEV) feature.
> >
> > The mask is applied when 4GB tables are created (UefiCapsule.c), and
> > when the tables are expanded on-demand by page-faults above 4GB's
> (X64Entry.c).
> >
> > Cc: Feng Tian <feng.tian@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Brijesh Singh <brijesh.singh@amd.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Leo Duran <leo.duran@amd.com>
> > Reviewed-by: Star Zeng <star.zeng@intel.com>
> > ---
> > MdeModulePkg/Universal/CapsulePei/CapsulePei.inf | 2 ++
> > .../Universal/CapsulePei/Common/CommonHeader.h | 7 +++++++
> > MdeModulePkg/Universal/CapsulePei/UefiCapsule.c | 13 ++++++++----
> > MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c | 23
> +++++++++++++++-------
> > 4 files changed, 34 insertions(+), 11 deletions(-)
> >
>
> [snipped]
>
> > diff --git a/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> > b/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> > index 5ad95d2..2197502 100644
> > --- a/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> > +++ b/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> > @@ -2,6 +2,8 @@
> > The X64 entrypoint is used to process capsule in long mode.
> >
> > Copyright (c) 2011 - 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 @@ -29,6 +31,7 @@ typedef struct _PAGE_FAULT_CONTEXT {
> > UINT64 PhyMask;
> > UINTN PageFaultBuffer;
> > UINTN PageFaultIndex;
> > + UINT64 PteMemoryEncryptionAddressOrMask;
> > //
> > // Store the uplink information for each page being used.
> > //
> > @@ -114,6 +117,7 @@ AcquirePage (
> > )
> > {
> > UINTN Address;
> > + UINT64 AddressSetMask;
> >
> > Address = PageFaultContext->PageFaultBuffer + EFI_PAGES_TO_SIZE
> (PageFaultContext->PageFaultIndex);
> > ZeroMem ((VOID *) Address, EFI_PAGES_TO_SIZE (1)); @@ -121,14
> > +125,16 @@ AcquirePage (
> > //
> > // Cut the previous uplink if it exists and wasn't overwritten.
> > //
> > - if
> > ((PageFaultContext->PageFaultUplink[PageFaultContext->PageFaultIndex]
> > != NULL) &&
> > ((*PageFaultContext->PageFaultUplink[PageFaultContext-
> >PageFaultIndex]
> > & PageFaultContext->PhyMask) == Address)) {
> > + if ((PageFaultContext->PageFaultUplink[PageFaultContext-
> >PageFaultIndex] != NULL) &&
> > +
> > + ((*PageFaultContext->PageFaultUplink[PageFaultContext-
> >PageFaultInde
> > + x] & PageFaultContext->PhyMask) == Address)) {
>
> No real change at here except the line feed added.
> You were going to update code at here, but forgot to do the real change?
>
> Will you do similar change for [PATCH v3 4/4]
>
> Thanks,
> Star
>
> > *PageFaultContext->PageFaultUplink[PageFaultContext-
> >PageFaultIndex] = 0;
> > }
> >
> > //
> > // Link & Record the current uplink.
> > //
> > - *Uplink = Address | IA32_PG_P | IA32_PG_RW;
> > + AddressSetMask =
> > + PageFaultContext->PteMemoryEncryptionAddressOrMask;
> > + *Uplink = Address | (AddressSetMask &
> PAGING_4K_ADDRESS_MASK_64) |
> > + IA32_PG_P | IA32_PG_RW;
> > PageFaultContext->PageFaultUplink[PageFaultContext->PageFaultIndex]
> > = Uplink;
> >
> > PageFaultContext->PageFaultIndex =
> > (PageFaultContext->PageFaultIndex + 1) % EXTRA_PAGE_TABLE_PAGES;
> @@ -153,6 +159,7 @@ PageFaultHandler (
> > UINT64 *PageTable;
> > UINT64 PFAddress;
> > UINTN PTIndex;
> > + UINT64 AddressSetMask;
> >
> > //
> > // Get the IDT Descriptor.
> > @@ -163,6 +170,7 @@ PageFaultHandler (
> > //
> > PageFaultContext = (PAGE_FAULT_CONTEXT *) (UINTN) (Idtr.Base -
> sizeof (PAGE_FAULT_CONTEXT));
> > PhyMask = PageFaultContext->PhyMask;
> > + AddressSetMask =
> > + PageFaultContext->PteMemoryEncryptionAddressOrMask;
> >
> > PFAddress = AsmReadCr2 ();
> > DEBUG ((EFI_D_ERROR, "CapsuleX64 - PageFaultHandler: Cr2 - %lx\n",
> > PFAddress)); @@ -179,19 +187,19 @@ PageFaultHandler (
> > if ((PageTable[PTIndex] & IA32_PG_P) == 0) {
> > AcquirePage (PageFaultContext, &PageTable[PTIndex]);
> > }
> > - PageTable = (UINT64*)(UINTN)(PageTable[PTIndex] & PhyMask);
> > + PageTable = (UINT64*)(UINTN)((PageTable[PTIndex] &
> ~(AddressSetMask
> > + & PAGING_4K_ADDRESS_MASK_64)) & PhyMask);
> > PTIndex = BitFieldRead64 (PFAddress, 30, 38);
> > // PDPTE
> > if (PageFaultContext->Page1GSupport) {
> > - PageTable[PTIndex] = (PFAddress & ~((1ull << 30) - 1)) | IA32_PG_P |
> IA32_PG_RW | IA32_PG_PS;
> > + PageTable[PTIndex] = ((PFAddress | (AddressSetMask &
> > + PAGING_1G_ADDRESS_MASK_64)) & ~((1ull << 30) - 1)) | IA32_PG_P |
> > + IA32_PG_RW | IA32_PG_PS;
> > } else {
> > if ((PageTable[PTIndex] & IA32_PG_P) == 0) {
> > AcquirePage (PageFaultContext, &PageTable[PTIndex]);
> > }
> > - PageTable = (UINT64*)(UINTN)(PageTable[PTIndex] & PhyMask);
> > + PageTable = (UINT64*)(UINTN)((PageTable[PTIndex] &
> > + ~(AddressSetMask & PAGING_4K_ADDRESS_MASK_64)) & PhyMask);
> > PTIndex = BitFieldRead64 (PFAddress, 21, 29);
> > // PD
> > - PageTable[PTIndex] = (PFAddress & ~((1ull << 21) - 1)) | IA32_PG_P |
> IA32_PG_RW | IA32_PG_PS;
> > + PageTable[PTIndex] = ((PFAddress | (AddressSetMask &
> > + PAGING_2M_ADDRESS_MASK_64)) & ~((1ull << 21) - 1)) | IA32_PG_P |
> > + IA32_PG_RW | IA32_PG_PS;
> > }
> >
> > return NULL;
> > @@ -244,6 +252,7 @@ _ModuleEntryPoint (
> > // Hook page fault handler to handle >4G request.
> > //
> > PageFaultIdtTable.PageFaultContext.Page1GSupport =
> > EntrypointContext->Page1GSupport;
> > +
> PageFaultIdtTable.PageFaultContext.PteMemoryEncryptionAddressOrMask
> > + = EntrypointContext->PteMemoryEncryptionAddressOrMask;
> > IdtEntry = (IA32_IDT_GATE_DESCRIPTOR *) (X64Idtr.Base + (14 * sizeof
> (IA32_IDT_GATE_DESCRIPTOR)));
> > HookPageFaultHandler (IdtEntry,
> > &(PageFaultIdtTable.PageFaultContext));
> >
> > @@ -298,4 +307,4 @@ _ModuleEntryPoint (
> > //
> > ASSERT (FALSE);
> > return EFI_SUCCESS;
> > -}
> > \ No newline at end of file
> > +}
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/4] MdeModulePkg/Universal/CapsulePei: Add support for PCD PcdPteMemoryEncryptionAddressOrMask
2017-02-21 16:42 ` Duran, Leo
@ 2017-02-22 1:20 ` Zeng, Star
2017-02-22 15:07 ` Duran, Leo
0 siblings, 1 reply; 10+ messages in thread
From: Zeng, Star @ 2017-02-22 1:20 UTC (permalink / raw)
To: Duran, Leo, edk2-devel@ml01.01.org
Cc: Singh, Brijesh, Tian, Feng, Laszlo Ersek, Zeng, Star
Shouldn't
((*PageFaultContext->PageFaultUplink[PageFaultContext->PageFaultIndex] & PageFaultContext->PhyMask) == Address)
be
(((*PageFaultContext->PageFaultUplink[PageFaultContext->PageFaultIndex] & ~(AddressSetMask & PAGING_4K_ADDRESS_MASK_64)) & PageFaultContext->PhyMask) == Address)
like you did at other place?
Thanks,
Star
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Duran, Leo
Sent: Wednesday, February 22, 2017 12:43 AM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@ml01.01.org
Cc: Singh, Brijesh <brijesh.singh@amd.com>; Tian, Feng <feng.tian@intel.com>; Laszlo Ersek <lersek@redhat.com>
Subject: Re: [edk2] [PATCH v3 2/4] MdeModulePkg/Universal/CapsulePei: Add support for PCD PcdPteMemoryEncryptionAddressOrMask
Hi Star,
Please double-check the complete [PATCH v3 2/4].
Yes, there is a non-functional change where I did break a 'very long' line into 2 lines as you noted (I can put that back as it was before if so required).
However the intended functional changes are applied in the rest of the patch in lines where I reference 'AddressSetMask'.
As for [PATCH v3 4/4]
The intended functional changes are applied... please confirm, or please let me know what seems to be missing.
Thanks,
Leo.
> -----Original Message-----
> From: Zeng, Star [mailto:star.zeng@intel.com]
> Sent: Monday, February 20, 2017 12:05 AM
> To: Duran, Leo <leo.duran@amd.com>; edk2-devel@ml01.01.org
> Cc: Laszlo Ersek <lersek@redhat.com>; Feng Tian <feng.tian@intel.com>;
> Singh, Brijesh <brijesh.singh@amd.com>; star.zeng@intel.com
> Subject: Re: [edk2] [PATCH v3 2/4] MdeModulePkg/Universal/CapsulePei:
> Add support for PCD PcdPteMemoryEncryptionAddressOrMask
>
> Leo,
>
> Comments added inline.
>
> On 2017/2/17 5:02, Leo Duran wrote:
> > This PCD holds the address mask for page table entries when memory
> > encryption is enabled on AMD processors supporting the Secure
> > Encrypted Virtualization (SEV) feature.
> >
> > The mask is applied when 4GB tables are created (UefiCapsule.c), and
> > when the tables are expanded on-demand by page-faults above 4GB's
> (X64Entry.c).
> >
> > Cc: Feng Tian <feng.tian@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Brijesh Singh <brijesh.singh@amd.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Leo Duran <leo.duran@amd.com>
> > Reviewed-by: Star Zeng <star.zeng@intel.com>
> > ---
> > MdeModulePkg/Universal/CapsulePei/CapsulePei.inf | 2 ++
> > .../Universal/CapsulePei/Common/CommonHeader.h | 7 +++++++
> > MdeModulePkg/Universal/CapsulePei/UefiCapsule.c | 13 ++++++++----
> > MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c | 23
> +++++++++++++++-------
> > 4 files changed, 34 insertions(+), 11 deletions(-)
> >
>
> [snipped]
>
> > diff --git a/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> > b/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> > index 5ad95d2..2197502 100644
> > --- a/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> > +++ b/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> > @@ -2,6 +2,8 @@
> > The X64 entrypoint is used to process capsule in long mode.
> >
> > Copyright (c) 2011 - 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 @@ -29,6 +31,7 @@ typedef struct _PAGE_FAULT_CONTEXT {
> > UINT64 PhyMask;
> > UINTN PageFaultBuffer;
> > UINTN PageFaultIndex;
> > + UINT64 PteMemoryEncryptionAddressOrMask;
> > //
> > // Store the uplink information for each page being used.
> > //
> > @@ -114,6 +117,7 @@ AcquirePage (
> > )
> > {
> > UINTN Address;
> > + UINT64 AddressSetMask;
> >
> > Address = PageFaultContext->PageFaultBuffer + EFI_PAGES_TO_SIZE
> (PageFaultContext->PageFaultIndex);
> > ZeroMem ((VOID *) Address, EFI_PAGES_TO_SIZE (1)); @@ -121,14
> > +125,16 @@ AcquirePage (
> > //
> > // Cut the previous uplink if it exists and wasn't overwritten.
> > //
> > - if
> >
> >((PageFaultContext->PageFaultUplink[PageFaultContext->PageFaultIndex]
> > != NULL) &&
> > ((*PageFaultContext->PageFaultUplink[PageFaultContext-
> >PageFaultIndex]
> > & PageFaultContext->PhyMask) == Address)) {
> > + if ((PageFaultContext->PageFaultUplink[PageFaultContext-
> >PageFaultIndex] != NULL) &&
> > +
> > + ((*PageFaultContext->PageFaultUplink[PageFaultContext-
> >PageFaultInde
> > + x] & PageFaultContext->PhyMask) == Address)) {
>
> No real change at here except the line feed added.
> You were going to update code at here, but forgot to do the real change?
>
> Will you do similar change for [PATCH v3 4/4]
>
> Thanks,
> Star
>
> > *PageFaultContext->PageFaultUplink[PageFaultContext-
> >PageFaultIndex] = 0;
> > }
> >
> > //
> > // Link & Record the current uplink.
> > //
> > - *Uplink = Address | IA32_PG_P | IA32_PG_RW;
> > + AddressSetMask =
> > + PageFaultContext->PteMemoryEncryptionAddressOrMask;
> > + *Uplink = Address | (AddressSetMask &
> PAGING_4K_ADDRESS_MASK_64) |
> > + IA32_PG_P | IA32_PG_RW;
> >
> > PageFaultContext->PageFaultUplink[PageFaultContext->PageFaultIndex]
> > = Uplink;
> >
> > PageFaultContext->PageFaultIndex =
> > (PageFaultContext->PageFaultIndex + 1) % EXTRA_PAGE_TABLE_PAGES;
> @@ -153,6 +159,7 @@ PageFaultHandler (
> > UINT64 *PageTable;
> > UINT64 PFAddress;
> > UINTN PTIndex;
> > + UINT64 AddressSetMask;
> >
> > //
> > // Get the IDT Descriptor.
> > @@ -163,6 +170,7 @@ PageFaultHandler (
> > //
> > PageFaultContext = (PAGE_FAULT_CONTEXT *) (UINTN) (Idtr.Base -
> sizeof (PAGE_FAULT_CONTEXT));
> > PhyMask = PageFaultContext->PhyMask;
> > + AddressSetMask =
> > + PageFaultContext->PteMemoryEncryptionAddressOrMask;
> >
> > PFAddress = AsmReadCr2 ();
> > DEBUG ((EFI_D_ERROR, "CapsuleX64 - PageFaultHandler: Cr2 -
> > %lx\n", PFAddress)); @@ -179,19 +187,19 @@ PageFaultHandler (
> > if ((PageTable[PTIndex] & IA32_PG_P) == 0) {
> > AcquirePage (PageFaultContext, &PageTable[PTIndex]);
> > }
> > - PageTable = (UINT64*)(UINTN)(PageTable[PTIndex] & PhyMask);
> > + PageTable = (UINT64*)(UINTN)((PageTable[PTIndex] &
> ~(AddressSetMask
> > + & PAGING_4K_ADDRESS_MASK_64)) & PhyMask);
> > PTIndex = BitFieldRead64 (PFAddress, 30, 38);
> > // PDPTE
> > if (PageFaultContext->Page1GSupport) {
> > - PageTable[PTIndex] = (PFAddress & ~((1ull << 30) - 1)) | IA32_PG_P |
> IA32_PG_RW | IA32_PG_PS;
> > + PageTable[PTIndex] = ((PFAddress | (AddressSetMask &
> > + PAGING_1G_ADDRESS_MASK_64)) & ~((1ull << 30) - 1)) | IA32_PG_P |
> > + IA32_PG_RW | IA32_PG_PS;
> > } else {
> > if ((PageTable[PTIndex] & IA32_PG_P) == 0) {
> > AcquirePage (PageFaultContext, &PageTable[PTIndex]);
> > }
> > - PageTable = (UINT64*)(UINTN)(PageTable[PTIndex] & PhyMask);
> > + PageTable = (UINT64*)(UINTN)((PageTable[PTIndex] &
> > + ~(AddressSetMask & PAGING_4K_ADDRESS_MASK_64)) & PhyMask);
> > PTIndex = BitFieldRead64 (PFAddress, 21, 29);
> > // PD
> > - PageTable[PTIndex] = (PFAddress & ~((1ull << 21) - 1)) | IA32_PG_P |
> IA32_PG_RW | IA32_PG_PS;
> > + PageTable[PTIndex] = ((PFAddress | (AddressSetMask &
> > + PAGING_2M_ADDRESS_MASK_64)) & ~((1ull << 21) - 1)) | IA32_PG_P |
> > + IA32_PG_RW | IA32_PG_PS;
> > }
> >
> > return NULL;
> > @@ -244,6 +252,7 @@ _ModuleEntryPoint (
> > // Hook page fault handler to handle >4G request.
> > //
> > PageFaultIdtTable.PageFaultContext.Page1GSupport =
> > EntrypointContext->Page1GSupport;
> > +
> PageFaultIdtTable.PageFaultContext.PteMemoryEncryptionAddressOrMask
> > + = EntrypointContext->PteMemoryEncryptionAddressOrMask;
> > IdtEntry = (IA32_IDT_GATE_DESCRIPTOR *) (X64Idtr.Base + (14 *
> > sizeof
> (IA32_IDT_GATE_DESCRIPTOR)));
> > HookPageFaultHandler (IdtEntry,
> > &(PageFaultIdtTable.PageFaultContext));
> >
> > @@ -298,4 +307,4 @@ _ModuleEntryPoint (
> > //
> > ASSERT (FALSE);
> > return EFI_SUCCESS;
> > -}
> > \ No newline at end of file
> > +}
> >
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/4] MdeModulePkg/Universal/CapsulePei: Add support for PCD PcdPteMemoryEncryptionAddressOrMask
2017-02-22 1:20 ` Zeng, Star
@ 2017-02-22 15:07 ` Duran, Leo
0 siblings, 0 replies; 10+ messages in thread
From: Duran, Leo @ 2017-02-22 15:07 UTC (permalink / raw)
To: 'Zeng, Star', edk2-devel@ml01.01.org
Cc: Singh, Brijesh, Tian, Feng, Laszlo Ersek
Hi Star,
Please see my reply below.
Thanks,
Leo.
> -----Original Message-----
> From: Zeng, Star [mailto:star.zeng@intel.com]
> Sent: Tuesday, February 21, 2017 7:20 PM
> To: Duran, Leo <leo.duran@amd.com>; edk2-devel@ml01.01.org
> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Tian, Feng
> <feng.tian@intel.com>; Laszlo Ersek <lersek@redhat.com>; Zeng, Star
> <star.zeng@intel.com>
> Subject: RE: [edk2] [PATCH v3 2/4] MdeModulePkg/Universal/CapsulePei:
> Add support for PCD PcdPteMemoryEncryptionAddressOrMask
>
> Shouldn't
>
> ((*PageFaultContext->PageFaultUplink[PageFaultContext->PageFaultIndex]
> & PageFaultContext->PhyMask) == Address)
>
> be
>
> (((*PageFaultContext->PageFaultUplink[PageFaultContext-
> >PageFaultIndex] & ~(AddressSetMask &
> PAGING_4K_ADDRESS_MASK_64)) & PageFaultContext->PhyMask) ==
> Address)
>
> like you did at other place?
[Duran, Leo]
Yes, I agree... I will take care of that in v4 of the set.
>
> Thanks,
> Star
>
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Duran, Leo
> Sent: Wednesday, February 22, 2017 12:43 AM
> To: Zeng, Star <star.zeng@intel.com>; edk2-devel@ml01.01.org
> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Tian, Feng
> <feng.tian@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: Re: [edk2] [PATCH v3 2/4] MdeModulePkg/Universal/CapsulePei:
> Add support for PCD PcdPteMemoryEncryptionAddressOrMask
>
> Hi Star,
>
> Please double-check the complete [PATCH v3 2/4].
>
> Yes, there is a non-functional change where I did break a 'very long' line into
> 2 lines as you noted (I can put that back as it was before if so required).
> However the intended functional changes are applied in the rest of the patch
> in lines where I reference 'AddressSetMask'.
>
> As for [PATCH v3 4/4]
> The intended functional changes are applied... please confirm, or please let
> me know what seems to be missing.
>
> Thanks,
> Leo.
>
> > -----Original Message-----
> > From: Zeng, Star [mailto:star.zeng@intel.com]
> > Sent: Monday, February 20, 2017 12:05 AM
> > To: Duran, Leo <leo.duran@amd.com>; edk2-devel@ml01.01.org
> > Cc: Laszlo Ersek <lersek@redhat.com>; Feng Tian <feng.tian@intel.com>;
> > Singh, Brijesh <brijesh.singh@amd.com>; star.zeng@intel.com
> > Subject: Re: [edk2] [PATCH v3 2/4] MdeModulePkg/Universal/CapsulePei:
> > Add support for PCD PcdPteMemoryEncryptionAddressOrMask
> >
> > Leo,
> >
> > Comments added inline.
> >
> > On 2017/2/17 5:02, Leo Duran wrote:
> > > This PCD holds the address mask for page table entries when memory
> > > encryption is enabled on AMD processors supporting the Secure
> > > Encrypted Virtualization (SEV) feature.
> > >
> > > The mask is applied when 4GB tables are created (UefiCapsule.c), and
> > > when the tables are expanded on-demand by page-faults above 4GB's
> > (X64Entry.c).
> > >
> > > Cc: Feng Tian <feng.tian@intel.com>
> > > Cc: Star Zeng <star.zeng@intel.com>
> > > Cc: Laszlo Ersek <lersek@redhat.com>
> > > Cc: Brijesh Singh <brijesh.singh@amd.com>
> > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > Signed-off-by: Leo Duran <leo.duran@amd.com>
> > > Reviewed-by: Star Zeng <star.zeng@intel.com>
> > > ---
> > > MdeModulePkg/Universal/CapsulePei/CapsulePei.inf | 2 ++
> > > .../Universal/CapsulePei/Common/CommonHeader.h | 7 +++++++
> > > MdeModulePkg/Universal/CapsulePei/UefiCapsule.c | 13 ++++++++---
> -
> > > MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c | 23
> > +++++++++++++++-------
> > > 4 files changed, 34 insertions(+), 11 deletions(-)
> > >
> >
> > [snipped]
> >
> > > diff --git a/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> > > b/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> > > index 5ad95d2..2197502 100644
> > > --- a/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> > > +++ b/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> > > @@ -2,6 +2,8 @@
> > > The X64 entrypoint is used to process capsule in long mode.
> > >
> > > Copyright (c) 2011 - 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 @@ -29,6 +31,7 @@ typedef struct _PAGE_FAULT_CONTEXT {
> > > UINT64 PhyMask;
> > > UINTN PageFaultBuffer;
> > > UINTN PageFaultIndex;
> > > + UINT64 PteMemoryEncryptionAddressOrMask;
> > > //
> > > // Store the uplink information for each page being used.
> > > //
> > > @@ -114,6 +117,7 @@ AcquirePage (
> > > )
> > > {
> > > UINTN Address;
> > > + UINT64 AddressSetMask;
> > >
> > > Address = PageFaultContext->PageFaultBuffer + EFI_PAGES_TO_SIZE
> > (PageFaultContext->PageFaultIndex);
> > > ZeroMem ((VOID *) Address, EFI_PAGES_TO_SIZE (1)); @@ -121,14
> > > +125,16 @@ AcquirePage (
> > > //
> > > // Cut the previous uplink if it exists and wasn't overwritten.
> > > //
> > > - if
> > >
> > >((PageFaultContext->PageFaultUplink[PageFaultContext-
> >PageFaultIndex]
> > > != NULL) &&
> > > ((*PageFaultContext->PageFaultUplink[PageFaultContext-
> > >PageFaultIndex]
> > > & PageFaultContext->PhyMask) == Address)) {
> > > + if ((PageFaultContext->PageFaultUplink[PageFaultContext-
> > >PageFaultIndex] != NULL) &&
> > > +
> > > + ((*PageFaultContext->PageFaultUplink[PageFaultContext-
> > >PageFaultInde
> > > + x] & PageFaultContext->PhyMask) == Address)) {
> >
> > No real change at here except the line feed added.
> > You were going to update code at here, but forgot to do the real change?
> >
> > Will you do similar change for [PATCH v3 4/4]
> >
> > Thanks,
> > Star
> >
> > > *PageFaultContext->PageFaultUplink[PageFaultContext-
> > >PageFaultIndex] = 0;
> > > }
> > >
> > > //
> > > // Link & Record the current uplink.
> > > //
> > > - *Uplink = Address | IA32_PG_P | IA32_PG_RW;
> > > + AddressSetMask =
> > > + PageFaultContext->PteMemoryEncryptionAddressOrMask;
> > > + *Uplink = Address | (AddressSetMask &
> > PAGING_4K_ADDRESS_MASK_64) |
> > > + IA32_PG_P | IA32_PG_RW;
> > >
> > > PageFaultContext->PageFaultUplink[PageFaultContext->PageFaultIndex]
> > > = Uplink;
> > >
> > > PageFaultContext->PageFaultIndex =
> > > (PageFaultContext->PageFaultIndex + 1) % EXTRA_PAGE_TABLE_PAGES;
> > @@ -153,6 +159,7 @@ PageFaultHandler (
> > > UINT64 *PageTable;
> > > UINT64 PFAddress;
> > > UINTN PTIndex;
> > > + UINT64 AddressSetMask;
> > >
> > > //
> > > // Get the IDT Descriptor.
> > > @@ -163,6 +170,7 @@ PageFaultHandler (
> > > //
> > > PageFaultContext = (PAGE_FAULT_CONTEXT *) (UINTN) (Idtr.Base -
> > sizeof (PAGE_FAULT_CONTEXT));
> > > PhyMask = PageFaultContext->PhyMask;
> > > + AddressSetMask =
> > > + PageFaultContext->PteMemoryEncryptionAddressOrMask;
> > >
> > > PFAddress = AsmReadCr2 ();
> > > DEBUG ((EFI_D_ERROR, "CapsuleX64 - PageFaultHandler: Cr2 -
> > > %lx\n", PFAddress)); @@ -179,19 +187,19 @@ PageFaultHandler (
> > > if ((PageTable[PTIndex] & IA32_PG_P) == 0) {
> > > AcquirePage (PageFaultContext, &PageTable[PTIndex]);
> > > }
> > > - PageTable = (UINT64*)(UINTN)(PageTable[PTIndex] & PhyMask);
> > > + PageTable = (UINT64*)(UINTN)((PageTable[PTIndex] &
> > ~(AddressSetMask
> > > + & PAGING_4K_ADDRESS_MASK_64)) & PhyMask);
> > > PTIndex = BitFieldRead64 (PFAddress, 30, 38);
> > > // PDPTE
> > > if (PageFaultContext->Page1GSupport) {
> > > - PageTable[PTIndex] = (PFAddress & ~((1ull << 30) - 1)) | IA32_PG_P |
> > IA32_PG_RW | IA32_PG_PS;
> > > + PageTable[PTIndex] = ((PFAddress | (AddressSetMask &
> > > + PAGING_1G_ADDRESS_MASK_64)) & ~((1ull << 30) - 1)) | IA32_PG_P |
> > > + IA32_PG_RW | IA32_PG_PS;
> > > } else {
> > > if ((PageTable[PTIndex] & IA32_PG_P) == 0) {
> > > AcquirePage (PageFaultContext, &PageTable[PTIndex]);
> > > }
> > > - PageTable = (UINT64*)(UINTN)(PageTable[PTIndex] & PhyMask);
> > > + PageTable = (UINT64*)(UINTN)((PageTable[PTIndex] &
> > > + ~(AddressSetMask & PAGING_4K_ADDRESS_MASK_64)) & PhyMask);
> > > PTIndex = BitFieldRead64 (PFAddress, 21, 29);
> > > // PD
> > > - PageTable[PTIndex] = (PFAddress & ~((1ull << 21) - 1)) | IA32_PG_P |
> > IA32_PG_RW | IA32_PG_PS;
> > > + PageTable[PTIndex] = ((PFAddress | (AddressSetMask &
> > > + PAGING_2M_ADDRESS_MASK_64)) & ~((1ull << 21) - 1)) | IA32_PG_P |
> > > + IA32_PG_RW | IA32_PG_PS;
> > > }
> > >
> > > return NULL;
> > > @@ -244,6 +252,7 @@ _ModuleEntryPoint (
> > > // Hook page fault handler to handle >4G request.
> > > //
> > > PageFaultIdtTable.PageFaultContext.Page1GSupport =
> > > EntrypointContext->Page1GSupport;
> > > +
> >
> PageFaultIdtTable.PageFaultContext.PteMemoryEncryptionAddressOrMask
> > > + = EntrypointContext->PteMemoryEncryptionAddressOrMask;
> > > IdtEntry = (IA32_IDT_GATE_DESCRIPTOR *) (X64Idtr.Base + (14 *
> > > sizeof
> > (IA32_IDT_GATE_DESCRIPTOR)));
> > > HookPageFaultHandler (IdtEntry,
> > > &(PageFaultIdtTable.PageFaultContext));
> > >
> > > @@ -298,4 +307,4 @@ _ModuleEntryPoint (
> > > //
> > > ASSERT (FALSE);
> > > return EFI_SUCCESS;
> > > -}
> > > \ No newline at end of file
> > > +}
> > >
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 3/4] UefiCpuPkg/Universal/Acpi/S3Resume2Pei: Add support for PCD PcdPteMemoryEncryptionAddressOrMask
2017-02-16 21:02 [PATCH v3 0/4] Add PCD PcdPteMemoryEncryptionAddressOrMask Leo Duran
2017-02-16 21:02 ` [PATCH v3 1/4] MdeModulePkg: " Leo Duran
2017-02-16 21:02 ` [PATCH v3 2/4] MdeModulePkg/Universal/CapsulePei: Add support for " Leo Duran
@ 2017-02-16 21:02 ` Leo Duran
2017-02-16 21:02 ` [PATCH v3 4/4] MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe: " Leo Duran
2017-02-16 21:56 ` [PATCH v3 0/4] Add " Laszlo Ersek
4 siblings, 0 replies; 10+ messages in thread
From: Leo Duran @ 2017-02-16 21:02 UTC (permalink / raw)
To: edk2-devel
Cc: Leo Duran, Jeff Fan, Feng Tian, Star Zeng, Laszlo Ersek,
Brijesh Singh
This PCD holds the address mask for page table entries when memory
encryption is enabled on AMD processors supporting the Secure Encrypted
Virtualization (SEV) feature.
The mask is applied when page tables are created (S3Resume.c).
CC: Jeff Fan <jeff.fan@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leo Duran <leo.duran@amd.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
---
UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c | 16 ++++++++++++----
UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf | 2 ++
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
index d306fba..3701957 100644
--- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
+++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
@@ -5,6 +5,7 @@
control is passed to OS waking up handler.
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
@@ -58,6 +59,10 @@
#define STACK_ALIGN_DOWN(Ptr) \
((UINTN)(Ptr) & ~(UINTN)(CPU_STACK_ALIGNMENT - 1))
+#define PAGING_4K_ADDRESS_MASK_64 0x000FFFFFFFFFF000ull
+#define PAGING_2M_ADDRESS_MASK_64 0x000FFFFFFFE00000ull
+#define PAGING_1G_ADDRESS_MASK_64 0x000FFFFFC0000000ull
+
#pragma pack(1)
typedef union {
struct {
@@ -614,6 +619,7 @@ RestoreS3PageTables (
VOID *Hob;
BOOLEAN Page1GSupport;
PAGE_TABLE_1G_ENTRY *PageDirectory1GEntry;
+ UINT64 AddressSetMask;
//
// NOTE: We have to ASSUME the page table generation format, because we do not know whole page table information.
@@ -682,6 +688,8 @@ RestoreS3PageTables (
NumberOfPml4EntriesNeeded = (UINT32)LShiftU64 (1, (PhysicalAddressBits - 39));
NumberOfPdpEntriesNeeded = 512;
}
+
+ AddressSetMask = PcdGet64 (PcdPteMemoryEncryptionAddressOrMask);
PageMapLevel4Entry = PageMap;
PageAddress = 0;
@@ -696,7 +704,7 @@ RestoreS3PageTables (
//
// Make a PML4 Entry
//
- PageMapLevel4Entry->Uint64 = (UINT64)(UINTN)PageDirectoryPointerEntry;
+ PageMapLevel4Entry->Uint64 = (UINT64)(UINTN)PageDirectoryPointerEntry | (AddressSetMask & PAGING_4K_ADDRESS_MASK_64);
PageMapLevel4Entry->Bits.ReadWrite = 1;
PageMapLevel4Entry->Bits.Present = 1;
@@ -707,7 +715,7 @@ RestoreS3PageTables (
//
// Fill in the Page Directory entries
//
- PageDirectory1GEntry->Uint64 = (UINT64)PageAddress;
+ PageDirectory1GEntry->Uint64 = (UINT64)PageAddress | (AddressSetMask & PAGING_1G_ADDRESS_MASK_64);
PageDirectory1GEntry->Bits.ReadWrite = 1;
PageDirectory1GEntry->Bits.Present = 1;
PageDirectory1GEntry->Bits.MustBe1 = 1;
@@ -724,7 +732,7 @@ RestoreS3PageTables (
//
// Fill in a Page Directory Pointer Entries
//
- PageDirectoryPointerEntry->Uint64 = (UINT64)(UINTN)PageDirectoryEntry;
+ PageDirectoryPointerEntry->Uint64 = (UINT64)(UINTN)PageDirectoryEntry | (AddressSetMask & PAGING_4K_ADDRESS_MASK_64);
PageDirectoryPointerEntry->Bits.ReadWrite = 1;
PageDirectoryPointerEntry->Bits.Present = 1;
@@ -732,7 +740,7 @@ RestoreS3PageTables (
//
// Fill in the Page Directory entries
//
- PageDirectoryEntry->Uint64 = (UINT64)PageAddress;
+ PageDirectoryEntry->Uint64 = (UINT64)PageAddress | (AddressSetMask & PAGING_2M_ADDRESS_MASK_64);
PageDirectoryEntry->Bits.ReadWrite = 1;
PageDirectoryEntry->Bits.Present = 1;
PageDirectoryEntry->Bits.MustBe1 = 1;
diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
index 73aeca3..d514523 100644
--- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
+++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
@@ -6,6 +6,7 @@
# control is passed to OS waking up handler.
#
# Copyright (c) 2010 - 2014, 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
@@ -91,6 +92,7 @@
[Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable ## SOMETIMES_CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask ## CONSUMES
[Depex]
TRUE
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 4/4] MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe: Add support for PCD PcdPteMemoryEncryptionAddressOrMask
2017-02-16 21:02 [PATCH v3 0/4] Add PCD PcdPteMemoryEncryptionAddressOrMask Leo Duran
` (2 preceding siblings ...)
2017-02-16 21:02 ` [PATCH v3 3/4] UefiCpuPkg/Universal/Acpi/S3Resume2Pei: " Leo Duran
@ 2017-02-16 21:02 ` Leo Duran
2017-02-16 21:56 ` [PATCH v3 0/4] Add " Laszlo Ersek
4 siblings, 0 replies; 10+ messages in thread
From: Leo Duran @ 2017-02-16 21:02 UTC (permalink / raw)
To: edk2-devel
Cc: Leo Duran, Jeff Fan, Feng Tian, Star Zeng, Laszlo Ersek,
Brijesh Singh
This PCD holds the address mask for page table entries when memory
encryption is enabled on AMD processors supporting the Secure Encrypted
Virtualization (SEV) feature.
This module updates the under-4GB page tables configured by the S3-Resume
code in UefiCpuPkg/Universal/Acpi/S3Resume2Pei. The mask is saved at module
start (ScriptExecute.c), and applied when tables are expanded on-demand by
page-faults above 4GB's (SetIdtEntry.c).
CC: Jeff Fan <jeff.fan@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leo Duran <leo.duran@amd.com>
---
.../Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf | 2 ++
.../Universal/Acpi/BootScriptExecutorDxe/ScriptExecute.c | 7 +++++++
.../Universal/Acpi/BootScriptExecutorDxe/ScriptExecute.h | 2 ++
.../Acpi/BootScriptExecutorDxe/X64/SetIdtEntry.c | 16 +++++++++++-----
4 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf b/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
index 7cd38cf..29af7f5 100644
--- a/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
+++ b/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
@@ -5,6 +5,7 @@
# depends on any PEI or DXE service.
#
# 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
@@ -85,6 +86,7 @@
gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable ## SOMETIMES_CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryProfilePropertyMask ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask ## CONSUMES
[Depex]
gEfiLockBoxProtocolGuid
diff --git a/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/ScriptExecute.c b/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/ScriptExecute.c
index f67fbca..5146b33 100644
--- a/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/ScriptExecute.c
+++ b/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/ScriptExecute.c
@@ -5,6 +5,7 @@
in the entry point. The functionality is to interpret and restore the S3 boot script
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
@@ -23,6 +24,7 @@ EFI_GUID mBootScriptExecutorImageGuid = {
};
BOOLEAN mPage1GSupport = FALSE;
+UINT64 mPteMemoryEncryptionAddressOrMask = 0;
/**
Entry function of Boot script exector. This function will be executed in
@@ -440,6 +442,11 @@ BootScriptExecutorEntryPoint (
}
}
+ //
+ // Save OrMask to fix-up Address field in page-fault handler
+ //
+ mPteMemoryEncryptionAddressOrMask = PcdGet64 (PcdPteMemoryEncryptionAddressOrMask);
+
BufferSize = sizeof (BOOT_SCRIPT_EXECUTOR_VARIABLE);
BootScriptExecutorBuffer = 0xFFFFFFFF;
diff --git a/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/ScriptExecute.h b/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/ScriptExecute.h
index 772347a..cb88549 100644
--- a/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/ScriptExecute.h
+++ b/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/ScriptExecute.h
@@ -5,6 +5,7 @@
in the entry point. The functionality is to interpret and restore the S3 boot script
Copyright (c) 2006 - 2014, 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
@@ -87,5 +88,6 @@ SetIdtEntry (
extern UINT32 AsmFixAddress16;
extern UINT32 AsmJmpAddr32;
extern BOOLEAN mPage1GSupport;
+extern UINT64 mPteMemoryEncryptionAddressOrMask;
#endif //_BOOT_SCRIPT_EXECUTOR_H_
diff --git a/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/X64/SetIdtEntry.c b/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/X64/SetIdtEntry.c
index 6674560..37724f0 100644
--- a/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/X64/SetIdtEntry.c
+++ b/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/X64/SetIdtEntry.c
@@ -4,6 +4,8 @@
Set a IDT entry for interrupt vector 3 for debug purpose for x64 platform
Copyright (c) 2006 - 2015, 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
@@ -25,6 +27,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#define IA32_PG_RW BIT1
#define IA32_PG_PS BIT7
+#define PAGING_4K_ADDRESS_MASK_64 0x000FFFFFFFFFF000ull
+#define PAGING_2M_ADDRESS_MASK_64 0x000FFFFFFFE00000ull
+#define PAGING_1G_ADDRESS_MASK_64 0x000FFFFFC0000000ull
+
UINT64 mPhyMask;
VOID *mOriginalHandler;
UINTN mPageFaultBuffer;
@@ -207,7 +213,7 @@ AcquirePage (
//
// Link & Record the current uplink.
//
- *Uplink = Address | IA32_PG_P | IA32_PG_RW;
+ *Uplink = Address | (mPteMemoryEncryptionAddressOrMask & PAGING_4K_ADDRESS_MASK_64) | IA32_PG_P | IA32_PG_RW;
mPageFaultUplink[mPageFaultIndex] = Uplink;
mPageFaultIndex = (mPageFaultIndex + 1) % EXTRA_PAGE_TABLE_PAGES;
@@ -245,19 +251,19 @@ PageFaultHandler (
if ((PageTable[PTIndex] & IA32_PG_P) == 0) {
AcquirePage (&PageTable[PTIndex]);
}
- PageTable = (UINT64*)(UINTN)(PageTable[PTIndex] & mPhyMask);
+ PageTable = (UINT64*)(UINTN)((PageTable[PTIndex] & ~(mPteMemoryEncryptionAddressOrMask & PAGING_4K_ADDRESS_MASK_64)) & mPhyMask);
PTIndex = BitFieldRead64 (PFAddress, 30, 38);
// PDPTE
if (mPage1GSupport) {
- PageTable[PTIndex] = (PFAddress & ~((1ull << 30) - 1)) | IA32_PG_P | IA32_PG_RW | IA32_PG_PS;
+ PageTable[PTIndex] = ((PFAddress | (mPteMemoryEncryptionAddressOrMask & PAGING_1G_ADDRESS_MASK_64)) & ~((1ull << 30) - 1)) | IA32_PG_P | IA32_PG_RW | IA32_PG_PS;
} else {
if ((PageTable[PTIndex] & IA32_PG_P) == 0) {
AcquirePage (&PageTable[PTIndex]);
}
- PageTable = (UINT64*)(UINTN)(PageTable[PTIndex] & mPhyMask);
+ PageTable = (UINT64*)(UINTN)((PageTable[PTIndex] & ~(mPteMemoryEncryptionAddressOrMask & PAGING_4K_ADDRESS_MASK_64)) & mPhyMask);
PTIndex = BitFieldRead64 (PFAddress, 21, 29);
// PD
- PageTable[PTIndex] = (PFAddress & ~((1ull << 21) - 1)) | IA32_PG_P | IA32_PG_RW | IA32_PG_PS;
+ PageTable[PTIndex] = ((PFAddress | (mPteMemoryEncryptionAddressOrMask & PAGING_2M_ADDRESS_MASK_64)) & ~((1ull << 21) - 1)) | IA32_PG_P | IA32_PG_RW | IA32_PG_PS;
}
return TRUE;
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 0/4] Add PCD PcdPteMemoryEncryptionAddressOrMask
2017-02-16 21:02 [PATCH v3 0/4] Add PCD PcdPteMemoryEncryptionAddressOrMask Leo Duran
` (3 preceding siblings ...)
2017-02-16 21:02 ` [PATCH v3 4/4] MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe: " Leo Duran
@ 2017-02-16 21:56 ` Laszlo Ersek
4 siblings, 0 replies; 10+ messages in thread
From: Laszlo Ersek @ 2017-02-16 21:56 UTC (permalink / raw)
To: Leo Duran; +Cc: edk2-devel
On 02/16/17 22:02, Leo Duran wrote:
> This new PCD holds the address mask for page table entries when memory
> encryption is enabled on AMD processors supporting the Secure Encrypted
> Virtualization (SEV) feature.
>
> This mask is be applied when creating 1:1 virtual to physical mapping tables.
> For example, the OvmfPkg sets the PCD when launching SEV-enabled guests.
>
> Changes since v2:
> - Apply mask to PCD to keep value within Address field
> - Use variable instead of PCD in Page-Fault Handler
>
> To-Do:
> - Add PCD support for page tables in UefiCpuPg/PiSmmCpuDxeSmm
>From a very superficial skim -- none of the patches are for OvmfPkg
after all! :) --, this approach looks good to me.
If I recall correctly, we're going to set the PCD in OVMF dynamically.
Is that right?
That usually requires investigating the possible orderings between "PCD
producer" and "PCD consumers". For DXE_DRIVER and DXE_SMM_DRIVER
consumers, it is good enough to set the PCD in OvmfPkg/PlatformPei
(which is a PEIM, so it's bound to run earlier -- PEI comes before DXE).
For PEIM consumers though, more thought might be necessary. OvmfPkg does
not pull in CapsulePei, so no need to worry about that.
Wrt. S3Resume2Pei: the dispatch order of S3Resume2Pei vs. PlatformPei is
unspecified (both have plain TRUE depexes). However, S3Resume2Pei does
not consume the PCD in its entry point function, only on the following
call path:
DxeLoadCore() [MdeModulePkg/Core/DxeIplPeim/DxeLoad.c]
S3RestoreConfig2() [UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c]
RestoreS3PageTables()
PcdGet64(...)
IOW, the PCD is not consumed when S3Resume2Pei starts, but when the DXE
Initial Program Load PEIM realizes, at the end of the PEI phase, that
we're resuming, so it branches to EFI_PEI_S3_RESUME2_PPI, rather than
loading the DXE core (and starting the DXE phase).
So, I think setting the PCD in OvmfPkg/PlatformPei will be safe.
(BTW, if we had no such ordering guarantees between PlatformPei and
another PEIM, we could still make it work: we could hook a NULL class
library instance into all such PEIMs, and the lib constructor function
would (re-)set the PCD afresh in each, from whatever hw characteristics
SEV is detectable. Edk2 is awesome like that.
For an instance of this pattern (albeit in the DXE phase), please search
the OVMF DSC files for
"OvmfPkg/Library/SmbiosVersionLib/DetectSmbiosVersionLib.inf".)
... Perhaps this has been obvious, but I thought it wouldn't be useless
to mention.
Thanks
Laszlo
> Leo Duran (4):
> MdeModulePkg: Add PCD PcdPteMemoryEncryptionAddressOrMask
> MdeModulePkg/Universal/CapsulePei: Add support for PCD
> PcdPteMemoryEncryptionAddressOrMask
> UefiCpuPkg/Universal/Acpi/S3Resume2Pei: Add support for PCD
> PcdPteMemoryEncryptionAddressOrMask
> MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe: Add support for
> PCD PcdPteMemoryEncryptionAddressOrMask
>
> MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 5 +++-
> MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 29 ++++++++++++++++------
> MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h | 6 +++++
> MdeModulePkg/MdeModulePkg.dec | 8 ++++++
> .../BootScriptExecutorDxe.inf | 2 ++
> .../Acpi/BootScriptExecutorDxe/ScriptExecute.c | 7 ++++++
> .../Acpi/BootScriptExecutorDxe/ScriptExecute.h | 2 ++
> .../Acpi/BootScriptExecutorDxe/X64/SetIdtEntry.c | 16 ++++++++----
> MdeModulePkg/Universal/CapsulePei/CapsulePei.inf | 2 ++
> .../Universal/CapsulePei/Common/CommonHeader.h | 7 ++++++
> MdeModulePkg/Universal/CapsulePei/UefiCapsule.c | 13 +++++++---
> MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c | 23 +++++++++++------
> UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c | 16 +++++++++---
> .../Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf | 2 ++
> 14 files changed, 109 insertions(+), 29 deletions(-)
>
^ permalink raw reply [flat|nested] 10+ messages in thread