qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2)
@ 2024-04-30 12:27 Philippe Mathieu-Daudé
  2024-04-30 12:27 ` [PATCH v3 01/13] accel/tcg: Restrict qemu_plugin_vcpu_exit_hook() to TCG plugins Philippe Mathieu-Daudé
                   ` (13 more replies)
  0 siblings, 14 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-30 12:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Anton Johansson, Ilya Leoshkevich, Richard Henderson,
	Philippe =?unknown-8bit?q?Mathieu-Daud=C3=A9?=

Missing WASM testing by Ilya (branch available at
https://gitlab.com/philmd/qemu/-/commits/tcg_flush_jmp_cache)

Since v2:
- Move cpu_loop_exit_requested() to "exec/cpu-loop.h"
- Added R-b tags

Since v1:
- First 13 patches queued
- Restrict qemu_plugin_vcpu_exit_hook() to (TCG) plugins
- Restrict cpu_plugin_mem_cbs_enabled() to TCG (plugins)
- Addressed Richard review comments on the others:
  - Move cpu_plugin_mem_cbs_enabled()
  - Do not move mem_io_pc, waiting for [*]
  - Mention can_do_io restricted

Finish extracting TCG fields from CPUState:
- Extract tcg_cpu_exit() from cpu_exit()
- Introduce AccelOpsClass::exit_vcpu_thread()
- cpu_exit() calls exit_vcpu_thread=tcg_cpu_exit for TCG
- Forward declare TaskState and more uses of get_task_state()
- Introduce TCG AccelCPUState
- Move TCG specific fields from CPUState to AccelCPUState
- Restrict "exec/tlb-common.h" to TCG
- Restrict iommu_notifiers, icount to system emulation

[*] https://lore.kernel.org/qemu-devel/20240416040609.1313605-3-richard.henderson@linaro.org/

Based-on: https://gitlab.com/philmd/qemu/-/commits/accel-next

Philippe Mathieu-Daudé (13):
  accel/tcg: Restrict qemu_plugin_vcpu_exit_hook() to TCG plugins
  accel/tcg: Restrict cpu_plugin_mem_cbs_enabled() to TCG
  accel/tcg: Move @plugin_mem_cbs from CPUState to
    CPUNegativeOffsetState
  accel/tcg: Move @plugin_state from CPUState to TCG AccelCPUState
  accel/tcg: Restrict cpu_loop_exit_requested() to TCG
  accel/tcg: Restrict IcountDecr / can_do_io / CPUTLB to TCG
  accel/tcg: Move @jmp_env from CPUState to TCG AccelCPUState
  accel/tcg: Move @cflags_next_tb from CPUState to TCG AccelCPUState
  accel/tcg: Move @iommu_notifiers from CPUState to TCG AccelCPUState
  accel/tcg: Move @tcg_cflags from CPUState to TCG AccelCPUState
  accel/tcg: Restrict icount to system emulation
  accel/tcg: Move icount fields from CPUState to TCG AccelCPUState
  accel/tcg: Move @tb_jmp_cache from CPUState to TCG AccelCPUState

 accel/tcg/internal-common.h      | 18 ++++++++++
 accel/tcg/tb-jmp-cache.h         |  4 +--
 accel/tcg/tcg-accel-ops.h        |  1 +
 accel/tcg/vcpu-state.h           | 20 +++++++++++
 include/exec/cpu-loop.h          | 35 +++++++++++++++++++
 include/exec/exec-all.h          | 17 ----------
 include/exec/tlb-common.h        |  4 +++
 include/hw/core/cpu.h            | 58 ++++++++------------------------
 include/qemu/plugin.h            |  2 +-
 include/qemu/typedefs.h          |  1 -
 accel/tcg/cpu-exec-common.c      |  2 +-
 accel/tcg/cpu-exec.c             | 52 +++++++++++++++-------------
 accel/tcg/cputlb.c               |  2 +-
 accel/tcg/icount-common.c        |  7 ++--
 accel/tcg/plugin-gen.c           |  9 +++--
 accel/tcg/tb-maint.c             |  6 ++--
 accel/tcg/tcg-accel-ops-icount.c | 14 ++++----
 accel/tcg/tcg-accel-ops.c        |  2 ++
 accel/tcg/translate-all.c        |  9 ++---
 accel/tcg/watchpoint.c           |  5 +--
 hw/core/cpu-common.c             |  9 +++--
 linux-user/main.c                |  2 +-
 plugins/core.c                   |  9 ++---
 system/physmem.c                 | 37 +++++++++++++++-----
 target/arm/tcg/helper-a64.c      |  1 +
 target/s390x/tcg/mem_helper.c    |  1 +
 26 files changed, 195 insertions(+), 132 deletions(-)
 create mode 100644 include/exec/cpu-loop.h

-- 
2.41.0



^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v3 01/13] accel/tcg: Restrict qemu_plugin_vcpu_exit_hook() to TCG plugins
  2024-04-30 12:27 [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2) Philippe Mathieu-Daudé
@ 2024-04-30 12:27 ` Philippe Mathieu-Daudé
  2024-04-30 12:27 ` [PATCH v3 02/13] accel/tcg: Restrict cpu_plugin_mem_cbs_enabled() to TCG Philippe Mathieu-Daudé
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-30 12:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Anton Johansson, Ilya Leoshkevich, Richard Henderson,
	Philippe Mathieu-Daudé

qemu_plugin_vcpu_exit_hook() is specific to TCG plugins,
so must be restricted to it in cpu_common_unrealizefn(),
similarly to how qemu_plugin_create_vcpu_state() is
restricted in the cpu_common_realizefn() counterpart.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20240429213050.55177-2-philmd@linaro.org>
---
 hw/core/cpu-common.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index cbafc79033..f2826d0409 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -30,7 +30,9 @@
 #include "hw/boards.h"
 #include "hw/qdev-properties.h"
 #include "trace.h"
+#ifdef CONFIG_PLUGIN
 #include "qemu/plugin.h"
+#endif
 
 CPUState *cpu_by_arch_id(int64_t id)
 {
@@ -226,9 +228,11 @@ static void cpu_common_unrealizefn(DeviceState *dev)
     CPUState *cpu = CPU(dev);
 
     /* Call the plugin hook before clearing the cpu is fully unrealized */
+#ifdef CONFIG_PLUGIN
     if (tcg_enabled()) {
         qemu_plugin_vcpu_exit_hook(cpu);
     }
+#endif
 
     /* NOTE: latest generic point before the cpu is fully unrealized */
     cpu_exec_unrealizefn(cpu);
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 02/13] accel/tcg: Restrict cpu_plugin_mem_cbs_enabled() to TCG
  2024-04-30 12:27 [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2) Philippe Mathieu-Daudé
  2024-04-30 12:27 ` [PATCH v3 01/13] accel/tcg: Restrict qemu_plugin_vcpu_exit_hook() to TCG plugins Philippe Mathieu-Daudé
@ 2024-04-30 12:27 ` Philippe Mathieu-Daudé
  2024-04-30 12:27 ` [PATCH v3 03/13] accel/tcg: Move @plugin_mem_cbs from CPUState to CPUNegativeOffsetState Philippe Mathieu-Daudé
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-30 12:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Anton Johansson, Ilya Leoshkevich, Richard Henderson,
	Philippe Mathieu-Daudé

So far cpu_plugin_mem_cbs_enabled() is only called from
TCG, so reduce it to accel/tcg/.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <5f59c754-44e5-4743-a2dd-87ef8e13eadf@linaro.org>
---
 accel/tcg/internal-common.h | 17 +++++++++++++++++
 include/hw/core/cpu.h       | 17 -----------------
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/accel/tcg/internal-common.h b/accel/tcg/internal-common.h
index df317e7496..5061687900 100644
--- a/accel/tcg/internal-common.h
+++ b/accel/tcg/internal-common.h
@@ -24,6 +24,23 @@ static inline bool cpu_in_serial_context(CPUState *cs)
     return !tcg_cflags_has(cs, CF_PARALLEL) || cpu_in_exclusive_context(cs);
 }
 
+/**
+ * cpu_plugin_mem_cbs_enabled() - are plugin memory callbacks enabled?
+ * @cs: CPUState pointer
+ *
+ * The memory callbacks are installed if a plugin has instrumented an
+ * instruction for memory. This can be useful to know if you want to
+ * force a slow path for a series of memory accesses.
+ */
+static inline bool cpu_plugin_mem_cbs_enabled(const CPUState *cpu)
+{
+#ifdef CONFIG_PLUGIN
+    return !!cpu->plugin_mem_cbs;
+#else
+    return false;
+#endif
+}
+
 void tcg_cpu_exit(CPUState *cpu);
 
 #endif
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index beb37342e9..55555be618 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -1109,23 +1109,6 @@ void cpu_watchpoint_remove_by_ref(CPUState *cpu, CPUWatchpoint *watchpoint);
 void cpu_watchpoint_remove_all(CPUState *cpu, int mask);
 #endif
 
-/**
- * cpu_plugin_mem_cbs_enabled() - are plugin memory callbacks enabled?
- * @cs: CPUState pointer
- *
- * The memory callbacks are installed if a plugin has instrumented an
- * instruction for memory. This can be useful to know if you want to
- * force a slow path for a series of memory accesses.
- */
-static inline bool cpu_plugin_mem_cbs_enabled(const CPUState *cpu)
-{
-#ifdef CONFIG_PLUGIN
-    return !!cpu->plugin_mem_cbs;
-#else
-    return false;
-#endif
-}
-
 /**
  * cpu_get_address_space:
  * @cpu: CPU to get address space from
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 03/13] accel/tcg: Move @plugin_mem_cbs from CPUState to CPUNegativeOffsetState
  2024-04-30 12:27 [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2) Philippe Mathieu-Daudé
  2024-04-30 12:27 ` [PATCH v3 01/13] accel/tcg: Restrict qemu_plugin_vcpu_exit_hook() to TCG plugins Philippe Mathieu-Daudé
  2024-04-30 12:27 ` [PATCH v3 02/13] accel/tcg: Restrict cpu_plugin_mem_cbs_enabled() to TCG Philippe Mathieu-Daudé
@ 2024-04-30 12:27 ` Philippe Mathieu-Daudé
  2024-04-30 12:27 ` [PATCH v3 04/13] accel/tcg: Move @plugin_state from CPUState to TCG AccelCPUState Philippe Mathieu-Daudé
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-30 12:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Anton Johansson, Ilya Leoshkevich, Richard Henderson,
	Philippe Mathieu-Daudé

@plugin_mem_cbs is accessed by tcg generated code, move it
to CPUNegativeOffsetState.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20240429213050.55177-4-philmd@linaro.org>
---
 accel/tcg/internal-common.h |  2 +-
 include/hw/core/cpu.h       | 13 +++++++------
 include/qemu/plugin.h       |  2 +-
 accel/tcg/plugin-gen.c      |  5 +++--
 plugins/core.c              |  2 +-
 5 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/accel/tcg/internal-common.h b/accel/tcg/internal-common.h
index 5061687900..867426500f 100644
--- a/accel/tcg/internal-common.h
+++ b/accel/tcg/internal-common.h
@@ -35,7 +35,7 @@ static inline bool cpu_in_serial_context(CPUState *cs)
 static inline bool cpu_plugin_mem_cbs_enabled(const CPUState *cpu)
 {
 #ifdef CONFIG_PLUGIN
-    return !!cpu->plugin_mem_cbs;
+    return !!cpu->neg.plugin_mem_cbs;
 #else
     return false;
 #endif
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 55555be618..571ef3e514 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -342,9 +342,16 @@ typedef union IcountDecr {
  * CPUNegativeOffsetState: Elements of CPUState most efficiently accessed
  *                         from CPUArchState, via small negative offsets.
  * @can_do_io: True if memory-mapped IO is allowed.
+ * @plugin_mem_cbs: active plugin memory callbacks
  */
 typedef struct CPUNegativeOffsetState {
     CPUTLB tlb;
+#ifdef CONFIG_PLUGIN
+    /*
+     * The callback pointer are accessed via TCG (see gen_empty_mem_helper).
+     */
+    GArray *plugin_mem_cbs;
+#endif
     IcountDecr icount_decr;
     bool can_do_io;
 } CPUNegativeOffsetState;
@@ -416,7 +423,6 @@ struct qemu_work_item;
  * @kvm_fd: vCPU file descriptor for KVM.
  * @work_mutex: Lock to prevent multiple access to @work_list.
  * @work_list: List of pending asynchronous work.
- * @plugin_mem_cbs: active plugin memory callbacks
  * @plugin_state: per-CPU plugin state
  * @ignore_memory_transaction_failures: Cached copy of the MachineState
  *    flag of the same name: allows the board to suppress calling of the
@@ -509,11 +515,6 @@ struct CPUState {
     QemuLockCnt in_ioctl_lock;
 
 #ifdef CONFIG_PLUGIN
-    /*
-     * The callback pointer stays in the main CPUState as it is
-     * accessed via TCG (see gen_empty_mem_helper).
-     */
-    GArray *plugin_mem_cbs;
     CPUPluginState *plugin_state;
 #endif
 
diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index 41db748eda..99a32446e9 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -229,7 +229,7 @@ void qemu_plugin_add_dyn_cb_arr(GArray *arr);
 
 static inline void qemu_plugin_disable_mem_helpers(CPUState *cpu)
 {
-    cpu->plugin_mem_cbs = NULL;
+    cpu->neg.plugin_mem_cbs = NULL;
 }
 
 /**
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index cd78ef94a1..fd268c79b5 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -178,7 +178,7 @@ static void gen_empty_mem_helper(void)
     TCGv_ptr ptr = tcg_temp_ebb_new_ptr();
 
     tcg_gen_movi_ptr(ptr, 0);
-    tcg_gen_st_ptr(ptr, tcg_env, offsetof(CPUState, plugin_mem_cbs) -
+    tcg_gen_st_ptr(ptr, tcg_env, offsetof(CPUState, neg.plugin_mem_cbs) -
                                  offsetof(ArchCPU, env));
     tcg_temp_free_ptr(ptr);
 }
@@ -634,7 +634,8 @@ void plugin_gen_disable_mem_helpers(void)
         return;
     }
     tcg_gen_st_ptr(tcg_constant_ptr(NULL), tcg_env,
-                   offsetof(CPUState, plugin_mem_cbs) - offsetof(ArchCPU, env));
+                   offsetof(CPUState, neg.plugin_mem_cbs) -
+                   offsetof(ArchCPU, env));
 }
 
 static void plugin_gen_tb_udata(const struct qemu_plugin_tb *ptb,
diff --git a/plugins/core.c b/plugins/core.c
index 09c98382f5..a097d02788 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -496,7 +496,7 @@ void exec_inline_op(struct qemu_plugin_dyn_cb *cb, int cpu_index)
 void qemu_plugin_vcpu_mem_cb(CPUState *cpu, uint64_t vaddr,
                              MemOpIdx oi, enum qemu_plugin_mem_rw rw)
 {
-    GArray *arr = cpu->plugin_mem_cbs;
+    GArray *arr = cpu->neg.plugin_mem_cbs;
     size_t i;
 
     if (arr == NULL) {
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 04/13] accel/tcg: Move @plugin_state from CPUState to TCG AccelCPUState
  2024-04-30 12:27 [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2) Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2024-04-30 12:27 ` [PATCH v3 03/13] accel/tcg: Move @plugin_mem_cbs from CPUState to CPUNegativeOffsetState Philippe Mathieu-Daudé
@ 2024-04-30 12:27 ` Philippe Mathieu-Daudé
  2024-04-30 12:27 ` [PATCH v3 05/13] accel/tcg: Restrict cpu_loop_exit_requested() to TCG Philippe Mathieu-Daudé
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-30 12:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Anton Johansson, Ilya Leoshkevich, Richard Henderson,
	Philippe Mathieu-Daudé

@plugin_state is specific to TCG accelerator, move it to
its AccelCPUState.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20240429213050.55177-5-philmd@linaro.org>
---
 accel/tcg/vcpu-state.h | 5 +++++
 include/hw/core/cpu.h  | 5 -----
 accel/tcg/plugin-gen.c | 4 +++-
 hw/core/cpu-common.c   | 3 ++-
 plugins/core.c         | 7 ++++---
 5 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/accel/tcg/vcpu-state.h b/accel/tcg/vcpu-state.h
index e30368edae..35c2695a77 100644
--- a/accel/tcg/vcpu-state.h
+++ b/accel/tcg/vcpu-state.h
@@ -10,11 +10,16 @@
 
 /**
  * AccelCPUState: vCPU fields specific to TCG accelerator
+ * @plugin_state: per-CPU plugin state
  */
 struct AccelCPUState {
 #ifdef CONFIG_USER_ONLY
     TaskState *ts;
 #endif /* !CONFIG_USER_ONLY */
+
+#ifdef CONFIG_PLUGIN
+    CPUPluginState *plugin_state;
+#endif /* CONFIG_PLUGIN */
 };
 
 #ifdef CONFIG_USER_ONLY
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 571ef3e514..91e793e590 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -423,7 +423,6 @@ struct qemu_work_item;
  * @kvm_fd: vCPU file descriptor for KVM.
  * @work_mutex: Lock to prevent multiple access to @work_list.
  * @work_list: List of pending asynchronous work.
- * @plugin_state: per-CPU plugin state
  * @ignore_memory_transaction_failures: Cached copy of the MachineState
  *    flag of the same name: allows the board to suppress calling of the
  *    CPU do_transaction_failed hook function.
@@ -514,10 +513,6 @@ struct CPUState {
     /* Use by accel-block: CPU is executing an ioctl() */
     QemuLockCnt in_ioctl_lock;
 
-#ifdef CONFIG_PLUGIN
-    CPUPluginState *plugin_state;
-#endif
-
     /* TODO Move common fields from CPUArchState here. */
     int cpu_index;
     int cluster_index;
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index fd268c79b5..88d720d549 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -52,6 +52,7 @@
 #include "exec/plugin-gen.h"
 #include "exec/translator.h"
 #include "exec/helper-proto-common.h"
+#include "accel/tcg/vcpu-state.h"
 
 #define HELPER_H  "accel/tcg/plugin-helpers.h"
 #include "exec/helper-info.c.inc"
@@ -872,7 +873,8 @@ bool plugin_gen_tb_start(CPUState *cpu, const DisasContextBase *db,
 {
     bool ret = false;
 
-    if (test_bit(QEMU_PLUGIN_EV_VCPU_TB_TRANS, cpu->plugin_state->event_mask)) {
+    if (test_bit(QEMU_PLUGIN_EV_VCPU_TB_TRANS,
+                 cpu->accel->plugin_state->event_mask)) {
         struct qemu_plugin_tb *ptb = tcg_ctx->plugin_tb;
         int i;
 
diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index f2826d0409..ea342213d6 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -31,6 +31,7 @@
 #include "hw/qdev-properties.h"
 #include "trace.h"
 #ifdef CONFIG_PLUGIN
+#include "accel/tcg/vcpu-state.h"
 #include "qemu/plugin.h"
 #endif
 
@@ -215,7 +216,7 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
     /* Plugin initialization must wait until the cpu start executing code */
 #ifdef CONFIG_PLUGIN
     if (tcg_enabled()) {
-        cpu->plugin_state = qemu_plugin_create_vcpu_state();
+        cpu->accel->plugin_state = qemu_plugin_create_vcpu_state();
         async_run_on_cpu(cpu, qemu_plugin_vcpu_init__async, RUN_ON_CPU_NULL);
     }
 #endif
diff --git a/plugins/core.c b/plugins/core.c
index a097d02788..722224e5d8 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -28,6 +28,7 @@
 #include "exec/tb-flush.h"
 #include "tcg/tcg.h"
 #include "tcg/tcg-op.h"
+#include "accel/tcg/vcpu-state.h"
 #include "plugin.h"
 
 struct qemu_plugin_cb {
@@ -55,7 +56,7 @@ struct qemu_plugin_ctx *plugin_id_to_ctx_locked(qemu_plugin_id_t id)
 
 static void plugin_cpu_update__async(CPUState *cpu, run_on_cpu_data data)
 {
-    bitmap_copy(cpu->plugin_state->event_mask,
+    bitmap_copy(cpu->accel->plugin_state->event_mask,
                 &data.host_ulong, QEMU_PLUGIN_EV_MAX);
     tcg_flush_jmp_cache(cpu);
 }
@@ -396,7 +397,7 @@ qemu_plugin_vcpu_syscall(CPUState *cpu, int64_t num, uint64_t a1, uint64_t a2,
     struct qemu_plugin_cb *cb, *next;
     enum qemu_plugin_event ev = QEMU_PLUGIN_EV_VCPU_SYSCALL;
 
-    if (!test_bit(ev, cpu->plugin_state->event_mask)) {
+    if (!test_bit(ev, cpu->accel->plugin_state->event_mask)) {
         return;
     }
 
@@ -418,7 +419,7 @@ void qemu_plugin_vcpu_syscall_ret(CPUState *cpu, int64_t num, int64_t ret)
     struct qemu_plugin_cb *cb, *next;
     enum qemu_plugin_event ev = QEMU_PLUGIN_EV_VCPU_SYSCALL_RET;
 
-    if (!test_bit(ev, cpu->plugin_state->event_mask)) {
+    if (!test_bit(ev, cpu->accel->plugin_state->event_mask)) {
         return;
     }
 
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 05/13] accel/tcg: Restrict cpu_loop_exit_requested() to TCG
  2024-04-30 12:27 [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2) Philippe Mathieu-Daudé
                   ` (3 preceding siblings ...)
  2024-04-30 12:27 ` [PATCH v3 04/13] accel/tcg: Move @plugin_state from CPUState to TCG AccelCPUState Philippe Mathieu-Daudé
@ 2024-04-30 12:27 ` Philippe Mathieu-Daudé
  2024-04-30 12:28 ` [PATCH v3 06/13] accel/tcg: Restrict IcountDecr / can_do_io / CPUTLB " Philippe Mathieu-Daudé
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-30 12:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Anton Johansson, Ilya Leoshkevich, Richard Henderson,
	Philippe Mathieu-Daudé

Next commit will restrict IcountDecr to TCG, so the
inlined cpu_loop_exit_requested(), which is specific
to TCG, won't compile when TCG is disabled. Move it
to the new "exec/cpu-loop.h" header.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20240428221450.26460-12-philmd@linaro.org>
---
 include/exec/cpu-loop.h       | 35 +++++++++++++++++++++++++++++++++++
 include/exec/exec-all.h       | 17 -----------------
 accel/tcg/cpu-exec.c          |  1 +
 target/arm/tcg/helper-a64.c   |  1 +
 target/s390x/tcg/mem_helper.c |  1 +
 5 files changed, 38 insertions(+), 17 deletions(-)
 create mode 100644 include/exec/cpu-loop.h

diff --git a/include/exec/cpu-loop.h b/include/exec/cpu-loop.h
new file mode 100644
index 0000000000..36ce4064fd
--- /dev/null
+++ b/include/exec/cpu-loop.h
@@ -0,0 +1,35 @@
+/*
+ *  Translation CPU loop (target agnostic)
+ *
+ *  Copyright (c) 2003 Fabrice Bellard
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+#ifndef TRANSLATION_CPU_LOOP_H
+#define TRANSLATION_CPU_LOOP_H
+
+#ifndef CONFIG_TCG
+#error Can only include this header with TCG
+#endif
+
+#include "qemu/atomic.h"
+#include "hw/core/cpu.h"
+
+/**
+ * cpu_loop_exit_requested:
+ * @cpu: The CPU state to be tested
+ *
+ * Indicate if somebody asked for a return of the CPU to the main loop
+ * (e.g., via cpu_exit() or cpu_interrupt()).
+ *
+ * This is helpful for architectures that support interruptible
+ * instructions. After writing back all state to registers/memory, this
+ * call can be used to check if it makes sense to return to the main loop
+ * or to continue executing the interruptible instruction.
+ */
+static inline bool cpu_loop_exit_requested(CPUState *cpu)
+{
+    return (int32_t)qatomic_read(&cpu->neg.icount_decr.u32) < 0;
+}
+
+#endif
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 2cd7b8f61b..544e35dd24 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -29,23 +29,6 @@
 #include "exec/translation-block.h"
 #include "qemu/clang-tsa.h"
 
-/**
- * cpu_loop_exit_requested:
- * @cpu: The CPU state to be tested
- *
- * Indicate if somebody asked for a return of the CPU to the main loop
- * (e.g., via cpu_exit() or cpu_interrupt()).
- *
- * This is helpful for architectures that support interruptible
- * instructions. After writing back all state to registers/memory, this
- * call can be used to check if it makes sense to return to the main loop
- * or to continue executing the interruptible instruction.
- */
-static inline bool cpu_loop_exit_requested(CPUState *cpu)
-{
-    return (int32_t)qatomic_read(&cpu->neg.icount_decr.u32) < 0;
-}
-
 #if !defined(CONFIG_USER_ONLY) && defined(CONFIG_TCG)
 /* cputlb.c */
 /**
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 9af66bc191..eedba056ba 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -32,6 +32,7 @@
 #include "qemu/main-loop.h"
 #include "sysemu/cpus.h"
 #include "exec/cpu-all.h"
+#include "exec/cpu-loop.h"
 #include "sysemu/cpu-timers.h"
 #include "exec/replay-core.h"
 #include "sysemu/tcg.h"
diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
index 0ea8668ab4..b294f6bfd0 100644
--- a/target/arm/tcg/helper-a64.c
+++ b/target/arm/tcg/helper-a64.c
@@ -29,6 +29,7 @@
 #include "internals.h"
 #include "qemu/crc32c.h"
 #include "exec/exec-all.h"
+#include "exec/cpu-loop.h"
 #include "exec/cpu_ldst.h"
 #include "qemu/int128.h"
 #include "qemu/atomic128.h"
diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
index 6a308c5553..8435825fa5 100644
--- a/target/s390x/tcg/mem_helper.c
+++ b/target/s390x/tcg/mem_helper.c
@@ -26,6 +26,7 @@
 #include "exec/helper-proto.h"
 #include "exec/exec-all.h"
 #include "exec/page-protection.h"
+#include "exec/cpu-loop.h"
 #include "exec/cpu_ldst.h"
 #include "hw/core/tcg-cpu-ops.h"
 #include "qemu/int128.h"
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 06/13] accel/tcg: Restrict IcountDecr / can_do_io / CPUTLB to TCG
  2024-04-30 12:27 [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2) Philippe Mathieu-Daudé
                   ` (4 preceding siblings ...)
  2024-04-30 12:27 ` [PATCH v3 05/13] accel/tcg: Restrict cpu_loop_exit_requested() to TCG Philippe Mathieu-Daudé
@ 2024-04-30 12:28 ` Philippe Mathieu-Daudé
  2024-04-30 12:28 ` [PATCH v3 07/13] accel/tcg: Move @jmp_env from CPUState to TCG AccelCPUState Philippe Mathieu-Daudé
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-30 12:28 UTC (permalink / raw)
  To: qemu-devel
  Cc: Anton Johansson, Ilya Leoshkevich, Richard Henderson,
	Philippe Mathieu-Daudé

IcountDecr union, the can_do_io field, the CPUTLB* structures
and the "exec/tlb-common.h" header are only required for TCG.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20240428221450.26460-16-philmd@linaro.org>
---
 include/exec/tlb-common.h | 4 ++++
 include/hw/core/cpu.h     | 9 ++++++---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/include/exec/tlb-common.h b/include/exec/tlb-common.h
index dc5a5faa0b..a529c9f056 100644
--- a/include/exec/tlb-common.h
+++ b/include/exec/tlb-common.h
@@ -19,6 +19,10 @@
 #ifndef EXEC_TLB_COMMON_H
 #define EXEC_TLB_COMMON_H 1
 
+#ifndef CONFIG_TCG
+#error Can only include this header with TCG
+#endif
+
 #define CPU_TLB_ENTRY_BITS 5
 
 /* Minimalized TLB entry for use by TCG fast path. */
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 91e793e590..47b499f9f1 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -27,7 +27,6 @@
 #include "exec/vaddr.h"
 #include "exec/memattrs.h"
 #include "exec/mmu-access-type.h"
-#include "exec/tlb-common.h"
 #include "qapi/qapi-types-run-state.h"
 #include "qemu/bitmap.h"
 #include "qemu/rcu_queue.h"
@@ -256,6 +255,9 @@ typedef struct CPUTLBEntryFull {
     } extra;
 } CPUTLBEntryFull;
 
+#ifdef CONFIG_TCG
+#include "exec/tlb-common.h"
+
 /*
  * Data elements that are per MMU mode, minus the bits accessed by
  * the TCG fast path.
@@ -311,11 +313,9 @@ typedef struct CPUTLBCommon {
  * negative offsets are at the end of the struct.
  */
 typedef struct CPUTLB {
-#ifdef CONFIG_TCG
     CPUTLBCommon c;
     CPUTLBDesc d[NB_MMU_MODES];
     CPUTLBDescFast f[NB_MMU_MODES];
-#endif
 } CPUTLB;
 
 /*
@@ -337,6 +337,7 @@ typedef union IcountDecr {
 #endif
     } u16;
 } IcountDecr;
+#endif
 
 /**
  * CPUNegativeOffsetState: Elements of CPUState most efficiently accessed
@@ -345,6 +346,7 @@ typedef union IcountDecr {
  * @plugin_mem_cbs: active plugin memory callbacks
  */
 typedef struct CPUNegativeOffsetState {
+#ifdef CONFIG_TCG
     CPUTLB tlb;
 #ifdef CONFIG_PLUGIN
     /*
@@ -354,6 +356,7 @@ typedef struct CPUNegativeOffsetState {
 #endif
     IcountDecr icount_decr;
     bool can_do_io;
+#endif
 } CPUNegativeOffsetState;
 
 struct KVMState;
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 07/13] accel/tcg: Move @jmp_env from CPUState to TCG AccelCPUState
  2024-04-30 12:27 [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2) Philippe Mathieu-Daudé
                   ` (5 preceding siblings ...)
  2024-04-30 12:28 ` [PATCH v3 06/13] accel/tcg: Restrict IcountDecr / can_do_io / CPUTLB " Philippe Mathieu-Daudé
@ 2024-04-30 12:28 ` Philippe Mathieu-Daudé
  2024-04-30 12:28 ` [PATCH v3 08/13] accel/tcg: Move @cflags_next_tb " Philippe Mathieu-Daudé
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-30 12:28 UTC (permalink / raw)
  To: qemu-devel
  Cc: Anton Johansson, Ilya Leoshkevich, Richard Henderson,
	Philippe Mathieu-Daudé

@jmp_env is specific to TCG accelerator, move it to its AccelCPUState.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20240428221450.26460-17-philmd@linaro.org>
---
 accel/tcg/internal-common.h | 1 +
 accel/tcg/tcg-accel-ops.h   | 1 +
 accel/tcg/vcpu-state.h      | 2 ++
 include/hw/core/cpu.h       | 1 -
 accel/tcg/cpu-exec-common.c | 2 +-
 accel/tcg/cpu-exec.c        | 6 +++---
 6 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/accel/tcg/internal-common.h b/accel/tcg/internal-common.h
index 867426500f..cb507053f5 100644
--- a/accel/tcg/internal-common.h
+++ b/accel/tcg/internal-common.h
@@ -11,6 +11,7 @@
 
 #include "exec/cpu-common.h"
 #include "exec/translation-block.h"
+#include "accel/tcg/vcpu-state.h"
 
 extern int64_t max_delay;
 extern int64_t max_advance;
diff --git a/accel/tcg/tcg-accel-ops.h b/accel/tcg/tcg-accel-ops.h
index 44c4079972..ed41a087a3 100644
--- a/accel/tcg/tcg-accel-ops.h
+++ b/accel/tcg/tcg-accel-ops.h
@@ -13,6 +13,7 @@
 #define TCG_ACCEL_OPS_H
 
 #include "sysemu/cpus.h"
+#include "accel/tcg/vcpu-state.h"
 
 void tcg_cpu_destroy(CPUState *cpu);
 int tcg_cpu_exec(CPUState *cpu);
diff --git a/accel/tcg/vcpu-state.h b/accel/tcg/vcpu-state.h
index 35c2695a77..3a0ea2d47a 100644
--- a/accel/tcg/vcpu-state.h
+++ b/accel/tcg/vcpu-state.h
@@ -13,6 +13,8 @@
  * @plugin_state: per-CPU plugin state
  */
 struct AccelCPUState {
+    sigjmp_buf jmp_env;
+
 #ifdef CONFIG_USER_ONLY
     TaskState *ts;
 #endif /* !CONFIG_USER_ONLY */
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 47b499f9f1..f1fe43dbea 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -475,7 +475,6 @@ struct CPUState {
     int64_t icount_budget;
     int64_t icount_extra;
     uint64_t random_seed;
-    sigjmp_buf jmp_env;
 
     QemuMutex work_mutex;
     QSIMPLEQ_HEAD(, qemu_work_item) work_list;
diff --git a/accel/tcg/cpu-exec-common.c b/accel/tcg/cpu-exec-common.c
index bc9b1a260e..ec45482305 100644
--- a/accel/tcg/cpu-exec-common.c
+++ b/accel/tcg/cpu-exec-common.c
@@ -38,7 +38,7 @@ void cpu_loop_exit(CPUState *cpu)
     cpu->neg.can_do_io = true;
     /* Undo any setting in generated code.  */
     qemu_plugin_disable_mem_helpers(cpu);
-    siglongjmp(cpu->jmp_env, 1);
+    siglongjmp(cpu->accel->jmp_env, 1);
 }
 
 void cpu_loop_exit_restore(CPUState *cpu, uintptr_t pc)
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index eedba056ba..443b688c01 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -554,7 +554,7 @@ static void cpu_exec_longjmp_cleanup(CPUState *cpu)
      * support such a thing.  We'd have to properly register unwind info
      * for the JIT for EH, rather that just for GDB.
      *
-     * Alternative 2: Set and restore cpu->jmp_env in tb_gen_code to
+     * Alternative 2: Set and restore cpu->accel->jmp_env in tb_gen_code to
      * capture the cpu_loop_exit longjmp, perform the cleanup, and
      * jump again to arrive here.
      */
@@ -578,7 +578,7 @@ void cpu_exec_step_atomic(CPUState *cpu)
     uint32_t flags, cflags;
     int tb_exit;
 
-    if (sigsetjmp(cpu->jmp_env, 0) == 0) {
+    if (sigsetjmp(cpu->accel->jmp_env, 0) == 0) {
         start_exclusive();
         g_assert(cpu == current_cpu);
         g_assert(!cpu->running);
@@ -1039,7 +1039,7 @@ cpu_exec_loop(CPUState *cpu, SyncClocks *sc)
 static int cpu_exec_setjmp(CPUState *cpu, SyncClocks *sc)
 {
     /* Prepare setjmp context for exception handling. */
-    if (unlikely(sigsetjmp(cpu->jmp_env, 0) != 0)) {
+    if (unlikely(sigsetjmp(cpu->accel->jmp_env, 0) != 0)) {
         cpu_exec_longjmp_cleanup(cpu);
     }
 
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 08/13] accel/tcg: Move @cflags_next_tb from CPUState to TCG AccelCPUState
  2024-04-30 12:27 [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2) Philippe Mathieu-Daudé
                   ` (6 preceding siblings ...)
  2024-04-30 12:28 ` [PATCH v3 07/13] accel/tcg: Move @jmp_env from CPUState to TCG AccelCPUState Philippe Mathieu-Daudé
@ 2024-04-30 12:28 ` Philippe Mathieu-Daudé
  2024-04-30 12:28 ` [PATCH v3 09/13] accel/tcg: Move @iommu_notifiers " Philippe Mathieu-Daudé
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-30 12:28 UTC (permalink / raw)
  To: qemu-devel
  Cc: Anton Johansson, Ilya Leoshkevich, Richard Henderson,
	Philippe Mathieu-Daudé

@cflags_next_tb is specific to TCG accelerator, move it to
its AccelCPUState.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20240428221450.26460-19-philmd@linaro.org>
---
 accel/tcg/vcpu-state.h    |  2 ++
 include/hw/core/cpu.h     |  1 -
 accel/tcg/cpu-exec.c      | 12 ++++++------
 accel/tcg/tb-maint.c      |  4 ++--
 accel/tcg/tcg-accel-ops.c |  1 +
 accel/tcg/translate-all.c |  2 +-
 accel/tcg/watchpoint.c    |  5 +++--
 hw/core/cpu-common.c      |  1 -
 8 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/accel/tcg/vcpu-state.h b/accel/tcg/vcpu-state.h
index 3a0ea2d47a..5b09279140 100644
--- a/accel/tcg/vcpu-state.h
+++ b/accel/tcg/vcpu-state.h
@@ -13,6 +13,8 @@
  * @plugin_state: per-CPU plugin state
  */
 struct AccelCPUState {
+    uint32_t cflags_next_tb;
+
     sigjmp_buf jmp_env;
 
 #ifdef CONFIG_USER_ONLY
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index f1fe43dbea..97a0baf874 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -468,7 +468,6 @@ struct CPUState {
     bool crash_occurred;
     bool exit_request;
     int exclusive_context_count;
-    uint32_t cflags_next_tb;
     /* updates protected by BQL */
     uint32_t interrupt_request;
     int singlestep_enabled;
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 443b688c01..2edfad78b3 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -721,7 +721,7 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
         if (replay_has_exception()
             && cpu->neg.icount_decr.u16.low + cpu->icount_extra == 0) {
             /* Execute just one insn to trigger exception pending in the log */
-            cpu->cflags_next_tb = (curr_cflags(cpu) & ~CF_USE_ICOUNT)
+            cpu->accel->cflags_next_tb = (curr_cflags(cpu) & ~CF_USE_ICOUNT)
                 | CF_NOIRQ | 1;
         }
 #endif
@@ -784,7 +784,7 @@ static inline bool icount_exit_request(CPUState *cpu)
     if (!icount_enabled()) {
         return false;
     }
-    if (cpu->cflags_next_tb != -1 && !(cpu->cflags_next_tb & CF_USE_ICOUNT)) {
+    if (!(cpu->accel->cflags_next_tb == -1 || cpu->accel->cflags_next_tb & CF_USE_ICOUNT)) {
         return false;
     }
     return cpu->neg.icount_decr.u16.low + cpu->icount_extra == 0;
@@ -798,7 +798,7 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
      * skip checking here. Any pending interrupts will get picked up
      * by the next TB we execute under normal cflags.
      */
-    if (cpu->cflags_next_tb != -1 && cpu->cflags_next_tb & CF_NOIRQ) {
+    if (cpu->accel->cflags_next_tb != -1 && cpu->accel->cflags_next_tb & CF_NOIRQ) {
         return false;
     }
 
@@ -948,7 +948,7 @@ static inline void cpu_loop_exec_tb(CPUState *cpu, TranslationBlock *tb,
     if (insns_left > 0 && insns_left < tb->icount)  {
         assert(insns_left <= CF_COUNT_MASK);
         assert(cpu->icount_extra == 0);
-        cpu->cflags_next_tb = (tb->cflags & ~CF_COUNT_MASK) | insns_left;
+        cpu->accel->cflags_next_tb = (tb->cflags & ~CF_COUNT_MASK) | insns_left;
     }
 #endif
 }
@@ -980,11 +980,11 @@ cpu_exec_loop(CPUState *cpu, SyncClocks *sc)
              * have CF_INVALID set, -1 is a convenient invalid value that
              * does not require tcg headers for cpu_common_reset.
              */
-            cflags = cpu->cflags_next_tb;
+            cflags = cpu->accel->cflags_next_tb;
             if (cflags == -1) {
                 cflags = curr_cflags(cpu);
             } else {
-                cpu->cflags_next_tb = -1;
+                cpu->accel->cflags_next_tb = -1;
             }
 
             if (check_for_breakpoints(cpu, pc, &cflags)) {
diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
index 19ae6793f3..2d5faca9fd 100644
--- a/accel/tcg/tb-maint.c
+++ b/accel/tcg/tb-maint.c
@@ -1084,7 +1084,7 @@ bool tb_invalidate_phys_page_unwind(tb_page_addr_t addr, uintptr_t pc)
     if (current_tb_modified) {
         /* Force execution of one insn next time.  */
         CPUState *cpu = current_cpu;
-        cpu->cflags_next_tb = 1 | CF_NOIRQ | curr_cflags(current_cpu);
+        cpu->accel->cflags_next_tb = 1 | CF_NOIRQ | curr_cflags(current_cpu);
         return true;
     }
     return false;
@@ -1154,7 +1154,7 @@ tb_invalidate_phys_page_range__locked(struct page_collection *pages,
     if (current_tb_modified) {
         page_collection_unlock(pages);
         /* Force execution of one insn next time.  */
-        current_cpu->cflags_next_tb = 1 | CF_NOIRQ | curr_cflags(current_cpu);
+        current_cpu->accel->cflags_next_tb = 1 | CF_NOIRQ | curr_cflags(current_cpu);
         mmap_unlock();
         cpu_loop_exit_noexc(current_cpu);
     }
diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
index 56bbad9fcd..d9132a5835 100644
--- a/accel/tcg/tcg-accel-ops.c
+++ b/accel/tcg/tcg-accel-ops.c
@@ -89,6 +89,7 @@ static void tcg_cpu_reset_hold(CPUState *cpu)
 
     qatomic_set(&cpu->neg.icount_decr.u32, 0);
     cpu->neg.can_do_io = true;
+    cpu->accel->cflags_next_tb = -1;
 }
 
 /* mask must never be zero, except for A20 change call */
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index b67adce20e..3a8199a761 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -631,7 +631,7 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr)
      * operations only (which execute after completion) so we don't
      * double instrument the instruction.
      */
-    cpu->cflags_next_tb = curr_cflags(cpu) | CF_MEMI_ONLY | n;
+    cpu->accel->cflags_next_tb = curr_cflags(cpu) | CF_MEMI_ONLY | n;
 
     if (qemu_loglevel_mask(CPU_LOG_EXEC)) {
         vaddr pc = cpu->cc->get_pc(cpu);
diff --git a/accel/tcg/watchpoint.c b/accel/tcg/watchpoint.c
index d3aab11458..0a40bfdc85 100644
--- a/accel/tcg/watchpoint.c
+++ b/accel/tcg/watchpoint.c
@@ -26,6 +26,7 @@
 #include "sysemu/replay.h"
 #include "hw/core/tcg-cpu-ops.h"
 #include "hw/core/cpu.h"
+#include "accel/tcg/vcpu-state.h"
 
 /*
  * Return true if this watchpoint address matches the specified
@@ -100,7 +101,7 @@ void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
                  */
                 if (!cpu->neg.can_do_io) {
                     /* Force execution of one insn next time.  */
-                    cpu->cflags_next_tb = 1 | CF_NOIRQ | curr_cflags(cpu);
+                    cpu->accel->cflags_next_tb = 1 | CF_NOIRQ | curr_cflags(cpu);
                     cpu_loop_exit_restore(cpu, ra);
                 }
                 /*
@@ -132,7 +133,7 @@ void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
                 cpu_loop_exit(cpu);
             } else {
                 /* Force execution of one insn next time.  */
-                cpu->cflags_next_tb = 1 | CF_NOIRQ | curr_cflags(cpu);
+                cpu->accel->cflags_next_tb = 1 | CF_NOIRQ | curr_cflags(cpu);
                 mmap_unlock();
                 cpu_loop_exit_noexc(cpu);
             }
diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index ea342213d6..b6631b6245 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -124,7 +124,6 @@ static void cpu_common_reset_hold(Object *obj, ResetType type)
     cpu->icount_extra = 0;
     cpu->exception_index = -1;
     cpu->crash_occurred = false;
-    cpu->cflags_next_tb = -1;
 
     cpu_exec_reset_hold(cpu);
 }
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 09/13] accel/tcg: Move @iommu_notifiers from CPUState to TCG AccelCPUState
  2024-04-30 12:27 [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2) Philippe Mathieu-Daudé
                   ` (7 preceding siblings ...)
  2024-04-30 12:28 ` [PATCH v3 08/13] accel/tcg: Move @cflags_next_tb " Philippe Mathieu-Daudé
@ 2024-04-30 12:28 ` Philippe Mathieu-Daudé
  2024-04-30 12:28 ` [PATCH v3 10/13] accel/tcg: Move @tcg_cflags " Philippe Mathieu-Daudé
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-30 12:28 UTC (permalink / raw)
  To: qemu-devel
  Cc: Anton Johansson, Ilya Leoshkevich, Richard Henderson,
	Philippe Mathieu-Daudé

@iommu_notifiers is specific to TCG system emulation, move it to
AccelCPUState.

Restrict TCG specific code in system/physmem.c, adding an empty
stub for tcg_register_iommu_notifier().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20240428221450.26460-20-philmd@linaro.org>
---
 accel/tcg/vcpu-state.h |  3 +++
 include/hw/core/cpu.h  |  3 ---
 system/physmem.c       | 37 ++++++++++++++++++++++++++++---------
 3 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/accel/tcg/vcpu-state.h b/accel/tcg/vcpu-state.h
index 5b09279140..51e54ca535 100644
--- a/accel/tcg/vcpu-state.h
+++ b/accel/tcg/vcpu-state.h
@@ -19,6 +19,9 @@ struct AccelCPUState {
 
 #ifdef CONFIG_USER_ONLY
     TaskState *ts;
+#else
+    /* track IOMMUs whose translations we've cached in the TCG TLB */
+    GArray *iommu_notifiers;
 #endif /* !CONFIG_USER_ONLY */
 
 #ifdef CONFIG_PLUGIN
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 97a0baf874..f3cbb944eb 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -539,9 +539,6 @@ struct CPUState {
     /* Used for user-only emulation of prctl(PR_SET_UNALIGN). */
     bool prctl_unalign_sigbus;
 
-    /* track IOMMUs whose translations we've cached in the TCG TLB */
-    GArray *iommu_notifiers;
-
     /*
      * MUST BE LAST in order to minimize the displacement to CPUArchState.
      */
diff --git a/system/physmem.c b/system/physmem.c
index 44e477a1a5..1e003e42bb 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -27,6 +27,8 @@
 #include "qemu/madvise.h"
 
 #ifdef CONFIG_TCG
+#include "exec/translate-all.h"
+#include "accel/tcg/vcpu-state.h"
 #include "hw/core/tcg-cpu-ops.h"
 #endif /* CONFIG_TCG */
 
@@ -578,6 +580,8 @@ MemoryRegion *flatview_translate(FlatView *fv, hwaddr addr, hwaddr *xlat,
     return mr;
 }
 
+#ifdef CONFIG_TCG
+
 typedef struct TCGIOMMUNotifier {
     IOMMUNotifier n;
     MemoryRegion *mr;
@@ -614,17 +618,20 @@ static void tcg_register_iommu_notifier(CPUState *cpu,
     TCGIOMMUNotifier *notifier = NULL;
     int i;
 
-    for (i = 0; i < cpu->iommu_notifiers->len; i++) {
-        notifier = g_array_index(cpu->iommu_notifiers, TCGIOMMUNotifier *, i);
+    for (i = 0; i < cpu->accel->iommu_notifiers->len; i++) {
+        notifier = g_array_index(cpu->accel->iommu_notifiers,
+                                 TCGIOMMUNotifier *, i);
         if (notifier->mr == mr && notifier->iommu_idx == iommu_idx) {
             break;
         }
     }
-    if (i == cpu->iommu_notifiers->len) {
+    if (i == cpu->accel->iommu_notifiers->len) {
         /* Not found, add a new entry at the end of the array */
-        cpu->iommu_notifiers = g_array_set_size(cpu->iommu_notifiers, i + 1);
+        cpu->accel->iommu_notifiers = g_array_set_size(cpu->accel->iommu_notifiers,
+                                                       i + 1);
         notifier = g_new0(TCGIOMMUNotifier, 1);
-        g_array_index(cpu->iommu_notifiers, TCGIOMMUNotifier *, i) = notifier;
+        g_array_index(cpu->accel->iommu_notifiers,
+                      TCGIOMMUNotifier *, i) = notifier;
 
         notifier->mr = mr;
         notifier->iommu_idx = iommu_idx;
@@ -656,19 +663,31 @@ void tcg_iommu_free_notifier_list(CPUState *cpu)
     int i;
     TCGIOMMUNotifier *notifier;
 
-    for (i = 0; i < cpu->iommu_notifiers->len; i++) {
-        notifier = g_array_index(cpu->iommu_notifiers, TCGIOMMUNotifier *, i);
+    for (i = 0; i < cpu->accel->iommu_notifiers->len; i++) {
+        notifier = g_array_index(cpu->accel->iommu_notifiers,
+                                 TCGIOMMUNotifier *, i);
         memory_region_unregister_iommu_notifier(notifier->mr, &notifier->n);
         g_free(notifier);
     }
-    g_array_free(cpu->iommu_notifiers, true);
+    g_array_free(cpu->accel->iommu_notifiers, true);
 }
 
 void tcg_iommu_init_notifier_list(CPUState *cpu)
 {
-    cpu->iommu_notifiers = g_array_new(false, true, sizeof(TCGIOMMUNotifier *));
+    cpu->accel->iommu_notifiers = g_array_new(false, true,
+                                              sizeof(TCGIOMMUNotifier *));
 }
 
+#else
+
+static void tcg_register_iommu_notifier(CPUState *cpu,
+                                        IOMMUMemoryRegion *iommu_mr,
+                                        int iommu_idx)
+{
+}
+
+#endif
+
 /* Called from RCU critical section */
 MemoryRegionSection *
 address_space_translate_for_iotlb(CPUState *cpu, int asidx, hwaddr orig_addr,
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 10/13] accel/tcg: Move @tcg_cflags from CPUState to TCG AccelCPUState
  2024-04-30 12:27 [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2) Philippe Mathieu-Daudé
                   ` (8 preceding siblings ...)
  2024-04-30 12:28 ` [PATCH v3 09/13] accel/tcg: Move @iommu_notifiers " Philippe Mathieu-Daudé
@ 2024-04-30 12:28 ` Philippe Mathieu-Daudé
  2024-04-30 12:28 ` [PATCH v3 11/13] accel/tcg: Restrict icount to system emulation Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-30 12:28 UTC (permalink / raw)
  To: qemu-devel
  Cc: Anton Johansson, Ilya Leoshkevich, Richard Henderson,
	Philippe Mathieu-Daudé

@tcg_cflags is specific to TCG accelerator, move it to
its AccelCPUState.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20240428221450.26460-23-philmd@linaro.org>
---
 accel/tcg/vcpu-state.h | 2 ++
 include/hw/core/cpu.h  | 4 +---
 accel/tcg/cpu-exec.c   | 6 +++---
 linux-user/main.c      | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/accel/tcg/vcpu-state.h b/accel/tcg/vcpu-state.h
index 51e54ca535..008fe847b4 100644
--- a/accel/tcg/vcpu-state.h
+++ b/accel/tcg/vcpu-state.h
@@ -10,9 +10,11 @@
 
 /**
  * AccelCPUState: vCPU fields specific to TCG accelerator
+ * @cflags: Pre-computed cflags for this cpu.
  * @plugin_state: per-CPU plugin state
  */
 struct AccelCPUState {
+    uint32_t cflags;
     uint32_t cflags_next_tb;
 
     sigjmp_buf jmp_env;
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index f3cbb944eb..e546e67f4d 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -394,9 +394,8 @@ struct qemu_work_item;
  *   to a cluster this will be UNASSIGNED_CLUSTER_INDEX; otherwise it will
  *   be the same as the cluster-id property of the CPU object's TYPE_CPU_CLUSTER
  *   QOM parent.
- *   Under TCG this value is propagated to @tcg_cflags.
+ *   Under TCG this value is propagated to @accel->cflags.
  *   See TranslationBlock::TCG CF_CLUSTER_MASK.
- * @tcg_cflags: Pre-computed cflags for this cpu.
  * @nr_cores: Number of cores within this CPU package.
  * @nr_threads: Number of threads within this CPU core.
  * @running: #true if CPU is currently running (lockless).
@@ -517,7 +516,6 @@ struct CPUState {
     /* TODO Move common fields from CPUArchState here. */
     int cpu_index;
     int cluster_index;
-    uint32_t tcg_cflags;
     uint32_t halted;
     int32_t exception_index;
 
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 2edfad78b3..2af0e964c1 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -150,17 +150,17 @@ static void init_delay_params(SyncClocks *sc, const CPUState *cpu)
 
 bool tcg_cflags_has(CPUState *cpu, uint32_t flags)
 {
-    return cpu->tcg_cflags & flags;
+    return cpu->accel->cflags & flags;
 }
 
 void tcg_cflags_set(CPUState *cpu, uint32_t flags)
 {
-    cpu->tcg_cflags |= flags;
+    cpu->accel->cflags |= flags;
 }
 
 uint32_t curr_cflags(CPUState *cpu)
 {
-    uint32_t cflags = cpu->tcg_cflags;
+    uint32_t cflags = cpu->accel->cflags;
 
     /*
      * Record gdb single-step.  We should be exiting the TB by raising
diff --git a/linux-user/main.c b/linux-user/main.c
index 5f7f03f4b0..8be06627da 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -241,7 +241,7 @@ CPUArchState *cpu_copy(CPUArchState *env)
     /* Reset non arch specific state */
     cpu_reset(new_cpu);
 
-    new_cpu->tcg_cflags = cpu->tcg_cflags;
+    new_cpu->accel->cflags = cpu->accel->cflags;
     memcpy(new_env, env, sizeof(CPUArchState));
 #if defined(TARGET_I386) || defined(TARGET_X86_64)
     new_env->gdt.base = target_mmap(0, sizeof(uint64_t) * TARGET_GDT_ENTRIES,
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 11/13] accel/tcg: Restrict icount to system emulation
  2024-04-30 12:27 [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2) Philippe Mathieu-Daudé
                   ` (9 preceding siblings ...)
  2024-04-30 12:28 ` [PATCH v3 10/13] accel/tcg: Move @tcg_cflags " Philippe Mathieu-Daudé
@ 2024-04-30 12:28 ` Philippe Mathieu-Daudé
  2024-04-30 12:28 ` [PATCH v3 12/13] accel/tcg: Move icount fields from CPUState to TCG AccelCPUState Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-30 12:28 UTC (permalink / raw)
  To: qemu-devel
  Cc: Anton Johansson, Ilya Leoshkevich, Richard Henderson,
	Philippe Mathieu-Daudé

So far we don't support icount on user emulation.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20240428221450.26460-24-philmd@linaro.org>
---
 accel/tcg/cpu-exec.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 2af0e964c1..49b4b57a56 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -781,6 +781,9 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
 
 static inline bool icount_exit_request(CPUState *cpu)
 {
+#if defined(CONFIG_USER_ONLY)
+    return false;
+#else
     if (!icount_enabled()) {
         return false;
     }
@@ -788,6 +791,7 @@ static inline bool icount_exit_request(CPUState *cpu)
         return false;
     }
     return cpu->neg.icount_decr.u16.low + cpu->icount_extra == 0;
+#endif
 }
 
 static inline bool cpu_handle_interrupt(CPUState *cpu,
@@ -802,12 +806,14 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
         return false;
     }
 
+#if !defined(CONFIG_USER_ONLY)
     /* Clear the interrupt flag now since we're processing
      * cpu->interrupt_request and cpu->exit_request.
      * Ensure zeroing happens before reading cpu->exit_request or
      * cpu->interrupt_request (see also smp_wmb in cpu_exit())
      */
     qatomic_set_mb(&cpu->neg.icount_decr.u16.high, 0);
+#endif /* !CONFIG_USER_ONLY */
 
     if (unlikely(qatomic_read(&cpu->interrupt_request))) {
         int interrupt_request;
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 12/13] accel/tcg: Move icount fields from CPUState to TCG AccelCPUState
  2024-04-30 12:27 [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2) Philippe Mathieu-Daudé
                   ` (10 preceding siblings ...)
  2024-04-30 12:28 ` [PATCH v3 11/13] accel/tcg: Restrict icount to system emulation Philippe Mathieu-Daudé
@ 2024-04-30 12:28 ` Philippe Mathieu-Daudé
  2024-04-30 12:28 ` [PATCH v3 13/13] accel/tcg: Move @tb_jmp_cache " Philippe Mathieu-Daudé
  2024-04-30 17:55 ` [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2) Ilya Leoshkevich
  13 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-30 12:28 UTC (permalink / raw)
  To: qemu-devel
  Cc: Anton Johansson, Ilya Leoshkevich, Richard Henderson,
	Philippe Mathieu-Daudé

Both @icount_budget and @icount_extra fields are specific
to TCG accelerator, move them to its AccelCPUState.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20240428221450.26460-25-philmd@linaro.org>
---
 accel/tcg/vcpu-state.h           |  4 ++++
 include/hw/core/cpu.h            |  3 ---
 accel/tcg/cpu-exec.c             | 14 +++++++-------
 accel/tcg/icount-common.c        |  7 ++++---
 accel/tcg/tcg-accel-ops-icount.c | 14 +++++++-------
 accel/tcg/tcg-accel-ops.c        |  1 +
 hw/core/cpu-common.c             |  1 -
 7 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/accel/tcg/vcpu-state.h b/accel/tcg/vcpu-state.h
index 008fe847b4..716a0119c4 100644
--- a/accel/tcg/vcpu-state.h
+++ b/accel/tcg/vcpu-state.h
@@ -11,6 +11,7 @@
 /**
  * AccelCPUState: vCPU fields specific to TCG accelerator
  * @cflags: Pre-computed cflags for this cpu.
+ * @icount_extra: Instructions until next timer event.
  * @plugin_state: per-CPU plugin state
  */
 struct AccelCPUState {
@@ -22,6 +23,9 @@ struct AccelCPUState {
 #ifdef CONFIG_USER_ONLY
     TaskState *ts;
 #else
+    int64_t icount_budget;
+    int64_t icount_extra;
+
     /* track IOMMUs whose translations we've cached in the TCG TLB */
     GArray *iommu_notifiers;
 #endif /* !CONFIG_USER_ONLY */
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index e546e67f4d..f06bb1e93a 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -409,7 +409,6 @@ struct qemu_work_item;
  * @unplug: Indicates a pending CPU unplug request.
  * @crash_occurred: Indicates the OS reported a crash (panic) for this CPU
  * @singlestep_enabled: Flags for single-stepping.
- * @icount_extra: Instructions until next timer event.
  * @cpu_ases: Pointer to array of CPUAddressSpaces (which define the
  *            AddressSpaces this CPU has)
  * @num_ases: number of CPUAddressSpaces in @cpu_ases
@@ -470,8 +469,6 @@ struct CPUState {
     /* updates protected by BQL */
     uint32_t interrupt_request;
     int singlestep_enabled;
-    int64_t icount_budget;
-    int64_t icount_extra;
     uint64_t random_seed;
 
     QemuMutex work_mutex;
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 49b4b57a56..a491278082 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -75,7 +75,7 @@ static void align_clocks(SyncClocks *sc, CPUState *cpu)
         return;
     }
 
-    cpu_icount = cpu->icount_extra + cpu->neg.icount_decr.u16.low;
+    cpu_icount = cpu->accel->icount_extra + cpu->neg.icount_decr.u16.low;
     sc->diff_clk += icount_to_ns(sc->last_cpu_icount - cpu_icount);
     sc->last_cpu_icount = cpu_icount;
 
@@ -126,7 +126,7 @@ static void init_delay_params(SyncClocks *sc, CPUState *cpu)
     sc->realtime_clock = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT);
     sc->diff_clk = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - sc->realtime_clock;
     sc->last_cpu_icount
-        = cpu->icount_extra + cpu->neg.icount_decr.u16.low;
+        = cpu->accel->icount_extra + cpu->neg.icount_decr.u16.low;
     if (sc->diff_clk < max_delay) {
         max_delay = sc->diff_clk;
     }
@@ -719,7 +719,7 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
     if (cpu->exception_index < 0) {
 #ifndef CONFIG_USER_ONLY
         if (replay_has_exception()
-            && cpu->neg.icount_decr.u16.low + cpu->icount_extra == 0) {
+            && cpu->neg.icount_decr.u16.low + cpu->accel->icount_extra == 0) {
             /* Execute just one insn to trigger exception pending in the log */
             cpu->accel->cflags_next_tb = (curr_cflags(cpu) & ~CF_USE_ICOUNT)
                 | CF_NOIRQ | 1;
@@ -790,7 +790,7 @@ static inline bool icount_exit_request(CPUState *cpu)
     if (!(cpu->accel->cflags_next_tb == -1 || cpu->accel->cflags_next_tb & CF_USE_ICOUNT)) {
         return false;
     }
-    return cpu->neg.icount_decr.u16.low + cpu->icount_extra == 0;
+    return cpu->neg.icount_decr.u16.low + cpu->accel->icount_extra == 0;
 #endif
 }
 
@@ -942,9 +942,9 @@ static inline void cpu_loop_exec_tb(CPUState *cpu, TranslationBlock *tb,
     /* Ensure global icount has gone forward */
     icount_update(cpu);
     /* Refill decrementer and continue execution.  */
-    int32_t insns_left = MIN(0xffff, cpu->icount_budget);
+    int32_t insns_left = MIN(0xffff, cpu->accel->icount_budget);
     cpu->neg.icount_decr.u16.low = insns_left;
-    cpu->icount_extra = cpu->icount_budget - insns_left;
+    cpu->accel->icount_extra = cpu->accel->icount_budget - insns_left;
 
     /*
      * If the next tb has more instructions than we have left to
@@ -953,7 +953,7 @@ static inline void cpu_loop_exec_tb(CPUState *cpu, TranslationBlock *tb,
      */
     if (insns_left > 0 && insns_left < tb->icount)  {
         assert(insns_left <= CF_COUNT_MASK);
-        assert(cpu->icount_extra == 0);
+        assert(cpu->accel->icount_extra == 0);
         cpu->accel->cflags_next_tb = (tb->cflags & ~CF_COUNT_MASK) | insns_left;
     }
 #endif
diff --git a/accel/tcg/icount-common.c b/accel/tcg/icount-common.c
index 8d3d3a7e9d..ff503f8e96 100644
--- a/accel/tcg/icount-common.c
+++ b/accel/tcg/icount-common.c
@@ -38,6 +38,7 @@
 #include "sysemu/cpu-timers.h"
 #include "sysemu/cpu-throttle.h"
 #include "sysemu/cpu-timers-internal.h"
+#include "accel/tcg/vcpu-state.h"
 
 /*
  * ICOUNT: Instruction Counter
@@ -71,8 +72,8 @@ static void icount_enable_adaptive(void)
  */
 static int64_t icount_get_executed(CPUState *cpu)
 {
-    return (cpu->icount_budget -
-            (cpu->neg.icount_decr.u16.low + cpu->icount_extra));
+    return (cpu->accel->icount_budget -
+            (cpu->neg.icount_decr.u16.low + cpu->accel->icount_extra));
 }
 
 /*
@@ -83,7 +84,7 @@ static int64_t icount_get_executed(CPUState *cpu)
 static void icount_update_locked(CPUState *cpu)
 {
     int64_t executed = icount_get_executed(cpu);
-    cpu->icount_budget -= executed;
+    cpu->accel->icount_budget -= executed;
 
     qatomic_set_i64(&timers_state.qemu_icount,
                     timers_state.qemu_icount + executed);
diff --git a/accel/tcg/tcg-accel-ops-icount.c b/accel/tcg/tcg-accel-ops-icount.c
index 9e1ae66f65..75073ec23f 100644
--- a/accel/tcg/tcg-accel-ops-icount.c
+++ b/accel/tcg/tcg-accel-ops-icount.c
@@ -112,16 +112,16 @@ void icount_prepare_for_run(CPUState *cpu, int64_t cpu_budget)
      * asynchronously by cpu_exit/cpu_interrupt/tcg_handle_interrupt
      */
     g_assert(cpu->neg.icount_decr.u16.low == 0);
-    g_assert(cpu->icount_extra == 0);
+    g_assert(cpu->accel->icount_extra == 0);
 
     replay_mutex_lock();
 
-    cpu->icount_budget = MIN(icount_get_limit(), cpu_budget);
-    insns_left = MIN(0xffff, cpu->icount_budget);
+    cpu->accel->icount_budget = MIN(icount_get_limit(), cpu_budget);
+    insns_left = MIN(0xffff, cpu->accel->icount_budget);
     cpu->neg.icount_decr.u16.low = insns_left;
-    cpu->icount_extra = cpu->icount_budget - insns_left;
+    cpu->accel->icount_extra = cpu->accel->icount_budget - insns_left;
 
-    if (cpu->icount_budget == 0) {
+    if (cpu->accel->icount_budget == 0) {
         /*
          * We're called without the BQL, so must take it while
          * we're calling timer handlers.
@@ -139,8 +139,8 @@ void icount_process_data(CPUState *cpu)
 
     /* Reset the counters */
     cpu->neg.icount_decr.u16.low = 0;
-    cpu->icount_extra = 0;
-    cpu->icount_budget = 0;
+    cpu->accel->icount_extra = 0;
+    cpu->accel->icount_budget = 0;
 
     replay_account_executed_instructions();
 
diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
index d9132a5835..3b28cab0b5 100644
--- a/accel/tcg/tcg-accel-ops.c
+++ b/accel/tcg/tcg-accel-ops.c
@@ -89,6 +89,7 @@ static void tcg_cpu_reset_hold(CPUState *cpu)
 
     qatomic_set(&cpu->neg.icount_decr.u32, 0);
     cpu->neg.can_do_io = true;
+    cpu->accel->icount_extra = 0;
     cpu->accel->cflags_next_tb = -1;
 }
 
diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index b6631b6245..e03d31876f 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -121,7 +121,6 @@ static void cpu_common_reset_hold(Object *obj, ResetType type)
     cpu->interrupt_request = 0;
     cpu->halted = cpu->start_powered_off;
     cpu->mem_io_pc = 0;
-    cpu->icount_extra = 0;
     cpu->exception_index = -1;
     cpu->crash_occurred = false;
 
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 13/13] accel/tcg: Move @tb_jmp_cache from CPUState to TCG AccelCPUState
  2024-04-30 12:27 [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2) Philippe Mathieu-Daudé
                   ` (11 preceding siblings ...)
  2024-04-30 12:28 ` [PATCH v3 12/13] accel/tcg: Move icount fields from CPUState to TCG AccelCPUState Philippe Mathieu-Daudé
@ 2024-04-30 12:28 ` Philippe Mathieu-Daudé
  2024-04-30 17:55 ` [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2) Ilya Leoshkevich
  13 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-30 12:28 UTC (permalink / raw)
  To: qemu-devel
  Cc: Anton Johansson, Ilya Leoshkevich, Richard Henderson,
	Philippe Mathieu-Daudé

@tb_jmp_cache is specific to TCG accelerator, move it to its
AccelCPUState. Remove the NULL check in tcg_flush_jmp_cache().

Cc: Ilya Leoshkevich <iii@linux.ibm.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20240428221450.26460-21-philmd@linaro.org>
---
 accel/tcg/tb-jmp-cache.h  | 4 ++--
 accel/tcg/vcpu-state.h    | 2 ++
 include/hw/core/cpu.h     | 2 --
 include/qemu/typedefs.h   | 1 -
 accel/tcg/cpu-exec.c      | 7 +++----
 accel/tcg/cputlb.c        | 2 +-
 accel/tcg/tb-maint.c      | 2 +-
 accel/tcg/translate-all.c | 7 +------
 8 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/accel/tcg/tb-jmp-cache.h b/accel/tcg/tb-jmp-cache.h
index 184bb3e3e2..c3a505e394 100644
--- a/accel/tcg/tb-jmp-cache.h
+++ b/accel/tcg/tb-jmp-cache.h
@@ -22,12 +22,12 @@
  * non-NULL value of 'tb'.  Strictly speaking pc is only needed for
  * CF_PCREL, but it's used always for simplicity.
  */
-struct CPUJumpCache {
+typedef struct CPUJumpCache {
     struct rcu_head rcu;
     struct {
         TranslationBlock *tb;
         vaddr pc;
     } array[TB_JMP_CACHE_SIZE];
-};
+} CPUJumpCache;
 
 #endif /* ACCEL_TCG_TB_JMP_CACHE_H */
diff --git a/accel/tcg/vcpu-state.h b/accel/tcg/vcpu-state.h
index 716a0119c4..7fff5413e9 100644
--- a/accel/tcg/vcpu-state.h
+++ b/accel/tcg/vcpu-state.h
@@ -7,6 +7,7 @@
 #define ACCEL_TCG_VCPU_STATE_H
 
 #include "hw/core/cpu.h"
+#include "tb-jmp-cache.h"
 
 /**
  * AccelCPUState: vCPU fields specific to TCG accelerator
@@ -19,6 +20,7 @@ struct AccelCPUState {
     uint32_t cflags_next_tb;
 
     sigjmp_buf jmp_env;
+    CPUJumpCache tb_jmp_cache;
 
 #ifdef CONFIG_USER_ONLY
     TaskState *ts;
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index f06bb1e93a..c0c28befd3 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -479,8 +479,6 @@ struct CPUState {
     AddressSpace *as;
     MemoryRegion *memory;
 
-    CPUJumpCache *tb_jmp_cache;
-
     GArray *gdb_regs;
     int gdb_num_regs;
     int gdb_num_g_regs;
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 36f2825725..daf9009332 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -44,7 +44,6 @@ typedef struct CPUAddressSpace CPUAddressSpace;
 typedef struct CPUArchState CPUArchState;
 typedef struct CPUPluginState CPUPluginState;
 typedef struct CpuInfoFast CpuInfoFast;
-typedef struct CPUJumpCache CPUJumpCache;
 typedef struct CPUState CPUState;
 typedef struct CPUTLBEntryFull CPUTLBEntryFull;
 typedef struct DeviceListener DeviceListener;
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index a491278082..f813c68d1e 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -262,7 +262,7 @@ static inline TranslationBlock *tb_lookup(CPUState *cpu, vaddr pc,
     tcg_debug_assert(!(cflags & CF_INVALID));
 
     hash = tb_jmp_cache_hash_func(pc);
-    jc = cpu->tb_jmp_cache;
+    jc = &cpu->accel->tb_jmp_cache;
 
     tb = qatomic_read(&jc->array[hash].tb);
     if (likely(tb &&
@@ -1011,7 +1011,7 @@ cpu_exec_loop(CPUState *cpu, SyncClocks *sc)
                  * for the fast lookup
                  */
                 h = tb_jmp_cache_hash_func(pc);
-                jc = cpu->tb_jmp_cache;
+                jc = &cpu->accel->tb_jmp_cache;
                 jc->array[h].pc = pc;
                 qatomic_set(&jc->array[h].tb, tb);
             }
@@ -1090,7 +1090,6 @@ bool tcg_exec_realizefn(CPUState *cpu, Error **errp)
         tcg_target_initialized = true;
     }
 
-    cpu->tb_jmp_cache = g_new0(CPUJumpCache, 1);
     tlb_init(cpu);
 #ifndef CONFIG_USER_ONLY
     tcg_iommu_init_notifier_list(cpu);
@@ -1108,5 +1107,5 @@ void tcg_exec_unrealizefn(CPUState *cpu)
 #endif /* !CONFIG_USER_ONLY */
 
     tlb_destroy(cpu);
-    g_free_rcu(cpu->tb_jmp_cache, rcu);
+    g_free_rcu(&cpu->accel->tb_jmp_cache, rcu);
 }
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index cdb3e12dfb..eaa60d1da2 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -156,7 +156,7 @@ static void tlb_window_reset(CPUTLBDesc *desc, int64_t ns,
 
 static void tb_jmp_cache_clear_page(CPUState *cpu, vaddr page_addr)
 {
-    CPUJumpCache *jc = cpu->tb_jmp_cache;
+    CPUJumpCache *jc = &cpu->accel->tb_jmp_cache;
     int i, i0;
 
     if (unlikely(!jc)) {
diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
index 2d5faca9fd..83758648f2 100644
--- a/accel/tcg/tb-maint.c
+++ b/accel/tcg/tb-maint.c
@@ -888,7 +888,7 @@ static void tb_jmp_cache_inval_tb(TranslationBlock *tb)
         uint32_t h = tb_jmp_cache_hash_func(tb->pc);
 
         CPU_FOREACH(cpu) {
-            CPUJumpCache *jc = cpu->tb_jmp_cache;
+            CPUJumpCache *jc = &cpu->accel->tb_jmp_cache;
 
             if (qatomic_read(&jc->array[h].tb) == tb) {
                 qatomic_set(&jc->array[h].tb, NULL);
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 3a8199a761..9b02f21b23 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -652,12 +652,7 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr)
  */
 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;
-    }
+    CPUJumpCache *jc = &cpu->accel->tb_jmp_cache;
 
     for (int i = 0; i < TB_JMP_CACHE_SIZE; i++) {
         qatomic_set(&jc->array[i].tb, NULL);
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2)
  2024-04-30 12:27 [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2) Philippe Mathieu-Daudé
                   ` (12 preceding siblings ...)
  2024-04-30 12:28 ` [PATCH v3 13/13] accel/tcg: Move @tb_jmp_cache " Philippe Mathieu-Daudé
@ 2024-04-30 17:55 ` Ilya Leoshkevich
  2024-04-30 18:45   ` Philippe Mathieu-Daudé
  13 siblings, 1 reply; 20+ messages in thread
From: Ilya Leoshkevich @ 2024-04-30 17:55 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Anton Johansson, Richard Henderson

On Tue, Apr 30, 2024 at 02:27:54PM +0200, Philippe Mathieu-Daudé wrote:
> Missing WASM testing by Ilya (branch available at
> https://gitlab.com/philmd/qemu/-/commits/tcg_flush_jmp_cache)

Hmm, it dies very early now:

  # gdb --args ./qemu-s390x -L /usr/s390x-linux-gnu /build/wasmtime/target/s390x-unknown-linux-gnu/debug/deps/component_fuzz_util-d10a3a6b4ad8af47

  Thread 1 "qemu-s390x" received signal SIGSEGV, Segmentation fault.
  0x000055555559b718 in cpu_common_realizefn (dev=0x5555557c28c0, errp=<optimized out>) at ../home/iii/myrepos/qemu/hw/core/cpu-common.c:217
  217             cpu->accel->plugin_state = qemu_plugin_create_vcpu_state();

  (gdb) bt
  #0  0x000055555559b718 in cpu_common_realizefn (dev=0x5555557c28c0, errp=<optimized out>) at ../home/iii/myrepos/qemu/hw/core/cpu-common.c:217
  #1  0x000055555559f59a in s390_cpu_realizefn (dev=0x5555557c28c0, errp=0x7fffffffe1a0) at ../home/iii/myrepos/qemu/target/s390x/cpu.c:284
  #2  0x000055555563f76b in device_set_realized (obj=<optimized out>, value=<optimized out>, errp=0x7fffffffe2e0) at ../home/iii/myrepos/qemu/hw/core/qdev.c:510
  #3  0x000055555564363d in property_set_bool (obj=0x5555557c28c0, v=<optimized out>, name=<optimized out>, opaque=0x5555557a9140, errp=0x7fffffffe2e0) at ../home/iii/myrepos/qemu/qom/object.c:2362
  #4  0x0000555555646b9b in object_property_set (obj=obj@entry=0x5555557c28c0, name=name@entry=0x5555556e8ae2 "realized", v=v@entry=0x5555557c6650, errp=errp@entry=0x7fffffffe2e0)
      at ../home/iii/myrepos/qemu/qom/object.c:1471
  #5  0x000055555564a43f in object_property_set_qobject (obj=obj@entry=0x5555557c28c0, name=name@entry=0x5555556e8ae2 "realized", value=value@entry=0x5555557a7a90, errp=errp@entry=0x7fffffffe2e0)
      at ../home/iii/myrepos/qemu/qom/qom-qobject.c:28
  #6  0x0000555555647204 in object_property_set_bool (obj=0x5555557c28c0, name=name@entry=0x5555556e8ae2 "realized", value=value@entry=true, errp=errp@entry=0x7fffffffe2e0)
      at ../home/iii/myrepos/qemu/qom/object.c:1541
  #7  0x000055555564025c in qdev_realize (dev=<optimized out>, bus=bus@entry=0x0, errp=errp@entry=0x7fffffffe2e0) at ../home/iii/myrepos/qemu/hw/core/qdev.c:291
  #8  0x000055555559bbb4 in cpu_create (typename=<optimized out>) at ../home/iii/myrepos/qemu/hw/core/cpu-common.c:61
  #9  0x000055555559a467 in main (argc=4, argv=0x7fffffffeaa8, envp=<optimized out>) at ../home/iii/myrepos/qemu/linux-user/main.c:811

  (gdb) p cpu
  $1 = (CPUState *) 0x5555557c28c0
  (gdb) p cpu->accel
  $2 = (AccelCPUState *) 0x0

Configured with: '/home/iii/myrepos/qemu/configure' '--target-list=s390x-linux-user' '--disable-tools' '--disable-slirp' '--disable-fdt' '--disable-capstone' '--disable-docs'

If you don't see what can be wrong here right away, I can debug this.

> Since v2:
> - Move cpu_loop_exit_requested() to "exec/cpu-loop.h"
> - Added R-b tags
> 
> Since v1:
> - First 13 patches queued
> - Restrict qemu_plugin_vcpu_exit_hook() to (TCG) plugins
> - Restrict cpu_plugin_mem_cbs_enabled() to TCG (plugins)
> - Addressed Richard review comments on the others:
>   - Move cpu_plugin_mem_cbs_enabled()
>   - Do not move mem_io_pc, waiting for [*]
>   - Mention can_do_io restricted
> 
> Finish extracting TCG fields from CPUState:
> - Extract tcg_cpu_exit() from cpu_exit()
> - Introduce AccelOpsClass::exit_vcpu_thread()
> - cpu_exit() calls exit_vcpu_thread=tcg_cpu_exit for TCG
> - Forward declare TaskState and more uses of get_task_state()
> - Introduce TCG AccelCPUState
> - Move TCG specific fields from CPUState to AccelCPUState
> - Restrict "exec/tlb-common.h" to TCG
> - Restrict iommu_notifiers, icount to system emulation
> 
> [*] https://lore.kernel.org/qemu-devel/20240416040609.1313605-3-richard.henderson@linaro.org/
> 
> Based-on: https://gitlab.com/philmd/qemu/-/commits/accel-next
> 
> Philippe Mathieu-Daudé (13):
>   accel/tcg: Restrict qemu_plugin_vcpu_exit_hook() to TCG plugins
>   accel/tcg: Restrict cpu_plugin_mem_cbs_enabled() to TCG
>   accel/tcg: Move @plugin_mem_cbs from CPUState to
>     CPUNegativeOffsetState
>   accel/tcg: Move @plugin_state from CPUState to TCG AccelCPUState
>   accel/tcg: Restrict cpu_loop_exit_requested() to TCG
>   accel/tcg: Restrict IcountDecr / can_do_io / CPUTLB to TCG
>   accel/tcg: Move @jmp_env from CPUState to TCG AccelCPUState
>   accel/tcg: Move @cflags_next_tb from CPUState to TCG AccelCPUState
>   accel/tcg: Move @iommu_notifiers from CPUState to TCG AccelCPUState
>   accel/tcg: Move @tcg_cflags from CPUState to TCG AccelCPUState
>   accel/tcg: Restrict icount to system emulation
>   accel/tcg: Move icount fields from CPUState to TCG AccelCPUState
>   accel/tcg: Move @tb_jmp_cache from CPUState to TCG AccelCPUState
> 
>  accel/tcg/internal-common.h      | 18 ++++++++++
>  accel/tcg/tb-jmp-cache.h         |  4 +--
>  accel/tcg/tcg-accel-ops.h        |  1 +
>  accel/tcg/vcpu-state.h           | 20 +++++++++++
>  include/exec/cpu-loop.h          | 35 +++++++++++++++++++
>  include/exec/exec-all.h          | 17 ----------
>  include/exec/tlb-common.h        |  4 +++
>  include/hw/core/cpu.h            | 58 ++++++++------------------------
>  include/qemu/plugin.h            |  2 +-
>  include/qemu/typedefs.h          |  1 -
>  accel/tcg/cpu-exec-common.c      |  2 +-
>  accel/tcg/cpu-exec.c             | 52 +++++++++++++++-------------
>  accel/tcg/cputlb.c               |  2 +-
>  accel/tcg/icount-common.c        |  7 ++--
>  accel/tcg/plugin-gen.c           |  9 +++--
>  accel/tcg/tb-maint.c             |  6 ++--
>  accel/tcg/tcg-accel-ops-icount.c | 14 ++++----
>  accel/tcg/tcg-accel-ops.c        |  2 ++
>  accel/tcg/translate-all.c        |  9 ++---
>  accel/tcg/watchpoint.c           |  5 +--
>  hw/core/cpu-common.c             |  9 +++--
>  linux-user/main.c                |  2 +-
>  plugins/core.c                   |  9 ++---
>  system/physmem.c                 | 37 +++++++++++++++-----
>  target/arm/tcg/helper-a64.c      |  1 +
>  target/s390x/tcg/mem_helper.c    |  1 +
>  26 files changed, 195 insertions(+), 132 deletions(-)
>  create mode 100644 include/exec/cpu-loop.h
> 
> -- 
> 2.41.0
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2)
  2024-04-30 17:55 ` [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2) Ilya Leoshkevich
@ 2024-04-30 18:45   ` Philippe Mathieu-Daudé
  2024-04-30 19:00     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-30 18:45 UTC (permalink / raw)
  To: Ilya Leoshkevich, qemu-devel; +Cc: Anton Johansson, Richard Henderson

Hi Ilya,

On 30/4/24 19:55, Ilya Leoshkevich wrote:
> On Tue, Apr 30, 2024 at 02:27:54PM +0200, Philippe Mathieu-Daudé wrote:
>> Missing WASM testing by Ilya (branch available at
>> https://gitlab.com/philmd/qemu/-/commits/tcg_flush_jmp_cache)
> 
> Hmm, it dies very early now:
> 
>    # gdb --args ./qemu-s390x -L /usr/s390x-linux-gnu /build/wasmtime/target/s390x-unknown-linux-gnu/debug/deps/component_fuzz_util-d10a3a6b4ad8af47
> 
>    Thread 1 "qemu-s390x" received signal SIGSEGV, Segmentation fault.
>    0x000055555559b718 in cpu_common_realizefn (dev=0x5555557c28c0, errp=<optimized out>) at ../home/iii/myrepos/qemu/hw/core/cpu-common.c:217
>    217             cpu->accel->plugin_state = qemu_plugin_create_vcpu_state();
> 
>    (gdb) bt
>    #0  0x000055555559b718 in cpu_common_realizefn (dev=0x5555557c28c0, errp=<optimized out>) at ../home/iii/myrepos/qemu/hw/core/cpu-common.c:217
>    #1  0x000055555559f59a in s390_cpu_realizefn (dev=0x5555557c28c0, errp=0x7fffffffe1a0) at ../home/iii/myrepos/qemu/target/s390x/cpu.c:284
>    #2  0x000055555563f76b in device_set_realized (obj=<optimized out>, value=<optimized out>, errp=0x7fffffffe2e0) at ../home/iii/myrepos/qemu/hw/core/qdev.c:510
>    #3  0x000055555564363d in property_set_bool (obj=0x5555557c28c0, v=<optimized out>, name=<optimized out>, opaque=0x5555557a9140, errp=0x7fffffffe2e0) at ../home/iii/myrepos/qemu/qom/object.c:2362
>    #4  0x0000555555646b9b in object_property_set (obj=obj@entry=0x5555557c28c0, name=name@entry=0x5555556e8ae2 "realized", v=v@entry=0x5555557c6650, errp=errp@entry=0x7fffffffe2e0)
>        at ../home/iii/myrepos/qemu/qom/object.c:1471
>    #5  0x000055555564a43f in object_property_set_qobject (obj=obj@entry=0x5555557c28c0, name=name@entry=0x5555556e8ae2 "realized", value=value@entry=0x5555557a7a90, errp=errp@entry=0x7fffffffe2e0)
>        at ../home/iii/myrepos/qemu/qom/qom-qobject.c:28
>    #6  0x0000555555647204 in object_property_set_bool (obj=0x5555557c28c0, name=name@entry=0x5555556e8ae2 "realized", value=value@entry=true, errp=errp@entry=0x7fffffffe2e0)
>        at ../home/iii/myrepos/qemu/qom/object.c:1541
>    #7  0x000055555564025c in qdev_realize (dev=<optimized out>, bus=bus@entry=0x0, errp=errp@entry=0x7fffffffe2e0) at ../home/iii/myrepos/qemu/hw/core/qdev.c:291
>    #8  0x000055555559bbb4 in cpu_create (typename=<optimized out>) at ../home/iii/myrepos/qemu/hw/core/cpu-common.c:61
>    #9  0x000055555559a467 in main (argc=4, argv=0x7fffffffeaa8, envp=<optimized out>) at ../home/iii/myrepos/qemu/linux-user/main.c:811
> 
>    (gdb) p cpu
>    $1 = (CPUState *) 0x5555557c28c0
>    (gdb) p cpu->accel
>    $2 = (AccelCPUState *) 0x0
> 
> Configured with: '/home/iii/myrepos/qemu/configure' '--target-list=s390x-linux-user' '--disable-tools' '--disable-slirp' '--disable-fdt' '--disable-capstone' '--disable-docs'
> 
> If you don't see what can be wrong here right away, I can debug this.

Useful enough I guess, but I'll ask you to test again later.

Does it work without the last patch?

Is it possible to share component_fuzz_util-d10a3a6b4ad8af47?

Thanks for the testing,

Phil.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2)
  2024-04-30 18:45   ` Philippe Mathieu-Daudé
@ 2024-04-30 19:00     ` Philippe Mathieu-Daudé
  2024-04-30 21:42       ` Ilya Leoshkevich
  0 siblings, 1 reply; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-30 19:00 UTC (permalink / raw)
  To: Ilya Leoshkevich, qemu-devel; +Cc: Anton Johansson, Richard Henderson

On 30/4/24 20:45, Philippe Mathieu-Daudé wrote:
> Hi Ilya,
> 
> On 30/4/24 19:55, Ilya Leoshkevich wrote:
>> On Tue, Apr 30, 2024 at 02:27:54PM +0200, Philippe Mathieu-Daudé wrote:
>>> Missing WASM testing by Ilya (branch available at
>>> https://gitlab.com/philmd/qemu/-/commits/tcg_flush_jmp_cache)
>>
>> Hmm, it dies very early now:
>>
>>    # gdb --args ./qemu-s390x -L /usr/s390x-linux-gnu 
>> /build/wasmtime/target/s390x-unknown-linux-gnu/debug/deps/component_fuzz_util-d10a3a6b4ad8af47
>>
>>    Thread 1 "qemu-s390x" received signal SIGSEGV, Segmentation fault.
>>    0x000055555559b718 in cpu_common_realizefn (dev=0x5555557c28c0, 
>> errp=<optimized out>) at 
>> ../home/iii/myrepos/qemu/hw/core/cpu-common.c:217
>>    217             cpu->accel->plugin_state = 
>> qemu_plugin_create_vcpu_state();
>>
>>    (gdb) bt
>>    #0  0x000055555559b718 in cpu_common_realizefn (dev=0x5555557c28c0, 
>> errp=<optimized out>) at 
>> ../home/iii/myrepos/qemu/hw/core/cpu-common.c:217
>>    #1  0x000055555559f59a in s390_cpu_realizefn (dev=0x5555557c28c0, 
>> errp=0x7fffffffe1a0) at ../home/iii/myrepos/qemu/target/s390x/cpu.c:284
>>    #2  0x000055555563f76b in device_set_realized (obj=<optimized out>, 
>> value=<optimized out>, errp=0x7fffffffe2e0) at 
>> ../home/iii/myrepos/qemu/hw/core/qdev.c:510
>>    #3  0x000055555564363d in property_set_bool (obj=0x5555557c28c0, 
>> v=<optimized out>, name=<optimized out>, opaque=0x5555557a9140, 
>> errp=0x7fffffffe2e0) at ../home/iii/myrepos/qemu/qom/object.c:2362
>>    #4  0x0000555555646b9b in object_property_set 
>> (obj=obj@entry=0x5555557c28c0, name=name@entry=0x5555556e8ae2 
>> "realized", v=v@entry=0x5555557c6650, errp=errp@entry=0x7fffffffe2e0)
>>        at ../home/iii/myrepos/qemu/qom/object.c:1471
>>    #5  0x000055555564a43f in object_property_set_qobject 
>> (obj=obj@entry=0x5555557c28c0, name=name@entry=0x5555556e8ae2 
>> "realized", value=value@entry=0x5555557a7a90, 
>> errp=errp@entry=0x7fffffffe2e0)
>>        at ../home/iii/myrepos/qemu/qom/qom-qobject.c:28
>>    #6  0x0000555555647204 in object_property_set_bool 
>> (obj=0x5555557c28c0, name=name@entry=0x5555556e8ae2 "realized", 
>> value=value@entry=true, errp=errp@entry=0x7fffffffe2e0)
>>        at ../home/iii/myrepos/qemu/qom/object.c:1541
>>    #7  0x000055555564025c in qdev_realize (dev=<optimized out>, 
>> bus=bus@entry=0x0, errp=errp@entry=0x7fffffffe2e0) at 
>> ../home/iii/myrepos/qemu/hw/core/qdev.c:291
>>    #8  0x000055555559bbb4 in cpu_create (typename=<optimized out>) at 
>> ../home/iii/myrepos/qemu/hw/core/cpu-common.c:61
>>    #9  0x000055555559a467 in main (argc=4, argv=0x7fffffffeaa8, 
>> envp=<optimized out>) at ../home/iii/myrepos/qemu/linux-user/main.c:811
>>
>>    (gdb) p cpu
>>    $1 = (CPUState *) 0x5555557c28c0
>>    (gdb) p cpu->accel
>>    $2 = (AccelCPUState *) 0x0
>>
>> Configured with: '/home/iii/myrepos/qemu/configure' 
>> '--target-list=s390x-linux-user' '--disable-tools' '--disable-slirp' 
>> '--disable-fdt' '--disable-capstone' '--disable-docs'
>>
>> If you don't see what can be wrong here right away, I can debug this.

I added this commit in the same branch:

-- >8 --
Author: Philippe Mathieu-Daudé <philmd@linaro.org>
Date:   Tue Apr 30 20:57:15 2024 +0200

     accel/tcg: Initialize TCG plugins in cpu-target.c

     Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

diff --git a/cpu-target.c b/cpu-target.c
index 5af120e8aa..585533cfa3 100644
--- a/cpu-target.c
+++ b/cpu-target.c
@@ -46,6 +46,10 @@
  #include "hw/core/accel-cpu.h"
  #include "trace/trace-root.h"
  #include "qemu/accel.h"
+#ifdef CONFIG_PLUGIN
+#include "accel/tcg/vcpu-state.h"
+#include "qemu/plugin.h"
+#endif

  #ifndef CONFIG_USER_ONLY
  static int cpu_common_post_load(void *opaque, int version_id)
@@ -131,6 +135,13 @@ const VMStateDescription vmstate_cpu_common = {
  };
  #endif

+#ifdef CONFIG_PLUGIN
+static void qemu_plugin_vcpu_init__async(CPUState *cpu, run_on_cpu_data 
unused)
+{
+    qemu_plugin_vcpu_init_hook(cpu);
+}
+#endif
+
  bool cpu_exec_realizefn(CPUState *cpu, Error **errp)
  {
      /* cache the cpu class for the hotpath */
@@ -143,6 +154,15 @@ bool cpu_exec_realizefn(CPUState *cpu, Error **errp)
      /* Wait until cpu initialization complete before exposing cpu. */
      cpu_list_add(cpu);

+#ifdef CONFIG_PLUGIN
+    assert(cpu->accel);
+    /* Plugin initialization must wait until the cpu start executing 
code */
+    if (tcg_enabled()) {
+        cpu->accel->plugin_state = qemu_plugin_create_vcpu_state();
+        async_run_on_cpu(cpu, qemu_plugin_vcpu_init__async, 
RUN_ON_CPU_NULL);
+    }
+#endif
+
  #ifdef CONFIG_USER_ONLY
      assert(qdev_get_vmsd(DEVICE(cpu)) == NULL ||
             qdev_get_vmsd(DEVICE(cpu))->unmigratable);
@@ -171,6 +191,13 @@ void cpu_exec_unrealizefn(CPUState *cpu)
      }
  #endif

+#ifdef CONFIG_PLUGIN
+    /* Call the plugin hook before clearing the cpu is fully unrealized */
+    if (tcg_enabled()) {
+        qemu_plugin_vcpu_exit_hook(cpu);
+    }
+#endif
+
      cpu_list_remove(cpu);
      /*
       * Now that the vCPU has been removed from the RCU list, we can call
diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index e03d31876f..cd8bd99131 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -30,10 +30,6 @@
  #include "hw/boards.h"
  #include "hw/qdev-properties.h"
  #include "trace.h"
-#ifdef CONFIG_PLUGIN
-#include "accel/tcg/vcpu-state.h"
-#include "qemu/plugin.h"
-#endif

  CPUState *cpu_by_arch_id(int64_t id)
  {
@@ -181,13 +177,6 @@ static void cpu_common_parse_features(const char 
*typename, char *features,
      }
  }

-#ifdef CONFIG_PLUGIN
-static void qemu_plugin_vcpu_init__async(CPUState *cpu, run_on_cpu_data 
unused)
-{
-    qemu_plugin_vcpu_init_hook(cpu);
-}
-#endif
-
  static void cpu_common_realizefn(DeviceState *dev, Error **errp)
  {
      CPUState *cpu = CPU(dev);
@@ -211,14 +200,6 @@ static void cpu_common_realizefn(DeviceState *dev, 
Error **errp)
          cpu_resume(cpu);
      }

-    /* Plugin initialization must wait until the cpu start executing 
code */
-#ifdef CONFIG_PLUGIN
-    if (tcg_enabled()) {
-        cpu->accel->plugin_state = qemu_plugin_create_vcpu_state();
-        async_run_on_cpu(cpu, qemu_plugin_vcpu_init__async, 
RUN_ON_CPU_NULL);
-    }
-#endif
-
      /* NOTE: latest generic point where the cpu is fully realized */
  }

@@ -226,13 +207,6 @@ static void cpu_common_unrealizefn(DeviceState *dev)
  {
      CPUState *cpu = CPU(dev);

-    /* Call the plugin hook before clearing the cpu is fully unrealized */
-#ifdef CONFIG_PLUGIN
-    if (tcg_enabled()) {
-        qemu_plugin_vcpu_exit_hook(cpu);
-    }
-#endif
-
      /* NOTE: latest generic point before the cpu is fully unrealized */
      cpu_exec_unrealizefn(cpu);
  }
---

Totally untested here because it is late (only built...).


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2)
  2024-04-30 19:00     ` Philippe Mathieu-Daudé
@ 2024-04-30 21:42       ` Ilya Leoshkevich
  2024-05-02 10:27         ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 20+ messages in thread
From: Ilya Leoshkevich @ 2024-04-30 21:42 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Anton Johansson, Richard Henderson

On Tue, Apr 30, 2024 at 09:00:17PM +0200, Philippe Mathieu-Daudé wrote:
> On 30/4/24 20:45, Philippe Mathieu-Daudé wrote:
> > Hi Ilya,
> > 
> > On 30/4/24 19:55, Ilya Leoshkevich wrote:
> > > On Tue, Apr 30, 2024 at 02:27:54PM +0200, Philippe Mathieu-Daudé wrote:
> > > > Missing WASM testing by Ilya (branch available at
> > > > https://gitlab.com/philmd/qemu/-/commits/tcg_flush_jmp_cache)
> > > 
> > > Hmm, it dies very early now:
> > > 
> > >    # gdb --args ./qemu-s390x -L /usr/s390x-linux-gnu /build/wasmtime/target/s390x-unknown-linux-gnu/debug/deps/component_fuzz_util-d10a3a6b4ad8af47
> > > 
> > >    Thread 1 "qemu-s390x" received signal SIGSEGV, Segmentation fault.
> > >    0x000055555559b718 in cpu_common_realizefn (dev=0x5555557c28c0,
> > > errp=<optimized out>) at
> > > ../home/iii/myrepos/qemu/hw/core/cpu-common.c:217
> > >    217             cpu->accel->plugin_state =
> > > qemu_plugin_create_vcpu_state();
> > > 
> > >    (gdb) bt
> > >    #0  0x000055555559b718 in cpu_common_realizefn
> > > (dev=0x5555557c28c0, errp=<optimized out>) at
> > > ../home/iii/myrepos/qemu/hw/core/cpu-common.c:217
> > >    #1  0x000055555559f59a in s390_cpu_realizefn (dev=0x5555557c28c0,
> > > errp=0x7fffffffe1a0) at
> > > ../home/iii/myrepos/qemu/target/s390x/cpu.c:284
> > >    #2  0x000055555563f76b in device_set_realized (obj=<optimized
> > > out>, value=<optimized out>, errp=0x7fffffffe2e0) at
> > > ../home/iii/myrepos/qemu/hw/core/qdev.c:510
> > >    #3  0x000055555564363d in property_set_bool (obj=0x5555557c28c0,
> > > v=<optimized out>, name=<optimized out>, opaque=0x5555557a9140,
> > > errp=0x7fffffffe2e0) at ../home/iii/myrepos/qemu/qom/object.c:2362
> > >    #4  0x0000555555646b9b in object_property_set
> > > (obj=obj@entry=0x5555557c28c0, name=name@entry=0x5555556e8ae2
> > > "realized", v=v@entry=0x5555557c6650,
> > > errp=errp@entry=0x7fffffffe2e0)
> > >        at ../home/iii/myrepos/qemu/qom/object.c:1471
> > >    #5  0x000055555564a43f in object_property_set_qobject
> > > (obj=obj@entry=0x5555557c28c0, name=name@entry=0x5555556e8ae2
> > > "realized", value=value@entry=0x5555557a7a90,
> > > errp=errp@entry=0x7fffffffe2e0)
> > >        at ../home/iii/myrepos/qemu/qom/qom-qobject.c:28
> > >    #6  0x0000555555647204 in object_property_set_bool
> > > (obj=0x5555557c28c0, name=name@entry=0x5555556e8ae2 "realized",
> > > value=value@entry=true, errp=errp@entry=0x7fffffffe2e0)
> > >        at ../home/iii/myrepos/qemu/qom/object.c:1541
> > >    #7  0x000055555564025c in qdev_realize (dev=<optimized out>,
> > > bus=bus@entry=0x0, errp=errp@entry=0x7fffffffe2e0) at
> > > ../home/iii/myrepos/qemu/hw/core/qdev.c:291
> > >    #8  0x000055555559bbb4 in cpu_create (typename=<optimized out>)
> > > at ../home/iii/myrepos/qemu/hw/core/cpu-common.c:61
> > >    #9  0x000055555559a467 in main (argc=4, argv=0x7fffffffeaa8,
> > > envp=<optimized out>) at
> > > ../home/iii/myrepos/qemu/linux-user/main.c:811
> > > 
> > >    (gdb) p cpu
> > >    $1 = (CPUState *) 0x5555557c28c0
> > >    (gdb) p cpu->accel
> > >    $2 = (AccelCPUState *) 0x0
> > > 
> > > Configured with: '/home/iii/myrepos/qemu/configure'
> > > '--target-list=s390x-linux-user' '--disable-tools' '--disable-slirp'
> > > '--disable-fdt' '--disable-capstone' '--disable-docs'
> > > 
> > > If you don't see what can be wrong here right away, I can debug this.
> 
> I added this commit in the same branch:
> 
> -- >8 --
> Author: Philippe Mathieu-Daudé <philmd@linaro.org>
> Date:   Tue Apr 30 20:57:15 2024 +0200
> 
>     accel/tcg: Initialize TCG plugins in cpu-target.c
> 
>     Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> diff --git a/cpu-target.c b/cpu-target.c
> index 5af120e8aa..585533cfa3 100644
> --- a/cpu-target.c
> +++ b/cpu-target.c
> @@ -46,6 +46,10 @@
>  #include "hw/core/accel-cpu.h"
>  #include "trace/trace-root.h"
>  #include "qemu/accel.h"
> +#ifdef CONFIG_PLUGIN
> +#include "accel/tcg/vcpu-state.h"
> +#include "qemu/plugin.h"
> +#endif
> 
>  #ifndef CONFIG_USER_ONLY
>  static int cpu_common_post_load(void *opaque, int version_id)
> @@ -131,6 +135,13 @@ const VMStateDescription vmstate_cpu_common = {
>  };
>  #endif
> 
> +#ifdef CONFIG_PLUGIN
> +static void qemu_plugin_vcpu_init__async(CPUState *cpu, run_on_cpu_data
> unused)
> +{
> +    qemu_plugin_vcpu_init_hook(cpu);
> +}
> +#endif
> +
>  bool cpu_exec_realizefn(CPUState *cpu, Error **errp)
>  {
>      /* cache the cpu class for the hotpath */
> @@ -143,6 +154,15 @@ bool cpu_exec_realizefn(CPUState *cpu, Error **errp)
>      /* Wait until cpu initialization complete before exposing cpu. */
>      cpu_list_add(cpu);
> 
> +#ifdef CONFIG_PLUGIN
> +    assert(cpu->accel);
> +    /* Plugin initialization must wait until the cpu start executing code
> */
> +    if (tcg_enabled()) {
> +        cpu->accel->plugin_state = qemu_plugin_create_vcpu_state();
> +        async_run_on_cpu(cpu, qemu_plugin_vcpu_init__async,
> RUN_ON_CPU_NULL);
> +    }
> +#endif
> +
>  #ifdef CONFIG_USER_ONLY
>      assert(qdev_get_vmsd(DEVICE(cpu)) == NULL ||
>             qdev_get_vmsd(DEVICE(cpu))->unmigratable);
> @@ -171,6 +191,13 @@ void cpu_exec_unrealizefn(CPUState *cpu)
>      }
>  #endif
> 
> +#ifdef CONFIG_PLUGIN
> +    /* Call the plugin hook before clearing the cpu is fully unrealized */
> +    if (tcg_enabled()) {
> +        qemu_plugin_vcpu_exit_hook(cpu);
> +    }
> +#endif
> +
>      cpu_list_remove(cpu);
>      /*
>       * Now that the vCPU has been removed from the RCU list, we can call
> diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
> index e03d31876f..cd8bd99131 100644
> --- a/hw/core/cpu-common.c
> +++ b/hw/core/cpu-common.c
> @@ -30,10 +30,6 @@
>  #include "hw/boards.h"
>  #include "hw/qdev-properties.h"
>  #include "trace.h"
> -#ifdef CONFIG_PLUGIN
> -#include "accel/tcg/vcpu-state.h"
> -#include "qemu/plugin.h"
> -#endif
> 
>  CPUState *cpu_by_arch_id(int64_t id)
>  {
> @@ -181,13 +177,6 @@ static void cpu_common_parse_features(const char
> *typename, char *features,
>      }
>  }
> 
> -#ifdef CONFIG_PLUGIN
> -static void qemu_plugin_vcpu_init__async(CPUState *cpu, run_on_cpu_data
> unused)
> -{
> -    qemu_plugin_vcpu_init_hook(cpu);
> -}
> -#endif
> -
>  static void cpu_common_realizefn(DeviceState *dev, Error **errp)
>  {
>      CPUState *cpu = CPU(dev);
> @@ -211,14 +200,6 @@ static void cpu_common_realizefn(DeviceState *dev,
> Error **errp)
>          cpu_resume(cpu);
>      }
> 
> -    /* Plugin initialization must wait until the cpu start executing code
> */
> -#ifdef CONFIG_PLUGIN
> -    if (tcg_enabled()) {
> -        cpu->accel->plugin_state = qemu_plugin_create_vcpu_state();
> -        async_run_on_cpu(cpu, qemu_plugin_vcpu_init__async,
> RUN_ON_CPU_NULL);
> -    }
> -#endif
> -
>      /* NOTE: latest generic point where the cpu is fully realized */
>  }
> 
> @@ -226,13 +207,6 @@ static void cpu_common_unrealizefn(DeviceState *dev)
>  {
>      CPUState *cpu = CPU(dev);
> 
> -    /* Call the plugin hook before clearing the cpu is fully unrealized */
> -#ifdef CONFIG_PLUGIN
> -    if (tcg_enabled()) {
> -        qemu_plugin_vcpu_exit_hook(cpu);
> -    }
> -#endif
> -
>      /* NOTE: latest generic point before the cpu is fully unrealized */
>      cpu_exec_unrealizefn(cpu);
>  }
> ---
> 
> Totally untested here because it is late (only built...).

Now I get:

  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 "../home/iii/myrepos/qemu/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 "../home/iii/myrepos/qemu/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 ../home/iii/myrepos/qemu/cpu-target.c:158
  #7  0x000055555559f50b in s390_cpu_realizefn (dev=0x5555557c28c0, errp=0x7fffffffe1a0) at ../home/iii/myrepos/qemu/target/s390x/cpu.c:261
  #8  0x000055555563f78b in device_set_realized (obj=<optimized out>, value=<optimized out>, errp=0x7fffffffe2e0) at ../home/iii/myrepos/qemu/hw/core/qdev.c:510
  #9  0x000055555564365d in property_set_bool (obj=0x5555557c28c0, v=<optimized out>, name=<optimized out>, opaque=0x5555557a9140, errp=0x7fffffffe2e0) at ../home/iii/myrepos/qemu/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 ../home/iii/myrepos/qemu/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 ../home/iii/myrepos/qemu/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 ../home/iii/myrepos/qemu/qom/object.c:1541
  #13 0x000055555564027c in qdev_realize (dev=<optimized out>, bus=bus@entry=0x0, errp=errp@entry=0x7fffffffe2e0) at ../home/iii/myrepos/qemu/hw/core/qdev.c:291
  #14 0x000055555559bb54 in cpu_create (typename=<optimized out>) at ../home/iii/myrepos/qemu/hw/core/cpu-common.c:57
  #15 0x000055555559a467 in main (argc=4, argv=0x7fffffffeaa8, envp=<optimized out>) at ../home/iii/myrepos/qemu/linux-user/main.c:811

Here is the executable file: http://0x0.st/XXHp.gz
sha256sum: 58eb8d2a90c08f772ae94e20a7a8c7567bd886fe022a6b9e117912cc13acbd82

Best regards,
Ilya


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2)
  2024-04-30 21:42       ` Ilya Leoshkevich
@ 2024-05-02 10:27         ` Philippe Mathieu-Daudé
  2024-05-02 13:35           ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-02 10:27 UTC (permalink / raw)
  To: Ilya Leoshkevich, qemu-devel, Richard Henderson
  Cc: Anton Johansson, Peter Maydell

On 30/4/24 23:42, Ilya Leoshkevich wrote:
> On Tue, Apr 30, 2024 at 09:00:17PM +0200, Philippe Mathieu-Daudé wrote:
>> On 30/4/24 20:45, Philippe Mathieu-Daudé wrote:
>>> Hi Ilya,
>>>
>>> On 30/4/24 19:55, Ilya Leoshkevich wrote:
>>>> On Tue, Apr 30, 2024 at 02:27:54PM +0200, Philippe Mathieu-Daudé wrote:
>>>>> Missing WASM testing by Ilya (branch available at
>>>>> https://gitlab.com/philmd/qemu/-/commits/tcg_flush_jmp_cache)
>>>>
>>>> Hmm, it dies very early now:
>>>>
>>>>     # gdb --args ./qemu-s390x -L /usr/s390x-linux-gnu /build/wasmtime/target/s390x-unknown-linux-gnu/debug/deps/component_fuzz_util-d10a3a6b4ad8af47
>>>>
>>>>     Thread 1 "qemu-s390x" received signal SIGSEGV, Segmentation fault.
>>>>     0x000055555559b718 in cpu_common_realizefn (dev=0x5555557c28c0,
>>>> errp=<optimized out>) at
>>>> ../home/iii/myrepos/qemu/hw/core/cpu-common.c:217
>>>>     217             cpu->accel->plugin_state =
>>>> qemu_plugin_create_vcpu_state();
>>>>
>>>>     (gdb) bt
>>>>     #0  0x000055555559b718 in cpu_common_realizefn
>>>> (dev=0x5555557c28c0, errp=<optimized out>) at
>>>> ../home/iii/myrepos/qemu/hw/core/cpu-common.c:217
>>>>     #1  0x000055555559f59a in s390_cpu_realizefn (dev=0x5555557c28c0,
>>>> errp=0x7fffffffe1a0) at
>>>> ../home/iii/myrepos/qemu/target/s390x/cpu.c:284
>>>>     #2  0x000055555563f76b in device_set_realized (obj=<optimized
>>>> out>, value=<optimized out>, errp=0x7fffffffe2e0) at
>>>> ../home/iii/myrepos/qemu/hw/core/qdev.c:510
>>>>     #3  0x000055555564363d in property_set_bool (obj=0x5555557c28c0,
>>>> v=<optimized out>, name=<optimized out>, opaque=0x5555557a9140,
>>>> errp=0x7fffffffe2e0) at ../home/iii/myrepos/qemu/qom/object.c:2362
>>>>     #4  0x0000555555646b9b in object_property_set
>>>> (obj=obj@entry=0x5555557c28c0, name=name@entry=0x5555556e8ae2
>>>> "realized", v=v@entry=0x5555557c6650,
>>>> errp=errp@entry=0x7fffffffe2e0)
>>>>         at ../home/iii/myrepos/qemu/qom/object.c:1471
>>>>     #5  0x000055555564a43f in object_property_set_qobject
>>>> (obj=obj@entry=0x5555557c28c0, name=name@entry=0x5555556e8ae2
>>>> "realized", value=value@entry=0x5555557a7a90,
>>>> errp=errp@entry=0x7fffffffe2e0)
>>>>         at ../home/iii/myrepos/qemu/qom/qom-qobject.c:28
>>>>     #6  0x0000555555647204 in object_property_set_bool
>>>> (obj=0x5555557c28c0, name=name@entry=0x5555556e8ae2 "realized",
>>>> value=value@entry=true, errp=errp@entry=0x7fffffffe2e0)
>>>>         at ../home/iii/myrepos/qemu/qom/object.c:1541
>>>>     #7  0x000055555564025c in qdev_realize (dev=<optimized out>,
>>>> bus=bus@entry=0x0, errp=errp@entry=0x7fffffffe2e0) at
>>>> ../home/iii/myrepos/qemu/hw/core/qdev.c:291
>>>>     #8  0x000055555559bbb4 in cpu_create (typename=<optimized out>)
>>>> at ../home/iii/myrepos/qemu/hw/core/cpu-common.c:61
>>>>     #9  0x000055555559a467 in main (argc=4, argv=0x7fffffffeaa8,
>>>> envp=<optimized out>) at
>>>> ../home/iii/myrepos/qemu/linux-user/main.c:811
>>>>
>>>>     (gdb) p cpu
>>>>     $1 = (CPUState *) 0x5555557c28c0
>>>>     (gdb) p cpu->accel
>>>>     $2 = (AccelCPUState *) 0x0
>>>>
>>>> Configured with: '/home/iii/myrepos/qemu/configure'
>>>> '--target-list=s390x-linux-user' '--disable-tools' '--disable-slirp'
>>>> '--disable-fdt' '--disable-capstone' '--disable-docs'
>>>>


> Now I get:
> 
>    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 "../home/iii/myrepos/qemu/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 "../home/iii/myrepos/qemu/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 ../home/iii/myrepos/qemu/cpu-target.c:158
>    #7  0x000055555559f50b in s390_cpu_realizefn (dev=0x5555557c28c0, errp=0x7fffffffe1a0) at ../home/iii/myrepos/qemu/target/s390x/cpu.c:261
>    #8  0x000055555563f78b in device_set_realized (obj=<optimized out>, value=<optimized out>, errp=0x7fffffffe2e0) at ../home/iii/myrepos/qemu/hw/core/qdev.c:510
>    #9  0x000055555564365d in property_set_bool (obj=0x5555557c28c0, v=<optimized out>, name=<optimized out>, opaque=0x5555557a9140, errp=0x7fffffffe2e0) at ../home/iii/myrepos/qemu/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 ../home/iii/myrepos/qemu/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 ../home/iii/myrepos/qemu/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 ../home/iii/myrepos/qemu/qom/object.c:1541
>    #13 0x000055555564027c in qdev_realize (dev=<optimized out>, bus=bus@entry=0x0, errp=errp@entry=0x7fffffffe2e0) at ../home/iii/myrepos/qemu/hw/core/qdev.c:291
>    #14 0x000055555559bb54 in cpu_create (typename=<optimized out>) at ../home/iii/myrepos/qemu/hw/core/cpu-common.c:57
>    #15 0x000055555559a467 in main (argc=4, argv=0x7fffffffeaa8, envp=<optimized out>) at ../home/iii/myrepos/qemu/linux-user/main.c:811

 From code review I think the problem is my commit bb6cf6f016
("accel/tcg: Factor tcg_cpu_reset_hold() out") which wanted
to restrict tlb_flush() to system emulation, but inadvertently
also restricted tcg_flush_jmp_cache(), which was before called
via Realize -> Reset -> cpu_common_reset_hold(). Apparently
now this code can't happen on user emulation.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2)
  2024-05-02 10:27         ` Philippe Mathieu-Daudé
@ 2024-05-02 13:35           ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-02 13:35 UTC (permalink / raw)
  To: Ilya Leoshkevich, qemu-devel, Richard Henderson
  Cc: Anton Johansson, Peter Maydell

On 2/5/24 12:27, Philippe Mathieu-Daudé wrote:
> On 30/4/24 23:42, Ilya Leoshkevich wrote:
>> On Tue, Apr 30, 2024 at 09:00:17PM +0200, Philippe Mathieu-Daudé wrote:
>>> On 30/4/24 20:45, Philippe Mathieu-Daudé wrote:
>>>> Hi Ilya,
>>>>
>>>> On 30/4/24 19:55, Ilya Leoshkevich wrote:
>>>>> On Tue, Apr 30, 2024 at 02:27:54PM +0200, Philippe Mathieu-Daudé 
>>>>> wrote:
>>>>>> Missing WASM testing by Ilya (branch available at
>>>>>> https://gitlab.com/philmd/qemu/-/commits/tcg_flush_jmp_cache)
>>>>>
>>>>> Hmm, it dies very early now:
>>>>>
>>>>>     # gdb --args ./qemu-s390x -L /usr/s390x-linux-gnu 
>>>>> /build/wasmtime/target/s390x-unknown-linux-gnu/debug/deps/component_fuzz_util-d10a3a6b4ad8af47
>>>>>
>>>>>     Thread 1 "qemu-s390x" received signal SIGSEGV, Segmentation fault.
>>>>>     0x000055555559b718 in cpu_common_realizefn (dev=0x5555557c28c0,
>>>>> errp=<optimized out>) at
>>>>> ../home/iii/myrepos/qemu/hw/core/cpu-common.c:217
>>>>>     217             cpu->accel->plugin_state =
>>>>> qemu_plugin_create_vcpu_state();
>>>>>
>>>>>     (gdb) bt
>>>>>     #0  0x000055555559b718 in cpu_common_realizefn
>>>>> (dev=0x5555557c28c0, errp=<optimized out>) at
>>>>> ../home/iii/myrepos/qemu/hw/core/cpu-common.c:217
>>>>>     #1  0x000055555559f59a in s390_cpu_realizefn (dev=0x5555557c28c0,
>>>>> errp=0x7fffffffe1a0) at


>>>>>     (gdb) p cpu
>>>>>     $1 = (CPUState *) 0x5555557c28c0
>>>>>     (gdb) p cpu->accel
>>>>>     $2 = (AccelCPUState *) 0x0
>>>>>
>>>>> Configured with: '/home/iii/myrepos/qemu/configure'
>>>>> '--target-list=s390x-linux-user' '--disable-tools' '--disable-slirp'
>>>>> '--disable-fdt' '--disable-capstone' '--disable-docs'
>>>>>
> 
> 
>> Now I get:
>>
>>    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 
>> "../home/iii/myrepos/qemu/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 
>> "../home/iii/myrepos/qemu/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 
>> ../home/iii/myrepos/qemu/cpu-target.c:158
>>    #7  0x000055555559f50b in s390_cpu_realizefn (dev=0x5555557c28c0, 
>> errp=0x7fffffffe1a0) at ../home/iii/myrepos/qemu/target/s390x/cpu.c:261

>  From code review I think the problem is my commit bb6cf6f016
> ("accel/tcg: Factor tcg_cpu_reset_hold() out") which wanted
> to restrict tlb_flush() to system emulation, but inadvertently
> also restricted tcg_flush_jmp_cache(), which was before called
> via Realize -> Reset -> cpu_common_reset_hold(). Apparently
> now this code can't happen on user emulation.

This is indeed the root cause, I'll post a series fixing it.


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2024-05-02 13:35 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-30 12:27 [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2) Philippe Mathieu-Daudé
2024-04-30 12:27 ` [PATCH v3 01/13] accel/tcg: Restrict qemu_plugin_vcpu_exit_hook() to TCG plugins Philippe Mathieu-Daudé
2024-04-30 12:27 ` [PATCH v3 02/13] accel/tcg: Restrict cpu_plugin_mem_cbs_enabled() to TCG Philippe Mathieu-Daudé
2024-04-30 12:27 ` [PATCH v3 03/13] accel/tcg: Move @plugin_mem_cbs from CPUState to CPUNegativeOffsetState Philippe Mathieu-Daudé
2024-04-30 12:27 ` [PATCH v3 04/13] accel/tcg: Move @plugin_state from CPUState to TCG AccelCPUState Philippe Mathieu-Daudé
2024-04-30 12:27 ` [PATCH v3 05/13] accel/tcg: Restrict cpu_loop_exit_requested() to TCG Philippe Mathieu-Daudé
2024-04-30 12:28 ` [PATCH v3 06/13] accel/tcg: Restrict IcountDecr / can_do_io / CPUTLB " Philippe Mathieu-Daudé
2024-04-30 12:28 ` [PATCH v3 07/13] accel/tcg: Move @jmp_env from CPUState to TCG AccelCPUState Philippe Mathieu-Daudé
2024-04-30 12:28 ` [PATCH v3 08/13] accel/tcg: Move @cflags_next_tb " Philippe Mathieu-Daudé
2024-04-30 12:28 ` [PATCH v3 09/13] accel/tcg: Move @iommu_notifiers " Philippe Mathieu-Daudé
2024-04-30 12:28 ` [PATCH v3 10/13] accel/tcg: Move @tcg_cflags " Philippe Mathieu-Daudé
2024-04-30 12:28 ` [PATCH v3 11/13] accel/tcg: Restrict icount to system emulation Philippe Mathieu-Daudé
2024-04-30 12:28 ` [PATCH v3 12/13] accel/tcg: Move icount fields from CPUState to TCG AccelCPUState Philippe Mathieu-Daudé
2024-04-30 12:28 ` [PATCH v3 13/13] accel/tcg: Move @tb_jmp_cache " Philippe Mathieu-Daudé
2024-04-30 17:55 ` [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2) Ilya Leoshkevich
2024-04-30 18:45   ` Philippe Mathieu-Daudé
2024-04-30 19:00     ` Philippe Mathieu-Daudé
2024-04-30 21:42       ` Ilya Leoshkevich
2024-05-02 10:27         ` Philippe Mathieu-Daudé
2024-05-02 13:35           ` Philippe Mathieu-Daudé

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