On 4/15/2024 3:57 AM, Bi, Dandan wrote:
Hi Taylor,
With this patch, MAT contains some entries with Attribute - 0x8000000000000000,
doesn't have EFI_MEMORY_RO or EFI_MEMORY_XP.
After revert this patch, don't see such entries in MAT.
a. MAT with this patch:
Entry (0x609E4268)
Type - 0x5
PhysicalStart - 0x00000000769CF000
VirtualStart - 0x0000000000000000
NumberOfPages - 0x0000000000000016
Attribute - 0x8000000000000000
Entry (0x609E4298)
Type - 0x5
PhysicalStart - 0x00000000769E5000
VirtualStart - 0x0000000000000000
NumberOfPages - 0x0000000000000001
Attribute - 0x8000000000004000
Entry (0x609E42C8)
Type - 0x5
PhysicalStart - 0x00000000769E6000
VirtualStart - 0x0000000000000000
NumberOfPages - 0x0000000000000002
Attribute - 0x8000000000020000
b. MAT without this patch:
Entry (0x609E4268)
Type - 0x5
PhysicalStart - 0x00000000769CF000
VirtualStart - 0x0000000000000000
NumberOfPages - 0x0000000000000017
Attribute - 0x8000000000004000
Entry (0x609E4298)
Type - 0x5
PhysicalStart - 0x00000000769E6000
VirtualStart - 0x0000000000000000
NumberOfPages - 0x0000000000000002
Attribute - 0x8000000000020000
1. For example, when OldRecord in old memory map with:
Type - 0x00000005
Attribute - 0x800000000000000F
PhysicalStart - 0x769CF000
PhysicalStart is smaller than ImageBase 0x769E5000, with this patch, it
will create a new memory descriptor entry for range 0x769CF000~0x769E5000 and
without EFI_MEMORY_RO or EFI_MEMORY_XP Attribute.
Then it will only contain EFI_MEMORY_RUNTIME Attribute in MAT as doing
MemoryAttributesEntry->Attribute &=
(EFI_MEMORY_RO|EFI_MEMORY_XP|EFI_MEMORY_RUNTIME); when install MAT.
It seems not aligned with UEFI Spec " The only valid bits for Attribute field
currently are EFI_MEMORY_RO ,EFI_MEMORY_XP , plus EFI_MEMORY_RUNTIME "?
Could you please help double check? Thanks.
Agreed that this is currently the behaviour and that the range should
have a restrictive access attribute. More below.
2. In function SetNewRecord, it semes already cover the DATA entry before the
CODE and the DATA entry after the CODE.
And old SplitRecord function without this patch, also has the entry to
cover the reaming range of this record if no more image covered by this range.
Why do we still need this patch? Could you please help explain? Thanks.
GetMemoryMap() will merge adjacent descriptors which have the same type
and attributes. This means that a single EfiRuntimeServicesCode
descriptor within the memory map returned by CoreGetMemoryMap() could
describe memory with the following layout (NOTE: this layout is odd but
needs to be handled to fulfill the SplitTable() contract):
-------------------------
Some EfiRuntimeServicesCode memory
-------------------------
Runtime Image DATA Section
-------------------------
Runtime Image CODE Section
-------------------------
Runtime Image DATA Section
-------------------------
Some EfiRuntimeServicesCode memory
-------------------------
In this possible layout, the pre-patch logic would assume that the
regions before and after the image were part of the image's data
sections and would mark them as EFI_MEMORY_XP. The post-patch logic does
not mark them with any access attributes which is fine but the DXE MAT
logic needs to walk the memory map returned by SplitTable() to add
access attributes to runtime regions which don't have any.
In your example, because PhysicalStart < ImageBase and
0769CF000-0x769E5000 is EfiRuntimeServicesCode, the access attribute
should technically be EFI_MEMORY_RO. It's likely that
0769CF000-0x769E5000is just the unused memory bucket and so it might be
best to mark it both EFI_MEMORY_RO and EFI_MEMORY_XP.
*An open question to the community:* Are there cases where runtime
executable code is in a buffer separate from a loaded runtime image? Can
the EfiRuntimeServicesCode memory regions not part of an image be
specified in the MAT as both EFI_MEMORY_RO and EFI_MEMORY_XP, or even
dropped entirely?
Thanks :)
-Taylor
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117813): https://edk2.groups.io/g/devel/message/117813
Mute This Topic: https://groups.io/mt/105477564/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
