From: Gerd Hoffmann Date: Tue, 14 Feb 2017 18:09:59 +0000 (+0100) Subject: cirrus/vnc: zap bitblit support from console code. X-Git-Url: http://xenbits.xensource.com/gitweb?a=commitdiff_plain;h=6b0966a4422a6374db0f250b181bb4276c9e7a10;p=qemu-xen.git cirrus/vnc: zap 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. This is XSA-211. Signed-off-by: Gerd Hoffmann Signed-off-by: Stefano Stabellini --- diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index 5531305b26..fff85066f6 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -756,11 +756,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) - graphic_hw_update(s->vga.con); - (*s->cirrus_rop) (s, s->vga.vram_ptr + (s->cirrus_blt_dstaddr & s->cirrus_addr_mask), s->vga.vram_ptr + @@ -769,10 +764,9 @@ 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->vga.con, - sx, sy, dx, dy, - s->cirrus_blt_width / depth, - s->cirrus_blt_height); + dpy_gfx_update(s->vga.con, dx, dy, + s->cirrus_blt_width / depth, + s->cirrus_blt_height); } /* we don't have to notify the display that this portion has diff --git a/include/ui/console.h b/include/ui/console.h index 8a866176db..1c3b06e4b0 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -158,10 +158,6 @@ typedef struct DisplayChangeListenerOps { int x, int y, int w, int h); void (*dpy_gfx_switch)(DisplayChangeListener *dcl, struct DisplaySurface *new_surface); - void (*dpy_gfx_copy)(DisplayChangeListener *dcl, - int src_x, int src_y, - int dst_x, int dst_y, int w, int h); - void (*dpy_text_cursor)(DisplayChangeListener *dcl, int x, int y); void (*dpy_text_resize)(DisplayChangeListener *dcl, @@ -217,8 +213,6 @@ int dpy_set_ui_info(QemuConsole *con, QemuUIInfo *info); void dpy_gfx_update(QemuConsole *con, int x, int y, int w, int h); void dpy_gfx_replace_surface(QemuConsole *con, DisplaySurface *surface); -void dpy_gfx_copy(QemuConsole *con, int src_x, int src_y, - int dst_x, int dst_y, int w, int h); void dpy_text_cursor(QemuConsole *con, int x, int y); void dpy_text_update(QemuConsole *con, int x, int y, int w, int h); void dpy_text_resize(QemuConsole *con, int w, int h); @@ -302,8 +296,6 @@ void text_consoles_set_display(DisplayState *ds); void console_select(unsigned int index); void console_color_init(DisplayState *ds); void qemu_console_resize(QemuConsole *con, int width, int height); -void qemu_console_copy(QemuConsole *con, int src_x, int src_y, - int dst_x, int dst_y, int w, int h); DisplaySurface *qemu_console_surface(QemuConsole *con); DisplayState *qemu_console_displaystate(QemuConsole *console); diff --git a/ui/console.c b/ui/console.c index e057755c04..a62b0a6c77 100644 --- a/ui/console.c +++ b/ui/console.c @@ -1418,27 +1418,6 @@ void dpy_refresh(DisplayState *s) } } -void dpy_gfx_copy(QemuConsole *con, int src_x, int src_y, - int dst_x, int dst_y, int w, int h) -{ - DisplayState *s = con->ds; - DisplayChangeListener *dcl; - - if (!qemu_console_is_visible(con)) { - return; - } - QLIST_FOREACH(dcl, &s->listeners, next) { - if (con != (dcl->con ? dcl->con : active_console)) { - continue; - } - if (dcl->ops->dpy_gfx_copy) { - dcl->ops->dpy_gfx_copy(dcl, src_x, src_y, dst_x, dst_y, w, h); - } else { /* TODO */ - dcl->ops->dpy_gfx_update(dcl, dst_x, dst_y, w, h); - } - } -} - void dpy_text_cursor(QemuConsole *con, int x, int y) { DisplayState *s = con->ds; @@ -1857,13 +1836,6 @@ void qemu_console_resize(QemuConsole *s, int width, int height) dpy_gfx_replace_surface(s, surface); } -void qemu_console_copy(QemuConsole *con, int src_x, int src_y, - int dst_x, int dst_y, int w, int h) -{ - assert(con->console_type == GRAPHIC_CONSOLE); - dpy_gfx_copy(con, src_x, src_y, dst_x, dst_y, w, h); -} - DisplaySurface *qemu_console_surface(QemuConsole *console) { return console->surface; diff --git a/ui/vnc.c b/ui/vnc.c index 647a356cd4..02ca3a63ef 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -730,96 +730,6 @@ int vnc_send_framebuffer_update(VncState *vs, int x, int y, int w, int h) return n; } -static void vnc_copy(VncState *vs, int src_x, int src_y, int dst_x, int dst_y, int w, int h) -{ - /* send bitblit op to the vnc client */ - vnc_lock_output(vs); - vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE); - vnc_write_u8(vs, 0); - vnc_write_u16(vs, 1); /* number of rects */ - vnc_framebuffer_update(vs, dst_x, dst_y, w, h, VNC_ENCODING_COPYRECT); - vnc_write_u16(vs, src_x); - vnc_write_u16(vs, src_y); - vnc_unlock_output(vs); - vnc_flush(vs); -} - -static void vnc_dpy_copy(DisplayChangeListener *dcl, - int src_x, int src_y, - int dst_x, int dst_y, int w, int h) -{ - VncDisplay *vd = container_of(dcl, VncDisplay, dcl); - VncState *vs, *vn; - uint8_t *src_row; - uint8_t *dst_row; - int i, x, y, pitch, inc, w_lim, s; - int cmp_bytes; - - vnc_refresh_server_surface(vd); - QTAILQ_FOREACH_SAFE(vs, &vd->clients, next, vn) { - if (vnc_has_feature(vs, VNC_FEATURE_COPYRECT)) { - vs->force_update = 1; - vnc_update_client(vs, 1, true); - /* vs might be free()ed here */ - } - } - - /* do bitblit op on the local surface too */ - pitch = vnc_server_fb_stride(vd); - src_row = vnc_server_fb_ptr(vd, src_x, src_y); - dst_row = vnc_server_fb_ptr(vd, dst_x, dst_y); - y = dst_y; - inc = 1; - if (dst_y > src_y) { - /* copy backwards */ - src_row += pitch * (h-1); - dst_row += pitch * (h-1); - pitch = -pitch; - y = dst_y + h - 1; - inc = -1; - } - w_lim = w - (VNC_DIRTY_PIXELS_PER_BIT - (dst_x % VNC_DIRTY_PIXELS_PER_BIT)); - if (w_lim < 0) { - w_lim = w; - } else { - w_lim = w - (w_lim % VNC_DIRTY_PIXELS_PER_BIT); - } - for (i = 0; i < h; i++) { - for (x = 0; x <= w_lim; - x += s, src_row += cmp_bytes, dst_row += cmp_bytes) { - if (x == w_lim) { - if ((s = w - w_lim) == 0) - break; - } else if (!x) { - s = (VNC_DIRTY_PIXELS_PER_BIT - - (dst_x % VNC_DIRTY_PIXELS_PER_BIT)); - s = MIN(s, w_lim); - } else { - s = VNC_DIRTY_PIXELS_PER_BIT; - } - cmp_bytes = s * VNC_SERVER_FB_BYTES; - if (memcmp(src_row, dst_row, cmp_bytes) == 0) - continue; - memmove(dst_row, src_row, cmp_bytes); - QTAILQ_FOREACH(vs, &vd->clients, next) { - if (!vnc_has_feature(vs, VNC_FEATURE_COPYRECT)) { - set_bit(((x + dst_x) / VNC_DIRTY_PIXELS_PER_BIT), - vs->dirty[y]); - } - } - } - src_row += pitch - w * VNC_SERVER_FB_BYTES; - dst_row += pitch - w * VNC_SERVER_FB_BYTES; - y += inc; - } - - QTAILQ_FOREACH(vs, &vd->clients, next) { - if (vnc_has_feature(vs, VNC_FEATURE_COPYRECT)) { - vnc_copy(vs, src_x, src_y, dst_x, dst_y, w, h); - } - } -} - static void vnc_mouse_set(DisplayChangeListener *dcl, int x, int y, int visible) { @@ -2925,7 +2835,6 @@ static void vnc_listen_websocket_read(void *opaque) static const DisplayChangeListenerOps dcl_ops = { .dpy_name = "vnc", .dpy_refresh = vnc_refresh, - .dpy_gfx_copy = vnc_dpy_copy, .dpy_gfx_update = vnc_dpy_update, .dpy_gfx_switch = vnc_dpy_switch, .dpy_mouse_set = vnc_mouse_set,