[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 6/7] monitor: "i": Add ARM specifics
From: |
Richard Henderson |
Subject: |
Re: [Qemu-devel] [PATCH 6/7] monitor: "i": Add ARM specifics |
Date: |
Tue, 05 May 2015 10:43:30 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 |
On 05/05/2015 10:19 AM, Peter Maydell wrote:
> On 5 May 2015 at 05:45, Peter Crosthwaite <address@hidden> wrote:
>> Add the ARM specific disassembly flags setup, so ARM can be correctly
>> disassembled from the monitor.
>>
>> Signed-off-by: Peter Crosthwaite <address@hidden>
>> ---
>> 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~
- [Qemu-devel] [PATCH 3/7] disas: cris: Fix 0 buffer length case, (continued)
- [Qemu-devel] [PATCH 3/7] disas: cris: Fix 0 buffer length case, Peter Crosthwaite, 2015/05/05
- [Qemu-devel] [PATCH 1/7] disas: Create factored out fn for monitor and target disas, Peter Crosthwaite, 2015/05/05
- [Qemu-devel] [PATCH 4/7] disas: cris: Migrate setup to common code, Peter Crosthwaite, 2015/05/05
- [Qemu-devel] [PATCH 5/7] disas: arm-a64: Make printfer and stream variable, Peter Crosthwaite, 2015/05/05
- [Qemu-devel] [PATCH 6/7] monitor: "i": Add ARM specifics, Peter Crosthwaite, 2015/05/05
- Re: [Qemu-devel] [PATCH 6/7] monitor: "i": Add ARM specifics, Claudio Fontana, 2015/05/05
- Re: [Qemu-devel] [PATCH 6/7] monitor: "i": Add ARM specifics, Peter Maydell, 2015/05/05
- Re: [Qemu-devel] [PATCH 6/7] monitor: "i": Add ARM specifics, Peter Crosthwaite, 2015/05/06
- Re: [Qemu-devel] [PATCH 6/7] monitor: "i": Add ARM specifics, Richard Henderson, 2015/05/06
- Re: [Qemu-devel] [PATCH 6/7] monitor: "i": Add ARM specifics, Paolo Bonzini, 2015/05/06
- Re: [Qemu-devel] [PATCH 6/7] monitor: "i": Add ARM specifics, Stefano Stabellini, 2015/05/06
- Re: [Qemu-devel] [PATCH 6/7] monitor: "i": Add ARM specifics, Peter Maydell, 2015/05/06
- Re: [Qemu-devel] [PATCH 6/7] monitor: "i": Add ARM specifics, Richard Henderson, 2015/05/06
- Re: [Qemu-devel] [PATCH 6/7] monitor: "i": Add ARM specifics, Peter Maydell, 2015/05/06
[Qemu-devel] [PATCH 7/7] disas: arm: Use target_disas impl for monitor, Peter Crosthwaite, 2015/05/05