* [PATCH V4 0/3] DXE Memory Protection
@ 2017-02-21 6:57 Jiewen Yao
2017-02-21 6:57 ` [PATCH V4 1/3] UefiCpuPkg/CpuDxe: Add memory attribute setting Jiewen Yao
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Jiewen Yao @ 2017-02-21 6:57 UTC (permalink / raw)
To: edk2-devel
Cc: Jeff Fan, Michael Kinney, Leif Lindholm, Ard Biesheuvel,
Star Zeng, Feng Tian
==== V4 ====
1) Remove ARM patch. (Which was already submitted by Ard Biesheuvel in another series)
2) Unprotect RT image at ExitBootServices (feedback from Ard Biesheuvel)
3) Round up the ImageSize on protection (feedback from Ard Biesheuvel)
==== V3 ====
1) Add PCD for policy control (feedback from Ard Biesheuvel)
(Discussed with Mike Kinney)
+ # BIT0 - Image from unknown device. <BR>
+ # BIT1 - Image from firmware volume.<BR>
+ # @Prompt Set image protection policy.
+ # @ValidRange 0x80000002 | 0x00000000 - 0x0000001F
+ gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x00000002|UINT32|0x00001047
2) Remove unused function in CpuDxe.(feedback from Liming Gao)
3) Add commit log on link option assumption (feedback from Feng Tian)
4) Rename file PageTableLib.h/.c to CpuPageTable.h/.c file (from Jeff Fan)
5) Remove multi-entrypoint usage (from Liming Gao/Mike Kinney)
==== V2 ====
1) Clean up ArmPkg, (feedback from Leif Lindholm)
==== V1 ====
This series patch provides capability to protect PE/COFF image
in DXE memory.
If the UEFI image is page aligned, the image code section is set to read
only and the image data section is set to non-executable.
The DxeCore calls CpuArchProtocol->SetMemoryAttributes() to protect
the image.
Tested platform: NT32/Quark IA32/OVMF IA32/OVMF IA32X64/Intel internal X64/
Tested OS: UEFI Win10, UEFI Ubuntu 16.04.
Untested platform: ARM/AARCH64.
Can ARM/AARCH64 owner help to take a look and try the ARM platform?
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
Jiewen Yao (3):
UefiCpuPkg/CpuDxe: Add memory attribute setting.
MdeModulePkg/dec: add PcdImageProtectionPolicy.
MdeModulePkg/DxeCore: Add UEFI image protection.
MdeModulePkg/Core/Dxe/DxeMain.h | 61 ++
MdeModulePkg/Core/Dxe/DxeMain.inf | 5 +-
MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 5 +-
MdeModulePkg/Core/Dxe/Image/Image.c | 7 +-
MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 769 +++++++++++++++++++
MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 24 +-
MdeModulePkg/MdeModulePkg.dec | 10 +
UefiCpuPkg/CpuDxe/CpuDxe.c | 141 ++--
UefiCpuPkg/CpuDxe/CpuDxe.inf | 5 +-
UefiCpuPkg/CpuDxe/CpuPageTable.c | 779 ++++++++++++++++++++
UefiCpuPkg/CpuDxe/CpuPageTable.h | 113 +++
11 files changed, 1832 insertions(+), 87 deletions(-)
create mode 100644 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.c
create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.h
--
2.7.4.windows.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH V4 1/3] UefiCpuPkg/CpuDxe: Add memory attribute setting.
2017-02-21 6:57 [PATCH V4 0/3] DXE Memory Protection Jiewen Yao
@ 2017-02-21 6:57 ` Jiewen Yao
2017-03-09 11:52 ` Anthony PERARD
2017-02-21 6:57 ` [PATCH V4 2/3] MdeModulePkg/dec: add PcdImageProtectionPolicy Jiewen Yao
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Jiewen Yao @ 2017-02-21 6:57 UTC (permalink / raw)
To: edk2-devel; +Cc: Jeff Fan, Michael Kinney
Add memory attribute setting in CpuArch protocol.
Previous SetMemoryAttributes() API only supports cache attribute setting.
This patch updated SetMemoryAttributes() API to support memory attribute
setting by updating CPU page table.
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
UefiCpuPkg/CpuDxe/CpuDxe.c | 141 ++--
UefiCpuPkg/CpuDxe/CpuDxe.inf | 5 +-
UefiCpuPkg/CpuDxe/CpuPageTable.c | 779 ++++++++++++++++++++
UefiCpuPkg/CpuDxe/CpuPageTable.h | 113 +++
4 files changed, 977 insertions(+), 61 deletions(-)
diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c
index f6d0a67..3f3ddad 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.c
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.c
@@ -1,7 +1,7 @@
/** @file
CPU DXE Module to produce CPU ARCH Protocol.
- Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2008 - 2017, Intel Corporation. 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
@@ -14,6 +14,10 @@
#include "CpuDxe.h"
#include "CpuMp.h"
+#include "CpuPageTable.h"
+
+#define CACHE_ATTRIBUTE_MASK (EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB | EFI_MEMORY_UCE | EFI_MEMORY_WP)
+#define MEMORY_ATTRIBUTE_MASK (EFI_MEMORY_RP | EFI_MEMORY_XP | EFI_MEMORY_RO)
//
// Global Variables
@@ -368,10 +372,9 @@ CpuSetMemoryAttributes (
EFI_STATUS MpStatus;
EFI_MP_SERVICES_PROTOCOL *MpService;
MTRR_SETTINGS MtrrSettings;
-
- if (!IsMtrrSupported ()) {
- return EFI_UNSUPPORTED;
- }
+ UINT64 CacheAttributes;
+ UINT64 MemoryAttributes;
+ MTRR_MEMORY_CACHE_TYPE CurrentCacheType;
//
// If this function is called because GCD SetMemorySpaceAttributes () is called
@@ -384,69 +387,87 @@ CpuSetMemoryAttributes (
return EFI_SUCCESS;
}
- switch (Attributes) {
- case EFI_MEMORY_UC:
- CacheType = CacheUncacheable;
- break;
- case EFI_MEMORY_WC:
- CacheType = CacheWriteCombining;
- break;
+ CacheAttributes = Attributes & CACHE_ATTRIBUTE_MASK;
+ MemoryAttributes = Attributes & MEMORY_ATTRIBUTE_MASK;
- case EFI_MEMORY_WT:
- CacheType = CacheWriteThrough;
- break;
+ if (Attributes != (CacheAttributes | MemoryAttributes)) {
+ return EFI_INVALID_PARAMETER;
+ }
- case EFI_MEMORY_WP:
- CacheType = CacheWriteProtected;
- break;
+ if (CacheAttributes != 0) {
+ if (!IsMtrrSupported ()) {
+ return EFI_UNSUPPORTED;
+ }
- case EFI_MEMORY_WB:
- CacheType = CacheWriteBack;
- break;
+ switch (CacheAttributes) {
+ case EFI_MEMORY_UC:
+ CacheType = CacheUncacheable;
+ break;
- case EFI_MEMORY_UCE:
- case EFI_MEMORY_RP:
- case EFI_MEMORY_XP:
- case EFI_MEMORY_RUNTIME:
- return EFI_UNSUPPORTED;
+ case EFI_MEMORY_WC:
+ CacheType = CacheWriteCombining;
+ break;
- default:
- return EFI_INVALID_PARAMETER;
- }
- //
- // call MTRR libary function
- //
- Status = MtrrSetMemoryAttribute (
- BaseAddress,
- Length,
- CacheType
- );
+ case EFI_MEMORY_WT:
+ CacheType = CacheWriteThrough;
+ break;
- if (!RETURN_ERROR (Status)) {
- MpStatus = gBS->LocateProtocol (
- &gEfiMpServiceProtocolGuid,
- NULL,
- (VOID **)&MpService
- );
- //
- // Synchronize the update with all APs
- //
- if (!EFI_ERROR (MpStatus)) {
- MtrrGetAllMtrrs (&MtrrSettings);
- MpStatus = MpService->StartupAllAPs (
- MpService, // This
- SetMtrrsFromBuffer, // Procedure
- FALSE, // SingleThread
- NULL, // WaitEvent
- 0, // TimeoutInMicrosecsond
- &MtrrSettings, // ProcedureArgument
- NULL // FailedCpuList
- );
- ASSERT (MpStatus == EFI_SUCCESS || MpStatus == EFI_NOT_STARTED);
+ case EFI_MEMORY_WP:
+ CacheType = CacheWriteProtected;
+ break;
+
+ case EFI_MEMORY_WB:
+ CacheType = CacheWriteBack;
+ break;
+
+ default:
+ return EFI_INVALID_PARAMETER;
+ }
+ CurrentCacheType = MtrrGetMemoryAttribute(BaseAddress);
+ if (CurrentCacheType != CacheType) {
+ //
+ // call MTRR libary function
+ //
+ Status = MtrrSetMemoryAttribute (
+ BaseAddress,
+ Length,
+ CacheType
+ );
+
+ if (!RETURN_ERROR (Status)) {
+ MpStatus = gBS->LocateProtocol (
+ &gEfiMpServiceProtocolGuid,
+ NULL,
+ (VOID **)&MpService
+ );
+ //
+ // Synchronize the update with all APs
+ //
+ if (!EFI_ERROR (MpStatus)) {
+ MtrrGetAllMtrrs (&MtrrSettings);
+ MpStatus = MpService->StartupAllAPs (
+ MpService, // This
+ SetMtrrsFromBuffer, // Procedure
+ FALSE, // SingleThread
+ NULL, // WaitEvent
+ 0, // TimeoutInMicrosecsond
+ &MtrrSettings, // ProcedureArgument
+ NULL // FailedCpuList
+ );
+ ASSERT (MpStatus == EFI_SUCCESS || MpStatus == EFI_NOT_STARTED);
+ }
+ }
+ if (EFI_ERROR(Status)) {
+ return Status;
+ }
}
}
- return (EFI_STATUS) Status;
+
+ //
+ // Set memory attribute by page table
+ //
+ return AssignMemoryPageAttributes (NULL, BaseAddress, Length, MemoryAttributes, AllocatePages);
}
/**
@@ -888,6 +909,8 @@ InitializeCpu (
{
EFI_STATUS Status;
EFI_EVENT IdleLoopEvent;
+
+ InitializePageTableLib();
InitializeFloatingPointUnits ();
diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.inf b/UefiCpuPkg/CpuDxe/CpuDxe.inf
index bf389bb..f61b2c9 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.inf
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.inf
@@ -1,7 +1,7 @@
## @file
# CPU driver installs CPU Architecture Protocol and CPU MP protocol.
#
-# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2008 - 2017, Intel Corporation. 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
@@ -19,7 +19,6 @@
FILE_GUID = 1A1E4886-9517-440e-9FDE-3BE44CEE2136
MODULE_TYPE = DXE_DRIVER
VERSION_STRING = 1.0
-
ENTRY_POINT = InitializeCpu
[Packages]
@@ -52,6 +51,8 @@
CpuGdt.h
CpuMp.c
CpuMp.h
+ CpuPageTable.h
+ CpuPageTable.c
[Sources.IA32]
Ia32/CpuAsm.asm
diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c
new file mode 100644
index 0000000..202d1d9
--- /dev/null
+++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
@@ -0,0 +1,779 @@
+/** @file
+ Page table management support.
+
+ Copyright (c) 2017, Intel Corporation. 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 <Base.h>
+#include <Uefi.h>
+#include <Library/BaseLib.h>
+#include <Library/CpuLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/DebugLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Protocol/MpService.h>
+#include "CpuPageTable.h"
+
+///
+/// Page Table Entry
+///
+#define IA32_PG_P BIT0
+#define IA32_PG_RW BIT1
+#define IA32_PG_U BIT2
+#define IA32_PG_WT BIT3
+#define IA32_PG_CD BIT4
+#define IA32_PG_A BIT5
+#define IA32_PG_D BIT6
+#define IA32_PG_PS BIT7
+#define IA32_PG_PAT_2M BIT12
+#define IA32_PG_PAT_4K IA32_PG_PS
+#define IA32_PG_PMNT BIT62
+#define IA32_PG_NX BIT63
+
+#define PAGE_ATTRIBUTE_BITS (IA32_PG_D | IA32_PG_A | IA32_PG_U | IA32_PG_RW | IA32_PG_P)
+//
+// Bits 1, 2, 5, 6 are reserved in the IA32 PAE PDPTE
+// X64 PAE PDPTE does not have such restriction
+//
+#define IA32_PAE_PDPTE_ATTRIBUTE_BITS (IA32_PG_P)
+
+#define PAGE_PROGATE_BITS (IA32_PG_NX | PAGE_ATTRIBUTE_BITS)
+
+#define PAGING_4K_MASK 0xFFF
+#define PAGING_2M_MASK 0x1FFFFF
+#define PAGING_1G_MASK 0x3FFFFFFF
+
+#define PAGING_PAE_INDEX_MASK 0x1FF
+
+#define PAGING_4K_ADDRESS_MASK_64 0x000FFFFFFFFFF000ull
+#define PAGING_2M_ADDRESS_MASK_64 0x000FFFFFFFE00000ull
+#define PAGING_1G_ADDRESS_MASK_64 0x000FFFFFC0000000ull
+
+typedef enum {
+ PageNone,
+ Page4K,
+ Page2M,
+ Page1G,
+} PAGE_ATTRIBUTE;
+
+typedef struct {
+ PAGE_ATTRIBUTE Attribute;
+ UINT64 Length;
+ UINT64 AddressMask;
+} PAGE_ATTRIBUTE_TABLE;
+
+typedef enum {
+ PageActionAssign,
+ PageActionSet,
+ PageActionClear,
+} PAGE_ACTION;
+
+PAGE_ATTRIBUTE_TABLE mPageAttributeTable[] = {
+ {Page4K, SIZE_4KB, PAGING_4K_ADDRESS_MASK_64},
+ {Page2M, SIZE_2MB, PAGING_2M_ADDRESS_MASK_64},
+ {Page1G, SIZE_1GB, PAGING_1G_ADDRESS_MASK_64},
+};
+
+/**
+ Enable write protection function for AP.
+
+ @param[in,out] Buffer The pointer to private data buffer.
+**/
+VOID
+EFIAPI
+SyncCpuEnableWriteProtection (
+ IN OUT VOID *Buffer
+ )
+{
+ AsmWriteCr0 (AsmReadCr0 () | BIT16);
+}
+
+/**
+ CpuFlushTlb function for AP.
+
+ @param[in,out] Buffer The pointer to private data buffer.
+**/
+VOID
+EFIAPI
+SyncCpuFlushTlb (
+ IN OUT VOID *Buffer
+ )
+{
+ CpuFlushTlb();
+}
+
+/**
+ Sync memory page attributes for AP.
+
+ @param[in] Procedure A pointer to the function to be run on enabled APs of
+ the system.
+**/
+VOID
+SyncMemoryPageAttributesAp (
+ IN EFI_AP_PROCEDURE Procedure
+ )
+{
+ EFI_STATUS Status;
+ EFI_MP_SERVICES_PROTOCOL *MpService;
+
+ Status = gBS->LocateProtocol (
+ &gEfiMpServiceProtocolGuid,
+ NULL,
+ (VOID **)&MpService
+ );
+ //
+ // Synchronize the update with all APs
+ //
+ if (!EFI_ERROR (Status)) {
+ Status = MpService->StartupAllAPs (
+ MpService, // This
+ Procedure, // Procedure
+ FALSE, // SingleThread
+ NULL, // WaitEvent
+ 0, // TimeoutInMicrosecsond
+ NULL, // ProcedureArgument
+ NULL // FailedCpuList
+ );
+ ASSERT (Status == EFI_SUCCESS || Status == EFI_NOT_STARTED || Status == EFI_NOT_READY);
+ }
+}
+
+/**
+ Return current paging context.
+
+ @param[in,out] PagingContext The paging context.
+**/
+VOID
+GetCurrentPagingContext (
+ IN OUT PAGE_TABLE_LIB_PAGING_CONTEXT *PagingContext
+ )
+{
+ UINT32 RegEax;
+ UINT32 RegEdx;
+
+ ZeroMem(PagingContext, sizeof(*PagingContext));
+ if (sizeof(UINTN) == sizeof(UINT64)) {
+ PagingContext->MachineType = IMAGE_FILE_MACHINE_X64;
+ } else {
+ PagingContext->MachineType = IMAGE_FILE_MACHINE_I386;
+ }
+ if ((AsmReadCr0 () & BIT31) != 0) {
+ PagingContext->ContextData.X64.PageTableBase = (AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64);
+ if ((AsmReadCr0 () & BIT16) == 0) {
+ AsmWriteCr0 (AsmReadCr0 () | BIT16);
+ SyncMemoryPageAttributesAp (SyncCpuEnableWriteProtection);
+ }
+ } else {
+ PagingContext->ContextData.X64.PageTableBase = 0;
+ }
+
+ if ((AsmReadCr4 () & BIT4) != 0) {
+ PagingContext->ContextData.Ia32.Attributes |= PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PSE;
+ }
+ if ((AsmReadCr4 () & BIT5) != 0) {
+ PagingContext->ContextData.Ia32.Attributes |= PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PAE;
+ }
+ if ((AsmReadCr0 () & BIT16) != 0) {
+ PagingContext->ContextData.Ia32.Attributes |= PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_WP_ENABLE;
+ }
+
+ AsmCpuid (0x80000000, &RegEax, NULL, NULL, NULL);
+ if (RegEax > 0x80000000) {
+ AsmCpuid (0x80000001, NULL, NULL, NULL, &RegEdx);
+ if ((RegEdx & BIT20) != 0) {
+ // XD supported
+ if ((AsmReadMsr64 (0x000001A0) & BIT34) == 0) {
+ // XD enabled
+ if ((AsmReadMsr64 (0xC0000080) & BIT11) != 0) {
+ // XD activated
+ PagingContext->ContextData.Ia32.Attributes |= PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_XD_ACTIVATED;
+ }
+ }
+ }
+ if ((RegEdx & BIT26) != 0) {
+ PagingContext->ContextData.Ia32.Attributes |= PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PAGE_1G_SUPPORT;
+ }
+ }
+}
+
+/**
+ Return length according to page attributes.
+
+ @param[in] PageAttributes The page attribute of the page entry.
+
+ @return The length of page entry.
+**/
+UINTN
+PageAttributeToLength (
+ IN PAGE_ATTRIBUTE PageAttribute
+ )
+{
+ UINTN Index;
+ for (Index = 0; Index < sizeof(mPageAttributeTable)/sizeof(mPageAttributeTable[0]); Index++) {
+ if (PageAttribute == mPageAttributeTable[Index].Attribute) {
+ return (UINTN)mPageAttributeTable[Index].Length;
+ }
+ }
+ return 0;
+}
+
+/**
+ Return address mask according to page attributes.
+
+ @param[in] PageAttributes The page attribute of the page entry.
+
+ @return The address mask of page entry.
+**/
+UINTN
+PageAttributeToMask (
+ IN PAGE_ATTRIBUTE PageAttribute
+ )
+{
+ UINTN Index;
+ for (Index = 0; Index < sizeof(mPageAttributeTable)/sizeof(mPageAttributeTable[0]); Index++) {
+ if (PageAttribute == mPageAttributeTable[Index].Attribute) {
+ return (UINTN)mPageAttributeTable[Index].AddressMask;
+ }
+ }
+ return 0;
+}
+
+/**
+ Return page table entry to match the address.
+
+ @param[in] PagingContext The paging context.
+ @param[in] Address The address to be checked.
+ @param[out] PageAttributes The page attribute of the page entry.
+
+ @return The page entry.
+**/
+VOID *
+GetPageTableEntry (
+ IN PAGE_TABLE_LIB_PAGING_CONTEXT *PagingContext,
+ IN PHYSICAL_ADDRESS Address,
+ OUT PAGE_ATTRIBUTE *PageAttribute
+ )
+{
+ UINTN Index1;
+ UINTN Index2;
+ UINTN Index3;
+ UINTN Index4;
+ UINT64 *L1PageTable;
+ UINT64 *L2PageTable;
+ UINT64 *L3PageTable;
+ UINT64 *L4PageTable;
+
+ ASSERT (PagingContext != NULL);
+
+ Index4 = ((UINTN)RShiftU64 (Address, 39)) & PAGING_PAE_INDEX_MASK;
+ Index3 = ((UINTN)Address >> 30) & PAGING_PAE_INDEX_MASK;
+ Index2 = ((UINTN)Address >> 21) & PAGING_PAE_INDEX_MASK;
+ Index1 = ((UINTN)Address >> 12) & PAGING_PAE_INDEX_MASK;
+
+ if (PagingContext->MachineType == IMAGE_FILE_MACHINE_X64) {
+ L4PageTable = (UINT64 *)(UINTN)PagingContext->ContextData.X64.PageTableBase;
+ if (L4PageTable[Index4] == 0) {
+ *PageAttribute = PageNone;
+ return NULL;
+ }
+
+ L3PageTable = (UINT64 *)(UINTN)(L4PageTable[Index4] & PAGING_4K_ADDRESS_MASK_64);
+ } else {
+ ASSERT((PagingContext->ContextData.Ia32.Attributes & PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PAE) != 0);
+ L3PageTable = (UINT64 *)(UINTN)PagingContext->ContextData.Ia32.PageTableBase;
+ }
+ if (L3PageTable[Index3] == 0) {
+ *PageAttribute = PageNone;
+ return NULL;
+ }
+ if ((L3PageTable[Index3] & IA32_PG_PS) != 0) {
+ // 1G
+ *PageAttribute = Page1G;
+ return &L3PageTable[Index3];
+ }
+
+ L2PageTable = (UINT64 *)(UINTN)(L3PageTable[Index3] & PAGING_4K_ADDRESS_MASK_64);
+ if (L2PageTable[Index2] == 0) {
+ *PageAttribute = PageNone;
+ return NULL;
+ }
+ if ((L2PageTable[Index2] & IA32_PG_PS) != 0) {
+ // 2M
+ *PageAttribute = Page2M;
+ return &L2PageTable[Index2];
+ }
+
+ // 4k
+ L1PageTable = (UINT64 *)(UINTN)(L2PageTable[Index2] & PAGING_4K_ADDRESS_MASK_64);
+ if ((L1PageTable[Index1] == 0) && (Address != 0)) {
+ *PageAttribute = PageNone;
+ return NULL;
+ }
+ *PageAttribute = Page4K;
+ return &L1PageTable[Index1];
+}
+
+/**
+ Return memory attributes of page entry.
+
+ @param[in] PageEntry The page entry.
+
+ @return Memory attributes of page entry.
+**/
+UINT64
+GetAttributesFromPageEntry (
+ IN UINT64 *PageEntry
+ )
+{
+ UINT64 Attributes;
+ Attributes = 0;
+ if ((*PageEntry & IA32_PG_P) == 0) {
+ Attributes |= EFI_MEMORY_RP;
+ }
+ if ((*PageEntry & IA32_PG_RW) == 0) {
+ Attributes |= EFI_MEMORY_RO;
+ }
+ if ((*PageEntry & IA32_PG_NX) != 0) {
+ Attributes |= EFI_MEMORY_XP;
+ }
+ return Attributes;
+}
+
+/**
+ Modify memory attributes of page entry.
+
+ @param[in] PagingContext The paging context.
+ @param[in] PageEntry The page entry.
+ @param[in] Attributes The bit mask of attributes to modify for the memory region.
+ @param[in] PageAction The page action.
+ @param[out] IsModified TRUE means page table modified. FALSE means page table not modified.
+**/
+VOID
+ConvertPageEntryAttribute (
+ IN PAGE_TABLE_LIB_PAGING_CONTEXT *PagingContext,
+ IN UINT64 *PageEntry,
+ IN UINT64 Attributes,
+ IN PAGE_ACTION PageAction,
+ OUT BOOLEAN *IsModified
+ )
+{
+ UINT64 CurrentPageEntry;
+ UINT64 NewPageEntry;
+
+ CurrentPageEntry = *PageEntry;
+ NewPageEntry = CurrentPageEntry;
+ if ((Attributes & EFI_MEMORY_RP) != 0) {
+ switch (PageAction) {
+ case PageActionAssign:
+ case PageActionSet:
+ NewPageEntry &= ~(UINT64)IA32_PG_P;
+ break;
+ case PageActionClear:
+ NewPageEntry |= IA32_PG_P;
+ break;
+ }
+ } else {
+ switch (PageAction) {
+ case PageActionAssign:
+ NewPageEntry |= IA32_PG_P;
+ break;
+ case PageActionSet:
+ case PageActionClear:
+ break;
+ }
+ }
+ if ((Attributes & EFI_MEMORY_RO) != 0) {
+ switch (PageAction) {
+ case PageActionAssign:
+ case PageActionSet:
+ NewPageEntry &= ~(UINT64)IA32_PG_RW;
+ break;
+ case PageActionClear:
+ NewPageEntry |= IA32_PG_RW;
+ break;
+ }
+ } else {
+ switch (PageAction) {
+ case PageActionAssign:
+ NewPageEntry |= IA32_PG_RW;
+ break;
+ case PageActionSet:
+ case PageActionClear:
+ break;
+ }
+ }
+ if ((PagingContext->ContextData.Ia32.Attributes & PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_XD_ACTIVATED) != 0) {
+ if ((Attributes & EFI_MEMORY_XP) != 0) {
+ switch (PageAction) {
+ case PageActionAssign:
+ case PageActionSet:
+ NewPageEntry |= IA32_PG_NX;
+ break;
+ case PageActionClear:
+ NewPageEntry &= ~IA32_PG_NX;
+ break;
+ }
+ } else {
+ switch (PageAction) {
+ case PageActionAssign:
+ NewPageEntry &= ~IA32_PG_NX;
+ break;
+ case PageActionSet:
+ case PageActionClear:
+ break;
+ }
+ }
+ }
+ *PageEntry = NewPageEntry;
+ if (CurrentPageEntry != NewPageEntry) {
+ *IsModified = TRUE;
+ DEBUG ((DEBUG_INFO, "ConvertPageEntryAttribute 0x%lx", CurrentPageEntry));
+ DEBUG ((DEBUG_INFO, "->0x%lx\n", NewPageEntry));
+ } else {
+ *IsModified = FALSE;
+ }
+}
+
+/**
+ This function returns if there is need to split page entry.
+
+ @param[in] BaseAddress The base address to be checked.
+ @param[in] Length The length to be checked.
+ @param[in] PageEntry The page entry to be checked.
+ @param[in] PageAttribute The page attribute of the page entry.
+
+ @retval SplitAttributes on if there is need to split page entry.
+**/
+PAGE_ATTRIBUTE
+NeedSplitPage (
+ IN PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length,
+ IN UINT64 *PageEntry,
+ IN PAGE_ATTRIBUTE PageAttribute
+ )
+{
+ UINT64 PageEntryLength;
+
+ PageEntryLength = PageAttributeToLength (PageAttribute);
+
+ if (((BaseAddress & (PageEntryLength - 1)) == 0) && (Length >= PageEntryLength)) {
+ return PageNone;
+ }
+
+ if (((BaseAddress & PAGING_2M_MASK) != 0) || (Length < SIZE_2MB)) {
+ return Page4K;
+ }
+
+ return Page2M;
+}
+
+/**
+ This function splits one page entry to small page entries.
+
+ @param[in] PageEntry The page entry to be splitted.
+ @param[in] PageAttribute The page attribute of the page entry.
+ @param[in] SplitAttribute How to split the page entry.
+ @param[in] AllocatePagesFunc If page split is needed, this function is used to allocate more pages.
+
+ @retval RETURN_SUCCESS The page entry is splitted.
+ @retval RETURN_UNSUPPORTED The page entry does not support to be splitted.
+ @retval RETURN_OUT_OF_RESOURCES No resource to split page entry.
+**/
+RETURN_STATUS
+SplitPage (
+ IN UINT64 *PageEntry,
+ IN PAGE_ATTRIBUTE PageAttribute,
+ IN PAGE_ATTRIBUTE SplitAttribute,
+ IN PAGE_TABLE_LIB_ALLOCATE_PAGES AllocatePagesFunc
+ )
+{
+ UINT64 BaseAddress;
+ UINT64 *NewPageEntry;
+ UINTN Index;
+
+ ASSERT (PageAttribute == Page2M || PageAttribute == Page1G);
+
+ ASSERT (AllocatePagesFunc != NULL);
+
+ if (PageAttribute == Page2M) {
+ //
+ // Split 2M to 4K
+ //
+ ASSERT (SplitAttribute == Page4K);
+ if (SplitAttribute == Page4K) {
+ NewPageEntry = AllocatePagesFunc (1);
+ DEBUG ((DEBUG_INFO, "Split - 0x%x\n", NewPageEntry));
+ if (NewPageEntry == NULL) {
+ return RETURN_OUT_OF_RESOURCES;
+ }
+ BaseAddress = *PageEntry & PAGING_2M_ADDRESS_MASK_64;
+ for (Index = 0; Index < SIZE_4KB / sizeof(UINT64); Index++) {
+ NewPageEntry[Index] = BaseAddress + SIZE_4KB * Index + ((*PageEntry) & PAGE_PROGATE_BITS);
+ }
+ (*PageEntry) = (UINT64)(UINTN)NewPageEntry + ((*PageEntry) & PAGE_PROGATE_BITS);
+ return RETURN_SUCCESS;
+ } else {
+ return RETURN_UNSUPPORTED;
+ }
+ } else if (PageAttribute == Page1G) {
+ //
+ // Split 1G to 2M
+ // No need support 1G->4K directly, we should use 1G->2M, then 2M->4K to get more compact page table.
+ //
+ ASSERT (SplitAttribute == Page2M || SplitAttribute == Page4K);
+ if ((SplitAttribute == Page2M || SplitAttribute == Page4K)) {
+ NewPageEntry = AllocatePagesFunc (1);
+ DEBUG ((DEBUG_INFO, "Split - 0x%x\n", NewPageEntry));
+ if (NewPageEntry == NULL) {
+ return RETURN_OUT_OF_RESOURCES;
+ }
+ BaseAddress = *PageEntry & PAGING_1G_ADDRESS_MASK_64;
+ for (Index = 0; Index < SIZE_4KB / sizeof(UINT64); Index++) {
+ NewPageEntry[Index] = BaseAddress + SIZE_2MB * Index + IA32_PG_PS + ((*PageEntry) & PAGE_PROGATE_BITS);
+ }
+ (*PageEntry) = (UINT64)(UINTN)NewPageEntry + ((*PageEntry) & PAGE_PROGATE_BITS);
+ return RETURN_SUCCESS;
+ } else {
+ return RETURN_UNSUPPORTED;
+ }
+ } else {
+ return RETURN_UNSUPPORTED;
+ }
+}
+
+/**
+ This function modifies the page attributes for the memory region specified by BaseAddress and
+ Length from their current attributes to the attributes specified by Attributes.
+
+ Caller should make sure BaseAddress and Length is at page boundary.
+
+ @param[in] PagingContext The paging context. NULL means get page table from current CPU context.
+ @param[in] BaseAddress The physical address that is the start address of a memory region.
+ @param[in] Length The size in bytes of the memory region.
+ @param[in] Attributes The bit mask of attributes to modify for the memory region.
+ @param[in] PageAction The page action.
+ @param[in] AllocatePagesFunc If page split is needed, this function is used to allocate more pages.
+ NULL mean page split is unsupported.
+ @param[out] IsSplitted TRUE means page table splitted. FALSE means page table not splitted.
+ @param[out] IsModified TRUE means page table modified. FALSE means page table not modified.
+
+ @retval RETURN_SUCCESS The attributes were modified for the memory region.
+ @retval RETURN_ACCESS_DENIED The attributes for the memory resource range specified by
+ BaseAddress and Length cannot be modified.
+ @retval RETURN_INVALID_PARAMETER Length is zero.
+ Attributes specified an illegal combination of attributes that
+ cannot be set together.
+ @retval RETURN_OUT_OF_RESOURCES There are not enough system resources to modify the attributes of
+ the memory resource range.
+ @retval RETURN_UNSUPPORTED The processor does not support one or more bytes of the memory
+ resource range specified by BaseAddress and Length.
+ The bit mask of attributes is not support for the memory resource
+ range specified by BaseAddress and Length.
+**/
+RETURN_STATUS
+ConvertMemoryPageAttributes (
+ IN PAGE_TABLE_LIB_PAGING_CONTEXT *PagingContext OPTIONAL,
+ IN PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length,
+ IN UINT64 Attributes,
+ IN PAGE_ACTION PageAction,
+ IN PAGE_TABLE_LIB_ALLOCATE_PAGES AllocatePagesFunc OPTIONAL,
+ OUT BOOLEAN *IsSplitted, OPTIONAL
+ OUT BOOLEAN *IsModified OPTIONAL
+ )
+{
+ PAGE_TABLE_LIB_PAGING_CONTEXT CurrentPagingContext;
+ UINT64 *PageEntry;
+ PAGE_ATTRIBUTE PageAttribute;
+ UINTN PageEntryLength;
+ PAGE_ATTRIBUTE SplitAttribute;
+ RETURN_STATUS Status;
+ BOOLEAN IsEntryModified;
+
+ if ((BaseAddress & (SIZE_4KB - 1)) != 0) {
+ DEBUG ((DEBUG_ERROR, "BaseAddress(0x%lx) is not aligned!\n", BaseAddress));
+ return EFI_UNSUPPORTED;
+ }
+ if ((Length & (SIZE_4KB - 1)) != 0) {
+ DEBUG ((DEBUG_ERROR, "Length(0x%lx) is not aligned!\n", Length));
+ return EFI_UNSUPPORTED;
+ }
+ if (Length == 0) {
+ DEBUG ((DEBUG_ERROR, "Length is 0!\n"));
+ return RETURN_INVALID_PARAMETER;
+ }
+
+ if ((Attributes & ~(EFI_MEMORY_RP | EFI_MEMORY_RO | EFI_MEMORY_XP)) != 0) {
+ DEBUG ((DEBUG_ERROR, "Attributes(0x%lx) has unsupported bit\n", Attributes));
+ return EFI_UNSUPPORTED;
+ }
+
+ if (PagingContext == NULL) {
+ GetCurrentPagingContext (&CurrentPagingContext);
+ } else {
+ CopyMem (&CurrentPagingContext, PagingContext, sizeof(CurrentPagingContext));
+ }
+ switch(CurrentPagingContext.MachineType) {
+ case IMAGE_FILE_MACHINE_I386:
+ if (CurrentPagingContext.ContextData.Ia32.PageTableBase == 0) {
+ DEBUG ((DEBUG_ERROR, "PageTable is 0!\n"));
+ if (Attributes == 0) {
+ return EFI_SUCCESS;
+ } else {
+ return EFI_UNSUPPORTED;
+ }
+ }
+ if ((CurrentPagingContext.ContextData.Ia32.Attributes & PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PAE) == 0) {
+ DEBUG ((DEBUG_ERROR, "Non-PAE Paging!\n"));
+ return EFI_UNSUPPORTED;
+ }
+ break;
+ case IMAGE_FILE_MACHINE_X64:
+ ASSERT (CurrentPagingContext.ContextData.X64.PageTableBase != 0);
+ break;
+ default:
+ ASSERT(FALSE);
+ return EFI_UNSUPPORTED;
+ break;
+ }
+
+// DEBUG ((DEBUG_ERROR, "ConvertMemoryPageAttributes(%x) - %016lx, %016lx, %02lx\n", IsSet, BaseAddress, Length, Attributes));
+
+ if (IsSplitted != NULL) {
+ *IsSplitted = FALSE;
+ }
+ if (IsModified != NULL) {
+ *IsModified = FALSE;
+ }
+
+ //
+ // Below logic is to check 2M/4K page to make sure we donot waist memory.
+ //
+ while (Length != 0) {
+ PageEntry = GetPageTableEntry (&CurrentPagingContext, BaseAddress, &PageAttribute);
+ if (PageEntry == NULL) {
+ return RETURN_UNSUPPORTED;
+ }
+ PageEntryLength = PageAttributeToLength (PageAttribute);
+ SplitAttribute = NeedSplitPage (BaseAddress, Length, PageEntry, PageAttribute);
+ if (SplitAttribute == PageNone) {
+ ConvertPageEntryAttribute (&CurrentPagingContext, PageEntry, Attributes, PageAction, &IsEntryModified);
+ if (IsEntryModified) {
+ if (IsModified != NULL) {
+ *IsModified = TRUE;
+ }
+ }
+ //
+ // Convert success, move to next
+ //
+ BaseAddress += PageEntryLength;
+ Length -= PageEntryLength;
+ } else {
+ if (AllocatePagesFunc == NULL) {
+ return RETURN_UNSUPPORTED;
+ }
+ Status = SplitPage (PageEntry, PageAttribute, SplitAttribute, AllocatePagesFunc);
+ if (RETURN_ERROR (Status)) {
+ return RETURN_UNSUPPORTED;
+ }
+ if (IsSplitted != NULL) {
+ *IsSplitted = TRUE;
+ }
+ if (IsModified != NULL) {
+ *IsModified = TRUE;
+ }
+ //
+ // Just split current page
+ // Convert success in next around
+ //
+ }
+ }
+
+ return RETURN_SUCCESS;
+}
+
+/**
+ This function assigns the page attributes for the memory region specified by BaseAddress and
+ Length from their current attributes to the attributes specified by Attributes.
+
+ Caller should make sure BaseAddress and Length is at page boundary.
+
+ Caller need guarentee the TPL <= TPL_NOTIFY, if there is split page request.
+
+ @param[in] PagingContext The paging context. NULL means get page table from current CPU context.
+ @param[in] BaseAddress The physical address that is the start address of a memory region.
+ @param[in] Length The size in bytes of the memory region.
+ @param[in] Attributes The bit mask of attributes to set for the memory region.
+ @param[in] AllocatePagesFunc If page split is needed, this function is used to allocate more pages.
+ NULL mean page split is unsupported.
+
+ @retval RETURN_SUCCESS The attributes were cleared for the memory region.
+ @retval RETURN_ACCESS_DENIED The attributes for the memory resource range specified by
+ BaseAddress and Length cannot be modified.
+ @retval RETURN_INVALID_PARAMETER Length is zero.
+ Attributes specified an illegal combination of attributes that
+ cannot be set together.
+ @retval RETURN_OUT_OF_RESOURCES There are not enough system resources to modify the attributes of
+ the memory resource range.
+ @retval RETURN_UNSUPPORTED The processor does not support one or more bytes of the memory
+ resource range specified by BaseAddress and Length.
+ The bit mask of attributes is not support for the memory resource
+ range specified by BaseAddress and Length.
+**/
+RETURN_STATUS
+EFIAPI
+AssignMemoryPageAttributes (
+ IN PAGE_TABLE_LIB_PAGING_CONTEXT *PagingContext OPTIONAL,
+ IN PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length,
+ IN UINT64 Attributes,
+ IN PAGE_TABLE_LIB_ALLOCATE_PAGES AllocatePagesFunc OPTIONAL
+ )
+{
+ RETURN_STATUS Status;
+ BOOLEAN IsModified;
+ BOOLEAN IsSplitted;
+
+// DEBUG((DEBUG_INFO, "AssignMemoryPageAttributes: 0x%lx - 0x%lx (0x%lx)\n", BaseAddress, Length, Attributes));
+ Status = ConvertMemoryPageAttributes (PagingContext, BaseAddress, Length, Attributes, PageActionAssign, AllocatePagesFunc, &IsSplitted, &IsModified);
+ if (!EFI_ERROR(Status)) {
+ if ((PagingContext == NULL) && IsModified) {
+ //
+ // Flush TLB as last step
+ //
+ CpuFlushTlb();
+ SyncMemoryPageAttributesAp (SyncCpuFlushTlb);
+ }
+ }
+
+ return Status;
+}
+
+/**
+ Initialize the Page Table lib.
+**/
+VOID
+InitializePageTableLib (
+ VOID
+ )
+{
+ PAGE_TABLE_LIB_PAGING_CONTEXT CurrentPagingContext;
+
+ GetCurrentPagingContext (&CurrentPagingContext);
+ DEBUG ((DEBUG_INFO, "CurrentPagingContext:\n", CurrentPagingContext.MachineType));
+ DEBUG ((DEBUG_INFO, " MachineType - 0x%x\n", CurrentPagingContext.MachineType));
+ DEBUG ((DEBUG_INFO, " PageTableBase - 0x%x\n", CurrentPagingContext.ContextData.X64.PageTableBase));
+ DEBUG ((DEBUG_INFO, " Attributes - 0x%x\n", CurrentPagingContext.ContextData.X64.Attributes));
+
+ return ;
+}
+
diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.h b/UefiCpuPkg/CpuDxe/CpuPageTable.h
new file mode 100644
index 0000000..eaff595
--- /dev/null
+++ b/UefiCpuPkg/CpuDxe/CpuPageTable.h
@@ -0,0 +1,113 @@
+/** @file
+ Page table management header file.
+
+ Copyright (c) 2017, Intel Corporation. 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 _PAGE_TABLE_LIB_H_
+#define _PAGE_TABLE_LIB_H_
+
+#include <IndustryStandard/PeImage.h>
+
+#define PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PSE BIT0
+#define PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PAE BIT1
+#define PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PAGE_1G_SUPPORT BIT2
+#define PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_WP_ENABLE BIT30
+#define PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_XD_ACTIVATED BIT31
+// Other bits are reserved for future use
+typedef struct {
+ UINT32 PageTableBase;
+ UINT32 Reserved;
+ UINT32 Attributes;
+} PAGE_TABLE_LIB_PAGING_CONTEXT_IA32;
+
+typedef struct {
+ UINT64 PageTableBase;
+ UINT32 Attributes;
+} PAGE_TABLE_LIB_PAGING_CONTEXT_X64;
+
+typedef union {
+ PAGE_TABLE_LIB_PAGING_CONTEXT_IA32 Ia32;
+ PAGE_TABLE_LIB_PAGING_CONTEXT_X64 X64;
+} PAGE_TABLE_LIB_PAGING_CONTEXT_DATA;
+
+typedef struct {
+ //
+ // PE32+ Machine type for EFI images
+ //
+ // #define IMAGE_FILE_MACHINE_I386 0x014c
+ // #define IMAGE_FILE_MACHINE_X64 0x8664
+ //
+ UINT16 MachineType;
+ PAGE_TABLE_LIB_PAGING_CONTEXT_DATA ContextData;
+} PAGE_TABLE_LIB_PAGING_CONTEXT;
+
+/**
+ Allocates one or more 4KB pages for page table.
+
+ @param Pages The number of 4 KB pages to allocate.
+
+ @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+typedef
+VOID *
+(EFIAPI *PAGE_TABLE_LIB_ALLOCATE_PAGES) (
+ IN UINTN Pages
+ );
+
+/**
+ This function assigns the page attributes for the memory region specified by BaseAddress and
+ Length from their current attributes to the attributes specified by Attributes.
+
+ Caller should make sure BaseAddress and Length is at page boundary.
+
+ Caller need guarentee the TPL <= TPL_NOTIFY, if there is split page request.
+
+ @param PagingContext The paging context. NULL means get page table from current CPU context.
+ @param BaseAddress The physical address that is the start address of a memory region.
+ @param Length The size in bytes of the memory region.
+ @param Attributes The bit mask of attributes to set for the memory region.
+ @param AllocatePagesFunc If page split is needed, this function is used to allocate more pages.
+ NULL mean page split is unsupported.
+
+ @retval RETURN_SUCCESS The attributes were cleared for the memory region.
+ @retval RETURN_ACCESS_DENIED The attributes for the memory resource range specified by
+ BaseAddress and Length cannot be modified.
+ @retval RETURN_INVALID_PARAMETER Length is zero.
+ Attributes specified an illegal combination of attributes that
+ cannot be set together.
+ @retval RETURN_OUT_OF_RESOURCES There are not enough system resources to modify the attributes of
+ the memory resource range.
+ @retval RETURN_UNSUPPORTED The processor does not support one or more bytes of the memory
+ resource range specified by BaseAddress and Length.
+ The bit mask of attributes is not support for the memory resource
+ range specified by BaseAddress and Length.
+**/
+RETURN_STATUS
+EFIAPI
+AssignMemoryPageAttributes (
+ IN PAGE_TABLE_LIB_PAGING_CONTEXT *PagingContext OPTIONAL,
+ IN PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length,
+ IN UINT64 Attributes,
+ IN PAGE_TABLE_LIB_ALLOCATE_PAGES AllocatePagesFunc OPTIONAL
+ );
+
+/**
+ Initialize the Page Table lib.
+**/
+VOID
+InitializePageTableLib (
+ VOID
+ );
+
+#endif
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH V4 2/3] MdeModulePkg/dec: add PcdImageProtectionPolicy.
2017-02-21 6:57 [PATCH V4 0/3] DXE Memory Protection Jiewen Yao
2017-02-21 6:57 ` [PATCH V4 1/3] UefiCpuPkg/CpuDxe: Add memory attribute setting Jiewen Yao
@ 2017-02-21 6:57 ` Jiewen Yao
2017-02-21 6:57 ` [PATCH V4 3/3] MdeModulePkg/DxeCore: Add UEFI image protection Jiewen Yao
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Jiewen Yao @ 2017-02-21 6:57 UTC (permalink / raw)
To: edk2-devel; +Cc: Star Zeng, Feng Tian, Michael Kinney
Add PCD for image protection policy.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
MdeModulePkg/MdeModulePkg.dec | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 273cd7e..ab0490f 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1087,6 +1087,16 @@
# @Prompt Memory profile driver path.
gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryProfileDriverPath|{0x0}|VOID*|0x00001043
+ ## Set image protection policy. The policy is bitwise.
+ # If a bit is set, the image will be protected by DxeCore if it is aligned.
+ # The code section becomes read-only, and the data section becomes non-executable.
+ # If a bit is clear, the image will not be protected.<BR><BR>
+ # BIT0 - Image from unknown device. <BR>
+ # BIT1 - Image from firmware volume.<BR>
+ # @Prompt Set image protection policy.
+ # @ValidRange 0x80000002 | 0x00000000 - 0x0000001F
+ gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x00000002|UINT32|0x00001047
+
## PCI Serial Device Info. It is an array of Device, Function, and Power Management
# information that describes the path that contains zero or more PCI to PCI briges
# followed by a PCI serial device. Each array entry is 4-bytes in length. The
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH V4 3/3] MdeModulePkg/DxeCore: Add UEFI image protection.
2017-02-21 6:57 [PATCH V4 0/3] DXE Memory Protection Jiewen Yao
2017-02-21 6:57 ` [PATCH V4 1/3] UefiCpuPkg/CpuDxe: Add memory attribute setting Jiewen Yao
2017-02-21 6:57 ` [PATCH V4 2/3] MdeModulePkg/dec: add PcdImageProtectionPolicy Jiewen Yao
@ 2017-02-21 6:57 ` Jiewen Yao
2017-02-21 7:23 ` [PATCH V4 0/3] DXE Memory Protection Fan, Jeff
2017-02-21 8:36 ` Ard Biesheuvel
4 siblings, 0 replies; 12+ messages in thread
From: Jiewen Yao @ 2017-02-21 6:57 UTC (permalink / raw)
To: edk2-devel; +Cc: Star Zeng, Feng Tian, Michael Kinney
If the UEFI image is page aligned, the image code section is set to read
only and the image data section is set to non-executable.
1) This policy is applied for all UEFI image including boot service driver,
runtime driver or application.
2) This policy is applied only if the UEFI image meets the page alignment
requirement.
3) This policy is applied only if the Source UEFI image matches the
PcdImageProtectionPolicy definition.
4) This policy is not applied to the non-PE image region.
The DxeCore calls CpuArchProtocol->SetMemoryAttributes() to protect
the image. If the CpuArch protocol is not installed yet, the DxeCore
enqueues the protection request. Once the CpuArch is installed, the
DxeCore dequeues the protection request and applies policy.
Once the image is unloaded, the protection is removed automatically.
The UEFI runtime image protection is teared down at ExitBootServices(),
the runtime image code relocation need write code segment at
SetVirtualAddressMap(). We cannot assume OS/Loader has taken over
page table at that time.
NOTE: It is per-requisite that code section and data section
should not be not merged. That is same criteria for SMM/runtime driver.
We are not able to detect during BIOS boot, because
we can only get LINK warning below:
"LINK : warning LNK4254: section '.data' (C0000040) merged into
'.text' (60000020) with different attributes"
But final attribute in PE code section is same.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
MdeModulePkg/Core/Dxe/DxeMain.h | 61 ++
MdeModulePkg/Core/Dxe/DxeMain.inf | 5 +-
MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 5 +-
MdeModulePkg/Core/Dxe/Image/Image.c | 7 +-
MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 769 ++++++++++++++++++++
MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 24 +-
6 files changed, 845 insertions(+), 26 deletions(-)
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
index ae35fbb..b14be9a 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.h
+++ b/MdeModulePkg/Core/Dxe/DxeMain.h
@@ -267,6 +267,26 @@ typedef struct {
#define LOADED_IMAGE_PRIVATE_DATA_FROM_THIS(a) \
CR(a, LOADED_IMAGE_PRIVATE_DATA, Info, LOADED_IMAGE_PRIVATE_DATA_SIGNATURE)
+#define IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE SIGNATURE_32 ('I','P','R','C')
+
+typedef struct {
+ UINT32 Signature;
+ LIST_ENTRY Link;
+ EFI_PHYSICAL_ADDRESS CodeSegmentBase;
+ UINT64 CodeSegmentSize;
+} IMAGE_PROPERTIES_RECORD_CODE_SECTION;
+
+#define IMAGE_PROPERTIES_RECORD_SIGNATURE SIGNATURE_32 ('I','P','R','D')
+
+typedef struct {
+ UINT32 Signature;
+ LIST_ENTRY Link;
+ EFI_PHYSICAL_ADDRESS ImageBase;
+ UINT64 ImageSize;
+ UINTN CodeSegmentCount;
+ LIST_ENTRY CodeSegmentList;
+} IMAGE_PROPERTIES_RECORD;
+
//
// DXE Core Global Variables
//
@@ -2859,6 +2879,15 @@ CoreInitializeMemoryAttributesTable (
);
/**
+ Initialize Memory Protection support.
+**/
+VOID
+EFIAPI
+CoreInitializeMemoryProtection (
+ VOID
+ );
+
+/**
Install MemoryAttributesTable on memory allocation.
@param[in] MemoryType EFI memory type.
@@ -2888,4 +2917,36 @@ RemoveImageRecord (
IN EFI_RUNTIME_IMAGE_ENTRY *RuntimeImage
);
+/**
+ Protect UEFI image.
+
+ @param[in] LoadedImage The loaded image protocol
+ @param[in] LoadedImageDevicePath The loaded image device path protocol
+**/
+VOID
+ProtectUefiImage (
+ IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage,
+ IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath
+ );
+
+/**
+ Unprotect UEFI image.
+
+ @param[in] LoadedImage The loaded image protocol
+ @param[in] LoadedImageDevicePath The loaded image device path protocol
+**/
+VOID
+UnprotectUefiImage (
+ IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage,
+ IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath
+ );
+
+/**
+ ExitBootServices Callback function for memory protection.
+**/
+VOID
+MemoryProtectionExitBootServicesCallback (
+ VOID
+ );
+
#endif
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf
index 13a2381..371e91c 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -3,7 +3,7 @@
#
# It provides an implementation of DXE Core that is compliant with DXE CIS.
#
-# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2006 - 2017, Intel Corporation. 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
@@ -42,6 +42,7 @@
Misc/InstallConfigurationTable.c
Misc/PropertiesTable.c
Misc/MemoryAttributesTable.c
+ Misc/MemoryProtection.c
Library/Library.c
Hand/DriverSupport.c
Hand/Notify.c
@@ -159,6 +160,7 @@
gEfiHiiPackageListProtocolGuid ## SOMETIMES_PRODUCES
gEfiEbcProtocolGuid ## SOMETIMES_CONSUMES
gEfiSmmBase2ProtocolGuid ## SOMETIMES_CONSUMES
+ gEfiBlockIoProtocolGuid ## SOMETIMES_CONSUMES
# Arch Protocols
gEfiBdsArchProtocolGuid ## CONSUMES
@@ -188,6 +190,7 @@
gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryProfilePropertyMask ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryProfileDriverPath ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy ## CONSUMES
# [Hob]
# RESOURCE_DESCRIPTOR ## CONSUMES
diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
index 21cd61a..91e94a7 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
+++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
@@ -1,7 +1,7 @@
/** @file
DXE Core Main Entry Point
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2017, Intel Corporation. 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
@@ -398,6 +398,7 @@ DxeMain (
CoreInitializePropertiesTable ();
CoreInitializeMemoryAttributesTable ();
+ CoreInitializeMemoryProtection ();
//
// Get persisted vector hand-off info from GUIDeed HOB again due to HobStart may be updated,
@@ -781,6 +782,8 @@ CoreExitBootServices (
//
gCpu->DisableInterrupt (gCpu);
+ MemoryProtectionExitBootServicesCallback();
+
//
// Clear the non-runtime values of the EFI System Table
//
diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c b/MdeModulePkg/Core/Dxe/Image/Image.c
index 4a8a16d..652da8b 100644
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -1,7 +1,7 @@
/** @file
Core image handling services to load and unload PeImage.
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2017, Intel Corporation. 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
@@ -203,6 +203,8 @@ CoreInitializeImageServices (
);
}
+ ProtectUefiImage (&Image->Info, Image->LoadedImageDevicePath);
+
return Status;
}
@@ -862,6 +864,8 @@ CoreUnloadAndCloseImage (
UnregisterMemoryProfileImage (Image);
}
+ UnprotectUefiImage (&Image->Info, Image->LoadedImageDevicePath);
+
if (Image->Ebc != NULL) {
//
// If EBC protocol exists we must perform cleanups for this image.
@@ -1341,6 +1345,7 @@ CoreLoadImageCommon (
goto Done;
}
}
+ ProtectUefiImage (&Image->Info, Image->LoadedImageDevicePath);
//
// Success. Return the image handle
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
new file mode 100644
index 0000000..c36612a
--- /dev/null
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -0,0 +1,769 @@
+/** @file
+ UEFI Memory Protection support.
+
+ If the UEFI image is page aligned, the image code section is set to read only
+ and the image data section is set to non-executable.
+
+ 1) This policy is applied for all UEFI image including boot service driver,
+ runtime driver or application.
+ 2) This policy is applied only if the UEFI image meets the page alignment
+ requirement.
+ 3) This policy is applied only if the Source UEFI image matches the
+ PcdImageProtectionPolicy definition.
+ 4) This policy is not applied to the non-PE image region.
+
+ The DxeCore calls CpuArchProtocol->SetMemoryAttributes() to protect
+ the image. If the CpuArch protocol is not installed yet, the DxeCore
+ enqueues the protection request. Once the CpuArch is installed, the
+ DxeCore dequeues the protection request and applies policy.
+
+ Once the image is unloaded, the protection is removed automatically.
+
+Copyright (c) 2017, Intel Corporation. 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 <PiDxe.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/DxeServicesTableLib.h>
+#include <Library/DebugLib.h>
+#include <Library/UefiLib.h>
+
+#include <Guid/EventGroup.h>
+#include <Guid/MemoryAttributesTable.h>
+#include <Guid/PropertiesTable.h>
+
+#include <Protocol/FirmwareVolume2.h>
+#include <Protocol/BlockIo.h>
+#include <Protocol/SimpleFileSystem.h>
+
+#include "DxeMain.h"
+
+#define CACHE_ATTRIBUTE_MASK (EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB | EFI_MEMORY_UCE | EFI_MEMORY_WP)
+#define MEMORY_ATTRIBUTE_MASK (EFI_MEMORY_RP | EFI_MEMORY_XP | EFI_MEMORY_RO)
+
+//
+// Image type definitions
+//
+#define IMAGE_UNKNOWN 0x00000001
+#define IMAGE_FROM_FV 0x00000002
+
+//
+// Protection policy bit definition
+//
+#define DO_NOT_PROTECT 0x00000000
+#define PROTECT_IF_ALIGNED_ELSE_ALLOW 0x00000001
+
+UINT32 mImageProtectionPolicy;
+
+/**
+ Sort code section in image record, based upon CodeSegmentBase from low to high.
+
+ @param ImageRecord image record to be sorted
+**/
+VOID
+SortImageRecordCodeSection (
+ IN IMAGE_PROPERTIES_RECORD *ImageRecord
+ );
+
+/**
+ Check if code section in image record is valid.
+
+ @param ImageRecord image record to be checked
+
+ @retval TRUE image record is valid
+ @retval FALSE image record is invalid
+**/
+BOOLEAN
+IsImageRecordCodeSectionValid (
+ IN IMAGE_PROPERTIES_RECORD *ImageRecord
+ );
+
+/**
+ Get the image type.
+
+ @param[in] File This is a pointer to the device path of the file that is
+ being dispatched.
+
+ @return UINT32 Image Type
+**/
+UINT32
+GetImageType (
+ IN CONST EFI_DEVICE_PATH_PROTOCOL *File
+ )
+{
+ EFI_STATUS Status;
+ EFI_HANDLE DeviceHandle;
+ EFI_DEVICE_PATH_PROTOCOL *TempDevicePath;
+
+ if (File == NULL) {
+ return IMAGE_UNKNOWN;
+ }
+
+ //
+ // First check to see if File is from a Firmware Volume
+ //
+ DeviceHandle = NULL;
+ TempDevicePath = (EFI_DEVICE_PATH_PROTOCOL *) File;
+ Status = gBS->LocateDevicePath (
+ &gEfiFirmwareVolume2ProtocolGuid,
+ &TempDevicePath,
+ &DeviceHandle
+ );
+ if (!EFI_ERROR (Status)) {
+ Status = gBS->OpenProtocol (
+ DeviceHandle,
+ &gEfiFirmwareVolume2ProtocolGuid,
+ NULL,
+ NULL,
+ NULL,
+ EFI_OPEN_PROTOCOL_TEST_PROTOCOL
+ );
+ if (!EFI_ERROR (Status)) {
+ return IMAGE_FROM_FV;
+ }
+ }
+ return IMAGE_UNKNOWN;
+}
+
+/**
+ Get UEFI image protection policy based upon image type.
+
+ @param[in] ImageType The UEFI image type
+
+ @return UEFI image protection policy
+**/
+UINT32
+GetProtectionPolicyFromImageType (
+ IN UINT32 ImageType
+ )
+{
+ if ((ImageType & mImageProtectionPolicy) == 0) {
+ return DO_NOT_PROTECT;
+ } else {
+ return PROTECT_IF_ALIGNED_ELSE_ALLOW;
+ }
+}
+
+/**
+ Get UEFI image protection policy based upon loaded image device path.
+
+ @param[in] LoadedImage The loaded image protocol
+ @param[in] LoadedImageDevicePath The loaded image device path protocol
+
+ @return UEFI image protection policy
+**/
+UINT32
+GetUefiImageProtectionPolicy (
+ IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage,
+ IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath
+ )
+{
+ BOOLEAN InSmm;
+ UINT32 ImageType;
+ UINT32 ProtectionPolicy;
+
+ //
+ // Check SMM
+ //
+ InSmm = FALSE;
+ if (gSmmBase2 != NULL) {
+ gSmmBase2->InSmm (gSmmBase2, &InSmm);
+ }
+ if (InSmm) {
+ return FALSE;
+ }
+
+ //
+ // Check DevicePath
+ //
+ if (LoadedImage == gDxeCoreLoadedImage) {
+ ImageType = IMAGE_FROM_FV;
+ } else {
+ ImageType = GetImageType (LoadedImageDevicePath);
+ }
+ ProtectionPolicy = GetProtectionPolicyFromImageType (ImageType);
+ return ProtectionPolicy;
+}
+
+
+/**
+ Set UEFI image memory attributes.
+
+ @param[in] BaseAddress Specified start address
+ @param[in] Length Specified length
+ @param[in] Attributes Specified attributes
+**/
+VOID
+SetUefiImageMemoryAttributes (
+ IN UINT64 BaseAddress,
+ IN UINT64 Length,
+ IN UINT64 Attributes
+ )
+{
+ EFI_STATUS Status;
+ EFI_GCD_MEMORY_SPACE_DESCRIPTOR Descriptor;
+ UINT64 FinalAttributes;
+
+ Status = CoreGetMemorySpaceDescriptor(BaseAddress, &Descriptor);
+ ASSERT_EFI_ERROR(Status);
+
+ FinalAttributes = (Descriptor.Attributes & CACHE_ATTRIBUTE_MASK) | (Attributes & MEMORY_ATTRIBUTE_MASK);
+
+ DEBUG ((DEBUG_INFO, "SetUefiImageMemoryAttributes - 0x%016lx - 0x%016lx (0x%016lx)\n", BaseAddress, Length, FinalAttributes));
+
+ ASSERT(gCpu != NULL);
+ gCpu->SetMemoryAttributes (gCpu, BaseAddress, Length, FinalAttributes);
+}
+
+/**
+ Set UEFI image protection attributes.
+
+ @param[in] ImageRecord A UEFI image record
+ @param[in] Protect TRUE: Protect the UEFI image.
+ FALSE: Unprotect the UEFI image.
+**/
+VOID
+SetUefiImageProtectionAttributes (
+ IN IMAGE_PROPERTIES_RECORD *ImageRecord,
+ IN BOOLEAN Protect
+ )
+{
+ IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
+ LIST_ENTRY *ImageRecordCodeSectionLink;
+ LIST_ENTRY *ImageRecordCodeSectionEndLink;
+ LIST_ENTRY *ImageRecordCodeSectionList;
+ UINT64 CurrentBase;
+ UINT64 ImageEnd;
+ UINT64 Attribute;
+
+ ImageRecordCodeSectionList = &ImageRecord->CodeSegmentList;
+
+ CurrentBase = ImageRecord->ImageBase;
+ ImageEnd = ImageRecord->ImageBase + ImageRecord->ImageSize;
+
+ ImageRecordCodeSectionLink = ImageRecordCodeSectionList->ForwardLink;
+ ImageRecordCodeSectionEndLink = ImageRecordCodeSectionList;
+ while (ImageRecordCodeSectionLink != ImageRecordCodeSectionEndLink) {
+ ImageRecordCodeSection = CR (
+ ImageRecordCodeSectionLink,
+ IMAGE_PROPERTIES_RECORD_CODE_SECTION,
+ Link,
+ IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE
+ );
+ ImageRecordCodeSectionLink = ImageRecordCodeSectionLink->ForwardLink;
+
+ ASSERT (CurrentBase <= ImageRecordCodeSection->CodeSegmentBase);
+ if (CurrentBase < ImageRecordCodeSection->CodeSegmentBase) {
+ //
+ // DATA
+ //
+ if (Protect) {
+ Attribute = EFI_MEMORY_XP;
+ } else {
+ Attribute = 0;
+ }
+ SetUefiImageMemoryAttributes (
+ CurrentBase,
+ ImageRecordCodeSection->CodeSegmentBase - CurrentBase,
+ Attribute
+ );
+ }
+ //
+ // CODE
+ //
+ if (Protect) {
+ Attribute = EFI_MEMORY_RO;
+ } else {
+ Attribute = 0;
+ }
+ SetUefiImageMemoryAttributes (
+ ImageRecordCodeSection->CodeSegmentBase,
+ ImageRecordCodeSection->CodeSegmentSize,
+ Attribute
+ );
+ CurrentBase = ImageRecordCodeSection->CodeSegmentBase + ImageRecordCodeSection->CodeSegmentSize;
+ }
+ //
+ // Last DATA
+ //
+ ASSERT (CurrentBase <= ImageEnd);
+ if (CurrentBase < ImageEnd) {
+ //
+ // DATA
+ //
+ if (Protect) {
+ Attribute = EFI_MEMORY_XP;
+ } else {
+ Attribute = 0;
+ }
+ SetUefiImageMemoryAttributes (
+ CurrentBase,
+ ImageEnd - CurrentBase,
+ Attribute
+ );
+ }
+ return ;
+}
+
+/**
+ Return if the PE image section is aligned.
+
+ @param[in] SectionAlignment PE/COFF section alignment
+ @param[in] MemoryType PE/COFF image memory type
+
+ @retval TRUE The PE image section is aligned.
+ @retval FALSE The PE image section is not aligned.
+**/
+BOOLEAN
+IsMemoryProtectionSectionAligned (
+ IN UINT32 SectionAlignment,
+ IN EFI_MEMORY_TYPE MemoryType
+ )
+{
+ UINT32 PageAlignment;
+
+ switch (MemoryType) {
+ case EfiRuntimeServicesCode:
+ case EfiACPIMemoryNVS:
+ PageAlignment = EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT;
+ break;
+ case EfiRuntimeServicesData:
+ case EfiACPIReclaimMemory:
+ ASSERT (FALSE);
+ PageAlignment = EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT;
+ break;
+ case EfiBootServicesCode:
+ case EfiLoaderCode:
+ case EfiReservedMemoryType:
+ PageAlignment = EFI_PAGE_SIZE;
+ break;
+ default:
+ ASSERT (FALSE);
+ PageAlignment = EFI_PAGE_SIZE;
+ break;
+ }
+
+ if ((SectionAlignment & (PageAlignment - 1)) != 0) {
+ return FALSE;
+ } else {
+ return TRUE;
+ }
+}
+
+/**
+ Free Image record.
+
+ @param[in] ImageRecord A UEFI image record
+**/
+VOID
+FreeImageRecord (
+ IN IMAGE_PROPERTIES_RECORD *ImageRecord
+ )
+{
+ LIST_ENTRY *CodeSegmentListHead;
+ IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
+
+ CodeSegmentListHead = &ImageRecord->CodeSegmentList;
+ while (!IsListEmpty (CodeSegmentListHead)) {
+ ImageRecordCodeSection = CR (
+ CodeSegmentListHead->ForwardLink,
+ IMAGE_PROPERTIES_RECORD_CODE_SECTION,
+ Link,
+ IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE
+ );
+ RemoveEntryList (&ImageRecordCodeSection->Link);
+ FreePool (ImageRecordCodeSection);
+ }
+
+ if (ImageRecord->Link.ForwardLink != NULL) {
+ RemoveEntryList (&ImageRecord->Link);
+ }
+ FreePool (ImageRecord);
+}
+
+/**
+ Protect or unprotect UEFI image common function.
+
+ @param[in] LoadedImage The loaded image protocol
+ @param[in] LoadedImageDevicePath The loaded image device path protocol
+ @param[in] Protect TRUE: Protect the UEFI image.
+ FALSE: Unprotect the UEFI image.
+**/
+VOID
+ProtectUefiImageCommon (
+ IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage,
+ IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath,
+ IN BOOLEAN Protect
+ )
+{
+ VOID *ImageAddress;
+ EFI_IMAGE_DOS_HEADER *DosHdr;
+ UINT32 PeCoffHeaderOffset;
+ UINT32 SectionAlignment;
+ EFI_IMAGE_SECTION_HEADER *Section;
+ EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr;
+ UINT8 *Name;
+ UINTN Index;
+ IMAGE_PROPERTIES_RECORD *ImageRecord;
+ CHAR8 *PdbPointer;
+ IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
+ UINT16 Magic;
+ BOOLEAN IsAligned;
+ UINT32 ProtectionPolicy;
+
+ DEBUG ((DEBUG_INFO, "ProtectUefiImageCommon - 0x%x\n", LoadedImage));
+ DEBUG ((DEBUG_INFO, " - 0x%016lx - 0x%016lx\n", (EFI_PHYSICAL_ADDRESS)(UINTN)LoadedImage->ImageBase, LoadedImage->ImageSize));
+
+ if (gCpu == NULL) {
+ return ;
+ }
+
+ ProtectionPolicy = GetUefiImageProtectionPolicy (LoadedImage, LoadedImageDevicePath);
+ switch (ProtectionPolicy) {
+ case DO_NOT_PROTECT:
+ return ;
+ case PROTECT_IF_ALIGNED_ELSE_ALLOW:
+ break;
+ default:
+ ASSERT(FALSE);
+ return ;
+ }
+
+ ImageRecord = AllocateZeroPool (sizeof(*ImageRecord));
+ if (ImageRecord == NULL) {
+ return ;
+ }
+ ImageRecord->Signature = IMAGE_PROPERTIES_RECORD_SIGNATURE;
+
+ //
+ // Step 1: record whole region
+ //
+ ImageRecord->ImageBase = (EFI_PHYSICAL_ADDRESS)(UINTN)LoadedImage->ImageBase;
+ ImageRecord->ImageSize = LoadedImage->ImageSize;
+
+ ImageAddress = LoadedImage->ImageBase;
+
+ PdbPointer = PeCoffLoaderGetPdbPointer ((VOID*) (UINTN) ImageAddress);
+ if (PdbPointer != NULL) {
+ DEBUG ((DEBUG_VERBOSE, " Image - %a\n", PdbPointer));
+ }
+
+ //
+ // Check PE/COFF image
+ //
+ DosHdr = (EFI_IMAGE_DOS_HEADER *) (UINTN) ImageAddress;
+ PeCoffHeaderOffset = 0;
+ if (DosHdr->e_magic == EFI_IMAGE_DOS_SIGNATURE) {
+ PeCoffHeaderOffset = DosHdr->e_lfanew;
+ }
+
+ Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINT8 *) (UINTN) ImageAddress + PeCoffHeaderOffset);
+ if (Hdr.Pe32->Signature != EFI_IMAGE_NT_SIGNATURE) {
+ DEBUG ((DEBUG_VERBOSE, "Hdr.Pe32->Signature invalid - 0x%x\n", Hdr.Pe32->Signature));
+ // It might be image in SMM.
+ goto Finish;
+ }
+
+ //
+ // Get SectionAlignment
+ //
+ if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ //
+ // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
+ // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
+ // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
+ // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
+ //
+ Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
+ } else {
+ //
+ // Get the magic value from the PE/COFF Optional Header
+ //
+ Magic = Hdr.Pe32->OptionalHeader.Magic;
+ }
+ if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ SectionAlignment = Hdr.Pe32->OptionalHeader.SectionAlignment;
+ } else {
+ SectionAlignment = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
+ }
+
+ IsAligned = IsMemoryProtectionSectionAligned (SectionAlignment, LoadedImage->ImageCodeType);
+ if (!IsAligned) {
+ DEBUG ((DEBUG_VERBOSE, "!!!!!!!! ProtectUefiImageCommon - Section Alignment(0x%x) is incorrect !!!!!!!!\n",
+ SectionAlignment));
+ PdbPointer = PeCoffLoaderGetPdbPointer ((VOID*) (UINTN) ImageAddress);
+ if (PdbPointer != NULL) {
+ DEBUG ((DEBUG_VERBOSE, "!!!!!!!! Image - %a !!!!!!!!\n", PdbPointer));
+ }
+ goto Finish;
+ }
+
+ Section = (EFI_IMAGE_SECTION_HEADER *) (
+ (UINT8 *) (UINTN) ImageAddress +
+ PeCoffHeaderOffset +
+ sizeof(UINT32) +
+ sizeof(EFI_IMAGE_FILE_HEADER) +
+ Hdr.Pe32->FileHeader.SizeOfOptionalHeader
+ );
+ ImageRecord->CodeSegmentCount = 0;
+ InitializeListHead (&ImageRecord->CodeSegmentList);
+ for (Index = 0; Index < Hdr.Pe32->FileHeader.NumberOfSections; Index++) {
+ Name = Section[Index].Name;
+ DEBUG ((
+ DEBUG_VERBOSE,
+ " Section - '%c%c%c%c%c%c%c%c'\n",
+ Name[0],
+ Name[1],
+ Name[2],
+ Name[3],
+ Name[4],
+ Name[5],
+ Name[6],
+ Name[7]
+ ));
+
+ if ((Section[Index].Characteristics & EFI_IMAGE_SCN_CNT_CODE) != 0) {
+ DEBUG ((DEBUG_VERBOSE, " VirtualSize - 0x%08x\n", Section[Index].Misc.VirtualSize));
+ DEBUG ((DEBUG_VERBOSE, " VirtualAddress - 0x%08x\n", Section[Index].VirtualAddress));
+ DEBUG ((DEBUG_VERBOSE, " SizeOfRawData - 0x%08x\n", Section[Index].SizeOfRawData));
+ DEBUG ((DEBUG_VERBOSE, " PointerToRawData - 0x%08x\n", Section[Index].PointerToRawData));
+ DEBUG ((DEBUG_VERBOSE, " PointerToRelocations - 0x%08x\n", Section[Index].PointerToRelocations));
+ DEBUG ((DEBUG_VERBOSE, " PointerToLinenumbers - 0x%08x\n", Section[Index].PointerToLinenumbers));
+ DEBUG ((DEBUG_VERBOSE, " NumberOfRelocations - 0x%08x\n", Section[Index].NumberOfRelocations));
+ DEBUG ((DEBUG_VERBOSE, " NumberOfLinenumbers - 0x%08x\n", Section[Index].NumberOfLinenumbers));
+ DEBUG ((DEBUG_VERBOSE, " Characteristics - 0x%08x\n", Section[Index].Characteristics));
+
+ //
+ // Step 2: record code section
+ //
+ ImageRecordCodeSection = AllocatePool (sizeof(*ImageRecordCodeSection));
+ if (ImageRecordCodeSection == NULL) {
+ return ;
+ }
+ ImageRecordCodeSection->Signature = IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE;
+
+ ImageRecordCodeSection->CodeSegmentBase = (UINTN)ImageAddress + Section[Index].VirtualAddress;
+ ImageRecordCodeSection->CodeSegmentSize = ALIGN_VALUE(Section[Index].SizeOfRawData, SectionAlignment);
+
+ DEBUG ((DEBUG_VERBOSE, "ImageCode: 0x%016lx - 0x%016lx\n", ImageRecordCodeSection->CodeSegmentBase, ImageRecordCodeSection->CodeSegmentSize));
+
+ InsertTailList (&ImageRecord->CodeSegmentList, &ImageRecordCodeSection->Link);
+ ImageRecord->CodeSegmentCount++;
+ }
+ }
+
+ if (ImageRecord->CodeSegmentCount == 0) {
+ DEBUG ((DEBUG_ERROR, "!!!!!!!! ProtectUefiImageCommon - CodeSegmentCount is 0 !!!!!!!!\n"));
+ PdbPointer = PeCoffLoaderGetPdbPointer ((VOID*) (UINTN) ImageAddress);
+ if (PdbPointer != NULL) {
+ DEBUG ((DEBUG_ERROR, "!!!!!!!! Image - %a !!!!!!!!\n", PdbPointer));
+ }
+ goto Finish;
+ }
+
+ //
+ // Final
+ //
+ SortImageRecordCodeSection (ImageRecord);
+ //
+ // Check overlap all section in ImageBase/Size
+ //
+ if (!IsImageRecordCodeSectionValid (ImageRecord)) {
+ DEBUG ((DEBUG_ERROR, "IsImageRecordCodeSectionValid - FAIL\n"));
+ goto Finish;
+ }
+
+ //
+ // Round up the ImageSize, some CPU arch may return EFI_UNSUPPORTED if ImageSize is not aligned.
+ // Given that the loader always allocates full pages, we know the space after the image is not used.
+ //
+ ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize, EFI_PAGE_SIZE);
+
+ //
+ // CPU ARCH present. Update memory attribute directly.
+ //
+ SetUefiImageProtectionAttributes (ImageRecord, Protect);
+
+ //
+ // Clean up
+ //
+ FreeImageRecord (ImageRecord);
+
+Finish:
+ return ;
+}
+
+/**
+ Protect UEFI image.
+
+ @param[in] LoadedImage The loaded image protocol
+ @param[in] LoadedImageDevicePath The loaded image device path protocol
+**/
+VOID
+ProtectUefiImage (
+ IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage,
+ IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath
+ )
+{
+ if (PcdGet32(PcdImageProtectionPolicy) != 0) {
+ ProtectUefiImageCommon (LoadedImage, LoadedImageDevicePath, TRUE);
+ }
+}
+
+/**
+ Unprotect UEFI image.
+
+ @param[in] LoadedImage The loaded image protocol
+ @param[in] LoadedImageDevicePath The loaded image device path protocol
+**/
+VOID
+UnprotectUefiImage (
+ IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage,
+ IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath
+ )
+{
+ if (PcdGet32(PcdImageProtectionPolicy) != 0) {
+ ProtectUefiImageCommon (LoadedImage, LoadedImageDevicePath, FALSE);
+ }
+}
+
+/**
+ A notification for CPU_ARCH protocol.
+
+ @param[in] Event Event whose notification function is being invoked.
+ @param[in] Context Pointer to the notification function's context,
+ which is implementation-dependent.
+
+**/
+VOID
+EFIAPI
+MemoryProtectionCpuArchProtocolNotify (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ EFI_STATUS Status;
+ EFI_LOADED_IMAGE_PROTOCOL *LoadedImage;
+ EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath;
+ UINTN NoHandles;
+ EFI_HANDLE *HandleBuffer;
+ UINTN Index;
+
+ DEBUG ((DEBUG_INFO, "MemoryProtectionCpuArchProtocolNotify:\n"));
+ Status = CoreLocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&gCpu);
+ if (EFI_ERROR (Status)) {
+ return;
+ }
+
+ Status = gBS->LocateHandleBuffer (
+ ByProtocol,
+ &gEfiLoadedImageProtocolGuid,
+ NULL,
+ &NoHandles,
+ &HandleBuffer
+ );
+ if (EFI_ERROR (Status) && (NoHandles == 0)) {
+ return ;
+ }
+
+ for (Index = 0; Index < NoHandles; Index++) {
+ Status = gBS->HandleProtocol (
+ HandleBuffer[Index],
+ &gEfiLoadedImageProtocolGuid,
+ (VOID **)&LoadedImage
+ );
+ if (EFI_ERROR(Status)) {
+ continue;
+ }
+ Status = gBS->HandleProtocol (
+ HandleBuffer[Index],
+ &gEfiLoadedImageDevicePathProtocolGuid,
+ (VOID **)&LoadedImageDevicePath
+ );
+ if (EFI_ERROR(Status)) {
+ LoadedImageDevicePath = NULL;
+ }
+
+ ProtectUefiImage (LoadedImage, LoadedImageDevicePath);
+ }
+
+ CoreCloseEvent (Event);
+ return;
+}
+
+/**
+ ExitBootServices Callback function for memory protection.
+**/
+VOID
+MemoryProtectionExitBootServicesCallback (
+ VOID
+ )
+{
+ EFI_RUNTIME_IMAGE_ENTRY *RuntimeImage;
+ LIST_ENTRY *Link;
+
+ //
+ // We need remove the RT protection, because RT relocation need write code segment
+ // at SetVirtualAddressMap(). We cannot assume OS/Loader has taken over page table at that time.
+ //
+ // Firmware does not own page tables after ExitBootServices(), so the OS would
+ // have to relax protection of RT code pages across SetVirtualAddressMap(), or
+ // delay setting protections on RT code pages until after SetVirtualAddressMap().
+ // OS may set protection on RT based upon EFI_MEMORY_ATTRIBUTES_TABLE later.
+ //
+ if (mImageProtectionPolicy != 0) {
+ for (Link = gRuntime->ImageHead.ForwardLink; Link != &gRuntime->ImageHead; Link = Link->ForwardLink) {
+ RuntimeImage = BASE_CR (Link, EFI_RUNTIME_IMAGE_ENTRY, Link);
+ SetUefiImageMemoryAttributes ((UINT64)(UINTN)RuntimeImage->ImageBase, ALIGN_VALUE(RuntimeImage->ImageSize, EFI_PAGE_SIZE), 0);
+ }
+ }
+}
+
+/**
+ Initialize Memory Protection support.
+**/
+VOID
+EFIAPI
+CoreInitializeMemoryProtection (
+ VOID
+ )
+{
+ EFI_STATUS Status;
+ EFI_EVENT Event;
+ VOID *Registration;
+
+ mImageProtectionPolicy = PcdGet32(PcdImageProtectionPolicy);
+
+ if (mImageProtectionPolicy != 0) {
+ Status = CoreCreateEvent (
+ EVT_NOTIFY_SIGNAL,
+ TPL_CALLBACK,
+ MemoryProtectionCpuArchProtocolNotify,
+ NULL,
+ &Event
+ );
+ ASSERT_EFI_ERROR(Status);
+
+ //
+ // Register for protocol notifactions on this event
+ //
+ Status = CoreRegisterProtocolNotify (
+ &gEfiCpuArchProtocolGuid,
+ Event,
+ &Registration
+ );
+ ASSERT_EFI_ERROR(Status);
+ }
+ return ;
+}
diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
index 7ecad89..5ea20db 100644
--- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
+++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
@@ -1,7 +1,7 @@
/** @file
UEFI PropertiesTable support
-Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2017, Intel Corporation. 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
@@ -36,26 +36,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#define PREVIOUS_MEMORY_DESCRIPTOR(MemoryDescriptor, Size) \
((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)(MemoryDescriptor) - (Size)))
-#define IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE SIGNATURE_32 ('I','P','R','C')
-
-typedef struct {
- UINT32 Signature;
- LIST_ENTRY Link;
- EFI_PHYSICAL_ADDRESS CodeSegmentBase;
- UINT64 CodeSegmentSize;
-} IMAGE_PROPERTIES_RECORD_CODE_SECTION;
-
-#define IMAGE_PROPERTIES_RECORD_SIGNATURE SIGNATURE_32 ('I','P','R','D')
-
-typedef struct {
- UINT32 Signature;
- LIST_ENTRY Link;
- EFI_PHYSICAL_ADDRESS ImageBase;
- UINT64 ImageSize;
- UINTN CodeSegmentCount;
- LIST_ENTRY CodeSegmentList;
-} IMAGE_PROPERTIES_RECORD;
-
#define IMAGE_PROPERTIES_PRIVATE_DATA_SIGNATURE SIGNATURE_32 ('I','P','P','D')
typedef struct {
@@ -864,7 +844,6 @@ SwapImageRecordCodeSection (
@param ImageRecord image record to be sorted
**/
-STATIC
VOID
SortImageRecordCodeSection (
IN IMAGE_PROPERTIES_RECORD *ImageRecord
@@ -915,7 +894,6 @@ SortImageRecordCodeSection (
@retval TRUE image record is valid
@retval FALSE image record is invalid
**/
-STATIC
BOOLEAN
IsImageRecordCodeSectionValid (
IN IMAGE_PROPERTIES_RECORD *ImageRecord
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH V4 0/3] DXE Memory Protection
2017-02-21 6:57 [PATCH V4 0/3] DXE Memory Protection Jiewen Yao
` (2 preceding siblings ...)
2017-02-21 6:57 ` [PATCH V4 3/3] MdeModulePkg/DxeCore: Add UEFI image protection Jiewen Yao
@ 2017-02-21 7:23 ` Fan, Jeff
2017-02-21 8:36 ` Ard Biesheuvel
4 siblings, 0 replies; 12+ messages in thread
From: Fan, Jeff @ 2017-02-21 7:23 UTC (permalink / raw)
To: Yao, Jiewen, edk2-devel@lists.01.org
Cc: Kinney, Michael D, Leif Lindholm, Ard Biesheuvel, Zeng, Star,
Tian, Feng
Reviewed-by: Jeff Fan <jeff.fan@intel.com>
-----Original Message-----
From: Yao, Jiewen
Sent: Tuesday, February 21, 2017 2:57 PM
To: edk2-devel@lists.01.org
Cc: Fan, Jeff; Kinney, Michael D; Leif Lindholm; Ard Biesheuvel; Zeng, Star; Tian, Feng
Subject: [PATCH V4 0/3] DXE Memory Protection
==== V4 ====
1) Remove ARM patch. (Which was already submitted by Ard Biesheuvel in another series)
2) Unprotect RT image at ExitBootServices (feedback from Ard Biesheuvel)
3) Round up the ImageSize on protection (feedback from Ard Biesheuvel)
==== V3 ====
1) Add PCD for policy control (feedback from Ard Biesheuvel) (Discussed with Mike Kinney)
+ # BIT0 - Image from unknown device. <BR>
+ # BIT1 - Image from firmware volume.<BR>
+ # @Prompt Set image protection policy.
+ # @ValidRange 0x80000002 | 0x00000000 - 0x0000001F
+
+ gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x00000002|UIN
+ T32|0x00001047
2) Remove unused function in CpuDxe.(feedback from Liming Gao)
3) Add commit log on link option assumption (feedback from Feng Tian)
4) Rename file PageTableLib.h/.c to CpuPageTable.h/.c file (from Jeff Fan)
5) Remove multi-entrypoint usage (from Liming Gao/Mike Kinney)
==== V2 ====
1) Clean up ArmPkg, (feedback from Leif Lindholm)
==== V1 ====
This series patch provides capability to protect PE/COFF image in DXE memory.
If the UEFI image is page aligned, the image code section is set to read only and the image data section is set to non-executable.
The DxeCore calls CpuArchProtocol->SetMemoryAttributes() to protect the image.
Tested platform: NT32/Quark IA32/OVMF IA32/OVMF IA32X64/Intel internal X64/ Tested OS: UEFI Win10, UEFI Ubuntu 16.04.
Untested platform: ARM/AARCH64.
Can ARM/AARCH64 owner help to take a look and try the ARM platform?
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
Jiewen Yao (3):
UefiCpuPkg/CpuDxe: Add memory attribute setting.
MdeModulePkg/dec: add PcdImageProtectionPolicy.
MdeModulePkg/DxeCore: Add UEFI image protection.
MdeModulePkg/Core/Dxe/DxeMain.h | 61 ++
MdeModulePkg/Core/Dxe/DxeMain.inf | 5 +-
MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 5 +-
MdeModulePkg/Core/Dxe/Image/Image.c | 7 +-
MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 769 +++++++++++++++++++ MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 24 +-
MdeModulePkg/MdeModulePkg.dec | 10 +
UefiCpuPkg/CpuDxe/CpuDxe.c | 141 ++--
UefiCpuPkg/CpuDxe/CpuDxe.inf | 5 +-
UefiCpuPkg/CpuDxe/CpuPageTable.c | 779 ++++++++++++++++++++
UefiCpuPkg/CpuDxe/CpuPageTable.h | 113 +++
11 files changed, 1832 insertions(+), 87 deletions(-) create mode 100644 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.c create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.h
--
2.7.4.windows.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V4 0/3] DXE Memory Protection
2017-02-21 6:57 [PATCH V4 0/3] DXE Memory Protection Jiewen Yao
` (3 preceding siblings ...)
2017-02-21 7:23 ` [PATCH V4 0/3] DXE Memory Protection Fan, Jeff
@ 2017-02-21 8:36 ` Ard Biesheuvel
2017-02-21 8:39 ` Yao, Jiewen
4 siblings, 1 reply; 12+ messages in thread
From: Ard Biesheuvel @ 2017-02-21 8:36 UTC (permalink / raw)
To: Jiewen Yao, Leif Lindholm
Cc: edk2-devel@lists.01.org, Jeff Fan, Michael Kinney, Star Zeng,
Feng Tian
On 21 February 2017 at 06:57, Jiewen Yao <jiewen.yao@intel.com> wrote:
> ==== V4 ====
> 1) Remove ARM patch. (Which was already submitted by Ard Biesheuvel in another series)
> 2) Unprotect RT image at ExitBootServices (feedback from Ard Biesheuvel)
> 3) Round up the ImageSize on protection (feedback from Ard Biesheuvel)
>
Hello Jiewen,
This works fine now on AARCH64.
Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Could you please wait a little bit before merging this? Leif and I
need to get the AARCH64 prepatory patches merged first, but I am
hoping to do that today.
Thanks,
Ard.
> ==== V3 ====
> 1) Add PCD for policy control (feedback from Ard Biesheuvel)
> (Discussed with Mike Kinney)
> + # BIT0 - Image from unknown device. <BR>
> + # BIT1 - Image from firmware volume.<BR>
> + # @Prompt Set image protection policy.
> + # @ValidRange 0x80000002 | 0x00000000 - 0x0000001F
> + gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x00000002|UINT32|0x00001047
>
> 2) Remove unused function in CpuDxe.(feedback from Liming Gao)
> 3) Add commit log on link option assumption (feedback from Feng Tian)
> 4) Rename file PageTableLib.h/.c to CpuPageTable.h/.c file (from Jeff Fan)
> 5) Remove multi-entrypoint usage (from Liming Gao/Mike Kinney)
>
> ==== V2 ====
> 1) Clean up ArmPkg, (feedback from Leif Lindholm)
>
> ==== V1 ====
> This series patch provides capability to protect PE/COFF image
> in DXE memory.
> If the UEFI image is page aligned, the image code section is set to read
> only and the image data section is set to non-executable.
>
> The DxeCore calls CpuArchProtocol->SetMemoryAttributes() to protect
> the image.
>
> Tested platform: NT32/Quark IA32/OVMF IA32/OVMF IA32X64/Intel internal X64/
> Tested OS: UEFI Win10, UEFI Ubuntu 16.04.
>
> Untested platform: ARM/AARCH64.
> Can ARM/AARCH64 owner help to take a look and try the ARM platform?
>
>
> Cc: Jeff Fan <jeff.fan@intel.com>
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
>
>
> Jiewen Yao (3):
> UefiCpuPkg/CpuDxe: Add memory attribute setting.
> MdeModulePkg/dec: add PcdImageProtectionPolicy.
> MdeModulePkg/DxeCore: Add UEFI image protection.
>
> MdeModulePkg/Core/Dxe/DxeMain.h | 61 ++
> MdeModulePkg/Core/Dxe/DxeMain.inf | 5 +-
> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 5 +-
> MdeModulePkg/Core/Dxe/Image/Image.c | 7 +-
> MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 769 +++++++++++++++++++
> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 24 +-
> MdeModulePkg/MdeModulePkg.dec | 10 +
> UefiCpuPkg/CpuDxe/CpuDxe.c | 141 ++--
> UefiCpuPkg/CpuDxe/CpuDxe.inf | 5 +-
> UefiCpuPkg/CpuDxe/CpuPageTable.c | 779 ++++++++++++++++++++
> UefiCpuPkg/CpuDxe/CpuPageTable.h | 113 +++
> 11 files changed, 1832 insertions(+), 87 deletions(-)
> create mode 100644 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.c
> create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.h
>
> --
> 2.7.4.windows.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V4 0/3] DXE Memory Protection
2017-02-21 8:36 ` Ard Biesheuvel
@ 2017-02-21 8:39 ` Yao, Jiewen
2017-02-21 17:25 ` Ard Biesheuvel
0 siblings, 1 reply; 12+ messages in thread
From: Yao, Jiewen @ 2017-02-21 8:39 UTC (permalink / raw)
To: Ard Biesheuvel, Leif Lindholm
Cc: edk2-devel@lists.01.org, Fan, Jeff, Kinney, Michael D, Zeng, Star,
Tian, Feng
No problem. ☺
I appreciate your help to confirm it working on AARCH64.
Thank you
Yao Jiewen
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: Tuesday, February 21, 2017 4:36 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; Leif Lindholm <leif.lindholm@linaro.org>
Cc: edk2-devel@lists.01.org; Fan, Jeff <jeff.fan@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Zeng, Star <star.zeng@intel.com>; Tian, Feng <feng.tian@intel.com>
Subject: Re: [PATCH V4 0/3] DXE Memory Protection
On 21 February 2017 at 06:57, Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
> ==== V4 ====
> 1) Remove ARM patch. (Which was already submitted by Ard Biesheuvel in another series)
> 2) Unprotect RT image at ExitBootServices (feedback from Ard Biesheuvel)
> 3) Round up the ImageSize on protection (feedback from Ard Biesheuvel)
>
Hello Jiewen,
This works fine now on AARCH64.
Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>>
Could you please wait a little bit before merging this? Leif and I
need to get the AARCH64 prepatory patches merged first, but I am
hoping to do that today.
Thanks,
Ard.
> ==== V3 ====
> 1) Add PCD for policy control (feedback from Ard Biesheuvel)
> (Discussed with Mike Kinney)
> + # BIT0 - Image from unknown device. <BR>
> + # BIT1 - Image from firmware volume.<BR>
> + # @Prompt Set image protection policy.
> + # @ValidRange 0x80000002 | 0x00000000 - 0x0000001F
> + gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x00000002|UINT32|0x00001047
>
> 2) Remove unused function in CpuDxe.(feedback from Liming Gao)
> 3) Add commit log on link option assumption (feedback from Feng Tian)
> 4) Rename file PageTableLib.h/.c to CpuPageTable.h/.c file (from Jeff Fan)
> 5) Remove multi-entrypoint usage (from Liming Gao/Mike Kinney)
>
> ==== V2 ====
> 1) Clean up ArmPkg, (feedback from Leif Lindholm)
>
> ==== V1 ====
> This series patch provides capability to protect PE/COFF image
> in DXE memory.
> If the UEFI image is page aligned, the image code section is set to read
> only and the image data section is set to non-executable.
>
> The DxeCore calls CpuArchProtocol->SetMemoryAttributes() to protect
> the image.
>
> Tested platform: NT32/Quark IA32/OVMF IA32/OVMF IA32X64/Intel internal X64/
> Tested OS: UEFI Win10, UEFI Ubuntu 16.04.
>
> Untested platform: ARM/AARCH64.
> Can ARM/AARCH64 owner help to take a look and try the ARM platform?
>
>
> Cc: Jeff Fan <jeff.fan@intel.com<mailto:jeff.fan@intel.com>>
> Cc: Michael Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> Cc: Leif Lindholm <leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>>
> Cc: Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com>>
> Cc: Feng Tian <feng.tian@intel.com<mailto:feng.tian@intel.com>>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
>
>
> Jiewen Yao (3):
> UefiCpuPkg/CpuDxe: Add memory attribute setting.
> MdeModulePkg/dec: add PcdImageProtectionPolicy.
> MdeModulePkg/DxeCore: Add UEFI image protection.
>
> MdeModulePkg/Core/Dxe/DxeMain.h | 61 ++
> MdeModulePkg/Core/Dxe/DxeMain.inf | 5 +-
> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 5 +-
> MdeModulePkg/Core/Dxe/Image/Image.c | 7 +-
> MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 769 +++++++++++++++++++
> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 24 +-
> MdeModulePkg/MdeModulePkg.dec | 10 +
> UefiCpuPkg/CpuDxe/CpuDxe.c | 141 ++--
> UefiCpuPkg/CpuDxe/CpuDxe.inf | 5 +-
> UefiCpuPkg/CpuDxe/CpuPageTable.c | 779 ++++++++++++++++++++
> UefiCpuPkg/CpuDxe/CpuPageTable.h | 113 +++
> 11 files changed, 1832 insertions(+), 87 deletions(-)
> create mode 100644 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.c
> create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.h
>
> --
> 2.7.4.windows.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V4 0/3] DXE Memory Protection
2017-02-21 8:39 ` Yao, Jiewen
@ 2017-02-21 17:25 ` Ard Biesheuvel
0 siblings, 0 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2017-02-21 17:25 UTC (permalink / raw)
To: Yao, Jiewen
Cc: Leif Lindholm, edk2-devel@lists.01.org, Fan, Jeff,
Kinney, Michael D, Zeng, Star, Tian, Feng
On 21 February 2017 at 08:39, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> No problem. J
>
Pushed now. Thanks for you patience.
>
>
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Tuesday, February 21, 2017 4:36 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; Leif Lindholm
> <leif.lindholm@linaro.org>
> Cc: edk2-devel@lists.01.org; Fan, Jeff <jeff.fan@intel.com>; Kinney, Michael
> D <michael.d.kinney@intel.com>; Zeng, Star <star.zeng@intel.com>; Tian, Feng
> <feng.tian@intel.com>
> Subject: Re: [PATCH V4 0/3] DXE Memory Protection
>
>
>
> On 21 February 2017 at 06:57, Jiewen Yao <jiewen.yao@intel.com> wrote:
>> ==== V4 ====
>> 1) Remove ARM patch. (Which was already submitted by Ard Biesheuvel in
>> another series)
>> 2) Unprotect RT image at ExitBootServices (feedback from Ard Biesheuvel)
>> 3) Round up the ImageSize on protection (feedback from Ard Biesheuvel)
>>
>
> Hello Jiewen,
>
> This works fine now on AARCH64.
>
> Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Could you please wait a little bit before merging this? Leif and I
> need to get the AARCH64 prepatory patches merged first, but I am
> hoping to do that today.
>
> Thanks,
> Ard.
>
>> ==== V3 ====
>> 1) Add PCD for policy control (feedback from Ard Biesheuvel)
>> (Discussed with Mike Kinney)
>> + # BIT0 - Image from unknown device. <BR>
>> + # BIT1 - Image from firmware volume.<BR>
>> + # @Prompt Set image protection policy.
>> + # @ValidRange 0x80000002 | 0x00000000 - 0x0000001F
>> +
>> gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x00000002|UINT32|0x00001047
>>
>> 2) Remove unused function in CpuDxe.(feedback from Liming Gao)
>> 3) Add commit log on link option assumption (feedback from Feng Tian)
>> 4) Rename file PageTableLib.h/.c to CpuPageTable.h/.c file (from Jeff Fan)
>> 5) Remove multi-entrypoint usage (from Liming Gao/Mike Kinney)
>>
>> ==== V2 ====
>> 1) Clean up ArmPkg, (feedback from Leif Lindholm)
>>
>> ==== V1 ====
>> This series patch provides capability to protect PE/COFF image
>> in DXE memory.
>> If the UEFI image is page aligned, the image code section is set to read
>> only and the image data section is set to non-executable.
>>
>> The DxeCore calls CpuArchProtocol->SetMemoryAttributes() to protect
>> the image.
>>
>> Tested platform: NT32/Quark IA32/OVMF IA32/OVMF IA32X64/Intel internal
>> X64/
>> Tested OS: UEFI Win10, UEFI Ubuntu 16.04.
>>
>> Untested platform: ARM/AARCH64.
>> Can ARM/AARCH64 owner help to take a look and try the ARM platform?
>>
>>
>> Cc: Jeff Fan <jeff.fan@intel.com>
>> Cc: Michael Kinney <michael.d.kinney@intel.com>
>> Cc: Leif Lindholm <leif.lindholm@linaro.org>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: Star Zeng <star.zeng@intel.com>
>> Cc: Feng Tian <feng.tian@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
>>
>>
>> Jiewen Yao (3):
>> UefiCpuPkg/CpuDxe: Add memory attribute setting.
>> MdeModulePkg/dec: add PcdImageProtectionPolicy.
>> MdeModulePkg/DxeCore: Add UEFI image protection.
>>
>> MdeModulePkg/Core/Dxe/DxeMain.h | 61 ++
>> MdeModulePkg/Core/Dxe/DxeMain.inf | 5 +-
>> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 5 +-
>> MdeModulePkg/Core/Dxe/Image/Image.c | 7 +-
>> MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 769 +++++++++++++++++++
>> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 24 +-
>> MdeModulePkg/MdeModulePkg.dec | 10 +
>> UefiCpuPkg/CpuDxe/CpuDxe.c | 141 ++--
>> UefiCpuPkg/CpuDxe/CpuDxe.inf | 5 +-
>> UefiCpuPkg/CpuDxe/CpuPageTable.c | 779 ++++++++++++++++++++
>> UefiCpuPkg/CpuDxe/CpuPageTable.h | 113 +++
>> 11 files changed, 1832 insertions(+), 87 deletions(-)
>> create mode 100644 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
>> create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.c
>> create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.h
>>
>> --
>> 2.7.4.windows.1
>>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V4 1/3] UefiCpuPkg/CpuDxe: Add memory attribute setting.
2017-02-21 6:57 ` [PATCH V4 1/3] UefiCpuPkg/CpuDxe: Add memory attribute setting Jiewen Yao
@ 2017-03-09 11:52 ` Anthony PERARD
2017-03-10 1:02 ` Fan, Jeff
0 siblings, 1 reply; 12+ messages in thread
From: Anthony PERARD @ 2017-03-09 11:52 UTC (permalink / raw)
To: Jiewen Yao; +Cc: edk2-devel, Michael Kinney, Jeff Fan
On Tue, Feb 21, 2017 at 02:57:07PM +0800, Jiewen Yao wrote:
> Add memory attribute setting in CpuArch protocol.
> Previous SetMemoryAttributes() API only supports cache attribute setting.
>
> This patch updated SetMemoryAttributes() API to support memory attribute
> setting by updating CPU page table.
>
> Cc: Jeff Fan <jeff.fan@intel.com>
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
Hi,
This patch doesn't work on AMD (when running OVMF on Xen). With an AMD
cpu, reading MSR 0x1a0 cause a General Protection Fault.
> + AsmCpuid (0x80000000, &RegEax, NULL, NULL, NULL);
> + if (RegEax > 0x80000000) {
> + AsmCpuid (0x80000001, NULL, NULL, NULL, &RegEdx);
> + if ((RegEdx & BIT20) != 0) {
> + // XD supported
This next read is where the fault is taken.
> + if ((AsmReadMsr64 (0x000001A0) & BIT34) == 0) {
> + // XD enabled
> + if ((AsmReadMsr64 (0xC0000080) & BIT11) != 0) {
> + // XD activated
> + PagingContext->ContextData.Ia32.Attributes |= PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_XD_ACTIVATED;
> + }
> + }
> + }
>From OVMF output:
!!!! X64 Exception Type - 0D(#GP - General Protection) CPU Apic ID - 00000000 !!!!
RIP - 000000002F323ABE, CS - 0000000000000018, RFLAGS - 0000000000010002
ExceptionData - 0000000000000000
RAX - 0000000080000001, RCX - 00000000000001A0, RDX - 000000002FD3FBFF
RBX - 000000002F422CB8, RSP - 000000002F422C68, RBP - 000000002EE0AD98
RSI - 000000002F3AF018, RDI - 000000002F3229E4
R8 - 0000000000000000, R9 - 0000000000000000, R10 - 0000000000000000
R11 - 000000002F422BAD, R12 - 0000000000000000, R13 - 0000000000000000
R14 - 0000000000000000, R15 - 000000002F44A980
DS - 0000000000000008, ES - 0000000000000008, FS - 0000000000000008
GS - 0000000000000008, SS - 0000000000000008
CR0 - 00000000C0010033, CR2 - 0000000000000000, CR3 - 000000002F3C1000
CR4 - 0000000000000668, CR8 - 0000000000000000
DR0 - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000
DR3 - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400
GDTR - 00000000FFFFFF80 000000000000001F, LDTR - 0000000000000000
IDTR - 000000002B68DD90 000000000000021F, TR - 0000000000000000
FXSAVE_STATE - 000000002F4228C0
!!!! Find PE image /home/osstest/build.106538.build-amd64/xen/tools/firmware/ovmf-dir-remote/Build/OvmfX64/DEBUG_GCC49/X64/UefiCpuPkg/CpuDxe/CpuDxe/DEBUG/CpuDxe.dll (ImageBase=000000002F321000, EntryPoint=000000002F321271) !!!!
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V4 1/3] UefiCpuPkg/CpuDxe: Add memory attribute setting.
2017-03-09 11:52 ` Anthony PERARD
@ 2017-03-10 1:02 ` Fan, Jeff
2017-03-10 15:21 ` Anthony PERARD
0 siblings, 1 reply; 12+ messages in thread
From: Fan, Jeff @ 2017-03-10 1:02 UTC (permalink / raw)
To: Anthony PERARD, Yao, Jiewen; +Cc: edk2-devel@lists.01.org, Kinney, Michael D
Anthony,
MSR 0x1A0 is architectural MSR defined in IA32 SDM.
Have you tried AMD real platform?
Thanks!
Jeff
-----Original Message-----
From: Anthony PERARD [mailto:anthony.perard@citrix.com]
Sent: Thursday, March 09, 2017 7:53 PM
To: Yao, Jiewen
Cc: edk2-devel@lists.01.org; Kinney, Michael D; Fan, Jeff
Subject: Re: [edk2] [PATCH V4 1/3] UefiCpuPkg/CpuDxe: Add memory attribute setting.
On Tue, Feb 21, 2017 at 02:57:07PM +0800, Jiewen Yao wrote:
> Add memory attribute setting in CpuArch protocol.
> Previous SetMemoryAttributes() API only supports cache attribute setting.
>
> This patch updated SetMemoryAttributes() API to support memory
> attribute setting by updating CPU page table.
>
> Cc: Jeff Fan <jeff.fan@intel.com>
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
Hi,
This patch doesn't work on AMD (when running OVMF on Xen). With an AMD cpu, reading MSR 0x1a0 cause a General Protection Fault.
> + AsmCpuid (0x80000000, &RegEax, NULL, NULL, NULL); if (RegEax >
> + 0x80000000) {
> + AsmCpuid (0x80000001, NULL, NULL, NULL, &RegEdx);
> + if ((RegEdx & BIT20) != 0) {
> + // XD supported
This next read is where the fault is taken.
> + if ((AsmReadMsr64 (0x000001A0) & BIT34) == 0) {
> + // XD enabled
> + if ((AsmReadMsr64 (0xC0000080) & BIT11) != 0) {
> + // XD activated
> + PagingContext->ContextData.Ia32.Attributes |= PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_XD_ACTIVATED;
> + }
> + }
> + }
>From OVMF output:
!!!! X64 Exception Type - 0D(#GP - General Protection) CPU Apic ID - 00000000 !!!!
RIP - 000000002F323ABE, CS - 0000000000000018, RFLAGS - 0000000000010002 ExceptionData - 0000000000000000 RAX - 0000000080000001, RCX - 00000000000001A0, RDX - 000000002FD3FBFF RBX - 000000002F422CB8, RSP - 000000002F422C68, RBP - 000000002EE0AD98 RSI - 000000002F3AF018, RDI - 000000002F3229E4
R8 - 0000000000000000, R9 - 0000000000000000, R10 - 0000000000000000
R11 - 000000002F422BAD, R12 - 0000000000000000, R13 - 0000000000000000
R14 - 0000000000000000, R15 - 000000002F44A980
DS - 0000000000000008, ES - 0000000000000008, FS - 0000000000000008
GS - 0000000000000008, SS - 0000000000000008
CR0 - 00000000C0010033, CR2 - 0000000000000000, CR3 - 000000002F3C1000
CR4 - 0000000000000668, CR8 - 0000000000000000
DR0 - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000
DR3 - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400 GDTR - 00000000FFFFFF80 000000000000001F, LDTR - 0000000000000000
IDTR - 000000002B68DD90 000000000000021F, TR - 0000000000000000
FXSAVE_STATE - 000000002F4228C0
!!!! Find PE image /home/osstest/build.106538.build-amd64/xen/tools/firmware/ovmf-dir-remote/Build/OvmfX64/DEBUG_GCC49/X64/UefiCpuPkg/CpuDxe/CpuDxe/DEBUG/CpuDxe.dll (ImageBase=000000002F321000, EntryPoint=000000002F321271) !!!!
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V4 1/3] UefiCpuPkg/CpuDxe: Add memory attribute setting.
2017-03-10 1:02 ` Fan, Jeff
@ 2017-03-10 15:21 ` Anthony PERARD
2017-03-13 1:58 ` Fan, Jeff
0 siblings, 1 reply; 12+ messages in thread
From: Anthony PERARD @ 2017-03-10 15:21 UTC (permalink / raw)
To: Fan, Jeff; +Cc: Yao, Jiewen, edk2-devel@lists.01.org, Kinney, Michael D
On Fri, Mar 10, 2017 at 01:02:22AM +0000, Fan, Jeff wrote:
> Anthony,
>
> MSR 0x1A0 is architectural MSR defined in IA32 SDM.
>
> Have you tried AMD real platform?
Yes, I did, and unless I've made a mistake, trying to read msr 0x1a0
does generate a GPF.
Also, I've now take a look into the AMD manuals[1] and more specificaly
"AMD64 Architecture Programmer’s Manual Volume 2: System
Programming"[2], there is no sign of MSR 0x1a0 (in Appendix A).
As for the XD bit on AMD, (well NX actually), it is describe at 5.6.3,
in the "Page Translation and Protection" chapter.
About MSR 0x1a0 been architectural defined, I guess that applies only to
Intel CPUs, all of them.
[1]: http://developer.amd.com/resources/developer-guides-manuals/
[2]: http://support.amd.com/TechDocs/24593.pdf
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V4 1/3] UefiCpuPkg/CpuDxe: Add memory attribute setting.
2017-03-10 15:21 ` Anthony PERARD
@ 2017-03-13 1:58 ` Fan, Jeff
0 siblings, 0 replies; 12+ messages in thread
From: Fan, Jeff @ 2017-03-13 1:58 UTC (permalink / raw)
To: Anthony PERARD; +Cc: Yao, Jiewen, edk2-devel@lists.01.org, Kinney, Michael D
Anthony,
It's very interesting. Some INTEL IA-32 architectural MSR (like 0x1A0, IA32_MISC_ENABLE) is not supported by AMD processors.
For this case, I think reading 0x1A0 is not necessary. Reading CPUID.80000001H:EDK[20] is enough to check if XD feature is supported or not.
I will create the patch to remove the following check.
if ((AsmReadMsr64 (0x000001A0) & BIT34) == 0) {
// XD enabled
I suggest to review such issue case by case when we encounter unsupported architectural MSRs on AMD processors in the future.
Thanks!
Jeff
-----Original Message-----
From: Anthony PERARD [mailto:anthony.perard@citrix.com]
Sent: Friday, March 10, 2017 11:22 PM
To: Fan, Jeff
Cc: Yao, Jiewen; edk2-devel@lists.01.org; Kinney, Michael D
Subject: Re: [edk2] [PATCH V4 1/3] UefiCpuPkg/CpuDxe: Add memory attribute setting.
On Fri, Mar 10, 2017 at 01:02:22AM +0000, Fan, Jeff wrote:
> Anthony,
>
> MSR 0x1A0 is architectural MSR defined in IA32 SDM.
>
> Have you tried AMD real platform?
Yes, I did, and unless I've made a mistake, trying to read msr 0x1a0 does generate a GPF.
Also, I've now take a look into the AMD manuals[1] and more specificaly
"AMD64 Architecture Programmer’s Manual Volume 2: System Programming"[2], there is no sign of MSR 0x1a0 (in Appendix A).
As for the XD bit on AMD, (well NX actually), it is describe at 5.6.3, in the "Page Translation and Protection" chapter.
About MSR 0x1a0 been architectural defined, I guess that applies only to Intel CPUs, all of them.
[1]: http://developer.amd.com/resources/developer-guides-manuals/
[2]: http://support.amd.com/TechDocs/24593.pdf
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-03-13 1:58 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-21 6:57 [PATCH V4 0/3] DXE Memory Protection Jiewen Yao
2017-02-21 6:57 ` [PATCH V4 1/3] UefiCpuPkg/CpuDxe: Add memory attribute setting Jiewen Yao
2017-03-09 11:52 ` Anthony PERARD
2017-03-10 1:02 ` Fan, Jeff
2017-03-10 15:21 ` Anthony PERARD
2017-03-13 1:58 ` Fan, Jeff
2017-02-21 6:57 ` [PATCH V4 2/3] MdeModulePkg/dec: add PcdImageProtectionPolicy Jiewen Yao
2017-02-21 6:57 ` [PATCH V4 3/3] MdeModulePkg/DxeCore: Add UEFI image protection Jiewen Yao
2017-02-21 7:23 ` [PATCH V4 0/3] DXE Memory Protection Fan, Jeff
2017-02-21 8:36 ` Ard Biesheuvel
2017-02-21 8:39 ` Yao, Jiewen
2017-02-21 17:25 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox