On 27/09/2022 13.06, Bin Meng wrote:
> From: Xuzhou Cheng <xuzhou.cheng@windriver.com>
>
> Currently signal SIGIPI [=SIGUSR1] is used to kick the dummy CPU
> when qtest accelerator is used. However SIGUSR1 is unsupported on
> Windows. To support Windows, we add a QemuSemaphore CPUState::sem
> to kick the dummy CPU instead.
>
> As a result of this, the POSIX implementation via signal is no
> longer needed and can use the same path as Windows.
>
> Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Replace signal by the semaphore on posix too
This does not work - it breaks running with KVM on Linux. When I add this
patch, I get:
$ ./qemu-system-x86_64 -machine pc -smp 4 -enable-kvm
qemu-system-x86_64: ../../devel/qemu/util/qemu-thread-posix.c:86:
qemu_mutex_lock_impl: Assertion `mutex->initialized' failed.
Aborted (core dumped)
I think "SIG_IPI" is used for kicking a CPU out of the kernel mode there, so
you cannot simply replace it wrt KVM.
Thomas
> include/hw/core/cpu.h | 1 +
> accel/dummy-cpus.c | 15 ++-------------
> softmmu/cpus.c | 10 +---------
> accel/meson.build | 1 +
> accel/qtest/meson.build | 1 +
> 5 files changed, 6 insertions(+), 22 deletions(-)
>
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 500503da13..2f46c37dc1 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -326,6 +326,7 @@ struct CPUState {
> #ifdef _WIN32
> HANDLE hThread;
> #endif
> + QemuSemaphore sem;
> int thread_id;
> bool running, has_waiter;
> struct QemuCond *halt_cond;
> diff --git a/accel/dummy-cpus.c b/accel/dummy-cpus.c
> index 10429fdfb2..3769d3db0a 100644
> --- a/accel/dummy-cpus.c
> +++ b/accel/dummy-cpus.c
> @@ -21,8 +21,6 @@
> static void *dummy_cpu_thread_fn(void *arg)
> {
> CPUState *cpu = arg;
> - sigset_t waitset;
> - int r;
>
> rcu_register_thread();
>
> @@ -32,23 +30,13 @@ static void *dummy_cpu_thread_fn(void *arg)
> cpu->can_do_io = 1;
> current_cpu = cpu;
>
> - sigemptyset(&waitset);
> - sigaddset(&waitset, SIG_IPI);
> -
> /* signal CPU creation */
> cpu_thread_signal_created(cpu);
> qemu_guest_random_seed_thread_part2(cpu->random_seed);
>
> do {
> qemu_mutex_unlock_iothread();
> - do {
> - int sig;
> - r = sigwait(&waitset, &sig);
> - } while (r == -1 && (errno == EAGAIN || errno == EINTR));
> - if (r == -1) {
> - perror("sigwait");
> - exit(1);
> - }
> + qemu_sem_wait(&cpu->sem);
> qemu_mutex_lock_iothread();
> qemu_wait_io_event(cpu);
> } while (!cpu->unplug);
> @@ -67,6 +55,7 @@ void dummy_start_vcpu_thread(CPUState *cpu)
> qemu_cond_init(cpu->halt_cond);
> snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/DUMMY",
> cpu->cpu_index);
> + qemu_sem_init(&cpu->sem, 0);
> qemu_thread_create(cpu->thread, thread_name, dummy_cpu_thread_fn, cpu,
> QEMU_THREAD_JOINABLE);
> }
> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
> index 23b30484b2..2a992d0d5f 100644
> --- a/softmmu/cpus.c
> +++ b/softmmu/cpus.c
> @@ -437,19 +437,11 @@ void qemu_wait_io_event(CPUState *cpu)
>
> void cpus_kick_thread(CPUState *cpu)
> {
> -#ifndef _WIN32
> - int err;
> -
> if (cpu->thread_kicked) {
> return;
> }
> cpu->thread_kicked = true;
> - err = pthread_kill(cpu->thread->thread, SIG_IPI);
> - if (err && err != ESRCH) {
> - fprintf(stderr, "qemu:%s: %s", __func__, strerror(err));
> - exit(1);
> - }
> -#endif
> + qemu_sem_post(&cpu->sem);
> }
>
> void qemu_cpu_kick(CPUState *cpu)
> diff --git a/accel/meson.build b/accel/meson.build
> index b9a963cf80..b21c85dc0a 100644
> --- a/accel/meson.build
> +++ b/accel/meson.build
> @@ -17,4 +17,5 @@ dummy_ss.add(files(
> ))
>
> specific_ss.add_all(when: ['CONFIG_SOFTMMU', 'CONFIG_POSIX'], if_true: dummy_ss)
> +specific_ss.add_all(when: ['CONFIG_WIN32'], if_true: dummy_ss)
> specific_ss.add_all(when: ['CONFIG_XEN'], if_true: dummy_ss)
> diff --git a/accel/qtest/meson.build b/accel/qtest/meson.build
> index 4c65600293..a4876fc0f2 100644
> --- a/accel/qtest/meson.build
> +++ b/accel/qtest/meson.build
> @@ -1,2 +1,3 @@
> qtest_module_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_POSIX'],
> if_true: files('qtest.c'))
> +qtest_module_ss.add(when: ['CONFIG_WIN32'], if_true: files('qtest.c'))