remove qemu shadow_vram patch for performance

PATCH In review Zhai, Edwin 8 participants 14 messages Mar 15, 2007 – Mar 23, 2007

lore

Share & tools

lore

Zhai, Edwin

author

2007-03-15 03:30 UTC #0 lore raw

PATCH 1/1 remove qemu shadow_vram patch for performance

remove qemu shadow_vram patch and force a whole screen update each time for performance.

W/O this patch, there is huge performance drop in HVM domain when adding other guest(windows or linux with xwindow).

shadow_vram_revert.patch - revert the shadow_vram patch shadow_vram_force_update.patch - explictly redraw screen each time

Signed-off-by Liang, Kan <[email protected]>

Signed-off-by Zhai, Edwin <[email protected]>

Signature · 2 lines

best rgds,
edwin

# HG changeset patch
# User Edwin Zhai <[email protected]>
# Date 1173862864 -28800
# Node ID 6c4f61e2be6f8bf4a12d4044c97f47eb1d14b860 # Parent 3c38150de7fd4328a1d8e95b70b966d2ac481e12 revert shadow_vram patch

tools/ioemu/hw/vga.c +0 ~~-112~~

diff -r 3c38150de7fd -r 6c4f61e2be6f tools/ioemu/hw/vga.c
@@ -1359,105 +1359,6 @@ void vga_invalidate_scanlines(VGAState *
     }
 }
 
-static inline int cmp_vram(VGAState *s, int offset, int n)
-{
-    long *vp, *sp;
-
-    if (s->vram_shadow == NULL)
-        return 1;
-    vp = (long *)(s->vram_ptr + offset);
-    sp = (long *)(s->vram_shadow + offset);
-    while ((n -= sizeof(*vp)) >= 0) {
-        if (*vp++ != *sp++) {
-            memcpy(sp - 1, vp - 1, n + sizeof(*vp));
-            return 1;
-        }
-    }
-    return 0;
-}
-
-#ifdef USE_SSE2
-
-#include <signal.h>
-#include <setjmp.h>
-#include <emmintrin.h>
-
-int sse2_ok = 1;
-
-static inline unsigned int cpuid_edx(unsigned int op)
-{
-    unsigned int eax, edx;
-
-#ifdef __x86_64__
-#define __bx "rbx"
-#else
-#define __bx "ebx"
-#endif
-    __asm__("push %%"__bx"; cpuid; pop %%"__bx
-            : "=a" (eax), "=d" (edx)
-            : "0" (op)
-            : "cx");
-#undef __bx
-
-    return edx;
-}
-
-jmp_buf sse_jbuf;
-
-void intr(int sig)
-{
-    sse2_ok = 0;
-    longjmp(sse_jbuf, 1);
-}
-
-void check_sse2(void)
-{
-    /* Check 1: What does CPUID say? */
-    if ((cpuid_edx(1) & 0x4000000) == 0) {
-        sse2_ok = 0;
-        return;
-    }
-
-    /* Check 2: Can we use SSE2 in anger? */
-    signal(SIGILL, intr);
-    if (setjmp(sse_jbuf) == 0)
-        __asm__("xorps %xmm0,%xmm0\n");
-}
-
-int vram_dirty(VGAState *s, int offset, int n)
-{
-    __m128i *sp, *vp;
-
-    if (s->vram_shadow == NULL)
-        return 1;
-    if (sse2_ok == 0)
-        return cmp_vram(s, offset, n);
-    vp = (__m128i *)(s->vram_ptr + offset);
-    sp = (__m128i *)(s->vram_shadow + offset);
-    while ((n -= sizeof(*vp)) >= 0) {
-        if (_mm_movemask_epi8(_mm_cmpeq_epi8(*sp, *vp)) != 0xffff) {
-            while (n >= 0) {
-                _mm_store_si128(sp++, _mm_load_si128(vp++));
-                n -= sizeof(*vp);
-            }
-            return 1;
-        }
-        sp++;
-        vp++;
-    }
-    return 0;
-}
-#else /* !USE_SSE2 */
-int vram_dirty(VGAState *s, int offset, int n)
-{
-    return cmp_vram(s, offset, n);
-}
-
-void check_sse2(void)
-{
-}
-#endif /* !USE_SSE2 */
-
 /* 
  * graphic modes
  */

@@ -1554,11 +1455,6 @@ static void vga_draw_graphic(VGAState *s
     printf("w=%d h=%d v=%d line_offset=%d cr[0x09]=0x%02x cr[0x17]=0x%02x linecmp=%d sr[0x01]=0x%02x\n",
            width, height, v, line_offset, s->cr[9], s->cr[0x17], s->line_compare, s->sr[0x01]);
 #endif
-
-    for (y = 0; y < s->vram_size; y += TARGET_PAGE_SIZE)
-        if (vram_dirty(s, y, TARGET_PAGE_SIZE))
-            cpu_physical_memory_set_dirty(s->vram_offset + y);
-
     addr1 = (s->start_addr * 4);
     bwidth = width * 4;
     y_start = -1;

@@ -1994,14 +1890,6 @@ void vga_common_init(VGAState *s, Displa
 
     vga_reset(s);
 
-    check_sse2();
-    s->vram_shadow = qemu_malloc(vga_ram_size+TARGET_PAGE_SIZE+1);
-    if (s->vram_shadow == NULL)
-        fprintf(stderr, "Cannot allocate %d bytes for VRAM shadow, "
-                "mouse will be slow\n", vga_ram_size);
-    s->vram_shadow = (uint8_t *)((long)(s->vram_shadow + TARGET_PAGE_SIZE - 1)
-                                 & ~(TARGET_PAGE_SIZE - 1));
-
     /* Video RAM must be 128-bit aligned for SSE optimizations later */
     s->vram_alloc = qemu_malloc(vga_ram_size + 15);
     s->vram_ptr = (uint8_t *)((long)(s->vram_alloc + 15) & ~15L);

tools/ioemu/hw/vga_int.h +0 -1

diff -r 3c38150de7fd -r 6c4f61e2be6f tools/ioemu/hw/vga_int.h
@@ -80,7 +80,6 @@
 #define VGA_STATE_COMMON                                                \
     uint8_t *vram_alloc;                                                \
     uint8_t *vram_ptr;                                                  \
-    uint8_t *vram_shadow;                                               \
     unsigned long vram_offset;                                          \
     unsigned int vram_size;                                             \
     unsigned long bios_offset;                                          \

# HG changeset patch
# User Edwin Zhai <[email protected]>
# Date 1173872741 -28800
# Node ID c23a03f6ae56dd4699451766b12ca7d54ef6e703 # Parent 6c4f61e2be6f8bf4a12d4044c97f47eb1d14b860 explict redraw screen as no shadow_vram now

tools/ioemu/hw/vga.c +3 -0

diff -r 6c4f61e2be6f -r c23a03f6ae56 tools/ioemu/hw/vga.c
@@ -1486,6 +1486,9 @@ static void vga_draw_graphic(VGAState *s
         }
         /* explicit invalidation for the hardware cursor */
         update |= (s->invalidated_y_table[y >> 5] >> (y & 0x1f)) & 1;
+
+        /* force vga update as no shadow vram now */
+        update |= VGA_DIRTY_FLAG;
         if (update) {
             if (y_start < 0)
                 y_start = y;

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Keir Fraser

2007-03-15 10:50 UTC #1 lore raw

Quoted 8 lines · Edwin

remove qemu shadow_vram patch and force a whole screen update each time for performance.
W/O this patch, there is huge performance drop in HVM domain when adding other guest(windows or linux with xwindow).
shadow_vram_revert.patch - revert the shadow_vram patch shadow_vram_force_update.patch - explictly redraw screen each time

How can updating the whole screen 30 times a second be faster than the memcmp() that we do currently?

-- Keir

Zhai, Edwin

author

2007-03-16 03:04 UTC #2 lore raw

Quoted 8 lines · Keir Fraser

Quoted 3 lines · Edwin
remove qemu shadow_vram patch and force a whole screen update each time for performance.

Quoted 8 lines

Quoted 5 lines
W/O this patch, there is huge performance drop in HVM domain when adding other guest(windows or linux with xwindow).
shadow_vram_revert.patch - revert the shadow_vram patch shadow_vram_force_update.patch - explictly redraw screen each time
How can updating the whole screen 30 times a second be faster than the memcmp() that we do currently?

as far as i can tell, the bottle neck is that orig method does memcmp and memcpy byte by byte. furthermore, orig method can void a update by multiple memcmp only if all bytes are equal, which is in the minority.

there is no doubt we need a vram dirty for qemu, but current one is not the best. we can make a new one in future by shadow or something else.

thanks,

Quoted 3 lines

-- Keir

Signature · 2 lines

best rgds,
edwin

Li, Xin B

2007-03-20 08:38 UTC #3 lore raw

RE: [PATCH][HVM] remove qemu shadow_vram patch forperformance

Keir, do you think this patch is OK?
-Xin

Quoted 55 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Zhai, Edwin
Sent: Friday, March 16, 2007 11:04 AM
To: Keir Fraser
Cc: Ian Pratt; [email protected]; Zhai, Edwin
Subject: Re: [Xen-devel] [PATCH][HVM] remove qemu shadow_vram patch forperformance
Quoted 6 lines · Keir Fraser
Quoted 1 line · Edwin
remove qemu shadow_vram patch and force a whole screen
update each time for
Quoted 3 lines
Quoted 3 lines
performance.
W/O this patch, there is huge performance drop in HVM
domain when adding other
Quoted 6 lines
Quoted 4 lines
guest(windows or linux with xwindow).
shadow_vram_revert.patch - revert the shadow_vram patch shadow_vram_force_update.patch - explictly redraw screen each time
How can updating the whole screen 30 times a second be
faster than the
Quoted 1 line
memcmp() that we do currently?
as far as i can tell, the bottle neck is that orig method does memcmp and memcpy
byte by byte. furthermore, orig method can void a update by multiple memcmp only
if all bytes are equal, which is in the minority.
there is no doubt we need a vram dirty for qemu, but current one is not the
best. we can make a new one in future by shadow or something else.
thanks,
Quoted 3 lines
-- Keir
Signature · 7 lines
best rgds,
edwin

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Keir Fraser

2007-03-20 08:47 UTC #4 lore raw

Re: [PATCH][HVM] remove qemu shadow_vram patch forperformance

I punted on this one. :-) Christian wasn't sure about it either, hence it's not been checked in.

-- Keir

Quoted 63 lines · Xin B

Keir, do you think this patch is OK?
-Xin
Quoted 40 lines
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Zhai, Edwin
Sent: Friday, March 16, 2007 11:04 AM
To: Keir Fraser
Cc: Ian Pratt; [email protected]; Zhai, Edwin
Subject: Re: [Xen-devel] [PATCH][HVM] remove qemu shadow_vram patch forperformance
Quoted 6 lines · Keir Fraser
Quoted 1 line · Edwin
remove qemu shadow_vram patch and force a whole screen
update each time for
Quoted 3 lines
Quoted 3 lines
performance.
W/O this patch, there is huge performance drop in HVM
domain when adding other
Quoted 6 lines
Quoted 4 lines
guest(windows or linux with xwindow).
shadow_vram_revert.patch - revert the shadow_vram patch shadow_vram_force_update.patch - explictly redraw screen each time
How can updating the whole screen 30 times a second be
faster than the
Quoted 1 line
memcmp() that we do currently?
as far as i can tell, the bottle neck is that orig method does memcmp and memcpy
byte by byte. furthermore, orig method can void a update by multiple memcmp only
if all bytes are equal, which is in the minority.
there is no doubt we need a vram dirty for qemu, but current one is not the
best. we can make a new one in future by shadow or something else.
Quoted 15 lines
thanks,
Quoted 3 lines
-- Keir
Signature · 7 lines
best rgds,
edwin

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Li, Xin B

2007-03-20 09:36 UTC #5 lore raw

RE: [PATCH][HVM] remove qemu shadow_vram patch forperformance

The major problem this patch is trying to fix is the performance drop when multiple HVM guests are running, and I think we need a better solution for this.
-Xin

Quoted 81 lines

-----Original Message-----
From: Keir Fraser [mailto:[email protected]]
Sent: Tuesday, March 20, 2007 4:48 PM
To: Li, Xin B; Zhai, Edwin; Keir Fraser
Cc: Ian Pratt; [email protected]
Subject: Re: [Xen-devel] [PATCH][HVM] remove qemu shadow_vram patch forperformance
I punted on this one. :-) Christian wasn't sure about it either, hence it's
not been checked in.
-- Keir
Quoted 6 lines · Xin B
Keir, do you think this patch is OK?
-Xin
Quoted 3 lines
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of
Zhai, Edwin
Quoted 57 lines
Quoted 37 lines
Sent: Friday, March 16, 2007 11:04 AM
To: Keir Fraser
Cc: Ian Pratt; [email protected]; Zhai, Edwin
Subject: Re: [Xen-devel] [PATCH][HVM] remove qemu shadow_vram patch forperformance
Quoted 6 lines · Keir Fraser
Quoted 1 line · Edwin
remove qemu shadow_vram patch and force a whole screen
update each time for
Quoted 3 lines
Quoted 3 lines
performance.
W/O this patch, there is huge performance drop in HVM
domain when adding other
Quoted 6 lines
Quoted 4 lines
guest(windows or linux with xwindow).
shadow_vram_revert.patch - revert the shadow_vram patch shadow_vram_force_update.patch - explictly redraw screen each time
How can updating the whole screen 30 times a second be
faster than the
Quoted 1 line
memcmp() that we do currently?
as far as i can tell, the bottle neck is that orig method does memcmp and memcpy
byte by byte. furthermore, orig method can void a update by multiple memcmp only
if all bytes are equal, which is in the minority.
there is no doubt we need a vram dirty for qemu, but current one is not the
best. we can make a new one in future by shadow or something else.
Quoted 15 lines
thanks,
Quoted 3 lines
-- Keir
Signature · 7 lines
best rgds,
edwin

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Keir Fraser

2007-03-20 09:45 UTC #6 lore raw

Re: [PATCH][HVM] remove qemu shadow_vram patch forperformance

Quoted 5 lines · Xin B

The major problem this patch is trying to fix is the performance drop when multiple HVM guests are running, and I think we need a better solution for this.
From my own reading of the code I would have thought that by removing that

SSE memcmp we'd be replacing periodic SSE block memcmp with periodic pixel-by-pixel comparison (which is what I think we end up doing if we get past the SSE memcmp?). I'm thinking particularly of the VNC side of things rather than SDL here, by the way: you guys use SDL most of the time, right?

-- Keir

Li, Xin B

2007-03-20 09:48 UTC #7 lore raw

RE: [PATCH][HVM] remove qemu shadow_vram patch forperformance

Quoted 10 lines

-----Original Message-----
From: Keir Fraser [mailto:[email protected]]
Sent: Tuesday, March 20, 2007 5:45 PM
To: Li, Xin B; Zhai, Edwin; Keir Fraser
Cc: Ian Pratt; [email protected]
Subject: Re: [Xen-devel] [PATCH][HVM] remove qemu shadow_vram patch forperformance

Quoted 7 lines

Quoted 3 lines · Xin B
The major problem this patch is trying to fix is the performance drop when multiple HVM guests are running, and I think we need a better solution for this.
From my own reading of the code I would have thought that by removing

that

Quoted 2 lines

SSE memcmp we'd be replacing periodic SSE block memcmp with periodic pixel-by-pixel comparison (which is what I think we end up doing if we

get

Quoted 3 lines

past the SSE memcmp?). I'm thinking particularly of the VNC side of things rather than SDL here,
by the way: you guys use SDL most of the time, right?

Yes :-)
-Xin

Zhai, Edwin

author

2007-03-21 02:38 UTC #8 lore raw

Re: [PATCH][HVM] remove qemu shadow_vram patch forperformance

In reply to Keir Fraser

Quoted 11 lines · Keir Fraser

Quoted 5 lines · Xin B
The major problem this patch is trying to fix is the performance drop when multiple HVM guests are running, and I think we need a better solution for this.
From my own reading of the code I would have thought that by removing that
SSE memcmp we'd be replacing periodic SSE block memcmp with periodic pixel-by-pixel comparison (which is what I think we end up doing if we get past the SSE memcmp?). I'm thinking particularly of the VNC side of things rather than SDL here, by the way: you guys use SDL most of the time, right?

our developer prefer SDL, but our performance data shows that VNS also has such issue, although is a little better than SDL.

Quoted 3 lines

-- Keir

Signature · 2 lines

best rgds,
edwin

Dong, Eddie

2007-03-21 20:50 UTC #9 lore raw

RE: [PATCH][HVM] remove qemu shadow_vram patchforperformance

In reply to Keir Fraser

Quoted 6 lines · Keir Fraser

Quoted 3 lines · Xin B
The major problem this patch is trying to fix is the performance drop when multiple HVM guests are running, and I think we need a better solution for this.

Quoted 8 lines

Quoted 2 lines
From my own reading of the code I would have thought that by removing that
SSE memcmp we'd be replacing periodic SSE block memcmp with periodic pixel-by-pixel comparison (which is what I think we end up doing if we get past the SSE memcmp?). I'm thinking particularly of the VNC side of things rather than SDL here, by the way: you guys use SDL most of the time, right?

Keir:
I confirmed with Don that this memcmp will happen no matter the
Qemu
window is active or not (i.e. the screen is covered by other Windows). So to
answer the question Anthony and people have, using memcmp all Qemu window
need to do periodic memory comparation and may have many page fault when accessing to shadow vram buffer used for comparation, and thus eat a lot of CPU cycle and system bus cycles. Without the memcmp, only the top Qemu window (usually most other windows are hidden) will do full screen update, while others won't.
In benchmark where we run 8 VMs, it is very obvious that we will
benefit from no memcmp. Given that using shadow page table to indicate a dirty page needs to solve the gpn or mfn to multiple gva backlink issue,
pulling in this solution for 3.0.5 will increase HVM scalability obviously.
thanks,eddie

Christian Limpach

2007-03-22 23:40 UTC #10 lore raw

Re: [PATCH][HVM] remove qemu shadow_vram patch forperformance

In reply to Keir Fraser

Quoted 2 lines · Keir Fraser

I punted on this one. :-) Christian wasn't sure about it either, hence it's not been checked in.

My main concern is that with the patch we will be doing the transform from vga ram into the display memory everytime (vga_draw_line, some versions are quite expensive) and I don't see how that could be faster than doing the sse2 optimized compare and copy?

When you test this, are the displays active or not, i.e. is the display changing a lot or are they at the screen saver?

Maybe if the SDL frontend notices that its window is obscured, it should tell the rest of the code that a scan is not required...

Same for VNC...

christian

Quoted 76 lines

-- Keir
Quoted 59 lines · Xin B
Keir, do you think this patch is OK?
-Xin
Quoted 46 lines
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Zhai, Edwin
Sent: Friday, March 16, 2007 11:04 AM
To: Keir Fraser
Cc: Ian Pratt; [email protected]; Zhai, Edwin
Subject: Re: [Xen-devel] [PATCH][HVM] remove qemu shadow_vram patch forperformance
Quoted 6 lines · Keir Fraser
Quoted 1 line · Edwin
remove qemu shadow_vram patch and force a whole screen
update each time for
Quoted 3 lines
Quoted 3 lines
performance.
W/O this patch, there is huge performance drop in HVM
domain when adding other
Quoted 6 lines
Quoted 4 lines
guest(windows or linux with xwindow).
shadow_vram_revert.patch - revert the shadow_vram patch shadow_vram_force_update.patch - explictly redraw screen each time
How can updating the whole screen 30 times a second be
faster than the
Quoted 1 line
memcmp() that we do currently?
as far as i can tell, the bottle neck is that orig method does memcmp and memcpy
byte by byte. furthermore, orig method can void a update by multiple memcmp only
if all bytes are equal, which is in the minority.
there is no doubt we need a vram dirty for qemu, but current one is not the
best. we can make a new one in future by shadow or something else.
thanks,
Quoted 3 lines
-- Keir
Quoted 9 lines
--
best rgds,
edwin
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
Quoted 4 lines
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Dong, Eddie

2007-03-23 00:38 UTC #11 lore raw

RE: [PATCH][HVM] remove qemu shadow_vram patchforperformance

Quoted 9 lines · Christian Limpach

Quoted 2 lines · Keir Fraser
I punted on this one. :-) Christian wasn't sure about it either, hence it's not been checked in.
My main concern is that with the patch we will be doing the transform from vga ram into the display memory everytime (vga_draw_line, some versions are quite expensive) and I don't see how that could be faster than doing the sse2 optimized compare and copy?

Quoted 8 lines

When you test this, are the displays active or not, i.e. is the display changing a lot or are they at the screen saver?
Maybe if the SDL frontend notices that its window is obscured, it should tell the rest of the code that a scan is not required...
Same for VNC...

Christian:
It doesn't matter if the screen is changing contents or not.
Comparing shadow vram with vram already consumes 2X memory bus cycle. In case of 8 VMs, we get 16X memory bus cycle (and a lot of cache flush , page fault for shadow vram and flush of other TLBs).
While w/o memcmp, we only got 1 full screen update.
Given that in multi core environment, different processor
running
different Qemu Windows will compete the memory access. So it seems very nature the compare approach will become a bottle neck when VM # increases.
BTW, We didn't see performance issue with single VM either, and
in multiple VM environment, most Qemu windows are usually hidden.

thanks, eddie

Anthony Liguori

2007-03-21 21:09 UTC #12 lore raw

In reply to Keir Fraser

Quoted 7 lines · Keir Fraser

Quoted 3 lines · Edwin
remove qemu shadow_vram patch and force a whole screen update each time for performance.

Quoted 8 lines

Quoted 5 lines
W/O this patch, there is huge performance drop in HVM domain when adding other guest(windows or linux with xwindow).
shadow_vram_revert.patch - revert the shadow_vram patch shadow_vram_force_update.patch - explictly redraw screen each time
How can updating the whole screen 30 times a second be faster than the memcmp() that we do currently?

It really depends. The VNC display already has a minimization mechanism so doign the memcmp() in the vga driver doesn't help at all.

For SDL, when using X, it's going to be doing an XShmImage so the difference is only in the size of update (no data is transferred to the X server). If the X server is double buffering the framebuffer (which I have to assume it's doing), then you're just paying the cost of a memcpy() in the X server that you would normally pay in qemu-dm.

However, if you're forwarding SDL over X, or for some reason your X server isn't using XShmImage, this patch will slow things down considerably.

Perhaps this should be made a configuration option?

Regards,

Anthony Liguori

Quoted 1 line

-- Keir

Anthony Liguori

2007-03-21 21:09 UTC #13 lore raw

In reply to Keir Fraser

Quoted 7 lines · Keir Fraser

Quoted 3 lines · Edwin
remove qemu shadow_vram patch and force a whole screen update each time for performance.

Quoted 8 lines

Quoted 5 lines
W/O this patch, there is huge performance drop in HVM domain when adding other guest(windows or linux with xwindow).
shadow_vram_revert.patch - revert the shadow_vram patch shadow_vram_force_update.patch - explictly redraw screen each time
How can updating the whole screen 30 times a second be faster than the memcmp() that we do currently?

It really depends. The VNC display already has a minimization mechanism so doign the memcmp() in the vga driver doesn't help at all.

However, if you're forwarding SDL over X, or for some reason your X server isn't using XShmImage, this patch will slow things down considerably.

Perhaps this should be made a configuration option?

Regards,

Anthony Liguori

Quoted 1 line

-- Keir