diff mbox series

[qemu] target/riscv: Only check ext_zca for 16-bit aligned PC.

Message ID 174044811783.27139.4910941776283875756-0@git.sr.ht (mailing list archive)
State New, archived
Headers show
Series [qemu] target/riscv: Only check ext_zca for 16-bit aligned PC. | expand

Commit Message

~yuming Feb. 25, 2025, 1:36 a.m. UTC 
From: Yu-Ming Chang <yumin686@andestech.com>

Since C always implies Zca, Zca is always enabled when 16-bit
insructions are supported. we can only check ext_zca to allow
16-bit aligned PC addresses.

Signed-off-by: Yu-Ming Chang <yumin686@andestech.com>
---
 target/riscv/insn_trans/trans_rvi.c.inc | 5 ++---
 target/riscv/op_helper.c                | 4 ++--
 target/riscv/translate.c                | 2 +-
 3 files changed, 5 insertions(+), 6 deletions(-)

Comments

Alistair Francis Feb. 26, 2025, 11:19 p.m. UTC | #1 
On Tue, Feb 25, 2025 at 11:49 AM ~yuming <yuming@git.sr.ht> wrote:
>
> From: Yu-Ming Chang <yumin686@andestech.com>
>
> Since C always implies Zca, Zca is always enabled when 16-bit
> insructions are supported. we can only check ext_zca to allow
> 16-bit aligned PC addresses.
>
> Signed-off-by: Yu-Ming Chang <yumin686@andestech.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/insn_trans/trans_rvi.c.inc | 5 ++---
>  target/riscv/op_helper.c                | 4 ++--
>  target/riscv/translate.c                | 2 +-
>  3 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
> index 96c218a9d7..e5965201a7 100644
> --- a/target/riscv/insn_trans/trans_rvi.c.inc
> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> @@ -106,7 +106,7 @@ 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 (!ctx->cfg_ptr->ext_zca) {
>          TCGv t0 = tcg_temp_new();
>
>          misaligned = gen_new_label();
> @@ -236,8 +236,7 @@ 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 &&
> -        (a->imm & 0x3)) {
> +    if (!ctx->cfg_ptr->ext_zca && (a->imm & 0x3)) {
>          /* misaligned */
>          TCGv target_pc = tcg_temp_new();
>          gen_pc_plus_diff(target_pc, ctx, a->imm);
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index ce1256f439..68882136d7 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -276,7 +276,7 @@ target_ulong helper_sret(CPURISCVState *env)
>      }
>
>      target_ulong retpc = env->sepc;
> -    if (!riscv_has_ext(env, RVC) && (retpc & 0x3)) {
> +    if (!env_archcpu(env)->cfg.ext_zca && (retpc & 0x3)) {
>          riscv_raise_exception(env, RISCV_EXCP_INST_ADDR_MIS, GETPC());
>      }
>
> @@ -349,7 +349,7 @@ 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 (!env_archcpu(env)->cfg.ext_zca && (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 698b74f7a8..34eeed50be 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -566,7 +566,7 @@ 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 (!ctx->cfg_ptr->ext_zca) {
>          if ((imm & 0x3) != 0) {
>              TCGv target_pc = tcg_temp_new();
>              gen_pc_plus_diff(target_pc, ctx, imm);
> --
> 2.45.3
>
Alistair Francis March 6, 2025, 2:59 a.m. UTC | #2 
On Tue, Feb 25, 2025 at 11:49 AM ~yuming <yuming@git.sr.ht> wrote:
>
> From: Yu-Ming Chang <yumin686@andestech.com>
>
> Since C always implies Zca, Zca is always enabled when 16-bit
> insructions are supported. we can only check ext_zca to allow
> 16-bit aligned PC addresses.
>
> Signed-off-by: Yu-Ming Chang <yumin686@andestech.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/insn_trans/trans_rvi.c.inc | 5 ++---
>  target/riscv/op_helper.c                | 4 ++--
>  target/riscv/translate.c                | 2 +-
>  3 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
> index 96c218a9d7..e5965201a7 100644
> --- a/target/riscv/insn_trans/trans_rvi.c.inc
> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> @@ -106,7 +106,7 @@ 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 (!ctx->cfg_ptr->ext_zca) {
>          TCGv t0 = tcg_temp_new();
>
>          misaligned = gen_new_label();
> @@ -236,8 +236,7 @@ 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 &&
> -        (a->imm & 0x3)) {
> +    if (!ctx->cfg_ptr->ext_zca && (a->imm & 0x3)) {
>          /* misaligned */
>          TCGv target_pc = tcg_temp_new();
>          gen_pc_plus_diff(target_pc, ctx, a->imm);
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index ce1256f439..68882136d7 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -276,7 +276,7 @@ target_ulong helper_sret(CPURISCVState *env)
>      }
>
>      target_ulong retpc = env->sepc;
> -    if (!riscv_has_ext(env, RVC) && (retpc & 0x3)) {
> +    if (!env_archcpu(env)->cfg.ext_zca && (retpc & 0x3)) {
>          riscv_raise_exception(env, RISCV_EXCP_INST_ADDR_MIS, GETPC());
>      }
>
> @@ -349,7 +349,7 @@ 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 (!env_archcpu(env)->cfg.ext_zca && (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 698b74f7a8..34eeed50be 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -566,7 +566,7 @@ 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 (!ctx->cfg_ptr->ext_zca) {
>          if ((imm & 0x3) != 0) {
>              TCGv target_pc = tcg_temp_new();
>              gen_pc_plus_diff(target_pc, ctx, imm);
> --
> 2.45.3
>
Alistair Francis March 6, 2025, 3:16 a.m. UTC | #3 
On Tue, Feb 25, 2025 at 11:49 AM ~yuming <yuming@git.sr.ht> wrote:
>
> From: Yu-Ming Chang <yumin686@andestech.com>
>
> Since C always implies Zca, Zca is always enabled when 16-bit
> insructions are supported. we can only check ext_zca to allow
> 16-bit aligned PC addresses.

Urgh! Sorry about this

Zca is only in priv spec version 1.12 or newer. So although C does
always imply Zca, that's only true for 1.12 priv specs.

This patch as is breaks older CPUs (like the sifive_u).

That's my fault as I told you to use Zca, but unfortunately it doesn't work.

So, the best bet is probably a helper function that checks the priv
spec version then based on that checks either Zca or C. That way
when/if we drop versions before 1.12 we can update the code.

Alistair

>
> Signed-off-by: Yu-Ming Chang <yumin686@andestech.com>
> ---
>  target/riscv/insn_trans/trans_rvi.c.inc | 5 ++---
>  target/riscv/op_helper.c                | 4 ++--
>  target/riscv/translate.c                | 2 +-
>  3 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
> index 96c218a9d7..e5965201a7 100644
> --- a/target/riscv/insn_trans/trans_rvi.c.inc
> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> @@ -106,7 +106,7 @@ 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 (!ctx->cfg_ptr->ext_zca) {
>          TCGv t0 = tcg_temp_new();
>
>          misaligned = gen_new_label();
> @@ -236,8 +236,7 @@ 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 &&
> -        (a->imm & 0x3)) {
> +    if (!ctx->cfg_ptr->ext_zca && (a->imm & 0x3)) {
>          /* misaligned */
>          TCGv target_pc = tcg_temp_new();
>          gen_pc_plus_diff(target_pc, ctx, a->imm);
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index ce1256f439..68882136d7 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -276,7 +276,7 @@ target_ulong helper_sret(CPURISCVState *env)
>      }
>
>      target_ulong retpc = env->sepc;
> -    if (!riscv_has_ext(env, RVC) && (retpc & 0x3)) {
> +    if (!env_archcpu(env)->cfg.ext_zca && (retpc & 0x3)) {
>          riscv_raise_exception(env, RISCV_EXCP_INST_ADDR_MIS, GETPC());
>      }
>
> @@ -349,7 +349,7 @@ 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 (!env_archcpu(env)->cfg.ext_zca && (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 698b74f7a8..34eeed50be 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -566,7 +566,7 @@ 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 (!ctx->cfg_ptr->ext_zca) {
>          if ((imm & 0x3) != 0) {
>              TCGv target_pc = tcg_temp_new();
>              gen_pc_plus_diff(target_pc, ctx, imm);
> --
> 2.45.3
>
diff mbox series

Patch

diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
index 96c218a9d7..e5965201a7 100644
--- a/target/riscv/insn_trans/trans_rvi.c.inc
+++ b/target/riscv/insn_trans/trans_rvi.c.inc
@@ -106,7 +106,7 @@  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 (!ctx->cfg_ptr->ext_zca) {
         TCGv t0 = tcg_temp_new();
 
         misaligned = gen_new_label();
@@ -236,8 +236,7 @@  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 &&
-        (a->imm & 0x3)) {
+    if (!ctx->cfg_ptr->ext_zca && (a->imm & 0x3)) {
         /* misaligned */
         TCGv target_pc = tcg_temp_new();
         gen_pc_plus_diff(target_pc, ctx, a->imm);
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index ce1256f439..68882136d7 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -276,7 +276,7 @@  target_ulong helper_sret(CPURISCVState *env)
     }
 
     target_ulong retpc = env->sepc;
-    if (!riscv_has_ext(env, RVC) && (retpc & 0x3)) {
+    if (!env_archcpu(env)->cfg.ext_zca && (retpc & 0x3)) {
         riscv_raise_exception(env, RISCV_EXCP_INST_ADDR_MIS, GETPC());
     }
 
@@ -349,7 +349,7 @@  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 (!env_archcpu(env)->cfg.ext_zca && (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 698b74f7a8..34eeed50be 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -566,7 +566,7 @@  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 (!ctx->cfg_ptr->ext_zca) {
         if ((imm & 0x3) != 0) {
             TCGv target_pc = tcg_temp_new();
             gen_pc_plus_diff(target_pc, ctx, imm);