From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web12.4541.1645599761858797216 for ; Tue, 22 Feb 2022 23:02:42 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=kOGQYS6Z; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) 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 dfw.source.kernel.org (Postfix) with ESMTPS id E0C2E614AA for ; Wed, 23 Feb 2022 07:02:40 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4E44FC340F1 for ; Wed, 23 Feb 2022 07:02:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1645599760; bh=kjApRbm30Cb/9hVgY5Rat2uufhusvTgHnAMg+b4BIPE=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=kOGQYS6ZKG9jnLk2M6YcVQr5gFfCsfQcHp76oU7FpWelK15rPvn4kx6Xp0JuhX5FQ bkEMRPKihbakbHjWuPA3cu3skv4RLxgtzXYM2TdnB7fFsK+FTbbPNdwtTuJUm9PuRE Z86IkeEn8kjvAR3DkPDs1QpUam3iMIu4cyM14FJThHlP/bfhye2EkKkpOF3o6Itrlp 8R8WzrHJ+KkeweFjsO6U+p3Rbwum6OBP7uODXNzQl4E1lWLHrvcVPblgimAGVids51 cYdcI/JLloy4DXzk9OnKthQmUP0LJ7Mt7uXfoMUfePX+thBFa0PrWZx9Rhwpg86pbz b8DeBX24g2WNQ== Received: by mail-yw1-f182.google.com with SMTP id 00721157ae682-2d62593ad9bso199544167b3.8 for ; Tue, 22 Feb 2022 23:02:40 -0800 (PST) X-Gm-Message-State: AOAM5332+5TY8+1y2ajZyiZg7CDWGjiroosXCDcUJBrrxex45KnrZ8ST kyT3K3n3LNZgY6ocJkDh7ltgJG6+/LM6nrOx64I= X-Google-Smtp-Source: ABdhPJyCgK4ykw8LgGB2kOGpBgeXMEh7pQrjbIFoMxSca16v48PmEt9eLdYYeKYwbu6tSBJ1xBR5wNtqZ5sobHLyomc= X-Received: by 2002:a81:7d04:0:b0:2d0:d0e2:126f with SMTP id y4-20020a817d04000000b002d0d0e2126fmr27283461ywc.485.1645599759399; Tue, 22 Feb 2022 23:02:39 -0800 (PST) MIME-Version: 1.0 References: <122c32bb19ed0730ef166b9f46d3b112bc9ed937.1645497637.git.ashishsingha@nvidia.com> In-Reply-To: <122c32bb19ed0730ef166b9f46d3b112bc9ed937.1645497637.git.ashishsingha@nvidia.com> From: "Ard Biesheuvel" Date: Wed, 23 Feb 2022 08:02:28 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] ArmPkg: Invalidate Instruction Cache On MMU Enable To: Ashish Singhal , Marc Zyngier Cc: edk2-devel-groups-io , Sami Mujawar , Ard Biesheuvel , Leif Lindholm Content-Type: text/plain; charset="UTF-8" (+ Marc) On Tue, 22 Feb 2022 at 03:42, Ashish Singhal wrote: > > Even with MMU turned off, instruction cache can speculate > and fetch instructions. This can cause a crash if region > being executed has been modified recently. With this patch, > we ensure that instruction cache is invalidated right after > MMU has been enabled and any potentially stale instruction > fetched earlier has been discarded. > I don't follow this reasoning. Every time the UEFI image loader loads an image into memory, it does so with MMU and caches enabled, and the code regions are cleaned to the PoU before being invalidated in the I-cache. So how could these regions be stale? The only code that needs special care here is the little trampoline that executes with the MMU off, but this is being taken care of explicitly, by cleaning the code to the PoC before invoking it. > This is specially helpful when the memory attributes of a > region in MMU are being changed and some instructions > operating on the region are prefetched in the instruction > cache. > This sounds like a TLB problem not a cache problem. For the sake of posterity, could you include a more detailed description of the issue in the commit log? IIRC, this is about speculative instruction fetches hitting device memory locations? As I mentioned before, the architecture does not permit speculative instruction fetches for regions mapped with the XN attribute. Is the issue occurring under virtualization? Is there a stage 2 mapping that lacks the XN attribute for the device memory region in question? > Signed-off-by: Ashish Singhal > --- > ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 4 +++- > ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S | 2 ++ > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S > index d3cc1e8671..9648245182 100644 > --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S > +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S > @@ -89,7 +89,9 @@ ASM_FUNC(ArmEnableMmu) > dsb nsh > isb > msr sctlr_el3, x0 // Write back > -4: isb > +4: ic iallu > + dsb sy > + isb > ret > > > diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S > index 66ebca571e..56cc2dd73f 100644 > --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S > +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S > @@ -37,6 +37,8 @@ > > // re-enable the MMU > msr sctlr_el\el, x8 > + ic iallu > + dsb sy > isb > .endm > > -- > 2.17.1 >