cpus: make pause_all_cpus() play with SMP on single threaded TCG
PATCH 1/1 cpus: make pause_all_cpus() play with SMP on single threaded TCG
pause_all_cpus() is sometimes called from a VCPU thread (e.g. s390x during special reset). It cannot deal with multiple VCPUs per Thread (single threaded TCG) yet.
Booting an s390x guest with -smp 2 and single threaded TCG from disk currently fails. The DIAG 308 will issue a pause_all_cpus() and wait forever for the CPUs to actually stop. But it is waiting for itself.
So let's stop all VCPUs belonging to the current thread. Factor out stopping of a VCPU.
--- cpus.c | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-)
cpus.c +19 -13
@@ -1057,13 +1057,22 @@ static void qemu_tcg_destroy_vcpu(CPUState *cpu) { } +static void qemu_cpu_stop (CPUState *cpu,bool exit) +{ +g_assert (qemu_cpu_is_self (cpu)); + cpu->stop = false; + cpu->stopped = true; +if (exit) { +cpu_exit (cpu); + } +qemu_cond_broadcast (&qemu_pause_cond); +} +static void qemu_wait_io_event_common (CPUState *cpu) {atomic_mb_set (&cpu->thread_kicked, false);if (cpu->stop) { - cpu->stop = false; - cpu->stopped = true; -qemu_cond_broadcast (&qemu_pause_cond); +qemu_cpu_stop (cpu, false); }process_queued_cpu_work (cpu); }
@@ -1610,12 +1619,12 @@ void pause_all_vcpus(void)qemu_clock_enable (QEMU_CLOCK_VIRTUAL , false);CPU_FOREACH (cpu) { - cpu->stop = true; -qemu_cpu_kick (cpu); - } - -if (qemu_in_vcpu_thread ()) { -cpu_stop_current (); +if (qemu_cpu_is_self (cpu)) { +qemu_cpu_stop (cpu, true); + }else { + cpu->stop = true; +qemu_cpu_kick (cpu); + } }while (!all_vcpus_paused ()) {
@@ -1799,10 +1808,7 @@ void qemu_init_vcpu(CPUState *cpu)void cpu_stop_current (void ) {if (current_cpu) { - current_cpu->stop = false; - current_cpu->stopped = true; -cpu_exit (current_cpu); -qemu_cond_broadcast (&qemu_pause_cond); +qemu_cpu_stop (current_cpu, true); } }
Signature · 1 line
2.14.3
Quoted 66 lines · David Hildenbrand
pause_all_cpus()is sometimes called from a VCPU thread (e.g. s390x during special reset). It cannot deal with multiple VCPUs per Thread (single threaded TCG) yet.Booting an s390x guest with -smp 2 and single threaded TCG from disk currently fails. The DIAG 308 will issue a
pause_all_cpus()and wait forever for the CPUs to actually stop. But it is waiting for itself.So let's stop all VCPUs belonging to the current thread. Factor out stopping of a VCPU.
Signed-off-by David Hildenbrand <[email protected]>--- cpus.c | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-)cpus.c +18
-9@@ -1057,13 +1057,22 @@ static void qemu_tcg_destroy_vcpu(CPUState *cpu) { } +static void qemu_cpu_stop (CPUState *cpu,bool exit) +{ +g_assert (qemu_cpu_is_self (cpu)); + cpu->stop = false; + cpu->stopped = true; +if (exit) { +cpu_exit (cpu); + } +qemu_cond_broadcast (&qemu_pause_cond); +} +static void qemu_wait_io_event_common (CPUState *cpu) {atomic_mb_set (&cpu->thread_kicked, false);if (cpu->stop) { - cpu->stop = false; - cpu->stopped = true; -qemu_cond_broadcast (&qemu_pause_cond); +qemu_cpu_stop (cpu, false); }process_queued_cpu_work (cpu); }@@ -1610,12 +1619,12 @@ void pause_all_vcpus(void)qemu_clock_enable (QEMU_CLOCK_VIRTUAL , false);CPU_FOREACH (cpu) { - cpu->stop = true; -qemu_cpu_kick (cpu); - } - -if (qemu_in_vcpu_thread ()) { -cpu_stop_current (); +if (qemu_cpu_is_self (cpu)) { +qemu_cpu_stop (cpu, true); + }else { + cpu->stop = true; +qemu_cpu_kick (cpu); + } }while (!all_vcpus_paused ()) {
cpus.c void qemu_init_vcpu(CPUState *cpu) · line 1813
Quoted 10 lines
cpus.c +1
-4@@ -1799,10 +1808,7 @@ void qemu_init_vcpu(CPUState *cpu)void cpu_stop_current (void ) {if (current_cpu) { - current_cpu->stop = false; - current_cpu->stopped = true; -cpu_exit (current_cpu); -qemu_cond_broadcast (&qemu_pause_cond); +qemu_cpu_stop (current_cpu, true); } }
This function is not needed anymore, because vm_stop can just call qemu_cpu_stop. Does the following squash look fine to you?
cpus.c +6 -14
@@ -1057,14 +1057,11 @@ static void qemu_tcg_destroy_vcpu(CPUState *cpu) { } -static void qemu_cpu_stop (CPUState *cpu,bool exit) +static void qemu_cpu_stop (CPUState *cpu) {g_assert (qemu_cpu_is_self (cpu)); cpu->stop = false; cpu->stopped = true; -if (exit) { -cpu_exit (cpu); - }qemu_cond_broadcast (&qemu_pause_cond); }
@@ -1072,7 +1069,7 @@ static void qemu_wait_io_event_common(CPUState *cpu) {atomic_mb_set (&cpu->thread_kicked, false);if (cpu->stop) { -qemu_cpu_stop (cpu, false); +qemu_cpu_stop (cpu); }process_queued_cpu_work (cpu); }
@@ -1620,7 +1617,8 @@ void pause_all_vcpus(void)qemu_clock_enable (QEMU_CLOCK_VIRTUAL , false);CPU_FOREACH (cpu) {if (qemu_cpu_is_self (cpu)) { -qemu_cpu_stop (cpu, true); +qemu_cpu_stop (cpu); +cpu_exit (cpu); }else { cpu->stop = true;qemu_cpu_kick (cpu);
@@ -1802,13 +1800,6 @@ void qemu_init_vcpu(CPUState *cpu) } } -void cpu_stop_current (void ) -{ -if (current_cpu) { -qemu_cpu_stop (current_cpu, true); - } -} -int vm_stop (RunState state) {if (qemu_in_vcpu_thread ()) {
@@ -1818,7 +1809,8 @@ int vm_stop(RunState state)* FIXME: should not return to device code in case * vm_stop() has been requested. */ -cpu_stop_current (); +qemu_cpu_stop (current_cpu); +cpu_exit (current_cpu);return 0 ; }
cpus.c int vm_stop(RunState state) · line 1813
Quoted 11 lines
int
vm_stop(RunState state){ if (qemu_in_vcpu_thread()) {cpus.c +2
-1@@ -1818,7 +1809,8 @@ int vm_stop(RunState state)* FIXME: should not return to device code in case * vm_stop() has been requested. */ -cpu_stop_current (); +qemu_cpu_stop (current_cpu); +cpu_exit (current_cpu);
We're doing the cpu_exit() now after the broadcast, is this ok?
Also we drop the check for current_cpu, I assume this is also ok.
Quoted 3 lines
cpus.c +0
-0return 0 ; }
Signature · 3 lines
Thanks, David / dhildenb
cpus.c void qemu_init_vcpu(CPUState *cpu) · line 1810
Quoted 7 lines
cpus.c +0
-6-void cpu_stop_current (void ) -{ -if (current_cpu) { -qemu_cpu_stop (current_cpu, true); - } -}
Btw. this does not compile as this is used also in vl.c
Quoted 15 lines
cpus.c +2
-2-int vm_stop (RunState state) {if (qemu_in_vcpu_thread ()) {@@ -1818,7 +1809,8 @@ int vm_stop(RunState state)* FIXME: should not return to device code in case * vm_stop() has been requested. */ -cpu_stop_current (); +qemu_cpu_stop (current_cpu); +cpu_exit (current_cpu);return 0 ; }
Signature · 3 lines
Thanks, David / dhildenb
Quoted 8 lines · David Hildenbrand
cpus.c void qemu_init_vcpu(CPUState *cpu) · line 1810
Quoted 6 lines
cpus.c +0
-6-void cpu_stop_current (void ) -{ -if (current_cpu) { -qemu_cpu_stop (current_cpu, true); - } -}Btw. this does not compile as this is used also in
vl.c
Doh, then I'm applying your patch untouched.
Paolo