Re: [PATCH 3/8] KVM: Handle device assignment to guests

[Avi Kivity <avi@qumranet.com>]

Ben-Ami Yassour wrote:
> On Thu, 2008-07-17 at 12:50 +0300, Avi Kivity wrote:
>   
>> Ben-Ami Yassour wrote:
>>     
>>> On Wed, 2008-07-16 at 18:04 +0300, Avi Kivity wrote:
>>>   
>>>       
>> If a level triggered interrupt remains active after the eoi, the ioapic 
>> has to inject it.  This is used to support shared interrupts, or when 
>> 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.
>>
>>     
>
> The difference is that for emulated devices, qemu is resetting the irr
> 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 the
> eoi function, and actually this is what the ack notify handler is doing
> (by calling pci_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.
>   

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 gsi)
> {
> 	union ioapic_redir_entry *ent;
>
> 	ent = &ioapic->redirtbl[gsi];
> 	ASSERT(ent->fields.trig_mode == IOAPIC_LEVEL_TRIG);
>
> 	ent->fields.remote_irr = 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?
>
>   

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


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