Hi


On Mon, Jun 8, 2026 at 4:51 PM Cédric Le Goater <[email protected]> wrote:
>
> Hello Marc-André,
>
> On 6/8/26 09:16, Marc-André Lureau wrote:
> > The QemuGraphicConsole holds a strong QOM link back to the device via
> > its "device" property (OBJ_PROP_LINK_STRONG). When graphic_console_close()
> > is only called from vfio_display_finalize() during object finalize, this
> > creates a ref-cycle deadlock: the device can't reach refcount 0 because
> > the console holds a strong ref, but the console's ref is only dropped by
> > graphic_console_close() which runs inside finalize.
> >
> > Split the display teardown into two phases:
> > - vfio_display_exit(): called during unrealize (vfio_exitfn), closes
> >    the graphic console to break the ref cycle, and removes display
> >    region subregions while the parent memory regions are still alive.
> > - vfio_display_finalize(): remains in finalize (vfio_pci_put_device),
> >    frees display region memory, dmabuf, and edid resources. The region
> >    memory contains QOM child objects (MemoryRegions) that must stay
> >    alive until QOM finalization has processed them.
> >
> > Signed-off-by: Marc-André Lureau <[email protected]>
> > ---
> >   hw/vfio/pci.h     |  1 +
> >   hw/vfio/display.c | 30 +++++++++++++++++++-----------
> >   hw/vfio/pci.c     |  1 +
> >   3 files changed, 21 insertions(+), 11 deletions(-)
> >
> > diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> > index c3a1f53d350..cf567115870 100644
> > --- a/hw/vfio/pci.h
> > +++ b/hw/vfio/pci.h
> > @@ -270,6 +270,7 @@ bool vfio_populate_vga(VFIOPCIDevice *vdev, Error 
> > **errp);
> >
> >   void vfio_display_reset(VFIOPCIDevice *vdev);
> >   bool vfio_display_probe(VFIOPCIDevice *vdev, Error **errp);
> > +void vfio_display_exit(VFIOPCIDevice *vdev);
> >   void vfio_display_finalize(VFIOPCIDevice *vdev);
> >
> >   extern const VMStateDescription vfio_display_vmstate;
> > diff --git a/hw/vfio/display.c b/hw/vfio/display.c
> > index 8f91e83da88..34cc25ee0e0 100644
> > --- a/hw/vfio/display.c
> > +++ b/hw/vfio/display.c
> > @@ -505,15 +505,6 @@ static bool vfio_display_region_init(VFIOPCIDevice 
> > *vdev, Error **errp)
> >       return true;
> >   }
> >
> > -static void vfio_display_region_exit(VFIODisplay *dpy)
> > -{
> > -    if (!dpy->region.buffer.size) {
> > -        return;
> > -    }
> > -
> > -    vfio_region_exit(&dpy->region.buffer);
> > -    vfio_region_finalize(&dpy->region.buffer);
> > -}
> >
> >   /* ---------------------------------------------------------------------- 
> > */
> >
> > @@ -547,17 +538,34 @@ bool vfio_display_probe(VFIOPCIDevice *vdev, Error 
> > **errp)
> >       return false;
> >   }
> >
> > -void vfio_display_finalize(VFIOPCIDevice *vdev)
> > +void vfio_display_exit(VFIOPCIDevice *vdev)
> >   {
> >       if (!vdev->dpy) {
> >           return;
> >       }
> >
> > +    if (display_opengl) {
> > +        qemu_console_set_display_gl_ctx(vdev->dpy->con, NULL);
> > +    }
> >       qemu_graphic_console_close(vdev->dpy->con);
> > +    if (vdev->dpy->region.buffer.size) {
> > +        vfio_region_exit(&vdev->dpy->region.buffer);
> > +    }
> > +}
> > +
> > +void vfio_display_finalize(VFIOPCIDevice *vdev)
> > +{
> > +    if (!vdev->dpy) {
> > +        return;
> > +    }
> > +
> >       vfio_display_dmabuf_exit(vdev->dpy);
>
> vfio_display_dmabuf_exit() will use dpy->con after it's closed :
>
>    qemu_console_gl_release_dmabuf(dpy->con, dmabuf->buf);
>
> It is OK ?

No, we should free the dmabufs during vfio_display_exit() before the
console is closed.

thanks

> if so,
>
>
> Acked-by: Cédric Le Goater <[email protected]>
>
> Thanks,
>
> C.
>
>
>
> > -    vfio_display_region_exit(vdev->dpy);
> > +    if (vdev->dpy->region.buffer.size) {
> > +        vfio_region_finalize(&vdev->dpy->region.buffer);
> > +    }
> >       vfio_display_edid_exit(vdev->dpy);
> >       g_free(vdev->dpy);
> > +    vdev->dpy = NULL;
> >   }
> >
> >   static bool migrate_needed(void *opaque)
> > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> > index 9c06b25e637..78beacd24e1 100644
> > --- a/hw/vfio/pci.c
> > +++ b/hw/vfio/pci.c
> > @@ -3624,6 +3624,7 @@ static void vfio_exitfn(PCIDevice *pdev)
> >       VFIOPCIDevice *vdev = VFIO_PCI_DEVICE(pdev);
> >       VFIODevice *vbasedev = &vdev->vbasedev;
> >
> > +    vfio_display_exit(vdev);
> >       vfio_unregister_req_notifier(vdev);
> >       vfio_unregister_err_notifier(vdev);
> >       pci_device_set_intx_routing_notifier(pdev, NULL);
> >
>
>

Reply via email to