Re: [PATCH] kvm: don't clear level from irqfd

Alex Williamson
2012-06-27T14:42:00Z

On Wed, 2012-06-27 at 14:38 +0300, Michael S. Tsirkin wrote:
> On Wed, Jun 27, 2012 at 01:22:54PM +0300, Gleb Natapov wrote:
> > On Wed, Jun 27, 2012 at 01:20:24PM +0300, Michael S. Tsirkin wrote:
> > > On Wed, Jun 27, 2012 at 01:03:46PM +0300, Gleb Natapov wrote:
> > > > On Wed, Jun 27, 2012 at 12:13:43PM +0300, Michael S. Tsirkin wrote:
> > > > > irqfd sets and clears the level immediately.
> > > > > This used to be necessary for MSI interrupts but isn't anymore.
> > > > >
> > > > > Note! We never officially supported irqfd for level
> > > > > interrupts but it usually seemed to kind of work,
> > > > > and this change will break it. There are no known
> > > > > real users of that feature though - only old vfio prototypes.
> > > > > Removing this line will make it easier to add
> > > > > real support for level irqfds in the future.
> > > > >
> > > > Did we officially supported irqfd for ioapic/pic edge interrupts?
> > > > Because those will break to.
> > >
> > > Hard to say - as Alex points out it was never documented.
> > If we will start dropping everything that was never documented we will
> > hardly have working VMM at the end :)
>
> Generally true.
>
> However I just realized that this interface does not work
> reliably for ioapic/pic edge either. More or less for the same reason that
> it does not work for edge: we set bit in IRR, try to inject,
> then clear even if injection failed (e.g. because it is low
> priority). Guest will never get an interrupt in this case.
>
> So what happened e.g. for Alex's vfio prototype is he never
> hit an interrupt window in light testing.
>
> What we would need to do to fix it is:
> > > > > - kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID, irqfd->gsi, 1);
> > > > > - kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID, irqfd->gsi, 0);
> > > > > + kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID, irqfd->gsi, 0);
> > > > > + kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID, irqfd->gsi, 1);
>
> Which will fix edge but break level same as this patch.
> But is it worth fixing? Maybe declare it broken and be
> done with it?

Ugh, so if I understand correctly, the case where kvm_set_irq() returns
<0, we're broken. VFIO has masked the physical device and has no error
path to know that the interrupt inject was unsuccessful. We almost need
an error eventfd that bounces the interrupt back to userspace for the
qemu interrupt model to handle, but that has it's own set of
complications. Suggestions? Declaring the interface broken isn't very
useful. Thanks,

Alex