[PATCH 0/3] linux-user: Call tcg_flush_jmp_cache() before re-using threads

[PATCH 0/3] linux-user: Call tcg_flush_jmp_cache() before re-using threads

[Philippe Mathieu-Daudé]

Fix a bug reported by Ilya in:
https://lore.kernel.org/qemu-devel/uuuk6a2vo24yrrqrchjxaeko3utqshrdu6txcnqziokpg7dkom@4l4kd3dqh6jc/

Commit bb6cf6f0168 ("accel/tcg: Factor tcg_cpu_reset_hold()
out") wanted to restrict tlb_flush() to system emulation,
but inadvertently also restricted tcg_flush_jmp_cache(),
which was before called on user emulation via:

  Realize -> Reset -> cpu_common_reset_hold()

Since threads (vCPUs) use a common CPUJumpCache, when many
threads are created / joined, they eventually end re-using
a CPUJumpCache entry, which was cleared when the first vCPU
was allocated (via Realize) but then stayed dirty.

Have cpu_exec_reset_hold() call the common tcg_exec_reset()
helper on user emulation, eventually calling tcg_flush_jmp_cache().

Philippe Mathieu-Daudé (3):
  linux-user: Only include 'exec/tb-flush.h' header when necessary
  accel/tcg: Factor out common tcg_exec_reset() helper
  accel/tcg: Implement cpu_exec_reset_hold() on user emulation

 accel/tcg/internal-common.h | 1 +
 linux-user/user-internals.h | 1 -
 accel/tcg/cpu-exec-common.c | 6 ++++++
 accel/tcg/tcg-accel-ops.c   | 4 ++--
 accel/tcg/user-exec-stub.c  | 4 ----
 accel/tcg/user-exec.c       | 5 +++++
 linux-user/mmap.c           | 1 +
 linux-user/syscall.c        | 1 +
 8 files changed, 16 insertions(+), 7 deletions(-)

-- 
2.47.1

[PATCH 1/3] linux-user: Only include 'exec/tb-flush.h' header when necessary

[Philippe Mathieu-Daudé]

Very few source files require to access "exec/tb-flush.h"
declarations, and except a pair, they all include it
explicitly. No need to overload the generic "user-internals.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 linux-user/user-internals.h | 1 -
 linux-user/mmap.c           | 1 +
 linux-user/syscall.c        | 1 +
 3 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/linux-user/user-internals.h b/linux-user/user-internals.h
index b9b05c1d11f..4aa253b5663 100644
--- a/linux-user/user-internals.h
+++ b/linux-user/user-internals.h
@@ -20,7 +20,6 @@
 
 #include "user/thunk.h"
 #include "exec/exec-all.h"
-#include "exec/tb-flush.h"
 #include "qemu/log.h"
 
 extern char *exec_path;
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 6828b17a63f..d1f36e6f16b 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -21,6 +21,7 @@
 #include "trace.h"
 #include "exec/log.h"
 #include "exec/page-protection.h"
+#include "exec/tb-flush.h"
 #include "exec/translation-block.h"
 #include "qemu.h"
 #include "user/page-protection.h"
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 78c7c0b34ef..cbbfcf10d28 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -26,6 +26,7 @@
 #include "tcg/startup.h"
 #include "target_mman.h"
 #include "exec/page-protection.h"
+#include "exec/tb-flush.h"
 #include "exec/translation-block.h"
 #include <elf.h>
 #include <endian.h>
-- 
2.47.1

[PATCH 2/3] accel/tcg: Factor out common tcg_exec_reset() helper

[Philippe Mathieu-Daudé]

Since tcg_cpu_reset_hold() is a system emulation specific
helper, factor tcg_exec_reset() out so we can use it from
user emulation, similarly to the [un]realize() handlers.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/tcg/internal-common.h | 1 +
 accel/tcg/cpu-exec-common.c | 6 ++++++
 accel/tcg/tcg-accel-ops.c   | 4 ++--
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/accel/tcg/internal-common.h b/accel/tcg/internal-common.h
index c8d714256cb..8b474fc1256 100644
--- a/accel/tcg/internal-common.h
+++ b/accel/tcg/internal-common.h
@@ -55,6 +55,7 @@ void cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb,
 
 bool tcg_exec_realizefn(CPUState *cpu, Error **errp);
 void tcg_exec_unrealizefn(CPUState *cpu);
+void tcg_exec_reset(CPUState *cpu);
 
 /* current cflags for hashing/comparison */
 uint32_t curr_cflags(CPUState *cpu);
diff --git a/accel/tcg/cpu-exec-common.c b/accel/tcg/cpu-exec-common.c
index 6ecfc4e7c21..72ab9c3d977 100644
--- a/accel/tcg/cpu-exec-common.c
+++ b/accel/tcg/cpu-exec-common.c
@@ -21,6 +21,7 @@
 #include "system/cpus.h"
 #include "system/tcg.h"
 #include "qemu/plugin.h"
+#include "exec/tb-flush.h"
 #include "internal-common.h"
 
 bool tcg_allowed;
@@ -56,3 +57,8 @@ void cpu_loop_exit_atomic(CPUState *cpu, uintptr_t pc)
     cpu->exception_index = EXCP_ATOMIC;
     cpu_loop_exit_restore(cpu, pc);
 }
+
+void tcg_exec_reset(CPUState *cpu)
+{
+    tcg_flush_jmp_cache(cpu);
+}
diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
index 6e3f1fa92b2..4fe6821b017 100644
--- a/accel/tcg/tcg-accel-ops.c
+++ b/accel/tcg/tcg-accel-ops.c
@@ -34,7 +34,6 @@
 #include "qemu/timer.h"
 #include "exec/exec-all.h"
 #include "exec/hwaddr.h"
-#include "exec/tb-flush.h"
 #include "exec/translation-block.h"
 #include "gdbstub/enums.h"
 
@@ -44,6 +43,7 @@
 #include "tcg-accel-ops-mttcg.h"
 #include "tcg-accel-ops-rr.h"
 #include "tcg-accel-ops-icount.h"
+#include "internal-common.h"
 
 /* common functionality among all TCG variants */
 
@@ -83,7 +83,7 @@ int tcg_cpu_exec(CPUState *cpu)
 
 static void tcg_cpu_reset_hold(CPUState *cpu)
 {
-    tcg_flush_jmp_cache(cpu);
+    tcg_exec_reset(cpu);
 
     tlb_flush(cpu);
 }
-- 
2.47.1

[PATCH 3/3] accel/tcg: Implement cpu_exec_reset_hold() on user emulation

[Philippe Mathieu-Daudé]

Commit bb6cf6f0168 ("accel/tcg: Factor tcg_cpu_reset_hold()
out") wanted to restrict tlb_flush() to system emulation,
but inadvertently also restricted tcg_flush_jmp_cache(),
which was before called on user emulation via:

  Realize -> Reset -> cpu_common_reset_hold()

Since threads (vCPUs) use a common CPUJumpCache, when many
threads are created / joined, they eventually end re-using
a CPUJumpCache entry, which was cleared when the first vCPU
was allocated (via Realize) but then stayed dirty, leading to:

  Thread 1 "qemu-s390x" received signal SIGABRT, Aborted.
  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
  44      ./nptl/pthread_kill.c: No such file or directory.
  (gdb) bt
  #0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
  #1  0x00007ffff7c41e8f in __pthread_kill_internal (signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:78
  #2  0x00007ffff7bf2fb2 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
  #3  0x00007ffff7bdd472 in __GI_abort () at ./stdlib/abort.c:79
  #4  0x00007ffff7bdd395 in __assert_fail_base (fmt=0x7ffff7d51a90 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=assertion@entry=0x5555556d71b8 "cpu->accel",
      file=file@entry=0x5555556d70e0 "cpu-target.c", line=line@entry=158, function=function@entry=0x5555556d7260 <__PRETTY_FUNCTION__.3> "cpu_exec_realizefn") at ./assert/assert.c:92
  #5  0x00007ffff7bebeb2 in __GI___assert_fail (assertion=assertion@entry=0x5555556d71b8 "cpu->accel", file=file@entry=0x5555556d70e0 "cpu-target.c", line=line@entry=158,
      function=function@entry=0x5555556d7260 <__PRETTY_FUNCTION__.3> "cpu_exec_realizefn") at ./assert/assert.c:101
  #6  0x00005555555d44ca in cpu_exec_realizefn (cpu=cpu@entry=0x5555557c28c0, errp=errp@entry=0x7fffffffe140) at cpu-target.c:158
  #7  0x000055555559f50b in s390_cpu_realizefn (dev=0x5555557c28c0, errp=0x7fffffffe1a0) at target/s390x/cpu.c:261
  #8  0x000055555563f78b in device_set_realized (obj=<optimized out>, value=<optimized out>, errp=0x7fffffffe2e0) at hw/core/qdev.c:510
  #9  0x000055555564365d in property_set_bool (obj=0x5555557c28c0, v=<optimized out>, name=<optimized out>, opaque=0x5555557a9140, errp=0x7fffffffe2e0) at qom/object.c:2362
  #10 0x0000555555646bbb in object_property_set (obj=obj@entry=0x5555557c28c0, name=name@entry=0x5555556e8ae2 "realized", v=v@entry=0x5555557c6650, errp=errp@entry=0x7fffffffe2e0)
      at qom/object.c:1471
  #11 0x000055555564a45f in object_property_set_qobject (obj=obj@entry=0x5555557c28c0, name=name@entry=0x5555556e8ae2 "realized", value=value@entry=0x5555557a7a90, errp=errp@entry=0x7fffffffe2e0)
      at qom/qom-qobject.c:28
  #12 0x0000555555647224 in object_property_set_bool (obj=0x5555557c28c0, name=name@entry=0x5555556e8ae2 "realized", value=value@entry=true, errp=errp@entry=0x7fffffffe2e0)
      at qom/object.c:1541
  #13 0x000055555564027c in qdev_realize (dev=<optimized out>, bus=bus@entry=0x0, errp=errp@entry=0x7fffffffe2e0) at hw/core/qdev.c:291
  #14 0x000055555559bb54 in cpu_create (typename=<optimized out>) at hw/core/cpu-common.c:57
  #15 0x000055555559a467 in main (argc=4, argv=0x7fffffffeaa8, envp=<optimized out>) at linux-user/main.c:811

Have cpu_exec_reset_hold() call the common tcg_exec_reset()
helper on user emulation, eventually calling tcg_flush_jmp_cache().

Fixes: bb6cf6f0168 ("accel/tcg: Factor tcg_cpu_reset_hold() out")
Reported-by: Ilya Leoshkevich <iii@linux.ibm.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/tcg/user-exec-stub.c | 4 ----
 accel/tcg/user-exec.c      | 5 +++++
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/accel/tcg/user-exec-stub.c b/accel/tcg/user-exec-stub.c
index 4fbe2dbdc88..2dc6fd9c4e8 100644
--- a/accel/tcg/user-exec-stub.c
+++ b/accel/tcg/user-exec-stub.c
@@ -14,10 +14,6 @@ void qemu_init_vcpu(CPUState *cpu)
 {
 }
 
-void cpu_exec_reset_hold(CPUState *cpu)
-{
-}
-
 /* User mode emulation does not support record/replay yet.  */
 
 bool replay_exception(void)
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 0561c4f6dc7..92640f07ed7 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -40,6 +40,11 @@ __thread uintptr_t helper_retaddr;
 
 //#define DEBUG_SIGNAL
 
+void cpu_exec_reset_hold(CPUState *cpu)
+{
+    tcg_exec_reset(cpu);
+}
+
 void cpu_interrupt(CPUState *cpu, int mask)
 {
     g_assert(bql_locked());
-- 
2.47.1

Re: [PATCH 0/3] linux-user: Call tcg_flush_jmp_cache() before re-using threads

[Philippe Mathieu-Daudé]

Hi Ilya,

ping? :)

On 2/1/25 19:25, Philippe Mathieu-Daudé wrote:
> Fix a bug reported by Ilya in:
> https://lore.kernel.org/qemu-devel/uuuk6a2vo24yrrqrchjxaeko3utqshrdu6txcnqziokpg7dkom@4l4kd3dqh6jc/
> 
> Commit bb6cf6f0168 ("accel/tcg: Factor tcg_cpu_reset_hold()
> out") wanted to restrict tlb_flush() to system emulation,
> but inadvertently also restricted tcg_flush_jmp_cache(),
> which was before called on user emulation via:
> 
>    Realize -> Reset -> cpu_common_reset_hold()
> 
> Since threads (vCPUs) use a common CPUJumpCache, when many
> threads are created / joined, they eventually end re-using
> a CPUJumpCache entry, which was cleared when the first vCPU
> was allocated (via Realize) but then stayed dirty.
> 
> Have cpu_exec_reset_hold() call the common tcg_exec_reset()
> helper on user emulation, eventually calling tcg_flush_jmp_cache().
> 
> Philippe Mathieu-Daudé (3):
>    linux-user: Only include 'exec/tb-flush.h' header when necessary
>    accel/tcg: Factor out common tcg_exec_reset() helper
>    accel/tcg: Implement cpu_exec_reset_hold() on user emulation
> 
>   accel/tcg/internal-common.h | 1 +
>   linux-user/user-internals.h | 1 -
>   accel/tcg/cpu-exec-common.c | 6 ++++++
>   accel/tcg/tcg-accel-ops.c   | 4 ++--
>   accel/tcg/user-exec-stub.c  | 4 ----
>   accel/tcg/user-exec.c       | 5 +++++
>   linux-user/mmap.c           | 1 +
>   linux-user/syscall.c        | 1 +
>   8 files changed, 16 insertions(+), 7 deletions(-)
> 

Re: [PATCH 1/3] linux-user: Only include 'exec/tb-flush.h' header when necessary

[Ilya Leoshkevich]

On Thu, 2025-01-02 at 19:25 +0100, Philippe Mathieu-Daudé wrote:
> Very few source files require to access "exec/tb-flush.h"
> declarations, and except a pair, they all include it
> explicitly. No need to overload the generic "user-internals.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  linux-user/user-internals.h | 1 -
>  linux-user/mmap.c           | 1 +
>  linux-user/syscall.c        | 1 +
>  3 files changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>

Re: [PATCH 2/3] accel/tcg: Factor out common tcg_exec_reset() helper

[Ilya Leoshkevich]

On Thu, 2025-01-02 at 19:25 +0100, Philippe Mathieu-Daudé wrote:
> Since tcg_cpu_reset_hold() is a system emulation specific
> helper, factor tcg_exec_reset() out so we can use it from
> user emulation, similarly to the [un]realize() handlers.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  accel/tcg/internal-common.h | 1 +
>  accel/tcg/cpu-exec-common.c | 6 ++++++
>  accel/tcg/tcg-accel-ops.c   | 4 ++--
>  3 files changed, 9 insertions(+), 2 deletions(-)

Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>

Re: [PATCH 3/3] accel/tcg: Implement cpu_exec_reset_hold() on user emulation

[Ilya Leoshkevich]

On Thu, 2025-01-02 at 19:25 +0100, Philippe Mathieu-Daudé wrote:
> Commit bb6cf6f0168 ("accel/tcg: Factor tcg_cpu_reset_hold()
> out") wanted to restrict tlb_flush() to system emulation,
> but inadvertently also restricted tcg_flush_jmp_cache(),
> which was before called on user emulation via:
> 
>   Realize -> Reset -> cpu_common_reset_hold()
> 
> Since threads (vCPUs) use a common CPUJumpCache, when many
> threads are created / joined, they eventually end re-using
> a CPUJumpCache entry, which was cleared when the first vCPU
> was allocated (via Realize) but then stayed dirty, leading to:

How are jump caches shared between qemu-user vCPUs?
I found the following, but this looks private and zeroed out
during initialization:

bool tcg_exec_realizefn(CPUState *cpu, Error **errp)
[...]
    cpu->tb_jmp_cache = g_new0(CPUJumpCache, 1);

I was also wondering whether vCPUs themselves may be recycled, but
it doesn't seem to be the case, since do_fork() -> cpu_copy() ->
cpu_create() -> object_new() -> object_new_with_type() calls
g_malloc().



Btw, I tried to reproduce the original issue, but bumped into something
seemingly unrelated. To make matters worse, debugging seems to be
broken, so it may take some time before I can properly test this
change.

Thread 2 received signal SIGSEGV, Segmentation fault.
[Switching to Thread 37607.37622]
0x000002aa00a6a64c in cs_option (ud=140251083477344,
type=CS_OPT_SYNTAX, value=2) at capstone/cs.c:782
782             return arch_configs[handle->arch].arch_option(handle,
type, value);
(gdb) info threads
Ignoring packet error, continuing...

Re: [PATCH 3/3] accel/tcg: Implement cpu_exec_reset_hold() on user emulation

[Ilya Leoshkevich]

On Fri, 2025-01-10 at 00:43 +0100, Ilya Leoshkevich wrote:
> On Thu, 2025-01-02 at 19:25 +0100, Philippe Mathieu-Daudé wrote:
> > Commit bb6cf6f0168 ("accel/tcg: Factor tcg_cpu_reset_hold()
> > out") wanted to restrict tlb_flush() to system emulation,
> > but inadvertently also restricted tcg_flush_jmp_cache(),
> > which was before called on user emulation via:
> > 
> >   Realize -> Reset -> cpu_common_reset_hold()
> > 
> > Since threads (vCPUs) use a common CPUJumpCache, when many
> > threads are created / joined, they eventually end re-using
> > a CPUJumpCache entry, which was cleared when the first vCPU
> > was allocated (via Realize) but then stayed dirty, leading to:
> 
> How are jump caches shared between qemu-user vCPUs?
> I found the following, but this looks private and zeroed out
> during initialization:
> 
> bool tcg_exec_realizefn(CPUState *cpu, Error **errp)
> [...]
>     cpu->tb_jmp_cache = g_new0(CPUJumpCache, 1);
> 
> I was also wondering whether vCPUs themselves may be recycled, but
> it doesn't seem to be the case, since do_fork() -> cpu_copy() ->
> cpu_create() -> object_new() -> object_new_with_type() calls
> g_malloc().
> 
> 
> 
> Btw, I tried to reproduce the original issue, but bumped into
> something
> seemingly unrelated. To make matters worse, debugging seems to be
> broken, so it may take some time before I can properly test this
> change.
> 
> Thread 2 received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 37607.37622]
> 0x000002aa00a6a64c in cs_option (ud=140251083477344,
> type=CS_OPT_SYNTAX, value=2) at capstone/cs.c:782
> 782             return arch_configs[handle->arch].arch_option(handle,
> type, value);
> (gdb) info threads
> Ignoring packet error, continuing...

With a small debugger fix [1] I finally managed to investigate and fix
the crash, which turned out to be not caused by QEMU [2], and with that
the testsuite ran without further issues. So I don't seem to be able to
reproduce the original issue to verify this series.

[1]
https://lore.kernel.org/qemu-devel/20250113134658.68376-1-iii@linux.ibm.com/
[2] https://github.com/capstone-rust/capstone-rs/pull/166

Re: [PATCH 1/3] linux-user: Only include 'exec/tb-flush.h' header when necessary

[Pierrick Bouvier]

On 1/2/25 10:25, Philippe Mathieu-Daudé wrote:
> Very few source files require to access "exec/tb-flush.h"
> declarations, and except a pair, they all include it
> explicitly. No need to overload the generic "user-internals.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   linux-user/user-internals.h | 1 -
>   linux-user/mmap.c           | 1 +
>   linux-user/syscall.c        | 1 +
>   3 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/user-internals.h b/linux-user/user-internals.h
> index b9b05c1d11f..4aa253b5663 100644
> --- a/linux-user/user-internals.h
> +++ b/linux-user/user-internals.h
> @@ -20,7 +20,6 @@
>   
>   #include "user/thunk.h"
>   #include "exec/exec-all.h"
> -#include "exec/tb-flush.h"
>   #include "qemu/log.h"
>   
>   extern char *exec_path;
> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> index 6828b17a63f..d1f36e6f16b 100644
> --- a/linux-user/mmap.c
> +++ b/linux-user/mmap.c
> @@ -21,6 +21,7 @@
>   #include "trace.h"
>   #include "exec/log.h"
>   #include "exec/page-protection.h"
> +#include "exec/tb-flush.h"
>   #include "exec/translation-block.h"
>   #include "qemu.h"
>   #include "user/page-protection.h"
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 78c7c0b34ef..cbbfcf10d28 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -26,6 +26,7 @@
>   #include "tcg/startup.h"
>   #include "target_mman.h"
>   #include "exec/page-protection.h"
> +#include "exec/tb-flush.h"
>   #include "exec/translation-block.h"
>   #include <elf.h>
>   #include <endian.h>

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 2/3] accel/tcg: Factor out common tcg_exec_reset() helper

[Pierrick Bouvier]

On 1/2/25 10:25, Philippe Mathieu-Daudé wrote:
> Since tcg_cpu_reset_hold() is a system emulation specific
> helper, factor tcg_exec_reset() out so we can use it from
> user emulation, similarly to the [un]realize() handlers.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   accel/tcg/internal-common.h | 1 +
>   accel/tcg/cpu-exec-common.c | 6 ++++++
>   accel/tcg/tcg-accel-ops.c   | 4 ++--
>   3 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/accel/tcg/internal-common.h b/accel/tcg/internal-common.h
> index c8d714256cb..8b474fc1256 100644
> --- a/accel/tcg/internal-common.h
> +++ b/accel/tcg/internal-common.h
> @@ -55,6 +55,7 @@ void cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb,
>   
>   bool tcg_exec_realizefn(CPUState *cpu, Error **errp);
>   void tcg_exec_unrealizefn(CPUState *cpu);
> +void tcg_exec_reset(CPUState *cpu);
>   
>   /* current cflags for hashing/comparison */
>   uint32_t curr_cflags(CPUState *cpu);
> diff --git a/accel/tcg/cpu-exec-common.c b/accel/tcg/cpu-exec-common.c
> index 6ecfc4e7c21..72ab9c3d977 100644
> --- a/accel/tcg/cpu-exec-common.c
> +++ b/accel/tcg/cpu-exec-common.c
> @@ -21,6 +21,7 @@
>   #include "system/cpus.h"
>   #include "system/tcg.h"
>   #include "qemu/plugin.h"
> +#include "exec/tb-flush.h"
>   #include "internal-common.h"
>   
>   bool tcg_allowed;
> @@ -56,3 +57,8 @@ void cpu_loop_exit_atomic(CPUState *cpu, uintptr_t pc)
>       cpu->exception_index = EXCP_ATOMIC;
>       cpu_loop_exit_restore(cpu, pc);
>   }
> +
> +void tcg_exec_reset(CPUState *cpu)
> +{
> +    tcg_flush_jmp_cache(cpu);
> +}
> diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
> index 6e3f1fa92b2..4fe6821b017 100644
> --- a/accel/tcg/tcg-accel-ops.c
> +++ b/accel/tcg/tcg-accel-ops.c
> @@ -34,7 +34,6 @@
>   #include "qemu/timer.h"
>   #include "exec/exec-all.h"
>   #include "exec/hwaddr.h"
> -#include "exec/tb-flush.h"
>   #include "exec/translation-block.h"
>   #include "gdbstub/enums.h"
>   
> @@ -44,6 +43,7 @@
>   #include "tcg-accel-ops-mttcg.h"
>   #include "tcg-accel-ops-rr.h"
>   #include "tcg-accel-ops-icount.h"
> +#include "internal-common.h"
>   
>   /* common functionality among all TCG variants */
>   
> @@ -83,7 +83,7 @@ int tcg_cpu_exec(CPUState *cpu)
>   
>   static void tcg_cpu_reset_hold(CPUState *cpu)
>   {
> -    tcg_flush_jmp_cache(cpu);
> +    tcg_exec_reset(cpu);
>   
>       tlb_flush(cpu);
>   }

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 3/3] accel/tcg: Implement cpu_exec_reset_hold() on user emulation

[Pierrick Bouvier]

On 1/2/25 10:25, Philippe Mathieu-Daudé wrote:
> Commit bb6cf6f0168 ("accel/tcg: Factor tcg_cpu_reset_hold()
> out") wanted to restrict tlb_flush() to system emulation,
> but inadvertently also restricted tcg_flush_jmp_cache(),
> which was before called on user emulation via:
> 
>    Realize -> Reset -> cpu_common_reset_hold()
> 
> Since threads (vCPUs) use a common CPUJumpCache, when many
> threads are created / joined, they eventually end re-using
> a CPUJumpCache entry, which was cleared when the first vCPU
> was allocated (via Realize) but then stayed dirty, leading to:
> 
>    Thread 1 "qemu-s390x" received signal SIGABRT, Aborted.
>    __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
>    44      ./nptl/pthread_kill.c: No such file or directory.
>    (gdb) bt
>    #0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
>    #1  0x00007ffff7c41e8f in __pthread_kill_internal (signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:78
>    #2  0x00007ffff7bf2fb2 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
>    #3  0x00007ffff7bdd472 in __GI_abort () at ./stdlib/abort.c:79
>    #4  0x00007ffff7bdd395 in __assert_fail_base (fmt=0x7ffff7d51a90 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=assertion@entry=0x5555556d71b8 "cpu->accel",
>        file=file@entry=0x5555556d70e0 "cpu-target.c", line=line@entry=158, function=function@entry=0x5555556d7260 <__PRETTY_FUNCTION__.3> "cpu_exec_realizefn") at ./assert/assert.c:92
>    #5  0x00007ffff7bebeb2 in __GI___assert_fail (assertion=assertion@entry=0x5555556d71b8 "cpu->accel", file=file@entry=0x5555556d70e0 "cpu-target.c", line=line@entry=158,
>        function=function@entry=0x5555556d7260 <__PRETTY_FUNCTION__.3> "cpu_exec_realizefn") at ./assert/assert.c:101
>    #6  0x00005555555d44ca in cpu_exec_realizefn (cpu=cpu@entry=0x5555557c28c0, errp=errp@entry=0x7fffffffe140) at cpu-target.c:158
>    #7  0x000055555559f50b in s390_cpu_realizefn (dev=0x5555557c28c0, errp=0x7fffffffe1a0) at target/s390x/cpu.c:261
>    #8  0x000055555563f78b in device_set_realized (obj=<optimized out>, value=<optimized out>, errp=0x7fffffffe2e0) at hw/core/qdev.c:510
>    #9  0x000055555564365d in property_set_bool (obj=0x5555557c28c0, v=<optimized out>, name=<optimized out>, opaque=0x5555557a9140, errp=0x7fffffffe2e0) at qom/object.c:2362
>    #10 0x0000555555646bbb in object_property_set (obj=obj@entry=0x5555557c28c0, name=name@entry=0x5555556e8ae2 "realized", v=v@entry=0x5555557c6650, errp=errp@entry=0x7fffffffe2e0)
>        at qom/object.c:1471
>    #11 0x000055555564a45f in object_property_set_qobject (obj=obj@entry=0x5555557c28c0, name=name@entry=0x5555556e8ae2 "realized", value=value@entry=0x5555557a7a90, errp=errp@entry=0x7fffffffe2e0)
>        at qom/qom-qobject.c:28
>    #12 0x0000555555647224 in object_property_set_bool (obj=0x5555557c28c0, name=name@entry=0x5555556e8ae2 "realized", value=value@entry=true, errp=errp@entry=0x7fffffffe2e0)
>        at qom/object.c:1541
>    #13 0x000055555564027c in qdev_realize (dev=<optimized out>, bus=bus@entry=0x0, errp=errp@entry=0x7fffffffe2e0) at hw/core/qdev.c:291
>    #14 0x000055555559bb54 in cpu_create (typename=<optimized out>) at hw/core/cpu-common.c:57
>    #15 0x000055555559a467 in main (argc=4, argv=0x7fffffffeaa8, envp=<optimized out>) at linux-user/main.c:811
> 
> Have cpu_exec_reset_hold() call the common tcg_exec_reset()
> helper on user emulation, eventually calling tcg_flush_jmp_cache().
> 
> Fixes: bb6cf6f0168 ("accel/tcg: Factor tcg_cpu_reset_hold() out")
> Reported-by: Ilya Leoshkevich <iii@linux.ibm.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   accel/tcg/user-exec-stub.c | 4 ----
>   accel/tcg/user-exec.c      | 5 +++++
>   2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/accel/tcg/user-exec-stub.c b/accel/tcg/user-exec-stub.c
> index 4fbe2dbdc88..2dc6fd9c4e8 100644
> --- a/accel/tcg/user-exec-stub.c
> +++ b/accel/tcg/user-exec-stub.c
> @@ -14,10 +14,6 @@ void qemu_init_vcpu(CPUState *cpu)
>   {
>   }
>   
> -void cpu_exec_reset_hold(CPUState *cpu)
> -{
> -}
> -
>   /* User mode emulation does not support record/replay yet.  */
>   
>   bool replay_exception(void)
> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
> index 0561c4f6dc7..92640f07ed7 100644
> --- a/accel/tcg/user-exec.c
> +++ b/accel/tcg/user-exec.c
> @@ -40,6 +40,11 @@ __thread uintptr_t helper_retaddr;
>   
>   //#define DEBUG_SIGNAL
>   
> +void cpu_exec_reset_hold(CPUState *cpu)
> +{
> +    tcg_exec_reset(cpu);
> +}
> +
>   void cpu_interrupt(CPUState *cpu, int mask)
>   {
>       g_assert(bql_locked());

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>