[edk2-devel] [PATCH v3 2/2] ArmPkg/ArmMmuLib AARCH64: invalidate page tables before populating them

Ard Biesheuvel posted 2 patches 5 years, 11 months ago
There is a newer version of this series
[edk2-devel] [PATCH v3 2/2] ArmPkg/ArmMmuLib AARCH64: invalidate page tables before populating them
Posted by Ard Biesheuvel 5 years, 11 months ago
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.

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

diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index e36594fea3ad..10ca8bac6a3f 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -217,6 +217,14 @@ UpdateRegionMappingRec (
           return EFI_OUT_OF_RESOURCES;
         }
 
+        if (!ArmMmuEnabled ()) {
+          //
+          // Make sure we are not inadvertently hitting in the caches
+          // when populating the page tables.
+          //
+          InvalidateDataCacheRange (TranslationTable, EFI_PAGE_SIZE);
+        }
+
         if ((*Entry & TT_TYPE_MASK) == TT_TYPE_BLOCK_ENTRY) {
           //
           // We are splitting an existing block entry, so we have to populate
@@ -581,6 +589,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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55604): https://edk2.groups.io/g/devel/message/55604
Mute This Topic: https://groups.io/mt/71776545/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 2/2] ArmPkg/ArmMmuLib AARCH64: invalidate page tables before populating them
Posted by Leif Lindholm 5 years, 11 months ago
On Fri, Mar 06, 2020 at 17:12:46 +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
> should invalidate page tables right after allocation like we do now on
> ARM, if the MMU is still disabled at that point.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

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

> ---
>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> index e36594fea3ad..10ca8bac6a3f 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> @@ -217,6 +217,14 @@ UpdateRegionMappingRec (
>            return EFI_OUT_OF_RESOURCES;
>          }
>  
> +        if (!ArmMmuEnabled ()) {
> +          //
> +          // Make sure we are not inadvertently hitting in the caches
> +          // when populating the page tables.
> +          //
> +          InvalidateDataCacheRange (TranslationTable, EFI_PAGE_SIZE);
> +        }
> +
>          if ((*Entry & TT_TYPE_MASK) == TT_TYPE_BLOCK_ENTRY) {
>            //
>            // We are splitting an existing block entry, so we have to populate
> @@ -581,6 +589,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
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55613): https://edk2.groups.io/g/devel/message/55613
Mute This Topic: https://groups.io/mt/71776545/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-