On 20 February 2016 at 04:59, Jiewen Yao <[email protected]> wrote: > 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" <[email protected]> > Cc: "Ard Biesheuvel" <[email protected]> > Cc: "Gao, Liming" <[email protected]> > ---
This looks correct now on my system Tested-by: Ard Biesheuvel <[email protected]> Thanks! > MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 108 > +++++++++------------------ > 1 file changed, 36 insertions(+), 72 deletions(-) > > diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c > b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c > index 6e5ad03..66c5eb6 100644 > --- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c > +++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c > @@ -81,13 +81,10 @@ 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 +196,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 +232,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 +390,11 @@ SetNewRecord ( > // > // DATA > // > - NewRecord->Type = EfiRuntimeServicesData; > + if (mIsConstructingMemoryAttributesTable && !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 +407,11 @@ SetNewRecord ( > // > // CODE > // > - NewRecord->Type = EfiRuntimeServicesCode; > + if (mIsConstructingMemoryAttributesTable && !mPropertiesTableEnable) { > + NewRecord->Type = TempRecord.Type; > + } else { > + NewRecord->Type = EfiRuntimeServicesCode; > + } > NewRecord->PhysicalStart = ImageRecordCodeSection->CodeSegmentBase; > NewRecord->VirtualStart = 0; > NewRecord->NumberOfPages = > EfiSizeToPages(ImageRecordCodeSection->CodeSegmentSize); > @@ -467,7 +435,11 @@ SetNewRecord ( > // Final DATA > // > if (TempRecord.PhysicalStart < ImageEnd) { > - NewRecord->Type = EfiRuntimeServicesData; > + if (mIsConstructingMemoryAttributesTable && !mPropertiesTableEnable) { > + NewRecord->Type = TempRecord.Type; > + } else { > + NewRecord->Type = EfiRuntimeServicesData; > + } > NewRecord->PhysicalStart = TempRecord.PhysicalStart; > NewRecord->VirtualStart = 0; > NewRecord->NumberOfPages = EfiSizeToPages (ImageEnd - > TempRecord.PhysicalStart); > @@ -549,6 +521,7 @@ SplitRecord ( > UINT64 PhysicalEnd; > UINTN NewRecordCount; > UINTN TotalNewRecordCount; > + BOOLEAN IsLastRecordData; > > if (MaxSplitRecordCount == 0) { > CopyMem (NewRecord, OldRecord, DescriptorSize); > @@ -576,7 +549,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 (mIsConstructingMemoryAttributesTable && !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 +569,11 @@ SplitRecord ( > // Last record is CODE, create a new DATA entry. > // > NewRecord = NEXT_MEMORY_DESCRIPTOR (NewRecord, DescriptorSize); > - NewRecord->Type = EfiRuntimeServicesData; > + if (mIsConstructingMemoryAttributesTable && > !mPropertiesTableEnable) { > + NewRecord->Type = TempRecord.Type; > + } else { > + NewRecord->Type = EfiRuntimeServicesData; > + } > NewRecord->PhysicalStart = TempRecord.PhysicalStart; > NewRecord->VirtualStart = 0; > NewRecord->NumberOfPages = TempRecord.NumberOfPages; > @@ -814,38 +801,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 +1374,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 [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
