On Fri, Nov 13, 2015 at 12:52 AM, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote: > > > On 12/11/2015 20:07, Matt Gingell wrote: >> This patch adds a call to kvm_arch_interrupt_allowed to ensure ready for >> interrupt is reported to user space correctly. This addresses a problem >> observed in QEMU when kvm->ready_for_interrupt is set but the x86 >> interrupt flag is clear. >> >> Additionally, test that the APIC is ready to accept an interrupt before >> reporting we are ready for injection. >> >> Reviewed-by: Andy Honig <ahonig@xxxxxxxxxx> >> Signed-off-by: Matt Gingell <gingell@xxxxxxxxxx> > > I think you need to add the same call to dm_request_for_irq_injection, like > > - return (irqchip_split(vcpu->kvm) > - ? kvm_apic_accept_pic_intr(vcpu) > - : kvm_arch_interrupt_allowed(vcpu)); > + if (!kvm_arch_interrupt_allowed(vcpu)) > + return false; > + > + return !lapic_in_kernel(vcpu) || kvm_apic_accept_pic_intr(vcpu); > > At this point, just to err on the safe side, we probably should test > kvm_event_needs_reinjection(vcpu) as well in dm_request_for_irq_injection. This is definitely necessary. Without it, it's possible to bounce back and forth between userspace and the kernel. (Actually ran into this in testing yesterday evening. If you ever want to stress test legacy interrupt handling devices, try booting Plan9) > > We can then make a new function kvm_vcpu_ready_for_interrupt_injection > with the sequence of tests (kvm_cpu_has_interrupt, > kvm_arch_interrupt_allowed, kvm_event_needs_reinjection, possibly > kvm_apic_accept_pic_intr) so that: > > - dm_request_for_irq_injection becomes simply > > return (vcpu->run->request_interrupt_window && > likely(!pic_in_kernel(vcpu->kvm)); > > - the caller of dm_request_for_irq_injection does > > if (dm_request_for_irq_injection(vcpu) && > kvm_vcpu_ready_for_interrupt_injection(vcpu)) > > - post_kvm_run_save's assignment becomes > > kvm_run->ready_for_interrupt_injection = > !pic_in_kernel(vcpu->kvm) || > kvm_vcpu_ready_for_interrupt_injection(vcpu); > > The code would make a lot of sense then; I hope it will work too. :) > > Paolo "ceci n'est pas une patch" -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
