Sync AARCH64 GCD Capabilities with Page Table (#89)

Please ensure you have read the [contribution
docs](https://github.com/microsoft/mu/blob/master/CONTRIBUTING.md) prior
to submitting the pull request. In particular,
[pull request
guidelines](https://github.com/microsoft/mu/blob/master/CONTRIBUTING.md#pull-request-best-practices).

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.

This patch goes one step further than the x86 GCD syncing by applying
any existing non-cache attributes (which it is not supported to update)
in the page table (today this is still the same set as in x86 but is
more extensible for when new attributes are added) to the GCD
capabilities and attributes. In this way, the GCD will actually be fully
synced to the already existent state of the page table.

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.

For each item, place an "x" in between `[` and `]` if true. Example:
`[x]`.
_(you can also check items in the GitHub UI)_

- [x] Impacts functionality?
- **Functionality** - Does the change ultimately impact how firmware
functions?
- Examples: Add a new library, publish a new PPI, update an algorithm,
...
- [x] Impacts security?
- **Security** - Does the change have a direct security impact on an
application,
    flow, or firmware?
  - Examples: Crypto algorithm change, buffer overflow fix, parameter
    validation improvement, ...
- [ ] Breaking change?
- **Breaking change** - Will anyone consuming this change experience a
break
    in build or boot behavior?
- Examples: Add a new library class, move a module to a different repo,
call
    a function in a new library class in a pre-existing module, ...
- [ ] Includes tests?
  - **Tests** - Does the change include any explicit test code?
  - Examples: Unit tests, integration tests, robot tests, ...
- [ ] Includes documentation?
- **Documentation** - Does the change contain explicit documentation
additions
    outside direct code modifications (and comments)?
- Examples: Update readme file, add feature readme file, link to
documentation
    on an a separate Web page, ...

This was tested on QEMU and physical hardware that was experiencing the
issue where `EFI_MEMORY_XP` was being removed when an additional
attribute was set.

N/A.

Author: os-d

ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c

@@ -90,6 +90,7 @@ SetGcdMemorySpaceAttributes (
   UINTN                 EndIndex;
   EFI_PHYSICAL_ADDRESS  RegionStart;
   UINT64                RegionLength;
+  UINT64                Capabilities; // MU_CHANGE: Sync AARCH64 GCD Capabilities with Page Table
 
   DEBUG ((
     DEBUG_GCD,
@@ -146,14 +147,57 @@ SetGcdMemorySpaceAttributes (
       RegionLength = MemorySpaceMap[Index].BaseAddress + MemorySpaceMap[Index].Length - RegionStart;
     }
 
+    // MU_CHANGE START: Sync AARCH64 GCD Capabilities with Page Table
+    // 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
+        ));
+      ASSERT_EFI_ERROR (Status);
+      continue;
+    }
+
     //
     // Set memory attributes according to MTRR attribute and the original attribute 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);
+      continue;
+    }
+
+    // MU_CHANGE END
   }
 
   return EFI_SUCCESS;