accel/stubs/meson.build | 1 + accel/stubs/tcg-stub.c | 20 ++ accel/tcg/cpu-exec-common.c | 33 +- accel/tcg/cpu-exec.c | 116 ++++++ accel/tcg/internal-common.h | 13 +- accel/tcg/tb-jmp-cache.h | 2 +- accel/tcg/tcg-accel-ops.c | 1 + accel/tcg/translator.c | 94 ++++- cpu-common.c | 11 + cpu-target.c | 3 + gdbstub/user.c | 14 + include/gdbstub/user.h | 11 + include/hw/core/cpu.h | 11 + include/system/tcg.h | 21 ++ include/tcg/tcg-op-common.h | 15 +- include/tcg/tcg-op.h | 12 + include/tcg/tcg-opc.h | 9 +- include/tcg/tcg.h | 2 + monitor/hmp-cmds.c | 5 + system/runstate-hmp-cmds.c | 4 + target/alpha/translate.c | 4 +- target/arm/tcg/translate-a64.c | 4 +- target/arm/tcg/translate.c | 10 +- target/avr/translate.c | 4 +- target/hexagon/translate.c | 4 +- target/hppa/translate.c | 6 +- target/i386/tcg/translate.c | 2 +- .../tcg/insn_trans/trans_branch.c.inc | 2 +- target/loongarch/tcg/translate.c | 4 +- target/m68k/translate.c | 2 +- target/microblaze/translate.c | 4 +- target/mips/tcg/nanomips_translate.c.inc | 2 +- target/mips/tcg/translate.c | 6 +- target/or1k/translate.c | 4 +- target/ppc/translate.c | 4 +- target/riscv/tcg/insn_trans/trans_rvzce.c.inc | 4 +- target/riscv/tcg/translate.c | 2 +- target/rx/translate.c | 4 +- target/s390x/tcg/translate.c | 5 +- target/sh4/translate.c | 4 +- target/sparc/translate.c | 4 +- target/tricore/translate.c | 4 +- tcg/tcg-op-ldst.c | 3 +- tcg/tcg-op.c | 142 +++++++- tcg/tcg.c | 132 ++++++- tcg/x86_64/tcg-target.c.inc | 41 +++ tcg/x86_64/tcg-target.h | 3 + tests/tcg/multiarch/Makefile.target | 12 +- tests/tcg/multiarch/gdbstub/xpage-bp.py | 37 ++ tests/tcg/multiarch/test-indirect-irq.c | 62 ++++ tests/tcg/multiarch/test-xpage-chain.c | 336 ++++++++++++++++++ 51 files changed, 1192 insertions(+), 63 deletions(-) create mode 100644 accel/stubs/tcg-stub.c create mode 100644 tests/tcg/multiarch/gdbstub/xpage-bp.py create mode 100644 tests/tcg/multiarch/test-indirect-irq.c create mode 100644 tests/tcg/multiarch/test-xpage-chain.c
For guests running large amounts of code, most of what TCG executes is not
translated guest work but the fixed overhead around it. Blocks are short and
there are a great many of them, so the constant cost at each end of a block
(the interrupt poll and the can_do_io stores on entry, the dispatch on exit)
ends up dominating everything else.
The workload throughout is 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 --static --enable-lto --target-list=alpha-linux-user build. It
executes 34.2 billion TBs at 6.04 guest instructions each, and 24.6% of its
TB exits cannot use goto_tb. That is a representative shape for any guest
whose text is much larger than a page: indirect calls and returns
everywhere, plus direct branches that merely crossed a page boundary.
The first three patches are ordinary cleanups that stand on their own; patch
3 picked up review tags in v2. Patches 4 and 5 are preparation split out of
v3's patch 4 at Richard's request and move nothing on their own. The
remaining four are marked RFC individually and are where the interesting
questions are.
1 accel/tcg: fold the dynamic cflags into CPUState::tcg_cflags
curr_cflags() recomputed three unlikely tests on every one of the run's
8.4 billion dispatches, from state that changes only when gdb enables
single-step, when one-insn-per-tb is toggled, or when the log mask
moves. Fold each into tcg_cflags where it changes. -5.15%
2 accel/tcg: enlarge the TB jump cache to 64K entries
4096 entries is too small for a guest running a large program;
tb_htable_lookup() is 6.10% of samples. 16 bits is the knee of the
sizing curve, at 1 MiB per CPUState. -5.92%, -8.71% wall
3 accel/tcg: skip the can_do_io stores in user-only builds
Two stores per TB that nothing in a user-only build reads: 68 billion
of them over the run. -4.57%, -4.53% wall
4 tcg: pass the destination to tcg_gen_lookup_and_goto_ptr()
Preparation. The destination PC is already in a TCG temp at every one
of the 38 call sites; give the helper wrapper the option of taking it
rather than discarding it. Five targets pass it; the rest pass NULL
and keep today's behavior.
5 accel/tcg: give the TB jump cache a second base pointer for generated
code
Preparation. CPUState::tb_jmp_cache_probe is a base pointer only
generated code reads. Pointing it at a shared zero-filled cache makes
every lookup through it miss, which is how the conditions an inline
probe cannot check force it back into the helper. Nothing reads it
yet.
6 RFC: tcg: probe the TB jump cache inline instead of calling a helper
95.8% of those 8.4 billion helper_lookup_tb_ptr() calls hit the jump
cache. Emit the probe inline (hash, four guarded loads, goto_ptr) and
call the helper only on a miss. -34.67%, -25.94% wall
7 RFC: accel/tcg: allow cross-page goto_tb chaining in user-only builds
translator_use_goto_tb() refuses to chain across a page. In user-only
builds the invalidation path already covers what that was protecting
against: every mmap/mprotect/munmap reaches page_set_flags(), which
invalidates and unlinks. Lift it there for runs that can never acquire
a breakpoint, keep it for system mode. -2.75%, -4.84% wall
8 RFC: accel/tcg: poison the jump cache instead of polling for indirect
exits
A block only needs the icount_decr poll if it can leave by goto_tb;
every other exit already passes through a dispatch. Poison the probe
pointer from patch 5 when an exit is requested, so every dispatch
misses into the helper, which returns the epilogue. Emit the poll only
in blocks that emitted a goto_tb. -2.52%, -1.96% wall
9 RFC: tcg: fold a guest displacement into the host addressing mode
tcg_gen_qemu_ld/st cannot express a based access, so a target with a
displacement in its encodings materializes the address with an lea
that the host addressing mode would have done for free. Fold a
preceding constant add into a new argument on the op, opt-in per
backend, wired up for x86_64 user-only. -5.70%, -3.20% wall
Each percentage is against the patch before it. Every stage was measured in
one session on the same host, so end to end, from an unmodified LTO build of
the same base to the full series:
instructions retired: 1,646,994,254,249 -> 819,262,147,022 -50.26%
wall clock: 133.19s -> 77.30s -41.96%
Those are the v3 measurements. The machine they were taken on is busy, so v4
has not been re-measured. Nothing in the v4 changes is expected to move them
-- the splits are pure reorganization, the new alignment test in patch 9
reaches the same answer for everything the alpha frontend emits, and the
emulated compiler's output is unchanged -- but they are not a measurement of
this posting and should not be read as one.
The two figures do not track each other, and that is the interesting part:
what the series removes is cheap, well-predicted, highly pipelined work, so
it retires far more instructions than it saves time. Patch 6 also cuts L1
icache load misses by 39.0%, because a dispatch no longer jumps into qemu's
.text and evicts translated code; qemu's own .text falls from 38.8% to 5.3%
of profile samples.
Every step builds and runs on its own, so the series bisects, and the
emulated compiler produces byte-identical assembly output at every step,
which is the correctness check these patches most need. Three new tests
cover the hazards the series creates, all under tests/tcg/multiarch as of
this revision: test-xpage-chain.c and gdbstub/xpage-bp.py (patch 7) and
test-indirect-irq.c (patch 8). Each fails or hangs if the mechanism it
covers is removed, which is what makes them tests of the new behavior rather
than of the old.
Changes since v3
================
The biggest change is that v3's patch 4 is split three ways, as Richard
asked: the tcg_gen_lookup_and_goto_ptr() API change is now patch 4, the
tb_jmp_cache_probe base pointer and its poison are patch 5, and the inline
probe itself is patch 6. The other structural change is that both guest
tests move from tests/tcg/alpha to tests/tcg/multiarch, as Alex asked, so
every *-user target runs them. tests/tcg/alpha is now byte-identical to
master again.
1 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 (Richard). Queue the
per-CPU update with async_run_on_cpu() rather than
async_safe_run_on_cpu(): halting the other vCPUs buys nothing, since
the queued work already runs on the owning CPU's own thread (Alex).
Alex also asked whether there are cross-vCPU updates of tcg_cflags at
all; with this change the monitor path has none, and the only
remaining writer from another thread is cpu_single_step(), which is
neither new nor made worse here. Stub moved to accel/stubs/, where the
other accelerator stubs live (Philippe).
2 Commit message only: a linux-user process is not a single vCPU. The
cache is per CPUState and linux-user creates one per guest thread, so
a threaded guest pays the 1 MiB per thread, exactly as system
emulation pays it per vCPU (Richard).
3 Unchanged.
4 New, split out of v3's patch 4. No functional change from v3. Its
commit message no longer claims most targets can simply pass a PC:
five do, six cannot because their TB pc is derived (avr doubles it,
i386's is pre-segmentation, riscv masks it, hppa derives it from the
IAQ, hexagon adjusts it in hardware loops, sparc puts npc in cs_base),
and seven look like they could but are untested.
5 Also new, split out of v3's patch 4. The poison cache is a static
object rather than one allocated on first use (Richard, who asked for
const; the commit message says why it is plain static and lands in
.bss).
6 What remains of v3's patch 4. Emit the softmmu form of
tb_jmp_cache_hash_func() under CONFIG_SOFTMMU rather than the
user-only form everywhere: v3 was wrong for system mode, and only not
a correctness bug because a wrong index simply misses (Richard).
Compare the pc before testing tb for NULL, assert that
TranslationBlock::flags is 8-byte aligned since folding the two guards
into one 64-bit load relies on it, zero-extend a 32-bit guest PC
instead of falling back to the helper, and describe cs_base in the
probe as a second word of target-specific flags rather than by name
(all Richard).
7 Test moved to tests/tcg/multiarch (Alex). The hand-written branch went
with it: falling off the end of a page is a cross-page goto_tb just
the same, and needs no per-architecture branch encoding or
displacement arithmetic, only "set the return value" and "return".
Built and run under qemu-user on aarch64, alpha, arm, hppa,
loongarch64, m68k, mips, ppc, ppc64le, riscv64, s390x, sh4, sparc64
and x86_64; ppc64 ELFv1 skips, because a function pointer there is a
descriptor rather than a code address.
8 Test likewise moved to tests/tcg/multiarch (Alex). Nothing in it is
architecture specific: the loop is a computed goto, which every
target's compiler supports, so it covers whichever targets go on to
use the inline probe.
9 Hoist the compilation mode tests -- tcg_use_softmmu and the 64-bit
address type -- out of the backend hook into fold_ldst_disp(), so the
loop is not entered at all when the mode rules the fold out. Pass
MemOp rather than MemOpIdx to the hook; nothing about the mmu_idx is
relevant to it. Move the alignment test into generic code as
ldst_disp_needs_align(), so a backend need not repeat the
atom_and_align_for_opc() call; the exact answer depends on the host's
atomicity capabilities, which the generic pass does not know, so it
answers for the most restrictive host. That is the same answer for
everything the frontends actually emit, and conservative for the
handful of MO_ATOM_WITHIN16 and MO_ATOM_SUBALIGN accesses. (All
Richard.) What is left of the x86_64 hook is the guest_base test, so
it now lives beside x86_guest_base under the CONFIG_USER_ONLY that
declares it. Also refuse a displacement that does not fit the int32_t
out_disp() takes, which is unreachable with any real guest_base but
which the interface could not have carried.
What I would most like reviewed
===============================
- Patch 7 reverses a deliberate decision made in d3a2a1d803 on the
strength of an argument about the user-only invalidation paths, plus a
gate on whether gdb can ever attach.
- Patch 8's un-poison in the main loop races a concurrent poison from
another thread. I believe the existing barrier around
icount_decr.u16.high covers it, but my testing was single-threaded user
mode.
- Patch 6 treats cpu flags, cflags and cs_base as translation-time
constants in its guards, reads a jump cache entry without qatomic_read(),
and leaves one_insn_per_tb and -d nochain toggles visible only at the
next non-inline exit.
- Patch 9 only examines the immediately preceding op, refuses any access
with a slow path (so user-only, and no alignment check), and leaves the
i128 pairs alone. Richard asked whether the alignment test could stay on
the base register when the displacement is itself aligned; it can, and
the reason the fold is still refused there is the slow path handing
addr_reg to the helper. Recording the displacement in TCGLabelQemuLdst
and emitting one lea on the slow path would cover alignment-checked
accesses too, at no fast path cost. Not attempted here.
- Patch 2's 1 MiB per CPUState is easy to justify for a single-threaded
linux-user process and less obvious for system emulation with many
vCPUs, or for a heavily threaded guest. It may want to be sized per
target or made tunable rather than raised unconditionally.
Patches 6 and 9 are wired up for alpha and x86_64 respectively; everything
else is target-independent, and no other backend changes behavior or needs
touching.
v3: https://lore.kernel.org/qemu-devel/20260822190818.1829249-1-mattst88@gmail.com/
v2: https://lore.kernel.org/qemu-devel/20260817190038.580257-1-mattst88@gmail.com/
Matt Turner (9):
accel/tcg: fold the dynamic cflags into CPUState::tcg_cflags
accel/tcg: enlarge the TB jump cache to 64K entries
accel/tcg: skip the can_do_io stores in user-only builds
tcg: pass the destination to tcg_gen_lookup_and_goto_ptr()
accel/tcg: give the TB jump cache a second base pointer for generated
code
RFC: tcg: probe the TB jump cache inline instead of calling a helper
RFC: accel/tcg: allow cross-page goto_tb chaining in user-only builds
RFC: accel/tcg: poison the jump cache instead of polling for indirect
exits
RFC: tcg: fold a guest displacement into the host addressing mode
accel/stubs/meson.build | 1 +
accel/stubs/tcg-stub.c | 20 ++
accel/tcg/cpu-exec-common.c | 33 +-
accel/tcg/cpu-exec.c | 116 ++++++
accel/tcg/internal-common.h | 13 +-
accel/tcg/tb-jmp-cache.h | 2 +-
accel/tcg/tcg-accel-ops.c | 1 +
accel/tcg/translator.c | 94 ++++-
cpu-common.c | 11 +
cpu-target.c | 3 +
gdbstub/user.c | 14 +
include/gdbstub/user.h | 11 +
include/hw/core/cpu.h | 11 +
include/system/tcg.h | 21 ++
include/tcg/tcg-op-common.h | 15 +-
include/tcg/tcg-op.h | 12 +
include/tcg/tcg-opc.h | 9 +-
include/tcg/tcg.h | 2 +
monitor/hmp-cmds.c | 5 +
system/runstate-hmp-cmds.c | 4 +
target/alpha/translate.c | 4 +-
target/arm/tcg/translate-a64.c | 4 +-
target/arm/tcg/translate.c | 10 +-
target/avr/translate.c | 4 +-
target/hexagon/translate.c | 4 +-
target/hppa/translate.c | 6 +-
target/i386/tcg/translate.c | 2 +-
.../tcg/insn_trans/trans_branch.c.inc | 2 +-
target/loongarch/tcg/translate.c | 4 +-
target/m68k/translate.c | 2 +-
target/microblaze/translate.c | 4 +-
target/mips/tcg/nanomips_translate.c.inc | 2 +-
target/mips/tcg/translate.c | 6 +-
target/or1k/translate.c | 4 +-
target/ppc/translate.c | 4 +-
target/riscv/tcg/insn_trans/trans_rvzce.c.inc | 4 +-
target/riscv/tcg/translate.c | 2 +-
target/rx/translate.c | 4 +-
target/s390x/tcg/translate.c | 5 +-
target/sh4/translate.c | 4 +-
target/sparc/translate.c | 4 +-
target/tricore/translate.c | 4 +-
tcg/tcg-op-ldst.c | 3 +-
tcg/tcg-op.c | 142 +++++++-
tcg/tcg.c | 132 ++++++-
tcg/x86_64/tcg-target.c.inc | 41 +++
tcg/x86_64/tcg-target.h | 3 +
tests/tcg/multiarch/Makefile.target | 12 +-
tests/tcg/multiarch/gdbstub/xpage-bp.py | 37 ++
tests/tcg/multiarch/test-indirect-irq.c | 62 ++++
tests/tcg/multiarch/test-xpage-chain.c | 336 ++++++++++++++++++
51 files changed, 1192 insertions(+), 63 deletions(-)
create mode 100644 accel/stubs/tcg-stub.c
create mode 100644 tests/tcg/multiarch/gdbstub/xpage-bp.py
create mode 100644 tests/tcg/multiarch/test-indirect-irq.c
create mode 100644 tests/tcg/multiarch/test-xpage-chain.c
base-commit: eea8fe61b8be8f3016e522e6af24924a0266ca95
--
2.54.0
For guests running large amounts of code, most of what TCG executes is not
translated guest work but the fixed overhead around it. Blocks are short and
there are a great many of them, so the constant cost at each end of a block
(the interrupt poll and the can_do_io stores on entry, the dispatch on exit)
ends up dominating everything else.
The workload throughout is 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 --static --enable-lto --target-list=alpha-linux-user build. It
executes 34.2 billion TBs at 6.04 guest instructions each, and 24.6% of its
TB exits cannot use goto_tb. That is a representative shape for any guest
whose text is much larger than a page: indirect calls and returns
everywhere, plus direct branches that merely crossed a page boundary.
The first three patches are ordinary cleanups that stand on their own; patch
1 picked up a review tag in v4 and patch 3 in v2. Patches 4 and 5 are
preparation and move nothing on their own. The remaining four are marked RFC
individually and are where the interesting questions are.
1 accel/tcg: fold the dynamic cflags into CPUState::tcg_cflags
curr_cflags() recomputed three unlikely tests on every one of the run's
8.4 billion dispatches, from state that changes only when gdb enables
single-step, when one-insn-per-tb is toggled, or when the log mask
moves. Fold each into tcg_cflags where it changes. -5.15%
2 accel/tcg: enlarge the TB jump cache to 64K entries
4096 entries is too small for a guest running a large program;
tb_htable_lookup() is 6.10% of samples. 16 bits is the knee of the
sizing curve, at 1 MiB per CPUState. -5.92%, -8.71% wall
3 accel/tcg: skip the can_do_io stores in user-only builds
Two stores per TB that nothing in a user-only build reads: 68 billion
of them over the run. -4.57%, -4.53% wall
4 tcg: add tcg_gen_goto_jc_{i32,i64,tl}()
Preparation. A second dispatch interface alongside
tcg_gen_lookup_and_goto_ptr(), which is unchanged. The caller passes
the destination PC and thereby states that the CPU state is already
the destination's, which is what an inline lookup needs and what the
existing interface cannot promise. --enable-debug-tcg checks that
claim at runtime. Five targets are migrated; every other target and
every unmigrated call site is untouched.
5 accel/tcg: add CF_NO_GOTO_JC, set while a breakpoint is present
Preparation. The one thing an inline jump cache probe cannot check is
breakpoints, and it does not have to: the probe compares cflags, so a
cflag set while cpu->breakpoints is non-empty keeps such blocks both
from dispatching inline and from being reached by a block that does.
Nothing reads it yet.
6 RFC: tcg: probe the TB jump cache inline instead of calling a helper
95.8% of those 8.4 billion helper_lookup_tb_ptr() calls hit the jump
cache. Emit the probe inline (hash, four guarded loads, goto_ptr) and
call the helper only on a miss. -34.67%, -25.94% wall
7 RFC: accel/tcg: allow cross-page goto_tb chaining in user-only builds
translator_use_goto_tb() refuses to chain across a page. In user-only
builds the invalidation path already covers what that was protecting
against: every mmap/mprotect/munmap reaches page_set_flags(), which
invalidates and unlinks. Lift it there for runs that can never acquire
a breakpoint, keep it for system mode. -2.75%, -4.84% wall
8 RFC: accel/tcg: poison the jump cache instead of polling for indirect
exits
A block only needs the icount_decr poll if it can leave by goto_tb;
every other exit already passes through a dispatch. Give generated
code its own jump cache base pointer and point it at a read-only page
of zeroes when an exit is requested, so every dispatch misses into the
helper, which returns the epilogue. Emit the poll only in blocks that
emitted a goto_tb. -2.52%, -1.96% wall
9 RFC: tcg: fold a guest displacement into the host addressing mode
tcg_gen_qemu_ld/st cannot express a based access, so a target with a
displacement in its encodings materializes the address with an lea
that the host addressing mode would have done for free. Fold a
preceding constant add into a new argument on the op, opt-in per
backend, wired up for x86_64 user-only. -5.70%, -3.20% wall
Each percentage is against the patch before it. Every stage was measured in
one session on the same host, so end to end, from an unmodified LTO build of
the same base to the full series:
instructions retired: 1,646,994,254,249 -> 819,262,147,022 -50.26%
wall clock: 133.19s -> 77.30s -41.96%
Those are still the v3 measurements, unchanged and not re-run; the machine
they were taken on is busy. Nothing in v4 or v5 is expected to move them --
both revisions are reorganization, and the emulated compiler's output is
still byte-identical -- but they are not a measurement of this posting and
should not be read as one.
The two figures do not track each other, and that is the interesting part:
what the series removes is cheap, well-predicted, highly pipelined work, so
it retires far more instructions than it saves time. Patch 6 also cuts L1
icache load misses by 39.0%, because a dispatch no longer jumps into qemu's
.text and evicts translated code; qemu's own .text falls from 38.8% to 5.3%
of profile samples.
Every step builds and runs on its own, so the series bisects, and the
emulated compiler produces byte-identical assembly output at every step,
which is the correctness check these patches most need. Three tests under
tests/tcg/multiarch cover the hazards the series creates: test-xpage-chain.c
and gdbstub/xpage-bp.py (patch 7) and test-indirect-irq.c (patch 8). Each
fails or hangs if the mechanism it covers is removed, which is what makes
them tests of the new behavior rather than of the old.
Changes since v4
================
The structural change is that v4's patches 4 and 5 are gone, replaced by
different patches with the same job.
v4's patch 4 added an argument to tcg_gen_lookup_and_goto_ptr() and made
every caller pass NULL. Richard's example is target/arm, where
DISAS_UPDATE_NOCHAIN needs the helper because the state has changed while
DISAS_JUMP does not; that subtlety, he pointed out, means the existing
interface should not be adjusted at all, and that targets should migrate to
a new one instead. So v5 leaves tcg_gen_lookup_and_goto_ptr(void) as it was
and adds tcg_gen_goto_jc_{i32,i64,tl}() beside it. Only the five migrated
targets are touched, rather than all twenty; the diffstat is a good summary
of the difference.
v4's patch 5 gave generated code a second jump cache base pointer and
poisoned it when a breakpoint was inserted, which needed a cross-thread
poison, an un-poison, and a double check of the breakpoint list. Richard
suggested a cflag instead, which is both simpler and sufficient: the probe
already compares cflags, so CF_NO_GOTO_JC alone keeps both the block itself
and anything chaining to it off the inline path. The whole poison mechanism
leaves this patch. The base pointer moves down to patch 8, where a pending
exit is the only reason left to want one, that being a per-execution
condition no cflag can express.
1 Reviewed-by: Richard Henderson.
2 Unchanged.
3 Unchanged.
4 Replaces "tcg: pass the destination to tcg_gen_lookup_and_goto_ptr()".
New interface rather than a changed one; _i32 and _i64 entry points
with a _tl alias in tcg-op.h rather than one entry point taking a
TCGTemp, since single-binary targets build once and stop relying on
TARGET_LONG_BITS; and a new helper_goto_jc_check() that asserts pc,
flags and cs_base against get_tb_cpu_state() under CONFIG_DEBUG_TCG.
(All Richard.) The contract is now written on the declaration rather
than left to be inferred.
5 Replaces "accel/tcg: give the TB jump cache a second base pointer for
generated code" (Richard, as above). That leaves a residual window,
in which blocks translated before the insert keep chaining on their
old cflags until the vCPU reaches its main loop. It is the same window
goto_tb chaining already has, and in system mode gdb inserts
breakpoints with the vCPUs stopped, so there is none. The commit
message says so rather than leaving it implicit.
6 Build the folded flags/cflags constant with deposit64() rather than
under #if HOST_BIG_ENDIAN, so both arms compile on every host
(Richard). Read cpu->tb_jmp_cache directly and honor CF_NO_GOTO_JC,
following patch 5. Commit message notes the backend expansion this
wants as a follow-up, which is Richard's list: x86_64 and s390x can
compare against a memory operand, and aarch64 has shift-add for the
entry address, ldp to load (tb, pc) and (cs_base, flags), and ccmp to
halve the branches. Not attempted here: the probe as posted is correct
on every backend, and the expansions are strictly additive and deserve
their own numbers, particularly the aarch64 one, which changes the
shape enough that it should be measured on aarch64 hardware.
7 Unchanged.
8 Gains CPUState::tb_jmp_cache_probe from v4's patch 5. With breakpoints
handled by a cflag, a pending exit is the only reason left to poison,
so there is one condition rather than two, no cross-thread poison from
cpu_breakpoint_insert(), and no unrealized or NULL state for either
helper to consider: the probe is initialized alongside tb_jmp_cache
and unrealize leaves it pointing at the poison (Richard). The poison
is now a page-aligned allocation mapped read-only at startup rather
than a writable .bss object (Richard); qemu_mprotect_ro() is added for
it beside the existing _rw, _rwx and _none forms. That also settles
v4's own note about a 1 MiB object that is never written.
9 Unchanged.
Testing
=======
alpha, loongarch64, mips, mipsel, mips64, ppc, ppc64 and s390x all build and
run an indirect-dispatch exerciser (computed-goto back edges, function
pointer calls, returns and a longjmp out of a SIGALRM handler) to
completion under --enable-debug-tcg, so helper_goto_jc_check()'s assertions
have actually been exercised on every migrated target rather than only on
alpha. The gdbstub path (insert, hit, backtrace, delete, continue) and
test-indirect-irq still pass, and the emulated compiler's output is still
byte-identical.
What I would most like reviewed
===============================
- Patch 7 reverses a deliberate decision made in d3a2a1d803 on the
strength of an argument about the user-only invalidation paths, plus a
gate on whether gdb can ever attach.
- Patch 8's un-poison in the main loop races a concurrent poison from
another thread. I believe the existing barrier around
icount_decr.u16.high covers it, but my testing was single-threaded user
mode.
- Patch 5's argument that a cflag is sufficient rests on the probe
comparing cflags and on the residual window being one goto_tb chaining
already accepts. Both seem clearly true to me, which is why they are
worth a second reader.
- Patch 6 treats cpu flags, cflags and cs_base as translation-time
constants in its guards, reads a jump cache entry without qatomic_read(),
and leaves one_insn_per_tb and -d nochain toggles visible only at the
next non-inline exit.
- Patch 9 only examines the immediately preceding op, refuses any access
with a slow path (so user-only, and no alignment check), and leaves the
i128 pairs alone. Richard asked whether the alignment test could stay on
the base register when the displacement is itself aligned; it can, and
the reason the fold is still refused there is the slow path handing
addr_reg to the helper. Recording the displacement in TCGLabelQemuLdst
and emitting one lea on the slow path would cover alignment-checked
accesses too, at no fast path cost. Not attempted here.
- Patch 2's 1 MiB per CPUState is easy to justify for a single-threaded
linux-user process and less obvious for system emulation with many
vCPUs, or for a heavily threaded guest. It may want to be sized per
target or made tunable rather than raised unconditionally.
Patches 6 and 9 are wired up for alpha and x86_64 respectively; everything
else is target-independent, and no other backend changes behavior or needs
touching.
v4: https://lore.kernel.org/qemu-devel/20260827050241.3713332-1-mattst88@gmail.com/
v3: https://lore.kernel.org/qemu-devel/20260822190818.1829249-1-mattst88@gmail.com/
v2: https://lore.kernel.org/qemu-devel/20260817190038.580257-1-mattst88@gmail.com/
Matt Turner (9):
accel/tcg: fold the dynamic cflags into CPUState::tcg_cflags
accel/tcg: enlarge the TB jump cache to 64K entries
accel/tcg: skip the can_do_io stores in user-only builds
tcg: add tcg_gen_goto_jc_{i32,i64,tl}()
accel/tcg: add CF_NO_GOTO_JC, set while a breakpoint is present
RFC: tcg: probe the TB jump cache inline instead of calling a helper
RFC: accel/tcg: allow cross-page goto_tb chaining in user-only builds
RFC: accel/tcg: poison the jump cache instead of polling for indirect
exits
RFC: tcg: fold a guest displacement into the host addressing mode
accel/stubs/meson.build | 1 +
accel/stubs/tcg-stub.c | 16 +
accel/tcg/cpu-exec-common.c | 44 ++-
accel/tcg/cpu-exec.c | 121 +++++++
accel/tcg/internal-common.h | 20 +-
accel/tcg/tb-jmp-cache.h | 2 +-
accel/tcg/tcg-accel-ops.c | 2 +
accel/tcg/tcg-runtime.h | 4 +
accel/tcg/translator.c | 94 ++++-
cpu-common.c | 7 +
cpu-target.c | 3 +
gdbstub/user.c | 14 +
include/exec/translation-block.h | 1 +
include/gdbstub/user.h | 11 +
include/hw/core/cpu.h | 9 +
include/qemu/mprotect.h | 1 +
include/system/tcg.h | 12 +
include/tcg/tcg-op-common.h | 21 ++
include/tcg/tcg-op.h | 2 +
include/tcg/tcg-opc.h | 9 +-
include/tcg/tcg.h | 2 +
monitor/hmp-cmds.c | 5 +
system/runstate-hmp-cmds.c | 4 +
target/alpha/translate.c | 4 +-
.../tcg/insn_trans/trans_branch.c.inc | 2 +-
target/loongarch/tcg/translate.c | 4 +-
target/mips/tcg/nanomips_translate.c.inc | 2 +-
target/mips/tcg/translate.c | 6 +-
target/ppc/translate.c | 4 +-
target/s390x/tcg/translate.c | 4 +-
tcg/tcg-op-ldst.c | 3 +-
tcg/tcg-op.c | 182 +++++++++-
tcg/tcg.c | 132 ++++++-
tcg/x86_64/tcg-target.c.inc | 41 +++
tcg/x86_64/tcg-target.h | 3 +
tests/tcg/multiarch/Makefile.target | 12 +-
tests/tcg/multiarch/gdbstub/xpage-bp.py | 37 ++
tests/tcg/multiarch/test-indirect-irq.c | 62 ++++
tests/tcg/multiarch/test-xpage-chain.c | 336 ++++++++++++++++++
util/osdep.c | 9 +
40 files changed, 1216 insertions(+), 32 deletions(-)
create mode 100644 accel/stubs/tcg-stub.c
create mode 100644 tests/tcg/multiarch/gdbstub/xpage-bp.py
create mode 100644 tests/tcg/multiarch/test-indirect-irq.c
create mode 100644 tests/tcg/multiarch/test-xpage-chain.c
base-commit: eea8fe61b8be8f3016e522e6af24924a0266ca95
--
2.54.0
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.0The per-CPU TB jump cache has held 4096 entries since it was introduced.
That is too small for guests running large programs: an emulated compiler
misses often enough that the fallback qht lookup shows up prominently in
a profile.
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. The
compile performs 34.2 billion TB executions, of which 8.4 billion take
the indirect dispatch path.
Sizing curve, on top of the preceding patch, instructions retired and
wall clock:
12 bits ( 64 KiB): 1,562,204,796,597 132.58s
14 bits ( 256 KiB): 1,493,318,515,396 -4.41% 124.67s -5.97%
16 bits ( 1 MiB): 1,469,772,951,575 -5.92% 121.04s -8.71%
18 bits ( 4 MiB): 1,462,309,832,762 -6.39% 119.82s -9.62%
16 bits is the knee. 18 buys another 0.47% of instructions for four times
the memory. It does show a further 1.01% of wall clock, which is outside
the 0.70% run-to-run spread at 16 bits, so the effect is probably real --
but paying four times the memory for it is a poor trade, and instructions
retired does not account for the data cache pressure of a 4 MiB table.
In a perf profile the mechanism is visible directly: tb_htable_lookup(),
which is where qht_lookup_custom() lands once it is inlined in an LTO
build, falls from 6.10% of samples to 1.66%.
The cost is memory: the cache grows from 64 KiB to 1 MiB, once per
CPUState. In linux-user that is per guest thread rather than per process,
so a threaded guest pays it as many times as it has threads, exactly as
system emulation pays it per vCPU. The allocation is g_new0(), so the
pages are faulted in as the cache is touched and a thread that runs a
small amount of code touches a small part of it, but the address space is
committed either way.
So this may still want to be tunable, or scaled from the number of CPUs,
rather than raised unconditionally. I do not have a threaded workload where
the smaller cache is the better trade, and would welcome one.
v4: Fix the claim that a linux-user process is a single vCPU. The cache is
per CPUState, and linux-user creates one per guest thread. Pointed out
by Richard Henderson.
Signed-off-by: Matt Turner <mattst88@gmail.com>
---
accel/tcg/tb-jmp-cache.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git ./accel/tcg/tb-jmp-cache.h ./accel/tcg/tb-jmp-cache.h
index c3a505e394..268dacd7ba 100644
--- ./accel/tcg/tb-jmp-cache.h
+++ ./accel/tcg/tb-jmp-cache.h
@@ -12,7 +12,7 @@
#include "qemu/rcu.h"
#include "exec/cpu-common.h"
-#define TB_JMP_CACHE_BITS 12
+#define TB_JMP_CACHE_BITS 16
#define TB_JMP_CACHE_SIZE (1 << TB_JMP_CACHE_BITS)
/*
--
2.54.0Every translation block stores to cpu->neg.can_do_io twice: false before
the first instruction, true before the last one. Nothing reads it in a
user-only build. There is no memory-mapped I/O in linux-user, and every
reader is in system_ss: cputlb.c, watchpoint.c, icount-common.c and
tcg-accel-ops-icount.c.
Two stores per TB is not much on its own, but TBs are short. An emulated
alpha gcc 16.2.0 compiling the SQLite 3.45.1 amalgamation (255k lines,
-O2) executes 34.2 billion TBs at 6.04 guest instructions each, so this is
68 billion stores for nothing.
Measured on an x86-64 host, LTO build, on top of the preceding two
patches:
before: 1,469,772,951,575 instructions
after: 1,402,667,803,616 instructions -4.57%
before: 121.04s wall clock
after: 115.56s wall clock -4.53%
The emulated compiler produces byte-identical output.
v3: Use #ifndef CONFIG_USER_ONLY again rather than
if (IS_ENABLED(CONFIG_USER_ONLY)). QEMU's IS_ENABLED() is IS_EMPTY(),
which is only true for a symbol Meson defines empty; CONFIG_USER_ONLY
is defined as 1, so the test was always false and v2 emitted the two
stores after all. The measurements above are from the working form.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
Signed-off-by: Matt Turner <mattst88@gmail.com>
---
accel/tcg/translator.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git ./accel/tcg/translator.c ./accel/tcg/translator.c
index 57daded60f..6c8fcd7a20 100644
--- ./accel/tcg/translator.c
+++ ./accel/tcg/translator.c
@@ -21,12 +21,14 @@
#include "disas/disas.h"
#include "tb-internal.h"
+#ifndef CONFIG_USER_ONLY
static void set_can_do_io(DisasContextBase *db, bool val)
{
QEMU_BUILD_BUG_ON(sizeof_field(CPUState, neg.can_do_io) != 1);
tcg_gen_st8_i32(tcg_constant_i32(val), tcg_env,
offsetof(CPUState, neg.can_do_io) - sizeof(CPUState));
}
+#endif
bool translator_io_start(DisasContextBase *db)
{
@@ -210,17 +212,25 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns,
/*
* Manage can_do_io for the translation block: set to false before
* the first insn and set to true before the last insn.
+ *
+ * Nothing reads can_do_io in user-only builds. There is no MMIO
+ * there, and every reader (cputlb.c, watchpoint.c, icount) is in
+ * system_ss, so skip the two stores per TB entirely.
*/
if (db->num_insns == 1) {
tcg_debug_assert(first_insn_start == db->insn_start);
} else {
tcg_debug_assert(first_insn_start != db->insn_start);
+#ifndef CONFIG_USER_ONLY
tcg_ctx->emit_before_op = first_insn_start;
set_can_do_io(db, false);
+#endif
}
+#ifndef CONFIG_USER_ONLY
tcg_ctx->emit_before_op = db->insn_start;
set_can_do_io(db, true);
tcg_ctx->emit_before_op = NULL;
+#endif
/* May be used by disas_log or plugin callbacks. */
tb->size = db->pc_next - db->pc_first;
--
2.54.0
On 8/31/26 17:48, Matt Turner wrote: > Every translation block stores to cpu->neg.can_do_io twice: false before > the first instruction, true before the last one. Nothing reads it in a > user-only build. There is no memory-mapped I/O in linux-user, and every > reader is in system_ss: cputlb.c, watchpoint.c, icount-common.c and > tcg-accel-ops-icount.c. > > Two stores per TB is not much on its own, but TBs are short. An emulated > alpha gcc 16.2.0 compiling the SQLite 3.45.1 amalgamation (255k lines, > -O2) executes 34.2 billion TBs at 6.04 guest instructions each, so this is > 68 billion stores for nothing. > > Measured on an x86-64 host, LTO build, on top of the preceding two > patches: > > before: 1,469,772,951,575 instructions > after: 1,402,667,803,616 instructions -4.57% > > before: 121.04s wall clock > after: 115.56s wall clock -4.53% > > The emulated compiler produces byte-identical output. > > v3: Use #ifndef CONFIG_USER_ONLY again rather than > if (IS_ENABLED(CONFIG_USER_ONLY)). QEMU's IS_ENABLED() is IS_EMPTY(), > which is only true for a symbol Meson defines empty; CONFIG_USER_ONLY > is defined as 1, so the test was always false and v2 emitted the two > stores after all. The measurements above are from the working form. > > Reviewed-by: Richard Henderson<richard.henderson@linaro.org> > Reviewed-by: Philippe Mathieu-Daudé<philmd@oss.qualcomm.com> > Signed-off-by: Matt Turner<mattst88@gmail.com> > --- > accel/tcg/translator.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) Cherry-picked this to tcg-next. r~
tcg_gen_lookup_and_goto_ptr() takes no arguments and emits a call to
helper_lookup_tb_ptr(), which recovers the destination PC from env by
calling back into the target through TCGCPUOps::get_tb_cpu_state(). At
translation time the caller often already has the destination PC in a temp,
and knows the flags, cflags and cs_base any destination it may reach has to
match, because they are the ones the block being generated was translated
with. A later patch uses that to look the destination up inline.
But it is not something every caller of tcg_gen_lookup_and_goto_ptr() can
promise, and the promise is subtle. target/arm has
case DISAS_UPDATE_NOCHAIN:
gen_update_pc(dc, curr_insn_len(dc));
/* fall through */
case DISAS_JUMP:
gen_goto_ptr();
break;
where DISAS_JUMP could make the promise and DISAS_UPDATE_NOCHAIN could not,
because it is there precisely because the state changed. Two call sites,
one line apart, on opposite sides of the contract.
So add a second entry point rather than growing an argument on the first.
tcg_gen_lookup_and_goto_ptr() keeps today's meaning and today's signature:
dispatch, and let the helper work out where. tcg_gen_goto_jc_*() means
dispatch to the destination that env already describes, and takes the pc as
proof that the caller knows which one that is. Targets migrate one call
site at a time, and a call site that cannot promise simply does not move.
The contract is:
- @pc holds exactly what get_tb_cpu_state() reports as the destination pc.
- The flags and cs_base it reports are the ones this block was translated
with, which is what lets them be constants in the generated code.
--enable-debug-tcg checks all three against get_tb_cpu_state() at run time,
via a new helper_goto_jc_check(). That turns a mistake into an assertion at
the offending call site instead of a block that runs with someone else's
flags.
Five targets have a call site whose pc temp is that key by construction, and
are migrated here: alpha, loongarch, mips, ppc and s390x. Nothing else
changes; the generated code does not change either, since goto_jc still
emits the same helper call for now.
For six targets the TB pc is derived and passing the pc temp would be wrong:
avr's TB pc is the word address doubled, i386's is eip before segmentation,
riscv masks it to 32 bits when xl is MXL_RV32, hppa derives it from the IAQ,
hexagon adjusts it inside a hardware loop, and sparc puts npc in cs_base.
The remaining seven -- arm, m68k, microblaze, or1k, rx, sh4 and tricore --
have call sites that look like they could move, but I have not convinced
myself of the contract for them and have nothing to test them with. Each is
a one-line change for whoever wants it, and debug-tcg will say if it is
wrong.
The i32 and i64 forms are separate functions, with a _tl alias in tcg-op.h,
as for most everything else. A translator built for more than one value of
TARGET_LONG_BITS cannot include tcg-op.h and calls the sized form directly,
which is what s390x does here.
Neither form takes the TranslationBlock: tcg_ctx->gen_tb is the block being
generated, the same one tcg_gen_goto_tb() and tcg_gen_lookup_and_goto_ptr()
already read, so there is no way for a caller to pass the wrong one.
v4: Split out of "tcg: probe the TB jump cache inline instead of calling a
helper", which did the API change and the inline probe in one patch.
Requested by Richard Henderson.
v5: Add a new interface rather than growing an argument on
tcg_gen_lookup_and_goto_ptr(), and check the contract under
--enable-debug-tcg. Requested by Richard Henderson, who named it
tcg_gen_goto_jc_*(); the DISAS_UPDATE_NOCHAIN example above is his.
v5: Define _i32 and _i64 entry points with a _tl alias in tcg-op.h, rather
than one entry point taking a TCGTemp. Requested by Richard Henderson:
as targets migrate to single-binary, code is built once and stops
relying on TARGET_LONG_BITS, so the TCGTemp split was the wrong shape.
Signed-off-by: Matt Turner <mattst88@gmail.com>
---
accel/tcg/cpu-exec.c | 27 +++++++++
accel/tcg/tcg-runtime.h | 4 ++
include/tcg/tcg-op-common.h | 20 +++++++
include/tcg/tcg-op.h | 2 +
target/alpha/translate.c | 4 +-
.../tcg/insn_trans/trans_branch.c.inc | 2 +-
target/loongarch/tcg/translate.c | 4 +-
target/mips/tcg/nanomips_translate.c.inc | 2 +-
target/mips/tcg/translate.c | 6 +-
target/ppc/translate.c | 4 +-
target/s390x/tcg/translate.c | 4 +-
tcg/tcg-op.c | 59 +++++++++++++++++--
12 files changed, 119 insertions(+), 19 deletions(-)
diff --git ./accel/tcg/cpu-exec.c ./accel/tcg/cpu-exec.c
index 148e0f583e..ca90a77a7b 100644
--- ./accel/tcg/cpu-exec.c
+++ ./accel/tcg/cpu-exec.c
@@ -407,6 +407,33 @@ const void *HELPER(lookup_tb_ptr)(CPUArchState *env)
return tb->tc.ptr;
}
+#ifdef CONFIG_DEBUG_TCG
+/**
+ * helper_goto_jc_check: check the contract of tcg_gen_goto_jc_*()
+ * @env: current cpu state
+ * @pc: the destination pc the caller passed at translation time
+ * @flags: the flags the dispatching block was translated with
+ * @cs_base: the cs_base the dispatching block was translated with
+ *
+ * A goto_jc looks the destination up on the caller's @pc with the flags and
+ * cs_base of the block doing the dispatching, so all three have to be what
+ * get_tb_cpu_state() reports by the time the dispatch runs. That is a
+ * property of the translator, not of the generated code, so check it here
+ * rather than leaving a target that gets it wrong to be debugged as a block
+ * running with someone else's flags.
+ */
+void HELPER(goto_jc_check)(CPUArchState *env, uint64_t pc, uint64_t flags,
+ uint64_t cs_base)
+{
+ CPUState *cpu = env_cpu(env);
+ TCGTBCPUState s = cpu->cc->tcg_ops->get_tb_cpu_state(cpu);
+
+ assert(s.pc == pc);
+ assert(s.flags == flags);
+ assert(s.cs_base == cs_base);
+}
+#endif
+
/* Return the current PC from CPU, which may be cached in TB. */
static vaddr log_pc(CPUState *cpu, const TranslationBlock *tb)
{
diff --git ./accel/tcg/tcg-runtime.h ./accel/tcg/tcg-runtime.h
index 0b832176b3..ec99170698 100644
--- ./accel/tcg/tcg-runtime.h
+++ ./accel/tcg/tcg-runtime.h
@@ -22,6 +22,10 @@ DEF_HELPER_FLAGS_1(ctpop_i64, TCG_CALL_NO_RWG_SE, i64, i64)
DEF_HELPER_FLAGS_1(lookup_tb_ptr, TCG_CALL_NO_WG_SE, cptr, env)
+#ifdef CONFIG_DEBUG_TCG
+DEF_HELPER_FLAGS_4(goto_jc_check, TCG_CALL_NO_WG_SE, void, env, i64, i64, i64)
+#endif
+
DEF_HELPER_FLAGS_1(exit_atomic, TCG_CALL_NO_WG, noreturn, env)
#ifndef IN_HELPER_PROTO
diff --git ./include/tcg/tcg-op-common.h ./include/tcg/tcg-op-common.h
index 9b321f959c..4f334faaaa 100644
--- ./include/tcg/tcg-op-common.h
+++ ./include/tcg/tcg-op-common.h
@@ -85,6 +85,26 @@ void tcg_gen_goto_tb(unsigned idx);
*/
void tcg_gen_lookup_and_goto_ptr(void);
+/**
+ * tcg_gen_goto_jc_i32() - dispatch to the destination TB via the jump cache
+ * tcg_gen_goto_jc_i64() - dispatch to the destination TB via the jump cache
+ * @pc: temp holding the destination guest PC
+ *
+ * As tcg_gen_lookup_and_goto_ptr(), but the caller states where the
+ * dispatch is going, which allows the lookup to be done inline.
+ *
+ * The contract is that when this runs, the CPU state must already be
+ * exactly the destination's: @pc must hold what get_tb_cpu_state() would
+ * report as the destination pc, and the flags and cs_base it would report
+ * must be the ones the block being generated was translated with. A
+ * translator that has not finished updating the state, or whose pc is
+ * derived rather than being the lookup key -- avr's word address, i386's
+ * eip before segmentation -- must use tcg_gen_lookup_and_goto_ptr()
+ * instead. --enable-debug-tcg checks the contract at runtime.
+ */
+void tcg_gen_goto_jc_i32(TCGv_i32 pc);
+void tcg_gen_goto_jc_i64(TCGv_i64 pc);
+
void tcg_gen_plugin_cb(unsigned from);
void tcg_gen_plugin_mem_cb(TCGv_i64 addr, unsigned meminfo);
diff --git ./include/tcg/tcg-op.h ./include/tcg/tcg-op.h
index 3721164236..cd4794d745 100644
--- ./include/tcg/tcg-op.h
+++ ./include/tcg/tcg-op.h
@@ -38,6 +38,7 @@ typedef TCGv_i32 TCGv;
#define tcgv_tl_temp tcgv_i32_temp
#define tcg_gen_qemu_ld_tl tcg_gen_qemu_ld_i32
#define tcg_gen_qemu_st_tl tcg_gen_qemu_st_i32
+#define tcg_gen_goto_jc_tl tcg_gen_goto_jc_i32
#elif TARGET_LONG_BITS == 64
typedef TCGv_i64 TCGv;
#define tcg_temp_new() tcg_temp_new_i64()
@@ -45,6 +46,7 @@ typedef TCGv_i64 TCGv;
#define tcgv_tl_temp tcgv_i64_temp
#define tcg_gen_qemu_ld_tl tcg_gen_qemu_ld_i64
#define tcg_gen_qemu_st_tl tcg_gen_qemu_st_i64
+#define tcg_gen_goto_jc_tl tcg_gen_goto_jc_i64
#else
#error Unhandled TARGET_LONG_BITS value
#endif
diff --git ./target/alpha/translate.c ./target/alpha/translate.c
index c66e3f9c14..8318487cd8 100644
--- ./target/alpha/translate.c
+++ ./target/alpha/translate.c
@@ -449,7 +449,7 @@ static void gen_goto_tb(DisasContext *ctx, unsigned tb_slot_idx, int32_t disp)
tcg_gen_exit_tb(ctx->base.tb, tb_slot_idx);
} else {
gen_pc_disp(ctx, cpu_pc, disp);
- tcg_gen_lookup_and_goto_ptr();
+ tcg_gen_goto_jc_tl(cpu_pc);
}
}
@@ -2917,7 +2917,7 @@ static void alpha_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
gen_pc_disp(ctx, cpu_pc, 0);
/* FALLTHRU */
case DISAS_PC_UPDATED:
- tcg_gen_lookup_and_goto_ptr();
+ tcg_gen_goto_jc_tl(cpu_pc);
break;
case DISAS_PC_UPDATED_NOCHAIN:
tcg_gen_exit_tb(NULL, 0);
diff --git ./target/loongarch/tcg/insn_trans/trans_branch.c.inc ./target/loongarch/tcg/insn_trans/trans_branch.c.inc
index da07778658..d4318dfa43 100644
--- ./target/loongarch/tcg/insn_trans/trans_branch.c.inc
+++ ./target/loongarch/tcg/insn_trans/trans_branch.c.inc
@@ -27,7 +27,7 @@ static bool trans_jirl(DisasContext *ctx, arg_jirl *a)
tcg_gen_mov_tl(cpu_pc, addr);
tcg_gen_movi_tl(dest, make_address_pc(ctx, ctx->base.pc_next + 4));
gen_set_gpr(a->rd, dest, EXT_NONE);
- tcg_gen_lookup_and_goto_ptr();
+ tcg_gen_goto_jc_tl(cpu_pc);
ctx->base.is_jmp = DISAS_NORETURN;
return true;
}
diff --git ./target/loongarch/tcg/translate.c ./target/loongarch/tcg/translate.c
index 124dce6269..6ac0c3773a 100644
--- ./target/loongarch/tcg/translate.c
+++ ./target/loongarch/tcg/translate.c
@@ -111,7 +111,7 @@ static void gen_goto_tb(DisasContext *ctx, unsigned tb_slot_idx, vaddr dest)
tcg_gen_exit_tb(ctx->base.tb, tb_slot_idx);
} else {
tcg_gen_movi_tl(cpu_pc, dest);
- tcg_gen_lookup_and_goto_ptr();
+ tcg_gen_goto_jc_tl(cpu_pc);
}
}
@@ -311,7 +311,7 @@ static void loongarch_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
switch (ctx->base.is_jmp) {
case DISAS_STOP:
tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next);
- tcg_gen_lookup_and_goto_ptr();
+ tcg_gen_goto_jc_tl(cpu_pc);
break;
case DISAS_TOO_MANY:
gen_goto_tb(ctx, 0, ctx->base.pc_next);
diff --git ./target/mips/tcg/nanomips_translate.c.inc ./target/mips/tcg/nanomips_translate.c.inc
index 4b0b01ba37..106f49990d 100644
--- ./target/mips/tcg/nanomips_translate.c.inc
+++ ./target/mips/tcg/nanomips_translate.c.inc
@@ -2406,7 +2406,7 @@ static void gen_compute_nanomips_pbalrsc_branch(DisasContext *ctx, int rs,
/* unconditional branch to register */
tcg_gen_mov_tl(cpu_PC, btarget);
- tcg_gen_lookup_and_goto_ptr();
+ tcg_gen_goto_jc_tl(cpu_PC);
}
/* nanoMIPS Branches */
diff --git ./target/mips/tcg/translate.c ./target/mips/tcg/translate.c
index e3467d1525..dea1ba4c1e 100644
--- ./target/mips/tcg/translate.c
+++ ./target/mips/tcg/translate.c
@@ -4374,7 +4374,7 @@ static void gen_goto_tb(DisasContext *ctx, unsigned tb_slot_idx,
tcg_gen_exit_tb(ctx->base.tb, tb_slot_idx);
} else {
gen_save_pc(dest);
- tcg_gen_lookup_and_goto_ptr();
+ tcg_gen_goto_jc_tl(cpu_PC);
}
}
@@ -11014,7 +11014,7 @@ static void gen_branch(DisasContext *ctx, int insn_bytes)
} else {
tcg_gen_mov_tl(cpu_PC, btarget);
}
- tcg_gen_lookup_and_goto_ptr();
+ tcg_gen_goto_jc_tl(cpu_PC);
break;
default:
LOG_DISAS("unknown branch 0x%x\n", proc_hflags);
@@ -15244,7 +15244,7 @@ static void mips_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
switch (ctx->base.is_jmp) {
case DISAS_STOP:
gen_save_pc(ctx->base.pc_next);
- tcg_gen_lookup_and_goto_ptr();
+ tcg_gen_goto_jc_tl(cpu_PC);
break;
case DISAS_NEXT:
case DISAS_TOO_MANY:
diff --git ./target/ppc/translate.c ./target/ppc/translate.c
index 06ed2adf10..21e21102fc 100644
--- ./target/ppc/translate.c
+++ ./target/ppc/translate.c
@@ -3664,7 +3664,7 @@ static void gen_lookup_and_goto_ptr(DisasContext *ctx)
pmu_count_insns(ctx);
}
- tcg_gen_lookup_and_goto_ptr();
+ tcg_gen_goto_jc_tl(cpu_nip);
}
}
@@ -6690,7 +6690,7 @@ static void ppc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
pmu_count_insns(ctx);
}
- tcg_gen_lookup_and_goto_ptr();
+ tcg_gen_goto_jc_tl(cpu_nip);
break;
case DISAS_EXIT_UPDATE:
diff --git ./target/s390x/tcg/translate.c ./target/s390x/tcg/translate.c
index 1b6023168b..906951c7df 100644
--- ./target/s390x/tcg/translate.c
+++ ./target/s390x/tcg/translate.c
@@ -1162,7 +1162,7 @@ static DisasJumpType help_branch(DisasContext *s, DisasCompare *c,
tcg_gen_goto_tb(0);
tcg_gen_exit_tb(s->base.tb, 0);
} else {
- tcg_gen_lookup_and_goto_ptr();
+ tcg_gen_goto_jc_i64(psw_addr);
}
gen_set_label(lab);
@@ -6477,7 +6477,7 @@ static void s390x_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
if (dc->exit_to_mainloop) {
tcg_gen_exit_tb(NULL, 0);
} else {
- tcg_gen_lookup_and_goto_ptr();
+ tcg_gen_goto_jc_i64(psw_addr);
}
break;
default:
diff --git ./tcg/tcg-op.c ./tcg/tcg-op.c
index 28d3b2a847..a2f35359fe 100644
--- ./tcg/tcg-op.c
+++ ./tcg/tcg-op.c
@@ -2715,18 +2715,65 @@ void tcg_gen_goto_tb(unsigned idx)
tcg_gen_op1i(INDEX_op_goto_tb, 0, idx);
}
+static void gen_lookup_tb_ptr_and_goto(void)
+{
+ TCGv_ptr ptr = tcg_temp_ebb_new_ptr();
+
+ gen_helper_lookup_tb_ptr(ptr, tcg_env);
+ tcg_gen_op1i(INDEX_op_goto_ptr, TCG_TYPE_PTR, tcgv_ptr_arg(ptr));
+ tcg_temp_free_ptr(ptr);
+}
+
void tcg_gen_lookup_and_goto_ptr(void)
{
- TCGv_ptr ptr;
-
if (tcg_ctx->gen_tb->cflags & CF_NO_GOTO_PTR) {
tcg_gen_exit_tb(NULL, 0);
return;
}
plugin_gen_disable_mem_helpers();
- ptr = tcg_temp_ebb_new_ptr();
- gen_helper_lookup_tb_ptr(ptr, tcg_env);
- tcg_gen_op1i(INDEX_op_goto_ptr, TCG_TYPE_PTR, tcgv_ptr_arg(ptr));
- tcg_temp_free_ptr(ptr);
+ gen_lookup_tb_ptr_and_goto();
+}
+
+/*
+ * The common half of tcg_gen_goto_jc_i32() and tcg_gen_goto_jc_i64(). @pc
+ * is widened to i64 because the jump cache is keyed on a vaddr; for a
+ * 32-bit guest PC that is its zero extension.
+ */
+static void gen_goto_jc(TCGv_i64 pc)
+{
+ const TranslationBlock *tb = tcg_ctx->gen_tb;
+
+ if (tb->cflags & CF_NO_GOTO_PTR) {
+ tcg_gen_exit_tb(NULL, 0);
+ return;
+ }
+
+ plugin_gen_disable_mem_helpers();
+
+#ifdef CONFIG_DEBUG_TCG
+ /*
+ * The caller has asserted that env already describes the destination.
+ * Check it, rather than leaving a target that gets it wrong to be
+ * debugged as a block that runs with someone else's flags.
+ */
+ gen_helper_goto_jc_check(tcg_env, pc, tcg_constant_i64(tb->flags),
+ tcg_constant_i64(tb->cs_base));
+#endif
+
+ gen_lookup_tb_ptr_and_goto();
+}
+
+void tcg_gen_goto_jc_i64(TCGv_i64 pc)
+{
+ gen_goto_jc(pc);
+}
+
+void tcg_gen_goto_jc_i32(TCGv_i32 pc)
+{
+ TCGv_i64 pc64 = tcg_temp_ebb_new_i64();
+
+ tcg_gen_extu_i32_i64(pc64, pc);
+ gen_goto_jc(pc64);
+ tcg_temp_free_i64(pc64);
}
--
2.54.0The next patch dispatches a goto_jc by probing the TB jump cache from
generated code. That probe cannot check everything helper_lookup_tb_ptr()
checks, and the one that matters 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.
The probe does compare the destination's cflags against the cflags of the
block doing the dispatching, and only takes the destination when they are
equal. So a cflag is all that is needed. Add CF_NO_GOTO_JC, set it in
CPUState::tcg_cflags while cpu->breakpoints is non-empty, and blocks
translated from then on both decline to dispatch inline themselves -- the
next patch makes them emit the plain helper call -- and are unreachable from
blocks that do, because their cflags no longer match.
The two ends of the flag are cpu_breakpoint_insert() and
cpu_breakpoint_remove_by_ref(), which are the only places the list changes.
Both already run either on the CPU's own thread or with it stopped, or reach
another CPU exactly as cpu_single_step() does, which is where the previous
patch put the same kind of update.
That leaves blocks translated before the breakpoint was inserted, which are
still live and still chain to each other. They do so on the old cflags, so
inline dispatch among them keeps working until the vCPU reaches its main
loop, which then looks up with the new cflags and translates afresh. In
system mode gdb inserts breakpoints with the vCPUs stopped, so there is no
window at all. In user mode the window is the one goto_tb chaining already
has: a chained direct jump consults nothing either, and is not broken by
inserting a breakpoint.
Nothing reads CF_NO_GOTO_JC yet; the next patch does.
v5: New patch, replacing "accel/tcg: give the TB jump cache a second base
pointer for generated code", which forced the same fallback by pointing
generated code at a zero-filled jump cache when a breakpoint was
inserted, and needed a cross-thread poison and an un-poison race to do
it. Richard Henderson suggested a cflag instead, and pointed out that
the previous patch had already shown how to update tcg_cflags from
gdbstub. The base pointer comes back later in the series, for pending
exits, which a cflag cannot express.
Signed-off-by: Matt Turner <mattst88@gmail.com>
---
accel/tcg/cpu-exec-common.c | 13 ++++++++++++-
cpu-common.c | 7 +++++++
include/exec/translation-block.h | 1 +
3 files changed, 20 insertions(+), 1 deletion(-)
diff --git ./accel/tcg/cpu-exec-common.c ./accel/tcg/cpu-exec-common.c
index 9f3517f36b..a3148bbf8f 100644
--- ./accel/tcg/cpu-exec-common.c
+++ ./accel/tcg/cpu-exec-common.c
@@ -41,7 +41,7 @@ void tcg_cflags_set(CPUState *cpu, uint32_t flags)
* 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)
+ CF_SINGLE_STEP | CF_NO_GOTO_JC)
void tcg_update_cflags(CPUState *cpu)
{
@@ -62,6 +62,17 @@ void tcg_update_cflags(CPUState *cpu)
cflags |= CF_NO_GOTO_TB;
}
+ /*
+ * A block that dispatches through the jump cache inline does not consult
+ * cpu->breakpoints, and inserting a breakpoint deliberately invalidates
+ * nothing. Give blocks translated while one is set a distinct cflags, so
+ * that they neither dispatch inline themselves nor are reached by a block
+ * that does, and check_for_breakpoints() gets to run on every dispatch.
+ */
+ if (unlikely(!QTAILQ_EMPTY(&cpu->breakpoints))) {
+ cflags |= CF_NO_GOTO_JC;
+ }
+
cpu->tcg_cflags = cflags;
}
diff --git ./cpu-common.c ./cpu-common.c
index adb76b3a78..3178601987 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,9 @@ int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int flags,
*breakpoint = bp;
}
+ /* The first breakpoint takes the CPU off the inline dispatch path. */
+ tcg_update_cflags(cpu);
+
trace_breakpoint_insert(cpu->cpu_index, pc, flags);
return 0;
}
@@ -456,6 +460,9 @@ void cpu_breakpoint_remove_by_ref(CPUState *cpu, CPUBreakpoint *bp)
{
QTAILQ_REMOVE(&cpu->breakpoints, bp, entry);
+ /* The last breakpoint puts the CPU back on it. */
+ tcg_update_cflags(cpu);
+
trace_breakpoint_remove(cpu->cpu_index, bp->pc, bp->flags);
g_free(bp);
}
diff --git ./include/exec/translation-block.h ./include/exec/translation-block.h
index 40cc699031..8c4778c681 100644
--- ./include/exec/translation-block.h
+++ ./include/exec/translation-block.h
@@ -84,6 +84,7 @@ struct TranslationBlock {
#define CF_NOIRQ 0x00010000 /* Generate an uninterruptible TB */
#define CF_PCREL 0x00020000 /* Opcodes in TB are PC-relative */
#define CF_BP_PAGE 0x00040000 /* Breakpoint present in code page */
+#define CF_NO_GOTO_JC 0x00080000 /* Do not dispatch via the inline probe */
#define CF_CLUSTER_MASK 0xff000000 /* Top 8 bits are cluster ID */
#define CF_CLUSTER_SHIFT 24
--
2.54.0Every indirect branch that cannot use goto_tb ends in a dispatch that calls
helper_lookup_tb_ptr(). For an emulated compiler that is 8.4 billion helper
calls in a single translation unit: 24.6% of all TB exits take this path,
because jsr/ret/jmp have a register destination and because goto_tb is
restricted to same-page targets.
The helper itself is already tight, but each call pays for a call frame,
the can_do_io store, the get_tb_cpu_state() indirect call through
TCGCPUOps, curr_cflags(), and a breakpoint check, before it gets to the
jump cache probe that almost always hits (95.8% for this workload).
Emit the probe inline instead, for the callers that have migrated to
tcg_gen_goto_jc_*(). Those supply what it needs: the destination PC is in a
TCG temp, and the flags, cflags and cs_base the destination must match are
constants at translation time. The fast path is therefore a hash, four
guarded loads and a goto_ptr. Only a miss calls the helper, which still owns
filling the cache.
Two details matter for the generated code. The flags and cflags guards are
folded into a single aligned 64-bit load and compare, since the fields are
adjacent. And each path emits its own goto_ptr rather than branching to a
shared one: a temp live across the label is spilled and reloaded on every
dispatch, which cost 6.3% on its own.
The flags and cflags constants are safe against the other things that can
change them. CF_PARALLEL is only ever set by begin_parallel_context(),
which flushes first, so no block predating it survives to dispatch. gdb
single-step is only turned on with the CPU stopped, and a block translated
without CF_SINGLE_STEP can only be re-entered through tb_lookup(), which
from then on demands the new cflags -- so a stale-cflags block is never the
one running. Breakpoints are handled by CF_NO_GOTO_JC, added by the previous
patch: while one is set, blocks are translated with a cflags that both keeps
them off the inline path and keeps them unreachable from blocks already on
it. What is left is one_insn_per_tb and -d nochain; see below.
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, LTO
build, on top of the preceding patches:
before: 1,402,667,803,616 instructions
after: 916,415,123,244 instructions -34.67%
before: 115.56s wall clock
after: 85.59s wall clock -25.94%
The gap between the two is the point at which this stops being a
straight-line win: the helper call was highly predictable work that the
host pipelined well, so removing it retires far fewer instructions than it
saves time. IPC falls from 2.48 to 2.17 across this patch for that reason.
Despite emitting more code, this also reduces instruction cache pressure,
because a dispatch no longer jumps into qemu's .text and evicts translated
code:
before: 11,735,141,703 L1-icache-load-misses
after: 7,154,863,292 L1-icache-load-misses -39.0%
The mechanism is visible directly in a profile: helper_lookup_tb_ptr()
falls from 31.01% of samples to 0.35%, and qemu's own .text falls from
38.8% to 5.3%, with the balance moving into generated code.
Combined with the preceding patches, against an unmodified LTO build,
1,646,994,254,249 instructions fall to 916,415,123,244, or -44.36%. The
emulated compiler produces byte-identical output throughout.
A follow-up worth having: the probe is emitted entirely out of generic TCG
ops, and several backends can do much better than the result. x86_64 and
s390x have memory-operand comparisons; aarch64 can form env + off + h * 16
with a shift-add, load (tb, pc) and (cs_base, flags) with two ldp, and halve
the branches with ccmp. That wants a backend expansion of a dedicated
opcode, which is a separate series.
Open issues, hence RFC:
- one_insn_per_tb and CPU_LOG_TB_NOCHAIN can be toggled from the monitor
while a vCPU is inside a block that was translated without them. The
block keeps dispatching inline on the old cflags until it exits for some
other reason. This is the same window goto_tb chaining already has, since
a chained direct jump consults nothing either, but it is worth saying out
loud.
- The jump cache entry is read without qatomic_read(); entries are
invalidated concurrently by setting tb to NULL.
- Only alpha has been measured. The other four targets that use goto_jc are
built and boot-tested only.
v4: Split out of the patch that also changed the
tcg_gen_lookup_and_goto_ptr() API and introduced tb_jmp_cache_probe,
which are now the two preceding patches. Requested by Richard
Henderson.
v4: Emit the softmmu form of tb_jmp_cache_hash_func() under
CONFIG_SOFTMMU rather than the user-only form everywhere. v3 emitted
the user-only hash unconditionally, which was wrong for system mode
and was only not a correctness bug because a wrong index simply
misses. Caught by Richard Henderson. tcg-op.c is compiled once per
build rather than once per target, but CONFIG_SOFTMMU is set for it,
and TARGET_PAGE_BITS -- a load from target_page here -- is fixed long
before any translation happens.
v4: Compare the pc before testing tb for NULL. On a hash miss the pc is
the field most likely to differ, and an unused entry has a zero pc
that only pc 0 can match, so the tb test buys nothing ahead of it.
Suggested by Richard Henderson.
v4: Assert that offsetof(TranslationBlock, flags) is 8-byte aligned, since
folding the flags and cflags guards into one 64-bit load relies on it
and nothing else does. Requested by Richard Henderson.
v4: Zero-extend a 32-bit guest PC instead of falling back to the helper.
Suggested by Richard Henderson. The high half then folds to a compare
against zero.
v4: Describe cs_base in the probe as a second word of target-specific
flags rather than by name. Suggested by Richard Henderson.
v5: Build the folded flags/cflags constant with deposit64() rather than
under #if HOST_BIG_ENDIAN, so both arms compile on every host.
Requested by Richard Henderson.
v5: Read cpu->tb_jmp_cache directly, and honor CF_NO_GOTO_JC rather than a
poisoned base pointer, which is no longer how breakpoints are handled.
A separate base pointer comes back later in the series for pending
exits.
v5: Note the backend expansion this wants as a follow-up. Suggested by
Richard Henderson, whose list it is.
Signed-off-by: Matt Turner <mattst88@gmail.com>
---
include/tcg/tcg-op-common.h | 3 +-
tcg/tcg-op.c | 110 +++++++++++++++++++++++++++++++++++-
2 files changed, 111 insertions(+), 2 deletions(-)
diff --git ./include/tcg/tcg-op-common.h ./include/tcg/tcg-op-common.h
index 4f334faaaa..f41f3ee58f 100644
--- ./include/tcg/tcg-op-common.h
+++ ./include/tcg/tcg-op-common.h
@@ -91,7 +91,8 @@ void tcg_gen_lookup_and_goto_ptr(void);
* @pc: temp holding the destination guest PC
*
* As tcg_gen_lookup_and_goto_ptr(), but the caller states where the
- * dispatch is going, which allows the lookup to be done inline.
+ * dispatch is going, so the TB jump cache is probed inline and only a miss
+ * reaches helper_lookup_tb_ptr().
*
* The contract is that when this runs, the CPU state must already be
* exactly the destination's: @pc must hold what get_tb_cpu_state() would
diff --git ./tcg/tcg-op.c ./tcg/tcg-op.c
index a2f35359fe..b10b2d66d5 100644
--- ./tcg/tcg-op.c
+++ ./tcg/tcg-op.c
@@ -28,6 +28,8 @@
#include "tcg/tcg-op-common.h"
#include "exec/translation-block.h"
#include "exec/plugin-gen.h"
+#include "hw/core/cpu.h"
+#include "../accel/tcg/tb-hash.h"
#include "tcg-internal.h"
#include "tcg-has.h"
@@ -2735,6 +2737,102 @@ void tcg_gen_lookup_and_goto_ptr(void)
gen_lookup_tb_ptr_and_goto();
}
+static void gen_jmp_cache_hash(TCGv_i64 h, TCGv_i64 pc)
+{
+#ifdef CONFIG_SOFTMMU
+ /*
+ * tb_jmp_cache_hash_func(), softmmu form. TARGET_PAGE_BITS is a load
+ * from target_page in this translation unit, but it is decided long
+ * before any translation happens, so it is a constant here.
+ */
+ int shift = TARGET_PAGE_BITS - TB_JMP_PAGE_BITS;
+ TCGv_i64 tmp = tcg_temp_ebb_new_i64();
+
+ tcg_gen_shri_i64(tmp, pc, shift);
+ tcg_gen_xor_i64(tmp, tmp, pc);
+ tcg_gen_shri_i64(h, tmp, shift);
+ tcg_gen_andi_i64(h, h, TB_JMP_PAGE_MASK);
+ tcg_gen_andi_i64(tmp, tmp, TB_JMP_ADDR_MASK);
+ tcg_gen_or_i64(h, h, tmp);
+ tcg_temp_free_i64(tmp);
+#else
+ /* tb_jmp_cache_hash_func(), user-only form. */
+ tcg_gen_shri_i64(h, pc, TB_JMP_CACHE_BITS);
+ tcg_gen_xor_i64(h, h, pc);
+ tcg_gen_andi_i64(h, h, TB_JMP_CACHE_SIZE - 1);
+#endif
+}
+
+static void gen_jmp_cache_probe(TCGv_i64 pc, const TranslationBlock *tb)
+{
+ TCGv_ptr jc, ent, tbp, ptr;
+ TCGv_i64 h, tmp;
+ TCGLabel *slow;
+ uint64_t fpair;
+
+ QEMU_BUILD_BUG_ON(sizeof(((CPUJumpCache *)0)->array[0]) != 16);
+ QEMU_BUILD_BUG_ON(offsetof(CPUJumpCache, array[0].pc) % 8 != 0);
+ /* One 64-bit load has to cover both, so they must be adjacent... */
+ QEMU_BUILD_BUG_ON(offsetof(TranslationBlock, cflags) !=
+ offsetof(TranslationBlock, flags) + 4);
+ /* ...and aligned, which nothing else currently relies on. */
+ QEMU_BUILD_BUG_ON(offsetof(TranslationBlock, flags) % 8 != 0);
+
+ jc = tcg_temp_ebb_new_ptr();
+ ent = tcg_temp_ebb_new_ptr();
+ tbp = tcg_temp_ebb_new_ptr();
+ ptr = tcg_temp_ebb_new_ptr();
+ h = tcg_temp_ebb_new_i64();
+ tmp = tcg_temp_ebb_new_i64();
+ slow = gen_new_label();
+
+ /* ent = &jc->array[tb_jmp_cache_hash_func(pc)] */
+ gen_jmp_cache_hash(h, pc);
+ tcg_gen_shli_i64(h, h, 4);
+
+ tcg_gen_ld_ptr(jc, tcg_env,
+ offsetof(CPUState, tb_jmp_cache) - sizeof(CPUState));
+ tcg_gen_trunc_i64_ptr(ent, h);
+ tcg_gen_add_ptr(ent, jc, ent);
+
+ /*
+ * The pc first: on a hash miss it is the field most likely to differ,
+ * and an entry whose tb is NULL has a zero pc that only pc 0 matches.
+ */
+ tcg_gen_ld_i64(tmp, ent, offsetof(CPUJumpCache, array[0].pc));
+ tcg_gen_brcond_i64(TCG_COND_NE, tmp, pc, slow);
+
+ tcg_gen_ld_ptr(tbp, ent, offsetof(CPUJumpCache, array[0].tb));
+ tcg_gen_brcondi_ptr(TCG_COND_EQ, tbp, 0, slow);
+
+ /*
+ * flags and cflags are adjacent uint32_t, so one aligned 64-bit load
+ * and compare covers both.
+ */
+ fpair = (HOST_BIG_ENDIAN
+ ? deposit64(tb->cflags, 32, 32, tb->flags)
+ : deposit64(tb->flags, 32, 32, tb->cflags));
+ tcg_gen_ld_i64(tmp, tbp, offsetof(TranslationBlock, flags));
+ tcg_gen_brcondi_i64(TCG_COND_NE, tmp, fpair, slow);
+
+ /*
+ * cs_base is a second word of target-specific flags despite the name,
+ * and the pc alone does not imply it on a target that uses it.
+ */
+ tcg_gen_ld_i64(tmp, tbp, offsetof(TranslationBlock, cs_base));
+ tcg_gen_brcondi_i64(TCG_COND_NE, tmp, tb->cs_base, slow);
+
+ tcg_gen_ld_ptr(ptr, tbp, offsetof(TranslationBlock, tc.ptr));
+ tcg_gen_op1i(INDEX_op_goto_ptr, TCG_TYPE_PTR, tcgv_ptr_arg(ptr));
+
+ /*
+ * Emit a second goto_ptr rather than branching to a shared one: a temp
+ * live across the label would be spilled and reloaded on every dispatch.
+ */
+ gen_set_label(slow);
+ gen_lookup_tb_ptr_and_goto();
+}
+
/*
* The common half of tcg_gen_goto_jc_i32() and tcg_gen_goto_jc_i64(). @pc
* is widened to i64 because the jump cache is keyed on a vaddr; for a
@@ -2761,7 +2859,17 @@ static void gen_goto_jc(TCGv_i64 pc)
tcg_constant_i64(tb->cs_base));
#endif
- gen_lookup_tb_ptr_and_goto();
+ /*
+ * A breakpoint is the one thing the probe cannot check for itself, so
+ * while one is set the flag is set too and every dispatch takes the
+ * helper, which does check. See tcg_update_cflags().
+ */
+ if (tb->cflags & CF_NO_GOTO_JC) {
+ gen_lookup_tb_ptr_and_goto();
+ return;
+ }
+
+ gen_jmp_cache_probe(pc, tb);
}
void tcg_gen_goto_jc_i64(TCGv_i64 pc)
--
2.54.0translator_use_goto_tb() refuses to chain unless the destination is on the
same page as the start of the TB. For guests whose text is much larger than
a page this is expensive: an emulated alpha gcc compiling a 255k line
translation unit takes the indirect dispatch path for 8.4 billion of its
34.2 billion TB exits, and a large share of those are ordinary direct
branches that simply crossed an 8 KiB page boundary.
The restriction was made unconditional by d3a2a1d803 ("accel/tcg:
Introduce translator_use_goto_tb"), whose rationale was:
Various targets avoid the page crossing test for CONFIG_USER_ONLY,
but that is wrong: mmap and mprotect can change page permissions.
That is true, but in user-only builds the invalidation path already covers
it. There are no page tables: every mmap, mprotect and munmap reaches
page_set_flags(), which calls tb_invalidate_phys_range() whenever the flags
actually change, and tb_phys_invalidate() calls tb_jmp_unlink() to reset
incoming jumps. A chained cross-page jump is therefore broken whenever the
destination page's permissions change. This is not true in system mode,
where TBs are keyed by physical address and a page table change invalidates
nothing, so the restriction is kept there.
The rule protects one more thing, which the original rationale does not
mention: it guarantees that execution cannot enter a page without a TB
lookup, and so without check_for_breakpoints(). That is what makes a
breakpoint set after a block was translated take effect, since insertion
deliberately invalidates nothing. A link established before the breakpoint
was set would jump straight over it.
So the chaining is only enabled for a run that can never acquire a
breakpoint. In user-only mode every breakpoint comes from gdb -- BP_CPU is
g_assert_not_reached() there, and the guest cannot ask for one -- and gdb
has to be requested with -g before the first block is translated, even
though with suspend=n it may connect later. gdb_may_set_breakpoints()
reports whether it was, and is fixed for the lifetime of the process.
Add tests/tcg/multiarch/test-xpage-chain.c to cover both hazards directly.
It writes the last instruction of one page and the first of the next, so
that the fall-through between them is a cross-page goto_tb, runs it 200000
times so the chain is established, then checks that mprotect(PROT_NONE)
makes the next call fault, and that different code written into the page
once it is mapped back runs rather than a stale translation.
The two instructions -- set the return value register, and return -- are
all the architecture specific code there is; thirteen architectures supply
them and the rest skip.
The test detects the hazard it is meant to detect: with the
tb_invalidate_phys_range() call in page_set_flags() commented out, it fails
both phases, executing page B after PROT_NONE and returning the stale
result.
Run with -b, the same binary stops once the chain is established and lets
tests/tcg/multiarch/gdbstub/xpage-bp.py set a breakpoint on the far side of it,
which the next call has to stop on. With gdb_may_set_breakpoints() forced to
false so that the chaining stays on under gdb, that breakpoint is missed and
the test fails, which is what makes it a test of the gate rather than of
gdb.
Measured with qemu-alpha running an emulated alpha gcc 16.2.0 compiling the
SQLite 3.45.1 amalgamation on an x86-64 host, LTO build, on top of the
preceding patches:
before: 916,415,123,244 instructions
after: 891,254,240,071 instructions -2.75%
before: 85.59s wall clock
after: 81.45s wall clock -4.84%
Note that this is worth more in time than in instructions, the reverse of
the preceding patch: a chained jump replaces a cache probe whose loads can
miss, so the instructions it removes are more expensive than average.
Measured before the inline jump cache probe, when a missed chain cost a
helper call rather than an inline probe, the same change was worth -7.9%.
RFC because this reverses a deliberate decision and the reasoning above
wants review from someone who knows the invalidation paths better than I
do.
v3: Only take the shortcut when no gdbstub was requested. The same-page
rule also forces a lookup, and so a breakpoint check, on entry to every
page; without that, a chain established before a breakpoint was set runs
past it. Reported by Richard Henderson.
v3: Change translator_use_goto_tb() rather than translator_is_same_page().
i386, riscv and s390x call translator_is_same_page() for something else
-- enforcing that only a single-insn TB may cross a page -- and v2
changed their TB boundaries in user-only mode as a side effect. alpha
does not call it, so the numbers above are unaffected.
v3: Add the gdbstub half of the test.
v4: Move the test to tests/tcg/multiarch so that every *-user target runs
it, rather than only alpha. Requested by Alex Bennee. The direct branch
is gone with it: a fall-through off the end of a page is a cross-page
goto_tb just the same, and needs no per-architecture branch encoding or
displacement arithmetic, only "set the return value" and "return".
Built and run under qemu-user on aarch64, alpha, arm, hppa,
loongarch64, m68k, mips, ppc, ppc64le, riscv64, s390x, sh4, sparc64
and x86_64; ppc64 ELFv1 skips, because a function pointer there is a
descriptor rather than a code address.
Signed-off-by: Matt Turner <mattst88@gmail.com>
---
accel/tcg/translator.c | 33 ++-
gdbstub/user.c | 14 +
include/gdbstub/user.h | 11 +
tests/tcg/multiarch/Makefile.target | 12 +-
tests/tcg/multiarch/gdbstub/xpage-bp.py | 37 +++
tests/tcg/multiarch/test-xpage-chain.c | 336 ++++++++++++++++++++++++
6 files changed, 441 insertions(+), 2 deletions(-)
create mode 100644 tests/tcg/multiarch/gdbstub/xpage-bp.py
create mode 100644 tests/tcg/multiarch/test-xpage-chain.c
diff --git ./accel/tcg/translator.c ./accel/tcg/translator.c
index 6c8fcd7a20..8879cd626f 100644
--- ./accel/tcg/translator.c
+++ ./accel/tcg/translator.c
@@ -15,6 +15,9 @@
#include "accel/tcg/cpu-mmu-index.h"
#include "exec/target_page.h"
#include "exec/translator.h"
+#ifdef CONFIG_USER_ONLY
+#include "gdbstub/user.h"
+#endif
#include "exec/plugin-gen.h"
#include "tcg/tcg-op-common.h"
#include "internal-common.h"
@@ -110,6 +113,34 @@ bool translator_is_same_page(const DisasContextBase *db, vaddr addr)
return ((addr ^ db->pc_first) & TARGET_PAGE_MASK) == 0;
}
+/*
+ * Whether a direct jump may be chained to a destination outside the page
+ * the TB started in.
+ *
+ * In user-only mode there are no page tables. Every mmap, mprotect and
+ * munmap goes through page_set_flags(), which calls tb_invalidate_phys_range()
+ * whenever the flags actually change, and tb_phys_invalidate() unlinks
+ * incoming jumps. A cross-page link is therefore broken whenever the
+ * destination page's permissions change.
+ *
+ * What the same-page rule also provides is that execution cannot enter a page
+ * without a TB lookup, and so without check_for_breakpoints(), which is what
+ * makes a breakpoint set after a block was translated take effect. Nothing
+ * invalidates on breakpoint insertion, so a link established beforehand would
+ * jump straight over it. In user-only mode breakpoints only ever come from
+ * gdb -- BP_CPU is g_assert_not_reached() there and the guest has no way to
+ * ask for one -- and gdb has to be requested with -g before the first block
+ * is translated, so a run that has no gdbstub can never acquire a breakpoint.
+ */
+static bool use_cross_page_goto_tb(void)
+{
+#ifdef CONFIG_USER_ONLY
+ return !gdb_may_set_breakpoints();
+#else
+ return false;
+#endif
+}
+
bool translator_use_goto_tb(DisasContextBase *db, vaddr dest)
{
/* Suppress goto_tb if requested. */
@@ -118,7 +149,7 @@ bool translator_use_goto_tb(DisasContextBase *db, vaddr dest)
}
/* Check for the dest on the same page as the start of the TB. */
- return translator_is_same_page(db, dest);
+ return use_cross_page_goto_tb() || translator_is_same_page(db, dest);
}
void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns,
diff --git ./gdbstub/user.c ./gdbstub/user.c
index 9e6f9a6f37..d810f0f38c 100644
--- ./gdbstub/user.c
+++ ./gdbstub/user.c
@@ -470,6 +470,18 @@ static void *gdbserver_accept_thread(void *arg)
#define USAGE "\nUsage: -g {port|path}[,suspend={y|n}]"
+/*
+ * Set before the guest runs and never cleared, so that code translated at
+ * any point can rely on it: with suspend=n gdb may connect long after
+ * startup, and once connected it can insert a breakpoint at any time.
+ */
+static bool gdbserver_requested;
+
+bool gdb_may_set_breakpoints(void)
+{
+ return gdbserver_requested;
+}
+
bool gdbserver_start(const char *args, Error **errp)
{
g_auto(GStrv) argv = g_strsplit(args, ",", 0);
@@ -513,6 +525,8 @@ bool gdbserver_start(const char *args, Error **errp)
return false;
}
+ gdbserver_requested = true;
+
if (suspend) {
if (gdbserver_accept(port, gdb_fd, port_or_path)) {
gdb_handlesig(first_cpu, 0, NULL, NULL, 0);
diff --git ./include/gdbstub/user.h ./include/gdbstub/user.h
index 654986d483..c091cd9758 100644
--- ./include/gdbstub/user.h
+++ ./include/gdbstub/user.h
@@ -11,6 +11,17 @@
#define MAX_SIGINFO_LENGTH 128
+/**
+ * gdb_may_set_breakpoints() - whether a breakpoint can ever be inserted
+ *
+ * In user-only mode every breakpoint comes from gdb, and gdb is only ever
+ * reachable if -g was given at startup, before the guest ran a single
+ * instruction. A run that has no gdbstub can therefore never acquire a
+ * breakpoint, which lets translation take shortcuts that a breakpoint
+ * would invalidate. Stays true once true, even if gdb detaches.
+ */
+bool gdb_may_set_breakpoints(void);
+
/**
* gdb_handlesig() - yield control to gdb
* @cpu: CPU
diff --git ./tests/tcg/multiarch/Makefile.target ./tests/tcg/multiarch/Makefile.target
index ab4bf9c5d5..f8a91fed2c 100644
--- ./tests/tcg/multiarch/Makefile.target
+++ ./tests/tcg/multiarch/Makefile.target
@@ -143,6 +143,15 @@ run-gdbstub-follow-fork-mode-parent: follow-fork-mode
--bin $< --test $(MULTIARCH_SRC)/gdbstub/follow-fork-mode-parent.py, \
following parents on fork)
+# The chaining this exercises is only enabled when no gdbstub was requested,
+# so what is under test here is that requesting one turns it back off.
+run-gdbstub-xpage-bp: test-xpage-chain
+ $(call run-test, $@, $(GDB_SCRIPT) \
+ --gdb $(GDB) \
+ --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
+ --bin "$< -b" --test $(MULTIARCH_SRC)/gdbstub/xpage-bp.py, \
+ breakpoint behind an established cross-page chain)
+
run-gdbstub-late-attach: late-attach
$(call run-test, $@, env LATE_ATTACH_PY=1 $(GDB_SCRIPT) \
--gdb $(GDB) \
@@ -159,7 +168,8 @@ EXTRA_RUNS += run-gdbstub-sha1 run-gdbstub-qxfer-auxv-read \
run-gdbstub-registers run-gdbstub-prot-none \
run-gdbstub-catch-syscalls run-gdbstub-follow-fork-mode-child \
run-gdbstub-follow-fork-mode-parent \
- run-gdbstub-qxfer-siginfo-read run-gdbstub-late-attach
+ run-gdbstub-qxfer-siginfo-read run-gdbstub-late-attach \
+ run-gdbstub-xpage-bp
# ARM Compatible Semi Hosting Tests
#
diff --git ./tests/tcg/multiarch/gdbstub/xpage-bp.py ./tests/tcg/multiarch/gdbstub/xpage-bp.py
new file mode 100644
index 0000000000..f40024f16d
--- /dev/null
+++ ./tests/tcg/multiarch/gdbstub/xpage-bp.py
@@ -0,0 +1,37 @@
+"""Test that a breakpoint set after a cross-page chain is established is hit.
+
+translator_use_goto_tb() lets a direct branch chain to another page in
+user-only builds, which is only safe because a run with no gdbstub can never
+acquire a breakpoint. This runs with one, so the chaining must be off and
+the breakpoint must still be reached.
+
+This runs as a sourced script (via -x, via run-test.py).
+
+SPDX-License-Identifier: GPL-2.0-or-later
+"""
+from test_gdbstub import main, report
+
+
+def run_test():
+ """Run through the tests one by one"""
+ gdb.Breakpoint("break_here")
+ gdb.execute("continue")
+
+ # The chain exists by now; put a breakpoint on the far side of it.
+ target = int(gdb.parse_and_eval("(unsigned long)page_b_entry"))
+ if target == 0:
+ report(True, "no code emitters for this architecture, skipped")
+ return
+ gdb.execute("break *{}".format(target))
+ gdb.execute("continue")
+
+ pc = int(gdb.parse_and_eval("(unsigned long)$pc"))
+ report(pc == target, "stopped at {:#x}, expected {:#x}".format(pc, target))
+
+ gdb.execute("delete")
+ gdb.execute("continue")
+ exitcode = int(gdb.parse_and_eval("$_exitcode"))
+ report(exitcode == 0, "{} == 0".format(exitcode))
+
+
+main(run_test)
diff --git ./tests/tcg/multiarch/test-xpage-chain.c ./tests/tcg/multiarch/test-xpage-chain.c
new file mode 100644
index 0000000000..a4e34149e7
--- /dev/null
+++ ./tests/tcg/multiarch/test-xpage-chain.c
@@ -0,0 +1,336 @@
+/*
+ * Cross-page TB chaining hazard test.
+ *
+ * Two adjacent pages of hand-written code. The last instruction of page A
+ * sets the return value and falls through into page B, which returns; a TB
+ * always ends at a page boundary, so page A reaches page B through a
+ * cross-page goto_tb.
+ *
+ * Phase 1: run it enough times that QEMU chains TB_A -> TB_B.
+ * Phase 2: mprotect page B away. Re-running must fault.
+ * Phase 3: map it back and write different code into it. Re-running must
+ * execute the NEW code, not a stale chained translation.
+ *
+ * With -b, phases 2 and 3 are replaced by a stop at break_here(), where the
+ * gdbstub test sets a breakpoint on page B -- after the chain exists -- and
+ * checks that re-running the chain still stops on it. See
+ * tests/tcg/multiarch/gdbstub/xpage-bp.py.
+ *
+ * The code the two pages hold is architecture specific, so each
+ * architecture supplies two emitters:
+ *
+ * emit_set_ret(p, val) - set the integer return value register to val
+ * emit_ret(p) - return to the caller
+ *
+ * both writing at @p and returning the number of bytes written. Neither
+ * may contain a branch: the fall-through from page A into page B is the
+ * whole point, and a delay slot must not straddle the boundary. An
+ * architecture that supplies neither skips the test.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <setjmp.h>
+#include <signal.h>
+#include <stdint.h>
+#include <sys/mman.h>
+#include <unistd.h>
+
+static inline size_t put32(void *p, uint32_t insn)
+{
+ memcpy(p, &insn, sizeof(insn));
+ return sizeof(insn);
+}
+
+static inline size_t put16(void *p, uint16_t insn)
+{
+ memcpy(p, &insn, sizeof(insn));
+ return sizeof(insn);
+}
+
+#if defined(__aarch64__)
+#define HAVE_EMITTERS
+/* movz w0, #val */
+static size_t emit_set_ret(void *p, int val)
+{
+ return put32(p, 0x52800000u | ((uint32_t)val << 5));
+}
+static size_t emit_ret(void *p)
+{
+ return put32(p, 0xd65f03c0u); /* ret */
+}
+#elif defined(__alpha__)
+#define HAVE_EMITTERS
+/* lda $0, val($31) */
+static size_t emit_set_ret(void *p, int val)
+{
+ return put32(p, 0x201f0000u | (uint16_t)val);
+}
+static size_t emit_ret(void *p)
+{
+ return put32(p, 0x6bfa8001u); /* ret */
+}
+#elif defined(__arm__)
+#define HAVE_EMITTERS
+/* mov r0, #val */
+static size_t emit_set_ret(void *p, int val)
+{
+ return put32(p, 0xe3a00000u | (uint8_t)val);
+}
+static size_t emit_ret(void *p)
+{
+ return put32(p, 0xe12fff1eu); /* bx lr */
+}
+#elif defined(__hppa__)
+#define HAVE_EMITTERS
+/* ldi val, %ret0 */
+static size_t emit_set_ret(void *p, int val)
+{
+ return put32(p, 0x341c0000u | ((uint32_t)val << 1));
+}
+static size_t emit_ret(void *p)
+{
+ size_t n = put32(p, 0xe840c000u); /* bv %r0(%rp) */
+ return n + put32((char *)p + n, 0x08000240u); /* nop (delay slot) */
+}
+#elif defined(__i386__) || defined(__x86_64__)
+#define HAVE_EMITTERS
+/* mov $val, %eax */
+static size_t emit_set_ret(void *p, int val)
+{
+ uint32_t imm = val;
+ *(unsigned char *)p = 0xb8;
+ return 1 + put32((char *)p + 1, imm);
+}
+static size_t emit_ret(void *p)
+{
+ *(unsigned char *)p = 0xc3; /* ret */
+ return 1;
+}
+#elif defined(__loongarch64)
+#define HAVE_EMITTERS
+/* ori $a0, $zero, val */
+static size_t emit_set_ret(void *p, int val)
+{
+ return put32(p, 0x03800004u | ((uint32_t)val << 10));
+}
+static size_t emit_ret(void *p)
+{
+ return put32(p, 0x4c000020u); /* jr $ra */
+}
+#elif defined(__m68k__)
+#define HAVE_EMITTERS
+/* moveq #val, %d0 */
+static size_t emit_set_ret(void *p, int val)
+{
+ return put16(p, 0x7000u | (uint8_t)val);
+}
+static size_t emit_ret(void *p)
+{
+ return put16(p, 0x4e75u); /* rts */
+}
+#elif defined(__mips__)
+#define HAVE_EMITTERS
+/* li $v0, val */
+static size_t emit_set_ret(void *p, int val)
+{
+ return put32(p, 0x24020000u | (uint16_t)val);
+}
+static size_t emit_ret(void *p)
+{
+ size_t n = put32(p, 0x03e00008u); /* jr $ra */
+ return n + put32((char *)p + n, 0x00000000u); /* nop (delay slot) */
+}
+/*
+ * ELFv1 function pointers are descriptors rather than code addresses, so
+ * there is nothing to call the raw code through.
+ */
+#elif defined(__powerpc__) && \
+ (!defined(__powerpc64__) || (defined(_CALL_ELF) && _CALL_ELF == 2))
+#define HAVE_EMITTERS
+/* li r3, val */
+static size_t emit_set_ret(void *p, int val)
+{
+ return put32(p, 0x38600000u | (uint16_t)val);
+}
+static size_t emit_ret(void *p)
+{
+ return put32(p, 0x4e800020u); /* blr */
+}
+#elif defined(__riscv)
+#define HAVE_EMITTERS
+/* addi a0, zero, val -- the 4 byte form, never c.li */
+static size_t emit_set_ret(void *p, int val)
+{
+ return put32(p, 0x00000513u | ((uint32_t)val << 20));
+}
+static size_t emit_ret(void *p)
+{
+ return put32(p, 0x00008067u); /* jalr zero, 0(ra) */
+}
+#elif defined(__s390x__)
+#define HAVE_EMITTERS
+/* lghi %r2, val */
+static size_t emit_set_ret(void *p, int val)
+{
+ size_t n = put16(p, 0xa729u);
+ return n + put16((char *)p + n, (uint16_t)val);
+}
+static size_t emit_ret(void *p)
+{
+ return put16(p, 0x07feu); /* br %r14 */
+}
+#elif defined(__sh__)
+#define HAVE_EMITTERS
+/* mov #val, r0 */
+static size_t emit_set_ret(void *p, int val)
+{
+ return put16(p, 0xe000u | (uint8_t)val);
+}
+static size_t emit_ret(void *p)
+{
+ size_t n = put16(p, 0x000bu); /* rts */
+ return n + put16((char *)p + n, 0x0009u); /* nop (delay slot) */
+}
+#elif defined(__sparc__)
+#define HAVE_EMITTERS
+/* mov val, %o0 */
+static size_t emit_set_ret(void *p, int val)
+{
+ return put32(p, 0x90102000u | (uint32_t)(val & 0x1fff));
+}
+static size_t emit_ret(void *p)
+{
+ size_t n = put32(p, 0x81c3e008u); /* retl */
+ return n + put32((char *)p + n, 0x01000000u); /* nop (delay slot) */
+}
+#endif
+
+/* Where the fall-through lands, for the gdbstub test to breakpoint on. */
+void *page_b_entry;
+
+/* Somewhere for the gdbstub test to stop once the chain is established. */
+void __attribute__((noinline)) break_here(void)
+{
+ asm volatile ("");
+}
+
+#ifdef HAVE_EMITTERS
+static sigjmp_buf jb;
+/*
+ * Written by the SIGSEGV handler and read by main(), so it must not be
+ * cached in a register across the faulting call.
+ */
+static volatile sig_atomic_t caught;
+
+static void segv(int sig)
+{
+ caught = 1;
+ siglongjmp(jb, 1);
+}
+#endif
+
+int main(int argc, char **argv)
+{
+ bool bp_mode = argc > 1 && strcmp(argv[1], "-b") == 0;
+#ifndef HAVE_EMITTERS
+ printf("SKIP: no code emitters for this architecture\n");
+ if (bp_mode) {
+ break_here();
+ }
+ return 0;
+#else
+ unsigned char tmp[16];
+ struct sigaction sa;
+ long (*fn)(void);
+ size_t setlen, n;
+ long ps = sysconf(_SC_PAGESIZE);
+ int rc = 0;
+ unsigned char *m = mmap(NULL, 2 * ps, PROT_READ | PROT_WRITE | PROT_EXEC,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ if (m == MAP_FAILED) {
+ perror("mmap");
+ return 2;
+ }
+
+ unsigned char *pb = m + ps;
+
+ /*
+ * Page A ends with the store to the return value register, so that the
+ * next instruction executed is the first one on page B.
+ */
+ setlen = emit_set_ret(tmp, 1);
+ memcpy(pb - setlen, tmp, setlen);
+ emit_ret(pb);
+ __builtin___clear_cache((char *)m, (char *)m + 2 * ps);
+
+ page_b_entry = pb;
+ fn = (long (*)(void))(pb - setlen);
+
+ for (int i = 0; i < 200000; i++) {
+ if (fn() != 1) {
+ printf("FAIL: phase 1 wrong result\n");
+ return 1;
+ }
+ }
+ printf("phase 1 ok (chained)\n");
+
+ if (bp_mode) {
+ /*
+ * The chain from page A to page B now exists. gdb puts a breakpoint
+ * on page_b_entry here; the call below has to stop on it rather than
+ * jump over it.
+ */
+ break_here();
+ if (fn() != 1) {
+ printf("FAIL: bp phase wrong result\n");
+ return 1;
+ }
+ printf("bp phase ok\n");
+ return 0;
+ }
+
+ memset(&sa, 0, sizeof(sa));
+ sa.sa_handler = segv;
+ sigemptyset(&sa.sa_mask);
+ if (sigaction(SIGSEGV, &sa, NULL) != 0) {
+ perror("sigaction");
+ return 2;
+ }
+ if (mprotect(pb, ps, PROT_NONE) != 0) {
+ perror("mprotect");
+ return 2;
+ }
+ if (sigsetjmp(jb, 1) == 0) {
+ fn();
+ printf("FAIL: phase 2 executed page B after mprotect(PROT_NONE)\n");
+ rc = 1;
+ } else if (!caught) {
+ printf("FAIL: phase 2 longjmp without entering the handler\n");
+ rc = 1;
+ } else {
+ printf("phase 2 ok (faulted)\n");
+ }
+
+ /* Phase 3: map back, overwrite, expect the new code to run. */
+ if (mprotect(pb, ps, PROT_READ | PROT_WRITE | PROT_EXEC) != 0) {
+ perror("mprotect back");
+ return 2;
+ }
+ n = emit_set_ret(pb, 2);
+ emit_ret(pb + n);
+ __builtin___clear_cache((char *)pb, (char *)pb + ps);
+
+ long r = fn();
+ if (r != 2) {
+ printf("FAIL: phase 3 returned %ld, expected 2 (stale chain)\n", r);
+ rc = 1;
+ } else {
+ printf("phase 3 ok (new code ran)\n");
+ }
+ return rc;
+#endif
+}
--
2.54.0On 8/31/26 17:48, Matt Turner wrote:
> translator_use_goto_tb() refuses to chain unless the destination is on the
> same page as the start of the TB. For guests whose text is much larger than
> a page this is expensive: an emulated alpha gcc compiling a 255k line
> translation unit takes the indirect dispatch path for 8.4 billion of its
> 34.2 billion TB exits, and a large share of those are ordinary direct
> branches that simply crossed an 8 KiB page boundary.
>
> The restriction was made unconditional by d3a2a1d803 ("accel/tcg:
> Introduce translator_use_goto_tb"), whose rationale was:
>
> Various targets avoid the page crossing test for CONFIG_USER_ONLY,
> but that is wrong: mmap and mprotect can change page permissions.
>
> That is true, but in user-only builds the invalidation path already covers
> it. There are no page tables: every mmap, mprotect and munmap reaches
> page_set_flags(), which calls tb_invalidate_phys_range() whenever the flags
> actually change, and tb_phys_invalidate() calls tb_jmp_unlink() to reset
> incoming jumps. A chained cross-page jump is therefore broken whenever the
> destination page's permissions change. This is not true in system mode,
> where TBs are keyed by physical address and a page table change invalidates
> nothing, so the restriction is kept there.
>
> The rule protects one more thing, which the original rationale does not
> mention: it guarantees that execution cannot enter a page without a TB
> lookup, and so without check_for_breakpoints(). That is what makes a
> breakpoint set after a block was translated take effect, since insertion
> deliberately invalidates nothing. A link established before the breakpoint
> was set would jump straight over it.
>
> So the chaining is only enabled for a run that can never acquire a
> breakpoint. In user-only mode every breakpoint comes from gdb -- BP_CPU is
> g_assert_not_reached() there, and the guest cannot ask for one -- and gdb
> has to be requested with -g before the first block is translated, even
> though with suspend=n it may connect later. gdb_may_set_breakpoints()
> reports whether it was, and is fixed for the lifetime of the process.
>
> Add tests/tcg/multiarch/test-xpage-chain.c to cover both hazards directly.
> It writes the last instruction of one page and the first of the next, so
> that the fall-through between them is a cross-page goto_tb, runs it 200000
> times so the chain is established, then checks that mprotect(PROT_NONE)
> makes the next call fault, and that different code written into the page
> once it is mapped back runs rather than a stale translation.
>
> The two instructions -- set the return value register, and return -- are
> all the architecture specific code there is; thirteen architectures supply
> them and the rest skip.
>
> The test detects the hazard it is meant to detect: with the
> tb_invalidate_phys_range() call in page_set_flags() commented out, it fails
> both phases, executing page B after PROT_NONE and returning the stale
> result.
>
> Run with -b, the same binary stops once the chain is established and lets
> tests/tcg/multiarch/gdbstub/xpage-bp.py set a breakpoint on the far side of it,
> which the next call has to stop on. With gdb_may_set_breakpoints() forced to
> false so that the chaining stays on under gdb, that breakpoint is missed and
> the test fails, which is what makes it a test of the gate rather than of
> gdb.
>
> Measured with qemu-alpha running an emulated alpha gcc 16.2.0 compiling the
> SQLite 3.45.1 amalgamation on an x86-64 host, LTO build, on top of the
> preceding patches:
>
> before: 916,415,123,244 instructions
> after: 891,254,240,071 instructions -2.75%
>
> before: 85.59s wall clock
> after: 81.45s wall clock -4.84%
>
> Note that this is worth more in time than in instructions, the reverse of
> the preceding patch: a chained jump replaces a cache probe whose loads can
> miss, so the instructions it removes are more expensive than average.
>
> Measured before the inline jump cache probe, when a missed chain cost a
> helper call rather than an inline probe, the same change was worth -7.9%.
Cherry-picked this to tcg-next.
r~
Every translation block begins by loading cpu->neg.icount_decr.u32, testing
it and branching to the exit path. That is three host instructions at the top
of every TB, and blocks are short: an emulated alpha gcc 16.2.0 compiling a
255k line translation unit executes 34.2 billion of them at 6.04 guest
instructions each.
A block does not need to poll if every way out of it already reaches a check.
A goto_tb does not: it chains straight into its destination, with nothing in
between that looks at icount_decr, so the destination has to poll on entry.
An indirect exit does. The out-of-line path calls helper_lookup_tb_ptr()
every time, so it only needs the helper to return the epilogue while an exit
is pending. The inline probe needs a way to be told, so give it one:
CPUState::tb_jmp_cache_probe, the base pointer it reads. Normally that is
cpu->tb_jmp_cache; pointed at a shared page of zeroes instead, every entry
the probe finds has a NULL tb, every dispatch misses, and a miss lands in the
same helper. The real jump cache is untouched, so no cache contents are lost,
and the fast path pays nothing: the base was a load from CPUState either way.
The two places that set icount_decr.u16.high poison the probe; the main loop
puts it back once cpu_handle_interrupt() has cleared the reason. The poison
is a single read-only mapping shared by every CPU, because nothing may ever
write to it and a stray store into a page every vCPU dispatches through is
worth trapping rather than debugging.
The poll is therefore emitted only in blocks that emit a goto_tb. Whether a
block does is not known until its last exit has been generated, so the
decision is deferred and the load and branch are emitted retroactively at the
head of the block in gen_tb_end(), using the same emit_before_op mechanism
the can_do_io stores use. icount opts out and keeps the counter
unconditionally.
Interrupt latency is bounded at one block, as before. It does not depend on
the shape of the guest's control flow graph: a block either polls on entry or
is checked on the way out, and no run of blocks can avoid both. What changes
is where the check sits, not how often one happens.
tests/tcg/multiarch/test-indirect-irq.c is added for this: a loop whose only
back edge is an indirect branch, under alarm(1). That loop's block emits no
goto_tb, so it no longer polls, and the test passes only because the dispatch
notices instead -- it hangs if the poison is removed, which is what makes it a
test of the new mechanism rather than of the old poll. Nothing in it is
architecture specific: the loop is a computed goto, which every target's
compiler supports, so it covers whichever targets go on to use the inline
probe. The other alpha tests still pass and the emulated compiler still
produces byte-identical output.
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, LTO build, on
top of the preceding patches:
before: 891,254,240,071 instructions
after: 868,811,832,620 instructions -2.52%
before: 81.45s wall clock
after: 79.85s wall clock -1.96%
The emulated compiler produces byte-identical output.
RFC because:
- The un-poison in the main loop races a concurrent poison from another
thread. The existing barrier around icount_decr.u16.high covers it -- a
poison that lands after the sync also re-set the flag, and exit_request was
stored before it -- but this deserves more eyes than the single-threaded
user-mode testing I have given it.
- Only the inline probe needs the poison, and only alpha uses the inline
probe today. Targets on the out-of-line path are covered by the helper
check alone, but that has not been measured.
v3: Rebased onto the removal of "only poll for interrupts in blocks that can
close a cycle", which v2 sat on top of and which is dropped: it let a
straight-line run of arbitrary length go unchecked, since a block with no
backward edge polled nowhere (Richard).
The rule is now that a block polls iff it emits a goto_tb, rather than
iff it can close a control flow cycle. That keeps the bound at one block
without any analysis of the guest's control flow graph, so the objection
to the dropped patch does not carry over. The deferred-emission machinery
it needs moves here from that patch; DisasContextBase::needs_exit_check
and the hook in translator_use_goto_tb() are gone with it, and the flag
is now set by tcg_gen_goto_tb() rather than by goto_ptr emission.
All of v2's measurements were dropped: they were taken with the
cycle-analysis patch underneath, which changes both the baseline and
what is left to remove, so none of them described this patch. The
numbers above are a fresh measurement of the series as it now stands.
v4: Moved the test from tests/tcg/alpha/ to tests/tcg/multiarch/: the
mechanism is generic and nothing in the test is alpha specific (Alex).
The performance numbers above are the v3 measurements, not re-run: the
machine they were taken on is busy.
v5: CPUState::tb_jmp_cache_probe moves here from what was patch 5, which
used it for breakpoints too. Breakpoints are now a cflag, so a pending
exit is the only reason left to poison, and the machinery shrinks to
match: no NULL states to handle, no cross-thread poison from
cpu_breakpoint_insert(), and one condition rather than two.
v5: Map the poison read-only rather than leaving it a writable .bss object.
Requested by Richard Henderson. It costs a page-aligned 1MB allocation
at startup instead of nothing on disk, which the enforcement is worth.
qemu_mprotect_ro() is added for it, alongside the _rw, _rwx and _none
forms already there.
v5: Drop the NULL checks in the poison and sync helpers. Requested by
Richard Henderson: the sync is only ever called by the main loop, so it
cannot see an unrealized CPU, and unrealize now leaves the probe pointing
at the poison rather than at NULL, so neither has an unrealized state to
consider.
Signed-off-by: Matt Turner <mattst88@gmail.com>
---
accel/tcg/cpu-exec.c | 91 +++++++++++++++++++++++++
accel/tcg/internal-common.h | 9 +++
accel/tcg/tcg-accel-ops.c | 2 +
accel/tcg/translator.c | 51 +++++++++++++-
include/hw/core/cpu.h | 9 +++
include/qemu/mprotect.h | 1 +
include/tcg/tcg.h | 2 +
tcg/tcg-op.c | 21 +++++-
tests/tcg/multiarch/test-indirect-irq.c | 62 +++++++++++++++++
util/osdep.c | 9 +++
10 files changed, 253 insertions(+), 4 deletions(-)
create mode 100644 tests/tcg/multiarch/test-indirect-irq.c
diff --git ./accel/tcg/cpu-exec.c ./accel/tcg/cpu-exec.c
index ca90a77a7b..7a371e6928 100644
--- ./accel/tcg/cpu-exec.c
+++ ./accel/tcg/cpu-exec.c
@@ -19,6 +19,9 @@
#include "qemu/osdep.h"
#include "qemu/qemu-print.h"
+#include "qemu/error-report.h"
+#include "qemu/memalign.h"
+#include "qemu/mprotect.h"
#include "qapi/error.h"
#include "qapi/type-helpers.h"
#include "hw/core/cpu.h"
@@ -388,6 +391,16 @@ const void *HELPER(lookup_tb_ptr)(CPUArchState *env)
*/
cpu->neg.can_do_io = true;
+ /*
+ * A block that dispatches indirectly does not emit the icount_decr poll,
+ * so this is where a pending exit is noticed for that path: either the
+ * probe was poisoned and every dispatch arrives here, or the target uses
+ * the out-of-line lookup and always did.
+ */
+ if (unlikely(cpu_loop_exit_requested(cpu))) {
+ return tcg_code_gen_epilogue;
+ }
+
TCGTBCPUState s = cpu->cc->tcg_ops->get_tb_cpu_state(cpu);
s.cflags = curr_cflags(cpu);
@@ -779,6 +792,70 @@ 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 returns the epilogue while an exit is
+ * pending. 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. Mapped read-only, since
+ * nothing may write to it and a stray store into a shared page every vCPU
+ * dispatches through is worth trapping rather than debugging.
+ */
+static CPUJumpCache *tb_jmp_cache_poison;
+
+static void tb_jmp_cache_poison_init(void)
+{
+ size_t align = qemu_real_host_page_size();
+ size_t size = ROUND_UP(sizeof(CPUJumpCache), align);
+ void *p = qemu_memalign(align, size);
+
+ memset(p, 0, size);
+ if (qemu_mprotect_ro(p, size) < 0) {
+ /* Only the enforcement is lost; the zeroes are what matter. */
+ warn_report("could not write-protect the jump cache poison");
+ }
+ tb_jmp_cache_poison = p;
+}
+
+/*
+ * Poison @cpu's probe, from any thread. 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.
+ */
+void tcg_cpu_poison_jmp_cache(CPUState *cpu)
+{
+ qatomic_set(&cpu->tb_jmp_cache_probe, tb_jmp_cache_poison);
+}
+
+/*
+ * Called from @cpu's own main loop, which is the only context that can
+ * establish that no reason to be poisoned is left.
+ */
+void tcg_cpu_sync_jmp_cache(CPUState *cpu)
+{
+ if (qatomic_read(&cpu->tb_jmp_cache_probe) == cpu->tb_jmp_cache) {
+ return;
+ }
+
+ qatomic_set(&cpu->tb_jmp_cache_probe, cpu->tb_jmp_cache);
+
+ /*
+ * Another thread may have set icount_decr.u16.high after the caller
+ * decided no exit was pending, and its poison may have landed before
+ * the store above. Order that store against the re-read, so the race
+ * is lost in the safe direction: an exit that is still pending here
+ * poisons again, and the dispatch after it returns to the main loop.
+ */
+ smp_mb();
+ if (unlikely(cpu_loop_exit_requested(cpu))) {
+ tcg_cpu_poison_jmp_cache(cpu);
+ }
+}
+
void tcg_kick_vcpu_thread(CPUState *cpu)
{
/*
@@ -791,6 +868,9 @@ void tcg_kick_vcpu_thread(CPUState *cpu)
/* Ensure cpu_exec will see the exit request after TCG has exited. */
qatomic_store_release(&cpu->neg.icount_decr.u16.high, -1);
+
+ /* Blocks that only dispatch indirectly do not poll; stop them chaining. */
+ tcg_cpu_poison_jmp_cache(cpu);
}
static inline bool icount_exit_request(CPUState *cpu)
@@ -991,6 +1071,13 @@ cpu_exec_loop(CPUState *cpu, SyncClocks *sc)
break;
}
+ /*
+ * cpu_handle_interrupt() has just cleared everything that would
+ * make a dispatch have to come back here, so this is where the
+ * probe is allowed to return after a poison.
+ */
+ tcg_cpu_sync_jmp_cache(cpu);
+
tb = tb_lookup(cpu, s);
if (tb == NULL) {
CPUJumpCache *jc;
@@ -1092,6 +1179,7 @@ bool tcg_exec_realizefn(CPUState *cpu, Error **errp)
assert(tcg_ops->get_tb_cpu_state);
assert(tcg_ops->mmu_index);
tcg_ops->initialize();
+ tb_jmp_cache_poison_init();
tcg_target_initialized = true;
}
@@ -1099,6 +1187,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);
@@ -1116,5 +1205,7 @@ void tcg_exec_unrealizefn(CPUState *cpu)
#endif /* !CONFIG_USER_ONLY */
tlb_destroy(cpu);
+ /* Not NULL: nothing then has to special-case an unrealized CPU. */
+ tcg_cpu_poison_jmp_cache(cpu);
g_free_rcu(cpu->tb_jmp_cache, rcu);
}
diff --git ./accel/tcg/internal-common.h ./accel/tcg/internal-common.h
index 853d1b51ee..6faa039850 100644
--- ./accel/tcg/internal-common.h
+++ ./accel/tcg/internal-common.h
@@ -144,6 +144,15 @@ void page_table_config_init(void);
G_NORETURN void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr);
#endif /* CONFIG_USER_ONLY */
+/*
+ * Force @cpu's generated code back into helper_lookup_tb_ptr(), which
+ * re-checks everything the inline jump cache probe cannot. Safe to call
+ * from any thread. tcg_cpu_sync_jmp_cache() undoes it, and is for the
+ * owning CPU's main loop only.
+ */
+void tcg_cpu_poison_jmp_cache(CPUState *cpu);
+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 ./accel/tcg/tcg-accel-ops.c ./accel/tcg/tcg-accel-ops.c
index 560fe2554b..63a15f1689 100644
--- ./accel/tcg/tcg-accel-ops.c
+++ ./accel/tcg/tcg-accel-ops.c
@@ -38,6 +38,7 @@
#include "exec/cputlb.h"
#include "exec/hwaddr.h"
#include "exec/tb-flush.h"
+#include "internal-common.h"
#include "exec/translation-block.h"
#include "exec/watchpoint.h"
#include "gdbstub/enums.h"
@@ -106,6 +107,7 @@ void tcg_handle_interrupt(CPUState *cpu, int mask)
qemu_cpu_kick(cpu);
} else {
qatomic_set(&cpu->neg.icount_decr.u16.high, -1);
+ tcg_cpu_poison_jmp_cache(cpu);
}
}
diff --git ./accel/tcg/translator.c ./accel/tcg/translator.c
index 8879cd626f..89d255bd04 100644
--- ./accel/tcg/translator.c
+++ ./accel/tcg/translator.c
@@ -45,12 +45,35 @@ bool translator_io_start(DisasContextBase *db)
return true;
}
+/*
+ * A block that ends in a goto_tb chains straight to its destination: nothing
+ * between the two looks at icount_decr, so the destination has to poll on
+ * entry. A block whose exits are all indirect does not, because the dispatch
+ * itself notices -- a pending exit poisons tb_jmp_cache_probe, so the probe
+ * misses into helper_lookup_tb_ptr(), which returns the epilogue. Every block
+ * therefore either polls on entry or is checked as it leaves, which bounds
+ * interrupt latency at one block without looking at the shape of the guest's
+ * control flow graph.
+ *
+ * Which kind a block is is not known until its last exit has been emitted, so
+ * defer the decision to gen_tb_end() and emit the poll retroactively.
+ *
+ * icount needs the counter unconditionally, so it opts out.
+ */
+static bool defer_exit_check(uint32_t cflags)
+{
+ return !(cflags & CF_USE_ICOUNT);
+}
+
static TCGOp *gen_tb_start(DisasContextBase *db, uint32_t cflags)
{
TCGv_i32 count = NULL;
TCGOp *icount_start_insn = NULL;
- if ((cflags & CF_USE_ICOUNT) || !(cflags & CF_NOIRQ)) {
+ tcg_ctx->exit_check_needed = false;
+
+ if ((cflags & CF_USE_ICOUNT) ||
+ (!(cflags & CF_NOIRQ) && !defer_exit_check(cflags))) {
count = tcg_temp_new_i32();
tcg_gen_ld_i32(count, tcg_env,
offsetof(CPUState, neg.icount_decr.u32) -
@@ -76,6 +99,9 @@ static TCGOp *gen_tb_start(DisasContextBase *db, uint32_t cflags)
*/
if (cflags & CF_NOIRQ) {
tcg_ctx->exitreq_label = NULL;
+ } else if (defer_exit_check(cflags)) {
+ /* Emitted retroactively by gen_tb_end(), if this TB emits a goto_tb. */
+ tcg_ctx->exitreq_label = gen_new_label();
} else {
tcg_ctx->exitreq_label = gen_new_label();
tcg_gen_brcondi_i32(TCG_COND_LT, count, 0, tcg_ctx->exitreq_label);
@@ -91,7 +117,8 @@ static TCGOp *gen_tb_start(DisasContextBase *db, uint32_t cflags)
}
static void gen_tb_end(const TranslationBlock *tb, uint32_t cflags,
- TCGOp *icount_start_insn, int num_insns)
+ TCGOp *icount_start_insn, int num_insns,
+ TCGOp *first_insn_start)
{
if (cflags & CF_USE_ICOUNT) {
/*
@@ -102,6 +129,23 @@ static void gen_tb_end(const TranslationBlock *tb, uint32_t cflags,
tcgv_i32_arg(tcg_constant_i32(num_insns)));
}
+ if (tcg_ctx->exitreq_label && defer_exit_check(cflags) &&
+ !(cflags & CF_NOIRQ)) {
+ if (tcg_ctx->exit_check_needed) {
+ TCGv_i32 count = tcg_temp_new_i32();
+ TCGOp *save = tcg_ctx->emit_before_op;
+
+ tcg_ctx->emit_before_op = first_insn_start;
+ tcg_gen_ld_i32(count, tcg_env,
+ offsetof(CPUState, neg.icount_decr.u32) -
+ sizeof(CPUState));
+ tcg_gen_brcondi_i32(TCG_COND_LT, count, 0, tcg_ctx->exitreq_label);
+ tcg_ctx->emit_before_op = save;
+ } else {
+ tcg_ctx->exitreq_label = NULL;
+ }
+ }
+
if (tcg_ctx->exitreq_label) {
gen_set_label(tcg_ctx->exitreq_label);
tcg_gen_exit_tb(tb, TB_EXIT_REQUESTED);
@@ -238,7 +282,8 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns,
/* Emit code to exit the TB, as indicated by db->is_jmp. */
ops->tb_stop(db, cpu);
- gen_tb_end(tb, cflags, icount_start_insn, db->num_insns);
+ gen_tb_end(tb, cflags, icount_start_insn, db->num_insns,
+ first_insn_start);
/*
* Manage can_do_io for the translation block: set to false before
diff --git ./include/hw/core/cpu.h ./include/hw/core/cpu.h
index 81af7b9ee1..c8669f2cad 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: the base the inline jump cache probe reads.
+ *
+ * Normally @tb_jmp_cache. Pointed at a shared read-only page of zeroes
+ * while an exit is pending, so that every inline dispatch misses and
+ * falls back to helper_lookup_tb_ptr(), which returns to the main loop.
+ * Only generated code and the accessors in cpu-exec.c may touch it.
+ */
+ struct CPUJumpCache *tb_jmp_cache_probe;
GArray *gdb_regs;
int gdb_num_regs;
diff --git ./include/qemu/mprotect.h ./include/qemu/mprotect.h
index 1e83d1433e..4fc13d79f6 100644
--- ./include/qemu/mprotect.h
+++ ./include/qemu/mprotect.h
@@ -8,6 +8,7 @@
#define QEMU_MPROTECT_H
int qemu_mprotect_rw(void *addr, size_t size);
+int qemu_mprotect_ro(void *addr, size_t size);
int qemu_mprotect_rwx(void *addr, size_t size);
int qemu_mprotect_none(void *addr, size_t size);
diff --git ./include/tcg/tcg.h ./include/tcg/tcg.h
index 7669dc1c2d..df08c10544 100644
--- ./include/tcg/tcg.h
+++ ./include/tcg/tcg.h
@@ -389,6 +389,8 @@ struct TCGContext {
struct TCGLabelPoolData *pool_labels;
TCGLabel *exitreq_label;
+ /* Set by goto_tb emission: this TB chains without reaching a check. */
+ bool exit_check_needed;
#ifdef CONFIG_PLUGIN
/*
diff --git ./tcg/tcg-op.c ./tcg/tcg-op.c
index b10b2d66d5..1c5c5ec1d3 100644
--- ./tcg/tcg-op.c
+++ ./tcg/tcg-op.c
@@ -2713,6 +2713,13 @@ void tcg_gen_goto_tb(unsigned idx)
tcg_debug_assert((tcg_ctx->goto_tb_issue_mask & (1 << idx)) == 0);
tcg_ctx->goto_tb_issue_mask |= 1 << idx;
#endif
+ /*
+ * A goto_tb chains straight into the destination, with nothing in between
+ * that looks at icount_decr, so this TB has to poll on entry. See
+ * defer_exit_check().
+ */
+ tcg_ctx->exit_check_needed = true;
+
plugin_gen_disable_mem_helpers();
tcg_gen_op1i(INDEX_op_goto_tb, 0, idx);
}
@@ -2790,8 +2797,13 @@ static void gen_jmp_cache_probe(TCGv_i64 pc, const TranslationBlock *tb)
gen_jmp_cache_hash(h, pc);
tcg_gen_shli_i64(h, h, 4);
+ /*
+ * Not cpu->tb_jmp_cache: the probe reads its own base so that the main
+ * loop can poison it, which is how a pending exit forces every dispatch
+ * back into the helper. See tcg_cpu_sync_jmp_cache().
+ */
tcg_gen_ld_ptr(jc, tcg_env,
- offsetof(CPUState, tb_jmp_cache) - sizeof(CPUState));
+ offsetof(CPUState, tb_jmp_cache_probe) - sizeof(CPUState));
tcg_gen_trunc_i64_ptr(ent, h);
tcg_gen_add_ptr(ent, jc, ent);
@@ -2849,6 +2861,13 @@ static void gen_goto_jc(TCGv_i64 pc)
plugin_gen_disable_mem_helpers();
+ /*
+ * Neither path below needs an icount_decr poll. The helper returns to
+ * the main loop while an exit is pending, and a pending exit poisons
+ * tb_jmp_cache_probe, so the inline probe finds a NULL tb and falls into
+ * that same helper.
+ */
+
#ifdef CONFIG_DEBUG_TCG
/*
* The caller has asserted that env already describes the destination.
diff --git ./tests/tcg/multiarch/test-indirect-irq.c ./tests/tcg/multiarch/test-indirect-irq.c
new file mode 100644
index 0000000000..a672faf641
--- /dev/null
+++ ./tests/tcg/multiarch/test-indirect-irq.c
@@ -0,0 +1,62 @@
+/*
+ * A loop whose only back edge is an indirect branch must still be
+ * interruptible.
+ *
+ * Blocks that dispatch indirectly do not emit the icount_decr poll; a pending
+ * exit instead poisons the inline jump cache probe so that the dispatch falls
+ * into helper_lookup_tb_ptr(), which returns to the main loop. If that
+ * mechanism breaks, this program never leaves the loop and the test times
+ * out rather than failing an assertion.
+ *
+ * A computed goto is used deliberately: a plain while(1) would end the block
+ * with a direct backward branch, that is a goto_tb, and a block that emits a
+ * goto_tb still polls -- so it would not exercise the path under test.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include <assert.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+/* Written by the handler, read by the loop, so it must not be cached. */
+static volatile sig_atomic_t fired;
+/* Read after the loop, so the loop must not optimize the increment away. */
+static volatile unsigned long iterations;
+
+static void handler(int sig)
+{
+ fired = 1;
+}
+
+int main(void)
+{
+ /*
+ * Indexing a table with a volatile index, rather than jumping through a
+ * volatile pointer: gcc happily proves a single-valued pointer constant
+ * and emits a direct branch, which is the case this test is not about.
+ */
+ volatile int idx = 0;
+ void *target[2];
+ struct sigaction sa;
+
+ memset(&sa, 0, sizeof(sa));
+ sa.sa_handler = handler;
+ sigemptyset(&sa.sa_mask);
+ assert(sigaction(SIGALRM, &sa, NULL) == 0);
+ alarm(1);
+
+ target[0] = &&spin;
+ target[1] = &&out;
+spin:
+ iterations++;
+ if (!fired) {
+ goto *target[idx];
+ }
+out:
+
+ printf("interrupted after %lu iterations\n", iterations);
+ return 0;
+}
diff --git ./util/osdep.c ./util/osdep.c
index 4a8b8b5a90..9c72a42b4f 100644
--- ./util/osdep.c
+++ ./util/osdep.c
@@ -99,6 +99,15 @@ int qemu_mprotect_rw(void *addr, size_t size)
#endif
}
+int qemu_mprotect_ro(void *addr, size_t size)
+{
+#ifdef _WIN32
+ return qemu_mprotect__osdep(addr, size, PAGE_READONLY);
+#else
+ return qemu_mprotect__osdep(addr, size, PROT_READ);
+#endif
+}
+
int qemu_mprotect_rwx(void *addr, size_t size)
{
#ifdef _WIN32
--
2.54.0Nothing in the TCG frontend interface can express a based memory access.
tcg_gen_qemu_ld/st take an address and nothing else, so a target with a
displacement in its load and store encodings -- which is most of them --
has to materialize the address first:
ldq a1,8(a0) -> mov 0x80(%rbp),%rbx reload a0
lea 0x8(%rbx),%r12 address
mov (%r12),%r12 the load
mov %r12,0x88(%rbp) spill a1
The lea is pure loss on a host whose addressing mode has a displacement
field sitting empty. It also needs a register, at the point in a block
where pressure is highest.
Fold it. After optimization, look for an add of a constant immediately
before a guest access, defining that access's address operand, and move the
constant into a new second constant argument on the op. The add is left for
liveness to remove, so nothing breaks if its result has another use. Only
the immediately preceding op is examined: that is what the frontends emit,
and a window of one op means the pass does not have to reason about what
could have happened in between. The one thing it does check is that the add
did not clobber the base it read, since the access now reads that base
directly.
Targets opt in with TCG_TARGET_HAS_ldst_disp and an out_disp member on
TCGOutOpQemuLdSt. Without it the pass does not run, the displacement stays
zero and the existing out member is called exactly as before, so no other
backend changes behavior or needs touching.
The fold is refused unless the access has no slow path at all, since the
slow path hands addr_reg to the helper and that register no longer holds
the full guest address. That is decided generically: user-only, because
softmmu compares the unadjusted address against the TLB; a 64-bit address
type, because a 32-bit one wraps where a host displacement would not; and
no alignment test on the access. For x86_64 the displacement goes in the
disp32 that prepare_host_addr() already fills in for guest_base, so all the
backend has left to check is that guest_base plus the displacement still
fits there.
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, LTO build,
on top of the preceding patches, against a control measured in the same
session:
before: 868,811,832,620 instructions, 79.85s
after: 819,262,147,022 instructions, 77.30s
-5.70% instructions, -3.20% wall
Emitted code shrinks from 50.55MB to 48.80MB over the run, 167.4 to 161.6
bytes per block. Per Alpha opcode, the host bytes emitted for an access
fall as expected and nothing else moves:
ldq 18.3 -> 15.4 ldah 20.9 -> 20.9
ldl 16.6 -> 14.1 lda 12.9 -> 12.9
stq 12.8 -> 9.7 mov 9.8 -> 9.8
The emulated compiler produces byte-identical output and the alpha tests
still pass, including with a non-zero guest_base forced via -B.
RFC because:
- Only wired up for x86_64, and only for qemu_ld and qemu_st; the i128
qemu_ld2 and qemu_st2 pairs are left alone.
- Requiring that no slow path exists is stricter than necessary. The fast
path test can stay on the base register as long as the displacement is
itself a multiple of the required alignment, which it is for anything a
frontend emits for a struct or stack access. Recording the displacement
in TCGLabelQemuLdst and emitting one lea on the slow path would then
cover alignment-checked accesses too, at no fast path cost.
- Softmmu wants the displacement folded into the TLB comparison as well,
which is a bigger change than this one.
- A one op window catches everything the frontends emit today but is
trivially defeated by anything scheduled in between.
v4:
- Hoisted the compilation mode tests -- tcg_use_softmmu and the 64-bit
address type -- out of the backend hook and into fold_ldst_disp(), next
to the TCG_TARGET_HAS_ldst_disp test, so the loop is not entered at all
when the mode rules the fold out.
- Pass MemOp rather than MemOpIdx to the backend hook; nothing about the
mmu_idx is relevant to it.
- Moved the alignment test into generic code as ldst_disp_needs_align(),
so a backend does not have to repeat the atom_and_align_for_opc() call.
The exact answer depends on the host's atomicity capabilities, which the
generic pass does not know, so it answers for the most restrictive host.
That is the same answer for everything the frontends actually emit --
MO_ATOM_IFALIGN is the default -- and conservative for the handful of
MO_ATOM_WITHIN16 and MO_ATOM_SUBALIGN accesses, which lose the fold on a
host that could have taken it.
- What is left of the x86_64 hook is the guest_base test, so it now lives
beside x86_guest_base under the CONFIG_USER_ONLY that declares it.
- Refuse a displacement that does not fit in an int32_t, which is what
out_disp() takes. Not reachable with any real guest_base, but the pass
should not offer the backend something the interface cannot carry.
- The numbers above are unchanged from v3: they have not been re-measured
on the restructured patch, which is not expected to move them since the
accesses in this workload are all MO_ATOM_IFALIGN.
Signed-off-by: Matt Turner <mattst88@gmail.com>
---
include/tcg/tcg-opc.h | 9 ++-
tcg/tcg-op-ldst.c | 3 +-
tcg/tcg.c | 132 +++++++++++++++++++++++++++++++++++-
tcg/x86_64/tcg-target.c.inc | 41 +++++++++++
tcg/x86_64/tcg-target.h | 3 +
5 files changed, 184 insertions(+), 4 deletions(-)
diff --git ./include/tcg/tcg-opc.h ./include/tcg/tcg-opc.h
index f3a81d5d7f..92fd34d3e3 100644
--- ./include/tcg/tcg-opc.h
+++ ./include/tcg/tcg-opc.h
@@ -125,8 +125,13 @@ DEF(goto_ptr, 0, 1, 0, TCG_OPF_BB_EXIT | TCG_OPF_BB_END)
DEF(plugin_cb, 0, 0, 1, TCG_OPF_NOT_PRESENT)
DEF(plugin_mem_cb, 0, 1, 1, TCG_OPF_NOT_PRESENT)
-DEF(qemu_ld, 1, 1, 1, TCG_OPF_CALL_CLOBBER | TCG_OPF_SIDE_EFFECTS | TCG_OPF_INT)
-DEF(qemu_st, 0, 2, 1, TCG_OPF_CALL_CLOBBER | TCG_OPF_SIDE_EFFECTS | TCG_OPF_INT)
+/*
+ * The second constant argument is a displacement to add to the address,
+ * zero unless a target advertises TCG_TARGET_HAS_ldst_disp and the fold in
+ * fold_ldst_disp() applied.
+ */
+DEF(qemu_ld, 1, 1, 2, TCG_OPF_CALL_CLOBBER | TCG_OPF_SIDE_EFFECTS | TCG_OPF_INT)
+DEF(qemu_st, 0, 2, 2, TCG_OPF_CALL_CLOBBER | TCG_OPF_SIDE_EFFECTS | TCG_OPF_INT)
DEF(qemu_ld2, 2, 1, 1, TCG_OPF_CALL_CLOBBER | TCG_OPF_SIDE_EFFECTS | TCG_OPF_INT)
DEF(qemu_st2, 0, 3, 1, TCG_OPF_CALL_CLOBBER | TCG_OPF_SIDE_EFFECTS | TCG_OPF_INT)
diff --git ./tcg/tcg-op-ldst.c ./tcg/tcg-op-ldst.c
index 22211ccb45..ffc5e651a6 100644
--- ./tcg/tcg-op-ldst.c
+++ ./tcg/tcg-op-ldst.c
@@ -92,7 +92,8 @@ static MemOp tcg_canonicalize_memop(MemOp op, bool is64, bool st)
static void gen_ldst1(TCGOpcode opc, TCGType type, TCGTemp *v,
TCGTemp *addr, MemOpIdx oi)
{
- TCGOp *op = tcg_gen_op3(opc, type, temp_arg(v), temp_arg(addr), oi);
+ /* The trailing zero is the address displacement; see fold_ldst_disp(). */
+ TCGOp *op = tcg_gen_op4(opc, type, temp_arg(v), temp_arg(addr), oi, 0);
TCGOP_FLAGS(op) = get_memop(oi) & MO_SIZE;
}
diff --git ./tcg/tcg.c ./tcg/tcg.c
index 489df0e738..466604eb97 100644
--- ./tcg/tcg.c
+++ ./tcg/tcg.c
@@ -1058,6 +1058,13 @@ typedef struct TCGOutOpQemuLdSt {
TCGOutOp base;
void (*out)(TCGContext *s, TCGType type, TCGReg dest,
TCGReg addr, MemOpIdx oi);
+ /*
+ * As out(), for an access at addr + disp. Only required of targets that
+ * define TCG_TARGET_HAS_ldst_disp; for everyone else fold_ldst_disp()
+ * never runs and the displacement is always zero.
+ */
+ void (*out_disp)(TCGContext *s, TCGType type, TCGReg dest,
+ TCGReg addr, MemOpIdx oi, int32_t disp);
} TCGOutOpQemuLdSt;
typedef struct TCGOutOpQemuLdSt2 {
@@ -3574,6 +3581,123 @@ static void move_label_uses(TCGLabel *to, TCGLabel *from)
QSIMPLEQ_CONCAT(&to->branches, &from->branches);
}
+#ifndef TCG_TARGET_HAS_ldst_disp
+#define TCG_TARGET_HAS_ldst_disp 0
+#define tcg_target_ldst_disp_ok(s, opc, disp) false
+#endif
+
+/*
+ * Return true if @opc needs an alignment test in the fast path.
+ *
+ * atom_and_align_for_opc() gives the exact answer, but only once the host's
+ * atomicity capabilities are known, and those belong to the backend. Answer
+ * instead for the most restrictive host, which is valid for all of them.
+ */
+static bool ldst_disp_needs_align(MemOp opc)
+{
+ MemOp size = opc & MO_SIZE;
+
+ if (memop_alignment_bits(opc)) {
+ return true;
+ }
+ switch (opc & MO_ATOM_MASK) {
+ case MO_ATOM_NONE:
+ case MO_ATOM_IFALIGN:
+ case MO_ATOM_IFALIGN_PAIR:
+ return false;
+ case MO_ATOM_WITHIN16:
+ /* Misalignment implies !within16, and therefore no atomicity. */
+ return size != MO_128;
+ case MO_ATOM_WITHIN16_PAIR:
+ case MO_ATOM_SUBALIGN:
+ return size != MO_8;
+ default:
+ g_assert_not_reached();
+ }
+}
+
+/*
+ * Fold "add addr, base, $disp" into the guest access that follows it, so
+ * that the displacement becomes part of the host addressing mode instead of
+ * a separate instruction. Frontends have no way to express this: there is
+ * no displacement operand on tcg_gen_qemu_ld/st, so a based access always
+ * costs an extra add, and an extra register to hold its result.
+ *
+ * Only an add in the op immediately before the access is recognized. That
+ * is what the frontends emit, and a window of one op means no analysis is
+ * needed of what might have happened in between. The add is left in place;
+ * liveness removes it if its result has no other use.
+ */
+static void __attribute__((noinline))
+fold_ldst_disp(TCGContext *s)
+{
+ TCGOp *op;
+
+ /*
+ * The fold requires that the access have no slow path, because the slow
+ * path hands the address operand to the helper and that register no
+ * longer holds the complete guest address. That means user-only, since
+ * softmmu compares the unadjusted address against the TLB. It also
+ * requires a 64-bit address type: for a 32-bit one the add wraps and a
+ * host displacement would not.
+ */
+ if (!TCG_TARGET_HAS_ldst_disp || tcg_use_softmmu ||
+ s->addr_type != TCG_TYPE_I64) {
+ return;
+ }
+
+ QTAILQ_FOREACH(op, &s->ops, link) {
+ TCGOp *prev;
+ TCGTemp *cts;
+ int64_t disp;
+ MemOp opc;
+
+ switch (op->opc) {
+ case INDEX_op_qemu_ld:
+ case INDEX_op_qemu_st:
+ break;
+ default:
+ continue;
+ }
+
+ opc = get_memop(op->args[2]);
+ if (ldst_disp_needs_align(opc)) {
+ continue;
+ }
+
+ prev = QTAILQ_PREV(op, link);
+ if (prev == NULL || prev->opc != INDEX_op_add ||
+ TCGOP_TYPE(prev) != s->addr_type) {
+ continue;
+ }
+
+ /*
+ * The add must define the address operand, and must not have
+ * clobbered the base it read: after the fold the access reads the
+ * base directly, so the base has to still hold its original value.
+ */
+ if (prev->args[0] != op->args[1] || prev->args[0] == prev->args[1]) {
+ continue;
+ }
+
+ cts = arg_temp(prev->args[2]);
+ if (cts->kind != TEMP_CONST) {
+ continue;
+ }
+ /* out_disp() takes an int32_t, so anything wider cannot be passed. */
+ disp = cts->val;
+ if (disp != (int32_t)disp) {
+ continue;
+ }
+ if (disp == 0 || !tcg_target_ldst_disp_ok(s, opc, disp)) {
+ continue;
+ }
+
+ op->args[1] = prev->args[1];
+ op->args[3] = disp;
+ }
+}
+
/* Reachable analysis : remove unreachable code. */
static void __attribute__((noinline))
reachable_code_pass(TCGContext *s)
@@ -5728,7 +5852,12 @@ static void tcg_reg_alloc_op(TCGContext *s, const TCGOp *op)
const TCGOutOpQemuLdSt *out =
container_of(all_outop[op->opc], TCGOutOpQemuLdSt, base);
- out->out(s, type, new_args[0], new_args[1], new_args[2]);
+ if (new_args[3]) {
+ out->out_disp(s, type, new_args[0], new_args[1],
+ new_args[2], new_args[3]);
+ } else {
+ out->out(s, type, new_args[0], new_args[1], new_args[2]);
+ }
}
break;
@@ -6611,6 +6740,7 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb, uint64_t pc_start)
tcg_temp_ebb_reset_freed(s);
tcg_optimize(s);
+ fold_ldst_disp(s);
reachable_code_pass(s);
liveness_pass_0(s);
diff --git ./tcg/x86_64/tcg-target.c.inc ./tcg/x86_64/tcg-target.c.inc
index 2c8f1f3e58..9b177d3475 100644
--- ./tcg/x86_64/tcg-target.c.inc
+++ ./tcg/x86_64/tcg-target.c.inc
@@ -1892,6 +1892,18 @@ static HostAddress x86_guest_base = {
.index = -1
};
+/*
+ * Whether the displacement of a guest access can be folded into the host
+ * addressing mode rather than materialized by a separate lea. The generic
+ * pass has already established that the access has no slow path, so all
+ * that is left is guest_base, which shares the disp32 field.
+ */
+static bool tcg_target_ldst_disp_ok(TCGContext *s, MemOp opc, int32_t disp)
+{
+ int64_t ofs = (int64_t)x86_guest_base.ofs + disp;
+ return ofs == (int32_t)ofs;
+}
+
#if defined(__linux__)
# include <asm/prctl.h>
# include <sys/prctl.h>
@@ -1917,6 +1929,7 @@ static inline int setup_guest_base_seg(void)
#endif
#else
# define x86_guest_base (*(HostAddress *)({ qemu_build_not_reached(); NULL; }))
+# define tcg_target_ldst_disp_ok(s, opc, disp) false
#endif /* CONFIG_USER_ONLY */
#ifndef setup_guest_base_seg
# define setup_guest_base_seg() 0
@@ -2183,9 +2196,23 @@ static void tgen_qemu_ld(TCGContext *s, TCGType type, TCGReg data,
}
}
+static void tgen_qemu_ld_disp(TCGContext *s, TCGType type, TCGReg data,
+ TCGReg addr, MemOpIdx oi, int32_t disp)
+{
+ TCGLabelQemuLdst *ldst;
+ HostAddress h;
+
+ ldst = prepare_host_addr(s, &h, addr, oi, true);
+ /* tcg_target_ldst_disp_ok() has ruled out every slow path. */
+ tcg_debug_assert(ldst == NULL);
+ h.ofs += disp;
+ tcg_out_qemu_ld_direct(s, data, -1, h, type, get_memop(oi));
+}
+
static const TCGOutOpQemuLdSt outop_qemu_ld = {
.base.static_constraint = C_O1_I1(r, L),
.out = tgen_qemu_ld,
+ .out_disp = tgen_qemu_ld_disp,
};
static void tgen_qemu_ld2(TCGContext *s, TCGType type, TCGReg datalo,
@@ -2321,9 +2348,23 @@ static void tgen_qemu_st(TCGContext *s, TCGType type, TCGReg data,
}
}
+static void tgen_qemu_st_disp(TCGContext *s, TCGType type, TCGReg data,
+ TCGReg addr, MemOpIdx oi, int32_t disp)
+{
+ TCGLabelQemuLdst *ldst;
+ HostAddress h;
+
+ ldst = prepare_host_addr(s, &h, addr, oi, false);
+ /* tcg_target_ldst_disp_ok() has ruled out every slow path. */
+ tcg_debug_assert(ldst == NULL);
+ h.ofs += disp;
+ tcg_out_qemu_st_direct(s, data, -1, h, get_memop(oi));
+}
+
static const TCGOutOpQemuLdSt outop_qemu_st = {
.base.static_constraint = C_O0_I2(L, L),
.out = tgen_qemu_st,
+ .out_disp = tgen_qemu_st_disp,
};
static void tgen_qemu_st2(TCGContext *s, TCGType type, TCGReg datalo,
diff --git ./tcg/x86_64/tcg-target.h ./tcg/x86_64/tcg-target.h
index 7ebae56a7d..8f2315c15e 100644
--- ./tcg/x86_64/tcg-target.h
+++ ./tcg/x86_64/tcg-target.h
@@ -30,6 +30,9 @@
#define TCG_TARGET_NB_REGS 32
#define MAX_CODE_GEN_BUFFER_SIZE (2 * GiB)
+/* A guest displacement can go in the disp32 of the addressing mode. */
+#define TCG_TARGET_HAS_ldst_disp 1
+
typedef enum {
TCG_REG_EAX = 0,
TCG_REG_ECX,
--
2.54.0© 2016 - 2026 Red Hat, Inc.