[RESEND][PATCH] qemu-kvm: Drop vga dirty logging workarounds

[RESEND][PATCH] qemu-kvm: Drop vga dirty logging workarounds

[Jan Kiszka]

These diffs to upstream should all date back to the days qemu-kvm
supported vga dirty logging with restricted/broken kvm kernel modules.
We no longer do, so there is no need for those workarounds. Even worse
they can trigger internal bug checks these days:

BUG: kvm_dirty_pages_log_change: invalid parameters 00000000000a8000-00000000000affff

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

When the log_start/stop callbacks are upstream and about to be merged
back, you likely want this patch to be applied first. :)

 hw/cirrus_vga.c |   16 ----------------
 hw/vga-pci.c    |    2 --
 hw/vga.c        |   44 ++++++++------------------------------------
 3 files changed, 8 insertions(+), 54 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index b58dd7e..22529c9 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -32,7 +32,6 @@
 #include "console.h"
 #include "vga_int.h"
 #include "kvm.h"
-#include "qemu-kvm.h"
 #include "loader.h"
 
 /*
@@ -2488,7 +2487,6 @@ static CPUWriteMemoryFunc * const cirrus_linear_bitblt_write[3] = {
 
 static void map_linear_vram(CirrusVGAState *s)
 {
-    vga_dirty_log_stop(&s->vga);
     if (!s->vga.map_addr && s->vga.lfb_addr && s->vga.lfb_end) {
         s->vga.map_addr = s->vga.lfb_addr;
         s->vga.map_end = s->vga.lfb_end;
@@ -2501,16 +2499,11 @@ static void map_linear_vram(CirrusVGAState *s)
 #ifndef TARGET_IA64
     s->vga.lfb_vram_mapped = 0;
 
-    cpu_register_physical_memory(isa_mem_base + 0xa0000, 0x8000,
-                                (s->vga.vram_offset + s->cirrus_bank_base[0]) | IO_MEM_UNASSIGNED);
-    cpu_register_physical_memory(isa_mem_base + 0xa8000, 0x8000,
-                                (s->vga.vram_offset + s->cirrus_bank_base[1]) | IO_MEM_UNASSIGNED);
     if (!(s->cirrus_srcptr != s->cirrus_srcptr_end)
         && !((s->vga.sr[0x07] & 0x01) == 0)
         && !((s->vga.gr[0x0B] & 0x14) == 0x14)
         && !(s->vga.gr[0x0B] & 0x02)) {
 
-        vga_dirty_log_stop(&s->vga);
         cpu_register_physical_memory(isa_mem_base + 0xa0000, 0x8000,
                                     (s->vga.vram_offset + s->cirrus_bank_base[0]) | IO_MEM_RAM);
         cpu_register_physical_memory(isa_mem_base + 0xa8000, 0x8000,
@@ -2529,7 +2522,6 @@ static void map_linear_vram(CirrusVGAState *s)
 
 static void unmap_linear_vram(CirrusVGAState *s)
 {
-    vga_dirty_log_stop(&s->vga);
     if (s->vga.map_addr && s->vga.lfb_addr && s->vga.lfb_end) {
         s->vga.map_addr = s->vga.map_end = 0;
          cpu_register_physical_memory(s->vga.lfb_addr, s->vga.vram_size,
@@ -2537,8 +2529,6 @@ static void unmap_linear_vram(CirrusVGAState *s)
     }
     cpu_register_physical_memory(isa_mem_base + 0xa0000, 0x20000,
                                  s->vga.vga_io_memory);
-
-    vga_dirty_log_start(&s->vga);
 }
 
 /* Compute the memory access functions */
@@ -3073,8 +3063,6 @@ static void cirrus_pci_lfb_map(PCIDevice *d, int region_num,
 {
     CirrusVGAState *s = &DO_UPCAST(PCICirrusVGAState, dev, d)->cirrus_vga;
 
-    vga_dirty_log_stop(&s->vga);
-
     /* XXX: add byte swapping apertures */
     cpu_register_physical_memory(addr, s->vga.vram_size,
 				 s->cirrus_linear_io_addr);
@@ -3106,14 +3094,10 @@ static void pci_cirrus_write_config(PCIDevice *d,
     PCICirrusVGAState *pvs = DO_UPCAST(PCICirrusVGAState, dev, d);
     CirrusVGAState *s = &pvs->cirrus_vga;
 
-    vga_dirty_log_stop(&s->vga);
-
     pci_default_write_config(d, address, val, len);
     if (s->vga.map_addr && d->io_regions[0].addr == PCI_BAR_UNMAPPED)
         s->vga.map_addr = 0;
     cirrus_update_memory_access(s);
-
-    vga_dirty_log_start(&s->vga);
 }
 
 static int pci_cirrus_vga_initfn(PCIDevice *dev)
diff --git a/hw/vga-pci.c b/hw/vga-pci.c
index 1ef2d96..ce9ec45 100644
--- a/hw/vga-pci.c
+++ b/hw/vga-pci.c
@@ -65,11 +65,9 @@ static void pci_vga_write_config(PCIDevice *d,
     PCIVGAState *pvs = container_of(d, PCIVGAState, dev);
     VGACommonState *s = &pvs->vga;
 
-    vga_dirty_log_stop(s);
     pci_default_write_config(d, address, val, len);
     if (s->map_addr && pvs->dev.io_regions[0].addr == -1)
         s->map_addr = 0;
-    vga_dirty_log_start(s);
 }
 
 static int pci_vga_initfn(PCIDevice *dev)
diff --git a/hw/vga.c b/hw/vga.c
index 3d15c15..ab339a9 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -1258,8 +1258,6 @@ static void vga_draw_text(VGACommonState *s, int full_update)
     vga_draw_glyph8_func *vga_draw_glyph8;
     vga_draw_glyph9_func *vga_draw_glyph9;
 
-    vga_dirty_log_stop(s);
-
     /* compute font data address (in plane 2) */
     v = s->sr[3];
     offset = (((v >> 4) & 1) | ((v << 1) & 6)) * 8192 * 4 + 2;
@@ -1571,65 +1569,40 @@ static void vga_sync_dirty_bitmap(VGACommonState *s)
     }
 #endif
 
-    vga_dirty_log_start(s);
-}
-
-static int s1, s2, s3;
-
-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(VGACommonState *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);
     }
 
 #ifdef CONFIG_BOCHS_VBE
     if (kvm_enabled() && s->vbe_mapped) {
-        if (!s3) {
-            kvm_log_start(VBE_DISPI_LFB_PHYSICAL_ADDRESS, s->vram_size);
-        }
-        s3 = 1;
+        kvm_log_start(VBE_DISPI_LFB_PHYSICAL_ADDRESS, s->vram_size);
     }
 #endif
 }
 
 void vga_dirty_log_stop(VGACommonState *s)
 {
-    if (kvm_enabled() && s->map_addr && s1)
+    if (kvm_enabled() && s->map_addr)
 	kvm_log_stop(s->map_addr, s->map_end - s->map_addr);
 
-    if (kvm_enabled() && s->lfb_vram_mapped && s1) {
+    if (kvm_enabled() && s->lfb_vram_mapped) {
 	kvm_log_stop(isa_mem_base + 0xa0000, 0x8000);
 	kvm_log_stop(isa_mem_base + 0xa8000, 0x8000);
     }
 
 #ifdef CONFIG_BOCHS_VBE
-    if (kvm_enabled() && s->vbe_mapped && s3) {
+    if (kvm_enabled() && s->vbe_mapped) {
 	kvm_log_stop(VBE_DISPI_LFB_PHYSICAL_ADDRESS, s->vram_size);
     }
 #endif
-
-    s1 = s2 = s3 = 0;
 }
 
 void vga_dirty_log_restart(VGACommonState *s)
@@ -1867,7 +1840,6 @@ static void vga_draw_blank(VGACommonState *s, int full_update)
         return;
     if (s->last_scr_width <= 0 || s->last_scr_height <= 0)
         return;
-    vga_dirty_log_stop(s);
 
     s->rgb_to_pixel =
         rgb_to_pixel_dup_table[get_depth_index(s->ds)];
-- 
1.7.1

Re: [RESEND][PATCH] qemu-kvm: Drop vga dirty logging workarounds

[Marcelo Tosatti]

On Mon, Feb 14, 2011 at 07:19:20PM +0100, Jan Kiszka wrote:
> These diffs to upstream should all date back to the days qemu-kvm
> supported vga dirty logging with restricted/broken kvm kernel modules.
> We no longer do, so there is no need for those workarounds. Even worse
> they can trigger internal bug checks these days:
> 
> BUG: kvm_dirty_pages_log_change: invalid parameters 00000000000a8000-00000000000affff
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> 
> When the log_start/stop callbacks are upstream and about to be merged
> back, you likely want this patch to be applied first. :)

Applied, thanks.