On 2026/08/26 10:38 AM, Chinmay Rath wrote: > From: Ojaswin Mujoo <[email protected]> > > Implement wait instruction using decodetree with > proper handling of WC and PL fields across different ISA > versions. Since the opcode changed between ISA 2.x and 3.x > we have defined 2 different decodetree instructions that > are handled by the same helper. > > The changes have been tested by comparing the tcg generated > before and after the change for wait instruction. The test > was repeated for 2 types of machines: > > Power10 - which should use the newer wait implementation > e500mc - which should use older PPC_WAIT implementation > > Both generated the same tcg ops with and without the change. > Further, with the changes we were able to boot into Fedora > distro as well. > > Signed-off-by: Ojaswin Mujoo <[email protected]> > Reviewed-by: Glenn Miles <[email protected]> > Signed-off-by: Chinmay Rath <[email protected]> > --- > target/ppc/insn32.decode | 15 +++ > target/ppc/translate.c | 91 ----------------- > target/ppc/translate/storage-ctrl-impl.c.inc | 100 +++++++++++++++++++ > 3 files changed, 115 insertions(+), 91 deletions(-) > > diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode > index 67fb181542..3b8387b60c 100644 > --- a/target/ppc/insn32.decode > +++ b/target/ppc/insn32.decode > @@ -1480,6 +1480,21 @@ ISYNC 010011 ----- ----- ----- 0010010110 - > @XL_bfa ...... bf:3 .. bfa:3 .. ..... ..... ..... . > MCRF 010011 ... -- ... -- ----- 00000 00000 - @XL_bfa > > +# Wait Instructions > + > +# We can use the same X_wait argument set for both WAIT and older ISA2.x > +# compliant WAIT_2_x. Depending on the version, we may or may not use the > +# arguments. For more information, check do_wait() helper. > + > +&X_wait wc pl > +@X_wait ...... ..- wc:2 --- pl:2 ----- .......... - &X_wait > + > +# According to ISA v3.1, bit 6 and 7 are implementation dependent and "unless > +# the intention is to use the implementation-dependent field, these bits > must be > +# coded zero". Hence, we code them to 0. > +WAIT 011111 00-.. ---.. ----- 0000011110 - @X_wait > +WAIT_ISA_2_X 011111 ---.. ---.. ----- 0000111110 - @X_wait > + > # Branch History Rolling Buffer (BHRB) Instructions > > &XFX_bhrbe rt bhrbe > diff --git a/target/ppc/translate.c b/target/ppc/translate.c > index f7c7f53d82..2d5157080c 100644 > --- a/target/ppc/translate.c > +++ b/target/ppc/translate.c > @@ -2727,95 +2727,6 @@ static inline void gen_check_tlb_flush(DisasContext > *ctx, bool global) > static inline void gen_check_tlb_flush(DisasContext *ctx, bool global) { } > #endif > > -/* wait */ > -static void gen_wait(DisasContext *ctx) > -{ > - uint32_t wc; > - > - if (ctx->insns_flags & PPC_WAIT) { > - /* v2.03-v2.07 define an older incompatible 'wait' encoding. */ > - > - if (ctx->insns_flags2 & PPC2_PM_ISA206) { > - /* v2.06 introduced the WC field. WC > 0 may be treated as > no-op. */ > - wc = WC(ctx->opcode); > - } else { > - wc = 0; > - } > - > - } else if (ctx->insns_flags2 & PPC2_ISA300) { > - /* v3.0 defines a new 'wait' encoding. */ > - wc = WC(ctx->opcode); > - if (ctx->insns_flags2 & PPC2_ISA310) { > - uint32_t pl = PL(ctx->opcode); > - > - /* WC 1,2 may be treated as no-op. WC 3 is reserved. */ > - if (wc == 3) { > - gen_invalid(ctx); > - return; > - } > - > - /* PL 1-3 are reserved. If WC=2 then the insn is treated as > noop. */ > - if (pl > 0 && wc != 2) { > - gen_invalid(ctx); > - return; > - } > - > - } else { /* ISA300 */ > - /* WC 1-3 are reserved */ > - if (wc > 0) { > - gen_invalid(ctx); > - return; > - } > - } > - > - } else { > - warn_report("wait instruction decoded with wrong ISA flags."); > - gen_invalid(ctx); > - return; > - } > - > - /* > - * wait without WC field or with WC=0 waits for an exception / interrupt > - * to occur. > - */ > - if (wc == 0) { > - TCGv_i32 t0 = tcg_constant_i32(1); > - tcg_gen_st_i32(t0, tcg_env, > - -offsetof(PowerPCCPU, env) + offsetof(CPUState, > halted)); > - /* 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 just wait until an exception occurs because > - * ignoring their other wake-up conditions could cause a hang. > - * > - * For v2.06 and 2.07, wc=1,2,3 are architected but may be implemented as > - * no-ops. > - * > - * wc=1 and wc=3 explicitly allow the instruction to be treated as a > no-op. > - * > - * wc=2 waits for an implementation-specific condition, such could be > - * always true, so it can be implemented as a no-op. > - * > - * For v3.1, wc=1,2 are architected but may be implemented as no-ops. > - * > - * wc=1 (waitrsv) waits for an exception or a reservation to be lost. > - * Reservation-loss may have implementation-specific conditions, so it > - * can be implemented as a no-op. > - * > - * wc=2 waits for an exception or an amount of time to pass. This > - * amount is implementation-specific so it can be implemented as a > - * no-op. > - * > - * ISA v3.1 allows 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 should be architecturally correct > - * (if suboptimal). > - */ > -} > - > #if defined(TARGET_PPC64) > static void gen_doze(DisasContext *ctx) > { > @@ -5197,8 +5108,6 @@ GEN_HANDLER(lswx, 0x1F, 0x15, 0x10, 0x00000001, > PPC_STRING), > GEN_HANDLER(stswi, 0x1F, 0x15, 0x16, 0x00000001, PPC_STRING), > GEN_HANDLER(stswx, 0x1F, 0x15, 0x14, 0x00000001, PPC_STRING), > /* ISA v3.0 changed the extended opcode from 62 to 30 */ > -GEN_HANDLER(wait, 0x1F, 0x1E, 0x01, 0x039FF801, PPC_WAIT), > -GEN_HANDLER_E(wait, 0x1F, 0x1E, 0x00, 0x039CF801, PPC_NONE, PPC2_ISA300), > GEN_HANDLER(rfi, 0x13, 0x12, 0x01, 0x03FF8001, PPC_FLOW), > #if defined(TARGET_PPC64) > GEN_HANDLER(rfid, 0x13, 0x12, 0x00, 0x03FF8001, PPC_64B), > diff --git a/target/ppc/translate/storage-ctrl-impl.c.inc > b/target/ppc/translate/storage-ctrl-impl.c.inc > index 943229c438..21c7999d5d 100644 > --- a/target/ppc/translate/storage-ctrl-impl.c.inc > +++ b/target/ppc/translate/storage-ctrl-impl.c.inc > @@ -270,3 +270,103 @@ static bool do_tlbie(DisasContext *ctx, arg_X_tlbie *a, > bool local) > > TRANS_FLAGS(MEM_TLBIE, TLBIE, do_tlbie, false) > TRANS_FLAGS(MEM_TLBIE, TLBIEL, do_tlbie, true) > + > +/* > + * Decodetree populates a->wc and a->pl based on ISA v3.1, however they may > + * or may not be used based on the ISA: > + * For ISA < 2.06 - both wc and pl are ignored > + * For ISA == 2.06, 2.07, 3.0 - wc is used and pl is ignored > + * For ISA 3.1 - both wc and pl are used > + */ > +static bool do_wait(DisasContext *ctx, arg_X_wait *a) > +{ > + uint32_t wc; > + > + if (ctx->insns_flags & PPC_WAIT) { > + /* v2.03-v2.07 define an older incompatible 'wait' encoding. */ > + > + if (ctx->insns_flags2 & PPC2_PM_ISA206) { > + /* v2.06 introduced the WC field. WC > 0 may be treated as > no-op. */ > + wc = a->wc; > + } else { > + wc = 0; > + } > + > + } else if (ctx->insns_flags2 & PPC2_ISA300) { > + /* v3.0 defines a new 'wait' encoding. */ > + wc = a->wc; > + if (ctx->insns_flags2 & PPC2_ISA310) { > + uint32_t pl = a->pl; > + > + /* WC 1,2 may be treated as no-op. WC 3 is reserved. */ > + if (wc == 3) { > + gen_invalid(ctx); > + return true; > + } > + > + /* PL 1-3 are reserved. If WC=2 then the insn is treated as > noop. */ > + if (pl > 0 && wc != 2) { > + gen_invalid(ctx); > + return true; > + } > + > + } else { /* ISA300 */ > + /* WC 1-3 are reserved */ > + if (wc > 0) { > + gen_invalid(ctx); > + return true; > + } > + } > + > + } else { > + warn_report("wait instruction decoded with wrong ISA flags."); > + gen_invalid(ctx); > + return true; > + } > + > + /* > + * wait without WC field or with WC=0 waits for an exception / interrupt > + * to occur. > + */ > + if (wc == 0) { > + TCGv_i32 t0 = tcg_constant_i32(1); > + tcg_gen_st_i32(t0, tcg_env, > + -offsetof(PowerPCCPU, env) + offsetof(CPUState, > halted)); > + /* 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 just wait until an exception occurs because > + * ignoring their other wake-up conditions could cause a hang. > + * > + * For v2.06 and 2.07, wc=1,2,3 are architected but may be implemented as > + * no-ops. > + * > + * wc=1 and wc=3 explicitly allow the instruction to be treated as a > no-op. > + * > + * wc=2 waits for an implementation-specific condition, such could be > + * always true, so it can be implemented as a no-op. > + * > + * For v3.1, wc=1,2 are architected but may be implemented as no-ops. > + * > + * wc=1 (waitrsv) waits for an exception or a reservation to be lost. > + * Reservation-loss may have implementation-specific conditions, so it > + * can be implemented as a no-op. > + * > + * wc=2 waits for an exception or an amount of time to pass. This > + * amount is implementation-specific so it can be implemented as a > + * no-op. > + * > + * ISA v3.1 allows 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 should be architecturally correct > + * (if suboptimal). > + */ > + return true;
The return true is aligned under the comment's */ instead of the function body indent level. > +} > + > +TRANS_FLAGS(WAIT, WAIT_ISA_2_X, do_wait) > +TRANS_FLAGS2(ISA300, WAIT, do_wait) > + This extra new line needs to be dropped to avoid git warning. Rest looks good to me. Reviewed-by: Amit Machhiwal <[email protected]> > -- > 2.55.0 >
