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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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.

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);
+    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