qemu-kvm: Fix guest resetting v2
PATCH 2/2 qemu-kvm: Fix guest resetting v2
Resetting guests used to be racy, deadlock-prone, or simply broken (for SMP). This patch fixes the issues, following Marcelo's suggestion to consolidate the reset activity in the I/O thread. All vcpus are cleanly stopped before the emulated hardware is reset, and kvm_arch_cpu_reset is introduced and invoked to ensure that non-boot cpus are put into the right state on x86. Note that other arch may need to look into this service as well to get SMP reset right.
Moreover, a safety check is added to pause/resume_all_threads to ensure that they are only invoked in I/O thread context.
--- qemu/qemu-kvm-ia64.c | 4 ++++ qemu/qemu-kvm-powerpc.c | 4 ++++ qemu/qemu-kvm-x86.c | 16 ++++++++++++++++ qemu/qemu-kvm.c | 34 +++++++++++++++++----------------- qemu/qemu-kvm.h | 1 + qemu/vl.c | 6 +----- 6 files changed, 43 insertions(+), 22 deletions(-)
qemu/qemu-kvm-ia64.c +4 -0
@@ -61,3 +61,7 @@ int kvm_arch_try_push_interrupts(void *ovoid kvm_arch_update_regs_for_sipi (CPUState *env) { } + +void kvm_arch_cpu_reset (CPUState *env) +{ +}
qemu/qemu-kvm-powerpc.c +4 -0
@@ -213,3 +213,7 @@ int handle_powerpc_dcr_write(int vcpu, ureturn 0 ;/* XXX ignore failed DCR ops */ } + +void kvm_arch_cpu_reset (CPUState *env) +{ +}
qemu/qemu-kvm-x86.c +16 -0
@@ -671,3 +671,19 @@ int handle_tpr_access(void *opaque, intkvm_tpr_access_report (cpu_single_env, rip, is_write);return 0 ; } + +void kvm_arch_cpu_reset (CPUState *env) +{ +struct kvm_mp_state mp_state = { .mp_state =KVM_MP_STATE_UNINITIALIZED }; + +kvm_arch_load_regs (env); +if (env->cpu_index !=0 ) { +if (kvm_irqchip_in_kernel (kvm_context)) +kvm_set_mpstate (kvm_context, env->cpu_index, &mp_state); +else { + env->interrupt_request &= ~CPU_INTERRUPT_HARD ; + env->hflags |=HF_HALTED_MASK ; + env->exception_index =EXCP_HLT ; + } + } +}
qemu/qemu-kvm.c +17 -17
@@ -32,8 +32,6 @@ kvm_context_t kvm_context;extern int smp_cpus; -static int qemu_kvm_reset_requested; -pthread_mutex_t qemu_mutex =PTHREAD_MUTEX_INITIALIZER ;pthread_cond_t qemu_aio_cond =PTHREAD_COND_INITIALIZER ;pthread_cond_t qemu_vcpu_cond =PTHREAD_COND_INITIALIZER ;
@@ -53,7 +51,6 @@ struct vcpu_info {int signalled;int stop;int stopped; -int reload_regs;int created; } vcpu_info[256 ];
@@ -251,6 +248,8 @@ static void pause_all_threads(void) {int i; +assert (!cpu_single_env); +for (i =0 ; i < smp_cpus; ++i) { vcpu_info[i].stop =1 ;pthread_kill (vcpu_info[i].thread,SIG_IPI );
@@ -263,6 +262,8 @@ static void resume_all_threads(void) {int i; +assert (!cpu_single_env); +for (i =0 ; i < smp_cpus; ++i) { vcpu_info[i].stop =0 ; vcpu_info[i].stopped =0 ;
@@ -307,15 +308,18 @@ static void setup_kernel_sigmask(CPUStatkvm_set_signal_mask (kvm_context, env->cpu_index, &set); } -void qemu_kvm_system_reset_request (void ) +void qemu_kvm_system_reset (void ) {int i; -for (i =0 ; i < smp_cpus; ++i) { - vcpu_info[i].reload_regs =1 ; -pthread_kill (vcpu_info[i].thread,SIG_IPI ); - } +pause_all_threads (); +qemu_system_reset (); + +for (i =0 ; i < smp_cpus; ++i) +kvm_arch_cpu_reset (vcpu_info[i].env); + +resume_all_threads (); }static int kvm_main_loop_cpu (CPUState *env)
@@ -348,11 +352,6 @@ static int kvm_main_loop_cpu(CPUState *ekvm_cpu_exec (env); env->interrupt_request &= ~CPU_INTERRUPT_EXIT ;kvm_main_loop_wait (env,0 ); -if (info->reload_regs) { - info->reload_regs =0 ; -if (env->cpu_index ==0 )/* ap needs to be placed in INIT */ -kvm_arch_load_regs (env); - } }pthread_mutex_unlock (&qemu_mutex);return 0 ;
@@ -535,10 +534,8 @@ int kvm_main_loop(void)break ; else if (qemu_powerdown_requested()) qemu_system_powerdown(); - else if (qemu_reset_requested()) { -pthread_kill (vcpu_info[0 ].thread,SIG_IPI ); - qemu_kvm_reset_requested =1 ; - } + else if (qemu_reset_requested()) + qemu_kvm_system_reset(); }pause_all_threads ();
@@ -647,6 +644,9 @@ static int kvm_halt(void *opaque, int vcstatic int kvm_shutdown (void *opaque,int vcpu) { +/* stop the current vcpu from going back to guest mode */ + vcpu_info[cpu_single_env->cpu_index].stopped =1 ; +qemu_system_reset_request ();return 1 ; }
qemu/qemu-kvm.h +1 -0
@@ -59,6 +59,7 @@ void kvm_arch_post_kvm_run(void *opaque,int kvm_arch_has_work (CPUState *env);int kvm_arch_try_push_interrupts (void *opaque);void kvm_arch_update_regs_for_sipi (CPUState *env); +void kvm_arch_cpu_reset (CPUState *env);CPUState *qemu_kvm_cpu_env (int index);
qemu/vl.c +1 -5
@@ -7302,11 +7302,7 @@ void qemu_system_reset_request(void) }if (cpu_single_env)cpu_interrupt (cpu_single_env,CPU_INTERRUPT_EXIT ); -#ifdef USE_KVM -if (kvm_allowed) -if (!no_reboot) -qemu_kvm_system_reset_request (); -#endif +main_loop_break (); }void qemu_system_shutdown_request (void )
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
Quoted 9 lines · Jan Kiszka
Resetting guests used to be racy, deadlock-prone, or simply broken (for SMP). This patch fixes the issues, following Marcelo's suggestion to consolidate the reset activity in the I/O thread. All vcpus are cleanly stopped before the emulated hardware is reset, and
kvm_arch_cpu_resetis introduced and invoked to ensure that non-boot cpus are put into the right state on x86. Note that other arch may need to look into this service as well to get SMP reset right.
hmm. This means that reset is executed asynchronously of the calling cpu.
Well, in a real machine a keyboard controller reset is asynchronous as well, so I guess that's not an issue.
Signature · 8 lines
error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
Quoted 11 lines · Avi Kivity
Quoted 9 lines · Jan Kiszka
Resetting guests used to be racy, deadlock-prone, or simply broken (for SMP). This patch fixes the issues, following Marcelo's suggestion to consolidate the reset activity in the I/O thread. All vcpus are cleanly stopped before the emulated hardware is reset, and
kvm_arch_cpu_resetis introduced and invoked to ensure that non-boot cpus are put into the right state on x86. Note that other arch may need to look into this service as well to get SMP reset right.
Quoted 1 line
hmm. This means that reset is executed asynchronously of the calling cpu.
Nope, the calling cpu is put in stopped state after informing the I/O thread about the reset request. We just cannot control when the request is handled /wrt to other cpus, but that's not different from real life I guess.
Jan
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
kvm-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Quoted 14 lines · Jan Kiszka
Quoted 12 lines · Avi Kivity
Quoted 10 lines · Jan Kiszka
Resetting guests used to be racy, deadlock-prone, or simply broken (for SMP). This patch fixes the issues, following Marcelo's suggestion to consolidate the reset activity in the I/O thread. All vcpus are cleanly stopped before the emulated hardware is reset, and
kvm_arch_cpu_resetis introduced and invoked to ensure that non-boot cpus are put into the right state on x86. Note that other arch may need to look into this service as well to get SMP reset right.
Quoted 8 lines
Quoted 2 lines
hmm. This means that reset is executed asynchronously of the calling cpu.
Nope, the calling cpu is put in stopped state after informing the I/O thread about the reset request. We just cannot control when the request is handled /wrt to other cpus, but that's not different from real life I guess.
That's true for triple-faults, but not for ordinary keyboard controller resets. All you do there is call main_loop_break().
But I think that's fine; on a real machine the keyboard controller reset is processed by a separate processor (the keyboard controller) which has nonzero latency.
Signature · 8 lines
error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone