Re: [Qemu-devel] [RFC] ppc/tcg: send cpu to sleep for simple endless guest loops

[David Gibson <david@gibson.dropbear.id.au>]

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

On Thu, Jun 28, 2018 at 10:35:24PM +0200, Sebastian Bauer wrote:
> When a branch instructions points to itself, only external events will
> change the internally observable cpu state. This change will adjust the
> behaviour of QEMU such that it sends the emulated cpu into a sleep state
> if this case is detected.
> 
> The effect for guests whose idle task contains this endless loop (e.g.,
> AmigaOS) is that they no longer hog the cpu of the host if they are idle.
> 
> Signed-off-by: Sebastian Bauer <mail@sebastianbauer.info>
> 
> ---
> 
> This is a RFC because I'm not really familiar with the internals
> of QEMU. There are probably other (better) ways to achive a similar
> behaviour. For that reason I haven't tested the change thoroughly
> yet, except that it produces the desired effect.
> 
> The patch will change the behaviour wrt. the instruction count which
> is no longer the same. But I don't think that this will matter.
> 
> I will be grateful for any expressed concerns and suggestion of that
> matter.
> 
> Patch is based on ppc-for-3.0.

Doesn't look insane to me, though I'm really not a TCG expert.

I will note that its utility is probably pretty limited.  I think most
OSes have a more complex idle loop than an unconditional branch to
self.

> ---
>  target/ppc/helper.h    |  2 ++
>  target/ppc/translate.c | 19 +++++++++++++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
> index d751f0e219..5f9b157417 100644
> --- a/target/ppc/helper.h
> +++ b/target/ppc/helper.h
> @@ -799,3 +799,5 @@ DEF_HELPER_4(dscliq, void, env, fprp, fprp, i32)
>  
>  DEF_HELPER_1(tbegin, void, env)
>  DEF_HELPER_FLAGS_1(fixup_thrm, TCG_CALL_NO_RWG, void, env)
> +
> +DEF_HELPER_1(sleep, void, env)
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 3a215a1dc6..7db53003ea 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -3469,6 +3469,18 @@ static inline void gen_setlr(DisasContext *ctx, target_ulong nip)
>      tcg_gen_movi_tl(cpu_lr, nip);
>  }
>  
> +/* Helper for bringing the cpu to the sleep state so it will
> + * react only on "external" events */
> +void helper_sleep(CPUPPCState *env)
> +{
> +    CPUState *cs = CPU(ppc_env_get_cpu(env));
> +
> +    cs->exception_index = EXCP_HALTED;
> +    cs->exit_request = 1;
> +    cs->halted = 1;
> +    cpu_loop_exit(cs);
> +}
> +
>  /* b ba bl bla */
>  static void gen_b(DisasContext *ctx)
>  {
> @@ -3483,6 +3495,13 @@ static void gen_b(DisasContext *ctx)
>      } else {
>          target = li;
>      }
> +
> +    if (target == ctx->base.pc_next - 4) {
> +        /* Endless-loop, CPU can now react on "external" events only, so we
> +         * simply go to sleep in order to save host processing resources */
> +        gen_helper_sleep(cpu_env);
> +    }
> +
>      if (LK(ctx->opcode)) {
>          gen_setlr(ctx, ctx->base.pc_next);
>      }

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]