Re: [Xen-devel] [PATCH 5/7] xen/arm32: Invalidate BTB on guest exit for Cortex A17 and 12

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 25 Jan 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 25/01/18 01:02, Stefano Stabellini wrote:
> > On Fri, 19 Jan 2018, Julien Grall wrote:
> > > In order to avoid aliasing attackes agains the branch predictor, let's
> > > invalidate the BTB on guest exist. This is made complicated by the fact
> > > that we cannot take a branch invalidating the BTB.
> > > 
> > > This is based on the first version posrted by Marc Zyngier on Linux-arm
> > > mailing list (see [1]).
> > > 
> > > This is part of XSA-254.
> > > 
> > > Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> > > Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> > > 
> > > [1] https://www.spinics.net/lists/arm-kernel/msg627032.html
> > > ---
> > >   xen/arch/arm/arm32/entry.S | 55
> > > ++++++++++++++++++++++++++++++++++++++++++++++
> > >   xen/arch/arm/cpuerrata.c   | 19 ++++++++++++++++
> > >   2 files changed, 74 insertions(+)
> > > 
> > > diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
> > > index 54a1733f87..c6ec0aa399 100644
> > > --- a/xen/arch/arm/arm32/entry.S
> > > +++ b/xen/arch/arm/arm32/entry.S
> > > @@ -160,6 +160,61 @@ GLOBAL(hyp_traps_vector)
> > >           b trap_irq                      /* 0x18 - IRQ */
> > >           b trap_fiq                      /* 0x1c - FIQ */
> > >   +        .align 5
> > > +GLOBAL(hyp_traps_vector_bp_inv)
> > > +        /*
> > > +         * We encode the exception entry in the bottom 3 bits of
> > > +         * SP, and we have to guarantee to be 8 bytes aligned.
> > > +         */
> > > +        add sp, sp, #1                  /* Reset            7 */
> > > +        add sp, sp, #1                  /* Undef            6 */
> > > +        add sp, sp, #1                  /* Hypervisor Call  5 */
> > > +        add sp, sp, #1                  /* Prefetch abort   4 */
> > > +        add sp, sp, #1                  /* Data abort       3 */
> > > +        add sp, sp, #1                  /* Hypervisor       2 */
> > > +        add sp, sp, #1                  /* IRQ              1 */
> > > +        nop                             /* FIQ              0 */
> > 
> > Clever! Things that you don't read every day :-)
> 
> Thanks Marc for the idea :).
> 
> > 
> > 
> > > +        mcr      p15, 0, r0, c7, c5, 6       /* BPIALL */
> > > +        isb
> > > +
> > > +        /*
> > > +         * As we cannot use any temporary registers and cannot
> > > +         * clobber SP, we can decode the exception entry using
> > > +         * an unrolled binary search.
> > > +         */
> > > +        tst sp, #4
> > > +        bne 1f
> > > +
> > > +        tst sp, #2
> > > +        bne 3f
> > > +
> > > +        tst sp, #1
> > > +        bic sp, sp, #0x7
> > > +        bne trap_irq
> > > +        b   trap_fiq
> > 
> > I might be confused, but this is the case where sp == 0x7, right?
> > Shouldn't we have b trap_reset here?
> > 
> > Similarly the branch just above corresponds to 0x6, which should be
> > bne trap_undefined_instruction.
> > 
> > What am I getting wrong?
> 
> The tst instruction performs a bitwise AND on a register value (here sp). The
> result will be used to update the condition flags.
> 
> So with tst sp, #4 the result will either be 0x100 or 0x000. The former will
> clear Z flag while the latter set Z flag.
> 
> This means that bne will branch only when bit 2 is set. So the only way to end
> here is because the first 3-bit are equal to 0x00X. This corresponds to
> IRQ/FIQ vector.

I got it the other way around, thanks for the explanation :-)

Signed-off-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel