[Qemu-devel] [PATCH 0/7] usb: add migration support.

[Qemu-devel] [PATCH 0/7] usb: add migration support.

[Gerd Hoffmann]

  Hi,

This patch series kickstarts migration support for USB devices.
For starters we'll cover the hub and the HID devices.

The patch series also adds 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.

The patches apply cleanly on top of the "usb descriptor overhaul + more"
patch series v4 sent out a few days ago.

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 |   16 +++
 hw/usb-hid.c |  317 ++++++++++++++++++++++++++++++++++++++++------------------
 hw/usb-hub.c |   24 +++++
 hw/usb.h     |   10 +-
 ui/vnc.c     |    4 +-
 6 files changed, 275 insertions(+), 106 deletions(-)

[Qemu-devel] [PATCH 1/7] 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>
---
 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 1fec163..8c5ed39 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);
 }
 
@@ -525,83 +554,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;
 
@@ -640,12 +677,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;
 }
 
@@ -705,12 +739,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)
@@ -759,13 +791,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;
         }
@@ -800,13 +833,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/7] 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 8c5ed39..9444051 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))];
@@ -525,7 +547,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)
@@ -540,8 +561,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)
@@ -642,6 +661,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)
@@ -677,7 +698,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;
@@ -688,6 +709,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;
 }
 
@@ -797,7 +823,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/7] 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 9444051..8148d66 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;
@@ -587,7 +586,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) {
@@ -610,12 +609,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 */
@@ -661,7 +660,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;
@@ -699,8 +698,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;
 }
 
@@ -710,8 +709,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;
@@ -819,12 +818,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/7] 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/7] 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 163a683..f341220 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/7] 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/7] 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 8148d66..32ee57c 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;
@@ -892,12 +892,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,
@@ -910,6 +970,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,
@@ -922,6 +983,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] Re: [PATCH 2/7] usb keyboard: add event event queue

[Paolo Bonzini]

On 01/14/2011 12:55 PM, Gerd Hoffmann wrote:
> 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(-)

Nice elegant patch.

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

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

[Paolo Bonzini]

On 01/14/2011 12:55 PM, Gerd Hoffmann wrote:
> 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 9444051..8148d66 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;
> @@ -587,7 +586,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) {
> @@ -610,12 +609,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 */
> @@ -661,7 +660,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;
> @@ -699,8 +698,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;
>   }
>
> @@ -710,8 +709,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;
> @@ -819,12 +818,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;
>           }

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo