Re: [Qemu-devel] [PATCH 4/4] Addition of Cavium instruction in disassembler

[Khansa Butt <khansa@kics.edu.pk>]

[-- Attachment #1: Type: text/plain, Size: 2916 bytes --]

On Mon, Aug 15, 2011 at 9:37 PM, Richard Henderson <rth@twiddle.net> wrote:

> On 08/15/2011 04:25 AM, khansa@kics.edu.pk wrote:
> > index 1334b8e..0137657 100644
> > --- a/disas.c
> > +++ b/disas.c
> > @@ -140,6 +140,7 @@ print_insn_thumb1(bfd_vma pc, disassemble_info *info)
> >      i386 - nonzero means 16 bit code
> >      arm  - nonzero means thumb code
> >      ppc  - nonzero means little endian
> > +    mips64 - zero means standard MIPS ISA, 1 means Octeon CPU.
> ...
> > +++ b/target-mips/translate.c
> > @@ -12860,6 +12860,9 @@ done_generating:
> >      LOG_DISAS("\n");
> >      if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
> >          qemu_log("IN: %s\n", lookup_symbol(pc_start));
> > +        if (env->insn_flags & CPU_OCTEON)
> > +            log_target_disas(pc_start, ctx.pc - pc_start, 1);
> > +        else
> >          log_target_disas(pc_start, ctx.pc - pc_start, 0);
>
> disas.c has access to cpu.h.  You'd do well to avoid a magic number
> here and pass along env->insn_flags right from the beginning, and...
>
> > @@ -196,6 +197,9 @@ void target_disas(FILE *out, target_ulong code,
> target_ulong size, int flags)
> >      print_insn = print_insn_m68k;
> >  #elif defined(TARGET_MIPS)
> >  #ifdef TARGET_WORDS_BIGENDIAN
> > +    if (flags)
> > +        disasm_info.flags = flags << 16;
> > +        print_insn = print_insn_big_mips;
> >      print_insn = print_insn_big_mips;
>
> Notice anything funny here in the last two lines?
>

I 'm sorry for this stupid thing as I was in a hurry because of lot of
things.


> Notice anything incorrect about the coding style?
>
> It's a bit sad that we're limited to pre-GPLv3, because we're going
> to get more and more out of sync with the binutils disassembler.
> But that said, let's not go out of our way to totally change the style.
>

 If I pass env->insn_flags to target_disas() how can I pass this flag info
from disas.c to mips-dis.c. can i do following thing for MIPS in disas.c
if (flags & 0xFFFF) {
        disasm_info.mach = flags & 0xFFFF;

>
> Give mips-dis.c access to target-mips/mips-defs.h.  Map between that
> and the existing membership field in struct mips_opcode.  Put some
> real code back into OPCODE_IS_MEMBER.
>

how can I make mips-defs.h 's definitions visible to mips-dis.c?
there are lot of similar definitions in both so will it not create problem.

Actually some of Cavium 's instructions have same Opcode field as Loongson
and COP2 instructions. whenever Cavium 's instruction comes, there is a
need
to pass info from translate.c to mips-dis.c so that correct instruction
would be
selected. but  at the same time other MIPS standard instructions should
also
print in the log as Cavium 's ISA includes MIPS64r2.
I'm sorry i'm bothering you for this. But i'm stuck with this how to
correctly print
Cavium instructions instead of Loongson , A workaround that is acceptable
to
mips maintainer as well.



>
>
> r~
>
>

[-- Attachment #2: Type: text/html, Size: 4204 bytes --]