Acked-by: Michael D Kinney <michael.d.kin...@intel.com> No objection to merging for the stable tag.
Mike > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Leif > Lindholm > Sent: Monday, May 22, 2023 3:56 AM > To: Ard Biesheuvel <a...@kernel.org> > Cc: devel@edk2.groups.io; Gao, Liming <gaolim...@byosoft.com.cn>; > Kinney, Michael D <michael.d.kin...@intel.com>; Oliver Steffen > <ostef...@redhat.com> > Subject: Re: [edk2-devel] [PATCH edk2-stable202305] ArmPkg/ArmMmuLib: > Add missing ISB after page table update > > 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 <gaolim...@byosoft.com.cn> > > Cc: Leif Lindholm <quic_llind...@quicinc.com> > > Cc: Michael D Kinney <michael.d.kin...@intel.com> > > Reported-by: Oliver Steffen <ostef...@redhat.com> > > Tested-by: Oliver Steffen <ostef...@redhat.com> > > Signed-off-by: Ard Biesheuvel <a...@kernel.org> > > Reviewed-by: Leif Lindholm <quic_llind...@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 > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#105127): https://edk2.groups.io/g/devel/message/105127 Mute This Topic: https://groups.io/mt/99027974/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
