[PATCH 00/11] kvmtool: Support new VGIC kernel features

[PATCH 00/11] kvmtool: Support new VGIC kernel features

[Andre Przywara]

Hi,

this series updates kvmtool to support some new KVM kernel features
related to the virtual GIC.
The first half of the series is mostly from Marc and has been
lingering around for some while, the second part is a slightly revised
and updated version of the virtual GICv3 support series.

We now use the KVM_CREATE_DEVICE interface to create a virtual GIC
and only fall back to the now legacy KVM_CREATE_IRQCHIP call if the
former is not supported by the kernel.
Also we use two new features the KVM_CREATE_DEVICE interface
introduces:
* We now set the number of actually used interrupts to avoid
  allocating too many of them without ever using them.
* We tell the kernel explicitly that we are finished with the GIC
  initialisation. This is a requirement for future VGIC versions.

The final five patches introduce virtual GICv3 support, so on
supported hardware (and given kernel support) the user can ask KVM to
emulate a GICv3, lifting the 8 VCPU limit of KVM. This is done by
specifying "--gicv3" on the command line.
As the kernel currently only supports this on ARM64, this parameter
is valid for the arm64 kvmtool build. But as the GIC is shared in
kvmtool, I had to add 9/11 to not break the build on ARM.

This series goes on top of the latest kvmtool version from Pekka's
github repo, which has seen a merge of the Linux 3.18 kernel base
just recently (master branch):
https://github.com/penberg/linux-kvm.git

For your convenience this series is also available in the
kvmtool-gicv3 branch of:
git://linux-arm.org/linux-ap.git
http://linux-arm.org/linux-ap.git

Cheers,
Andre.

Andre Przywara (7):
  kvmtool: add new VGIC_GRP definition (pulled from 3.19-rc1)
  kvmtool: finish VGIC initialisation explicitly
  kvmtool: prepare for instantiating different IRQ chip devices
  kvmtool: public header definitions from GICv3 emulation patch series
  kvmtool: add required GICv3 defines also to ARM
  kvmtool: add support for supplying GICv3 redistributor addresses
  kvmtool: add command line parameter to instantiate a vGICv3

Marc Zyngier (4):
  kvmtool: AArch64: Reserve two 64k pages for GIC CPU interface
  kvmtool: AArch{32,64}: use KVM_CREATE_DEVICE & co to instanciate the
    GIC
  kvmtool: irq: add irq__get_nr_allocated_lines
  kvmtool: AArch{32,64}: dynamically configure the number of GIC
    interrupts

 arch/arm64/include/uapi/asm/kvm.h                  |    7 +
 include/uapi/linux/kvm.h                           |    2 +
 tools/kvm/arm/aarch32/arm-cpu.c                    |    2 +-
 tools/kvm/arm/aarch32/include/kvm/kvm-arch.h       |    8 +
 tools/kvm/arm/aarch64/arm-cpu.c                    |    5 +-
 tools/kvm/arm/aarch64/include/kvm/kvm-arch.h       |    2 +-
 .../kvm/arm/aarch64/include/kvm/kvm-config-arch.h  |    4 +-
 tools/kvm/arm/gic.c                                |  178 ++++++++++++++++++--
 tools/kvm/arm/include/arm-common/gic.h             |    9 +-
 tools/kvm/arm/include/arm-common/kvm-arch.h        |   18 +-
 tools/kvm/arm/include/arm-common/kvm-config-arch.h |    1 +
 tools/kvm/arm/kvm-cpu.c                            |    4 +-
 tools/kvm/arm/kvm.c                                |    7 +-
 tools/kvm/include/kvm/irq.h                        |    1 +
 tools/kvm/irq.c                                    |    5 +
 15 files changed, 228 insertions(+), 25 deletions(-)

-- 
1.7.9.5

[PATCH 01/11] kvmtool: add new VGIC_GRP definition (pulled from 3.19-rc1)

[Andre Przywara]

We are using the explicit VGIC_CTRL_INIT ioctl, which was introduced
with 3.19-rc1, so at the moment we need to pull this patch from
upstream Linux. This patch can be reverted as soon as a newer Linux
tree is merged into kvmtool.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 tools/kvm/arm/include/arm-common/gic.h |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/kvm/arm/include/arm-common/gic.h b/tools/kvm/arm/include/arm-common/gic.h
index 5a36f2c..5487125 100644
--- a/tools/kvm/arm/include/arm-common/gic.h
+++ b/tools/kvm/arm/include/arm-common/gic.h
@@ -21,6 +21,11 @@
 #define GIC_MAX_CPUS			8
 #define GIC_MAX_IRQ			255
 
+#ifndef KVM_DEV_ARM_VGIC_GRP_CTRL
+#define KVM_DEV_ARM_VGIC_GRP_CTRL	4
+#define   KVM_DEV_ARM_VGIC_CTRL_INIT	0
+#endif
+
 struct kvm;
 
 int gic__alloc_irqnum(void);
-- 
1.7.9.5

[PATCH 02/11] kvmtool: AArch64: Reserve two 64k pages for GIC CPU interface

[Andre Przywara]

From: Marc Zyngier <marc.zyngier@arm.com>

On AArch64 system with a GICv2, the GICC range can be aligned
to the last 4k block of a 64k page, ending up straddling two
64k pages. In order not to conflict with the distributor mapping,
allocate two 64k pages to the CPU interface.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 tools/kvm/arm/aarch64/include/kvm/kvm-arch.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/kvm/arm/aarch64/include/kvm/kvm-arch.h b/tools/kvm/arm/aarch64/include/kvm/kvm-arch.h
index 2f08a26..4925736 100644
--- a/tools/kvm/arm/aarch64/include/kvm/kvm-arch.h
+++ b/tools/kvm/arm/aarch64/include/kvm/kvm-arch.h
@@ -2,7 +2,7 @@
 #define KVM__KVM_ARCH_H
 
 #define ARM_GIC_DIST_SIZE	0x10000
-#define ARM_GIC_CPUI_SIZE	0x10000
+#define ARM_GIC_CPUI_SIZE	0x20000
 
 #define ARM_KERN_OFFSET(kvm)	((kvm)->cfg.arch.aarch32_guest	?	\
 				0x8000				:	\
-- 
1.7.9.5

[PATCH 03/11] kvmtool: AArch{32,64}: use KVM_CREATE_DEVICE & co to instanciate the GIC

[Andre Przywara]

From: Marc Zyngier <marc.zyngier@arm.com>

As of 3.14, KVM/arm supports the creation/configuration of the GIC through
a more generic device API, which is now the preferred way to do so.

Plumb the new API in, and allow the old code to be used as a fallback.

[Andre: Rename some functions on the way to differentiate between
creation and initialisation more clearly.]

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 tools/kvm/arm/gic.c                    |   60 ++++++++++++++++++++++++++++----
 tools/kvm/arm/include/arm-common/gic.h |    2 +-
 tools/kvm/arm/kvm.c                    |    6 ++--
 3 files changed, 57 insertions(+), 11 deletions(-)

diff --git a/tools/kvm/arm/gic.c b/tools/kvm/arm/gic.c
index 5d8cbe6..ce5f7fa 100644
--- a/tools/kvm/arm/gic.c
+++ b/tools/kvm/arm/gic.c
@@ -7,7 +7,41 @@
 #include <linux/byteorder.h>
 #include <linux/kvm.h>
 
-int gic__init_irqchip(struct kvm *kvm)
+static int gic_fd = -1;
+
+static int gic__create_device(struct kvm *kvm)
+{
+	int err;
+	u64 cpu_if_addr = ARM_GIC_CPUI_BASE;
+	u64 dist_addr = ARM_GIC_DIST_BASE;
+	struct kvm_create_device gic_device = {
+		.type	= KVM_DEV_TYPE_ARM_VGIC_V2,
+	};
+	struct kvm_device_attr cpu_if_attr = {
+		.group	= KVM_DEV_ARM_VGIC_GRP_ADDR,
+		.attr	= KVM_VGIC_V2_ADDR_TYPE_CPU,
+		.addr	= (u64)(unsigned long)&cpu_if_addr,
+	};
+	struct kvm_device_attr dist_attr = {
+		.group	= KVM_DEV_ARM_VGIC_GRP_ADDR,
+		.attr	= KVM_VGIC_V2_ADDR_TYPE_DIST,
+		.addr	= (u64)(unsigned long)&dist_addr,
+	};
+
+	err = ioctl(kvm->vm_fd, KVM_CREATE_DEVICE, &gic_device);
+	if (err)
+		return err;
+
+	gic_fd = gic_device.fd;
+
+	err = ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &cpu_if_attr);
+	if (err)
+		return err;
+
+	return ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &dist_attr);
+}
+
+static int gic__create_irqchip(struct kvm *kvm)
 {
 	int err;
 	struct kvm_arm_device_addr gic_addr[] = {
@@ -23,12 +57,6 @@ int gic__init_irqchip(struct kvm *kvm)
 		}
 	};
 
-	if (kvm->nrcpus > GIC_MAX_CPUS) {
-		pr_warning("%d CPUS greater than maximum of %d -- truncating\n",
-				kvm->nrcpus, GIC_MAX_CPUS);
-		kvm->nrcpus = GIC_MAX_CPUS;
-	}
-
 	err = ioctl(kvm->vm_fd, KVM_CREATE_IRQCHIP);
 	if (err)
 		return err;
@@ -41,6 +69,24 @@ int gic__init_irqchip(struct kvm *kvm)
 	return err;
 }
 
+int gic__create(struct kvm *kvm)
+{
+	int err;
+
+	if (kvm->nrcpus > GIC_MAX_CPUS) {
+		pr_warning("%d CPUS greater than maximum of %d -- truncating\n",
+				kvm->nrcpus, GIC_MAX_CPUS);
+		kvm->nrcpus = GIC_MAX_CPUS;
+	}
+
+	/* Try the new way first, and fallback on legacy method otherwise */
+	err = gic__create_device(kvm);
+	if (err)
+		err = gic__create_irqchip(kvm);
+
+	return err;
+}
+
 void gic__generate_fdt_nodes(void *fdt, u32 phandle)
 {
 	u64 reg_prop[] = {
diff --git a/tools/kvm/arm/include/arm-common/gic.h b/tools/kvm/arm/include/arm-common/gic.h
index 5487125..9bbe81e 100644
--- a/tools/kvm/arm/include/arm-common/gic.h
+++ b/tools/kvm/arm/include/arm-common/gic.h
@@ -29,7 +29,7 @@
 struct kvm;
 
 int gic__alloc_irqnum(void);
-int gic__init_irqchip(struct kvm *kvm);
+int gic__create(struct kvm *kvm);
 void gic__generate_fdt_nodes(void *fdt, u32 phandle);
 
 #endif /* ARM_COMMON__GIC_H */
diff --git a/tools/kvm/arm/kvm.c b/tools/kvm/arm/kvm.c
index 58ad9fa..bcd2533 100644
--- a/tools/kvm/arm/kvm.c
+++ b/tools/kvm/arm/kvm.c
@@ -81,7 +81,7 @@ void kvm__arch_init(struct kvm *kvm, const char *hugetlbfs_path, u64 ram_size)
 	madvise(kvm->arch.ram_alloc_start, kvm->arch.ram_alloc_size,
 		MADV_MERGEABLE | MADV_HUGEPAGE);
 
-	/* Initialise the virtual GIC. */
-	if (gic__init_irqchip(kvm))
-		die("Failed to initialise virtual GIC");
+	/* Create the virtual GIC. */
+	if (gic__create(kvm))
+		die("Failed to create virtual GIC");
 }
-- 
1.7.9.5

[PATCH 04/11] kvmtool: irq: add irq__get_nr_allocated_lines

[Andre Przywara]

From: Marc Zyngier <marc.zyngier@arm.com>

The ARM GIC emulation needs to be told the number of interrupts
it has to support. As commit 1c262fa1dc7b ("kvm tools: irq: make
irq__alloc_line generic") made the interrupt counter private,
add a new accessor returning the number of interrupt lines we've
allocated so far.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 tools/kvm/include/kvm/irq.h |    1 +
 tools/kvm/irq.c             |    5 +++++
 2 files changed, 6 insertions(+)

diff --git a/tools/kvm/include/kvm/irq.h b/tools/kvm/include/kvm/irq.h
index 4cec6f0..8a78e43 100644
--- a/tools/kvm/include/kvm/irq.h
+++ b/tools/kvm/include/kvm/irq.h
@@ -11,6 +11,7 @@
 struct kvm;
 
 int irq__alloc_line(void);
+int irq__get_nr_allocated_lines(void);
 
 int irq__init(struct kvm *kvm);
 int irq__exit(struct kvm *kvm);
diff --git a/tools/kvm/irq.c b/tools/kvm/irq.c
index 33ea8d2..71eaa05 100644
--- a/tools/kvm/irq.c
+++ b/tools/kvm/irq.c
@@ -7,3 +7,8 @@ int irq__alloc_line(void)
 {
 	return next_line++;
 }
+
+int irq__get_nr_allocated_lines(void)
+{
+	return next_line - KVM_IRQ_OFFSET;
+}
-- 
1.7.9.5

[PATCH 05/11] kvmtool: AArch{32,64}: dynamically configure the number of GIC interrupts

[Andre Przywara]

From: Marc Zyngier <marc.zyngier@arm.com>

In order to reduce the memory usage of large guests (as well
as improve performance), tell KVM about the number of interrupts
we require.

To avoid synchronization with the various device creation,
use a late_init callback to compute the GIC configuration.
[Andre: rename to gic__init_gic() to ease future expansion]

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 tools/kvm/arm/gic.c |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/tools/kvm/arm/gic.c b/tools/kvm/arm/gic.c
index ce5f7fa..6277af8 100644
--- a/tools/kvm/arm/gic.c
+++ b/tools/kvm/arm/gic.c
@@ -1,10 +1,12 @@
 #include "kvm/fdt.h"
+#include "kvm/irq.h"
 #include "kvm/kvm.h"
 #include "kvm/virtio.h"
 
 #include "arm-common/gic.h"
 
 #include <linux/byteorder.h>
+#include <linux/kernel.h>
 #include <linux/kvm.h>
 
 static int gic_fd = -1;
@@ -87,6 +89,29 @@ int gic__create(struct kvm *kvm)
 	return err;
 }
 
+static int gic__init_gic(struct kvm *kvm)
+{
+	int lines = irq__get_nr_allocated_lines();
+	u32 nr_irqs = ALIGN(lines, 32) + GIC_SPI_IRQ_BASE;
+	struct kvm_device_attr nr_irqs_attr = {
+		.group	= KVM_DEV_ARM_VGIC_GRP_NR_IRQS,
+		.addr	= (u64)(unsigned long)&nr_irqs,
+	};
+
+	/*
+	 * If we didn't use the KVM_CREATE_DEVICE method, KVM will
+	 * give us some default number of interrupts.
+	 */
+	if (gic_fd < 0)
+		return 0;
+
+	if (!ioctl(gic_fd, KVM_HAS_DEVICE_ATTR, &nr_irqs_attr))
+		return ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &nr_irqs_attr);
+
+	return 0;
+}
+late_init(gic__init_gic)
+
 void gic__generate_fdt_nodes(void *fdt, u32 phandle)
 {
 	u64 reg_prop[] = {
-- 
1.7.9.5

[PATCH 06/11] kvmtool: finish VGIC initialisation explicitly

[Andre Przywara]

Since Linux 3.19-rc1 there is a new API to explicitly initialise
the in-kernel GIC emulation by a userland KVM device call.
Use that to tell the kernel we are finished with the GIC
initialisation, since the automatic GIC init will only be provided
as a legacy functionality in the future.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 tools/kvm/arm/gic.c |   25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/tools/kvm/arm/gic.c b/tools/kvm/arm/gic.c
index 6277af8..8d47562 100644
--- a/tools/kvm/arm/gic.c
+++ b/tools/kvm/arm/gic.c
@@ -89,24 +89,43 @@ int gic__create(struct kvm *kvm)
 	return err;
 }
 
+/*
+ * Sets the number of used interrupts and finalizes the GIC init explicitly.
+ */
 static int gic__init_gic(struct kvm *kvm)
 {
+	int ret;
+
 	int lines = irq__get_nr_allocated_lines();
 	u32 nr_irqs = ALIGN(lines, 32) + GIC_SPI_IRQ_BASE;
 	struct kvm_device_attr nr_irqs_attr = {
 		.group	= KVM_DEV_ARM_VGIC_GRP_NR_IRQS,
 		.addr	= (u64)(unsigned long)&nr_irqs,
 	};
+	struct kvm_device_attr vgic_init_attr = {
+		.group	= KVM_DEV_ARM_VGIC_GRP_CTRL,
+		.attr	= KVM_DEV_ARM_VGIC_CTRL_INIT,
+	};
 
 	/*
 	 * If we didn't use the KVM_CREATE_DEVICE method, KVM will
-	 * give us some default number of interrupts.
+	 * give us some default number of interrupts. The GIC initialization
+	 * will be done automatically in this case.
 	 */
 	if (gic_fd < 0)
 		return 0;
 
-	if (!ioctl(gic_fd, KVM_HAS_DEVICE_ATTR, &nr_irqs_attr))
-		return ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &nr_irqs_attr);
+	if (!ioctl(gic_fd, KVM_HAS_DEVICE_ATTR, &nr_irqs_attr)) {
+		ret = ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &nr_irqs_attr);
+		if (ret)
+			return ret;
+	}
+
+	if (!ioctl(gic_fd, KVM_HAS_DEVICE_ATTR, &vgic_init_attr)) {
+		ret = ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &vgic_init_attr);
+		if (ret)
+			return ret;
+	}
 
 	return 0;
 }
-- 
1.7.9.5

[PATCH 07/11] kvmtool: prepare for instantiating different IRQ chip devices

[Andre Przywara]

Extend the vGIC handling code to deal with different IRQ chip devices
instead of hard-coding the GICv2 in.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 tools/kvm/arm/aarch32/arm-cpu.c        |    2 +-
 tools/kvm/arm/aarch64/arm-cpu.c        |    2 +-
 tools/kvm/arm/gic.c                    |   52 ++++++++++++++++++++++++--------
 tools/kvm/arm/include/arm-common/gic.h |    4 +--
 tools/kvm/arm/kvm.c                    |    2 +-
 5 files changed, 45 insertions(+), 17 deletions(-)

diff --git a/tools/kvm/arm/aarch32/arm-cpu.c b/tools/kvm/arm/aarch32/arm-cpu.c
index 946e443..ce68dc6 100644
--- a/tools/kvm/arm/aarch32/arm-cpu.c
+++ b/tools/kvm/arm/aarch32/arm-cpu.c
@@ -12,7 +12,7 @@ static void generate_fdt_nodes(void *fdt, struct kvm *kvm, u32 gic_phandle)
 {
 	int timer_interrupts[4] = {13, 14, 11, 10};
 
-	gic__generate_fdt_nodes(fdt, gic_phandle);
+	gic__generate_fdt_nodes(fdt, gic_phandle, KVM_DEV_TYPE_ARM_VGIC_V2);
 	timer__generate_fdt_nodes(fdt, kvm, timer_interrupts);
 }
 
diff --git a/tools/kvm/arm/aarch64/arm-cpu.c b/tools/kvm/arm/aarch64/arm-cpu.c
index 8efe877..a70d6bb 100644
--- a/tools/kvm/arm/aarch64/arm-cpu.c
+++ b/tools/kvm/arm/aarch64/arm-cpu.c
@@ -12,7 +12,7 @@
 static void generate_fdt_nodes(void *fdt, struct kvm *kvm, u32 gic_phandle)
 {
 	int timer_interrupts[4] = {13, 14, 11, 10};
-	gic__generate_fdt_nodes(fdt, gic_phandle);
+	gic__generate_fdt_nodes(fdt, gic_phandle, KVM_DEV_TYPE_ARM_VGIC_V2);
 	timer__generate_fdt_nodes(fdt, kvm, timer_interrupts);
 }
 
diff --git a/tools/kvm/arm/gic.c b/tools/kvm/arm/gic.c
index 8d47562..f4bda339 100644
--- a/tools/kvm/arm/gic.c
+++ b/tools/kvm/arm/gic.c
@@ -11,13 +11,13 @@
 
 static int gic_fd = -1;
 
-static int gic__create_device(struct kvm *kvm)
+static int gic__create_device(struct kvm *kvm, u32 type)
 {
 	int err;
 	u64 cpu_if_addr = ARM_GIC_CPUI_BASE;
 	u64 dist_addr = ARM_GIC_DIST_BASE;
 	struct kvm_create_device gic_device = {
-		.type	= KVM_DEV_TYPE_ARM_VGIC_V2,
+		.type	= type,
 	};
 	struct kvm_device_attr cpu_if_attr = {
 		.group	= KVM_DEV_ARM_VGIC_GRP_ADDR,
@@ -36,11 +36,19 @@ static int gic__create_device(struct kvm *kvm)
 
 	gic_fd = gic_device.fd;
 
-	err = ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &cpu_if_attr);
-	if (err)
-		return err;
+	switch (type) {
+	case KVM_DEV_TYPE_ARM_VGIC_V2:
+		err = ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &cpu_if_attr);
+		if (err)
+			return err;
+		break;
+	default:
+		return -ENODEV;
+	}
 
-	return ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &dist_attr);
+	err = ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &dist_attr);
+
+	return err;
 }
 
 static int gic__create_irqchip(struct kvm *kvm)
@@ -71,7 +79,7 @@ static int gic__create_irqchip(struct kvm *kvm)
 	return err;
 }
 
-int gic__create(struct kvm *kvm)
+static int gicv2__create_irqchip(struct kvm *kvm)
 {
 	int err;
 
@@ -82,13 +90,22 @@ int gic__create(struct kvm *kvm)
 	}
 
 	/* Try the new way first, and fallback on legacy method otherwise */
-	err = gic__create_device(kvm);
+	err = gic__create_device(kvm, KVM_DEV_TYPE_ARM_VGIC_V2);
 	if (err)
 		err = gic__create_irqchip(kvm);
 
 	return err;
 }
 
+int gic__create(struct kvm *kvm, u32 type)
+{
+	switch (type) {
+	case KVM_DEV_TYPE_ARM_VGIC_V2:
+		return gicv2__create_irqchip(kvm);
+	}
+	return -ENODEV;
+}
+
 /*
  * Sets the number of used interrupts and finalizes the GIC init explicitly.
  */
@@ -131,15 +148,26 @@ static int gic__init_gic(struct kvm *kvm)
 }
 late_init(gic__init_gic)
 
-void gic__generate_fdt_nodes(void *fdt, u32 phandle)
+void gic__generate_fdt_nodes(void *fdt, u32 phandle, u32 type)
 {
+	const char *compatible;
 	u64 reg_prop[] = {
-		cpu_to_fdt64(ARM_GIC_DIST_BASE), cpu_to_fdt64(ARM_GIC_DIST_SIZE),
-		cpu_to_fdt64(ARM_GIC_CPUI_BASE), cpu_to_fdt64(ARM_GIC_CPUI_SIZE),
+		cpu_to_fdt64(ARM_GIC_DIST_BASE),
+		cpu_to_fdt64(ARM_GIC_DIST_SIZE),
+		cpu_to_fdt64(ARM_GIC_CPUI_BASE),
+		cpu_to_fdt64(ARM_GIC_CPUI_SIZE),
 	};
 
+	switch (type) {
+	case KVM_DEV_TYPE_ARM_VGIC_V2:
+		compatible = "arm,cortex-a15-gic";
+		break;
+	default:
+		return;
+	}
+
 	_FDT(fdt_begin_node(fdt, "intc"));
-	_FDT(fdt_property_string(fdt, "compatible", "arm,cortex-a15-gic"));
+	_FDT(fdt_property_string(fdt, "compatible", compatible));
 	_FDT(fdt_property_cell(fdt, "#interrupt-cells", GIC_FDT_IRQ_NUM_CELLS));
 	_FDT(fdt_property(fdt, "interrupt-controller", NULL, 0));
 	_FDT(fdt_property(fdt, "reg", reg_prop, sizeof(reg_prop)));
diff --git a/tools/kvm/arm/include/arm-common/gic.h b/tools/kvm/arm/include/arm-common/gic.h
index 9bbe81e..149f2ca 100644
--- a/tools/kvm/arm/include/arm-common/gic.h
+++ b/tools/kvm/arm/include/arm-common/gic.h
@@ -29,7 +29,7 @@
 struct kvm;
 
 int gic__alloc_irqnum(void);
-int gic__create(struct kvm *kvm);
-void gic__generate_fdt_nodes(void *fdt, u32 phandle);
+int gic__create(struct kvm *kvm, u32 type);
+void gic__generate_fdt_nodes(void *fdt, u32 phandle, u32 type);
 
 #endif /* ARM_COMMON__GIC_H */
diff --git a/tools/kvm/arm/kvm.c b/tools/kvm/arm/kvm.c
index bcd2533..8d31fd7 100644
--- a/tools/kvm/arm/kvm.c
+++ b/tools/kvm/arm/kvm.c
@@ -82,6 +82,6 @@ void kvm__arch_init(struct kvm *kvm, const char *hugetlbfs_path, u64 ram_size)
 		MADV_MERGEABLE | MADV_HUGEPAGE);
 
 	/* Create the virtual GIC. */
-	if (gic__create(kvm))
+	if (gic__create(kvm, KVM_DEV_TYPE_ARM_VGIC_V2))
 		die("Failed to create virtual GIC");
 }
-- 
1.7.9.5

[PATCH 08/11] kvmtool: public header definitions from GICv3 emulation patch series

[Andre Przywara]

This pulls the necessary defines for the GICv3 constants from the
Linux tree into kvmtool for now. Should be obsolete as soon as
the vGICv3 patches are upstream and kvmtool is rebased on top of
it.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm64/include/uapi/asm/kvm.h |    7 +++++++
 include/uapi/linux/kvm.h          |    2 ++
 2 files changed, 9 insertions(+)

diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index 8e38878..2ed873a 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -78,6 +78,13 @@ struct kvm_regs {
 #define KVM_VGIC_V2_DIST_SIZE		0x1000
 #define KVM_VGIC_V2_CPU_SIZE		0x2000
 
+/* Supported VGICv3 address types  */
+#define KVM_VGIC_V3_ADDR_TYPE_DIST	2
+#define KVM_VGIC_V3_ADDR_TYPE_REDIST	3
+
+#define KVM_VGIC_V3_DIST_SIZE		SZ_64K
+#define KVM_VGIC_V3_REDIST_SIZE		(2 * SZ_64K)
+
 #define KVM_ARM_VCPU_POWER_OFF		0 /* CPU is started in OFF state */
 #define KVM_ARM_VCPU_EL1_32BIT		1 /* CPU running a 32bit VM */
 #define KVM_ARM_VCPU_PSCI_0_2		2 /* CPU uses PSCI v0.2 */
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 6076882..24cb129 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -960,6 +960,8 @@ enum kvm_device_type {
 #define KVM_DEV_TYPE_ARM_VGIC_V2	KVM_DEV_TYPE_ARM_VGIC_V2
 	KVM_DEV_TYPE_FLIC,
 #define KVM_DEV_TYPE_FLIC		KVM_DEV_TYPE_FLIC
+	KVM_DEV_TYPE_ARM_VGIC_V3,
+#define KVM_DEV_TYPE_ARM_VGIC_V3	KVM_DEV_TYPE_ARM_VGIC_V3
 	KVM_DEV_TYPE_MAX,
 };
 
-- 
1.7.9.5

[PATCH 09/11] kvmtool: add required GICv3 defines also to ARM

[Andre Przywara]

The virtual GICv3 support in the kernel is currently for ARM64 only,
so the ARM uapi headers lack the defines for the GICv3 KVM device
function IDs.
However this will break compilation for ARM with GICv3 support later,
as the GIC code in kvmtool is shared between ARM and ARM64.
Splitting this deliberately makes no sense, also #ifdef-ing GICv3
specific code is messy and serves no purpose other than work
around the missing definitions. Since GICv3 emulation may be extended
to ARM in the future, let's fix compilation for ARM by providing the
two required #define's in a kvmtool local header for the time being.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 tools/kvm/arm/aarch32/include/kvm/kvm-arch.h |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/kvm/arm/aarch32/include/kvm/kvm-arch.h b/tools/kvm/arm/aarch32/include/kvm/kvm-arch.h
index 1632e3c..11dc9a6 100644
--- a/tools/kvm/arm/aarch32/include/kvm/kvm-arch.h
+++ b/tools/kvm/arm/aarch32/include/kvm/kvm-arch.h
@@ -8,6 +8,14 @@
 
 #define ARM_MAX_MEMORY(...)	ARM_LOMAP_MAX_MEMORY
 
+/*
+ * This is here for the time being until KVM/ARM gets virtual GICv3
+ * emulation also. The uapi headers provide that definition only
+ * for ARM64.
+ */
+#define KVM_VGIC_V3_ADDR_TYPE_DIST	2
+#define KVM_VGIC_V3_ADDR_TYPE_REDIST	3
+
 #include "arm-common/kvm-arch.h"
 
 #endif /* KVM__KVM_ARCH_H */
-- 
1.7.9.5

[PATCH 10/11] kvmtool: add support for supplying GICv3 redistributor addresses

[Andre Przywara]

The code currently is assuming fixed sized memory regions for the
distributor and CPU interface. GICv3 needs a dynamic allocation of
it's redistributor region, since it's size depends on the number of
vCPUs.
Also add the necessary code to create a GICv3 IRQ chip instance.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 tools/kvm/arm/gic.c                         |   30 ++++++++++++++++++++++++---
 tools/kvm/arm/include/arm-common/kvm-arch.h |   18 ++++++++++++----
 tools/kvm/arm/kvm-cpu.c                     |    4 +++-
 3 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/tools/kvm/arm/gic.c b/tools/kvm/arm/gic.c
index f4bda339..9435715 100644
--- a/tools/kvm/arm/gic.c
+++ b/tools/kvm/arm/gic.c
@@ -10,12 +10,14 @@
 #include <linux/kvm.h>
 
 static int gic_fd = -1;
+static int nr_redists;
 
 static int gic__create_device(struct kvm *kvm, u32 type)
 {
 	int err;
 	u64 cpu_if_addr = ARM_GIC_CPUI_BASE;
 	u64 dist_addr = ARM_GIC_DIST_BASE;
+	u64 redist_addr;
 	struct kvm_create_device gic_device = {
 		.type	= type,
 	};
@@ -26,9 +28,13 @@ static int gic__create_device(struct kvm *kvm, u32 type)
 	};
 	struct kvm_device_attr dist_attr = {
 		.group	= KVM_DEV_ARM_VGIC_GRP_ADDR,
-		.attr	= KVM_VGIC_V2_ADDR_TYPE_DIST,
 		.addr	= (u64)(unsigned long)&dist_addr,
 	};
+	struct kvm_device_attr redist_attr = {
+		.group	= KVM_DEV_ARM_VGIC_GRP_ADDR,
+		.attr	= KVM_VGIC_V3_ADDR_TYPE_REDIST,
+		.addr	= (u64)(unsigned long)&redist_addr,
+	};
 
 	err = ioctl(kvm->vm_fd, KVM_CREATE_DEVICE, &gic_device);
 	if (err)
@@ -41,12 +47,22 @@ static int gic__create_device(struct kvm *kvm, u32 type)
 		err = ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &cpu_if_attr);
 		if (err)
 			return err;
+		dist_attr.attr = KVM_VGIC_V2_ADDR_TYPE_DIST;
+		break;
+	case KVM_DEV_TYPE_ARM_VGIC_V3:
+		dist_attr.attr = KVM_VGIC_V3_ADDR_TYPE_DIST;
+		redist_addr = dist_addr - nr_redists * ARM_GIC_REDIST_SIZE;
 		break;
 	default:
 		return -ENODEV;
 	}
 
 	err = ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &dist_attr);
+	if (err)
+		return err;
+
+	if (type == KVM_DEV_TYPE_ARM_VGIC_V3)
+		err = ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &redist_attr);
 
 	return err;
 }
@@ -154,17 +170,25 @@ void gic__generate_fdt_nodes(void *fdt, u32 phandle, u32 type)
 	u64 reg_prop[] = {
 		cpu_to_fdt64(ARM_GIC_DIST_BASE),
 		cpu_to_fdt64(ARM_GIC_DIST_SIZE),
-		cpu_to_fdt64(ARM_GIC_CPUI_BASE),
-		cpu_to_fdt64(ARM_GIC_CPUI_SIZE),
+		0, 0,				/* to be filled */
 	};
 
 	switch (type) {
 	case KVM_DEV_TYPE_ARM_VGIC_V2:
 		compatible = "arm,cortex-a15-gic";
+		reg_prop[2] = ARM_GIC_CPUI_BASE;
+		reg_prop[3] = ARM_GIC_CPUI_SIZE;
+		break;
+	case KVM_DEV_TYPE_ARM_VGIC_V3:
+		compatible = "arm,gic-v3";
+		reg_prop[2] = ARM_GIC_DIST_BASE - nr_redists * ARM_GIC_REDIST_SIZE;
+		reg_prop[3] = ARM_GIC_REDIST_SIZE * nr_redists;
 		break;
 	default:
 		return;
 	}
+	reg_prop[2] = cpu_to_fdt64(reg_prop[2]);
+	reg_prop[3] = cpu_to_fdt64(reg_prop[3]);
 
 	_FDT(fdt_begin_node(fdt, "intc"));
 	_FDT(fdt_property_string(fdt, "compatible", compatible));
diff --git a/tools/kvm/arm/include/arm-common/kvm-arch.h b/tools/kvm/arm/include/arm-common/kvm-arch.h
index 082131d..be66a76 100644
--- a/tools/kvm/arm/include/arm-common/kvm-arch.h
+++ b/tools/kvm/arm/include/arm-common/kvm-arch.h
@@ -17,10 +17,8 @@
 
 #define ARM_GIC_DIST_BASE	(ARM_AXI_AREA - ARM_GIC_DIST_SIZE)
 #define ARM_GIC_CPUI_BASE	(ARM_GIC_DIST_BASE - ARM_GIC_CPUI_SIZE)
-#define ARM_GIC_SIZE		(ARM_GIC_DIST_SIZE + ARM_GIC_CPUI_SIZE)
 
 #define ARM_IOPORT_SIZE		(ARM_MMIO_AREA - ARM_IOPORT_AREA)
-#define ARM_VIRTIO_MMIO_SIZE	(ARM_AXI_AREA - (ARM_MMIO_AREA + ARM_GIC_SIZE))
 #define ARM_PCI_CFG_SIZE	(1ULL << 24)
 #define ARM_PCI_MMIO_SIZE	(ARM_MEMORY_AREA - \
 				(ARM_AXI_AREA + ARM_PCI_CFG_SIZE))
@@ -30,6 +28,13 @@
 #define KVM_PCI_MMIO_AREA	(KVM_PCI_CFG_AREA + ARM_PCI_CFG_SIZE)
 #define KVM_VIRTIO_MMIO_AREA	ARM_MMIO_AREA
 
+/*
+ * On a GICv3 there must be one redistributor per vCPU.
+ * The value here is the size for one, we multiply this at runtime with
+ * the number of requested vCPUs to get the actual size.
+ */
+#define ARM_GIC_REDIST_SIZE	0x20000
+
 #define KVM_IRQ_OFFSET		GIC_SPI_IRQ_BASE
 
 #define KVM_VM_TYPE		0
@@ -45,9 +50,14 @@ static inline bool arm_addr_in_ioport_region(u64 phys_addr)
 	return phys_addr >= KVM_IOPORT_AREA && phys_addr < limit;
 }
 
-static inline bool arm_addr_in_virtio_mmio_region(u64 phys_addr)
+static inline bool arm_addr_in_virtio_mmio_region(int nr_redists, u64 phys_addr)
 {
-	u64 limit = KVM_VIRTIO_MMIO_AREA + ARM_VIRTIO_MMIO_SIZE;
+	u64 limit = ARM_AXI_AREA - ARM_GIC_DIST_SIZE;
+
+	if (nr_redists)
+		limit -= ARM_GIC_REDIST_SIZE * nr_redists;
+	else
+		limit -= ARM_GIC_CPUI_SIZE;
 	return phys_addr >= KVM_VIRTIO_MMIO_AREA && phys_addr < limit;
 }
 
diff --git a/tools/kvm/arm/kvm-cpu.c b/tools/kvm/arm/kvm-cpu.c
index ab08815..a3344fa 100644
--- a/tools/kvm/arm/kvm-cpu.c
+++ b/tools/kvm/arm/kvm-cpu.c
@@ -142,7 +142,9 @@ bool kvm_cpu__handle_exit(struct kvm_cpu *vcpu)
 bool kvm_cpu__emulate_mmio(struct kvm_cpu *vcpu, u64 phys_addr, u8 *data,
 			   u32 len, u8 is_write)
 {
-	if (arm_addr_in_virtio_mmio_region(phys_addr)) {
+	int nr_redists = 0;
+
+	if (arm_addr_in_virtio_mmio_region(nr_redists, phys_addr)) {
 		return kvm__emulate_mmio(vcpu, phys_addr, data, len, is_write);
 	} else if (arm_addr_in_ioport_region(phys_addr)) {
 		int direction = is_write ? KVM_EXIT_IO_OUT : KVM_EXIT_IO_IN;
-- 
1.7.9.5

[PATCH 11/11] kvmtool: add command line parameter to instantiate a vGICv3

[Andre Przywara]

Add the command line parameter "--gicv3" to request GICv3 emulation
in the kernel. Connect that to the already existing GICv3 code.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 tools/kvm/arm/aarch64/arm-cpu.c                    |    5 ++++-
 .../kvm/arm/aarch64/include/kvm/kvm-config-arch.h  |    4 +++-
 tools/kvm/arm/gic.c                                |   14 ++++++++++++++
 tools/kvm/arm/include/arm-common/kvm-config-arch.h |    1 +
 tools/kvm/arm/kvm-cpu.c                            |    2 +-
 tools/kvm/arm/kvm.c                                |    3 ++-
 6 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/tools/kvm/arm/aarch64/arm-cpu.c b/tools/kvm/arm/aarch64/arm-cpu.c
index a70d6bb..46d6d6a 100644
--- a/tools/kvm/arm/aarch64/arm-cpu.c
+++ b/tools/kvm/arm/aarch64/arm-cpu.c
@@ -12,7 +12,10 @@
 static void generate_fdt_nodes(void *fdt, struct kvm *kvm, u32 gic_phandle)
 {
 	int timer_interrupts[4] = {13, 14, 11, 10};
-	gic__generate_fdt_nodes(fdt, gic_phandle, KVM_DEV_TYPE_ARM_VGIC_V2);
+	gic__generate_fdt_nodes(fdt, gic_phandle,
+				kvm->cfg.arch.gicv3 ?
+					KVM_DEV_TYPE_ARM_VGIC_V3 :
+					KVM_DEV_TYPE_ARM_VGIC_V2);
 	timer__generate_fdt_nodes(fdt, kvm, timer_interrupts);
 }
 
diff --git a/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h b/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
index 89860ae..106e52f 100644
--- a/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
+++ b/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
@@ -3,7 +3,9 @@
 
 #define ARM_OPT_ARCH_RUN(cfg)						\
 	OPT_BOOLEAN('\0', "aarch32", &(cfg)->aarch32_guest,		\
-			"Run AArch32 guest"),
+			"Run AArch32 guest"),				\
+	OPT_BOOLEAN('\0', "gicv3", &(cfg)->gicv3,			\
+			"Use a GICv3 interrupt controller in the guest"),
 
 #include "arm-common/kvm-config-arch.h"
 
diff --git a/tools/kvm/arm/gic.c b/tools/kvm/arm/gic.c
index 9435715..3825fab 100644
--- a/tools/kvm/arm/gic.c
+++ b/tools/kvm/arm/gic.c
@@ -95,6 +95,17 @@ static int gic__create_irqchip(struct kvm *kvm)
 	return err;
 }
 
+static int gicv3__create_irqchip(struct kvm *kvm)
+{
+	if (kvm->nrcpus > 255) {
+		pr_warning("%d CPUS greater than maximum of %d -- truncating\n",
+				kvm->nrcpus, 255);
+		kvm->nrcpus = 255;
+	}
+
+	return gic__create_device(kvm, KVM_DEV_TYPE_ARM_VGIC_V3);
+}
+
 static int gicv2__create_irqchip(struct kvm *kvm)
 {
 	int err;
@@ -118,6 +129,9 @@ int gic__create(struct kvm *kvm, u32 type)
 	switch (type) {
 	case KVM_DEV_TYPE_ARM_VGIC_V2:
 		return gicv2__create_irqchip(kvm);
+	case KVM_DEV_TYPE_ARM_VGIC_V3:
+		nr_redists = kvm->cfg.nrcpus;
+		return gicv3__create_irqchip(kvm);
 	}
 	return -ENODEV;
 }
diff --git a/tools/kvm/arm/include/arm-common/kvm-config-arch.h b/tools/kvm/arm/include/arm-common/kvm-config-arch.h
index a8ebd94..0c27f88 100644
--- a/tools/kvm/arm/include/arm-common/kvm-config-arch.h
+++ b/tools/kvm/arm/include/arm-common/kvm-config-arch.h
@@ -8,6 +8,7 @@ struct kvm_config_arch {
 	unsigned int	force_cntfrq;
 	bool		virtio_trans_pci;
 	bool		aarch32_guest;
+	bool		gicv3;
 };
 
 #define OPT_ARCH_RUN(pfx, cfg)							\
diff --git a/tools/kvm/arm/kvm-cpu.c b/tools/kvm/arm/kvm-cpu.c
index a3344fa..f94fd6e 100644
--- a/tools/kvm/arm/kvm-cpu.c
+++ b/tools/kvm/arm/kvm-cpu.c
@@ -142,7 +142,7 @@ bool kvm_cpu__handle_exit(struct kvm_cpu *vcpu)
 bool kvm_cpu__emulate_mmio(struct kvm_cpu *vcpu, u64 phys_addr, u8 *data,
 			   u32 len, u8 is_write)
 {
-	int nr_redists = 0;
+	int nr_redists = vcpu->kvm->cfg.arch.gicv3 ? vcpu->kvm->nrcpus : 0;
 
 	if (arm_addr_in_virtio_mmio_region(nr_redists, phys_addr)) {
 		return kvm__emulate_mmio(vcpu, phys_addr, data, len, is_write);
diff --git a/tools/kvm/arm/kvm.c b/tools/kvm/arm/kvm.c
index 8d31fd7..2bccea5 100644
--- a/tools/kvm/arm/kvm.c
+++ b/tools/kvm/arm/kvm.c
@@ -82,6 +82,7 @@ void kvm__arch_init(struct kvm *kvm, const char *hugetlbfs_path, u64 ram_size)
 		MADV_MERGEABLE | MADV_HUGEPAGE);
 
 	/* Create the virtual GIC. */
-	if (gic__create(kvm, KVM_DEV_TYPE_ARM_VGIC_V2))
+	if (gic__create(kvm, kvm->cfg.arch.gicv3 ?
+			KVM_DEV_TYPE_ARM_VGIC_V3 : KVM_DEV_TYPE_ARM_VGIC_V2))
 		die("Failed to create virtual GIC");
 }
-- 
1.7.9.5

Re: [PATCH 03/11] kvmtool: AArch{32,64}: use KVM_CREATE_DEVICE & co to instanciate the GIC

[Will Deacon]

On Fri, Jan 23, 2015 at 04:35:02PM +0000, Andre Przywara wrote:
> From: Marc Zyngier <marc.zyngier@arm.com>
> 
> As of 3.14, KVM/arm supports the creation/configuration of the GIC through
> a more generic device API, which is now the preferred way to do so.
> 
> Plumb the new API in, and allow the old code to be used as a fallback.
> 
> [Andre: Rename some functions on the way to differentiate between
> creation and initialisation more clearly.]
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  tools/kvm/arm/gic.c                    |   60 ++++++++++++++++++++++++++++----
>  tools/kvm/arm/include/arm-common/gic.h |    2 +-
>  tools/kvm/arm/kvm.c                    |    6 ++--
>  3 files changed, 57 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/kvm/arm/gic.c b/tools/kvm/arm/gic.c
> index 5d8cbe6..ce5f7fa 100644
> --- a/tools/kvm/arm/gic.c
> +++ b/tools/kvm/arm/gic.c
> @@ -7,7 +7,41 @@
>  #include <linux/byteorder.h>
>  #include <linux/kvm.h>
>  
> -int gic__init_irqchip(struct kvm *kvm)
> +static int gic_fd = -1;
> +
> +static int gic__create_device(struct kvm *kvm)
> +{
> +	int err;
> +	u64 cpu_if_addr = ARM_GIC_CPUI_BASE;
> +	u64 dist_addr = ARM_GIC_DIST_BASE;
> +	struct kvm_create_device gic_device = {
> +		.type	= KVM_DEV_TYPE_ARM_VGIC_V2,
> +	};
> +	struct kvm_device_attr cpu_if_attr = {
> +		.group	= KVM_DEV_ARM_VGIC_GRP_ADDR,
> +		.attr	= KVM_VGIC_V2_ADDR_TYPE_CPU,
> +		.addr	= (u64)(unsigned long)&cpu_if_addr,
> +	};
> +	struct kvm_device_attr dist_attr = {
> +		.group	= KVM_DEV_ARM_VGIC_GRP_ADDR,
> +		.attr	= KVM_VGIC_V2_ADDR_TYPE_DIST,
> +		.addr	= (u64)(unsigned long)&dist_addr,
> +	};
> +
> +	err = ioctl(kvm->vm_fd, KVM_CREATE_DEVICE, &gic_device);
> +	if (err)
> +		return err;
> +
> +	gic_fd = gic_device.fd;
> +
> +	err = ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &cpu_if_attr);
> +	if (err)
> +		return err;
> +
> +	return ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &dist_attr);
> +}
> +
> +static int gic__create_irqchip(struct kvm *kvm)
>  {
>  	int err;
>  	struct kvm_arm_device_addr gic_addr[] = {
> @@ -23,12 +57,6 @@ int gic__init_irqchip(struct kvm *kvm)
>  		}
>  	};
>  
> -	if (kvm->nrcpus > GIC_MAX_CPUS) {
> -		pr_warning("%d CPUS greater than maximum of %d -- truncating\n",
> -				kvm->nrcpus, GIC_MAX_CPUS);
> -		kvm->nrcpus = GIC_MAX_CPUS;
> -	}
> -
>  	err = ioctl(kvm->vm_fd, KVM_CREATE_IRQCHIP);
>  	if (err)
>  		return err;
> @@ -41,6 +69,24 @@ int gic__init_irqchip(struct kvm *kvm)
>  	return err;
>  }
>  
> +int gic__create(struct kvm *kvm)
> +{
> +	int err;
> +
> +	if (kvm->nrcpus > GIC_MAX_CPUS) {
> +		pr_warning("%d CPUS greater than maximum of %d -- truncating\n",
> +				kvm->nrcpus, GIC_MAX_CPUS);
> +		kvm->nrcpus = GIC_MAX_CPUS;
> +	}
> +
> +	/* Try the new way first, and fallback on legacy method otherwise */
> +	err = gic__create_device(kvm);
> +	if (err)
> +		err = gic__create_irqchip(kvm);

This fallback doesn't look safe to me:

  - gic_fd might remain initialised
  - What does the kernel vgic driver do if you've already done
    a successful KVM_CREATE_DEVICE and then try to use the legacy method?

Will

Re: [PATCH 11/11] kvmtool: add command line parameter to instantiate a vGICv3

[Will Deacon]

On Fri, Jan 23, 2015 at 04:35:10PM +0000, Andre Przywara wrote:
> Add the command line parameter "--gicv3" to request GICv3 emulation
> in the kernel. Connect that to the already existing GICv3 code.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  tools/kvm/arm/aarch64/arm-cpu.c                    |    5 ++++-
>  .../kvm/arm/aarch64/include/kvm/kvm-config-arch.h  |    4 +++-
>  tools/kvm/arm/gic.c                                |   14 ++++++++++++++
>  tools/kvm/arm/include/arm-common/kvm-config-arch.h |    1 +
>  tools/kvm/arm/kvm-cpu.c                            |    2 +-
>  tools/kvm/arm/kvm.c                                |    3 ++-
>  6 files changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/kvm/arm/aarch64/arm-cpu.c b/tools/kvm/arm/aarch64/arm-cpu.c
> index a70d6bb..46d6d6a 100644
> --- a/tools/kvm/arm/aarch64/arm-cpu.c
> +++ b/tools/kvm/arm/aarch64/arm-cpu.c
> @@ -12,7 +12,10 @@
>  static void generate_fdt_nodes(void *fdt, struct kvm *kvm, u32 gic_phandle)
>  {
>  	int timer_interrupts[4] = {13, 14, 11, 10};
> -	gic__generate_fdt_nodes(fdt, gic_phandle, KVM_DEV_TYPE_ARM_VGIC_V2);
> +	gic__generate_fdt_nodes(fdt, gic_phandle,
> +				kvm->cfg.arch.gicv3 ?
> +					KVM_DEV_TYPE_ARM_VGIC_V3 :
> +					KVM_DEV_TYPE_ARM_VGIC_V2);
>  	timer__generate_fdt_nodes(fdt, kvm, timer_interrupts);
>  }
>  
> diff --git a/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h b/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
> index 89860ae..106e52f 100644
> --- a/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
> +++ b/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
> @@ -3,7 +3,9 @@
>  
>  #define ARM_OPT_ARCH_RUN(cfg)						\
>  	OPT_BOOLEAN('\0', "aarch32", &(cfg)->aarch32_guest,		\
> -			"Run AArch32 guest"),
> +			"Run AArch32 guest"),				\
> +	OPT_BOOLEAN('\0', "gicv3", &(cfg)->gicv3,			\
> +			"Use a GICv3 interrupt controller in the guest"),

On a GICv3-capable system, why would I *not* want to enable this option?
In other words, could we make this the default behaviour on systems that
support it, and if you need an override then it should be something like
--force-gicv2.

Or am I missing a key piece of the puzzle?

Will

Re: [PATCH 11/11] kvmtool: add command line parameter to instantiate a vGICv3

[Andre Przywara]

Hi Will,

On 26/01/15 11:30, Will Deacon wrote:
> On Fri, Jan 23, 2015 at 04:35:10PM +0000, Andre Przywara wrote:
>> Add the command line parameter "--gicv3" to request GICv3 emulation
>> in the kernel. Connect that to the already existing GICv3 code.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>  tools/kvm/arm/aarch64/arm-cpu.c                    |    5 ++++-
>>  .../kvm/arm/aarch64/include/kvm/kvm-config-arch.h  |    4 +++-
>>  tools/kvm/arm/gic.c                                |   14 ++++++++++++++
>>  tools/kvm/arm/include/arm-common/kvm-config-arch.h |    1 +
>>  tools/kvm/arm/kvm-cpu.c                            |    2 +-
>>  tools/kvm/arm/kvm.c                                |    3 ++-
>>  6 files changed, 25 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/kvm/arm/aarch64/arm-cpu.c b/tools/kvm/arm/aarch64/arm-cpu.c
>> index a70d6bb..46d6d6a 100644
>> --- a/tools/kvm/arm/aarch64/arm-cpu.c
>> +++ b/tools/kvm/arm/aarch64/arm-cpu.c
>> @@ -12,7 +12,10 @@
>>  static void generate_fdt_nodes(void *fdt, struct kvm *kvm, u32 gic_phandle)
>>  {
>>  	int timer_interrupts[4] = {13, 14, 11, 10};
>> -	gic__generate_fdt_nodes(fdt, gic_phandle, KVM_DEV_TYPE_ARM_VGIC_V2);
>> +	gic__generate_fdt_nodes(fdt, gic_phandle,
>> +				kvm->cfg.arch.gicv3 ?
>> +					KVM_DEV_TYPE_ARM_VGIC_V3 :
>> +					KVM_DEV_TYPE_ARM_VGIC_V2);
>>  	timer__generate_fdt_nodes(fdt, kvm, timer_interrupts);
>>  }
>>  
>> diff --git a/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h b/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
>> index 89860ae..106e52f 100644
>> --- a/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
>> +++ b/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
>> @@ -3,7 +3,9 @@
>>  
>>  #define ARM_OPT_ARCH_RUN(cfg)						\
>>  	OPT_BOOLEAN('\0', "aarch32", &(cfg)->aarch32_guest,		\
>> -			"Run AArch32 guest"),
>> +			"Run AArch32 guest"),				\
>> +	OPT_BOOLEAN('\0', "gicv3", &(cfg)->gicv3,			\
>> +			"Use a GICv3 interrupt controller in the guest"),
> 
> On a GICv3-capable system, why would I *not* want to enable this option?
> In other words, could we make this the default behaviour on systems that
> support it, and if you need an override then it should be something like
> --force-gicv2.

Well, you could have a guest kernel < 3.17, which does not have GICv3
support. In general I consider GICv2 better tested, so I reckon that
people will only want to use GICv3 emulation if there is a need for it
(non-compat GICv3 host or more than 8 VCPUs in the guest). That probably
changes over time, but for the time being I'd better keep the default at
GICv2 emulation.

Having said that, there could be a fallback in case GICv2 emulation is
not available. Let me take a look at that.
Also thinking about the future (ITS emulation) I found that I'd like to
replace this option with something more generic like --irqchip=.

Cheers,
Andre.

Re: [PATCH 11/11] kvmtool: add command line parameter to instantiate a vGICv3

[Marc Zyngier]

On 26/01/15 11:43, Andre Przywara wrote:
> Hi Will,
> 
> On 26/01/15 11:30, Will Deacon wrote:
>> On Fri, Jan 23, 2015 at 04:35:10PM +0000, Andre Przywara wrote:
>>> Add the command line parameter "--gicv3" to request GICv3 emulation
>>> in the kernel. Connect that to the already existing GICv3 code.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>> ---
>>>  tools/kvm/arm/aarch64/arm-cpu.c                    |    5 ++++-
>>>  .../kvm/arm/aarch64/include/kvm/kvm-config-arch.h  |    4 +++-
>>>  tools/kvm/arm/gic.c                                |   14 ++++++++++++++
>>>  tools/kvm/arm/include/arm-common/kvm-config-arch.h |    1 +
>>>  tools/kvm/arm/kvm-cpu.c                            |    2 +-
>>>  tools/kvm/arm/kvm.c                                |    3 ++-
>>>  6 files changed, 25 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tools/kvm/arm/aarch64/arm-cpu.c b/tools/kvm/arm/aarch64/arm-cpu.c
>>> index a70d6bb..46d6d6a 100644
>>> --- a/tools/kvm/arm/aarch64/arm-cpu.c
>>> +++ b/tools/kvm/arm/aarch64/arm-cpu.c
>>> @@ -12,7 +12,10 @@
>>>  static void generate_fdt_nodes(void *fdt, struct kvm *kvm, u32 gic_phandle)
>>>  {
>>>  	int timer_interrupts[4] = {13, 14, 11, 10};
>>> -	gic__generate_fdt_nodes(fdt, gic_phandle, KVM_DEV_TYPE_ARM_VGIC_V2);
>>> +	gic__generate_fdt_nodes(fdt, gic_phandle,
>>> +				kvm->cfg.arch.gicv3 ?
>>> +					KVM_DEV_TYPE_ARM_VGIC_V3 :
>>> +					KVM_DEV_TYPE_ARM_VGIC_V2);
>>>  	timer__generate_fdt_nodes(fdt, kvm, timer_interrupts);
>>>  }
>>>  
>>> diff --git a/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h b/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
>>> index 89860ae..106e52f 100644
>>> --- a/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
>>> +++ b/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
>>> @@ -3,7 +3,9 @@
>>>  
>>>  #define ARM_OPT_ARCH_RUN(cfg)						\
>>>  	OPT_BOOLEAN('\0', "aarch32", &(cfg)->aarch32_guest,		\
>>> -			"Run AArch32 guest"),
>>> +			"Run AArch32 guest"),				\
>>> +	OPT_BOOLEAN('\0', "gicv3", &(cfg)->gicv3,			\
>>> +			"Use a GICv3 interrupt controller in the guest"),
>>
>> On a GICv3-capable system, why would I *not* want to enable this option?
>> In other words, could we make this the default behaviour on systems that
>> support it, and if you need an override then it should be something like
>> --force-gicv2.
> 
> Well, you could have a guest kernel < 3.17, which does not have GICv3
> support. In general I consider GICv2 better tested, so I reckon that
> people will only want to use GICv3 emulation if there is a need for it
> (non-compat GICv3 host or more than 8 VCPUs in the guest). That probably
> changes over time, but for the time being I'd better keep the default at
> GICv2 emulation.

I think there is slightly more to it. You want the same command-line
options to give you the same result on different platform (provided that
the HW is available, see below). Changing the default depending on the
platform you're is not very good for reproducibility.

> Having said that, there could be a fallback in case GICv2 emulation is
> not available. Let me take a look at that.

You could try and pick a GICv3 emulation if v2 is not available, and
probably print a warning in that case.

> Also thinking about the future (ITS emulation) I found that I'd like to
> replace this option with something more generic like --irqchip=.

That's an orthogonal issue, but yes, this is probably better.

Thanks,

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

Re: [PATCH 03/11] kvmtool: AArch{32,64}: use KVM_CREATE_DEVICE & co to instanciate the GIC

[Andre Przywara]

On 26/01/15 11:26, Will Deacon wrote:
> On Fri, Jan 23, 2015 at 04:35:02PM +0000, Andre Przywara wrote:
>> From: Marc Zyngier <marc.zyngier@arm.com>
>>
>> As of 3.14, KVM/arm supports the creation/configuration of the GIC through
>> a more generic device API, which is now the preferred way to do so.
>>
>> Plumb the new API in, and allow the old code to be used as a fallback.
>>
>> [Andre: Rename some functions on the way to differentiate between
>> creation and initialisation more clearly.]
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>  tools/kvm/arm/gic.c                    |   60 ++++++++++++++++++++++++++++----
>>  tools/kvm/arm/include/arm-common/gic.h |    2 +-
>>  tools/kvm/arm/kvm.c                    |    6 ++--
>>  3 files changed, 57 insertions(+), 11 deletions(-)
>>
>> diff --git a/tools/kvm/arm/gic.c b/tools/kvm/arm/gic.c
>> index 5d8cbe6..ce5f7fa 100644
>> --- a/tools/kvm/arm/gic.c
>> +++ b/tools/kvm/arm/gic.c
>> @@ -7,7 +7,41 @@
>>  #include <linux/byteorder.h>
>>  #include <linux/kvm.h>
>>  
>> -int gic__init_irqchip(struct kvm *kvm)
>> +static int gic_fd = -1;
>> +
>> +static int gic__create_device(struct kvm *kvm)
>> +{
>> +	int err;
>> +	u64 cpu_if_addr = ARM_GIC_CPUI_BASE;
>> +	u64 dist_addr = ARM_GIC_DIST_BASE;
>> +	struct kvm_create_device gic_device = {
>> +		.type	= KVM_DEV_TYPE_ARM_VGIC_V2,
>> +	};
>> +	struct kvm_device_attr cpu_if_attr = {
>> +		.group	= KVM_DEV_ARM_VGIC_GRP_ADDR,
>> +		.attr	= KVM_VGIC_V2_ADDR_TYPE_CPU,
>> +		.addr	= (u64)(unsigned long)&cpu_if_addr,
>> +	};
>> +	struct kvm_device_attr dist_attr = {
>> +		.group	= KVM_DEV_ARM_VGIC_GRP_ADDR,
>> +		.attr	= KVM_VGIC_V2_ADDR_TYPE_DIST,
>> +		.addr	= (u64)(unsigned long)&dist_addr,
>> +	};
>> +
>> +	err = ioctl(kvm->vm_fd, KVM_CREATE_DEVICE, &gic_device);
>> +	if (err)
>> +		return err;
>> +
>> +	gic_fd = gic_device.fd;
>> +
>> +	err = ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &cpu_if_attr);
>> +	if (err)
>> +		return err;
>> +
>> +	return ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &dist_attr);
>> +}
>> +
>> +static int gic__create_irqchip(struct kvm *kvm)
>>  {
>>  	int err;
>>  	struct kvm_arm_device_addr gic_addr[] = {
>> @@ -23,12 +57,6 @@ int gic__init_irqchip(struct kvm *kvm)
>>  		}
>>  	};
>>  
>> -	if (kvm->nrcpus > GIC_MAX_CPUS) {
>> -		pr_warning("%d CPUS greater than maximum of %d -- truncating\n",
>> -				kvm->nrcpus, GIC_MAX_CPUS);
>> -		kvm->nrcpus = GIC_MAX_CPUS;
>> -	}
>> -
>>  	err = ioctl(kvm->vm_fd, KVM_CREATE_IRQCHIP);
>>  	if (err)
>>  		return err;
>> @@ -41,6 +69,24 @@ int gic__init_irqchip(struct kvm *kvm)
>>  	return err;
>>  }
>>  
>> +int gic__create(struct kvm *kvm)
>> +{
>> +	int err;
>> +
>> +	if (kvm->nrcpus > GIC_MAX_CPUS) {
>> +		pr_warning("%d CPUS greater than maximum of %d -- truncating\n",
>> +				kvm->nrcpus, GIC_MAX_CPUS);
>> +		kvm->nrcpus = GIC_MAX_CPUS;
>> +	}
>> +
>> +	/* Try the new way first, and fallback on legacy method otherwise */
>> +	err = gic__create_device(kvm);
>> +	if (err)
>> +		err = gic__create_irqchip(kvm);
> 
> This fallback doesn't look safe to me:
> 
>   - gic_fd might remain initialised
>   - What does the kernel vgic driver do if you've already done
>     a successful KVM_CREATE_DEVICE and then try to use the legacy method?

Good point. I think we need to cleanup the device by closing the fd (and
resetting the variable to -1) in case any of the subsequent ioctls
return with an error (e.g. due to unaligned addresses).
I have to check what happens in the kernel in that case, though.

Cheers,
Andre.