* [PATCH v5 0/5] Nonatomic interrupt injection @ 2010-08-30 12:35 Avi Kivity 2010-08-30 12:35 ` [PATCH v5 1/5] KVM: Check for pending events before attempting injection Avi Kivity ` (4 more replies) 0 siblings, 5 replies; 20+ messages in thread From: Avi Kivity @ 2010-08-30 12:35 UTC (permalink / raw) To: Marcelo Tosatti, kvm This patchset changes interrupt injection to be done from normal process context instead of interrupts disabled context. This is useful for real mode interrupt injection on Intel without the current hacks (injecting as a software interrupt of a vm86 task), reducing latencies, and later, for allowing nested virtualization code to use kvm_read_guest()/kvm_write_guest() instead of kmap() to access the guest vmcb/vmcs. Seems to survive a hack that cancels every 16th entry, after injection has already taken place. With the PIC reset fix posted earlier, this passes autotest on both AMD and Intel, with in-kernel irqchip. I'll run -no-kvm-irqchip tests shortly. Please review carefully, esp. the first patch. Any missing kvm_make_request() there may result in a hung guest. v5: add KVM_REQ_EVENT to KVM_INTERRUPT path change first patch to sample KVM_REQ_EVENT before checking vcpu->requests for the entry abort path; improves bisectability (noticed by Gleb) v4: add KVM_REQ_EVENT after lapic restore v3: close new race between injection and entry fix Intel real-mode injection cancellation v2: svm support (easier than expected) fix silly vmx warning Avi Kivity (5): KVM: Check for pending events before attempting injection KVM: VMX: Split up vmx_complete_interrupts() KVM: VMX: Move real-mode interrupt injection fixup to vmx_complete_interrupts() KVM: VMX: Parameterize vmx_complete_interrupts() for both exit and entry KVM: Non-atomic interrupt injection arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/i8259.c | 1 + arch/x86/kvm/lapic.c | 13 +++++- arch/x86/kvm/svm.c | 20 ++++++++- arch/x86/kvm/vmx.c | 87 +++++++++++++++++++++++++++++---------- arch/x86/kvm/x86.c | 45 ++++++++++++++------ include/linux/kvm_host.h | 1 + 7 files changed, 130 insertions(+), 38 deletions(-) ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 1/5] KVM: Check for pending events before attempting injection 2010-08-30 12:35 [PATCH v5 0/5] Nonatomic interrupt injection Avi Kivity @ 2010-08-30 12:35 ` Avi Kivity 2011-02-07 6:00 ` [regression] KVM: hangs and "irq timeout" booting HURD unless -no-kvm-irqchip passed Jonathan Nieder 2010-08-30 12:35 ` [PATCH v5 2/5] KVM: VMX: Split up vmx_complete_interrupts() Avi Kivity ` (3 subsequent siblings) 4 siblings, 1 reply; 20+ messages in thread From: Avi Kivity @ 2010-08-30 12:35 UTC (permalink / raw) To: Marcelo Tosatti, kvm Instead of blindly attempting to inject an event before each guest entry, check for a possible event first in vcpu->requests. Sites that can trigger event injection are modified to set KVM_REQ_EVENT: - interrupt, nmi window opening - ppr updates - i8259 output changes - local apic irr changes - rflags updates - gif flag set - event set on exit This improves non-injecting entry performance, and sets the stage for non-atomic injection. Signed-off-by: Avi Kivity <avi@redhat.com> --- arch/x86/kvm/i8259.c | 1 + arch/x86/kvm/lapic.c | 13 +++++++++++-- arch/x86/kvm/svm.c | 8 +++++++- arch/x86/kvm/vmx.c | 6 ++++++ arch/x86/kvm/x86.c | 41 ++++++++++++++++++++++++++++++++--------- include/linux/kvm_host.h | 1 + 6 files changed, 58 insertions(+), 12 deletions(-) diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c index 5662ff6..6281a32 100644 --- a/arch/x86/kvm/i8259.c +++ b/arch/x86/kvm/i8259.c @@ -67,6 +67,7 @@ static void pic_unlock(struct kvm_pic *s) if (!found) return; + kvm_make_request(KVM_REQ_EVENT, found); kvm_vcpu_kick(found); } } diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 77d8c0f..c6f2f15 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -259,9 +259,10 @@ static inline int apic_find_highest_isr(struct kvm_lapic *apic) static void apic_update_ppr(struct kvm_lapic *apic) { - u32 tpr, isrv, ppr; + u32 tpr, isrv, ppr, old_ppr; int isr; + old_ppr = apic_get_reg(apic, APIC_PROCPRI); tpr = apic_get_reg(apic, APIC_TASKPRI); isr = apic_find_highest_isr(apic); isrv = (isr != -1) ? isr : 0; @@ -274,7 +275,10 @@ static void apic_update_ppr(struct kvm_lapic *apic) apic_debug("vlapic %p, ppr 0x%x, isr 0x%x, isrv 0x%x", apic, ppr, isr, isrv); - apic_set_reg(apic, APIC_PROCPRI, ppr); + if (old_ppr != ppr) { + apic_set_reg(apic, APIC_PROCPRI, ppr); + kvm_make_request(KVM_REQ_EVENT, apic->vcpu); + } } static void apic_set_tpr(struct kvm_lapic *apic, u32 tpr) @@ -391,6 +395,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode, break; } + kvm_make_request(KVM_REQ_EVENT, vcpu); kvm_vcpu_kick(vcpu); break; @@ -416,6 +421,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode, "INIT on a runnable vcpu %d\n", vcpu->vcpu_id); vcpu->arch.mp_state = KVM_MP_STATE_INIT_RECEIVED; + kvm_make_request(KVM_REQ_EVENT, vcpu); kvm_vcpu_kick(vcpu); } else { apic_debug("Ignoring de-assert INIT to vcpu %d\n", @@ -430,6 +436,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode, result = 1; vcpu->arch.sipi_vector = vector; vcpu->arch.mp_state = KVM_MP_STATE_SIPI_RECEIVED; + kvm_make_request(KVM_REQ_EVENT, vcpu); kvm_vcpu_kick(vcpu); } break; @@ -475,6 +482,7 @@ static void apic_set_eoi(struct kvm_lapic *apic) trigger_mode = IOAPIC_EDGE_TRIG; if (!(apic_get_reg(apic, APIC_SPIV) & APIC_SPIV_DIRECTED_EOI)) kvm_ioapic_update_eoi(apic->vcpu->kvm, vector, trigger_mode); + kvm_make_request(KVM_REQ_EVENT, apic->vcpu); } static void apic_send_ipi(struct kvm_lapic *apic) @@ -1152,6 +1160,7 @@ void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu) update_divide_count(apic); start_apic_timer(apic); apic->irr_pending = true; + kvm_make_request(KVM_REQ_EVENT, vcpu); } void __kvm_migrate_apic_timer(struct kvm_vcpu *vcpu) diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 69a32bb..8884e51 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -2288,6 +2288,7 @@ static int stgi_interception(struct vcpu_svm *svm) svm->next_rip = kvm_rip_read(&svm->vcpu) + 3; skip_emulated_instruction(&svm->vcpu); + kvm_make_request(KVM_REQ_EVENT, &svm->vcpu); enable_gif(svm); @@ -2663,6 +2664,7 @@ static int interrupt_window_interception(struct vcpu_svm *svm) { struct kvm_run *kvm_run = svm->vcpu.run; + kvm_make_request(KVM_REQ_EVENT, &svm->vcpu); svm_clear_vintr(svm); svm->vmcb->control.int_ctl &= ~V_IRQ_MASK; /* @@ -3108,8 +3110,10 @@ static void svm_complete_interrupts(struct vcpu_svm *svm) svm->int3_injected = 0; - if (svm->vcpu.arch.hflags & HF_IRET_MASK) + if (svm->vcpu.arch.hflags & HF_IRET_MASK) { svm->vcpu.arch.hflags &= ~(HF_NMI_MASK | HF_IRET_MASK); + kvm_make_request(KVM_REQ_EVENT, &svm->vcpu); + } svm->vcpu.arch.nmi_injected = false; kvm_clear_exception_queue(&svm->vcpu); @@ -3118,6 +3122,8 @@ static void svm_complete_interrupts(struct vcpu_svm *svm) if (!(exitintinfo & SVM_EXITINTINFO_VALID)) return; + kvm_make_request(KVM_REQ_EVENT, &svm->vcpu); + vector = exitintinfo & SVM_EXITINTINFO_VEC_MASK; type = exitintinfo & SVM_EXITINTINFO_TYPE_MASK; diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 676555c..4255856 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -3333,6 +3333,7 @@ static int handle_wrmsr(struct kvm_vcpu *vcpu) static int handle_tpr_below_threshold(struct kvm_vcpu *vcpu) { + kvm_make_request(KVM_REQ_EVENT, vcpu); return 1; } @@ -3345,6 +3346,8 @@ static int handle_interrupt_window(struct kvm_vcpu *vcpu) cpu_based_vm_exec_control &= ~CPU_BASED_VIRTUAL_INTR_PENDING; vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, cpu_based_vm_exec_control); + kvm_make_request(KVM_REQ_EVENT, vcpu); + ++vcpu->stat.irq_window_exits; /* @@ -3601,6 +3604,7 @@ static int handle_nmi_window(struct kvm_vcpu *vcpu) cpu_based_vm_exec_control &= ~CPU_BASED_VIRTUAL_NMI_PENDING; vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, cpu_based_vm_exec_control); ++vcpu->stat.nmi_window_exits; + kvm_make_request(KVM_REQ_EVENT, vcpu); return 1; } @@ -3834,6 +3838,8 @@ static void vmx_complete_interrupts(struct vcpu_vmx *vmx) if (!idtv_info_valid) return; + kvm_make_request(KVM_REQ_EVENT, &vmx->vcpu); + vector = idt_vectoring_info & VECTORING_INFO_VECTOR_MASK; type = idt_vectoring_info & VECTORING_INFO_TYPE_MASK; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index cc6d6cd..ac13791 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -284,6 +284,8 @@ static void kvm_multiple_exception(struct kvm_vcpu *vcpu, u32 prev_nr; int class1, class2; + kvm_make_request(KVM_REQ_EVENT, vcpu); + if (!vcpu->arch.exception.pending) { queue: vcpu->arch.exception.pending = true; @@ -339,6 +341,7 @@ void kvm_inject_page_fault(struct kvm_vcpu *vcpu, unsigned long addr, void kvm_inject_nmi(struct kvm_vcpu *vcpu) { + kvm_make_request(KVM_REQ_EVENT, vcpu); vcpu->arch.nmi_pending = 1; } EXPORT_SYMBOL_GPL(kvm_inject_nmi); @@ -2317,6 +2320,7 @@ static int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu, return -ENXIO; kvm_queue_interrupt(vcpu, irq->irq, false); + kvm_make_request(KVM_REQ_EVENT, vcpu); return 0; } @@ -2470,6 +2474,8 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu, if (events->flags & KVM_VCPUEVENT_VALID_SIPI_VECTOR) vcpu->arch.sipi_vector = events->sipi_vector; + kvm_make_request(KVM_REQ_EVENT, vcpu); + return 0; } @@ -4201,6 +4207,7 @@ restart: toggle_interruptibility(vcpu, vcpu->arch.emulate_ctxt.interruptibility); kvm_x86_ops->set_rflags(vcpu, vcpu->arch.emulate_ctxt.eflags); + kvm_make_request(KVM_REQ_EVENT, vcpu); memcpy(vcpu->arch.regs, c->regs, sizeof c->regs); kvm_rip_write(vcpu, vcpu->arch.emulate_ctxt.eip); @@ -4870,6 +4877,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) int r; bool req_int_win = !irqchip_in_kernel(vcpu->kvm) && vcpu->run->request_interrupt_window; + bool req_event; if (vcpu->requests) { if (kvm_check_request(KVM_REQ_MMU_RELOAD, vcpu)) @@ -4917,8 +4925,12 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) local_irq_disable(); + req_event = kvm_check_request(KVM_REQ_EVENT, vcpu); + if (!atomic_read(&vcpu->guest_mode) || vcpu->requests || need_resched() || signal_pending(current)) { + if (req_event) + kvm_make_request(KVM_REQ_EVENT, vcpu); atomic_set(&vcpu->guest_mode, 0); smp_wmb(); local_irq_enable(); @@ -4927,17 +4939,19 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) goto out; } - inject_pending_event(vcpu); + if (req_event) { + inject_pending_event(vcpu); - /* enable NMI/IRQ window open exits if needed */ - if (vcpu->arch.nmi_pending) - kvm_x86_ops->enable_nmi_window(vcpu); - else if (kvm_cpu_has_interrupt(vcpu) || req_int_win) - kvm_x86_ops->enable_irq_window(vcpu); + /* enable NMI/IRQ window open exits if needed */ + if (vcpu->arch.nmi_pending) + kvm_x86_ops->enable_nmi_window(vcpu); + else if (kvm_cpu_has_interrupt(vcpu) || req_int_win) + kvm_x86_ops->enable_irq_window(vcpu); - if (kvm_lapic_enabled(vcpu)) { - update_cr8_intercept(vcpu); - kvm_lapic_sync_to_vapic(vcpu); + if (kvm_lapic_enabled(vcpu)) { + update_cr8_intercept(vcpu); + kvm_lapic_sync_to_vapic(vcpu); + } } srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx); @@ -5177,6 +5191,8 @@ int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs) vcpu->arch.exception.pending = false; + kvm_make_request(KVM_REQ_EVENT, vcpu); + return 0; } @@ -5240,6 +5256,7 @@ int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu, struct kvm_mp_state *mp_state) { vcpu->arch.mp_state = mp_state->mp_state; + kvm_make_request(KVM_REQ_EVENT, vcpu); return 0; } @@ -5261,6 +5278,7 @@ int kvm_task_switch(struct kvm_vcpu *vcpu, u16 tss_selector, int reason, memcpy(vcpu->arch.regs, c->regs, sizeof c->regs); kvm_rip_write(vcpu, vcpu->arch.emulate_ctxt.eip); kvm_x86_ops->set_rflags(vcpu, vcpu->arch.emulate_ctxt.eflags); + kvm_make_request(KVM_REQ_EVENT, vcpu); return EMULATE_DONE; } EXPORT_SYMBOL_GPL(kvm_task_switch); @@ -5331,6 +5349,8 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu, !is_protmode(vcpu)) vcpu->arch.mp_state = KVM_MP_STATE_RUNNABLE; + kvm_make_request(KVM_REQ_EVENT, vcpu); + return 0; } @@ -5563,6 +5583,8 @@ int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu) vcpu->arch.dr6 = DR6_FIXED_1; vcpu->arch.dr7 = DR7_FIXED_1; + kvm_make_request(KVM_REQ_EVENT, vcpu); + return kvm_x86_ops->vcpu_reset(vcpu); } @@ -5870,6 +5892,7 @@ void kvm_set_rflags(struct kvm_vcpu *vcpu, unsigned long rflags) kvm_is_linear_rip(vcpu, vcpu->arch.singlestep_rip)) rflags |= X86_EFLAGS_TF; kvm_x86_ops->set_rflags(vcpu, rflags); + kvm_make_request(KVM_REQ_EVENT, vcpu); } EXPORT_SYMBOL_GPL(kvm_set_rflags); diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index f2ecdd5..a08d267 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -39,6 +39,7 @@ #define KVM_REQ_KVMCLOCK_UPDATE 8 #define KVM_REQ_KICK 9 #define KVM_REQ_DEACTIVATE_FPU 10 +#define KVM_REQ_EVENT 11 #define KVM_USERSPACE_IRQ_SOURCE_ID 0 -- 1.7.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [regression] KVM: hangs and "irq timeout" booting HURD unless -no-kvm-irqchip passed 2010-08-30 12:35 ` [PATCH v5 1/5] KVM: Check for pending events before attempting injection Avi Kivity @ 2011-02-07 6:00 ` Jonathan Nieder 2011-02-07 12:39 ` Avi Kivity 0 siblings, 1 reply; 20+ messages in thread From: Jonathan Nieder @ 2011-02-07 6:00 UTC (permalink / raw) To: Avi Kivity; +Cc: Marcelo Tosatti, kvm, Michael Tokarev, Guillem Jover Hi kvm-ers, When I boot the HURD with kvm -m 768 -net nic,model=ne2k_pci -net user hurd-installed.qemu it hangs and eventually produces two messages: hd0: unexpected_intr: status=0x58 { DriveReady SeekComplete DataRequest } hd0: irq timeout: status=0x58 { DriveReady Seek Complete DataRequest } More details below[1]. Adding -no-kvm-irqchip to the kvm command line fixes it --- no more hangs or confusing messages (thanks, Guillem!). Bisects (thanks to Michael for the idea) to v2.6.37-rc1~142^2~39 (KVM: Check for pending events before attempting injection, 2010-07-27). Bisection log and kernel configuration available upon request (but probably not too relevant --- the 2.6.37 distro kernel from Debian exhibits the same problem). Reproducible with kvm/master (2d4b4d26, 2011-02-01). CPU is a dual-core AMD Athlon II P360, family 16, model 6. Any hints for tracking this down? For those wanting to follow along at home, you can find a HURD cd to try at [2]. Thanks for kvm --- now that I have hardware that can handle it, it's become a favorite toy. :) Jonathan [1] Background: http://bugs.debian.org/612105 Result: 1. Normal boot procedure. Eventually the following message from HURD is on the console: start ext2fs: Hurd server bootstrap: ext2fs[device:hd0s1] exec 2. Hang. I hit ctrl+alt, "info registers", and so on in the hope of waking it up. Eventually... 3. A flood of repeated kb_setleds1: unexpected state (1) messages. 4. Another message and another hang: hd0: unexpected_intr: status=0x58 { DriveReady SeekComplete DataRequest } hd0: irq timeout: status=0x58 { DriveReady Seek Complete DataRequest } (the two messages overlap sometimes). [2] http://people.debian.org/~sthibault/hurd-i386/installer/cdimage/ ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [regression] KVM: hangs and "irq timeout" booting HURD unless -no-kvm-irqchip passed 2011-02-07 6:00 ` [regression] KVM: hangs and "irq timeout" booting HURD unless -no-kvm-irqchip passed Jonathan Nieder @ 2011-02-07 12:39 ` Avi Kivity 2011-02-07 12:45 ` Gleb Natapov 0 siblings, 1 reply; 20+ messages in thread From: Avi Kivity @ 2011-02-07 12:39 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Marcelo Tosatti, kvm, Michael Tokarev, Guillem Jover On 02/07/2011 08:00 AM, Jonathan Nieder wrote: > Hi kvm-ers, > > When I boot the HURD with > > kvm -m 768 -net nic,model=ne2k_pci -net user hurd-installed.qemu > > it hangs and eventually produces two messages: > > hd0: unexpected_intr: status=0x58 { DriveReady SeekComplete DataRequest } > hd0: irq timeout: status=0x58 { DriveReady Seek Complete DataRequest } > > More details below[1]. > > Adding -no-kvm-irqchip to the kvm command line fixes it --- no > more hangs or confusing messages (thanks, Guillem!). > > Bisects (thanks to Michael for the idea) to > > v2.6.37-rc1~142^2~39 (KVM: Check for pending events before > attempting injection, 2010-07-27). > > Bisection log and kernel configuration available upon request (but > probably not too relevant --- the 2.6.37 distro kernel from Debian > exhibits the same problem). Reproducible with kvm/master (2d4b4d26, > 2011-02-01). > > CPU is a dual-core AMD Athlon II P360, family 16, model 6. > > Any hints for tracking this down? For those wanting to follow along > at home, you can find a HURD cd to try at [2]. > Reproduced on AMD, not on Intel. Given that I see a few PIC and PIT PIOs before the hang, the problem is likely at the PIT. Will look further. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [regression] KVM: hangs and "irq timeout" booting HURD unless -no-kvm-irqchip passed 2011-02-07 12:39 ` Avi Kivity @ 2011-02-07 12:45 ` Gleb Natapov 2011-02-07 13:27 ` Gleb Natapov 0 siblings, 1 reply; 20+ messages in thread From: Gleb Natapov @ 2011-02-07 12:45 UTC (permalink / raw) To: Avi Kivity Cc: Jonathan Nieder, Marcelo Tosatti, kvm, Michael Tokarev, Guillem Jover On Mon, Feb 07, 2011 at 02:39:01PM +0200, Avi Kivity wrote: > On 02/07/2011 08:00 AM, Jonathan Nieder wrote: > >Hi kvm-ers, > > > >When I boot the HURD with > > > > kvm -m 768 -net nic,model=ne2k_pci -net user hurd-installed.qemu > > > >it hangs and eventually produces two messages: > > > > hd0: unexpected_intr: status=0x58 { DriveReady SeekComplete DataRequest } > > hd0: irq timeout: status=0x58 { DriveReady Seek Complete DataRequest } > > > >More details below[1]. > > > >Adding -no-kvm-irqchip to the kvm command line fixes it --- no > >more hangs or confusing messages (thanks, Guillem!). > > > >Bisects (thanks to Michael for the idea) to > > > > v2.6.37-rc1~142^2~39 (KVM: Check for pending events before > > attempting injection, 2010-07-27). > > > >Bisection log and kernel configuration available upon request (but > >probably not too relevant --- the 2.6.37 distro kernel from Debian > >exhibits the same problem). Reproducible with kvm/master (2d4b4d26, > >2011-02-01). > > > >CPU is a dual-core AMD Athlon II P360, family 16, model 6. > > > >Any hints for tracking this down? For those wanting to follow along > >at home, you can find a HURD cd to try at [2]. > > > > Reproduced on AMD, not on Intel. Given that I see a few PIC and PIT > PIOs before the hang, the problem is likely at the PIT. Will look > further. > I can reproduce it on Intel. Looking into it. Looks like PIC isr_ack problem again. -- Gleb. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [regression] KVM: hangs and "irq timeout" booting HURD unless -no-kvm-irqchip passed 2011-02-07 12:45 ` Gleb Natapov @ 2011-02-07 13:27 ` Gleb Natapov 2011-02-08 1:40 ` Jonathan Nieder 0 siblings, 1 reply; 20+ messages in thread From: Gleb Natapov @ 2011-02-07 13:27 UTC (permalink / raw) To: Avi Kivity Cc: Jonathan Nieder, Marcelo Tosatti, kvm, Michael Tokarev, Guillem Jover On Mon, Feb 07, 2011 at 02:45:13PM +0200, Gleb Natapov wrote: > On Mon, Feb 07, 2011 at 02:39:01PM +0200, Avi Kivity wrote: > > On 02/07/2011 08:00 AM, Jonathan Nieder wrote: > > >Hi kvm-ers, > > > > > >When I boot the HURD with > > > > > > kvm -m 768 -net nic,model=ne2k_pci -net user hurd-installed.qemu > > > > > >it hangs and eventually produces two messages: > > > > > > hd0: unexpected_intr: status=0x58 { DriveReady SeekComplete DataRequest } > > > hd0: irq timeout: status=0x58 { DriveReady Seek Complete DataRequest } > > > > > >More details below[1]. > > > > > >Adding -no-kvm-irqchip to the kvm command line fixes it --- no > > >more hangs or confusing messages (thanks, Guillem!). > > > > > >Bisects (thanks to Michael for the idea) to > > > > > > v2.6.37-rc1~142^2~39 (KVM: Check for pending events before > > > attempting injection, 2010-07-27). > > > > > >Bisection log and kernel configuration available upon request (but > > >probably not too relevant --- the 2.6.37 distro kernel from Debian > > >exhibits the same problem). Reproducible with kvm/master (2d4b4d26, > > >2011-02-01). > > > > > >CPU is a dual-core AMD Athlon II P360, family 16, model 6. > > > > > >Any hints for tracking this down? For those wanting to follow along > > >at home, you can find a HURD cd to try at [2]. > > > > > > > Reproduced on AMD, not on Intel. Given that I see a few PIC and PIT > > PIOs before the hang, the problem is likely at the PIT. Will look > > further. > > > I can reproduce it on Intel. Looking into it. Looks like PIC isr_ack > problem again. > Is this patch helps? diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c index 3cece05..62b1dde 100644 --- a/arch/x86/kvm/i8259.c +++ b/arch/x86/kvm/i8259.c @@ -549,6 +549,9 @@ static void pic_irq_request(struct kvm *kvm, int level) struct kvm_pic *s = pic_irqchip(kvm); int irq = pic_get_irq(&s->pics[0]); + if (s->output && !level) + s->pics[0].isr_ack = 0xff; + s->output = level; if (vcpu && level && (s->pics[0].isr_ack & (1 << irq))) { s->pics[0].isr_ack &= ~(1 << irq); -- Gleb. ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [regression] KVM: hangs and "irq timeout" booting HURD unless -no-kvm-irqchip passed 2011-02-07 13:27 ` Gleb Natapov @ 2011-02-08 1:40 ` Jonathan Nieder 2011-02-08 7:40 ` Michael Tokarev 2011-02-08 12:00 ` Gleb Natapov 0 siblings, 2 replies; 20+ messages in thread From: Jonathan Nieder @ 2011-02-08 1:40 UTC (permalink / raw) To: Gleb Natapov Cc: Avi Kivity, Marcelo Tosatti, kvm, Michael Tokarev, Guillem Jover Gleb Natapov wrote: > Is this patch helps? > > diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c > index 3cece05..62b1dde 100644 > --- a/arch/x86/kvm/i8259.c > +++ b/arch/x86/kvm/i8259.c > @@ -549,6 +549,9 @@ static void pic_irq_request(struct kvm *kvm, int level) > struct kvm_pic *s = pic_irqchip(kvm); > int irq = pic_get_irq(&s->pics[0]); > > + if (s->output && !level) > + s->pics[0].isr_ack = 0xff; > + Yes, it does (tested on top of kvm/master). Thanks! ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [regression] KVM: hangs and "irq timeout" booting HURD unless -no-kvm-irqchip passed 2011-02-08 1:40 ` Jonathan Nieder @ 2011-02-08 7:40 ` Michael Tokarev 2011-02-08 12:00 ` Gleb Natapov 1 sibling, 0 replies; 20+ messages in thread From: Michael Tokarev @ 2011-02-08 7:40 UTC (permalink / raw) To: Jonathan Nieder Cc: Gleb Natapov, Avi Kivity, Marcelo Tosatti, kvm, Guillem Jover 08.02.2011 04:40, Jonathan Nieder wrote: > Gleb Natapov wrote: > >> Is this patch helps? >> >> diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c >> index 3cece05..62b1dde 100644 >> --- a/arch/x86/kvm/i8259.c >> +++ b/arch/x86/kvm/i8259.c >> @@ -549,6 +549,9 @@ static void pic_irq_request(struct kvm *kvm, int level) >> struct kvm_pic *s = pic_irqchip(kvm); >> int irq = pic_get_irq(&s->pics[0]); >> >> + if (s->output && !level) >> + s->pics[0].isr_ack = 0xff; >> + > > Yes, it does (tested on top of kvm/master). Thanks! And FWIW, I still can't trigger this issue on my machines - with any of my custom kernels (2.6.37 and 2.6.38git included), but debian 2.6.37-trunk-amd64 triggers it. On all my kernels hurd installation goes way further. /mjt ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [regression] KVM: hangs and "irq timeout" booting HURD unless -no-kvm-irqchip passed 2011-02-08 1:40 ` Jonathan Nieder 2011-02-08 7:40 ` Michael Tokarev @ 2011-02-08 12:00 ` Gleb Natapov 2011-02-08 14:22 ` Marcelo Tosatti 2011-02-08 20:40 ` Jonathan Nieder 1 sibling, 2 replies; 20+ messages in thread From: Gleb Natapov @ 2011-02-08 12:00 UTC (permalink / raw) To: Jonathan Nieder Cc: Avi Kivity, Marcelo Tosatti, kvm, Michael Tokarev, Guillem Jover On Mon, Feb 07, 2011 at 07:40:55PM -0600, Jonathan Nieder wrote: > Gleb Natapov wrote: > > > Is this patch helps? > > > > diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c > > index 3cece05..62b1dde 100644 > > --- a/arch/x86/kvm/i8259.c > > +++ b/arch/x86/kvm/i8259.c > > @@ -549,6 +549,9 @@ static void pic_irq_request(struct kvm *kvm, int level) > > struct kvm_pic *s = pic_irqchip(kvm); > > int irq = pic_get_irq(&s->pics[0]); > > > > + if (s->output && !level) > > + s->pics[0].isr_ack = 0xff; > > + > > Yes, it does (tested on top of kvm/master). Thanks! Thanks for testing. Can you test this one too please: diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c index 3cece05..5528484 100644 --- a/arch/x86/kvm/i8259.c +++ b/arch/x86/kvm/i8259.c @@ -62,9 +62,6 @@ static void pic_unlock(struct kvm_pic *s) } if (!found) - found = s->kvm->bsp_vcpu; - - if (!found) return; kvm_make_request(KVM_REQ_EVENT, found); @@ -75,7 +72,6 @@ static void pic_unlock(struct kvm_pic *s) static void pic_clear_isr(struct kvm_kpic_state *s, int irq) { s->isr &= ~(1 << irq); - s->isr_ack |= (1 << irq); if (s != &s->pics_state->pics[0]) irq += 8; /* @@ -89,16 +85,6 @@ static void pic_clear_isr(struct kvm_kpic_state *s, int irq) pic_lock(s->pics_state); } -void kvm_pic_clear_isr_ack(struct kvm *kvm) -{ - struct kvm_pic *s = pic_irqchip(kvm); - - pic_lock(s); - s->pics[0].isr_ack = 0xff; - s->pics[1].isr_ack = 0xff; - pic_unlock(s); -} - /* * set irq level. If an edge is detected, then the IRR is set to 1 */ @@ -281,7 +267,6 @@ void kvm_pic_reset(struct kvm_kpic_state *s) s->irr = 0; s->imr = 0; s->isr = 0; - s->isr_ack = 0xff; s->priority_add = 0; s->irq_base = 0; s->read_reg_select = 0; @@ -545,15 +530,11 @@ static int picdev_read(struct kvm_io_device *this, */ static void pic_irq_request(struct kvm *kvm, int level) { - struct kvm_vcpu *vcpu = kvm->bsp_vcpu; struct kvm_pic *s = pic_irqchip(kvm); - int irq = pic_get_irq(&s->pics[0]); - s->output = level; - if (vcpu && level && (s->pics[0].isr_ack & (1 << irq))) { - s->pics[0].isr_ack &= ~(1 << irq); + if (!s->output) s->wakeup_needed = true; - } + s->output = level; } static const struct kvm_io_device_ops picdev_ops = { @@ -575,8 +556,6 @@ struct kvm_pic *kvm_create_pic(struct kvm *kvm) s->pics[1].elcr_mask = 0xde; s->pics[0].pics_state = s; s->pics[1].pics_state = s; - s->pics[0].isr_ack = 0xff; - s->pics[1].isr_ack = 0xff; /* * Initialize PIO device diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index bcc0efc..2dc5fc4 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -2648,8 +2648,6 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu, vcpu->arch.interrupt.pending = events->interrupt.injected; vcpu->arch.interrupt.nr = events->interrupt.nr; vcpu->arch.interrupt.soft = events->interrupt.soft; - if (vcpu->arch.interrupt.pending && irqchip_in_kernel(vcpu->kvm)) - kvm_pic_clear_isr_ack(vcpu->kvm); if (events->flags & KVM_VCPUEVENT_VALID_SHADOW) kvm_x86_ops->set_interrupt_shadow(vcpu, events->interrupt.shadow); @@ -5617,8 +5615,6 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu, if (pending_vec < max_bits) { kvm_queue_interrupt(vcpu, pending_vec, false); pr_debug("Set back pending irq %d\n", pending_vec); - if (irqchip_in_kernel(vcpu->kvm)) - kvm_pic_clear_isr_ack(vcpu->kvm); } kvm_set_segment(vcpu, &sregs->cs, VCPU_SREG_CS); -- Gleb. ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [regression] KVM: hangs and "irq timeout" booting HURD unless -no-kvm-irqchip passed 2011-02-08 12:00 ` Gleb Natapov @ 2011-02-08 14:22 ` Marcelo Tosatti 2011-02-08 14:41 ` Gleb Natapov 2011-02-08 14:43 ` Avi Kivity 2011-02-08 20:40 ` Jonathan Nieder 1 sibling, 2 replies; 20+ messages in thread From: Marcelo Tosatti @ 2011-02-08 14:22 UTC (permalink / raw) To: Gleb Natapov Cc: Jonathan Nieder, Avi Kivity, kvm, Michael Tokarev, Guillem Jover On Tue, Feb 08, 2011 at 02:00:37PM +0200, Gleb Natapov wrote: > On Mon, Feb 07, 2011 at 07:40:55PM -0600, Jonathan Nieder wrote: > > Gleb Natapov wrote: > > > > > Is this patch helps? > > > > > > diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c > > > index 3cece05..62b1dde 100644 > > > --- a/arch/x86/kvm/i8259.c > > > +++ b/arch/x86/kvm/i8259.c > > > @@ -549,6 +549,9 @@ static void pic_irq_request(struct kvm *kvm, int level) > > > struct kvm_pic *s = pic_irqchip(kvm); > > > int irq = pic_get_irq(&s->pics[0]); > > > > > > + if (s->output && !level) > > > + s->pics[0].isr_ack = 0xff; > > > + > > > > Yes, it does (tested on top of kvm/master). Thanks! > Thanks for testing. Can you test this one too please: > > diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c > index 3cece05..5528484 100644 > --- a/arch/x86/kvm/i8259.c > +++ b/arch/x86/kvm/i8259.c > @@ -62,9 +62,6 @@ static void pic_unlock(struct kvm_pic *s) > } > > if (!found) > - found = s->kvm->bsp_vcpu; > - > - if (!found) > return; > > kvm_make_request(KVM_REQ_EVENT, found); Gleb, I don't think the isr_ack logic is overly complex that it should be removed. For some cases it is still beneficial, see example case on commit e48258009d941, which is not handled by kick coalescing of kvm_vcpu_kick. What is the problem with Hurd exactly? ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [regression] KVM: hangs and "irq timeout" booting HURD unless -no-kvm-irqchip passed 2011-02-08 14:22 ` Marcelo Tosatti @ 2011-02-08 14:41 ` Gleb Natapov 2011-02-08 14:43 ` Avi Kivity 1 sibling, 0 replies; 20+ messages in thread From: Gleb Natapov @ 2011-02-08 14:41 UTC (permalink / raw) To: Marcelo Tosatti Cc: Jonathan Nieder, Avi Kivity, kvm, Michael Tokarev, Guillem Jover On Tue, Feb 08, 2011 at 12:22:53PM -0200, Marcelo Tosatti wrote: > On Tue, Feb 08, 2011 at 02:00:37PM +0200, Gleb Natapov wrote: > > On Mon, Feb 07, 2011 at 07:40:55PM -0600, Jonathan Nieder wrote: > > > Gleb Natapov wrote: > > > > > > > Is this patch helps? > > > > > > > > diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c > > > > index 3cece05..62b1dde 100644 > > > > --- a/arch/x86/kvm/i8259.c > > > > +++ b/arch/x86/kvm/i8259.c > > > > @@ -549,6 +549,9 @@ static void pic_irq_request(struct kvm *kvm, int level) > > > > struct kvm_pic *s = pic_irqchip(kvm); > > > > int irq = pic_get_irq(&s->pics[0]); > > > > > > > > + if (s->output && !level) > > > > + s->pics[0].isr_ack = 0xff; > > > > + > > > > > > Yes, it does (tested on top of kvm/master). Thanks! > > Thanks for testing. Can you test this one too please: > > > > diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c > > index 3cece05..5528484 100644 > > --- a/arch/x86/kvm/i8259.c > > +++ b/arch/x86/kvm/i8259.c > > @@ -62,9 +62,6 @@ static void pic_unlock(struct kvm_pic *s) > > } > > > > if (!found) > > - found = s->kvm->bsp_vcpu; > > - > > - if (!found) > > return; > > > > kvm_make_request(KVM_REQ_EVENT, found); > > Gleb, > > I don't think the isr_ack logic is overly complex that it should be Well we continue to find bugs in it. > removed. For some cases it is still beneficial, see example case on > commit e48258009d941, which is not handled by kick coalescing of > kvm_vcpu_kick. I think the code I've send will not exhibit the behaviour described by this commit since in that case s->output will never become zero (kvm_apic_accept_pic_intr() should return false). > > What is the problem with Hurd exactly? The problem is not with Hurd, but with our pic emulation. PIT sends interrupt while it is unmasked in PIC, HURD never gets it (probably interrupts are disabled by vcpu), HURD masks PIT interrupt, then it unmasks it and never gets it since previous one was not acked yet and isr_ack logic prevent vcpu from been kicked again (and since KVM_REQ_EVENT is set as part of kick logic vcpu never checks interrupt availability again). -- Gleb. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [regression] KVM: hangs and "irq timeout" booting HURD unless -no-kvm-irqchip passed 2011-02-08 14:22 ` Marcelo Tosatti 2011-02-08 14:41 ` Gleb Natapov @ 2011-02-08 14:43 ` Avi Kivity 2011-02-08 14:47 ` Gleb Natapov 1 sibling, 1 reply; 20+ messages in thread From: Avi Kivity @ 2011-02-08 14:43 UTC (permalink / raw) To: Marcelo Tosatti Cc: Gleb Natapov, Jonathan Nieder, kvm, Michael Tokarev, Guillem Jover On 02/08/2011 04:22 PM, Marcelo Tosatti wrote: > I don't think the isr_ack logic is overly complex that it should be > removed. For some cases it is still beneficial, see example case on > commit e48258009d941, which is not handled by kick coalescing of > kvm_vcpu_kick. On the other hand, I think it can be done differently. For example LVT0 is probably programmed to mask interrupts; we can simply look at it and not kick if that's the case. We can use notifiers from the lapic to the pic to avoid looking at lapic data. The advantage in this way is that we avoid introducing state, instead relying on existing state. New state is always bad since it has to be kept in sync with guest visible state. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [regression] KVM: hangs and "irq timeout" booting HURD unless -no-kvm-irqchip passed 2011-02-08 14:43 ` Avi Kivity @ 2011-02-08 14:47 ` Gleb Natapov 2011-02-08 14:57 ` Avi Kivity 0 siblings, 1 reply; 20+ messages in thread From: Gleb Natapov @ 2011-02-08 14:47 UTC (permalink / raw) To: Avi Kivity Cc: Marcelo Tosatti, Jonathan Nieder, kvm, Michael Tokarev, Guillem Jover On Tue, Feb 08, 2011 at 04:43:33PM +0200, Avi Kivity wrote: > On 02/08/2011 04:22 PM, Marcelo Tosatti wrote: > >I don't think the isr_ack logic is overly complex that it should be > >removed. For some cases it is still beneficial, see example case on > >commit e48258009d941, which is not handled by kick coalescing of > >kvm_vcpu_kick. > > On the other hand, I think it can be done differently. For example > LVT0 is probably programmed to mask interrupts; we can simply look > at it and not kick if that's the case. We can use notifiers from > the lapic to the pic to avoid looking at lapic data. > I believe this is what my patch is doing. Look at pic_unlock(). The code search for vcpu to kick by calling kvm_apic_accept_pic_intr() function (which checks that LVT is masked). If no vcpu is found we kicks bsp. Why? I removed that. > The advantage in this way is that we avoid introducing state, > instead relying on existing state. New state is always bad since it > has to be kept in sync with guest visible state. > > -- > error compiling committee.c: too many arguments to function -- Gleb. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [regression] KVM: hangs and "irq timeout" booting HURD unless -no-kvm-irqchip passed 2011-02-08 14:47 ` Gleb Natapov @ 2011-02-08 14:57 ` Avi Kivity 2011-02-08 17:44 ` Marcelo Tosatti 0 siblings, 1 reply; 20+ messages in thread From: Avi Kivity @ 2011-02-08 14:57 UTC (permalink / raw) To: Gleb Natapov Cc: Marcelo Tosatti, Jonathan Nieder, kvm, Michael Tokarev, Guillem Jover On 02/08/2011 04:47 PM, Gleb Natapov wrote: > On Tue, Feb 08, 2011 at 04:43:33PM +0200, Avi Kivity wrote: > > On 02/08/2011 04:22 PM, Marcelo Tosatti wrote: > > >I don't think the isr_ack logic is overly complex that it should be > > >removed. For some cases it is still beneficial, see example case on > > >commit e48258009d941, which is not handled by kick coalescing of > > >kvm_vcpu_kick. > > > > On the other hand, I think it can be done differently. For example > > LVT0 is probably programmed to mask interrupts; we can simply look > > at it and not kick if that's the case. We can use notifiers from > > the lapic to the pic to avoid looking at lapic data. > > > I believe this is what my patch is doing. Look at pic_unlock(). The code > search for vcpu to kick by calling kvm_apic_accept_pic_intr() function > (which checks that LVT is masked). It does indeed. > If no vcpu is found we kicks bsp. > Why? I removed that. The code that looks for a vcpu that has LVT0 unmasked is newer than the isr_ack code (see cfe149e91b82). So it looks like the isr_ack code is indeed unnecessary now. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [regression] KVM: hangs and "irq timeout" booting HURD unless -no-kvm-irqchip passed 2011-02-08 14:57 ` Avi Kivity @ 2011-02-08 17:44 ` Marcelo Tosatti 0 siblings, 0 replies; 20+ messages in thread From: Marcelo Tosatti @ 2011-02-08 17:44 UTC (permalink / raw) To: Avi Kivity Cc: Gleb Natapov, Jonathan Nieder, kvm, Michael Tokarev, Guillem Jover On Tue, Feb 08, 2011 at 04:57:16PM +0200, Avi Kivity wrote: > On 02/08/2011 04:47 PM, Gleb Natapov wrote: > >On Tue, Feb 08, 2011 at 04:43:33PM +0200, Avi Kivity wrote: > >> On 02/08/2011 04:22 PM, Marcelo Tosatti wrote: > >> >I don't think the isr_ack logic is overly complex that it should be > >> >removed. For some cases it is still beneficial, see example case on > >> >commit e48258009d941, which is not handled by kick coalescing of > >> >kvm_vcpu_kick. > >> > >> On the other hand, I think it can be done differently. For example > >> LVT0 is probably programmed to mask interrupts; we can simply look > >> at it and not kick if that's the case. We can use notifiers from > >> the lapic to the pic to avoid looking at lapic data. > >> > >I believe this is what my patch is doing. Look at pic_unlock(). The code > >search for vcpu to kick by calling kvm_apic_accept_pic_intr() function > >(which checks that LVT is masked). > > It does indeed. > > >If no vcpu is found we kicks bsp. > >Why? I removed that. > > The code that looks for a vcpu that has LVT0 unmasked is newer than > the isr_ack code (see cfe149e91b82). So it looks like the isr_ack > code is indeed unnecessary now. Right. Patch looks fine. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [regression] KVM: hangs and "irq timeout" booting HURD unless -no-kvm-irqchip passed 2011-02-08 12:00 ` Gleb Natapov 2011-02-08 14:22 ` Marcelo Tosatti @ 2011-02-08 20:40 ` Jonathan Nieder 1 sibling, 0 replies; 20+ messages in thread From: Jonathan Nieder @ 2011-02-08 20:40 UTC (permalink / raw) To: Gleb Natapov Cc: Avi Kivity, Marcelo Tosatti, kvm, Michael Tokarev, Guillem Jover Gleb Natapov wrote: > Thanks for testing. Can you test this one too please: > > i8259.c | 25 ++----------------------- > x86.c | 4 ---- > 2 files changed, 2 insertions(+), 27 deletions(-) Yes, it works, too. (Tested against v2.6.37.) ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 2/5] KVM: VMX: Split up vmx_complete_interrupts() 2010-08-30 12:35 [PATCH v5 0/5] Nonatomic interrupt injection Avi Kivity 2010-08-30 12:35 ` [PATCH v5 1/5] KVM: Check for pending events before attempting injection Avi Kivity @ 2010-08-30 12:35 ` Avi Kivity 2010-08-30 12:35 ` [PATCH v5 3/5] KVM: VMX: Move real-mode interrupt injection fixup to vmx_complete_interrupts() Avi Kivity ` (2 subsequent siblings) 4 siblings, 0 replies; 20+ messages in thread From: Avi Kivity @ 2010-08-30 12:35 UTC (permalink / raw) To: Marcelo Tosatti, kvm vmx_complete_interrupts() does too much, split it up: - vmx_vcpu_run() gets the "cache important vmcs fields" part - a new vmx_complete_atomic_exit() gets the parts that must be done atomically - a new vmx_recover_nmi_blocking() does what its name says - vmx_complete_interrupts() retains the event injection recovery code This helps in reducing the work done in atomic context. Signed-off-by: Avi Kivity <avi@redhat.com> --- arch/x86/kvm/vmx.c | 39 +++++++++++++++++++++++++++------------ 1 files changed, 27 insertions(+), 12 deletions(-) diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 4255856..521df28 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -125,6 +125,7 @@ struct vcpu_vmx { unsigned long host_rsp; int launched; u8 fail; + u32 exit_intr_info; u32 idt_vectoring_info; struct shared_msr_entry *guest_msrs; int nmsrs; @@ -3781,18 +3782,9 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr) vmcs_write32(TPR_THRESHOLD, irr); } -static void vmx_complete_interrupts(struct vcpu_vmx *vmx) +static void vmx_complete_atomic_exit(struct vcpu_vmx *vmx) { - u32 exit_intr_info; - u32 idt_vectoring_info = vmx->idt_vectoring_info; - bool unblock_nmi; - u8 vector; - int type; - bool idtv_info_valid; - - exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO); - - vmx->exit_reason = vmcs_read32(VM_EXIT_REASON); + u32 exit_intr_info = vmx->exit_intr_info; /* Handle machine checks before interrupts are enabled */ if ((vmx->exit_reason == EXIT_REASON_MCE_DURING_VMENTRY) @@ -3807,8 +3799,16 @@ static void vmx_complete_interrupts(struct vcpu_vmx *vmx) asm("int $2"); kvm_after_handle_nmi(&vmx->vcpu); } +} - idtv_info_valid = idt_vectoring_info & VECTORING_INFO_VALID_MASK; +static void vmx_recover_nmi_blocking(struct vcpu_vmx *vmx) +{ + u32 exit_intr_info = vmx->exit_intr_info; + bool unblock_nmi; + u8 vector; + bool idtv_info_valid; + + idtv_info_valid = vmx->idt_vectoring_info & VECTORING_INFO_VALID_MASK; if (cpu_has_virtual_nmis()) { unblock_nmi = (exit_intr_info & INTR_INFO_UNBLOCK_NMI) != 0; @@ -3830,6 +3830,16 @@ static void vmx_complete_interrupts(struct vcpu_vmx *vmx) } else if (unlikely(vmx->soft_vnmi_blocked)) vmx->vnmi_blocked_time += ktime_to_ns(ktime_sub(ktime_get(), vmx->entry_time)); +} + +static void vmx_complete_interrupts(struct vcpu_vmx *vmx) +{ + u32 idt_vectoring_info = vmx->idt_vectoring_info; + u8 vector; + int type; + bool idtv_info_valid; + + idtv_info_valid = idt_vectoring_info & VECTORING_INFO_VALID_MASK; vmx->vcpu.arch.nmi_injected = false; kvm_clear_exception_queue(&vmx->vcpu); @@ -4042,6 +4052,11 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu) asm("mov %0, %%ds; mov %0, %%es" : : "r"(__USER_DS)); vmx->launched = 1; + vmx->exit_reason = vmcs_read32(VM_EXIT_REASON); + vmx->exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO); + + vmx_complete_atomic_exit(vmx); + vmx_recover_nmi_blocking(vmx); vmx_complete_interrupts(vmx); } -- 1.7.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 3/5] KVM: VMX: Move real-mode interrupt injection fixup to vmx_complete_interrupts() 2010-08-30 12:35 [PATCH v5 0/5] Nonatomic interrupt injection Avi Kivity 2010-08-30 12:35 ` [PATCH v5 1/5] KVM: Check for pending events before attempting injection Avi Kivity 2010-08-30 12:35 ` [PATCH v5 2/5] KVM: VMX: Split up vmx_complete_interrupts() Avi Kivity @ 2010-08-30 12:35 ` Avi Kivity 2010-08-30 12:35 ` [PATCH v5 4/5] KVM: VMX: Parameterize vmx_complete_interrupts() for both exit and entry Avi Kivity 2010-08-30 12:35 ` [PATCH v5 5/5] KVM: Non-atomic interrupt injection Avi Kivity 4 siblings, 0 replies; 20+ messages in thread From: Avi Kivity @ 2010-08-30 12:35 UTC (permalink / raw) To: Marcelo Tosatti, kvm This allows reuse of vmx_complete_interrupts() for cancelling injections. Signed-off-by: Avi Kivity <avi@redhat.com> --- arch/x86/kvm/vmx.c | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-) diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 521df28..39dd627 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -182,6 +182,7 @@ static int init_rmode(struct kvm *kvm); static u64 construct_eptp(unsigned long root_hpa); static void kvm_cpu_vmxon(u64 addr); static void kvm_cpu_vmxoff(void); +static void fixup_rmode_irq(struct vcpu_vmx *vmx); static DEFINE_PER_CPU(struct vmcs *, vmxarea); static DEFINE_PER_CPU(struct vmcs *, current_vmcs); @@ -3834,11 +3835,15 @@ static void vmx_recover_nmi_blocking(struct vcpu_vmx *vmx) static void vmx_complete_interrupts(struct vcpu_vmx *vmx) { - u32 idt_vectoring_info = vmx->idt_vectoring_info; + u32 idt_vectoring_info; u8 vector; int type; bool idtv_info_valid; + if (vmx->rmode.irq.pending) + fixup_rmode_irq(vmx); + + idt_vectoring_info = vmx->idt_vectoring_info; idtv_info_valid = idt_vectoring_info & VECTORING_INFO_VALID_MASK; vmx->vcpu.arch.nmi_injected = false; @@ -4046,8 +4051,6 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu) vcpu->arch.regs_dirty = 0; vmx->idt_vectoring_info = vmcs_read32(IDT_VECTORING_INFO_FIELD); - if (vmx->rmode.irq.pending) - fixup_rmode_irq(vmx); asm("mov %0, %%ds; mov %0, %%es" : : "r"(__USER_DS)); vmx->launched = 1; -- 1.7.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 4/5] KVM: VMX: Parameterize vmx_complete_interrupts() for both exit and entry 2010-08-30 12:35 [PATCH v5 0/5] Nonatomic interrupt injection Avi Kivity ` (2 preceding siblings ...) 2010-08-30 12:35 ` [PATCH v5 3/5] KVM: VMX: Move real-mode interrupt injection fixup to vmx_complete_interrupts() Avi Kivity @ 2010-08-30 12:35 ` Avi Kivity 2010-08-30 12:35 ` [PATCH v5 5/5] KVM: Non-atomic interrupt injection Avi Kivity 4 siblings, 0 replies; 20+ messages in thread From: Avi Kivity @ 2010-08-30 12:35 UTC (permalink / raw) To: Marcelo Tosatti, kvm Currently vmx_complete_interrupts() can decode event information from vmx exit fields into the generic kvm event queues. Make it able to decode the information from the entry fields as well by parametrizing it. Signed-off-by: Avi Kivity <avi@redhat.com> --- arch/x86/kvm/vmx.c | 34 +++++++++++++++++++++------------- 1 files changed, 21 insertions(+), 13 deletions(-) diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 39dd627..c6d37df 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -182,7 +182,7 @@ static int init_rmode(struct kvm *kvm); static u64 construct_eptp(unsigned long root_hpa); static void kvm_cpu_vmxon(u64 addr); static void kvm_cpu_vmxoff(void); -static void fixup_rmode_irq(struct vcpu_vmx *vmx); +static void fixup_rmode_irq(struct vcpu_vmx *vmx, u32 *idt_vectoring_info); static DEFINE_PER_CPU(struct vmcs *, vmxarea); static DEFINE_PER_CPU(struct vmcs *, current_vmcs); @@ -3833,17 +3833,18 @@ static void vmx_recover_nmi_blocking(struct vcpu_vmx *vmx) ktime_to_ns(ktime_sub(ktime_get(), vmx->entry_time)); } -static void vmx_complete_interrupts(struct vcpu_vmx *vmx) +static void __vmx_complete_interrupts(struct vcpu_vmx *vmx, + u32 idt_vectoring_info, + int instr_len_field, + int error_code_field) { - u32 idt_vectoring_info; u8 vector; int type; bool idtv_info_valid; if (vmx->rmode.irq.pending) - fixup_rmode_irq(vmx); + fixup_rmode_irq(vmx, &idt_vectoring_info); - idt_vectoring_info = vmx->idt_vectoring_info; idtv_info_valid = idt_vectoring_info & VECTORING_INFO_VALID_MASK; vmx->vcpu.arch.nmi_injected = false; @@ -3871,18 +3872,18 @@ static void vmx_complete_interrupts(struct vcpu_vmx *vmx) break; case INTR_TYPE_SOFT_EXCEPTION: vmx->vcpu.arch.event_exit_inst_len = - vmcs_read32(VM_EXIT_INSTRUCTION_LEN); + vmcs_read32(instr_len_field); /* fall through */ case INTR_TYPE_HARD_EXCEPTION: if (idt_vectoring_info & VECTORING_INFO_DELIVER_CODE_MASK) { - u32 err = vmcs_read32(IDT_VECTORING_ERROR_CODE); + u32 err = vmcs_read32(error_code_field); kvm_queue_exception_e(&vmx->vcpu, vector, err); } else kvm_queue_exception(&vmx->vcpu, vector); break; case INTR_TYPE_SOFT_INTR: vmx->vcpu.arch.event_exit_inst_len = - vmcs_read32(VM_EXIT_INSTRUCTION_LEN); + vmcs_read32(instr_len_field); /* fall through */ case INTR_TYPE_EXT_INTR: kvm_queue_interrupt(&vmx->vcpu, vector, @@ -3893,24 +3894,31 @@ static void vmx_complete_interrupts(struct vcpu_vmx *vmx) } } +static void vmx_complete_interrupts(struct vcpu_vmx *vmx) +{ + __vmx_complete_interrupts(vmx, vmx->idt_vectoring_info, + VM_EXIT_INSTRUCTION_LEN, + IDT_VECTORING_ERROR_CODE); +} + /* * Failure to inject an interrupt should give us the information * in IDT_VECTORING_INFO_FIELD. However, if the failure occurs * when fetching the interrupt redirection bitmap in the real-mode * tss, this doesn't happen. So we do it ourselves. */ -static void fixup_rmode_irq(struct vcpu_vmx *vmx) +static void fixup_rmode_irq(struct vcpu_vmx *vmx, u32 *idt_vectoring_info) { vmx->rmode.irq.pending = 0; if (kvm_rip_read(&vmx->vcpu) + 1 != vmx->rmode.irq.rip) return; kvm_rip_write(&vmx->vcpu, vmx->rmode.irq.rip); - if (vmx->idt_vectoring_info & VECTORING_INFO_VALID_MASK) { - vmx->idt_vectoring_info &= ~VECTORING_INFO_TYPE_MASK; - vmx->idt_vectoring_info |= INTR_TYPE_EXT_INTR; + if (*idt_vectoring_info & VECTORING_INFO_VALID_MASK) { + *idt_vectoring_info &= ~VECTORING_INFO_TYPE_MASK; + *idt_vectoring_info |= INTR_TYPE_EXT_INTR; return; } - vmx->idt_vectoring_info = + *idt_vectoring_info = VECTORING_INFO_VALID_MASK | INTR_TYPE_EXT_INTR | vmx->rmode.irq.vector; -- 1.7.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 5/5] KVM: Non-atomic interrupt injection 2010-08-30 12:35 [PATCH v5 0/5] Nonatomic interrupt injection Avi Kivity ` (3 preceding siblings ...) 2010-08-30 12:35 ` [PATCH v5 4/5] KVM: VMX: Parameterize vmx_complete_interrupts() for both exit and entry Avi Kivity @ 2010-08-30 12:35 ` Avi Kivity 4 siblings, 0 replies; 20+ messages in thread From: Avi Kivity @ 2010-08-30 12:35 UTC (permalink / raw) To: Marcelo Tosatti, kvm Change the interrupt injection code to work from preemptible, interrupts enabled context. This works by adding a ->cancel_injection() operation that undoes an injection in case we were not able to actually enter the guest (this condition could never happen with atomic injection). Signed-off-by: Avi Kivity <avi@redhat.com> --- arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/svm.c | 12 ++++++++++++ arch/x86/kvm/vmx.c | 11 +++++++++++ arch/x86/kvm/x86.c | 36 ++++++++++++++++-------------------- 4 files changed, 40 insertions(+), 20 deletions(-) diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 9b30285..8004b34 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -510,6 +510,7 @@ struct kvm_x86_ops { void (*queue_exception)(struct kvm_vcpu *vcpu, unsigned nr, bool has_error_code, u32 error_code, bool reinject); + void (*cancel_injection)(struct kvm_vcpu *vcpu); int (*interrupt_allowed)(struct kvm_vcpu *vcpu); int (*nmi_allowed)(struct kvm_vcpu *vcpu); bool (*get_nmi_mask)(struct kvm_vcpu *vcpu); diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 8884e51..e64a5b1 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -3160,6 +3160,17 @@ static void svm_complete_interrupts(struct vcpu_svm *svm) } } +static void svm_cancel_injection(struct kvm_vcpu *vcpu) +{ + struct vcpu_svm *svm = to_svm(vcpu); + struct vmcb_control_area *control = &svm->vmcb->control; + + control->exit_int_info = control->event_inj; + control->exit_int_info_err = control->event_inj_err; + control->event_inj = 0; + svm_complete_interrupts(svm); +} + #ifdef CONFIG_X86_64 #define R "r" #else @@ -3523,6 +3534,7 @@ static struct kvm_x86_ops svm_x86_ops = { .set_irq = svm_set_irq, .set_nmi = svm_inject_nmi, .queue_exception = svm_queue_exception, + .cancel_injection = svm_cancel_injection, .interrupt_allowed = svm_interrupt_allowed, .nmi_allowed = svm_nmi_allowed, .get_nmi_mask = svm_get_nmi_mask, diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index c6d37df..41b1ff6 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -3901,6 +3901,16 @@ static void vmx_complete_interrupts(struct vcpu_vmx *vmx) IDT_VECTORING_ERROR_CODE); } +static void vmx_cancel_injection(struct kvm_vcpu *vcpu) +{ + __vmx_complete_interrupts(to_vmx(vcpu), + vmcs_read32(VM_ENTRY_INTR_INFO_FIELD), + VM_ENTRY_INSTRUCTION_LEN, + VM_ENTRY_EXCEPTION_ERROR_CODE); + + vmcs_write32(VM_ENTRY_INTR_INFO_FIELD, 0); +} + /* * Failure to inject an interrupt should give us the information * in IDT_VECTORING_INFO_FIELD. However, if the failure occurs @@ -4354,6 +4364,7 @@ static struct kvm_x86_ops vmx_x86_ops = { .set_irq = vmx_inject_irq, .set_nmi = vmx_inject_nmi, .queue_exception = vmx_queue_exception, + .cancel_injection = vmx_cancel_injection, .interrupt_allowed = vmx_interrupt_allowed, .nmi_allowed = vmx_nmi_allowed, .get_nmi_mask = vmx_get_nmi_mask, diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index ac13791..bfe8f51 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4877,7 +4877,6 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) int r; bool req_int_win = !irqchip_in_kernel(vcpu->kvm) && vcpu->run->request_interrupt_window; - bool req_event; if (vcpu->requests) { if (kvm_check_request(KVM_REQ_MMU_RELOAD, vcpu)) @@ -4913,6 +4912,21 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) if (unlikely(r)) goto out; + if (kvm_check_request(KVM_REQ_EVENT, vcpu)) { + inject_pending_event(vcpu); + + /* enable NMI/IRQ window open exits if needed */ + if (vcpu->arch.nmi_pending) + kvm_x86_ops->enable_nmi_window(vcpu); + else if (kvm_cpu_has_interrupt(vcpu) || req_int_win) + kvm_x86_ops->enable_irq_window(vcpu); + + if (kvm_lapic_enabled(vcpu)) { + update_cr8_intercept(vcpu); + kvm_lapic_sync_to_vapic(vcpu); + } + } + preempt_disable(); kvm_x86_ops->prepare_guest_switch(vcpu); @@ -4925,35 +4939,17 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) local_irq_disable(); - req_event = kvm_check_request(KVM_REQ_EVENT, vcpu); - if (!atomic_read(&vcpu->guest_mode) || vcpu->requests || need_resched() || signal_pending(current)) { - if (req_event) - kvm_make_request(KVM_REQ_EVENT, vcpu); atomic_set(&vcpu->guest_mode, 0); smp_wmb(); local_irq_enable(); preempt_enable(); + kvm_x86_ops->cancel_injection(vcpu); r = 1; goto out; } - if (req_event) { - inject_pending_event(vcpu); - - /* enable NMI/IRQ window open exits if needed */ - if (vcpu->arch.nmi_pending) - kvm_x86_ops->enable_nmi_window(vcpu); - else if (kvm_cpu_has_interrupt(vcpu) || req_int_win) - kvm_x86_ops->enable_irq_window(vcpu); - - if (kvm_lapic_enabled(vcpu)) { - update_cr8_intercept(vcpu); - kvm_lapic_sync_to_vapic(vcpu); - } - } - srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx); kvm_guest_enter(); -- 1.7.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
end of thread, other threads:[~2011-02-08 20:40 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-08-30 12:35 [PATCH v5 0/5] Nonatomic interrupt injection Avi Kivity 2010-08-30 12:35 ` [PATCH v5 1/5] KVM: Check for pending events before attempting injection Avi Kivity 2011-02-07 6:00 ` [regression] KVM: hangs and "irq timeout" booting HURD unless -no-kvm-irqchip passed Jonathan Nieder 2011-02-07 12:39 ` Avi Kivity 2011-02-07 12:45 ` Gleb Natapov 2011-02-07 13:27 ` Gleb Natapov 2011-02-08 1:40 ` Jonathan Nieder 2011-02-08 7:40 ` Michael Tokarev 2011-02-08 12:00 ` Gleb Natapov 2011-02-08 14:22 ` Marcelo Tosatti 2011-02-08 14:41 ` Gleb Natapov 2011-02-08 14:43 ` Avi Kivity 2011-02-08 14:47 ` Gleb Natapov 2011-02-08 14:57 ` Avi Kivity 2011-02-08 17:44 ` Marcelo Tosatti 2011-02-08 20:40 ` Jonathan Nieder 2010-08-30 12:35 ` [PATCH v5 2/5] KVM: VMX: Split up vmx_complete_interrupts() Avi Kivity 2010-08-30 12:35 ` [PATCH v5 3/5] KVM: VMX: Move real-mode interrupt injection fixup to vmx_complete_interrupts() Avi Kivity 2010-08-30 12:35 ` [PATCH v5 4/5] KVM: VMX: Parameterize vmx_complete_interrupts() for both exit and entry Avi Kivity 2010-08-30 12:35 ` [PATCH v5 5/5] KVM: Non-atomic interrupt injection Avi Kivity
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.