Re: [PATCH] ArmPkg/ArmMmuLib AARCH64: invalidate page tables before populating them

["Ard Biesheuvel" <ard.biesheuvel@linaro.org>]

On Thu, 5 Mar 2020 at 22:50, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> As it turns out, ARMv8 (DDI 0487E.a D4.4.5) 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>

Ugh, still not sufficient. I'll send a v2 tomorrow.

> ---
>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 25 ++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> index 204e33c75f95..b5d6b66806f8 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> @@ -282,6 +282,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,6 +315,14 @@ 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
> @@ -697,6 +714,14 @@ ArmConfigureMmu (
>      *TranslationTableSize = RootTableEntryCount * sizeof(UINT64);
>    }
>
> +  if (!ArmMmuEnabled ()) {
> +    //
> +    // 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
>