Re: [PATCH 1/6] KVM: Check for pending events before attempting injection

[Avi Kivity <avi@redhat.com>]

  On 07/28/2010 07:37 PM, Marcelo Tosatti wrote:
> On Wed, Jul 28, 2010 at 07:31:03PM +0300, Avi Kivity wrote:
>>   On 07/28/2010 07:21 PM, Marcelo Tosatti wrote:
>>> On Tue, Jul 27, 2010 at 04:19:35PM +0300, Avi Kivity wrote:
>>>> Instead of blindly attempting to inject an event before each guest entry,
>>>> check for a possible event first in vcpu->requests.  Sites that can trigger
>>>> event injection are modified to set KVM_REQ_EVENT:
>>>>
>>>> - interrupt, nmi window opening
>>>> - ppr updates
>>>> - i8259 output changes
>>>> - local apic irr changes
>>>> - rflags updates
>>>> - gif flag set
>>>> - event set on exit
>>>>
>>>> This improves non-injecting entry performance, and sets the stage for
>>>> non-atomic injection.
>>>>
>>>> Signed-off-by: Avi Kivity<avi@redhat.com>
>>>> ---
>>>>   arch/x86/kvm/i8259.c     |    1 +
>>>>   arch/x86/kvm/lapic.c     |   12 ++++++++++--
>>>>   arch/x86/kvm/svm.c       |    8 +++++++-
>>>>   arch/x86/kvm/vmx.c       |    6 ++++++
>>>>   arch/x86/kvm/x86.c       |   35 ++++++++++++++++++++++++++---------
>>>>   include/linux/kvm_host.h |    1 +
>>>>   6 files changed, 51 insertions(+), 12 deletions(-)
>>>>
>>>> @@ -4731,17 +4737,19 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>>>>   		goto out;
>>>>   	}
>>>>
>>>> -	inject_pending_event(vcpu);
>>>> +	if (kvm_check_request(KVM_REQ_EVENT, vcpu)) {
>>>> +		inject_pending_event(vcpu);
>>>>
>>>> -	/* enable NMI/IRQ window open exits if needed */
>>>> -	if (vcpu->arch.nmi_pending)
>>>> -		kvm_x86_ops->enable_nmi_window(vcpu);
>>>> -	else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
>>>> -		kvm_x86_ops->enable_irq_window(vcpu);
>>>> +		/* enable NMI/IRQ window open exits if needed */
>>>> +		if (vcpu->arch.nmi_pending)
>>>> +			kvm_x86_ops->enable_nmi_window(vcpu);
>>>> +		else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
>>>> +			kvm_x86_ops->enable_irq_window(vcpu);
>>> Problem is it might not be possible to inject the event signalled by
>>> KVM_REQ_EVENT, say an interrupt from an irqchip, if there is an event
>>> that needs reinjection (or an exception).
>> That can happen event now, no?  A pending exception, interrupt comes
>> along, injection picks up the exception but leaves the interrupt.
>>
>> Now the situation can be more complicated:
>>
>> - pending exception
>> - injection
>> - interrupt, sets KVM_REQ_EVENT
>> - notices KVM_REQ_EVENT
>> - drops KVM_REQ_EVENT, cancels exception (made pending again)
>> - goes back
>> - injection (injects exception again, interrupt is pending)
>>
>> as far as I can tell, this is all fine.
> But you cleared KVM_REQ_EVENT. Which means you're not going to inject
> the pending interrupt on the next entry.

Doh.  So we need to set KVM_REQ_EVENT again, after the final check for 
vcpu->requests, to make sure we redo injection again.

So we can make inject_pending_event() return true if there's more in the 
queue, and if it did, re-raise KVM_REQ_EVENT just before entry?

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.