Re: [Qemu-devel] [PATCH 2/2] target/mips: Add entries for the Toshiba's R3900 and R5900 cores

[Aleksandar Markovic]

> From: Philippe Mathieu-Daudé <address@hidden> on behalf of Philippe > 
> Mathieu-Daudé <address@hidden>
> Sent: Sunday, September 9, 2018 3:34 AM
> To: Fredrik Noring; Aleksandar Markovic
> Cc: Philippe Mathieu-Daudé; address@hidden; Richard Henderson; Aurelien Jarno
> Subject: [PATCH 2/2] target/mips: Add entries for the Toshiba's R3900 and 
> R5900 cores
> 
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
>  target/mips/mips-defs.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/target/mips/mips-defs.h b/target/mips/mips-defs.h
> index c8e99791ad..9875bdac82 100644
> --- a/target/mips/mips-defs.h
> +++ b/target/mips/mips-defs.h
> @@ -56,6 +56,8 @@
>  #define                INSN_LOONGSON2E  0x20000000
>  #define                INSN_LOONGSON2F  0x40000000
>  #define                INSN_VR54XX     0x80000000
> +#define     INSN_R3900       0x100000000ULL
> +#define     INSN_R5900       0x200000000ULL
> 
>  /* MIPS CPU defines. */
>  #define                CPU_MIPS1       (ISA_MIPS1)

I don't think we should add flags for not yet supported CPUs ot ASEs. 
(Corresponding ELF flags in other headers are OK, since they originate from 
external headers - however, "insn_flags" is a purely internal QEMU construct.)

Instead, I would reorganize existing flags (as you, as well, indicated in a 
previous message).

Let's say, something like this:


/*
 * insn_flags
 */

/*
 * bits 0-31 MIPS base instruction sets
 */ 
#define ISA_MIPS1         0x0000000000000001
#define ISA_MIPS2         0x0000000000000002
#define ISA_MIPS3         0x0000000000000004
#define ISA_MIPS4         0x0000000000000008
#define ISA_MIPS5         0x0000000000000010
#define ISA_MIPS32        0x0000000000000020
#define ISA_MIPS32R2      0x0000000000000040
#define ISA_MIPS64        0x0000000000000080
#define ISA_MIPS64R2      0x0000000000000100
#define ISA_MIPS32R3      0x0000000000000200
#define ISA_MIPS64R3      0x0000000000000400
#define ISA_MIPS32R5      0x0000000000000800
#define ISA_MIPS64R5      0x0000000000001000
#define ISA_MIPS32R6      0x0000000000002000
#define ISA_MIPS64R6      0x0000000000004000
#define ISA_NANOMIPS32    0x0000000000008000

/*
 * bits 32-47 MIPS ASEs
 */ 
#define ASE_MIPS16        0x0000000100000000
#define ASE_MIPS3D        0x0000000200000000
#define ASE_MDMX          0x0000000400000000
#define ASE_DSP           0x0000000800000000
#define ASE_DSPR2         0x0000001000000000
#define ASE_DSPR3         0x0000002000000000
#define ASE_MT            0x0000004000000000
#define ASE_SMARTMIPS     0x0000008000000000
#define ASE_MICROMIPS     0x0000010000000000
#define ASE_MSA           0x0000020000000000

/*
 * bits 48-55 vendor-specific base instruction sets
 */ 
#define INSN_LOONGSON2E   0x0001000000000000
#define INSN_LOONGSON2F   0x0002000000000000
#define INSN_VR54XX       0x0004000000000000

/*
 * bits 56-63 vendor-specific ASEs
 */ 

Notice that I inserted ASE_DSPR3, that was missing (there is an instruction 
from DSP R3 already implemented, and needing this flag for correct availability 
control.).

INSN_R5900 would belong to the third category. ASE_MXU and ASE MXU2 (not yet 
implemented in QEMU) from Igenic would be examples for the fourth category. The 
fourth category would be empty for now.

Thanks,
Aleksandar