[Qemu-devel] [PATCH] target-mips: Tighten ISA level checks

[Maciej W. Rozycki]

Tighten ISA level checks down to MIPS II that many of our instructions
are missing.  Also make sure any 64-bit instruction enables are only
applied to 64-bit processors, that is ones that implement at least the
MIPS III ISA.

Signed-off-by: Maciej W. Rozycki <address@hidden>
---
Hi,

 As usually with changes that require manual code review I may have 
missed something here, but I believe I did not.  I went thoroughly 
through all the code and checked all the instructions.  Of course that 
was before MIPSr6, but I do hope for the latter the right checks have 
been implemented from the beginning.

 I rigorously tested this change by running full GCC, G++ and glibc 
mips-linux-gnu toolchain test suites under Linux (Debian Wheezy) run in 
QEMU in the system emulation mode, for the following multilibs:

-EB
-EB -mips16
-EB -mmicromips
-EB -mabi=n32
-EB -mabi=64

and the -EL variants of same.  Of these standard MIPS o32 testing was 
run on a 24Kf processor and n64/n32 testing was run on a 5KEf processor, 
using a 32-bit and a 64-bit kernel respectively.  MIPS16 o32 testing was 
run on an artificial 5KEf-mips16 processor -- like a real 5KEf one, but 
with the MIPS16 ASE enabled, and a 64-bit kernel.  Finally microMIPS o32 
testing was run on an artificial 24Kf-micromips processor -- like a real 
24Kf one, but with the microMIPS ASE enabled, and a 32-bit kernel built 
as microMIPS code itself.

 That should have executed enough paths in the emulator, both for user 
instructions and privileged (CP0) instructions, as both the user side 
and the kernel side were tested at the same time.

 The original test result counts were as follows:

Result           Count
ERROR               20
FAIL               300
PASS           1732076
XPASS              335
XFAIL             6527
UNRESOLVED          26
UNSUPPORTED      50854
Total          1790138

After the change they were:

Result           Count
ERROR               20
FAIL               303
PASS           1732073
XPASS              336
XFAIL             6526
UNRESOLVED          26
UNSUPPORTED      50854
Total          1790138

The changes in results were as follows:

PASS  -> FAIL:  qemu-64/glibc.sum:        malloc/tst-mallocfork
PASS  -> FAIL:  qemu-mips16/glibc.sum:    nptl/tst-setuid3
PASS  -> FAIL:  qemu-n32-el/glibc.sum:    malloc/tst-malloc-usable
PASS  -> FAIL:  qemu-n32/glibc.sum:       nptl/tst-cancel17
FAIL  -> PASS:  qemu-micromips/glibc.sum: nptl/tst-setuid3
XFAIL -> XPASS: qemu-micromips/glibc.sum: nptl/tst-cancel7

-- which I qualify as intermittent failures that unfortunately happen 
all the time, also on real hardware, so they do not appear to be issues 
arising from this change to QEMU.

 I believe this satisfies quality verification requirements for such an 
extensive change.  Please apply.

  Maciej

qemu-mips-isa-check.diff
Index: qemu-git-trunk/target-mips/cpu.h
===================================================================
--- qemu-git-trunk.orig/target-mips/cpu.h       2014-11-12 07:38:07.577674675 
+0000
+++ qemu-git-trunk/target-mips/cpu.h    2014-11-12 07:38:24.077713983 +0000
@@ -831,9 +831,10 @@ static inline void compute_hflags(CPUMIP
         env->hflags |= (env->CP0_Status >> CP0St_KSU) & MIPS_HFLAG_KSU;
     }
 #if defined(TARGET_MIPS64)
-    if (((env->hflags & MIPS_HFLAG_KSU) != MIPS_HFLAG_UM) ||
-        (env->CP0_Status & (1 << CP0St_PX)) ||
-        (env->CP0_Status & (1 << CP0St_UX))) {
+    if ((env->insn_flags & ISA_MIPS3) &&
+        (((env->hflags & MIPS_HFLAG_KSU) != MIPS_HFLAG_UM) ||
+         (env->CP0_Status & (1 << CP0St_PX)) ||
+         (env->CP0_Status & (1 << CP0St_UX)))) {
         env->hflags |= MIPS_HFLAG_64;
     }
 
Index: qemu-git-trunk/target-mips/helper.c
===================================================================
--- qemu-git-trunk.orig/target-mips/helper.c    2014-11-12 07:33:08.548361020 
+0000
+++ qemu-git-trunk/target-mips/helper.c 2014-11-12 07:38:24.077713983 +0000
@@ -527,7 +527,9 @@ void mips_cpu_do_interrupt(CPUState *cs)
         env->CP0_DEPC = exception_resume_pc(env);
         env->hflags &= ~MIPS_HFLAG_BMASK;
  enter_debug_mode:
-        env->hflags |= MIPS_HFLAG_DM | MIPS_HFLAG_64 | MIPS_HFLAG_CP0;
+        if (env->insn_flags & ISA_MIPS3)
+            env->hflags |= MIPS_HFLAG_64;
+        env->hflags |= MIPS_HFLAG_DM | MIPS_HFLAG_CP0;
         env->hflags &= ~(MIPS_HFLAG_KSU);
         /* EJTAG probe trap enable is not implemented... */
         if (!(env->CP0_Status & (1 << CP0St_EXL)))
@@ -548,7 +550,9 @@ void mips_cpu_do_interrupt(CPUState *cs)
         env->CP0_ErrorEPC = exception_resume_pc(env);
         env->hflags &= ~MIPS_HFLAG_BMASK;
         env->CP0_Status |= (1 << CP0St_ERL) | (1 << CP0St_BEV);
-        env->hflags |= MIPS_HFLAG_64 | MIPS_HFLAG_CP0;
+        if (env->insn_flags & ISA_MIPS3)
+            env->hflags |= MIPS_HFLAG_64;
+        env->hflags |= MIPS_HFLAG_CP0;
         env->hflags &= ~(MIPS_HFLAG_KSU);
         if (!(env->CP0_Status & (1 << CP0St_EXL)))
             env->CP0_Cause &= ~(1U << CP0Ca_BD);
@@ -726,7 +730,9 @@ void mips_cpu_do_interrupt(CPUState *cs)
                 env->CP0_Cause &= ~(1U << CP0Ca_BD);
             }
             env->CP0_Status |= (1 << CP0St_EXL);
-            env->hflags |= MIPS_HFLAG_64 | MIPS_HFLAG_CP0;
+            if (env->insn_flags & ISA_MIPS3)
+                env->hflags |= MIPS_HFLAG_64;
+            env->hflags |= MIPS_HFLAG_CP0;
             env->hflags &= ~(MIPS_HFLAG_KSU);
         }
         env->hflags &= ~MIPS_HFLAG_BMASK;
Index: qemu-git-trunk/target-mips/translate.c
===================================================================
--- qemu-git-trunk.orig/target-mips/translate.c 2014-11-12 07:38:03.579009630 
+0000
+++ qemu-git-trunk/target-mips/translate.c      2014-11-12 07:38:24.077713983 
+0000
@@ -2398,7 +2398,14 @@ static void gen_cop1_ldst(DisasContext *
 {
     if (ctx->CP0_Config1 & (1 << CP0C1_FP)) {
         check_cp1_enabled(ctx);
-        gen_flt_ldst(ctx, op, rt, rs, imm);
+        switch (op) {
+        case OPC_LDC1:
+        case OPC_SDC1:
+            check_insn(ctx, ISA_MIPS2);
+            /* Fallthrough */
+        default:
+            gen_flt_ldst(ctx, op, rt, rs, imm);
+        }
     } else {
         generate_exception_err(ctx, EXCP_CpU, 1);
     }
@@ -10993,26 +11000,32 @@ static void decode_i64_mips16 (DisasCont
 {
     switch (funct) {
     case I64_LDSP:
+        check_insn(ctx, ISA_MIPS3);
         check_mips_64(ctx);
         offset = extended ? offset : offset << 3;
         gen_ld(ctx, OPC_LD, ry, 29, offset);
         break;
     case I64_SDSP:
+        check_insn(ctx, ISA_MIPS3);
         check_mips_64(ctx);
         offset = extended ? offset : offset << 3;
         gen_st(ctx, OPC_SD, ry, 29, offset);
         break;
     case I64_SDRASP:
+        check_insn(ctx, ISA_MIPS3);
         check_mips_64(ctx);
         offset = extended ? offset : (ctx->opcode & 0xff) << 3;
         gen_st(ctx, OPC_SD, 31, 29, offset);
         break;
     case I64_DADJSP:
+        check_insn(ctx, ISA_MIPS3);
         check_mips_64(ctx);
         offset = extended ? offset : ((int8_t)ctx->opcode) << 3;
         gen_arith_imm(ctx, OPC_DADDIU, 29, 29, offset);
         break;
     case I64_LDPC:
+        check_insn(ctx, ISA_MIPS3);
+        check_mips_64(ctx);
         if (extended && (ctx->hflags & MIPS_HFLAG_BMASK)) {
             generate_exception(ctx, EXCP_RI);
         } else {
@@ -11021,16 +11034,19 @@ static void decode_i64_mips16 (DisasCont
         }
         break;
     case I64_DADDIU5:
+        check_insn(ctx, ISA_MIPS3);
         check_mips_64(ctx);
         offset = extended ? offset : ((int8_t)(offset << 3)) >> 3;
         gen_arith_imm(ctx, OPC_DADDIU, ry, ry, offset);
         break;
     case I64_DADDIUPC:
+        check_insn(ctx, ISA_MIPS3);
         check_mips_64(ctx);
         offset = extended ? offset : offset << 2;
         gen_addiupc(ctx, ry, offset, 1, extended);
         break;
     case I64_DADDIUSP:
+        check_insn(ctx, ISA_MIPS3);
         check_mips_64(ctx);
         offset = extended ? offset : offset << 2;
         gen_arith_imm(ctx, OPC_DADDIU, ry, 29, offset);
@@ -11099,6 +11115,7 @@ static int decode_extended_mips16_opc (C
         break;
 #if defined(TARGET_MIPS64)
     case M16_OPC_LD:
+        check_insn(ctx, ISA_MIPS3);
         check_mips_64(ctx);
         gen_ld(ctx, OPC_LD, ry, rx, offset);
         break;
@@ -11143,6 +11160,7 @@ static int decode_extended_mips16_opc (C
             gen_arith_imm(ctx, OPC_ADDIU, 29, 29, imm);
             break;
         case I8_SVRS:
+            check_insn(ctx, ISA_MIPS32);
             {
                 int xsregs = (ctx->opcode >> 24) & 0x7;
                 int aregs = (ctx->opcode >> 16) & 0xf;
@@ -11176,6 +11194,8 @@ static int decode_extended_mips16_opc (C
         break;
 #if defined(TARGET_MIPS64)
     case M16_OPC_SD:
+        check_insn(ctx, ISA_MIPS3);
+        check_mips_64(ctx);
         gen_st(ctx, OPC_SD, ry, rx, offset);
         break;
 #endif
@@ -11202,6 +11222,8 @@ static int decode_extended_mips16_opc (C
         break;
 #if defined(TARGET_MIPS64)
     case M16_OPC_LWU:
+        check_insn(ctx, ISA_MIPS3);
+        check_mips_64(ctx);
         gen_ld(ctx, OPC_LWU, ry, rx, offset);
         break;
 #endif
@@ -11291,6 +11313,7 @@ static int decode_mips16_opc (CPUMIPSSta
             break;
         case 0x1:
 #if defined(TARGET_MIPS64)
+            check_insn(ctx, ISA_MIPS3);
             check_mips_64(ctx);
             gen_shift_imm(ctx, OPC_DSLL, rx, ry, sa);
 #else
@@ -11307,6 +11330,7 @@ static int decode_mips16_opc (CPUMIPSSta
         break;
 #if defined(TARGET_MIPS64)
     case M16_OPC_LD:
+        check_insn(ctx, ISA_MIPS3);
         check_mips_64(ctx);
         gen_ld(ctx, OPC_LD, ry, rx, offset << 3);
         break;
@@ -11317,6 +11341,7 @@ static int decode_mips16_opc (CPUMIPSSta
 
             if ((ctx->opcode >> 4) & 1) {
 #if defined(TARGET_MIPS64)
+                check_insn(ctx, ISA_MIPS3);
                 check_mips_64(ctx);
                 gen_arith_imm(ctx, OPC_DADDIU, ry, rx, imm);
 #else
@@ -11368,6 +11393,7 @@ static int decode_mips16_opc (CPUMIPSSta
                               ((int8_t)ctx->opcode) << 3);
                 break;
             case I8_SVRS:
+                check_insn(ctx, ISA_MIPS32);
                 {
                     int do_ra = ctx->opcode & (1 << 6);
                     int do_s0 = ctx->opcode & (1 << 5);
@@ -11423,6 +11449,7 @@ static int decode_mips16_opc (CPUMIPSSta
         break;
 #if defined(TARGET_MIPS64)
     case M16_OPC_SD:
+        check_insn(ctx, ISA_MIPS3);
         check_mips_64(ctx);
         gen_st(ctx, OPC_SD, ry, rx, offset << 3);
         break;
@@ -11450,6 +11477,7 @@ static int decode_mips16_opc (CPUMIPSSta
         break;
 #if defined (TARGET_MIPS64)
     case M16_OPC_LWU:
+        check_insn(ctx, ISA_MIPS3);
         check_mips_64(ctx);
         gen_ld(ctx, OPC_LWU, ry, rx, offset << 2);
         break;
@@ -11481,10 +11509,12 @@ static int decode_mips16_opc (CPUMIPSSta
 #if defined(TARGET_MIPS64)
             case RRR_DADDU:
                 mips32_op = OPC_DADDU;
+                check_insn(ctx, ISA_MIPS3);
                 check_mips_64(ctx);
                 break;
             case RRR_DSUBU:
                 mips32_op = OPC_DSUBU;
+                check_insn(ctx, ISA_MIPS3);
                 check_mips_64(ctx);
                 break;
 #endif
@@ -11506,6 +11536,9 @@ static int decode_mips16_opc (CPUMIPSSta
                 int link = (ctx->opcode >> 6) & 0x1;
                 int ra = (ctx->opcode >> 5) & 0x1;
 
+                if (nd)
+                    check_insn(ctx, ISA_MIPS32);
+
                 if (link) {
                     op = OPC_JALR;
                 } else {
@@ -11547,6 +11580,7 @@ static int decode_mips16_opc (CPUMIPSSta
             break;
 #if defined (TARGET_MIPS64)
         case RR_DSRL:
+            check_insn(ctx, ISA_MIPS3);
             check_mips_64(ctx);
             gen_shift_imm(ctx, OPC_DSRL, ry, ry, sa);
             break;
@@ -11573,6 +11607,7 @@ static int decode_mips16_opc (CPUMIPSSta
             gen_HILO(ctx, OPC_MFHI, 0, rx);
             break;
         case RR_CNVT:
+            check_insn(ctx, ISA_MIPS32);
             switch (cnvt_op) {
             case RR_RY_CNVT_ZEB:
                 tcg_gen_ext8u_tl(cpu_gpr[rx], cpu_gpr[rx]);
@@ -11588,10 +11623,12 @@ static int decode_mips16_opc (CPUMIPSSta
                 break;
 #if defined (TARGET_MIPS64)
             case RR_RY_CNVT_ZEW:
+                check_insn(ctx, ISA_MIPS64);
                 check_mips_64(ctx);
                 tcg_gen_ext32u_tl(cpu_gpr[rx], cpu_gpr[rx]);
                 break;
             case RR_RY_CNVT_SEW:
+                check_insn(ctx, ISA_MIPS64);
                 check_mips_64(ctx);
                 tcg_gen_ext32s_tl(cpu_gpr[rx], cpu_gpr[rx]);
                 break;
@@ -11606,18 +11643,22 @@ static int decode_mips16_opc (CPUMIPSSta
             break;
 #if defined (TARGET_MIPS64)
         case RR_DSRA:
+            check_insn(ctx, ISA_MIPS3);
             check_mips_64(ctx);
             gen_shift_imm(ctx, OPC_DSRA, ry, ry, sa);
             break;
         case RR_DSLLV:
+            check_insn(ctx, ISA_MIPS3);
             check_mips_64(ctx);
             gen_shift(ctx, OPC_DSLLV, ry, rx, ry);
             break;
         case RR_DSRLV:
+            check_insn(ctx, ISA_MIPS3);
             check_mips_64(ctx);
             gen_shift(ctx, OPC_DSRLV, ry, rx, ry);
             break;
         case RR_DSRAV:
+            check_insn(ctx, ISA_MIPS3);
             check_mips_64(ctx);
             gen_shift(ctx, OPC_DSRAV, ry, rx, ry);
             break;
@@ -11636,18 +11677,22 @@ static int decode_mips16_opc (CPUMIPSSta
             break;
 #if defined (TARGET_MIPS64)
         case RR_DMULT:
+            check_insn(ctx, ISA_MIPS3);
             check_mips_64(ctx);
             gen_muldiv(ctx, OPC_DMULT, 0, rx, ry);
             break;
         case RR_DMULTU:
+            check_insn(ctx, ISA_MIPS3);
             check_mips_64(ctx);
             gen_muldiv(ctx, OPC_DMULTU, 0, rx, ry);
             break;
         case RR_DDIV:
+            check_insn(ctx, ISA_MIPS3);
             check_mips_64(ctx);
             gen_muldiv(ctx, OPC_DDIV, 0, rx, ry);
             break;
         case RR_DDIVU:
+            check_insn(ctx, ISA_MIPS3);
             check_mips_64(ctx);
             gen_muldiv(ctx, OPC_DDIVU, 0, rx, ry);
             break;
@@ -13212,20 +13257,26 @@ static void decode_micromips32_opc (CPUM
             /* COP2: Not implemented. */
             generate_exception_err(ctx, EXCP_CpU, 2);
             break;
-        case LWP:
-        case SWP:
 #ifdef TARGET_MIPS64
         case LDP:
         case SDP:
+            check_insn(ctx, ISA_MIPS3);
+            check_mips_64(ctx);
+            /* Fallthrough */
 #endif
+        case LWP:
+        case SWP:
             gen_ldst_pair(ctx, minor, rt, rs, SIMM(ctx->opcode, 0, 12));
             break;
-        case LWM32:
-        case SWM32:
 #ifdef TARGET_MIPS64
         case LDM:
         case SDM:
+            check_insn(ctx, ISA_MIPS3);
+            check_mips_64(ctx);
+            /* Fallthrough */
 #endif
+        case LWM32:
+        case SWM32:
             gen_ldst_multiple(ctx, minor, rt, rs, SIMM(ctx->opcode, 0, 12));
             break;
         default:
@@ -13649,21 +13700,33 @@ static void decode_micromips32_opc (CPUM
             goto do_st_lr;
 #if defined(TARGET_MIPS64)
         case LDL:
+            check_insn(ctx, ISA_MIPS3);
+            check_mips_64(ctx);
             mips32_op = OPC_LDL;
             goto do_ld_lr;
         case SDL:
+            check_insn(ctx, ISA_MIPS3);
+            check_mips_64(ctx);
             mips32_op = OPC_SDL;
             goto do_st_lr;
         case LDR:
+            check_insn(ctx, ISA_MIPS3);
+            check_mips_64(ctx);
             mips32_op = OPC_LDR;
             goto do_ld_lr;
         case SDR:
+            check_insn(ctx, ISA_MIPS3);
+            check_mips_64(ctx);
             mips32_op = OPC_SDR;
             goto do_st_lr;
         case LWU:
+            check_insn(ctx, ISA_MIPS3);
+            check_mips_64(ctx);
             mips32_op = OPC_LWU;
             goto do_ld_lr;
         case LLD:
+            check_insn(ctx, ISA_MIPS3);
+            check_mips_64(ctx);
             mips32_op = OPC_LLD;
             goto do_ld_lr;
 #endif
@@ -13681,6 +13744,8 @@ static void decode_micromips32_opc (CPUM
             break;
 #if defined(TARGET_MIPS64)
         case SCD:
+            check_insn(ctx, ISA_MIPS3);
+            check_mips_64(ctx);
             gen_st_cond(ctx, OPC_SCD, rt, rs, SIMM(ctx->opcode, 0, 12));
             break;
 #endif
@@ -13790,9 +13855,13 @@ static void decode_micromips32_opc (CPUM
         goto do_ld;
 #ifdef TARGET_MIPS64
     case LD32:
+        check_insn(ctx, ISA_MIPS3);
+        check_mips_64(ctx);
         mips32_op = OPC_LD;
         goto do_ld;
     case SD32:
+        check_insn(ctx, ISA_MIPS3);
+        check_mips_64(ctx);
         mips32_op = OPC_SD;
         goto do_st;
 #endif
@@ -16322,6 +16391,7 @@ static void decode_opc_special(CPUMIPSSt
         break;
     case OPC_TGE ... OPC_TEQ: /* Traps */
     case OPC_TNE:
+        check_insn(ctx, ISA_MIPS2);
         gen_trap(ctx, op1, rs, rt, -1);
         break;
     case OPC_LSA: /* OPC_PMON */
@@ -16346,6 +16416,7 @@ static void decode_opc_special(CPUMIPSSt
         generate_exception(ctx, EXCP_BREAK);
         break;
     case OPC_SYNC:
+        check_insn(ctx, ISA_MIPS2);
         /* Treat as NOP. */
         break;
 
@@ -18392,7 +18463,9 @@ static void decode_opc(CPUMIPSState *env
         case OPC_BGEZL:
         case OPC_BLTZALL:
         case OPC_BGEZALL:
+            check_insn(ctx, ISA_MIPS2);
             check_insn_opc_removed(ctx, ISA_MIPS32R6);
+            /* Fallthrough */
         case OPC_BLTZ:
         case OPC_BGEZ:
             gen_compute_branch(ctx, op1, 4, rs, -1, imm << 2, 4);
@@ -18412,6 +18485,7 @@ static void decode_opc(CPUMIPSState *env
             break;
         case OPC_TGEI ... OPC_TEQI: /* REGIMM traps */
         case OPC_TNEI:
+            check_insn(ctx, ISA_MIPS2);
             check_insn_opc_removed(ctx, ISA_MIPS32R6);
             gen_trap(ctx, op1, rs, -1, imm);
             break;
@@ -18618,15 +18692,20 @@ static void decode_opc(CPUMIPSState *env
         break;
     case OPC_BEQL:
     case OPC_BNEL:
+        check_insn(ctx, ISA_MIPS2);
          check_insn_opc_removed(ctx, ISA_MIPS32R6);
+        /* Fallthrough */
     case OPC_BEQ:
     case OPC_BNE:
          gen_compute_branch(ctx, op, 4, rs, rt, imm << 2, 4);
          break;
-    case OPC_LWL: /* Load and stores */
+    case OPC_LL: /* Load and stores */
+        check_insn(ctx, ISA_MIPS2);
+        /* Fallthrough */
+    case OPC_LWL:
     case OPC_LWR:
-    case OPC_LL:
         check_insn_opc_removed(ctx, ISA_MIPS32R6);
+         /* Fallthrough */
     case OPC_LB ... OPC_LH:
     case OPC_LW ... OPC_LHU:
          gen_ld(ctx, op, rt, rs, imm);
@@ -18639,6 +18718,7 @@ static void decode_opc(CPUMIPSState *env
          gen_st(ctx, op, rt, rs, imm);
          break;
     case OPC_SC:
+        check_insn(ctx, ISA_MIPS2);
          check_insn_opc_removed(ctx, ISA_MIPS32R6);
          gen_st_cond(ctx, op, rt, rs, imm);
          break;
@@ -18682,6 +18762,7 @@ static void decode_opc(CPUMIPSState *env
         case OPC_DMTC1:
             check_cp1_enabled(ctx);
             check_insn(ctx, ISA_MIPS3);
+            check_mips_64(ctx);
             gen_cp1(ctx, op1, rt, rd);
             break;
 #endif
@@ -18854,18 +18935,24 @@ static void decode_opc(CPUMIPSState *env
             check_cp1_enabled(ctx);
             op1 = MASK_CP3(ctx->opcode);
             switch (op1) {
+            case OPC_LUXC1:
+            case OPC_SUXC1:
+                check_insn(ctx, ISA_MIPS5 | ISA_MIPS32R2);
+                /* Fallthrough */
             case OPC_LWXC1:
             case OPC_LDXC1:
-            case OPC_LUXC1:
             case OPC_SWXC1:
             case OPC_SDXC1:
-            case OPC_SUXC1:
+                check_insn(ctx, ISA_MIPS4 | ISA_MIPS32R2);
                 gen_flt3_ldst(ctx, op1, sa, rd, rs, rt);
                 break;
             case OPC_PREFX:
+                check_insn(ctx, ISA_MIPS4 | ISA_MIPS32R2);
                 /* Treat as NOP. */
                 break;
             case OPC_ALNV_PS:
+                check_insn(ctx, ISA_MIPS5 | ISA_MIPS32R2);
+                /* Fallthrough */
             case OPC_MADD_S:
             case OPC_MADD_D:
             case OPC_MADD_PS:
@@ -18878,6 +18965,7 @@ static void decode_opc(CPUMIPSState *env
             case OPC_NMSUB_S:
             case OPC_NMSUB_D:
             case OPC_NMSUB_PS:
+                check_insn(ctx, ISA_MIPS4 | ISA_MIPS32R2);
                 gen_flt3_arith(ctx, op1, sa, rs, rd, rt);
                 break;
             default: