| Message ID | 20240604071833.962574-5-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/i386: fixes for INHIBIT_IRQ, TF and RF | expand |
On 6/4/24 02:18, Paolo Bonzini wrote: > Use decode.c's support for intercepts, doing the check in TCG-generated > code rather than the helper. This is cleaner because it allows removing > the eip_addend argument to helper_pause(), even though it adds a bit of > bloat for opcode 0x90's new decoding function. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > target/i386/helper.h | 2 +- > target/i386/tcg/helper-tcg.h | 1 - > target/i386/tcg/misc_helper.c | 10 +--------- > target/i386/tcg/sysemu/misc_helper.c | 2 +- > target/i386/tcg/decode-new.c.inc | 15 ++++++++++++++- > target/i386/tcg/emit.c.inc | 20 ++++++++------------ > 6 files changed, 25 insertions(+), 25 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > +static void decode_90(DisasContext *s, CPUX86State *env, X86OpEntry *entry, uint8_t *b) > +{ > + static X86OpEntry pause = X86_OP_ENTRY0(PAUSE, svm(PAUSE)); > + static X86OpEntry nop = X86_OP_ENTRY0(NOP); > + static X86OpEntry xchg_ax = X86_OP_ENTRY2(XCHG, 0,v, LoBits,v); > + > + if (REX_B(s)) { > + *entry = xchg_ax; > + } else { > + *entry = (s->prefix & PREFIX_REPZ) ? pause : nop; > + } > +} Thanks. I had wished for this instead of > static void gen_XCHG(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode) > { > - if (decode->b == 0x90 && !REX_B(s)) { > - if (s->prefix & PREFIX_REPZ) { > - gen_update_cc_op(s); > - gen_update_eip_cur(s); > - gen_helper_pause(tcg_env, cur_insn_len_i32(s)); > - s->base.is_jmp = DISAS_NORETURN; > - } > - /* No writeback. */ > - decode->op[0].unit = X86_OP_SKIP; > - return; > - } this from the beginning, but since it wasn't wrong, I didn't mention it. r~
On Tue, Jun 4, 2024 at 12:59 PM Richard Henderson <richard.henderson@linaro.org> wrote: > > On 6/4/24 02:18, Paolo Bonzini wrote: > > Use decode.c's support for intercepts, doing the check in TCG-generated > > code rather than the helper. This is cleaner because it allows removing > > the eip_addend argument to helper_pause(), even though it adds a bit of > > bloat for opcode 0x90's new decoding function. > > > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > --- > > target/i386/helper.h | 2 +- > > target/i386/tcg/helper-tcg.h | 1 - > > target/i386/tcg/misc_helper.c | 10 +--------- > > target/i386/tcg/sysemu/misc_helper.c | 2 +- > > target/i386/tcg/decode-new.c.inc | 15 ++++++++++++++- > > target/i386/tcg/emit.c.inc | 20 ++++++++------------ > > 6 files changed, 25 insertions(+), 25 deletions(-) > > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > > > +static void decode_90(DisasContext *s, CPUX86State *env, X86OpEntry *entry, uint8_t *b) > > +{ > > + static X86OpEntry pause = X86_OP_ENTRY0(PAUSE, svm(PAUSE)); > > + static X86OpEntry nop = X86_OP_ENTRY0(NOP); > > + static X86OpEntry xchg_ax = X86_OP_ENTRY2(XCHG, 0,v, LoBits,v); > > + > > + if (REX_B(s)) { > > + *entry = xchg_ax; > > + } else { > > + *entry = (s->prefix & PREFIX_REPZ) ? pause : nop; > > + } > > +} > > Thanks. I had wished for this > from the beginning, but since it wasn't wrong, I didn't mention it. Yeah, I was still making up my mind on the style. Using set_cc_op() for BMI instructions was also silly, I'll fix it soon. Basically I want to get to a point where the common code and the helpers are all set for implementing APX, then we'll see. Paolo
diff --git a/target/i386/helper.h b/target/i386/helper.h index c244dbb4812..2f46cffabd8 100644 --- a/target/i386/helper.h +++ b/target/i386/helper.h @@ -53,7 +53,7 @@ DEF_HELPER_1(sysenter, void, env) DEF_HELPER_2(sysexit, void, env, int) DEF_HELPER_2(syscall, void, env, int) DEF_HELPER_2(sysret, void, env, int) -DEF_HELPER_FLAGS_2(pause, TCG_CALL_NO_WG, noreturn, env, int) +DEF_HELPER_FLAGS_1(pause, TCG_CALL_NO_WG, noreturn, env) DEF_HELPER_FLAGS_3(raise_interrupt, TCG_CALL_NO_WG, noreturn, env, int, int) DEF_HELPER_FLAGS_2(raise_exception, TCG_CALL_NO_WG, noreturn, env, int) DEF_HELPER_FLAGS_1(icebp, TCG_CALL_NO_WG, noreturn, env) diff --git a/target/i386/tcg/helper-tcg.h b/target/i386/tcg/helper-tcg.h index 6a5505e7b4c..43180b58314 100644 --- a/target/i386/tcg/helper-tcg.h +++ b/target/i386/tcg/helper-tcg.h @@ -91,7 +91,6 @@ extern const uint8_t parity_table[256]; /* misc_helper.c */ void cpu_load_eflags(CPUX86State *env, int eflags, int update_mask); -G_NORETURN void do_pause(CPUX86State *env); /* sysemu/svm_helper.c */ #ifndef CONFIG_USER_ONLY diff --git a/target/i386/tcg/misc_helper.c b/target/i386/tcg/misc_helper.c index b0f0f7b893b..8316d42ffcd 100644 --- a/target/i386/tcg/misc_helper.c +++ b/target/i386/tcg/misc_helper.c @@ -88,7 +88,7 @@ G_NORETURN void helper_rdpmc(CPUX86State *env) raise_exception_err(env, EXCP06_ILLOP, 0); } -G_NORETURN void do_pause(CPUX86State *env) +G_NORETURN void helper_pause(CPUX86State *env) { CPUState *cs = env_cpu(env); @@ -97,14 +97,6 @@ G_NORETURN void do_pause(CPUX86State *env) cpu_loop_exit(cs); } -G_NORETURN void helper_pause(CPUX86State *env, int next_eip_addend) -{ - cpu_svm_check_intercept_param(env, SVM_EXIT_PAUSE, 0, GETPC()); - env->eip += next_eip_addend; - - do_pause(env); -} - uint64_t helper_rdpkru(CPUX86State *env, uint32_t ecx) { if ((env->cr[4] & CR4_PKE_MASK) == 0) { diff --git a/target/i386/tcg/sysemu/misc_helper.c b/target/i386/tcg/sysemu/misc_helper.c index e41c88346cb..093cc2d0f90 100644 --- a/target/i386/tcg/sysemu/misc_helper.c +++ b/target/i386/tcg/sysemu/misc_helper.c @@ -547,7 +547,7 @@ G_NORETURN void helper_mwait(CPUX86State *env, int next_eip_addend) /* XXX: not complete but not completely erroneous */ if (cs->cpu_index != 0 || CPU_NEXT(cs) != NULL) { - do_pause(env); + helper_pause(env); } else { helper_hlt(env); } diff --git a/target/i386/tcg/decode-new.c.inc b/target/i386/tcg/decode-new.c.inc index 376d2bdabe1..c2d8da8d14e 100644 --- a/target/i386/tcg/decode-new.c.inc +++ b/target/i386/tcg/decode-new.c.inc @@ -1359,6 +1359,19 @@ static void decode_group11(DisasContext *s, CPUX86State *env, X86OpEntry *entry, } } +static void decode_90(DisasContext *s, CPUX86State *env, X86OpEntry *entry, uint8_t *b) +{ + static X86OpEntry pause = X86_OP_ENTRY0(PAUSE, svm(PAUSE)); + static X86OpEntry nop = X86_OP_ENTRY0(NOP); + static X86OpEntry xchg_ax = X86_OP_ENTRY2(XCHG, 0,v, LoBits,v); + + if (REX_B(s)) { + *entry = xchg_ax; + } else { + *entry = (s->prefix & PREFIX_REPZ) ? pause : nop; + } +} + static const X86OpEntry opcodes_root[256] = { [0x00] = X86_OP_ENTRY2(ADD, E,b, G,b, lock), [0x01] = X86_OP_ENTRY2(ADD, E,v, G,v, lock), @@ -1441,7 +1454,7 @@ static const X86OpEntry opcodes_root[256] = { [0x86] = X86_OP_ENTRY2(XCHG, E,b, G,b, xchg), [0x87] = X86_OP_ENTRY2(XCHG, E,v, G,v, xchg), - [0x90] = X86_OP_ENTRY2(XCHG, 0,v, LoBits,v), + [0x90] = X86_OP_GROUP0(90), [0x91] = X86_OP_ENTRY2(XCHG, 0,v, LoBits,v), [0x92] = X86_OP_ENTRY2(XCHG, 0,v, LoBits,v), [0x93] = X86_OP_ENTRY2(XCHG, 0,v, LoBits,v), diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc index 2e94e8ec56f..f90f3d3c589 100644 --- a/target/i386/tcg/emit.c.inc +++ b/target/i386/tcg/emit.c.inc @@ -2350,6 +2350,14 @@ static void gen_PANDN(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode) decode->op[1].offset, vec_len, vec_len); } +static void gen_PAUSE(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode) +{ + gen_update_cc_op(s); + gen_update_eip_next(s); + gen_helper_pause(tcg_env); + s->base.is_jmp = DISAS_NORETURN; +} + static void gen_PCMPESTRI(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode) { TCGv_i32 imm = tcg_constant8u_i32(decode->immediate); @@ -4014,18 +4022,6 @@ static void gen_WAIT(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode) static void gen_XCHG(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode) { - if (decode->b == 0x90 && !REX_B(s)) { - if (s->prefix & PREFIX_REPZ) { - gen_update_cc_op(s); - gen_update_eip_cur(s); - gen_helper_pause(tcg_env, cur_insn_len_i32(s)); - s->base.is_jmp = DISAS_NORETURN; - } - /* No writeback. */ - decode->op[0].unit = X86_OP_SKIP; - return; - } - if (s->prefix & PREFIX_LOCK) { tcg_gen_atomic_xchg_tl(s->T0, s->A0, s->T1, s->mem_index, decode->op[0].ot | MO_LE);
Use decode.c's support for intercepts, doing the check in TCG-generated code rather than the helper. This is cleaner because it allows removing the eip_addend argument to helper_pause(), even though it adds a bit of bloat for opcode 0x90's new decoding function. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- target/i386/helper.h | 2 +- target/i386/tcg/helper-tcg.h | 1 - target/i386/tcg/misc_helper.c | 10 +--------- target/i386/tcg/sysemu/misc_helper.c | 2 +- target/i386/tcg/decode-new.c.inc | 15 ++++++++++++++- target/i386/tcg/emit.c.inc | 20 ++++++++------------ 6 files changed, 25 insertions(+), 25 deletions(-)