| Message ID | 1506437019-17946-4-git-send-email-owen.smith@citrix.com |
|---|---|
| State | New |
| Headers | show |
| Series | xenfb: Enablement for Windows PV HID frontend | expand |
On Tue, Sep 26, 2017 at 02:43:39PM +0000, Owen Smith wrote: > Writes "feature-raw-pointer" during init to indicate the backend > can pass raw unscaled values for absolute axes to the frontend. > Frontends set "request-raw-pointer" to indicate the backend should > not attempt to scale absolute values to console size. > "request-raw-pointer" is only valid if "request-abs-pointer" is > also set. Raw unscaled pointer values are in the range [0, 0x7fff] > > Signed-off-by: Owen Smith <owen.smith@citrix.com> Hi Owen, Why did you remove the following from the commit description? > "feature-raw-pointer" and "request-raw-pointer" added to Xen > header in commit 7868654ff7fe5e4a2eeae2b277644fa884a5031e I think that with it, you could have kept stefano's reviewed-by tag. Thanks,
On Mon, 2 Oct 2017, Anthony PERARD wrote: > On Tue, Sep 26, 2017 at 02:43:39PM +0000, Owen Smith wrote: > > Writes "feature-raw-pointer" during init to indicate the backend > > can pass raw unscaled values for absolute axes to the frontend. > > Frontends set "request-raw-pointer" to indicate the backend should > > not attempt to scale absolute values to console size. > > "request-raw-pointer" is only valid if "request-abs-pointer" is > > also set. Raw unscaled pointer values are in the range [0, 0x7fff] > > > > Signed-off-by: Owen Smith <owen.smith@citrix.com> > > Hi Owen, > > Why did you remove the following from the commit description? > > "feature-raw-pointer" and "request-raw-pointer" added to Xen > > header in commit 7868654ff7fe5e4a2eeae2b277644fa884a5031e > > I think that with it, you could have kept stefano's reviewed-by tag. Hi Anthony, Have you tested this series with a few of different guests? Do you consider it safe to merge? If so, we can send it upstream (either via xen or via ui as Gerd kindly offered).
On Tue, Oct 10, 2017 at 04:52:48PM -0700, Stefano Stabellini wrote: > On Mon, 2 Oct 2017, Anthony PERARD wrote: > > On Tue, Sep 26, 2017 at 02:43:39PM +0000, Owen Smith wrote: > > > Writes "feature-raw-pointer" during init to indicate the backend > > > can pass raw unscaled values for absolute axes to the frontend. > > > Frontends set "request-raw-pointer" to indicate the backend should > > > not attempt to scale absolute values to console size. > > > "request-raw-pointer" is only valid if "request-abs-pointer" is > > > also set. Raw unscaled pointer values are in the range [0, 0x7fff] > > > > > > Signed-off-by: Owen Smith <owen.smith@citrix.com> > > > > Hi Owen, > > > > Why did you remove the following from the commit description? > > > "feature-raw-pointer" and "request-raw-pointer" added to Xen > > > header in commit 7868654ff7fe5e4a2eeae2b277644fa884a5031e > > > > I think that with it, you could have kept stefano's reviewed-by tag. > > Hi Anthony, > > Have you tested this series with a few of different guests? Do you > consider it safe to merge? If so, we can send it upstream (either via > xen or via ui as Gerd kindly offered). Yes, I think it's fine. The only observation I have is that on a Linux guest, when I have usbdevice=tablet, with this series the pv mouse seems to become the primary way of gueting mouse events (without, the tablet is primary). So, on my VNC client instead of having both mouse in the guest and on my desktop being at the same place, there is like a zoom of the mouse (the zoom centered on the top-left corner). (That's better than relative mouse event that we can get with the emulation.)
On Wed, 11 Oct 2017, Anthony PERARD wrote: > On Tue, Oct 10, 2017 at 04:52:48PM -0700, Stefano Stabellini wrote: > > On Mon, 2 Oct 2017, Anthony PERARD wrote: > > > On Tue, Sep 26, 2017 at 02:43:39PM +0000, Owen Smith wrote: > > > > Writes "feature-raw-pointer" during init to indicate the backend > > > > can pass raw unscaled values for absolute axes to the frontend. > > > > Frontends set "request-raw-pointer" to indicate the backend should > > > > not attempt to scale absolute values to console size. > > > > "request-raw-pointer" is only valid if "request-abs-pointer" is > > > > also set. Raw unscaled pointer values are in the range [0, 0x7fff] > > > > > > > > Signed-off-by: Owen Smith <owen.smith@citrix.com> > > > > > > Hi Owen, > > > > > > Why did you remove the following from the commit description? > > > > "feature-raw-pointer" and "request-raw-pointer" added to Xen > > > > header in commit 7868654ff7fe5e4a2eeae2b277644fa884a5031e > > > > > > I think that with it, you could have kept stefano's reviewed-by tag. > > > > Hi Anthony, > > > > Have you tested this series with a few of different guests? Do you > > consider it safe to merge? If so, we can send it upstream (either via > > xen or via ui as Gerd kindly offered). > > Yes, I think it's fine. > > The only observation I have is that on a Linux guest, when I have > usbdevice=tablet, with this series the pv mouse seems to become the > primary way of gueting mouse events (without, the tablet is primary). > So, on my VNC client instead of having both mouse in the guest and on my > desktop being at the same place, there is like a zoom of the mouse (the > zoom centered on the top-left corner). (That's better than relative > mouse event that we can get with the emulation.) Thanks for testing. I am not completely sure about what should be the right behavior when both usbdevice=tablet and pvmouse are present. Typically, PV devices take precedence over emulated devices, so maybe it is OK that PV mouse is the primary device in this case. But we would need to document this behavioral change in the commit descriptions. The other question is whether the "zoom of the mouse" you are seeing is normal or whether we can "fix" it somehow. I guess it has always been the case for PV mouse? It is not something new, is it?
> -----Original Message----- > From: Xen-devel [mailto:xen-devel-bounces@lists.xen.org] On Behalf Of > Stefano Stabellini > Sent: 11 October 2017 21:19 > To: Anthony Perard <anthony.perard@citrix.com> > Cc: qemu-devel@nongnu.org; xen-devel@lists.xenproject.org; Stefano > Stabellini <sstabellini@kernel.org>; Owen Smith <owen.smith@citrix.com>; > kraxel@redhat.com > Subject: Re: [Xen-devel] [PATCH 3/3 v4] xenfb: Add [feature|request]-raw- > pointer > > On Wed, 11 Oct 2017, Anthony PERARD wrote: > > On Tue, Oct 10, 2017 at 04:52:48PM -0700, Stefano Stabellini wrote: > > > On Mon, 2 Oct 2017, Anthony PERARD wrote: > > > > On Tue, Sep 26, 2017 at 02:43:39PM +0000, Owen Smith wrote: > > > > > Writes "feature-raw-pointer" during init to indicate the backend > > > > > can pass raw unscaled values for absolute axes to the frontend. > > > > > Frontends set "request-raw-pointer" to indicate the backend should > > > > > not attempt to scale absolute values to console size. > > > > > "request-raw-pointer" is only valid if "request-abs-pointer" is > > > > > also set. Raw unscaled pointer values are in the range [0, 0x7fff] > > > > > > > > > > Signed-off-by: Owen Smith <owen.smith@citrix.com> > > > > > > > > Hi Owen, > > > > > > > > Why did you remove the following from the commit description? > > > > > "feature-raw-pointer" and "request-raw-pointer" added to Xen > > > > > header in commit 7868654ff7fe5e4a2eeae2b277644fa884a5031e > > > > > > > > I think that with it, you could have kept stefano's reviewed-by tag. > > > > > > Hi Anthony, > > > > > > Have you tested this series with a few of different guests? Do you > > > consider it safe to merge? If so, we can send it upstream (either via > > > xen or via ui as Gerd kindly offered). > > > > Yes, I think it's fine. > > > > The only observation I have is that on a Linux guest, when I have > > usbdevice=tablet, with this series the pv mouse seems to become the > > primary way of gueting mouse events (without, the tablet is primary). > > So, on my VNC client instead of having both mouse in the guest and on my > > desktop being at the same place, there is like a zoom of the mouse (the > > zoom centered on the top-left corner). (That's better than relative > > mouse event that we can get with the emulation.) > > Thanks for testing. I am not completely sure about what should be the > right behavior when both usbdevice=tablet and pvmouse are present. > Typically, PV devices take precedence over emulated devices, so maybe > it is OK that PV mouse is the primary device in this case. > > But we would need to document this behavioral change in the commit > descriptions. > It's probably OS specific though. I guess the behaviour changed because the OS favours absolute pointing devices over relative ones and how it has two absolute ones to choose from. How it reconciles those, who knows? Paul > The other question is whether the "zoom of the mouse" you are seeing is > normal or whether we can "fix" it somehow. I guess it has always been > the case for PV mouse? It is not something new, is it? > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > https://lists.xen.org/xen-devel
Hi, > It's probably OS specific though. I guess the behaviour changed > because the OS favours absolute pointing devices over relative ones > and how it has two absolute ones to choose from. How it reconciles > those, who knows? Typically hid emulation calls qemu_input_handler_activate() when the guest initializes the device, which moves the device to the top of the priority list. Visible effect on a typical guest with ps/2 mouse and usb-tablet is that qemu switches from relative mode (mouse) to absolute mode (tablet) when the guest loads the usb hid driver. I suspect pvmouse is doing the same thing. So it may simply depend on guest driver load order whenever pvmouse or usb-tablet is used. Simplest fix is probably to only attach the device you plan to use to the guest. If you can't turn off pvmouse for xen guests then you might want drop the qemu_input_handler_activate() call, so it behaves simliar to the ps/2 mouse (is used in case no other pointer device is present). HTH, Gerd
> -----Original Message----- > From: Gerd Hoffmann [mailto:kraxel@redhat.com] > Sent: 12 October 2017 10:26 > To: Paul Durrant <Paul.Durrant@citrix.com>; 'Stefano Stabellini' > <sstabellini@kernel.org>; Anthony Perard <anthony.perard@citrix.com> > Cc: qemu-devel@nongnu.org; xen-devel@lists.xenproject.org; Owen Smith > <owen.smith@citrix.com> > Subject: Re: [Xen-devel] [PATCH 3/3 v4] xenfb: Add [feature|request]-raw- > pointer > > Hi, > > > It's probably OS specific though. I guess the behaviour changed > > because the OS favours absolute pointing devices over relative ones > > and how it has two absolute ones to choose from. How it reconciles > > those, who knows? > > Typically hid emulation calls qemu_input_handler_activate() when the > guest initializes the device, which moves the device to the top of the > priority list. > > Visible effect on a typical guest with ps/2 mouse and usb-tablet is > that qemu switches from relative mode (mouse) to absolute mode (tablet) > when the guest loads the usb hid driver. > > I suspect pvmouse is doing the same thing. So it may simply depend on > guest driver load order whenever pvmouse or usb-tablet is used. > > Simplest fix is probably to only attach the device you plan to use to > the guest. If you can't turn off pvmouse for xen guests then you might > want drop the qemu_input_handler_activate() call, so it behaves simliar > to the ps/2 mouse (is used in case no other pointer device is present). Avoiding the activate call sounds reasonable and should avoid the behavioural change. Cheers, Paul > > HTH, > Gerd
On Wed, Oct 11, 2017 at 01:19:25PM -0700, Stefano Stabellini wrote: > On Wed, 11 Oct 2017, Anthony PERARD wrote: > > The only observation I have is that on a Linux guest, when I have > > usbdevice=tablet, with this series the pv mouse seems to become the > > primary way of gueting mouse events (without, the tablet is primary). > > So, on my VNC client instead of having both mouse in the guest and on my > > desktop being at the same place, there is like a zoom of the mouse (the > > zoom centered on the top-left corner). (That's better than relative > > mouse event that we can get with the emulation.) > > Thanks for testing. I am not completely sure about what should be the > right behavior when both usbdevice=tablet and pvmouse are present. > Typically, PV devices take precedence over emulated devices, so maybe > it is OK that PV mouse is the primary device in this case. > > But we would need to document this behavioral change in the commit > descriptions. Maybe something like "WARNING: pvmouse is fixed and now works!" :) I think the change come from the fact that without the second patch, the pv mouse doesn't work (or at least, I did not manage to make it work). > The other question is whether the "zoom of the mouse" you are seeing is > normal or whether we can "fix" it somehow. I guess it has always been > the case for PV mouse? It is not something new, is it? Yes, I think it always as been the case for PV mouse. I pretty sure I've seen this behavior long time ago.
On Thu, 12 Oct 2017, Paul Durrant wrote: > > -----Original Message----- > > From: Gerd Hoffmann [mailto:kraxel@redhat.com] > > Sent: 12 October 2017 10:26 > > To: Paul Durrant <Paul.Durrant@citrix.com>; 'Stefano Stabellini' > > <sstabellini@kernel.org>; Anthony Perard <anthony.perard@citrix.com> > > Cc: qemu-devel@nongnu.org; xen-devel@lists.xenproject.org; Owen Smith > > <owen.smith@citrix.com> > > Subject: Re: [Xen-devel] [PATCH 3/3 v4] xenfb: Add [feature|request]-raw- > > pointer > > > > Hi, > > > > > It's probably OS specific though. I guess the behaviour changed > > > because the OS favours absolute pointing devices over relative ones > > > and how it has two absolute ones to choose from. How it reconciles > > > those, who knows? > > > > Typically hid emulation calls qemu_input_handler_activate() when the > > guest initializes the device, which moves the device to the top of the > > priority list. > > > > Visible effect on a typical guest with ps/2 mouse and usb-tablet is > > that qemu switches from relative mode (mouse) to absolute mode (tablet) > > when the guest loads the usb hid driver. > > > > I suspect pvmouse is doing the same thing. So it may simply depend on > > guest driver load order whenever pvmouse or usb-tablet is used. > > > > Simplest fix is probably to only attach the device you plan to use to > > the guest. If you can't turn off pvmouse for xen guests then you might > > want drop the qemu_input_handler_activate() call, so it behaves simliar > > to the ps/2 mouse (is used in case no other pointer device is present). > > Avoiding the activate call sounds reasonable and should avoid the behavioural change. +1 Owen, are you up for resubmitting the series with this small change?
> -----Original Message----- > From: Stefano Stabellini [mailto:sstabellini@kernel.org] > Sent: 12 October 2017 18:27 > To: Paul Durrant <Paul.Durrant@citrix.com> > Cc: 'Gerd Hoffmann' <kraxel@redhat.com>; 'Stefano Stabellini' > <sstabellini@kernel.org>; Anthony Perard <anthony.perard@citrix.com>; > qemu-devel@nongnu.org; xen-devel@lists.xenproject.org; Owen Smith > <owen.smith@citrix.com> > Subject: RE: [Xen-devel] [PATCH 3/3 v4] xenfb: Add [feature|request]-raw- > pointer > > On Thu, 12 Oct 2017, Paul Durrant wrote: > > > -----Original Message----- > > > From: Gerd Hoffmann [mailto:kraxel@redhat.com] > > > Sent: 12 October 2017 10:26 > > > To: Paul Durrant <Paul.Durrant@citrix.com>; 'Stefano Stabellini' > > > <sstabellini@kernel.org>; Anthony Perard <anthony.perard@citrix.com> > > > Cc: qemu-devel@nongnu.org; xen-devel@lists.xenproject.org; Owen > > > Smith <owen.smith@citrix.com> > > > Subject: Re: [Xen-devel] [PATCH 3/3 v4] xenfb: Add > > > [feature|request]-raw- pointer > > > > > > Hi, > > > > > > > It's probably OS specific though. I guess the behaviour changed > > > > because the OS favours absolute pointing devices over relative > > > > ones and how it has two absolute ones to choose from. How it > > > > reconciles those, who knows? > > > > > > Typically hid emulation calls qemu_input_handler_activate() when the > > > guest initializes the device, which moves the device to the top of > > > the priority list. > > > > > > Visible effect on a typical guest with ps/2 mouse and usb-tablet is > > > that qemu switches from relative mode (mouse) to absolute mode > > > (tablet) when the guest loads the usb hid driver. > > > > > > I suspect pvmouse is doing the same thing. So it may simply depend > > > on guest driver load order whenever pvmouse or usb-tablet is used. > > > > > > Simplest fix is probably to only attach the device you plan to use > > > to the guest. If you can't turn off pvmouse for xen guests then you > > > might want drop the qemu_input_handler_activate() call, so it > > > behaves simliar to the ps/2 mouse (is used in case no other pointer > device is present). > > > > Avoiding the activate call sounds reasonable and should avoid the > behavioural change. > > +1 > > Owen, are you up for resubmitting the series with this small change? Having just rebuilt my xen / linux host, and checked qemu, it looks like the keycodemapdb patches have gone in, so I'll rebase and re-submit this series Owen
diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c index 33361b4..29428ae 100644 --- a/hw/display/xenfb.c +++ b/hw/display/xenfb.c @@ -52,6 +52,7 @@ struct common { struct XenInput { struct common c; int abs_pointer_wanted; /* Whether guest supports absolute pointer */ + int raw_pointer_wanted; /* Whether guest supports raw (unscaled) pointer */ QemuInputHandlerState *qkbd; QemuInputHandlerState *qmou; int mouse_axes[INPUT_AXIS__MAX]; @@ -264,21 +265,23 @@ static void xenfb_mouse_sync(DeviceState *dev) in->abs_pointer_wanted); if (in->abs_pointer_wanted) { - QemuConsole *con = qemu_console_lookup_by_index(0); - DisplaySurface *surface; - int dw, dh; - - if (!con) { - xen_pv_printf(&in->c.xendev, 0, "No QEMU console available"); - return; - } + if (!in->raw_pointer_wanted) { + QemuConsole *con = qemu_console_lookup_by_index(0); + DisplaySurface *surface; + int dw, dh; + + if (!con) { + xen_pv_printf(&in->c.xendev, 0, "No QEMU console available"); + return; + } - surface = qemu_console_surface(con); - dw = surface_width(surface); - dh = surface_height(surface); + surface = qemu_console_surface(con); + dw = surface_width(surface); + dh = surface_height(surface); - dx = dx * (dw - 1) / 0x7fff; - dy = dy * (dh - 1) / 0x7fff; + dx = dx * (dw - 1) / 0x7fff; + dy = dy * (dh - 1) / 0x7fff; + } xenfb_send_position(in, dx, dy, dz); } else { @@ -312,6 +315,7 @@ static QemuInputHandler xenfb_rel_mouse = { static int input_init(struct XenDevice *xendev) { xenstore_write_be_int(xendev, "feature-abs-pointer", 1); + xenstore_write_be_int(xendev, "feature-raw-pointer", 1); return 0; } @@ -335,6 +339,13 @@ static void input_connected(struct XenDevice *xendev) &in->abs_pointer_wanted) == -1) { in->abs_pointer_wanted = 0; } + if (xenstore_read_fe_int(xendev, "request-raw-pointer", + &in->raw_pointer_wanted) == -1) { + in->raw_pointer_wanted = 0; + } + if (in->raw_pointer_wanted && !in->abs_pointer_wanted) { + xen_pv_printf(xendev, 0, "raw pointer set without absolute pointer."); + } if (in->qkbd) { qemu_input_handler_unregister(in->qkbd);
Writes "feature-raw-pointer" during init to indicate the backend can pass raw unscaled values for absolute axes to the frontend. Frontends set "request-raw-pointer" to indicate the backend should not attempt to scale absolute values to console size. "request-raw-pointer" is only valid if "request-abs-pointer" is also set. Raw unscaled pointer values are in the range [0, 0x7fff] Signed-off-by: Owen Smith <owen.smith@citrix.com> --- hw/display/xenfb.c | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-)