From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57285) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XS8le-0000Tj-Tc for qemu-devel@nongnu.org; Thu, 11 Sep 2014 14:06:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XS8la-0006Cd-4I for qemu-devel@nongnu.org; Thu, 11 Sep 2014 14:06:50 -0400 Received: from smtp.citrix.com ([66.165.176.89]:64706) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XS8lZ-0006CT-TF for qemu-devel@nongnu.org; Thu, 11 Sep 2014 14:06:46 -0400 Date: Thu, 11 Sep 2014 19:04:56 +0100 From: Stefano Stabellini In-Reply-To: <6624BC057AF4E240B6D036F5CC505B12041F0053@AMSPEX01CL02.citrite.net> Message-ID: References: <1410356546-22264-1-git-send-email-owen.smith@citrix.com> <1410356546-22264-4-git-send-email-owen.smith@citrix.com> <6624BC057AF4E240B6D036F5CC505B12041F0053@AMSPEX01CL02.citrite.net> MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Subject: Re: [Qemu-devel] [PATCH 3/3] xenfb: Add "feature-no-abs-rescale" for Windows PV frontend List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Owen Smith Cc: Stefano Stabellini , "qemu-devel@nongnu.org" , "xen-devel@lists.xen.org" On Thu, 11 Sep 2014, Owen Smith wrote: > > -----Original Message----- > > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > > Sent: 11 September 2014 02:23 > > To: Owen Smith > > Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org; Stefano Stabellini > > Subject: Re: [PATCH 3/3] xenfb: Add "feature-no-abs-rescale" for Windows > > PV frontend > > > > On Wed, 10 Sep 2014, Owen smith wrote: > > > Windows PV frontend requires absolute mouse values in the range [0, > > > 0x7fff]. Add a feature to not rescale the axes to DisplaySurface size. > > > Also allows Windows PV frontend to connect without a vfb device. Moves > > > the rescaling of axes to a seperate function with additional > > > null-pointer checks. > > > > > > Signed-off-by: Owen smith > > > --- > > > hw/display/xenfb.c | 52 > > > +++++++++++++++++++++++++++++++++++++--------------- > > > 1 file changed, 37 insertions(+), 15 deletions(-) > > > > > > diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c index > > > c4375c8..65c373b 100644 > > > --- a/hw/display/xenfb.c > > > +++ b/hw/display/xenfb.c > > > @@ -61,6 +61,7 @@ struct common { > > > struct XenInput { > > > struct common c; > > > int abs_pointer_wanted; /* Whether guest supports absolute > > > pointer */ > > > + int no_abs_rescale; /* Whether guest wants rescaled abs axes */ > > > int button_state; /* Last seen pointer button state */ > > > int extended; > > > QEMUPutKbdEntry *qkbd; > > > @@ -325,6 +326,25 @@ static void xenfb_key_event(void *opaque, int > > scancode) > > > xenfb_send_key(xenfb, down, scancode2linux[scancode]); } > > > > > > +static void xenfb_rescale_mouse(struct XenInput *xenfb, int *dx, int > > > +*dy) { > > > + DisplaySurface *surface; > > > + int dw, dh; > > > + > > > + if (xenfb->c.con == NULL) { > > > + return; > > > + } > > > + surface = qemu_console_surface(xenfb->c.con); > > > + if (surface == NULL) { > > > + return; > > > + } > > > + dw = surface_width(surface); > > > + dh = surface_height(surface); > > > + > > > + *dx = *dx * (dw - 1) / 0x7fff; > > > + *dy = *dy * (dh - 1) / 0x7fff; > > > +} > > > + > > > /* > > > * Send a mouse event from the client to the guest OS > > > * > > > @@ -338,18 +358,16 @@ static void xenfb_mouse_event(void *opaque, > > > int dx, int dy, int dz, int button_state) { > > > struct XenInput *xenfb = opaque; > > > - DisplaySurface *surface = qemu_console_surface(xenfb->c.con); > > > - int dw = surface_width(surface); > > > - int dh = surface_height(surface); > > > int i; > > > > > > - if (xenfb->abs_pointer_wanted) > > > - xenfb_send_position(xenfb, > > > - dx * (dw - 1) / 0x7fff, > > > - dy * (dh - 1) / 0x7fff, > > > - dz); > > > - else > > > + if (xenfb->abs_pointer_wanted) { > > > + if (!xenfb->no_abs_rescale) { > > > + xenfb_rescale_mouse(xenfb, &dx, &dy); > > > + } > > > + xenfb_send_position(xenfb, dx, dy, dz); > > > + } else { > > > xenfb_send_motion(xenfb, dx, dy, dz); > > > + } > > > > > > for (i = 0 ; i < 8 ; i++) { > > > int lastDown = xenfb->button_state & (1 << i); @@ -366,6 +382,7 > > @@ > > > static void xenfb_mouse_event(void *opaque, static int > > > input_init(struct XenDevice *xendev) { > > > xenstore_write_be_int(xendev, "feature-abs-pointer", 1); > > > + xenstore_write_be_int(xendev, "feature-no-abs-rescale", 1); > > > return 0; > > > } > > > > > > @@ -374,7 +391,17 @@ static int input_initialise(struct XenDevice > > *xendev) > > > struct XenInput *in = container_of(xendev, struct XenInput, c.xendev); > > > int rc; > > > > > > - if (!in->c.con) { > > > + if (xenstore_read_fe_int(xendev, "request-abs-pointer", > > > + &in->abs_pointer_wanted) == -1) { > > > + in->abs_pointer_wanted = 0; > > > + } > > > + > > > + if (xenstore_read_fe_int(xendev, "request-no-abs-rescale", > > > + &in->no_abs_rescale) == -1) { > > > + in->no_abs_rescale = 0; > > > + } > > > > I don't think is safe to move checking for "request-abs-pointer" so > > early: what if the guest writes request-abs-pointer after initialise but before > > connect? > > > > Also it would be nice to document this protocol extension somewhere. I do > > realize that the existing documentation on vfb is somewhat lacking. > > > > I'll change this to read the feature flags during the input_connected for v2. > I've spotted that neither feature has anything in the xen header, so I'll include > a patch to document this with v2. Thanks > > > + if (!in->c.con && !in->no_abs_rescale) { > > > > Why this change? > > > > Without an additional check here, the frontend would not connect. in->c.con is > not initialised without a vfb device. Overloading no_abs_rescale here is not right, > I'll add an option that the frontend can set to ignore the absence of the console. You don't need a new option for that. We wouldn't be introducing any regression by letting vkdb connect without a vfb device. You could just remove the check. > > > > > > xen_be_printf(xendev, 1, "ds not set (yet)\n"); > > > return -1; > > > } > > > @@ -390,11 +417,6 @@ static void input_connected(struct XenDevice > > > *xendev) { > > > struct XenInput *in = container_of(xendev, struct XenInput, > > > c.xendev); > > > > > > - if (xenstore_read_fe_int(xendev, "request-abs-pointer", > > > - &in->abs_pointer_wanted) == -1) { > > > - in->abs_pointer_wanted = 0; > > > - } > > > - > > > if (in->qmouse) { > > > qemu_remove_mouse_event_handler(in->qmouse); > > > } > > > -- > > > 2.1.0 > > > >