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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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