DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com AC50A6147A
Subject: Re: [PATCH v3 2/4] KVM: X86: Fix loss of exception which has not yet
 injected
To: Wanpeng Li <kernellwp@gmail.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        kvm <kvm@vger.kernel.org>,
        =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= <rkrcmar@redhat.com>,
        Wanpeng Li <wanpeng.li@hotmail.com>
References: <1503548506-4457-1-git-send-email-wanpeng.li@hotmail.com>
 <1503548506-4457-2-git-send-email-wanpeng.li@hotmail.com>
 <CANRm+CwD7ff2qa7ywRwx_ThqradFvpEU6OQm_tp2=iBpU+LKXQ@mail.gmail.com>
 <f19b7992-155b-0809-805c-d61f5f8c1c1d@redhat.com>
 <CANRm+CxQuLnsiTkhGPiU4fF-JK52CY_n1oOeAsv+8LvTfsKqzA@mail.gmail.com>
From: Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <9d68348f-624d-9e27-d30d-64a18ac83feb@redhat.com>
Date: Thu, 24 Aug 2017 11:35:47 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.2.1
MIME-Version: 1.0
In-Reply-To: <CANRm+CxQuLnsiTkhGPiU4fF-JK52CY_n1oOeAsv+8LvTfsKqzA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2509
Lines: 66

On 24/08/2017 11:13, Wanpeng Li wrote:
> 2017-08-24 16:57 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>> 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.

Paolo

> Regards,
> Wanpeng Li
> 
>> should set req_immediate_exit to true.
>>
>> Does this help?
>>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index b698b2f135a2..76d5a192be6c 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) {
>>
>> Paolo