[RFC 0/9] KVM-VFIO IRQ forward control

[RFC 0/9] KVM-VFIO IRQ forward control

[Eric Auger]

This RFC proposes an integration of "ARM: Forwarding physical
interrupts to a guest VM" (http://lwn.net/Articles/603514/) in
KVM.

It enables to transform a VFIO platform driver IRQ into a forwarded
IRQ. The direct benefit is that, for a level sensitive IRQ, a VM
switch can be avoided on guest virtual IRQ completion. Before this
patch, a maintenance IRQ was triggered on the virtual IRQ completion.

When the IRQ is forwarded, the VFIO platform driver does not need to
disable the IRQ anymore. Indeed when returning from the IRQ handler
the IRQ is not deactivated. Only its priority is lowered. This means
the same IRQ cannot hit before the guest completes the virtual IRQ
and the GIC automatically deactivates the corresponding physical IRQ.

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 this latter becomes "transparent".

The current integration is based on an extension of the KVM-VFIO
device, previously used by KVM to interact with VFIO groups. The
patch serie now enables KVM to directly interact with a VFIO
platform device. The VFIO external API was extended for that purpose.

Th KVM-VFIO device can get/put the vfio platform device, check its
integrity and type, get the IRQ number associated to an IRQ index.

The KVM-VFIO is extended with an architecture specific implementation.
IRQ forward control is implemented in the ARM specific part.

from a user point of view, the functionality is provided through new
KVM-VFIO device commands, KVM_DEV_VFIO_DEVICE_(DE)ASSIGN_IRQ
and the capability can be checked with KVM_HAS_DEVICE_ATTR.
Assignment can only be changed when the physical IRQ is not active.
It is the responsability of the user to do this check.

This patch serie has the following dependencies:
- "ARM: Forwarding physical interrupts to a guest VM"
  (http://lwn.net/Articles/603514/) in
- [PATCH v2] irqfd for ARM
  which itself depends on
  - arm/arm64: KVM: Various VGIC cleanups and improvements
    http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/263685.html
- and obviously the VFIO platform driver serie:
  [RFC PATCH v6 00/20] VFIO support for platform devices on ARM
  https://www.mail-archive.com/kvm@vger.kernel.org/msg103247.html

Integrated pieces can be found at
git://git.linaro.org/people/eric.auger/linux.git
on branch 3.17rc1_forward_integ_v0

This was was tested on Calxeda Miday, assigning the xgmac main IRQ.


Eric Auger (9):
  KVM: ARM: VGIC: fix multiple injection of level sensitive forwarded
    IRQ
  KVM: ARM: VGIC: add forwarded irq rbtree lock
  VFIO: platform: handler tests whether the IRQ is forwarded
  KVM: KVM-VFIO: update user API to program forwarded IRQ
  VFIO: Extend external user API
  KVM: KVM-VFIO: allow arch specific implementation
  KVM: KVM-VFIO: add new VFIO external API hooks
  KVM: KVM-VFIO: add kvm_vfio_arch_data and accessors
  KVM: KVM_VFIO: ARM: implement irq forwarding control

 Documentation/virtual/kvm/devices/vfio.txt |  25 ++
 arch/arm/include/asm/kvm_host.h            |  16 +
 arch/arm/include/uapi/asm/kvm.h            |   6 +
 arch/arm/kvm/Makefile                      |   2 +-
 arch/arm/kvm/kvm_vfio_arm.c                | 599 +++++++++++++++++++++++++++++
 drivers/vfio/platform/vfio_platform_irq.c  |   7 +-
 drivers/vfio/vfio.c                        |  35 ++
 include/kvm/arm_vgic.h                     |   1 +
 include/linux/kvm_host.h                   |  30 ++
 include/linux/vfio.h                       |   4 +
 include/uapi/linux/kvm.h                   |   3 +
 virt/kvm/arm/vgic.c                        |  55 ++-
 virt/kvm/vfio.c                            |  92 +++++
 13 files changed, 862 insertions(+), 13 deletions(-)
 create mode 100644 arch/arm/kvm/kvm_vfio_arm.c

-- 
1.9.1

[RFC 1/9] KVM: ARM: VGIC: fix multiple injection of level sensitive forwarded IRQ

[Eric Auger]

Fix multiple injection of level sensitive forwarded IRQs.
With current code, the second injection fails since the state bitmaps
are not reset (process_maintenance is not called anymore).
New implementation consists in fully bypassing the vgic state
management for forwarded IRQ (checks are ignored in
vgic_update_irq_pending). This obviously assumes the forwarded IRQ is
injected from kernel side.

---
  It was attempted to reset the states in __kvm_vgic_sync_hwstate, checking
  the emptied LR of forwarded IRQ. However surprisingly this solution does
  not seem to work. Some times, a new forwarded IRQ injection is observed
  while the LR of the previous instance was not observed as empty.

Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
 include/kvm/arm_vgic.h | 1 +
 virt/kvm/arm/vgic.c    | 9 +++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 743020f..3da244f 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -177,6 +177,7 @@ struct vgic_dist {
 	unsigned long		irq_pending_on_cpu;
 
 	struct rb_root		irq_phys_map;
+	spinlock_t			rb_tree_lock;
 #endif
 };
 
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 0007300..195c10c 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -1517,14 +1517,18 @@ static bool vgic_update_irq_pending(struct kvm *kvm, int cpuid,
 	int edge_triggered, level_triggered;
 	int enabled;
 	bool ret = true;
+	bool is_forwarded;
 
 	spin_lock(&dist->lock);
 
 	vcpu = kvm_get_vcpu(kvm, cpuid);
+	is_forwarded = (vgic_get_phys_irq(vcpu, irq_num) >0);
+		
 	edge_triggered = vgic_irq_is_edge(vcpu, irq_num);
 	level_triggered = !edge_triggered;
 
-	if (!vgic_validate_injection(vcpu, irq_num, level)) {
+	if (!is_forwarded &&
+		!vgic_validate_injection(vcpu, irq_num, level)) {
 		ret = false;
 		goto out;
 	}
@@ -1557,7 +1561,8 @@ static bool vgic_update_irq_pending(struct kvm *kvm, int cpuid,
 		goto out;
 	}
 
-	if (level_triggered && vgic_irq_is_queued(vcpu, irq_num)) {
+	if (!is_forwarded &&
+		level_triggered && vgic_irq_is_queued(vcpu, irq_num)) {
 		/*
 		 * Level interrupt in progress, will be picked up
 		 * when EOId.
-- 
1.9.1

[RFC 2/9] KVM: ARM: VGIC: add forwarded irq rbtree lock

[Eric Auger]

add a lock related to the rb tree manipulation. The rb tree can be
searched in one thread (irqfd handler for instance) and map/unmap
happen in another.

Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
 virt/kvm/arm/vgic.c | 46 +++++++++++++++++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 9 deletions(-)

diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 195c10c..3311e0a 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -1628,9 +1628,15 @@ static struct rb_root *vgic_get_irq_phys_map(struct kvm_vcpu *vcpu,
 
 int vgic_map_phys_irq(struct kvm_vcpu *vcpu, int virt_irq, int phys_irq)
 {
-	struct rb_root *root = vgic_get_irq_phys_map(vcpu, virt_irq);
-	struct rb_node **new = &root->rb_node, *parent = NULL;
+	struct rb_root *root;
+	struct rb_node **new, *parent = NULL;
 	struct irq_phys_map *new_map;
+	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
+
+	spin_lock(&dist->rb_tree_lock);
+
+	root = vgic_get_irq_phys_map(vcpu, virt_irq);
+	new = &root->rb_node;
 
 	/* Boilerplate rb_tree code */
 	while (*new) {
@@ -1642,13 +1648,17 @@ int vgic_map_phys_irq(struct kvm_vcpu *vcpu, int virt_irq, int phys_irq)
 			new = &(*new)->rb_left;
 		else if (this->virt_irq > virt_irq)
 			new = &(*new)->rb_right;
-		else
+		else {
+			spin_unlock(&dist->rb_tree_lock);
 			return -EEXIST;
+		}
 	}
 
 	new_map = kzalloc(sizeof(*new_map), GFP_KERNEL);
-	if (!new_map)
+	if (!new_map) {
+		spin_unlock(&dist->rb_tree_lock);
 		return -ENOMEM;
+	}
 
 	new_map->virt_irq = virt_irq;
 	new_map->phys_irq = phys_irq;
@@ -1656,6 +1666,8 @@ int vgic_map_phys_irq(struct kvm_vcpu *vcpu, int virt_irq, int phys_irq)
 	rb_link_node(&new_map->node, parent, new);
 	rb_insert_color(&new_map->node, root);
 
+	spin_unlock(&dist->rb_tree_lock);
+
 	return 0;
 }
 
@@ -1683,24 +1695,39 @@ static struct irq_phys_map *vgic_irq_map_search(struct kvm_vcpu *vcpu,
 
 int vgic_get_phys_irq(struct kvm_vcpu *vcpu, int virt_irq)
 {
-	struct irq_phys_map *map = vgic_irq_map_search(vcpu, virt_irq);
+	struct irq_phys_map *map;
+	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
+	int ret;
+
+	spin_lock(&dist->rb_tree_lock);
+	map = vgic_irq_map_search(vcpu, virt_irq);
 
 	if (map)
-		return map->phys_irq;
+		ret = map->phys_irq;
+	else
+		ret =  -ENOENT;
+
+	spin_unlock(&dist->rb_tree_lock);
+	return ret;
 
-	return -ENOENT;
 }
 
 int vgic_unmap_phys_irq(struct kvm_vcpu *vcpu, int virt_irq, int phys_irq)
 {
-	struct irq_phys_map *map = vgic_irq_map_search(vcpu, virt_irq);
+	struct irq_phys_map *map;
+	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
+
+	spin_lock(&dist->rb_tree_lock);
+
+	map = vgic_irq_map_search(vcpu, virt_irq);
 
 	if (map && map->phys_irq == phys_irq) {
 		rb_erase(&map->node, vgic_get_irq_phys_map(vcpu, virt_irq));
 		kfree(map);
+		spin_unlock(&dist->rb_tree_lock);
 		return 0;
 	}
-
+	spin_unlock(&dist->rb_tree_lock);
 	return -ENOENT;
 }
 
@@ -1896,6 +1923,7 @@ int kvm_vgic_create(struct kvm *kvm)
 	}
 
 	spin_lock_init(&kvm->arch.vgic.lock);
+	spin_lock_init(&kvm->arch.vgic.rb_tree_lock);
 	kvm->arch.vgic.in_kernel = true;
 	kvm->arch.vgic.vctrl_base = vgic->vctrl_base;
 	kvm->arch.vgic.vgic_dist_base = VGIC_ADDR_UNDEF;
-- 
1.9.1

[RFC 3/9] VFIO: platform: handler tests whether the IRQ is forwarded

[Eric Auger]

In case the IRQ is forwarded, the VFIO platform IRQ handler does not
need to disable the IRQ anymore. In that mode, when the handler completes
the IRQ is not deactivated but only its priority is lowered.

Some other actor (typically a guest) is supposed to deactivate the IRQ,
allowing at that time a new physical IRQ to hit.

In virtualization use case, the physical IRQ is automatically completed
by the interrupt controller when the guest completes the corresponding
virtual IRQ.

Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
 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 6768508..1f851b2 100644
--- a/drivers/vfio/platform/vfio_platform_irq.c
+++ b/drivers/vfio/platform/vfio_platform_irq.c
@@ -88,13 +88,18 @@ static irqreturn_t vfio_irq_handler(int irq, void *dev_id)
 	struct vfio_platform_irq *irq_ctx = dev_id;
 	unsigned long flags;
 	int ret = IRQ_NONE;
+	struct irq_data *d;
+	bool is_forwarded;
 
 	spin_lock_irqsave(&irq_ctx->lock, flags);
 
 	if (!irq_ctx->masked) {
 		ret = IRQ_HANDLED;
+		d = irq_get_irq_data(irq_ctx->hwirq);
+		is_forwarded = irqd_irq_forwarded(d);
 
-		if (irq_ctx->flags & VFIO_IRQ_INFO_AUTOMASKED) {
+		if (irq_ctx->flags & VFIO_IRQ_INFO_AUTOMASKED &&
+						!is_forwarded) {
 			disable_irq_nosync(irq_ctx->hwirq);
 			irq_ctx->masked = true;
 		}
-- 
1.9.1

[RFC 4/9] KVM: KVM-VFIO: update user API to program forwarded IRQ

[Eric Auger]

add new device group commands:
- KVM_DEV_VFIO_DEVICE_ASSIGN_IRQ and
  KVM_DEV_VFIO_DEVICE_DEASSIGN_IRQ

which enable to turn forwarded IRQ mode on/off.

Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
 Documentation/virtual/kvm/devices/vfio.txt | 25 +++++++++++++++++++++++++
 arch/arm/include/uapi/asm/kvm.h            |  6 ++++++
 include/uapi/linux/kvm.h                   |  3 +++
 3 files changed, 34 insertions(+)

diff --git a/Documentation/virtual/kvm/devices/vfio.txt b/Documentation/virtual/kvm/devices/vfio.txt
index ef51740..c8b3fa1 100644
--- a/Documentation/virtual/kvm/devices/vfio.txt
+++ b/Documentation/virtual/kvm/devices/vfio.txt
@@ -13,6 +13,7 @@ VFIO-group is held by KVM.
 
 Groups:
   KVM_DEV_VFIO_GROUP
+  KVM_DEV_VFIO_DEVICE
 
 KVM_DEV_VFIO_GROUP attributes:
   KVM_DEV_VFIO_GROUP_ADD: Add a VFIO group to VFIO-KVM device tracking
@@ -20,3 +21,27 @@ KVM_DEV_VFIO_GROUP attributes:
 
 For each, kvm_device_attr.addr points to an int32_t file descriptor
 for the VFIO group.
+
+KVM_DEV_VFIO_DEVICE attributes:
+  KVM_DEV_VFIO_DEVICE_ASSIGN_IRQ
+  KVM_DEV_VFIO_DEVICE_DEASSIGN_IRQ
+
+For each, kvm_device_attr.addr points to an kvm_arch_forwarded_irq.
+This user API makes possible to create a special IRQ handling mode,
+currently supported only on ARM, where KVM and a VFIO platform driver
+collaborate to improve IRQ handling performance.
+fd represents the file descriptor of a valid VFIO device whose physical
+IRQ, referenced by its irq_index is injected to the VM guest_irq.
+
+On ASSIGN_IRQ, KVM-VFIO device programs:
+- the host, to not complete the physical IRQ itself.
+- the GIC, to automatically complete the physical IRQ when the guest
+  completes the virtual IRQ
+This avoid trapping the end-of-interrupt for level sensitive IRQ.
+
+On DEASSIGN_IRQ, one come back to the mode where the host completes the
+physical IRQ and the guest only completes the virtual IRQ.
+
+It is up to the caller of this API to get the assurance the IRQ is not
+outstanding when the ASSIGN/DEASSIGN is called. This could lead to some
+inconsistency on who is going to complete the IRQ.
diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
index 3034c66..1920b33 100644
--- a/arch/arm/include/uapi/asm/kvm.h
+++ b/arch/arm/include/uapi/asm/kvm.h
@@ -109,6 +109,12 @@ struct kvm_sync_regs {
 struct kvm_arch_memory_slot {
 };
 
+struct kvm_arch_forwarded_irq {
+	__u32 fd; /* file desciptor of the VFIO device */
+	__u32 irq_index; /* platform device index of the IRQ */
+	__u32 guest_irq; /* virtual IRQ number */
+};
+
 /* If you need to interpret the index values, here is the key: */
 #define KVM_REG_ARM_COPROC_MASK		0x000000000FFF0000
 #define KVM_REG_ARM_COPROC_SHIFT	16
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index cf3a2ff..b149ba8 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -954,6 +954,9 @@ struct kvm_device_attr {
 #define  KVM_DEV_VFIO_GROUP			1
 #define   KVM_DEV_VFIO_GROUP_ADD			1
 #define   KVM_DEV_VFIO_GROUP_DEL			2
+#define  KVM_DEV_VFIO_DEVICE			2
+#define   KVM_DEV_VFIO_DEVICE_ASSIGN_IRQ			1
+#define   KVM_DEV_VFIO_DEVICE_DEASSIGN_IRQ			2
 #define KVM_DEV_TYPE_ARM_VGIC_V2	5
 #define KVM_DEV_TYPE_FLIC		6
 
-- 
1.9.1

[RFC 5/9] VFIO: Extend external user API

[Eric Auger]

New functions are added to be called from ARM KVM-VFIO device.

- vfio_device_get_external_user enables to get a vfio device from
  its fd
- vfio_device_put_external_user puts the vfio device
- vfio_external_get_type enables to retrieve the type of the device
  (PCI or platform)
- vfio_external_get_base_device enables to get the
  struct device*, useful to access the platform_device

Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
 drivers/vfio/vfio.c  | 35 +++++++++++++++++++++++++++++++++++
 include/linux/vfio.h |  4 ++++
 2 files changed, 39 insertions(+)

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 8e84471..c93b9e4 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1401,6 +1401,41 @@ void vfio_group_put_external_user(struct vfio_group *group)
 }
 EXPORT_SYMBOL_GPL(vfio_group_put_external_user);
 
+struct vfio_device *vfio_device_get_external_user(struct file *filep)
+{
+	struct vfio_device *vdev = filep->private_data;
+
+	if (filep->f_op != &vfio_device_fops)
+		return ERR_PTR(-EINVAL);
+
+	vfio_device_get(vdev);
+	return vdev;
+}
+EXPORT_SYMBOL_GPL(vfio_device_get_external_user);
+
+void vfio_device_put_external_user(struct vfio_device *vdev)
+{
+	vfio_device_put(vdev);
+}
+EXPORT_SYMBOL_GPL(vfio_device_put_external_user);
+
+int vfio_external_get_type(struct vfio_device *vdev)
+{
+	if (!strcmp(vdev->ops->name,  "vfio-platform"))
+		return VFIO_DEVICE_FLAGS_PLATFORM;
+	else if (!strcmp(vdev->ops->name,  "vfio-pci"))
+		return VFIO_DEVICE_FLAGS_PCI;
+	else
+		return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(vfio_external_get_type);
+
+struct device *vfio_external_get_base_device(struct vfio_device *vdev)
+{
+	return vdev->dev;
+}
+EXPORT_SYMBOL_GPL(vfio_external_get_base_device);
+
 int vfio_external_user_iommu_id(struct vfio_group *group)
 {
 	return iommu_group_id(group->iommu_group);
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index ffe04ed..19e98eb 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -99,6 +99,10 @@ extern void vfio_group_put_external_user(struct vfio_group *group);
 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 struct vfio_device *vfio_device_get_external_user(struct file *filep);
+extern void vfio_device_put_external_user(struct vfio_device *vdev);
+extern int vfio_external_get_type(struct vfio_device *vdev);
+extern struct device *vfio_external_get_base_device(struct vfio_device *vdev);
 
 struct pci_dev;
 #ifdef CONFIG_EEH
-- 
1.9.1

[RFC 6/9] KVM: KVM-VFIO: allow arch specific implementation

[Eric Auger]

introduce a new option __KVM_HAVE_ARCH_KVM_VFIO option.
When set the generic KVM-VFIO code calls architecture dependent
code.

the architecture dependent hooks are
- kvm_arch_vfio_has_attr
- kvm_arch_vfio_set_attr
- kvm_arch_vfio_init
- kvm_arch_vfio_destroy

Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
 include/linux/kvm_host.h | 30 ++++++++++++++++++++++++++++++
 virt/kvm/vfio.c          |  9 +++++++++
 2 files changed, 39 insertions(+)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index a4c33b3..c4ce4af 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1075,6 +1075,36 @@ extern struct kvm_device_ops kvm_vfio_ops;
 extern struct kvm_device_ops kvm_arm_vgic_v2_ops;
 extern struct kvm_device_ops kvm_flic_ops;
 
+#ifdef __KVM_HAVE_ARCH_KVM_VFIO
+
+int kvm_arch_vfio_has_attr(struct kvm_device *dev,
+			   struct kvm_device_attr *attr);
+int kvm_arch_vfio_set_attr(struct kvm_device *dev,
+			   struct kvm_device_attr *attr);
+int kvm_arch_vfio_init(struct kvm_device *dev);
+
+void kvm_arch_vfio_destroy(struct kvm_device *dev);
+
+#else
+static inline int kvm_arch_vfio_has_attr(struct kvm_device *dev,
+				  struct kvm_device_attr *attr)
+{
+	return -ENXIO;
+}
+static inline int kvm_arch_vfio_set_attr(struct kvm_device *dev,
+				  struct kvm_device_attr *attr)
+{
+	return -ENXIO;
+}
+static inline int kvm_arch_vfio_init(struct kvm_device *dev)
+{
+	return 0;
+}
+static inline void kvm_arch_vfio_destroy(struct kvm_device *dev)
+{
+}
+#endif
+
 #ifdef CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT
 
 static inline void kvm_vcpu_set_in_spin_loop(struct kvm_vcpu *vcpu, bool val)
diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
index ba1a93f..89d3b75 100644
--- a/virt/kvm/vfio.c
+++ b/virt/kvm/vfio.c
@@ -207,6 +207,8 @@ static int kvm_vfio_set_attr(struct kvm_device *dev,
 	switch (attr->group) {
 	case KVM_DEV_VFIO_GROUP:
 		return kvm_vfio_set_group(dev, attr->attr, attr->addr);
+	default:
+		return kvm_arch_vfio_set_attr(dev, attr);
 	}
 
 	return -ENXIO;
@@ -224,6 +226,9 @@ static int kvm_vfio_has_attr(struct kvm_device *dev,
 		}
 
 		break;
+
+	default:
+		kvm_arch_vfio_has_attr(dev, attr);
 	}
 
 	return -ENXIO;
@@ -234,6 +239,8 @@ static void kvm_vfio_destroy(struct kvm_device *dev)
 	struct kvm_vfio *kv = dev->private;
 	struct kvm_vfio_group *kvg, *tmp;
 
+	kvm_arch_vfio_destroy(dev);
+
 	list_for_each_entry_safe(kvg, tmp, &kv->group_list, node) {
 		kvm_vfio_group_put_external_user(kvg->vfio_group);
 		list_del(&kvg->node);
@@ -265,6 +272,8 @@ static int kvm_vfio_create(struct kvm_device *dev, u32 type)
 
 	dev->private = kv;
 
+	kvm_arch_vfio_init(dev);
+
 	return 0;
 }
 
-- 
1.9.1

[RFC 7/9] KVM: KVM-VFIO: add new VFIO external API hooks

[Eric Auger]

add functions that implement the gateway to the extended
external VFIO API:
- kvm_vfio_device_get_external_user
- kvm_vfio_device_put_external_user
- kvm_vfio_external_get_type
- kvm_vfio_external_get_base_device

Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
 arch/arm/include/asm/kvm_host.h |  6 ++++
 virt/kvm/vfio.c                 | 62 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 6dfb404..62cbf5b 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -171,6 +171,12 @@ void kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte);
 unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu);
 int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices);
 
+struct vfio_device;
+struct vfio_device *kvm_vfio_device_get_external_user(struct file *filep);
+void kvm_vfio_device_put_external_user(struct vfio_device *vdev);
+int kvm_vfio_external_get_type(struct vfio_device *vdev);
+struct device *kvm_vfio_external_get_base_device(struct vfio_device *vdev);
+
 /* We do not have shadow page tables, hence the empty hooks */
 static inline int kvm_age_hva(struct kvm *kvm, unsigned long hva)
 {
diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
index 89d3b75..f1c4e35 100644
--- a/virt/kvm/vfio.c
+++ b/virt/kvm/vfio.c
@@ -59,6 +59,67 @@ static void kvm_vfio_group_put_external_user(struct vfio_group *vfio_group)
 	symbol_put(vfio_group_put_external_user);
 }
 
+struct vfio_device *kvm_vfio_device_get_external_user(struct file *filep)
+{
+	struct vfio_device *vdev;
+	struct vfio_device *(*fn)(struct file *);
+
+	fn = symbol_get(vfio_device_get_external_user);
+	if (!fn)
+		return ERR_PTR(-EINVAL);
+
+	vdev = fn(filep);
+
+	symbol_put(vfio_device_get_external_user);
+
+	return vdev;
+}
+
+void kvm_vfio_device_put_external_user(struct vfio_device *vdev)
+{
+	void (*fn)(struct vfio_device *);
+
+	fn = symbol_get(vfio_device_put_external_user);
+	if (!fn)
+		return;
+
+	fn(vdev);
+
+	symbol_put(vfio_device_put_external_user);
+}
+
+int kvm_vfio_external_get_type(struct vfio_device *vdev)
+{
+	int (*fn)(struct vfio_device *);
+	int ret;
+
+	fn = symbol_get(vfio_external_get_type);
+	if (!fn)
+		return -EINVAL;
+
+	ret = fn(vdev);
+
+	symbol_put(vfio_external_get_type);
+
+	return ret;
+}
+
+struct device *kvm_vfio_external_get_base_device(struct vfio_device *vdev)
+{
+	struct device *(*fn)(struct vfio_device *);
+	struct device *dev;
+
+	fn = symbol_get(vfio_external_get_base_device);
+	if (!fn)
+		return NULL;
+
+	dev = fn(vdev);
+
+	symbol_put(vfio_external_get_base_device);
+
+	return dev;
+}
+
 static bool kvm_vfio_group_is_coherent(struct vfio_group *vfio_group)
 {
 	long (*fn)(struct vfio_group *, unsigned long);
@@ -277,6 +338,7 @@ static int kvm_vfio_create(struct kvm_device *dev, u32 type)
 	return 0;
 }
 
+
 struct kvm_device_ops kvm_vfio_ops = {
 	.name = "kvm-vfio",
 	.create = kvm_vfio_create,
-- 
1.9.1

[RFC 8/9] KVM: KVM-VFIO: add kvm_vfio_arch_data and accessors

[Eric Auger]

add a pointer to architecture specific data in kvm_vfio struct
add accessors to keep kvm_vfio private

Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
 arch/arm/include/asm/kvm_host.h |  8 ++++++++
 virt/kvm/vfio.c                 | 21 +++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 62cbf5b..4f1edbf 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -177,6 +177,14 @@ void kvm_vfio_device_put_external_user(struct vfio_device *vdev);
 int kvm_vfio_external_get_type(struct vfio_device *vdev);
 struct device *kvm_vfio_external_get_base_device(struct vfio_device *vdev);
 
+struct kvm_vfio;
+struct kvm_vfio_arch_data;
+void kvm_vfio_device_set_arch_data(struct kvm_vfio *kv,
+				   struct kvm_vfio_arch_data *ptr);
+struct kvm_vfio_arch_data *kvm_vfio_device_get_arch_data(struct kvm_vfio *kv);
+void kvm_vfio_lock(struct kvm_vfio *kv);
+void kvm_vfio_unlock(struct kvm_vfio *kv);
+
 /* We do not have shadow page tables, hence the empty hooks */
 static inline int kvm_age_hva(struct kvm *kvm, unsigned long hva)
 {
diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
index f1c4e35..177b71e 100644
--- a/virt/kvm/vfio.c
+++ b/virt/kvm/vfio.c
@@ -28,6 +28,7 @@ struct kvm_vfio {
 	struct list_head group_list;
 	struct mutex lock;
 	bool noncoherent;
+	struct kvm_vfio_arch_data *arch_data;
 };
 
 static struct vfio_group *kvm_vfio_group_get_external_user(struct file *filep)
@@ -338,6 +339,26 @@ static int kvm_vfio_create(struct kvm_device *dev, u32 type)
 	return 0;
 }
 
+void kvm_vfio_device_set_arch_data(struct kvm_vfio *kv,
+				   struct kvm_vfio_arch_data *ptr)
+{
+	kv->arch_data = ptr;
+}
+
+struct kvm_vfio_arch_data *kvm_vfio_device_get_arch_data(struct kvm_vfio *kv)
+{
+	return kv->arch_data;
+}
+
+void kvm_vfio_lock(struct kvm_vfio *kv)
+{
+	mutex_lock(&kv->lock);
+}
+
+void kvm_vfio_unlock(struct kvm_vfio *kv)
+{
+	mutex_unlock(&kv->lock);
+}
 
 struct kvm_device_ops kvm_vfio_ops = {
 	.name = "kvm-vfio",
-- 
1.9.1

[RFC 9/9] KVM: KVM_VFIO: ARM: implement irq forwarding control

[Eric Auger]

Implements ARM specific KVM-VFIO device group commands:
- KVM_DEV_VFIO_DEVICE_ASSIGN_IRQ
- KVM_DEV_VFIO_DEVICE_DEASSIGN_IRQ
capability can be queried using KVM_HAS_DEVICE_ATTR.

The new commands enable to set IRQ forwarding on/off for a given
IRQ index of a VFIO platform device.

as soon as a forwarded irq is set, a reference to the VFIO device
is taken by the kvm-vfio device.

The kvm-vfio device stores in the kvm_vfio_arch_data the list
of "assigned" devices (kvm_vfio_device). Each kvm_vfio_device
stores the list of assigned IRQs (potentially allowed a subset of
IRQ to be forwarded)

The kvm-vfio device programs both the GIC and vGIC. Also it
clears the active bit on destruction, in case the guest did not
do it itself.

Changing the forwarded state is not allowed in the critical
section starting from VFIO IRQ handler to LR programming. It is
up to the client to take care of this.

Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
 arch/arm/include/asm/kvm_host.h |   2 +
 arch/arm/kvm/Makefile           |   2 +-
 arch/arm/kvm/kvm_vfio_arm.c     | 599 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 602 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/kvm/kvm_vfio_arm.c

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 4f1edbf..5c300f6 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -25,6 +25,8 @@
 #include <asm/fpstate.h>
 #include <kvm/arm_arch_timer.h>
 
+#define __KVM_HAVE_ARCH_KVM_VFIO
+
 #if defined(CONFIG_KVM_ARM_MAX_VCPUS)
 #define KVM_MAX_VCPUS CONFIG_KVM_ARM_MAX_VCPUS
 #else
diff --git a/arch/arm/kvm/Makefile b/arch/arm/kvm/Makefile
index ea1fa76..26a5a42 100644
--- a/arch/arm/kvm/Makefile
+++ b/arch/arm/kvm/Makefile
@@ -19,7 +19,7 @@ kvm-arm-y = $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o $(KVM)/eventfd.o $(KVM)/vf
 
 obj-y += kvm-arm.o init.o interrupts.o
 obj-y += arm.o handle_exit.o guest.o mmu.o emulate.o reset.o
-obj-y += coproc.o coproc_a15.o coproc_a7.o mmio.o psci.o perf.o
+obj-y += coproc.o coproc_a15.o coproc_a7.o mmio.o psci.o perf.o kvm_vfio_arm.o
 obj-$(CONFIG_KVM_ARM_VGIC) += $(KVM)/arm/vgic.o
 obj-$(CONFIG_KVM_ARM_VGIC) += $(KVM)/arm/vgic-v2.o
 obj-$(CONFIG_KVM_ARM_TIMER) += $(KVM)/arm/arch_timer.o
diff --git a/arch/arm/kvm/kvm_vfio_arm.c b/arch/arm/kvm/kvm_vfio_arm.c
new file mode 100644
index 0000000..6619e0b
--- /dev/null
+++ b/arch/arm/kvm/kvm_vfio_arm.c
@@ -0,0 +1,599 @@
+/*
+ * Copyright (C) 2014 Linaro Ltd.
+ * Authors: Eric Auger <eric.auger@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/errno.h>
+#include <linux/file.h>
+#include <linux/kvm_host.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/vfio.h>
+#include <linux/irq.h>
+#include <asm/kvm_host.h>
+#include <asm/kvm.h>
+#include <linux/irq.h>
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
+
+struct vfio_device;
+
+enum kvm_irq_fwd_action {
+	KVM_VFIO_IRQ_SET_FORWARD,
+	KVM_VFIO_IRQ_SET_NORMAL,
+	KVM_VFIO_IRQ_CLEANUP,
+};
+
+/* internal structure describing a forwarded IRQ */
+struct __kvm_arch_fwd_irq {
+	struct list_head link;
+	__u32 irq_index; /* platform device irq index */
+	__u32 hwirq; /*physical IRQ */
+	__u32 guest_irq; /* virtual IRQ */
+	struct kvm_vcpu *vcpu; /* vcpu to inject into*/
+};
+
+struct kvm_vfio_device {
+	struct list_head node;
+	struct vfio_device *vfio_device;
+	/* list of forwarded IRQs for that VFIO device */
+	struct list_head fwd_irq_list;
+	int fd;
+};
+
+struct kvm_vfio_arch_data {
+	/* list of kvm_vfio_devices for which some IRQs are forwarded*/
+	struct list_head assigned_device_list;
+};
+
+/**
+ * set_fwd_state - change the forwarded state of an IRQ
+ * @pfwd: the forwarded irq struct
+ * @action: action to perform (set forward, set back normal, cleanup)
+ *
+ * programs the GIC and VGIC
+ * returns the VGIC map/unmap return status
+ * It is the responsability of the caller to make sure the physical IRQ
+ * is not active. there is a critical section between the start of the
+ * VFIO IRQ handler and LR programming.
+ */
+int set_fwd_state(struct __kvm_arch_fwd_irq *pfwd,
+		  enum kvm_irq_fwd_action action)
+{
+	int ret;
+	struct irq_desc *desc = irq_to_desc(pfwd->hwirq);
+	struct irq_data *d = &desc->irq_data;
+	struct irq_chip *chip = desc->irq_data.chip;
+
+	disable_irq(pfwd->hwirq);
+	/* no fwd state change can happen if the IRQ is in progress */
+	if (irqd_irq_inprogress(d)) {
+		kvm_err("%s cannot change fwd state (IRQ %d in progress\n",
+			__func__, pfwd->hwirq);
+		enable_irq(pfwd->hwirq);
+		return -1;
+	}
+
+	if (action == KVM_VFIO_IRQ_SET_FORWARD) {
+		irqd_set_irq_forwarded(d);
+		ret = vgic_map_phys_irq(pfwd->vcpu,
+					pfwd->guest_irq + VGIC_NR_PRIVATE_IRQS,
+					pfwd->hwirq);
+	} else if (action == KVM_VFIO_IRQ_SET_NORMAL) {
+		irqd_clr_irq_forwarded(d);
+		ret = vgic_unmap_phys_irq(pfwd->vcpu,
+					  pfwd->guest_irq +
+						VGIC_NR_PRIVATE_IRQS,
+					  pfwd->hwirq);
+	} else if (action == KVM_VFIO_IRQ_CLEANUP) {
+		irqd_clr_irq_forwarded(d);
+		/*
+		 * in case the guest did not complete the
+		 * virtual IRQ, let's do it for him.
+		 * when cleanup is called, VCPU have already
+		 * been freed, do not manipulate VGIC
+		 */
+		chip->irq_eoi(d);
+		ret = 0;
+	} else {
+		enable_irq(pfwd->hwirq);
+		ret = -EINVAL;
+	}
+
+	enable_irq(pfwd->hwirq);
+	return ret;
+}
+
+/**
+ * find_in_assigned_devices - look for the device in the assigned
+ * device list
+ * @kv: the kvm-vfio device
+ * @vdev: the vfio_device to look for
+ *
+ * returns the associated kvm_vfio_device if the device is known,
+ * meaning at least 1 IRQ is forwarded for this device.
+ * in the device is not registered, returns NULL.
+ */
+struct kvm_vfio_device *find_in_assigned_devices(struct kvm_vfio *kv,
+						 struct vfio_device *vdev)
+{
+	struct kvm_vfio_device *kvm_vdev_iter;
+	struct kvm_vfio_arch_data *arch_data =
+			kvm_vfio_device_get_arch_data(kv);
+
+	list_for_each_entry(kvm_vdev_iter,
+			    &arch_data->assigned_device_list, node) {
+		if (kvm_vdev_iter->vfio_device == vdev)
+			return kvm_vdev_iter;
+	}
+	return NULL;
+}
+
+/**
+ * find_in_fwd_irq - look for a forwarded irq in the device IRQ list
+ * @kvm_vdev: the kvm_vfio_device
+ * @irq_index: irq index
+ *
+ * returns the forwarded irq struct if it exists, NULL in the negative
+ */
+struct __kvm_arch_fwd_irq *find_in_fwd_irq(struct kvm_vfio_device *kvm_vdev,
+					   int irq_index)
+{
+	struct __kvm_arch_fwd_irq *fwd_irq_iter;
+
+	list_for_each_entry(fwd_irq_iter, &kvm_vdev->fwd_irq_list, link) {
+		if (fwd_irq_iter->irq_index == irq_index)
+			return fwd_irq_iter;
+	}
+	return NULL;
+}
+
+/**
+ * remove_assigned_device - put a given device from the list
+ * @kv: the kvm-vfio device
+ * @vdev: the vfio-device to remove
+ *
+ * change the state of all forwarded IRQs, free the forwarded IRQ list,
+ * remove the corresponding kvm_vfio_device from the assigned device
+ * list.
+ * returns true if the device could be removed, false in the negative
+ */
+bool remove_assigned_device(struct kvm_vfio *kv,
+			    struct vfio_device *vdev)
+{
+	struct kvm_vfio_device *kvm_vdev_iter, *tmp_vdev;
+	struct __kvm_arch_fwd_irq *fwd_irq_iter, *tmp_irq;
+	bool removed = false;
+	struct kvm_vfio_arch_data *arch_data =
+			kvm_vfio_device_get_arch_data(kv);
+	int ret;
+
+	list_for_each_entry_safe(kvm_vdev_iter, tmp_vdev,
+				 &arch_data->assigned_device_list, node) {
+		if (kvm_vdev_iter->vfio_device == vdev) {
+			/* loop on all its forwarded IRQ */
+			list_for_each_entry_safe(fwd_irq_iter, tmp_irq,
+						 &kvm_vdev_iter->fwd_irq_list,
+						 link) {
+				ret = set_fwd_state(fwd_irq_iter,
+						    KVM_VFIO_IRQ_SET_NORMAL);
+				if (ret < 0)
+					return ret;
+				list_del(&fwd_irq_iter->link);
+				kfree(fwd_irq_iter);
+			}
+			/* all IRQs could be deassigned */
+			list_del(&kvm_vdev_iter->node);
+			kvm_vfio_device_put_external_user(
+				kvm_vdev_iter->vfio_device);
+			kfree(kvm_vdev_iter);
+			removed = true;
+			break;
+		}
+	}
+	return removed;
+}
+
+/**
+ * remove_fwd_irq - remove a forwarded irq
+ *
+ * @kv: kvm-vfio device
+ * kvm_vdev: the kvm_vfio_device the IRQ belongs to
+ * irq_index: the index of the IRQ
+ *
+ * change the forwarded state of the IRQ, remove the IRQ from
+ * the device forwarded IRQ list. In case it is the last one,
+ * put the device
+ */
+int remove_fwd_irq(struct kvm_vfio *kv,
+		   struct kvm_vfio_device *kvm_vdev,
+		   int irq_index)
+{
+	struct __kvm_arch_fwd_irq *fwd_irq_iter, *tmp_irq;
+	int ret = -1;
+
+	list_for_each_entry_safe(fwd_irq_iter, tmp_irq,
+				 &kvm_vdev->fwd_irq_list, link) {
+			if (fwd_irq_iter->irq_index == irq_index) {
+				ret = set_fwd_state(fwd_irq_iter,
+						    KVM_VFIO_IRQ_SET_NORMAL);
+				if (ret < 0)
+					break;
+				list_del(&fwd_irq_iter->link);
+				kfree(fwd_irq_iter);
+				ret = 0;
+				break;
+			}
+	}
+	if (list_empty(&kvm_vdev->fwd_irq_list))
+		remove_assigned_device(kv, kvm_vdev->vfio_device);
+
+	return ret;
+}
+
+/**
+ * free_all_fwd_irq - cancel forwarded IRQs and put all devices
+ * @kv: kvm-vfio device
+ *
+ * loop on all got devices and their associated forwarded IRQs
+ * restore the non forwarded state, remove IRQs and their devices from
+ * the respective list, put the vfio platform devices
+ *
+ * When this function is called, the vcpu already are destroyed. No
+ * vgic manipulation can happen hence the KVM_VFIO_IRQ_CLEANUP
+ * set_fwd_state action
+ */
+int free_all_fwd_irq(struct kvm_vfio *kv)
+{
+	struct __kvm_arch_fwd_irq *fwd_irq_iter, *tmp_irq;
+	struct kvm_vfio_device *kvm_vdev_iter, *tmp_vdev;
+	struct kvm_vfio_arch_data *arch_data =
+			kvm_vfio_device_get_arch_data(kv);
+
+	/* loop on all the assigned devices */
+	list_for_each_entry_safe(kvm_vdev_iter, tmp_vdev,
+				 &arch_data->assigned_device_list, node) {
+
+		/* loop on all its forwarded IRQ */
+		list_for_each_entry_safe(fwd_irq_iter, tmp_irq,
+					 &kvm_vdev_iter->fwd_irq_list, link) {
+			set_fwd_state(fwd_irq_iter, KVM_VFIO_IRQ_CLEANUP);
+			list_del(&fwd_irq_iter->link);
+			kfree(fwd_irq_iter);
+		}
+		list_del(&kvm_vdev_iter->node);
+		kvm_vfio_device_put_external_user(kvm_vdev_iter->vfio_device);
+		kfree(kvm_vdev_iter);
+	}
+	return 0;
+}
+
+/**
+ * get_vfio_device - returns the vfio-device corresponding to this fd
+ * @fd:fd of the vfio platform device
+ *
+ * checks it is a vfio device
+ * increment its ref counter
+ */
+static struct vfio_device *get_vfio_device(int fd)
+{
+	struct fd f;
+	struct vfio_device *vdev;
+
+	f = fdget(fd);
+	if (!f.file)
+		return NULL;
+	vdev = kvm_vfio_device_get_external_user(f.file);
+	fdput(f);
+	return vdev;
+}
+
+/**
+ * put_vfio_device: put the vfio platform device
+ * @vdev: vfio_device to put
+ *
+ * decrement the ref counter
+ */
+static void put_vfio_device(struct vfio_device *vdev)
+{
+	kvm_vfio_device_put_external_user(vdev);
+}
+
+/**
+ * validate_forward-checks whether forwarding a given IRQ is meaningful
+ * @vdev:  vfio_device the IRQ belongs to
+ * @fwd_irq: user struct containing the irq_index to forward
+ * @kvm_vdev: if a forwarded IRQ already exists for that VFIO device,
+ * kvm_vfio_device that holds it
+ * @hwirq: irq numberthe irq index corresponds to
+ *
+ * checks the vfio-device is a platform vfio device
+ * checks the irq_index corresponds to an actual hwirq and
+ * checks this hwirq is not already forwarded
+ * returns < 0 on following errors:
+ * not a platform device, bad irq index, already forwarded
+ */
+static int validate_forward(struct kvm_vfio *kv,
+			    struct vfio_device *vdev,
+			    struct kvm_arch_forwarded_irq *fwd_irq,
+			    struct kvm_vfio_device **kvm_vdev,
+			    int *hwirq)
+{
+	int type;
+	struct device *dev;
+	struct platform_device *platdev;
+
+	*hwirq = -1;
+	*kvm_vdev = NULL;
+	type = kvm_vfio_external_get_type(vdev);
+	if (type & VFIO_DEVICE_FLAGS_PLATFORM) {
+		dev = kvm_vfio_external_get_base_device(vdev);
+		platdev = to_platform_device(dev);
+		*hwirq = platform_get_irq(platdev, fwd_irq->irq_index);
+		if (*hwirq < 0) {
+			kvm_err("%s incorrect index\n",	__func__);
+			return -EINVAL;
+		}
+	} else {
+		kvm_err("%s not a platform device\n", __func__);
+		return -EINVAL;
+	}
+	/* is a ref to this device already owned by the KVM-VFIO device? */
+	*kvm_vdev = find_in_assigned_devices(kv, vdev);
+	if (*kvm_vdev) {
+		if (find_in_fwd_irq(*kvm_vdev, fwd_irq->irq_index)) {
+			kvm_err("%s irq %d already forwarded\n",
+				__func__, *hwirq);
+			return -EINVAL;
+		}
+	}
+	return 0;
+}
+
+/**
+ * validate_deassign: check a deassignment is meaningful
+ * @kv: the kvm_vfio device
+ * @vdev: the vfio_device whose irq to deassign belongs to
+ * @fwd_irq: the user struct that contains the fd and irq_index of the irq
+ * @kvm_vdev: the kvm_vfio_device the forwarded irq belongs to, if
+ * it exists
+ *
+ * returns 0 if the provided irq effectively is forwarded
+ * (a ref to this vfio_device is hold and this irq belongs to
+ * the forwarded irq of this device)
+ * returns -EINVAL in the negative
+ */
+static int validate_deassign(struct kvm_vfio *kv,
+			     struct vfio_device *vdev,
+			     struct kvm_arch_forwarded_irq *fwd_irq,
+			     struct kvm_vfio_device **kvm_vdev)
+{
+	struct __kvm_arch_fwd_irq *pfwd;
+
+	*kvm_vdev = find_in_assigned_devices(kv, vdev);
+	if (!kvm_vdev) {
+		kvm_err("%s no forwarded irq for this device\n", __func__);
+		return -EINVAL;
+	}
+	pfwd = find_in_fwd_irq(*kvm_vdev, fwd_irq->irq_index);
+	if (!pfwd) {
+		kvm_err("%s irq %d is not forwarded\n", __func__, fwd_irq->fd);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+/**
+ * insert_and_forward - set a forwarded IRQ
+ * @kdev: the kvm device
+ * @vdev: the vfio device the IRQ belongs to
+ * @fwd_irq: the user struct containing the irq_index and guest irq
+ * @must_put: tells the caller whether the vfio_device must be put after
+ * the call (ref must be released in case a ref onto this device was
+ * wad already hold or in case of new device and failure)
+ *
+ * validate the injection, activate forward and store the information
+ * about which irq and which device is concerned so that on deassign or
+ * kvm-vfio destruction everuthing can be cleaned up.
+ */
+static int insert_and_forward(struct kvm_device *kdev,
+			      struct vfio_device *vdev,
+			      struct kvm_arch_forwarded_irq *fwd_irq,
+			      bool *must_put)
+{
+	int ret;
+	struct __kvm_arch_fwd_irq *pfwd = NULL;
+	struct kvm_vfio_device *kvm_vdev = NULL;
+	struct kvm_vfio *kv = kdev->private;
+	struct kvm_vfio_arch_data *arch_data =
+			kvm_vfio_device_get_arch_data(kv);
+	int hwirq;
+
+	*must_put = true;
+	ret = validate_forward(kv, vdev, fwd_irq, &kvm_vdev, &hwirq);
+	if (ret < 0)
+		return -EINVAL;
+
+	pfwd = kzalloc(sizeof(*pfwd), GFP_KERNEL);
+	if (!pfwd)
+		return -ENOMEM;
+
+	pfwd->irq_index = fwd_irq->irq_index;
+	pfwd->guest_irq = fwd_irq->guest_irq;
+	pfwd->hwirq = hwirq;
+	pfwd->vcpu = kvm_get_vcpu(kdev->kvm, 0);
+	ret = set_fwd_state(pfwd, KVM_VFIO_IRQ_SET_FORWARD);
+	if (ret < 0) {
+		set_fwd_state(pfwd, KVM_VFIO_IRQ_CLEANUP);
+		kfree(pfwd);
+		return ret;
+	}
+
+	if (!kvm_vdev) {
+		/* create & insert the new device and keep the ref */
+		kvm_vdev = kzalloc(sizeof(*kvm_vdev), GFP_KERNEL);
+		if (!kvm_vdev) {
+			set_fwd_state(pfwd, false);
+			kfree(pfwd);
+			return -ENOMEM;
+		}
+
+		kvm_vdev->vfio_device = vdev;
+		kvm_vdev->fd = fwd_irq->fd;
+		INIT_LIST_HEAD(&kvm_vdev->fwd_irq_list);
+		list_add(&kvm_vdev->node, &arch_data->assigned_device_list);
+		/*
+		 * the only case where we keep the ref:
+		 * new device and forward setting successful
+		 */
+		*must_put = false;
+	}
+
+	list_add(&pfwd->link, &kvm_vdev->fwd_irq_list);
+
+	kvm_debug("forwarding set for fd=%d, hwirq=%d, guest_irq=%d\n",
+		  fwd_irq->fd, hwirq, fwd_irq->guest_irq);
+
+	return 0;
+}
+
+/**
+ * cancel_and_delete_forward - remove a forwarded IRQ
+ * @kdev: the kvm device
+ * @vdev: the vfio_device
+ * @fwd_irq: user struct
+ * after checking this IRQ effectively is forwarded, change its state,
+ * remove it from the corresponding kvm_vfio_device list
+ */
+static int cancel_and_delete_forward(struct kvm_device *kdev,
+				     struct vfio_device *vdev,
+				     struct kvm_arch_forwarded_irq *fwd_irq)
+{
+	struct kvm_vfio *kv = kdev->private;
+	struct kvm_vfio_device *kvm_vdev;
+	int ret;
+
+	ret = validate_deassign(kv, vdev, fwd_irq, &kvm_vdev);
+	if (ret < 0)
+		return -EINVAL;
+
+	ret = remove_fwd_irq(kv, kvm_vdev, fwd_irq->irq_index);
+	if (ret < 0)
+		kvm_err("%s fail cancelling forward (fd=%d, index=%d)\n",
+				__func__, fwd_irq->fd, fwd_irq->irq_index);
+	else
+		kvm_debug("%s forward cancelled for IRQ (fd=%d, index=%d)\n",
+				__func__, fwd_irq->fd, fwd_irq->irq_index);
+	return ret;
+}
+
+/**
+ * kvm_vfio_set_device - the top function for interracting with a vfio
+ * device
+ */
+
+static int kvm_vfio_set_device(struct kvm_device *kdev, long attr, u64 arg)
+{
+	struct kvm_vfio *kv = kdev->private;
+	struct vfio_device *vdev;
+	struct kvm_arch_forwarded_irq fwd_irq; /* user struct */
+	int32_t __user *argp = (int32_t __user *)(unsigned long)arg;
+
+	switch (attr) {
+	case KVM_DEV_VFIO_DEVICE_ASSIGN_IRQ:{
+		bool must_put;
+		int ret;
+
+		if (copy_from_user(&fwd_irq, argp, sizeof(fwd_irq)))
+			return -EFAULT;
+		vdev = get_vfio_device(fwd_irq.fd);
+		if (IS_ERR(vdev))
+			return PTR_ERR(vdev);
+		kvm_vfio_lock(kv);
+		ret = insert_and_forward(kdev, vdev, &fwd_irq, &must_put);
+		if (must_put)
+			put_vfio_device(vdev);
+		kvm_vfio_unlock(kv);
+		return ret;
+		}
+	case KVM_DEV_VFIO_DEVICE_DEASSIGN_IRQ: {
+		int ret;
+
+		if (copy_from_user(&fwd_irq, argp, sizeof(fwd_irq)))
+			return -EFAULT;
+		vdev = get_vfio_device(fwd_irq.fd);
+		if (IS_ERR(vdev))
+			return PTR_ERR(vdev);
+
+		kvm_vfio_device_put_external_user(vdev);
+		kvm_vfio_lock(kv);
+		ret = cancel_and_delete_forward(kdev, vdev, &fwd_irq);
+		kvm_vfio_unlock(kv);
+		return ret;
+	}
+	default:
+		return -ENXIO;
+	}
+}
+
+void kvm_arch_vfio_destroy(struct kvm_device *dev)
+{
+	struct kvm_vfio *kv = dev->private;
+	struct kvm_vfio_arch_data *arch_data =
+			kvm_vfio_device_get_arch_data(kv);
+
+	free_all_fwd_irq(kv);
+	kfree(arch_data);
+}
+
+int kvm_arch_vfio_init(struct kvm_device *dev)
+{
+	struct kvm_vfio *kv = dev->private;
+	struct kvm_vfio_arch_data *ptr;
+
+	ptr = kzalloc(sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return -ENOMEM;
+	INIT_LIST_HEAD(&ptr->assigned_device_list);
+	kvm_vfio_device_set_arch_data(kv, ptr);
+	return 0;
+}
+
+
+int kvm_arch_vfio_has_attr(struct kvm_device *dev,
+			   struct kvm_device_attr *attr)
+{
+	switch (attr->group) {
+	case KVM_DEV_VFIO_DEVICE:
+		switch (attr->attr) {
+		case KVM_DEV_VFIO_DEVICE_ASSIGN_IRQ:
+		case KVM_DEV_VFIO_DEVICE_DEASSIGN_IRQ:
+			return 0;
+		}
+		break;
+	}
+	return -ENXIO;
+}
+
+int kvm_arch_vfio_set_attr(struct kvm_device *dev,
+			   struct kvm_device_attr *attr)
+{
+	switch (attr->group) {
+	case KVM_DEV_VFIO_DEVICE:
+		return kvm_vfio_set_device(dev, attr->attr, attr->addr);
+	default:
+		return -ENXIO;
+	}
+}
+
+
-- 
1.9.1

Re: [RFC 0/9] KVM-VFIO IRQ forward control

[Alex Williamson]

On Mon, 2014-08-25 at 15:27 +0200, Eric Auger wrote:
> This RFC proposes an integration of "ARM: Forwarding physical
> interrupts to a guest VM" (http://lwn.net/Articles/603514/) in
> KVM.
> 
> It enables to transform a VFIO platform driver IRQ into a forwarded
> IRQ. The direct benefit is that, for a level sensitive IRQ, a VM
> switch can be avoided on guest virtual IRQ completion. Before this
> patch, a maintenance IRQ was triggered on the virtual IRQ completion.
> 
> When the IRQ is forwarded, the VFIO platform driver does not need to
> disable the IRQ anymore. Indeed when returning from the IRQ handler
> the IRQ is not deactivated. Only its priority is lowered. This means
> the same IRQ cannot hit before the guest completes the virtual IRQ
> and the GIC automatically deactivates the corresponding physical IRQ.
> 
> 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 this latter becomes "transparent".
> 
> The current integration is based on an extension of the KVM-VFIO
> device, previously used by KVM to interact with VFIO groups. The
> patch serie now enables KVM to directly interact with a VFIO
> platform device. The VFIO external API was extended for that purpose.
> 
> Th KVM-VFIO device can get/put the vfio platform device, check its
> integrity and type, get the IRQ number associated to an IRQ index.
> 
> The KVM-VFIO is extended with an architecture specific implementation.
> IRQ forward control is implemented in the ARM specific part.
> 
> from a user point of view, the functionality is provided through new
> KVM-VFIO device commands, KVM_DEV_VFIO_DEVICE_(DE)ASSIGN_IRQ
> and the capability can be checked with KVM_HAS_DEVICE_ATTR.
> Assignment can only be changed when the physical IRQ is not active.
> It is the responsability of the user to do this check.
> 
> This patch serie has the following dependencies:
> - "ARM: Forwarding physical interrupts to a guest VM"
>   (http://lwn.net/Articles/603514/) in
> - [PATCH v2] irqfd for ARM
>   which itself depends on
>   - arm/arm64: KVM: Various VGIC cleanups and improvements
>     http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/263685.html
> - and obviously the VFIO platform driver serie:
>   [RFC PATCH v6 00/20] VFIO support for platform devices on ARM
>   https://www.mail-archive.com/kvm@vger.kernel.org/msg103247.html
> 
> Integrated pieces can be found at
> git://git.linaro.org/people/eric.auger/linux.git
> on branch 3.17rc1_forward_integ_v0
> 
> This was was tested on Calxeda Miday, assigning the xgmac main IRQ.

Presumably this optimization should provide lower interrupt exit latency
and lower CPU overhead since we avoid the entire EOI path of the
resampler.  Does it?  It seems like there should be a measurable
improvement with something like netperf TCP_RR with this series.
Thanks,

Alex

> Eric Auger (9):
>   KVM: ARM: VGIC: fix multiple injection of level sensitive forwarded
>     IRQ
>   KVM: ARM: VGIC: add forwarded irq rbtree lock
>   VFIO: platform: handler tests whether the IRQ is forwarded
>   KVM: KVM-VFIO: update user API to program forwarded IRQ
>   VFIO: Extend external user API
>   KVM: KVM-VFIO: allow arch specific implementation
>   KVM: KVM-VFIO: add new VFIO external API hooks
>   KVM: KVM-VFIO: add kvm_vfio_arch_data and accessors
>   KVM: KVM_VFIO: ARM: implement irq forwarding control
> 
>  Documentation/virtual/kvm/devices/vfio.txt |  25 ++
>  arch/arm/include/asm/kvm_host.h            |  16 +
>  arch/arm/include/uapi/asm/kvm.h            |   6 +
>  arch/arm/kvm/Makefile                      |   2 +-
>  arch/arm/kvm/kvm_vfio_arm.c                | 599 +++++++++++++++++++++++++++++
>  drivers/vfio/platform/vfio_platform_irq.c  |   7 +-
>  drivers/vfio/vfio.c                        |  35 ++
>  include/kvm/arm_vgic.h                     |   1 +
>  include/linux/kvm_host.h                   |  30 ++
>  include/linux/vfio.h                       |   4 +
>  include/uapi/linux/kvm.h                   |   3 +
>  virt/kvm/arm/vgic.c                        |  55 ++-
>  virt/kvm/vfio.c                            |  92 +++++
>  13 files changed, 862 insertions(+), 13 deletions(-)
>  create mode 100644 arch/arm/kvm/kvm_vfio_arm.c
> 

Re: [RFC 4/9] KVM: KVM-VFIO: update user API to program forwarded IRQ

[Alex Williamson]

On Mon, 2014-08-25 at 15:27 +0200, Eric Auger wrote:
> add new device group commands:
> - KVM_DEV_VFIO_DEVICE_ASSIGN_IRQ and
>   KVM_DEV_VFIO_DEVICE_DEASSIGN_IRQ
> 
> which enable to turn forwarded IRQ mode on/off.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> ---
>  Documentation/virtual/kvm/devices/vfio.txt | 25 +++++++++++++++++++++++++
>  arch/arm/include/uapi/asm/kvm.h            |  6 ++++++
>  include/uapi/linux/kvm.h                   |  3 +++
>  3 files changed, 34 insertions(+)
> 
> diff --git a/Documentation/virtual/kvm/devices/vfio.txt b/Documentation/virtual/kvm/devices/vfio.txt
> index ef51740..c8b3fa1 100644
> --- a/Documentation/virtual/kvm/devices/vfio.txt
> +++ b/Documentation/virtual/kvm/devices/vfio.txt
> @@ -13,6 +13,7 @@ VFIO-group is held by KVM.
>  
>  Groups:
>    KVM_DEV_VFIO_GROUP
> +  KVM_DEV_VFIO_DEVICE
>  
>  KVM_DEV_VFIO_GROUP attributes:
>    KVM_DEV_VFIO_GROUP_ADD: Add a VFIO group to VFIO-KVM device tracking
> @@ -20,3 +21,27 @@ KVM_DEV_VFIO_GROUP attributes:
>  
>  For each, kvm_device_attr.addr points to an int32_t file descriptor
>  for the VFIO group.
> +
> +KVM_DEV_VFIO_DEVICE attributes:
> +  KVM_DEV_VFIO_DEVICE_ASSIGN_IRQ
> +  KVM_DEV_VFIO_DEVICE_DEASSIGN_IRQ
> +
> +For each, kvm_device_attr.addr points to an kvm_arch_forwarded_irq.
> +This user API makes possible to create a special IRQ handling mode,
> +currently supported only on ARM, where KVM and a VFIO platform driver
> +collaborate to improve IRQ handling performance.
> +fd represents the file descriptor of a valid VFIO device whose physical
> +IRQ, referenced by its irq_index is injected to the VM guest_irq.
> +
> +On ASSIGN_IRQ, KVM-VFIO device programs:
> +- the host, to not complete the physical IRQ itself.
> +- the GIC, to automatically complete the physical IRQ when the guest
> +  completes the virtual IRQ
> +This avoid trapping the end-of-interrupt for level sensitive IRQ.
> +
> +On DEASSIGN_IRQ, one come back to the mode where the host completes the
> +physical IRQ and the guest only completes the virtual IRQ.
> +
> +It is up to the caller of this API to get the assurance the IRQ is not
> +outstanding when the ASSIGN/DEASSIGN is called. This could lead to some
> +inconsistency on who is going to complete the IRQ.

Why not call these FORWARD/UNFORWARD or something since the operation
isn't really doing anything with assignment of the IRQ.  The IRQ is
already "assigned", we're modifying the behavior.

> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
> index 3034c66..1920b33 100644
> --- a/arch/arm/include/uapi/asm/kvm.h
> +++ b/arch/arm/include/uapi/asm/kvm.h
> @@ -109,6 +109,12 @@ struct kvm_sync_regs {
>  struct kvm_arch_memory_slot {
>  };
>  
> +struct kvm_arch_forwarded_irq {
> +	__u32 fd; /* file desciptor of the VFIO device */
> +	__u32 irq_index; /* platform device index of the IRQ */

The vfio-platform device IRQ index?  ARM is the only implementation we
have of this, but to keep it generic maybe the comment should read "vfio
device IRQ index".

> +	__u32 guest_irq; /* virtual IRQ number */

This would be a GSI or similar concept if we were on x86.  I'm a little
confused that were using an arch structure here rather than trying to
keep the kvm-vfio device interface neutral.  Maybe it makes sense, I'm
not sure.  Thanks,

Alex

> +};
> +
>  /* If you need to interpret the index values, here is the key: */
>  #define KVM_REG_ARM_COPROC_MASK		0x000000000FFF0000
>  #define KVM_REG_ARM_COPROC_SHIFT	16
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index cf3a2ff..b149ba8 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -954,6 +954,9 @@ struct kvm_device_attr {
>  #define  KVM_DEV_VFIO_GROUP			1
>  #define   KVM_DEV_VFIO_GROUP_ADD			1
>  #define   KVM_DEV_VFIO_GROUP_DEL			2
> +#define  KVM_DEV_VFIO_DEVICE			2
> +#define   KVM_DEV_VFIO_DEVICE_ASSIGN_IRQ			1
> +#define   KVM_DEV_VFIO_DEVICE_DEASSIGN_IRQ			2
>  #define KVM_DEV_TYPE_ARM_VGIC_V2	5
>  #define KVM_DEV_TYPE_FLIC		6
>  

Re: [RFC 5/9] VFIO: Extend external user API

[Alex Williamson]

On Mon, 2014-08-25 at 15:27 +0200, Eric Auger wrote:
> New functions are added to be called from ARM KVM-VFIO device.
> 
> - vfio_device_get_external_user enables to get a vfio device from
>   its fd
> - vfio_device_put_external_user puts the vfio device
> - vfio_external_get_type enables to retrieve the type of the device
>   (PCI or platform)
> - vfio_external_get_base_device enables to get the
>   struct device*, useful to access the platform_device
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> ---
>  drivers/vfio/vfio.c  | 35 +++++++++++++++++++++++++++++++++++
>  include/linux/vfio.h |  4 ++++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 8e84471..c93b9e4 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -1401,6 +1401,41 @@ void vfio_group_put_external_user(struct vfio_group *group)
>  }
>  EXPORT_SYMBOL_GPL(vfio_group_put_external_user);
>  
> +struct vfio_device *vfio_device_get_external_user(struct file *filep)
> +{
> +	struct vfio_device *vdev = filep->private_data;
> +
> +	if (filep->f_op != &vfio_device_fops)
> +		return ERR_PTR(-EINVAL);
> +
> +	vfio_device_get(vdev);
> +	return vdev;
> +}
> +EXPORT_SYMBOL_GPL(vfio_device_get_external_user);
> +
> +void vfio_device_put_external_user(struct vfio_device *vdev)
> +{
> +	vfio_device_put(vdev);
> +}
> +EXPORT_SYMBOL_GPL(vfio_device_put_external_user);
> +
> +int vfio_external_get_type(struct vfio_device *vdev)
> +{
> +	if (!strcmp(vdev->ops->name,  "vfio-platform"))
> +		return VFIO_DEVICE_FLAGS_PLATFORM;
> +	else if (!strcmp(vdev->ops->name,  "vfio-pci"))
> +		return VFIO_DEVICE_FLAGS_PCI;
> +	else
> +		return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(vfio_external_get_type);

Returning the bit of the flag we use in get_device_info looks rather
sloppy here.  Should we define a new enum for use with this?  Actually,
is this interface even necessary?  If we can get the struct device then
we can get the bus_type and keep vfio out of this.

For both of these last two, I like to use the convention that where
there is a "get" there is a matching "put".  These aren't reference
counting anything, so let's not use get in the name.

> +
> +struct device *vfio_external_get_base_device(struct vfio_device *vdev)
> +{
> +	return vdev->dev;
> +}
> +EXPORT_SYMBOL_GPL(vfio_external_get_base_device);
> +

Looks almost too simple, but reviewing the object lifecycles, this all
looks safe.  Thanks,

Alex

>  int vfio_external_user_iommu_id(struct vfio_group *group)
>  {
>  	return iommu_group_id(group->iommu_group);
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index ffe04ed..19e98eb 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -99,6 +99,10 @@ extern void vfio_group_put_external_user(struct vfio_group *group);
>  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 struct vfio_device *vfio_device_get_external_user(struct file *filep);
> +extern void vfio_device_put_external_user(struct vfio_device *vdev);
> +extern int vfio_external_get_type(struct vfio_device *vdev);
> +extern struct device *vfio_external_get_base_device(struct vfio_device *vdev);
>  
>  struct pci_dev;
>  #ifdef CONFIG_EEH

Re: [RFC 8/9] KVM: KVM-VFIO: add kvm_vfio_arch_data and accessors

[Alex Williamson]

On Mon, 2014-08-25 at 15:27 +0200, Eric Auger wrote:
> add a pointer to architecture specific data in kvm_vfio struct
> add accessors to keep kvm_vfio private
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> ---
>  arch/arm/include/asm/kvm_host.h |  8 ++++++++
>  virt/kvm/vfio.c                 | 21 +++++++++++++++++++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 62cbf5b..4f1edbf 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -177,6 +177,14 @@ void kvm_vfio_device_put_external_user(struct vfio_device *vdev);
>  int kvm_vfio_external_get_type(struct vfio_device *vdev);
>  struct device *kvm_vfio_external_get_base_device(struct vfio_device *vdev);
>  
> +struct kvm_vfio;
> +struct kvm_vfio_arch_data;
> +void kvm_vfio_device_set_arch_data(struct kvm_vfio *kv,
> +				   struct kvm_vfio_arch_data *ptr);
> +struct kvm_vfio_arch_data *kvm_vfio_device_get_arch_data(struct kvm_vfio *kv);
> +void kvm_vfio_lock(struct kvm_vfio *kv);
> +void kvm_vfio_unlock(struct kvm_vfio *kv);
> +
>  /* We do not have shadow page tables, hence the empty hooks */
>  static inline int kvm_age_hva(struct kvm *kvm, unsigned long hva)
>  {
> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
> index f1c4e35..177b71e 100644
> --- a/virt/kvm/vfio.c
> +++ b/virt/kvm/vfio.c
> @@ -28,6 +28,7 @@ struct kvm_vfio {
>  	struct list_head group_list;
>  	struct mutex lock;
>  	bool noncoherent;
> +	struct kvm_vfio_arch_data *arch_data;
>  };
>  
>  static struct vfio_group *kvm_vfio_group_get_external_user(struct file *filep)
> @@ -338,6 +339,26 @@ static int kvm_vfio_create(struct kvm_device *dev, u32 type)
>  	return 0;
>  }
>  
> +void kvm_vfio_device_set_arch_data(struct kvm_vfio *kv,
> +				   struct kvm_vfio_arch_data *ptr)
> +{
> +	kv->arch_data = ptr;
> +}
> +
> +struct kvm_vfio_arch_data *kvm_vfio_device_get_arch_data(struct kvm_vfio *kv)
> +{

My preference would be s/get_//

> +	return kv->arch_data;
> +}
> +
> +void kvm_vfio_lock(struct kvm_vfio *kv)
> +{
> +	mutex_lock(&kv->lock);
> +}
> +
> +void kvm_vfio_unlock(struct kvm_vfio *kv)
> +{
> +	mutex_unlock(&kv->lock);
> +}

Gosh, what could go wrong...

>  
>  struct kvm_device_ops kvm_vfio_ops = {
>  	.name = "kvm-vfio",

Re: [RFC 9/9] KVM: KVM_VFIO: ARM: implement irq forwarding control

[Alex Williamson]

On Mon, 2014-08-25 at 15:27 +0200, Eric Auger wrote:
> Implements ARM specific KVM-VFIO device group commands:
> - KVM_DEV_VFIO_DEVICE_ASSIGN_IRQ
> - KVM_DEV_VFIO_DEVICE_DEASSIGN_IRQ
> capability can be queried using KVM_HAS_DEVICE_ATTR.
> 
> The new commands enable to set IRQ forwarding on/off for a given
> IRQ index of a VFIO platform device.
> 
> as soon as a forwarded irq is set, a reference to the VFIO device
> is taken by the kvm-vfio device.
> 
> The kvm-vfio device stores in the kvm_vfio_arch_data the list
> of "assigned" devices (kvm_vfio_device). Each kvm_vfio_device
> stores the list of assigned IRQs (potentially allowed a subset of
> IRQ to be forwarded)
> 
> The kvm-vfio device programs both the GIC and vGIC. Also it
> clears the active bit on destruction, in case the guest did not
> do it itself.
> 
> Changing the forwarded state is not allowed in the critical
> section starting from VFIO IRQ handler to LR programming. It is
> up to the client to take care of this.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> ---
>  arch/arm/include/asm/kvm_host.h |   2 +
>  arch/arm/kvm/Makefile           |   2 +-
>  arch/arm/kvm/kvm_vfio_arm.c     | 599 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 602 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/kvm/kvm_vfio_arm.c

I'm really happy that it seems like the kvm-vfio device is going to work
for you, but I think too much stuff is being pushed out to arch code
here.  Exporting the interfaces in patches 7 & 8 are setting the stage
for duplicate code for anyone wanting to implement device attributes.
Instead, I think the core code should support the list of
kvm_vfio_devices with proper cleanup, and we should attempt to access
the kvm_vfio_ callbacks as little as possible from arch code.  Thanks,

Alex

Re: [RFC 0/9] KVM-VFIO IRQ forward control

[Eric Auger]

On 08/26/2014 07:49 PM, Alex Williamson wrote:
> On Mon, 2014-08-25 at 15:27 +0200, Eric Auger wrote:
>> This RFC proposes an integration of "ARM: Forwarding physical
>> interrupts to a guest VM" (http://lwn.net/Articles/603514/) in
>> KVM.
>>
>> It enables to transform a VFIO platform driver IRQ into a forwarded
>> IRQ. The direct benefit is that, for a level sensitive IRQ, a VM
>> switch can be avoided on guest virtual IRQ completion. Before this
>> patch, a maintenance IRQ was triggered on the virtual IRQ completion.
>>
>> When the IRQ is forwarded, the VFIO platform driver does not need to
>> disable the IRQ anymore. Indeed when returning from the IRQ handler
>> the IRQ is not deactivated. Only its priority is lowered. This means
>> the same IRQ cannot hit before the guest completes the virtual IRQ
>> and the GIC automatically deactivates the corresponding physical IRQ.
>>
>> 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 this latter becomes "transparent".
>>
>> The current integration is based on an extension of the KVM-VFIO
>> device, previously used by KVM to interact with VFIO groups. The
>> patch serie now enables KVM to directly interact with a VFIO
>> platform device. The VFIO external API was extended for that purpose.
>>
>> Th KVM-VFIO device can get/put the vfio platform device, check its
>> integrity and type, get the IRQ number associated to an IRQ index.
>>
>> The KVM-VFIO is extended with an architecture specific implementation.
>> IRQ forward control is implemented in the ARM specific part.
>>
>> from a user point of view, the functionality is provided through new
>> KVM-VFIO device commands, KVM_DEV_VFIO_DEVICE_(DE)ASSIGN_IRQ
>> and the capability can be checked with KVM_HAS_DEVICE_ATTR.
>> Assignment can only be changed when the physical IRQ is not active.
>> It is the responsability of the user to do this check.
>>
>> This patch serie has the following dependencies:
>> - "ARM: Forwarding physical interrupts to a guest VM"
>>   (http://lwn.net/Articles/603514/) in
>> - [PATCH v2] irqfd for ARM
>>   which itself depends on
>>   - arm/arm64: KVM: Various VGIC cleanups and improvements
>>     http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/263685.html
>> - and obviously the VFIO platform driver serie:
>>   [RFC PATCH v6 00/20] VFIO support for platform devices on ARM
>>   https://www.mail-archive.com/kvm@vger.kernel.org/msg103247.html
>>
>> Integrated pieces can be found at
>> git://git.linaro.org/people/eric.auger/linux.git
>> on branch 3.17rc1_forward_integ_v0
>>
>> This was was tested on Calxeda Miday, assigning the xgmac main IRQ.
> 
> Presumably this optimization should provide lower interrupt exit latency
> and lower CPU overhead since we avoid the entire EOI path of the
> resampler.  Does it?  It seems like there should be a measurable
> improvement with something like netperf TCP_RR with this series.
> Thanks,

Hi Alex,

I will publish some performance figures soon. I am currently missing a
second node to run netserver.

My preliminary understanding is perf improvement will come from
1) reduction of EOI latency
2) potential saving of VM switches,

with 2) depending on  thephysical IRQ rate.

VERY HIGH RATE:
Without the patch (traditional irqfd on ARM with maintenance IRQ):
guest completes the vIRQ -> maintenance IRQ handler -> guest->host VM
switch -> resamplefd trigger (virqfd) -> enable physical IRQ -> new
physical IRQ hits -> VFIO handler -> fd trigger -> injection in guest ->
host->guest VM switch

with the patch
guest completes the vIRQ -> GIC completes the physical IRQ -> new
physical IRQ hits -> guest->host VM switch -> VFIO handler -> fd trigger
-> injection in guest -> host->guest VM switch

=> Same number of VM switches

SLOWER RATE:
Without the patch:
guest completes the vIRQ -> maintenance IRQ handler -> guest->host VM
switch -> resamplefd trigger (virqfd) -> enable physical IRQ
[host ..]
host->guest VM switch
[guest ..]
physical IRQ hits -> guest->host VM switch -> VFIO handler -> fd trigger
-> injection in guest -> host->guest VM switch

With that patch:
guest completes the vIRQ -> GIC completes the physical IRQ
[guest ..]
physical IRQ hits -> guest->host VM switch -> VFIO handler -> fd trigger
-> injection in guest -> host->guest VM switch

Hence less VM switches with that patch. But it is also related to
scheduling, relative load of host/guest...

Any comment welcome!

Best Regards

Eric
> 
> Alex
> 
>> Eric Auger (9):
>>   KVM: ARM: VGIC: fix multiple injection of level sensitive forwarded
>>     IRQ
>>   KVM: ARM: VGIC: add forwarded irq rbtree lock
>>   VFIO: platform: handler tests whether the IRQ is forwarded
>>   KVM: KVM-VFIO: update user API to program forwarded IRQ
>>   VFIO: Extend external user API
>>   KVM: KVM-VFIO: allow arch specific implementation
>>   KVM: KVM-VFIO: add new VFIO external API hooks
>>   KVM: KVM-VFIO: add kvm_vfio_arch_data and accessors
>>   KVM: KVM_VFIO: ARM: implement irq forwarding control
>>
>>  Documentation/virtual/kvm/devices/vfio.txt |  25 ++
>>  arch/arm/include/asm/kvm_host.h            |  16 +
>>  arch/arm/include/uapi/asm/kvm.h            |   6 +
>>  arch/arm/kvm/Makefile                      |   2 +-
>>  arch/arm/kvm/kvm_vfio_arm.c                | 599 +++++++++++++++++++++++++++++
>>  drivers/vfio/platform/vfio_platform_irq.c  |   7 +-
>>  drivers/vfio/vfio.c                        |  35 ++
>>  include/kvm/arm_vgic.h                     |   1 +
>>  include/linux/kvm_host.h                   |  30 ++
>>  include/linux/vfio.h                       |   4 +
>>  include/uapi/linux/kvm.h                   |   3 +
>>  virt/kvm/arm/vgic.c                        |  55 ++-
>>  virt/kvm/vfio.c                            |  92 +++++
>>  13 files changed, 862 insertions(+), 13 deletions(-)
>>  create mode 100644 arch/arm/kvm/kvm_vfio_arm.c
>>
> 
> 
> 

Re: [RFC 4/9] KVM: KVM-VFIO: update user API to program forwarded IRQ

[Eric Auger]

On 08/26/2014 09:01 PM, Alex Williamson wrote:
> On Mon, 2014-08-25 at 15:27 +0200, Eric Auger wrote:
>> add new device group commands:
>> - KVM_DEV_VFIO_DEVICE_ASSIGN_IRQ and
>>   KVM_DEV_VFIO_DEVICE_DEASSIGN_IRQ
>>
>> which enable to turn forwarded IRQ mode on/off.
>>
>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>> ---
>>  Documentation/virtual/kvm/devices/vfio.txt | 25 +++++++++++++++++++++++++
>>  arch/arm/include/uapi/asm/kvm.h            |  6 ++++++
>>  include/uapi/linux/kvm.h                   |  3 +++
>>  3 files changed, 34 insertions(+)
>>
>> diff --git a/Documentation/virtual/kvm/devices/vfio.txt b/Documentation/virtual/kvm/devices/vfio.txt
>> index ef51740..c8b3fa1 100644
>> --- a/Documentation/virtual/kvm/devices/vfio.txt
>> +++ b/Documentation/virtual/kvm/devices/vfio.txt
>> @@ -13,6 +13,7 @@ VFIO-group is held by KVM.
>>  
>>  Groups:
>>    KVM_DEV_VFIO_GROUP
>> +  KVM_DEV_VFIO_DEVICE
>>  
>>  KVM_DEV_VFIO_GROUP attributes:
>>    KVM_DEV_VFIO_GROUP_ADD: Add a VFIO group to VFIO-KVM device tracking
>> @@ -20,3 +21,27 @@ KVM_DEV_VFIO_GROUP attributes:
>>  
>>  For each, kvm_device_attr.addr points to an int32_t file descriptor
>>  for the VFIO group.
>> +
>> +KVM_DEV_VFIO_DEVICE attributes:
>> +  KVM_DEV_VFIO_DEVICE_ASSIGN_IRQ
>> +  KVM_DEV_VFIO_DEVICE_DEASSIGN_IRQ
>> +
>> +For each, kvm_device_attr.addr points to an kvm_arch_forwarded_irq.
>> +This user API makes possible to create a special IRQ handling mode,
>> +currently supported only on ARM, where KVM and a VFIO platform driver
>> +collaborate to improve IRQ handling performance.
>> +fd represents the file descriptor of a valid VFIO device whose physical
>> +IRQ, referenced by its irq_index is injected to the VM guest_irq.
>> +
>> +On ASSIGN_IRQ, KVM-VFIO device programs:
>> +- the host, to not complete the physical IRQ itself.
>> +- the GIC, to automatically complete the physical IRQ when the guest
>> +  completes the virtual IRQ
>> +This avoid trapping the end-of-interrupt for level sensitive IRQ.
>> +
>> +On DEASSIGN_IRQ, one come back to the mode where the host completes the
>> +physical IRQ and the guest only completes the virtual IRQ.
>> +
>> +It is up to the caller of this API to get the assurance the IRQ is not
>> +outstanding when the ASSIGN/DEASSIGN is called. This could lead to some
>> +inconsistency on who is going to complete the IRQ.
> 
> Why not call these FORWARD/UNFORWARD or something since the operation
> isn't really doing anything with assignment of the IRQ.  The IRQ is
> already "assigned", we're modifying the behavior.

Sure I will change the name.

> 
>> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
>> index 3034c66..1920b33 100644
>> --- a/arch/arm/include/uapi/asm/kvm.h
>> +++ b/arch/arm/include/uapi/asm/kvm.h
>> @@ -109,6 +109,12 @@ struct kvm_sync_regs {
>>  struct kvm_arch_memory_slot {
>>  };
>>  
>> +struct kvm_arch_forwarded_irq {
>> +	__u32 fd; /* file desciptor of the VFIO device */
>> +	__u32 irq_index; /* platform device index of the IRQ */
> 
> The vfio-platform device IRQ index?  ARM is the only implementation we
> have of this, but to keep it generic maybe the comment should read "vfio
> device IRQ index".

I will replace by vfio device IRQ index.

> 
>> +	__u32 guest_irq; /* virtual IRQ number */
> 
> This would be a GSI or similar concept if we were on x86.

ok for GSI now this naming seems better understood by the ARM community
too;-)

  I'm a little
> confused that were using an arch structure here rather than trying to
> keep the kvm-vfio device interface neutral.

I will move much more things on the generic side then then assuming that
someone in the future may use such functionality. Actually the only ARM
specific implementation is the GIC interrupt controller programming. As
far as I see the rest is generic, in terms of API.

Thanks

Eric

  Maybe it makes sense, I'm
> not sure.  Thanks,
> 
> Alex
> 
>> +};
>> +
>>  /* If you need to interpret the index values, here is the key: */
>>  #define KVM_REG_ARM_COPROC_MASK		0x000000000FFF0000
>>  #define KVM_REG_ARM_COPROC_SHIFT	16
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index cf3a2ff..b149ba8 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -954,6 +954,9 @@ struct kvm_device_attr {
>>  #define  KVM_DEV_VFIO_GROUP			1
>>  #define   KVM_DEV_VFIO_GROUP_ADD			1
>>  #define   KVM_DEV_VFIO_GROUP_DEL			2
>> +#define  KVM_DEV_VFIO_DEVICE			2
>> +#define   KVM_DEV_VFIO_DEVICE_ASSIGN_IRQ			1
>> +#define   KVM_DEV_VFIO_DEVICE_DEASSIGN_IRQ			2
>>  #define KVM_DEV_TYPE_ARM_VGIC_V2	5
>>  #define KVM_DEV_TYPE_FLIC		6
>>  
> 
> 
> 

Re: [RFC 5/9] VFIO: Extend external user API

[Eric Auger]

On 08/26/2014 09:02 PM, Alex Williamson wrote:
> On Mon, 2014-08-25 at 15:27 +0200, Eric Auger wrote:
>> New functions are added to be called from ARM KVM-VFIO device.
>>
>> - vfio_device_get_external_user enables to get a vfio device from
>>   its fd
>> - vfio_device_put_external_user puts the vfio device
>> - vfio_external_get_type enables to retrieve the type of the device
>>   (PCI or platform)
>> - vfio_external_get_base_device enables to get the
>>   struct device*, useful to access the platform_device
>>
>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>> ---
>>  drivers/vfio/vfio.c  | 35 +++++++++++++++++++++++++++++++++++
>>  include/linux/vfio.h |  4 ++++
>>  2 files changed, 39 insertions(+)
>>
>> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
>> index 8e84471..c93b9e4 100644
>> --- a/drivers/vfio/vfio.c
>> +++ b/drivers/vfio/vfio.c
>> @@ -1401,6 +1401,41 @@ void vfio_group_put_external_user(struct vfio_group *group)
>>  }
>>  EXPORT_SYMBOL_GPL(vfio_group_put_external_user);
>>  
>> +struct vfio_device *vfio_device_get_external_user(struct file *filep)
>> +{
>> +	struct vfio_device *vdev = filep->private_data;
>> +
>> +	if (filep->f_op != &vfio_device_fops)
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	vfio_device_get(vdev);
>> +	return vdev;
>> +}
>> +EXPORT_SYMBOL_GPL(vfio_device_get_external_user);
>> +
>> +void vfio_device_put_external_user(struct vfio_device *vdev)
>> +{
>> +	vfio_device_put(vdev);
>> +}
>> +EXPORT_SYMBOL_GPL(vfio_device_put_external_user);
>> +
>> +int vfio_external_get_type(struct vfio_device *vdev)
>> +{
>> +	if (!strcmp(vdev->ops->name,  "vfio-platform"))
>> +		return VFIO_DEVICE_FLAGS_PLATFORM;
>> +	else if (!strcmp(vdev->ops->name,  "vfio-pci"))
>> +		return VFIO_DEVICE_FLAGS_PCI;
>> +	else
>> +		return -EINVAL;
>> +}
>> +EXPORT_SYMBOL_GPL(vfio_external_get_type);
> 
> Returning the bit of the flag we use in get_device_info looks rather
> sloppy here.  Should we define a new enum for use with this?  Actually,
> is this interface even necessary?  If we can get the struct device then
> we can get the bus_type and keep vfio out of this.

thanks for the nit. I will try to get rid of it using it.
> 
> For both of these last two, I like to use the convention that where
> there is a "get" there is a matching "put".  These aren't reference
> counting anything, so let's not use get in the name.

I will rename.

Thanks

Eric
> 
>> +
>> +struct device *vfio_external_get_base_device(struct vfio_device *vdev)
>> +{
>> +	return vdev->dev;
>> +}
>> +EXPORT_SYMBOL_GPL(vfio_external_get_base_device);
>> +
> 
> Looks almost too simple, but reviewing the object lifecycles, this all
> looks safe.  Thanks,
> 
> Alex
> 
>>  int vfio_external_user_iommu_id(struct vfio_group *group)
>>  {
>>  	return iommu_group_id(group->iommu_group);
>> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
>> index ffe04ed..19e98eb 100644
>> --- a/include/linux/vfio.h
>> +++ b/include/linux/vfio.h
>> @@ -99,6 +99,10 @@ extern void vfio_group_put_external_user(struct vfio_group *group);
>>  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 struct vfio_device *vfio_device_get_external_user(struct file *filep);
>> +extern void vfio_device_put_external_user(struct vfio_device *vdev);
>> +extern int vfio_external_get_type(struct vfio_device *vdev);
>> +extern struct device *vfio_external_get_base_device(struct vfio_device *vdev);
>>  
>>  struct pci_dev;
>>  #ifdef CONFIG_EEH
> 
> 
> 

Re: [RFC 8/9] KVM: KVM-VFIO: add kvm_vfio_arch_data and accessors

[Eric Auger]

On 08/26/2014 09:02 PM, Alex Williamson wrote:
> On Mon, 2014-08-25 at 15:27 +0200, Eric Auger wrote:
>> add a pointer to architecture specific data in kvm_vfio struct
>> add accessors to keep kvm_vfio private
>>
>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>> ---
>>  arch/arm/include/asm/kvm_host.h |  8 ++++++++
>>  virt/kvm/vfio.c                 | 21 +++++++++++++++++++++
>>  2 files changed, 29 insertions(+)
>>
>> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
>> index 62cbf5b..4f1edbf 100644
>> --- a/arch/arm/include/asm/kvm_host.h
>> +++ b/arch/arm/include/asm/kvm_host.h
>> @@ -177,6 +177,14 @@ void kvm_vfio_device_put_external_user(struct vfio_device *vdev);
>>  int kvm_vfio_external_get_type(struct vfio_device *vdev);
>>  struct device *kvm_vfio_external_get_base_device(struct vfio_device *vdev);
>>  
>> +struct kvm_vfio;
>> +struct kvm_vfio_arch_data;
>> +void kvm_vfio_device_set_arch_data(struct kvm_vfio *kv,
>> +				   struct kvm_vfio_arch_data *ptr);
>> +struct kvm_vfio_arch_data *kvm_vfio_device_get_arch_data(struct kvm_vfio *kv);
>> +void kvm_vfio_lock(struct kvm_vfio *kv);
>> +void kvm_vfio_unlock(struct kvm_vfio *kv);
>> +
>>  /* We do not have shadow page tables, hence the empty hooks */
>>  static inline int kvm_age_hva(struct kvm *kvm, unsigned long hva)
>>  {
>> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
>> index f1c4e35..177b71e 100644
>> --- a/virt/kvm/vfio.c
>> +++ b/virt/kvm/vfio.c
>> @@ -28,6 +28,7 @@ struct kvm_vfio {
>>  	struct list_head group_list;
>>  	struct mutex lock;
>>  	bool noncoherent;
>> +	struct kvm_vfio_arch_data *arch_data;
>>  };
>>  
>>  static struct vfio_group *kvm_vfio_group_get_external_user(struct file *filep)
>> @@ -338,6 +339,26 @@ static int kvm_vfio_create(struct kvm_device *dev, u32 type)
>>  	return 0;
>>  }
>>  
>> +void kvm_vfio_device_set_arch_data(struct kvm_vfio *kv,
>> +				   struct kvm_vfio_arch_data *ptr)
>> +{
>> +	kv->arch_data = ptr;
>> +}
>> +
>> +struct kvm_vfio_arch_data *kvm_vfio_device_get_arch_data(struct kvm_vfio *kv)
>> +{
> 
> My preference would be s/get_//
ok
> 
>> +	return kv->arch_data;
>> +}
>> +
>> +void kvm_vfio_lock(struct kvm_vfio *kv)
>> +{
>> +	mutex_lock(&kv->lock);
>> +}
>> +
>> +void kvm_vfio_unlock(struct kvm_vfio *kv)
>> +{
>> +	mutex_unlock(&kv->lock);
>> +}
> 
> Gosh, what could go wrong...
Hum sorry I did not understand what you meant here

Thanks

Eric
> 
>>  
>>  struct kvm_device_ops kvm_vfio_ops = {
>>  	.name = "kvm-vfio",
> 
> 
> 

Re: [RFC 9/9] KVM: KVM_VFIO: ARM: implement irq forwarding control

[Eric Auger]

On 08/26/2014 09:02 PM, Alex Williamson wrote:
> On Mon, 2014-08-25 at 15:27 +0200, Eric Auger wrote:
>> Implements ARM specific KVM-VFIO device group commands:
>> - KVM_DEV_VFIO_DEVICE_ASSIGN_IRQ
>> - KVM_DEV_VFIO_DEVICE_DEASSIGN_IRQ
>> capability can be queried using KVM_HAS_DEVICE_ATTR.
>>
>> The new commands enable to set IRQ forwarding on/off for a given
>> IRQ index of a VFIO platform device.
>>
>> as soon as a forwarded irq is set, a reference to the VFIO device
>> is taken by the kvm-vfio device.
>>
>> The kvm-vfio device stores in the kvm_vfio_arch_data the list
>> of "assigned" devices (kvm_vfio_device). Each kvm_vfio_device
>> stores the list of assigned IRQs (potentially allowed a subset of
>> IRQ to be forwarded)
>>
>> The kvm-vfio device programs both the GIC and vGIC. Also it
>> clears the active bit on destruction, in case the guest did not
>> do it itself.
>>
>> Changing the forwarded state is not allowed in the critical
>> section starting from VFIO IRQ handler to LR programming. It is
>> up to the client to take care of this.
>>
>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>> ---
>>  arch/arm/include/asm/kvm_host.h |   2 +
>>  arch/arm/kvm/Makefile           |   2 +-
>>  arch/arm/kvm/kvm_vfio_arm.c     | 599 ++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 602 insertions(+), 1 deletion(-)
>>  create mode 100644 arch/arm/kvm/kvm_vfio_arm.c
> 
> I'm really happy that it seems like the kvm-vfio device is going to work
> for you, but I think too much stuff is being pushed out to arch code
> here.  Exporting the interfaces in patches 7 & 8 are setting the stage
> for duplicate code for anyone wanting to implement device attributes.
> Instead, I think the core code should support the list of
> kvm_vfio_devices with proper cleanup, and we should attempt to access
> the kvm_vfio_ callbacks as little as possible from arch code.  Thanks,

OK. my next iteration will feature much more generic code.

Thanks for the review

Best Regards

Eric

> 
> Alex
> 

Re: [RFC 8/9] KVM: KVM-VFIO: add kvm_vfio_arch_data and accessors

[Alex Williamson]

On Wed, 2014-08-27 at 17:22 +0200, Eric Auger wrote:
> On 08/26/2014 09:02 PM, Alex Williamson wrote:
> > On Mon, 2014-08-25 at 15:27 +0200, Eric Auger wrote:
> >> add a pointer to architecture specific data in kvm_vfio struct
> >> add accessors to keep kvm_vfio private
> >>
> >> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> >> ---
> >>  arch/arm/include/asm/kvm_host.h |  8 ++++++++
> >>  virt/kvm/vfio.c                 | 21 +++++++++++++++++++++
> >>  2 files changed, 29 insertions(+)
> >>
> >> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> >> index 62cbf5b..4f1edbf 100644
> >> --- a/arch/arm/include/asm/kvm_host.h
> >> +++ b/arch/arm/include/asm/kvm_host.h
> >> @@ -177,6 +177,14 @@ void kvm_vfio_device_put_external_user(struct vfio_device *vdev);
> >>  int kvm_vfio_external_get_type(struct vfio_device *vdev);
> >>  struct device *kvm_vfio_external_get_base_device(struct vfio_device *vdev);
> >>  
> >> +struct kvm_vfio;
> >> +struct kvm_vfio_arch_data;
> >> +void kvm_vfio_device_set_arch_data(struct kvm_vfio *kv,
> >> +				   struct kvm_vfio_arch_data *ptr);
> >> +struct kvm_vfio_arch_data *kvm_vfio_device_get_arch_data(struct kvm_vfio *kv);
> >> +void kvm_vfio_lock(struct kvm_vfio *kv);
> >> +void kvm_vfio_unlock(struct kvm_vfio *kv);
> >> +
> >>  /* We do not have shadow page tables, hence the empty hooks */
> >>  static inline int kvm_age_hva(struct kvm *kvm, unsigned long hva)
> >>  {
> >> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
> >> index f1c4e35..177b71e 100644
> >> --- a/virt/kvm/vfio.c
> >> +++ b/virt/kvm/vfio.c
> >> @@ -28,6 +28,7 @@ struct kvm_vfio {
> >>  	struct list_head group_list;
> >>  	struct mutex lock;
> >>  	bool noncoherent;
> >> +	struct kvm_vfio_arch_data *arch_data;
> >>  };
> >>  
> >>  static struct vfio_group *kvm_vfio_group_get_external_user(struct file *filep)
> >> @@ -338,6 +339,26 @@ static int kvm_vfio_create(struct kvm_device *dev, u32 type)
> >>  	return 0;
> >>  }
> >>  
> >> +void kvm_vfio_device_set_arch_data(struct kvm_vfio *kv,
> >> +				   struct kvm_vfio_arch_data *ptr)
> >> +{
> >> +	kv->arch_data = ptr;
> >> +}
> >> +
> >> +struct kvm_vfio_arch_data *kvm_vfio_device_get_arch_data(struct kvm_vfio *kv)
> >> +{
> > 
> > My preference would be s/get_//
> ok
> > 
> >> +	return kv->arch_data;
> >> +}
> >> +
> >> +void kvm_vfio_lock(struct kvm_vfio *kv)
> >> +{
> >> +	mutex_lock(&kv->lock);
> >> +}
> >> +
> >> +void kvm_vfio_unlock(struct kvm_vfio *kv)
> >> +{
> >> +	mutex_unlock(&kv->lock);
> >> +}
> > 
> > Gosh, what could go wrong...
> Hum sorry I did not understand what you meant here

Sorry, I was just sarcastically noting that exposing an internal lock
like this seems to be asking for trouble.  As you rework it to pull more
into the common code and generalize the architecture callouts, I hope we
can avoid exporting these locks.  Thanks,

Alex

Re: [RFC 8/9] KVM: KVM-VFIO: add kvm_vfio_arch_data and accessors

[Eric Auger]

On 08/27/2014 05:37 PM, Alex Williamson wrote:
> On Wed, 2014-08-27 at 17:22 +0200, Eric Auger wrote:
>> On 08/26/2014 09:02 PM, Alex Williamson wrote:
>>> On Mon, 2014-08-25 at 15:27 +0200, Eric Auger wrote:
>>>> add a pointer to architecture specific data in kvm_vfio struct
>>>> add accessors to keep kvm_vfio private
>>>>
>>>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>>>> ---
>>>>  arch/arm/include/asm/kvm_host.h |  8 ++++++++
>>>>  virt/kvm/vfio.c                 | 21 +++++++++++++++++++++
>>>>  2 files changed, 29 insertions(+)
>>>>
>>>> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
>>>> index 62cbf5b..4f1edbf 100644
>>>> --- a/arch/arm/include/asm/kvm_host.h
>>>> +++ b/arch/arm/include/asm/kvm_host.h
>>>> @@ -177,6 +177,14 @@ void kvm_vfio_device_put_external_user(struct vfio_device *vdev);
>>>>  int kvm_vfio_external_get_type(struct vfio_device *vdev);
>>>>  struct device *kvm_vfio_external_get_base_device(struct vfio_device *vdev);
>>>>  
>>>> +struct kvm_vfio;
>>>> +struct kvm_vfio_arch_data;
>>>> +void kvm_vfio_device_set_arch_data(struct kvm_vfio *kv,
>>>> +				   struct kvm_vfio_arch_data *ptr);
>>>> +struct kvm_vfio_arch_data *kvm_vfio_device_get_arch_data(struct kvm_vfio *kv);
>>>> +void kvm_vfio_lock(struct kvm_vfio *kv);
>>>> +void kvm_vfio_unlock(struct kvm_vfio *kv);
>>>> +
>>>>  /* We do not have shadow page tables, hence the empty hooks */
>>>>  static inline int kvm_age_hva(struct kvm *kvm, unsigned long hva)
>>>>  {
>>>> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
>>>> index f1c4e35..177b71e 100644
>>>> --- a/virt/kvm/vfio.c
>>>> +++ b/virt/kvm/vfio.c
>>>> @@ -28,6 +28,7 @@ struct kvm_vfio {
>>>>  	struct list_head group_list;
>>>>  	struct mutex lock;
>>>>  	bool noncoherent;
>>>> +	struct kvm_vfio_arch_data *arch_data;
>>>>  };
>>>>  
>>>>  static struct vfio_group *kvm_vfio_group_get_external_user(struct file *filep)
>>>> @@ -338,6 +339,26 @@ static int kvm_vfio_create(struct kvm_device *dev, u32 type)
>>>>  	return 0;
>>>>  }
>>>>  
>>>> +void kvm_vfio_device_set_arch_data(struct kvm_vfio *kv,
>>>> +				   struct kvm_vfio_arch_data *ptr)
>>>> +{
>>>> +	kv->arch_data = ptr;
>>>> +}
>>>> +
>>>> +struct kvm_vfio_arch_data *kvm_vfio_device_get_arch_data(struct kvm_vfio *kv)
>>>> +{
>>>
>>> My preference would be s/get_//
>> ok
>>>
>>>> +	return kv->arch_data;
>>>> +}
>>>> +
>>>> +void kvm_vfio_lock(struct kvm_vfio *kv)
>>>> +{
>>>> +	mutex_lock(&kv->lock);
>>>> +}
>>>> +
>>>> +void kvm_vfio_unlock(struct kvm_vfio *kv)
>>>> +{
>>>> +	mutex_unlock(&kv->lock);
>>>> +}
>>>
>>> Gosh, what could go wrong...
>> Hum sorry I did not understand what you meant here
> 
> Sorry, I was just sarcastically noting that exposing an internal lock
> like this seems to be asking for trouble.  As you rework it to pull more
> into the common code and generalize the architecture callouts, I hope we
> can avoid exporting these locks.  Thanks,
ok thanks. No problem I learnt a new word ;-)

> 
> Alex
>