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(-)
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);
--
2.45.1On 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
© 2016 - 2024 Red Hat, Inc.