On Fri, 2014-11-21 at 06:06 +0000, Wu, Feng wrote: > > > -----Original Message----- > > From: Alex Williamson [mailto:alex.williamson@xxxxxxxxxx] > > Sent: Thursday, November 20, 2014 11:54 PM > > To: Wu, Feng > > Cc: pbonzini@xxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; eric.auger > > Subject: Re: [RFC PATCH v1 1/2] vfio: Add new interrupt group for VFIO > > > > On Thu, 2014-11-20 at 17:05 +0800, Feng Wu wrote: > > > Add new group KVM_DEV_VFIO_INTERRUPT and command > > > KVM_DEV_VFIO_DEVIE_POSTING_IRQ related to it. > > > > > > This is used for VT-d Posted-Interrupts setup. > > > > Eric proposed an interface for ARM forwarded interrupts[1] using group > > KVM_DEV_VFIO_DEVICE with attributes > > KVM_DEV_VFIO_DEVICE_ASSIGN_IRQ and > > KVM_DEV_VFIO_DEVICE_DEASSIGN_IRQ. Why are we proposing yet another > > group and attributes here? Why can't we re-use the ones Eric proposes? > > > > I totally agree that I can reuse Eric's proposals. However, as Eric mentioned in > his reply, I am using another data structure. So how about adding my own > attribute, say, KVM_DEV_VFIO_DEVICE_POSTING_IRQ in group KVM_DEV_VFIO_DEVICE. Right, Eric's latest proposal (sorry I picked the v1 links by mistake in my previous reply) includes: KVM_DEV_VFIO_DEVICE attributes: KVM_DEV_VFIO_DEVICE_FORWARD_IRQ KVM_DEV_VFIO_DEVICE_UNFORWARD_IRQ So I think we'd want to add something similar for posted interrupts, some sort of "start" and "stop" attribute. At the QEMU level we'll want to abstract both of these as opportunistic IRQ accelerators, but at the KVM-VFIO level we probably need to make them distinct using a separate set of attributes. Who knows, maybe one day ARM will support posted interrupts and Intel will support forwarding... I expect the calls from the KVM-VFIO device into VFIO at the kernel level to be largely the same between the different attributes though. Thanks, Alex > > [1] https://lkml.org/lkml/2014/8/25/258 > > > > > Signed-off-by: Feng Wu <feng.wu@xxxxxxxxx> > > > --- > > > Documentation/virtual/kvm/devices/vfio.txt | 8 ++++++++ > > > include/uapi/linux/kvm.h | 14 ++++++++++++++ > > > 2 files changed, 22 insertions(+), 0 deletions(-) > > > > > > diff --git a/Documentation/virtual/kvm/devices/vfio.txt > > b/Documentation/virtual/kvm/devices/vfio.txt > > > index ef51740..bd99176 100644 > > > --- a/Documentation/virtual/kvm/devices/vfio.txt > > > +++ b/Documentation/virtual/kvm/devices/vfio.txt > > > @@ -13,6 +13,7 @@ VFIO-group is held by KVM. > > > > > > Groups: > > > KVM_DEV_VFIO_GROUP > > > + KVM_DEV_VFIO_INTERRUPT > > > > > > KVM_DEV_VFIO_GROUP attributes: > > > KVM_DEV_VFIO_GROUP_ADD: Add a VFIO group to VFIO-KVM device > > tracking > > > @@ -20,3 +21,10 @@ KVM_DEV_VFIO_GROUP attributes: > > > > > > For each, kvm_device_attr.addr points to an int32_t file descriptor > > > for the VFIO group. > > > + > > > +KVM_DEV_VFIO_INTERRUPT attributes: > > > + KVM_DEV_VFIO_INTERRUPT_POSTING_IRQ: Set up the interrupt > > configuration for > > > +VT-d Posted-Interrrupts > > > + > > > +For each, kvm_device_attr.addr points to struct kvm_posted_intr, which > > > +include the needed information for VT-d Posted-Interrupts setup. > > > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > > > index 6076882..5544fcc 100644 > > > --- a/include/uapi/linux/kvm.h > > > +++ b/include/uapi/linux/kvm.h > > > @@ -943,9 +943,23 @@ struct kvm_device_attr { > > > __u64 addr; /* userspace address of attr data */ > > > }; > > > > > > +struct virq_info { > > > + __u32 index; /* index of the msi/msix entry */ > > > + int virq; /* virq of the interrupt */ > > > +}; > > > + > > > +struct kvm_posted_intr { > > > + __u32 fd; /* file descriptor of the VFIO device */ > > > + __u32 count; > > > + bool msix; > > > > Note that MSI-X (as opposed to MSI) is a PCI concept. Being a VFIO > > interface this should operate at VFIO IRQ index and sub-index. > > Yes, I will use VFIO stuff instead. > > Thanks, > Feng > > > > > > + struct virq_info virq_info[0]; > > > +}; > > > + > > > #define KVM_DEV_VFIO_GROUP 1 > > > #define KVM_DEV_VFIO_GROUP_ADD 1 > > > #define KVM_DEV_VFIO_GROUP_DEL 2 > > > +#define KVM_DEV_VFIO_INTERRUPT 2 > > > +#define KVM_DEV_VFIO_INTERRUPT_POSTING_IRQ 1 > > > > > > enum kvm_device_type { > > > KVM_DEV_TYPE_FSL_MPIC_20 = 1, > > > > > -- 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