Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752787AbdHXJrg (ORCPT ); Thu, 24 Aug 2017 05:47:36 -0400 Received: from mail-oi0-f68.google.com ([209.85.218.68]:36202 "EHLO mail-oi0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751537AbdHXJrK (ORCPT ); Thu, 24 Aug 2017 05:47:10 -0400 MIME-Version: 1.0 In-Reply-To: <9d68348f-624d-9e27-d30d-64a18ac83feb@redhat.com> References: <1503548506-4457-1-git-send-email-wanpeng.li@hotmail.com> <1503548506-4457-2-git-send-email-wanpeng.li@hotmail.com> <9d68348f-624d-9e27-d30d-64a18ac83feb@redhat.com> From: Wanpeng Li Date: Thu, 24 Aug 2017 17:47:09 +0800 Message-ID: Subject: Re: [PATCH v3 2/4] KVM: X86: Fix loss of exception which has not yet injected To: Paolo Bonzini Cc: "linux-kernel@vger.kernel.org" , kvm , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Wanpeng Li Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2782 Lines: 74 2017-08-24 17:35 GMT+08:00 Paolo Bonzini : > On 24/08/2017 11:13, Wanpeng Li wrote: >> 2017-08-24 16:57 GMT+08:00 Paolo Bonzini : >>> On 24/08/2017 08:52, Wanpeng Li wrote: >>>>> @@ -6862,6 +6876,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) >>>>> kvm_x86_ops->enable_nmi_window(vcpu); >>>>> if (kvm_cpu_has_injectable_intr(vcpu) || req_int_win) >>>>> kvm_x86_ops->enable_irq_window(vcpu); >>>>> + WARN_ON(vcpu->arch.exception.pending); >>>> >>>> This WARN_ON() is suggested during the review of last version, >>>> however, there are many cases in inject_pending_event() can result in >>>> return directly w/ vcpu->arch.exception.pending is true. Actually I >>>> have already catched the warning several times during the testing. I >>>> think we should remove it when committing. >>> >>> No, it's a good thing that it's failing, because it's finding a bug. >>> There's no such thing as an "exception window", so at the very least it >> >> Good point, the code looks good, I will fold it in next version. >> However, I still can observe the warning. > > In patch 4, vmx_check_nested_events must clear vcpu->ex.pending. It is true, but I can still observe the warning. diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 6f88a79..4f97c4f 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -10990,6 +10990,8 @@ static int vmx_check_nested_events(struct kvm_vcpu *vcpu, bool external_intr) if (vmx->nested.nested_run_pending) return -EBUSY; + vcpu->arch.exception.pending = false; + vcpu->arch.exception.injected = true; nested_vmx_inject_exception_vmexit(vcpu, exit_qual); return 0; } diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index b698b2f..77e3031 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -6365,14 +6365,20 @@ static int inject_pending_event(struct kvm_vcpu *vcpu, bool req_int_win) return 0; } - if (vcpu->arch.nmi_injected) { - kvm_x86_ops->set_nmi(vcpu); - return 0; - } + /* + * Exceptions must be injected immediately, or the exception + * frame will have the address of the NMI or interrupt handler. + */ + if (!vcpu->arch.exception.pending) { + if (vcpu->arch.nmi_injected) { + kvm_x86_ops->set_nmi(vcpu); + return 0; + } - if (vcpu->arch.interrupt.pending) { - kvm_x86_ops->set_irq(vcpu); - return 0; + if (vcpu->arch.interrupt.pending) { + kvm_x86_ops->set_irq(vcpu); + return 0; + } } if (is_guest_mode(vcpu) && kvm_x86_ops->check_nested_events) {