ArmPkg/CpuDxe ARM: honour RO/XP attributes in SetMemoryAttributes()

Enable the use of strict memory permissions on ARM by processing the
EFI_MEMORY_RO and EFI_MEMORY_XP rather than ignoring them. As before,
calls to CpuArchProtocol::SetMemoryAttributes that only set RO/XP
bits will preserve the cacheability attributes. Permissions attributes
are not preserved when setting the memory type only: the way the memory
permission attributes are defined does not allows for that, and so this
situation does not deviate from other architectures.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

diff --git a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
index a2993cf16a35eb95cc36edd6ac5cf4d43117aec3..d3c307f4831788a0bb4a4ba944957d97f743ae2a 100644 (file)
--- a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
+++ b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
@@ -19,6 +19,13 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Library/MemoryAllocationLib.h>\r
 #include "CpuDxe.h"\r
 \r
 #include <Library/MemoryAllocationLib.h>\r
 #include "CpuDxe.h"\r
 \r

+#define CACHE_ATTRIBUTE_MASK   (EFI_MEMORY_UC | \\r
+                                EFI_MEMORY_WC | \\r
+                                EFI_MEMORY_WT | \\r
+                                EFI_MEMORY_WB | \\r
+                                EFI_MEMORY_UCE | \\r
+                                EFI_MEMORY_WP)\r
+\r

 // First Level Descriptors\r
 typedef UINT32    ARM_FIRST_LEVEL_DESCRIPTOR;\r
 \r
 // First Level Descriptors\r
 typedef UINT32    ARM_FIRST_LEVEL_DESCRIPTOR;\r
 \r


@@ -374,50 +381,48 @@ UpdatePageEntries (
 \r
   // EntryMask: bitmask of values to change (1 = change this value, 0 = leave alone)\r
   // EntryValue: values at bit positions specified by EntryMask\r
 \r
   // EntryMask: bitmask of values to change (1 = change this value, 0 = leave alone)\r
   // EntryValue: values at bit positions specified by EntryMask\r

-  EntryMask = TT_DESCRIPTOR_PAGE_TYPE_MASK;\r
-  EntryValue = TT_DESCRIPTOR_PAGE_TYPE_PAGE;\r
-  // Although the PI spec is unclear on this the GCD guarantees that only\r
-  // one Attribute bit is set at a time, so we can safely use a switch statement\r
-  switch (Attributes) {\r
-    case EFI_MEMORY_UC:\r
-      // modify cacheability attributes\r
-      EntryMask |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_MASK;\r
-      // map to strongly ordered\r
-      EntryValue |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_STRONGLY_ORDERED; // TEX[2:0] = 0, C=0, B=0\r
-      break;\r
-\r
-    case EFI_MEMORY_WC:\r
-      // modify cacheability attributes\r
-      EntryMask |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_MASK;\r
-      // map to normal non-cachable\r
-      EntryValue |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_NON_CACHEABLE; // TEX [2:0]= 001 = 0x2, B=0, C=0\r
-      break;\r
-\r
-    case EFI_MEMORY_WT:\r
-      // modify cacheability attributes\r
-      EntryMask |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_MASK;\r
-      // write through with no-allocate\r
-      EntryValue |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_WRITE_THROUGH_NO_ALLOC; // TEX [2:0] = 0, C=1, B=0\r
-      break;\r
-\r
-    case EFI_MEMORY_WB:\r
-      // modify cacheability attributes\r
-      EntryMask |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_MASK;\r
-      // write back (with allocate)\r
-      EntryValue |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_WRITE_BACK_ALLOC; // TEX [2:0] = 001, C=1, B=1\r
-      break;\r
+  EntryMask = TT_DESCRIPTOR_PAGE_TYPE_MASK | TT_DESCRIPTOR_PAGE_AP_MASK;\r
+  if ((Attributes & EFI_MEMORY_XP) != 0) {\r
+    EntryValue = TT_DESCRIPTOR_PAGE_TYPE_PAGE_XN;\r
+  } else {\r
+    EntryValue = TT_DESCRIPTOR_PAGE_TYPE_PAGE;\r
+  }\r

 \r
 \r

-    case EFI_MEMORY_WP:\r
-    case EFI_MEMORY_XP:\r
-    case EFI_MEMORY_UCE:\r
-      // cannot be implemented UEFI definition unclear for ARM\r
-      // Cause a page fault if these ranges are accessed.\r
-      EntryValue = TT_DESCRIPTOR_PAGE_TYPE_FAULT;\r
-      DEBUG ((EFI_D_PAGE, "SetMemoryAttributes(): setting page %lx with unsupported attribute %x will page fault on access\n", BaseAddress, Attributes));\r
-      break;\r
+  // Although the PI spec is unclear on this, the GCD guarantees that only\r
+  // one Attribute bit is set at a time, so the order of the conditionals below\r
+  // is irrelevant. If no memory attribute is specified, we preserve whatever\r
+  // memory type is set in the page tables, and update the permission attributes\r
+  // only.\r
+  if (Attributes & EFI_MEMORY_UC) {\r
+    // modify cacheability attributes\r
+    EntryMask |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_MASK;\r
+    // map to strongly ordered\r
+    EntryValue |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_STRONGLY_ORDERED; // TEX[2:0] = 0, C=0, B=0\r
+  } else if (Attributes & EFI_MEMORY_WC) {\r
+    // modify cacheability attributes\r
+    EntryMask |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_MASK;\r
+    // map to normal non-cachable\r
+    EntryValue |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_NON_CACHEABLE; // TEX [2:0]= 001 = 0x2, B=0, C=0\r
+  } else if (Attributes & EFI_MEMORY_WT) {\r
+    // modify cacheability attributes\r
+    EntryMask |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_MASK;\r
+    // write through with no-allocate\r
+    EntryValue |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_WRITE_THROUGH_NO_ALLOC; // TEX [2:0] = 0, C=1, B=0\r
+  } else if (Attributes & EFI_MEMORY_WB) {\r
+    // modify cacheability attributes\r
+    EntryMask |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_MASK;\r
+    // write back (with allocate)\r
+    EntryValue |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_WRITE_BACK_ALLOC; // TEX [2:0] = 001, C=1, B=1\r
+  } else if (Attributes & CACHE_ATTRIBUTE_MASK) {\r
+    // catch unsupported memory type attributes\r
+    ASSERT (FALSE);\r
+    return EFI_UNSUPPORTED;\r
+  }\r

 \r
 \r

-    default:\r
-      return EFI_UNSUPPORTED;\r
+  if ((Attributes & EFI_MEMORY_RO) != 0) {\r
+    EntryValue |= TT_DESCRIPTOR_PAGE_AP_RO_RO;\r
+  } else {\r
+    EntryValue |= TT_DESCRIPTOR_PAGE_AP_RW_RW;\r

   }\r
 \r
   // Obtain page table base\r
   }\r
 \r
   // Obtain page table base\r


@@ -520,53 +525,49 @@ UpdateSectionEntries (
   // EntryValue: values at bit positions specified by EntryMask\r
 \r
   // Make sure we handle a section range that is unmapped\r
   // EntryValue: values at bit positions specified by EntryMask\r
 \r
   // Make sure we handle a section range that is unmapped\r

-  EntryMask = TT_DESCRIPTOR_SECTION_TYPE_MASK;\r
+  EntryMask = TT_DESCRIPTOR_SECTION_TYPE_MASK | TT_DESCRIPTOR_SECTION_XN_MASK |\r
+              TT_DESCRIPTOR_SECTION_AP_MASK;\r

   EntryValue = TT_DESCRIPTOR_SECTION_TYPE_SECTION;\r
 \r
   EntryValue = TT_DESCRIPTOR_SECTION_TYPE_SECTION;\r
 \r

-  // Although the PI spec is unclear on this the GCD guarantees that only\r
-  // one Attribute bit is set at a time, so we can safely use a switch statement\r
-  switch(Attributes) {\r
-    case EFI_MEMORY_UC:\r
-      // modify cacheability attributes\r
-      EntryMask |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK;\r
-      // map to strongly ordered\r
-      EntryValue |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_STRONGLY_ORDERED; // TEX[2:0] = 0, C=0, B=0\r
-      break;\r
-\r
-    case EFI_MEMORY_WC:\r
-      // modify cacheability attributes\r
-      EntryMask |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK;\r
-      // map to normal non-cachable\r
-      EntryValue |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_NON_CACHEABLE; // TEX [2:0]= 001 = 0x2, B=0, C=0\r
-      break;\r
-\r
-    case EFI_MEMORY_WT:\r
-      // modify cacheability attributes\r
-      EntryMask |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK;\r
-      // write through with no-allocate\r
-      EntryValue |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_WRITE_THROUGH_NO_ALLOC; // TEX [2:0] = 0, C=1, B=0\r
-      break;\r
-\r
-    case EFI_MEMORY_WB:\r
-      // modify cacheability attributes\r
-      EntryMask |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK;\r
-      // write back (with allocate)\r
-      EntryValue |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_WRITE_BACK_ALLOC; // TEX [2:0] = 001, C=1, B=1\r
-      break;\r
-\r
-    case EFI_MEMORY_WP:\r
-    case EFI_MEMORY_XP:\r
-    case EFI_MEMORY_RP:\r
-    case EFI_MEMORY_UCE:\r
-      // cannot be implemented UEFI definition unclear for ARM\r
-      // Cause a page fault if these ranges are accessed.\r
-      EntryValue = TT_DESCRIPTOR_SECTION_TYPE_FAULT;\r
-      DEBUG ((EFI_D_PAGE, "SetMemoryAttributes(): setting section %lx with unsupported attribute %x will page fault on access\n", BaseAddress, Attributes));\r
-      break;\r
+  // Although the PI spec is unclear on this, the GCD guarantees that only\r
+  // one Attribute bit is set at a time, so the order of the conditionals below\r
+  // is irrelevant. If no memory attribute is specified, we preserve whatever\r
+  // memory type is set in the page tables, and update the permission attributes\r
+  // only.\r
+  if (Attributes & EFI_MEMORY_UC) {\r
+    // modify cacheability attributes\r
+    EntryMask |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK;\r
+    // map to strongly ordered\r
+    EntryValue |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_STRONGLY_ORDERED; // TEX[2:0] = 0, C=0, B=0\r
+  } else if (Attributes & EFI_MEMORY_WC) {\r
+    // modify cacheability attributes\r
+    EntryMask |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK;\r
+    // map to normal non-cachable\r
+    EntryValue |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_NON_CACHEABLE; // TEX [2:0]= 001 = 0x2, B=0, C=0\r
+  } else if (Attributes & EFI_MEMORY_WT) {\r
+    // modify cacheability attributes\r
+    EntryMask |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK;\r
+    // write through with no-allocate\r
+    EntryValue |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_WRITE_THROUGH_NO_ALLOC; // TEX [2:0] = 0, C=1, B=0\r
+  } else if (Attributes & EFI_MEMORY_WB) {\r
+    // modify cacheability attributes\r
+    EntryMask |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK;\r
+    // write back (with allocate)\r
+    EntryValue |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_WRITE_BACK_ALLOC; // TEX [2:0] = 001, C=1, B=1\r
+  } else if (Attributes & CACHE_ATTRIBUTE_MASK) {\r
+    // catch unsupported memory type attributes\r
+    ASSERT (FALSE);\r
+    return EFI_UNSUPPORTED;\r
+  }\r

 \r
 \r

+  if (Attributes & EFI_MEMORY_RO) {\r
+    EntryValue |= TT_DESCRIPTOR_SECTION_AP_RO_RO;\r
+  } else {\r
+    EntryValue |= TT_DESCRIPTOR_SECTION_AP_RW_RW;\r
+  }\r

 \r
 \r

-    default:\r
-      return EFI_UNSUPPORTED;\r
+  if (Attributes & EFI_MEMORY_XP) {\r
+    EntryValue |= TT_DESCRIPTOR_SECTION_XN_MASK;\r

   }\r
 \r
   // obtain page table base\r
   }\r
 \r
   // obtain page table base\r


@@ -697,13 +698,6 @@ SetMemoryAttributes (
     return EFI_SUCCESS;\r
   }\r
 \r
     return EFI_SUCCESS;\r
   }\r
 \r

-  //\r
-  // Ignore invocations that only modify permission bits\r
-  //\r
-  if ((Attributes & EFI_MEMORY_CACHETYPE_MASK) == 0) {\r
-    return EFI_SUCCESS;\r
-  }\r
-\r

   FlushTlbs = FALSE;\r
   while (Length > 0) {\r
     if ((BaseAddress % TT_DESCRIPTOR_SECTION_SIZE == 0) &&\r
   FlushTlbs = FALSE;\r
   while (Length > 0) {\r
     if ((BaseAddress % TT_DESCRIPTOR_SECTION_SIZE == 0) &&\r