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

["Michael S. Tsirkin" <mst@redhat.com>]

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?


> > But at least they actually worked reliably. Hmm.  One way to address
> > this is to add kvm_trigger_irq which will trigger edge interrupts
> > unconditionally.
> > 
> > Not sure it's worth it though.
> > 
> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > ---
> > > >  virt/kvm/eventfd.c | 1 -
> > > >  1 file changed, 1 deletion(-)
> > > > 
> > > > diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> > > > index f59c1e8..4db6b01 100644
> > > > --- a/virt/kvm/eventfd.c
> > > > +++ b/virt/kvm/eventfd.c
> > > > @@ -68,7 +68,6 @@ irqfd_inject(struct work_struct *work)
> > > >  	struct kvm *kvm = irqfd->kvm;
> > > >  
> > > >  	kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID, irqfd->gsi, 1);
> > > > -	kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID, irqfd->gsi, 0);
> > > >  }
> > > >  
> > > >  /*
> > > > -- 
> > > > MST
> > > 
> > > --
> > > 			Gleb.
> 
> --
> 			Gleb.