[Qemu-devel] vnc regression with -vga vmware

[Qemu-devel] vnc regression with -vga vmware

[Serge Hallyn]

Hi,

upstream git HEAD appears to have regressed with -vga vmware -vnc.

If I run 

	./qemu-system-x86_64 -enable-kvm -vnc :1 -m 1024 -cdrom ~/trusty-desktop-amd64.iso -vga vmware

then tightvncviewer gives me:

Connected to RFB server, using protocol version 3.8
No authentication needed
Authentication successful
Desktop name "QEMU"
VNC server default format:
32 bits per pixel.
Least significant byte first in each pixel.
True colour: max red 255 green 255 blue 255, shift red 16 green 8
blue 0
Using default colormap which is TrueColor.  Pixel format:
32 bits per pixel.
Least significant byte first in each pixel.
True colour: max red 255 green 255 blue 255, shift red 16
green 8 blue 0
Same machine: preferring raw encoding
Rect too large: 16x4 at (2352, 1766)

gvncviewer simply says 'Disconnected from server'.

It works fine if I don't use -vga vmware.

I bisected it to commit 12b316d: ui/vnc: optimize dirty bitmap tracking

-serge

Re: [Qemu-devel] vnc regression with -vga vmware

[Gerd Hoffmann]

On Fr, 2014-03-14 at 11:06 -0500, Serge Hallyn wrote:
> Hi,
> 
> upstream git HEAD appears to have regressed with -vga vmware -vnc.
> 
> If I run 
> 
> 	./qemu-system-x86_64 -enable-kvm -vnc :1 -m 1024 -cdrom ~/trusty-desktop-amd64.iso -vga vmware
> 
> then tightvncviewer gives me:
> 
> Connected to RFB server, using protocol version 3.8
> No authentication needed
> Authentication successful
> Desktop name "QEMU"
> VNC server default format:
> 32 bits per pixel.
> Least significant byte first in each pixel.
> True colour: max red 255 green 255 blue 255, shift red 16 green 8
> blue 0
> Using default colormap which is TrueColor.  Pixel format:
> 32 bits per pixel.
> Least significant byte first in each pixel.
> True colour: max red 255 green 255 blue 255, shift red 16
> green 8 blue 0
> Same machine: preferring raw encoding
> Rect too large: 16x4 at (2352, 1766)
> 
> gvncviewer simply says 'Disconnected from server'.
> 
> It works fine if I don't use -vga vmware.
> 
> I bisected it to commit 12b316d: ui/vnc: optimize dirty bitmap tracking

Peter, that is yours, any idea what this is?

cheers,
  Gerd

Re: [Qemu-devel] vnc regression with -vga vmware

[Peter Lieven]

On 17.03.2014 14:44, Gerd Hoffmann wrote:
> On Fr, 2014-03-14 at 11:06 -0500, Serge Hallyn wrote:
>> Hi,
>>
>> upstream git HEAD appears to have regressed with -vga vmware -vnc.
>>
>> If I run
>>
>> 	./qemu-system-x86_64 -enable-kvm -vnc :1 -m 1024 -cdrom ~/trusty-desktop-amd64.iso -vga vmware
>>
>> then tightvncviewer gives me:
>>
>> Connected to RFB server, using protocol version 3.8
>> No authentication needed
>> Authentication successful
>> Desktop name "QEMU"
>> VNC server default format:
>> 32 bits per pixel.
>> Least significant byte first in each pixel.
>> True colour: max red 255 green 255 blue 255, shift red 16 green 8
>> blue 0
>> Using default colormap which is TrueColor.  Pixel format:
>> 32 bits per pixel.
>> Least significant byte first in each pixel.
>> True colour: max red 255 green 255 blue 255, shift red 16
>> green 8 blue 0
>> Same machine: preferring raw encoding
>> Rect too large: 16x4 at (2352, 1766)
>>
>> gvncviewer simply says 'Disconnected from server'.
>>
>> It works fine if I don't use -vga vmware.
>>
>> I bisected it to commit 12b316d: ui/vnc: optimize dirty bitmap tracking
> Peter, that is yours, any idea what this is?
not yet, I will have a look. at which resolution does -vga vmware work?

Peter
>
> cheers,
>    Gerd
>
>
>

Re: [Qemu-devel] vnc regression with -vga vmware

[Peter Lieven]

Serge,

can you confirm this happens at a resolution change?

Peter

On 17.03.2014 14:44, Gerd Hoffmann wrote:
> On Fr, 2014-03-14 at 11:06 -0500, Serge Hallyn wrote:
>> Hi,
>>
>> upstream git HEAD appears to have regressed with -vga vmware -vnc.
>>
>> If I run
>>
>> 	./qemu-system-x86_64 -enable-kvm -vnc :1 -m 1024 -cdrom ~/trusty-desktop-amd64.iso -vga vmware
>>
>> then tightvncviewer gives me:
>>
>> Connected to RFB server, using protocol version 3.8
>> No authentication needed
>> Authentication successful
>> Desktop name "QEMU"
>> VNC server default format:
>> 32 bits per pixel.
>> Least significant byte first in each pixel.
>> True colour: max red 255 green 255 blue 255, shift red 16 green 8
>> blue 0
>> Using default colormap which is TrueColor.  Pixel format:
>> 32 bits per pixel.
>> Least significant byte first in each pixel.
>> True colour: max red 255 green 255 blue 255, shift red 16
>> green 8 blue 0
>> Same machine: preferring raw encoding
>> Rect too large: 16x4 at (2352, 1766)
>>
>> gvncviewer simply says 'Disconnected from server'.
>>
>> It works fine if I don't use -vga vmware.
>>
>> I bisected it to commit 12b316d: ui/vnc: optimize dirty bitmap tracking
> Peter, that is yours, any idea what this is?
>
> cheers,
>    Gerd
>
>


-- 

Mit freundlichen Grüßen

Peter Lieven

...........................................................

   KAMP Netzwerkdienste GmbH
   Vestische Str. 89-91 | 46117 Oberhausen
   Tel: +49 (0) 208.89 402-50 | Fax: +49 (0) 208.89 402-40
   pl@kamp.de | http://www.kamp.de

   Geschäftsführer: Heiner Lante | Michael Lante
   Amtsgericht Duisburg | HRB Nr. 12154
   USt-Id-Nr.: DE 120607556

...........................................................

Re: [Qemu-devel] vnc regression with -vga vmware

[Serge Hallyn]

It does happen then as well (I suppose), but after X is done setting
up, it happens every time I try to connect.

Quoting Peter Lieven (pl@kamp.de):
> Serge,
> 
> can you confirm this happens at a resolution change?
> 
> Peter
> 
> On 17.03.2014 14:44, Gerd Hoffmann wrote:
> >On Fr, 2014-03-14 at 11:06 -0500, Serge Hallyn wrote:
> >>Hi,
> >>
> >>upstream git HEAD appears to have regressed with -vga vmware -vnc.
> >>
> >>If I run
> >>
> >>	./qemu-system-x86_64 -enable-kvm -vnc :1 -m 1024 -cdrom ~/trusty-desktop-amd64.iso -vga vmware
> >>
> >>then tightvncviewer gives me:
> >>
> >>Connected to RFB server, using protocol version 3.8
> >>No authentication needed
> >>Authentication successful
> >>Desktop name "QEMU"
> >>VNC server default format:
> >>32 bits per pixel.
> >>Least significant byte first in each pixel.
> >>True colour: max red 255 green 255 blue 255, shift red 16 green 8
> >>blue 0
> >>Using default colormap which is TrueColor.  Pixel format:
> >>32 bits per pixel.
> >>Least significant byte first in each pixel.
> >>True colour: max red 255 green 255 blue 255, shift red 16
> >>green 8 blue 0
> >>Same machine: preferring raw encoding
> >>Rect too large: 16x4 at (2352, 1766)
> >>
> >>gvncviewer simply says 'Disconnected from server'.
> >>
> >>It works fine if I don't use -vga vmware.
> >>
> >>I bisected it to commit 12b316d: ui/vnc: optimize dirty bitmap tracking
> >Peter, that is yours, any idea what this is?
> >
> >cheers,
> >   Gerd
> >
> >
> 
> 
> -- 
> 
> Mit freundlichen Grüßen
> 
> Peter Lieven
> 
> ...........................................................
> 
>   KAMP Netzwerkdienste GmbH
>   Vestische Str. 89-91 | 46117 Oberhausen
>   Tel: +49 (0) 208.89 402-50 | Fax: +49 (0) 208.89 402-40
>   pl@kamp.de | http://www.kamp.de
> 
>   Geschäftsführer: Heiner Lante | Michael Lante
>   Amtsgericht Duisburg | HRB Nr. 12154
>   USt-Id-Nr.: DE 120607556
> 
> ...........................................................
> 

Re: [Qemu-devel] vnc regression with -vga vmware

[Peter Lieven]

I found 2 issues:

- with vmware VGA the server surface and the client desktop size are out of sync for some time
at a resolution change. the server surface gets updates for x coordinates that are out of bound
for the client.
- the max width of the client (2360) is not dividable by 16 (VNC_DIRTY_PIXELS_PER_BIT).

I will try to fix this in ui/vnc but we should definetly look for the root cause.

Peter



On 17.03.2014 15:10, Serge Hallyn wrote:
> It does happen then as well (I suppose), but after X is done setting
> up, it happens every time I try to connect.
>
> Quoting Peter Lieven (pl@kamp.de):
>> Serge,
>>
>> can you confirm this happens at a resolution change?
>>
>> Peter
>>
>> On 17.03.2014 14:44, Gerd Hoffmann wrote:
>>> On Fr, 2014-03-14 at 11:06 -0500, Serge Hallyn wrote:
>>>> Hi,
>>>>
>>>> upstream git HEAD appears to have regressed with -vga vmware -vnc.
>>>>
>>>> If I run
>>>>
>>>> 	./qemu-system-x86_64 -enable-kvm -vnc :1 -m 1024 -cdrom ~/trusty-desktop-amd64.iso -vga vmware
>>>>
>>>> then tightvncviewer gives me:
>>>>
>>>> Connected to RFB server, using protocol version 3.8
>>>> No authentication needed
>>>> Authentication successful
>>>> Desktop name "QEMU"
>>>> VNC server default format:
>>>> 32 bits per pixel.
>>>> Least significant byte first in each pixel.
>>>> True colour: max red 255 green 255 blue 255, shift red 16 green 8
>>>> blue 0
>>>> Using default colormap which is TrueColor.  Pixel format:
>>>> 32 bits per pixel.
>>>> Least significant byte first in each pixel.
>>>> True colour: max red 255 green 255 blue 255, shift red 16
>>>> green 8 blue 0
>>>> Same machine: preferring raw encoding
>>>> Rect too large: 16x4 at (2352, 1766)
>>>>
>>>> gvncviewer simply says 'Disconnected from server'.
>>>>
>>>> It works fine if I don't use -vga vmware.
>>>>
>>>> I bisected it to commit 12b316d: ui/vnc: optimize dirty bitmap tracking
>>> Peter, that is yours, any idea what this is?
>>>
>>> cheers,
>>>    Gerd
>>>
>>>
>>
>> -- 
>>
>> Mit freundlichen Grüßen
>>
>> Peter Lieven
>>
>> ...........................................................
>>
>>    KAMP Netzwerkdienste GmbH
>>    Vestische Str. 89-91 | 46117 Oberhausen
>>    Tel: +49 (0) 208.89 402-50 | Fax: +49 (0) 208.89 402-40
>>    pl@kamp.de | http://www.kamp.de
>>
>>    Geschäftsführer: Heiner Lante | Michael Lante
>>    Amtsgericht Duisburg | HRB Nr. 12154
>>    USt-Id-Nr.: DE 120607556
>>
>> ...........................................................
>>

Re: [Qemu-devel] vnc regression with -vga vmware

[Serge Hallyn]

Quoting Peter Lieven (lieven-lists@dlhnet.de):
> I found 2 issues:
> 
> - with vmware VGA the server surface and the client desktop size are out of sync for some time
> at a resolution change. the server surface gets updates for x coordinates that are out of bound
> for the client.
> - the max width of the client (2360) is not dividable by 16 (VNC_DIRTY_PIXELS_PER_BIT).
> 
> I will try to fix this in ui/vnc but we should definetly look for the root cause.

Thanks, Peter!

-serge

Re: [Qemu-devel] vnc regression with -vga vmware

[Peter Lieven]

On 17.03.2014 16:19, Serge Hallyn wrote:
> Quoting Peter Lieven (lieven-lists@dlhnet.de):
>> I found 2 issues:
>>
>> - with vmware VGA the server surface and the client desktop size are out of sync for some time
>> at a resolution change. the server surface gets updates for x coordinates that are out of bound
>> for the client.
>> - the max width of the client (2360) is not dividable by 16 (VNC_DIRTY_PIXELS_PER_BIT).
>>
>> I will try to fix this in ui/vnc but we should definetly look for the root cause.
> Thanks, Peter!
>
> -serge


The vmware vga driver seems to do some nasty things.
I receive the msg like this (independent of ui/vnc):
vmsvga_update_rect: update x was < 0 (-65)

Can you try the following patch:

diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index bd2c108..6ae3348 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -25,6 +25,7 @@
  #include "hw/loader.h"
  #include "trace.h"
  #include "ui/console.h"
+#include "ui/vnc.h"
  #include "hw/pci/pci.h"

  #undef VERBOSE
@@ -218,7 +219,7 @@ enum {

  /* These values can probably be changed arbitrarily.  */
  #define SVGA_SCRATCH_SIZE               0x8000
-#define SVGA_MAX_WIDTH                  2360
+#define SVGA_MAX_WIDTH                  ROUND_UP(2360, VNC_DIRTY_PIXELS_PER_BIT)
  #define SVGA_MAX_HEIGHT                 1770

  #ifdef VERBOSE
diff --git a/ui/vnc.c b/ui/vnc.c
index 9c84b3e..5925774 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -888,7 +888,7 @@ static int vnc_update_client(VncState *vs, int has_dirty, bool sync)
          VncDisplay *vd = vs->vd;
          VncJob *job;
          int y;
-        int height;
+        int height, width;
          int n = 0;

          if (vs->output.offset && !vs->audio_cap && !vs->force_update)
@@ -907,6 +907,7 @@ static int vnc_update_client(VncState *vs, int has_dirty, bool sync)
          job = vnc_job_new(vs);

          height = MIN(pixman_image_get_height(vd->server), vs->client_height);
+        width = MIN(pixman_image_get_width(vd->server), vs->client_width);

          y = 0;
          for (;;) {
@@ -925,8 +926,11 @@ static int vnc_update_client(VncState *vs, int has_dirty, bool sync)
                                      VNC_DIRTY_BPL(vs), x);
              bitmap_clear(vs->dirty[y], x, x2 - x);
              h = find_and_clear_dirty_height(vs, y, x, x2, height);
-            n += vnc_job_add_rect(job, x * VNC_DIRTY_PIXELS_PER_BIT, y,
-                                  (x2 - x) * VNC_DIRTY_PIXELS_PER_BIT, h);
+            x2 = MIN(x2, width / VNC_DIRTY_PIXELS_PER_BIT);
+            if (x2 > x) {
+                n += vnc_job_add_rect(job, x * VNC_DIRTY_PIXELS_PER_BIT, y,
+                                      (x2 - x) * VNC_DIRTY_PIXELS_PER_BIT, h);
+            }
          }

          vnc_job_push(job);

Peter

Re: [Qemu-devel] vnc regression with -vga vmware

[Serge Hallyn]

Quoting Peter Lieven (pl@kamp.de):
> 
> On 17.03.2014 16:19, Serge Hallyn wrote:
> >Quoting Peter Lieven (lieven-lists@dlhnet.de):
> >>I found 2 issues:
> >>
> >>- with vmware VGA the server surface and the client desktop size are out of sync for some time
> >>at a resolution change. the server surface gets updates for x coordinates that are out of bound
> >>for the client.
> >>- the max width of the client (2360) is not dividable by 16 (VNC_DIRTY_PIXELS_PER_BIT).
> >>
> >>I will try to fix this in ui/vnc but we should definetly look for the root cause.
> >Thanks, Peter!
> >
> >-serge
> 
> 
> The vmware vga driver seems to do some nasty things.
> I receive the msg like this (independent of ui/vnc):
> vmsvga_update_rect: update x was < 0 (-65)
> 
> Can you try the following patch:

Indeed, with this patch it seems quite stable, thanks.

-serge

> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
> index bd2c108..6ae3348 100644
> --- a/hw/display/vmware_vga.c
> +++ b/hw/display/vmware_vga.c
> @@ -25,6 +25,7 @@
>  #include "hw/loader.h"
>  #include "trace.h"
>  #include "ui/console.h"
> +#include "ui/vnc.h"
>  #include "hw/pci/pci.h"
> 
>  #undef VERBOSE
> @@ -218,7 +219,7 @@ enum {
> 
>  /* These values can probably be changed arbitrarily.  */
>  #define SVGA_SCRATCH_SIZE               0x8000
> -#define SVGA_MAX_WIDTH                  2360
> +#define SVGA_MAX_WIDTH                  ROUND_UP(2360, VNC_DIRTY_PIXELS_PER_BIT)
>  #define SVGA_MAX_HEIGHT                 1770
> 
>  #ifdef VERBOSE
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 9c84b3e..5925774 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -888,7 +888,7 @@ static int vnc_update_client(VncState *vs, int has_dirty, bool sync)
>          VncDisplay *vd = vs->vd;
>          VncJob *job;
>          int y;
> -        int height;
> +        int height, width;
>          int n = 0;
> 
>          if (vs->output.offset && !vs->audio_cap && !vs->force_update)
> @@ -907,6 +907,7 @@ static int vnc_update_client(VncState *vs, int has_dirty, bool sync)
>          job = vnc_job_new(vs);
> 
>          height = MIN(pixman_image_get_height(vd->server), vs->client_height);
> +        width = MIN(pixman_image_get_width(vd->server), vs->client_width);
> 
>          y = 0;
>          for (;;) {
> @@ -925,8 +926,11 @@ static int vnc_update_client(VncState *vs, int has_dirty, bool sync)
>                                      VNC_DIRTY_BPL(vs), x);
>              bitmap_clear(vs->dirty[y], x, x2 - x);
>              h = find_and_clear_dirty_height(vs, y, x, x2, height);
> -            n += vnc_job_add_rect(job, x * VNC_DIRTY_PIXELS_PER_BIT, y,
> -                                  (x2 - x) * VNC_DIRTY_PIXELS_PER_BIT, h);
> +            x2 = MIN(x2, width / VNC_DIRTY_PIXELS_PER_BIT);
> +            if (x2 > x) {
> +                n += vnc_job_add_rect(job, x * VNC_DIRTY_PIXELS_PER_BIT, y,
> +                                      (x2 - x) * VNC_DIRTY_PIXELS_PER_BIT, h);
> +            }
>          }
> 
>          vnc_job_push(job);
> 
> Peter
> 
> 
> 

Re: [Qemu-devel] vnc regression with -vga vmware

[Peter Lieven]

On 17.03.2014 18:35, Serge Hallyn wrote:
> Quoting Peter Lieven (pl@kamp.de):
>> On 17.03.2014 16:19, Serge Hallyn wrote:
>>> Quoting Peter Lieven (lieven-lists@dlhnet.de):
>>>> I found 2 issues:
>>>>
>>>> - with vmware VGA the server surface and the client desktop size are out of sync for some time
>>>> at a resolution change. the server surface gets updates for x coordinates that are out of bound
>>>> for the client.
>>>> - the max width of the client (2360) is not dividable by 16 (VNC_DIRTY_PIXELS_PER_BIT).
>>>>
>>>> I will try to fix this in ui/vnc but we should definetly look for the root cause.
>>> Thanks, Peter!
>>>
>>> -serge
>>
>> The vmware vga driver seems to do some nasty things.
>> I receive the msg like this (independent of ui/vnc):
>> vmsvga_update_rect: update x was < 0 (-65)
>>
>> Can you try the following patch:
> Indeed, with this patch it seems quite stable, thanks.
Thanks for the feedback. I will send a patch to the list
shortly.

Peter
>
> -serge
>
>> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
>> index bd2c108..6ae3348 100644
>> --- a/hw/display/vmware_vga.c
>> +++ b/hw/display/vmware_vga.c
>> @@ -25,6 +25,7 @@
>>   #include "hw/loader.h"
>>   #include "trace.h"
>>   #include "ui/console.h"
>> +#include "ui/vnc.h"
>>   #include "hw/pci/pci.h"
>>
>>   #undef VERBOSE
>> @@ -218,7 +219,7 @@ enum {
>>
>>   /* These values can probably be changed arbitrarily.  */
>>   #define SVGA_SCRATCH_SIZE               0x8000
>> -#define SVGA_MAX_WIDTH                  2360
>> +#define SVGA_MAX_WIDTH                  ROUND_UP(2360, VNC_DIRTY_PIXELS_PER_BIT)
>>   #define SVGA_MAX_HEIGHT                 1770
>>
>>   #ifdef VERBOSE
>> diff --git a/ui/vnc.c b/ui/vnc.c
>> index 9c84b3e..5925774 100644
>> --- a/ui/vnc.c
>> +++ b/ui/vnc.c
>> @@ -888,7 +888,7 @@ static int vnc_update_client(VncState *vs, int has_dirty, bool sync)
>>           VncDisplay *vd = vs->vd;
>>           VncJob *job;
>>           int y;
>> -        int height;
>> +        int height, width;
>>           int n = 0;
>>
>>           if (vs->output.offset && !vs->audio_cap && !vs->force_update)
>> @@ -907,6 +907,7 @@ static int vnc_update_client(VncState *vs, int has_dirty, bool sync)
>>           job = vnc_job_new(vs);
>>
>>           height = MIN(pixman_image_get_height(vd->server), vs->client_height);
>> +        width = MIN(pixman_image_get_width(vd->server), vs->client_width);
>>
>>           y = 0;
>>           for (;;) {
>> @@ -925,8 +926,11 @@ static int vnc_update_client(VncState *vs, int has_dirty, bool sync)
>>                                       VNC_DIRTY_BPL(vs), x);
>>               bitmap_clear(vs->dirty[y], x, x2 - x);
>>               h = find_and_clear_dirty_height(vs, y, x, x2, height);
>> -            n += vnc_job_add_rect(job, x * VNC_DIRTY_PIXELS_PER_BIT, y,
>> -                                  (x2 - x) * VNC_DIRTY_PIXELS_PER_BIT, h);
>> +            x2 = MIN(x2, width / VNC_DIRTY_PIXELS_PER_BIT);
>> +            if (x2 > x) {
>> +                n += vnc_job_add_rect(job, x * VNC_DIRTY_PIXELS_PER_BIT, y,
>> +                                      (x2 - x) * VNC_DIRTY_PIXELS_PER_BIT, h);
>> +            }
>>           }
>>
>>           vnc_job_push(job);
>>
>> Peter
>>
>>
>>