* [PATCH 1/2] Timer event should not unconditionally unhalt vcpu. @ 2009-03-23 10:12 Gleb Natapov 2009-03-23 10:12 ` [PATCH 2/2] Interrupt unhalts vcpu when it shouldn't Gleb Natapov 2009-03-23 13:11 ` [PATCH 1/2] Timer event should not unconditionally unhalt vcpu Gleb Natapov 0 siblings, 2 replies; 9+ messages in thread From: Gleb Natapov @ 2009-03-23 10:12 UTC (permalink / raw) To: avi; +Cc: kvm Currently timer events are processed before entering guest mode. Move it to main vcpu event loop since timer events should be processed even while vcpu is haled. Timer may cause interrupt/nmi to be injected and only then vcpu will be unhalted. Signed-off-by: Gleb Natapov <gleb@redhat.com> --- arch/ia64/kvm/kvm-ia64.c | 6 +++-- arch/x86/kvm/x86.c | 54 ++++++++++++++++++++++++++-------------------- virt/kvm/kvm_main.c | 5 +++- 3 files changed, 37 insertions(+), 28 deletions(-) diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c index c25347f..db0b2a5 100644 --- a/arch/ia64/kvm/kvm-ia64.c +++ b/arch/ia64/kvm/kvm-ia64.c @@ -488,10 +488,10 @@ int kvm_emulate_halt(struct kvm_vcpu *vcpu) hrtimer_cancel(p_ht); vcpu->arch.ht_active = 0; - if (test_and_clear_bit(KVM_REQ_UNHALT, &vcpu->requests)) + if (test_and_clear_bit(KVM_REQ_UNHALT, &vcpu->requests) || + kvm_cpu_has_pending_timer(vcpu)) if (vcpu->arch.mp_state == KVM_MP_STATE_HALTED) - vcpu->arch.mp_state = - KVM_MP_STATE_RUNNABLE; + vcpu->arch.mp_state = KVM_MP_STATE_RUNNABLE; if (vcpu->arch.mp_state != KVM_MP_STATE_RUNNABLE) return -EINTR; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 03431b2..cfe8213 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3126,9 +3126,6 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) } } - clear_bit(KVM_REQ_PENDING_TIMER, &vcpu->requests); - kvm_inject_pending_timer_irqs(vcpu); - preempt_disable(); kvm_x86_ops->prepare_guest_switch(vcpu); @@ -3228,6 +3225,7 @@ out: return r; } + static int __vcpu_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) { int r; @@ -3254,29 +3252,39 @@ static int __vcpu_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) kvm_vcpu_block(vcpu); down_read(&vcpu->kvm->slots_lock); if (test_and_clear_bit(KVM_REQ_UNHALT, &vcpu->requests)) - if (vcpu->arch.mp_state == KVM_MP_STATE_HALTED) + { + switch(vcpu->arch.mp_state) { + case KVM_MP_STATE_HALTED: vcpu->arch.mp_state = - KVM_MP_STATE_RUNNABLE; - if (vcpu->arch.mp_state != KVM_MP_STATE_RUNNABLE) - r = -EINTR; + KVM_MP_STATE_RUNNABLE; + case KVM_MP_STATE_RUNNABLE: + break; + case KVM_MP_STATE_SIPI_RECEIVED: + default: + r = -EINTR; + continue; + } + } } - if (r > 0) { - if (dm_request_for_irq_injection(vcpu, kvm_run)) { - r = -EINTR; - kvm_run->exit_reason = KVM_EXIT_INTR; - ++vcpu->stat.request_irq_exits; - } - if (signal_pending(current)) { - r = -EINTR; - kvm_run->exit_reason = KVM_EXIT_INTR; - ++vcpu->stat.signal_exits; - } - if (need_resched()) { - up_read(&vcpu->kvm->slots_lock); - kvm_resched(vcpu); - down_read(&vcpu->kvm->slots_lock); - } + clear_bit(KVM_REQ_PENDING_TIMER, &vcpu->requests); + if (kvm_cpu_has_pending_timer(vcpu)) + kvm_inject_pending_timer_irqs(vcpu); + + if (dm_request_for_irq_injection(vcpu, kvm_run)) { + r = -EINTR; + kvm_run->exit_reason = KVM_EXIT_INTR; + ++vcpu->stat.request_irq_exits; + } + if (signal_pending(current)) { + r = -EINTR; + kvm_run->exit_reason = KVM_EXIT_INTR; + ++vcpu->stat.signal_exits; + } + if (need_resched()) { + up_read(&vcpu->kvm->slots_lock); + kvm_resched(vcpu); + down_read(&vcpu->kvm->slots_lock); } } diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 92ef725..273490f 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -1613,11 +1613,12 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu) prepare_to_wait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE); if (kvm_cpu_has_interrupt(vcpu) || - kvm_cpu_has_pending_timer(vcpu) || - kvm_arch_vcpu_runnable(vcpu)) { + kvm_arch_vcpu_runnable(vcpu)) { set_bit(KVM_REQ_UNHALT, &vcpu->requests); break; } + if (kvm_cpu_has_pending_timer(vcpu)) + break; if (signal_pending(current)) break; ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] Interrupt unhalts vcpu when it shouldn't 2009-03-23 10:12 [PATCH 1/2] Timer event should not unconditionally unhalt vcpu Gleb Natapov @ 2009-03-23 10:12 ` Gleb Natapov 2009-03-23 14:31 ` Avi Kivity 2009-03-24 10:13 ` Avi Kivity 2009-03-23 13:11 ` [PATCH 1/2] Timer event should not unconditionally unhalt vcpu Gleb Natapov 1 sibling, 2 replies; 9+ messages in thread From: Gleb Natapov @ 2009-03-23 10:12 UTC (permalink / raw) To: avi; +Cc: kvm kvm_vcpu_block() unhalts vpu on an interrupt/timer without checking if interrupt window is actually opened. Signed-off-by: Gleb Natapov <gleb@redhat.com> --- arch/ia64/kvm/kvm-ia64.c | 6 ++++++ arch/powerpc/kvm/powerpc.c | 6 ++++++ arch/s390/kvm/interrupt.c | 6 ++++++ arch/x86/include/asm/kvm_host.h | 2 +- arch/x86/kvm/svm.c | 10 ++++++++++ arch/x86/kvm/vmx.c | 8 +++++++- arch/x86/kvm/x86.c | 5 +++++ include/linux/kvm_host.h | 1 + virt/kvm/kvm_main.c | 3 ++- 9 files changed, 44 insertions(+), 3 deletions(-) diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c index db0b2a5..fdda799 100644 --- a/arch/ia64/kvm/kvm-ia64.c +++ b/arch/ia64/kvm/kvm-ia64.c @@ -1960,6 +1960,12 @@ int kvm_cpu_has_interrupt(struct kvm_vcpu *vcpu) return 0; } +int kvm_arch_interrupt_allowed(struct kvm_vcpu *vcpu) +{ + /* do real check here */ + return 1; +} + int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu) { return vcpu->arch.timer_fired; diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index 9057335..2cf915e 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -41,6 +41,12 @@ int kvm_cpu_has_interrupt(struct kvm_vcpu *v) return !!(v->arch.pending_exceptions); } +int kvm_arch_interrupt_allowed(struct kvm_vcpu *vcpu) +{ + /* do real check here */ + return 1; +} + int kvm_arch_vcpu_runnable(struct kvm_vcpu *v) { return !(v->arch.msr & MSR_WE); diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c index 0189356..4ed4c3a 100644 --- a/arch/s390/kvm/interrupt.c +++ b/arch/s390/kvm/interrupt.c @@ -318,6 +318,12 @@ int kvm_cpu_has_interrupt(struct kvm_vcpu *vcpu) return rc; } +int kvm_arch_interrupt_allowed(struct kvm_vcpu *vcpu) +{ + /* do real check here */ + return 1; +} + int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu) { return 0; diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 4627627..8351c4d 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -521,7 +521,7 @@ struct kvm_x86_ops { void (*inject_pending_irq)(struct kvm_vcpu *vcpu); void (*inject_pending_vectors)(struct kvm_vcpu *vcpu, struct kvm_run *run); - + int (*interrupt_allowed)(struct kvm_vcpu *vcpu); int (*set_tss_addr)(struct kvm *kvm, unsigned int addr); int (*get_tdp_level)(void); int (*get_mt_mask_shift)(void); diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index da23fd3..70c9dd3 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -2285,6 +2285,15 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu) vmcb->control.intercept_cr_write |= INTERCEPT_CR8_MASK; } +static int svm_interrupt_allowed(struct kvm_vcpu *vcpu) +{ + struct vcpu_svm *svm = to_svm(vcpu); + struct vmcb *vmcb = svm->vmcb; + return (vmcb->save.rflags & X86_EFLAGS_IF) && + !(vmcb->control.int_state & SVM_INTERRUPT_SHADOW_MASK) && + (svm->vcpu.arch.hflags & HF_GIF_MASK); +} + static void svm_intr_assist(struct kvm_vcpu *vcpu) { struct vcpu_svm *svm = to_svm(vcpu); @@ -2664,6 +2673,7 @@ static struct kvm_x86_ops svm_x86_ops = { .exception_injected = svm_exception_injected, .inject_pending_irq = svm_intr_assist, .inject_pending_vectors = do_interrupt_requests, + .interrupt_allowed = svm_interrupt_allowed, .set_tss_addr = svm_set_tss_addr, .get_tdp_level = get_npt_level, diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 5cf28df..dcc1036 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -2489,6 +2489,12 @@ static void vmx_update_window_states(struct kvm_vcpu *vcpu) GUEST_INTR_STATE_MOV_SS))); } +static int vmx_interrupt_allowed(struct kvm_vcpu *vcpu) +{ + vmx_update_window_states(vcpu); + return vcpu->arch.interrupt_window_open; +} + static void kvm_do_inject_irq(struct kvm_vcpu *vcpu) { int word_index = __ffs(vcpu->arch.irq_summary); @@ -3703,7 +3709,7 @@ static struct kvm_x86_ops vmx_x86_ops = { .exception_injected = vmx_exception_injected, .inject_pending_irq = vmx_intr_assist, .inject_pending_vectors = do_interrupt_requests, - + .interrupt_allowed = vmx_interrupt_allowed, .set_tss_addr = vmx_set_tss_addr, .get_tdp_level = get_ept_level, .get_mt_mask_shift = vmx_get_mt_mask_shift, diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index cfe8213..83822c0 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4465,3 +4465,8 @@ void kvm_vcpu_kick(struct kvm_vcpu *vcpu) smp_call_function_single(ipi_pcpu, vcpu_kick_intr, vcpu, 0); put_cpu(); } + +int kvm_arch_interrupt_allowed(struct kvm_vcpu *vcpu) +{ + return kvm_x86_ops->interrupt_allowed(vcpu); +} diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 11eb702..095ebb6 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -298,6 +298,7 @@ int kvm_arch_hardware_setup(void); void kvm_arch_hardware_unsetup(void); void kvm_arch_check_processor_compat(void *rtn); int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu); +int kvm_arch_interrupt_allowed(struct kvm_vcpu *vcpu); void kvm_free_physmem(struct kvm *kvm); diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 273490f..8aa3b95 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -1612,7 +1612,8 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu) for (;;) { prepare_to_wait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE); - if (kvm_cpu_has_interrupt(vcpu) || + if ((kvm_arch_interrupt_allowed(vcpu) && + kvm_cpu_has_interrupt(vcpu)) || kvm_arch_vcpu_runnable(vcpu)) { set_bit(KVM_REQ_UNHALT, &vcpu->requests); break; ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] Interrupt unhalts vcpu when it shouldn't 2009-03-23 10:12 ` [PATCH 2/2] Interrupt unhalts vcpu when it shouldn't Gleb Natapov @ 2009-03-23 14:31 ` Avi Kivity 2009-03-23 15:17 ` Avi Kivity 2009-03-24 10:13 ` Avi Kivity 1 sibling, 1 reply; 9+ messages in thread From: Avi Kivity @ 2009-03-23 14:31 UTC (permalink / raw) To: Gleb Natapov; +Cc: kvm Gleb Natapov wrote: > kvm_vcpu_block() unhalts vpu on an interrupt/timer without checking > if interrupt window is actually opened. > > > +static int svm_interrupt_allowed(struct kvm_vcpu *vcpu) > +{ > + struct vcpu_svm *svm = to_svm(vcpu); > + struct vmcb *vmcb = svm->vmcb; > + return (vmcb->save.rflags & X86_EFLAGS_IF) && > + !(vmcb->control.int_state & SVM_INTERRUPT_SHADOW_MASK) && > + (svm->vcpu.arch.hflags & HF_GIF_MASK); > +} > + > > +static int vmx_interrupt_allowed(struct kvm_vcpu *vcpu) > +{ > + vmx_update_window_states(vcpu); > + return vcpu->arch.interrupt_window_open; > +} > + > static void kvm_do_inject_irq(struct kvm_vcpu *vcpu) > } > + > +int kvm_arch_interrupt_allowed(struct kvm_vcpu *vcpu) > +{ > + return kvm_x86_ops->interrupt_allowed(vcpu); > +} > If the guest enables interrupts but sets tpr/cr8 to block interrupts, we'll spin (like we do now). So I think this should be called kvm_arch_can_accept_interrupt() and take tpr into account. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] Interrupt unhalts vcpu when it shouldn't 2009-03-23 14:31 ` Avi Kivity @ 2009-03-23 15:17 ` Avi Kivity 2009-03-24 5:24 ` Sheng Yang 0 siblings, 1 reply; 9+ messages in thread From: Avi Kivity @ 2009-03-23 15:17 UTC (permalink / raw) To: Gleb Natapov; +Cc: kvm, Marcelo Tosatti, Sheng Yang Avi Kivity wrote: > Gleb Natapov wrote: >> kvm_vcpu_block() unhalts vpu on an interrupt/timer without checking >> if interrupt window is actually opened. >> >> >> +static int svm_interrupt_allowed(struct kvm_vcpu *vcpu) >> +{ >> + struct vcpu_svm *svm = to_svm(vcpu); >> + struct vmcb *vmcb = svm->vmcb; >> + return (vmcb->save.rflags & X86_EFLAGS_IF) && + >> !(vmcb->control.int_state & SVM_INTERRUPT_SHADOW_MASK) && >> + (svm->vcpu.arch.hflags & HF_GIF_MASK); >> +} >> + >> >> +static int vmx_interrupt_allowed(struct kvm_vcpu *vcpu) >> +{ >> + vmx_update_window_states(vcpu); >> + return vcpu->arch.interrupt_window_open; >> +} >> + >> static void kvm_do_inject_irq(struct kvm_vcpu *vcpu) >> } >> + >> +int kvm_arch_interrupt_allowed(struct kvm_vcpu *vcpu) >> +{ >> + return kvm_x86_ops->interrupt_allowed(vcpu); >> +} >> > > If the guest enables interrupts but sets tpr/cr8 to block interrupts, > we'll spin (like we do now). > > So I think this should be called kvm_arch_can_accept_interrupt() and > take tpr into account. > kvm_cpu_has_interrupt() takes the tpr into account, so we're okay here. Marcelo, Sheng? -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] Interrupt unhalts vcpu when it shouldn't 2009-03-23 15:17 ` Avi Kivity @ 2009-03-24 5:24 ` Sheng Yang 0 siblings, 0 replies; 9+ messages in thread From: Sheng Yang @ 2009-03-24 5:24 UTC (permalink / raw) To: Avi Kivity; +Cc: Gleb Natapov, kvm, Marcelo Tosatti On Monday 23 March 2009 23:17:42 Avi Kivity wrote: > Avi Kivity wrote: > > Gleb Natapov wrote: > >> kvm_vcpu_block() unhalts vpu on an interrupt/timer without checking > >> if interrupt window is actually opened. > >> > >> > >> +static int svm_interrupt_allowed(struct kvm_vcpu *vcpu) > >> +{ > >> + struct vcpu_svm *svm = to_svm(vcpu); > >> + struct vmcb *vmcb = svm->vmcb; > >> + return (vmcb->save.rflags & X86_EFLAGS_IF) && + > >> !(vmcb->control.int_state & SVM_INTERRUPT_SHADOW_MASK) && > >> + (svm->vcpu.arch.hflags & HF_GIF_MASK); > >> +} > >> + > >> > >> +static int vmx_interrupt_allowed(struct kvm_vcpu *vcpu) > >> +{ > >> + vmx_update_window_states(vcpu); > >> + return vcpu->arch.interrupt_window_open; > >> +} > >> + > >> static void kvm_do_inject_irq(struct kvm_vcpu *vcpu) > >> } > >> + > >> +int kvm_arch_interrupt_allowed(struct kvm_vcpu *vcpu) > >> +{ > >> + return kvm_x86_ops->interrupt_allowed(vcpu); > >> +} > > > > If the guest enables interrupts but sets tpr/cr8 to block interrupts, > > we'll spin (like we do now). > > > > So I think this should be called kvm_arch_can_accept_interrupt() and > > take tpr into account. > > kvm_cpu_has_interrupt() takes the tpr into account, so we're okay here. > > Marcelo, Sheng? Yes, looks good to me. -- regards Yang, Sheng ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] Interrupt unhalts vcpu when it shouldn't 2009-03-23 10:12 ` [PATCH 2/2] Interrupt unhalts vcpu when it shouldn't Gleb Natapov 2009-03-23 14:31 ` Avi Kivity @ 2009-03-24 10:13 ` Avi Kivity 1 sibling, 0 replies; 9+ messages in thread From: Avi Kivity @ 2009-03-24 10:13 UTC (permalink / raw) To: Gleb Natapov; +Cc: kvm Gleb Natapov wrote: > kvm_vcpu_block() unhalts vpu on an interrupt/timer without checking > if interrupt window is actually opened. > > Applied both, thanks. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] Timer event should not unconditionally unhalt vcpu. 2009-03-23 10:12 [PATCH 1/2] Timer event should not unconditionally unhalt vcpu Gleb Natapov 2009-03-23 10:12 ` [PATCH 2/2] Interrupt unhalts vcpu when it shouldn't Gleb Natapov @ 2009-03-23 13:11 ` Gleb Natapov 2009-03-23 14:26 ` Avi Kivity 1 sibling, 1 reply; 9+ messages in thread From: Gleb Natapov @ 2009-03-23 13:11 UTC (permalink / raw) To: avi; +Cc: kvm On Mon, Mar 23, 2009 at 12:12:06PM +0200, Gleb Natapov wrote: > Currently timer events are processed before entering guest mode. Move it > to main vcpu event loop since timer events should be processed even while > vcpu is haled. Timer may cause interrupt/nmi to be injected and only then > vcpu will be unhalted. > Use this one instead. Previous broke -no-kvm-irqchip option. Signed-off-by: Gleb Natapov <gleb@redhat.com> diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c index c25347f..db0b2a5 100644 --- a/arch/ia64/kvm/kvm-ia64.c +++ b/arch/ia64/kvm/kvm-ia64.c @@ -488,10 +488,10 @@ int kvm_emulate_halt(struct kvm_vcpu *vcpu) hrtimer_cancel(p_ht); vcpu->arch.ht_active = 0; - if (test_and_clear_bit(KVM_REQ_UNHALT, &vcpu->requests)) + if (test_and_clear_bit(KVM_REQ_UNHALT, &vcpu->requests) || + kvm_cpu_has_pending_timer(vcpu)) if (vcpu->arch.mp_state == KVM_MP_STATE_HALTED) - vcpu->arch.mp_state = - KVM_MP_STATE_RUNNABLE; + vcpu->arch.mp_state = KVM_MP_STATE_RUNNABLE; if (vcpu->arch.mp_state != KVM_MP_STATE_RUNNABLE) return -EINTR; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 03431b2..9cdfe1b 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3126,9 +3126,6 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) } } - clear_bit(KVM_REQ_PENDING_TIMER, &vcpu->requests); - kvm_inject_pending_timer_irqs(vcpu); - preempt_disable(); kvm_x86_ops->prepare_guest_switch(vcpu); @@ -3228,6 +3225,7 @@ out: return r; } + static int __vcpu_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) { int r; @@ -3254,29 +3252,42 @@ static int __vcpu_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) kvm_vcpu_block(vcpu); down_read(&vcpu->kvm->slots_lock); if (test_and_clear_bit(KVM_REQ_UNHALT, &vcpu->requests)) - if (vcpu->arch.mp_state == KVM_MP_STATE_HALTED) + { + switch(vcpu->arch.mp_state) { + case KVM_MP_STATE_HALTED: vcpu->arch.mp_state = - KVM_MP_STATE_RUNNABLE; - if (vcpu->arch.mp_state != KVM_MP_STATE_RUNNABLE) - r = -EINTR; + KVM_MP_STATE_RUNNABLE; + case KVM_MP_STATE_RUNNABLE: + break; + case KVM_MP_STATE_SIPI_RECEIVED: + default: + r = -EINTR; + break; + } + } } - if (r > 0) { - if (dm_request_for_irq_injection(vcpu, kvm_run)) { - r = -EINTR; - kvm_run->exit_reason = KVM_EXIT_INTR; - ++vcpu->stat.request_irq_exits; - } - if (signal_pending(current)) { - r = -EINTR; - kvm_run->exit_reason = KVM_EXIT_INTR; - ++vcpu->stat.signal_exits; - } - if (need_resched()) { - up_read(&vcpu->kvm->slots_lock); - kvm_resched(vcpu); - down_read(&vcpu->kvm->slots_lock); - } + if (r <= 0) + break; + + clear_bit(KVM_REQ_PENDING_TIMER, &vcpu->requests); + if (kvm_cpu_has_pending_timer(vcpu)) + kvm_inject_pending_timer_irqs(vcpu); + + if (dm_request_for_irq_injection(vcpu, kvm_run)) { + r = -EINTR; + kvm_run->exit_reason = KVM_EXIT_INTR; + ++vcpu->stat.request_irq_exits; + } + if (signal_pending(current)) { + r = -EINTR; + kvm_run->exit_reason = KVM_EXIT_INTR; + ++vcpu->stat.signal_exits; + } + if (need_resched()) { + up_read(&vcpu->kvm->slots_lock); + kvm_resched(vcpu); + down_read(&vcpu->kvm->slots_lock); } } diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 92ef725..273490f 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -1613,11 +1613,12 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu) prepare_to_wait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE); if (kvm_cpu_has_interrupt(vcpu) || - kvm_cpu_has_pending_timer(vcpu) || - kvm_arch_vcpu_runnable(vcpu)) { + kvm_arch_vcpu_runnable(vcpu)) { set_bit(KVM_REQ_UNHALT, &vcpu->requests); break; } + if (kvm_cpu_has_pending_timer(vcpu)) + break; if (signal_pending(current)) break; -- Gleb. ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] Timer event should not unconditionally unhalt vcpu. 2009-03-23 13:11 ` [PATCH 1/2] Timer event should not unconditionally unhalt vcpu Gleb Natapov @ 2009-03-23 14:26 ` Avi Kivity 2009-03-23 16:50 ` Marcelo Tosatti 0 siblings, 1 reply; 9+ messages in thread From: Avi Kivity @ 2009-03-23 14:26 UTC (permalink / raw) To: Gleb Natapov; +Cc: kvm, Marcelo Tosatti, Yang, Sheng Gleb Natapov wrote: > On Mon, Mar 23, 2009 at 12:12:06PM +0200, Gleb Natapov wrote: > >> Currently timer events are processed before entering guest mode. Move it >> to main vcpu event loop since timer events should be processed even while >> vcpu is haled. Timer may cause interrupt/nmi to be injected and only then >> vcpu will be unhalted. >> >> > Use this one instead. Previous broke -no-kvm-irqchip option. > Looks good to me. But this is tricky code. Marcelo, Sheng, your opinions? -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] Timer event should not unconditionally unhalt vcpu. 2009-03-23 14:26 ` Avi Kivity @ 2009-03-23 16:50 ` Marcelo Tosatti 0 siblings, 0 replies; 9+ messages in thread From: Marcelo Tosatti @ 2009-03-23 16:50 UTC (permalink / raw) To: Avi Kivity; +Cc: Gleb Natapov, kvm, Yang, Sheng On Mon, Mar 23, 2009 at 04:26:34PM +0200, Avi Kivity wrote: > Gleb Natapov wrote: >> On Mon, Mar 23, 2009 at 12:12:06PM +0200, Gleb Natapov wrote: >> >>> Currently timer events are processed before entering guest mode. Move it >>> to main vcpu event loop since timer events should be processed even while >>> vcpu is haled. Timer may cause interrupt/nmi to be injected and only then >>> vcpu will be unhalted. >>> >>> >> Use this one instead. Previous broke -no-kvm-irqchip option. >> > > Looks good to me. But this is tricky code. Marcelo, Sheng, your opinions? Looks good. Checking for timer interrupts after guest entry is strange, but it can be changed in the future. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2009-03-24 10:13 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-03-23 10:12 [PATCH 1/2] Timer event should not unconditionally unhalt vcpu Gleb Natapov 2009-03-23 10:12 ` [PATCH 2/2] Interrupt unhalts vcpu when it shouldn't Gleb Natapov 2009-03-23 14:31 ` Avi Kivity 2009-03-23 15:17 ` Avi Kivity 2009-03-24 5:24 ` Sheng Yang 2009-03-24 10:13 ` Avi Kivity 2009-03-23 13:11 ` [PATCH 1/2] Timer event should not unconditionally unhalt vcpu Gleb Natapov 2009-03-23 14:26 ` Avi Kivity 2009-03-23 16:50 ` Marcelo Tosatti
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox