On Mon, 21 Mar 2011, Tixy wrote:
On Mon, 2011-03-21 at 14:56 -0400, Nicolas Pitre wrote:
I'd suggest:
insn = ((u16 *)pc)[0]; if (insn_is_wide(insn)) { insn <<= 16; insn |= ((u16 *)pc)[1]; }
effectively using a big endian representation, but which is similar to ARM mode instructions, and which would make sense regardless of the instruction mode/width when printed.
This is what I've done in the kprobe code that handles thumb, except that I've also shifted 16-bit thumb instructions into the top half. This was so we always know where the first half-word is and can test it for 16/32-bit size with "if(first_half<0xe800)".
However, I've just realised that I didn't need to do this, with your example "if(inst<0xe800)" still works; so my other patches can be simplified :-)
Another reason to keep the 16-bit insn in the low half-word is to simplify printing.
printk("instruction: 0x%04x\n", insn);
would display something sensible regardless of the mode or width (no fancy spacing in the 32-bit Thumb2 case but that's not so bad).
We could even just test the upper 16-bits for non-zero to indicate 32-bit. E.g. to store a thumb instruction:
u16* pc = (u16 *)addr; u16 half = inst>>16; if (half) *pc++ = half; *pc++ = half&0xffff;
which should compile to just three or four CPU instructions.
Indeed. And given the pointer is u16*, you may omit the 0xffff mask.
Nicolas