From b79a9b6e5b5657534615dd8e574d58305e16841c Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Thu, 2 Jul 2026 10:10:56 -0700 Subject: [PATCH 1/6] accel/tcg: Use TLB_FORCE_SLOW not TLB_MMIO for user-only plugins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In 6d03226b422 we set TLB_MMIO to a non-zero value for user-only so that we could return a non-zero value from probe_* functions so that we could force callers like Arm SVE vector moves to use the slow path rather than direct access. All for the sake of exposing these accesses to plugins. Back then, TLB_FORCE_SLOW did not exist, so TLB_MMIO seemed like a reasonable solution. However, user-only doesn't really have MMIO and this has knock-on effects, like forcing Arm SVE first-fault vector loads to stop. Better to use TLB_FORCE_SLOW as a more exact trigger for plugins. Cc: qemu-stable@nongnu.org Fixes: 6d03226b422 ("plugins: force slow path when plugins instrument memory ops") Acked-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson Message-ID: <20260702171057.47998-1-richard.henderson@linaro.org> --- accel/tcg/user-exec.c | 4 ++-- include/exec/tlb-flags.h | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c index 7704e4017d..e4629b837d 100644 --- a/accel/tcg/user-exec.c +++ b/accel/tcg/user-exec.c @@ -769,7 +769,7 @@ static int probe_access_internal(CPUArchState *env, vaddr addr, if (page_flags & acc_flag) { if (access_type != MMU_INST_FETCH && cpu_plugin_mem_cbs_enabled(env_cpu(env))) { - return TLB_MMIO; + return TLB_FORCE_SLOW; } return 0; /* success */ } @@ -804,7 +804,7 @@ void *probe_access(CPUArchState *env, vaddr addr, int size, g_assert(-(addr | TARGET_PAGE_MASK) >= size); flags = probe_access_internal(env, addr, size, access_type, false, ra); - g_assert((flags & ~TLB_MMIO) == 0); + g_assert((flags & ~TLB_FORCE_SLOW) == 0); return size ? g2h_vaddr(env_cpu(env), addr) : NULL; } diff --git a/include/exec/tlb-flags.h b/include/exec/tlb-flags.h index 357e79095c..e12cddf234 100644 --- a/include/exec/tlb-flags.h +++ b/include/exec/tlb-flags.h @@ -27,12 +27,13 @@ /* * Allow some level of source compatibility with softmmu. - * Invalid is set when the page does not have requested permissions. - * MMIO is set when we want the target helper to use the functional + * INVALID is set when the page does not have requested permissions. + * FORCE_SLOW is set when we want the target helper to use the functional * interface for load/store so that plugins see the access. */ #define TLB_INVALID_MASK (1 << 0) -#define TLB_MMIO (1 << 1) +#define TLB_FORCE_SLOW (1 << 1) +#define TLB_MMIO 0 #define TLB_WATCHPOINT 0 #else From e03b7dac65d96d7d9b34bb88803029cf5ec7e4a9 Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Mon, 6 Jul 2026 18:51:23 +0200 Subject: [PATCH 2/6] accel/tcg: Make PageFlagsNodes' start and last immutable page_check_range() may race with pageflags_set_clear() as follows: T1 T2 ------------------------------------- -------------------------------- p = pageflags_find(start, last); interval_tree_remove(&p->itree, ...); p->itree.start = last + 1; if (start < p->itree.start) { ret = false; interval_tree_insert(&p->itree, ...); leading to errors like fail indirect write 0x72f0a659aff0 (Bad address) in vma-pthread test. I am able to reliably reproduce this on a machine with 32 SMT threads as follows in about 25 seconds: jobs=32; \ seq "$jobs" | \ time -p parallel \ --jobs="$jobs" \ --halt=now,done=1 \ --ungroup \ ' _={}; while ./qemu-s390x tests/tcg/s390x-linux-user/vma-pthread; do printf .; done ' Also wasmtime project reported a similar failure pattern in their CI [1] with a similar reproducer [2]. There are other races like this. In general, region bounds mutating underneath the reader are very hard to reason about. So fix this by preventing mutations and creating copies instead. Use RCU guards in readers to avoid uses-after-frees. Now, when the reader finds a node, it may fearlessly access its fields and be certain that at some point in time the respective region had the respective bounds and permissions. The downside is slightly more expensive mprotect(), but complexity reduction is worth it. Lockless field accesses should probably be wrapped in qatomic_read(), but this is a pre-existing issue, so do not change it here. [1] https://github.com/bytecodealliance/wasmtime/issues/10000 [2] https://gist.github.com/alexcrichton/f14f23a892ffb9df2522754572d51b1c Cc: qemu-stable@nongnu.org Reported-by: Alex Crichton Reported-by: Ulrich Weigand Fixes: 67ff2186b0a4 ("accel/tcg: Use interval tree for user-only page tracking") Signed-off-by: Ilya Leoshkevich Reviewed-by: Richard Henderson Reviewed-by: Pierrick Bouvier Signed-off-by: Richard Henderson Message-ID: <20260706165445.57418-2-iii@linux.ibm.com> --- accel/tcg/user-exec.c | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c index e4629b837d..a35aca7889 100644 --- a/accel/tcg/user-exec.c +++ b/accel/tcg/user-exec.c @@ -239,13 +239,16 @@ void page_dump(FILE *f) int page_get_flags(vaddr address) { - PageFlagsNode *p = pageflags_find(address, address); + PageFlagsNode *p; + + RCU_READ_LOCK_GUARD(); /* * See util/interval-tree.c re lockless lookups: no false positives but * there are false negatives. If we find nothing, retry with the mmap * lock acquired. */ + p = pageflags_find(address, address); if (p) { return p->flags; } @@ -301,15 +304,15 @@ static void pageflags_create_merge(vaddr start, vaddr last, int flags) if (prev) { if (next) { - prev->itree.last = next->itree.last; + pageflags_create(prev->itree.start, next->itree.last, flags); g_free_rcu(next, rcu); } else { - prev->itree.last = last; + pageflags_create(prev->itree.start, last, flags); } - interval_tree_insert(&prev->itree, &pageflags_root); + g_free_rcu(prev, rcu); } else if (next) { - next->itree.start = start; - interval_tree_insert(&next->itree, &pageflags_root); + pageflags_create(start, next->itree.last, flags); + g_free_rcu(next, rcu); } else { pageflags_create(start, last, flags); } @@ -371,8 +374,8 @@ static bool pageflags_set_clear(vaddr start, vaddr last, if (set_flags != merge_flags) { if (p_start < start) { interval_tree_remove(&p->itree, &pageflags_root); - p->itree.last = start - 1; - interval_tree_insert(&p->itree, &pageflags_root); + pageflags_create(p_start, start - 1, p_flags); + g_free_rcu(p, rcu); if (last < p_last) { if (merge_flags & PAGE_VALID) { @@ -394,11 +397,11 @@ static bool pageflags_set_clear(vaddr start, vaddr last, } if (last < p_last) { interval_tree_remove(&p->itree, &pageflags_root); - p->itree.start = last + 1; - interval_tree_insert(&p->itree, &pageflags_root); + pageflags_create(last + 1, p_last, p_flags); if (merge_flags & PAGE_VALID) { pageflags_create(start, last, merge_flags); } + g_free_rcu(p, rcu); } else { if (merge_flags & PAGE_VALID) { p->flags = merge_flags; @@ -419,8 +422,8 @@ static bool pageflags_set_clear(vaddr start, vaddr last, if (set_flags == p_flags) { if (start < p_start) { interval_tree_remove(&p->itree, &pageflags_root); - p->itree.start = start; - interval_tree_insert(&p->itree, &pageflags_root); + pageflags_create(start, p_last, p_flags); + g_free_rcu(p, rcu); } if (p_last < last) { start = p_last + 1; @@ -432,8 +435,8 @@ static bool pageflags_set_clear(vaddr start, vaddr last, /* Maybe split out head and/or tail ranges with the original flags. */ interval_tree_remove(&p->itree, &pageflags_root); if (p_start < start) { - p->itree.last = start - 1; - interval_tree_insert(&p->itree, &pageflags_root); + pageflags_create(p_start, start - 1, p_flags); + g_free_rcu(p, rcu); if (p_last < last) { goto restart; @@ -442,8 +445,8 @@ static bool pageflags_set_clear(vaddr start, vaddr last, pageflags_create(last + 1, p_last, p_flags); } } else if (last < p_last) { - p->itree.start = last + 1; - interval_tree_insert(&p->itree, &pageflags_root); + pageflags_create(last + 1, p_last, p_flags); + g_free_rcu(p, rcu); } else { g_free_rcu(p, rcu); goto restart; @@ -505,6 +508,8 @@ bool page_check_range(vaddr start, vaddr len, int flags) return false; /* wrap around */ } + RCU_READ_LOCK_GUARD(); + locked = have_mmap_lock(); while (true) { PageFlagsNode *p = pageflags_find(start, last); From 39678e16e805d92ebaa465172bd92f5c245a1756 Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Mon, 6 Jul 2026 18:51:24 +0200 Subject: [PATCH 3/6] tests/tcg/multiarch: Improve mutator randomness Currently mutators perform the same actions, because the RNG seed is derived from the current time in seconds. Mix in thread ID. Signed-off-by: Ilya Leoshkevich Reviewed-by: Richard Henderson Reviewed-by: Pierrick Bouvier Signed-off-by: Richard Henderson Message-ID: <20260706165445.57418-3-iii@linux.ibm.com> --- tests/tcg/multiarch/vma-pthread.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/tcg/multiarch/vma-pthread.c b/tests/tcg/multiarch/vma-pthread.c index 7045da08fc..0b8ce9751b 100644 --- a/tests/tcg/multiarch/vma-pthread.c +++ b/tests/tcg/multiarch/vma-pthread.c @@ -11,6 +11,7 @@ * pages, which are guaranteed to have the respective protection bit set. * Two mutator threads change the non-fixed protection bits randomly. */ +#define _GNU_SOURCE #include #include #include @@ -121,7 +122,7 @@ static void *thread_mutate(void *arg) unsigned int seed; int prot, ret; - seed = (unsigned int)time(NULL); + seed = (unsigned int)time(NULL) + (unsigned int)gettid(); for (i = 0; i < 10000; i++) { start_idx = rand_r(&seed) & PAGE_IDX_MASK; end_idx = rand_r(&seed) & PAGE_IDX_MASK; From db6b71f1ad74a7e6ebef961cc7534fe83e12980a Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Mon, 6 Jul 2026 18:51:25 +0200 Subject: [PATCH 4/6] Revert "tests/tcg: skip the vma-pthread test on CI" Now that the page_check_range() race condition is fixed, let vma-pthread run on CI again. This reverts commit 5842de51573fdbd7299ab4b33d64b7446cc07649. Signed-off-by: Ilya Leoshkevich Reviewed-by: Richard Henderson Reviewed-by: Pierrick Bouvier Signed-off-by: Richard Henderson Message-ID: <20260706165445.57418-4-iii@linux.ibm.com> --- tests/tcg/multiarch/Makefile.target | 9 --------- 1 file changed, 9 deletions(-) diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target index 508149d57b..ab4bf9c5d5 100644 --- a/tests/tcg/multiarch/Makefile.target +++ b/tests/tcg/multiarch/Makefile.target @@ -60,15 +60,6 @@ tb-link: LDFLAGS+=-lpthread # code to work around the compiler. sha1: CFLAGS+=-Wno-stringop-overread -Wno-unknown-warning-option -# The vma-pthread seems very sensitive on gitlab and we currently -# don't know if its exposing a real bug or the test is flaky. -ifneq ($(GITLAB_CI),) -run-vma-pthread: vma-pthread - $(call skip-test, $<, "flaky on CI?") -run-plugin-vma-pthread-with-%: vma-pthread - $(call skip-test, $<, "flaky on CI?") -endif - run-test-mmap: test-mmap $(call run-test, test-mmap, $(QEMU) $<, $< (default)) From c7093e3704ab5d0bef4fa44fb6fa76ff5565c89f Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Tue, 7 Jul 2026 14:28:19 +0200 Subject: [PATCH 5/6] tcg/x86_64: declare MO_ATOM_WITHIN16 host atomicity support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Just like aarch64's prepare_host_addr(), x86_64 should use MO_ATOM_WITHIN16 for the memop when it's capable. Unlike aarch64, which needs to check a CPU feature, x86 has been capable since P6 family processors and newer (see Intel SDM Vol. 3 §11.1.1). Since a 16-byte aligned region always fits within a 16-byte multiple sized cache line (x86_64 implementations always have cache lines of at least 64 bytes), then this enables riscv cpu models with Zama16b to use the fast path, just as cpu models without Zama16b do. Cc: LIU Zhiwei Signed-off-by: Andrew Jones Reviewed-by: Richard Henderson [rth: Update both atom_and_align_for_opc calls] Signed-off-by: Richard Henderson Message-ID: <20260707122819.114105-1-andrew.jones@oss.qualcomm.com> --- tcg/x86_64/tcg-target.c.inc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tcg/x86_64/tcg-target.c.inc b/tcg/x86_64/tcg-target.c.inc index 92251f8327..1fc45e4ec6 100644 --- a/tcg/x86_64/tcg-target.c.inc +++ b/tcg/x86_64/tcg-target.c.inc @@ -1783,7 +1783,7 @@ bool tcg_target_has_memory_bswap(MemOp memop) * Reject 16-byte memop with 16-byte atomicity, i.e. VMOVDQA, * but do allow a pair of 64-bit operations, i.e. MOVBEQ. */ - aa = atom_and_align_for_opc(tcg_ctx, memop, MO_ATOM_IFALIGN, true); + aa = atom_and_align_for_opc(tcg_ctx, memop, MO_ATOM_WITHIN16, true); return aa.atom < MO_128; } @@ -1934,7 +1934,7 @@ static TCGLabelQemuLdst *prepare_host_addr(TCGContext *s, HostAddress *h, *h = x86_guest_base; } h->base = addr; - h->aa = atom_and_align_for_opc(s, opc, MO_ATOM_IFALIGN, s_bits == MO_128); + h->aa = atom_and_align_for_opc(s, opc, MO_ATOM_WITHIN16, s_bits == MO_128); a_mask = (1 << h->aa.align) - 1; if (tcg_use_softmmu) { From c56ebd64b82aa4d4a4e2144abbf9568ef593b836 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Tue, 23 Jun 2026 07:06:09 -0700 Subject: [PATCH 6/6] tcg/loongarch64: Fix cmp_vec with TCG_COND_NE For NE we need to invert EQ, not swap operands. Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3589 Signed-off-by: Richard Henderson Message-ID: <20260623140609.645445-1-richard.henderson@linaro.org> --- tcg/loongarch64/tcg-target.c.inc | 39 +++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/tcg/loongarch64/tcg-target.c.inc b/tcg/loongarch64/tcg-target.c.inc index c3350c90fc..182dcfd5eb 100644 --- a/tcg/loongarch64/tcg-target.c.inc +++ b/tcg/loongarch64/tcg-target.c.inc @@ -2371,19 +2371,36 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc, default: g_assert_not_reached(); } - break; - } - - insn = cmp_vec_insn[cond][lasx][vece]; - if (insn == 0) { - TCGArg t; - t = a1, a1 = a2, a2 = t; - cond = tcg_swap_cond(cond); - insn = cmp_vec_insn[cond][lasx][vece]; - tcg_debug_assert(insn != 0); + } else { + switch (cond) { + case TCG_COND_EQ: + case TCG_COND_LE: + case TCG_COND_LEU: + case TCG_COND_LT: + case TCG_COND_LTU: + insn = cmp_vec_insn[cond][lasx][vece]; + tcg_out32(s, encode_vdvjvk_insn(insn, a0, a1, a2)); + break; + case TCG_COND_GE: + case TCG_COND_GEU: + case TCG_COND_GT: + case TCG_COND_GTU: + insn = cmp_vec_insn[tcg_swap_cond(cond)][lasx][vece]; + tcg_out32(s, encode_vdvjvk_insn(insn, a0, a2, a1)); + break; + case TCG_COND_NE: + /* ne -> not(eq) */ + insn = cmp_vec_insn[TCG_COND_EQ][lasx][vece]; + tcg_out32(s, encode_vdvjvk_insn(insn, a0, a1, a2)); + insn = lasx ? OPC_XVNOR_V : OPC_VNOR_V; + tcg_out32(s, encode_vdvjvk_insn(insn, a0, a0, a0)); + break; + default: + g_assert_not_reached(); + } } } - goto vdvjvk; + break; case INDEX_op_add_vec: tcg_out_addsub_vec(s, lasx, vece, a0, a1, a2, const_args[2], true); break;