diff mbox

[12/21] Remove odd hack in vga.c

Message ID 1241040038-17183-13-git-send-email-aliguori@us.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Anthony Liguori April 29, 2009, 9:20 p.m. UTC
I looked closely at the vga code in kvm-userspace a while ago and merged
every fix I could understand into upstream QEMU.  This particular change makes
no sense to me.  I could not figure out from revision history what it actually
fixed.  I'm fairly certain it's not useful today.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/vga.c |   27 ++++-----------------------
 1 files changed, 4 insertions(+), 23 deletions(-)

Comments

Avi Kivity April 30, 2009, 9:39 a.m. UTC | #1
Anthony Liguori wrote:
> I looked closely at the vga code in kvm-userspace a while ago and merged
> every fix I could understand into upstream QEMU.  This particular change makes
> no sense to me.  I could not figure out from revision history what it actually
> fixed.  I'm fairly certain it's not useful today.
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  hw/vga.c |   27 ++++-----------------------
>  1 files changed, 4 insertions(+), 23 deletions(-)
>
> diff --git a/hw/vga.c b/hw/vga.c
> index d96f1be..385184a 100644
> --- a/hw/vga.c
> +++ b/hw/vga.c
> @@ -2227,33 +2227,14 @@ typedef struct PCIVGAState {
>      VGAState vga_state;
>  } PCIVGAState;
>  
> -static int s1, s2;
> -
> -static void mark_dirty(target_phys_addr_t start, target_phys_addr_t len)
> -{
> -    target_phys_addr_t end = start + len;
> -
> -    while (start < end) {
> -        cpu_physical_memory_set_dirty(cpu_get_physical_page_desc(start));
> -        start += TARGET_PAGE_SIZE;
> -    }
> -}
> -
>  void vga_dirty_log_start(VGAState *s)
>  {
>      if (kvm_enabled() && s->map_addr)
> -        if (!s1) {
> -            kvm_log_start(s->map_addr, s->map_end - s->map_addr);
> -            mark_dirty(s->map_addr, s->map_end - s->map_addr);
> -            s1 = 1;
> -        }
> +        kvm_log_start(s->map_addr, s->map_end - s->map_addr);
> +
>      if (kvm_enabled() && s->lfb_vram_mapped) {
> -        if (!s2) {
> -            kvm_log_start(isa_mem_base + 0xa0000, 0x8000);
> -            kvm_log_start(isa_mem_base + 0xa8000, 0x8000);
> -            mark_dirty(isa_mem_base + 0xa0000, 0x10000);
> -        }
> -        s2 = 1;
> +        kvm_log_start(isa_mem_base + 0xa0000, 0x8000);
> +        kvm_log_start(isa_mem_base + 0xa8000, 0x8000);
>      }
>  }
>  
>   

This makes live migration and vga dirty tracking work together.  
Unfortunately since the last merge with qemu it's broken.

We have a shared resource, the log_dirty flag of memory slots.  We can't 
call log_start() and log_stop() from different users and expect things 
to work.

One cleaner way to fix this is to add a parameter containing the mask 
which will be used by the client to access the qemu bytemap.  
log_start() can OR this parameter with its own copy, and log_stop() can 
AND NOT the same thing.  When the local copy is nonzero, the slot dirty 
log is enabled.
diff mbox

Patch

diff --git a/hw/vga.c b/hw/vga.c
index d96f1be..385184a 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -2227,33 +2227,14 @@  typedef struct PCIVGAState {
     VGAState vga_state;
 } PCIVGAState;
 
-static int s1, s2;
-
-static void mark_dirty(target_phys_addr_t start, target_phys_addr_t len)
-{
-    target_phys_addr_t end = start + len;
-
-    while (start < end) {
-        cpu_physical_memory_set_dirty(cpu_get_physical_page_desc(start));
-        start += TARGET_PAGE_SIZE;
-    }
-}
-
 void vga_dirty_log_start(VGAState *s)
 {
     if (kvm_enabled() && s->map_addr)
-        if (!s1) {
-            kvm_log_start(s->map_addr, s->map_end - s->map_addr);
-            mark_dirty(s->map_addr, s->map_end - s->map_addr);
-            s1 = 1;
-        }
+        kvm_log_start(s->map_addr, s->map_end - s->map_addr);
+
     if (kvm_enabled() && s->lfb_vram_mapped) {
-        if (!s2) {
-            kvm_log_start(isa_mem_base + 0xa0000, 0x8000);
-            kvm_log_start(isa_mem_base + 0xa8000, 0x8000);
-            mark_dirty(isa_mem_base + 0xa0000, 0x10000);
-        }
-        s2 = 1;
+        kvm_log_start(isa_mem_base + 0xa0000, 0x8000);
+        kvm_log_start(isa_mem_base + 0xa8000, 0x8000);
     }
 }