From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58260) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1e5C50-0002r4-Ol for qemu-devel@nongnu.org; Thu, 19 Oct 2017 10:45:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1e5C4w-0000zB-OK for qemu-devel@nongnu.org; Thu, 19 Oct 2017 10:45:50 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33444) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1e5C4w-0000yn-El for qemu-devel@nongnu.org; Thu, 19 Oct 2017 10:45:46 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 41A51806B3 for ; Thu, 19 Oct 2017 14:45:45 +0000 (UTC) Date: Thu, 19 Oct 2017 15:45:37 +0100 From: "Daniel P. Berrange" Message-ID: <20171019144537.GT8408@redhat.com> Reply-To: "Daniel P. Berrange" References: <20171019142848.572-1-berrange@redhat.com> <20171019142848.572-5-berrange@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20171019142848.572-5-berrange@redhat.com> Subject: Re: [Qemu-devel] [PATCH v1 4/9] ps2: fix scancodes sent for Alt-Print key combination (aka SysRq) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Gerd Hoffmann On Thu, Oct 19, 2017 at 03:28:43PM +0100, Daniel P. Berrange wrote: > 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 > 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 > > 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 > --- > 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; Presumably the addition of this field means I should do something with the vmstate for migration. I'm unclear exactly how to modify that data in the right way that correctly preserves back compatibility ? 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 :|