[Qemu-devel] [PATCH v2 0/5] xenfb: Add support for Windows PV frontend

[Qemu-devel] [PATCH v2 0/5] xenfb: Add support for Windows PV frontend

[Owen smith]

This patch series contains improvments for the Xen vkbd backend to support a 
Windows PV frontend mouse and keyboard. This allows VNC connections to have 
an absolute pointer without the USB tabled device enabled, and any unneccessary
polling associated with the USB devices.

Changes over v1:
* Split the original first patch into keyboard handler changes and 
  activation of mouse handler
* Document "page-gref" in commit comment
* Document "feature-no-abs-rescale" in commit comment
* Add additional patch adding "feature-no-console" to selectivly disable
  the requirement for a vfb device. When frontend sets "request-no-console",
  backend will not wait for a console before vkbd device transitions to 
  connected

Owen smith (5):
  xenfb: Unregister keyboard event handler correctly
  xenfb: Activate mouse event handler
  xenfb: Add option to use a grant ref for shared page
  xenfb: Add "feature-no-abs-rescale"
  xenfb: Add "feature-no-console"

 hw/display/xenfb.c | 113 +++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 89 insertions(+), 24 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH v2 1/5] xenfb: Unregister keyboard event handler correctly

[Owen smith]

The keyboard event handler was not being removed, a NULL-callback
entry was being added to intercept events. Use the unregister call
to remove the keyboard event handler when appropriate.

Signed-off-by: Owen smith <owen.smith@citrix.com>
---
 hw/display/xenfb.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
index 07ddc9d..9dcf9b6 100644
--- a/hw/display/xenfb.c
+++ b/hw/display/xenfb.c
@@ -62,6 +62,7 @@ struct XenInput {
     int abs_pointer_wanted; /* Whether guest supports absolute pointer */
     int button_state;       /* Last seen pointer button state */
     int extended;
+    QEMUPutKbdEntry *qkbd;
     QEMUPutMouseEntry *qmouse;
 };
 
@@ -364,7 +365,6 @@ static int input_initialise(struct XenDevice *xendev)
     if (rc != 0)
 	return rc;
 
-    qemu_add_kbd_event_handler(xenfb_key_event, in);
     return 0;
 }
 
@@ -383,6 +383,11 @@ static void input_connected(struct XenDevice *xendev)
     in->qmouse = qemu_add_mouse_event_handler(xenfb_mouse_event, in,
 					      in->abs_pointer_wanted,
 					      "Xen PVFB Mouse");
+
+    if (in->qkbd) {
+        qemu_remove_kbd_event_handler(in->qkbd);
+    }
+    in->qkbd = qemu_add_kbd_event_handler(xenfb_key_event, in);
 }
 
 static void input_disconnect(struct XenDevice *xendev)
@@ -393,7 +398,10 @@ static void input_disconnect(struct XenDevice *xendev)
 	qemu_remove_mouse_event_handler(in->qmouse);
 	in->qmouse = NULL;
     }
-    qemu_add_kbd_event_handler(NULL, NULL);
+    if (in->qkbd) {
+        qemu_remove_kbd_event_handler(in->qkbd);
+        in->qkbd = NULL;
+    }
     common_unbind(&in->c);
 }
 
-- 
2.1.0

[Qemu-devel] [PATCH v2 2/5] xenfb: Activate mouse event handler

[Owen smith]

Without activating the mouse event handler, mouse events are not
delivered to the new handler. Input events are only delivered to
the first matching handler in the input chain. Activating a handler
moves the handler to the start of the chain.

Note: qemu_add_kbd_event_handler adds and activates the keyboard
handler, where qemu_add_mouse_event_handler does not activate the
mouse handler.

Signed-off-by: Owen smith <owen.smith@citrix.com>
---
 hw/display/xenfb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
index 9dcf9b6..69471e9 100644
--- a/hw/display/xenfb.c
+++ b/hw/display/xenfb.c
@@ -383,6 +383,7 @@ static void input_connected(struct XenDevice *xendev)
     in->qmouse = qemu_add_mouse_event_handler(xenfb_mouse_event, in,
 					      in->abs_pointer_wanted,
 					      "Xen PVFB Mouse");
+    qemu_activate_mouse_event_handler(in->qmouse);
 
     if (in->qkbd) {
         qemu_remove_kbd_event_handler(in->qkbd);
-- 
2.1.0

[Qemu-devel] [PATCH v2 3/5] xenfb: Add option to use a grant ref for shared page

[Owen smith]

Adds "page-gref" to the frontend location to specify the grant
reference of the shared page. Adds the DEVOPS_FLAG_NEED_GNTDEV to
both vfb and vkbd device flags. "page-ref" is checked first to
avoid breaking existing frontends.

"page-gref"
    Value: <uint32_t>
    Grant reference to use to map the shared ring. Only used if
    "page-ref" is not set.

Signed-off-by: Owen smith <owen.smith@citrix.com>
---
 hw/display/xenfb.c | 44 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
index 69471e9..829036f 100644
--- a/hw/display/xenfb.c
+++ b/hw/display/xenfb.c
@@ -54,6 +54,7 @@
 struct common {
     struct XenDevice  xendev;  /* must be first */
     void              *page;
+    int               page_gref;
     QemuConsole       *con;
 };
 
@@ -96,22 +97,38 @@ static int common_bind(struct common *c)
 {
     uint64_t mfn;
 
-    if (xenstore_read_fe_uint64(&c->xendev, "page-ref", &mfn) == -1)
-	return -1;
-    assert(mfn == (xen_pfn_t)mfn);
+    if (xenstore_read_fe_uint64(&c->xendev, "page-ref", &mfn) == -1) {
+        if (xenstore_read_fe_int(&c->xendev, "page-gref", &c->page_gref) == -1) {
+            return -1;
+        }
+    }
 
     if (xenstore_read_fe_int(&c->xendev, "event-channel", &c->xendev.remote_port) == -1)
 	return -1;
 
-    c->page = xc_map_foreign_range(xen_xc, c->xendev.dom,
-				   XC_PAGE_SIZE,
-				   PROT_READ | PROT_WRITE, mfn);
+    if (c->page_gref) {
+        c->page = xc_gnttab_map_grant_ref(c->xendev.gnttabdev,
+                                          c->xendev.dom, c->page_gref,
+                                          PROT_READ | PROT_WRITE);
+    } else {
+        assert(mfn == (xen_pfn_t)mfn);
+        c->page = xc_map_foreign_range(xen_xc, c->xendev.dom,
+                                       XC_PAGE_SIZE,
+                                       PROT_READ | PROT_WRITE, mfn);
+    }
     if (c->page == NULL)
 	return -1;
 
     xen_be_bind_evtchn(&c->xendev);
-    xen_be_printf(&c->xendev, 1, "ring mfn %"PRIx64", remote-port %d, local-port %d\n",
-		  mfn, c->xendev.remote_port, c->xendev.local_port);
+    if (c->page_gref) {
+        xen_be_printf(&c->xendev, 1,
+                      "ring gref %d, remote-port %d, local-port %d\n",
+                      c->page_gref, c->xendev.remote_port, c->xendev.local_port);
+    } else {
+        xen_be_printf(&c->xendev, 1,
+                      "ring mfn %"PRIx64", remote-port %d, local-port %d\n",
+                      mfn, c->xendev.remote_port, c->xendev.local_port);
+    }
 
     return 0;
 }
@@ -120,8 +137,13 @@ static void common_unbind(struct common *c)
 {
     xen_be_unbind_evtchn(&c->xendev);
     if (c->page) {
-	munmap(c->page, XC_PAGE_SIZE);
-	c->page = NULL;
+        if (c->page_gref) {
+            xc_gnttab_munmap(c->xendev.gnttabdev, c->page, 1);
+        } else {
+            munmap(c->page, XC_PAGE_SIZE);
+        }
+        c->page = NULL;
+        c->page_gref = 0;
     }
 }
 
@@ -954,6 +976,7 @@ static void fb_event(struct XenDevice *xendev)
 
 struct XenDevOps xen_kbdmouse_ops = {
     .size       = sizeof(struct XenInput),
+    .flags      = DEVOPS_FLAG_NEED_GNTDEV,
     .init       = input_init,
     .initialise = input_initialise,
     .connected  = input_connected,
@@ -963,6 +986,7 @@ struct XenDevOps xen_kbdmouse_ops = {
 
 struct XenDevOps xen_framebuffer_ops = {
     .size       = sizeof(struct XenFB),
+    .flags      = DEVOPS_FLAG_NEED_GNTDEV,
     .init       = fb_init,
     .initialise = fb_initialise,
     .disconnect = fb_disconnect,
-- 
2.1.0

[Qemu-devel] [PATCH v2 4/5] xenfb: Add "feature-no-abs-rescale"

[Owen smith]

Some frontends may require absolute axes that are not scaled to
DisplaySurface sizes.

backend: feature-no-abs-rescale
    Value: 0/1 (boolean)
    Default: 0

    Indicates whether backend supports unscaled absolute axes. Unscaled
    axes are in the range [0, 0x7fff].

frontend: request-no-abs-rescale
    Value: 0/1 (boolean)
    Default: 0

    When set to 1, frontend is requesting unscaled absolute axes.
    Unscaled absolute axes are only reported when absolute axes are used
    (when "request-abs-pointer" is also set to "1"). Frontend sets
    this value before connecting.

Signed-off-by: Owen smith <owen.smith@citrix.com>
---
 hw/display/xenfb.c | 44 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
index 829036f..0794075 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;     /* Guest wants unscaled absolute axes */
     int button_state;       /* Last seen pointer button state */
     int extended;
     QEMUPutKbdEntry *qkbd;
@@ -329,6 +330,26 @@ static void xenfb_key_event(void *opaque, int scancode)
     xenfb_send_key(xenfb, down, scancode2linux[scancode]);
 }
 
+static void xenfb_mouse_rescale(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
  *
@@ -342,18 +363,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
-	xenfb_send_motion(xenfb, dx, dy, dz);
+    if (xenfb->abs_pointer_wanted) {
+        if (!xenfb->no_abs_rescale) {
+            xenfb_mouse_rescale(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);
@@ -370,6 +389,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;
 }
 
@@ -398,6 +418,10 @@ static void input_connected(struct XenDevice *xendev)
                              &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;
+    }
 
     if (in->qmouse) {
         qemu_remove_mouse_event_handler(in->qmouse);
-- 
2.1.0

[Qemu-devel] [PATCH v2 5/5] xenfb: Add "feature-no-console"

[Owen smith]

Added feature to allow vkbd connection without waiting for vfb device.
Windows PV frontend requires connection without a vfb device, Ubuntu HVM
would not start correctly without the wait for vfb device.

backend: feature-no-console
    Value: 0/1 (boolean)
    Default Value: 0

    A value of "1" indicates that the backend supports vkbd devices
    without waiting for the corresponding vfb device. Frontends
    set "request-no-console" before connect to allow the backend to
    connect without waiting for the vfb device.

frontend: request-no-console
    Value: 0/1 (boolean)
    Default Value: 0

    A frontend sets a value of "1" to prevent the backend waiting for
    the vfb device to become available and attach to a console.

Signed-off-by: Owen smith <owen.smith@citrix.com>
---
 hw/display/xenfb.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
index 0794075..501fe72 100644
--- a/hw/display/xenfb.c
+++ b/hw/display/xenfb.c
@@ -390,6 +390,7 @@ 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);
+    xenstore_write_be_int(xendev, "feature-no-console", 1);
     return 0;
 }
 
@@ -399,8 +400,15 @@ static int input_initialise(struct XenDevice *xendev)
     int rc;
 
     if (!in->c.con) {
-        xen_be_printf(xendev, 1, "ds not set (yet)\n");
-        return -1;
+        int no_console;
+        rc = xenstore_read_fe_int(xendev, "request-no-console", &no_console);
+        if (rc == -1) {
+            no_console = 0;
+        }
+        if (!no_console) {
+            xen_be_printf(xendev, 1, "ds not set (yet)\n");
+            return -1;
+        }
     }
 
     rc = common_bind(&in->c);
-- 
2.1.0

Re: [Qemu-devel] [Xen-devel] [PATCH v2 3/5] xenfb: Add option to use a grant ref for shared page

[Ian Campbell]

On Wed, 2014-09-17 at 15:30 +0100, Owen smith wrote:
> Adds "page-gref" to the frontend location to specify the grant

xen.git/xen/include/public/io/fbif.h is a bit sparse in terms of docs
right now, but could I encourage you to add the new keys which you are
implementing in this series to it please?

(If you were minded to also document the other existing ones at the same
time that would be super-awesome(tm)).

Ian.

Re: [Qemu-devel] [PATCH v2 1/5] xenfb: Unregister keyboard event handler correctly

[Stefano Stabellini]

On Wed, 17 Sep 2014, Owen smith wrote:
> The keyboard event handler was not being removed, a NULL-callback
> entry was being added to intercept events. Use the unregister call
> to remove the keyboard event handler when appropriate.
> 
> Signed-off-by: Owen smith <owen.smith@citrix.com>

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>


>  hw/display/xenfb.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
> index 07ddc9d..9dcf9b6 100644
> --- a/hw/display/xenfb.c
> +++ b/hw/display/xenfb.c
> @@ -62,6 +62,7 @@ struct XenInput {
>      int abs_pointer_wanted; /* Whether guest supports absolute pointer */
>      int button_state;       /* Last seen pointer button state */
>      int extended;
> +    QEMUPutKbdEntry *qkbd;
>      QEMUPutMouseEntry *qmouse;
>  };
>  
> @@ -364,7 +365,6 @@ static int input_initialise(struct XenDevice *xendev)
>      if (rc != 0)
>  	return rc;
>  
> -    qemu_add_kbd_event_handler(xenfb_key_event, in);
>      return 0;
>  }
>  
> @@ -383,6 +383,11 @@ static void input_connected(struct XenDevice *xendev)
>      in->qmouse = qemu_add_mouse_event_handler(xenfb_mouse_event, in,
>  					      in->abs_pointer_wanted,
>  					      "Xen PVFB Mouse");
> +
> +    if (in->qkbd) {
> +        qemu_remove_kbd_event_handler(in->qkbd);
> +    }
> +    in->qkbd = qemu_add_kbd_event_handler(xenfb_key_event, in);
>  }
>  
>  static void input_disconnect(struct XenDevice *xendev)
> @@ -393,7 +398,10 @@ static void input_disconnect(struct XenDevice *xendev)
>  	qemu_remove_mouse_event_handler(in->qmouse);
>  	in->qmouse = NULL;
>      }
> -    qemu_add_kbd_event_handler(NULL, NULL);
> +    if (in->qkbd) {
> +        qemu_remove_kbd_event_handler(in->qkbd);
> +        in->qkbd = NULL;
> +    }
>      common_unbind(&in->c);
>  }
>  
> -- 
> 2.1.0
> 

Re: [Qemu-devel] [PATCH v2 2/5] xenfb: Activate mouse event handler

[Stefano Stabellini]

On Wed, 17 Sep 2014, Owen smith wrote:
> Without activating the mouse event handler, mouse events are not
> delivered to the new handler. Input events are only delivered to
> the first matching handler in the input chain. Activating a handler
> moves the handler to the start of the chain.
> 
> Note: qemu_add_kbd_event_handler adds and activates the keyboard
> handler, where qemu_add_mouse_event_handler does not activate the
> mouse handler.

how strange


> Signed-off-by: Owen smith <owen.smith@citrix.com>

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>


> ---
>  hw/display/xenfb.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
> index 9dcf9b6..69471e9 100644
> --- a/hw/display/xenfb.c
> +++ b/hw/display/xenfb.c
> @@ -383,6 +383,7 @@ static void input_connected(struct XenDevice *xendev)
>      in->qmouse = qemu_add_mouse_event_handler(xenfb_mouse_event, in,
>  					      in->abs_pointer_wanted,
>  					      "Xen PVFB Mouse");
> +    qemu_activate_mouse_event_handler(in->qmouse);
>  
>      if (in->qkbd) {
>          qemu_remove_kbd_event_handler(in->qkbd);
> -- 
> 2.1.0
> 

Re: [Qemu-devel] [PATCH v2 3/5] xenfb: Add option to use a grant ref for shared page

[Stefano Stabellini]

On Wed, 17 Sep 2014, Owen smith wrote:
> Adds "page-gref" to the frontend location to specify the grant
> reference of the shared page. Adds the DEVOPS_FLAG_NEED_GNTDEV to
> both vfb and vkbd device flags. "page-ref" is checked first to
> avoid breaking existing frontends.
> 
> "page-gref"
>     Value: <uint32_t>
>     Grant reference to use to map the shared ring. Only used if
>     "page-ref" is not set.
> 
> Signed-off-by: Owen smith <owen.smith@citrix.com>

As Ian pointed out, the docs need to be updated before I am going to
push this change upstream.
Aside from that

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>


>  hw/display/xenfb.c | 44 ++++++++++++++++++++++++++++++++++----------
>  1 file changed, 34 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
> index 69471e9..829036f 100644
> --- a/hw/display/xenfb.c
> +++ b/hw/display/xenfb.c
> @@ -54,6 +54,7 @@
>  struct common {
>      struct XenDevice  xendev;  /* must be first */
>      void              *page;
> +    int               page_gref;
>      QemuConsole       *con;
>  };
>  
> @@ -96,22 +97,38 @@ static int common_bind(struct common *c)
>  {
>      uint64_t mfn;
>  
> -    if (xenstore_read_fe_uint64(&c->xendev, "page-ref", &mfn) == -1)
> -	return -1;
> -    assert(mfn == (xen_pfn_t)mfn);
> +    if (xenstore_read_fe_uint64(&c->xendev, "page-ref", &mfn) == -1) {
> +        if (xenstore_read_fe_int(&c->xendev, "page-gref", &c->page_gref) == -1) {
> +            return -1;
> +        }
> +    }
>  
>      if (xenstore_read_fe_int(&c->xendev, "event-channel", &c->xendev.remote_port) == -1)
>  	return -1;
>  
> -    c->page = xc_map_foreign_range(xen_xc, c->xendev.dom,
> -				   XC_PAGE_SIZE,
> -				   PROT_READ | PROT_WRITE, mfn);
> +    if (c->page_gref) {
> +        c->page = xc_gnttab_map_grant_ref(c->xendev.gnttabdev,
> +                                          c->xendev.dom, c->page_gref,
> +                                          PROT_READ | PROT_WRITE);
> +    } else {
> +        assert(mfn == (xen_pfn_t)mfn);
> +        c->page = xc_map_foreign_range(xen_xc, c->xendev.dom,
> +                                       XC_PAGE_SIZE,
> +                                       PROT_READ | PROT_WRITE, mfn);
> +    }
>      if (c->page == NULL)
>  	return -1;
>  
>      xen_be_bind_evtchn(&c->xendev);
> -    xen_be_printf(&c->xendev, 1, "ring mfn %"PRIx64", remote-port %d, local-port %d\n",
> -		  mfn, c->xendev.remote_port, c->xendev.local_port);
> +    if (c->page_gref) {
> +        xen_be_printf(&c->xendev, 1,
> +                      "ring gref %d, remote-port %d, local-port %d\n",
> +                      c->page_gref, c->xendev.remote_port, c->xendev.local_port);
> +    } else {
> +        xen_be_printf(&c->xendev, 1,
> +                      "ring mfn %"PRIx64", remote-port %d, local-port %d\n",
> +                      mfn, c->xendev.remote_port, c->xendev.local_port);
> +    }
>  
>      return 0;
>  }
> @@ -120,8 +137,13 @@ static void common_unbind(struct common *c)
>  {
>      xen_be_unbind_evtchn(&c->xendev);
>      if (c->page) {
> -	munmap(c->page, XC_PAGE_SIZE);
> -	c->page = NULL;
> +        if (c->page_gref) {
> +            xc_gnttab_munmap(c->xendev.gnttabdev, c->page, 1);
> +        } else {
> +            munmap(c->page, XC_PAGE_SIZE);
> +        }
> +        c->page = NULL;
> +        c->page_gref = 0;
>      }
>  }
>  
> @@ -954,6 +976,7 @@ static void fb_event(struct XenDevice *xendev)
>  
>  struct XenDevOps xen_kbdmouse_ops = {
>      .size       = sizeof(struct XenInput),
> +    .flags      = DEVOPS_FLAG_NEED_GNTDEV,
>      .init       = input_init,
>      .initialise = input_initialise,
>      .connected  = input_connected,
> @@ -963,6 +986,7 @@ struct XenDevOps xen_kbdmouse_ops = {
>  
>  struct XenDevOps xen_framebuffer_ops = {
>      .size       = sizeof(struct XenFB),
> +    .flags      = DEVOPS_FLAG_NEED_GNTDEV,
>      .init       = fb_init,
>      .initialise = fb_initialise,
>      .disconnect = fb_disconnect,
> -- 
> 2.1.0
> 

Re: [Qemu-devel] [PATCH v2 4/5] xenfb: Add "feature-no-abs-rescale"

[Stefano Stabellini]

On Wed, 17 Sep 2014, Owen smith wrote:
> Some frontends may require absolute axes that are not scaled to
> DisplaySurface sizes.
> 
> backend: feature-no-abs-rescale
>     Value: 0/1 (boolean)
>     Default: 0
> 
>     Indicates whether backend supports unscaled absolute axes. Unscaled
>     axes are in the range [0, 0x7fff].
> 
> frontend: request-no-abs-rescale
>     Value: 0/1 (boolean)
>     Default: 0
> 
>     When set to 1, frontend is requesting unscaled absolute axes.
>     Unscaled absolute axes are only reported when absolute axes are used
>     (when "request-abs-pointer" is also set to "1"). Frontend sets
>     this value before connecting.
> 
> Signed-off-by: Owen smith <owen.smith@citrix.com>

Docs need to be updated before I send this upstream. However:

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>


>  hw/display/xenfb.c | 44 ++++++++++++++++++++++++++++++++++----------
>  1 file changed, 34 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
> index 829036f..0794075 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;     /* Guest wants unscaled absolute axes */
>      int button_state;       /* Last seen pointer button state */
>      int extended;
>      QEMUPutKbdEntry *qkbd;
> @@ -329,6 +330,26 @@ static void xenfb_key_event(void *opaque, int scancode)
>      xenfb_send_key(xenfb, down, scancode2linux[scancode]);
>  }
>  
> +static void xenfb_mouse_rescale(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
>   *
> @@ -342,18 +363,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
> -	xenfb_send_motion(xenfb, dx, dy, dz);
> +    if (xenfb->abs_pointer_wanted) {
> +        if (!xenfb->no_abs_rescale) {
> +            xenfb_mouse_rescale(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);
> @@ -370,6 +389,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;
>  }
>  
> @@ -398,6 +418,10 @@ static void input_connected(struct XenDevice *xendev)
>                               &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;
> +    }
>  
>      if (in->qmouse) {
>          qemu_remove_mouse_event_handler(in->qmouse);
> -- 
> 2.1.0
> 

Re: [Qemu-devel] [PATCH v2 5/5] xenfb: Add "feature-no-console"

[Stefano Stabellini]

On Wed, 17 Sep 2014, Owen smith wrote:
> Added feature to allow vkbd connection without waiting for vfb device.
> Windows PV frontend requires connection without a vfb device, Ubuntu HVM
> would not start correctly without the wait for vfb device.

This is a good description of the problem. Could you please add few more
words on why Ubuntu HVM would not start without the wait? If the
frontend prints an error, please include that too.


> backend: feature-no-console

This is a pretty bad name for this feature. I suggest
feature-stand-alone or feature-no-vfb.

>     Value: 0/1 (boolean)
>     Default Value: 0
> 
>     A value of "1" indicates that the backend supports vkbd devices
>     without waiting for the corresponding vfb device. Frontends
>     set "request-no-console" before connect to allow the backend to
>     connect without waiting for the vfb device.
> 
> frontend: request-no-console
>     Value: 0/1 (boolean)
>     Default Value: 0
> 
>     A frontend sets a value of "1" to prevent the backend waiting for
>     the vfb device to become available and attach to a console.
> 
> Signed-off-by: Owen smith <owen.smith@citrix.com>
> 
>  hw/display/xenfb.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
> index 0794075..501fe72 100644
> --- a/hw/display/xenfb.c
> +++ b/hw/display/xenfb.c
> @@ -390,6 +390,7 @@ 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);
> +    xenstore_write_be_int(xendev, "feature-no-console", 1);
>      return 0;
>  }
>  
> @@ -399,8 +400,15 @@ static int input_initialise(struct XenDevice *xendev)
>      int rc;
>  
>      if (!in->c.con) {
> -        xen_be_printf(xendev, 1, "ds not set (yet)\n");
> -        return -1;
> +        int no_console;
> +        rc = xenstore_read_fe_int(xendev, "request-no-console", &no_console);
> +        if (rc == -1) {
> +            no_console = 0;
> +        }
> +        if (!no_console) {
> +            xen_be_printf(xendev, 1, "ds not set (yet)\n");
> +            return -1;
> +        }
>      }
>      rc = common_bind(&in->c);
> -- 
> 2.1.0
>