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

[mirror_edk2.git] / ArmPkg / Drivers / CpuDxe / Arm / Mmu.c

diff --git a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
index 63da8ba8cb88567b6ed8943ffcb22903ede1e56f..d3c307f4831788a0bb4a4ba944957d97f743ae2a 100644 (file)
--- a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
+++ b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
@@ -3,6 +3,7 @@
 Copyright (c) 2009, Hewlett-Packard Company. All rights reserved.<BR>\r
 Portions copyright (c) 2010, Apple Inc. All rights reserved.<BR>\r
 Portions copyright (c) 2013, ARM Ltd. All rights reserved.<BR>\r
 Copyright (c) 2009, Hewlett-Packard Company. All rights reserved.<BR>\r
 Portions copyright (c) 2010, Apple Inc. All rights reserved.<BR>\r
 Portions copyright (c) 2013, ARM Ltd. All rights reserved.<BR>\r

+Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>\r

 \r
 This program and the accompanying materials\r
 are licensed and made available under the terms and conditions of the BSD License\r
 \r
 This program and the accompanying materials\r
 are licensed and made available under the terms and conditions of the BSD License\r


@@ -18,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


@@ -62,7 +70,7 @@ SectionToGcdAttributes (
   // determine protection attributes\r
   switch(SectionAttributes & TT_DESCRIPTOR_SECTION_AP_MASK) {\r
     case TT_DESCRIPTOR_SECTION_AP_NO_NO: // no read, no write\r
   // determine protection attributes\r
   switch(SectionAttributes & TT_DESCRIPTOR_SECTION_AP_MASK) {\r
     case TT_DESCRIPTOR_SECTION_AP_NO_NO: // no read, no write\r

-      //*GcdAttributes |= EFI_MEMORY_WP | EFI_MEMORY_RP;\r
+      //*GcdAttributes |= EFI_MEMORY_RO | EFI_MEMORY_RP;\r

       break;\r
 \r
     case TT_DESCRIPTOR_SECTION_AP_RW_NO:\r
       break;\r
 \r
     case TT_DESCRIPTOR_SECTION_AP_RW_NO:\r


@@ -73,7 +81,7 @@ SectionToGcdAttributes (
     // read only cases map to write-protect\r
     case TT_DESCRIPTOR_SECTION_AP_RO_NO:\r
     case TT_DESCRIPTOR_SECTION_AP_RO_RO:\r
     // read only cases map to write-protect\r
     case TT_DESCRIPTOR_SECTION_AP_RO_NO:\r
     case TT_DESCRIPTOR_SECTION_AP_RO_RO:\r

-      *GcdAttributes |= EFI_MEMORY_WP;\r
+      *GcdAttributes |= EFI_MEMORY_RO;\r

       break;\r
 \r
     default:\r
       break;\r
 \r
     default:\r


@@ -126,7 +134,7 @@ PageToGcdAttributes (
   // determine protection attributes\r
   switch(PageAttributes & TT_DESCRIPTOR_PAGE_AP_MASK) {\r
     case TT_DESCRIPTOR_PAGE_AP_NO_NO: // no read, no write\r
   // determine protection attributes\r
   switch(PageAttributes & TT_DESCRIPTOR_PAGE_AP_MASK) {\r
     case TT_DESCRIPTOR_PAGE_AP_NO_NO: // no read, no write\r

-      //*GcdAttributes |= EFI_MEMORY_WP | EFI_MEMORY_RP;\r
+      //*GcdAttributes |= EFI_MEMORY_RO | EFI_MEMORY_RP;\r

       break;\r
 \r
     case TT_DESCRIPTOR_PAGE_AP_RW_NO:\r
       break;\r
 \r
     case TT_DESCRIPTOR_PAGE_AP_RW_NO:\r


@@ -137,7 +145,7 @@ PageToGcdAttributes (
     // read only cases map to write-protect\r
     case TT_DESCRIPTOR_PAGE_AP_RO_NO:\r
     case TT_DESCRIPTOR_PAGE_AP_RO_RO:\r
     // read only cases map to write-protect\r
     case TT_DESCRIPTOR_PAGE_AP_RO_NO:\r
     case TT_DESCRIPTOR_PAGE_AP_RO_RO:\r

-      *GcdAttributes |= EFI_MEMORY_WP;\r
+      *GcdAttributes |= EFI_MEMORY_RO;\r

       break;\r
 \r
     default:\r
       break;\r
 \r
     default:\r


@@ -297,6 +305,11 @@ SyncCacheConfig (
       }\r
       NextRegionLength += TT_DESCRIPTOR_SECTION_SIZE;\r
     } else if (TT_DESCRIPTOR_SECTION_TYPE_IS_PAGE_TABLE(FirstLevelTable[i])) {\r
       }\r
       NextRegionLength += TT_DESCRIPTOR_SECTION_SIZE;\r
     } else if (TT_DESCRIPTOR_SECTION_TYPE_IS_PAGE_TABLE(FirstLevelTable[i])) {\r

+      // In this case any bits set in the 'NextSectionAttributes' are garbage and were set from\r
+      // bits that are actually part of the pagetable address.  We clear it out to zero so that\r
+      // the SyncCacheConfigPage will use the page attributes instead of trying to convert the\r
+      // section attributes into page attributes\r
+      NextSectionAttributes = 0;\r

       Status = SyncCacheConfigPage (\r
           i,FirstLevelTable[i],\r
           NumberOfDescriptors, MemorySpaceMap,\r
       Status = SyncCacheConfigPage (\r
           i,FirstLevelTable[i],\r
           NumberOfDescriptors, MemorySpaceMap,\r


@@ -341,10 +354,11 @@ SyncCacheConfig (
 \r
 EFI_STATUS\r
 UpdatePageEntries (\r
 \r
 EFI_STATUS\r
 UpdatePageEntries (\r

-  IN EFI_PHYSICAL_ADDRESS      BaseAddress,\r
-  IN UINT64                    Length,\r
-  IN UINT64                    Attributes,\r
-  IN EFI_PHYSICAL_ADDRESS      VirtualMask\r
+  IN  EFI_PHYSICAL_ADDRESS      BaseAddress,\r
+  IN  UINT64                    Length,\r
+  IN  UINT64                    Attributes,\r
+  IN  EFI_PHYSICAL_ADDRESS      VirtualMask,\r
+  OUT BOOLEAN                   *FlushTlbs OPTIONAL\r

   )\r
 {\r
   EFI_STATUS    Status;\r
   )\r
 {\r
   EFI_STATUS    Status;\r


@@ -367,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


@@ -440,6 +452,9 @@ UpdatePageEntries (
 \r
       // Re-read descriptor\r
       Descriptor = FirstLevelTable[FirstLevelIdx];\r
 \r
       // Re-read descriptor\r
       Descriptor = FirstLevelTable[FirstLevelIdx];\r

+      if (FlushTlbs != NULL) {\r
+        *FlushTlbs = TRUE;\r
+      }\r

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


@@ -465,15 +480,16 @@ UpdatePageEntries (
 \r
     if (CurrentPageTableEntry  != PageTableEntry) {\r
       Mva = (VOID *)(UINTN)((((UINTN)FirstLevelIdx) << TT_DESCRIPTOR_SECTION_BASE_SHIFT) + (PageTableIndex << TT_DESCRIPTOR_PAGE_BASE_SHIFT));\r
 \r
     if (CurrentPageTableEntry  != PageTableEntry) {\r
       Mva = (VOID *)(UINTN)((((UINTN)FirstLevelIdx) << TT_DESCRIPTOR_SECTION_BASE_SHIFT) + (PageTableIndex << TT_DESCRIPTOR_PAGE_BASE_SHIFT));\r

-      if ((CurrentPageTableEntry & TT_DESCRIPTOR_PAGE_CACHEABLE_MASK) == TT_DESCRIPTOR_PAGE_CACHEABLE_MASK) {\r
-        // The current section mapping is cacheable so Clean/Invalidate the MVA of the page\r
-        // Note assumes switch(Attributes), not ARMv7 possibilities\r
-        WriteBackInvalidateDataCacheRange (Mva, TT_DESCRIPTOR_PAGE_SIZE);\r
-      }\r

 \r
       // Only need to update if we are changing the entry\r
       PageTable[PageTableIndex] = PageTableEntry;\r
       ArmUpdateTranslationTableEntry ((VOID *)&PageTable[PageTableIndex], Mva);\r
 \r
       // Only need to update if we are changing the entry\r
       PageTable[PageTableIndex] = PageTableEntry;\r
       ArmUpdateTranslationTableEntry ((VOID *)&PageTable[PageTableIndex], Mva);\r

+\r
+      // Clean/invalidate the cache for this page, but only\r
+      // if we are modifying the memory type attributes\r
+      if (((CurrentPageTableEntry ^ PageTableEntry) & TT_DESCRIPTOR_PAGE_CACHE_POLICY_MASK) != 0) {\r
+        WriteBackInvalidateDataCacheRange (Mva, TT_DESCRIPTOR_PAGE_SIZE);\r
+      }\r

     }\r
 \r
     Status = EFI_SUCCESS;\r
     }\r
 \r
     Status = EFI_SUCCESS;\r


@@ -509,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


@@ -575,7 +587,12 @@ UpdateSectionEntries (
     // has this descriptor already been coverted to pages?\r
     if (TT_DESCRIPTOR_SECTION_TYPE_IS_PAGE_TABLE(CurrentDescriptor)) {\r
       // forward this 1MB range to page table function instead\r
     // has this descriptor already been coverted to pages?\r
     if (TT_DESCRIPTOR_SECTION_TYPE_IS_PAGE_TABLE(CurrentDescriptor)) {\r
       // forward this 1MB range to page table function instead\r

-      Status = UpdatePageEntries ((FirstLevelIdx + i) << TT_DESCRIPTOR_SECTION_BASE_SHIFT, TT_DESCRIPTOR_SECTION_SIZE, Attributes, VirtualMask);\r
+      Status = UpdatePageEntries (\r
+                 (FirstLevelIdx + i) << TT_DESCRIPTOR_SECTION_BASE_SHIFT,\r
+                 TT_DESCRIPTOR_SECTION_SIZE,\r
+                 Attributes,\r
+                 VirtualMask,\r
+                 NULL);\r

     } else {\r
       // still a section entry\r
 \r
     } else {\r
       // still a section entry\r
 \r


@@ -590,15 +607,16 @@ UpdateSectionEntries (
 \r
       if (CurrentDescriptor  != Descriptor) {\r
         Mva = (VOID *)(UINTN)(((UINTN)FirstLevelTable) << TT_DESCRIPTOR_SECTION_BASE_SHIFT);\r
 \r
       if (CurrentDescriptor  != Descriptor) {\r
         Mva = (VOID *)(UINTN)(((UINTN)FirstLevelTable) << TT_DESCRIPTOR_SECTION_BASE_SHIFT);\r

-        if ((CurrentDescriptor & TT_DESCRIPTOR_SECTION_CACHEABLE_MASK) == TT_DESCRIPTOR_SECTION_CACHEABLE_MASK) {\r
-          // The current section mapping is cacheable so Clean/Invalidate the MVA of the section\r
-          // Note assumes switch(Attributes), not ARMv7 possabilities\r
-          WriteBackInvalidateDataCacheRange (Mva, SIZE_1MB);\r
-        }\r

 \r
         // Only need to update if we are changing the descriptor\r
         FirstLevelTable[FirstLevelIdx + i] = Descriptor;\r
         ArmUpdateTranslationTableEntry ((VOID *)&FirstLevelTable[FirstLevelIdx + i], Mva);\r
 \r
         // Only need to update if we are changing the descriptor\r
         FirstLevelTable[FirstLevelIdx + i] = Descriptor;\r
         ArmUpdateTranslationTableEntry ((VOID *)&FirstLevelTable[FirstLevelIdx + i], Mva);\r

+\r
+        // Clean/invalidate the cache for this section, but only\r
+        // if we are modifying the memory type attributes\r
+        if (((CurrentDescriptor ^ Descriptor) & TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK) != 0) {\r
+          WriteBackInvalidateDataCacheRange (Mva, SIZE_1MB);\r
+        }\r

       }\r
 \r
       Status = EFI_SUCCESS;\r
       }\r
 \r
       Status = EFI_SUCCESS;\r


@@ -673,27 +691,59 @@ SetMemoryAttributes (
   )\r
 {\r
   EFI_STATUS    Status;\r
   )\r
 {\r
   EFI_STATUS    Status;\r

+  UINT64        ChunkLength;\r
+  BOOLEAN       FlushTlbs;\r

 \r
 \r

-  if(((BaseAddress & 0xFFFFF) == 0) && ((Length & 0xFFFFF) == 0)) {\r
-    // Is the base and length a multiple of 1 MB?\r
-    DEBUG ((EFI_D_PAGE, "SetMemoryAttributes(): MMU section 0x%x length 0x%x to %lx\n", (UINTN)BaseAddress, (UINTN)Length, Attributes));\r
-    Status = UpdateSectionEntries (BaseAddress, Length, Attributes, VirtualMask);\r
-  } else {\r
-    // Base and/or length is not a multiple of 1 MB\r
-    DEBUG ((EFI_D_PAGE, "SetMemoryAttributes(): MMU page 0x%x length 0x%x to %lx\n", (UINTN)BaseAddress, (UINTN)Length, Attributes));\r
-    Status = UpdatePageEntries (BaseAddress, Length, Attributes, VirtualMask);\r
+  if (Length == 0) {\r
+    return EFI_SUCCESS;\r

   }\r
 \r
   }\r
 \r

-  // Flush d-cache so descriptors make it back to uncached memory for subsequent table walks\r
-  // flush and invalidate pages\r
-  //TODO: Do we really need to invalidate the caches everytime we change the memory attributes ?\r
-  ArmCleanInvalidateDataCache ();\r
+  FlushTlbs = FALSE;\r
+  while (Length > 0) {\r
+    if ((BaseAddress % TT_DESCRIPTOR_SECTION_SIZE == 0) &&\r
+        Length >= TT_DESCRIPTOR_SECTION_SIZE) {\r
+\r
+      ChunkLength = Length - Length % TT_DESCRIPTOR_SECTION_SIZE;\r
+\r
+      DEBUG ((DEBUG_PAGE,\r
+        "SetMemoryAttributes(): MMU section 0x%lx length 0x%lx to %lx\n",\r
+        BaseAddress, ChunkLength, Attributes));\r

 \r
 \r

-  ArmInvalidateInstructionCache ();\r
+      Status = UpdateSectionEntries (BaseAddress, ChunkLength, Attributes,\r
+                 VirtualMask);\r
+\r
+      FlushTlbs = TRUE;\r
+    } else {\r
+\r
+      //\r
+      // Process page by page until the next section boundary, but only if\r
+      // we have more than a section's worth of area to deal with after that.\r
+      //\r
+      ChunkLength = TT_DESCRIPTOR_SECTION_SIZE -\r
+                    (BaseAddress % TT_DESCRIPTOR_SECTION_SIZE);\r
+      if (ChunkLength + TT_DESCRIPTOR_SECTION_SIZE > Length) {\r
+        ChunkLength = Length;\r
+      }\r

 \r
 \r

-  // Invalidate all TLB entries so changes are synced\r
-  ArmInvalidateTlb ();\r
+      DEBUG ((DEBUG_PAGE,\r
+        "SetMemoryAttributes(): MMU page 0x%lx length 0x%lx to %lx\n",\r
+        BaseAddress, ChunkLength, Attributes));\r

 \r
 \r

+      Status = UpdatePageEntries (BaseAddress, ChunkLength, Attributes,\r
+                 VirtualMask, &FlushTlbs);\r
+    }\r
+\r
+    if (EFI_ERROR (Status)) {\r
+      break;\r
+    }\r
+\r
+    BaseAddress += ChunkLength;\r
+    Length -= ChunkLength;\r
+  }\r
+\r
+  if (FlushTlbs) {\r
+    ArmInvalidateTlb ();\r
+  }\r

   return Status;\r
 }\r
 \r
   return Status;\r
 }\r
 \r


@@ -725,20 +775,14 @@ EfiAttributeToArmAttribute (
       ArmAttributes = TT_DESCRIPTOR_SECTION_CACHE_POLICY_WRITE_BACK_ALLOC; // TEX [2:0] = 001, C=1, B=1\r
       break;\r
 \r
       ArmAttributes = 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
     default:\r
     case EFI_MEMORY_UCE:\r
     default:\r

-      // Cannot be implemented UEFI definition unclear for ARM\r
-      // Cause a page fault if these ranges are accessed.\r

       ArmAttributes = TT_DESCRIPTOR_SECTION_TYPE_FAULT;\r
       ArmAttributes = TT_DESCRIPTOR_SECTION_TYPE_FAULT;\r

-      DEBUG ((EFI_D_PAGE, "SetMemoryAttributes(): Unsupported attribute %x will page fault on access\n", EfiAttributes));\r

       break;\r
   }\r
 \r
   // Determine protection attributes\r
       break;\r
   }\r
 \r
   // Determine protection attributes\r

-  if (EfiAttributes & EFI_MEMORY_WP) {\r
+  if (EfiAttributes & EFI_MEMORY_RO) {\r

     ArmAttributes |= TT_DESCRIPTOR_SECTION_AP_RO_RO;\r
   } else {\r
     ArmAttributes |= TT_DESCRIPTOR_SECTION_AP_RW_RW;\r
     ArmAttributes |= TT_DESCRIPTOR_SECTION_AP_RO_RO;\r
   } else {\r
     ArmAttributes |= TT_DESCRIPTOR_SECTION_AP_RW_RW;\r