Page and section entries in the page tables are updated using the
helper ArmUpdateTranslationTableEntry(), which cleans the page
table entry to the PoC, and invalidates the TLB entry covering
the page described by the entry being updated.

Since we may be updating section entries, we might be leaving stale
TLB entries at this point (for all pages in the section except the
first one), which will be invalidated wholesale at the end of
SetMemoryAttributes(). At that point, all caches are cleaned *and*
invalidated as well.

This cache maintenance is costly and unnecessary. The TLB maintenance
is only necessary if we updated any section entries, since any page
by page entries that have been updated will have been invalidated
individually by ArmUpdateTranslationTableEntry().

So drop the clean/invalidate of the caches, and only perform the
full TLB flush if UpdateSectionEntries() was called.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
index 046620f79185..fe0ddee447b0 100644
--- a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
+++ b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
@@ -680,6 +680,7 @@ SetMemoryAttributes (
 {
   EFI_STATUS    Status;
   UINT64        ChunkLength;
+  BOOLEAN       FlushTlbs;
 
   //
   // Ignore invocations that only modify permission bits
@@ -688,6 +689,7 @@ SetMemoryAttributes (
     return EFI_SUCCESS;
   }
 
+  FlushTlbs = FALSE;
   while (Length > 0) {
     if ((BaseAddress % TT_DESCRIPTOR_SECTION_SIZE == 0) &&
         Length >= TT_DESCRIPTOR_SECTION_SIZE) {
@@ -700,6 +702,8 @@ SetMemoryAttributes (
 
       Status = UpdateSectionEntries (BaseAddress, ChunkLength, Attributes,
                  VirtualMask);
+
+      FlushTlbs = TRUE;
     } else {
 
       //
@@ -728,16 +732,9 @@ SetMemoryAttributes (
     Length -= ChunkLength;
   }
 
-  // Flush d-cache so descriptors make it back to uncached memory for subsequent table walks
-  // flush and invalidate pages
-  //TODO: Do we really need to invalidate the caches everytime we change the memory attributes ?
-  ArmCleanInvalidateDataCache ();
-
-  ArmInvalidateInstructionCache ();
-
-  // Invalidate all TLB entries so changes are synced
-  ArmInvalidateTlb ();
-
+  if (FlushTlbs) {
+    ArmInvalidateTlb ();
+  }
   return Status;
 }
 
-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel