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. 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
