On 05/05/2015 10:19 AM, Peter Maydell wrote: > On 5 May 2015 at 05:45, Peter Crosthwaite <crosthwaitepe...@gmail.com> wrote: >> Add the ARM specific disassembly flags setup, so ARM can be correctly >> disassembled from the monitor. >> >> Signed-off-by: Peter Crosthwaite <crosthwaite.pe...@gmail.com> >> --- >> monitor.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/monitor.c b/monitor.c >> index d831d98..9d9f1e2 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -1217,6 +1217,17 @@ static void memory_dump(Monitor *mon, int count, int >> format, int wsize, >> int flags; >> flags = 0; >> env = mon_get_cpu(); >> +#ifdef TARGET_ARM >> + if (env->thumb) { >> + flags |= 1; >> + } >> + if (env->bswap_code) { >> + flags |= 2; >> + } >> + if (env->aarch64) { >> + flags |= 4; >> + } >> +#endif > > monitor.c has no business poking around in the CPU state > internals like this... You probably want a CPU method > get_disas_flags() or something. > > -- PMM >
While this patch set does improve the current dismal state of affairs, I think the ideal solution is a cpu method that takes care of all the disassembly info setup. Indeed, the flags setup becomes less obscure when, instead of #ifdef TARGET_I386 if (wsize == 2) { flags = 1; } else if (wsize == 4) { flags = 0; } else { /* as default we use the current CS size */ flags = 0; if (env) { #ifdef TARGET_X86_64 if ((env->efer & MSR_EFER_LMA) && (env->segs[R_CS].flags & DESC_L_MASK)) flags = 2; else #endif if (!(env->segs[R_CS].flags & DESC_B_MASK)) flags = 1; } } in one place and #if defined(TARGET_I386) if (flags == 2) { s.info.mach = bfd_mach_x86_64; } else if (flags == 1) { s.info.mach = bfd_mach_i386_i8086; } else { s.info.mach = bfd_mach_i386_i386; } print_insn = print_insn_i386; in another, we merge the two so that we get s.info.mach = bfd_mach_i386_i8086; if (env->hflags & (1U << HF_CS32_SHIFT)) { s.info.mach = bfd_mach_i386_i386; } #ifdef TARGET_X86_64 if (env->hflags & (1U << HF_CS64_SHIFT)) { s.info.mach = bfd_mach_x86_64; } #endif I'm not sure what the right interface for this is, exactly. But I'd think that the CPUDebug structure would be initialized in the caller -- target or monitor -- while the mach and whatnot get filled in by the cpu hook. Maybe even have the cpu hook return the print_insn function to use. r~