On Mon, Feb 03, 2014 at 05:50:20PM +0100, Andrew Jones wrote:
> On Sat, Feb 01, 2014 at 06:29:27PM -0800, Christoffer Dall wrote:
> > On Tue, Jan 21, 2014 at 05:22:03PM +0100, Andrew Jones wrote:
> > > Add support for tests to use exception handlers.
> > >
> > > v2 -> v3:
> > > - squashed in 'arm: Simplify exceptions_init in cstart.S' from
> > > Christoffer Dall
> > > - suggested function name changes and comment additions [Christoffer Dall]
> > > - fix a bug with stack restore from usr mode exceptions that Christoffer
> > > pointed out. Add a get_sp() accessor too.
> >
> > hmmm, how did you address this, the only change I can see is using r6
> > instead of r2, which I'm not sure how changes anything.
>
> It also adds
>
> /* make sure we restore sp_svc and lr_svc on mode change */
> str r6, [sp, #S_SP]
> str lr, [sp, #S_LR]
>
> lower down. Needed to switch to r6 from r2 for that
>
> >
> > The problem is that your ldmia is replacing the usr sp, not the svc sp,
> > so maybe you need to do ldmia sp!, [ r0 - pc ]^ (don't remember if that
> > particular combination works) or you need to do something more fancy...
>
> I'm not sure about the magic ! + ^ stuff, but what I've done does fix
> the problem. I tested before/after the fix (actually that's why I also
> added the get_sp).
>
Ah, I know, my original comment was wrong, what you were in fact doing
was restoring the user sp to the svc reg, the write to the CPSR from the
SPSR happens after restoring all the other regs (except the PC)
according to the ldm exception return pseudocode.
ok, I think it's correct now, except for the bit about the lr, see
below.
> > > diff --git a/arm/cstart.S b/arm/cstart.S
> > > index 4de04b62c28c2..74674fd41c1b3 100644
> > > --- a/arm/cstart.S
> > > +++ b/arm/cstart.S
> > > @@ -1,3 +1,7 @@
> > > +#define __ASSEMBLY__
> > > +#include "arm/asm-offsets.h"
> > > +#include "arm/ptrace.h"
> > > +#include "arm/cp15.h"
> > >
> > > .arm
> > >
> > > @@ -10,6 +14,13 @@ start:
> > > * See the kernel doc Documentation/arm/Booting
> > > */
> > > ldr sp, =stacktop
> > > + push {r0-r3}
> > > +
> > > + /* set up vector table and mode stacks */
> > > + bl exceptions_init
> > > +
> > > + /* complete setup */
> > > + pop {r0-r3}
> > > bl setup
> > >
> > > /* start the test */
> > > @@ -20,9 +31,166 @@ start:
> > > bl exit
> > > b halt
> > >
> > > +.macro set_mode_stack mode, stack
> > > + add \stack, #S_FRAME_SIZE
> > > + msr cpsr_c, #(\mode | PSR_I_BIT | PSR_F_BIT)
> > > + mov sp, \stack
> > > +.endm
> > > +
> > > +exceptions_init:
> > > + mrc p15, 0, r2, c1, c0, 0 @ read SCTLR
> > > + bic r2, #CR_V @ SCTLR.V := 0
> > > + mcr p15, 0, r2, c1, c0, 0 @ write SCTLR
> > > + ldr r2, =vector_table
> > > + mcr p15, 0, r2, c12, c0, 0 @ write VBAR
> > > +
> > > + mrs r2, cpsr
> > > + ldr r1, =exception_stacks
> > > + /* first frame for svc mode */
> > > + set_mode_stack UND_MODE, r1
> > > + set_mode_stack ABT_MODE, r1
> > > + set_mode_stack IRQ_MODE, r1
> > > + set_mode_stack FIQ_MODE, r1
> > > + msr cpsr_cxsf, r2 @ back to svc mode
> > > + mov pc, lr
> > > +
> > > .text
> > >
> > > .globl halt
> > > halt:
> > > 1: wfi
> > > b 1b
> > > +
> > > +/*
> > > + * Vector stubs.
> > > + * Simplified version of the Linux kernel implementation
> > > + * arch/arm/kernel/entry-armv.S
> > > + *
> > > + * Each mode has an S_FRAME_SIZE sized stack initialized
> > > + * in exceptions_init
> > > + */
> > > +.macro vector_stub, name, vec, mode, correction=0
> > > +.align 5
> > > +vector_\name:
> > > +.if \correction
> > > + sub lr, lr, #\correction
> > > +.endif
> > > + /*
> > > + * Save r0, r1, lr_<exception> (parent PC)
> > > + * and spsr_<exception> (parent CPSR)
> > > + */
> > > + str r0, [sp, #S_R0]
> > > + str r1, [sp, #S_R1]
> > > + str lr, [sp, #S_PC]
> > > + mrs r0, spsr
> > > + str r0, [sp, #S_PSR]
> > > +
> > > + /* Prepare for SVC32 mode. */
> > > + mrs r0, cpsr
> > > + bic r0, #MODE_MASK
> > > + orr r0, #SVC_MODE
> > > + msr spsr_cxsf, r0
> > > +
> > > + /* Branch to handler in SVC mode */
> > > + mov r0, #\vec
> > > + mov r1, sp
> > > + ldr lr, =vector_common
> > > + movs pc, lr
> > > +.endm
> > > +
> > > +vector_stub rst, 0, UND_MODE
> > > +vector_stub und, 1, UND_MODE
> > > +vector_stub pabt, 3, ABT_MODE, 4
> > > +vector_stub dabt, 4, ABT_MODE, 8
> > > +vector_stub irq, 6, IRQ_MODE, 4
> > > +vector_stub fiq, 7, FIQ_MODE, 4
> > > +
> > > +.align 5
> > > +vector_svc:
> > > + /*
> > > + * Save r0, r1, lr_<exception> (parent PC)
> > > + * and spsr_<exception> (parent CPSR)
> > > + */
> > > + push { r1 }
> > > + ldr r1, =exception_stacks
> > > + str r0, [r1, #S_R0]
> > > + pop { r0 }
> > > + str r0, [r1, #S_R1]
> > > + str lr, [r1, #S_PC]
> > > + mrs r0, spsr
> > > + str r0, [r1, #S_PSR]
> > > +
> > > + /*
> > > + * Branch to handler, still in SVC mode.
> > > + * r0 := 2 is the svc vector number.
> > > + */
> > > + mov r0, #2
> > > + ldr lr, =vector_common
> > > + mov pc, lr
> > > +
> > > +vector_common:
> > > + /* make room for pt_regs */
> > > + sub sp, #S_FRAME_SIZE
> > > + tst sp, #4 @ check stack alignment
> > > + subne sp, #4
> > > +
> > > + /* store registers r0-r12 */
> > > + stmia sp, { r0-r12 } @ stored wrong r0 and r1, fix later
> > > +
> > > + /* get registers saved in the stub */
> > > + ldr r2, [r1, #S_R0] @ r0
> > > + ldr r3, [r1, #S_R1] @ r1
> > > + ldr r4, [r1, #S_PC] @ lr_<exception> (parent PC)
> > > + ldr r5, [r1, #S_PSR] @ spsr_<exception> (parent CPSR)
> > > +
> > > + /* fix r0 and r1 */
> > > + str r2, [sp, #S_R0]
> > > + str r3, [sp, #S_R1]
> > > +
> > > + /* store sp_svc, if we were in usr mode we'll fix this later */
> > > + add r6, sp, #S_FRAME_SIZE
> > > + addne r6, #4 @ stack wasn't aligned
> > > + str r6, [sp, #S_SP]
> > > +
> > > + str lr, [sp, #S_LR] @ store lr_svc, fix later for usr mode
> > > + str r4, [sp, #S_PC] @ store lr_<exception>
> > > + str r5, [sp, #S_PSR] @ store spsr_<exception>
> > > +
> > > + /* set ORIG_r0 */
> > > + mov r2, #-1
> > > + str r2, [sp, #S_OLD_R0]
> > > +
> > > + /* if we were in usr mode then we need sp_usr and lr_usr instead */
> > > + and r1, r5, #MODE_MASK
> > > + cmp r1, #USR_MODE
> > > + bne 1f
> > > + add r1, sp, #S_SP
> > > + stmia r1, { sp,lr }^
> > > +
> > > + /* Call the handler. r0 is the vector number, r1 := pt_regs */
> > > +1: mov r1, sp
> > > + bl do_handle_exception
> > > +
> > > + /* make sure we restore sp_svc and lr_svc on mode change */
> > > + str r6, [sp, #S_SP]
> > > + str lr, [sp, #S_LR]
I don't think store of lr is necessary, lr was just overwritten by the
bl instruction above, and it will get overwritten at exception entry to
SVC mode again.
> > > +
> > > + /* return from exception */
> > > + msr spsr_cxsf, r5
> > > + ldmia sp, { r0-pc }^
> > > +
> > > +.align 5
> > > +vector_addrexcptn:
> > > + b vector_addrexcptn
> > > +
> > > +.section .text.ex
> > > +.align 5
> > > +vector_table:
> > > + b vector_rst
> > > + b vector_und
> > > + b vector_svc
> > > + b vector_pabt
> > > + b vector_dabt
> > > + b vector_addrexcptn @ should never happen
> > > + b vector_irq
> > > + b vector_fiq
[...]
Thanks,
-Christoffer
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
