Patchwork [1/2] Support for multiple keyboard devices

login
register
mail settings
Submitter Shahar Havivi
Date March 23, 2010, 7:58 p.m.
Message ID <d3d126b55f6763c8d7d1e0fffad1390c52b35263.1269373411.git.shaharh@redhat.com>
Download mbox | patch
Permalink /patch/48362/
State New
Headers show

Comments

Shahar Havivi - March 23, 2010, 7:58 p.m.
Currently you get segfault when trying to remove keyboard (device_del
monitor command) because no keyboard handling is done.

This patch add QEMUPutKbdEntry structure, handling each keyboard entry.
Adding a keyboard add to the list, removing keyboard select the previous
keyboard in list.

Signed-off-by: Shahar Havivi <shaharh@redhat.com>
---
 console.h            |   12 ++++++-
 hw/adb.c             |    2 +-
 hw/escc.c            |    3 +-
 hw/musicpal.c        |    2 +-
 hw/nseries.c         |    4 +-
 hw/palm.c            |    2 +-
 hw/ps2.c             |    2 +-
 hw/pxa2xx_keypad.c   |    2 +-
 hw/spitz.c           |    2 +-
 hw/stellaris_input.c |    2 +-
 hw/syborg_keyboard.c |    2 +-
 hw/usb-hid.c         |   10 ++++--
 hw/xenfb.c           |    4 +-
 input.c              |   90 ++++++++++++++++++++++++++++++++++++++++++++-----
 14 files changed, 112 insertions(+), 27 deletions(-)
Markus Armbruster - March 26, 2010, 9:46 a.m.
Shahar Havivi <shaharh@redhat.com> writes:

> Currently you get segfault when trying to remove keyboard (device_del
> monitor command) because no keyboard handling is done.
>
> This patch add QEMUPutKbdEntry structure, handling each keyboard entry.
> Adding a keyboard add to the list, removing keyboard select the previous
> keyboard in list.
>
> Signed-off-by: Shahar Havivi <shaharh@redhat.com>
> ---
>  console.h            |   12 ++++++-
>  hw/adb.c             |    2 +-
>  hw/escc.c            |    3 +-
>  hw/musicpal.c        |    2 +-
>  hw/nseries.c         |    4 +-
>  hw/palm.c            |    2 +-
>  hw/ps2.c             |    2 +-
>  hw/pxa2xx_keypad.c   |    2 +-
>  hw/spitz.c           |    2 +-
>  hw/stellaris_input.c |    2 +-
>  hw/syborg_keyboard.c |    2 +-
>  hw/usb-hid.c         |   10 ++++--
>  hw/xenfb.c           |    4 +-
>  input.c              |   90 ++++++++++++++++++++++++++++++++++++++++++++-----
>  14 files changed, 112 insertions(+), 27 deletions(-)
>
> diff --git a/console.h b/console.h
> index 6def115..16c9c3d 100644
> --- a/console.h
> +++ b/console.h
> @@ -41,7 +41,17 @@ typedef struct QEMUPutLEDEntry {
>      QTAILQ_ENTRY(QEMUPutLEDEntry) next;
>  } QEMUPutLEDEntry;
>  
> -void qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque);
> +typedef struct QEMUPutKbdEntry {
> +    char *qemu_put_kbd_name;
> +    QEMUPutKBDEvent *qemu_put_kbd_event;
> +    void *qemu_put_kbd_event_opaque;
> +    struct QEMUPutKbdEntry *next;
> +} QEMUPutKbdEntry;
> +
> +QEMUPutKbdEntry *qemu_add_kbd_event_handler(QEMUPutKBDEvent *func,
> +                                            void *opaque,
> +                                            const char *name);
> +void qemu_remove_kbd_event_handler(QEMUPutKbdEntry *entry);
>  QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func,
>                                                  void *opaque, int absolute,
>                                                  const char *name);
[...]
> diff --git a/input.c b/input.c
> index 8f0941e..563ecad 100644
> --- a/input.c
> +++ b/input.c
> @@ -28,20 +28,14 @@
>  #include "console.h"
>  #include "qjson.h"
>  
> -static QEMUPutKBDEvent *qemu_put_kbd_event;
> -static void *qemu_put_kbd_event_opaque;
> +static QEMUPutKbdEntry *qemu_put_kbd_event_head;
> +static QEMUPutKbdEntry *qemu_put_kbd_event_current;
>  static QTAILQ_HEAD(, QEMUPutLEDEntry) led_handlers = QTAILQ_HEAD_INITIALIZER(led_handlers);
>  static QTAILQ_HEAD(, QEMUPutMouseEntry) mouse_handlers =
>      QTAILQ_HEAD_INITIALIZER(mouse_handlers);
>  static NotifierList mouse_mode_notifiers = 
>      NOTIFIER_LIST_INITIALIZER(mouse_mode_notifiers);
>  
> -void qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque)
> -{
> -    qemu_put_kbd_event_opaque = opaque;
> -    qemu_put_kbd_event = func;
> -}
> -
>  static void check_mode_change(void)
>  {
>      static int current_is_absolute, current_has_absolute;
> @@ -60,6 +54,81 @@ static void check_mode_change(void)
>      current_has_absolute = has_absolute;
>  }
>  
> +QEMUPutKbdEntry *qemu_add_kbd_event_handler(QEMUPutKBDEvent *func,
> +                                            void *opaque,
> +                                            const char *name)
> +{
> +    QEMUPutKbdEntry *s, *cursor;
> +
> +    cursor = qemu_put_kbd_event_head;
> +    while (cursor) {
> +        if (cursor->qemu_put_kbd_event == func &&
> +            cursor->qemu_put_kbd_event_opaque == opaque) {
> +
> +            qemu_put_kbd_event_current = cursor;
> +            return cursor;
> +        }
> +        cursor = cursor->next;
> +    }

You're chasing list pointers.  Why not use a suitable list type from
qemu-queue.h?

> +
> +    s = qemu_mallocz(sizeof(QEMUPutKbdEntry));
> +
> +    s->qemu_put_kbd_event_opaque = opaque;
> +    s->qemu_put_kbd_event = func;
> +    s->qemu_put_kbd_name = qemu_strdup(name);
> +    s->next = NULL;
> +
> +    if (!qemu_put_kbd_event_head) {
> +        qemu_put_kbd_event_head = s;
> +        qemu_put_kbd_event_current = s;
> +        return s;
> +    }
> +
> +    cursor = qemu_put_kbd_event_head;
> +    while (cursor->next) {
> +        cursor = cursor->next;
> +    }
> +
> +    cursor->next = s;
> +    qemu_put_kbd_event_current = s;
> +
> +    return s;
> +}
> +
> +void qemu_remove_kbd_event_handler(QEMUPutKbdEntry *entry)
> +{
> +    QEMUPutKbdEntry *prev = NULL, *cursor;
> +
> +    if (!qemu_put_kbd_event_head || !entry) {
> +        return;
> +    }
> +
> +    cursor = qemu_put_kbd_event_head;
> +    while (cursor && cursor != entry) {
> +        prev = cursor;
> +        cursor = cursor->next;
> +    }
> +
> +    if (cursor == NULL) {
> +        return;
> +    } else if (prev == NULL) {
> +        qemu_put_kbd_event_head = cursor->next;
> +        if (qemu_put_kbd_event_current == entry) {
> +            qemu_put_kbd_event_current = cursor->next;
> +        }
> +        qemu_free(entry);
> +        return;
> +    }
> +
> +    prev->next = entry->next;
> +
> +    if (qemu_put_kbd_event_current == entry) {
> +        qemu_put_kbd_event_current = prev;
> +    }
> +
> +    qemu_free(entry);

More list pointer chasing.

> +}
> +
>  QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func,
>                                                  void *opaque, int absolute,
>                                                  const char *name)
> @@ -123,8 +192,9 @@ void qemu_remove_led_event_handler(QEMUPutLEDEntry *entry)
>  
>  void kbd_put_keycode(int keycode)
>  {
> -    if (qemu_put_kbd_event) {
> -        qemu_put_kbd_event(qemu_put_kbd_event_opaque, keycode);
> +    if (qemu_put_kbd_event_current) {
> +        qemu_put_kbd_event_current->qemu_put_kbd_event(
> +            qemu_put_kbd_event_current->qemu_put_kbd_event_opaque, keycode);
>      }
>  }

Patch

diff --git a/console.h b/console.h
index 6def115..16c9c3d 100644
--- a/console.h
+++ b/console.h
@@ -41,7 +41,17 @@  typedef struct QEMUPutLEDEntry {
     QTAILQ_ENTRY(QEMUPutLEDEntry) next;
 } QEMUPutLEDEntry;
 
-void qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque);
+typedef struct QEMUPutKbdEntry {
+    char *qemu_put_kbd_name;
+    QEMUPutKBDEvent *qemu_put_kbd_event;
+    void *qemu_put_kbd_event_opaque;
+    struct QEMUPutKbdEntry *next;
+} QEMUPutKbdEntry;
+
+QEMUPutKbdEntry *qemu_add_kbd_event_handler(QEMUPutKBDEvent *func,
+                                            void *opaque,
+                                            const char *name);
+void qemu_remove_kbd_event_handler(QEMUPutKbdEntry *entry);
 QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func,
                                                 void *opaque, int absolute,
                                                 const char *name);
diff --git a/hw/adb.c b/hw/adb.c
index 4fb7a62..09afcf9 100644
--- a/hw/adb.c
+++ b/hw/adb.c
@@ -304,7 +304,7 @@  void adb_kbd_init(ADBBusState *bus)
     s = qemu_mallocz(sizeof(KBDState));
     d = adb_register_device(bus, ADB_KEYBOARD, adb_kbd_request,
                             adb_kbd_reset, s);
-    qemu_add_kbd_event_handler(adb_kbd_put_keycode, d);
+    qemu_add_kbd_event_handler(adb_kbd_put_keycode, d, "adb");
     register_savevm("adb_kbd", -1, 1, adb_kbd_save,
                     adb_kbd_load, s);
 }
diff --git a/hw/escc.c b/hw/escc.c
index 6d2fd36..2b21d98 100644
--- a/hw/escc.c
+++ b/hw/escc.c
@@ -919,7 +919,8 @@  static int escc_init1(SysBusDevice *dev)
                                      "QEMU Sun Mouse");
     }
     if (s->chn[1].type == kbd) {
-        qemu_add_kbd_event_handler(sunkbd_event, &s->chn[1]);
+        qemu_add_kbd_event_handler(sunkbd_event, &s->chn[1],
+                                   "QEMU Sun Keyboard");
     }
 
     return 0;
diff --git a/hw/musicpal.c b/hw/musicpal.c
index 7fc9fb3..aca8a88 100644
--- a/hw/musicpal.c
+++ b/hw/musicpal.c
@@ -1447,7 +1447,7 @@  static int musicpal_key_init(SysBusDevice *dev)
 
     qdev_init_gpio_out(&dev->qdev, s->out, ARRAY_SIZE(s->out));
 
-    qemu_add_kbd_event_handler(musicpal_key_event, s);
+    qemu_add_kbd_event_handler(musicpal_key_event, s, "Musicpal");
 
     return 0;
 }
diff --git a/hw/nseries.c b/hw/nseries.c
index 0273eee..abfcec3 100644
--- a/hw/nseries.c
+++ b/hw/nseries.c
@@ -262,7 +262,7 @@  static void n800_tsc_kbd_setup(struct n800_s *s)
         if (n800_keys[i] >= 0)
             s->keymap[n800_keys[i]] = i;
 
-    qemu_add_kbd_event_handler(n800_key_event, s);
+    qemu_add_kbd_event_handler(n800_key_event, s, "Nokia n800");
 
     tsc210x_set_transform(s->ts.chip, &n800_pointercal);
 }
@@ -371,7 +371,7 @@  static void n810_kbd_setup(struct n800_s *s)
         if (n810_keys[i] > 0)
             s->keymap[n810_keys[i]] = i;
 
-    qemu_add_kbd_event_handler(n810_key_event, s);
+    qemu_add_kbd_event_handler(n810_key_event, s, "Nokia n810");
 
     /* Attach the LM8322 keyboard to the I2C bus,
      * should happen in n8x0_i2c_setup and s->kbd be initialised here.  */
diff --git a/hw/palm.c b/hw/palm.c
index 6d19167..1b405d4 100644
--- a/hw/palm.c
+++ b/hw/palm.c
@@ -228,7 +228,7 @@  static void palmte_init(ram_addr_t ram_size,
 
     palmte_microwire_setup(cpu);
 
-    qemu_add_kbd_event_handler(palmte_button_event, cpu);
+    qemu_add_kbd_event_handler(palmte_button_event, cpu, "Palm Keyboard");
 
     palmte_gpio_setup(cpu);
 
diff --git a/hw/ps2.c b/hw/ps2.c
index f0b206a..886da37 100644
--- a/hw/ps2.c
+++ b/hw/ps2.c
@@ -596,7 +596,7 @@  void *ps2_kbd_init(void (*update_irq)(void *, int), void *update_arg)
     s->common.update_arg = update_arg;
     s->scancode_set = 2;
     vmstate_register(0, &vmstate_ps2_keyboard, s);
-    qemu_add_kbd_event_handler(ps2_put_keycode, s);
+    qemu_add_kbd_event_handler(ps2_put_keycode, s, "QEMU PS/2 Keyboard");
     qemu_register_reset(ps2_kbd_reset, s);
     return s;
 }
diff --git a/hw/pxa2xx_keypad.c b/hw/pxa2xx_keypad.c
index 060df58..7fa5af9 100644
--- a/hw/pxa2xx_keypad.c
+++ b/hw/pxa2xx_keypad.c
@@ -332,5 +332,5 @@  void pxa27x_register_keypad(PXA2xxKeyPadState *kp, struct keymap *map,
     }
 
     kp->map = map;
-    qemu_add_kbd_event_handler((QEMUPutKBDEvent *) pxa27x_keyboard_event, kp);
+    qemu_add_kbd_event_handler((QEMUPutKBDEvent *) pxa27x_keyboard_event, kp, "PXA keypad");
 }
diff --git a/hw/spitz.c b/hw/spitz.c
index 564519b..64e8a6e 100644
--- a/hw/spitz.c
+++ b/hw/spitz.c
@@ -506,7 +506,7 @@  static void spitz_keyboard_register(PXA2xxState *cpu)
         pxa2xx_gpio_out_set(cpu->gpio, spitz_gpio_key_strobe[i], s->strobe[i]);
 
     spitz_keyboard_pre_map(s);
-    qemu_add_kbd_event_handler((QEMUPutKBDEvent *) spitz_keyboard_handler, s);
+    qemu_add_kbd_event_handler((QEMUPutKBDEvent *) spitz_keyboard_handler, s, "Spitz keyboard");
 
     register_savevm("spitz_keyboard", 0, 0,
                     spitz_keyboard_save, spitz_keyboard_load, s);
diff --git a/hw/stellaris_input.c b/hw/stellaris_input.c
index 33395a4..775cb46 100644
--- a/hw/stellaris_input.c
+++ b/hw/stellaris_input.c
@@ -85,7 +85,7 @@  void stellaris_gamepad_init(int n, qemu_irq *irq, const int *keycode)
         s->buttons[i].keycode = keycode[i];
     }
     s->num_buttons = n;
-    qemu_add_kbd_event_handler(stellaris_gamepad_put_key, s);
+    qemu_add_kbd_event_handler(stellaris_gamepad_put_key, s, "Stellaris Gamepad");
     register_savevm("stellaris_gamepad", -1, 1,
                     stellaris_gamepad_save, stellaris_gamepad_load, s);
 }
diff --git a/hw/syborg_keyboard.c b/hw/syborg_keyboard.c
index 4a562f8..d1651b4 100644
--- a/hw/syborg_keyboard.c
+++ b/hw/syborg_keyboard.c
@@ -218,7 +218,7 @@  static int syborg_keyboard_init(SysBusDevice *dev)
     }
     s->key_fifo = qemu_mallocz(s->fifo_size * sizeof(s->key_fifo[0]));
 
-    qemu_add_kbd_event_handler(syborg_keyboard_event, s);
+    qemu_add_kbd_event_handler(syborg_keyboard_event, s, "Syborg Keyboard");
 
     register_savevm("syborg_keyboard", -1, 1,
                     syborg_keyboard_save, syborg_keyboard_load, s);
diff --git a/hw/usb-hid.c b/hw/usb-hid.c
index 8e6c6e0..dbab5d3 100644
--- a/hw/usb-hid.c
+++ b/hw/usb-hid.c
@@ -55,6 +55,7 @@  typedef struct USBKeyboardState {
     uint8_t leds;
     uint8_t key[16];
     int keys;
+    QEMUPutKbdEntry *eh_entry;
 } USBKeyboardState;
 
 typedef struct USBHIDState {
@@ -633,7 +634,8 @@  static void usb_keyboard_handle_reset(USBDevice *dev)
 {
     USBHIDState *s = (USBHIDState *)dev;
 
-    qemu_add_kbd_event_handler(usb_keyboard_event, s);
+    s->kbd.eh_entry = qemu_add_kbd_event_handler(usb_keyboard_event, s,
+                                                 dev->product_desc);
     s->protocol = 1;
 }
 
@@ -854,9 +856,11 @@  static void usb_hid_handle_destroy(USBDevice *dev)
 {
     USBHIDState *s = (USBHIDState *)dev;
 
-    if (s->kind != USB_KEYBOARD)
+    if (s->kind != USB_KEYBOARD) {
         qemu_remove_mouse_event_handler(s->ptr.eh_entry);
-    /* TODO: else */
+    } else {
+        qemu_remove_kbd_event_handler(s->kbd.eh_entry);
+    }
 }
 
 static int usb_hid_initfn(USBDevice *dev, int kind)
diff --git a/hw/xenfb.c b/hw/xenfb.c
index 422cd53..2c700bd 100644
--- a/hw/xenfb.c
+++ b/hw/xenfb.c
@@ -373,7 +373,7 @@  static int input_connect(struct XenDevice *xendev)
     if (rc != 0)
 	return rc;
 
-    qemu_add_kbd_event_handler(xenfb_key_event, in);
+    qemu_add_kbd_event_handler(xenfb_key_event, in, "Xen Keyboard");
     in->qmouse = qemu_add_mouse_event_handler(xenfb_mouse_event, in,
 					      in->abs_pointer_wanted,
 					      "Xen PVFB Mouse");
@@ -388,7 +388,7 @@  static void input_disconnect(struct XenDevice *xendev)
 	qemu_remove_mouse_event_handler(in->qmouse);
 	in->qmouse = NULL;
     }
-    qemu_add_kbd_event_handler(NULL, NULL);
+    qemu_add_kbd_event_handler(NULL, NULL, NULL);
     common_unbind(&in->c);
 }
 
diff --git a/input.c b/input.c
index 8f0941e..563ecad 100644
--- a/input.c
+++ b/input.c
@@ -28,20 +28,14 @@ 
 #include "console.h"
 #include "qjson.h"
 
-static QEMUPutKBDEvent *qemu_put_kbd_event;
-static void *qemu_put_kbd_event_opaque;
+static QEMUPutKbdEntry *qemu_put_kbd_event_head;
+static QEMUPutKbdEntry *qemu_put_kbd_event_current;
 static QTAILQ_HEAD(, QEMUPutLEDEntry) led_handlers = QTAILQ_HEAD_INITIALIZER(led_handlers);
 static QTAILQ_HEAD(, QEMUPutMouseEntry) mouse_handlers =
     QTAILQ_HEAD_INITIALIZER(mouse_handlers);
 static NotifierList mouse_mode_notifiers = 
     NOTIFIER_LIST_INITIALIZER(mouse_mode_notifiers);
 
-void qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque)
-{
-    qemu_put_kbd_event_opaque = opaque;
-    qemu_put_kbd_event = func;
-}
-
 static void check_mode_change(void)
 {
     static int current_is_absolute, current_has_absolute;
@@ -60,6 +54,81 @@  static void check_mode_change(void)
     current_has_absolute = has_absolute;
 }
 
+QEMUPutKbdEntry *qemu_add_kbd_event_handler(QEMUPutKBDEvent *func,
+                                            void *opaque,
+                                            const char *name)
+{
+    QEMUPutKbdEntry *s, *cursor;
+
+    cursor = qemu_put_kbd_event_head;
+    while (cursor) {
+        if (cursor->qemu_put_kbd_event == func &&
+            cursor->qemu_put_kbd_event_opaque == opaque) {
+
+            qemu_put_kbd_event_current = cursor;
+            return cursor;
+        }
+        cursor = cursor->next;
+    }
+
+    s = qemu_mallocz(sizeof(QEMUPutKbdEntry));
+
+    s->qemu_put_kbd_event_opaque = opaque;
+    s->qemu_put_kbd_event = func;
+    s->qemu_put_kbd_name = qemu_strdup(name);
+    s->next = NULL;
+
+    if (!qemu_put_kbd_event_head) {
+        qemu_put_kbd_event_head = s;
+        qemu_put_kbd_event_current = s;
+        return s;
+    }
+
+    cursor = qemu_put_kbd_event_head;
+    while (cursor->next) {
+        cursor = cursor->next;
+    }
+
+    cursor->next = s;
+    qemu_put_kbd_event_current = s;
+
+    return s;
+}
+
+void qemu_remove_kbd_event_handler(QEMUPutKbdEntry *entry)
+{
+    QEMUPutKbdEntry *prev = NULL, *cursor;
+
+    if (!qemu_put_kbd_event_head || !entry) {
+        return;
+    }
+
+    cursor = qemu_put_kbd_event_head;
+    while (cursor && cursor != entry) {
+        prev = cursor;
+        cursor = cursor->next;
+    }
+
+    if (cursor == NULL) {
+        return;
+    } else if (prev == NULL) {
+        qemu_put_kbd_event_head = cursor->next;
+        if (qemu_put_kbd_event_current == entry) {
+            qemu_put_kbd_event_current = cursor->next;
+        }
+        qemu_free(entry);
+        return;
+    }
+
+    prev->next = entry->next;
+
+    if (qemu_put_kbd_event_current == entry) {
+        qemu_put_kbd_event_current = prev;
+    }
+
+    qemu_free(entry);
+}
+
 QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func,
                                                 void *opaque, int absolute,
                                                 const char *name)
@@ -123,8 +192,9 @@  void qemu_remove_led_event_handler(QEMUPutLEDEntry *entry)
 
 void kbd_put_keycode(int keycode)
 {
-    if (qemu_put_kbd_event) {
-        qemu_put_kbd_event(qemu_put_kbd_event_opaque, keycode);
+    if (qemu_put_kbd_event_current) {
+        qemu_put_kbd_event_current->qemu_put_kbd_event(
+            qemu_put_kbd_event_current->qemu_put_kbd_event_opaque, keycode);
     }
 }