Re: [Xen-devel] [PATCH] xen/kbdif: Add frontend's virtual device options

[Oleksandr Andrushchenko <andr2000@xxxxxxxxx>]

On 04/27/2018 12:14 AM, Dmitry Torokhov wrote:
> Hi Oleksandr,
>
> On Tue, Apr 24, 2018 at 10:31:38AM +0300, Oleksandr Andrushchenko wrote:
> > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> >
> > It is now not possible to control if and which virtual devices
> > are created by the frontend, e.g. keyboard and pointer devices
> > are always created and multi-touch device is created if the
> > backend advertises multi-touch support. In some cases this
> > behavior is not desirable and better control over the frontend's
> > configuration is required.
> >
> > Add new frontend options, so it is possible to individually
> > control set of exposed virtual devices for each guest OS:
> >   - set opt-no-pointer if no pointer device needs to be created
> >   - set opt-no-keyboard if no keyboard device needs to be created
> >   - set opt-no-multi-touch if no multi-touch device needs to be created
> >
> > Keep old behavior by default.
> >
> > Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> > ---
> >   xen/include/public/io/kbdif.h | 26 ++++++++++++++++++++++++++
> >   1 file changed, 26 insertions(+)
> >
> > diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h
> > index 3ce54e9a44c1..703433156c1f 100644
> > --- a/xen/include/public/io/kbdif.h
> > +++ b/xen/include/public/io/kbdif.h
> > @@ -132,6 +132,29 @@
> >    *      OBSOLETE, not recommended for use.
> >    *      PFN of the shared page.
> >    *
> > + *--------------------------- Frontend Options -------------------------------
> > + *
> > + * For the below options if corresponding XenStore entry omitted,
> > + * then the value must be treated as set to 0.
> > + *
> > + * opt-no-pointer
> > + *      Values:         <uint>
> > + *
> > + *      If no virtual pointer device to be exposed by the frontend then
> > + *      this must be set to 1.
> > + *
> > + * opt-no-keyboard
> > + *      Values:         <uint>
> > + *
> > + *      If no virtual keyboard device to be exposed by the frontend then
> > + *      this must be set to 1.
> > + *
> > + * opt-no-multi-touch
> > + *      Values:         <uint>
> > + *
> > + *      If no virtual multi-touch device to be exposed by the frontend then
> > + *      this must be set to 1.
> > + *
> >    *----------------------- Multi-touch Device Parameters 
> > -----------------------
> >    *
> >    * multi-touch-num-contacts
> > @@ -181,6 +204,9 @@
> >   #define XENKBD_FIELD_FEAT_MTOUCH       "feature-multi-touch"
> >   #define XENKBD_FIELD_REQ_ABS_POINTER   "request-abs-pointer"
> >   #define XENKBD_FIELD_REQ_MTOUCH        "request-multi-touch"
> > +#define XENKBD_FIELD_OPT_NO_PTR        "opt-no-pointer"
> > +#define XENKBD_FIELD_OPT_NO_KBD        "opt-no-keyboard"
> > +#define XENKBD_FIELD_OPT_NO_MTOUCH     "opt-no-multi-touch"
> So we have "feature-multi-touch" and "opt-no-multi-touch". What is the
> difference between feature and option?
Indeed, it can be made simpler.
This is how it looks in XenStore for a given frontend:

/local/domain/2/device/vkbd/0/feature-multi-touch = "1"
/local/domain/2/device/vkbd/0/request-multi-touch = "1"

Thus, feature-multi-touch field already controls if multi-touch
device is created *per frontend*, so the multi-touch control
is already there. (request-multi-touch field is set by the
frontend to request multi-touch events to be sent by the backend,
e.g. this is sort of negotiation that backend offers multi-touch
to frontend and if frontend is capable and willing to support that
it then replies by setting this field).

So, "opt-no-multi-touch" will be removed from the protocol.
> Also, for the pointer and keyboard, if we decide that features and
> options can be merged, can we keep them positive (i.e.
> "feature-keyboard" or "feature-rel-pointer") and mention in the protocol
> spec that they are assumed to be present by default?
As Dmitry suggested at [1]:

"I think you can have them as "feature-pointer" and "feature-keyboard"
(no negation), but assume not present considered enabled. I.e.

    kbd = xenbus_read_unsigned(..., XENKBD_FIELD_FEAT_KEYBOARD, 1);
    if (kbd) {
        ...
    }
"

So, I'll add *per frontend* features:
- feature-keyboard
- feature-pointer (without "rel", we just need to enable the pointer
device which is by default "rel": there is a dedicated feature for
"abs" - feature-abs-pointer).

/local/domain/2/device/vkbd/0/feature-keyboard = "1"
/local/domain/2/device/vkbd/0/feature-pointer = "1"

For backward compatibility the default will be "1" if the field is
not present. opt-no-keyboard and opt-no-pointer will be removed.

I will send v1 with the changes above.
> >   #define XENKBD_FIELD_RING_GREF         "page-gref"
> >   #define XENKBD_FIELD_EVT_CHANNEL       "event-channel"
> >   #define XENKBD_FIELD_WIDTH             "width"
> > --
> > 2.17.0
> Thanks.
Thank you,
Oleksandr

[1] https://lkml.org/lkml/2018/4/26/865

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel