[Qemu-devel] [PULL 0/8] usb patch queue

[Qemu-devel] [PULL 0/8] usb patch queue

[Gerd Hoffmann]

  Hi,

Here is the current usb patch queue.  What is in there?

Migration support for USB devices.  For starters hub and HID devices are
covered.

Event queues for the usb mouse/table (thanks to Paolo and the Xen folks)
and the usb keyboard, so we can have the queue included in the migration
state right from the start and avoid the compatibility issues we would
get when adding this later on.

Also includes is a warning fix from blueswirl.

please pull,
  Gerd

The following changes since commit 0bfe006c5380c5f8a485a55ded3329fbbc224396:

  multiboot: Fix upper memory size in multiboot info (2011-01-23 22:44:13 +0100)

are available in the git repository at:
  git://anongit.freedesktop.org/spice/qemu usb.5

Blue Swirl (1):
      usb-bus: use snprintf

Gerd Hoffmann (6):
      usb keyboard: add event event queue
      usb hid: move head+n to common struct
      vnc: fix numlock+capslock tracking
      usb core: add migration support
      usb hub: add migration support
      usb hid: add migration support

Paolo Bonzini (1):
      add event queueing to USB HID

 hw/hw.h      |   10 ++
 hw/usb-bus.c |   28 ++++-
 hw/usb-hid.c |  317 ++++++++++++++++++++++++++++++++++++++++------------------
 hw/usb-hub.c |   24 +++++
 hw/usb.h     |   10 +-
 ui/vnc.c     |    4 +-
 6 files changed, 282 insertions(+), 111 deletions(-)

[Qemu-devel] [PATCH 1/8] add event queueing to USB HID

[Gerd Hoffmann]

From: Paolo Bonzini <pbonzini@redhat.com>

The polling nature of the USB HID device makes it very hard to double
click or drag while on a high-latency VNC connection.  This patch,
based on work done in the Xen qemu-dm tree by Ian Jackson, fixes this
bug by adding an event queue to the device.  The event queue associates
each movement with the correct button state, and remembers all button
presses and releases as well.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Gerd Hoffman <kraxel@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb-hid.c |  221 +++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 127 insertions(+), 94 deletions(-)

diff --git a/hw/usb-hid.c b/hw/usb-hid.c
index 90a2b49..ea49543 100644
--- a/hw/usb-hid.c
+++ b/hw/usb-hid.c
@@ -45,9 +45,19 @@
 #define USB_TABLET    2
 #define USB_KEYBOARD  3
 
+typedef struct USBPointerEvent {
+    int32_t xdx, ydy; /* relative iff it's a mouse, otherwise absolute */
+    int32_t dz, buttons_state;
+} USBPointerEvent;
+
+#define QUEUE_LENGTH    16 /* should be enough for a triple-click */
+#define QUEUE_MASK      (QUEUE_LENGTH-1u)
+#define QUEUE_INCR(v)   ((v)++, (v) &= QUEUE_MASK)
+
 typedef struct USBMouseState {
-    int dx, dy, dz, buttons_state;
-    int x, y;
+    USBPointerEvent queue[QUEUE_LENGTH];
+    uint32_t head; /* index into circular queue */
+    uint32_t n;
     int mouse_grabbed;
     QEMUPutMouseEntry *eh_entry;
 } USBMouseState;
@@ -433,31 +443,50 @@ static void usb_hid_changed(USBHIDState *hs)
     usb_wakeup(&hs->dev);
 }
 
-static void usb_mouse_event(void *opaque,
-                            int dx1, int dy1, int dz1, int buttons_state)
-{
-    USBHIDState *hs = opaque;
-    USBMouseState *s = &hs->ptr;
-
-    s->dx += dx1;
-    s->dy += dy1;
-    s->dz += dz1;
-    s->buttons_state = buttons_state;
+static void usb_pointer_event_clear(USBPointerEvent *e, int buttons) {
+    e->xdx = e->ydy = e->dz = 0;
+    e->buttons_state = buttons;
+}
 
-    usb_hid_changed(hs);
+static void usb_pointer_event_combine(USBPointerEvent *e, int xyrel,
+                                      int x1, int y1, int z1) {
+    if (xyrel) {
+        e->xdx += x1;
+        e->ydy += y1;
+    } else {
+        e->xdx = x1;
+        e->ydy = y1;
+    }
+    e->dz += z1;
 }
 
-static void usb_tablet_event(void *opaque,
-			     int x, int y, int dz, int buttons_state)
+static void usb_pointer_event(void *opaque,
+                              int x1, int y1, int z1, int buttons_state)
 {
     USBHIDState *hs = opaque;
     USBMouseState *s = &hs->ptr;
-
-    s->x = x;
-    s->y = y;
-    s->dz += dz;
-    s->buttons_state = buttons_state;
-
+    unsigned use_slot = (s->head + s->n - 1) & QUEUE_MASK;
+    unsigned previous_slot = (use_slot - 1) & QUEUE_MASK;
+
+    /* We combine events where feasible to keep the queue small.  We shouldn't
+     * combine anything with the first event of a particular button state, as
+     * that would change the location of the button state change.  When the
+     * queue is empty, a second event is needed because we don't know if
+     * the first event changed the button state.  */
+    if (s->n == QUEUE_LENGTH) {
+        /* Queue full.  Discard old button state, combine motion normally.  */
+        s->queue[use_slot].buttons_state = buttons_state;
+    } else if (s->n < 2 ||
+               s->queue[use_slot].buttons_state != buttons_state ||
+               s->queue[previous_slot].buttons_state != s->queue[use_slot].buttons_state) {
+        /* Cannot or should not combine, so add an empty item to the queue.  */
+        QUEUE_INCR(use_slot);
+        s->n++;
+        usb_pointer_event_clear(&s->queue[use_slot], buttons_state);
+    }
+    usb_pointer_event_combine(&s->queue[use_slot],
+                              hs->kind == USB_MOUSE,
+                              x1, y1, z1);
     usb_hid_changed(hs);
 }
 
@@ -528,83 +557,91 @@ static inline int int_clamp(int val, int vmin, int vmax)
         return val;
 }
 
-static int usb_mouse_poll(USBHIDState *hs, uint8_t *buf, int len)
+static int usb_pointer_poll(USBHIDState *hs, uint8_t *buf, int len)
 {
     int dx, dy, dz, b, l;
+    int index;
     USBMouseState *s = &hs->ptr;
+    USBPointerEvent *e;
 
     if (!s->mouse_grabbed) {
         qemu_activate_mouse_event_handler(s->eh_entry);
-	s->mouse_grabbed = 1;
+        s->mouse_grabbed = 1;
     }
 
-    dx = int_clamp(s->dx, -127, 127);
-    dy = int_clamp(s->dy, -127, 127);
-    dz = int_clamp(s->dz, -127, 127);
-
-    s->dx -= dx;
-    s->dy -= dy;
-    s->dz -= dz;
+    /* When the buffer is empty, return the last event.  Relative
+       movements will all be zero.  */
+    index = (s->n ? s->head : s->head - 1);
+    e = &s->queue[index & QUEUE_MASK];
 
-    /* Appears we have to invert the wheel direction */
-    dz = 0 - dz;
+    if (hs->kind == USB_MOUSE) {
+        dx = int_clamp(e->xdx, -127, 127);
+        dy = int_clamp(e->ydy, -127, 127);
+        e->xdx -= dx;
+        e->ydy -= dy;
+    } else {
+        dx = e->xdx;
+        dy = e->ydy;
+    }
+    dz = int_clamp(e->dz, -127, 127);
+    e->dz -= dz;
 
     b = 0;
-    if (s->buttons_state & MOUSE_EVENT_LBUTTON)
+    if (e->buttons_state & MOUSE_EVENT_LBUTTON)
         b |= 0x01;
-    if (s->buttons_state & MOUSE_EVENT_RBUTTON)
+    if (e->buttons_state & MOUSE_EVENT_RBUTTON)
         b |= 0x02;
-    if (s->buttons_state & MOUSE_EVENT_MBUTTON)
+    if (e->buttons_state & MOUSE_EVENT_MBUTTON)
         b |= 0x04;
 
-    l = 0;
-    if (len > l)
-        buf[l ++] = b;
-    if (len > l)
-        buf[l ++] = dx;
-    if (len > l)
-        buf[l ++] = dy;
-    if (len > l)
-        buf[l ++] = dz;
-    return l;
-}
-
-static int usb_tablet_poll(USBHIDState *hs, uint8_t *buf, int len)
-{
-    int dz, b, l;
-    USBMouseState *s = &hs->ptr;
-
-    if (!s->mouse_grabbed) {
-        qemu_activate_mouse_event_handler(s->eh_entry);
-	s->mouse_grabbed = 1;
+    if (s->n &&
+        !e->dz &&
+        (hs->kind == USB_TABLET || (!e->xdx && !e->ydy))) {
+        /* that deals with this event */
+        QUEUE_INCR(s->head);
+        s->n--;
     }
 
-    dz = int_clamp(s->dz, -127, 127);
-    s->dz -= dz;
-
     /* Appears we have to invert the wheel direction */
     dz = 0 - dz;
-    b = 0;
-    if (s->buttons_state & MOUSE_EVENT_LBUTTON)
-        b |= 0x01;
-    if (s->buttons_state & MOUSE_EVENT_RBUTTON)
-        b |= 0x02;
-    if (s->buttons_state & MOUSE_EVENT_MBUTTON)
-        b |= 0x04;
+    l = 0;
+    switch (hs->kind) {
+    case USB_MOUSE:
+        if (len > l)
+            buf[l++] = b;
+        if (len > l)
+            buf[l++] = dx;
+        if (len > l)
+            buf[l++] = dy;
+        if (len > l)
+            buf[l++] = dz;
+        break;
 
-    buf[0] = b;
-    buf[1] = s->x & 0xff;
-    buf[2] = s->x >> 8;
-    buf[3] = s->y & 0xff;
-    buf[4] = s->y >> 8;
-    buf[5] = dz;
-    l = 6;
+    case USB_TABLET:
+        if (len > l)
+            buf[l++] = b;
+        if (len > l)
+            buf[l++] = dx & 0xff;
+        if (len > l)
+            buf[l++] = dx >> 8;
+        if (len > l)
+            buf[l++] = dy & 0xff;
+        if (len > l)
+            buf[l++] = dy >> 8;
+        if (len > l)
+            buf[l++] = dz;
+        break;
+
+    default:
+        abort();
+    }
 
     return l;
 }
 
-static int usb_keyboard_poll(USBKeyboardState *s, uint8_t *buf, int len)
+static int usb_keyboard_poll(USBHIDState *hs, uint8_t *buf, int len)
 {
+    USBKeyboardState *s = &hs->kbd;
     if (len < 2)
         return 0;
 
@@ -643,12 +680,9 @@ static void usb_mouse_handle_reset(USBDevice *dev)
 {
     USBHIDState *s = (USBHIDState *)dev;
 
-    s->ptr.dx = 0;
-    s->ptr.dy = 0;
-    s->ptr.dz = 0;
-    s->ptr.x = 0;
-    s->ptr.y = 0;
-    s->ptr.buttons_state = 0;
+    memset (s->ptr.queue, 0, sizeof (s->ptr.queue));
+    s->ptr.head = 0;
+    s->ptr.n = 0;
     s->protocol = 1;
 }
 
@@ -708,12 +742,10 @@ static int usb_hid_handle_control(USBDevice *dev, int request, int value,
         }
         break;
     case GET_REPORT:
-	if (s->kind == USB_MOUSE)
-            ret = usb_mouse_poll(s, data, length);
-	else if (s->kind == USB_TABLET)
-            ret = usb_tablet_poll(s, data, length);
+        if (s->kind == USB_MOUSE || s->kind == USB_TABLET)
+            ret = usb_pointer_poll(s, data, length);
         else if (s->kind == USB_KEYBOARD)
-            ret = usb_keyboard_poll(&s->kbd, data, length);
+            ret = usb_keyboard_poll(s, data, length);
         break;
     case SET_REPORT:
         if (s->kind == USB_KEYBOARD)
@@ -762,13 +794,14 @@ static int usb_hid_handle_data(USBDevice *dev, USBPacket *p)
             if (!s->changed && (!s->idle || s->next_idle_clock - curtime > 0))
                 return USB_RET_NAK;
             usb_hid_set_next_idle(s, curtime);
-            s->changed = 0;
-            if (s->kind == USB_MOUSE)
-                ret = usb_mouse_poll(s, p->data, p->len);
-            else if (s->kind == USB_TABLET)
-                ret = usb_tablet_poll(s, p->data, p->len);
-            else if (s->kind == USB_KEYBOARD)
-                ret = usb_keyboard_poll(&s->kbd, p->data, p->len);
+            if (s->kind == USB_MOUSE || s->kind == USB_TABLET) {
+                ret = usb_pointer_poll(s, p->data, p->len);
+                s->changed = s->ptr.n > 0;
+            }
+            else if (s->kind == USB_KEYBOARD) {
+                ret = usb_keyboard_poll(s, p->data, p->len);
+                s->changed = 0;
+            }
         } else {
             goto fail;
         }
@@ -803,13 +836,13 @@ static int usb_hid_initfn(USBDevice *dev, int kind)
     s->kind = kind;
 
     if (s->kind == USB_MOUSE) {
-        s->ptr.eh_entry = qemu_add_mouse_event_handler(usb_mouse_event, s,
+        s->ptr.eh_entry = qemu_add_mouse_event_handler(usb_pointer_event, s,
                                                        0, "QEMU USB Mouse");
     } else if (s->kind == USB_TABLET) {
-        s->ptr.eh_entry = qemu_add_mouse_event_handler(usb_tablet_event, s,
+        s->ptr.eh_entry = qemu_add_mouse_event_handler(usb_pointer_event, s,
                                                        1, "QEMU USB Tablet");
     }
-        
+
     /* Force poll routine to be run and grab input the first time.  */
     s->changed = 1;
     return 0;
-- 
1.7.1

[Qemu-devel] [PATCH 2/8] usb keyboard: add event event queue

[Gerd Hoffmann]

This patch adds a event queue to the usb keyboard.  This makes sure the
guest will see all key events even if they come in bursts.  With this
patch applied sending Ctrl-Alt-Del using vncviewer's F8 menu works.
Also with autosuspend enabled the first keypress on a suspended keyboard
takes a little longer to be delivered to the guest because the usb bus
must be resumed first.  Without event queue this easily gets lost.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb-hid.c |   38 ++++++++++++++++++++++++++++++++------
 1 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/hw/usb-hid.c b/hw/usb-hid.c
index ea49543..5f1451b 100644
--- a/hw/usb-hid.c
+++ b/hw/usb-hid.c
@@ -63,6 +63,9 @@ typedef struct USBMouseState {
 } USBMouseState;
 
 typedef struct USBKeyboardState {
+    uint32_t keycodes[QUEUE_LENGTH];
+    uint32_t head; /* index into circular queue */
+    uint32_t n;
     uint16_t modifiers;
     uint8_t leds;
     uint8_t key[16];
@@ -494,8 +497,27 @@ static void usb_keyboard_event(void *opaque, int keycode)
 {
     USBHIDState *hs = opaque;
     USBKeyboardState *s = &hs->kbd;
+    int slot;
+
+    if (s->n == QUEUE_LENGTH) {
+        fprintf(stderr, "usb-kbd: warning: key event queue full\n");
+        return;
+    }
+    slot = (s->head + s->n) & QUEUE_MASK; s->n++;
+    s->keycodes[slot] = keycode;
+    usb_hid_changed(hs);
+}
+
+static void usb_keyboard_process_keycode(USBKeyboardState *s)
+{
     uint8_t hid_code, key;
-    int i;
+    int i, keycode, slot;
+
+    if (s->n == 0) {
+        return;
+    }
+    slot = s->head & QUEUE_MASK; QUEUE_INCR(s->head); s->n--;
+    keycode = s->keycodes[slot];
 
     key = keycode & 0x7f;
     hid_code = usb_hid_usage_keys[key | ((s->modifiers >> 1) & (1 << 7))];
@@ -528,7 +550,6 @@ static void usb_keyboard_event(void *opaque, int keycode)
             if (s->key[i] == hid_code) {
                 s->key[i] = s->key[-- s->keys];
                 s->key[s->keys] = 0x00;
-                usb_hid_changed(hs);
                 break;
             }
         if (i < 0)
@@ -543,8 +564,6 @@ static void usb_keyboard_event(void *opaque, int keycode)
         } else
             return;
     }
-
-    usb_hid_changed(hs);
 }
 
 static inline int int_clamp(int val, int vmin, int vmax)
@@ -645,6 +664,8 @@ static int usb_keyboard_poll(USBHIDState *hs, uint8_t *buf, int len)
     if (len < 2)
         return 0;
 
+    usb_keyboard_process_keycode(s);
+
     buf[0] = s->modifiers & 0xff;
     buf[1] = 0;
     if (s->keys > 6)
@@ -680,7 +701,7 @@ static void usb_mouse_handle_reset(USBDevice *dev)
 {
     USBHIDState *s = (USBHIDState *)dev;
 
-    memset (s->ptr.queue, 0, sizeof (s->ptr.queue));
+    memset(s->ptr.queue, 0, sizeof (s->ptr.queue));
     s->ptr.head = 0;
     s->ptr.n = 0;
     s->protocol = 1;
@@ -691,6 +712,11 @@ static void usb_keyboard_handle_reset(USBDevice *dev)
     USBHIDState *s = (USBHIDState *)dev;
 
     qemu_add_kbd_event_handler(usb_keyboard_event, s);
+    memset(s->kbd.keycodes, 0, sizeof (s->kbd.keycodes));
+    s->kbd.head = 0;
+    s->kbd.n = 0;
+    memset(s->kbd.key, 0, sizeof (s->kbd.key));
+    s->kbd.keys = 0;
     s->protocol = 1;
 }
 
@@ -800,7 +826,7 @@ static int usb_hid_handle_data(USBDevice *dev, USBPacket *p)
             }
             else if (s->kind == USB_KEYBOARD) {
                 ret = usb_keyboard_poll(s, p->data, p->len);
-                s->changed = 0;
+                s->changed = s->kbd.n > 0;
             }
         } else {
             goto fail;
-- 
1.7.1

[Qemu-devel] [PATCH 3/8] usb hid: move head+n to common struct

[Gerd Hoffmann]

This patch moves the 'head' and 'n' fields from USBMouseState and
USBKeyboardState to the common USBHIDState struct.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb-hid.c |   46 ++++++++++++++++++++++------------------------
 1 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/hw/usb-hid.c b/hw/usb-hid.c
index 5f1451b..168e67d 100644
--- a/hw/usb-hid.c
+++ b/hw/usb-hid.c
@@ -56,16 +56,12 @@ typedef struct USBPointerEvent {
 
 typedef struct USBMouseState {
     USBPointerEvent queue[QUEUE_LENGTH];
-    uint32_t head; /* index into circular queue */
-    uint32_t n;
     int mouse_grabbed;
     QEMUPutMouseEntry *eh_entry;
 } USBMouseState;
 
 typedef struct USBKeyboardState {
     uint32_t keycodes[QUEUE_LENGTH];
-    uint32_t head; /* index into circular queue */
-    uint32_t n;
     uint16_t modifiers;
     uint8_t leds;
     uint8_t key[16];
@@ -78,6 +74,8 @@ typedef struct USBHIDState {
         USBMouseState ptr;
         USBKeyboardState kbd;
     };
+    uint32_t head; /* index into circular queue */
+    uint32_t n;
     int kind;
     int protocol;
     uint8_t idle;
@@ -468,7 +466,7 @@ static void usb_pointer_event(void *opaque,
 {
     USBHIDState *hs = opaque;
     USBMouseState *s = &hs->ptr;
-    unsigned use_slot = (s->head + s->n - 1) & QUEUE_MASK;
+    unsigned use_slot = (hs->head + hs->n - 1) & QUEUE_MASK;
     unsigned previous_slot = (use_slot - 1) & QUEUE_MASK;
 
     /* We combine events where feasible to keep the queue small.  We shouldn't
@@ -476,15 +474,15 @@ static void usb_pointer_event(void *opaque,
      * that would change the location of the button state change.  When the
      * queue is empty, a second event is needed because we don't know if
      * the first event changed the button state.  */
-    if (s->n == QUEUE_LENGTH) {
+    if (hs->n == QUEUE_LENGTH) {
         /* Queue full.  Discard old button state, combine motion normally.  */
         s->queue[use_slot].buttons_state = buttons_state;
-    } else if (s->n < 2 ||
+    } else if (hs->n < 2 ||
                s->queue[use_slot].buttons_state != buttons_state ||
                s->queue[previous_slot].buttons_state != s->queue[use_slot].buttons_state) {
         /* Cannot or should not combine, so add an empty item to the queue.  */
         QUEUE_INCR(use_slot);
-        s->n++;
+        hs->n++;
         usb_pointer_event_clear(&s->queue[use_slot], buttons_state);
     }
     usb_pointer_event_combine(&s->queue[use_slot],
@@ -499,24 +497,25 @@ static void usb_keyboard_event(void *opaque, int keycode)
     USBKeyboardState *s = &hs->kbd;
     int slot;
 
-    if (s->n == QUEUE_LENGTH) {
+    if (hs->n == QUEUE_LENGTH) {
         fprintf(stderr, "usb-kbd: warning: key event queue full\n");
         return;
     }
-    slot = (s->head + s->n) & QUEUE_MASK; s->n++;
+    slot = (hs->head + hs->n) & QUEUE_MASK; hs->n++;
     s->keycodes[slot] = keycode;
     usb_hid_changed(hs);
 }
 
-static void usb_keyboard_process_keycode(USBKeyboardState *s)
+static void usb_keyboard_process_keycode(USBHIDState *hs)
 {
+    USBKeyboardState *s = &hs->kbd;
     uint8_t hid_code, key;
     int i, keycode, slot;
 
-    if (s->n == 0) {
+    if (hs->n == 0) {
         return;
     }
-    slot = s->head & QUEUE_MASK; QUEUE_INCR(s->head); s->n--;
+    slot = hs->head & QUEUE_MASK; QUEUE_INCR(hs->head); hs->n--;
     keycode = s->keycodes[slot];
 
     key = keycode & 0x7f;
@@ -590,7 +589,7 @@ static int usb_pointer_poll(USBHIDState *hs, uint8_t *buf, int len)
 
     /* When the buffer is empty, return the last event.  Relative
        movements will all be zero.  */
-    index = (s->n ? s->head : s->head - 1);
+    index = (hs->n ? hs->head : hs->head - 1);
     e = &s->queue[index & QUEUE_MASK];
 
     if (hs->kind == USB_MOUSE) {
@@ -613,12 +612,12 @@ static int usb_pointer_poll(USBHIDState *hs, uint8_t *buf, int len)
     if (e->buttons_state & MOUSE_EVENT_MBUTTON)
         b |= 0x04;
 
-    if (s->n &&
+    if (hs->n &&
         !e->dz &&
         (hs->kind == USB_TABLET || (!e->xdx && !e->ydy))) {
         /* that deals with this event */
-        QUEUE_INCR(s->head);
-        s->n--;
+        QUEUE_INCR(hs->head);
+        hs->n--;
     }
 
     /* Appears we have to invert the wheel direction */
@@ -664,7 +663,7 @@ static int usb_keyboard_poll(USBHIDState *hs, uint8_t *buf, int len)
     if (len < 2)
         return 0;
 
-    usb_keyboard_process_keycode(s);
+    usb_keyboard_process_keycode(hs);
 
     buf[0] = s->modifiers & 0xff;
     buf[1] = 0;
@@ -702,8 +701,8 @@ static void usb_mouse_handle_reset(USBDevice *dev)
     USBHIDState *s = (USBHIDState *)dev;
 
     memset(s->ptr.queue, 0, sizeof (s->ptr.queue));
-    s->ptr.head = 0;
-    s->ptr.n = 0;
+    s->head = 0;
+    s->n = 0;
     s->protocol = 1;
 }
 
@@ -713,8 +712,8 @@ static void usb_keyboard_handle_reset(USBDevice *dev)
 
     qemu_add_kbd_event_handler(usb_keyboard_event, s);
     memset(s->kbd.keycodes, 0, sizeof (s->kbd.keycodes));
-    s->kbd.head = 0;
-    s->kbd.n = 0;
+    s->head = 0;
+    s->n = 0;
     memset(s->kbd.key, 0, sizeof (s->kbd.key));
     s->kbd.keys = 0;
     s->protocol = 1;
@@ -822,12 +821,11 @@ static int usb_hid_handle_data(USBDevice *dev, USBPacket *p)
             usb_hid_set_next_idle(s, curtime);
             if (s->kind == USB_MOUSE || s->kind == USB_TABLET) {
                 ret = usb_pointer_poll(s, p->data, p->len);
-                s->changed = s->ptr.n > 0;
             }
             else if (s->kind == USB_KEYBOARD) {
                 ret = usb_keyboard_poll(s, p->data, p->len);
-                s->changed = s->kbd.n > 0;
             }
+            s->changed = s->n > 0;
         } else {
             goto fail;
         }
-- 
1.7.1

[Qemu-devel] [PATCH 4/8] vnc: fix numlock+capslock tracking

[Gerd Hoffmann]

This patch makes the numlock+capslock tracking logic only look at
keydown events.  Without this patch the vnc server will insert
bogous capslock keypress in case it sees the following key sequence:

  shift down --- 'A' down --- shift up  --- 'A' up
                                         ^ here

It doesn't hurt with a PS/2 keyboard, but it disturbs the USB Keyboard.
And with the key event queue just added to the usb keyboard the guest
will actually notice.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 495d6d6..0820d99 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1504,7 +1504,7 @@ static void do_key_event(VncState *vs, int down, int keycode, int sym)
         break;
     }
 
-    if (vs->vd->lock_key_sync &&
+    if (down && vs->vd->lock_key_sync &&
         keycode_is_keypad(vs->vd->kbd_layout, keycode)) {
         /* If the numlock state needs to change then simulate an additional
            keypress before sending this one.  This will happen if the user
@@ -1523,7 +1523,7 @@ static void do_key_event(VncState *vs, int down, int keycode, int sym)
         }
     }
 
-    if (vs->vd->lock_key_sync &&
+    if (down && vs->vd->lock_key_sync &&
         ((sym >= 'A' && sym <= 'Z') || (sym >= 'a' && sym <= 'z'))) {
         /* If the capslock state needs to change then simulate an additional
            keypress before sending this one.  This will happen if the user
-- 
1.7.1

[Qemu-devel] [PATCH 5/8] usb core: add migration support

[Gerd Hoffmann]

Yes, seriously.  There is no migration support at all for usb devices.
They loose state, especially the device address, and stop responding
because of that.  Oops.

Luckily there is so much broken usb hardware out there that the guest
usually just kicks the device hard (via port reset and
reinitialization), then continues without a hitch.  So we got away with
that in a surprising high number of cases.

The arrival of remote wakeup (which enables autosuspend support) changes
that picture though.  The usb devices also forget that it they are
supposed to wakeup, so they don't do that.  The host also doesn't notice
the device stopped working in case it suspended the device and thus
expects it waking up instead of polling it.  Result is that your mouse
is dead.

Lets start fixing that.  Add a vmstate struct for USBDevice.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/hw.h      |   10 ++++++++++
 hw/usb-bus.c |   16 ++++++++++++++++
 hw/usb.h     |   10 +++++-----
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index dd993de..5e24329 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -587,6 +587,16 @@ extern const VMStateDescription vmstate_i2c_slave;
     .offset     = vmstate_offset_value(_state, _field, i2c_slave),   \
 }
 
+extern const VMStateDescription vmstate_usb_device;
+
+#define VMSTATE_USB_DEVICE(_field, _state) {                         \
+    .name       = (stringify(_field)),                               \
+    .size       = sizeof(USBDevice),                                 \
+    .vmsd       = &vmstate_usb_device,                               \
+    .flags      = VMS_STRUCT,                                        \
+    .offset     = vmstate_offset_value(_state, _field, USBDevice),   \
+}
+
 #define vmstate_offset_macaddr(_state, _field)                       \
     vmstate_offset_array(_state, _field.a, uint8_t,                \
                          sizeof(typeof_field(_state, _field)))
diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index 6e2e5fd..ac56fbc 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -23,6 +23,22 @@ static struct BusInfo usb_bus_info = {
 static int next_usb_bus = 0;
 static QTAILQ_HEAD(, USBBus) busses = QTAILQ_HEAD_INITIALIZER(busses);
 
+const VMStateDescription vmstate_usb_device = {
+    .name = "USBDevice",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField []) {
+        VMSTATE_UINT8(addr, USBDevice),
+        VMSTATE_INT32(state, USBDevice),
+        VMSTATE_INT32(remote_wakeup, USBDevice),
+        VMSTATE_INT32(setup_state, USBDevice),
+        VMSTATE_INT32(setup_len, USBDevice),
+        VMSTATE_INT32(setup_index, USBDevice),
+        VMSTATE_UINT8_ARRAY(setup_buf, USBDevice, 8),
+        VMSTATE_END_OF_LIST(),
+    }
+};
+
 void usb_bus_new(USBBus *bus, DeviceState *host)
 {
     qbus_create_inplace(&bus->qbus, &usb_bus_info, host, NULL);
diff --git a/hw/usb.h b/hw/usb.h
index 5c1da3e..d3d755d 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -165,13 +165,13 @@ struct USBDevice {
     int auto_attach;
     int attached;
 
-    int state;
+    int32_t state;
     uint8_t setup_buf[8];
     uint8_t data_buf[1024];
-    int remote_wakeup;
-    int setup_state;
-    int setup_len;
-    int setup_index;
+    int32_t remote_wakeup;
+    int32_t setup_state;
+    int32_t setup_len;
+    int32_t setup_index;
 
     QLIST_HEAD(, USBDescString) strings;
     const USBDescDevice *device;
-- 
1.7.1

[Qemu-devel] [PATCH 6/8] usb hub: add migration support

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb-hub.c |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/hw/usb-hub.c b/hw/usb-hub.c
index 78698ca..3dd31ba 100644
--- a/hw/usb-hub.c
+++ b/hw/usb-hub.c
@@ -544,11 +544,35 @@ static int usb_hub_initfn(USBDevice *dev)
     return 0;
 }
 
+static const VMStateDescription vmstate_usb_hub_port = {
+    .name = "usb-hub-port",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField []) {
+        VMSTATE_UINT16(wPortStatus, USBHubPort),
+        VMSTATE_UINT16(wPortChange, USBHubPort),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const VMStateDescription vmstate_usb_hub = {
+    .name = "usb-hub",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField []) {
+        VMSTATE_USB_DEVICE(dev, USBHubState),
+        VMSTATE_STRUCT_ARRAY(ports, USBHubState, NUM_PORTS, 0,
+                             vmstate_usb_hub_port, USBHubPort),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static struct USBDeviceInfo hub_info = {
     .product_desc   = "QEMU USB Hub",
     .qdev.name      = "usb-hub",
     .qdev.fw_name    = "hub",
     .qdev.size      = sizeof(USBHubState),
+    .qdev.vmsd      = &vmstate_usb_hub,
     .usb_desc       = &desc_hub,
     .init           = usb_hub_initfn,
     .handle_packet  = usb_hub_handle_packet,
-- 
1.7.1

[Qemu-devel] [PATCH 7/8] usb hid: add migration support

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb-hid.c |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/hw/usb-hid.c b/hw/usb-hid.c
index 168e67d..79b20df 100644
--- a/hw/usb-hid.c
+++ b/hw/usb-hid.c
@@ -65,7 +65,7 @@ typedef struct USBKeyboardState {
     uint16_t modifiers;
     uint8_t leds;
     uint8_t key[16];
-    int keys;
+    int32_t keys;
 } USBKeyboardState;
 
 typedef struct USBHIDState {
@@ -77,7 +77,7 @@ typedef struct USBHIDState {
     uint32_t head; /* index into circular queue */
     uint32_t n;
     int kind;
-    int protocol;
+    int32_t protocol;
     uint8_t idle;
     int64_t next_idle_clock;
     int changed;
@@ -895,12 +895,72 @@ void usb_hid_datain_cb(USBDevice *dev, void *opaque, void (*datain)(void *))
     s->datain = datain;
 }
 
+static int usb_hid_post_load(void *opaque, int version_id)
+{
+    USBHIDState *s = opaque;
+
+    if (s->idle) {
+        usb_hid_set_next_idle(s, qemu_get_clock(vm_clock));
+    }
+    return 0;
+}
+
+static const VMStateDescription vmstate_usb_ptr_queue = {
+    .name = "usb-ptr-queue",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField []) {
+        VMSTATE_INT32(xdx, USBPointerEvent),
+        VMSTATE_INT32(ydy, USBPointerEvent),
+        VMSTATE_INT32(dz, USBPointerEvent),
+        VMSTATE_INT32(buttons_state, USBPointerEvent),
+        VMSTATE_END_OF_LIST()
+    }
+};
+static const VMStateDescription vmstate_usb_ptr = {
+    .name = "usb-ptr",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .post_load = usb_hid_post_load,
+    .fields = (VMStateField []) {
+        VMSTATE_USB_DEVICE(dev, USBHIDState),
+        VMSTATE_STRUCT_ARRAY(ptr.queue, USBHIDState, QUEUE_LENGTH, 0,
+                             vmstate_usb_ptr_queue, USBPointerEvent),
+        VMSTATE_UINT32(head, USBHIDState),
+        VMSTATE_UINT32(n, USBHIDState),
+        VMSTATE_INT32(protocol, USBHIDState),
+        VMSTATE_UINT8(idle, USBHIDState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const VMStateDescription vmstate_usb_kbd = {
+    .name = "usb-kbd",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .post_load = usb_hid_post_load,
+    .fields = (VMStateField []) {
+        VMSTATE_USB_DEVICE(dev, USBHIDState),
+        VMSTATE_UINT32_ARRAY(kbd.keycodes, USBHIDState, QUEUE_LENGTH),
+        VMSTATE_UINT32(head, USBHIDState),
+        VMSTATE_UINT32(n, USBHIDState),
+        VMSTATE_UINT16(kbd.modifiers, USBHIDState),
+        VMSTATE_UINT8(kbd.leds, USBHIDState),
+        VMSTATE_UINT8_ARRAY(kbd.key, USBHIDState, 16),
+        VMSTATE_INT32(kbd.keys, USBHIDState),
+        VMSTATE_INT32(protocol, USBHIDState),
+        VMSTATE_UINT8(idle, USBHIDState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static struct USBDeviceInfo hid_info[] = {
     {
         .product_desc   = "QEMU USB Tablet",
         .qdev.name      = "usb-tablet",
         .usbdevice_name = "tablet",
         .qdev.size      = sizeof(USBHIDState),
+        .qdev.vmsd      = &vmstate_usb_ptr,
         .usb_desc       = &desc_tablet,
         .init           = usb_tablet_initfn,
         .handle_packet  = usb_generic_handle_packet,
@@ -913,6 +973,7 @@ static struct USBDeviceInfo hid_info[] = {
         .qdev.name      = "usb-mouse",
         .usbdevice_name = "mouse",
         .qdev.size      = sizeof(USBHIDState),
+        .qdev.vmsd      = &vmstate_usb_ptr,
         .usb_desc       = &desc_mouse,
         .init           = usb_mouse_initfn,
         .handle_packet  = usb_generic_handle_packet,
@@ -925,6 +986,7 @@ static struct USBDeviceInfo hid_info[] = {
         .qdev.name      = "usb-kbd",
         .usbdevice_name = "keyboard",
         .qdev.size      = sizeof(USBHIDState),
+        .qdev.vmsd      = &vmstate_usb_kbd,
         .usb_desc       = &desc_keyboard,
         .init           = usb_keyboard_initfn,
         .handle_packet  = usb_generic_handle_packet,
-- 
1.7.1

[Qemu-devel] [PATCH 8/8] usb-bus: use snprintf

[Gerd Hoffmann]

From: Blue Swirl <blauwirbel@gmail.com>

Avoid this warning from OpenBSD linker:
  LINK  i386-softmmu/qemu
../usb-bus.o(.text+0x27c): In function `usb_get_fw_dev_path':
/src/qemu/hw/usb-bus.c:294: warning: sprintf() is often misused,
please use snprintf()

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb-bus.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index ac56fbc..abc7e61 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -298,20 +298,22 @@ static char *usb_get_fw_dev_path(DeviceState *qdev)
 {
     USBDevice *dev = DO_UPCAST(USBDevice, qdev, qdev);
     char *fw_path, *in;
-    int pos = 0;
+    ssize_t pos = 0, fw_len;
     long nr;
 
-    fw_path = qemu_malloc(32 + strlen(dev->port->path) * 6);
+    fw_len = 32 + strlen(dev->port->path) * 6;
+    fw_path = qemu_malloc(fw_len);
     in = dev->port->path;
-    while (true) {
+    while (fw_len - pos > 0) {
         nr = strtol(in, &in, 10);
         if (in[0] == '.') {
             /* some hub between root port and device */
-            pos += sprintf(fw_path + pos, "hub@%ld/", nr);
+            pos += snprintf(fw_path + pos, fw_len - pos, "hub@%ld/", nr);
             in++;
         } else {
             /* the device itself */
-            pos += sprintf(fw_path + pos, "%s@%ld", qdev_fw_name(qdev), nr);
+            pos += snprintf(fw_path + pos, fw_len - pos, "%s@%ld",
+                            qdev_fw_name(qdev), nr);
             break;
         }
     }
-- 
1.7.1

[Qemu-devel] Re: [PATCH 4/8] vnc: fix numlock+capslock tracking

[Paolo Bonzini]

On 01/24/2011 05:30 PM, Gerd Hoffmann wrote:
> This patch makes the numlock+capslock tracking logic only look at
> keydown events.  Without this patch the vnc server will insert
> bogous capslock keypress in case it sees the following key sequence:
>
>    shift down --- 'A' down --- shift up  --- 'A' up
>                                           ^ here
>
> It doesn't hurt with a PS/2 keyboard, but it disturbs the USB Keyboard.
> And with the key event queue just added to the usb keyboard the guest
> will actually notice.
>
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
> ---
>   ui/vnc.c |    4 ++--
>   1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 495d6d6..0820d99 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -1504,7 +1504,7 @@ static void do_key_event(VncState *vs, int down, int keycode, int sym)
>           break;
>       }
>
> -    if (vs->vd->lock_key_sync&&
> +    if (down&&  vs->vd->lock_key_sync&&
>           keycode_is_keypad(vs->vd->kbd_layout, keycode)) {
>           /* If the numlock state needs to change then simulate an additional
>              keypress before sending this one.  This will happen if the user
> @@ -1523,7 +1523,7 @@ static void do_key_event(VncState *vs, int down, int keycode, int sym)
>           }
>       }
>
> -    if (vs->vd->lock_key_sync&&
> +    if (down&&  vs->vd->lock_key_sync&&
>           ((sym>= 'A'&&  sym<= 'Z') || (sym>= 'a'&&  sym<= 'z'))) {
>           /* If the capslock state needs to change then simulate an additional
>              keypress before sending this one.  This will happen if the user

This should be 1/8 or 2/8 in the series.

Also, perhaps these four could go in 0.14?  The USB device are really 
much more usable (especially the keyboard) with them.

Paolo

[Qemu-devel] Re: [PATCH 4/8] vnc: fix numlock+capslock tracking

[Gerd Hoffmann]

   Hi,

> This should be 1/8 or 2/8 in the series.
>
> Also, perhaps these four could go in 0.14? The USB device are really
> much more usable (especially the keyboard) with them.

IMHO the whole series should go to 0.14, otherwise the remote wakeup 
support added recently will simply not play together with migration.

cheers,
   Gerd

Re: [Qemu-devel] Re: [PATCH 4/8] vnc: fix numlock+capslock tracking

[Anthony Liguori]

On 01/28/2011 01:58 PM, Gerd Hoffmann wrote:
>   Hi,
>
>> This should be 1/8 or 2/8 in the series.
>>
>> Also, perhaps these four could go in 0.14? The USB device are really
>> much more usable (especially the keyboard) with them.
>
> IMHO the whole series should go to 0.14, otherwise the remote wakeup 
> support added recently will simply not play together with migration.

Yeah, and for something as core as "add migration" support, I'm inclined 
to agree.  I'm amazed this hasn't been a bigger problem to date.

Regards,

Anthony Liguori

> cheers,
>   Gerd
>

Re: [Qemu-devel] [PULL 0/8] usb patch queue

[Anthony Liguori]

On 01/24/2011 10:30 AM, Gerd Hoffmann wrote:
>    Hi,
>
> Here is the current usb patch queue.  What is in there?
>
> Migration support for USB devices.  For starters hub and HID devices are
> covered.
>
> Event queues for the usb mouse/table (thanks to Paolo and the Xen folks)
> and the usb keyboard, so we can have the queue included in the migration
> state right from the start and avoid the compatibility issues we would
> get when adding this later on.
>
> Also includes is a warning fix from blueswirl.
>    

Pulled.  Thanks.

Regards,

Anthony Liguori

> please pull,
>    Gerd
>
> The following changes since commit 0bfe006c5380c5f8a485a55ded3329fbbc224396:
>
>    multiboot: Fix upper memory size in multiboot info (2011-01-23 22:44:13 +0100)
>
> are available in the git repository at:
>    git://anongit.freedesktop.org/spice/qemu usb.5
>
> Blue Swirl (1):
>        usb-bus: use snprintf
>
> Gerd Hoffmann (6):
>        usb keyboard: add event event queue
>        usb hid: move head+n to common struct
>        vnc: fix numlock+capslock tracking
>        usb core: add migration support
>        usb hub: add migration support
>        usb hid: add migration support
>
> Paolo Bonzini (1):
>        add event queueing to USB HID
>
>   hw/hw.h      |   10 ++
>   hw/usb-bus.c |   28 ++++-
>   hw/usb-hid.c |  317 ++++++++++++++++++++++++++++++++++++++++------------------
>   hw/usb-hub.c |   24 +++++
>   hw/usb.h     |   10 +-
>   ui/vnc.c     |    4 +-
>   6 files changed, 282 insertions(+), 111 deletions(-)
>
>
>