From: "Leif Lindholm" <quic_llindhol@quicinc.com>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: <devel@edk2.groups.io>, Liming Gao <gaoliming@byosoft.com.cn>,
Michael D Kinney <michael.d.kinney@intel.com>,
Oliver Steffen <osteffen@redhat.com>
Subject: Re: [PATCH edk2-stable202305] ArmPkg/ArmMmuLib: Add missing ISB after page table update
Date: Mon, 22 May 2023 11:56:05 +0100 [thread overview]
Message-ID: <ZGtKRQG4qmcEee1V@qc-i7.hemma.eciton.net> (raw)
In-Reply-To: <20230520091958.310712-1-ardb@kernel.org>
On Sat, May 20, 2023 at 11:19:58 +0200, Ard Biesheuvel wrote:
> The helper that updates live page table entries writes a zero entry,
> invalidates the covered address range from the TLBs, and finally writes
> the actual entry. This ensures that no TLB conflicts can occur.
>
> Writing the final entry needs to complete before any translations can be
> performed, as otherwise, the zero entry, which describes an invalid
> translation, may be observed by the page table walker, resulting in a
> translation fault. For this reason, the final write is followed by a DSB
> barrier instruction.
>
> However, this barrier will not stall the pipeline, and instruction
> fetches may still hit this invalid translation, as has been observed and
> reported by Oliver. To ensure that the new translation is fully active
> before returning from this helper, we have to insert an ISB barrier as
> well.
>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Reported-by: Oliver Steffen <osteffen@redhat.com>
> Tested-by: Oliver Steffen <osteffen@redhat.com>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
We need this in the stable tag.
Note: the isb instruction forces the synchronization of certain
architectural events. It has no other effects. I.e., any issues
exposed by this addition would already have been present before it.
As such, I would suggest this addition need *not* affect the stable
tag schedule.
/
Leif
> ---
> ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> index 887439bc042f0f16..1f0d8057926933d7 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> @@ -65,6 +65,7 @@
> // write updated entry
> str x1, [x0]
> dsb nshst
> + isb
>
> .L2_\@:
> .endm
> --
> 2.39.2
>
next prev parent reply other threads:[~2023-05-22 10:56 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-20 9:19 [PATCH edk2-stable202305] ArmPkg/ArmMmuLib: Add missing ISB after page table update Ard Biesheuvel
2023-05-22 8:57 ` Ard Biesheuvel
2023-05-22 10:56 ` Leif Lindholm [this message]
2023-05-22 14:08 ` [edk2-devel] " Michael D Kinney
2023-05-22 16:19 ` Ard Biesheuvel
2023-05-23 0:43 ` Michael D Kinney
2023-05-23 6:53 ` Ard Biesheuvel
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=ZGtKRQG4qmcEee1V@qc-i7.hemma.eciton.net \
--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