lkml.org 
[lkml]   [2011]   [Jan]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    Subject[PATCH] ARM: vfp: Fix up exception location in Thumb mode
    From
    On Saturday, 15 January 2011, Russell King - ARM Linux
    <linux@arm.linux.org.uk> wrote:
    > On Sat, Jan 15, 2011 at 03:31:04PM +0000, Catalin Marinas wrote:
    >> On 14 January 2011 17:30, Russell King - ARM Linux
    >> <linux@arm.linux.org.uk> wrote:
    >> > diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
    >> > index 2b46fea..5876eec 100644
    >> > --- a/arch/arm/kernel/entry-armv.S
    >> > +++ b/arch/arm/kernel/entry-armv.S
    >> > @@ -461,27 +461,35 @@ ENDPROC(__irq_usr)
    >> >        .align  5
    >> >  __und_usr:
    >> >        usr_entry
    >> > -
    >> > -       @
    >> > -       @ fall through to the emulation code, which returns using r9 if
    >> > -       @ it has emulated the instruction, or the more conventional lr
    >> > -       @ if we are to treat this as a real undefined instruction
    >> >        @
    >> > -       @  r0 - instruction
    >> > +       @ The emulation code returns using r9 if it has emulated the
    >> > +       @ instruction, or the more conventional lr if we are to treat
    >> > +       @ this as a real undefined instruction
    >> >        @
    >> >        adr     r9, BSYM(ret_from_exception)
    >> >        adr     lr, BSYM(__und_usr_unknown)
    >> > +       @
    >> > +       @ r2 = regs->ARM_pc, which is either 2 or 4 bytes ahead of the
    >> > +       @ faulting instruction depending on Thumb mode.
    >> > +       @ r3 = regs->ARM_cpsr
    >> > +       @
    >> >        tst     r3, #PSR_T_BIT                  @ Thumb mode?
    >> > -       itet    eq                              @ explicit IT needed for the 1f label
    >> > +       itttt   eq                              @ explicit IT needed for the 1f label
    >> >        subeq   r4, r2, #4                      @ ARM instr at LR - 4
    >> > -       subne   r4, r2, #2                      @ Thumb instr at LR - 2
    >> >  1:     ldreqt  r0, [r4]
    >>
    >> The itttt above should just be itt. The reveq is conditionally
    >> compiled and beq doesn't necessarily need one.
    >
    > It's a reveq, so I thought we should cover all the instructions with
    > an 'eq' conditional for thumb.

    If the it instruction doesn't cover all instructions, gas generates
    some more its. But in this case, for little endian, the it instruction
    covers more since reveq isn't included and having the beq not last in
    the block I think is unpredictable. If you really want to optimise the
    big endian case not to have an additional it generated by gas, you can
    write ittt so that beq is included with little endian but not with big
    endian. I wouldn't bother much for an extra it anyway.

    >> >  #ifdef CONFIG_CPU_ENDIAN_BE8
    >> >        reveq   r0, r0                          @ little endian instruction
    >> >  #endif
    >> > +       @
    >> > +       @ r0 = 32-bit ARM instruction which caused the exception
    >> > +       @ r2 = PC value for the following instruction (:= regs->ARM_pc)
    >>
    >> Is r2 here always the PC value following instruction? If the Thumb
    >> instruction was 32-bit, it just points in the middle of the faulting
    >> instruction.
    >
    > Is the T bit ever zero in this case?  The code here is:
    >
    >         tst     r3, #PSR_T_BIT
    >         subeq   r4, r2, #4
    > 1:      ldreqt  r0, [r4]
    >         reveq   r0, r0
    >         beq     call_fpe

    You can have the T bit set but the instruction a 32-bit Thumb in which
    case r2 is in the middle of such instruction rather than the next.
    Unless you only refer to the ARM mode, in which case the comment is
    fine.

    > So, if !T, then we subtract 4 and load the instruction (which was the
    > faulting instruction).  So r2 is the following instruction.
    >
    > Ah, maybe you're getting confused by the comment.  Should we put
    > an 'eq' suffix on the end of each line? ;)

    Maybe mention that this is ARM. I think documenting this code is
    difficult anyway. I found myself not reading the comments at all when
    revisiting this code :) but they may be useful for others.


    --
    Catalin
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2011-01-16 12:51    [W:4.346 / U:0.032 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site