[Qemu-devel] [PULL 0/7] Ui and input 20170623 patches

[Qemu-devel] [PULL 0/7] Ui and input 20170623 patches

[Gerd Hoffmann]

The following changes since commit e18a639164e162b3a4f078ab60606cd2f163d934:

  Merge remote-tracking branch 'remotes/kraxel/tags/usb-20170621-pull-request' into staging (2017-06-22 15:09:48 +0100)

are available in the git repository at:

  git://git.kraxel.org/qemu tags/ui-and-input-20170623-pull-request

for you to fetch changes up to 6e24ee0c1e4b6c0c9c748acab77ecd113c942a4d:

  ps2: reset queue in ps2_reset_keyboard (2017-06-23 11:51:50 +0200)

----------------------------------------------------------------

----------------------------------------------------------------

Alexander Graf (2):
  input: Decrement queue count on kbd delay
  hid: Reset kbd modifiers on reset

Gerd Hoffmann (5):
  keymaps: add tracing
  sdl2: add assert to make coverity happy
  ps2: add and use PS2State typedef
  ps2: add ps2_reset_queue
  ps2: reset queue in ps2_reset_keyboard

 include/hw/input/ps2.h  |  4 ++--
 include/qemu/typedefs.h |  1 +
 hw/input/hid.c          |  1 +
 hw/input/ps2.c          | 28 ++++++++++++++++------------
 ui/input.c              |  1 +
 ui/keymaps.c            | 33 ++++++++++++++++++---------------
 ui/sdl2.c               |  1 +
 ui/trace-events         |  5 +++++
 8 files changed, 45 insertions(+), 29 deletions(-)

-- 
2.9.3

[Qemu-devel] [PULL 1/7] keymaps: add tracing

[Gerd Hoffmann]

Drop commented debug logging, add trace points instead.

Also cleanup parser code a bit, the key name is copied into a new
variable instead of patching the input line, that way we can log
the unmodified line.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20170606134736.26080-1-kraxel@redhat.com
---
 ui/keymaps.c    | 33 ++++++++++++++++++---------------
 ui/trace-events |  5 +++++
 2 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/ui/keymaps.c b/ui/keymaps.c
index 8899a0b31e..fa00b82027 100644
--- a/ui/keymaps.c
+++ b/ui/keymaps.c
@@ -25,6 +25,7 @@
 #include "qemu/osdep.h"
 #include "keymaps.h"
 #include "sysemu/sysemu.h"
+#include "trace.h"
 
 static int get_keysym(const name2keysym_t *table,
                       const char *name)
@@ -71,18 +72,14 @@ static void add_to_key_range(struct key_range **krp, int code) {
 
 static void add_keysym(char *line, int keysym, int keycode, kbd_layout_t *k) {
     if (keysym < MAX_NORMAL_KEYCODE) {
-        /* fprintf(stderr,"Setting keysym %s (%d) to %d\n",
-                   line, keysym, keycode); */
+        trace_keymap_add("normal", keysym, keycode, line);
         k->keysym2keycode[keysym] = keycode;
     } else {
         if (k->extra_count >= MAX_EXTRA_COUNT) {
             fprintf(stderr, "Warning: Could not assign keysym %s (0x%x)"
                     " because of memory constraints.\n", line, keysym);
         } else {
-#if 0
-            fprintf(stderr, "Setting %d: %d,%d\n",
-                    k->extra_count, keysym, keycode);
-#endif
+            trace_keymap_add("extra", keysym, keycode, line);
             k->keysym2keycode_extra[k->extra_count].
             keysym = keysym;
             k->keysym2keycode_extra[k->extra_count].
@@ -99,9 +96,11 @@ static kbd_layout_t *parse_keyboard_layout(const name2keysym_t *table,
     FILE *f;
     char * filename;
     char line[1024];
+    char keyname[64];
     int len;
 
     filename = qemu_find_file(QEMU_FILE_TYPE_KEYMAP, language);
+    trace_keymap_parse(filename);
     f = filename ? fopen(filename, "r") : NULL;
     g_free(filename);
     if (!f) {
@@ -130,18 +129,21 @@ static kbd_layout_t *parse_keyboard_layout(const name2keysym_t *table,
         if (!strncmp(line, "include ", 8)) {
             parse_keyboard_layout(table, line + 8, k);
         } else {
-            char *end_of_keysym = line;
-            while (*end_of_keysym != 0 && *end_of_keysym != ' ') {
-                end_of_keysym++;
+            int offset = 0;
+            while (line[offset] != 0 &&
+                   line[offset] != ' ' &&
+                   offset < sizeof(keyname) - 1) {
+                keyname[offset] = line[offset];
+                offset++;
             }
-            if (*end_of_keysym) {
+            keyname[offset] = 0;
+            if (strlen(keyname)) {
                 int keysym;
-                *end_of_keysym = 0;
-                keysym = get_keysym(table, line);
+                keysym = get_keysym(table, keyname);
                 if (keysym == 0) {
                     /* fprintf(stderr, "Warning: unknown keysym %s\n", line);*/
                 } else {
-                    const char *rest = end_of_keysym + 1;
+                    const char *rest = line + offset + 1;
                     int keycode = strtol(rest, NULL, 0);
 
                     if (strstr(rest, "numlock")) {
@@ -165,10 +167,10 @@ static kbd_layout_t *parse_keyboard_layout(const name2keysym_t *table,
 
                     if (strstr(rest, "addupper")) {
                         char *c;
-                        for (c = line; *c; c++) {
+                        for (c = keyname; *c; c++) {
                             *c = qemu_toupper(*c);
                         }
-                        keysym = get_keysym(table, line);
+                        keysym = get_keysym(table, keyname);
                         if (keysym) {
                             add_keysym(line, keysym,
                                        keycode | SCANCODE_SHIFT, k);
@@ -194,6 +196,7 @@ int keysym2scancode(void *kbd_layout, int keysym)
     kbd_layout_t *k = kbd_layout;
     if (keysym < MAX_NORMAL_KEYCODE) {
         if (k->keysym2keycode[keysym] == 0) {
+            trace_keymap_unmapped(keysym);
             fprintf(stderr, "Warning: no scancode found for keysym %d\n",
                     keysym);
         }
diff --git a/ui/trace-events b/ui/trace-events
index 93fe5482e6..19ce5f85f6 100644
--- a/ui/trace-events
+++ b/ui/trace-events
@@ -46,3 +46,8 @@ qemu_spice_create_primary_surface(int qid, uint32_t sid, void *surface, int asyn
 qemu_spice_destroy_primary_surface(int qid, uint32_t sid, int async) "%d sid=%u async=%d"
 qemu_spice_wakeup(uint32_t qid) "%d"
 qemu_spice_create_update(uint32_t left, uint32_t right, uint32_t top, uint32_t bottom) "lr %d -> %d,  tb -> %d -> %d"
+
+# ui/keymaps.c
+keymap_parse(const char *file) "file %s"
+keymap_add(const char *type, int sym, int code, const char *line) "%-6s sym=0x%04x code=0x%04x (line: %s)"
+keymap_unmapped(int sym) "sym=0x%04x"
-- 
2.9.3

[Qemu-devel] [PULL 2/7] input: Decrement queue count on kbd delay

[Gerd Hoffmann]

From: Alexander Graf <agraf@suse.de>

Delays in the input layer are special cased input events. Every input
event is accounted for in a global intput queue count. The special cased
delays however did not get removed from the queue, leading to queue overruns
and thus silent key drops after typing quite a few characters.

Signed-off-by: Alexander Graf <agraf@suse.de>
Message-id: 1498117318-162102-1-git-send-email-agraf@suse.de
Fixes: be1a7176 ("input: add support for kbd delays")
Cc: qemu-stable@nongnu.org
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/input.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/input.c b/ui/input.c
index 2abd46de93..af05f06368 100644
--- a/ui/input.c
+++ b/ui/input.c
@@ -256,6 +256,7 @@ static void qemu_input_queue_process(void *opaque)
     item = QTAILQ_FIRST(queue);
     g_assert(item->type == QEMU_INPUT_QUEUE_DELAY);
     QTAILQ_REMOVE(queue, item, node);
+    queue_count--;
     g_free(item);
 
     while (!QTAILQ_EMPTY(queue)) {
-- 
2.9.3

[Qemu-devel] [PULL 3/7] hid: Reset kbd modifiers on reset

[Gerd Hoffmann]

From: Alexander Graf <agraf@suse.de>

When resetting the keyboard, we need to reset not just the pending keystrokes,
but also any pending modifiers. Otherwise there's a race when we're getting
reset while running an escape sequence (modifier 0x100).

Cc: qemu-stable@nongnu.org
Signed-off-by: Alexander Graf <agraf@suse.de>
Message-id: 1498117295-162030-1-git-send-email-agraf@suse.de
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/input/hid.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/input/hid.c b/hw/input/hid.c
index 93887ecc43..0d049ff61c 100644
--- a/hw/input/hid.c
+++ b/hw/input/hid.c
@@ -487,6 +487,7 @@ void hid_reset(HIDState *hs)
         memset(hs->kbd.keycodes, 0, sizeof(hs->kbd.keycodes));
         memset(hs->kbd.key, 0, sizeof(hs->kbd.key));
         hs->kbd.keys = 0;
+        hs->kbd.modifiers = 0;
         break;
     case HID_MOUSE:
     case HID_TABLET:
-- 
2.9.3

[Qemu-devel] [PULL 4/7] sdl2: add assert to make coverity happy

[Gerd Hoffmann]

There is a loop a few lines up counting consoles and setting
sdl2_num_outputs accordingly, so con ptr can't be NULL there.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20170621122234.12751-1-kraxel@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/sdl2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index f76ee1081f..53dd447fd2 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -804,6 +804,7 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
     sdl2_console = g_new0(struct sdl2_console, sdl2_num_outputs);
     for (i = 0; i < sdl2_num_outputs; i++) {
         QemuConsole *con = qemu_console_lookup_by_index(i);
+        assert(con != NULL);
         if (!qemu_console_is_graphic(con)) {
             sdl2_console[i].hidden = true;
         }
-- 
2.9.3

[Qemu-devel] [PULL 5/7] ps2: add and use PS2State typedef

[Gerd Hoffmann]

Cleanup: Create and use a typedef for PS2State and stop passing void
pointers.  No functional change.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20170606112105.13331-2-kraxel@redhat.com
---
 include/hw/input/ps2.h  |  4 ++--
 include/qemu/typedefs.h |  1 +
 hw/input/ps2.c          | 12 +++++-------
 3 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/include/hw/input/ps2.h b/include/hw/input/ps2.h
index 7f0a80af9d..94709b8502 100644
--- a/include/hw/input/ps2.h
+++ b/include/hw/input/ps2.h
@@ -36,8 +36,8 @@ void *ps2_kbd_init(void (*update_irq)(void *, int), void *update_arg);
 void *ps2_mouse_init(void (*update_irq)(void *, int), void *update_arg);
 void ps2_write_mouse(void *, int val);
 void ps2_write_keyboard(void *, int val);
-uint32_t ps2_read_data(void *);
-void ps2_queue(void *, int b);
+uint32_t ps2_read_data(PS2State *s);
+void ps2_queue(PS2State *s, int b);
 void ps2_keyboard_set_translation(void *opaque, int mode);
 void ps2_mouse_fake_event(void *opaque);
 
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index f745d5faf7..2706aabedf 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -76,6 +76,7 @@ typedef struct PixelFormat PixelFormat;
 typedef struct PostcopyDiscardState PostcopyDiscardState;
 typedef struct Property Property;
 typedef struct PropertyInfo PropertyInfo;
+typedef struct PS2State PS2State;
 typedef struct QEMUBH QEMUBH;
 typedef struct QemuConsole QemuConsole;
 typedef struct QEMUFile QEMUFile;
diff --git a/hw/input/ps2.c b/hw/input/ps2.c
index 1d3a440bbd..37f8cb842e 100644
--- a/hw/input/ps2.c
+++ b/hw/input/ps2.c
@@ -85,12 +85,12 @@ typedef struct {
     int rptr, wptr, count;
 } PS2Queue;
 
-typedef struct {
+struct PS2State {
     PS2Queue queue;
     int32_t write_cmd;
     void (*update_irq)(void *, int);
     void *update_arg;
-} PS2State;
+};
 
 typedef struct {
     PS2State common;
@@ -551,9 +551,8 @@ static uint8_t translate_table[256] = {
     0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, 0xfe, 0xff,
 };
 
-void ps2_queue(void *opaque, int b)
+void ps2_queue(PS2State *s, int b)
 {
-    PS2State *s = (PS2State *)opaque;
     PS2Queue *q = &s->queue;
 
     if (q->count >= PS2_QUEUE_SIZE - 1)
@@ -692,13 +691,12 @@ static void ps2_keyboard_event(DeviceState *dev, QemuConsole *src,
     }
 }
 
-uint32_t ps2_read_data(void *opaque)
+uint32_t ps2_read_data(PS2State *s)
 {
-    PS2State *s = (PS2State *)opaque;
     PS2Queue *q;
     int val, index;
 
-    trace_ps2_read_data(opaque);
+    trace_ps2_read_data(s);
     q = &s->queue;
     if (q->count == 0) {
         /* NOTE: if no data left, we return the last keyboard one
-- 
2.9.3

[Qemu-devel] [PULL 6/7] ps2: add ps2_reset_queue

[Gerd Hoffmann]

Factor out ps2 queue reset to a separate function.
No functional change.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20170606112105.13331-3-kraxel@redhat.com
---
 hw/input/ps2.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/input/ps2.c b/hw/input/ps2.c
index 37f8cb842e..2416b58cc0 100644
--- a/hw/input/ps2.c
+++ b/hw/input/ps2.c
@@ -551,6 +551,15 @@ static uint8_t translate_table[256] = {
     0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, 0xfe, 0xff,
 };
 
+static void ps2_reset_queue(PS2State *s)
+{
+    PS2Queue *q = &s->queue;
+
+    q->rptr = 0;
+    q->wptr = 0;
+    q->count = 0;
+}
+
 void ps2_queue(PS2State *s, int b)
 {
     PS2Queue *q = &s->queue;
@@ -1079,12 +1088,8 @@ void ps2_write_mouse(void *opaque, int val)
 
 static void ps2_common_reset(PS2State *s)
 {
-    PS2Queue *q;
     s->write_cmd = -1;
-    q = &s->queue;
-    q->rptr = 0;
-    q->wptr = 0;
-    q->count = 0;
+    ps2_reset_queue(s);
     s->update_irq(s->update_arg, 0);
 }
 
-- 
2.9.3

[Qemu-devel] [PULL 7/7] ps2: reset queue in ps2_reset_keyboard

[Gerd Hoffmann]

When the guest resets the keyboard also clear the queue.  It is highly
unlikely that the guest is still interested in the events stuck in the
queue, and it avoids confusing the guest in case the queue is full and
the ACK can't be queued up.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1372583
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20170606112105.13331-4-kraxel@redhat.com
---
 hw/input/ps2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/input/ps2.c b/hw/input/ps2.c
index 2416b58cc0..3ba05efd06 100644
--- a/hw/input/ps2.c
+++ b/hw/input/ps2.c
@@ -740,6 +740,7 @@ static void ps2_reset_keyboard(PS2KbdState *s)
     trace_ps2_reset_keyboard(s);
     s->scan_enabled = 1;
     s->scancode_set = 2;
+    ps2_reset_queue(&s->common);
     ps2_set_ledstate(s, 0);
 }
 
-- 
2.9.3

Re: [Qemu-devel] [PULL 0/7] Ui and input 20170623 patches

[Peter Maydell]

On 23 June 2017 at 11:39, Gerd Hoffmann <kraxel@redhat.com> wrote:
> The following changes since commit e18a639164e162b3a4f078ab60606cd2f163d934:
>
>   Merge remote-tracking branch 'remotes/kraxel/tags/usb-20170621-pull-request' into staging (2017-06-22 15:09:48 +0100)
>
> are available in the git repository at:
>
>   git://git.kraxel.org/qemu tags/ui-and-input-20170623-pull-request
>
> for you to fetch changes up to 6e24ee0c1e4b6c0c9c748acab77ecd113c942a4d:
>
>   ps2: reset queue in ps2_reset_keyboard (2017-06-23 11:51:50 +0200)
>
> ----------------------------------------------------------------
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM