qemu-kvm: Drop vga dirty logging workarounds

RFC PATCH Superseded Jan Kiszka 2 participants 4 messages Nov 15, 2010 KVM

Share & tools

Jan Kiszka

author

2010-11-15 10:33 UTC #0 lore raw

RFC PATCH 1/1 qemu-kvm: Drop vga dirty logging workarounds

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 <[email protected]>

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

hw/cirrus_vga.c +0 ~~-16~~

index a580b57..35b8b0e 100644
@@ -32,7 +32,6 @@
 #include "console.h"
 #include "vga_int.h"
 #include "kvm.h"
-#include "qemu-kvm.h"
 #include "loader.h"
 
 /*

@@ -2553,7 +2552,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;

@@ -2566,16 +2564,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,

@@ -2594,7 +2587,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,

@@ -2602,8 +2594,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 */

@@ -3156,8 +3146,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);

@@ -3189,14 +3177,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)

hw/vga-pci.c +0 -2

index 3907871..2315f70 100644
@@ -68,11 +68,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)

hw/vga.c +8 ~~-36~~

88 lines not on this page · raw message

Signature · 1 line

1.7.1

Avi Kivity

2010-11-15 13:16 UTC #1 lore raw

Quoted 6 lines · 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

I'd like to apply this. What kind of testing has this seen? autotest likely isn't a good enough test.

Signature · 1 line

error compiling committee.c: too many arguments to function

Jan Kiszka

author

2010-11-15 13:26 UTC #2 lore raw

Quoted 10 lines · Avi Kivity

Quoted 6 lines · 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
I'd like to apply this. What kind of testing has this seen? autotest likely isn't a good enough test.

No systematic testing.

It's based on the fact that I'm not aware of any VGA issues in upstream when KVM is enabled, on the fact that explicit log enable/disable became obsolete when switching to upstream's logging backend, and the annoying BUG messages only issued under qemu-kvm. There just remains a slight uncertainty about what mark_dirty and s[123] were once doing - or may still be useful for.

Jan

Signature · 2 lines

Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Avi Kivity

2010-11-15 13:29 UTC #3 lore raw

Quoted 9 lines · Jan Kiszka

Quoted 8 lines · Avi Kivity
Quoted 6 lines · 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

Quoted 7 lines

Quoted 2 lines
I'd like to apply this. What kind of testing has this seen? autotest likely isn't a good enough test.
No systematic testing.
It's based on the fact that I'm not aware of any VGA issues in upstream when KVM is enabled,

I don't think upstream+kvm sees a lot of testing.

Quoted 5 lines

on the fact that explicit log enable/disable became
obsolete when switching to upstream's logging backend, and the annoying BUG messages only issued under qemu-kvm. There just remains a slight uncertainty about what mark_dirty and s[123] were once doing - or may still be useful for.

I remember re-adding some kvm-specific dirty logging code after regressions were uncovered, so I'm a little worried. I guess we can risk it, we can always revert it (or fix) if it turns out badly.

Signature · 1 line

error compiling committee.c: too many arguments to function