[RFC PATCH 0/2] kvm: introduce KVM_IRQ_ROUTING_EVENTFD

[RFC PATCH 0/2] kvm: introduce KVM_IRQ_ROUTING_EVENTFD

[Peter Xu]

This series tried to introduce a new IRQ_ROUTING type named "eventfd".

This work is totally RFC, and the idea is invoked when I was trying to
cope with a VT-d vIOMMU error handling case when guest setup incorrect
IRTE. Since this idea is quite interesting to me (which looks like a
reversed version of irqfd, since both of them are doing convertions of
virq <-> eventfd, just in different directions), and I see it
relatively safe to have (it touches little other part of the codes),
so here came the series.

I posted this series mainly wanted to know whether we like this, and
also I'm curious about whether this feature can be leveraged in other
use cases outside my "vIOMMU problem".

If we think this is something safe/good to have, I can continue to
work on RFC v2 to provide workable userspace test program (or QEMU
branch) that leverages this new feature. If not, any kind of NACK is
also welcomed. :-)

Please kindly review. Thanks,

Peter Xu (2):
  kvm: x86: add new gsi route type EVENTFD
  kvm: add new cap KVM_CAP_GSI_EVENTFD

 arch/x86/kvm/irq_comm.c  | 33 +++++++++++++++++++++++++++++++++
 arch/x86/kvm/x86.c       |  1 +
 include/linux/kvm_host.h |  7 +++++++
 include/uapi/linux/kvm.h |  7 +++++++
 virt/kvm/irqchip.c       |  6 ++++++
 5 files changed, 54 insertions(+)

-- 
2.7.4

[RFC PATCH 1/2] kvm: x86: add new gsi route type EVENTFD

[Peter Xu]

A new GSI routing type KVM_GSI_ROUTING_EVENTFD is introduced. This type
of irq routing does not trigger any guest event directly, instead, it
triggers a pre-configured eventfd handle, so that a gsi interrupt can be
forwarded to somewhere else, like, a userspace program (QEMU).

This idea was invoked when I was trying to solve an emulated VT-d issue
when guest kernel setup incorrect IRTE. When that happens, instead of
raising error immediately, what we should do is to keep the error, and
inject this error to vIOMMU when the specific interrupt is triggered.

However this is very hard to be achieved since for now vIOMMU is working
in userspace, while currently there is no simple way that kernel irq can
talk to a userspace program.

With this patch, we can easily provide such a way that when guest fault
irq is triggered, kernel can notify user program by signaling the
corresponding eventfd handle (which should have been configured before
hand by the userspace vIOMMU). Then userspace vIOMMU can process the
realtime irq error and report to guest properly.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 arch/x86/kvm/irq_comm.c  | 33 +++++++++++++++++++++++++++++++++
 include/linux/kvm_host.h |  7 +++++++
 include/uapi/linux/kvm.h |  6 ++++++
 virt/kvm/irqchip.c       |  6 ++++++
 4 files changed, 52 insertions(+)

diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index 6c01916..d37de0b 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -23,6 +23,7 @@
 #include <linux/kvm_host.h>
 #include <linux/slab.h>
 #include <linux/export.h>
+#include <linux/eventfd.h>
 #include <trace/events/kvm.h>
 
 #include <asm/msidef.h>
@@ -179,6 +180,16 @@ static int kvm_hv_set_sint(struct kvm_kernel_irq_routing_entry *e,
 	return kvm_hv_synic_set_irq(kvm, e->hv_sint.vcpu, e->hv_sint.sint);
 }
 
+static int kvm_set_gsi_eventfd(struct kvm_kernel_irq_routing_entry *e,
+			       struct kvm *kvm, int irq_source_id, int level,
+			       bool line_status)
+{
+	if (!level || eventfd_signal(e->eventfd.ctx, 1) != 1)
+		return -1;
+
+	return 1;
+}
+
 int kvm_arch_set_irq_inatomic(struct kvm_kernel_irq_routing_entry *e,
 			      struct kvm *kvm, int irq_source_id, int level,
 			      bool line_status)
@@ -292,6 +303,7 @@ int kvm_set_routing_entry(struct kvm *kvm,
 	int r = -EINVAL;
 	int delta;
 	unsigned max_pin;
+	struct eventfd_ctx *eventfd;
 
 	switch (ue->type) {
 	case KVM_IRQ_ROUTING_IRQCHIP:
@@ -332,6 +344,15 @@ int kvm_set_routing_entry(struct kvm *kvm,
 		e->hv_sint.vcpu = ue->u.hv_sint.vcpu;
 		e->hv_sint.sint = ue->u.hv_sint.sint;
 		break;
+	case KVM_IRQ_ROUTING_EVENTFD:
+		eventfd = eventfd_ctx_fdget(ue->u.eventfd.fd);
+		if (IS_ERR(eventfd)) {
+			r = PTR_ERR(eventfd);
+			goto out;
+		}
+		e->eventfd.ctx = eventfd;
+		e->set = kvm_set_gsi_eventfd;
+		break;
 	default:
 		goto out;
 	}
@@ -448,3 +469,15 @@ void kvm_arch_irq_routing_update(struct kvm *kvm)
 {
 	kvm_hv_irq_routing_update(kvm);
 }
+
+void free_irq_routing_entry(struct kvm_kernel_irq_routing_entry *e)
+{
+	switch (e->type) {
+	case KVM_IRQ_ROUTING_EVENTFD:
+		if (e->eventfd.ctx)
+			eventfd_ctx_put(e->eventfd.ctx);
+		break;
+	default:
+		break;
+	}
+}
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 1c5190d..0eee182 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -306,6 +306,10 @@ struct kvm_hv_sint {
 	u32 sint;
 };
 
+struct kvm_gsi_eventfd {
+	struct eventfd_ctx *ctx;
+};
+
 struct kvm_kernel_irq_routing_entry {
 	u32 gsi;
 	u32 type;
@@ -326,6 +330,7 @@ struct kvm_kernel_irq_routing_entry {
 		} msi;
 		struct kvm_s390_adapter_int adapter;
 		struct kvm_hv_sint hv_sint;
+		struct kvm_gsi_eventfd eventfd;
 	};
 	struct hlist_node link;
 };
@@ -340,6 +345,8 @@ struct kvm_irq_routing_table {
 	 */
 	struct hlist_head map[0];
 };
+
+void free_irq_routing_entry(struct kvm_kernel_irq_routing_entry *e);
 #endif
 
 #ifndef KVM_PRIVATE_MEM_SLOTS
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index cac48ed..efe530c 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -902,11 +902,16 @@ struct kvm_irq_routing_hv_sint {
 	__u32 sint;
 };
 
+struct kvm_irq_routing_eventfd {
+	__u32 fd;
+};
+
 /* gsi routing entry types */
 #define KVM_IRQ_ROUTING_IRQCHIP 1
 #define KVM_IRQ_ROUTING_MSI 2
 #define KVM_IRQ_ROUTING_S390_ADAPTER 3
 #define KVM_IRQ_ROUTING_HV_SINT 4
+#define KVM_IRQ_ROUTING_EVENTFD 5
 
 struct kvm_irq_routing_entry {
 	__u32 gsi;
@@ -918,6 +923,7 @@ struct kvm_irq_routing_entry {
 		struct kvm_irq_routing_msi msi;
 		struct kvm_irq_routing_s390_adapter adapter;
 		struct kvm_irq_routing_hv_sint hv_sint;
+		struct kvm_irq_routing_eventfd eventfd;
 		__u32 pad[8];
 	} u;
 };
diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
index 3bcc999..35612ce 100644
--- a/virt/kvm/irqchip.c
+++ b/virt/kvm/irqchip.c
@@ -109,6 +109,11 @@ int kvm_set_irq(struct kvm *kvm, int irq_source_id, u32 irq, int level,
 	return ret;
 }
 
+void __attribute__((weak))
+free_irq_routing_entry(struct kvm_kernel_irq_routing_entry *e)
+{
+}
+
 static void free_irq_routing_table(struct kvm_irq_routing_table *rt)
 {
 	int i;
@@ -121,6 +126,7 @@ static void free_irq_routing_table(struct kvm_irq_routing_table *rt)
 		struct hlist_node *n;
 
 		hlist_for_each_entry_safe(e, n, &rt->map[i], link) {
+			free_irq_routing_entry(e);
 			hlist_del(&e->link);
 			kfree(e);
 		}
-- 
2.7.4

Re: [RFC PATCH 1/2] kvm: x86: add new gsi route type EVENTFD

[Paolo Bonzini]

On 08/02/2017 08:58, Peter Xu wrote:
> This idea was invoked when I was trying to solve an emulated VT-d issue
> when guest kernel setup incorrect IRTE. When that happens, instead of
> raising error immediately, what we should do is to keep the error, and
> inject this error to vIOMMU when the specific interrupt is triggered.
> 
> However this is very hard to be achieved since for now vIOMMU is working
> in userspace, while currently there is no simple way that kernel irq can
> talk to a userspace program.
> 
> With this patch, we can easily provide such a way that when guest fault
> irq is triggered, kernel can notify user program by signaling the
> corresponding eventfd handle

I think I understand the scenario, but I don't understand why it needs
kernel intervention.  Why couldn't this be handled entirely in
userspace, without ever setting up a GSI route or irqfd in KVM?  In
other words, you're doing

 write(irqfd)         read(irq fault eventfd)
       |                       ^
       v                       |
      KVM -------> KVM_GSI_ROUTING_EVENTFD

but why is this needed as opposed to just

 write(irqfd) ------> read(irq fault eventfd)

?

Thanks,

Paolo

Re: [RFC PATCH 1/2] kvm: x86: add new gsi route type EVENTFD

[Peter Xu]

On Wed, Feb 08, 2017 at 09:26:10AM +0100, Paolo Bonzini wrote:
> 
> 
> On 08/02/2017 08:58, Peter Xu wrote:
> > This idea was invoked when I was trying to solve an emulated VT-d issue
> > when guest kernel setup incorrect IRTE. When that happens, instead of
> > raising error immediately, what we should do is to keep the error, and
> > inject this error to vIOMMU when the specific interrupt is triggered.
> > 
> > However this is very hard to be achieved since for now vIOMMU is working
> > in userspace, while currently there is no simple way that kernel irq can
> > talk to a userspace program.
> > 
> > With this patch, we can easily provide such a way that when guest fault
> > irq is triggered, kernel can notify user program by signaling the
> > corresponding eventfd handle
> 
> I think I understand the scenario, but I don't understand why it needs
> kernel intervention.  Why couldn't this be handled entirely in
> userspace, without ever setting up a GSI route or irqfd in KVM?  In
> other words, you're doing
> 
>  write(irqfd)         read(irq fault eventfd)
>        |                       ^
>        v                       |
>       KVM -------> KVM_GSI_ROUTING_EVENTFD
> 
> but why is this needed as opposed to just
> 
>  write(irqfd) ------> read(irq fault eventfd)
> 
> ?

Paolo,

Thanks for pointing out this issue. This is one of my concern as well,
and Jan has had the same comment before (sorry I forgot to cc Jan,
doing it now). I was trying to identify the risk for both solutions. I
posted this just trying to choose one way out of the two, and
currently this series is my clumsy choice.

I agree that logicall this can be done all in userspace. Now the
problem is that, I am afraid we can't do it easily, and lots of codes
may need to be touched for QEMU to achieve a whole userspace solution
- now we not only need irqfd and virq to setup a route, we need to
prepare a fault sink for each of the irqfd. Not to say that we have
lots of assumption in QEMU that virq and irqfd are treated seperately.
That'll of course expand the test scope as well, covering all the
devices using irqfd. Of course we can try to abstract that out into
something common, I am just afraid that'll be still a relatively big
change, and I am still uncertain about the "size" of it.

With this feature, everything will be in control totally in vIOMMU
side in QEMU. It'll be fairly straightforward and clean. But of course
I understand your concern since we should keep KVM as simple as
possible, even this is only tens of lines of changes (I wouldn't dare
to post a series for this for more than 100 LOCs :-).

And, even if we don't like this feature bit, I am still not sure
whether I should move on on QEMU side to do a possible big change only
to benefit error handling in VT-d vIOMMU. Any of your further comment
would be welcomed to help me settle this down.

(Or, maybe I should just put this problem aside for now. Comparing to
 solve this, I think fixing the IRTE bug in kernel might be easier :-)

Thanks,

-- peterx

[RFC PATCH 2/2] kvm: add new cap KVM_CAP_GSI_EVENTFD

[Peter Xu]

This is corresponding capability bit for the newly introduced
KVM_GSI_ROUTING_EVENTFD type. Currently it only works for x86.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 arch/x86/kvm/x86.c       | 1 +
 include/uapi/linux/kvm.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 51ccfe0..73428029 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2633,6 +2633,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_DISABLE_QUIRKS:
 	case KVM_CAP_SET_BOOT_CPU_ID:
  	case KVM_CAP_SPLIT_IRQCHIP:
+	case KVM_CAP_GSI_EVENTFD:
 #ifdef CONFIG_KVM_DEVICE_ASSIGNMENT
 	case KVM_CAP_ASSIGN_DEV_IRQ:
 	case KVM_CAP_PCI_2_3:
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index efe530c..7e7854d 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -871,6 +871,7 @@ struct kvm_ppc_smmu_info {
 #define KVM_CAP_S390_USER_INSTR0 130
 #define KVM_CAP_MSI_DEVID 131
 #define KVM_CAP_PPC_HTM 132
+#define KVM_CAP_GSI_EVENTFD 133
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
2.7.4