[PATCH v2 0/4] accel/tcg: Call tcg_flush_jmp_cache() again when creating user-mode cpu

[PATCH v2 0/4] accel/tcg: Call tcg_flush_jmp_cache() again when creating user-mode cpu

[Philippe Mathieu-Daudé]

Since v1:
- Dropped patch #1 (use meson subdir_done)

Fix a bug introduced during a refactor in commit bb6cf6f016
("accel/tcg: Factor tcg_cpu_reset_hold() out"), affecting
user emulation. (cpu_reset is called during thread creation,
after cpu_realize).

This issue was noticed by Ilya when running WASM:
https://lore.kernel.org/qemu-devel/20221027141856.w5umjgklawgu7pqv@heavy/

Philippe Mathieu-Daudé (4):
  accel/tcg: Move system emulation files under sysemu/ subdirectory
  accel/tcg: Do not define cpu_exec_reset_hold() as stub
  accel/tcg: Introduce common tcg_exec_cpu_reset_hold() method
  accel/tcg: Always call tcg_flush_jmp_cache() on reset

 accel/tcg/internal-common.h                   |  2 ++
 accel/tcg/{ => sysemu}/tcg-accel-ops-icount.h |  0
 accel/tcg/{ => sysemu}/tcg-accel-ops-mttcg.h  |  0
 accel/tcg/{ => sysemu}/tcg-accel-ops-rr.h     |  0
 accel/tcg/{ => sysemu}/tcg-accel-ops.h        |  0
 accel/tcg/cpu-exec.c                          |  6 ++++++
 accel/tcg/{ => sysemu}/icount-common.c        |  0
 accel/tcg/{ => sysemu}/monitor.c              |  4 ++--
 accel/tcg/{ => sysemu}/tcg-accel-ops-icount.c |  0
 accel/tcg/{ => sysemu}/tcg-accel-ops-mttcg.c  |  0
 accel/tcg/{ => sysemu}/tcg-accel-ops-rr.c     |  0
 accel/tcg/{ => sysemu}/tcg-accel-ops.c        |  5 ++---
 accel/tcg/translate-all.c                     |  5 -----
 accel/tcg/user-exec-stub.c                    |  4 ----
 accel/tcg/user-exec.c                         |  5 +++++
 accel/tcg/meson.build                         | 14 ++------------
 accel/tcg/sysemu/meson.build                  | 11 +++++++++++
 17 files changed, 30 insertions(+), 26 deletions(-)
 rename accel/tcg/{ => sysemu}/tcg-accel-ops-icount.h (100%)
 rename accel/tcg/{ => sysemu}/tcg-accel-ops-mttcg.h (100%)
 rename accel/tcg/{ => sysemu}/tcg-accel-ops-rr.h (100%)
 rename accel/tcg/{ => sysemu}/tcg-accel-ops.h (100%)
 rename accel/tcg/{ => sysemu}/icount-common.c (100%)
 rename accel/tcg/{ => sysemu}/monitor.c (99%)
 rename accel/tcg/{ => sysemu}/tcg-accel-ops-icount.c (100%)
 rename accel/tcg/{ => sysemu}/tcg-accel-ops-mttcg.c (100%)
 rename accel/tcg/{ => sysemu}/tcg-accel-ops-rr.c (100%)
 rename accel/tcg/{ => sysemu}/tcg-accel-ops.c (99%)
 create mode 100644 accel/tcg/sysemu/meson.build

-- 
2.41.0

[PATCH v2 1/4] accel/tcg: Move system emulation files under sysemu/ subdirectory

[Philippe Mathieu-Daudé]

Some files are specific to system emulation. Move them under
their own sysemu/ directory. This might help to notice what
is affected (user, system or both) when doing global refactors.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/tcg/{ => sysemu}/tcg-accel-ops-icount.h |  0
 accel/tcg/{ => sysemu}/tcg-accel-ops-mttcg.h  |  0
 accel/tcg/{ => sysemu}/tcg-accel-ops-rr.h     |  0
 accel/tcg/{ => sysemu}/tcg-accel-ops.h        |  0
 accel/tcg/{ => sysemu}/icount-common.c        |  0
 accel/tcg/{ => sysemu}/monitor.c              |  4 ++--
 accel/tcg/{ => sysemu}/tcg-accel-ops-icount.c |  0
 accel/tcg/{ => sysemu}/tcg-accel-ops-mttcg.c  |  0
 accel/tcg/{ => sysemu}/tcg-accel-ops-rr.c     |  0
 accel/tcg/{ => sysemu}/tcg-accel-ops.c        |  0
 accel/tcg/meson.build                         | 14 ++------------
 accel/tcg/sysemu/meson.build                  | 11 +++++++++++
 12 files changed, 15 insertions(+), 14 deletions(-)
 rename accel/tcg/{ => sysemu}/tcg-accel-ops-icount.h (100%)
 rename accel/tcg/{ => sysemu}/tcg-accel-ops-mttcg.h (100%)
 rename accel/tcg/{ => sysemu}/tcg-accel-ops-rr.h (100%)
 rename accel/tcg/{ => sysemu}/tcg-accel-ops.h (100%)
 rename accel/tcg/{ => sysemu}/icount-common.c (100%)
 rename accel/tcg/{ => sysemu}/monitor.c (99%)
 rename accel/tcg/{ => sysemu}/tcg-accel-ops-icount.c (100%)
 rename accel/tcg/{ => sysemu}/tcg-accel-ops-mttcg.c (100%)
 rename accel/tcg/{ => sysemu}/tcg-accel-ops-rr.c (100%)
 rename accel/tcg/{ => sysemu}/tcg-accel-ops.c (100%)
 create mode 100644 accel/tcg/sysemu/meson.build

diff --git a/accel/tcg/tcg-accel-ops-icount.h b/accel/tcg/sysemu/tcg-accel-ops-icount.h
similarity index 100%
rename from accel/tcg/tcg-accel-ops-icount.h
rename to accel/tcg/sysemu/tcg-accel-ops-icount.h
diff --git a/accel/tcg/tcg-accel-ops-mttcg.h b/accel/tcg/sysemu/tcg-accel-ops-mttcg.h
similarity index 100%
rename from accel/tcg/tcg-accel-ops-mttcg.h
rename to accel/tcg/sysemu/tcg-accel-ops-mttcg.h
diff --git a/accel/tcg/tcg-accel-ops-rr.h b/accel/tcg/sysemu/tcg-accel-ops-rr.h
similarity index 100%
rename from accel/tcg/tcg-accel-ops-rr.h
rename to accel/tcg/sysemu/tcg-accel-ops-rr.h
diff --git a/accel/tcg/tcg-accel-ops.h b/accel/tcg/sysemu/tcg-accel-ops.h
similarity index 100%
rename from accel/tcg/tcg-accel-ops.h
rename to accel/tcg/sysemu/tcg-accel-ops.h
diff --git a/accel/tcg/icount-common.c b/accel/tcg/sysemu/icount-common.c
similarity index 100%
rename from accel/tcg/icount-common.c
rename to accel/tcg/sysemu/icount-common.c
diff --git a/accel/tcg/monitor.c b/accel/tcg/sysemu/monitor.c
similarity index 99%
rename from accel/tcg/monitor.c
rename to accel/tcg/sysemu/monitor.c
index 093efe9714..620c18d267 100644
--- a/accel/tcg/monitor.c
+++ b/accel/tcg/sysemu/monitor.c
@@ -17,8 +17,8 @@
 #include "sysemu/cpu-timers.h"
 #include "sysemu/tcg.h"
 #include "tcg/tcg.h"
-#include "internal-common.h"
-#include "tb-context.h"
+#include "../internal-common.h"
+#include "../tb-context.h"
 
 
 static void dump_drift_info(GString *buf)
diff --git a/accel/tcg/tcg-accel-ops-icount.c b/accel/tcg/sysemu/tcg-accel-ops-icount.c
similarity index 100%
rename from accel/tcg/tcg-accel-ops-icount.c
rename to accel/tcg/sysemu/tcg-accel-ops-icount.c
diff --git a/accel/tcg/tcg-accel-ops-mttcg.c b/accel/tcg/sysemu/tcg-accel-ops-mttcg.c
similarity index 100%
rename from accel/tcg/tcg-accel-ops-mttcg.c
rename to accel/tcg/sysemu/tcg-accel-ops-mttcg.c
diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/sysemu/tcg-accel-ops-rr.c
similarity index 100%
rename from accel/tcg/tcg-accel-ops-rr.c
rename to accel/tcg/sysemu/tcg-accel-ops-rr.c
diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/sysemu/tcg-accel-ops.c
similarity index 100%
rename from accel/tcg/tcg-accel-ops.c
rename to accel/tcg/sysemu/tcg-accel-ops.c
diff --git a/accel/tcg/meson.build b/accel/tcg/meson.build
index aef80de967..1b91f26a7f 100644
--- a/accel/tcg/meson.build
+++ b/accel/tcg/meson.build
@@ -18,19 +18,9 @@ if get_option('plugins')
 endif
 specific_ss.add_all(when: 'CONFIG_TCG', if_true: tcg_specific_ss)
 
-specific_ss.add(when: ['CONFIG_SYSTEM_ONLY', 'CONFIG_TCG'], if_true: files(
+specific_ss.add(when: ['CONFIG_SYSTEM_ONLY', 'CONFIG_TCG'], if_true: files( # SoftMMU
   'cputlb.c',
   'watchpoint.c',
 ))
 
-system_ss.add(when: ['CONFIG_TCG'], if_true: files(
-  'icount-common.c',
-  'monitor.c',
-))
-
-tcg_module_ss.add(when: ['CONFIG_SYSTEM_ONLY', 'CONFIG_TCG'], if_true: files(
-  'tcg-accel-ops.c',
-  'tcg-accel-ops-mttcg.c',
-  'tcg-accel-ops-icount.c',
-  'tcg-accel-ops-rr.c',
-))
+subdir('sysemu')
diff --git a/accel/tcg/sysemu/meson.build b/accel/tcg/sysemu/meson.build
new file mode 100644
index 0000000000..d0b4939ba9
--- /dev/null
+++ b/accel/tcg/sysemu/meson.build
@@ -0,0 +1,11 @@
+system_ss.add(files(
+  'icount-common.c',
+  'monitor.c',
+))
+
+tcg_module_ss.add(when: ['CONFIG_SYSTEM_ONLY'], if_true: files(
+  'tcg-accel-ops.c',
+  'tcg-accel-ops-mttcg.c',
+  'tcg-accel-ops-icount.c',
+  'tcg-accel-ops-rr.c',
+))
-- 
2.41.0

[PATCH v2 2/4] accel/tcg: Do not define cpu_exec_reset_hold() as stub

[Philippe Mathieu-Daudé]

In commit 1b5120d74b ("accel: Introduce cpu_exec_reset_hold()")
we defined the user emulation backend cpu_exec_reset_hold() as
a stub. This was a mistake, since common fields are set in
CPU reset, and user emulation needs these fields to be resetted.
Move cpu_exec_reset_hold() back.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/tcg/user-exec-stub.c | 4 ----
 accel/tcg/user-exec.c      | 4 ++++
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/accel/tcg/user-exec-stub.c b/accel/tcg/user-exec-stub.c
index 4fbe2dbdc8..2dc6fd9c4e 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 1c621477ad..6a108eb212 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -37,6 +37,10 @@ __thread uintptr_t helper_retaddr;
 
 //#define DEBUG_SIGNAL
 
+void cpu_exec_reset_hold(CPUState *cpu)
+{
+}
+
 /*
  * Adjust the pc to pass to cpu_restore_state; return the memop type.
  */
-- 
2.41.0

[PATCH v2 3/4] accel/tcg: Introduce common tcg_exec_cpu_reset_hold() method

[Philippe Mathieu-Daudé]

Introduce a method called by cpu_reset(), common
to both system and user emulation.

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

diff --git a/accel/tcg/internal-common.h b/accel/tcg/internal-common.h
index edefd0dcb7..b85285e2ff 100644
--- a/accel/tcg/internal-common.h
+++ b/accel/tcg/internal-common.h
@@ -23,4 +23,6 @@ static inline bool cpu_in_serial_context(CPUState *cs)
     return !(cs->tcg_cflags & CF_PARALLEL) || cpu_in_exclusive_context(cs);
 }
 
+void tcg_exec_cpu_reset_hold(CPUState *cpu);
+
 #endif
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 225e5fbd3e..1bf85c324d 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -1096,3 +1096,7 @@ void tcg_exec_unrealizefn(CPUState *cpu)
     tlb_destroy(cpu);
     g_free_rcu(cpu->tb_jmp_cache, rcu);
 }
+
+void tcg_exec_cpu_reset_hold(CPUState *cpu)
+{
+}
diff --git a/accel/tcg/sysemu/tcg-accel-ops.c b/accel/tcg/sysemu/tcg-accel-ops.c
index 2c7b0cc09e..82c8368f87 100644
--- a/accel/tcg/sysemu/tcg-accel-ops.c
+++ b/accel/tcg/sysemu/tcg-accel-ops.c
@@ -36,7 +36,7 @@
 #include "exec/hwaddr.h"
 #include "exec/tb-flush.h"
 #include "exec/gdbstub.h"
-
+#include "../internal-common.h"
 #include "hw/core/cpu.h"
 
 #include "tcg-accel-ops.h"
@@ -82,6 +82,7 @@ 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/user-exec.c b/accel/tcg/user-exec.c
index 6a108eb212..85e92ec8f2 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -39,6 +39,7 @@ __thread uintptr_t helper_retaddr;
 
 void cpu_exec_reset_hold(CPUState *cpu)
 {
+    tcg_exec_cpu_reset_hold(cpu);
 }
 
 /*
-- 
2.41.0

[PATCH v2 4/4] accel/tcg: Always call tcg_flush_jmp_cache() on reset

[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.

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(-)

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);
     }
-- 
2.41.0

Re: [PATCH v2 1/4] accel/tcg: Move system emulation files under sysemu/ subdirectory

[Philippe Mathieu-Daudé]

On 3/5/24 14:25, Philippe Mathieu-Daudé wrote:
> Some files are specific to system emulation. Move them under
> their own sysemu/ directory. This might help to notice what
> is affected (user, system or both) when doing global refactors.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   accel/tcg/{ => sysemu}/tcg-accel-ops-icount.h |  0
>   accel/tcg/{ => sysemu}/tcg-accel-ops-mttcg.h  |  0
>   accel/tcg/{ => sysemu}/tcg-accel-ops-rr.h     |  0
>   accel/tcg/{ => sysemu}/tcg-accel-ops.h        |  0
>   accel/tcg/{ => sysemu}/icount-common.c        |  0
>   accel/tcg/{ => sysemu}/monitor.c              |  4 ++--
>   accel/tcg/{ => sysemu}/tcg-accel-ops-icount.c |  0
>   accel/tcg/{ => sysemu}/tcg-accel-ops-mttcg.c  |  0
>   accel/tcg/{ => sysemu}/tcg-accel-ops-rr.c     |  0
>   accel/tcg/{ => sysemu}/tcg-accel-ops.c        |  0
>   accel/tcg/meson.build                         | 14 ++------------
>   accel/tcg/sysemu/meson.build                  | 11 +++++++++++
>   12 files changed, 15 insertions(+), 14 deletions(-)
>   rename accel/tcg/{ => sysemu}/tcg-accel-ops-icount.h (100%)
>   rename accel/tcg/{ => sysemu}/tcg-accel-ops-mttcg.h (100%)
>   rename accel/tcg/{ => sysemu}/tcg-accel-ops-rr.h (100%)
>   rename accel/tcg/{ => sysemu}/tcg-accel-ops.h (100%)
>   rename accel/tcg/{ => sysemu}/icount-common.c (100%)
>   rename accel/tcg/{ => sysemu}/monitor.c (99%)
>   rename accel/tcg/{ => sysemu}/tcg-accel-ops-icount.c (100%)
>   rename accel/tcg/{ => sysemu}/tcg-accel-ops-mttcg.c (100%)
>   rename accel/tcg/{ => sysemu}/tcg-accel-ops-rr.c (100%)
>   rename accel/tcg/{ => sysemu}/tcg-accel-ops.c (100%)
>   create mode 100644 accel/tcg/sysemu/meson.build


> diff --git a/accel/tcg/meson.build b/accel/tcg/meson.build
> index aef80de967..1b91f26a7f 100644
> --- a/accel/tcg/meson.build
> +++ b/accel/tcg/meson.build
> @@ -18,19 +18,9 @@ if get_option('plugins')
>   endif
>   specific_ss.add_all(when: 'CONFIG_TCG', if_true: tcg_specific_ss)
>   
> -specific_ss.add(when: ['CONFIG_SYSTEM_ONLY', 'CONFIG_TCG'], if_true: files(
> +specific_ss.add(when: ['CONFIG_SYSTEM_ONLY', 'CONFIG_TCG'], if_true: files( # SoftMMU
>     'cputlb.c',
>     'watchpoint.c',
>   ))
>   
> -system_ss.add(when: ['CONFIG_TCG'], if_true: files(
> -  'icount-common.c',
> -  'monitor.c',
> -))
> -
> -tcg_module_ss.add(when: ['CONFIG_SYSTEM_ONLY', 'CONFIG_TCG'], if_true: files(
> -  'tcg-accel-ops.c',
> -  'tcg-accel-ops-mttcg.c',
> -  'tcg-accel-ops-icount.c',
> -  'tcg-accel-ops-rr.c',
> -))
> +subdir('sysemu')
> diff --git a/accel/tcg/sysemu/meson.build b/accel/tcg/sysemu/meson.build
> new file mode 100644
> index 0000000000..d0b4939ba9
> --- /dev/null
> +++ b/accel/tcg/sysemu/meson.build
> @@ -0,0 +1,11 @@
> +system_ss.add(files(

Oops, broken rebase, sorry...

> +  'icount-common.c',
> +  'monitor.c',
> +))
> +
> +tcg_module_ss.add(when: ['CONFIG_SYSTEM_ONLY'], if_true: files(
> +  'tcg-accel-ops.c',
> +  'tcg-accel-ops-mttcg.c',
> +  'tcg-accel-ops-icount.c',
> +  'tcg-accel-ops-rr.c',
> +))