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 <srutherford@xxxxxxxxxx>
> Signed-off-by: Matt Gingell <gingell@xxxxxxxxxx>
> ---
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
