qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
To: "Philippe Mathieu-Daudé" <philmd@linaro.org>, qemu-devel@nongnu.org
Cc: "Ilya Leoshkevich" <iii@linux.ibm.com>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Riku Voipio" <riku.voipio@iki.fi>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Laurent Vivier" <laurent@vivier.eu>
Subject: Re: [PATCH 3/3] accel/tcg: Implement cpu_exec_reset_hold() on user emulation
Date: Tue, 21 Jan 2025 16:46:42 -0800	[thread overview]
Message-ID: <c08a859c-be9a-42cd-b261-44981b6a5b4c@linaro.org> (raw)
In-Reply-To: <20250102182521.65428-4-philmd@linaro.org>

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>


  parent reply	other threads:[~2025-01-22  0:47 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-02 18:25 [PATCH 0/3] linux-user: Call tcg_flush_jmp_cache() before re-using threads Philippe Mathieu-Daudé
2025-01-02 18:25 ` [PATCH 1/3] linux-user: Only include 'exec/tb-flush.h' header when necessary Philippe Mathieu-Daudé
2025-01-09 22:13   ` Ilya Leoshkevich
2025-01-22  0:41   ` Pierrick Bouvier
2025-01-02 18:25 ` [PATCH 2/3] accel/tcg: Factor out common tcg_exec_reset() helper Philippe Mathieu-Daudé
2025-01-09 22:16   ` Ilya Leoshkevich
2025-01-22  0:41   ` Pierrick Bouvier
2025-01-02 18:25 ` [PATCH 3/3] accel/tcg: Implement cpu_exec_reset_hold() on user emulation Philippe Mathieu-Daudé
2025-01-09 23:43   ` Ilya Leoshkevich
2025-01-14 20:52     ` Ilya Leoshkevich
2025-01-22  0:46   ` Pierrick Bouvier [this message]
2025-01-09 10:59 ` [PATCH 0/3] linux-user: Call tcg_flush_jmp_cache() before re-using threads Philippe Mathieu-Daudé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c08a859c-be9a-42cd-b261-44981b6a5b4c@linaro.org \
    --to=pierrick.bouvier@linaro.org \
    --cc=alex.bennee@linaro.org \
    --cc=iii@linux.ibm.com \
    --cc=laurent@vivier.eu \
    --cc=pbonzini@redhat.com \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=riku.voipio@iki.fi \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).