[Qemu-devel] [PULL 0/9] Input 20171023 patches

[Qemu-devel] [PULL 0/9] Input 20171023 patches

[Gerd Hoffmann]

The following changes since commit e822e81e350825dd94f41ee2538ff1432b812eb9:

  Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2017-10-20 15:04:00 +0100)

are available in the git repository at:

  git://git.kraxel.org/qemu tags/input-20171023-pull-request

for you to fetch changes up to 2182608f1ca00f4559486e9173a8e4242dd2d65d:

  ui: pull in latest keycodemapdb (2017-10-23 10:50:02 +0200)

----------------------------------------------------------------
input: fixes for ui input code and ps/2 keyboard (mostly sysrq key)

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

Daniel P. Berrange (9):
  input: use hex in ps2 keycode trace events
  ui: fix crash with sendkey and raw key numbers
  ui: use correct union field for key number
  ps2: fix scancodes sent for Alt-Print key combination (aka SysRq)
  ps2: fix scancodes sent for Shift/Ctrl+Print key combination
  ps2: fix scancodess sent for Pause key in AT set 1
  ps2: fix scancodes sent for Ctrl+Pause key combination
  ui: normalize the 'sysrq' key into the 'print' key
  ui: pull in latest keycodemapdb

 hw/input/ps2.c        | 207 +++++++++++++++++++++++++++++++++++++++++---------
 ui/input-legacy.c     |   5 ++
 ui/input.c            |  16 +++-
 hw/input/trace-events |   3 +-
 qapi/ui.json          |   7 ++
 ui/keycodemapdb       |   2 +-
 6 files changed, 201 insertions(+), 39 deletions(-)

-- 
2.9.3

[Qemu-devel] [PULL 1/9] input: use hex in ps2 keycode trace events

[Gerd Hoffmann]

From: "Daniel P. Berrange" <berrange@redhat.com>

Hardware scancodes are all documented in hex, so use that in trace
events to make it easier to understand.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20171019142848.572-2-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/input/trace-events | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/input/trace-events b/hw/input/trace-events
index 6fcb3c063f..d04132d342 100644
--- a/hw/input/trace-events
+++ b/hw/input/trace-events
@@ -1,7 +1,7 @@
 # See docs/devel/tracing.txt for syntax documentation.
 
 # hw/input/ps2.c
-ps2_put_keycode(void *opaque, int keycode) "%p keycode %d"
+ps2_put_keycode(void *opaque, int keycode) "%p keycode 0x%02x"
 ps2_read_data(void *opaque) "%p"
 ps2_set_ledstate(void *s, int ledstate) "%p ledstate %d"
 ps2_reset_keyboard(void *s) "%p"
-- 
2.9.3

[Qemu-devel] [PULL 2/9] ui: fix crash with sendkey and raw key numbers

[Gerd Hoffmann]

From: "Daniel P. Berrange" <berrange@redhat.com>

Previously we enforced that all key events are using QKeyCodes
at time they are sent:

  commit af07e5ff02ae6d4258fc5331007811d0b1c4d35a
  Author: Daniel P. Berrange <berrange@redhat.com>
  Date:   Fri Sep 29 11:12:00 2017 +0100

    ui: convert key events to QKeyCodes immediately

This commit forget to fix the code for the legacy 'sendkey'
command which still accepts key numbers from the user, which
then need converting to QKeyCodes

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20171019142848.572-3-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/input-legacy.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/ui/input-legacy.c b/ui/input-legacy.c
index 6bc3525499..c75aba1549 100644
--- a/ui/input-legacy.c
+++ b/ui/input-legacy.c
@@ -76,6 +76,11 @@ static KeyValue *copy_key_value(KeyValue *src)
 {
     KeyValue *dst = g_new(KeyValue, 1);
     memcpy(dst, src, sizeof(*src));
+    if (dst->type == KEY_VALUE_KIND_NUMBER) {
+        QKeyCode code = qemu_input_key_number_to_qcode(dst->u.number.data);
+        dst->type = KEY_VALUE_KIND_QCODE;
+        dst->u.qcode.data = code;
+    }
     return dst;
 }
 
-- 
2.9.3

[Qemu-devel] [PULL 3/9] ui: use correct union field for key number

[Gerd Hoffmann]

From: "Daniel P. Berrange" <berrange@redhat.com>

The code converting key numbers to QKeyCode in the 'input-send-event'
command mistakenly accessed the key->u.qcode union field instead of
the key->u.number field. This is harmless because the fields use the
same size datatype in both cases, but none the less it should be fixed
to avoid confusion.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20171019142848.572-4-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/input.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/input.c b/ui/input.c
index 290b47354a..4e821f8f2b 100644
--- a/ui/input.c
+++ b/ui/input.c
@@ -162,7 +162,7 @@ void qmp_input_send_event(bool has_device, const char *device,
         if (evt->type == INPUT_EVENT_KIND_KEY &&
             evt->u.key.data->key->type == KEY_VALUE_KIND_NUMBER) {
             KeyValue *key = evt->u.key.data->key;
-            QKeyCode code = qemu_input_key_number_to_qcode(key->u.qcode.data);
+            QKeyCode code = qemu_input_key_number_to_qcode(key->u.number.data);
             qemu_input_event_send_key_qcode(con, code, evt->u.key.data->down);
         } else {
             qemu_input_event_send(con, evt);
-- 
2.9.3

[Qemu-devel] [PULL 4/9] ps2: fix scancodes sent for Alt-Print key combination (aka SysRq)

[Gerd Hoffmann]

From: "Daniel P. Berrange" <berrange@redhat.com>

The 'Print' key is special in the AT set 1 / set 2 scancode definitions.

An unmodified 'Print' key is supposed to send

 AT Set 1:  e0 2a e0 37 (Down)  e0 b7 e0 aa (Up)
 AT Set 2:  e0 12 e0 7c (Down)  e0 f0 7c e0 f0 12 (Up)

which QEMU gets right. When pressed in combination with the 'Alt_L' or 'Alt_R'
keys (which signify SysRq), the scancodes are required to follow a different
scheme. With Alt_L, the expected sequences are

 AT set 1:  38, 54 (Down) d4, b8 (Up)
 AT set 2:  11, 84 (Down) f0 84, f0 11 (Up)

And with Alt_R

 AT set 1:  e0 38, 54 (Down) d4, e0 b8 (Up)
 AT set 2:  e0 11, 84 (Down) f0 84, f0 e0 11 (Up)

It is actually slightly more complicated than that, because (according results
of 'showkey -s', keyboards will in fact first release the currently pressed
modifier before sending the sequence above (which effectively re-presses &
then releases the modifier) and finally re-press the original modifier
afterwards. IOW, with Alt_L we need to send

 AT set 1:  b8, 38, 54 (Down) d4, b8, 38 (Up)
 AT set 2:  f0 11, 11, 84 (Down) f0 84, f0 11, 11 (Up)

And with Alt_R

 AT set 1:  e0 b8, e0 38, 54 (Down) d4, e0 b8, e0 38 (Up)
 AT set 2:  e0 f0 11, e0 11, 84 (Down) f0 84, e0 f0 11, e0 11 (Up)

The AT set 3 scancodes have no special handling for Alt-Print.

Rather than fixing the handling of the 'print' key in the ps2 driver to consider
the Alt modifiers, way back, a patch was commited that defined an extra 'sysrq'
key name:

  commit f2289cb6924afc97b2a75d21bfc9217024d11741
  Author: balrog <balrog@c046a42c-6fe2-441c-8c8c-71466251a162>
  Date:   Wed Jun 4 10:14:16 2008 +0000

    Add sysrq to key names known by "sendkey".

    Adding sysrq keycode to the table enabling running sysrq debugging in
    the guest via the monitor sendkey command, like:

    (qemu) sendkey alt-sysrq-t

    Tested on x86-64 target and Linux guest.

    Signed-off-by: Ryan Harper <ryanh@us.ibm.com>

With this patch QEMU would send

 AT set 1:  38, 54 (Down) d4, b8 (Up)
 AT set 2:  11, 84 (Down) f0 84, f0 11 (Up)

but this doesn't match what actual real keyboards send, as it is not releasing
the original modifier & pressing it again afterwards. In addition the original
problem remains, and a new problem was added:

  - The sequence 'alt-print-t' is still broken, acting as if 'print-t' was
    requested
  - The sequence 'sysrq-t' is broken, injecting an undefine scancode sequence
    tot he guest os (bare 0x54)

To deal with this mess we make these changes to the ps2 code, so that we track
the state of modifier keys (Alt, Shift, Ctrl - both left & right). Then we can
vary what scancodes are sent for Q_KEY_CODE_PRINT according to the Alt key
modifier state

Interestingly, it appears that of operating systems I've checked (Linux, FreeBSD
and OpenSolaris), none of them actually bother to validate the full sequences
for a unmodified 'Print' key. They all just ignore the leading "e0 2a" and
trigger based off "e0 37" alone. The latter two byte sequence is what keyboards
send with 'Print' is combined with 'Shift' or 'Ctrl' modifiers.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 20171019142848.572-5-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/input/ps2.c        | 137 ++++++++++++++++++++++++++++++++++++++++++--------
 hw/input/trace-events |   1 +
 2 files changed, 118 insertions(+), 20 deletions(-)

diff --git a/hw/input/ps2.c b/hw/input/ps2.c
index dff3f1e024..1e6f6ae9b6 100644
--- a/hw/input/ps2.c
+++ b/hw/input/ps2.c
@@ -78,6 +78,14 @@
 
 #define PS2_QUEUE_SIZE 16  /* Buffer size required by PS/2 protocol */
 
+/* Bits for 'modifiers' field in PS2KbdState */
+#define MOD_CTRL_L  (1 << 0)
+#define MOD_SHIFT_L (1 << 1)
+#define MOD_ALT_L   (1 << 2)
+#define MOD_CTRL_R  (1 << 3)
+#define MOD_SHIFT_R (1 << 4)
+#define MOD_ALT_R   (1 << 5)
+
 typedef struct {
     /* Keep the data array 256 bytes long, which compatibility
      with older qemu versions. */
@@ -99,6 +107,7 @@ typedef struct {
     int scancode_set; /* 1=XT, 2=AT, 3=PS/2 */
     int ledstate;
     bool need_high_bit;
+    unsigned int modifiers; /* bitmask of MOD_* constants above */
 } PS2KbdState;
 
 typedef struct {
@@ -545,6 +554,26 @@ static uint8_t translate_table[256] = {
     0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, 0xfe, 0xff,
 };
 
+static unsigned int ps2_modifier_bit(QKeyCode key)
+{
+    switch (key) {
+    case Q_KEY_CODE_CTRL:
+        return MOD_CTRL_L;
+    case Q_KEY_CODE_CTRL_R:
+        return MOD_CTRL_R;
+    case Q_KEY_CODE_SHIFT:
+        return MOD_SHIFT_L;
+    case Q_KEY_CODE_SHIFT_R:
+        return MOD_SHIFT_R;
+    case Q_KEY_CODE_ALT:
+        return MOD_ALT_L;
+    case Q_KEY_CODE_ALT_R:
+        return MOD_ALT_R;
+    default:
+        return 0;
+    }
+}
+
 static void ps2_reset_queue(PS2State *s)
 {
     PS2Queue *q = &s->queue;
@@ -596,11 +625,20 @@ static void ps2_keyboard_event(DeviceState *dev, QemuConsole *src,
     InputKeyEvent *key = evt->u.key.data;
     int qcode;
     uint16_t keycode;
+    int mod;
 
     qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
     assert(evt->type == INPUT_EVENT_KIND_KEY);
     qcode = qemu_input_key_value_to_qcode(key->key);
 
+    mod = ps2_modifier_bit(qcode);
+    trace_ps2_keyboard_event(s, qcode, key->down, mod, s->modifiers);
+    if (key->down) {
+        s->modifiers |= mod;
+    } else {
+        s->modifiers &= ~mod;
+    }
+
     if (s->scancode_set == 1) {
         if (qcode == Q_KEY_CODE_PAUSE) {
             if (key->down) {
@@ -612,16 +650,42 @@ static void ps2_keyboard_event(DeviceState *dev, QemuConsole *src,
                 ps2_put_keycode(s, 0xc5);
             }
         } else if (qcode == Q_KEY_CODE_PRINT) {
-            if (key->down) {
-                ps2_put_keycode(s, 0xe0);
-                ps2_put_keycode(s, 0x2a);
-                ps2_put_keycode(s, 0xe0);
-                ps2_put_keycode(s, 0x37);
+            if (s->modifiers & MOD_ALT_L) {
+                if (key->down) {
+                    ps2_put_keycode(s, 0xb8);
+                    ps2_put_keycode(s, 0x38);
+                    ps2_put_keycode(s, 0x54);
+                } else {
+                    ps2_put_keycode(s, 0xd4);
+                    ps2_put_keycode(s, 0xb8);
+                    ps2_put_keycode(s, 0x38);
+                }
+            } else if (s->modifiers & MOD_ALT_R) {
+                if (key->down) {
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0xb8);
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0x38);
+                    ps2_put_keycode(s, 0x54);
+                } else {
+                    ps2_put_keycode(s, 0xd4);
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0xb8);
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0x38);
+                }
             } else {
-                ps2_put_keycode(s, 0xe0);
-                ps2_put_keycode(s, 0xb7);
-                ps2_put_keycode(s, 0xe0);
-                ps2_put_keycode(s, 0xaa);
+                if (key->down) {
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0x2a);
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0x37);
+                } else {
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0xb7);
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0xaa);
+                }
             }
         } else {
             keycode = qcode_to_keycode_set1[qcode];
@@ -651,18 +715,50 @@ static void ps2_keyboard_event(DeviceState *dev, QemuConsole *src,
                 ps2_put_keycode(s, 0x77);
             }
         } else if (qcode == Q_KEY_CODE_PRINT) {
-            if (key->down) {
-                ps2_put_keycode(s, 0xe0);
-                ps2_put_keycode(s, 0x12);
-                ps2_put_keycode(s, 0xe0);
-                ps2_put_keycode(s, 0x7c);
+            if (s->modifiers & MOD_ALT_L) {
+                if (key->down) {
+                    ps2_put_keycode(s, 0xf0);
+                    ps2_put_keycode(s, 0x11);
+                    ps2_put_keycode(s, 0x11);
+                    ps2_put_keycode(s, 0x84);
+                } else {
+                    ps2_put_keycode(s, 0xf0);
+                    ps2_put_keycode(s, 0x84);
+                    ps2_put_keycode(s, 0xf0);
+                    ps2_put_keycode(s, 0x11);
+                    ps2_put_keycode(s, 0x11);
+                }
+            } else if (s->modifiers & MOD_ALT_R) {
+                if (key->down) {
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0xf0);
+                    ps2_put_keycode(s, 0x11);
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0x11);
+                    ps2_put_keycode(s, 0x84);
+                } else {
+                    ps2_put_keycode(s, 0xf0);
+                    ps2_put_keycode(s, 0x84);
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0xf0);
+                    ps2_put_keycode(s, 0x11);
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0x11);
+                }
             } else {
-                ps2_put_keycode(s, 0xe0);
-                ps2_put_keycode(s, 0xf0);
-                ps2_put_keycode(s, 0x7c);
-                ps2_put_keycode(s, 0xe0);
-                ps2_put_keycode(s, 0xf0);
-                ps2_put_keycode(s, 0x12);
+                if (key->down) {
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0x12);
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0x7c);
+                } else {
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0xf0);
+                    ps2_put_keycode(s, 0x7c);
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0xf0);
+                    ps2_put_keycode(s, 0x12);
+                }
             }
         } else {
             keycode = qcode_to_keycode_set2[qcode];
@@ -1125,6 +1221,7 @@ static void ps2_kbd_reset(void *opaque)
     s->scan_enabled = 0;
     s->translate = 0;
     s->scancode_set = 2;
+    s->modifiers = 0;
 }
 
 static void ps2_mouse_reset(void *opaque)
diff --git a/hw/input/trace-events b/hw/input/trace-events
index d04132d342..88150ef7a6 100644
--- a/hw/input/trace-events
+++ b/hw/input/trace-events
@@ -2,6 +2,7 @@
 
 # hw/input/ps2.c
 ps2_put_keycode(void *opaque, int keycode) "%p keycode 0x%02x"
+ps2_keyboard_event(void *opaque, int qcode, int down, unsigned int modifier, unsigned int modifiers) "%p qcode %d down %d modifier 0x%x modifiers 0x%x"
 ps2_read_data(void *opaque) "%p"
 ps2_set_ledstate(void *s, int ledstate) "%p ledstate %d"
 ps2_reset_keyboard(void *s) "%p"
-- 
2.9.3

[Qemu-devel] [PULL 5/9] ps2: fix scancodes sent for Shift/Ctrl+Print key combination

[Gerd Hoffmann]

From: "Daniel P. Berrange" <berrange@redhat.com>

The 'Print' key is special in the AT set 1 / set 2 scancode definitions.

An unmodified 'Print' key is supposed to send

 AT Set 1:  e0 2a e0 37 (Down)  e0 b7 e0 aa (Up)
 AT Set 2:  e0 12 e0 7c (Down)  e0 f0 7c e0 f0 12 (Up)

which QEMU gets right. When combined with Shift/Ctrl (both left and right
variants), the leading two bytes should be dropped, resulting in

 AT Set 1:  e0 37 (Down)  e0 b7 (Up)
 AT Set 2:  e0 7c (Down)  e0 f0 7c (Up)

This difference is pretty benign, since of all the operating systems I have
checked (Linux, FreeBSD and OpenStack), none bother to check the leading two
bytes anyway. This change none the less makes the ps2 device better follow real
hardware behaviour.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 20171019142848.572-6-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/input/ps2.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/hw/input/ps2.c b/hw/input/ps2.c
index 1e6f6ae9b6..c35b410e4d 100644
--- a/hw/input/ps2.c
+++ b/hw/input/ps2.c
@@ -674,6 +674,15 @@ static void ps2_keyboard_event(DeviceState *dev, QemuConsole *src,
                     ps2_put_keycode(s, 0xe0);
                     ps2_put_keycode(s, 0x38);
                 }
+            } else if (s->modifiers & (MOD_SHIFT_L | MOD_CTRL_L |
+                                       MOD_SHIFT_R | MOD_CTRL_R)) {
+                if (key->down) {
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0x37);
+                } else {
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0xb7);
+                }
             } else {
                 if (key->down) {
                     ps2_put_keycode(s, 0xe0);
@@ -745,6 +754,16 @@ static void ps2_keyboard_event(DeviceState *dev, QemuConsole *src,
                     ps2_put_keycode(s, 0xe0);
                     ps2_put_keycode(s, 0x11);
                 }
+            } else if (s->modifiers & (MOD_SHIFT_L | MOD_CTRL_L |
+                                       MOD_SHIFT_R | MOD_CTRL_R)) {
+                if (key->down) {
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0x7c);
+                } else {
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0xf0);
+                    ps2_put_keycode(s, 0x7c);
+                }
             } else {
                 if (key->down) {
                     ps2_put_keycode(s, 0xe0);
-- 
2.9.3

[Qemu-devel] [PULL 6/9] ps2: fix scancodess sent for Pause key in AT set 1

[Gerd Hoffmann]

From: "Daniel P. Berrange" <berrange@redhat.com>

The ps2 device was previously fixed to send the special Pause/Print
scancode sequences in:

  commit 8c10e0baf0260b59a4e984744462a18016662e3e
  Author: Hervé Poussineau <hpoussin@reactos.org>
  Date:   Thu Sep 15 22:06:26 2016 +0200

    ps2: use QEMU qcodes instead of scancodes

The sequence used for Pause had a small typo in the AT set 1, with a 0xe1
accidentally changed to 0x91.  This is not immediately visible with Linux
guests since they run the ps2 device with AT set 2 scancodes.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 20171019142848.572-7-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/input/ps2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/input/ps2.c b/hw/input/ps2.c
index c35b410e4d..133cc2aa64 100644
--- a/hw/input/ps2.c
+++ b/hw/input/ps2.c
@@ -645,7 +645,7 @@ static void ps2_keyboard_event(DeviceState *dev, QemuConsole *src,
                 ps2_put_keycode(s, 0xe1);
                 ps2_put_keycode(s, 0x1d);
                 ps2_put_keycode(s, 0x45);
-                ps2_put_keycode(s, 0x91);
+                ps2_put_keycode(s, 0xe1);
                 ps2_put_keycode(s, 0x9d);
                 ps2_put_keycode(s, 0xc5);
             }
-- 
2.9.3

[Qemu-devel] [PULL 7/9] ps2: fix scancodes sent for Ctrl+Pause key combination

[Gerd Hoffmann]

From: "Daniel P. Berrange" <berrange@redhat.com>

The 'Pause' key is special in the AT set 1 / set 2 scancode definitions.

An unmodified 'Pause' key is supposed to send

 AT Set 1:  e1 1d 45 91 9d c5 (Down)  <nothing> (Up)
 AT Set 2:  e1 14 77 e1 f0 14 f0 77 (Down)  <nothing> (Up)

which QEMU gets right. When combined with Ctrl (both left and right variants),
a different sequence is expected

 AT Set 1:  e0 46 e0 c6 (Down)  <nothing> (Up)
 AT Set 2:  e0 7e e0 f0 73 (Down)  <nothing> (Up)

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 20171019142848.572-8-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/input/ps2.c | 51 +++++++++++++++++++++++++++++++++++----------------
 1 file changed, 35 insertions(+), 16 deletions(-)

diff --git a/hw/input/ps2.c b/hw/input/ps2.c
index 133cc2aa64..f388a23c8e 100644
--- a/hw/input/ps2.c
+++ b/hw/input/ps2.c
@@ -641,13 +641,22 @@ static void ps2_keyboard_event(DeviceState *dev, QemuConsole *src,
 
     if (s->scancode_set == 1) {
         if (qcode == Q_KEY_CODE_PAUSE) {
-            if (key->down) {
-                ps2_put_keycode(s, 0xe1);
-                ps2_put_keycode(s, 0x1d);
-                ps2_put_keycode(s, 0x45);
-                ps2_put_keycode(s, 0xe1);
-                ps2_put_keycode(s, 0x9d);
-                ps2_put_keycode(s, 0xc5);
+            if (s->modifiers & (MOD_CTRL_L | MOD_CTRL_R)) {
+                if (key->down) {
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0x46);
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0xc6);
+                }
+            } else {
+                if (key->down) {
+                    ps2_put_keycode(s, 0xe1);
+                    ps2_put_keycode(s, 0x1d);
+                    ps2_put_keycode(s, 0x45);
+                    ps2_put_keycode(s, 0xe1);
+                    ps2_put_keycode(s, 0x9d);
+                    ps2_put_keycode(s, 0xc5);
+                }
             }
         } else if (qcode == Q_KEY_CODE_PRINT) {
             if (s->modifiers & MOD_ALT_L) {
@@ -713,15 +722,25 @@ static void ps2_keyboard_event(DeviceState *dev, QemuConsole *src,
         }
     } else if (s->scancode_set == 2) {
         if (qcode == Q_KEY_CODE_PAUSE) {
-            if (key->down) {
-                ps2_put_keycode(s, 0xe1);
-                ps2_put_keycode(s, 0x14);
-                ps2_put_keycode(s, 0x77);
-                ps2_put_keycode(s, 0xe1);
-                ps2_put_keycode(s, 0xf0);
-                ps2_put_keycode(s, 0x14);
-                ps2_put_keycode(s, 0xf0);
-                ps2_put_keycode(s, 0x77);
+            if (s->modifiers & (MOD_CTRL_L | MOD_CTRL_R)) {
+                if (key->down) {
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0x7e);
+                    ps2_put_keycode(s, 0xe0);
+                    ps2_put_keycode(s, 0xf0);
+                    ps2_put_keycode(s, 0x7e);
+                }
+            } else {
+                if (key->down) {
+                    ps2_put_keycode(s, 0xe1);
+                    ps2_put_keycode(s, 0x14);
+                    ps2_put_keycode(s, 0x77);
+                    ps2_put_keycode(s, 0xe1);
+                    ps2_put_keycode(s, 0xf0);
+                    ps2_put_keycode(s, 0x14);
+                    ps2_put_keycode(s, 0xf0);
+                    ps2_put_keycode(s, 0x77);
+                }
             }
         } else if (qcode == Q_KEY_CODE_PRINT) {
             if (s->modifiers & MOD_ALT_L) {
-- 
2.9.3

[Qemu-devel] [PULL 8/9] ui: normalize the 'sysrq' key into the 'print' key

[Gerd Hoffmann]

From: "Daniel P. Berrange" <berrange@redhat.com>

The 'sysrq' key was mistakenly added to QEMU to deal with incorrect handling
of the 'print' key in the ps2 device:

  commit f2289cb6924afc97b2a75d21bfc9217024d11741
  Author: balrog <balrog@c046a42c-6fe2-441c-8c8c-71466251a162>
  Date:   Wed Jun 4 10:14:16 2008 +0000

    Add sysrq to key names known by "sendkey".

    Adding sysrq keycode to the table enabling running sysrq debugging in
    the guest via the monitor sendkey command, like:

    (qemu) sendkey alt-sysrq-t

    Tested on x86-64 target and Linux guest.

    Signed-off-by: Ryan Harper <ryanh@us.ibm.com>

The ps2 device is now fixed wrt modifiers and the 'print' key. Further the
handling of the 'sysrq' key has some problems of its own, documented in the
previous commit. To cleanup this mess, we convert any use of 'sysrq' into
'print' prior to dispatching the event to device models.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 20171019142848.572-9-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/input.c   | 14 ++++++++++++++
 qapi/ui.json |  7 +++++++
 2 files changed, 21 insertions(+)

diff --git a/ui/input.c b/ui/input.c
index 4e821f8f2b..3e2d324278 100644
--- a/ui/input.c
+++ b/ui/input.c
@@ -353,6 +353,20 @@ void qemu_input_event_send(QemuConsole *src, InputEvent *evt)
     assert(!(evt->type == INPUT_EVENT_KIND_KEY &&
              evt->u.key.data->key->type == KEY_VALUE_KIND_NUMBER));
 
+
+    /*
+     * 'sysrq' was mistakenly added to hack around the fact that
+     * the ps2 driver was not generating correct scancodes sequences
+     * when 'alt+print' was pressed. This flaw is now fixed and the
+     * 'sysrq' key serves no further purpose. We normalize it to
+     * 'print', so that downstream receivers of the event don't
+     * neeed to deal with this mistake
+     */
+    if (evt->type == INPUT_EVENT_KIND_KEY &&
+        evt->u.key.data->key->u.qcode.data == Q_KEY_CODE_SYSRQ) {
+        evt->u.key.data->key->u.qcode.data = Q_KEY_CODE_PRINT;
+    }
+
     if (!runstate_is_running() && !runstate_check(RUN_STATE_SUSPENDED)) {
         return;
     }
diff --git a/qapi/ui.json b/qapi/ui.json
index e5d6610b4a..07b468f625 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -748,6 +748,13 @@
 # @ac_bookmarks: since 2.10
 # altgr, altgr_r: dropped in 2.10
 #
+# 'sysrq' was mistakenly added to hack around the fact that
+# the ps2 driver was not generating correct scancodes sequences
+# when 'alt+print' was pressed. This flaw is now fixed and the
+# 'sysrq' key serves no further purpose. Any further use of
+# 'sysrq' will be transparently changed to 'print', so they
+# are effectively synonyms.
+#
 # Since: 1.3.0
 #
 ##
-- 
2.9.3

[Qemu-devel] [PULL 9/9] ui: pull in latest keycodemapdb

[Gerd Hoffmann]

From: "Daniel P. Berrange" <berrange@redhat.com>

Latest keycodemapdb has a fix for Sun keyboard Pause mapping
and backcompat fix for QEMU's treatment of 0xb7 as an alternative
to 0x54 for triggering Print/SysRq

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 20171019142848.572-10-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/keycodemapdb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/keycodemapdb b/ui/keycodemapdb
index 56ce5650d2..10739aa260 160000
--- a/ui/keycodemapdb
+++ b/ui/keycodemapdb
@@ -1 +1 @@
-Subproject commit 56ce5650d2c6ea216b4580df44b9a6dd3bc92c3b
+Subproject commit 10739aa26051a5d49d88132604539d3ed085e72e
-- 
2.9.3

Re: [Qemu-devel] [PULL 0/9] Input 20171023 patches

[Peter Maydell]

On 23 October 2017 at 10:19, Gerd Hoffmann <kraxel@redhat.com> wrote:
> The following changes since commit e822e81e350825dd94f41ee2538ff1432b812eb9:
>
>   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2017-10-20 15:04:00 +0100)
>
> are available in the git repository at:
>
>   git://git.kraxel.org/qemu tags/input-20171023-pull-request
>
> for you to fetch changes up to 2182608f1ca00f4559486e9173a8e4242dd2d65d:
>
>   ui: pull in latest keycodemapdb (2017-10-23 10:50:02 +0200)
>
> ----------------------------------------------------------------
> input: fixes for ui input code and ps/2 keyboard (mostly sysrq key)
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PULL 4/9] ps2: fix scancodes sent for Alt-Print key combination (aka SysRq)

[Daniel P. Berrange]

On Mon, Oct 23, 2017 at 11:19:42AM +0200, Gerd Hoffmann wrote:
> From: "Daniel P. Berrange" <berrange@redhat.com>
> 
> The 'Print' key is special in the AT set 1 / set 2 scancode definitions.
> 
> An unmodified 'Print' key is supposed to send
> 
>  AT Set 1:  e0 2a e0 37 (Down)  e0 b7 e0 aa (Up)
>  AT Set 2:  e0 12 e0 7c (Down)  e0 f0 7c e0 f0 12 (Up)
> 
> which QEMU gets right. When pressed in combination with the 'Alt_L' or 'Alt_R'
> keys (which signify SysRq), the scancodes are required to follow a different
> scheme. With Alt_L, the expected sequences are
> 
>  AT set 1:  38, 54 (Down) d4, b8 (Up)
>  AT set 2:  11, 84 (Down) f0 84, f0 11 (Up)
> 
> And with Alt_R
> 
>  AT set 1:  e0 38, 54 (Down) d4, e0 b8 (Up)
>  AT set 2:  e0 11, 84 (Down) f0 84, f0 e0 11 (Up)
> 
> It is actually slightly more complicated than that, because (according results
> of 'showkey -s', keyboards will in fact first release the currently pressed
> modifier before sending the sequence above (which effectively re-presses &
> then releases the modifier) and finally re-press the original modifier
> afterwards. IOW, with Alt_L we need to send
> 
>  AT set 1:  b8, 38, 54 (Down) d4, b8, 38 (Up)
>  AT set 2:  f0 11, 11, 84 (Down) f0 84, f0 11, 11 (Up)
> 
> And with Alt_R
> 
>  AT set 1:  e0 b8, e0 38, 54 (Down) d4, e0 b8, e0 38 (Up)
>  AT set 2:  e0 f0 11, e0 11, 84 (Down) f0 84, e0 f0 11, e0 11 (Up)
> 
> The AT set 3 scancodes have no special handling for Alt-Print.
> 
> Rather than fixing the handling of the 'print' key in the ps2 driver to consider
> the Alt modifiers, way back, a patch was commited that defined an extra 'sysrq'
> key name:
> 
>   commit f2289cb6924afc97b2a75d21bfc9217024d11741
>   Author: balrog <balrog@c046a42c-6fe2-441c-8c8c-71466251a162>
>   Date:   Wed Jun 4 10:14:16 2008 +0000
> 
>     Add sysrq to key names known by "sendkey".
> 
>     Adding sysrq keycode to the table enabling running sysrq debugging in
>     the guest via the monitor sendkey command, like:
> 
>     (qemu) sendkey alt-sysrq-t
> 
>     Tested on x86-64 target and Linux guest.
> 
>     Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
> 
> With this patch QEMU would send
> 
>  AT set 1:  38, 54 (Down) d4, b8 (Up)
>  AT set 2:  11, 84 (Down) f0 84, f0 11 (Up)
> 
> but this doesn't match what actual real keyboards send, as it is not releasing
> the original modifier & pressing it again afterwards. In addition the original
> problem remains, and a new problem was added:
> 
>   - The sequence 'alt-print-t' is still broken, acting as if 'print-t' was
>     requested
>   - The sequence 'sysrq-t' is broken, injecting an undefine scancode sequence
>     tot he guest os (bare 0x54)
> 
> To deal with this mess we make these changes to the ps2 code, so that we track
> the state of modifier keys (Alt, Shift, Ctrl - both left & right). Then we can
> vary what scancodes are sent for Q_KEY_CODE_PRINT according to the Alt key
> modifier state
> 
> Interestingly, it appears that of operating systems I've checked (Linux, FreeBSD
> and OpenSolaris), none of them actually bother to validate the full sequences
> for a unmodified 'Print' key. They all just ignore the leading "e0 2a" and
> trigger based off "e0 37" alone. The latter two byte sequence is what keyboards
> send with 'Print' is combined with 'Shift' or 'Ctrl' modifiers.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> Message-id: 20171019142848.572-5-berrange@redhat.com
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/input/ps2.c        | 137 ++++++++++++++++++++++++++++++++++++++++++--------
>  hw/input/trace-events |   1 +
>  2 files changed, 118 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/input/ps2.c b/hw/input/ps2.c
> index dff3f1e024..1e6f6ae9b6 100644
> --- a/hw/input/ps2.c
> +++ b/hw/input/ps2.c
> @@ -78,6 +78,14 @@
>  
>  #define PS2_QUEUE_SIZE 16  /* Buffer size required by PS/2 protocol */
>  
> +/* Bits for 'modifiers' field in PS2KbdState */
> +#define MOD_CTRL_L  (1 << 0)
> +#define MOD_SHIFT_L (1 << 1)
> +#define MOD_ALT_L   (1 << 2)
> +#define MOD_CTRL_R  (1 << 3)
> +#define MOD_SHIFT_R (1 << 4)
> +#define MOD_ALT_R   (1 << 5)
> +
>  typedef struct {
>      /* Keep the data array 256 bytes long, which compatibility
>       with older qemu versions. */
> @@ -99,6 +107,7 @@ typedef struct {
>      int scancode_set; /* 1=XT, 2=AT, 3=PS/2 */
>      int ledstate;
>      bool need_high_bit;
> +    unsigned int modifiers; /* bitmask of MOD_* constants above */
>  } PS2KbdState;

No one confirmed whether or not this new field needs to be added to the
VMSTATE sections, and if so, how todo this in a back compatible manner....


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PULL 4/9] ps2: fix scancodes sent for Alt-Print key combination (aka SysRq)

[Gerd Hoffmann]

  Hi,

> No one confirmed whether or not this new field needs to be added to
> the
> VMSTATE sections, and if so, how todo this in a back compatible
> manner....

Good question.  Sky isn't falling if we don't send this over, worst
case is a minor keyboard glitch, affecting only the SysRq key, right?

Annoying things like sticky modifier keys should not happen.  So I'd
tend to simply leave things as-is.

When sending it over we need a subsection for it.  Simplest way would
be to only send over the section in the unlikely case that the field is
non-zero.  But this might fail migration to older qemu versions in case
someone hits a modifier key the wrong moment.  Alternatively send the
sibsection only on new machine types using a compat property, but that
needs a bit more boilerplate.

cheers,
  Gerd

Re: [Qemu-devel] [PULL 4/9] ps2: fix scancodes sent for Alt-Print key combination (aka SysRq)

[Daniel P. Berrange]

On Wed, Nov 01, 2017 at 08:58:32AM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > No one confirmed whether or not this new field needs to be added to
> > the
> > VMSTATE sections, and if so, how todo this in a back compatible
> > manner....
> 
> Good question.  Sky isn't falling if we don't send this over, worst
> case is a minor keyboard glitch, affecting only the SysRq key, right?
> 
> Annoying things like sticky modifier keys should not happen.  So I'd
> tend to simply leave things as-is.

Yeah, we would only gitch that key combo. It should be (practically) impossible
to hit, because from the user input POV its just a single key, that internally
we turn into a sequence of keys when injecting.

> 
> When sending it over we need a subsection for it.  Simplest way would
> be to only send over the section in the unlikely case that the field is
> non-zero.  But this might fail migration to older qemu versions in case
> someone hits a modifier key the wrong moment.  Alternatively send the
> sibsection only on new machine types using a compat property, but that
> needs a bit more boilerplate.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|