2015-07-16 21:26:54

by Alex Williamson

[permalink] [raw]
Subject: [PATCH v2] virt: IRQ bypass manager

When a physical I/O device is assigned to a virtual machine through
facilities like VFIO and KVM, the interrupt for the device generally
bounces through the host system before being injected into the VM.
However, hardware technologies exist that often allow the host to be
bypassed for some of these scenarios. Intel Posted Interrupts allow
the specified physical edge interrupts to be directly injected into a
guest when delivered to a physical processor while the vCPU is
running. ARM IRQ Forwarding allows forwarded physical interrupts to
be directly deactivated by the guest.

The IRQ bypass manager here is meant to provide the shim to connect
interrupt producers, generally the host physical device driver, with
interrupt consumers, generally the hypervisor, in order to configure
these bypass mechanism. To do this, we base the connection on a
shared, opaque token. For KVM-VFIO this is expected to be an
eventfd_ctx since this is the connection we already use to connect an
eventfd to an irqfd on the in-kernel path. When a producer and
consumer with matching tokens is found, callbacks via both registered
participants allow the bypass facilities to be automatically enabled.

Signed-off-by: Alex Williamson <[email protected]>
---

I'm not sure if we're settled on everything, I think Eric is out of
the office and we haven't really resolved the opaque data element.
I think it has problems because it assumes which side gets to provide
the data and implies that the other side knows that the data is
compatible. We also don't need to pass it as a callback argument if
we put it on the consumer/produce struct and consider it public.
However, I'm posting this to record the current state of this code
and to make sure I'm not the bottleneck.

v2 Makes the rest of the changes suggested by Eric, various comment
fixes, error paths for the add callbacks, and adds a MAINTAINERS
entry for virt/lib. Thanks,

Alex

MAINTAINERS | 7 +
include/linux/irqbypass.h | 90 ++++++++++++++++
virt/lib/Kconfig | 2
virt/lib/Makefile | 1
virt/lib/irqbypass.c | 260 +++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 360 insertions(+)
create mode 100644 include/linux/irqbypass.h
create mode 100644 virt/lib/Kconfig
create mode 100644 virt/lib/Makefile
create mode 100644 virt/lib/irqbypass.c

diff --git a/MAINTAINERS b/MAINTAINERS
index d8afd29..8e728cb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10613,6 +10613,13 @@ L: [email protected]
S: Maintained
F: drivers/net/ethernet/via/via-velocity.*

+VIRT LIB
+M: Alex Williamson <[email protected]>
+M: Paolo Bonzini <[email protected]>
+L: [email protected]
+S: Supported
+F: virt/lib/
+
VIVID VIRTUAL VIDEO DRIVER
M: Hans Verkuil <[email protected]>
L: [email protected]
diff --git a/include/linux/irqbypass.h b/include/linux/irqbypass.h
new file mode 100644
index 0000000..fde7b64
--- /dev/null
+++ b/include/linux/irqbypass.h
@@ -0,0 +1,90 @@
+/*
+ * IRQ offload/bypass manager
+ *
+ * Copyright (C) 2015 Red Hat, Inc.
+ * Copyright (c) 2015 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef IRQBYPASS_H
+#define IRQBYPASS_H
+
+#include <linux/list.h>
+
+struct irq_bypass_consumer;
+
+/*
+ * Theory of operation
+ *
+ * The IRQ bypass manager is a simple set of lists and callbacks that allows
+ * IRQ producers (ex. physical interrupt sources) to be matched to IRQ
+ * consumers (ex. virtualization hardware that allows IRQ bypass or offload)
+ * via a shared token (ex. eventfd_ctx). Producers and consumers register
+ * independently. When a token match is found, the optional @stop callback
+ * will be called for each participant. The pair will then be connected via
+ * the @add_* callbacks, and finally the optional @start callback will allow
+ * any final coordination. When either participant is unregistered, the
+ * process is repeated using the @del_* callbacks in place of the @add_*
+ * callbacks. Match tokens must be unique per producer/consumer, 1:N pairings
+ * are not supported.
+ */
+
+/**
+ * struct irq_bypass_producer - IRQ bypass producer definition
+ * @node: IRQ bypass manager private list management
+ * @token: opaque token to match between producer and consumer
+ * @irq: Linux IRQ number for the producer device
+ * @add_consumer: Connect the IRQ producer to an IRQ consumer
+ * @del_consumer: Disconnect the IRQ producer from an IRQ consumer
+ * @stop: Perform any quiesce operations necessary prior to add/del (optional)
+ * @start: Perform any startup operations necessary after add/del (optional)
+ *
+ * The IRQ bypass producer structure represents an interrupt source for
+ * participation in possible host bypass, for instance an interrupt vector
+ * for a physical device assigned to a VM.
+ */
+struct irq_bypass_producer {
+ struct list_head node;
+ void *token;
+ int irq;
+ int (*add_consumer)(struct irq_bypass_producer *,
+ struct irq_bypass_consumer *);
+ void (*del_consumer)(struct irq_bypass_producer *,
+ struct irq_bypass_consumer *);
+ void (*stop)(struct irq_bypass_producer *);
+ void (*start)(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
+ * @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)
+ * @start: Perform any startup operations necessary after add/del (optional)
+ *
+ * The IRQ bypass consumer structure represents an interrupt sink for
+ * participation in possible host bypass, for instance a hypervisor may
+ * support offloads to allow bypassing the host entirely or offload
+ * portions of the interrupt handling to the VM.
+ */
+struct irq_bypass_consumer {
+ struct list_head node;
+ void *token;
+ int (*add_producer)(struct irq_bypass_consumer *,
+ struct irq_bypass_producer *);
+ void (*del_producer)(struct irq_bypass_consumer *,
+ struct irq_bypass_producer *);
+ void (*stop)(struct irq_bypass_consumer *);
+ void (*start)(struct irq_bypass_consumer *);
+};
+
+int irq_bypass_register_producer(struct irq_bypass_producer *);
+void irq_bypass_unregister_producer(struct irq_bypass_producer *);
+int irq_bypass_register_consumer(struct irq_bypass_consumer *);
+void irq_bypass_unregister_consumer(struct irq_bypass_consumer *);
+
+#endif /* IRQBYPASS_H */
diff --git a/virt/lib/Kconfig b/virt/lib/Kconfig
new file mode 100644
index 0000000..89a414f
--- /dev/null
+++ b/virt/lib/Kconfig
@@ -0,0 +1,2 @@
+config IRQ_BYPASS_MANAGER
+ tristate
diff --git a/virt/lib/Makefile b/virt/lib/Makefile
new file mode 100644
index 0000000..901228d
--- /dev/null
+++ b/virt/lib/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_IRQ_BYPASS_MANAGER) += irqbypass.o
diff --git a/virt/lib/irqbypass.c b/virt/lib/irqbypass.c
new file mode 100644
index 0000000..6ae8e0f
--- /dev/null
+++ b/virt/lib/irqbypass.c
@@ -0,0 +1,260 @@
+/*
+ * IRQ offload/bypass manager
+ *
+ * Copyright (C) 2015 Red Hat, Inc.
+ * Copyright (c) 2015 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Various virtualization hardware acceleration techniques allow bypassing or
+ * offloading interrupts received from devices around the host kernel. Posted
+ * Interrupts on Intel VT-d systems can allow interrupts to be received
+ * directly by a virtual machine. ARM IRQ Forwarding allows forwarded physical
+ * interrupts to be directly deactivated by the guest. This manager allows
+ * interrupt producers and consumers to find each other to enable this sort of
+ * bypass.
+ */
+
+#include <linux/irqbypass.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("IRQ bypass manager utility module");
+
+static LIST_HEAD(producers);
+static LIST_HEAD(consumers);
+static DEFINE_MUTEX(lock);
+
+/* @lock must be held when calling connect */
+static int __connect(struct irq_bypass_producer *prod,
+ struct irq_bypass_consumer *cons)
+{
+ int ret;
+
+ if (prod->stop)
+ prod->stop(prod);
+ if (cons->stop)
+ cons->stop(cons);
+
+ ret = prod->add_consumer(prod, cons);
+ if (!ret) {
+ ret = cons->add_producer(cons, prod);
+ if (ret)
+ prod->del_consumer(prod, cons);
+ }
+
+ if (cons->start)
+ cons->start(cons);
+ if (prod->start)
+ prod->start(prod);
+
+ return ret;
+}
+
+/* @lock must be held when calling disconnect */
+static void __disconnect(struct irq_bypass_producer *prod,
+ struct irq_bypass_consumer *cons)
+{
+ if (prod->stop)
+ prod->stop(prod);
+ if (cons->stop)
+ cons->stop(cons);
+
+ cons->del_producer(cons, prod);
+ prod->del_consumer(prod, cons);
+
+ if (cons->start)
+ cons->start(cons);
+ if (prod->start)
+ prod->start(prod);
+}
+
+/**
+ * irq_bypass_register_producer - register IRQ bypass producer
+ * @producer: pointer to producer structure
+ *
+ * Add the provided IRQ producer to the list of producers and connect
+ * with any matching token found on the IRQ consumers list.
+ */
+int irq_bypass_register_producer(struct irq_bypass_producer *producer)
+{
+ struct irq_bypass_producer *tmp;
+ struct irq_bypass_consumer *consumer;
+ int ret = 0;
+
+ if (!producer->add_consumer || !producer->del_consumer)
+ return -EINVAL;
+
+ might_sleep();
+
+ if (!try_module_get(THIS_MODULE))
+ return -ENODEV;
+
+ mutex_lock(&lock);
+
+ list_for_each_entry(tmp, &producers, node) {
+ if (tmp->token == producer->token) {
+ mutex_unlock(&lock);
+ module_put(THIS_MODULE);
+ return -EINVAL;
+ }
+ }
+
+ list_for_each_entry(consumer, &consumers, node) {
+ if (consumer->token == producer->token) {
+ ret = __connect(producer, consumer);
+ break;
+ }
+ }
+
+ if (!ret)
+ list_add(&producer->node, &producers);
+
+ mutex_unlock(&lock);
+
+ if (ret)
+ module_put(THIS_MODULE);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(irq_bypass_register_producer);
+
+/**
+ * irq_bypass_unregister_producer - unregister IRQ bypass producer
+ * @producer: pointer to producer structure
+ *
+ * Remove a previously registered IRQ producer from the list of producers
+ * and disconnect it from any connected IRQ consumer.
+ */
+void irq_bypass_unregister_producer(struct irq_bypass_producer *producer)
+{
+ struct irq_bypass_producer *tmp;
+ struct irq_bypass_consumer *consumer;
+
+ might_sleep();
+
+ if (!try_module_get(THIS_MODULE))
+ return; /* nothing in the list anyway */
+
+ mutex_lock(&lock);
+
+ list_for_each_entry(tmp, &producers, node) {
+ if (tmp->token == producer->token)
+ break;
+ }
+
+ if (tmp) {
+ list_for_each_entry(consumer, &consumers, node) {
+ if (consumer->token == producer->token) {
+ __disconnect(producer, consumer);
+ break;
+ }
+ }
+
+ list_del(&producer->node);
+ }
+
+ mutex_unlock(&lock);
+
+ module_put(THIS_MODULE);
+ if (tmp)
+ module_put(THIS_MODULE);
+}
+EXPORT_SYMBOL_GPL(irq_bypass_unregister_producer);
+
+/**
+ * irq_bypass_register_consumer - register IRQ bypass consumer
+ * @consumer: pointer to consumer structure
+ *
+ * Add the provided IRQ consumer to the list of consumers and connect
+ * with any matching token found on the IRQ producer list.
+ */
+int irq_bypass_register_consumer(struct irq_bypass_consumer *consumer)
+{
+ struct irq_bypass_consumer *tmp;
+ struct irq_bypass_producer *producer;
+ int ret = 0;
+
+ if (!consumer->add_producer || !consumer->del_producer)
+ return -EINVAL;
+
+ might_sleep();
+
+ if (!try_module_get(THIS_MODULE))
+ return -ENODEV;
+
+ mutex_lock(&lock);
+
+ list_for_each_entry(tmp, &consumers, node) {
+ if (tmp->token == consumer->token) {
+ mutex_unlock(&lock);
+ module_put(THIS_MODULE);
+ return -EINVAL;
+ }
+ }
+
+ list_for_each_entry(producer, &producers, node) {
+ if (producer->token == consumer->token) {
+ ret = __connect(producer, consumer);
+ break;
+ }
+ }
+
+ if (!ret)
+ list_add(&consumer->node, &consumers);
+
+ mutex_unlock(&lock);
+
+ if (ret)
+ module_put(THIS_MODULE);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(irq_bypass_register_consumer);
+
+/**
+ * irq_bypass_unregister_consumer - unregister IRQ bypass consumer
+ * @consumer: pointer to consumer structure
+ *
+ * Remove a previously registered IRQ consumer from the list of consumers
+ * and disconnect it from any connected IRQ producer.
+ */
+void irq_bypass_unregister_consumer(struct irq_bypass_consumer *consumer)
+{
+ struct irq_bypass_consumer *tmp;
+ struct irq_bypass_producer *producer;
+
+ might_sleep();
+
+ if (!try_module_get(THIS_MODULE))
+ return; /* nothing in the list anyway */
+
+ mutex_lock(&lock);
+
+ list_for_each_entry(tmp, &consumers, node) {
+ if (tmp->token == consumer->token)
+ break;
+ }
+
+ if (tmp) {
+ list_for_each_entry(producer, &producers, node) {
+ if (producer->token == consumer->token) {
+ __disconnect(producer, consumer);
+ break;
+ }
+ }
+
+ list_del(&consumer->node);
+ }
+
+ mutex_unlock(&lock);
+
+ module_put(THIS_MODULE);
+ if (tmp)
+ module_put(THIS_MODULE);
+}
+EXPORT_SYMBOL_GPL(irq_bypass_unregister_consumer);


2015-07-17 07:26:25

by Christian Borntraeger

[permalink] [raw]
Subject: Re: [PATCH v2] virt: IRQ bypass manager

Am 16.07.2015 um 23:26 schrieb Alex Williamson:
> When a physical I/O device is assigned to a virtual machine through
> facilities like VFIO and KVM, the interrupt for the device generally
> bounces through the host system before being injected into the VM.
> However, hardware technologies exist that often allow the host to be
> bypassed for some of these scenarios. Intel Posted Interrupts allow
> the specified physical edge interrupts to be directly injected into a
> guest when delivered to a physical processor while the vCPU is
> running. ARM IRQ Forwarding allows forwarded physical interrupts to
> be directly deactivated by the guest.
>
> The IRQ bypass manager here is meant to provide the shim to connect
> interrupt producers, generally the host physical device driver, with
> interrupt consumers, generally the hypervisor, in order to configure
> these bypass mechanism. To do this, we base the connection on a
> shared, opaque token. For KVM-VFIO this is expected to be an
> eventfd_ctx since this is the connection we already use to connect an
> eventfd to an irqfd on the in-kernel path. When a producer and
> consumer with matching tokens is found, callbacks via both registered
> participants allow the bypass facilities to be automatically enabled.
>
> Signed-off-by: Alex Williamson <[email protected]>
> ---
>
> I'm not sure if we're settled on everything, I think Eric is out of
> the office and we haven't really resolved the opaque data element.
> I think it has problems because it assumes which side gets to provide
> the data and implies that the other side knows that the data is
> compatible. We also don't need to pass it as a callback argument if
> we put it on the consumer/produce struct and consider it public.
> However, I'm posting this to record the current state of this code
> and to make sure I'm not the bottleneck.



Yi Min,

can you have a look if the IRQ passthrough that the pci virtualization
on s390 has would work with this? As far as I can tell, this looks like
we can implement whatever we want in kvm/s390 and this infrastructure
just connects vfio and kvm without assuming anything about the details
but lets double check.





>
> v2 Makes the rest of the changes suggested by Eric, various comment
> fixes, error paths for the add callbacks, and adds a MAINTAINERS
> entry for virt/lib. Thanks,
>
> Alex
>
> MAINTAINERS | 7 +
> include/linux/irqbypass.h | 90 ++++++++++++++++
> virt/lib/Kconfig | 2
> virt/lib/Makefile | 1
> virt/lib/irqbypass.c | 260 +++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 360 insertions(+)
> create mode 100644 include/linux/irqbypass.h
> create mode 100644 virt/lib/Kconfig
> create mode 100644 virt/lib/Makefile
> create mode 100644 virt/lib/irqbypass.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d8afd29..8e728cb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10613,6 +10613,13 @@ L: [email protected]
> S: Maintained
> F: drivers/net/ethernet/via/via-velocity.*
>
> +VIRT LIB
> +M: Alex Williamson <[email protected]>
> +M: Paolo Bonzini <[email protected]>
> +L: [email protected]
> +S: Supported
> +F: virt/lib/
> +
> VIVID VIRTUAL VIDEO DRIVER
> M: Hans Verkuil <[email protected]>
> L: [email protected]
> diff --git a/include/linux/irqbypass.h b/include/linux/irqbypass.h
> new file mode 100644
> index 0000000..fde7b64
> --- /dev/null
> +++ b/include/linux/irqbypass.h
> @@ -0,0 +1,90 @@
> +/*
> + * IRQ offload/bypass manager
> + *
> + * Copyright (C) 2015 Red Hat, Inc.
> + * Copyright (c) 2015 Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#ifndef IRQBYPASS_H
> +#define IRQBYPASS_H
> +
> +#include <linux/list.h>
> +
> +struct irq_bypass_consumer;
> +
> +/*
> + * Theory of operation
> + *
> + * The IRQ bypass manager is a simple set of lists and callbacks that allows
> + * IRQ producers (ex. physical interrupt sources) to be matched to IRQ
> + * consumers (ex. virtualization hardware that allows IRQ bypass or offload)
> + * via a shared token (ex. eventfd_ctx). Producers and consumers register
> + * independently. When a token match is found, the optional @stop callback
> + * will be called for each participant. The pair will then be connected via
> + * the @add_* callbacks, and finally the optional @start callback will allow
> + * any final coordination. When either participant is unregistered, the
> + * process is repeated using the @del_* callbacks in place of the @add_*
> + * callbacks. Match tokens must be unique per producer/consumer, 1:N pairings
> + * are not supported.
> + */
> +
> +/**
> + * struct irq_bypass_producer - IRQ bypass producer definition
> + * @node: IRQ bypass manager private list management
> + * @token: opaque token to match between producer and consumer
> + * @irq: Linux IRQ number for the producer device
> + * @add_consumer: Connect the IRQ producer to an IRQ consumer
> + * @del_consumer: Disconnect the IRQ producer from an IRQ consumer
> + * @stop: Perform any quiesce operations necessary prior to add/del (optional)
> + * @start: Perform any startup operations necessary after add/del (optional)
> + *
> + * The IRQ bypass producer structure represents an interrupt source for
> + * participation in possible host bypass, for instance an interrupt vector
> + * for a physical device assigned to a VM.
> + */
> +struct irq_bypass_producer {
> + struct list_head node;
> + void *token;
> + int irq;
> + int (*add_consumer)(struct irq_bypass_producer *,
> + struct irq_bypass_consumer *);
> + void (*del_consumer)(struct irq_bypass_producer *,
> + struct irq_bypass_consumer *);
> + void (*stop)(struct irq_bypass_producer *);
> + void (*start)(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
> + * @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)
> + * @start: Perform any startup operations necessary after add/del (optional)
> + *
> + * The IRQ bypass consumer structure represents an interrupt sink for
> + * participation in possible host bypass, for instance a hypervisor may
> + * support offloads to allow bypassing the host entirely or offload
> + * portions of the interrupt handling to the VM.
> + */
> +struct irq_bypass_consumer {
> + struct list_head node;
> + void *token;
> + int (*add_producer)(struct irq_bypass_consumer *,
> + struct irq_bypass_producer *);
> + void (*del_producer)(struct irq_bypass_consumer *,
> + struct irq_bypass_producer *);
> + void (*stop)(struct irq_bypass_consumer *);
> + void (*start)(struct irq_bypass_consumer *);
> +};
> +
> +int irq_bypass_register_producer(struct irq_bypass_producer *);
> +void irq_bypass_unregister_producer(struct irq_bypass_producer *);
> +int irq_bypass_register_consumer(struct irq_bypass_consumer *);
> +void irq_bypass_unregister_consumer(struct irq_bypass_consumer *);
> +
> +#endif /* IRQBYPASS_H */
> diff --git a/virt/lib/Kconfig b/virt/lib/Kconfig
> new file mode 100644
> index 0000000..89a414f
> --- /dev/null
> +++ b/virt/lib/Kconfig
> @@ -0,0 +1,2 @@
> +config IRQ_BYPASS_MANAGER
> + tristate
> diff --git a/virt/lib/Makefile b/virt/lib/Makefile
> new file mode 100644
> index 0000000..901228d
> --- /dev/null
> +++ b/virt/lib/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_IRQ_BYPASS_MANAGER) += irqbypass.o
> diff --git a/virt/lib/irqbypass.c b/virt/lib/irqbypass.c
> new file mode 100644
> index 0000000..6ae8e0f
> --- /dev/null
> +++ b/virt/lib/irqbypass.c
> @@ -0,0 +1,260 @@
> +/*
> + * IRQ offload/bypass manager
> + *
> + * Copyright (C) 2015 Red Hat, Inc.
> + * Copyright (c) 2015 Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Various virtualization hardware acceleration techniques allow bypassing or
> + * offloading interrupts received from devices around the host kernel. Posted
> + * Interrupts on Intel VT-d systems can allow interrupts to be received
> + * directly by a virtual machine. ARM IRQ Forwarding allows forwarded physical
> + * interrupts to be directly deactivated by the guest. This manager allows
> + * interrupt producers and consumers to find each other to enable this sort of
> + * bypass.
> + */
> +
> +#include <linux/irqbypass.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("IRQ bypass manager utility module");
> +
> +static LIST_HEAD(producers);
> +static LIST_HEAD(consumers);
> +static DEFINE_MUTEX(lock);
> +
> +/* @lock must be held when calling connect */
> +static int __connect(struct irq_bypass_producer *prod,
> + struct irq_bypass_consumer *cons)
> +{
> + int ret;
> +
> + if (prod->stop)
> + prod->stop(prod);
> + if (cons->stop)
> + cons->stop(cons);
> +
> + ret = prod->add_consumer(prod, cons);
> + if (!ret) {
> + ret = cons->add_producer(cons, prod);
> + if (ret)
> + prod->del_consumer(prod, cons);
> + }
> +
> + if (cons->start)
> + cons->start(cons);
> + if (prod->start)
> + prod->start(prod);
> +
> + return ret;
> +}
> +
> +/* @lock must be held when calling disconnect */
> +static void __disconnect(struct irq_bypass_producer *prod,
> + struct irq_bypass_consumer *cons)
> +{
> + if (prod->stop)
> + prod->stop(prod);
> + if (cons->stop)
> + cons->stop(cons);
> +
> + cons->del_producer(cons, prod);
> + prod->del_consumer(prod, cons);
> +
> + if (cons->start)
> + cons->start(cons);
> + if (prod->start)
> + prod->start(prod);
> +}
> +
> +/**
> + * irq_bypass_register_producer - register IRQ bypass producer
> + * @producer: pointer to producer structure
> + *
> + * Add the provided IRQ producer to the list of producers and connect
> + * with any matching token found on the IRQ consumers list.
> + */
> +int irq_bypass_register_producer(struct irq_bypass_producer *producer)
> +{
> + struct irq_bypass_producer *tmp;
> + struct irq_bypass_consumer *consumer;
> + int ret = 0;
> +
> + if (!producer->add_consumer || !producer->del_consumer)
> + return -EINVAL;
> +
> + might_sleep();
> +
> + if (!try_module_get(THIS_MODULE))
> + return -ENODEV;
> +
> + mutex_lock(&lock);
> +
> + list_for_each_entry(tmp, &producers, node) {
> + if (tmp->token == producer->token) {
> + mutex_unlock(&lock);
> + module_put(THIS_MODULE);
> + return -EINVAL;
> + }
> + }
> +
> + list_for_each_entry(consumer, &consumers, node) {
> + if (consumer->token == producer->token) {
> + ret = __connect(producer, consumer);
> + break;
> + }
> + }
> +
> + if (!ret)
> + list_add(&producer->node, &producers);
> +
> + mutex_unlock(&lock);
> +
> + if (ret)
> + module_put(THIS_MODULE);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(irq_bypass_register_producer);
> +
> +/**
> + * irq_bypass_unregister_producer - unregister IRQ bypass producer
> + * @producer: pointer to producer structure
> + *
> + * Remove a previously registered IRQ producer from the list of producers
> + * and disconnect it from any connected IRQ consumer.
> + */
> +void irq_bypass_unregister_producer(struct irq_bypass_producer *producer)
> +{
> + struct irq_bypass_producer *tmp;
> + struct irq_bypass_consumer *consumer;
> +
> + might_sleep();
> +
> + if (!try_module_get(THIS_MODULE))
> + return; /* nothing in the list anyway */
> +
> + mutex_lock(&lock);
> +
> + list_for_each_entry(tmp, &producers, node) {
> + if (tmp->token == producer->token)
> + break;
> + }
> +
> + if (tmp) {
> + list_for_each_entry(consumer, &consumers, node) {
> + if (consumer->token == producer->token) {
> + __disconnect(producer, consumer);
> + break;
> + }
> + }
> +
> + list_del(&producer->node);
> + }
> +
> + mutex_unlock(&lock);
> +
> + module_put(THIS_MODULE);
> + if (tmp)
> + module_put(THIS_MODULE);
> +}
> +EXPORT_SYMBOL_GPL(irq_bypass_unregister_producer);
> +
> +/**
> + * irq_bypass_register_consumer - register IRQ bypass consumer
> + * @consumer: pointer to consumer structure
> + *
> + * Add the provided IRQ consumer to the list of consumers and connect
> + * with any matching token found on the IRQ producer list.
> + */
> +int irq_bypass_register_consumer(struct irq_bypass_consumer *consumer)
> +{
> + struct irq_bypass_consumer *tmp;
> + struct irq_bypass_producer *producer;
> + int ret = 0;
> +
> + if (!consumer->add_producer || !consumer->del_producer)
> + return -EINVAL;
> +
> + might_sleep();
> +
> + if (!try_module_get(THIS_MODULE))
> + return -ENODEV;
> +
> + mutex_lock(&lock);
> +
> + list_for_each_entry(tmp, &consumers, node) {
> + if (tmp->token == consumer->token) {
> + mutex_unlock(&lock);
> + module_put(THIS_MODULE);
> + return -EINVAL;
> + }
> + }
> +
> + list_for_each_entry(producer, &producers, node) {
> + if (producer->token == consumer->token) {
> + ret = __connect(producer, consumer);
> + break;
> + }
> + }
> +
> + if (!ret)
> + list_add(&consumer->node, &consumers);
> +
> + mutex_unlock(&lock);
> +
> + if (ret)
> + module_put(THIS_MODULE);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(irq_bypass_register_consumer);
> +
> +/**
> + * irq_bypass_unregister_consumer - unregister IRQ bypass consumer
> + * @consumer: pointer to consumer structure
> + *
> + * Remove a previously registered IRQ consumer from the list of consumers
> + * and disconnect it from any connected IRQ producer.
> + */
> +void irq_bypass_unregister_consumer(struct irq_bypass_consumer *consumer)
> +{
> + struct irq_bypass_consumer *tmp;
> + struct irq_bypass_producer *producer;
> +
> + might_sleep();
> +
> + if (!try_module_get(THIS_MODULE))
> + return; /* nothing in the list anyway */
> +
> + mutex_lock(&lock);
> +
> + list_for_each_entry(tmp, &consumers, node) {
> + if (tmp->token == consumer->token)
> + break;
> + }
> +
> + if (tmp) {
> + list_for_each_entry(producer, &producers, node) {
> + if (producer->token == consumer->token) {
> + __disconnect(producer, consumer);
> + break;
> + }
> + }
> +
> + list_del(&consumer->node);
> + }
> +
> + mutex_unlock(&lock);
> +
> + module_put(THIS_MODULE);
> + if (tmp)
> + module_put(THIS_MODULE);
> +}
> +EXPORT_SYMBOL_GPL(irq_bypass_unregister_consumer);
>
> --
> 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
>

2015-07-18 18:29:40

by Eric Auger

[permalink] [raw]
Subject: Re: [PATCH v2] virt: IRQ bypass manager

Hi Alex,
On 07/16/2015 11:26 PM, Alex Williamson wrote:
> When a physical I/O device is assigned to a virtual machine through
> facilities like VFIO and KVM, the interrupt for the device generally
> bounces through the host system before being injected into the VM.
> However, hardware technologies exist that often allow the host to be
> bypassed for some of these scenarios. Intel Posted Interrupts allow
> the specified physical edge interrupts to be directly injected into a
> guest when delivered to a physical processor while the vCPU is
> running. ARM IRQ Forwarding allows forwarded physical interrupts to
> be directly deactivated by the guest.
>
> The IRQ bypass manager here is meant to provide the shim to connect
> interrupt producers, generally the host physical device driver, with
> interrupt consumers, generally the hypervisor, in order to configure
> these bypass mechanism. To do this, we base the connection on a
> shared, opaque token. For KVM-VFIO this is expected to be an
> eventfd_ctx since this is the connection we already use to connect an
> eventfd to an irqfd on the in-kernel path. When a producer and
> consumer with matching tokens is found, callbacks via both registered
> participants allow the bypass facilities to be automatically enabled.
>
> Signed-off-by: Alex Williamson <[email protected]>
> ---
>
> I'm not sure if we're settled on everything, I think Eric is out of
> the office and we haven't really resolved the opaque data element.
> I think it has problems because it assumes which side gets to provide
> the data and implies that the other side knows that the data is
> compatible. We also don't need to pass it as a callback argument if
> we put it on the consumer/produce struct and consider it public.
> However, I'm posting this to record the current state of this code
> and to make sure I'm not the bottleneck.
Yes please apologize for this silence. I have a limited access to my emails.

I agree with you on the fact the opaque data suggestion has some flaws,
you already pointed out. I think it is worth investigating your proposal
of not vfio-masking the forwarded interrupt when unforwarding. As of now
I am not 100% sure it will work since desynchronization between the VFIO
state and the virtual GIC state is introduced. guest is likely to
deactivate the IRQ, causing the resamplefd to unmask the non-masked IRQ
(leads to some kernel warnings although not a serious problem). If a new
physical non forwarded IRQ occurs, the deactivation of the n-1th
previously forwarded IRQ can unmask it, ... So I need to analyze all
those wicked cases.

But this v2 helps fixing the forward case and that's definitively good!

Thanks

Eric
>
> v2 Makes the rest of the changes suggested by Eric, various comment
> fixes, error paths for the add callbacks, and adds a MAINTAINERS
> entry for virt/lib. Thanks,
>
> Alex
>
> MAINTAINERS | 7 +
> include/linux/irqbypass.h | 90 ++++++++++++++++
> virt/lib/Kconfig | 2
> virt/lib/Makefile | 1
> virt/lib/irqbypass.c | 260 +++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 360 insertions(+)
> create mode 100644 include/linux/irqbypass.h
> create mode 100644 virt/lib/Kconfig
> create mode 100644 virt/lib/Makefile
> create mode 100644 virt/lib/irqbypass.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d8afd29..8e728cb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10613,6 +10613,13 @@ L: [email protected]
> S: Maintained
> F: drivers/net/ethernet/via/via-velocity.*
>
> +VIRT LIB
> +M: Alex Williamson <[email protected]>
> +M: Paolo Bonzini <[email protected]>
> +L: [email protected]
> +S: Supported
> +F: virt/lib/
> +
> VIVID VIRTUAL VIDEO DRIVER
> M: Hans Verkuil <[email protected]>
> L: [email protected]
> diff --git a/include/linux/irqbypass.h b/include/linux/irqbypass.h
> new file mode 100644
> index 0000000..fde7b64
> --- /dev/null
> +++ b/include/linux/irqbypass.h
> @@ -0,0 +1,90 @@
> +/*
> + * IRQ offload/bypass manager
> + *
> + * Copyright (C) 2015 Red Hat, Inc.
> + * Copyright (c) 2015 Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#ifndef IRQBYPASS_H
> +#define IRQBYPASS_H
> +
> +#include <linux/list.h>
> +
> +struct irq_bypass_consumer;
> +
> +/*
> + * Theory of operation
> + *
> + * The IRQ bypass manager is a simple set of lists and callbacks that allows
> + * IRQ producers (ex. physical interrupt sources) to be matched to IRQ
> + * consumers (ex. virtualization hardware that allows IRQ bypass or offload)
> + * via a shared token (ex. eventfd_ctx). Producers and consumers register
> + * independently. When a token match is found, the optional @stop callback
> + * will be called for each participant. The pair will then be connected via
> + * the @add_* callbacks, and finally the optional @start callback will allow
> + * any final coordination. When either participant is unregistered, the
> + * process is repeated using the @del_* callbacks in place of the @add_*
> + * callbacks. Match tokens must be unique per producer/consumer, 1:N pairings
> + * are not supported.
> + */
> +
> +/**
> + * struct irq_bypass_producer - IRQ bypass producer definition
> + * @node: IRQ bypass manager private list management
> + * @token: opaque token to match between producer and consumer
> + * @irq: Linux IRQ number for the producer device
> + * @add_consumer: Connect the IRQ producer to an IRQ consumer
> + * @del_consumer: Disconnect the IRQ producer from an IRQ consumer
> + * @stop: Perform any quiesce operations necessary prior to add/del (optional)
> + * @start: Perform any startup operations necessary after add/del (optional)
> + *
> + * The IRQ bypass producer structure represents an interrupt source for
> + * participation in possible host bypass, for instance an interrupt vector
> + * for a physical device assigned to a VM.
> + */
> +struct irq_bypass_producer {
> + struct list_head node;
> + void *token;
> + int irq;
> + int (*add_consumer)(struct irq_bypass_producer *,
> + struct irq_bypass_consumer *);
> + void (*del_consumer)(struct irq_bypass_producer *,
> + struct irq_bypass_consumer *);
> + void (*stop)(struct irq_bypass_producer *);
> + void (*start)(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
> + * @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)
> + * @start: Perform any startup operations necessary after add/del (optional)
> + *
> + * The IRQ bypass consumer structure represents an interrupt sink for
> + * participation in possible host bypass, for instance a hypervisor may
> + * support offloads to allow bypassing the host entirely or offload
> + * portions of the interrupt handling to the VM.
> + */
> +struct irq_bypass_consumer {
> + struct list_head node;
> + void *token;
> + int (*add_producer)(struct irq_bypass_consumer *,
> + struct irq_bypass_producer *);
> + void (*del_producer)(struct irq_bypass_consumer *,
> + struct irq_bypass_producer *);
> + void (*stop)(struct irq_bypass_consumer *);
> + void (*start)(struct irq_bypass_consumer *);
> +};
> +
> +int irq_bypass_register_producer(struct irq_bypass_producer *);
> +void irq_bypass_unregister_producer(struct irq_bypass_producer *);
> +int irq_bypass_register_consumer(struct irq_bypass_consumer *);
> +void irq_bypass_unregister_consumer(struct irq_bypass_consumer *);
> +
> +#endif /* IRQBYPASS_H */
> diff --git a/virt/lib/Kconfig b/virt/lib/Kconfig
> new file mode 100644
> index 0000000..89a414f
> --- /dev/null
> +++ b/virt/lib/Kconfig
> @@ -0,0 +1,2 @@
> +config IRQ_BYPASS_MANAGER
> + tristate
> diff --git a/virt/lib/Makefile b/virt/lib/Makefile
> new file mode 100644
> index 0000000..901228d
> --- /dev/null
> +++ b/virt/lib/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_IRQ_BYPASS_MANAGER) += irqbypass.o
> diff --git a/virt/lib/irqbypass.c b/virt/lib/irqbypass.c
> new file mode 100644
> index 0000000..6ae8e0f
> --- /dev/null
> +++ b/virt/lib/irqbypass.c
> @@ -0,0 +1,260 @@
> +/*
> + * IRQ offload/bypass manager
> + *
> + * Copyright (C) 2015 Red Hat, Inc.
> + * Copyright (c) 2015 Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Various virtualization hardware acceleration techniques allow bypassing or
> + * offloading interrupts received from devices around the host kernel. Posted
> + * Interrupts on Intel VT-d systems can allow interrupts to be received
> + * directly by a virtual machine. ARM IRQ Forwarding allows forwarded physical
> + * interrupts to be directly deactivated by the guest. This manager allows
> + * interrupt producers and consumers to find each other to enable this sort of
> + * bypass.
> + */
> +
> +#include <linux/irqbypass.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("IRQ bypass manager utility module");
> +
> +static LIST_HEAD(producers);
> +static LIST_HEAD(consumers);
> +static DEFINE_MUTEX(lock);
> +
> +/* @lock must be held when calling connect */
> +static int __connect(struct irq_bypass_producer *prod,
> + struct irq_bypass_consumer *cons)
> +{
> + int ret;
> +
> + if (prod->stop)
> + prod->stop(prod);
> + if (cons->stop)
> + cons->stop(cons);
> +
> + ret = prod->add_consumer(prod, cons);
> + if (!ret) {
> + ret = cons->add_producer(cons, prod);
> + if (ret)
> + prod->del_consumer(prod, cons);
> + }
> +
> + if (cons->start)
> + cons->start(cons);
> + if (prod->start)
> + prod->start(prod);
> +
> + return ret;
> +}
> +
> +/* @lock must be held when calling disconnect */
> +static void __disconnect(struct irq_bypass_producer *prod,
> + struct irq_bypass_consumer *cons)
> +{
> + if (prod->stop)
> + prod->stop(prod);
> + if (cons->stop)
> + cons->stop(cons);
> +
> + cons->del_producer(cons, prod);
> + prod->del_consumer(prod, cons);
> +
> + if (cons->start)
> + cons->start(cons);
> + if (prod->start)
> + prod->start(prod);
> +}
> +
> +/**
> + * irq_bypass_register_producer - register IRQ bypass producer
> + * @producer: pointer to producer structure
> + *
> + * Add the provided IRQ producer to the list of producers and connect
> + * with any matching token found on the IRQ consumers list.
> + */
> +int irq_bypass_register_producer(struct irq_bypass_producer *producer)
> +{
> + struct irq_bypass_producer *tmp;
> + struct irq_bypass_consumer *consumer;
> + int ret = 0;
> +
> + if (!producer->add_consumer || !producer->del_consumer)
> + return -EINVAL;
> +
> + might_sleep();
> +
> + if (!try_module_get(THIS_MODULE))
> + return -ENODEV;
> +
> + mutex_lock(&lock);
> +
> + list_for_each_entry(tmp, &producers, node) {
> + if (tmp->token == producer->token) {
> + mutex_unlock(&lock);
> + module_put(THIS_MODULE);
> + return -EINVAL;
> + }
> + }
> +
> + list_for_each_entry(consumer, &consumers, node) {
> + if (consumer->token == producer->token) {
> + ret = __connect(producer, consumer);
> + break;
> + }
> + }
> +
> + if (!ret)
> + list_add(&producer->node, &producers);
> +
> + mutex_unlock(&lock);
> +
> + if (ret)
> + module_put(THIS_MODULE);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(irq_bypass_register_producer);
> +
> +/**
> + * irq_bypass_unregister_producer - unregister IRQ bypass producer
> + * @producer: pointer to producer structure
> + *
> + * Remove a previously registered IRQ producer from the list of producers
> + * and disconnect it from any connected IRQ consumer.
> + */
> +void irq_bypass_unregister_producer(struct irq_bypass_producer *producer)
> +{
> + struct irq_bypass_producer *tmp;
> + struct irq_bypass_consumer *consumer;
> +
> + might_sleep();
> +
> + if (!try_module_get(THIS_MODULE))
> + return; /* nothing in the list anyway */
> +
> + mutex_lock(&lock);
> +
> + list_for_each_entry(tmp, &producers, node) {
> + if (tmp->token == producer->token)
> + break;
> + }
> +
> + if (tmp) {
> + list_for_each_entry(consumer, &consumers, node) {
> + if (consumer->token == producer->token) {
> + __disconnect(producer, consumer);
> + break;
> + }
> + }
> +
> + list_del(&producer->node);
> + }
> +
> + mutex_unlock(&lock);
> +
> + module_put(THIS_MODULE);
> + if (tmp)
> + module_put(THIS_MODULE);
> +}
> +EXPORT_SYMBOL_GPL(irq_bypass_unregister_producer);
> +
> +/**
> + * irq_bypass_register_consumer - register IRQ bypass consumer
> + * @consumer: pointer to consumer structure
> + *
> + * Add the provided IRQ consumer to the list of consumers and connect
> + * with any matching token found on the IRQ producer list.
> + */
> +int irq_bypass_register_consumer(struct irq_bypass_consumer *consumer)
> +{
> + struct irq_bypass_consumer *tmp;
> + struct irq_bypass_producer *producer;
> + int ret = 0;
> +
> + if (!consumer->add_producer || !consumer->del_producer)
> + return -EINVAL;
> +
> + might_sleep();
> +
> + if (!try_module_get(THIS_MODULE))
> + return -ENODEV;
> +
> + mutex_lock(&lock);
> +
> + list_for_each_entry(tmp, &consumers, node) {
> + if (tmp->token == consumer->token) {
> + mutex_unlock(&lock);
> + module_put(THIS_MODULE);
> + return -EINVAL;
> + }
> + }
> +
> + list_for_each_entry(producer, &producers, node) {
> + if (producer->token == consumer->token) {
> + ret = __connect(producer, consumer);
> + break;
> + }
> + }
> +
> + if (!ret)
> + list_add(&consumer->node, &consumers);
> +
> + mutex_unlock(&lock);
> +
> + if (ret)
> + module_put(THIS_MODULE);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(irq_bypass_register_consumer);
> +
> +/**
> + * irq_bypass_unregister_consumer - unregister IRQ bypass consumer
> + * @consumer: pointer to consumer structure
> + *
> + * Remove a previously registered IRQ consumer from the list of consumers
> + * and disconnect it from any connected IRQ producer.
> + */
> +void irq_bypass_unregister_consumer(struct irq_bypass_consumer *consumer)
> +{
> + struct irq_bypass_consumer *tmp;
> + struct irq_bypass_producer *producer;
> +
> + might_sleep();
> +
> + if (!try_module_get(THIS_MODULE))
> + return; /* nothing in the list anyway */
> +
> + mutex_lock(&lock);
> +
> + list_for_each_entry(tmp, &consumers, node) {
> + if (tmp->token == consumer->token)
> + break;
> + }
> +
> + if (tmp) {
> + list_for_each_entry(producer, &producers, node) {
> + if (producer->token == consumer->token) {
> + __disconnect(producer, consumer);
> + break;
> + }
> + }
> +
> + list_del(&consumer->node);
> + }
> +
> + mutex_unlock(&lock);
> +
> + module_put(THIS_MODULE);
> + if (tmp)
> + module_put(THIS_MODULE);
> +}
> +EXPORT_SYMBOL_GPL(irq_bypass_unregister_consumer);
>

2015-08-05 13:25:20

by Eric Auger

[permalink] [raw]
Subject: Re: [PATCH v2] virt: IRQ bypass manager

Hi Alex,
On 07/16/2015 11:26 PM, Alex Williamson wrote:
> When a physical I/O device is assigned to a virtual machine through
> facilities like VFIO and KVM, the interrupt for the device generally
> bounces through the host system before being injected into the VM.
> However, hardware technologies exist that often allow the host to be
> bypassed for some of these scenarios. Intel Posted Interrupts allow
> the specified physical edge interrupts to be directly injected into a
> guest when delivered to a physical processor while the vCPU is
> running. ARM IRQ Forwarding allows forwarded physical interrupts to
> be directly deactivated by the guest.
>
> The IRQ bypass manager here is meant to provide the shim to connect
> interrupt producers, generally the host physical device driver, with
> interrupt consumers, generally the hypervisor, in order to configure
> these bypass mechanism. To do this, we base the connection on a
> shared, opaque token. For KVM-VFIO this is expected to be an
> eventfd_ctx since this is the connection we already use to connect an
> eventfd to an irqfd on the in-kernel path. When a producer and
> consumer with matching tokens is found, callbacks via both registered
> participants allow the bypass facilities to be automatically enabled.
>
> Signed-off-by: Alex Williamson <[email protected]>
> ---
>
> I'm not sure if we're settled on everything, I think Eric is out of
> the office and we haven't really resolved the opaque data element.
> I think it has problems because it assumes which side gets to provide
> the data and implies that the other side knows that the data is
> compatible. We also don't need to pass it as a callback argument if
> we put it on the consumer/produce struct and consider it public.
> However, I'm posting this to record the current state of this code
> and to make sure I'm not the bottleneck.
>
> v2 Makes the rest of the changes suggested by Eric, various comment
> fixes, error paths for the add callbacks, and adds a MAINTAINERS
> entry for virt/lib. Thanks,
>
> Alex
>
> MAINTAINERS | 7 +
> include/linux/irqbypass.h | 90 ++++++++++++++++
> virt/lib/Kconfig | 2
> virt/lib/Makefile | 1
> virt/lib/irqbypass.c | 260 +++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 360 insertions(+)
> create mode 100644 include/linux/irqbypass.h
> create mode 100644 virt/lib/Kconfig
> create mode 100644 virt/lib/Makefile
> create mode 100644 virt/lib/irqbypass.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d8afd29..8e728cb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10613,6 +10613,13 @@ L: [email protected]
> S: Maintained
> F: drivers/net/ethernet/via/via-velocity.*
>
> +VIRT LIB
> +M: Alex Williamson <[email protected]>
> +M: Paolo Bonzini <[email protected]>
> +L: [email protected]
> +S: Supported
> +F: virt/lib/
> +
> VIVID VIRTUAL VIDEO DRIVER
> M: Hans Verkuil <[email protected]>
> L: [email protected]
> diff --git a/include/linux/irqbypass.h b/include/linux/irqbypass.h
> new file mode 100644
> index 0000000..fde7b64
> --- /dev/null
> +++ b/include/linux/irqbypass.h
> @@ -0,0 +1,90 @@
> +/*
> + * IRQ offload/bypass manager
> + *
> + * Copyright (C) 2015 Red Hat, Inc.
> + * Copyright (c) 2015 Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#ifndef IRQBYPASS_H
> +#define IRQBYPASS_H
> +
> +#include <linux/list.h>
> +
> +struct irq_bypass_consumer;
> +
> +/*
> + * Theory of operation
> + *
> + * The IRQ bypass manager is a simple set of lists and callbacks that allows
> + * IRQ producers (ex. physical interrupt sources) to be matched to IRQ
> + * consumers (ex. virtualization hardware that allows IRQ bypass or offload)
> + * via a shared token (ex. eventfd_ctx). Producers and consumers register
> + * independently. When a token match is found, the optional @stop callback
> + * will be called for each participant. The pair will then be connected via
> + * the @add_* callbacks, and finally the optional @start callback will allow
> + * any final coordination. When either participant is unregistered, the
> + * process is repeated using the @del_* callbacks in place of the @add_*
> + * callbacks. Match tokens must be unique per producer/consumer, 1:N pairings
> + * are not supported.
> + */
> +
> +/**
> + * struct irq_bypass_producer - IRQ bypass producer definition
> + * @node: IRQ bypass manager private list management
> + * @token: opaque token to match between producer and consumer
> + * @irq: Linux IRQ number for the producer device
> + * @add_consumer: Connect the IRQ producer to an IRQ consumer
> + * @del_consumer: Disconnect the IRQ producer from an IRQ consumer
> + * @stop: Perform any quiesce operations necessary prior to add/del (optional)
> + * @start: Perform any startup operations necessary after add/del (optional)
> + *
> + * The IRQ bypass producer structure represents an interrupt source for
> + * participation in possible host bypass, for instance an interrupt vector
> + * for a physical device assigned to a VM.
> + */
> +struct irq_bypass_producer {
> + struct list_head node;
> + void *token;
> + int irq;
> + int (*add_consumer)(struct irq_bypass_producer *,
> + struct irq_bypass_consumer *);
> + void (*del_consumer)(struct irq_bypass_producer *,
> + struct irq_bypass_consumer *);
> + void (*stop)(struct irq_bypass_producer *);
> + void (*start)(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
> + * @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)
> + * @start: Perform any startup operations necessary after add/del (optional)
> + *
> + * The IRQ bypass consumer structure represents an interrupt sink for
> + * participation in possible host bypass, for instance a hypervisor may
> + * support offloads to allow bypassing the host entirely or offload
> + * portions of the interrupt handling to the VM.
> + */
> +struct irq_bypass_consumer {
> + struct list_head node;
> + void *token;
> + int (*add_producer)(struct irq_bypass_consumer *,
> + struct irq_bypass_producer *);
> + void (*del_producer)(struct irq_bypass_consumer *,
> + struct irq_bypass_producer *);
> + void (*stop)(struct irq_bypass_consumer *);
> + void (*start)(struct irq_bypass_consumer *);
> +};
> +
> +int irq_bypass_register_producer(struct irq_bypass_producer *);
> +void irq_bypass_unregister_producer(struct irq_bypass_producer *);
> +int irq_bypass_register_consumer(struct irq_bypass_consumer *);
> +void irq_bypass_unregister_consumer(struct irq_bypass_consumer *);
> +
> +#endif /* IRQBYPASS_H */
> diff --git a/virt/lib/Kconfig b/virt/lib/Kconfig
> new file mode 100644
> index 0000000..89a414f
> --- /dev/null
> +++ b/virt/lib/Kconfig
> @@ -0,0 +1,2 @@
> +config IRQ_BYPASS_MANAGER
> + tristate
> diff --git a/virt/lib/Makefile b/virt/lib/Makefile
> new file mode 100644
> index 0000000..901228d
> --- /dev/null
> +++ b/virt/lib/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_IRQ_BYPASS_MANAGER) += irqbypass.o
> diff --git a/virt/lib/irqbypass.c b/virt/lib/irqbypass.c
> new file mode 100644
> index 0000000..6ae8e0f
> --- /dev/null
> +++ b/virt/lib/irqbypass.c
> @@ -0,0 +1,260 @@
> +/*
> + * IRQ offload/bypass manager
> + *
> + * Copyright (C) 2015 Red Hat, Inc.
> + * Copyright (c) 2015 Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Various virtualization hardware acceleration techniques allow bypassing or
> + * offloading interrupts received from devices around the host kernel. Posted
> + * Interrupts on Intel VT-d systems can allow interrupts to be received
> + * directly by a virtual machine. ARM IRQ Forwarding allows forwarded physical
> + * interrupts to be directly deactivated by the guest. This manager allows
> + * interrupt producers and consumers to find each other to enable this sort of
> + * bypass.
> + */
> +
> +#include <linux/irqbypass.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("IRQ bypass manager utility module");
> +
> +static LIST_HEAD(producers);
> +static LIST_HEAD(consumers);
> +static DEFINE_MUTEX(lock);
> +
> +/* @lock must be held when calling connect */
> +static int __connect(struct irq_bypass_producer *prod,
> + struct irq_bypass_consumer *cons)
> +{
> + int ret;
> +
> + if (prod->stop)
> + prod->stop(prod);
> + if (cons->stop)
> + cons->stop(cons);
> +
> + ret = prod->add_consumer(prod, cons);
> + if (!ret) {
> + ret = cons->add_producer(cons, prod);
> + if (ret)
> + prod->del_consumer(prod, cons);
> + }
> +
> + if (cons->start)
> + cons->start(cons);
> + if (prod->start)
> + prod->start(prod);
> +
> + return ret;
> +}
> +
> +/* @lock must be held when calling disconnect */
> +static void __disconnect(struct irq_bypass_producer *prod,
> + struct irq_bypass_consumer *cons)
> +{
> + if (prod->stop)
> + prod->stop(prod);
> + if (cons->stop)
> + cons->stop(cons);
> +
> + cons->del_producer(cons, prod);
> + prod->del_consumer(prod, cons);
> +
> + if (cons->start)
> + cons->start(cons);
> + if (prod->start)
> + prod->start(prod);
> +}
> +
> +/**
> + * irq_bypass_register_producer - register IRQ bypass producer
> + * @producer: pointer to producer structure
> + *
> + * Add the provided IRQ producer to the list of producers and connect
> + * with any matching token found on the IRQ consumers list.
> + */
> +int irq_bypass_register_producer(struct irq_bypass_producer *producer)
> +{
> + struct irq_bypass_producer *tmp;
> + struct irq_bypass_consumer *consumer;
> + int ret = 0;
> +
> + if (!producer->add_consumer || !producer->del_consumer)
> + return -EINVAL;
> +
> + might_sleep();
> +
> + if (!try_module_get(THIS_MODULE))
> + return -ENODEV;
> +
> + mutex_lock(&lock);
> +
> + list_for_each_entry(tmp, &producers, node) {
> + if (tmp->token == producer->token) {
> + mutex_unlock(&lock);
> + module_put(THIS_MODULE);
> + return -EINVAL;
> + }
> + }
> +
> + list_for_each_entry(consumer, &consumers, node) {
> + if (consumer->token == producer->token) {
> + ret = __connect(producer, consumer);
> + break;
> + }
> + }
> +
> + if (!ret)
> + list_add(&producer->node, &producers);
> +
> + mutex_unlock(&lock);
> +
> + if (ret)
> + module_put(THIS_MODULE);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(irq_bypass_register_producer);
> +
> +/**
> + * irq_bypass_unregister_producer - unregister IRQ bypass producer
> + * @producer: pointer to producer structure
> + *
> + * Remove a previously registered IRQ producer from the list of producers
> + * and disconnect it from any connected IRQ consumer.
> + */
> +void irq_bypass_unregister_producer(struct irq_bypass_producer *producer)
> +{
> + struct irq_bypass_producer *tmp;
> + struct irq_bypass_consumer *consumer;
> +
> + might_sleep();
> +
> + if (!try_module_get(THIS_MODULE))
> + return; /* nothing in the list anyway */
> +
> + mutex_lock(&lock);
> +
> + list_for_each_entry(tmp, &producers, node) {
> + if (tmp->token == producer->token)
> + break;
> + }
> +
> + if (tmp) {
> + list_for_each_entry(consumer, &consumers, node) {
> + if (consumer->token == producer->token) {
> + __disconnect(producer, consumer);
> + break;
> + }
> + }
> +
> + list_del(&producer->node);
> + }
> +
> + mutex_unlock(&lock);
> +
> + module_put(THIS_MODULE);
> + if (tmp)
> + module_put(THIS_MODULE);
> +}
> +EXPORT_SYMBOL_GPL(irq_bypass_unregister_producer);
> +
> +/**
> + * irq_bypass_register_consumer - register IRQ bypass consumer
> + * @consumer: pointer to consumer structure
> + *
> + * Add the provided IRQ consumer to the list of consumers and connect
> + * with any matching token found on the IRQ producer list.
> + */
> +int irq_bypass_register_consumer(struct irq_bypass_consumer *consumer)
> +{
> + struct irq_bypass_consumer *tmp;
> + struct irq_bypass_producer *producer;
> + int ret = 0;
> +
> + if (!consumer->add_producer || !consumer->del_producer)
> + return -EINVAL;
> +
> + might_sleep();
> +
> + if (!try_module_get(THIS_MODULE))
> + return -ENODEV;
> +
> + mutex_lock(&lock);
> +
> + list_for_each_entry(tmp, &consumers, node) {
> + if (tmp->token == consumer->token) {
> + mutex_unlock(&lock);
> + module_put(THIS_MODULE);
> + return -EINVAL;
> + }
> + }
> +
> + list_for_each_entry(producer, &producers, node) {
> + if (producer->token == consumer->token) {
> + ret = __connect(producer, consumer);
> + break;
> + }
> + }
> +
> + if (!ret)
> + list_add(&consumer->node, &consumers);
> +
> + mutex_unlock(&lock);
> +
> + if (ret)
> + module_put(THIS_MODULE);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(irq_bypass_register_consumer);
> +
> +/**
> + * irq_bypass_unregister_consumer - unregister IRQ bypass consumer
> + * @consumer: pointer to consumer structure
> + *
> + * Remove a previously registered IRQ consumer from the list of consumers
> + * and disconnect it from any connected IRQ producer.
> + */
> +void irq_bypass_unregister_consumer(struct irq_bypass_consumer *consumer)
> +{
> + struct irq_bypass_consumer *tmp;
> + struct irq_bypass_producer *producer;
> +
> + might_sleep();
> +
> + if (!try_module_get(THIS_MODULE))
> + return; /* nothing in the list anyway */
> +
> + mutex_lock(&lock);
> +
> + list_for_each_entry(tmp, &consumers, node) {
> + if (tmp->token == consumer->token)
> + break;
> + }
> +
> + if (tmp) {
I think this does not work anymore now the consumer is not necessarily
part of the consumers list (in case of registration failure).

Best Regards

Eric
> + list_for_each_entry(producer, &producers, node) {
> + if (producer->token == consumer->token) {
> + __disconnect(producer, consumer);
> + break;
> + }
> + }
> +
> + list_del(&consumer->node);
> + }
> +
> + mutex_unlock(&lock);
> +
> + module_put(THIS_MODULE);
> + if (tmp)
> + module_put(THIS_MODULE);
> +}
> +EXPORT_SYMBOL_GPL(irq_bypass_unregister_consumer);
>

2015-08-05 16:29:14

by Alex Williamson

[permalink] [raw]
Subject: Re: [PATCH v2] virt: IRQ bypass manager

On Wed, 2015-08-05 at 15:23 +0200, Eric Auger wrote:
> Hi Alex,
> On 07/16/2015 11:26 PM, Alex Williamson wrote:
> > When a physical I/O device is assigned to a virtual machine through
> > facilities like VFIO and KVM, the interrupt for the device generally
> > bounces through the host system before being injected into the VM.
> > However, hardware technologies exist that often allow the host to be
> > bypassed for some of these scenarios. Intel Posted Interrupts allow
> > the specified physical edge interrupts to be directly injected into a
> > guest when delivered to a physical processor while the vCPU is
> > running. ARM IRQ Forwarding allows forwarded physical interrupts to
> > be directly deactivated by the guest.
> >
> > The IRQ bypass manager here is meant to provide the shim to connect
> > interrupt producers, generally the host physical device driver, with
> > interrupt consumers, generally the hypervisor, in order to configure
> > these bypass mechanism. To do this, we base the connection on a
> > shared, opaque token. For KVM-VFIO this is expected to be an
> > eventfd_ctx since this is the connection we already use to connect an
> > eventfd to an irqfd on the in-kernel path. When a producer and
> > consumer with matching tokens is found, callbacks via both registered
> > participants allow the bypass facilities to be automatically enabled.
> >
> > Signed-off-by: Alex Williamson <[email protected]>
> > ---
> >
> > I'm not sure if we're settled on everything, I think Eric is out of
> > the office and we haven't really resolved the opaque data element.
> > I think it has problems because it assumes which side gets to provide
> > the data and implies that the other side knows that the data is
> > compatible. We also don't need to pass it as a callback argument if
> > we put it on the consumer/produce struct and consider it public.
> > However, I'm posting this to record the current state of this code
> > and to make sure I'm not the bottleneck.
> >
> > v2 Makes the rest of the changes suggested by Eric, various comment
> > fixes, error paths for the add callbacks, and adds a MAINTAINERS
> > entry for virt/lib. Thanks,
> >
> > Alex
> >
> > MAINTAINERS | 7 +
> > include/linux/irqbypass.h | 90 ++++++++++++++++
> > virt/lib/Kconfig | 2
> > virt/lib/Makefile | 1
> > virt/lib/irqbypass.c | 260 +++++++++++++++++++++++++++++++++++++++++++++
> > 5 files changed, 360 insertions(+)
> > create mode 100644 include/linux/irqbypass.h
> > create mode 100644 virt/lib/Kconfig
> > create mode 100644 virt/lib/Makefile
> > create mode 100644 virt/lib/irqbypass.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index d8afd29..8e728cb 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -10613,6 +10613,13 @@ L: [email protected]
> > S: Maintained
> > F: drivers/net/ethernet/via/via-velocity.*
> >
> > +VIRT LIB
> > +M: Alex Williamson <[email protected]>
> > +M: Paolo Bonzini <[email protected]>
> > +L: [email protected]
> > +S: Supported
> > +F: virt/lib/
> > +
> > VIVID VIRTUAL VIDEO DRIVER
> > M: Hans Verkuil <[email protected]>
> > L: [email protected]
> > diff --git a/include/linux/irqbypass.h b/include/linux/irqbypass.h
> > new file mode 100644
> > index 0000000..fde7b64
> > --- /dev/null
> > +++ b/include/linux/irqbypass.h
> > @@ -0,0 +1,90 @@
> > +/*
> > + * IRQ offload/bypass manager
> > + *
> > + * Copyright (C) 2015 Red Hat, Inc.
> > + * Copyright (c) 2015 Linaro Ltd.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +#ifndef IRQBYPASS_H
> > +#define IRQBYPASS_H
> > +
> > +#include <linux/list.h>
> > +
> > +struct irq_bypass_consumer;
> > +
> > +/*
> > + * Theory of operation
> > + *
> > + * The IRQ bypass manager is a simple set of lists and callbacks that allows
> > + * IRQ producers (ex. physical interrupt sources) to be matched to IRQ
> > + * consumers (ex. virtualization hardware that allows IRQ bypass or offload)
> > + * via a shared token (ex. eventfd_ctx). Producers and consumers register
> > + * independently. When a token match is found, the optional @stop callback
> > + * will be called for each participant. The pair will then be connected via
> > + * the @add_* callbacks, and finally the optional @start callback will allow
> > + * any final coordination. When either participant is unregistered, the
> > + * process is repeated using the @del_* callbacks in place of the @add_*
> > + * callbacks. Match tokens must be unique per producer/consumer, 1:N pairings
> > + * are not supported.
> > + */
> > +
> > +/**
> > + * struct irq_bypass_producer - IRQ bypass producer definition
> > + * @node: IRQ bypass manager private list management
> > + * @token: opaque token to match between producer and consumer
> > + * @irq: Linux IRQ number for the producer device
> > + * @add_consumer: Connect the IRQ producer to an IRQ consumer
> > + * @del_consumer: Disconnect the IRQ producer from an IRQ consumer
> > + * @stop: Perform any quiesce operations necessary prior to add/del (optional)
> > + * @start: Perform any startup operations necessary after add/del (optional)
> > + *
> > + * The IRQ bypass producer structure represents an interrupt source for
> > + * participation in possible host bypass, for instance an interrupt vector
> > + * for a physical device assigned to a VM.
> > + */
> > +struct irq_bypass_producer {
> > + struct list_head node;
> > + void *token;
> > + int irq;
> > + int (*add_consumer)(struct irq_bypass_producer *,
> > + struct irq_bypass_consumer *);
> > + void (*del_consumer)(struct irq_bypass_producer *,
> > + struct irq_bypass_consumer *);
> > + void (*stop)(struct irq_bypass_producer *);
> > + void (*start)(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
> > + * @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)
> > + * @start: Perform any startup operations necessary after add/del (optional)
> > + *
> > + * The IRQ bypass consumer structure represents an interrupt sink for
> > + * participation in possible host bypass, for instance a hypervisor may
> > + * support offloads to allow bypassing the host entirely or offload
> > + * portions of the interrupt handling to the VM.
> > + */
> > +struct irq_bypass_consumer {
> > + struct list_head node;
> > + void *token;
> > + int (*add_producer)(struct irq_bypass_consumer *,
> > + struct irq_bypass_producer *);
> > + void (*del_producer)(struct irq_bypass_consumer *,
> > + struct irq_bypass_producer *);
> > + void (*stop)(struct irq_bypass_consumer *);
> > + void (*start)(struct irq_bypass_consumer *);
> > +};
> > +
> > +int irq_bypass_register_producer(struct irq_bypass_producer *);
> > +void irq_bypass_unregister_producer(struct irq_bypass_producer *);
> > +int irq_bypass_register_consumer(struct irq_bypass_consumer *);
> > +void irq_bypass_unregister_consumer(struct irq_bypass_consumer *);
> > +
> > +#endif /* IRQBYPASS_H */
> > diff --git a/virt/lib/Kconfig b/virt/lib/Kconfig
> > new file mode 100644
> > index 0000000..89a414f
> > --- /dev/null
> > +++ b/virt/lib/Kconfig
> > @@ -0,0 +1,2 @@
> > +config IRQ_BYPASS_MANAGER
> > + tristate
> > diff --git a/virt/lib/Makefile b/virt/lib/Makefile
> > new file mode 100644
> > index 0000000..901228d
> > --- /dev/null
> > +++ b/virt/lib/Makefile
> > @@ -0,0 +1 @@
> > +obj-$(CONFIG_IRQ_BYPASS_MANAGER) += irqbypass.o
> > diff --git a/virt/lib/irqbypass.c b/virt/lib/irqbypass.c
> > new file mode 100644
> > index 0000000..6ae8e0f
> > --- /dev/null
> > +++ b/virt/lib/irqbypass.c
> > @@ -0,0 +1,260 @@
> > +/*
> > + * IRQ offload/bypass manager
> > + *
> > + * Copyright (C) 2015 Red Hat, Inc.
> > + * Copyright (c) 2015 Linaro Ltd.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * Various virtualization hardware acceleration techniques allow bypassing or
> > + * offloading interrupts received from devices around the host kernel. Posted
> > + * Interrupts on Intel VT-d systems can allow interrupts to be received
> > + * directly by a virtual machine. ARM IRQ Forwarding allows forwarded physical
> > + * interrupts to be directly deactivated by the guest. This manager allows
> > + * interrupt producers and consumers to find each other to enable this sort of
> > + * bypass.
> > + */
> > +
> > +#include <linux/irqbypass.h>
> > +#include <linux/list.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_DESCRIPTION("IRQ bypass manager utility module");
> > +
> > +static LIST_HEAD(producers);
> > +static LIST_HEAD(consumers);
> > +static DEFINE_MUTEX(lock);
> > +
> > +/* @lock must be held when calling connect */
> > +static int __connect(struct irq_bypass_producer *prod,
> > + struct irq_bypass_consumer *cons)
> > +{
> > + int ret;
> > +
> > + if (prod->stop)
> > + prod->stop(prod);
> > + if (cons->stop)
> > + cons->stop(cons);
> > +
> > + ret = prod->add_consumer(prod, cons);
> > + if (!ret) {
> > + ret = cons->add_producer(cons, prod);
> > + if (ret)
> > + prod->del_consumer(prod, cons);
> > + }
> > +
> > + if (cons->start)
> > + cons->start(cons);
> > + if (prod->start)
> > + prod->start(prod);
> > +
> > + return ret;
> > +}
> > +
> > +/* @lock must be held when calling disconnect */
> > +static void __disconnect(struct irq_bypass_producer *prod,
> > + struct irq_bypass_consumer *cons)
> > +{
> > + if (prod->stop)
> > + prod->stop(prod);
> > + if (cons->stop)
> > + cons->stop(cons);
> > +
> > + cons->del_producer(cons, prod);
> > + prod->del_consumer(prod, cons);
> > +
> > + if (cons->start)
> > + cons->start(cons);
> > + if (prod->start)
> > + prod->start(prod);
> > +}
> > +
> > +/**
> > + * irq_bypass_register_producer - register IRQ bypass producer
> > + * @producer: pointer to producer structure
> > + *
> > + * Add the provided IRQ producer to the list of producers and connect
> > + * with any matching token found on the IRQ consumers list.
> > + */
> > +int irq_bypass_register_producer(struct irq_bypass_producer *producer)
> > +{
> > + struct irq_bypass_producer *tmp;
> > + struct irq_bypass_consumer *consumer;
> > + int ret = 0;
> > +
> > + if (!producer->add_consumer || !producer->del_consumer)
> > + return -EINVAL;
> > +
> > + might_sleep();
> > +
> > + if (!try_module_get(THIS_MODULE))
> > + return -ENODEV;
> > +
> > + mutex_lock(&lock);
> > +
> > + list_for_each_entry(tmp, &producers, node) {
> > + if (tmp->token == producer->token) {
> > + mutex_unlock(&lock);
> > + module_put(THIS_MODULE);
> > + return -EINVAL;
> > + }
> > + }
> > +
> > + list_for_each_entry(consumer, &consumers, node) {
> > + if (consumer->token == producer->token) {
> > + ret = __connect(producer, consumer);
> > + break;
> > + }
> > + }
> > +
> > + if (!ret)
> > + list_add(&producer->node, &producers);
> > +
> > + mutex_unlock(&lock);
> > +
> > + if (ret)
> > + module_put(THIS_MODULE);
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(irq_bypass_register_producer);
> > +
> > +/**
> > + * irq_bypass_unregister_producer - unregister IRQ bypass producer
> > + * @producer: pointer to producer structure
> > + *
> > + * Remove a previously registered IRQ producer from the list of producers
> > + * and disconnect it from any connected IRQ consumer.
> > + */
> > +void irq_bypass_unregister_producer(struct irq_bypass_producer *producer)
> > +{
> > + struct irq_bypass_producer *tmp;
> > + struct irq_bypass_consumer *consumer;
> > +
> > + might_sleep();
> > +
> > + if (!try_module_get(THIS_MODULE))
> > + return; /* nothing in the list anyway */
> > +
> > + mutex_lock(&lock);
> > +
> > + list_for_each_entry(tmp, &producers, node) {
> > + if (tmp->token == producer->token)
> > + break;
> > + }
> > +
> > + if (tmp) {
> > + list_for_each_entry(consumer, &consumers, node) {
> > + if (consumer->token == producer->token) {
> > + __disconnect(producer, consumer);
> > + break;
> > + }
> > + }
> > +
> > + list_del(&producer->node);
> > + }
> > +
> > + mutex_unlock(&lock);
> > +
> > + module_put(THIS_MODULE);
> > + if (tmp)
> > + module_put(THIS_MODULE);
> > +}
> > +EXPORT_SYMBOL_GPL(irq_bypass_unregister_producer);
> > +
> > +/**
> > + * irq_bypass_register_consumer - register IRQ bypass consumer
> > + * @consumer: pointer to consumer structure
> > + *
> > + * Add the provided IRQ consumer to the list of consumers and connect
> > + * with any matching token found on the IRQ producer list.
> > + */
> > +int irq_bypass_register_consumer(struct irq_bypass_consumer *consumer)
> > +{
> > + struct irq_bypass_consumer *tmp;
> > + struct irq_bypass_producer *producer;
> > + int ret = 0;
> > +
> > + if (!consumer->add_producer || !consumer->del_producer)
> > + return -EINVAL;
> > +
> > + might_sleep();
> > +
> > + if (!try_module_get(THIS_MODULE))
> > + return -ENODEV;
> > +
> > + mutex_lock(&lock);
> > +
> > + list_for_each_entry(tmp, &consumers, node) {
> > + if (tmp->token == consumer->token) {
> > + mutex_unlock(&lock);
> > + module_put(THIS_MODULE);
> > + return -EINVAL;
> > + }
> > + }
> > +
> > + list_for_each_entry(producer, &producers, node) {
> > + if (producer->token == consumer->token) {
> > + ret = __connect(producer, consumer);
> > + break;
> > + }
> > + }
> > +
> > + if (!ret)
> > + list_add(&consumer->node, &consumers);
> > +
> > + mutex_unlock(&lock);
> > +
> > + if (ret)
> > + module_put(THIS_MODULE);
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(irq_bypass_register_consumer);
> > +
> > +/**
> > + * irq_bypass_unregister_consumer - unregister IRQ bypass consumer
> > + * @consumer: pointer to consumer structure
> > + *
> > + * Remove a previously registered IRQ consumer from the list of consumers
> > + * and disconnect it from any connected IRQ producer.
> > + */
> > +void irq_bypass_unregister_consumer(struct irq_bypass_consumer *consumer)
> > +{
> > + struct irq_bypass_consumer *tmp;
> > + struct irq_bypass_producer *producer;
> > +
> > + might_sleep();
> > +
> > + if (!try_module_get(THIS_MODULE))
> > + return; /* nothing in the list anyway */
> > +
> > + mutex_lock(&lock);
> > +
> > + list_for_each_entry(tmp, &consumers, node) {
> > + if (tmp->token == consumer->token)
> > + break;
> > + }
> > +
> > + if (tmp) {
> I think this does not work anymore now the consumer is not necessarily
> part of the consumers list (in case of registration failure).

Hi Eric,

Sorry, I'm not following. To have a robust interface we need to protect
against callers unregistering things that were never registered
regardless of the tweak to our internal API. If the register failed,
tmp should run the the list and be NULL here, so we'll simply unlock,
release the module reference and be done. What am I missing? Thanks,

Alex

> > + list_for_each_entry(producer, &producers, node) {
> > + if (producer->token == consumer->token) {
> > + __disconnect(producer, consumer);
> > + break;
> > + }
> > + }
> > +
> > + list_del(&consumer->node);
> > + }
> > +
> > + mutex_unlock(&lock);
> > +
> > + module_put(THIS_MODULE);
> > + if (tmp)
> > + module_put(THIS_MODULE);
> > +}
> > +EXPORT_SYMBOL_GPL(irq_bypass_unregister_consumer);
> >
>