Re: [Qemu-devel] [PATCH 03/13] target-alpha: Implement rs/rc properly.

[Aurelien Jarno <aurelien@aurel32.net>]

On Mon, Mar 15, 2010 at 07:49:42AM -0700, Richard Henderson wrote:
> This is a per-cpu flag; there's no need for a spinlock of any kind.
> 
> We were also failing to manipulate the flag with $31 as a target reg
> and failing to clear the flag on execution of a return-from-interrupt
> instruction.
> 
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  linux-user/main.c        |    5 +++++
>  target-alpha/helper.h    |    2 --
>  target-alpha/op_helper.c |   28 ++--------------------------
>  target-alpha/translate.c |   19 +++++++++++++++----
>  4 files changed, 22 insertions(+), 32 deletions(-)
> 
> diff --git a/linux-user/main.c b/linux-user/main.c
> index ca49cc4..5252881 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -2357,6 +2357,11 @@ void cpu_loop (CPUState *env)
>      while (1) {
>          trapnr = cpu_alpha_exec (env);
>  
> +	/* All of the traps imply a transition through PALcode, which
> +	   implies an REI instruction has been executed.  Which means
> +	   that the intr_flag should be cleared.  */
> +	env->intr_flag = 0;
> +

The indentation here is wrong, you should use spaces instead of tabs.

>          switch (trapnr) {
>          case EXCP_RESET:
>              fprintf(stderr, "Reset requested. Exit\n");
> diff --git a/target-alpha/helper.h b/target-alpha/helper.h
> index 8e11304..c378195 100644
> --- a/target-alpha/helper.h
> +++ b/target-alpha/helper.h
> @@ -2,8 +2,6 @@
>  
>  DEF_HELPER_2(excp, void, int, int)
>  DEF_HELPER_FLAGS_0(load_pcc, TCG_CALL_CONST | TCG_CALL_PURE, i64)
> -DEF_HELPER_FLAGS_0(rc, TCG_CALL_CONST, i64)
> -DEF_HELPER_FLAGS_0(rs, TCG_CALL_CONST, i64)
>  
>  DEF_HELPER_2(addqv, i64, i64, i64)
>  DEF_HELPER_2(addlv, i64, i64, i64)
> diff --git a/target-alpha/op_helper.c b/target-alpha/op_helper.c
> index 2419dc4..84867b8 100644
> --- a/target-alpha/op_helper.c
> +++ b/target-alpha/op_helper.c
> @@ -47,32 +47,6 @@ void helper_store_fpcr (uint64_t val)
>      cpu_alpha_store_fpcr (env, val);
>  }
>  
> -static spinlock_t intr_cpu_lock = SPIN_LOCK_UNLOCKED;
> -
> -uint64_t helper_rs(void)
> -{
> -    uint64_t tmp;
> -
> -    spin_lock(&intr_cpu_lock);
> -    tmp = env->intr_flag;
> -    env->intr_flag = 1;
> -    spin_unlock(&intr_cpu_lock);
> -
> -    return tmp;
> -}
> -
> -uint64_t helper_rc(void)
> -{
> -    uint64_t tmp;
> -
> -    spin_lock(&intr_cpu_lock);
> -    tmp = env->intr_flag;
> -    env->intr_flag = 0;
> -    spin_unlock(&intr_cpu_lock);
> -
> -    return tmp;
> -}
> -
>  uint64_t helper_addqv (uint64_t op1, uint64_t op2)
>  {
>      uint64_t tmp = op1;
> @@ -1211,6 +1185,7 @@ void helper_hw_rei (void)
>  {
>      env->pc = env->ipr[IPR_EXC_ADDR] & ~3;
>      env->ipr[IPR_EXC_ADDR] = env->ipr[IPR_EXC_ADDR] & 1;
> +    env->intr_flag = 0;
>      /* XXX: re-enable interrupts and memory mapping */
>  }
>  
> @@ -1218,6 +1193,7 @@ void helper_hw_ret (uint64_t a)
>  {
>      env->pc = a & ~3;
>      env->ipr[IPR_EXC_ADDR] = a & 1;
> +    env->intr_flag = 0;
>      /* XXX: re-enable interrupts and memory mapping */
>  }
>  
> diff --git a/target-alpha/translate.c b/target-alpha/translate.c
> index b677378..188e76c 100644
> --- a/target-alpha/translate.c
> +++ b/target-alpha/translate.c
> @@ -1266,6 +1266,19 @@ static inline void gen_cmp(TCGCond cond, int ra, int rb, int rc, int islit,
>      gen_set_label(l2);
>  }
>  
> +static void gen_rx(int ra, int set)
> +{
> +    TCGv_i32 tmp;
> +
> +    if (ra != 31) {
> +        tcg_gen_ld8u_i64(cpu_ir[ra], cpu_env, offsetof(CPUState, intr_flag));
> +    }
> +
> +    tmp = tcg_const_i32(set);
> +    tcg_gen_st8_i32(tmp, cpu_env, offsetof(CPUState, intr_flag));
> +    tcg_temp_free_i32(tmp);
> +}
> +
>  static inline int translate_one(DisasContext *ctx, uint32_t insn)
>  {
>      uint32_t palcode;
> @@ -2359,16 +2372,14 @@ static inline int translate_one(DisasContext *ctx, uint32_t insn)
>              break;
>          case 0xE000:
>              /* RC */
> -            if (ra != 31)
> -                gen_helper_rc(cpu_ir[ra]);
> +            gen_rx(ra, 0);
>              break;
>          case 0xE800:
>              /* ECB */
>              break;
>          case 0xF000:
>              /* RS */
> -            if (ra != 31)
> -                gen_helper_rs(cpu_ir[ra]);
> +            gen_rx(ra, 1);
>              break;
>          case 0xF800:
>              /* WH64 */
> -- 
> 1.6.6.1
> 
> 
> 
> 

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