On Tue, Sep 6, 2022 at 12:09 PM Richard Henderson <richard.hender...@linaro.org> wrote: > > Like gen_update_cc_op, sync EIP before doing something > that could raise an exception. Replace all gen_jmp_im > that use s->base.pc_next.
The commit message seems a bit off (it sounds like adding stuff rather than refactoring), but anyway: Reviewed-by: Paolo Bonzini <pbonz...@redhat.com> Paolo > --- > target/i386/tcg/translate.c | 52 ++++++++++++++++++++----------------- > 1 file changed, 28 insertions(+), 24 deletions(-) > > diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c > index 0210382f77..83cb925571 100644 > --- a/target/i386/tcg/translate.c > +++ b/target/i386/tcg/translate.c > @@ -511,10 +511,14 @@ static inline void gen_op_st_rm_T0_A0(DisasContext *s, > int idx, int d) > } > } > > -static inline void gen_jmp_im(DisasContext *s, target_ulong pc) > +static void gen_jmp_im(DisasContext *s, target_ulong pc) > { > - tcg_gen_movi_tl(s->tmp0, pc); > - gen_op_jmp_v(s->tmp0); > + gen_op_jmp_v(tcg_constant_tl(pc)); > +} > + > +static void gen_update_eip_cur(DisasContext *s) > +{ > + gen_jmp_im(s, s->base.pc_next - s->cs_base); > } > > /* Compute SEG:REG into A0. SEG is selected from the override segment > @@ -703,7 +707,7 @@ static bool gen_check_io(DisasContext *s, MemOp ot, > TCGv_i32 port, > target_ulong next_eip = s->pc - s->cs_base; > > gen_update_cc_op(s); > - gen_jmp_im(s, cur_eip); > + gen_update_eip_cur(s); > if (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) { > svm_flags |= SVM_IOIO_REP_MASK; > } > @@ -1335,7 +1339,7 @@ static void gen_helper_fp_arith_STN_ST0(int op, int > opreg) > static void gen_exception(DisasContext *s, int trapno) > { > gen_update_cc_op(s); > - gen_jmp_im(s, s->base.pc_next - s->cs_base); > + gen_update_eip_cur(s); > gen_helper_raise_exception(cpu_env, tcg_const_i32(trapno)); > s->base.is_jmp = DISAS_NORETURN; > } > @@ -2605,7 +2609,7 @@ static void gen_unknown_opcode(CPUX86State *env, > DisasContext *s) > static void gen_interrupt(DisasContext *s, int intno) > { > gen_update_cc_op(s); > - gen_jmp_im(s, s->base.pc_next - s->cs_base); > + gen_update_eip_cur(s); > gen_helper_raise_interrupt(cpu_env, tcg_constant_i32(intno), > tcg_constant_i32(s->pc - s->base.pc_next)); > s->base.is_jmp = DISAS_NORETURN; > @@ -6796,7 +6800,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) > do_lret: > if (PE(s) && !VM86(s)) { > gen_update_cc_op(s); > - gen_jmp_im(s, s->base.pc_next - s->cs_base); > + gen_update_eip_cur(s); > gen_helper_lret_protected(cpu_env, tcg_const_i32(dflag - 1), > tcg_const_i32(val)); > } else { > @@ -7292,7 +7296,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) > } > if (prefixes & PREFIX_REPZ) { > gen_update_cc_op(s); > - gen_jmp_im(s, s->base.pc_next - s->cs_base); > + gen_update_eip_cur(s); > gen_helper_pause(cpu_env, tcg_const_i32(s->pc - > s->base.pc_next)); > s->base.is_jmp = DISAS_NORETURN; > } > @@ -7318,7 +7322,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) > if (CODE64(s)) > goto illegal_op; > gen_update_cc_op(s); > - gen_jmp_im(s, s->base.pc_next - s->cs_base); > + gen_update_eip_cur(s); > gen_helper_into(cpu_env, tcg_const_i32(s->pc - s->base.pc_next)); > break; > #ifdef WANT_ICEBP > @@ -7425,7 +7429,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) > case 0x132: /* rdmsr */ > if (check_cpl0(s)) { > gen_update_cc_op(s); > - gen_jmp_im(s, s->base.pc_next - s->cs_base); > + gen_update_eip_cur(s); > if (b & 2) { > gen_helper_rdmsr(cpu_env); > } else { > @@ -7437,7 +7441,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) > break; > case 0x131: /* rdtsc */ > gen_update_cc_op(s); > - gen_jmp_im(s, s->base.pc_next - s->cs_base); > + gen_update_eip_cur(s); > if (tb_cflags(s->base.tb) & CF_USE_ICOUNT) { > gen_io_start(); > } > @@ -7448,7 +7452,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) > break; > case 0x133: /* rdpmc */ > gen_update_cc_op(s); > - gen_jmp_im(s, s->base.pc_next - s->cs_base); > + gen_update_eip_cur(s); > gen_helper_rdpmc(cpu_env); > s->base.is_jmp = DISAS_NORETURN; > break; > @@ -7478,7 +7482,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) > case 0x105: /* syscall */ > /* XXX: is it usable in real mode ? */ > gen_update_cc_op(s); > - gen_jmp_im(s, s->base.pc_next - s->cs_base); > + gen_update_eip_cur(s); > gen_helper_syscall(cpu_env, tcg_const_i32(s->pc - s->base.pc_next)); > /* TF handling for the syscall insn is different. The TF bit is > checked > after the syscall insn completes. This allows #DB to not be > @@ -7504,13 +7508,13 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) > #endif > case 0x1a2: /* cpuid */ > gen_update_cc_op(s); > - gen_jmp_im(s, s->base.pc_next - s->cs_base); > + gen_update_eip_cur(s); > gen_helper_cpuid(cpu_env); > break; > case 0xf4: /* hlt */ > if (check_cpl0(s)) { > gen_update_cc_op(s); > - gen_jmp_im(s, s->base.pc_next - s->cs_base); > + gen_update_eip_cur(s); > gen_helper_hlt(cpu_env, tcg_const_i32(s->pc - s->base.pc_next)); > s->base.is_jmp = DISAS_NORETURN; > } > @@ -7607,7 +7611,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) > goto illegal_op; > } > gen_update_cc_op(s); > - gen_jmp_im(s, s->base.pc_next - s->cs_base); > + gen_update_eip_cur(s); > tcg_gen_mov_tl(s->A0, cpu_regs[R_EAX]); > gen_extu(s->aflag, s->A0); > gen_add_A0_ds_seg(s); > @@ -7619,7 +7623,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) > goto illegal_op; > } > gen_update_cc_op(s); > - gen_jmp_im(s, s->base.pc_next - s->cs_base); > + gen_update_eip_cur(s); > gen_helper_mwait(cpu_env, tcg_const_i32(s->pc - > s->base.pc_next)); > s->base.is_jmp = DISAS_NORETURN; > break; > @@ -7697,7 +7701,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) > break; > } > gen_update_cc_op(s); > - gen_jmp_im(s, s->base.pc_next - s->cs_base); > + gen_update_eip_cur(s); > gen_helper_vmrun(cpu_env, tcg_const_i32(s->aflag - 1), > tcg_const_i32(s->pc - s->base.pc_next)); > tcg_gen_exit_tb(NULL, 0); > @@ -7709,7 +7713,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) > goto illegal_op; > } > gen_update_cc_op(s); > - gen_jmp_im(s, s->base.pc_next - s->cs_base); > + gen_update_eip_cur(s); > gen_helper_vmmcall(cpu_env); > break; > > @@ -7721,7 +7725,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) > break; > } > gen_update_cc_op(s); > - gen_jmp_im(s, s->base.pc_next - s->cs_base); > + gen_update_eip_cur(s); > gen_helper_vmload(cpu_env, tcg_const_i32(s->aflag - 1)); > break; > > @@ -7733,7 +7737,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) > break; > } > gen_update_cc_op(s); > - gen_jmp_im(s, s->base.pc_next - s->cs_base); > + gen_update_eip_cur(s); > gen_helper_vmsave(cpu_env, tcg_const_i32(s->aflag - 1)); > break; > > @@ -7759,7 +7763,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) > break; > } > gen_update_cc_op(s); > - gen_jmp_im(s, s->base.pc_next - s->cs_base); > + gen_update_eip_cur(s); > gen_helper_clgi(cpu_env); > break; > > @@ -7905,7 +7909,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) > goto illegal_op; > } > gen_update_cc_op(s); > - gen_jmp_im(s, s->base.pc_next - s->cs_base); > + gen_update_eip_cur(s); > if (tb_cflags(s->base.tb) & CF_USE_ICOUNT) { > gen_io_start(); > } > @@ -8818,7 +8822,7 @@ static void i386_tr_tb_stop(DisasContextBase *dcbase, > CPUState *cpu) > DisasContext *dc = container_of(dcbase, DisasContext, base); > > if (dc->base.is_jmp == DISAS_TOO_MANY) { > - gen_jmp_im(dc, dc->base.pc_next - dc->cs_base); > + gen_update_eip_cur(dc); > gen_eob(dc); > } > } > -- > 2.34.1 >