Sync AARCH64 GCD Capabilities with Page Table

On AARCH64 systems, the GCD is not fully synced with the page table. On
x86 systems, the GCD is synced by adding `EFI_MEMORY_RO`,
`EFI_MEMORY_RP`, and `EFI_MEMORY_XP` to the current capabilities of the
GCD, then the page table attributes are set on the GCD attributes.

However, on AARCH64, the GCD capabilities do not get updated, instead
only the attributes from the page table are masked by the existing GCD
capabilities, which means that any new page table attribute which are
already set are dropped and the GCD does not reflect the state of the
system. This has been seen to cause issues where memory in the page
table that was marked `EFI_MEMORY_XP` had an additional attribute set
using the GCD capabilities, which did not include `EFI_MEMORY_XP`, this
caused the page table to be updated to lose `EFI_MEMORY_XP`, which is a
potential security issue.

The existing behavior on AARCH64 systems is an implementation error, it
assumes one of two things:
- The page table attributes must be a subset of the GCD capabilities
- The GCD does not need to have its capabilities synced to what the page
table attributes are

The first is incorrect as important attributes such as `EFI_MEMORY_XP`
do not get applied to the GCD capabilities by default and therefore must
be synced back. This comment from ArmPkg's CpuDxe driver helps explain:

```c
  // The GCD implementation maintains its own copy of the state of memory
  // space attributes.  GCD needs to know what the initial memory space
  // attributes are.  The CPU Arch. Protocol does not provide a
  // GetMemoryAttributes function for GCD to get this so we must resort to
  // calling GCD (as if we were a client) to update its copy of the
  // attributes.  This is bad architecture and should be replaced with a
  // way for GCD to query the CPU Arch. driver of the existing memory
  // space attributes instead.
```

However, this comment misses that updating the capabilities is critical
to updating the attributes.

The second is incorrect because significant pieces of core code
reference the GCD attributes instead of the page table attributes. For
example, NonDiscoverablePciDeviceDxe uses the GCD capabilities and
attributes when interacting with a non-discoverable PCI device. When the
GCD is not synced to the page table, we get the errors and security
concerns listed above.

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

ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c

@@ -90,6 +90,7 @@ SetGcdMemorySpaceAttributes (
   UINTN                 EndIndex;
   EFI_PHYSICAL_ADDRESS  RegionStart;
   UINT64                RegionLength;
+  UINT64                Capabilities;
 
   DEBUG ((
     DEBUG_GCD,
@@ -146,14 +147,56 @@ SetGcdMemorySpaceAttributes (
       RegionLength = MemorySpaceMap[Index].BaseAddress + MemorySpaceMap[Index].Length - RegionStart;
     }
 
+    // Always add RO, RP, and XP, as all memory is capable of supporting these types (they are software constructs,
+    // not hardware features) and they are critical to maintaining a security boundary.
+    Capabilities = MemorySpaceMap[Index].Capabilities | EFI_MEMORY_RO | EFI_MEMORY_RP | EFI_MEMORY_XP;
+
+    // Update GCD capabilities as these may have changed in the page table from the original GCD setting
+    // this follows the same pattern as x86 GCD and Page Table syncing
+    Status = gDS->SetMemorySpaceCapabilities (
+                    RegionStart,
+                    RegionLength,
+                    Capabilities
+                    );
+
+    if (EFI_ERROR (Status)) {
+      DEBUG ((
+        DEBUG_ERROR,
+        "%a - failed to update GCD capabilities: 0x%llx on memory region: 0x%llx length: 0x%llx Status: %r\n",
+        __func__,
+        Capabilities,
+        RegionStart,
+        RegionLength,
+        Status
+        ));
+
+      // If we fail to set capabilities, we should assert as this is a GCD internal error, but follow the previous
+      // behavior and try to set the attributes (which may or may not fail)
+      ASSERT_EFI_ERROR (Status);
+    }
+
     //
-    // Set memory attributes according to MTRR attribute and the original attribute of descriptor
+    // Set memory attributes according to page table attributes and the original attributes of descriptor
     //
-    gDS->SetMemorySpaceAttributes (
-           RegionStart,
-           RegionLength,
-           (MemorySpaceMap[Index].Attributes & ~EFI_MEMORY_CACHETYPE_MASK) | (MemorySpaceMap[Index].Capabilities & Attributes)
-           );
+    Status = gDS->SetMemorySpaceAttributes (
+                    RegionStart,
+                    RegionLength,
+                    (MemorySpaceMap[Index].Attributes & ~EFI_MEMORY_CACHETYPE_MASK) | (Attributes & Capabilities)
+                    );
+
+    if (EFI_ERROR (Status)) {
+      DEBUG ((
+        DEBUG_ERROR,
+        "%a - failed to update GCD attributes: 0x%llx on memory region: 0x%llx length: 0x%llx Status: %r\n",
+        __func__,
+        Attributes,
+        RegionStart,
+        RegionLength,
+        Status
+        ));
+
+      ASSERT_EFI_ERROR (Status);
+    }
   }
 
   return EFI_SUCCESS;