[PATCH 1/3] kvm tools: Introduce IRQ registry

[PATCH 1/3] kvm tools: Introduce IRQ registry

[Sasha Levin]

Instead of having static definitions of devices, Use a
dynamic registry of pci devices.

The structure is a rbtree which holds device types (net,
blk, etc). Each device entry holds a list of IRQ lines
associated with that device (pin).

Devices dynamically register upon initialization, and receive
a set of: device id, irq pin and irq line.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/Makefile               |    2 +
 tools/kvm/include/kvm/irq.h      |   24 +++++++++
 tools/kvm/include/linux/module.h |    6 ++
 tools/kvm/irq.c                  |  107 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 139 insertions(+), 0 deletions(-)
 create mode 100644 tools/kvm/include/kvm/irq.h
 create mode 100644 tools/kvm/include/linux/module.h
 create mode 100644 tools/kvm/irq.c

diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile
index 89c7e3d..fb839fc 100644
--- a/tools/kvm/Makefile
+++ b/tools/kvm/Makefile
@@ -39,6 +39,8 @@ OBJS    += kvm-run.o
 OBJS    += qcow.o
 OBJS    += mptable.o
 OBJS    += threadpool.o
+OBJS    += irq.o
+OBJS    += ../../lib/rbtree.o
 
 DEPS	:= $(patsubst %.o,%.d,$(OBJS))
 
diff --git a/tools/kvm/include/kvm/irq.h b/tools/kvm/include/kvm/irq.h
new file mode 100644
index 0000000..7a75a0c
--- /dev/null
+++ b/tools/kvm/include/kvm/irq.h
@@ -0,0 +1,24 @@
+#ifndef KVM__IRQ_H
+#define KVM__IRQ_H
+
+#include <linux/types.h>
+#include <linux/rbtree.h>
+#include <linux/list.h>
+
+struct irq_line {
+	u8			line;
+	struct list_head	node;
+};
+
+struct pci_dev {
+	struct rb_node		node;
+	u32			id;
+	u8			pin;
+	struct list_head	lines;
+};
+
+int irq__register_device(u32 dev, u8 *num, u8 *pin, u8 *line);
+
+struct rb_node *irq__get_pci_tree(void);
+
+#endif
diff --git a/tools/kvm/include/linux/module.h b/tools/kvm/include/linux/module.h
new file mode 100644
index 0000000..0e4c6a3
--- /dev/null
+++ b/tools/kvm/include/linux/module.h
@@ -0,0 +1,6 @@
+#ifndef KVM__LINUX_MODULE_H
+#define KVM__LINUX_MODULE_H
+
+#define EXPORT_SYMBOL(name)
+
+#endif
diff --git a/tools/kvm/irq.c b/tools/kvm/irq.c
new file mode 100644
index 0000000..4f4305f
--- /dev/null
+++ b/tools/kvm/irq.c
@@ -0,0 +1,107 @@
+#include "kvm/irq.h"
+
+#include <linux/types.h>
+#include <linux/rbtree.h>
+#include <linux/list.h>
+
+#include <stddef.h>
+#include <stdlib.h>
+
+static u8		next_pin	= 1;
+static u8		next_line	= 3;
+static u8		next_dev	= 1;
+static struct rb_root	pci_tree	= RB_ROOT;
+
+static struct pci_dev *search(struct rb_root *root, u32 id)
+{
+	struct rb_node *node = root->rb_node;
+
+	while (node) {
+		struct pci_dev *data = container_of(node, struct pci_dev, node);
+		int result;
+
+		result = id - data->id;
+
+		if (result < 0)
+			node = node->rb_left;
+		else if (result > 0)
+			node = node->rb_right;
+		else
+			return data;
+	}
+	return NULL;
+}
+
+static int insert(struct rb_root *root, struct pci_dev *data)
+{
+	struct rb_node **new = &(root->rb_node), *parent = NULL;
+
+	/* Figure out where to put new node */
+	while (*new) {
+		struct pci_dev *this	= container_of(*new, struct pci_dev, node);
+		int result		= data->id - this->id;
+
+		parent = *new;
+		if (result < 0)
+			new = &((*new)->rb_left);
+		else if (result > 0)
+			new = &((*new)->rb_right);
+		else
+			return 0;
+	}
+
+	/* Add new node and rebalance tree. */
+	rb_link_node(&data->node, parent, new);
+	rb_insert_color(&data->node, root);
+
+	return 1;
+}
+
+int irq__register_device(u32 dev, u8 *num, u8 *pin, u8 *line)
+{
+	struct pci_dev *node;
+
+	node = search(&pci_tree, dev);
+
+	if (!node) {
+		/* We haven't found a node - First device of it's kind */
+		node = malloc(sizeof(*node));
+		if (node == NULL)
+			return -1;
+
+		*node = (struct pci_dev) {
+			.id	= dev,
+			.pin	= next_pin++,
+		};
+
+		INIT_LIST_HEAD(&node->lines);
+
+		if (insert(&pci_tree, node) != 1) {
+			free(node);
+			return -1;
+		}
+	}
+
+	if (node) {
+		/* This device already has a pin assigned, give out a new line and device id */
+		struct irq_line *new = malloc(sizeof(*new));
+		if (new == NULL)
+			return -1;
+
+		new->line	= next_line++;
+		*line		= new->line;
+		*pin		= node->pin;
+		*num		= next_dev++;
+
+		list_add(&new->node, &node->lines);
+
+		return 0;
+	}
+
+	return -1;
+}
+
+struct rb_node *irq__get_pci_tree(void)
+{
+	return rb_first(&pci_tree);
+}
-- 
1.7.5.rc3

[PATCH 2/3] kvm tools: Dynamically add devices when creating mptable

[Sasha Levin]

Enumerate registered devices to build a complete
and updated mptable containing all registered pci
devices.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/kvm-run.c |    4 ++--
 tools/kvm/mptable.c |   30 +++++++++++-------------------
 2 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/tools/kvm/kvm-run.c b/tools/kvm/kvm-run.c
index d5a952f..bb228fe 100644
--- a/tools/kvm/kvm-run.c
+++ b/tools/kvm/kvm-run.c
@@ -446,8 +446,6 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
 
 	rtc__init();
 
-	kvm__setup_bios(kvm);
-
 	serial8250__init(kvm);
 
 	pci__init();
@@ -479,6 +477,8 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
 
 	kvm__start_timer(kvm);
 
+	kvm__setup_bios(kvm);
+
 	for (i = 0; i < nrcpus; i++) {
 		kvm_cpus[i] = kvm_cpu__init(kvm, i);
 		if (!kvm_cpus[i])
diff --git a/tools/kvm/mptable.c b/tools/kvm/mptable.c
index 5a24336..c75d9ce 100644
--- a/tools/kvm/mptable.c
+++ b/tools/kvm/mptable.c
@@ -3,6 +3,7 @@
 #include "kvm/apic.h"
 #include "kvm/mptable.h"
 #include "kvm/util.h"
+#include "kvm/irq.h"
 
 #include <linux/kernel.h>
 #include <string.h>
@@ -87,6 +88,7 @@ void mptable_setup(struct kvm *kvm, unsigned int ncpus)
 	struct mpc_bus *mpc_bus;
 	struct mpc_ioapic *mpc_ioapic;
 	struct mpc_intsrc *mpc_intsrc;
+	struct rb_node *pci_tree;
 
 	const int pcibusid = 0;
 	const int isabusid = 1;
@@ -186,30 +188,20 @@ void mptable_setup(struct kvm *kvm, unsigned int ncpus)
 	 *
 	 * Also note we use PCI irqs here, no for ISA bus yet.
 	 */
-	mpc_intsrc		= last_addr;
 
-	/* src irq = virtio console irq pin, dst irq = virtio console irq */
-	mptable_add_irq_src(mpc_intsrc, pcibusid, 2, ioapicid, 13);
-	last_addr = (void *)&mpc_intsrc[1];
-	nentries++;
+	for (pci_tree = irq__get_pci_tree(); pci_tree; pci_tree = rb_next(pci_tree)) {
+		struct pci_dev *dev = rb_entry(pci_tree, struct pci_dev, node);
+		struct irq_line *tmp;
 
-	/* Currently we define 4 possible virtio-blk devices */
-	for (i = 0; i < 4; i++) {
-		mpc_intsrc		= last_addr;
+		list_for_each_entry(tmp, &dev->lines, node) {
+			mpc_intsrc = last_addr;
 
-		/* src irq = virtio blk irq pin, dst irq = virtio blk irq */
-		mptable_add_irq_src(mpc_intsrc, pcibusid, 1, ioapicid, 9 + i);
-		last_addr = (void *)&mpc_intsrc[1];
-		nentries++;
+			mptable_add_irq_src(mpc_intsrc, pcibusid, dev->pin, ioapicid, tmp->line);
+			last_addr = (void *)&mpc_intsrc[1];
+			nentries++;
+		}
 	}
 
-	mpc_intsrc		= last_addr;
-
-	/* src irq = virtio net irq pin, dst irq = virtio net irq */
-	mptable_add_irq_src(mpc_intsrc, pcibusid, 3, ioapicid, 14);
-	last_addr = (void *)&mpc_intsrc[1];
-	nentries++;
-
 	/*
 	 * Local IRQs assignment (LINT0, LINT1)
 	 */
-- 
1.7.5.rc3

[PATCH 3/3] kvm tools: Convert virtio devices to use IRQ registry

[Sasha Levin]

Instead of using static IRQ/device data, register the device
upon initialization and use the assign parameters when issuing
IRQs.

Clean up static definitions of IRQs.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/include/kvm/virtio-pci-dev.h |   21 -------------
 tools/kvm/virtio/blk.c                 |   17 +++++++----
 tools/kvm/virtio/console.c             |   40 ++++++++++++++-----------
 tools/kvm/virtio/net.c                 |   40 ++++++++++++++-----------
 tools/kvm/virtio/rng.c                 |   50 ++++++++++++++++++--------------
 5 files changed, 85 insertions(+), 83 deletions(-)

diff --git a/tools/kvm/include/kvm/virtio-pci-dev.h b/tools/kvm/include/kvm/virtio-pci-dev.h
index 431289d..d2274d0 100644
--- a/tools/kvm/include/kvm/virtio-pci-dev.h
+++ b/tools/kvm/include/kvm/virtio-pci-dev.h
@@ -16,25 +16,4 @@
 #define PCI_SUBSYSTEM_ID_VIRTIO_CONSOLE		0x0003
 #define PCI_SUBSYSTEM_ID_VIRTIO_RNG		0x0004
 
-enum {
-	PCI_VIRTIO_BLK_DEVNUM			= 10,
-	PCI_VIRTIO_CONSOLE_DEVNUM		= 2,
-	PCI_VIRTIO_NET_DEVNUM			= 3,
-	PCI_VIRTIO_RNG_DEVNUM			= 4,
-};
-
-enum {
-	VIRTIO_BLK_PIN				= 1,
-	VIRTIO_CONSOLE_PIN			= 2,
-	VIRTIO_NET_PIN				= 3,
-	VIRTIO_RNG_PIN				= 4,
-};
-
-enum {
-	VIRTIO_RNG_IRQ				= 11,
-	VIRTIO_CONSOLE_IRQ			= 13,
-	VIRTIO_NET_IRQ				= 14,
-	VIRTIO_BLK_IRQ				= 15,
-};
-
 #endif /* VIRTIO_PCI_DEV_H_ */
diff --git a/tools/kvm/virtio/blk.c b/tools/kvm/virtio/blk.c
index 5a73dd6..accfc3e 100644
--- a/tools/kvm/virtio/blk.c
+++ b/tools/kvm/virtio/blk.c
@@ -2,7 +2,7 @@
 
 #include "kvm/virtio-pci.h"
 #include "kvm/virtio-pci-dev.h"
-
+#include "kvm/irq.h"
 #include "kvm/disk-image.h"
 #include "kvm/virtio.h"
 #include "kvm/ioport.h"
@@ -103,7 +103,7 @@ static bool virtio_blk_pci_io_in(struct kvm *self, u16 port, void *data, int siz
 		break;
 	case VIRTIO_PCI_ISR:
 		ioport__write8(data, 0x1);
-		kvm__irq_line(self, VIRTIO_BLK_IRQ + bdev->idx, 0);
+		kvm__irq_line(self, bdev->pci_device.irq_line, 0);
 		break;
 	case VIRTIO_MSI_CONFIG_VECTOR:
 		ioport__write16(data, bdev->config_vector);
@@ -167,7 +167,7 @@ static void virtio_blk_do_io(struct kvm *kvm, void *param)
 	while (virt_queue__available(vq))
 		virtio_blk_do_io_request(kvm, bdev, vq);
 
-	kvm__irq_line(kvm, VIRTIO_BLK_IRQ + bdev->idx, 1);
+	kvm__irq_line(kvm, bdev->pci_device.irq_line, 1);
 }
 
 static bool virtio_blk_pci_io_out(struct kvm *self, u16 port, void *data, int size, u32 count)
@@ -257,6 +257,7 @@ static int virtio_blk_find_empty_dev(void)
 void virtio_blk__init(struct kvm *self, struct disk_image *disk)
 {
 	u16 blk_dev_base_addr;
+	u8 dev, pin, line;
 	struct blk_dev *bdev;
 	int new_dev_idx;
 
@@ -291,12 +292,16 @@ void virtio_blk__init(struct kvm *self, struct disk_image *disk)
 			.subsys_vendor_id	= PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET,
 			.subsys_id		= PCI_SUBSYSTEM_ID_VIRTIO_BLK,
 			.bar[0]			= blk_dev_base_addr | PCI_BASE_ADDRESS_SPACE_IO,
-			.irq_pin		= VIRTIO_BLK_PIN,
-			.irq_line		= VIRTIO_BLK_IRQ + new_dev_idx,
 		},
 	};
 
-	pci__register(&bdev->pci_device, PCI_VIRTIO_BLK_DEVNUM + new_dev_idx);
+	if (irq__register_device(PCI_DEVICE_ID_VIRTIO_BLK, &dev, &pin, &line) < 0)
+		return;
+
+	bdev->pci_device.irq_pin	= pin;
+	bdev->pci_device.irq_line	= line;
+
+	pci__register(&bdev->pci_device, dev);
 
 	ioport__register(blk_dev_base_addr, &virtio_blk_io_ops, IOPORT_VIRTIO_BLK_SIZE);
 }
diff --git a/tools/kvm/virtio/console.c b/tools/kvm/virtio/console.c
index 02272b4..188227b 100644
--- a/tools/kvm/virtio/console.c
+++ b/tools/kvm/virtio/console.c
@@ -10,6 +10,7 @@
 #include "kvm/kvm.h"
 #include "kvm/pci.h"
 #include "kvm/threadpool.h"
+#include "kvm/irq.h"
 
 #include <linux/virtio_console.h>
 #include <linux/virtio_ring.h>
@@ -28,6 +29,17 @@
 #define VIRTIO_CONSOLE_RX_QUEUE		0
 #define VIRTIO_CONSOLE_TX_QUEUE		1
 
+static struct pci_device_header virtio_console_pci_device = {
+	.vendor_id		= PCI_VENDOR_ID_REDHAT_QUMRANET,
+	.device_id		= PCI_DEVICE_ID_VIRTIO_CONSOLE,
+	.header_type		= PCI_HEADER_TYPE_NORMAL,
+	.revision_id		= 0,
+	.class			= 0x078000,
+	.subsys_vendor_id	= PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET,
+	.subsys_id		= PCI_SUBSYSTEM_ID_VIRTIO_CONSOLE,
+	.bar[0]			= IOPORT_VIRTIO_CONSOLE | PCI_BASE_ADDRESS_SPACE_IO,
+};
+
 struct con_dev {
 	pthread_mutex_t			mutex;
 
@@ -73,7 +85,7 @@ static void virtio_console__inject_interrupt_callback(struct kvm *self, void *pa
 		head = virt_queue__get_iov(vq, iov, &out, &in, self);
 		len = term_getc_iov(CONSOLE_VIRTIO, iov, in);
 		virt_queue__set_used_elem(vq, head, len);
-		kvm__irq_line(self, VIRTIO_CONSOLE_IRQ, 1);
+		kvm__irq_line(self, virtio_console_pci_device.irq_line, 1);
 	}
 
 	mutex_unlock(&cdev.mutex);
@@ -128,7 +140,7 @@ static bool virtio_console_pci_io_in(struct kvm *self, u16 port, void *data, int
 		break;
 	case VIRTIO_PCI_ISR:
 		ioport__write8(data, 0x1);
-		kvm__irq_line(self, VIRTIO_CONSOLE_IRQ, 0);
+		kvm__irq_line(self, virtio_console_pci_device.irq_line, 0);
 		break;
 	case VIRTIO_MSI_CONFIG_VECTOR:
 		ioport__write16(data, cdev.config_vector);
@@ -158,7 +170,7 @@ static void virtio_console_handle_callback(struct kvm *self, void *param)
 		virt_queue__set_used_elem(vq, head, len);
 	}
 
-	kvm__irq_line(self, VIRTIO_CONSOLE_IRQ, 1);
+	kvm__irq_line(self, virtio_console_pci_device.irq_line, 1);
 }
 
 static bool virtio_console_pci_io_out(struct kvm *self, u16 port, void *data, int size, u32 count)
@@ -222,21 +234,15 @@ static struct ioport_operations virtio_console_io_ops = {
 	.io_out			= virtio_console_pci_io_out,
 };
 
-static struct pci_device_header virtio_console_pci_device = {
-	.vendor_id		= PCI_VENDOR_ID_REDHAT_QUMRANET,
-	.device_id		= PCI_DEVICE_ID_VIRTIO_CONSOLE,
-	.header_type		= PCI_HEADER_TYPE_NORMAL,
-	.revision_id		= 0,
-	.class			= 0x078000,
-	.subsys_vendor_id	= PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET,
-	.subsys_id		= PCI_SUBSYSTEM_ID_VIRTIO_CONSOLE,
-	.bar[0]			= IOPORT_VIRTIO_CONSOLE | PCI_BASE_ADDRESS_SPACE_IO,
-	.irq_pin		= VIRTIO_CONSOLE_PIN,
-	.irq_line		= VIRTIO_CONSOLE_IRQ,
-};
-
 void virtio_console__init(struct kvm *self)
 {
-	pci__register(&virtio_console_pci_device, PCI_VIRTIO_CONSOLE_DEVNUM);
+	u8 dev, line, pin;
+
+	if (irq__register_device(PCI_DEVICE_ID_VIRTIO_CONSOLE, &dev, &pin, &line) < 0)
+		return;
+
+	virtio_console_pci_device.irq_pin	= pin;
+	virtio_console_pci_device.irq_line	= line;
+	pci__register(&virtio_console_pci_device, dev);
 	ioport__register(IOPORT_VIRTIO_CONSOLE, &virtio_console_io_ops, IOPORT_VIRTIO_CONSOLE_SIZE);
 }
diff --git a/tools/kvm/virtio/net.c b/tools/kvm/virtio/net.c
index df69ab3..99ea22a 100644
--- a/tools/kvm/virtio/net.c
+++ b/tools/kvm/virtio/net.c
@@ -8,6 +8,7 @@
 #include "kvm/util.h"
 #include "kvm/kvm.h"
 #include "kvm/pci.h"
+#include "kvm/irq.h"
 
 #include <linux/virtio_net.h>
 #include <linux/if_tun.h>
@@ -26,6 +27,17 @@
 #define VIRTIO_NET_RX_QUEUE		0
 #define VIRTIO_NET_TX_QUEUE		1
 
+static struct pci_device_header virtio_net_pci_device = {
+	.vendor_id		= PCI_VENDOR_ID_REDHAT_QUMRANET,
+	.device_id		= PCI_DEVICE_ID_VIRTIO_NET,
+	.header_type		= PCI_HEADER_TYPE_NORMAL,
+	.revision_id		= 0,
+	.class			= 0x020000,
+	.subsys_vendor_id	= PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET,
+	.subsys_id		= PCI_SUBSYSTEM_ID_VIRTIO_NET,
+	.bar[0]			= IOPORT_VIRTIO_NET | PCI_BASE_ADDRESS_SPACE_IO,
+};
+
 struct net_device {
 	pthread_mutex_t			mutex;
 
@@ -89,7 +101,7 @@ static void *virtio_net_rx_thread(void *p)
 			len	= readv(net_device.tap_fd, iov, in);
 			virt_queue__set_used_elem(vq, head, len);
 			/* We should interrupt guest right now, otherwise latency is huge. */
-			kvm__irq_line(self, VIRTIO_NET_IRQ, 1);
+			kvm__irq_line(self, virtio_net_pci_device.irq_line, 1);
 		}
 
 	}
@@ -123,7 +135,7 @@ static void *virtio_net_tx_thread(void *p)
 			virt_queue__set_used_elem(vq, head, len);
 		}
 
-		kvm__irq_line(self, VIRTIO_NET_IRQ, 1);
+		kvm__irq_line(self, virtio_net_pci_device.irq_line, 1);
 	}
 
 	pthread_exit(NULL);
@@ -176,7 +188,7 @@ static bool virtio_net_pci_io_in(struct kvm *self, u16 port, void *data, int siz
 		break;
 	case VIRTIO_PCI_ISR:
 		ioport__write8(data, 0x1);
-		kvm__irq_line(self, VIRTIO_NET_IRQ, 0);
+		kvm__irq_line(self, virtio_net_pci_device.irq_line, 0);
 		break;
 	case VIRTIO_MSI_CONFIG_VECTOR:
 		ioport__write16(data, net_device.config_vector);
@@ -266,19 +278,6 @@ static struct ioport_operations virtio_net_io_ops = {
 	.io_out	= virtio_net_pci_io_out,
 };
 
-static struct pci_device_header virtio_net_pci_device = {
-	.vendor_id		= PCI_VENDOR_ID_REDHAT_QUMRANET,
-	.device_id		= PCI_DEVICE_ID_VIRTIO_NET,
-	.header_type		= PCI_HEADER_TYPE_NORMAL,
-	.revision_id		= 0,
-	.class			= 0x020000,
-	.subsys_vendor_id	= PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET,
-	.subsys_id		= PCI_SUBSYSTEM_ID_VIRTIO_NET,
-	.bar[0]			= IOPORT_VIRTIO_NET | PCI_BASE_ADDRESS_SPACE_IO,
-	.irq_pin		= VIRTIO_NET_PIN,
-	.irq_line		= VIRTIO_NET_IRQ,
-};
-
 static bool virtio_net__tap_init(const struct virtio_net_parameters *params)
 {
 	int sock = socket(AF_INET, SOCK_STREAM, 0);
@@ -380,7 +379,14 @@ static void virtio_net__io_thread_init(struct kvm *self)
 void virtio_net__init(const struct virtio_net_parameters *params)
 {
 	if (virtio_net__tap_init(params)) {
-		pci__register(&virtio_net_pci_device, PCI_VIRTIO_NET_DEVNUM);
+		u8 dev, line, pin;
+
+		if (irq__register_device(PCI_DEVICE_ID_VIRTIO_NET, &dev, &pin, &line) < 0)
+			return;
+
+		virtio_net_pci_device.irq_pin	= pin;
+		virtio_net_pci_device.irq_line	= line;
+		pci__register(&virtio_net_pci_device, dev);
 		ioport__register(IOPORT_VIRTIO_NET, &virtio_net_io_ops, IOPORT_VIRTIO_NET_SIZE);
 
 		virtio_net__io_thread_init(params->self);
diff --git a/tools/kvm/virtio/rng.c b/tools/kvm/virtio/rng.c
index f6034e4..d355cf8 100644
--- a/tools/kvm/virtio/rng.c
+++ b/tools/kvm/virtio/rng.c
@@ -11,6 +11,7 @@
 #include "kvm/kvm.h"
 #include "kvm/pci.h"
 #include "kvm/threadpool.h"
+#include "kvm/irq.h"
 
 #include <linux/virtio_ring.h>
 #include <linux/virtio_rng.h>
@@ -23,15 +24,26 @@
 #define NUM_VIRT_QUEUES				1
 #define VIRTIO_RNG_QUEUE_SIZE			128
 
+static struct pci_device_header virtio_rng_pci_device = {
+	.vendor_id		= PCI_VENDOR_ID_REDHAT_QUMRANET,
+	.device_id		= PCI_DEVICE_ID_VIRTIO_RNG,
+	.header_type		= PCI_HEADER_TYPE_NORMAL,
+	.revision_id		= 0,
+	.class			= 0x010000,
+	.subsys_vendor_id	= PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET,
+	.subsys_id		= PCI_SUBSYSTEM_ID_VIRTIO_RNG,
+	.bar[0]			= IOPORT_VIRTIO_RNG | PCI_BASE_ADDRESS_SPACE_IO,
+};
+
 struct rng_dev {
-	u8					status;
-	u16					config_vector;
-	int					fd;
+	u8			status;
+	u16			config_vector;
+	int			fd;
 
 	/* virtio queue */
-	u16					queue_selector;
-	struct virt_queue			vqs[NUM_VIRT_QUEUES];
-	void					*jobs[NUM_VIRT_QUEUES];
+	u16			queue_selector;
+	struct virt_queue	vqs[NUM_VIRT_QUEUES];
+	void			*jobs[NUM_VIRT_QUEUES];
 };
 
 static struct rng_dev rdev;
@@ -61,7 +73,7 @@ static bool virtio_rng_pci_io_in(struct kvm *kvm, u16 port, void *data, int size
 		break;
 	case VIRTIO_PCI_ISR:
 		ioport__write8(data, 0x1);
-		kvm__irq_line(kvm, VIRTIO_RNG_IRQ, 0);
+		kvm__irq_line(kvm, virtio_rng_pci_device.irq_line, 0);
 		break;
 	case VIRTIO_MSI_CONFIG_VECTOR:
 		ioport__write16(data, rdev.config_vector);
@@ -94,7 +106,7 @@ static void virtio_rng_do_io(struct kvm *kvm, void *param)
 
 	while (virt_queue__available(vq)) {
 		virtio_rng_do_io_request(kvm, vq);
-		kvm__irq_line(kvm, VIRTIO_RNG_IRQ, 1);
+		kvm__irq_line(kvm, virtio_rng_pci_device.irq_line, 1);
 	}
 }
 
@@ -151,26 +163,20 @@ static struct ioport_operations virtio_rng_io_ops = {
 	.io_out				= virtio_rng_pci_io_out,
 };
 
-static struct pci_device_header virtio_rng_pci_device = {
-	.vendor_id			= PCI_VENDOR_ID_REDHAT_QUMRANET,
-	.device_id			= PCI_DEVICE_ID_VIRTIO_RNG,
-	.header_type			= PCI_HEADER_TYPE_NORMAL,
-	.revision_id			= 0,
-	.class				= 0x010000,
-	.subsys_vendor_id		= PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET,
-	.subsys_id			= PCI_SUBSYSTEM_ID_VIRTIO_RNG,
-	.bar[0]				= IOPORT_VIRTIO_RNG | PCI_BASE_ADDRESS_SPACE_IO,
-	.irq_pin			= VIRTIO_RNG_PIN,
-	.irq_line			= VIRTIO_RNG_IRQ,
-};
-
 void virtio_rng__init(struct kvm *kvm)
 {
+	u8 pin, line, dev;
+
 	rdev.fd = open("/dev/urandom", O_RDONLY);
 	if (rdev.fd < 0)
 		die("Failed initializing RNG");
 
-	pci__register(&virtio_rng_pci_device, PCI_VIRTIO_RNG_DEVNUM);
+	if (irq__register_device(PCI_DEVICE_ID_VIRTIO_RNG, &dev, &pin, &line) < 0)
+		return;
+
+	virtio_rng_pci_device.irq_pin	= pin;
+	virtio_rng_pci_device.irq_line	= line;
+	pci__register(&virtio_rng_pci_device, dev);
 
 	ioport__register(IOPORT_VIRTIO_RNG, &virtio_rng_io_ops, IOPORT_VIRTIO_RNG_SIZE);
 }
-- 
1.7.5.rc3

Re: [PATCH 3/3] kvm tools: Convert virtio devices to use IRQ registry

[Ingo Molnar]

* Sasha Levin <levinsasha928@gmail.com> wrote:

> +	bdev->pci_device.irq_pin	= pin;
> +	bdev->pci_device.irq_line	= line;

One small remaining naming inconsistency caught my eyes. The generic convention 
should be something like:

 - structure names should be along the 'struct xyz_device' scheme

 - structure field names should be 'xyz_dev'

 - variable names within xyz driver's .c file should be 'xdev',
   but 'xyz_dev' is OK too, especially if used in some other file)

In that sense, the above should be:

	bdev->pci_dev.irq_pin	= pin;
	bdev->pci_dev.irq_line	= line;

This could be fixed in a followup patch - and there's more of the same 
inconsistency in other driver files as well.

If such details are sorted out early on in a project's lifetime it will be 
applied in a very natural way as the code grows.

Thanks,

	Ingo

Re: [PATCH 3/3] kvm tools: Convert virtio devices to use IRQ registry

[Sasha Levin]

On Fri, 2011-05-06 at 13:56 +0200, Ingo Molnar wrote:
> * Sasha Levin <levinsasha928@gmail.com> wrote:
> 
> > +	bdev->pci_device.irq_pin	= pin;
> > +	bdev->pci_device.irq_line	= line;
> 
> One small remaining naming inconsistency caught my eyes. The generic convention 
> should be something like:
> 
>  - structure names should be along the 'struct xyz_device' scheme
> 
>  - structure field names should be 'xyz_dev'
> 
>  - variable names within xyz driver's .c file should be 'xdev',
>    but 'xyz_dev' is OK too, especially if used in some other file)
> 
> In that sense, the above should be:
> 
> 	bdev->pci_dev.irq_pin	= pin;
> 	bdev->pci_dev.irq_line	= line;
> 
> This could be fixed in a followup patch - and there's more of the same 
> inconsistency in other driver files as well.
> 
> If such details are sorted out early on in a project's lifetime it will be 
> applied in a very natural way as the code grows.

The struct name there actually refers to a PCI header of a device, and
not an actual device.

I'll rename it to pci_hdr instead of making it pci_dev, since it looks
more confusing than desirable.

-- 

Sasha.

Re: [PATCH 3/3] kvm tools: Convert virtio devices to use IRQ registry

[Ingo Molnar]

* Sasha Levin <levinsasha928@gmail.com> wrote:

> On Fri, 2011-05-06 at 13:56 +0200, Ingo Molnar wrote:
> > * Sasha Levin <levinsasha928@gmail.com> wrote:
> > 
> > > +	bdev->pci_device.irq_pin	= pin;
> > > +	bdev->pci_device.irq_line	= line;
> > 
> > One small remaining naming inconsistency caught my eyes. The generic convention 
> > should be something like:
> > 
> >  - structure names should be along the 'struct xyz_device' scheme
> > 
> >  - structure field names should be 'xyz_dev'
> > 
> >  - variable names within xyz driver's .c file should be 'xdev',
> >    but 'xyz_dev' is OK too, especially if used in some other file)
> > 
> > In that sense, the above should be:
> > 
> > 	bdev->pci_dev.irq_pin	= pin;
> > 	bdev->pci_dev.irq_line	= line;
> > 
> > This could be fixed in a followup patch - and there's more of the same 
> > inconsistency in other driver files as well.
> > 
> > If such details are sorted out early on in a project's lifetime it will be 
> > applied in a very natural way as the code grows.
> 
> The struct name there actually refers to a PCI header of a device, and
> not an actual device.
> 
> I'll rename it to pci_hdr instead of making it pci_dev, since it looks
> more confusing than desirable.

Yeah, indeed that would be good, it certainly confused me! :-)

Thanks,

	Ingo

Re: [PATCH 1/3] kvm tools: Introduce IRQ registry

[Pekka Enberg]

On Fri, 6 May 2011, Sasha Levin wrote:
> +int irq__register_device(u32 dev, u8 *num, u8 *pin, u8 *line)
> +{
> +	struct pci_dev *node;
> +
> +	node = search(&pci_tree, dev);
> +
> +	if (!node) {
> +		/* We haven't found a node - First device of it's kind */
> +		node = malloc(sizeof(*node));
> +		if (node == NULL)
> +			return -1;
> +

This is never free'd:

   # KVM session ended normally.
==29360==
==29360== HEAP SUMMARY:
==29360==     in use at exit: 1,008 bytes in 8 blocks
==29360==   total heap usage: 23 allocs, 15 frees, 33,565,229 bytes 
allocated
==29360==
==29360== 24 bytes in 1 blocks are possibly lost in loss record 2 of 7
==29360==    at 0x4C275D8: malloc (vg_replace_malloc.c:236)
==29360==    by 0x40849F: irq__register_device (irq.c:87)
==29360==    by 0x404E4A: virtio_blk__init (blk.c:298)
==29360==    by 0x40766E: kvm_cmd_run (kvm-run.c:388)
==29360==    by 0x404636: main (main.c:16)
==29360==
==29360== 576 bytes in 2 blocks are possibly lost in loss record 7 of 7
==29360==    at 0x4C268FC: calloc (vg_replace_malloc.c:467)
==29360==    by 0x4012455: _dl_allocate_tls (dl-tls.c:300)
==29360==    by 0x503D728: pthread_create@@GLIBC_2.2.5 
(allocatestack.c:561)
==29360==    by 0x4082A1: thread_pool__init (threadpool.c:121)
==29360==    by 0x407827: kvm_cmd_run (kvm-run.c:447)
==29360==    by 0x404636: main (main.c:16)
==29360==
==29360== LEAK SUMMARY:
==29360==    definitely lost: 0 bytes in 0 blocks
==29360==    indirectly lost: 0 bytes in 0 blocks
==29360==      possibly lost: 600 bytes in 3 blocks
==29360==    still reachable: 408 bytes in 5 blocks
==29360==         suppressed: 0 bytes in 0 blocks
==29360== Reachable blocks (those to which a pointer was found) are not 
shown.
==29360== To see them, rerun with: --leak-check=full --show-reachable=yes
==29360==
==29360== For counts of detected and suppressed errors, rerun with: -v
==29360== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 4 from 4)

While it's not an error per se, I'd really like to keep things clean when 
running under valgrind.

 			Pekka