[PATCH 0/2] ArmPkg/ArmMmuLib ARM: some cleanups

[PATCH 0/2] ArmPkg/ArmMmuLib ARM: some cleanups

[Ard Biesheuvel]

A pair of cleanups for issues that I ran into while working on the
set/way cache maintenance stuff.

Ard Biesheuvel (2):
  ArmPkg/ArmMmuLib ARM: simplify assignment of TTBR0 system register
  ArmPkg/ArmMmuLib ARM: drop memory type check for page tables

 ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 22 +++-----------------
 1 file changed, 3 insertions(+), 19 deletions(-)

-- 
2.17.1

[PATCH 1/2] ArmPkg/ArmMmuLib ARM: simplify assignment of TTBR0 system register

[Ard Biesheuvel]

The expression passed into ArmSetTTBR0 () in ArmConfigureMmu() is
sub-optimal at several levels:
- TranslationTable is already aligned, and if it wasn't, doing it
  here wouldn't help
- TTBRAttributes is guaranteed not to have any bits set outside of
  the 0x7f mask, so the mask operation is pointless as well,
- an additional (UINTN) cast for good measure is also not needed.

So simplify the expression.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
index 11a1e704beab..a6f44dbd5f21 100644
--- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
@@ -392,7 +392,7 @@ ArmConfigureMmu (
     }
   }
 
-  ArmSetTTBR0 ((VOID *)(UINTN)(((UINTN)TranslationTable & ~TRANSLATION_TABLE_SECTION_ALIGNMENT_MASK) | (TTBRAttributes & 0x7F)));
+  ArmSetTTBR0 ((VOID *)((UINTN)TranslationTable | TTBRAttributes));
 
   //
   // The TTBCR register value is undefined at reset in the Non-Secure world.
-- 
2.17.1

[PATCH 2/2] ArmPkg/ArmMmuLib ARM: drop memory type check for page tables

[Ard Biesheuvel]

We already expect normal memory to be mapped writeback cacheable if
EDK2 itself is to make use of it, so doing an early sanity check on
the memory type of the allocation that the page tables happened to
land in isn't very useful. So let's drop it.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 20 ++------------------
 1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
index a6f44dbd5f21..15e836e75e8e 100644
--- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
@@ -328,7 +328,6 @@ ArmConfigureMmu (
   )
 {
   VOID                          *TranslationTable;
-  ARM_MEMORY_REGION_ATTRIBUTES  TranslationTableAttribute;
   UINT32                        TTBRAttributes;
 
   TranslationTable = AllocateAlignedPages (
@@ -353,28 +352,13 @@ ArmConfigureMmu (
   InvalidateDataCacheRange (TranslationTable, TRANSLATION_TABLE_SECTION_SIZE);
   ZeroMem (TranslationTable, TRANSLATION_TABLE_SECTION_SIZE);
 
-  // By default, mark the translation table as belonging to a uncached region
-  TranslationTableAttribute = ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
   while (MemoryTable->Length != 0) {
-    // Find the memory attribute for the Translation Table
-    if (((UINTN)TranslationTable >= MemoryTable->PhysicalBase) && ((UINTN)TranslationTable <= MemoryTable->PhysicalBase - 1 + MemoryTable->Length)) {
-      TranslationTableAttribute = MemoryTable->Attributes;
-    }
-
     FillTranslationTable (TranslationTable, MemoryTable);
     MemoryTable++;
   }
 
-  // Translate the Memory Attributes into Translation Table Register Attributes
-  if ((TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK) ||
-      (TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK)) {
-    TTBRAttributes = ArmHasMpExtensions () ? TTBR_MP_WRITE_BACK_ALLOC : TTBR_WRITE_BACK_ALLOC;
-  } else {
-    // Page tables must reside in memory mapped as write-back cacheable
-    ASSERT (0);
-    return RETURN_UNSUPPORTED;
-  }
-
+  TTBRAttributes = ArmHasMpExtensions () ? TTBR_MP_WRITE_BACK_ALLOC
+                                         : TTBR_WRITE_BACK_ALLOC;
   if (TTBRAttributes & TTBR_SHAREABLE) {
     if (PreferNonshareableMemory ()) {
       TTBRAttributes ^= TTBR_SHAREABLE;
-- 
2.17.1

Re: [PATCH 0/2] ArmPkg/ArmMmuLib ARM: some cleanups

[Leif Lindholm]

On Thu, Mar 05, 2020 at 13:59:05 +0100, Ard Biesheuvel wrote:
> A pair of cleanups for issues that I ran into while working on the
> set/way cache maintenance stuff.

For the series:
Reviewed-by: Leif Lindholm <leif@nuviainc.com>


> Ard Biesheuvel (2):
>   ArmPkg/ArmMmuLib ARM: simplify assignment of TTBR0 system register
>   ArmPkg/ArmMmuLib ARM: drop memory type check for page tables
> 
>  ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 22 +++-----------------
>  1 file changed, 3 insertions(+), 19 deletions(-)
> 
> -- 
> 2.17.1
>