[PATCH v3 00/10] accel/tcg: Improve tb_flush usage

[PATCH v3 00/10] 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.  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.

Changes for v3:
  - Rename to tb_flush__exclusive_or_serial; retain the serial
    check within, not updating to exclusive only.
  - Rename the existing tb_flush to queue_tb_flush; retain the
    sequence number check for duplicated flushes.
  - Drop EXCP_TB_FLUSH.
  - Improve buffer overflow flush when we're already serial.
  - Other minor comment/review updates.


r~


Richard Henderson (10):
  gdbstub: Remove tb_flush uses
  target/alpha: Simplify call_pal implementation
  target/riscv: Record misa_ext in TCGTBCPUState.cs_base
  hw/ppc/spapr: Use tb_invalidate_phys_range in h_page_init
  accel/tcg: Split out tb_flush__exclusive_or_serial
  accel/tcg: Move post-load tb_flush to vm_change_state hook
  plugins: Use tb_flush__exclusive_or_serial
  linux-user: Split out begin_parallel_context
  accel/tcg: Create queue_tb_flush from tb_flush
  accel/tcg: Improve buffer overflow in tb_gen_code

 include/exec/tb-flush.h     | 30 ++++++++++++++--------
 linux-user/user-internals.h | 16 ++++++++++++
 target/alpha/helper.h       |  1 -
 accel/tcg/plugin-gen.c      |  4 +--
 accel/tcg/tb-maint.c        | 50 +++++++++++++++++++++----------------
 accel/tcg/tcg-all.c         | 21 ++++++++++++++++
 accel/tcg/translate-all.c   |  6 ++++-
 gdbstub/system.c            |  5 ----
 gdbstub/user.c              |  3 ---
 hw/core/cpu-system.c        |  9 -------
 hw/ppc/spapr_hcall.c        |  4 +--
 linux-user/alpha/cpu_loop.c |  5 ----
 linux-user/mmap.c           | 13 ++--------
 linux-user/syscall.c        |  7 +-----
 plugins/core.c              |  6 ++---
 plugins/loader.c            |  3 +--
 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, 108 insertions(+), 108 deletions(-)

-- 
2.43.0

[PATCH v3 01/10] gdbstub: Remove tb_flush uses

[Richard Henderson]

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

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 gdbstub/system.c | 5 -----
 gdbstub/user.c   | 3 ---
 2 files changed, 8 deletions(-)

diff --git a/gdbstub/system.c b/gdbstub/system.c
index 5be0d3c58c..5221c579d9 100644
--- a/gdbstub/system.c
+++ b/gdbstub/system.c
@@ -18,13 +18,11 @@
 #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"
 #include "system/runstate.h"
 #include "system/replay.h"
-#include "system/tcg.h"
 #include "hw/core/cpu.h"
 #include "hw/cpu/cluster.h"
 #include "hw/boards.h"
@@ -174,9 +172,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 v3 02/10] 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().

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/alpha/helper.h       |  1 -
 linux-user/alpha/cpu_loop.c |  5 -----
 target/alpha/sys_helper.c   |  6 ------
 target/alpha/translate.c    | 21 ++++++---------------
 4 files changed, 6 insertions(+), 27 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/linux-user/alpha/cpu_loop.c b/linux-user/alpha/cpu_loop.c
index bb8346b509..f93597c400 100644
--- a/linux-user/alpha/cpu_loop.c
+++ b/linux-user/alpha/cpu_loop.c
@@ -94,11 +94,6 @@ void cpu_loop(CPUAlphaState *env)
                 break;
             case 0x86:
                 /* IMB */
-                /* ??? We can probably elide the code using page_unprotect
-                   that is checking for self-modifying code.  Instead we
-                   could simply call tb_flush here.  Until we work out the
-                   changes required to turn off the extra write protection,
-                   this can be a no-op.  */
                 break;
             case 0x9E:
                 /* RDUNIQUE */
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 v3 03/10] 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.

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
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 v3 04/10] 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.

Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
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 c594d4b916..8c1e0a4817 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, dst + len - 1);
         } else {
             g_assert_not_reached();
         }
-- 
2.43.0

Re: [PATCH v3 04/10] hw/ppc/spapr: Use tb_invalidate_phys_range in h_page_init

[Philippe Mathieu-Daudé]

On 23/9/25 23:54, Richard Henderson wrote:
> We only need invalidate tbs from a single page, not flush
> all translations.
> 
> Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   hw/ppc/spapr_hcall.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH v3 05/10] accel/tcg: Split out tb_flush__exclusive_or_serial

[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.

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

diff --git a/include/exec/tb-flush.h b/include/exec/tb-flush.h
index 142c240d94..090ffc8818 100644
--- a/include/exec/tb-flush.h
+++ b/include/exec/tb-flush.h
@@ -8,6 +8,21 @@
 #ifndef _TB_FLUSH_H_
 #define _TB_FLUSH_H_
 
+/**
+ * tb_flush__exclusive_or_serial()
+ *
+ * Used to flush all the translation blocks in the system.  Mostly this is
+ * used to empty the code generation buffer after it is full.  Sometimes it
+ * is used when it is simpler to flush everything than work out which
+ * individual translations are now invalid.
+ *
+ * Must be called from an exclusive or serial context, e.g. start_exclusive,
+ * vm_stop, or when there is only one vcpu.  Note that start_exclusive cannot
+ * be called from within the cpu run loop, so this cannot be called from
+ * within target code.
+ */
+void tb_flush__exclusive_or_serial(void);
+
 /**
  * tb_flush() - flush all translation blocks
  * @cs: CPUState (must be valid, but treated as anonymous pointer)
diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
index 0048316f99..7be9a1c4de 100644
--- a/accel/tcg/tb-maint.c
+++ b/accel/tcg/tb-maint.c
@@ -36,6 +36,9 @@
 #include "internal-common.h"
 #ifdef CONFIG_USER_ONLY
 #include "user/page-protection.h"
+#define runstate_is_running()  true
+#else
+#include "system/runstate.h"
 #endif
 
 
@@ -88,7 +91,10 @@ static IntervalTreeRoot tb_root;
 
 static void tb_remove_all(void)
 {
-    assert_memory_lock();
+    /*
+     * Only called from tb_flush__exclusive_or_serial, where we have already
+     * asserted that we're in an exclusive state.
+     */
     memset(&tb_root, 0, sizeof(tb_root));
 }
 
@@ -756,17 +762,19 @@ 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_or_serial(void)
 {
-    bool did_flush = false;
+    CPUState *cpu;
 
-    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;
+    assert(tcg_enabled());
+    /* Note that cpu_in_serial_context checks cpu_in_exclusive_context. */
+    assert(!runstate_is_running() ||
+           (current_cpu && cpu_in_serial_context(current_cpu)));
 
     CPU_FOREACH(cpu) {
         tcg_flush_jmp_cache(cpu);
@@ -778,11 +786,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_or_serial();
     }
 }
 
-- 
2.43.0

Re: [PATCH v3 05/10] accel/tcg: Split out tb_flush__exclusive_or_serial

[Philippe Mathieu-Daudé]

On 23/9/25 23:54, 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.
> 
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/exec/tb-flush.h | 15 +++++++++++++++
>   accel/tcg/tb-maint.c    | 39 +++++++++++++++++++++++++--------------
>   2 files changed, 40 insertions(+), 14 deletions(-)
> 
> diff --git a/include/exec/tb-flush.h b/include/exec/tb-flush.h
> index 142c240d94..090ffc8818 100644
> --- a/include/exec/tb-flush.h
> +++ b/include/exec/tb-flush.h
> @@ -8,6 +8,21 @@
>   #ifndef _TB_FLUSH_H_
>   #define _TB_FLUSH_H_
>   
> +/**
> + * tb_flush__exclusive_or_serial()
> + *
> + * Used to flush all the translation blocks in the system.  Mostly this is
> + * used to empty the code generation buffer after it is full.  Sometimes it
> + * is used when it is simpler to flush everything than work out which
> + * individual translations are now invalid.
> + *
> + * Must be called from an exclusive or serial context, e.g. start_exclusive,
> + * vm_stop, or when there is only one vcpu.  Note that start_exclusive cannot
> + * be called from within the cpu run loop, so this cannot be called from
> + * within target code.
> + */
> +void tb_flush__exclusive_or_serial(void);
> +
>   /**
>    * tb_flush() - flush all translation blocks
>    * @cs: CPUState (must be valid, but treated as anonymous pointer)
> diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
> index 0048316f99..7be9a1c4de 100644
> --- a/accel/tcg/tb-maint.c
> +++ b/accel/tcg/tb-maint.c
> @@ -36,6 +36,9 @@
>   #include "internal-common.h"
>   #ifdef CONFIG_USER_ONLY
>   #include "user/page-protection.h"
> +#define runstate_is_running()  true
> +#else
> +#include "system/runstate.h"
>   #endif
>   
>   
> @@ -88,7 +91,10 @@ static IntervalTreeRoot tb_root;
>   
>   static void tb_remove_all(void)
>   {
> -    assert_memory_lock();
> +    /*
> +     * Only called from tb_flush__exclusive_or_serial, where we have already
> +     * asserted that we're in an exclusive state.
> +     */
>       memset(&tb_root, 0, sizeof(tb_root));
>   }
>   
> @@ -756,17 +762,19 @@ 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_or_serial(void)
>   {
> -    bool did_flush = false;
> +    CPUState *cpu;
>   
> -    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;

Since reworking, here I'd add a tracing event to help debugging:

        trace_tb_flush();

> +    assert(tcg_enabled());
> +    /* Note that cpu_in_serial_context checks cpu_in_exclusive_context. */
> +    assert(!runstate_is_running() ||
> +           (current_cpu && cpu_in_serial_context(current_cpu)));
>   
>       CPU_FOREACH(cpu) {
>           tcg_flush_jmp_cache(cpu);
> @@ -778,11 +786,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_or_serial();
>       }
>   }
>   

[PATCH v3 06/10] 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.

By moving the call from cpu_common_post_load to a tcg-specific
vm_change_state_handler, we do even better than that: we only
flush when called from HMP triggered loadvm, when we had old
state to flush.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/tcg-all.c  | 21 +++++++++++++++++++++
 hw/core/cpu-system.c |  9 ---------
 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index 5125e1a4e2..18ea0c58b0 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_or_serial();
+    }
+}
+#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 09c928c1f9..f601a083d1 100644
--- a/hw/core/cpu-system.c
+++ b/hw/core/cpu-system.c
@@ -23,7 +23,6 @@
 #include "system/address-spaces.h"
 #include "exec/cputlb.h"
 #include "system/memory.h"
-#include "exec/tb-flush.h"
 #include "qemu/target-info.h"
 #include "hw/qdev-core.h"
 #include "hw/qdev-properties.h"
@@ -207,14 +206,6 @@ static int cpu_common_post_load(void *opaque, int version_id)
         cpu_reset_interrupt(cpu, 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 v3 07/10] plugins: Use tb_flush__exclusive_or_serial

[Richard Henderson]

In all cases, we are already within start_exclusive.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/plugin-gen.c | 4 ++--
 plugins/core.c         | 6 ++----
 plugins/loader.c       | 3 +--
 3 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 9920381a84..1ffcb4b2d2 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -102,8 +102,8 @@ static TCGv_i32 gen_cpu_index(void)
     /*
      * Optimize when we run with a single vcpu. All values using cpu_index,
      * including scoreboard index, will be optimized out.
-     * User-mode calls tb_flush when setting this flag. In system-mode, all
-     * vcpus are created before generating code.
+     * User-mode flushes all TBs when setting this flag.
+     * In system-mode, all vcpus are created before generating code.
      */
     if (!tcg_cflags_has(current_cpu, CF_PARALLEL)) {
         return tcg_constant_i32(current_cpu->cpu_index);
diff --git a/plugins/core.c b/plugins/core.c
index c6e9ef1478..ead09fd2f1 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_or_serial();
     }
     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_or_serial();
     end_exclusive();
 
     /* now it's safe to handle the exit case */
diff --git a/plugins/loader.c b/plugins/loader.c
index 8f0d75c904..ba10ebac99 100644
--- a/plugins/loader.c
+++ b/plugins/loader.c
@@ -377,8 +377,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_or_serial();
     plugin_reset_destroy(data);
 }
 
-- 
2.43.0

[PATCH v3 08/10] linux-user: Split out begin_parallel_context

[Richard Henderson]

Move the test/flush/set block to a new function.
Use tb_flush__exclusive_or_serial while we're at it.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/user-internals.h | 16 ++++++++++++++++
 linux-user/mmap.c           | 13 ++-----------
 linux-user/syscall.c        |  7 +------
 3 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/linux-user/user-internals.h b/linux-user/user-internals.h
index 691b9a1775..7099349ec8 100644
--- a/linux-user/user-internals.h
+++ b/linux-user/user-internals.h
@@ -20,6 +20,8 @@
 
 #include "user/thunk.h"
 #include "qemu/log.h"
+#include "exec/tb-flush.h"
+#include "exec/translation-block.h"
 
 extern char *exec_path;
 void init_task_state(TaskState *ts);
@@ -172,6 +174,20 @@ static inline int regpairs_aligned(CPUArchState *cpu_env, int num) { return 0; }
  */
 void preexit_cleanup(CPUArchState *env, int code);
 
+/**
+ * begin_parallel_context
+ * @cs: the CPU context
+ *
+ * Called when starting the second vcpu, or joining shared memory.
+ */
+static inline void begin_parallel_context(CPUState *cs)
+{
+    if (!tcg_cflags_has(cs, CF_PARALLEL)) {
+        tb_flush__exclusive_or_serial();
+        tcg_cflags_set(cs, CF_PARALLEL);
+    }
+}
+
 /*
  * Include target-specific struct and function definitions;
  * they may need access to the target-independent structures
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 002e1e668e..847092a28a 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -22,8 +22,6 @@
 #include "exec/log.h"
 #include "exec/page-protection.h"
 #include "exec/mmap-lock.h"
-#include "exec/tb-flush.h"
-#include "exec/translation-block.h"
 #include "qemu.h"
 #include "user/page-protection.h"
 #include "user-internals.h"
@@ -1007,11 +1005,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
      * be atomic with respect to an external process.
      */
     if (ret != -1 && (flags & MAP_TYPE) != MAP_PRIVATE) {
-        CPUState *cpu = thread_cpu;
-        if (!tcg_cflags_has(cpu, CF_PARALLEL)) {
-            tcg_cflags_set(cpu, CF_PARALLEL);
-            tb_flush(cpu);
-        }
+        begin_parallel_context(thread_cpu);
     }
 
     return ret;
@@ -1448,10 +1442,7 @@ abi_ulong target_shmat(CPUArchState *cpu_env, int shmid,
      * supported by the host -- anything that requires EXCP_ATOMIC will not
      * be atomic with respect to an external process.
      */
-    if (!tcg_cflags_has(cpu, CF_PARALLEL)) {
-        tcg_cflags_set(cpu, CF_PARALLEL);
-        tb_flush(cpu);
-    }
+    begin_parallel_context(cpu);
 
     if (qemu_loglevel_mask(CPU_LOG_PAGE)) {
         FILE *f = qemu_log_trylock();
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 91360a072c..101dc427a4 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -27,8 +27,6 @@
 #include "target_mman.h"
 #include "exec/page-protection.h"
 #include "exec/mmap-lock.h"
-#include "exec/tb-flush.h"
-#include "exec/translation-block.h"
 #include <elf.h>
 #include <endian.h>
 #include <grp.h>
@@ -6631,10 +6629,7 @@ static int do_fork(CPUArchState *env, unsigned int flags, abi_ulong newsp,
          * generate code for parallel execution and flush old translations.
          * Do this now so that the copy gets CF_PARALLEL too.
          */
-        if (!tcg_cflags_has(cpu, CF_PARALLEL)) {
-            tcg_cflags_set(cpu, CF_PARALLEL);
-            tb_flush(cpu);
-        }
+        begin_parallel_context(cpu);
 
         /* we create a new CPU instance. */
         new_env = cpu_copy(env);
-- 
2.43.0

Re: [PATCH v3 08/10] linux-user: Split out begin_parallel_context

[Philippe Mathieu-Daudé]

On 23/9/25 23:54, Richard Henderson wrote:
> Move the test/flush/set block to a new function.
> Use tb_flush__exclusive_or_serial while we're at it.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/user-internals.h | 16 ++++++++++++++++
>   linux-user/mmap.c           | 13 ++-----------
>   linux-user/syscall.c        |  7 +------
>   3 files changed, 19 insertions(+), 17 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH v3 09/10] accel/tcg: Create queue_tb_flush from tb_flush

[Richard Henderson]

Rename the function and remove the path which
performs the flush immediately.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/tb-flush.h   | 17 ++++++-----------
 accel/tcg/tb-maint.c      | 11 +++--------
 accel/tcg/translate-all.c |  2 +-
 3 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/include/exec/tb-flush.h b/include/exec/tb-flush.h
index 090ffc8818..e971d4ba6d 100644
--- a/include/exec/tb-flush.h
+++ b/include/exec/tb-flush.h
@@ -24,19 +24,14 @@
 void tb_flush__exclusive_or_serial(void);
 
 /**
- * tb_flush() - flush all translation blocks
- * @cs: CPUState (must be valid, but treated as anonymous pointer)
+ * queue_tb_flush() - add flush to the cpu work queue
+ * @cs: CPUState
  *
- * 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.
+ * Flush all translation blocks the next time @cs processes the work queue.
+ * This should generally be followed by cpu_loop_exit(), so that the work
+ * queue is processed promptly.
  */
-void tb_flush(CPUState *cs);
+void queue_tb_flush(CPUState *cs);
 
 void tcg_flush_jmp_cache(CPUState *cs);
 
diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
index 7be9a1c4de..5b1a5b599e 100644
--- a/accel/tcg/tb-maint.c
+++ b/accel/tcg/tb-maint.c
@@ -797,17 +797,12 @@ static void do_tb_flush(CPUState *cpu, run_on_cpu_data tb_flush_count)
     }
 }
 
-void tb_flush(CPUState *cpu)
+void queue_tb_flush(CPUState *cs)
 {
     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));
-        }
+        async_safe_run_on_cpu(cs, do_tb_flush,
+                              RUN_ON_CPU_HOST_INT(tb_flush_count));
     }
 }
 
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index d468667b0d..31bc0f8bc5 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -289,7 +289,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu, TCGTBCPUState s)
     tb = tcg_tb_alloc(tcg_ctx);
     if (unlikely(!tb)) {
         /* flush must be done */
-        tb_flush(cpu);
+        queue_tb_flush(cpu);
         mmap_unlock();
         /* Make the execution loop process the flush as soon as possible.  */
         cpu->exception_index = EXCP_INTERRUPT;
-- 
2.43.0

Re: [PATCH v3 09/10] accel/tcg: Create queue_tb_flush from tb_flush

[Philippe Mathieu-Daudé]

On 23/9/25 23:54, Richard Henderson wrote:
> Rename the function and remove the path which
> performs the flush immediately.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/exec/tb-flush.h   | 17 ++++++-----------
>   accel/tcg/tb-maint.c      | 11 +++--------
>   accel/tcg/translate-all.c |  2 +-
>   3 files changed, 10 insertions(+), 20 deletions(-)
> 
> diff --git a/include/exec/tb-flush.h b/include/exec/tb-flush.h
> index 090ffc8818..e971d4ba6d 100644
> --- a/include/exec/tb-flush.h
> +++ b/include/exec/tb-flush.h
> @@ -24,19 +24,14 @@
>   void tb_flush__exclusive_or_serial(void);
>   
>   /**
> - * tb_flush() - flush all translation blocks
> - * @cs: CPUState (must be valid, but treated as anonymous pointer)
> + * queue_tb_flush() - add flush to the cpu work queue
> + * @cs: CPUState
>    *
> - * 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.
> + * Flush all translation blocks the next time @cs processes the work queue.
> + * This should generally be followed by cpu_loop_exit(), so that the work
> + * queue is processed promptly.
>    */
> -void tb_flush(CPUState *cs);
> +void queue_tb_flush(CPUState *cs);
>   
>   void tcg_flush_jmp_cache(CPUState *cs);

Preferably rename as tcg_queue_tb_flush(), regardless:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH v3 10/10] accel/tcg: Improve buffer overflow in tb_gen_code

[Richard Henderson]

If we only have one vcpu, or within cpu_exec_step_atomic,
we needn't jump all the way back out to the cpu loop to
empty the code gen buffer.

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

diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 31bc0f8bc5..da9d7f1675 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -289,6 +289,10 @@ TranslationBlock *tb_gen_code(CPUState *cpu, TCGTBCPUState s)
     tb = tcg_tb_alloc(tcg_ctx);
     if (unlikely(!tb)) {
         /* flush must be done */
+        if (cpu_in_serial_context(cpu)) {
+            tb_flush__exclusive_or_serial();
+            goto buffer_overflow;
+        }
         queue_tb_flush(cpu);
         mmap_unlock();
         /* Make the execution loop process the flush as soon as possible.  */
-- 
2.43.0

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

[Philippe Mathieu-Daudé]

On 23/9/25 23:54, 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.  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.
> 
> Changes for v3:
>    - Rename to tb_flush__exclusive_or_serial; retain the serial
>      check within, not updating to exclusive only.
>    - Rename the existing tb_flush to queue_tb_flush; retain the
>      sequence number check for duplicated flushes.
>    - Drop EXCP_TB_FLUSH.

When using my split-accel branch, I had good results with v2.

I'm getting crashes with v3:

Termination Reason:    Namespace SIGNAL, Code 4 Illegal instruction: 4

Thread 4 Crashed:
0   ???                           	       0x34400fb00 ???
1   qemu-system-aarch64-unsigned  	       0x100fd74dc cpu_tb_exec + 160 (cpu-exec.c:441)
2   qemu-system-aarch64-unsigned  	       0x100fd829c cpu_loop_exec_tb + 32 (cpu-exec.c:897) [inlined]
3   qemu-system-aarch64-unsigned  	       0x100fd829c cpu_exec_loop + 904 (cpu-exec.c:1009)
4   qemu-system-aarch64-unsigned  	       0x100fd7a30 cpu_exec_setjmp + 48 (cpu-exec.c:1026)
5   qemu-system-aarch64-unsigned  	       0x100fd7954 cpu_exec + 496 (cpu-exec.c:1052)
6   qemu-system-aarch64-unsigned  	       0x100ff9bc4 tcg_cpu_exec + 44 (tcg-accel-ops.c:97)
7   qemu-system-aarch64-unsigned  	       0x100ffa4c4 mttcg_cpu_exec + 28 (tcg-accel-ops-mttcg.c:146)
8   qemu-system-aarch64-unsigned  	       0x10140bdac split_cpu_thread_routine + 540 (split-accel-ops.c:87)
9   qemu-system-aarch64-unsigned  	       0x10158f7e0 qemu_thread_start + 132 (qemu-thread-posix.c:393)

Thread 5:
0   qemu-system-aarch64-unsigned  	       0x100fb04e0 tcg_out_ld + 160 (tcg-target.c.inc:1268)
1   qemu-system-aarch64-unsigned  	       0x100fab3f8 tcg_reg_alloc_op + 240 [inlined]
2   qemu-system-aarch64-unsigned  	       0x100fab3f8 tcg_gen_code + 7932 (tcg.c:7018)
3   qemu-system-aarch64-unsigned  	       0x100fe9e88 setjmp_gen_code + 184 (translate-all.c:257)
4   qemu-system-aarch64-unsigned  	       0x100fe9780 tb_gen_code + 364 (translate-all.c:324)
5   qemu-system-aarch64-unsigned  	       0x100fd8138 cpu_exec_loop + 548 (cpu-exec.c:980)
6   qemu-system-aarch64-unsigned  	       0x100fd7a30 cpu_exec_setjmp + 48 (cpu-exec.c:1026)
7   qemu-system-aarch64-unsigned  	       0x100fd7954 cpu_exec + 496 (cpu-exec.c:1052)
8   qemu-system-aarch64-unsigned  	       0x100ff9bc4 tcg_cpu_exec + 44 (tcg-accel-ops.c:97)
9   qemu-system-aarch64-unsigned  	       0x100ffa4c4 mttcg_cpu_exec + 28 (tcg-accel-ops-mttcg.c:146)
10  qemu-system-aarch64-unsigned  	       0x10140bdac split_cpu_thread_routine + 540 (split-accel-ops.c:87)
11  qemu-system-aarch64-unsigned  	       0x10158f7e0 qemu_thread_start + 132 (qemu-thread-posix.c:393)

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

[Richard Henderson]

On 9/24/25 05:14, Philippe Mathieu-Daudé wrote:
> On 23/9/25 23:54, 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.  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.
>>
>> Changes for v3:
>>    - Rename to tb_flush__exclusive_or_serial; retain the serial
>>      check within, not updating to exclusive only.
>>    - Rename the existing tb_flush to queue_tb_flush; retain the
>>      sequence number check for duplicated flushes.
>>    - Drop EXCP_TB_FLUSH.
> 
> When using my split-accel branch, I had good results with v2.
> 
> I'm getting crashes with v3:

Are you sure you didn't just get lucky with v2?
There's little functional change to v3...

Point me at your branch?  While I can't test HVF, I can double-check at how you integrated 
the flush.


r~