From d3b787fa7ddea1c66ee59eb332d1523c67db8cf6 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 6 Jun 2017 15:47:36 +0200 Subject: [PATCH 1/7] keymaps: add tracing 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 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" From 77b0359bf414ad666d1714dc9888f1017c08e283 Mon Sep 17 00:00:00 2001 From: Alexander Graf Date: Thu, 22 Jun 2017 09:41:58 +0200 Subject: [PATCH 2/7] input: Decrement queue count on kbd delay 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 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 --- 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)) { From 51dbea77a29ea46173373a6dad4ebd95d4661f42 Mon Sep 17 00:00:00 2001 From: Alexander Graf Date: Thu, 22 Jun 2017 09:41:35 +0200 Subject: [PATCH 3/7] hid: Reset kbd modifiers on reset 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 Message-id: 1498117295-162030-1-git-send-email-agraf@suse.de Signed-off-by: Gerd Hoffmann --- 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: From 85970a627f308483053f18a342037245d0fee469 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 21 Jun 2017 14:22:34 +0200 Subject: [PATCH 4/7] sdl2: add assert to make coverity happy 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 Message-id: 20170621122234.12751-1-kraxel@redhat.com Signed-off-by: Gerd Hoffmann --- 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; } From 8498bb8d2e19f5c280aaac6be772fad5980e7519 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 6 Jun 2017 13:21:03 +0200 Subject: [PATCH 5/7] ps2: add and use PS2State typedef Cleanup: Create and use a typedef for PS2State and stop passing void pointers. No functional change. Signed-off-by: Gerd Hoffmann Message-id: 20170606112105.13331-2-kraxel@redhat.com --- hw/input/ps2.c | 12 +++++------- include/hw/input/ps2.h | 4 ++-- include/qemu/typedefs.h | 1 + 3 files changed, 8 insertions(+), 9 deletions(-) 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 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; From 954ee55bd528a97347f19f815e00f1e375f82f0a Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 6 Jun 2017 13:21:04 +0200 Subject: [PATCH 6/7] ps2: add ps2_reset_queue Factor out ps2 queue reset to a separate function. No functional change. Signed-off-by: Gerd Hoffmann 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); } From 6e24ee0c1e4b6c0c9c748acab77ecd113c942a4d Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 6 Jun 2017 13:21:05 +0200 Subject: [PATCH 7/7] ps2: reset queue in ps2_reset_keyboard 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 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); }