On Tue, 2014-12-02 at 02:08 +0000, Wu, Feng wrote:
>
> > -----Original Message-----
> > From: Eric Auger [mailto:[email protected]]
> > Sent: Monday, December 01, 2014 6:10 PM
> > To: Alex Williamson
> > Cc: Wu, Feng; [email protected]; [email protected]; [email protected]
> > Subject: Re: [RFC PATCH v2 1/2] KVM: kvm-vfio: User API for VT-d
> > Posted-Interrupts
> >
> > On 11/25/2014 05:10 PM, Alex Williamson wrote:
> > > On Tue, 2014-11-25 at 16:01 +0100, Eric Auger wrote:
> > >> On 11/25/2014 01:23 PM, 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 <[email protected]>
> > >>> ---
> > >>> 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..39dee86 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
> > >>> + the IRQ to guests.
> > >>> +For this attribute, kvm_device_attr.addr points to a kvm_posted_intr
> > 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.
> > >>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > >>> index a269a42..e5f86ad 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
> > >>>
> > >>> 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 */
> > >>> };
> > >>>
> > Hi Feng, Alex,
> > I am currently reworking my code to use something closer to this struct.
> > Would you agree with following changes?
> > >>> +struct kvm_posted_intr {
> > kvm_posted_irq
>
> Hi Alex,
>
> Do you mean changing the structure name to "kvm_posted_irq"? I am okay
> If you think this name is also suitable for ARM forwarded irq. Or we can find
> a more common name, such as "struct kvm_accel_irq", what is your opinion,
> Alex?
I'd think something like struct kvm_vfio_dev_irq describes it fairly
well.
> > >>> + __u32 argsz;
> > >>> + __u32 fd; /* file descriptor of the VFIO device */
> > >>> + __u32 index; /* VFIO device IRQ index */
> > >>> + __u32 start;
> > >>> + __u32 count;
> > >>> + int virq[0]; /* gsi, ie. virtual IRQ number */
> > __u32 gsi[];
>
> I think this change is okay to me. If Alex also agree, I will follow this in
> the
> next post.
>
> > >>> +};
> > >> Hi Feng,
> > >>
> > >> This struct could be used by arm code too. If Alex agrees I could use
> > >> that one instead. We just need to find a common sensible name
> > >
> > > Yep, the interface might as well support batch setup. The vfio code
> > > uses -1 for teardown if we want to avoid FORWARD vs UNFORWARD we could
> > > let the data in the structure define which operation to do.
> >
> > In case we remove the unforward and use fd=1 to tear down, the virq=gsi
> > must uniquely identify the struct. For ARM I think this is true, we
> > cannot have several physical IRQ forwarded to the same GSI. I don't know
> > about posted irqs or other archs.
It makes more sense to me that fd is the real vfio_device fd that we
uniquely match to existing forwarded/posted IRQs by
{vfio_device,index,start[count]}. If gsi was then signed, passing -1
could be used to teardown that forward/posting. Passing fd=1, ie.
stdout, is pretty non-intuitive to me. Thanks,
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
