Re: [PATCH 9/9] arm: add vectors support

[Andrew Jones <drjones@redhat.com>]

On Thu, Oct 17, 2013 at 11:58:43AM -0700, Christoffer Dall wrote:
> On Thu, Oct 17, 2013 at 12:38:59PM +0200, Andrew Jones wrote:
> > On Wed, Oct 16, 2013 at 06:06:42PM -0700, Christoffer Dall wrote:
> > > > +++ b/arm/cstart.S
> > > > @@ -1,5 +1,6 @@
> > > >  
> > > >  #define CR_B	(1 << 7)
> > > > +#define CR_V	(1 << 13)
> > > >  
> > > >  .arm
> > > >  
> > > > @@ -12,6 +13,13 @@ start:
> > > >  	push	{ r0-r3 }		@push r3 too for 8-byte alignment
> > > >  
> > > >  	mrc	p15, 0, r8, c1, c0, 0	@r8 = sctrl
> > > > +
> > > > +	/* set up vector table */
> > > > +	bic	r8, #CR_V		@sctrl.V = 0
> > > > +	mcr	p15, 0, r8, c1, c0, 0
> > > > +	ldr	r0, =vector_table	@vbar = vector_table
> > > > +	mcr     p15, 0, r0, c12, c0, 0
> > > > +
> > > >  	bl	get_endianness
> > > >  	bl	io_init
> > > >  
> > > > @@ -41,6 +49,44 @@ halt:
> > > >  1:	wfi
> > > >  	b	1b
> > > >  
> > > > +vector_common:
> > > > +	add	r2, sp, #(14 * 4)
> > > 
> > > this looks weird, what are you pointing to here?
> > 
> > What sp was at the time of exception. So if you look at ex_regs->sp,
> > then you'll see what sp was when the exception occurred, not that plus
> > what we're pushing on now for the handler.
> > 
> 
> Hmmm, so you're assuming that all exceptions will be taken from SVC
> mode?  I assume we will run tests in more than SVC mode, no?
> 
> Also note that the lr you're pushing here is not the lr at the time the
> exception occurs, but the return address from the exception.  If the SVC
> instruction is executed from SVC mode, the original lr is lost iirc, and
> the caller needs to save it.  If you're from user mode, something like
> 
>     stm sp, {r0-lr}^
> 
> will take care of this for you, and if you're from svc
> mode, you may want consider doing something like
> 
>     push {sp, lr}
>     push {r0-r12}
> 
> instead (assuming this is only ever compiled in ARM mode, not Thumb2, in
> which case the whole thing gets more complicated.

I think the lr pushing should be ok. That part is done in the macro that
all vectors start with.  It got snipped from this mail, so here it is

.macro m_vector, v
	push    { r0-r12,lr }
	mov     r1, \v
	b       vector_common
.endm

I realize it may still not be correct, just as the calculated sp
may not be correct, depending on which mode,vector combo is used,
but I was expecting to have different paths in the C code for
fixing it up. Although that said, I haven't thought about it a
bunch yet, so maybe it won't work that way...

drew