Hi Yanbo,

Can you confirm that the following resolves the issue you're seeing?

[PATCH v1] MdeModulePkg: Fixup MAT Attributes After Splitting EFI Memory Map (groups.io)

-Taylor

On 4/15/2024 6:17 PM, Taylor Beebe wrote:

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-0x769E5000 is 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.

_._,_._,_