Re: [Qemu-devel] [PATCH v8 9/9] target-mips: Implement FCR31's R/W bitmask and related functionalities

[Leon Alrae]

> @@ -110,9 +110,11 @@ struct CPUMIPSFPUContext {
>  #define FCR0_PRID 8
>  #define FCR0_REV 0
>      /* fcsr */
> +    uint32_t fcr31_rw_bitmask;
>      uint32_t fcr31;
> -#define FCR31_ABS2008 19
> -#define FCR31_NAN2008 18
> +#define FCR31_NAN2008     18
> +#define FCR31_ABS2008     19

Now the order is inconsistent with other #defines in cpu.h, I think there's
no need to touch these lines.

> +#define FCR31_FS          24
> @@ -813,14 +815,21 @@ static inline void restore_rounding_mode(CPUMIPSState 
> *env)
>  
>  static inline void restore_flush_mode(CPUMIPSState *env)
>  {
> -    set_flush_to_zero((env->active_fpu.fcr31 & (1 << 24)) != 0,
> +    set_flush_to_zero((env->active_fpu.fcr31 & (1 << FCR31_FS)) != 0,

it would be ideal to move this change with FCR31_FS to a separate patch as
this isn't directly related to implementing fcr31_rw_bitmask.

>                        &env->active_fpu.fp_status);
>  }
>  
> +static inline void restore_snan_bit_mode(CPUMIPSState *env)
> +{
> +    set_snan_bit_is_one(((env->active_fpu.fcr31 & (1 << FCR31_NAN2008)) == 
> 0),

outermost parentheses can be removed

>      set_float_exception_flags(0, &env->active_fpu.fp_status);
> -    if ((GET_FP_ENABLE(env->active_fpu.fcr31) | 0x20) & 
> GET_FP_CAUSE(env->active_fpu.fcr31))
> +    if ((GET_FP_ENABLE(env->active_fpu.fcr31) | 0x20) &
> +         GET_FP_CAUSE(env->active_fpu.fcr31)) {
>          do_raise_exception(env, EXCP_FPE, GETPC());
> +    }

Unrelated cosmetic change, pelase remove it from this patch.

> @@ -465,6 +473,7 @@ static const mips_def_t mips_defs[] =
>                      (1 << FCR0_L) | (1 << FCR0_W) | (1 << FCR0_D) |
>                      (1 << FCR0_S) | (0x00 << FCR0_PRID) | (0x0 << FCR0_REV),
>          .CP1_fcr31 = (1 << FCR31_ABS2008) | (1 << FCR31_NAN2008),
> +        .CP1_fcr31_rw_bitmask = 0xFF83FFFF,
>          .SEGBITS = 32,
>          .PABITS = 32,
>          .insn_flags = CPU_MIPS32R6 | ASE_MICROMIPS,

> @@ -686,6 +705,7 @@ static const mips_def_t mips_defs[] =
>                      (1 << FCR0_L) | (1 << FCR0_W) | (1 << FCR0_D) |
>                      (1 << FCR0_S) | (0x00 << FCR0_PRID) | (0x0 << FCR0_REV),
>          .CP1_fcr31 = (1 << FCR31_ABS2008) | (1 << FCR31_NAN2008),
> +        .CP1_fcr31_rw_bitmask = 0xFF83FFFF,
>          .SEGBITS = 48,
>          .PABITS = 48,
>          .insn_flags = CPU_MIPS64R6 | ASE_MSA,

In MIPS R6 the floating point condition codes have been removed, consequently
bits 31:25 and 23 became read-only 0. The fcr31_rw_bitmask for
mips32r6-generic and mips64r6-generic cores should be 0x0103FFFF.

Otherwise the patch looks good to me.

Thanks,
Leon