Re: [Qemu-devel] [PATCH v2 for-2.4 1/2] target-arm: Split DISAS_YIELD from DISAS_WFE

[Peter Crosthwaite]

On Tue, Jun 30, 2015 at 6:51 AM, Peter Maydell <address@hidden> wrote:
> Currently we use DISAS_WFE for both WFE and YIELD instructions.
> This is functionally correct because at the moment both of them
> are implemented as "yield this CPU back to the top level loop so
> another CPU has a chance to run". However it's rather confusing
> that YIELD ends up calling HELPER(wfe), and if we ever want to
> implement real behaviour for WFE and SEV it's likely to trip us up.
>
> Split out the yield codepath to use DISAS_YIELD and a new
> HELPER(yield) function, and have HELPER(wfe) call HELPER(yield).
>
> Signed-off-by: Peter Maydell <address@hidden>

Reviewed-by: Peter Crosthwaite <address@hidden>

> ---
>  target-arm/helper.h        |  1 +
>  target-arm/op_helper.c     | 18 +++++++++++++++---
>  target-arm/translate-a64.c |  6 ++++++
>  target-arm/translate.h     |  1 +
>  4 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/target-arm/helper.h b/target-arm/helper.h
> index fc885de..827b33d 100644
> --- a/target-arm/helper.h
> +++ b/target-arm/helper.h
> @@ -50,6 +50,7 @@ DEF_HELPER_2(exception_internal, void, env, i32)
>  DEF_HELPER_4(exception_with_syndrome, void, env, i32, i32, i32)
>  DEF_HELPER_1(wfi, void, env)
>  DEF_HELPER_1(wfe, void, env)
> +DEF_HELPER_1(yield, void, env)
>  DEF_HELPER_1(pre_hvc, void, env)
>  DEF_HELPER_2(pre_smc, void, env, i32)
>
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 7fa32c4..663c05d 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -323,13 +323,25 @@ void HELPER(wfi)(CPUARMState *env)
>
>  void HELPER(wfe)(CPUARMState *env)
>  {
> -    CPUState *cs = CPU(arm_env_get_cpu(env));
> -
> -    /* Don't actually halt the CPU, just yield back to top
> +    /* This is a hint instruction that is semantically different
> +     * from YIELD even though we currently implement it identically.
> +     * Don't actually halt the CPU, just yield back to top
>       * level loop. This is not going into a "low power state"
>       * (ie halting until some event occurs), so we never take
>       * a configurable trap to a different exception level.
>       */
> +    HELPER(yield)(env);
> +}
> +
> +void HELPER(yield)(CPUARMState *env)
> +{
> +    ARMCPU *cpu = arm_env_get_cpu(env);
> +    CPUState *cs = CPU(cpu);
> +
> +    /* This is a non-trappable hint instruction that generally indicates
> +     * that the guest is currently busy-looping. Yield control back to the
> +     * top level loop so that a more deserving VCPU has a chance to run.
> +     */
>      cs->exception_index = EXCP_YIELD;
>      cpu_loop_exit(cs);
>  }
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index e077f2d..689f2be 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -1199,6 +1199,8 @@ static void handle_hint(DisasContext *s, uint32_t insn,
>          s->is_jmp = DISAS_WFI;
>          return;
>      case 1: /* YIELD */
> +        s->is_jmp = DISAS_YIELD;
> +        return;
>      case 2: /* WFE */
>          s->is_jmp = DISAS_WFE;
>          return;
> @@ -11107,6 +11109,10 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu,
>              gen_a64_set_pc_im(dc->pc);
>              gen_helper_wfe(cpu_env);
>              break;
> +        case DISAS_YIELD:
> +            gen_a64_set_pc_im(dc->pc);
> +            gen_helper_yield(cpu_env);
> +            break;
>          case DISAS_WFI:
>              /* This is a special case because we don't want to just halt the 
> CPU
>               * if trying to debug across a WFI.
> diff --git a/target-arm/translate.h b/target-arm/translate.h
> index bcdcf11..9ab978f 100644
> --- a/target-arm/translate.h
> +++ b/target-arm/translate.h
> @@ -103,6 +103,7 @@ static inline int default_exception_el(DisasContext *s)
>  #define DISAS_WFE 7
>  #define DISAS_HVC 8
>  #define DISAS_SMC 9
> +#define DISAS_YIELD 10
>
>  #ifdef TARGET_AARCH64
>  void a64_translate_init(void);
> --
> 1.9.1
>
>