On 19 February 2016 at 09:12, Jiewen Yao <jiewen....@intel.com> 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" <jiewen....@intel.com>
> Cc: "Ard Biesheuvel" <ard.biesheu...@linaro.org>
> Cc: "Gao, Liming" <liming....@intel.com>
Thanks for the quick fix
Unfortunately, the table appears corrupted now:
UEFI memmap:
0x00013c0e0000-0x00013c15ffff [Runtime Code |RUN| | | | | ]
0x00013c160000-0x00013c17ffff [Runtime Data |RUN| | | | | ]
0x00013c1a0000-0x00013c1dffff [Runtime Code |RUN| | | | | ]
0x00013c1e0000-0x00013c27ffff [Runtime Data |RUN| | | | | ]
0x00013c280000-0x00013c2cffff [Runtime Code |RUN| | | | | ]
0x00013c2d0000-0x00013c31ffff [Runtime Data |RUN| | | | | ]
0x00013c320000-0x00013c36ffff [Runtime Code |RUN| | | | | ]
0x00013f650000-0x00013f6dffff [Runtime Code |RUN| | | | | ]
0x00013f6f0000-0x00013f80ffff [Runtime Data |RUN| | | | | ]
Memory Attributes table:
0x00013c0e0000-0x00013c0effff [Runtime Code |RUN| |XP| | | ]
0x00013c0f0000-0x00013c0fffff [Runtime Code |RUN| | | | |RO]
0x00013c100000-0x00013c12ffff [Runtime Code |RUN| |XP| | | ]
0x00013c130000-0x00013c13ffff [Runtime Code |RUN| | | | |RO]
0x00013c140000-0x00013c14ffff [Runtime Code |RUN| |XP| | | ]
0x00013c140000-0x00013c14ffff [Runtime Code |RUN| |XP| | | ]
0x00013c1a0000-0x00013c1affff [Runtime Code |RUN| |XP| | | ]
0x00013c1b0000-0x00013c1bffff [Runtime Code |RUN| | | | |RO]
0x00013c1c0000-0x00013c1cffff [Runtime Code |RUN| |XP| | | ]
0x00013c1c0000-0x00013c1cffff [Runtime Code |RUN| |XP| | | ]
0x00013c280000-0x00013c28ffff [Runtime Code |RUN| |XP| | | ]
0x00013c290000-0x00013c29ffff [Runtime Code |RUN| | | | |RO]
0x00013c2a0000-0x00013c2bffff [Runtime Code |RUN| |XP| | | ]
0x00013c2a0000-0x00013c2bffff [Runtime Code |RUN| |XP| | | ]
0x00013c320000-0x00013c32ffff [Runtime Code |RUN| |XP| | | ]
0x00013c330000-0x00013c33ffff [Runtime Code |RUN| | | | |RO]
0x00013c340000-0x00013c35ffff [Runtime Code |RUN| |XP| | | ]
0x00013c340000-0x00013c35ffff [Runtime Code |RUN| |XP| | | ]
0x00013f650000-0x00013f65ffff [Runtime Code |RUN| |XP| | | ]
0x00013f660000-0x00013f66ffff [Runtime Code |RUN| | | | |RO]
0x00013f670000-0x00013f69ffff [Runtime Code |RUN| |XP| | | ]
0x00013f6a0000-0x00013f6affff [Runtime Code |RUN| | | | |RO]
0x00013f6b0000-0x00013f6cffff [Runtime Code |RUN| |XP| | | ]
0x00013f6b0000-0x00013f6cffff [Runtime Code |RUN| |XP| | | ]
0x00013f6f0000-0x00013f80ffff [Runtime Data |RUN| |XP| | | ]
There are duplicate entries in the memattr table, and some others are missing
> ---
> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 89
> ++++++----------------------
> 1 file changed, 19 insertions(+), 70 deletions(-)
>
> diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> index 6e5ad03..b052148 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);
> @@ -814,38 +786,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 +1359,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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
