Re: Xen Virtual Keyboard modalias breaking uevents

Re: Xen Virtual Keyboard modalias breaking uevents

[Phillip Susi]

It appears that input/input.c is responsible for the insane modalias
length.  If I am reading input_print_modalias() correctly, it appends a
"k" plus every key code that the keyboard supports, and the Xen Virtual
Keyboard supports a lot of keycodes.  Why does it do this?

Phillip Susi writes:

> So I have finally drilled down to the modalias for the Xen Virtual
> Keyboard driver being so long ( over 2KB ) that it causes an -ENOMEM
> when trying to add it to the environment for uevents.  This causes
> coldplug to fail, which causes the script doing coldplug as part of the
> debian-installer init to fail, which causes a kernel panic when init
> exits, which then for reasons I have yet to understand, causes the Xen
> domU to reboot.
>
> Why is this modalias so huge?  Can we pare it down, or or is there
> another solution to get uevents working on this device again?  Maybe the
> environment block size needs to be increased?  I don't know.

Re: Xen Virtual Keyboard modalias breaking uevents

[Dmitry Torokhov]

On Thu, Apr 29, 2021 at 04:10:09PM -0400, Phillip Susi wrote:
> 
> It appears that input/input.c is responsible for the insane modalias
> length.  If I am reading input_print_modalias() correctly, it appends a
> "k" plus every key code that the keyboard supports,

Not every keyboard, but all keycodes above KEY_MIN_INTERESTING which is
KEY_MUTE, so that interested handlers could match on devices they are
interested in without first opening them or poking through sysfs.

> and the Xen Virtual
> Keyboard supports a lot of keycodes.  Why does it do this?

I don't know why Xen keyboard exports that many keycodes ;) In general,
my recommendation is to mirror the physical device when possible, and
instantiate several devices so there is 1:1 relationship between virtual
and physical devices.

In cases where it is not feasible, we need to be more careful about
declaring capabilities. For xen-kbdfront I do not think the keyboard
portion should be declaring keys from the various BTN_* ranges.


>  
> Phillip Susi writes:
> 
> > So I have finally drilled down to the modalias for the Xen Virtual
> > Keyboard driver being so long ( over 2KB ) that it causes an -ENOMEM
> > when trying to add it to the environment for uevents.  This causes
> > coldplug to fail, which causes the script doing coldplug as part of the
> > debian-installer init to fail, which causes a kernel panic when init
> > exits, which then for reasons I have yet to understand, causes the Xen
> > domU to reboot.
> >
> > Why is this modalias so huge?  Can we pare it down, or or is there
> > another solution to get uevents working on this device again?  Maybe the
> > environment block size needs to be increased?  I don't know.
> 

Thanks.

-- 
Dmitry

Re: Xen Virtual Keyboard modalias breaking uevents

[Phillip Susi]

Dmitry Torokhov writes:

> Not every keyboard, but all keycodes above KEY_MIN_INTERESTING which is
> KEY_MUTE, so that interested handlers could match on devices they are
> interested in without first opening them or poking through sysfs.

/Shouldn't/ they be reading sysfs attributes to find that information
out though?  Isn't modalias there to help modprobe find the right module
that wants to bind to this device, which doesn't happen for input
devices?  If user space is looking at this information then isn't it
getting it by reading from sysfs anyway?

What in user space looks at input devices other than X and Wayland?  And
those aren't looking for particular "interesting" keys are they?

> I don't know why Xen keyboard exports that many keycodes ;) In general,
> my recommendation is to mirror the physical device when possible, and
> instantiate several devices so there is 1:1 relationship between virtual
> and physical devices.

Xen guys: any input as to why it supports so many "interesting" keys?

Re: Xen Virtual Keyboard modalias breaking uevents

[Dmitry Torokhov]

On Thu, Apr 29, 2021 at 08:11:03PM -0400, Phillip Susi wrote:
> 
> Dmitry Torokhov writes:
> 
> > Not every keyboard, but all keycodes above KEY_MIN_INTERESTING which is
> > KEY_MUTE, so that interested handlers could match on devices they are
> > interested in without first opening them or poking through sysfs.
> 
> /Shouldn't/ they be reading sysfs attributes to find that information
> out though?  Isn't modalias there to help modprobe find the right module
> that wants to bind to this device, which doesn't happen for input
> devices?  If user space is looking at this information then isn't it
> getting it by reading from sysfs anyway?

Userspace may or may not have access to sysfs (it does not have to be
mounted) and one can have modules (input handlers) that want to bind to
a specific device (see joydev, mousedev as examples, although they are
not looking for specific keys).

> 
> What in user space looks at input devices other than X and Wayland?  And
> those aren't looking for particular "interesting" keys are they?
> 
> > I don't know why Xen keyboard exports that many keycodes ;) In general,
> > my recommendation is to mirror the physical device when possible, and
> > instantiate several devices so there is 1:1 relationship between virtual
> > and physical devices.
> 
> Xen guys: any input as to why it supports so many "interesting" keys?
> 

Thanks.

-- 
Dmitry

Re: Xen Virtual Keyboard modalias breaking uevents

[Phillip Susi]

Dmitry Torokhov writes:

> Userspace may or may not have access to sysfs (it does not have to be
> mounted)

How would userspace even enumerate the input devices and read their
modalias without sysfs?

Re: Xen Virtual Keyboard modalias breaking uevents

[Phillip Susi]

Dmitry Torokhov writes:

> I don't know why Xen keyboard exports that many keycodes ;) In general,
> my recommendation is to mirror the physical device when possible, and
> instantiate several devices so there is 1:1 relationship between virtual
> and physical devices.

I'm still trying to wrap my head around why keys are even declared in
the first place.  PS/2 ports have no idea what keys are on the keyboard
plugged into them, so I guess they don't declare any?  And that doesn't
stop them from emitting any of the scan codes, so what is the use in
declaring them in the first place?

A lot of "interesting" buttons don't seem very interesting to me, such
as left and right parenthesis.  Is a user space mail program really
going to bypass X11/wayland and open input devices directly to look for
someone to press the "send mail" key?  Even if it did, why would it only
want to open a keyboard that advertises that it has such a key instead
of listening to all keyboards?  Even if all USB keyboards report all of
their special keys, the fact that you could still have a PS/2 keyboard
that has a "send mail" key on it means that the reporting function can
not be relied on and so you just have to listen on all keyboards anyhow.

I guess as long as not reporting keys doesn't stop you from using them,
then the Xen Virtual Keyboard driver should just report none, like the
PS/2 keyboard driver.

[PATCH] Xen Keyboard: don't advertise every key known to man

[Phillip Susi]

For reasons I still don't understand, the input subsystem allows
input devices to advertise what keys they have, and adds this
information to the modalias for the device.  The Xen Virtual
Keyboard was advertising every known key, which resulted in a
modalias string over 2 KiB in length, which caused uevents to
fail with -ENOMEM ( when trying to add the modalias to the env ).
Remove this advertisement.
---
 drivers/input/misc/xen-kbdfront.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c
index 4ff5cd2a6d8d..d4bd558afda9 100644
--- a/drivers/input/misc/xen-kbdfront.c
+++ b/drivers/input/misc/xen-kbdfront.c
@@ -254,11 +254,6 @@ static int xenkbd_probe(struct xenbus_device *dev,
 		kbd->id.product = 0xffff;
 
 		__set_bit(EV_KEY, kbd->evbit);
-		for (i = KEY_ESC; i < KEY_UNKNOWN; i++)
-			__set_bit(i, kbd->keybit);
-		for (i = KEY_OK; i < KEY_MAX; i++)
-			__set_bit(i, kbd->keybit);
-
 		ret = input_register_device(kbd);
 		if (ret) {
 			input_free_device(kbd);
-- 
2.30.2

Re: [PATCH] Xen Keyboard: don't advertise every key known to man

[Dmitry Torokhov]

On Thu, May 06, 2021 at 02:36:54PM +0000, Phillip Susi wrote:
> For reasons I still don't understand, the input subsystem allows
> input devices to advertise what keys they have, and adds this
> information to the modalias for the device.  The Xen Virtual
> Keyboard was advertising every known key, which resulted in a
> modalias string over 2 KiB in length, which caused uevents to
> fail with -ENOMEM ( when trying to add the modalias to the env ).
> Remove this advertisement.
> ---
>  drivers/input/misc/xen-kbdfront.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c
> index 4ff5cd2a6d8d..d4bd558afda9 100644
> --- a/drivers/input/misc/xen-kbdfront.c
> +++ b/drivers/input/misc/xen-kbdfront.c
> @@ -254,11 +254,6 @@ static int xenkbd_probe(struct xenbus_device *dev,
>  		kbd->id.product = 0xffff;
>  
>  		__set_bit(EV_KEY, kbd->evbit);
> -		for (i = KEY_ESC; i < KEY_UNKNOWN; i++)
> -			__set_bit(i, kbd->keybit);
> -		for (i = KEY_OK; i < KEY_MAX; i++)
> -			__set_bit(i, kbd->keybit);
> -

By doing this you are stopping delivery of all key events from this
device.

Thanks.

-- 
Dmitry

Re: [PATCH] Xen Keyboard: don't advertise every key known to man

[Phillip Susi]

Dmitry Torokhov writes:

> By doing this you are stopping delivery of all key events from this
> device.

It does?  How does the PS/2 keyboard driver work then?  It has no way of
knowning what keys the keyboard has other than waiting to see what scan
codes are emitted.

If the keys must be advertised in order to emit them at runtime, then I
see no other possible fix than to remove the codes from the modalias
string in the input subsystem.  Or maybe allow certain drivers that
don't know to set some sort of flag that allows them to emit all codes
at runtime, but NOT advertise them so you end up with a mile long
modalias.

Re: [PATCH] Xen Keyboard: don't advertise every key known to man

[Phillip Susi]

Phillip Susi writes:

> Dmitry Torokhov writes:
>
>> By doing this you are stopping delivery of all key events from this
>> device.

Hrm... I don't have very many "interesting" keys to test, but when I hit
the menu key, I see KEY_COMPOSE, which is > KEY_MIN_INTERESTING.  When I
press the button to have my vnc client send a windows key, I see
KEY_LEFTCTRL+KEY_ESC.  I also see KEY_PAUSE when I hit that key and it
is also "interesting".  I get the same thing with or without this patch,
so it does not appear to be breaking delivery of the keys that are no
longer being advertised.

Oddly though, libinput list-devices does not even show the Xen Virtual
Keyboard.  It's sysfs path is /sys/class/input/input1, but it also does
not have a device node in /dev/input so I can't even ask libinput to
only monitor that device.

Ok... this is really odd.. it does show the device without this patch,
and not with it.  The input events I was seeing were coming through the
"AT Translated Set 2 keyboard" and no events come though the Xen Virtual
Keyboard ( even without this patch ).  This makes me wonder why we have
this device at all?