[U-Boot] [PATCH 3/9] ARM: HYP/non-sec: add a barrier after setting SCR.NS==1

[Christoffer Dall <christoffer.dall@linaro.org>]

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.zyngier@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.zyngier@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