public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ard Biesheuvel" <ardb@kernel.org>
To: devel@edk2.groups.io
Cc: quic_llindhol@quicinc.com, sami.mujawar@arm.com,
	Ard Biesheuvel <ardb@kernel.org>, Marc Zyngier <maz@kernel.org>,
	Alexander Graf <graf@amazon.com>
Subject: [PATCH 2/7] ArmPkg/ArmMmuLib: use shadow page tables for break-before-make at EL1
Date: Mon,  4 Jul 2022 19:52:47 +0200	[thread overview]
Message-ID: <20220704175252.4015120-3-ardb@kernel.org> (raw)
In-Reply-To: <20220704175252.4015120-1-ardb@kernel.org>

When executing at EL1, disabling and re-enabling the MMU every time we
need to replace a live translation entry is slightly problematic, given
that memory accesses performed with the MMU off have non-cacheable
attributes and are therefore non-coherent. On bare metal, we can deal
with this by adding some barriers and cache invalidation instructions,
but when running under virtualization, elaborate trapping and cache
maintenance logic is necessary on the part of the hypervisor, and this
is better avoided.

So let's switch to a different approach when running at EL1, and use
two sets of page tables with different ASIDs, and non-global attributes
for all mappings. This allows us to switch between those sets without
having to care about break-before-make, which means we can manipulate
the primary translation while running from the secondary.

To avoid splitting block mappings unnecessarily in the shadow page
tables, add a special case to the recursive mapping routines to retain
a block mapping that already covers a region that we are trying to map,
and has the right attributes.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c                      |   3 +
 ArmPkg/Include/Chipset/AArch64Mmu.h                      |   1 +
 ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c         | 112 ++++++++++++++------
 ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S |  15 ++-
 4 files changed, 96 insertions(+), 35 deletions(-)

diff --git a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
index 8bb33046e707..1ccd9f7fece8 100644
--- a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
+++ b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
@@ -313,6 +313,9 @@ EfiAttributeToArmAttribute (
     ArmAttributes |= TT_PXN_MASK;
   }
 
+  if (ArmReadCurrentEL () == AARCH64_EL1) {
+    ArmAttributes |= TT_NG;
+  }
   return ArmAttributes;
 }
 
diff --git a/ArmPkg/Include/Chipset/AArch64Mmu.h b/ArmPkg/Include/Chipset/AArch64Mmu.h
index 2ea2cc0a874d..763dc53908e2 100644
--- a/ArmPkg/Include/Chipset/AArch64Mmu.h
+++ b/ArmPkg/Include/Chipset/AArch64Mmu.h
@@ -67,6 +67,7 @@
 
 #define TT_NS  BIT5
 #define TT_AF  BIT10
+#define TT_NG  BIT11
 
 #define TT_SH_NON_SHAREABLE    (0x0 << 8)
 #define TT_SH_OUTER_SHAREABLE  (0x2 << 8)
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index b85ae5a30768..d5c19600dce8 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -25,23 +25,31 @@ ArmMemoryAttributeToPageAttribute (
   IN ARM_MEMORY_REGION_ATTRIBUTES  Attributes
   )
 {
+  UINT64  NonGlobal;
+
+  if (ArmReadCurrentEL () == AARCH64_EL1) {
+    NonGlobal = TT_NG;
+  } else {
+    NonGlobal = 0;
+  }
+
   switch (Attributes) {
     case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHAREABLE:
     case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHAREABLE:
-      return TT_ATTR_INDX_MEMORY_WRITE_BACK;
+      return TT_ATTR_INDX_MEMORY_WRITE_BACK | NonGlobal;
 
     case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK:
     case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK:
-      return TT_ATTR_INDX_MEMORY_WRITE_BACK | TT_SH_INNER_SHAREABLE;
+      return TT_ATTR_INDX_MEMORY_WRITE_BACK | TT_SH_INNER_SHAREABLE | NonGlobal;
 
     case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH:
     case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_THROUGH:
-      return TT_ATTR_INDX_MEMORY_WRITE_THROUGH | TT_SH_INNER_SHAREABLE;
+      return TT_ATTR_INDX_MEMORY_WRITE_THROUGH | TT_SH_INNER_SHAREABLE | NonGlobal;
 
     // Uncached and device mappings are treated as outer shareable by default,
     case ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED:
     case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_UNCACHED_UNBUFFERED:
-      return TT_ATTR_INDX_MEMORY_NON_CACHEABLE;
+      return TT_ATTR_INDX_MEMORY_NON_CACHEABLE | NonGlobal;
 
     default:
       ASSERT (0);
@@ -50,7 +58,7 @@ ArmMemoryAttributeToPageAttribute (
       if (ArmReadCurrentEL () == AARCH64_EL2) {
         return TT_ATTR_INDX_DEVICE_MEMORY | TT_XN_MASK;
       } else {
-        return TT_ATTR_INDX_DEVICE_MEMORY | TT_UXN_MASK | TT_PXN_MASK;
+        return TT_ATTR_INDX_DEVICE_MEMORY | TT_UXN_MASK | TT_PXN_MASK | TT_NG;
       }
   }
 }
@@ -203,7 +211,13 @@ UpdateRegionMappingRecursive (
     {
       ASSERT (Level < 3);
 
-      if (!IsTableEntry (*Entry, Level)) {
+      if (IsBlockEntry (*Entry, Level) && AttributeClearMask == 0 &&
+          ((*Entry & TT_ATTRIBUTES_MASK) ^ AttributeSetMask) == 0) {
+        // The existing block entry already covers the region we are
+        // trying to map with the correct attributes so no need to do
+        // anything here
+        continue;
+      } else if (!IsTableEntry (*Entry, Level)) {
         //
         // No table entry exists yet, so we need to allocate a page table
         // for the next level.
@@ -304,7 +318,8 @@ UpdateRegionMapping (
   IN  UINT64  RegionStart,
   IN  UINT64  RegionLength,
   IN  UINT64  AttributeSetMask,
-  IN  UINT64  AttributeClearMask
+  IN  UINT64  AttributeClearMask,
+  IN  UINT64  *RootTable
   )
 {
   UINTN  T0SZ;
@@ -320,7 +335,7 @@ UpdateRegionMapping (
            RegionStart + RegionLength,
            AttributeSetMask,
            AttributeClearMask,
-           ArmGetTTBR0BaseAddress (),
+           RootTable,
            GetRootTableLevel (T0SZ)
            );
 }
@@ -336,7 +351,7 @@ FillTranslationTable (
            MemoryRegion->VirtualBase,
            MemoryRegion->Length,
            ArmMemoryAttributeToPageAttribute (MemoryRegion->Attributes) | TT_AF,
-           0
+           0, RootTable
            );
 }
 
@@ -380,6 +395,10 @@ GcdAttributeToPageAttribute (
     PageAttributes |= TT_AP_NO_RO;
   }
 
+  if (ArmReadCurrentEL () == AARCH64_EL1) {
+    PageAttributes |= TT_NG;
+  }
+
   return PageAttributes | TT_AF;
 }
 
@@ -390,8 +409,9 @@ ArmSetMemoryAttributes (
   IN UINT64                Attributes
   )
 {
-  UINT64  PageAttributes;
-  UINT64  PageAttributeMask;
+  UINT64       PageAttributes;
+  UINT64       PageAttributeMask;
+  EFI_STATUS   Status;
 
   PageAttributes    = GcdAttributeToPageAttribute (Attributes);
   PageAttributeMask = 0;
@@ -404,13 +424,22 @@ ArmSetMemoryAttributes (
     PageAttributes   &= TT_AP_MASK | TT_UXN_MASK | TT_PXN_MASK;
     PageAttributeMask = ~(TT_ADDRESS_MASK_BLOCK_ENTRY | TT_AP_MASK |
                           TT_PXN_MASK | TT_XN_MASK);
+  } else if (ArmReadCurrentEL () == AARCH64_EL1) {
+    // Update the shadow page tables
+    Status = UpdateRegionMapping (BaseAddress, Length,
+               (PageAttributes & ~(TT_AP_MASK | TT_PXN_MASK | TT_UXN_MASK)) | TT_AP_NO_RW,
+               0, ArmGetTTBR0BaseAddress () + EFI_PAGE_SIZE);
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
   }
 
   return UpdateRegionMapping (
            BaseAddress,
            Length,
            PageAttributes,
-           PageAttributeMask
+           PageAttributeMask,
+           ArmGetTTBR0BaseAddress ()
            );
 }
 
@@ -423,7 +452,8 @@ SetMemoryRegionAttribute (
   IN  UINT64                BlockEntryMask
   )
 {
-  return UpdateRegionMapping (BaseAddress, Length, Attributes, BlockEntryMask);
+  return UpdateRegionMapping (BaseAddress, Length, Attributes,
+           BlockEntryMask, ArmGetTTBR0BaseAddress ());
 }
 
 EFI_STATUS
@@ -503,13 +533,15 @@ ArmConfigureMmu (
   OUT UINTN                         *TranslationTableSize OPTIONAL
   )
 {
-  VOID        *TranslationTable;
-  UINTN       MaxAddressBits;
-  UINT64      MaxAddress;
-  UINTN       T0SZ;
-  UINTN       RootTableEntryCount;
-  UINT64      TCR;
-  EFI_STATUS  Status;
+  UINT64                        *TranslationTable;
+  UINTN                         MaxAddressBits;
+  UINT64                        MaxAddress;
+  UINTN                         T0SZ;
+  UINTN                         RootTableEntryCount;
+  UINT64                        TCR;
+  EFI_STATUS                    Status;
+  ARM_MEMORY_REGION_DESCRIPTOR  *MemTab;
+  UINTN                         NumRootPages;
 
   if (MemoryTable == NULL) {
     ASSERT (MemoryTable != NULL);
@@ -538,6 +570,8 @@ ArmConfigureMmu (
     // Note: Bits 23 and 31 are reserved(RES1) bits in TCR_EL2
     TCR = T0SZ | (1UL << 31) | (1UL << 23) | TCR_TG0_4KB;
 
+    NumRootPages = 1;
+
     // Set the Physical Address Size using MaxAddress
     if (MaxAddress < SIZE_4GB) {
       TCR |= TCR_PS_4GB;
@@ -564,6 +598,8 @@ ArmConfigureMmu (
     // Due to Cortex-A57 erratum #822227 we must set TG1[1] == 1, regardless of EPD1.
     TCR = T0SZ | TCR_TG0_4KB | TCR_TG1_4KB | TCR_EPD1;
 
+    NumRootPages = 2;
+
     // Set the Physical Address Size using MaxAddress
     if (MaxAddress < SIZE_4GB) {
       TCR |= TCR_IPS_4GB;
@@ -608,19 +644,11 @@ ArmConfigureMmu (
   ArmSetTCR (TCR);
 
   // Allocate pages for translation table
-  TranslationTable = AllocatePages (1);
+  TranslationTable = AllocatePages (NumRootPages);
   if (TranslationTable == NULL) {
     return EFI_OUT_OF_RESOURCES;
   }
 
-  //
-  // We set TTBR0 just after allocating the table to retrieve its location from
-  // the subsequent functions without needing to pass this value across the
-  // functions. The MMU is only enabled after the translation tables are
-  // populated.
-  //
-  ArmSetTTBR0 (TranslationTable);
-
   if (TranslationTableBase != NULL) {
     *TranslationTableBase = TranslationTable;
   }
@@ -639,13 +667,11 @@ ArmConfigureMmu (
     );
   ZeroMem (TranslationTable, RootTableEntryCount * sizeof (UINT64));
 
-  while (MemoryTable->Length != 0) {
-    Status = FillTranslationTable (TranslationTable, MemoryTable);
+  for (MemTab = MemoryTable; MemTab->Length != 0; MemTab++) {
+    Status = FillTranslationTable (TranslationTable, MemTab);
     if (EFI_ERROR (Status)) {
       goto FreeTranslationTable;
     }
-
-    MemoryTable++;
   }
 
   //
@@ -661,16 +687,34 @@ ArmConfigureMmu (
     MAIR_ATTR (TT_ATTR_INDX_MEMORY_WRITE_BACK, MAIR_ATTR_NORMAL_MEMORY_WRITE_BACK)
     );
 
+  ArmSetTTBR0 (TranslationTable);
+
   ArmDisableAlignmentCheck ();
   ArmEnableStackAlignmentCheck ();
   ArmEnableInstructionCache ();
   ArmEnableDataCache ();
 
   ArmEnableMmu ();
+
+  if (NumRootPages > 1) {
+    //
+    // Clone all memory ranges into the shadow page tables that we will use
+    // to temporarily switch to when manipulating live entries
+    //
+    ZeroMem (TranslationTable + TT_ENTRY_COUNT,
+      RootTableEntryCount * sizeof (UINT64));
+
+    for (MemTab = MemoryTable; MemTab->Length != 0; MemTab++) {
+      Status = FillTranslationTable (TranslationTable + TT_ENTRY_COUNT, MemTab);
+      if (EFI_ERROR (Status)) {
+        goto FreeTranslationTable;
+      }
+    }
+  }
   return EFI_SUCCESS;
 
 FreeTranslationTable:
-  FreePages (TranslationTable, 1);
+  FreePages (TranslationTable, NumRootPages);
   return Status;
 }
 
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
index 66ebca571e63..6929e081ed8d 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
@@ -59,7 +59,20 @@ ASM_FUNC(ArmReplaceLiveTranslationEntry)
   dsb   nsh
 
   EL1_OR_EL2_OR_EL3(x3)
-1:__replace_entry 1
+1:mrs   x8, ttbr0_el1
+  add   x9, x8, #0x1000        // advance to shadow page table
+  orr   x9, x9, #1 << 48       // use different ASID for shadow translations
+  msr   ttbr0_el1, x9
+  isb
+  str   x1, [x0]               // install the entry and make it observeable
+  dsb   ishst                  // to the page table walker
+  isb
+  lsr   x2, x2, #12
+  tlbi  vae1is, x2             // invalidate the updated entry
+  dsb   ish
+  isb
+  msr   ttbr0_el1, x8          // switch back to original translation
+  isb
   b     4f
 2:__replace_entry 2
   b     4f
-- 
2.35.1


  parent reply	other threads:[~2022-07-04 17:53 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-04 17:52 [PATCH 0/7] ArmVirtPkg/ArmVirtQemu: avoid stores with MMU off Ard Biesheuvel
2022-07-04 17:52 ` [PATCH 1/7] ArmPkg/ArmMmuLib: don't replace table entries with block entries Ard Biesheuvel
2022-07-04 17:52 ` Ard Biesheuvel [this message]
2022-07-04 17:52 ` [PATCH 3/7] ArmPkg/ArmMmuLib: permit initial configuration with MMU enabled Ard Biesheuvel
2022-07-04 17:52 ` [PATCH 4/7] ArmPlatformPkg/PrePeiCore: permit entry with the " Ard Biesheuvel
2022-07-04 17:52 ` [PATCH 5/7] ArmVirtPkg/ArmVirtQemu: implement ArmPlatformLib with static ID map Ard Biesheuvel
2022-07-04 17:52 ` [PATCH 6/7] ArmVirtPkg/ArmVirtQemu: use first 128 MiB as permanent PEI memory Ard Biesheuvel
2022-07-04 17:52 ` [PATCH 7/7] ArmVirtPkg/ArmVirtQemu: enable initial ID map at early boot Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220704175252.4015120-3-ardb@kernel.org \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox