From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@qumranet.com>
Subject: Re: [PATCH 3/8] KVM: Handle device assignment to guests
Date: Thu, 17 Jul 2008 21:04:30 +0300
Message-ID: <487F89AE.2090807@qumranet.com>
References: <1216214225-18030-1-git-send-email-benami@il.ibm.com>	 <1216214225-18030-2-git-send-email-benami@il.ibm.com>	 <1216214225-18030-3-git-send-email-benami@il.ibm.com>	 <1216214225-18030-4-git-send-email-benami@il.ibm.com>	 <487E0E10.4000009@qumranet.com>	 <1216287258.31546.337.camel@cluwyn.haifa.ibm.com>	 <487F15CE.5050204@qumranet.com> <1216316443.31546.355.camel@cluwyn.haifa.ibm.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8;
	format=flowed
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: amit.shah@qumranet.com, kvm@vger.kernel.org,
	Muli Ben-Yehuda <muli@il.ibm.com>, weidong.han@intel.com,
	anthony@codemonkey.ws
To: Ben-Ami Yassour <benami@il.ibm.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from il.qumranet.com ([212.179.150.194]:12418 "EHLO il.qumranet.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1757652AbYGQSEf (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 17 Jul 2008 14:04:35 -0400
In-Reply-To: <1216316443.31546.355.camel@cluwyn.haifa.ibm.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Ben-Ami Yassour wrote:
> On Thu, 2008-07-17 at 12:50 +0300, Avi Kivity wrote:
>  =20
>> Ben-Ami Yassour wrote:
>>    =20
>>> On Wed, 2008-07-16 at 18:04 +0300, Avi Kivity wrote:
>>>  =20
>>>      =20
>> If a level triggered interrupt remains active after the eoi, the ioa=
pic=20
>> has to inject it.  This is used to support shared interrupts, or whe=
n=20
>> the device has re-raised the line by the time the ack arrives.
>>
>> I don't see why it should behave differently for assigned devices.
>>
>>    =20
>
> The difference is that for emulated devices, qemu is resetting the ir=
r
> bit. For assigned devices it does not, and that's the difference.
> The first chance that we can clear the irr bit for real devices is th=
e
> eoi function, and actually this is what the ack notify handler is doi=
ng
> (by calling =EF=BB=BF=EF=BB=BFpci_set_irq(kvm,irq,0) ).
> I was able to remove the code in ioapic by calling the ack notify
> handler before the irr check, and it seems to work fine.
>  =20

Okay.

> (to make it work, I also had to remove the queuing of the ack handler
> which was not necessary, as you mentioned in earlier comment)
>
> The eoi function now looks like this:
> static void __kvm_ioapic_update_eoi(struct kvm_ioapic *ioapic, int gs=
i)
> {
> 	union ioapic_redir_entry *ent;
>
> 	ent =3D &ioapic->redirtbl[gsi];
> 	ASSERT(ent->fields.trig_mode =3D=3D IOAPIC_LEVEL_TRIG);
>
> 	ent->fields.remote_irr =3D 0;
>
> 	if (ioapic->ack_notifier)
> 		ioapic->ack_notifier(ioapic->kvm, gsi);
>
> 	if (!ent->fields.mask && (ioapic->irr & (1 << gsi)))
> 		ioapic_service(ioapic, gsi);
> }
>
> Any comments on such an approach?
>
>  =20

I think it's fine.  The point where the ack notifier is called is=20
between the end of service of the old interrupt, and the beginning of=20
service of a potential new interrupt (from the same device or some othe=
r=20
device on the same guest line).


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