[PATCH 0/9] accel/tcg: Extract some x86-specific code

[PATCH 0/9] accel/tcg: Extract some x86-specific code

[Philippe Mathieu-Daudé]

accel/tcg/ ought to be target agnostic. This series remove
some x86 code, addressing part of "Work still remains" from
Anjo's series:
https://lore.kernel.org/qemu-devel/20240119144024.14289-1-anjo@rev.ng/

Based-on: <20240124075609.14756-1-philmd@linaro.org>
          "Move perf and debuginfo support to tcg"

Philippe Mathieu-Daudé (9):
  accel/tcg: Rename tcg_ss[] -> tcg_specific_ss[] in meson
  accel/tcg: Rename tcg_cpus_destroy() -> tcg_cpu_destroy()
  accel/tcg: Rename tcg_cpus_exec() -> tcg_cpu_exec()
  accel/tcg: Un-inline icount_exit_request() for clarity
  accel/tcg: Hoist CPUClass arg to functions with external linkage
  accel/tcg: Introduce TCGCPUOps::need_replay_interrupt() handler
  target/i386: Extract x86_need_replay_interrupt() from accel/tcg/
  accel/tcg: Introduce TCGCPUOps::cpu_exec_halt() handler
  target/i386: Extract x86_cpu_exec_halt() from accel/tcg/

 accel/tcg/tcg-accel-ops.h           |   4 +-
 include/hw/core/tcg-cpu-ops.h       |   7 ++
 target/i386/tcg/helper-tcg.h        |   2 +
 accel/tcg/cpu-exec.c                | 125 ++++++++++++----------------
 accel/tcg/tcg-accel-ops-mttcg.c     |   4 +-
 accel/tcg/tcg-accel-ops-rr.c        |   4 +-
 accel/tcg/tcg-accel-ops.c           |   4 +-
 target/i386/tcg/sysemu/seg_helper.c |  23 +++++
 target/i386/tcg/tcg-cpu.c           |   2 +
 accel/tcg/meson.build               |  12 +--
 10 files changed, 103 insertions(+), 84 deletions(-)

-- 
2.41.0

[PATCH 1/9] accel/tcg: Rename tcg_ss[] -> tcg_specific_ss[] in meson

[Philippe Mathieu-Daudé]

tcg_ss[] source set contains target-specific units.
Rename it as 'tcg_specific_ss[]' for clarity.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/tcg/meson.build | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/accel/tcg/meson.build b/accel/tcg/meson.build
index 46f7d53eeb..aef80de967 100644
--- a/accel/tcg/meson.build
+++ b/accel/tcg/meson.build
@@ -1,8 +1,8 @@
-tcg_ss = ss.source_set()
 common_ss.add(when: 'CONFIG_TCG', if_true: files(
   'cpu-exec-common.c',
 ))
-tcg_ss.add(files(
+tcg_specific_ss = ss.source_set()
+tcg_specific_ss.add(files(
   'tcg-all.c',
   'cpu-exec.c',
   'tb-maint.c',
@@ -11,12 +11,12 @@ tcg_ss.add(files(
   'translate-all.c',
   'translator.c',
 ))
-tcg_ss.add(when: 'CONFIG_USER_ONLY', if_true: files('user-exec.c'))
-tcg_ss.add(when: 'CONFIG_SYSTEM_ONLY', if_false: files('user-exec-stub.c'))
+tcg_specific_ss.add(when: 'CONFIG_USER_ONLY', if_true: files('user-exec.c'))
+tcg_specific_ss.add(when: 'CONFIG_SYSTEM_ONLY', if_false: files('user-exec-stub.c'))
 if get_option('plugins')
-  tcg_ss.add(files('plugin-gen.c'))
+  tcg_specific_ss.add(files('plugin-gen.c'))
 endif
-specific_ss.add_all(when: 'CONFIG_TCG', if_true: tcg_ss)
+specific_ss.add_all(when: 'CONFIG_TCG', if_true: tcg_specific_ss)
 
 specific_ss.add(when: ['CONFIG_SYSTEM_ONLY', 'CONFIG_TCG'], if_true: files(
   'cputlb.c',
-- 
2.41.0

[PATCH 2/9] accel/tcg: Rename tcg_cpus_destroy() -> tcg_cpu_destroy()

[Philippe Mathieu-Daudé]

tcg_cpus_destroy() operates on a single vCPU, rename it
as 'tcg_cpu_destroy'.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/tcg/tcg-accel-ops.h       | 2 +-
 accel/tcg/tcg-accel-ops-mttcg.c | 2 +-
 accel/tcg/tcg-accel-ops-rr.c    | 2 +-
 accel/tcg/tcg-accel-ops.c       | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/accel/tcg/tcg-accel-ops.h b/accel/tcg/tcg-accel-ops.h
index f9bc6330e2..17c7ed00eb 100644
--- a/accel/tcg/tcg-accel-ops.h
+++ b/accel/tcg/tcg-accel-ops.h
@@ -14,7 +14,7 @@
 
 #include "sysemu/cpus.h"
 
-void tcg_cpus_destroy(CPUState *cpu);
+void tcg_cpu_destroy(CPUState *cpu);
 int tcg_cpus_exec(CPUState *cpu);
 void tcg_handle_interrupt(CPUState *cpu, int mask);
 void tcg_cpu_init_cflags(CPUState *cpu, bool parallel);
diff --git a/accel/tcg/tcg-accel-ops-mttcg.c b/accel/tcg/tcg-accel-ops-mttcg.c
index af7307013a..bcba314a65 100644
--- a/accel/tcg/tcg-accel-ops-mttcg.c
+++ b/accel/tcg/tcg-accel-ops-mttcg.c
@@ -118,7 +118,7 @@ static void *mttcg_cpu_thread_fn(void *arg)
         qemu_wait_io_event(cpu);
     } while (!cpu->unplug || cpu_can_run(cpu));
 
-    tcg_cpus_destroy(cpu);
+    tcg_cpu_destroy(cpu);
     bql_unlock();
     rcu_remove_force_rcu_notifier(&force_rcu.notifier);
     rcu_unregister_thread();
diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
index 3208035d85..0617f66b5b 100644
--- a/accel/tcg/tcg-accel-ops-rr.c
+++ b/accel/tcg/tcg-accel-ops-rr.c
@@ -131,7 +131,7 @@ static void rr_deal_with_unplugged_cpus(void)
 
     CPU_FOREACH(cpu) {
         if (cpu->unplug && !cpu_can_run(cpu)) {
-            tcg_cpus_destroy(cpu);
+            tcg_cpu_destroy(cpu);
             break;
         }
     }
diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
index 813065c0ec..9b84b84218 100644
--- a/accel/tcg/tcg-accel-ops.c
+++ b/accel/tcg/tcg-accel-ops.c
@@ -63,7 +63,7 @@ void tcg_cpu_init_cflags(CPUState *cpu, bool parallel)
     cpu->tcg_cflags |= cflags;
 }
 
-void tcg_cpus_destroy(CPUState *cpu)
+void tcg_cpu_destroy(CPUState *cpu)
 {
     cpu_thread_signal_destroyed(cpu);
 }
-- 
2.41.0

[PATCH 3/9] accel/tcg: Rename tcg_cpus_exec() -> tcg_cpu_exec()

[Philippe Mathieu-Daudé]

tcg_cpus_exec() operates on a single vCPU, rename it
as 'tcg_cpu_exec'.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/tcg/tcg-accel-ops.h       | 2 +-
 accel/tcg/tcg-accel-ops-mttcg.c | 2 +-
 accel/tcg/tcg-accel-ops-rr.c    | 2 +-
 accel/tcg/tcg-accel-ops.c       | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/accel/tcg/tcg-accel-ops.h b/accel/tcg/tcg-accel-ops.h
index 17c7ed00eb..44c4079972 100644
--- a/accel/tcg/tcg-accel-ops.h
+++ b/accel/tcg/tcg-accel-ops.h
@@ -15,7 +15,7 @@
 #include "sysemu/cpus.h"
 
 void tcg_cpu_destroy(CPUState *cpu);
-int tcg_cpus_exec(CPUState *cpu);
+int tcg_cpu_exec(CPUState *cpu);
 void tcg_handle_interrupt(CPUState *cpu, int mask);
 void tcg_cpu_init_cflags(CPUState *cpu, bool parallel);
 
diff --git a/accel/tcg/tcg-accel-ops-mttcg.c b/accel/tcg/tcg-accel-ops-mttcg.c
index bcba314a65..c552b45b8e 100644
--- a/accel/tcg/tcg-accel-ops-mttcg.c
+++ b/accel/tcg/tcg-accel-ops-mttcg.c
@@ -92,7 +92,7 @@ static void *mttcg_cpu_thread_fn(void *arg)
         if (cpu_can_run(cpu)) {
             int r;
             bql_unlock();
-            r = tcg_cpus_exec(cpu);
+            r = tcg_cpu_exec(cpu);
             bql_lock();
             switch (r) {
             case EXCP_DEBUG:
diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
index 0617f66b5b..894e73e52c 100644
--- a/accel/tcg/tcg-accel-ops-rr.c
+++ b/accel/tcg/tcg-accel-ops-rr.c
@@ -258,7 +258,7 @@ static void *rr_cpu_thread_fn(void *arg)
                 if (icount_enabled()) {
                     icount_prepare_for_run(cpu, cpu_budget);
                 }
-                r = tcg_cpus_exec(cpu);
+                r = tcg_cpu_exec(cpu);
                 if (icount_enabled()) {
                     icount_process_data(cpu);
                 }
diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
index 9b84b84218..9c957f421c 100644
--- a/accel/tcg/tcg-accel-ops.c
+++ b/accel/tcg/tcg-accel-ops.c
@@ -68,7 +68,7 @@ void tcg_cpu_destroy(CPUState *cpu)
     cpu_thread_signal_destroyed(cpu);
 }
 
-int tcg_cpus_exec(CPUState *cpu)
+int tcg_cpu_exec(CPUState *cpu)
 {
     int ret;
     assert(tcg_enabled());
-- 
2.41.0

[PATCH 4/9] accel/tcg: Un-inline icount_exit_request() for clarity

[Philippe Mathieu-Daudé]

Convert packed logic to dumb icount_exit_request() helper.
No functional change intended.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/tcg/cpu-exec.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 6b3f66930e..d61b285d5e 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -791,6 +791,17 @@ static inline bool need_replay_interrupt(int interrupt_request)
 }
 #endif /* !CONFIG_USER_ONLY */
 
+static inline bool icount_exit_request(CPUState *cpu)
+{
+    if (!icount_enabled()) {
+        return false;
+    }
+    if (cpu->cflags_next_tb != -1 && !(cpu->cflags_next_tb & CF_USE_ICOUNT)) {
+        return false;
+    }
+    return cpu->neg.icount_decr.u16.low + cpu->icount_extra == 0;
+}
+
 static inline bool cpu_handle_interrupt(CPUState *cpu,
                                         TranslationBlock **last_tb)
 {
@@ -896,10 +907,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
     }
 
     /* Finally, check if we need to exit to the main loop.  */
-    if (unlikely(qatomic_read(&cpu->exit_request))
-        || (icount_enabled()
-            && (cpu->cflags_next_tb == -1 || cpu->cflags_next_tb & CF_USE_ICOUNT)
-            && cpu->neg.icount_decr.u16.low + cpu->icount_extra == 0)) {
+    if (unlikely(qatomic_read(&cpu->exit_request)) || icount_exit_request(cpu)) {
         qatomic_set(&cpu->exit_request, 0);
         if (cpu->exception_index == -1) {
             cpu->exception_index = EXCP_INTERRUPT;
-- 
2.41.0

[PATCH 5/9] accel/tcg: Hoist CPUClass arg to functions with external linkage

[Philippe Mathieu-Daudé]

Hoist the CPUClass argument from most of these internal helpers:

 - check_for_breakpoints_slow
 - check_for_breakpoints()
 - cpu_tb_exec()
 - cpu_exec_enter()
 - cpu_exec_exit()
 - cpu_handle_halt()
 - cpu_handle_debug_exception()
 - cpu_handle_exception()
 - need_replay_interrupt()
 - cpu_handle_interrupt()
 - cpu_loop_exec_tb()
 - cpu_exec_loop()
 - cpu_exec_setjmp()

to the following ones with external linkage:

 - lookup_tb_ptr()
 - cpu_exec_step_atomic()
 - cpu_exec()

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/tcg/cpu-exec.c | 82 ++++++++++++++++++++------------------------
 1 file changed, 37 insertions(+), 45 deletions(-)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index d61b285d5e..b10472cbc7 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -324,8 +324,8 @@ static void log_cpu_exec(vaddr pc, CPUState *cpu,
     }
 }
 
-static bool check_for_breakpoints_slow(CPUState *cpu, vaddr pc,
-                                       uint32_t *cflags)
+static bool check_for_breakpoints_slow(CPUClass *cc, CPUState *cpu,
+                                       vaddr pc, uint32_t *cflags)
 {
     CPUBreakpoint *bp;
     bool match_page = false;
@@ -357,7 +357,6 @@ static bool check_for_breakpoints_slow(CPUState *cpu, vaddr pc,
 #ifdef CONFIG_USER_ONLY
                 g_assert_not_reached();
 #else
-                CPUClass *cc = CPU_GET_CLASS(cpu);
                 assert(cc->tcg_ops->debug_check_breakpoint);
                 match_bp = cc->tcg_ops->debug_check_breakpoint(cpu);
 #endif
@@ -390,11 +389,11 @@ static bool check_for_breakpoints_slow(CPUState *cpu, vaddr pc,
     return false;
 }
 
-static inline bool check_for_breakpoints(CPUState *cpu, vaddr pc,
-                                         uint32_t *cflags)
+static inline bool check_for_breakpoints(CPUClass *cc, CPUState *cpu,
+                                         vaddr pc, uint32_t *cflags)
 {
     return unlikely(!QTAILQ_EMPTY(&cpu->breakpoints)) &&
-        check_for_breakpoints_slow(cpu, pc, cflags);
+        check_for_breakpoints_slow(cc, cpu, pc, cflags);
 }
 
 /**
@@ -408,6 +407,7 @@ static inline bool check_for_breakpoints(CPUState *cpu, vaddr pc,
 const void *HELPER(lookup_tb_ptr)(CPUArchState *env)
 {
     CPUState *cpu = env_cpu(env);
+    CPUClass *cc = CPU_GET_CLASS(cpu);
     TranslationBlock *tb;
     vaddr pc;
     uint64_t cs_base;
@@ -416,7 +416,7 @@ const void *HELPER(lookup_tb_ptr)(CPUArchState *env)
     cpu_get_tb_cpu_state(env, &pc, &cs_base, &flags);
 
     cflags = curr_cflags(cpu);
-    if (check_for_breakpoints(cpu, pc, &cflags)) {
+    if (check_for_breakpoints(cc, cpu, pc, &cflags)) {
         cpu_loop_exit(cpu);
     }
 
@@ -443,7 +443,7 @@ const void *HELPER(lookup_tb_ptr)(CPUArchState *env)
  * affect the impact of CFI in environment with high security requirements
  */
 static inline TranslationBlock * QEMU_DISABLE_CFI
-cpu_tb_exec(CPUState *cpu, TranslationBlock *itb, int *tb_exit)
+cpu_tb_exec(CPUClass *cc, CPUState *cpu, TranslationBlock *itb, int *tb_exit)
 {
     CPUArchState *env = cpu_env(cpu);
     uintptr_t ret;
@@ -476,8 +476,6 @@ cpu_tb_exec(CPUState *cpu, TranslationBlock *itb, int *tb_exit)
          * counter hit zero); we must restore the guest PC to the address
          * of the start of the TB.
          */
-        CPUClass *cc = CPU_GET_CLASS(cpu);
-
         if (cc->tcg_ops->synchronize_from_tb) {
             cc->tcg_ops->synchronize_from_tb(cpu, last_tb);
         } else {
@@ -509,19 +507,15 @@ cpu_tb_exec(CPUState *cpu, TranslationBlock *itb, int *tb_exit)
 }
 
 
-static void cpu_exec_enter(CPUState *cpu)
+static void cpu_exec_enter(CPUClass *cc, CPUState *cpu)
 {
-    CPUClass *cc = CPU_GET_CLASS(cpu);
-
     if (cc->tcg_ops->cpu_exec_enter) {
         cc->tcg_ops->cpu_exec_enter(cpu);
     }
 }
 
-static void cpu_exec_exit(CPUState *cpu)
+static void cpu_exec_exit(CPUClass *cc, CPUState *cpu)
 {
-    CPUClass *cc = CPU_GET_CLASS(cpu);
-
     if (cc->tcg_ops->cpu_exec_exit) {
         cc->tcg_ops->cpu_exec_exit(cpu);
     }
@@ -566,6 +560,7 @@ static void cpu_exec_longjmp_cleanup(CPUState *cpu)
 
 void cpu_exec_step_atomic(CPUState *cpu)
 {
+    CPUClass *cc = CPU_GET_CLASS(cpu);
     CPUArchState *env = cpu_env(cpu);
     TranslationBlock *tb;
     vaddr pc;
@@ -600,11 +595,11 @@ void cpu_exec_step_atomic(CPUState *cpu)
             mmap_unlock();
         }
 
-        cpu_exec_enter(cpu);
+        cpu_exec_enter(cc, cpu);
         /* execute the generated code */
         trace_exec_tb(tb, pc);
-        cpu_tb_exec(cpu, tb, &tb_exit);
-        cpu_exec_exit(cpu);
+        cpu_tb_exec(cc, cpu, tb, &tb_exit);
+        cpu_exec_exit(cc, cpu);
     } else {
         cpu_exec_longjmp_cleanup(cpu);
     }
@@ -673,7 +668,7 @@ static inline void tb_add_jump(TranslationBlock *tb, int n,
     return;
 }
 
-static inline bool cpu_handle_halt(CPUState *cpu)
+static inline bool cpu_handle_halt(CPUClass *cc, CPUState *cpu)
 {
 #ifndef CONFIG_USER_ONLY
     if (cpu->halted) {
@@ -697,9 +692,8 @@ static inline bool cpu_handle_halt(CPUState *cpu)
     return false;
 }
 
-static inline void cpu_handle_debug_exception(CPUState *cpu)
+static inline void cpu_handle_debug_exception(CPUClass *cc, CPUState *cpu)
 {
-    CPUClass *cc = CPU_GET_CLASS(cpu);
     CPUWatchpoint *wp;
 
     if (!cpu->watchpoint_hit) {
@@ -713,7 +707,7 @@ static inline void cpu_handle_debug_exception(CPUState *cpu)
     }
 }
 
-static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
+static inline bool cpu_handle_exception(CPUClass *cc, CPUState *cpu, int *ret)
 {
     if (cpu->exception_index < 0) {
 #ifndef CONFIG_USER_ONLY
@@ -730,7 +724,7 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
         /* exit request from the cpu execution loop */
         *ret = cpu->exception_index;
         if (*ret == EXCP_DEBUG) {
-            cpu_handle_debug_exception(cpu);
+            cpu_handle_debug_exception(cc, cpu);
         }
         cpu->exception_index = -1;
         return true;
@@ -740,7 +734,6 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
            which will be handled outside the cpu execution
            loop */
 #if defined(TARGET_I386)
-        CPUClass *cc = CPU_GET_CLASS(cpu);
         cc->tcg_ops->fake_user_interrupt(cpu);
 #endif /* TARGET_I386 */
         *ret = cpu->exception_index;
@@ -748,7 +741,6 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
         return true;
 #else
         if (replay_exception()) {
-            CPUClass *cc = CPU_GET_CLASS(cpu);
             bql_lock();
             cc->tcg_ops->do_interrupt(cpu);
             bql_unlock();
@@ -761,7 +753,7 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
                  * next instruction.
                  */
                 *ret = EXCP_DEBUG;
-                cpu_handle_debug_exception(cpu);
+                cpu_handle_debug_exception(cc, cpu);
                 return true;
             }
         } else if (!replay_has_interrupt()) {
@@ -781,7 +773,7 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
  * "real" interrupt event later. It does not need to be recorded for
  * replay purposes.
  */
-static inline bool need_replay_interrupt(int interrupt_request)
+static inline bool need_replay_interrupt(CPUClass *cc, int interrupt_request)
 {
 #if defined(TARGET_I386)
     return !(interrupt_request & CPU_INTERRUPT_POLL);
@@ -802,7 +794,7 @@ static inline bool icount_exit_request(CPUState *cpu)
     return cpu->neg.icount_decr.u16.low + cpu->icount_extra == 0;
 }
 
-static inline bool cpu_handle_interrupt(CPUState *cpu,
+static inline bool cpu_handle_interrupt(CPUClass *cc, CPUState *cpu,
                                         TranslationBlock **last_tb)
 {
     /*
@@ -870,11 +862,9 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
            True when it is, and we should restart on a new TB,
            and via longjmp via cpu_loop_exit.  */
         else {
-            CPUClass *cc = CPU_GET_CLASS(cpu);
-
             if (cc->tcg_ops->cpu_exec_interrupt &&
                 cc->tcg_ops->cpu_exec_interrupt(cpu, interrupt_request)) {
-                if (need_replay_interrupt(interrupt_request)) {
+                if (need_replay_interrupt(cc, interrupt_request)) {
                     replay_interrupt();
                 }
                 /*
@@ -918,14 +908,15 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
     return false;
 }
 
-static inline void cpu_loop_exec_tb(CPUState *cpu, TranslationBlock *tb,
+static inline void cpu_loop_exec_tb(CPUClass *cc, CPUState *cpu,
+                                    TranslationBlock *tb,
                                     vaddr pc, TranslationBlock **last_tb,
                                     int *tb_exit)
 {
     int32_t insns_left;
 
     trace_exec_tb(tb, pc);
-    tb = cpu_tb_exec(cpu, tb, tb_exit);
+    tb = cpu_tb_exec(cc, cpu, tb, tb_exit);
     if (*tb_exit != TB_EXIT_REQUESTED) {
         *last_tb = tb;
         return;
@@ -970,16 +961,16 @@ static inline void cpu_loop_exec_tb(CPUState *cpu, TranslationBlock *tb,
 /* main execution loop */
 
 static int __attribute__((noinline))
-cpu_exec_loop(CPUState *cpu, SyncClocks *sc)
+cpu_exec_loop(CPUClass *cc, CPUState *cpu, SyncClocks *sc)
 {
     int ret;
 
     /* if an exception is pending, we execute it here */
-    while (!cpu_handle_exception(cpu, &ret)) {
+    while (!cpu_handle_exception(cc, cpu, &ret)) {
         TranslationBlock *last_tb = NULL;
         int tb_exit = 0;
 
-        while (!cpu_handle_interrupt(cpu, &last_tb)) {
+        while (!cpu_handle_interrupt(cc, cpu, &last_tb)) {
             TranslationBlock *tb;
             vaddr pc;
             uint64_t cs_base;
@@ -1001,7 +992,7 @@ cpu_exec_loop(CPUState *cpu, SyncClocks *sc)
                 cpu->cflags_next_tb = -1;
             }
 
-            if (check_for_breakpoints(cpu, pc, &cflags)) {
+            if (check_for_breakpoints(cc, cpu, pc, &cflags)) {
                 break;
             }
 
@@ -1046,7 +1037,7 @@ cpu_exec_loop(CPUState *cpu, SyncClocks *sc)
                 tb_add_jump(last_tb, tb_exit, tb);
             }
 
-            cpu_loop_exec_tb(cpu, tb, pc, &last_tb, &tb_exit);
+            cpu_loop_exec_tb(cc, cpu, tb, pc, &last_tb, &tb_exit);
 
             /* Try to align the host and virtual clocks
                if the guest is in advance */
@@ -1056,30 +1047,31 @@ cpu_exec_loop(CPUState *cpu, SyncClocks *sc)
     return ret;
 }
 
-static int cpu_exec_setjmp(CPUState *cpu, SyncClocks *sc)
+static int cpu_exec_setjmp(CPUClass *cc, CPUState *cpu, SyncClocks *sc)
 {
     /* Prepare setjmp context for exception handling. */
     if (unlikely(sigsetjmp(cpu->jmp_env, 0) != 0)) {
         cpu_exec_longjmp_cleanup(cpu);
     }
 
-    return cpu_exec_loop(cpu, sc);
+    return cpu_exec_loop(cc, cpu, sc);
 }
 
 int cpu_exec(CPUState *cpu)
 {
     int ret;
     SyncClocks sc = { 0 };
+    CPUClass *cc = CPU_GET_CLASS(cpu);
 
     /* replay_interrupt may need current_cpu */
     current_cpu = cpu;
 
-    if (cpu_handle_halt(cpu)) {
+    if (cpu_handle_halt(cc, cpu)) {
         return EXCP_HALTED;
     }
 
     WITH_RCU_READ_LOCK_GUARD() {
-        cpu_exec_enter(cpu);
+        cpu_exec_enter(cc, cpu);
 
         /*
          * Calculate difference between guest clock and host clock.
@@ -1089,9 +1081,9 @@ int cpu_exec(CPUState *cpu)
          */
         init_delay_params(&sc, cpu);
 
-        ret = cpu_exec_setjmp(cpu, &sc);
+        ret = cpu_exec_setjmp(cc, cpu, &sc);
 
-        cpu_exec_exit(cpu);
+        cpu_exec_exit(cc, cpu);
     };
 
     return ret;
-- 
2.41.0

[PATCH 6/9] accel/tcg: Introduce TCGCPUOps::need_replay_interrupt() handler

[Philippe Mathieu-Daudé]

In order to make accel/tcg/ target agnostic,
introduce the need_replay_interrupt() handler.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/core/tcg-cpu-ops.h | 5 +++++
 accel/tcg/cpu-exec.c          | 5 ++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
index 479713a36e..2fae3ac70f 100644
--- a/include/hw/core/tcg-cpu-ops.h
+++ b/include/hw/core/tcg-cpu-ops.h
@@ -170,6 +170,11 @@ struct TCGCPUOps {
      */
     bool (*io_recompile_replay_branch)(CPUState *cpu,
                                        const TranslationBlock *tb);
+    /**
+     * @need_replay_interrupt: Return %true if @interrupt_request
+     * needs to be recorded for replay purposes.
+     */
+    bool (*need_replay_interrupt)(int interrupt_request);
 #endif /* !CONFIG_USER_ONLY */
 #endif /* NEED_CPU_H */
 
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index b10472cbc7..4ab7d6c896 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -778,7 +778,10 @@ static inline bool need_replay_interrupt(CPUClass *cc, int interrupt_request)
 #if defined(TARGET_I386)
     return !(interrupt_request & CPU_INTERRUPT_POLL);
 #else
-    return true;
+    if (!cc->tcg_ops->need_replay_interrupt) {
+        return true;
+    }
+    return cc->tcg_ops->need_replay_interrupt(interrupt_request);
 #endif
 }
 #endif /* !CONFIG_USER_ONLY */
-- 
2.41.0

[PATCH 7/9] target/i386: Extract x86_need_replay_interrupt() from accel/tcg/

[Philippe Mathieu-Daudé]

Move this x86-specific code out of the generic accel/tcg/.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/i386/tcg/helper-tcg.h        |  1 +
 accel/tcg/cpu-exec.c                |  9 ---------
 target/i386/tcg/sysemu/seg_helper.c | 10 ++++++++++
 target/i386/tcg/tcg-cpu.c           |  1 +
 4 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/target/i386/tcg/helper-tcg.h b/target/i386/tcg/helper-tcg.h
index ce34b737bb..253b1f561e 100644
--- a/target/i386/tcg/helper-tcg.h
+++ b/target/i386/tcg/helper-tcg.h
@@ -39,6 +39,7 @@ QEMU_BUILD_BUG_ON(TCG_PHYS_ADDR_BITS > TARGET_PHYS_ADDR_SPACE_BITS);
  */
 void x86_cpu_do_interrupt(CPUState *cpu);
 #ifndef CONFIG_USER_ONLY
+bool x86_need_replay_interrupt(int interrupt_request);
 bool x86_cpu_exec_interrupt(CPUState *cpu, int int_req);
 #endif
 
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 4ab7d6c896..5a978a9e72 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -768,21 +768,12 @@ static inline bool cpu_handle_exception(CPUClass *cc, CPUState *cpu, int *ret)
 }
 
 #ifndef CONFIG_USER_ONLY
-/*
- * CPU_INTERRUPT_POLL is a virtual event which gets converted into a
- * "real" interrupt event later. It does not need to be recorded for
- * replay purposes.
- */
 static inline bool need_replay_interrupt(CPUClass *cc, int interrupt_request)
 {
-#if defined(TARGET_I386)
-    return !(interrupt_request & CPU_INTERRUPT_POLL);
-#else
     if (!cc->tcg_ops->need_replay_interrupt) {
         return true;
     }
     return cc->tcg_ops->need_replay_interrupt(interrupt_request);
-#endif
 }
 #endif /* !CONFIG_USER_ONLY */
 
diff --git a/target/i386/tcg/sysemu/seg_helper.c b/target/i386/tcg/sysemu/seg_helper.c
index 1cb5a0db45..e6f42282bb 100644
--- a/target/i386/tcg/sysemu/seg_helper.c
+++ b/target/i386/tcg/sysemu/seg_helper.c
@@ -127,6 +127,16 @@ void x86_cpu_do_interrupt(CPUState *cs)
     }
 }
 
+bool x86_need_replay_interrupt(int interrupt_request)
+{
+    /*
+     * CPU_INTERRUPT_POLL is a virtual event which gets converted into a
+     * "real" interrupt event later. It does not need to be recorded for
+     * replay purposes.
+     */
+    return !(interrupt_request & CPU_INTERRUPT_POLL);
+}
+
 bool x86_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
 {
     X86CPU *cpu = X86_CPU(cs);
diff --git a/target/i386/tcg/tcg-cpu.c b/target/i386/tcg/tcg-cpu.c
index e1405b7be9..255d56d4c3 100644
--- a/target/i386/tcg/tcg-cpu.c
+++ b/target/i386/tcg/tcg-cpu.c
@@ -123,6 +123,7 @@ static const struct TCGCPUOps x86_tcg_ops = {
     .do_unaligned_access = x86_cpu_do_unaligned_access,
     .debug_excp_handler = breakpoint_handler,
     .debug_check_breakpoint = x86_debug_check_breakpoint,
+    .need_replay_interrupt = x86_need_replay_interrupt,
 #endif /* !CONFIG_USER_ONLY */
 };
 
-- 
2.41.0

[PATCH 8/9] accel/tcg: Introduce TCGCPUOps::cpu_exec_halt() handler

[Philippe Mathieu-Daudé]

In order to make accel/tcg/ target agnostic,
introduce the cpu_exec_halt() handler.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/core/tcg-cpu-ops.h | 2 ++
 accel/tcg/cpu-exec.c          | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
index 2fae3ac70f..3307338c80 100644
--- a/include/hw/core/tcg-cpu-ops.h
+++ b/include/hw/core/tcg-cpu-ops.h
@@ -114,6 +114,8 @@ struct TCGCPUOps {
     void (*record_sigbus)(CPUState *cpu, vaddr addr,
                           MMUAccessType access_type, uintptr_t ra);
 #else
+    /** @cpu_exec_halt: Callback for handling halt in cpu_exec */
+    void (*cpu_exec_halt)(CPUState *cpu);
     /** @cpu_exec_interrupt: Callback for processing interrupts in cpu_exec */
     bool (*cpu_exec_interrupt)(CPUState *cpu, int interrupt_request);
     /**
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 5a978a9e72..390a9644da 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -681,6 +681,9 @@ static inline bool cpu_handle_halt(CPUClass *cc, CPUState *cpu)
             bql_unlock();
         }
 #endif /* TARGET_I386 */
+        if (cc->tcg_ops->cpu_exec_halt) {
+            cc->tcg_ops->cpu_exec_halt(cpu);
+        }
         if (!cpu_has_work(cpu)) {
             return true;
         }
-- 
2.41.0

[PATCH 9/9] target/i386: Extract x86_cpu_exec_halt() from accel/tcg/

[Philippe Mathieu-Daudé]

Move this x86-specific code out of the generic accel/tcg/.

Reported-by: Anton Johansson <anjo@rev.ng>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/i386/tcg/helper-tcg.h        |  1 +
 accel/tcg/cpu-exec.c                | 12 ------------
 target/i386/tcg/sysemu/seg_helper.c | 13 +++++++++++++
 target/i386/tcg/tcg-cpu.c           |  1 +
 4 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/target/i386/tcg/helper-tcg.h b/target/i386/tcg/helper-tcg.h
index 253b1f561e..effc2c1c98 100644
--- a/target/i386/tcg/helper-tcg.h
+++ b/target/i386/tcg/helper-tcg.h
@@ -39,6 +39,7 @@ QEMU_BUILD_BUG_ON(TCG_PHYS_ADDR_BITS > TARGET_PHYS_ADDR_SPACE_BITS);
  */
 void x86_cpu_do_interrupt(CPUState *cpu);
 #ifndef CONFIG_USER_ONLY
+void x86_cpu_exec_halt(CPUState *cpu);
 bool x86_need_replay_interrupt(int interrupt_request);
 bool x86_cpu_exec_interrupt(CPUState *cpu, int int_req);
 #endif
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 390a9644da..7662f4973d 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -30,9 +30,6 @@
 #include "qemu/rcu.h"
 #include "exec/log.h"
 #include "qemu/main-loop.h"
-#if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY)
-#include "hw/i386/apic.h"
-#endif
 #include "sysemu/cpus.h"
 #include "exec/cpu-all.h"
 #include "sysemu/cpu-timers.h"
@@ -672,15 +669,6 @@ static inline bool cpu_handle_halt(CPUClass *cc, CPUState *cpu)
 {
 #ifndef CONFIG_USER_ONLY
     if (cpu->halted) {
-#if defined(TARGET_I386)
-        if (cpu->interrupt_request & CPU_INTERRUPT_POLL) {
-            X86CPU *x86_cpu = X86_CPU(cpu);
-            bql_lock();
-            apic_poll_irq(x86_cpu->apic_state);
-            cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL);
-            bql_unlock();
-        }
-#endif /* TARGET_I386 */
         if (cc->tcg_ops->cpu_exec_halt) {
             cc->tcg_ops->cpu_exec_halt(cpu);
         }
diff --git a/target/i386/tcg/sysemu/seg_helper.c b/target/i386/tcg/sysemu/seg_helper.c
index e6f42282bb..2db8083748 100644
--- a/target/i386/tcg/sysemu/seg_helper.c
+++ b/target/i386/tcg/sysemu/seg_helper.c
@@ -20,6 +20,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/log.h"
+#include "qemu/main-loop.h"
 #include "cpu.h"
 #include "exec/helper-proto.h"
 #include "exec/cpu_ldst.h"
@@ -127,6 +128,18 @@ void x86_cpu_do_interrupt(CPUState *cs)
     }
 }
 
+void x86_cpu_exec_halt(CPUState *cpu)
+{
+    if (cpu->interrupt_request & CPU_INTERRUPT_POLL) {
+        X86CPU *x86_cpu = X86_CPU(cpu);
+
+        bql_lock();
+        apic_poll_irq(x86_cpu->apic_state);
+        cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL);
+        bql_unlock();
+    }
+}
+
 bool x86_need_replay_interrupt(int interrupt_request)
 {
     /*
diff --git a/target/i386/tcg/tcg-cpu.c b/target/i386/tcg/tcg-cpu.c
index 255d56d4c3..3028b57c97 100644
--- a/target/i386/tcg/tcg-cpu.c
+++ b/target/i386/tcg/tcg-cpu.c
@@ -119,6 +119,7 @@ static const struct TCGCPUOps x86_tcg_ops = {
 #else
     .tlb_fill = x86_cpu_tlb_fill,
     .do_interrupt = x86_cpu_do_interrupt,
+    .cpu_exec_halt = x86_cpu_exec_halt,
     .cpu_exec_interrupt = x86_cpu_exec_interrupt,
     .do_unaligned_access = x86_cpu_do_unaligned_access,
     .debug_excp_handler = breakpoint_handler,
-- 
2.41.0

Re: [PATCH 0/9] accel/tcg: Extract some x86-specific code

[Philippe Mathieu-Daudé]

On 24/1/24 11:16, Philippe Mathieu-Daudé wrote:
> accel/tcg/ ought to be target agnostic. This series remove
> some x86 code, addressing part of "Work still remains" from
> Anjo's series:

Oops, s/Anjo/Anton/

> https://lore.kernel.org/qemu-devel/20240119144024.14289-1-anjo@rev.ng/
> 
> Based-on: <20240124075609.14756-1-philmd@linaro.org>
>            "Move perf and debuginfo support to tcg"
> 
> Philippe Mathieu-Daudé (9):
>    accel/tcg: Rename tcg_ss[] -> tcg_specific_ss[] in meson
>    accel/tcg: Rename tcg_cpus_destroy() -> tcg_cpu_destroy()
>    accel/tcg: Rename tcg_cpus_exec() -> tcg_cpu_exec()
>    accel/tcg: Un-inline icount_exit_request() for clarity
>    accel/tcg: Hoist CPUClass arg to functions with external linkage
>    accel/tcg: Introduce TCGCPUOps::need_replay_interrupt() handler
>    target/i386: Extract x86_need_replay_interrupt() from accel/tcg/
>    accel/tcg: Introduce TCGCPUOps::cpu_exec_halt() handler
>    target/i386: Extract x86_cpu_exec_halt() from accel/tcg/
> 
>   accel/tcg/tcg-accel-ops.h           |   4 +-
>   include/hw/core/tcg-cpu-ops.h       |   7 ++
>   target/i386/tcg/helper-tcg.h        |   2 +
>   accel/tcg/cpu-exec.c                | 125 ++++++++++++----------------
>   accel/tcg/tcg-accel-ops-mttcg.c     |   4 +-
>   accel/tcg/tcg-accel-ops-rr.c        |   4 +-
>   accel/tcg/tcg-accel-ops.c           |   4 +-
>   target/i386/tcg/sysemu/seg_helper.c |  23 +++++
>   target/i386/tcg/tcg-cpu.c           |   2 +
>   accel/tcg/meson.build               |  12 +--
>   10 files changed, 103 insertions(+), 84 deletions(-)
> 

Re: [PATCH 1/9] accel/tcg: Rename tcg_ss[] -> tcg_specific_ss[] in meson

[Anton Johansson via]

On 24/01/24, Philippe Mathieu-Daudé wrote:
> tcg_ss[] source set contains target-specific units.
> Rename it as 'tcg_specific_ss[]' for clarity.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  accel/tcg/meson.build | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/accel/tcg/meson.build b/accel/tcg/meson.build
> index 46f7d53eeb..aef80de967 100644
> --- a/accel/tcg/meson.build
> +++ b/accel/tcg/meson.build
> @@ -1,8 +1,8 @@
> -tcg_ss = ss.source_set()
>  common_ss.add(when: 'CONFIG_TCG', if_true: files(
>    'cpu-exec-common.c',
>  ))
> -tcg_ss.add(files(
> +tcg_specific_ss = ss.source_set()
> +tcg_specific_ss.add(files(
>    'tcg-all.c',
>    'cpu-exec.c',
>    'tb-maint.c',
> @@ -11,12 +11,12 @@ tcg_ss.add(files(
>    'translate-all.c',
>    'translator.c',
>  ))
> -tcg_ss.add(when: 'CONFIG_USER_ONLY', if_true: files('user-exec.c'))
> -tcg_ss.add(when: 'CONFIG_SYSTEM_ONLY', if_false: files('user-exec-stub.c'))
> +tcg_specific_ss.add(when: 'CONFIG_USER_ONLY', if_true: files('user-exec.c'))
> +tcg_specific_ss.add(when: 'CONFIG_SYSTEM_ONLY', if_false: files('user-exec-stub.c'))
>  if get_option('plugins')
> -  tcg_ss.add(files('plugin-gen.c'))
> +  tcg_specific_ss.add(files('plugin-gen.c'))
>  endif
> -specific_ss.add_all(when: 'CONFIG_TCG', if_true: tcg_ss)
> +specific_ss.add_all(when: 'CONFIG_TCG', if_true: tcg_specific_ss)
>  
>  specific_ss.add(when: ['CONFIG_SYSTEM_ONLY', 'CONFIG_TCG'], if_true: files(
>    'cputlb.c',
> -- 
> 2.41.0
> 
Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH 2/9] accel/tcg: Rename tcg_cpus_destroy() -> tcg_cpu_destroy()

[Anton Johansson via]

On 24/01/24, Philippe Mathieu-Daudé wrote:
> tcg_cpus_destroy() operates on a single vCPU, rename it
> as 'tcg_cpu_destroy'.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  accel/tcg/tcg-accel-ops.h       | 2 +-
>  accel/tcg/tcg-accel-ops-mttcg.c | 2 +-
>  accel/tcg/tcg-accel-ops-rr.c    | 2 +-
>  accel/tcg/tcg-accel-ops.c       | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/accel/tcg/tcg-accel-ops.h b/accel/tcg/tcg-accel-ops.h
> index f9bc6330e2..17c7ed00eb 100644
> --- a/accel/tcg/tcg-accel-ops.h
> +++ b/accel/tcg/tcg-accel-ops.h
> @@ -14,7 +14,7 @@
>  
>  #include "sysemu/cpus.h"
>  
> -void tcg_cpus_destroy(CPUState *cpu);
> +void tcg_cpu_destroy(CPUState *cpu);
>  int tcg_cpus_exec(CPUState *cpu);
>  void tcg_handle_interrupt(CPUState *cpu, int mask);
>  void tcg_cpu_init_cflags(CPUState *cpu, bool parallel);
> diff --git a/accel/tcg/tcg-accel-ops-mttcg.c b/accel/tcg/tcg-accel-ops-mttcg.c
> index af7307013a..bcba314a65 100644
> --- a/accel/tcg/tcg-accel-ops-mttcg.c
> +++ b/accel/tcg/tcg-accel-ops-mttcg.c
> @@ -118,7 +118,7 @@ static void *mttcg_cpu_thread_fn(void *arg)
>          qemu_wait_io_event(cpu);
>      } while (!cpu->unplug || cpu_can_run(cpu));
>  
> -    tcg_cpus_destroy(cpu);
> +    tcg_cpu_destroy(cpu);
>      bql_unlock();
>      rcu_remove_force_rcu_notifier(&force_rcu.notifier);
>      rcu_unregister_thread();
> diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
> index 3208035d85..0617f66b5b 100644
> --- a/accel/tcg/tcg-accel-ops-rr.c
> +++ b/accel/tcg/tcg-accel-ops-rr.c
> @@ -131,7 +131,7 @@ static void rr_deal_with_unplugged_cpus(void)
>  
>      CPU_FOREACH(cpu) {
>          if (cpu->unplug && !cpu_can_run(cpu)) {
> -            tcg_cpus_destroy(cpu);
> +            tcg_cpu_destroy(cpu);
>              break;
>          }
>      }
> diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
> index 813065c0ec..9b84b84218 100644
> --- a/accel/tcg/tcg-accel-ops.c
> +++ b/accel/tcg/tcg-accel-ops.c
> @@ -63,7 +63,7 @@ void tcg_cpu_init_cflags(CPUState *cpu, bool parallel)
>      cpu->tcg_cflags |= cflags;
>  }
>  
> -void tcg_cpus_destroy(CPUState *cpu)
> +void tcg_cpu_destroy(CPUState *cpu)
>  {
>      cpu_thread_signal_destroyed(cpu);
>  }
> -- 
> 2.41.0
> 
Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH 3/9] accel/tcg: Rename tcg_cpus_exec() -> tcg_cpu_exec()

[Anton Johansson via]

On 24/01/24, Philippe Mathieu-Daudé wrote:
> tcg_cpus_exec() operates on a single vCPU, rename it
> as 'tcg_cpu_exec'.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  accel/tcg/tcg-accel-ops.h       | 2 +-
>  accel/tcg/tcg-accel-ops-mttcg.c | 2 +-
>  accel/tcg/tcg-accel-ops-rr.c    | 2 +-
>  accel/tcg/tcg-accel-ops.c       | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/accel/tcg/tcg-accel-ops.h b/accel/tcg/tcg-accel-ops.h
> index 17c7ed00eb..44c4079972 100644
> --- a/accel/tcg/tcg-accel-ops.h
> +++ b/accel/tcg/tcg-accel-ops.h
> @@ -15,7 +15,7 @@
>  #include "sysemu/cpus.h"
>  
>  void tcg_cpu_destroy(CPUState *cpu);
> -int tcg_cpus_exec(CPUState *cpu);
> +int tcg_cpu_exec(CPUState *cpu);
>  void tcg_handle_interrupt(CPUState *cpu, int mask);
>  void tcg_cpu_init_cflags(CPUState *cpu, bool parallel);
>  
> diff --git a/accel/tcg/tcg-accel-ops-mttcg.c b/accel/tcg/tcg-accel-ops-mttcg.c
> index bcba314a65..c552b45b8e 100644
> --- a/accel/tcg/tcg-accel-ops-mttcg.c
> +++ b/accel/tcg/tcg-accel-ops-mttcg.c
> @@ -92,7 +92,7 @@ static void *mttcg_cpu_thread_fn(void *arg)
>          if (cpu_can_run(cpu)) {
>              int r;
>              bql_unlock();
> -            r = tcg_cpus_exec(cpu);
> +            r = tcg_cpu_exec(cpu);
>              bql_lock();
>              switch (r) {
>              case EXCP_DEBUG:
> diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
> index 0617f66b5b..894e73e52c 100644
> --- a/accel/tcg/tcg-accel-ops-rr.c
> +++ b/accel/tcg/tcg-accel-ops-rr.c
> @@ -258,7 +258,7 @@ static void *rr_cpu_thread_fn(void *arg)
>                  if (icount_enabled()) {
>                      icount_prepare_for_run(cpu, cpu_budget);
>                  }
> -                r = tcg_cpus_exec(cpu);
> +                r = tcg_cpu_exec(cpu);
>                  if (icount_enabled()) {
>                      icount_process_data(cpu);
>                  }
> diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
> index 9b84b84218..9c957f421c 100644
> --- a/accel/tcg/tcg-accel-ops.c
> +++ b/accel/tcg/tcg-accel-ops.c
> @@ -68,7 +68,7 @@ void tcg_cpu_destroy(CPUState *cpu)
>      cpu_thread_signal_destroyed(cpu);
>  }
>  
> -int tcg_cpus_exec(CPUState *cpu)
> +int tcg_cpu_exec(CPUState *cpu)
>  {
>      int ret;
>      assert(tcg_enabled());
> -- 
> 2.41.0
> 
Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH 4/9] accel/tcg: Un-inline icount_exit_request() for clarity

[Anton Johansson via]

On 24/01/24, Philippe Mathieu-Daudé wrote:
> Convert packed logic to dumb icount_exit_request() helper.
> No functional change intended.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  accel/tcg/cpu-exec.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 6b3f66930e..d61b285d5e 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -791,6 +791,17 @@ static inline bool need_replay_interrupt(int interrupt_request)
>  }
>  #endif /* !CONFIG_USER_ONLY */
>  
> +static inline bool icount_exit_request(CPUState *cpu)
> +{
> +    if (!icount_enabled()) {
> +        return false;
> +    }
> +    if (cpu->cflags_next_tb != -1 && !(cpu->cflags_next_tb & CF_USE_ICOUNT)) {
> +        return false;
> +    }
> +    return cpu->neg.icount_decr.u16.low + cpu->icount_extra == 0;
> +}
> +
>  static inline bool cpu_handle_interrupt(CPUState *cpu,
>                                          TranslationBlock **last_tb)
>  {
> @@ -896,10 +907,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
>      }
>  
>      /* Finally, check if we need to exit to the main loop.  */
> -    if (unlikely(qatomic_read(&cpu->exit_request))
> -        || (icount_enabled()
> -            && (cpu->cflags_next_tb == -1 || cpu->cflags_next_tb & CF_USE_ICOUNT)
> -            && cpu->neg.icount_decr.u16.low + cpu->icount_extra == 0)) {
> +    if (unlikely(qatomic_read(&cpu->exit_request)) || icount_exit_request(cpu)) {
>          qatomic_set(&cpu->exit_request, 0);
>          if (cpu->exception_index == -1) {
>              cpu->exception_index = EXCP_INTERRUPT;
> -- 
> 2.41.0
> 
Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH 5/9] accel/tcg: Hoist CPUClass arg to functions with external linkage

[Anton Johansson via]

On 24/01/24, Philippe Mathieu-Daudé wrote:
> Hoist the CPUClass argument from most of these internal helpers:
> 
>  - check_for_breakpoints_slow
>  - check_for_breakpoints()
>  - cpu_tb_exec()
>  - cpu_exec_enter()
>  - cpu_exec_exit()
>  - cpu_handle_halt()
>  - cpu_handle_debug_exception()
>  - cpu_handle_exception()
>  - need_replay_interrupt()
>  - cpu_handle_interrupt()
>  - cpu_loop_exec_tb()
>  - cpu_exec_loop()
>  - cpu_exec_setjmp()
> 
> to the following ones with external linkage:
> 
>  - lookup_tb_ptr()
>  - cpu_exec_step_atomic()
>  - cpu_exec()
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  accel/tcg/cpu-exec.c | 82 ++++++++++++++++++++------------------------
>  1 file changed, 37 insertions(+), 45 deletions(-)
> 
Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH 6/9] accel/tcg: Introduce TCGCPUOps::need_replay_interrupt() handler

[Anton Johansson via]

On 24/01/24, Philippe Mathieu-Daudé wrote:
> In order to make accel/tcg/ target agnostic,
> introduce the need_replay_interrupt() handler.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/hw/core/tcg-cpu-ops.h | 5 +++++
>  accel/tcg/cpu-exec.c          | 5 ++++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
> index 479713a36e..2fae3ac70f 100644
> --- a/include/hw/core/tcg-cpu-ops.h
> +++ b/include/hw/core/tcg-cpu-ops.h
> @@ -170,6 +170,11 @@ struct TCGCPUOps {
>       */
>      bool (*io_recompile_replay_branch)(CPUState *cpu,
>                                         const TranslationBlock *tb);
> +    /**
> +     * @need_replay_interrupt: Return %true if @interrupt_request
> +     * needs to be recorded for replay purposes.
> +     */
> +    bool (*need_replay_interrupt)(int interrupt_request);
>  #endif /* !CONFIG_USER_ONLY */
>  #endif /* NEED_CPU_H */
>  
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index b10472cbc7..4ab7d6c896 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -778,7 +778,10 @@ static inline bool need_replay_interrupt(CPUClass *cc, int interrupt_request)
>  #if defined(TARGET_I386)
>      return !(interrupt_request & CPU_INTERRUPT_POLL);
>  #else
> -    return true;
> +    if (!cc->tcg_ops->need_replay_interrupt) {
> +        return true;
> +    }
> +    return cc->tcg_ops->need_replay_interrupt(interrupt_request);
>  #endif
>  }
>  #endif /* !CONFIG_USER_ONLY */
> -- 
> 2.41.0
> 
Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH 7/9] target/i386: Extract x86_need_replay_interrupt() from accel/tcg/

[Anton Johansson via]

On 24/01/24, Philippe Mathieu-Daudé wrote:
> Move this x86-specific code out of the generic accel/tcg/.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/i386/tcg/helper-tcg.h        |  1 +
>  accel/tcg/cpu-exec.c                |  9 ---------
>  target/i386/tcg/sysemu/seg_helper.c | 10 ++++++++++
>  target/i386/tcg/tcg-cpu.c           |  1 +
>  4 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/target/i386/tcg/helper-tcg.h b/target/i386/tcg/helper-tcg.h
> index ce34b737bb..253b1f561e 100644
> --- a/target/i386/tcg/helper-tcg.h
> +++ b/target/i386/tcg/helper-tcg.h
> @@ -39,6 +39,7 @@ QEMU_BUILD_BUG_ON(TCG_PHYS_ADDR_BITS > TARGET_PHYS_ADDR_SPACE_BITS);
>   */
>  void x86_cpu_do_interrupt(CPUState *cpu);
>  #ifndef CONFIG_USER_ONLY
> +bool x86_need_replay_interrupt(int interrupt_request);
>  bool x86_cpu_exec_interrupt(CPUState *cpu, int int_req);
>  #endif
>  
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 4ab7d6c896..5a978a9e72 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -768,21 +768,12 @@ static inline bool cpu_handle_exception(CPUClass *cc, CPUState *cpu, int *ret)
>  }
>  
>  #ifndef CONFIG_USER_ONLY
> -/*
> - * CPU_INTERRUPT_POLL is a virtual event which gets converted into a
> - * "real" interrupt event later. It does not need to be recorded for
> - * replay purposes.
> - */
>  static inline bool need_replay_interrupt(CPUClass *cc, int interrupt_request)
>  {
> -#if defined(TARGET_I386)
> -    return !(interrupt_request & CPU_INTERRUPT_POLL);
> -#else
>      if (!cc->tcg_ops->need_replay_interrupt) {
>          return true;
>      }
>      return cc->tcg_ops->need_replay_interrupt(interrupt_request);
> -#endif
>  }
>  #endif /* !CONFIG_USER_ONLY */
>  
> diff --git a/target/i386/tcg/sysemu/seg_helper.c b/target/i386/tcg/sysemu/seg_helper.c
> index 1cb5a0db45..e6f42282bb 100644
> --- a/target/i386/tcg/sysemu/seg_helper.c
> +++ b/target/i386/tcg/sysemu/seg_helper.c
> @@ -127,6 +127,16 @@ void x86_cpu_do_interrupt(CPUState *cs)
>      }
>  }
>  
> +bool x86_need_replay_interrupt(int interrupt_request)
> +{
> +    /*
> +     * CPU_INTERRUPT_POLL is a virtual event which gets converted into a
> +     * "real" interrupt event later. It does not need to be recorded for
> +     * replay purposes.
> +     */
> +    return !(interrupt_request & CPU_INTERRUPT_POLL);
> +}
> +
>  bool x86_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>  {
>      X86CPU *cpu = X86_CPU(cs);
> diff --git a/target/i386/tcg/tcg-cpu.c b/target/i386/tcg/tcg-cpu.c
> index e1405b7be9..255d56d4c3 100644
> --- a/target/i386/tcg/tcg-cpu.c
> +++ b/target/i386/tcg/tcg-cpu.c
> @@ -123,6 +123,7 @@ static const struct TCGCPUOps x86_tcg_ops = {
>      .do_unaligned_access = x86_cpu_do_unaligned_access,
>      .debug_excp_handler = breakpoint_handler,
>      .debug_check_breakpoint = x86_debug_check_breakpoint,
> +    .need_replay_interrupt = x86_need_replay_interrupt,
>  #endif /* !CONFIG_USER_ONLY */
>  };
>  
> -- 
> 2.41.0
> 
Ah this makes me happy!:)
Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH 8/9] accel/tcg: Introduce TCGCPUOps::cpu_exec_halt() handler

[Anton Johansson via]

On 24/01/24, Philippe Mathieu-Daudé wrote:
> In order to make accel/tcg/ target agnostic,
> introduce the cpu_exec_halt() handler.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/hw/core/tcg-cpu-ops.h | 2 ++
>  accel/tcg/cpu-exec.c          | 3 +++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
> index 2fae3ac70f..3307338c80 100644
> --- a/include/hw/core/tcg-cpu-ops.h
> +++ b/include/hw/core/tcg-cpu-ops.h
> @@ -114,6 +114,8 @@ struct TCGCPUOps {
>      void (*record_sigbus)(CPUState *cpu, vaddr addr,
>                            MMUAccessType access_type, uintptr_t ra);
>  #else
> +    /** @cpu_exec_halt: Callback for handling halt in cpu_exec */
> +    void (*cpu_exec_halt)(CPUState *cpu);
>      /** @cpu_exec_interrupt: Callback for processing interrupts in cpu_exec */
>      bool (*cpu_exec_interrupt)(CPUState *cpu, int interrupt_request);
>      /**
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 5a978a9e72..390a9644da 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -681,6 +681,9 @@ static inline bool cpu_handle_halt(CPUClass *cc, CPUState *cpu)
>              bql_unlock();
>          }
>  #endif /* TARGET_I386 */
> +        if (cc->tcg_ops->cpu_exec_halt) {
> +            cc->tcg_ops->cpu_exec_halt(cpu);
> +        }
>          if (!cpu_has_work(cpu)) {
>              return true;
>          }
> -- 
> 2.41.0
> 
Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH 9/9] target/i386: Extract x86_cpu_exec_halt() from accel/tcg/

[Anton Johansson via]

On 24/01/24, Philippe Mathieu-Daudé wrote:
> Move this x86-specific code out of the generic accel/tcg/.
> 
> Reported-by: Anton Johansson <anjo@rev.ng>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/i386/tcg/helper-tcg.h        |  1 +
>  accel/tcg/cpu-exec.c                | 12 ------------
>  target/i386/tcg/sysemu/seg_helper.c | 13 +++++++++++++
>  target/i386/tcg/tcg-cpu.c           |  1 +
>  4 files changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/target/i386/tcg/helper-tcg.h b/target/i386/tcg/helper-tcg.h
> index 253b1f561e..effc2c1c98 100644
> --- a/target/i386/tcg/helper-tcg.h
> +++ b/target/i386/tcg/helper-tcg.h
> @@ -39,6 +39,7 @@ QEMU_BUILD_BUG_ON(TCG_PHYS_ADDR_BITS > TARGET_PHYS_ADDR_SPACE_BITS);
>   */
>  void x86_cpu_do_interrupt(CPUState *cpu);
>  #ifndef CONFIG_USER_ONLY
> +void x86_cpu_exec_halt(CPUState *cpu);
>  bool x86_need_replay_interrupt(int interrupt_request);
>  bool x86_cpu_exec_interrupt(CPUState *cpu, int int_req);
>  #endif
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 390a9644da..7662f4973d 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -30,9 +30,6 @@
>  #include "qemu/rcu.h"
>  #include "exec/log.h"
>  #include "qemu/main-loop.h"
> -#if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY)
> -#include "hw/i386/apic.h"
> -#endif
>  #include "sysemu/cpus.h"
>  #include "exec/cpu-all.h"
>  #include "sysemu/cpu-timers.h"
> @@ -672,15 +669,6 @@ static inline bool cpu_handle_halt(CPUClass *cc, CPUState *cpu)
>  {
>  #ifndef CONFIG_USER_ONLY
>      if (cpu->halted) {
> -#if defined(TARGET_I386)
> -        if (cpu->interrupt_request & CPU_INTERRUPT_POLL) {
> -            X86CPU *x86_cpu = X86_CPU(cpu);
> -            bql_lock();
> -            apic_poll_irq(x86_cpu->apic_state);
> -            cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL);
> -            bql_unlock();
> -        }
> -#endif /* TARGET_I386 */
>          if (cc->tcg_ops->cpu_exec_halt) {
>              cc->tcg_ops->cpu_exec_halt(cpu);
>          }
> diff --git a/target/i386/tcg/sysemu/seg_helper.c b/target/i386/tcg/sysemu/seg_helper.c
> index e6f42282bb..2db8083748 100644
> --- a/target/i386/tcg/sysemu/seg_helper.c
> +++ b/target/i386/tcg/sysemu/seg_helper.c
> @@ -20,6 +20,7 @@
>  
>  #include "qemu/osdep.h"
>  #include "qemu/log.h"
> +#include "qemu/main-loop.h"
>  #include "cpu.h"
>  #include "exec/helper-proto.h"
>  #include "exec/cpu_ldst.h"
> @@ -127,6 +128,18 @@ void x86_cpu_do_interrupt(CPUState *cs)
>      }
>  }
>  
> +void x86_cpu_exec_halt(CPUState *cpu)
> +{
> +    if (cpu->interrupt_request & CPU_INTERRUPT_POLL) {
> +        X86CPU *x86_cpu = X86_CPU(cpu);
> +
> +        bql_lock();
> +        apic_poll_irq(x86_cpu->apic_state);
> +        cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL);
> +        bql_unlock();
> +    }
> +}
> +
>  bool x86_need_replay_interrupt(int interrupt_request)
>  {
>      /*
> diff --git a/target/i386/tcg/tcg-cpu.c b/target/i386/tcg/tcg-cpu.c
> index 255d56d4c3..3028b57c97 100644
> --- a/target/i386/tcg/tcg-cpu.c
> +++ b/target/i386/tcg/tcg-cpu.c
> @@ -119,6 +119,7 @@ static const struct TCGCPUOps x86_tcg_ops = {
>  #else
>      .tlb_fill = x86_cpu_tlb_fill,
>      .do_interrupt = x86_cpu_do_interrupt,
> +    .cpu_exec_halt = x86_cpu_exec_halt,
>      .cpu_exec_interrupt = x86_cpu_exec_interrupt,
>      .do_unaligned_access = x86_cpu_do_unaligned_access,
>      .debug_excp_handler = breakpoint_handler,
> -- 
> 2.41.0
> 
Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH 7/9] target/i386: Extract x86_need_replay_interrupt() from accel/tcg/

[Philippe Mathieu-Daudé]

On 24/1/24 18:17, Anton Johansson wrote:
> On 24/01/24, Philippe Mathieu-Daudé wrote:
>> Move this x86-specific code out of the generic accel/tcg/.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/i386/tcg/helper-tcg.h        |  1 +
>>   accel/tcg/cpu-exec.c                |  9 ---------
>>   target/i386/tcg/sysemu/seg_helper.c | 10 ++++++++++
>>   target/i386/tcg/tcg-cpu.c           |  1 +
>>   4 files changed, 12 insertions(+), 9 deletions(-)

> Ah this makes me happy!:)

There are still few more, but this is a start...

$ git grep -wA3 TARGET_I386 accel/tcg
accel/tcg/cpu-exec.c:311:#if defined(TARGET_I386)
accel/tcg/cpu-exec.c-312-                flags |= CPU_DUMP_CCOP;
accel/tcg/cpu-exec.c-313-#endif
--
accel/tcg/cpu-exec.c:727:#if defined(TARGET_I386)
accel/tcg/cpu-exec.c-728-        cc->tcg_ops->fake_user_interrupt(cpu);
accel/tcg/cpu-exec.c:729:#endif /* TARGET_I386 */
--
accel/tcg/cpu-exec.c:826:#if defined(TARGET_I386)
accel/tcg/cpu-exec.c-827-        else if (interrupt_request & 
CPU_INTERRUPT_INIT) {
accel/tcg/cpu-exec.c-828-            X86CPU *x86_cpu = X86_CPU(cpu);
accel/tcg/cpu-exec.c-829-            CPUArchState *env = &x86_cpu->env;
...
accel/tcg/cpu-exec.c:844:#endif /* !TARGET_I386 */
--

> Reviewed-by: Anton Johansson <anjo@rev.ng>

Thanks!

Re: [PATCH 1/9] accel/tcg: Rename tcg_ss[] -> tcg_specific_ss[] in meson

[Richard Henderson]

On 1/24/24 20:16, Philippe Mathieu-Daudé wrote:
> tcg_ss[] source set contains target-specific units.
> Rename it as 'tcg_specific_ss[]' for clarity.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   accel/tcg/meson.build | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)

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

r~

Re: [PATCH 2/9] accel/tcg: Rename tcg_cpus_destroy() -> tcg_cpu_destroy()

[Richard Henderson]

On 1/24/24 20:16, Philippe Mathieu-Daudé wrote:
> tcg_cpus_destroy() operates on a single vCPU, rename it
> as 'tcg_cpu_destroy'.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   accel/tcg/tcg-accel-ops.h       | 2 +-
>   accel/tcg/tcg-accel-ops-mttcg.c | 2 +-
>   accel/tcg/tcg-accel-ops-rr.c    | 2 +-
>   accel/tcg/tcg-accel-ops.c       | 2 +-
>   4 files changed, 4 insertions(+), 4 deletions(-)

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

r~

Re: [PATCH 3/9] accel/tcg: Rename tcg_cpus_exec() -> tcg_cpu_exec()

[Richard Henderson]

On 1/24/24 20:16, Philippe Mathieu-Daudé wrote:
> tcg_cpus_exec() operates on a single vCPU, rename it
> as 'tcg_cpu_exec'.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   accel/tcg/tcg-accel-ops.h       | 2 +-
>   accel/tcg/tcg-accel-ops-mttcg.c | 2 +-
>   accel/tcg/tcg-accel-ops-rr.c    | 2 +-
>   accel/tcg/tcg-accel-ops.c       | 2 +-
>   4 files changed, 4 insertions(+), 4 deletions(-)

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

r~

Re: [PATCH 4/9] accel/tcg: Un-inline icount_exit_request() for clarity

[Richard Henderson]

On 1/24/24 20:16, Philippe Mathieu-Daudé wrote:
> Convert packed logic to dumb icount_exit_request() helper.
> No functional change intended.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   accel/tcg/cpu-exec.c | 16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)

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

r~

Re: [PATCH 5/9] accel/tcg: Hoist CPUClass arg to functions with external linkage

[Richard Henderson]

On 1/24/24 20:16, Philippe Mathieu-Daudé wrote:
> Hoist the CPUClass argument from most of these internal helpers:
> 
>   - check_for_breakpoints_slow
>   - check_for_breakpoints()
>   - cpu_tb_exec()
>   - cpu_exec_enter()
>   - cpu_exec_exit()
>   - cpu_handle_halt()
>   - cpu_handle_debug_exception()
>   - cpu_handle_exception()
>   - need_replay_interrupt()
>   - cpu_handle_interrupt()
>   - cpu_loop_exec_tb()
>   - cpu_exec_loop()
>   - cpu_exec_setjmp()
> 
> to the following ones with external linkage:
> 
>   - lookup_tb_ptr()
>   - cpu_exec_step_atomic()
>   - cpu_exec()
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   accel/tcg/cpu-exec.c | 82 ++++++++++++++++++++------------------------
>   1 file changed, 37 insertions(+), 45 deletions(-)

I'm not so keen on this.  Does it really make a difference?
What about simply making more use of CPUState->cc instead?


r~

Re: [PATCH 6/9] accel/tcg: Introduce TCGCPUOps::need_replay_interrupt() handler

[Richard Henderson]

On 1/24/24 20:16, Philippe Mathieu-Daudé wrote:
> In order to make accel/tcg/ target agnostic,
> introduce the need_replay_interrupt() handler.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   include/hw/core/tcg-cpu-ops.h | 5 +++++
>   accel/tcg/cpu-exec.c          | 5 ++++-
>   2 files changed, 9 insertions(+), 1 deletion(-)

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


r~

Re: [PATCH 7/9] target/i386: Extract x86_need_replay_interrupt() from accel/tcg/

[Richard Henderson]

On 1/24/24 20:16, Philippe Mathieu-Daudé wrote:
> Move this x86-specific code out of the generic accel/tcg/.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   target/i386/tcg/helper-tcg.h        |  1 +
>   accel/tcg/cpu-exec.c                |  9 ---------
>   target/i386/tcg/sysemu/seg_helper.c | 10 ++++++++++
>   target/i386/tcg/tcg-cpu.c           |  1 +
>   4 files changed, 12 insertions(+), 9 deletions(-)

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

r~

Re: [PATCH 8/9] accel/tcg: Introduce TCGCPUOps::cpu_exec_halt() handler

[Richard Henderson]

On 1/24/24 20:16, Philippe Mathieu-Daudé wrote:
> In order to make accel/tcg/ target agnostic,
> introduce the cpu_exec_halt() handler.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

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


r~

Re: [PATCH 9/9] target/i386: Extract x86_cpu_exec_halt() from accel/tcg/

[Richard Henderson]

On 1/24/24 20:16, Philippe Mathieu-Daudé wrote:
> Move this x86-specific code out of the generic accel/tcg/.
> 
> Reported-by: Anton Johansson<anjo@rev.ng>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   target/i386/tcg/helper-tcg.h        |  1 +
>   accel/tcg/cpu-exec.c                | 12 ------------
>   target/i386/tcg/sysemu/seg_helper.c | 13 +++++++++++++
>   target/i386/tcg/tcg-cpu.c           |  1 +
>   4 files changed, 15 insertions(+), 12 deletions(-)

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

r~

Re: [PATCH 5/9] accel/tcg: Hoist CPUClass arg to functions with external linkage

[Philippe Mathieu-Daudé]

On 24/1/24 23:59, Richard Henderson wrote:
> On 1/24/24 20:16, Philippe Mathieu-Daudé wrote:
>> Hoist the CPUClass argument from most of these internal helpers:
>>
>>   - check_for_breakpoints_slow
>>   - check_for_breakpoints()
>>   - cpu_tb_exec()
>>   - cpu_exec_enter()
>>   - cpu_exec_exit()
>>   - cpu_handle_halt()
>>   - cpu_handle_debug_exception()
>>   - cpu_handle_exception()
>>   - need_replay_interrupt()
>>   - cpu_handle_interrupt()
>>   - cpu_loop_exec_tb()
>>   - cpu_exec_loop()
>>   - cpu_exec_setjmp()
>>
>> to the following ones with external linkage:
>>
>>   - lookup_tb_ptr()
>>   - cpu_exec_step_atomic()
>>   - cpu_exec()
>>
>> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
>> ---
>>   accel/tcg/cpu-exec.c | 82 ++++++++++++++++++++------------------------
>>   1 file changed, 37 insertions(+), 45 deletions(-)
> 
> I'm not so keen on this.  Does it really make a difference?
> What about simply making more use of CPUState->cc instead?

TIL CPUState->cc... Which makes me wonder why this isn't handler
generically via QOM macros.

> 
> 
> r~

Re: [PATCH 6/9] accel/tcg: Introduce TCGCPUOps::need_replay_interrupt() handler

[Pavel Dovgalyuk]

Reviewed-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>

On 24.01.2024 13:16, Philippe Mathieu-Daudé wrote:
> In order to make accel/tcg/ target agnostic,
> introduce the need_replay_interrupt() handler.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/core/tcg-cpu-ops.h | 5 +++++
>   accel/tcg/cpu-exec.c          | 5 ++++-
>   2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
> index 479713a36e..2fae3ac70f 100644
> --- a/include/hw/core/tcg-cpu-ops.h
> +++ b/include/hw/core/tcg-cpu-ops.h
> @@ -170,6 +170,11 @@ struct TCGCPUOps {
>        */
>       bool (*io_recompile_replay_branch)(CPUState *cpu,
>                                          const TranslationBlock *tb);
> +    /**
> +     * @need_replay_interrupt: Return %true if @interrupt_request
> +     * needs to be recorded for replay purposes.
> +     */
> +    bool (*need_replay_interrupt)(int interrupt_request);
>   #endif /* !CONFIG_USER_ONLY */
>   #endif /* NEED_CPU_H */
>   
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index b10472cbc7..4ab7d6c896 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -778,7 +778,10 @@ static inline bool need_replay_interrupt(CPUClass *cc, int interrupt_request)
>   #if defined(TARGET_I386)
>       return !(interrupt_request & CPU_INTERRUPT_POLL);
>   #else
> -    return true;
> +    if (!cc->tcg_ops->need_replay_interrupt) {
> +        return true;
> +    }
> +    return cc->tcg_ops->need_replay_interrupt(interrupt_request);
>   #endif
>   }
>   #endif /* !CONFIG_USER_ONLY */

Re: [PATCH 7/9] target/i386: Extract x86_need_replay_interrupt() from accel/tcg/

[Pavel Dovgalyuk]

Reviewed-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>

On 24.01.2024 13:16, Philippe Mathieu-Daudé wrote:
> Move this x86-specific code out of the generic accel/tcg/.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/i386/tcg/helper-tcg.h        |  1 +
>   accel/tcg/cpu-exec.c                |  9 ---------
>   target/i386/tcg/sysemu/seg_helper.c | 10 ++++++++++
>   target/i386/tcg/tcg-cpu.c           |  1 +
>   4 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/target/i386/tcg/helper-tcg.h b/target/i386/tcg/helper-tcg.h
> index ce34b737bb..253b1f561e 100644
> --- a/target/i386/tcg/helper-tcg.h
> +++ b/target/i386/tcg/helper-tcg.h
> @@ -39,6 +39,7 @@ QEMU_BUILD_BUG_ON(TCG_PHYS_ADDR_BITS > TARGET_PHYS_ADDR_SPACE_BITS);
>    */
>   void x86_cpu_do_interrupt(CPUState *cpu);
>   #ifndef CONFIG_USER_ONLY
> +bool x86_need_replay_interrupt(int interrupt_request);
>   bool x86_cpu_exec_interrupt(CPUState *cpu, int int_req);
>   #endif
>   
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 4ab7d6c896..5a978a9e72 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -768,21 +768,12 @@ static inline bool cpu_handle_exception(CPUClass *cc, CPUState *cpu, int *ret)
>   }
>   
>   #ifndef CONFIG_USER_ONLY
> -/*
> - * CPU_INTERRUPT_POLL is a virtual event which gets converted into a
> - * "real" interrupt event later. It does not need to be recorded for
> - * replay purposes.
> - */
>   static inline bool need_replay_interrupt(CPUClass *cc, int interrupt_request)
>   {
> -#if defined(TARGET_I386)
> -    return !(interrupt_request & CPU_INTERRUPT_POLL);
> -#else
>       if (!cc->tcg_ops->need_replay_interrupt) {
>           return true;
>       }
>       return cc->tcg_ops->need_replay_interrupt(interrupt_request);
> -#endif
>   }
>   #endif /* !CONFIG_USER_ONLY */
>   
> diff --git a/target/i386/tcg/sysemu/seg_helper.c b/target/i386/tcg/sysemu/seg_helper.c
> index 1cb5a0db45..e6f42282bb 100644
> --- a/target/i386/tcg/sysemu/seg_helper.c
> +++ b/target/i386/tcg/sysemu/seg_helper.c
> @@ -127,6 +127,16 @@ void x86_cpu_do_interrupt(CPUState *cs)
>       }
>   }
>   
> +bool x86_need_replay_interrupt(int interrupt_request)
> +{
> +    /*
> +     * CPU_INTERRUPT_POLL is a virtual event which gets converted into a
> +     * "real" interrupt event later. It does not need to be recorded for
> +     * replay purposes.
> +     */
> +    return !(interrupt_request & CPU_INTERRUPT_POLL);
> +}
> +
>   bool x86_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>   {
>       X86CPU *cpu = X86_CPU(cs);
> diff --git a/target/i386/tcg/tcg-cpu.c b/target/i386/tcg/tcg-cpu.c
> index e1405b7be9..255d56d4c3 100644
> --- a/target/i386/tcg/tcg-cpu.c
> +++ b/target/i386/tcg/tcg-cpu.c
> @@ -123,6 +123,7 @@ static const struct TCGCPUOps x86_tcg_ops = {
>       .do_unaligned_access = x86_cpu_do_unaligned_access,
>       .debug_excp_handler = breakpoint_handler,
>       .debug_check_breakpoint = x86_debug_check_breakpoint,
> +    .need_replay_interrupt = x86_need_replay_interrupt,
>   #endif /* !CONFIG_USER_ONLY */
>   };
>   

Re: [PATCH 0/9] accel/tcg: Extract some x86-specific code

[Richard Henderson]

On 1/24/24 20:16, Philippe Mathieu-Daudé wrote:
> accel/tcg/ ought to be target agnostic. This series remove
> some x86 code, addressing part of "Work still remains" from
> Anjo's series:
> https://lore.kernel.org/qemu-devel/20240119144024.14289-1-anjo@rev.ng/
> 
> Based-on:<20240124075609.14756-1-philmd@linaro.org>
>            "Move perf and debuginfo support to tcg"
> 
> Philippe Mathieu-Daudé (9):
>    accel/tcg: Rename tcg_ss[] -> tcg_specific_ss[] in meson
>    accel/tcg: Rename tcg_cpus_destroy() -> tcg_cpu_destroy()
>    accel/tcg: Rename tcg_cpus_exec() -> tcg_cpu_exec()
>    accel/tcg: Un-inline icount_exit_request() for clarity
>    accel/tcg: Hoist CPUClass arg to functions with external linkage
>    accel/tcg: Introduce TCGCPUOps::need_replay_interrupt() handler
>    target/i386: Extract x86_need_replay_interrupt() from accel/tcg/
>    accel/tcg: Introduce TCGCPUOps::cpu_exec_halt() handler
>    target/i386: Extract x86_cpu_exec_halt() from accel/tcg/

Queued, with patch 5 replaced with usage of cpu->cc.

r~