[Qemu-devel] RE: [PATCH] Another fix to VMware depth computation

[Tian, Kevin]

Looks no response here. Could anyone let me know anything else I need do to make
this patch ready for acceptance? I'd like to follow any guideline if there is.

I also CC Anthony here since he worked on a similar fix before. If any other 
stakeholder
I should CC, please let me know.

Thanks a lot,
Kevin

>From: Tian, Kevin
>Sent: Monday, June 28, 2010 3:59 PM
>
>(patch made upon 0.12.4 release; tested on 0.12 branch; build test on master)
>
> console.h       |   13 -------------
> hw/vmware_vga.c |   10 ++--------
> qemu-common.h   |    1 -
> vl.c            |    8 --------
> 4 files changed, 2 insertions(+), 30 deletions(-)
>
>=====
>    Another fix to VMware depth computation
>
>    VMware SVGA presents to the guest with the depth of the host surface it 
> renders
>    to, and rejects to work if the two sides are mismatched. One problem is 
> that
>    current VMware VGA may calculate a wrong host depth, and then memcpy from 
> virtual
>    framebuffer to host surface may trigger segmentation fault. For example, 
> when
>    launching Qemu in a VNC connection, VMware SVGA thinks depth as '32', 
> however the
>    actual depth of VNC is '16'. The fault also happens when the host depth is 
> not
>    32 bit.
>
>    4b5db3749c5fdba93e1ac0e8748c9a9a1064319f tempts to fix a similar issue, by
>    changing from hard-coded 24bit depth to instead query the surface allocator
>    (e.g. sdl). However it doesn't really work, because the point where query
>    is invoked is earlier than the point where sdl is initialized. At query 
> time,
>    qemu uses a default surface allocator which, again, provides another 
> hard-coded
>    depth value - 32bit. So it happens to make VMware SVGA working on some 
> hosts,
>    but still fails in others.
>
>    To solve this issue, this commit introduces a postcall interface to display
>    surface, which is walked after surface allocators are actually initialized.
>    At that point it's then safe to query host depth and present to the guest.
>
>    Signed-off-by Kevin Tian <address@hidden>
>
>diff --git a/console.h b/console.h
>index dfc8ae4..05fbf17 100644
>--- a/console.h
>+++ b/console.h
>@@ -122,6 +122,12 @@ struct DisplayAllocator {
>     void (*free_displaysurface)(DisplaySurface *surface);
> };
>
>+struct DisplayPostCallback {
>+    void (*postcall) (void *);
>+    void *parm;
>+    struct DisplayPostCallback *next;
>+};
>+
> struct DisplayState {
>     struct DisplaySurface *surface;
>     void *opaque;
>@@ -129,6 +135,7 @@ struct DisplayState {
>
>     struct DisplayAllocator* allocator;
>     struct DisplayChangeListener* listeners;
>+    struct DisplayPostCallback* postcalls;
>
>     void (*mouse_set)(int x, int y, int on);
>     void (*cursor_define)(int width, int height, int bpp, int hot_x, int 
> hot_y,
>@@ -185,6 +192,12 @@ static inline void 
>register_displaychangelistener(DisplayState *ds,
>DisplayChang
>     ds->listeners = dcl;
> }
>
>+static inline void register_displaypostcallback(DisplayState *ds, 
>DisplayPostCallback *dpc)
>+{
>+    dpc->next = ds->postcalls;
>+    ds->postcalls = dpc;
>+}
>+
> static inline void dpy_update(DisplayState *s, int x, int y, int w, int h)
> {
>     struct DisplayChangeListener *dcl = s->listeners;
>diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
>index 01bb85b..d73cca6 100644
>--- a/hw/vmware_vga.c
>+++ b/hw/vmware_vga.c
>@@ -927,8 +927,9 @@ static void vmsvga_update_display(void *opaque)
>     }
> }
>
>-static void vmsvga_reset(struct vmsvga_state_s *s)
>+static void vmsvga_reset(void *parm)
> {
>+    struct vmsvga_state_s *s = (struct vmsvga_state_s *)parm;
>     s->index = 0;
>     s->enable = 0;
>     s->config = 0;
>@@ -1133,6 +1134,8 @@ static const VMStateDescription vmstate_vmware_vga = {
>
> static void vmsvga_init(struct vmsvga_state_s *s, int vga_ram_size)
> {
>+    DisplayPostCallback *dpc;
>+
>     s->scratch_size = SVGA_SCRATCH_SIZE;
>     s->scratch = qemu_malloc(s->scratch_size * 4);
>
>@@ -1160,7 +1163,10 @@ static void vmsvga_init(struct vmsvga_state_s *s, int
>vga_ram_size)
>
>     rom_add_vga(VGABIOS_FILENAME);
>
>-    vmsvga_reset(s);
>+    dpc = qemu_mallocz(sizeof(DisplayPostCallback));
>+    dpc->postcall = vmsvga_reset;
>+    dpc->parm = s;
>+    register_displaypostcallback(s->vga.ds, dpc);
> }
>
> static void pci_vmsvga_map_ioport(PCIDevice *pci_dev, int region_num,
>diff --git a/qemu-common.h b/qemu-common.h
>index a23afbc..19f107a 100644
>--- a/qemu-common.h
>+++ b/qemu-common.h
>@@ -198,6 +198,7 @@ typedef struct DisplayState DisplayState;
> typedef struct DisplayChangeListener DisplayChangeListener;
> typedef struct DisplaySurface DisplaySurface;
> typedef struct DisplayAllocator DisplayAllocator;
>+typedef struct DisplayPostCallback DisplayPostCallback;
> typedef struct PixelFormat PixelFormat;
> typedef struct TextConsole TextConsole;
> typedef TextConsole QEMUConsole;
>diff --git a/vl.c b/vl.c
>index 39182ea..9a3e9fd 100644
>--- a/vl.c
>+++ b/vl.c
>@@ -4863,6 +4863,7 @@ int main(int argc, char **argv, char **envp)
>     char boot_devices[33] = "cad"; /* default to HD->floppy->CD-ROM */
>     DisplayState *ds;
>     DisplayChangeListener *dcl;
>+    DisplayPostCallback *dpc;
>     int cyls, heads, secs, translation;
>     QemuOpts *hda_opts = NULL, *opts;
>     int optind;
>@@ -6053,6 +6053,13 @@ int main(int argc, char **argv, char **envp)
>     }
>     dpy_resize(ds);
>
>+    dpc = ds->postcalls;
>+    while (dpc != NULL) {
>+        if (dpc->postcall != NULL)
>+            dpc->postcall(dpc->parm);
>+        dpc = dpc->next;
>+    }
>+
>     dcl = ds->listeners;
>     while (dcl != NULL) {
>         if (dcl->dpy_refresh != NULL) {
>
>Thanks,
>Kevin