qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] hw/input/hid: support alternative sysrq/break scancodes for gtk-vnc
@ 2016-10-28 14:51 Peter Korsgaard
  2016-11-07 12:06 ` Peter Korsgaard
  2016-11-09 13:49 ` Gerd Hoffmann
  0 siblings, 2 replies; 3+ messages in thread
From: Peter Korsgaard @ 2016-10-28 14:51 UTC (permalink / raw)
  To: berrange, kraxel, qemu-devel; +Cc: Peter Korsgaard

The printscreen/sysrq and pause/break keys currently don't work for guests
using -usbdevice keyboard when accessed through vnc with a gtk-vnc based
client.

The reason for this is a mismatch between gtk-vnc and qemu in how these keys
should be mapped to XT keycodes.

On the original IBM XT these keys behaved differently than other keys.

Quoting from https://www.win.tue.nl/~aeb/linux/kbd/scancodes-1.html:

The keys PrtSc/SysRq and Pause/Break are special. The former produces
scancode e0 2a e0 37 when no modifier key is pressed simultaneously, e0 37
together with Shift or Ctrl, but 54 together with (left or right) Alt.  (And
one gets the expected sequences upon release.  But see below.) The latter
produces scancode sequence e1 1d 45 e1 9d c5 when pressed (without modifier)
and nothing at all upon release.  However, together with (left or right)
Ctrl, one gets e0 46 e0 c6, and again nothing at release.  It does not
repeat.

Gtk-vnc supports the 'QEMU Extended Key Event Message' RFB extension to send
raw XT keycodes directly to qemu, but the specification doesn't explicitly
specify how to map such long/complicated keycode sequences.  From the spec
(https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#qemu-extended-key-event-message)

The keycode is the XT keycode that produced the keysym. An XT keycode is an
XT make scancode sequence encoded to fit in a single U32 quantity.  Single
byte XT scancodes with a byte value less than 0x7f are encoded as is.
2-byte XT scancodes whose first byte is 0xe0 and second byte is less than
0x7f are encoded with the high bit of the first byte set

hid.c currently expects the keycode sequence with shift/ctl for sysrq (e0 37
-> 0xb7 in RFB), whereas gtk-vnc uses the sequence with alt (0x54).
Likewise, hid.c expects the code without modifiers (e1 1d 45 -> 0xc5 in
RFB), whereas gtk-vnc sends the keycode sequence with ctrl for pause (e0 46
-> 0xc6 in RFB).

See keymaps.cvs in gtk-vnc for the mapping used:
https://git.gnome.org/browse/gtk-vnc/tree/src/keymaps.csv#n150

Now, it isn't obvious to me which sequence is really "right", but as the
0x54/0xc6 keycodes are currently unused in hid.c, supporting both seems like
the pragmatic solution to me.  The USB HID keyboard boot protocol used by
hid.c doesn't have any other mapping applicable to these keys.

The other guest keyboard interfaces (ps/2, virtio, ..) are not affected,
because they handle these keys differently.

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
---
 hw/input/hid.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/input/hid.c b/hw/input/hid.c
index 5e2850e..fa9cc4c 100644
--- a/hw/input/hid.c
+++ b/hw/input/hid.c
@@ -46,7 +46,7 @@ static const uint8_t hid_usage_keys[0x100] = {
     0xe2, 0x2c, 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0x3e,
     0x3f, 0x40, 0x41, 0x42, 0x43, 0x53, 0x47, 0x5f,
     0x60, 0x61, 0x56, 0x5c, 0x5d, 0x5e, 0x57, 0x59,
-    0x5a, 0x5b, 0x62, 0x63, 0x00, 0x00, 0x64, 0x44,
+    0x5a, 0x5b, 0x62, 0x63, 0x46, 0x00, 0x64, 0x44,
     0x45, 0x68, 0x69, 0x6a, 0x6b, 0x6c, 0x6d, 0x6e,
     0xe8, 0xe9, 0x71, 0x72, 0x73, 0x00, 0x00, 0x00,
     0x00, 0x00, 0x00, 0x85, 0x00, 0x00, 0x00, 0x00,
@@ -61,7 +61,7 @@ static const uint8_t hid_usage_keys[0x100] = {
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
     0x00, 0x00, 0x00, 0x00, 0x00, 0x54, 0x00, 0x46,
     0xe6, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x48, 0x00, 0x4a,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x48, 0x48, 0x4a,
     0x52, 0x4b, 0x00, 0x50, 0x00, 0x4f, 0x00, 0x4d,
     0x51, 0x4e, 0x49, 0x4c, 0x00, 0x00, 0x00, 0x00,
     0x00, 0x00, 0x00, 0xe3, 0xe7, 0x65, 0x00, 0x00,
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] hw/input/hid: support alternative sysrq/break scancodes for gtk-vnc
  2016-10-28 14:51 [Qemu-devel] [PATCH] hw/input/hid: support alternative sysrq/break scancodes for gtk-vnc Peter Korsgaard
@ 2016-11-07 12:06 ` Peter Korsgaard
  2016-11-09 13:49 ` Gerd Hoffmann
  1 sibling, 0 replies; 3+ messages in thread
From: Peter Korsgaard @ 2016-11-07 12:06 UTC (permalink / raw)
  To: berrange, kraxel, qemu-devel; +Cc: Peter Korsgaard

Ping? Any feedback? Anything else needed?

Original patch is here: http://patchwork.ozlabs.org/patch/688469/

On Fri, Oct 28, 2016 at 4:51 PM, Peter Korsgaard <peter@korsgaard.com>
wrote:

> The printscreen/sysrq and pause/break keys currently don't work for guests
> using -usbdevice keyboard when accessed through vnc with a gtk-vnc based
> client.
>
> The reason for this is a mismatch between gtk-vnc and qemu in how these
> keys
> should be mapped to XT keycodes.
>
> On the original IBM XT these keys behaved differently than other keys.
>
> Quoting from https://www.win.tue.nl/~aeb/linux/kbd/scancodes-1.html:
>
> The keys PrtSc/SysRq and Pause/Break are special. The former produces
> scancode e0 2a e0 37 when no modifier key is pressed simultaneously, e0 37
> together with Shift or Ctrl, but 54 together with (left or right) Alt.
> (And
> one gets the expected sequences upon release.  But see below.) The latter
> produces scancode sequence e1 1d 45 e1 9d c5 when pressed (without
> modifier)
> and nothing at all upon release.  However, together with (left or right)
> Ctrl, one gets e0 46 e0 c6, and again nothing at release.  It does not
> repeat.
>
> Gtk-vnc supports the 'QEMU Extended Key Event Message' RFB extension to
> send
> raw XT keycodes directly to qemu, but the specification doesn't explicitly
> specify how to map such long/complicated keycode sequences.  From the spec
> (https://github.com/rfbproto/rfbproto/blob/master/rfbproto.
> rst#qemu-extended-key-event-message)
>
> The keycode is the XT keycode that produced the keysym. An XT keycode is an
> XT make scancode sequence encoded to fit in a single U32 quantity.  Single
> byte XT scancodes with a byte value less than 0x7f are encoded as is.
> 2-byte XT scancodes whose first byte is 0xe0 and second byte is less than
> 0x7f are encoded with the high bit of the first byte set
>
> hid.c currently expects the keycode sequence with shift/ctl for sysrq (e0
> 37
> -> 0xb7 in RFB), whereas gtk-vnc uses the sequence with alt (0x54).
> Likewise, hid.c expects the code without modifiers (e1 1d 45 -> 0xc5 in
> RFB), whereas gtk-vnc sends the keycode sequence with ctrl for pause (e0 46
> -> 0xc6 in RFB).
>
> See keymaps.cvs in gtk-vnc for the mapping used:
> https://git.gnome.org/browse/gtk-vnc/tree/src/keymaps.csv#n150
>
> Now, it isn't obvious to me which sequence is really "right", but as the
> 0x54/0xc6 keycodes are currently unused in hid.c, supporting both seems
> like
> the pragmatic solution to me.  The USB HID keyboard boot protocol used by
> hid.c doesn't have any other mapping applicable to these keys.
>
> The other guest keyboard interfaces (ps/2, virtio, ..) are not affected,
> because they handle these keys differently.
>
> Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
> ---
>  hw/input/hid.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/input/hid.c b/hw/input/hid.c
> index 5e2850e..fa9cc4c 100644
> --- a/hw/input/hid.c
> +++ b/hw/input/hid.c
> @@ -46,7 +46,7 @@ static const uint8_t hid_usage_keys[0x100] = {
>      0xe2, 0x2c, 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0x3e,
>      0x3f, 0x40, 0x41, 0x42, 0x43, 0x53, 0x47, 0x5f,
>      0x60, 0x61, 0x56, 0x5c, 0x5d, 0x5e, 0x57, 0x59,
> -    0x5a, 0x5b, 0x62, 0x63, 0x00, 0x00, 0x64, 0x44,
> +    0x5a, 0x5b, 0x62, 0x63, 0x46, 0x00, 0x64, 0x44,
>      0x45, 0x68, 0x69, 0x6a, 0x6b, 0x6c, 0x6d, 0x6e,
>      0xe8, 0xe9, 0x71, 0x72, 0x73, 0x00, 0x00, 0x00,
>      0x00, 0x00, 0x00, 0x85, 0x00, 0x00, 0x00, 0x00,
> @@ -61,7 +61,7 @@ static const uint8_t hid_usage_keys[0x100] = {
>      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>      0x00, 0x00, 0x00, 0x00, 0x00, 0x54, 0x00, 0x46,
>      0xe6, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> -    0x00, 0x00, 0x00, 0x00, 0x00, 0x48, 0x00, 0x4a,
> +    0x00, 0x00, 0x00, 0x00, 0x00, 0x48, 0x48, 0x4a,
>      0x52, 0x4b, 0x00, 0x50, 0x00, 0x4f, 0x00, 0x4d,
>      0x51, 0x4e, 0x49, 0x4c, 0x00, 0x00, 0x00, 0x00,
>      0x00, 0x00, 0x00, 0xe3, 0xe7, 0x65, 0x00, 0x00,
> --
> 2.9.3
>
>


-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] hw/input/hid: support alternative sysrq/break scancodes for gtk-vnc
  2016-10-28 14:51 [Qemu-devel] [PATCH] hw/input/hid: support alternative sysrq/break scancodes for gtk-vnc Peter Korsgaard
  2016-11-07 12:06 ` Peter Korsgaard
@ 2016-11-09 13:49 ` Gerd Hoffmann
  1 sibling, 0 replies; 3+ messages in thread
From: Gerd Hoffmann @ 2016-11-09 13:49 UTC (permalink / raw)
  To: Peter Korsgaard; +Cc: berrange, qemu-devel

  Hi,

> Now, it isn't obvious to me which sequence is really "right", but as the
> 0x54/0xc6 keycodes are currently unused in hid.c, supporting both seems like
> the pragmatic solution to me.  The USB HID keyboard boot protocol used by
> hid.c doesn't have any other mapping applicable to these keys.
> 
> The other guest keyboard interfaces (ps/2, virtio, ..) are not affected,
> because they handle these keys differently.

A better fix would be to switch hid.c to use Q_KEY_CODE_* too.
Patches doing that are welcome.  It's a big change though and not
reasonable to do during the freeze, so I'll go pick up this two-line fix
for 2.8 nevertheless.

thanks,
  Gerd

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-11-09 13:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-28 14:51 [Qemu-devel] [PATCH] hw/input/hid: support alternative sysrq/break scancodes for gtk-vnc Peter Korsgaard
2016-11-07 12:06 ` Peter Korsgaard
2016-11-09 13:49 ` Gerd Hoffmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).