Re: [Xen-devel] [PATCH] Allow mini-os to unbind all evtchns

[Grzegorz Milos <gm281@xxxxxxxxx>]

I've reworked the patch a little bit. I've added "flags" field to 
ev_action_t instead of creating a separate bitmap. The basic idea is 
still the some though.
This patch makes sure that all the ports are marked unbound on init, 
which your patch failed to do.
We can merge flags and count fields together, if we care about the space 
wasted.
Cheers
Gregor

(Keir could you apply please?)

Jacob Gorm Hansen wrote:
> On Wed, 2006-11-29 at 12:25 +0000, Keir Fraser wrote:
> > On 29/11/06 12:16, "Grzegorz Milos" <gm281@xxxxxxxxx> wrote:
> >
> > > The patch looks good to me. I only have one comment: it would be good
> > > to have unbind_all_ports in the events header file.
> > It looked bogus to me. What if event channels are bound/unbound/bound
> > multiple times during the mini-os lifetime (can't happen right now, but
> > maybe in future). The port array will overflow. Perhaps it should be a
> > bitmap instead (set on bind; clear on unbind)?
> I have attached a new version that uses a bitmap instead and declares
> unbind_all_ports() in events.h.
>
> regards,
> Jacob
>
>
> ------------------------------------------------------------------------
>
> Signed-off-by: Jacob Gorm Hansen <jacobg@xxxxxxx>
>
> diff -r 52ae8dd4bc75 extras/mini-os/events.c
> --- a/extras/mini-os/events.c   Tue Oct 17 22:09:52 2006 +0100
> +++ b/extras/mini-os/events.c   Thu Nov 30 17:34:29 2006 +0100
> @@ -31,10 +31,26 @@ typedef struct _ev_action_t {
>      u32 count;
>  } ev_action_t;
>  
> +unsigned long bound_ports[NR_EVS/(8*sizeof(unsigned long))];
>  
>  static ev_action_t ev_actions[NR_EVS];
>  void default_handler(evtchn_port_t port, struct pt_regs *regs, void *data);
>  
> +void unbind_all_ports(void)
> +{
> +    int i;
> +
> +    for(i=0; i<NR_EVS; i++)
> +    {
> +        if(test_and_clear_bit(i,bound_ports))
> +        {
> +            struct evtchn_close close;
> +            mask_evtchn(i);
> +            close.port = i;
> +            HYPERVISOR_event_channel_op(EVTCHNOP_close, &close);
> +        }
> +    }
> +}
>  
>  /*
>   * Demux events to different handlers.
> @@ -99,6 +115,7 @@ int bind_virq(uint32_t virq, evtchn_hand
>                 printk("Failed to bind virtual IRQ %d\n", virq);
>                 return 1;
>      }
> +    set_bit(op.port,bound_ports);
>      bind_evtchn(op.port, handler, data);
>         return 0;
>  }
> @@ -173,6 +190,7 @@ int evtchn_bind_interdomain(domid_t pal,
>      int err = HYPERVISOR_event_channel_op(EVTCHNOP_bind_interdomain, &op);
>      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! */
>      *local_port = bind_evtchn(port, handler, data);
> diff -r 52ae8dd4bc75 extras/mini-os/include/events.h
> --- a/extras/mini-os/include/events.h   Tue Oct 17 22:09:52 2006 +0100
> +++ b/extras/mini-os/include/events.h   Thu Nov 30 17:34:29 2006 +0100
> @@ -36,6 +36,7 @@ int evtchn_bind_interdomain(domid_t pal,
>  int evtchn_bind_interdomain(domid_t pal, evtchn_port_t remote_port,
>                                                         evtchn_handler_t 
> handler, void *data,
>                                                         evtchn_port_t 
> *local_port);
> +void unbind_all_ports(void);
>  
>  static inline int notify_remote_via_evtchn(evtchn_port_t port)
>  {
diff -r ae47b592ae32 extras/mini-os/events.c
--- a/extras/mini-os/events.c   Tue Nov 21 21:38:51 2006 +0100
+++ b/extras/mini-os/events.c   Wed Dec 06 22:18:52 2006 +0000
@@ -23,18 +23,35 @@
 #include <lib.h>
 
 #define NR_EVS 1024
+#define EVNTS_PORT_BOUND    0x00000001 
 
 /* this represents a event handler. Chaining or sharing is not allowed */
 typedef struct _ev_action_t {
        evtchn_handler_t handler;
        void *data;
     u32 count;
+    u32 flags; 
 } ev_action_t;
-
 
 static ev_action_t ev_actions[NR_EVS];
 void default_handler(evtchn_port_t port, struct pt_regs *regs, void *data);
 
+void unbind_all_ports(void)
+{
+    int i;
+
+    for(i=0; i<NR_EVS; i++)
+    {
+        if(ev_actions[i].flags & EVNTS_PORT_BOUND)
+        {
+            struct evtchn_close close;
+            mask_evtchn(i);
+            close.port = i;
+            HYPERVISOR_event_channel_op(EVTCHNOP_close, &close);
+            ev_actions[i].flags &= ~EVNTS_PORT_BOUND;
+        }
+    }
+}
 
 /*
  * Demux events to different handlers.
@@ -99,6 +116,7 @@ int bind_virq(uint32_t virq, evtchn_hand
                printk("Failed to bind virtual IRQ %d\n", virq);
                return 1;
     }
+    ev_actions[op.port].flags |= EVNTS_PORT_BOUND;
     bind_evtchn(op.port, handler, data);
        return 0;
 }
@@ -131,6 +149,8 @@ void init_events(void)
     for ( i = 0; i < NR_EVS; i++ )
        {
         ev_actions[i].handler = default_handler;
+        /* Clear all flags, EVNTS_PORT_BOUND in particular */
+        ev_actions[i].flags = 0;
         mask_evtchn(i);
     }
 }
@@ -174,6 +194,7 @@ int evtchn_bind_interdomain(domid_t pal,
     if (err)
                return err;
        evtchn_port_t port = op.local_port;
+    ev_actions[port].flags |= EVNTS_PORT_BOUND;
     clear_evtchn(port);              /* Without, handler gets invoked now! */
     *local_port = bind_evtchn(port, handler, data);
     return err;
diff -r ae47b592ae32 extras/mini-os/include/events.h
--- a/extras/mini-os/include/events.h   Tue Nov 21 21:38:51 2006 +0100
+++ b/extras/mini-os/include/events.h   Wed Dec 06 22:08:28 2006 +0000
@@ -36,6 +36,7 @@ int evtchn_bind_interdomain(domid_t pal,
 int evtchn_bind_interdomain(domid_t pal, evtchn_port_t remote_port,
                                                        evtchn_handler_t 
handler, void *data,
                                                        evtchn_port_t 
*local_port);
+void unbind_all_ports(void);
 
 static inline int notify_remote_via_evtchn(evtchn_port_t port)
 {

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel