xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Paul Durrant <Paul.Durrant@citrix.com>
To: 'Oleksandr Andrushchenko' <andr2000@gmail.com>,
	Owen Smith <owen.smith@citrix.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Subject: Re: [PATCH] kbdif.h: Introduce feature-vkbd-standalone
Date: Wed, 14 Jun 2017 08:54:34 +0000	[thread overview]
Message-ID: <2fda91e1ccfd458283a76372b0d5ea1e@AMSPEX02CL03.citrite.net> (raw)
In-Reply-To: <21f28412-6cc3-60a9-455e-f0bfcf579b5b@gmail.com>

> -----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

      reply	other threads:[~2017-06-14  8:54 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-08 13:09 [PATCH] kbdif.h: Introduce feature-vkbd-standalone Owen Smith
2017-06-12  7:07 ` Oleksandr Andrushchenko
2017-06-13 14:59   ` Owen Smith
2017-06-14  7:48     ` Oleksandr Andrushchenko
2017-06-14  8:54       ` Paul Durrant [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2fda91e1ccfd458283a76372b0d5ea1e@AMSPEX02CL03.citrite.net \
    --to=paul.durrant@citrix.com \
    --cc=andr2000@gmail.com \
    --cc=konrad.wilk@oracle.com \
    --cc=owen.smith@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).