On Mon, Jul 20, 2026 at 06:49:49PM +0530, Chinmay Rath wrote: > > On 7/15/26 14:29, Ojaswin Mujoo wrote: > > On Mon, Jul 06, 2026 at 10:50:39PM +0530, Nikhil Kumar Singh wrote: > > > On 20/05/26 21:37, Chinmay Rath wrote: > > > > From: Ojaswin Mujoo<[email protected]> > > > > > > > > Convert the following instrunctions to decodetree specification: > > > The word should be spelled `instructions` and not `instrunctions`. Please > > > fix this in the > > > commit message. > > Hi Nikhil, thanks for the review. I'll make the change. > > > > bc > > > > bca > > > > bcl > > > > bcla > > > > > > > > bclr > > > > bclrl > > > > > > > > bcctr > > > > bcctrl > > > > > > > > bctar > > > > bctarl > > > > > > > > The branch was tested by comparing the qemu -D log -d op,in_asm output > > > > for > > > > each instruction and also for the various combinations of branch option > > > > (BO) bits for the conditional branch instructions. Additionally, gdb > > > > was used to confirm the LR and other register's behavior is consistent > > > > to legacy behavior. > > > > > > > > Further, the changes also pass a boot test into Fedora distro. > > > > > > > > Signed-off-by: Ojaswin Mujoo<[email protected]> > > > > Signed-off-by: Chinmay Rath<[email protected]> > > > > --- > > > > target/ppc/insn32.decode | 13 ++ > > > > target/ppc/translate.c | 148 ----------------------- > > > > target/ppc/translate/branch-impl.c.inc | 157 > > > > +++++++++++++++++++++++++ > > > > 3 files changed, 170 insertions(+), 148 deletions(-) > > > > > > > > diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode > > > > index b98fe01a84..34b36cbef6 100644 > > > > --- a/target/ppc/insn32.decode > > > > +++ b/target/ppc/insn32.decode > > > > @@ -1485,6 +1485,19 @@ CLRBHRB 011111 ----- ----- ----- > > > > 0110101110 - > > > > B 010010 ........................ . . @I_b > > > > +%bd 2:14 !function=times_4 > > > > +&bcond bo:uint32_t bi bd aa:bool lk:bool > > > > +@B_bcond ...... bo:5 bi:5 .............. aa:1 lk:1 &bcond bd=%bd > > > > + > > > > +BC 010000 ..... ..... .............. . . @B_bcond > > > > + > > > > +&bclr bo bi bh lk:bool > > > > +@XL_bclr ...... bo:5 bi:5 --- bh:2 .......... lk:1 &bclr > > > > + > > > > +BCLR 010011 ..... ..... ---.. 0000010000 . @XL_bclr > > > > +BCCTR 010011 ..... ..... ---.. 1000010000 . @XL_bclr > > > > +BCTAR 010011 ..... ..... ---.. 1000110000 . @XL_bclr > > > > + > > > > ## Misc POWER instructions > > > > ATTN 000000 00000 00000 00000 0100000000 0 > > > > diff --git a/target/ppc/translate.c b/target/ppc/translate.c > > > > index 37a164951f..22125c30a5 100644 > > > > --- a/target/ppc/translate.c > > > > +++ b/target/ppc/translate.c > > > > @@ -3089,150 +3089,6 @@ static inline void gen_setlr(DisasContext *ctx, > > > > target_ulong nip) > > > > tcg_gen_movi_tl(cpu_lr, nip); > > > > } > > > > -#define BCOND_IM 0 > > > > -#define BCOND_LR 1 > > > > -#define BCOND_CTR 2 > > > > -#define BCOND_TAR 3 > > > > - > > > > -static void gen_bcond(DisasContext *ctx, int type) > > > > -{ > > > > - uint32_t bo = BO(ctx->opcode); > > > > - TCGLabel *l1; > > > > - TCGv target; > > > > - target_long bhrb_type = BHRB_TYPE_OTHER; > > > > - > > > > - if (type == BCOND_LR || type == BCOND_CTR || type == BCOND_TAR) { > > > > - target = tcg_temp_new(); > > > > - if (type == BCOND_CTR) { > > > > - tcg_gen_mov_tl(target, cpu_ctr); > > > > - } else if (type == BCOND_TAR) { > > > > - gen_load_spr(target, SPR_TAR); > > > > - } else { > > > > - tcg_gen_mov_tl(target, cpu_lr); > > > > - } > > > > - if (!LK(ctx->opcode)) { > > > > - bhrb_type |= BHRB_TYPE_INDIRECT; > > > > - } > > > > - bhrb_type |= BHRB_TYPE_XL_FORM; > > > > - } else { > > > > - target = NULL; > > > > - } > > > > - if (LK(ctx->opcode)) { > > > > - gen_setlr(ctx, ctx->base.pc_next); > > > > - bhrb_type |= BHRB_TYPE_CALL; > > > > - } > > > > - l1 = gen_new_label(); > > > > - if ((bo & 0x4) == 0) { > > > > - /* Decrement and test CTR */ > > > > - TCGv temp = tcg_temp_new(); > > > > - > > > > - if (type == BCOND_CTR) { > > > > - /* > > > > - * All ISAs up to v3 describe this form of bcctr as > > > > invalid but > > > > - * some processors, ie. 64-bit server processors compliant > > > > with > > > > - * arch 2.x, do implement a "test and decrement" logic > > > > instead, > > > > - * as described in their respective UMs. This logic > > > > involves CTR > > > > - * to act as both the branch target and a counter, which > > > > makes > > > > - * it basically useless and thus never used in real code. > > > > - * > > > > - * This form was hence chosen to trigger extra > > > > micro-architectural > > > > - * side-effect on real HW needed for the Spectre v2 > > > > workaround. > > > > - * It is up to guests that implement such workaround, ie. > > > > linux, to > > > > - * use this form in a way it just triggers the side-effect > > > > without > > > > - * doing anything else harmful. > > > > - */ > > > > - if (unlikely(!is_book3s_arch2x(ctx))) { > > > > - gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL); > > > > - return; > > > > - } > > > > - > > > > - if (NARROW_MODE(ctx)) { > > > > - tcg_gen_ext32u_tl(temp, cpu_ctr); > > > > - } else { > > > > - tcg_gen_mov_tl(temp, cpu_ctr); > > > > - } > > > > - if (bo & 0x2) { > > > > - tcg_gen_brcondi_tl(TCG_COND_NE, temp, 0, l1); > > > > - } else { > > > > - tcg_gen_brcondi_tl(TCG_COND_EQ, temp, 0, l1); > > > > - } > > > > - tcg_gen_subi_tl(cpu_ctr, cpu_ctr, 1); > > > > - } else { > > > > - tcg_gen_subi_tl(cpu_ctr, cpu_ctr, 1); > > > > - if (NARROW_MODE(ctx)) { > > > > - tcg_gen_ext32u_tl(temp, cpu_ctr); > > > > - } else { > > > > - tcg_gen_mov_tl(temp, cpu_ctr); > > > > - } > > > > - if (bo & 0x2) { > > > > - tcg_gen_brcondi_tl(TCG_COND_NE, temp, 0, l1); > > > > - } else { > > > > - tcg_gen_brcondi_tl(TCG_COND_EQ, temp, 0, l1); > > > > - } > > > > - } > > > > - bhrb_type |= BHRB_TYPE_COND; > > > > - } > > > > - if ((bo & 0x10) == 0) { > > > > - /* Test CR */ > > > > - uint32_t bi = BI(ctx->opcode); > > > > - uint32_t mask = 0x08 >> (bi & 0x03); > > > > - TCGv_i32 temp = tcg_temp_new_i32(); > > > > - > > > > - if (bo & 0x8) { > > > > - tcg_gen_andi_i32(temp, cpu_crf[bi >> 2], mask); > > > > - tcg_gen_brcondi_i32(TCG_COND_EQ, temp, 0, l1); > > > > - } else { > > > > - tcg_gen_andi_i32(temp, cpu_crf[bi >> 2], mask); > > > > - tcg_gen_brcondi_i32(TCG_COND_NE, temp, 0, l1); > > > > - } > > > > - bhrb_type |= BHRB_TYPE_COND; > > > > - } > > > > - > > > > - gen_update_branch_history(ctx, ctx->cia, target, bhrb_type); > > > > - > > > > - if (type == BCOND_IM) { > > > > - target_ulong li = (target_long)((int16_t)(BD(ctx->opcode))); > > > > - if (likely(AA(ctx->opcode) == 0)) { > > > > - gen_goto_tb(ctx, 0, ctx->cia + li); > > > > - } else { > > > > - gen_goto_tb(ctx, 0, li); > > > > - } > > > > - } else { > > > > - if (NARROW_MODE(ctx)) { > > > > - tcg_gen_andi_tl(cpu_nip, target, (uint32_t)~3); > > > > - } else { > > > > - tcg_gen_andi_tl(cpu_nip, target, ~3); > > > > - } > > > > - gen_lookup_and_goto_ptr(ctx); > > > > - } > > > > - if ((bo & 0x14) != 0x14) { > > > > - /* fallthrough case */ > > > > - gen_set_label(l1); > > > > - gen_goto_tb(ctx, 1, ctx->base.pc_next); > > > > - } > > > > - ctx->base.is_jmp = DISAS_NORETURN; > > > > -} > > > > - > > > > -static void gen_bc(DisasContext *ctx) > > > > -{ > > > > - gen_bcond(ctx, BCOND_IM); > > > > -} > > > > - > > > > -static void gen_bcctr(DisasContext *ctx) > > > > -{ > > > > - gen_bcond(ctx, BCOND_CTR); > > > > -} > > > > - > > > > -static void gen_bclr(DisasContext *ctx) > > > > -{ > > > > - gen_bcond(ctx, BCOND_LR); > > > > -} > > > > - > > > > -static void gen_bctar(DisasContext *ctx) > > > > -{ > > > > - gen_bcond(ctx, BCOND_TAR); > > > > -} > > > > - > > > > /*** Condition register logical > > > > ***/ > > > > #define GEN_CRLOGIC(name, tcg_op, opc) > > > > \ > > > > static void glue(gen_, name)(DisasContext *ctx) > > > > \ > > > > @@ -5367,10 +5223,6 @@ 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(bc, 0x10, 0xFF, 0xFF, 0x00000000, PPC_FLOW), > > > > -GEN_HANDLER(bcctr, 0x13, 0x10, 0x10, 0x00000000, PPC_FLOW), > > > > -GEN_HANDLER(bclr, 0x13, 0x10, 0x00, 0x00000000, PPC_FLOW), > > > > -GEN_HANDLER_E(bctar, 0x13, 0x10, 0x11, 0x0000E000, PPC_NONE, > > > > PPC2_BCTAR_ISA207), > > > > 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/branch-impl.c.inc > > > > b/target/ppc/translate/branch-impl.c.inc > > > > index 745f71afd1..44ed40422e 100644 > > > > --- a/target/ppc/translate/branch-impl.c.inc > > > > +++ b/target/ppc/translate/branch-impl.c.inc > > > > @@ -55,3 +55,160 @@ static bool trans_B(DisasContext *ctx, arg_I_b *a) > > > > return true; > > > > } > > > > + > > > > +#define BCOND_IM 0 > > > > +#define BCOND_LR 1 > > > > +#define BCOND_CTR 2 > > > > +#define BCOND_TAR 3 > > > > + > > > > +static bool bcond_helper(DisasContext *ctx, int type, uint32_t bo, int > > > > bi, > > > > + int bd, int bh, bool aa, bool lk) > > > > +{ > > > > + TCGLabel *l1; > > > > + TCGv target; > > > > + target_long bhrb_type = BHRB_TYPE_OTHER; > > > > + > > > > + if (type == BCOND_IM && bh != -1) { > > > > + /* BCOND_IM should never use bh */ > > > > + return false; > > > > + } else if (type != BCOND_IM && (bd != -1 || aa != 0)) { > > > > + /* Other BCOND types should never use bd or aa */ > > > > + return false; > > > > + } > > > > + > > > > + if (type == BCOND_LR || type == BCOND_CTR || type == BCOND_TAR) { > > > > + target = tcg_temp_new(); > > > > + if (type == BCOND_CTR) { > > > > + tcg_gen_mov_tl(target, cpu_ctr); > > > > + } else if (type == BCOND_TAR) { > > > > + gen_load_spr(target, SPR_TAR); > > > > + } else { > > > > + tcg_gen_mov_tl(target, cpu_lr); > > > > + } > > > > + if (!lk) { > > > > + bhrb_type |= BHRB_TYPE_INDIRECT; > > > > + } > > > > + bhrb_type |= BHRB_TYPE_XL_FORM; > > > > + } else { > > > > + target = NULL; > > > > + } > > > > + if (lk) { > > > > + gen_setlr(ctx, ctx->base.pc_next); > > > > + bhrb_type |= BHRB_TYPE_CALL; > > > > + } > > > > + l1 = gen_new_label(); > > > > + if ((bo & 0x4) == 0) { > > > > + /* Decrement and test CTR */ > > > > + TCGv temp = tcg_temp_new(); > > > > + > > > > + if (type == BCOND_CTR) { > > > > + /* > > > > + * All ISAs up to v3 describe this form of bcctr as > > > > invalid but > > > > + * some processors, ie. 64-bit server processors compliant > > > > with > > > > + * arch 2.x, do implement a "test and decrement" logic > > > > instead, > > > > + * as described in their respective UMs. This logic > > > > involves CTR > > > > + * to act as both the branch target and a counter, which > > > > makes > > > > + * it basically useless and thus never used in real code. > > > > + * > > > > + * This form was hence chosen to trigger extra > > > > micro-architectural > > > > + * side-effect on real HW needed for the Spectre v2 > > > > workaround. > > > > + * It is up to guests that implement such workaround, ie. > > > > linux, to > > > > + * use this form in a way it just triggers the side-effect > > > > without > > > > + * doing anything else harmful. > > > > + */ > > > > + if (unlikely(!is_book3s_arch2x(ctx))) { > > > > + gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL); > > > > + return true; > > > > + } > > > > + > > > > + if (NARROW_MODE(ctx)) { > > > > + tcg_gen_ext32u_tl(temp, cpu_ctr); > > > > + } else { > > > > + tcg_gen_mov_tl(temp, cpu_ctr); > > > > + } > > > > + if (bo & 0x2) { > > > > + tcg_gen_brcondi_tl(TCG_COND_NE, temp, 0, l1); > > > > + } else { > > > > + tcg_gen_brcondi_tl(TCG_COND_EQ, temp, 0, l1); > > > > + } > > > > + tcg_gen_subi_tl(cpu_ctr, cpu_ctr, 1); > > > > + } else { > > > > + tcg_gen_subi_tl(cpu_ctr, cpu_ctr, 1); > > > > + if (NARROW_MODE(ctx)) { > > > > + tcg_gen_ext32u_tl(temp, cpu_ctr); > > > > + } else { > > > > + tcg_gen_mov_tl(temp, cpu_ctr); > > > > + } > > > > + if (bo & 0x2) { > > > > + tcg_gen_brcondi_tl(TCG_COND_NE, temp, 0, l1); > > > > + } else { > > > > + tcg_gen_brcondi_tl(TCG_COND_EQ, temp, 0, l1); > > > > + } > > > > + } > > > > + bhrb_type |= BHRB_TYPE_COND; > > > > + } > > > > + if ((bo & 0x10) == 0) { > > > > + /* Test CR */ > > > > + uint32_t mask = 0x08 >> (bi & 0x03); > > > > + TCGv_i32 temp = tcg_temp_new_i32(); > > > > + > > > > + if (bo & 0x8) { > > > > + tcg_gen_andi_i32(temp, cpu_crf[bi >> 2], mask); > > > > + tcg_gen_brcondi_i32(TCG_COND_EQ, temp, 0, l1); > > > > + } else { > > > > + tcg_gen_andi_i32(temp, cpu_crf[bi >> 2], mask); > > > > + tcg_gen_brcondi_i32(TCG_COND_NE, temp, 0, l1); > > > > + } > > > > + bhrb_type |= BHRB_TYPE_COND; > > > > + } > > > > + > > > > + gen_update_branch_history(ctx, ctx->cia, target, bhrb_type); > > > > + > > > > + if (type == BCOND_IM) { > > > > + target_ulong li = (target_long)((int16_t)(bd)); > > > > + if (likely(aa == 0)) { > > > > + gen_goto_tb(ctx, 0, ctx->cia + li); > > > > + } else { > > > > + gen_goto_tb(ctx, 0, li); > > > > + } > > > > + } else { > > > > + if (NARROW_MODE(ctx)) { > > > > + tcg_gen_andi_tl(cpu_nip, target, (uint32_t)~3); > > > > + } else { > > > > + tcg_gen_andi_tl(cpu_nip, target, ~3); > > > > + } > > > > + gen_lookup_and_goto_ptr(ctx); > > > > + } > > > > + if ((bo & 0x14) != 0x14) { > > > > + /* fallthrough case */ > > > > + gen_set_label(l1); > > > > + gen_goto_tb(ctx, 1, ctx->base.pc_next); > > > > + } > > > > + ctx->base.is_jmp = DISAS_NORETURN; > > > > + > > > > + return true; > > > > +} > > > > + > > > > +static bool trans_BC(DisasContext *ctx, arg_bcond *a) > > > > +{ > > > > + /* > > > > + * bh is not used for bc variants hence we pass -1 > > > > + */ > > > > + return bcond_helper(ctx, BCOND_IM, a->bo, a->bi, a->bd, -1, a->aa, > > > > a->lk); > > > > +} > > > > + > > > > +/* > > > > + * This helper is shared by bclr, bcctr and bctar. > > > > + */ > > > > +static bool bclr_helper(DisasContext *ctx, arg_bclr *a, int type) > > > > +{ > > > > + /* > > > > + * bd and aa is not used for bc variants hence we pass -1 and 0 > > > > respectively > > > > + */ > > > > + return bcond_helper(ctx, type, a->bo, a->bi, -1, a->bh, 0, a->lk); > > > > +} > > > > + > > > > +TRANS(BCLR, bclr_helper, BCOND_LR) > > > > +TRANS(BCCTR, bclr_helper, BCOND_CTR) > > > > +TRANS(BCTAR, bclr_helper, BCOND_TAR) > > > > + > > > The patch looks mostly fine, but I noticed one small issue regarding the > > > bctar instruction. > > > > > > Actually, in the legacy implementation, bctar was properly guarded by the > > > ISA 2.07 feature flag like this: > > > GEN_HANDLER_E(bctar, 0x13, 0x10, 0x11, 0x0000E000, PPC_NONE, > > > PPC2_BCTAR_ISA207) > > > > > > But in the new decodetree implementation, it is getting mapped directly > > > using the standard macro > > > TRANS(BCTAR, bclr_helper, BCOND_TAR) > > Yes you are right, I missed this. Thanks for pointing it out. > > > > > Since the TRANS macro doesn't apply any secondary instruction flags (and > > > even bclr_helper / bcond_helper are not checking for it), > > > this check is being missed out. > Hi Nikhil, > Thanks for catching this ! > > > > > > It could be better if you can just write a dedicated translation handler > > > for > > > the same to enforce the BCTAR_ISA207 check. > > > I guess Chinmay can tell better on this. > > I think we can just do the following and it should be good enough: > > > > -TRANS(BCTAR, bclr_helper, BCOND_TAR) > > +TRANS_FLAGS2(BCTAR_ISA207, BCTAR, bclr_helper, BCOND_TAR) > > > > If this looks okay, I can send a v2. > > Hi Ojas, > Since it is just a single line change, I will take care of it while posting > v2 series.
Sure Chinmay, thanks! > > Regards, > Chinmay > > > > > Regards, > > ojaswin > > > > > Thanks > > > ~ Nikhil > > > >
