[PATCH v8 00/16] kvmtool: arm: ITS emulation and GSI routing support

[PATCH v8 00/16] kvmtool: arm: ITS emulation and GSI routing support

[Andre Przywara]

Hi,

this series teaches kvmtool how to support KVM's ITS emulation. Also
(as this is somewhat related and has been co-developed) it enables GSI
routing for ARM/ARM64, which allows IRQFDs to be used, for instance
to use vhost_net. At the moment this is dependent on the guest
using the ITS emulation, but GICv2M support may be added at a later time.

The first six patches are generic fixes and refactoring to pave the
road for the rest of the patches. Most importantly patch 3/16 pulls
the GSI routing code from x86 into generic code.
The following six patches add ITS emulation support. They reserve and
register the required ITS register frame and populate a DT node with
the necessary data. Also the patches add the device ID to the
KVM_SIGNAL_MSI ioctl.
Patches 13 and 14 enable IRQ GSI routing for ARM/ARM64.
This is needed to use IRQFDs, which is a prerequisite for vhost
functionality, for instance. The code sets up the (dummy) SPI
routing table and adds the device ID to the routing entry.
The last two patches enables the guest ITS support by extending the
existing --irqchip= parameter to allow "--irqchip=gicv3-its", both
for ARM64 and ARM (kernel support for this is still pending).

These patches make use of the recently (4.8-rc) merged KVM kernel
functionality, both Eric's IRQ routing series and the ITS emulation.
It can also be found in my kvmtool git repository [1].

Compared to v7 of the series this is mostly bugfixing and updates,
also adds the 32-bit support provided by Vladimir.

Cheers,
Andre.

[1] git://linux-arm.org/kvmtool.git (branch: its/v8)
    http://www.linux-arm.org/git?p=kvmtool.git;a=log;h=refs/heads/its/v8

Andre Przywara (15):
  FDT: introduce global phandle allocation
  arm: use new phandle allocation functions
  irq: move IRQ routing into irq.c
  MSI-X: update GSI routing after changed MSI-X configuration
  virtio: fix endianness check for vhost support
  PCI: Only allocate IRQ routing entry when available
  update public Linux headers for GICv3 ITS emulation
  arm: gic: allow 32-bit compilation
  arm: allow creation of an MSI register frame region
  arm: FDT: create MSI controller DT node
  add kvm__check_vm_capability
  PCI: inject PCI device ID on MSI injection
  arm: setup SPI IRQ routing tables
  extend GSI IRQ routing to take a device ID
  arm64: enable GICv3-ITS emulation

Vladimir Murzin (1):
  arm: add support for vGICv3 and vITS

 Makefile                                 |   5 +-
 arm/aarch32/arm-cpu.c                    |   4 +-
 arm/aarch32/include/asm/kvm.h            |   4 +-
 arm/aarch32/include/kvm/kvm-arch.h       |   3 -
 arm/aarch64/arm-cpu.c                    |   5 +-
 arm/aarch64/include/asm/kvm.h            |   2 +
 arm/aarch64/include/kvm/kvm-arch.h       |   3 -
 arm/fdt.c                                |   8 +-
 arm/gic.c                                | 132 ++++++++++++++++++++++++++++-
 arm/include/arm-common/gic.h             |   3 +-
 arm/include/arm-common/kvm-arch.h        |   2 +
 arm/include/arm-common/kvm-config-arch.h |   2 +-
 arm/include/arm-common/kvm-cpu-arch.h    |   3 +-
 arm/include/arm-common/pci.h             |   2 +-
 arm/irq.c                                |   9 --
 arm/pci.c                                |  13 ++-
 hw/pci-shmem.c                           |   5 +-
 include/kvm/fdt.h                        |  10 +--
 include/kvm/irq.h                        |   8 +-
 include/kvm/kvm.h                        |   1 +
 include/kvm/virtio.h                     |   9 +-
 include/linux/kvm.h                      |  19 ++++-
 irq.c                                    | 138 +++++++++++++++++++++++++++++++
 kvm-fdt.c                                |  26 ++++++
 kvm.c                                    |  28 +++++++
 mips/irq.c                               |  10 ---
 powerpc/irq.c                            |  31 -------
 virtio/net.c                             |   2 +-
 virtio/pci.c                             |  87 ++++++++++++++++---
 x86/include/asm/kvm.h                    |   6 +-
 x86/irq.c                                |  45 ++--------
 31 files changed, 480 insertions(+), 145 deletions(-)
 delete mode 100644 arm/irq.c
 create mode 100644 kvm-fdt.c
 delete mode 100644 mips/irq.c
 delete mode 100644 powerpc/irq.c

-- 
2.9.0

[PATCH v8 01/16] FDT: introduce global phandle allocation

[Andre Przywara]

Allocating an FDT phandle (a unique identifier) using a static
variable in a static inline function defined in a header file works
only if all users are in the same source file. So trying to allocate
a handle from two different compilation units fails.
Introduce global phandle allocation and reference code to properly
allocate unique phandles.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 Makefile          |  1 +
 arm/fdt.c         |  2 +-
 arm/gic.c         |  2 +-
 include/kvm/fdt.h | 10 +++++-----
 kvm-fdt.c         | 26 ++++++++++++++++++++++++++
 5 files changed, 34 insertions(+), 7 deletions(-)
 create mode 100644 kvm-fdt.c

diff --git a/Makefile b/Makefile
index 1f0196f..e4a4002 100644
--- a/Makefile
+++ b/Makefile
@@ -98,6 +98,7 @@ OBJS	+= kvm-ipc.o
 OBJS	+= builtin-sandbox.o
 OBJS	+= virtio/mmio.o
 OBJS	+= hw/i8042.o
+OBJS	+= kvm-fdt.o
 
 # Translate uname -m into ARCH string
 ARCH ?= $(shell uname -m | sed -e s/i.86/i386/ -e s/ppc.*/powerpc/ \
diff --git a/arm/fdt.c b/arm/fdt.c
index 381d48f..8bcfffb 100644
--- a/arm/fdt.c
+++ b/arm/fdt.c
@@ -114,7 +114,7 @@ static int setup_fdt(struct kvm *kvm)
 {
 	struct device_header *dev_hdr;
 	u8 staging_fdt[FDT_MAX_SIZE];
-	u32 gic_phandle		= fdt__alloc_phandle();
+	u32 gic_phandle		= fdt__get_phandle(PHANDLE_GIC);
 	u64 mem_reg_prop[]	= {
 		cpu_to_fdt64(kvm->arch.memory_guest_start),
 		cpu_to_fdt64(kvm->ram_size),
diff --git a/arm/gic.c b/arm/gic.c
index d6d6dd0..b60437e 100644
--- a/arm/gic.c
+++ b/arm/gic.c
@@ -222,7 +222,7 @@ void gic__generate_fdt_nodes(void *fdt, u32 phandle, enum irqchip_type type)
 	_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)));
-	_FDT(fdt_property_cell(fdt, "phandle", phandle));
+	_FDT(fdt_property_cell(fdt, "phandle", fdt__get_phandle(PHANDLE_GIC)));
 	_FDT(fdt_end_node(fdt));
 }
 
diff --git a/include/kvm/fdt.h b/include/kvm/fdt.h
index 53d85a4..cd2bb72 100644
--- a/include/kvm/fdt.h
+++ b/include/kvm/fdt.h
@@ -8,6 +8,10 @@
 #include <linux/types.h>
 
 #define FDT_MAX_SIZE	0x10000
+#define FDT_INVALID_PHANDLE 0
+#define FDT_IS_VALID_PHANDLE(phandle) ((phandle) != FDT_INVALID_PHANDLE)
+
+enum phandles {PHANDLE_GIC, PHANDLES_MAX};
 
 /* Those definitions are generic FDT values for specifying IRQ
  * types and are used in the Linux kernel internally as well as in
@@ -33,10 +37,6 @@ enum irq_type {
 		}							\
 	} while (0)
 
-static inline u32 fdt__alloc_phandle(void)
-{
-	static u32 phandle = 0;
-	return ++phandle;
-}
+u32 fdt__get_phandle(enum phandles phandle);
 
 #endif /* KVM__FDT_H */
diff --git a/kvm-fdt.c b/kvm-fdt.c
new file mode 100644
index 0000000..d05f3fe
--- /dev/null
+++ b/kvm-fdt.c
@@ -0,0 +1,26 @@
+/*
+ * Commonly used FDT functions.
+ */
+
+#include <stdio.h>
+#include "kvm/fdt.h"
+#include "kvm/util.h"
+
+u32 phandles[PHANDLES_MAX] = {};
+u32 next_phandle = 1;
+
+u32 fdt__get_phandle(enum phandles phandle)
+{
+	u32 ret;
+
+	if (phandle >= PHANDLES_MAX)
+		return FDT_INVALID_PHANDLE;
+
+	ret = phandles[phandle];
+	if (ret == FDT_INVALID_PHANDLE) {
+		ret = next_phandle++;
+		phandles[phandle] = ret;
+	}
+
+	return ret;
+}
-- 
2.9.0

[PATCH v8 02/16] arm: use new phandle allocation functions

[Andre Przywara]

To refer to the GIC FDT node, we used to pass the GIC phandle to most
of the functions dealing with FDT nodes.
Since we now have a global phandle reference, use that to refer to the
GIC handle in various places and get rid of the now unneeded parameter
passing.

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

diff --git a/arm/aarch32/arm-cpu.c b/arm/aarch32/arm-cpu.c
index d8d6293..27a8e17 100644
--- a/arm/aarch32/arm-cpu.c
+++ b/arm/aarch32/arm-cpu.c
@@ -8,11 +8,11 @@
 #include <linux/byteorder.h>
 #include <linux/types.h>
 
-static void generate_fdt_nodes(void *fdt, struct kvm *kvm, u32 gic_phandle)
+static void generate_fdt_nodes(void *fdt, struct kvm *kvm)
 {
 	int timer_interrupts[4] = {13, 14, 11, 10};
 
-	gic__generate_fdt_nodes(fdt, gic_phandle, IRQCHIP_GICV2);
+	gic__generate_fdt_nodes(fdt, IRQCHIP_GICV2);
 	timer__generate_fdt_nodes(fdt, kvm, timer_interrupts);
 }
 
diff --git a/arm/aarch64/arm-cpu.c b/arm/aarch64/arm-cpu.c
index c21c0bb..d7572b7 100644
--- a/arm/aarch64/arm-cpu.c
+++ b/arm/aarch64/arm-cpu.c
@@ -10,10 +10,11 @@
 #include <linux/byteorder.h>
 #include <linux/types.h>
 
-static void generate_fdt_nodes(void *fdt, struct kvm *kvm, u32 gic_phandle)
+static void generate_fdt_nodes(void *fdt, struct kvm *kvm)
 {
 	int timer_interrupts[4] = {13, 14, 11, 10};
-	gic__generate_fdt_nodes(fdt, gic_phandle, kvm->cfg.arch.irqchip);
+
+	gic__generate_fdt_nodes(fdt, kvm->cfg.arch.irqchip);
 	timer__generate_fdt_nodes(fdt, kvm, timer_interrupts);
 	pmu__generate_fdt_nodes(fdt, kvm);
 }
diff --git a/arm/fdt.c b/arm/fdt.c
index 8bcfffb..f6c8a4c 100644
--- a/arm/fdt.c
+++ b/arm/fdt.c
@@ -125,7 +125,7 @@ static int setup_fdt(struct kvm *kvm)
 						     kvm->arch.dtb_guest_start);
 	void (*generate_mmio_fdt_nodes)(void *, struct device_header *,
 					void (*)(void *, u8, enum irq_type));
-	void (*generate_cpu_peripheral_fdt_nodes)(void *, struct kvm *, u32)
+	void (*generate_cpu_peripheral_fdt_nodes)(void *, struct kvm *)
 					= kvm->cpus[0]->generate_fdt_nodes;
 
 	/* Create new tree without a reserve map */
@@ -166,7 +166,7 @@ static int setup_fdt(struct kvm *kvm)
 	/* CPU and peripherals (interrupt controller, timers, etc) */
 	generate_cpu_nodes(fdt, kvm);
 	if (generate_cpu_peripheral_fdt_nodes)
-		generate_cpu_peripheral_fdt_nodes(fdt, kvm, gic_phandle);
+		generate_cpu_peripheral_fdt_nodes(fdt, kvm);
 
 	/* Virtio MMIO devices */
 	dev_hdr = device__first_dev(DEVICE_BUS_MMIO);
@@ -185,7 +185,7 @@ static int setup_fdt(struct kvm *kvm)
 	}
 
 	/* PCI host controller */
-	pci__generate_fdt_nodes(fdt, gic_phandle);
+	pci__generate_fdt_nodes(fdt);
 
 	/* PSCI firmware */
 	_FDT(fdt_begin_node(fdt, "psci"));
diff --git a/arm/gic.c b/arm/gic.c
index b60437e..2c1a547 100644
--- a/arm/gic.c
+++ b/arm/gic.c
@@ -194,7 +194,7 @@ static int gic__init_gic(struct kvm *kvm)
 }
 late_init(gic__init_gic)
 
-void gic__generate_fdt_nodes(void *fdt, u32 phandle, enum irqchip_type type)
+void gic__generate_fdt_nodes(void *fdt, enum irqchip_type type)
 {
 	const char *compatible;
 	u64 reg_prop[] = {
diff --git a/arm/include/arm-common/gic.h b/arm/include/arm-common/gic.h
index 4fde5ac..b43a180 100644
--- a/arm/include/arm-common/gic.h
+++ b/arm/include/arm-common/gic.h
@@ -30,6 +30,6 @@ struct kvm;
 
 int gic__alloc_irqnum(void);
 int gic__create(struct kvm *kvm, enum irqchip_type type);
-void gic__generate_fdt_nodes(void *fdt, u32 phandle, enum irqchip_type type);
+void gic__generate_fdt_nodes(void *fdt, enum irqchip_type type);
 
 #endif /* ARM_COMMON__GIC_H */
diff --git a/arm/include/arm-common/kvm-cpu-arch.h b/arm/include/arm-common/kvm-cpu-arch.h
index 8a6a6e7..923d2c4 100644
--- a/arm/include/arm-common/kvm-cpu-arch.h
+++ b/arm/include/arm-common/kvm-cpu-arch.h
@@ -25,8 +25,7 @@ struct kvm_cpu {
 
 	struct kvm_coalesced_mmio_ring	*ring;
 
-	void		(*generate_fdt_nodes)(void *fdt, struct kvm* kvm,
-					      u32 gic_phandle);
+	void		(*generate_fdt_nodes)(void *fdt, struct kvm* kvm);
 };
 
 struct kvm_arm_target {
diff --git a/arm/include/arm-common/pci.h b/arm/include/arm-common/pci.h
index ee87725..9008a0e 100644
--- a/arm/include/arm-common/pci.h
+++ b/arm/include/arm-common/pci.h
@@ -1,6 +1,6 @@
 #ifndef ARM_COMMON__PCI_H
 #define ARM_COMMON__PCI_H
 
-void pci__generate_fdt_nodes(void *fdt, u32 gic_phandle);
+void pci__generate_fdt_nodes(void *fdt);
 
 #endif /* ARM_COMMON__PCI_H */
diff --git a/arm/pci.c b/arm/pci.c
index 99a8130..9630657 100644
--- a/arm/pci.c
+++ b/arm/pci.c
@@ -21,11 +21,12 @@ struct of_interrupt_map_entry {
 	struct of_gic_irq		gic_irq;
 } __attribute__((packed));
 
-void pci__generate_fdt_nodes(void *fdt, u32 gic_phandle)
+void pci__generate_fdt_nodes(void *fdt)
 {
 	struct device_header *dev_hdr;
 	struct of_interrupt_map_entry irq_map[OF_PCI_IRQ_MAP_MAX];
 	unsigned nentries = 0;
+	u32 gic_phandle = fdt__get_phandle(PHANDLE_GIC);
 	/* Bus range */
 	u32 bus_range[] = { cpu_to_fdt32(0), cpu_to_fdt32(1), };
 	/* Configuration Space */
-- 
2.9.0

[PATCH v8 03/16] irq: move IRQ routing into irq.c

[Andre Przywara]

The current IRQ routing code in x86/irq.c is mostly implementing a
generic KVM interface which other architectures may use too.
Move the code to set up an MSI route into the generic irq.c file and
guard it with the KVM_CAP_IRQ_ROUTING capability to return an error
if the kernel does not support interrupt routing.
This also removes the dummy implementations for all other
architectures and only leaves the x86 specific code in x86/irq.c.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 Makefile          |  4 +--
 arm/irq.c         |  9 ------
 hw/pci-shmem.c    |  2 ++
 include/kvm/irq.h |  5 ++++
 irq.c             | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 mips/irq.c        | 10 -------
 powerpc/irq.c     | 31 --------------------
 virtio/pci.c      | 21 +++++++++-----
 x86/irq.c         | 45 ++++------------------------
 9 files changed, 114 insertions(+), 100 deletions(-)
 delete mode 100644 arm/irq.c
 delete mode 100644 mips/irq.c
 delete mode 100644 powerpc/irq.c

diff --git a/Makefile b/Makefile
index e4a4002..8ca887f 100644
--- a/Makefile
+++ b/Makefile
@@ -138,7 +138,6 @@ ifeq ($(ARCH), powerpc)
 	DEFINES += -DCONFIG_PPC
 	OBJS	+= powerpc/boot.o
 	OBJS	+= powerpc/ioport.o
-	OBJS	+= powerpc/irq.o
 	OBJS	+= powerpc/kvm.o
 	OBJS	+= powerpc/cpu_info.o
 	OBJS	+= powerpc/kvm-cpu.o
@@ -153,7 +152,7 @@ ifeq ($(ARCH), powerpc)
 endif
 
 # ARM
-OBJS_ARM_COMMON		:= arm/fdt.o arm/gic.o arm/ioport.o arm/irq.o \
+OBJS_ARM_COMMON		:= arm/fdt.o arm/gic.o arm/ioport.o \
 			   arm/kvm.o arm/kvm-cpu.o arm/pci.o arm/timer.o \
 			   arm/pmu.o
 HDRS_ARM_COMMON		:= arm/include
@@ -186,7 +185,6 @@ ifeq ($(ARCH),mips)
 	ARCH_INCLUDE	:= mips/include
 	OBJS		+= mips/kvm.o
 	OBJS		+= mips/kvm-cpu.o
-	OBJS		+= mips/irq.o
 endif
 ###
 
diff --git a/arm/irq.c b/arm/irq.c
deleted file mode 100644
index d8f44df..0000000
--- a/arm/irq.c
+++ /dev/null
@@ -1,9 +0,0 @@
-#include "kvm/irq.h"
-#include "kvm/kvm.h"
-#include "kvm/util.h"
-
-int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg)
-{
-	die(__FUNCTION__);
-	return 0;
-}
diff --git a/hw/pci-shmem.c b/hw/pci-shmem.c
index a1c5ab7..7ce98cb 100644
--- a/hw/pci-shmem.c
+++ b/hw/pci-shmem.c
@@ -136,6 +136,8 @@ int pci_shmem__get_local_irqfd(struct kvm *kvm)
 
 		if (pci_shmem_pci_device.msix.ctrl & cpu_to_le16(PCI_MSIX_FLAGS_ENABLE)) {
 			gsi = irq__add_msix_route(kvm, &msix_table[0].msg);
+			if (gsi < 0)
+				return gsi;
 		} else {
 			gsi = pci_shmem_pci_device.irq_line;
 		}
diff --git a/include/kvm/irq.h b/include/kvm/irq.h
index 8a78e43..bb71521 100644
--- a/include/kvm/irq.h
+++ b/include/kvm/irq.h
@@ -10,11 +10,16 @@
 
 struct kvm;
 
+extern struct kvm_irq_routing *irq_routing;
+extern int next_gsi;
+
 int irq__alloc_line(void);
 int irq__get_nr_allocated_lines(void);
 
 int irq__init(struct kvm *kvm);
 int irq__exit(struct kvm *kvm);
+
+int irq__allocate_routing_entry(void);
 int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg);
 
 #endif
diff --git a/irq.c b/irq.c
index 71eaa05..a742aa2 100644
--- a/irq.c
+++ b/irq.c
@@ -1,7 +1,19 @@
+#include <stdlib.h>
+#include <sys/ioctl.h>
+#include <linux/types.h>
+#include <linux/kvm.h>
+#include <errno.h>
+
+#include "kvm/kvm.h"
 #include "kvm/irq.h"
 #include "kvm/kvm-arch.h"
 
 static u8 next_line = KVM_IRQ_OFFSET;
+static int allocated_gsis = 0;
+
+int next_gsi;
+
+struct kvm_irq_routing *irq_routing = NULL;
 
 int irq__alloc_line(void)
 {
@@ -12,3 +24,78 @@ int irq__get_nr_allocated_lines(void)
 {
 	return next_line - KVM_IRQ_OFFSET;
 }
+
+int irq__allocate_routing_entry(void)
+{
+	size_t table_size = sizeof(struct kvm_irq_routing);
+	size_t old_size = table_size;
+	int nr_entries = 0;
+
+	if (irq_routing)
+		nr_entries = irq_routing->nr;
+
+	if (nr_entries < allocated_gsis)
+		return 0;
+
+	old_size += sizeof(struct kvm_irq_routing_entry) * allocated_gsis;
+	allocated_gsis = ALIGN(nr_entries + 1, 32);
+	table_size += sizeof(struct kvm_irq_routing_entry) * allocated_gsis;
+	irq_routing = realloc(irq_routing, table_size);
+
+	if (irq_routing == NULL)
+		return -ENOMEM;
+	memset((void *)irq_routing + old_size, 0, table_size - old_size);
+
+	irq_routing->nr = nr_entries;
+	irq_routing->flags = 0;
+
+	return 0;
+}
+
+static bool check_for_irq_routing(struct kvm *kvm)
+{
+	static int has_irq_routing = 0;
+
+	if (has_irq_routing == 0) {
+		if (kvm__supports_extension(kvm, KVM_CAP_IRQ_ROUTING))
+			has_irq_routing = 1;
+		else
+			has_irq_routing = -1;
+	}
+
+	return has_irq_routing > 0;
+}
+
+int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg)
+{
+	int r;
+
+	if (!check_for_irq_routing(kvm))
+		return -ENXIO;
+
+	r = irq__allocate_routing_entry();
+	if (r)
+		return r;
+
+	irq_routing->entries[irq_routing->nr++] =
+		(struct kvm_irq_routing_entry) {
+			.gsi = next_gsi,
+			.type = KVM_IRQ_ROUTING_MSI,
+			.u.msi.address_hi = msg->address_hi,
+			.u.msi.address_lo = msg->address_lo,
+			.u.msi.data = msg->data,
+		};
+
+	r = ioctl(kvm->vm_fd, KVM_SET_GSI_ROUTING, irq_routing);
+	if (r)
+		return r;
+
+	return next_gsi++;
+}
+
+int __attribute__((weak)) irq__exit(struct kvm *kvm)
+{
+	free(irq_routing);
+	return 0;
+}
+dev_base_exit(irq__exit);
diff --git a/mips/irq.c b/mips/irq.c
deleted file mode 100644
index c1ff6bb..0000000
--- a/mips/irq.c
+++ /dev/null
@@ -1,10 +0,0 @@
-#include "kvm/irq.h"
-#include "kvm/kvm.h"
-
-#include <stdlib.h>
-
-int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg)
-{
-	pr_warning("irq__add_msix_route");
-	return 1;
-}
diff --git a/powerpc/irq.c b/powerpc/irq.c
deleted file mode 100644
index 03f2fe7..0000000
--- a/powerpc/irq.c
+++ /dev/null
@@ -1,31 +0,0 @@
-/*
- * PPC64 IRQ routines
- *
- * Copyright 2011 Matt Evans <matt@ozlabs.org>, IBM Corporation.
- *
- * 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.
- */
-
-#include "kvm/devices.h"
-#include "kvm/irq.h"
-#include "kvm/kvm.h"
-#include "kvm/util.h"
-
-#include <linux/types.h>
-#include <linux/rbtree.h>
-#include <linux/list.h>
-#include <linux/kvm.h>
-#include <sys/ioctl.h>
-
-#include <stddef.h>
-#include <stdlib.h>
-
-#include "kvm/pci.h"
-
-int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg)
-{
-	die(__FUNCTION__);
-	return 0;
-}
diff --git a/virtio/pci.c b/virtio/pci.c
index 90fcd64..072e5b7 100644
--- a/virtio/pci.c
+++ b/virtio/pci.c
@@ -156,7 +156,8 @@ static bool virtio_pci__specific_io_out(struct kvm *kvm, struct virtio_device *v
 					void *data, int size, int offset)
 {
 	struct virtio_pci *vpci = vdev->virtio;
-	u32 config_offset, gsi, vec;
+	u32 config_offset, vec;
+	int gsi;
 	int type = virtio__get_dev_specific_field(offset - 20, virtio_pci__msix_enabled(vpci),
 							&config_offset);
 	if (type == VIRTIO_PCI_O_MSIX) {
@@ -166,21 +167,27 @@ static bool virtio_pci__specific_io_out(struct kvm *kvm, struct virtio_device *v
 			if (vec == VIRTIO_MSI_NO_VECTOR)
 				break;
 
-			gsi = irq__add_msix_route(kvm, &vpci->msix_table[vec].msg);
-
-			vpci->config_gsi = gsi;
+			gsi = irq__add_msix_route(kvm,
+						  &vpci->msix_table[vec].msg);
+			if (gsi >= 0)
+				vpci->config_gsi = gsi;
 			break;
 		case VIRTIO_MSI_QUEUE_VECTOR:
-			vec = vpci->vq_vector[vpci->queue_selector] = ioport__read16(data);
+			vec = ioport__read16(data);
+			vpci->vq_vector[vpci->queue_selector] = vec;
 
 			if (vec == VIRTIO_MSI_NO_VECTOR)
 				break;
 
-			gsi = irq__add_msix_route(kvm, &vpci->msix_table[vec].msg);
+			gsi = irq__add_msix_route(kvm,
+						  &vpci->msix_table[vec].msg);
+			if (gsi < 0)
+				break;
 			vpci->gsis[vpci->queue_selector] = gsi;
 			if (vdev->ops->notify_vq_gsi)
 				vdev->ops->notify_vq_gsi(kvm, vpci->dev,
-							vpci->queue_selector, gsi);
+							 vpci->queue_selector,
+							 gsi);
 			break;
 		};
 
diff --git a/x86/irq.c b/x86/irq.c
index 72177e7..db465a1 100644
--- a/x86/irq.c
+++ b/x86/irq.c
@@ -11,20 +11,15 @@
 #include <stddef.h>
 #include <stdlib.h>
 
-#define IRQ_MAX_GSI			64
 #define IRQCHIP_MASTER			0
 #define IRQCHIP_SLAVE			1
 #define IRQCHIP_IOAPIC			2
 
-/* First 24 GSIs are routed between IRQCHIPs and IOAPICs */
-static u32 gsi = 24;
-
-struct kvm_irq_routing *irq_routing;
-
 static int irq__add_routing(u32 gsi, u32 type, u32 irqchip, u32 pin)
 {
-	if (gsi >= IRQ_MAX_GSI)
-		return -ENOSPC;
+	int r = irq__allocate_routing_entry();
+	if (r)
+		return r;
 
 	irq_routing->entries[irq_routing->nr++] =
 		(struct kvm_irq_routing_entry) {
@@ -41,11 +36,6 @@ int irq__init(struct kvm *kvm)
 {
 	int i, r;
 
-	irq_routing = calloc(sizeof(struct kvm_irq_routing) +
-			IRQ_MAX_GSI * sizeof(struct kvm_irq_routing_entry), 1);
-	if (irq_routing == NULL)
-		return -ENOMEM;
-
 	/* Hook first 8 GSIs to master IRQCHIP */
 	for (i = 0; i < 8; i++)
 		if (i != 2)
@@ -69,33 +59,8 @@ int irq__init(struct kvm *kvm)
 		return errno;
 	}
 
-	return 0;
-}
-dev_base_init(irq__init);
+	next_gsi = i;
 
-int irq__exit(struct kvm *kvm)
-{
-	free(irq_routing);
 	return 0;
 }
-dev_base_exit(irq__exit);
-
-int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg)
-{
-	int r;
-
-	irq_routing->entries[irq_routing->nr++] =
-		(struct kvm_irq_routing_entry) {
-			.gsi = gsi,
-			.type = KVM_IRQ_ROUTING_MSI,
-			.u.msi.address_hi = msg->address_hi,
-			.u.msi.address_lo = msg->address_lo,
-			.u.msi.data = msg->data,
-		};
-
-	r = ioctl(kvm->vm_fd, KVM_SET_GSI_ROUTING, irq_routing);
-	if (r)
-		return r;
-
-	return gsi++;
-}
+dev_base_init(irq__init);
-- 
2.9.0

[PATCH v8 04/16] MSI-X: update GSI routing after changed MSI-X configuration

[Andre Przywara]

When we set up GSI routing to map MSIs to KVM's GSI numbers, we
write the current device's MSI setup into the kernel routing table.
However the device driver in the guest can use PCI configuration space
accesses to change the MSI configuration (address and/or payload data).
Whenever this happens after we have setup the routing table already,
we must amend the previously sent data.
So when MSI-X PCI config space accesses write address or payload,
find the associated GSI number and the matching routing table entry
and update the kernel routing table (only if the data has changed).

This fixes vhost-net, where the queue's IRQFD was setup before the
MSI vectors.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 include/kvm/irq.h |  1 +
 irq.c             | 31 +++++++++++++++++++++++++++++++
 virtio/pci.c      | 36 +++++++++++++++++++++++++++++++++---
 3 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/include/kvm/irq.h b/include/kvm/irq.h
index bb71521..f35eb7e 100644
--- a/include/kvm/irq.h
+++ b/include/kvm/irq.h
@@ -21,5 +21,6 @@ int irq__exit(struct kvm *kvm);
 
 int irq__allocate_routing_entry(void);
 int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg);
+void irq__update_msix_route(struct kvm *kvm, u32 gsi, struct msi_msg *msg);
 
 #endif
diff --git a/irq.c b/irq.c
index a742aa2..895e5eb 100644
--- a/irq.c
+++ b/irq.c
@@ -93,6 +93,37 @@ int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg)
 	return next_gsi++;
 }
 
+static bool update_data(u32 *ptr, u32 newdata)
+{
+	if (*ptr == newdata)
+		return false;
+
+	*ptr = newdata;
+	return true;
+}
+
+void irq__update_msix_route(struct kvm *kvm, u32 gsi, struct msi_msg *msg)
+{
+	struct kvm_irq_routing_msi *entry;
+	unsigned int i;
+	bool changed;
+
+	for (i = 0; i < irq_routing->nr; i++)
+		if (gsi == irq_routing->entries[i].gsi)
+			break;
+	if (i == irq_routing->nr)
+		return;
+
+	entry = &irq_routing->entries[i].u.msi;
+
+	changed  = update_data(&entry->address_hi, msg->address_hi);
+	changed |= update_data(&entry->address_lo, msg->address_lo);
+	changed |= update_data(&entry->data, msg->data);
+
+	if (changed)
+		ioctl(kvm->vm_fd, KVM_SET_GSI_ROUTING, irq_routing);
+}
+
 int __attribute__((weak)) irq__exit(struct kvm *kvm)
 {
 	free(irq_routing);
diff --git a/virtio/pci.c b/virtio/pci.c
index 072e5b7..b3b4aac 100644
--- a/virtio/pci.c
+++ b/virtio/pci.c
@@ -152,6 +152,30 @@ static bool virtio_pci__io_in(struct ioport *ioport, struct kvm_cpu *vcpu, u16 p
 	return ret;
 }
 
+static void update_msix_map(struct virtio_pci *vpci,
+			    struct msix_table *msix_entry, u32 vecnum)
+{
+	u32 gsi, i;
+
+	/* Find the GSI number used for that vector */
+	if (vecnum == vpci->config_vector) {
+		gsi = vpci->config_gsi;
+	} else {
+		for (i = 0; i < VIRTIO_PCI_MAX_VQ; i++)
+			if (vpci->vq_vector[i] == vecnum)
+				break;
+		if (i == VIRTIO_PCI_MAX_VQ)
+			return;
+		gsi = vpci->gsis[i];
+	}
+
+	if (gsi == 0)
+		return;
+
+	msix_entry = &msix_entry[vecnum];
+	irq__update_msix_route(vpci->kvm, gsi, &msix_entry->msg);
+}
+
 static bool virtio_pci__specific_io_out(struct kvm *kvm, struct virtio_device *vdev, u16 port,
 					void *data, int size, int offset)
 {
@@ -270,10 +294,16 @@ static void virtio_pci__msix_mmio_callback(struct kvm_cpu *vcpu,
 		offset	= vpci->msix_io_block;
 	}
 
-	if (is_write)
-		memcpy(table + addr - offset, data, len);
-	else
+	if (!is_write) {
 		memcpy(data, table + addr - offset, len);
+		return;
+	}
+
+	memcpy(table + addr - offset, data, len);
+
+	/* Did we just update the address or payload? */
+	if (addr % 0x10 < 0xc)
+		update_msix_map(vpci, table, (addr - offset) / 16);
 }
 
 static void virtio_pci__signal_msi(struct kvm *kvm, struct virtio_pci *vpci, int vec)
-- 
2.9.0

[PATCH v8 05/16] virtio: fix endianness check for vhost support

[Andre Przywara]

Currently we deny any VHOST_* functionality if the architecture
supports guests with different endianness than the host. Most of the
time even on those architectures the endianness of guest and host are
the same, though, so we are denying the glory of VHOST needlessly.
Switch from compile time determination to a run time scheme, which
takes the actual endianness of the guest into account.
For this we change the semantics of VIRTIO_ENDIAN_HOST to return the
actual endianness of the host (the endianness of kvmtool at compile
time, really). The actual check in vhost_net now compares this against
the guest endianness.
This enables vhost support on ARM and ARM64.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 include/kvm/virtio.h | 9 +++++++--
 virtio/net.c         | 2 +-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/include/kvm/virtio.h b/include/kvm/virtio.h
index 768ee96..66530fd 100644
--- a/include/kvm/virtio.h
+++ b/include/kvm/virtio.h
@@ -17,10 +17,15 @@
 #define VIRTIO_PCI_O_CONFIG	0
 #define VIRTIO_PCI_O_MSIX	1
 
-#define VIRTIO_ENDIAN_HOST	0
 #define VIRTIO_ENDIAN_LE	(1 << 0)
 #define VIRTIO_ENDIAN_BE	(1 << 1)
 
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+#define VIRTIO_ENDIAN_HOST VIRTIO_ENDIAN_LE
+#else
+#define VIRTIO_ENDIAN_HOST VIRTIO_ENDIAN_BE
+#endif
+
 struct virt_queue {
 	struct vring	vring;
 	u32		pfn;
@@ -40,7 +45,7 @@ struct virt_queue {
 #define VIRTIO_RING_ENDIAN VIRTIO_ENDIAN_HOST
 #endif
 
-#if (VIRTIO_RING_ENDIAN & (VIRTIO_ENDIAN_LE | VIRTIO_ENDIAN_BE))
+#if VIRTIO_RING_ENDIAN != VIRTIO_ENDIAN_HOST
 
 static inline __u16 __virtio_g2h_u16(u16 endian, __u16 val)
 {
diff --git a/virtio/net.c b/virtio/net.c
index 6d1be65..e94e37a 100644
--- a/virtio/net.c
+++ b/virtio/net.c
@@ -531,7 +531,7 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 	}
 
 	if (queue->endian != VIRTIO_ENDIAN_HOST)
-		die_perror("VHOST requires VIRTIO_ENDIAN_HOST");
+		die_perror("VHOST requires the same endianness in guest and host");
 
 	state.num = queue->vring.num;
 	r = ioctl(ndev->vhost_fd, VHOST_SET_VRING_NUM, &state);
-- 
2.9.0

[PATCH v8 06/16] PCI: Only allocate IRQ routing entry when available

[Andre Przywara]

If we need to inject an MSI into the guest, we rely at the moment on a
working GSI MSI routing functionality. However we can get away without
IRQ routing, if the host supports MSI injection via the KVM_SIGNAL_MSI
ioctl.
So we try the GSI routing first, but if that fails due to a missing
IRQ routing functionality, we fall back to KVM_SIGNAL_MSI (if that is
supported).

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 virtio/pci.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/virtio/pci.c b/virtio/pci.c
index b3b4aac..604b9ec 100644
--- a/virtio/pci.c
+++ b/virtio/pci.c
@@ -193,8 +193,14 @@ static bool virtio_pci__specific_io_out(struct kvm *kvm, struct virtio_device *v
 
 			gsi = irq__add_msix_route(kvm,
 						  &vpci->msix_table[vec].msg);
-			if (gsi >= 0)
+			if (gsi >= 0) {
 				vpci->config_gsi = gsi;
+				break;
+			}
+			if (gsi == -ENXIO &&
+			    vpci->features & VIRTIO_PCI_F_SIGNAL_MSI)
+					break;
+			die("failed to configure MSIs");
 			break;
 		case VIRTIO_MSI_QUEUE_VECTOR:
 			vec = ioport__read16(data);
@@ -205,8 +211,14 @@ static bool virtio_pci__specific_io_out(struct kvm *kvm, struct virtio_device *v
 
 			gsi = irq__add_msix_route(kvm,
 						  &vpci->msix_table[vec].msg);
-			if (gsi < 0)
+			if (gsi < 0) {
+				if (gsi == -ENXIO &&
+				    vpci->features & VIRTIO_PCI_F_SIGNAL_MSI)
+					break;
+
+				die("failed to configure MSIs");
 				break;
+			}
 			vpci->gsis[vpci->queue_selector] = gsi;
 			if (vdev->ops->notify_vq_gsi)
 				vdev->ops->notify_vq_gsi(kvm, vpci->dev,
-- 
2.9.0

[PATCH v8 07/16] update public Linux headers for GICv3 ITS emulation

[Andre Przywara]

The GICv3 ITS emulation brings some additions to the headers, so
lets update kvmtool's version of the headers to Linux' v4.8-rc2.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arm/aarch32/include/asm/kvm.h |  4 ++--
 arm/aarch64/include/asm/kvm.h |  2 ++
 include/linux/kvm.h           | 19 +++++++++++++++++--
 x86/include/asm/kvm.h         |  6 +++---
 4 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/arm/aarch32/include/asm/kvm.h b/arm/aarch32/include/asm/kvm.h
index df3f60c..a2b3eb3 100644
--- a/arm/aarch32/include/asm/kvm.h
+++ b/arm/aarch32/include/asm/kvm.h
@@ -139,8 +139,8 @@ struct kvm_arch_memory_slot {
 #define ARM_CP15_REG64(...) __ARM_CP15_REG64(__VA_ARGS__)
 
 #define KVM_REG_ARM_TIMER_CTL		ARM_CP15_REG32(0, 14, 3, 1)
-#define KVM_REG_ARM_TIMER_CNT		ARM_CP15_REG64(1, 14) 
-#define KVM_REG_ARM_TIMER_CVAL		ARM_CP15_REG64(3, 14) 
+#define KVM_REG_ARM_TIMER_CNT		ARM_CP15_REG64(1, 14)
+#define KVM_REG_ARM_TIMER_CVAL		ARM_CP15_REG64(3, 14)
 
 /* Normal registers are mapped as coprocessor 16. */
 #define KVM_REG_ARM_CORE		(0x0010 << KVM_REG_ARM_COPROC_SHIFT)
diff --git a/arm/aarch64/include/asm/kvm.h b/arm/aarch64/include/asm/kvm.h
index f209ea1..3051f86 100644
--- a/arm/aarch64/include/asm/kvm.h
+++ b/arm/aarch64/include/asm/kvm.h
@@ -87,9 +87,11 @@ struct kvm_regs {
 /* Supported VGICv3 address types  */
 #define KVM_VGIC_V3_ADDR_TYPE_DIST	2
 #define KVM_VGIC_V3_ADDR_TYPE_REDIST	3
+#define KVM_VGIC_ITS_ADDR_TYPE		4
 
 #define KVM_VGIC_V3_DIST_SIZE		SZ_64K
 #define KVM_VGIC_V3_REDIST_SIZE		(2 * SZ_64K)
+#define KVM_VGIC_V3_ITS_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 */
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index a7f1f80..300ef25 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -865,6 +865,11 @@ struct kvm_ppc_smmu_info {
 #define KVM_CAP_SPAPR_TCE_64 125
 #define KVM_CAP_ARM_PMU_V3 126
 #define KVM_CAP_VCPU_ATTRIBUTES 127
+#define KVM_CAP_MAX_VCPU_ID 128
+#define KVM_CAP_X2APIC_API 129
+#define KVM_CAP_S390_USER_INSTR0 130
+#define KVM_CAP_MSI_DEVID 131
+#define KVM_CAP_PPC_HTM 132
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
@@ -877,7 +882,10 @@ struct kvm_irq_routing_msi {
 	__u32 address_lo;
 	__u32 address_hi;
 	__u32 data;
-	__u32 pad;
+	union {
+		__u32 pad;
+		__u32 devid;
+	};
 };
 
 struct kvm_irq_routing_s390_adapter {
@@ -1023,12 +1031,14 @@ struct kvm_one_reg {
 	__u64 addr;
 };
 
+#define KVM_MSI_VALID_DEVID	(1U << 0)
 struct kvm_msi {
 	__u32 address_lo;
 	__u32 address_hi;
 	__u32 data;
 	__u32 flags;
-	__u8  pad[16];
+	__u32 devid;
+	__u8  pad[12];
 };
 
 struct kvm_arm_device_addr {
@@ -1073,6 +1083,8 @@ enum kvm_device_type {
 #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_ARM_VGIC_ITS,
+#define KVM_DEV_TYPE_ARM_VGIC_ITS	KVM_DEV_TYPE_ARM_VGIC_ITS
 	KVM_DEV_TYPE_MAX,
 };
 
@@ -1312,4 +1324,7 @@ struct kvm_assigned_msix_entry {
 	__u16 padding[3];
 };
 
+#define KVM_X2APIC_API_USE_32BIT_IDS            (1ULL << 0)
+#define KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK  (1ULL << 1)
+
 #endif /* __LINUX_KVM_H */
diff --git a/x86/include/asm/kvm.h b/x86/include/asm/kvm.h
index cd54147..739c0c5 100644
--- a/x86/include/asm/kvm.h
+++ b/x86/include/asm/kvm.h
@@ -216,9 +216,9 @@ struct kvm_cpuid_entry2 {
 	__u32 padding[3];
 };
 
-#define KVM_CPUID_FLAG_SIGNIFCANT_INDEX		BIT(0)
-#define KVM_CPUID_FLAG_STATEFUL_FUNC		BIT(1)
-#define KVM_CPUID_FLAG_STATE_READ_NEXT		BIT(2)
+#define KVM_CPUID_FLAG_SIGNIFCANT_INDEX		(1 << 0)
+#define KVM_CPUID_FLAG_STATEFUL_FUNC		(1 << 1)
+#define KVM_CPUID_FLAG_STATE_READ_NEXT		(1 << 2)
 
 /* for KVM_SET_CPUID2 */
 struct kvm_cpuid2 {
-- 
2.9.0

[PATCH v8 08/16] arm: gic: allow 32-bit compilation

[Andre Przywara]

The ARM GIC code is shared between ARM and arm64, though the header
files from the Linux kernel are actually different, reflecting the
different level of GIC emulation support between the two architectures.
Since it is conceivable that ARM will gain GICv3 (and possibly ITS)
emulation support too, lets conditionally define those missing
symbols in the source file until an upcoming ARM header update eventually
includes those.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arm/gic.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arm/gic.c b/arm/gic.c
index 2c1a547..4d12f31 100644
--- a/arm/gic.c
+++ b/arm/gic.c
@@ -18,6 +18,14 @@
 #define KVM_VGIC_V3_ADDR_TYPE_REDIST 3
 #endif
 
+#ifndef KVM_VGIC_ITS_ADDR_TYPE
+#define KVM_VGIC_ITS_ADDR_TYPE 4
+#endif
+
+#ifndef KVM_VGIC_V3_ITS_SIZE
+#define KVM_VGIC_V3_ITS_SIZE (2 * SZ_64K)
+#endif
+
 static int gic_fd = -1;
 static u64 gic_redists_base;
 static u64 gic_redists_size;
-- 
2.9.0

[PATCH v8 09/16] arm: allow creation of an MSI register frame region

[Andre Przywara]

The GICv3 ITS expects a separate 64K page to hold ITS registers.
Add a function to reserve such a page in the guest's I/O memory and
use that for the ITS vGIC type.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arm/gic.c                    | 63 ++++++++++++++++++++++++++++++++++++++++++++
 arm/include/arm-common/gic.h |  1 +
 2 files changed, 64 insertions(+)

diff --git a/arm/gic.c b/arm/gic.c
index 4d12f31..ed9016d 100644
--- a/arm/gic.c
+++ b/arm/gic.c
@@ -8,6 +8,7 @@
 #include <linux/byteorder.h>
 #include <linux/kernel.h>
 #include <linux/kvm.h>
+#include <linux/sizes.h>
 
 /* Those names are not defined for ARM (yet) */
 #ifndef KVM_VGIC_V3_ADDR_TYPE_DIST
@@ -29,6 +30,8 @@
 static int gic_fd = -1;
 static u64 gic_redists_base;
 static u64 gic_redists_size;
+static u64 gic_msi_base;
+static u64 gic_msi_size = 0;
 
 int irqchip_parser(const struct option *opt, const char *arg, int unset)
 {
@@ -46,6 +49,56 @@ int irqchip_parser(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
+static int gic__create_its_frame(struct kvm *kvm, u64 its_frame_addr)
+{
+	struct kvm_create_device its_device = {
+		.type = KVM_DEV_TYPE_ARM_VGIC_ITS,
+		.flags	= 0,
+	};
+	struct kvm_device_attr its_attr = {
+		.group	= KVM_DEV_ARM_VGIC_GRP_ADDR,
+		.attr	= KVM_VGIC_ITS_ADDR_TYPE,
+		.addr	= (u64)(unsigned long)&its_frame_addr,
+	};
+	struct kvm_device_attr its_init_attr = {
+		.group	= KVM_DEV_ARM_VGIC_GRP_CTRL,
+		.attr	= KVM_DEV_ARM_VGIC_CTRL_INIT,
+	};
+	int err;
+
+	err = ioctl(kvm->vm_fd, KVM_CREATE_DEVICE, &its_device);
+	if (err) {
+		fprintf(stderr,
+			"GICv3 ITS requested, but kernel does not support it.\n");
+		fprintf(stderr, "Try --irqchip=gicv3 instead\n");
+		return err;
+	}
+
+	err = ioctl(its_device.fd, KVM_HAS_DEVICE_ATTR, &its_attr);
+	if (err) {
+		close(its_device.fd);
+		its_device.fd = -1;
+		return err;
+	}
+
+	err = ioctl(its_device.fd, KVM_SET_DEVICE_ATTR, &its_attr);
+	if (err)
+		return err;
+
+	return ioctl(its_device.fd, KVM_SET_DEVICE_ATTR, &its_init_attr);
+}
+
+static int gic__create_msi_frame(struct kvm *kvm, enum irqchip_type type,
+				 u64 msi_frame_addr)
+{
+	switch (type) {
+	case IRQCHIP_GICV3_ITS:
+		return gic__create_its_frame(kvm, msi_frame_addr);
+	default:	/* No MSI frame needed */
+		return 0;
+	}
+}
+
 static int gic__create_device(struct kvm *kvm, enum irqchip_type type)
 {
 	int err;
@@ -75,6 +128,7 @@ static int gic__create_device(struct kvm *kvm, enum irqchip_type type)
 		dist_attr.attr  = KVM_VGIC_V2_ADDR_TYPE_DIST;
 		break;
 	case IRQCHIP_GICV3:
+	case IRQCHIP_GICV3_ITS:
 		gic_device.type = KVM_DEV_TYPE_ARM_VGIC_V3;
 		dist_attr.attr  = KVM_VGIC_V3_ADDR_TYPE_DIST;
 		break;
@@ -90,6 +144,7 @@ static int gic__create_device(struct kvm *kvm, enum irqchip_type type)
 	case IRQCHIP_GICV2:
 		err = ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &cpu_if_attr);
 		break;
+	case IRQCHIP_GICV3_ITS:
 	case IRQCHIP_GICV3:
 		err = ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &redist_attr);
 		break;
@@ -101,6 +156,10 @@ static int gic__create_device(struct kvm *kvm, enum irqchip_type type)
 	if (err)
 		goto out_err;
 
+	err = gic__create_msi_frame(kvm, type, gic_msi_base);
+	if (err)
+		goto out_err;
+
 	return 0;
 
 out_err:
@@ -144,9 +203,13 @@ int gic__create(struct kvm *kvm, enum irqchip_type type)
 	switch (type) {
 	case IRQCHIP_GICV2:
 		break;
+	case IRQCHIP_GICV3_ITS:
+		gic_msi_size = KVM_VGIC_V3_ITS_SIZE;
+		/* fall through */
 	case IRQCHIP_GICV3:
 		gic_redists_size = kvm->cfg.nrcpus * ARM_GIC_REDIST_SIZE;
 		gic_redists_base = ARM_GIC_DIST_BASE - gic_redists_size;
+		gic_msi_base = gic_redists_base - gic_msi_size;
 		break;
 	default:
 		return -ENODEV;
diff --git a/arm/include/arm-common/gic.h b/arm/include/arm-common/gic.h
index b43a180..433dd23 100644
--- a/arm/include/arm-common/gic.h
+++ b/arm/include/arm-common/gic.h
@@ -24,6 +24,7 @@
 enum irqchip_type {
 	IRQCHIP_GICV2,
 	IRQCHIP_GICV3,
+	IRQCHIP_GICV3_ITS,
 };
 
 struct kvm;
-- 
2.9.0

[PATCH v8 10/16] arm: FDT: create MSI controller DT node

[Andre Przywara]

The ARM GICv3 ITS requires a separate device tree node to describe
the ITS. Add this as a child to the GIC interrupt controller node
to let a guest discover and use the ITS if the user requests it.
Since we now need to specify #address-cells for the GIC node, we
have to add two zeroes to the interrupt map to match that.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arm/gic.c         | 23 ++++++++++++++++++++++-
 arm/pci.c         | 12 ++++++++++--
 include/kvm/fdt.h |  2 +-
 3 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/arm/gic.c b/arm/gic.c
index ed9016d..44f9365 100644
--- a/arm/gic.c
+++ b/arm/gic.c
@@ -267,7 +267,8 @@ late_init(gic__init_gic)
 
 void gic__generate_fdt_nodes(void *fdt, enum irqchip_type type)
 {
-	const char *compatible;
+	const char *compatible, *msi_compatible = NULL;
+	u64 msi_prop[2];
 	u64 reg_prop[] = {
 		cpu_to_fdt64(ARM_GIC_DIST_BASE), cpu_to_fdt64(ARM_GIC_DIST_SIZE),
 		0, 0,				/* to be filled */
@@ -279,6 +280,9 @@ void gic__generate_fdt_nodes(void *fdt, enum irqchip_type type)
 		reg_prop[2] = cpu_to_fdt64(ARM_GIC_CPUI_BASE);
 		reg_prop[3] = cpu_to_fdt64(ARM_GIC_CPUI_SIZE);
 		break;
+	case IRQCHIP_GICV3_ITS:
+		msi_compatible = "arm,gic-v3-its";
+		/* fall-through */
 	case IRQCHIP_GICV3:
 		compatible = "arm,gic-v3";
 		reg_prop[2] = cpu_to_fdt64(gic_redists_base);
@@ -294,6 +298,23 @@ void gic__generate_fdt_nodes(void *fdt, enum irqchip_type type)
 	_FDT(fdt_property(fdt, "interrupt-controller", NULL, 0));
 	_FDT(fdt_property(fdt, "reg", reg_prop, sizeof(reg_prop)));
 	_FDT(fdt_property_cell(fdt, "phandle", fdt__get_phandle(PHANDLE_GIC)));
+	_FDT(fdt_property_cell(fdt, "#address-cells", 2));
+	_FDT(fdt_property_cell(fdt, "#size-cells", 2));
+
+	if (msi_compatible) {
+		_FDT(fdt_property(fdt, "ranges", NULL, 0));
+
+		_FDT(fdt_begin_node(fdt, "msic"));
+		_FDT(fdt_property_string(fdt, "compatible", msi_compatible));
+		_FDT(fdt_property(fdt, "msi-controller", NULL, 0));
+		_FDT(fdt_property_cell(fdt, "phandle",
+				       fdt__get_phandle(PHANDLE_MSI)));
+		msi_prop[0] = cpu_to_fdt64(gic_msi_base);
+		msi_prop[1] = cpu_to_fdt64(gic_msi_size);
+		_FDT(fdt_property(fdt, "reg", msi_prop, sizeof(msi_prop)));
+		_FDT(fdt_end_node(fdt));
+	}
+
 	_FDT(fdt_end_node(fdt));
 }
 
diff --git a/arm/pci.c b/arm/pci.c
index 9630657..104349a 100644
--- a/arm/pci.c
+++ b/arm/pci.c
@@ -18,6 +18,8 @@ struct of_gic_irq {
 struct of_interrupt_map_entry {
 	struct of_pci_irq_mask		pci_irq_mask;
 	u32				gic_phandle;
+	u32				gic_addr_hi;
+	u32				gic_addr_lo;
 	struct of_gic_irq		gic_irq;
 } __attribute__((packed));
 
@@ -26,7 +28,7 @@ void pci__generate_fdt_nodes(void *fdt)
 	struct device_header *dev_hdr;
 	struct of_interrupt_map_entry irq_map[OF_PCI_IRQ_MAP_MAX];
 	unsigned nentries = 0;
-	u32 gic_phandle = fdt__get_phandle(PHANDLE_GIC);
+	u32 phandle;
 	/* Bus range */
 	u32 bus_range[] = { cpu_to_fdt32(0), cpu_to_fdt32(1), };
 	/* Configuration Space */
@@ -65,7 +67,11 @@ void pci__generate_fdt_nodes(void *fdt)
 	_FDT(fdt_property(fdt, "bus-range", bus_range, sizeof(bus_range)));
 	_FDT(fdt_property(fdt, "reg", &cfg_reg_prop, sizeof(cfg_reg_prop)));
 	_FDT(fdt_property(fdt, "ranges", ranges, sizeof(ranges)));
+	phandle = fdt__get_phandle(PHANDLE_MSI);
+	if (FDT_IS_VALID_PHANDLE(phandle))
+		_FDT(fdt_property_cell(fdt, "msi-parent", phandle));
 
+	phandle = fdt__get_phandle(PHANDLE_GIC);
 	/* Generate the interrupt map ... */
 	dev_hdr = device__first_dev(DEVICE_BUS_PCI);
 	while (dev_hdr && nentries < ARRAY_SIZE(irq_map)) {
@@ -84,7 +90,9 @@ void pci__generate_fdt_nodes(void *fdt)
 				},
 				.pci_pin	= cpu_to_fdt32(pin),
 			},
-			.gic_phandle	= cpu_to_fdt32(gic_phandle),
+			.gic_phandle	= cpu_to_fdt32(phandle),
+			.gic_addr_hi	= 0,
+			.gic_addr_lo	= 0,
 			.gic_irq = {
 				.type	= cpu_to_fdt32(GIC_FDT_IRQ_TYPE_SPI),
 				.num	= cpu_to_fdt32(irq - GIC_SPI_IRQ_BASE),
diff --git a/include/kvm/fdt.h b/include/kvm/fdt.h
index cd2bb72..8006e8f 100644
--- a/include/kvm/fdt.h
+++ b/include/kvm/fdt.h
@@ -11,7 +11,7 @@
 #define FDT_INVALID_PHANDLE 0
 #define FDT_IS_VALID_PHANDLE(phandle) ((phandle) != FDT_INVALID_PHANDLE)
 
-enum phandles {PHANDLE_GIC, PHANDLES_MAX};
+enum phandles {PHANDLE_GIC, PHANDLE_MSI, PHANDLES_MAX};
 
 /* Those definitions are generic FDT values for specifying IRQ
  * types and are used in the Linux kernel internally as well as in
-- 
2.9.0

[PATCH v8 11/16] add kvm__check_vm_capability

[Andre Przywara]

KVM capabilities can be per-VM, in this case the ioctl should be
issued on the VM file descriptor, not on the system fd.
Since this feature is guarded by a (system) capability itself, wrap
the call into a function of its own.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 include/kvm/kvm.h |  1 +
 kvm.c             | 28 ++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/include/kvm/kvm.h b/include/kvm/kvm.h
index 4a76ec2..a76a25d 100644
--- a/include/kvm/kvm.h
+++ b/include/kvm/kvm.h
@@ -129,6 +129,7 @@ static inline bool host_ptr_in_ram(struct kvm *kvm, void *p)
 }
 
 bool kvm__supports_extension(struct kvm *kvm, unsigned int extension);
+bool kvm__supports_vm_extension(struct kvm *kvm, unsigned int extension);
 
 static inline void kvm__set_thread_name(const char *name)
 {
diff --git a/kvm.c b/kvm.c
index 7fa76f7..665ed14 100644
--- a/kvm.c
+++ b/kvm.c
@@ -93,6 +93,34 @@ const char *kvm__get_dir(void)
 	return kvm_dir;
 }
 
+bool kvm__supports_vm_extension(struct kvm *kvm, unsigned int extension)
+{
+	static int supports_vm_ext_check = 0;
+	int ret;
+
+	switch (supports_vm_ext_check) {
+	case 0:
+		ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION,
+			    KVM_CAP_CHECK_EXTENSION_VM);
+		if (ret <= 0) {
+			supports_vm_ext_check = -1;
+			return false;
+		}
+		supports_vm_ext_check = 1;
+		/* fall through */
+	case 1:
+		break;
+	case -1:
+		return false;
+	}
+
+	ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, extension);
+	if (ret < 0)
+		return false;
+
+	return ret;
+}
+
 bool kvm__supports_extension(struct kvm *kvm, unsigned int extension)
 {
 	int ret;
-- 
2.9.0

[PATCH v8 12/16] PCI: inject PCI device ID on MSI injection

[Andre Przywara]

The ITS emulation requires a unique device ID to be passed along the
MSI payload when kvmtool wants to trigger an MSI in the guest.
According to the proposed changes to the interface add the PCI
bus/device/function triple to the structure passed with the ioctl.
Check the respective capability before actually adding the device ID
to the kvm_msi struct.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 virtio/pci.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/virtio/pci.c b/virtio/pci.c
index 604b9ec..e9f81f7 100644
--- a/virtio/pci.c
+++ b/virtio/pci.c
@@ -318,14 +318,28 @@ static void virtio_pci__msix_mmio_callback(struct kvm_cpu *vcpu,
 		update_msix_map(vpci, table, (addr - offset) / 16);
 }
 
-static void virtio_pci__signal_msi(struct kvm *kvm, struct virtio_pci *vpci, int vec)
+static void virtio_pci__signal_msi(struct kvm *kvm, struct virtio_pci *vpci,
+				   int vec)
 {
+	static int needs_devid = 0;
 	struct kvm_msi msi = {
 		.address_lo = vpci->msix_table[vec].msg.address_lo,
 		.address_hi = vpci->msix_table[vec].msg.address_hi,
 		.data = vpci->msix_table[vec].msg.data,
 	};
 
+	if (needs_devid == 0) {
+		if (kvm__supports_vm_extension(kvm, KVM_CAP_MSI_DEVID))
+			needs_devid = 1;
+		else
+			needs_devid = -1;
+	}
+
+	if (needs_devid > 0) {
+		msi.flags = KVM_MSI_VALID_DEVID;
+		msi.devid = vpci->dev_hdr.dev_num << 3;
+	}
+
 	ioctl(kvm->vm_fd, KVM_SIGNAL_MSI, &msi);
 }
 
-- 
2.9.0

[PATCH v8 13/16] arm: setup SPI IRQ routing tables

[Andre Przywara]

Since we soon start using GSI routing on ARM platforms too, we have
to setup the initial SPI routing table. Before the first call to
KVM_SET_GSI_ROUTING, the kernel holds this table internally, but this
is overwritten with the ioctl, so we have to explicitly set it up
here.
The routing is actually not used for IRQs triggered by KVM_IRQ_LINE,
but it needs to be here anyway. We use a simple 1:1 mapping.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arm/gic.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/arm/gic.c b/arm/gic.c
index 44f9365..c422e36 100644
--- a/arm/gic.c
+++ b/arm/gic.c
@@ -27,6 +27,8 @@
 #define KVM_VGIC_V3_ITS_SIZE (2 * SZ_64K)
 #endif
 
+#define IRQCHIP_GIC 0
+
 static int gic_fd = -1;
 static u64 gic_redists_base;
 static u64 gic_redists_size;
@@ -49,6 +51,34 @@ int irqchip_parser(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
+static int irq__routing_init(struct kvm *kvm)
+{
+	int r;
+	int irqlines = ALIGN(irq__get_nr_allocated_lines(), 32);
+
+	/*
+	 * This describes the default routing that the kernel uses without
+	 * any routing explicitly set up via KVM_SET_GSI_ROUTING. So we
+	 * don't need to commit these setting right now. The first actual
+	 * user (MSI routing) will engage these mappings then.
+	 */
+	for (next_gsi = 0; next_gsi < irqlines; next_gsi++) {
+		r = irq__allocate_routing_entry();
+		if (r)
+			return r;
+
+		irq_routing->entries[irq_routing->nr++] =
+			(struct kvm_irq_routing_entry) {
+				.gsi = next_gsi,
+				.type = KVM_IRQ_ROUTING_IRQCHIP,
+				.u.irqchip.irqchip = IRQCHIP_GIC,
+				.u.irqchip.pin = next_gsi,
+		};
+	}
+
+	return 0;
+}
+
 static int gic__create_its_frame(struct kvm *kvm, u64 its_frame_addr)
 {
 	struct kvm_create_device its_device = {
@@ -255,6 +285,8 @@ static int gic__init_gic(struct kvm *kvm)
 			return ret;
 	}
 
+	irq__routing_init(kvm);
+
 	if (!ioctl(gic_fd, KVM_HAS_DEVICE_ATTR, &vgic_init_attr)) {
 		ret = ioctl(gic_fd, KVM_SET_DEVICE_ATTR, &vgic_init_attr);
 		if (ret)
-- 
2.9.0

[PATCH v8 14/16] extend GSI IRQ routing to take a device ID

[Andre Przywara]

For ITS emulation we need the device ID along with the MSI payload
and doorbell address to identify an MSI, so we need to put it in the
GSI IRQ routing table too.
There is a per-VM capability by which the kernel signals the need for
a device ID, so check this and put the device ID into the routing
table if needed.
For PCI devices we take the bus/device/function triplet and and that
to the routing setup call.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 hw/pci-shmem.c    |  3 ++-
 include/kvm/irq.h |  2 +-
 irq.c             | 24 ++++++++++++++++++++++--
 virtio/pci.c      |  6 ++++--
 4 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/hw/pci-shmem.c b/hw/pci-shmem.c
index 7ce98cb..512b5b0 100644
--- a/hw/pci-shmem.c
+++ b/hw/pci-shmem.c
@@ -135,7 +135,8 @@ int pci_shmem__get_local_irqfd(struct kvm *kvm)
 			return fd;
 
 		if (pci_shmem_pci_device.msix.ctrl & cpu_to_le16(PCI_MSIX_FLAGS_ENABLE)) {
-			gsi = irq__add_msix_route(kvm, &msix_table[0].msg);
+			gsi = irq__add_msix_route(kvm, &msix_table[0].msg,
+						  pci_shmem_device.dev_num << 3);
 			if (gsi < 0)
 				return gsi;
 		} else {
diff --git a/include/kvm/irq.h b/include/kvm/irq.h
index f35eb7e..ee059e3 100644
--- a/include/kvm/irq.h
+++ b/include/kvm/irq.h
@@ -20,7 +20,7 @@ int irq__init(struct kvm *kvm);
 int irq__exit(struct kvm *kvm);
 
 int irq__allocate_routing_entry(void);
-int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg);
+int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg, u32 device_id);
 void irq__update_msix_route(struct kvm *kvm, u32 gsi, struct msi_msg *msg);
 
 #endif
diff --git a/irq.c b/irq.c
index 895e5eb..4c2721a 100644
--- a/irq.c
+++ b/irq.c
@@ -66,7 +66,21 @@ static bool check_for_irq_routing(struct kvm *kvm)
 	return has_irq_routing > 0;
 }
 
-int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg)
+static bool check_for_msi_devid(struct kvm *kvm)
+{
+	static int needs_devid = 0;
+
+	if (needs_devid == 0) {
+		if (kvm__supports_vm_extension(kvm, KVM_CAP_MSI_DEVID))
+			needs_devid = 1;
+		else
+			needs_devid = -1;
+	}
+
+	return needs_devid > 0;
+}
+
+int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg, u32 device_id)
 {
 	int r;
 
@@ -77,7 +91,7 @@ int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg)
 	if (r)
 		return r;
 
-	irq_routing->entries[irq_routing->nr++] =
+	irq_routing->entries[irq_routing->nr] =
 		(struct kvm_irq_routing_entry) {
 			.gsi = next_gsi,
 			.type = KVM_IRQ_ROUTING_MSI,
@@ -86,6 +100,12 @@ int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg)
 			.u.msi.data = msg->data,
 		};
 
+	if (check_for_msi_devid(kvm)) {
+		irq_routing->entries[irq_routing->nr].flags = KVM_MSI_VALID_DEVID;
+		irq_routing->entries[irq_routing->nr].u.msi.devid = device_id;
+	}
+	irq_routing->nr++;
+
 	r = ioctl(kvm->vm_fd, KVM_SET_GSI_ROUTING, irq_routing);
 	if (r)
 		return r;
diff --git a/virtio/pci.c b/virtio/pci.c
index e9f81f7..da9a555 100644
--- a/virtio/pci.c
+++ b/virtio/pci.c
@@ -192,7 +192,8 @@ static bool virtio_pci__specific_io_out(struct kvm *kvm, struct virtio_device *v
 				break;
 
 			gsi = irq__add_msix_route(kvm,
-						  &vpci->msix_table[vec].msg);
+						  &vpci->msix_table[vec].msg,
+						  vpci->dev_hdr.dev_num << 3);
 			if (gsi >= 0) {
 				vpci->config_gsi = gsi;
 				break;
@@ -210,7 +211,8 @@ static bool virtio_pci__specific_io_out(struct kvm *kvm, struct virtio_device *v
 				break;
 
 			gsi = irq__add_msix_route(kvm,
-						  &vpci->msix_table[vec].msg);
+						  &vpci->msix_table[vec].msg,
+						  vpci->dev_hdr.dev_num << 3);
 			if (gsi < 0) {
 				if (gsi == -ENXIO &&
 				    vpci->features & VIRTIO_PCI_F_SIGNAL_MSI)
-- 
2.9.0

[PATCH v8 15/16] arm64: enable GICv3-ITS emulation

[Andre Przywara]

With everything in place for the ITS emulation add a new option to the
--irqchip parameter to allow the user to specify --irqchip=gicv3-its
to enable the ITS emulation.
This will trigger creating the FDT node and an ITS register frame to
tell the kernel we want ITS emulation in the guest.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arm/gic.c                                | 2 ++
 arm/include/arm-common/kvm-config-arch.h | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arm/gic.c b/arm/gic.c
index c422e36..cd4f518 100644
--- a/arm/gic.c
+++ b/arm/gic.c
@@ -43,6 +43,8 @@ int irqchip_parser(const struct option *opt, const char *arg, int unset)
 		*type = IRQCHIP_GICV2;
 	} else if (!strcmp(arg, "gicv3")) {
 		*type = IRQCHIP_GICV3;
+	} else if (!strcmp(arg, "gicv3-its")) {
+		*type = IRQCHIP_GICV3_ITS;
 	} else {
 		pr_err("irqchip: unknown type \"%s\"\n", arg);
 		return -1;
diff --git a/arm/include/arm-common/kvm-config-arch.h b/arm/include/arm-common/kvm-config-arch.h
index ed626b5..b48e720 100644
--- a/arm/include/arm-common/kvm-config-arch.h
+++ b/arm/include/arm-common/kvm-config-arch.h
@@ -27,7 +27,7 @@ int irqchip_parser(const struct option *opt, const char *arg, int unset);
 		    "Force virtio devices to use PCI as their default "		\
 		    "transport"),						\
         OPT_CALLBACK('\0', "irqchip", &(cfg)->irqchip,				\
-		     "[gicv2|gicv3]",					\
+		     "[gicv2|gicv3|gicv3-its]",					\
 		     "Type of interrupt controller to emulate in the guest",	\
 		     irqchip_parser, NULL),
 
-- 
2.9.0

[PATCH v8 16/16] arm: add support for vGICv3 and vITS

[Andre Przywara]

From: Vladimir Murzin <vladimir.murzin@arm.com>

KVM/arm got support for vGICv3 and vITS recently, so make use of it.

Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arm/aarch32/arm-cpu.c              | 2 +-
 arm/aarch32/include/kvm/kvm-arch.h | 3 ---
 arm/aarch64/include/kvm/kvm-arch.h | 3 ---
 arm/include/arm-common/kvm-arch.h  | 2 ++
 4 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/arm/aarch32/arm-cpu.c b/arm/aarch32/arm-cpu.c
index 27a8e17..16bba55 100644
--- a/arm/aarch32/arm-cpu.c
+++ b/arm/aarch32/arm-cpu.c
@@ -12,7 +12,7 @@ static void generate_fdt_nodes(void *fdt, struct kvm *kvm)
 {
 	int timer_interrupts[4] = {13, 14, 11, 10};
 
-	gic__generate_fdt_nodes(fdt, IRQCHIP_GICV2);
+	gic__generate_fdt_nodes(fdt, kvm->cfg.arch.irqchip);
 	timer__generate_fdt_nodes(fdt, kvm, timer_interrupts);
 }
 
diff --git a/arm/aarch32/include/kvm/kvm-arch.h b/arm/aarch32/include/kvm/kvm-arch.h
index 1632e3c..cd31e72 100644
--- a/arm/aarch32/include/kvm/kvm-arch.h
+++ b/arm/aarch32/include/kvm/kvm-arch.h
@@ -1,9 +1,6 @@
 #ifndef KVM__KVM_ARCH_H
 #define KVM__KVM_ARCH_H
 
-#define ARM_GIC_DIST_SIZE	0x1000
-#define ARM_GIC_CPUI_SIZE	0x2000
-
 #define ARM_KERN_OFFSET(...)	0x8000
 
 #define ARM_MAX_MEMORY(...)	ARM_LOMAP_MAX_MEMORY
diff --git a/arm/aarch64/include/kvm/kvm-arch.h b/arm/aarch64/include/kvm/kvm-arch.h
index 4925736..9de623a 100644
--- a/arm/aarch64/include/kvm/kvm-arch.h
+++ b/arm/aarch64/include/kvm/kvm-arch.h
@@ -1,9 +1,6 @@
 #ifndef KVM__KVM_ARCH_H
 #define KVM__KVM_ARCH_H
 
-#define ARM_GIC_DIST_SIZE	0x10000
-#define ARM_GIC_CPUI_SIZE	0x20000
-
 #define ARM_KERN_OFFSET(kvm)	((kvm)->cfg.arch.aarch32_guest	?	\
 				0x8000				:	\
 				0x80000)
diff --git a/arm/include/arm-common/kvm-arch.h b/arm/include/arm-common/kvm-arch.h
index 0f5fb7f..c83c45f 100644
--- a/arm/include/arm-common/kvm-arch.h
+++ b/arm/include/arm-common/kvm-arch.h
@@ -18,6 +18,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_GIC_DIST_SIZE	0x10000
+#define ARM_GIC_CPUI_SIZE	0x20000
 
 #define ARM_IOPORT_SIZE		(ARM_MMIO_AREA - ARM_IOPORT_AREA)
 #define ARM_VIRTIO_MMIO_SIZE	(ARM_AXI_AREA - (ARM_MMIO_AREA + ARM_GIC_SIZE))
-- 
2.9.0

[PATCH v8 01/16] FDT: introduce global phandle allocation

[Marc Zyngier]

Hi Andre,

On 04/11/16 17:31, Andre Przywara wrote:
> Allocating an FDT phandle (a unique identifier) using a static
> variable in a static inline function defined in a header file works
> only if all users are in the same source file. So trying to allocate
> a handle from two different compilation units fails.
> Introduce global phandle allocation and reference code to properly
> allocate unique phandles.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  Makefile          |  1 +
>  arm/fdt.c         |  2 +-
>  arm/gic.c         |  2 +-
>  include/kvm/fdt.h | 10 +++++-----
>  kvm-fdt.c         | 26 ++++++++++++++++++++++++++
>  5 files changed, 34 insertions(+), 7 deletions(-)
>  create mode 100644 kvm-fdt.c
> 
> diff --git a/Makefile b/Makefile
> index 1f0196f..e4a4002 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -98,6 +98,7 @@ OBJS	+= kvm-ipc.o
>  OBJS	+= builtin-sandbox.o
>  OBJS	+= virtio/mmio.o
>  OBJS	+= hw/i8042.o
> +OBJS	+= kvm-fdt.o
>  
>  # Translate uname -m into ARCH string
>  ARCH ?= $(shell uname -m | sed -e s/i.86/i386/ -e s/ppc.*/powerpc/ \
> diff --git a/arm/fdt.c b/arm/fdt.c
> index 381d48f..8bcfffb 100644
> --- a/arm/fdt.c
> +++ b/arm/fdt.c
> @@ -114,7 +114,7 @@ static int setup_fdt(struct kvm *kvm)
>  {
>  	struct device_header *dev_hdr;
>  	u8 staging_fdt[FDT_MAX_SIZE];
> -	u32 gic_phandle		= fdt__alloc_phandle();
> +	u32 gic_phandle		= fdt__get_phandle(PHANDLE_GIC);
>  	u64 mem_reg_prop[]	= {
>  		cpu_to_fdt64(kvm->arch.memory_guest_start),
>  		cpu_to_fdt64(kvm->ram_size),
> diff --git a/arm/gic.c b/arm/gic.c
> index d6d6dd0..b60437e 100644
> --- a/arm/gic.c
> +++ b/arm/gic.c
> @@ -222,7 +222,7 @@ void gic__generate_fdt_nodes(void *fdt, u32 phandle, enum irqchip_type type)
>  	_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)));
> -	_FDT(fdt_property_cell(fdt, "phandle", phandle));
> +	_FDT(fdt_property_cell(fdt, "phandle", fdt__get_phandle(PHANDLE_GIC)));
>  	_FDT(fdt_end_node(fdt));
>  }
>  
> diff --git a/include/kvm/fdt.h b/include/kvm/fdt.h
> index 53d85a4..cd2bb72 100644
> --- a/include/kvm/fdt.h
> +++ b/include/kvm/fdt.h
> @@ -8,6 +8,10 @@
>  #include <linux/types.h>
>  
>  #define FDT_MAX_SIZE	0x10000
> +#define FDT_INVALID_PHANDLE 0
> +#define FDT_IS_VALID_PHANDLE(phandle) ((phandle) != FDT_INVALID_PHANDLE)
> +
> +enum phandles {PHANDLE_GIC, PHANDLES_MAX};
>  
>  /* Those definitions are generic FDT values for specifying IRQ
>   * types and are used in the Linux kernel internally as well as in
> @@ -33,10 +37,6 @@ enum irq_type {
>  		}							\
>  	} while (0)
>  
> -static inline u32 fdt__alloc_phandle(void)
> -{
> -	static u32 phandle = 0;
> -	return ++phandle;
> -}
> +u32 fdt__get_phandle(enum phandles phandle);
>  
>  #endif /* KVM__FDT_H */
> diff --git a/kvm-fdt.c b/kvm-fdt.c
> new file mode 100644
> index 0000000..d05f3fe
> --- /dev/null
> +++ b/kvm-fdt.c
> @@ -0,0 +1,26 @@
> +/*
> + * Commonly used FDT functions.
> + */
> +
> +#include <stdio.h>
> +#include "kvm/fdt.h"
> +#include "kvm/util.h"
> +
> +u32 phandles[PHANDLES_MAX] = {};

It is a bit weird that you're initializing this to zero...

> +u32 next_phandle = 1;
> +
> +u32 fdt__get_phandle(enum phandles phandle)
> +{
> +	u32 ret;
> +
> +	if (phandle >= PHANDLES_MAX)
> +		return FDT_INVALID_PHANDLE;
> +
> +	ret = phandles[phandle];
> +	if (ret == FDT_INVALID_PHANDLE) {

and yet test against a #define that isn't the initializer.

Also, given that fdt__get_phandle() can now fail by returning
FDT_INVALID_HANDLE, maybe we should abort instead of returning something
that is definitely wrong and use it blindly (which is going to be fun to
debug...).


> +		ret = next_phandle++;
> +		phandles[phandle] = ret;
> +	}
> +
> +	return ret;
> +}
> 

Thanks,

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

[PATCH v8 01/16] FDT: introduce global phandle allocation

[Marc Zyngier]

On 04/11/16 17:31, Andre Przywara wrote:
> Allocating an FDT phandle (a unique identifier) using a static
> variable in a static inline function defined in a header file works
> only if all users are in the same source file. So trying to allocate
> a handle from two different compilation units fails.
> Introduce global phandle allocation and reference code to properly
> allocate unique phandles.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  Makefile          |  1 +
>  arm/fdt.c         |  2 +-
>  arm/gic.c         |  2 +-
>  include/kvm/fdt.h | 10 +++++-----
>  kvm-fdt.c         | 26 ++++++++++++++++++++++++++
>  5 files changed, 34 insertions(+), 7 deletions(-)
>  create mode 100644 kvm-fdt.c
> 
> diff --git a/Makefile b/Makefile
> index 1f0196f..e4a4002 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -98,6 +98,7 @@ OBJS	+= kvm-ipc.o
>  OBJS	+= builtin-sandbox.o
>  OBJS	+= virtio/mmio.o
>  OBJS	+= hw/i8042.o
> +OBJS	+= kvm-fdt.o
>  
>  # Translate uname -m into ARCH string
>  ARCH ?= $(shell uname -m | sed -e s/i.86/i386/ -e s/ppc.*/powerpc/ \
> diff --git a/arm/fdt.c b/arm/fdt.c
> index 381d48f..8bcfffb 100644
> --- a/arm/fdt.c
> +++ b/arm/fdt.c
> @@ -114,7 +114,7 @@ static int setup_fdt(struct kvm *kvm)
>  {
>  	struct device_header *dev_hdr;
>  	u8 staging_fdt[FDT_MAX_SIZE];
> -	u32 gic_phandle		= fdt__alloc_phandle();
> +	u32 gic_phandle		= fdt__get_phandle(PHANDLE_GIC);
>  	u64 mem_reg_prop[]	= {
>  		cpu_to_fdt64(kvm->arch.memory_guest_start),
>  		cpu_to_fdt64(kvm->ram_size),
> diff --git a/arm/gic.c b/arm/gic.c
> index d6d6dd0..b60437e 100644
> --- a/arm/gic.c
> +++ b/arm/gic.c
> @@ -222,7 +222,7 @@ void gic__generate_fdt_nodes(void *fdt, u32 phandle, enum irqchip_type type)
>  	_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)));
> -	_FDT(fdt_property_cell(fdt, "phandle", phandle));
> +	_FDT(fdt_property_cell(fdt, "phandle", fdt__get_phandle(PHANDLE_GIC)));
>  	_FDT(fdt_end_node(fdt));
>  }
>  
> diff --git a/include/kvm/fdt.h b/include/kvm/fdt.h
> index 53d85a4..cd2bb72 100644
> --- a/include/kvm/fdt.h
> +++ b/include/kvm/fdt.h
> @@ -8,6 +8,10 @@
>  #include <linux/types.h>
>  
>  #define FDT_MAX_SIZE	0x10000
> +#define FDT_INVALID_PHANDLE 0
> +#define FDT_IS_VALID_PHANDLE(phandle) ((phandle) != FDT_INVALID_PHANDLE)
> +
> +enum phandles {PHANDLE_GIC, PHANDLES_MAX};

Another nit here: PHANDLE_GIC is pretty much ARM-specific, while FDT is
supposed to be generic. Can't we move the enum to be architecture
specific and not put this in an architecture agnostic one?

Thanks,

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

[PATCH v8 02/16] arm: use new phandle allocation functions

[Marc Zyngier]

On 04/11/16 17:31, Andre Przywara wrote:
> To refer to the GIC FDT node, we used to pass the GIC phandle to most
> of the functions dealing with FDT nodes.
> Since we now have a global phandle reference, use that to refer to the
> GIC handle in various places and get rid of the now unneeded parameter
> passing.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Despite the comments on the previous patch, this one is a good cleanup.

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

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

[PATCH v8 03/16] irq: move IRQ routing into irq.c

[Marc Zyngier]

On 04/11/16 17:31, Andre Przywara wrote:
> The current IRQ routing code in x86/irq.c is mostly implementing a
> generic KVM interface which other architectures may use too.
> Move the code to set up an MSI route into the generic irq.c file and
> guard it with the KVM_CAP_IRQ_ROUTING capability to return an error
> if the kernel does not support interrupt routing.
> This also removes the dummy implementations for all other
> architectures and only leaves the x86 specific code in x86/irq.c.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

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

[PATCH v8 04/16] MSI-X: update GSI routing after changed MSI-X configuration

[Marc Zyngier]

On 04/11/16 17:31, Andre Przywara wrote:
> When we set up GSI routing to map MSIs to KVM's GSI numbers, we
> write the current device's MSI setup into the kernel routing table.
> However the device driver in the guest can use PCI configuration space
> accesses to change the MSI configuration (address and/or payload data).
> Whenever this happens after we have setup the routing table already,
> we must amend the previously sent data.
> So when MSI-X PCI config space accesses write address or payload,
> find the associated GSI number and the matching routing table entry
> and update the kernel routing table (only if the data has changed).
> 
> This fixes vhost-net, where the queue's IRQFD was setup before the
> MSI vectors.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  include/kvm/irq.h |  1 +
>  irq.c             | 31 +++++++++++++++++++++++++++++++
>  virtio/pci.c      | 36 +++++++++++++++++++++++++++++++++---
>  3 files changed, 65 insertions(+), 3 deletions(-)
> 
> diff --git a/include/kvm/irq.h b/include/kvm/irq.h
> index bb71521..f35eb7e 100644
> --- a/include/kvm/irq.h
> +++ b/include/kvm/irq.h
> @@ -21,5 +21,6 @@ int irq__exit(struct kvm *kvm);
>  
>  int irq__allocate_routing_entry(void);
>  int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg);
> +void irq__update_msix_route(struct kvm *kvm, u32 gsi, struct msi_msg *msg);
>  
>  #endif
> diff --git a/irq.c b/irq.c
> index a742aa2..895e5eb 100644
> --- a/irq.c
> +++ b/irq.c
> @@ -93,6 +93,37 @@ int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg)
>  	return next_gsi++;
>  }
>  
> +static bool update_data(u32 *ptr, u32 newdata)
> +{
> +	if (*ptr == newdata)
> +		return false;
> +
> +	*ptr = newdata;
> +	return true;
> +}
> +
> +void irq__update_msix_route(struct kvm *kvm, u32 gsi, struct msi_msg *msg)
> +{
> +	struct kvm_irq_routing_msi *entry;
> +	unsigned int i;
> +	bool changed;
> +
> +	for (i = 0; i < irq_routing->nr; i++)
> +		if (gsi == irq_routing->entries[i].gsi)
> +			break;
> +	if (i == irq_routing->nr)
> +		return;
> +
> +	entry = &irq_routing->entries[i].u.msi;
> +
> +	changed  = update_data(&entry->address_hi, msg->address_hi);
> +	changed |= update_data(&entry->address_lo, msg->address_lo);
> +	changed |= update_data(&entry->data, msg->data);
> +
> +	if (changed)
> +		ioctl(kvm->vm_fd, KVM_SET_GSI_ROUTING, irq_routing);

Check the return value and let the caller know if something has failed?

> +}
> +
>  int __attribute__((weak)) irq__exit(struct kvm *kvm)
>  {
>  	free(irq_routing);
> diff --git a/virtio/pci.c b/virtio/pci.c
> index 072e5b7..b3b4aac 100644
> --- a/virtio/pci.c
> +++ b/virtio/pci.c
> @@ -152,6 +152,30 @@ static bool virtio_pci__io_in(struct ioport *ioport, struct kvm_cpu *vcpu, u16 p
>  	return ret;
>  }
>  
> +static void update_msix_map(struct virtio_pci *vpci,
> +			    struct msix_table *msix_entry, u32 vecnum)
> +{
> +	u32 gsi, i;
> +
> +	/* Find the GSI number used for that vector */
> +	if (vecnum == vpci->config_vector) {
> +		gsi = vpci->config_gsi;
> +	} else {
> +		for (i = 0; i < VIRTIO_PCI_MAX_VQ; i++)
> +			if (vpci->vq_vector[i] == vecnum)
> +				break;
> +		if (i == VIRTIO_PCI_MAX_VQ)
> +			return;
> +		gsi = vpci->gsis[i];
> +	}
> +
> +	if (gsi == 0)
> +		return;
> +
> +	msix_entry = &msix_entry[vecnum];
> +	irq__update_msix_route(vpci->kvm, gsi, &msix_entry->msg);
> +}
> +
>  static bool virtio_pci__specific_io_out(struct kvm *kvm, struct virtio_device *vdev, u16 port,
>  					void *data, int size, int offset)
>  {
> @@ -270,10 +294,16 @@ static void virtio_pci__msix_mmio_callback(struct kvm_cpu *vcpu,
>  		offset	= vpci->msix_io_block;
>  	}
>  
> -	if (is_write)
> -		memcpy(table + addr - offset, data, len);
> -	else
> +	if (!is_write) {
>  		memcpy(data, table + addr - offset, len);
> +		return;
> +	}
> +
> +	memcpy(table + addr - offset, data, len);
> +
> +	/* Did we just update the address or payload? */
> +	if (addr % 0x10 < 0xc)
> +		update_msix_map(vpci, table, (addr - offset) / 16);

Where are these constants coming from? Please stick to either decimal or
hex...

>  }
>  
>  static void virtio_pci__signal_msi(struct kvm *kvm, struct virtio_pci *vpci, int vec)
> 

Thanks,

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

[PATCH v8 01/16] FDT: introduce global phandle allocation

[Andre Przywara]

Hi Marc,

On 09/12/16 11:55, Marc Zyngier wrote:
> Hi Andre,
> 
> On 04/11/16 17:31, Andre Przywara wrote:
>> Allocating an FDT phandle (a unique identifier) using a static
>> variable in a static inline function defined in a header file works
>> only if all users are in the same source file. So trying to allocate
>> a handle from two different compilation units fails.
>> Introduce global phandle allocation and reference code to properly
>> allocate unique phandles.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>  Makefile          |  1 +
>>  arm/fdt.c         |  2 +-
>>  arm/gic.c         |  2 +-
>>  include/kvm/fdt.h | 10 +++++-----
>>  kvm-fdt.c         | 26 ++++++++++++++++++++++++++
>>  5 files changed, 34 insertions(+), 7 deletions(-)
>>  create mode 100644 kvm-fdt.c
>>
>> diff --git a/Makefile b/Makefile
>> index 1f0196f..e4a4002 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -98,6 +98,7 @@ OBJS	+= kvm-ipc.o
>>  OBJS	+= builtin-sandbox.o
>>  OBJS	+= virtio/mmio.o
>>  OBJS	+= hw/i8042.o
>> +OBJS	+= kvm-fdt.o
>>  
>>  # Translate uname -m into ARCH string
>>  ARCH ?= $(shell uname -m | sed -e s/i.86/i386/ -e s/ppc.*/powerpc/ \
>> diff --git a/arm/fdt.c b/arm/fdt.c
>> index 381d48f..8bcfffb 100644
>> --- a/arm/fdt.c
>> +++ b/arm/fdt.c
>> @@ -114,7 +114,7 @@ static int setup_fdt(struct kvm *kvm)
>>  {
>>  	struct device_header *dev_hdr;
>>  	u8 staging_fdt[FDT_MAX_SIZE];
>> -	u32 gic_phandle		= fdt__alloc_phandle();
>> +	u32 gic_phandle		= fdt__get_phandle(PHANDLE_GIC);
>>  	u64 mem_reg_prop[]	= {
>>  		cpu_to_fdt64(kvm->arch.memory_guest_start),
>>  		cpu_to_fdt64(kvm->ram_size),
>> diff --git a/arm/gic.c b/arm/gic.c
>> index d6d6dd0..b60437e 100644
>> --- a/arm/gic.c
>> +++ b/arm/gic.c
>> @@ -222,7 +222,7 @@ void gic__generate_fdt_nodes(void *fdt, u32 phandle, enum irqchip_type type)
>>  	_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)));
>> -	_FDT(fdt_property_cell(fdt, "phandle", phandle));
>> +	_FDT(fdt_property_cell(fdt, "phandle", fdt__get_phandle(PHANDLE_GIC)));
>>  	_FDT(fdt_end_node(fdt));
>>  }
>>  
>> diff --git a/include/kvm/fdt.h b/include/kvm/fdt.h
>> index 53d85a4..cd2bb72 100644
>> --- a/include/kvm/fdt.h
>> +++ b/include/kvm/fdt.h
>> @@ -8,6 +8,10 @@
>>  #include <linux/types.h>
>>  
>>  #define FDT_MAX_SIZE	0x10000
>> +#define FDT_INVALID_PHANDLE 0
>> +#define FDT_IS_VALID_PHANDLE(phandle) ((phandle) != FDT_INVALID_PHANDLE)
>> +
>> +enum phandles {PHANDLE_GIC, PHANDLES_MAX};
>>  
>>  /* Those definitions are generic FDT values for specifying IRQ
>>   * types and are used in the Linux kernel internally as well as in
>> @@ -33,10 +37,6 @@ enum irq_type {
>>  		}							\
>>  	} while (0)
>>  
>> -static inline u32 fdt__alloc_phandle(void)
>> -{
>> -	static u32 phandle = 0;
>> -	return ++phandle;
>> -}
>> +u32 fdt__get_phandle(enum phandles phandle);
>>  
>>  #endif /* KVM__FDT_H */
>> diff --git a/kvm-fdt.c b/kvm-fdt.c
>> new file mode 100644
>> index 0000000..d05f3fe
>> --- /dev/null
>> +++ b/kvm-fdt.c
>> @@ -0,0 +1,26 @@
>> +/*
>> + * Commonly used FDT functions.
>> + */
>> +
>> +#include <stdio.h>
>> +#include "kvm/fdt.h"
>> +#include "kvm/util.h"
>> +
>> +u32 phandles[PHANDLES_MAX] = {};
> 
> It is a bit weird that you're initializing this to zero...
> 
>> +u32 next_phandle = 1;
>> +
>> +u32 fdt__get_phandle(enum phandles phandle)
>> +{
>> +	u32 ret;
>> +
>> +	if (phandle >= PHANDLES_MAX)
>> +		return FDT_INVALID_PHANDLE;
>> +
>> +	ret = phandles[phandle];
>> +	if (ret == FDT_INVALID_PHANDLE) {
> 
> and yet test against a #define that isn't the initializer.

Well, yes. The problem is that AFAIK you cannot initialize an array
easily with all the values getting set to something other than zero.
So I could write
	u32 phandles[PHANDLES_MAX] = {FDT_INVALID_PHANDLE};
above, but as that would only set the first member to
FDT_INVALID_PHANDLE (and all the others to 0), so it would rely on the
define actually being zero to work reliably. So my thought was to avoid
readers falling into this trap by not specifying the reset value
explicitly. Also that's the reason that 0 is the invalid value, which I
don't like too much, tbh.

So shall I make this a comment then?

Or do I miss something here?

> Also, given that fdt__get_phandle() can now fail by returning
> FDT_INVALID_HANDLE, maybe we should abort instead of returning something
> that is definitely wrong and use it blindly (which is going to be fun to
> debug...).

Yes, good point. Given that this hints at an internal error, die() seems
to be the appropriate action here.

Cheers,
Andre.

> 
> 
>> +		ret = next_phandle++;
>> +		phandles[phandle] = ret;
>> +	}
>> +
>> +	return ret;
>> +}
>>
> 
> Thanks,
> 
> 	M.
> 

[PATCH v8 01/16] FDT: introduce global phandle allocation

[Andre Przywara]

Hi Marc,

On 09/12/16 12:03, Marc Zyngier wrote:
> On 04/11/16 17:31, Andre Przywara wrote:
>> Allocating an FDT phandle (a unique identifier) using a static
>> variable in a static inline function defined in a header file works
>> only if all users are in the same source file. So trying to allocate
>> a handle from two different compilation units fails.
>> Introduce global phandle allocation and reference code to properly
>> allocate unique phandles.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>  Makefile          |  1 +
>>  arm/fdt.c         |  2 +-
>>  arm/gic.c         |  2 +-
>>  include/kvm/fdt.h | 10 +++++-----
>>  kvm-fdt.c         | 26 ++++++++++++++++++++++++++
>>  5 files changed, 34 insertions(+), 7 deletions(-)
>>  create mode 100644 kvm-fdt.c
>>
>> diff --git a/Makefile b/Makefile
>> index 1f0196f..e4a4002 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -98,6 +98,7 @@ OBJS	+= kvm-ipc.o
>>  OBJS	+= builtin-sandbox.o
>>  OBJS	+= virtio/mmio.o
>>  OBJS	+= hw/i8042.o
>> +OBJS	+= kvm-fdt.o
>>  
>>  # Translate uname -m into ARCH string
>>  ARCH ?= $(shell uname -m | sed -e s/i.86/i386/ -e s/ppc.*/powerpc/ \
>> diff --git a/arm/fdt.c b/arm/fdt.c
>> index 381d48f..8bcfffb 100644
>> --- a/arm/fdt.c
>> +++ b/arm/fdt.c
>> @@ -114,7 +114,7 @@ static int setup_fdt(struct kvm *kvm)
>>  {
>>  	struct device_header *dev_hdr;
>>  	u8 staging_fdt[FDT_MAX_SIZE];
>> -	u32 gic_phandle		= fdt__alloc_phandle();
>> +	u32 gic_phandle		= fdt__get_phandle(PHANDLE_GIC);
>>  	u64 mem_reg_prop[]	= {
>>  		cpu_to_fdt64(kvm->arch.memory_guest_start),
>>  		cpu_to_fdt64(kvm->ram_size),
>> diff --git a/arm/gic.c b/arm/gic.c
>> index d6d6dd0..b60437e 100644
>> --- a/arm/gic.c
>> +++ b/arm/gic.c
>> @@ -222,7 +222,7 @@ void gic__generate_fdt_nodes(void *fdt, u32 phandle, enum irqchip_type type)
>>  	_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)));
>> -	_FDT(fdt_property_cell(fdt, "phandle", phandle));
>> +	_FDT(fdt_property_cell(fdt, "phandle", fdt__get_phandle(PHANDLE_GIC)));
>>  	_FDT(fdt_end_node(fdt));
>>  }
>>  
>> diff --git a/include/kvm/fdt.h b/include/kvm/fdt.h
>> index 53d85a4..cd2bb72 100644
>> --- a/include/kvm/fdt.h
>> +++ b/include/kvm/fdt.h
>> @@ -8,6 +8,10 @@
>>  #include <linux/types.h>
>>  
>>  #define FDT_MAX_SIZE	0x10000
>> +#define FDT_INVALID_PHANDLE 0
>> +#define FDT_IS_VALID_PHANDLE(phandle) ((phandle) != FDT_INVALID_PHANDLE)
>> +
>> +enum phandles {PHANDLE_GIC, PHANDLES_MAX};
> 
> Another nit here: PHANDLE_GIC is pretty much ARM-specific, while FDT is
> supposed to be generic. Can't we move the enum to be architecture
> specific and not put this in an architecture agnostic one?

Yes, that indeed makes sense.
This requires an include file for every architecture, but well ...

Cheers,
Andre.

[PATCH v8 04/16] MSI-X: update GSI routing after changed MSI-X configuration

[Andre Przywara]

Hi,

On 09/12/16 17:13, Marc Zyngier wrote:
> On 04/11/16 17:31, Andre Przywara wrote:
>> When we set up GSI routing to map MSIs to KVM's GSI numbers, we
>> write the current device's MSI setup into the kernel routing table.
>> However the device driver in the guest can use PCI configuration space
>> accesses to change the MSI configuration (address and/or payload data).
>> Whenever this happens after we have setup the routing table already,
>> we must amend the previously sent data.
>> So when MSI-X PCI config space accesses write address or payload,
>> find the associated GSI number and the matching routing table entry
>> and update the kernel routing table (only if the data has changed).
>>
>> This fixes vhost-net, where the queue's IRQFD was setup before the
>> MSI vectors.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>  include/kvm/irq.h |  1 +
>>  irq.c             | 31 +++++++++++++++++++++++++++++++
>>  virtio/pci.c      | 36 +++++++++++++++++++++++++++++++++---
>>  3 files changed, 65 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/kvm/irq.h b/include/kvm/irq.h
>> index bb71521..f35eb7e 100644
>> --- a/include/kvm/irq.h
>> +++ b/include/kvm/irq.h
>> @@ -21,5 +21,6 @@ int irq__exit(struct kvm *kvm);
>>  
>>  int irq__allocate_routing_entry(void);
>>  int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg);
>> +void irq__update_msix_route(struct kvm *kvm, u32 gsi, struct msi_msg *msg);
>>  
>>  #endif
>> diff --git a/irq.c b/irq.c
>> index a742aa2..895e5eb 100644
>> --- a/irq.c
>> +++ b/irq.c
>> @@ -93,6 +93,37 @@ int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg)
>>  	return next_gsi++;
>>  }
>>  
>> +static bool update_data(u32 *ptr, u32 newdata)
>> +{
>> +	if (*ptr == newdata)
>> +		return false;
>> +
>> +	*ptr = newdata;
>> +	return true;
>> +}
>> +
>> +void irq__update_msix_route(struct kvm *kvm, u32 gsi, struct msi_msg *msg)
>> +{
>> +	struct kvm_irq_routing_msi *entry;
>> +	unsigned int i;
>> +	bool changed;
>> +
>> +	for (i = 0; i < irq_routing->nr; i++)
>> +		if (gsi == irq_routing->entries[i].gsi)
>> +			break;
>> +	if (i == irq_routing->nr)
>> +		return;
>> +
>> +	entry = &irq_routing->entries[i].u.msi;
>> +
>> +	changed  = update_data(&entry->address_hi, msg->address_hi);
>> +	changed |= update_data(&entry->address_lo, msg->address_lo);
>> +	changed |= update_data(&entry->data, msg->data);
>> +
>> +	if (changed)
>> +		ioctl(kvm->vm_fd, KVM_SET_GSI_ROUTING, irq_routing);
> 
> Check the return value and let the caller know if something has failed?

As the caller is a void function and the call chain for this originates
in an MMIO access triggered by the guest (update MSI information in the
PCI config space), I guess again die() would be the appropriate action here?

> 
>> +}
>> +
>>  int __attribute__((weak)) irq__exit(struct kvm *kvm)
>>  {
>>  	free(irq_routing);
>> diff --git a/virtio/pci.c b/virtio/pci.c
>> index 072e5b7..b3b4aac 100644
>> --- a/virtio/pci.c
>> +++ b/virtio/pci.c
>> @@ -152,6 +152,30 @@ static bool virtio_pci__io_in(struct ioport *ioport, struct kvm_cpu *vcpu, u16 p
>>  	return ret;
>>  }
>>  
>> +static void update_msix_map(struct virtio_pci *vpci,
>> +			    struct msix_table *msix_entry, u32 vecnum)
>> +{
>> +	u32 gsi, i;
>> +
>> +	/* Find the GSI number used for that vector */
>> +	if (vecnum == vpci->config_vector) {
>> +		gsi = vpci->config_gsi;
>> +	} else {
>> +		for (i = 0; i < VIRTIO_PCI_MAX_VQ; i++)
>> +			if (vpci->vq_vector[i] == vecnum)
>> +				break;
>> +		if (i == VIRTIO_PCI_MAX_VQ)
>> +			return;
>> +		gsi = vpci->gsis[i];
>> +	}
>> +
>> +	if (gsi == 0)
>> +		return;
>> +
>> +	msix_entry = &msix_entry[vecnum];
>> +	irq__update_msix_route(vpci->kvm, gsi, &msix_entry->msg);
>> +}
>> +
>>  static bool virtio_pci__specific_io_out(struct kvm *kvm, struct virtio_device *vdev, u16 port,
>>  					void *data, int size, int offset)
>>  {
>> @@ -270,10 +294,16 @@ static void virtio_pci__msix_mmio_callback(struct kvm_cpu *vcpu,
>>  		offset	= vpci->msix_io_block;
>>  	}
>>  
>> -	if (is_write)
>> -		memcpy(table + addr - offset, data, len);
>> -	else
>> +	if (!is_write) {
>>  		memcpy(data, table + addr - offset, len);
>> +		return;
>> +	}
>> +
>> +	memcpy(table + addr - offset, data, len);
>> +
>> +	/* Did we just update the address or payload? */
>> +	if (addr % 0x10 < 0xc)
>> +		update_msix_map(vpci, table, (addr - offset) / 16);
> 
> Where are these constants coming from? Please stick to either decimal or
> hex...

Sure, seems to be a leftover from my initial hacking approach. Thanks
for spotting that.

Cheers,
Andre.

> 
>>  }
>>  
>>  static void virtio_pci__signal_msi(struct kvm *kvm, struct virtio_pci *vpci, int vec)
>>
> 
> Thanks,
> 
> 	M.
> 

[PATCH v8 01/16] FDT: introduce global phandle allocation

[Andrew Jones]

On Mon, Dec 19, 2016 at 06:43:29PM +0000, Andre Przywara wrote:
> Well, yes. The problem is that AFAIK you cannot initialize an array
> easily with all the values getting set to something other than zero.

u32 phandles[PHANDLES_MAX] = { [0 ... PHANDLES_MAX-1] = FDT_INVALID_PHANDLE };

(Unrelated, but could you please add format.subjectprefix = PATCH kvmtool
 to your git config. I just suggested via a kvmtool README patch...)

Thanks,
drew

[PATCH v8 01/16] FDT: introduce global phandle allocation

[André Przywara]

Hi Marc, Will,

On 09/12/16 12:03, Marc Zyngier wrote:
> On 04/11/16 17:31, Andre Przywara wrote:
>> Allocating an FDT phandle (a unique identifier) using a static
>> variable in a static inline function defined in a header file works
>> only if all users are in the same source file. So trying to allocate
>> a handle from two different compilation units fails.
>> Introduce global phandle allocation and reference code to properly
>> allocate unique phandles.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>  Makefile          |  1 +
>>  arm/fdt.c         |  2 +-
>>  arm/gic.c         |  2 +-
>>  include/kvm/fdt.h | 10 +++++-----
>>  kvm-fdt.c         | 26 ++++++++++++++++++++++++++
>>  5 files changed, 34 insertions(+), 7 deletions(-)
>>  create mode 100644 kvm-fdt.c
>>
>> diff --git a/Makefile b/Makefile
>> index 1f0196f..e4a4002 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -98,6 +98,7 @@ OBJS	+= kvm-ipc.o
>>  OBJS	+= builtin-sandbox.o
>>  OBJS	+= virtio/mmio.o
>>  OBJS	+= hw/i8042.o
>> +OBJS	+= kvm-fdt.o
>>  
>>  # Translate uname -m into ARCH string
>>  ARCH ?= $(shell uname -m | sed -e s/i.86/i386/ -e s/ppc.*/powerpc/ \
>> diff --git a/arm/fdt.c b/arm/fdt.c
>> index 381d48f..8bcfffb 100644
>> --- a/arm/fdt.c
>> +++ b/arm/fdt.c
>> @@ -114,7 +114,7 @@ static int setup_fdt(struct kvm *kvm)
>>  {
>>  	struct device_header *dev_hdr;
>>  	u8 staging_fdt[FDT_MAX_SIZE];
>> -	u32 gic_phandle		= fdt__alloc_phandle();
>> +	u32 gic_phandle		= fdt__get_phandle(PHANDLE_GIC);
>>  	u64 mem_reg_prop[]	= {
>>  		cpu_to_fdt64(kvm->arch.memory_guest_start),
>>  		cpu_to_fdt64(kvm->ram_size),
>> diff --git a/arm/gic.c b/arm/gic.c
>> index d6d6dd0..b60437e 100644
>> --- a/arm/gic.c
>> +++ b/arm/gic.c
>> @@ -222,7 +222,7 @@ void gic__generate_fdt_nodes(void *fdt, u32 phandle, enum irqchip_type type)
>>  	_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)));
>> -	_FDT(fdt_property_cell(fdt, "phandle", phandle));
>> +	_FDT(fdt_property_cell(fdt, "phandle", fdt__get_phandle(PHANDLE_GIC)));
>>  	_FDT(fdt_end_node(fdt));
>>  }
>>  
>> diff --git a/include/kvm/fdt.h b/include/kvm/fdt.h
>> index 53d85a4..cd2bb72 100644
>> --- a/include/kvm/fdt.h
>> +++ b/include/kvm/fdt.h
>> @@ -8,6 +8,10 @@
>>  #include <linux/types.h>
>>  
>>  #define FDT_MAX_SIZE	0x10000
>> +#define FDT_INVALID_PHANDLE 0
>> +#define FDT_IS_VALID_PHANDLE(phandle) ((phandle) != FDT_INVALID_PHANDLE)
>> +
>> +enum phandles {PHANDLE_GIC, PHANDLES_MAX};
> 
> Another nit here: PHANDLE_GIC is pretty much ARM-specific, while FDT is
> supposed to be generic. Can't we move the enum to be architecture
> specific and not put this in an architecture agnostic one?

So while trying to find the best possible solution for this seemingly
simple problem, I was wondering why we had this allocation function in
the first place?
It seems a bit overkill to allocate a handle if we could just go with
static values.
I changed the first two patches now to have an enum per architecture to
list all possible handles and then just using those values directly
instead of going through another layer of indirection.

So is there anything that will require dynamic phandles in the future?
This version proposed here can't really cope with them anyway and in the
moment it's just about _one_ GIC phandle and _one_ ITS phandle, so a
static assignment is much simpler.

Will we need multiple ITSes for device passthrough? Or would it just be
one ITS for all hardware devices and one for emulated virtio?

Or do I miss anything else here?

Cheers,
Andre.

> 
> Thanks,
> 
> 	M.
> 

[PATCH v8 01/16] FDT: introduce global phandle allocation

[Marc Zyngier]

On Wed, Feb 01 2017 at  4:44:19 pm GMT, Andr? Przywara <andre.przywara@arm.com> wrote:

Hi Andre,

> Hi Marc, Will,
>
> On 09/12/16 12:03, Marc Zyngier wrote:
>> On 04/11/16 17:31, Andre Przywara wrote:
>>> Allocating an FDT phandle (a unique identifier) using a static
>>> variable in a static inline function defined in a header file works
>>> only if all users are in the same source file. So trying to allocate
>>> a handle from two different compilation units fails.
>>> Introduce global phandle allocation and reference code to properly
>>> allocate unique phandles.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>> ---
>>>  Makefile          |  1 +
>>>  arm/fdt.c         |  2 +-
>>>  arm/gic.c         |  2 +-
>>>  include/kvm/fdt.h | 10 +++++-----
>>>  kvm-fdt.c         | 26 ++++++++++++++++++++++++++
>>>  5 files changed, 34 insertions(+), 7 deletions(-)
>>>  create mode 100644 kvm-fdt.c
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 1f0196f..e4a4002 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -98,6 +98,7 @@ OBJS	+= kvm-ipc.o
>>>  OBJS	+= builtin-sandbox.o
>>>  OBJS	+= virtio/mmio.o
>>>  OBJS	+= hw/i8042.o
>>> +OBJS	+= kvm-fdt.o
>>>  
>>>  # Translate uname -m into ARCH string
>>>  ARCH ?= $(shell uname -m | sed -e s/i.86/i386/ -e s/ppc.*/powerpc/ \
>>> diff --git a/arm/fdt.c b/arm/fdt.c
>>> index 381d48f..8bcfffb 100644
>>> --- a/arm/fdt.c
>>> +++ b/arm/fdt.c
>>> @@ -114,7 +114,7 @@ static int setup_fdt(struct kvm *kvm)
>>>  {
>>>  	struct device_header *dev_hdr;
>>>  	u8 staging_fdt[FDT_MAX_SIZE];
>>> -	u32 gic_phandle		= fdt__alloc_phandle();
>>> +	u32 gic_phandle		= fdt__get_phandle(PHANDLE_GIC);
>>>  	u64 mem_reg_prop[]	= {
>>>  		cpu_to_fdt64(kvm->arch.memory_guest_start),
>>>  		cpu_to_fdt64(kvm->ram_size),
>>> diff --git a/arm/gic.c b/arm/gic.c
>>> index d6d6dd0..b60437e 100644
>>> --- a/arm/gic.c
>>> +++ b/arm/gic.c
>>> @@ -222,7 +222,7 @@ void gic__generate_fdt_nodes(void *fdt, u32 phandle, enum irqchip_type type)
>>>  	_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)));
>>> -	_FDT(fdt_property_cell(fdt, "phandle", phandle));
>>> +	_FDT(fdt_property_cell(fdt, "phandle", fdt__get_phandle(PHANDLE_GIC)));
>>>  	_FDT(fdt_end_node(fdt));
>>>  }
>>>  
>>> diff --git a/include/kvm/fdt.h b/include/kvm/fdt.h
>>> index 53d85a4..cd2bb72 100644
>>> --- a/include/kvm/fdt.h
>>> +++ b/include/kvm/fdt.h
>>> @@ -8,6 +8,10 @@
>>>  #include <linux/types.h>
>>>  
>>>  #define FDT_MAX_SIZE	0x10000
>>> +#define FDT_INVALID_PHANDLE 0
>>> +#define FDT_IS_VALID_PHANDLE(phandle) ((phandle) != FDT_INVALID_PHANDLE)
>>> +
>>> +enum phandles {PHANDLE_GIC, PHANDLES_MAX};
>> 
>> Another nit here: PHANDLE_GIC is pretty much ARM-specific, while FDT is
>> supposed to be generic. Can't we move the enum to be architecture
>> specific and not put this in an architecture agnostic one?
>
> So while trying to find the best possible solution for this seemingly
> simple problem, I was wondering why we had this allocation function in
> the first place?
> It seems a bit overkill to allocate a handle if we could just go with
> static values.
> I changed the first two patches now to have an enum per architecture to
> list all possible handles and then just using those values directly
> instead of going through another layer of indirection.

Yes, that probably make sense, at least for the time being.

> So is there anything that will require dynamic phandles in the future?
> This version proposed here can't really cope with them anyway and in the
> moment it's just about _one_ GIC phandle and _one_ ITS phandle, so a
> static assignment is much simpler.
>
> Will we need multiple ITSes for device passthrough? Or would it just be
> one ITS for all hardware devices and one for emulated virtio?

Having multiple ITSes is definitely on the cards (pass-through and
emulated devices are one case, where once of them would be driven using
GICv4 and the other be purely virtual). This probably would translate
into having multiple PCIe host controllers.

So maybe we don't need the full breath of an allocator yet, but I reckon
that we shouldn't pretend that there is no use for it, forever.

Thanks,

	M.
-- 
Jazz is not dead, it just smell funny.

[PATCH v8 01/16] FDT: introduce global phandle allocation

[Andre Przywara]

Hi Marc,

On 01/02/17 17:13, Marc Zyngier wrote:
> On Wed, Feb 01 2017 at  4:44:19 pm GMT, Andr? Przywara <andre.przywara@arm.com> wrote:
> 
> Hi Andre,
> 
>> Hi Marc, Will,
>>
>> On 09/12/16 12:03, Marc Zyngier wrote:
>>> On 04/11/16 17:31, Andre Przywara wrote:
>>>> Allocating an FDT phandle (a unique identifier) using a static
>>>> variable in a static inline function defined in a header file works
>>>> only if all users are in the same source file. So trying to allocate
>>>> a handle from two different compilation units fails.
>>>> Introduce global phandle allocation and reference code to properly
>>>> allocate unique phandles.
>>>>
>>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>>> ---
>>>>  Makefile          |  1 +
>>>>  arm/fdt.c         |  2 +-
>>>>  arm/gic.c         |  2 +-
>>>>  include/kvm/fdt.h | 10 +++++-----
>>>>  kvm-fdt.c         | 26 ++++++++++++++++++++++++++
>>>>  5 files changed, 34 insertions(+), 7 deletions(-)
>>>>  create mode 100644 kvm-fdt.c
>>>>
>>>> diff --git a/Makefile b/Makefile
>>>> index 1f0196f..e4a4002 100644
>>>> --- a/Makefile
>>>> +++ b/Makefile
>>>> @@ -98,6 +98,7 @@ OBJS	+= kvm-ipc.o
>>>>  OBJS	+= builtin-sandbox.o
>>>>  OBJS	+= virtio/mmio.o
>>>>  OBJS	+= hw/i8042.o
>>>> +OBJS	+= kvm-fdt.o
>>>>  
>>>>  # Translate uname -m into ARCH string
>>>>  ARCH ?= $(shell uname -m | sed -e s/i.86/i386/ -e s/ppc.*/powerpc/ \
>>>> diff --git a/arm/fdt.c b/arm/fdt.c
>>>> index 381d48f..8bcfffb 100644
>>>> --- a/arm/fdt.c
>>>> +++ b/arm/fdt.c
>>>> @@ -114,7 +114,7 @@ static int setup_fdt(struct kvm *kvm)
>>>>  {
>>>>  	struct device_header *dev_hdr;
>>>>  	u8 staging_fdt[FDT_MAX_SIZE];
>>>> -	u32 gic_phandle		= fdt__alloc_phandle();
>>>> +	u32 gic_phandle		= fdt__get_phandle(PHANDLE_GIC);
>>>>  	u64 mem_reg_prop[]	= {
>>>>  		cpu_to_fdt64(kvm->arch.memory_guest_start),
>>>>  		cpu_to_fdt64(kvm->ram_size),
>>>> diff --git a/arm/gic.c b/arm/gic.c
>>>> index d6d6dd0..b60437e 100644
>>>> --- a/arm/gic.c
>>>> +++ b/arm/gic.c
>>>> @@ -222,7 +222,7 @@ void gic__generate_fdt_nodes(void *fdt, u32 phandle, enum irqchip_type type)
>>>>  	_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)));
>>>> -	_FDT(fdt_property_cell(fdt, "phandle", phandle));
>>>> +	_FDT(fdt_property_cell(fdt, "phandle", fdt__get_phandle(PHANDLE_GIC)));
>>>>  	_FDT(fdt_end_node(fdt));
>>>>  }
>>>>  
>>>> diff --git a/include/kvm/fdt.h b/include/kvm/fdt.h
>>>> index 53d85a4..cd2bb72 100644
>>>> --- a/include/kvm/fdt.h
>>>> +++ b/include/kvm/fdt.h
>>>> @@ -8,6 +8,10 @@
>>>>  #include <linux/types.h>
>>>>  
>>>>  #define FDT_MAX_SIZE	0x10000
>>>> +#define FDT_INVALID_PHANDLE 0
>>>> +#define FDT_IS_VALID_PHANDLE(phandle) ((phandle) != FDT_INVALID_PHANDLE)
>>>> +
>>>> +enum phandles {PHANDLE_GIC, PHANDLES_MAX};
>>>
>>> Another nit here: PHANDLE_GIC is pretty much ARM-specific, while FDT is
>>> supposed to be generic. Can't we move the enum to be architecture
>>> specific and not put this in an architecture agnostic one?
>>
>> So while trying to find the best possible solution for this seemingly
>> simple problem, I was wondering why we had this allocation function in
>> the first place?
>> It seems a bit overkill to allocate a handle if we could just go with
>> static values.
>> I changed the first two patches now to have an enum per architecture to
>> list all possible handles and then just using those values directly
>> instead of going through another layer of indirection.
> 
> Yes, that probably make sense, at least for the time being.
> 
>> So is there anything that will require dynamic phandles in the future?
>> This version proposed here can't really cope with them anyway and in the
>> moment it's just about _one_ GIC phandle and _one_ ITS phandle, so a
>> static assignment is much simpler.
>>
>> Will we need multiple ITSes for device passthrough? Or would it just be
>> one ITS for all hardware devices and one for emulated virtio?
> 
> Having multiple ITSes is definitely on the cards (pass-through and
> emulated devices are one case, where once of them would be driven using
> GICv4 and the other be purely virtual). This probably would translate
> into having multiple PCIe host controllers.

OK, I see. At the moment the kvmtool code seems to rely on having only a
single PCI controller using a static setup. As changing this would
require some rework (for instance to allow dynamic MMIO allocation), I
would like to refrain from adding multiple ITS support prematurely now.
We should do this later in conjunction with multiple PCI controller
support, I believe.

> So maybe we don't need the full breath of an allocator yet, but I reckon
> that we shouldn't pretend that there is no use for it, forever.

OK, got it.

I will post my version with the static phandle setup, feel free to shoot
it down if this is too much of a simplification or if you have the
feeling that removing the allocation will bite us later.

Cheers,
Andre.