This ensures we don't start the thread until cpu_common_realizefn has finished. This ensures that plugins will always run qemu_plugin_vcpu_init__async first before any other states. It doesn't totally eliminate the race that plugin_cpu_update__locked has to work around though. I found this while reviewing the ips plugin which makes heavy use of the vcpu phase callbacks.
An alternative might be to move the explicit creation of vCPU threads to qdev_machine_creation_done()? It doesn't affect user-mode which already has a thread to execute in and ensures the QOM object has completed creation in cpu_create() before continuing. Signed-off-by: Alex Bennée <alex.ben...@linaro.org> Cc: Pierrick Bouvier <pierrick.bouv...@linaro.org> Cc: Philippe Mathieu-Daudé <phi...@linaro.org> --- include/hw/core/cpu.h | 8 ++++++++ accel/tcg/user-exec-stub.c | 5 +++++ hw/core/cpu-common.c | 7 ++++++- plugins/core.c | 5 +++++ system/cpus.c | 15 ++++++++++----- 5 files changed, 34 insertions(+), 6 deletions(-) diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index bb398e8237..6920699585 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -1041,6 +1041,14 @@ void end_exclusive(void); */ void qemu_init_vcpu(CPUState *cpu); +/** + * qemu_start_vcpu: + * @cpu: The vCPU to start. + * + * Create the vCPU thread and start it running. + */ +void qemu_start_vcpu(CPUState *cpu); + #define SSTEP_ENABLE 0x1 /* Enable simulated HW single stepping */ #define SSTEP_NOIRQ 0x2 /* Do not use IRQ while single stepping */ #define SSTEP_NOTIMER 0x4 /* Do not Timers while single stepping */ diff --git a/accel/tcg/user-exec-stub.c b/accel/tcg/user-exec-stub.c index 4fbe2dbdc8..162bb72bbe 100644 --- a/accel/tcg/user-exec-stub.c +++ b/accel/tcg/user-exec-stub.c @@ -18,6 +18,11 @@ void cpu_exec_reset_hold(CPUState *cpu) { } +void qemu_start_vcpu(CPUState *cpu) +{ + /* NOP for user-mode, we already have a thread */ +} + /* User mode emulation does not support record/replay yet. */ bool replay_exception(void) diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c index 0f0a247f56..68895ddd59 100644 --- a/hw/core/cpu-common.c +++ b/hw/core/cpu-common.c @@ -230,7 +230,12 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp) } #endif - /* NOTE: latest generic point where the cpu is fully realized */ + /* + * With everything set up we can finally start the vCPU thread. + * This is a NOP for linux-user. + * NOTE: latest generic point where the cpu is fully realized + */ + qemu_start_vcpu(cpu); } static void cpu_common_unrealizefn(DeviceState *dev) diff --git a/plugins/core.c b/plugins/core.c index 0726bc7f25..1e5da7853b 100644 --- a/plugins/core.c +++ b/plugins/core.c @@ -65,6 +65,11 @@ 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); + /* + * There is a race condition between the starting of the vCPU + * thread at the end of cpu_common_realizefn and when realized is + * finally set. + */ if (DEVICE(cpu)->realized) { async_run_on_cpu(cpu, plugin_cpu_update__async, mask); } else { diff --git a/system/cpus.c b/system/cpus.c index d3640c9503..7dd8464c5e 100644 --- a/system/cpus.c +++ b/system/cpus.c @@ -488,11 +488,13 @@ void cpus_kick_thread(CPUState *cpu) void qemu_cpu_kick(CPUState *cpu) { - qemu_cond_broadcast(cpu->halt_cond); - if (cpus_accel->kick_vcpu_thread) { - cpus_accel->kick_vcpu_thread(cpu); - } else { /* default */ - cpus_kick_thread(cpu); + if (cpu->halt_cond) { + qemu_cond_broadcast(cpu->halt_cond); + if (cpus_accel->kick_vcpu_thread) { + cpus_accel->kick_vcpu_thread(cpu); + } else { /* default */ + cpus_kick_thread(cpu); + } } } @@ -674,7 +676,10 @@ void qemu_init_vcpu(CPUState *cpu) cpu->num_ases = 1; cpu_address_space_init(cpu, 0, "cpu-memory", cpu->memory); } +} +void qemu_start_vcpu(CPUState *cpu) +{ /* accelerators all implement the AccelOpsClass */ g_assert(cpus_accel != NULL && cpus_accel->create_vcpu_thread != NULL); cpus_accel->create_vcpu_thread(cpu); -- 2.39.2