diff mbox

[v4,08/16] KVM: kvm-vfio: User API for IRQ forwarding

Message ID 1434019912-15423-9-git-send-email-feng.wu@intel.com
State New
Headers show

Commit Message

Wu, Feng June 11, 2015, 10:51 a.m. UTC
From: Eric Auger <eric.auger@linaro.org>

This patch adds and documents a new KVM_DEV_VFIO_DEVICE group
and 2 device attributes: KVM_DEV_VFIO_DEVICE_FORWARD_IRQ,
KVM_DEV_VFIO_DEVICE_UNFORWARD_IRQ. The purpose is to be able
to set a VFIO device IRQ as forwarded or not forwarded.
the command takes as argument a handle to a new struct named
kvm_vfio_dev_irq.

Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
 Documentation/virtual/kvm/devices/vfio.txt | 34 ++++++++++++++++++++++++------
 include/uapi/linux/kvm.h                   | 12 +++++++++++
 2 files changed, 40 insertions(+), 6 deletions(-)

Comments

Auger Eric June 11, 2015, 1:37 p.m. UTC | #1
Hi Feng,
On 06/11/2015 12:51 PM, Feng Wu wrote:
> From: Eric Auger <eric.auger@linaro.org>
> 
> This patch adds and documents a new KVM_DEV_VFIO_DEVICE group
> and 2 device attributes: KVM_DEV_VFIO_DEVICE_FORWARD_IRQ,
> KVM_DEV_VFIO_DEVICE_UNFORWARD_IRQ. The purpose is to be able
> to set a VFIO device IRQ as forwarded or not forwarded.
> the command takes as argument a handle to a new struct named
> kvm_vfio_dev_irq.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> ---
>  Documentation/virtual/kvm/devices/vfio.txt | 34 ++++++++++++++++++++++++------
>  include/uapi/linux/kvm.h                   | 12 +++++++++++
>  2 files changed, 40 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/virtual/kvm/devices/vfio.txt b/Documentation/virtual/kvm/devices/vfio.txt
> index ef51740..6186e6d 100644
> --- a/Documentation/virtual/kvm/devices/vfio.txt
> +++ b/Documentation/virtual/kvm/devices/vfio.txt
> @@ -4,15 +4,20 @@ VFIO virtual device
>  Device types supported:
>    KVM_DEV_TYPE_VFIO
>  
> -Only one VFIO instance may be created per VM.  The created device
> -tracks VFIO groups in use by the VM and features of those groups
> -important to the correctness and acceleration of the VM.  As groups
> -are enabled and disabled for use by the VM, KVM should be updated
> -about their presence.  When registered with KVM, a reference to the
> -VFIO-group is held by KVM.
> +Only one VFIO instance may be created per VM.
> +
> +The created device tracks VFIO groups in use by the VM and features
> +of those groups important to the correctness and acceleration of
> +the VM.  As groups are enabled and disabled for use by the VM, KVM
> +should be updated about their presence.  When registered with KVM,
> +a reference to the VFIO-group is held by KVM.
> +
> +The device also enables to control some IRQ settings of VFIO devices:
> +forwarding/posting.
>  
>  Groups:
>    KVM_DEV_VFIO_GROUP
> +  KVM_DEV_VFIO_DEVICE
>  
>  KVM_DEV_VFIO_GROUP attributes:
>    KVM_DEV_VFIO_GROUP_ADD: Add a VFIO group to VFIO-KVM device tracking
> @@ -20,3 +25,20 @@ 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_DEVICE attributes:
> +  KVM_DEV_VFIO_DEVICE_FORWARD_IRQ: set a VFIO device IRQ as forwarded
> +  KVM_DEV_VFIO_DEVICE_UNFORWARD_IRQ: set a VFIO device IRQ as not forwarded
> +
> +For each, kvm_device_attr.addr points to a kvm_vfio_dev_irq struct.
> +
> +When forwarded, a physical IRQ is completed by the guest and not by the
> +host. This requires HW support in the interrupt controller.
> +
> +Forwarding can only be set when the corresponding VFIO IRQ is not masked
> +(would it be through VFIO_DEVICE_SET_IRQS command or as a consequence of this
> +IRQ being currently handled) or active at interrupt controller level.
> +In such a situation, -EAGAIN is returned. It is advised to to set the
> +forwarding before the VFIO signaling is set up, this avoids trial and errors.
> +
> +Unforwarding can happen at any time.
Unfortunately the above text is out of context of your series. If it
simplifies things take the ownership of that patch file and remove my
description of KVM_DEV_VFIO_DEVICE_FORWARD_IRQ.
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 4b60056..798f3e4 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -999,6 +999,9 @@ struct kvm_device_attr {
>  #define  KVM_DEV_VFIO_GROUP			1
>  #define   KVM_DEV_VFIO_GROUP_ADD			1
>  #define   KVM_DEV_VFIO_GROUP_DEL			2
> +#define  KVM_DEV_VFIO_DEVICE			2
> +#define   KVM_DEV_VFIO_DEVICE_FORWARD_IRQ			1
> +#define   KVM_DEV_VFIO_DEVICE_UNFORWARD_IRQ			2
same here
Best Regards

Eric
>  
>  enum kvm_device_type {
>  	KVM_DEV_TYPE_FSL_MPIC_20	= 1,
> @@ -1018,6 +1021,15 @@ enum kvm_device_type {
>  	KVM_DEV_TYPE_MAX,
>  };
>  
> +struct kvm_vfio_dev_irq {
> +	__u32	argsz;		/* structure length */
> +	__u32	fd;		/* file descriptor of the VFIO device */
> +	__u32	index;		/* VFIO device IRQ index */
> +	__u32	start;		/* start of subindex range */
> +	__u32	count;		/* size of subindex range */
> +	__u32	gsi[];		/* gsi, ie. virtual IRQ number */
> +};
> +
>  /*
>   * ioctls for VM fds
>   */
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Auger Eric June 15, 2015, 4:17 p.m. UTC | #2
Hi Alex, all,
On 06/12/2015 09:03 PM, Alex Williamson wrote:
> On Fri, 2015-06-12 at 21:48 +0300, Avi Kivity wrote:
>> On 06/12/2015 06:41 PM, Alex Williamson wrote:
>>> On Fri, 2015-06-12 at 00:23 +0000, Wu, Feng wrote:
>>>>> -----Original Message-----
>>>>> From: Avi Kivity [mailto:avi.kivity@gmail.com]
>>>>> Sent: Friday, June 12, 2015 3:59 AM
>>>>> To: Wu, Feng; kvm@vger.kernel.org; linux-kernel@vger.kernel.org
>>>>> Cc: pbonzini@redhat.com; mtosatti@redhat.com;
>>>>> alex.williamson@redhat.com; eric.auger@linaro.org
>>>>> Subject: Re: [v4 08/16] KVM: kvm-vfio: User API for IRQ forwarding
>>>>>
>>>>> On 06/11/2015 01:51 PM, Feng Wu wrote:
>>>>>> From: Eric Auger <eric.auger@linaro.org>
>>>>>>
>>>>>> This patch adds and documents a new KVM_DEV_VFIO_DEVICE group
>>>>>> and 2 device attributes: KVM_DEV_VFIO_DEVICE_FORWARD_IRQ,
>>>>>> KVM_DEV_VFIO_DEVICE_UNFORWARD_IRQ. The purpose is to be able
>>>>>> to set a VFIO device IRQ as forwarded or not forwarded.
>>>>>> the command takes as argument a handle to a new struct named
>>>>>> kvm_vfio_dev_irq.
>>>>> Is there no way to do this automatically?  After all, vfio knows that a
>>>>> device interrupt is forwarded to some eventfd, and kvm knows that some
>>>>> eventfd is forwarded to a guest interrupt.  If they compare notes
>>>>> through a central registry, they can figure out that the interrupt needs
>>>>> to be forwarded.
>>>> Oh, just like Eric mentioned in his reply, this description is out of context of
>>>> this series, I will remove them in the next version.
>>>
>>> I suspect Avi's question was more general.  While forward/unforward is
>>> out of context for this series, it's very similar in nature to
>>> enabling/disabling posted interrupts.  So I think the question remains
>>> whether we really need userspace to participate in creating this
>>> shortcut or if kvm and vfio can some how orchestrate figuring it out
>>> automatically.
>>>
>>> Personally I don't know how we could do it automatically.  We've always
>>> relied on userspace to independently setup vfio and kvm such that
>>> neither have any idea that the other is there and update each side
>>> independently when anything changes.  So it seems consistent to continue
>>> that here.  It doesn't seem like there's much to gain performance-wise
>>> either, updates should be a relatively rare event I'd expect.
>>>
>>> There's really no metadata associated with an eventfd, so "comparing
>>> notes" automatically might imply some central registration entity.  That
>>> immediately sounds like a much more complex solution, but maybe Avi has
>>> some ideas to manage it.  Thanks,
>>>
>>
>> The idea is to have a central registry maintained by a posted interrupts 
>> manager.  Both vfio and kvm pass the filp (along with extra information) 
>> to the posted interrupts manager, which, when it detects a filp match, 
>> tells each of them what to do.
>>
>> The advantages are:
>> - old userspace gains the optimization without change
>> - a userspace API is more expensive to maintain than internal kernel 
>> interfaces (CVEs, documentation, maintaining backwards compatibility)
>> - if you can do it without a new interface, this indicates that all the 
>> information in the new interface is redundant.  That means you have to 
>> check it for consistency with the existing information, so it's extra 
>> work (likely, it's exactly what the posted interrupt manager would be 
>> doing anyway).
> 
> Yep, those all sound like good things and I believe that's similar in
> design to the way we had originally discussed this interaction at
> LPC/KVM Forum several years ago.  I'd be in favor of that approach.

I guess this discussion also is relevant wrt "[RFC v6 00/16] KVM-VFIO
IRQ forward control" series? Or is that "central registry maintained by
a posted interrupts manager" something more specific to x86?

Thank you in advance

Best Regards

Eric

> Thanks,
> 
> Alex
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Auger Eric June 24, 2015, 4:25 p.m. UTC | #3
Hi Joerg,

On 06/24/2015 05:50 PM, Joerg Roedel wrote:
> On Mon, Jun 15, 2015 at 06:17:03PM +0200, Eric Auger wrote:
>> I guess this discussion also is relevant wrt "[RFC v6 00/16] KVM-VFIO
>> IRQ forward control" series? Or is that "central registry maintained by
>> a posted interrupts manager" something more specific to x86?
> 
> From what I understood so far, the feature you implemented for ARM is a
> bit different from the ones that get introduced to x86.
> 
> Can you please share some details on how the ARM version works? I am
> interested in how the GICv2 is configured for IRQ forwarding. The
> question is whether the forwarding information needs to be updated from
> KVM and what information about the IRQ KVM needs for this.

The principle is that when you inject a virtual IRQ to a guest, you
program a register in the GIC, known as a list register. There you put
both the virtual IRQ you want to inject but also the physical IRQ it is
linked with (HWbit mode set = forwarding set). When the guest completes
the virtual IRQ the GIC HW automatically deactivates the physical IRQ
found in the list register. In that mode the physical IRQ deactivation
is under the ownership of the guest (actually automatically done by the HW).

If HWbit mode is *not* set (forwarding not set), you do not specify the
HW IRQ in the list register. The host deactivates the physical IRQ &
masks it before triggering the virtual IRQ. Only the virtual IRQ ID is
programmed in the list register. When the guest completes the virtual
IRQ, a physical maintenance IRQ is triggered. The hyp mode is entered
and eventually the host unmasks the IRQ.

Some illustrations can be found in
http://www.linux-kvm.org/images/a/a8/01x04-ARMdevice.pdf

Hope it helps

Eric
> 
> 
> 	Joerg
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
diff mbox

Patch

diff --git a/Documentation/virtual/kvm/devices/vfio.txt b/Documentation/virtual/kvm/devices/vfio.txt
index ef51740..6186e6d 100644
--- a/Documentation/virtual/kvm/devices/vfio.txt
+++ b/Documentation/virtual/kvm/devices/vfio.txt
@@ -4,15 +4,20 @@  VFIO virtual device
 Device types supported:
   KVM_DEV_TYPE_VFIO
 
-Only one VFIO instance may be created per VM.  The created device
-tracks VFIO groups in use by the VM and features of those groups
-important to the correctness and acceleration of the VM.  As groups
-are enabled and disabled for use by the VM, KVM should be updated
-about their presence.  When registered with KVM, a reference to the
-VFIO-group is held by KVM.
+Only one VFIO instance may be created per VM.
+
+The created device tracks VFIO groups in use by the VM and features
+of those groups important to the correctness and acceleration of
+the VM.  As groups are enabled and disabled for use by the VM, KVM
+should be updated about their presence.  When registered with KVM,
+a reference to the VFIO-group is held by KVM.
+
+The device also enables to control some IRQ settings of VFIO devices:
+forwarding/posting.
 
 Groups:
   KVM_DEV_VFIO_GROUP
+  KVM_DEV_VFIO_DEVICE
 
 KVM_DEV_VFIO_GROUP attributes:
   KVM_DEV_VFIO_GROUP_ADD: Add a VFIO group to VFIO-KVM device tracking
@@ -20,3 +25,20 @@  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_DEVICE attributes:
+  KVM_DEV_VFIO_DEVICE_FORWARD_IRQ: set a VFIO device IRQ as forwarded
+  KVM_DEV_VFIO_DEVICE_UNFORWARD_IRQ: set a VFIO device IRQ as not forwarded
+
+For each, kvm_device_attr.addr points to a kvm_vfio_dev_irq struct.
+
+When forwarded, a physical IRQ is completed by the guest and not by the
+host. This requires HW support in the interrupt controller.
+
+Forwarding can only be set when the corresponding VFIO IRQ is not masked
+(would it be through VFIO_DEVICE_SET_IRQS command or as a consequence of this
+IRQ being currently handled) or active at interrupt controller level.
+In such a situation, -EAGAIN is returned. It is advised to to set the
+forwarding before the VFIO signaling is set up, this avoids trial and errors.
+
+Unforwarding can happen at any time.
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 4b60056..798f3e4 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -999,6 +999,9 @@  struct kvm_device_attr {
 #define  KVM_DEV_VFIO_GROUP			1
 #define   KVM_DEV_VFIO_GROUP_ADD			1
 #define   KVM_DEV_VFIO_GROUP_DEL			2
+#define  KVM_DEV_VFIO_DEVICE			2
+#define   KVM_DEV_VFIO_DEVICE_FORWARD_IRQ			1
+#define   KVM_DEV_VFIO_DEVICE_UNFORWARD_IRQ			2
 
 enum kvm_device_type {
 	KVM_DEV_TYPE_FSL_MPIC_20	= 1,
@@ -1018,6 +1021,15 @@  enum kvm_device_type {
 	KVM_DEV_TYPE_MAX,
 };
 
+struct kvm_vfio_dev_irq {
+	__u32	argsz;		/* structure length */
+	__u32	fd;		/* file descriptor of the VFIO device */
+	__u32	index;		/* VFIO device IRQ index */
+	__u32	start;		/* start of subindex range */
+	__u32	count;		/* size of subindex range */
+	__u32	gsi[];		/* gsi, ie. virtual IRQ number */
+};
+
 /*
  * ioctls for VM fds
  */