Hi On Tue, Jun 4, 2024 at 9:49 PM Kim, Dongwon <dongwon....@intel.com> wrote:
> On 6/4/2024 4:12 AM, Marc-André Lureau wrote: > > Hi > > > > On Thu, May 30, 2024 at 2:44 AM <dongwon....@intel.com > > <mailto:dongwon....@intel.com>> wrote: > > > > From: Dongwon <dongwon....@intel.com <mailto:dongwon....@intel.com>> > > > > Make sure rendering of the current frame is finished before switching > > the run state to RUN_STATE_SAVE_VM by waiting for egl-sync object to > be > > signaled. > > > > > > Can you expand on what this solves? > > In current scheme, guest waits for the fence to be signaled for each > frame it submits before moving to the next frame. If the guest’s state > is saved while it is still waiting for the fence, The guest will > continue to wait for the fence that was signaled while ago when it is > restored to the point. One way to prevent it is to get it finish the > current frame before changing the state. > After the UI sets a fence, hw_ops->gl_block(true) gets called, which will block virtio-gpu/virgl from processing commands (until the fence is signaled and gl_block/false called again). But this "blocking" state is not saved. So how does this affect save/restore? Please give more details, thanks > > > > > > Cc: Marc-André Lureau <marcandre.lur...@redhat.com > > <mailto:marcandre.lur...@redhat.com>> > > Cc: Vivek Kasireddy <vivek.kasire...@intel.com > > <mailto:vivek.kasire...@intel.com>> > > Signed-off-by: Dongwon Kim <dongwon....@intel.com > > <mailto:dongwon....@intel.com>> > > --- > > ui/egl-helpers.c | 2 -- > > ui/gtk.c | 19 +++++++++++++++++++ > > 2 files changed, 19 insertions(+), 2 deletions(-) > > > > diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c > > index 99b2ebbe23..dafeb36074 100644 > > --- a/ui/egl-helpers.c > > +++ b/ui/egl-helpers.c > > @@ -396,8 +396,6 @@ void egl_dmabuf_create_fence(QemuDmaBuf *dmabuf) > > fence_fd = eglDupNativeFenceFDANDROID(qemu_egl_display, > > sync); > > qemu_dmabuf_set_fence_fd(dmabuf, fence_fd); > > - eglDestroySyncKHR(qemu_egl_display, sync); > > - qemu_dmabuf_set_sync(dmabuf, NULL); > > > > > > If this function is called multiple times, it will now set a new > > fence_fd each time, and potentially leak older fd. Maybe it could first > > check if a fence_fd exists instead. > > We can make that change. > > > > > } > > } > > > > diff --git a/ui/gtk.c b/ui/gtk.c > > index 93b13b7a30..cf0dd6abed 100644 > > --- a/ui/gtk.c > > +++ b/ui/gtk.c > > @@ -600,9 +600,12 @@ void gd_hw_gl_flushed(void *vcon) > > > > fence_fd = qemu_dmabuf_get_fence_fd(dmabuf); > > if (fence_fd >= 0) { > > + void *sync = qemu_dmabuf_get_sync(dmabuf); > > qemu_set_fd_handler(fence_fd, NULL, NULL, NULL); > > close(fence_fd); > > qemu_dmabuf_set_fence_fd(dmabuf, -1); > > + eglDestroySyncKHR(qemu_egl_display, sync); > > + qemu_dmabuf_set_sync(dmabuf, NULL); > > graphic_hw_gl_block(vc->gfx.dcl.con, false); > > } > > } > > @@ -682,6 +685,22 @@ static const DisplayGLCtxOps egl_ctx_ops = { > > static void gd_change_runstate(void *opaque, bool running, > > RunState state) > > { > > GtkDisplayState *s = opaque; > > + QemuDmaBuf *dmabuf; > > + int i; > > + > > + if (state == RUN_STATE_SAVE_VM) { > > + for (i = 0; i < s->nb_vcs; i++) { > > + VirtualConsole *vc = &s->vc[i]; > > + dmabuf = vc->gfx.guest_fb.dmabuf; > > + if (dmabuf && qemu_dmabuf_get_fence_fd(dmabuf) >= 0) { > > + /* wait for the rendering to be completed */ > > + eglClientWaitSync(qemu_egl_display, > > + qemu_dmabuf_get_sync(dmabuf), > > + EGL_SYNC_FLUSH_COMMANDS_BIT_KHR, > > + 1000000000); > > > > > > I don't think adding waiting points in the migration path is > > appropriate. Perhaps once you explain the actual issue, it will be > > easier to help. > > > > + } > > + } > > + } > > > > gd_update_caption(s); > > } > > -- > > 2.34.1 > > > > > > > > > > -- > > Marc-André Lureau > > -- Marc-André Lureau
