SUZUKI Kazuhiro
2008-Feb-08 08:42 UTC
[Xen-devel] [PATCH]Fix a restored domain cannot use mouse and keyboard
Hi all, I found a bug that a restored domain could not use mouse and keyboard, when the mouse is moved while saving the domain. The following patch fixes it. Thanks. KAZ Signed-off-by: Kazuhiro Suzuki <kaz@jp.fujitsu.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Markus Armbruster
2008-Feb-08 15:17 UTC
Re: [Xen-devel] [PATCH]Fix a restored domain cannot use mouse and keyboard
SUZUKI Kazuhiro <kaz@jp.fujitsu.com> writes:> Hi all, > > I found a bug that a restored domain could not use mouse and > keyboard, when the mouse is moved while saving the domain. > > The following patch fixes it. > > Thanks. > KAZYou didn''t tell us what exactly went wrong, so I can only guess. I guess that when the ring buffer fills up completely during save, then xenkbd_resume() finds it full, and as long as it remains full, no further events go in, and no notifications go to the event channel. As long as the (new) backend doesn''t get a notification on its event channel, it doesn''t take out anything, so the ring buffer remains full. Deadlock. Is that correct? The fix loses the contents of the ring buffer. I guess that''s tolerable. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
SUZUKI Kazuhiro
2008-Feb-12 01:24 UTC
Re: [Xen-devel] [PATCH]Fix a restored domain cannot use mouse and keyboard
Hi, Sorry for my late reply. Qemu generates(produces) events to the event channel but no event on it are processed during save. When a domain is restored, pending events and selectors are cleared as follows in tools/libxc/xc_domain_restore.c. 1125 /* clear any pending events and the selector */ 1126 MEMSET_ARRAY_FIELD(new_shared_info, evtchn_pending, 0); 1127 for ( i = 0; i < MAX_VIRT_CPUS; i++ ) 1128 SET_FIELD(new_shared_info, vcpu_info[i].evtchn_pending_sel, 0); So it processes(consumes) no pending event on the event channel and page->in_cons doesn''t increase. Qemu has the following code in tools/ioemu/hw/xenfb.c. 568 prod = page->in_prod; 569 if (prod - page->in_cons == XENKBD_IN_RING_LEN) { 570 errno = EAGAIN; 571 return -1; 572 } If the ring buffer fills up to (prod - page->in_cons =XENKBD_IN_RING_LEN), then no event notifications go to the event channel. Thanks. KAZ From: Markus Armbruster <armbru@redhat.com> Subject: Re: [Xen-devel] [PATCH]Fix a restored domain cannot use mouse and keyboard Date: Fri, 08 Feb 2008 16:17:01 +0100> SUZUKI Kazuhiro <kaz@jp.fujitsu.com> writes: > > > Hi all, > > > > I found a bug that a restored domain could not use mouse and > > keyboard, when the mouse is moved while saving the domain. > > > > The following patch fixes it. > > > > Thanks. > > KAZ > > You didn''t tell us what exactly went wrong, so I can only guess. I > guess that when the ring buffer fills up completely during save, then > xenkbd_resume() finds it full, and as long as it remains full, no > further events go in, and no notifications go to the event channel. > As long as the (new) backend doesn''t get a notification on its event > channel, it doesn''t take out anything, so the ring buffer remains > full. Deadlock. > > Is that correct? > > The fix loses the contents of the ring buffer. I guess that''s > tolerable. > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Markus Armbruster
2008-Feb-12 09:22 UTC
Re: [Xen-devel] [PATCH]Fix a restored domain cannot use mouse and keyboard
SUZUKI Kazuhiro <kaz@jp.fujitsu.com> writes:> Hi, > > Sorry for my late reply. > > Qemu generates(produces) events to the event channel but no event on > it are processed during save. > > When a domain is restored, pending events and selectors are cleared > as follows in tools/libxc/xc_domain_restore.c. > > 1125 /* clear any pending events and the selector */ > 1126 MEMSET_ARRAY_FIELD(new_shared_info, evtchn_pending, 0); > 1127 for ( i = 0; i < MAX_VIRT_CPUS; i++ ) > 1128 SET_FIELD(new_shared_info, vcpu_info[i].evtchn_pending_sel, 0); > > So it processes(consumes) no pending event on the event channel and > page->in_cons doesn''t increase. > > Qemu has the following code in tools/ioemu/hw/xenfb.c. > > 568 prod = page->in_prod; > 569 if (prod - page->in_cons == XENKBD_IN_RING_LEN) { > 570 errno = EAGAIN; > 571 return -1; > 572 } > > If the ring buffer fills up to (prod - page->in_cons => XENKBD_IN_RING_LEN), then no event notifications go to the event > channel. > > Thanks. > KAZLet me paraphrase to make sure I understand: * Backend (qemu) produces events into xenkbd in ring and signals frontend through the event channel. * Frontend (vkbd driver) consumes events from xenkbd ring when it gets signalled through the event channel. * At some time, the frontend stops doing that, and the ring starts filling up. The event is pending to signal the ring is not empty. * The frontend''s domain is restored, and any pending events are cleared. If the event was pending, we''re now in an unhealthy state: the ring buffer is not empty, but the event doesn''t signal that. The event remains clear until the backend produces or consumes another event. Since the frontend never produces events, the latter can''t happen. If the ring buffer is completely full when the event gets cleared, the former can''t happen until the frontend consumes an event. * The frontend resumes. It consumes events from xenkbd ring when it gets signalled through the event channel. If the ring buffer was completely full when the event got cleared on restore, we''re deadlocked: the backend can''t produce until the frontend consumed, and the frontend doesn''t consume until the backend produced. Is this correct? I''m not sure your fix is completely safe. Once the frontend exposed the shared page to the backend, it is not supposed to touch in_prod and out_cons, and it may only increase in_cons and out_prod. I think you better do xenkbd_disconnect_backend(info); + info->page->in_cons = info->page->in_prod; return xenkbd_connect_backend(dev, info); Instead of discarding in events, you could perhaps run input_handler() to consume them. Not sure that makes sense. You simply can''t discard out events here safely. Academic, since the xenkbd frontend never produces any. Note that xenfb.c has exactly the same problems. But there, the in event bug doesn''t bite, because the backend never produces any. It should be fixed anyway. And the out event issue is not academic, as xenfb frontend does produce them. It''s an issue only if the event signalling ring buffer not empty to the *backend* gets cleared on restore. Does it? _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2008-Feb-12 10:09 UTC
Re: [Xen-devel] [PATCH]Fix a restored domain cannot use mouse and keyboard
On 12/2/08 09:22, "Markus Armbruster" <armbru@redhat.com> wrote:> I''m not sure your fix is completely safe. Once the frontend exposed > the shared page to the backend, it is not supposed to touch in_prod > and out_cons, and it may only increase in_cons and out_prod. I think > you better do > > xenkbd_disconnect_backend(info); > + info->page->in_cons = info->page->in_prod; > return xenkbd_connect_backend(dev, info);The page is not exposed to the new backend until xenkbd_connect_backend(). So the originally proposed fix is correct in this regard, I think. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel