[PATCH 1/9] kvm tools: Add optional parameter used in ioport callbacks

[PATCH 1/9] kvm tools: Add optional parameter used in ioport callbacks

[Sasha Levin]

Allow specifying an optional parameter when registering an
ioport range. The callback functions provided by the registering
module will be called with the same parameter.

This may be used to keep context during callbacks on IO operations.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/include/kvm/ioport.h |    3 ++
 tools/kvm/ioport.c             |   54 +++++++++++++++++++++++++++++----------
 2 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/tools/kvm/include/kvm/ioport.h b/tools/kvm/include/kvm/ioport.h
index 8253938..2a8d74d 100644
--- a/tools/kvm/include/kvm/ioport.h
+++ b/tools/kvm/include/kvm/ioport.h
@@ -25,11 +25,14 @@ struct kvm;
 struct ioport_operations {
 	bool (*io_in)(struct kvm *kvm, u16 port, void *data, int size, u32 count);
 	bool (*io_out)(struct kvm *kvm, u16 port, void *data, int size, u32 count);
+	bool (*io_in_param)(struct kvm *kvm, u16 port, void *data, int size, u32 count, void *param);
+	bool (*io_out_param)(struct kvm *kvm, u16 port, void *data, int size, u32 count, void *param);
 };
 
 void ioport__setup_legacy(void);
 
 void ioport__register(u16 port, struct ioport_operations *ops, int count);
+void ioport__register_param(u16 port, struct ioport_operations *ops, int count, void *param);
 
 static inline u8 ioport__read8(u8 *data)
 {
diff --git a/tools/kvm/ioport.c b/tools/kvm/ioport.c
index 1f13960..159d089 100644
--- a/tools/kvm/ioport.c
+++ b/tools/kvm/ioport.c
@@ -18,6 +18,7 @@
 struct ioport_entry {
 	struct rb_int_node		node;
 	struct ioport_operations	*ops;
+	void				*param;
 };
 
 static struct rb_root ioport_tree = RB_ROOT;
@@ -89,6 +90,29 @@ void ioport__register(u16 port, struct ioport_operations *ops, int count)
 	ioport_insert(&ioport_tree, entry);
 }
 
+void ioport__register_param(u16 port, struct ioport_operations *ops, int count, void *param)
+{
+	struct ioport_entry *entry;
+
+	entry = ioport_search(&ioport_tree, port);
+	if (entry) {
+		pr_warning("ioport re-registered: %x", port);
+		rb_int_erase(&ioport_tree, &entry->node);
+	}
+
+	entry = malloc(sizeof(*entry));
+	if (entry == NULL)
+		die("Failed allocating new ioport entry");
+
+	*entry = (struct ioport_entry) {
+		.node	= RB_INT_INIT(port, port + count),
+		.ops	= ops,
+		.param	= param,
+	};
+
+	ioport_insert(&ioport_tree, entry);
+}
+
 static const char *to_direction(int direction)
 {
 	if (direction == KVM_EXIT_IO_IN)
@@ -105,30 +129,32 @@ static void ioport_error(u16 port, void *data, int direction, int size, u32 coun
 bool kvm__emulate_io(struct kvm *kvm, u16 port, void *data, int direction, int size, u32 count)
 {
 	struct ioport_operations *ops;
-	bool ret;
+	bool ret = false;
 	struct ioport_entry *entry;
+	void *param;
 
 	entry = ioport_search(&ioport_tree, port);
 	if (!entry)
 		goto error;
 
-	ops = entry->ops;
+	ops	= entry->ops;
+	param	= entry->param;
 
 	if (direction == KVM_EXIT_IO_IN) {
-		if (!ops->io_in)
-			goto error;
-
-		ret = ops->io_in(kvm, port, data, size, count);
-		if (!ret)
-			goto error;
+		if (!param && ops->io_in)
+			ret = ops->io_in(kvm, port, data, size, count);
+		if (param && ops->io_in_param)
+			ret = ops->io_in_param(kvm, port, data, size, count, param);
 	} else {
-		if (!ops->io_out)
-			goto error;
-
-		ret = ops->io_out(kvm, port, data, size, count);
-		if (!ret)
-			goto error;
+		if (!param && ops->io_out)
+			ret = ops->io_out(kvm, port, data, size, count);
+		if (param && ops->io_out_param)
+			ret = ops->io_out_param(kvm, port, data, size, count, param);
 	}
+
+	if (!ret)
+		goto error;
+
 	return true;
 error:
 	if (ioport_debug)
-- 
1.7.5.rc3

[PATCH 2/9] kvm tools: Add basic ioport dynamic allocation

[Sasha Levin]

Add a very simple allocation of ioports.

This prevents the need to coordinate ioports between different
modules.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/include/kvm/ioport.h |    5 +++++
 tools/kvm/ioport.c             |    7 +++++++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/tools/kvm/include/kvm/ioport.h b/tools/kvm/include/kvm/ioport.h
index 2a8d74d..bc7ea02 100644
--- a/tools/kvm/include/kvm/ioport.h
+++ b/tools/kvm/include/kvm/ioport.h
@@ -7,6 +7,9 @@
 
 /* some ports we reserve for own use */
 #define IOPORT_DBG			0xe0
+#define IOPORT_START			0x6200
+#define IOPORT_SIZE			0x400
+
 #define IOPORT_VESA			0xa200
 #define IOPORT_VESA_SIZE		256
 #define IOPORT_VIRTIO_P9		0xb200	/* Virtio 9P device */
@@ -34,6 +37,8 @@ void ioport__setup_legacy(void);
 void ioport__register(u16 port, struct ioport_operations *ops, int count);
 void ioport__register_param(u16 port, struct ioport_operations *ops, int count, void *param);
 
+u16 ioport__find_free_range(void);
+
 static inline u8 ioport__read8(u8 *data)
 {
 	return *data;
diff --git a/tools/kvm/ioport.c b/tools/kvm/ioport.c
index 159d089..0db5a87 100644
--- a/tools/kvm/ioport.c
+++ b/tools/kvm/ioport.c
@@ -126,6 +126,13 @@ static void ioport_error(u16 port, void *data, int direction, int size, u32 coun
 	fprintf(stderr, "IO error: %s port=%x, size=%d, count=%u\n", to_direction(direction), port, size, count);
 }
 
+u16 ioport__find_free_range(void)
+{
+	static u16 cur_loc;
+
+	return IOPORT_START + (cur_loc++ * IOPORT_SIZE);
+}
+
 bool kvm__emulate_io(struct kvm *kvm, u16 port, void *data, int direction, int size, u32 count)
 {
 	struct ioport_operations *ops;
-- 
1.7.5.rc3

[PATCH 3/9] kvm tools: Use ioport context to control blk devices

[Sasha Levin]

Since ioports now has the ability to pass context to its
callbacks, we can implement multiple blk devices more efficiently.

We can get a ptr to the 'current' blk dev on each ioport call, which
means that we don't need to keep track of the blk device allocation
and ioport distribution within the module.

The advantages are easier management of multiple blk devices and
removal of any hardcoded limits to the amount of possible blk
devices.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/include/kvm/ioport.h |    2 -
 tools/kvm/virtio/blk.c         |   75 ++++++++++++++-------------------------
 2 files changed, 27 insertions(+), 50 deletions(-)

diff --git a/tools/kvm/include/kvm/ioport.h b/tools/kvm/include/kvm/ioport.h
index bc7ea02..2fe751c 100644
--- a/tools/kvm/include/kvm/ioport.h
+++ b/tools/kvm/include/kvm/ioport.h
@@ -14,8 +14,6 @@
 #define IOPORT_VESA_SIZE		256
 #define IOPORT_VIRTIO_P9		0xb200	/* Virtio 9P device */
 #define IOPORT_VIRTIO_P9_SIZE		256
-#define IOPORT_VIRTIO_BLK		0xc200	/* Virtio block device */
-#define IOPORT_VIRTIO_BLK_SIZE		0x200
 #define IOPORT_VIRTIO_CONSOLE		0xd200	/* Virtio console device */
 #define IOPORT_VIRTIO_CONSOLE_SIZE	256
 #define IOPORT_VIRTIO_NET		0xe200	/* Virtio network device */
diff --git a/tools/kvm/virtio/blk.c b/tools/kvm/virtio/blk.c
index 25ce61f..4157e06 100644
--- a/tools/kvm/virtio/blk.c
+++ b/tools/kvm/virtio/blk.c
@@ -14,6 +14,7 @@
 #include <linux/virtio_ring.h>
 #include <linux/virtio_blk.h>
 
+#include <linux/list.h>
 #include <linux/types.h>
 #include <pthread.h>
 
@@ -34,15 +35,16 @@ struct blk_dev_job {
 
 struct blk_dev {
 	pthread_mutex_t			mutex;
+	struct list_head		list;
 
 	struct virtio_blk_config	blk_config;
 	struct disk_image		*disk;
+	u64				base_addr;
 	u32				host_features;
 	u32				guest_features;
 	u16				config_vector;
 	u8				status;
 	u8				isr;
-	u8				idx;
 
 	/* virtio queue */
 	u16				queue_selector;
@@ -52,7 +54,7 @@ struct blk_dev {
 	struct pci_device_header	pci_hdr;
 };
 
-static struct blk_dev *bdevs[VIRTIO_BLK_MAX_DEV];
+static LIST_HEAD(bdevs);
 
 static bool virtio_blk_dev_in(struct blk_dev *bdev, void *data, unsigned long offset, int size, u32 count)
 {
@@ -66,22 +68,14 @@ static bool virtio_blk_dev_in(struct blk_dev *bdev, void *data, unsigned long of
 	return true;
 }
 
-/* Translate port into device id + offset in that device addr space */
-static void virtio_blk_port2dev(u16 port, u16 base, u16 size, u16 *dev_idx, u16 *offset)
-{
-	*dev_idx	= (port - base) / size;
-	*offset		= port - (base + *dev_idx * size);
-}
-
-static bool virtio_blk_pci_io_in(struct kvm *kvm, u16 port, void *data, int size, u32 count)
+static bool virtio_blk_pci_io_in(struct kvm *kvm, u16 port, void *data, int size, u32 count, void *param)
 {
 	struct blk_dev *bdev;
-	u16 offset, dev_idx;
+	u16 offset;
 	bool ret = true;
 
-	virtio_blk_port2dev(port, IOPORT_VIRTIO_BLK, IOPORT_VIRTIO_BLK_SIZE, &dev_idx, &offset);
-
-	bdev = bdevs[dev_idx];
+	bdev	= param;
+	offset	= port - bdev->base_addr;
 
 	mutex_lock(&bdev->mutex);
 
@@ -178,15 +172,14 @@ static void virtio_blk_do_io(struct kvm *kvm, void *param)
 	virt_queue__trigger_irq(vq, bdev->pci_hdr.irq_line, &bdev->isr, kvm);
 }
 
-static bool virtio_blk_pci_io_out(struct kvm *kvm, u16 port, void *data, int size, u32 count)
+static bool virtio_blk_pci_io_out(struct kvm *kvm, u16 port, void *data, int size, u32 count, void *param)
 {
 	struct blk_dev *bdev;
-	u16 offset, dev_idx;
+	u16 offset;
 	bool ret = true;
 
-	virtio_blk_port2dev(port, IOPORT_VIRTIO_BLK, IOPORT_VIRTIO_BLK_SIZE, &dev_idx, &offset);
-
-	bdev = bdevs[dev_idx];
+	bdev	= param;
+	offset	= port - bdev->base_addr;
 
 	mutex_lock(&bdev->mutex);
 
@@ -246,48 +239,29 @@ static bool virtio_blk_pci_io_out(struct kvm *kvm, u16 port, void *data, int siz
 }
 
 static struct ioport_operations virtio_blk_io_ops = {
-	.io_in		= virtio_blk_pci_io_in,
-	.io_out		= virtio_blk_pci_io_out,
+	.io_in_param	= virtio_blk_pci_io_in,
+	.io_out_param	= virtio_blk_pci_io_out,
 };
 
-static int virtio_blk_find_empty_dev(void)
-{
-	int i;
-
-	for (i = 0; i < VIRTIO_BLK_MAX_DEV; i++) {
-		if (bdevs[i] == NULL)
-			return i;
-	}
-
-	return -1;
-}
-
 void virtio_blk__init(struct kvm *kvm, struct disk_image *disk)
 {
 	u16 blk_dev_base_addr;
 	u8 dev, pin, line;
 	struct blk_dev *bdev;
-	int new_dev_idx;
 
 	if (!disk)
 		return;
 
-	new_dev_idx		= virtio_blk_find_empty_dev();
-	if (new_dev_idx < 0)
-		die("Could not find an empty block device slot");
-
-	bdevs[new_dev_idx]	= calloc(1, sizeof(struct blk_dev));
-	if (bdevs[new_dev_idx] == NULL)
+	bdev = calloc(1, sizeof(struct blk_dev));
+	if (bdev == NULL)
 		die("Failed allocating bdev");
 
-	bdev			= bdevs[new_dev_idx];
-
-	blk_dev_base_addr	= IOPORT_VIRTIO_BLK + new_dev_idx * IOPORT_VIRTIO_BLK_SIZE;
+	blk_dev_base_addr	= ioport__find_free_range();
 
 	*bdev			= (struct blk_dev) {
 		.mutex				= PTHREAD_MUTEX_INITIALIZER,
 		.disk				= disk,
-		.idx				= new_dev_idx,
+		.base_addr			= blk_dev_base_addr,
 		.blk_config			= (struct virtio_blk_config) {
 			.capacity		= disk->size / SECTOR_SIZE,
 			.seg_max		= DISK_SEG_MAX,
@@ -310,6 +284,8 @@ void virtio_blk__init(struct kvm *kvm, struct disk_image *disk)
 		.host_features			= (1UL << VIRTIO_BLK_F_SEG_MAX | 1UL << VIRTIO_BLK_F_FLUSH),
 	};
 
+	list_add_tail(&bdev->list, &bdevs);
+
 	if (irq__register_device(VIRTIO_ID_BLOCK, &dev, &pin, &line) < 0)
 		return;
 
@@ -318,7 +294,7 @@ void virtio_blk__init(struct kvm *kvm, struct disk_image *disk)
 
 	pci__register(&bdev->pci_hdr, dev);
 
-	ioport__register(blk_dev_base_addr, &virtio_blk_io_ops, IOPORT_VIRTIO_BLK_SIZE);
+	ioport__register_param(blk_dev_base_addr, &virtio_blk_io_ops, IOPORT_SIZE, bdev);
 }
 
 void virtio_blk__init_all(struct kvm *kvm)
@@ -331,8 +307,11 @@ void virtio_blk__init_all(struct kvm *kvm)
 
 void virtio_blk__delete_all(struct kvm *kvm)
 {
-	int i;
+	while (!list_empty(&bdevs)) {
+		struct blk_dev *bdev;
 
-	for (i = 0; i < kvm->nr_disks; i++)
-		free(bdevs[i]);
+		bdev = list_first_entry(&bdevs, struct blk_dev, list);
+		list_del(&bdev->list);
+		free(bdev);
+	}
 }
-- 
1.7.5.rc3

[PATCH 4/9] kvm tools: Add support for multiple virtio-rng devices

[Sasha Levin]

Since multiple hardware rng devices of the same type are currently
unsupported by the kernel, this serves more as an example of a basic
virtio driver under kvm tools and can be used to debug the PCI layer.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/include/kvm/ioport.h        |    2 -
 tools/kvm/include/kvm/parse-options.h |    9 +++
 tools/kvm/include/kvm/virtio-rng.h    |    1 +
 tools/kvm/kvm-run.c                   |    8 ++-
 tools/kvm/virtio/rng.c                |  128 ++++++++++++++++++++++-----------
 5 files changed, 102 insertions(+), 46 deletions(-)

diff --git a/tools/kvm/include/kvm/ioport.h b/tools/kvm/include/kvm/ioport.h
index 2fe751c..709e18e 100644
--- a/tools/kvm/include/kvm/ioport.h
+++ b/tools/kvm/include/kvm/ioport.h
@@ -18,8 +18,6 @@
 #define IOPORT_VIRTIO_CONSOLE_SIZE	256
 #define IOPORT_VIRTIO_NET		0xe200	/* Virtio network device */
 #define IOPORT_VIRTIO_NET_SIZE		256
-#define IOPORT_VIRTIO_RNG		0xf200	/* Virtio network device */
-#define IOPORT_VIRTIO_RNG_SIZE		256
 
 struct kvm;
 
diff --git a/tools/kvm/include/kvm/parse-options.h b/tools/kvm/include/kvm/parse-options.h
index 2d5c99e..6bf9a1d 100644
--- a/tools/kvm/include/kvm/parse-options.h
+++ b/tools/kvm/include/kvm/parse-options.h
@@ -132,6 +132,15 @@ intptr_t defval;
 	.help = (h)                         \
 }
 
+#define OPT_INCR(s, l, v, h)                \
+{                                           \
+	.type = OPTION_INCR,	            \
+	.short_name = (s),                  \
+	.long_name = (l),                   \
+	.value = check_vtype(v, int *),     \
+	.help = (h)                         \
+}
+
 #define OPT_GROUP(h)                        \
 {                                           \
 	.type = OPTION_GROUP,               \
diff --git a/tools/kvm/include/kvm/virtio-rng.h b/tools/kvm/include/kvm/virtio-rng.h
index 7015c1f..c0a413b 100644
--- a/tools/kvm/include/kvm/virtio-rng.h
+++ b/tools/kvm/include/kvm/virtio-rng.h
@@ -4,5 +4,6 @@
 struct kvm;
 
 void virtio_rng__init(struct kvm *kvm);
+void virtio_rng__delete_all(struct kvm *kvm);
 
 #endif /* KVM__RNG_VIRTIO_H */
diff --git a/tools/kvm/kvm-run.c b/tools/kvm/kvm-run.c
index adbb25b..76b5782 100644
--- a/tools/kvm/kvm-run.c
+++ b/tools/kvm/kvm-run.c
@@ -52,6 +52,7 @@ static __thread struct kvm_cpu *current_kvm_cpu;
 
 static u64 ram_size;
 static u8  image_count;
+static int virtio_rng;
 static const char *kernel_cmdline;
 static const char *kernel_filename;
 static const char *vmlinux_filename;
@@ -66,7 +67,6 @@ static const char *script;
 static const char *virtio_9p_dir;
 static bool single_step;
 static bool readonly_image[MAX_DISK_IMAGES];
-static bool virtio_rng;
 static bool vnc;
 extern bool ioport_debug;
 extern int  active_console;
@@ -107,7 +107,7 @@ static const struct option options[] = {
 	OPT_CALLBACK('d', "disk", NULL, "image", "Disk image", img_name_parser),
 	OPT_STRING('\0', "console", &console, "serial or virtio",
 			"Console to use"),
-	OPT_BOOLEAN('\0', "rng", &virtio_rng,
+	OPT_INCR('\0', "rng", &virtio_rng,
 			"Enable virtio Random Number Generator"),
 	OPT_STRING('\0', "kvm-dev", &kvm_dev, "kvm-dev", "KVM device file"),
 	OPT_STRING('\0', "virtio-9p", &virtio_9p_dir, "root dir",
@@ -570,7 +570,8 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
 		virtio_console__init(kvm);
 
 	if (virtio_rng)
-		virtio_rng__init(kvm);
+		while (virtio_rng--)
+			virtio_rng__init(kvm);
 
 	if (!network)
 		network = DEFAULT_NETWORK;
@@ -631,6 +632,7 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
 	}
 
 	virtio_blk__delete_all(kvm);
+	virtio_rng__delete_all(kvm);
 
 	disk_image__close_all(kvm->disks, image_count);
 	kvm__delete(kvm);
diff --git a/tools/kvm/virtio/rng.c b/tools/kvm/virtio/rng.c
index 9bd0098..7dbed31 100644
--- a/tools/kvm/virtio/rng.c
+++ b/tools/kvm/virtio/rng.c
@@ -5,7 +5,6 @@
 #include "kvm/disk-image.h"
 #include "kvm/virtio.h"
 #include "kvm/ioport.h"
-#include "kvm/mutex.h"
 #include "kvm/util.h"
 #include "kvm/kvm.h"
 #include "kvm/pci.h"
@@ -15,6 +14,7 @@
 #include <linux/virtio_ring.h>
 #include <linux/virtio_rng.h>
 
+#include <linux/list.h>
 #include <fcntl.h>
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -23,18 +23,17 @@
 #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		= VIRTIO_ID_RNG,
-	.bar[0]			= IOPORT_VIRTIO_RNG | PCI_BASE_ADDRESS_SPACE_IO,
+struct rng_dev_job {
+	struct virt_queue		*vq;
+	struct rng_dev			*rdev;
+	void				*job_id;
 };
 
 struct rng_dev {
+	struct pci_device_header pci_hdr;
+	struct list_head	list;
+
+	u16			base_addr;
 	u8			status;
 	u8			isr;
 	u16			config_vector;
@@ -43,17 +42,19 @@ struct rng_dev {
 	/* virtio queue */
 	u16			queue_selector;
 	struct virt_queue	vqs[NUM_VIRT_QUEUES];
-	void			*jobs[NUM_VIRT_QUEUES];
+	struct rng_dev_job	jobs[NUM_VIRT_QUEUES];
 };
 
-static struct rng_dev rdev;
+static LIST_HEAD(rdevs);
 
-static bool virtio_rng_pci_io_in(struct kvm *kvm, u16 port, void *data, int size, u32 count)
+static bool virtio_rng_pci_io_in(struct kvm *kvm, u16 port, void *data, int size, u32 count, void *param)
 {
 	unsigned long offset;
 	bool ret = true;
+	struct rng_dev *rdev;
 
-	offset = port - IOPORT_VIRTIO_RNG;
+	rdev = param;
+	offset = port - rdev->base_addr;
 
 	switch (offset) {
 	case VIRTIO_PCI_HOST_FEATURES:
@@ -63,21 +64,21 @@ static bool virtio_rng_pci_io_in(struct kvm *kvm, u16 port, void *data, int size
 		ret		= false;
 		break;
 	case VIRTIO_PCI_QUEUE_PFN:
-		ioport__write32(data, rdev.vqs[rdev.queue_selector].pfn);
+		ioport__write32(data, rdev->vqs[rdev->queue_selector].pfn);
 		break;
 	case VIRTIO_PCI_QUEUE_NUM:
 		ioport__write16(data, VIRTIO_RNG_QUEUE_SIZE);
 		break;
 	case VIRTIO_PCI_STATUS:
-		ioport__write8(data, rdev.status);
+		ioport__write8(data, rdev->status);
 		break;
 	case VIRTIO_PCI_ISR:
-		ioport__write8(data, rdev.isr);
-		kvm__irq_line(kvm, virtio_rng_pci_device.irq_line, VIRTIO_IRQ_LOW);
-		rdev.isr = VIRTIO_IRQ_LOW;
+		ioport__write8(data, rdev->isr);
+		kvm__irq_line(kvm, rdev->pci_hdr.irq_line, VIRTIO_IRQ_LOW);
+		rdev->isr = VIRTIO_IRQ_LOW;
 		break;
 	case VIRTIO_MSI_CONFIG_VECTOR:
-		ioport__write16(data, rdev.config_vector);
+		ioport__write16(data, rdev->config_vector);
 		break;
 	default:
 		ret		= false;
@@ -87,14 +88,14 @@ static bool virtio_rng_pci_io_in(struct kvm *kvm, u16 port, void *data, int size
 	return ret;
 }
 
-static bool virtio_rng_do_io_request(struct kvm *kvm, struct virt_queue *queue)
+static bool virtio_rng_do_io_request(struct kvm *kvm, struct rng_dev *rdev, struct virt_queue *queue)
 {
 	struct iovec iov[VIRTIO_RNG_QUEUE_SIZE];
 	unsigned int len = 0;
 	u16 out, in, head;
 
 	head		= virt_queue__get_iov(queue, iov, &out, &in, kvm);
-	len		= readv(rdev.fd, iov, in);
+	len		= readv(rdev->fd, iov, in);
 
 	virt_queue__set_used_elem(queue, head, len);
 
@@ -103,20 +104,24 @@ static bool virtio_rng_do_io_request(struct kvm *kvm, struct virt_queue *queue)
 
 static void virtio_rng_do_io(struct kvm *kvm, void *param)
 {
-	struct virt_queue *vq = param;
+	struct rng_dev_job *job = param;
+	struct virt_queue *vq = job->vq;
+	struct rng_dev *rdev = job->rdev;
 
 	while (virt_queue__available(vq)) {
-		virtio_rng_do_io_request(kvm, vq);
-		virt_queue__trigger_irq(vq, virtio_rng_pci_device.irq_line, &rdev.isr, kvm);
+		virtio_rng_do_io_request(kvm, rdev, vq);
+		virt_queue__trigger_irq(vq, rdev->pci_hdr.irq_line, &rdev->isr, kvm);
 	}
 }
 
-static bool virtio_rng_pci_io_out(struct kvm *kvm, u16 port, void *data, int size, u32 count)
+static bool virtio_rng_pci_io_out(struct kvm *kvm, u16 port, void *data, int size, u32 count, void *param)
 {
 	unsigned long offset;
 	bool ret = true;
+	struct rng_dev *rdev;
 
-	offset		= port - IOPORT_VIRTIO_RNG;
+	rdev = param;
+	offset = port - rdev->base_addr;
 
 	switch (offset) {
 	case VIRTIO_MSI_QUEUE_VECTOR:
@@ -124,32 +129,40 @@ static bool virtio_rng_pci_io_out(struct kvm *kvm, u16 port, void *data, int siz
 		break;
 	case VIRTIO_PCI_QUEUE_PFN: {
 		struct virt_queue *queue;
+		struct rng_dev_job *job;
 		void *p;
 
-		queue			= &rdev.vqs[rdev.queue_selector];
+		queue			= &rdev->vqs[rdev->queue_selector];
 		queue->pfn		= ioport__read32(data);
 		p			= guest_pfn_to_host(kvm, queue->pfn);
 
+		job = &rdev->jobs[rdev->queue_selector];
+
 		vring_init(&queue->vring, VIRTIO_RNG_QUEUE_SIZE, p, VIRTIO_PCI_VRING_ALIGN);
 
-		rdev.jobs[rdev.queue_selector] = thread_pool__add_job(kvm, virtio_rng_do_io, queue);
+		*job			= (struct rng_dev_job) {
+			.vq			= queue,
+			.rdev			= rdev,
+		};
+
+		job->job_id = thread_pool__add_job(kvm, virtio_rng_do_io, job);
 
 		break;
 	}
 	case VIRTIO_PCI_QUEUE_SEL:
-		rdev.queue_selector	= ioport__read16(data);
+		rdev->queue_selector	= ioport__read16(data);
 		break;
 	case VIRTIO_PCI_QUEUE_NOTIFY: {
 		u16 queue_index;
 		queue_index		= ioport__read16(data);
-		thread_pool__do_job(rdev.jobs[queue_index]);
+		thread_pool__do_job(rdev->jobs[queue_index].job_id);
 		break;
 	}
 	case VIRTIO_PCI_STATUS:
-		rdev.status		= ioport__read8(data);
+		rdev->status		= ioport__read8(data);
 		break;
 	case VIRTIO_MSI_CONFIG_VECTOR:
-		rdev.config_vector	= VIRTIO_MSI_NO_VECTOR;
+		rdev->config_vector	= VIRTIO_MSI_NO_VECTOR;
 		break;
 	default:
 		ret			= false;
@@ -160,24 +173,57 @@ static bool virtio_rng_pci_io_out(struct kvm *kvm, u16 port, void *data, int siz
 }
 
 static struct ioport_operations virtio_rng_io_ops = {
-	.io_in				= virtio_rng_pci_io_in,
-	.io_out				= virtio_rng_pci_io_out,
+	.io_in_param			= virtio_rng_pci_io_in,
+	.io_out_param			= virtio_rng_pci_io_out,
 };
 
 void virtio_rng__init(struct kvm *kvm)
 {
 	u8 pin, line, dev;
+	u16 rdev_base_addr;
+	struct rng_dev *rdev;
+
+	rdev = malloc(sizeof(*rdev));
+	if (rdev == NULL)
+		return;
+
+	rdev_base_addr = ioport__find_free_range();
+
+	rdev->pci_hdr = (struct pci_device_header) {
+		.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		= VIRTIO_ID_RNG,
+		.bar[0]			= rdev_base_addr | PCI_BASE_ADDRESS_SPACE_IO,
+	};
 
-	rdev.fd = open("/dev/urandom", O_RDONLY);
-	if (rdev.fd < 0)
+	rdev->base_addr = rdev_base_addr;
+	rdev->fd = open("/dev/urandom", O_RDONLY);
+	if (rdev->fd < 0)
 		die("Failed initializing RNG");
 
 	if (irq__register_device(VIRTIO_ID_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);
+	rdev->pci_hdr.irq_pin	= pin;
+	rdev->pci_hdr.irq_line	= line;
+	pci__register(&rdev->pci_hdr, dev);
 
-	ioport__register(IOPORT_VIRTIO_RNG, &virtio_rng_io_ops, IOPORT_VIRTIO_RNG_SIZE);
+	ioport__register_param(rdev_base_addr, &virtio_rng_io_ops, IOPORT_SIZE, rdev);
+
+	list_add_tail(&rdev->list, &rdevs);
+}
+
+void virtio_rng__delete_all(struct kvm *kvm)
+{
+	while (!list_empty(&rdevs)) {
+		struct rng_dev *rdev;
+
+		rdev = list_first_entry(&rdevs, struct rng_dev, list);
+		list_del(&rdev->list);
+		free(rdev);
+	}
 }
-- 
1.7.5.rc3

[PATCH 5/9] kvm tools: Use dynamic IO port allocation in vesa driver

[Sasha Levin]

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/hw/vesa.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/kvm/hw/vesa.c b/tools/kvm/hw/vesa.c
index 6ab07ee..eb3ee65 100644
--- a/tools/kvm/hw/vesa.c
+++ b/tools/kvm/hw/vesa.c
@@ -49,7 +49,6 @@ static struct pci_device_header vesa_pci_device = {
 	.class			= 0x030000,
 	.subsys_vendor_id	= PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET,
 	.subsys_id		= PCI_SUBSYSTEM_ID_VESA,
-	.bar[0]			= IOPORT_VESA   | PCI_BASE_ADDRESS_SPACE_IO,
 	.bar[1]			= VESA_MEM_ADDR | PCI_BASE_ADDRESS_SPACE_MEMORY,
 };
 
@@ -66,16 +65,18 @@ void vesa__init(struct kvm *kvm)
 {
 	u8 dev, line, pin;
 	pthread_t thread;
+	u16 vesa_base_addr;
 
 	if (irq__register_device(PCI_DEVICE_ID_VESA, &dev, &pin, &line) < 0)
 		return;
 
 	vesa_pci_device.irq_pin		= pin;
 	vesa_pci_device.irq_line	= line;
-
+	vesa_base_addr			= ioport__find_free_range();
+	vesa_pci_device.bar[0]		= vesa_base_addr | PCI_BASE_ADDRESS_SPACE_IO;
 	pci__register(&vesa_pci_device, dev);
 
-	ioport__register(IOPORT_VESA, &vesa_io_ops, IOPORT_VESA_SIZE);
+	ioport__register(vesa_base_addr, &vesa_io_ops, IOPORT_SIZE);
 
 	kvm__register_mmio(VESA_MEM_ADDR, VESA_MEM_SIZE, &vesa_mmio_callback);
 
-- 
1.7.5.rc3

[PATCH 6/9] kvm tools: Use dynamic IO port allocation in 9p driver

[Sasha Levin]

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/virtio/9p.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
index e307592..f8cb96d 100644
--- a/tools/kvm/virtio/9p.c
+++ b/tools/kvm/virtio/9p.c
@@ -50,7 +50,6 @@ static struct pci_device_header virtio_p9_pci_device = {
 	.class			= 0x010000,
 	.subsys_vendor_id	= PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET,
 	.subsys_id		= VIRTIO_ID_9P,
-	.bar[0]			= IOPORT_VIRTIO_P9 | PCI_BASE_ADDRESS_SPACE_IO,
 };
 
 struct p9_dev {
@@ -59,6 +58,7 @@ struct p9_dev {
 	u16			config_vector;
 	u32			features;
 	struct virtio_9p_config	*config;
+	u16			base_addr;
 
 	/* virtio queue */
 	u16			queue_selector;
@@ -96,7 +96,7 @@ static bool virtio_p9_pci_io_in(struct kvm *kvm, u16 port, void *data, int size,
 	unsigned long offset;
 	bool ret = true;
 
-	offset = port - IOPORT_VIRTIO_P9;
+	offset = port - p9dev.base_addr;
 
 	switch (offset) {
 	case VIRTIO_PCI_HOST_FEATURES:
@@ -584,7 +584,7 @@ static bool virtio_p9_pci_io_out(struct kvm *kvm, u16 port, void *data, int size
 	unsigned long offset;
 	bool ret = true;
 
-	offset		= port - IOPORT_VIRTIO_P9;
+	offset = port - p9dev.base_addr;
 
 	switch (offset) {
 	case VIRTIO_MSI_QUEUE_VECTOR:
@@ -636,6 +636,7 @@ void virtio_9p__init(struct kvm *kvm, const char *root)
 {
 	u8 pin, line, dev;
 	u32 i, root_len;
+	u16 p9_base_addr;
 
 	p9dev.config = calloc(1, sizeof(*p9dev.config) + sizeof(VIRTIO_P9_TAG));
 	if (p9dev.config == NULL)
@@ -662,7 +663,10 @@ void virtio_9p__init(struct kvm *kvm, const char *root)
 
 	virtio_p9_pci_device.irq_pin	= pin;
 	virtio_p9_pci_device.irq_line	= line;
+	p9_base_addr			= ioport__find_free_range();
+	virtio_p9_pci_device.bar[0]	= p9_base_addr | PCI_BASE_ADDRESS_SPACE_IO;
+	p9dev.base_addr			= p9_base_addr;
 	pci__register(&virtio_p9_pci_device, dev);
 
-	ioport__register(IOPORT_VIRTIO_P9, &virtio_p9_io_ops, IOPORT_VIRTIO_P9_SIZE);
+	ioport__register(p9_base_addr, &virtio_p9_io_ops, IOPORT_SIZE);
 }
-- 
1.7.5.rc3

[PATCH 7/9] kvm tools: Use dynamic IO port allocation in virtio-console driver

[Sasha Levin]

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/virtio/console.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/tools/kvm/virtio/console.c b/tools/kvm/virtio/console.c
index affff0b..eeb5dc9 100644
--- a/tools/kvm/virtio/console.c
+++ b/tools/kvm/virtio/console.c
@@ -36,7 +36,6 @@ static struct pci_device_header virtio_console_pci_device = {
 	.class			= 0x078000,
 	.subsys_vendor_id	= PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET,
 	.subsys_id		= VIRTIO_ID_CONSOLE,
-	.bar[0]			= IOPORT_VIRTIO_CONSOLE | PCI_BASE_ADDRESS_SPACE_IO,
 };
 
 struct con_dev {
@@ -50,6 +49,7 @@ struct con_dev {
 	u8				status;
 	u8				isr;
 	u16				queue_selector;
+	u16				base_addr;
 
 	void				*jobs[VIRTIO_CONSOLE_NUM_QUEUES];
 };
@@ -113,7 +113,7 @@ static bool virtio_console_pci_io_device_specific_in(void *data, unsigned long o
 
 static bool virtio_console_pci_io_in(struct kvm *kvm, u16 port, void *data, int size, u32 count)
 {
-	unsigned long offset = port - IOPORT_VIRTIO_CONSOLE;
+	unsigned long offset = port - cdev.base_addr;
 	bool ret = true;
 
 	mutex_lock(&cdev.mutex);
@@ -181,7 +181,7 @@ static void virtio_console_handle_callback(struct kvm *kvm, void *param)
 
 static bool virtio_console_pci_io_out(struct kvm *kvm, u16 port, void *data, int size, u32 count)
 {
-	unsigned long offset = port - IOPORT_VIRTIO_CONSOLE;
+	unsigned long offset = port - cdev.base_addr;
 	bool ret = true;
 
 	mutex_lock(&cdev.mutex);
@@ -243,12 +243,16 @@ static struct ioport_operations virtio_console_io_ops = {
 void virtio_console__init(struct kvm *kvm)
 {
 	u8 dev, line, pin;
+	u16 console_base_addr;
 
 	if (irq__register_device(VIRTIO_ID_CONSOLE, &dev, &pin, &line) < 0)
 		return;
 
 	virtio_console_pci_device.irq_pin	= pin;
 	virtio_console_pci_device.irq_line	= line;
+	console_base_addr			= ioport__find_free_range();
+	virtio_console_pci_device.bar[0]	= console_base_addr | PCI_BASE_ADDRESS_SPACE_IO;
+	cdev.base_addr				= console_base_addr;
 	pci__register(&virtio_console_pci_device, dev);
-	ioport__register(IOPORT_VIRTIO_CONSOLE, &virtio_console_io_ops, IOPORT_VIRTIO_CONSOLE_SIZE);
+	ioport__register(console_base_addr, &virtio_console_io_ops, IOPORT_SIZE);
 }
-- 
1.7.5.rc3

[PATCH 8/9] kvm tools: Use dynamic IO port allocation in virtio-net driver

[Sasha Levin]

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/virtio/net.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/tools/kvm/virtio/net.c b/tools/kvm/virtio/net.c
index 649bc0f..765ae56d 100644
--- a/tools/kvm/virtio/net.c
+++ b/tools/kvm/virtio/net.c
@@ -37,7 +37,6 @@ static struct pci_device_header pci_header = {
 	.class				= 0x020000,
 	.subsys_vendor_id		= PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET,
 	.subsys_id			= VIRTIO_ID_NET,
-	.bar[0]				= IOPORT_VIRTIO_NET | PCI_BASE_ADDRESS_SPACE_IO,
 };
 
 struct net_device {
@@ -51,6 +50,7 @@ struct net_device {
 	u8				status;
 	u8				isr;
 	u16				queue_selector;
+	u16				base_addr;
 
 	pthread_t			io_rx_thread;
 	pthread_mutex_t			io_rx_lock;
@@ -166,7 +166,7 @@ static bool virtio_net_pci_io_device_specific_in(void *data, unsigned long offse
 
 static bool virtio_net_pci_io_in(struct kvm *kvm, u16 port, void *data, int size, u32 count)
 {
-	unsigned long	offset	= port - IOPORT_VIRTIO_NET;
+	unsigned long	offset	= port - ndev.base_addr;
 	bool		ret	= true;
 
 	mutex_lock(&ndev.mutex);
@@ -230,7 +230,7 @@ static void virtio_net_handle_callback(struct kvm *kvm, u16 queue_index)
 
 static bool virtio_net_pci_io_out(struct kvm *kvm, u16 port, void *data, int size, u32 count)
 {
-	unsigned long	offset		= port - IOPORT_VIRTIO_NET;
+	unsigned long	offset		= port - ndev.base_addr;
 	bool		ret		= true;
 
 	mutex_lock(&ndev.mutex);
@@ -387,14 +387,19 @@ void virtio_net__init(const struct virtio_net_parameters *params)
 {
 	if (virtio_net__tap_init(params)) {
 		u8 dev, line, pin;
+		u16 net_base_addr;
 
 		if (irq__register_device(VIRTIO_ID_NET, &dev, &pin, &line) < 0)
 			return;
 
 		pci_header.irq_pin	= pin;
 		pci_header.irq_line	= line;
+		net_base_addr		= ioport__find_free_range();
+		pci_header.bar[0]	= net_base_addr | PCI_BASE_ADDRESS_SPACE_IO;
+		ndev.base_addr		= net_base_addr;
+
 		pci__register(&pci_header, dev);
-		ioport__register(IOPORT_VIRTIO_NET, &virtio_net_io_ops, IOPORT_VIRTIO_NET_SIZE);
+		ioport__register(net_base_addr, &virtio_net_io_ops, IOPORT_SIZE);
 
 		virtio_net__io_thread_init(params->kvm);
 	}
-- 
1.7.5.rc3

[PATCH 9/9] kvm tools: Remove static ioport allocations

[Sasha Levin]

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/include/kvm/ioport.h |    9 ---------
 1 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/tools/kvm/include/kvm/ioport.h b/tools/kvm/include/kvm/ioport.h
index 709e18e..cc57cdd 100644
--- a/tools/kvm/include/kvm/ioport.h
+++ b/tools/kvm/include/kvm/ioport.h
@@ -10,15 +10,6 @@
 #define IOPORT_START			0x6200
 #define IOPORT_SIZE			0x400
 
-#define IOPORT_VESA			0xa200
-#define IOPORT_VESA_SIZE		256
-#define IOPORT_VIRTIO_P9		0xb200	/* Virtio 9P device */
-#define IOPORT_VIRTIO_P9_SIZE		256
-#define IOPORT_VIRTIO_CONSOLE		0xd200	/* Virtio console device */
-#define IOPORT_VIRTIO_CONSOLE_SIZE	256
-#define IOPORT_VIRTIO_NET		0xe200	/* Virtio network device */
-#define IOPORT_VIRTIO_NET_SIZE		256
-
 struct kvm;
 
 struct ioport_operations {
-- 
1.7.5.rc3

Re: [PATCH 2/9] kvm tools: Add basic ioport dynamic allocation

[Ingo Molnar]

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

> +u16 ioport__find_free_range(void)
> +{
> +	static u16 cur_loc;

Please don't put statics inside function bodies! (even if they are 
only used within a single function)

I had to look three times to discover that it's really a global 
variable. These should be where other global variables are, or should 
be put before the function, in plain sight.

Also, if you do that i'd suggest a more descriptive name - 
free_io_port_idx perhaps?

> +	return IOPORT_START + (cur_loc++ * IOPORT_SIZE);

So this is SMP unsafe really. While ioport registrations are 
currently only used from initdev() functions and are thus serialized, 
it's not completely unfeasible that we would want to have async 
initcalls like the kernel does, to improve bootup/startup 
performance!

So it would be nice to make this all SMP safe. A single mutex would 
suffice i suspect.

Thanks,

	Ingo

Re: [PATCH 7/9] kvm tools: Use dynamic IO port allocation in virtio-console driver

[Ingo Molnar]

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

>  void virtio_console__init(struct kvm *kvm)
>  {
>  	u8 dev, line, pin;
> +	u16 console_base_addr;
>  
>  	if (irq__register_device(VIRTIO_ID_CONSOLE, &dev, &pin, &line) < 0)
>  		return;
>  
>  	virtio_console_pci_device.irq_pin	= pin;
>  	virtio_console_pci_device.irq_line	= line;
> +	console_base_addr			= ioport__find_free_range();
> +	virtio_console_pci_device.bar[0]	= console_base_addr | PCI_BASE_ADDRESS_SPACE_IO;
> +	cdev.base_addr				= console_base_addr;
>  	pci__register(&virtio_console_pci_device, dev);
> -	ioport__register(IOPORT_VIRTIO_CONSOLE, &virtio_console_io_ops, IOPORT_VIRTIO_CONSOLE_SIZE);
> +	ioport__register(console_base_addr, &virtio_console_io_ops, IOPORT_SIZE);

Why is the ioport registration done in two steps?

Wouldnt a better sequence be something like:

> +	console_base_addr			= ioport__register(&virtio_console_io_ops, IOPORT_SIZE);
>
> +	virtio_console_pci_device.bar[0]	= console_base_addr | PCI_BASE_ADDRESS_SPACE_IO;
> +	cdev.base_addr				= console_base_addr;
>  	pci__register(&virtio_console_pci_device, dev);

I.e. first register the ioport range - this would also get a free 
range for you, and then register the PCI driver?

Or something even more compact could be done i suspect - all of the 
drivers seem to be using the same registration sequence.

Thanks,

	Ingo

Re: [PATCH 9/9] kvm tools: Remove static ioport allocations

[Ingo Molnar]

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

> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
>  tools/kvm/include/kvm/ioport.h |    9 ---------
>  1 files changed, 0 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/kvm/include/kvm/ioport.h b/tools/kvm/include/kvm/ioport.h
> index 709e18e..cc57cdd 100644
> --- a/tools/kvm/include/kvm/ioport.h
> +++ b/tools/kvm/include/kvm/ioport.h
> @@ -10,15 +10,6 @@
>  #define IOPORT_START			0x6200
>  #define IOPORT_SIZE			0x400
>  
> -#define IOPORT_VESA			0xa200
> -#define IOPORT_VESA_SIZE		256
> -#define IOPORT_VIRTIO_P9		0xb200	/* Virtio 9P device */
> -#define IOPORT_VIRTIO_P9_SIZE		256
> -#define IOPORT_VIRTIO_CONSOLE		0xd200	/* Virtio console device */
> -#define IOPORT_VIRTIO_CONSOLE_SIZE	256
> -#define IOPORT_VIRTIO_NET		0xe200	/* Virtio network device */
> -#define IOPORT_VIRTIO_NET_SIZE		256

Btw., nice end result :-)

Thanks,

	Ingo

Re: [PATCH 7/9] kvm tools: Use dynamic IO port allocation in virtio-console driver

[Sasha Levin]

On Wed, 2011-05-25 at 16:42 +0200, Ingo Molnar wrote:
> * Sasha Levin <levinsasha928@gmail.com> wrote:
> 
> >  void virtio_console__init(struct kvm *kvm)
> >  {
> >  	u8 dev, line, pin;
> > +	u16 console_base_addr;
> >  
> >  	if (irq__register_device(VIRTIO_ID_CONSOLE, &dev, &pin, &line) < 0)
> >  		return;
> >  
> >  	virtio_console_pci_device.irq_pin	= pin;
> >  	virtio_console_pci_device.irq_line	= line;
> > +	console_base_addr			= ioport__find_free_range();
> > +	virtio_console_pci_device.bar[0]	= console_base_addr | PCI_BASE_ADDRESS_SPACE_IO;
> > +	cdev.base_addr				= console_base_addr;
> >  	pci__register(&virtio_console_pci_device, dev);
> > -	ioport__register(IOPORT_VIRTIO_CONSOLE, &virtio_console_io_ops, IOPORT_VIRTIO_CONSOLE_SIZE);
> > +	ioport__register(console_base_addr, &virtio_console_io_ops, IOPORT_SIZE);
> 
> Why is the ioport registration done in two steps?
> 
> Wouldnt a better sequence be something like:
> 
> > +	console_base_addr			= ioport__register(&virtio_console_io_ops, IOPORT_SIZE);
> >
> > +	virtio_console_pci_device.bar[0]	= console_base_addr | PCI_BASE_ADDRESS_SPACE_IO;
> > +	cdev.base_addr				= console_base_addr;
> >  	pci__register(&virtio_console_pci_device, dev);
> 
> I.e. first register the ioport range - this would also get a free 
> range for you, and then register the PCI driver?
> 
> Or something even more compact could be done i suspect - all of the 
> drivers seem to be using the same registration sequence.

Some drivers need explicit IO ports, such as the serial driver.

It was a question of whether I create another registration function
(which may be as simple as calling 'ioport__find_free_range()' before
calling the real registration) but it would lead to two 'registration'
functions behaving very differently - one returning a new port and one
that just does nothing except registering.

-- 

Sasha.