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

[Thomas Huth <thuth@redhat.com>]

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?

Also, unless you've copied the code from another project that uses C++, 
why did you use C++ here? QEMU is C by default, we only allow C++ for 
some files that have been taken from other C++ projects and need to be 
kept in sync from time to time. So if you wrote this code from scratch, 
please use C instead.

  Thanks,
   Thomas


> +extern "C" {
> +#include "qemu/osdep.h"
> +#include "qemu/bitops.h"
> +#include "disas/dis-asm.h"
> +}
> +
> +#include "loongson2f.h"
> +
> +int print_insn_loongson2f(bfd_vma addr, disassemble_info *info)
> +{
> +    bfd_byte buffer[4];
> +    uint32_t insn32;
> +    int status;
> +    Decoder *decoder = new Decoder();
> +
> +    status = info->read_memory_func(addr, buffer, 4, info);
> +    if (status != 0) {
> +        info->memory_error_func(status, addr, info);
> +        return -1;
> +    }
> +    if (info->endian == BFD_ENDIAN_BIG) {
> +        insn32 = bfd_getb32(buffer);
> +    } else {
> +        insn32 = bfd_getl32(buffer);
> +    }
> +
> +    status = decoder->decode32(info, insn32);
> +
> +    delete decoder;
> +
> +    return status == 0 ? -1 : 4;
> +}