| Message ID | 20230329032346.55185-4-liweiwei@iscas.ac.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/riscv: Fix pointer mask related support | expand |
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; > }
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; >> }
On 2023/3/29 11: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(+) > > 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); target_ulong next_pc = ctx->base.pc_next + a->imm; gen_set_pc_imm(ctx, next_pc); > 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); I think this patch is better than it in v6. So this patch, Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com> Zhiwei > gen_exception_inst_addr_mis(ctx); > return; > }
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; }