qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH for 2.10] ps2: fix sending of PAUSE/BREAK scancodes
@ 2017-07-24 16:46 Daniel P. Berrange
  2017-07-24 19:55 ` Hervé Poussineau
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel P. Berrange @ 2017-07-24 16:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann, Hervé Poussineau, Daniel P. Berrange

The processing of the scancodes for PAUSE/BREAK  has been broken since
the conversion to qcodes 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

When using a VNC client, with the raw scancode extension, the client
will send a scancode of 0xc6 for both PAUSE and BREAK. There is mistakenly
no entry in the qcode_to_number table for this scancode, so
ps2_keyboard_event() just generates a log message and discards the
scancode

When using a SPICE client, it will also send 0xc6 for BREAK, but
will send 0xe1 0x1d 0x45 0xe1 0x9d 0xc5 for PAUSE. There is no
entry in the qcode_to_number table for the scancode 0xe1 because
it is a special XT keyboard prefix not mapping to any QKeyCode.
Again ps2_keyboard_event() just generates a log message and discards
the scancode. The following 0x1d, 0x45, 0x9d, 0xc5 scancodes get
handled correctly. Fixing this just requires special casing 0xe1 so
it is directly queued for sending to the guest, skipping any conversion
to QKeyCode.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 hw/input/ps2.c    | 7 +++++++
 ui/input-keymap.c | 1 +
 2 files changed, 8 insertions(+)

diff --git a/hw/input/ps2.c b/hw/input/ps2.c
index 3ba05efd06..a132d1ba72 100644
--- a/hw/input/ps2.c
+++ b/hw/input/ps2.c
@@ -607,6 +607,13 @@ static void ps2_keyboard_event(DeviceState *dev, QemuConsole *src,
     assert(evt->type == INPUT_EVENT_KIND_KEY);
     qcode = qemu_input_key_value_to_qcode(key->key);
 
+    if (qcode == 0 &&
+        key->key->type == KEY_VALUE_KIND_NUMBER &&
+        key->key->u.number.data == 0x61) {
+        ps2_put_keycode(s, 0xe1);
+        return;
+    }
+
     if (s->scancode_set == 1) {
         if (qcode == Q_KEY_CODE_PAUSE) {
             if (key->down) {
diff --git a/ui/input-keymap.c b/ui/input-keymap.c
index 8a1476fc48..9211f835be 100644
--- a/ui/input-keymap.c
+++ b/ui/input-keymap.c
@@ -98,6 +98,7 @@ static const int qcode_to_number[] = {
     [Q_KEY_CODE_KP_ENTER] = 0x9c,
     [Q_KEY_CODE_KP_DECIMAL] = 0x53,
     [Q_KEY_CODE_SYSRQ] = 0x54,
+    [Q_KEY_CODE_PAUSE] = 0xc6,
 
     [Q_KEY_CODE_KP_0] = 0x52,
     [Q_KEY_CODE_KP_1] = 0x4f,
-- 
2.13.3

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

* Re: [Qemu-devel] [PATCH for 2.10] ps2: fix sending of PAUSE/BREAK scancodes
  2017-07-24 16:46 [Qemu-devel] [PATCH for 2.10] ps2: fix sending of PAUSE/BREAK scancodes Daniel P. Berrange
@ 2017-07-24 19:55 ` Hervé Poussineau
  2017-07-25  8:32   ` Daniel P. Berrange
  0 siblings, 1 reply; 10+ messages in thread
From: Hervé Poussineau @ 2017-07-24 19:55 UTC (permalink / raw)
  To: Daniel P. Berrange, qemu-devel; +Cc: Gerd Hoffmann

Le 24/07/2017 à 18:46, Daniel P. Berrange a écrit :
> The processing of the scancodes for PAUSE/BREAK  has been broken since
> the conversion to qcodes 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
>
> When using a VNC client, with the raw scancode extension, the client
> will send a scancode of 0xc6 for both PAUSE and BREAK. There is mistakenly
> no entry in the qcode_to_number table for this scancode, so
> ps2_keyboard_event() just generates a log message and discards the
> scancode
>
> When using a SPICE client, it will also send 0xc6 for BREAK, but
> will send 0xe1 0x1d 0x45 0xe1 0x9d 0xc5 for PAUSE. There is no
> entry in the qcode_to_number table for the scancode 0xe1 because
> it is a special XT keyboard prefix not mapping to any QKeyCode.
> Again ps2_keyboard_event() just generates a log message and discards
> the scancode. The following 0x1d, 0x45, 0x9d, 0xc5 scancodes get
> handled correctly. Fixing this just requires special casing 0xe1 so
> it is directly queued for sending to the guest, skipping any conversion
> to QKeyCode.

Your commit message is divided in 2 parts, and you're touching 1 file for each part of the commit message.
Should it be 2 patches instead?

>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  hw/input/ps2.c    | 7 +++++++
>  ui/input-keymap.c | 1 +
>  2 files changed, 8 insertions(+)
>
> diff --git a/hw/input/ps2.c b/hw/input/ps2.c
> index 3ba05efd06..a132d1ba72 100644
> --- a/hw/input/ps2.c
> +++ b/hw/input/ps2.c
> @@ -607,6 +607,13 @@ static void ps2_keyboard_event(DeviceState *dev, QemuConsole *src,
>      assert(evt->type == INPUT_EVENT_KIND_KEY);
>      qcode = qemu_input_key_value_to_qcode(key->key);
>
> +    if (qcode == 0 &&
> +        key->key->type == KEY_VALUE_KIND_NUMBER &&
> +        key->key->u.number.data == 0x61) {
> +        ps2_put_keycode(s, 0xe1);
> +        return;
> +    }
> +

You're putting some specific code for spice in ps2 emulation.
IMO, the workaround should be moved to spice keyboard handling (ui/spice-input.c), which needs to generate a qcode instead of a scancode.
Here, you're making ps2 emulation work again with spice, but you're missing all other emulations using qcodes. Do you want to also modify them?
I understand that it may be the easiest way to fix the regression for 2.10, but it needs at least a comment.

>      if (s->scancode_set == 1) {
>          if (qcode == Q_KEY_CODE_PAUSE) {
>              if (key->down) {
> diff --git a/ui/input-keymap.c b/ui/input-keymap.c
> index 8a1476fc48..9211f835be 100644
> --- a/ui/input-keymap.c
> +++ b/ui/input-keymap.c
> @@ -98,6 +98,7 @@ static const int qcode_to_number[] = {
>      [Q_KEY_CODE_KP_ENTER] = 0x9c,
>      [Q_KEY_CODE_KP_DECIMAL] = 0x53,
>      [Q_KEY_CODE_SYSRQ] = 0x54,
> +    [Q_KEY_CODE_PAUSE] = 0xc6,

This part: Acked-by: Hervé Poussineau <hpoussin@reactos.org>

>
>      [Q_KEY_CODE_KP_0] = 0x52,
>      [Q_KEY_CODE_KP_1] = 0x4f,
>

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

* Re: [Qemu-devel] [PATCH for 2.10] ps2: fix sending of PAUSE/BREAK scancodes
  2017-07-24 19:55 ` Hervé Poussineau
@ 2017-07-25  8:32   ` Daniel P. Berrange
  2017-07-25 11:53     ` Gerd Hoffmann
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel P. Berrange @ 2017-07-25  8:32 UTC (permalink / raw)
  To: Hervé Poussineau; +Cc: qemu-devel, Gerd Hoffmann

On Mon, Jul 24, 2017 at 09:55:20PM +0200, Hervé Poussineau wrote:
> Le 24/07/2017 à 18:46, Daniel P. Berrange a écrit :
> > The processing of the scancodes for PAUSE/BREAK  has been broken since
> > the conversion to qcodes 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
> > 
> > When using a VNC client, with the raw scancode extension, the client
> > will send a scancode of 0xc6 for both PAUSE and BREAK. There is mistakenly
> > no entry in the qcode_to_number table for this scancode, so
> > ps2_keyboard_event() just generates a log message and discards the
> > scancode
> > 
> > When using a SPICE client, it will also send 0xc6 for BREAK, but
> > will send 0xe1 0x1d 0x45 0xe1 0x9d 0xc5 for PAUSE. There is no
> > entry in the qcode_to_number table for the scancode 0xe1 because
> > it is a special XT keyboard prefix not mapping to any QKeyCode.
> > Again ps2_keyboard_event() just generates a log message and discards
> > the scancode. The following 0x1d, 0x45, 0x9d, 0xc5 scancodes get
> > handled correctly. Fixing this just requires special casing 0xe1 so
> > it is directly queued for sending to the guest, skipping any conversion
> > to QKeyCode.
> 
> Your commit message is divided in 2 parts, and you're touching 1 file for each part of the commit message.
> Should it be 2 patches instead?
> 
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >  hw/input/ps2.c    | 7 +++++++
> >  ui/input-keymap.c | 1 +
> >  2 files changed, 8 insertions(+)
> > 
> > diff --git a/hw/input/ps2.c b/hw/input/ps2.c
> > index 3ba05efd06..a132d1ba72 100644
> > --- a/hw/input/ps2.c
> > +++ b/hw/input/ps2.c
> > @@ -607,6 +607,13 @@ static void ps2_keyboard_event(DeviceState *dev, QemuConsole *src,
> >      assert(evt->type == INPUT_EVENT_KIND_KEY);
> >      qcode = qemu_input_key_value_to_qcode(key->key);
> > 
> > +    if (qcode == 0 &&
> > +        key->key->type == KEY_VALUE_KIND_NUMBER &&
> > +        key->key->u.number.data == 0x61) {
> > +        ps2_put_keycode(s, 0xe1);
> > +        return;
> > +    }
> > +
> 
> You're putting some specific code for spice in ps2 emulation.
> IMO, the workaround should be moved to spice keyboard handling (ui/spice-input.c),
> which needs to generate a qcode instead of a scancode.

This isn't really a spice specific hack. QEMU internal code is *not* required
to use qcodes - the KeyValue struct is a union that allows use of either qcodes
or XT scancodes, and the latter is what all the frontends (SPICE, VNC, GTk, SDL)
use. QCodes are really only input by the monitor (the sendkey command).

> Here, you're making ps2 emulation work again with spice, but you're missing all
> other emulations using qcodes. Do you want to also modify them?

I checked the USB keyboard device and that works fine. AFAIK, the PS/2
code is the only code that takes XT scancodes, converts to qcode and
then converts qcodes back to scancodes - most others avoid this kind of
roundtripping via qcodes.

> I understand that it may be the easiest way to fix the regression for 2.10, but
> it needs at least a comment.


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 :|

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

* Re: [Qemu-devel] [PATCH for 2.10] ps2: fix sending of PAUSE/BREAK scancodes
  2017-07-25  8:32   ` Daniel P. Berrange
@ 2017-07-25 11:53     ` Gerd Hoffmann
  2017-07-26 11:28       ` Daniel P. Berrange
  0 siblings, 1 reply; 10+ messages in thread
From: Gerd Hoffmann @ 2017-07-25 11:53 UTC (permalink / raw)
  To: Daniel P. Berrange, Hervé Poussineau; +Cc: qemu-devel

  Hi,

> > You're putting some specific code for spice in ps2 emulation.
> > IMO, the workaround should be moved to spice keyboard handling
> > (ui/spice-input.c),
> > which needs to generate a qcode instead of a scancode.
> 
> This isn't really a spice specific hack. QEMU internal code is *not*
> required
> to use qcodes 

qcodes are prefered in new code though.

> - the KeyValue struct is a union that allows use of either qcodes
> or XT scancodes, and the latter is what all the frontends (SPICE,
> VNC, GTk, SDL)
> use. QCodes are really only input by the monitor (the sendkey
> command).

Well, PAUSE is actually sent as qcode by sdl and gtk.  This avoids
special cases in the input layer (PAUSE is the only three scancodes key
sequence).  IMO spice should do the same.  I want switch UIs to qcodes
anyway.

cheers,
  Gerd

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

* Re: [Qemu-devel] [PATCH for 2.10] ps2: fix sending of PAUSE/BREAK scancodes
  2017-07-25 11:53     ` Gerd Hoffmann
@ 2017-07-26 11:28       ` Daniel P. Berrange
  2017-07-26 11:44         ` Gerd Hoffmann
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel P. Berrange @ 2017-07-26 11:28 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: Hervé Poussineau, qemu-devel

On Tue, Jul 25, 2017 at 01:53:40PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > > You're putting some specific code for spice in ps2 emulation.
> > > IMO, the workaround should be moved to spice keyboard handling
> > > (ui/spice-input.c),
> > > which needs to generate a qcode instead of a scancode.
> > 
> > This isn't really a spice specific hack. QEMU internal code is *not*
> > required
> > to use qcodes 
> 
> qcodes are prefered in new code though.
> 
> > - the KeyValue struct is a union that allows use of either qcodes
> > or XT scancodes, and the latter is what all the frontends (SPICE,
> > VNC, GTk, SDL)
> > use. QCodes are really only input by the monitor (the sendkey
> > command).
> 
> Well, PAUSE is actually sent as qcode by sdl and gtk.  This avoids
> special cases in the input layer (PAUSE is the only three scancodes key
> sequence).  IMO spice should do the same.  I want switch UIs to qcodes
> anyway.

qcodes as currently defined cover only a subset of the AT set1 scancodes,
so we need to define countless more qcodes before we consider converting
UIs to use qcodes.

Aside from the pause/break bug, the changes to ps2 driver to round trip
via qcodes have now made it impossible to send a large number of key
sequences to the guest OS :-( Admittedly the missing key codes are not
so commonly used, but it is still a notable regression in functionality
today

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 :|

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

* Re: [Qemu-devel] [PATCH for 2.10] ps2: fix sending of PAUSE/BREAK scancodes
  2017-07-26 11:28       ` Daniel P. Berrange
@ 2017-07-26 11:44         ` Gerd Hoffmann
  2017-07-26 12:05           ` Daniel P. Berrange
  0 siblings, 1 reply; 10+ messages in thread
From: Gerd Hoffmann @ 2017-07-26 11:44 UTC (permalink / raw)
  To: Daniel P. Berrange; +Cc: Hervé Poussineau, qemu-devel

  Hi,

> qcodes as currently defined cover only a subset of the AT set1
> scancodes,
> so we need to define countless more qcodes before we consider
> converting
> UIs to use qcodes.
> 
> Aside from the pause/break bug, the changes to ps2 driver to round
> trip
> via qcodes have now made it impossible to send a large number of key
> sequences to the guest OS :-( Admittedly the missing key codes are
> not
> so commonly used, but it is still a notable regression in
> functionality
> today

My keymap branch carries fixes for that now:
https://www.kraxel.org/cgit/qemu/log/?h=work/xkbcommon

For your keycodemapdb patches I'd suggest to cherry-pick at least the
"ui: move qemu_input_linux_to_qcode()" patch.

Then have sdl + gtk generate linux evdev codes using keycodemapdb, map
that to qcodes using qemu_input_linux_to_qcode(), submit qcodes to the
qemu input layer.

cheers,
  Gerd

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

* Re: [Qemu-devel] [PATCH for 2.10] ps2: fix sending of PAUSE/BREAK scancodes
  2017-07-26 11:44         ` Gerd Hoffmann
@ 2017-07-26 12:05           ` Daniel P. Berrange
  2017-07-26 12:33             ` Gerd Hoffmann
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel P. Berrange @ 2017-07-26 12:05 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: Hervé Poussineau, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1795 bytes --]

On Wed, Jul 26, 2017 at 01:44:07PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > qcodes as currently defined cover only a subset of the AT set1
> > scancodes,
> > so we need to define countless more qcodes before we consider
> > converting
> > UIs to use qcodes.
> > 
> > Aside from the pause/break bug, the changes to ps2 driver to round
> > trip
> > via qcodes have now made it impossible to send a large number of key
> > sequences to the guest OS :-( Admittedly the missing key codes are
> > not
> > so commonly used, but it is still a notable regression in
> > functionality
> > today
> 
> My keymap branch carries fixes for that now:
> https://www.kraxel.org/cgit/qemu/log/?h=work/xkbcommon
> 
> For your keycodemapdb patches I'd suggest to cherry-pick at least the
> "ui: move qemu_input_linux_to_qcode()" patch.

FYI, after adding qcodes to the keycodemapdb, (but not having picked
your branch), I see 131 keys where there is a known AT set 1 scancode,
but no corresponding QKeyCode.

I'll happily add the rest of them if you want (attaching the list)

> Then have sdl + gtk generate linux evdev codes using keycodemapdb, map
> that to qcodes using qemu_input_linux_to_qcode(), submit qcodes to the
> qemu input layer.

I was actually just going add an entry for QKeyCode names to the
keycodemapdb, so we can convert straight to that, and also let
us auto-generate the qemu_input_linux_to_qcode() table.

Strangely there is no Linux key code defined for AltGr, so if we
map via Linux key codes, we would loose ability to send AltGr

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 :|

[-- Attachment #2: missing.txt --]
[-- Type: text/plain, Size: 8232 bytes --]

      Linux Key Name    AT Set 1 scancode
                                     0x54                     
  KEY_ZENKAKUHANKAKU                 0x76                     
        KEY_KATAKANA                 0x78                     
KEY_KATAKANAHIRAGANA                 0x70                     
        KEY_MUHENKAN                 0x7b                     
       KEY_KPJPCOMMA                 0x5c                     
        KEY_LINEFEED                 0x5b                     
           KEY_MACRO               0xe06f                     
            KEY_MUTE               0xe020                     
      KEY_VOLUMEDOWN               0xe02e                     
        KEY_VOLUMEUP               0xe030                     
           KEY_POWER               0xe05e                     
         KEY_KPEQUAL                 0x59                     
     KEY_KPPLUSMINUS               0xe04e                     
           KEY_PAUSE               0xe046                     
           KEY_SCALE               0xe00b                     
           KEY_HANJA               0xe00d                     
         KEY_COMPOSE               0xe05d                     
            KEY_STOP               0xe068                     
           KEY_AGAIN               0xe005                     
           KEY_PROPS               0xe006                     
            KEY_UNDO               0xe007                     
           KEY_FRONT               0xe00c                     
            KEY_COPY               0xe078                     
            KEY_OPEN                 0x64                     
           KEY_PASTE                 0x65                     
            KEY_FIND               0xe041                     
             KEY_CUT               0xe03c                     
            KEY_HELP               0xe075                     
            KEY_MENU               0xe01e                     
            KEY_CALC               0xe021                     
           KEY_SETUP                 0x66                     
           KEY_SLEEP               0xe05f                     
          KEY_WAKEUP               0xe063                     
            KEY_FILE                 0x67                     
        KEY_SENDFILE                 0x68                     
      KEY_DELETEFILE                 0x69                     
            KEY_XFER               0xe013                     
           KEY_PROG1               0xe01f                     
           KEY_PROG2               0xe017                     
             KEY_WWW               0xe002                     
           KEY_MSDOS                 0x6a                     
      KEY_SCREENLOCK               0xe012                     
       KEY_DIRECTION                 0x6b                     
    KEY_CYCLEWINDOWS               0xe026                     
            KEY_MAIL               0xe06c                     
       KEY_BOOKMARKS               0xe066                     
        KEY_COMPUTER               0xe06b                     
            KEY_BACK               0xe06a                     
         KEY_FORWARD               0xe069                     
         KEY_CLOSECD               0xe023                     
         KEY_EJECTCD                 0x6c                     
    KEY_EJECTCLOSECD               0xe07d                     
        KEY_NEXTSONG               0xe019                     
       KEY_PLAYPAUSE               0xe022                     
    KEY_PREVIOUSSONG               0xe010                     
          KEY_STOPCD               0xe024                     
          KEY_RECORD               0xe031                     
          KEY_REWIND               0xe018                     
           KEY_PHONE                 0x63                     
             KEY_ISO                 0x70                     
          KEY_CONFIG               0xe001                     
        KEY_HOMEPAGE               0xe032                     
         KEY_REFRESH               0xe067                     
            KEY_EXIT                 0x71                     
            KEY_MOVE                 0x72                     
            KEY_EDIT               0xe008                     
        KEY_SCROLLUP                 0x75                     
      KEY_SCROLLDOWN               0xe00f                     
     KEY_KPLEFTPAREN               0xe076                     
    KEY_KPRIGHTPAREN               0xe07b                     
             KEY_NEW               0xe009                     
            KEY_REDO               0xe00a                     
             KEY_F13                 0x5d                     
             KEY_F14                 0x5e                     
             KEY_F15                 0x5f                     
             KEY_F16                 0x55                     
             KEY_F17               0xe003                     
             KEY_F18               0xe077                     
             KEY_F19               0xe004                     
             KEY_F20                 0x5a                     
             KEY_F21                 0x74                     
             KEY_F22               0xe079                     
             KEY_F23                 0x6d                     
             KEY_F24                 0x6f                     
                                   0xe015                     
                                   0xe016                     
                                   0xe01a                     
                                   0xe01b                     
                                   0xe027                     
          KEY_PLAYCD               0xe028                     
         KEY_PAUSECD               0xe029                     
           KEY_PROG3               0xe02b                     
           KEY_PROG4               0xe02c                     
       KEY_DASHBOARD               0xe02d                     
         KEY_SUSPEND               0xe025                     
           KEY_CLOSE               0xe02f                     
            KEY_PLAY               0xe033                     
     KEY_FASTFORWARD               0xe034                     
       KEY_BASSBOOST               0xe036                     
           KEY_PRINT               0xe039                     
              KEY_HP               0xe03a                     
          KEY_CAMERA               0xe03b                     
           KEY_SOUND               0xe03d                     
        KEY_QUESTION               0xe03e                     
           KEY_EMAIL               0xe03f                     
            KEY_CHAT               0xe040                     
          KEY_SEARCH               0xe065                     
         KEY_CONNECT               0xe042                     
         KEY_FINANCE               0xe043                     
           KEY_SPORT               0xe044                     
            KEY_SHOP               0xe045                     
        KEY_ALTERASE               0xe014                     
          KEY_CANCEL               0xe04a                     
  KEY_BRIGHTNESSDOWN               0xe04c                     
    KEY_BRIGHTNESSUP               0xe054                     
           KEY_MEDIA               0xe06d                     
 KEY_SWITCHVIDEOMODE               0xe056                     
  KEY_KBDILLUMTOGGLE               0xe057                     
    KEY_KBDILLUMDOWN               0xe058                     
      KEY_KBDILLUMUP               0xe059                     
            KEY_SEND               0xe05a                     
           KEY_REPLY               0xe064                     
     KEY_FORWARDMAIL               0xe00e                     
            KEY_SAVE               0xe055                     
       KEY_DOCUMENTS               0xe070                     
         KEY_BATTERY               0xe071                     
       KEY_BLUETOOTH               0xe072                     
            KEY_WLAN               0xe073                     
             KEY_UWB               0xe074                     

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

* Re: [Qemu-devel] [PATCH for 2.10] ps2: fix sending of PAUSE/BREAK scancodes
  2017-07-26 12:05           ` Daniel P. Berrange
@ 2017-07-26 12:33             ` Gerd Hoffmann
  2017-07-26 12:45               ` Daniel P. Berrange
  0 siblings, 1 reply; 10+ messages in thread
From: Gerd Hoffmann @ 2017-07-26 12:33 UTC (permalink / raw)
  To: Daniel P. Berrange; +Cc: Hervé Poussineau, qemu-devel

  Hi,

> FYI, after adding qcodes to the keycodemapdb, (but not having picked
> your branch),

I don't think this is a good idea.  qkeycode numbers are a qemu-private 
thing, they can (and did in the past) change.  The names are api and
must not change.

> I was actually just going add an entry for QKeyCode names to the
> keycodemapdb, so we can convert straight to that, and also let
> us auto-generate the qemu_input_linux_to_qcode() table.

Ok, with the names it should work.

> Strangely there is no Linux key code defined for AltGr, so if we
> map via Linux key codes, we would loose ability to send AltGr

KEY_RIGHTALT?

cheers,
  Gerd

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

* Re: [Qemu-devel] [PATCH for 2.10] ps2: fix sending of PAUSE/BREAK scancodes
  2017-07-26 12:33             ` Gerd Hoffmann
@ 2017-07-26 12:45               ` Daniel P. Berrange
  2017-07-27  9:50                 ` Daniel P. Berrange
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel P. Berrange @ 2017-07-26 12:45 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: Hervé Poussineau, qemu-devel

On Wed, Jul 26, 2017 at 02:33:10PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > FYI, after adding qcodes to the keycodemapdb, (but not having picked
> > your branch),
> 
> I don't think this is a good idea.  qkeycode numbers are a qemu-private 
> thing, they can (and did in the past) change.  The names are api and
> must not change.

Sorry, yes, I meant only adding the names  - libvirt needs to know the
qcode names and what they map to in other key maps, to use the send-key
command. 

> > I was actually just going add an entry for QKeyCode names to the
> > keycodemapdb, so we can convert straight to that, and also let
> > us auto-generate the qemu_input_linux_to_qcode() table.
> 
> Ok, with the names it should work.
> 
> > Strangely there is no Linux key code defined for AltGr, so if we
> > map via Linux key codes, we would loose ability to send AltGr
> 
> KEY_RIGHTALT?

It seems KEY_RIGHTALT is different from AltGr - it maps to AT set 1
extended scancode 0xe0 0x38, where as AltGr generates 0x64 on my
keyboard


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 :|

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

* Re: [Qemu-devel] [PATCH for 2.10] ps2: fix sending of PAUSE/BREAK scancodes
  2017-07-26 12:45               ` Daniel P. Berrange
@ 2017-07-27  9:50                 ` Daniel P. Berrange
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel P. Berrange @ 2017-07-27  9:50 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: Hervé Poussineau, qemu-devel

On Wed, Jul 26, 2017 at 01:45:51PM +0100, Daniel P. Berrange wrote:
> On Wed, Jul 26, 2017 at 02:33:10PM +0200, Gerd Hoffmann wrote:
> >   Hi,
> > 
> > > FYI, after adding qcodes to the keycodemapdb, (but not having picked
> > > your branch),
> > 
> > I don't think this is a good idea.  qkeycode numbers are a qemu-private 
> > thing, they can (and did in the past) change.  The names are api and
> > must not change.
> 
> Sorry, yes, I meant only adding the names  - libvirt needs to know the
> qcode names and what they map to in other key maps, to use the send-key
> command. 
> 
> > > I was actually just going add an entry for QKeyCode names to the
> > > keycodemapdb, so we can convert straight to that, and also let
> > > us auto-generate the qemu_input_linux_to_qcode() table.
> > 
> > Ok, with the names it should work.
> > 
> > > Strangely there is no Linux key code defined for AltGr, so if we
> > > map via Linux key codes, we would loose ability to send AltGr
> > 
> > KEY_RIGHTALT?
> 
> It seems KEY_RIGHTALT is different from AltGr - it maps to AT set 1
> extended scancode 0xe0 0x38, where as AltGr generates 0x64 on my
> keyboard

Opps, no I was mis-interpreting the evdev output.

Physical key AltGr *does* produce  KEY_RIGHTALT, Linux keycode 100,
AT set1 scan code 0xe0 0x38, QEMU qcode alt_r, QEMU scancode 0xb8


What is confusing is that QEMU defines qcodes for alt, alt_r *and*
altgr and altgr_r.


QEMU's  altgr scancode is 0x64 (qcode_to_number). The only Linux
keycode that produces that AT set1 scancode is KEY_OPEN. There is
no mapping in qcode_to_keycode_set1 for this, which is odd given
that the set1 mapping should be the same as qcode_to_number mapping
except different high bit handling. There is a mapping in
qcode_to_keycode_set2 which corresponds to 0x08 and afaik that
should not exist as there's no set2 scancode 0x08


QEMU's  altgr_r scan code is 0xe4 (qcode_to_number), which is equiv
to at set1 0xe0 0x64, and that should not exist afaik. There is
no mapping in qcode_to_keycode_set1, which is again odd given that
this should be the same as qcode_to_number.  There is a mapping
in qcode_to_keycode_set2 which corresponds to 0xe0 0x08 and afaik
that's not defined for set2 scancode.


So afaict, the altgr & altgr_r  qcodes are just broken and should
not even exist.

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 :|

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

end of thread, other threads:[~2017-07-27  9:50 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-24 16:46 [Qemu-devel] [PATCH for 2.10] ps2: fix sending of PAUSE/BREAK scancodes Daniel P. Berrange
2017-07-24 19:55 ` Hervé Poussineau
2017-07-25  8:32   ` Daniel P. Berrange
2017-07-25 11:53     ` Gerd Hoffmann
2017-07-26 11:28       ` Daniel P. Berrange
2017-07-26 11:44         ` Gerd Hoffmann
2017-07-26 12:05           ` Daniel P. Berrange
2017-07-26 12:33             ` Gerd Hoffmann
2017-07-26 12:45               ` Daniel P. Berrange
2017-07-27  9:50                 ` Daniel P. Berrange

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).