cirrus/vnc: zap drop bitblit support from console code.

There is a special code path (dpy_gfx_copy) to allow graphic emulation
notify user interface code about bitblit operations carryed out by
guests.  It is supported by cirrus and vnc server.  The intended purpose
is to optimize display scrolls and just send over the scroll op instead
of a full display update.

This is rarely used these days though because modern guests simply don't
use the cirrus blitter any more.  Any linux guest using the cirrus drm
driver doesn't.  Any windows guest newer than winxp doesn't ship with a
cirrus driver any more and thus uses the cirrus as simple framebuffer.

So this code tends to bitrot and bugs can go unnoticed for a long time.
See for example commit "3e10c3e vnc: fix qemu crash because of SIGSEGV"
which fixes a bug lingering in the code for almost a year, added by
commit "c7628bf vnc: only alloc server surface with clients connected".

Also the vnc server will throttle the frame rate in case it figures the
network can't keep up (send buffers are full).  This doesn't work with
dpy_gfx_copy, for any copy operation sent to the vnc client we have to
send all outstanding updates beforehand, otherwise the vnc client might
run the client side blit on outdated data and thereby corrupt the
display.  So this dpy_gfx_copy "optimization" might even make things
worse on slow network links.

Lets kill it once for all.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
These changes (dropping dpy_copy and all its references and
implementations) reimplemented for qemu-xen-traditional.

This is XSA-211.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

diff --git a/console.c b/console.c
index d4f1ad0e0657685ff5e92352359d85f285160fcb..e61b53b168b81077b433fe394cceb80db1c129b5 100644 (file)
--- a/console.c
+++ b/console.c
@@ -1399,14 +1399,6 @@ void qemu_console_resize(DisplayState *ds, int width, int height)
     }
 }
 
-void qemu_console_copy(DisplayState *ds, int src_x, int src_y,
-                int dst_x, int dst_y, int w, int h)
-{
-    if (is_graphic_console()) {
-        dpy_copy(ds, src_x, src_y, dst_x, dst_y, w, h);
-    }
-}
-
 PixelFormat qemu_different_endianness_pixelformat(int bpp)
 {
     PixelFormat pf;

diff --git a/console.h b/console.h
index 14b42f3bb844462eba74566bacf82368be0156df..8306cc401cabdfe7d62194b803596021514184f6 100644 (file)
--- a/console.h
+++ b/console.h
@@ -98,8 +98,6 @@ struct DisplayChangeListener {
     void (*dpy_resize)(struct DisplayState *s);
     void (*dpy_setdata)(struct DisplayState *s);
     void (*dpy_refresh)(struct DisplayState *s);
-    void (*dpy_copy)(struct DisplayState *s, int src_x, int src_y,
-                     int dst_x, int dst_y, int w, int h);
     void (*dpy_fill)(struct DisplayState *s, int x, int y,
                      int w, int h, uint32_t c);
     void (*dpy_text_cursor)(struct DisplayState *s, int x, int y);
@@ -211,18 +209,6 @@ static inline void dpy_refresh(DisplayState *s)
     }
 }
 
-static inline void dpy_copy(struct DisplayState *s, int src_x, int src_y,
-                             int dst_x, int dst_y, int w, int h) {
-    struct DisplayChangeListener *dcl = s->listeners;
-    while (dcl != NULL) {
-        if (dcl->dpy_copy)
-            dcl->dpy_copy(s, src_x, src_y, dst_x, dst_y, w, h);
-        else /* TODO */
-            dcl->dpy_update(s, dst_x, dst_y, w, h);
-        dcl = dcl->next;
-    }
-}
-
 static inline void dpy_fill(struct DisplayState *s, int x, int y,
                              int w, int h, uint32_t c) {
     struct DisplayChangeListener *dcl = s->listeners;
@@ -297,8 +283,6 @@ void text_consoles_set_display(DisplayState *ds);
 void console_select(unsigned int index);
 void console_color_init(DisplayState *ds);
 void qemu_console_resize(DisplayState *ds, int width, int height);
-void qemu_console_copy(DisplayState *ds, int src_x, int src_y,
-                int dst_x, int dst_y, int w, int h);
 
 /* sdl.c */
 void sdl_display_init(DisplayState *ds, int full_screen, int no_frame, int opengl_enabled);

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index 06b4a3b000846836c67b3856c33490be9dec1c3e..4e85b900f58453aa31a327c24503c631eea18f33 100644 (file)
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -793,11 +793,6 @@ static void cirrus_do_copy(CirrusVGAState *s, int dst, int src, int w, int h)
         }
     }
 
-    /* we have to flush all pending changes so that the copy
-       is generated at the appropriate moment in time */
-    if (notify)
-       vga_hw_update();
-
     (*s->cirrus_rop) (s, s->vram_ptr +
                      (s->cirrus_blt_dstaddr & s->cirrus_addr_mask),
                      s->vram_ptr +
@@ -806,13 +801,13 @@ static void cirrus_do_copy(CirrusVGAState *s, int dst, int src, int w, int h)
                      s->cirrus_blt_width, s->cirrus_blt_height);
 
     if (notify)
-       qemu_console_copy(s->ds,
-                         sx, sy, dx, dy,
-                         s->cirrus_blt_width / depth,
-                         s->cirrus_blt_height);
+       dpy_update(s->ds,
+                   dx, dy,
+                   s->cirrus_blt_width / depth,
+                   s->cirrus_blt_height);
 
     /* we don't have to notify the display that this portion has
-       changed since qemu_console_copy implies this */
+       changed since dpy_update implies this */
 
     cirrus_invalidate_region(s, s->cirrus_blt_dstaddr,
                                s->cirrus_blt_dstpitch, s->cirrus_blt_width,

diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index d1cba28a3fe44d964234140b947e4a94abb8e199..c38e43ccf271b17bd9a26494f13c989463d51d05 100644 (file)
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -383,6 +383,7 @@ static inline void vmsvga_copy_rect(struct vmsvga_state_s *s,
 
 # ifdef DIRECT_VRAM
     if (s->ds->dpy_copy)
+#  error This configuration is not supported.  See XSA-211.
         qemu_console_copy(s->ds, x0, y0, x1, y1, w, h);
     else
 # endif

diff --git a/vnc.c b/vnc.c
index 61d1555204ab6920d97a4c050674d93a4a1fb637..0e611970908c5ca888d5fd9a244c6ea0b871551a 100644 (file)
--- a/vnc.c
+++ b/vnc.c
@@ -572,36 +572,6 @@ static void send_framebuffer_update(VncState *vs, int x, int y, int w, int h)
            send_framebuffer_update_raw(vs, x, y, w, h);
 }
 
-static void vnc_copy(DisplayState *ds, int src_x, int src_y, int dst_x, int dst_y, int w, int h)
-{
-    VncState *vs = ds->opaque;
-    int updating_client = 1;
-
-    if (!vs->update_requested ||
-        src_x < vs->visible_x || src_y < vs->visible_y ||
-       dst_x < vs->visible_x || dst_y < vs->visible_y ||
-       (src_x + w) > (vs->visible_x + vs->visible_w) ||
-       (src_y + h) > (vs->visible_y + vs->visible_h) ||
-       (dst_x + w) > (vs->visible_x + vs->visible_w) ||
-       (dst_y + h) > (vs->visible_y + vs->visible_h))
-       updating_client = 0;
-
-    if (updating_client)
-        _vnc_update_client(vs);
-
-    if (updating_client && vs->csock != -1 && !vs->has_update) {
-       vnc_write_u8(vs, 0);  /* msg id */
-       vnc_write_u8(vs, 0);
-       vnc_write_u16(vs, 1); /* number of rects */
-       vnc_framebuffer_update(vs, dst_x, dst_y, w, h, 1);
-       vnc_write_u16(vs, src_x);
-       vnc_write_u16(vs, src_y);
-       vnc_flush(vs);
-        vs->update_requested--;
-    } else
-       framebuffer_set_updated(vs, dst_x, dst_y, w, h);
-}
-
 static int find_update_height(VncState *vs, int y, int maxy, int last_x, int x)
 {
     int h;
@@ -1543,16 +1513,12 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
     vs->has_pointer_type_change = 0;
     vs->has_WMVi = 0;
     vs->absolute = -1;
-    dcl->dpy_copy = NULL;
 
     for (i = n_encodings - 1; i >= 0; i--) {
        switch (encodings[i]) {
        case 0: /* Raw */
            vs->has_hextile = 0;
            break;
-       case 1: /* CopyRect */
-           dcl->dpy_copy = vnc_copy;
-           break;
        case 5: /* Hextile */
            vs->has_hextile = 1;
            break;
@@ -2459,7 +2425,6 @@ static void vnc_listen_read(void *opaque)
        vs->has_resize = 0;
        vs->has_hextile = 0;
         vs->update_requested = 0;
-       dcl->dpy_copy = NULL;
        vnc_timer_init(vs);
     }
 }