[Qemu-devel] Re: [PATCH 5/5] linux fbdev display driver.

[Gerd Hoffmann]

  Hi,

+static void fbdev_free_displaysurface(DisplaySurface *surface)
+{
+    if (surface == NULL)
+        return;
+
+    if (surface->flags&  QEMU_ALLOCATED_FLAG) {
+        qemu_free(surface->data);
+    }
+
+    surface->data = NULL;

This is pretty pointless ...

+    qemu_free(surface);

... as you free surface anyway ;)

@@ -910,7 +959,17 @@ void fbdev_display_init(DisplayState *ds, const char 
*device)
      dcl->dpy_update  = fbdev_update;
      dcl->dpy_resize  = fbdev_resize;
      dcl->dpy_refresh = fbdev_refresh;
+    dcl->dpy_setdata = fbdev_setdata;
      register_displaychangelistener(ds, dcl);
+
+    da = qemu_mallocz(sizeof (DisplayAllocator));
+    da->create_displaysurface = fbdev_create_displaysurface;
+    da->resize_displaysurface = fbdev_resize_displaysurface;
+    da->free_displaysurface = fbdev_free_displaysurface;
+
+    if (register_displayallocator(ds, da) == da) {
+        dpy_resize(ds);
+    }

You register the display allocator, but don't unregister in 
fbdev_display_uninit().

You are just lucky that fbdev_cleanup() forgets to unmap the framebuffer.

Apply the attached fix, start qemu with vnc, then do "change fbdev on" 
and "change fbdev off" in the monitor and watch qemu segfault.

Also after "change fbdev on" the guest screen isn't rendered correctly.

cheers,
  Gerd

>From 685849ae48eaef7927b90e012fb6afb4494052d0 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kra...@redhat.com>
Date: Thu, 17 Jun 2010 12:32:53 +0200
Subject: [PATCH] fbdev: unmap framebuffer on cleanup

---
 fbdev.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/fbdev.c b/fbdev.c
index 6623a4f..1a95ede 100644
--- a/fbdev.c
+++ b/fbdev.c
@@ -518,6 +518,10 @@ static void fbdev_cleanup(void)
         fprintf(stderr, "%s\n", __FUNCTION__);
 
     /* restore console */
+    if (fb_mem != NULL) {
+        munmap(fb_mem, fb_fix.smem_len+fb_mem_offset);
+        fb_mem = NULL;
+    }
     if (fb != -1) {
         if (ioctl(fb,FBIOPUT_VSCREENINFO, &fb_ovar) < 0)
             perror("ioctl FBIOPUT_VSCREENINFO");
-- 
1.6.5.2