qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v7 4/4] hw: convert virtio-input-hid device to k


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v7 4/4] hw: convert virtio-input-hid device to keycodemapdb
Date: Wed, 17 Jan 2018 13:02:07 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2

On 01/17/2018 10:41 AM, Daniel P. Berrange wrote:
> Replace the keymap_qcode table with automatically generated
> tables.
> 
> Missing entries in keymap_qcode now fixed:
> 

> 
> When a keycode is removed from the list of possible keycodes that host can
> send to the guest, it means that the guest OS will think it is possible
> to receive a key that in pratice can never be generated, which is harmless.

s/pratice/practice/

> 
> When a keycode is added to the list of possible keycodes that the host can
> send to the guest, it means that the guest OS can see an unexpected event.
> The Linux virtio_input.c driver code simply forwards this event to the
> input_event() method in the Linux input subsystem. This in turn calls
> input_handle_event(), which then calls input_get_disposition(). This method
> checks if the input event is present in the permitted keys bitmap, and if
> not returns INPUT_IGNORE_EVENT. Thus the unexpected event will get dropped,
> which is harmless.
> 
> If the guest OS reboots, or otherwise re-initializes the virt-input device,
> it will read the new keycode bitmap. No matter how many keys are defined,
> the config space has a fixed 128 byte bitmap. There is, however, a size
> field defiend which says how many bytes in the bitmap are used. So the guest

s/defiend/defined/

> OS reads the size of the bitmap, and then it reads the data from bitmap upto

s/upto/up to/

> the designated size. So if the guest OS re-initializes at precisely the time
> that QEMU is migrated across versions, in the worst case, it could conceivably
> read the old size field, but then get the newly updated bitmap.  If a key were
> added this is harmless, since it simply means it may not process the newly
> added key. If a key were removed, then it could be readnig a byte from the

s/readnig/reading/

> bitmap that was not initialized. Fortunately QEMU always memsets() the entire

maybe s/memsets()/memset()s/

> bitmap to 0, prior to setting keybits. Thus the guest OS will simply read
> zeros, which is again harmless.
> 
> Based on this analysis, it is believed that there is no need to preserve the
> virtio-input-hid keymaps across migration, as the host<->guest ABI change is
> harmless and self-resolving at time of guest reboot.
> 
> NB, this behaviour should perhaps be formalized in the virtio-input spec
> to declare how guest OS drivers should be written to be robust in their
> handling of the potentially changable key bitmaps.

Your analysis of the issues, as well as the advice to enhance the
virtio-input spec to document that a guest must not react negatively to
the change, both sound reasonable.

> 
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
>  hw/input/virtio-input-hid.c | 136 
> +++-----------------------------------------
>  1 file changed, 9 insertions(+), 127 deletions(-)

Fun diffstat ratio!

I have not closely reviewed the series, so much as noticing the grammar
on the fly.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]