| Message ID | 20240503123456.28866-6-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | accel/tcg: Call tcg_flush_jmp_cache() again when creating user-mode cpu | expand |
Hi, Am 03.05.24 um 14:34 schrieb Philippe Mathieu-Daudé: > In commit bb6cf6f016 ("accel/tcg: Factor tcg_cpu_reset_hold() out") > we unfortunately restricted the tcg_flush_jmp_cache() to system > emulation. Move it to the common tcg_exec_cpu_reset_hold() handler > so user emulation gets the jmp_cache initialized when threads > are created. > > Remove the NULL check in tcg_flush_jmp_cache() from commit 4e4fa6c12d > ("accel/tcg: Complete cpu initialization before registration") which > was a band-aid fix for incorrect commit bb6cf6f016. > AFAICT, commit 4e4fa6c12d was already present in v7.2.0, while commit bb6cf6f016 only later in v8.2.0. So is it really fine to remove the NULL check? Best Regards, Fiona
03.05.2024 15:34, Philippe Mathieu-Daudé wrote: > In commit bb6cf6f016 ("accel/tcg: Factor tcg_cpu_reset_hold() out") > we unfortunately restricted the tcg_flush_jmp_cache() to system > emulation. Move it to the common tcg_exec_cpu_reset_hold() handler > so user emulation gets the jmp_cache initialized when threads > are created. > > Remove the NULL check in tcg_flush_jmp_cache() from commit 4e4fa6c12d > ("accel/tcg: Complete cpu initialization before registration") which > was a band-aid fix for incorrect commit bb6cf6f016. > > Cc: qemu-stable@nongnu.org > Fixes: bb6cf6f016 ("accel/tcg: Factor tcg_cpu_reset_hold() out") > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Hi! Has this change been forgotten, or is it not appropriate anymore? Thanks, /mjt
Hi Michael, On 11/8/24 19:43, Michael Tokarev wrote: > 03.05.2024 15:34, Philippe Mathieu-Daudé wrote: >> In commit bb6cf6f016 ("accel/tcg: Factor tcg_cpu_reset_hold() out") >> we unfortunately restricted the tcg_flush_jmp_cache() to system >> emulation. Move it to the common tcg_exec_cpu_reset_hold() handler >> so user emulation gets the jmp_cache initialized when threads >> are created. >> >> Remove the NULL check in tcg_flush_jmp_cache() from commit 4e4fa6c12d >> ("accel/tcg: Complete cpu initialization before registration") which >> was a band-aid fix for incorrect commit bb6cf6f016. >> >> Cc: qemu-stable@nongnu.org >> Fixes: bb6cf6f016 ("accel/tcg: Factor tcg_cpu_reset_hold() out") >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > Hi! > > Has this change been forgotten, or is it not appropriate anymore? Not forgotten and still need to be fixed, however unfortunately this exposed a bug in user-mode SYS_exit_group when using plugins (see qemu_plugin_disable_mem_helpers call in qemu_plugin_user_exit). Pierrick is working on it, and I'll rebase this series once his work gets merged. Next release :/ Regards, Phil.
13.08.2024 18:10, Philippe Mathieu-Daudé wrote: >> Has this change been forgotten, or is it not appropriate anymore? > > Not forgotten and still need to be fixed, however unfortunately > this exposed a bug in user-mode SYS_exit_group when using plugins > (see qemu_plugin_disable_mem_helpers call in qemu_plugin_user_exit). > > Pierrick is working on it, and I'll rebase this series once his > work gets merged. Next release :/ Hm.. Do we have an idea *which* next release it will be? ;)) Thanks, /mjt
On 20/12/24 22:08, Michael Tokarev wrote: > 13.08.2024 18:10, Philippe Mathieu-Daudé wrote: > >>> Has this change been forgotten, or is it not appropriate anymore? >> >> Not forgotten and still need to be fixed, however unfortunately >> this exposed a bug in user-mode SYS_exit_group when using plugins >> (see qemu_plugin_disable_mem_helpers call in qemu_plugin_user_exit). >> >> Pierrick is working on it, and I'll rebase this series once his >> work gets merged. Next release :/ > > Hm.. Do we have an idea *which* next release it will be? ;)) Hopefully 10.0, otherwise 10.1.
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c index 1bf85c324d..7e04df2902 100644 --- a/accel/tcg/cpu-exec.c +++ b/accel/tcg/cpu-exec.c @@ -36,6 +36,7 @@ #include "exec/replay-core.h" #include "sysemu/tcg.h" #include "exec/helper-proto-common.h" +#include "exec/tb-flush.h" #include "tb-jmp-cache.h" #include "tb-hash.h" #include "tb-context.h" @@ -1099,4 +1100,5 @@ void tcg_exec_unrealizefn(CPUState *cpu) void tcg_exec_cpu_reset_hold(CPUState *cpu) { + tcg_flush_jmp_cache(cpu); } diff --git a/accel/tcg/sysemu/tcg-accel-ops.c b/accel/tcg/sysemu/tcg-accel-ops.c index 82c8368f87..13e450c088 100644 --- a/accel/tcg/sysemu/tcg-accel-ops.c +++ b/accel/tcg/sysemu/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/gdbstub.h" #include "../internal-common.h" #include "hw/core/cpu.h" @@ -83,7 +82,6 @@ int tcg_cpu_exec(CPUState *cpu) static void tcg_cpu_reset_hold(CPUState *cpu) { tcg_exec_cpu_reset_hold(cpu); - tcg_flush_jmp_cache(cpu); tlb_flush(cpu); } diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index 83cc14fbde..93202fa3c1 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -663,11 +663,6 @@ void tcg_flush_jmp_cache(CPUState *cpu) { CPUJumpCache *jc = cpu->tb_jmp_cache; - /* During early initialization, the cache may not yet be allocated. */ - if (unlikely(jc == NULL)) { - return; - } - for (int i = 0; i < TB_JMP_CACHE_SIZE; i++) { qatomic_set(&jc->array[i].tb, NULL); }
In commit bb6cf6f016 ("accel/tcg: Factor tcg_cpu_reset_hold() out") we unfortunately restricted the tcg_flush_jmp_cache() to system emulation. Move it to the common tcg_exec_cpu_reset_hold() handler so user emulation gets the jmp_cache initialized when threads are created. Remove the NULL check in tcg_flush_jmp_cache() from commit 4e4fa6c12d ("accel/tcg: Complete cpu initialization before registration") which was a band-aid fix for incorrect commit bb6cf6f016. Cc: qemu-stable@nongnu.org Fixes: bb6cf6f016 ("accel/tcg: Factor tcg_cpu_reset_hold() out") Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- accel/tcg/cpu-exec.c | 2 ++ accel/tcg/sysemu/tcg-accel-ops.c | 2 -- accel/tcg/translate-all.c | 5 ----- 3 files changed, 2 insertions(+), 7 deletions(-)