From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46395) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WXB2Z-0007Pv-NF for qemu-devel@nongnu.org; Mon, 07 Apr 2014 11:00:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WXB2T-0007bs-D2 for qemu-devel@nongnu.org; Mon, 07 Apr 2014 11:00:51 -0400 Received: from mail-lb0-f174.google.com ([209.85.217.174]:51488) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WXB2S-0007bP-TJ for qemu-devel@nongnu.org; Mon, 07 Apr 2014 11:00:45 -0400 Received: by mail-lb0-f174.google.com with SMTP id u14so4845533lbd.19 for ; Mon, 07 Apr 2014 08:00:43 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1396882231-6660-1-git-send-email-tommusta@gmail.com> References: <1396882231-6660-1-git-send-email-tommusta@gmail.com> From: Peter Maydell Date: Mon, 7 Apr 2014 16:00:23 +0100 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH] monitor: QEMU Monitor Instruction Disassembly Incorrect for PowerPC LE Mode List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Tom Musta Cc: "qemu-ppc@nongnu.org" , QEMU Developers On 7 April 2014 15:50, Tom Musta wrote: > The monitor support for disassembling instructions does not honor the MSR[LE] > bit for PowerPC processors. > > This change enhances the monitor_disas() routine by supporting a flag bit > for Little Endian mode. Bit 16 is used since that bit was used in the > analagous guest disassembly routine target_disas(). It seems a touch dubious to assume the area of memory being dissassembled is necessarily the same endianness the CPU happens to be currently... It would be nice to fix the comment at the top of target_disas() which currently claims something totally different and wrong for the PPC flags semantics. > Reported-by: Anton Blanchard > Signed-off-by: Tom Musta > > --- > > The bug can be easily observed by dumping the first few instructions of an > interrupt vector (0x300 is the Data Storage Interrupt handler in PPC) > > (qemu) xp/8i 0x300 > 0x0000000000000300: .long 0x60 > 0x0000000000000304: lhzu r18,-19843(r3) > 0x0000000000000308: .long 0x60 > 0x000000000000030c: lhzu r18,-20099(r2) > 0x0000000000000310: lwz r0,11769(0) > 0x0000000000000314: lhzu r23,8317(r2) > 0x0000000000000318: .long 0x7813427c > 0x000000000000031c: lbz r0,19961(0) > > With the patch applied, the disassembly now looks correct: > > (qemu) xp/8i 0x300 > 0x0000000000000300: nop > 0x0000000000000304: mtsprg 2,r13 > 0x0000000000000308: nop > 0x000000000000030c: mfsprg r13,1 > 0x0000000000000310: std r9,128(r13) > 0x0000000000000314: mfspr r9,896 > 0x0000000000000318: mr r2,r2 > 0x000000000000031c: std r10,136(r13) > > disas.c | 3 +++ > monitor.c | 3 +++ > 2 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/disas.c b/disas.c > index 79e6944..c7804b2 100644 > --- a/disas.c > +++ b/disas.c > @@ -489,6 +489,9 @@ void monitor_disas(Monitor *mon, CPUArchState *env, > #else > s.info.mach = bfd_mach_ppc; > #endif > + if (flags >> 16) { > + s.info.endian = BFD_ENDIAN_LITTLE; > + } If you do this then this code will need to change if you ever add another flag; better to just test bit 16. (The code in target_disas() for PPC also gets this wrong.) Should monitor_disas() support pulling the info.mach from the flags the way target_disas() does? thanks -- PMM