edk2 icon indicating copy to clipboard operation
edk2 copied to clipboard

ArmPkg: ArmMmuLib: Pre-Allocate Page Table Memory

Open os-d opened this issue 4 months ago • 7 comments

Description

Allocating memory when memory protection is active can cause the below infinite loop:

  1. gCpu->SetMemoryAttributes(EFI_MEMORY_RO)
  2. ArmSetMemoryAttributes ()
  3. SetMemoryRegionAttribute()
  4. UpdateRegionMapping()
  5. UpdateRegionMappingRecursive()
  6. AllocatePages() -> Need memory for a translation table entry
  7. CoreAllocatePages()
  8. ApplyMemoryProtectionPolicy() -> Policy says new page should be XN
  9. gCpu->SetMemoryAttributes()
  10. Back to 3

To fix this previously, CpuDxe would update conventional memory to be XN prior to installing the CpuArch protocol. However, when we transition to setting EFI_MEMORY_RP on free memory, this will no longer work. This a prerequisite to bring in the EFI_MEMORY_RP on free memory feature that has been discussed on the mailing list.

This PR updates ArmMmuLib to reserve page table memory for allocation during table spits to prevent the infinite loop. This follows the same pattern that the x86 CpuDxe does to preallocate page table memory: https://github.com/tianocore/edk2/blob/f962adc8a089949ecc730ba17f08234b52e3952d/UefiCpuPkg/CpuDxe/CpuPageTable.c#L1117.

It also updates the consumers of ArmMmuLib to consume the PEI version of the lib in the same commit so as to not break them.

Continuous-integration-options: PatchCheck.ignore-multi-package

  • [x] Breaking change?
    • Breaking change - Does this PR cause a break in build or boot behavior?
    • Examples: Does it add a new library class or move a module to a different repo.
  • [ ] Impacts security?
    • Security - Does this PR have a direct security impact?
    • Examples: Crypto algorithm change or buffer overflow fix.
  • [ ] Includes tests?
    • Tests - Does this PR include any explicit test code?
    • Examples: Unit tests or integration tests.

How This Was Tested

Tested on ArmVirtQemu by creating the scenario where the infinite loop (without the XN remap routine in place) and booting successfully. This was also tested using the EFI_MEMORY_RP on free memory feature that is pending upstreaming. This has been shipping in platforms.

Integration Instructions

Platforms which are using ArmMmuBaseLib for PEIM, PEI_CORE, and SEC modules will need to switch those module types to use ArmMmuPeiLib.

[LibraryClasses.common.SEC, LibraryClasses.common.PEIM, LibraryClasses.common.PEI_CORE]
  ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf

Platforms will also need to remove gArmTokenSpaceGuid.PcdRemapUnusedMemoryNx as it has been removed.

os-d avatar Oct 03 '24 21:10 os-d