Hi Aleksandar, On 9/11/18 7:18 AM, Aleksandar Markovic wrote: >> From: Philippe Mathieu-Daudé <philippe.mathieu.da...@gmail.com> on behalf of >> Philippe > Mathieu-Daudé <f4...@amsat.org> >> Sent: Sunday, September 9, 2018 3:34 AM >> To: Fredrik Noring; Aleksandar Markovic >> Cc: Philippe Mathieu-Daudé; qemu-devel@nongnu.org; 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é <f4...@amsat.org> >> --- >> 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.)
OK. I added those definitions to avoid Fredrik's series clashing with my work, but I'll adapt later. > Instead, I would reorganize existing flags (as you, as well, indicated in a > previous message). Good. > 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 > */ Yes, this is cleaner, I like it. Beware we have to add the ULL suffix. > 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.). OK. > 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. OK, thanks for the review! Phil.
