2015-08-03 17:21:38

by Eric Auger

[permalink] [raw]
Subject: [PATCH v4 0/5] KVM: irqfd consumer based on IRQ bypass manager

This series transforms irqfd into an IRQ bypass consumer and
introduce the infrastructure shared by Intel posted-interrupts
and ARM forwarded IRQ series.

The bypass manager gets compiled for x86 and arm/arm64 when
KVM is used. A new kvm_irqfd.h header is created to externalize
some irqfd declarations going to be used by architecture specific
code. A new CONFIG_HAVE_KVM_IRQ_BYPASS option is introduced
to enable architecture specific IRQ manager hooks.

the series depends on the IRQ bypass manager:
- [PATCH v2] virt: IRQ bypass manager
(https://lkml.org/lkml/2015/7/16/810)

can be found at:
https://git.linaro.org/people/eric.auger/linux.git/shortlog/refs/heads/v4.2-rc5-irqfd-consumer-v4

Best Regards

Eric

History:

v3 -> v4:
- fix compilation for arm/arm64
- rearrange signoffs/history on some patch files

v2 -> v3 (Feng Wu):
- rebase on IRQ bypass manager [PATCH v2] virt: IRQ bypass manager:
https://lkml.org/lkml/2015/7/16/810
- Correct some minor errors and typo
- Add something needed for posted-interrupts

v1 -> v2:
- isolate the bypass manager and irqfd consumer in this series
- take into account Paolo's comments and use container_of strategy and
remove additional fields introduced in v1.
- create kvm_irqfd.h
- add unregistration in irqfd_shutdown

v1: originally part of [RFC 00/17] ARM IRQ forward control based on IRQ
bypass manager (https://lkml.org/lkml/2015/7/2/268)


Eric Auger (4):
KVM: arm/arm64: select IRQ_BYPASS_MANAGER
KVM: create kvm_irqfd.h
KVM: introduce kvm_arch functions for IRQ bypass
KVM: eventfd: add irq bypass consumer management

Feng Wu (1):
KVM: x86: select IRQ_BYPASS_MANAGER

arch/arm/kvm/Kconfig | 2 +
arch/arm/kvm/Makefile | 1 +
arch/arm64/kvm/Kconfig | 2 +
arch/arm64/kvm/Makefile | 1 +
arch/x86/kvm/Kconfig | 2 +
arch/x86/kvm/Makefile | 3 ++
include/linux/kvm_host.h | 33 +++++++++++++++
include/linux/kvm_irqfd.h | 71 +++++++++++++++++++++++++++++++
virt/kvm/Kconfig | 3 ++
virt/kvm/eventfd.c | 105 +++++++++++++++-------------------------------
10 files changed, 151 insertions(+), 72 deletions(-)
create mode 100644 include/linux/kvm_irqfd.h

--
1.9.1


2015-08-03 17:23:39

by Eric Auger

[permalink] [raw]
Subject: [PATCH v4 1/5] KVM: x86: select IRQ_BYPASS_MANAGER

From: Feng Wu <[email protected]>

Select IRQ_BYPASS_MANAGER for x86 when CONFIG_KVM is set

Signed-off-by: Feng Wu <[email protected]>
---
arch/x86/kvm/Kconfig | 2 ++
arch/x86/kvm/Makefile | 3 +++
2 files changed, 5 insertions(+)

diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index d8a1d56..c951d44 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -3,6 +3,7 @@
#

source "virt/kvm/Kconfig"
+source "virt/lib/Kconfig"

menuconfig VIRTUALIZATION
bool "Virtualization"
@@ -28,6 +29,7 @@ config KVM
select ANON_INODES
select HAVE_KVM_IRQCHIP
select HAVE_KVM_IRQFD
+ select IRQ_BYPASS_MANAGER
select HAVE_KVM_IRQ_ROUTING
select HAVE_KVM_EVENTFD
select KVM_APIC_ARCHITECTURE
diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
index 67d215c..05cc2d7 100644
--- a/arch/x86/kvm/Makefile
+++ b/arch/x86/kvm/Makefile
@@ -6,6 +6,9 @@ CFLAGS_svm.o := -I.
CFLAGS_vmx.o := -I.

KVM := ../../../virt/kvm
+LIB := ../../../virt/lib
+
+obj-$(CONFIG_IRQ_BYPASS_MANAGER) += $(LIB)/

kvm-y += $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o \
$(KVM)/eventfd.o $(KVM)/irqchip.o $(KVM)/vfio.o
--
1.9.1

2015-08-03 17:21:42

by Eric Auger

[permalink] [raw]
Subject: [PATCH v4 2/5] KVM: arm/arm64: select IRQ_BYPASS_MANAGER

Select IRQ_BYPASS_MANAGER when CONFIG_KVM is set
Also add compilation of virt/lib.

Signed-off-by: Eric Auger <[email protected]>
Signed-off-by: Feng Wu <[email protected]>

---

v3 -> v4:
- add compilation of virt/lib in arm/arm64 KVM

v2 -> v3:
- [Feng Wu] Correct a typo in 'arch/arm64/kvm/Kconfig'

v1 -> v2:
- also set IRQ_BYPASS_MANAGER for arm64
---
arch/arm/kvm/Kconfig | 2 ++
arch/arm/kvm/Makefile | 1 +
arch/arm64/kvm/Kconfig | 2 ++
arch/arm64/kvm/Makefile | 1 +
4 files changed, 6 insertions(+)

diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
index bfb915d..3c565b9 100644
--- a/arch/arm/kvm/Kconfig
+++ b/arch/arm/kvm/Kconfig
@@ -3,6 +3,7 @@
#

source "virt/kvm/Kconfig"
+source "virt/lib/Kconfig"

menuconfig VIRTUALIZATION
bool "Virtualization"
@@ -31,6 +32,7 @@ config KVM
select KVM_VFIO
select HAVE_KVM_EVENTFD
select HAVE_KVM_IRQFD
+ select IRQ_BYPASS_MANAGER
depends on ARM_VIRT_EXT && ARM_LPAE && ARM_ARCH_TIMER
---help---
Support hosting virtualized guest machines.
diff --git a/arch/arm/kvm/Makefile b/arch/arm/kvm/Makefile
index c5eef02c..a6a41dd 100644
--- a/arch/arm/kvm/Makefile
+++ b/arch/arm/kvm/Makefile
@@ -24,3 +24,4 @@ obj-y += $(KVM)/arm/vgic.o
obj-y += $(KVM)/arm/vgic-v2.o
obj-y += $(KVM)/arm/vgic-v2-emul.o
obj-y += $(KVM)/arm/arch_timer.o
+obj-y += ../../../virt/lib/
diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index bfffe8f..2509539 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -3,6 +3,7 @@
#

source "virt/kvm/Kconfig"
+source "virt/lib/Kconfig"

menuconfig VIRTUALIZATION
bool "Virtualization"
@@ -31,6 +32,7 @@ config KVM
select KVM_VFIO
select HAVE_KVM_EVENTFD
select HAVE_KVM_IRQFD
+ select IRQ_BYPASS_MANAGER
---help---
Support hosting virtualized guest machines.

diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index f90f4aa..55eec69 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -27,3 +27,4 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic-v3.o
kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic-v3-emul.o
kvm-$(CONFIG_KVM_ARM_HOST) += vgic-v3-switch.o
kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arch_timer.o
+kvm-$(CONFIG_KVM_ARM_HOST) += ../../../virt/lib/
--
1.9.1

2015-08-03 17:21:47

by Eric Auger

[permalink] [raw]
Subject: [PATCH v4 3/5] KVM: create kvm_irqfd.h

Move _irqfd_resampler and _irqfd struct declarations in a new
public header: kvm_irqfd.h. They are respectively renamed into
kvm_kernel_irqfd_resampler and kvm_kernel_irqfd. Those datatypes
will be used by architecture specific code, in the context of
IRQ bypass manager integration.

Signed-off-by: Eric Auger <[email protected]>
---
include/linux/kvm_irqfd.h | 69 ++++++++++++++++++++++++++++++++++
virt/kvm/eventfd.c | 95 ++++++++++++-----------------------------------
2 files changed, 92 insertions(+), 72 deletions(-)
create mode 100644 include/linux/kvm_irqfd.h

diff --git a/include/linux/kvm_irqfd.h b/include/linux/kvm_irqfd.h
new file mode 100644
index 0000000..f926b39
--- /dev/null
+++ b/include/linux/kvm_irqfd.h
@@ -0,0 +1,69 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ * 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.
+ *
+ * irqfd: Allows an fd to be used to inject an interrupt to the guest
+ * Credit goes to Avi Kivity for the original idea.
+ */
+
+#ifndef __LINUX_KVM_IRQFD_H
+#define __LINUX_KVM_IRQFD_H
+
+#include <linux/kvm_host.h>
+#include <linux/poll.h>
+
+/*
+ * Resampling irqfds are a special variety of irqfds used to emulate
+ * level triggered interrupts. The interrupt is asserted on eventfd
+ * trigger. On acknowledgment through the irq ack notifier, the
+ * interrupt is de-asserted and userspace is notified through the
+ * resamplefd. All resamplers on the same gsi are de-asserted
+ * together, so we don't need to track the state of each individual
+ * user. We can also therefore share the same irq source ID.
+ */
+struct kvm_kernel_irqfd_resampler {
+ struct kvm *kvm;
+ /*
+ * List of resampling struct _irqfd objects sharing this gsi.
+ * RCU list modified under kvm->irqfds.resampler_lock
+ */
+ struct list_head list;
+ struct kvm_irq_ack_notifier notifier;
+ /*
+ * Entry in list of kvm->irqfd.resampler_list. Use for sharing
+ * resamplers among irqfds on the same gsi.
+ * Accessed and modified under kvm->irqfds.resampler_lock
+ */
+ struct list_head link;
+};
+
+struct kvm_kernel_irqfd {
+ /* Used for MSI fast-path */
+ struct kvm *kvm;
+ wait_queue_t wait;
+ /* Update side is protected by irqfds.lock */
+ struct kvm_kernel_irq_routing_entry irq_entry;
+ seqcount_t irq_entry_sc;
+ /* Used for level IRQ fast-path */
+ int gsi;
+ struct work_struct inject;
+ /* The resampler used by this irqfd (resampler-only) */
+ struct kvm_kernel_irqfd_resampler *resampler;
+ /* Eventfd notified on resample (resampler-only) */
+ struct eventfd_ctx *resamplefd;
+ /* Entry in list of irqfds for a resampler (resampler-only) */
+ struct list_head resampler_link;
+ /* Used for setup/shutdown */
+ struct eventfd_ctx *eventfd;
+ struct list_head list;
+ poll_table pt;
+ struct work_struct shutdown;
+};
+
+#endif /* __LINUX_KVM_IRQFD_H */
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index 9ff4193..647ffb8 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -23,6 +23,7 @@

#include <linux/kvm_host.h>
#include <linux/kvm.h>
+#include <linux/kvm_irqfd.h>
#include <linux/workqueue.h>
#include <linux/syscalls.h>
#include <linux/wait.h>
@@ -39,68 +40,14 @@
#include <kvm/iodev.h>

#ifdef CONFIG_HAVE_KVM_IRQFD
-/*
- * --------------------------------------------------------------------
- * irqfd: Allows an fd to be used to inject an interrupt to the guest
- *
- * Credit goes to Avi Kivity for the original idea.
- * --------------------------------------------------------------------
- */
-
-/*
- * Resampling irqfds are a special variety of irqfds used to emulate
- * level triggered interrupts. The interrupt is asserted on eventfd
- * trigger. On acknowledgement through the irq ack notifier, the
- * interrupt is de-asserted and userspace is notified through the
- * resamplefd. All resamplers on the same gsi are de-asserted
- * together, so we don't need to track the state of each individual
- * user. We can also therefore share the same irq source ID.
- */
-struct _irqfd_resampler {
- struct kvm *kvm;
- /*
- * List of resampling struct _irqfd objects sharing this gsi.
- * RCU list modified under kvm->irqfds.resampler_lock
- */
- struct list_head list;
- struct kvm_irq_ack_notifier notifier;
- /*
- * Entry in list of kvm->irqfd.resampler_list. Use for sharing
- * resamplers among irqfds on the same gsi.
- * Accessed and modified under kvm->irqfds.resampler_lock
- */
- struct list_head link;
-};
-
-struct _irqfd {
- /* Used for MSI fast-path */
- struct kvm *kvm;
- wait_queue_t wait;
- /* Update side is protected by irqfds.lock */
- struct kvm_kernel_irq_routing_entry irq_entry;
- seqcount_t irq_entry_sc;
- /* Used for level IRQ fast-path */
- int gsi;
- struct work_struct inject;
- /* The resampler used by this irqfd (resampler-only) */
- struct _irqfd_resampler *resampler;
- /* Eventfd notified on resample (resampler-only) */
- struct eventfd_ctx *resamplefd;
- /* Entry in list of irqfds for a resampler (resampler-only) */
- struct list_head resampler_link;
- /* Used for setup/shutdown */
- struct eventfd_ctx *eventfd;
- struct list_head list;
- poll_table pt;
- struct work_struct shutdown;
-};

static struct workqueue_struct *irqfd_cleanup_wq;

static void
irqfd_inject(struct work_struct *work)
{
- struct _irqfd *irqfd = container_of(work, struct _irqfd, inject);
+ struct kvm_kernel_irqfd *irqfd =
+ container_of(work, struct kvm_kernel_irqfd, inject);
struct kvm *kvm = irqfd->kvm;

if (!irqfd->resampler) {
@@ -121,12 +68,13 @@ irqfd_inject(struct work_struct *work)
static void
irqfd_resampler_ack(struct kvm_irq_ack_notifier *kian)
{
- struct _irqfd_resampler *resampler;
+ struct kvm_kernel_irqfd_resampler *resampler;
struct kvm *kvm;
- struct _irqfd *irqfd;
+ struct kvm_kernel_irqfd *irqfd;
int idx;

- resampler = container_of(kian, struct _irqfd_resampler, notifier);
+ resampler = container_of(kian,
+ struct kvm_kernel_irqfd_resampler, notifier);
kvm = resampler->kvm;

kvm_set_irq(kvm, KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID,
@@ -141,9 +89,9 @@ irqfd_resampler_ack(struct kvm_irq_ack_notifier *kian)
}

static void
-irqfd_resampler_shutdown(struct _irqfd *irqfd)
+irqfd_resampler_shutdown(struct kvm_kernel_irqfd *irqfd)
{
- struct _irqfd_resampler *resampler = irqfd->resampler;
+ struct kvm_kernel_irqfd_resampler *resampler = irqfd->resampler;
struct kvm *kvm = resampler->kvm;

mutex_lock(&kvm->irqfds.resampler_lock);
@@ -168,7 +116,8 @@ irqfd_resampler_shutdown(struct _irqfd *irqfd)
static void
irqfd_shutdown(struct work_struct *work)
{
- struct _irqfd *irqfd = container_of(work, struct _irqfd, shutdown);
+ struct kvm_kernel_irqfd *irqfd =
+ container_of(work, struct kvm_kernel_irqfd, shutdown);
u64 cnt;

/*
@@ -198,7 +147,7 @@ irqfd_shutdown(struct work_struct *work)

/* assumes kvm->irqfds.lock is held */
static bool
-irqfd_is_active(struct _irqfd *irqfd)
+irqfd_is_active(struct kvm_kernel_irqfd *irqfd)
{
return list_empty(&irqfd->list) ? false : true;
}
@@ -209,7 +158,7 @@ irqfd_is_active(struct _irqfd *irqfd)
* assumes kvm->irqfds.lock is held
*/
static void
-irqfd_deactivate(struct _irqfd *irqfd)
+irqfd_deactivate(struct kvm_kernel_irqfd *irqfd)
{
BUG_ON(!irqfd_is_active(irqfd));

@@ -224,7 +173,8 @@ irqfd_deactivate(struct _irqfd *irqfd)
static int
irqfd_wakeup(wait_queue_t *wait, unsigned mode, int sync, void *key)
{
- struct _irqfd *irqfd = container_of(wait, struct _irqfd, wait);
+ struct kvm_kernel_irqfd *irqfd =
+ container_of(wait, struct kvm_kernel_irqfd, wait);
unsigned long flags = (unsigned long)key;
struct kvm_kernel_irq_routing_entry irq;
struct kvm *kvm = irqfd->kvm;
@@ -274,12 +224,13 @@ 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);
+ struct kvm_kernel_irqfd *irqfd =
+ container_of(pt, struct kvm_kernel_irqfd, pt);
add_wait_queue(wqh, &irqfd->wait);
}

/* Must be called under irqfds.lock */
-static void irqfd_update(struct kvm *kvm, struct _irqfd *irqfd)
+static void irqfd_update(struct kvm *kvm, struct kvm_kernel_irqfd *irqfd)
{
struct kvm_kernel_irq_routing_entry *e;
struct kvm_kernel_irq_routing_entry entries[KVM_NR_IRQCHIPS];
@@ -304,7 +255,7 @@ static void irqfd_update(struct kvm *kvm, struct _irqfd *irqfd)
static int
kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)
{
- struct _irqfd *irqfd, *tmp;
+ struct kvm_kernel_irqfd *irqfd, *tmp;
struct fd f;
struct eventfd_ctx *eventfd = NULL, *resamplefd = NULL;
int ret;
@@ -340,7 +291,7 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)
irqfd->eventfd = eventfd;

if (args->flags & KVM_IRQFD_FLAG_RESAMPLE) {
- struct _irqfd_resampler *resampler;
+ struct kvm_kernel_irqfd_resampler *resampler;

resamplefd = eventfd_ctx_fdget(args->resamplefd);
if (IS_ERR(resamplefd)) {
@@ -525,7 +476,7 @@ kvm_eventfd_init(struct kvm *kvm)
static int
kvm_irqfd_deassign(struct kvm *kvm, struct kvm_irqfd *args)
{
- struct _irqfd *irqfd, *tmp;
+ struct kvm_kernel_irqfd *irqfd, *tmp;
struct eventfd_ctx *eventfd;

eventfd = eventfd_ctx_fdget(args->fd);
@@ -581,7 +532,7 @@ kvm_irqfd(struct kvm *kvm, struct kvm_irqfd *args)
void
kvm_irqfd_release(struct kvm *kvm)
{
- struct _irqfd *irqfd, *tmp;
+ struct kvm_kernel_irqfd *irqfd, *tmp;

spin_lock_irq(&kvm->irqfds.lock);

@@ -604,7 +555,7 @@ kvm_irqfd_release(struct kvm *kvm)
*/
void kvm_irq_routing_update(struct kvm *kvm)
{
- struct _irqfd *irqfd;
+ struct kvm_kernel_irqfd *irqfd;

spin_lock_irq(&kvm->irqfds.lock);

--
1.9.1

2015-08-03 17:22:42

by Eric Auger

[permalink] [raw]
Subject: [PATCH v4 4/5] KVM: introduce kvm_arch functions for IRQ bypass

This patch introduces
- kvm_arch_irq_bypass_add_producer
- kvm_arch_irq_bypass_del_producer
- kvm_arch_irq_bypass_stop
- kvm_arch_irq_bypass_start

They make possible to specialize the KVM IRQ bypass consumer in
case CONFIG_KVM_HAVE_IRQ_BYPASS is set.

Signed-off-by: Eric Auger <[email protected]>
Signed-off-by: Feng Wu <[email protected]>

---

v2 -> v3 (Feng Wu):
- use 'kvm_arch_irq_bypass_start' instead of 'kvm_arch_irq_bypass_resume'
- Remove 'kvm_arch_irq_bypass_update', which is not needed to be
a irqbypass callback per Alex's comments.
- Make kvm_arch_irq_bypass_add_producer return 'int'

v1 -> v2:
- use CONFIG_KVM_HAVE_IRQ_BYPASS instead CONFIG_IRQ_BYPASS_MANAGER
- rename all functions according to Paolo's proposal
- add kvm_arch_irq_bypass_update according to Feng's need
---
include/linux/kvm_host.h | 33 +++++++++++++++++++++++++++++++++
virt/kvm/Kconfig | 3 +++
2 files changed, 36 insertions(+)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 05e99b8..84b5feb 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -24,6 +24,7 @@
#include <linux/err.h>
#include <linux/irqflags.h>
#include <linux/context_tracking.h>
+#include <linux/irqbypass.h>
#include <asm/signal.h>

#include <linux/kvm.h>
@@ -1151,5 +1152,37 @@ static inline void kvm_vcpu_set_dy_eligible(struct kvm_vcpu *vcpu, bool val)
{
}
#endif /* CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT */
+
+#ifdef CONFIG_HAVE_KVM_IRQ_BYPASS
+
+int kvm_arch_irq_bypass_add_producer(struct irq_bypass_consumer *,
+ struct irq_bypass_producer *);
+void kvm_arch_irq_bypass_del_producer(struct irq_bypass_consumer *,
+ struct irq_bypass_producer *);
+void kvm_arch_irq_bypass_stop(struct irq_bypass_consumer *);
+void kvm_arch_irq_bypass_start(struct irq_bypass_consumer *);
+
+#else
+
+static inline int kvm_arch_irq_bypass_add_producer(
+ struct irq_bypass_consumer *cons,
+ struct irq_bypass_producer *prod)
+{
+ return -1;
+}
+static inline void kvm_arch_irq_bypass_del_producer(
+ struct irq_bypass_consumer *cons,
+ struct irq_bypass_producer *prod)
+{
+}
+static inline void kvm_arch_irq_bypass_stop(
+ struct irq_bypass_consumer *cons)
+{
+}
+static inline void kvm_arch_irq_bypass_start(
+ struct irq_bypass_consumer *cons)
+{
+}
+#endif /* CONFIG_HAVE_KVM_IRQ_BYPASS */
#endif

diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
index e2c876d..9f8014d 100644
--- a/virt/kvm/Kconfig
+++ b/virt/kvm/Kconfig
@@ -47,3 +47,6 @@ config KVM_GENERIC_DIRTYLOG_READ_PROTECT
config KVM_COMPAT
def_bool y
depends on COMPAT && !S390
+
+config HAVE_KVM_IRQ_BYPASS
+ bool
--
1.9.1

2015-08-03 17:21:49

by Eric Auger

[permalink] [raw]
Subject: [PATCH v4 5/5] KVM: eventfd: add irq bypass consumer management

This patch adds the registration/unregistration of an
irq_bypass_consumer on irqfd assignment/deassignment.

Signed-off-by: Eric Auger <[email protected]>
Signed-off-by: Feng Wu <[email protected]>

---

v2 -> v3 (Feng Wu):
- Use kvm_arch_irq_bypass_start
- Remove kvm_arch_irq_bypass_update
- Add member 'struct irq_bypass_producer *producer' in
'struct kvm_kernel_irqfd', it is needed by posted interrupt.
- Remove 'irq_bypass_unregister_consumer' in kvm_irqfd_deassign()

v1 -> v2:
- populate of kvm and gsi removed
- unregister the consumer on irqfd_shutdown
---
include/linux/kvm_irqfd.h | 2 ++
virt/kvm/eventfd.c | 10 ++++++++++
2 files changed, 12 insertions(+)

diff --git a/include/linux/kvm_irqfd.h b/include/linux/kvm_irqfd.h
index f926b39..0c1de05 100644
--- a/include/linux/kvm_irqfd.h
+++ b/include/linux/kvm_irqfd.h
@@ -64,6 +64,8 @@ struct kvm_kernel_irqfd {
struct list_head list;
poll_table pt;
struct work_struct shutdown;
+ struct irq_bypass_consumer consumer;
+ struct irq_bypass_producer *producer;
};

#endif /* __LINUX_KVM_IRQFD_H */
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index 647ffb8..08855de 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -35,6 +35,7 @@
#include <linux/srcu.h>
#include <linux/slab.h>
#include <linux/seqlock.h>
+#include <linux/irqbypass.h>
#include <trace/events/kvm.h>

#include <kvm/iodev.h>
@@ -140,6 +141,7 @@ irqfd_shutdown(struct work_struct *work)
/*
* It is now safe to release the object's resources
*/
+ irq_bypass_unregister_consumer(&irqfd->consumer);
eventfd_ctx_put(irqfd->eventfd);
kfree(irqfd);
}
@@ -380,6 +382,14 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)
*/
fdput(f);

+ irqfd->consumer.token = (void *)irqfd->eventfd;
+ irqfd->consumer.add_producer = kvm_arch_irq_bypass_add_producer;
+ irqfd->consumer.del_producer = kvm_arch_irq_bypass_del_producer;
+ irqfd->consumer.stop = kvm_arch_irq_bypass_stop;
+ irqfd->consumer.start = kvm_arch_irq_bypass_start;
+ ret = irq_bypass_register_consumer(&irqfd->consumer);
+ WARN_ON(ret);
+
return 0;

fail:
--
1.9.1

2015-08-07 20:09:17

by Alex Williamson

[permalink] [raw]
Subject: Re: [PATCH v4 4/5] KVM: introduce kvm_arch functions for IRQ bypass

On Mon, 2015-08-03 at 19:20 +0200, Eric Auger wrote:
> This patch introduces
> - kvm_arch_irq_bypass_add_producer
> - kvm_arch_irq_bypass_del_producer
> - kvm_arch_irq_bypass_stop
> - kvm_arch_irq_bypass_start
>
> They make possible to specialize the KVM IRQ bypass consumer in
> case CONFIG_KVM_HAVE_IRQ_BYPASS is set.
>
> Signed-off-by: Eric Auger <[email protected]>
> Signed-off-by: Feng Wu <[email protected]>
>
> ---
>
> v2 -> v3 (Feng Wu):
> - use 'kvm_arch_irq_bypass_start' instead of 'kvm_arch_irq_bypass_resume'
> - Remove 'kvm_arch_irq_bypass_update', which is not needed to be
> a irqbypass callback per Alex's comments.
> - Make kvm_arch_irq_bypass_add_producer return 'int'
>
> v1 -> v2:
> - use CONFIG_KVM_HAVE_IRQ_BYPASS instead CONFIG_IRQ_BYPASS_MANAGER
> - rename all functions according to Paolo's proposal
> - add kvm_arch_irq_bypass_update according to Feng's need
> ---
> include/linux/kvm_host.h | 33 +++++++++++++++++++++++++++++++++
> virt/kvm/Kconfig | 3 +++
> 2 files changed, 36 insertions(+)
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 05e99b8..84b5feb 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -24,6 +24,7 @@
> #include <linux/err.h>
> #include <linux/irqflags.h>
> #include <linux/context_tracking.h>
> +#include <linux/irqbypass.h>
> #include <asm/signal.h>
>
> #include <linux/kvm.h>
> @@ -1151,5 +1152,37 @@ static inline void kvm_vcpu_set_dy_eligible(struct kvm_vcpu *vcpu, bool val)
> {
> }
> #endif /* CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT */
> +
> +#ifdef CONFIG_HAVE_KVM_IRQ_BYPASS
> +
> +int kvm_arch_irq_bypass_add_producer(struct irq_bypass_consumer *,
> + struct irq_bypass_producer *);
> +void kvm_arch_irq_bypass_del_producer(struct irq_bypass_consumer *,
> + struct irq_bypass_producer *);
> +void kvm_arch_irq_bypass_stop(struct irq_bypass_consumer *);
> +void kvm_arch_irq_bypass_start(struct irq_bypass_consumer *);
> +
> +#else

Do we really need static inline stubs? When would they get used? How
would they work since we call them via function pointers?

> +
> +static inline int kvm_arch_irq_bypass_add_producer(
> + struct irq_bypass_consumer *cons,
> + struct irq_bypass_producer *prod)
> +{
> + return -1;

No reason not to stick with standard errno values, is there? -EINVAL
Thanks,

Alex

> +}
> +static inline void kvm_arch_irq_bypass_del_producer(
> + struct irq_bypass_consumer *cons,
> + struct irq_bypass_producer *prod)
> +{
> +}
> +static inline void kvm_arch_irq_bypass_stop(
> + struct irq_bypass_consumer *cons)
> +{
> +}
> +static inline void kvm_arch_irq_bypass_start(
> + struct irq_bypass_consumer *cons)
> +{
> +}
> +#endif /* CONFIG_HAVE_KVM_IRQ_BYPASS */
> #endif
>
> diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
> index e2c876d..9f8014d 100644
> --- a/virt/kvm/Kconfig
> +++ b/virt/kvm/Kconfig
> @@ -47,3 +47,6 @@ config KVM_GENERIC_DIRTYLOG_READ_PROTECT
> config KVM_COMPAT
> def_bool y
> depends on COMPAT && !S390
> +
> +config HAVE_KVM_IRQ_BYPASS
> + bool


2015-08-07 20:09:19

by Alex Williamson

[permalink] [raw]
Subject: Re: [PATCH v4 5/5] KVM: eventfd: add irq bypass consumer management

On Mon, 2015-08-03 at 19:20 +0200, Eric Auger wrote:
> This patch adds the registration/unregistration of an
> irq_bypass_consumer on irqfd assignment/deassignment.
>
> Signed-off-by: Eric Auger <[email protected]>
> Signed-off-by: Feng Wu <[email protected]>
>
> ---
>
> v2 -> v3 (Feng Wu):
> - Use kvm_arch_irq_bypass_start
> - Remove kvm_arch_irq_bypass_update
> - Add member 'struct irq_bypass_producer *producer' in
> 'struct kvm_kernel_irqfd', it is needed by posted interrupt.
> - Remove 'irq_bypass_unregister_consumer' in kvm_irqfd_deassign()
>
> v1 -> v2:
> - populate of kvm and gsi removed
> - unregister the consumer on irqfd_shutdown
> ---
> include/linux/kvm_irqfd.h | 2 ++
> virt/kvm/eventfd.c | 10 ++++++++++
> 2 files changed, 12 insertions(+)
>
> diff --git a/include/linux/kvm_irqfd.h b/include/linux/kvm_irqfd.h
> index f926b39..0c1de05 100644
> --- a/include/linux/kvm_irqfd.h
> +++ b/include/linux/kvm_irqfd.h
> @@ -64,6 +64,8 @@ struct kvm_kernel_irqfd {
> struct list_head list;
> poll_table pt;
> struct work_struct shutdown;
> + struct irq_bypass_consumer consumer;
> + struct irq_bypass_producer *producer;
> };
>
> #endif /* __LINUX_KVM_IRQFD_H */
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index 647ffb8..08855de 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -35,6 +35,7 @@
> #include <linux/srcu.h>
> #include <linux/slab.h>
> #include <linux/seqlock.h>
> +#include <linux/irqbypass.h>
> #include <trace/events/kvm.h>
>
> #include <kvm/iodev.h>
> @@ -140,6 +141,7 @@ irqfd_shutdown(struct work_struct *work)
> /*
> * It is now safe to release the object's resources
> */
> + irq_bypass_unregister_consumer(&irqfd->consumer);
> eventfd_ctx_put(irqfd->eventfd);
> kfree(irqfd);
> }
> @@ -380,6 +382,14 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)
> */
> fdput(f);
>
> + irqfd->consumer.token = (void *)irqfd->eventfd;
> + irqfd->consumer.add_producer = kvm_arch_irq_bypass_add_producer;
> + irqfd->consumer.del_producer = kvm_arch_irq_bypass_del_producer;
> + irqfd->consumer.stop = kvm_arch_irq_bypass_stop;
> + irqfd->consumer.start = kvm_arch_irq_bypass_start;
> + ret = irq_bypass_register_consumer(&irqfd->consumer);
> + WARN_ON(ret);

This seems like a lazy way to handle this error. What is the stack
trace from this WARN_ON going to tell us that we didn't already know?
If we get the WARN_ON, it's probably means that an incompatible producer
registered the token first. It means we can't do bypass, but it doesn't
tell us anything about whether bypass is or is not enabled. Wouldn't a
pr_info/debug() suffice for that? Thanks,

Alex

> +
> return 0;
>
> fail:


2015-08-10 08:43:50

by Eric Auger

[permalink] [raw]
Subject: Re: [PATCH v4 4/5] KVM: introduce kvm_arch functions for IRQ bypass

On 08/07/2015 10:09 PM, Alex Williamson wrote:
> On Mon, 2015-08-03 at 19:20 +0200, Eric Auger wrote:
>> This patch introduces
>> - kvm_arch_irq_bypass_add_producer
>> - kvm_arch_irq_bypass_del_producer
>> - kvm_arch_irq_bypass_stop
>> - kvm_arch_irq_bypass_start
>>
>> They make possible to specialize the KVM IRQ bypass consumer in
>> case CONFIG_KVM_HAVE_IRQ_BYPASS is set.
>>
>> Signed-off-by: Eric Auger <[email protected]>
>> Signed-off-by: Feng Wu <[email protected]>
>>
>> ---
>>
>> v2 -> v3 (Feng Wu):
>> - use 'kvm_arch_irq_bypass_start' instead of 'kvm_arch_irq_bypass_resume'
>> - Remove 'kvm_arch_irq_bypass_update', which is not needed to be
>> a irqbypass callback per Alex's comments.
>> - Make kvm_arch_irq_bypass_add_producer return 'int'
>>
>> v1 -> v2:
>> - use CONFIG_KVM_HAVE_IRQ_BYPASS instead CONFIG_IRQ_BYPASS_MANAGER
>> - rename all functions according to Paolo's proposal
>> - add kvm_arch_irq_bypass_update according to Feng's need
>> ---
>> include/linux/kvm_host.h | 33 +++++++++++++++++++++++++++++++++
>> virt/kvm/Kconfig | 3 +++
>> 2 files changed, 36 insertions(+)
>>
>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> index 05e99b8..84b5feb 100644
>> --- a/include/linux/kvm_host.h
>> +++ b/include/linux/kvm_host.h
>> @@ -24,6 +24,7 @@
>> #include <linux/err.h>
>> #include <linux/irqflags.h>
>> #include <linux/context_tracking.h>
>> +#include <linux/irqbypass.h>
>> #include <asm/signal.h>
>>
>> #include <linux/kvm.h>
>> @@ -1151,5 +1152,37 @@ static inline void kvm_vcpu_set_dy_eligible(struct kvm_vcpu *vcpu, bool val)
>> {
>> }
>> #endif /* CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT */
>> +
>> +#ifdef CONFIG_HAVE_KVM_IRQ_BYPASS
>> +
>> +int kvm_arch_irq_bypass_add_producer(struct irq_bypass_consumer *,
>> + struct irq_bypass_producer *);
>> +void kvm_arch_irq_bypass_del_producer(struct irq_bypass_consumer *,
>> + struct irq_bypass_producer *);
>> +void kvm_arch_irq_bypass_stop(struct irq_bypass_consumer *);
>> +void kvm_arch_irq_bypass_start(struct irq_bypass_consumer *);
>> +
>> +#else
>
> Do we really need static inline stubs? When would they get used?
This addresses the case where another arch would use an irq bypass
producer (such as vfio platform driver ) and irqfd without
implementing/needing forwarding/posting stuff.

I see 2 solutions: either we have those stubs or in eventfd.c we guard
irq_bypass_unregister_consumer(&irqfd->consumer);
and
ret = irq_bypass_register_consumer(&irqfd->consumer);
by

#ifdef CONFIG_HAVE_KVM_IRQ_BYPASS
#endif

This latter solution maybe is better.

How
> would they work since we call them via function pointers?
Just tested that without the ARM forwarding implementation of kvm_arch
functions and that runs fine.
>
>> +
>> +static inline int kvm_arch_irq_bypass_add_producer(
>> + struct irq_bypass_consumer *cons,
>> + struct irq_bypass_producer *prod)
>> +{
>> + return -1;
>
> No reason not to stick with standard errno values, is there? -EINVAL
> Thanks,
sure

Thanks

Eric
>
> Alex
>
>> +}
>> +static inline void kvm_arch_irq_bypass_del_producer(
>> + struct irq_bypass_consumer *cons,
>> + struct irq_bypass_producer *prod)
>> +{
>> +}
>> +static inline void kvm_arch_irq_bypass_stop(
>> + struct irq_bypass_consumer *cons)
>> +{
>> +}
>> +static inline void kvm_arch_irq_bypass_start(
>> + struct irq_bypass_consumer *cons)
>> +{
>> +}
>> +#endif /* CONFIG_HAVE_KVM_IRQ_BYPASS */
>> #endif
>>
>> diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
>> index e2c876d..9f8014d 100644
>> --- a/virt/kvm/Kconfig
>> +++ b/virt/kvm/Kconfig
>> @@ -47,3 +47,6 @@ config KVM_GENERIC_DIRTYLOG_READ_PROTECT
>> config KVM_COMPAT
>> def_bool y
>> depends on COMPAT && !S390
>> +
>> +config HAVE_KVM_IRQ_BYPASS
>> + bool
>
>
>

2015-08-10 08:45:00

by Eric Auger

[permalink] [raw]
Subject: Re: [PATCH v4 5/5] KVM: eventfd: add irq bypass consumer management

Hi Alex,
On 08/07/2015 10:09 PM, Alex Williamson wrote:
> On Mon, 2015-08-03 at 19:20 +0200, Eric Auger wrote:
>> This patch adds the registration/unregistration of an
>> irq_bypass_consumer on irqfd assignment/deassignment.
>>
>> Signed-off-by: Eric Auger <[email protected]>
>> Signed-off-by: Feng Wu <[email protected]>
>>
>> ---
>>
>> v2 -> v3 (Feng Wu):
>> - Use kvm_arch_irq_bypass_start
>> - Remove kvm_arch_irq_bypass_update
>> - Add member 'struct irq_bypass_producer *producer' in
>> 'struct kvm_kernel_irqfd', it is needed by posted interrupt.
>> - Remove 'irq_bypass_unregister_consumer' in kvm_irqfd_deassign()
>>
>> v1 -> v2:
>> - populate of kvm and gsi removed
>> - unregister the consumer on irqfd_shutdown
>> ---
>> include/linux/kvm_irqfd.h | 2 ++
>> virt/kvm/eventfd.c | 10 ++++++++++
>> 2 files changed, 12 insertions(+)
>>
>> diff --git a/include/linux/kvm_irqfd.h b/include/linux/kvm_irqfd.h
>> index f926b39..0c1de05 100644
>> --- a/include/linux/kvm_irqfd.h
>> +++ b/include/linux/kvm_irqfd.h
>> @@ -64,6 +64,8 @@ struct kvm_kernel_irqfd {
>> struct list_head list;
>> poll_table pt;
>> struct work_struct shutdown;
>> + struct irq_bypass_consumer consumer;
>> + struct irq_bypass_producer *producer;
>> };
>>
>> #endif /* __LINUX_KVM_IRQFD_H */
>> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
>> index 647ffb8..08855de 100644
>> --- a/virt/kvm/eventfd.c
>> +++ b/virt/kvm/eventfd.c
>> @@ -35,6 +35,7 @@
>> #include <linux/srcu.h>
>> #include <linux/slab.h>
>> #include <linux/seqlock.h>
>> +#include <linux/irqbypass.h>
>> #include <trace/events/kvm.h>
>>
>> #include <kvm/iodev.h>
>> @@ -140,6 +141,7 @@ irqfd_shutdown(struct work_struct *work)
>> /*
>> * It is now safe to release the object's resources
>> */
>> + irq_bypass_unregister_consumer(&irqfd->consumer);
>> eventfd_ctx_put(irqfd->eventfd);
>> kfree(irqfd);
>> }
>> @@ -380,6 +382,14 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)
>> */
>> fdput(f);
>>
>> + irqfd->consumer.token = (void *)irqfd->eventfd;
>> + irqfd->consumer.add_producer = kvm_arch_irq_bypass_add_producer;
>> + irqfd->consumer.del_producer = kvm_arch_irq_bypass_del_producer;
>> + irqfd->consumer.stop = kvm_arch_irq_bypass_stop;
>> + irqfd->consumer.start = kvm_arch_irq_bypass_start;
>> + ret = irq_bypass_register_consumer(&irqfd->consumer);
>> + WARN_ON(ret);
>
> This seems like a lazy way to handle this error. What is the stack
> trace from this WARN_ON going to tell us that we didn't already know?
> If we get the WARN_ON, it's probably means that an incompatible producer
> registered the token first. It means we can't do bypass, but it doesn't
> tell us anything about whether bypass is or is not enabled. Wouldn't a
> pr_info/debug() suffice for that? Thanks,

Sure I will output some more relevant traces.

Thanks

Eric
>
> Alex
>
>> +
>> return 0;
>>
>> fail:
>
>
>