From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@redhat.com>
Subject: Re: [PATCH v2 3/3] KVM: Non-atomic interrupt injection
Date: Wed, 21 Jul 2010 08:37:26 +0300
Message-ID: <4C468796.4060700@redhat.com>
References: <1279631827-4016-1-git-send-email-avi@redhat.com> <1279631827-4016-4-git-send-email-avi@redhat.com> <20100721005533.GA9989@amt.cnet>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: kvm@vger.kernel.org
To: Marcelo Tosatti <mtosatti@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:59062 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751378Ab0GUFha (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 21 Jul 2010 01:37:30 -0400
Received: from int-mx08.intmail.prod.int.phx2.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.21])
	by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o6L5bUT3009836
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK)
	for <kvm@vger.kernel.org>; Wed, 21 Jul 2010 01:37:30 -0400
In-Reply-To: <20100721005533.GA9989@amt.cnet>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 07/21/2010 03:55 AM, Marcelo Tosatti wrote:
>
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -4709,6 +4709,19 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>>   	if (unlikely(r))
>>   		goto out;
>>
>> +	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);
>> +
>> +	if (kvm_lapic_enabled(vcpu)) {
>> +		update_cr8_intercept(vcpu);
>> +		kvm_lapic_sync_to_vapic(vcpu);
>> +	}
>> +
>>   	preempt_disable();
>>
>>   	kvm_x86_ops->prepare_guest_switch(vcpu);
>> @@ -4727,23 +4740,11 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>>   		smp_wmb();
>>   		local_irq_enable();
>>   		preempt_enable();
>> +		kvm_x86_ops->cancel_injection(vcpu);
>>   		r = 1;
>>   		goto out;
>>   	}
>>
>> -	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);
>> -
>> -	if (kvm_lapic_enabled(vcpu)) {
>> -		update_cr8_intercept(vcpu);
>> -		kvm_lapic_sync_to_vapic(vcpu);
>> -	}
>> -
>>   	srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx);
>>
>>   	kvm_guest_enter();
>>      
> This breaks
>
> int kvm_lapic_find_highest_irr(struct kvm_vcpu *vcpu)
> {
>          struct kvm_lapic *apic = vcpu->arch.apic;
>          int highest_irr;
>
>          /* This may race with setting of irr in __apic_accept_irq() and
>           * value returned may be wrong, but kvm_vcpu_kick() in
>           * __apic_accept_irq
>           * will cause vmexit immediately and the value will be
>           * recalculated
>           * on the next vmentry.
>           */
>
> (also valid for nmi_pending and PIC). Can't simply move
> atomic_set(guest_mode, 1) in preemptible section as that would make it
> possible for kvm_vcpu_kick to IPI stale vcpu->cpu.
>    

Right.  Can fix by adding a kvm_make_request() to force the retry loop.

> Also should undo vmx.rmode.* ?
>    

Elaborate?

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