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

2016-06-06 Thread 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 = 0xFF83,
>  .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 = 0xFF83,
>  .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 0x0103.

Otherwise the patch looks good to me.

Thanks,
Leon



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

2016-06-03 Thread Aleksandar Markovic
From: Aleksandar Markovic 

This patch implements read and write access rules for Mips floating
point control and status register (FCR31). The change can be divided
into following parts:

- Add fields that will keep FCR31's R/W bitmask in procesor
  definitions and processor float_status structure.

- Add appropriate value for FCR31's R/W bitmask for each supported
  processor.

- Add preprocessor definition of FCR31's FS bit, and update
  related code for setting this bit.

- Add function for setting snan_bit_is_one, and integrate it in
  appropriate places.

- Modify handling of CTC1 (case 31) instruction to use FCR31's R/W
  bitmask.

- Modify handling user mode executables for Mips, in relation to the
  bit EF_MIPS_NAN2008 from ELF header, that is in turn related to
  reading and writing to FCR31.

- Modify gdb behavior in relation to FCR31.

Signed-off-by: Thomas Schwinge 
Signed-off-by: Maciej W. Rozycki 
Signed-off-by: Aleksandar Markovic 
---
 linux-user/main.c| 14 ++
 target-mips/cpu.h| 15 ---
 target-mips/gdbstub.c|  8 +++-
 target-mips/op_helper.c  | 18 ++
 target-mips/translate.c  |  5 ++---
 target-mips/translate_init.c | 26 ++
 6 files changed, 63 insertions(+), 23 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 5f3ec97..cc21057 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -4608,6 +4608,20 @@ int main(int argc, char **argv, char **envp)
 if (regs->cp0_epc & 1) {
 env->hflags |= MIPS_HFLAG_M16;
 }
+if (((info->elf_flags & EF_MIPS_NAN2008) != 0) !=
+((env->active_fpu.fcr31 & (1 << FCR31_NAN2008)) != 0)) {
+if ((env->active_fpu.fcr31_rw_bitmask &
+  (1 << FCR31_NAN2008)) == 0) {
+fprintf(stderr, "ELF binary's NaN mode not supported by 
CPU\n");
+exit(1);
+}
+if ((info->elf_flags & EF_MIPS_NAN2008) != 0) {
+env->active_fpu.fcr31 |= (1 << FCR31_NAN2008);
+} else {
+env->active_fpu.fcr31 &= ~(1 << FCR31_NAN2008);
+}
+restore_snan_bit_mode(env);
+}
 }
 #elif defined(TARGET_OPENRISC)
 {
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 6478420..f71bb94 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -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
+#define FCR31_FS  24
 #define SET_FP_COND(num,env) do { ((env).fcr31) |= ((num) ? (1 << ((num) + 
24)) : (1 << 23)); } while(0)
 #define CLEAR_FP_COND(num,env)   do { ((env).fcr31) &= ~((num) ? (1 << ((num) 
+ 24)) : (1 << 23)); } while(0)
 #define GET_FP_COND(env) env).fcr31 >> 24) & 0xfe) | (((env).fcr31 
>> 23) & 0x1))
@@ -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,
   &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),
+&env->active_fpu.fp_status);
+}
+
 static inline void restore_fp_status(CPUMIPSState *env)
 {
 restore_rounding_mode(env);
 restore_flush_mode(env);
+restore_snan_bit_mode(env);
 }
 
 static inline void restore_msa_fp_status(CPUMIPSState *env)
diff --git a/target-mips/gdbstub.c b/target-mips/gdbstub.c
index b0b4a32..aacc721 100644
--- a/target-mips/gdbstub.c
+++ b/target-mips/gdbstub.c
@@ -89,11 +89,9 @@ int mips_cpu_gdb_write_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 if (env->CP0_Config1 & (1 << CP0C1_FP) && n >= 38 && n < 72) {
 switch (n) {
 case 70:
-env->active_fpu.fcr31 = tmp & 0xFF83;
-/* set rounding mode */
-restore_rounding_mode(env);
-/* set flush-to-zero mode */
-restore_flush_mode(env);
+env->active_fpu.fcr31 = (tmp & env->active_fpu.fcr31_rw_bitmask) |
+  (env->active_fpu.fcr31 & 
~(env->active_fpu.fcr31_rw_bitmask));
+restore_fp_status(env);
 break;
 case 71:
 /* FIR is read-only.  Ignore writes.  */
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 0d1e959..2d1d809 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -2576,24 +2576,18 @@ void helper_ctc1(CPUMIPSState *env, target_ulong arg1, 
uint32_t fs, uint32_t rt)
  ((arg1 & 0x4) << 22);
 break;
 case 31:
-if (env->i