[Qemu-devel] [PULL for-2.4 0/4] input: fixes for 2.4

[Qemu-devel] [PULL for-2.4 0/4] input: fixes for 2.4

[Gerd Hoffmann]

  Hi,

A few input fixes for 2.4.  Also enable virtio-input builds on
non-linux hosts after fixing up the ioctl include.

please pull,
  Gerd

The following changes since commit f3a1b5068cea303a55e2a21a97e66d057eaae638:

  Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2015-07-13 13:35:51 +0100)

are available in the git repository at:


  git://git.kraxel.org/qemu tags/pull-input-20150714-1

for you to fetch changes up to 8121c09e8a52fd47254479d8f5ccbbc20e7bb718:

  hid: clarify hid_keyboard_process_keycode (2015-07-14 13:48:45 +0200)

----------------------------------------------------------------
input: fixes for 2.4

----------------------------------------------------------------
Gerd Hoffmann (2):
      virtio-input: move sys/ioctl.h include
      virtio-input: enable on non-linux hosts

Lin Ma (1):
      virtio-input: fix segfault in virtio_input_hid_properties

Paolo Bonzini (1):
      hid: clarify hid_keyboard_process_keycode

 hw/input/Makefile.objs                 |  2 +-
 hw/input/hid.c                         | 32 ++++++++++++++++++++++++++++----
 hw/input/virtio-input-hid.c            |  1 +
 hw/input/virtio-input-host.c           |  1 +
 include/standard-headers/linux/input.h |  1 -
 scripts/update-linux-headers.sh        |  1 +
 6 files changed, 32 insertions(+), 6 deletions(-)

[Qemu-devel] [PULL 1/4] virtio-input: fix segfault in virtio_input_hid_properties

[Gerd Hoffmann]

From: Lin Ma <lma@suse.com>

commit 5cce173 introduced virtio-input segfault, This patch fixes it.

Signed-off-by: Lin Ma <lma@suse.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/input/virtio-input-hid.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/input/virtio-input-hid.c b/hw/input/virtio-input-hid.c
index 616a815..4d85dad 100644
--- a/hw/input/virtio-input-hid.c
+++ b/hw/input/virtio-input-hid.c
@@ -308,6 +308,7 @@ static void virtio_input_hid_handle_status(VirtIOInput *vinput,
 static Property virtio_input_hid_properties[] = {
     DEFINE_PROP_STRING("display", VirtIOInputHID, display),
     DEFINE_PROP_UINT32("head", VirtIOInputHID, head, 0),
+    DEFINE_PROP_END_OF_LIST(),
 };
 
 static void virtio_input_hid_class_init(ObjectClass *klass, void *data)
-- 
1.8.3.1

[Qemu-devel] [PULL 2/4] virtio-input: move sys/ioctl.h include

[Gerd Hoffmann]

Drop from include/standard-headers/linux/input.h
Add to hw/input/virtio-input-host.c instead.

That allows to build virtio-input (except pass-through) on windows.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/input/virtio-input-host.c           | 1 +
 include/standard-headers/linux/input.h | 1 -
 scripts/update-linux-headers.sh        | 1 +
 3 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/input/virtio-input-host.c b/hw/input/virtio-input-host.c
index f7e3d84..8978f16 100644
--- a/hw/input/virtio-input-host.c
+++ b/hw/input/virtio-input-host.c
@@ -11,6 +11,7 @@
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-input.h"
 
+#include <sys/ioctl.h>
 #include "standard-headers/linux/input.h"
 
 /* ----------------------------------------------------------------- */
diff --git a/include/standard-headers/linux/input.h b/include/standard-headers/linux/input.h
index a459dd2..b003c67 100644
--- a/include/standard-headers/linux/input.h
+++ b/include/standard-headers/linux/input.h
@@ -10,7 +10,6 @@
 
 
 #include <sys/time.h>
-#include <sys/ioctl.h>
 #include <sys/types.h>
 #include "standard-headers/linux/types.h"
 
diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
index 47378d9..f0e830c 100755
--- a/scripts/update-linux-headers.sh
+++ b/scripts/update-linux-headers.sh
@@ -56,6 +56,7 @@ cp_virtio() {
                 -e 's/__bitwise__//' \
                 -e 's/__attribute__((packed))/QEMU_PACKED/' \
                 -e 's/__inline__/inline/' \
+                -e '/sys\/ioctl.h/d' \
                 "$f" > "$to/$header";
         done
     fi
-- 
1.8.3.1

[Qemu-devel] [PULL 3/4] virtio-input: enable on non-linux hosts

[Gerd Hoffmann]

As we include a copy of the linux input layer header file in the qemu
source tree (include/standard-headers/linux/input.h) the virtio-input
bits (except pass-through) should build and work just fine on non-linux
hosts too.  Tweak Makefile accordingly.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-trivial@nongnu.org
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/input/Makefile.objs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/input/Makefile.objs b/hw/input/Makefile.objs
index 624ba7e..7715d72 100644
--- a/hw/input/Makefile.objs
+++ b/hw/input/Makefile.objs
@@ -8,9 +8,9 @@ common-obj-$(CONFIG_STELLARIS_INPUT) += stellaris_input.o
 common-obj-$(CONFIG_TSC2005) += tsc2005.o
 common-obj-$(CONFIG_VMMOUSE) += vmmouse.o
 
-ifeq ($(CONFIG_LINUX),y)
 common-obj-$(CONFIG_VIRTIO) += virtio-input.o
 common-obj-$(CONFIG_VIRTIO) += virtio-input-hid.o
+ifeq ($(CONFIG_LINUX),y)
 common-obj-$(CONFIG_VIRTIO) += virtio-input-host.o
 endif
 
-- 
1.8.3.1

[Qemu-devel] [PULL 4/4] hid: clarify hid_keyboard_process_keycode

[Gerd Hoffmann]

From: Paolo Bonzini <pbonzini@redhat.com>

Coverity thinks the fallthroughs are smelly.  They are correct, but
everything else in this function is like "wut?".

Refer explicitly to bits 8 and 9 of hs->kbd.modifiers instead of
shifting right first and using (1 << 7).  Document what the scancode
is when hid_code is 0xe0.  And add plenty of comments.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/input/hid.c | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/hw/input/hid.c b/hw/input/hid.c
index 6841cb8..21ebd9e 100644
--- a/hw/input/hid.c
+++ b/hw/input/hid.c
@@ -239,7 +239,7 @@ static void hid_keyboard_event(DeviceState *dev, QemuConsole *src,
 
 static void hid_keyboard_process_keycode(HIDState *hs)
 {
-    uint8_t hid_code, key;
+    uint8_t hid_code, index, key;
     int i, keycode, slot;
 
     if (hs->n == 0) {
@@ -249,7 +249,8 @@ static void hid_keyboard_process_keycode(HIDState *hs)
     keycode = hs->kbd.keycodes[slot];
 
     key = keycode & 0x7f;
-    hid_code = hid_usage_keys[key | ((hs->kbd.modifiers >> 1) & (1 << 7))];
+    index = key | ((hs->kbd.modifiers & (1 << 8)) >> 1);
+    hid_code = hid_usage_keys[index];
     hs->kbd.modifiers &= ~(1 << 8);
 
     switch (hid_code) {
@@ -257,18 +258,41 @@ static void hid_keyboard_process_keycode(HIDState *hs)
         return;
 
     case 0xe0:
+        assert(key == 0x1d);
         if (hs->kbd.modifiers & (1 << 9)) {
-            hs->kbd.modifiers ^= 3 << 8;
+            /* The hid_codes for the 0xe1/0x1d scancode sequence are 0xe9/0xe0.
+             * Here we're processing the second hid_code.  By dropping bit 9
+             * and setting bit 8, the scancode after 0x1d will access the
+             * second half of the table.
+             */
+            hs->kbd.modifiers ^= (1 << 8) | (1 << 9);
             return;
         }
+        /* fall through to process Ctrl_L */
     case 0xe1 ... 0xe7:
+        /* Ctrl_L/Ctrl_R, Shift_L/Shift_R, Alt_L/Alt_R, Win_L/Win_R.
+         * Handle releases here, or fall through to process presses.
+         */
         if (keycode & (1 << 7)) {
             hs->kbd.modifiers &= ~(1 << (hid_code & 0x0f));
             return;
         }
-    case 0xe8 ... 0xef:
+        /* fall through */
+    case 0xe8 ... 0xe9:
+        /* USB modifiers are just 1 byte long.  Bits 8 and 9 of
+         * hs->kbd.modifiers implement a state machine that detects the
+         * 0xe0 and 0xe1/0x1d sequences.  These bits do not follow the
+         * usual rules where bit 7 marks released keys; they are cleared
+         * elsewhere in the function as the state machine dictates.
+         */
         hs->kbd.modifiers |= 1 << (hid_code & 0x0f);
         return;
+
+    case 0xea ... 0xef:
+        abort();
+
+    default:
+        break;
     }
 
     if (keycode & (1 << 7)) {
-- 
1.8.3.1

Re: [Qemu-devel] [PULL for-2.4 0/4] input: fixes for 2.4

[Peter Maydell]

On 16 July 2015 at 16:38, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
> A few input fixes for 2.4.  Also enable virtio-input builds on
> non-linux hosts after fixing up the ioctl include.
>
> please pull,
>   Gerd
>
> The following changes since commit f3a1b5068cea303a55e2a21a97e66d057eaae638:
>
>   Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2015-07-13 13:35:51 +0100)
>
> are available in the git repository at:
>
>
>   git://git.kraxel.org/qemu tags/pull-input-20150714-1
>
> for you to fetch changes up to 8121c09e8a52fd47254479d8f5ccbbc20e7bb718:
>
>   hid: clarify hid_keyboard_process_keycode (2015-07-14 13:48:45 +0200)
>
> ----------------------------------------------------------------
> input: fixes for 2.4
>
> ----------------------------------------------------------------

I'm afraid this doesn't build for Windows:

In file included from
/home/petmay01/linaro/qemu-for-merges/hw/input/virtio-input.c:13:
/home/petmay01/linaro/qemu-for-merges/include/standard-headers/linux/input.h:890:1:
error: "SW_MAX" redefined
In file included from
/usr/lib/gcc/i586-mingw32msvc/4.2.1-sjlj/../../../../i586-mingw32msvc/include/windows.h:55,
                 from
/home/petmay01/linaro/qemu-for-merges/include/sysemu/os-win32.h:29,
                 from
/home/petmay01/linaro/qemu-for-merges/include/qemu-common.h:48,
                 from
/home/petmay01/linaro/qemu-for-merges/include/qemu/iov.h:17,
                 from
/home/petmay01/linaro/qemu-for-merges/hw/input/virtio-input.c:7:
/usr/lib/gcc/i586-mingw32msvc/4.2.1-sjlj/../../../../i586-mingw32msvc/include/winuser.h:729:1:
error: this is the location of the previous definition


thanks
-- PMM

Re: [Qemu-devel] [PULL for-2.4 0/4] input: fixes for 2.4

[Gerd Hoffmann]

  Hi,

> I'm afraid this doesn't build for Windows:
> 
> In file included from
> /home/petmay01/linaro/qemu-for-merges/hw/input/virtio-input.c:13:
> /home/petmay01/linaro/qemu-for-merges/include/standard-headers/linux/input.h:890:1:
> error: "SW_MAX" redefined
> In file included from
> /usr/lib/gcc/i586-mingw32msvc/4.2.1-sjlj/../../../../i586-mingw32msvc/include/windows.h:55,
>                  from
> /home/petmay01/linaro/qemu-for-merges/include/sysemu/os-win32.h:29,
>                  from
> /home/petmay01/linaro/qemu-for-merges/include/qemu-common.h:48,
>                  from
> /home/petmay01/linaro/qemu-for-merges/include/qemu/iov.h:17,
>                  from
> /home/petmay01/linaro/qemu-for-merges/hw/input/virtio-input.c:7:
> /usr/lib/gcc/i586-mingw32msvc/4.2.1-sjlj/../../../../i586-mingw32msvc/include/winuser.h:729:1:
> error: this is the location of the previous definition

Lovely.  Looks like a bug in the mingw headers to me, especially as my
machine cross-builds this just fine for both win32 and win64.

We are in hard-freeze though, so no time to experiments here, I'll redo
the pull with patch #3 dropped, lets sort this post-2.4.

cheers,
  Gerd