| Message ID | 1396882231-6660-1-git-send-email-tommusta@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 7 April 2014 15:50, Tom Musta <tommusta@gmail.com> 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 <anton@samba.org> > Signed-off-by: Tom Musta <tommusta@gmail.com> > > --- > > 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
On 4/7/2014 10:00 AM, Peter Maydell wrote: > It seems a touch dubious to assume the area of memory > being dissassembled is necessarily the same endianness > the CPU happens to be currently... I don't disagree but it is less dubious than relying on TARGET_WORDS_BIGENDIAN, which is what the code currently does. I think the only other alternative is to put it back on the user via options on the command, e.g. xp/8il xp/8ib But, I would contend that the using the MSR[LE] bit would still be a better default than TARGET_WORDS_BIGENDIAN if an option is not specified. Thanks for the other comments ... I will incorporate.
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; + } print_insn = print_insn_ppc; #elif defined(TARGET_M68K) print_insn = print_insn_m68k; diff --git a/monitor.c b/monitor.c index 342e83b..5c854fc 100644 --- a/monitor.c +++ b/monitor.c @@ -1309,6 +1309,9 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize, } } #endif +#ifdef TARGET_PPC + flags = msr_le << 16; +#endif monitor_disas(mon, env, addr, count, is_physical, flags); return; }
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(). Reported-by: Anton Blanchard <anton@samba.org> Signed-off-by: Tom Musta <tommusta@gmail.com> --- 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(-)