public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2] ArmPkg/ArmMmuLib AARCH64: invalidate page tables before populating them
@ 2020-03-06 10:40 Ard Biesheuvel
  2020-03-06 13:03 ` Leif Lindholm
  0 siblings, 1 reply; 2+ messages in thread
From: Ard Biesheuvel @ 2020-03-06 10:40 UTC (permalink / raw)
  To: devel; +Cc: leif, Ard Biesheuvel

As it turns out, ARMv8 also permits accesses made with the MMU and
caches off to hit in the caches, so to ensure that any modifications
we make before enabling the MMU are visible afterwards as well, we
should invalidate page tables right after allocation like we do now on
ARM, if the MMU is still disabled at that point.

Also, make sure that we don't only invalidate block and page entries
when updating the individual entries, but give table entries the same
treatment.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
v2:
- drop redundant MMU enabled check when allocating the root table
- add dmb+ivac for individual table entries (the change that was merged
  already only does this on block/page entries)

 ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 30 ++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index 204e33c75f95..d4d823780a6a 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -129,6 +129,8 @@ ReplaceLiveEntry (
 {
   if (!ArmMmuEnabled ()) {
     *Entry = Value;
+    ArmDataMemoryBarrier ();
+    ArmInvalidateDataCacheEntryByMVA ((UINTN)Entry);
   } else {
     ArmReplaceLiveTranslationEntry (Entry, Value, RegionStart);
   }
@@ -282,6 +284,15 @@ GetBlockEntryListFromAddress (
           return NULL;
         }
 
+        if (!ArmMmuEnabled ()) {
+          //
+          // Make sure we are not inadvertently hitting in the caches
+          // when populating the page tables.
+          //
+          InvalidateDataCacheRange (TranslationTable,
+            TT_ENTRY_COUNT * sizeof(UINT64));
+        }
+
         // Populate the newly created lower level table
         SubTableBlockEntry = TranslationTable;
         for (Index = 0; Index < TT_ENTRY_COUNT; Index++) {
@@ -306,10 +317,23 @@ GetBlockEntryListFromAddress (
           return NULL;
         }
 
+        if (!ArmMmuEnabled ()) {
+          //
+          // Make sure we are not inadvertently hitting in the caches
+          // when populating the page tables.
+          //
+          InvalidateDataCacheRange (TranslationTable,
+            TT_ENTRY_COUNT * sizeof(UINT64));
+        }
         ZeroMem (TranslationTable, TT_ENTRY_COUNT * sizeof(UINT64));
 
         // Fill the new BlockEntry with the TranslationTable
         *BlockEntry = ((UINTN)TranslationTable & TT_ADDRESS_MASK_DESCRIPTION_TABLE) | TT_TYPE_TABLE_ENTRY;
+
+        if (!ArmMmuEnabled ()) {
+          ArmDataMemoryBarrier ();
+          ArmInvalidateDataCacheEntryByMVA ((UINTN)BlockEntry);
+        }
       }
     }
   }
@@ -697,6 +721,12 @@ ArmConfigureMmu (
     *TranslationTableSize = RootTableEntryCount * sizeof(UINT64);
   }
 
+  //
+  // Make sure we are not inadvertently hitting in the caches
+  // when populating the page tables.
+  //
+  InvalidateDataCacheRange (TranslationTable,
+    RootTableEntryCount * sizeof(UINT64));
   ZeroMem (TranslationTable, RootTableEntryCount * sizeof(UINT64));
 
   TranslationTableAttribute = TT_ATTR_INDX_INVALID;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] ArmPkg/ArmMmuLib AARCH64: invalidate page tables before populating them
  2020-03-06 10:40 [PATCH v2] ArmPkg/ArmMmuLib AARCH64: invalidate page tables before populating them Ard Biesheuvel
@ 2020-03-06 13:03 ` Leif Lindholm
  0 siblings, 0 replies; 2+ messages in thread
From: Leif Lindholm @ 2020-03-06 13:03 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: devel

On Fri, Mar 06, 2020 at 11:40:32 +0100, Ard Biesheuvel wrote:
> As it turns out, ARMv8 also permits accesses made with the MMU and
> caches off to hit in the caches, so to ensure that any modifications
> we make before enabling the MMU are visible afterwards as well, we

Urgh. I thought v8 had changed that behaviour.

> should invalidate page tables right after allocation like we do now on
> ARM, if the MMU is still disabled at that point.
> 
> Also, make sure that we don't only invalidate block and page entries
> when updating the individual entries, but give table entries the same
> treatment.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

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

> ---
> v2:
> - drop redundant MMU enabled check when allocating the root table
> - add dmb+ivac for individual table entries (the change that was merged
>   already only does this on block/page entries)
> 
>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 30 ++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> index 204e33c75f95..d4d823780a6a 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> @@ -129,6 +129,8 @@ ReplaceLiveEntry (
>  {
>    if (!ArmMmuEnabled ()) {
>      *Entry = Value;
> +    ArmDataMemoryBarrier ();
> +    ArmInvalidateDataCacheEntryByMVA ((UINTN)Entry);
>    } else {
>      ArmReplaceLiveTranslationEntry (Entry, Value, RegionStart);
>    }
> @@ -282,6 +284,15 @@ GetBlockEntryListFromAddress (
>            return NULL;
>          }
>  
> +        if (!ArmMmuEnabled ()) {
> +          //
> +          // Make sure we are not inadvertently hitting in the caches
> +          // when populating the page tables.
> +          //
> +          InvalidateDataCacheRange (TranslationTable,
> +            TT_ENTRY_COUNT * sizeof(UINT64));
> +        }
> +
>          // Populate the newly created lower level table
>          SubTableBlockEntry = TranslationTable;
>          for (Index = 0; Index < TT_ENTRY_COUNT; Index++) {
> @@ -306,10 +317,23 @@ GetBlockEntryListFromAddress (
>            return NULL;
>          }
>  
> +        if (!ArmMmuEnabled ()) {
> +          //
> +          // Make sure we are not inadvertently hitting in the caches
> +          // when populating the page tables.
> +          //
> +          InvalidateDataCacheRange (TranslationTable,
> +            TT_ENTRY_COUNT * sizeof(UINT64));
> +        }
>          ZeroMem (TranslationTable, TT_ENTRY_COUNT * sizeof(UINT64));
>  
>          // Fill the new BlockEntry with the TranslationTable
>          *BlockEntry = ((UINTN)TranslationTable & TT_ADDRESS_MASK_DESCRIPTION_TABLE) | TT_TYPE_TABLE_ENTRY;
> +
> +        if (!ArmMmuEnabled ()) {
> +          ArmDataMemoryBarrier ();
> +          ArmInvalidateDataCacheEntryByMVA ((UINTN)BlockEntry);
> +        }
>        }
>      }
>    }
> @@ -697,6 +721,12 @@ ArmConfigureMmu (
>      *TranslationTableSize = RootTableEntryCount * sizeof(UINT64);
>    }
>  
> +  //
> +  // Make sure we are not inadvertently hitting in the caches
> +  // when populating the page tables.
> +  //
> +  InvalidateDataCacheRange (TranslationTable,
> +    RootTableEntryCount * sizeof(UINT64));
>    ZeroMem (TranslationTable, RootTableEntryCount * sizeof(UINT64));
>  
>    TranslationTableAttribute = TT_ATTR_INDX_INVALID;
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-03-06 13:03 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-06 10:40 [PATCH v2] ArmPkg/ArmMmuLib AARCH64: invalidate page tables before populating them Ard Biesheuvel
2020-03-06 13:03 ` Leif Lindholm

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox