* [PATCH 1/2] kvm tools: Fix rbtree-interval balancing
@ 2011-05-21 8:51 Sasha Levin
2011-05-21 8:51 ` [PATCH 2/2] kvm tools: Modify ioport to use interval rbtree Sasha Levin
0 siblings, 1 reply; 6+ messages in thread
From: Sasha Levin @ 2011-05-21 8:51 UTC (permalink / raw)
To: penberg; +Cc: mingo, asias.hejun, gorcunov, prasadjoshi124, kvm, Sasha Levin
Augmentation is started on the pre-rotation node found in the search,
augment the rotated node instead.
Max high is the max of max highs below it, not the max of highs below it.
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
tools/kvm/util/rbtree-interval.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/kvm/util/rbtree-interval.c b/tools/kvm/util/rbtree-interval.c
index 735e912..d02fbf0 100644
--- a/tools/kvm/util/rbtree-interval.c
+++ b/tools/kvm/util/rbtree-interval.c
@@ -51,9 +51,9 @@ static void update_node_max_high(struct rb_node *node, void *arg)
i_node->max_high = i_node->high;
if (node->rb_left)
- i_node->max_high = max(i_node->max_high, rb_int(node->rb_left)->high);
+ i_node->max_high = max(i_node->max_high, rb_int(node->rb_left)->max_high);
if (node->rb_right)
- i_node->max_high = max(i_node->max_high, rb_int(node->rb_right)->high);
+ i_node->max_high = max(i_node->max_high, rb_int(node->rb_right)->max_high);
}
int rb_int_insert(struct rb_root *root, struct rb_int_node *i_node)
@@ -75,7 +75,7 @@ int rb_int_insert(struct rb_root *root, struct rb_int_node *i_node)
rb_link_node(&i_node->node, parent, node);
rb_insert_color(&i_node->node, root);
- rb_augment_insert(*node, update_node_max_high, NULL);
+ rb_augment_insert(&i_node->node, update_node_max_high, NULL);
return 1;
}
--
1.7.5.rc3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] kvm tools: Modify ioport to use interval rbtree
2011-05-21 8:51 [PATCH 1/2] kvm tools: Fix rbtree-interval balancing Sasha Levin
@ 2011-05-21 8:51 ` Sasha Levin
2011-05-21 10:31 ` Cyrill Gorcunov
0 siblings, 1 reply; 6+ messages in thread
From: Sasha Levin @ 2011-05-21 8:51 UTC (permalink / raw)
To: penberg; +Cc: mingo, asias.hejun, gorcunov, prasadjoshi124, kvm, Sasha Levin
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(-)
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;
--
1.7.5.rc3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] kvm tools: Modify ioport to use interval rbtree
2011-05-21 8:51 ` [PATCH 2/2] kvm tools: Modify ioport to use interval rbtree Sasha Levin
@ 2011-05-21 10:31 ` Cyrill Gorcunov
2011-05-21 10:55 ` Sasha Levin
0 siblings, 1 reply; 6+ messages in thread
From: Cyrill Gorcunov @ 2011-05-21 10:31 UTC (permalink / raw)
To: Sasha Levin; +Cc: penberg, mingo, asias.hejun, prasadjoshi124, kvm
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?
--
Cyrill
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] kvm tools: Modify ioport to use interval rbtree
2011-05-21 10:31 ` Cyrill Gorcunov
@ 2011-05-21 10:55 ` Sasha Levin
2011-05-21 12:08 ` Cyrill Gorcunov
0 siblings, 1 reply; 6+ messages in thread
From: Sasha Levin @ 2011-05-21 10:55 UTC (permalink / raw)
To: Cyrill Gorcunov; +Cc: penberg, mingo, asias.hejun, prasadjoshi124, kvm
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.
--
Sasha.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] kvm tools: Modify ioport to use interval rbtree
2011-05-21 10:55 ` Sasha Levin
@ 2011-05-21 12:08 ` Cyrill Gorcunov
2011-05-21 12:16 ` Cyrill Gorcunov
0 siblings, 1 reply; 6+ messages in thread
From: Cyrill Gorcunov @ 2011-05-21 12:08 UTC (permalink / raw)
To: Sasha Levin; +Cc: penberg, mingo, asias.hejun, prasadjoshi124, kvm
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 ;)
--
Cyrill
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] kvm tools: Modify ioport to use interval rbtree
2011-05-21 12:08 ` Cyrill Gorcunov
@ 2011-05-21 12:16 ` Cyrill Gorcunov
0 siblings, 0 replies; 6+ messages in thread
From: Cyrill Gorcunov @ 2011-05-21 12:16 UTC (permalink / raw)
To: Sasha Levin, penberg; +Cc: mingo, asias.hejun, prasadjoshi124, kvm
On 05/21/2011 04:08 PM, Cyrill Gorcunov wrote:
> 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 ;)
>
What about this one on top? Pekka?
--
kvm tools: Print out a warning on io-port re-registration
We only support re-registartion of dummy io-port operations
so anything other should yield a warning and been considered
harmful until inspected and confirmed.
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
Pekka, for some reason dropping the dummy io-ports for
serial console is not what I like. To be fair -- I can't
explain why, some gut feeling ;)
tools/kvm/ioport.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
Index: linux-2.6.git/tools/kvm/ioport.c
===================================================================
--- linux-2.6.git.orig/tools/kvm/ioport.c
+++ linux-2.6.git/tools/kvm/ioport.c
@@ -72,8 +72,17 @@ void ioport__register(u16 port, struct i
struct ioport_entry *entry;
entry = ioport_search(&ioport_tree, port);
- if (entry)
+ if (entry) {
+ /*
+ * Only dummy io-port operations are supposed to be
+ * re-registered, anything other should be considered
+ * harmfull and issue warning until inspected and confirmed.
+ */
+ if (entry->ops != &dummy_read_write_ioport_ops &&
+ entry->ops != &dummy_write_only_ioport_ops)
+ pr_warning("Non-dummy ioport re-registered: %x", port);
rb_int_erase(&ioport_tree, &entry->node);
+ }
entry = malloc(sizeof(*entry));
if (entry == NULL)
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-05-21 12:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-21 8:51 [PATCH 1/2] kvm tools: Fix rbtree-interval balancing Sasha Levin
2011-05-21 8:51 ` [PATCH 2/2] kvm tools: Modify ioport to use interval rbtree Sasha Levin
2011-05-21 10:31 ` Cyrill Gorcunov
2011-05-21 10:55 ` Sasha Levin
2011-05-21 12:08 ` Cyrill Gorcunov
2011-05-21 12:16 ` Cyrill Gorcunov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.