Re: [PATCH v2 4/6] kvm: Extend irqfd to support level interrupts

[Alex Williamson <alex.williamson@redhat.com>]

On Thu, 2012-06-28 at 11:46 +0300, Gleb Natapov wrote:
> On Thu, Jun 28, 2012 at 11:41:05AM +0300, Michael S. Tsirkin wrote:
> > On Thu, Jun 28, 2012 at 11:35:41AM +0300, Gleb Natapov wrote:
> > > On Thu, Jun 28, 2012 at 11:34:35AM +0300, Michael S. Tsirkin wrote:
> > > > On Thu, Jun 28, 2012 at 09:34:31AM +0300, Gleb Natapov wrote:
> > > > > On Thu, Jun 28, 2012 at 01:31:29AM +0300, Michael S. Tsirkin wrote:
> > > > > > On Wed, Jun 27, 2012 at 04:04:18PM -0600, Alex Williamson wrote:
> > > > > > > On Wed, 2012-06-27 at 18:26 +0300, Michael S. Tsirkin wrote:
> > > > > > > > On Tue, Jun 26, 2012 at 11:09:46PM -0600, Alex Williamson wrote:
> > > > > > > > > @@ -71,6 +130,14 @@ irqfd_inject(struct work_struct *work)
> > > > > > > > >  	kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID, irqfd->gsi, 0);
> > > > > > > > >  }
> > > > > > > > >  
> > > > > > > > > +static void
> > > > > > > > > +irqfd_inject_level(struct work_struct *work)
> > > > > > > > > +{
> > > > > > > > > +	struct _irqfd *irqfd = container_of(work, struct _irqfd, inject);
> > > > > > > > > +
> > > > > > > > > +	kvm_set_irq(irqfd->kvm, irqfd->source->id, irqfd->gsi, 1);
> > > > > > > > > +}
> > > > > > > > > +
> > > > > > > > >  /*
> > > > > > > > >   * Race-free decouple logic (ordering is critical)
> > > > > > > > >   */
> > > > > > > > 
> > > > > > > > 
> > > > > > > > Why is it safe to ignore return value here?
> > > > > > > > needs a comment.
> > > > > > > 
> > > > > > > Well, it seems like you and Gleb came to the conclusion that it's safe,
> > > > > > > but I can really follow from the list thread.  Can you explain and I'll
> > > > > > > add a comment?  Thanks,
> > > > > > > 
> > > > > > > Alex
> > > > > > 
> > > > > > We merely talked about edge interrupts.
> > > > > > 
> > > > > In fact it would have been nice to return -EBUSY when write() to level
> > > > > irqfd is coalesced.
> > > > 
> > > > Possibly nice but not really practical.
> > > > 
> > > What do you mean by that? Impossible to implement or not useful?
> > 
> > Impossible to implement and also does not match normal eventfd
> > semantics.
> > 
> Hmm, I remember we discussed using irqfd for level triggered interrupt ~2
> years ago and came to a conclusion that eventfd is a bad fit for it,
> was true than is true now. Not be able to detect coalescing will make
> irqfd level interrupts inferior to IRQ_LINE ioctl.

Why do we care about coalescing?  I've been worried we need to re-inject
based on the return value of kvm_set_irq(), but re-reading specs and
code, we always post the interrupt to the irr.  For device assignment we
don't really care if kvm_set_irq() managed to actually inject the
interrupt, we're happy as long as it eventually hits the vcpu.  Current
device assignment uses kvm_set_irq() without looking for coalescing.
KVM_LINE_STATUS is the only caller that does something with the return
value and neither apic nor ioapic code in qemu do anything with the
value other than update accounting stats.  What am I missing that makes
the return value worth knowing?  Thanks,

Alex