Re: [PATCH v3 00/13] exec: Rework around CPUState user fields (part 2)

[Philippe Mathieu-Daudé]

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