[Qemu-devel] [RFC] target/mips: Initial support for MIPS R5900

[Qemu-devel] [RFC] target/mips: Initial support for MIPS R5900

[Fredrik Noring]

Hi,

This patch implements initial QEMU support for the MIPS R5900. Primarily
code generated by GCC. The only special instruction needed for this, as far
as I can tell, is the three-operand multiply.

The MIPS R5900 is normally taken to be MIPS3, but it has MOVN, MOVZ and PREF
defined in MIPS4 which is why ISA_MIPS4 is chosen for this patch.

I have tested the patch with Linux user mode emulation on a root filesystem
compiled for R5900 and it worked without apparent issues, apart from:

	qemu: Unsupported syscall: 4352 (seccomp)

Some flags in the mips_defs array are marked FIXME as I don't know the
proper values.

Would this patch be an acceptable initial submission for the MIPS R5900?

Fredrik

 ---
 include/elf.h                    |  3 +++
 linux-user/mips/target_elf.h     |  3 +++
 target/mips/mips-defs.h          |  2 ++
 target/mips/translate.c          | 31 ++++++++++++++++++++++++++++++-
 target/mips/translate_init.inc.c | 20 ++++++++++++++++++++
 5 files changed, 58 insertions(+), 1 deletion(-)

--- a/include/elf.h
+++ b/include/elf.h
@@ -48,6 +48,8 @@ typedef int64_t  Elf64_Sxword;
 #define EF_MIPS_ARCH_32R6       0x90000000      /* MIPS32r6 code.  */
 #define EF_MIPS_ARCH_64R6       0xa0000000      /* MIPS64r6 code.  */
 
+#define EF_MIPS_MACH_5900       0x00920000
+
 /* The ABI of a file. */
 #define EF_MIPS_ABI_O32		0x00001000	/* O32 ABI.  */
 #define EF_MIPS_ABI_O64		0x00002000	/* O32 extended for 64 bit.  */
@@ -62,6 +64,7 @@ typedef int64_t  Elf64_Sxword;
 #define EF_MIPS_FP64      0x00000200
 #define EF_MIPS_NAN2008   0x00000400
 #define EF_MIPS_ARCH      0xf0000000
+#define EF_MIPS_MACH      0x00ff0000
 
 /* These constants define the different elf file types */
 #define ET_NONE   0
--- a/linux-user/mips/target_elf.h
+++ b/linux-user/mips/target_elf.h
@@ -12,6 +12,9 @@ static inline const char *cpu_get_model(uint32_t eflags)
     if ((eflags & EF_MIPS_ARCH) == EF_MIPS_ARCH_32R6) {
         return "mips32r6-generic";
     }
+    if ((eflags & EF_MIPS_MACH) == EF_MIPS_MACH_5900) {
+        return "R5900";
+    }
     return "24Kf";
 }
 #endif
--- a/target/mips/mips-defs.h
+++ b/target/mips/mips-defs.h
@@ -52,6 +52,7 @@
 #define   ASE_MSA       0x01000000
 
 /* Chip specific instructions. */
+#define		INSN_R5900	0x10000000
 #define		INSN_LOONGSON2E  0x20000000
 #define		INSN_LOONGSON2F  0x40000000
 #define		INSN_VR54XX	0x80000000
@@ -62,6 +63,7 @@
 #define		CPU_MIPS3	(CPU_MIPS2 | ISA_MIPS3)
 #define		CPU_MIPS4	(CPU_MIPS3 | ISA_MIPS4)
 #define		CPU_VR54XX	(CPU_MIPS4 | INSN_VR54XX)
+#define		CPU_R5900	(CPU_MIPS4 | INSN_R5900)
 #define		CPU_LOONGSON2E  (CPU_MIPS3 | INSN_LOONGSON2E)
 #define		CPU_LOONGSON2F  (CPU_MIPS3 | INSN_LOONGSON2F)
 
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -3618,6 +3618,31 @@ static void gen_muldiv(DisasContext *ctx, uint32_t opc,
     tcg_temp_free(t1);
 }
 
+static void gen_mul_r5900 (DisasContext *ctx, uint32_t opc,
+                            int rd, int rs, int rt)
+{
+    TCGv t0 = tcg_temp_new();
+    TCGv t1 = tcg_temp_new();
+
+    gen_load_gpr(t0, rs);
+    gen_load_gpr(t1, rt);
+
+    switch (opc) {
+    case OPC_MULT:
+    case OPC_MULTU:
+        tcg_gen_mul_tl(cpu_gpr[rd], t0, t1);
+        break;
+    default:
+        MIPS_INVAL("mul R5900");
+        generate_exception_end(ctx, EXCP_RI);
+        goto out;
+    }
+
+ out:
+    tcg_temp_free(t0);
+    tcg_temp_free(t1);
+}
+
 static void gen_mul_vr54xx (DisasContext *ctx, uint32_t opc,
                             int rd, int rs, int rt)
 {
@@ -17357,7 +17382,11 @@ static void decode_opc_special_legacy(CPUMIPSState *env, DisasContext *ctx)
         break;
     case OPC_MULT:
     case OPC_MULTU:
-        if (sa) {
+        if (ctx->insn_flags & INSN_R5900) {
+            gen_muldiv(ctx, op1, 0, rs, rt);
+            if (rd != 0)
+                gen_mul_r5900(ctx, op1, rd, rs, rt);
+        } else if (sa) {
             check_insn(ctx, INSN_VR54XX);
             op1 = MASK_MUL_VR54XX(ctx->opcode);
             gen_mul_vr54xx(ctx, op1, rd, rs, rt);
--- a/target/mips/translate_init.inc.c
+++ b/target/mips/translate_init.inc.c
@@ -411,6 +411,26 @@ const mips_def_t mips_defs[] =
         .mmu_type = MMU_TYPE_R4000,
     },
     {
+        .name = "R5900",
+        .CP0_PRid = 0x00003800,
+        /* No L2 cache, icache size 32k, dcache size 32k, uncached coherency. */
+        .CP0_Config0 = (1 << 17) | (0x3 << 9) | (0x3 << 6) | (0x2 << CP0C0_K0),
+        /* Note: Config1 is only used internally, the R5900 has only Config0. */
+        .CP0_Config1 = (1 << CP0C1_FP) | (47 << CP0C1_MMU),
+        .CP0_LLAddr_rw_bitmask = 0xFFFFFFFF,	/* FIXME */
+        .CP0_LLAddr_shift = 4,			/* FIXME */
+        .SYNCI_Step = 16,			/* FIXME */
+        .CCRes = 2,				/* FIXME */
+        .CP0_Status_rw_bitmask = 0x3678FFFF,	/* FIXME */
+        .CP1_fcr0 = (0x38 << FCR0_PRID) | (0x0 << FCR0_REV),
+        .CP1_fcr31 = 0,
+        .CP1_fcr31_rw_bitmask = 0x0183FFFF,	/* FIXME */
+        .SEGBITS = 40,				/* FIXME */
+        .PABITS = 36,				/* FIXME */
+        .insn_flags = CPU_R5900,
+        .mmu_type = MMU_TYPE_R4000,		/* FIXME */
+    },
+    {
         /* A generic CPU supporting MIPS32 Release 6 ISA.
            FIXME: Support IEEE 754-2008 FP.
                   Eventually this should be replaced by a real CPU model. */

Re: [Qemu-devel] [RFC] target/mips: Initial support for MIPS R5900

[Fredrik Noring]

> I have tested the patch with Linux user mode emulation on a root filesystem
> compiled for R5900 and it worked without apparent issues, apart from:
> 
> 	qemu: Unsupported syscall: 4352 (seccomp)

I have also observed the following QEMU crash:

qemu-mipsel: qemu/accel/tcg/cpu-exec.c:634: cpu_loop_exec_tb: Assertion `use_icount' failed.
qemu: uncaught target signal 11 (Segmentation fault) - core dumped

It occurred when compiling Perl, and it seems to be identical to this bug
reported by John Paul Adrian Glaubitz for SH4:

https://bugs.launchpad.net/qemu/+bug/1768246

Fredrik

Re: [Qemu-devel] [RFC] target/mips: Initial support for MIPS R5900

["Jürgen Urban"]

Hello Fredrik,

> Gesendet: Samstag, 07. Juli 2018 um 21:41 Uhr
> Von: "Fredrik Noring" <noring@nocrew.org>
> An: qemu-devel@nongnu.org
> Cc: "Maciej W. Rozycki" <macro@linux-mips.org>, "Jürgen Urban" <JuergenUrban@gmx.de>
> Betreff: [RFC] target/mips: Initial support for MIPS R5900
>
> Hi,
> 
> This patch implements initial QEMU support for the MIPS R5900. Primarily
> code generated by GCC. The only special instruction needed for this, as far
> as I can tell, is the three-operand multiply.
> 
> The MIPS R5900 is normally taken to be MIPS3, but it has MOVN, MOVZ and PREF
> defined in MIPS4 which is why ISA_MIPS4 is chosen for this patch.
> 
> I have tested the patch with Linux user mode emulation on a root filesystem
> compiled for R5900 and it worked without apparent issues, apart from:
> 
> 	qemu: Unsupported syscall: 4352 (seccomp)
> 
> Some flags in the mips_defs array are marked FIXME as I don't know the
> proper values.
> 
> Would this patch be an acceptable initial submission for the MIPS R5900?

I think it is a good first step.
As the GCC doesn't generate VU instructions and there are several optimisations missing in the GCC, I assume it will not be able to handle a larger test like:
https://sourceforge.net/p/kernelloader/ps2linux-toolchain/ci/master/tree/minimalistic

As it will later access /dev/ps2gs, you will need to run the qemu on the PS2 to be able to run the test completely.
Note: As the official Linux kernel has several bugs and the test tries to exploit them, it may not be possible to execute the test without my PS2 Linux patches.

Best regards
Jürgen Urban

Re: [Qemu-devel] [RFC] target/mips: Initial support for MIPS R5900

[Maciej W. Rozycki]

On Sat, 7 Jul 2018, Fredrik Noring wrote:

> The MIPS R5900 is normally taken to be MIPS3, but it has MOVN, MOVZ and PREF
> defined in MIPS4 which is why ISA_MIPS4 is chosen for this patch.

 It also has several instructions removed, so I don't think you can really 
just mark it MIPS IV without special-casing those instructions, or the 
emulation won't be accurate (and consequently programs that use them won't 
trigger exceptions that they are supposed to).

> Some flags in the mips_defs array are marked FIXME as I don't know the
> proper values.

 Well, the FPU is non-standard so until you implement it I'd rather kept 
it disabled and then all the FPU-related settings can go for now.  For the 
rest see below.

> --- a/target/mips/translate_init.inc.c
> +++ b/target/mips/translate_init.inc.c
> @@ -411,6 +411,26 @@ const mips_def_t mips_defs[] =
>          .mmu_type = MMU_TYPE_R4000,
>      },
>      {
> +        .name = "R5900",
> +        .CP0_PRid = 0x00003800,
> +        /* No L2 cache, icache size 32k, dcache size 32k, uncached coherency. */
> +        .CP0_Config0 = (1 << 17) | (0x3 << 9) | (0x3 << 6) | (0x2 << CP0C0_K0),
> +        /* Note: Config1 is only used internally, the R5900 has only Config0. */
> +        .CP0_Config1 = (1 << CP0C1_FP) | (47 << CP0C1_MMU),

 So I'd clear CP0C1_FP then; also make sure accessing CP0.Config1 from 
emulated code does what it does on actual hardware.

> +        .CP0_LLAddr_rw_bitmask = 0xFFFFFFFF,	/* FIXME */
> +        .CP0_LLAddr_shift = 4,			/* FIXME */

 No LL/SC in the R5900, so the LLAddr settings can go.

> +        .SYNCI_Step = 16,			/* FIXME */

 SYNCI is MIPS32r2+, so this can go.

> +        .CCRes = 2,				/* FIXME */

 Likewise, CCRes is MIPS32r2+, so this can go.

> +        .CP0_Status_rw_bitmask = 0x3678FFFF,	/* FIXME */

 This has to indicate which bits in CP0.Status are writable.  Check with 
the manual and/or actual hardware.

> +        .CP1_fcr0 = (0x38 << FCR0_PRID) | (0x0 << FCR0_REV),
> +        .CP1_fcr31 = 0,
> +        .CP1_fcr31_rw_bitmask = 0x0183FFFF,	/* FIXME */

 This is all FPU stuff and it can go.

> +        .SEGBITS = 40,				/* FIXME */

 This is the number of virtual address bits.  Determined by the highest 
writable CP0.EntryHi.VPN2 bit.

> +        .PABITS = 36,				/* FIXME */

 Likewise physical address bits.  Determined by the highest writable 
CP0.EntryLo0.PFN and CP0.EntryLo1.PFN bit.

> +        .insn_flags = CPU_R5900,
> +        .mmu_type = MMU_TYPE_R4000,		/* FIXME */

 This looks right to me.

 FWIW; I don't have any authority for QEMU maintenance.

  Maciej

Re: [Qemu-devel] [RFC] target/mips: Initial support for MIPS R5900

[Fredrik Noring]

Thank you for your review, Maciej,

> > The MIPS R5900 is normally taken to be MIPS3, but it has MOVN, MOVZ and PREF
> > defined in MIPS4 which is why ISA_MIPS4 is chosen for this patch.
> 
>  It also has several instructions removed, so I don't think you can really 
> just mark it MIPS IV without special-casing those instructions, or the 
> emulation won't be accurate (and consequently programs that use them won't 
> trigger exceptions that they are supposed to).

Agreed. However, complete and perfect emulation of the R5900 will require a
substantial amount of work. What level is suitable for an initial patch?

> > Some flags in the mips_defs array are marked FIXME as I don't know the
> > proper values.
> 
>  Well, the FPU is non-standard so until you implement it I'd rather kept 
> it disabled and then all the FPU-related settings can go for now.  For the 
> rest see below.

The kernel traps FPU instructions to emulate them accurately, and unless the
FPU is enabled here the QEMU Linux user space emulator crashes with "illegal
hardware instruction" on valid programs. Can QEMU be instructed to emulate
the FPU only for Linux user space programs as opposed to hardware emulation?

> > --- a/target/mips/translate_init.inc.c
> > +++ b/target/mips/translate_init.inc.c
> > @@ -411,6 +411,26 @@ const mips_def_t mips_defs[] =
> >          .mmu_type = MMU_TYPE_R4000,
> >      },
> >      {
> > +        .name = "R5900",
> > +        .CP0_PRid = 0x00003800,
> > +        /* No L2 cache, icache size 32k, dcache size 32k, uncached coherency. */
> > +        .CP0_Config0 = (1 << 17) | (0x3 << 9) | (0x3 << 6) | (0x2 << CP0C0_K0),
> > +        /* Note: Config1 is only used internally, the R5900 has only Config0. */
> > +        .CP0_Config1 = (1 << CP0C1_FP) | (47 << CP0C1_MMU),
> 
>  So I'd clear CP0C1_FP then; also make sure accessing CP0.Config1 from 
> emulated code does what it does on actual hardware.
> 
> > +        .CP0_LLAddr_rw_bitmask = 0xFFFFFFFF,	/* FIXME */
> > +        .CP0_LLAddr_shift = 4,			/* FIXME */
> 
>  No LL/SC in the R5900, so the LLAddr settings can go.

Again, the kernel emulates LL/SC so I suppose some kind indication is needed
for that in the QEMU Linux user space emulator?

> > +        .SYNCI_Step = 16,			/* FIXME */
> 
>  SYNCI is MIPS32r2+, so this can go.
> 
> > +        .CCRes = 2,				/* FIXME */
> 
>  Likewise, CCRes is MIPS32r2+, so this can go.

Sure!

> > +        .CP0_Status_rw_bitmask = 0x3678FFFF,	/* FIXME */
> 
>  This has to indicate which bits in CP0.Status are writable.  Check with 
> the manual and/or actual hardware.

The TX79 manual describes the writable bits on pages 4.16-4.18, and it seems
0xF4C79C1F would be those.

> > +        .CP1_fcr0 = (0x38 << FCR0_PRID) | (0x0 << FCR0_REV),
> > +        .CP1_fcr31 = 0,
> > +        .CP1_fcr31_rw_bitmask = 0x0183FFFF,	/* FIXME */
> 
>  This is all FPU stuff and it can go.
> 
> > +        .SEGBITS = 40,				/* FIXME */
> 
>  This is the number of virtual address bits.  Determined by the highest 
> writable CP0.EntryHi.VPN2 bit.

VPN2 is bits 31:13 and so it is 19 bits wide, according to page 4.14 of the
TX79 manual.

> > +        .PABITS = 36,				/* FIXME */
> 
>  Likewise physical address bits.  Determined by the highest writable 
> CP0.EntryLo0.PFN and CP0.EntryLo1.PFN bit.

PFN is bits 25:6 and so it is 20 bits wide, according to page 4.8 of the
TX79 manual.

> > +        .insn_flags = CPU_R5900,
> > +        .mmu_type = MMU_TYPE_R4000,		/* FIXME */
> 
>  This looks right to me.

Good. :)

>  FWIW; I don't have any authority for QEMU maintenance.

The MAINTAINERS file indicates that Aurelien Jarno and Aleksandar Markovic
have authority over the MIPS emulation (CCed to this post).

Fredrik

Re: [Qemu-devel] [RFC] target/mips: Initial support for MIPS R5900

[Richard Henderson]

On 08/01/2018 09:39 AM, Fredrik Noring wrote:
>  Can QEMU be instructed to emulate
> the FPU only for Linux user space programs as opposed to hardware emulation?

Yes, that can be done.  I would suggest something like

/*
 * Hardware traps to the operating system for emulation.
 * For user-only, qemu is the operating system, so we
 * emulate the trap and emulate by simply emulating the
 * instruction directly.
 */
#ifdef CONFIG_USER_ONLY
# define CP0C1_FP_USER_ONLY  (1 << CP0C1_FP)
#else
# define CP0C1_FP_USER_ONLY  0
#endif

and include that in your initialization of CP0_Config1.


r~

Re: [Qemu-devel] [PATCH v2] target/mips: Initial support for MIPS R5900

[Fredrik Noring]

Thank you, Richard,

> >  Can QEMU be instructed to emulate
> > the FPU only for Linux user space programs as opposed to hardware emulation?
> 
> Yes, that can be done.  I would suggest something like
> 
> /*
>  * Hardware traps to the operating system for emulation.
>  * For user-only, qemu is the operating system, so we
>  * emulate the trap and emulate by simply emulating the
>  * instruction directly.
>  */
> #ifdef CONFIG_USER_ONLY
> # define CP0C1_FP_USER_ONLY  (1 << CP0C1_FP)
> #else
> # define CP0C1_FP_USER_ONLY  0
> #endif
> 
> and include that in your initialization of CP0_Config1.

I've made both CP0C1_FP and the LL/SC emulation conditional on user-only,
and adjusted the patch after the review by Maciej, as shown below.

Aleksandar, Aurelien, Maciej -- are you happy with this initial v2 patch?

Fredrik

Signed-off-by: Fredrik Noring <noring@nocrew.org>

 ---
 include/elf.h                    |    3 ++
 linux-user/mips/target_elf.h     |    3 ++
 target/mips/mips-defs.h          |    2 +
 target/mips/translate.c          |   31 ++++++++++++++++++++++++++-
 target/mips/translate_init.inc.c |   44 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 82 insertions(+), 1 deletion(-)

--- a/include/elf.h
+++ b/include/elf.h
@@ -48,6 +48,8 @@ typedef int64_t  Elf64_Sxword;
 #define EF_MIPS_ARCH_32R6       0x90000000      /* MIPS32r6 code.  */
 #define EF_MIPS_ARCH_64R6       0xa0000000      /* MIPS64r6 code.  */
 
+#define EF_MIPS_MACH_5900       0x00920000
+
 /* The ABI of a file. */
 #define EF_MIPS_ABI_O32		0x00001000	/* O32 ABI.  */
 #define EF_MIPS_ABI_O64		0x00002000	/* O32 extended for 64 bit.  */
@@ -62,6 +64,7 @@ typedef int64_t  Elf64_Sxword;
 #define EF_MIPS_FP64      0x00000200
 #define EF_MIPS_NAN2008   0x00000400
 #define EF_MIPS_ARCH      0xf0000000
+#define EF_MIPS_MACH      0x00ff0000
 
 /* These constants define the different elf file types */
 #define ET_NONE   0
--- a/linux-user/mips/target_elf.h
+++ b/linux-user/mips/target_elf.h
@@ -12,6 +12,9 @@ static inline const char *cpu_get_model(uint32_t eflags)
     if ((eflags & EF_MIPS_ARCH) == EF_MIPS_ARCH_32R6) {
         return "mips32r6-generic";
     }
+    if ((eflags & EF_MIPS_MACH) == EF_MIPS_MACH_5900) {
+        return "R5900";
+    }
     return "24Kf";
 }
 #endif
--- a/target/mips/mips-defs.h
+++ b/target/mips/mips-defs.h
@@ -52,6 +52,7 @@
 #define   ASE_MSA       0x01000000
 
 /* Chip specific instructions. */
+#define		INSN_R5900	0x10000000
 #define		INSN_LOONGSON2E  0x20000000
 #define		INSN_LOONGSON2F  0x40000000
 #define		INSN_VR54XX	0x80000000
@@ -62,6 +63,7 @@
 #define		CPU_MIPS3	(CPU_MIPS2 | ISA_MIPS3)
 #define		CPU_MIPS4	(CPU_MIPS3 | ISA_MIPS4)
 #define		CPU_VR54XX	(CPU_MIPS4 | INSN_VR54XX)
+#define		CPU_R5900	(CPU_MIPS4 | INSN_R5900)
 #define		CPU_LOONGSON2E  (CPU_MIPS3 | INSN_LOONGSON2E)
 #define		CPU_LOONGSON2F  (CPU_MIPS3 | INSN_LOONGSON2F)
 
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -3618,6 +3618,31 @@ static void gen_muldiv(DisasContext *ctx, uint32_t opc,
     tcg_temp_free(t1);
 }
 
+static void gen_mul_r5900 (DisasContext *ctx, uint32_t opc,
+                            int rd, int rs, int rt)
+{
+    TCGv t0 = tcg_temp_new();
+    TCGv t1 = tcg_temp_new();
+
+    gen_load_gpr(t0, rs);
+    gen_load_gpr(t1, rt);
+
+    switch (opc) {
+    case OPC_MULT:
+    case OPC_MULTU:
+        tcg_gen_mul_tl(cpu_gpr[rd], t0, t1);
+        break;
+    default:
+        MIPS_INVAL("mul R5900");
+        generate_exception_end(ctx, EXCP_RI);
+        goto out;
+    }
+
+ out:
+    tcg_temp_free(t0);
+    tcg_temp_free(t1);
+}
+
 static void gen_mul_vr54xx (DisasContext *ctx, uint32_t opc,
                             int rd, int rs, int rt)
 {
@@ -17357,7 +17382,11 @@ static void decode_opc_special_legacy(CPUMIPSState *env, DisasContext *ctx)
         break;
     case OPC_MULT:
     case OPC_MULTU:
-        if (sa) {
+        if (ctx->insn_flags & INSN_R5900) {
+            gen_muldiv(ctx, op1, 0, rs, rt);
+            if (rd != 0)
+                gen_mul_r5900(ctx, op1, rd, rs, rt);
+        } else if (sa) {
             check_insn(ctx, INSN_VR54XX);
             op1 = MASK_MUL_VR54XX(ctx->opcode);
             gen_mul_vr54xx(ctx, op1, rd, rs, rt);
--- a/target/mips/translate_init.inc.c
+++ b/target/mips/translate_init.inc.c
@@ -410,6 +410,50 @@ const mips_def_t mips_defs[] =
         .insn_flags = CPU_MIPS32R5 | ASE_MSA,
         .mmu_type = MMU_TYPE_R4000,
     },
+    {
+        .name = "R5900",
+        .CP0_PRid = 0x00003800,
+        /* No L2 cache, icache size 32k, dcache size 32k, uncached coherency. */
+        .CP0_Config0 = (1 << 17) | (0x3 << 9) | (0x3 << 6) | (0x2 << CP0C0_K0),
+        /* Note: Config1 is only used internally, the R5900 has only Config0. */
+        .CP0_Status_rw_bitmask = 0xF4C79C1F,
+#ifdef CONFIG_USER_ONLY
+	/*
+	 * R5900 hardware traps to the Linux kernel for IEEE 754-1985 and LL/SC
+	 * emulation. For user-only, qemu is the kernel, so we emulate the traps
+	 * by simply emulating the instructions directly.
+	 */
+        .CP0_Config1 = (1 << CP0C1_FP) | (47 << CP0C1_MMU),
+        .CP0_LLAddr_rw_bitmask = 0xFFFFFFFF,
+        .CP0_LLAddr_shift = 4,
+        .CP1_fcr0 = (0x38 << FCR0_PRID) | (0x0 << FCR0_REV),
+        .CP1_fcr31 = 0,
+        .CP1_fcr31_rw_bitmask = 0x0183FFFF,
+#else
+	/*
+	 * The R5900 COP1 FPU implements single-precision floating-point
+	 * operations but is not entirely IEEE 754-1985 compatible. In
+	 * particular,
+	 *
+	 * - NaN (not a number) and plus/minus infinities are not supported;
+	 * - exception mechanisms are not fully supported;
+	 * - denormalized numbers are not supported;
+	 * - rounding towards nearest and plus/minus infinities are not supported;
+	 * - computed results usually differs in the least significant bit;
+	 * - saturating instructions can differ more than the least significant bit.
+	 *
+	 * Since only rounding towards zero is supported, the two least
+	 * significant bits of FCR31 are hardwired to 01.
+	 *
+	 * FPU emulation is disabled here until it is implemented.
+	 */
+        .CP0_Config1 = (47 << CP0C1_MMU),
+#endif /* CONFIG_USER_ONLY */
+        .SEGBITS = 19,
+        .PABITS = 20,
+        .insn_flags = CPU_R5900,
+        .mmu_type = MMU_TYPE_R4000,
+    },
     {
         /* A generic CPU supporting MIPS32 Release 6 ISA.
            FIXME: Support IEEE 754-2008 FP.

Re: [Qemu-devel] [RFC] target/mips: Initial support for MIPS R5900

[Philippe Mathieu-Daudé]

[-- Attachment #1: Type: text/plain, Size: 969 bytes --]

Hi,

On 7/7/18 4:41 PM, Fredrik Noring wrote:
[...]
> --- a/target/mips/mips-defs.h
> +++ b/target/mips/mips-defs.h
> @@ -52,6 +52,7 @@
>  #define   ASE_MSA       0x01000000
>  
>  /* Chip specific instructions. */
> +#define		INSN_R5900	0x10000000

We have 4 bits to store the chip-specific instructions and this is the
last bit available.

Any objection to upraise CPUMIPSState.insn_flags to a uint64_t?
I'd then shift the chip-specific bits to the 32 upper bits, to keep this
flag clean.

>  #define		INSN_LOONGSON2E  0x20000000
>  #define		INSN_LOONGSON2F  0x40000000
>  #define		INSN_VR54XX	0x80000000
> @@ -62,6 +63,7 @@
>  #define		CPU_MIPS3	(CPU_MIPS2 | ISA_MIPS3)
>  #define		CPU_MIPS4	(CPU_MIPS3 | ISA_MIPS4)
>  #define		CPU_VR54XX	(CPU_MIPS4 | INSN_VR54XX)
> +#define		CPU_R5900	(CPU_MIPS4 | INSN_R5900)
>  #define		CPU_LOONGSON2E  (CPU_MIPS3 | INSN_LOONGSON2E)
>  #define		CPU_LOONGSON2F  (CPU_MIPS3 | INSN_LOONGSON2F)
[...]


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH v2] target/mips: Initial support for MIPS R5900

[Aleksandar Markovic]

》From: Fredrik Noring <noring@nocrew.org>
》Sent: Friday, September 7, 2018 9:16 PM
》To: Richard Henderson; Aurelien Jarno; Aleksandar Markovic; Maciej W. Rozycki
》Cc: Jürgen Urban; qemu-devel@nongnu.org
》Subject: Re: [PATCH v2] target/mips: Initial support for MIPS R5900

Please:

- rebase your changes to the latest QEMU code
- organize the changes in the form of patch series
- provide links to or attach relevant documentation
- in cover letter, outline what is needed for full QEMU support of the cpu in question
- describe testing and verification
- remove the unclear word 'initial' from the title
- outline your plan for providing full support - can you commit enough resources to do the job in a reasonable timeframe?

Otherwise, I am generally happy with your patch.

Thanks,
Aleksandar

Re: [Qemu-devel] [PATCH v2] target/mips: Initial support for MIPS R5900

[Maciej W. Rozycki]

Hi Fredrik,

> Aleksandar, Aurelien, Maciej -- are you happy with this initial v2 patch?

 I have skimmed over and I have a couple of comments.  I'll try to 
finalise them ASAP, however I'm currently at the GNU Tools Cauldron and 
much of my time is taken by the event.

  Maciej

Re: [Qemu-devel] [PATCH v3] target/mips: Support R5900 GCC programs in user mode

[Fredrik Noring]

Hi Aleksandar,

> Please:
> 
> - rebase your changes to the latest QEMU code

Sure. V2 applied to 3.0.0 and this v3 applies to HEAD (commit 19b599f7664b).

> - organize the changes in the form of patch series

What kind of granularity do you have in mind? The patch is quite small with
79 insertions and 1 deletion in total.

> - provide links to or attach relevant documentation

The most relevant manual is probably Toshiba TX System RISC TX79 Core
Architecture:

http://www.lukasz.dk/files/tx79architecture.pdf

> - in cover letter, outline what is needed for full QEMU support of the cpu
>   in question

The primary purpose of this patch is to support programs compiled by GCC for
the R5900 target. This enables QEMU to run R5900 Linux distributions, for
example Gentoo. In particular, this avoids issues with cross compilation.
R5900 hardware is typically limited to 32 MiB of RAM, which is insufficient 
for running GCC in many cases.

> - describe testing and verification

This patch has been tested with Gentoo compiled for R5900, including native
compilation of several packages under QEMU. During testing of 2.12.50 I
discovered two problems which I reported and I believe are unrelated to the
patch itself:

The error

	qemu: Unsupported syscall: 4352 (seccomp)

was reported during Gentoo package installations, and QEMU crashed with

	qemu-mipsel: qemu/accel/tcg/cpu-exec.c:634: cpu_loop_exec_tb: Assertion `use_icount' failed.
	qemu: uncaught target signal 11 (Segmentation fault) - core dumped

when compiling Perl under Gentoo. That crash seems to be related to the bug

https://bugs.launchpad.net/qemu/+bug/1768246

for SH4, which appears to have a fix for SH4 in

commit 5b38d0264064055255db991e29d938491f9e8a32
Author: Laurent Vivier <laurent@vivier.eu>
Date:   Sat Aug 11 10:23:28 2018 +0200

    sh4: fix use_icount with linux-user
    
    This fixes java in a linux-user chroot:
      $ java --version
      qemu-sh4: .../accel/tcg/cpu-exec.c:634: cpu_loop_exec_tb: Assertion `use_icount' failed.
      qemu: uncaught target signal 6 (Aborted) - core dumped
      Aborted (core dumped)
    
    In gen_conditional_jump() in the GUSA_EXCLUSIVE part, we must reset
    base.is_jmp to DISAS_NEXT after the gen_goto_tb() as it is done in
    gen_delayed_conditional_jump() after the gen_jump().
    
    Bug: https://bugs.launchpad.net/qemu/+bug/1768246
    Fixes: 4834871bc95b67343248100e2a75ae0d287bc08b
           ("target/sh4: Convert to DisasJumpType")
    Reported-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
    Signed-off-by: Laurent Vivier <laurent@vivier.eu>
    Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
    Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
    Message-Id: <20180811082328.11268-1-laurent@vivier.eu>

> - remove the unclear word 'initial' from the title

Sure.

> - outline your plan for providing full support - can you commit enough
>   resources to do the job in a reasonable timeframe?

For its intended purpose, to support R5900 GCC programs in user mode, I
believe this patch is sufficiently complete as it stands.
 
> Otherwise, I am generally happy with your patch.

Good!

Fredrik

Signed-off-by: Fredrik Noring <noring@nocrew.org>

 ---
 linux-user/mips/target_elf.h     |    3 ++
 target/mips/mips-defs.h          |    2 +
 target/mips/translate.c          |   31 ++++++++++++++++++++++++++-
 target/mips/translate_init.inc.c |   44 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 79 insertions(+), 1 deletion(-)

--- a/linux-user/mips/target_elf.h
+++ b/linux-user/mips/target_elf.h
@@ -12,6 +12,9 @@ static inline const char *cpu_get_model(uint32_t eflags)
     if ((eflags & EF_MIPS_ARCH) == EF_MIPS_ARCH_32R6) {
         return "mips32r6-generic";
     }
+    if ((eflags & EF_MIPS_MACH) == EF_MIPS_MACH_5900) {
+        return "R5900";
+    }
     return "24Kf";
 }
 #endif
--- a/target/mips/mips-defs.h
+++ b/target/mips/mips-defs.h
@@ -53,6 +53,7 @@
 #define   ASE_MSA       0x01000000
 
 /* Chip specific instructions. */
+#define		INSN_R5900	0x10000000
 #define		INSN_LOONGSON2E  0x20000000
 #define		INSN_LOONGSON2F  0x40000000
 #define		INSN_VR54XX	0x80000000
@@ -63,6 +64,7 @@
 #define		CPU_MIPS3	(CPU_MIPS2 | ISA_MIPS3)
 #define		CPU_MIPS4	(CPU_MIPS3 | ISA_MIPS4)
 #define		CPU_VR54XX	(CPU_MIPS4 | INSN_VR54XX)
+#define		CPU_R5900	(CPU_MIPS4 | INSN_R5900)
 #define		CPU_LOONGSON2E  (CPU_MIPS3 | INSN_LOONGSON2E)
 #define		CPU_LOONGSON2F  (CPU_MIPS3 | INSN_LOONGSON2F)
 
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -3768,6 +3768,31 @@ static void gen_muldiv(DisasContext *ctx, uint32_t opc,
     tcg_temp_free(t1);
 }
 
+static void gen_mul_r5900 (DisasContext *ctx, uint32_t opc,
+                            int rd, int rs, int rt)
+{
+    TCGv t0 = tcg_temp_new();
+    TCGv t1 = tcg_temp_new();
+
+    gen_load_gpr(t0, rs);
+    gen_load_gpr(t1, rt);
+
+    switch (opc) {
+    case OPC_MULT:
+    case OPC_MULTU:
+        tcg_gen_mul_tl(cpu_gpr[rd], t0, t1);
+        break;
+    default:
+        MIPS_INVAL("mul R5900");
+        generate_exception_end(ctx, EXCP_RI);
+        goto out;
+    }
+
+ out:
+    tcg_temp_free(t0);
+    tcg_temp_free(t1);
+}
+
 static void gen_mul_vr54xx (DisasContext *ctx, uint32_t opc,
                             int rd, int rs, int rt)
 {
@@ -22374,7 +22399,11 @@ static void decode_opc_special_legacy(CPUMIPSState *env, DisasContext *ctx)
         break;
     case OPC_MULT:
     case OPC_MULTU:
-        if (sa) {
+        if (ctx->insn_flags & INSN_R5900) {
+            gen_muldiv(ctx, op1, 0, rs, rt);
+            if (rd != 0)
+                gen_mul_r5900(ctx, op1, rd, rs, rt);
+        } else if (sa) {
             check_insn(ctx, INSN_VR54XX);
             op1 = MASK_MUL_VR54XX(ctx->opcode);
             gen_mul_vr54xx(ctx, op1, rd, rs, rt);
--- a/target/mips/translate_init.inc.c
+++ b/target/mips/translate_init.inc.c
@@ -410,6 +410,50 @@ const mips_def_t mips_defs[] =
         .insn_flags = CPU_MIPS32R5 | ASE_MSA,
         .mmu_type = MMU_TYPE_R4000,
     },
+    {
+        .name = "R5900",
+        .CP0_PRid = 0x00003800,
+        /* No L2 cache, icache size 32k, dcache size 32k, uncached coherency. */
+        .CP0_Config0 = (1 << 17) | (0x3 << 9) | (0x3 << 6) | (0x2 << CP0C0_K0),
+        /* Note: Config1 is only used internally, the R5900 has only Config0. */
+        .CP0_Status_rw_bitmask = 0xF4C79C1F,
+#ifdef CONFIG_USER_ONLY
+	/*
+	 * R5900 hardware traps to the Linux kernel for IEEE 754-1985 and LL/SC
+	 * emulation. For user-only, qemu is the kernel, so we emulate the traps
+	 * by simply emulating the instructions directly.
+	 */
+        .CP0_Config1 = (1 << CP0C1_FP) | (47 << CP0C1_MMU),
+        .CP0_LLAddr_rw_bitmask = 0xFFFFFFFF,
+        .CP0_LLAddr_shift = 4,
+        .CP1_fcr0 = (0x38 << FCR0_PRID) | (0x0 << FCR0_REV),
+        .CP1_fcr31 = 0,
+        .CP1_fcr31_rw_bitmask = 0x0183FFFF,
+#else
+	/*
+	 * The R5900 COP1 FPU implements single-precision floating-point
+	 * operations but is not entirely IEEE 754-1985 compatible. In
+	 * particular,
+	 *
+	 * - NaN (not a number) and plus/minus infinities are not supported;
+	 * - exception mechanisms are not fully supported;
+	 * - denormalized numbers are not supported;
+	 * - rounding towards nearest and plus/minus infinities are not supported;
+	 * - computed results usually differs in the least significant bit;
+	 * - saturating instructions can differ more than the least significant bit.
+	 *
+	 * Since only rounding towards zero is supported, the two least
+	 * significant bits of FCR31 are hardwired to 01.
+	 *
+	 * FPU emulation is disabled here until it is implemented.
+	 */
+        .CP0_Config1 = (47 << CP0C1_MMU),
+#endif /* CONFIG_USER_ONLY */
+        .SEGBITS = 19,
+        .PABITS = 20,
+        .insn_flags = CPU_R5900,
+        .mmu_type = MMU_TYPE_R4000,
+    },
     {
         /* A generic CPU supporting MIPS32 Release 6 ISA.
            FIXME: Support IEEE 754-2008 FP.

Re: [Qemu-devel] [PATCH v3] target/mips: Support R5900 GCC programs in user mode

[Aleksandar Markovic]

> > - organize the changes in the form of patch series
> 
> What kind of granularity do you have in mind? The patch is quite small with
> 79 insertions and 1 deletion in total.

For patch organization, you may find these links useful:

https://wiki.qemu.org/Contribute/SubmitAPatch

and 

https://wiki.openstack.org/wiki/GitCommitMessages

Patch size is not the best criterium while organizing patches.

One good organization of your changes would be:

Patch 1: Add preprocessor constants INSN_R5900 and CPU_R5900
Patch 2: Amend OPC_MULT/OPC_MULTU handling for R5900
Patch 3: Update cpu_get_model() for R5900 case
Patch 4: Add defintion of R5900 CPU

This will allow us to discuss individual changes separately.

Thanks,
Aleksandar

Re: [Qemu-devel] [PATCH v2] target/mips: Initial support for MIPS R5900

[Maciej W. Rozycki]

Hi Fredrik,

> Aleksandar, Aurelien, Maciej -- are you happy with this initial v2 patch?

 I have been more thorough on this occasion, and I do hope I have caught 
everything.  See the notes below, in addition to what the others wrote.  

 Please apply to v3 accordingly; I started writing this before you sent 
that version.

> --- a/target/mips/mips-defs.h
> +++ b/target/mips/mips-defs.h
> @@ -62,6 +63,7 @@
>  #define		CPU_MIPS3	(CPU_MIPS2 | ISA_MIPS3)
>  #define		CPU_MIPS4	(CPU_MIPS3 | ISA_MIPS4)
>  #define		CPU_VR54XX	(CPU_MIPS4 | INSN_VR54XX)
> +#define		CPU_R5900	(CPU_MIPS4 | INSN_R5900)

 I think this has to be (CPU_MIPS3 | INSN_R5900) really.

 While the CPU does support MIPS IV MOVN, MOVZ and PREF instructions, 
that's all it has from that ISA.  Even the Tx79, which was supposed to 
have a regular IEEE-754 FPU, did not have the extra FP condition bits, 
MOVN.fmt, MOVZ.fmt or any of the MOVF* or MOVT* instructions, indexed 
load/store instructions, multiply-accumulate instructions, etc. defined.

 So I think the R5900 has to be treated as a MIPS III implementation, with 
some aberrations.  I don't expect it to be a big deal to add INSN_R5900 
qualification to MOVN, MOVZ and PREF instruction emulation code right 
away.

 Then presumably you are going to add emulation for all the missing MIPS 
III instructions to the Linux kernel eventually, to match the MIPS III 
Linux user ABI?  Apart from LLD and SCD discussed previously this will 
have to include DDIV, DDIVU, DMULT and DMULTU.

 Eventually you'll have to remove all these instructions (plus LL and SC) 
from the system emulation mode.  In fact I think it would make sense to do 
that right away, because I believe it will be a reasonably simple update. 
However if it turns out to be a can of worms after all, then I think we 
can defer it, because we do not have a bare-metal environment ready for 
this CPU anyway (or do we?).

> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -17357,7 +17382,11 @@ static void decode_opc_special_legacy(CPUMIPSState *env, DisasContext *ctx)
>          break;
>      case OPC_MULT:
>      case OPC_MULTU:
> -        if (sa) {
> +        if (ctx->insn_flags & INSN_R5900) {
> +            gen_muldiv(ctx, op1, 0, rs, rt);
> +            if (rd != 0)
> +                gen_mul_r5900(ctx, op1, rd, rs, rt);
> +        } else if (sa) {
>              check_insn(ctx, INSN_VR54XX);
>              op1 = MASK_MUL_VR54XX(ctx->opcode);
>              gen_mul_vr54xx(ctx, op1, rd, rs, rt);

 I think there is no point in executing the multiplication twice if `rd != 
0' and also we want to continue trapping on `sa != 0' for the non-Vr54xx 
case, so how about:

        if (sa) {
            check_insn(ctx, INSN_VR54XX);
            op1 = MASK_MUL_VR54XX(ctx->opcode);
            gen_mul_vr54xx(ctx, op1, rd, rs, rt);
        } else if (rd && (ctx->insn_flags & INSN_R5900)) {
            gen_mul_r5900(ctx, op1, rd, rs, rt);
        } else {
            gen_muldiv(ctx, op1, rd & 3, rs, rt);
        }

?

> --- a/target/mips/translate_init.inc.c
> +++ b/target/mips/translate_init.inc.c
> @@ -410,6 +410,50 @@ const mips_def_t mips_defs[] =
>          .insn_flags = CPU_MIPS32R5 | ASE_MSA,
>          .mmu_type = MMU_TYPE_R4000,
>      },
> +    {
> +        .name = "R5900",
> +        .CP0_PRid = 0x00003800,
> +        /* No L2 cache, icache size 32k, dcache size 32k, uncached coherency. */
> +        .CP0_Config0 = (1 << 17) | (0x3 << 9) | (0x3 << 6) | (0x2 << CP0C0_K0),

 Why ICE set but DCE clear?  I guess this corresponds to the incorrect L2 
cache reference as bit #17 is SC on the original R4000.  That reference 
has to go, obviously, as there's no L2 cache indication in the R5900's 
Config register.

 As to ICE and DCE their reset defaults are both 0, so we might as well 
use that, as we don't emulate caches anyway.  If it turns out to matter to 
any software, then we'll have to provide a more correct Config register 
emulation as its writable bits are processor-specific.

> +        /* Note: Config1 is only used internally, the R5900 has only Config0. */
> +        .CP0_Status_rw_bitmask = 0xF4C79C1F,

 Maybe swap the two lines so that the Config1 register reference does not 
confusingly seem to describe the Status r/w bitmask?

> +#ifdef CONFIG_USER_ONLY
> +	/*
> +	 * R5900 hardware traps to the Linux kernel for IEEE 754-1985 and LL/SC
> +	 * emulation. For user-only, qemu is the kernel, so we emulate the traps
> +	 * by simply emulating the instructions directly.
> +	 */

 Please use spaces rather than tabs for indentation, as per QEMU's coding 
standard.

> +        .CP0_Config1 = (1 << CP0C1_FP) | (47 << CP0C1_MMU),
> +        .CP0_LLAddr_rw_bitmask = 0xFFFFFFFF,
> +        .CP0_LLAddr_shift = 4,
> +        .CP1_fcr0 = (0x38 << FCR0_PRID) | (0x0 << FCR0_REV),
> +        .CP1_fcr31 = 0,
> +        .CP1_fcr31_rw_bitmask = 0x0183FFFF,
> +#else
> +	/*
> +	 * The R5900 COP1 FPU implements single-precision floating-point
> +	 * operations but is not entirely IEEE 754-1985 compatible. In
> +	 * particular,
> +	 *
> +	 * - NaN (not a number) and plus/minus infinities are not supported;
> +	 * - exception mechanisms are not fully supported;
> +	 * - denormalized numbers are not supported;
> +	 * - rounding towards nearest and plus/minus infinities are not supported;
> +	 * - computed results usually differs in the least significant bit;
> +	 * - saturating instructions can differ more than the least significant bit.
> +	 *
> +	 * Since only rounding towards zero is supported, the two least
> +	 * significant bits of FCR31 are hardwired to 01.
> +	 *
> +	 * FPU emulation is disabled here until it is implemented.
> +	 */
> +        .CP0_Config1 = (47 << CP0C1_MMU),
> +#endif /* CONFIG_USER_ONLY */
> +        .SEGBITS = 19,

 This has to be 32, as with any MIPS CPU that implements 32-bit virtual 
addressing only.  Specifically EntryHi register's VPN2 field goes up to 
bit #31, which is (SEGBITS - 1).

> +        .PABITS = 20,

 And this has to be 32 as well, reflecting EntryLo0/1 registers' PFN 
fields going up to bit #25.  This corresponds to bit #31 on the external 
address bus, which is (PABITS - 1).

  Maciej

Re: [Qemu-devel] [PATCH v3] target/mips: Support R5900 GCC programs in user mode

[Fredrik Noring]

Many thanks for your review, Maciej,

>  I have been more thorough on this occasion, and I do hope I have caught 
> everything.  See the notes below, in addition to what the others wrote.  
> 
>  Please apply to v3 accordingly; I started writing this before you sent 
> that version.

Sure, next version submitted as a patch series will be a lot better, thanks!

>  I think this has to be (CPU_MIPS3 | INSN_R5900) really.
> 
>  While the CPU does support MIPS IV MOVN, MOVZ and PREF instructions, 
> that's all it has from that ISA.  Even the Tx79, which was supposed to 
> have a regular IEEE-754 FPU, did not have the extra FP condition bits, 
> MOVN.fmt, MOVZ.fmt or any of the MOVF* or MOVT* instructions, indexed 
> load/store instructions, multiply-accumulate instructions, etc. defined.
> 
>  So I think the R5900 has to be treated as a MIPS III implementation, with 
> some aberrations.  I don't expect it to be a big deal to add INSN_R5900 
> qualification to MOVN, MOVZ and PREF instruction emulation code right 
> away.

Trivial to add, in fact. :)

>  Then presumably you are going to add emulation for all the missing MIPS 
> III instructions to the Linux kernel eventually, to match the MIPS III 
> Linux user ABI?  Apart from LLD and SCD discussed previously this will 
> have to include DDIV, DDIVU, DMULT and DMULTU.

That sounds reasonable!

>  Eventually you'll have to remove all these instructions (plus LL and SC) 
> from the system emulation mode.  In fact I think it would make sense to do 
> that right away, because I believe it will be a reasonably simple update. 
> However if it turns out to be a can of worms after all, then I think we 
> can defer it, because we do not have a bare-metal environment ready for 
> this CPU anyway (or do we?).

Indeed, very simple. I added a new check function check_insn_opc_user_only,
similar to check_insn_opc_removed, conditional on USER_ONLY and so a simple

	check_insn_opc_user_only(ctx, INSN_R5900);

has to be added for these OPC cases. To test this I tried to build GCC with
the target mips64r5900el but it unfortunately didn't work as GCC failed to
compile itself:

	../sysdeps/mips/mips64/mul_1.S:49: Error: opcode not supported on
		this processor: r5900 (mips3) `dmultu $8,$7'

>  I think there is no point in executing the multiplication twice if `rd != 
> 0' and also we want to continue trapping on `sa != 0' for the non-Vr54xx 
> case, so how about:
> 
>         if (sa) {
>             check_insn(ctx, INSN_VR54XX);
>             op1 = MASK_MUL_VR54XX(ctx->opcode);
>             gen_mul_vr54xx(ctx, op1, rd, rs, rt);
>         } else if (rd && (ctx->insn_flags & INSN_R5900)) {
>             gen_mul_r5900(ctx, op1, rd, rs, rt);
>         } else {
>             gen_muldiv(ctx, op1, rd & 3, rs, rt);
>         }
> 
> ?

Agreed, that's better. The point was to reuse a bit of code, but now I've
folded all of it into gen_mul_r5900. There are also R5900 specific MULT1 and
MULTU1 instructions which might use it, so I folded the rd != 0 test into it
as well.

> > +        /* No L2 cache, icache size 32k, dcache size 32k, uncached coherency. */
> > +        .CP0_Config0 = (1 << 17) | (0x3 << 9) | (0x3 << 6) | (0x2 << CP0C0_K0),
> 
>  Why ICE set but DCE clear?  I guess this corresponds to the incorrect L2 
> cache reference as bit #17 is SC on the original R4000.  That reference 
> has to go, obviously, as there's no L2 cache indication in the R5900's 
> Config register.
> 
>  As to ICE and DCE their reset defaults are both 0, so we might as well 
> use that, as we don't emulate caches anyway.  If it turns out to matter to 
> any software, then we'll have to provide a more correct Config register 
> emulation as its writable bits are processor-specific.

Agreed, thanks for finding this!

> > +        /* Note: Config1 is only used internally, the R5900 has only Config0. */
> > +        .CP0_Status_rw_bitmask = 0xF4C79C1F,
> 
>  Maybe swap the two lines so that the Config1 register reference does not 
> confusingly seem to describe the Status r/w bitmask?

Done.

> > +#ifdef CONFIG_USER_ONLY
> > +	/*
> > +	 * R5900 hardware traps to the Linux kernel for IEEE 754-1985 and LL/SC
> > +	 * emulation. For user-only, qemu is the kernel, so we emulate the traps
> > +	 * by simply emulating the instructions directly.
> > +	 */
> 
>  Please use spaces rather than tabs for indentation, as per QEMU's coding 
> standard.

Done.

> > +        .SEGBITS = 19,
> 
>  This has to be 32, as with any MIPS CPU that implements 32-bit virtual 
> addressing only.  Specifically EntryHi register's VPN2 field goes up to 
> bit #31, which is (SEGBITS - 1).
> 
> > +        .PABITS = 20,
> 
>  And this has to be 32 as well, reflecting EntryLo0/1 registers' PFN 
> fields going up to bit #25.  This corresponds to bit #31 on the external 
> address bus, which is (PABITS - 1).

Right. I was unaware of these details, thanks!

Fredrik