]> git.proxmox.com Git - mirror_edk2.git/blobdiff - ArmPkg/Drivers/CpuDxe/Mmu.c
ARM Packages: Fixed coding style and typos
[mirror_edk2.git] / ArmPkg / Drivers / CpuDxe / Mmu.c
index 9ba5c8344883258cccf20811226ecdd890a9a34a..dcc7b682c06bae25c2d9fd1a469879c525df0115 100644 (file)
@@ -15,8 +15,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 --*/\r
 \r
 #include "CpuDxe.h"\r
-//FIXME: Remove this ARMv7 specific header\r
-#include <Chipset/ArmV7.h>\r
 \r
 // First Level Descriptors\r
 typedef UINT32    ARM_FIRST_LEVEL_DESCRIPTOR;\r
@@ -88,6 +86,70 @@ SectionToGcdAttributes (
   return EFI_SUCCESS;\r
 }\r
 \r
+EFI_STATUS\r
+PageToGcdAttributes (\r
+  IN  UINT32  PageAttributes,\r
+  OUT UINT64  *GcdAttributes\r
+  )\r
+{\r
+  *GcdAttributes = 0;\r
+\r
+  // determine cacheability attributes\r
+  switch(PageAttributes & TT_DESCRIPTOR_PAGE_CACHE_POLICY_MASK) {\r
+    case TT_DESCRIPTOR_PAGE_CACHE_POLICY_STRONGLY_ORDERED:\r
+      *GcdAttributes |= EFI_MEMORY_UC;\r
+      break;\r
+    case TT_DESCRIPTOR_PAGE_CACHE_POLICY_SHAREABLE_DEVICE:\r
+      *GcdAttributes |= EFI_MEMORY_UC;\r
+      break;\r
+    case TT_DESCRIPTOR_PAGE_CACHE_POLICY_WRITE_THROUGH_NO_ALLOC:\r
+      *GcdAttributes |= EFI_MEMORY_WT;\r
+      break;\r
+    case TT_DESCRIPTOR_PAGE_CACHE_POLICY_WRITE_BACK_NO_ALLOC:\r
+      *GcdAttributes |= EFI_MEMORY_WB;\r
+      break;\r
+    case TT_DESCRIPTOR_PAGE_CACHE_POLICY_NON_CACHEABLE:\r
+      *GcdAttributes |= EFI_MEMORY_WC;\r
+      break;\r
+    case TT_DESCRIPTOR_PAGE_CACHE_POLICY_WRITE_BACK_ALLOC:\r
+      *GcdAttributes |= EFI_MEMORY_WB;\r
+      break;\r
+    case TT_DESCRIPTOR_PAGE_CACHE_POLICY_NON_SHAREABLE_DEVICE:\r
+      *GcdAttributes |= EFI_MEMORY_UC;\r
+      break;\r
+    default:\r
+      return EFI_UNSUPPORTED;\r
+  }\r
+\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
+      break;\r
+\r
+    case TT_DESCRIPTOR_PAGE_AP_RW_NO:\r
+    case TT_DESCRIPTOR_PAGE_AP_RW_RW:\r
+      // normal read/write access, do not add additional attributes\r
+      break;\r
+\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
+      break;\r
+\r
+    default:\r
+      return EFI_UNSUPPORTED;\r
+  }\r
+\r
+  // now process eXectue Never attribute\r
+  if ((PageAttributes & TT_DESCRIPTOR_PAGE_XN_MASK) != 0 ) {\r
+    *GcdAttributes |= EFI_MEMORY_XP;\r
+  }\r
+\r
+  return EFI_SUCCESS;\r
+}\r
+\r
 /**\r
   Searches memory descriptors covered by given memory range.\r
 \r
@@ -215,6 +277,81 @@ SetGcdMemorySpaceAttributes (
   return EFI_SUCCESS;\r
 }\r
 \r
+EFI_STATUS\r
+SyncCacheConfigPage (\r
+  IN     UINT32                             SectionIndex,\r
+  IN     UINT32                             FirstLevelDescriptor,\r
+  IN     UINTN                              NumberOfDescriptors,\r
+  IN     EFI_GCD_MEMORY_SPACE_DESCRIPTOR    *MemorySpaceMap,\r
+  IN OUT EFI_PHYSICAL_ADDRESS               *NextRegionBase,\r
+  IN OUT UINT64                             *NextRegionLength,\r
+  IN OUT UINT32                             *NextSectionAttributes\r
+  )\r
+{\r
+  EFI_STATUS                          Status;\r
+  UINT32                              i;\r
+  volatile ARM_PAGE_TABLE_ENTRY       *SecondLevelTable;\r
+  UINT32                              NextPageAttributes = 0;\r
+  UINT32                              PageAttributes = 0;\r
+  UINT32                              BaseAddress;\r
+  UINT64                              GcdAttributes;\r
+\r
+  // Get the Base Address from FirstLevelDescriptor;\r
+  BaseAddress = TT_DESCRIPTOR_PAGE_BASE_ADDRESS(SectionIndex << TT_DESCRIPTOR_SECTION_BASE_SHIFT);\r
+\r
+  // Convert SectionAttributes into PageAttributes\r
+  NextPageAttributes =\r
+      TT_DESCRIPTOR_CONVERT_TO_PAGE_CACHE_POLICY(*NextSectionAttributes,0) |\r
+      TT_DESCRIPTOR_CONVERT_TO_PAGE_AP(*NextSectionAttributes);\r
+\r
+  // obtain page table base\r
+  SecondLevelTable = (ARM_PAGE_TABLE_ENTRY *)(FirstLevelDescriptor & TT_DESCRIPTOR_SECTION_PAGETABLE_ADDRESS_MASK);\r
+\r
+  for (i=0; i < TRANSLATION_TABLE_PAGE_COUNT; i++) {\r
+    if ((SecondLevelTable[i] & TT_DESCRIPTOR_PAGE_TYPE_MASK) == TT_DESCRIPTOR_PAGE_TYPE_PAGE) {\r
+      // extract attributes (cacheability and permissions)\r
+      PageAttributes = SecondLevelTable[i] & (TT_DESCRIPTOR_PAGE_CACHE_POLICY_MASK | TT_DESCRIPTOR_PAGE_AP_MASK);\r
+\r
+      if (NextPageAttributes == 0) {\r
+        // start on a new region\r
+        *NextRegionLength = 0;\r
+        *NextRegionBase = BaseAddress | (i << TT_DESCRIPTOR_PAGE_BASE_SHIFT);\r
+        NextPageAttributes = PageAttributes;\r
+      } else if (PageAttributes != NextPageAttributes) {\r
+        // Convert Section Attributes into GCD Attributes\r
+        Status = PageToGcdAttributes (NextPageAttributes, &GcdAttributes);\r
+        ASSERT_EFI_ERROR (Status);\r
+\r
+        // update GCD with these changes (this will recurse into our own CpuSetMemoryAttributes below which is OK)\r
+        SetGcdMemorySpaceAttributes (MemorySpaceMap, NumberOfDescriptors, *NextRegionBase, *NextRegionLength, GcdAttributes);\r
+\r
+        // start on a new region\r
+        *NextRegionLength = 0;\r
+        *NextRegionBase = BaseAddress | (i << TT_DESCRIPTOR_PAGE_BASE_SHIFT);\r
+        NextPageAttributes = PageAttributes;\r
+      }\r
+    } else if (NextPageAttributes != 0) {\r
+      // Convert Page Attributes into GCD Attributes\r
+      Status = PageToGcdAttributes (NextPageAttributes, &GcdAttributes);\r
+      ASSERT_EFI_ERROR (Status);\r
+\r
+      // update GCD with these changes (this will recurse into our own CpuSetMemoryAttributes below which is OK)\r
+      SetGcdMemorySpaceAttributes (MemorySpaceMap, NumberOfDescriptors, *NextRegionBase, *NextRegionLength, GcdAttributes);\r
+\r
+      *NextRegionLength = 0;\r
+      *NextRegionBase = BaseAddress | (i << TT_DESCRIPTOR_PAGE_BASE_SHIFT);\r
+      NextPageAttributes = 0;\r
+    }\r
+    *NextRegionLength += TT_DESCRIPTOR_PAGE_SIZE;\r
+  }\r
+\r
+  // Convert back PageAttributes into SectionAttributes\r
+  *NextSectionAttributes =\r
+      TT_DESCRIPTOR_CONVERT_TO_SECTION_CACHE_POLICY(NextPageAttributes,0) |\r
+      TT_DESCRIPTOR_CONVERT_TO_SECTION_AP(NextPageAttributes);\r
+\r
+  return EFI_SUCCESS;\r
+}\r
 \r
 EFI_STATUS\r
 SyncCacheConfig (\r
@@ -223,12 +360,11 @@ SyncCacheConfig (
 {\r
   EFI_STATUS                          Status;\r
   UINT32                              i;\r
-  UINT32                              Descriptor;\r
-  UINT32                              SectionAttributes;\r
   EFI_PHYSICAL_ADDRESS                NextRegionBase;\r
   UINT64                              NextRegionLength;\r
+  UINT32                              NextSectionAttributes = 0;\r
+  UINT32                              SectionAttributes = 0;\r
   UINT64                              GcdAttributes;\r
-  UINT32                              NextRegionAttributes = 0;\r
   volatile ARM_FIRST_LEVEL_DESCRIPTOR   *FirstLevelTable;\r
   UINTN                               NumberOfDescriptors;\r
   EFI_GCD_MEMORY_SPACE_DESCRIPTOR     *MemorySpaceMap;\r
@@ -256,46 +392,71 @@ SyncCacheConfig (
   // obtain page table base\r
   FirstLevelTable = (ARM_FIRST_LEVEL_DESCRIPTOR *)(ArmGetTTBR0BaseAddress ());\r
 \r
+  // Get the first region\r
+  NextSectionAttributes = FirstLevelTable[0] & (TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK | TT_DESCRIPTOR_SECTION_AP_MASK);\r
 \r
   // iterate through each 1MB descriptor\r
   NextRegionBase = NextRegionLength = 0;\r
-  for (i=0; i< TRANSLATION_TABLE_SECTION_COUNT; i++) {\r
-\r
-    // obtain existing descriptor and make sure it contains a valid Base Address even if it is a fault section\r
-    Descriptor = FirstLevelTable[i] | TT_DESCRIPTOR_SECTION_BASE_ADDRESS(i << TT_DESCRIPTOR_SECTION_BASE_SHIFT);\r
-\r
-    // extract attributes (cacheability and permissions)\r
-    SectionAttributes = Descriptor & (TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK | TT_DESCRIPTOR_SECTION_AP_MASK);\r
-\r
-    // do we already have an existing region (or are we about to finish)?\r
-    // Skip the first entry, and make sure we close on the last entry\r
-    if ( (NextRegionLength > 0) || (i == (TRANSLATION_TABLE_SECTION_COUNT-1)) ) {\r
-      // attributes are changing, update attributes in GCD\r
-      if (SectionAttributes != NextRegionAttributes) {\r
-        \r
-        // convert section entry attributes to GCD bitmask\r
-        Status = SectionToGcdAttributes (NextRegionAttributes, &GcdAttributes);\r
+  for (i=0; i < TRANSLATION_TABLE_SECTION_COUNT; i++) {\r
+    if ((FirstLevelTable[i] & TT_DESCRIPTOR_SECTION_TYPE_MASK) == TT_DESCRIPTOR_SECTION_TYPE_SECTION) {\r
+      // extract attributes (cacheability and permissions)\r
+      SectionAttributes = FirstLevelTable[i] & (TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK | TT_DESCRIPTOR_SECTION_AP_MASK);\r
+\r
+      if (NextSectionAttributes == 0) {\r
+        // start on a new region\r
+        NextRegionLength = 0;\r
+        NextRegionBase = TT_DESCRIPTOR_SECTION_BASE_ADDRESS(i << TT_DESCRIPTOR_SECTION_BASE_SHIFT);\r
+        NextSectionAttributes = SectionAttributes;\r
+      } else if (SectionAttributes != NextSectionAttributes) {\r
+        // Convert Section Attributes into GCD Attributes\r
+        Status = SectionToGcdAttributes (NextSectionAttributes, &GcdAttributes);\r
         ASSERT_EFI_ERROR (Status);\r
 \r
         // update GCD with these changes (this will recurse into our own CpuSetMemoryAttributes below which is OK)\r
         SetGcdMemorySpaceAttributes (MemorySpaceMap, NumberOfDescriptors, NextRegionBase, NextRegionLength, GcdAttributes);\r
 \r
-\r
         // start on a new region\r
         NextRegionLength = 0;\r
-        NextRegionBase = TT_DESCRIPTOR_SECTION_BASE_ADDRESS(Descriptor);\r
+        NextRegionBase = TT_DESCRIPTOR_SECTION_BASE_ADDRESS(i << TT_DESCRIPTOR_SECTION_BASE_SHIFT);\r
+        NextSectionAttributes = SectionAttributes;\r
       }\r
-    }\r
+      NextRegionLength += TT_DESCRIPTOR_SECTION_SIZE;\r
+    } else if (TT_DESCRIPTOR_SECTION_TYPE_IS_PAGE_TABLE(FirstLevelTable[i])) {\r
+      Status = SyncCacheConfigPage (\r
+          i,FirstLevelTable[i],\r
+          NumberOfDescriptors, MemorySpaceMap,\r
+          &NextRegionBase,&NextRegionLength,&NextSectionAttributes);\r
+      ASSERT_EFI_ERROR (Status);\r
+    } else {\r
+      // We do not support yet 16MB sections\r
+      ASSERT ((FirstLevelTable[i] & TT_DESCRIPTOR_SECTION_TYPE_MASK) != TT_DESCRIPTOR_SECTION_TYPE_SUPERSECTION);\r
 \r
-    // starting a new region?\r
-    if (NextRegionLength == 0) {\r
-      NextRegionAttributes = SectionAttributes;\r
-    }\r
+      // start on a new region\r
+      if (NextSectionAttributes != 0) {\r
+        // Convert Section Attributes into GCD Attributes\r
+        Status = SectionToGcdAttributes (NextSectionAttributes, &GcdAttributes);\r
+        ASSERT_EFI_ERROR (Status);\r
 \r
-    NextRegionLength += TT_DESCRIPTOR_SECTION_SIZE;\r
+        // update GCD with these changes (this will recurse into our own CpuSetMemoryAttributes below which is OK)\r
+        SetGcdMemorySpaceAttributes (MemorySpaceMap, NumberOfDescriptors, NextRegionBase, NextRegionLength, GcdAttributes);\r
 \r
+        NextRegionLength = 0;\r
+        NextRegionBase = TT_DESCRIPTOR_SECTION_BASE_ADDRESS(i << TT_DESCRIPTOR_SECTION_BASE_SHIFT);\r
+        NextSectionAttributes = 0;\r
+      }\r
+      NextRegionLength += TT_DESCRIPTOR_SECTION_SIZE;\r
+    }\r
   } // section entry loop\r
 \r
+  if (NextSectionAttributes != 0) {\r
+    // Convert Section Attributes into GCD Attributes\r
+    Status = SectionToGcdAttributes (NextSectionAttributes, &GcdAttributes);\r
+    ASSERT_EFI_ERROR (Status);\r
+\r
+    // update GCD with these changes (this will recurse into our own CpuSetMemoryAttributes below which is OK)\r
+    SetGcdMemorySpaceAttributes (MemorySpaceMap, NumberOfDescriptors, NextRegionBase, NextRegionLength, GcdAttributes);\r
+  }\r
+\r
   return EFI_SUCCESS;\r
 }\r
 \r
@@ -337,13 +498,8 @@ UpdatePageEntries (
     case EFI_MEMORY_UC:\r
       // modify cacheability attributes\r
       EntryMask |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_MASK;\r
-      if (FeaturePcdGet(PcdEfiUncachedMemoryToStronglyOrdered)) {\r
-        // map to strongly ordered\r
-        EntryValue |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_STRONGLY_ORDERED; // TEX[2:0] = 0, C=0, B=0\r
-      } else {\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
-      }\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
@@ -380,49 +536,49 @@ UpdatePageEntries (
       return EFI_UNSUPPORTED;\r
   }\r
 \r
-  // obtain page table base\r
+  // Obtain page table base\r
   FirstLevelTable = (ARM_FIRST_LEVEL_DESCRIPTOR *)ArmGetTTBR0BaseAddress ();\r
 \r
-  // calculate number of 4KB page table entries to change\r
-  NumPageEntries = Length/SIZE_4KB;\r
+  // Calculate number of 4KB page table entries to change\r
+  NumPageEntries = Length / TT_DESCRIPTOR_PAGE_SIZE;\r
   \r
-  // iterate for the number of 4KB pages to change\r
+  // Iterate for the number of 4KB pages to change\r
   Offset = 0;\r
-  for(p=0; p<NumPageEntries; p++) {\r
-    // calculate index into first level translation table for page table value\r
+  for(p = 0; p < NumPageEntries; p++) {\r
+    // Calculate index into first level translation table for page table value\r
     \r
     FirstLevelIdx = TT_DESCRIPTOR_SECTION_BASE_ADDRESS(BaseAddress + Offset) >> TT_DESCRIPTOR_SECTION_BASE_SHIFT;\r
     ASSERT (FirstLevelIdx < TRANSLATION_TABLE_SECTION_COUNT);\r
 \r
-    // read the descriptor from the first level page table\r
+    // Read the descriptor from the first level page table\r
     Descriptor = FirstLevelTable[FirstLevelIdx];\r
 \r
-    // does this descriptor need to be converted from section entry to 4K pages?\r
+    // Does this descriptor need to be converted from section entry to 4K pages?\r
     if (!TT_DESCRIPTOR_SECTION_TYPE_IS_PAGE_TABLE(Descriptor)) {\r
       Status = ConvertSectionToPages (FirstLevelIdx << TT_DESCRIPTOR_SECTION_BASE_SHIFT);\r
       if (EFI_ERROR(Status)) {\r
-        // exit for loop\r
+        // Exit for loop\r
         break; \r
       } \r
       \r
-      // re-read descriptor\r
+      // Re-read descriptor\r
       Descriptor = FirstLevelTable[FirstLevelIdx];\r
     }\r
 \r
-    // obtain page table base address\r
+    // Obtain page table base address\r
     PageTable = (ARM_PAGE_TABLE_ENTRY *)TT_DESCRIPTOR_PAGE_BASE_ADDRESS(Descriptor);\r
 \r
-    // calculate index into the page table\r
+    // Calculate index into the page table\r
     PageTableIndex = ((BaseAddress + Offset) & TT_DESCRIPTOR_PAGE_INDEX_MASK) >> TT_DESCRIPTOR_PAGE_BASE_SHIFT;\r
     ASSERT (PageTableIndex < TRANSLATION_TABLE_PAGE_COUNT);\r
 \r
-    // get the entry\r
+    // Get the entry\r
     CurrentPageTableEntry = PageTable[PageTableIndex];\r
 \r
-    // mask off appropriate fields\r
+    // Mask off appropriate fields\r
     PageTableEntry = CurrentPageTableEntry & ~EntryMask;\r
 \r
-    // mask in new attributes and/or permissions\r
+    // Mask in new attributes and/or permissions\r
     PageTableEntry |= EntryValue;\r
 \r
     if (VirtualMask != 0) {\r
@@ -435,7 +591,7 @@ UpdatePageEntries (
       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, SIZE_4KB);\r
+        WriteBackInvalidateDataCacheRange (Mva, TT_DESCRIPTOR_PAGE_SIZE);\r
       }\r
 \r
       // Only need to update if we are changing the entry  \r
@@ -444,9 +600,9 @@ UpdatePageEntries (
     }\r
 \r
     Status = EFI_SUCCESS;\r
-    Offset += SIZE_4KB;\r
+    Offset += TT_DESCRIPTOR_PAGE_SIZE;\r
     \r
-  } // end first level translation table loop\r
+  } // End first level translation table loop\r
 \r
   return Status;\r
 }\r
@@ -485,13 +641,8 @@ UpdateSectionEntries (
     case EFI_MEMORY_UC:\r
       // modify cacheability attributes\r
       EntryMask |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK;\r
-      if (FeaturePcdGet(PcdEfiUncachedMemoryToStronglyOrdered)) {\r
-        // map to strongly ordered\r
-        EntryValue |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_STRONGLY_ORDERED; // TEX[2:0] = 0, C=0, B=0\r
-      } else {\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
-      }\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
@@ -591,21 +742,21 @@ ConvertSectionToPages (
   UINT32                  SectionDescriptor;\r
   UINT32                  PageTableDescriptor;\r
   UINT32                  PageDescriptor;\r
-  UINT32                  i;\r
+  UINT32                  Index;\r
 \r
   volatile ARM_FIRST_LEVEL_DESCRIPTOR   *FirstLevelTable;\r
   volatile ARM_PAGE_TABLE_ENTRY         *PageTable;\r
 \r
   DEBUG ((EFI_D_PAGE, "Converting section at 0x%x to pages\n", (UINTN)BaseAddress));\r
 \r
-  // obtain page table base\r
+  // Obtain page table base\r
   FirstLevelTable = (ARM_FIRST_LEVEL_DESCRIPTOR *)ArmGetTTBR0BaseAddress ();\r
 \r
-  // calculate index into first level translation table for start of modification\r
+  // Calculate index into first level translation table for start of modification\r
   FirstLevelIdx = TT_DESCRIPTOR_SECTION_BASE_ADDRESS(BaseAddress) >> TT_DESCRIPTOR_SECTION_BASE_SHIFT;\r
   ASSERT (FirstLevelIdx < TRANSLATION_TABLE_SECTION_COUNT);\r
 \r
-  // get section attributes and convert to page attributes\r
+  // Get section attributes and convert to page attributes\r
   SectionDescriptor = FirstLevelTable[FirstLevelIdx];\r
   PageDescriptor = TT_DESCRIPTOR_PAGE_TYPE_PAGE;\r
   PageDescriptor |= TT_DESCRIPTOR_CONVERT_TO_PAGE_CACHE_POLICY(SectionDescriptor,0);\r
@@ -614,7 +765,7 @@ ConvertSectionToPages (
   PageDescriptor |= TT_DESCRIPTOR_CONVERT_TO_PAGE_NG(SectionDescriptor);\r
   PageDescriptor |= TT_DESCRIPTOR_CONVERT_TO_PAGE_S(SectionDescriptor);\r
 \r
-  // allocate a page table for the 4KB entries (we use up a full page even though we only need 1KB)\r
+  // Allocate a page table for the 4KB entries (we use up a full page even though we only need 1KB)\r
   Status = gBS->AllocatePages (AllocateAnyPages, EfiBootServicesData, 1, &PageTableAddr);\r
   if (EFI_ERROR(Status)) {\r
     return Status;\r
@@ -622,18 +773,18 @@ ConvertSectionToPages (
 \r
   PageTable = (volatile ARM_PAGE_TABLE_ENTRY *)(UINTN)PageTableAddr;\r
 \r
-  // write the page table entries out\r
-  for (i=0; i < TRANSLATION_TABLE_PAGE_COUNT; i++) {\r
-    PageTable[i] = TT_DESCRIPTOR_PAGE_BASE_ADDRESS(BaseAddress + (i << 12)) | PageDescriptor;\r
+  // Write the page table entries out\r
+  for (Index = 0; Index < TRANSLATION_TABLE_PAGE_COUNT; Index++) {\r
+    PageTable[Index] = TT_DESCRIPTOR_PAGE_BASE_ADDRESS(BaseAddress + (Index << 12)) | PageDescriptor;\r
   }\r
 \r
-  // flush d-cache so descriptors make it back to uncached memory for subsequent table walks\r
-  WriteBackInvalidateDataCacheRange ((VOID *)(UINTN)PageTableAddr, SIZE_4KB);\r
+  // Flush d-cache so descriptors make it back to uncached memory for subsequent table walks\r
+  WriteBackInvalidateDataCacheRange ((VOID *)(UINTN)PageTableAddr, TT_DESCRIPTOR_PAGE_SIZE);\r
 \r
-  // formulate page table entry, Domain=0, NS=0\r
+  // Formulate page table entry, Domain=0, NS=0\r
   PageTableDescriptor = (((UINTN)PageTableAddr) & TT_DESCRIPTOR_SECTION_PAGETABLE_ADDRESS_MASK) | TT_DESCRIPTOR_SECTION_TYPE_PAGE_TABLE;\r
 \r
-  // write the page table entry out, repalcing section entry\r
+  // Write the page table entry out, replacing section entry\r
   FirstLevelTable[FirstLevelIdx] = PageTableDescriptor;\r
 \r
   return EFI_SUCCESS;\r
@@ -652,23 +803,24 @@ SetMemoryAttributes (
   EFI_STATUS    Status;\r
   \r
   if(((BaseAddress & 0xFFFFF) == 0) && ((Length & 0xFFFFF) == 0)) {\r
-    // is the base and length a multiple of 1 MB?\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
+    // 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
   }\r
 \r
-  // flush d-cache so descriptors make it back to uncached memory for subsequent table walks\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
-  \r
+\r
   ArmInvalidateInstructionCache ();\r
 \r
-  // invalidate all TLB entries so changes are synced\r
-  ArmInvalidateTlb (); \r
+  // Invalidate all TLB entries so changes are synced\r
+  ArmInvalidateTlb ();\r
 \r
   return Status;\r
 }\r
@@ -705,7 +857,7 @@ CpuSetMemoryAttributes (
   )\r
 {\r
   DEBUG ((EFI_D_PAGE, "SetMemoryAttributes(%lx, %lx, %lx)\n", BaseAddress, Length, Attributes));\r
-  if ( ((BaseAddress & (SIZE_4KB-1)) != 0) || ((Length & (SIZE_4KB-1)) != 0)){\r
+  if ( ((BaseAddress & ~TT_DESCRIPTOR_PAGE_BASE_ADDRESS_MASK) != 0) || ((Length & ~TT_DESCRIPTOR_PAGE_BASE_ADDRESS_MASK) != 0)){\r
     // minimum granularity is SIZE_4KB (4KB on ARM)\r
     DEBUG ((EFI_D_PAGE, "SetMemoryAttributes(%lx, %lx, %lx): minimum ganularity is SIZE_4KB\n", BaseAddress, Length, Attributes));\r
     return EFI_UNSUPPORTED;\r