[PATCH] kbdif.h: Introduce feature-vkbd-standalone

[PATCH] kbdif.h: Introduce feature-vkbd-standalone

[Owen Smith]

Backends set "feature-vkbd-standalone" to 1 if they can connect
without waiting for the PV framebuffer. If this value is missing
or not 1, then a backend will wait for the PV framebuffer before
connecting, potentially causing the frontend to wait indefinitely.

Frontends set "request-vkbd-standalone" to 1 to request that the
backend does not wait for the PV framebuffer. Frontends that
require a standalone vkbd device should not attempt to connect
unless the backend advertises "feature-vkbd-standalone", and
should set "request-vkbd-standalone".

Backends that are standalone (i.e. do not have an associated PV
framebuffer) do not rescale absolute mouse or touch coordinates
to a the size of the (non-existant) PV framebuffer, and use the
range of [0, 0x7fff] for absolute values.

Signed-off-by: Owen Smith <owen.smith@citrix.com>
---
 xen/include/public/io/kbdif.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h
index dcbd71a..ca09080 100644
--- a/xen/include/public/io/kbdif.h
+++ b/xen/include/public/io/kbdif.h
@@ -63,6 +63,12 @@
  *      Backends, which support reporting of multi-touch events
  *      should set this to 1.
  *
+ * feature-vkbd-standalone
+ *      Values:         <uint>
+ *
+ *      Backends, which support a standalone vkbd, without requiring a vfb
+ *      device, should set this to 1.
+ *
  *------------------------- Pointer Device Parameters ------------------------
  *
  * width
@@ -98,6 +104,13 @@
  *
  *      Request backend to report multi-touch events.
  *
+ * request-vkbd-standalone
+ *      Values:         <uint>
+ *
+ *      Request backend to connect vkbd device without waiting for the
+ *      vfb device. Any absolute coordinates will NOT be scaled to
+ *      screen size, and will remain in the range [0, 0x7fff]
+ *
  *----------------------- Request Transport Parameters -----------------------
  *
  * event-channel
@@ -165,8 +178,10 @@
 
 #define XENKBD_FIELD_FEAT_ABS_POINTER  "feature-abs-pointer"
 #define XENKBD_FIELD_FEAT_MTOUCH       "feature-multi-touch"
+#define XENKBD_FIELD_FEAT_STANDALONE   "feature-vkbd-standalone"
 #define XENKBD_FIELD_REQ_ABS_POINTER   "request-abs-pointer"
 #define XENKBD_FIELD_REQ_MTOUCH        "request-multi-touch"
+#define XENKBD_FIELD_REQ_STANDALONE    "request-vkbd-standalone"
 #define XENKBD_FIELD_RING_GREF         "page-gref"
 #define XENKBD_FIELD_EVT_CHANNEL       "event-channel"
 #define XENKBD_FIELD_WIDTH             "width"
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] kbdif.h: Introduce feature-vkbd-standalone

[Oleksandr Andrushchenko]

Hi, Owen!

On 06/08/2017 04:09 PM, Owen Smith wrote:
> Backends set "feature-vkbd-standalone" to 1 if they can connect
> without waiting for the PV framebuffer. If this value is missing
> or not 1, then a backend will wait for the PV framebuffer before
> connecting, potentially causing the frontend to wait indefinitely.
This means that by the new option we *fix an existing
backend* and its particular behavior. What is more,
we introduce knowledge of virtual fb into generic virtual
kbd/ptr protocol which seems to be not correct (IMO).
For the reasons above, I would recommend fixing the
corresponding backend instead, for example by configuring
it appropriately wrt use-case you have.
> Frontends set "request-vkbd-standalone" to 1 to request that the
> backend does not wait for the PV framebuffer. Frontends that
> require a standalone vkbd device should not attempt to connect
> unless the backend advertises "feature-vkbd-standalone", and
> should set "request-vkbd-standalone".
Again, this looks very use-case specific
> Backends that are standalone (i.e. do not have an associated PV
> framebuffer) do not rescale absolute mouse or touch coordinates
> to a the size of the (non-existant) PV framebuffer, and use the
> range of [0, 0x7fff] for absolute values.
>
> Signed-off-by: Owen Smith <owen.smith@citrix.com>
> ---
>   xen/include/public/io/kbdif.h | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h
> index dcbd71a..ca09080 100644
> --- a/xen/include/public/io/kbdif.h
> +++ b/xen/include/public/io/kbdif.h
> @@ -63,6 +63,12 @@
>    *      Backends, which support reporting of multi-touch events
>    *      should set this to 1.
>    *
> + * feature-vkbd-standalone
> + *      Values:         <uint>
> + *
> + *      Backends, which support a standalone vkbd, without requiring a vfb
> + *      device, should set this to 1.
> + *
>    *------------------------- Pointer Device Parameters ------------------------
>    *
>    * width
> @@ -98,6 +104,13 @@
>    *
>    *      Request backend to report multi-touch events.
>    *
> + * request-vkbd-standalone
> + *      Values:         <uint>
> + *
> + *      Request backend to connect vkbd device without waiting for the
> + *      vfb device. Any absolute coordinates will NOT be scaled to
> + *      screen size, and will remain in the range [0, 0x7fff]
> + *
>    *----------------------- Request Transport Parameters -----------------------
>    *
>    * event-channel
> @@ -165,8 +178,10 @@
>   
>   #define XENKBD_FIELD_FEAT_ABS_POINTER  "feature-abs-pointer"
>   #define XENKBD_FIELD_FEAT_MTOUCH       "feature-multi-touch"
> +#define XENKBD_FIELD_FEAT_STANDALONE   "feature-vkbd-standalone"
>   #define XENKBD_FIELD_REQ_ABS_POINTER   "request-abs-pointer"
>   #define XENKBD_FIELD_REQ_MTOUCH        "request-multi-touch"
> +#define XENKBD_FIELD_REQ_STANDALONE    "request-vkbd-standalone"
>   #define XENKBD_FIELD_RING_GREF         "page-gref"
>   #define XENKBD_FIELD_EVT_CHANNEL       "event-channel"
>   #define XENKBD_FIELD_WIDTH             "width"
Thank you,
Oleksandr

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] kbdif.h: Introduce feature-vkbd-standalone

[Owen Smith]

[-- Attachment #1.1: Type: text/plain, Size: 4229 bytes --]

Hi Oleksandr,



The reason I’m proposing the additional feature flag is to differentiate between

older, broken, backends that will not connect without the vfb device

(qemu-upstream’s implementation in hw/display/xenfb.c) and fixed backends

(like patches I’ve posted to fix the qemu backend). Without a differentiator,

a frontend I’ve developed will get stuck waiting for the backend to connect,

and under Windows this effectively hangs the system.

The Qemu backend should be fixed to make the vkbd and vfb independent

devices. This proposal will help detect an incompatible backend and avoid a

VM hang.



(frontend WIP: http://xenbits.xen.org/gitweb/?p=pvdrivers/win/xenvkbd.git;a=tree)

Owen



From: Oleksandr Andrushchenko<mailto:andr2000@gmail.com>
Sent: 12 June 2017 08:07
To: Owen Smith<mailto:owen.smith@citrix.com>; xen-devel@lists.xenproject.org<mailto:xen-devel@lists.xenproject.org>
Subject: Re: [Xen-devel] [PATCH] kbdif.h: Introduce feature-vkbd-standalone



Hi, Owen!

On 06/08/2017 04:09 PM, Owen Smith wrote:
> Backends set "feature-vkbd-standalone" to 1 if they can connect
> without waiting for the PV framebuffer. If this value is missing
> or not 1, then a backend will wait for the PV framebuffer before
> connecting, potentially causing the frontend to wait indefinitely.
This means that by the new option we *fix an existing
backend* and its particular behavior. What is more,
we introduce knowledge of virtual fb into generic virtual
kbd/ptr protocol which seems to be not correct (IMO).
For the reasons above, I would recommend fixing the
corresponding backend instead, for example by configuring
it appropriately wrt use-case you have.
> Frontends set "request-vkbd-standalone" to 1 to request that the
> backend does not wait for the PV framebuffer. Frontends that
> require a standalone vkbd device should not attempt to connect
> unless the backend advertises "feature-vkbd-standalone", and
> should set "request-vkbd-standalone".
Again, this looks very use-case specific
> Backends that are standalone (i.e. do not have an associated PV
> framebuffer) do not rescale absolute mouse or touch coordinates
> to a the size of the (non-existant) PV framebuffer, and use the
> range of [0, 0x7fff] for absolute values.
>
> Signed-off-by: Owen Smith <owen.smith@citrix.com>
> ---
>   xen/include/public/io/kbdif.h | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h
> index dcbd71a..ca09080 100644
> --- a/xen/include/public/io/kbdif.h
> +++ b/xen/include/public/io/kbdif.h
> @@ -63,6 +63,12 @@
>    *      Backends, which support reporting of multi-touch events
>    *      should set this to 1.
>    *
> + * feature-vkbd-standalone
> + *      Values:         <uint>
> + *
> + *      Backends, which support a standalone vkbd, without requiring a vfb
> + *      device, should set this to 1.
> + *
>    *------------------------- Pointer Device Parameters ------------------------
>    *
>    * width
> @@ -98,6 +104,13 @@
>    *
>    *      Request backend to report multi-touch events.
>    *
> + * request-vkbd-standalone
> + *      Values:         <uint>
> + *
> + *      Request backend to connect vkbd device without waiting for the
> + *      vfb device. Any absolute coordinates will NOT be scaled to
> + *      screen size, and will remain in the range [0, 0x7fff]
> + *
>    *----------------------- Request Transport Parameters -----------------------
>    *
>    * event-channel
> @@ -165,8 +178,10 @@
>
>   #define XENKBD_FIELD_FEAT_ABS_POINTER  "feature-abs-pointer"
>   #define XENKBD_FIELD_FEAT_MTOUCH       "feature-multi-touch"
> +#define XENKBD_FIELD_FEAT_STANDALONE   "feature-vkbd-standalone"
>   #define XENKBD_FIELD_REQ_ABS_POINTER   "request-abs-pointer"
>   #define XENKBD_FIELD_REQ_MTOUCH        "request-multi-touch"
> +#define XENKBD_FIELD_REQ_STANDALONE    "request-vkbd-standalone"
>   #define XENKBD_FIELD_RING_GREF         "page-gref"
>   #define XENKBD_FIELD_EVT_CHANNEL       "event-channel"
>   #define XENKBD_FIELD_WIDTH             "width"
Thank you,
Oleksandr

[-- Attachment #1.2: Type: text/html, Size: 7467 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] kbdif.h: Introduce feature-vkbd-standalone

[Oleksandr Andrushchenko]

Hi, Owen!

On 06/13/2017 05:59 PM, Owen Smith wrote:
>
> Hi Oleksandr,
>
> The reason I’m proposing the additional feature flag is to 
> differentiate between
>
> older, broken, backends that will not connect without the vfb device
>
> (qemu-upstream’s implementation in hw/display/xenfb.c) and fixed backends
>
> (like patches I’ve posted to fix the qemu backend). Without a 
> differentiator,
>
> a frontend I’ve developed will get stuck waiting for the backend to 
> connect,
>
> and under Windows this effectively hangs the system.
>
I do understand your intention, but this way we are about to solve
particular problem of a particular implementation.
IMO, protocol is something that does not dictate how a backend
or frontend should be implemented. Neither if some particular
implementation has bugs then provide a way those can be worked around.
>
> The Qemu backend should be fixed to make the vkbd and vfb independent
>
> devices.
>
That is my point, if back/front are broken then we should fix those,
not help via protocol changes to work around. What will be the next
change if some other back/front steps in?
>
> This proposal will help detect an incompatible backend and avoid a
>
> VM hang.
>
> (frontend WIP: 
> http://xenbits.xen.org/gitweb/?p=pvdrivers/win/xenvkbd.git;a=tree)
>
> Owen
>
Anyway, I would like to hear from Konrad on this
>
> *From: *Oleksandr Andrushchenko <mailto:andr2000@gmail.com>
> *Sent: *12 June 2017 08:07
> *To: *Owen Smith <mailto:owen.smith@citrix.com>; 
> xen-devel@lists.xenproject.org <mailto:xen-devel@lists.xenproject.org>
> *Subject: *Re: [Xen-devel] [PATCH] kbdif.h: Introduce 
> feature-vkbd-standalone
>
> Hi, Owen!
>
> On 06/08/2017 04:09 PM, Owen Smith wrote:
> > Backends set "feature-vkbd-standalone" to 1 if they can connect
> > without waiting for the PV framebuffer. If this value is missing
> > or not 1, then a backend will wait for the PV framebuffer before
> > connecting, potentially causing the frontend to wait indefinitely.
> This means that by the new option we *fix an existing
> backend* and its particular behavior. What is more,
> we introduce knowledge of virtual fb into generic virtual
> kbd/ptr protocol which seems to be not correct (IMO).
> For the reasons above, I would recommend fixing the
> corresponding backend instead, for example by configuring
> it appropriately wrt use-case you have.
> > Frontends set "request-vkbd-standalone" to 1 to request that the
> > backend does not wait for the PV framebuffer. Frontends that
> > require a standalone vkbd device should not attempt to connect
> > unless the backend advertises "feature-vkbd-standalone", and
> > should set "request-vkbd-standalone".
> Again, this looks very use-case specific
> > Backends that are standalone (i.e. do not have an associated PV
> > framebuffer) do not rescale absolute mouse or touch coordinates
> > to a the size of the (non-existant) PV framebuffer, and use the
> > range of [0, 0x7fff] for absolute values.
> >
> > Signed-off-by: Owen Smith <owen.smith@citrix.com>
> > ---
> >   xen/include/public/io/kbdif.h | 15 +++++++++++++++
> >   1 file changed, 15 insertions(+)
> >
> > diff --git a/xen/include/public/io/kbdif.h 
> b/xen/include/public/io/kbdif.h
> > index dcbd71a..ca09080 100644
> > --- a/xen/include/public/io/kbdif.h
> > +++ b/xen/include/public/io/kbdif.h
> > @@ -63,6 +63,12 @@
> >    *      Backends, which support reporting of multi-touch events
> >    *      should set this to 1.
> >    *
> > + * feature-vkbd-standalone
> > + *      Values:         <uint>
> > + *
> > + *      Backends, which support a standalone vkbd, without 
> requiring a vfb
> > + *      device, should set this to 1.
> > + *
> >    *------------------------- Pointer Device Parameters 
> ------------------------
> >    *
> >    * width
> > @@ -98,6 +104,13 @@
> >    *
> >    *      Request backend to report multi-touch events.
> >    *
> > + * request-vkbd-standalone
> > + *      Values:         <uint>
> > + *
> > + *      Request backend to connect vkbd device without waiting for the
> > + *      vfb device. Any absolute coordinates will NOT be scaled to
> > + *      screen size, and will remain in the range [0, 0x7fff]
> > + *
> >    *----------------------- Request Transport Parameters 
> -----------------------
> >    *
> >    * event-channel
> > @@ -165,8 +178,10 @@
> >
> >   #define XENKBD_FIELD_FEAT_ABS_POINTER "feature-abs-pointer"
> >   #define XENKBD_FIELD_FEAT_MTOUCH "feature-multi-touch"
> > +#define XENKBD_FIELD_FEAT_STANDALONE "feature-vkbd-standalone"
> >   #define XENKBD_FIELD_REQ_ABS_POINTER "request-abs-pointer"
> >   #define XENKBD_FIELD_REQ_MTOUCH "request-multi-touch"
> > +#define XENKBD_FIELD_REQ_STANDALONE "request-vkbd-standalone"
> >   #define XENKBD_FIELD_RING_GREF         "page-gref"
> >   #define XENKBD_FIELD_EVT_CHANNEL "event-channel"
> >   #define XENKBD_FIELD_WIDTH             "width"
> Thank you,
> Oleksandr
Thank you,
Oleksandr

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] kbdif.h: Introduce feature-vkbd-standalone

[Paul Durrant]

> -----Original Message-----
> From: Xen-devel [mailto:xen-devel-bounces@lists.xen.org] On Behalf Of
> Oleksandr Andrushchenko
> Sent: 14 June 2017 08:48
> To: Owen Smith <owen.smith@citrix.com>; xen-devel@lists.xenproject.org;
> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Subject: Re: [Xen-devel] [PATCH] kbdif.h: Introduce feature-vkbd-
> standalone
> 
> Hi, Owen!
> 
> On 06/13/2017 05:59 PM, Owen Smith wrote:
> >
> > Hi Oleksandr,
> >
> > The reason I’m proposing the additional feature flag is to
> > differentiate between
> >
> > older, broken, backends that will not connect without the vfb device
> >
> > (qemu-upstream’s implementation in hw/display/xenfb.c) and fixed
> backends
> >
> > (like patches I’ve posted to fix the qemu backend). Without a
> > differentiator,
> >
> > a frontend I’ve developed will get stuck waiting for the backend to
> > connect,
> >
> > and under Windows this effectively hangs the system.
> >
> I do understand your intention, but this way we are about to solve
> particular problem of a particular implementation.
> IMO, protocol is something that does not dictate how a backend
> or frontend should be implemented. Neither if some particular
> implementation has bugs then provide a way those can be worked around.

That's the intention of "feature-vkbd-standalone"... If it is not there then it means the backend is old and broken and the Windows frontend cannot use it.

I agree that 'request-vkbd-standalone' should not be necessary if the vkbd and vfb protocols are intended to be truly independent.

  Paul

> >
> > The Qemu backend should be fixed to make the vkbd and vfb independent
> >
> > devices.
> >
> That is my point, if back/front are broken then we should fix those,
> not help via protocol changes to work around. What will be the next
> change if some other back/front steps in?
> >
> > This proposal will help detect an incompatible backend and avoid a
> >
> > VM hang.
> >
> > (frontend WIP:
> > http://xenbits.xen.org/gitweb/?p=pvdrivers/win/xenvkbd.git;a=tree)
> >
> > Owen
> >
> Anyway, I would like to hear from Konrad on this
> >
> > *From: *Oleksandr Andrushchenko <mailto:andr2000@gmail.com>
> > *Sent: *12 June 2017 08:07
> > *To: *Owen Smith <mailto:owen.smith@citrix.com>;
> > xen-devel@lists.xenproject.org <mailto:xen-devel@lists.xenproject.org>
> > *Subject: *Re: [Xen-devel] [PATCH] kbdif.h: Introduce
> > feature-vkbd-standalone
> >
> > Hi, Owen!
> >
> > On 06/08/2017 04:09 PM, Owen Smith wrote:
> > > Backends set "feature-vkbd-standalone" to 1 if they can connect
> > > without waiting for the PV framebuffer. If this value is missing
> > > or not 1, then a backend will wait for the PV framebuffer before
> > > connecting, potentially causing the frontend to wait indefinitely.
> > This means that by the new option we *fix an existing
> > backend* and its particular behavior. What is more,
> > we introduce knowledge of virtual fb into generic virtual
> > kbd/ptr protocol which seems to be not correct (IMO).
> > For the reasons above, I would recommend fixing the
> > corresponding backend instead, for example by configuring
> > it appropriately wrt use-case you have.
> > > Frontends set "request-vkbd-standalone" to 1 to request that the
> > > backend does not wait for the PV framebuffer. Frontends that
> > > require a standalone vkbd device should not attempt to connect
> > > unless the backend advertises "feature-vkbd-standalone", and
> > > should set "request-vkbd-standalone".
> > Again, this looks very use-case specific
> > > Backends that are standalone (i.e. do not have an associated PV
> > > framebuffer) do not rescale absolute mouse or touch coordinates
> > > to a the size of the (non-existant) PV framebuffer, and use the
> > > range of [0, 0x7fff] for absolute values.
> > >
> > > Signed-off-by: Owen Smith <owen.smith@citrix.com>
> > > ---
> > >   xen/include/public/io/kbdif.h | 15 +++++++++++++++
> > >   1 file changed, 15 insertions(+)
> > >
> > > diff --git a/xen/include/public/io/kbdif.h
> > b/xen/include/public/io/kbdif.h
> > > index dcbd71a..ca09080 100644
> > > --- a/xen/include/public/io/kbdif.h
> > > +++ b/xen/include/public/io/kbdif.h
> > > @@ -63,6 +63,12 @@
> > >    *      Backends, which support reporting of multi-touch events
> > >    *      should set this to 1.
> > >    *
> > > + * feature-vkbd-standalone
> > > + *      Values:         <uint>
> > > + *
> > > + *      Backends, which support a standalone vkbd, without
> > requiring a vfb
> > > + *      device, should set this to 1.
> > > + *
> > >    *------------------------- Pointer Device Parameters
> > ------------------------
> > >    *
> > >    * width
> > > @@ -98,6 +104,13 @@
> > >    *
> > >    *      Request backend to report multi-touch events.
> > >    *
> > > + * request-vkbd-standalone
> > > + *      Values:         <uint>
> > > + *
> > > + *      Request backend to connect vkbd device without waiting for the
> > > + *      vfb device. Any absolute coordinates will NOT be scaled to
> > > + *      screen size, and will remain in the range [0, 0x7fff]
> > > + *
> > >    *----------------------- Request Transport Parameters
> > -----------------------
> > >    *
> > >    * event-channel
> > > @@ -165,8 +178,10 @@
> > >
> > >   #define XENKBD_FIELD_FEAT_ABS_POINTER "feature-abs-pointer"
> > >   #define XENKBD_FIELD_FEAT_MTOUCH "feature-multi-touch"
> > > +#define XENKBD_FIELD_FEAT_STANDALONE "feature-vkbd-
> standalone"
> > >   #define XENKBD_FIELD_REQ_ABS_POINTER "request-abs-pointer"
> > >   #define XENKBD_FIELD_REQ_MTOUCH "request-multi-touch"
> > > +#define XENKBD_FIELD_REQ_STANDALONE "request-vkbd-standalone"
> > >   #define XENKBD_FIELD_RING_GREF         "page-gref"
> > >   #define XENKBD_FIELD_EVT_CHANNEL "event-channel"
> > >   #define XENKBD_FIELD_WIDTH             "width"
> > Thank you,
> > Oleksandr
> Thank you,
> Oleksandr
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel