On Fri, Nov 22, 2013 at 10:56:05AM +0000, Marc Zyngier wrote: > On 22/11/13 01:51, Christoffer Dall wrote: > > On 21 November 2013 00:59, Marc Zyngier <marc.zyng...@arm.com> wrote: > >> A CP15 instruction execution can be reordered, requiring an > >> isb to be sure it is executed in program order. > >> > >> Signed-off-by: Marc Zyngier <marc.zyng...@arm.com> > >> --- > >> arch/arm/cpu/armv7/nonsec_virt.S | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/arch/arm/cpu/armv7/nonsec_virt.S > >> b/arch/arm/cpu/armv7/nonsec_virt.S > >> index 29987cd..648066f 100644 > >> --- a/arch/arm/cpu/armv7/nonsec_virt.S > >> +++ b/arch/arm/cpu/armv7/nonsec_virt.S > >> @@ -47,6 +47,7 @@ _secure_monitor: > >> #endif > >> > >> mcr p15, 0, r1, c1, c1, 0 @ write SCR (with NS bit > >> set) > >> + isb > >> > >> #ifdef CONFIG_ARMV7_VIRT > >> mrceq p15, 0, r0, c12, c0, 1 @ get MVBAR value > >> -- > >> 1.8.2.3 > >> > > Does this matter? Are we not still in monitor mode and therefore > > secure and the exception return below will surely be ordered by the > > cpu after the mcr, right? or no? > > You need to look at what is between the SCR access and the exception > return (and you cannot see that from the patch): There's a write to > HVBAR that can only be executed if SCR.NS==1. > > If the write to SCR is delayed, the whole thing will stop very quickly... > Ah, I didn't realize this specific about PL2-mode system control registers and relied on the fact that we were just in monitor mode and that the setting of this bit essentially didn't matter; I obviously got this wrong, and I think to be fair that Andre had this isb in his original code and removed it after my review.
/my bad. Thanks for the explanation. -Christoffer _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
