diff mbox series

[v38,11/22] target/avr: Add instruction disassembly function

Message ID 20191208183922.13757-12-mrolnik@gmail.com
State New
Headers show
Series QEMU AVR 8 bit cores | expand

Commit Message

Michael Rolnik Dec. 8, 2019, 6:39 p.m. UTC 
Provide function disassembles executed instruction when `-d in_asm` is
provided

Example:
`./avr-softmmu/qemu-system-avr -bios free-rtos/Demo/AVR_ATMega2560_GCC/demo.elf -d in_asm` will produce something like the following

```
    ...
    IN:
    0x0000014a:  CALL      0x3808

    IN: main
    0x00003808:  CALL      0x4b4

    IN: vParTestInitialise
    0x000004b4:  LDI       r24, 255
    0x000004b6:  STS       r24, 0
    0x000004b8:  MULS      r16, r20
    0x000004ba:  OUT       $1, r24
    0x000004bc:  LDS       r24, 0
    0x000004be:  MULS      r16, r20
    0x000004c0:  OUT       $2, r24
    0x000004c2:  RET
    ...
```

Signed-off-by: Michael Rolnik <mrolnik@gmail.com>
Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Suggested-by: Aleksandar Markovic <aleksandar.m.mail@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/avr/cpu.h       |   1 +
 target/avr/cpu.c       |   2 +-
 target/avr/disas.c     | 226 +++++++++++++++++++++++++++++++++++++++++
 target/avr/translate.c |  11 ++
 4 files changed, 239 insertions(+), 1 deletion(-)
 create mode 100644 target/avr/disas.c

Comments

Aleksandar Markovic Dec. 9, 2019, 6:10 p.m. UTC | #1 
On Sunday, December 8, 2019, Michael Rolnik <mrolnik@gmail.com> wrote:

> Provide function disassembles executed instruction when `-d in_asm` is
> provided
>
> Example:
> `./avr-softmmu/qemu-system-avr -bios free-rtos/Demo/AVR_ATMega2560_GCC/demo.elf
> -d in_asm` will produce something like the following
>
> ```
>     ...
>     IN:
>     0x0000014a:  CALL      0x3808
>
>     IN: main
>     0x00003808:  CALL      0x4b4
>
>     IN: vParTestInitialise
>     0x000004b4:  LDI       r24, 255
>     0x000004b6:  STS       r24, 0
>     0x000004b8:  MULS      r16, r20
>     0x000004ba:  OUT       $1, r24
>     0x000004bc:  LDS       r24, 0
>     0x000004be:  MULS      r16, r20
>     0x000004c0:  OUT       $2, r24
>     0x000004c2:  RET
>     ...
> ```
>
> Signed-off-by: Michael Rolnik <mrolnik@gmail.com>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Suggested-by: Aleksandar Markovic <aleksandar.m.mail@gmail.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  target/avr/cpu.h       |   1 +
>  target/avr/cpu.c       |   2 +-
>  target/avr/disas.c     | 226 +++++++++++++++++++++++++++++++++++++++++
>  target/avr/translate.c |  11 ++
>  4 files changed, 239 insertions(+), 1 deletion(-)
>  create mode 100644 target/avr/disas.c
>
> diff --git a/target/avr/cpu.h b/target/avr/cpu.h
> index c217eefeb4..a8a3e7ade6 100644
> --- a/target/avr/cpu.h
> +++ b/target/avr/cpu.h
> @@ -178,6 +178,7 @@ bool avr_cpu_exec_interrupt(CPUState *cpu, int
> int_req);
>  hwaddr avr_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
>  int avr_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
>  int avr_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
> +int avr_print_insn(bfd_vma addr, disassemble_info *info);
>
>  static inline int avr_feature(CPUAVRState *env, int feature)
>  {
> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
> index c5cafcae3c..be4b921e4d 100644
> --- a/target/avr/cpu.c
> +++ b/target/avr/cpu.c
> @@ -83,7 +83,7 @@ static void avr_cpu_reset(CPUState *cs)
>  static void avr_cpu_disas_set_info(CPUState *cpu, disassemble_info *info)
>  {
>      info->mach = bfd_arch_avr;
> -    info->print_insn = NULL;
> +    info->print_insn = avr_print_insn;
>  }
>
>  static void avr_cpu_realizefn(DeviceState *dev, Error **errp)
> diff --git a/target/avr/disas.c b/target/avr/disas.c
> new file mode 100644
> index 0000000000..22863d2eb1
> --- /dev/null
> +++ b/target/avr/disas.c
> @@ -0,0 +1,226 @@
> +/*
> + * AVR disassembler
> + *
> + * Copyright (c) 2019 Richard Henderson <rth@twiddle.net>
> + * Copyright (c) 2019 Michael Rolnik <mrolnik@gmail.com>
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "cpu.h"
> +
> +typedef struct {
> +    disassemble_info *info;
> +    uint16_t next_word;
> +    bool next_word_used;
> +} DisasContext;
> +
> +static int to_regs_16_31_by_one(DisasContext *ctx, int indx)
> +{
> +    return 16 + (indx % 16);
> +}
> +
> +static int to_regs_16_23_by_one(DisasContext *ctx, int indx)
> +{
> +    return 16 + (indx % 8);
> +}
> +static int to_regs_24_30_by_two(DisasContext *ctx, int indx)
> +{
> +    return 24 + (indx % 4) * 2;
> +}
> +static int to_regs_00_30_by_two(DisasContext *ctx, int indx)
> +{
> +    return (indx % 16) * 2;
> +}
> +
> +static uint16_t next_word(DisasContext *ctx)
> +{
> +    ctx->next_word_used = true;
> +    return ctx->next_word;
> +}
> +
> +static int append_16(DisasContext *ctx, int x)
> +{
> +    return x << 16 | next_word(ctx);
> +}
> +
> +
> +/* Include the auto-generated decoder.  */
> +static bool decode_insn(DisasContext *ctx, uint16_t insn);
> +#include "decode_insn.inc.c"
> +
> +#define output(mnemonic, format, ...) \
> +    (pctx->info->fprintf_func(pctx->info->stream, "%-9s " format, \
> +                        mnemonic, ##__VA_ARGS__))
> +
> +int avr_print_insn(bfd_vma addr, disassemble_info *info)
> +{
> +    DisasContext ctx;
> +    DisasContext *pctx = &ctx;
> +    bfd_byte buffer[4];
> +    uint16_t insn;
> +    int status;
> +
> +    ctx.info = info;
> +
> +    status = info->read_memory_func(addr, buffer, 4, info);
> +    if (status != 0) {
> +        info->memory_error_func(status, addr, info);
> +        return -1;
> +    }
> +    insn = bfd_getl16(buffer);
> +    ctx.next_word = bfd_getl16(buffer + 2);
> +    ctx.next_word_used = false;
> +
> +    if (!decode_insn(&ctx, insn)) {
> +        output(".db", "0x%02x, 0x%02x", buffer[0], buffer[1]);
> +    }
> +
> +    return ctx.next_word_used ? 4 : 2;
> +}


Hi, Michael,

What is the role of ctx.next_word_used, if it looks it is always "false"?

Related to this, how do you disassemble 32-bit-coded instructions? From
this patch, it looks you treat all AVR instructions as 16-bit-coded?

Thanks,
Aleksandar


> +
> +
> +#define INSN(opcode, format, ...)                                       \
> +static bool trans_##opcode(DisasContext *pctx, arg_##opcode * a)        \
> +{                                                                       \
> +    output(#opcode, format, ##__VA_ARGS__);                             \
> +    return true;                                                        \
> +}
> +
> +#define INSN_MNEMONIC(opcode, mnemonic, format, ...)                    \
> +static bool trans_##opcode(DisasContext *pctx, arg_##opcode * a)        \
> +{                                                                       \
> +    output(mnemonic, format, ##__VA_ARGS__);                            \
> +    return true;                                                        \
> +}
> +
> +/*
> + *   C       Z       N       V       S       H       T       I
> + *   0       1       2       3       4       5       6       7
> + */
> +static const char *brbc[] = {
> +    "BRCC", "BRNE", "BRPL", "BRVC", "BRGE", "BRHC", "BRTC", "BRID"
> +};
> +
> +static const char *brbs[] = {
> +    "BRCS", "BREQ", "BRMI", "BRVS", "BRLT", "BRHS", "BRTS", "BRIE"
> +};
> +
> +static const char *bset[] = {
> +    "SEC",  "SEZ",  "SEN",  "SEZ",  "SES",  "SEH",  "SET",  "SEI"
> +};
> +
> +static const char *bclr[] = {
> +    "CLC",  "CLZ",  "CLN",  "CLZ",  "CLS",  "CLH",  "CLT",  "CLI"
> +};
> +
> +INSN(ADC,    "r%d, r%d", a->rd, a->rr)
> +INSN(ADD,    "r%d, r%d", a->rd, a->rr)
> +INSN(ADIW,   "r%d:r%d, %d", a->rd + 1, a->rd, a->imm)
> +INSN(AND,    "r%d, r%d", a->rd, a->rr)
> +INSN(ANDI,   "r%d, %d", a->rd, a->imm)
> +INSN(ASR,    "r%d", a->rd)
> +INSN_MNEMONIC(BCLR,  bclr[a->bit], "")
> +INSN(BLD,    "r%d, %d", a->rd, a->bit)
> +INSN_MNEMONIC(BRBC,  brbc[a->bit], ".%+d", a->imm * 2)
> +INSN_MNEMONIC(BRBS,  brbs[a->bit], ".%+d", a->imm * 2)
> +INSN(BREAK,  "")
> +INSN_MNEMONIC(BSET,  bset[a->bit], "")
> +INSN(BST,    "r%d, %d", a->rd, a->bit)
> +INSN(CALL,   "0x%x", a->imm * 2)
> +INSN(CBI,    "%d, %d", a->reg, a->bit)
> +INSN(COM,    "r%d", a->rd)
> +INSN(CP,     "r%d, r%d", a->rd, a->rr)
> +INSN(CPC,    "r%d, r%d", a->rd, a->rr)
> +INSN(CPI,    "r%d, %d", a->rd, a->imm)
> +INSN(CPSE,   "r%d, r%d", a->rd, a->rr)
> +INSN(DEC,    "r%d", a->rd)
> +INSN(DES,    "%d", a->imm)
> +INSN(EICALL, "")
> +INSN(EIJMP,  "")
> +INSN(ELPM1,  "")
> +INSN(ELPM2,  "r%d, Z", a->rd)
> +INSN(ELPMX,  "r%d, Z+", a->rd)
> +INSN(EOR,    "r%d, r%d", a->rd, a->rr)
> +INSN(FMUL,   "r%d, r%d", a->rd, a->rr)
> +INSN(FMULS,  "r%d, r%d", a->rd, a->rr)
> +INSN(FMULSU, "r%d, r%d", a->rd, a->rr)
> +INSN(ICALL,  "")
> +INSN(IJMP,   "")
> +INSN(IN,     "r%d, $%d", a->rd, a->imm)
> +INSN(INC,    "r%d", a->rd)
> +INSN(JMP,    "0x%x", a->imm * 2)
> +INSN(LAC,    "Z, r%d", a->rd)
> +INSN(LAS,    "Z, r%d", a->rd)
> +INSN(LAT,    "Z, r%d", a->rd)
> +INSN(LDDY,   "r%d, Y+%d", a->rd, a->imm)
> +INSN(LDDZ,   "r%d, Z+%d", a->rd, a->imm)
> +INSN(LDI,    "r%d, %d", a->rd, a->imm)
> +INSN(LDS,    "r%d, %d", a->rd, a->imm)
> +INSN(LDX1,   "r%d, X", a->rd)
> +INSN(LDX2,   "r%d, X+", a->rd)
> +INSN(LDX3,   "r%d, -X", a->rd)
> +INSN(LDY2,   "r%d, Y+", a->rd)
> +INSN(LDY3,   "r%d, -Y", a->rd)
> +INSN(LDZ2,   "r%d, Z+", a->rd)
> +INSN(LDZ3,   "r%d, -Z", a->rd)
> +INSN(LPM1,   "")
> +INSN(LPM2,   "r%d, Z", a->rd)
> +INSN(LPMX,   "r%d, Z+", a->rd)
> +INSN(LSR,    "r%d", a->rd)
> +INSN(MOV,    "r%d, r%d", a->rd, a->rr)
> +INSN(MOVW,   "r%d:r%d, r%d,r:r%d", a->rd + 1, a->rd, a->rr + 1, a->rr)
> +INSN(MUL,    "r%d, r%d", a->rd, a->rr)
> +INSN(MULS,   "r%d, r%d", a->rd, a->rr)
> +INSN(MULSU,  "r%d, r%d", a->rd, a->rr)
> +INSN(NEG,    "r%d", a->rd)
> +INSN(NOP,    "")
> +INSN(OR,     "r%d, r%d", a->rd, a->rr)
> +INSN(ORI,    "r%d, %d", a->rd, a->imm)
> +INSN(OUT,    "$%d, r%d", a->imm, a->rd)
> +INSN(POP,    "r%d", a->rd)
> +INSN(PUSH,   "r%d", a->rd)
> +INSN(RCALL,  ".%+d", a->imm * 2)
> +INSN(RET,    "")
> +INSN(RETI,   "")
> +INSN(RJMP,   ".%+d", a->imm * 2)
> +INSN(ROR,    "r%d", a->rd)
> +INSN(SBC,    "r%d, r%d", a->rd, a->rr)
> +INSN(SBCI,   "r%d, %d", a->rd, a->imm)
> +INSN(SBI,    "$%d, %d", a->reg, a->bit)
> +INSN(SBIC,   "$%d, %d", a->reg, a->bit)
> +INSN(SBIS,   "$%d, %d", a->reg, a->bit)
> +INSN(SBIW,   "r%d:r%d, %d", a->rd + 1, a->rd, a->imm)
> +INSN(SBRC,   "r%d, %d", a->rr, a->bit)
> +INSN(SBRS,   "r%d, %d", a->rr, a->bit)
> +INSN(SLEEP,  "")
> +INSN(SPM,    "")
> +INSN(SPMX,   "Z+")
> +INSN(STDY,   "r%d, Y+%d", a->rd, a->imm)
> +INSN(STDZ,   "r%d, Z+%d", a->rd, a->imm)
> +INSN(STS,    "r%d, %d", a->rd, a->imm)
> +INSN(STX1,   "r%d, X", a->rr)
> +INSN(STX2,   "r%d, X+", a->rr)
> +INSN(STX3,   "r%d, -X", a->rr)
> +INSN(STY2,   "r%d, Y+", a->rd)
> +INSN(STY3,   "r%d, -Y", a->rd)
> +INSN(STZ2,   "r%d, Z+", a->rd)
> +INSN(STZ3,   "r%d, -Z", a->rd)
> +INSN(SUB,    "r%d, r%d", a->rd, a->rr)
> +INSN(SUBI,   "r%d, %d", a->rd, a->imm)
> +INSN(SWAP,   "r%d", a->rd)
> +INSN(WDR,    "")
> +INSN(XCH,    "Z, r%d", a->rd)
> +
> diff --git a/target/avr/translate.c b/target/avr/translate.c
> index c8c6f798bf..a621195817 100644
> --- a/target/avr/translate.c
> +++ b/target/avr/translate.c
> @@ -2917,6 +2917,17 @@ done_generating:
>
>      tb->size = (ctx.npc - pc_start) * 2;
>      tb->icount = num_insns;
> +
> +#ifdef DEBUG_DISAS
> +    if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
> +        && qemu_log_in_addr_range(tb->pc)) {
> +        qemu_log_lock();
> +        qemu_log("IN: %s\n", lookup_symbol(tb->pc));
> +        log_target_disas(cs, tb->pc, tb->size);
> +        qemu_log("\n");
> +        qemu_log_unlock();
> +    }
> +#endif
>  }
>
>  void restore_state_to_opc(CPUAVRState *env, TranslationBlock *tb,
> --
> 2.17.2 (Apple Git-113)
>
>
Michael Rolnik Dec. 9, 2019, 6:27 p.m. UTC | #2 
Hi Aleksandar.

1. all instructions are 16 bit long except CALL & JMP they are 32 bit long
2. next_word_used is set to true by next_word when called by append_16 when
CALL & JMP are parsed

Regards,
Michael Rolnik

On Mon, Dec 9, 2019 at 8:10 PM Aleksandar Markovic <
aleksandar.m.mail@gmail.com> wrote:

>
>
> On Sunday, December 8, 2019, Michael Rolnik <mrolnik@gmail.com> wrote:
>
>> Provide function disassembles executed instruction when `-d in_asm` is
>> provided
>>
>> Example:
>> `./avr-softmmu/qemu-system-avr -bios
>> free-rtos/Demo/AVR_ATMega2560_GCC/demo.elf -d in_asm` will produce
>> something like the following
>>
>> ```
>>     ...
>>     IN:
>>     0x0000014a:  CALL      0x3808
>>
>>     IN: main
>>     0x00003808:  CALL      0x4b4
>>
>>     IN: vParTestInitialise
>>     0x000004b4:  LDI       r24, 255
>>     0x000004b6:  STS       r24, 0
>>     0x000004b8:  MULS      r16, r20
>>     0x000004ba:  OUT       $1, r24
>>     0x000004bc:  LDS       r24, 0
>>     0x000004be:  MULS      r16, r20
>>     0x000004c0:  OUT       $2, r24
>>     0x000004c2:  RET
>>     ...
>> ```
>>
>> Signed-off-by: Michael Rolnik <mrolnik@gmail.com>
>> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
>> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Suggested-by: Aleksandar Markovic <aleksandar.m.mail@gmail.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  target/avr/cpu.h       |   1 +
>>  target/avr/cpu.c       |   2 +-
>>  target/avr/disas.c     | 226 +++++++++++++++++++++++++++++++++++++++++
>>  target/avr/translate.c |  11 ++
>>  4 files changed, 239 insertions(+), 1 deletion(-)
>>  create mode 100644 target/avr/disas.c
>>
>> diff --git a/target/avr/cpu.h b/target/avr/cpu.h
>> index c217eefeb4..a8a3e7ade6 100644
>> --- a/target/avr/cpu.h
>> +++ b/target/avr/cpu.h
>> @@ -178,6 +178,7 @@ bool avr_cpu_exec_interrupt(CPUState *cpu, int
>> int_req);
>>  hwaddr avr_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
>>  int avr_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
>>  int avr_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
>> +int avr_print_insn(bfd_vma addr, disassemble_info *info);
>>
>>  static inline int avr_feature(CPUAVRState *env, int feature)
>>  {
>> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
>> index c5cafcae3c..be4b921e4d 100644
>> --- a/target/avr/cpu.c
>> +++ b/target/avr/cpu.c
>> @@ -83,7 +83,7 @@ static void avr_cpu_reset(CPUState *cs)
>>  static void avr_cpu_disas_set_info(CPUState *cpu, disassemble_info *info)
>>  {
>>      info->mach = bfd_arch_avr;
>> -    info->print_insn = NULL;
>> +    info->print_insn = avr_print_insn;
>>  }
>>
>>  static void avr_cpu_realizefn(DeviceState *dev, Error **errp)
>> diff --git a/target/avr/disas.c b/target/avr/disas.c
>> new file mode 100644
>> index 0000000000..22863d2eb1
>> --- /dev/null
>> +++ b/target/avr/disas.c
>> @@ -0,0 +1,226 @@
>> +/*
>> + * AVR disassembler
>> + *
>> + * Copyright (c) 2019 Richard Henderson <rth@twiddle.net>
>> + * Copyright (c) 2019 Michael Rolnik <mrolnik@gmail.com>
>> + *
>> + * This program is free software: you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation, either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "cpu.h"
>> +
>> +typedef struct {
>> +    disassemble_info *info;
>> +    uint16_t next_word;
>> +    bool next_word_used;
>> +} DisasContext;
>> +
>> +static int to_regs_16_31_by_one(DisasContext *ctx, int indx)
>> +{
>> +    return 16 + (indx % 16);
>> +}
>> +
>> +static int to_regs_16_23_by_one(DisasContext *ctx, int indx)
>> +{
>> +    return 16 + (indx % 8);
>> +}
>> +static int to_regs_24_30_by_two(DisasContext *ctx, int indx)
>> +{
>> +    return 24 + (indx % 4) * 2;
>> +}
>> +static int to_regs_00_30_by_two(DisasContext *ctx, int indx)
>> +{
>> +    return (indx % 16) * 2;
>> +}
>> +
>> +static uint16_t next_word(DisasContext *ctx)
>> +{
>> +    ctx->next_word_used = true;
>> +    return ctx->next_word;
>> +}
>> +
>> +static int append_16(DisasContext *ctx, int x)
>> +{
>> +    return x << 16 | next_word(ctx);
>> +}
>> +
>> +
>> +/* Include the auto-generated decoder.  */
>> +static bool decode_insn(DisasContext *ctx, uint16_t insn);
>> +#include "decode_insn.inc.c"
>> +
>> +#define output(mnemonic, format, ...) \
>> +    (pctx->info->fprintf_func(pctx->info->stream, "%-9s " format, \
>> +                        mnemonic, ##__VA_ARGS__))
>> +
>> +int avr_print_insn(bfd_vma addr, disassemble_info *info)
>> +{
>> +    DisasContext ctx;
>> +    DisasContext *pctx = &ctx;
>> +    bfd_byte buffer[4];
>> +    uint16_t insn;
>> +    int status;
>> +
>> +    ctx.info = info;
>> +
>> +    status = info->read_memory_func(addr, buffer, 4, info);
>> +    if (status != 0) {
>> +        info->memory_error_func(status, addr, info);
>> +        return -1;
>> +    }
>> +    insn = bfd_getl16(buffer);
>> +    ctx.next_word = bfd_getl16(buffer + 2);
>> +    ctx.next_word_used = false;
>> +
>> +    if (!decode_insn(&ctx, insn)) {
>> +        output(".db", "0x%02x, 0x%02x", buffer[0], buffer[1]);
>> +    }
>> +
>> +    return ctx.next_word_used ? 4 : 2;
>> +}
>
>
> Hi, Michael,
>
> What is the role of ctx.next_word_used, if it looks it is always "false"?
>
> Related to this, how do you disassemble 32-bit-coded instructions? From
> this patch, it looks you treat all AVR instructions as 16-bit-coded?
>
> Thanks,
> Aleksandar
>
>
>> +
>> +
>> +#define INSN(opcode, format, ...)                                       \
>> +static bool trans_##opcode(DisasContext *pctx, arg_##opcode * a)        \
>> +{                                                                       \
>> +    output(#opcode, format, ##__VA_ARGS__);                             \
>> +    return true;                                                        \
>> +}
>> +
>> +#define INSN_MNEMONIC(opcode, mnemonic, format, ...)                    \
>> +static bool trans_##opcode(DisasContext *pctx, arg_##opcode * a)        \
>> +{                                                                       \
>> +    output(mnemonic, format, ##__VA_ARGS__);                            \
>> +    return true;                                                        \
>> +}
>> +
>> +/*
>> + *   C       Z       N       V       S       H       T       I
>> + *   0       1       2       3       4       5       6       7
>> + */
>> +static const char *brbc[] = {
>> +    "BRCC", "BRNE", "BRPL", "BRVC", "BRGE", "BRHC", "BRTC", "BRID"
>> +};
>> +
>> +static const char *brbs[] = {
>> +    "BRCS", "BREQ", "BRMI", "BRVS", "BRLT", "BRHS", "BRTS", "BRIE"
>> +};
>> +
>> +static const char *bset[] = {
>> +    "SEC",  "SEZ",  "SEN",  "SEZ",  "SES",  "SEH",  "SET",  "SEI"
>> +};
>> +
>> +static const char *bclr[] = {
>> +    "CLC",  "CLZ",  "CLN",  "CLZ",  "CLS",  "CLH",  "CLT",  "CLI"
>> +};
>> +
>> +INSN(ADC,    "r%d, r%d", a->rd, a->rr)
>> +INSN(ADD,    "r%d, r%d", a->rd, a->rr)
>> +INSN(ADIW,   "r%d:r%d, %d", a->rd + 1, a->rd, a->imm)
>> +INSN(AND,    "r%d, r%d", a->rd, a->rr)
>> +INSN(ANDI,   "r%d, %d", a->rd, a->imm)
>> +INSN(ASR,    "r%d", a->rd)
>> +INSN_MNEMONIC(BCLR,  bclr[a->bit], "")
>> +INSN(BLD,    "r%d, %d", a->rd, a->bit)
>> +INSN_MNEMONIC(BRBC,  brbc[a->bit], ".%+d", a->imm * 2)
>> +INSN_MNEMONIC(BRBS,  brbs[a->bit], ".%+d", a->imm * 2)
>> +INSN(BREAK,  "")
>> +INSN_MNEMONIC(BSET,  bset[a->bit], "")
>> +INSN(BST,    "r%d, %d", a->rd, a->bit)
>> +INSN(CALL,   "0x%x", a->imm * 2)
>> +INSN(CBI,    "%d, %d", a->reg, a->bit)
>> +INSN(COM,    "r%d", a->rd)
>> +INSN(CP,     "r%d, r%d", a->rd, a->rr)
>> +INSN(CPC,    "r%d, r%d", a->rd, a->rr)
>> +INSN(CPI,    "r%d, %d", a->rd, a->imm)
>> +INSN(CPSE,   "r%d, r%d", a->rd, a->rr)
>> +INSN(DEC,    "r%d", a->rd)
>> +INSN(DES,    "%d", a->imm)
>> +INSN(EICALL, "")
>> +INSN(EIJMP,  "")
>> +INSN(ELPM1,  "")
>> +INSN(ELPM2,  "r%d, Z", a->rd)
>> +INSN(ELPMX,  "r%d, Z+", a->rd)
>> +INSN(EOR,    "r%d, r%d", a->rd, a->rr)
>> +INSN(FMUL,   "r%d, r%d", a->rd, a->rr)
>> +INSN(FMULS,  "r%d, r%d", a->rd, a->rr)
>> +INSN(FMULSU, "r%d, r%d", a->rd, a->rr)
>> +INSN(ICALL,  "")
>> +INSN(IJMP,   "")
>> +INSN(IN,     "r%d, $%d", a->rd, a->imm)
>> +INSN(INC,    "r%d", a->rd)
>> +INSN(JMP,    "0x%x", a->imm * 2)
>> +INSN(LAC,    "Z, r%d", a->rd)
>> +INSN(LAS,    "Z, r%d", a->rd)
>> +INSN(LAT,    "Z, r%d", a->rd)
>> +INSN(LDDY,   "r%d, Y+%d", a->rd, a->imm)
>> +INSN(LDDZ,   "r%d, Z+%d", a->rd, a->imm)
>> +INSN(LDI,    "r%d, %d", a->rd, a->imm)
>> +INSN(LDS,    "r%d, %d", a->rd, a->imm)
>> +INSN(LDX1,   "r%d, X", a->rd)
>> +INSN(LDX2,   "r%d, X+", a->rd)
>> +INSN(LDX3,   "r%d, -X", a->rd)
>> +INSN(LDY2,   "r%d, Y+", a->rd)
>> +INSN(LDY3,   "r%d, -Y", a->rd)
>> +INSN(LDZ2,   "r%d, Z+", a->rd)
>> +INSN(LDZ3,   "r%d, -Z", a->rd)
>> +INSN(LPM1,   "")
>> +INSN(LPM2,   "r%d, Z", a->rd)
>> +INSN(LPMX,   "r%d, Z+", a->rd)
>> +INSN(LSR,    "r%d", a->rd)
>> +INSN(MOV,    "r%d, r%d", a->rd, a->rr)
>> +INSN(MOVW,   "r%d:r%d, r%d,r:r%d", a->rd + 1, a->rd, a->rr + 1, a->rr)
>> +INSN(MUL,    "r%d, r%d", a->rd, a->rr)
>> +INSN(MULS,   "r%d, r%d", a->rd, a->rr)
>> +INSN(MULSU,  "r%d, r%d", a->rd, a->rr)
>> +INSN(NEG,    "r%d", a->rd)
>> +INSN(NOP,    "")
>> +INSN(OR,     "r%d, r%d", a->rd, a->rr)
>> +INSN(ORI,    "r%d, %d", a->rd, a->imm)
>> +INSN(OUT,    "$%d, r%d", a->imm, a->rd)
>> +INSN(POP,    "r%d", a->rd)
>> +INSN(PUSH,   "r%d", a->rd)
>> +INSN(RCALL,  ".%+d", a->imm * 2)
>> +INSN(RET,    "")
>> +INSN(RETI,   "")
>> +INSN(RJMP,   ".%+d", a->imm * 2)
>> +INSN(ROR,    "r%d", a->rd)
>> +INSN(SBC,    "r%d, r%d", a->rd, a->rr)
>> +INSN(SBCI,   "r%d, %d", a->rd, a->imm)
>> +INSN(SBI,    "$%d, %d", a->reg, a->bit)
>> +INSN(SBIC,   "$%d, %d", a->reg, a->bit)
>> +INSN(SBIS,   "$%d, %d", a->reg, a->bit)
>> +INSN(SBIW,   "r%d:r%d, %d", a->rd + 1, a->rd, a->imm)
>> +INSN(SBRC,   "r%d, %d", a->rr, a->bit)
>> +INSN(SBRS,   "r%d, %d", a->rr, a->bit)
>> +INSN(SLEEP,  "")
>> +INSN(SPM,    "")
>> +INSN(SPMX,   "Z+")
>> +INSN(STDY,   "r%d, Y+%d", a->rd, a->imm)
>> +INSN(STDZ,   "r%d, Z+%d", a->rd, a->imm)
>> +INSN(STS,    "r%d, %d", a->rd, a->imm)
>> +INSN(STX1,   "r%d, X", a->rr)
>> +INSN(STX2,   "r%d, X+", a->rr)
>> +INSN(STX3,   "r%d, -X", a->rr)
>> +INSN(STY2,   "r%d, Y+", a->rd)
>> +INSN(STY3,   "r%d, -Y", a->rd)
>> +INSN(STZ2,   "r%d, Z+", a->rd)
>> +INSN(STZ3,   "r%d, -Z", a->rd)
>> +INSN(SUB,    "r%d, r%d", a->rd, a->rr)
>> +INSN(SUBI,   "r%d, %d", a->rd, a->imm)
>> +INSN(SWAP,   "r%d", a->rd)
>> +INSN(WDR,    "")
>> +INSN(XCH,    "Z, r%d", a->rd)
>> +
>> diff --git a/target/avr/translate.c b/target/avr/translate.c
>> index c8c6f798bf..a621195817 100644
>> --- a/target/avr/translate.c
>> +++ b/target/avr/translate.c
>> @@ -2917,6 +2917,17 @@ done_generating:
>>
>>      tb->size = (ctx.npc - pc_start) * 2;
>>      tb->icount = num_insns;
>> +
>> +#ifdef DEBUG_DISAS
>> +    if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
>> +        && qemu_log_in_addr_range(tb->pc)) {
>> +        qemu_log_lock();
>> +        qemu_log("IN: %s\n", lookup_symbol(tb->pc));
>> +        log_target_disas(cs, tb->pc, tb->size);
>> +        qemu_log("\n");
>> +        qemu_log_unlock();
>> +    }
>> +#endif
>>  }
>>
>>  void restore_state_to_opc(CPUAVRState *env, TranslationBlock *tb,
>> --
>> 2.17.2 (Apple Git-113)
>>
>>
Aleksandar Markovic Dec. 10, 2019, 4:21 a.m. UTC | #3 
On Monday, December 9, 2019, Michael Rolnik <mrolnik@gmail.com> wrote:

> Hi Aleksandar.
>
> 1. all instructions are 16 bit long except CALL & JMP they are 32 bit long
>

Accordingto the doc, LDS and STS also have 32-bit coding.



> 2. next_word_used is set to true by next_word when called by append_16
> when CALL & JMP are parsed
>
> Regards,
> Michael Rolnik
>
> On Mon, Dec 9, 2019 at 8:10 PM Aleksandar Markovic <
> aleksandar.m.mail@gmail.com> wrote:
>
>>
>>
>> On Sunday, December 8, 2019, Michael Rolnik <mrolnik@gmail.com> wrote:
>>
>>> Provide function disassembles executed instruction when `-d in_asm` is
>>> provided
>>>
>>> Example:
>>> `./avr-softmmu/qemu-system-avr -bios free-rtos/Demo/AVR_ATMega2560_GCC/demo.elf
>>> -d in_asm` will produce something like the following
>>>
>>> ```
>>>     ...
>>>     IN:
>>>     0x0000014a:  CALL      0x3808
>>>
>>>     IN: main
>>>     0x00003808:  CALL      0x4b4
>>>
>>>     IN: vParTestInitialise
>>>     0x000004b4:  LDI       r24, 255
>>>     0x000004b6:  STS       r24, 0
>>>     0x000004b8:  MULS      r16, r20
>>>     0x000004ba:  OUT       $1, r24
>>>     0x000004bc:  LDS       r24, 0
>>>     0x000004be:  MULS      r16, r20
>>>     0x000004c0:  OUT       $2, r24
>>>     0x000004c2:  RET
>>>     ...
>>> ```
>>>
>>> Signed-off-by: Michael Rolnik <mrolnik@gmail.com>
>>> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
>>> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> Suggested-by: Aleksandar Markovic <aleksandar.m.mail@gmail.com>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>>  target/avr/cpu.h       |   1 +
>>>  target/avr/cpu.c       |   2 +-
>>>  target/avr/disas.c     | 226 +++++++++++++++++++++++++++++++++++++++++
>>>  target/avr/translate.c |  11 ++
>>>  4 files changed, 239 insertions(+), 1 deletion(-)
>>>  create mode 100644 target/avr/disas.c
>>>
>>> diff --git a/target/avr/cpu.h b/target/avr/cpu.h
>>> index c217eefeb4..a8a3e7ade6 100644
>>> --- a/target/avr/cpu.h
>>> +++ b/target/avr/cpu.h
>>> @@ -178,6 +178,7 @@ bool avr_cpu_exec_interrupt(CPUState *cpu, int
>>> int_req);
>>>  hwaddr avr_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
>>>  int avr_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
>>>  int avr_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
>>> +int avr_print_insn(bfd_vma addr, disassemble_info *info);
>>>
>>>  static inline int avr_feature(CPUAVRState *env, int feature)
>>>  {
>>> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
>>> index c5cafcae3c..be4b921e4d 100644
>>> --- a/target/avr/cpu.c
>>> +++ b/target/avr/cpu.c
>>> @@ -83,7 +83,7 @@ static void avr_cpu_reset(CPUState *cs)
>>>  static void avr_cpu_disas_set_info(CPUState *cpu, disassemble_info
>>> *info)
>>>  {
>>>      info->mach = bfd_arch_avr;
>>> -    info->print_insn = NULL;
>>> +    info->print_insn = avr_print_insn;
>>>  }
>>>
>>>  static void avr_cpu_realizefn(DeviceState *dev, Error **errp)
>>> diff --git a/target/avr/disas.c b/target/avr/disas.c
>>> new file mode 100644
>>> index 0000000000..22863d2eb1
>>> --- /dev/null
>>> +++ b/target/avr/disas.c
>>> @@ -0,0 +1,226 @@
>>> +/*
>>> + * AVR disassembler
>>> + *
>>> + * Copyright (c) 2019 Richard Henderson <rth@twiddle.net>
>>> + * Copyright (c) 2019 Michael Rolnik <mrolnik@gmail.com>
>>> + *
>>> + * This program is free software: you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation, either version 2 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> + * GNU General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License
>>> + * along with this program.  If not, see <http://www.gnu.org/licenses/>
>>> .
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "cpu.h"
>>> +
>>> +typedef struct {
>>> +    disassemble_info *info;
>>> +    uint16_t next_word;
>>> +    bool next_word_used;
>>> +} DisasContext;
>>> +
>>> +static int to_regs_16_31_by_one(DisasContext *ctx, int indx)
>>> +{
>>> +    return 16 + (indx % 16);
>>> +}
>>> +
>>> +static int to_regs_16_23_by_one(DisasContext *ctx, int indx)
>>> +{
>>> +    return 16 + (indx % 8);
>>> +}
>>> +static int to_regs_24_30_by_two(DisasContext *ctx, int indx)
>>> +{
>>> +    return 24 + (indx % 4) * 2;
>>> +}
>>> +static int to_regs_00_30_by_two(DisasContext *ctx, int indx)
>>> +{
>>> +    return (indx % 16) * 2;
>>> +}
>>> +
>>> +static uint16_t next_word(DisasContext *ctx)
>>> +{
>>> +    ctx->next_word_used = true;
>>> +    return ctx->next_word;
>>> +}
>>> +
>>> +static int append_16(DisasContext *ctx, int x)
>>> +{
>>> +    return x << 16 | next_word(ctx);
>>> +}
>>> +
>>> +
>>> +/* Include the auto-generated decoder.  */
>>> +static bool decode_insn(DisasContext *ctx, uint16_t insn);
>>> +#include "decode_insn.inc.c"
>>> +
>>> +#define output(mnemonic, format, ...) \
>>> +    (pctx->info->fprintf_func(pctx->info->stream, "%-9s " format, \
>>> +                        mnemonic, ##__VA_ARGS__))
>>> +
>>> +int avr_print_insn(bfd_vma addr, disassemble_info *info)
>>> +{
>>> +    DisasContext ctx;
>>> +    DisasContext *pctx = &ctx;
>>> +    bfd_byte buffer[4];
>>> +    uint16_t insn;
>>> +    int status;
>>> +
>>> +    ctx.info = info;
>>> +
>>> +    status = info->read_memory_func(addr, buffer, 4, info);
>>> +    if (status != 0) {
>>> +        info->memory_error_func(status, addr, info);
>>> +        return -1;
>>> +    }
>>> +    insn = bfd_getl16(buffer);
>>> +    ctx.next_word = bfd_getl16(buffer + 2);
>>> +    ctx.next_word_used = false;
>>> +
>>> +    if (!decode_insn(&ctx, insn)) {
>>> +        output(".db", "0x%02x, 0x%02x", buffer[0], buffer[1]);
>>> +    }
>>> +
>>> +    return ctx.next_word_used ? 4 : 2;
>>> +}
>>
>>
>> Hi, Michael,
>>
>> What is the role of ctx.next_word_used, if it looks it is always "false"?
>>
>> Related to this, how do you disassemble 32-bit-coded instructions? From
>> this patch, it looks you treat all AVR instructions as 16-bit-coded?
>>
>> Thanks,
>> Aleksandar
>>
>>
>>> +
>>> +
>>> +#define INSN(opcode, format, ...)
>>>  \
>>> +static bool trans_##opcode(DisasContext *pctx, arg_##opcode * a)
>>> \
>>> +{
>>>  \
>>> +    output(#opcode, format, ##__VA_ARGS__);
>>>  \
>>> +    return true;
>>> \
>>> +}
>>> +
>>> +#define INSN_MNEMONIC(opcode, mnemonic, format, ...)
>>> \
>>> +static bool trans_##opcode(DisasContext *pctx, arg_##opcode * a)
>>> \
>>> +{
>>>  \
>>> +    output(mnemonic, format, ##__VA_ARGS__);
>>> \
>>> +    return true;
>>> \
>>> +}
>>> +
>>> +/*
>>> + *   C       Z       N       V       S       H       T       I
>>> + *   0       1       2       3       4       5       6       7
>>> + */
>>> +static const char *brbc[] = {
>>> +    "BRCC", "BRNE", "BRPL", "BRVC", "BRGE", "BRHC", "BRTC", "BRID"
>>> +};
>>> +
>>> +static const char *brbs[] = {
>>> +    "BRCS", "BREQ", "BRMI", "BRVS", "BRLT", "BRHS", "BRTS", "BRIE"
>>> +};
>>> +
>>> +static const char *bset[] = {
>>> +    "SEC",  "SEZ",  "SEN",  "SEZ",  "SES",  "SEH",  "SET",  "SEI"
>>> +};
>>> +
>>> +static const char *bclr[] = {
>>> +    "CLC",  "CLZ",  "CLN",  "CLZ",  "CLS",  "CLH",  "CLT",  "CLI"
>>> +};
>>> +
>>> +INSN(ADC,    "r%d, r%d", a->rd, a->rr)
>>> +INSN(ADD,    "r%d, r%d", a->rd, a->rr)
>>> +INSN(ADIW,   "r%d:r%d, %d", a->rd + 1, a->rd, a->imm)
>>> +INSN(AND,    "r%d, r%d", a->rd, a->rr)
>>> +INSN(ANDI,   "r%d, %d", a->rd, a->imm)
>>> +INSN(ASR,    "r%d", a->rd)
>>> +INSN_MNEMONIC(BCLR,  bclr[a->bit], "")
>>> +INSN(BLD,    "r%d, %d", a->rd, a->bit)
>>> +INSN_MNEMONIC(BRBC,  brbc[a->bit], ".%+d", a->imm * 2)
>>> +INSN_MNEMONIC(BRBS,  brbs[a->bit], ".%+d", a->imm * 2)
>>> +INSN(BREAK,  "")
>>> +INSN_MNEMONIC(BSET,  bset[a->bit], "")
>>> +INSN(BST,    "r%d, %d", a->rd, a->bit)
>>> +INSN(CALL,   "0x%x", a->imm * 2)
>>> +INSN(CBI,    "%d, %d", a->reg, a->bit)
>>> +INSN(COM,    "r%d", a->rd)
>>> +INSN(CP,     "r%d, r%d", a->rd, a->rr)
>>> +INSN(CPC,    "r%d, r%d", a->rd, a->rr)
>>> +INSN(CPI,    "r%d, %d", a->rd, a->imm)
>>> +INSN(CPSE,   "r%d, r%d", a->rd, a->rr)
>>> +INSN(DEC,    "r%d", a->rd)
>>> +INSN(DES,    "%d", a->imm)
>>> +INSN(EICALL, "")
>>> +INSN(EIJMP,  "")
>>> +INSN(ELPM1,  "")
>>> +INSN(ELPM2,  "r%d, Z", a->rd)
>>> +INSN(ELPMX,  "r%d, Z+", a->rd)
>>> +INSN(EOR,    "r%d, r%d", a->rd, a->rr)
>>> +INSN(FMUL,   "r%d, r%d", a->rd, a->rr)
>>> +INSN(FMULS,  "r%d, r%d", a->rd, a->rr)
>>> +INSN(FMULSU, "r%d, r%d", a->rd, a->rr)
>>> +INSN(ICALL,  "")
>>> +INSN(IJMP,   "")
>>> +INSN(IN,     "r%d, $%d", a->rd, a->imm)
>>> +INSN(INC,    "r%d", a->rd)
>>> +INSN(JMP,    "0x%x", a->imm * 2)
>>> +INSN(LAC,    "Z, r%d", a->rd)
>>> +INSN(LAS,    "Z, r%d", a->rd)
>>> +INSN(LAT,    "Z, r%d", a->rd)
>>> +INSN(LDDY,   "r%d, Y+%d", a->rd, a->imm)
>>> +INSN(LDDZ,   "r%d, Z+%d", a->rd, a->imm)
>>> +INSN(LDI,    "r%d, %d", a->rd, a->imm)
>>> +INSN(LDS,    "r%d, %d", a->rd, a->imm)
>>> +INSN(LDX1,   "r%d, X", a->rd)
>>> +INSN(LDX2,   "r%d, X+", a->rd)
>>> +INSN(LDX3,   "r%d, -X", a->rd)
>>> +INSN(LDY2,   "r%d, Y+", a->rd)
>>> +INSN(LDY3,   "r%d, -Y", a->rd)
>>> +INSN(LDZ2,   "r%d, Z+", a->rd)
>>> +INSN(LDZ3,   "r%d, -Z", a->rd)
>>> +INSN(LPM1,   "")
>>> +INSN(LPM2,   "r%d, Z", a->rd)
>>> +INSN(LPMX,   "r%d, Z+", a->rd)
>>> +INSN(LSR,    "r%d", a->rd)
>>> +INSN(MOV,    "r%d, r%d", a->rd, a->rr)
>>> +INSN(MOVW,   "r%d:r%d, r%d,r:r%d", a->rd + 1, a->rd, a->rr + 1, a->rr)
>>> +INSN(MUL,    "r%d, r%d", a->rd, a->rr)
>>> +INSN(MULS,   "r%d, r%d", a->rd, a->rr)
>>> +INSN(MULSU,  "r%d, r%d", a->rd, a->rr)
>>> +INSN(NEG,    "r%d", a->rd)
>>> +INSN(NOP,    "")
>>> +INSN(OR,     "r%d, r%d", a->rd, a->rr)
>>> +INSN(ORI,    "r%d, %d", a->rd, a->imm)
>>> +INSN(OUT,    "$%d, r%d", a->imm, a->rd)
>>> +INSN(POP,    "r%d", a->rd)
>>> +INSN(PUSH,   "r%d", a->rd)
>>> +INSN(RCALL,  ".%+d", a->imm * 2)
>>> +INSN(RET,    "")
>>> +INSN(RETI,   "")
>>> +INSN(RJMP,   ".%+d", a->imm * 2)
>>> +INSN(ROR,    "r%d", a->rd)
>>> +INSN(SBC,    "r%d, r%d", a->rd, a->rr)
>>> +INSN(SBCI,   "r%d, %d", a->rd, a->imm)
>>> +INSN(SBI,    "$%d, %d", a->reg, a->bit)
>>> +INSN(SBIC,   "$%d, %d", a->reg, a->bit)
>>> +INSN(SBIS,   "$%d, %d", a->reg, a->bit)
>>> +INSN(SBIW,   "r%d:r%d, %d", a->rd + 1, a->rd, a->imm)
>>> +INSN(SBRC,   "r%d, %d", a->rr, a->bit)
>>> +INSN(SBRS,   "r%d, %d", a->rr, a->bit)
>>> +INSN(SLEEP,  "")
>>> +INSN(SPM,    "")
>>> +INSN(SPMX,   "Z+")
>>> +INSN(STDY,   "r%d, Y+%d", a->rd, a->imm)
>>> +INSN(STDZ,   "r%d, Z+%d", a->rd, a->imm)
>>> +INSN(STS,    "r%d, %d", a->rd, a->imm)
>>> +INSN(STX1,   "r%d, X", a->rr)
>>> +INSN(STX2,   "r%d, X+", a->rr)
>>> +INSN(STX3,   "r%d, -X", a->rr)
>>> +INSN(STY2,   "r%d, Y+", a->rd)
>>> +INSN(STY3,   "r%d, -Y", a->rd)
>>> +INSN(STZ2,   "r%d, Z+", a->rd)
>>> +INSN(STZ3,   "r%d, -Z", a->rd)
>>> +INSN(SUB,    "r%d, r%d", a->rd, a->rr)
>>> +INSN(SUBI,   "r%d, %d", a->rd, a->imm)
>>> +INSN(SWAP,   "r%d", a->rd)
>>> +INSN(WDR,    "")
>>> +INSN(XCH,    "Z, r%d", a->rd)
>>> +
>>> diff --git a/target/avr/translate.c b/target/avr/translate.c
>>> index c8c6f798bf..a621195817 100644
>>> --- a/target/avr/translate.c
>>> +++ b/target/avr/translate.c
>>> @@ -2917,6 +2917,17 @@ done_generating:
>>>
>>>      tb->size = (ctx.npc - pc_start) * 2;
>>>      tb->icount = num_insns;
>>> +
>>> +#ifdef DEBUG_DISAS
>>> +    if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
>>> +        && qemu_log_in_addr_range(tb->pc)) {
>>> +        qemu_log_lock();
>>> +        qemu_log("IN: %s\n", lookup_symbol(tb->pc));
>>> +        log_target_disas(cs, tb->pc, tb->size);
>>> +        qemu_log("\n");
>>> +        qemu_log_unlock();
>>> +    }
>>> +#endif
>>>  }
>>>
>>>  void restore_state_to_opc(CPUAVRState *env, TranslationBlock *tb,
>>> --
>>> 2.17.2 (Apple Git-113)
>>>
>>>
>
> --
> Best Regards,
> Michael Rolnik
>
Michael Rolnik Dec. 10, 2019, 7:18 a.m. UTC | #4 
You are right. See at the bottom of the file. There is a comment about it

Sent from my cell phone, please ignore typos

On Tue, Dec 10, 2019, 6:21 AM Aleksandar Markovic <
aleksandar.m.mail@gmail.com> wrote:

>
>
> On Monday, December 9, 2019, Michael Rolnik <mrolnik@gmail.com> wrote:
>
>> Hi Aleksandar.
>>
>> 1. all instructions are 16 bit long except CALL & JMP they are 32 bit long
>>
>
> Accordingto the doc, LDS and STS also have 32-bit coding.
>
>
>
>> 2. next_word_used is set to true by next_word when called by append_16
>> when CALL & JMP are parsed
>>
>> Regards,
>> Michael Rolnik
>>
>> On Mon, Dec 9, 2019 at 8:10 PM Aleksandar Markovic <
>> aleksandar.m.mail@gmail.com> wrote:
>>
>>>
>>>
>>> On Sunday, December 8, 2019, Michael Rolnik <mrolnik@gmail.com> wrote:
>>>
>>>> Provide function disassembles executed instruction when `-d in_asm` is
>>>> provided
>>>>
>>>> Example:
>>>> `./avr-softmmu/qemu-system-avr -bios
>>>> free-rtos/Demo/AVR_ATMega2560_GCC/demo.elf -d in_asm` will produce
>>>> something like the following
>>>>
>>>> ```
>>>>     ...
>>>>     IN:
>>>>     0x0000014a:  CALL      0x3808
>>>>
>>>>     IN: main
>>>>     0x00003808:  CALL      0x4b4
>>>>
>>>>     IN: vParTestInitialise
>>>>     0x000004b4:  LDI       r24, 255
>>>>     0x000004b6:  STS       r24, 0
>>>>     0x000004b8:  MULS      r16, r20
>>>>     0x000004ba:  OUT       $1, r24
>>>>     0x000004bc:  LDS       r24, 0
>>>>     0x000004be:  MULS      r16, r20
>>>>     0x000004c0:  OUT       $2, r24
>>>>     0x000004c2:  RET
>>>>     ...
>>>> ```
>>>>
>>>> Signed-off-by: Michael Rolnik <mrolnik@gmail.com>
>>>> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
>>>> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>> Suggested-by: Aleksandar Markovic <aleksandar.m.mail@gmail.com>
>>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>> ---
>>>>  target/avr/cpu.h       |   1 +
>>>>  target/avr/cpu.c       |   2 +-
>>>>  target/avr/disas.c     | 226 +++++++++++++++++++++++++++++++++++++++++
>>>>  target/avr/translate.c |  11 ++
>>>>  4 files changed, 239 insertions(+), 1 deletion(-)
>>>>  create mode 100644 target/avr/disas.c
>>>>
>>>> diff --git a/target/avr/cpu.h b/target/avr/cpu.h
>>>> index c217eefeb4..a8a3e7ade6 100644
>>>> --- a/target/avr/cpu.h
>>>> +++ b/target/avr/cpu.h
>>>> @@ -178,6 +178,7 @@ bool avr_cpu_exec_interrupt(CPUState *cpu, int
>>>> int_req);
>>>>  hwaddr avr_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
>>>>  int avr_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
>>>>  int avr_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
>>>> +int avr_print_insn(bfd_vma addr, disassemble_info *info);
>>>>
>>>>  static inline int avr_feature(CPUAVRState *env, int feature)
>>>>  {
>>>> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
>>>> index c5cafcae3c..be4b921e4d 100644
>>>> --- a/target/avr/cpu.c
>>>> +++ b/target/avr/cpu.c
>>>> @@ -83,7 +83,7 @@ static void avr_cpu_reset(CPUState *cs)
>>>>  static void avr_cpu_disas_set_info(CPUState *cpu, disassemble_info
>>>> *info)
>>>>  {
>>>>      info->mach = bfd_arch_avr;
>>>> -    info->print_insn = NULL;
>>>> +    info->print_insn = avr_print_insn;
>>>>  }
>>>>
>>>>  static void avr_cpu_realizefn(DeviceState *dev, Error **errp)
>>>> diff --git a/target/avr/disas.c b/target/avr/disas.c
>>>> new file mode 100644
>>>> index 0000000000..22863d2eb1
>>>> --- /dev/null
>>>> +++ b/target/avr/disas.c
>>>> @@ -0,0 +1,226 @@
>>>> +/*
>>>> + * AVR disassembler
>>>> + *
>>>> + * Copyright (c) 2019 Richard Henderson <rth@twiddle.net>
>>>> + * Copyright (c) 2019 Michael Rolnik <mrolnik@gmail.com>
>>>> + *
>>>> + * This program is free software: you can redistribute it and/or modify
>>>> + * it under the terms of the GNU General Public License as published by
>>>> + * the Free Software Foundation, either version 2 of the License, or
>>>> + * (at your option) any later version.
>>>> + *
>>>> + * This program is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>>> + * GNU General Public License for more details.
>>>> + *
>>>> + * You should have received a copy of the GNU General Public License
>>>> + * along with this program.  If not, see <http://www.gnu.org/licenses/
>>>> >.
>>>> + */
>>>> +
>>>> +#include "qemu/osdep.h"
>>>> +#include "cpu.h"
>>>> +
>>>> +typedef struct {
>>>> +    disassemble_info *info;
>>>> +    uint16_t next_word;
>>>> +    bool next_word_used;
>>>> +} DisasContext;
>>>> +
>>>> +static int to_regs_16_31_by_one(DisasContext *ctx, int indx)
>>>> +{
>>>> +    return 16 + (indx % 16);
>>>> +}
>>>> +
>>>> +static int to_regs_16_23_by_one(DisasContext *ctx, int indx)
>>>> +{
>>>> +    return 16 + (indx % 8);
>>>> +}
>>>> +static int to_regs_24_30_by_two(DisasContext *ctx, int indx)
>>>> +{
>>>> +    return 24 + (indx % 4) * 2;
>>>> +}
>>>> +static int to_regs_00_30_by_two(DisasContext *ctx, int indx)
>>>> +{
>>>> +    return (indx % 16) * 2;
>>>> +}
>>>> +
>>>> +static uint16_t next_word(DisasContext *ctx)
>>>> +{
>>>> +    ctx->next_word_used = true;
>>>> +    return ctx->next_word;
>>>> +}
>>>> +
>>>> +static int append_16(DisasContext *ctx, int x)
>>>> +{
>>>> +    return x << 16 | next_word(ctx);
>>>> +}
>>>> +
>>>> +
>>>> +/* Include the auto-generated decoder.  */
>>>> +static bool decode_insn(DisasContext *ctx, uint16_t insn);
>>>> +#include "decode_insn.inc.c"
>>>> +
>>>> +#define output(mnemonic, format, ...) \
>>>> +    (pctx->info->fprintf_func(pctx->info->stream, "%-9s " format, \
>>>> +                        mnemonic, ##__VA_ARGS__))
>>>> +
>>>> +int avr_print_insn(bfd_vma addr, disassemble_info *info)
>>>> +{
>>>> +    DisasContext ctx;
>>>> +    DisasContext *pctx = &ctx;
>>>> +    bfd_byte buffer[4];
>>>> +    uint16_t insn;
>>>> +    int status;
>>>> +
>>>> +    ctx.info = info;
>>>> +
>>>> +    status = info->read_memory_func(addr, buffer, 4, info);
>>>> +    if (status != 0) {
>>>> +        info->memory_error_func(status, addr, info);
>>>> +        return -1;
>>>> +    }
>>>> +    insn = bfd_getl16(buffer);
>>>> +    ctx.next_word = bfd_getl16(buffer + 2);
>>>> +    ctx.next_word_used = false;
>>>> +
>>>> +    if (!decode_insn(&ctx, insn)) {
>>>> +        output(".db", "0x%02x, 0x%02x", buffer[0], buffer[1]);
>>>> +    }
>>>> +
>>>> +    return ctx.next_word_used ? 4 : 2;
>>>> +}
>>>
>>>
>>> Hi, Michael,
>>>
>>> What is the role of ctx.next_word_used, if it looks it is always "false"?
>>>
>>> Related to this, how do you disassemble 32-bit-coded instructions? From
>>> this patch, it looks you treat all AVR instructions as 16-bit-coded?
>>>
>>> Thanks,
>>> Aleksandar
>>>
>>>
>>>> +
>>>> +
>>>> +#define INSN(opcode, format, ...)
>>>>  \
>>>> +static bool trans_##opcode(DisasContext *pctx, arg_##opcode * a)
>>>>   \
>>>> +{
>>>>  \
>>>> +    output(#opcode, format, ##__VA_ARGS__);
>>>>  \
>>>> +    return true;
>>>>   \
>>>> +}
>>>> +
>>>> +#define INSN_MNEMONIC(opcode, mnemonic, format, ...)
>>>>   \
>>>> +static bool trans_##opcode(DisasContext *pctx, arg_##opcode * a)
>>>>   \
>>>> +{
>>>>  \
>>>> +    output(mnemonic, format, ##__VA_ARGS__);
>>>>   \
>>>> +    return true;
>>>>   \
>>>> +}
>>>> +
>>>> +/*
>>>> + *   C       Z       N       V       S       H       T       I
>>>> + *   0       1       2       3       4       5       6       7
>>>> + */
>>>> +static const char *brbc[] = {
>>>> +    "BRCC", "BRNE", "BRPL", "BRVC", "BRGE", "BRHC", "BRTC", "BRID"
>>>> +};
>>>> +
>>>> +static const char *brbs[] = {
>>>> +    "BRCS", "BREQ", "BRMI", "BRVS", "BRLT", "BRHS", "BRTS", "BRIE"
>>>> +};
>>>> +
>>>> +static const char *bset[] = {
>>>> +    "SEC",  "SEZ",  "SEN",  "SEZ",  "SES",  "SEH",  "SET",  "SEI"
>>>> +};
>>>> +
>>>> +static const char *bclr[] = {
>>>> +    "CLC",  "CLZ",  "CLN",  "CLZ",  "CLS",  "CLH",  "CLT",  "CLI"
>>>> +};
>>>> +
>>>> +INSN(ADC,    "r%d, r%d", a->rd, a->rr)
>>>> +INSN(ADD,    "r%d, r%d", a->rd, a->rr)
>>>> +INSN(ADIW,   "r%d:r%d, %d", a->rd + 1, a->rd, a->imm)
>>>> +INSN(AND,    "r%d, r%d", a->rd, a->rr)
>>>> +INSN(ANDI,   "r%d, %d", a->rd, a->imm)
>>>> +INSN(ASR,    "r%d", a->rd)
>>>> +INSN_MNEMONIC(BCLR,  bclr[a->bit], "")
>>>> +INSN(BLD,    "r%d, %d", a->rd, a->bit)
>>>> +INSN_MNEMONIC(BRBC,  brbc[a->bit], ".%+d", a->imm * 2)
>>>> +INSN_MNEMONIC(BRBS,  brbs[a->bit], ".%+d", a->imm * 2)
>>>> +INSN(BREAK,  "")
>>>> +INSN_MNEMONIC(BSET,  bset[a->bit], "")
>>>> +INSN(BST,    "r%d, %d", a->rd, a->bit)
>>>> +INSN(CALL,   "0x%x", a->imm * 2)
>>>> +INSN(CBI,    "%d, %d", a->reg, a->bit)
>>>> +INSN(COM,    "r%d", a->rd)
>>>> +INSN(CP,     "r%d, r%d", a->rd, a->rr)
>>>> +INSN(CPC,    "r%d, r%d", a->rd, a->rr)
>>>> +INSN(CPI,    "r%d, %d", a->rd, a->imm)
>>>> +INSN(CPSE,   "r%d, r%d", a->rd, a->rr)
>>>> +INSN(DEC,    "r%d", a->rd)
>>>> +INSN(DES,    "%d", a->imm)
>>>> +INSN(EICALL, "")
>>>> +INSN(EIJMP,  "")
>>>> +INSN(ELPM1,  "")
>>>> +INSN(ELPM2,  "r%d, Z", a->rd)
>>>> +INSN(ELPMX,  "r%d, Z+", a->rd)
>>>> +INSN(EOR,    "r%d, r%d", a->rd, a->rr)
>>>> +INSN(FMUL,   "r%d, r%d", a->rd, a->rr)
>>>> +INSN(FMULS,  "r%d, r%d", a->rd, a->rr)
>>>> +INSN(FMULSU, "r%d, r%d", a->rd, a->rr)
>>>> +INSN(ICALL,  "")
>>>> +INSN(IJMP,   "")
>>>> +INSN(IN,     "r%d, $%d", a->rd, a->imm)
>>>> +INSN(INC,    "r%d", a->rd)
>>>> +INSN(JMP,    "0x%x", a->imm * 2)
>>>> +INSN(LAC,    "Z, r%d", a->rd)
>>>> +INSN(LAS,    "Z, r%d", a->rd)
>>>> +INSN(LAT,    "Z, r%d", a->rd)
>>>> +INSN(LDDY,   "r%d, Y+%d", a->rd, a->imm)
>>>> +INSN(LDDZ,   "r%d, Z+%d", a->rd, a->imm)
>>>> +INSN(LDI,    "r%d, %d", a->rd, a->imm)
>>>> +INSN(LDS,    "r%d, %d", a->rd, a->imm)
>>>> +INSN(LDX1,   "r%d, X", a->rd)
>>>> +INSN(LDX2,   "r%d, X+", a->rd)
>>>> +INSN(LDX3,   "r%d, -X", a->rd)
>>>> +INSN(LDY2,   "r%d, Y+", a->rd)
>>>> +INSN(LDY3,   "r%d, -Y", a->rd)
>>>> +INSN(LDZ2,   "r%d, Z+", a->rd)
>>>> +INSN(LDZ3,   "r%d, -Z", a->rd)
>>>> +INSN(LPM1,   "")
>>>> +INSN(LPM2,   "r%d, Z", a->rd)
>>>> +INSN(LPMX,   "r%d, Z+", a->rd)
>>>> +INSN(LSR,    "r%d", a->rd)
>>>> +INSN(MOV,    "r%d, r%d", a->rd, a->rr)
>>>> +INSN(MOVW,   "r%d:r%d, r%d,r:r%d", a->rd + 1, a->rd, a->rr + 1, a->rr)
>>>> +INSN(MUL,    "r%d, r%d", a->rd, a->rr)
>>>> +INSN(MULS,   "r%d, r%d", a->rd, a->rr)
>>>> +INSN(MULSU,  "r%d, r%d", a->rd, a->rr)
>>>> +INSN(NEG,    "r%d", a->rd)
>>>> +INSN(NOP,    "")
>>>> +INSN(OR,     "r%d, r%d", a->rd, a->rr)
>>>> +INSN(ORI,    "r%d, %d", a->rd, a->imm)
>>>> +INSN(OUT,    "$%d, r%d", a->imm, a->rd)
>>>> +INSN(POP,    "r%d", a->rd)
>>>> +INSN(PUSH,   "r%d", a->rd)
>>>> +INSN(RCALL,  ".%+d", a->imm * 2)
>>>> +INSN(RET,    "")
>>>> +INSN(RETI,   "")
>>>> +INSN(RJMP,   ".%+d", a->imm * 2)
>>>> +INSN(ROR,    "r%d", a->rd)
>>>> +INSN(SBC,    "r%d, r%d", a->rd, a->rr)
>>>> +INSN(SBCI,   "r%d, %d", a->rd, a->imm)
>>>> +INSN(SBI,    "$%d, %d", a->reg, a->bit)
>>>> +INSN(SBIC,   "$%d, %d", a->reg, a->bit)
>>>> +INSN(SBIS,   "$%d, %d", a->reg, a->bit)
>>>> +INSN(SBIW,   "r%d:r%d, %d", a->rd + 1, a->rd, a->imm)
>>>> +INSN(SBRC,   "r%d, %d", a->rr, a->bit)
>>>> +INSN(SBRS,   "r%d, %d", a->rr, a->bit)
>>>> +INSN(SLEEP,  "")
>>>> +INSN(SPM,    "")
>>>> +INSN(SPMX,   "Z+")
>>>> +INSN(STDY,   "r%d, Y+%d", a->rd, a->imm)
>>>> +INSN(STDZ,   "r%d, Z+%d", a->rd, a->imm)
>>>> +INSN(STS,    "r%d, %d", a->rd, a->imm)
>>>> +INSN(STX1,   "r%d, X", a->rr)
>>>> +INSN(STX2,   "r%d, X+", a->rr)
>>>> +INSN(STX3,   "r%d, -X", a->rr)
>>>> +INSN(STY2,   "r%d, Y+", a->rd)
>>>> +INSN(STY3,   "r%d, -Y", a->rd)
>>>> +INSN(STZ2,   "r%d, Z+", a->rd)
>>>> +INSN(STZ3,   "r%d, -Z", a->rd)
>>>> +INSN(SUB,    "r%d, r%d", a->rd, a->rr)
>>>> +INSN(SUBI,   "r%d, %d", a->rd, a->imm)
>>>> +INSN(SWAP,   "r%d", a->rd)
>>>> +INSN(WDR,    "")
>>>> +INSN(XCH,    "Z, r%d", a->rd)
>>>> +
>>>> diff --git a/target/avr/translate.c b/target/avr/translate.c
>>>> index c8c6f798bf..a621195817 100644
>>>> --- a/target/avr/translate.c
>>>> +++ b/target/avr/translate.c
>>>> @@ -2917,6 +2917,17 @@ done_generating:
>>>>
>>>>      tb->size = (ctx.npc - pc_start) * 2;
>>>>      tb->icount = num_insns;
>>>> +
>>>> +#ifdef DEBUG_DISAS
>>>> +    if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
>>>> +        && qemu_log_in_addr_range(tb->pc)) {
>>>> +        qemu_log_lock();
>>>> +        qemu_log("IN: %s\n", lookup_symbol(tb->pc));
>>>> +        log_target_disas(cs, tb->pc, tb->size);
>>>> +        qemu_log("\n");
>>>> +        qemu_log_unlock();
>>>> +    }
>>>> +#endif
>>>>  }
>>>>
>>>>  void restore_state_to_opc(CPUAVRState *env, TranslationBlock *tb,
>>>> --
>>>> 2.17.2 (Apple Git-113)
>>>>
>>>>
>>
>> --
>> Best Regards,
>> Michael Rolnik
>>
>
Aleksandar Markovic Dec. 12, 2019, 9:12 a.m. UTC | #5 
On Tue, Dec 10, 2019 at 8:18 AM Michael Rolnik <mrolnik@gmail.com> wrote:
>
> You are right. See at the bottom of the file. There is a comment about it
>

Sorry, what file?

I also see that you disassemble instructions regardless of what AVR
CPU the current executable is built for, don't you? OK, not a very big
deal, but can be confusing for end user if disassembly text of an
instruction that is not supported by a particular CPU is displayed as
if it is supported.

> Sent from my cell phone, please ignore typos
>
> On Tue, Dec 10, 2019, 6:21 AM Aleksandar Markovic <aleksandar.m.mail@gmail.com> wrote:
>>
>>
>>
>> On Monday, December 9, 2019, Michael Rolnik <mrolnik@gmail.com> wrote:
>>>
>>> Hi Aleksandar.
>>>
>>> 1. all instructions are 16 bit long except CALL & JMP they are 32 bit long
>>
>>
>> Accordingto the doc, LDS and STS also have 32-bit coding.
>>
>>
>>>
>>> 2. next_word_used is set to true by next_word when called by append_16 when CALL & JMP are parsed
>>>
>>> Regards,
>>> Michael Rolnik
>>>
>>> On Mon, Dec 9, 2019 at 8:10 PM Aleksandar Markovic <aleksandar.m.mail@gmail.com> wrote:
>>>>
>>>>
>>>>
>>>> On Sunday, December 8, 2019, Michael Rolnik <mrolnik@gmail.com> wrote:
>>>>>
>>>>> Provide function disassembles executed instruction when `-d in_asm` is
>>>>> provided
>>>>>
>>>>> Example:
>>>>> `./avr-softmmu/qemu-system-avr -bios free-rtos/Demo/AVR_ATMega2560_GCC/demo.elf -d in_asm` will produce something like the following
>>>>>
>>>>> ```
>>>>>     ...
>>>>>     IN:
>>>>>     0x0000014a:  CALL      0x3808
>>>>>
>>>>>     IN: main
>>>>>     0x00003808:  CALL      0x4b4
>>>>>
>>>>>     IN: vParTestInitialise
>>>>>     0x000004b4:  LDI       r24, 255
>>>>>     0x000004b6:  STS       r24, 0
>>>>>     0x000004b8:  MULS      r16, r20
>>>>>     0x000004ba:  OUT       $1, r24
>>>>>     0x000004bc:  LDS       r24, 0
>>>>>     0x000004be:  MULS      r16, r20
>>>>>     0x000004c0:  OUT       $2, r24
>>>>>     0x000004c2:  RET
>>>>>     ...
>>>>> ```
>>>>>
>>>>> Signed-off-by: Michael Rolnik <mrolnik@gmail.com>
>>>>> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
>>>>> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>> Suggested-by: Aleksandar Markovic <aleksandar.m.mail@gmail.com>
>>>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>> ---
>>>>>  target/avr/cpu.h       |   1 +
>>>>>  target/avr/cpu.c       |   2 +-
>>>>>  target/avr/disas.c     | 226 +++++++++++++++++++++++++++++++++++++++++
>>>>>  target/avr/translate.c |  11 ++
>>>>>  4 files changed, 239 insertions(+), 1 deletion(-)
>>>>>  create mode 100644 target/avr/disas.c
>>>>>
>>>>> diff --git a/target/avr/cpu.h b/target/avr/cpu.h
>>>>> index c217eefeb4..a8a3e7ade6 100644
>>>>> --- a/target/avr/cpu.h
>>>>> +++ b/target/avr/cpu.h
>>>>> @@ -178,6 +178,7 @@ bool avr_cpu_exec_interrupt(CPUState *cpu, int int_req);
>>>>>  hwaddr avr_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
>>>>>  int avr_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
>>>>>  int avr_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
>>>>> +int avr_print_insn(bfd_vma addr, disassemble_info *info);
>>>>>
>>>>>  static inline int avr_feature(CPUAVRState *env, int feature)
>>>>>  {
>>>>> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
>>>>> index c5cafcae3c..be4b921e4d 100644
>>>>> --- a/target/avr/cpu.c
>>>>> +++ b/target/avr/cpu.c
>>>>> @@ -83,7 +83,7 @@ static void avr_cpu_reset(CPUState *cs)
>>>>>  static void avr_cpu_disas_set_info(CPUState *cpu, disassemble_info *info)
>>>>>  {
>>>>>      info->mach = bfd_arch_avr;
>>>>> -    info->print_insn = NULL;
>>>>> +    info->print_insn = avr_print_insn;
>>>>>  }
>>>>>
>>>>>  static void avr_cpu_realizefn(DeviceState *dev, Error **errp)
>>>>> diff --git a/target/avr/disas.c b/target/avr/disas.c
>>>>> new file mode 100644
>>>>> index 0000000000..22863d2eb1
>>>>> --- /dev/null
>>>>> +++ b/target/avr/disas.c
>>>>> @@ -0,0 +1,226 @@
>>>>> +/*
>>>>> + * AVR disassembler
>>>>> + *
>>>>> + * Copyright (c) 2019 Richard Henderson <rth@twiddle.net>
>>>>> + * Copyright (c) 2019 Michael Rolnik <mrolnik@gmail.com>
>>>>> + *
>>>>> + * This program is free software: you can redistribute it and/or modify
>>>>> + * it under the terms of the GNU General Public License as published by
>>>>> + * the Free Software Foundation, either version 2 of the License, or
>>>>> + * (at your option) any later version.
>>>>> + *
>>>>> + * This program is distributed in the hope that it will be useful,
>>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>>>> + * GNU General Public License for more details.
>>>>> + *
>>>>> + * You should have received a copy of the GNU General Public License
>>>>> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>>>> + */
>>>>> +
>>>>> +#include "qemu/osdep.h"
>>>>> +#include "cpu.h"
>>>>> +
>>>>> +typedef struct {
>>>>> +    disassemble_info *info;
>>>>> +    uint16_t next_word;
>>>>> +    bool next_word_used;
>>>>> +} DisasContext;
>>>>> +
>>>>> +static int to_regs_16_31_by_one(DisasContext *ctx, int indx)
>>>>> +{
>>>>> +    return 16 + (indx % 16);
>>>>> +}
>>>>> +
>>>>> +static int to_regs_16_23_by_one(DisasContext *ctx, int indx)
>>>>> +{
>>>>> +    return 16 + (indx % 8);
>>>>> +}
>>>>> +static int to_regs_24_30_by_two(DisasContext *ctx, int indx)
>>>>> +{
>>>>> +    return 24 + (indx % 4) * 2;
>>>>> +}
>>>>> +static int to_regs_00_30_by_two(DisasContext *ctx, int indx)
>>>>> +{
>>>>> +    return (indx % 16) * 2;
>>>>> +}
>>>>> +
>>>>> +static uint16_t next_word(DisasContext *ctx)
>>>>> +{
>>>>> +    ctx->next_word_used = true;
>>>>> +    return ctx->next_word;
>>>>> +}
>>>>> +
>>>>> +static int append_16(DisasContext *ctx, int x)
>>>>> +{
>>>>> +    return x << 16 | next_word(ctx);
>>>>> +}
>>>>> +
>>>>> +
>>>>> +/* Include the auto-generated decoder.  */
>>>>> +static bool decode_insn(DisasContext *ctx, uint16_t insn);
>>>>> +#include "decode_insn.inc.c"
>>>>> +
>>>>> +#define output(mnemonic, format, ...) \
>>>>> +    (pctx->info->fprintf_func(pctx->info->stream, "%-9s " format, \
>>>>> +                        mnemonic, ##__VA_ARGS__))
>>>>> +
>>>>> +int avr_print_insn(bfd_vma addr, disassemble_info *info)
>>>>> +{
>>>>> +    DisasContext ctx;
>>>>> +    DisasContext *pctx = &ctx;
>>>>> +    bfd_byte buffer[4];
>>>>> +    uint16_t insn;
>>>>> +    int status;
>>>>> +
>>>>> +    ctx.info = info;
>>>>> +
>>>>> +    status = info->read_memory_func(addr, buffer, 4, info);
>>>>> +    if (status != 0) {
>>>>> +        info->memory_error_func(status, addr, info);
>>>>> +        return -1;
>>>>> +    }
>>>>> +    insn = bfd_getl16(buffer);
>>>>> +    ctx.next_word = bfd_getl16(buffer + 2);
>>>>> +    ctx.next_word_used = false;
>>>>> +
>>>>> +    if (!decode_insn(&ctx, insn)) {
>>>>> +        output(".db", "0x%02x, 0x%02x", buffer[0], buffer[1]);
>>>>> +    }
>>>>> +
>>>>> +    return ctx.next_word_used ? 4 : 2;
>>>>> +}
>>>>
>>>>
>>>> Hi, Michael,
>>>>
>>>> What is the role of ctx.next_word_used, if it looks it is always "false"?
>>>>
>>>> Related to this, how do you disassemble 32-bit-coded instructions? From this patch, it looks you treat all AVR instructions as 16-bit-coded?
>>>>
>>>> Thanks,
>>>> Aleksandar
>>>>
>>>>>
>>>>> +
>>>>> +
>>>>> +#define INSN(opcode, format, ...)                                       \
>>>>> +static bool trans_##opcode(DisasContext *pctx, arg_##opcode * a)        \
>>>>> +{                                                                       \
>>>>> +    output(#opcode, format, ##__VA_ARGS__);                             \
>>>>> +    return true;                                                        \
>>>>> +}
>>>>> +
>>>>> +#define INSN_MNEMONIC(opcode, mnemonic, format, ...)                    \
>>>>> +static bool trans_##opcode(DisasContext *pctx, arg_##opcode * a)        \
>>>>> +{                                                                       \
>>>>> +    output(mnemonic, format, ##__VA_ARGS__);                            \
>>>>> +    return true;                                                        \
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + *   C       Z       N       V       S       H       T       I
>>>>> + *   0       1       2       3       4       5       6       7
>>>>> + */
>>>>> +static const char *brbc[] = {
>>>>> +    "BRCC", "BRNE", "BRPL", "BRVC", "BRGE", "BRHC", "BRTC", "BRID"
>>>>> +};
>>>>> +
>>>>> +static const char *brbs[] = {
>>>>> +    "BRCS", "BREQ", "BRMI", "BRVS", "BRLT", "BRHS", "BRTS", "BRIE"
>>>>> +};
>>>>> +
>>>>> +static const char *bset[] = {
>>>>> +    "SEC",  "SEZ",  "SEN",  "SEZ",  "SES",  "SEH",  "SET",  "SEI"
>>>>> +};
>>>>> +
>>>>> +static const char *bclr[] = {
>>>>> +    "CLC",  "CLZ",  "CLN",  "CLZ",  "CLS",  "CLH",  "CLT",  "CLI"
>>>>> +};
>>>>> +
>>>>> +INSN(ADC,    "r%d, r%d", a->rd, a->rr)
>>>>> +INSN(ADD,    "r%d, r%d", a->rd, a->rr)
>>>>> +INSN(ADIW,   "r%d:r%d, %d", a->rd + 1, a->rd, a->imm)
>>>>> +INSN(AND,    "r%d, r%d", a->rd, a->rr)
>>>>> +INSN(ANDI,   "r%d, %d", a->rd, a->imm)
>>>>> +INSN(ASR,    "r%d", a->rd)
>>>>> +INSN_MNEMONIC(BCLR,  bclr[a->bit], "")
>>>>> +INSN(BLD,    "r%d, %d", a->rd, a->bit)
>>>>> +INSN_MNEMONIC(BRBC,  brbc[a->bit], ".%+d", a->imm * 2)
>>>>> +INSN_MNEMONIC(BRBS,  brbs[a->bit], ".%+d", a->imm * 2)
>>>>> +INSN(BREAK,  "")
>>>>> +INSN_MNEMONIC(BSET,  bset[a->bit], "")
>>>>> +INSN(BST,    "r%d, %d", a->rd, a->bit)
>>>>> +INSN(CALL,   "0x%x", a->imm * 2)
>>>>> +INSN(CBI,    "%d, %d", a->reg, a->bit)
>>>>> +INSN(COM,    "r%d", a->rd)
>>>>> +INSN(CP,     "r%d, r%d", a->rd, a->rr)
>>>>> +INSN(CPC,    "r%d, r%d", a->rd, a->rr)
>>>>> +INSN(CPI,    "r%d, %d", a->rd, a->imm)
>>>>> +INSN(CPSE,   "r%d, r%d", a->rd, a->rr)
>>>>> +INSN(DEC,    "r%d", a->rd)
>>>>> +INSN(DES,    "%d", a->imm)
>>>>> +INSN(EICALL, "")
>>>>> +INSN(EIJMP,  "")
>>>>> +INSN(ELPM1,  "")
>>>>> +INSN(ELPM2,  "r%d, Z", a->rd)
>>>>> +INSN(ELPMX,  "r%d, Z+", a->rd)
>>>>> +INSN(EOR,    "r%d, r%d", a->rd, a->rr)
>>>>> +INSN(FMUL,   "r%d, r%d", a->rd, a->rr)
>>>>> +INSN(FMULS,  "r%d, r%d", a->rd, a->rr)
>>>>> +INSN(FMULSU, "r%d, r%d", a->rd, a->rr)
>>>>> +INSN(ICALL,  "")
>>>>> +INSN(IJMP,   "")
>>>>> +INSN(IN,     "r%d, $%d", a->rd, a->imm)
>>>>> +INSN(INC,    "r%d", a->rd)
>>>>> +INSN(JMP,    "0x%x", a->imm * 2)
>>>>> +INSN(LAC,    "Z, r%d", a->rd)
>>>>> +INSN(LAS,    "Z, r%d", a->rd)
>>>>> +INSN(LAT,    "Z, r%d", a->rd)
>>>>> +INSN(LDDY,   "r%d, Y+%d", a->rd, a->imm)
>>>>> +INSN(LDDZ,   "r%d, Z+%d", a->rd, a->imm)
>>>>> +INSN(LDI,    "r%d, %d", a->rd, a->imm)
>>>>> +INSN(LDS,    "r%d, %d", a->rd, a->imm)
>>>>> +INSN(LDX1,   "r%d, X", a->rd)
>>>>> +INSN(LDX2,   "r%d, X+", a->rd)
>>>>> +INSN(LDX3,   "r%d, -X", a->rd)
>>>>> +INSN(LDY2,   "r%d, Y+", a->rd)
>>>>> +INSN(LDY3,   "r%d, -Y", a->rd)
>>>>> +INSN(LDZ2,   "r%d, Z+", a->rd)
>>>>> +INSN(LDZ3,   "r%d, -Z", a->rd)
>>>>> +INSN(LPM1,   "")
>>>>> +INSN(LPM2,   "r%d, Z", a->rd)
>>>>> +INSN(LPMX,   "r%d, Z+", a->rd)
>>>>> +INSN(LSR,    "r%d", a->rd)
>>>>> +INSN(MOV,    "r%d, r%d", a->rd, a->rr)
>>>>> +INSN(MOVW,   "r%d:r%d, r%d,r:r%d", a->rd + 1, a->rd, a->rr + 1, a->rr)
>>>>> +INSN(MUL,    "r%d, r%d", a->rd, a->rr)
>>>>> +INSN(MULS,   "r%d, r%d", a->rd, a->rr)
>>>>> +INSN(MULSU,  "r%d, r%d", a->rd, a->rr)
>>>>> +INSN(NEG,    "r%d", a->rd)
>>>>> +INSN(NOP,    "")
>>>>> +INSN(OR,     "r%d, r%d", a->rd, a->rr)
>>>>> +INSN(ORI,    "r%d, %d", a->rd, a->imm)
>>>>> +INSN(OUT,    "$%d, r%d", a->imm, a->rd)
>>>>> +INSN(POP,    "r%d", a->rd)
>>>>> +INSN(PUSH,   "r%d", a->rd)
>>>>> +INSN(RCALL,  ".%+d", a->imm * 2)
>>>>> +INSN(RET,    "")
>>>>> +INSN(RETI,   "")
>>>>> +INSN(RJMP,   ".%+d", a->imm * 2)
>>>>> +INSN(ROR,    "r%d", a->rd)
>>>>> +INSN(SBC,    "r%d, r%d", a->rd, a->rr)
>>>>> +INSN(SBCI,   "r%d, %d", a->rd, a->imm)
>>>>> +INSN(SBI,    "$%d, %d", a->reg, a->bit)
>>>>> +INSN(SBIC,   "$%d, %d", a->reg, a->bit)
>>>>> +INSN(SBIS,   "$%d, %d", a->reg, a->bit)
>>>>> +INSN(SBIW,   "r%d:r%d, %d", a->rd + 1, a->rd, a->imm)
>>>>> +INSN(SBRC,   "r%d, %d", a->rr, a->bit)
>>>>> +INSN(SBRS,   "r%d, %d", a->rr, a->bit)
>>>>> +INSN(SLEEP,  "")
>>>>> +INSN(SPM,    "")
>>>>> +INSN(SPMX,   "Z+")
>>>>> +INSN(STDY,   "r%d, Y+%d", a->rd, a->imm)
>>>>> +INSN(STDZ,   "r%d, Z+%d", a->rd, a->imm)
>>>>> +INSN(STS,    "r%d, %d", a->rd, a->imm)
>>>>> +INSN(STX1,   "r%d, X", a->rr)
>>>>> +INSN(STX2,   "r%d, X+", a->rr)
>>>>> +INSN(STX3,   "r%d, -X", a->rr)
>>>>> +INSN(STY2,   "r%d, Y+", a->rd)
>>>>> +INSN(STY3,   "r%d, -Y", a->rd)
>>>>> +INSN(STZ2,   "r%d, Z+", a->rd)
>>>>> +INSN(STZ3,   "r%d, -Z", a->rd)
>>>>> +INSN(SUB,    "r%d, r%d", a->rd, a->rr)
>>>>> +INSN(SUBI,   "r%d, %d", a->rd, a->imm)
>>>>> +INSN(SWAP,   "r%d", a->rd)
>>>>> +INSN(WDR,    "")
>>>>> +INSN(XCH,    "Z, r%d", a->rd)
>>>>> +
>>>>> diff --git a/target/avr/translate.c b/target/avr/translate.c
>>>>> index c8c6f798bf..a621195817 100644
>>>>> --- a/target/avr/translate.c
>>>>> +++ b/target/avr/translate.c
>>>>> @@ -2917,6 +2917,17 @@ done_generating:
>>>>>
>>>>>      tb->size = (ctx.npc - pc_start) * 2;
>>>>>      tb->icount = num_insns;
>>>>> +
>>>>> +#ifdef DEBUG_DISAS
>>>>> +    if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
>>>>> +        && qemu_log_in_addr_range(tb->pc)) {
>>>>> +        qemu_log_lock();
>>>>> +        qemu_log("IN: %s\n", lookup_symbol(tb->pc));
>>>>> +        log_target_disas(cs, tb->pc, tb->size);
>>>>> +        qemu_log("\n");
>>>>> +        qemu_log_unlock();
>>>>> +    }
>>>>> +#endif
>>>>>  }
>>>>>
>>>>>  void restore_state_to_opc(CPUAVRState *env, TranslationBlock *tb,
>>>>> --
>>>>> 2.17.2 (Apple Git-113)
>>>>>
>>>
>>>
>>> --
>>> Best Regards,
>>> Michael Rolnik
Michael Rolnik Dec. 17, 2019, 9:24 p.m. UTC | #6 
Aleksandar.

1. inst.decode file
2. avr features are not accessible from avr_print_insn as it does not
receive a pointer to CPU context. So, there is not way to inform the user
that some instructions are not supported unless I define several
different avr_print_insn functions.

Regards,
Michael Rolnik



On Thu, Dec 12, 2019 at 11:12 AM Aleksandar Markovic <
aleksandar.m.mail@gmail.com> wrote:

> On Tue, Dec 10, 2019 at 8:18 AM Michael Rolnik <mrolnik@gmail.com> wrote:
> >
> > You are right. See at the bottom of the file. There is a comment about it
> >
>
> Sorry, what file?
>
> I also see that you disassemble instructions regardless of what AVR
> CPU the current executable is built for, don't you? OK, not a very big
> deal, but can be confusing for end user if disassembly text of an
> instruction that is not supported by a particular CPU is displayed as
> if it is supported.
>
> > Sent from my cell phone, please ignore typos
> >
> > On Tue, Dec 10, 2019, 6:21 AM Aleksandar Markovic <
> aleksandar.m.mail@gmail.com> wrote:
> >>
> >>
> >>
> >> On Monday, December 9, 2019, Michael Rolnik <mrolnik@gmail.com> wrote:
> >>>
> >>> Hi Aleksandar.
> >>>
> >>> 1. all instructions are 16 bit long except CALL & JMP they are 32 bit
> long
> >>
> >>
> >> Accordingto the doc, LDS and STS also have 32-bit coding.
> >>
> >>
> >>>
> >>> 2. next_word_used is set to true by next_word when called by append_16
> when CALL & JMP are parsed
> >>>
> >>> Regards,
> >>> Michael Rolnik
> >>>
> >>> On Mon, Dec 9, 2019 at 8:10 PM Aleksandar Markovic <
> aleksandar.m.mail@gmail.com> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On Sunday, December 8, 2019, Michael Rolnik <mrolnik@gmail.com>
> wrote:
> >>>>>
> >>>>> Provide function disassembles executed instruction when `-d in_asm`
> is
> >>>>> provided
> >>>>>
> >>>>> Example:
> >>>>> `./avr-softmmu/qemu-system-avr -bios
> free-rtos/Demo/AVR_ATMega2560_GCC/demo.elf -d in_asm` will produce
> something like the following
> >>>>>
> >>>>> ```
> >>>>>     ...
> >>>>>     IN:
> >>>>>     0x0000014a:  CALL      0x3808
> >>>>>
> >>>>>     IN: main
> >>>>>     0x00003808:  CALL      0x4b4
> >>>>>
> >>>>>     IN: vParTestInitialise
> >>>>>     0x000004b4:  LDI       r24, 255
> >>>>>     0x000004b6:  STS       r24, 0
> >>>>>     0x000004b8:  MULS      r16, r20
> >>>>>     0x000004ba:  OUT       $1, r24
> >>>>>     0x000004bc:  LDS       r24, 0
> >>>>>     0x000004be:  MULS      r16, r20
> >>>>>     0x000004c0:  OUT       $2, r24
> >>>>>     0x000004c2:  RET
> >>>>>     ...
> >>>>> ```
> >>>>>
> >>>>> Signed-off-by: Michael Rolnik <mrolnik@gmail.com>
> >>>>> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> >>>>> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >>>>> Suggested-by: Aleksandar Markovic <aleksandar.m.mail@gmail.com>
> >>>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >>>>> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >>>>> ---
> >>>>>  target/avr/cpu.h       |   1 +
> >>>>>  target/avr/cpu.c       |   2 +-
> >>>>>  target/avr/disas.c     | 226
> +++++++++++++++++++++++++++++++++++++++++
> >>>>>  target/avr/translate.c |  11 ++
> >>>>>  4 files changed, 239 insertions(+), 1 deletion(-)
> >>>>>  create mode 100644 target/avr/disas.c
> >>>>>
> >>>>> diff --git a/target/avr/cpu.h b/target/avr/cpu.h
> >>>>> index c217eefeb4..a8a3e7ade6 100644
> >>>>> --- a/target/avr/cpu.h
> >>>>> +++ b/target/avr/cpu.h
> >>>>> @@ -178,6 +178,7 @@ bool avr_cpu_exec_interrupt(CPUState *cpu, int
> int_req);
> >>>>>  hwaddr avr_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
> >>>>>  int avr_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
> >>>>>  int avr_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int
> reg);
> >>>>> +int avr_print_insn(bfd_vma addr, disassemble_info *info);
> >>>>>
> >>>>>  static inline int avr_feature(CPUAVRState *env, int feature)
> >>>>>  {
> >>>>> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
> >>>>> index c5cafcae3c..be4b921e4d 100644
> >>>>> --- a/target/avr/cpu.c
> >>>>> +++ b/target/avr/cpu.c
> >>>>> @@ -83,7 +83,7 @@ static void avr_cpu_reset(CPUState *cs)
> >>>>>  static void avr_cpu_disas_set_info(CPUState *cpu, disassemble_info
> *info)
> >>>>>  {
> >>>>>      info->mach = bfd_arch_avr;
> >>>>> -    info->print_insn = NULL;
> >>>>> +    info->print_insn = avr_print_insn;
> >>>>>  }
> >>>>>
> >>>>>  static void avr_cpu_realizefn(DeviceState *dev, Error **errp)
> >>>>> diff --git a/target/avr/disas.c b/target/avr/disas.c
> >>>>> new file mode 100644
> >>>>> index 0000000000..22863d2eb1
> >>>>> --- /dev/null
> >>>>> +++ b/target/avr/disas.c
> >>>>> @@ -0,0 +1,226 @@
> >>>>> +/*
> >>>>> + * AVR disassembler
> >>>>> + *
> >>>>> + * Copyright (c) 2019 Richard Henderson <rth@twiddle.net>
> >>>>> + * Copyright (c) 2019 Michael Rolnik <mrolnik@gmail.com>
> >>>>> + *
> >>>>> + * This program is free software: you can redistribute it and/or
> modify
> >>>>> + * it under the terms of the GNU General Public License as
> published by
> >>>>> + * the Free Software Foundation, either version 2 of the License, or
> >>>>> + * (at your option) any later version.
> >>>>> + *
> >>>>> + * This program is distributed in the hope that it will be useful,
> >>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >>>>> + * GNU General Public License for more details.
> >>>>> + *
> >>>>> + * You should have received a copy of the GNU General Public License
> >>>>> + * along with this program.  If not, see <
> http://www.gnu.org/licenses/>.
> >>>>> + */
> >>>>> +
> >>>>> +#include "qemu/osdep.h"
> >>>>> +#include "cpu.h"
> >>>>> +
> >>>>> +typedef struct {
> >>>>> +    disassemble_info *info;
> >>>>> +    uint16_t next_word;
> >>>>> +    bool next_word_used;
> >>>>> +} DisasContext;
> >>>>> +
> >>>>> +static int to_regs_16_31_by_one(DisasContext *ctx, int indx)
> >>>>> +{
> >>>>> +    return 16 + (indx % 16);
> >>>>> +}
> >>>>> +
> >>>>> +static int to_regs_16_23_by_one(DisasContext *ctx, int indx)
> >>>>> +{
> >>>>> +    return 16 + (indx % 8);
> >>>>> +}
> >>>>> +static int to_regs_24_30_by_two(DisasContext *ctx, int indx)
> >>>>> +{
> >>>>> +    return 24 + (indx % 4) * 2;
> >>>>> +}
> >>>>> +static int to_regs_00_30_by_two(DisasContext *ctx, int indx)
> >>>>> +{
> >>>>> +    return (indx % 16) * 2;
> >>>>> +}
> >>>>> +
> >>>>> +static uint16_t next_word(DisasContext *ctx)
> >>>>> +{
> >>>>> +    ctx->next_word_used = true;
> >>>>> +    return ctx->next_word;
> >>>>> +}
> >>>>> +
> >>>>> +static int append_16(DisasContext *ctx, int x)
> >>>>> +{
> >>>>> +    return x << 16 | next_word(ctx);
> >>>>> +}
> >>>>> +
> >>>>> +
> >>>>> +/* Include the auto-generated decoder.  */
> >>>>> +static bool decode_insn(DisasContext *ctx, uint16_t insn);
> >>>>> +#include "decode_insn.inc.c"
> >>>>> +
> >>>>> +#define output(mnemonic, format, ...) \
> >>>>> +    (pctx->info->fprintf_func(pctx->info->stream, "%-9s " format, \
> >>>>> +                        mnemonic, ##__VA_ARGS__))
> >>>>> +
> >>>>> +int avr_print_insn(bfd_vma addr, disassemble_info *info)
> >>>>> +{
> >>>>> +    DisasContext ctx;
> >>>>> +    DisasContext *pctx = &ctx;
> >>>>> +    bfd_byte buffer[4];
> >>>>> +    uint16_t insn;
> >>>>> +    int status;
> >>>>> +
> >>>>> +    ctx.info = info;
> >>>>> +
> >>>>> +    status = info->read_memory_func(addr, buffer, 4, info);
> >>>>> +    if (status != 0) {
> >>>>> +        info->memory_error_func(status, addr, info);
> >>>>> +        return -1;
> >>>>> +    }
> >>>>> +    insn = bfd_getl16(buffer);
> >>>>> +    ctx.next_word = bfd_getl16(buffer + 2);
> >>>>> +    ctx.next_word_used = false;
> >>>>> +
> >>>>> +    if (!decode_insn(&ctx, insn)) {
> >>>>> +        output(".db", "0x%02x, 0x%02x", buffer[0], buffer[1]);
> >>>>> +    }
> >>>>> +
> >>>>> +    return ctx.next_word_used ? 4 : 2;
> >>>>> +}
> >>>>
> >>>>
> >>>> Hi, Michael,
> >>>>
> >>>> What is the role of ctx.next_word_used, if it looks it is always
> "false"?
> >>>>
> >>>> Related to this, how do you disassemble 32-bit-coded instructions?
> From this patch, it looks you treat all AVR instructions as 16-bit-coded?
> >>>>
> >>>> Thanks,
> >>>> Aleksandar
> >>>>
> >>>>>
> >>>>> +
> >>>>> +
> >>>>> +#define INSN(opcode, format, ...)
>      \
> >>>>> +static bool trans_##opcode(DisasContext *pctx, arg_##opcode * a)
>     \
> >>>>> +{
>      \
> >>>>> +    output(#opcode, format, ##__VA_ARGS__);
>      \
> >>>>> +    return true;
>     \
> >>>>> +}
> >>>>> +
> >>>>> +#define INSN_MNEMONIC(opcode, mnemonic, format, ...)
>     \
> >>>>> +static bool trans_##opcode(DisasContext *pctx, arg_##opcode * a)
>     \
> >>>>> +{
>      \
> >>>>> +    output(mnemonic, format, ##__VA_ARGS__);
>     \
> >>>>> +    return true;
>     \
> >>>>> +}
> >>>>> +
> >>>>> +/*
> >>>>> + *   C       Z       N       V       S       H       T       I
> >>>>> + *   0       1       2       3       4       5       6       7
> >>>>> + */
> >>>>> +static const char *brbc[] = {
> >>>>> +    "BRCC", "BRNE", "BRPL", "BRVC", "BRGE", "BRHC", "BRTC", "BRID"
> >>>>> +};
> >>>>> +
> >>>>> +static const char *brbs[] = {
> >>>>> +    "BRCS", "BREQ", "BRMI", "BRVS", "BRLT", "BRHS", "BRTS", "BRIE"
> >>>>> +};
> >>>>> +
> >>>>> +static const char *bset[] = {
> >>>>> +    "SEC",  "SEZ",  "SEN",  "SEZ",  "SES",  "SEH",  "SET",  "SEI"
> >>>>> +};
> >>>>> +
> >>>>> +static const char *bclr[] = {
> >>>>> +    "CLC",  "CLZ",  "CLN",  "CLZ",  "CLS",  "CLH",  "CLT",  "CLI"
> >>>>> +};
> >>>>> +
> >>>>> +INSN(ADC,    "r%d, r%d", a->rd, a->rr)
> >>>>> +INSN(ADD,    "r%d, r%d", a->rd, a->rr)
> >>>>> +INSN(ADIW,   "r%d:r%d, %d", a->rd + 1, a->rd, a->imm)
> >>>>> +INSN(AND,    "r%d, r%d", a->rd, a->rr)
> >>>>> +INSN(ANDI,   "r%d, %d", a->rd, a->imm)
> >>>>> +INSN(ASR,    "r%d", a->rd)
> >>>>> +INSN_MNEMONIC(BCLR,  bclr[a->bit], "")
> >>>>> +INSN(BLD,    "r%d, %d", a->rd, a->bit)
> >>>>> +INSN_MNEMONIC(BRBC,  brbc[a->bit], ".%+d", a->imm * 2)
> >>>>> +INSN_MNEMONIC(BRBS,  brbs[a->bit], ".%+d", a->imm * 2)
> >>>>> +INSN(BREAK,  "")
> >>>>> +INSN_MNEMONIC(BSET,  bset[a->bit], "")
> >>>>> +INSN(BST,    "r%d, %d", a->rd, a->bit)
> >>>>> +INSN(CALL,   "0x%x", a->imm * 2)
> >>>>> +INSN(CBI,    "%d, %d", a->reg, a->bit)
> >>>>> +INSN(COM,    "r%d", a->rd)
> >>>>> +INSN(CP,     "r%d, r%d", a->rd, a->rr)
> >>>>> +INSN(CPC,    "r%d, r%d", a->rd, a->rr)
> >>>>> +INSN(CPI,    "r%d, %d", a->rd, a->imm)
> >>>>> +INSN(CPSE,   "r%d, r%d", a->rd, a->rr)
> >>>>> +INSN(DEC,    "r%d", a->rd)
> >>>>> +INSN(DES,    "%d", a->imm)
> >>>>> +INSN(EICALL, "")
> >>>>> +INSN(EIJMP,  "")
> >>>>> +INSN(ELPM1,  "")
> >>>>> +INSN(ELPM2,  "r%d, Z", a->rd)
> >>>>> +INSN(ELPMX,  "r%d, Z+", a->rd)
> >>>>> +INSN(EOR,    "r%d, r%d", a->rd, a->rr)
> >>>>> +INSN(FMUL,   "r%d, r%d", a->rd, a->rr)
> >>>>> +INSN(FMULS,  "r%d, r%d", a->rd, a->rr)
> >>>>> +INSN(FMULSU, "r%d, r%d", a->rd, a->rr)
> >>>>> +INSN(ICALL,  "")
> >>>>> +INSN(IJMP,   "")
> >>>>> +INSN(IN,     "r%d, $%d", a->rd, a->imm)
> >>>>> +INSN(INC,    "r%d", a->rd)
> >>>>> +INSN(JMP,    "0x%x", a->imm * 2)
> >>>>> +INSN(LAC,    "Z, r%d", a->rd)
> >>>>> +INSN(LAS,    "Z, r%d", a->rd)
> >>>>> +INSN(LAT,    "Z, r%d", a->rd)
> >>>>> +INSN(LDDY,   "r%d, Y+%d", a->rd, a->imm)
> >>>>> +INSN(LDDZ,   "r%d, Z+%d", a->rd, a->imm)
> >>>>> +INSN(LDI,    "r%d, %d", a->rd, a->imm)
> >>>>> +INSN(LDS,    "r%d, %d", a->rd, a->imm)
> >>>>> +INSN(LDX1,   "r%d, X", a->rd)
> >>>>> +INSN(LDX2,   "r%d, X+", a->rd)
> >>>>> +INSN(LDX3,   "r%d, -X", a->rd)
> >>>>> +INSN(LDY2,   "r%d, Y+", a->rd)
> >>>>> +INSN(LDY3,   "r%d, -Y", a->rd)
> >>>>> +INSN(LDZ2,   "r%d, Z+", a->rd)
> >>>>> +INSN(LDZ3,   "r%d, -Z", a->rd)
> >>>>> +INSN(LPM1,   "")
> >>>>> +INSN(LPM2,   "r%d, Z", a->rd)
> >>>>> +INSN(LPMX,   "r%d, Z+", a->rd)
> >>>>> +INSN(LSR,    "r%d", a->rd)
> >>>>> +INSN(MOV,    "r%d, r%d", a->rd, a->rr)
> >>>>> +INSN(MOVW,   "r%d:r%d, r%d,r:r%d", a->rd + 1, a->rd, a->rr + 1,
> a->rr)
> >>>>> +INSN(MUL,    "r%d, r%d", a->rd, a->rr)
> >>>>> +INSN(MULS,   "r%d, r%d", a->rd, a->rr)
> >>>>> +INSN(MULSU,  "r%d, r%d", a->rd, a->rr)
> >>>>> +INSN(NEG,    "r%d", a->rd)
> >>>>> +INSN(NOP,    "")
> >>>>> +INSN(OR,     "r%d, r%d", a->rd, a->rr)
> >>>>> +INSN(ORI,    "r%d, %d", a->rd, a->imm)
> >>>>> +INSN(OUT,    "$%d, r%d", a->imm, a->rd)
> >>>>> +INSN(POP,    "r%d", a->rd)
> >>>>> +INSN(PUSH,   "r%d", a->rd)
> >>>>> +INSN(RCALL,  ".%+d", a->imm * 2)
> >>>>> +INSN(RET,    "")
> >>>>> +INSN(RETI,   "")
> >>>>> +INSN(RJMP,   ".%+d", a->imm * 2)
> >>>>> +INSN(ROR,    "r%d", a->rd)
> >>>>> +INSN(SBC,    "r%d, r%d", a->rd, a->rr)
> >>>>> +INSN(SBCI,   "r%d, %d", a->rd, a->imm)
> >>>>> +INSN(SBI,    "$%d, %d", a->reg, a->bit)
> >>>>> +INSN(SBIC,   "$%d, %d", a->reg, a->bit)
> >>>>> +INSN(SBIS,   "$%d, %d", a->reg, a->bit)
> >>>>> +INSN(SBIW,   "r%d:r%d, %d", a->rd + 1, a->rd, a->imm)
> >>>>> +INSN(SBRC,   "r%d, %d", a->rr, a->bit)
> >>>>> +INSN(SBRS,   "r%d, %d", a->rr, a->bit)
> >>>>> +INSN(SLEEP,  "")
> >>>>> +INSN(SPM,    "")
> >>>>> +INSN(SPMX,   "Z+")
> >>>>> +INSN(STDY,   "r%d, Y+%d", a->rd, a->imm)
> >>>>> +INSN(STDZ,   "r%d, Z+%d", a->rd, a->imm)
> >>>>> +INSN(STS,    "r%d, %d", a->rd, a->imm)
> >>>>> +INSN(STX1,   "r%d, X", a->rr)
> >>>>> +INSN(STX2,   "r%d, X+", a->rr)
> >>>>> +INSN(STX3,   "r%d, -X", a->rr)
> >>>>> +INSN(STY2,   "r%d, Y+", a->rd)
> >>>>> +INSN(STY3,   "r%d, -Y", a->rd)
> >>>>> +INSN(STZ2,   "r%d, Z+", a->rd)
> >>>>> +INSN(STZ3,   "r%d, -Z", a->rd)
> >>>>> +INSN(SUB,    "r%d, r%d", a->rd, a->rr)
> >>>>> +INSN(SUBI,   "r%d, %d", a->rd, a->imm)
> >>>>> +INSN(SWAP,   "r%d", a->rd)
> >>>>> +INSN(WDR,    "")
> >>>>> +INSN(XCH,    "Z, r%d", a->rd)
> >>>>> +
> >>>>> diff --git a/target/avr/translate.c b/target/avr/translate.c
> >>>>> index c8c6f798bf..a621195817 100644
> >>>>> --- a/target/avr/translate.c
> >>>>> +++ b/target/avr/translate.c
> >>>>> @@ -2917,6 +2917,17 @@ done_generating:
> >>>>>
> >>>>>      tb->size = (ctx.npc - pc_start) * 2;
> >>>>>      tb->icount = num_insns;
> >>>>> +
> >>>>> +#ifdef DEBUG_DISAS
> >>>>> +    if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
> >>>>> +        && qemu_log_in_addr_range(tb->pc)) {
> >>>>> +        qemu_log_lock();
> >>>>> +        qemu_log("IN: %s\n", lookup_symbol(tb->pc));
> >>>>> +        log_target_disas(cs, tb->pc, tb->size);
> >>>>> +        qemu_log("\n");
> >>>>> +        qemu_log_unlock();
> >>>>> +    }
> >>>>> +#endif
> >>>>>  }
> >>>>>
> >>>>>  void restore_state_to_opc(CPUAVRState *env, TranslationBlock *tb,
> >>>>> --
> >>>>> 2.17.2 (Apple Git-113)
> >>>>>
> >>>
> >>>
> >>> --
> >>> Best Regards,
> >>> Michael Rolnik
>
Aleksandar Markovic Dec. 17, 2019, 10:33 p.m. UTC | #7 
On Tuesday, December 17, 2019, Michael Rolnik <mrolnik@gmail.com> wrote:

> Aleksandar.
>
> 1. inst.decode file
> 2. avr features are not accessible from avr_print_insn as it does not
> receive a pointer to CPU context. So, there is not way to inform the user
> that some instructions are not supported unless I define several
> different avr_print_insn functions.
>
>
OK, this is not a crucial feature. If I were you, I would leave it for
future, as one of "nice to have" things. It is possible to implement it, of
course, with some additions to the decoder, but don't spend your energy on
that now, that is my advice.

But patch 1 restructuring is a must. You have to form several logical units
out of it.

inst.decode is written to be convenient to the author (you), but it should
be convenient to the reader, please rearrange items to be as in the ISA
document (even though we both know it is not convenient to you).

The review takes forever, but you are up to one of the most serious tasks
in QEMU, so it is expected, no reason to worry.

Best regards,

Aleksandar



> Regards,
> Michael Rolnik
>
>
>
> On Thu, Dec 12, 2019 at 11:12 AM Aleksandar Markovic <
> aleksandar.m.mail@gmail.com> wrote:
>
>> On Tue, Dec 10, 2019 at 8:18 AM Michael Rolnik <mrolnik@gmail.com> wrote:
>> >
>> > You are right. See at the bottom of the file. There is a comment about
>> it
>> >
>>
>> Sorry, what file?
>>
>> I also see that you disassemble instructions regardless of what AVR
>> CPU the current executable is built for, don't you? OK, not a very big
>> deal, but can be confusing for end user if disassembly text of an
>> instruction that is not supported by a particular CPU is displayed as
>> if it is supported.
>>
>> > Sent from my cell phone, please ignore typos
>> >
>> > On Tue, Dec 10, 2019, 6:21 AM Aleksandar Markovic <
>> aleksandar.m.mail@gmail.com> wrote:
>> >>
>> >>
>> >>
>> >> On Monday, December 9, 2019, Michael Rolnik <mrolnik@gmail.com> wrote:
>> >>>
>> >>> Hi Aleksandar.
>> >>>
>> >>> 1. all instructions are 16 bit long except CALL & JMP they are 32 bit
>> long
>> >>
>> >>
>> >> Accordingto the doc, LDS and STS also have 32-bit coding.
>> >>
>> >>
>> >>>
>> >>> 2. next_word_used is set to true by next_word when called by
>> append_16 when CALL & JMP are parsed
>> >>>
>> >>> Regards,
>> >>> Michael Rolnik
>> >>>
>> >>> On Mon, Dec 9, 2019 at 8:10 PM Aleksandar Markovic <
>> aleksandar.m.mail@gmail.com> wrote:
>> >>>>
>> >>>>
>> >>>>
>> >>>> On Sunday, December 8, 2019, Michael Rolnik <mrolnik@gmail.com>
>> wrote:
>> >>>>>
>> >>>>> Provide function disassembles executed instruction when `-d in_asm`
>> is
>> >>>>> provided
>> >>>>>
>> >>>>> Example:
>> >>>>> `./avr-softmmu/qemu-system-avr -bios free-rtos/Demo/AVR_ATMega2560_GCC/demo.elf
>> -d in_asm` will produce something like the following
>> >>>>>
>> >>>>> ```
>> >>>>>     ...
>> >>>>>     IN:
>> >>>>>     0x0000014a:  CALL      0x3808
>> >>>>>
>> >>>>>     IN: main
>> >>>>>     0x00003808:  CALL      0x4b4
>> >>>>>
>> >>>>>     IN: vParTestInitialise
>> >>>>>     0x000004b4:  LDI       r24, 255
>> >>>>>     0x000004b6:  STS       r24, 0
>> >>>>>     0x000004b8:  MULS      r16, r20
>> >>>>>     0x000004ba:  OUT       $1, r24
>> >>>>>     0x000004bc:  LDS       r24, 0
>> >>>>>     0x000004be:  MULS      r16, r20
>> >>>>>     0x000004c0:  OUT       $2, r24
>> >>>>>     0x000004c2:  RET
>> >>>>>     ...
>> >>>>> ```
>> >>>>>
>> >>>>> Signed-off-by: Michael Rolnik <mrolnik@gmail.com>
>> >>>>> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
>> >>>>> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> >>>>> Suggested-by: Aleksandar Markovic <aleksandar.m.mail@gmail.com>
>> >>>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> >>>>> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> >>>>> ---
>> >>>>>  target/avr/cpu.h       |   1 +
>> >>>>>  target/avr/cpu.c       |   2 +-
>> >>>>>  target/avr/disas.c     | 226 ++++++++++++++++++++++++++++++
>> +++++++++++
>> >>>>>  target/avr/translate.c |  11 ++
>> >>>>>  4 files changed, 239 insertions(+), 1 deletion(-)
>> >>>>>  create mode 100644 target/avr/disas.c
>> >>>>>
>> >>>>> diff --git a/target/avr/cpu.h b/target/avr/cpu.h
>> >>>>> index c217eefeb4..a8a3e7ade6 100644
>> >>>>> --- a/target/avr/cpu.h
>> >>>>> +++ b/target/avr/cpu.h
>> >>>>> @@ -178,6 +178,7 @@ bool avr_cpu_exec_interrupt(CPUState *cpu, int
>> int_req);
>> >>>>>  hwaddr avr_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
>> >>>>>  int avr_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int
>> reg);
>> >>>>>  int avr_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int
>> reg);
>> >>>>> +int avr_print_insn(bfd_vma addr, disassemble_info *info);
>> >>>>>
>> >>>>>  static inline int avr_feature(CPUAVRState *env, int feature)
>> >>>>>  {
>> >>>>> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
>> >>>>> index c5cafcae3c..be4b921e4d 100644
>> >>>>> --- a/target/avr/cpu.c
>> >>>>> +++ b/target/avr/cpu.c
>> >>>>> @@ -83,7 +83,7 @@ static void avr_cpu_reset(CPUState *cs)
>> >>>>>  static void avr_cpu_disas_set_info(CPUState *cpu,
>> disassemble_info *info)
>> >>>>>  {
>> >>>>>      info->mach = bfd_arch_avr;
>> >>>>> -    info->print_insn = NULL;
>> >>>>> +    info->print_insn = avr_print_insn;
>> >>>>>  }
>> >>>>>
>> >>>>>  static void avr_cpu_realizefn(DeviceState *dev, Error **errp)
>> >>>>> diff --git a/target/avr/disas.c b/target/avr/disas.c
>> >>>>> new file mode 100644
>> >>>>> index 0000000000..22863d2eb1
>> >>>>> --- /dev/null
>> >>>>> +++ b/target/avr/disas.c
>> >>>>> @@ -0,0 +1,226 @@
>> >>>>> +/*
>> >>>>> + * AVR disassembler
>> >>>>> + *
>> >>>>> + * Copyright (c) 2019 Richard Henderson <rth@twiddle.net>
>> >>>>> + * Copyright (c) 2019 Michael Rolnik <mrolnik@gmail.com>
>> >>>>> + *
>> >>>>> + * This program is free software: you can redistribute it and/or
>> modify
>> >>>>> + * it under the terms of the GNU General Public License as
>> published by
>> >>>>> + * the Free Software Foundation, either version 2 of the License,
>> or
>> >>>>> + * (at your option) any later version.
>> >>>>> + *
>> >>>>> + * This program is distributed in the hope that it will be useful,
>> >>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> >>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> >>>>> + * GNU General Public License for more details.
>> >>>>> + *
>> >>>>> + * You should have received a copy of the GNU General Public
>> License
>> >>>>> + * along with this program.  If not, see <
>> http://www.gnu.org/licenses/>.
>> >>>>> + */
>> >>>>> +
>> >>>>> +#include "qemu/osdep.h"
>> >>>>> +#include "cpu.h"
>> >>>>> +
>> >>>>> +typedef struct {
>> >>>>> +    disassemble_info *info;
>> >>>>> +    uint16_t next_word;
>> >>>>> +    bool next_word_used;
>> >>>>> +} DisasContext;
>> >>>>> +
>> >>>>> +static int to_regs_16_31_by_one(DisasContext *ctx, int indx)
>> >>>>> +{
>> >>>>> +    return 16 + (indx % 16);
>> >>>>> +}
>> >>>>> +
>> >>>>> +static int to_regs_16_23_by_one(DisasContext *ctx, int indx)
>> >>>>> +{
>> >>>>> +    return 16 + (indx % 8);
>> >>>>> +}
>> >>>>> +static int to_regs_24_30_by_two(DisasContext *ctx, int indx)
>> >>>>> +{
>> >>>>> +    return 24 + (indx % 4) * 2;
>> >>>>> +}
>> >>>>> +static int to_regs_00_30_by_two(DisasContext *ctx, int indx)
>> >>>>> +{
>> >>>>> +    return (indx % 16) * 2;
>> >>>>> +}
>> >>>>> +
>> >>>>> +static uint16_t next_word(DisasContext *ctx)
>> >>>>> +{
>> >>>>> +    ctx->next_word_used = true;
>> >>>>> +    return ctx->next_word;
>> >>>>> +}
>> >>>>> +
>> >>>>> +static int append_16(DisasContext *ctx, int x)
>> >>>>> +{
>> >>>>> +    return x << 16 | next_word(ctx);
>> >>>>> +}
>> >>>>> +
>> >>>>> +
>> >>>>> +/* Include the auto-generated decoder.  */
>> >>>>> +static bool decode_insn(DisasContext *ctx, uint16_t insn);
>> >>>>> +#include "decode_insn.inc.c"
>> >>>>> +
>> >>>>> +#define output(mnemonic, format, ...) \
>> >>>>> +    (pctx->info->fprintf_func(pctx->info->stream, "%-9s " format,
>> \
>> >>>>> +                        mnemonic, ##__VA_ARGS__))
>> >>>>> +
>> >>>>> +int avr_print_insn(bfd_vma addr, disassemble_info *info)
>> >>>>> +{
>> >>>>> +    DisasContext ctx;
>> >>>>> +    DisasContext *pctx = &ctx;
>> >>>>> +    bfd_byte buffer[4];
>> >>>>> +    uint16_t insn;
>> >>>>> +    int status;
>> >>>>> +
>> >>>>> +    ctx.info = info;
>> >>>>> +
>> >>>>> +    status = info->read_memory_func(addr, buffer, 4, info);
>> >>>>> +    if (status != 0) {
>> >>>>> +        info->memory_error_func(status, addr, info);
>> >>>>> +        return -1;
>> >>>>> +    }
>> >>>>> +    insn = bfd_getl16(buffer);
>> >>>>> +    ctx.next_word = bfd_getl16(buffer + 2);
>> >>>>> +    ctx.next_word_used = false;
>> >>>>> +
>> >>>>> +    if (!decode_insn(&ctx, insn)) {
>> >>>>> +        output(".db", "0x%02x, 0x%02x", buffer[0], buffer[1]);
>> >>>>> +    }
>> >>>>> +
>> >>>>> +    return ctx.next_word_used ? 4 : 2;
>> >>>>> +}
>> >>>>
>> >>>>
>> >>>> Hi, Michael,
>> >>>>
>> >>>> What is the role of ctx.next_word_used, if it looks it is always
>> "false"?
>> >>>>
>> >>>> Related to this, how do you disassemble 32-bit-coded instructions?
>> From this patch, it looks you treat all AVR instructions as 16-bit-coded?
>> >>>>
>> >>>> Thanks,
>> >>>> Aleksandar
>> >>>>
>> >>>>>
>> >>>>> +
>> >>>>> +
>> >>>>> +#define INSN(opcode, format, ...)
>>      \
>> >>>>> +static bool trans_##opcode(DisasContext *pctx, arg_##opcode * a)
>>       \
>> >>>>> +{
>>      \
>> >>>>> +    output(#opcode, format, ##__VA_ARGS__);
>>      \
>> >>>>> +    return true;
>>       \
>> >>>>> +}
>> >>>>> +
>> >>>>> +#define INSN_MNEMONIC(opcode, mnemonic, format, ...)
>>       \
>> >>>>> +static bool trans_##opcode(DisasContext *pctx, arg_##opcode * a)
>>       \
>> >>>>> +{
>>      \
>> >>>>> +    output(mnemonic, format, ##__VA_ARGS__);
>>       \
>> >>>>> +    return true;
>>       \
>> >>>>> +}
>> >>>>> +
>> >>>>> +/*
>> >>>>> + *   C       Z       N       V       S       H       T       I
>> >>>>> + *   0       1       2       3       4       5       6       7
>> >>>>> + */
>> >>>>> +static const char *brbc[] = {
>> >>>>> +    "BRCC", "BRNE", "BRPL", "BRVC", "BRGE", "BRHC", "BRTC", "BRID"
>> >>>>> +};
>> >>>>> +
>> >>>>> +static const char *brbs[] = {
>> >>>>> +    "BRCS", "BREQ", "BRMI", "BRVS", "BRLT", "BRHS", "BRTS", "BRIE"
>> >>>>> +};
>> >>>>> +
>> >>>>> +static const char *bset[] = {
>> >>>>> +    "SEC",  "SEZ",  "SEN",  "SEZ",  "SES",  "SEH",  "SET",  "SEI"
>> >>>>> +};
>> >>>>> +
>> >>>>> +static const char *bclr[] = {
>> >>>>> +    "CLC",  "CLZ",  "CLN",  "CLZ",  "CLS",  "CLH",  "CLT",  "CLI"
>> >>>>> +};
>> >>>>> +
>> >>>>> +INSN(ADC,    "r%d, r%d", a->rd, a->rr)
>> >>>>> +INSN(ADD,    "r%d, r%d", a->rd, a->rr)
>> >>>>> +INSN(ADIW,   "r%d:r%d, %d", a->rd + 1, a->rd, a->imm)
>> >>>>> +INSN(AND,    "r%d, r%d", a->rd, a->rr)
>> >>>>> +INSN(ANDI,   "r%d, %d", a->rd, a->imm)
>> >>>>> +INSN(ASR,    "r%d", a->rd)
>> >>>>> +INSN_MNEMONIC(BCLR,  bclr[a->bit], "")
>> >>>>> +INSN(BLD,    "r%d, %d", a->rd, a->bit)
>> >>>>> +INSN_MNEMONIC(BRBC,  brbc[a->bit], ".%+d", a->imm * 2)
>> >>>>> +INSN_MNEMONIC(BRBS,  brbs[a->bit], ".%+d", a->imm * 2)
>> >>>>> +INSN(BREAK,  "")
>> >>>>> +INSN_MNEMONIC(BSET,  bset[a->bit], "")
>> >>>>> +INSN(BST,    "r%d, %d", a->rd, a->bit)
>> >>>>> +INSN(CALL,   "0x%x", a->imm * 2)
>> >>>>> +INSN(CBI,    "%d, %d", a->reg, a->bit)
>> >>>>> +INSN(COM,    "r%d", a->rd)
>> >>>>> +INSN(CP,     "r%d, r%d", a->rd, a->rr)
>> >>>>> +INSN(CPC,    "r%d, r%d", a->rd, a->rr)
>> >>>>> +INSN(CPI,    "r%d, %d", a->rd, a->imm)
>> >>>>> +INSN(CPSE,   "r%d, r%d", a->rd, a->rr)
>> >>>>> +INSN(DEC,    "r%d", a->rd)
>> >>>>> +INSN(DES,    "%d", a->imm)
>> >>>>> +INSN(EICALL, "")
>> >>>>> +INSN(EIJMP,  "")
>> >>>>> +INSN(ELPM1,  "")
>> >>>>> +INSN(ELPM2,  "r%d, Z", a->rd)
>> >>>>> +INSN(ELPMX,  "r%d, Z+", a->rd)
>> >>>>> +INSN(EOR,    "r%d, r%d", a->rd, a->rr)
>> >>>>> +INSN(FMUL,   "r%d, r%d", a->rd, a->rr)
>> >>>>> +INSN(FMULS,  "r%d, r%d", a->rd, a->rr)
>> >>>>> +INSN(FMULSU, "r%d, r%d", a->rd, a->rr)
>> >>>>> +INSN(ICALL,  "")
>> >>>>> +INSN(IJMP,   "")
>> >>>>> +INSN(IN,     "r%d, $%d", a->rd, a->imm)
>> >>>>> +INSN(INC,    "r%d", a->rd)
>> >>>>> +INSN(JMP,    "0x%x", a->imm * 2)
>> >>>>> +INSN(LAC,    "Z, r%d", a->rd)
>> >>>>> +INSN(LAS,    "Z, r%d", a->rd)
>> >>>>> +INSN(LAT,    "Z, r%d", a->rd)
>> >>>>> +INSN(LDDY,   "r%d, Y+%d", a->rd, a->imm)
>> >>>>> +INSN(LDDZ,   "r%d, Z+%d", a->rd, a->imm)
>> >>>>> +INSN(LDI,    "r%d, %d", a->rd, a->imm)
>> >>>>> +INSN(LDS,    "r%d, %d", a->rd, a->imm)
>> >>>>> +INSN(LDX1,   "r%d, X", a->rd)
>> >>>>> +INSN(LDX2,   "r%d, X+", a->rd)
>> >>>>> +INSN(LDX3,   "r%d, -X", a->rd)
>> >>>>> +INSN(LDY2,   "r%d, Y+", a->rd)
>> >>>>> +INSN(LDY3,   "r%d, -Y", a->rd)
>> >>>>> +INSN(LDZ2,   "r%d, Z+", a->rd)
>> >>>>> +INSN(LDZ3,   "r%d, -Z", a->rd)
>> >>>>> +INSN(LPM1,   "")
>> >>>>> +INSN(LPM2,   "r%d, Z", a->rd)
>> >>>>> +INSN(LPMX,   "r%d, Z+", a->rd)
>> >>>>> +INSN(LSR,    "r%d", a->rd)
>> >>>>> +INSN(MOV,    "r%d, r%d", a->rd, a->rr)
>> >>>>> +INSN(MOVW,   "r%d:r%d, r%d,r:r%d", a->rd + 1, a->rd, a->rr + 1,
>> a->rr)
>> >>>>> +INSN(MUL,    "r%d, r%d", a->rd, a->rr)
>> >>>>> +INSN(MULS,   "r%d, r%d", a->rd, a->rr)
>> >>>>> +INSN(MULSU,  "r%d, r%d", a->rd, a->rr)
>> >>>>> +INSN(NEG,    "r%d", a->rd)
>> >>>>> +INSN(NOP,    "")
>> >>>>> +INSN(OR,     "r%d, r%d", a->rd, a->rr)
>> >>>>> +INSN(ORI,    "r%d, %d", a->rd, a->imm)
>> >>>>> +INSN(OUT,    "$%d, r%d", a->imm, a->rd)
>> >>>>> +INSN(POP,    "r%d", a->rd)
>> >>>>> +INSN(PUSH,   "r%d", a->rd)
>> >>>>> +INSN(RCALL,  ".%+d", a->imm * 2)
>> >>>>> +INSN(RET,    "")
>> >>>>> +INSN(RETI,   "")
>> >>>>> +INSN(RJMP,   ".%+d", a->imm * 2)
>> >>>>> +INSN(ROR,    "r%d", a->rd)
>> >>>>> +INSN(SBC,    "r%d, r%d", a->rd, a->rr)
>> >>>>> +INSN(SBCI,   "r%d, %d", a->rd, a->imm)
>> >>>>> +INSN(SBI,    "$%d, %d", a->reg, a->bit)
>> >>>>> +INSN(SBIC,   "$%d, %d", a->reg, a->bit)
>> >>>>> +INSN(SBIS,   "$%d, %d", a->reg, a->bit)
>> >>>>> +INSN(SBIW,   "r%d:r%d, %d", a->rd + 1, a->rd, a->imm)
>> >>>>> +INSN(SBRC,   "r%d, %d", a->rr, a->bit)
>> >>>>> +INSN(SBRS,   "r%d, %d", a->rr, a->bit)
>> >>>>> +INSN(SLEEP,  "")
>> >>>>> +INSN(SPM,    "")
>> >>>>> +INSN(SPMX,   "Z+")
>> >>>>> +INSN(STDY,   "r%d, Y+%d", a->rd, a->imm)
>> >>>>> +INSN(STDZ,   "r%d, Z+%d", a->rd, a->imm)
>> >>>>> +INSN(STS,    "r%d, %d", a->rd, a->imm)
>> >>>>> +INSN(STX1,   "r%d, X", a->rr)
>> >>>>> +INSN(STX2,   "r%d, X+", a->rr)
>> >>>>> +INSN(STX3,   "r%d, -X", a->rr)
>> >>>>> +INSN(STY2,   "r%d, Y+", a->rd)
>> >>>>> +INSN(STY3,   "r%d, -Y", a->rd)
>> >>>>> +INSN(STZ2,   "r%d, Z+", a->rd)
>> >>>>> +INSN(STZ3,   "r%d, -Z", a->rd)
>> >>>>> +INSN(SUB,    "r%d, r%d", a->rd, a->rr)
>> >>>>> +INSN(SUBI,   "r%d, %d", a->rd, a->imm)
>> >>>>> +INSN(SWAP,   "r%d", a->rd)
>> >>>>> +INSN(WDR,    "")
>> >>>>> +INSN(XCH,    "Z, r%d", a->rd)
>> >>>>> +
>> >>>>> diff --git a/target/avr/translate.c b/target/avr/translate.c
>> >>>>> index c8c6f798bf..a621195817 100644
>> >>>>> --- a/target/avr/translate.c
>> >>>>> +++ b/target/avr/translate.c
>> >>>>> @@ -2917,6 +2917,17 @@ done_generating:
>> >>>>>
>> >>>>>      tb->size = (ctx.npc - pc_start) * 2;
>> >>>>>      tb->icount = num_insns;
>> >>>>> +
>> >>>>> +#ifdef DEBUG_DISAS
>> >>>>> +    if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
>> >>>>> +        && qemu_log_in_addr_range(tb->pc)) {
>> >>>>> +        qemu_log_lock();
>> >>>>> +        qemu_log("IN: %s\n", lookup_symbol(tb->pc));
>> >>>>> +        log_target_disas(cs, tb->pc, tb->size);
>> >>>>> +        qemu_log("\n");
>> >>>>> +        qemu_log_unlock();
>> >>>>> +    }
>> >>>>> +#endif
>> >>>>>  }
>> >>>>>
>> >>>>>  void restore_state_to_opc(CPUAVRState *env, TranslationBlock *tb,
>> >>>>> --
>> >>>>> 2.17.2 (Apple Git-113)
>> >>>>>
>> >>>
>> >>>
>> >>> --
>> >>> Best Regards,
>> >>> Michael Rolnik
>>
>
>
> --
> Best Regards,
> Michael Rolnik
>
diff mbox series

Patch

diff --git a/target/avr/cpu.h b/target/avr/cpu.h
index c217eefeb4..a8a3e7ade6 100644
--- a/target/avr/cpu.h
+++ b/target/avr/cpu.h
@@ -178,6 +178,7 @@  bool avr_cpu_exec_interrupt(CPUState *cpu, int int_req);
 hwaddr avr_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 int avr_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
 int avr_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
+int avr_print_insn(bfd_vma addr, disassemble_info *info);
 
 static inline int avr_feature(CPUAVRState *env, int feature)
 {
diff --git a/target/avr/cpu.c b/target/avr/cpu.c
index c5cafcae3c..be4b921e4d 100644
--- a/target/avr/cpu.c
+++ b/target/avr/cpu.c
@@ -83,7 +83,7 @@  static void avr_cpu_reset(CPUState *cs)
 static void avr_cpu_disas_set_info(CPUState *cpu, disassemble_info *info)
 {
     info->mach = bfd_arch_avr;
-    info->print_insn = NULL;
+    info->print_insn = avr_print_insn;
 }
 
 static void avr_cpu_realizefn(DeviceState *dev, Error **errp)
diff --git a/target/avr/disas.c b/target/avr/disas.c
new file mode 100644
index 0000000000..22863d2eb1
--- /dev/null
+++ b/target/avr/disas.c
@@ -0,0 +1,226 @@ 
+/*
+ * AVR disassembler
+ *
+ * Copyright (c) 2019 Richard Henderson <rth@twiddle.net>
+ * Copyright (c) 2019 Michael Rolnik <mrolnik@gmail.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+
+typedef struct {
+    disassemble_info *info;
+    uint16_t next_word;
+    bool next_word_used;
+} DisasContext;
+
+static int to_regs_16_31_by_one(DisasContext *ctx, int indx)
+{
+    return 16 + (indx % 16);
+}
+
+static int to_regs_16_23_by_one(DisasContext *ctx, int indx)
+{
+    return 16 + (indx % 8);
+}
+static int to_regs_24_30_by_two(DisasContext *ctx, int indx)
+{
+    return 24 + (indx % 4) * 2;
+}
+static int to_regs_00_30_by_two(DisasContext *ctx, int indx)
+{
+    return (indx % 16) * 2;
+}
+
+static uint16_t next_word(DisasContext *ctx)
+{
+    ctx->next_word_used = true;
+    return ctx->next_word;
+}
+
+static int append_16(DisasContext *ctx, int x)
+{
+    return x << 16 | next_word(ctx);
+}
+
+
+/* Include the auto-generated decoder.  */
+static bool decode_insn(DisasContext *ctx, uint16_t insn);
+#include "decode_insn.inc.c"
+
+#define output(mnemonic, format, ...) \
+    (pctx->info->fprintf_func(pctx->info->stream, "%-9s " format, \
+                        mnemonic, ##__VA_ARGS__))
+
+int avr_print_insn(bfd_vma addr, disassemble_info *info)
+{
+    DisasContext ctx;
+    DisasContext *pctx = &ctx;
+    bfd_byte buffer[4];
+    uint16_t insn;
+    int status;
+
+    ctx.info = info;
+
+    status = info->read_memory_func(addr, buffer, 4, info);
+    if (status != 0) {
+        info->memory_error_func(status, addr, info);
+        return -1;
+    }
+    insn = bfd_getl16(buffer);
+    ctx.next_word = bfd_getl16(buffer + 2);
+    ctx.next_word_used = false;
+
+    if (!decode_insn(&ctx, insn)) {
+        output(".db", "0x%02x, 0x%02x", buffer[0], buffer[1]);
+    }
+
+    return ctx.next_word_used ? 4 : 2;
+}
+
+
+#define INSN(opcode, format, ...)                                       \
+static bool trans_##opcode(DisasContext *pctx, arg_##opcode * a)        \
+{                                                                       \
+    output(#opcode, format, ##__VA_ARGS__);                             \
+    return true;                                                        \
+}
+
+#define INSN_MNEMONIC(opcode, mnemonic, format, ...)                    \
+static bool trans_##opcode(DisasContext *pctx, arg_##opcode * a)        \
+{                                                                       \
+    output(mnemonic, format, ##__VA_ARGS__);                            \
+    return true;                                                        \
+}
+
+/*
+ *   C       Z       N       V       S       H       T       I
+ *   0       1       2       3       4       5       6       7
+ */
+static const char *brbc[] = {
+    "BRCC", "BRNE", "BRPL", "BRVC", "BRGE", "BRHC", "BRTC", "BRID"
+};
+
+static const char *brbs[] = {
+    "BRCS", "BREQ", "BRMI", "BRVS", "BRLT", "BRHS", "BRTS", "BRIE"
+};
+
+static const char *bset[] = {
+    "SEC",  "SEZ",  "SEN",  "SEZ",  "SES",  "SEH",  "SET",  "SEI"
+};
+
+static const char *bclr[] = {
+    "CLC",  "CLZ",  "CLN",  "CLZ",  "CLS",  "CLH",  "CLT",  "CLI"
+};
+
+INSN(ADC,    "r%d, r%d", a->rd, a->rr)
+INSN(ADD,    "r%d, r%d", a->rd, a->rr)
+INSN(ADIW,   "r%d:r%d, %d", a->rd + 1, a->rd, a->imm)
+INSN(AND,    "r%d, r%d", a->rd, a->rr)
+INSN(ANDI,   "r%d, %d", a->rd, a->imm)
+INSN(ASR,    "r%d", a->rd)
+INSN_MNEMONIC(BCLR,  bclr[a->bit], "")
+INSN(BLD,    "r%d, %d", a->rd, a->bit)
+INSN_MNEMONIC(BRBC,  brbc[a->bit], ".%+d", a->imm * 2)
+INSN_MNEMONIC(BRBS,  brbs[a->bit], ".%+d", a->imm * 2)
+INSN(BREAK,  "")
+INSN_MNEMONIC(BSET,  bset[a->bit], "")
+INSN(BST,    "r%d, %d", a->rd, a->bit)
+INSN(CALL,   "0x%x", a->imm * 2)
+INSN(CBI,    "%d, %d", a->reg, a->bit)
+INSN(COM,    "r%d", a->rd)
+INSN(CP,     "r%d, r%d", a->rd, a->rr)
+INSN(CPC,    "r%d, r%d", a->rd, a->rr)
+INSN(CPI,    "r%d, %d", a->rd, a->imm)
+INSN(CPSE,   "r%d, r%d", a->rd, a->rr)
+INSN(DEC,    "r%d", a->rd)
+INSN(DES,    "%d", a->imm)
+INSN(EICALL, "")
+INSN(EIJMP,  "")
+INSN(ELPM1,  "")
+INSN(ELPM2,  "r%d, Z", a->rd)
+INSN(ELPMX,  "r%d, Z+", a->rd)
+INSN(EOR,    "r%d, r%d", a->rd, a->rr)
+INSN(FMUL,   "r%d, r%d", a->rd, a->rr)
+INSN(FMULS,  "r%d, r%d", a->rd, a->rr)
+INSN(FMULSU, "r%d, r%d", a->rd, a->rr)
+INSN(ICALL,  "")
+INSN(IJMP,   "")
+INSN(IN,     "r%d, $%d", a->rd, a->imm)
+INSN(INC,    "r%d", a->rd)
+INSN(JMP,    "0x%x", a->imm * 2)
+INSN(LAC,    "Z, r%d", a->rd)
+INSN(LAS,    "Z, r%d", a->rd)
+INSN(LAT,    "Z, r%d", a->rd)
+INSN(LDDY,   "r%d, Y+%d", a->rd, a->imm)
+INSN(LDDZ,   "r%d, Z+%d", a->rd, a->imm)
+INSN(LDI,    "r%d, %d", a->rd, a->imm)
+INSN(LDS,    "r%d, %d", a->rd, a->imm)
+INSN(LDX1,   "r%d, X", a->rd)
+INSN(LDX2,   "r%d, X+", a->rd)
+INSN(LDX3,   "r%d, -X", a->rd)
+INSN(LDY2,   "r%d, Y+", a->rd)
+INSN(LDY3,   "r%d, -Y", a->rd)
+INSN(LDZ2,   "r%d, Z+", a->rd)
+INSN(LDZ3,   "r%d, -Z", a->rd)
+INSN(LPM1,   "")
+INSN(LPM2,   "r%d, Z", a->rd)
+INSN(LPMX,   "r%d, Z+", a->rd)
+INSN(LSR,    "r%d", a->rd)
+INSN(MOV,    "r%d, r%d", a->rd, a->rr)
+INSN(MOVW,   "r%d:r%d, r%d,r:r%d", a->rd + 1, a->rd, a->rr + 1, a->rr)
+INSN(MUL,    "r%d, r%d", a->rd, a->rr)
+INSN(MULS,   "r%d, r%d", a->rd, a->rr)
+INSN(MULSU,  "r%d, r%d", a->rd, a->rr)
+INSN(NEG,    "r%d", a->rd)
+INSN(NOP,    "")
+INSN(OR,     "r%d, r%d", a->rd, a->rr)
+INSN(ORI,    "r%d, %d", a->rd, a->imm)
+INSN(OUT,    "$%d, r%d", a->imm, a->rd)
+INSN(POP,    "r%d", a->rd)
+INSN(PUSH,   "r%d", a->rd)
+INSN(RCALL,  ".%+d", a->imm * 2)
+INSN(RET,    "")
+INSN(RETI,   "")
+INSN(RJMP,   ".%+d", a->imm * 2)
+INSN(ROR,    "r%d", a->rd)
+INSN(SBC,    "r%d, r%d", a->rd, a->rr)
+INSN(SBCI,   "r%d, %d", a->rd, a->imm)
+INSN(SBI,    "$%d, %d", a->reg, a->bit)
+INSN(SBIC,   "$%d, %d", a->reg, a->bit)
+INSN(SBIS,   "$%d, %d", a->reg, a->bit)
+INSN(SBIW,   "r%d:r%d, %d", a->rd + 1, a->rd, a->imm)
+INSN(SBRC,   "r%d, %d", a->rr, a->bit)
+INSN(SBRS,   "r%d, %d", a->rr, a->bit)
+INSN(SLEEP,  "")
+INSN(SPM,    "")
+INSN(SPMX,   "Z+")
+INSN(STDY,   "r%d, Y+%d", a->rd, a->imm)
+INSN(STDZ,   "r%d, Z+%d", a->rd, a->imm)
+INSN(STS,    "r%d, %d", a->rd, a->imm)
+INSN(STX1,   "r%d, X", a->rr)
+INSN(STX2,   "r%d, X+", a->rr)
+INSN(STX3,   "r%d, -X", a->rr)
+INSN(STY2,   "r%d, Y+", a->rd)
+INSN(STY3,   "r%d, -Y", a->rd)
+INSN(STZ2,   "r%d, Z+", a->rd)
+INSN(STZ3,   "r%d, -Z", a->rd)
+INSN(SUB,    "r%d, r%d", a->rd, a->rr)
+INSN(SUBI,   "r%d, %d", a->rd, a->imm)
+INSN(SWAP,   "r%d", a->rd)
+INSN(WDR,    "")
+INSN(XCH,    "Z, r%d", a->rd)
+
diff --git a/target/avr/translate.c b/target/avr/translate.c
index c8c6f798bf..a621195817 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -2917,6 +2917,17 @@  done_generating:
 
     tb->size = (ctx.npc - pc_start) * 2;
     tb->icount = num_insns;
+
+#ifdef DEBUG_DISAS
+    if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
+        && qemu_log_in_addr_range(tb->pc)) {
+        qemu_log_lock();
+        qemu_log("IN: %s\n", lookup_symbol(tb->pc));
+        log_target_disas(cs, tb->pc, tb->size);
+        qemu_log("\n");
+        qemu_log_unlock();
+    }
+#endif
 }
 
 void restore_state_to_opc(CPUAVRState *env, TranslationBlock *tb,