[PATCH v4 5/9] accel/tcg: give the TB jump cache a second base pointer for generated code

[Matt Turner <mattst88@gmail.com>]

Add CPUState::tb_jmp_cache_probe, a base pointer that only generated code
will read. Normally it is cpu->tb_jmp_cache. Pointing it instead at a
shared, permanently zero-filled CPUJumpCache makes every entry generated
code finds have a NULL tb, so every lookup done through it misses. The
real jump cache is not touched, so nothing is lost and recovery is a single
store.

Nothing reads it yet. The next patch probes the jump cache from generated
code, and that probe cannot check everything helper_lookup_tb_ptr() checks;
poisoning this pointer is how the conditions it cannot check force it back
into the helper. The one that matters here is breakpoints.
check_for_breakpoints() raises EXCP_DEBUG on an exact pc match and selects
CF_BP_PAGE cflags for the rest of the page, and inserting a breakpoint
deliberately invalidates no TB, so a block translated before the breakpoint
was set is still sitting in the jump cache and would be dispatched to
directly.

cpu_breakpoint_insert() poisons the target CPU, rather than leaving it to
that CPU's own main loop, because gdb inserts a breakpoint into every CPU
(tcg_insert_gdbstub_breakpoint()) and a thread already inside generated
code dispatching to itself need never return to its main loop. The main
loop puts the pointer back once the last breakpoint is gone, re-checking
after the store so that it loses a race with a concurrent insert in the
safe direction.

The poison cache is a plain static rather than a const one so that it lands
in .bss: a megabyte of const zeroes would be a megabyte of .rodata in every
emulator binary, whereas .bss costs nothing on disk and faults in only the
handful of pages a poisoned run happens to probe.

v4: Split out of "tcg: probe the TB jump cache inline instead of calling a
    helper". Requested by Richard Henderson.

v4: Make the poison a static object rather than allocating one on first
    use. Suggested by Richard Henderson, who asked for const; see above for
    why it is not.

Signed-off-by: Matt Turner <mattst88@gmail.com>
---
 accel/stubs/tcg-stub.c      |  6 ++-
 accel/tcg/cpu-exec.c        | 96 +++++++++++++++++++++++++++++++++++++
 accel/tcg/internal-common.h |  2 +
 cpu-common.c                | 11 +++++
 include/hw/core/cpu.h       |  9 ++++
 include/system/tcg.h        |  9 ++++
 6 files changed, 132 insertions(+), 1 deletion(-)

diff --git ./accel/stubs/tcg-stub.c ./accel/stubs/tcg-stub.c
index f9e1bd22d6..8298e4a1f5 100644
--- ./accel/stubs/tcg-stub.c
+++ ./accel/stubs/tcg-stub.c
@@ -1,6 +1,6 @@
 /*
  * 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.
+ * cpu-target.c, cpu-common.c or the HMP command handlers but not TCG.
  *
  * SPDX-License-Identifier: GPL-2.0-or-later
  */
@@ -14,3 +14,7 @@ void tcg_update_cflags(CPUState *cpu)
 void tcg_update_all_cflags(void)
 {
 }
+
+void tcg_cpu_poison_jmp_cache(CPUState *cpu)
+{
+}
diff --git ./accel/tcg/cpu-exec.c ./accel/tcg/cpu-exec.c
index 148e0f583e..c2a9679cd7 100644
--- ./accel/tcg/cpu-exec.c
+++ ./accel/tcg/cpu-exec.c
@@ -752,6 +752,93 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
     return false;
 }
 
+/*
+ * The inline jump cache probe reads cpu->tb_jmp_cache_probe and takes the
+ * slow path when the entry it finds has a NULL tb.  Pointing the probe at a
+ * region that is all zeroes therefore forces every indirect dispatch into
+ * helper_lookup_tb_ptr(), which does the full lookup the inline probe only
+ * approximates.  The real jump cache is untouched, so no contents are lost
+ * and recovery is a single store.
+ *
+ * Only ever read from, and only one entry per dispatch, so one shared
+ * zero-filled cache is enough for every CPU.  Not const: that would put a
+ * megabyte of zeroes in .rodata and so in the binary, where .bss costs
+ * nothing on disk and only faults in the handful of pages a poisoned run
+ * happens to probe.
+ */
+static CPUJumpCache tb_jmp_cache_poison;
+
+/*
+ * Whether the generated code may dispatch to the next block by itself.
+ *
+ * The inline probe matches on the destination pc and on the flags and
+ * cflags the dispatching block was translated with.  It does not consult
+ * cpu->breakpoints, so it must not run while one is set: setting a
+ * breakpoint deliberately invalidates nothing, and check_for_breakpoints()
+ * both raises EXCP_DEBUG on an exact match and picks CF_BP_PAGE cflags for
+ * the rest of the page.  A block translated before the breakpoint was set is
+ * therefore still in the jump cache, and dispatching to it inline would step
+ * straight over the breakpoint.
+ */
+static bool tcg_cpu_may_dispatch(CPUState *cpu)
+{
+    return QTAILQ_EMPTY(&cpu->breakpoints);
+}
+
+/*
+ * Poison @cpu's probe, from any thread.  Called when a breakpoint is
+ * inserted, which is what makes the poison take effect at the dispatch
+ * after the insert rather than whenever @cpu next reaches its main loop:
+ * a vCPU chaining indirectly need never reach it, and would run past a
+ * breakpoint another thread had just set.
+ *
+ * A plain store is enough.  The value only ever costs a slow path that is
+ * correct on its own, and the generated code re-reads the base on every
+ * dispatch.  Un-poisoning is tcg_cpu_sync_jmp_cache()'s job.
+ */
+void tcg_cpu_poison_jmp_cache(CPUState *cpu)
+{
+    if (qatomic_read(&cpu->tb_jmp_cache_probe) != NULL) {
+        qatomic_set(&cpu->tb_jmp_cache_probe, &tb_jmp_cache_poison);
+    }
+}
+
+/*
+ * Called from the main loop, which is the only context that can establish
+ * that no reason to be poisoned is left.  Cheap enough to call every time
+ * round: the common case is a load, a compare and no store at all.
+ */
+void tcg_cpu_sync_jmp_cache(CPUState *cpu)
+{
+    CPUJumpCache *want;
+
+    if (qatomic_read(&cpu->tb_jmp_cache_probe) == NULL) {
+        return;  /* not realized, or already unrealized */
+    }
+
+    want = tcg_cpu_may_dispatch(cpu)
+           ? cpu->tb_jmp_cache
+           : &tb_jmp_cache_poison;
+
+    if (qatomic_read(&cpu->tb_jmp_cache_probe) != want) {
+        qatomic_set(&cpu->tb_jmp_cache_probe, want);
+
+        if (want == cpu->tb_jmp_cache) {
+            /*
+             * Un-poisoning races a concurrent tcg_cpu_poison_jmp_cache():
+             * the reason may have appeared after tcg_cpu_may_dispatch() read
+             * it, and the poison may have landed before the store above.
+             * Order that store against the re-read below, so that the race
+             * is lost in the safe direction.
+             */
+            smp_mb();
+            if (!tcg_cpu_may_dispatch(cpu)) {
+                tcg_cpu_poison_jmp_cache(cpu);
+            }
+        }
+    }
+}
+
 void tcg_kick_vcpu_thread(CPUState *cpu)
 {
     /*
@@ -964,6 +1051,13 @@ cpu_exec_loop(CPUState *cpu, SyncClocks *sc)
                 break;
             }
 
+            /*
+             * Reaching here means the main loop has just re-evaluated
+             * everything the inline probe assumes, so this is where the
+             * probe is allowed to come back after a poison.
+             */
+            tcg_cpu_sync_jmp_cache(cpu);
+
             tb = tb_lookup(cpu, s);
             if (tb == NULL) {
                 CPUJumpCache *jc;
@@ -1072,6 +1166,7 @@ bool tcg_exec_realizefn(CPUState *cpu, Error **errp)
     tcg_update_cflags(cpu);
 
     cpu->tb_jmp_cache = g_new0(CPUJumpCache, 1);
+    qatomic_set(&cpu->tb_jmp_cache_probe, cpu->tb_jmp_cache);
     tlb_init(cpu);
 #ifndef CONFIG_USER_ONLY
     tcg_iommu_init_notifier_list(cpu);
@@ -1089,5 +1184,6 @@ void tcg_exec_unrealizefn(CPUState *cpu)
 #endif /* !CONFIG_USER_ONLY */
 
     tlb_destroy(cpu);
+    qatomic_set(&cpu->tb_jmp_cache_probe, NULL);
     g_free_rcu(cpu->tb_jmp_cache, rcu);
 }
diff --git ./accel/tcg/internal-common.h ./accel/tcg/internal-common.h
index 853d1b51ee..9d1f6712d6 100644
--- ./accel/tcg/internal-common.h
+++ ./accel/tcg/internal-common.h
@@ -144,6 +144,8 @@ void page_table_config_init(void);
 G_NORETURN void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr);
 #endif /* CONFIG_USER_ONLY */
 
+void tcg_cpu_sync_jmp_cache(CPUState *cpu);
+
 void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr);
 void tb_set_jmp_target(TranslationBlock *tb, int n, uintptr_t addr);
 
diff --git ./cpu-common.c ./cpu-common.c
index adb76b3a78..3aed0156e6 100644
--- ./cpu-common.c
+++ ./cpu-common.c
@@ -22,6 +22,7 @@
 #include "exec/cpu-common.h"
 #include "hw/core/cpu.h"
 #include "qemu/lockable.h"
+#include "system/tcg.h"
 #include "trace/trace-root.h"
 
 QemuMutex qemu_cpu_list_lock;
@@ -429,6 +430,16 @@ int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int flags,
         *breakpoint = bp;
     }
 
+    /*
+     * Nothing is invalidated here, so blocks translated before this point
+     * are still live and still dispatch to each other without consulting
+     * cpu->breakpoints.  Stop the ones that can: a TCG vCPU dispatching
+     * inline reads a base pointer that this poisons, so the next dispatch
+     * takes the slow path and sees the new breakpoint.  @cpu may be another
+     * thread, and may be running.
+     */
+    tcg_cpu_poison_jmp_cache(cpu);
+
     trace_breakpoint_insert(cpu->cpu_index, pc, flags);
     return 0;
 }
diff --git ./include/hw/core/cpu.h ./include/hw/core/cpu.h
index 81af7b9ee1..bd2cdd2a0b 100644
--- ./include/hw/core/cpu.h
+++ ./include/hw/core/cpu.h
@@ -519,6 +519,15 @@ struct CPUState {
     MemoryRegion *memory;
 
     struct CPUJumpCache *tb_jmp_cache;
+    /*
+     * @tb_jmp_cache_probe: base the inline jump cache probe reads.
+     *
+     * Normally @tb_jmp_cache.  Pointed at a shared page of zeroes to force
+     * every inline dispatch to miss and fall back to helper_lookup_tb_ptr();
+     * see tcg_cpu_sync_jmp_cache().  NULL before tcg_exec_realizefn() and
+     * after tcg_exec_unrealizefn().
+     */
+    struct CPUJumpCache *tb_jmp_cache_probe;
 
     GArray *gdb_regs;
     int gdb_num_regs;
diff --git ./include/system/tcg.h ./include/system/tcg.h
index 2c2dbc753b..bf05db1329 100644
--- ./include/system/tcg.h
+++ ./include/system/tcg.h
@@ -29,6 +29,15 @@ extern bool tcg_allowed;
 void tcg_update_cflags(CPUState *cpu);
 void tcg_update_all_cflags(void);
 
+/*
+ * Force @cpu's generated code back into the slow dispatch path, which
+ * re-checks everything the inline jump cache probe assumes.  Safe to call
+ * from any thread, and a no-op for a CPU that is not running TCG.  Call
+ * whenever something the probe cannot see changes under a running vCPU;
+ * the main loop undoes it once the reason is gone.
+ */
+void tcg_cpu_poison_jmp_cache(CPUState *cpu);
+
 /**
  * qemu_tcg_mttcg_enabled:
  * Check whether we are running MultiThread TCG or not.
-- 
2.54.0