From nobody Tue Feb 10 23:01:14 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) client-ip=66.175.222.108; envelope-from=bounce+27952+94331+1787277+3901457@groups.io; helo=mail02.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+94331+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=kernel.org ARC-Seal: i=1; a=rsa-sha256; t=1664180733; cv=none; d=zohomail.com; s=zohoarc; b=JfSB4ao/5S4Q4+cOa2upKaZHMoAh8ngumTOB5g3YtHr4jPcZkdznSwk8+XO4AtdDiq+RW1ppy/2xJ0sIRhHQA5NoyuqKYyzzOgJb5T9lWhNKrP2785UfPFuS27/FMnKxB83lZREmbXq1zxKNDJohUxAubDP4QBU8wj3eFjqf2P0= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1664180733; h=Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:References:Sender:Subject:To; bh=JeYt1SVv48L4+zEEzgK89QU3Y9NooukINF1En9UomEg=; b=U9Lbusq0sWwJKQ1rHZGw+MD8qvUc3HwykdGRGUAZzUVtksGzrdmB5k2fZs9U7sy/lXAwsWyouZVoFTTEvSwDH2RO1GbHm/pqtPSJoR1Ax+k4JSF3yxm5pUPVlaF3D4C84WqAgdilk5tsnu1jM/O82SSVQLXiOnw3KZig92ZjL20= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+94331+1787277+3901457@groups.io; dmarc=fail header.from= (p=none dis=none) Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by mx.zohomail.com with SMTPS id 1664180733081279.3001906921413; Mon, 26 Sep 2022 01:25:33 -0700 (PDT) Return-Path: X-Received: by 127.0.0.2 with SMTP id CGwrYY1788612xwfpuY6Su2C; Mon, 26 Sep 2022 01:25:32 -0700 X-Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by mx.groups.io with SMTP id smtpd.web10.25936.1664180731372879402 for ; Mon, 26 Sep 2022 01:25:32 -0700 X-Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sin.source.kernel.org (Postfix) with ESMTPS id BE6C0CE1091; Mon, 26 Sep 2022 08:25:28 +0000 (UTC) X-Received: by smtp.kernel.org (Postfix) with ESMTPSA id 98BCFC433C1; Mon, 26 Sep 2022 08:25:25 +0000 (UTC) From: "Ard Biesheuvel" To: devel@edk2.groups.io Cc: Ard Biesheuvel , Leif Lindholm , Alexander Graf Subject: [edk2-devel] [PATCH v3 06/16] ArmPkg/ArmMmuLib: Disable and re-enable MMU only when needed Date: Mon, 26 Sep 2022 10:25:01 +0200 Message-Id: <20220926082511.2110797-7-ardb@kernel.org> In-Reply-To: <20220926082511.2110797-1-ardb@kernel.org> References: <20220926082511.2110797-1-ardb@kernel.org> MIME-Version: 1.0 Precedence: Bulk List-Unsubscribe: List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,ardb@kernel.org X-Gm-Message-State: Rdx70v1Y84b7ImcLUkOL0cHsx1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1664180732; bh=DIRjx+Eckg+1oZ4tMYZRwHJtzzrRx7DrKGqA9TTKIks=; h=Cc:Date:From:Reply-To:Subject:To; b=G5oNR+hU9o2deSPy9w00zD0ajpWIm6Lbyimx5CUA+ZqtzTkeBqApMl0db96NZo57EBB 4pKRDoHWbtraS6sa0NN7CaMmdHp1zkEcK3CRPcVYut43k3bHQkMz06mDgUJbNGSRm2Vkh DmUvlhT2RtOyBuaVqipQwD8fqOgug0jX/Fk= X-ZohoMail-DKIM: pass (identity @groups.io) X-ZM-MESSAGEID: 1664180734582100002 Content-Type: text/plain; charset="utf-8" When updating a page table descriptor in a way that requires break before make, we temporarily disable the MMU to ensure that we don't unmap the memory region that the code itself is executing from. However, this is a condition we can check in a straight-forward manner, and if the regions are disjoint, we don't have to bother with the MMU controls, and we can just perform an ordinary break before make. Signed-off-by: Ard Biesheuvel Acked-by: Leif Lindholm --- ArmPkg/Include/Library/ArmMmuLib.h | 7 +- ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 102 +++++++++++= +++++---- ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S | 43 +++++++-- 3 files changed, 123 insertions(+), 29 deletions(-) diff --git a/ArmPkg/Include/Library/ArmMmuLib.h b/ArmPkg/Include/Library/Ar= mMmuLib.h index 7538a8274a72..b745e2230e7e 100644 --- a/ArmPkg/Include/Library/ArmMmuLib.h +++ b/ArmPkg/Include/Library/ArmMmuLib.h @@ -52,9 +52,10 @@ ArmClearMemoryRegionReadOnly ( VOID EFIAPI ArmReplaceLiveTranslationEntry ( - IN UINT64 *Entry, - IN UINT64 Value, - IN UINT64 RegionStart + IN UINT64 *Entry, + IN UINT64 Value, + IN UINT64 RegionStart, + IN BOOLEAN DisableMmu ); =20 EFI_STATUS diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Libr= ary/ArmMmuLib/AArch64/ArmMmuLibCore.c index 34f1031c4de3..4d75788ed2b2 100644 --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c @@ -18,6 +18,17 @@ #include #include #include +#include + +STATIC +VOID ( + EFIAPI *mReplaceLiveEntryFunc + )( + IN UINT64 *Entry, + IN UINT64 Value, + IN UINT64 RegionStart, + IN BOOLEAN DisableMmu + ) =3D ArmReplaceLiveTranslationEntry; =20 STATIC UINT64 @@ -83,14 +94,40 @@ ReplaceTableEntry ( IN UINT64 *Entry, IN UINT64 Value, IN UINT64 RegionStart, + IN UINT64 BlockMask, IN BOOLEAN IsLiveBlockMapping ) { - if (!ArmMmuEnabled () || !IsLiveBlockMapping) { + BOOLEAN DisableMmu; + + // + // Replacing a live block entry with a table entry (or vice versa) requi= res a + // break-before-make sequence as per the architecture. This means the ma= pping + // must be made invalid and cleaned from the TLBs first, and this is a b= it of + // a hassle if the mapping in question covers the code that is actually = doing + // the mapping and the unmapping, and so we only bother with this if act= ually + // necessary. + // + + if (!IsLiveBlockMapping || !ArmMmuEnabled ()) { + // If the mapping is not a live block mapping, or the MMU is not on ye= t, we + // can simply overwrite the entry. *Entry =3D Value; ArmUpdateTranslationTableEntry (Entry, (VOID *)(UINTN)RegionStart); } else { - ArmReplaceLiveTranslationEntry (Entry, Value, RegionStart); + // If the mapping in question does not cover the code that updates the + // entry in memory, or the entry that we are intending to update, we c= an + // use an ordinary break before make. Otherwise, we will need to + // temporarily disable the MMU. + DisableMmu =3D FALSE; + if ((((RegionStart ^ (UINTN)ArmReplaceLiveTranslationEntry) & ~BlockMa= sk) =3D=3D 0) || + (((RegionStart ^ (UINTN)Entry) & ~BlockMask) =3D=3D 0)) + { + DisableMmu =3D TRUE; + DEBUG ((DEBUG_WARN, "%a: splitting block entry with MMU disabled\n",= __FUNCTION__)); + } + + ArmReplaceLiveTranslationEntry (Entry, Value, RegionStart, DisableMmu); } } =20 @@ -155,12 +192,13 @@ IsTableEntry ( STATIC EFI_STATUS UpdateRegionMappingRecursive ( - IN UINT64 RegionStart, - IN UINT64 RegionEnd, - IN UINT64 AttributeSetMask, - IN UINT64 AttributeClearMask, - IN UINT64 *PageTable, - IN UINTN Level + IN UINT64 RegionStart, + IN UINT64 RegionEnd, + IN UINT64 AttributeSetMask, + IN UINT64 AttributeClearMask, + IN UINT64 *PageTable, + IN UINTN Level, + IN BOOLEAN TableIsLive ) { UINTN BlockShift; @@ -170,6 +208,7 @@ UpdateRegionMappingRecursive ( UINT64 EntryValue; VOID *TranslationTable; EFI_STATUS Status; + BOOLEAN NextTableIsLive; =20 ASSERT (((RegionStart | RegionEnd) & EFI_PAGE_MASK) =3D=3D 0); =20 @@ -198,7 +237,14 @@ UpdateRegionMappingRecursive ( // the next level. No block mappings are allowed at all at level 0, // so in that case, we have to recurse unconditionally. // + // One special case to take into account is any region that covers the= page + // table itself: if we'd cover such a region with block mappings, we a= re + // more likely to end up in the situation later where we need to disab= le + // the MMU in order to update page table entries safely, so prefer page + // mappings in that particular case. + // if ((Level =3D=3D 0) || (((RegionStart | BlockEnd) & BlockMask) !=3D 0= ) || + ((Level < 3) && (((UINT64)PageTable & ~BlockMask) =3D=3D RegionSta= rt)) || IsTableEntry (*Entry, Level)) { ASSERT (Level < 3); @@ -234,7 +280,8 @@ UpdateRegionMappingRecursive ( *Entry & TT_ATTRIBUTES_MASK, 0, TranslationTable, - Level + 1 + Level + 1, + FALSE ); if (EFI_ERROR (Status)) { // @@ -246,8 +293,11 @@ UpdateRegionMappingRecursive ( return Status; } } + + NextTableIsLive =3D FALSE; } else { TranslationTable =3D (VOID *)(UINTN)(*Entry & TT_ADDRESS_MASK_BLOC= K_ENTRY); + NextTableIsLive =3D TableIsLive; } =20 // @@ -259,7 +309,8 @@ UpdateRegionMappingRecursive ( AttributeSetMask, AttributeClearMask, TranslationTable, - Level + 1 + Level + 1, + NextTableIsLive ); if (EFI_ERROR (Status)) { if (!IsTableEntry (*Entry, Level)) { @@ -282,7 +333,8 @@ UpdateRegionMappingRecursive ( Entry, EntryValue, RegionStart, - IsBlockEntry (*Entry, Level) + BlockMask, + TableIsLive && IsBlockEntry (*Entry, Level) ); } } else { @@ -291,7 +343,7 @@ UpdateRegionMappingRecursive ( EntryValue |=3D (Level =3D=3D 3) ? TT_TYPE_BLOCK_ENTRY_LEVEL3 : TT_TYPE_BLOCK_ENTRY; =20 - ReplaceTableEntry (Entry, EntryValue, RegionStart, FALSE); + ReplaceTableEntry (Entry, EntryValue, RegionStart, BlockMask, FALSE); } } =20 @@ -301,10 +353,11 @@ UpdateRegionMappingRecursive ( STATIC EFI_STATUS UpdateRegionMapping ( - IN UINT64 RegionStart, - IN UINT64 RegionLength, - IN UINT64 AttributeSetMask, - IN UINT64 AttributeClearMask + IN UINT64 RegionStart, + IN UINT64 RegionLength, + IN UINT64 AttributeSetMask, + IN UINT64 AttributeClearMask, + IN BOOLEAN TableIsLive ) { UINTN T0SZ; @@ -321,7 +374,8 @@ UpdateRegionMapping ( AttributeSetMask, AttributeClearMask, ArmGetTTBR0BaseAddress (), - GetRootTableLevel (T0SZ) + GetRootTableLevel (T0SZ), + TableIsLive ); } =20 @@ -336,7 +390,8 @@ FillTranslationTable ( MemoryRegion->VirtualBase, MemoryRegion->Length, ArmMemoryAttributeToPageAttribute (MemoryRegion->Attributes) | = TT_AF, - 0 + 0, + FALSE ); } =20 @@ -410,7 +465,8 @@ ArmSetMemoryAttributes ( BaseAddress, Length, PageAttributes, - PageAttributeMask + PageAttributeMask, + TRUE ); } =20 @@ -423,7 +479,13 @@ SetMemoryRegionAttribute ( IN UINT64 BlockEntryMask ) { - return UpdateRegionMapping (BaseAddress, Length, Attributes, BlockEntryM= ask); + return UpdateRegionMapping ( + BaseAddress, + Length, + Attributes, + BlockEntryMask, + TRUE + ); } =20 EFI_STATUS diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S b/Arm= Pkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S index 66ebca571e63..e936a5be4e11 100644 --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S @@ -12,6 +12,14 @@ =20 .macro __replace_entry, el =20 + // check whether we should disable the MMU + cbz x3, .L1_\@ + + // clean and invalidate first so that we don't clobber + // adjacent entries that are dirty in the caches + dc civac, x0 + dsb nsh + // disable the MMU mrs x8, sctlr_el\el bic x9, x8, #CTRL_M_BIT @@ -38,8 +46,33 @@ // re-enable the MMU msr sctlr_el\el, x8 isb + b .L2_\@ + +.L1_\@: + // write invalid entry + str xzr, [x0] + dsb nshst + + // flush translations for the target address from the TLBs + lsr x2, x2, #12 + .if \el =3D=3D 1 + tlbi vaae1, x2 + .else + tlbi vae\el, x2 + .endif + dsb nsh + + // write updated entry + str x1, [x0] + dsb nshst + +.L2_\@: .endm =20 + // Align this routine to a log2 upper bound of its size, so that it is + // guaranteed not to cross a page or block boundary. + .balign 0x200 + //VOID //ArmReplaceLiveTranslationEntry ( // IN UINT64 *Entry, @@ -53,12 +86,7 @@ ASM_FUNC(ArmReplaceLiveTranslationEntry) msr daifset, #0xf isb =20 - // clean and invalidate first so that we don't clobber - // adjacent entries that are dirty in the caches - dc civac, x0 - dsb nsh - - EL1_OR_EL2_OR_EL3(x3) + EL1_OR_EL2_OR_EL3(x5) 1:__replace_entry 1 b 4f 2:__replace_entry 2 @@ -72,3 +100,6 @@ ASM_GLOBAL ASM_PFX(ArmReplaceLiveTranslationEntrySize) =20 ASM_PFX(ArmReplaceLiveTranslationEntrySize): .long . - ArmReplaceLiveTranslationEntry + + // Double check that we did not overrun the assumed maximum size + .org ArmReplaceLiveTranslationEntry + 0x200 --=20 2.35.1 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#94331): https://edk2.groups.io/g/devel/message/94331 Mute This Topic: https://groups.io/mt/93922697/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-