Re: [PATCH v1] riscv: Add support for Zibi extension

[Chao Liu <chao.liu.zevorn@gmail.com>]

Hi Zhibo,

Thanks for the patch. I have some comments below.

On Tue, Feb 03, 2026 at 04:21:44PM +0800, 洪志博 wrote:
> From: Zhibo Hong <hongzhibo@bytedance.com>
>
The patch is missing a commit message. Please add a title and description
explaining what the Zibi extension is and what instructions are being
implemented. A link to the specification would also be helpful.

> Signed-off-by: Zhibo Hong <hongzhibo@bytedance.com>
> ---
>  disas/riscv.c                              | 17 +++++++
>  disas/riscv.h                              |  2 +
>  target/riscv/cpu.c                         |  2 +
>  target/riscv/cpu_cfg_fields.h.inc          |  1 +
>  target/riscv/insn32.decode                 |  9 ++++
>  target/riscv/insn_trans/trans_rvzibi.c.inc | 54 ++++++++++++++++++++++
>  target/riscv/translate.c                   |  6 +++
>  7 files changed, 91 insertions(+)
>  create mode 100644 target/riscv/insn_trans/trans_rvzibi.c.inc
> 
> diff --git a/disas/riscv.c b/disas/riscv.c
> index 85cd2a9c2a..92862835f0 100644
> --- a/disas/riscv.c
> +++ b/disas/riscv.c
> @@ -984,6 +984,8 @@ typedef enum {
>      rv_op_ssamoswap_d = 953,
>      rv_op_c_sspush = 954,
>      rv_op_c_sspopchk = 955,
> +    rv_op_beqi      = 956,
> +    rv_op_bnei      = 957,
>  } rv_op;
>  
>  /* register names */
> @@ -2254,6 +2256,8 @@ const rv_opcode_data rvi_opcode_data[] = {
>        rv_op_sspush, 0 },
>      { "c.sspopchk", rv_codec_cmop_ss, rv_fmt_rs1, NULL, rv_op_sspopchk,
>        rv_op_sspopchk, 0 },
> +    { "beqi", rv_codec_bi, rv_fmt_rs1_imm1_offset, NULL, 0, 0, 0 },
> +    { "bnei", rv_codec_bi, rv_fmt_rs1_imm1_offset, NULL, 0, 0, 0 },
>  };
>  
>  /* CSR names */
> @@ -3997,6 +4001,8 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
>              switch ((inst >> 12) & 0b111) {
>              case 0: op = rv_op_beq; break;
>              case 1: op = rv_op_bne; break;
> +            case 2: op = rv_op_beqi; break;
> +            case 3: op = rv_op_bnei; break;
>              case 4: op = rv_op_blt; break;
>              case 5: op = rv_op_bge; break;
>              case 6: op = rv_op_bltu; break;
> @@ -4529,6 +4535,12 @@ static uint32_t operand_imml(rv_inst inst)
>      return (inst << 38) >> 58;
>  }
>  
> +static int32_t operand_bi(rv_inst inst)
> +{
> +    uint32_t cimm = (inst << 39) >> 59;
> +    return cimm == 0 ? -1 : cimm;
> +}
> +
>  static uint32_t calculate_stack_adj(rv_isa isa, uint32_t rlist, uint32_t spimm)
>  {
>      int xlen_bytes_log2 = isa == rv64 ? 3 : 2;
> @@ -4948,6 +4960,11 @@ static void decode_inst_operands(rv_decode *dec, rv_isa isa)
>          dec->rs1 = dec->rs2 = operand_crs1(inst);
>          dec->imm = 0;
>          break;
> +    case rv_codec_bi:
> +        dec->rs1 = operand_rs1(inst);
> +        dec->imm = operand_sbimm12(inst);
> +        dec->imm1 = operand_bi(inst);
> +        break;
>      };
>  }
>  
> diff --git a/disas/riscv.h b/disas/riscv.h
> index d211700cb2..452c788278 100644
> --- a/disas/riscv.h
> +++ b/disas/riscv.h
> @@ -168,6 +168,7 @@ typedef enum {
>      rv_codec_fli,
>      rv_codec_lp,
>      rv_codec_cmop_ss,
> +    rv_codec_bi,
>  } rv_codec;
>  
>  /* structures */
> @@ -305,5 +306,6 @@ enum {
>  #define rv_fmt_rd_rs1_immh_imml_addr  "O\t0,(1),i,j"
>  #define rv_fmt_rd2_imm                "O\t0,2,(1),i"
>  #define rv_fmt_fli                    "O\t3,h"
> +#define rv_fmt_rs1_imm1_offset        "O\t1,j,o"
>  
>  #endif /* DISAS_RISCV_H */
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index e95eea0249..0a1cb41c96 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -120,6 +120,7 @@ static void riscv_cpu_cfg_merge(RISCVCPUConfig *dest, const RISCVCPUConfig *src)
>   * instead.
>   */
>  const RISCVIsaExtData isa_edata_arr[] = {
> +    ISA_EXT_DATA_ENTRY(zibi, PRIV_VERSION_1_13_0, ext_zibi),
>      ISA_EXT_DATA_ENTRY(zic64b, PRIV_VERSION_1_12_0, ext_zic64b),
>      ISA_EXT_DATA_ENTRY(zicbom, PRIV_VERSION_1_12_0, ext_zicbom),
>      ISA_EXT_DATA_ENTRY(zicbop, PRIV_VERSION_1_12_0, ext_zicbop),
> @@ -1238,6 +1239,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
>      MULTI_EXT_CFG_BOOL("ssccfg", ext_ssccfg, false),
>      MULTI_EXT_CFG_BOOL("smctr", ext_smctr, false),
>      MULTI_EXT_CFG_BOOL("ssctr", ext_ssctr, false),
> +    MULTI_EXT_CFG_BOOL("zibi", ext_zibi, false),
>      MULTI_EXT_CFG_BOOL("zifencei", ext_zifencei, true),
>      MULTI_EXT_CFG_BOOL("zicfilp", ext_zicfilp, false),
>      MULTI_EXT_CFG_BOOL("zicfiss", ext_zicfiss, false),
> diff --git a/target/riscv/cpu_cfg_fields.h.inc b/target/riscv/cpu_cfg_fields.h.inc
> index 70ec650abf..046c656bd9 100644
> --- a/target/riscv/cpu_cfg_fields.h.inc
> +++ b/target/riscv/cpu_cfg_fields.h.inc
> @@ -30,6 +30,7 @@ BOOL_FIELD(ext_zks)
>  BOOL_FIELD(ext_zksed)
>  BOOL_FIELD(ext_zksh)
>  BOOL_FIELD(ext_zkt)
> +BOOL_FIELD(ext_zibi)
>  BOOL_FIELD(ext_zifencei)
>  BOOL_FIELD(ext_zicntr)
>  BOOL_FIELD(ext_zicsr)
> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> index 6e35c4b1e6..0a92e79508 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -20,6 +20,7 @@
>  %rs3       27:5
>  %rs2       20:5
>  %rs1       15:5
> +%rs1_3     17:3                  !function=ex_rvc_register

This definition is added but not used anywhere. Is it needed? If not, please
remove it.

>  %rd        7:5
>  %sh5       20:5
>  %sh6       20:6
> @@ -40,6 +41,7 @@
>  %imm_z6   26:1 15:5
>  %imm_mop5 30:1 26:2 20:2
>  %imm_mop3 30:1 26:2
> +%imm_bi   20:5                   !function=ex_bi
>  
>  # Argument sets:
>  &empty
> @@ -60,6 +62,7 @@
>  &k_aes     shamt rs2 rs1 rd
>  &mop5 imm rd rs1
>  &mop3 imm rd rs1 rs2
> +&bi   imm imm2 rs1
>  
>  # Formats 32:
>  @r       .......   ..... ..... ... ..... ....... &r                %rs2 %rs1 %rd
> @@ -111,6 +114,8 @@
>  # Formats 128:
>  @sh6       ...... ...... ..... ... ..... ....... &shift shamt=%sh6 %rs1 %rd
>  
> +@bi      ....... ..... ... .. ... ..... ....... &bi imm=%imm_b imm2=%imm_bi rs1=%rs1
> +
>  # *** Privileged Instructions ***
>  ecall       000000000000     00000 000 00000 1110011
>  ebreak      000000000001     00000 000 00000 1110011
> @@ -1084,3 +1089,7 @@ sb_aqrl  00111 . . ..... ..... 000 ..... 0101111 @atom_st
>  sh_aqrl  00111 . . ..... ..... 001 ..... 0101111 @atom_st
>  sw_aqrl  00111 . . ..... ..... 010 ..... 0101111 @atom_st
>  sd_aqrl  00111 . . ..... ..... 011 ..... 0101111 @atom_st
> +
> +# *** Zibi Extension ***
> +beqi       ....... ..... ..... 010 ..... 1100011 @bi
> +bnei       ....... ..... ..... 011 ..... 1100011 @bi
> \ No newline at end of file

Missing newline at end of file. Please run script/checkpatch.pl check
it.

> diff --git a/target/riscv/insn_trans/trans_rvzibi.c.inc b/target/riscv/insn_trans/trans_rvzibi.c.inc
> new file mode 100644
> index 0000000000..24bc0806aa
> --- /dev/null
> +++ b/target/riscv/insn_trans/trans_rvzibi.c.inc
Missing copyright header and SPDX license identifier. Please add something
like:

/*
 * RISC-V translation routines for the Zibi Extension.
 *
 * Copyright (c) 2025 Zhibo Hong <hongzhibo@bytedance.com>
 *
 * SPDX-License-Identifier: GPL-2.0-or-later
 */

> @@ -0,0 +1,54 @@
> +#define REQUIRE_ZIBI(ctx) do {                  \
> +    if (!ctx->cfg_ptr->ext_zibi) {              \
> +        return false;                           \
> +    }                                           \
> +} while (0)
> +
> +static bool gen_immediate_branch(DisasContext *ctx, arg_bi *a, TCGCond cond)
> +{
> +    TCGLabel *l = gen_new_label();
> +    TCGv src1 = get_gpr(ctx, a->rs1, EXT_SIGN);
> +    TCGv imm2 = tcg_constant_tl(a->imm2);
> +    target_ulong orig_pc_save = ctx->pc_save;
> +
> +    if (get_xl(ctx) == MXL_RV128) {
> +        TCGv src1h = get_gprh(ctx, a->rs1);
> +        TCGv imm2h = tcg_constant_tl(a->imm2 >= 0 ? 0 : -1);
> +        TCGv tmp = tcg_temp_new();
> +
> +        cond = gen_compare_i128(false, tmp, src1, src1h, imm2, imm2h, cond);
> +        tcg_gen_brcondi_tl(cond, tmp, 0, l);
> +    } else {
> +        tcg_gen_brcond_tl(cond, src1, imm2, l);
> +    }
> +    gen_goto_tb(ctx, 1, ctx->cur_insn_len);
> +    ctx->pc_save = orig_pc_save;
> +
> +    gen_set_label(l); /* branch taken */
> +
This function is missing CTR (Control Transfer Records) support. Please add
the smctr/ssctr handling similar to gen_branch() in trans_rvi.c.inc:

#ifndef CONFIG_USER_ONLY
    if (ctx->cfg_ptr->ext_smctr || ctx->cfg_ptr->ext_ssctr) {
        TCGv type = tcg_constant_tl(CTRDATA_TYPE_NONTAKEN_BRANCH);
        ...
    }
#endif

And similarly for the taken branch path with CTRDATA_TYPE_TAKEN_BRANCH.

> +    if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca &&
Please use riscv_cpu_allow_16bit_insn() instead of directly checking RVC/zca,
to be consistent with the existing code in trans_rvi.c.inc:

    if (!riscv_cpu_allow_16bit_insn(ctx->cfg_ptr,
                                    ctx->priv_ver,
                                    ctx->misa_ext) &&
        (a->imm & 0x3)) {

Thanks,
Chao
> +        (a->imm & 0x3)) {
> +        /* misaligned */
> +        TCGv target_pc = tcg_temp_new();
> +        gen_pc_plus_diff(target_pc, ctx, a->imm);
> +        gen_exception_inst_addr_mis(ctx, target_pc);
> +    } else {
> +        gen_goto_tb(ctx, 0, a->imm);
> +    }
> +    ctx->pc_save = -1;
> +    ctx->base.is_jmp = DISAS_NORETURN;
> +
> +    return true;
> +}
> +
> +static bool trans_beqi(DisasContext *ctx, arg_beqi *a)
> +{
> +    REQUIRE_ZIBI(ctx);
> +    return gen_immediate_branch(ctx, a, TCG_COND_EQ);
> +}
> +
> +static bool trans_bnei(DisasContext *ctx, arg_bnei *a)
> +{
> +    REQUIRE_ZIBI(ctx);
> +    return gen_immediate_branch(ctx, a, TCG_COND_NE);
> +}
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index f687c75fe4..d0b83ebe01 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -858,6 +858,11 @@ static int ex_rvc_shiftri(DisasContext *ctx, int imm)
>      return imm;
>  }
>  
> +static int ex_bi(DisasContext *ctx, int imm)
> +{
> +    return imm == 0 ? -1 : imm;
> +}
> +
>  /* Include the auto-generated decoder for 32 bit insn */
>  #include "decode-insn32.c.inc"
>  
> @@ -1215,6 +1220,7 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
>  #include "insn_trans/trans_xthead.c.inc"
>  #include "insn_trans/trans_xventanacondops.c.inc"
>  #include "insn_trans/trans_xmips.c.inc"
> +#include "insn_trans/trans_rvzibi.c.inc"
>  
>  /* Include the auto-generated decoder for 16 bit insn */
>  #include "decode-insn16.c.inc"
> -- 
> 2.39.5
>