[Qemu-devel] VMware SVGA depth computation problem

[Tian, Kevin]

I'm using Qemu 0.12.4 and VMware SVGA driver. Then I suffered from segmentation
fault when using VNC connecting to my host. After digging into the problem, I 
found
it's due to inconsistent depth computation. With VNC connection, depth of SDL
display is set to 16, however what VMware SVGA emulation calculates to 32, which
then causes segmentation fault.

I found that Anthony hunted a similar issue before:

----

commit 4b5db3749c5fdba93e1ac0e8748c9a9a1064319f
Author: Anthony Liguori <address@hidden>
Date:   Fri Dec 18 08:08:09 2009 +1000

    Fix VMware VGA depth computation

    VMware VGA requires that the depth presented to the guest is the same as the
    DisplaySurface that it renders to.  This is because it performs a very simpl
    memcpy() to blit from one surface to another.

    We currently hardcode a 24-bit depth.  The surface allocator for SDL may, an
    usually will, allocate a surface with a different depth causing screen
    corruption.

    This changes the code to allocate the DisplaySurface before initializing the
    device which allows the depth of the DisplaySurface to be used instead of
    hardcoding something.

    Signed-off-by: Anthony Liguori <address@hidden>
    (cherry picked from commit a6109ff1b5d7184a9d490c4ff94f175940232ebd)

----

However it's not a real fix. The point where vmsvga_reset is invoked is earlier
than the point where sdl_display_init is called. This implicates that even with
below fix:

-    s->depth = 24;
-    s->bypp = (s->depth + 7) >> 3;
+    s->depth = ds_get_bits_per_pixel(s->vga.ds);
+    s->bypp = ds_get_bytes_per_pixel(s->vga.ds);

'ds' at the time actually comes from default display allocator which simply 
makes
another hardcode depth as '32'. If Anthony happens to work on a host with 32bit
depth, this patch works. However in other cases where host depth is not 32bit, 
the trouble is still there.

To fix it, we have to find an appropriate point after host display driver is 
initialized.

In the start I move the calculation of {depth, bypp, weight} to the 1st time 
when
SVGA register is accessed. It works on my VNC connection with right depth '16'
returned. However when I try it on my host directly, the depth is still counted 
as
'16' although SDL on my host is actually 32bit.

Then I spent more time to find the exact cause. The reason is that display 
surface
re-allocated in the middle. Before vmware vga driver is loaded in VM, Xserver
uses old way to access VGA BIOS. At that time, VBE BPP is set to '16' by BOCHS
VGA BIOS. When new depth is different from previous one, vga_draw_graphics
will free previous surface and re-allocate a new surface with new depth. Then
later when 1st access to VMware SVGA register, the surface is already not the
one created by SDL driver.

Since the surface is volatile, the simple fix would be to do depth calculation 
right
after display driver is initialized, but before the VM is executed. For example,
some place after:
----(vl.c)
    switch (display_type) {
    case DT_NOGRAPHIC:
        break;
#if defined(CONFIG_CURSES)
    case DT_CURSES:
        curses_display_init(ds, full_screen);
        break;
#endif
#if defined(CONFIG_SDL)
    case DT_SDL:
        sdl_display_init(ds, full_screen, no_frame);
        break;
#elif defined(CONFIG_COCOA)
    case DT_SDL:
        cocoa_display_init(ds, full_screen);
        break;
#endif
    case DT_VNC:
        vnc_display_init(ds);
        if (vnc_display_open(ds, vnc_display) < 0)
            exit(1);

        if (show_vnc_port) {
            printf("VNC server running on `%s'\n", vnc_display_local_addr(ds));
        }
        break;
    default:
        break;
    }
    dpy_resize(ds);
----

I searched but didn't find any existing callbacks for this purpose. So put my 
question
here in case you know some points I can easily hook to instead of creating a new
one for this purpose. :-)

Thanks,
Kevin