From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45534) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ypzpb-00065e-8z for qemu-devel@nongnu.org; Wed, 06 May 2015 09:57:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YpzpW-00084a-5i for qemu-devel@nongnu.org; Wed, 06 May 2015 09:57:47 -0400 Received: from mail-qg0-x22f.google.com ([2607:f8b0:400d:c04::22f]:34230) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YpzpW-00084U-22 for qemu-devel@nongnu.org; Wed, 06 May 2015 09:57:42 -0400 Received: by qgfi89 with SMTP id i89so4734043qgf.1 for ; Wed, 06 May 2015 06:57:41 -0700 (PDT) Sender: Richard Henderson Message-ID: <554A1DD1.8060602@twiddle.net> Date: Wed, 06 May 2015 06:57:37 -0700 From: Richard Henderson MIME-Version: 1.0 References: <55490142.5030901@twiddle.net> In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 6/7] monitor: "i": Add ARM specifics List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite Cc: Edgar Iglesias , Peter Maydell , "claudio.fontana" , Peter Crosthwaite , Alexander Graf , QEMU Developers , Peter Crosthwaite , "Edgar E. Iglesias" 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~