* [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.