[RFC 00/17] ARM IRQ forward control based on IRQ bypass manager

[RFC 00/17] ARM IRQ forward control based on IRQ bypass manager

[Eric Auger]

This series allows to set ARM IRQ forwarding between a VFIO platform
device physical IRQ and a guest virtual IRQ.

The setting is coordinated by the prototype IRQ bypass manager.
This kernel integration seems now prefered to previous kvm-vfio device
user api:
- [RFC v6 00/16] KVM-VFIO IRQ forward control,
  https://lkml.org/lkml/2015/4/13/353).

Some rationale can be found in IRQ bypass manager thread:
https://lkml.org/lkml/2015/6/29/268

The principle is the VFIO platform driver registers a producer struct
on VFIO_IRQ_SET_ACTION_TRIGGER while KVM irqfd registers a consumer struct
on the irqfd assignment. This leads to a handshake based on the eventfd
context (used as token) match.

When either of the producer/consumer module disappears, this leads to
an unregistration and the link is disconnected.

Structure of the series:
[1-6] Modifications in the VFIO (platform) driver to prepare for dynamic
      switch between automasked/masked mode
[7-8] Introduce halt/resume guest capability
[9] irq bypass manager proto as sent by Alex
[10-17] Adaptations to support forwarding on top of IRQ bypass manager

Dependencies:
1- [PATCH 00/10] arm/arm64: KVM: Active interrupt state switching for
   shared devices (http://www.spinics.net/lists/kvm/msg117411.html)
2- RFC "ARM: Forwarding physical interrupts to a guest VM"
   (http://lwn.net/Articles/603514/)
3- IRQ bypass manager proto: https://lkml.org/lkml/2015/6/29/268
4- [RFC v2 0/4] chip/vgic adaptations for forwarded irq
   http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/323183.html

All those pieces can be found at:
https://git.linaro.org/people/eric.auger/linux.git/shortlog/refs/heads/v4.2-rc1-bypass-fwd

More backgroung on ARM IRQ forwarding in the text below and at
http://www.linux-kvm.org/images/a/a8/01x04-ARMdevice.pdf.

A forwarded IRQ is deactivated by the guest and not by the host.
When the guest deactivates the associated virtual IRQ, the interrupt
controller automatically completes the physical IRQ. Obviously
this requires some HW support in the interrupt controller. This is
the case for ARM GICv2.

The direct benefit is that, for a level sensitive IRQ, a VM exit
can be avoided on forwarded IRQ completion.

When the IRQ is forwarded, the VFIO platform driver does not need to
mask the physical IRQ anymore before signaling the eventfd. Indeed
genirq lowers the running priority, enabling other physical IRQ to hit
except that one.

Besides, the injection still is based on irqfd triggering. The only
impact on irqfd process is resamplefd is not called anymore on
virtual IRQ completion since deactivation is not trapped by KVM.

This was tested on Calxeda Midway, assigning the xgmac main IRQ

kvm-vfio v6 -> rfc based on IRQ bypass manager
see previous history in https://lkml.org/lkml/2015/4/13/353).

Best Regards

Eric


Alex Williamson (1):
  bypass: IRQ bypass manager proto by Alex

Eric Auger (16):
  VFIO: platform: test forwarded state when selecting IRQ handler
  VFIO: platform: single handler using function pointer
  VFIO: Introduce vfio_device_external_ops
  VFIO: pci: initialize vfio_device_external_ops
  VFIO: platform: implement vfio_device_external_ops callbacks
  VFIO: add vfio_external_{mask|is_active|set_automasked}
  KVM: arm: rename pause into power_off
  kvm: arm/arm64: implement kvm_arm_[halt,resume]_guest
  KVM: arm: select IRQ_BYPASS_MANAGER
  VFIO: platform: select IRQ_BYPASS_MANAGER
  irq: bypass: Extend skeleton for ARM forwarding control
  KVM: introduce kvm_arch functions for IRQ bypass
  KVM: arm/arm64: vgic: forwarding control
  KVM: arm/arm64: implement IRQ bypass consumer functions
  KVM: eventfd: add irq bypass consumer management
  VFIO: platform: add irq bypass producer management

 arch/arm/include/asm/kvm_host.h               |   5 +-
 arch/arm/kvm/Kconfig                          |   1 +
 arch/arm/kvm/arm.c                            |  60 +++++++-
 arch/arm/kvm/psci.c                           |  10 +-
 arch/arm64/include/asm/kvm_host.h             |   3 +
 arch/x86/kvm/Kconfig                          |   1 +
 drivers/vfio/pci/Kconfig                      |   1 +
 drivers/vfio/pci/vfio_pci.c                   |   1 +
 drivers/vfio/pci/vfio_pci_intrs.c             |   6 +
 drivers/vfio/platform/Kconfig                 |   1 +
 drivers/vfio/platform/vfio_platform_common.c  |   9 ++
 drivers/vfio/platform/vfio_platform_irq.c     | 160 ++++++++++++++++++++-
 drivers/vfio/platform/vfio_platform_private.h |  14 ++
 drivers/vfio/vfio.c                           |  39 ++++++
 include/kvm/arm_vgic.h                        |   7 +
 include/linux/irqbypass.h                     |  43 ++++++
 include/linux/kvm_host.h                      |  27 ++++
 include/linux/vfio.h                          |  34 +++++
 kernel/irq/Kconfig                            |   3 +
 kernel/irq/Makefile                           |   1 +
 kernel/irq/bypass.c                           | 156 +++++++++++++++++++++
 virt/kvm/arm/vgic.c                           | 195 ++++++++++++++++++++++++++
 virt/kvm/eventfd.c                            |  20 +++
 23 files changed, 778 insertions(+), 19 deletions(-)
 create mode 100644 include/linux/irqbypass.h
 create mode 100644 kernel/irq/bypass.c

-- 
1.9.1

[RFC 01/17] VFIO: platform: test forwarded state when selecting IRQ handler

[Eric Auger]

In case the IRQ is forwarded, the VFIO platform IRQ handler does not
need to disable the IRQ anymore.

When setting the IRQ handler we now also test the forwarded state. In
case the IRQ is forwarded we select the vfio_irq_handler.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

v3 -> v4:
- change title

v2 -> v3:
- forwarded state was tested in the handler. Now the forwarded state
  is tested before setting the handler. This definitively limits
  the dynamics of forwarded state changes but I don't think there is
  a use case where we need to be able to change the state at any time.

Conflicts:
	drivers/vfio/platform/vfio_platform_irq.c
---
 drivers/vfio/platform/vfio_platform_irq.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c
index 88bba57..132bb3f 100644
--- a/drivers/vfio/platform/vfio_platform_irq.c
+++ b/drivers/vfio/platform/vfio_platform_irq.c
@@ -229,8 +229,13 @@ static int vfio_platform_set_irq_trigger(struct vfio_platform_device *vdev,
 {
 	struct vfio_platform_irq *irq = &vdev->irqs[index];
 	irq_handler_t handler;
+	struct irq_data *d;
+	bool is_forwarded;
 
-	if (vdev->irqs[index].flags & VFIO_IRQ_INFO_AUTOMASKED)
+	d = irq_get_irq_data(irq->hwirq);
+	is_forwarded = irqd_irq_forwarded(d);
+
+	if (vdev->irqs[index].flags & VFIO_IRQ_INFO_AUTOMASKED && !is_forwarded)
 		handler = vfio_automasked_irq_handler;
 	else
 		handler = vfio_irq_handler;
-- 
1.9.1

[RFC 02/17] VFIO: platform: single handler using function pointer

[Eric Auger]

A single handler now is registered whatever the use case: automasked
or not. A function pointer is set according to the wished behavior
and the handler calls this function.

The irq lock is taken/released in the root handler. eventfd_signal can
be called in regions not allowed to sleep.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

v4: creation
---
 drivers/vfio/platform/vfio_platform_irq.c     | 21 +++++++++++++++------
 drivers/vfio/platform/vfio_platform_private.h |  1 +
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c
index 132bb3f..8eb65c1 100644
--- a/drivers/vfio/platform/vfio_platform_irq.c
+++ b/drivers/vfio/platform/vfio_platform_irq.c
@@ -147,11 +147,8 @@ static int vfio_platform_set_irq_unmask(struct vfio_platform_device *vdev,
 static irqreturn_t vfio_automasked_irq_handler(int irq, void *dev_id)
 {
 	struct vfio_platform_irq *irq_ctx = dev_id;
-	unsigned long flags;
 	int ret = IRQ_NONE;
 
-	spin_lock_irqsave(&irq_ctx->lock, flags);
-
 	if (!irq_ctx->masked) {
 		ret = IRQ_HANDLED;
 
@@ -160,8 +157,6 @@ static irqreturn_t vfio_automasked_irq_handler(int irq, void *dev_id)
 		irq_ctx->masked = true;
 	}
 
-	spin_unlock_irqrestore(&irq_ctx->lock, flags);
-
 	if (ret == IRQ_HANDLED)
 		eventfd_signal(irq_ctx->trigger, 1);
 
@@ -177,6 +172,19 @@ static irqreturn_t vfio_irq_handler(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t vfio_handler(int irq, void *dev_id)
+{
+	struct vfio_platform_irq *irq_ctx = dev_id;
+	unsigned long flags;
+	irqreturn_t ret;
+
+	spin_lock_irqsave(&irq_ctx->lock, flags);
+	ret = irq_ctx->handler(irq, dev_id);
+	spin_unlock_irqrestore(&irq_ctx->lock, flags);
+
+	return ret;
+}
+
 static int vfio_set_trigger(struct vfio_platform_device *vdev, int index,
 			    int fd, irq_handler_t handler)
 {
@@ -206,9 +214,10 @@ static int vfio_set_trigger(struct vfio_platform_device *vdev, int index,
 	}
 
 	irq->trigger = trigger;
+	irq->handler = handler;
 
 	irq_set_status_flags(irq->hwirq, IRQ_NOAUTOEN);
-	ret = request_irq(irq->hwirq, handler, 0, irq->name, irq);
+	ret = request_irq(irq->hwirq, vfio_handler, 0, irq->name, irq);
 	if (ret) {
 		kfree(irq->name);
 		eventfd_ctx_put(trigger);
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index 1c9b3d5..413f575 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -37,6 +37,7 @@ struct vfio_platform_irq {
 	spinlock_t		lock;
 	struct virqfd		*unmask;
 	struct virqfd		*mask;
+	irqreturn_t (*handler)(int irq, void *dev_id);
 };
 
 struct vfio_platform_region {
-- 
1.9.1

[RFC 03/17] VFIO: Introduce vfio_device_external_ops

[Eric Auger]

New bus callbacks are introduced. They correspond to external
functions. To avoid messing up the main vfio_device_ops
struct, a new vfio_device_external_ops struct is introduced.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

v6: creation
---
 include/linux/vfio.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index ddb4409..d79e8a9 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -19,6 +19,23 @@
 #include <uapi/linux/vfio.h>
 
 /**
+ * struct vfio_device_external_ops - VFIO bus driver device callbacks
+ * used as external API
+ * @mask: mask any IRQ defined by triplet
+ * @is_active: returns whether any IRQ defined by triplet is active
+ * @set_automasked: sets the automasked flag of triplet's IRQ
+ */
+struct vfio_device_external_ops  {
+	int (*mask)(void *device_data, unsigned index, unsigned start,
+		    unsigned count);
+	int (*is_active)(void *device_data, unsigned index, unsigned start,
+			 unsigned count);
+	int (*set_automasked)(void *device_data, unsigned index,
+			      unsigned start, unsigned count,
+			       bool automasked);
+};
+
+/**
  * struct vfio_device_ops - VFIO bus driver device callbacks
  *
  * @open: Called when userspace creates new file descriptor for device
@@ -42,6 +59,7 @@ struct vfio_device_ops {
 			 unsigned long arg);
 	int	(*mmap)(void *device_data, struct vm_area_struct *vma);
 	void	(*request)(void *device_data, unsigned int count);
+	struct vfio_device_external_ops *external_ops;
 };
 
 extern int vfio_add_group_dev(struct device *dev,
-- 
1.9.1

[RFC 04/17] VFIO: pci: initialize vfio_device_external_ops

[Eric Auger]

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

v6: creation
---
 drivers/vfio/pci/vfio_pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 964ad57..1e48125 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -929,6 +929,7 @@ static const struct vfio_device_ops vfio_pci_ops = {
 	.write		= vfio_pci_write,
 	.mmap		= vfio_pci_mmap,
 	.request	= vfio_pci_request,
+	.external_ops	= NULL,
 };
 
 static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
-- 
1.9.1

[RFC 05/17] VFIO: platform: implement vfio_device_external_ops callbacks

[Eric Auger]

This patch adds the implementation for the 3 external callbacks of
vfio_device_external_ops struct, namely active, is_active,
set_automasked. Also vfio_device_ops and vfio_device_external_ops are
set accordingly.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

v6: creation
---
 drivers/vfio/platform/vfio_platform_common.c  |  7 ++++
 drivers/vfio/platform/vfio_platform_irq.c     | 49 +++++++++++++++++++++++++++
 drivers/vfio/platform/vfio_platform_private.h | 11 ++++++
 3 files changed, 67 insertions(+)

diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index e43efb5..9acfca6 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -520,6 +520,12 @@ static int vfio_platform_mmap(void *device_data, struct vm_area_struct *vma)
 	return -EINVAL;
 }
 
+static struct vfio_device_external_ops vfio_platform_external_ops = {
+	.mask		= vfio_platform_external_mask,
+	.is_active	= vfio_platform_external_is_active,
+	.set_automasked	= vfio_platform_external_set_automasked,
+};
+
 static const struct vfio_device_ops vfio_platform_ops = {
 	.name		= "vfio-platform",
 	.open		= vfio_platform_open,
@@ -528,6 +534,7 @@ static const struct vfio_device_ops vfio_platform_ops = {
 	.read		= vfio_platform_read,
 	.write		= vfio_platform_write,
 	.mmap		= vfio_platform_mmap,
+	.external_ops	= &vfio_platform_external_ops
 };
 
 int vfio_platform_probe_common(struct vfio_platform_device *vdev,
diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c
index 8eb65c1..f6d83ed 100644
--- a/drivers/vfio/platform/vfio_platform_irq.c
+++ b/drivers/vfio/platform/vfio_platform_irq.c
@@ -231,6 +231,55 @@ static int vfio_set_trigger(struct vfio_platform_device *vdev, int index,
 	return 0;
 }
 
+int vfio_platform_external_mask(void *device_data, unsigned index,
+				 unsigned start, unsigned count)
+{
+	struct vfio_platform_device *vdev = device_data;
+
+	vfio_platform_mask(&vdev->irqs[index]);
+	return 0;
+}
+
+int vfio_platform_external_is_active(void *device_data, unsigned index,
+				      unsigned start, unsigned count)
+{
+	unsigned long flags;
+	struct vfio_platform_device *vdev = device_data;
+	struct vfio_platform_irq *irq = &vdev->irqs[index];
+	bool active, masked, outstanding;
+	int ret;
+
+	spin_lock_irqsave(&irq->lock, flags);
+
+	ret = irq_get_irqchip_state(irq->hwirq, IRQCHIP_STATE_ACTIVE, &active);
+	BUG_ON(ret);
+	masked = irq->masked;
+	outstanding = active || masked;
+
+	spin_unlock_irqrestore(&irq->lock, flags);
+	return outstanding;
+}
+
+int vfio_platform_external_set_automasked(void *device_data, unsigned index,
+					   unsigned start, unsigned count,
+					   bool automasked)
+{
+	unsigned long flags;
+	struct vfio_platform_device *vdev = device_data;
+	struct vfio_platform_irq *irq = &vdev->irqs[index];
+
+	spin_lock_irqsave(&irq->lock, flags);
+	if (automasked) {
+		irq->flags |= VFIO_IRQ_INFO_AUTOMASKED;
+		irq->handler = vfio_automasked_irq_handler;
+	} else {
+		irq->flags &= ~VFIO_IRQ_INFO_AUTOMASKED;
+		irq->handler = vfio_irq_handler;
+	}
+	spin_unlock_irqrestore(&irq->lock, flags);
+	return 0;
+}
+
 static int vfio_platform_set_irq_trigger(struct vfio_platform_device *vdev,
 					 unsigned index, unsigned start,
 					 unsigned count, uint32_t flags,
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index 413f575..5f46c68 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -90,4 +90,15 @@ extern int vfio_platform_set_irqs_ioctl(struct vfio_platform_device *vdev,
 					unsigned start, unsigned count,
 					void *data);
 
+extern int vfio_platform_external_mask(void *device_data, unsigned index,
+				       unsigned start, unsigned count);
+extern int vfio_platform_external_is_active(void *device_data,
+					    unsigned index, unsigned start,
+					    unsigned count);
+extern int vfio_platform_external_set_automasked(void *device_data,
+						 unsigned index,
+						 unsigned start,
+						 unsigned count,
+						 bool automasked);
+
 #endif /* VFIO_PLATFORM_PRIVATE_H */
-- 
1.9.1

[RFC 06/17] VFIO: add vfio_external_{mask|is_active|set_automasked}

[Eric Auger]

Introduces 3 new external functions aimed at doing actions
on VFIO devices:
- mask VFIO IRQ
- get the active status of VFIO IRQ (active at interrupt
  controller level or masked by the level-sensitive automasking).
- change the automasked property and switch the IRQ handler
  (between automasked/ non automasked)

Their implementation is based on bus specific callbacks.

Note there is no way to discriminate between user-space
masking and automasked handler masking. As a consequence, is_active
will return true in case the IRQ was masked by the user-space.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

v5 -> v6:
- implementation now uses external ops
- prototype changed (index, start, count) and returns int

V4: creation
---
 drivers/vfio/vfio.c  | 39 +++++++++++++++++++++++++++++++++++++++
 include/linux/vfio.h | 16 ++++++++++++++++
 2 files changed, 55 insertions(+)

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 2fb29df..af6901e 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1527,6 +1527,45 @@ long vfio_external_check_extension(struct vfio_group *group, unsigned long arg)
 }
 EXPORT_SYMBOL_GPL(vfio_external_check_extension);
 
+int vfio_external_mask(struct vfio_device *vdev, unsigned index,
+			unsigned start, unsigned count)
+{
+	if (vdev->ops->external_ops &&
+		vdev->ops->external_ops->mask)
+		return vdev->ops->external_ops->mask(vdev->device_data,
+						     index, start, count);
+	else
+		return -ENXIO;
+}
+EXPORT_SYMBOL_GPL(vfio_external_mask);
+
+int vfio_external_is_active(struct vfio_device *vdev, unsigned index,
+			     unsigned start, unsigned count)
+{
+	if (vdev->ops->external_ops &&
+		vdev->ops->external_ops->is_active)
+		return vdev->ops->external_ops->is_active(vdev->device_data,
+							  index, start, count);
+	else
+		return -ENXIO;
+}
+EXPORT_SYMBOL_GPL(vfio_external_is_active);
+
+int vfio_external_set_automasked(struct vfio_device *vdev,
+				  unsigned index, unsigned start,
+				  unsigned count, bool automasked)
+{
+	if (vdev->ops->external_ops &&
+		vdev->ops->external_ops->set_automasked)
+		return vdev->ops->external_ops->set_automasked(
+							vdev->device_data,
+							index, start,
+							count, automasked);
+	else
+		return -ENXIO;
+}
+EXPORT_SYMBOL_GPL(vfio_external_set_automasked);
+
 /**
  * Module/class support
  */
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index d79e8a9..31d3c95 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -107,6 +107,22 @@ extern int vfio_external_user_iommu_id(struct vfio_group *group);
 extern long vfio_external_check_extension(struct vfio_group *group,
 					  unsigned long arg);
 
+extern int vfio_external_mask(struct vfio_device *vdev, unsigned index,
+			       unsigned start, unsigned count);
+/*
+ * returns whether the VFIO IRQ is active:
+ * true if not yet deactivated at interrupt controller level or if
+ * automasked (level sensitive IRQ). Unfortunately there is no way to
+ * discriminate between handler auto-masking and user-space masking
+ */
+extern int vfio_external_is_active(struct vfio_device *vdev,
+				    unsigned index, unsigned start,
+				    unsigned count);
+
+extern int vfio_external_set_automasked(struct vfio_device *vdev,
+					 unsigned index, unsigned start,
+					 unsigned count, bool automasked);
+
 struct pci_dev;
 #ifdef CONFIG_EEH
 extern void vfio_spapr_pci_eeh_open(struct pci_dev *pdev);
-- 
1.9.1

[RFC 07/17] KVM: arm: rename pause into power_off

[Eric Auger]

The kvm_vcpu_arch pause field is renamed into power_off to prepare
for the introduction of a new pause field.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

v4 -> v5:
- fix compilation issue on arm64 (add power_off field in kvm_host.h)
---
 arch/arm/include/asm/kvm_host.h   |  4 ++--
 arch/arm/kvm/arm.c                | 10 +++++-----
 arch/arm/kvm/psci.c               | 10 +++++-----
 arch/arm64/include/asm/kvm_host.h |  4 ++--
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index e896d2c..304004d 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -129,8 +129,8 @@ struct kvm_vcpu_arch {
 	 * here.
 	 */
 
-	/* Don't run the guest on this vcpu */
-	bool pause;
+	/* vcpu power-off state */
+	bool power_off;
 
 	/* IO related fields */
 	struct kvm_decode mmio_decode;
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index bcdf799..7537e68 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -475,7 +475,7 @@ static void vcpu_pause(struct kvm_vcpu *vcpu)
 {
 	wait_queue_head_t *wq = kvm_arch_vcpu_wq(vcpu);
 
-	wait_event_interruptible(*wq, !vcpu->arch.pause);
+	wait_event_interruptible(*wq, !vcpu->arch.power_off);
 }
 
 static int kvm_vcpu_initialized(struct kvm_vcpu *vcpu)
@@ -525,7 +525,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
 
 		update_vttbr(vcpu->kvm);
 
-		if (vcpu->arch.pause)
+		if (vcpu->arch.power_off)
 			vcpu_pause(vcpu);
 
 		/*
@@ -766,12 +766,12 @@ static int kvm_arch_vcpu_ioctl_vcpu_init(struct kvm_vcpu *vcpu,
 	vcpu_reset_hcr(vcpu);
 
 	/*
-	 * Handle the "start in power-off" case by marking the VCPU as paused.
+	 * Handle the "start in power-off" case.
 	 */
 	if (test_bit(KVM_ARM_VCPU_POWER_OFF, vcpu->arch.features))
-		vcpu->arch.pause = true;
+		vcpu->arch.power_off = true;
 	else
-		vcpu->arch.pause = false;
+		vcpu->arch.power_off = false;
 
 	return 0;
 }
diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
index 4b94b51..134971a 100644
--- a/arch/arm/kvm/psci.c
+++ b/arch/arm/kvm/psci.c
@@ -63,7 +63,7 @@ static unsigned long kvm_psci_vcpu_suspend(struct kvm_vcpu *vcpu)
 
 static void kvm_psci_vcpu_off(struct kvm_vcpu *vcpu)
 {
-	vcpu->arch.pause = true;
+	vcpu->arch.power_off = true;
 }
 
 static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
@@ -87,7 +87,7 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
 	 */
 	if (!vcpu)
 		return PSCI_RET_INVALID_PARAMS;
-	if (!vcpu->arch.pause) {
+	if (!vcpu->arch.power_off) {
 		if (kvm_psci_version(source_vcpu) != KVM_ARM_PSCI_0_1)
 			return PSCI_RET_ALREADY_ON;
 		else
@@ -115,7 +115,7 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
 	 * the general puspose registers are undefined upon CPU_ON.
 	 */
 	*vcpu_reg(vcpu, 0) = context_id;
-	vcpu->arch.pause = false;
+	vcpu->arch.power_off = false;
 	smp_mb();		/* Make sure the above is visible */
 
 	wq = kvm_arch_vcpu_wq(vcpu);
@@ -152,7 +152,7 @@ static unsigned long kvm_psci_vcpu_affinity_info(struct kvm_vcpu *vcpu)
 	kvm_for_each_vcpu(i, tmp, kvm) {
 		mpidr = kvm_vcpu_get_mpidr_aff(tmp);
 		if (((mpidr & target_affinity_mask) == target_affinity) &&
-		    !tmp->arch.pause) {
+		    !tmp->arch.power_off) {
 			return PSCI_0_2_AFFINITY_LEVEL_ON;
 		}
 	}
@@ -175,7 +175,7 @@ static void kvm_prepare_system_event(struct kvm_vcpu *vcpu, u32 type)
 	 * re-initialized.
 	 */
 	kvm_for_each_vcpu(i, tmp, vcpu->kvm) {
-		tmp->arch.pause = true;
+		tmp->arch.power_off = true;
 		kvm_vcpu_kick(tmp);
 	}
 
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 2709db2..009da6b 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -122,8 +122,8 @@ struct kvm_vcpu_arch {
 	 * here.
 	 */
 
-	/* Don't run the guest */
-	bool pause;
+	/* vcpu power-off state */
+	bool power_off;
 
 	/* IO related fields */
 	struct kvm_decode mmio_decode;
-- 
1.9.1

[RFC 08/17] kvm: arm/arm64: implement kvm_arm_[halt,resume]_guest

[Eric Auger]

On halt, the guest is forced to exit and prevented from being
re-entered. This is synchronous.

Those two operations will be needed for IRQ forwarding setting.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

RFC:
- rename the function and this latter becomes static
- remove __KVM_HAVE_ARCH_HALT_GUEST

v4 -> v5: add arm64 support
- also defines __KVM_HAVE_ARCH_HALT_GUEST for arm64
- add pause field
---
 arch/arm/include/asm/kvm_host.h   |  3 +++
 arch/arm/kvm/arm.c                | 32 +++++++++++++++++++++++++++++---
 arch/arm64/include/asm/kvm_host.h |  3 +++
 3 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 304004d..899ae27 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -132,6 +132,9 @@ struct kvm_vcpu_arch {
 	/* vcpu power-off state */
 	bool power_off;
 
+	/* Don't run the guest */
+	bool pause;
+
 	/* IO related fields */
 	struct kvm_decode mmio_decode;
 
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 7537e68..4be6715 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -471,11 +471,36 @@ bool kvm_arch_intc_initialized(struct kvm *kvm)
 	return vgic_initialized(kvm);
 }
 
+static void kvm_arm_halt_guest(struct kvm *kvm)
+{
+	int i;
+	struct kvm_vcpu *vcpu;
+
+	kvm_for_each_vcpu(i, vcpu, kvm)
+		vcpu->arch.pause = true;
+	force_vm_exit(cpu_all_mask);
+}
+
+static void kvm_arm_resume_guest(struct kvm *kvm)
+{
+	int i;
+	struct kvm_vcpu *vcpu;
+
+	kvm_for_each_vcpu(i, vcpu, kvm) {
+		wait_queue_head_t *wq = kvm_arch_vcpu_wq(vcpu);
+
+		vcpu->arch.pause = false;
+		wake_up_interruptible(wq);
+	}
+}
+
+
 static void vcpu_pause(struct kvm_vcpu *vcpu)
 {
 	wait_queue_head_t *wq = kvm_arch_vcpu_wq(vcpu);
 
-	wait_event_interruptible(*wq, !vcpu->arch.power_off);
+	wait_event_interruptible(*wq, ((!vcpu->arch.power_off) &&
+				       (!vcpu->arch.pause)));
 }
 
 static int kvm_vcpu_initialized(struct kvm_vcpu *vcpu)
@@ -525,7 +550,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
 
 		update_vttbr(vcpu->kvm);
 
-		if (vcpu->arch.power_off)
+		if (vcpu->arch.power_off || vcpu->arch.pause)
 			vcpu_pause(vcpu);
 
 		/*
@@ -551,7 +576,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
 			run->exit_reason = KVM_EXIT_INTR;
 		}
 
-		if (ret <= 0 || need_new_vmid_gen(vcpu->kvm)) {
+		if (ret <= 0 || need_new_vmid_gen(vcpu->kvm) ||
+		    vcpu->arch.pause) {
 			local_irq_enable();
 			preempt_enable();
 			kvm_vgic_sync_hwstate(vcpu);
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 009da6b..69e3785 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -125,6 +125,9 @@ struct kvm_vcpu_arch {
 	/* vcpu power-off state */
 	bool power_off;
 
+	/* Don't run the guest */
+	bool pause;
+
 	/* IO related fields */
 	struct kvm_decode mmio_decode;
 
-- 
1.9.1

[RFC 09/17] bypass: IRQ bypass manager proto by Alex

[Eric Auger]

From: Alex Williamson <alex.williamson@redhat.com>

There are plenty of details to be filled in, but I think the basics
looks something like the code below.  The IRQ bypass manager just
defines a pair of structures, one for interrupt producers and one for
interrupt consumers.  I'm certain that we'll need more callbacks than
I've defined below, but figuring out what those should be for the best
abstraction is the hardest part of this idea.  The manager provides both
registration and de-registration interfaces for both types of objects
and keeps lists for each, protected by a lock.  The manager doesn't even
really need to know what the match token is, but I assume for our
purposes it will be an eventfd_ctx.

On the vfio side, the producer struct would be embedded in the
vfio_pci_irq_ctx struct.  KVM would probably embed the consumer struct
in _irqfd.  As I've coded below, the IRQ bypass manager calls the
consumer callbacks, so the producer struct would need fields or
callbacks to provide the consumer the info it needs.  AIUI the Posted
Interrupt model, VFIO only needs to provide data to the consumer.  For
IRQ Forwarding, I think the producer needs to be informed when bypass is
active to model the incoming interrupt as edge vs level.

I've prototyped the base IRQ bypass manager here as static, but I don't
see any reason it couldn't be a module that's loaded by dependency when
either vfio-pci or kvm-intel is loaded (or other producer/consumer
objects).

Is this a reasonable starting point to craft the additional fields and
callbacks and interaction of who calls who that we need to support
Posted Interrupts and IRQ Forwarding?  Is the AMD version of this still
alive?  Thanks,

Alex
---
 arch/x86/kvm/Kconfig              |   1 +
 drivers/vfio/pci/Kconfig          |   1 +
 drivers/vfio/pci/vfio_pci_intrs.c |   6 ++
 include/linux/irqbypass.h         |  23 ++++++++
 kernel/irq/Kconfig                |   3 +
 kernel/irq/Makefile               |   1 +
 kernel/irq/bypass.c               | 116 ++++++++++++++++++++++++++++++++++++++
 virt/kvm/eventfd.c                |   4 ++
 8 files changed, 155 insertions(+)
 create mode 100644 include/linux/irqbypass.h
 create mode 100644 kernel/irq/bypass.c

diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index d8a1d56..86d0d77 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -61,6 +61,7 @@ config KVM_INTEL
 	depends on KVM
 	# for perf_guest_get_msrs():
 	depends on CPU_SUP_INTEL
+	select IRQ_BYPASS_MANAGER
 	---help---
 	  Provides support for KVM on Intel processors equipped with the VT
 	  extensions.
diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
index 579d83b..02912f1 100644
--- a/drivers/vfio/pci/Kconfig
+++ b/drivers/vfio/pci/Kconfig
@@ -2,6 +2,7 @@ config VFIO_PCI
 	tristate "VFIO support for PCI devices"
 	depends on VFIO && PCI && EVENTFD
 	select VFIO_VIRQFD
+	select IRQ_BYPASS_MANAGER
 	help
 	  Support for the PCI VFIO bus driver.  This is required to make
 	  use of PCI drivers using the VFIO framework.
diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
index 1f577b4..4e053be 100644
--- a/drivers/vfio/pci/vfio_pci_intrs.c
+++ b/drivers/vfio/pci/vfio_pci_intrs.c
@@ -181,6 +181,7 @@ static int vfio_intx_set_signal(struct vfio_pci_device *vdev, int fd)
 
 	if (vdev->ctx[0].trigger) {
 		free_irq(pdev->irq, vdev);
+		/* irq_bypass_unregister_producer(); */
 		kfree(vdev->ctx[0].name);
 		eventfd_ctx_put(vdev->ctx[0].trigger);
 		vdev->ctx[0].trigger = NULL;
@@ -214,6 +215,8 @@ static int vfio_intx_set_signal(struct vfio_pci_device *vdev, int fd)
 		return ret;
 	}
 
+	/* irq_bypass_register_producer(); */
+
 	/*
 	 * INTx disable will stick across the new irq setup,
 	 * disable_irq won't.
@@ -319,6 +322,7 @@ static int vfio_msi_set_vector_signal(struct vfio_pci_device *vdev,
 
 	if (vdev->ctx[vector].trigger) {
 		free_irq(irq, vdev->ctx[vector].trigger);
+		/* irq_bypass_unregister_producer(); */
 		kfree(vdev->ctx[vector].name);
 		eventfd_ctx_put(vdev->ctx[vector].trigger);
 		vdev->ctx[vector].trigger = NULL;
@@ -360,6 +364,8 @@ static int vfio_msi_set_vector_signal(struct vfio_pci_device *vdev,
 		return ret;
 	}
 
+	/* irq_bypass_register_producer(); */
+
 	vdev->ctx[vector].trigger = trigger;
 
 	return 0;
diff --git a/include/linux/irqbypass.h b/include/linux/irqbypass.h
new file mode 100644
index 0000000..718508e
--- /dev/null
+++ b/include/linux/irqbypass.h
@@ -0,0 +1,23 @@
+#ifndef IRQBYPASS_H
+#define IRQBYPASS_H
+
+#include <linux/list.h>
+
+struct irq_bypass_producer {
+	struct list_head node;
+	void *token;
+	/* TBD */
+};
+
+struct irq_bypass_consumer {
+	struct list_head node;
+	void *token;
+	void (*add_producer)(struct irq_bypass_producer *);
+	void (*del_producer)(struct irq_bypass_producer *);
+};
+
+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/kernel/irq/Kconfig b/kernel/irq/Kconfig
index 9a76e3b..4502cdc 100644
--- a/kernel/irq/Kconfig
+++ b/kernel/irq/Kconfig
@@ -100,4 +100,7 @@ config SPARSE_IRQ
 
 	  If you don't know what to do here, say N.
 
+config IRQ_BYPASS_MANAGER
+	bool
+
 endmenu
diff --git a/kernel/irq/Makefile b/kernel/irq/Makefile
index d121235..a30ed77 100644
--- a/kernel/irq/Makefile
+++ b/kernel/irq/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_PROC_FS) += proc.o
 obj-$(CONFIG_GENERIC_PENDING_IRQ) += migration.o
 obj-$(CONFIG_PM_SLEEP) += pm.o
 obj-$(CONFIG_GENERIC_MSI_IRQ) += msi.o
+obj-$(CONFIG_IRQ_BYPASS_MANAGER) += bypass.o
diff --git a/kernel/irq/bypass.c b/kernel/irq/bypass.c
new file mode 100644
index 0000000..5d0f92b
--- /dev/null
+++ b/kernel/irq/bypass.c
@@ -0,0 +1,116 @@
+/*
+ * IRQ offload/bypass manager
+ *
+ * Various virtualization hardware acceleration techniques allow bypassing
+ * or offloading interrupts receieved from devices around the host kernel.
+ * Posted Interrupts on Intel VT-d systems can allow interrupts to be
+ * recieved directly by a virtual machine.  ARM IRQ Forwarding can allow
+ * level triggered device interrupts to be de-asserted directly by the VM.
+ * 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>
+
+static LIST_HEAD(producers);
+static LIST_HEAD(consumers);
+static DEFINE_MUTEX(lock);
+
+int irq_bypass_register_producer(struct irq_bypass_producer *producer)
+{
+	struct irq_bypass_producer *tmp;
+	struct irq_bypass_consumer *consumer;
+	int ret = 0;
+
+	mutex_lock(&lock);
+
+	list_for_each_entry(tmp, &producers, node) {
+		if (tmp->token == producer->token) {
+			ret = -EINVAL;
+			goto unlock;
+		}
+	}
+
+	list_add(&producer->node, &producers);
+
+	list_for_each_entry(consumer, &consumers, node) {
+		if (consumer->token == producer->token) {
+			consumer->add_producer(producer);
+			break;
+		}
+	}
+unlock:
+	mutex_unlock(&lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(irq_bypass_register_producer);
+
+void irq_bypass_unregister_producer(struct irq_bypass_producer *producer)
+{
+	struct irq_bypass_consumer *consumer;
+
+	mutex_lock(&lock);
+
+	list_for_each_entry(consumer, &consumers, node) {
+		if (consumer->token == producer->token) {
+			consumer->del_producer(producer);
+			break;
+		}
+	}
+
+	list_del(&producer->node);
+
+	mutex_unlock(&lock);
+}
+EXPORT_SYMBOL_GPL(irq_bypass_unregister_producer);
+
+int irq_bypass_register_consumer(struct irq_bypass_consumer *consumer)
+{
+	struct irq_bypass_consumer *tmp;
+	struct irq_bypass_producer *producer;
+	int ret = 0;
+
+	mutex_lock(&lock);
+
+	list_for_each_entry(tmp, &consumers, node) {
+		if (tmp->token == consumer->token) {
+			ret = -EINVAL;
+			goto unlock;
+		}
+	}
+
+	list_add(&consumer->node, &consumers);
+
+	list_for_each_entry(producer, &producers, node) {
+		if (producer->token == consumer->token) {
+			consumer->add_producer(producer);
+			break;
+		}
+	}
+unlock:
+	mutex_unlock(&lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(irq_bypass_register_consumer);
+
+void irq_bypass_unregister_consumer(struct irq_bypass_consumer *consumer)
+{
+	struct irq_bypass_producer *producer;
+
+	mutex_lock(&lock);
+
+	list_for_each_entry(producer, &producers, node) {
+		if (producer->token == consumer->token) {
+			consumer->del_producer(producer);
+			break;
+		}
+	}
+
+	list_del(&consumer->node);
+
+	mutex_unlock(&lock);
+}
+EXPORT_SYMBOL_GPL(irq_bypass_unregister_consumer);
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index 9ff4193..f3da161 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -429,6 +429,8 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)
 	 */
 	fdput(f);
 
+	/* irq_bypass_register_consumer(); */
+
 	return 0;
 
 fail:
@@ -528,6 +530,8 @@ kvm_irqfd_deassign(struct kvm *kvm, struct kvm_irqfd *args)
 	struct _irqfd *irqfd, *tmp;
 	struct eventfd_ctx *eventfd;
 
+	/* irq_bypass_unregister_consumer() */
+
 	eventfd = eventfd_ctx_fdget(args->fd);
 	if (IS_ERR(eventfd))
 		return PTR_ERR(eventfd);
-- 
1.9.1

[RFC 10/17] KVM: arm: select IRQ_BYPASS_MANAGER

[Eric Auger]

Select IRQ_BYPASS_MANAGER when CONFIG_KVM is set

Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
 arch/arm/kvm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
index bfb915d..7d38d25 100644
--- a/arch/arm/kvm/Kconfig
+++ b/arch/arm/kvm/Kconfig
@@ -31,6 +31,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.
-- 
1.9.1

[RFC 11/17] VFIO: platform: select IRQ_BYPASS_MANAGER

[Eric Auger]

Select IRQ_BYPASS_MANAGER when CONFIG_VFIO_PLATFORM is set

Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
 drivers/vfio/platform/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig
index bb30128..c2f3dce 100644
--- a/drivers/vfio/platform/Kconfig
+++ b/drivers/vfio/platform/Kconfig
@@ -2,6 +2,7 @@ config VFIO_PLATFORM
 	tristate "VFIO support for platform devices"
 	depends on VFIO && EVENTFD && (ARM || ARM64)
 	select VFIO_VIRQFD
+	select IRQ_BYPASS_MANAGER
 	help
 	  Support for platform devices with VFIO. This is required to make
 	  use of platform devices present on the system using the VFIO
-- 
1.9.1

[RFC 12/17] irq: bypass: Extend skeleton for ARM forwarding control

[Eric Auger]

- [add,del]_[consumer,producer] updated to takes both the consumer and
  producer handles. This is requested to combine info from both,
  typically to link the source irq owned by the producer with the gsi
  owned by the consumer (forwarded IRQ setup).
- new functions are added: [stop,resume]_[consumer, producer]. Those are
  needed for forwarding since the state change requires to entermingle
  actions at consumer, producer.
- On handshake, we now call connect, disconnect which features the more
  complex sequence.
- new fields are added on producer side: linux irq, vfio_device handle,
  active which reflects whether the source is active (at interrupt
  controller level or at VFIO level - automasked -) and finally an
  opaque pointer which will be used to point to the vfio_platform_device
  in this series.
- new fields on consumer side: the kvm handle, the gsi

Integration of posted interrupt series will help to refine those choices

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

- connect/disconnect could become a cb too. For forwarding it may make
  sense to have failure at connection: this would happen when the physical
  IRQ is either active at irqchip level or VFIO masked. This means some
  of the cb should return an error and this error management could be
  prod/cons specific. Where to attach the connect/disconnect cb: to the
  cons or prod, to both?
- Hence may be sensible to do the list_add only if connect returns 0
- disconnect would not be allowed to fail.
---
 include/linux/irqbypass.h | 26 ++++++++++++++++++++++---
 kernel/irq/bypass.c       | 48 +++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 67 insertions(+), 7 deletions(-)

diff --git a/include/linux/irqbypass.h b/include/linux/irqbypass.h
index 718508e..591ae3f 100644
--- a/include/linux/irqbypass.h
+++ b/include/linux/irqbypass.h
@@ -3,17 +3,37 @@
 
 #include <linux/list.h>
 
+struct vfio_device;
+struct irq_bypass_consumer;
+struct kvm;
+
 struct irq_bypass_producer {
 	struct list_head node;
 	void *token;
-	/* TBD */
+	unsigned int irq; /* host physical irq */
+	struct vfio_device *vdev; /* vfio device that requested irq */
+	/* is irq active at irqchip or VFIO masked? */
+	bool active;
+	void *opaque;
+	void (*stop_producer)(struct irq_bypass_producer *);
+	void (*resume_producer)(struct irq_bypass_producer *);
+	void (*add_consumer)(struct irq_bypass_producer *,
+			     struct irq_bypass_consumer *);
+	void (*del_consumer)(struct irq_bypass_producer *,
+			     struct irq_bypass_consumer *);
 };
 
 struct irq_bypass_consumer {
 	struct list_head node;
 	void *token;
-	void (*add_producer)(struct irq_bypass_producer *);
-	void (*del_producer)(struct irq_bypass_producer *);
+	unsigned int gsi;		/* the guest gsi */
+	struct kvm *kvm;
+	void (*stop_consumer)(struct irq_bypass_consumer *);
+	void (*resume_consumer)(struct irq_bypass_consumer *);
+	void (*add_producer)(struct irq_bypass_consumer *,
+			     struct irq_bypass_producer *);
+	void (*del_producer)(struct irq_bypass_consumer *,
+			     struct irq_bypass_producer *);
 };
 
 int irq_bypass_register_producer(struct irq_bypass_producer *);
diff --git a/kernel/irq/bypass.c b/kernel/irq/bypass.c
index 5d0f92b..fb31fef 100644
--- a/kernel/irq/bypass.c
+++ b/kernel/irq/bypass.c
@@ -19,6 +19,46 @@ static LIST_HEAD(producers);
 static LIST_HEAD(consumers);
 static DEFINE_MUTEX(lock);
 
+/* lock must be hold when calling connect */
+static void connect(struct irq_bypass_producer *prod,
+		    struct irq_bypass_consumer *cons)
+{
+	pr_info("++++ %s prod(%d) -> cons(%d)\n",
+		__func__, prod->irq, cons->gsi);
+	if (prod->stop_producer)
+		prod->stop_producer(prod);
+	if (cons->stop_consumer)
+		cons->stop_consumer(cons);
+	if (prod->add_consumer)
+		prod->add_consumer(prod, cons);
+	if (cons->add_producer)
+		cons->add_producer(cons, prod);
+	if (cons->resume_consumer)
+		cons->resume_consumer(cons);
+	if (prod->resume_producer)
+		prod->resume_producer(prod);
+}
+
+/* lock must be hold when calling disconnect */
+static void disconnect(struct irq_bypass_producer *prod,
+		       struct irq_bypass_consumer *cons)
+{
+	pr_info("---- %s prod(%d) -> cons(%d)\n",
+		__func__, prod->irq, cons->gsi);
+	if (prod->stop_producer)
+		prod->stop_producer(prod);
+	if (cons->stop_consumer)
+		cons->stop_consumer(cons);
+	if (cons->del_producer)
+		cons->del_producer(cons, prod);
+	if (prod->del_consumer)
+		prod->del_consumer(prod, cons);
+	if (cons->resume_consumer)
+		cons->resume_consumer(cons);
+	if (prod->resume_producer)
+		prod->resume_producer(prod);
+}
+
 int irq_bypass_register_producer(struct irq_bypass_producer *producer)
 {
 	struct irq_bypass_producer *tmp;
@@ -38,7 +78,7 @@ int irq_bypass_register_producer(struct irq_bypass_producer *producer)
 
 	list_for_each_entry(consumer, &consumers, node) {
 		if (consumer->token == producer->token) {
-			consumer->add_producer(producer);
+			connect(producer, consumer);
 			break;
 		}
 	}
@@ -56,7 +96,7 @@ void irq_bypass_unregister_producer(struct irq_bypass_producer *producer)
 
 	list_for_each_entry(consumer, &consumers, node) {
 		if (consumer->token == producer->token) {
-			consumer->del_producer(producer);
+			disconnect(producer, consumer);
 			break;
 		}
 	}
@@ -86,7 +126,7 @@ int irq_bypass_register_consumer(struct irq_bypass_consumer *consumer)
 
 	list_for_each_entry(producer, &producers, node) {
 		if (producer->token == consumer->token) {
-			consumer->add_producer(producer);
+			connect(producer, consumer);
 			break;
 		}
 	}
@@ -104,7 +144,7 @@ void irq_bypass_unregister_consumer(struct irq_bypass_consumer *consumer)
 
 	list_for_each_entry(producer, &producers, node) {
 		if (producer->token == consumer->token) {
-			consumer->del_producer(producer);
+			disconnect(producer, consumer);
 			break;
 		}
 	}
-- 
1.9.1

[RFC 13/17] KVM: introduce kvm_arch functions for IRQ bypass

[Eric Auger]

This patch introduces
- kvm_arch_add_producer
- kvm_arch_del_producer
- kvm_arch_stop_consumer
- kvm_arch_resume_consumer

They make possible to specialize the KVM IRQ bypass consumer.

Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
 include/linux/kvm_host.h | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 9564fd7..8e981e9 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>
@@ -1133,5 +1134,31 @@ static inline void kvm_vcpu_set_dy_eligible(struct kvm_vcpu *vcpu, bool val)
 {
 }
 #endif /* CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT */
+
+#ifdef CONFIG_IRQ_BYPASS_MANAGER
+
+void kvm_arch_add_producer(struct irq_bypass_consumer *,
+			   struct irq_bypass_producer *);
+void kvm_arch_del_producer(struct irq_bypass_consumer *,
+			   struct irq_bypass_producer *);
+void kvm_arch_stop_consumer(struct irq_bypass_consumer *);
+void kvm_arch_resume_consumer(struct irq_bypass_consumer *);
+
+#else
+void kvm_arch_add_producer(struct irq_bypass_consumer *,
+			   struct irq_bypass_producer *)
+{
+}
+void kvm_arch_del_producer(struct irq_bypass_consumer *,
+			   struct irq_bypass_producer *)
+{
+}
+void kvm_arch_stop_consumer(struct irq_bypass_consumer *)
+{
+}
+void kvm_arch_resume_consumer(struct irq_bypass_consumer *)
+{
+}
+#endif /* CONFIG_IRQ_BYPASS_MANAGER */
 #endif
 
-- 
1.9.1

[RFC 14/17] KVM: arm/arm64: vgic: forwarding control

[Eric Auger]

Implements kvm_vgic_[set|unset]_forward.

Handle low-level VGIC programming: physical IRQ/guest IRQ mapping,
list register cleanup, VGIC state machine. Also interacts with
the irqchip.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

bypass rfc:
- rename kvm_arch_{set|unset}_forward into
  kvm_vgic_{set|unset}_forward. Remove __KVM_HAVE_ARCH_HALT_GUEST.
  The function is bound to be called by ARM code only.

v4 -> v5:
- fix arm64 compilation issues, ie. also defines
  __KVM_HAVE_ARCH_HALT_GUEST for arm64

v3 -> v4:
- code originally located in kvm_vfio_arm.c
- kvm_arch_vfio_{set|unset}_forward renamed into
  kvm_arch_{set|unset}_forward
- split into 2 functions (set/unset) since unset does not fail anymore
- unset can be invoked at whatever time. Extra care is taken to handle
  transition in VGIC state machine, LR cleanup, ...

v2 -> v3:
- renaming of kvm_arch_set_fwd_state into kvm_arch_vfio_set_forward
- takes a bool arg instead of kvm_fwd_irq_action enum
- removal of KVM_VFIO_IRQ_CLEANUP
- platform device check now happens here
- more precise errors returned
- irq_eoi handled externally to this patch (VGIC)
- correct enable_irq bug done twice
- reword the commit message
- correct check of platform_bus_type
- use raw_spin_lock_irqsave and check the validity of the handler
---
 include/kvm/arm_vgic.h |   7 ++
 virt/kvm/arm/vgic.c    | 195 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 202 insertions(+)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 5d47d60..93b379f 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -353,6 +353,13 @@ int vgic_unmap_phys_irq(struct kvm_vcpu *vcpu, struct irq_phys_map *map);
 bool vgic_get_phys_irq_active(struct irq_phys_map *map);
 void vgic_set_phys_irq_active(struct irq_phys_map *map, bool active);
 
+int kvm_vgic_set_forward(struct kvm *kvm,
+			 unsigned int host_irq, unsigned int guest_irq);
+
+void kvm_vgic_unset_forward(struct kvm *kvm,
+			    unsigned int host_irq, unsigned int guest_irq,
+			    bool *active);
+
 #define irqchip_in_kernel(k)	(!!((k)->arch.vgic.in_kernel))
 #define vgic_initialized(k)	(!!((k)->arch.vgic.nr_cpus))
 #define vgic_ready(k)		((k)->arch.vgic.ready)
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index eef35d9..9efc839 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -2402,3 +2402,198 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
 {
 	return 0;
 }
+
+/**
+ * kvm_vgic_set_forward - Set IRQ forwarding
+ *
+ * @kvm: handle to the VM
+ * @host_irq: physical IRQ number
+ * @guest_irq: virtual IRQ number
+ *
+ * This function is supposed to be called only if the IRQ
+ * is not in progress: ie. not active at GIC level and not
+ * currently under injection in the KVM. The physical IRQ must
+ * also be disabled and the guest must have been exited and
+ * prevented from being re-entered.
+ */
+int kvm_vgic_set_forward(struct kvm *kvm,
+			 unsigned int host_irq,
+			 unsigned int guest_irq)
+{
+	struct irq_desc *desc = irq_to_desc(host_irq);
+	struct irq_phys_map *map = NULL;
+	struct irq_data *d;
+	unsigned long flags;
+	struct kvm_vcpu *vcpu = kvm_get_vcpu(kvm, 0);
+	int spi_id = guest_irq + VGIC_NR_PRIVATE_IRQS;
+	struct vgic_dist *dist = &kvm->arch.vgic;
+
+	kvm_debug("%s host_irq=%d guest_irq=%d\n",
+		__func__, host_irq, guest_irq);
+
+	if (!vcpu)
+		return 0;
+
+	spin_lock(&dist->lock);
+
+	raw_spin_lock_irqsave(&desc->lock, flags);
+	d = &desc->irq_data;
+	irqd_set_irq_forwarded(d);
+	/*
+	 * next physical IRQ will be be handled as forwarded
+	 * by the host (priority drop only)
+	 */
+
+	raw_spin_unlock_irqrestore(&desc->lock, flags);
+
+	/*
+	 * need to release the dist spin_lock here since
+	 * vgic_map_phys_irq can sleep
+	 */
+	spin_unlock(&dist->lock);
+	map = vgic_map_phys_irq(vcpu, spi_id, host_irq, false);
+	/*
+	 * next guest_irq injection will be considered as
+	 * forwarded and next flush will program LR
+	 * without maintenance IRQ but with HW bit set
+	 */
+	return !map;
+}
+
+/**
+ * kvm_vgic_unset_forward - Unset IRQ forwarding
+ *
+ * @kvm: handle to the VM
+ * @host_irq: physical IRQ number
+ * @guest_irq: virtual IRQ number
+ * @active: returns whether the physical IRQ is active
+ *
+ * This function must be called when the host_irq is disabled
+ * and guest has been exited and prevented from being re-entered.
+ *
+ */
+void kvm_vgic_unset_forward(struct kvm *kvm,
+			    unsigned int host_irq,
+			    unsigned int guest_irq,
+			    bool *active)
+{
+	struct kvm_vcpu *vcpu = kvm_get_vcpu(kvm, 0);
+	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
+	struct vgic_dist *dist = &kvm->arch.vgic;
+	int ret, lr;
+	struct vgic_lr vlr;
+	struct irq_desc *desc = irq_to_desc(host_irq);
+	struct irq_data *d;
+	unsigned long flags;
+	int spi_id = guest_irq + VGIC_NR_PRIVATE_IRQS;
+	struct irq_chip *chip;
+	bool queued, needs_deactivate = true;
+	struct irq_phys_map *map;
+
+	kvm_debug("%s host_irq=%d guest_irq=%d\n",
+		__func__, host_irq, guest_irq);
+
+	spin_lock(&dist->lock);
+
+	irq_get_irqchip_state(host_irq, IRQCHIP_STATE_ACTIVE, active);
+
+	if (!vcpu)
+		goto out;
+
+	map = vgic_irq_map_search(vcpu, spi_id);
+	BUG_ON(!map);
+	ret = vgic_unmap_phys_irq(vcpu, map);
+	BUG_ON(ret);
+	/*
+	 * subsequent update_irq_pending or flush will handle this
+	 * irq as forwarded
+	 */
+
+	if (likely(!(*active))) {
+		/*
+		 * the IRQ was not active. It may happen the handle_fasteoi_irq
+		 * is entered afterwards due to lazy disable_irq. If this
+		 * happens the IRQ will be deactivated and never be injected.
+		 * let's simply prepare the states for subsequent non forwarded
+		 * injection
+		 */
+		vgic_dist_irq_clear_level(vcpu, spi_id);
+		vgic_dist_irq_clear_pending(vcpu, spi_id);
+		vgic_irq_clear_queued(vcpu, spi_id);
+		needs_deactivate = false;
+		goto out;
+	}
+
+	/* is there any list register with valid state? */
+	lr = vgic_cpu->vgic_irq_lr_map[spi_id];
+	queued = false;
+	if (lr != LR_EMPTY) {
+		vlr = vgic_get_lr(vcpu, lr);
+		if (vlr.state & LR_STATE_MASK)
+			queued = true;
+	}
+
+	if (!queued) {
+		vgic_irq_clear_queued(vcpu, spi_id);
+		if (vgic_dist_irq_is_pending(vcpu, spi_id)) {
+			/*
+			 * IRQ is injected but not yet queued. LR will be
+			 * written with EOI_INT and process_maintenance will
+			 * reset the states: queued, level(resampler). Pending
+			 * will be reset on flush.
+			 */
+			vgic_dist_irq_set_level(vcpu, spi_id);
+		} else {
+			/*
+			 * We are before the injection (update_irq_pending).
+			 * This is the most tricky window. Due to the usage of
+			 * disable_irq in generic unforward part (mandated by
+			 * resamplefd unmask VFIO integration), there is a risk
+			 * the fasteoi handler returns without calling the VFIO
+			 * handler and deactivates the physical IRQ (lazy
+			 * disable). Hence we cannot know whether the IRQ will
+			 * ever be injected. The only solution found is to do as
+			 * if the IRQ were not active. The active parameter
+			 * typically is used by the caller to know whether
+			 * it needs to mask * the IRQ. If not duly masked,
+			 * another physical IRQ may hit again while the previous
+			 * virtual IRQ is in progress. Update_irq_pending
+			 * validate_injection will prevent this injection.
+			 */
+			vgic_dist_irq_clear_level(vcpu, spi_id);
+			*active = false;
+		}
+		goto out;
+	}
+
+	/*
+	 * the virtual IRQ is queued and a valid LR exists, let's patch it for
+	 * EOI maintenance
+	 */
+	vlr.state |= LR_EOI_INT;
+	vgic_set_lr(vcpu, lr, vlr);
+	/*
+	 * we expect a maintenance IRQ which will reset the
+	 * queued, pending and level states
+	 */
+	vgic_dist_irq_set_level(vcpu, spi_id);
+	vgic_dist_irq_set_pending(vcpu, spi_id);
+	vgic_irq_set_queued(vcpu, spi_id);
+
+out:
+
+	raw_spin_lock_irqsave(&desc->lock, flags);
+	d = irq_desc_get_irq_data(desc);
+	if (needs_deactivate) {
+		chip = irq_data_get_irq_chip(d);
+		chip->irq_eoi(d);
+	}
+	irqd_clr_irq_forwarded(d);
+	/* next occurrence will be deactivated by the host */
+	raw_spin_unlock_irqrestore(&desc->lock, flags);
+
+	*active = *active && vcpu;
+
+	spin_unlock(&dist->lock);
+}
+
-- 
1.9.1

[RFC 15/17] KVM: arm/arm64: implement IRQ bypass consumer functions

[Eric Auger]

- kvm_arch_add_producer: perform VGIC/irqchip settings for forwarding
- kvm_arch_del_producer: same for inverse operation
- kvm_arch_stop_consumer: halt guest execution
- kvm_arch_resume_consumer resume guest execution

Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
 arch/arm/kvm/arm.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 4be6715..f9b9b1e 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -1146,6 +1146,28 @@ struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr)
 	return NULL;
 }
 
+void kvm_arch_add_producer(struct irq_bypass_consumer *cons,
+			   struct irq_bypass_producer *prod)
+{
+	kvm_vgic_set_forward(cons->kvm, prod->irq, cons->gsi);
+}
+void kvm_arch_del_producer(struct irq_bypass_consumer *cons,
+			   struct irq_bypass_producer *prod)
+{
+	kvm_vgic_unset_forward(cons->kvm, prod->irq, cons->gsi,
+			       &prod->active);
+}
+
+void kvm_arch_stop_consumer(struct irq_bypass_consumer *cons)
+{
+	kvm_arm_halt_guest(cons->kvm);
+}
+
+void kvm_arch_resume_consumer(struct irq_bypass_consumer *cons)
+{
+	kvm_arm_resume_guest(cons->kvm);
+}
+
 /**
  * Initialize Hyp-mode and memory mappings on all CPUs.
  */
-- 
1.9.1

[RFC 16/17] KVM: eventfd: add irq bypass consumer management

[Eric Auger]

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

Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
 virt/kvm/eventfd.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index f3da161..425a47b 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -34,6 +34,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>
@@ -93,6 +94,7 @@ struct _irqfd {
 	struct list_head list;
 	poll_table pt;
 	struct work_struct shutdown;
+	struct irq_bypass_consumer *cons;
 };
 
 static struct workqueue_struct *irqfd_cleanup_wq;
@@ -429,7 +431,21 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)
 	 */
 	fdput(f);
 
-	/* irq_bypass_register_consumer(); */
+	irqfd->cons = kzalloc(sizeof(struct irq_bypass_consumer),
+			      GFP_KERNEL);
+	if (!irqfd->cons) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+	irqfd->cons->token = (void *)irqfd->eventfd;
+	irqfd->cons->gsi = irqfd->gsi;
+	irqfd->cons->kvm = kvm;
+	irqfd->cons->add_producer = kvm_arch_add_producer;
+	irqfd->cons->del_producer = kvm_arch_del_producer;
+	irqfd->cons->stop_consumer = kvm_arch_stop_consumer;
+	irqfd->cons->resume_consumer = kvm_arch_resume_consumer;
+	ret = irq_bypass_register_consumer(irqfd->cons);
+	WARN_ON(ret);
 
 	return 0;
 
@@ -530,8 +546,6 @@ kvm_irqfd_deassign(struct kvm *kvm, struct kvm_irqfd *args)
 	struct _irqfd *irqfd, *tmp;
 	struct eventfd_ctx *eventfd;
 
-	/* irq_bypass_unregister_consumer() */
-
 	eventfd = eventfd_ctx_fdget(args->fd);
 	if (IS_ERR(eventfd))
 		return PTR_ERR(eventfd);
@@ -550,6 +564,8 @@ kvm_irqfd_deassign(struct kvm *kvm, struct kvm_irqfd *args)
 			irqfd->irq_entry.type = 0;
 			write_seqcount_end(&irqfd->irq_entry_sc);
 			irqfd_deactivate(irqfd);
+			irq_bypass_unregister_consumer(irqfd->cons);
+			kfree(irqfd->cons);
 		}
 	}
 
-- 
1.9.1

[RFC 17/17] VFIO: platform: add irq bypass producer management

[Eric Auger]

This patch adds irq_bypass_producer registration/unregistration.
VFIO producer callbacks are populated:
- stop/resume producer simply consist in disabling/enabling the host irq
- add/del consumer: basically set the automasked flag to false/true

The vfio_platform_device pointer is passed as producer opaque.

We also cache the device handle in vfio_platform_device. This
makes possible to easily retrieve the vfio_device at registration.

Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
 drivers/vfio/platform/vfio_platform_common.c  |  2 +
 drivers/vfio/platform/vfio_platform_irq.c     | 83 +++++++++++++++++++++++++++
 drivers/vfio/platform/vfio_platform_private.h |  2 +
 3 files changed, 87 insertions(+)

diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index 9acfca6..12d4540 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -546,6 +546,8 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev,
 	if (!vdev)
 		return -EINVAL;
 
+	vdev->dev = dev;
+
 	group = iommu_group_get(dev);
 	if (!group) {
 		pr_err("VFIO: No IOMMU group for device %s\n", vdev->name);
diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c
index f6d83ed..0061e6e 100644
--- a/drivers/vfio/platform/vfio_platform_irq.c
+++ b/drivers/vfio/platform/vfio_platform_irq.c
@@ -20,6 +20,7 @@
 #include <linux/types.h>
 #include <linux/vfio.h>
 #include <linux/irq.h>
+#include <linux/irqbypass.h>
 
 #include "vfio_platform_private.h"
 
@@ -185,6 +186,70 @@ static irqreturn_t vfio_handler(int irq, void *dev_id)
 	return ret;
 }
 
+static void vfio_platform_stop_producer(struct irq_bypass_producer *prod)
+{
+	pr_info("%s disable %d\n", __func__, prod->irq);
+	disable_irq(prod->irq);
+}
+
+static void vfio_platform_resume_producer(struct irq_bypass_producer *prod)
+{
+	pr_info("%s enable %d\n", __func__, prod->irq);
+	enable_irq(prod->irq);
+}
+
+static void vfio_platform_add_consumer(struct irq_bypass_producer *prod,
+				       struct irq_bypass_consumer *cons)
+{
+	int i, ret;
+	struct vfio_platform_device *vdev =
+		(struct vfio_platform_device *)prod->opaque;
+
+	pr_info("%s irq=%d gsi =%d\n", __func__, prod->irq, cons->gsi);
+
+	for (i = 0; i < vdev->num_irqs; i++) {
+		if (vdev->irqs[i].prod == prod)
+			break;
+	}
+	WARN_ON(i == vdev->num_irqs);
+
+	//TODO
+	/*
+	 * if the IRQ is active at irqchip level or VFIO (auto)masked
+	 * this means the host IRQ is already under injection in the
+	 * guest and this not safe to change the forwarding state at
+	 * that stage.
+	 * It is not possible to differentiate user-space masking
+	 * from auto-masking, leading to possible false detection of
+	 * active state.
+	 */
+	prod->active = vfio_external_is_active(prod->vdev, i, 0, 0);
+
+	ret = vfio_external_set_automasked(prod->vdev, i, 0, 0, false);
+	WARN_ON(ret);
+}
+
+static void vfio_platform_del_consumer(struct irq_bypass_producer *prod,
+				       struct irq_bypass_consumer *cons)
+{
+	int i;
+	struct vfio_platform_device *vdev =
+		(struct vfio_platform_device *)prod->opaque;
+
+	pr_info("%s irq=%d gsi =%d\n", __func__, prod->irq, cons->gsi);
+
+	for (i = 0; i < vdev->num_irqs; i++) {
+		if (vdev->irqs[i].prod == prod)
+			break;
+	}
+	WARN_ON(i == vdev->num_irqs);
+
+	if (prod->active)
+		vfio_external_mask(prod->vdev, i, 0, 0);
+
+	vfio_external_set_automasked(prod->vdev, i, 0, 0, true);
+}
+
 static int vfio_set_trigger(struct vfio_platform_device *vdev, int index,
 			    int fd, irq_handler_t handler)
 {
@@ -192,8 +257,11 @@ static int vfio_set_trigger(struct vfio_platform_device *vdev, int index,
 	struct eventfd_ctx *trigger;
 	int ret;
 
+
 	if (irq->trigger) {
 		free_irq(irq->hwirq, irq);
+		irq_bypass_unregister_producer(irq->prod);
+		kfree(irq->prod);
 		kfree(irq->name);
 		eventfd_ctx_put(irq->trigger);
 		irq->trigger = NULL;
@@ -225,6 +293,21 @@ static int vfio_set_trigger(struct vfio_platform_device *vdev, int index,
 		return ret;
 	}
 
+	irq->prod = kzalloc(sizeof(struct irq_bypass_producer),
+			    GFP_KERNEL);
+	if (!irq->prod)
+		return -ENOMEM;
+	irq->prod->token = (void *)trigger;
+	irq->prod->irq = irq->hwirq;
+	irq->prod->vdev = vfio_device_get_from_dev(vdev->dev);
+	irq->prod->opaque = (void *)vdev;
+	irq->prod->add_consumer = vfio_platform_add_consumer;
+	irq->prod->del_consumer = vfio_platform_del_consumer;
+	irq->prod->stop_producer = vfio_platform_stop_producer;
+	irq->prod->resume_producer = vfio_platform_resume_producer;
+	ret = irq_bypass_register_producer(irq->prod);
+	WARN_ON(ret);
+
 	if (!irq->masked)
 		enable_irq(irq->hwirq);
 
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index 5f46c68..1255306 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -38,6 +38,7 @@ struct vfio_platform_irq {
 	struct virqfd		*unmask;
 	struct virqfd		*mask;
 	irqreturn_t (*handler)(int irq, void *dev_id);
+	struct irq_bypass_producer *prod;
 };
 
 struct vfio_platform_region {
@@ -57,6 +58,7 @@ struct vfio_platform_device {
 	u32				num_irqs;
 	int				refcnt;
 	struct mutex			igate;
+	struct device			*dev;
 
 	/*
 	 * These fields should be filled by the bus specific binder
-- 
1.9.1

Re: [RFC 12/17] irq: bypass: Extend skeleton for ARM forwarding control

[Paolo Bonzini]

On 02/07/2015 15:17, Eric Auger wrote:
> - new fields are added on producer side: linux irq, vfio_device handle,
>   active which reflects whether the source is active (at interrupt
>   controller level or at VFIO level - automasked -) and finally an
>   opaque pointer which will be used to point to the vfio_platform_device
>   in this series.

Linux IRQ and active should be okay.  As to the vfio_device handle, you
should link it from the vfio_platform_device instead.  And for the
vfio_platform_device, you can link it from the vfio_platform_irq instead.

Once you've done this, embed the irq_bypass_producer struct in the
vfio_platform_irq struct; in the new kvm_arch_* functions, go back to
the vfio_platform_irq struct via container_of.  From there you can
retrieve pointers to the vfio_platform_device and the vfio_device.

> - new fields on consumer side: the kvm handle, the gsi

You do not need to add these.  Instead, add the kvm handle to irqfd
only.  Like above, embed the irq_bypass_consumer struct in the irqfd
struct; in the new kvm_arch_* functions, go back to the
vfio_platform_irq struct via container_of.

Paolo

Re: [RFC 13/17] KVM: introduce kvm_arch functions for IRQ bypass

[Paolo Bonzini]

On 02/07/2015 15:17, Eric Auger wrote:
> +#ifdef CONFIG_IRQ_BYPASS_MANAGER

Please use a separate symbol CONFIG_KVM_HAVE_IRQ_BYPASS.

> +void kvm_arch_add_producer(struct irq_bypass_consumer *,
> +			   struct irq_bypass_producer *);

add_irq_bypass_producer, and so on below.

Paolo

> +void kvm_arch_del_producer(struct irq_bypass_consumer *,
> +			   struct irq_bypass_producer *);
> +void kvm_arch_stop_consumer(struct irq_bypass_consumer *);
> +void kvm_arch_resume_consumer(struct irq_bypass_consumer *);
> +
> +#else
> +void kvm_arch_add_producer(struct irq_bypass_consumer *,
> +			   struct irq_bypass_producer *)
> +{
> +}
> +void kvm_arch_del_producer(struct irq_bypass_consumer *,
> +			   struct irq_bypass_producer *)
> +{
> +}
> +void kvm_arch_stop_consumer(struct irq_bypass_consumer *)
> +{
> +}
> +void kvm_arch_resume_consumer(struct irq_bypass_consumer *)
> +{
> +}

Re: [RFC 16/17] KVM: eventfd: add irq bypass consumer management

[Paolo Bonzini]

On 02/07/2015 15:17, Eric Auger wrote:
> This patch adds the registration/unregistration of an
> irq_bypass_consumer on irqfd assignment/deassignment.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> ---
>  virt/kvm/eventfd.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index f3da161..425a47b 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -34,6 +34,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>
> @@ -93,6 +94,7 @@ struct _irqfd {
>  	struct list_head list;
>  	poll_table pt;
>  	struct work_struct shutdown;
> +	struct irq_bypass_consumer *cons;
>  };
>  
>  static struct workqueue_struct *irqfd_cleanup_wq;
> @@ -429,7 +431,21 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)
>  	 */
>  	fdput(f);
>  
> -	/* irq_bypass_register_consumer(); */
> +	irqfd->cons = kzalloc(sizeof(struct irq_bypass_consumer),
> +			      GFP_KERNEL);

Apart from the struct embedding technique I suggested in patch 12, this
looks very reasonable.  Thanks!

Paolo

> +	if (!irqfd->cons) {
> +		ret = -ENOMEM;
> +		goto fail;
> +	}
> +	irqfd->cons->token = (void *)irqfd->eventfd;
> +	irqfd->cons->gsi = irqfd->gsi;
> +	irqfd->cons->kvm = kvm;
> +	irqfd->cons->add_producer = kvm_arch_add_producer;
> +	irqfd->cons->del_producer = kvm_arch_del_producer;
> +	irqfd->cons->stop_consumer = kvm_arch_stop_consumer;
> +	irqfd->cons->resume_consumer = kvm_arch_resume_consumer;
> +	ret = irq_bypass_register_consumer(irqfd->cons);
> +	WARN_ON(ret);
>  
>  	return 0;
>  
> @@ -530,8 +546,6 @@ kvm_irqfd_deassign(struct kvm *kvm, struct kvm_irqfd *args)
>  	struct _irqfd *irqfd, *tmp;
>  	struct eventfd_ctx *eventfd;
>  
> -	/* irq_bypass_unregister_consumer() */
> -
>  	eventfd = eventfd_ctx_fdget(args->fd);
>  	if (IS_ERR(eventfd))
>  		return PTR_ERR(eventfd);
> @@ -550,6 +564,8 @@ kvm_irqfd_deassign(struct kvm *kvm, struct kvm_irqfd *args)
>  			irqfd->irq_entry.type = 0;
>  			write_seqcount_end(&irqfd->irq_entry_sc);
>  			irqfd_deactivate(irqfd);
> +			irq_bypass_unregister_consumer(irqfd->cons);
> +			kfree(irqfd->cons);
>  		}
>  	}
>  
> 

Re: [RFC 16/17] KVM: eventfd: add irq bypass consumer management

[Eric Auger]

On 07/02/2015 03:42 PM, Paolo Bonzini wrote:
> 
> 
> On 02/07/2015 15:17, Eric Auger wrote:
>> This patch adds the registration/unregistration of an
>> irq_bypass_consumer on irqfd assignment/deassignment.
>>
>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>> ---
>>  virt/kvm/eventfd.c | 22 +++++++++++++++++++---
>>  1 file changed, 19 insertions(+), 3 deletions(-)
>>
>> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
>> index f3da161..425a47b 100644
>> --- a/virt/kvm/eventfd.c
>> +++ b/virt/kvm/eventfd.c
>> @@ -34,6 +34,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>
>> @@ -93,6 +94,7 @@ struct _irqfd {
>>  	struct list_head list;
>>  	poll_table pt;
>>  	struct work_struct shutdown;
>> +	struct irq_bypass_consumer *cons;
>>  };
>>  
>>  static struct workqueue_struct *irqfd_cleanup_wq;
>> @@ -429,7 +431,21 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)
>>  	 */
>>  	fdput(f);
>>  
>> -	/* irq_bypass_register_consumer(); */
>> +	irqfd->cons = kzalloc(sizeof(struct irq_bypass_consumer),
>> +			      GFP_KERNEL);
> 
> Apart from the struct embedding technique I suggested in patch 12, this
> looks very reasonable.  Thanks!

Hi Paolo,

thanks for the swift feedback. I will respin shortly with the advised
embedding technique.

Best Regards

Eric
> 
> Paolo
> 
>> +	if (!irqfd->cons) {
>> +		ret = -ENOMEM;
>> +		goto fail;
>> +	}
>> +	irqfd->cons->token = (void *)irqfd->eventfd;
>> +	irqfd->cons->gsi = irqfd->gsi;
>> +	irqfd->cons->kvm = kvm;
>> +	irqfd->cons->add_producer = kvm_arch_add_producer;
>> +	irqfd->cons->del_producer = kvm_arch_del_producer;
>> +	irqfd->cons->stop_consumer = kvm_arch_stop_consumer;
>> +	irqfd->cons->resume_consumer = kvm_arch_resume_consumer;
>> +	ret = irq_bypass_register_consumer(irqfd->cons);
>> +	WARN_ON(ret);
>>  
>>  	return 0;
>>  
>> @@ -530,8 +546,6 @@ kvm_irqfd_deassign(struct kvm *kvm, struct kvm_irqfd *args)
>>  	struct _irqfd *irqfd, *tmp;
>>  	struct eventfd_ctx *eventfd;
>>  
>> -	/* irq_bypass_unregister_consumer() */
>> -
>>  	eventfd = eventfd_ctx_fdget(args->fd);
>>  	if (IS_ERR(eventfd))
>>  		return PTR_ERR(eventfd);
>> @@ -550,6 +564,8 @@ kvm_irqfd_deassign(struct kvm *kvm, struct kvm_irqfd *args)
>>  			irqfd->irq_entry.type = 0;
>>  			write_seqcount_end(&irqfd->irq_entry_sc);
>>  			irqfd_deactivate(irqfd);
>> +			irq_bypass_unregister_consumer(irqfd->cons);
>> +			kfree(irqfd->cons);
>>  		}
>>  	}
>>  
>>

RE: [RFC 09/17] bypass: IRQ bypass manager proto by Alex

[Wu, Feng]

> -----Original Message-----
> From: Eric Auger [mailto:eric.auger@linaro.org]
> Sent: Thursday, July 02, 2015 9:17 PM
> To: eric.auger@st.com; eric.auger@linaro.org;
> linux-arm-kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
> kvm@vger.kernel.org; christoffer.dall@linaro.org; marc.zyngier@arm.com;
> alex.williamson@redhat.com; pbonzini@redhat.com; avi.kivity@gmail.com;
> mtosatti@redhat.com; Wu, Feng; joro@8bytes.org;
> b.reynal@virtualopensystems.com
> Cc: linux-kernel@vger.kernel.org; patches@linaro.org
> Subject: [RFC 09/17] bypass: IRQ bypass manager proto by Alex
> 
> From: Alex Williamson <alex.williamson@redhat.com>
> 
> There are plenty of details to be filled in, but I think the basics
> looks something like the code below.  The IRQ bypass manager just
> defines a pair of structures, one for interrupt producers and one for
> interrupt consumers.  I'm certain that we'll need more callbacks than
> I've defined below, but figuring out what those should be for the best
> abstraction is the hardest part of this idea.  The manager provides both
> registration and de-registration interfaces for both types of objects
> and keeps lists for each, protected by a lock.  The manager doesn't even
> really need to know what the match token is, but I assume for our
> purposes it will be an eventfd_ctx.
> 
> On the vfio side, the producer struct would be embedded in the
> vfio_pci_irq_ctx struct.  KVM would probably embed the consumer struct
> in _irqfd.  As I've coded below, the IRQ bypass manager calls the
> consumer callbacks, so the producer struct would need fields or
> callbacks to provide the consumer the info it needs.  AIUI the Posted
> Interrupt model, VFIO only needs to provide data to the consumer.  For
> IRQ Forwarding, I think the producer needs to be informed when bypass is
> active to model the incoming interrupt as edge vs level.
> 
> I've prototyped the base IRQ bypass manager here as static, but I don't
> see any reason it couldn't be a module that's loaded by dependency when
> either vfio-pci or kvm-intel is loaded (or other producer/consumer
> objects).
> 
> Is this a reasonable starting point to craft the additional fields and
> callbacks and interaction of who calls who that we need to support
> Posted Interrupts and IRQ Forwarding?  Is the AMD version of this still
> alive?  Thanks,
> 
> Alex

In fact, I also implement a RFC patch for this new framework. I am
thinking, can we discuss all the requirements for irq forwarding and
posted interrupts, and make it a separate patchset as a general
layer? Then we can continue to push arch specific stuff, it is more
clear and easy.

Thanks,
Feng

> ---
>  arch/x86/kvm/Kconfig              |   1 +
>  drivers/vfio/pci/Kconfig          |   1 +
>  drivers/vfio/pci/vfio_pci_intrs.c |   6 ++
>  include/linux/irqbypass.h         |  23 ++++++++
>  kernel/irq/Kconfig                |   3 +
>  kernel/irq/Makefile               |   1 +
>  kernel/irq/bypass.c               | 116
> ++++++++++++++++++++++++++++++++++++++
>  virt/kvm/eventfd.c                |   4 ++
>  8 files changed, 155 insertions(+)
>  create mode 100644 include/linux/irqbypass.h
>  create mode 100644 kernel/irq/bypass.c
> 
> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
> index d8a1d56..86d0d77 100644
> --- a/arch/x86/kvm/Kconfig
> +++ b/arch/x86/kvm/Kconfig
> @@ -61,6 +61,7 @@ config KVM_INTEL
>  	depends on KVM
>  	# for perf_guest_get_msrs():
>  	depends on CPU_SUP_INTEL
> +	select IRQ_BYPASS_MANAGER
>  	---help---
>  	  Provides support for KVM on Intel processors equipped with the VT
>  	  extensions.
> diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
> index 579d83b..02912f1 100644
> --- a/drivers/vfio/pci/Kconfig
> +++ b/drivers/vfio/pci/Kconfig
> @@ -2,6 +2,7 @@ config VFIO_PCI
>  	tristate "VFIO support for PCI devices"
>  	depends on VFIO && PCI && EVENTFD
>  	select VFIO_VIRQFD
> +	select IRQ_BYPASS_MANAGER
>  	help
>  	  Support for the PCI VFIO bus driver.  This is required to make
>  	  use of PCI drivers using the VFIO framework.
> diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
> index 1f577b4..4e053be 100644
> --- a/drivers/vfio/pci/vfio_pci_intrs.c
> +++ b/drivers/vfio/pci/vfio_pci_intrs.c
> @@ -181,6 +181,7 @@ static int vfio_intx_set_signal(struct vfio_pci_device
> *vdev, int fd)
> 
>  	if (vdev->ctx[0].trigger) {
>  		free_irq(pdev->irq, vdev);
> +		/* irq_bypass_unregister_producer(); */
>  		kfree(vdev->ctx[0].name);
>  		eventfd_ctx_put(vdev->ctx[0].trigger);
>  		vdev->ctx[0].trigger = NULL;
> @@ -214,6 +215,8 @@ static int vfio_intx_set_signal(struct vfio_pci_device
> *vdev, int fd)
>  		return ret;
>  	}
> 
> +	/* irq_bypass_register_producer(); */
> +
>  	/*
>  	 * INTx disable will stick across the new irq setup,
>  	 * disable_irq won't.
> @@ -319,6 +322,7 @@ static int vfio_msi_set_vector_signal(struct
> vfio_pci_device *vdev,
> 
>  	if (vdev->ctx[vector].trigger) {
>  		free_irq(irq, vdev->ctx[vector].trigger);
> +		/* irq_bypass_unregister_producer(); */
>  		kfree(vdev->ctx[vector].name);
>  		eventfd_ctx_put(vdev->ctx[vector].trigger);
>  		vdev->ctx[vector].trigger = NULL;
> @@ -360,6 +364,8 @@ static int vfio_msi_set_vector_signal(struct
> vfio_pci_device *vdev,
>  		return ret;
>  	}
> 
> +	/* irq_bypass_register_producer(); */
> +
>  	vdev->ctx[vector].trigger = trigger;
> 
>  	return 0;
> diff --git a/include/linux/irqbypass.h b/include/linux/irqbypass.h
> new file mode 100644
> index 0000000..718508e
> --- /dev/null
> +++ b/include/linux/irqbypass.h
> @@ -0,0 +1,23 @@
> +#ifndef IRQBYPASS_H
> +#define IRQBYPASS_H
> +
> +#include <linux/list.h>
> +
> +struct irq_bypass_producer {
> +	struct list_head node;
> +	void *token;
> +	/* TBD */
> +};
> +
> +struct irq_bypass_consumer {
> +	struct list_head node;
> +	void *token;
> +	void (*add_producer)(struct irq_bypass_producer *);
> +	void (*del_producer)(struct irq_bypass_producer *);
> +};
> +
> +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/kernel/irq/Kconfig b/kernel/irq/Kconfig
> index 9a76e3b..4502cdc 100644
> --- a/kernel/irq/Kconfig
> +++ b/kernel/irq/Kconfig
> @@ -100,4 +100,7 @@ config SPARSE_IRQ
> 
>  	  If you don't know what to do here, say N.
> 
> +config IRQ_BYPASS_MANAGER
> +	bool
> +
>  endmenu
> diff --git a/kernel/irq/Makefile b/kernel/irq/Makefile
> index d121235..a30ed77 100644
> --- a/kernel/irq/Makefile
> +++ b/kernel/irq/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_PROC_FS) += proc.o
>  obj-$(CONFIG_GENERIC_PENDING_IRQ) += migration.o
>  obj-$(CONFIG_PM_SLEEP) += pm.o
>  obj-$(CONFIG_GENERIC_MSI_IRQ) += msi.o
> +obj-$(CONFIG_IRQ_BYPASS_MANAGER) += bypass.o
> diff --git a/kernel/irq/bypass.c b/kernel/irq/bypass.c
> new file mode 100644
> index 0000000..5d0f92b
> --- /dev/null
> +++ b/kernel/irq/bypass.c
> @@ -0,0 +1,116 @@
> +/*
> + * IRQ offload/bypass manager
> + *
> + * Various virtualization hardware acceleration techniques allow bypassing
> + * or offloading interrupts receieved from devices around the host kernel.
> + * Posted Interrupts on Intel VT-d systems can allow interrupts to be
> + * recieved directly by a virtual machine.  ARM IRQ Forwarding can allow
> + * level triggered device interrupts to be de-asserted directly by the VM.
> + * 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>
> +
> +static LIST_HEAD(producers);
> +static LIST_HEAD(consumers);
> +static DEFINE_MUTEX(lock);
> +
> +int irq_bypass_register_producer(struct irq_bypass_producer *producer)
> +{
> +	struct irq_bypass_producer *tmp;
> +	struct irq_bypass_consumer *consumer;
> +	int ret = 0;
> +
> +	mutex_lock(&lock);
> +
> +	list_for_each_entry(tmp, &producers, node) {
> +		if (tmp->token == producer->token) {
> +			ret = -EINVAL;
> +			goto unlock;
> +		}
> +	}
> +
> +	list_add(&producer->node, &producers);
> +
> +	list_for_each_entry(consumer, &consumers, node) {
> +		if (consumer->token == producer->token) {
> +			consumer->add_producer(producer);
> +			break;
> +		}
> +	}
> +unlock:
> +	mutex_unlock(&lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(irq_bypass_register_producer);
> +
> +void irq_bypass_unregister_producer(struct irq_bypass_producer *producer)
> +{
> +	struct irq_bypass_consumer *consumer;
> +
> +	mutex_lock(&lock);
> +
> +	list_for_each_entry(consumer, &consumers, node) {
> +		if (consumer->token == producer->token) {
> +			consumer->del_producer(producer);
> +			break;
> +		}
> +	}
> +
> +	list_del(&producer->node);
> +
> +	mutex_unlock(&lock);
> +}
> +EXPORT_SYMBOL_GPL(irq_bypass_unregister_producer);
> +
> +int irq_bypass_register_consumer(struct irq_bypass_consumer *consumer)
> +{
> +	struct irq_bypass_consumer *tmp;
> +	struct irq_bypass_producer *producer;
> +	int ret = 0;
> +
> +	mutex_lock(&lock);
> +
> +	list_for_each_entry(tmp, &consumers, node) {
> +		if (tmp->token == consumer->token) {
> +			ret = -EINVAL;
> +			goto unlock;
> +		}
> +	}
> +
> +	list_add(&consumer->node, &consumers);
> +
> +	list_for_each_entry(producer, &producers, node) {
> +		if (producer->token == consumer->token) {
> +			consumer->add_producer(producer);
> +			break;
> +		}
> +	}
> +unlock:
> +	mutex_unlock(&lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(irq_bypass_register_consumer);
> +
> +void irq_bypass_unregister_consumer(struct irq_bypass_consumer
> *consumer)
> +{
> +	struct irq_bypass_producer *producer;
> +
> +	mutex_lock(&lock);
> +
> +	list_for_each_entry(producer, &producers, node) {
> +		if (producer->token == consumer->token) {
> +			consumer->del_producer(producer);
> +			break;
> +		}
> +	}
> +
> +	list_del(&consumer->node);
> +
> +	mutex_unlock(&lock);
> +}
> +EXPORT_SYMBOL_GPL(irq_bypass_unregister_consumer);
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index 9ff4193..f3da161 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -429,6 +429,8 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd
> *args)
>  	 */
>  	fdput(f);
> 
> +	/* irq_bypass_register_consumer(); */
> +
>  	return 0;
> 
>  fail:
> @@ -528,6 +530,8 @@ kvm_irqfd_deassign(struct kvm *kvm, struct kvm_irqfd
> *args)
>  	struct _irqfd *irqfd, *tmp;
>  	struct eventfd_ctx *eventfd;
> 
> +	/* irq_bypass_unregister_consumer() */
> +
>  	eventfd = eventfd_ctx_fdget(args->fd);
>  	if (IS_ERR(eventfd))
>  		return PTR_ERR(eventfd);
> --
> 1.9.1

RE: [RFC 12/17] irq: bypass: Extend skeleton for ARM forwarding control

[Wu, Feng]

> -----Original Message-----
> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> Sent: Thursday, July 02, 2015 9:41 PM
> To: Eric Auger; eric.auger@st.com; linux-arm-kernel@lists.infradead.org;
> kvmarm@lists.cs.columbia.edu; kvm@vger.kernel.org;
> christoffer.dall@linaro.org; marc.zyngier@arm.com;
> alex.williamson@redhat.com; avi.kivity@gmail.com; mtosatti@redhat.com;
> Wu, Feng; joro@8bytes.org; b.reynal@virtualopensystems.com
> Cc: linux-kernel@vger.kernel.org; patches@linaro.org
> Subject: Re: [RFC 12/17] irq: bypass: Extend skeleton for ARM forwarding
> control
> 
> 
> 
> On 02/07/2015 15:17, Eric Auger wrote:
> > - new fields are added on producer side: linux irq, vfio_device handle,
> >   active which reflects whether the source is active (at interrupt
> >   controller level or at VFIO level - automasked -) and finally an
> >   opaque pointer which will be used to point to the vfio_platform_device
> >   in this series.
> 
> Linux IRQ and active should be okay.  As to the vfio_device handle, you
> should link it from the vfio_platform_device instead.  And for the
> vfio_platform_device, you can link it from the vfio_platform_irq instead.
> 
> Once you've done this, embed the irq_bypass_producer struct in the
> vfio_platform_irq struct; in the new kvm_arch_* functions, go back to
> the vfio_platform_irq struct via container_of.  From there you can
> retrieve pointers to the vfio_platform_device and the vfio_device.
> 
> > - new fields on consumer side: the kvm handle, the gsi
> 
> You do not need to add these.  Instead, add the kvm handle to irqfd
> only.  Like above, embed the irq_bypass_consumer struct in the irqfd
> struct; in the new kvm_arch_* functions, go back to the
> vfio_platform_irq struct via container_of.
> 

I also need the gsi field here, for posted-interrupts, I need 'gsi', 'irq' to
update the IRTE.

Thanks,
Feng


> Paolo

RE: [RFC 12/17] irq: bypass: Extend skeleton for ARM forwarding control

[Wu, Feng]

> -----Original Message-----
> From: Wu, Feng
> Sent: Friday, July 03, 2015 10:20 AM
> To: Paolo Bonzini; Eric Auger; eric.auger@st.com;
> linux-arm-kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
> kvm@vger.kernel.org; christoffer.dall@linaro.org; marc.zyngier@arm.com;
> alex.williamson@redhat.com; avi.kivity@gmail.com; mtosatti@redhat.com;
> joro@8bytes.org; b.reynal@virtualopensystems.com
> Cc: linux-kernel@vger.kernel.org; patches@linaro.org; Wu, Feng
> Subject: RE: [RFC 12/17] irq: bypass: Extend skeleton for ARM forwarding
> control
> 
> 
> 
> > -----Original Message-----
> > From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> > Sent: Thursday, July 02, 2015 9:41 PM
> > To: Eric Auger; eric.auger@st.com; linux-arm-kernel@lists.infradead.org;
> > kvmarm@lists.cs.columbia.edu; kvm@vger.kernel.org;
> > christoffer.dall@linaro.org; marc.zyngier@arm.com;
> > alex.williamson@redhat.com; avi.kivity@gmail.com; mtosatti@redhat.com;
> > Wu, Feng; joro@8bytes.org; b.reynal@virtualopensystems.com
> > Cc: linux-kernel@vger.kernel.org; patches@linaro.org
> > Subject: Re: [RFC 12/17] irq: bypass: Extend skeleton for ARM forwarding
> > control
> >
> >
> >
> > On 02/07/2015 15:17, Eric Auger wrote:
> > > - new fields are added on producer side: linux irq, vfio_device handle,
> > >   active which reflects whether the source is active (at interrupt
> > >   controller level or at VFIO level - automasked -) and finally an
> > >   opaque pointer which will be used to point to the vfio_platform_device
> > >   in this series.
> >
> > Linux IRQ and active should be okay.  As to the vfio_device handle, you
> > should link it from the vfio_platform_device instead.  And for the
> > vfio_platform_device, you can link it from the vfio_platform_irq instead.
> >
> > Once you've done this, embed the irq_bypass_producer struct in the
> > vfio_platform_irq struct; in the new kvm_arch_* functions, go back to
> > the vfio_platform_irq struct via container_of.  From there you can
> > retrieve pointers to the vfio_platform_device and the vfio_device.
> >
> > > - new fields on consumer side: the kvm handle, the gsi
> >
> > You do not need to add these.  Instead, add the kvm handle to irqfd
> > only.  Like above, embed the irq_bypass_consumer struct in the irqfd
> > struct; in the new kvm_arch_* functions, go back to the
> > vfio_platform_irq struct via container_of.
> >
> 
> I also need the gsi field here, for posted-interrupts, I need 'gsi', 'irq' to
> update the IRTE.

Oh... we can get gsi from irq_bypass_consumer -> _irqfd -> gsi, so it
is not needed in irq_bypass_consumer. Got it! :)

Thanks,
Feng

> 
> Thanks,
> Feng
> 
> 
> > Paolo

RE: [RFC 12/17] irq: bypass: Extend skeleton for ARM forwarding control

[Wu, Feng]

> -----Original Message-----
> From: Eric Auger [mailto:eric.auger@linaro.org]
> Sent: Thursday, July 02, 2015 9:17 PM
> To: eric.auger@st.com; eric.auger@linaro.org;
> linux-arm-kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
> kvm@vger.kernel.org; christoffer.dall@linaro.org; marc.zyngier@arm.com;
> alex.williamson@redhat.com; pbonzini@redhat.com; avi.kivity@gmail.com;
> mtosatti@redhat.com; Wu, Feng; joro@8bytes.org;
> b.reynal@virtualopensystems.com
> Cc: linux-kernel@vger.kernel.org; patches@linaro.org
> Subject: [RFC 12/17] irq: bypass: Extend skeleton for ARM forwarding control
> 
> - [add,del]_[consumer,producer] updated to takes both the consumer and
>   producer handles. This is requested to combine info from both,
>   typically to link the source irq owned by the producer with the gsi
>   owned by the consumer (forwarded IRQ setup).
> - new functions are added: [stop,resume]_[consumer, producer]. Those are
>   needed for forwarding since the state change requires to entermingle
>   actions at consumer, producer.
> - On handshake, we now call connect, disconnect which features the more
>   complex sequence.
> - new fields are added on producer side: linux irq, vfio_device handle,
>   active which reflects whether the source is active (at interrupt
>   controller level or at VFIO level - automasked -) and finally an
>   opaque pointer which will be used to point to the vfio_platform_device
>   in this series.
> - new fields on consumer side: the kvm handle, the gsi
> 
> Integration of posted interrupt series will help to refine those choices

On PI side, I need another filed as below,

struct irq_bypass_consumer {
       struct list_head node;
       void *token;
+      unsigned irq;	/*got from producer when registered*/
       void (*add_producer)(struct irq_bypass_producer *,
                            struct irq_bypass_consumer *);
       void (*del_producer)(struct irq_bypass_producer *,
                            struct irq_bypass_consumer *);
+      void (*update)(struct irq_bypass_consumer *);
};

'update' is used to update the IRTE, while irq is initialized when
registered, which is used to find the right IRTE.

Thanks,
Feng

> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> 
> ---
> 
> - connect/disconnect could become a cb too. For forwarding it may make
>   sense to have failure at connection: this would happen when the physical
>   IRQ is either active at irqchip level or VFIO masked. This means some
>   of the cb should return an error and this error management could be
>   prod/cons specific. Where to attach the connect/disconnect cb: to the
>   cons or prod, to both?
> - Hence may be sensible to do the list_add only if connect returns 0
> - disconnect would not be allowed to fail.
> ---
>  include/linux/irqbypass.h | 26 ++++++++++++++++++++++---
>  kernel/irq/bypass.c       | 48
> +++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 67 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/irqbypass.h b/include/linux/irqbypass.h
> index 718508e..591ae3f 100644
> --- a/include/linux/irqbypass.h
> +++ b/include/linux/irqbypass.h
> @@ -3,17 +3,37 @@
> 
>  #include <linux/list.h>
> 
> +struct vfio_device;
> +struct irq_bypass_consumer;
> +struct kvm;
> +
>  struct irq_bypass_producer {
>  	struct list_head node;
>  	void *token;
> -	/* TBD */
> +	unsigned int irq; /* host physical irq */
> +	struct vfio_device *vdev; /* vfio device that requested irq */
> +	/* is irq active at irqchip or VFIO masked? */
> +	bool active;
> +	void *opaque;
> +	void (*stop_producer)(struct irq_bypass_producer *);
> +	void (*resume_producer)(struct irq_bypass_producer *);
> +	void (*add_consumer)(struct irq_bypass_producer *,
> +			     struct irq_bypass_consumer *);
> +	void (*del_consumer)(struct irq_bypass_producer *,
> +			     struct irq_bypass_consumer *);
>  };
> 
>  struct irq_bypass_consumer {
>  	struct list_head node;
>  	void *token;
> -	void (*add_producer)(struct irq_bypass_producer *);
> -	void (*del_producer)(struct irq_bypass_producer *);
> +	unsigned int gsi;		/* the guest gsi */
> +	struct kvm *kvm;
> +	void (*stop_consumer)(struct irq_bypass_consumer *);
> +	void (*resume_consumer)(struct irq_bypass_consumer *);
> +	void (*add_producer)(struct irq_bypass_consumer *,
> +			     struct irq_bypass_producer *);
> +	void (*del_producer)(struct irq_bypass_consumer *,
> +			     struct irq_bypass_producer *);
>  };
> 
>  int irq_bypass_register_producer(struct irq_bypass_producer *);
> diff --git a/kernel/irq/bypass.c b/kernel/irq/bypass.c
> index 5d0f92b..fb31fef 100644
> --- a/kernel/irq/bypass.c
> +++ b/kernel/irq/bypass.c
> @@ -19,6 +19,46 @@ static LIST_HEAD(producers);
>  static LIST_HEAD(consumers);
>  static DEFINE_MUTEX(lock);
> 
> +/* lock must be hold when calling connect */
> +static void connect(struct irq_bypass_producer *prod,
> +		    struct irq_bypass_consumer *cons)
> +{
> +	pr_info("++++ %s prod(%d) -> cons(%d)\n",
> +		__func__, prod->irq, cons->gsi);
> +	if (prod->stop_producer)
> +		prod->stop_producer(prod);
> +	if (cons->stop_consumer)
> +		cons->stop_consumer(cons);
> +	if (prod->add_consumer)
> +		prod->add_consumer(prod, cons);
> +	if (cons->add_producer)
> +		cons->add_producer(cons, prod);
> +	if (cons->resume_consumer)
> +		cons->resume_consumer(cons);
> +	if (prod->resume_producer)
> +		prod->resume_producer(prod);
> +}
> +
> +/* lock must be hold when calling disconnect */
> +static void disconnect(struct irq_bypass_producer *prod,
> +		       struct irq_bypass_consumer *cons)
> +{
> +	pr_info("---- %s prod(%d) -> cons(%d)\n",
> +		__func__, prod->irq, cons->gsi);
> +	if (prod->stop_producer)
> +		prod->stop_producer(prod);
> +	if (cons->stop_consumer)
> +		cons->stop_consumer(cons);
> +	if (cons->del_producer)
> +		cons->del_producer(cons, prod);
> +	if (prod->del_consumer)
> +		prod->del_consumer(prod, cons);
> +	if (cons->resume_consumer)
> +		cons->resume_consumer(cons);
> +	if (prod->resume_producer)
> +		prod->resume_producer(prod);
> +}
> +
>  int irq_bypass_register_producer(struct irq_bypass_producer *producer)
>  {
>  	struct irq_bypass_producer *tmp;
> @@ -38,7 +78,7 @@ int irq_bypass_register_producer(struct
> irq_bypass_producer *producer)
> 
>  	list_for_each_entry(consumer, &consumers, node) {
>  		if (consumer->token == producer->token) {
> -			consumer->add_producer(producer);
> +			connect(producer, consumer);
>  			break;
>  		}
>  	}
> @@ -56,7 +96,7 @@ void irq_bypass_unregister_producer(struct
> irq_bypass_producer *producer)
> 
>  	list_for_each_entry(consumer, &consumers, node) {
>  		if (consumer->token == producer->token) {
> -			consumer->del_producer(producer);
> +			disconnect(producer, consumer);
>  			break;
>  		}
>  	}
> @@ -86,7 +126,7 @@ int irq_bypass_register_consumer(struct
> irq_bypass_consumer *consumer)
> 
>  	list_for_each_entry(producer, &producers, node) {
>  		if (producer->token == consumer->token) {
> -			consumer->add_producer(producer);
> +			connect(producer, consumer);
>  			break;
>  		}
>  	}
> @@ -104,7 +144,7 @@ void irq_bypass_unregister_consumer(struct
> irq_bypass_consumer *consumer)
> 
>  	list_for_each_entry(producer, &producers, node) {
>  		if (producer->token == consumer->token) {
> -			consumer->del_producer(producer);
> +			disconnect(producer, consumer);
>  			break;
>  		}
>  	}
> --
> 1.9.1

Re: [RFC 09/17] bypass: IRQ bypass manager proto by Alex

[Eric Auger]

Hi Feng,
On 07/03/2015 04:16 AM, Wu, Feng wrote:
> 
> 
>> -----Original Message-----
>> From: Eric Auger [mailto:eric.auger@linaro.org]
>> Sent: Thursday, July 02, 2015 9:17 PM
>> To: eric.auger@st.com; eric.auger@linaro.org;
>> linux-arm-kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
>> kvm@vger.kernel.org; christoffer.dall@linaro.org; marc.zyngier@arm.com;
>> alex.williamson@redhat.com; pbonzini@redhat.com; avi.kivity@gmail.com;
>> mtosatti@redhat.com; Wu, Feng; joro@8bytes.org;
>> b.reynal@virtualopensystems.com
>> Cc: linux-kernel@vger.kernel.org; patches@linaro.org
>> Subject: [RFC 09/17] bypass: IRQ bypass manager proto by Alex
>>
>> From: Alex Williamson <alex.williamson@redhat.com>
>>
>> There are plenty of details to be filled in, but I think the basics
>> looks something like the code below.  The IRQ bypass manager just
>> defines a pair of structures, one for interrupt producers and one for
>> interrupt consumers.  I'm certain that we'll need more callbacks than
>> I've defined below, but figuring out what those should be for the best
>> abstraction is the hardest part of this idea.  The manager provides both
>> registration and de-registration interfaces for both types of objects
>> and keeps lists for each, protected by a lock.  The manager doesn't even
>> really need to know what the match token is, but I assume for our
>> purposes it will be an eventfd_ctx.
>>
>> On the vfio side, the producer struct would be embedded in the
>> vfio_pci_irq_ctx struct.  KVM would probably embed the consumer struct
>> in _irqfd.  As I've coded below, the IRQ bypass manager calls the
>> consumer callbacks, so the producer struct would need fields or
>> callbacks to provide the consumer the info it needs.  AIUI the Posted
>> Interrupt model, VFIO only needs to provide data to the consumer.  For
>> IRQ Forwarding, I think the producer needs to be informed when bypass is
>> active to model the incoming interrupt as edge vs level.
>>
>> I've prototyped the base IRQ bypass manager here as static, but I don't
>> see any reason it couldn't be a module that's loaded by dependency when
>> either vfio-pci or kvm-intel is loaded (or other producer/consumer
>> objects).
>>
>> Is this a reasonable starting point to craft the additional fields and
>> callbacks and interaction of who calls who that we need to support
>> Posted Interrupts and IRQ Forwarding?  Is the AMD version of this still
>> alive?  Thanks,
>>
>> Alex
> 
> In fact, I also implement a RFC patch for this new framework. I am
> thinking, can we discuss all the requirements for irq forwarding and
> posted interrupts, and make it a separate patchset as a general
> layer? Then we can continue to push arch specific stuff, it is more
> clear and easy.

Sure. I intend to respin today according to Paolo's directives and I
will put common patches in a separate series. Let's see next week with
Alex how he prefers things to be handled.

Best Regards

Eric


> 
> Thanks,
> Feng
> 
>> ---
>>  arch/x86/kvm/Kconfig              |   1 +
>>  drivers/vfio/pci/Kconfig          |   1 +
>>  drivers/vfio/pci/vfio_pci_intrs.c |   6 ++
>>  include/linux/irqbypass.h         |  23 ++++++++
>>  kernel/irq/Kconfig                |   3 +
>>  kernel/irq/Makefile               |   1 +
>>  kernel/irq/bypass.c               | 116
>> ++++++++++++++++++++++++++++++++++++++
>>  virt/kvm/eventfd.c                |   4 ++
>>  8 files changed, 155 insertions(+)
>>  create mode 100644 include/linux/irqbypass.h
>>  create mode 100644 kernel/irq/bypass.c
>>
>> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
>> index d8a1d56..86d0d77 100644
>> --- a/arch/x86/kvm/Kconfig
>> +++ b/arch/x86/kvm/Kconfig
>> @@ -61,6 +61,7 @@ config KVM_INTEL
>>  	depends on KVM
>>  	# for perf_guest_get_msrs():
>>  	depends on CPU_SUP_INTEL
>> +	select IRQ_BYPASS_MANAGER
>>  	---help---
>>  	  Provides support for KVM on Intel processors equipped with the VT
>>  	  extensions.
>> diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
>> index 579d83b..02912f1 100644
>> --- a/drivers/vfio/pci/Kconfig
>> +++ b/drivers/vfio/pci/Kconfig
>> @@ -2,6 +2,7 @@ config VFIO_PCI
>>  	tristate "VFIO support for PCI devices"
>>  	depends on VFIO && PCI && EVENTFD
>>  	select VFIO_VIRQFD
>> +	select IRQ_BYPASS_MANAGER
>>  	help
>>  	  Support for the PCI VFIO bus driver.  This is required to make
>>  	  use of PCI drivers using the VFIO framework.
>> diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
>> index 1f577b4..4e053be 100644
>> --- a/drivers/vfio/pci/vfio_pci_intrs.c
>> +++ b/drivers/vfio/pci/vfio_pci_intrs.c
>> @@ -181,6 +181,7 @@ static int vfio_intx_set_signal(struct vfio_pci_device
>> *vdev, int fd)
>>
>>  	if (vdev->ctx[0].trigger) {
>>  		free_irq(pdev->irq, vdev);
>> +		/* irq_bypass_unregister_producer(); */
>>  		kfree(vdev->ctx[0].name);
>>  		eventfd_ctx_put(vdev->ctx[0].trigger);
>>  		vdev->ctx[0].trigger = NULL;
>> @@ -214,6 +215,8 @@ static int vfio_intx_set_signal(struct vfio_pci_device
>> *vdev, int fd)
>>  		return ret;
>>  	}
>>
>> +	/* irq_bypass_register_producer(); */
>> +
>>  	/*
>>  	 * INTx disable will stick across the new irq setup,
>>  	 * disable_irq won't.
>> @@ -319,6 +322,7 @@ static int vfio_msi_set_vector_signal(struct
>> vfio_pci_device *vdev,
>>
>>  	if (vdev->ctx[vector].trigger) {
>>  		free_irq(irq, vdev->ctx[vector].trigger);
>> +		/* irq_bypass_unregister_producer(); */
>>  		kfree(vdev->ctx[vector].name);
>>  		eventfd_ctx_put(vdev->ctx[vector].trigger);
>>  		vdev->ctx[vector].trigger = NULL;
>> @@ -360,6 +364,8 @@ static int vfio_msi_set_vector_signal(struct
>> vfio_pci_device *vdev,
>>  		return ret;
>>  	}
>>
>> +	/* irq_bypass_register_producer(); */
>> +
>>  	vdev->ctx[vector].trigger = trigger;
>>
>>  	return 0;
>> diff --git a/include/linux/irqbypass.h b/include/linux/irqbypass.h
>> new file mode 100644
>> index 0000000..718508e
>> --- /dev/null
>> +++ b/include/linux/irqbypass.h
>> @@ -0,0 +1,23 @@
>> +#ifndef IRQBYPASS_H
>> +#define IRQBYPASS_H
>> +
>> +#include <linux/list.h>
>> +
>> +struct irq_bypass_producer {
>> +	struct list_head node;
>> +	void *token;
>> +	/* TBD */
>> +};
>> +
>> +struct irq_bypass_consumer {
>> +	struct list_head node;
>> +	void *token;
>> +	void (*add_producer)(struct irq_bypass_producer *);
>> +	void (*del_producer)(struct irq_bypass_producer *);
>> +};
>> +
>> +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/kernel/irq/Kconfig b/kernel/irq/Kconfig
>> index 9a76e3b..4502cdc 100644
>> --- a/kernel/irq/Kconfig
>> +++ b/kernel/irq/Kconfig
>> @@ -100,4 +100,7 @@ config SPARSE_IRQ
>>
>>  	  If you don't know what to do here, say N.
>>
>> +config IRQ_BYPASS_MANAGER
>> +	bool
>> +
>>  endmenu
>> diff --git a/kernel/irq/Makefile b/kernel/irq/Makefile
>> index d121235..a30ed77 100644
>> --- a/kernel/irq/Makefile
>> +++ b/kernel/irq/Makefile
>> @@ -7,3 +7,4 @@ obj-$(CONFIG_PROC_FS) += proc.o
>>  obj-$(CONFIG_GENERIC_PENDING_IRQ) += migration.o
>>  obj-$(CONFIG_PM_SLEEP) += pm.o
>>  obj-$(CONFIG_GENERIC_MSI_IRQ) += msi.o
>> +obj-$(CONFIG_IRQ_BYPASS_MANAGER) += bypass.o
>> diff --git a/kernel/irq/bypass.c b/kernel/irq/bypass.c
>> new file mode 100644
>> index 0000000..5d0f92b
>> --- /dev/null
>> +++ b/kernel/irq/bypass.c
>> @@ -0,0 +1,116 @@
>> +/*
>> + * IRQ offload/bypass manager
>> + *
>> + * Various virtualization hardware acceleration techniques allow bypassing
>> + * or offloading interrupts receieved from devices around the host kernel.
>> + * Posted Interrupts on Intel VT-d systems can allow interrupts to be
>> + * recieved directly by a virtual machine.  ARM IRQ Forwarding can allow
>> + * level triggered device interrupts to be de-asserted directly by the VM.
>> + * 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>
>> +
>> +static LIST_HEAD(producers);
>> +static LIST_HEAD(consumers);
>> +static DEFINE_MUTEX(lock);
>> +
>> +int irq_bypass_register_producer(struct irq_bypass_producer *producer)
>> +{
>> +	struct irq_bypass_producer *tmp;
>> +	struct irq_bypass_consumer *consumer;
>> +	int ret = 0;
>> +
>> +	mutex_lock(&lock);
>> +
>> +	list_for_each_entry(tmp, &producers, node) {
>> +		if (tmp->token == producer->token) {
>> +			ret = -EINVAL;
>> +			goto unlock;
>> +		}
>> +	}
>> +
>> +	list_add(&producer->node, &producers);
>> +
>> +	list_for_each_entry(consumer, &consumers, node) {
>> +		if (consumer->token == producer->token) {
>> +			consumer->add_producer(producer);
>> +			break;
>> +		}
>> +	}
>> +unlock:
>> +	mutex_unlock(&lock);
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(irq_bypass_register_producer);
>> +
>> +void irq_bypass_unregister_producer(struct irq_bypass_producer *producer)
>> +{
>> +	struct irq_bypass_consumer *consumer;
>> +
>> +	mutex_lock(&lock);
>> +
>> +	list_for_each_entry(consumer, &consumers, node) {
>> +		if (consumer->token == producer->token) {
>> +			consumer->del_producer(producer);
>> +			break;
>> +		}
>> +	}
>> +
>> +	list_del(&producer->node);
>> +
>> +	mutex_unlock(&lock);
>> +}
>> +EXPORT_SYMBOL_GPL(irq_bypass_unregister_producer);
>> +
>> +int irq_bypass_register_consumer(struct irq_bypass_consumer *consumer)
>> +{
>> +	struct irq_bypass_consumer *tmp;
>> +	struct irq_bypass_producer *producer;
>> +	int ret = 0;
>> +
>> +	mutex_lock(&lock);
>> +
>> +	list_for_each_entry(tmp, &consumers, node) {
>> +		if (tmp->token == consumer->token) {
>> +			ret = -EINVAL;
>> +			goto unlock;
>> +		}
>> +	}
>> +
>> +	list_add(&consumer->node, &consumers);
>> +
>> +	list_for_each_entry(producer, &producers, node) {
>> +		if (producer->token == consumer->token) {
>> +			consumer->add_producer(producer);
>> +			break;
>> +		}
>> +	}
>> +unlock:
>> +	mutex_unlock(&lock);
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(irq_bypass_register_consumer);
>> +
>> +void irq_bypass_unregister_consumer(struct irq_bypass_consumer
>> *consumer)
>> +{
>> +	struct irq_bypass_producer *producer;
>> +
>> +	mutex_lock(&lock);
>> +
>> +	list_for_each_entry(producer, &producers, node) {
>> +		if (producer->token == consumer->token) {
>> +			consumer->del_producer(producer);
>> +			break;
>> +		}
>> +	}
>> +
>> +	list_del(&consumer->node);
>> +
>> +	mutex_unlock(&lock);
>> +}
>> +EXPORT_SYMBOL_GPL(irq_bypass_unregister_consumer);
>> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
>> index 9ff4193..f3da161 100644
>> --- a/virt/kvm/eventfd.c
>> +++ b/virt/kvm/eventfd.c
>> @@ -429,6 +429,8 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd
>> *args)
>>  	 */
>>  	fdput(f);
>>
>> +	/* irq_bypass_register_consumer(); */
>> +
>>  	return 0;
>>
>>  fail:
>> @@ -528,6 +530,8 @@ kvm_irqfd_deassign(struct kvm *kvm, struct kvm_irqfd
>> *args)
>>  	struct _irqfd *irqfd, *tmp;
>>  	struct eventfd_ctx *eventfd;
>>
>> +	/* irq_bypass_unregister_consumer() */
>> +
>>  	eventfd = eventfd_ctx_fdget(args->fd);
>>  	if (IS_ERR(eventfd))
>>  		return PTR_ERR(eventfd);
>> --
>> 1.9.1
> 

Re: [RFC 12/17] irq: bypass: Extend skeleton for ARM forwarding control

[Paolo Bonzini]

On 03/07/2015 04:43, Wu, Feng wrote:
> 
> struct irq_bypass_consumer {
>        struct list_head node;
>        void *token;
> +      unsigned irq;	/*got from producer when registered*/
>        void (*add_producer)(struct irq_bypass_producer *,
>                             struct irq_bypass_consumer *);
>        void (*del_producer)(struct irq_bypass_producer *,
>                             struct irq_bypass_consumer *);
> +      void (*update)(struct irq_bypass_consumer *);
> };
> 
> 'update' is used to update the IRTE, while irq is initialized when
> registered, which is used to find the right IRTE.

Feel free to add "update" in your PI patches.  I am not sure if "irq"
belongs here or in the containing struct.  You can play with both and
submit the version that looks better to you.

Paolo

Re: [RFC 12/17] irq: bypass: Extend skeleton for ARM forwarding control

[Eric Auger]

Paolo,
On 07/03/2015 04:24 AM, Wu, Feng wrote:
> 
> 
>> -----Original Message-----
>> From: Wu, Feng
>> Sent: Friday, July 03, 2015 10:20 AM
>> To: Paolo Bonzini; Eric Auger; eric.auger@st.com;
>> linux-arm-kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
>> kvm@vger.kernel.org; christoffer.dall@linaro.org; marc.zyngier@arm.com;
>> alex.williamson@redhat.com; avi.kivity@gmail.com; mtosatti@redhat.com;
>> joro@8bytes.org; b.reynal@virtualopensystems.com
>> Cc: linux-kernel@vger.kernel.org; patches@linaro.org; Wu, Feng
>> Subject: RE: [RFC 12/17] irq: bypass: Extend skeleton for ARM forwarding
>> control
>>
>>
>>
>>> -----Original Message-----
>>> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
>>> Sent: Thursday, July 02, 2015 9:41 PM
>>> To: Eric Auger; eric.auger@st.com; linux-arm-kernel@lists.infradead.org;
>>> kvmarm@lists.cs.columbia.edu; kvm@vger.kernel.org;
>>> christoffer.dall@linaro.org; marc.zyngier@arm.com;
>>> alex.williamson@redhat.com; avi.kivity@gmail.com; mtosatti@redhat.com;
>>> Wu, Feng; joro@8bytes.org; b.reynal@virtualopensystems.com
>>> Cc: linux-kernel@vger.kernel.org; patches@linaro.org
>>> Subject: Re: [RFC 12/17] irq: bypass: Extend skeleton for ARM forwarding
>>> control
>>>
>>>
>>>
>>> On 02/07/2015 15:17, Eric Auger wrote:
>>>> - new fields are added on producer side: linux irq, vfio_device handle,
>>>>   active which reflects whether the source is active (at interrupt
>>>>   controller level or at VFIO level - automasked -) and finally an
>>>>   opaque pointer which will be used to point to the vfio_platform_device
>>>>   in this series.
>>>
>>> Linux IRQ and active should be okay.  As to the vfio_device handle, you
>>> should link it from the vfio_platform_device instead.  And for the
>>> vfio_platform_device, you can link it from the vfio_platform_irq instead.
>>>
>>> Once you've done this, embed the irq_bypass_producer struct in the
>>> vfio_platform_irq struct; in the new kvm_arch_* functions, go back to
>>> the vfio_platform_irq struct via container_of.  From there you can
>>> retrieve pointers to the vfio_platform_device and the vfio_device.
>>>
>>>> - new fields on consumer side: the kvm handle, the gsi
>>>
>>> You do not need to add these.  Instead, add the kvm handle to irqfd
>>> only.  Like above, embed the irq_bypass_consumer struct in the irqfd
>>> struct; in the new kvm_arch_* functions, go back to the
>>> vfio_platform_irq struct via container_of.
>>>
>>
>> I also need the gsi field here, for posted-interrupts, I need 'gsi', 'irq' to
>> update the IRTE.
> 
> Oh... we can get gsi from irq_bypass_consumer -> _irqfd -> gsi, so it
> is not needed in irq_bypass_consumer. Got it! :)

The issue I have is that struct _irqfd is local to eventfd.c so it
cannot be used in archi specific code. Is it acceptable to move it to
kvm_host.h, naming it something like kvm_kernel_irqfd (as done for
kvm_kernel_irq_routing_entry)? Would also need to move _irqfd_resampler
there (kvm_kernel_irqfd_resampler).

irqfd user struct cannot be used in a standalone manner since we miss
the kvm handle.

Thanks

Eric



> 
> Thanks,
> Feng
> 
>>
>> Thanks,
>> Feng
>>
>>
>>> Paolo

RE: [RFC 12/17] irq: bypass: Extend skeleton for ARM forwarding control

[Wu, Feng]

> -----Original Message-----
> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo
> Bonzini
> Sent: Friday, July 03, 2015 2:52 PM
> To: Wu, Feng; Eric Auger; eric.auger@st.com;
> linux-arm-kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
> kvm@vger.kernel.org; christoffer.dall@linaro.org; marc.zyngier@arm.com;
> alex.williamson@redhat.com; avi.kivity@gmail.com; mtosatti@redhat.com;
> joro@8bytes.org; b.reynal@virtualopensystems.com
> Cc: linux-kernel@vger.kernel.org; patches@linaro.org
> Subject: Re: [RFC 12/17] irq: bypass: Extend skeleton for ARM forwarding
> control
> 
> 
> 
> On 03/07/2015 04:43, Wu, Feng wrote:
> >
> > struct irq_bypass_consumer {
> >        struct list_head node;
> >        void *token;
> > +      unsigned irq;	/*got from producer when registered*/
> >        void (*add_producer)(struct irq_bypass_producer *,
> >                             struct irq_bypass_consumer *);
> >        void (*del_producer)(struct irq_bypass_producer *,
> >                             struct irq_bypass_consumer *);
> > +      void (*update)(struct irq_bypass_consumer *);
> > };
> >
> > 'update' is used to update the IRTE, while irq is initialized when
> > registered, which is used to find the right IRTE.
> 
> Feel free to add "update" in your PI patches.  I am not sure if "irq"
> belongs here or in the containing struct.  You can play with both and
> submit the version that looks better to you.

Thanks for your review, Paolo. In my understanding, irq comes from
the producer side, while gsi belongs to the consumer, so we need
to get the irq from the producer somewhere. I am not sure adding
irq here is the good way, but what I need is in the 'update' function,
I have irq, gsi in hand. :)

Thanks,
Feng

> 
> Paolo

Re: [RFC 12/17] irq: bypass: Extend skeleton for ARM forwarding control

[Paolo Bonzini]

On 03/07/2015 08:54, Eric Auger wrote:
>> > Oh... we can get gsi from irq_bypass_consumer -> _irqfd -> gsi, so it
>> > is not needed in irq_bypass_consumer. Got it! :)
> The issue I have is that struct _irqfd is local to eventfd.c so it
> cannot be used in archi specific code. Is it acceptable to move it to
> kvm_host.h, naming it something like kvm_kernel_irqfd (as done for
> kvm_kernel_irq_routing_entry)? Would also need to move _irqfd_resampler
> there (kvm_kernel_irqfd_resampler).

Yes, that's okay.  Can you put it in a new header file kvm_irqfd.h though?

Paolo

> irqfd user struct cannot be used in a standalone manner since we miss
> the kvm handle.

Re: [RFC 12/17] irq: bypass: Extend skeleton for ARM forwarding control

[Paolo Bonzini]

On 03/07/2015 09:00, Wu, Feng wrote:
>>> > > struct irq_bypass_consumer {
>>> > >        struct list_head node;
>>> > >        void *token;
>>> > > +      unsigned irq;	/*got from producer when registered*/
>>> > >        void (*add_producer)(struct irq_bypass_producer *,
>>> > >                             struct irq_bypass_consumer *);
>>> > >        void (*del_producer)(struct irq_bypass_producer *,
>>> > >                             struct irq_bypass_consumer *);
>>> > > +      void (*update)(struct irq_bypass_consumer *);
>>> > > };
>>> > >
>>> > > 'update' is used to update the IRTE, while irq is initialized when
>>> > > registered, which is used to find the right IRTE.
>> > 
>> > Feel free to add "update" in your PI patches.  I am not sure if "irq"
>> > belongs here or in the containing struct.  You can play with both and
>> > submit the version that looks better to you.
> Thanks for your review, Paolo. In my understanding, irq comes from
> the producer side, while gsi belongs to the consumer, so we need
> to get the irq from the producer somewhere. I am not sure adding
> irq here is the good way, but what I need is in the 'update' function,
> I have irq, gsi in hand. :)

It's difficult to say without seeing the patches...  The IRQ is stored
in the producer already with Eric's changes.  If you need to store the
old IRQ value, because "update" needs to do something with it, then I
think "irq" belongs in the container struct.

Perhaps "update" needs to have a producer argument as well?

Paolo

Re: [RFC 12/17] irq: bypass: Extend skeleton for ARM forwarding control

[Paolo Bonzini]

On 02/07/2015 15:17, Eric Auger wrote:
> +	void (*stop_producer)(struct irq_bypass_producer *);
> +	void (*resume_producer)(struct irq_bypass_producer *);

Also, can you call these just "stop"/"resume" ...

> +	void (*add_consumer)(struct irq_bypass_producer *,
> +			     struct irq_bypass_consumer *);
> +	void (*del_consumer)(struct irq_bypass_producer *,
> +			     struct irq_bypass_consumer *);
>  };
>  
>  struct irq_bypass_consumer {
>  	struct list_head node;
>  	void *token;
> -	void (*add_producer)(struct irq_bypass_producer *);
> -	void (*del_producer)(struct irq_bypass_producer *);
> +	unsigned int gsi;		/* the guest gsi */
> +	struct kvm *kvm;
> +	void (*stop_consumer)(struct irq_bypass_consumer *);
> +	void (*resume_consumer)(struct irq_bypass_consumer *);

... and same here?  The KVM functions could be named

- kvm_arch_irq_bypass_add_producer
- kvm_arch_irq_bypass_del_producer
- kvm_arch_irq_bypass_stop
- kvm_arch_irq_bypass_resume

Paolo

RE: [RFC 12/17] irq: bypass: Extend skeleton for ARM forwarding control

[Wu, Feng]

> -----Original Message-----
> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> Sent: Friday, July 03, 2015 3:06 PM
> To: Wu, Feng; Eric Auger; eric.auger@st.com;
> linux-arm-kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
> kvm@vger.kernel.org; christoffer.dall@linaro.org; marc.zyngier@arm.com;
> alex.williamson@redhat.com; avi.kivity@gmail.com; mtosatti@redhat.com;
> joro@8bytes.org; b.reynal@virtualopensystems.com
> Cc: linux-kernel@vger.kernel.org; patches@linaro.org
> Subject: Re: [RFC 12/17] irq: bypass: Extend skeleton for ARM forwarding
> control
> 
> 
> 
> On 03/07/2015 09:00, Wu, Feng wrote:
> >>> > > struct irq_bypass_consumer {
> >>> > >        struct list_head node;
> >>> > >        void *token;
> >>> > > +      unsigned irq;	/*got from producer when registered*/
> >>> > >        void (*add_producer)(struct irq_bypass_producer *,
> >>> > >                             struct irq_bypass_consumer *);
> >>> > >        void (*del_producer)(struct irq_bypass_producer *,
> >>> > >                             struct irq_bypass_consumer *);
> >>> > > +      void (*update)(struct irq_bypass_consumer *);
> >>> > > };
> >>> > >
> >>> > > 'update' is used to update the IRTE, while irq is initialized when
> >>> > > registered, which is used to find the right IRTE.
> >> >
> >> > Feel free to add "update" in your PI patches.  I am not sure if "irq"
> >> > belongs here or in the containing struct.  You can play with both and
> >> > submit the version that looks better to you.
> > Thanks for your review, Paolo. In my understanding, irq comes from
> > the producer side, while gsi belongs to the consumer, so we need
> > to get the irq from the producer somewhere. I am not sure adding
> > irq here is the good way, but what I need is in the 'update' function,
> > I have irq, gsi in hand. :)
> 
> It's difficult to say without seeing the patches...  The IRQ is stored
> in the producer already with Eric's changes.  If you need to store the
> old IRQ value, because "update" needs to do something with it, then I
> think "irq" belongs in the container struct.
> 
> Perhaps "update" needs to have a producer argument as well?

I also consider this method, basically, I will call 'update' in irqfd_update(),
but seems I need do extra things to get the producer structure (such as,
iterate the producer list to find the one with the same 'token') before
calling 'update' from consumer side. I am not sure it is worth doing
that way.

Thanks,
Feng

> 
> Paolo

Re: [RFC 08/17] kvm: arm/arm64: implement kvm_arm_[halt,resume]_guest

[Eric Auger]

Christoffer, Marc,
On 07/02/2015 03:17 PM, Eric Auger wrote:
> On halt, the guest is forced to exit and prevented from being
> re-entered. This is synchronous.
> 
> Those two operations will be needed for IRQ forwarding setting.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
would you agree to handle this ARM functionality separately from the
forwarding series?

This includes 2 patch files, that one +
https://lkml.org/lkml/2015/7/2/288. This functionality is needed for
forwarding control since when changing the forwarding state we need to
"freeze" the state of the physical/virtual IRQ to undertake proper
actions. Stopping the guest makes sure it won't deactivate the virtual
IRQ while we are doing state change actions.

The forwarding series is quite heterogeneous (VFIO platform driver,
vgic, irq bypass manager) and I think it would simplify the review process.

Please let me know if you agree. If yes, I will post a separate series.

Best Regards

Eric
> 
> ---
> 
> RFC:
> - rename the function and this latter becomes static
> - remove __KVM_HAVE_ARCH_HALT_GUEST
> 
> v4 -> v5: add arm64 support
> - also defines __KVM_HAVE_ARCH_HALT_GUEST for arm64
> - add pause field
> ---
>  arch/arm/include/asm/kvm_host.h   |  3 +++
>  arch/arm/kvm/arm.c                | 32 +++++++++++++++++++++++++++++---
>  arch/arm64/include/asm/kvm_host.h |  3 +++
>  3 files changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 304004d..899ae27 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -132,6 +132,9 @@ struct kvm_vcpu_arch {
>  	/* vcpu power-off state */
>  	bool power_off;
>  
> +	/* Don't run the guest */
> +	bool pause;
> +
>  	/* IO related fields */
>  	struct kvm_decode mmio_decode;
>  
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 7537e68..4be6715 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -471,11 +471,36 @@ bool kvm_arch_intc_initialized(struct kvm *kvm)
>  	return vgic_initialized(kvm);
>  }
>  
> +static void kvm_arm_halt_guest(struct kvm *kvm)
> +{
> +	int i;
> +	struct kvm_vcpu *vcpu;
> +
> +	kvm_for_each_vcpu(i, vcpu, kvm)
> +		vcpu->arch.pause = true;
> +	force_vm_exit(cpu_all_mask);
> +}
> +
> +static void kvm_arm_resume_guest(struct kvm *kvm)
> +{
> +	int i;
> +	struct kvm_vcpu *vcpu;
> +
> +	kvm_for_each_vcpu(i, vcpu, kvm) {
> +		wait_queue_head_t *wq = kvm_arch_vcpu_wq(vcpu);
> +
> +		vcpu->arch.pause = false;
> +		wake_up_interruptible(wq);
> +	}
> +}
> +
> +
>  static void vcpu_pause(struct kvm_vcpu *vcpu)
>  {
>  	wait_queue_head_t *wq = kvm_arch_vcpu_wq(vcpu);
>  
> -	wait_event_interruptible(*wq, !vcpu->arch.power_off);
> +	wait_event_interruptible(*wq, ((!vcpu->arch.power_off) &&
> +				       (!vcpu->arch.pause)));
>  }
>  
>  static int kvm_vcpu_initialized(struct kvm_vcpu *vcpu)
> @@ -525,7 +550,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  
>  		update_vttbr(vcpu->kvm);
>  
> -		if (vcpu->arch.power_off)
> +		if (vcpu->arch.power_off || vcpu->arch.pause)
>  			vcpu_pause(vcpu);
>  
>  		/*
> @@ -551,7 +576,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  			run->exit_reason = KVM_EXIT_INTR;
>  		}
>  
> -		if (ret <= 0 || need_new_vmid_gen(vcpu->kvm)) {
> +		if (ret <= 0 || need_new_vmid_gen(vcpu->kvm) ||
> +		    vcpu->arch.pause) {
>  			local_irq_enable();
>  			preempt_enable();
>  			kvm_vgic_sync_hwstate(vcpu);
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 009da6b..69e3785 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -125,6 +125,9 @@ struct kvm_vcpu_arch {
>  	/* vcpu power-off state */
>  	bool power_off;
>  
> +	/* Don't run the guest */
> +	bool pause;
> +
>  	/* IO related fields */
>  	struct kvm_decode mmio_decode;
>  
> 

Re: [RFC 08/17] kvm: arm/arm64: implement kvm_arm_[halt,resume]_guest

[Marc Zyngier]

On 03/07/15 12:55, Eric Auger wrote:
> Christoffer, Marc,
> On 07/02/2015 03:17 PM, Eric Auger wrote:
>> On halt, the guest is forced to exit and prevented from being
>> re-entered. This is synchronous.
>>
>> Those two operations will be needed for IRQ forwarding setting.
>>
>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> would you agree to handle this ARM functionality separately from the
> forwarding series?
> 
> This includes 2 patch files, that one +
> https://lkml.org/lkml/2015/7/2/288. This functionality is needed for
> forwarding control since when changing the forwarding state we need to
> "freeze" the state of the physical/virtual IRQ to undertake proper
> actions. Stopping the guest makes sure it won't deactivate the virtual
> IRQ while we are doing state change actions.
> 
> The forwarding series is quite heterogeneous (VFIO platform driver,
> vgic, irq bypass manager) and I think it would simplify the review process.
> 
> Please let me know if you agree. If yes, I will post a separate series.

I don't mind, I trust you to do what's best for these series to be
easily reviewed (if that is at all possible).

The only thing is that this patch implements a feature that will
otherwise be unused, so annotating the functions with __maybe_unused
would avoid warnings.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [RFC 12/17] irq: bypass: Extend skeleton for ARM forwarding control

[Eric Auger]

Hi Paolo,
On 07/02/2015 03:40 PM, Paolo Bonzini wrote:
> 
> 
> On 02/07/2015 15:17, Eric Auger wrote:
>> - new fields are added on producer side: linux irq, vfio_device handle,
>>   active which reflects whether the source is active (at interrupt
>>   controller level or at VFIO level - automasked -) and finally an
>>   opaque pointer which will be used to point to the vfio_platform_device
>>   in this series.
> 
> Linux IRQ and active should be okay.  As to the vfio_device handle, you
> should link it from the vfio_platform_device instead.  And for the
> vfio_platform_device, you can link it from the vfio_platform_irq instead.
For this last one, I don't think this is achievable since if I store the
vfio_platform_irq in the opaque, it matches irqs[i] of
vfio_platform_device and I don't have any mean to retrieve "i" when
calling container_of.


	struct vfio_platform_irq *irq =
		container_of(prod, struct vfio_platform_irq, producer);
	struct vfio_platform_device *vpdev =
		container_of(irq, struct vfio_platform_device,  irqs[i?]);


struct vfio_platform_device {
../..
        struct vfio_platform_irq        *irqs;
../..
}
So I think I still need to pass vfio_platform_device in the opaque and
look on irqs array to identify the right vfio_platform_irq *.

Do I miss sthg?

- Eric


> 
> Once you've done this, embed the irq_bypass_producer struct in the
> vfio_platform_irq struct; in the new kvm_arch_* functions, go back to
> the vfio_platform_irq struct via container_of.  From there you can
> retrieve pointers to the vfio_platform_device and the vfio_device.
> 
>> - new fields on consumer side: the kvm handle, the gsi
> 
> You do not need to add these.  Instead, add the kvm handle to irqfd
> only.  Like above, embed the irq_bypass_consumer struct in the irqfd
> struct; in the new kvm_arch_* functions, go back to the
> vfio_platform_irq struct via container_of.
> 
> Paolo
> 

Re: [RFC 12/17] irq: bypass: Extend skeleton for ARM forwarding control

[Paolo Bonzini]

On 03/07/2015 15:12, Eric Auger wrote:
>> > Linux IRQ and active should be okay.  As to the vfio_device handle, you
>> > should link it from the vfio_platform_device instead.  And for the
>> > vfio_platform_device, you can link it from the vfio_platform_irq instead.
> For this last one, I don't think this is achievable since if I store the
> vfio_platform_irq in the opaque, it matches irqs[i] of
> vfio_platform_device and I don't have any mean to retrieve "i" when
> calling container_of.

Right, notice I said "link it":

	struct vfio_platform_irq *irq =
		container_of(prod, struct vfio_platform_irq, producer);
	struct vfio_platform_device *vpdev = irq->vpdev;
	struct vfio_device *vdev = vpdev->vdev;

Would this be okay?

Paolo

Re: [RFC 12/17] irq: bypass: Extend skeleton for ARM forwarding control

[Eric Auger]

On 07/03/2015 07:20 PM, Paolo Bonzini wrote:
> 
> 
> On 03/07/2015 15:12, Eric Auger wrote:
>>>> Linux IRQ and active should be okay.  As to the vfio_device handle, you
>>>> should link it from the vfio_platform_device instead.  And for the
>>>> vfio_platform_device, you can link it from the vfio_platform_irq instead.
>> For this last one, I don't think this is achievable since if I store the
>> vfio_platform_irq in the opaque, it matches irqs[i] of
>> vfio_platform_device and I don't have any mean to retrieve "i" when
>> calling container_of.
> 
> Right, notice I said "link it":
> 
> 	struct vfio_platform_irq *irq =
> 		container_of(prod, struct vfio_platform_irq, producer);
> 	struct vfio_platform_device *vpdev = irq->vpdev;
> 	struct vfio_device *vdev = vpdev->vdev;
> 
> Would this be okay?

Yes that's what I did. I added the vfio_device handle in struct
vfio_platform_irq

Thanks ;-)

Have a nice WE

Eric
> 
> Paolo
> 

RE: [RFC 16/17] KVM: eventfd: add irq bypass consumer management

[Wu, Feng]

> -----Original Message-----
> From: Eric Auger [mailto:eric.auger@linaro.org]
> Sent: Thursday, July 02, 2015 9:17 PM
> To: eric.auger@st.com; eric.auger@linaro.org;
> linux-arm-kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
> kvm@vger.kernel.org; christoffer.dall@linaro.org; marc.zyngier@arm.com;
> alex.williamson@redhat.com; pbonzini@redhat.com; avi.kivity@gmail.com;
> mtosatti@redhat.com; Wu, Feng; joro@8bytes.org;
> b.reynal@virtualopensystems.com
> Cc: linux-kernel@vger.kernel.org; patches@linaro.org
> Subject: [RFC 16/17] 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 <eric.auger@linaro.org>
> ---
>  virt/kvm/eventfd.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index f3da161..425a47b 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -34,6 +34,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>
> @@ -93,6 +94,7 @@ struct _irqfd {
>  	struct list_head list;
>  	poll_table pt;
>  	struct work_struct shutdown;
> +	struct irq_bypass_consumer *cons;
>  };
> 
>  static struct workqueue_struct *irqfd_cleanup_wq;
> @@ -429,7 +431,21 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd
> *args)
>  	 */
>  	fdput(f);
> 
> -	/* irq_bypass_register_consumer(); */
> +	irqfd->cons = kzalloc(sizeof(struct irq_bypass_consumer),
> +			      GFP_KERNEL);
> +	if (!irqfd->cons) {
> +		ret = -ENOMEM;
> +		goto fail;
> +	}
> +	irqfd->cons->token = (void *)irqfd->eventfd;
> +	irqfd->cons->gsi = irqfd->gsi;
> +	irqfd->cons->kvm = kvm;
> +	irqfd->cons->add_producer = kvm_arch_add_producer;
> +	irqfd->cons->del_producer = kvm_arch_del_producer;
> +	irqfd->cons->stop_consumer = kvm_arch_stop_consumer;
> +	irqfd->cons->resume_consumer = kvm_arch_resume_consumer;
> +	ret = irq_bypass_register_consumer(irqfd->cons);
> +	WARN_ON(ret);
> 
>  	return 0;
> 
> @@ -530,8 +546,6 @@ kvm_irqfd_deassign(struct kvm *kvm, struct kvm_irqfd
> *args)
>  	struct _irqfd *irqfd, *tmp;
>  	struct eventfd_ctx *eventfd;
> 
> -	/* irq_bypass_unregister_consumer() */
> -
>  	eventfd = eventfd_ctx_fdget(args->fd);
>  	if (IS_ERR(eventfd))
>  		return PTR_ERR(eventfd);
> @@ -550,6 +564,8 @@ kvm_irqfd_deassign(struct kvm *kvm, struct kvm_irqfd
> *args)
>  			irqfd->irq_entry.type = 0;
>  			write_seqcount_end(&irqfd->irq_entry_sc);
>  			irqfd_deactivate(irqfd);
> +			irq_bypass_unregister_consumer(irqfd->cons);
> +			kfree(irqfd->cons);

There may be an issue here. 'irqfd' is freed in irqfd_deactivate() --> ... --.>irqfd_shutdown(),
and irqfd_deactivate() can be called in the other two places below:
	- irqfd_wakeup()
	- kvm_irqfd_release()
I think we also need to call irq_bypass_unregister_consumer() there, right?

Thanks,
Feng


>  		}
>  	}
> 
> --
> 1.9.1

Re: [RFC 16/17] KVM: eventfd: add irq bypass consumer management

[Eric Auger]

Hi Feng,
On 07/06/2015 09:55 AM, Wu, Feng wrote:
> 
> 
>> -----Original Message-----
>> From: Eric Auger [mailto:eric.auger@linaro.org]
>> Sent: Thursday, July 02, 2015 9:17 PM
>> To: eric.auger@st.com; eric.auger@linaro.org;
>> linux-arm-kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
>> kvm@vger.kernel.org; christoffer.dall@linaro.org; marc.zyngier@arm.com;
>> alex.williamson@redhat.com; pbonzini@redhat.com; avi.kivity@gmail.com;
>> mtosatti@redhat.com; Wu, Feng; joro@8bytes.org;
>> b.reynal@virtualopensystems.com
>> Cc: linux-kernel@vger.kernel.org; patches@linaro.org
>> Subject: [RFC 16/17] 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 <eric.auger@linaro.org>
>> ---
>>  virt/kvm/eventfd.c | 22 +++++++++++++++++++---
>>  1 file changed, 19 insertions(+), 3 deletions(-)
>>
>> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
>> index f3da161..425a47b 100644
>> --- a/virt/kvm/eventfd.c
>> +++ b/virt/kvm/eventfd.c
>> @@ -34,6 +34,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>
>> @@ -93,6 +94,7 @@ struct _irqfd {
>>  	struct list_head list;
>>  	poll_table pt;
>>  	struct work_struct shutdown;
>> +	struct irq_bypass_consumer *cons;
>>  };
>>
>>  static struct workqueue_struct *irqfd_cleanup_wq;
>> @@ -429,7 +431,21 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd
>> *args)
>>  	 */
>>  	fdput(f);
>>
>> -	/* irq_bypass_register_consumer(); */
>> +	irqfd->cons = kzalloc(sizeof(struct irq_bypass_consumer),
>> +			      GFP_KERNEL);
>> +	if (!irqfd->cons) {
>> +		ret = -ENOMEM;
>> +		goto fail;
>> +	}
>> +	irqfd->cons->token = (void *)irqfd->eventfd;
>> +	irqfd->cons->gsi = irqfd->gsi;
>> +	irqfd->cons->kvm = kvm;
>> +	irqfd->cons->add_producer = kvm_arch_add_producer;
>> +	irqfd->cons->del_producer = kvm_arch_del_producer;
>> +	irqfd->cons->stop_consumer = kvm_arch_stop_consumer;
>> +	irqfd->cons->resume_consumer = kvm_arch_resume_consumer;
>> +	ret = irq_bypass_register_consumer(irqfd->cons);
>> +	WARN_ON(ret);
>>
>>  	return 0;
>>
>> @@ -530,8 +546,6 @@ kvm_irqfd_deassign(struct kvm *kvm, struct kvm_irqfd
>> *args)
>>  	struct _irqfd *irqfd, *tmp;
>>  	struct eventfd_ctx *eventfd;
>>
>> -	/* irq_bypass_unregister_consumer() */
>> -
>>  	eventfd = eventfd_ctx_fdget(args->fd);
>>  	if (IS_ERR(eventfd))
>>  		return PTR_ERR(eventfd);
>> @@ -550,6 +564,8 @@ kvm_irqfd_deassign(struct kvm *kvm, struct kvm_irqfd
>> *args)
>>  			irqfd->irq_entry.type = 0;
>>  			write_seqcount_end(&irqfd->irq_entry_sc);
>>  			irqfd_deactivate(irqfd);
>> +			irq_bypass_unregister_consumer(irqfd->cons);
>> +			kfree(irqfd->cons);
> 
> There may be an issue here. 'irqfd' is freed in irqfd_deactivate() --> ... --.>irqfd_shutdown(),
> and irqfd_deactivate() can be called in the other two places below:
> 	- irqfd_wakeup()
> 	- kvm_irqfd_release()
> I think we also need to call irq_bypass_unregister_consumer() there, right?
yes you're right. what about doing the unregistration in irqfd_shutdown
then?

Thanks for spotting this.

Eric

> 
> Thanks,
> Feng
> 
> 
>>  		}
>>  	}
>>
>> --
>> 1.9.1
> 

RE: [RFC 16/17] KVM: eventfd: add irq bypass consumer management

[Wu, Feng]

> -----Original Message-----
> From: Eric Auger [mailto:eric.auger@linaro.org]
> Sent: Monday, July 06, 2015 7:20 PM
> To: Wu, Feng; eric.auger@st.com; linux-arm-kernel@lists.infradead.org;
> kvmarm@lists.cs.columbia.edu; kvm@vger.kernel.org;
> christoffer.dall@linaro.org; marc.zyngier@arm.com;
> alex.williamson@redhat.com; pbonzini@redhat.com; avi.kivity@gmail.com;
> mtosatti@redhat.com; joro@8bytes.org; b.reynal@virtualopensystems.com
> Cc: linux-kernel@vger.kernel.org; patches@linaro.org
> Subject: Re: [RFC 16/17] KVM: eventfd: add irq bypass consumer management
> 
> Hi Feng,
> On 07/06/2015 09:55 AM, Wu, Feng wrote:
> >
> >
> >> -----Original Message-----
> >> From: Eric Auger [mailto:eric.auger@linaro.org]
> >> Sent: Thursday, July 02, 2015 9:17 PM
> >> To: eric.auger@st.com; eric.auger@linaro.org;
> >> linux-arm-kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
> >> kvm@vger.kernel.org; christoffer.dall@linaro.org; marc.zyngier@arm.com;
> >> alex.williamson@redhat.com; pbonzini@redhat.com; avi.kivity@gmail.com;
> >> mtosatti@redhat.com; Wu, Feng; joro@8bytes.org;
> >> b.reynal@virtualopensystems.com
> >> Cc: linux-kernel@vger.kernel.org; patches@linaro.org
> >> Subject: [RFC 16/17] 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 <eric.auger@linaro.org>
> >> ---
> >>  virt/kvm/eventfd.c | 22 +++++++++++++++++++---
> >>  1 file changed, 19 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> >> index f3da161..425a47b 100644
> >> --- a/virt/kvm/eventfd.c
> >> +++ b/virt/kvm/eventfd.c
> >> @@ -34,6 +34,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>
> >> @@ -93,6 +94,7 @@ struct _irqfd {
> >>  	struct list_head list;
> >>  	poll_table pt;
> >>  	struct work_struct shutdown;
> >> +	struct irq_bypass_consumer *cons;
> >>  };
> >>
> >>  static struct workqueue_struct *irqfd_cleanup_wq;
> >> @@ -429,7 +431,21 @@ kvm_irqfd_assign(struct kvm *kvm, struct
> kvm_irqfd
> >> *args)
> >>  	 */
> >>  	fdput(f);
> >>
> >> -	/* irq_bypass_register_consumer(); */
> >> +	irqfd->cons = kzalloc(sizeof(struct irq_bypass_consumer),
> >> +			      GFP_KERNEL);
> >> +	if (!irqfd->cons) {
> >> +		ret = -ENOMEM;
> >> +		goto fail;
> >> +	}
> >> +	irqfd->cons->token = (void *)irqfd->eventfd;
> >> +	irqfd->cons->gsi = irqfd->gsi;
> >> +	irqfd->cons->kvm = kvm;
> >> +	irqfd->cons->add_producer = kvm_arch_add_producer;
> >> +	irqfd->cons->del_producer = kvm_arch_del_producer;
> >> +	irqfd->cons->stop_consumer = kvm_arch_stop_consumer;
> >> +	irqfd->cons->resume_consumer = kvm_arch_resume_consumer;
> >> +	ret = irq_bypass_register_consumer(irqfd->cons);
> >> +	WARN_ON(ret);
> >>
> >>  	return 0;
> >>
> >> @@ -530,8 +546,6 @@ kvm_irqfd_deassign(struct kvm *kvm, struct
> kvm_irqfd
> >> *args)
> >>  	struct _irqfd *irqfd, *tmp;
> >>  	struct eventfd_ctx *eventfd;
> >>
> >> -	/* irq_bypass_unregister_consumer() */
> >> -
> >>  	eventfd = eventfd_ctx_fdget(args->fd);
> >>  	if (IS_ERR(eventfd))
> >>  		return PTR_ERR(eventfd);
> >> @@ -550,6 +564,8 @@ kvm_irqfd_deassign(struct kvm *kvm, struct
> kvm_irqfd
> >> *args)
> >>  			irqfd->irq_entry.type = 0;
> >>  			write_seqcount_end(&irqfd->irq_entry_sc);
> >>  			irqfd_deactivate(irqfd);
> >> +			irq_bypass_unregister_consumer(irqfd->cons);
> >> +			kfree(irqfd->cons);
> >
> > There may be an issue here. 'irqfd' is freed in irqfd_deactivate() --> ...
> --.>irqfd_shutdown(),
> > and irqfd_deactivate() can be called in the other two places below:
> > 	- irqfd_wakeup()
> > 	- kvm_irqfd_release()
> > I think we also need to call irq_bypass_unregister_consumer() there, right?
> yes you're right. what about doing the unregistration in irqfd_shutdown
> then?

I am fine with this!

Thanks,
Feng
> 
> Thanks for spotting this.
> 
> Eric
> 
> >
> > Thanks,
> > Feng
> >
> >
> >>  		}
> >>  	}
> >>
> >> --
> >> 1.9.1
> >