We have been seeing the same problem with a 32bit Mini-OS guest.
I suspect the problem is in the initialisation order. Looking at
start_kernel() in kernel.c:
arch_init(si);
trap_init();
/* ENABLE EVENT DELIVERY. This is disabled at start of day. */
__sti();
<code omitted...>
/* Set up events. */
init_events();
The function arch_init() registers hypervisor_callback, and __sti()
enables events to be delivered. Entry point hypervisor_callback is in
the assembly code in x86_32.S which calls do_hypervisor_callback() in
hypervisor.c, which in turn calls do_event() in events.c.
That's all correct so far.
Suppose a callback occurs between the calls of __sti() and
init_events(). The function do_event() calls the handler indirectly via
the array ev_actions. But ev_actions is initialised in init_events(),
so if do_event() is called too early, the function pointer "handler" in
ev_actions will still be 0 (default for static storage). We start again
at virtual address 0, which is the entry point of Mini-OS, but %esi will
not now point to the start_info page. I think this explains why Mini-OS
sometimes gets "restarted", and when it does the start_info page seems
to be garbage.
This seems to explain the "restart". However, my understanding was that
an event port has to be explicitly unmasked in order for any event to
be delivered (i.e. that all event ports are masked by default).
Unmasking is done by bind_evtchn() or bind_virq() in event.s. All calls
to these functions happen after init_events() (in init_time(),
init_console() and init_xenbus()). I guess the best way to verify if
your scenario is correct is to put 'printk' in do_events(). Could you
try this Melvin?
I am not convinced that Grzegorz' patch closes the window of opportunity
for a misplaced callback, but it does reduce it. Shouldn't __sti() be
after init_events(), not before it?
That certainly cannot hurt. I'm attaching a patch. Keir could you apply?
Patch description:
Events enabled after init_events() call. This guarantees that internal
datastructures are set up correctly. Problem spotted/explained by Melvin
Anderson and Micha Moffie.
Signed-off-by: Grzegorz Milos <gm281@xxxxxxxxx>
Thanks are due to Micha Moffie, who came up with key insights.
diff -r f7cc9ce481d9 -r 669ea1ab20d4 extras/mini-os/kernel.c
--- a/extras/mini-os/kernel.c Wed Nov 15 22:35:24 2006 +0000
+++ b/extras/mini-os/kernel.c Fri Nov 17 15:22:54 2006 +0000
@@ -100,10 +100,7 @@ void start_kernel(start_info_t *si)
arch_init(si);
trap_init();
-
- /* ENABLE EVENT DELIVERY. This is disabled at start of day. */
- __sti();
-
+
/* print out some useful information */
printk("Xen Minimal OS!\n");
printk("start_info: %p\n", si);
@@ -118,6 +115,9 @@ void start_kernel(start_info_t *si)
/* Set up events. */
init_events();
+
+ /* ENABLE EVENT DELIVERY. This is disabled at start of day. */
+ __sti();
arch_print_info();
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
| 
Home