* [Qemu-devel] [PATCH 0/3] xenfb: Add support for Windows PV frontend @ 2014-09-10 13:42 ` Owen smith 0 siblings, 0 replies; 26+ messages in thread From: Owen smith @ 2014-09-10 13:42 UTC (permalink / raw) To: qemu-devel; +Cc: stefano.stabellini, Owen smith, xen-devel 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 tablet device enabled, and any unneccessary polling associated with the USB devices. This patch series contains the following patches 1-2) Fixes to the vkbd device model 3) Adds a feature to not scale axes to DisplaySurface size Owen smith (3): xenfb: Unregister keyboard event handler correctly xenfb: Add option to use grant ref instead of mfn for shared page. xenfb: Add "feature-no-abs-rescale" for Windows PV frontend hw/display/xenfb.c | 104 +++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 77 insertions(+), 27 deletions(-) -- 2.1.0 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 0/3] xenfb: Add support for Windows PV frontend @ 2014-09-10 13:42 ` Owen smith 0 siblings, 0 replies; 26+ messages in thread From: Owen smith @ 2014-09-10 13:42 UTC (permalink / raw) To: qemu-devel; +Cc: stefano.stabellini, Owen smith, xen-devel 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 tablet device enabled, and any unneccessary polling associated with the USB devices. This patch series contains the following patches 1-2) Fixes to the vkbd device model 3) Adds a feature to not scale axes to DisplaySurface size Owen smith (3): xenfb: Unregister keyboard event handler correctly xenfb: Add option to use grant ref instead of mfn for shared page. xenfb: Add "feature-no-abs-rescale" for Windows PV frontend hw/display/xenfb.c | 104 +++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 77 insertions(+), 27 deletions(-) -- 2.1.0 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 1/3] xenfb: Unregister keyboard event handler correctly 2014-09-10 13:42 ` Owen smith @ 2014-09-10 13:42 ` Owen smith -1 siblings, 0 replies; 26+ messages in thread From: Owen smith @ 2014-09-10 13:42 UTC (permalink / raw) To: qemu-devel; +Cc: stefano.stabellini, Owen smith, xen-devel Keyboard event handler was replaced with a new handler on disconnect. Use the unregister function to remove keyboard handler. Signed-off-by: Owen smith <owen.smith@citrix.com> --- hw/display/xenfb.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c index 07ddc9d..2c39753 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,17 +383,26 @@ 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); + } + in->qkbd = qemu_add_kbd_event_handler(xenfb_key_event, in); } static void input_disconnect(struct XenDevice *xendev) { struct XenInput *in = container_of(xendev, struct XenInput, c.xendev); + if (in->qkbd) { + qemu_remove_kbd_event_handler(in->qkbd); + in->qkbd = NULL; + } if (in->qmouse) { qemu_remove_mouse_event_handler(in->qmouse); in->qmouse = NULL; } - qemu_add_kbd_event_handler(NULL, NULL); common_unbind(&in->c); } -- 2.1.0 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 1/3] xenfb: Unregister keyboard event handler correctly @ 2014-09-10 13:42 ` Owen smith 0 siblings, 0 replies; 26+ messages in thread From: Owen smith @ 2014-09-10 13:42 UTC (permalink / raw) To: qemu-devel; +Cc: stefano.stabellini, Owen smith, xen-devel Keyboard event handler was replaced with a new handler on disconnect. Use the unregister function to remove keyboard handler. Signed-off-by: Owen smith <owen.smith@citrix.com> --- hw/display/xenfb.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c index 07ddc9d..2c39753 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,17 +383,26 @@ 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); + } + in->qkbd = qemu_add_kbd_event_handler(xenfb_key_event, in); } static void input_disconnect(struct XenDevice *xendev) { struct XenInput *in = container_of(xendev, struct XenInput, c.xendev); + if (in->qkbd) { + qemu_remove_kbd_event_handler(in->qkbd); + in->qkbd = NULL; + } if (in->qmouse) { qemu_remove_mouse_event_handler(in->qmouse); in->qmouse = NULL; } - qemu_add_kbd_event_handler(NULL, NULL); common_unbind(&in->c); } -- 2.1.0 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 1/3] xenfb: Unregister keyboard event handler correctly 2014-09-10 13:42 ` Owen smith (?) @ 2014-09-11 0:33 ` Stefano Stabellini -1 siblings, 0 replies; 26+ messages in thread From: Stefano Stabellini @ 2014-09-11 0:33 UTC (permalink / raw) To: Owen smith; +Cc: stefano.stabellini, qemu-devel, xen-devel On Wed, 10 Sep 2014, Owen smith wrote: > Keyboard event handler was replaced with a new handler on disconnect. > Use the unregister function to remove keyboard handler. > > Signed-off-by: Owen smith <owen.smith@citrix.com> > --- > hw/display/xenfb.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c > index 07ddc9d..2c39753 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,17 +383,26 @@ 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); This change is not described in the commit message. Why is this necessary? Is it related to the keyboard change? > + 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) > { > struct XenInput *in = container_of(xendev, struct XenInput, c.xendev); > > + if (in->qkbd) { > + qemu_remove_kbd_event_handler(in->qkbd); > + in->qkbd = NULL; > + } > if (in->qmouse) { > qemu_remove_mouse_event_handler(in->qmouse); > in->qmouse = NULL; > } > - qemu_add_kbd_event_handler(NULL, NULL); > common_unbind(&in->c); > } > > -- > 2.1.0 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] xenfb: Unregister keyboard event handler correctly 2014-09-10 13:42 ` Owen smith (?) (?) @ 2014-09-11 0:33 ` Stefano Stabellini 2014-09-11 11:32 ` Owen Smith 2014-09-11 11:32 ` [Qemu-devel] " Owen Smith -1 siblings, 2 replies; 26+ messages in thread From: Stefano Stabellini @ 2014-09-11 0:33 UTC (permalink / raw) To: Owen smith; +Cc: stefano.stabellini, qemu-devel, xen-devel On Wed, 10 Sep 2014, Owen smith wrote: > Keyboard event handler was replaced with a new handler on disconnect. > Use the unregister function to remove keyboard handler. > > Signed-off-by: Owen smith <owen.smith@citrix.com> > --- > hw/display/xenfb.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c > index 07ddc9d..2c39753 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,17 +383,26 @@ 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); This change is not described in the commit message. Why is this necessary? Is it related to the keyboard change? > + 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) > { > struct XenInput *in = container_of(xendev, struct XenInput, c.xendev); > > + if (in->qkbd) { > + qemu_remove_kbd_event_handler(in->qkbd); > + in->qkbd = NULL; > + } > if (in->qmouse) { > qemu_remove_mouse_event_handler(in->qmouse); > in->qmouse = NULL; > } > - qemu_add_kbd_event_handler(NULL, NULL); > common_unbind(&in->c); > } > > -- > 2.1.0 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/3] xenfb: Unregister keyboard event handler correctly 2014-09-11 0:33 ` [Qemu-devel] " Stefano Stabellini @ 2014-09-11 11:32 ` Owen Smith 2014-09-11 11:32 ` [Qemu-devel] " Owen Smith 1 sibling, 0 replies; 26+ messages in thread From: Owen Smith @ 2014-09-11 11:32 UTC (permalink / raw) Cc: Stefano Stabellini, qemu-devel, xen-devel > -----Original Message----- > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > Sent: 11 September 2014 01:33 > To: Owen Smith > Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org; Stefano Stabellini > Subject: Re: [PATCH 1/3] xenfb: Unregister keyboard event handler correctly > > On Wed, 10 Sep 2014, Owen smith wrote: > > Keyboard event handler was replaced with a new handler on disconnect. > > Use the unregister function to remove keyboard handler. > > > > Signed-off-by: Owen smith <owen.smith@citrix.com> > > --- > > hw/display/xenfb.c | 13 +++++++++++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c index > > 07ddc9d..2c39753 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,17 +383,26 @@ 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); > > This change is not described in the commit message. > Why is this necessary? Is it related to the keyboard change? > I'll split this change into a separate patch for v2. Without this, the xenfb_mouse_event callback was not being called in response to mouse events, as events are only delivered to the first matching handler in the input chain. Calling qemu_add_kbd_event_handler activates the handler internally, but qemu_add_mouse_event_handler does not activate the handler. > > > + 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) { > > struct XenInput *in = container_of(xendev, struct XenInput, > > c.xendev); > > > > + if (in->qkbd) { > > + qemu_remove_kbd_event_handler(in->qkbd); > > + in->qkbd = NULL; > > + } > > if (in->qmouse) { > > qemu_remove_mouse_event_handler(in->qmouse); > > in->qmouse = NULL; > > } > > - qemu_add_kbd_event_handler(NULL, NULL); > > common_unbind(&in->c); > > } > > > > -- > > 2.1.0 > > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] xenfb: Unregister keyboard event handler correctly 2014-09-11 0:33 ` [Qemu-devel] " Stefano Stabellini 2014-09-11 11:32 ` Owen Smith @ 2014-09-11 11:32 ` Owen Smith 1 sibling, 0 replies; 26+ messages in thread From: Owen Smith @ 2014-09-11 11:32 UTC (permalink / raw) To: Stefano Stabellini; +Cc: qemu-devel, xen-devel > -----Original Message----- > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > Sent: 11 September 2014 01:33 > To: Owen Smith > Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org; Stefano Stabellini > Subject: Re: [PATCH 1/3] xenfb: Unregister keyboard event handler correctly > > On Wed, 10 Sep 2014, Owen smith wrote: > > Keyboard event handler was replaced with a new handler on disconnect. > > Use the unregister function to remove keyboard handler. > > > > Signed-off-by: Owen smith <owen.smith@citrix.com> > > --- > > hw/display/xenfb.c | 13 +++++++++++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c index > > 07ddc9d..2c39753 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,17 +383,26 @@ 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); > > This change is not described in the commit message. > Why is this necessary? Is it related to the keyboard change? > I'll split this change into a separate patch for v2. Without this, the xenfb_mouse_event callback was not being called in response to mouse events, as events are only delivered to the first matching handler in the input chain. Calling qemu_add_kbd_event_handler activates the handler internally, but qemu_add_mouse_event_handler does not activate the handler. > > > + 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) { > > struct XenInput *in = container_of(xendev, struct XenInput, > > c.xendev); > > > > + if (in->qkbd) { > > + qemu_remove_kbd_event_handler(in->qkbd); > > + in->qkbd = NULL; > > + } > > if (in->qmouse) { > > qemu_remove_mouse_event_handler(in->qmouse); > > in->qmouse = NULL; > > } > > - qemu_add_kbd_event_handler(NULL, NULL); > > common_unbind(&in->c); > > } > > > > -- > > 2.1.0 > > ^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 2/3] xenfb: Add option to use grant ref for shared page. 2014-09-10 13:42 ` Owen smith @ 2014-09-10 13:42 ` Owen smith -1 siblings, 0 replies; 26+ messages in thread From: Owen smith @ 2014-09-10 13:42 UTC (permalink / raw) To: qemu-devel; +Cc: stefano.stabellini, Owen smith, xen-devel The vkbd device should be able to use a grant reference to map the shared page. Signed-off-by: Owen smith <owen.smith@citrix.com> --- hw/display/xenfb.c | 39 +++++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c index 2c39753..c4375c8 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,36 @@ 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_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 (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; + } + 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 +135,13 @@ static void common_unbind(struct common *c) { xen_be_unbind_evtchn(&c->xendev); if (c->page) { - munmap(c->page, XC_PAGE_SIZE); + 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 +974,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, -- 2.1.0 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 2/3] xenfb: Add option to use grant ref for shared page. @ 2014-09-10 13:42 ` Owen smith 0 siblings, 0 replies; 26+ messages in thread From: Owen smith @ 2014-09-10 13:42 UTC (permalink / raw) To: qemu-devel; +Cc: stefano.stabellini, Owen smith, xen-devel The vkbd device should be able to use a grant reference to map the shared page. Signed-off-by: Owen smith <owen.smith@citrix.com> --- hw/display/xenfb.c | 39 +++++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c index 2c39753..c4375c8 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,36 @@ 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_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 (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; + } + 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 +135,13 @@ static void common_unbind(struct common *c) { xen_be_unbind_evtchn(&c->xendev); if (c->page) { - munmap(c->page, XC_PAGE_SIZE); + 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 +974,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, -- 2.1.0 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 2/3] xenfb: Add option to use grant ref for shared page. 2014-09-10 13:42 ` Owen smith (?) @ 2014-09-11 1:01 ` Stefano Stabellini -1 siblings, 0 replies; 26+ messages in thread From: Stefano Stabellini @ 2014-09-11 1:01 UTC (permalink / raw) To: Owen smith; +Cc: stefano.stabellini, qemu-devel, xen-devel On Wed, 10 Sep 2014, Owen smith wrote: > The vkbd device should be able to use a grant reference to map the > shared page. > > Signed-off-by: Owen smith <owen.smith@citrix.com> > --- > hw/display/xenfb.c | 39 +++++++++++++++++++++++++++++---------- > 1 file changed, 29 insertions(+), 10 deletions(-) > > diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c > index 2c39753..c4375c8 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,36 @@ 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_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 (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; > + } > + 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); > + } This is an extension of the protocol. It should be documented somewhere. At least in the commit message and somewhere under docs/misc or xen/include/public/io/fbif.h. > 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 +135,13 @@ static void common_unbind(struct common *c) > { > xen_be_unbind_evtchn(&c->xendev); > if (c->page) { > - munmap(c->page, XC_PAGE_SIZE); > + 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 +974,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, > -- > 2.1.0 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] xenfb: Add option to use grant ref for shared page. 2014-09-10 13:42 ` Owen smith (?) (?) @ 2014-09-11 1:01 ` Stefano Stabellini 2014-09-11 11:33 ` Owen Smith 2014-09-11 11:33 ` Owen Smith -1 siblings, 2 replies; 26+ messages in thread From: Stefano Stabellini @ 2014-09-11 1:01 UTC (permalink / raw) To: Owen smith; +Cc: stefano.stabellini, qemu-devel, xen-devel On Wed, 10 Sep 2014, Owen smith wrote: > The vkbd device should be able to use a grant reference to map the > shared page. > > Signed-off-by: Owen smith <owen.smith@citrix.com> > --- > hw/display/xenfb.c | 39 +++++++++++++++++++++++++++++---------- > 1 file changed, 29 insertions(+), 10 deletions(-) > > diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c > index 2c39753..c4375c8 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,36 @@ 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_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 (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; > + } > + 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); > + } This is an extension of the protocol. It should be documented somewhere. At least in the commit message and somewhere under docs/misc or xen/include/public/io/fbif.h. > 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 +135,13 @@ static void common_unbind(struct common *c) > { > xen_be_unbind_evtchn(&c->xendev); > if (c->page) { > - munmap(c->page, XC_PAGE_SIZE); > + 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 +974,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, > -- > 2.1.0 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] xenfb: Add option to use grant ref for shared page. 2014-09-11 1:01 ` [Qemu-devel] " Stefano Stabellini @ 2014-09-11 11:33 ` Owen Smith 2014-09-11 18:01 ` Stefano Stabellini 2014-09-11 18:01 ` [Qemu-devel] " Stefano Stabellini 2014-09-11 11:33 ` Owen Smith 1 sibling, 2 replies; 26+ messages in thread From: Owen Smith @ 2014-09-11 11:33 UTC (permalink / raw) To: Stefano Stabellini; +Cc: qemu-devel, xen-devel > -----Original Message----- > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > Sent: 11 September 2014 02:01 > To: Owen Smith > Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org; Stefano Stabellini > Subject: Re: [PATCH 2/3] xenfb: Add option to use grant ref for shared page. > > On Wed, 10 Sep 2014, Owen smith wrote: > > The vkbd device should be able to use a grant reference to map the > > shared page. > > > > Signed-off-by: Owen smith <owen.smith@citrix.com> > > --- > > hw/display/xenfb.c | 39 +++++++++++++++++++++++++++++---------- > > 1 file changed, 29 insertions(+), 10 deletions(-) > > > > diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c index > > 2c39753..c4375c8 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,36 @@ 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_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 (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; > > + } > > + 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); > > + } > > This is an extension of the protocol. It should be documented somewhere. > At least in the commit message and somewhere under docs/misc or > xen/include/public/io/fbif.h. > V2 will contain a better commit message, and I'll add a patch to the headers to document the changes. The same protocol extension docs should be in xen/include/public/io/fbif.h and xen/include/public/io/kdbif.h, as this change covers the vfb and vkbd, or would it be better to change the patch to only affect the vkbd? > > > 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 +135,13 @@ static void common_unbind(struct common *c) { > > xen_be_unbind_evtchn(&c->xendev); > > if (c->page) { > > - munmap(c->page, XC_PAGE_SIZE); > > + 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 +974,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, > > -- > > 2.1.0 > > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/3] xenfb: Add option to use grant ref for shared page. 2014-09-11 11:33 ` Owen Smith @ 2014-09-11 18:01 ` Stefano Stabellini 2014-09-11 18:01 ` [Qemu-devel] " Stefano Stabellini 1 sibling, 0 replies; 26+ messages in thread From: Stefano Stabellini @ 2014-09-11 18:01 UTC (permalink / raw) To: Owen Smith; +Cc: Stefano Stabellini, qemu-devel, xen-devel On Thu, 11 Sep 2014, Owen Smith wrote: > > -----Original Message----- > > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > > Sent: 11 September 2014 02:01 > > To: Owen Smith > > Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org; Stefano Stabellini > > Subject: Re: [PATCH 2/3] xenfb: Add option to use grant ref for shared page. > > > > On Wed, 10 Sep 2014, Owen smith wrote: > > > The vkbd device should be able to use a grant reference to map the > > > shared page. > > > > > > Signed-off-by: Owen smith <owen.smith@citrix.com> > > > --- > > > hw/display/xenfb.c | 39 +++++++++++++++++++++++++++++---------- > > > 1 file changed, 29 insertions(+), 10 deletions(-) > > > > > > diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c index > > > 2c39753..c4375c8 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,36 @@ 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_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 (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; > > > + } > > > + 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); > > > + } > > > > This is an extension of the protocol. It should be documented somewhere. > > At least in the commit message and somewhere under docs/misc or > > xen/include/public/io/fbif.h. > > > > V2 will contain a better commit message, and I'll add a patch to the headers > to document the changes. > The same protocol extension docs should be in xen/include/public/io/fbif.h > and xen/include/public/io/kdbif.h, as this change covers the vfb and vkbd, or > would it be better to change the patch to only affect the vkbd? I am OK with changing both vfb and vkbd but in that case you need to document both. > > > > > 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 +135,13 @@ static void common_unbind(struct common *c) { > > > xen_be_unbind_evtchn(&c->xendev); > > > if (c->page) { > > > - munmap(c->page, XC_PAGE_SIZE); > > > + 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 +974,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, > > > -- > > > 2.1.0 > > > > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] xenfb: Add option to use grant ref for shared page. 2014-09-11 11:33 ` Owen Smith 2014-09-11 18:01 ` Stefano Stabellini @ 2014-09-11 18:01 ` Stefano Stabellini 1 sibling, 0 replies; 26+ messages in thread From: Stefano Stabellini @ 2014-09-11 18:01 UTC (permalink / raw) To: Owen Smith; +Cc: Stefano Stabellini, qemu-devel, xen-devel On Thu, 11 Sep 2014, Owen Smith wrote: > > -----Original Message----- > > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > > Sent: 11 September 2014 02:01 > > To: Owen Smith > > Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org; Stefano Stabellini > > Subject: Re: [PATCH 2/3] xenfb: Add option to use grant ref for shared page. > > > > On Wed, 10 Sep 2014, Owen smith wrote: > > > The vkbd device should be able to use a grant reference to map the > > > shared page. > > > > > > Signed-off-by: Owen smith <owen.smith@citrix.com> > > > --- > > > hw/display/xenfb.c | 39 +++++++++++++++++++++++++++++---------- > > > 1 file changed, 29 insertions(+), 10 deletions(-) > > > > > > diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c index > > > 2c39753..c4375c8 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,36 @@ 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_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 (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; > > > + } > > > + 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); > > > + } > > > > This is an extension of the protocol. It should be documented somewhere. > > At least in the commit message and somewhere under docs/misc or > > xen/include/public/io/fbif.h. > > > > V2 will contain a better commit message, and I'll add a patch to the headers > to document the changes. > The same protocol extension docs should be in xen/include/public/io/fbif.h > and xen/include/public/io/kdbif.h, as this change covers the vfb and vkbd, or > would it be better to change the patch to only affect the vkbd? I am OK with changing both vfb and vkbd but in that case you need to document both. > > > > > 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 +135,13 @@ static void common_unbind(struct common *c) { > > > xen_be_unbind_evtchn(&c->xendev); > > > if (c->page) { > > > - munmap(c->page, XC_PAGE_SIZE); > > > + 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 +974,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, > > > -- > > > 2.1.0 > > > > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/3] xenfb: Add option to use grant ref for shared page. 2014-09-11 1:01 ` [Qemu-devel] " Stefano Stabellini 2014-09-11 11:33 ` Owen Smith @ 2014-09-11 11:33 ` Owen Smith 1 sibling, 0 replies; 26+ messages in thread From: Owen Smith @ 2014-09-11 11:33 UTC (permalink / raw) Cc: Stefano Stabellini, qemu-devel, xen-devel > -----Original Message----- > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > Sent: 11 September 2014 02:01 > To: Owen Smith > Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org; Stefano Stabellini > Subject: Re: [PATCH 2/3] xenfb: Add option to use grant ref for shared page. > > On Wed, 10 Sep 2014, Owen smith wrote: > > The vkbd device should be able to use a grant reference to map the > > shared page. > > > > Signed-off-by: Owen smith <owen.smith@citrix.com> > > --- > > hw/display/xenfb.c | 39 +++++++++++++++++++++++++++++---------- > > 1 file changed, 29 insertions(+), 10 deletions(-) > > > > diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c index > > 2c39753..c4375c8 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,36 @@ 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_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 (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; > > + } > > + 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); > > + } > > This is an extension of the protocol. It should be documented somewhere. > At least in the commit message and somewhere under docs/misc or > xen/include/public/io/fbif.h. > V2 will contain a better commit message, and I'll add a patch to the headers to document the changes. The same protocol extension docs should be in xen/include/public/io/fbif.h and xen/include/public/io/kdbif.h, as this change covers the vfb and vkbd, or would it be better to change the patch to only affect the vkbd? > > > 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 +135,13 @@ static void common_unbind(struct common *c) { > > xen_be_unbind_evtchn(&c->xendev); > > if (c->page) { > > - munmap(c->page, XC_PAGE_SIZE); > > + 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 +974,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, > > -- > > 2.1.0 > > ^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 3/3] xenfb: Add "feature-no-abs-rescale" for Windows PV frontend 2014-09-10 13:42 ` Owen smith @ 2014-09-10 13:42 ` Owen smith -1 siblings, 0 replies; 26+ messages in thread From: Owen smith @ 2014-09-10 13:42 UTC (permalink / raw) To: qemu-devel; +Cc: stefano.stabellini, Owen smith, xen-devel 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 <owen.smith@citrix.com> --- 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; + } + + if (!in->c.con && !in->no_abs_rescale) { 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 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 3/3] xenfb: Add "feature-no-abs-rescale" for Windows PV frontend @ 2014-09-10 13:42 ` Owen smith 0 siblings, 0 replies; 26+ messages in thread From: Owen smith @ 2014-09-10 13:42 UTC (permalink / raw) To: qemu-devel; +Cc: stefano.stabellini, Owen smith, xen-devel 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 <owen.smith@citrix.com> --- 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; + } + + if (!in->c.con && !in->no_abs_rescale) { 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 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] xenfb: Add "feature-no-abs-rescale" for Windows PV frontend 2014-09-10 13:42 ` Owen smith (?) @ 2014-09-11 1:23 ` Stefano Stabellini 2014-09-11 11:34 ` Owen Smith ` (2 more replies) -1 siblings, 3 replies; 26+ messages in thread From: Stefano Stabellini @ 2014-09-11 1:23 UTC (permalink / raw) To: Owen smith; +Cc: stefano.stabellini, qemu-devel, xen-devel 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 <owen.smith@citrix.com> > --- > 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. > + if (!in->c.con && !in->no_abs_rescale) { Why this change? > 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 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/3] xenfb: Add "feature-no-abs-rescale" for Windows PV frontend 2014-09-11 1:23 ` [Qemu-devel] " Stefano Stabellini @ 2014-09-11 11:34 ` Owen Smith 2014-09-11 11:34 ` [Qemu-devel] " Owen Smith 2014-09-29 12:20 ` Ian Campbell 2 siblings, 0 replies; 26+ messages in thread From: Owen Smith @ 2014-09-11 11:34 UTC (permalink / raw) Cc: Stefano Stabellini, qemu-devel, xen-devel > -----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 <owen.smith@citrix.com> > > --- > > 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. > > > + 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. > > > 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 > > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] xenfb: Add "feature-no-abs-rescale" for Windows PV frontend 2014-09-11 1:23 ` [Qemu-devel] " Stefano Stabellini 2014-09-11 11:34 ` Owen Smith @ 2014-09-11 11:34 ` Owen Smith 2014-09-11 18:04 ` Stefano Stabellini 2014-09-11 18:04 ` [Qemu-devel] " Stefano Stabellini 2014-09-29 12:20 ` Ian Campbell 2 siblings, 2 replies; 26+ messages in thread From: Owen Smith @ 2014-09-11 11:34 UTC (permalink / raw) To: Stefano Stabellini; +Cc: qemu-devel, xen-devel > -----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 <owen.smith@citrix.com> > > --- > > 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. > > > + 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. > > > 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 > > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/3] xenfb: Add "feature-no-abs-rescale" for Windows PV frontend 2014-09-11 11:34 ` [Qemu-devel] " Owen Smith @ 2014-09-11 18:04 ` Stefano Stabellini 2014-09-11 18:04 ` [Qemu-devel] " Stefano Stabellini 1 sibling, 0 replies; 26+ messages in thread From: Stefano Stabellini @ 2014-09-11 18:04 UTC (permalink / raw) To: Owen Smith; +Cc: Stefano Stabellini, qemu-devel, xen-devel 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 <owen.smith@citrix.com> > > > --- > > > 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 > > > > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] xenfb: Add "feature-no-abs-rescale" for Windows PV frontend 2014-09-11 11:34 ` [Qemu-devel] " Owen Smith 2014-09-11 18:04 ` Stefano Stabellini @ 2014-09-11 18:04 ` Stefano Stabellini 1 sibling, 0 replies; 26+ messages in thread From: Stefano Stabellini @ 2014-09-11 18:04 UTC (permalink / raw) To: Owen Smith; +Cc: Stefano Stabellini, qemu-devel, xen-devel 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 <owen.smith@citrix.com> > > > --- > > > 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 > > > > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [Xen-devel] [PATCH 3/3] xenfb: Add "feature-no-abs-rescale" for Windows PV frontend 2014-09-11 1:23 ` [Qemu-devel] " Stefano Stabellini @ 2014-09-29 12:20 ` Ian Campbell 2014-09-11 11:34 ` [Qemu-devel] " Owen Smith 2014-09-29 12:20 ` Ian Campbell 2 siblings, 0 replies; 26+ messages in thread From: Ian Campbell @ 2014-09-29 12:20 UTC (permalink / raw) To: Stefano Stabellini; +Cc: xen-devel, Owen smith, qemu-devel On Thu, 2014-09-11 at 02:23 +0100, Stefano Stabellini wrote: Stefano, In the context of this patch could you also take a look at Owen's xenfb documentation patches[0], I think you probably know the most about this particular protocol. Ian. [0] <1411376699-8175-1-git-send-email-owen.smith@citrix.com> and friends. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/3] xenfb: Add "feature-no-abs-rescale" for Windows PV frontend @ 2014-09-29 12:20 ` Ian Campbell 0 siblings, 0 replies; 26+ messages in thread From: Ian Campbell @ 2014-09-29 12:20 UTC (permalink / raw) To: Stefano Stabellini; +Cc: xen-devel, Owen smith, qemu-devel On Thu, 2014-09-11 at 02:23 +0100, Stefano Stabellini wrote: Stefano, In the context of this patch could you also take a look at Owen's xenfb documentation patches[0], I think you probably know the most about this particular protocol. Ian. [0] <1411376699-8175-1-git-send-email-owen.smith@citrix.com> and friends. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/3] xenfb: Add "feature-no-abs-rescale" for Windows PV frontend 2014-09-10 13:42 ` Owen smith (?) (?) @ 2014-09-11 1:23 ` Stefano Stabellini -1 siblings, 0 replies; 26+ messages in thread From: Stefano Stabellini @ 2014-09-11 1:23 UTC (permalink / raw) To: Owen smith; +Cc: stefano.stabellini, qemu-devel, xen-devel 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 <owen.smith@citrix.com> > --- > 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. > + if (!in->c.con && !in->no_abs_rescale) { Why this change? > 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 > ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2014-09-29 12:20 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-09-10 13:42 [Qemu-devel] [PATCH 0/3] xenfb: Add support for Windows PV frontend Owen smith 2014-09-10 13:42 ` Owen smith 2014-09-10 13:42 ` [Qemu-devel] [PATCH 1/3] xenfb: Unregister keyboard event handler correctly Owen smith 2014-09-10 13:42 ` Owen smith 2014-09-11 0:33 ` Stefano Stabellini 2014-09-11 0:33 ` [Qemu-devel] " Stefano Stabellini 2014-09-11 11:32 ` Owen Smith 2014-09-11 11:32 ` [Qemu-devel] " Owen Smith 2014-09-10 13:42 ` [Qemu-devel] [PATCH 2/3] xenfb: Add option to use grant ref for shared page Owen smith 2014-09-10 13:42 ` Owen smith 2014-09-11 1:01 ` Stefano Stabellini 2014-09-11 1:01 ` [Qemu-devel] " Stefano Stabellini 2014-09-11 11:33 ` Owen Smith 2014-09-11 18:01 ` Stefano Stabellini 2014-09-11 18:01 ` [Qemu-devel] " Stefano Stabellini 2014-09-11 11:33 ` Owen Smith 2014-09-10 13:42 ` [Qemu-devel] [PATCH 3/3] xenfb: Add "feature-no-abs-rescale" for Windows PV frontend Owen smith 2014-09-10 13:42 ` Owen smith 2014-09-11 1:23 ` [Qemu-devel] " Stefano Stabellini 2014-09-11 11:34 ` Owen Smith 2014-09-11 11:34 ` [Qemu-devel] " Owen Smith 2014-09-11 18:04 ` Stefano Stabellini 2014-09-11 18:04 ` [Qemu-devel] " Stefano Stabellini 2014-09-29 12:20 ` [Qemu-devel] [Xen-devel] " Ian Campbell 2014-09-29 12:20 ` Ian Campbell 2014-09-11 1:23 ` Stefano Stabellini
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.