On Mon, May 17, 2021 at 12:46:51PM +1000, Nicholas Piggin wrote: > ISA v3.1 adds new variations of wait, specified by the WC field. These > are not compatible with the wait 0 implementation, because they add > additional conditions that cause the processor to resume, which can > cause software to hang or run very slowly. > > Add the new wait variants with a trivial no-op implementation, which is > allowed, as explained in comments: software must not depend on any > particular architected WC condition having caused resumption of > execution, therefore a no-op implementation is architecturally correct. > > Signed-off-by: Nicholas Piggin <npig...@gmail.com>
Logic looks fine. There is no test on the CPU's features or model
here, though, so this will change behaviour for pre-3.1 CPUs as well.
What would invoking these wait variants (presumably reserved) on
earlier CPUs do?
> ---
> Implementing cpu_relax() in Linux with wait 2,0 (pause_short) causes a
> hang on boot without this patch.
>
> target/ppc/translate.c | 39 +++++++++++++++++++++++++++++++++------
> 1 file changed, 33 insertions(+), 6 deletions(-)
>
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index a6381208a5..80db450cab 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -3619,12 +3619,39 @@ static void gen_sync(DisasContext *ctx)
> /* wait */
> static void gen_wait(DisasContext *ctx)
> {
> - TCGv_i32 t0 = tcg_const_i32(1);
> - tcg_gen_st_i32(t0, cpu_env,
> - -offsetof(PowerPCCPU, env) + offsetof(CPUState, halted));
> - tcg_temp_free_i32(t0);
> - /* Stop translation, as the CPU is supposed to sleep from now */
> - gen_exception_nip(ctx, EXCP_HLT, ctx->base.pc_next);
> + uint32_t wc = (ctx->opcode >> 21) & 3;
> +
> + /*
> + * wait 0 waits for an exception to occur.
> + */
> + if (wc == 0) {
> + TCGv_i32 t0 = tcg_const_i32(1);
> + tcg_gen_st_i32(t0, cpu_env,
> + -offsetof(PowerPCCPU, env) + offsetof(CPUState,
> halted));
> + tcg_temp_free_i32(t0);
> + /* Stop translation, as the CPU is supposed to sleep from now */
> + gen_exception_nip(ctx, EXCP_HLT, ctx->base.pc_next);
> + }
> +
> + /*
> + * Other wait types must not wait until an exception occurs because
> + * ignoring their other wake-up conditions could cause a hang.
> + *
> + * wait 1 (waitrsv) waits for an exception or a reservation to be lost.
> + * This can happen for implementation specific reasons, so it can be
> + * implemented as a no-op.
> + *
> + * wait 2 waits for an exception or an amount of time to pass. This is
> + * implementation specific so it can be implemented as a no-op.
> + *
> + * wait 3 is reserved, so it may be implemented as a no-op.
> + *
> + * ISA v3.1 does allow for execution to resume "in the rare case of
> + * an implementation-dependent event", so in any case software must
> + * not depend on the architected resumption condition to become
> + * true, so no-op implementations are architecturally correct (if
> + * suboptimal).
> + */
> }
>
> #if defined(TARGET_PPC64)
--
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
signature.asc
Description: PGP signature
