Re: [PATCH 4/4][RFC] kvm: eoi_eventfd

[Alex Williamson <alex.williamson@xxxxxxxxxx>]

On Sun, 2012-06-24 at 11:24 +0300, Michael S. Tsirkin wrote:
> On Fri, Jun 22, 2012 at 04:16:53PM -0600, Alex Williamson wrote:
> > I think we're probably also going to need something like this.
> > When running in non-accelerated qemu, we're going to have to
> > create some kind of EOI notifier for drivers.  VFIO can make
> > additional improvements when running on KVM so it will probably
> > make use of the KVM_IRQFD_LEVEL_EOI interface, but we don't
> > want to have a generic EOI notifier in qemu that just stops
> > working when kvm-ioapic is enabled.  This is just a simple way
> > to register an eventfd using the existing KVM ack notifier.
> > I tried combining the ack notifier of the LEVEL_EOI interface
> > into this one, but it didn't work out well.  The code complexity
> > goes up a lot.
> > 
> > Signed-off-by: Alex Williamson <alex.williamson@xxxxxxxxxx>
> > ---
> > 
> >  Documentation/virtual/kvm/api.txt |   14 ++++++
> >  arch/x86/kvm/x86.c                |    1 
> >  include/linux/kvm.h               |   12 +++++
> >  include/linux/kvm_host.h          |    7 +++
> >  virt/kvm/eventfd.c                |   89 +++++++++++++++++++++++++++++++++++++
> >  virt/kvm/kvm_main.c               |    9 ++++
> >  6 files changed, 132 insertions(+)
> > 
> > diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> > index 2f8a0aa..69b1747 100644
> > --- a/Documentation/virtual/kvm/api.txt
> > +++ b/Documentation/virtual/kvm/api.txt
> > @@ -1998,6 +1998,20 @@ matched using kvm_irqfd.fd, kvm_irqfd.gsi, and kvm_irqfd.fd2.
> >  De-assigning automatically de-asserts the interrupt line setup through
> >  this interface.
> >  
> > +4.77 KVM_EOI_EVENTFD
> > +
> > +Capability: KVM_CAP_EOI_EVENTFD
> > +Architectures: x86
> > +Type: vm ioctl
> > +Parameters: struct kvm_eoi_eventfd (in)
> > +Returns: 0 on success, -1 on error
> > +
> > +This interface allows userspace to be notified through an eventfd for
> > +EOI writes to the in-kernel irqchip.  kvm_eoi_eventfd.fd specifies
> > +the eventfd to signal on EOI to kvm_eoi_eventfd.gsi.  To disable,
> > +use KVM_EOI_EVENTFD_FLAG_DEASSIGN and specify both the original fd
> > +and gsi.
> > +
> >  5. The kvm_run structure
> >  ------------------------
> >  
> 
> The patch looks like it only works for ioapic IRQs.  I think it's a good
> idea to explicitly limit this to level interrupts, straight away:
> there's no reason for userspace to need an exit on an edge interrupt.

Are you suggesting documentation or code to prevent users from binding
to an edge interrupt?

> I also suggest we put LEVEL somewhere in the name.

Ok

> With this eventfd, do we also need the fd2 parameter in the irqfd
> structure? It seems to be used for the same thing ...

Yes we still need fd2, this does not replace KVM_IRQFD_LEVEL_EOI.  The
models we need to support are:

1) assert from userspace (IRQ_LINE), eoi to userspace (EOI_EVENTFD),
de-assert from userspace (IRQ_LINE)

2a) assert from vfio (IRQFD.fd), de-assert in kvm, notify vfio
(IRQFD.fd2)

or

2b) assert from vfio (IRQFD.fd), eoi to vfio (EOI_EVENTFD), de-assert
from vfio (IRQFD.fd2)

This series enables 1) and 2a).  2b) has some pros and cons.  The pro is
that vfio could test the device to see if an interrupt is pending and
not de-assert if there is still an interrupt pending.  The con is that a
standard level interrupt cycle takes 3 transactions instead of 2.

Any case of triggering the interrupt externally via an irqfd requires
that the irqfd uses it's own irq_source_id so that it doesn't interfere
with KVM_USERSPACE_IRQ_SOURCE_ID.  So we can't mix IRQ_LINE and IRQFD
unless we want to completely expose a new irq source id allocation
mechanism and include it in all the interfaces (KVM_GET_IRQ_SOURCE_ID,
KVM_PUT_IRQ_SOURCE_ID, pass a source id to IRQFD and IRQ_LINE, etc).
Thanks,

Alex

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/