diff mbox

[2/2] kvm tools: Modify ioport to use interval rbtree

Message ID 1305967911-30045-2-git-send-email-levinsasha928@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sasha Levin May 21, 2011, 8:51 a.m. UTC
Currently the ioport implementation is based on a USHRT_MAX length
array of ptrs to ioport_operations.

Instead, use an interval rbtree to map the ioports to
ioport_operations.

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

Comments

Cyrill Gorcunov May 21, 2011, 10:31 a.m. UTC | #1
On 05/21/2011 12:51 PM, Sasha Levin wrote:
> Currently the ioport implementation is based on a USHRT_MAX length
> array of ptrs to ioport_operations.
> 
> Instead, use an interval rbtree to map the ioports to
> ioport_operations.
> 
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
...
> -static struct ioport_operations *ioport_ops[USHRT_MAX];
> -
>  void ioport__register(u16 port, struct ioport_operations *ops, int count)
>  {
> -	int i;
> +	struct ioport_entry *entry;
>  
> -	for (i = 0; i < count; i++)
> -		ioport_ops[port + i]	= ops;
> +	entry = ioport_search(&ioport_tree, port);
> +	if (entry)
> +		rb_int_erase(&ioport_tree, &entry->node);
> +

  Hi Sasha, if I understand this correct we're simply drop old registartion, right? I think
it should not be like that, if one port get used for several drivers/purposes we need a
ref-counting, but at moment I think we simply should not allow to re-register port without
previously unregister it. Or I miss something?
Sasha Levin May 21, 2011, 10:55 a.m. UTC | #2
On Sat, 2011-05-21 at 14:31 +0400, Cyrill Gorcunov wrote:
> On 05/21/2011 12:51 PM, Sasha Levin wrote:
> > Currently the ioport implementation is based on a USHRT_MAX length
> > array of ptrs to ioport_operations.
> > 
> > Instead, use an interval rbtree to map the ioports to
> > ioport_operations.
> > 
> > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> > ---
> ...
> > -static struct ioport_operations *ioport_ops[USHRT_MAX];
> > -
> >  void ioport__register(u16 port, struct ioport_operations *ops, int count)
> >  {
> > -	int i;
> > +	struct ioport_entry *entry;
> >  
> > -	for (i = 0; i < count; i++)
> > -		ioport_ops[port + i]	= ops;
> > +	entry = ioport_search(&ioport_tree, port);
> > +	if (entry)
> > +		rb_int_erase(&ioport_tree, &entry->node);
> > +
> 
>   Hi Sasha, if I understand this correct we're simply drop old registartion, right? I think
> it should not be like that, if one port get used for several drivers/purposes we need a
> ref-counting, but at moment I think we simply should not allow to re-register port without
> previously unregister it. Or I miss something?

Currently we register some ports as dummy ports in the ioport
initialization, and re-register them once they get someone who can use
them (for example, serial device).

Not allowing ports to re-register would mean we can't reassign ports to
serial console when the serial console module gets loaded.
Cyrill Gorcunov May 21, 2011, 12:08 p.m. UTC | #3
On 05/21/2011 02:55 PM, Sasha Levin wrote:
...
>>>  void ioport__register(u16 port, struct ioport_operations *ops, int count)
>>>  {
>>> -	int i;
>>> +	struct ioport_entry *entry;
>>>  
>>> -	for (i = 0; i < count; i++)
>>> -		ioport_ops[port + i]	= ops;
>>> +	entry = ioport_search(&ioport_tree, port);
>>> +	if (entry)
>>> +		rb_int_erase(&ioport_tree, &entry->node);
>>> +
>>
>>   Hi Sasha, if I understand this correct we're simply drop old registartion, right? I think
>> it should not be like that, if one port get used for several drivers/purposes we need a
>> ref-counting, but at moment I think we simply should not allow to re-register port without
>> previously unregister it. Or I miss something?
> 
> Currently we register some ports as dummy ports in the ioport
> initialization, and re-register them once they get someone who can use
> them (for example, serial device).
> 
> Not allowing ports to re-register would mean we can't reassign ports to
> serial console when the serial console module gets loaded.
> 

Yup, my bad, drop my complain, thanks ;)
diff mbox

Patch

diff --git a/tools/kvm/ioport.c b/tools/kvm/ioport.c
index 2f6c06c..ea19f2b 100644
--- a/tools/kvm/ioport.c
+++ b/tools/kvm/ioport.c
@@ -1,6 +1,8 @@ 
 #include "kvm/ioport.h"
 
 #include "kvm/kvm.h"
+#include "kvm/util.h"
+#include "kvm/rbtree-interval.h"
 
 #include <linux/kvm.h>	/* for KVM_EXIT_* */
 #include <linux/types.h>
@@ -11,8 +13,32 @@ 
 #include <stdlib.h>
 #include <stdio.h>
 
+#define ioport_node(n) rb_entry(n, struct ioport_entry, node)
+
+struct ioport_entry {
+	struct rb_int_node		node;
+	struct ioport_operations	*ops;
+};
+
+static struct rb_root ioport_tree = RB_ROOT;
 bool ioport_debug;
 
+static struct ioport_entry *ioport_search(struct rb_root *root, u64 addr)
+{
+	struct rb_int_node *node;
+
+	node = rb_int_search_single(root, addr);
+	if (node == NULL)
+		return NULL;
+
+	return ioport_node(node);
+}
+
+static int ioport_insert(struct rb_root *root, struct ioport_entry *data)
+{
+	return rb_int_insert(root, &data->node);
+}
+
 static bool debug_io_out(struct kvm *kvm, u16 port, void *data, int size, u32 count)
 {
 	exit(EXIT_SUCCESS);
@@ -41,14 +67,24 @@  static struct ioport_operations dummy_write_only_ioport_ops = {
 	.io_out		= dummy_io_out,
 };
 
-static struct ioport_operations *ioport_ops[USHRT_MAX];
-
 void ioport__register(u16 port, struct ioport_operations *ops, int count)
 {
-	int i;
+	struct ioport_entry *entry;
 
-	for (i = 0; i < count; i++)
-		ioport_ops[port + i]	= ops;
+	entry = ioport_search(&ioport_tree, port);
+	if (entry)
+		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,
+	};
+
+	ioport_insert(&ioport_tree, entry);
 }
 
 static const char *to_direction(int direction)
@@ -66,12 +102,16 @@  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 = ioport_ops[port];
+	struct ioport_operations *ops;
 	bool ret;
+	struct ioport_entry *entry;
 
-	if (!ops)
+	entry = ioport_search(&ioport_tree, port);
+	if (!entry)
 		goto error;
 
+	ops = entry->ops;
+
 	if (direction == KVM_EXIT_IO_IN) {
 		if (!ops->io_in)
 			goto error;