[PATCH v5 1/9] accel/tcg: fold the dynamic cflags into CPUState::tcg_cflags

[Matt Turner <mattst88@gmail.com>]

curr_cflags() is called once per TB dispatch, from helper_lookup_tb_ptr()
and from the cpu_exec() loop. It recomputes the same value every time:

    uint32_t cflags = cpu->tcg_cflags;
    if (unlikely(cpu_single_stepping(cpu))) { ... }
    else if (qatomic_read(&one_insn_per_tb)) { ... }
    else if (qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) { ... }

That is three loads and three branches on the hottest path in the
interpreter, for state that changes only when gdb enables single-step,
when one-insn-per-tb is toggled, or when the log mask changes.

None of the three has to be sampled at dispatch time. Fold each into
CPUState::tcg_cflags where it changes and curr_cflags() becomes a single
load of a field that TB lookup has to read anyway.

The derived bits -- CF_COUNT_MASK, CF_NO_GOTO_TB, CF_NO_GOTO_PTR and
CF_SINGLE_STEP -- are never set by tcg_cflags_set(), so tcg_update_cflags()
can recompute them in place without disturbing the rest, and conversely
tcg_cflags_set() ORs in its bits without disturbing them.

There are three places to call it:

  - tcg_exec_realizefn(), so that a CPU created after the command line has
    been parsed starts out with the right value. This covers user-only,
    where tcg_cpu_init_cflags() is not reached. linux-user's cpu_copy()
    copies tcg_cflags wholesale, so a cloned thread inherits it.

  - cpu_single_step(), which changes one CPU.  gdb is the only caller that
    matters; in system mode it runs with the vCPUs stopped, and in user mode
    gdb_continue_partial() can reach a thread that is still running, because
    gdb_handlesig() stops only the thread that trapped. That is exactly the
    plain cross-thread store to another CPU's CPUState that
    cpu->singlestep_flags already was, read back by that CPU through
    cpu_single_stepping() in curr_cflags(). This patch changes which field
    carries it, not who writes it or how.

  - hmp_one_insn_per_tb() and hmp_log(), which change every CPU while the
    vCPUs are running, so the update is queued with async_run_on_cpu() and
    each CPU writes its own cflags from its own thread. The command line
    spellings of those two settings need nothing: they are parsed before
    any CPU is realized, so tcg_exec_realizefn() picks them up.

Measured with qemu-alpha running an emulated alpha gcc 16.2.0 compiling
the SQLite 3.45.1 amalgamation (255k lines, -O2) on an x86-64 host, in a
build configured with --enable-lto:

    before: 1,646,994,254,249 instructions
    after:  1,562,204,796,597 instructions   -5.15%

That workload issues 8.4 billion dispatches, so the per-call saving is
small but the aggregate is not. The emulated compiler produces
byte-identical output before and after.

Wall clock does not move: 133.19s to 132.58s, a 0.46% difference against a
run-to-run spread larger than that. The removed work is a few predictable
loads and branches that the host executes largely in parallel with the
surrounding dispatch, so this patch is worth taking for the instruction
count and for what it enables, not for a time saving that can be measured
on its own.

v4: Update the cflags from the HMP handlers for 'log' and 'one-insn-per-tb'
    rather than from qemu_set_log_internal() and the accelerator property
    setter. Those are the paths that reach a running vCPU, and the monitor
    is the only thing that does. Suggested by Richard Henderson.

v4: Queue the per-CPU update with async_run_on_cpu() rather than
    async_safe_run_on_cpu(). Halting the other vCPUs buys nothing: the
    queued work already runs on the owning CPU's own thread. Suggested by
    Alex Bennee, who also asked whether there are cross-vCPU updates of
    tcg_cflags at all. With this change the monitor path has none: the
    only remaining writer from another thread is cpu_single_step(), above,
    which is neither new nor made worse here.

v4: Move the stub to accel/stubs/, which is where the other accelerator
    stubs live.

Signed-off-by: Matt Turner <mattst88@gmail.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/stubs/meson.build     |  1 +
 accel/stubs/tcg-stub.c      | 16 ++++++++++++++++
 accel/tcg/cpu-exec-common.c | 33 ++++++++++++++++++++++++++++++---
 accel/tcg/cpu-exec.c        |  3 +++
 accel/tcg/internal-common.h | 11 +++++++++--
 cpu-target.c                |  3 +++
 include/system/tcg.h        | 12 ++++++++++++
 monitor/hmp-cmds.c          |  5 +++++
 system/runstate-hmp-cmds.c  |  4 ++++
 9 files changed, 83 insertions(+), 5 deletions(-)
 create mode 100644 accel/stubs/tcg-stub.c

diff --git ./accel/stubs/meson.build ./accel/stubs/meson.build
index 7c6d7ad943..ccad583e64 100644
--- ./accel/stubs/meson.build
+++ ./accel/stubs/meson.build
@@ -4,6 +4,7 @@ stub_ss.add(files(
   'nitro-stub.c',
   'mshv-stub.c',
   'nvmm-stub.c',
+  'tcg-stub.c',
   'whpx-stub.c',
   'xen-stub.c',
 ))
diff --git ./accel/stubs/tcg-stub.c ./accel/stubs/tcg-stub.c
new file mode 100644
index 0000000000..f9e1bd22d6
--- /dev/null
+++ ./accel/stubs/tcg-stub.c
@@ -0,0 +1,16 @@
+/*
+ * Stubs for the TCG entry points in system/tcg.h, for binaries that link
+ * cpu-target.c or the HMP command handlers but not TCG.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include "qemu/osdep.h"
+#include "system/tcg.h"
+
+void tcg_update_cflags(CPUState *cpu)
+{
+}
+
+void tcg_update_all_cflags(void)
+{
+}
diff --git ./accel/tcg/cpu-exec-common.c ./accel/tcg/cpu-exec-common.c
index 44e84344f3..9f3517f36b 100644
--- ./accel/tcg/cpu-exec-common.c
+++ ./accel/tcg/cpu-exec-common.c
@@ -36,9 +36,16 @@ void tcg_cflags_set(CPUState *cpu, uint32_t flags)
     cpu->tcg_cflags |= flags;
 }
 
-uint32_t curr_cflags(CPUState *cpu)
+/*
+ * The bits of CPUState::tcg_cflags that tcg_cflags_set() never sets, because
+ * they are derived from gdb single-step, one-insn-per-tb and -d nochain.
+ */
+#define CF_DERIVED  (CF_COUNT_MASK | CF_NO_GOTO_TB | CF_NO_GOTO_PTR | \
+                     CF_SINGLE_STEP)
+
+void tcg_update_cflags(CPUState *cpu)
 {
-    uint32_t cflags = cpu->tcg_cflags;
+    uint32_t cflags = cpu->tcg_cflags & ~CF_DERIVED;
 
     /*
      * Record gdb single-step.  We should be exiting the TB by raising
@@ -55,7 +62,27 @@ uint32_t curr_cflags(CPUState *cpu)
         cflags |= CF_NO_GOTO_TB;
     }
 
-    return cflags;
+    cpu->tcg_cflags = cflags;
+}
+
+static void tcg_update_cflags_work(CPUState *cpu, run_on_cpu_data data)
+{
+    tcg_update_cflags(cpu);
+}
+
+void tcg_update_all_cflags(void)
+{
+    CPUState *cpu;
+
+    /*
+     * one-insn-per-tb and -d nochain can both be changed from the monitor
+     * while the vCPUs are running.  Queue the update onto each CPU rather
+     * than writing tcg_cflags from here, so that the field is only ever
+     * written by the CPU that owns it.
+     */
+    CPU_FOREACH(cpu) {
+        async_run_on_cpu(cpu, tcg_update_cflags_work, RUN_ON_CPU_NULL);
+    }
 }
 
 /* exit the current TB, but without causing any exception to be raised */
diff --git ./accel/tcg/cpu-exec.c ./accel/tcg/cpu-exec.c
index 257211235d..148e0f583e 100644
--- ./accel/tcg/cpu-exec.c
+++ ./accel/tcg/cpu-exec.c
@@ -1068,6 +1068,9 @@ bool tcg_exec_realizefn(CPUState *cpu, Error **errp)
         tcg_target_initialized = true;
     }
 
+    /* Pick up one-insn-per-tb and -d nochain from the command line. */
+    tcg_update_cflags(cpu);
+
     cpu->tb_jmp_cache = g_new0(CPUJumpCache, 1);
     tlb_init(cpu);
 #ifndef CONFIG_USER_ONLY
diff --git ./accel/tcg/internal-common.h ./accel/tcg/internal-common.h
index 9e7be2d78d..853d1b51ee 100644
--- ./accel/tcg/internal-common.h
+++ ./accel/tcg/internal-common.h
@@ -69,8 +69,15 @@ void tlb_destroy(CPUState *cpu);
 bool tcg_exec_realizefn(CPUState *cpu, Error **errp);
 void tcg_exec_unrealizefn(CPUState *cpu);
 
-/* current cflags for hashing/comparison */
-uint32_t curr_cflags(CPUState *cpu);
+/*
+ * Current cflags for hashing/comparison.  Everything that feeds into the
+ * value is folded into CPUState::tcg_cflags when it changes, by
+ * tcg_update_cflags(), so that TB dispatch only has to load it.
+ */
+static inline uint32_t curr_cflags(CPUState *cpu)
+{
+    return cpu->tcg_cflags;
+}
 
 void tb_check_watchpoint(CPUState *cpu, uintptr_t retaddr);
 
diff --git ./cpu-target.c ./cpu-target.c
index 4783845c9b..50be591acf 100644
--- ./cpu-target.c
+++ ./cpu-target.c
@@ -24,6 +24,7 @@
 #include "exec/replay-core.h"
 #include "exec/log.h"
 #include "hw/core/cpu.h"
+#include "system/tcg.h"
 #include "trace/trace-root.h"
 
 /* enable or disable single step mode. EXCP_DEBUG is returned by the
@@ -35,6 +36,8 @@ void cpu_single_step(CPUState *cpu, unsigned flags)
                                           cpu->singlestep_flags, flags);
         cpu->singlestep_flags = flags;
 
+        tcg_update_cflags(cpu);
+
 #if !defined(CONFIG_USER_ONLY)
         const AccelOpsClass *ops = cpus_get_accel();
         if (ops->update_guest_debug) {
diff --git ./include/system/tcg.h ./include/system/tcg.h
index 7622dcea30..2c2dbc753b 100644
--- ./include/system/tcg.h
+++ ./include/system/tcg.h
@@ -17,6 +17,18 @@ extern bool tcg_allowed;
 #define tcg_enabled() 0
 #endif
 
+/*
+ * Recompute the parts of CPUState::tcg_cflags that TB dispatch consumes but
+ * tcg_cflags_set() does not provide: gdb single-step, one-insn-per-tb and
+ * the CPU_LOG_TB_NOCHAIN log flag.  Call whenever one of those changes.
+ *
+ * tcg_update_cflags() updates one CPU and must be called from that CPU's
+ * thread, or with it stopped.  tcg_update_all_cflags() updates every CPU
+ * and is safe to call from the monitor while the vCPUs run.
+ */
+void tcg_update_cflags(CPUState *cpu);
+void tcg_update_all_cflags(void);
+
 /**
  * qemu_tcg_mttcg_enabled:
  * Check whether we are running MultiThread TCG or not.
diff --git ./monitor/hmp-cmds.c ./monitor/hmp-cmds.c
index 4e8d996dbb..b83551ea54 100644
--- ./monitor/hmp-cmds.c
+++ ./monitor/hmp-cmds.c
@@ -39,6 +39,7 @@
 #include "system/hw_accel.h"
 #include "system/memory.h"
 #include "system/system.h"
+#include "system/tcg.h"
 #include "disas/disas.h"
 
 /* Please update hmp-commands.hx when adding or changing commands */
@@ -335,7 +336,11 @@ void hmp_log(Monitor *mon, const QDict *qdict)
 
     if (!qemu_set_log(mask, &err)) {
         error_report_err(err);
+        return;
     }
+
+    /* CPU_LOG_TB_NOCHAIN feeds into the per-CPU cflags. */
+    tcg_update_all_cflags();
 }
 
 void hmp_gdbserver(Monitor *mon, const QDict *qdict)
diff --git ./system/runstate-hmp-cmds.c ./system/runstate-hmp-cmds.c
index 02d1d42bf3..86754a37f8 100644
--- ./system/runstate-hmp-cmds.c
+++ ./system/runstate-hmp-cmds.c
@@ -22,6 +22,7 @@
 #include "qapi/qapi-commands-run-state.h"
 #include "qobject/qdict.h"
 #include "qemu/accel.h"
+#include "system/tcg.h"
 
 void hmp_info_status(Monitor *mon, const QDict *qdict)
 {
@@ -64,6 +65,9 @@ void hmp_one_insn_per_tb(Monitor *mon, const QDict *qdict)
     /* If the property exists then setting it can never fail */
     object_property_set_bool(OBJECT(accel), "one-insn-per-tb",
                              newval, &error_abort);
+
+    /* one-insn-per-tb feeds into the per-CPU cflags. */
+    tcg_update_all_cflags();
 }
 
 void hmp_watchdog_action(Monitor *mon, const QDict *qdict)
-- 
2.54.0