diff mbox

[RFC,v1,1/2] vfio: Add new interrupt group for VFIO

Message ID 1416474316-32159-2-git-send-email-feng.wu@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Wu, Feng Nov. 20, 2014, 9:05 a.m. UTC
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.

Signed-off-by: Feng Wu <feng.wu@intel.com>
---
 Documentation/virtual/kvm/devices/vfio.txt |    8 ++++++++
 include/uapi/linux/kvm.h                   |   14 ++++++++++++++
 2 files changed, 22 insertions(+), 0 deletions(-)

Comments

Alex Williamson Nov. 20, 2014, 3:53 p.m. UTC | #1
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?

[1] https://lkml.org/lkml/2014/8/25/258

> Signed-off-by: Feng Wu <feng.wu@intel.com>
> ---
>  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.

> +	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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Auger Nov. 20, 2014, 4:03 p.m. UTC | #2
On 11/20/2014 04:53 PM, Alex Williamson wrote:
> 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?

Hi Alex, Feng

I share your point of view about the KVM_DEV_VFIO_DEVICE group. For the
attribute (renamed KVM_DEV_VFIO_DEVICE_FORWARD_IRQ in RFC v2) the issue
is I specify the kvm_device_attr.addr points to a kvm_arch_forwarded_irq
struct. Feng needs another struct - kvm_posted_intr -. An alternative is
to merge both structs if it makes sense.

Best Regards

Eric
> 
> [1] https://lkml.org/lkml/2014/8/25/258
> 
>> Signed-off-by: Feng Wu <feng.wu@intel.com>
>> ---
>>  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.
> 
>> +	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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alex Williamson Nov. 21, 2014, 7:35 p.m. UTC | #3
On Fri, 2014-11-21 at 06:06 +0000, Wu, Feng wrote:
> 
> > -----Original Message-----
> > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > Sent: Thursday, November 20, 2014 11:54 PM
> > To: Wu, Feng
> > Cc: pbonzini@redhat.com; kvm@vger.kernel.org; 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@intel.com>
> > > ---
> > >  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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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;
+	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,