Skip to contentSkip to conversation

minios: Fix lost events

PATCH New Samuel Thibault 1 participant 1 message Mar 17, 2008

lore
Share & tools
lore

Samuel Thibault

author
#0 lore raw

PATCH 1/1 minios: Fix lost events

minios: Fix lost events

evtchn_bind_interdomain used to clear any already pending event before binding a handler, because else the handler may be called before it is ready. That however leads to missed events, which I had to workaround for the HVM case.

This changes the semantics of bind_evtchn, and thus of all the event channel binding functions (bind_virq, evtchn_alloc_unbound, evtchn_bind_interdomain) into not unmasking the event itself, hence letting the caller initialize properly before unmasking the port (e.g. record the port number in an appropriate place).

Signed-off-by Samuel Thibault <[email protected]>
extras/mini-os/arch/ia64/time.c +5 -4
diff -r c149cedf76b9 -r d4b232bea2eb extras/mini-os/arch/ia64/time.c
@@ -246,7 +246,7 @@ init_time(void)
 {
 	uint64_t new;
 	efi_time_t tm;
-	int err = 0;
+	evtchn_port_t port = 0;
 
 	printk("Initialising time\n");
 	calculate_frequencies();

@@ -267,11 +267,12 @@ init_time(void)
 	} else
 		printk("efi_get_time() failed\n");
 
-	err = bind_virq(VIRQ_ITC, timer_interrupt, NULL);
-	if (err == -1) {
-		printk("XEN timer request chn bind failed %i\n", err);
+	port = bind_virq(VIRQ_ITC, timer_interrupt, NULL);
+	if (port == -1) {
+		printk("XEN timer request chn bind failed %i\n", port);
 		return;
 	}
+        unmask_evtchn(port);
 	itc_alt = ia64_get_itc();
 	itc_at_boot = itc_alt;
 	new = ia64_get_itc() + itm_val;
extras/mini-os/arch/x86/time.c +3 -1
diff -r c149cedf76b9 -r d4b232bea2eb extras/mini-os/arch/x86/time.c
@@ -222,6 +222,8 @@ static void timer_handler(evtchn_port_t 
 
 void init_time(void)
 {
+    evtchn_port_t port;
     printk("Initialising timer interface\n");
-    bind_virq(VIRQ_TIMER, &timer_handler, NULL);
+    port = bind_virq(VIRQ_TIMER, &timer_handler, NULL);
+    unmask_evtchn(port);
 }
extras/mini-os/blkfront.c +5 -9
diff -r c149cedf76b9 -r d4b232bea2eb extras/mini-os/blkfront.c
@@ -43,7 +43,7 @@ struct blkfront_dev {
 
     struct blkif_front_ring ring;
     grant_ref_t ring_ref;
-    evtchn_port_t evtchn, local_port;
+    evtchn_port_t evtchn;
     blkif_vdev_t handle;
 
     char *nodename;

@@ -92,14 +92,9 @@ struct blkfront_dev *init_blkfront(char 
     dev = malloc(sizeof(*dev));
     dev->nodename = strdup(nodename);
 
-    evtchn_alloc_unbound_t op;
-    op.dom = DOMID_SELF;
     snprintf(path, sizeof(path), "%s/backend-id", nodename);
-    dev->dom = op.remote_dom = xenbus_read_integer(path); 
-    HYPERVISOR_event_channel_op(EVTCHNOP_alloc_unbound, &op);
-    clear_evtchn(op.port);        /* Without, handler gets invoked now! */
-    dev->local_port = bind_evtchn(op.port, blkfront_handler, dev);
-    dev->evtchn=op.port;
+    dev->dom = xenbus_read_integer(path); 
+    evtchn_alloc_unbound(dev->dom, blkfront_handler, dev, &dev->evtchn);
 
     s = (struct blkif_sring*) alloc_page();
     memset(s,0,PAGE_SIZE);

@@ -194,6 +189,7 @@ done:
         snprintf(path, sizeof(path), "%s/feature-flush-cache", dev->backend);
         dev->flush = xenbus_read_integer(path);
     }
+    unmask_evtchn(dev->evtchn);
 
     printk("%u sectors of %u bytes\n", dev->sectors, dev->sector_size);
     printk("**************************\n");

@@ -219,7 +215,7 @@ void shutdown_blkfront(struct blkfront_d
     err = xenbus_printf(XBT_NIL, nodename, "state", "%u", 6);
     xenbus_wait_for_value(path,"6");
 
-    unbind_evtchn(dev->local_port);
+    unbind_evtchn(dev->evtchn);
 
     free(nodename);
     free(dev->backend);
extras/mini-os/console/xencons_ring.c +1 -0
diff -r c149cedf76b9 -r d4b232bea2eb extras/mini-os/console/xencons_ring.c
@@ -91,6 +91,7 @@ int xencons_ring_init(void)
 		printk("XEN console request chn bind failed %i\n", err);
 		return err;
 	}
+        unmask_evtchn(start_info.console.domU.evtchn);
 
 	/* In case we have in-flight data after save/restore... */
 	notify_daemon();
extras/mini-os/events.c +1 -5
diff -r c149cedf76b9 -r d4b232bea2eb extras/mini-os/events.c
@@ -86,9 +86,6 @@ evtchn_port_t bind_evtchn(evtchn_port_t 
 	ev_actions[port].data = data;
 	wmb();
 	ev_actions[port].handler = handler;
-
-	/* Finally unmask the port */
-	unmask_evtchn(port);
 
 	return port;
 }

@@ -191,8 +188,7 @@ int evtchn_bind_interdomain(domid_t pal,
     if (err)
 		return err;
     set_bit(op.local_port,bound_ports);
-	evtchn_port_t port = op.local_port;
-    clear_evtchn(port);	      /* Without, handler gets invoked now! */
+    evtchn_port_t port = op.local_port;
     *local_port = bind_evtchn(port, handler, data);
     return err;
 }
extras/mini-os/fbfront.c +10 -18
diff -r c149cedf76b9 -r d4b232bea2eb extras/mini-os/fbfront.c
@@ -26,7 +26,7 @@ struct kbdfront_dev {
     domid_t dom;
 
     struct xenkbd_page *page;
-    evtchn_port_t evtchn, local_port;
+    evtchn_port_t evtchn;
 
     char *nodename;
     char *backend;

@@ -68,14 +68,9 @@ struct kbdfront_dev *init_kbdfront(char 
     dev = malloc(sizeof(*dev));
     dev->nodename = strdup(nodename);
 
-    evtchn_alloc_unbound_t op;
-    op.dom = DOMID_SELF;
     snprintf(path, sizeof(path), "%s/backend-id", nodename);
-    dev->dom = op.remote_dom = xenbus_read_integer(path); 
-    HYPERVISOR_event_channel_op(EVTCHNOP_alloc_unbound, &op);
-    clear_evtchn(op.port);        /* Without, handler gets invoked now! */
-    dev->local_port = bind_evtchn(op.port, kbdfront_handler, dev);
-    dev->evtchn=op.port;
+    dev->dom = xenbus_read_integer(path); 
+    evtchn_alloc_unbound(dev->dom, kbdfront_handler, dev, &dev->evtchn);
 
     dev->page = s = (struct xenkbd_page*) alloc_page();
     memset(s,0,PAGE_SIZE);

@@ -151,6 +146,7 @@ done:
 
         err = xenbus_printf(XBT_NIL, nodename, "state", "%u", 4); /* connected */
     }
+    unmask_evtchn(dev->evtchn);
 
     printk("************************** KBDFRONT\n");
 

@@ -208,7 +204,7 @@ void shutdown_kbdfront(struct kbdfront_d
     err = xenbus_printf(XBT_NIL, nodename, "state", "%u", 6);
     xenbus_wait_for_value(path,"6");
 
-    unbind_evtchn(dev->local_port);
+    unbind_evtchn(dev->evtchn);
 
     free_pages(dev->page,0);
     free(nodename);

@@ -241,7 +237,7 @@ struct fbfront_dev {
     domid_t dom;
 
     struct xenfb_page *page;
-    evtchn_port_t evtchn, local_port;
+    evtchn_port_t evtchn;
 
     char *nodename;
     char *backend;

@@ -282,14 +278,9 @@ struct fbfront_dev *init_fbfront(char *n
     dev = malloc(sizeof(*dev));
     dev->nodename = strdup(nodename);
 
-    evtchn_alloc_unbound_t op;
-    op.dom = DOMID_SELF;
     snprintf(path, sizeof(path), "%s/backend-id", nodename);
-    dev->dom = op.remote_dom = xenbus_read_integer(path); 
-    HYPERVISOR_event_channel_op(EVTCHNOP_alloc_unbound, &op);
-    clear_evtchn(op.port);        /* Without, handler gets invoked now! */
-    dev->local_port = bind_evtchn(op.port, fbfront_handler, dev);
-    dev->evtchn=op.port;
+    dev->dom = xenbus_read_integer(path); 
+    evtchn_alloc_unbound(dev->dom, fbfront_handler, dev, &dev->evtchn);
 
     dev->page = s = (struct xenfb_page*) alloc_page();
     memset(s,0,PAGE_SIZE);

@@ -399,6 +390,7 @@ done:
 
         err = xenbus_printf(XBT_NIL, nodename, "state", "%u", 4); /* connected */
     }
+    unmask_evtchn(dev->evtchn);
 
     printk("************************** FBFRONT\n");
 

@@ -485,7 +477,7 @@ void shutdown_fbfront(struct fbfront_dev
     err = xenbus_printf(XBT_NIL, nodename, "state", "%u", 6);
     xenbus_wait_for_value(path,"6");
 
-    unbind_evtchn(dev->local_port);
+    unbind_evtchn(dev->evtchn);
 
     free_pages(dev->page,0);
     free(nodename);
extras/mini-os/fs-front.c +1 -0
diff -r c149cedf76b9 -r d4b232bea2eb extras/mini-os/fs-front.c
@@ -943,6 +943,7 @@ static int init_fs_import(struct fs_impo
                                 //ANY_CPU, 
                                 import, 
                                 &import->local_port));
+    unmask_evtchn(import->local_port);
 
     
     self_id = get_self_id();
extras/mini-os/netfront.c +8 -12
diff -r c149cedf76b9 -r d4b232bea2eb extras/mini-os/netfront.c
@@ -48,7 +48,7 @@ struct netfront_dev {
     struct netif_rx_front_ring rx;
     grant_ref_t tx_ring_ref;
     grant_ref_t rx_ring_ref;
-    evtchn_port_t evtchn, local_port;
+    evtchn_port_t evtchn;
 
     char *nodename;
     char *backend;

@@ -301,19 +301,14 @@ struct netfront_dev *init_netfront(char 
         dev->rx_buffers[i].page = (char*)alloc_page();
     }
 
-    evtchn_alloc_unbound_t op;
-    op.dom = DOMID_SELF;
     snprintf(path, sizeof(path), "%s/backend-id", nodename);
-    dev->dom = op.remote_dom = xenbus_read_integer(path);
-    HYPERVISOR_event_channel_op(EVTCHNOP_alloc_unbound, &op);
-    clear_evtchn(op.port);        /* Without, handler gets invoked now! */
+    dev->dom = xenbus_read_integer(path);
 #ifdef HAVE_LIBC
     if (thenetif_rx == NETIF_SELECT_RX)
-	dev->local_port = bind_evtchn(op.port, netfront_select_handler, dev);
+        evtchn_alloc_unbound(dev->dom, netfront_select_handler, dev, &dev->evtchn);
     else
 #endif
-	dev->local_port = bind_evtchn(op.port, netfront_handler, dev);
-    dev->evtchn=op.port;
+        evtchn_alloc_unbound(dev->dom, netfront_handler, dev, &dev->evtchn);
 
     txs = (struct netif_tx_sring*) alloc_page();
     rxs = (struct netif_rx_sring *) alloc_page();

@@ -388,9 +383,9 @@ done:
     msg = xenbus_read(XBT_NIL, path, &mac);
 
     if ((dev->backend == NULL) || (mac == NULL)) {
-        struct evtchn_close op = { dev->local_port };
+        struct evtchn_close op = { dev->evtchn };
         printk("%s: backend/mac failed\n", __func__);
-        unbind_evtchn(dev->local_port);
+        unbind_evtchn(dev->evtchn);
         HYPERVISOR_event_channel_op(EVTCHNOP_close, &op);
         return NULL;
     }

@@ -412,6 +407,7 @@ done:
     printk("**************************\n");
 
     init_rx_buffers(dev);
+    unmask_evtchn(dev->evtchn);
 
         /* Special conversion specifier 'hh' needed for __ia64__. Without
            this mini-os panics with 'Unaligned reference'. */

@@ -460,7 +456,7 @@ void shutdown_netfront(struct netfront_d
     err = xenbus_printf(XBT_NIL, nodename, "state", "%u", 6);
     xenbus_wait_for_value(path,"6");
 
-    unbind_evtchn(dev->local_port);
+    unbind_evtchn(dev->evtchn);
 
     free(nodename);
     free(dev->backend);
extras/mini-os/xenbus/xenbus.c +1 -0
diff -r c149cedf76b9 -r d4b232bea2eb extras/mini-os/xenbus/xenbus.c
@@ -257,6 +257,7 @@ void init_xenbus(void)
     err = bind_evtchn(start_info.store_evtchn,
 		      xenbus_evtchn_handler,
               NULL);
+    unmask_evtchn(start_info.store_evtchn);
     DEBUG("xenbus on irq %d\n", err);
 }
tools/libxc/xc_minios.c +3 -11
diff -r c149cedf76b9 -r d4b232bea2eb tools/libxc/xc_minios.c
@@ -165,14 +165,6 @@ static int port_alloc(int xce_handle) {
     return i;
 }
 
-static void poke_port(int xce_handle, evtchn_port_t port)
-{
-    shared_info_t *s = HYPERVISOR_shared_info;
-    printk("poking port %d\n", port);
-    synch_set_bit(port, &s->evtchn_pending[0]);
-    xc_evtchn_unmask(xce_handle, port);
-}
-
 static void evtchn_handler(evtchn_port_t port, struct pt_regs *regs, void *data)
 {
     int xce_handle = (intptr_t) data;

@@ -211,6 +203,7 @@ evtchn_port_or_error_t xc_evtchn_bind_un
     }
     files[xce_handle].evtchn.ports[i].bound = 1;
     files[xce_handle].evtchn.ports[i].port = port;
+    unmask_evtchn(port);
     return port;
 }
 

@@ -235,9 +228,7 @@ evtchn_port_or_error_t xc_evtchn_bind_in
     }
     files[xce_handle].evtchn.ports[i].bound = 1;
     files[xce_handle].evtchn.ports[i].port = local_port;
-/* Poke port on start: HVM won't send an event for the very first request since
- * we were not ready yet */
-    poke_port(xce_handle, local_port);
+    unmask_evtchn(local_port);
     return local_port;
 }
 

@@ -275,6 +266,7 @@ evtchn_port_or_error_t xc_evtchn_bind_vi
     }
     files[xce_handle].evtchn.ports[i].bound = 1;
     files[xce_handle].evtchn.ports[i].port = port;
+    unmask_evtchn(port);
     return port;
 }