[PATCH 0/3] accel: Factor tcg_cpu_reset_hold() out of cpu-common.c

[PATCH 0/3] accel: Factor tcg_cpu_reset_hold() out of cpu-common.c

[Philippe Mathieu-Daudé]

Hi,

We want to have exec/ code agnostic to accelerators.
still we use various call to TCG and KVM. This series
factor the TCG code from cpu_common_reset_hold() to an
accel-specific handler within AccelOpsClass.

Based-on: <20230915190009.68404-1-philmd@linaro.org>

Philippe Mathieu-Daudé (3):
  accel/tcg: Declare tcg_flush_jmp_cache() in 'exec/tb-flush.h'
  accel: Introduce cpu_exec_reset_hold()
  accel/tcg: Factor tcg_cpu_reset_hold() out

 include/exec/cpu-common.h  | 3 ---
 include/exec/tb-flush.h    | 2 ++
 include/hw/core/cpu.h      | 1 +
 include/sysemu/accel-ops.h | 1 +
 accel/stubs/tcg-stub.c     | 4 ----
 accel/tcg/cputlb.c         | 1 +
 accel/tcg/tcg-accel-ops.c  | 9 +++++++++
 accel/tcg/translate-all.c  | 8 --------
 accel/tcg/user-exec-stub.c | 4 ++++
 hw/core/cpu-common.c       | 5 +----
 plugins/core.c             | 1 -
 softmmu/cpus.c             | 7 +++++++
 12 files changed, 26 insertions(+), 20 deletions(-)

-- 
2.41.0

[PATCH 1/3] accel/tcg: Declare tcg_flush_jmp_cache() in 'exec/tb-flush.h'

[Philippe Mathieu-Daudé]

"exec/cpu-common.h" is meant to contain the declarations
related to CPU usable with any accelerator / target
combination.

tcg_flush_jmp_cache() is specific to TCG, so restrict its
declaration by moving it to "exec/tb-flush.h".

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

diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 41788c0bdd..1eff233565 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -39,7 +39,6 @@ void cpu_list_unlock(void);
 unsigned int cpu_list_generation_id_get(void);
 
 void tcg_flush_softmmu_tlb(CPUState *cs);
-void tcg_flush_jmp_cache(CPUState *cs);
 
 void tcg_iommu_init_notifier_list(CPUState *cpu);
 void tcg_iommu_free_notifier_list(CPUState *cpu);
diff --git a/include/exec/tb-flush.h b/include/exec/tb-flush.h
index d92d06565b..142c240d94 100644
--- a/include/exec/tb-flush.h
+++ b/include/exec/tb-flush.h
@@ -23,4 +23,6 @@
  */
 void tb_flush(CPUState *cs);
 
+void tcg_flush_jmp_cache(CPUState *cs);
+
 #endif /* _TB_FLUSH_H_ */
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index c643d66190..b220c4536c 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -24,6 +24,7 @@
 #include "exec/memory.h"
 #include "exec/cpu_ldst.h"
 #include "exec/cputlb.h"
+#include "exec/tb-flush.h"
 #include "exec/memory-internal.h"
 #include "exec/ram_addr.h"
 #include "tcg/tcg.h"
diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
index 3973591508..748539f289 100644
--- a/accel/tcg/tcg-accel-ops.c
+++ b/accel/tcg/tcg-accel-ops.c
@@ -34,6 +34,7 @@
 #include "qemu/timer.h"
 #include "exec/exec-all.h"
 #include "exec/hwaddr.h"
+#include "exec/tb-flush.h"
 #include "exec/gdbstub.h"
 
 #include "tcg-accel-ops.h"
diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index ced66c2b34..b3b5ce6702 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -27,6 +27,7 @@
 #include "qemu/main-loop.h"
 #include "exec/log.h"
 #include "exec/cpu-common.h"
+#include "exec/tb-flush.h"
 #include "qemu/error-report.h"
 #include "qemu/qemu-print.h"
 #include "sysemu/tcg.h"
diff --git a/plugins/core.c b/plugins/core.c
index 3c4e26c7ed..c95c00a830 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -21,7 +21,6 @@
 #include "qemu/xxhash.h"
 #include "qemu/rcu.h"
 #include "hw/core/cpu.h"
-#include "exec/cpu-common.h"
 
 #include "exec/exec-all.h"
 #include "exec/tb-flush.h"
-- 
2.41.0

[PATCH 2/3] accel: Introduce cpu_exec_reset_hold()

[Philippe Mathieu-Daudé]

Introduce cpu_exec_reset_hold() which call an accelerator
specific AccelOpsClass::cpu_reset_hold() handler.

Define a stub on TCG user emulation, because CPU reset is
irrelevant there.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/core/cpu.h      | 1 +
 include/sysemu/accel-ops.h | 1 +
 accel/tcg/user-exec-stub.c | 4 ++++
 hw/core/cpu-common.c       | 1 +
 softmmu/cpus.c             | 7 +++++++
 5 files changed, 14 insertions(+)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 92a4234439..7bbfa81dcd 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -1011,6 +1011,7 @@ void cpu_class_init_props(DeviceClass *dc);
 void cpu_exec_initfn(CPUState *cpu);
 void cpu_exec_realizefn(CPUState *cpu, Error **errp);
 void cpu_exec_unrealizefn(CPUState *cpu);
+void cpu_exec_reset_hold(CPUState *cpu);
 
 /**
  * target_words_bigendian:
diff --git a/include/sysemu/accel-ops.h b/include/sysemu/accel-ops.h
index 3c1fab4b1e..ef91fc28bb 100644
--- a/include/sysemu/accel-ops.h
+++ b/include/sysemu/accel-ops.h
@@ -30,6 +30,7 @@ struct AccelOpsClass {
     void (*ops_init)(AccelOpsClass *ops);
 
     bool (*cpus_are_resettable)(void);
+    void (*cpu_reset_hold)(CPUState *cpu);
 
     void (*create_vcpu_thread)(CPUState *cpu); /* MANDATORY NON-NULL */
     void (*kick_vcpu_thread)(CPUState *cpu);
diff --git a/accel/tcg/user-exec-stub.c b/accel/tcg/user-exec-stub.c
index 874e1f1a20..b1bf8bed89 100644
--- a/accel/tcg/user-exec-stub.c
+++ b/accel/tcg/user-exec-stub.c
@@ -16,6 +16,10 @@ void qemu_init_vcpu(CPUState *cpu)
 {
 }
 
+void cpu_exec_reset_hold(CPUState *cpu)
+{
+}
+
 /* User mode emulation does not support record/replay yet.  */
 
 bool replay_exception(void)
diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index b3b5ce6702..b50bc22fb7 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -137,6 +137,7 @@ static void cpu_common_reset_hold(Object *obj)
     cpu->crash_occurred = false;
     cpu->cflags_next_tb = -1;
 
+    cpu_exec_reset_hold(cpu);
     if (tcg_enabled()) {
         tcg_flush_jmp_cache(cpu);
         tcg_flush_softmmu_tlb(cpu);
diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index 0848e0dbdb..952f15868c 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -201,6 +201,13 @@ bool cpus_are_resettable(void)
     return true;
 }
 
+void cpu_exec_reset_hold(CPUState *cpu)
+{
+    if (cpus_accel->cpu_reset_hold) {
+        cpus_accel->cpu_reset_hold(cpu);
+    }
+}
+
 int64_t cpus_get_virtual_clock(void)
 {
     /*
-- 
2.41.0

[PATCH 3/3] accel/tcg: Factor tcg_cpu_reset_hold() out

[Philippe Mathieu-Daudé]

Factor the TCG specific code from cpu_common_reset_hold() to
tcg_cpu_reset_hold() within tcg-accel-ops.c. Since this file
is sysemu specific, we can inline tcg_flush_softmmu_tlb(),
removing its declaration in "exec/cpu-common.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/exec/cpu-common.h | 2 --
 accel/stubs/tcg-stub.c    | 4 ----
 accel/tcg/tcg-accel-ops.c | 8 ++++++++
 accel/tcg/translate-all.c | 8 --------
 hw/core/cpu-common.c      | 5 -----
 5 files changed, 8 insertions(+), 19 deletions(-)

diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 1eff233565..d71b4d712d 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -38,8 +38,6 @@ void cpu_list_lock(void);
 void cpu_list_unlock(void);
 unsigned int cpu_list_generation_id_get(void);
 
-void tcg_flush_softmmu_tlb(CPUState *cs);
-
 void tcg_iommu_init_notifier_list(CPUState *cpu);
 void tcg_iommu_free_notifier_list(CPUState *cpu);
 
diff --git a/accel/stubs/tcg-stub.c b/accel/stubs/tcg-stub.c
index a9e7a2d5b4..8a496a2a6f 100644
--- a/accel/stubs/tcg-stub.c
+++ b/accel/stubs/tcg-stub.c
@@ -22,10 +22,6 @@ void tlb_set_dirty(CPUState *cpu, vaddr vaddr)
 {
 }
 
-void tcg_flush_jmp_cache(CPUState *cpu)
-{
-}
-
 int probe_access_flags(CPUArchState *env, vaddr addr, int size,
                        MMUAccessType access_type, int mmu_idx,
                        bool nonfault, void **phost, uintptr_t retaddr)
diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
index 748539f289..0882464d03 100644
--- a/accel/tcg/tcg-accel-ops.c
+++ b/accel/tcg/tcg-accel-ops.c
@@ -78,6 +78,13 @@ int tcg_cpus_exec(CPUState *cpu)
     return ret;
 }
 
+static void tcg_cpu_reset_hold(CPUState *cpu)
+{
+    tcg_flush_jmp_cache(cpu);
+
+    tlb_flush(cpu);
+}
+
 /* mask must never be zero, except for A20 change call */
 void tcg_handle_interrupt(CPUState *cpu, int mask)
 {
@@ -206,6 +213,7 @@ static void tcg_accel_ops_init(AccelOpsClass *ops)
         }
     }
 
+    ops->cpu_reset_hold = tcg_cpu_reset_hold;
     ops->supports_guest_debug = tcg_supports_guest_debug;
     ops->insert_breakpoint = tcg_insert_breakpoint;
     ops->remove_breakpoint = tcg_remove_breakpoint;
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index b2d4e22c17..0f3ed835cb 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -801,11 +801,3 @@ void tcg_flush_jmp_cache(CPUState *cpu)
         qatomic_set(&jc->array[i].tb, NULL);
     }
 }
-
-/* This is a wrapper for common code that can not use CONFIG_SOFTMMU */
-void tcg_flush_softmmu_tlb(CPUState *cs)
-{
-#ifdef CONFIG_SOFTMMU
-    tlb_flush(cs);
-#endif
-}
diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index b50bc22fb7..23040e0398 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -27,7 +27,6 @@
 #include "qemu/main-loop.h"
 #include "exec/log.h"
 #include "exec/cpu-common.h"
-#include "exec/tb-flush.h"
 #include "qemu/error-report.h"
 #include "qemu/qemu-print.h"
 #include "sysemu/tcg.h"
@@ -138,10 +137,6 @@ static void cpu_common_reset_hold(Object *obj)
     cpu->cflags_next_tb = -1;
 
     cpu_exec_reset_hold(cpu);
-    if (tcg_enabled()) {
-        tcg_flush_jmp_cache(cpu);
-        tcg_flush_softmmu_tlb(cpu);
-    }
 }
 
 static bool cpu_common_has_work(CPUState *cs)
-- 
2.41.0

Re: [PATCH 0/3] accel: Factor tcg_cpu_reset_hold() out of cpu-common.c

[Anton Johansson via]

On 18/09/23, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> We want to have exec/ code agnostic to accelerators.
> still we use various call to TCG and KVM. This series
> factor the TCG code from cpu_common_reset_hold() to an
> accel-specific handler within AccelOpsClass.
> 
> Based-on: <20230915190009.68404-1-philmd@linaro.org>
> 
> Philippe Mathieu-Daudé (3):
>   accel/tcg: Declare tcg_flush_jmp_cache() in 'exec/tb-flush.h'
>   accel: Introduce cpu_exec_reset_hold()
>   accel/tcg: Factor tcg_cpu_reset_hold() out
> 
>  include/exec/cpu-common.h  | 3 ---
>  include/exec/tb-flush.h    | 2 ++
>  include/hw/core/cpu.h      | 1 +
>  include/sysemu/accel-ops.h | 1 +
>  accel/stubs/tcg-stub.c     | 4 ----
>  accel/tcg/cputlb.c         | 1 +
>  accel/tcg/tcg-accel-ops.c  | 9 +++++++++
>  accel/tcg/translate-all.c  | 8 --------
>  accel/tcg/user-exec-stub.c | 4 ++++
>  hw/core/cpu-common.c       | 5 +----
>  plugins/core.c             | 1 -
>  softmmu/cpus.c             | 7 +++++++
>  12 files changed, 26 insertions(+), 20 deletions(-)
> 
> -- 
> 2.41.0
> 
Series:
Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH 0/3] accel: Factor tcg_cpu_reset_hold() out of cpu-common.c

[Richard Henderson]

On 9/18/23 12:41, Philippe Mathieu-Daudé wrote:
> Philippe Mathieu-Daudé (3):
>    accel/tcg: Declare tcg_flush_jmp_cache() in 'exec/tb-flush.h'
>    accel: Introduce cpu_exec_reset_hold()
>    accel/tcg: Factor tcg_cpu_reset_hold() out

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

r~