Re: [Qemu-devel] [PATCH 2/2 v2] xenfb: Allow vkbd to connect without a DisplayState

[Stefano Stabellini]

On Mon, 3 Jul 2017, Owen Smith wrote:
> If the vkbd device model is registered and the vfb device model
> is not registered, the backend will not transition to connected.
> If there is no DisplayState, then the absolute coordinates cannot
> be scaled, and will remain in the range [0, 0x7fff].
> Backend writes "feature-raw-pointer" to indicate that the backend
> supports reporting absolute position without rescaling.
> The frontend uses "request-raw-pointer" to request raw unscaled
> pointer values. If there is no DisplayState, the absolute values
> are always raw unscaled values.
> 
> Signed-off-by: Owen Smith 
> ---
>  hw/display/xenfb.c | 36 ++--
>  1 file changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
> index 88815df..d40af6e 100644
> --- a/hw/display/xenfb.c
> +++ b/hw/display/xenfb.c
> @@ -53,6 +53,7 @@ struct common {
>  struct XenInput {
>  struct common c;
>  int abs_pointer_wanted; /* Whether guest supports absolute pointer */
> +int raw_pointer_wanted; /* Whether guest supports unscaled pointer */
>  int button_state;   /* Last seen pointer button state */
>  int extended;
>  /* kbd */
> @@ -329,18 +330,22 @@ 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;
> +int i, x, y;
> +if (xenfb->c.con && xenfb->raw_pointer_wanted != 1) {
> +DisplaySurface *surface = qemu_console_surface(xenfb->c.con);
> +int dw = surface_width(surface);
> +int dh = surface_height(surface);
> +x = dx * (dw - 1) / 0x7fff;
> +y = dy * (dh - 1) / 0x7fff;
> +} else {
> +x = dx;
> +y = dy;
> +}
>  
>  trace_xenfb_mouse_event(opaque, dx, dy, dz, button_state,
>  xenfb->abs_pointer_wanted);
>  if (xenfb->abs_pointer_wanted)

Shouldn't this be:

  if (xenfb->abs_pointer_wanted || xenfb->raw_pointer_wanted)

?
It it possible to have raw_pointer_wanted && !abs_pointer_wanted? If
not, we should check at connection or initialization time.


> - xenfb_send_position(xenfb,
> - dx * (dw - 1) / 0x7fff,
> - dy * (dh - 1) / 0x7fff,
> - dz);
> +xenfb_send_position(xenfb, x, y, dz);
>  else
>   xenfb_send_motion(xenfb, dx, dy, dz);
>  
> @@ -423,6 +428,7 @@ static void xenfb_legacy_mouse_sync(DeviceState *dev)
>  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;
>  }
>  
> @@ -432,8 +438,14 @@ static int input_initialise(struct XenDevice *xendev)
>  int rc;
>  
>  if (!in->c.con) {
> -xen_pv_printf(xendev, 1, "ds not set (yet)\n");
> -return -1;
> +char *vfb = xenstore_read_str(NULL, "device/vfb");

Isn't it better to do xenstore_read_str("device", "vfb") ?


> +if (vfb == NULL) {
> +/* there is no vfb, run vkbd on its own */
> +} else {

if (vfb != NULL)


> +free(vfb);

g_free


> +xen_pv_printf(xendev, 1, "ds not set (yet)\n");
> +return -1;
> +}
>  }
>  
>  rc = common_bind(>c);
> @@ -451,6 +463,10 @@ static void input_connected(struct XenDevice *xendev)
>   >abs_pointer_wanted) == -1) {
>  in->abs_pointer_wanted = 0;
>  }
> +if (xenstore_read_fe_int(xendev, "request-raw-pointer",
> + >raw_pointer_wanted) == -1) {
> +in->raw_pointer_wanted = 0;
> +}
>  
>  if (in->qkbd) {
>  qemu_input_handler_unregister(in->qkbd);
> -- 
> 2.1.4
> 

[Qemu-devel] [PATCH 2/2 v2] xenfb: Allow vkbd to connect without a DisplayState

[Owen Smith]

If the vkbd device model is registered and the vfb device model
is not registered, the backend will not transition to connected.
If there is no DisplayState, then the absolute coordinates cannot
be scaled, and will remain in the range [0, 0x7fff].
Backend writes "feature-raw-pointer" to indicate that the backend
supports reporting absolute position without rescaling.
The frontend uses "request-raw-pointer" to request raw unscaled
pointer values. If there is no DisplayState, the absolute values
are always raw unscaled values.

Signed-off-by: Owen Smith 
---
 hw/display/xenfb.c | 36 ++--
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
index 88815df..d40af6e 100644
--- a/hw/display/xenfb.c
+++ b/hw/display/xenfb.c
@@ -53,6 +53,7 @@ struct common {
 struct XenInput {
 struct common c;
 int abs_pointer_wanted; /* Whether guest supports absolute pointer */
+int raw_pointer_wanted; /* Whether guest supports unscaled pointer */
 int button_state;   /* Last seen pointer button state */
 int extended;
 /* kbd */
@@ -329,18 +330,22 @@ 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;
+int i, x, y;
+if (xenfb->c.con && xenfb->raw_pointer_wanted != 1) {
+DisplaySurface *surface = qemu_console_surface(xenfb->c.con);
+int dw = surface_width(surface);
+int dh = surface_height(surface);
+x = dx * (dw - 1) / 0x7fff;
+y = dy * (dh - 1) / 0x7fff;
+} else {
+x = dx;
+y = dy;
+}
 
 trace_xenfb_mouse_event(opaque, dx, dy, dz, button_state,
 xenfb->abs_pointer_wanted);
 if (xenfb->abs_pointer_wanted)
-   xenfb_send_position(xenfb,
-   dx * (dw - 1) / 0x7fff,
-   dy * (dh - 1) / 0x7fff,
-   dz);
+xenfb_send_position(xenfb, x, y, dz);
 else
xenfb_send_motion(xenfb, dx, dy, dz);
 
@@ -423,6 +428,7 @@ static void xenfb_legacy_mouse_sync(DeviceState *dev)
 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;
 }
 
@@ -432,8 +438,14 @@ static int input_initialise(struct XenDevice *xendev)
 int rc;
 
 if (!in->c.con) {
-xen_pv_printf(xendev, 1, "ds not set (yet)\n");
-return -1;
+char *vfb = xenstore_read_str(NULL, "device/vfb");
+if (vfb == NULL) {
+/* there is no vfb, run vkbd on its own */
+} else {
+free(vfb);
+xen_pv_printf(xendev, 1, "ds not set (yet)\n");
+return -1;
+}
 }
 
 rc = common_bind(>c);
@@ -451,6 +463,10 @@ static void input_connected(struct XenDevice *xendev)
  >abs_pointer_wanted) == -1) {
 in->abs_pointer_wanted = 0;
 }
+if (xenstore_read_fe_int(xendev, "request-raw-pointer",
+ >raw_pointer_wanted) == -1) {
+in->raw_pointer_wanted = 0;
+}
 
 if (in->qkbd) {
 qemu_input_handler_unregister(in->qkbd);
-- 
2.1.4