[PATCH] ARM: vfp: Fix up exception location in Thumb mode

On Wed, Jan 26, 2011 at 10:11 PM, Colin Cross [off-list ref] wrote:

On Wed, Jan 26, 2011 at 3:26 AM, Russell King - ARM Linux
[off-list ref] wrote:

quoted

On Tue, Jan 25, 2011 at 03:33:24PM -0800, Colin Cross wrote:

quoted

I think there is an additional change needed to
__und_usr_unknown/do_undefinstr. ?do_undefinstr, which gets called
directly in __und_usr as well as by mov pc, lr, expects regs->ARM_pc
to be the fault address, and not the next PC, and gets called for 2 or
4 byte instructions.

It expects it to be the next PC:

asmlinkage void __exception do_undefinstr(struct pt_regs *regs)
{
? ? ? ?unsigned int correction = thumb_mode(regs) ? 2 : 4;
? ? ? ?unsigned int instr;
? ? ? ?siginfo_t info;
? ? ? ?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.
? ? ? ? */
? ? ? ?regs->ARM_pc -= correction;

We expect the PC to be pointing at the next instruction to be executed.
This is the value of the PC saved by the CPU when entering the exception.
We correct the PC by four bytes for ARM mode to point at the previously
executed instruction.

For 16-bit Thumb mode, the PC is again pointing at the next instruction
to be executed, and this is the value saved by the CPU. ?So we correct
the PC by two bytes as that is the Thumb instruction size.

The problem comes with T2, where we advance the saved PC by two bytes
if the instruction was 32-bit such that it again points at the next
instruction to be executed. ?This is where the problem comes in because
we have two different chunks of code with completely different
expectations.

All do_undefinstr will do with the correction is subtract it off so
that pc points to the faulting instruction instead of the next
instruction, and calls any registered handlers. ?VFP_bounce sometimes
subtracts off 4 (it doesn't care about T2 mode there, because T2 VFP
instructions are all 32 bit), and sometimes leaves pc pointing at the
next instruction. ?If both were modified to expect the faulting
instruction's pc, do_undefinstr would leave pc unmodified, and
VFP_bounce would add 4 in the opposite cases from where it subtracts 4
now.

iwmmxt_task_enable and crunch_task_enable can also get called from __und_usr.

Currently, iwmmxt_task_enable will either end up in do_undefinstr
through mov pc, lr, or it will subtract 4 from the pc register value
and return through ret_from_exception. ?With the change above, it
would not need to modify the pc value at all.

crunch_task_enable is based on iwmmxt_task_enable, and works exactly the same.

quoted

Maybe we need to pass in the correction factor to do_undefinstr instead.

That just makes it even more complicated, the correction factor has to
be tracked through every code path.

RFC patch to follow.

Missed one case.  do_fpe calls into nwfpe_enter (I don't see any other
users of fp_enter), which assumes the PC in the registers on the stack
is faulting PC + 4, ignores r2, and uses r0 as the first instruction
to emulate.  It will need to be slightly modified in my patch.