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

[Leif Lindholm]

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 at linaro.org>

Reviewed-by: Leif Lindholm <leif at 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
> 

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

View/Reply Online (#55600): https://edk2.groups.io/g/devel/message/55600
Mute This Topic: https://groups.io/mt/71770592/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-