RE: [v2 17/25] KVM: kvm-vfio: User API for VT-d Posted-Interrupts

From: Wu, Feng
Date: Tue Dec 09 2014 - 06:38:49 EST




> -----Original Message-----
> From: Alex Williamson [mailto:alex.williamson@xxxxxxxxxx]
> Sent: Monday, December 08, 2014 1:21 PM
> To: Wu, Feng
> Cc: Eric Auger; tglx@xxxxxxxxxxxxx; mingo@xxxxxxxxxx; hpa@xxxxxxxxx;
> x86@xxxxxxxxxx; gleb@xxxxxxxxxx; pbonzini@xxxxxxxxxx;
> dwmw2@xxxxxxxxxxxxx; joro@xxxxxxxxxx; jiang.liu@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx;
> kvm@xxxxxxxxxxxxxxx
> Subject: Re: [v2 17/25] KVM: kvm-vfio: User API for VT-d Posted-Interrupts
>
> On Mon, 2014-12-08 at 04:58 +0000, Wu, Feng wrote:
> >
> > > -----Original Message-----
> > > From: kvm-owner@xxxxxxxxxxxxxxx [mailto:kvm-owner@xxxxxxxxxxxxxxx]
> On
> > > Behalf Of Eric Auger
> > > Sent: Thursday, December 04, 2014 10:05 PM
> > > To: Wu, Feng; tglx@xxxxxxxxxxxxx; mingo@xxxxxxxxxx; hpa@xxxxxxxxx;
> > > x86@xxxxxxxxxx; gleb@xxxxxxxxxx; pbonzini@xxxxxxxxxx;
> > > dwmw2@xxxxxxxxxxxxx; joro@xxxxxxxxxx; alex.williamson@xxxxxxxxxx;
> > > jiang.liu@xxxxxxxxxxxxxxx
> > > Cc: linux-kernel@xxxxxxxxxxxxxxx; iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx;
> > > kvm@xxxxxxxxxxxxxxx
> > > Subject: Re: [v2 17/25] KVM: kvm-vfio: User API for VT-d Posted-Interrupts
> > >
> > > Hi Feng,
> > > On 12/03/2014 08:39 AM, Feng Wu wrote:
> > > > This patch adds and documents a new attribute
> > > > KVM_DEV_VFIO_DEVICE_POSTING_IRQ in KVM_DEV_VFIO_DEVICE
> group.
> > > > This new attribute is used for VT-d Posted-Interrupts.
> > > >
> > > > When guest OS changes the interrupt configuration for an
> > > > assigned device, such as, MSI/MSIx data/address fields,
> > > > QEMU will use this IRQ attribute to tell KVM to update the
> > > > related IRTE according the VT-d Posted-Interrrupts Specification,
> > > > such as, the guest vector should be updated in the related IRTE.
> > > >
> > > > Signed-off-by: Feng Wu <feng.wu@xxxxxxxxx>
> > > > ---
> > > > Documentation/virtual/kvm/devices/vfio.txt | 9 +++++++++
> > > > include/uapi/linux/kvm.h | 10 ++++++++++
> > > > 2 files changed, 19 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/Documentation/virtual/kvm/devices/vfio.txt
> > > b/Documentation/virtual/kvm/devices/vfio.txt
> > > > index f7aff29..41e12b7 100644
> > > > --- a/Documentation/virtual/kvm/devices/vfio.txt
> > > > +++ b/Documentation/virtual/kvm/devices/vfio.txt
> > > > @@ -42,3 +42,12 @@ activated before VFIO_DEVICE_SET_IRQS has been
> > > called to trigger the IRQ
> > > > or associate an eventfd to it. Unforwarding can only be called while the
> > > > signaling has been disabled with VFIO_DEVICE_SET_IRQS. If this
> condition
> > > is
> > > > not satisfied, the command returns an -EBUSY.
> > > > +
> > > > + KVM_DEV_VFIO_DEVICE_POSTING_IRQ: Use posted interrtups
> > > mechanism to post
> > > typo
> > > > + the IRQ to guests.
> > > > +For this attribute, kvm_device_attr.addr points to a kvm_vfio_dev_irq
> > > struct.
> > > > +
> > > > +When guest OS changes the interrupt configuration for an assigned
> device,
> > > > +such as, MSI/MSIx data/address fields, QEMU will use this IRQ attribute
> > > > +to tell KVM to update the related IRTE according the VT-d
> > > Posted-Interrrupts
> > > > +Specification, such as, the guest vector should be updated in the related
> > > IRTE.
> > > For my curiosity are there any restrictions about the instant at which
> > > the change can be done?
> > > I do not get here how you deactivate the posting?
> >
> > The current method is if the hardware supports interrupts posting, we will
> > use it instead of interrupts remapping, since it has good performance. Why
> > do I need deactivate interrupts posting?
> >
> > Here is the reply to Alex for the same question:
> > "In fact, I don't think we need to stop the posted-interrupts. For setting
> > posted interrupts, we update the related IRTE according to the new
> > format. If the guest reboots, or unload the drivers, or some other
> > operations, the msi/msix will be disabled first, in this path, the irq
> > will be disabled the related IRTE is not used anymore."
>
> Right, and I'm still not sure I agree with that reasoning. We need to
> build the kernel interface to be generic, not tailored for a specific
> userspace. I don't really feel comfortable having something that can't
> be disabled via a similar path to it being enabled. For instance, what
> about a dynamic debug interface where we want to enable tracing and see
> each interrupt injected into the guest. At that point we'd want to
> disabled posted interrupts and direct KVM injection and route via QEMU.
> Thanks,
>
> Alex

Okay, I will think about this.

Thanks,
Feng
>
> > > > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > > > index a269a42..7d98650 100644
> > > > --- a/include/uapi/linux/kvm.h
> > > > +++ b/include/uapi/linux/kvm.h
> > > > @@ -949,6 +949,7 @@ struct kvm_device_attr {
> > > > #define KVM_DEV_VFIO_DEVICE 2
> > > > #define KVM_DEV_VFIO_DEVICE_FORWARD_IRQ 1
> > > > #define KVM_DEV_VFIO_DEVICE_UNFORWARD_IRQ 2
> > > > +#define KVM_DEV_VFIO_DEVICE_POSTING_IRQ 3
> > > Maybe we should align our naming verb vs verbing here?
> > > Best Regards
> > > Eric
> >
> > No problem, I will align my patch in the next version. Thanks!
> >
> > Thanks,
> > Feng
> >
> > > >
> > > > enum kvm_device_type {
> > > > KVM_DEV_TYPE_FSL_MPIC_20 = 1,
> > > > @@ -973,6 +974,15 @@ struct kvm_arch_forwarded_irq {
> > > > __u32 gsi; /* gsi, ie. virtual IRQ number */
> > > > };
> > > >
> > > > +struct kvm_vfio_dev_irq {
> > > > + __u32 argsz;
> > > > + __u32 fd; /* file descriptor of the VFIO device */
> > > > + __u32 index; /* VFIO device IRQ index */
> > > > + __u32 start;
> > > > + __u32 count;
> > > > + __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@xxxxxxxxxxxxxxx
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>