diff mbox series

[v3,03/12] plugins: Check if vCPU is realized

Message ID 20230912224107.29669-4-akihiko.odaki@daynix.com
State New
Headers show
Series gdbstub and TCG plugin improvements | expand

Commit Message

Akihiko Odaki Sept. 12, 2023, 10:40 p.m. UTC
The created member of CPUState tells if the vCPU thread is started, and
will be always false for the user space emulation that manages threads
independently. Use the realized member of DeviceState, which is valid
for both of the system and user space emulation.

Fixes: 54cb65d858 ("plugin: add core code")
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 plugins/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Philippe Mathieu-Daudé Sept. 13, 2023, 6:17 a.m. UTC | #1
On 13/9/23 00:40, Akihiko Odaki wrote:
> The created member of CPUState tells if the vCPU thread is started, and
> will be always false for the user space emulation that manages threads
> independently.

Per the docstring:

  /**
   * CPUState:

   * @created: Indicates whether the CPU thread has been
   *           successfully created.

Each CPU DeviceClass's DeviceRealize() handler which calls
qemu_init_vcpu(). Ah, what we miss is:

-- >8 --
--- a/accel/tcg/user-exec-stub.c
+++ b/accel/tcg/user-exec-stub.c
@@ -14,6 +14,7 @@ void cpu_remove_sync(CPUState *cpu)

  void qemu_init_vcpu(CPUState *cpu)
  {
+    cpu->created = true;
  }
---

Missed in commit c7f0f3b1c8 ("qtest: add test framework").

Does that help?

> Use the realized member of DeviceState, which is valid
> for both of the system and user space emulation.
> 
> Fixes: 54cb65d858 ("plugin: add core code")
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>   plugins/core.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/plugins/core.c b/plugins/core.c
> index 3c4e26c7ed..fcd33a2bff 100644
> --- a/plugins/core.c
> +++ b/plugins/core.c
> @@ -64,7 +64,7 @@ static void plugin_cpu_update__locked(gpointer k, gpointer v, gpointer udata)
>       CPUState *cpu = container_of(k, CPUState, cpu_index);
>       run_on_cpu_data mask = RUN_ON_CPU_HOST_ULONG(*plugin.mask);
>   
> -    if (cpu->created) {
> +    if (DEVICE(cpu)->realized) {
>           async_run_on_cpu(cpu, plugin_cpu_update__async, mask);
>       } else {
>           plugin_cpu_update__async(cpu, mask);
Akihiko Odaki Sept. 14, 2023, 5:16 p.m. UTC | #2
On 2023/09/13 15:17, Philippe Mathieu-Daudé wrote:
> On 13/9/23 00:40, Akihiko Odaki wrote:
>> The created member of CPUState tells if the vCPU thread is started, and
>> will be always false for the user space emulation that manages threads
>> independently.
> 
> Per the docstring:
> 
>   /**
>    * CPUState:
> 
>    * @created: Indicates whether the CPU thread has been
>    *           successfully created.
> 
> Each CPU DeviceClass's DeviceRealize() handler which calls
> qemu_init_vcpu(). Ah, what we miss is:
> 
> -- >8 --
> --- a/accel/tcg/user-exec-stub.c
> +++ b/accel/tcg/user-exec-stub.c
> @@ -14,6 +14,7 @@ void cpu_remove_sync(CPUState *cpu)
> 
>   void qemu_init_vcpu(CPUState *cpu)
>   {
> +    cpu->created = true;
>   }
> ---
> 
> Missed in commit c7f0f3b1c8 ("qtest: add test framework").

I think the member is never set for user space emulation since it was 
introduced with commit d6dc3d424e ("qemu: introduce iothread (Marcelo 
Tosatti)")

> 
> Does that help?

That will work, but I don't think it makes sense to set this member 
while the other members related to vCPU thread like the "thread" member 
are unset.
diff mbox series

Patch

diff --git a/plugins/core.c b/plugins/core.c
index 3c4e26c7ed..fcd33a2bff 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -64,7 +64,7 @@  static void plugin_cpu_update__locked(gpointer k, gpointer v, gpointer udata)
     CPUState *cpu = container_of(k, CPUState, cpu_index);
     run_on_cpu_data mask = RUN_ON_CPU_HOST_ULONG(*plugin.mask);
 
-    if (cpu->created) {
+    if (DEVICE(cpu)->realized) {
         async_run_on_cpu(cpu, plugin_cpu_update__async, mask);
     } else {
         plugin_cpu_update__async(cpu, mask);