Re: [PATCH 1/1] disas: mips: Add Loongson 2F disassembler

[Thomas Huth <thuth@redhat.com>]

On 03/07/2020 11.49, Stefan Brankovic wrote:
> 
> On 3.7.20. 09:59, Thomas Huth wrote:
>> On 02/07/2020 21.42, Stefan Brankovic wrote:
>>> Add disassembler for Loongson 2F instruction set.
>>>
>>> Testing is done by comparing qemu disassembly output, obtained by
>>> using -d in_asm command line option, with appropriate objdump output.
>>>
>>> Signed-off-by: Stefan Brankovic <stefan.brankovic@syrmia.com>
>>> ---
>>>   MAINTAINERS             |    1 +
>>>   configure               |    1 +
>>>   disas/Makefile.objs     |    1 +
>>>   disas/loongson2f.cpp    | 8134 +++++++++++++++++++++++++++++++++++++++
>>>   disas/loongson2f.h      | 2542 ++++++++++++
>>>   include/disas/dis-asm.h |    1 +
>>>   include/exec/poison.h   |    1 +
>>>   target/mips/cpu.c       |    4 +
>>>   8 files changed, 10685 insertions(+)
>>>   create mode 100644 disas/loongson2f.cpp
>>>   create mode 100644 disas/loongson2f.h
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 3abe3faa4e..913ed2a6d3 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -219,6 +219,7 @@ S: Maintained
>>>   F: target/mips/
>>>   F: default-configs/*mips*
>>>   F: disas/*mips*
>>> +F: disas/loongson*
>>>   F: docs/system/cpu-models-mips.rst.inc
>>>   F: hw/intc/mips_gic.c
>>>   F: hw/mips/
>>> diff --git a/configure b/configure
>>> index 597e909b53..e163dac53e 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -8102,6 +8102,7 @@ for i in $ARCH $TARGET_BASE_ARCH ; do
>>>       disas_config "MIPS"
>>>       if test -n "${cxx}"; then
>>>         disas_config "NANOMIPS"
>>> +      disas_config "LOONGSON2F"
>>>       fi
>>>     ;;
>>>     moxie*)
>>> diff --git a/disas/Makefile.objs b/disas/Makefile.objs
>>> index 3c1cdce026..0d5ee1e038 100644
>>> --- a/disas/Makefile.objs
>>> +++ b/disas/Makefile.objs
>>> @@ -14,6 +14,7 @@ common-obj-$(CONFIG_I386_DIS) += i386.o
>>>   common-obj-$(CONFIG_M68K_DIS) += m68k.o
>>>   common-obj-$(CONFIG_MICROBLAZE_DIS) += microblaze.o
>>>   common-obj-$(CONFIG_MIPS_DIS) += mips.o
>>> +common-obj-$(CONFIG_LOONGSON2F_DIS) += loongson2f.o
>>>   common-obj-$(CONFIG_NANOMIPS_DIS) += nanomips.o
>>>   common-obj-$(CONFIG_NIOS2_DIS) += nios2.o
>>>   common-obj-$(CONFIG_MOXIE_DIS) += moxie.o
>>> diff --git a/disas/loongson2f.cpp b/disas/loongson2f.cpp
>>> new file mode 100644
>>> index 0000000000..a2f32dcf93
>>> --- /dev/null
>>> +++ b/disas/loongson2f.cpp
>>> @@ -0,0 +1,8134 @@
>>
>> This file (and the header) lack a proper header comment. Which license
>> do you want to use for this code? Who wrote the initial implementation?
> I will add proper license comments in v2. I will use GPL2+ license.
> Thanks for reminding. This is the initial implementation, and I am the
> author.
>>
>> Also, unless you've copied the code from another project that uses
>> C++, why did you use C++ here?
> This is disassembler is written as a generic disassembler that can be
> integrated into other projects. In this case, it is integrated into
> QEMU. One of initial key requirements was that it uses C++ as its
> language.

Ok, fair, but please mention that rationale in the commit description
when you send v2.

 Thanks,
  Thomas