On Thu, Sep 27, 2012 at 11:11 AM, Will Deacon [off-list ref] wrote:

On Sat, Sep 15, 2012 at 04:35:59PM +0100, Christoffer Dall wrote:

quoted

When the guest accesses I/O memory this will create data abort
exceptions and they are handled by decoding the HSR information
(physical address, read/write, length, register) and forwarding reads
and writes to QEMU which performs the device emulation.

Certain classes of load/store operations do not support the syndrome
information provided in the HSR and we therefore must be able to fetch
the offending instruction from guest memory and decode it manually.

We only support instruction decoding for valid reasonable MMIO operations
where trapping them do not provide sufficient information in the HSR (no
16-bit Thumb instructions provide register writeback that we care about).

The following instruciton types are NOT supported for MMIO operations
despite the HSR not containing decode info:
 - any Load/Store multiple
 - any load/store exclusive
 - any load/store dual
 - anything with the PC as the dest register

[...]

quoted

+
+/******************************************************************************
+ * Load-Store instruction emulation
+ *****************************************************************************/
+
+/*
+ * Must be ordered with LOADS first and WRITES afterwards
+ * for easy distinction when doing MMIO.
+ */
+#define NUM_LD_INSTR  9
+enum INSTR_LS_INDEXES {
+       INSTR_LS_LDRBT, INSTR_LS_LDRT, INSTR_LS_LDR, INSTR_LS_LDRB,
+       INSTR_LS_LDRD, INSTR_LS_LDREX, INSTR_LS_LDRH, INSTR_LS_LDRSB,
+       INSTR_LS_LDRSH,
+       INSTR_LS_STRBT, INSTR_LS_STRT, INSTR_LS_STR, INSTR_LS_STRB,
+       INSTR_LS_STRD, INSTR_LS_STREX, INSTR_LS_STRH,
+       NUM_LS_INSTR
+};
+
+static u32 ls_instr[NUM_LS_INSTR][2] = {
+       {0x04700000, 0x0d700000}, /* LDRBT */
+       {0x04300000, 0x0d700000}, /* LDRT  */
+       {0x04100000, 0x0c500000}, /* LDR   */
+       {0x04500000, 0x0c500000}, /* LDRB  */
+       {0x000000d0, 0x0e1000f0}, /* LDRD  */
+       {0x01900090, 0x0ff000f0}, /* LDREX */
+       {0x001000b0, 0x0e1000f0}, /* LDRH  */
+       {0x001000d0, 0x0e1000f0}, /* LDRSB */
+       {0x001000f0, 0x0e1000f0}, /* LDRSH */
+       {0x04600000, 0x0d700000}, /* STRBT */
+       {0x04200000, 0x0d700000}, /* STRT  */
+       {0x04000000, 0x0c500000}, /* STR   */
+       {0x04400000, 0x0c500000}, /* STRB  */
+       {0x000000f0, 0x0e1000f0}, /* STRD  */
+       {0x01800090, 0x0ff000f0}, /* STREX */
+       {0x000000b0, 0x0e1000f0}  /* STRH  */
+};
+
+static inline int get_arm_ls_instr_index(u32 instr)
+{
+       return kvm_instr_index(instr, ls_instr, NUM_LS_INSTR);
+}
+
+/*
+ * Load-Store instruction decoding
+ */
+#define INSTR_LS_TYPE_BIT              26
+#define INSTR_LS_RD_MASK               0x0000f000
+#define INSTR_LS_RD_SHIFT              12
+#define INSTR_LS_RN_MASK               0x000f0000
+#define INSTR_LS_RN_SHIFT              16
+#define INSTR_LS_RM_MASK               0x0000000f
+#define INSTR_LS_OFFSET12_MASK         0x00000fff

I'm afraid you're not going to thank me much for this, but it's high time we
unified the various instruction decoding functions we have under arch/arm/
and this seems like a good opportunity for that. For example, look at the
following snippets (there is much more in the files I list) in addition to
what you have:

I think it would be great if we had a set of unified decoding functions!

However, I think it's a shame if we can't merge KVM because we want to
clean up this code. There would be nothing in the way of breaking API
or anything like that preventing us from cleaning this up after the
code has been merged.

Please consider reviewing the code for correctness and consider if the
code could be merged as is.

On the other hand, if you or Dave or anyone else wants to create a set
of patches that cleans this up in a timely manner, I will be happy to
merge them for my code as well ;)

asm/ptrace.h
-------------
#define PSR_T_BIT       0x00000020
#define PSR_F_BIT       0x00000040
#define PSR_I_BIT       0x00000080
#define PSR_A_BIT       0x00000100
#define PSR_E_BIT       0x00000200
#define PSR_J_BIT       0x01000000
#define PSR_Q_BIT       0x08000000
#define PSR_V_BIT       0x10000000
#define PSR_C_BIT       0x20000000
#define PSR_Z_BIT       0x40000000
#define PSR_N_BIT       0x80000000

mm/alignment.c
--------------
#define LDST_I_BIT(i)   (i & (1 << 26))         /* Immediate constant   */
#define LDST_P_BIT(i)   (i & (1 << 24))         /* Preindex             */
#define LDST_U_BIT(i)   (i & (1 << 23))         /* Add offset           */
#define LDST_W_BIT(i)   (i & (1 << 21))         /* Writeback            */
#define LDST_L_BIT(i)   (i & (1 << 20))         /* Load                 */

kernel/kprobes*.c
-----------------
static void __kprobes
emulate_ldr(struct kprobe *p, struct pt_regs *regs)
{
        kprobe_opcode_t insn = p->opcode;
        unsigned long pc = (unsigned long)p->addr + 8;
        int rt = (insn >> 12) & 0xf;
        int rn = (insn >> 16) & 0xf;
        int rm = insn & 0xf;

kernel/opcodes.c
----------------
static const unsigned short cc_map[16] = {
        0xF0F0,                 /* EQ == Z set            */
        0x0F0F,                 /* NE                     */
        0xCCCC,                 /* CS == C set            */
        0x3333,                 /* CC                     */
        0xFF00,                 /* MI == N set            */
        0x00FF,                 /* PL                     */
        0xAAAA,                 /* VS == V set            */
        0x5555,                 /* VC                     */
        0x0C0C,                 /* HI == C set && Z clear */
        0xF3F3,                 /* LS == C clear || Z set */
        0xAA55,                 /* GE == (N==V)           */
        0x55AA,                 /* LT == (N!=V)           */
        0x0A05,                 /* GT == (!Z && (N==V))   */
        0xF5FA,                 /* LE == (Z || (N!=V))    */
        0xFFFF,                 /* AL always              */
        0                       /* NV                     */
};

kernel/swp_emulate.c
--------------------
#define EXTRACT_REG_NUM(instruction, offset) \
        (((instruction) & (0xf << (offset))) >> (offset))
#define RN_OFFSET  16
#define RT_OFFSET  12
#define RT2_OFFSET  0


There are also bits and pieces with the patching frameworks and module
relocations that could benefit from some code sharing. Now, I think Dave had
some ideas about moving a load of this code into a common disassembler under
arch/arm/ so it would be great to tie that in here and implement that for
load/store instructions. Then other users can augment the number of
supported instruction classes as and when it is required.