Re: [Qemu-devel] [PATCH v6 4/7] target/mips: R5900 DMULT[U], DDIV[U], LL[D] and SC[D] are user only

[Philippe Mathieu-Daudé]

On 9/16/18 5:13 PM, Fredrik Noring wrote:
> The Linux kernel traps certain reserved instruction exceptions to
> emulate the corresponding instructions. QEMU is the kernel in user
> mode, so those traps are emulated by accepting the instructions.
> 
> This change adds the function check_insn_opc_user_only to signal a
> reserved instruction exception for flagged CPUs in QEMU system mode.
> 
> The MIPS III instructions DMULT[U], DDIV[U], LL[D] and SC[D] are not
> implemented in R5900 hardware. They are trapped and emulated by the
> Linux kernel and, accordingly, therefore QEMU user only instructions.
> 
> Signed-off-by: Fredrik Noring <address@hidden>
> ---
>  target/mips/translate.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 0c445c11c5..5a5021fe36 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -1887,6 +1887,21 @@ static inline void check_insn_opc_removed(DisasContext 
> *ctx, int flags)
>      }
>  }
>  
> +/*
> + * The Linux kernel traps certain reserved instruction exceptions to
> + * emulate the corresponding instructions. QEMU is the kernel in user
> + * mode, so those traps are emulated by accepting the instructions.
> + *
> + * A reserved instruction exception is generated for flagged CPUs if
> + * QEMU runs in system mode.
> + */
> +static inline void check_insn_opc_user_only(DisasContext *ctx, int flags)
> +{
> +#ifndef CONFIG_USER_ONLY
> +    check_insn_opc_removed(ctx, flags);
> +#endif
> +}
> +
>  /* This code generates a "reserved instruction" exception if the
>     CPU does not support 64-bit paired-single (PS) floating point data type */
>  static inline void check_ps(DisasContext *ctx)
> @@ -22465,6 +22480,7 @@ static void decode_opc_special_legacy(CPUMIPSState 
> *env, DisasContext *ctx)

Expanding the diff context for easier review:

   #if defined(TARGET_MIPS64)
       case OPC_DMULT:
       case OPC_DMULTU:

>      case OPC_DDIV:
>      case OPC_DDIVU:
>          check_insn(ctx, ISA_MIPS3);
> +        check_insn_opc_user_only(ctx, INSN_R5900);
>          check_mips_64(ctx);
>          gen_muldiv(ctx, op1, 0, rs, rt);
>          break;
> @@ -24968,6 +24984,7 @@ static void decode_opc(CPUMIPSState *env, 
> DisasContext *ctx)
>           break;
>      case OPC_LL: /* Load and stores */
>          check_insn(ctx, ISA_MIPS2);
> +        check_insn_opc_user_only(ctx, INSN_R5900);
>          /* Fallthrough */
>      case OPC_LWL:
>      case OPC_LWR:
> @@ -24993,6 +25010,7 @@ static void decode_opc(CPUMIPSState *env, 
> DisasContext *ctx)
>      case OPC_SC:
>          check_insn(ctx, ISA_MIPS2);
>           check_insn_opc_removed(ctx, ISA_MIPS32R6);
> +        check_insn_opc_user_only(ctx, INSN_R5900);
>           gen_st_cond(ctx, op, rt, rs, imm);
>           break;
>      case OPC_CACHE:
> @@ -25259,9 +25277,11 @@ static void decode_opc(CPUMIPSState *env, 
> DisasContext *ctx)
>  
>  #if defined(TARGET_MIPS64)
>      /* MIPS64 opcodes */
> +    case OPC_LLD:
> +        check_insn_opc_user_only(ctx, INSN_R5900);
> +        /* fall through */
>      case OPC_LDL:
>      case OPC_LDR:
> -    case OPC_LLD:
>          check_insn_opc_removed(ctx, ISA_MIPS32R6);
>          /* fall through */
>      case OPC_LWU:
> @@ -25282,6 +25302,7 @@ static void decode_opc(CPUMIPSState *env, 
> DisasContext *ctx)
>      case OPC_SCD:
>          check_insn_opc_removed(ctx, ISA_MIPS32R6);
>          check_insn(ctx, ISA_MIPS3);
> +        check_insn_opc_user_only(ctx, INSN_R5900);
>          check_mips_64(ctx);
>          gen_st_cond(ctx, op, rt, rs, imm);
>          break;
> 

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>