Re: [Qemu-devel] [PATCH 2/2] qxl: add QXL_IO_UPDATE_MEM for guest S3&S4 support

[yhalperi]

On 06/26/2011 08:47 PM, Alon Levy wrote:
> On Sun, Jun 26, 2011 at 12:59:06PM -0400, Yonit Halperin wrote:
> > Sorry for the late response, wasn't available.
> > I'm afraid  that (1) and (2) will indeed wakeup the worker, but will not assure 
> > emptying the command ring, as it depends on the client pipe size.
>
> I actually can't figure out what wakeup does (that's what both NOTIFY and
> NOTIFY_CURSOR do, see hw/qxl.c).
It turns on an event the worker is waiting for on epoll_wait.

What we did in prepare_to_sleep before was
> call flush_all_qxl_commands, which reads the command and cursor rings until
> they are empty (calling flush_cursor_commands and flush_display_commands), 
> waiting
> whenever the pipe is too large. (avoiding this wait still needs to be done, but
> after ensuring suspend is correct).
>
> More to the point this flush is done from handle_dev_destroy_surfaces, but
> this is not good enough since this also destroys the surfaces, before we have
> a chance to actually render the surfaces.
>
> Perhaps RED_WORKER_MESSAGE_STOP should do a flush_all_qxl_commands?
We can do it as long as it doesn't affect migration - does STOP happens 
before or after savevm? If it happens after - we can't touch the command 
ring, i.e., we can't call flush. And even if it happens before - do we 
really want to call flush during migration and presumably slow it down?

Cheers,
Yonit.

> > ----- Original Message -----
> > From: "Alon Levy"<address@hidden>
> > To: "Gerd Hoffmann"<address@hidden>
> > Cc: address@hidden, address@hidden
> > Sent: Wednesday, June 22, 2011 11:57:54 AM
> > Subject: Re: [Qemu-devel] [PATCH 2/2] qxl: add QXL_IO_UPDATE_MEM for guest 
> > S3&S4 support
> >
> > On Wed, Jun 22, 2011 at 11:13:19AM +0200, Gerd Hoffmann wrote:
> > >    Hi,
> > >
> > > > > worker call.  We can add a I/O command to ask qxl to push the
> > > > > release queue head to the release ring.
> > > >
> > > > So you suggest to replace QXL_IO_UPDATE_MEM with what, two io commands instead
> > > > of using the val parameter?
> > >
> > > I'd like to (a) avoid updating the libspice-server API if possible
> > > and (b) have one I/O command for each logical step.  So going into
> > > S3 could look like this:
> > >
> > >    (0) stop putting new commands into the rings
> > >    (1) QXL_IO_NOTIFY_CMD
> > >    (2) QXL_IO_NOTIFY_CURSOR
> > >        qxl calls notify(), to make the worker thread empty the command
> > >        rings before it processes the next dispatcher request.
> > >    (3) QXl_IO_FLUSH_SURFACES (to be implemented)
> > >        qxl calls stop()+start(), spice-server renders all surfaces,
> > >        thereby flushing state to device memory.
> > >    (4) QXL_IO_DESTROY_ALL_SURFACES
> > >        zap surfaces
> > >    (5) QXL_IO_FLUSH_RELEASE (to be implemented)
> > >        push release queue head into the release ring, so the guest
> > >        will see it and can release everything.
> > >
> > > (1)+(2)+(4) exist already.
> > > (3)+(5) can be done without libspice-server changes.
> > >
> > > Looks workable?
> >
> > yeah. Yonit?
> >
> > > cheers,
> > >    Gerd