On 2022-09-26 01:25, Ard Biesheuvel wrote:
> Drop the optimization that replaces table entries with block entries and
> frees the page tables in the subhierarchy that is being replaced. This
> rarely occurs in practice anyway, and will require more elaborate TLB
> maintenance once we switch to a different approach when running at EL1,
> where we no longer disable the MMU and nuke the TLB entirely every time
> we update a descriptor in a way that requires break-before-make (BBM).
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
/
Leif
> ---
> ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 20 ++------------------
> 1 file changed, 2 insertions(+), 18 deletions(-)
>
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> index e5ecc7375153..34f1031c4de3 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> @@ -197,12 +197,9 @@ UpdateRegionMappingRecursive (
> // than a block, and recurse to create the block or page entries at
>
> // the next level. No block mappings are allowed at all at level 0,
>
> // so in that case, we have to recurse unconditionally.
>
> - // If we are changing a table entry and the AttributeClearMask is non-zero,
>
> - // we cannot replace it with a block entry without potentially losing
>
> - // attribute information, so keep the table entry in that case.
>
> //
>
> if ((Level == 0) || (((RegionStart | BlockEnd) & BlockMask) != 0) ||
>
> - (IsTableEntry (*Entry, Level) && (AttributeClearMask != 0)))
>
> + IsTableEntry (*Entry, Level))
>
> {
>
> ASSERT (Level < 3);
>
>
>
> @@ -294,20 +291,7 @@ UpdateRegionMappingRecursive (
> EntryValue |= (Level == 3) ? TT_TYPE_BLOCK_ENTRY_LEVEL3
>
> : TT_TYPE_BLOCK_ENTRY;
>
>
>
> - if (IsTableEntry (*Entry, Level)) {
>
> - //
>
> - // We are replacing a table entry with a block entry. This is only
>
> - // possible if we are keeping none of the original attributes.
>
> - // We can free the table entry's page table, and all the ones below
>
> - // it, since we are dropping the only possible reference to it.
>
> - //
>
> - ASSERT (AttributeClearMask == 0);
>
> - TranslationTable = (VOID *)(UINTN)(*Entry & TT_ADDRESS_MASK_BLOCK_ENTRY);
>
> - ReplaceTableEntry (Entry, EntryValue, RegionStart, TRUE);
>
> - FreePageTablesRecursive (TranslationTable, Level + 1);
>
> - } else {
>
> - ReplaceTableEntry (Entry, EntryValue, RegionStart, FALSE);
>
> - }
>
> + ReplaceTableEntry (Entry, EntryValue, RegionStart, FALSE);
>
> }
>
> }
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#94366): https://edk2.groups.io/g/devel/message/94366
Mute This Topic: https://groups.io/mt/93922693/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-