* [PATCH] MdeModulePkg/PiSmmCore: AllocatePool should use MemoryType.
@ 2016-12-01 8:23 Jiewen Yao
2016-12-01 8:23 ` [PATCH] MdeModulePkg/PiSmmCore: MemoryAttributeTable need keep non-PE record Jiewen Yao
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Jiewen Yao @ 2016-12-01 8:23 UTC (permalink / raw)
To: edk2-devel; +Cc: Jeff Fan, Michael D Kinney, Laszlo Ersek
PiSmmCore supports page level protection based upon the Memory Type
(EfiRuntimeServicesCode/EfiRuntimeServicesData) and PE image.
However, the Memory Type information is ignored in AllocatePool().
If a caller calls AllocatePool with EfiRuntimeServicesCode,
the final memory is still allocated as EfiRuntimeServicesData.
This patch supports AllocatePool with EfiRuntimeServicesCode.
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 13 ++-
MdeModulePkg/Core/PiSmmCore/Pool.c | 66 +++++++++---
MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c | 114 +++++++++++---------
3 files changed, 124 insertions(+), 69 deletions(-)
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
index e2fee54..8df1e50 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
@@ -1109,8 +1109,9 @@ extern LIST_ENTRY mSmmMemoryMap;
#define MAX_POOL_INDEX (MAX_POOL_SHIFT - MIN_POOL_SHIFT + 1)
typedef struct {
- UINTN Size;
- BOOLEAN Available;
+ UINTN Size;
+ BOOLEAN Available;
+ EFI_MEMORY_TYPE Type;
} POOL_HEADER;
typedef struct {
@@ -1118,6 +1119,12 @@ typedef struct {
LIST_ENTRY Link;
} FREE_POOL_HEADER;
-extern LIST_ENTRY mSmmPoolLists[MAX_POOL_INDEX];
+typedef enum {
+ SmmPoolTypeCode,
+ SmmPoolTypeData,
+ SmmPoolTypeMax,
+} SMM_POOL_TYPE;
+
+extern LIST_ENTRY mSmmPoolLists[SmmPoolTypeMax][MAX_POOL_INDEX];
#endif
diff --git a/MdeModulePkg/Core/PiSmmCore/Pool.c b/MdeModulePkg/Core/PiSmmCore/Pool.c
index dcfd13e..6fb426c 100644
--- a/MdeModulePkg/Core/PiSmmCore/Pool.c
+++ b/MdeModulePkg/Core/PiSmmCore/Pool.c
@@ -14,7 +14,7 @@
#include "PiSmmCore.h"
-LIST_ENTRY mSmmPoolLists[MAX_POOL_INDEX];
+LIST_ENTRY mSmmPoolLists[SmmPoolTypeMax][MAX_POOL_INDEX];
//
// To cache the SMRAM base since when Loading modules At fixed address feature is enabled,
// all module is assigned an offset relative the SMRAM base in build time.
@@ -22,6 +22,30 @@ LIST_ENTRY mSmmPoolLists[MAX_POOL_INDEX];
GLOBAL_REMOVE_IF_UNREFERENCED EFI_PHYSICAL_ADDRESS gLoadModuleAtFixAddressSmramBase = 0;
/**
+ Convert a UEFI memory type to SMM pool type.
+
+ @param[in] PoolType Type of pool to allocate.
+
+ @return SMM pool type
+**/
+SMM_POOL_TYPE
+UefiMemoryTypeToSmmPoolType (
+ IN EFI_MEMORY_TYPE MemoryType
+ )
+{
+ ASSERT ((MemoryType == EfiRuntimeServicesCode) || (MemoryType == EfiRuntimeServicesData));
+ switch (MemoryType) {
+ case EfiRuntimeServicesCode:
+ return SmmPoolTypeCode;
+ case EfiRuntimeServicesData:
+ return SmmPoolTypeData;
+ default:
+ return SmmPoolTypeMax;
+ }
+}
+
+
+/**
Called to initialize the memory service.
@param SmramRangeCount Number of SMRAM Regions
@@ -35,15 +59,18 @@ SmmInitializeMemoryServices (
)
{
UINTN Index;
- UINT64 SmmCodeSize;
- UINTN CurrentSmramRangesIndex;
- UINT64 MaxSize;
+ UINT64 SmmCodeSize;
+ UINTN CurrentSmramRangesIndex;
+ UINT64 MaxSize;
+ UINTN SmmPoolTypeIndex;
//
// Initialize Pool list
//
- for (Index = ARRAY_SIZE (mSmmPoolLists); Index > 0;) {
- InitializeListHead (&mSmmPoolLists[--Index]);
+ for (SmmPoolTypeIndex = 0; SmmPoolTypeIndex < SmmPoolTypeMax; SmmPoolTypeIndex++) {
+ for (Index = 0; Index < ARRAY_SIZE (mSmmPoolLists[SmmPoolTypeIndex]); Index++) {
+ InitializeListHead (&mSmmPoolLists[SmmPoolTypeIndex][Index]);
+ }
}
CurrentSmramRangesIndex = 0;
//
@@ -117,6 +144,7 @@ SmmInitializeMemoryServices (
/**
Internal Function. Allocate a pool by specified PoolIndex.
+ @param PoolType Type of pool to allocate.
@param PoolIndex Index which indicate the Pool size.
@param FreePoolHdr The returned Free pool.
@@ -126,6 +154,7 @@ SmmInitializeMemoryServices (
**/
EFI_STATUS
InternalAllocPoolByIndex (
+ IN EFI_MEMORY_TYPE PoolType,
IN UINTN PoolIndex,
OUT FREE_POOL_HEADER **FreePoolHdr
)
@@ -133,25 +162,29 @@ InternalAllocPoolByIndex (
EFI_STATUS Status;
FREE_POOL_HEADER *Hdr;
EFI_PHYSICAL_ADDRESS Address;
+ SMM_POOL_TYPE SmmPoolType;
+
+ SmmPoolType = UefiMemoryTypeToSmmPoolType(PoolType);
ASSERT (PoolIndex <= MAX_POOL_INDEX);
Status = EFI_SUCCESS;
Hdr = NULL;
if (PoolIndex == MAX_POOL_INDEX) {
- Status = SmmInternalAllocatePages (AllocateAnyPages, EfiRuntimeServicesData, EFI_SIZE_TO_PAGES (MAX_POOL_SIZE << 1), &Address);
+ Status = SmmInternalAllocatePages (AllocateAnyPages, PoolType, EFI_SIZE_TO_PAGES (MAX_POOL_SIZE << 1), &Address);
if (EFI_ERROR (Status)) {
return EFI_OUT_OF_RESOURCES;
}
Hdr = (FREE_POOL_HEADER *) (UINTN) Address;
- } else if (!IsListEmpty (&mSmmPoolLists[PoolIndex])) {
- Hdr = BASE_CR (GetFirstNode (&mSmmPoolLists[PoolIndex]), FREE_POOL_HEADER, Link);
+ } else if (!IsListEmpty (&mSmmPoolLists[SmmPoolType][PoolIndex])) {
+ Hdr = BASE_CR (GetFirstNode (&mSmmPoolLists[SmmPoolType][PoolIndex]), FREE_POOL_HEADER, Link);
RemoveEntryList (&Hdr->Link);
} else {
- Status = InternalAllocPoolByIndex (PoolIndex + 1, &Hdr);
+ Status = InternalAllocPoolByIndex (PoolType, PoolIndex + 1, &Hdr);
if (!EFI_ERROR (Status)) {
Hdr->Header.Size >>= 1;
Hdr->Header.Available = TRUE;
- InsertHeadList (&mSmmPoolLists[PoolIndex], &Hdr->Link);
+ Hdr->Header.Type = PoolType;
+ InsertHeadList (&mSmmPoolLists[SmmPoolType][PoolIndex], &Hdr->Link);
Hdr = (FREE_POOL_HEADER*)((UINT8*)Hdr + Hdr->Header.Size);
}
}
@@ -159,6 +192,7 @@ InternalAllocPoolByIndex (
if (!EFI_ERROR (Status)) {
Hdr->Header.Size = MIN_POOL_SIZE << PoolIndex;
Hdr->Header.Available = FALSE;
+ Hdr->Header.Type = PoolType;
}
*FreePoolHdr = Hdr;
@@ -178,16 +212,19 @@ InternalFreePoolByIndex (
IN FREE_POOL_HEADER *FreePoolHdr
)
{
- UINTN PoolIndex;
+ UINTN PoolIndex;
+ SMM_POOL_TYPE SmmPoolType;
ASSERT ((FreePoolHdr->Header.Size & (FreePoolHdr->Header.Size - 1)) == 0);
ASSERT (((UINTN)FreePoolHdr & (FreePoolHdr->Header.Size - 1)) == 0);
ASSERT (FreePoolHdr->Header.Size >= MIN_POOL_SIZE);
+ SmmPoolType = UefiMemoryTypeToSmmPoolType(FreePoolHdr->Header.Type);
+
PoolIndex = (UINTN) (HighBitSet32 ((UINT32)FreePoolHdr->Header.Size) - MIN_POOL_SHIFT);
FreePoolHdr->Header.Available = TRUE;
ASSERT (PoolIndex < MAX_POOL_INDEX);
- InsertHeadList (&mSmmPoolLists[PoolIndex], &FreePoolHdr->Link);
+ InsertHeadList (&mSmmPoolLists[SmmPoolType][PoolIndex], &FreePoolHdr->Link);
return EFI_SUCCESS;
}
@@ -234,6 +271,7 @@ SmmInternalAllocatePool (
PoolHdr = (POOL_HEADER*)(UINTN)Address;
PoolHdr->Size = EFI_PAGES_TO_SIZE (Size);
PoolHdr->Available = FALSE;
+ PoolHdr->Type = PoolType;
*Buffer = PoolHdr + 1;
return Status;
}
@@ -244,7 +282,7 @@ SmmInternalAllocatePool (
PoolIndex++;
}
- Status = InternalAllocPoolByIndex (PoolIndex, &FreePoolHdr);
+ Status = InternalAllocPoolByIndex (PoolType, PoolIndex, &FreePoolHdr);
if (!EFI_ERROR(Status)) {
*Buffer = &FreePoolHdr->Header + 1;
}
diff --git a/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c b/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c
index d983cef..dda9f12 100644
--- a/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c
+++ b/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c
@@ -1596,6 +1596,7 @@ SmramProfileGetDataSize (
FREE_POOL_HEADER *Pool;
UINTN PoolListIndex;
UINTN Index;
+ UINTN SmmPoolTypeIndex;
ContextData = GetSmramProfileContext ();
if (ContextData == NULL) {
@@ -1638,19 +1639,20 @@ SmramProfileGetDataSize (
Node = Node->BackLink) {
Index++;
}
- for (PoolListIndex = 0; PoolListIndex < MAX_POOL_INDEX; PoolListIndex++) {
- FreePoolList = &mSmmPoolLists[PoolListIndex];
- for (Node = FreePoolList->BackLink;
- Node != FreePoolList;
- Node = Node->BackLink) {
- Pool = BASE_CR (Node, FREE_POOL_HEADER, Link);
- if (Pool->Header.Available) {
- Index++;
+ for (SmmPoolTypeIndex = 0; SmmPoolTypeIndex < SmmPoolTypeMax; SmmPoolTypeIndex++) {
+ for (PoolListIndex = 0; PoolListIndex < MAX_POOL_INDEX; PoolListIndex++) {
+ FreePoolList = &mSmmPoolLists[SmmPoolTypeIndex][PoolListIndex];
+ for (Node = FreePoolList->BackLink;
+ Node != FreePoolList;
+ Node = Node->BackLink) {
+ Pool = BASE_CR (Node, FREE_POOL_HEADER, Link);
+ if (Pool->Header.Available) {
+ Index++;
+ }
}
}
}
-
TotalSize += (sizeof (MEMORY_PROFILE_FREE_MEMORY) + Index * sizeof (MEMORY_PROFILE_DESCRIPTOR));
TotalSize += (sizeof (MEMORY_PROFILE_MEMORY_RANGE) + mFullSmramRangeCount * sizeof (MEMORY_PROFILE_DESCRIPTOR));
@@ -1698,6 +1700,7 @@ SmramProfileCopyData (
UINT64 RemainingSize;
UINTN PdbSize;
UINTN ActionStringSize;
+ UINTN SmmPoolTypeIndex;
ContextData = GetSmramProfileContext ();
if (ContextData == NULL) {
@@ -1785,14 +1788,16 @@ SmramProfileCopyData (
Node = Node->BackLink) {
Index++;
}
- for (PoolListIndex = 0; PoolListIndex < MAX_POOL_INDEX; PoolListIndex++) {
- FreePoolList = &mSmmPoolLists[MAX_POOL_INDEX - PoolListIndex - 1];
- for (Node = FreePoolList->BackLink;
- Node != FreePoolList;
- Node = Node->BackLink) {
- Pool = BASE_CR (Node, FREE_POOL_HEADER, Link);
- if (Pool->Header.Available) {
- Index++;
+ for (SmmPoolTypeIndex = 0; SmmPoolTypeIndex < SmmPoolTypeMax; SmmPoolTypeIndex++) {
+ for (PoolListIndex = 0; PoolListIndex < MAX_POOL_INDEX; PoolListIndex++) {
+ FreePoolList = &mSmmPoolLists[SmmPoolTypeIndex][MAX_POOL_INDEX - PoolListIndex - 1];
+ for (Node = FreePoolList->BackLink;
+ Node != FreePoolList;
+ Node = Node->BackLink) {
+ Pool = BASE_CR (Node, FREE_POOL_HEADER, Link);
+ if (Pool->Header.Available) {
+ Index++;
+ }
}
}
}
@@ -1827,29 +1832,31 @@ SmramProfileCopyData (
}
Offset += sizeof (MEMORY_PROFILE_DESCRIPTOR);
}
- for (PoolListIndex = 0; PoolListIndex < MAX_POOL_INDEX; PoolListIndex++) {
- FreePoolList = &mSmmPoolLists[MAX_POOL_INDEX - PoolListIndex - 1];
- for (Node = FreePoolList->BackLink;
- Node != FreePoolList;
- Node = Node->BackLink) {
- Pool = BASE_CR (Node, FREE_POOL_HEADER, Link);
- if (Pool->Header.Available) {
- if (*ProfileOffset < (Offset + sizeof (MEMORY_PROFILE_DESCRIPTOR))) {
- if (RemainingSize >= sizeof (MEMORY_PROFILE_DESCRIPTOR)) {
- MemoryProfileDescriptor = ProfileBuffer;
- MemoryProfileDescriptor->Header.Signature = MEMORY_PROFILE_DESCRIPTOR_SIGNATURE;
- MemoryProfileDescriptor->Header.Length = sizeof (MEMORY_PROFILE_DESCRIPTOR);
- MemoryProfileDescriptor->Header.Revision = MEMORY_PROFILE_DESCRIPTOR_REVISION;
- MemoryProfileDescriptor->Address = (PHYSICAL_ADDRESS) (UINTN) Pool;
- MemoryProfileDescriptor->Size = Pool->Header.Size;
-
- RemainingSize -= sizeof (MEMORY_PROFILE_DESCRIPTOR);
- ProfileBuffer = (UINT8 *) ProfileBuffer + sizeof (MEMORY_PROFILE_DESCRIPTOR);
- } else {
- goto Done;
+ for (SmmPoolTypeIndex = 0; SmmPoolTypeIndex < SmmPoolTypeMax; SmmPoolTypeIndex++) {
+ for (PoolListIndex = 0; PoolListIndex < MAX_POOL_INDEX; PoolListIndex++) {
+ FreePoolList = &mSmmPoolLists[SmmPoolTypeIndex][MAX_POOL_INDEX - PoolListIndex - 1];
+ for (Node = FreePoolList->BackLink;
+ Node != FreePoolList;
+ Node = Node->BackLink) {
+ Pool = BASE_CR (Node, FREE_POOL_HEADER, Link);
+ if (Pool->Header.Available) {
+ if (*ProfileOffset < (Offset + sizeof (MEMORY_PROFILE_DESCRIPTOR))) {
+ if (RemainingSize >= sizeof (MEMORY_PROFILE_DESCRIPTOR)) {
+ MemoryProfileDescriptor = ProfileBuffer;
+ MemoryProfileDescriptor->Header.Signature = MEMORY_PROFILE_DESCRIPTOR_SIGNATURE;
+ MemoryProfileDescriptor->Header.Length = sizeof (MEMORY_PROFILE_DESCRIPTOR);
+ MemoryProfileDescriptor->Header.Revision = MEMORY_PROFILE_DESCRIPTOR_REVISION;
+ MemoryProfileDescriptor->Address = (PHYSICAL_ADDRESS) (UINTN) Pool;
+ MemoryProfileDescriptor->Size = Pool->Header.Size;
+
+ RemainingSize -= sizeof (MEMORY_PROFILE_DESCRIPTOR);
+ ProfileBuffer = (UINT8 *) ProfileBuffer + sizeof (MEMORY_PROFILE_DESCRIPTOR);
+ } else {
+ goto Done;
+ }
}
+ Offset += sizeof (MEMORY_PROFILE_DESCRIPTOR);
}
- Offset += sizeof (MEMORY_PROFILE_DESCRIPTOR);
}
}
}
@@ -2577,6 +2584,7 @@ DumpFreePoolList (
UINTN PoolListIndex;
MEMORY_PROFILE_CONTEXT_DATA *ContextData;
BOOLEAN SmramProfileGettingStatus;
+ UINTN SmmPoolTypeIndex;
ContextData = GetSmramProfileContext ();
if (ContextData == NULL) {
@@ -2586,23 +2594,25 @@ DumpFreePoolList (
SmramProfileGettingStatus = mSmramProfileGettingStatus;
mSmramProfileGettingStatus = TRUE;
- DEBUG ((EFI_D_INFO, "======= SmramProfile begin =======\n"));
-
- for (PoolListIndex = 0; PoolListIndex < MAX_POOL_INDEX; PoolListIndex++) {
- DEBUG ((EFI_D_INFO, "FreePoolList (%d):\n", PoolListIndex));
- FreePoolList = &mSmmPoolLists[PoolListIndex];
- for (Node = FreePoolList->BackLink, Index = 0;
- Node != FreePoolList;
- Node = Node->BackLink, Index++) {
- Pool = BASE_CR (Node, FREE_POOL_HEADER, Link);
- DEBUG ((EFI_D_INFO, " Index - 0x%x\n", Index));
- DEBUG ((EFI_D_INFO, " PhysicalStart - 0x%016lx\n", (PHYSICAL_ADDRESS) (UINTN) Pool));
- DEBUG ((EFI_D_INFO, " Size - 0x%08x\n", Pool->Header.Size));
- DEBUG ((EFI_D_INFO, " Available - 0x%02x\n", Pool->Header.Available));
+ DEBUG ((DEBUG_INFO, "======= SmramProfile begin =======\n"));
+
+ for (SmmPoolTypeIndex = 0; SmmPoolTypeIndex < SmmPoolTypeMax; SmmPoolTypeIndex++) {
+ for (PoolListIndex = 0; PoolListIndex < MAX_POOL_INDEX; PoolListIndex++) {
+ DEBUG ((DEBUG_INFO, "FreePoolList(%d)(%d):\n", SmmPoolTypeIndex, PoolListIndex));
+ FreePoolList = &mSmmPoolLists[SmmPoolTypeIndex][PoolListIndex];
+ for (Node = FreePoolList->BackLink, Index = 0;
+ Node != FreePoolList;
+ Node = Node->BackLink, Index++) {
+ Pool = BASE_CR (Node, FREE_POOL_HEADER, Link);
+ DEBUG ((DEBUG_INFO, " Index - 0x%x\n", Index));
+ DEBUG ((DEBUG_INFO, " PhysicalStart - 0x%016lx\n", (PHYSICAL_ADDRESS) (UINTN) Pool));
+ DEBUG ((DEBUG_INFO, " Size - 0x%08x\n", Pool->Header.Size));
+ DEBUG ((DEBUG_INFO, " Available - 0x%02x\n", Pool->Header.Available));
+ }
}
}
- DEBUG ((EFI_D_INFO, "======= SmramProfile end =======\n"));
+ DEBUG ((DEBUG_INFO, "======= SmramProfile end =======\n"));
mSmramProfileGettingStatus = SmramProfileGettingStatus;
}
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] MdeModulePkg/PiSmmCore: MemoryAttributeTable need keep non-PE record.
2016-12-01 8:23 [PATCH] MdeModulePkg/PiSmmCore: AllocatePool should use MemoryType Jiewen Yao
@ 2016-12-01 8:23 ` Jiewen Yao
2016-12-07 5:27 ` Fan, Jeff
2016-12-01 8:23 ` [PATCH] MdeModulePkg/PiSmmCore; Use DEBUG_WARN for non 4k aligned image Jiewen Yao
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Jiewen Yao @ 2016-12-01 8:23 UTC (permalink / raw)
To: edk2-devel; +Cc: Jeff Fan, Michael D Kinney, Laszlo Ersek
Current memory attribute table implementation will only mark PE code
to be EfiRuntimeServicesCode, and mark rest to be EfiRuntimeServicesData.
However, there might be a case that a SMM code wants to allocate
EfiRuntimeServicesCode explicitly to let page table protect this region
to be read only. It is unsupported.
This patch enhances the current solution so that MemoryAttributeTable
does not touch non PE image record.
Only the PE image region is forced to be EfiRuntimeServicesCode for
code and EfiRuntimeServicesData for data.
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c | 91 +++++++++++---------
1 file changed, 51 insertions(+), 40 deletions(-)
diff --git a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c b/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
index 3a5a2c8..a6ab830 100644
--- a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
+++ b/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
@@ -268,15 +268,19 @@ EnforceMemoryMapAttribute (
MemoryMapEntry = MemoryMap;
MemoryMapEnd = (EFI_MEMORY_DESCRIPTOR *) ((UINT8 *) MemoryMap + MemoryMapSize);
while ((UINTN)MemoryMapEntry < (UINTN)MemoryMapEnd) {
- switch (MemoryMapEntry->Type) {
- case EfiRuntimeServicesCode:
- MemoryMapEntry->Attribute |= EFI_MEMORY_RO;
- break;
- case EfiRuntimeServicesData:
- MemoryMapEntry->Attribute |= EFI_MEMORY_XP;
- break;
+ if (MemoryMapEntry->Attribute != 0) {
+ // It is PE image, the attribute is already set.
+ } else {
+ switch (MemoryMapEntry->Type) {
+ case EfiRuntimeServicesCode:
+ MemoryMapEntry->Attribute = EFI_MEMORY_RO;
+ break;
+ case EfiRuntimeServicesData:
+ default:
+ MemoryMapEntry->Attribute |= EFI_MEMORY_XP;
+ break;
+ }
}
-
MemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorSize);
}
@@ -358,6 +362,21 @@ SetNewRecord (
PhysicalEnd = TempRecord.PhysicalStart + EfiPagesToSize(TempRecord.NumberOfPages);
NewRecordCount = 0;
+ //
+ // Always create a new entry for non-PE image record
+ //
+ if (ImageRecord->ImageBase > TempRecord.PhysicalStart) {
+ NewRecord->Type = TempRecord.Type;
+ NewRecord->PhysicalStart = TempRecord.PhysicalStart;
+ NewRecord->VirtualStart = 0;
+ NewRecord->NumberOfPages = EfiSizeToPages(ImageRecord->ImageBase - TempRecord.PhysicalStart);
+ NewRecord->Attribute = TempRecord.Attribute;
+ NewRecord = NEXT_MEMORY_DESCRIPTOR (NewRecord, DescriptorSize);
+ NewRecordCount ++;
+ TempRecord.PhysicalStart = ImageRecord->ImageBase;
+ TempRecord.NumberOfPages = EfiSizeToPages(PhysicalEnd - TempRecord.PhysicalStart);
+ }
+
ImageRecordCodeSectionList = &ImageRecord->CodeSegmentList;
ImageRecordCodeSectionLink = ImageRecordCodeSectionList->ForwardLink;
@@ -452,14 +471,10 @@ GetMaxSplitRecordCount (
if (ImageRecord == NULL) {
break;
}
- SplitRecordCount += (2 * ImageRecord->CodeSegmentCount + 1);
+ SplitRecordCount += (2 * ImageRecord->CodeSegmentCount + 2);
PhysicalStart = ImageRecord->ImageBase + ImageRecord->ImageSize;
} while ((ImageRecord != NULL) && (PhysicalStart < PhysicalEnd));
- if (SplitRecordCount != 0) {
- SplitRecordCount--;
- }
-
return SplitRecordCount;
}
@@ -516,28 +531,16 @@ SplitRecord (
//
// No more image covered by this range, stop
//
- if ((PhysicalEnd > PhysicalStart) && (ImageRecord != NULL)) {
+ if (PhysicalEnd > PhysicalStart) {
//
- // If this is still address in this record, need record.
+ // Always create a new entry for non-PE image record
//
- NewRecord = PREVIOUS_MEMORY_DESCRIPTOR (NewRecord, DescriptorSize);
- if (NewRecord->Type == EfiRuntimeServicesData) {
- //
- // Last record is DATA, just merge it.
- //
- NewRecord->NumberOfPages = EfiSizeToPages(PhysicalEnd - NewRecord->PhysicalStart);
- } else {
- //
- // Last record is CODE, create a new DATA entry.
- //
- NewRecord = NEXT_MEMORY_DESCRIPTOR (NewRecord, DescriptorSize);
- NewRecord->Type = EfiRuntimeServicesData;
- NewRecord->PhysicalStart = TempRecord.PhysicalStart;
- NewRecord->VirtualStart = 0;
- NewRecord->NumberOfPages = TempRecord.NumberOfPages;
- NewRecord->Attribute = TempRecord.Attribute | EFI_MEMORY_XP;
- TotalNewRecordCount ++;
- }
+ NewRecord->Type = TempRecord.Type;
+ NewRecord->PhysicalStart = TempRecord.PhysicalStart;
+ NewRecord->VirtualStart = 0;
+ NewRecord->NumberOfPages = TempRecord.NumberOfPages;
+ NewRecord->Attribute = TempRecord.Attribute;
+ TotalNewRecordCount ++;
}
break;
}
@@ -580,6 +583,8 @@ SplitRecord (
==>
+---------------+
| Record X |
+ +---------------+
+ | Record RtCode |
+---------------+ ----
| Record RtData | |
+---------------+ |
@@ -587,12 +592,16 @@ SplitRecord (
+---------------+ |
| Record RtData | |
+---------------+ ----
+ | Record RtCode |
+ +---------------+ ----
| Record RtData | |
+---------------+ |
| Record RtCode | |-> PE/COFF2
+---------------+ |
| Record RtData | |
+---------------+ ----
+ | Record RtCode |
+ +---------------+
| Record Y |
+---------------+
@@ -622,7 +631,7 @@ SplitTable (
UINTN TotalSplitRecordCount;
UINTN AdditionalRecordCount;
- AdditionalRecordCount = (2 * mImagePropertiesPrivateData.CodeSegmentCountMax + 1) * mImagePropertiesPrivateData.ImageRecordCount;
+ AdditionalRecordCount = (2 * mImagePropertiesPrivateData.CodeSegmentCountMax + 2) * mImagePropertiesPrivateData.ImageRecordCount;
TotalSplitRecordCount = 0;
//
@@ -648,11 +657,13 @@ SplitTable (
//
// Adjust IndexNew according to real split.
//
- CopyMem (
- ((UINT8 *)MemoryMap + (IndexNew + MaxSplitRecordCount - RealSplitRecordCount) * DescriptorSize),
- ((UINT8 *)MemoryMap + IndexNew * DescriptorSize),
- RealSplitRecordCount * DescriptorSize
- );
+ if (MaxSplitRecordCount != RealSplitRecordCount) {
+ CopyMem (
+ ((UINT8 *)MemoryMap + (IndexNew + MaxSplitRecordCount - RealSplitRecordCount) * DescriptorSize),
+ ((UINT8 *)MemoryMap + IndexNew * DescriptorSize),
+ (RealSplitRecordCount + 1) * DescriptorSize
+ );
+ }
IndexNew = IndexNew + MaxSplitRecordCount - RealSplitRecordCount;
TotalSplitRecordCount += RealSplitRecordCount;
IndexNew --;
@@ -744,7 +755,7 @@ SmmCoreGetMemoryMapMemoryAttributesTable (
return EFI_INVALID_PARAMETER;
}
- AdditionalRecordCount = (2 * mImagePropertiesPrivateData.CodeSegmentCountMax + 1) * mImagePropertiesPrivateData.ImageRecordCount;
+ AdditionalRecordCount = (2 * mImagePropertiesPrivateData.CodeSegmentCountMax + 2) * mImagePropertiesPrivateData.ImageRecordCount;
OldMemoryMapSize = *MemoryMapSize;
Status = SmmCoreGetMemoryMap (MemoryMapSize, MemoryMap, MapKey, DescriptorSize, DescriptorVersion);
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] MdeModulePkg/PiSmmCore; Use DEBUG_WARN for non 4k aligned image.
2016-12-01 8:23 [PATCH] MdeModulePkg/PiSmmCore: AllocatePool should use MemoryType Jiewen Yao
2016-12-01 8:23 ` [PATCH] MdeModulePkg/PiSmmCore: MemoryAttributeTable need keep non-PE record Jiewen Yao
@ 2016-12-01 8:23 ` Jiewen Yao
2016-12-07 2:59 ` Fan, Jeff
2016-12-01 8:23 ` [PATCH] MdeModulePkg/PiSmmCore: use EfiPagesToSize to prevent build error Jiewen Yao
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Jiewen Yao @ 2016-12-01 8:23 UTC (permalink / raw)
To: edk2-devel; +Cc: Jeff Fan, Michael D Kinney, Laszlo Ersek
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c b/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
index f7e5029..f8edb78 100644
--- a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
+++ b/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
@@ -1126,11 +1126,11 @@ SmmInsertImageRecord (
SetMemoryAttributesTableSectionAlignment (SectionAlignment);
if ((SectionAlignment & (EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT - 1)) != 0) {
- DEBUG ((DEBUG_ERROR, "SMM !!!!!!!! InsertImageRecord - Section Alignment(0x%x) is not %dK !!!!!!!!\n",
+ DEBUG ((DEBUG_WARN, "SMM !!!!!!!! InsertImageRecord - Section Alignment(0x%x) is not %dK !!!!!!!!\n",
SectionAlignment, EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT >> 10));
PdbPointer = PeCoffLoaderGetPdbPointer ((VOID*) (UINTN) ImageAddress);
if (PdbPointer != NULL) {
- DEBUG ((DEBUG_ERROR, "SMM !!!!!!!! Image - %a !!!!!!!!\n", PdbPointer));
+ DEBUG ((DEBUG_WARN, "SMM !!!!!!!! Image - %a !!!!!!!!\n", PdbPointer));
}
goto Finish;
}
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] MdeModulePkg/PiSmmCore: use EfiPagesToSize to prevent build error.
2016-12-01 8:23 [PATCH] MdeModulePkg/PiSmmCore: AllocatePool should use MemoryType Jiewen Yao
2016-12-01 8:23 ` [PATCH] MdeModulePkg/PiSmmCore: MemoryAttributeTable need keep non-PE record Jiewen Yao
2016-12-01 8:23 ` [PATCH] MdeModulePkg/PiSmmCore; Use DEBUG_WARN for non 4k aligned image Jiewen Yao
@ 2016-12-01 8:23 ` Jiewen Yao
2016-12-07 3:02 ` Fan, Jeff
2016-12-01 21:51 ` [PATCH] MdeModulePkg/PiSmmCore: AllocatePool should use MemoryType Laszlo Ersek
2016-12-07 1:59 ` Fan, Jeff
4 siblings, 1 reply; 11+ messages in thread
From: Jiewen Yao @ 2016-12-01 8:23 UTC (permalink / raw)
To: edk2-devel; +Cc: Jeff Fan, Michael D Kinney, Laszlo Ersek
EFI_PAGES_TO_SIZE only handles UINTN, so we use EfiPagesToSize
to handle UINT64.
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c b/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
index a6ab830..f7e5029 100644
--- a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
+++ b/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
@@ -138,7 +138,7 @@ SmmMemoryAttributesTableConsistencyCheck (
if (Address != 0) {
ASSERT (Address == MemoryMap->PhysicalStart);
}
- Address = MemoryMap->PhysicalStart + EFI_PAGES_TO_SIZE(MemoryMap->NumberOfPages);
+ Address = MemoryMap->PhysicalStart + EfiPagesToSize(MemoryMap->NumberOfPages);
MemoryMap = NEXT_MEMORY_DESCRIPTOR(MemoryMap, DescriptorSize);
}
}
@@ -1077,7 +1077,7 @@ SmmInsertImageRecord (
// Step 1: record whole region
//
ImageRecord->ImageBase = DriverEntry->ImageBuffer;
- ImageRecord->ImageSize = EFI_PAGES_TO_SIZE(DriverEntry->NumberOfPage);
+ ImageRecord->ImageSize = EfiPagesToSize(DriverEntry->NumberOfPage);
ImageAddress = (VOID *)(UINTN)DriverEntry->ImageBuffer;
@@ -1281,7 +1281,7 @@ SmmRemoveImageRecord (
DEBUG ((DEBUG_VERBOSE, "SMM RemoveImageRecord - 0x%x\n", DriverEntry));
DEBUG ((DEBUG_VERBOSE, "SMM RemoveImageRecord - 0x%016lx - 0x%016lx\n", DriverEntry->ImageBuffer, DriverEntry->NumberOfPage));
- ImageRecord = FindImageRecord (DriverEntry->ImageBuffer, EFI_PAGES_TO_SIZE(DriverEntry->NumberOfPage));
+ ImageRecord = FindImageRecord (DriverEntry->ImageBuffer, EfiPagesToSize(DriverEntry->NumberOfPage));
if (ImageRecord == NULL) {
DEBUG ((DEBUG_ERROR, "SMM !!!!!!!! ImageRecord not found !!!!!!!!\n"));
return ;
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] MdeModulePkg/PiSmmCore: AllocatePool should use MemoryType.
2016-12-01 8:23 [PATCH] MdeModulePkg/PiSmmCore: AllocatePool should use MemoryType Jiewen Yao
` (2 preceding siblings ...)
2016-12-01 8:23 ` [PATCH] MdeModulePkg/PiSmmCore: use EfiPagesToSize to prevent build error Jiewen Yao
@ 2016-12-01 21:51 ` Laszlo Ersek
2016-12-02 2:03 ` Yao, Jiewen
2016-12-07 1:59 ` Fan, Jeff
4 siblings, 1 reply; 11+ messages in thread
From: Laszlo Ersek @ 2016-12-01 21:51 UTC (permalink / raw)
To: Jiewen Yao, edk2-devel; +Cc: Michael D Kinney, Jeff Fan
On 12/01/16 09:23, Jiewen Yao wrote:
> PiSmmCore supports page level protection based upon the Memory Type
> (EfiRuntimeServicesCode/EfiRuntimeServicesData) and PE image.
>
> However, the Memory Type information is ignored in AllocatePool().
> If a caller calls AllocatePool with EfiRuntimeServicesCode,
> the final memory is still allocated as EfiRuntimeServicesData.
>
> This patch supports AllocatePool with EfiRuntimeServicesCode.
>
> Cc: Jeff Fan <jeff.fan@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
> MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 13 ++-
> MdeModulePkg/Core/PiSmmCore/Pool.c | 66 +++++++++---
> MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c | 114 +++++++++++---------
> 3 files changed, 124 insertions(+), 69 deletions(-)
series
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
(Please make sure to number the patches in the series next time.)
Thanks
Laszlo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MdeModulePkg/PiSmmCore: AllocatePool should use MemoryType.
2016-12-01 21:51 ` [PATCH] MdeModulePkg/PiSmmCore: AllocatePool should use MemoryType Laszlo Ersek
@ 2016-12-02 2:03 ` Yao, Jiewen
2016-12-02 9:44 ` Laszlo Ersek
0 siblings, 1 reply; 11+ messages in thread
From: Yao, Jiewen @ 2016-12-02 2:03 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel@ml01.01.org; +Cc: Kinney, Michael D, Fan, Jeff
Thank you Laszlo.
This time, I did not use number purposely.
My thinking is that each patch resolves a specific problem. Although I fixed all of them at same time, they can be check in independently.
Thank you
Yao Jiewen
From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Friday, December 2, 2016 5:51 AM
To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@ml01.01.org
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>
Subject: Re: [edk2] [PATCH] MdeModulePkg/PiSmmCore: AllocatePool should use MemoryType.
On 12/01/16 09:23, Jiewen Yao wrote:
> PiSmmCore supports page level protection based upon the Memory Type
> (EfiRuntimeServicesCode/EfiRuntimeServicesData) and PE image.
>
> However, the Memory Type information is ignored in AllocatePool().
> If a caller calls AllocatePool with EfiRuntimeServicesCode,
> the final memory is still allocated as EfiRuntimeServicesData.
>
> This patch supports AllocatePool with EfiRuntimeServicesCode.
>
> Cc: Jeff Fan <jeff.fan@intel.com<mailto:jeff.fan@intel.com>>
> Cc: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
> ---
> MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 13 ++-
> MdeModulePkg/Core/PiSmmCore/Pool.c | 66 +++++++++---
> MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c | 114 +++++++++++---------
> 3 files changed, 124 insertions(+), 69 deletions(-)
series
Regression-tested-by: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
(Please make sure to number the patches in the series next time.)
Thanks
Laszlo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MdeModulePkg/PiSmmCore: AllocatePool should use MemoryType.
2016-12-02 2:03 ` Yao, Jiewen
@ 2016-12-02 9:44 ` Laszlo Ersek
0 siblings, 0 replies; 11+ messages in thread
From: Laszlo Ersek @ 2016-12-02 9:44 UTC (permalink / raw)
To: Yao, Jiewen, edk2-devel@ml01.01.org; +Cc: Kinney, Michael D, Fan, Jeff
On 12/02/16 03:03, Yao, Jiewen wrote:
> Thank you Laszlo.
>
>
>
> This time, I did not use number purposely.
>
>
>
> My thinking is that each patch resolves a specific problem. Although I
> fixed all of them at same time, they can be check in independently.
Ah! I looked briefly at the patches and was wondering about the
connection between them. And, I applied them all together and tested
them together.
When I applied them with "git am", I was slightly annoyed that I had to
rename the patches myself, because the file names didn't have numbers
(from the subject lines) and so "git am *" wouldn't keep the original order.
So apparently this was all intentional :) In the future I'd still
recommend to use numbers, also a cover letter, and in the cover letter
just state that the patches are independent. The subject on the cover
letter can be "various fixes" or some such.
Thanks!
Laszlo
>
>
>
> Thank you
>
> Yao Jiewen
>
>
>
> *From:*Laszlo Ersek [mailto:lersek@redhat.com]
> *Sent:* Friday, December 2, 2016 5:51 AM
> *To:* Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@ml01.01.org
> *Cc:* Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff
> <jeff.fan@intel.com>
> *Subject:* Re: [edk2] [PATCH] MdeModulePkg/PiSmmCore: AllocatePool
> should use MemoryType.
>
>
>
> On 12/01/16 09:23, Jiewen Yao wrote:
>> PiSmmCore supports page level protection based upon the Memory Type
>> (EfiRuntimeServicesCode/EfiRuntimeServicesData) and PE image.
>>
>> However, the Memory Type information is ignored in AllocatePool().
>> If a caller calls AllocatePool with EfiRuntimeServicesCode,
>> the final memory is still allocated as EfiRuntimeServicesData.
>>
>> This patch supports AllocatePool with EfiRuntimeServicesCode.
>>
>> Cc: Jeff Fan <jeff.fan@intel.com <mailto:jeff.fan@intel.com>>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>
>> Cc: Laszlo Ersek <lersek@redhat.com <mailto:lersek@redhat.com>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com <mailto:jiewen.yao@intel.com>>
>> ---
>> MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 13 ++-
>> MdeModulePkg/Core/PiSmmCore/Pool.c | 66 +++++++++---
>> MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c | 114 +++++++++++---------
>> 3 files changed, 124 insertions(+), 69 deletions(-)
>
> series
> Regression-tested-by: Laszlo Ersek <lersek@redhat.com
> <mailto:lersek@redhat.com>>
>
> (Please make sure to number the patches in the series next time.)
>
> Thanks
> Laszlo
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MdeModulePkg/PiSmmCore: AllocatePool should use MemoryType.
2016-12-01 8:23 [PATCH] MdeModulePkg/PiSmmCore: AllocatePool should use MemoryType Jiewen Yao
` (3 preceding siblings ...)
2016-12-01 21:51 ` [PATCH] MdeModulePkg/PiSmmCore: AllocatePool should use MemoryType Laszlo Ersek
@ 2016-12-07 1:59 ` Fan, Jeff
4 siblings, 0 replies; 11+ messages in thread
From: Fan, Jeff @ 2016-12-07 1:59 UTC (permalink / raw)
To: Yao, Jiewen, edk2-devel@lists.01.org; +Cc: Kinney, Michael D, Laszlo Ersek
/**
+ Convert a UEFI memory type to SMM pool type.
+
+ @param[in] PoolType Type of pool to allocate.
[Jeff] Typo. *PoolType* should be *MemoryType*.
Reviewed-by: Jeff Fan <jeff.fan@intel.com> with this typo fix.
-----Original Message-----
From: Yao, Jiewen
Sent: Thursday, December 01, 2016 4:23 PM
To: edk2-devel@lists.01.org
Cc: Fan, Jeff; Kinney, Michael D; Laszlo Ersek
Subject: [PATCH] MdeModulePkg/PiSmmCore: AllocatePool should use MemoryType.
PiSmmCore supports page level protection based upon the Memory Type
(EfiRuntimeServicesCode/EfiRuntimeServicesData) and PE image.
However, the Memory Type information is ignored in AllocatePool().
If a caller calls AllocatePool with EfiRuntimeServicesCode, the final memory is still allocated as EfiRuntimeServicesData.
This patch supports AllocatePool with EfiRuntimeServicesCode.
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 13 ++-
MdeModulePkg/Core/PiSmmCore/Pool.c | 66 +++++++++---
MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c | 114 +++++++++++---------
3 files changed, 124 insertions(+), 69 deletions(-)
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
index e2fee54..8df1e50 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
@@ -1109,8 +1109,9 @@ extern LIST_ENTRY mSmmMemoryMap; #define MAX_POOL_INDEX (MAX_POOL_SHIFT - MIN_POOL_SHIFT + 1)
typedef struct {
- UINTN Size;
- BOOLEAN Available;
+ UINTN Size;
+ BOOLEAN Available;
+ EFI_MEMORY_TYPE Type;
} POOL_HEADER;
typedef struct {
@@ -1118,6 +1119,12 @@ typedef struct {
LIST_ENTRY Link;
} FREE_POOL_HEADER;
-extern LIST_ENTRY mSmmPoolLists[MAX_POOL_INDEX];
+typedef enum {
+ SmmPoolTypeCode,
+ SmmPoolTypeData,
+ SmmPoolTypeMax,
+} SMM_POOL_TYPE;
+
+extern LIST_ENTRY mSmmPoolLists[SmmPoolTypeMax][MAX_POOL_INDEX];
#endif
diff --git a/MdeModulePkg/Core/PiSmmCore/Pool.c b/MdeModulePkg/Core/PiSmmCore/Pool.c
index dcfd13e..6fb426c 100644
--- a/MdeModulePkg/Core/PiSmmCore/Pool.c
+++ b/MdeModulePkg/Core/PiSmmCore/Pool.c
@@ -14,7 +14,7 @@
#include "PiSmmCore.h"
-LIST_ENTRY mSmmPoolLists[MAX_POOL_INDEX];
+LIST_ENTRY mSmmPoolLists[SmmPoolTypeMax][MAX_POOL_INDEX];
//
// To cache the SMRAM base since when Loading modules At fixed address feature is enabled, // all module is assigned an offset relative the SMRAM base in build time.
@@ -22,6 +22,30 @@ LIST_ENTRY mSmmPoolLists[MAX_POOL_INDEX];
GLOBAL_REMOVE_IF_UNREFERENCED EFI_PHYSICAL_ADDRESS gLoadModuleAtFixAddressSmramBase = 0;
/**
+ Convert a UEFI memory type to SMM pool type.
+
+ @param[in] PoolType Type of pool to allocate.
+
+ @return SMM pool type
+**/
+SMM_POOL_TYPE
+UefiMemoryTypeToSmmPoolType (
+ IN EFI_MEMORY_TYPE MemoryType
+ )
+{
+ ASSERT ((MemoryType == EfiRuntimeServicesCode) || (MemoryType ==
+EfiRuntimeServicesData));
+ switch (MemoryType) {
+ case EfiRuntimeServicesCode:
+ return SmmPoolTypeCode;
+ case EfiRuntimeServicesData:
+ return SmmPoolTypeData;
+ default:
+ return SmmPoolTypeMax;
+ }
+}
+
+
+/**
Called to initialize the memory service.
@param SmramRangeCount Number of SMRAM Regions
@@ -35,15 +59,18 @@ SmmInitializeMemoryServices (
)
{
UINTN Index;
- UINT64 SmmCodeSize;
- UINTN CurrentSmramRangesIndex;
- UINT64 MaxSize;
+ UINT64 SmmCodeSize;
+ UINTN CurrentSmramRangesIndex;
+ UINT64 MaxSize;
+ UINTN SmmPoolTypeIndex;
//
// Initialize Pool list
//
- for (Index = ARRAY_SIZE (mSmmPoolLists); Index > 0;) {
- InitializeListHead (&mSmmPoolLists[--Index]);
+ for (SmmPoolTypeIndex = 0; SmmPoolTypeIndex < SmmPoolTypeMax; SmmPoolTypeIndex++) {
+ for (Index = 0; Index < ARRAY_SIZE (mSmmPoolLists[SmmPoolTypeIndex]); Index++) {
+ InitializeListHead (&mSmmPoolLists[SmmPoolTypeIndex][Index]);
+ }
}
CurrentSmramRangesIndex = 0;
//
@@ -117,6 +144,7 @@ SmmInitializeMemoryServices (
/**
Internal Function. Allocate a pool by specified PoolIndex.
+ @param PoolType Type of pool to allocate.
@param PoolIndex Index which indicate the Pool size.
@param FreePoolHdr The returned Free pool.
@@ -126,6 +154,7 @@ SmmInitializeMemoryServices ( **/ EFI_STATUS InternalAllocPoolByIndex (
+ IN EFI_MEMORY_TYPE PoolType,
IN UINTN PoolIndex,
OUT FREE_POOL_HEADER **FreePoolHdr
)
@@ -133,25 +162,29 @@ InternalAllocPoolByIndex (
EFI_STATUS Status;
FREE_POOL_HEADER *Hdr;
EFI_PHYSICAL_ADDRESS Address;
+ SMM_POOL_TYPE SmmPoolType;
+
+ SmmPoolType = UefiMemoryTypeToSmmPoolType(PoolType);
ASSERT (PoolIndex <= MAX_POOL_INDEX);
Status = EFI_SUCCESS;
Hdr = NULL;
if (PoolIndex == MAX_POOL_INDEX) {
- Status = SmmInternalAllocatePages (AllocateAnyPages, EfiRuntimeServicesData, EFI_SIZE_TO_PAGES (MAX_POOL_SIZE << 1), &Address);
+ Status = SmmInternalAllocatePages (AllocateAnyPages, PoolType,
+ EFI_SIZE_TO_PAGES (MAX_POOL_SIZE << 1), &Address);
if (EFI_ERROR (Status)) {
return EFI_OUT_OF_RESOURCES;
}
Hdr = (FREE_POOL_HEADER *) (UINTN) Address;
- } else if (!IsListEmpty (&mSmmPoolLists[PoolIndex])) {
- Hdr = BASE_CR (GetFirstNode (&mSmmPoolLists[PoolIndex]), FREE_POOL_HEADER, Link);
+ } else if (!IsListEmpty (&mSmmPoolLists[SmmPoolType][PoolIndex])) {
+ Hdr = BASE_CR (GetFirstNode
+ (&mSmmPoolLists[SmmPoolType][PoolIndex]), FREE_POOL_HEADER, Link);
RemoveEntryList (&Hdr->Link);
} else {
- Status = InternalAllocPoolByIndex (PoolIndex + 1, &Hdr);
+ Status = InternalAllocPoolByIndex (PoolType, PoolIndex + 1, &Hdr);
if (!EFI_ERROR (Status)) {
Hdr->Header.Size >>= 1;
Hdr->Header.Available = TRUE;
- InsertHeadList (&mSmmPoolLists[PoolIndex], &Hdr->Link);
+ Hdr->Header.Type = PoolType;
+ InsertHeadList (&mSmmPoolLists[SmmPoolType][PoolIndex],
+ &Hdr->Link);
Hdr = (FREE_POOL_HEADER*)((UINT8*)Hdr + Hdr->Header.Size);
}
}
@@ -159,6 +192,7 @@ InternalAllocPoolByIndex (
if (!EFI_ERROR (Status)) {
Hdr->Header.Size = MIN_POOL_SIZE << PoolIndex;
Hdr->Header.Available = FALSE;
+ Hdr->Header.Type = PoolType;
}
*FreePoolHdr = Hdr;
@@ -178,16 +212,19 @@ InternalFreePoolByIndex (
IN FREE_POOL_HEADER *FreePoolHdr
)
{
- UINTN PoolIndex;
+ UINTN PoolIndex;
+ SMM_POOL_TYPE SmmPoolType;
ASSERT ((FreePoolHdr->Header.Size & (FreePoolHdr->Header.Size - 1)) == 0);
ASSERT (((UINTN)FreePoolHdr & (FreePoolHdr->Header.Size - 1)) == 0);
ASSERT (FreePoolHdr->Header.Size >= MIN_POOL_SIZE);
+ SmmPoolType = UefiMemoryTypeToSmmPoolType(FreePoolHdr->Header.Type);
+
PoolIndex = (UINTN) (HighBitSet32 ((UINT32)FreePoolHdr->Header.Size) - MIN_POOL_SHIFT);
FreePoolHdr->Header.Available = TRUE;
ASSERT (PoolIndex < MAX_POOL_INDEX);
- InsertHeadList (&mSmmPoolLists[PoolIndex], &FreePoolHdr->Link);
+ InsertHeadList (&mSmmPoolLists[SmmPoolType][PoolIndex],
+ &FreePoolHdr->Link);
return EFI_SUCCESS;
}
@@ -234,6 +271,7 @@ SmmInternalAllocatePool (
PoolHdr = (POOL_HEADER*)(UINTN)Address;
PoolHdr->Size = EFI_PAGES_TO_SIZE (Size);
PoolHdr->Available = FALSE;
+ PoolHdr->Type = PoolType;
*Buffer = PoolHdr + 1;
return Status;
}
@@ -244,7 +282,7 @@ SmmInternalAllocatePool (
PoolIndex++;
}
- Status = InternalAllocPoolByIndex (PoolIndex, &FreePoolHdr);
+ Status = InternalAllocPoolByIndex (PoolType, PoolIndex,
+ &FreePoolHdr);
if (!EFI_ERROR(Status)) {
*Buffer = &FreePoolHdr->Header + 1;
}
diff --git a/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c b/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c
index d983cef..dda9f12 100644
--- a/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c
+++ b/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c
@@ -1596,6 +1596,7 @@ SmramProfileGetDataSize (
FREE_POOL_HEADER *Pool;
UINTN PoolListIndex;
UINTN Index;
+ UINTN SmmPoolTypeIndex;
ContextData = GetSmramProfileContext ();
if (ContextData == NULL) {
@@ -1638,19 +1639,20 @@ SmramProfileGetDataSize (
Node = Node->BackLink) {
Index++;
}
- for (PoolListIndex = 0; PoolListIndex < MAX_POOL_INDEX; PoolListIndex++) {
- FreePoolList = &mSmmPoolLists[PoolListIndex];
- for (Node = FreePoolList->BackLink;
- Node != FreePoolList;
- Node = Node->BackLink) {
- Pool = BASE_CR (Node, FREE_POOL_HEADER, Link);
- if (Pool->Header.Available) {
- Index++;
+ for (SmmPoolTypeIndex = 0; SmmPoolTypeIndex < SmmPoolTypeMax; SmmPoolTypeIndex++) {
+ for (PoolListIndex = 0; PoolListIndex < MAX_POOL_INDEX; PoolListIndex++) {
+ FreePoolList = &mSmmPoolLists[SmmPoolTypeIndex][PoolListIndex];
+ for (Node = FreePoolList->BackLink;
+ Node != FreePoolList;
+ Node = Node->BackLink) {
+ Pool = BASE_CR (Node, FREE_POOL_HEADER, Link);
+ if (Pool->Header.Available) {
+ Index++;
+ }
}
}
}
-
TotalSize += (sizeof (MEMORY_PROFILE_FREE_MEMORY) + Index * sizeof (MEMORY_PROFILE_DESCRIPTOR));
TotalSize += (sizeof (MEMORY_PROFILE_MEMORY_RANGE) + mFullSmramRangeCount * sizeof (MEMORY_PROFILE_DESCRIPTOR));
@@ -1698,6 +1700,7 @@ SmramProfileCopyData (
UINT64 RemainingSize;
UINTN PdbSize;
UINTN ActionStringSize;
+ UINTN SmmPoolTypeIndex;
ContextData = GetSmramProfileContext ();
if (ContextData == NULL) {
@@ -1785,14 +1788,16 @@ SmramProfileCopyData (
Node = Node->BackLink) {
Index++;
}
- for (PoolListIndex = 0; PoolListIndex < MAX_POOL_INDEX; PoolListIndex++) {
- FreePoolList = &mSmmPoolLists[MAX_POOL_INDEX - PoolListIndex - 1];
- for (Node = FreePoolList->BackLink;
- Node != FreePoolList;
- Node = Node->BackLink) {
- Pool = BASE_CR (Node, FREE_POOL_HEADER, Link);
- if (Pool->Header.Available) {
- Index++;
+ for (SmmPoolTypeIndex = 0; SmmPoolTypeIndex < SmmPoolTypeMax; SmmPoolTypeIndex++) {
+ for (PoolListIndex = 0; PoolListIndex < MAX_POOL_INDEX; PoolListIndex++) {
+ FreePoolList = &mSmmPoolLists[SmmPoolTypeIndex][MAX_POOL_INDEX - PoolListIndex - 1];
+ for (Node = FreePoolList->BackLink;
+ Node != FreePoolList;
+ Node = Node->BackLink) {
+ Pool = BASE_CR (Node, FREE_POOL_HEADER, Link);
+ if (Pool->Header.Available) {
+ Index++;
+ }
}
}
}
@@ -1827,29 +1832,31 @@ SmramProfileCopyData (
}
Offset += sizeof (MEMORY_PROFILE_DESCRIPTOR);
}
- for (PoolListIndex = 0; PoolListIndex < MAX_POOL_INDEX; PoolListIndex++) {
- FreePoolList = &mSmmPoolLists[MAX_POOL_INDEX - PoolListIndex - 1];
- for (Node = FreePoolList->BackLink;
- Node != FreePoolList;
- Node = Node->BackLink) {
- Pool = BASE_CR (Node, FREE_POOL_HEADER, Link);
- if (Pool->Header.Available) {
- if (*ProfileOffset < (Offset + sizeof (MEMORY_PROFILE_DESCRIPTOR))) {
- if (RemainingSize >= sizeof (MEMORY_PROFILE_DESCRIPTOR)) {
- MemoryProfileDescriptor = ProfileBuffer;
- MemoryProfileDescriptor->Header.Signature = MEMORY_PROFILE_DESCRIPTOR_SIGNATURE;
- MemoryProfileDescriptor->Header.Length = sizeof (MEMORY_PROFILE_DESCRIPTOR);
- MemoryProfileDescriptor->Header.Revision = MEMORY_PROFILE_DESCRIPTOR_REVISION;
- MemoryProfileDescriptor->Address = (PHYSICAL_ADDRESS) (UINTN) Pool;
- MemoryProfileDescriptor->Size = Pool->Header.Size;
-
- RemainingSize -= sizeof (MEMORY_PROFILE_DESCRIPTOR);
- ProfileBuffer = (UINT8 *) ProfileBuffer + sizeof (MEMORY_PROFILE_DESCRIPTOR);
- } else {
- goto Done;
+ for (SmmPoolTypeIndex = 0; SmmPoolTypeIndex < SmmPoolTypeMax; SmmPoolTypeIndex++) {
+ for (PoolListIndex = 0; PoolListIndex < MAX_POOL_INDEX; PoolListIndex++) {
+ FreePoolList = &mSmmPoolLists[SmmPoolTypeIndex][MAX_POOL_INDEX - PoolListIndex - 1];
+ for (Node = FreePoolList->BackLink;
+ Node != FreePoolList;
+ Node = Node->BackLink) {
+ Pool = BASE_CR (Node, FREE_POOL_HEADER, Link);
+ if (Pool->Header.Available) {
+ if (*ProfileOffset < (Offset + sizeof (MEMORY_PROFILE_DESCRIPTOR))) {
+ if (RemainingSize >= sizeof (MEMORY_PROFILE_DESCRIPTOR)) {
+ MemoryProfileDescriptor = ProfileBuffer;
+ MemoryProfileDescriptor->Header.Signature = MEMORY_PROFILE_DESCRIPTOR_SIGNATURE;
+ MemoryProfileDescriptor->Header.Length = sizeof (MEMORY_PROFILE_DESCRIPTOR);
+ MemoryProfileDescriptor->Header.Revision = MEMORY_PROFILE_DESCRIPTOR_REVISION;
+ MemoryProfileDescriptor->Address = (PHYSICAL_ADDRESS) (UINTN) Pool;
+ MemoryProfileDescriptor->Size = Pool->Header.Size;
+
+ RemainingSize -= sizeof (MEMORY_PROFILE_DESCRIPTOR);
+ ProfileBuffer = (UINT8 *) ProfileBuffer + sizeof (MEMORY_PROFILE_DESCRIPTOR);
+ } else {
+ goto Done;
+ }
}
+ Offset += sizeof (MEMORY_PROFILE_DESCRIPTOR);
}
- Offset += sizeof (MEMORY_PROFILE_DESCRIPTOR);
}
}
}
@@ -2577,6 +2584,7 @@ DumpFreePoolList (
UINTN PoolListIndex;
MEMORY_PROFILE_CONTEXT_DATA *ContextData;
BOOLEAN SmramProfileGettingStatus;
+ UINTN SmmPoolTypeIndex;
ContextData = GetSmramProfileContext ();
if (ContextData == NULL) {
@@ -2586,23 +2594,25 @@ DumpFreePoolList (
SmramProfileGettingStatus = mSmramProfileGettingStatus;
mSmramProfileGettingStatus = TRUE;
- DEBUG ((EFI_D_INFO, "======= SmramProfile begin =======\n"));
-
- for (PoolListIndex = 0; PoolListIndex < MAX_POOL_INDEX; PoolListIndex++) {
- DEBUG ((EFI_D_INFO, "FreePoolList (%d):\n", PoolListIndex));
- FreePoolList = &mSmmPoolLists[PoolListIndex];
- for (Node = FreePoolList->BackLink, Index = 0;
- Node != FreePoolList;
- Node = Node->BackLink, Index++) {
- Pool = BASE_CR (Node, FREE_POOL_HEADER, Link);
- DEBUG ((EFI_D_INFO, " Index - 0x%x\n", Index));
- DEBUG ((EFI_D_INFO, " PhysicalStart - 0x%016lx\n", (PHYSICAL_ADDRESS) (UINTN) Pool));
- DEBUG ((EFI_D_INFO, " Size - 0x%08x\n", Pool->Header.Size));
- DEBUG ((EFI_D_INFO, " Available - 0x%02x\n", Pool->Header.Available));
+ DEBUG ((DEBUG_INFO, "======= SmramProfile begin =======\n"));
+
+ for (SmmPoolTypeIndex = 0; SmmPoolTypeIndex < SmmPoolTypeMax; SmmPoolTypeIndex++) {
+ for (PoolListIndex = 0; PoolListIndex < MAX_POOL_INDEX; PoolListIndex++) {
+ DEBUG ((DEBUG_INFO, "FreePoolList(%d)(%d):\n", SmmPoolTypeIndex, PoolListIndex));
+ FreePoolList = &mSmmPoolLists[SmmPoolTypeIndex][PoolListIndex];
+ for (Node = FreePoolList->BackLink, Index = 0;
+ Node != FreePoolList;
+ Node = Node->BackLink, Index++) {
+ Pool = BASE_CR (Node, FREE_POOL_HEADER, Link);
+ DEBUG ((DEBUG_INFO, " Index - 0x%x\n", Index));
+ DEBUG ((DEBUG_INFO, " PhysicalStart - 0x%016lx\n", (PHYSICAL_ADDRESS) (UINTN) Pool));
+ DEBUG ((DEBUG_INFO, " Size - 0x%08x\n", Pool->Header.Size));
+ DEBUG ((DEBUG_INFO, " Available - 0x%02x\n", Pool->Header.Available));
+ }
}
}
- DEBUG ((EFI_D_INFO, "======= SmramProfile end =======\n"));
+ DEBUG ((DEBUG_INFO, "======= SmramProfile end =======\n"));
mSmramProfileGettingStatus = SmramProfileGettingStatus; }
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] MdeModulePkg/PiSmmCore; Use DEBUG_WARN for non 4k aligned image.
2016-12-01 8:23 ` [PATCH] MdeModulePkg/PiSmmCore; Use DEBUG_WARN for non 4k aligned image Jiewen Yao
@ 2016-12-07 2:59 ` Fan, Jeff
0 siblings, 0 replies; 11+ messages in thread
From: Fan, Jeff @ 2016-12-07 2:59 UTC (permalink / raw)
To: Yao, Jiewen, edk2-devel@lists.01.org; +Cc: Kinney, Michael D, Laszlo Ersek
Reviewed-by: Jeff Fan <jeff.fan@intel.com>
-----Original Message-----
From: Yao, Jiewen
Sent: Thursday, December 01, 2016 4:23 PM
To: edk2-devel@lists.01.org
Cc: Fan, Jeff; Kinney, Michael D; Laszlo Ersek
Subject: [PATCH] MdeModulePkg/PiSmmCore; Use DEBUG_WARN for non 4k aligned image.
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c b/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
index f7e5029..f8edb78 100644
--- a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
+++ b/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
@@ -1126,11 +1126,11 @@ SmmInsertImageRecord (
SetMemoryAttributesTableSectionAlignment (SectionAlignment);
if ((SectionAlignment & (EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT - 1)) != 0) {
- DEBUG ((DEBUG_ERROR, "SMM !!!!!!!! InsertImageRecord - Section Alignment(0x%x) is not %dK !!!!!!!!\n",
+ DEBUG ((DEBUG_WARN, "SMM !!!!!!!! InsertImageRecord - Section Alignment(0x%x) is not %dK !!!!!!!!\n",
SectionAlignment, EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT >> 10));
PdbPointer = PeCoffLoaderGetPdbPointer ((VOID*) (UINTN) ImageAddress);
if (PdbPointer != NULL) {
- DEBUG ((DEBUG_ERROR, "SMM !!!!!!!! Image - %a !!!!!!!!\n", PdbPointer));
+ DEBUG ((DEBUG_WARN, "SMM !!!!!!!! Image - %a !!!!!!!!\n", PdbPointer));
}
goto Finish;
}
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] MdeModulePkg/PiSmmCore: use EfiPagesToSize to prevent build error.
2016-12-01 8:23 ` [PATCH] MdeModulePkg/PiSmmCore: use EfiPagesToSize to prevent build error Jiewen Yao
@ 2016-12-07 3:02 ` Fan, Jeff
0 siblings, 0 replies; 11+ messages in thread
From: Fan, Jeff @ 2016-12-07 3:02 UTC (permalink / raw)
To: Yao, Jiewen, edk2-devel@lists.01.org; +Cc: Kinney, Michael D, Laszlo Ersek
Reviewed-by: Jeff Fan <jeff.fan@intel.com>
-----Original Message-----
From: Yao, Jiewen
Sent: Thursday, December 01, 2016 4:23 PM
To: edk2-devel@lists.01.org
Cc: Fan, Jeff; Kinney, Michael D; Laszlo Ersek
Subject: [PATCH] MdeModulePkg/PiSmmCore: use EfiPagesToSize to prevent build error.
EFI_PAGES_TO_SIZE only handles UINTN, so we use EfiPagesToSize to handle UINT64.
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c b/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
index a6ab830..f7e5029 100644
--- a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
+++ b/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
@@ -138,7 +138,7 @@ SmmMemoryAttributesTableConsistencyCheck (
if (Address != 0) {
ASSERT (Address == MemoryMap->PhysicalStart);
}
- Address = MemoryMap->PhysicalStart + EFI_PAGES_TO_SIZE(MemoryMap->NumberOfPages);
+ Address = MemoryMap->PhysicalStart +
+ EfiPagesToSize(MemoryMap->NumberOfPages);
MemoryMap = NEXT_MEMORY_DESCRIPTOR(MemoryMap, DescriptorSize);
}
}
@@ -1077,7 +1077,7 @@ SmmInsertImageRecord (
// Step 1: record whole region
//
ImageRecord->ImageBase = DriverEntry->ImageBuffer;
- ImageRecord->ImageSize = EFI_PAGES_TO_SIZE(DriverEntry->NumberOfPage);
+ ImageRecord->ImageSize = EfiPagesToSize(DriverEntry->NumberOfPage);
ImageAddress = (VOID *)(UINTN)DriverEntry->ImageBuffer;
@@ -1281,7 +1281,7 @@ SmmRemoveImageRecord (
DEBUG ((DEBUG_VERBOSE, "SMM RemoveImageRecord - 0x%x\n", DriverEntry));
DEBUG ((DEBUG_VERBOSE, "SMM RemoveImageRecord - 0x%016lx - 0x%016lx\n", DriverEntry->ImageBuffer, DriverEntry->NumberOfPage));
- ImageRecord = FindImageRecord (DriverEntry->ImageBuffer, EFI_PAGES_TO_SIZE(DriverEntry->NumberOfPage));
+ ImageRecord = FindImageRecord (DriverEntry->ImageBuffer,
+ EfiPagesToSize(DriverEntry->NumberOfPage));
if (ImageRecord == NULL) {
DEBUG ((DEBUG_ERROR, "SMM !!!!!!!! ImageRecord not found !!!!!!!!\n"));
return ;
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] MdeModulePkg/PiSmmCore: MemoryAttributeTable need keep non-PE record.
2016-12-01 8:23 ` [PATCH] MdeModulePkg/PiSmmCore: MemoryAttributeTable need keep non-PE record Jiewen Yao
@ 2016-12-07 5:27 ` Fan, Jeff
0 siblings, 0 replies; 11+ messages in thread
From: Fan, Jeff @ 2016-12-07 5:27 UTC (permalink / raw)
To: Yao, Jiewen, edk2-devel@lists.01.org; +Cc: Kinney, Michael D, Laszlo Ersek
Reviewed-by: Jeff Fan <jeff.fan@intel.com>
-----Original Message-----
From: Yao, Jiewen
Sent: Thursday, December 01, 2016 4:23 PM
To: edk2-devel@lists.01.org
Cc: Fan, Jeff; Kinney, Michael D; Laszlo Ersek
Subject: [PATCH] MdeModulePkg/PiSmmCore: MemoryAttributeTable need keep non-PE record.
Current memory attribute table implementation will only mark PE code to be EfiRuntimeServicesCode, and mark rest to be EfiRuntimeServicesData.
However, there might be a case that a SMM code wants to allocate EfiRuntimeServicesCode explicitly to let page table protect this region to be read only. It is unsupported.
This patch enhances the current solution so that MemoryAttributeTable does not touch non PE image record.
Only the PE image region is forced to be EfiRuntimeServicesCode for code and EfiRuntimeServicesData for data.
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c | 91 +++++++++++---------
1 file changed, 51 insertions(+), 40 deletions(-)
diff --git a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c b/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
index 3a5a2c8..a6ab830 100644
--- a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
+++ b/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
@@ -268,15 +268,19 @@ EnforceMemoryMapAttribute (
MemoryMapEntry = MemoryMap;
MemoryMapEnd = (EFI_MEMORY_DESCRIPTOR *) ((UINT8 *) MemoryMap + MemoryMapSize);
while ((UINTN)MemoryMapEntry < (UINTN)MemoryMapEnd) {
- switch (MemoryMapEntry->Type) {
- case EfiRuntimeServicesCode:
- MemoryMapEntry->Attribute |= EFI_MEMORY_RO;
- break;
- case EfiRuntimeServicesData:
- MemoryMapEntry->Attribute |= EFI_MEMORY_XP;
- break;
+ if (MemoryMapEntry->Attribute != 0) {
+ // It is PE image, the attribute is already set.
+ } else {
+ switch (MemoryMapEntry->Type) {
+ case EfiRuntimeServicesCode:
+ MemoryMapEntry->Attribute = EFI_MEMORY_RO;
+ break;
+ case EfiRuntimeServicesData:
+ default:
+ MemoryMapEntry->Attribute |= EFI_MEMORY_XP;
+ break;
+ }
}
-
MemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorSize);
}
@@ -358,6 +362,21 @@ SetNewRecord (
PhysicalEnd = TempRecord.PhysicalStart + EfiPagesToSize(TempRecord.NumberOfPages);
NewRecordCount = 0;
+ //
+ // Always create a new entry for non-PE image record // if
+ (ImageRecord->ImageBase > TempRecord.PhysicalStart) {
+ NewRecord->Type = TempRecord.Type;
+ NewRecord->PhysicalStart = TempRecord.PhysicalStart;
+ NewRecord->VirtualStart = 0;
+ NewRecord->NumberOfPages = EfiSizeToPages(ImageRecord->ImageBase - TempRecord.PhysicalStart);
+ NewRecord->Attribute = TempRecord.Attribute;
+ NewRecord = NEXT_MEMORY_DESCRIPTOR (NewRecord, DescriptorSize);
+ NewRecordCount ++;
+ TempRecord.PhysicalStart = ImageRecord->ImageBase;
+ TempRecord.NumberOfPages = EfiSizeToPages(PhysicalEnd -
+ TempRecord.PhysicalStart); }
+
ImageRecordCodeSectionList = &ImageRecord->CodeSegmentList;
ImageRecordCodeSectionLink = ImageRecordCodeSectionList->ForwardLink;
@@ -452,14 +471,10 @@ GetMaxSplitRecordCount (
if (ImageRecord == NULL) {
break;
}
- SplitRecordCount += (2 * ImageRecord->CodeSegmentCount + 1);
+ SplitRecordCount += (2 * ImageRecord->CodeSegmentCount + 2);
PhysicalStart = ImageRecord->ImageBase + ImageRecord->ImageSize;
} while ((ImageRecord != NULL) && (PhysicalStart < PhysicalEnd));
- if (SplitRecordCount != 0) {
- SplitRecordCount--;
- }
-
return SplitRecordCount;
}
@@ -516,28 +531,16 @@ SplitRecord (
//
// No more image covered by this range, stop
//
- if ((PhysicalEnd > PhysicalStart) && (ImageRecord != NULL)) {
+ if (PhysicalEnd > PhysicalStart) {
//
- // If this is still address in this record, need record.
+ // Always create a new entry for non-PE image record
//
- NewRecord = PREVIOUS_MEMORY_DESCRIPTOR (NewRecord, DescriptorSize);
- if (NewRecord->Type == EfiRuntimeServicesData) {
- //
- // Last record is DATA, just merge it.
- //
- NewRecord->NumberOfPages = EfiSizeToPages(PhysicalEnd - NewRecord->PhysicalStart);
- } else {
- //
- // Last record is CODE, create a new DATA entry.
- //
- NewRecord = NEXT_MEMORY_DESCRIPTOR (NewRecord, DescriptorSize);
- NewRecord->Type = EfiRuntimeServicesData;
- NewRecord->PhysicalStart = TempRecord.PhysicalStart;
- NewRecord->VirtualStart = 0;
- NewRecord->NumberOfPages = TempRecord.NumberOfPages;
- NewRecord->Attribute = TempRecord.Attribute | EFI_MEMORY_XP;
- TotalNewRecordCount ++;
- }
+ NewRecord->Type = TempRecord.Type;
+ NewRecord->PhysicalStart = TempRecord.PhysicalStart;
+ NewRecord->VirtualStart = 0;
+ NewRecord->NumberOfPages = TempRecord.NumberOfPages;
+ NewRecord->Attribute = TempRecord.Attribute;
+ TotalNewRecordCount ++;
}
break;
}
@@ -580,6 +583,8 @@ SplitRecord (
==>
+---------------+
| Record X |
+ +---------------+
+ | Record RtCode |
+---------------+ ----
| Record RtData | |
+---------------+ |
@@ -587,12 +592,16 @@ SplitRecord (
+---------------+ |
| Record RtData | |
+---------------+ ----
+ | Record RtCode |
+ +---------------+ ----
| Record RtData | |
+---------------+ |
| Record RtCode | |-> PE/COFF2
+---------------+ |
| Record RtData | |
+---------------+ ----
+ | Record RtCode |
+ +---------------+
| Record Y |
+---------------+
@@ -622,7 +631,7 @@ SplitTable (
UINTN TotalSplitRecordCount;
UINTN AdditionalRecordCount;
- AdditionalRecordCount = (2 * mImagePropertiesPrivateData.CodeSegmentCountMax + 1) * mImagePropertiesPrivateData.ImageRecordCount;
+ AdditionalRecordCount = (2 *
+ mImagePropertiesPrivateData.CodeSegmentCountMax + 2) *
+ mImagePropertiesPrivateData.ImageRecordCount;
TotalSplitRecordCount = 0;
//
@@ -648,11 +657,13 @@ SplitTable (
//
// Adjust IndexNew according to real split.
//
- CopyMem (
- ((UINT8 *)MemoryMap + (IndexNew + MaxSplitRecordCount - RealSplitRecordCount) * DescriptorSize),
- ((UINT8 *)MemoryMap + IndexNew * DescriptorSize),
- RealSplitRecordCount * DescriptorSize
- );
+ if (MaxSplitRecordCount != RealSplitRecordCount) {
+ CopyMem (
+ ((UINT8 *)MemoryMap + (IndexNew + MaxSplitRecordCount - RealSplitRecordCount) * DescriptorSize),
+ ((UINT8 *)MemoryMap + IndexNew * DescriptorSize),
+ (RealSplitRecordCount + 1) * DescriptorSize
+ );
+ }
IndexNew = IndexNew + MaxSplitRecordCount - RealSplitRecordCount;
TotalSplitRecordCount += RealSplitRecordCount;
IndexNew --;
@@ -744,7 +755,7 @@ SmmCoreGetMemoryMapMemoryAttributesTable (
return EFI_INVALID_PARAMETER;
}
- AdditionalRecordCount = (2 * mImagePropertiesPrivateData.CodeSegmentCountMax + 1) * mImagePropertiesPrivateData.ImageRecordCount;
+ AdditionalRecordCount = (2 *
+ mImagePropertiesPrivateData.CodeSegmentCountMax + 2) *
+ mImagePropertiesPrivateData.ImageRecordCount;
OldMemoryMapSize = *MemoryMapSize;
Status = SmmCoreGetMemoryMap (MemoryMapSize, MemoryMap, MapKey, DescriptorSize, DescriptorVersion);
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-12-07 5:28 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-01 8:23 [PATCH] MdeModulePkg/PiSmmCore: AllocatePool should use MemoryType Jiewen Yao
2016-12-01 8:23 ` [PATCH] MdeModulePkg/PiSmmCore: MemoryAttributeTable need keep non-PE record Jiewen Yao
2016-12-07 5:27 ` Fan, Jeff
2016-12-01 8:23 ` [PATCH] MdeModulePkg/PiSmmCore; Use DEBUG_WARN for non 4k aligned image Jiewen Yao
2016-12-07 2:59 ` Fan, Jeff
2016-12-01 8:23 ` [PATCH] MdeModulePkg/PiSmmCore: use EfiPagesToSize to prevent build error Jiewen Yao
2016-12-07 3:02 ` Fan, Jeff
2016-12-01 21:51 ` [PATCH] MdeModulePkg/PiSmmCore: AllocatePool should use MemoryType Laszlo Ersek
2016-12-02 2:03 ` Yao, Jiewen
2016-12-02 9:44 ` Laszlo Ersek
2016-12-07 1:59 ` Fan, Jeff
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox