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

[liweiwei]

On 2023/3/29 23:33, Richard Henderson wrote:
> 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.
Yeah. the pc will be restored to the current pc in gen_exception() after 
updating the invalid pc into badaddr.

Regards,

Weiwei Li

> 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;
> >           }