On 17/11/2015 00:26, Matt Gingell wrote:
> Before this patch, we incorrectly enter the guest without requesting an
> interrupt window if the IRQ chip is split between user space and the
> kernel.
> 
> Because lapic_in_kernel no longer implies the PIC is in the kernel, this
> patch tests pic_in_kernel to determining whether an interrupt window
> should be requested when entering the guest.
> 
> If the APIC is in the kernel and we request an interrupt window the
> guest will return immediately. If the APIC is masked the guest will not
> not make forward progress and unmask it, leading to a loop when KVM
> reenters and requests again. This patch adds a check to ensure the APIC
> is ready to accept an interrupt before requesting a window.
> 
> Reviewed-by: Steve Rutherford <srutherf...@google.com>
> Signed-off-by: Matt Gingell <ging...@google.com>
> ---
>  arch/x86/kvm/x86.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index c370eef..d57bdd9 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -6259,8 +6259,11 @@ void kvm_arch_mmu_notifier_invalidate_page(struct kvm 
> *kvm,
>  static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>  {
>       int r;
> -     bool req_int_win = !lapic_in_kernel(vcpu) &&
> -             vcpu->run->request_interrupt_window;
> +     bool req_int_win =
> +             vcpu->run->request_interrupt_window &&
> +             likely(!pic_in_kernel(vcpu->kvm)) &&
> +             (!lapic_in_kernel(vcpu) || kvm_apic_accept_pic_intr(vcpu));
> +

This can be

        bool req_int_win =
                dm_request_for_irq_injection(vcpu) &&
                (!lapic_in_kernel(vcpu) || kvm_apic_accept_pic_intr(vcpu));

I'll apply the patches and send them to Linus for 4.4-rc2, thanks!
These cleanups can go on top:

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 2e16068bba51..0bca1ec199df 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2661,12 +2661,24 @@ static int kvm_vcpu_ioctl_set_lapic(struct kvm_vcpu 
*vcpu,
        return 0;
 }
 
-static int kvm_vcpu_ready_for_interrupt_injection(struct kvm_vcpu *vcpu) {
+static int kvm_cpu_accept_dm_intr(struct kvm_vcpu *vcpu)
+{
+       return (!lapic_in_kernel(vcpu) ||
+               kvm_apic_accept_pic_intr(vcpu));
+}
+
+/*
+ * if userspace requested an interrupt window, check that the
+ * interrupt window is open.
+ *
+ * No need to exit to userspace if we already have an interrupt queued.
+ */
+static int kvm_vcpu_ready_for_interrupt_injection(struct kvm_vcpu *vcpu)
+{
        return kvm_arch_interrupt_allowed(vcpu) &&
                !kvm_cpu_has_interrupt(vcpu) &&
                !kvm_event_needs_reinjection(vcpu) &&
-               (!lapic_in_kernel(vcpu) ||
-                kvm_apic_accept_pic_intr(vcpu));
+               kvm_cpu_accept_dm_intr(vcpu);
 }
 
 static int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu,
@@ -5817,12 +5829,6 @@ static int emulator_fix_hypercall(struct 
x86_emulate_ctxt *ctxt)
        return emulator_write_emulated(ctxt, rip, instruction, 3, NULL);
 }
 
-/*
- * Check if userspace requested an interrupt window, and that the
- * interrupt window is open.
- *
- * No need to exit to userspace if we already have an interrupt queued.
- */
 static int dm_request_for_irq_injection(struct kvm_vcpu *vcpu)
 {
        return vcpu->run->request_interrupt_window &&
@@ -6253,9 +6259,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 {
        int r;
        bool req_int_win =
-               vcpu->run->request_interrupt_window &&
-               likely(!pic_in_kernel(vcpu->kvm)) &&
-               (!lapic_in_kernel(vcpu) || kvm_apic_accept_pic_intr(vcpu));
+               dm_request_for_irq_injection(vcpu) &&
+               kvm_cpu_accept_dm_intr(vcpu);
 
        bool req_immediate_exit = false;
 

A couple questions, that can be fixed by separate patches:

- should kvm_cpu_accept_dm_intr check that pending_external_vector == -1?

- should kvm_vcpu_ioctl_interrupt then use kvm_cpu_accept_dm_intr
instead of checking pending_external_vector == -1 directly?

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to