On 12/02/2014 05:02 PM, Alex Williamson wrote: > On Tue, 2014-12-02 at 08:52 +0100, Eric Auger wrote: >> On 12/02/2014 05:48 AM, Alex Williamson wrote: >>> On Tue, 2014-12-02 at 02:08 +0000, Wu, Feng wrote: >>>> >>>>> -----Original Message----- >>>>> From: Eric Auger [mailto:eric.auger@xxxxxxxxxx] >>>>> Sent: Monday, December 01, 2014 6:10 PM >>>>> To: Alex Williamson >>>>> Cc: Wu, Feng; pbonzini@xxxxxxxxxx; gleb@xxxxxxxxxx; kvm@xxxxxxxxxxxxxxx >>>>> 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 <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..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. >> ok for that name >>> >>>>>>>> + __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, >> sorry meant fd = -1 (typo) as in the original VFIO API. Personally I >> think I would prefer keeping the UNFORWARD but I will follow your guidance. > > If fd=-1 we can't uniquely identify the device and irq when there are > multiple devices. I'm not necessarily opposed to an UNFORWARD, just > show how it works better or more cleanly in the code. Thanks, OK thanks. Eric > > Alex > -- 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
