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

[Cyrill Gorcunov <gorcunov@gmail.com>]

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)