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 <address@hidden>
> 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
 `. `'   address@hidden         | address@hidden
   `-    people.debian.org/~aurel32 | www.aurel32.net