* [Patch 0/2] Fix request-abs-pointer (again)
@ 2010-07-07 13:40 John Haxby
2010-07-07 13:40 ` [PATCH 1/2] Introduce a new 'connected' xendev op called when Connected John Haxby
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: John Haxby @ 2010-07-07 13:40 UTC (permalink / raw)
To: xen-devel
A little while ago, Jeremy Fitzhardinge has a problem that was preventing
request-abs-pointer from being honoured. A simple fix for this in
805ed3b20492d2f4bb465bfda65cedd286e23209 actually turned out to be wrong.
Konrad Rzeszutek Wilk and Stefano Stabellini suggested a different
approach: introduce a new hook in xen_backend that, if defined, is called
when the frontend is connected. The first of these two patches does just
that (the old connect hook is now called "initialise" and the new hook is
called "connected"). The second patch uses this to set up the pointer at
the right time. I haven't been able to test this in a Xen 4.0
environment, but it works properly in a Xen 3.4 environment.
jch
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] Introduce a new 'connected' xendev op called when Connected. 2010-07-07 13:40 [Patch 0/2] Fix request-abs-pointer (again) John Haxby @ 2010-07-07 13:40 ` John Haxby 2010-07-07 16:40 ` Stefano Stabellini 2010-07-07 13:40 ` [PATCH 2/2] Move the xenfb pointer handler to the connected method John Haxby 2010-07-07 15:55 ` [Patch 0/2] Fix request-abs-pointer (again) Konrad Rzeszutek Wilk 2 siblings, 1 reply; 6+ messages in thread From: John Haxby @ 2010-07-07 13:40 UTC (permalink / raw) To: xen-devel; +Cc: John Haxby Rename the existing xendev 'connect' op to 'initialised' and introduce a new 'connected' op. This new op, if defined, is called when the backend is connected. Note that since there is no state transition this may be called more than once. Signed-off-by: John Haxby <john.haxby@oracle.com> --- hw/xen_backend.c | 39 +++++++++++++++++++++++++++++++++------ hw/xen_backend.h | 3 ++- hw/xen_console.c | 4 ++-- hw/xenfb.c | 8 ++++---- 4 files changed, 41 insertions(+), 13 deletions(-) diff --git a/hw/xen_backend.c b/hw/xen_backend.c index 8c92739..afd0bf2 100644 --- a/hw/xen_backend.c +++ b/hw/xen_backend.c @@ -401,13 +401,13 @@ static int xen_be_try_init(struct XenDevice *xendev) } /* - * Try to connect xendev. Depends on the frontend being ready + * Try to initialise xendev. Depends on the frontend being ready * for it (shared ring and evtchn info in xenstore, state being * Initialised or Connected). * * Goes to Connected on success. */ -static int xen_be_try_connect(struct XenDevice *xendev) +static int xen_be_try_initialise(struct XenDevice *xendev) { int rc = 0; @@ -421,10 +421,10 @@ static int xen_be_try_connect(struct XenDevice *xendev) } } - if (xendev->ops->connect) - rc = xendev->ops->connect(xendev); + if (xendev->ops->initialise) + rc = xendev->ops->initialise(xendev); if (rc != 0) { - xen_be_printf(xendev, 0, "connect() failed\n"); + xen_be_printf(xendev, 0, "initialise() failed\n"); return rc; } @@ -433,6 +433,28 @@ static int xen_be_try_connect(struct XenDevice *xendev) } /* + * Try to let xendev know that it is connected. Depends on the + * frontend being Connected. Note that this may be called more + * than once since the backend state is not modified. + */ +static void xen_be_try_connected(struct XenDevice *xendev) +{ + if (!xendev->ops->connected) + return; + + if (xendev->fe_state != XenbusStateConnected) { + if (xendev->ops->flags & DEVOPS_FLAG_IGNORE_STATE) { + xen_be_printf(xendev, 2, "frontend not ready, ignoring\n"); + } else { + xen_be_printf(xendev, 2, "frontend not ready (yet)\n"); + return; + } + } + + xendev->ops->connected(xendev); +} + +/* * Teardown connection. * * Goes to Closed when done. @@ -484,7 +506,12 @@ void xen_be_check_state(struct XenDevice *xendev) rc = xen_be_try_init(xendev); break; case XenbusStateInitWait: - rc = xen_be_try_connect(xendev); + rc = xen_be_try_initialise(xendev); + break; + case XenbusStateConnected: + /* xendev->be_state doesn't change */ + xen_be_try_connected(xendev); + rc = -1; break; case XenbusStateClosed: rc = xen_be_try_reset(xendev); diff --git a/hw/xen_backend.h b/hw/xen_backend.h index 672a857..7e89ef4 100644 --- a/hw/xen_backend.h +++ b/hw/xen_backend.h @@ -19,7 +19,8 @@ struct XenDevOps { uint32_t flags; void (*alloc)(struct XenDevice *xendev); int (*init)(struct XenDevice *xendev); - int (*connect)(struct XenDevice *xendev); + int (*initialise)(struct XenDevice *xendev); + void (*connected)(struct XenDevice *xendev); void (*event)(struct XenDevice *xendev); void (*disconnect)(struct XenDevice *xendev); int (*free)(struct XenDevice *xendev); diff --git a/hw/xen_console.c b/hw/xen_console.c index f1c2f8b..a4e97ca 100644 --- a/hw/xen_console.c +++ b/hw/xen_console.c @@ -209,7 +209,7 @@ static int con_init(struct XenDevice *xendev) return 0; } -static int con_connect(struct XenDevice *xendev) +static int con_initialise(struct XenDevice *xendev) { struct XenConsole *con = container_of(xendev, struct XenConsole, xendev); int limit; @@ -278,7 +278,7 @@ struct XenDevOps xen_console_ops = { .size = sizeof(struct XenConsole), .flags = DEVOPS_FLAG_IGNORE_STATE|DEVOPS_FLAG_NEED_GNTDEV, .init = con_init, - .connect = con_connect, + .initialise = con_initialise, .event = con_event, .disconnect = con_disconnect, }; diff --git a/hw/xenfb.c b/hw/xenfb.c index 97960ba..c96cfe6 100644 --- a/hw/xenfb.c +++ b/hw/xenfb.c @@ -361,7 +361,7 @@ static int input_init(struct XenDevice *xendev) return 0; } -static int input_connect(struct XenDevice *xendev) +static int input_initialise(struct XenDevice *xendev) { struct XenInput *in = container_of(xendev, struct XenInput, c.xendev); int rc; @@ -863,7 +863,7 @@ static int fb_init(struct XenDevice *xendev) return 0; } -static int fb_connect(struct XenDevice *xendev) +static int fb_initialise(struct XenDevice *xendev) { struct XenFB *fb = container_of(xendev, struct XenFB, c.xendev); struct xenfb_page *fb_page; @@ -957,7 +957,7 @@ static void fb_event(struct XenDevice *xendev) struct XenDevOps xen_kbdmouse_ops = { .size = sizeof(struct XenInput), .init = input_init, - .connect = input_connect, + .initialise = input_initialise, .disconnect = input_disconnect, .event = input_event, }; @@ -965,7 +965,7 @@ struct XenDevOps xen_kbdmouse_ops = { struct XenDevOps xen_framebuffer_ops = { .size = sizeof(struct XenFB), .init = fb_init, - .connect = fb_connect, + .initialise = fb_initialise, .disconnect = fb_disconnect, .event = fb_event, .frontend_changed = fb_frontend_changed, -- 1.7.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Introduce a new 'connected' xendev op called when Connected. 2010-07-07 13:40 ` [PATCH 1/2] Introduce a new 'connected' xendev op called when Connected John Haxby @ 2010-07-07 16:40 ` Stefano Stabellini 0 siblings, 0 replies; 6+ messages in thread From: Stefano Stabellini @ 2010-07-07 16:40 UTC (permalink / raw) To: John Haxby; +Cc: xen-devel@lists.xensource.com, Gerd Hoffmann CC'ing Gerd. On Wed, 7 Jul 2010, John Haxby wrote: > Rename the existing xendev 'connect' op to 'initialised' and introduce > a new 'connected' op. This new op, if defined, is called when the > backend is connected. Note that since there is no state transition this > may be called more than once. > > Signed-off-by: John Haxby <john.haxby@oracle.com> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > hw/xen_backend.c | 39 +++++++++++++++++++++++++++++++++------ > hw/xen_backend.h | 3 ++- > hw/xen_console.c | 4 ++-- > hw/xenfb.c | 8 ++++---- > 4 files changed, 41 insertions(+), 13 deletions(-) > > diff --git a/hw/xen_backend.c b/hw/xen_backend.c > index 8c92739..afd0bf2 100644 > --- a/hw/xen_backend.c > +++ b/hw/xen_backend.c > @@ -401,13 +401,13 @@ static int xen_be_try_init(struct XenDevice *xendev) > } > > /* > - * Try to connect xendev. Depends on the frontend being ready > + * Try to initialise xendev. Depends on the frontend being ready > * for it (shared ring and evtchn info in xenstore, state being > * Initialised or Connected). > * > * Goes to Connected on success. > */ > -static int xen_be_try_connect(struct XenDevice *xendev) > +static int xen_be_try_initialise(struct XenDevice *xendev) > { > int rc = 0; > > @@ -421,10 +421,10 @@ static int xen_be_try_connect(struct XenDevice *xendev) > } > } > > - if (xendev->ops->connect) > - rc = xendev->ops->connect(xendev); > + if (xendev->ops->initialise) > + rc = xendev->ops->initialise(xendev); > if (rc != 0) { > - xen_be_printf(xendev, 0, "connect() failed\n"); > + xen_be_printf(xendev, 0, "initialise() failed\n"); > return rc; > } > > @@ -433,6 +433,28 @@ static int xen_be_try_connect(struct XenDevice *xendev) > } > > /* > + * Try to let xendev know that it is connected. Depends on the > + * frontend being Connected. Note that this may be called more > + * than once since the backend state is not modified. > + */ > +static void xen_be_try_connected(struct XenDevice *xendev) > +{ > + if (!xendev->ops->connected) > + return; > + > + if (xendev->fe_state != XenbusStateConnected) { > + if (xendev->ops->flags & DEVOPS_FLAG_IGNORE_STATE) { > + xen_be_printf(xendev, 2, "frontend not ready, ignoring\n"); > + } else { > + xen_be_printf(xendev, 2, "frontend not ready (yet)\n"); > + return; > + } > + } > + > + xendev->ops->connected(xendev); > +} > + > +/* > * Teardown connection. > * > * Goes to Closed when done. > @@ -484,7 +506,12 @@ void xen_be_check_state(struct XenDevice *xendev) > rc = xen_be_try_init(xendev); > break; > case XenbusStateInitWait: > - rc = xen_be_try_connect(xendev); > + rc = xen_be_try_initialise(xendev); > + break; > + case XenbusStateConnected: > + /* xendev->be_state doesn't change */ > + xen_be_try_connected(xendev); > + rc = -1; > break; > case XenbusStateClosed: > rc = xen_be_try_reset(xendev); > diff --git a/hw/xen_backend.h b/hw/xen_backend.h > index 672a857..7e89ef4 100644 > --- a/hw/xen_backend.h > +++ b/hw/xen_backend.h > @@ -19,7 +19,8 @@ struct XenDevOps { > uint32_t flags; > void (*alloc)(struct XenDevice *xendev); > int (*init)(struct XenDevice *xendev); > - int (*connect)(struct XenDevice *xendev); > + int (*initialise)(struct XenDevice *xendev); > + void (*connected)(struct XenDevice *xendev); > void (*event)(struct XenDevice *xendev); > void (*disconnect)(struct XenDevice *xendev); > int (*free)(struct XenDevice *xendev); > diff --git a/hw/xen_console.c b/hw/xen_console.c > index f1c2f8b..a4e97ca 100644 > --- a/hw/xen_console.c > +++ b/hw/xen_console.c > @@ -209,7 +209,7 @@ static int con_init(struct XenDevice *xendev) > return 0; > } > > -static int con_connect(struct XenDevice *xendev) > +static int con_initialise(struct XenDevice *xendev) > { > struct XenConsole *con = container_of(xendev, struct XenConsole, xendev); > int limit; > @@ -278,7 +278,7 @@ struct XenDevOps xen_console_ops = { > .size = sizeof(struct XenConsole), > .flags = DEVOPS_FLAG_IGNORE_STATE|DEVOPS_FLAG_NEED_GNTDEV, > .init = con_init, > - .connect = con_connect, > + .initialise = con_initialise, > .event = con_event, > .disconnect = con_disconnect, > }; > diff --git a/hw/xenfb.c b/hw/xenfb.c > index 97960ba..c96cfe6 100644 > --- a/hw/xenfb.c > +++ b/hw/xenfb.c > @@ -361,7 +361,7 @@ static int input_init(struct XenDevice *xendev) > return 0; > } > > -static int input_connect(struct XenDevice *xendev) > +static int input_initialise(struct XenDevice *xendev) > { > struct XenInput *in = container_of(xendev, struct XenInput, c.xendev); > int rc; > @@ -863,7 +863,7 @@ static int fb_init(struct XenDevice *xendev) > return 0; > } > > -static int fb_connect(struct XenDevice *xendev) > +static int fb_initialise(struct XenDevice *xendev) > { > struct XenFB *fb = container_of(xendev, struct XenFB, c.xendev); > struct xenfb_page *fb_page; > @@ -957,7 +957,7 @@ static void fb_event(struct XenDevice *xendev) > struct XenDevOps xen_kbdmouse_ops = { > .size = sizeof(struct XenInput), > .init = input_init, > - .connect = input_connect, > + .initialise = input_initialise, > .disconnect = input_disconnect, > .event = input_event, > }; > @@ -965,7 +965,7 @@ struct XenDevOps xen_kbdmouse_ops = { > struct XenDevOps xen_framebuffer_ops = { > .size = sizeof(struct XenFB), > .init = fb_init, > - .connect = fb_connect, > + .initialise = fb_initialise, > .disconnect = fb_disconnect, > .event = fb_event, > .frontend_changed = fb_frontend_changed, > -- > 1.7.1 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] Move the xenfb pointer handler to the connected method 2010-07-07 13:40 [Patch 0/2] Fix request-abs-pointer (again) John Haxby 2010-07-07 13:40 ` [PATCH 1/2] Introduce a new 'connected' xendev op called when Connected John Haxby @ 2010-07-07 13:40 ` John Haxby 2010-07-07 16:41 ` Stefano Stabellini 2010-07-07 15:55 ` [Patch 0/2] Fix request-abs-pointer (again) Konrad Rzeszutek Wilk 2 siblings, 1 reply; 6+ messages in thread From: John Haxby @ 2010-07-07 13:40 UTC (permalink / raw) To: xen-devel; +Cc: John Haxby Ensure that we read "request-abs-pointer" after the frontend has written it. This means that we will correctly set up an ansolute or relative pointer handler correctly. Signed-off-by: John Haxby <john.haxby@oracle.com> --- hw/xenfb.c | 19 ++++++++++++++----- 1 files changed, 14 insertions(+), 5 deletions(-) diff --git a/hw/xenfb.c b/hw/xenfb.c index c96cfe6..29a873f 100644 --- a/hw/xenfb.c +++ b/hw/xenfb.c @@ -366,19 +366,27 @@ static int input_initialise(struct XenDevice *xendev) struct XenInput *in = container_of(xendev, struct XenInput, c.xendev); int rc; - if (xenstore_read_fe_int(xendev, "request-abs-pointer", - &in->abs_pointer_wanted) == -1) - in->abs_pointer_wanted = 0; - rc = common_bind(&in->c); if (rc != 0) return rc; qemu_add_kbd_event_handler(xenfb_key_event, in); + return 0; +} + +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); in->qmouse = qemu_add_mouse_event_handler(xenfb_mouse_event, in, in->abs_pointer_wanted, "Xen PVFB Mouse"); - return 0; } static void input_disconnect(struct XenDevice *xendev) @@ -958,6 +966,7 @@ struct XenDevOps xen_kbdmouse_ops = { .size = sizeof(struct XenInput), .init = input_init, .initialise = input_initialise, + .connected = input_connected, .disconnect = input_disconnect, .event = input_event, }; -- 1.7.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] Move the xenfb pointer handler to the connected method 2010-07-07 13:40 ` [PATCH 2/2] Move the xenfb pointer handler to the connected method John Haxby @ 2010-07-07 16:41 ` Stefano Stabellini 0 siblings, 0 replies; 6+ messages in thread From: Stefano Stabellini @ 2010-07-07 16:41 UTC (permalink / raw) To: John Haxby; +Cc: xen-devel@lists.xensource.com, Gerd Hoffmann CC'ing Gerd. On Wed, 7 Jul 2010, John Haxby wrote: > Ensure that we read "request-abs-pointer" after the frontend has written > it. This means that we will correctly set up an ansolute or relative > pointer handler correctly. > > Signed-off-by: John Haxby <john.haxby@oracle.com> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > hw/xenfb.c | 19 ++++++++++++++----- > 1 files changed, 14 insertions(+), 5 deletions(-) > > diff --git a/hw/xenfb.c b/hw/xenfb.c > index c96cfe6..29a873f 100644 > --- a/hw/xenfb.c > +++ b/hw/xenfb.c > @@ -366,19 +366,27 @@ static int input_initialise(struct XenDevice *xendev) > struct XenInput *in = container_of(xendev, struct XenInput, c.xendev); > int rc; > > - if (xenstore_read_fe_int(xendev, "request-abs-pointer", > - &in->abs_pointer_wanted) == -1) > - in->abs_pointer_wanted = 0; > - > rc = common_bind(&in->c); > if (rc != 0) > return rc; > > qemu_add_kbd_event_handler(xenfb_key_event, in); > + return 0; > +} > + > +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); > in->qmouse = qemu_add_mouse_event_handler(xenfb_mouse_event, in, > in->abs_pointer_wanted, > "Xen PVFB Mouse"); > - return 0; > } > > static void input_disconnect(struct XenDevice *xendev) > @@ -958,6 +966,7 @@ struct XenDevOps xen_kbdmouse_ops = { > .size = sizeof(struct XenInput), > .init = input_init, > .initialise = input_initialise, > + .connected = input_connected, > .disconnect = input_disconnect, > .event = input_event, > }; > -- > 1.7.1 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch 0/2] Fix request-abs-pointer (again) 2010-07-07 13:40 [Patch 0/2] Fix request-abs-pointer (again) John Haxby 2010-07-07 13:40 ` [PATCH 1/2] Introduce a new 'connected' xendev op called when Connected John Haxby 2010-07-07 13:40 ` [PATCH 2/2] Move the xenfb pointer handler to the connected method John Haxby @ 2010-07-07 15:55 ` Konrad Rzeszutek Wilk 2 siblings, 0 replies; 6+ messages in thread From: Konrad Rzeszutek Wilk @ 2010-07-07 15:55 UTC (permalink / raw) To: John Haxby; +Cc: xen-devel On Wed, Jul 07, 2010 at 02:40:00PM +0100, John Haxby wrote: > A little while ago, Jeremy Fitzhardinge has a problem that was preventing > request-abs-pointer from being honoured. A simple fix for this in > 805ed3b20492d2f4bb465bfda65cedd286e23209 actually turned out to be wrong. > > Konrad Rzeszutek Wilk and Stefano Stabellini suggested a different > approach: introduce a new hook in xen_backend that, if defined, is called > when the frontend is connected. The first of these two patches does just > that (the old connect hook is now called "initialise" and the new hook is > called "connected"). The second patch uses this to set up the pointer at > the right time. I haven't been able to test this in a Xen 4.0 > environment, but it works properly in a Xen 3.4 environment. I tested them by: 1). Reverting c/s 21260 and 21651 from Xen unstable 2). Applied both of your patches on top of add968aaf68cb57257428f8cfadb209f2614a6d8 and tested the pv-grub launch with success. I did not test the HVM stubdomain launch. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-07-07 16:41 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-07-07 13:40 [Patch 0/2] Fix request-abs-pointer (again) John Haxby 2010-07-07 13:40 ` [PATCH 1/2] Introduce a new 'connected' xendev op called when Connected John Haxby 2010-07-07 16:40 ` Stefano Stabellini 2010-07-07 13:40 ` [PATCH 2/2] Move the xenfb pointer handler to the connected method John Haxby 2010-07-07 16:41 ` Stefano Stabellini 2010-07-07 15:55 ` [Patch 0/2] Fix request-abs-pointer (again) Konrad Rzeszutek Wilk
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).