Re: [PATCH v37 10/17] target/avr: Add instruction disassembly function - Philippe Mathieu-Daudé

From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: Michael Rolnik <mrolnik@gmail.com>,
	Aleksandar Markovic <aleksandar.m.mail@gmail.com>,
	"thuth@redhat.com" <thuth@redhat.com>
Cc: "dovgaluk@ispras.ru" <dovgaluk@ispras.ru>,
	"me@xcancerberox.com.ar" <me@xcancerberox.com.ar>,
	"richard.henderson@linaro.org" <richard.henderson@linaro.org>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"imammedo@redhat.com" <imammedo@redhat.com>
Subject: Re: [PATCH v37 10/17] target/avr: Add instruction disassembly function
Date: Tue, 3 Dec 2019 12:18:01 +0100	[thread overview]
Message-ID: <b24d01f6-7112-8074-f0bf-623024ae4176@redhat.com> (raw)
In-Reply-To: <CAK4993g9sAkBSHiHz5RzMCXJdr-vHU=qGaBMq52GEBku3y=oNw@mail.gmail.com>

On 12/2/19 8:04 AM, Michael Rolnik wrote:
> Aleksandar.
> 
> If this code is going to be merge in 2019 I should modify al the 
> copyrights, right. or should I put 2020 in?

Usually the copyright date is when you first contributed your code to 
the world (here, the list). If a patch was on the list in 2018, even if 
you made modifications and repost it, (c) is 2018.

I'm not sure why Richard's (c) appears here, is target/avr/disas.c based 
on target/openrisc/disas.c? Then it looks correct to me, but IANAL.

> On Mon, Dec 2, 2019 at 2:28 AM Aleksandar Markovic 
> <aleksandar.m.mail@gmail.com <mailto:aleksandar.m.mail@gmail.com>> wrote:
> 
> 
> 
>     On Wednesday, November 27, 2019, Michael Rolnik <mrolnik@gmail.com
>     <mailto: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
>         <mailto:mrolnik@gmail.com>>
>         Suggested-by: Richard Henderson <richard.henderson@linaro.org
>         <mailto:richard.henderson@linaro.org>>
>         Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com
>         <mailto:philmd@redhat.com>>
>         Suggested-by: Aleksandar Markovic <aleksandar.m.mail@gmail.com
>         <mailto:aleksandar.m.mail@gmail.com>>
>         Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com
>         <mailto:philmd@redhat.com>>
>         Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com
>         <mailto:philmd@redhat.com>>
>         ---
>           target/avr/cpu.h       |   1 +
>           target/avr/cpu.c       |   2 +-
>           target/avr/disas.c     | 228
>         +++++++++++++++++++++++++++++++++++++++++
>           target/avr/translate.c |  11 ++
>           4 files changed, 241 insertions(+), 1 deletion(-)
>           create mode 100644 target/avr/disas.c
> 
>         diff --git a/target/avr/cpu.h b/target/avr/cpu.h
>         index 9ea5260165..a3e615a1eb 100644
>         --- a/target/avr/cpu.h
>         +++ b/target/avr/cpu.h
>         @@ -157,6 +157,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 dae56d7845..52ec21dd16 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..a51ade7c2a
>         --- /dev/null
>         +++ b/target/avr/disas.c
>         @@ -0,0 +1,228 @@
>         +/*
>         + * AVR disassembler
>         + *
>         + * Copyright (c) 2018 Richard Henderson <rth@twiddle.net
>         <mailto:rth@twiddle.net>>
> 
> 
>     Just a detail: since this file is created in 2019, the copyright
>     year should be 2019 too.
> 
>         + * Copyright (c) 2019 Michael Rolnik <mrolnik@gmail.com
>         <mailto: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/>.
>         + */
[...]