[PATCH 00/11] accel/tcg: Improve tb_flush usage

[PATCH 00/11] accel/tcg: Improve tb_flush usage

[Richard Henderson]

It is too easy to mis-use tb_flush().  For instance, because of
the cpu argument, some parts assumed that it needed to call the
global flush function for every cpu.  It is easy to forget that
the flush is not complete when the call returns: we have merely
queued work to the cpu run loop.

(Phil, I suspect this second case is what's biting split-accel.)

So: remove tb_flush and expose only the core as tb_flush__exclusive,
to be used only when we are already within an exclusive context.

In some cases (gdbstub, alpha, riscv, ppc spapr), we can eliminate
the need for tb_flush completely.

Lightly tested so far, and I'm off on holiday next, but I thought
this might help others working on split-accel in the meantime.


r~


Richard Henderson (11):
  gdbstub: Remove tb_flush uses
  accel/tcg: Split out tb_flush__exclusive
  target/alpha: Simplify call_pal implementation
  target/riscv: Record misa_ext in TCGTBCPUState.cs_base
  accel/tcg: Move post-load tb_flush to vm_change_state hook
  hw/ppc/spapr: Use tb_invalidate_phys_range in h_page_init
  linux-user: Use tb_flush_exclusive to start second thread
  plugins: Use tb_flush__exclusive
  accel/tcg: Introduce EXCP_TB_FLUSH
  accel/tcg: Use EXCP_TB_FLUSH in tb_gen_code
  accel/tcg: Remove tb_flush

 include/exec/cpu-common.h       |  1 +
 include/exec/tb-flush.h         | 18 +++++++---------
 target/alpha/helper.h           |  1 -
 accel/tcg/tb-maint.c            | 38 +++++++--------------------------
 accel/tcg/tcg-accel-ops-mttcg.c |  7 ++++++
 accel/tcg/tcg-accel-ops-rr.c    |  9 ++++++--
 accel/tcg/tcg-all.c             | 21 ++++++++++++++++++
 accel/tcg/translate-all.c       |  5 +----
 gdbstub/system.c                |  4 ----
 gdbstub/user.c                  |  3 ---
 hw/core/cpu-system.c            |  8 -------
 hw/ppc/spapr_hcall.c            |  4 ++--
 linux-user/mmap.c               |  4 ++--
 linux-user/syscall.c            |  2 +-
 plugins/core.c                  |  6 ++----
 plugins/loader.c                |  2 +-
 target/alpha/sys_helper.c       |  6 ------
 target/alpha/translate.c        | 21 ++++++------------
 target/riscv/csr.c              |  3 ---
 target/riscv/tcg/tcg-cpu.c      |  3 ++-
 20 files changed, 69 insertions(+), 97 deletions(-)

-- 
2.43.0

[PATCH 01/11] gdbstub: Remove tb_flush uses

[Richard Henderson]

This hasn't been needed since d828b92b8a6
("accel/tcg: Introduce CF_BP_PAGE").

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 gdbstub/system.c | 4 ----
 gdbstub/user.c   | 3 ---
 2 files changed, 7 deletions(-)

diff --git a/gdbstub/system.c b/gdbstub/system.c
index 5be0d3c58c..df3514dc74 100644
--- a/gdbstub/system.c
+++ b/gdbstub/system.c
@@ -18,7 +18,6 @@
 #include "gdbstub/syscalls.h"
 #include "gdbstub/commands.h"
 #include "exec/hwaddr.h"
-#include "exec/tb-flush.h"
 #include "accel/accel-ops.h"
 #include "accel/accel-cpu-ops.h"
 #include "system/cpus.h"
@@ -174,9 +173,6 @@ static void gdb_vm_state_change(void *opaque, bool running, RunState state)
         } else {
             trace_gdbstub_hit_break();
         }
-        if (tcg_enabled()) {
-            tb_flush(cpu);
-        }
         ret = GDB_SIGNAL_TRAP;
         break;
     case RUN_STATE_PAUSED:
diff --git a/gdbstub/user.c b/gdbstub/user.c
index 67403e5a25..2e14ded3f0 100644
--- a/gdbstub/user.c
+++ b/gdbstub/user.c
@@ -15,7 +15,6 @@
 #include "qemu/sockets.h"
 #include "qapi/error.h"
 #include "exec/hwaddr.h"
-#include "exec/tb-flush.h"
 #include "exec/gdbstub.h"
 #include "gdbstub/commands.h"
 #include "gdbstub/syscalls.h"
@@ -220,7 +219,6 @@ int gdb_handlesig(CPUState *cpu, int sig, const char *reason, void *siginfo,
 
     /* disable single step if it was enabled */
     cpu_single_step(cpu, 0);
-    tb_flush(cpu);
 
     if (sig != 0) {
         gdb_set_stop_cpu(cpu);
@@ -539,7 +537,6 @@ static void disable_gdbstub(CPUState *thread_cpu)
         /* no cpu_watchpoint_remove_all for user-mode */
         cpu_single_step(cpu, 0);
     }
-    tb_flush(thread_cpu);
 }
 
 void gdbserver_fork_end(CPUState *cpu, pid_t pid)
-- 
2.43.0

[PATCH 02/11] accel/tcg: Split out tb_flush__exclusive

[Richard Henderson]

Expose a routine to be called when no cpus are running.
Simplify the do_tb_flush run_on_cpu callback, because
that is explicitly called with start_exclusive; there
is no need for the mmap_lock as well.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/tb-flush.h |  1 +
 accel/tcg/tb-maint.c    | 28 ++++++++++++++--------------
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/include/exec/tb-flush.h b/include/exec/tb-flush.h
index 142c240d94..f3898d1826 100644
--- a/include/exec/tb-flush.h
+++ b/include/exec/tb-flush.h
@@ -23,6 +23,7 @@
  */
 void tb_flush(CPUState *cs);
 
+void tb_flush__exclusive(void);
 void tcg_flush_jmp_cache(CPUState *cs);
 
 #endif /* _TB_FLUSH_H_ */
diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
index 0048316f99..d1695be00b 100644
--- a/accel/tcg/tb-maint.c
+++ b/accel/tcg/tb-maint.c
@@ -756,17 +756,14 @@ static void tb_remove(TranslationBlock *tb)
 }
 #endif /* CONFIG_USER_ONLY */
 
-/* flush all the translation blocks */
-static void do_tb_flush(CPUState *cpu, run_on_cpu_data tb_flush_count)
+/*
+ * Flush all the translation blocks.
+ * Must be called from a context in which no cpus are running,
+ * e.g. start_exclusive() or vm_stop().
+ */
+void tb_flush__exclusive(void)
 {
-    bool did_flush = false;
-
-    mmap_lock();
-    /* If it is already been done on request of another CPU, just retry. */
-    if (tb_ctx.tb_flush_count != tb_flush_count.host_int) {
-        goto done;
-    }
-    did_flush = true;
+    CPUState *cpu;
 
     CPU_FOREACH(cpu) {
         tcg_flush_jmp_cache(cpu);
@@ -778,11 +775,14 @@ static void do_tb_flush(CPUState *cpu, run_on_cpu_data tb_flush_count)
     tcg_region_reset_all();
     /* XXX: flush processor icache at this point if cache flush is expensive */
     qatomic_inc(&tb_ctx.tb_flush_count);
+    qemu_plugin_flush_cb();
+}
 
-done:
-    mmap_unlock();
-    if (did_flush) {
-        qemu_plugin_flush_cb();
+static void do_tb_flush(CPUState *cpu, run_on_cpu_data tb_flush_count)
+{
+    /* If it is already been done on request of another CPU, just retry. */
+    if (tb_ctx.tb_flush_count == tb_flush_count.host_int) {
+        tb_flush__exclusive();
     }
 }
 
-- 
2.43.0

[PATCH 03/11] target/alpha: Simplify call_pal implementation

[Richard Henderson]

Since 288a5fe980f, we don't link translation blocks
directly to palcode entry points.  If we load palbr
from env instead of encoding the constant, we avoid
all need for tb_flush().

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/alpha/helper.h     |  1 -
 target/alpha/sys_helper.c |  6 ------
 target/alpha/translate.c  | 21 ++++++---------------
 3 files changed, 6 insertions(+), 22 deletions(-)

diff --git a/target/alpha/helper.h b/target/alpha/helper.h
index d60f208703..788d2fbf28 100644
--- a/target/alpha/helper.h
+++ b/target/alpha/helper.h
@@ -90,7 +90,6 @@ DEF_HELPER_FLAGS_2(ieee_input_s, TCG_CALL_NO_WG, void, env, i64)
 #if !defined (CONFIG_USER_ONLY)
 DEF_HELPER_FLAGS_1(tbia, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_FLAGS_2(tbis, TCG_CALL_NO_RWG, void, env, i64)
-DEF_HELPER_FLAGS_1(tb_flush, TCG_CALL_NO_RWG, void, env)
 
 DEF_HELPER_1(halt, void, i64)
 
diff --git a/target/alpha/sys_helper.c b/target/alpha/sys_helper.c
index 51e3254428..87e37605c1 100644
--- a/target/alpha/sys_helper.c
+++ b/target/alpha/sys_helper.c
@@ -20,7 +20,6 @@
 #include "qemu/osdep.h"
 #include "cpu.h"
 #include "exec/cputlb.h"
-#include "exec/tb-flush.h"
 #include "exec/helper-proto.h"
 #include "system/runstate.h"
 #include "system/system.h"
@@ -38,11 +37,6 @@ void helper_tbis(CPUAlphaState *env, uint64_t p)
     tlb_flush_page(env_cpu(env), p);
 }
 
-void helper_tb_flush(CPUAlphaState *env)
-{
-    tb_flush(env_cpu(env));
-}
-
 void helper_halt(uint64_t restart)
 {
     if (restart) {
diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index cebab0318c..f11b382438 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -48,8 +48,6 @@ struct DisasContext {
 
 #ifdef CONFIG_USER_ONLY
     MemOp unalign;
-#else
-    uint64_t palbr;
 #endif
     uint32_t tbflags;
     int mem_idx;
@@ -1155,7 +1153,6 @@ static DisasJumpType gen_call_pal(DisasContext *ctx, int palcode)
 #else
     {
         TCGv tmp = tcg_temp_new();
-        uint64_t entry;
 
         gen_pc_disp(ctx, tmp, 0);
         if (ctx->tbflags & ENV_FLAG_PAL_MODE) {
@@ -1165,12 +1162,11 @@ static DisasJumpType gen_call_pal(DisasContext *ctx, int palcode)
         }
         tcg_gen_st_i64(tmp, tcg_env, offsetof(CPUAlphaState, exc_addr));
 
-        entry = ctx->palbr;
-        entry += (palcode & 0x80
-                  ? 0x2000 + (palcode - 0x80) * 64
-                  : 0x1000 + palcode * 64);
-
-        tcg_gen_movi_i64(cpu_pc, entry);
+        tcg_gen_ld_i64(cpu_pc, tcg_env, offsetof(CPUAlphaState, palbr));
+        tcg_gen_addi_i64(cpu_pc, cpu_pc,
+                         palcode & 0x80
+                         ? 0x2000 + (palcode - 0x80) * 64
+                         : 0x1000 + palcode * 64);
         return DISAS_PC_UPDATED;
     }
 #endif
@@ -1292,11 +1288,7 @@ static DisasJumpType gen_mtpr(DisasContext *ctx, TCGv vb, int regno)
     case 7:
         /* PALBR */
         tcg_gen_st_i64(vb, tcg_env, offsetof(CPUAlphaState, palbr));
-        /* Changing the PAL base register implies un-chaining all of the TBs
-           that ended with a CALL_PAL.  Since the base register usually only
-           changes during boot, flushing everything works well.  */
-        gen_helper_tb_flush(tcg_env);
-        return DISAS_PC_STALE;
+        break;
 
     case 32 ... 39:
         /* Accessing the "non-shadow" general registers.  */
@@ -2874,7 +2866,6 @@ static void alpha_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cpu)
     ctx->ir = cpu_std_ir;
     ctx->unalign = (ctx->tbflags & TB_FLAG_UNALIGN ? MO_UNALN : MO_ALIGN);
 #else
-    ctx->palbr = env->palbr;
     ctx->ir = (ctx->tbflags & ENV_FLAG_PAL_MODE ? cpu_pal_ir : cpu_std_ir);
 #endif
 
-- 
2.43.0

[PATCH 04/11] target/riscv: Record misa_ext in TCGTBCPUState.cs_base

[Richard Henderson]

The tb_flush within write_misa was incorrect.  It assumed
that we could adjust the ISA of the current processor and
discard all TB and all would be well.  But MISA is per vcpu,
so globally flushing TB does not mean that the TB matches
the MISA of any given vcpu.

By recording misa in the tb state, we ensure that the code
generated matches the vcpu.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/csr.c         | 3 ---
 target/riscv/tcg/tcg-cpu.c | 3 ++-
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 8842e07a73..3c8989f522 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -25,7 +25,6 @@
 #include "pmu.h"
 #include "time_helper.h"
 #include "exec/cputlb.h"
-#include "exec/tb-flush.h"
 #include "exec/icount.h"
 #include "accel/tcg/getpc.h"
 #include "qemu/guest-random.h"
@@ -2173,8 +2172,6 @@ static RISCVException write_misa(CPURISCVState *env, int csrno,
         env->mstatus &= ~MSTATUS_FS;
     }
 
-    /* flush translation cache */
-    tb_flush(env_cpu(env));
     env->xl = riscv_cpu_mxl(env);
     return RISCV_EXCP_NONE;
 }
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 78fb279184..143ab079d4 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -191,7 +191,8 @@ static TCGTBCPUState riscv_get_tb_cpu_state(CPUState *cs)
 
     return (TCGTBCPUState){
         .pc = env->xl == MXL_RV32 ? env->pc & UINT32_MAX : env->pc,
-        .flags = flags
+        .flags = flags,
+        .cs_base = env->misa_ext,
     };
 }
 
-- 
2.43.0

[PATCH 05/11] accel/tcg: Move post-load tb_flush to vm_change_state hook

[Richard Henderson]

We need not call tb_flush once per cpu, only once per vmload.
Move the call from cpu_common_post_load to a tcg-specific
vm_change_state_handler.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/tcg-all.c  | 21 +++++++++++++++++++++
 hw/core/cpu-system.c |  8 --------
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index 5125e1a4e2..a0bc0e58c7 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -38,6 +38,8 @@
 #include "qemu/target-info.h"
 #ifndef CONFIG_USER_ONLY
 #include "hw/boards.h"
+#include "exec/tb-flush.h"
+#include "system/runstate.h"
 #endif
 #include "accel/accel-ops.h"
 #include "accel/accel-cpu-ops.h"
@@ -82,6 +84,23 @@ static void tcg_accel_instance_init(Object *obj)
 
 bool one_insn_per_tb;
 
+#ifndef CONFIG_USER_ONLY
+static void tcg_vm_change_state(void *opaque, bool running, RunState state)
+{
+    if (state == RUN_STATE_RESTORE_VM) {
+        /*
+         * loadvm will update the content of RAM, bypassing the usual
+         * mechanisms that ensure we flush TBs for writes to memory
+         * we've translated code from, so we must flush all TBs.
+         *
+         * vm_stop() has just stopped all cpus, so we are exclusive.
+         */
+        assert(!running);
+        tb_flush__exclusive();
+    }
+}
+#endif
+
 static int tcg_init_machine(AccelState *as, MachineState *ms)
 {
     TCGState *s = TCG_STATE(as);
@@ -124,6 +143,8 @@ static int tcg_init_machine(AccelState *as, MachineState *ms)
     default:
         g_assert_not_reached();
     }
+
+    qemu_add_vm_change_state_handler(tcg_vm_change_state, NULL);
 #endif
 
     tcg_allowed = true;
diff --git a/hw/core/cpu-system.c b/hw/core/cpu-system.c
index a975405d3a..c9099c6e7a 100644
--- a/hw/core/cpu-system.c
+++ b/hw/core/cpu-system.c
@@ -207,14 +207,6 @@ static int cpu_common_post_load(void *opaque, int version_id)
         cpu->interrupt_request &= ~0x01;
 
         tlb_flush(cpu);
-
-        /*
-         * loadvm has just updated the content of RAM, bypassing the
-         * usual mechanisms that ensure we flush TBs for writes to
-         * memory we've translated code from. So we must flush all TBs,
-         * which will now be stale.
-         */
-        tb_flush(cpu);
     }
 
     return 0;
-- 
2.43.0

[PATCH 06/11] hw/ppc/spapr: Use tb_invalidate_phys_range in h_page_init

[Richard Henderson]

We only need invalidate tbs from a single page, not flush
all translations.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 hw/ppc/spapr_hcall.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 1e936f35e4..aa2e5e1e84 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -8,7 +8,7 @@
 #include "qemu/main-loop.h"
 #include "qemu/module.h"
 #include "qemu/error-report.h"
-#include "exec/tb-flush.h"
+#include "exec/translation-block.h"
 #include "exec/target_page.h"
 #include "helper_regs.h"
 #include "hw/ppc/ppc.h"
@@ -301,7 +301,7 @@ static target_ulong h_page_init(PowerPCCPU *cpu, SpaprMachineState *spapr,
         if (kvm_enabled()) {
             kvmppc_icbi_range(cpu, pdst, len);
         } else if (tcg_enabled()) {
-            tb_flush(CPU(cpu));
+            tb_invalidate_phys_range(CPU(cpu), dst, len);
         } else {
             g_assert_not_reached();
         }
-- 
2.43.0

[PATCH 07/11] linux-user: Use tb_flush_exclusive to start second thread

[Richard Henderson]

When we start the second thread, we discard all translations
so that we can re-do them with CF_PARALLEL.  Since there is
as yet only one cpu, and we are processing a syscall, there
are no live translation blocks and we have exclusivity.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/mmap.c    | 4 ++--
 linux-user/syscall.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 002e1e668e..bd2bbaf1f4 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -1010,7 +1010,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
         CPUState *cpu = thread_cpu;
         if (!tcg_cflags_has(cpu, CF_PARALLEL)) {
             tcg_cflags_set(cpu, CF_PARALLEL);
-            tb_flush(cpu);
+            tb_flush__exclusive();
         }
     }
 
@@ -1450,7 +1450,7 @@ abi_ulong target_shmat(CPUArchState *cpu_env, int shmid,
      */
     if (!tcg_cflags_has(cpu, CF_PARALLEL)) {
         tcg_cflags_set(cpu, CF_PARALLEL);
-        tb_flush(cpu);
+        tb_flush__exclusive();
     }
 
     if (qemu_loglevel_mask(CPU_LOG_PAGE)) {
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 91360a072c..d9c394856f 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -6633,7 +6633,7 @@ static int do_fork(CPUArchState *env, unsigned int flags, abi_ulong newsp,
          */
         if (!tcg_cflags_has(cpu, CF_PARALLEL)) {
             tcg_cflags_set(cpu, CF_PARALLEL);
-            tb_flush(cpu);
+            tb_flush__exclusive();
         }
 
         /* we create a new CPU instance. */
-- 
2.43.0

[PATCH 08/11] plugins: Use tb_flush__exclusive

[Richard Henderson]

In all cases, we are already within start_exclusive.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 plugins/core.c   | 6 ++----
 plugins/loader.c | 2 +-
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/plugins/core.c b/plugins/core.c
index c6e9ef1478..4ae1a6ae17 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -248,7 +248,7 @@ static void plugin_grow_scoreboards__locked(CPUState *cpu)
         }
         plugin.scoreboard_alloc_size = scoreboard_size;
         /* force all tb to be flushed, as scoreboard pointers were changed. */
-        tb_flush(cpu);
+        tb_flush__exclusive();
     }
     end_exclusive();
 }
@@ -684,8 +684,6 @@ void qemu_plugin_user_exit(void)
      * with the one in fork_start(). That is:
      * - start_exclusive(), which acquires qemu_cpu_list_lock,
      *   must be called before acquiring plugin.lock.
-     * - tb_flush(), which acquires mmap_lock(), must be called
-     *   while plugin.lock is not held.
      */
     start_exclusive();
 
@@ -705,7 +703,7 @@ void qemu_plugin_user_exit(void)
     }
     qemu_rec_mutex_unlock(&plugin.lock);
 
-    tb_flush(current_cpu);
+    tb_flush__exclusive();
     end_exclusive();
 
     /* now it's safe to handle the exit case */
diff --git a/plugins/loader.c b/plugins/loader.c
index 8f0d75c904..6849e1c518 100644
--- a/plugins/loader.c
+++ b/plugins/loader.c
@@ -378,7 +378,7 @@ static void plugin_flush_destroy(CPUState *cpu, run_on_cpu_data arg)
     struct qemu_plugin_reset_data *data = arg.host_ptr;
 
     g_assert(cpu_in_exclusive_context(cpu));
-    tb_flush(cpu);
+    tb_flush__exclusive();
     plugin_reset_destroy(data);
 }
 
-- 
2.43.0

[PATCH 09/11] accel/tcg: Introduce EXCP_TB_FLUSH

[Richard Henderson]

We are going to disallow tb_flush from within the context
of a running cpu.  Introduce a tcg-internal exception to
return out of the cpu run loop and perform the flush there.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/cpu-common.h       | 1 +
 accel/tcg/tcg-accel-ops-mttcg.c | 7 +++++++
 accel/tcg/tcg-accel-ops-rr.c    | 9 +++++++--
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 9b658a3f48..ce9f116ac3 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -20,6 +20,7 @@
 #define EXCP_HALTED     0x10003 /* cpu is halted (waiting for external event) */
 #define EXCP_YIELD      0x10004 /* cpu wants to yield timeslice to another */
 #define EXCP_ATOMIC     0x10005 /* stop-the-world and emulate atomic */
+#define EXCP_TB_FLUSH   0x10006 /* stop-the-world and flush all tb */
 
 void cpu_exec_init_all(void);
 void cpu_exec_step_atomic(CPUState *cpu);
diff --git a/accel/tcg/tcg-accel-ops-mttcg.c b/accel/tcg/tcg-accel-ops-mttcg.c
index 337b993d3d..f21c86dc84 100644
--- a/accel/tcg/tcg-accel-ops-mttcg.c
+++ b/accel/tcg/tcg-accel-ops-mttcg.c
@@ -27,6 +27,7 @@
 #include "system/tcg.h"
 #include "system/replay.h"
 #include "exec/icount.h"
+#include "exec/tb-flush.h"
 #include "qemu/main-loop.h"
 #include "qemu/notify.h"
 #include "qemu/guest-random.h"
@@ -107,6 +108,12 @@ static void *mttcg_cpu_thread_fn(void *arg)
                 bql_unlock();
                 cpu_exec_step_atomic(cpu);
                 bql_lock();
+                break;
+            case EXCP_TB_FLUSH:
+                start_exclusive();
+                tb_flush__exclusive();
+                end_exclusive();
+                break;
             default:
                 /* Ignore everything else? */
                 break;
diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
index 6eec5c9eee..d4bf092736 100644
--- a/accel/tcg/tcg-accel-ops-rr.c
+++ b/accel/tcg/tcg-accel-ops-rr.c
@@ -32,6 +32,7 @@
 #include "qemu/notify.h"
 #include "qemu/guest-random.h"
 #include "exec/cpu-common.h"
+#include "exec/tb-flush.h"
 #include "tcg/startup.h"
 #include "tcg-accel-ops.h"
 #include "tcg-accel-ops-rr.h"
@@ -264,14 +265,18 @@ static void *rr_cpu_thread_fn(void *arg)
                 }
                 bql_lock();
 
-                if (r == EXCP_DEBUG) {
+                switch (r) {
+                case EXCP_DEBUG:
                     cpu_handle_guest_debug(cpu);
                     break;
-                } else if (r == EXCP_ATOMIC) {
+                case EXCP_ATOMIC:
                     bql_unlock();
                     cpu_exec_step_atomic(cpu);
                     bql_lock();
                     break;
+                case EXCP_TB_FLUSH:
+                    tb_flush__exclusive();
+                    break;
                 }
             } else if (cpu->stop) {
                 if (cpu->unplug) {
-- 
2.43.0

[PATCH 10/11] accel/tcg: Use EXCP_TB_FLUSH in tb_gen_code

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/translate-all.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index d468667b0d..d7cc346414 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -288,11 +288,8 @@ TranslationBlock *tb_gen_code(CPUState *cpu, TCGTBCPUState s)
     assert_no_pages_locked();
     tb = tcg_tb_alloc(tcg_ctx);
     if (unlikely(!tb)) {
-        /* flush must be done */
-        tb_flush(cpu);
         mmap_unlock();
-        /* Make the execution loop process the flush as soon as possible.  */
-        cpu->exception_index = EXCP_INTERRUPT;
+        cpu->exception_index = EXCP_TB_FLUSH;
         cpu_loop_exit(cpu);
     }
 
-- 
2.43.0

[PATCH 11/11] accel/tcg: Remove tb_flush

[Richard Henderson]

All uses have been replaced with tb_flush__exclusive.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/tb-flush.h | 19 ++++++++-----------
 accel/tcg/tb-maint.c    | 22 ----------------------
 2 files changed, 8 insertions(+), 33 deletions(-)

diff --git a/include/exec/tb-flush.h b/include/exec/tb-flush.h
index f3898d1826..d6586b9d5f 100644
--- a/include/exec/tb-flush.h
+++ b/include/exec/tb-flush.h
@@ -9,21 +9,18 @@
 #define _TB_FLUSH_H_
 
 /**
- * tb_flush() - flush all translation blocks
- * @cs: CPUState (must be valid, but treated as anonymous pointer)
+ * tb_flush__exclusive() - flush all translation blocks
  *
- * Used to flush all the translation blocks in the system. Sometimes
- * it is simpler to flush everything than work out which individual
- * translations are now invalid and ensure they are not called
- * anymore.
+ * Used to flush all the translation blocks in the system.
+ * Sometimes it is simpler to flush everything than work out which
+ * individual translations are now invalid and ensure they are
+ * not called anymore.
  *
- * tb_flush() takes care of running the flush in an exclusive context
- * if it is not already running in one. This means no guest code will
- * run until this complete.
+ * Must be called from an exclusive context, e.g. start_exclusive
+ * or vm_stop.
  */
-void tb_flush(CPUState *cs);
-
 void tb_flush__exclusive(void);
+
 void tcg_flush_jmp_cache(CPUState *cs);
 
 #endif /* _TB_FLUSH_H_ */
diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
index d1695be00b..0fe7a0de8a 100644
--- a/accel/tcg/tb-maint.c
+++ b/accel/tcg/tb-maint.c
@@ -778,28 +778,6 @@ void tb_flush__exclusive(void)
     qemu_plugin_flush_cb();
 }
 
-static void do_tb_flush(CPUState *cpu, run_on_cpu_data tb_flush_count)
-{
-    /* If it is already been done on request of another CPU, just retry. */
-    if (tb_ctx.tb_flush_count == tb_flush_count.host_int) {
-        tb_flush__exclusive();
-    }
-}
-
-void tb_flush(CPUState *cpu)
-{
-    if (tcg_enabled()) {
-        unsigned tb_flush_count = qatomic_read(&tb_ctx.tb_flush_count);
-
-        if (cpu_in_serial_context(cpu)) {
-            do_tb_flush(cpu, RUN_ON_CPU_HOST_INT(tb_flush_count));
-        } else {
-            async_safe_run_on_cpu(cpu, do_tb_flush,
-                                  RUN_ON_CPU_HOST_INT(tb_flush_count));
-        }
-    }
-}
-
 /* remove @orig from its @n_orig-th jump list */
 static inline void tb_remove_from_jmp_list(TranslationBlock *orig, int n_orig)
 {
-- 
2.43.0

Re: [PATCH 00/11] accel/tcg: Improve tb_flush usage

[Richard Henderson]

+cc target and migration maintainers

On 9/6/25 07:18, Richard Henderson wrote:
> It is too easy to mis-use tb_flush().  For instance, because of
> the cpu argument, some parts assumed that it needed to call the
> global flush function for every cpu.  It is easy to forget that
> the flush is not complete when the call returns: we have merely
> queued work to the cpu run loop.
> 
> (Phil, I suspect this second case is what's biting split-accel.)
> 
> So: remove tb_flush and expose only the core as tb_flush__exclusive,
> to be used only when we are already within an exclusive context.
> 
> In some cases (gdbstub, alpha, riscv, ppc spapr), we can eliminate
> the need for tb_flush completely.
> 
> Lightly tested so far, and I'm off on holiday next, but I thought
> this might help others working on split-accel in the meantime.
> 
> 
> r~
> 
> 
> Richard Henderson (11):
>    gdbstub: Remove tb_flush uses
>    accel/tcg: Split out tb_flush__exclusive
>    target/alpha: Simplify call_pal implementation
>    target/riscv: Record misa_ext in TCGTBCPUState.cs_base
>    accel/tcg: Move post-load tb_flush to vm_change_state hook
>    hw/ppc/spapr: Use tb_invalidate_phys_range in h_page_init
>    linux-user: Use tb_flush_exclusive to start second thread
>    plugins: Use tb_flush__exclusive
>    accel/tcg: Introduce EXCP_TB_FLUSH
>    accel/tcg: Use EXCP_TB_FLUSH in tb_gen_code
>    accel/tcg: Remove tb_flush
> 
>   include/exec/cpu-common.h       |  1 +
>   include/exec/tb-flush.h         | 18 +++++++---------
>   target/alpha/helper.h           |  1 -
>   accel/tcg/tb-maint.c            | 38 +++++++--------------------------
>   accel/tcg/tcg-accel-ops-mttcg.c |  7 ++++++
>   accel/tcg/tcg-accel-ops-rr.c    |  9 ++++++--
>   accel/tcg/tcg-all.c             | 21 ++++++++++++++++++
>   accel/tcg/translate-all.c       |  5 +----
>   gdbstub/system.c                |  4 ----
>   gdbstub/user.c                  |  3 ---
>   hw/core/cpu-system.c            |  8 -------
>   hw/ppc/spapr_hcall.c            |  4 ++--
>   linux-user/mmap.c               |  4 ++--
>   linux-user/syscall.c            |  2 +-
>   plugins/core.c                  |  6 ++----
>   plugins/loader.c                |  2 +-
>   target/alpha/sys_helper.c       |  6 ------
>   target/alpha/translate.c        | 21 ++++++------------
>   target/riscv/csr.c              |  3 ---
>   target/riscv/tcg/tcg-cpu.c      |  3 ++-
>   20 files changed, 69 insertions(+), 97 deletions(-)
> 

Re: [PATCH 00/11] accel/tcg: Improve tb_flush usage

[Pierrick Bouvier]

On 2025-09-06 07:18, Richard Henderson wrote:
> It is too easy to mis-use tb_flush().  For instance, because of
> the cpu argument, some parts assumed that it needed to call the
> global flush function for every cpu.  It is easy to forget that
> the flush is not complete when the call returns: we have merely
> queued work to the cpu run loop.
>

That's a good idea, and will remove the confusion that sometimes the 
tb_flush is immediate, and sometimes queued to run asynchronously, 
depending on current context.

> (Phil, I suspect this second case is what's biting split-accel.)
> 
> So: remove tb_flush and expose only the core as tb_flush__exclusive,
> to be used only when we are already within an exclusive context.
> 
> In some cases (gdbstub, alpha, riscv, ppc spapr), we can eliminate
> the need for tb_flush completely.
> 
> Lightly tested so far, and I'm off on holiday next, but I thought
> this might help others working on split-accel in the meantime.
> 
> 
> r~
> 
> 
> Richard Henderson (11):
>    gdbstub: Remove tb_flush uses
>    accel/tcg: Split out tb_flush__exclusive
>    target/alpha: Simplify call_pal implementation
>    target/riscv: Record misa_ext in TCGTBCPUState.cs_base
>    accel/tcg: Move post-load tb_flush to vm_change_state hook
>    hw/ppc/spapr: Use tb_invalidate_phys_range in h_page_init
>    linux-user: Use tb_flush_exclusive to start second thread
>    plugins: Use tb_flush__exclusive
>    accel/tcg: Introduce EXCP_TB_FLUSH
>    accel/tcg: Use EXCP_TB_FLUSH in tb_gen_code
>    accel/tcg: Remove tb_flush
> 
>   include/exec/cpu-common.h       |  1 +
>   include/exec/tb-flush.h         | 18 +++++++---------
>   target/alpha/helper.h           |  1 -
>   accel/tcg/tb-maint.c            | 38 +++++++--------------------------
>   accel/tcg/tcg-accel-ops-mttcg.c |  7 ++++++
>   accel/tcg/tcg-accel-ops-rr.c    |  9 ++++++--
>   accel/tcg/tcg-all.c             | 21 ++++++++++++++++++
>   accel/tcg/translate-all.c       |  5 +----
>   gdbstub/system.c                |  4 ----
>   gdbstub/user.c                  |  3 ---
>   hw/core/cpu-system.c            |  8 -------
>   hw/ppc/spapr_hcall.c            |  4 ++--
>   linux-user/mmap.c               |  4 ++--
>   linux-user/syscall.c            |  2 +-
>   plugins/core.c                  |  6 ++----
>   plugins/loader.c                |  2 +-
>   target/alpha/sys_helper.c       |  6 ------
>   target/alpha/translate.c        | 21 ++++++------------
>   target/riscv/csr.c              |  3 ---
>   target/riscv/tcg/tcg-cpu.c      |  3 ++-
>   20 files changed, 69 insertions(+), 97 deletions(-)
> 

Re: [PATCH 01/11] gdbstub: Remove tb_flush uses

[Pierrick Bouvier]

On 2025-09-06 07:18, Richard Henderson wrote:
> This hasn't been needed since d828b92b8a6
> ("accel/tcg: Introduce CF_BP_PAGE").
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   gdbstub/system.c | 4 ----
>   gdbstub/user.c   | 3 ---
>   2 files changed, 7 deletions(-)

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 02/11] accel/tcg: Split out tb_flush__exclusive

[Pierrick Bouvier]

On 2025-09-06 07:18, Richard Henderson wrote:
> Expose a routine to be called when no cpus are running.
> Simplify the do_tb_flush run_on_cpu callback, because
> that is explicitly called with start_exclusive; there
> is no need for the mmap_lock as well.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/exec/tb-flush.h |  1 +
>   accel/tcg/tb-maint.c    | 28 ++++++++++++++--------------
>   2 files changed, 15 insertions(+), 14 deletions(-)

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 04/11] target/riscv: Record misa_ext in TCGTBCPUState.cs_base

[Pierrick Bouvier]

On 2025-09-06 07:18, Richard Henderson wrote:
> The tb_flush within write_misa was incorrect.  It assumed
> that we could adjust the ISA of the current processor and
> discard all TB and all would be well.  But MISA is per vcpu,
> so globally flushing TB does not mean that the TB matches
> the MISA of any given vcpu.
> 
> By recording misa in the tb state, we ensure that the code
> generated matches the vcpu.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/riscv/csr.c         | 3 ---
>   target/riscv/tcg/tcg-cpu.c | 3 ++-
>   2 files changed, 2 insertions(+), 4 deletions(-)

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 05/11] accel/tcg: Move post-load tb_flush to vm_change_state hook

[Pierrick Bouvier]

On 2025-09-06 07:18, Richard Henderson wrote:
> We need not call tb_flush once per cpu, only once per vmload.
> Move the call from cpu_common_post_load to a tcg-specific
> vm_change_state_handler.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   accel/tcg/tcg-all.c  | 21 +++++++++++++++++++++
>   hw/core/cpu-system.c |  8 --------
>   2 files changed, 21 insertions(+), 8 deletions(-)

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 11/11] accel/tcg: Remove tb_flush

[Pierrick Bouvier]

On 2025-09-06 07:18, Richard Henderson wrote:
> All uses have been replaced with tb_flush__exclusive.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/exec/tb-flush.h | 19 ++++++++-----------
>   accel/tcg/tb-maint.c    | 22 ----------------------
>   2 files changed, 8 insertions(+), 33 deletions(-)

Well done!

Would that be interesting to add a
tcg_debug_assert(!current_cpu || cpu_in_serial_context(current_cpu));
to catch potential future issues?

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 05/11] accel/tcg: Move post-load tb_flush to vm_change_state hook

[Peter Xu]

On Sat, Sep 06, 2025 at 07:18:14AM +0200, Richard Henderson wrote:
> We need not call tb_flush once per cpu, only once per vmload.
> Move the call from cpu_common_post_load to a tcg-specific
> vm_change_state_handler.

The change looks correct.  Though the commit message here implies a
conversion from per-cpu flush to per-system flush, which IMHO is only part
of the change.

IIUC, the change is better than that, because previously when the flush is
in post_load(), it means the per-cpu flush will also happen in a live
migration, but here AFAIU the flush is only needed through a HMP
loadvm-styled migration, where there must be some stale data in the first
place. In case of live migrations there isn't any prior data to flush,
iiuc.

Maybe it would be nice to mention the other side of the changes in the
commit message too?  No strong feelings, though.

Thanks,

-- 
Peter Xu