kvm: qemu: stop/start cpus before/after devices
From: Yaniv Kamay <[email protected]>
Stop cpus before devices when stopping the VM, start cpus after devices when starting VM.
qemu/qemu-kvm.c +5 -14
@@ -285,7 +285,7 @@ static int all_threads_paused(void)return 1 ; } -static void pause_all_threads (void ) +void qemu_kvm_pause_all_threads (void ) {CPUState *penv = first_cpu;
@@ -305,7 +305,7 @@ static void pause_all_threads(void)qemu_cond_wait (&qemu_pause_cond); } -static void resume_all_threads (void ) +void qemu_kvm_resume_all_threads (void ) {CPUState *penv = first_cpu;
@@ -319,14 +319,6 @@ static void resume_all_threads(void) } } -static void kvm_vm_state_change_handler (void *context,int running,int reason) -{ -if (running) -resume_all_threads (); -else -pause_all_threads (); -} -static void update_regs_for_sipi (CPUState *env) {kvm_arch_update_regs_for_sipi (env);
@@ -371,7 +363,7 @@ static void qemu_kvm_system_reset(void) {CPUState *penv = first_cpu; -pause_all_threads (); +qemu_kvm_pause_all_threads ();qemu_system_reset ();
@@ -380,7 +372,7 @@ static void qemu_kvm_system_reset(void) penv = (CPUState *)penv->next_cpu; } -resume_all_threads (); +qemu_kvm_resume_all_threads (); }static int kvm_main_loop_cpu (CPUState *env)
@@ -466,7 +458,6 @@ int kvm_init_ap(void)#ifdef TARGET_I386 kvm_tpr_opt_setup ();#endif -qemu_add_vm_change_state_handler (kvm_vm_state_change_handler,NULL );signal (SIG_IPI , sig_ipi_handler);return 0 ;
@@ -610,7 +601,7 @@ int kvm_main_loop(void)#endif } -pause_all_threads (); +qemu_kvm_pause_all_threads ();pthread_mutex_unlock (&qemu_mutex);return 0 ;
qemu/qemu-kvm.h +3 -0
@@ -119,6 +119,9 @@ int qemu_kvm_register_coalesced_mmio(target_phys_addr_t addr,int qemu_kvm_unregister_coalesced_mmio (target_phys_addr_t addr,unsigned int size); +void qemu_kvm_pause_all_threads (void ); +void qemu_kvm_resume_all_threads (void ); +int kvm_coalesce_mmio_region (target_phys_addr_t start,ram_addr_t size);int kvm_uncoalesce_mmio_region (target_phys_addr_t start,ram_addr_t size);
qemu/vl.c +4 -0
@@ -3596,6 +3596,8 @@ void vm_start(void)cpu_enable_ticks (); vm_running =1 ;vm_state_notify (1 ,0 ); +if (kvm_enabled ()) +qemu_kvm_resume_all_threads ();qemu_rearm_alarm_timer (alarm_timer); } }
@@ -3605,6 +3607,8 @@ void vm_stop(int reason)if (vm_running) {cpu_disable_ticks (); vm_running =0 ; +if (kvm_enabled ()) +qemu_kvm_pause_all_threads ();vm_state_notify (0 , reason); } }
Quoted 6 lines · Marcelo Tosatti
From: Yaniv Kamay <[email protected]>
Stop cpus before devices when stopping the VM, start cpus after devices when starting VM.
Why is this needed?
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 9 lines · Avi Kivity
Quoted 6 lines · Marcelo Tosatti
From: Yaniv Kamay <[email protected]>
Stop cpus before devices when stopping the VM, start cpus after devices when starting VM.
Why is this needed?
A vcpu could access a stopped device otherwise.
Quoted 4 lines · Marcelo Tosatti
Quoted 3 lines · Avi Kivity
Quoted 2 lines · Marcelo Tosatti
From: Yaniv Kamay <[email protected]>
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Stop cpus before devices when stopping the VM, start cpus after devices when starting VM.
Why is this needed?
A vcpu could access a stopped device otherwise.
Actually on vm_stop its safe because the order happens to be correct, but on vm_start its the other way around (vcpus start first, and they should be started last).
Quoted 13 lines · Marcelo Tosatti
Quoted 11 lines · Marcelo Tosatti
Quoted 9 lines · Avi Kivity
Quoted 7 lines · Marcelo Tosatti
From: Yaniv Kamay <[email protected]>
Stop cpus before devices when stopping the VM, start cpus after devices when starting VM.
Quoted 9 lines
Quoted 4 lines
Quoted 2 lines
Why is this needed?
A vcpu could access a stopped device otherwise.
Actually on
vm_stopits safe because the order happens to be correct, but onvm_startits the other way around (vcpus start first, and they should be started last).
Right, applied the patch, thanks.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 13 lines · Marcelo Tosatti
Quoted 11 lines · Marcelo Tosatti
Quoted 9 lines · Avi Kivity
Quoted 7 lines · Marcelo Tosatti
From: Yaniv Kamay <[email protected]>
Stop cpus before devices when stopping the VM, start cpus after devices when starting VM.
Quoted 9 lines
Quoted 4 lines
Quoted 2 lines
Why is this needed?
A vcpu could access a stopped device otherwise.
Actually on
vm_stopits safe because the order happens to be correct, but onvm_startits the other way around (vcpus start first, and they should be started last).
But, we are holding qemu_mutex. How can vcpus access the devices?
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 15 lines · Avi Kivity
Quoted 14 lines · Marcelo Tosatti
Quoted 12 lines · Marcelo Tosatti
Quoted 10 lines · Avi Kivity
Quoted 6 lines · Marcelo Tosatti
From: Yaniv Kamay <[email protected]>
Stop cpus before devices when stopping the VM, start cpus after devices when starting VM.
Why is this needed?
Quoted 8 lines
Quoted 6 lines
Quoted 1 line
A vcpu could access a stopped device otherwise.
Actually on
vm_stopits safe because the order happens to be correct, but onvm_startits the other way around (vcpus start first, and they should be started last).But, we are holding
qemu_mutex. How can vcpus access the devices?
You're right, they can't. But its not bad to make it explicit, instead of relying on the locking behaviour?
Quoted 3 lines · Marcelo Tosatti
You're right, they can't. But its not bad to make it explicit, instead of relying on the locking behaviour?
You could say that we should depend on proper locking instead the order of things shutting down and starting up :)
It's not just a cpu that can access a device; another device could dma into it, so we need to quiesce the system first (including vcpus) and then pause all devices.
Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.