[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 6/7] monitor: "i": Add ARM specifics
From: |
Peter Crosthwaite |
Subject: |
Re: [Qemu-devel] [PATCH 6/7] monitor: "i": Add ARM specifics |
Date: |
Wed, 6 May 2015 00:06:51 -0700 |
On Tue, May 5, 2015 at 10:43 AM, Richard Henderson <address@hidden> wrote:
> 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.
>
So something else I just thought of and started worrying about, is the
the target_disas path is driving some of the flags from the disas
context, whereas a hook will only have access to the CPUState/env. Can
we rely on the env/CPUState always being up to date during
target_disas (which happens at translate time?) or will we need to go
field by field to make sure any env updates explicitly occur before
target_disas?
Regards,
Peter
>
>
> r~
>
- [Qemu-devel] [PATCH 4/7] disas: cris: Migrate setup to common code, (continued)
- [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 <=
- 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
Re: [Qemu-devel] [PATCH 0/7] disas: Unify target_disas and monitor_disas, Richard Henderson, 2015/05/05