Re: [Xen-devel] Help with test_and_clear_bit on events

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Wed, 2011-10-05 at 10:27 +0100, Daniel Castro wrote:
> On Tue, Oct 4, 2011 at 11:11 PM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> > On Tue, 2011-10-04 at 14:19 +0100, Ian Campbell wrote:
> >
> > Two more bugs:
> >
> > A domid is 16 bits not 64, see struct xen_add_to_physmap. So the
> > structure passed to the hypercall doesn't line up and you don't really
> > end up with the shared info in the right place.
> >
> > Secondly in:
> >
> >        int wait = test_and_clear_bit(event, shinfo->evtchn_pending);
> >        int ret = 1;
> >        while (!wait || ret){
> >                ret = hypercall_sched_op(SCHEDOP_poll, &poll);
> >                wait = test_and_clear_bit(event, shinfo->evtchn_pending);
> >        }
> >
> > This will always go through the loop at least once, even if wait is
> > already true because ret == 1. But if wait was already one then you have
> > cleared the bit so wait will then become 0 on the first time through the
> > loop and since you won't get any more events you get stuck in the loop.
> > I think you meant && !ret or something, but really you should be doing
> > BUG on !ret.
> >
> > With all those fixed things seem to work for me. I do
> > xenstore-write /local/domain/$(xl domid dHVM-1)/device/vbd/768/test foo
> > twice to get through the test code and it continues on to boot the
> > guest.
> 
> Thank you Ian, it works flawlessly... Sorry about the bug. Github repo
> is up to date.
> With this, do you consider that any other xenbus functionality should
> be added? It can read, write, directory and watch. Maybe, unwatch?

Now that you have the basic infrastructure working I think you can just
add what you need as you need it.

I suspect you won't actually need watch/unwatch since you can just poll
the node.

> Shall I continue with the blk rings?

Yeah!

> 
> >
> > My patch (with debug and hacks included) is below.
> >
> > Ian.
> >
> > diff --git a/src/xen-xs.c b/src/xen-xs.c
> > index 9f583f9..4c208d4 100644
> > --- a/src/xen-xs.c
> > +++ b/src/xen-xs.c
> > @@ -126,14 +126,21 @@ static void ring_wait(void)
> >        memset(&poll, 0, sizeof(poll));
> >        set_xen_guest_handle(poll.ports, &event);
> >        poll.nr_ports = 1;
> > -       dprintf(1,"evtchn_pending 0x%p , 0x%lx at event %d 
> > \n",shinfo->evtchn_pending,shinfo->evtchn_pending[event],event);
> > +       dprintf(1, "shared info %p\n", shinfo);
> > +       dprintf(1,"evtchn_pending %p , 0x%lx at event %d 
> > \n",shinfo->evtchn_pending,shinfo->evtchn_pending[0],event);
> > +       while ( !test_and_clear_bit(event, shinfo->evtchn_pending) )
> > +               hypercall_sched_op(SCHEDOP_poll, &poll);
> > +
> > +#if 0
> >        int wait = test_and_clear_bit(event, shinfo->evtchn_pending);
> > -       int ret = 1;
> > +       int ret = 0;
> > +       dprintf(1,"DEBUG bit clear is %d and ret %d\n",wait,ret);
> >        while (!wait || ret){
> >                ret = hypercall_sched_op(SCHEDOP_poll, &poll);
> >                wait = test_and_clear_bit(event, shinfo->evtchn_pending);
> >                dprintf(1,"DEBUG bit clear is %d and ret %d\n",wait,ret);
> >        }
> > +#endif
> >  }
> >
> >  static void ring_wait2(void)
> > diff --git a/src/xen.c b/src/xen.c
> > index e8469b0..312c206 100644
> > --- a/src/xen.c
> > +++ b/src/xen.c
> > @@ -154,11 +154,20 @@ struct shared_info *get_shared_info(void)
> >     xatp.domid = DOMID_SELF;
> >     xatp.space = XENMAPSPACE_shared_info;
> >     xatp.idx   = 0;
> > -    shared_info = malloc_high(sizeof(shared_info));
> > -    xatp.gpfn  = ((unsigned long)shared_info << PAGE_SHIFT);
> > +    shared_info = memalign_high(PAGE_SIZE, PAGE_SIZE);
> 
> Here was my big mistake...
> 
> > +    memset(shared_info, 0, PAGE_SIZE);
> > +    xatp.gpfn  = ((unsigned long)shared_info >> PAGE_SHIFT);
> > +    dprintf(1, "allocated shared info %d bytes at %p, gpfn 0x%lx\n",
> > +           sizeof(*shared_info), shared_info, xatp.gpfn);
> >     //xatp.gpfn  = malloc_high(sizeof(shared_info));
> >     //shared_info = (struct shared_info *)(xatp.gpfn << PAGE_SHIFT);
> >     if (hypercall_memory_op(XENMEM_add_to_physmap, &xatp) != 0)
> >         panic("MAP info page fail");
> > +    dprintf(1, "time is %d\n", shared_info->wc_sec);
> > +    dprintf(1, "evtchn_pending[0] 0x%08lx\n", 
> > shared_info->evtchn_pending[0]);
> > +    dprintf(1, "evtchn_mask[0] 0x%08lx\n", shared_info->evtchn_mask[0]);
> > +    dprintf(1, "VCPU0 evtchn_upcall_pending 0x%x\n", 
> > shared_info->vcpu_info[0].evtchn_upcall_pending);
> > +    dprintf(1, "VCPU0 evtchn_upcall_mask 0x%x\n", 
> > shared_info->vcpu_info[0].evtchn_upcall_mask);
> > +    dprintf(1, "VCPU0 evtchn_pending_sel 0x%08lx\n", 
> > shared_info->vcpu_info[0].evtchn_pending_sel);
> >     return shared_info;
> >  }
> > diff --git a/src/xen.h b/src/xen.h
> > index 1317af9..d838a7c 100644
> > --- a/src/xen.h
> > +++ b/src/xen.h
> > @@ -480,7 +480,7 @@ static struct xsd_errors __attribute__ ((unused)) 
> > xsd_errors[]
> >  #define XENMEM_add_to_physmap      7
> >  struct xen_add_to_physmap {
> >     /* Which domain to change the mapping for. */
> > -    u64 domid;
> > +    u16 domid;
> >
> >     /* Source mapping space. */
> >  #define XENMAPSPACE_shared_info 0 /* shared info page */
> > @@ -503,7 +503,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_add_to_physmap_t);
> >  /*
> >  * Wrappers for hypercalls
> >  */
> > -static int hypercall_xen_version( int cmd, void *arg)
> > +static inline int hypercall_xen_version( int cmd, void *arg)
> >  {
> >        return _hypercall2(int, xen_version, cmd, arg);
> >  }
> > @@ -520,7 +520,7 @@ static inline int hypercall_event_channel_op(int cmd, 
> > void *arg)
> >
> >  static inline int hypercall_memory_op(int cmd ,void *arg)
> >  {
> > -       return _hypercall2(int, memory_op, cmd ,arg);
> > +       return _hypercall2(int, memory_op, cmd, arg);
> >  }
> >
> >  static inline int hypercall_sched_op(int cmd, void *arg)
> >
> >
> >
> 
> 
> 



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