ArmPkg: ArmMmuLib: Pre-Allocate Page Table Memory

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 PR updates ArmMmuLib to reserve page table memory for allocation
during table spits to prevent the infinite loop.

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

Signed-off-by: Oliver Smith-Denny <[email protected]>

ArmPkg/ArmPkg.dec

@@ -116,6 +116,10 @@
   ## ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
   gArmScmiPerformanceProtocolGuid = { 0x9b8ba84, 0x3dd3, 0x49a6, { 0xa0, 0x5a, 0x31, 0x34, 0xa5, 0xf0, 0x7b, 0xad } }
 
+  ## Arm Page Table Memory Allocation Protocol
+  ## ArmPkg/Include/Protocol/ArmPageTableMemoryAllocation.h
+  gArmPageTableMemoryAllocationProtocolGuid = { 0x623be44e, 0x506d, 0x487f, {0xa3, 0xa6, 0x92, 0xda, 0xef, 0x30, 0x2c, 0x70 } }
+
 [Ppis]
   ## Include/Ppi/ArmMpCoreInfo.h
   gArmMpCoreInfoPpiGuid = { 0x6847cc74, 0xe9ec, 0x4f8f, {0xa2, 0x9d, 0xab, 0x44, 0xe7, 0x54, 0xa8, 0xfc} }
@@ -135,11 +139,6 @@
   # Define if the GICv3 controller should use the GICv2 legacy
   gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE|BOOLEAN|0x00000042
 
-  # Whether to remap all unused memory NX before installing the CPU arch
-  # protocol driver. This is needed on platforms that map all DRAM with RWX
-  # attributes initially, and can be disabled otherwise.
-  gArmTokenSpaceGuid.PcdRemapUnusedMemoryNx|TRUE|BOOLEAN|0x00000048
-
 [PcdsFeatureFlag.ARM]
   # Whether to map normal memory as non-shareable. FALSE is the safe choice, but
   # TRUE may be appropriate to fix performance problems if you don't care about

ArmPkg/ArmPkg.dsc

@@ -94,13 +94,15 @@
 [LibraryClasses.common.SEC]
   # ARM platforms have SEC modules with standard entry points, so we can generically link StackCheckLib
   NULL|MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf
+  ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuSecLib.inf
 
 [LibraryClasses.common.PEIM]
   HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
   PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
   MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
   PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
   PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
+  ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
 
 [Components.common]
   ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.inf
@@ -162,6 +164,7 @@
   ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.inf
   ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
   ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
+  ArmPkg/Library/ArmMmuLib/ArmMmuSecLib.inf
 
 [Components.AARCH64, Components.ARM]
   ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf

ArmPkg/Drivers/CpuDxe/CpuDxe.c

@@ -229,77 +229,6 @@ InitializeDma (
   CpuArchProtocol->DmaBufferAlignment = ArmCacheWritebackGranule ();
 }
 
-/**
-  Map all EfiConventionalMemory regions in the memory map with NX
-  attributes so that allocating or freeing EfiBootServicesData regions
-  does not result in changes to memory permission attributes.
-
-**/
-STATIC
-VOID
-RemapUnusedMemoryNx (
-  VOID
-  )
-{
-  UINT64                 TestBit;
-  UINTN                  MemoryMapSize;
-  UINTN                  MapKey;
-  UINTN                  DescriptorSize;
-  UINT32                 DescriptorVersion;
-  EFI_MEMORY_DESCRIPTOR  *MemoryMap;
-  EFI_MEMORY_DESCRIPTOR  *MemoryMapEntry;
-  EFI_MEMORY_DESCRIPTOR  *MemoryMapEnd;
-  EFI_STATUS             Status;
-
-  TestBit = LShiftU64 (1, EfiBootServicesData);
-  if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & TestBit) == 0) {
-    return;
-  }
-
-  MemoryMapSize = 0;
-  MemoryMap     = NULL;
-
-  Status = gBS->GetMemoryMap (
-                  &MemoryMapSize,
-                  MemoryMap,
-                  &MapKey,
-                  &DescriptorSize,
-                  &DescriptorVersion
-                  );
-  ASSERT (Status == EFI_BUFFER_TOO_SMALL);
-  do {
-    MemoryMap = (EFI_MEMORY_DESCRIPTOR *)AllocatePool (MemoryMapSize);
-    ASSERT (MemoryMap != NULL);
-    Status = gBS->GetMemoryMap (
-                    &MemoryMapSize,
-                    MemoryMap,
-                    &MapKey,
-                    &DescriptorSize,
-                    &DescriptorVersion
-                    );
-    if (EFI_ERROR (Status)) {
-      FreePool (MemoryMap);
-    }
-  } while (Status == EFI_BUFFER_TOO_SMALL);
-
-  ASSERT_EFI_ERROR (Status);
-
-  MemoryMapEntry = MemoryMap;
-  MemoryMapEnd   = (EFI_MEMORY_DESCRIPTOR *)((UINT8 *)MemoryMap + MemoryMapSize);
-  while ((UINTN)MemoryMapEntry < (UINTN)MemoryMapEnd) {
-    if (MemoryMapEntry->Type == EfiConventionalMemory) {
-      ArmSetMemoryAttributes (
-        MemoryMapEntry->PhysicalStart,
-        EFI_PAGES_TO_SIZE (MemoryMapEntry->NumberOfPages),
-        EFI_MEMORY_XP,
-        EFI_MEMORY_XP
-        );
-    }
-
-    MemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorSize);
-  }
-}
-
 EFI_STATUS
 CpuDxeInitialize (
   IN EFI_HANDLE        ImageHandle,
@@ -313,18 +242,10 @@ CpuDxeInitialize (
 
   InitializeDma (&mCpu);
 
-  //
-  // Once we install the CPU arch protocol, the DXE core's memory
-  // protection routines will invoke them to manage the permissions of page
-  // allocations as they are created. Given that this includes pages
-  // allocated for page tables by this driver, we must ensure that unused
-  // memory is mapped with the same permissions as boot services data
-  // regions. Otherwise, we may end up with unbounded recursion, due to the
-  // fact that updating permissions on a newly allocated page table may trigger
-  // a block entry split, which triggers a page table allocation, etc etc
-  //
-  if (FeaturePcdGet (PcdRemapUnusedMemoryNx)) {
-    RemapUnusedMemoryNx ();
+  Status = InitializePageTableMemory (ImageHandle);
+  if (EFI_ERROR (Status)) {
+    ASSERT (FALSE);
+    return Status;
   }
 
   Status = gBS->InstallMultipleProtocolInterfaces (
@@ -336,6 +257,11 @@ CpuDxeInitialize (
                   NULL
                   );
 
+  if (EFI_ERROR (Status)) {
+    ASSERT (FALSE);
+    return Status;
+  }
+
   //
   // Make sure GCD and MMU settings match. This API calls gDS->SetMemorySpaceAttributes ()
   // and that calls EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes, so this code needs to go

ArmPkg/Drivers/CpuDxe/CpuDxe.h

@@ -31,6 +31,7 @@
 #include <Protocol/DebugSupport.h>
 #include <Protocol/LoadedImage.h>
 #include <Protocol/MemoryAttribute.h>
+#include <Protocol/ArmPageTableMemoryAllocation.h>
 
 extern BOOLEAN  mIsFlushingGCD;
 
@@ -143,4 +144,19 @@ RegionAttributeToGcdAttribute (
   IN UINTN  PageAttributes
   );
 
+/**
+  Initialize the page table memory pool and produce the page table memory allocation
+  protocol.
+
+  @param[in] ImageHandle  Handle on which to install the protocol.
+
+  @retval EFI_SUCCESS           The page table pool was initialized and protocol produced.
+  @retval Others                The driver returned an error while initializing.
+**/
+EFI_STATUS
+EFIAPI
+InitializePageTableMemory (
+  IN EFI_HANDLE  ImageHandle
+  );
+
 #endif // CPU_DXE_H_

ArmPkg/Drivers/CpuDxe/CpuDxe.inf

@@ -24,6 +24,7 @@
   CpuMmuCommon.c
   Exception.c
   MemoryAttribute.c
+  PageTableMemoryAllocation.c
 
 [Sources.ARM]
   Arm/Mmu.c
@@ -40,6 +41,7 @@
 [LibraryClasses]
   ArmLib
   ArmMmuLib
+  BaseLib
   BaseMemoryLib
   CacheMaintenanceLib
   CpuLib
@@ -56,6 +58,7 @@
 [Protocols]
   gEfiCpuArchProtocolGuid
   gEfiMemoryAttributeProtocolGuid
+  gArmPageTableMemoryAllocationProtocolGuid
 
 [Guids]
   gEfiDebugImageInfoTableGuid
@@ -69,7 +72,6 @@
 
 [FeaturePcd.common]
   gArmTokenSpaceGuid.PcdDebuggerExceptionSupport
-  gArmTokenSpaceGuid.PcdRemapUnusedMemoryNx
 
 [Depex]
   gHardwareInterruptProtocolGuid OR gHardwareInterrupt2ProtocolGuid