[Qemu-devel] SH4: Implement FD bit

[Qemu-devel] SH4: Implement FD bit

[Vladimir Prus]

[-- Attachment #1: Type: text/plain, Size: 670 bytes --]


SH4 manual say that if a floating point instruction is executed while
FD bit in the status register is 1, an exception should be raised. QEMU
presently does not do that, so the kernel does not initialize FP state
for any thread, nor does it save/restore FP state. The most apparent
consequence is that while recent gcc/libc expect double-precision mode
to be set by kernel, they run in single-precision mode, and all FP code
produces wrong values.

This patch fixes this. It also fixes a couple of places where PC was
not updated before handling an exception, although both those places
deal with invalid instruction and don't lead to any user-visible bugs.

- Volodya

[-- Attachment #2: fp.diff --]
[-- Type: text/x-diff, Size: 5828 bytes --]

commit 0cbaa15c2db2168937fa154d5f1f625497392552
Author: Vladimir Prus <vladimir@codesourcery.com>
Date:   Mon Dec 1 19:28:32 2008 +0300

    SH: Implement FD bit
    
    	* target-sh4/cpu.h (cpu_get_tb_cpu_state): Include SR's FD bit in
    	the flags.
    	* target-sh4/helper.h (raise_fpu_disable, raise_slot_fpu_disable):
    	New helpers.
    	* targets-sh4/op_helper.c (helper_raise_fpu_disable)
    	(helper_raise_slot_fpu_disable): New.
    	* target-sh4/translate.c (CHECK_NOT_DELAY_SLOT, CHECK_PRIVILEGED):
    	Set PC to the right value.
    	(CHECK_FPU_ENABLED): New.
    	(_decode_opc): Use CHECK_FPU_ENABLED for FP instructions.

diff --git a/target-sh4/cpu.h b/target-sh4/cpu.h
index aae9b42..e6c658e 100644
--- a/target-sh4/cpu.h
+++ b/target-sh4/cpu.h
@@ -294,6 +294,7 @@ static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
                     | DELAY_SLOT_TRUE | DELAY_SLOT_CLEARME))     /* Bits  0- 3 */
             | (env->fpscr & (FPSCR_FR | FPSCR_SZ | FPSCR_PR))    /* Bits 19-21 */
             | (env->sr & (SR_MD | SR_RB))                        /* Bits 29-30 */
+            | (env->sr & SR_FD)                                  /* Bit 15 */
 	    | (env->store_requests ? TB_FLAG_PENDING_MOVCA : 0); /* Bit 4 */
 }
 
diff --git a/target-sh4/helper.h b/target-sh4/helper.h
index c3f6393..d995688 100644
--- a/target-sh4/helper.h
+++ b/target-sh4/helper.h
@@ -3,6 +3,8 @@
 DEF_HELPER_0(ldtlb, void)
 DEF_HELPER_0(raise_illegal_instruction, void)
 DEF_HELPER_0(raise_slot_illegal_instruction, void)
+DEF_HELPER_0(raise_fpu_disable, void)
+DEF_HELPER_0(raise_slot_fpu_disable, void)
 DEF_HELPER_0(debug, void)
 DEF_HELPER_1(sleep, void, i32)
 DEF_HELPER_1(trapa, void, i32)
diff --git a/target-sh4/op_helper.c b/target-sh4/op_helper.c
index c18d67c..b4982b0 100644
--- a/target-sh4/op_helper.c
+++ b/target-sh4/op_helper.c
@@ -89,6 +89,18 @@ void helper_raise_slot_illegal_instruction(void)
     cpu_loop_exit();
 }
 
+void helper_raise_fpu_disable(void)
+{
+  env->exception_index = 0x800;
+  cpu_loop_exit();
+}
+
+void helper_raise_slot_fpu_disable(void)
+{
+  env->exception_index = 0x820;
+  cpu_loop_exit();
+}
+
 void helper_debug(void)
 {
     env->exception_index = EXCP_DEBUG;
diff --git a/target-sh4/translate.c b/target-sh4/translate.c
index 80c0f30..d11a738 100644
--- a/target-sh4/translate.c
+++ b/target-sh4/translate.c
@@ -457,17 +457,36 @@ static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
 #define DREG(x) FREG(x) /* Assumes lsb of (x) is always 0 */
 
 #define CHECK_NOT_DELAY_SLOT \
-  if (ctx->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) \
-  {gen_helper_raise_slot_illegal_instruction(); ctx->bstate = BS_EXCP; \
-   return;}
+  if (ctx->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL))     \
+  {                                                           \
+      tcg_gen_movi_i32(cpu_pc, ctx->pc-2);		      \
+      gen_helper_raise_slot_illegal_instruction();            \
+      ctx->bstate = BS_EXCP;	                              \
+      return;                                                 \
+  }
 
 #define CHECK_PRIVILEGED                                      \
   if (IS_USER(ctx)) {                                         \
+      tcg_gen_movi_i32(cpu_pc, ctx->pc);                      \
       gen_helper_raise_illegal_instruction();                 \
       ctx->bstate = BS_EXCP;                                  \
       return;                                                 \
   }
 
+#define CHECK_FPU_ENABLED                                                \
+  if (ctx->flags & SR_FD)		                                 \
+      {                                                                  \
+         if (ctx->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) { 	 \
+            tcg_gen_movi_i32(cpu_pc, ctx->pc-2);                         \
+            gen_helper_raise_slot_fpu_disable();                         \
+	 } else {							 \
+            tcg_gen_movi_i32(cpu_pc, ctx->pc);                           \
+	    gen_helper_raise_fpu_disable();                              \
+	 }                                                               \
+	ctx->bstate = BS_EXCP;                                           \
+        return;                                                          \
+      }                                                        
+  
 static void _decode_opc(DisasContext * ctx)
 {
     /* This code tries to make movcal emulation sufficiently
@@ -504,6 +523,13 @@ static void _decode_opc(DisasContext * ctx)
 	      }
 	}
 
+    /* The 0xfffd instruction is underfined, so we don't want to
+       raise fpu disable exception on it.  */
+    if (((ctx->opcode & 0xf000) == 0xf000)
+	&& (ctx->opcode != 0xfffd))      
+      {
+	CHECK_FPU_ENABLED
+      }
 #if 0
     fprintf(stderr, "Translating opcode 0x%04x\n", ctx->opcode);
 #endif
@@ -1498,12 +1524,14 @@ static void _decode_opc(DisasContext * ctx)
 	LDST(mach, 0x400a, 0x4006, 0x000a, 0x4002, {})
 	LDST(macl, 0x401a, 0x4016, 0x001a, 0x4012, {})
 	LDST(pr,   0x402a, 0x4026, 0x002a, 0x4022, {})
-	LDST(fpul, 0x405a, 0x4056, 0x005a, 0x4052, {})
+	LDST(fpul, 0x405a, 0x4056, 0x005a, 0x4052, {CHECK_FPU_ENABLED})
     case 0x406a:		/* lds Rm,FPSCR */
+	CHECK_FPU_ENABLED
 	gen_helper_ld_fpscr(REG(B11_8));
 	ctx->bstate = BS_STOP;
 	return;
     case 0x4066:		/* lds.l @Rm+,FPSCR */
+        CHECK_FPU_ENABLED
 	{
 	    TCGv addr = tcg_temp_new();
 	    tcg_gen_qemu_ld32s(addr, REG(B11_8), ctx->memidx);
@@ -1514,9 +1542,11 @@ static void _decode_opc(DisasContext * ctx)
 	}
 	return;
     case 0x006a:		/* sts FPSCR,Rn */
+        CHECK_FPU_ENABLED
 	tcg_gen_andi_i32(REG(B11_8), cpu_fpscr, 0x003fffff);
 	return;
     case 0x4062:		/* sts FPSCR,@-Rn */
+        CHECK_FPU_ENABLED
 	{
 	    TCGv addr, val;
 	    val = tcg_temp_new();

Re: [Qemu-devel] SH4: Implement FD bit

[Aurelien Jarno]

On Mon, Dec 01, 2008 at 08:22:02PM +0300, Vladimir Prus wrote:
> 
> SH4 manual say that if a floating point instruction is executed while
> FD bit in the status register is 1, an exception should be raised. QEMU
> presently does not do that, so the kernel does not initialize FP state
> for any thread, nor does it save/restore FP state. The most apparent
> consequence is that while recent gcc/libc expect double-precision mode
> to be set by kernel, they run in single-precision mode, and all FP code
> produces wrong values.
> 
> This patch fixes this. It also fixes a couple of places where PC was
> not updated before handling an exception, although both those places
> deal with invalid instruction and don't lead to any user-visible bugs.
> 
> - Volodya

Thanks, applied.

> commit 0cbaa15c2db2168937fa154d5f1f625497392552
> Author: Vladimir Prus <vladimir@codesourcery.com>
> Date:   Mon Dec 1 19:28:32 2008 +0300
> 
>     SH: Implement FD bit
>     
>     	* target-sh4/cpu.h (cpu_get_tb_cpu_state): Include SR's FD bit in
>     	the flags.
>     	* target-sh4/helper.h (raise_fpu_disable, raise_slot_fpu_disable):
>     	New helpers.
>     	* targets-sh4/op_helper.c (helper_raise_fpu_disable)
>     	(helper_raise_slot_fpu_disable): New.
>     	* target-sh4/translate.c (CHECK_NOT_DELAY_SLOT, CHECK_PRIVILEGED):
>     	Set PC to the right value.
>     	(CHECK_FPU_ENABLED): New.
>     	(_decode_opc): Use CHECK_FPU_ENABLED for FP instructions.
> 
> diff --git a/target-sh4/cpu.h b/target-sh4/cpu.h
> index aae9b42..e6c658e 100644
> --- a/target-sh4/cpu.h
> +++ b/target-sh4/cpu.h
> @@ -294,6 +294,7 @@ static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
>                      | DELAY_SLOT_TRUE | DELAY_SLOT_CLEARME))     /* Bits  0- 3 */
>              | (env->fpscr & (FPSCR_FR | FPSCR_SZ | FPSCR_PR))    /* Bits 19-21 */
>              | (env->sr & (SR_MD | SR_RB))                        /* Bits 29-30 */
> +            | (env->sr & SR_FD)                                  /* Bit 15 */
>  	    | (env->store_requests ? TB_FLAG_PENDING_MOVCA : 0); /* Bit 4 */
>  }
>  
> diff --git a/target-sh4/helper.h b/target-sh4/helper.h
> index c3f6393..d995688 100644
> --- a/target-sh4/helper.h
> +++ b/target-sh4/helper.h
> @@ -3,6 +3,8 @@
>  DEF_HELPER_0(ldtlb, void)
>  DEF_HELPER_0(raise_illegal_instruction, void)
>  DEF_HELPER_0(raise_slot_illegal_instruction, void)
> +DEF_HELPER_0(raise_fpu_disable, void)
> +DEF_HELPER_0(raise_slot_fpu_disable, void)
>  DEF_HELPER_0(debug, void)
>  DEF_HELPER_1(sleep, void, i32)
>  DEF_HELPER_1(trapa, void, i32)
> diff --git a/target-sh4/op_helper.c b/target-sh4/op_helper.c
> index c18d67c..b4982b0 100644
> --- a/target-sh4/op_helper.c
> +++ b/target-sh4/op_helper.c
> @@ -89,6 +89,18 @@ void helper_raise_slot_illegal_instruction(void)
>      cpu_loop_exit();
>  }
>  
> +void helper_raise_fpu_disable(void)
> +{
> +  env->exception_index = 0x800;
> +  cpu_loop_exit();
> +}
> +
> +void helper_raise_slot_fpu_disable(void)
> +{
> +  env->exception_index = 0x820;
> +  cpu_loop_exit();
> +}
> +
>  void helper_debug(void)
>  {
>      env->exception_index = EXCP_DEBUG;
> diff --git a/target-sh4/translate.c b/target-sh4/translate.c
> index 80c0f30..d11a738 100644
> --- a/target-sh4/translate.c
> +++ b/target-sh4/translate.c
> @@ -457,17 +457,36 @@ static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
>  #define DREG(x) FREG(x) /* Assumes lsb of (x) is always 0 */
>  
>  #define CHECK_NOT_DELAY_SLOT \
> -  if (ctx->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) \
> -  {gen_helper_raise_slot_illegal_instruction(); ctx->bstate = BS_EXCP; \
> -   return;}
> +  if (ctx->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL))     \
> +  {                                                           \
> +      tcg_gen_movi_i32(cpu_pc, ctx->pc-2);		      \
> +      gen_helper_raise_slot_illegal_instruction();            \
> +      ctx->bstate = BS_EXCP;	                              \
> +      return;                                                 \
> +  }
>  
>  #define CHECK_PRIVILEGED                                      \
>    if (IS_USER(ctx)) {                                         \
> +      tcg_gen_movi_i32(cpu_pc, ctx->pc);                      \
>        gen_helper_raise_illegal_instruction();                 \
>        ctx->bstate = BS_EXCP;                                  \
>        return;                                                 \
>    }
>  
> +#define CHECK_FPU_ENABLED                                                \
> +  if (ctx->flags & SR_FD)		                                 \
> +      {                                                                  \
> +         if (ctx->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) { 	 \
> +            tcg_gen_movi_i32(cpu_pc, ctx->pc-2);                         \
> +            gen_helper_raise_slot_fpu_disable();                         \
> +	 } else {							 \
> +            tcg_gen_movi_i32(cpu_pc, ctx->pc);                           \
> +	    gen_helper_raise_fpu_disable();                              \
> +	 }                                                               \
> +	ctx->bstate = BS_EXCP;                                           \
> +        return;                                                          \
> +      }                                                        
> +  
>  static void _decode_opc(DisasContext * ctx)
>  {
>      /* This code tries to make movcal emulation sufficiently
> @@ -504,6 +523,13 @@ static void _decode_opc(DisasContext * ctx)
>  	      }
>  	}
>  
> +    /* The 0xfffd instruction is underfined, so we don't want to
> +       raise fpu disable exception on it.  */
> +    if (((ctx->opcode & 0xf000) == 0xf000)
> +	&& (ctx->opcode != 0xfffd))      
> +      {
> +	CHECK_FPU_ENABLED
> +      }
>  #if 0
>      fprintf(stderr, "Translating opcode 0x%04x\n", ctx->opcode);
>  #endif
> @@ -1498,12 +1524,14 @@ static void _decode_opc(DisasContext * ctx)
>  	LDST(mach, 0x400a, 0x4006, 0x000a, 0x4002, {})
>  	LDST(macl, 0x401a, 0x4016, 0x001a, 0x4012, {})
>  	LDST(pr,   0x402a, 0x4026, 0x002a, 0x4022, {})
> -	LDST(fpul, 0x405a, 0x4056, 0x005a, 0x4052, {})
> +	LDST(fpul, 0x405a, 0x4056, 0x005a, 0x4052, {CHECK_FPU_ENABLED})
>      case 0x406a:		/* lds Rm,FPSCR */
> +	CHECK_FPU_ENABLED
>  	gen_helper_ld_fpscr(REG(B11_8));
>  	ctx->bstate = BS_STOP;
>  	return;
>      case 0x4066:		/* lds.l @Rm+,FPSCR */
> +        CHECK_FPU_ENABLED
>  	{
>  	    TCGv addr = tcg_temp_new();
>  	    tcg_gen_qemu_ld32s(addr, REG(B11_8), ctx->memidx);
> @@ -1514,9 +1542,11 @@ static void _decode_opc(DisasContext * ctx)
>  	}
>  	return;
>      case 0x006a:		/* sts FPSCR,Rn */
> +        CHECK_FPU_ENABLED
>  	tcg_gen_andi_i32(REG(B11_8), cpu_fpscr, 0x003fffff);
>  	return;
>      case 0x4062:		/* sts FPSCR,@-Rn */
> +        CHECK_FPU_ENABLED
>  	{
>  	    TCGv addr, val;
>  	    val = tcg_temp_new();


-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

[Qemu-devel] Re: SH4: Implement FD bit

[Vladimir Prus]

Aurelien Jarno wrote:

>> 
>> SH4 manual say that if a floating point instruction is executed while
>> FD bit in the status register is 1, an exception should be raised. QEMU
>> presently does not do that, so the kernel does not initialize FP state
>> for any thread, nor does it save/restore FP state. The most apparent
>> consequence is that while recent gcc/libc expect double-precision mode
>> to be set by kernel, they run in single-precision mode, and all FP code
>> produces wrong values.
>> 
>> This patch fixes this. It also fixes a couple of places where PC was
>> not updated before handling an exception, although both those places
>> deal with invalid instruction and don't lead to any user-visible bugs.
>> 
>> - Volodya
> 
> Thanks, applied.

Thanks, but it looks like one bit of the patch somehow did not
make it into SVN. Specifically, this:

@@ -504,6 +523,13 @@ static void _decode_opc(DisasContext * ctx)
              }
        }
 
+    /* The 0xfffd instruction is underfined, so we don't want to
+       raise fpu disable exception on it.  */
+    if (((ctx->opcode & 0xf000) == 0xf000)
+       && (ctx->opcode != 0xfffd))      
+      {
+       CHECK_FPU_ENABLED
+      }

Is present in my post, and is not present in:

        http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5937

Maybe some merge issue?

Thanks,
Volodya

Re: [Qemu-devel] Re: SH4: Implement FD bit

[Aurelien Jarno]

On Tue, Dec 09, 2008 at 07:36:19PM +0300, Vladimir Prus wrote:
> Aurelien Jarno wrote:
> 
> >> 
> >> SH4 manual say that if a floating point instruction is executed while
> >> FD bit in the status register is 1, an exception should be raised. QEMU
> >> presently does not do that, so the kernel does not initialize FP state
> >> for any thread, nor does it save/restore FP state. The most apparent
> >> consequence is that while recent gcc/libc expect double-precision mode
> >> to be set by kernel, they run in single-precision mode, and all FP code
> >> produces wrong values.
> >> 
> >> This patch fixes this. It also fixes a couple of places where PC was
> >> not updated before handling an exception, although both those places
> >> deal with invalid instruction and don't lead to any user-visible bugs.
> >> 
> >> - Volodya
> > 
> > Thanks, applied.
> 
> Thanks, but it looks like one bit of the patch somehow did not
> make it into SVN. Specifically, this:
> 
> @@ -504,6 +523,13 @@ static void _decode_opc(DisasContext * ctx)
>               }
>         }
>  
> +    /* The 0xfffd instruction is underfined, so we don't want to
> +       raise fpu disable exception on it.  */
> +    if (((ctx->opcode & 0xf000) == 0xf000)
> +       && (ctx->opcode != 0xfffd))      
> +      {
> +       CHECK_FPU_ENABLED
> +      }
> 
> Is present in my post, and is not present in:
> 
>         http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5937
> 
> Maybe some merge issue?

Yes I haven't seen that one hunk of the patch has been rejected, so I
missed this part. I have just tried to merge it by hand, but it is not
possible to apply it. It looks like the patch was not done against the
current SVN.

Care to resend it against the current SVN?

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

[Qemu-devel] Re: Re: SH4: Implement FD bit

[Vladimir Prus]

[-- Attachment #1: Type: text/plain, Size: 1906 bytes --]

Aurelien Jarno wrote:

> On Tue, Dec 09, 2008 at 07:36:19PM +0300, Vladimir Prus wrote:
>> Aurelien Jarno wrote:
>> 
>> >> 
>> >> SH4 manual say that if a floating point instruction is executed while
>> >> FD bit in the status register is 1, an exception should be raised. QEMU
>> >> presently does not do that, so the kernel does not initialize FP state
>> >> for any thread, nor does it save/restore FP state. The most apparent
>> >> consequence is that while recent gcc/libc expect double-precision mode
>> >> to be set by kernel, they run in single-precision mode, and all FP code
>> >> produces wrong values.
>> >> 
>> >> This patch fixes this. It also fixes a couple of places where PC was
>> >> not updated before handling an exception, although both those places
>> >> deal with invalid instruction and don't lead to any user-visible bugs.
>> >> 
>> >> - Volodya
>> > 
>> > Thanks, applied.
>> 
>> Thanks, but it looks like one bit of the patch somehow did not
>> make it into SVN. Specifically, this:
>> 
>> @@ -504,6 +523,13 @@ static void _decode_opc(DisasContext * ctx)
>>               }
>>         }
>>  
>> +    /* The 0xfffd instruction is underfined, so we don't want to
>> +       raise fpu disable exception on it.  */
>> +    if (((ctx->opcode & 0xf000) == 0xf000)
>> +       && (ctx->opcode != 0xfffd))
>> +      {
>> +       CHECK_FPU_ENABLED
>> +      }
>> 
>> Is present in my post, and is not present in:
>> 
>>         http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5937
>> 
>> Maybe some merge issue?
> 
> Yes I haven't seen that one hunk of the patch has been rejected, so I
> missed this part. I have just tried to merge it by hand, but it is not
> possible to apply it. It looks like the patch was not done against the
> current SVN.
> 
> Care to resend it against the current SVN?

Attached is the remaining bit of the patch, against fresh SVN.

Thanks,
Volodya



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fd_remaining.diff --]
[-- Type: text/x-diff; name="fd_remaining.diff", Size: 1293 bytes --]

commit ffcb0cc661325b87995ae62dcdefa448e0b7296a
Author: Vladimir Prus <vladimir@codesourcery.com>
Date:   Mon Dec 1 19:28:32 2008 +0300

    SH: Implement FD bit
    
    	* target-sh4/cpu.h (cpu_get_tb_cpu_state): Include SR's FD bit in
    	the flags.
    	* target-sh4/helper.h (raise_fpu_disable, raise_slot_fpu_disable):
    	New helpers.
    	* targets-sh4/op_helper.c (helper_raise_fpu_disable)
    	(helper_raise_slot_fpu_disable): New.
    	* target-sh4/translate.c (CHECK_NOT_DELAY_SLOT, CHECK_PRIVILEGED):
    	Set PC to the right value.
    	(CHECK_FPU_ENABLED): New.
    	(_decode_opc): Use CHECK_FPU_ENABLED for FP instructions.

diff --git a/target-sh4/translate.c b/target-sh4/translate.c
index d6cfb7c..23a5d34 100644
--- a/target-sh4/translate.c
+++ b/target-sh4/translate.c
@@ -481,6 +481,15 @@ static void _decode_opc(DisasContext * ctx)
 #if 0
     fprintf(stderr, "Translating opcode 0x%04x\n", ctx->opcode);
 #endif
+
+    /* The 0xfffd instruction is underfined, so we don't want to
+       raise fpu disable exception on it.  */
+    if (((ctx->opcode & 0xf000) == 0xf000)
+	&& (ctx->opcode != 0xfffd))      
+      {
+	CHECK_FPU_ENABLED
+      }
+
     switch (ctx->opcode) {
     case 0x0019:		/* div0u */
 	tcg_gen_andi_i32(cpu_sr, cpu_sr, ~(SR_M | SR_Q | SR_T));

Re: [Qemu-devel] Re: Re: SH4: Implement FD bit

[Aurelien Jarno]

On Wed, Dec 10, 2008 at 02:04:27PM +0300, Vladimir Prus wrote:
> Aurelien Jarno wrote:
> 
> > On Tue, Dec 09, 2008 at 07:36:19PM +0300, Vladimir Prus wrote:
> >> Aurelien Jarno wrote:
> >> 
> >> >> 
> >> >> SH4 manual say that if a floating point instruction is executed while
> >> >> FD bit in the status register is 1, an exception should be raised. QEMU
> >> >> presently does not do that, so the kernel does not initialize FP state
> >> >> for any thread, nor does it save/restore FP state. The most apparent
> >> >> consequence is that while recent gcc/libc expect double-precision mode
> >> >> to be set by kernel, they run in single-precision mode, and all FP code
> >> >> produces wrong values.
> >> >> 
> >> >> This patch fixes this. It also fixes a couple of places where PC was
> >> >> not updated before handling an exception, although both those places
> >> >> deal with invalid instruction and don't lead to any user-visible bugs.
> >> >> 
> >> >> - Volodya
> >> > 
> >> > Thanks, applied.
> >> 
> >> Thanks, but it looks like one bit of the patch somehow did not
> >> make it into SVN. Specifically, this:
> >> 
> >> @@ -504,6 +523,13 @@ static void _decode_opc(DisasContext * ctx)
> >>               }
> >>         }
> >>  
> >> +    /* The 0xfffd instruction is underfined, so we don't want to
> >> +       raise fpu disable exception on it.  */
> >> +    if (((ctx->opcode & 0xf000) == 0xf000)
> >> +       && (ctx->opcode != 0xfffd))
> >> +      {
> >> +       CHECK_FPU_ENABLED
> >> +      }
> >> 
> >> Is present in my post, and is not present in:
> >> 
> >>         http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5937
> >> 
> >> Maybe some merge issue?
> > 
> > Yes I haven't seen that one hunk of the patch has been rejected, so I
> > missed this part. I have just tried to merge it by hand, but it is not
> > possible to apply it. It looks like the patch was not done against the
> > current SVN.
> > 
> > Care to resend it against the current SVN?
> 
> Attached is the remaining bit of the patch, against fresh SVN.

Actually I don't really like the idea of checking in the beginning of
the decode function for all instructions if they are a FP instruction,
and then checking if FPU is enabled. Given the number of instructions
(or group of instructions) concerned is not really big, I have committed
a patch that does the same by adding the check in each of the concerned
instruction.

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] Re: SH4: Implement FD bit

[Lionel Landwerlin]

Le mardi 09 décembre 2008 à 19:36 +0300, Vladimir Prus a écrit :
> Aurelien Jarno wrote:
> 
> >> 
> >> SH4 manual say that if a floating point instruction is executed while
> >> FD bit in the status register is 1, an exception should be raised. QEMU
> >> presently does not do that, so the kernel does not initialize FP state
> >> for any thread, nor does it save/restore FP state. The most apparent
> >> consequence is that while recent gcc/libc expect double-precision mode
> >> to be set by kernel, they run in single-precision mode, and all FP code
> >> produces wrong values.
> >> 
> >> This patch fixes this. It also fixes a couple of places where PC was
> >> not updated before handling an exception, although both those places
> >> deal with invalid instruction and don't lead to any user-visible bugs.
> >> 
> >> - Volodya
> > 
> > Thanks, applied.
> 
> Thanks, but it looks like one bit of the patch somehow did not
> make it into SVN. Specifically, this:
> 
> @@ -504,6 +523,13 @@ static void _decode_opc(DisasContext * ctx)
>               }
>         }
>  
> +    /* The 0xfffd instruction is underfined, so we don't want to
> +       raise fpu disable exception on it.  */
> +    if (((ctx->opcode & 0xf000) == 0xf000)
> +       && (ctx->opcode != 0xfffd))      
> +      {
> +       CHECK_FPU_ENABLED
> +      }
> 
> Is present in my post, and is not present in:
> 
>         http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5937
> 
> Maybe some merge issue?
> 
> Thanks,
> Volodya
> 

Vladimir (Volodya ?),

you probably know better than me sh4 opcodes, but your patch is breaking my sh4 user mode emulation.

$ ./sh4-linux-user/qemu-sh4 -strace -L /home/djdeath/taff/rootfs/ /middleware.dbg
1500 brk(0,0,0,1074264596,1074357772,0) = 0x00d98000
1500 uname(0x4007f6aa) = 0
1500 access("/etc/ld.so.preload",04) = -1 errno=2 (No such file or directory)
Unhandled trap: 0x800
pc=0x400977ea sr=0x00008001 pr=0x4008eae8 fpscr=0x00080000
spc=0x00000000 ssr=0x00000000 gbr=0x00000000 vbr=0x00000000
sgr=0x00000000 dbr=0x00000000 delayed_pc=0x400977e0 fpul=0x00000000
r0=0x000000e0 r1=0x00000000 r2=0x00008cf8 r3=0x4400302e
r4=0x4007f3f0 r5=0x00000000 r6=0x4007f57b r7=0x4008d200
r8=0x4007f3b8 r9=0x00b37cd0 r10=0x00000000 r11=0x4007f558
r12=0x400ae004 r13=0x00000000 r14=0x4007f3a4 r15=0x4007f3a4
r16=0x00000000 r17=0x00000000 r18=0x00000000 r19=0x00000000
r20=0x00000000 r21=0x00000000 r22=0x00000000 r23=0x00000000


According to you, should the FD bit be "up" by default in user mode emulation ?

By putting the FD bit "down" in cpu_sh4_reset(target-sh4/translate.c), I'm able to restore initial
behavior of user mode emulation (that means, qemu does not exits very early before libc initialization
(see the traces before)).

Maybe putting down the FD bit should be done in a user mode part only... I'm wondering...

Regards,

-- 
Lione Landwerlin                                         

O p e n W i d e                    14, rue Gaillon 75002 Paris

Re: [Qemu-devel] Re: SH4: Implement FD bit

[Laurent Desnogues]

On Thu, Dec 11, 2008 at 9:12 PM, Lionel Landwerlin
<lionel.landwerlin@openwide.fr> wrote:
> According to you, should the FD bit be "up" by default in user mode emulation ?
>
> By putting the FD bit "down" in cpu_sh4_reset(target-sh4/translate.c), I'm able to restore initial
> behavior of user mode emulation (that means, qemu does not exits very early before libc initialization
> (see the traces before)).
>
> Maybe putting down the FD bit should be done in a user mode part only... I'm wondering...

It doesn't look unacceptable to have some features enabled
in user mode that are not set at reset, since typically the OS
will enable these features either as its boot process or
depending on a process requirements.

BTW ins't the need for floating-point instructions contained
in some ELF SH4 flags?


Laurent

Re: [Qemu-devel] Re: SH4: Implement FD bit

[Lionel Landwerlin]

Le jeudi 11 décembre 2008 à 21:57 +0100, Laurent Desnogues a écrit :
> On Thu, Dec 11, 2008 at 9:12 PM, Lionel Landwerlin
> <lionel.landwerlin@openwide.fr> wrote:
> > According to you, should the FD bit be "up" by default in user mode emulation ?
> >
> > By putting the FD bit "down" in cpu_sh4_reset(target-sh4/translate.c), I'm able to restore initial
> > behavior of user mode emulation (that means, qemu does not exits very early before libc initialization
> > (see the traces before)).
> >
> > Maybe putting down the FD bit should be done in a user mode part only... I'm wondering...
> 
> It doesn't look unacceptable to have some features enabled
> in user mode that are not set at reset, since typically the OS
> will enable these features either as its boot process or
> depending on a process requirements.
> 
> BTW ins't the need for floating-point instructions contained
> in some ELF SH4 flags?
> 
> 
> Laurent
> 
> 

Hi Laurent,

According to you, where should I put this "fix" in linux-user/ ?


-- 
Lione Landwerlin                                         

O p e n W i d e                    14, rue Gaillon 75002 Paris

Re: [Qemu-devel] Re: SH4: Implement FD bit

[Laurent Desnogues]

On Thu, Dec 11, 2008 at 11:02 PM, Lionel Landwerlin
<lionel.landwerlin@openwide.fr> wrote:
>
> According to you, where should I put this "fix" in linux-user/ ?

If you look in target-sh4/translate.c/cpu_sh4_reset you will
find what you need.  For linux-user, CONFIG_USER_ONLY
is defined.

HTH,

Laurent