Re: [PATCH v4 2/2] Input: xen-kbdfront - allow better run-time configuration

Oleksandr Andrushchenko
2018-05-18T14:33:20Z

Sorry for top posting.

Due to a bug in the test used to verify this patch
I need to make two more changes, please see inline.

Please ignore this patch for now, I will send v5 soon.

Sorry for inconvenience,
Oleksandr

On 05/18/2018 12:59 PM, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>
> It is now only possible to control if multi-touch virtual device
> is created or not (via the corresponding XenStore entries),
> but keyboard and pointer devices are always created.
> In some cases this is not desirable. For example, if virtual
> keyboard device is exposed to Android then the latter won't
> automatically show on-screen keyboard as it expects that a
> physical keyboard device can be used for typing.
>
> Utilize keyboard and pointer device XenStore feature fields to
> configure which virtual devices are created:
> - set "feature-disable-keyboard" to 1 if no keyboard device
> needs to be created
> - set "feature-disable-pointer" to 1 if no pointer device
> needs to be created
> Keep old behavior by default.
>
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> Suggested-by: Andrii Chepurnyi <andrii_chepurnyi@epam.com>
> Tested-by: Andrii Chepurnyi <andrii_chepurnyi@epam.com>
> ---
> Changes since v3:
> - do not connect backend if all virtual devices are disabled (Dmitry)
>
> Changes since v2:
> - based on XenStore kbdif features to control which devices are
> exposed instead of module parameters.
>
> drivers/input/misc/xen-kbdfront.c | 177 ++++++++++++++++++------------
> 1 file changed, 106 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c
> index d91f3b1c5375..0e8926c75559 100644
> --- a/drivers/input/misc/xen-kbdfront.c
> +++ b/drivers/input/misc/xen-kbdfront.c
> @@ -63,6 +63,9 @@ static void xenkbd_disconnect_backend(struct xenkbd_info *);
> static void xenkbd_handle_motion_event(struct xenkbd_info *info,
> struct xenkbd_motion *motion)
> {
> + if (unlikely(!info->ptr))
> + return;
> +
> input_report_rel(info->ptr, REL_X, motion->rel_x);
> input_report_rel(info->ptr, REL_Y, motion->rel_y);
> if (motion->rel_z)
> @@ -73,6 +76,9 @@ static void xenkbd_handle_motion_event(struct xenkbd_info *info,
> static void xenkbd_handle_position_event(struct xenkbd_info *info,
> struct xenkbd_position *pos)
> {
> + if (unlikely(!info->ptr))
> + return;
> +
> input_report_abs(info->ptr, ABS_X, pos->abs_x);
> input_report_abs(info->ptr, ABS_Y, pos->abs_y);
> if (pos->rel_z)
> @@ -97,6 +103,9 @@ static void xenkbd_handle_key_event(struct xenkbd_info *info,
> return;
> }
>
> + if (unlikely(!dev))
> + return;
> +
> input_event(dev, EV_KEY, key->keycode, value);
> input_sync(dev);
> }
> @@ -192,7 +201,7 @@ static int xenkbd_probe(struct xenbus_device *dev,
> const struct xenbus_device_id *id)
> {
> int ret, i;
> - unsigned int abs, touch;
> + bool with_mtouch, with_kbd, with_ptr;
> struct xenkbd_info *info;
> struct input_dev *kbd, *ptr, *mtouch;
>
> @@ -211,93 +220,114 @@ static int xenkbd_probe(struct xenbus_device *dev,
> if (!info->page)
> goto error_nomem;
>
> - /* Set input abs params to match backend screen res */
> - abs = xenbus_read_unsigned(dev->otherend,
> - XENKBD_FIELD_FEAT_ABS_POINTER, 0);
> - ptr_size[KPARAM_X] = xenbus_read_unsigned(dev->otherend,
> - XENKBD_FIELD_WIDTH,
> - ptr_size[KPARAM_X]);
> - ptr_size[KPARAM_Y] = xenbus_read_unsigned(dev->otherend,
> - XENKBD_FIELD_HEIGHT,
> - ptr_size[KPARAM_Y]);
> - if (abs) {
> - ret = xenbus_write(XBT_NIL, dev->nodename,
> - XENKBD_FIELD_REQ_ABS_POINTER, "1");
> - if (ret) {
> - pr_warn("xenkbd: can't request abs-pointer\n");
> - abs = 0;
> - }
> - }
> + /*
> + * The below are reverse logic, e.g. if the feature is set, then
> + * do not expose the corresponding virtual device.
> + */
> + with_kbd = !xenbus_read_unsigned(dev->nodename,
According to the kbdif protocol this must be "dev->otherend"
> + XENKBD_FIELD_FEAT_DSBL_KEYBRD, 0);
> +
> + with_ptr = !xenbus_read_unsigned(dev->nodename,
According to the kbdif protocol this must be "dev->otherend"
> + XENKBD_FIELD_FEAT_DSBL_POINTER, 0);
>
> - touch = xenbus_read_unsigned(dev->nodename,
> - XENKBD_FIELD_FEAT_MTOUCH, 0);
> - if (touch) {
> + /* Direct logic: if set, then create multi-touch device. */
> + with_mtouch = xenbus_read_unsigned(dev->nodename,
According to the kbdif protocol this must be "dev->otherend"
> + XENKBD_FIELD_FEAT_MTOUCH, 0);
> + if (with_mtouch) {
> ret = xenbus_write(XBT_NIL, dev->nodename,
> XENKBD_FIELD_REQ_MTOUCH, "1");
> if (ret) {
> pr_warn("xenkbd: can't request multi-touch");
> - touch = 0;
> + with_mtouch = 0;
> }
> }
>
> /* keyboard */
> - kbd = input_allocate_device();
> - if (!kbd)
> - goto error_nomem;
> - kbd->name = "Xen Virtual Keyboard";
> - kbd->phys = info->phys;
> - kbd->id.bustype = BUS_PCI;
> - kbd->id.vendor = 0x5853;
> - 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);
> - xenbus_dev_fatal(dev, ret, "input_register_device(kbd)");
> - goto error;
> + if (with_kbd) {
> + kbd = input_allocate_device();
> + if (!kbd)
> + goto error_nomem;
> + kbd->name = "Xen Virtual Keyboard";
> + kbd->phys = info->phys;
> + kbd->id.bustype = BUS_PCI;
> + kbd->id.vendor = 0x5853;
> + 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);
> + xenbus_dev_fatal(dev, ret,
> + "input_register_device(kbd)");
> + goto error;
> + }
> + info->kbd = kbd;
> }
> - info->kbd = kbd;
>
> /* pointing device */
> - ptr = input_allocate_device();
> - if (!ptr)
> - goto error_nomem;
> - ptr->name = "Xen Virtual Pointer";
> - ptr->phys = info->phys;
> - ptr->id.bustype = BUS_PCI;
> - ptr->id.vendor = 0x5853;
> - ptr->id.product = 0xfffe;
> -
> - if (abs) {
> - __set_bit(EV_ABS, ptr->evbit);
> - input_set_abs_params(ptr, ABS_X, 0, ptr_size[KPARAM_X], 0, 0);
> - input_set_abs_params(ptr, ABS_Y, 0, ptr_size[KPARAM_Y], 0, 0);
> - } else {
> - input_set_capability(ptr, EV_REL, REL_X);
> - input_set_capability(ptr, EV_REL, REL_Y);
> - }
> - input_set_capability(ptr, EV_REL, REL_WHEEL);
> + if (with_ptr) {
> + unsigned int abs;
> +
> + /* Set input abs params to match backend screen res */
> + abs = xenbus_read_unsigned(dev->otherend,
> + XENKBD_FIELD_FEAT_ABS_POINTER, 0);
> + ptr_size[KPARAM_X] = xenbus_read_unsigned(dev->otherend,
> + XENKBD_FIELD_WIDTH,
> + ptr_size[KPARAM_X]);
> + ptr_size[KPARAM_Y] = xenbus_read_unsigned(dev->otherend,
> + XENKBD_FIELD_HEIGHT,
> + ptr_size[KPARAM_Y]);
> + if (abs) {
> + ret = xenbus_write(XBT_NIL, dev->nodename,
> + XENKBD_FIELD_REQ_ABS_POINTER, "1");
> + if (ret) {
> + pr_warn("xenkbd: can't request abs-pointer\n");
> + abs = 0;
> + }
> + }
>
> - __set_bit(EV_KEY, ptr->evbit);
> - for (i = BTN_LEFT; i <= BTN_TASK; i++)
> - __set_bit(i, ptr->keybit);
> + ptr = input_allocate_device();
> + if (!ptr)
> + goto error_nomem;
> + ptr->name = "Xen Virtual Pointer";
> + ptr->phys = info->phys;
> + ptr->id.bustype = BUS_PCI;
> + ptr->id.vendor = 0x5853;
> + ptr->id.product = 0xfffe;
> +
> + if (abs) {
> + __set_bit(EV_ABS, ptr->evbit);
> + input_set_abs_params(ptr, ABS_X, 0,
> + ptr_size[KPARAM_X], 0, 0);
> + input_set_abs_params(ptr, ABS_Y, 0,
> + ptr_size[KPARAM_Y], 0, 0);
> + } else {
> + input_set_capability(ptr, EV_REL, REL_X);
> + input_set_capability(ptr, EV_REL, REL_Y);
> + }
> + input_set_capability(ptr, EV_REL, REL_WHEEL);
>
> - ret = input_register_device(ptr);
> - if (ret) {
> - input_free_device(ptr);
> - xenbus_dev_fatal(dev, ret, "input_register_device(ptr)");
> - goto error;
> + __set_bit(EV_KEY, ptr->evbit);
> + for (i = BTN_LEFT; i <= BTN_TASK; i++)
> + __set_bit(i, ptr->keybit);
> +
> + ret = input_register_device(ptr);
> + if (ret) {
> + input_free_device(ptr);
> + xenbus_dev_fatal(dev, ret,
> + "input_register_device(ptr)");
> + goto error;
> + }
> + info->ptr = ptr;
> }
> - info->ptr = ptr;
>
> /* multi-touch device */
> - if (touch) {
> + if (with_mtouch) {
> int num_cont, width, height;
>
> mtouch = input_allocate_device();
> @@ -346,6 +376,11 @@ static int xenkbd_probe(struct xenbus_device *dev,
> info->mtouch = mtouch;
> }
>
> + if (!(with_kbd | with_ptr | with_mtouch)) {
> + ret = -ENXIO;
> + goto error;
> + }
> +
> ret = xenkbd_connect_backend(dev, info);
> if (ret < 0)
> goto error;