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.au...@linaro.org] > >>> Sent: Monday, December 01, 2014 6:10 PM > >>> To: Alex Williamson > >>> Cc: Wu, Feng; pbonz...@redhat.com; g...@kernel.org; kvm@vger.kernel.org > >>> 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...@intel.com> > >>>>>> --- > >>>>>> 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, Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
