[PATCH 02/11] target/i386: fix implementation of ICEBP

Paolo Bonzini posted 11 patches 5 months, 3 weeks ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>
[PATCH 02/11] target/i386: fix implementation of ICEBP
Posted by Paolo Bonzini 5 months, 3 weeks ago
ICEBP generates a trap-like exception, while gen_exception() produces
a fault.  Resurrect gen_update_eip_next() to implement the desired
semantics.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/helper.h          |  1 +
 target/i386/tcg/helper-tcg.h  |  3 +++
 target/i386/tcg/bpt_helper.c  |  6 ++++++
 target/i386/tcg/excp_helper.c | 20 ++++++++++++++++++++
 target/i386/tcg/translate.c   | 13 +++++++++++++
 target/i386/tcg/emit.c.inc    |  5 ++++-
 6 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/target/i386/helper.h b/target/i386/helper.h
index a52a1bf0f21..8f291a5f66f 100644
--- a/target/i386/helper.h
+++ b/target/i386/helper.h
@@ -56,6 +56,7 @@ DEF_HELPER_2(sysret, void, env, int)
 DEF_HELPER_FLAGS_2(pause, TCG_CALL_NO_WG, noreturn, env, int)
 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)
 DEF_HELPER_3(boundw, void, env, tl, int)
 DEF_HELPER_3(boundl, void, env, tl, int)
 
diff --git a/target/i386/tcg/helper-tcg.h b/target/i386/tcg/helper-tcg.h
index effc2c1c984..6a5505e7b4c 100644
--- a/target/i386/tcg/helper-tcg.h
+++ b/target/i386/tcg/helper-tcg.h
@@ -112,6 +112,9 @@ int exception_has_error_code(int intno);
 void do_smm_enter(X86CPU *cpu);
 
 /* bpt_helper.c */
+void do_end_instruction(CPUX86State *env);
+
+/* sysemu/bpt_helper.c */
 bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update);
 
 #endif /* I386_HELPER_TCG_H */
diff --git a/target/i386/tcg/bpt_helper.c b/target/i386/tcg/bpt_helper.c
index bc34ac27fea..9695b9dd041 100644
--- a/target/i386/tcg/bpt_helper.c
+++ b/target/i386/tcg/bpt_helper.c
@@ -37,3 +37,9 @@ void helper_rechecking_single_step(CPUX86State *env)
         helper_single_step(env);
     }
 }
+
+void do_end_instruction(CPUX86State *env)
+{
+    env->hflags &= ~HF_INHIBIT_IRQ_MASK; /* needed if sti is just before */
+    env->eflags &= ~HF_RF_MASK;
+}
diff --git a/target/i386/tcg/excp_helper.c b/target/i386/tcg/excp_helper.c
index 65e37ae2a0c..72387aac24f 100644
--- a/target/i386/tcg/excp_helper.c
+++ b/target/i386/tcg/excp_helper.c
@@ -140,6 +140,26 @@ G_NORETURN void raise_exception_ra(CPUX86State *env, int exception_index,
     raise_interrupt2(env, exception_index, 0, 0, 0, retaddr);
 }
 
+G_NORETURN void helper_icebp(CPUX86State *env)
+{
+    CPUState *cs = env_cpu(env);
+
+    do_end_instruction(env);
+
+    /*
+     * INT1 aka ICEBP generates a trap-like #DB, but it is pretty special.
+     *
+     * "Although the ICEBP instruction dispatches through IDT vector 1,
+     * that event is not interceptable by means of the #DB exception
+     * intercept".  Instead there is a separate fault-like ICEBP intercept.
+     */
+    cs->exception_index = EXCP01_DB;
+    env->error_code = 0;
+    env->exception_is_int = 0;
+    env->exception_next_eip = env->eip;
+    cpu_loop_exit(cs);
+}
+
 G_NORETURN void handle_unaligned_access(CPUX86State *env, vaddr vaddr,
                                         MMUAccessType access_type,
                                         uintptr_t retaddr)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index d438f8f76f7..77ed9c1db47 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -549,6 +549,19 @@ static inline void gen_op_st_rm_T0_A0(DisasContext *s, int idx, int d)
     }
 }
 
+static void gen_update_eip_next(DisasContext *s)
+{
+    assert(s->pc_save != -1);
+    if (tb_cflags(s->base.tb) & CF_PCREL) {
+        tcg_gen_addi_tl(cpu_eip, cpu_eip, s->pc - s->pc_save);
+    } else if (CODE64(s)) {
+        tcg_gen_movi_tl(cpu_eip, s->pc);
+    } else {
+        tcg_gen_movi_tl(cpu_eip, (uint32_t)(s->pc - s->cs_base));
+    }
+    s->pc_save = s->pc;
+}
+
 static void gen_update_eip_cur(DisasContext *s)
 {
     assert(s->pc_save != -1);
diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
index e990141454b..36127d99943 100644
--- a/target/i386/tcg/emit.c.inc
+++ b/target/i386/tcg/emit.c.inc
@@ -1858,7 +1858,10 @@ static void gen_INT(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
 
 static void gen_INT1(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
 {
-    gen_exception(s, EXCP01_DB);
+    gen_update_cc_op(s);
+    gen_update_eip_next(s);
+    gen_helper_icebp(tcg_env);
+    s->base.is_jmp = DISAS_NORETURN;
 }
 
 static void gen_INT3(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
-- 
2.45.1
Re: [PATCH 02/11] target/i386: fix implementation of ICEBP
Posted by Richard Henderson 5 months, 3 weeks ago
On 6/4/24 02:18, Paolo Bonzini wrote:
> ICEBP generates a trap-like exception, while gen_exception() produces
> a fault.  Resurrect gen_update_eip_next() to implement the desired
> semantics.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   target/i386/helper.h          |  1 +
>   target/i386/tcg/helper-tcg.h  |  3 +++
>   target/i386/tcg/bpt_helper.c  |  6 ++++++
>   target/i386/tcg/excp_helper.c | 20 ++++++++++++++++++++
>   target/i386/tcg/translate.c   | 13 +++++++++++++
>   target/i386/tcg/emit.c.inc    |  5 ++++-
>   6 files changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/helper.h b/target/i386/helper.h
> index a52a1bf0f21..8f291a5f66f 100644
> --- a/target/i386/helper.h
> +++ b/target/i386/helper.h
> @@ -56,6 +56,7 @@ DEF_HELPER_2(sysret, void, env, int)
>   DEF_HELPER_FLAGS_2(pause, TCG_CALL_NO_WG, noreturn, env, int)
>   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)
>   DEF_HELPER_3(boundw, void, env, tl, int)
>   DEF_HELPER_3(boundl, void, env, tl, int)
>   
> diff --git a/target/i386/tcg/helper-tcg.h b/target/i386/tcg/helper-tcg.h
> index effc2c1c984..6a5505e7b4c 100644
> --- a/target/i386/tcg/helper-tcg.h
> +++ b/target/i386/tcg/helper-tcg.h
> @@ -112,6 +112,9 @@ int exception_has_error_code(int intno);
>   void do_smm_enter(X86CPU *cpu);
>   
>   /* bpt_helper.c */
> +void do_end_instruction(CPUX86State *env);
> +
> +/* sysemu/bpt_helper.c */
>   bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update);
>   
>   #endif /* I386_HELPER_TCG_H */
> diff --git a/target/i386/tcg/bpt_helper.c b/target/i386/tcg/bpt_helper.c
> index bc34ac27fea..9695b9dd041 100644
> --- a/target/i386/tcg/bpt_helper.c
> +++ b/target/i386/tcg/bpt_helper.c
> @@ -37,3 +37,9 @@ void helper_rechecking_single_step(CPUX86State *env)
>           helper_single_step(env);
>       }
>   }
> +
> +void do_end_instruction(CPUX86State *env)
> +{
> +    env->hflags &= ~HF_INHIBIT_IRQ_MASK; /* needed if sti is just before */
> +    env->eflags &= ~HF_RF_MASK;
> +}

Two and insns.  Perhaps place as static inline in helper-tcg.h?

Either way,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~