Re: [PATCH 17/17] arm: vectors support

Andrew Jones
2014-02-04T07:12:24Z

On Mon, Feb 03, 2014 at 01:19:26PM -0800, Christoffer Dall wrote:
> 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.

I agree the lr restore is unnecessary, and I didn't test anything related
to that. I threw it in to balance the switch to sp_usr,lr_usr above, but
can replace that balancing with a comment instead.