Re: [PATCH 03/11] target/mips/mips-defs: Use ISA_MIPS32R2 definition to check Release 2

[Philippe Mathieu-Daudé]

On 12/16/20 4:16 PM, Jiaxun Yang wrote:
> 在 2020/12/16 21:43, Philippe Mathieu-Daudé 写道:
>> Use the single ISA_MIPS32R2 definition to check if the Release 2
>> ISA is supported, whether the CPU support 32/64-bit.
>>
>> For now we keep '32' in the definition name, we will rename it
>> as ISA_MIPS_R2 in few commits.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   target/mips/mips-defs.h    | 3 +--
>>   linux-user/mips/cpu_loop.c | 1 -
>>   target/mips/translate.c    | 4 ++--
>>   3 files changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/target/mips/mips-defs.h b/target/mips/mips-defs.h
>> index 2756e72a9d6..9cfa4c346bf 100644
>> --- a/target/mips/mips-defs.h
>> +++ b/target/mips/mips-defs.h
>> @@ -24,7 +24,6 @@
>>   #define ISA_MIPS5         0x0000000000000010ULL
>>   #define ISA_MIPS32        0x0000000000000020ULL
>>   #define ISA_MIPS32R2      0x0000000000000040ULL
>> -#define ISA_MIPS64R2      0x0000000000000100ULL
>>   #define ISA_MIPS32R3      0x0000000000000200ULL
>>   #define ISA_MIPS64R3      0x0000000000000400ULL
>>   #define ISA_MIPS32R5      0x0000000000000800ULL
>> @@ -81,7 +80,7 @@
>>     /* MIPS Technologies "Release 2" */
>>   #define CPU_MIPS32R2    (CPU_MIPS32 | ISA_MIPS32R2)
>> -#define CPU_MIPS64R2    (CPU_MIPS64 | CPU_MIPS32R2 | ISA_MIPS64R2)
>> +#define CPU_MIPS64R2    (CPU_MIPS64 | ISA_MIPS32R2)
>>     /* MIPS Technologies "Release 3" */
>>   #define CPU_MIPS32R3    (CPU_MIPS32R2 | ISA_MIPS32R3)
>> diff --git a/linux-user/mips/cpu_loop.c b/linux-user/mips/cpu_loop.c
>> index b58dbeb83d1..a2aa8167210 100644
>> --- a/linux-user/mips/cpu_loop.c
>> +++ b/linux-user/mips/cpu_loop.c
>> @@ -386,7 +386,6 @@ void target_cpu_copy_regs(CPUArchState *env,
>> struct target_pt_regs *regs)
>>       prog_req.fre &= interp_req.fre;
>>         bool cpu_has_mips_r2_r6 = env->insn_flags & ISA_MIPS32R2 ||
>> -                              env->insn_flags & ISA_MIPS64R2 ||
>>                                 env->insn_flags & ISA_MIPS32R6 ||
>>                                 env->insn_flags & ISA_MIPS64R6;
>>   diff --git a/target/mips/translate.c b/target/mips/translate.c
>> index 8c0ecfa17e1..0923dfdf451 100644
>> --- a/target/mips/translate.c
>> +++ b/target/mips/translate.c
>> @@ -28212,7 +28212,7 @@ static void decode_opc_special3(CPUMIPSState
>> *env, DisasContext *ctx)
>>       case OPC_DINSM:
>>       case OPC_DINSU:
>>       case OPC_DINS:
>> -        check_insn(ctx, ISA_MIPS64R2);
>> +        check_insn(ctx, ISA_MIPS32R2);
>>           check_mips_64(ctx);
> 

Sorry I respin v2 before noticing this reply.

> After this change, 32-bit CPUs emulated with TARGET_MIPS64
> and got CP0.Status.KX and CP0.Status.UX accidentally set won't
> emit RI exception.

32-bit CPUs shouldn't have MIPS_HFLAG_64 set, regardless
the build used. So check_mips_64() will emit it...

Anyhow, I'd rather remove 32-bit CPUs from 64-bit build unless
we are sure this works.

> 
> Probably we need to check ISA_MIPS64 when calculating
> MIPS_HFLAG_64 and setting ksux in cpu_mips_store_status?
> 
> Thanks.
> 
> - Jiaxun
> 
>>           gen_bitops(ctx, op1, rt, rs, sa, rd);
>>           break;
>> @@ -28232,7 +28232,7 @@ static void decode_opc_special3(CPUMIPSState
>> *env, DisasContext *ctx)
>>               decode_opc_special3_r6(env, ctx);
>>               break;
>>           default:
>> -            check_insn(ctx, ISA_MIPS64R2);
>> +            check_insn(ctx, ISA_MIPS32R2);
>>               check_mips_64(ctx);
>>               op2 = MASK_DBSHFL(ctx->opcode);
>>               gen_bshfl(ctx, op2, rt, rd);
>