[Qemu-devel] [PATCH] vnc: shared buffer: skip some optimizations.

[Gerd Hoffmann <kraxel@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 475 bytes --]

  Hi,

When using a shared display surface buffer some of the optimizations
done by the vnc server code don't work.  In shared buffer mode the guest
may update the screen while the vnc server looks at the framebuffer.
That in turn makes some code racy, the dirty bitmap walk through for
example, leading to screen corruption.  Right now this is visible with
xenfb only.  I expect simliar issues will show up for vga too once we
run the vcpus in threads.

please apply
  Gerd

[-- Attachment #2: 0001-vnc-shared-buffer-skip-some-optimizations.patch --]
[-- Type: text/plain, Size: 4346 bytes --]

>From eaff07da03b93fa59d6330a7e5e0e720d4573f3c Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Wed, 11 Mar 2009 15:30:23 +0100
Subject: [PATCH] vnc: shared buffer: skip some optimizations.

When using a shared display surface buffer some of the optimizations
done by the vnc server code don't work.  In shared buffer mode the guest
may update the screen while the vnc server looks at the framebuffer.
That in turn makes some code racy, the dirty bitmap walk through for
example, leading to screen corruption.  Right now this is visible with
xenfb only.  I expect simliar issues will show up for vga too once we
run the vcpus in threads.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 vnc.c |   78 +++++++++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 47 insertions(+), 31 deletions(-)

diff --git a/vnc.c b/vnc.c
index 81c842a..66f8946 100644
--- a/vnc.c
+++ b/vnc.c
@@ -649,7 +649,8 @@ static void vnc_dpy_copy(DisplayState *ds, int src_x, int src_y, int dst_x, int
     VncDisplay *vd = ds->opaque;
     VncState *vs = vd->clients;
     while (vs != NULL) {
-        if (vnc_has_feature(vs, VNC_FEATURE_COPYRECT))
+        if (vnc_has_feature(vs, VNC_FEATURE_COPYRECT) &&
+            !is_buffer_shared(ds->surface))
             vnc_copy(vs, src_x, src_y, dst_x, dst_y, w, h);
         else /* TODO */
             vnc_update(vs, dst_x, dst_y, w, h);
@@ -688,36 +689,51 @@ static void vnc_update_client(void *opaque)
 
         vnc_set_bits(width_mask, (ds_get_width(vs->ds) / 16), VNC_DIRTY_WORDS);
 
-	/* Walk through the dirty map and eliminate tiles that
-	   really aren't dirty */
-	row = ds_get_data(vs->ds);
-	old_row = vs->old_data;
-
-	for (y = 0; y < ds_get_height(vs->ds); y++) {
-	    if (vnc_and_bits(vs->dirty_row[y], width_mask, VNC_DIRTY_WORDS)) {
-		int x;
-		uint8_t *ptr;
-		char *old_ptr;
-
-		ptr = row;
-		old_ptr = (char*)old_row;
-
-		for (x = 0; x < ds_get_width(vs->ds); x += 16) {
-		    if (memcmp(old_ptr, ptr, 16 * ds_get_bytes_per_pixel(vs->ds)) == 0) {
-			vnc_clear_bit(vs->dirty_row[y], (x / 16));
-		    } else {
-			has_dirty = 1;
-			memcpy(old_ptr, ptr, 16 * ds_get_bytes_per_pixel(vs->ds));
-		    }
-
-		    ptr += 16 * ds_get_bytes_per_pixel(vs->ds);
-		    old_ptr += 16 * ds_get_bytes_per_pixel(vs->ds);
-		}
-	    }
+        if (!is_buffer_shared(vs->ds->surface)) {
+            /* Walk through the dirty map and eliminate tiles that
+               really aren't dirty */
+            row = ds_get_data(vs->ds);
+            old_row = vs->old_data;
+
+            for (y = 0; y < ds_get_height(vs->ds); y++) {
+                if (vnc_and_bits(vs->dirty_row[y], width_mask, VNC_DIRTY_WORDS)) {
+                    int x;
+                    uint8_t *ptr;
+                    char *old_ptr;
+
+                    ptr = row;
+                    old_ptr = (char*)old_row;
+
+                    for (x = 0; x < ds_get_width(vs->ds); x += 16) {
+                        if (memcmp(old_ptr, ptr, 16 * ds_get_bytes_per_pixel(vs->ds)) == 0) {
+                            vnc_clear_bit(vs->dirty_row[y], (x / 16));
+                        } else {
+                            has_dirty = 1;
+                            memcpy(old_ptr, ptr, 16 * ds_get_bytes_per_pixel(vs->ds));
+                        }
+
+                        ptr += 16 * ds_get_bytes_per_pixel(vs->ds);
+                        old_ptr += 16 * ds_get_bytes_per_pixel(vs->ds);
+                    }
+                }
 
-	    row += ds_get_linesize(vs->ds);
-	    old_row += ds_get_linesize(vs->ds);
-	}
+                row += ds_get_linesize(vs->ds);
+                old_row += ds_get_linesize(vs->ds);
+            }
+        } else {
+            /*
+             * shared buffer:
+             *   -> verifying the dirty map is racy as the guest may
+             *      update the screen while we are looking at it
+             *   -> skip the check
+             */
+            for (y = 0; y < ds_get_height(vs->ds); y++) {
+                if (vnc_and_bits(vs->dirty_row[y], width_mask, VNC_DIRTY_WORDS)) {
+                    has_dirty = 1;
+                    break;
+                }
+            }
+        }
 
 	if (!has_dirty && !vs->audio_cap) {
 	    qemu_mod_timer(vs->timer, qemu_get_clock(rt_clock) + VNC_REFRESH_INTERVAL);
-- 
1.6.1.3