irqfd

This allows an eventfd to be registered as an irq source with a guest. Any signaling operation on the eventfd (via userspace or kernel) will inject the registered GSI at the next available window.

Signed-off-by Gregory Haskins <[email protected]>

---

 arch/x86/kvm/Makefile    |    2 -
 arch/x86/kvm/x86.c       |    1 
 include/linux/kvm.h      |    7 ++
 include/linux/kvm_host.h |    7 ++
 virt/kvm/irqfd.c         |  146 ++++++++++++++++++++++++++++++++++++++++++++++
 virt/kvm/kvm_main.c      |   12 ++++
 6 files changed, 174 insertions(+), 1 deletions(-)
 create mode 100644 virt/kvm/irqfd.c

arch/x86/kvm/Makefile +1 -1

index b43c4ef..d5fff51 100644
@@ -3,7 +3,7 @@
 #
 
 common-objs = $(addprefix ../../../virt/kvm/, kvm_main.o ioapic.o \
-                coalesced_mmio.o irq_comm.o)
+                coalesced_mmio.o irq_comm.o irqfd.o)
 ifeq ($(CONFIG_KVM_TRACE),y)
 common-objs += $(addprefix ../../../virt/kvm/, kvm_trace.o)
 endif

arch/x86/kvm/x86.c +1 -0

index 8cb8542..88c45e6 100644
@@ -1027,6 +1027,7 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_REINJECT_CONTROL:
 	case KVM_CAP_IRQ_INJECT_STATUS:
 	case KVM_CAP_ASSIGN_DEV_IRQ:
+	case KVM_CAP_IRQFD:
 		r = 1;
 		break;
 	case KVM_CAP_COALESCED_MMIO:

include/linux/kvm.h +7 -0

index 3db5d8d..2404775 100644
@@ -415,6 +415,7 @@ struct kvm_trace_rec {
 #define KVM_CAP_ASSIGN_DEV_IRQ 29
 /* Another bug in KVM_SET_USER_MEMORY_REGION fixed: */
 #define KVM_CAP_JOIN_MEMORY_REGIONS_WORKS 30
+#define KVM_CAP_IRQFD 31
 
 #ifdef KVM_CAP_IRQ_ROUTING
 

@@ -454,6 +455,11 @@ struct kvm_irq_routing {
 
 #endif
 
+struct kvm_irqfd {
+	__u32 fd;
+	__u32 gsi;
+};
+
 /*
  * ioctls for VM fds
  */

@@ -498,6 +504,7 @@ struct kvm_irq_routing {
 #define KVM_ASSIGN_SET_MSIX_ENTRY \
 			_IOW(KVMIO, 0x74, struct kvm_assigned_msix_entry)
 #define KVM_DEASSIGN_DEV_IRQ       _IOW(KVMIO, 0x75, struct kvm_assigned_irq)
+#define KVM_ASSIGN_IRQFD          _IOW(KVMIO, 0x76, struct kvm_irqfd)
 
 /*
  * ioctls for vcpu fds

include/linux/kvm_host.h +7 -0

index 095ebb6..57f8dfe 100644
@@ -134,6 +134,10 @@ struct kvm {
 	struct list_head vm_list;
 	struct kvm_io_bus mmio_bus;
 	struct kvm_io_bus pio_bus;
+	struct {
+		struct list_head items;
+		int              src;
+	} irqfd;
 	struct kvm_vm_stat stat;
 	struct kvm_arch arch;
 	atomic_t users_count;

@@ -524,4 +528,7 @@ static inline void kvm_free_irq_routing(struct kvm *kvm) {}
 
 #endif
 
+int kvm_irqfd_assign(struct kvm *kvm, int fd, int gsi);
+void kvm_irqfd_release(struct kvm *kvm);
+
 #endif

virt/kvm/irqfd.c new file +88 -0

new file mode 100644
index 0000000..d2e0915
@@ -0,0 +1,146 @@
+/*
+ * irqfd: Allows an eventfd to be used to inject an interrupt to the guest
+ *
+ * Credit goes to Avi Kivity for the original idea.
+ *
+ * Copyright 2009 Novell.  All Rights Reserved.
+ *
+ * Author:
+ *	Gregory Haskins <[email protected]>
+ *
+ * This file is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+
+#include <linux/kvm_host.h>
+#include <linux/eventfd.h>
+#include <linux/workqueue.h>
+#include <linux/wait.h>
+#include <linux/poll.h>
+#include <linux/file.h>
+#include <linux/list.h>
+
+struct _irqfd {
+	struct kvm               *kvm;
+	int                       gsi;
+	struct file              *file;
+	struct list_head          list;
+	poll_table                pt;
+	wait_queue_head_t        *wqh;
+	wait_queue_t              wait;
+	struct work_struct        work;
+};
+
+static void
+irqfd_inject(struct work_struct *work)
+{
+	struct _irqfd *irqfd = container_of(work, struct _irqfd, work);
+	struct kvm *kvm = irqfd->kvm;
+
+	mutex_lock(&kvm->lock);
+	kvm_set_irq(kvm, kvm->irqfd.src, irqfd->gsi, 1);
+	mutex_unlock(&kvm->lock);
+}
+
+static int
+irqfd_wakeup(wait_queue_t *wait, unsigned mode, int sync, void *key)
+{
+	struct _irqfd *irqfd = container_of(wait, struct _irqfd, wait);
+
+	/*
+	 * The eventfd calls its wake_up with interrupts disabled,
+	 * so we need to defer the IRQ injection until later since we need
+	 * to acquire the kvm->lock to do so.
+	 */
+	schedule_work(&irqfd->work);
+
+	return 0;
+}
+
+static void
+irqfd_ptable_queue_proc(struct file *file, wait_queue_head_t *wqh,
+			poll_table *pt)
+{
+	struct _irqfd *irqfd = container_of(pt, struct _irqfd, pt);
+
+	irqfd->wqh = wqh;
+	add_wait_queue(wqh, &irqfd->wait);
+}
+
+int
+kvm_irqfd_assign(struct kvm *kvm, int fd, int gsi)
+{
+	struct _irqfd *irqfd;
+	struct file *file;
+	int ret;
+
+	irqfd = kzalloc(sizeof(*irqfd), GFP_KERNEL);
+	if (!irqfd)

virt/kvm/irqfd.c new file +29 -0

+	if (IS_ERR(file)) {
+		ret = PTR_ERR(file);
+		goto fail;
+	}
+
+	ret = file->f_op->poll(file, &irqfd->pt);
+	/* do we need to look for errors in ret? */
+
+	irqfd->file = file;
+
+	mutex_lock(&kvm->lock);
+	if (kvm->irqfd.src == -1) {
+		ret = kvm_request_irq_source_id(kvm);
+		BUG_ON(ret < 0);
+
+		kvm->irqfd.src = ret;
+	}
+
+	list_add_tail(&irqfd->list, &kvm->irqfd.items);
+
+	mutex_unlock(&kvm->lock);
+
+	return 0;
+
+fail:
+	kfree(irqfd);
+	return ret;
+}
+

virt/kvm/irqfd.c new file +9 -0

+
+		list_del(&irqfd->list);
+		kfree(irqfd);
+	}
+
+	if (kvm->irqfd.src != -1)
+		kvm_free_irq_source_id(kvm, kvm->irqfd.src);
+
+}

virt/kvm/kvm_main.c +12 -1

index 3265566..c822d79 100644
@@ -972,6 +972,8 @@ static struct kvm *kvm_create_vm(void)
 	atomic_inc(&kvm->mm->mm_count);
 	spin_lock_init(&kvm->mmu_lock);
 	kvm_io_bus_init(&kvm->pio_bus);
+	INIT_LIST_HEAD(&kvm->irqfd.items);
+	kvm->irqfd.src = -1;
 	mutex_init(&kvm->lock);
 	kvm_io_bus_init(&kvm->mmio_bus);
 	init_rwsem(&kvm->slots_lock);

@@ -1023,6 +1025,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
 	spin_lock(&kvm_lock);
 	list_del(&kvm->vm_list);
 	spin_unlock(&kvm_lock);
+	kvm_irqfd_release(kvm);
 	kvm_free_irq_routing(kvm);
 	kvm_io_bus_destroy(&kvm->pio_bus);
 	kvm_io_bus_destroy(&kvm->mmio_bus);

@@ -2197,6 +2200,15 @@ static long kvm_vm_ioctl(struct file *filp,
 	}
 #endif
 #endif /* KVM_CAP_IRQ_ROUTING */
+	case KVM_ASSIGN_IRQFD: {
+		struct kvm_irqfd data;
+
+		r = -EFAULT;
+		if (copy_from_user(&data, argp, sizeof data))
+			goto out;
+		r = kvm_irqfd_assign(kvm, data.fd, data.gsi);
+		break;
+	}
 	default:
 		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
 	}
 
--

To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Hi David,
See inline for an answer to your last question:

virt/kvm/irqfd.c new file +18 -0

+
+#include <linux/kvm_host.h>
+#include <linux/eventfd.h>
+#include <linux/workqueue.h>
+#include <linux/wait.h>
+#include <linux/poll.h>
+#include <linux/file.h>
+#include <linux/list.h>
+
+struct _irqfd {
+	struct kvm               *kvm;
+	int                       gsi;
+	struct file              *file;
+	struct list_head          list;
+	poll_table                pt;
+	wait_queue_head_t        *wqh;
+	wait_queue_t              wait;
+	struct work_struct        work;

virt/kvm/irqfd.c new file +7 -0

+	mutex_unlock(&kvm->lock);
+}
+
+static int
+irqfd_wakeup(wait_queue_t *wait, unsigned mode, int sync, void *key)
+{
+	struct _irqfd *irqfd = container_of(wait, struct _irqfd, wait);

virt/kvm/irqfd.c new file +16 -0

+int
+kvm_irqfd_assign(struct kvm *kvm, int fd, int gsi)
+{
+	struct _irqfd *irqfd;
+	struct file *file;
+	int ret;
+
+	irqfd = kzalloc(sizeof(*irqfd), GFP_KERNEL);
+	if (!irqfd)
+		return -ENOMEM;
+
+	irqfd->kvm = kvm;
+	irqfd->gsi = gsi;
+	INIT_LIST_HEAD(&irqfd->list);
+	init_waitqueue_func_entry(&irqfd->wait, irqfd_wakeup);
+	init_poll_funcptr(&irqfd->pt, irqfd_ptable_queue_proc);

Quoted 6 lines · Gregory Haskins

This allows an eventfd to be registered as an irq source with a guest. Any
signaling operation on the eventfd (via userspace or kernel) will inject the registered GSI at the next available window.

include/linux/kvm.h struct kvm_irq_routing { · line 462

Quoted 6 lines

include/linux/kvm.h +5 -0

+struct kvm_irqfd {
+    __u32 fd;
+    __u32 gsi;
+};
+

I think it's better to have ioctl create and return the fd. This way we aren't tied to eventfd (though it makes a lot of sense to use it).

Also, please add a flags field and some padding so we can extend it later.

Quoted 24 lines

virt/kvm/irqfd.c new file +24 -0

+
+#include <linux/kvm_host.h>
+#include <linux/eventfd.h>
+#include <linux/workqueue.h>
+#include <linux/wait.h>
+#include <linux/poll.h>
+#include <linux/file.h>
+#include <linux/list.h>
+
+struct _irqfd {
+    struct kvm               *kvm;
+    int                       gsi;
+    struct file              *file;
+    struct list_head          list;
+    poll_table                pt;
+    wait_queue_head_t        *wqh;
+    wait_queue_t              wait;
+    struct work_struct        work;
+};
+
+static void
+irqfd_inject(struct work_struct *work)
+{
+    struct _irqfd *irqfd = container_of(work, struct _irqfd, work);

virt/kvm/irqfd.c line 51

Quoted 5 lines

virt/kvm/irqfd.c new file +4 -0

+    struct kvm *kvm = irqfd->kvm;
+
+    mutex_lock(&kvm->lock);
+    kvm_set_irq(kvm, kvm->irqfd.src, irqfd->gsi, 1);

Need to lower the irq too (though irqfd only supports edge triggered interrupts).

Quoted 14 lines

virt/kvm/irqfd.c new file +14 -0

+    mutex_unlock(&kvm->lock);
+}
+
+static int
+irqfd_wakeup(wait_queue_t *wait, unsigned mode, int sync, void *key)
+{
+    struct _irqfd *irqfd = container_of(wait, struct _irqfd, wait);
+
+    /*
+     * The eventfd calls its wake_up with interrupts disabled,
+     * so we need to defer the IRQ injection until later since we need
+     * to acquire the kvm->lock to do so.
+     */
+    schedule_work(&irqfd->work);

virt/kvm/irqfd.c line 68

Quoted 4 lines

virt/kvm/irqfd.c new file +3 -0

+
+    return 0;
+}

One day we'll have lockless injection and we'll want to drop this. I guess if we create the fd ourselves we can make it work, but I don't see how we can do this with eventfd.

Quoted 21 lines

virt/kvm/irqfd.c new file +21 -0

+int
+kvm_irqfd_assign(struct kvm *kvm, int fd, int gsi)
+{
+    struct _irqfd *irqfd;
+    struct file *file;
+    int ret;
+
+    irqfd = kzalloc(sizeof(*irqfd), GFP_KERNEL);
+    if (!irqfd)
+        return -ENOMEM;
+
+    irqfd->kvm = kvm;
+    irqfd->gsi = gsi;
+    INIT_LIST_HEAD(&irqfd->list);
+    init_waitqueue_func_entry(&irqfd->wait, irqfd_wakeup);
+    init_poll_funcptr(&irqfd->pt, irqfd_ptable_queue_proc);
+    INIT_WORK(&irqfd->work, irqfd_inject);
+
+    file = eventfd_fget(fd);
+    if (IS_ERR(file)) {
+        ret = PTR_ERR(file);

virt/kvm/irqfd.c line 105

Quoted 6 lines

virt/kvm/irqfd.c new file +5 -0

+        goto fail;
+    }
+
+    ret = file->f_op->poll(file, &irqfd->pt);
+    /* do we need to look for errors in ret? */

Do we?

virt/kvm/irqfd.c line 112

Quoted 9 lines

virt/kvm/irqfd.c new file +7 -0

+
+    irqfd->file = file;
+
+    mutex_lock(&kvm->lock);
+    if (kvm->irqfd.src == -1) {
+        ret = kvm_request_irq_source_id(kvm);
+        BUG_ON(ret < 0);

I think you can reuse the userspace irq source (since it's just another way for userspace to inject an interrupt). It isn't really needed since the irq source stuff is only needed to support level triggered interrupts.

Quoted 6 lines

Quoted 6 lines

This allows an eventfd to be registered as an irq source with a guest. Any
signaling operation on the eventfd (via userspace or kernel) will inject the registered GSI at the next available window.

Quoted 10 lines

include/linux/kvm.h struct kvm_irq_routing { · line 462

Quoted 7 lines

include/linux/kvm.h +5 -0

+struct kvm_irqfd {
+    __u32 fd;
+    __u32 gsi;
+};
+

I think it's better to have ioctl create and return the fd. This way we aren't tied to eventfd (though it makes a lot of sense to use it).

I dont mind either way, but I am not sure it buys us much as the one driving the fd would need to understand if the interface is eventfd-esque or something else anyway. Let me know if you still want to see this changed.

Quoted 4 lines

Quoted 4 lines

virt/kvm/irqfd.c new file +4 -0

+static void
+irqfd_inject(struct work_struct *work)
+{
+    struct _irqfd *irqfd = container_of(work, struct _irqfd, work);

Quoted 10 lines

virt/kvm/irqfd.c line 51

Quoted 6 lines

virt/kvm/irqfd.c new file +4 -0

+    struct kvm *kvm = irqfd->kvm;
+
+    mutex_lock(&kvm->lock);
+    kvm_set_irq(kvm, kvm->irqfd.src, irqfd->gsi, 1);

Need to lower the irq too (though irqfd only supports edge triggered interrupts).

Should I just do back-to-back 1+0 inside the same lock?

Quoted 5 lines

One day we'll have lockless injection and we'll want to drop this. I guess if we create the fd ourselves we can make it work, but I don't see how we can do this with eventfd.

Hmm...this is a good point. There probably is no way to use eventfd "off the shelf" in a way that doesn't cause this callback to be in a critical section. Should we just worry about switching away from eventfd when this occurs, or should I implement a custom anon-fd now?

Quoted 17 lines · Gregory Haskins

Quoted 7 lines

Quoted 7 lines

This allows an eventfd to be registered as an irq source with a guest. Any
signaling operation on the eventfd (via userspace or kernel) will inject
the registered GSI at the next available window.

Quoted 9 lines

include/linux/kvm.h struct kvm_irq_routing { · line 462

Quoted 6 lines

include/linux/kvm.h +5 -0

+struct kvm_irqfd {
+    __u32 fd;
+    __u32 gsi;
+};
+

I think it's better to have ioctl create and return the fd. This way we aren't tied to eventfd (though it makes a lot of sense to use it).

Quoted 5 lines

I dont mind either way, but I am not sure it buys us much as the one driving the fd would need to understand if the interface is eventfd-esque or something else anyway. Let me know if you still want to see this changed.

Sure, the interface remains the same (write 8 bytes), but the implementation can change. For example, we can implement it to work from interrupt context, once we hack the locking appropriately.

Quoted 15 lines

Quoted 15 lines

virt/kvm/irqfd.c line 51

Quoted 11 lines

virt/kvm/irqfd.c new file +8 -0

+static void
+irqfd_inject(struct work_struct *work)
+{
+    struct _irqfd *irqfd = container_of(work, struct _irqfd, work);
+    struct kvm *kvm = irqfd->kvm;
+
+    mutex_lock(&kvm->lock);
+    kvm_set_irq(kvm, kvm->irqfd.src, irqfd->gsi, 1);

Need to lower the irq too (though irqfd only supports edge triggered interrupts).

Quoted 3 lines

Should I just do back-to-back 1+0 inside the same lock?

Yes. Might be nice to add a kvm_toggle_irq(), but let's leave that until later.

Quoted 10 lines

Quoted 9 lines

One day we'll have lockless injection and we'll want to drop this. I guess if we create the fd ourselves we can make it work, but I don't see how we can do this with eventfd.

Quoted 5 lines

Hmm...this is a good point. There probably is no way to use eventfd "off the shelf" in a way that doesn't cause this callback to be in a critical section. Should we just worry about switching away from eventfd when this occurs, or should I implement a custom anon-fd now?

I'd just go with eventfd, and switch when it becomes relevant. As long as the kernel allocates the fd, we're free to do as we like.

Quoted 4 lines

Sure, the interface remains the same (write 8 bytes), but the implementation can change. For example, we can implement it to work from interrupt context, once we hack the locking appropriately.

I was thinking more along the lines of eventfd_signal(). AIO and vbus currently use this interface, as opposed to the more polymorhpic f_ops->write().

Quoted 5 lines · Gregory Haskins

Quoted 4 lines

Sure, the interface remains the same (write 8 bytes), but the implementation can change. For example, we can implement it to work from interrupt context, once we hack the locking appropriately.

Quoted 5 lines

I was thinking more along the lines of eventfd_signal(). AIO and vbus currently use this interface, as opposed to the more polymorhpic f_ops->write().

But eventfd_signal basically marries us to eventfd.

Quoted 2 lines

But eventfd_signal basically marries us to eventfd.

Well, only if we expect the fd to have eventfd semantics. There are advantages to doing so, as we have discussed, because things like AIO can polymorhpically signal an interrupt without even knowing whats behind the eventfd. But this isn't a strict requirement to support AIO. Really all we need is a way for both kernel and userspace to signal. Perhaps I should export an "irqfd_signal()" function from kvm, which today will map to eventfd_signal(), and tomorrow to ??. I don't think using f_ops->write() is an option for in-kernel signaling, so we need some kind of interface here.

Does that sound reasonable?

Quoted 13 lines · Gregory Haskins

Quoted 2 lines

But eventfd_signal basically marries us to eventfd.

Well, only if we expect the fd to have eventfd semantics. There are advantages to doing so, as we have discussed, because things like AIO can polymorhpically signal an interrupt without even knowing whats behind the eventfd. But this isn't a strict requirement to support AIO. Really all we need is a way for both kernel and userspace to signal. Perhaps I should export an "irqfd_signal()" function from kvm, which today will map to eventfd_signal(), and tomorrow to ??. I don't think using f_ops->write() is an option for in-kernel signaling, so we need some kind of interface here.

Quoted 2 lines

Does that sound reasonable?

irqfd_signal() ties the user of irqfd to kvm. I want this user to be independent of kvm; it should work with eventfd, kvm's eventfd lookalike (if we move away from eventfd) or pipes.

Quoted 16 lines

Quoted 16 lines

Quoted 3 lines

But eventfd_signal basically marries us to eventfd.

Well, only if we expect the fd to have eventfd semantics. There are advantages to doing so, as we have discussed, because things like AIO can polymorhpically signal an interrupt without even knowing whats behind the eventfd. But this isn't a strict requirement to support AIO. Really all we need is a way for both kernel and userspace to signal. Perhaps I should export an "irqfd_signal()" function from kvm, which today will map to eventfd_signal(), and tomorrow to ??. I don't think using f_ops->write() is an option for in-kernel signaling, so we need some kind of interface here.

Does that sound reasonable?

Quoted 4 lines

irqfd_signal() ties the user of irqfd to kvm. I want this user to be independent of kvm; it should work with eventfd, kvm's eventfd lookalike (if we move away from eventfd) or pipes.

So what is your proposal for such interface?

Quoted 4 lines

So what is your proposal for such interface?

->write().

Quoted 5 lines · Avi Kivity

Quoted 4 lines

So what is your proposal for such interface?

Quoted 2 lines

->write().

Alternatively, a new fileop ->signal_event(), which would map to eventfd_signal() or irqfd_signal(). This would be defined to work in irq contexts. It may also be useful for uio interrupts.

Quoted 6 lines · Avi Kivity

Quoted 4 lines · Avi Kivity

Quoted 4 lines

So what is your proposal for such interface?

Quoted 8 lines

Quoted 3 lines

->write().

Alternatively, a new fileop ->signal_event(), which would map to eventfd_signal() or irqfd_signal(). This would be defined to work in irq contexts. It may also be useful for uio interrupts.

Hmm...I'm not crazy about either of those. write() has obvious limitations both from a interrupt execution context, as well as the awkwardness of dealing with creating+passing a viable "userspace" pointer from kernel code. On the other hand, a new fileop doesn't quite seem appropriate either since it doesn't apply to the overall fileop abstraction very well.

We could potentially have a separate vtable interface just for
event-type fds, and make eventfd and irqfd the first implementations. But I am not sure it is worth it. What I suggest is that we work within the existing eventfd interface. It was designed specifically to signal
events, after all.

If at some point in the future we need to ensure that the callbacks are
not invoked from a preempt-off/irq-off critical section, we can revist
the eventfd internals at that time. Note that since we would like to
support signaling from interrupt context anyway, trying to get rid of
the wqh critical section that we have today may be a fools errand (*). Instead, we should probably focus on making the injection path support non-preemptible contexts, as this will have the biggest benefits and
gains in the long run.

Quoted 28 lines · Gregory Haskins

Quoted 5 lines · Avi Kivity

Quoted 3 lines · Avi Kivity

Quoted 3 lines

So what is your proposal for such interface?

Quoted 8 lines

Quoted 3 lines

->write().

Alternatively, a new fileop ->signal_event(), which would map to eventfd_signal() or irqfd_signal(). This would be defined to work in irq contexts. It may also be useful for uio interrupts.

Hmm...I'm not crazy about either of those. write() has obvious limitations both from a interrupt execution context, as well as the awkwardness of dealing with creating+passing a viable "userspace" pointer from kernel code. On the other hand, a new fileop doesn't quite seem appropriate either since it doesn't apply to the overall fileop abstraction very well.

We could potentially have a separate vtable interface just for event-type fds, and make eventfd and irqfd the first implementations. But I am not sure it is worth it. What I suggest is that we work within the existing eventfd interface. It was designed specifically to signal events, after all.

Quoted 11 lines

If at some point in the future we need to ensure that the callbacks are not invoked from a preempt-off/irq-off critical section, we can revist the eventfd internals at that time. Note that since we would like to support signaling from interrupt context anyway, trying to get rid of the wqh critical section that we have today may be a fools errand (*). Instead, we should probably focus on making the injection path support
non-preemptible contexts, as this will have the biggest benefits and gains in the long run.

But again, you're forcing everyone who uses irqfd to require eventfd.

Maybe we should change eventfd_signal() to fall back to ->write if the file happens not to be an eventfd. It could also handle the nonpreemptible context as well.

Quoted 6 lines · Avi Kivity

One day we'll have lockless injection and we'll want to drop this. I guess if we create the fd ourselves we can make it work, but I don't see how we can do this with eventfd.

Hmm...this is a good point. There probably is no way to use eventfd "off the shelf" in a way that doesn't cause this callback to be in a critical section. Should we just worry about switching away from eventfd when this occurs, or should I implement a custom anon-fd now?

Something else to consider: eventfd supports calling eventfd_signal() from interrupt context, so
even if the callback were not invoked from a preempt/irq off CS due to the wqh->lock, the context may still be a CS anyway. Now, userspace and vbus based injection do not need to worry about this, but I wonder if this is a desirable attribute for some other source (device-assignment perhaps)?

If so, is there a way to design the lockless-injection code such that its still friendly to irq-context, or is this a mode that we would never want to support anyway? I think this would be a good thing to have at least to maintain compatibility with the existing eventfd interface, which has its own advantages.