[PATCH 0/5] Threaded MSI interrupt for VFIO PCI device

[PATCH 0/5] Threaded MSI interrupt for VFIO PCI device

[Yunhong Jiang]

When assigning a VFIO device to a KVM guest with low latency requirement, it  
is better to handle the interrupt in the hard interrupt context, to reduce 
the context switch to/from the IRQ thread.

Based on discussion on https://lkml.org/lkml/2015/10/26/764, the VFIO msi 
interrupt is changed to use request_threaded_irq(). The primary interrupt 
handler tries to set the guest interrupt atomically. If it fails to achieve 
it, a threaded interrupt handler will be invoked.

The irq_bypass manager is extended for this purpose. The KVM eventfd will 
provide a irqbypass consumer to handle the interrupt at hard interrupt 
context. The producer will invoke the consumer's handler then.

Yunhong Jiang (5):
  Extract the irqfd_wakeup_pollin/irqfd_wakeup_pollup
  Support runtime irq_bypass consumer
  Support threaded interrupt handling on VFIO
  Add the irq handling consumer
  Expose x86 kvm_arch_set_irq_inatomic()

 arch/x86/kvm/Kconfig              |   1 +
 drivers/vfio/pci/vfio_pci_intrs.c |  39 ++++++++++--
 include/linux/irqbypass.h         |   8 +++
 include/linux/kvm_host.h          |  19 +++++-
 include/linux/kvm_irqfd.h         |   1 +
 virt/kvm/Kconfig                  |   3 +
 virt/kvm/eventfd.c                | 131 ++++++++++++++++++++++++++------------
 virt/lib/irqbypass.c              |  82 ++++++++++++++++++------
 8 files changed, 214 insertions(+), 70 deletions(-)

-- 
1.8.3.1

[PATCH 1/5] KVM: Extract the irqfd_wakeup_pollin/irqfd_wakeup_pollup

[Yunhong Jiang]

Separate the irqfd_wakeup_pollin/irqfd_wakeup_pollup from the
irqfd_wakeup, so that we can reuse the logic for MSI fastpath injection.

Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
---
 virt/kvm/eventfd.c | 86 ++++++++++++++++++++++++++++++++----------------------
 1 file changed, 51 insertions(+), 35 deletions(-)

diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index 46dbc0a7dfc1..c31d43b762db 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -180,6 +180,53 @@ int __attribute__((weak)) kvm_arch_set_irq_inatomic(
 	return -EWOULDBLOCK;
 }
 
+static int
+irqfd_wakeup_pollin(struct kvm_kernel_irqfd *irqfd)
+{
+	struct kvm *kvm = irqfd->kvm;
+	struct kvm_kernel_irq_routing_entry irq;
+	unsigned seq;
+	int idx, ret;
+
+	idx = srcu_read_lock(&kvm->irq_srcu);
+	do {
+		seq = read_seqcount_begin(&irqfd->irq_entry_sc);
+		irq = irqfd->irq_entry;
+	} while (read_seqcount_retry(&irqfd->irq_entry_sc, seq));
+	/* An event has been signaled, inject an interrupt */
+	ret = kvm_arch_set_irq_inatomic(&irq, kvm,
+				KVM_USERSPACE_IRQ_SOURCE_ID, 1,
+				false);
+	srcu_read_unlock(&kvm->irq_srcu, idx);
+
+	return ret;
+}
+
+static int
+irqfd_wakeup_pollup(struct kvm_kernel_irqfd *irqfd)
+{
+	struct kvm *kvm = irqfd->kvm;
+	unsigned long flags;
+
+	spin_lock_irqsave(&kvm->irqfds.lock, flags);
+
+	/*
+	 * We must check if someone deactivated the irqfd before
+	 * we could acquire the irqfds.lock since the item is
+	 * deactivated from the KVM side before it is unhooked from
+	 * the wait-queue.  If it is already deactivated, we can
+	 * simply return knowing the other side will cleanup for us.
+	 * We cannot race against the irqfd going away since the
+	 * other side is required to acquire wqh->lock, which we hold
+	 */
+	if (irqfd_is_active(irqfd))
+		irqfd_deactivate(irqfd);
+
+	spin_unlock_irqrestore(&kvm->irqfds.lock, flags);
+
+	return 0;
+}
+
 /*
  * Called with wqh->lock held and interrupts disabled
  */
@@ -189,45 +236,14 @@ irqfd_wakeup(wait_queue_t *wait, unsigned mode, int sync, void *key)
 	struct kvm_kernel_irqfd *irqfd =
 		container_of(wait, struct kvm_kernel_irqfd, wait);
 	unsigned long flags = (unsigned long)key;
-	struct kvm_kernel_irq_routing_entry irq;
-	struct kvm *kvm = irqfd->kvm;
-	unsigned seq;
-	int idx;
 
-	if (flags & POLLIN) {
-		idx = srcu_read_lock(&kvm->irq_srcu);
-		do {
-			seq = read_seqcount_begin(&irqfd->irq_entry_sc);
-			irq = irqfd->irq_entry;
-		} while (read_seqcount_retry(&irqfd->irq_entry_sc, seq));
-		/* An event has been signaled, inject an interrupt */
-		if (kvm_arch_set_irq_inatomic(&irq, kvm,
-					      KVM_USERSPACE_IRQ_SOURCE_ID, 1,
-					      false) == -EWOULDBLOCK)
+	if (flags & POLLIN)
+		if (irqfd_wakeup_pollin(irqfd) == -EWOULDBLOCK)
 			schedule_work(&irqfd->inject);
-		srcu_read_unlock(&kvm->irq_srcu, idx);
-	}
 
-	if (flags & POLLHUP) {
+	if (flags & POLLHUP)
 		/* The eventfd is closing, detach from KVM */
-		unsigned long flags;
-
-		spin_lock_irqsave(&kvm->irqfds.lock, flags);
-
-		/*
-		 * We must check if someone deactivated the irqfd before
-		 * we could acquire the irqfds.lock since the item is
-		 * deactivated from the KVM side before it is unhooked from
-		 * the wait-queue.  If it is already deactivated, we can
-		 * simply return knowing the other side will cleanup for us.
-		 * We cannot race against the irqfd going away since the
-		 * other side is required to acquire wqh->lock, which we hold
-		 */
-		if (irqfd_is_active(irqfd))
-			irqfd_deactivate(irqfd);
-
-		spin_unlock_irqrestore(&kvm->irqfds.lock, flags);
-	}
+		irqfd_wakeup_pollup(irqfd);
 
 	return 0;
 }
-- 
1.8.3.1

[PATCH 2/5] VIRT: Support runtime irq_bypass consumer

[Yunhong Jiang]

Extend the irq_bypass manager to support runtime consumers. A runtime
irq_bypass consumer can handle interrupt when an interrupt triggered. A
runtime consumer has it's handle_irq() function set and passing a
irq_context for the irq handling.

A producer keep a link for the runtime consumers, so that it can invoke
each consumer's handle_irq() when irq invoked.

Currently the irq_bypass manager has several code path assuming there is
only one consumer/producer pair for each token. For example, when
register the producer, it exits the loop after finding one match
consumer.  This is updated to support both static consumer (like for
Posted Interrupt consumer) and runtime consumer.

Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
---
 include/linux/irqbypass.h |  8 +++++
 virt/lib/irqbypass.c      | 82 +++++++++++++++++++++++++++++++++++------------
 2 files changed, 69 insertions(+), 21 deletions(-)

diff --git a/include/linux/irqbypass.h b/include/linux/irqbypass.h
index 1551b5b2f4c2..d5bec0c7be3a 100644
--- a/include/linux/irqbypass.h
+++ b/include/linux/irqbypass.h
@@ -12,6 +12,7 @@
 #define IRQBYPASS_H
 
 #include <linux/list.h>
+#include <linux/srcu.h>
 
 struct irq_bypass_consumer;
 
@@ -47,6 +48,9 @@ struct irq_bypass_consumer;
  */
 struct irq_bypass_producer {
 	struct list_head node;
+	/* Update side is synchronized by the lock on irqbypass.c */
+	struct srcu_struct srcu;
+	struct list_head consumers;
 	void *token;
 	int irq;
 	int (*add_consumer)(struct irq_bypass_producer *,
@@ -61,6 +65,7 @@ struct irq_bypass_producer {
  * struct irq_bypass_consumer - IRQ bypass consumer definition
  * @node: IRQ bypass manager private list management
  * @token: opaque token to match between producer and consumer
+ * @sibling: consumers with same token list management
  * @add_producer: Connect the IRQ consumer to an IRQ producer
  * @del_producer: Disconnect the IRQ consumer from an IRQ producer
  * @stop: Perform any quiesce operations necessary prior to add/del (optional)
@@ -73,6 +78,7 @@ struct irq_bypass_producer {
  */
 struct irq_bypass_consumer {
 	struct list_head node;
+	struct list_head sibling;
 	void *token;
 	int (*add_producer)(struct irq_bypass_consumer *,
 			    struct irq_bypass_producer *);
@@ -80,6 +86,8 @@ struct irq_bypass_consumer {
 			     struct irq_bypass_producer *);
 	void (*stop)(struct irq_bypass_consumer *);
 	void (*start)(struct irq_bypass_consumer *);
+	int (*handle_irq)(void *arg);
+	void *irq_context;
 };
 
 int irq_bypass_register_producer(struct irq_bypass_producer *);
diff --git a/virt/lib/irqbypass.c b/virt/lib/irqbypass.c
index 09a03b5a21ff..43ef9e2c77dc 100644
--- a/virt/lib/irqbypass.c
+++ b/virt/lib/irqbypass.c
@@ -21,6 +21,7 @@
 #include <linux/list.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/rculist.h>
 
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("IRQ bypass manager utility module");
@@ -49,11 +50,8 @@ static int __connect(struct irq_bypass_producer *prod,
 			prod->del_consumer(prod, cons);
 	}
 
-	if (cons->start)
-		cons->start(cons);
-	if (prod->start)
-		prod->start(prod);
-
+	if (!ret && cons->handle_irq)
+		list_add_rcu(&cons->sibling, &prod->consumers);
 	return ret;
 }
 
@@ -71,6 +69,11 @@ static void __disconnect(struct irq_bypass_producer *prod,
 	if (prod->del_consumer)
 		prod->del_consumer(prod, cons);
 
+	if (cons->handle_irq) {
+		list_del_rcu(&cons->sibling);
+		synchronize_srcu(&prod->srcu);
+	}
+
 	if (cons->start)
 		cons->start(cons);
 	if (prod->start)
@@ -87,7 +90,8 @@ static void __disconnect(struct irq_bypass_producer *prod,
 int irq_bypass_register_producer(struct irq_bypass_producer *producer)
 {
 	struct irq_bypass_producer *tmp;
-	struct irq_bypass_consumer *consumer;
+	struct list_head *node, *next, siblings = LIST_HEAD_INIT(siblings);
+	int ret;
 
 	might_sleep();
 
@@ -96,6 +100,9 @@ int irq_bypass_register_producer(struct irq_bypass_producer *producer)
 
 	mutex_lock(&lock);
 
+	INIT_LIST_HEAD(&producer->consumers);
+	init_srcu_struct(&producer->srcu);
+
 	list_for_each_entry(tmp, &producers, node) {
 		if (tmp->token == producer->token) {
 			mutex_unlock(&lock);
@@ -104,23 +111,48 @@ int irq_bypass_register_producer(struct irq_bypass_producer *producer)
 		}
 	}
 
-	list_for_each_entry(consumer, &consumers, node) {
+	list_for_each_safe(node, next, &consumers) {
+		struct irq_bypass_consumer *consumer = container_of(
+			node, struct irq_bypass_consumer, node);
+
 		if (consumer->token == producer->token) {
-			int ret = __connect(producer, consumer);
-			if (ret) {
-				mutex_unlock(&lock);
-				module_put(THIS_MODULE);
-				return ret;
-			}
-			break;
+			ret = __connect(producer, consumer);
+			if (ret)
+				goto error;
+			/* Keep the connected consumers temply */
+			list_del(&consumer->node);
+			list_add_rcu(&consumer->node, &siblings);
 		}
 	}
 
+	list_for_each_safe(node, next, &siblings) {
+		struct irq_bypass_consumer *consumer = container_of(
+			node, struct irq_bypass_consumer, node);
+
+		list_del(&consumer->node);
+		list_add(&consumer->node, &consumers);
+		if (consumer->start)
+			consumer->start(consumer);
+	}
+
+	if (producer->start)
+		producer->start(producer);
 	list_add(&producer->node, &producers);
 
 	mutex_unlock(&lock);
-
 	return 0;
+
+error:
+	list_for_each_safe(node, next, &siblings) {
+		struct irq_bypass_consumer *consumer = container_of(
+			node, struct irq_bypass_consumer, node);
+
+		list_del(&consumer->node);
+		list_add(&consumer->node, &consumers);
+	}
+	mutex_unlock(&lock);
+	module_put(THIS_MODULE);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(irq_bypass_register_producer);
 
@@ -133,7 +165,7 @@ EXPORT_SYMBOL_GPL(irq_bypass_register_producer);
  */
 void irq_bypass_unregister_producer(struct irq_bypass_producer *producer)
 {
-	struct irq_bypass_producer *tmp;
+	struct irq_bypass_producer *tmp, *n;
 	struct irq_bypass_consumer *consumer;
 
 	might_sleep();
@@ -143,7 +175,7 @@ void irq_bypass_unregister_producer(struct irq_bypass_producer *producer)
 
 	mutex_lock(&lock);
 
-	list_for_each_entry(tmp, &producers, node) {
+	list_for_each_entry_safe(tmp, n, &producers, node) {
 		if (tmp->token != producer->token)
 			continue;
 
@@ -159,6 +191,7 @@ void irq_bypass_unregister_producer(struct irq_bypass_producer *producer)
 		break;
 	}
 
+	cleanup_srcu_struct(&producer->srcu);
 	mutex_unlock(&lock);
 
 	module_put(THIS_MODULE);
@@ -180,6 +213,9 @@ int irq_bypass_register_consumer(struct irq_bypass_consumer *consumer)
 	if (!consumer->add_producer || !consumer->del_producer)
 		return -EINVAL;
 
+	if (consumer->handle_irq && !consumer->irq_context)
+		return -EINVAL;
+
 	might_sleep();
 
 	if (!try_module_get(THIS_MODULE))
@@ -188,7 +224,7 @@ int irq_bypass_register_consumer(struct irq_bypass_consumer *consumer)
 	mutex_lock(&lock);
 
 	list_for_each_entry(tmp, &consumers, node) {
-		if (tmp->token == consumer->token) {
+		if (tmp == consumer) {
 			mutex_unlock(&lock);
 			module_put(THIS_MODULE);
 			return -EBUSY;
@@ -203,6 +239,10 @@ int irq_bypass_register_consumer(struct irq_bypass_consumer *consumer)
 				module_put(THIS_MODULE);
 				return ret;
 			}
+			if (consumer->start)
+				consumer->start(consumer);
+			if (producer->start)
+				producer->start(producer);
 			break;
 		}
 	}
@@ -224,7 +264,7 @@ EXPORT_SYMBOL_GPL(irq_bypass_register_consumer);
  */
 void irq_bypass_unregister_consumer(struct irq_bypass_consumer *consumer)
 {
-	struct irq_bypass_consumer *tmp;
+	struct irq_bypass_consumer *tmp, *n;
 	struct irq_bypass_producer *producer;
 
 	might_sleep();
@@ -234,8 +274,8 @@ void irq_bypass_unregister_consumer(struct irq_bypass_consumer *consumer)
 
 	mutex_lock(&lock);
 
-	list_for_each_entry(tmp, &consumers, node) {
-		if (tmp->token != consumer->token)
+	list_for_each_entry_safe(tmp, n, &consumers, node) {
+		if (tmp != consumer)
 			continue;
 
 		list_for_each_entry(producer, &producers, node) {
-- 
1.8.3.1

[PATCH 3/5] VFIO: Support threaded interrupt handling on VFIO

[Yunhong Jiang]

For VFIO device with MSI interrupt type, it's possible to handle the
interrupt on hard interrupt context without invoking the interrupt
thread. Handling the interrupt on hard interrupt context reduce the
interrupt latency.

Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
---
 drivers/vfio/pci/vfio_pci_intrs.c | 39 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
index 3b3ba15558b7..108d335c5656 100644
--- a/drivers/vfio/pci/vfio_pci_intrs.c
+++ b/drivers/vfio/pci/vfio_pci_intrs.c
@@ -236,12 +236,35 @@ static void vfio_intx_disable(struct vfio_pci_device *vdev)
 	kfree(vdev->ctx);
 }
 
+static irqreturn_t vfio_msihandler(int irq, void *arg)
+{
+	struct vfio_pci_irq_ctx *ctx = arg;
+	struct irq_bypass_producer *producer = &ctx->producer;
+	struct irq_bypass_consumer *consumer;
+	int ret = IRQ_HANDLED, idx;
+
+	idx = srcu_read_lock(&producer->srcu);
+
+	list_for_each_entry_rcu(consumer, &producer->consumers, sibling) {
+		/*
+		 * Invoke the thread handler if any consumer would block, but
+		 * finish all consumes.
+		 */
+		if (consumer->handle_irq(consumer->irq_context) == -EWOULDBLOCK)
+			ret = IRQ_WAKE_THREAD;
+		continue;
+	}
+
+	srcu_read_unlock(&producer->srcu, idx);
+	return ret;
+}
+
 /*
  * MSI/MSI-X
  */
-static irqreturn_t vfio_msihandler(int irq, void *arg)
+static irqreturn_t vfio_msihandler_threaded(int irq, void *arg)
 {
-	struct eventfd_ctx *trigger = arg;
+	struct eventfd_ctx *trigger = ((struct vfio_pci_irq_ctx *)arg)->trigger;
 
 	eventfd_signal(trigger, 1);
 	return IRQ_HANDLED;
@@ -318,7 +341,7 @@ static int vfio_msi_set_vector_signal(struct vfio_pci_device *vdev,
 		return -EINVAL;
 
 	if (vdev->ctx[vector].trigger) {
-		free_irq(irq, vdev->ctx[vector].trigger);
+		free_irq(irq, &vdev->ctx[vector]);
 		irq_bypass_unregister_producer(&vdev->ctx[vector].producer);
 		kfree(vdev->ctx[vector].name);
 		eventfd_ctx_put(vdev->ctx[vector].trigger);
@@ -353,8 +376,14 @@ static int vfio_msi_set_vector_signal(struct vfio_pci_device *vdev,
 		pci_write_msi_msg(irq, &msg);
 	}
 
-	ret = request_irq(irq, vfio_msihandler, 0,
-			  vdev->ctx[vector].name, trigger);
+	/*
+	 * Currently the primary handler for the thread_irq will be invoked on
+	 * a thread, the IRQF_ONESHOT is a hack for it.
+	 */
+	ret = request_threaded_irq(irq, vfio_msihandler,
+				   vfio_msihandler_threaded,
+				   IRQF_ONESHOT, vdev->ctx[vector].name,
+				   &vdev->ctx[vector]);
 	if (ret) {
 		kfree(vdev->ctx[vector].name);
 		eventfd_ctx_put(trigger);
-- 
1.8.3.1

[PATCH 4/5] KVM: Add the irq handling consumer

[Yunhong Jiang]

Add an irq_bypass consumer to the KVM eventfd, so that when a MSI interrupt
happens and triggerred from VFIO, it can be handled fast.

Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
---
 include/linux/kvm_irqfd.h |  1 +
 virt/kvm/eventfd.c        | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/include/linux/kvm_irqfd.h b/include/linux/kvm_irqfd.h
index 0c1de05098c8..5573d53ccebb 100644
--- a/include/linux/kvm_irqfd.h
+++ b/include/linux/kvm_irqfd.h
@@ -65,6 +65,7 @@ struct kvm_kernel_irqfd {
 	poll_table pt;
 	struct work_struct shutdown;
 	struct irq_bypass_consumer consumer;
+	struct irq_bypass_consumer fastpath;
 	struct irq_bypass_producer *producer;
 };
 
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index c31d43b762db..b20a2d1bbf73 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -144,6 +144,8 @@ irqfd_shutdown(struct work_struct *work)
 #ifdef CONFIG_HAVE_KVM_IRQ_BYPASS
 	irq_bypass_unregister_consumer(&irqfd->consumer);
 #endif
+	if (irqfd->fastpath.token)
+		irq_bypass_unregister_consumer(&irqfd->fastpath);
 	eventfd_ctx_put(irqfd->eventfd);
 	kfree(irqfd);
 }
@@ -203,6 +205,14 @@ irqfd_wakeup_pollin(struct kvm_kernel_irqfd *irqfd)
 }
 
 static int
+kvm_fastpath_irq(void *arg)
+{
+	struct kvm_kernel_irqfd *irqfd = arg;
+
+	return irqfd_wakeup_pollin(irqfd);
+}
+
+static int
 irqfd_wakeup_pollup(struct kvm_kernel_irqfd *irqfd)
 {
 	struct kvm *kvm = irqfd->kvm;
@@ -296,6 +306,34 @@ int  __attribute__((weak)) kvm_arch_update_irqfd_routing(
 }
 #endif
 
+static int kvm_fastpath_stub(struct irq_bypass_consumer *stub,
+			     struct irq_bypass_producer *stub1)
+{
+	return 0;
+}
+
+static void kvm_fastpath_stub1(struct irq_bypass_consumer *stub,
+			     struct irq_bypass_producer *stub1)
+{
+}
+
+static int setup_fastpath_consumer(struct kvm_kernel_irqfd *irqfd)
+{
+	int ret;
+
+	irqfd->fastpath.token = (void *)irqfd->eventfd;
+	irqfd->fastpath.add_producer = kvm_fastpath_stub;
+	irqfd->fastpath.del_producer = kvm_fastpath_stub1;
+	irqfd->fastpath.handle_irq = kvm_fastpath_irq;
+	irqfd->fastpath.irq_context = irqfd;
+	ret = irq_bypass_register_consumer(&irqfd->fastpath);
+
+	if (ret)
+		/* A special tag to indicate consumer not working */
+		irqfd->fastpath.token = (void *)0;
+	return ret;
+}
+
 static int
 kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)
 {
@@ -435,6 +473,10 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)
 				irqfd->consumer.token, ret);
 #endif
 
+	if (setup_fastpath_consumer(irqfd))
+		pr_info("irq bypass fastpath consumer (toke %p) registration fails: %d\n",
+				irqfd->eventfd, ret);
+
 	return 0;
 
 fail:
-- 
1.8.3.1

[PATCH 5/5] KVM: Expose x86 kvm_arch_set_irq_inatomic()

[Yunhong Jiang]

The x86 support setting irq in atomic, expose it to vfio driver.

Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
---
 arch/x86/kvm/Kconfig     |  1 +
 include/linux/kvm_host.h | 19 ++++++++++++++++---
 virt/kvm/Kconfig         |  3 +++
 virt/kvm/eventfd.c       |  9 ---------
 4 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 639a6e34500c..642e8b905c96 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -30,6 +30,7 @@ config KVM
 	select HAVE_KVM_IRQFD
 	select IRQ_BYPASS_MANAGER
 	select HAVE_KVM_IRQ_BYPASS
+	select KVM_SET_IRQ_INATOMIC
 	select HAVE_KVM_IRQ_ROUTING
 	select HAVE_KVM_EVENTFD
 	select KVM_APIC_ARCHITECTURE
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 590c46e672df..a6e237275928 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -852,9 +852,6 @@ int kvm_set_irq(struct kvm *kvm, int irq_source_id, u32 irq, int level,
 		bool line_status);
 int kvm_set_msi(struct kvm_kernel_irq_routing_entry *irq_entry, struct kvm *kvm,
 		int irq_source_id, int level, bool line_status);
-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);
 bool kvm_irq_has_notifier(struct kvm *kvm, unsigned irqchip, unsigned pin);
 void kvm_notify_acked_gsi(struct kvm *kvm, int gsi);
 void kvm_notify_acked_irq(struct kvm *kvm, unsigned irqchip, unsigned pin);
@@ -1207,4 +1204,20 @@ int kvm_arch_update_irqfd_routing(struct kvm *kvm, unsigned int host_irq,
 				  uint32_t guest_irq, bool set);
 #endif /* CONFIG_HAVE_KVM_IRQ_BYPASS */
 
+#ifndef CONFIG_KVM_SET_IRQ_INATOMIC
+int __attribute__((weak)) kvm_arch_set_irq_inatomic(
+				struct kvm_kernel_irq_routing_entry *irq,
+				struct kvm *kvm, int irq_source_id,
+				int level,
+				bool line_status)
+{
+	return -EWOULDBLOCK;
+}
+#else
+extern 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);
+#endif
+
 #endif
diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
index 7a79b6853583..7c99dd4724a4 100644
--- a/virt/kvm/Kconfig
+++ b/virt/kvm/Kconfig
@@ -50,3 +50,6 @@ config KVM_COMPAT
 
 config HAVE_KVM_IRQ_BYPASS
        bool
+
+config KVM_SET_IRQ_INATOMIC
+	bool
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index b20a2d1bbf73..405c26742380 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -173,15 +173,6 @@ irqfd_deactivate(struct kvm_kernel_irqfd *irqfd)
 	queue_work(irqfd_cleanup_wq, &irqfd->shutdown);
 }
 
-int __attribute__((weak)) kvm_arch_set_irq_inatomic(
-				struct kvm_kernel_irq_routing_entry *irq,
-				struct kvm *kvm, int irq_source_id,
-				int level,
-				bool line_status)
-{
-	return -EWOULDBLOCK;
-}
-
 static int
 irqfd_wakeup_pollin(struct kvm_kernel_irqfd *irqfd)
 {
-- 
1.8.3.1

Re: [PATCH 0/5] Threaded MSI interrupt for VFIO PCI device

[Alex Williamson]

On Thu, 2015-12-03 at 10:22 -0800, Yunhong Jiang wrote:
> When assigning a VFIO device to a KVM guest with low latency requirement, it  
> is better to handle the interrupt in the hard interrupt context, to reduce 
> the context switch to/from the IRQ thread.
> 
> Based on discussion on https://lkml.org/lkml/2015/10/26/764, the VFIO msi 
> interrupt is changed to use request_threaded_irq(). The primary interrupt 
> handler tries to set the guest interrupt atomically. If it fails to achieve 
> it, a threaded interrupt handler will be invoked.
> 
> The irq_bypass manager is extended for this purpose. The KVM eventfd will 
> provide a irqbypass consumer to handle the interrupt at hard interrupt 
> context. The producer will invoke the consumer's handler then.

Do you have any performance data?  Thanks,

Alex

Re: [PATCH 0/5] Threaded MSI interrupt for VFIO PCI device

[Yunhong Jiang]

On Thu, Dec 03, 2015 at 11:55:53AM -0700, Alex Williamson wrote:
> On Thu, 2015-12-03 at 10:22 -0800, Yunhong Jiang wrote:
> > When assigning a VFIO device to a KVM guest with low latency requirement, it  
> > is better to handle the interrupt in the hard interrupt context, to reduce 
> > the context switch to/from the IRQ thread.
> > 
> > Based on discussion on https://lkml.org/lkml/2015/10/26/764, the VFIO msi 
> > interrupt is changed to use request_threaded_irq(). The primary interrupt 
> > handler tries to set the guest interrupt atomically. If it fails to achieve 
> > it, a threaded interrupt handler will be invoked.
> > 
> > The irq_bypass manager is extended for this purpose. The KVM eventfd will 
> > provide a irqbypass consumer to handle the interrupt at hard interrupt 
> > context. The producer will invoke the consumer's handler then.
> 
> Do you have any performance data?  Thanks,

Sorry, I should include the data on the commit messages.

The test:
Launch a VM with a FPGA device, which triggers an interrpt every 1ms. The VM 
is launched on a isolated CPU with NONHZ_FULL enabled.
Two data are collected.  On the guest, a special program will check the 
latency from the time the interrupt been triggered to the time the interrupt 
received by guest IRS. On the host, I use the perf to collect the perf data.

The performance Data with the patch:

Host perf data:
[root@otcnfv02 test-tools]# perf kvm stat record -C 34 sleep 10
[root@otcnfv02 test-tools]# perf kvm stat report
Analyze events for all VMs, all VCPUs:
	     VM-EXIT    Samples  Samples%     Time%    Min Time    Max Time
Avg time
  EXTERNAL_INTERRUPT       9997    98.55%    99.31%      1.73us     17.07us
2.09us ( +-   0.21% )
   PAUSE_INSTRUCTION        127     1.25%     0.51%      0.69us      1.20us
0.84us ( +-   1.34% )
	   MSR_WRITE         20     0.20%     0.18%      1.62us      3.21us
1.93us ( +-   3.95% )

Guest data:
[nfv@rt-test-1 ~]$ ./run-int-test.sh
Latency is Min: 3.74us Max: 20.08us Avg: 4.49us
No of interrupts = 74995

The performance data without the patch:
Host perf data:
[root@otcnfv02 test-tools]# perf kvm stat record -C 34 sleep 10
[root@otcnfv02 test-tools]# perf kvm stat report
Analyze events for all VMs, all VCPUs:
	     VM-EXIT    Samples  Samples%     Time%    Min Time    Max Time
Avg time
   PAUSE_INSTRUCTION     141136    87.74%    50.39%      0.69us      8.51us      
0.77us ( +-   0.07% )
  EXTERNAL_INTERRUPT      19701    12.25%    49.59%      2.31us     15.93us      
5.46us ( +-   0.12% )
	   MSR_WRITE         24     0.01%     0.02%      1.51us      2.22us      
1.91us ( +-   1.91% )

Notice:
The EXTERNAL_INTERRUPT VMExit is different w/ the patch (9997) and w/o the 
patch (19701). It is because with threaded IRQ, the NOHZ_FULL it not working 
because two threads on the pCPU, thus we have both the FPGA device interrupt 
and the tick timer interrupt. After calculation, the average time for the 
FPGA device interrupt is 4.72 us.

Guest data:
[nfv@rt-test-1 ~]$ ./run-int-test.sh
Latency is Min: 6.70us Max: 50.38us Avg: 7.44us
No of interrupts = 42639

Thanks
--jyh

> 
> Alex
> 

Re: [PATCH 4/5] KVM: Add the irq handling consumer

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1183 bytes --]

[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
Hi Yunhong,

[auto build test ERROR on kvm/linux-next]
[also build test ERROR on v4.4-rc3 next-20151202]

url:    https://github.com/0day-ci/linux/commits/Yunhong-Jiang/KVM-Extract-the-irqfd_wakeup_pollin-irqfd_wakeup_pollup/20151204-024034
base:   https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next
config: arm64-allmodconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   arch/arm64/kvm/built-in.o: In function `kvm_irqfd_assign':
>> :(.text+0x10a70): undefined reference to `irq_bypass_register_consumer'
   arch/arm64/kvm/built-in.o: In function `irqfd_shutdown':
>> :(.text+0x10cd4): undefined reference to `irq_bypass_unregister_consumer'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 46627 bytes --]

Re: [PATCH 0/5] Threaded MSI interrupt for VFIO PCI device

[Paolo Bonzini]

Alex,

can you take a look at the extension to the irq bypass interface in
patch 2?  I'm not sure I understand what is the case where you have
multiple consumers for the same token.

Paolo

On 03/12/2015 19:22, Yunhong Jiang wrote:
> When assigning a VFIO device to a KVM guest with low latency requirement, it  
> is better to handle the interrupt in the hard interrupt context, to reduce 
> the context switch to/from the IRQ thread.
> 
> Based on discussion on https://lkml.org/lkml/2015/10/26/764, the VFIO msi 
> interrupt is changed to use request_threaded_irq(). The primary interrupt 
> handler tries to set the guest interrupt atomically. If it fails to achieve 
> it, a threaded interrupt handler will be invoked.
> 
> The irq_bypass manager is extended for this purpose. The KVM eventfd will 
> provide a irqbypass consumer to handle the interrupt at hard interrupt 
> context. The producer will invoke the consumer's handler then.
> 
> Yunhong Jiang (5):
>   Extract the irqfd_wakeup_pollin/irqfd_wakeup_pollup
>   Support runtime irq_bypass consumer
>   Support threaded interrupt handling on VFIO
>   Add the irq handling consumer
>   Expose x86 kvm_arch_set_irq_inatomic()
> 
>  arch/x86/kvm/Kconfig              |   1 +
>  drivers/vfio/pci/vfio_pci_intrs.c |  39 ++++++++++--
>  include/linux/irqbypass.h         |   8 +++
>  include/linux/kvm_host.h          |  19 +++++-
>  include/linux/kvm_irqfd.h         |   1 +
>  virt/kvm/Kconfig                  |   3 +
>  virt/kvm/eventfd.c                | 131 ++++++++++++++++++++++++++------------
>  virt/lib/irqbypass.c              |  82 ++++++++++++++++++------
>  8 files changed, 214 insertions(+), 70 deletions(-)
> 

Re: [PATCH 0/5] Threaded MSI interrupt for VFIO PCI device

[Alex Williamson]

On Wed, 2015-12-16 at 18:56 +0100, Paolo Bonzini wrote:
> Alex,
> 
> can you take a look at the extension to the irq bypass interface in
> patch 2?  I'm not sure I understand what is the case where you have
> multiple consumers for the same token.

The consumers would be, for instance, Intel PI + the threaded handler
added in this series.  These run independently, the PI bypass simply
makes the interrupt disappear from the host when it catches it, but if
the vCPU isn't running in the right place at the time of the interrupt,
it gets delivered to the host, in which case the secondary consumer
implementing handle_irq() provides a lower latency injection than the
eventfd path.  If PI isn't supported, only this latter consumer is
registered.

On the surface it seems like a reasonable solution, though having
multiple consumers implementing handle_irq() seems problematic.  Do we
get multiple injections if we call them all?  Should we have some way
to prioritize one handler versus another?  Perhaps KVM should have a
single unified consumer that can provide that sort of logic, though we
still need the srcu code added here to protect against registration and
irq_handler() races.  Thanks,

Alex

> On 03/12/2015 19:22, Yunhong Jiang wrote:
> > When assigning a VFIO device to a KVM guest with low latency
> > requirement, it  
> > is better to handle the interrupt in the hard interrupt context, to
> > reduce 
> > the context switch to/from the IRQ thread.
> > 
> > Based on discussion on https://lkml.org/lkml/2015/10/26/764, the
> > VFIO msi 
> > interrupt is changed to use request_threaded_irq(). The primary
> > interrupt 
> > handler tries to set the guest interrupt atomically. If it fails to
> > achieve 
> > it, a threaded interrupt handler will be invoked.
> > 
> > The irq_bypass manager is extended for this purpose. The KVM
> > eventfd will 
> > provide a irqbypass consumer to handle the interrupt at hard
> > interrupt 
> > context. The producer will invoke the consumer's handler then.
> > 
> > Yunhong Jiang (5):
> >   Extract the irqfd_wakeup_pollin/irqfd_wakeup_pollup
> >   Support runtime irq_bypass consumer
> >   Support threaded interrupt handling on VFIO
> >   Add the irq handling consumer
> >   Expose x86 kvm_arch_set_irq_inatomic()
> > 
> >  arch/x86/kvm/Kconfig              |   1 +
> >  drivers/vfio/pci/vfio_pci_intrs.c |  39 ++++++++++--
> >  include/linux/irqbypass.h         |   8 +++
> >  include/linux/kvm_host.h          |  19 +++++-
> >  include/linux/kvm_irqfd.h         |   1 +
> >  virt/kvm/Kconfig                  |   3 +
> >  virt/kvm/eventfd.c                | 131
> > ++++++++++++++++++++++++++------------
> >  virt/lib/irqbypass.c              |  82 ++++++++++++++++++------
> >  8 files changed, 214 insertions(+), 70 deletions(-)
> > 

Re: [PATCH 2/5] VIRT: Support runtime irq_bypass consumer

[Alex Williamson]

On Thu, 2015-12-03 at 10:22 -0800, Yunhong Jiang wrote:
> Extend the irq_bypass manager to support runtime consumers. A runtime
> irq_bypass consumer can handle interrupt when an interrupt triggered. A
> runtime consumer has it's handle_irq() function set and passing a
> irq_context for the irq handling.
> 
> A producer keep a link for the runtime consumers, so that it can invoke
> each consumer's handle_irq() when irq invoked.
> 
> Currently the irq_bypass manager has several code path assuming there is
> only one consumer/producer pair for each token. For example, when
> register the producer, it exits the loop after finding one match
> consumer.  This is updated to support both static consumer (like for
> Posted Interrupt consumer) and runtime consumer.
> 
> Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> ---
>  include/linux/irqbypass.h |  8 +++++
>  virt/lib/irqbypass.c      | 82 +++++++++++++++++++++++++++++++++++------------
>  2 files changed, 69 insertions(+), 21 deletions(-)
> 
> diff --git a/include/linux/irqbypass.h b/include/linux/irqbypass.h
> index 1551b5b2f4c2..d5bec0c7be3a 100644
> --- a/include/linux/irqbypass.h
> +++ b/include/linux/irqbypass.h
> @@ -12,6 +12,7 @@
>  #define IRQBYPASS_H
>  
>  #include 
> +#include 
>  
>  struct irq_bypass_consumer;
>  
> @@ -47,6 +48,9 @@ struct irq_bypass_consumer;
>   */
>  struct irq_bypass_producer {
>  	struct list_head node;
> +	/* Update side is synchronized by the lock on irqbypass.c */
> +	struct srcu_struct srcu;
> +	struct list_head consumers;
>  	void *token;
>  	int irq;
>  	int (*add_consumer)(struct irq_bypass_producer *,

Documentation?

> @@ -61,6 +65,7 @@ struct irq_bypass_producer {
>   * struct irq_bypass_consumer - IRQ bypass consumer definition
>   * @node: IRQ bypass manager private list management
>   * @token: opaque token to match between producer and consumer
> + * @sibling: consumers with same token list management
>   * @add_producer: Connect the IRQ consumer to an IRQ producer
>   * @del_producer: Disconnect the IRQ consumer from an IRQ producer
>   * @stop: Perform any quiesce operations necessary prior to add/del (optional)

What about @handle_irq and @irq_context?

> @@ -73,6 +78,7 @@ struct irq_bypass_producer {
>   */
>  struct irq_bypass_consumer {
>  	struct list_head node;
> +	struct list_head sibling;
>  	void *token;
>  	int (*add_producer)(struct irq_bypass_consumer *,
>  			    struct irq_bypass_producer *);
> @@ -80,6 +86,8 @@ struct irq_bypass_consumer {
>  			     struct irq_bypass_producer *);
>  	void (*stop)(struct irq_bypass_consumer *);
>  	void (*start)(struct irq_bypass_consumer *);
> +	int (*handle_irq)(void *arg);

If we called this with a pointer to the consumer, like the other
functions, the consumer could embed arg (irq_context) into their own
structure, or in this case, do a container_of and avoid storing the
irqfd pointer entirely.

> +	void *irq_context;
>  };
>  
>  int irq_bypass_register_producer(struct irq_bypass_producer *);

Re: [PATCH 3/5] VFIO: Support threaded interrupt handling on VFIO

[Alex Williamson]

On Thu, 2015-12-03 at 10:22 -0800, Yunhong Jiang wrote:
> For VFIO device with MSI interrupt type, it's possible to handle the
> interrupt on hard interrupt context without invoking the interrupt
> thread. Handling the interrupt on hard interrupt context reduce the
> interrupt latency.
> 
> Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> ---
>  drivers/vfio/pci/vfio_pci_intrs.c | 39 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 34 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
> index 3b3ba15558b7..108d335c5656 100644
> --- a/drivers/vfio/pci/vfio_pci_intrs.c
> +++ b/drivers/vfio/pci/vfio_pci_intrs.c
> @@ -236,12 +236,35 @@ static void vfio_intx_disable(struct vfio_pci_device *vdev)
>  	kfree(vdev->ctx);
>  }
>  
> +static irqreturn_t vfio_msihandler(int irq, void *arg)
> +{
> +	struct vfio_pci_irq_ctx *ctx = arg;
> +	struct irq_bypass_producer *producer = &ctx->producer;
> +	struct irq_bypass_consumer *consumer;
> +	int ret = IRQ_HANDLED, idx;
> +
> +	idx = srcu_read_lock(&producer->srcu);
> +
> +	list_for_each_entry_rcu(consumer, &producer->consumers, sibling) {
> +		/*
> +		 * Invoke the thread handler if any consumer would block, but
> +		 * finish all consumes.
> +		 */
> +		if (consumer->handle_irq(consumer->irq_context) == -EWOULDBLOCK)
> +			ret = IRQ_WAKE_THREAD;
> +		continue;
> +	}
> +
> +	srcu_read_unlock(&producer->srcu, idx);


There should be an irq bypass manager interface to abstract this.

Re: [PATCH 0/5] Threaded MSI interrupt for VFIO PCI device

[Paolo Bonzini]

On 16/12/2015 20:15, Alex Williamson wrote:
> The consumers would be, for instance, Intel PI + the threaded handler
> added in this series.  These run independently, the PI bypass simply
> makes the interrupt disappear from the host when it catches it, but if
> the vCPU isn't running in the right place at the time of the interrupt,
> it gets delivered to the host, in which case the secondary consumer
> implementing handle_irq() provides a lower latency injection than the
> eventfd path.  If PI isn't supported, only this latter consumer is
> registered.

I would implement the two in a single consumer, knowing that only one of
the two parts would effectively run.  But because of the possibility of
multiple consumers implementing handle_irq(), I am not sure if this is
feasible.

> On the surface it seems like a reasonable solution, though having
> multiple consumers implementing handle_irq() seems problematic.  Do we
> get multiple injections if we call them all?

Indeed.

> Should we have some way
> to prioritize one handler versus another?  Perhaps KVM should have a
> single unified consumer that can provide that sort of logic, though we
> still need the srcu code added here to protect against registration and
> irq_handler() races.  Thanks,

I'm happy to see that we have the same doubts. :)

Paolo

Re: [PATCH 0/5] Threaded MSI interrupt for VFIO PCI device

[Yunhong Jiang]

On Wed, Dec 16, 2015 at 12:15:23PM -0700, Alex Williamson wrote:
> On Wed, 2015-12-16 at 18:56 +0100, Paolo Bonzini wrote:
> > Alex,
> > 
> > can you take a look at the extension to the irq bypass interface in
> > patch 2?  I'm not sure I understand what is the case where you have
> > multiple consumers for the same token.
> 
> The consumers would be, for instance, Intel PI + the threaded handler
> added in this series.  These run independently, the PI bypass simply
> makes the interrupt disappear from the host when it catches it, but if
> the vCPU isn't running in the right place at the time of the interrupt,
> it gets delivered to the host, in which case the secondary consumer
> implementing handle_irq() provides a lower latency injection than the

Sorry for slow response.

If the PI is delivered to the host because guest is not running, I think it 
will not trigger the secondary consumer. The reason is, with PI, the 
interrupt will be delivered as the POSTED_INTR_VECTOR or 
POSTED_INTR_WAKEUP_VECTOR. So for the PI consumer will not be invoked on run 
time scenario.

> eventfd path.  If PI isn't supported, only this latter consumer is
> registered.
> 
> On the surface it seems like a reasonable solution, though having
> multiple consumers implementing handle_irq() seems problematic.  Do we

Yes, agree that has multiple consumers implementing handle_irq() seems not 
good. But I do think it can be helpful. A naive case is, a consumer can be 
created to log all the interrupt event, or to create a pipe for analysis.

> get multiple injections if we call them all?  Should we have some way

As discussed above, currently I think we have only one consumer to 
handle_irq(), so it should be ok? We can limit the framework to support only 
one consumer with handle_irq()?

> to prioritize one handler versus another?  Perhaps KVM should have a
> single unified consumer that can provide that sort of logic, though we
I'd think still different consumer for the PI and this fast_IRQ.

Thanks
--jyh

> still need the srcu code added here to protect against registration and
> irq_handler() races.  Thanks,
> 
> Alex
> 
> > On 03/12/2015 19:22, Yunhong Jiang wrote:
> > > When assigning a VFIO device to a KVM guest with low latency
> > > requirement, it  
> > > is better to handle the interrupt in the hard interrupt context, to
> > > reduce 
> > > the context switch to/from the IRQ thread.
> > > 
> > > Based on discussion on https://lkml.org/lkml/2015/10/26/764, the
> > > VFIO msi 
> > > interrupt is changed to use request_threaded_irq(). The primary
> > > interrupt 
> > > handler tries to set the guest interrupt atomically. If it fails to
> > > achieve 
> > > it, a threaded interrupt handler will be invoked.
> > > 
> > > The irq_bypass manager is extended for this purpose. The KVM
> > > eventfd will 
> > > provide a irqbypass consumer to handle the interrupt at hard
> > > interrupt 
> > > context. The producer will invoke the consumer's handler then.
> > > 
> > > Yunhong Jiang (5):
> > >   Extract the irqfd_wakeup_pollin/irqfd_wakeup_pollup
> > >   Support runtime irq_bypass consumer
> > >   Support threaded interrupt handling on VFIO
> > >   Add the irq handling consumer
> > >   Expose x86 kvm_arch_set_irq_inatomic()
> > > 
> > >  arch/x86/kvm/Kconfig              |   1 +
> > >  drivers/vfio/pci/vfio_pci_intrs.c |  39 ++++++++++--
> > >  include/linux/irqbypass.h         |   8 +++
> > >  include/linux/kvm_host.h          |  19 +++++-
> > >  include/linux/kvm_irqfd.h         |   1 +
> > >  virt/kvm/Kconfig                  |   3 +
> > >  virt/kvm/eventfd.c                | 131
> > > ++++++++++++++++++++++++++------------
> > >  virt/lib/irqbypass.c              |  82 ++++++++++++++++++------
> > >  8 files changed, 214 insertions(+), 70 deletions(-)
> > > 
> 

Re: [PATCH 0/5] Threaded MSI interrupt for VFIO PCI device

[Yunhong Jiang]

On Wed, Dec 16, 2015 at 10:55:12PM +0100, Paolo Bonzini wrote:
> 
> 
> On 16/12/2015 20:15, Alex Williamson wrote:
> > The consumers would be, for instance, Intel PI + the threaded handler
> > added in this series.  These run independently, the PI bypass simply
> > makes the interrupt disappear from the host when it catches it, but if
> > the vCPU isn't running in the right place at the time of the interrupt,
> > it gets delivered to the host, in which case the secondary consumer
> > implementing handle_irq() provides a lower latency injection than the
> > eventfd path.  If PI isn't supported, only this latter consumer is
> > registered.
> 
> I would implement the two in a single consumer, knowing that only one of
> the two parts would effectively run.  But because of the possibility of
> multiple consumers implementing handle_irq(), I am not sure if this is
> feasible.

So is it possible that we limit only one consumer with handle_irq(), as my 
previous response to Alex? We can extend it in future if we do need support 
multiple consumder implementing handle_irq()?

Thanks
--jyh

> 
> > On the surface it seems like a reasonable solution, though having
> > multiple consumers implementing handle_irq() seems problematic.  Do we
> > get multiple injections if we call them all?
> 
> Indeed.
> 
> > Should we have some way
> > to prioritize one handler versus another?  Perhaps KVM should have a
> > single unified consumer that can provide that sort of logic, though we
> > still need the srcu code added here to protect against registration and
> > irq_handler() races.  Thanks,
> 
> I'm happy to see that we have the same doubts. :)
> 
> Paolo
> --
> 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/