qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] MIPS/user: Fix reset CPU state initialization
@ 2012-06-08  1:04 Maciej W. Rozycki
  2012-06-08 13:37 ` Meador Inge
  2012-09-07 23:58 ` Aurelien Jarno
  0 siblings, 2 replies; 6+ messages in thread
From: Maciej W. Rozycki @ 2012-06-08  1:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: Maciej W. Rozycki, Aurelien Jarno


 This change updates the CPU reset sequence to use a common piece of code 
that figures out CPU state flags, fixing the problem with MIPS_HFLAG_COP1X 
not being set where applicable that causes floating-point MADD family 
instructions (and other instructions from the MIPS IV FP subset) to trap.

 As compute_hflags is now shared between op_helper.c and translate.c, the 
function is now moved to a common header.  There are no changes to this 
function.

 The problem was seen with the 24Kf MIPS32r2 processor in user emulation.  
The new approach prevents system and user emulation from diverging -- all 
the hflags state is initialized in one place now.

Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
---

 This is effectively a follow-up to Nathan's FCR0 fix -- please apply.

  Maciej

qemu-mips-hflags.patch
Index: qemu-git-trunk/target-mips/cpu.h
===================================================================
--- qemu-git-trunk.orig/target-mips/cpu.h	2012-06-07 03:15:53.645461055 +0100
+++ qemu-git-trunk/target-mips/cpu.h	2012-06-07 03:18:48.345427587 +0100
@@ -753,4 +753,53 @@ static inline void cpu_pc_from_tb(CPUMIP
     env->hflags |= tb->flags & MIPS_HFLAG_BMASK;
 }
 
+static inline void compute_hflags(CPUMIPSState *env)
+{
+    env->hflags &= ~(MIPS_HFLAG_COP1X | MIPS_HFLAG_64 | MIPS_HFLAG_CP0 |
+                     MIPS_HFLAG_F64 | MIPS_HFLAG_FPU | MIPS_HFLAG_KSU |
+                     MIPS_HFLAG_UX);
+    if (!(env->CP0_Status & (1 << CP0St_EXL)) &&
+        !(env->CP0_Status & (1 << CP0St_ERL)) &&
+        !(env->hflags & MIPS_HFLAG_DM)) {
+        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))) {
+        env->hflags |= MIPS_HFLAG_64;
+    }
+    if (env->CP0_Status & (1 << CP0St_UX)) {
+        env->hflags |= MIPS_HFLAG_UX;
+    }
+#endif
+    if ((env->CP0_Status & (1 << CP0St_CU0)) ||
+        !(env->hflags & MIPS_HFLAG_KSU)) {
+        env->hflags |= MIPS_HFLAG_CP0;
+    }
+    if (env->CP0_Status & (1 << CP0St_CU1)) {
+        env->hflags |= MIPS_HFLAG_FPU;
+    }
+    if (env->CP0_Status & (1 << CP0St_FR)) {
+        env->hflags |= MIPS_HFLAG_F64;
+    }
+    if (env->insn_flags & ISA_MIPS32R2) {
+        if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
+            env->hflags |= MIPS_HFLAG_COP1X;
+        }
+    } else if (env->insn_flags & ISA_MIPS32) {
+        if (env->hflags & MIPS_HFLAG_64) {
+            env->hflags |= MIPS_HFLAG_COP1X;
+        }
+    } else if (env->insn_flags & ISA_MIPS4) {
+        /* All supported MIPS IV CPUs use the XX (CU3) to enable
+           and disable the MIPS IV extensions to the MIPS III ISA.
+           Some other MIPS IV CPUs ignore the bit, so the check here
+           would be too restrictive for them.  */
+        if (env->CP0_Status & (1 << CP0St_CU3)) {
+            env->hflags |= MIPS_HFLAG_COP1X;
+        }
+    }
+}
+
 #endif /* !defined (__MIPS_CPU_H__) */
Index: qemu-git-trunk/target-mips/op_helper.c
===================================================================
--- qemu-git-trunk.orig/target-mips/op_helper.c	2012-06-07 03:15:53.645461055 +0100
+++ qemu-git-trunk/target-mips/op_helper.c	2012-06-07 03:18:48.345427587 +0100
@@ -32,55 +32,6 @@
 static inline void cpu_mips_tlb_flush (CPUMIPSState *env, int flush_global);
 #endif
 
-static inline void compute_hflags(CPUMIPSState *env)
-{
-    env->hflags &= ~(MIPS_HFLAG_COP1X | MIPS_HFLAG_64 | MIPS_HFLAG_CP0 |
-                     MIPS_HFLAG_F64 | MIPS_HFLAG_FPU | MIPS_HFLAG_KSU |
-                     MIPS_HFLAG_UX);
-    if (!(env->CP0_Status & (1 << CP0St_EXL)) &&
-        !(env->CP0_Status & (1 << CP0St_ERL)) &&
-        !(env->hflags & MIPS_HFLAG_DM)) {
-        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))) {
-        env->hflags |= MIPS_HFLAG_64;
-    }
-    if (env->CP0_Status & (1 << CP0St_UX)) {
-        env->hflags |= MIPS_HFLAG_UX;
-    }
-#endif
-    if ((env->CP0_Status & (1 << CP0St_CU0)) ||
-        !(env->hflags & MIPS_HFLAG_KSU)) {
-        env->hflags |= MIPS_HFLAG_CP0;
-    }
-    if (env->CP0_Status & (1 << CP0St_CU1)) {
-        env->hflags |= MIPS_HFLAG_FPU;
-    }
-    if (env->CP0_Status & (1 << CP0St_FR)) {
-        env->hflags |= MIPS_HFLAG_F64;
-    }
-    if (env->insn_flags & ISA_MIPS32R2) {
-        if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
-            env->hflags |= MIPS_HFLAG_COP1X;
-        }
-    } else if (env->insn_flags & ISA_MIPS32) {
-        if (env->hflags & MIPS_HFLAG_64) {
-            env->hflags |= MIPS_HFLAG_COP1X;
-        }
-    } else if (env->insn_flags & ISA_MIPS4) {
-        /* All supported MIPS IV CPUs use the XX (CU3) to enable
-           and disable the MIPS IV extensions to the MIPS III ISA.
-           Some other MIPS IV CPUs ignore the bit, so the check here
-           would be too restrictive for them.  */
-        if (env->CP0_Status & (1 << CP0St_CU3)) {
-            env->hflags |= MIPS_HFLAG_COP1X;
-        }
-    }
-}
-
 /*****************************************************************************/
 /* Exceptions processing helpers */
 
Index: qemu-git-trunk/target-mips/translate.c
===================================================================
--- qemu-git-trunk.orig/target-mips/translate.c	2012-06-07 03:18:46.275645763 +0100
+++ qemu-git-trunk/target-mips/translate.c	2012-06-07 03:18:48.345427587 +0100
@@ -12780,17 +12780,12 @@ void cpu_state_reset(CPUMIPSState *env)
     env->insn_flags = env->cpu_model->insn_flags;
 
 #if defined(CONFIG_USER_ONLY)
-    env->hflags = MIPS_HFLAG_UM;
+    env->CP0_Status = (MIPS_HFLAG_UM << CP0St_KSU);
     /* Enable access to the SYNCI_Step register.  */
     env->CP0_HWREna |= (1 << 1);
     if (env->CP0_Config1 & (1 << CP0C1_FP)) {
-        env->hflags |= MIPS_HFLAG_FPU;
-    }
-#ifdef TARGET_MIPS64
-    if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
-        env->hflags |= MIPS_HFLAG_F64;
+        env->CP0_Status |= (1 << CP0St_CU1);
     }
-#endif
 #else
     if (env->hflags & MIPS_HFLAG_BMASK) {
         /* If the exception was raised from a delay slot,
@@ -12820,7 +12815,6 @@ void cpu_state_reset(CPUMIPSState *env)
     }
     /* Count register increments in debug mode, EJTAG version 1 */
     env->CP0_Debug = (1 << CP0DB_CNT) | (0x1 << CP0DB_VER);
-    env->hflags = MIPS_HFLAG_CP0;
 
     if (env->CP0_Config3 & (1 << CP0C3_MT)) {
         int i;
@@ -12848,11 +12842,7 @@ void cpu_state_reset(CPUMIPSState *env)
         }
     }
 #endif
-#if defined(TARGET_MIPS64)
-    if (env->cpu_model->insn_flags & ISA_MIPS3) {
-        env->hflags |= MIPS_HFLAG_64;
-    }
-#endif
+    compute_hflags(env);
     env->exception_index = EXCP_NONE;
 }
 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] MIPS/user: Fix reset CPU state initialization
  2012-06-08  1:04 [Qemu-devel] [PATCH] MIPS/user: Fix reset CPU state initialization Maciej W. Rozycki
@ 2012-06-08 13:37 ` Meador Inge
  2012-06-08 16:20   ` Maciej W. Rozycki
  2012-09-07 23:58 ` Aurelien Jarno
  1 sibling, 1 reply; 6+ messages in thread
From: Meador Inge @ 2012-06-08 13:37 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: qemu-devel, Aurelien Jarno, Andreas Färber

On 06/07/2012 08:04 PM, Maciej W. Rozycki wrote:

>  This change updates the CPU reset sequence to use a common piece of code 
> that figures out CPU state flags, fixing the problem with MIPS_HFLAG_COP1X 
> not being set where applicable that causes floating-point MADD family 
> instructions (and other instructions from the MIPS IV FP subset) to trap.
> 
>  As compute_hflags is now shared between op_helper.c and translate.c, the 
> function is now moved to a common header.  There are no changes to this 
> function.
> 
>  The problem was seen with the 24Kf MIPS32r2 processor in user emulation.  
> The new approach prevents system and user emulation from diverging -- all 
> the hflags state is initialized in one place now.

I submitted a patch to fix this issue and the FCR0 issue a few months back [1].
Andreas reviewed it, but the patch never got committed.

[1] http://patchwork.ozlabs.org/patch/144353/

> Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
> ---
> 
>  This is effectively a follow-up to Nathan's FCR0 fix -- please apply.
> 
>   Maciej
> 
> qemu-mips-hflags.patch
> Index: qemu-git-trunk/target-mips/cpu.h
> ===================================================================
> --- qemu-git-trunk.orig/target-mips/cpu.h	2012-06-07 03:15:53.645461055 +0100
> +++ qemu-git-trunk/target-mips/cpu.h	2012-06-07 03:18:48.345427587 +0100
> @@ -753,4 +753,53 @@ static inline void cpu_pc_from_tb(CPUMIP
>      env->hflags |= tb->flags & MIPS_HFLAG_BMASK;
>  }
>  
> +static inline void compute_hflags(CPUMIPSState *env)
> +{
> +    env->hflags &= ~(MIPS_HFLAG_COP1X | MIPS_HFLAG_64 | MIPS_HFLAG_CP0 |
> +                     MIPS_HFLAG_F64 | MIPS_HFLAG_FPU | MIPS_HFLAG_KSU |
> +                     MIPS_HFLAG_UX);
> +    if (!(env->CP0_Status & (1 << CP0St_EXL)) &&
> +        !(env->CP0_Status & (1 << CP0St_ERL)) &&
> +        !(env->hflags & MIPS_HFLAG_DM)) {
> +        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))) {
> +        env->hflags |= MIPS_HFLAG_64;
> +    }
> +    if (env->CP0_Status & (1 << CP0St_UX)) {
> +        env->hflags |= MIPS_HFLAG_UX;
> +    }
> +#endif
> +    if ((env->CP0_Status & (1 << CP0St_CU0)) ||
> +        !(env->hflags & MIPS_HFLAG_KSU)) {
> +        env->hflags |= MIPS_HFLAG_CP0;
> +    }
> +    if (env->CP0_Status & (1 << CP0St_CU1)) {
> +        env->hflags |= MIPS_HFLAG_FPU;
> +    }
> +    if (env->CP0_Status & (1 << CP0St_FR)) {
> +        env->hflags |= MIPS_HFLAG_F64;
> +    }
> +    if (env->insn_flags & ISA_MIPS32R2) {
> +        if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
> +            env->hflags |= MIPS_HFLAG_COP1X;
> +        }
> +    } else if (env->insn_flags & ISA_MIPS32) {
> +        if (env->hflags & MIPS_HFLAG_64) {
> +            env->hflags |= MIPS_HFLAG_COP1X;
> +        }
> +    } else if (env->insn_flags & ISA_MIPS4) {
> +        /* All supported MIPS IV CPUs use the XX (CU3) to enable
> +           and disable the MIPS IV extensions to the MIPS III ISA.
> +           Some other MIPS IV CPUs ignore the bit, so the check here
> +           would be too restrictive for them.  */
> +        if (env->CP0_Status & (1 << CP0St_CU3)) {
> +            env->hflags |= MIPS_HFLAG_COP1X;
> +        }
> +    }
> +}
> +
>  #endif /* !defined (__MIPS_CPU_H__) */
> Index: qemu-git-trunk/target-mips/op_helper.c
> ===================================================================
> --- qemu-git-trunk.orig/target-mips/op_helper.c	2012-06-07 03:15:53.645461055 +0100
> +++ qemu-git-trunk/target-mips/op_helper.c	2012-06-07 03:18:48.345427587 +0100
> @@ -32,55 +32,6 @@
>  static inline void cpu_mips_tlb_flush (CPUMIPSState *env, int flush_global);
>  #endif
>  
> -static inline void compute_hflags(CPUMIPSState *env)
> -{
> -    env->hflags &= ~(MIPS_HFLAG_COP1X | MIPS_HFLAG_64 | MIPS_HFLAG_CP0 |
> -                     MIPS_HFLAG_F64 | MIPS_HFLAG_FPU | MIPS_HFLAG_KSU |
> -                     MIPS_HFLAG_UX);
> -    if (!(env->CP0_Status & (1 << CP0St_EXL)) &&
> -        !(env->CP0_Status & (1 << CP0St_ERL)) &&
> -        !(env->hflags & MIPS_HFLAG_DM)) {
> -        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))) {
> -        env->hflags |= MIPS_HFLAG_64;
> -    }
> -    if (env->CP0_Status & (1 << CP0St_UX)) {
> -        env->hflags |= MIPS_HFLAG_UX;
> -    }
> -#endif
> -    if ((env->CP0_Status & (1 << CP0St_CU0)) ||
> -        !(env->hflags & MIPS_HFLAG_KSU)) {
> -        env->hflags |= MIPS_HFLAG_CP0;
> -    }
> -    if (env->CP0_Status & (1 << CP0St_CU1)) {
> -        env->hflags |= MIPS_HFLAG_FPU;
> -    }
> -    if (env->CP0_Status & (1 << CP0St_FR)) {
> -        env->hflags |= MIPS_HFLAG_F64;
> -    }
> -    if (env->insn_flags & ISA_MIPS32R2) {
> -        if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
> -            env->hflags |= MIPS_HFLAG_COP1X;
> -        }
> -    } else if (env->insn_flags & ISA_MIPS32) {
> -        if (env->hflags & MIPS_HFLAG_64) {
> -            env->hflags |= MIPS_HFLAG_COP1X;
> -        }
> -    } else if (env->insn_flags & ISA_MIPS4) {
> -        /* All supported MIPS IV CPUs use the XX (CU3) to enable
> -           and disable the MIPS IV extensions to the MIPS III ISA.
> -           Some other MIPS IV CPUs ignore the bit, so the check here
> -           would be too restrictive for them.  */
> -        if (env->CP0_Status & (1 << CP0St_CU3)) {
> -            env->hflags |= MIPS_HFLAG_COP1X;
> -        }
> -    }
> -}
> -
>  /*****************************************************************************/
>  /* Exceptions processing helpers */
>  
> Index: qemu-git-trunk/target-mips/translate.c
> ===================================================================
> --- qemu-git-trunk.orig/target-mips/translate.c	2012-06-07 03:18:46.275645763 +0100
> +++ qemu-git-trunk/target-mips/translate.c	2012-06-07 03:18:48.345427587 +0100
> @@ -12780,17 +12780,12 @@ void cpu_state_reset(CPUMIPSState *env)
>      env->insn_flags = env->cpu_model->insn_flags;
>  
>  #if defined(CONFIG_USER_ONLY)
> -    env->hflags = MIPS_HFLAG_UM;
> +    env->CP0_Status = (MIPS_HFLAG_UM << CP0St_KSU);
>      /* Enable access to the SYNCI_Step register.  */
>      env->CP0_HWREna |= (1 << 1);
>      if (env->CP0_Config1 & (1 << CP0C1_FP)) {
> -        env->hflags |= MIPS_HFLAG_FPU;
> -    }
> -#ifdef TARGET_MIPS64
> -    if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
> -        env->hflags |= MIPS_HFLAG_F64;
> +        env->CP0_Status |= (1 << CP0St_CU1);
>      }
> -#endif
>  #else
>      if (env->hflags & MIPS_HFLAG_BMASK) {
>          /* If the exception was raised from a delay slot,
> @@ -12820,7 +12815,6 @@ void cpu_state_reset(CPUMIPSState *env)
>      }
>      /* Count register increments in debug mode, EJTAG version 1 */
>      env->CP0_Debug = (1 << CP0DB_CNT) | (0x1 << CP0DB_VER);
> -    env->hflags = MIPS_HFLAG_CP0;
>  
>      if (env->CP0_Config3 & (1 << CP0C3_MT)) {
>          int i;
> @@ -12848,11 +12842,7 @@ void cpu_state_reset(CPUMIPSState *env)
>          }
>      }
>  #endif
> -#if defined(TARGET_MIPS64)
> -    if (env->cpu_model->insn_flags & ISA_MIPS3) {
> -        env->hflags |= MIPS_HFLAG_64;
> -    }
> -#endif
> +    compute_hflags(env);
>      env->exception_index = EXCP_NONE;
>  }
>  
> 


-- 
Meador Inge
CodeSourcery / Mentor Embedded
http://www.mentor.com/embedded-software

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] MIPS/user: Fix reset CPU state initialization
  2012-06-08 13:37 ` Meador Inge
@ 2012-06-08 16:20   ` Maciej W. Rozycki
  2012-06-08 17:22     ` Andreas Färber
  0 siblings, 1 reply; 6+ messages in thread
From: Maciej W. Rozycki @ 2012-06-08 16:20 UTC (permalink / raw)
  To: Meador Inge; +Cc: qemu-devel, Aurelien Jarno, Andreas Färber

On Fri, 8 Jun 2012, Meador Inge wrote:

> >  The problem was seen with the 24Kf MIPS32r2 processor in user emulation.  
> > The new approach prevents system and user emulation from diverging -- all 
> > the hflags state is initialized in one place now.
> 
> I submitted a patch to fix this issue and the FCR0 issue a few months back [1].
> Andreas reviewed it, but the patch never got committed.
> 
> [1] http://patchwork.ozlabs.org/patch/144353/

 That's unfortunate -- hopefully this can be finally resolved now.

  Maciej

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] MIPS/user: Fix reset CPU state initialization
  2012-06-08 16:20   ` Maciej W. Rozycki
@ 2012-06-08 17:22     ` Andreas Färber
  2012-06-08 18:51       ` Maciej W. Rozycki
  0 siblings, 1 reply; 6+ messages in thread
From: Andreas Färber @ 2012-06-08 17:22 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: Meador Inge, qemu-devel, Aurelien Jarno

Am 08.06.2012 18:20, schrieb Maciej W. Rozycki:
> On Fri, 8 Jun 2012, Meador Inge wrote:
> 
>>>  The problem was seen with the 24Kf MIPS32r2 processor in user emulation.  
>>> The new approach prevents system and user emulation from diverging -- all 
>>> the hflags state is initialized in one place now.
>>
>> I submitted a patch to fix this issue and the FCR0 issue a few months back [1].
>> Andreas reviewed it, but the patch never got committed.
>>
>> [1] http://patchwork.ozlabs.org/patch/144353/

I fear that patch would need to be rebased now, since my cross-target
QOM refactoring was applied for v1.1.

>  That's unfortunate -- hopefully this can be finally resolved now.

The best solution would be if someone stepped up as target-mips
maintainer as long as Aurélien is busy, to review and queue such
patches. m68k, mips and sh4 keep falling through the cracks currently...

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] MIPS/user: Fix reset CPU state initialization
  2012-06-08 17:22     ` Andreas Färber
@ 2012-06-08 18:51       ` Maciej W. Rozycki
  0 siblings, 0 replies; 6+ messages in thread
From: Maciej W. Rozycki @ 2012-06-08 18:51 UTC (permalink / raw)
  To: Andreas Färber; +Cc: Meador Inge, qemu-devel, Aurelien Jarno

On Fri, 8 Jun 2012, Andreas Färber wrote:

> >>>  The problem was seen with the 24Kf MIPS32r2 processor in user emulation.  
> >>> The new approach prevents system and user emulation from diverging -- all 
> >>> the hflags state is initialized in one place now.
> >>
> >> I submitted a patch to fix this issue and the FCR0 issue a few months back [1].
> >> Andreas reviewed it, but the patch never got committed.
> >>
> >> [1] http://patchwork.ozlabs.org/patch/144353/
> 
> I fear that patch would need to be rebased now, since my cross-target
> QOM refactoring was applied for v1.1.

 Well, the versions I have just sent are against current master, so it 
might be easier to integrate them instead.

> >  That's unfortunate -- hopefully this can be finally resolved now.
> 
> The best solution would be if someone stepped up as target-mips
> maintainer as long as Aurélien is busy, to review and queue such
> patches. m68k, mips and sh4 keep falling through the cracks currently...

 I can't afford that, sorry; besides my knowledge of QEMU internals is 
probably still too limited at this stage.

  Maciej

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] MIPS/user: Fix reset CPU state initialization
  2012-06-08  1:04 [Qemu-devel] [PATCH] MIPS/user: Fix reset CPU state initialization Maciej W. Rozycki
  2012-06-08 13:37 ` Meador Inge
@ 2012-09-07 23:58 ` Aurelien Jarno
  1 sibling, 0 replies; 6+ messages in thread
From: Aurelien Jarno @ 2012-09-07 23:58 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: qemu-devel

On Fri, Jun 08, 2012 at 02:04:40AM +0100, Maciej W. Rozycki wrote:
> 
>  This change updates the CPU reset sequence to use a common piece of code 
> that figures out CPU state flags, fixing the problem with MIPS_HFLAG_COP1X 
> not being set where applicable that causes floating-point MADD family 
> instructions (and other instructions from the MIPS IV FP subset) to trap.
> 
>  As compute_hflags is now shared between op_helper.c and translate.c, the 
> function is now moved to a common header.  There are no changes to this 
> function.
> 
>  The problem was seen with the 24Kf MIPS32r2 processor in user emulation.  
> The new approach prevents system and user emulation from diverging -- all 
> the hflags state is initialized in one place now.
> 
> Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
> ---
> 
>  This is effectively a follow-up to Nathan's FCR0 fix -- please apply.
> 
>   Maciej

Thanks, applied.

> qemu-mips-hflags.patch
> Index: qemu-git-trunk/target-mips/cpu.h
> ===================================================================
> --- qemu-git-trunk.orig/target-mips/cpu.h	2012-06-07 03:15:53.645461055 +0100
> +++ qemu-git-trunk/target-mips/cpu.h	2012-06-07 03:18:48.345427587 +0100
> @@ -753,4 +753,53 @@ static inline void cpu_pc_from_tb(CPUMIP
>      env->hflags |= tb->flags & MIPS_HFLAG_BMASK;
>  }
>  
> +static inline void compute_hflags(CPUMIPSState *env)
> +{
> +    env->hflags &= ~(MIPS_HFLAG_COP1X | MIPS_HFLAG_64 | MIPS_HFLAG_CP0 |
> +                     MIPS_HFLAG_F64 | MIPS_HFLAG_FPU | MIPS_HFLAG_KSU |
> +                     MIPS_HFLAG_UX);
> +    if (!(env->CP0_Status & (1 << CP0St_EXL)) &&
> +        !(env->CP0_Status & (1 << CP0St_ERL)) &&
> +        !(env->hflags & MIPS_HFLAG_DM)) {
> +        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))) {
> +        env->hflags |= MIPS_HFLAG_64;
> +    }
> +    if (env->CP0_Status & (1 << CP0St_UX)) {
> +        env->hflags |= MIPS_HFLAG_UX;
> +    }
> +#endif
> +    if ((env->CP0_Status & (1 << CP0St_CU0)) ||
> +        !(env->hflags & MIPS_HFLAG_KSU)) {
> +        env->hflags |= MIPS_HFLAG_CP0;
> +    }
> +    if (env->CP0_Status & (1 << CP0St_CU1)) {
> +        env->hflags |= MIPS_HFLAG_FPU;
> +    }
> +    if (env->CP0_Status & (1 << CP0St_FR)) {
> +        env->hflags |= MIPS_HFLAG_F64;
> +    }
> +    if (env->insn_flags & ISA_MIPS32R2) {
> +        if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
> +            env->hflags |= MIPS_HFLAG_COP1X;
> +        }
> +    } else if (env->insn_flags & ISA_MIPS32) {
> +        if (env->hflags & MIPS_HFLAG_64) {
> +            env->hflags |= MIPS_HFLAG_COP1X;
> +        }
> +    } else if (env->insn_flags & ISA_MIPS4) {
> +        /* All supported MIPS IV CPUs use the XX (CU3) to enable
> +           and disable the MIPS IV extensions to the MIPS III ISA.
> +           Some other MIPS IV CPUs ignore the bit, so the check here
> +           would be too restrictive for them.  */
> +        if (env->CP0_Status & (1 << CP0St_CU3)) {
> +            env->hflags |= MIPS_HFLAG_COP1X;
> +        }
> +    }
> +}
> +
>  #endif /* !defined (__MIPS_CPU_H__) */
> Index: qemu-git-trunk/target-mips/op_helper.c
> ===================================================================
> --- qemu-git-trunk.orig/target-mips/op_helper.c	2012-06-07 03:15:53.645461055 +0100
> +++ qemu-git-trunk/target-mips/op_helper.c	2012-06-07 03:18:48.345427587 +0100
> @@ -32,55 +32,6 @@
>  static inline void cpu_mips_tlb_flush (CPUMIPSState *env, int flush_global);
>  #endif
>  
> -static inline void compute_hflags(CPUMIPSState *env)
> -{
> -    env->hflags &= ~(MIPS_HFLAG_COP1X | MIPS_HFLAG_64 | MIPS_HFLAG_CP0 |
> -                     MIPS_HFLAG_F64 | MIPS_HFLAG_FPU | MIPS_HFLAG_KSU |
> -                     MIPS_HFLAG_UX);
> -    if (!(env->CP0_Status & (1 << CP0St_EXL)) &&
> -        !(env->CP0_Status & (1 << CP0St_ERL)) &&
> -        !(env->hflags & MIPS_HFLAG_DM)) {
> -        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))) {
> -        env->hflags |= MIPS_HFLAG_64;
> -    }
> -    if (env->CP0_Status & (1 << CP0St_UX)) {
> -        env->hflags |= MIPS_HFLAG_UX;
> -    }
> -#endif
> -    if ((env->CP0_Status & (1 << CP0St_CU0)) ||
> -        !(env->hflags & MIPS_HFLAG_KSU)) {
> -        env->hflags |= MIPS_HFLAG_CP0;
> -    }
> -    if (env->CP0_Status & (1 << CP0St_CU1)) {
> -        env->hflags |= MIPS_HFLAG_FPU;
> -    }
> -    if (env->CP0_Status & (1 << CP0St_FR)) {
> -        env->hflags |= MIPS_HFLAG_F64;
> -    }
> -    if (env->insn_flags & ISA_MIPS32R2) {
> -        if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
> -            env->hflags |= MIPS_HFLAG_COP1X;
> -        }
> -    } else if (env->insn_flags & ISA_MIPS32) {
> -        if (env->hflags & MIPS_HFLAG_64) {
> -            env->hflags |= MIPS_HFLAG_COP1X;
> -        }
> -    } else if (env->insn_flags & ISA_MIPS4) {
> -        /* All supported MIPS IV CPUs use the XX (CU3) to enable
> -           and disable the MIPS IV extensions to the MIPS III ISA.
> -           Some other MIPS IV CPUs ignore the bit, so the check here
> -           would be too restrictive for them.  */
> -        if (env->CP0_Status & (1 << CP0St_CU3)) {
> -            env->hflags |= MIPS_HFLAG_COP1X;
> -        }
> -    }
> -}
> -
>  /*****************************************************************************/
>  /* Exceptions processing helpers */
>  
> Index: qemu-git-trunk/target-mips/translate.c
> ===================================================================
> --- qemu-git-trunk.orig/target-mips/translate.c	2012-06-07 03:18:46.275645763 +0100
> +++ qemu-git-trunk/target-mips/translate.c	2012-06-07 03:18:48.345427587 +0100
> @@ -12780,17 +12780,12 @@ void cpu_state_reset(CPUMIPSState *env)
>      env->insn_flags = env->cpu_model->insn_flags;
>  
>  #if defined(CONFIG_USER_ONLY)
> -    env->hflags = MIPS_HFLAG_UM;
> +    env->CP0_Status = (MIPS_HFLAG_UM << CP0St_KSU);
>      /* Enable access to the SYNCI_Step register.  */
>      env->CP0_HWREna |= (1 << 1);
>      if (env->CP0_Config1 & (1 << CP0C1_FP)) {
> -        env->hflags |= MIPS_HFLAG_FPU;
> -    }
> -#ifdef TARGET_MIPS64
> -    if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
> -        env->hflags |= MIPS_HFLAG_F64;
> +        env->CP0_Status |= (1 << CP0St_CU1);
>      }
> -#endif
>  #else
>      if (env->hflags & MIPS_HFLAG_BMASK) {
>          /* If the exception was raised from a delay slot,
> @@ -12820,7 +12815,6 @@ void cpu_state_reset(CPUMIPSState *env)
>      }
>      /* Count register increments in debug mode, EJTAG version 1 */
>      env->CP0_Debug = (1 << CP0DB_CNT) | (0x1 << CP0DB_VER);
> -    env->hflags = MIPS_HFLAG_CP0;
>  
>      if (env->CP0_Config3 & (1 << CP0C3_MT)) {
>          int i;
> @@ -12848,11 +12842,7 @@ void cpu_state_reset(CPUMIPSState *env)
>          }
>      }
>  #endif
> -#if defined(TARGET_MIPS64)
> -    if (env->cpu_model->insn_flags & ISA_MIPS3) {
> -        env->hflags |= MIPS_HFLAG_64;
> -    }
> -#endif
> +    compute_hflags(env);
>      env->exception_index = EXCP_NONE;
>  }
>  
> 
> 

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-09-07 23:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-08  1:04 [Qemu-devel] [PATCH] MIPS/user: Fix reset CPU state initialization Maciej W. Rozycki
2012-06-08 13:37 ` Meador Inge
2012-06-08 16:20   ` Maciej W. Rozycki
2012-06-08 17:22     ` Andreas Färber
2012-06-08 18:51       ` Maciej W. Rozycki
2012-09-07 23:58 ` Aurelien Jarno

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).