[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: |
Wed, 06 May 2015 06:57:37 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 |
On 05/05/2015 11:57 PM, Peter Crosthwaite wrote:
> So I have made a start on this. The ARM, MB and CRIS in this patch
> series is rather easy. Its X86 im having trouble with but your example
> here looks like most of the work ...
>
>> 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 {
>
> So here the monitor is actually using the argument memory-dump size to
> dictate the flags. Is this flawed and should we delete this wsize
> if-else and rely on the CPU-state driven logic for correct disas info
> selection? This wsize reliance seems unique to x86. I think we would
> have to give this up in a QOMified approach.
Hmm. I don't think that I've ever noticed the monitor disassembly could do
that. If I were going to do that kind of debugging I certainly wouldn't use
the monitor -- I'd use gdb. ;-)
If someone thinks we ought to keep that feature, speak now...
>> /* 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))
>
> This uses env->efer and segs to drive the flags...
>
>> 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)) {
>
> But your new implementation uses hflags. Are they the same state? I
> couldnt find easy correltation between MSR_EFER_LMA and HF_CSXX_SHIFT
> (although I do see that map to hflags HF_LMA?).
>
> Is your code a functional change?
It's not intended to be. Since I couldn't find where wsize was initialized, I
pulled the tests used by target-i386/translator.c, for dc->code32 and
dc->code64, since I knew where to find them right away. ;-)
Without going back to the manuals, I don't know the difference between CS64 and
LMA; from the code it appears only the behaviour of sysret, which seems
surprising.
> I went for adding print_insn to disassembly_info and passing just that
> to the hook. Patches soon! I might leave X86 out for the first spin.
Sounds good.
r~
- Re: [Qemu-devel] [PATCH 1/7] disas: Create factored out fn for monitor and target disas, (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, 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