Re: [PATCH qemu] target/riscv: Add check for 16-bit aligned PC for different priv versions.

2025-03-18 Thread Alistair Francis
On Thu, Mar 13, 2025 at 4:27 PM ~yuming  wrote:
>
> From: Yu-Ming Chang 
>
> For privilege version 1.12 or newer, C always implies Zca. We can only
> check ext_zca to allow 16-bit aligned PC addresses. For older privilege
> versions, we only check C.
>
> Signed-off-by: Yu-Ming Chang 

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/cpu.h  | 12 
>  target/riscv/insn_trans/trans_rvi.c.inc |  8 ++--
>  target/riscv/op_helper.c|  8 ++--
>  target/riscv/translate.c|  4 +++-
>  4 files changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 7de19b4183..51e49e03de 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -765,6 +765,18 @@ static inline RISCVMXL riscv_cpu_sxl(CPURISCVState *env)
>  }
>  #endif
>
> +static inline bool riscv_cpu_allow_16bit_insn(const RISCVCPUConfig *cfg,
> +  target_long priv_ver,
> +  uint32_t misa_ext)
> +{
> +/* In priv spec version 1.12 or newer, C always implies Zca */
> +if (priv_ver >= PRIV_VERSION_1_12_0) {
> +return cfg->ext_zca;
> +} else {
> +return misa_ext & RVC;
> +}
> +}
> +
>  /*
>   * Encode LMUL to lmul as follows:
>   * LMULvlmullmul
> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc 
> b/target/riscv/insn_trans/trans_rvi.c.inc
> index b55f56a5eb..b9c7160468 100644
> --- a/target/riscv/insn_trans/trans_rvi.c.inc
> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> @@ -151,7 +151,9 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
>  tcg_gen_ext32s_tl(target_pc, target_pc);
>  }
>
> -if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca) {
> +if (!riscv_cpu_allow_16bit_insn(ctx->cfg_ptr,
> +ctx->priv_ver,
> +ctx->misa_ext)) {
>  TCGv t0 = tcg_temp_new();
>
>  misaligned = gen_new_label();
> @@ -300,7 +302,9 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, 
> TCGCond cond)
>
>  gen_set_label(l); /* branch taken */
>
> -if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca &&
> +if (!riscv_cpu_allow_16bit_insn(ctx->cfg_ptr,
> +ctx->priv_ver,
> +ctx->misa_ext) &&
>  (a->imm & 0x3)) {
>  /* misaligned */
>  TCGv target_pc = tcg_temp_new();
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index 0d4220ba93..72dc48e58d 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -279,7 +279,9 @@ target_ulong helper_sret(CPURISCVState *env)
>  }
>
>  target_ulong retpc = env->sepc;
> -if (!riscv_has_ext(env, RVC) && (retpc & 0x3)) {
> +if (!riscv_cpu_allow_16bit_insn(&env_archcpu(env)->cfg,
> +env->priv_ver,
> +env->misa_ext) && (retpc & 0x3)) {
>  riscv_raise_exception(env, RISCV_EXCP_INST_ADDR_MIS, GETPC());
>  }
>
> @@ -357,7 +359,9 @@ static void check_ret_from_m_mode(CPURISCVState *env, 
> target_ulong retpc,
>  riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
>  }
>
> -if (!riscv_has_ext(env, RVC) && (retpc & 0x3)) {
> +if (!riscv_cpu_allow_16bit_insn(&env_archcpu(env)->cfg,
> +env->priv_ver,
> +env->misa_ext) && (retpc & 0x3)) {
>  riscv_raise_exception(env, RISCV_EXCP_INST_ADDR_MIS, GETPC());
>  }
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index eaa5d86eae..d6651f244f 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -606,7 +606,9 @@ static void gen_jal(DisasContext *ctx, int rd, 
> target_ulong imm)
>  TCGv succ_pc = dest_gpr(ctx, rd);
>
>  /* check misaligned: */
> -if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca) {
> +if (!riscv_cpu_allow_16bit_insn(ctx->cfg_ptr,
> +ctx->priv_ver,
> +ctx->misa_ext)) {
>  if ((imm & 0x3) != 0) {
>  TCGv target_pc = tcg_temp_new();
>  gen_pc_plus_diff(target_pc, ctx, imm);
> --
> 2.45.3
>



Re: [PATCH qemu] target/riscv: Add check for 16-bit aligned PC for different priv versions.

2025-03-18 Thread Alistair Francis
On Thu, Mar 13, 2025 at 4:27 PM ~yuming  wrote:
>
> From: Yu-Ming Chang 
>
> For privilege version 1.12 or newer, C always implies Zca. We can only
> check ext_zca to allow 16-bit aligned PC addresses. For older privilege
> versions, we only check C.
>
> Signed-off-by: Yu-Ming Chang 

Please increment the patch version when submitting new patches

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu.h  | 12 
>  target/riscv/insn_trans/trans_rvi.c.inc |  8 ++--
>  target/riscv/op_helper.c|  8 ++--
>  target/riscv/translate.c|  4 +++-
>  4 files changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 7de19b4183..51e49e03de 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -765,6 +765,18 @@ static inline RISCVMXL riscv_cpu_sxl(CPURISCVState *env)
>  }
>  #endif
>
> +static inline bool riscv_cpu_allow_16bit_insn(const RISCVCPUConfig *cfg,
> +  target_long priv_ver,
> +  uint32_t misa_ext)
> +{
> +/* In priv spec version 1.12 or newer, C always implies Zca */
> +if (priv_ver >= PRIV_VERSION_1_12_0) {
> +return cfg->ext_zca;
> +} else {
> +return misa_ext & RVC;
> +}
> +}
> +
>  /*
>   * Encode LMUL to lmul as follows:
>   * LMULvlmullmul
> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc 
> b/target/riscv/insn_trans/trans_rvi.c.inc
> index b55f56a5eb..b9c7160468 100644
> --- a/target/riscv/insn_trans/trans_rvi.c.inc
> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> @@ -151,7 +151,9 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
>  tcg_gen_ext32s_tl(target_pc, target_pc);
>  }
>
> -if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca) {
> +if (!riscv_cpu_allow_16bit_insn(ctx->cfg_ptr,
> +ctx->priv_ver,
> +ctx->misa_ext)) {
>  TCGv t0 = tcg_temp_new();
>
>  misaligned = gen_new_label();
> @@ -300,7 +302,9 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, 
> TCGCond cond)
>
>  gen_set_label(l); /* branch taken */
>
> -if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca &&
> +if (!riscv_cpu_allow_16bit_insn(ctx->cfg_ptr,
> +ctx->priv_ver,
> +ctx->misa_ext) &&
>  (a->imm & 0x3)) {
>  /* misaligned */
>  TCGv target_pc = tcg_temp_new();
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index 0d4220ba93..72dc48e58d 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -279,7 +279,9 @@ target_ulong helper_sret(CPURISCVState *env)
>  }
>
>  target_ulong retpc = env->sepc;
> -if (!riscv_has_ext(env, RVC) && (retpc & 0x3)) {
> +if (!riscv_cpu_allow_16bit_insn(&env_archcpu(env)->cfg,
> +env->priv_ver,
> +env->misa_ext) && (retpc & 0x3)) {
>  riscv_raise_exception(env, RISCV_EXCP_INST_ADDR_MIS, GETPC());
>  }
>
> @@ -357,7 +359,9 @@ static void check_ret_from_m_mode(CPURISCVState *env, 
> target_ulong retpc,
>  riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
>  }
>
> -if (!riscv_has_ext(env, RVC) && (retpc & 0x3)) {
> +if (!riscv_cpu_allow_16bit_insn(&env_archcpu(env)->cfg,
> +env->priv_ver,
> +env->misa_ext) && (retpc & 0x3)) {
>  riscv_raise_exception(env, RISCV_EXCP_INST_ADDR_MIS, GETPC());
>  }
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index eaa5d86eae..d6651f244f 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -606,7 +606,9 @@ static void gen_jal(DisasContext *ctx, int rd, 
> target_ulong imm)
>  TCGv succ_pc = dest_gpr(ctx, rd);
>
>  /* check misaligned: */
> -if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca) {
> +if (!riscv_cpu_allow_16bit_insn(ctx->cfg_ptr,
> +ctx->priv_ver,
> +ctx->misa_ext)) {
>  if ((imm & 0x3) != 0) {
>  TCGv target_pc = tcg_temp_new();
>  gen_pc_plus_diff(target_pc, ctx, imm);
> --
> 2.45.3
>