Hi Ard
Liming gave me minor update comment, and I have finished V3.
Should I wait for your validation on AArch64?
Thank you
Yao Jiewen
-----Original Message-----
From: Gao, Liming
Sent: Tuesday, February 23, 2016 1:10 PM
To: Yao, Jiewen; edk2-de...@ml01.01.org
Cc: Ard Biesheuvel
Subject: RE: [patch V3] MdeModulePkg: Fix Memory Attributes table type issue
Reviewed-by: Liming Gao <liming....@intel.com> -----Original Message-----
From: Yao, Jiewen
Sent: Tuesday, February 23, 2016 12:47 PM
To: edk2-de...@ml01.01.org
Cc: Yao, Jiewen <jiewen....@intel.com>; Ard Biesheuvel
<ard.biesheu...@linaro.org>; Gao, Liming <liming....@intel.com>
Subject: [patch V3] MdeModulePkg: Fix Memory Attributes table type issue
According to the spec, each entry in the Memory Attributes table shall have the
same type as the region it was carved out of in the UEFI memory map.
The current attribute uses RTData for PE Data, but it should be RTCode.
This patch fixed the issue. It is validated with or without PropertiesTable.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: "Yao, Jiewen" <jiewen....@intel.com>
Cc: "Ard Biesheuvel" <ard.biesheu...@linaro.org>
Cc: "Gao, Liming" <liming....@intel.com>
---
MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 6 --
MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 114 +++++++--------------
2 files changed, 37 insertions(+), 83 deletions(-)
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
index c84146a..416ed3d 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
@@ -72,8 +72,6 @@ CoreGetMemoryMapPropertiesTable (
extern EFI_PROPERTIES_TABLE mPropertiesTable;
-BOOLEAN mIsConstructingMemoryAttributesTable = FALSE;
-
/**
Install MemoryAttributesTable.
@@ -105,8 +103,6 @@ InstallMemoryAttributesTable (
return ;
}
- mIsConstructingMemoryAttributesTable = TRUE;
-
MemoryMapSize = 0;
MemoryMap = NULL;
Status = CoreGetMemoryMapPropertiesTable ( @@ -181,8 +177,6 @@
InstallMemoryAttributesTable (
Status = gBS->InstallConfigurationTable (&gEfiMemoryAttributesTableGuid,
MemoryAttributesTable);
ASSERT_EFI_ERROR (Status);
-
- mIsConstructingMemoryAttributesTable = FALSE; }
/**
diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
index 6e5ad03..ebe7096 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, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2016, 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 @@ -80,14 +80,7 @@
EFI_PROPERTIES_TABLE mPropertiesTable = {
EFI_LOCK mPropertiesTableLock = EFI_INITIALIZE_LOCK_VARIABLE
(TPL_NOTIFY);
-//
-// Temporary save for original memory map.
-// This is for MemoryAttributesTable only.
-//
-extern BOOLEAN mIsConstructingMemoryAttributesTable;
-EFI_MEMORY_DESCRIPTOR *mMemoryMapOrg;
-UINTN mMemoryMapOrgSize;
-UINTN mDescriptorSize;
+BOOLEAN mPropertiesTableEnable;
//
// Below functions are for MemoryMap
@@ -199,42 +192,6 @@ SortMemoryMap (
}
/**
- Check if this memory entry spans across original memory map boundary.
-
- @param PhysicalStart The PhysicalStart of memory
- @param NumberOfPages The NumberOfPages of memory
-
- @retval TRUE This memory entry spans across original memory map boundary.
- @retval FALSE This memory entry does not span cross original memory map
boundary.
-**/
-STATIC
-BOOLEAN
-DoesEntrySpanAcrossBoundary (
- IN UINT64 PhysicalStart,
- IN UINT64 NumberOfPages
- )
-{
- EFI_MEMORY_DESCRIPTOR *MemoryMapEntry;
- EFI_MEMORY_DESCRIPTOR *MemoryMapEnd;
- UINT64 MemoryBlockLength;
-
- MemoryMapEntry = mMemoryMapOrg;
- MemoryMapEnd = (EFI_MEMORY_DESCRIPTOR *) ((UINT8 *) mMemoryMapOrg +
mMemoryMapOrgSize);
- while (MemoryMapEntry < MemoryMapEnd) {
- MemoryBlockLength = (UINT64) (EfiPagesToSize
(MemoryMapEntry->NumberOfPages));
-
- if ((MemoryMapEntry->PhysicalStart <= PhysicalStart) &&
- (MemoryMapEntry->PhysicalStart + MemoryBlockLength > PhysicalStart) &&
- (MemoryMapEntry->PhysicalStart + MemoryBlockLength < PhysicalStart +
EfiPagesToSize (NumberOfPages))) {
- return TRUE;
- }
-
- MemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, mDescriptorSize);
- }
- return FALSE;
-}
-
-/**
Merge continous memory map entries whose have same attributes.
@param MemoryMap A pointer to the buffer in which firmware
places
@@ -271,8 +228,7 @@ MergeMemoryMap (
if (((UINTN)NextMemoryMapEntry < (UINTN)MemoryMapEnd) &&
(MemoryMapEntry->Type == NextMemoryMapEntry->Type) &&
(MemoryMapEntry->Attribute == NextMemoryMapEntry->Attribute) &&
- ((MemoryMapEntry->PhysicalStart + MemoryBlockLength) ==
NextMemoryMapEntry->PhysicalStart) &&
- (!DoesEntrySpanAcrossBoundary (MemoryMapEntry->PhysicalStart,
MemoryMapEntry->NumberOfPages + NextMemoryMapEntry->NumberOfPages))) {
+ ((MemoryMapEntry->PhysicalStart + MemoryBlockLength) ==
+ NextMemoryMapEntry->PhysicalStart)) {
MemoryMapEntry->NumberOfPages += NextMemoryMapEntry->NumberOfPages;
if (NewMemoryMapEntry != MemoryMapEntry) {
NewMemoryMapEntry->NumberOfPages +=
NextMemoryMapEntry->NumberOfPages;
@@ -430,7 +386,11 @@ SetNewRecord (
//
// DATA
//
- NewRecord->Type = EfiRuntimeServicesData;
+ if (!mPropertiesTableEnable) {
+ NewRecord->Type = TempRecord.Type;
+ } else {
+ NewRecord->Type = EfiRuntimeServicesData;
+ }
NewRecord->PhysicalStart = TempRecord.PhysicalStart;
NewRecord->VirtualStart = 0;
NewRecord->NumberOfPages =
EfiSizeToPages(ImageRecordCodeSection->CodeSegmentBase -
NewRecord->PhysicalStart); @@ -443,7 +403,11 @@ SetNewRecord (
//
// CODE
//
- NewRecord->Type = EfiRuntimeServicesCode;
+ if (!mPropertiesTableEnable) {
+ NewRecord->Type = TempRecord.Type;
+ } else {
+ NewRecord->Type = EfiRuntimeServicesCode;
+ }
NewRecord->PhysicalStart = ImageRecordCodeSection->CodeSegmentBase;
NewRecord->VirtualStart = 0;
NewRecord->NumberOfPages =
EfiSizeToPages(ImageRecordCodeSection->CodeSegmentSize);
@@ -467,7 +431,11 @@ SetNewRecord (
// Final DATA
//
if (TempRecord.PhysicalStart < ImageEnd) {
- NewRecord->Type = EfiRuntimeServicesData;
+ if (!mPropertiesTableEnable) {
+ NewRecord->Type = TempRecord.Type;
+ } else {
+ NewRecord->Type = EfiRuntimeServicesData;
+ }
NewRecord->PhysicalStart = TempRecord.PhysicalStart;
NewRecord->VirtualStart = 0;
NewRecord->NumberOfPages = EfiSizeToPages (ImageEnd -
TempRecord.PhysicalStart); @@ -549,6 +517,7 @@ SplitRecord (
UINT64 PhysicalEnd;
UINTN NewRecordCount;
UINTN TotalNewRecordCount;
+ BOOLEAN IsLastRecordData;
if (MaxSplitRecordCount == 0) {
CopyMem (NewRecord, OldRecord, DescriptorSize); @@ -576,7 +545,17 @@
SplitRecord (
// If this is still address in this record, need record.
//
NewRecord = PREVIOUS_MEMORY_DESCRIPTOR (NewRecord, DescriptorSize);
- if (NewRecord->Type == EfiRuntimeServicesData) {
+ IsLastRecordData = FALSE;
+ if (!mPropertiesTableEnable) {
+ if ((NewRecord->Attribute & EFI_MEMORY_XP) != 0) {
+ IsLastRecordData = TRUE;
+ }
+ } else {
+ if (NewRecord->Type == EfiRuntimeServicesData) {
+ IsLastRecordData = TRUE;
+ }
+ }
+ if (IsLastRecordData) {
//
// Last record is DATA, just merge it.
//
@@ -586,7 +565,11 @@ SplitRecord (
// Last record is CODE, create a new DATA entry.
//
NewRecord = NEXT_MEMORY_DESCRIPTOR (NewRecord, DescriptorSize);
- NewRecord->Type = EfiRuntimeServicesData;
+ if (!mPropertiesTableEnable) {
+ NewRecord->Type = TempRecord.Type;
+ } else {
+ NewRecord->Type = EfiRuntimeServicesData;
+ }
NewRecord->PhysicalStart = TempRecord.PhysicalStart;
NewRecord->VirtualStart = 0;
NewRecord->NumberOfPages = TempRecord.NumberOfPages; @@ -814,38
+797,13 @@ CoreGetMemoryMapPropertiesTable (
//
Status = EFI_BUFFER_TOO_SMALL;
} else {
- if (mIsConstructingMemoryAttributesTable) {
- //
- // If the memory map is constructed for memory attributes table,
- // save original memory map, because they will be checked later
- // to make sure the memory attributes table entry does not cross
- // the original memory map entry boundary.
- // This work must NOT be done in normal GetMemoryMap() because
- // allocating memory is not allowed due to MapKey update.
- //
- mDescriptorSize = *DescriptorSize;
- mMemoryMapOrgSize = *MemoryMapSize;
- mMemoryMapOrg = AllocateCopyPool (*MemoryMapSize, MemoryMap);
- if (mMemoryMapOrg == NULL) {
- Status = EFI_OUT_OF_RESOURCES;
- goto Exit;
- }
- }
-
//
// Split PE code/data
//
SplitTable (MemoryMapSize, MemoryMap, *DescriptorSize);
-
- if (mIsConstructingMemoryAttributesTable) {
- FreePool (mMemoryMapOrg);
- mMemoryMapOrg = NULL;
- mMemoryMapOrgSize = 0;
- }
}
}
-Exit:
CoreReleasePropertiesTableLock ();
return Status;
}
@@ -1412,6 +1370,8 @@ InstallPropertiesTable (
DEBUG ((EFI_D_VERBOSE, "Total Image Count - 0x%x\n",
mImagePropertiesPrivateData.ImageRecordCount));
DEBUG ((EFI_D_VERBOSE, "Dump ImageRecord:\n"));
DumpImageRecord ();
+
+ mPropertiesTableEnable = TRUE;
}
}
--
1.9.5.msysgit.0
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
