Re: [v4 08/16] KVM: kvm-vfio: User API for IRQ forwarding

Alex Williamson
2015-06-12T15:41:38Z

On Fri, 2015-06-12 at 00:23 +0000, Wu, Feng wrote:
>
> > -----Original Message-----
> > From: Avi Kivity [mailto:avi.kivity@gmail.com]
> > Sent: Friday, June 12, 2015 3:59 AM
> > To: Wu, Feng; kvm@vger.kernel.org; linux-kernel@vger.kernel.org
> > Cc: pbonzini@redhat.com; mtosatti@redhat.com;
> > alex.williamson@redhat.com; eric.auger@linaro.org
> > Subject: Re: [v4 08/16] KVM: kvm-vfio: User API for IRQ forwarding
> >
> > On 06/11/2015 01:51 PM, Feng Wu wrote:
> > > From: Eric Auger <eric.auger@linaro.org>
> > >
> > > This patch adds and documents a new KVM_DEV_VFIO_DEVICE group
> > > and 2 device attributes: KVM_DEV_VFIO_DEVICE_FORWARD_IRQ,
> > > KVM_DEV_VFIO_DEVICE_UNFORWARD_IRQ. The purpose is to be able
> > > to set a VFIO device IRQ as forwarded or not forwarded.
> > > the command takes as argument a handle to a new struct named
> > > kvm_vfio_dev_irq.
> >
> > Is there no way to do this automatically? After all, vfio knows that a
> > device interrupt is forwarded to some eventfd, and kvm knows that some
> > eventfd is forwarded to a guest interrupt. If they compare notes
> > through a central registry, they can figure out that the interrupt needs
> > to be forwarded.
>
> Oh, just like Eric mentioned in his reply, this description is out of context of
> this series, I will remove them in the next version.

I suspect Avi's question was more general. While forward/unforward is
out of context for this series, it's very similar in nature to
enabling/disabling posted interrupts. So I think the question remains
whether we really need userspace to participate in creating this
shortcut or if kvm and vfio can some how orchestrate figuring it out
automatically.

Personally I don't know how we could do it automatically. We've always
relied on userspace to independently setup vfio and kvm such that
neither have any idea that the other is there and update each side
independently when anything changes. So it seems consistent to continue
that here. It doesn't seem like there's much to gain performance-wise
either, updates should be a relatively rare event I'd expect.

There's really no metadata associated with an eventfd, so "comparing
notes" automatically might imply some central registration entity. That
immediately sounds like a much more complex solution, but maybe Avi has
some ideas to manage it. Thanks,

Alex

> > > Signed-off-by: Eric Auger <eric.auger@linaro.org>
> > > ---
> > > Documentation/virtual/kvm/devices/vfio.txt | 34
> > ++++++++++++++++++++++++------
> > > include/uapi/linux/kvm.h | 12 +++++++++++
> > > 2 files changed, 40 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/Documentation/virtual/kvm/devices/vfio.txt
> > b/Documentation/virtual/kvm/devices/vfio.txt
> > > index ef51740..6186e6d 100644
> > > --- a/Documentation/virtual/kvm/devices/vfio.txt
> > > +++ b/Documentation/virtual/kvm/devices/vfio.txt
> > > @@ -4,15 +4,20 @@ VFIO virtual device
> > > Device types supported:
> > > KVM_DEV_TYPE_VFIO
> > >
> > > -Only one VFIO instance may be created per VM. The created device
> > > -tracks VFIO groups in use by the VM and features of those groups
> > > -important to the correctness and acceleration of the VM. As groups
> > > -are enabled and disabled for use by the VM, KVM should be updated
> > > -about their presence. When registered with KVM, a reference to the
> > > -VFIO-group is held by KVM.
> > > +Only one VFIO instance may be created per VM.
> > > +
> > > +The created device tracks VFIO groups in use by the VM and features
> > > +of those groups important to the correctness and acceleration of
> > > +the VM. As groups are enabled and disabled for use by the VM, KVM
> > > +should be updated about their presence. When registered with KVM,
> > > +a reference to the VFIO-group is held by KVM.
> > > +
> > > +The device also enables to control some IRQ settings of VFIO devices:
> > > +forwarding/posting.
> > >
> > > Groups:
> > > KVM_DEV_VFIO_GROUP
> > > + KVM_DEV_VFIO_DEVICE
> > >
> > > KVM_DEV_VFIO_GROUP attributes:
> > > KVM_DEV_VFIO_GROUP_ADD: Add a VFIO group to VFIO-KVM device
> > tracking
> > > @@ -20,3 +25,20 @@ KVM_DEV_VFIO_GROUP attributes:
> > >
> > > For each, kvm_device_attr.addr points to an int32_t file descriptor
> > > for the VFIO group.
> > > +
> > > +KVM_DEV_VFIO_DEVICE attributes:
> > > + KVM_DEV_VFIO_DEVICE_FORWARD_IRQ: set a VFIO device IRQ as
> > forwarded
> > > + KVM_DEV_VFIO_DEVICE_UNFORWARD_IRQ: set a VFIO device IRQ as
> > not forwarded
> > > +
> > > +For each, kvm_device_attr.addr points to a kvm_vfio_dev_irq struct.
> > > +
> > > +When forwarded, a physical IRQ is completed by the guest and not by the
> > > +host. This requires HW support in the interrupt controller.
> > > +
> > > +Forwarding can only be set when the corresponding VFIO IRQ is not masked
> > > +(would it be through VFIO_DEVICE_SET_IRQS command or as a consequence
> > of this
> > > +IRQ being currently handled) or active at interrupt controller level.
> > > +In such a situation, -EAGAIN is returned. It is advised to to set the
> > > +forwarding before the VFIO signaling is set up, this avoids trial and errors.
> > > +
> > > +Unforwarding can happen at any time.
> > > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > > index 4b60056..798f3e4 100644
> > > --- a/include/uapi/linux/kvm.h
> > > +++ b/include/uapi/linux/kvm.h
> > > @@ -999,6 +999,9 @@ struct kvm_device_attr {
> > > #define KVM_DEV_VFIO_GROUP 1
> > > #define KVM_DEV_VFIO_GROUP_ADD 1
> > > #define KVM_DEV_VFIO_GROUP_DEL 2
> > > +#define KVM_DEV_VFIO_DEVICE 2
> > > +#define KVM_DEV_VFIO_DEVICE_FORWARD_IRQ 1
> > > +#define KVM_DEV_VFIO_DEVICE_UNFORWARD_IRQ 2
> > >
> > > enum kvm_device_type {
> > > KVM_DEV_TYPE_FSL_MPIC_20 = 1,
> > > @@ -1018,6 +1021,15 @@ enum kvm_device_type {
> > > KVM_DEV_TYPE_MAX,
> > > };
> > >
> > > +struct kvm_vfio_dev_irq {
> > > + __u32 argsz; /* structure length */
> > > + __u32 fd; /* file descriptor of the VFIO device */
> > > + __u32 index; /* VFIO device IRQ index */
> > > + __u32 start; /* start of subindex range */
> > > + __u32 count; /* size of subindex range */
> > > + __u32 gsi[]; /* gsi, ie. virtual IRQ number */
> > > +};
> > > +
> > > /*
> > > * ioctls for VM fds
> > > */
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html