Re: [PATCH v2 3/5] target/riscv: Sync cpu_pc before update badaddr

[Richard Henderson]

On 3/28/23 20:23, Weiwei Li wrote:
> We should sync cpu_pc before storing it into badaddr when mis-aligned
> exception is triggered.
>
> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> ---
>   target/riscv/insn_trans/trans_rvi.c.inc | 1 +
>   target/riscv/translate.c                | 1 +
>   2 files changed, 2 insertions(+)

Yes, badaddr should get the invalid pc, but surely epc should contain the pc of the branch 
instruction causing the fault.  I thought that was the primary reason to handle this 
exception here, rather than at the start of the translation loop.


r~

> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc 
> b/target/riscv/insn_trans/trans_rvi.c.inc
> index 4ad54e8a49..05d8b5d57f 100644
> --- a/target/riscv/insn_trans/trans_rvi.c.inc
> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> @@ -171,6 +171,7 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond 
> cond)
>   
>       if (!has_ext(ctx, RVC) && ((ctx->base.pc_next + a->imm) & 0x3)) {
>           /* misaligned */
> +        gen_set_pc_imm(ctx, ctx->base.pc_next + a->imm);
>           gen_exception_inst_addr_mis(ctx);
>       } else {
>           gen_goto_tb(ctx, 0, ctx->base.pc_next + a->imm);
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 0ee8ee147d..f7ddf4c50d 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -551,6 +551,7 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong 
> imm)
>       next_pc = ctx->base.pc_next + imm;
>       if (!has_ext(ctx, RVC)) {
>           if ((next_pc & 0x3) != 0) {
> +            gen_set_pc_imm(ctx, next_pc);
>               gen_exception_inst_addr_mis(ctx);
>               return;
>           }