Re: [PATCH] EmbeddedPkg/GdbDebugAgent ARM: use modern dialect for ldm/stm instructions

[Leif Lindholm <leif.lindholm@linaro.org>]

On Wed, Dec 12, 2018 at 03:49:07PM +0100, Ard Biesheuvel wrote:
> On Wed, 12 Dec 2018 at 15:41, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >
> > On Wed, Dec 12, 2018 at 01:45:17PM +0100, Ard Biesheuvel wrote:
> > > > > > > @@ -198,9 +199,9 @@ ASM_PFX(AsmCommonExceptionEntry):
> > > > > > >    and       R3, R1, #0x1f           @ Check CPSR to see if User or System Mode
> > > > > > >    cmp       R3, #0x1f               @ if ((CPSR == 0x10) || (CPSR == 0x1df))
> > > > > > >    cmpne     R3, #0x10               @
> > > > > > > -  stmeqed   R2, {lr}^               @   save unbanked lr
> > > > > > > +  stmdaeq   R2, {lr}^               @   save unbanked lr
> > > >
> > > > Then again, looking closer at these instructions, they're not really
> > > > doing stack operations. Just (ab)using the instruction to get at the
> > > > banked User mode LR from a different mode. So
> > > > a) The symmetry thing doesn't really apply, so the ED is actively
> > > >    misleading here.
> > > > b) This could trivially be changed to use FD anyway, just setting R2's
> > > >    offset from PC to #0x34 :)
> > > >
> > >
> > > Actually, since there is no writeback, what is the point of the
> > > decrement-after? Can't we just drop it?
> >
> > Possibly picked specifically to _not_ be mistaken for a stack
> > operation. In this scenario The DA just means don't add 4 to R2 before
> > using it as address.
> >
> 
> Indeed. But in the load counterpart below, it means increment-before,
> so it will restore lr from the wrong address.

I do believe you are correct :)
Let's nuke this one.

/
    Leif