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

Avi Kivity
2008-07-17T18:04:35Z

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.