On Wed, Nov 28, 2018 at 12:40:23 +0000, Alex Bennée wrote: > I was envisioning something more like the following so all the plugin > gubins could be kept in the core code: (snip) > static inline uint32_t arm_ldl_code(CPUARMState *env, target_ulong addr, > bool sctlr_b) > { > - uint32_t insn = cpu_ldl_code(env, addr); > - if (bswap_code(sctlr_b)) { > - return bswap32(insn); > - } > - return insn; > + return translator_ld32(env, addr, bswap_code(sctlr_b)); > } > > /* Ditto, for a halfword (Thumb) instruction */ > static inline uint16_t arm_lduw_code(CPUARMState *env, target_ulong addr, > bool sctlr_b) > { > - uint16_t insn; > #ifndef CONFIG_USER_ONLY > /* In big-endian (BE32) mode, adjacent Thumb instructions have been > swapped > within each word. Undo that now. */ > @@ -46,11 +40,7 @@ static inline uint16_t arm_lduw_code(CPUARMState *env, > target_ulong addr, > addr ^= 2; > } > #endif > - insn = cpu_lduw_code(env, addr); > - if (bswap_code(sctlr_b)) { > - return bswap16(insn); > - } > - return insn; > + return translator_ld16(env, addr, bswap_code(sctlr_b)); > }
I like this, thanks. However, for Thumb I think we still need to call qemu_plugin_insn_append directly: @@ -13304,11 +13306,16 @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) insn = arm_lduw_code(env, dc->pc, dc->sctlr_b); is_16bit = thumb_insn_is_16bit(dc, insn); dc->pc += 2; - if (!is_16bit) { + if (is_16bit) { + uint16_t insn16 = insn; + + qemu_plugin_insn_append(tcg_ctx->plugin_insn, &insn16, sizeof(insn16)); + } else { uint32_t insn2 = arm_lduw_code(env, dc->pc, dc->sctlr_b); insn = insn << 16 | insn2; dc->pc += 2; + qemu_plugin_insn_append(tcg_ctx->plugin_insn, &insn, sizeof(insn)); } Otherwise we might mess up the contents of 32-bit insns. Thanks, E.