From: Marc Zyngier <maz@kernel.org>
To: Ashish Singhal <ashishsingha@nvidia.com>
Cc: <devel@edk2.groups.io>, <quic_llindhol@quicinc.com>,
<ardb+tianocore@kernel.org>, <sami.mujawar@arm.com>
Subject: Re: [PATCH v2] ArmPkg: Invalidate Instruction Cache On MMU Enable
Date: Sat, 26 Feb 2022 21:18:34 +0000 [thread overview]
Message-ID: <87zgmd5nth.wl-maz@kernel.org> (raw)
In-Reply-To: <9f95ba0bb19fd034af27f4f564e5eeff0ec19fff.1645850486.git.ashishsingha@nvidia.com>
On Sat, 26 Feb 2022 04:43:37 +0000,
Ashish Singhal <ashishsingha@nvidia.com> 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,
Modified by what?
> we ensure that instruction cache is invalidated right after
> MMU has been enabled and any potentially stale instruction
> fetched earlier has been discarded.
>
> This is specially helpful when the memory attributes of a
> region in MMU are being changed and some instructions
Changed from what to what else? Are you concerned with the content of
the memory being changed? Or by the attribute being changed? Or both?
> operating on the region are prefetched in the instruction
> cache.
I don't see how this fixes anything. Yes, speculation occurs. But if
your icache contains crap, how is it safe to first enable the MMU
first and then nuke the icache? You could well be executing garbage at
that point.
Worse case, and assuming that you have an aliasing VIVT icache, this
will invalidate fetches that would alias with the layout of the memory
once the MMU is on. But as far as I know, EDK2 is entirely identity
mapped. I also don't think it uses instruction patching.
Finally, if you see speculative accesses on regions that shouldn't be
accessed as such, it could well be because the code is placed too
close to such a region, as mentioned in the ARM ARM (DDI0487H_a, page
D5-4828):
<quote>
Behavior of instruction fetches when all associated stages of
translation are disabled
[...]
To ensure architectural compliance, software must ensure that both of
the following apply:
• Instructions that will be executed when all associated stages of
address translation are disabled are located in blocks of the
address space, of the translation granule size, that contain only
memory that is tolerant to speculative accesses.
• Each block of the address space, of the translation granule size,
that immediately follows a similar block that holds instructions
that will be executed when all associated stages address translation
are disabled, contains only memory that is tolerant to speculative
accesses.
</quote>
Thanks,
M
--
Without deviation from the norm, progress is not possible.
next prev parent reply other threads:[~2022-02-26 21:18 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-26 4:43 [PATCH v2] ArmPkg: Invalidate Instruction Cache On MMU Enable Ashish Singhal
2022-02-26 21:18 ` Marc Zyngier [this message]
2022-02-26 21:48 ` Ashish Singhal
2022-02-27 15:36 ` Ashish Singhal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87zgmd5nth.wl-maz@kernel.org \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox