[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <AANLkTi=J2A1Qv6NGB1bsMr6d+1VFV26KEBQ=NJ=791s0@mail.gmail.com>
Date: Fri, 14 Jan 2011 11:23:39 -0800
From: Colin Cross <ccross@...roid.com>
To: Russell King - ARM Linux <linux@....linux.org.uk>
Cc: Catalin Marinas <catalin.marinas@....com>,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH] ARM: vfp: Fix up exception location in Thumb mode
On Fri, Jan 14, 2011 at 10:47 AM, Russell King - ARM Linux
<linux@....linux.org.uk> wrote:
> So... this incrementally on top of the previous patch (which I've
> reproduced below as there's a subtle comment change in there wrt IRQ
> state.)
>
> This means we have consistent state - both r2 and regs->ARM_pc always
> point to the next instruction to be executed in every case, which means
> its easy to understand and remember while reading through the code.
>
> diff -u b/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
> --- b/arch/arm/kernel/entry-armv.S
> +++ b/arch/arm/kernel/entry-armv.S
> @@ -499,10 +499,11 @@
> blo __und_usr_unknown
> 3: ldrht r0, [r4]
> add r2, r2, #2 @ r2 is PC + 2, make it PC + 4
> - orr r0, r0, r5, lsl #16
> + str r2, [sp, #S_PC] @ it's a 2x16bit instr, update
> + orr r0, r0, r5, lsl #16 @ regs->ARM_pc
> @
> @ r0 = the two 16-bit Thumb instructions which caused the exception
> - @ r2 = PC value for the following Thumb instruction (:= regs->ARM_pc+2)
> + @ r2 = PC value for the following Thumb instruction (:= regs->ARM_pc)
> @ r4 = PC value for the first 16-bit Thumb instruction
> @
> #else
>
> 8<-x-x-
>
> 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]
> #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)
> + @ r4 = PC value for the faulting instruction
> + @
> beq call_fpe
> +
> @ Thumb instruction
> #if __LINUX_ARM_ARCH__ >= 7
> + sub r4, r2, #2 @ Thumb instr at LR - 2
> 2:
> ARM( ldrht r5, [r4], #2 )
> THUMB( ldrht r5, [r4] )
> @@ -492,18 +500,19 @@ __und_usr:
> 3: ldrht r0, [r4]
> add r2, r2, #2 @ r2 is PC + 2, make it PC + 4
> orr r0, r0, r5, lsl #16
> + @
> + @ r0 = the two 16-bit Thumb instructions which caused the exception
> + @ r2 = PC value for the following Thumb instruction (:= regs->ARM_pc+2)
> + @ r4 = PC value for the first 16-bit Thumb instruction
> + @
> #else
> b __und_usr_unknown
> #endif
> - UNWIND(.fnend )
> + UNWIND(.fnend)
> ENDPROC(__und_usr)
>
> - @
> - @ fallthrough to call_fpe
> - @
> -
> /*
> - * The out of line fixup for the ldrt above.
> + * The out of line fixup for the ldrt instructions above.
> */
> .pushsection .fixup, "ax"
> 4: mov pc, r9
> @@ -534,11 +543,12 @@ ENDPROC(__und_usr)
> * NEON handler code.
> *
> * Emulators may wish to make use of the following registers:
> - * r0 = instruction opcode.
> - * r2 = PC+4
> + * r0 = instruction opcode (32-bit ARM or two 16-bit Thumb)
> + * r2 = PC value to resume execution after successful emulation
> * r9 = normal "successful" return address
> - * r10 = this threads thread_info structure.
> + * r10 = this threads thread_info structure
> * lr = unrecognised instruction return address
> + * IRQs disabled, FIQs enabled.
> */
> @
> @ Fall-through from Thumb-2 __und_usr
> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
> index ee57640..eeb9250 100644
> --- a/arch/arm/kernel/traps.c
> +++ b/arch/arm/kernel/traps.c
> @@ -347,9 +347,9 @@ asmlinkage void __exception do_undefinstr(struct pt_regs *regs)
> void __user *pc;
>
> /*
> - * According to the ARM ARM, PC is 2 or 4 bytes ahead,
> - * depending whether we're in Thumb mode or not.
> - * Correct this offset.
> + * According to the ARM ARM, the PC is 2 or 4 bytes ahead
> + * depending on Thumb mode. Correct this offset so that
> + * regs->ARM_pc points at the faulting instruction.
> */
> regs->ARM_pc -= correction;
>
> diff --git a/arch/arm/vfp/entry.S b/arch/arm/vfp/entry.S
> index 4fa9903..2bf6089 100644
> --- a/arch/arm/vfp/entry.S
> +++ b/arch/arm/vfp/entry.S
> @@ -19,6 +19,15 @@
> #include <asm/vfpmacros.h>
> #include "../kernel/entry-header.S"
>
> +@ VFP entry point.
> +@
> +@ r0 = instruction opcode (32-bit ARM or two 16-bit Thumb)
> +@ r2 = PC value to resume execution after successful emulation
> +@ r9 = normal "successful" return address
> +@ r10 = this threads thread_info structure
> +@ lr = unrecognised instruction return address
> +@ IRQs disabled.
> +@
> ENTRY(do_vfp)
> #ifdef CONFIG_PREEMPT
> ldr r4, [r10, #TI_PREEMPT] @ get preempt count
> diff --git a/arch/arm/vfp/vfphw.S b/arch/arm/vfp/vfphw.S
> index 9897dcf..7292921 100644
> --- a/arch/arm/vfp/vfphw.S
> +++ b/arch/arm/vfp/vfphw.S
> @@ -61,13 +61,13 @@
>
> @ VFP hardware support entry point.
> @
> -@ r0 = faulted instruction
> -@ r2 = faulted PC+4
> -@ r9 = successful return
> +@ r0 = instruction opcode (32-bit ARM or two 16-bit Thumb)
> +@ r2 = PC value to resume execution after successful emulation
> +@ r9 = normal "successful" return address
> @ r10 = vfp_state union
> @ r11 = CPU number
> -@ lr = failure return
> -
> +@ lr = unrecognised instruction return address
> +@ IRQs enabled.
> ENTRY(vfp_support_entry)
> DBGSTR3 "instr %08x pc %08x state %p", r0, r2, r10
I tested copying r2 to regs->ARM_pc like this patch does, and it fixes
my test case. Could this second patch go first so it can be applied
to stable?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists