[Qemu-devel] [PATCH 0/4] VGA optimization

[Qemu-devel] [PATCH 0/4] VGA optimization

[Glauber Costa]

You know the deal: This is the vga optimization all over again!

Some patches were already commited, this version only carries 4 of them.
The first two are just cleanups to make it fit.

The third is for kvm, and the fourth, for general qemu.

Note that unfortunately, it won't work for everybody: kvm host
kernel has a bug that prevents it, so whoever wants to test it,
will need a patched kernel (until the patch gets upstream kvm
and you upgrade your kernel).

[Qemu-devel] [PATCH 1/4] better type checking for vga

[Glauber Costa]

unsigned long is too bad of a type. Use ram_addr_t instead.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 hw/cirrus_vga.c |    4 ++--
 hw/vga.c        |    2 +-
 hw/vga_int.h    |    4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index af9c9e6..3cdc8e6 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -3231,7 +3231,7 @@ static void cirrus_init_common(CirrusVGAState * s, int device_id, int is_pci)
  ***************************************/
 
 void isa_cirrus_vga_init(DisplayState *ds, uint8_t *vga_ram_base,
-                         unsigned long vga_ram_offset, int vga_ram_size)
+                         ram_addr_t vga_ram_offset, int vga_ram_size)
 {
     CirrusVGAState *s;
 
@@ -3273,7 +3273,7 @@ static void cirrus_pci_mmio_map(PCIDevice *d, int region_num,
 }
 
 void pci_cirrus_vga_init(PCIBus *bus, DisplayState *ds, uint8_t *vga_ram_base,
-                         unsigned long vga_ram_offset, int vga_ram_size)
+                         ram_addr_t vga_ram_offset, int vga_ram_size)
 {
     PCICirrusVGAState *d;
     uint8_t *pci_conf;
diff --git a/hw/vga.c b/hw/vga.c
index bd59aae..b44b77d 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -2105,7 +2105,7 @@ static void vga_map(PCIDevice *pci_dev, int region_num,
 }
 
 void vga_common_init(VGAState *s, DisplayState *ds, uint8_t *vga_ram_base,
-                     unsigned long vga_ram_offset, int vga_ram_size)
+                     ram_addr_t vga_ram_offset, int vga_ram_size)
 {
     int i, j, v, b;
 
diff --git a/hw/vga_int.h b/hw/vga_int.h
index 82a755e..8a7a925 100644
--- a/hw/vga_int.h
+++ b/hw/vga_int.h
@@ -100,7 +100,7 @@ typedef void (* vga_update_retrace_info_fn)(struct VGAState *s);
 
 #define VGA_STATE_COMMON                                                \
     uint8_t *vram_ptr;                                                  \
-    unsigned long vram_offset;                                          \
+    ram_addr_t vram_offset;                                             \
     unsigned int vram_size;                                             \
     unsigned long bios_offset;                                          \
     unsigned int bios_size;                                             \
@@ -186,7 +186,7 @@ static inline int c6_to_8(int v)
 }
 
 void vga_common_init(VGAState *s, DisplayState *ds, uint8_t *vga_ram_base,
-                     unsigned long vga_ram_offset, int vga_ram_size);
+                     ram_addr_t vga_ram_offset, int vga_ram_size);
 void vga_init(VGAState *s);
 uint32_t vga_mem_readb(void *opaque, target_phys_addr_t addr);
 void vga_mem_writeb(void *opaque, target_phys_addr_t addr, uint32_t val);
-- 
1.5.6.5

[Qemu-devel] [PATCH 2/4] move vga_io_address to VGA State

[Glauber Costa]

It'll be reused later by the vga optimization.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 hw/cirrus_vga.c |    6 +++---
 hw/vga_int.h    |    1 +
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index 3cdc8e6..85359a8 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -3117,7 +3117,7 @@ static int cirrus_vga_load(QEMUFile *f, void *opaque, int version_id)
 
 static void cirrus_init_common(CirrusVGAState * s, int device_id, int is_pci)
 {
-    int vga_io_memory, i;
+    int i;
     static int inited;
 
     if (!inited) {
@@ -3156,10 +3156,10 @@ static void cirrus_init_common(CirrusVGAState * s, int device_id, int is_pci)
     register_ioport_read(0x3ba, 1, 1, vga_ioport_read, s);
     register_ioport_read(0x3da, 1, 1, vga_ioport_read, s);
 
-    vga_io_memory = cpu_register_io_memory(0, cirrus_vga_mem_read,
+    s->vga_io_memory = cpu_register_io_memory(0, cirrus_vga_mem_read,
                                            cirrus_vga_mem_write, s);
     cpu_register_physical_memory(isa_mem_base + 0x000a0000, 0x20000,
-                                 vga_io_memory);
+                                 s->vga_io_memory);
 
     s->sr[0x06] = 0x0f;
     if (device_id == CIRRUS_ID_CLGD5446) {
diff --git a/hw/vga_int.h b/hw/vga_int.h
index 8a7a925..4fa619a 100644
--- a/hw/vga_int.h
+++ b/hw/vga_int.h
@@ -129,6 +129,7 @@ typedef void (* vga_update_retrace_info_fn)(struct VGAState *s);
     int dac_8bit;                                                       \
     uint8_t palette[768];                                               \
     int32_t bank_offset;                                                \
+    int vga_io_memory;                                             \
     int (*get_bpp)(struct VGAState *s);                                 \
     void (*get_offsets)(struct VGAState *s,                             \
                         uint32_t *pline_offset,                         \
-- 
1.5.6.5

[Qemu-devel] [PATCH 3/4] kvm: Introduce kvm logging interface.

[Glauber Costa]

Introduce functions to control logging of memory regions.
We select regions based on its start address, a
guest_physical_addr (target_phys_addr_t, in qemu nomenclature).

The main user of this interface right now is VGA optimization
(a way of reducing the number of mmio exits).

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 kvm-all.c |  122 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 kvm.h     |    4 ++
 2 files changed, 113 insertions(+), 13 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index e44f296..39f826b 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -2,9 +2,11 @@
  * QEMU KVM support
  *
  * Copyright IBM, Corp. 2008
+ *           Red Hat, Inc. 2008
  *
  * Authors:
  *  Anthony Liguori   <aliguori@us.ibm.com>
+ *  Glauber Costa     <gcosta@redhat.com>
  *
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
@@ -41,6 +43,8 @@ typedef struct KVMSlot
     int flags;
 } KVMSlot;
 
+typedef struct kvm_dirty_log KVMDirtyLog;
+
 int kvm_allowed = 0;
 
 struct KVMState
@@ -82,6 +86,20 @@ static KVMSlot *kvm_lookup_slot(KVMState *s, target_phys_addr_t start_addr)
     return NULL;
 }
 
+static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
+{
+    struct kvm_userspace_memory_region mem;
+
+    mem.slot = slot->slot;
+    mem.guest_phys_addr = slot->start_addr;
+    mem.memory_size = slot->memory_size;
+    mem.userspace_addr = (unsigned long)phys_ram_base + slot->phys_offset;
+    mem.flags = slot->flags;
+
+    return kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem);
+}
+
+
 int kvm_init_vcpu(CPUState *env)
 {
     KVMState *s = kvm_state;
@@ -119,6 +137,97 @@ err:
     return ret;
 }
 
+/*
+ * dirty pages logging control
+ */
+static int kvm_dirty_pages_log_change(target_phys_addr_t phys_addr, target_phys_addr_t end_addr,
+                                      unsigned flags,
+                                      unsigned mask)
+{
+    KVMState *s = kvm_state;
+    KVMSlot *mem = kvm_lookup_slot(s, phys_addr);
+    if (mem == NULL)  {
+            dprintf("invalid parameters %llx-%llx\n", phys_addr, end_addr);
+            return -EINVAL;
+    }
+
+    flags = (mem->flags & ~mask) | flags;
+    /* Nothing changed, no need to issue ioctl */
+    if (flags == mem->flags)
+            return 0;
+
+    mem->flags = flags;
+
+    return kvm_set_user_memory_region(s, mem);
+}
+
+int kvm_log_start(target_phys_addr_t phys_addr, target_phys_addr_t end_addr)
+{
+        return kvm_dirty_pages_log_change(phys_addr, end_addr,
+                                          KVM_MEM_LOG_DIRTY_PAGES,
+                                          KVM_MEM_LOG_DIRTY_PAGES);
+}
+
+int kvm_log_stop(target_phys_addr_t phys_addr, target_phys_addr_t end_addr)
+{
+        return kvm_dirty_pages_log_change(phys_addr, end_addr,
+                                          0,
+                                          KVM_MEM_LOG_DIRTY_PAGES);
+}
+
+/**
+ * kvm_physical_sync_dirty_bitmap - Grab dirty bitmap from kernel space
+ * This function updates qemu's dirty bitmap using cpu_physical_memory_set_dirty().
+ * This means all bits are set to dirty.
+ *
+ * @start_add: start of logged region. This is what we use to search the memslot
+ * @end_addr: end of logged region.
+ */
+void kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr, target_phys_addr_t end_addr)
+{
+    KVMState *s = kvm_state;
+    KVMDirtyLog d;
+    KVMSlot *mem = kvm_lookup_slot(s, start_addr);
+    unsigned long alloc_size;
+    ram_addr_t addr;
+    target_phys_addr_t phys_addr = start_addr;
+
+    dprintf("sync addr: %llx into %lx\n", start_addr, mem->phys_offset);
+    if (mem == NULL) {
+            fprintf(stderr, "BUG: %s: invalid parameters\n", __func__);
+            return;
+    }
+
+    alloc_size = mem->memory_size >> TARGET_PAGE_BITS / sizeof(d.dirty_bitmap);
+    d.dirty_bitmap = qemu_mallocz(alloc_size);
+
+    if (d.dirty_bitmap == NULL) {
+        dprintf("Could not allocate dirty bitmap\n");
+        return;
+    }
+
+    d.slot = mem->slot;
+    dprintf("slot %d, phys_addr %llx, uaddr: %llx\n",
+            d.slot, mem->start_addr, mem->phys_offset);
+
+    if (kvm_vm_ioctl(s, KVM_GET_DIRTY_LOG, &d) == -1) {
+        dprintf("ioctl failed %d\n", errno);
+        goto out;
+    }
+
+    phys_addr = start_addr;
+    for (addr = mem->phys_offset; phys_addr < end_addr; phys_addr+= TARGET_PAGE_SIZE, addr += TARGET_PAGE_SIZE) {
+        unsigned long *bitmap = (unsigned long *)d.dirty_bitmap;
+        unsigned nr = (phys_addr - start_addr) >> TARGET_PAGE_BITS;
+        unsigned word = nr / (sizeof(*bitmap) * 8);
+        unsigned bit = nr % (sizeof(*bitmap) * 8);
+        if ((bitmap[word] >> bit) & 1)
+            cpu_physical_memory_set_dirty(addr);
+    }
+out:
+    qemu_free(d.dirty_bitmap);
+}
+
 int kvm_init(int smp_cpus)
 {
     KVMState *s;
@@ -316,19 +425,6 @@ int kvm_cpu_exec(CPUState *env)
     return ret;
 }
 
-static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
-{
-    struct kvm_userspace_memory_region mem;
-
-    mem.slot = slot->slot;
-    mem.guest_phys_addr = slot->start_addr;
-    mem.memory_size = slot->memory_size;
-    mem.userspace_addr = (unsigned long)phys_ram_base + slot->phys_offset;
-    mem.flags = slot->flags;
-
-    return kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem);
-}
-
 void kvm_set_phys_mem(target_phys_addr_t start_addr,
                       ram_addr_t size,
                       ram_addr_t phys_offset)
diff --git a/kvm.h b/kvm.h
index 304de27..fb952a3 100644
--- a/kvm.h
+++ b/kvm.h
@@ -38,6 +38,10 @@ void kvm_set_phys_mem(target_phys_addr_t start_addr,
                       ram_addr_t size,
                       ram_addr_t phys_offset);
 
+void kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr, target_phys_addr_t end_addr);
+
+int kvm_log_start(target_phys_addr_t phys_addr, target_phys_addr_t len);
+int kvm_log_stop(target_phys_addr_t phys_addr, target_phys_addr_t len);
 /* internal API */
 
 struct KVMState;
-- 
1.5.6.5

[Qemu-devel] [PATCH 4/4] vga optimization.

[Glauber Costa]

Hypervisors like KVM perform badly while doing mmio on
a loop, because it'll generate an exit on each access.
This is the case with VGA, which results in very bad
performance.

In this patch, we map the linear frame buffer as RAM,
make sure it has dirty region tracking enabled, and then
just let the region to be written.

Cleanups suggestions by:
  Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 cpu-all.h       |    2 +
 exec.c          |    6 +++++
 hw/cirrus_vga.c |   65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/vga.c        |   47 +++++++++++++++++++++++++++++++++++++++
 hw/vga_int.h    |    9 +++++++
 5 files changed, 129 insertions(+), 0 deletions(-)

diff --git a/cpu-all.h b/cpu-all.h
index aa4b770..5fa1e69 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -948,6 +948,8 @@ int cpu_physical_memory_set_dirty_tracking(int enable);
 
 int cpu_physical_memory_get_dirty_tracking(void);
 
+void cpu_physical_sync_dirty_bitmap(target_phys_addr_t start_addr, target_phys_addr_t end_addr);
+
 void dump_exec_info(FILE *f,
                     int (*cpu_fprintf)(FILE *f, const char *fmt, ...));
 
diff --git a/exec.c b/exec.c
index 4d39eaa..43a1b78 100644
--- a/exec.c
+++ b/exec.c
@@ -1887,6 +1887,12 @@ int cpu_physical_memory_get_dirty_tracking(void)
     return in_migration;
 }
 
+void cpu_physical_sync_dirty_bitmap(target_phys_addr_t start_addr, target_phys_addr_t end_addr)
+{
+    if (kvm_enabled())
+        kvm_physical_sync_dirty_bitmap(start_addr, end_addr);
+}
+
 static inline void tlb_update_dirty(CPUTLBEntry *tlb_entry)
 {
     ram_addr_t ram_addr;
diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index 85359a8..4cb16a9 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -31,6 +31,7 @@
 #include "pci.h"
 #include "console.h"
 #include "vga_int.h"
+#include "kvm.h"
 
 /*
  * TODO:
@@ -1228,6 +1229,12 @@ static void cirrus_update_bank_ptr(CirrusVGAState * s, unsigned bank_index)
     }
 
     if (limit > 0) {
+        /* Thinking about changing bank base? First, drop the dirty bitmap information
+         * on the current location, otherwise we lose this pointer forever */
+        if (s->lfb_vram_mapped) {
+            target_phys_addr_t base_addr = isa_mem_base + 0xa0000 + bank_index * 0x8000;
+            cpu_physical_sync_dirty_bitmap(base_addr, base_addr + 0x8000);
+        }
 	s->cirrus_bank_base[bank_index] = offset;
 	s->cirrus_bank_limit[bank_index] = limit;
     } else {
@@ -1356,6 +1363,7 @@ cirrus_hook_write_sr(CirrusVGAState * s, unsigned reg_index, int reg_value)
 	s->hw_cursor_y = (reg_value << 3) | (reg_index >> 5);
 	break;
     case 0x07:			// Extended Sequencer Mode
+    cirrus_update_memory_access(s);
     case 0x08:			// EEPROM Control
     case 0x09:			// Scratch Register 0
     case 0x0a:			// Scratch Register 1
@@ -1528,6 +1536,7 @@ cirrus_hook_write_gr(CirrusVGAState * s, unsigned reg_index, int reg_value)
 	s->gr[reg_index] = reg_value;
 	cirrus_update_bank_ptr(s, 0);
 	cirrus_update_bank_ptr(s, 1);
+        cirrus_update_memory_access(s);
         break;
     case 0x0B:
 	s->gr[reg_index] = reg_value;
@@ -2618,6 +2627,52 @@ static CPUWriteMemoryFunc *cirrus_linear_bitblt_write[3] = {
     cirrus_linear_bitblt_writel,
 };
 
+static void map_linear_vram(CirrusVGAState *s)
+{
+
+    if (!s->map_addr && s->lfb_addr && s->lfb_end) {
+        s->map_addr = s->lfb_addr;
+        s->map_end = s->lfb_end;
+        cpu_register_physical_memory(s->map_addr, s->map_end - s->map_addr, s->vram_offset);
+        vga_dirty_log_start((VGAState *)s);
+    }
+
+    if (!s->map_addr)
+        return;
+
+    s->lfb_vram_mapped = 0;
+
+    if (!(s->cirrus_srcptr != s->cirrus_srcptr_end)
+        && !((s->sr[0x07] & 0x01) == 0)
+        && !((s->gr[0x0B] & 0x14) == 0x14)
+        && !(s->gr[0x0B] & 0x02)) {
+
+        cpu_register_physical_memory(isa_mem_base + 0xa0000, 0x8000,
+                                    (s->vram_offset + s->cirrus_bank_base[0]) | IO_MEM_RAM);
+        cpu_register_physical_memory(isa_mem_base + 0xa8000, 0x8000,
+                                    (s->vram_offset + s->cirrus_bank_base[1]) | IO_MEM_RAM);
+
+        s->lfb_vram_mapped = 1;
+        vga_dirty_log_start((VGAState *)s);
+    }
+    else {
+        cpu_register_physical_memory(isa_mem_base + 0xa0000, 0x8000, s->vga_io_memory);
+        cpu_register_physical_memory(isa_mem_base + 0xa8000, 0x8000, s->vga_io_memory);
+    }
+
+}
+
+static void unmap_linear_vram(CirrusVGAState *s)
+{
+    if (s->map_addr && s->lfb_addr && s->lfb_end) {
+        vga_dirty_log_stop((VGAState *)s);
+        s->map_addr = s->map_end = 0;
+    }
+
+    cpu_register_physical_memory(isa_mem_base + 0xa0000, 0x20000,
+                                 s->vga_io_memory);
+}
+
 /* Compute the memory access functions */
 static void cirrus_update_memory_access(CirrusVGAState *s)
 {
@@ -2636,11 +2691,13 @@ static void cirrus_update_memory_access(CirrusVGAState *s)
 
 	mode = s->gr[0x05] & 0x7;
 	if (mode < 4 || mode > 5 || ((s->gr[0x0B] & 0x4) == 0)) {
+            map_linear_vram(s);
             s->cirrus_linear_write[0] = cirrus_linear_mem_writeb;
             s->cirrus_linear_write[1] = cirrus_linear_mem_writew;
             s->cirrus_linear_write[2] = cirrus_linear_mem_writel;
         } else {
         generic_io:
+            unmap_linear_vram(s);
             s->cirrus_linear_write[0] = cirrus_linear_writeb;
             s->cirrus_linear_write[1] = cirrus_linear_writew;
             s->cirrus_linear_write[2] = cirrus_linear_writel;
@@ -3102,6 +3159,7 @@ static int cirrus_vga_load(QEMUFile *f, void *opaque, int version_id)
     qemu_get_be32s(f, &s->hw_cursor_x);
     qemu_get_be32s(f, &s->hw_cursor_y);
 
+    cirrus_update_memory_access(s);
     /* force refresh */
     s->graphic_mode = -1;
     cirrus_update_bank_ptr(s, 0);
@@ -3261,6 +3319,13 @@ static void cirrus_pci_lfb_map(PCIDevice *d, int region_num,
 				 s->cirrus_linear_io_addr);
     cpu_register_physical_memory(addr + 0x1000000, 0x400000,
 				 s->cirrus_linear_bitblt_io_addr);
+
+    s->map_addr = s->map_end = 0;
+    s->lfb_addr = addr & TARGET_PAGE_MASK;
+    s->lfb_end = ((addr + VGA_RAM_SIZE) + TARGET_PAGE_SIZE - 1) & TARGET_PAGE_MASK;
+    /* account for overflow */
+    if (s->lfb_end < addr + VGA_RAM_SIZE)
+        s->lfb_end = addr + VGA_RAM_SIZE;
 }
 
 static void cirrus_pci_mmio_map(PCIDevice *d, int region_num,
diff --git a/hw/vga.c b/hw/vga.c
index b44b77d..dd78650 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -28,6 +28,7 @@
 #include "vga_int.h"
 #include "pixel_ops.h"
 #include "qemu-timer.h"
+#include "kvm.h"
 
 //#define DEBUG_VGA
 //#define DEBUG_VGA_MEM
@@ -1243,6 +1244,8 @@ static void vga_draw_text(VGAState *s, int full_update)
     vga_draw_glyph8_func *vga_draw_glyph8;
     vga_draw_glyph9_func *vga_draw_glyph9;
 
+    vga_dirty_log_stop(s);
+
     full_update |= update_palette16(s);
     palette = s->last_palette;
 
@@ -1556,6 +1559,18 @@ void vga_invalidate_scanlines(VGAState *s, int y1, int y2)
     }
 }
 
+static void vga_sync_dirty_bitmap(VGAState *s)
+{
+    if (s->map_addr)
+        cpu_physical_sync_dirty_bitmap(s->map_addr, s->map_end);
+
+    if (s->lfb_vram_mapped) {
+        cpu_physical_sync_dirty_bitmap(isa_mem_base + 0xa0000, 0xa8000);
+        cpu_physical_sync_dirty_bitmap(isa_mem_base + 0xa8000, 0xb0000);
+    }
+    vga_dirty_log_start(s);
+}
+
 /*
  * graphic modes
  */
@@ -1570,6 +1585,9 @@ static void vga_draw_graphic(VGAState *s, int full_update)
 
     full_update |= update_basic_params(s);
 
+    if (!full_update)
+        vga_sync_dirty_bitmap(s);
+
     s->get_resolution(s, &width, &height);
     disp_width = width;
 
@@ -1743,6 +1761,8 @@ static void vga_draw_blank(VGAState *s, int full_update)
         return;
     if (s->last_scr_width <= 0 || s->last_scr_height <= 0)
         return;
+    vga_dirty_log_stop(s);
+
     if (s->ds->depth == 8)
         val = s->rgb_to_pixel(0, 0, 0);
     else
@@ -2092,6 +2112,28 @@ typedef struct PCIVGAState {
     VGAState vga_state;
 } PCIVGAState;
 
+void vga_dirty_log_start(VGAState *s)
+{
+    if (kvm_enabled() && s->map_addr)
+        kvm_log_start(s->map_addr, s->map_end - s->map_addr);
+
+    if (kvm_enabled() && s->lfb_vram_mapped) {
+        kvm_log_start(isa_mem_base + 0xa0000, 0x8000);
+        kvm_log_start(isa_mem_base + 0xa8000, 0x8000);
+    }
+}
+
+void vga_dirty_log_stop(VGAState *s)
+{
+    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) {
+        kvm_log_stop(isa_mem_base + 0xa0000, 0x8000);
+        kvm_log_stop(isa_mem_base + 0xa8000, 0x8000);
+    }
+}
+
 static void vga_map(PCIDevice *pci_dev, int region_num,
                     uint32_t addr, uint32_t size, int type)
 {
@@ -2102,6 +2144,11 @@ static void vga_map(PCIDevice *pci_dev, int region_num,
     } else {
         cpu_register_physical_memory(addr, s->vram_size, s->vram_offset);
     }
+
+    s->map_addr = addr;
+    s->map_end = addr + VGA_RAM_SIZE;
+
+    vga_dirty_log_start(s);
 }
 
 void vga_common_init(VGAState *s, DisplayState *ds, uint8_t *vga_ram_base,
diff --git a/hw/vga_int.h b/hw/vga_int.h
index 4fa619a..b360d3e 100644
--- a/hw/vga_int.h
+++ b/hw/vga_int.h
@@ -102,6 +102,11 @@ typedef void (* vga_update_retrace_info_fn)(struct VGAState *s);
     uint8_t *vram_ptr;                                                  \
     ram_addr_t vram_offset;                                             \
     unsigned int vram_size;                                             \
+    uint32_t lfb_addr;                                                  \
+    uint32_t lfb_end;                                                   \
+    uint32_t map_addr;                                                  \
+    uint32_t map_end;                                                   \
+    uint32_t lfb_vram_mapped; /* whether 0xa0000 is mapped as ram */    \
     unsigned long bios_offset;                                          \
     unsigned int bios_size;                                             \
     target_phys_addr_t base_ctrl;                                       \
@@ -189,6 +194,10 @@ static inline int c6_to_8(int v)
 void vga_common_init(VGAState *s, DisplayState *ds, uint8_t *vga_ram_base,
                      ram_addr_t vga_ram_offset, int vga_ram_size);
 void vga_init(VGAState *s);
+
+void vga_dirty_log_start(VGAState *s);
+void vga_dirty_log_stop(VGAState *s);
+
 uint32_t vga_mem_readb(void *opaque, target_phys_addr_t addr);
 void vga_mem_writeb(void *opaque, target_phys_addr_t addr, uint32_t val);
 void vga_invalidate_scanlines(VGAState *s, int y1, int y2);
-- 
1.5.6.5

Re: [Qemu-devel] [PATCH 0/4] VGA optimization

[Anthony Liguori]

Glauber Costa wrote:
> You know the deal: This is the vga optimization all over again!
>
> Some patches were already commited, this version only carries 4 of them.
> The first two are just cleanups to make it fit.
>
> The third is for kvm, and the fourth, for general qemu.
>
> Note that unfortunately, it won't work for everybody: kvm host
> kernel has a bug that prevents it, so whoever wants to test it,
> will need a patched kernel (until the patch gets upstream kvm
> and you upgrade your kernel).
>   

Applied all.  Thanks.

Regards,

Anthony Liguori

>
>
>
>   

[Qemu-devel] [PATCH 0/4] VGA optimization

[Glauber Costa]

hey guys,

I hope this is the last version (Of course, once this is merged,
the optimizations of the optimization can start ;-) )

I split it in 4 patches. The first two ones are just moving
things out of the way, and then #3 and #4 do the real thing.
#3 kvm-side, #4 overall qemu.

They merge most of the suggestion Anthony and Stefano's sent
on last iteration.

Hope you like it.
 

Re: [Qemu-devel] [PATCH 0/4] VGA optimization

[Glauber Costa]

On Mon, Nov 10, 2008 at 4:37 PM, Glauber Costa <glommer@redhat.com> wrote:
> hey guys,
>
> I hope this is the last version (Of course, once this is merged,
> the optimizations of the optimization can start ;-) )
>
> I split it in 4 patches. The first two ones are just moving
> things out of the way, and then #3 and #4 do the real thing.
> #3 kvm-side, #4 overall qemu.
>
> They merge most of the suggestion Anthony and Stefano's sent
> on last iteration.
>
> Hope you like it.
I'm so smart, that I forgot the -n option to git _again_

move vga_io_address to VGA State is first
de-register mem region for MMIO is second
Introduce KVM logging interface, third
and vga optimization is fourth

sorry
>
>
>
>
>



-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

Re: [Qemu-devel] [PATCH 0/4] VGA optimization

[Anthony Liguori]

Glauber Costa wrote:
> hey guys,
>   

I gave you some bad advice that I think is causing the breakage I'm 
seeing now.  I suggested that you simply do a lookup to find the slot 
given a target_phys_addr_t but that isn't correct.  Let me explain why.

ram_addr_t represents a guest physical address.  From a ram_addr_t you 
can get a target_phys_addr_t.  Sometimes these are the same but they 
aren't always.

You can have multiple ram_addr_t's pointing to the same 
target_phys_addr_t.  This is ram aliasing and it happens for a variety 
of reasons.  In general, it's pretty expensive to map a ram_addr_t to a 
target_phys_addr_t because, among other things, for a range of 
(ram_addr_t, size_t), you may have many (target_phys_addr_t, size) 
tuples that you have to deal with.

vga_common_init() takes a target_phys_addr_t (well, it really takes an 
unsigned long, but that's a bug).  It takes this as an optimization.  It 
avoids having to do the conversion and ensures that it's one big linear 
region.

For dirty tracking, we have a bitmap indexed by target_phys_addr_t in 
QEMU.  This means that we can happily set dirty bits based on 
target_phys_addr_t's.  We don't have to worry about what ram_addr_t it 
came from because they all map to the same bits.

Since KVM uses a slot API, and that API is indexed in ram_addr_t's, we 
need to enable dirty tracking on the ram_addr_t's.  We don't have a 
ram_addr_t in the VGA code.

The solution is pretty simple.  We need to keep track of the 
ram_addr_t's in the VGA code and enable dirty tracking on the 
appropriate ram_addr_ts.

Regards,

Anthony Liguori

> I hope this is the last version (Of course, once this is merged,
> the optimizations of the optimization can start ;-) )
>
> I split it in 4 patches. The first two ones are just moving
> things out of the way, and then #3 and #4 do the real thing.
> #3 kvm-side, #4 overall qemu.
>
> They merge most of the suggestion Anthony and Stefano's sent
> on last iteration.
>
> Hope you like it.
>  
>
>
>
>   

Re: [Qemu-devel] [PATCH 0/4] VGA optimization

[Anthony Liguori]

Anthony Liguori wrote:
> Glauber Costa wrote:
>> hey guys,
>>   
>
> I gave you some bad advice that I think is causing the breakage I'm 
> seeing now.  I suggested that you simply do a lookup to find the slot 
> given a target_phys_addr_t but that isn't correct.  Let me explain why.

Except I'm completely backwards but at least consistently backwards :-)

Regards,

Anthony Liguori

> ram_addr_t represents a guest physical address.  From a ram_addr_t you 
> can get a target_phys_addr_t.  Sometimes these are the same but they 
> aren't always.
>
> You can have multiple ram_addr_t's pointing to the same 
> target_phys_addr_t.  This is ram aliasing and it happens for a variety 
> of reasons.  In general, it's pretty expensive to map a ram_addr_t to 
> a target_phys_addr_t because, among other things, for a range of 
> (ram_addr_t, size_t), you may have many (target_phys_addr_t, size) 
> tuples that you have to deal with.
>
> vga_common_init() takes a target_phys_addr_t (well, it really takes an 
> unsigned long, but that's a bug).  It takes this as an optimization.  
> It avoids having to do the conversion and ensures that it's one big 
> linear region.
>
> For dirty tracking, we have a bitmap indexed by target_phys_addr_t in 
> QEMU.  This means that we can happily set dirty bits based on 
> target_phys_addr_t's.  We don't have to worry about what ram_addr_t it 
> came from because they all map to the same bits.
>
> Since KVM uses a slot API, and that API is indexed in ram_addr_t's, we 
> need to enable dirty tracking on the ram_addr_t's.  We don't have a 
> ram_addr_t in the VGA code.
>
> The solution is pretty simple.  We need to keep track of the 
> ram_addr_t's in the VGA code and enable dirty tracking on the 
> appropriate ram_addr_ts.
>
> Regards,
>
> Anthony Liguori
>
>> I hope this is the last version (Of course, once this is merged,
>> the optimizations of the optimization can start ;-) )
>>
>> I split it in 4 patches. The first two ones are just moving
>> things out of the way, and then #3 and #4 do the real thing.
>> #3 kvm-side, #4 overall qemu.
>>
>> They merge most of the suggestion Anthony and Stefano's sent
>> on last iteration.
>>
>> Hope you like it.
>>  
>>
>>
>>
>>   
>

Re: [Qemu-devel] [PATCH 0/4] VGA optimization

[Glauber Costa]

On Mon, Nov 10, 2008 at 4:47 PM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> Anthony Liguori wrote:
>>
>> Glauber Costa wrote:
>>>
>>> hey guys,
>>>
>>
>> I gave you some bad advice that I think is causing the breakage I'm seeing
>> now.  I suggested that you simply do a lookup to find the slot given a
>> target_phys_addr_t but that isn't correct.  Let me explain why.
>
> Except I'm completely backwards but at least consistently backwards :-)

Which is amazing ;-) Your text made perfect sense after a sed line ;-)




-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

Re: [Qemu-devel] [PATCH 0/4] VGA optimization

[Glauber Costa]

On Mon, Nov 10, 2008 at 4:39 PM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> Glauber Costa wrote:
>>
>> hey guys,
>>
>
> I gave you some bad advice that I think is causing the breakage I'm seeing
> now.  I suggested that you simply do a lookup to find the slot given a
> target_phys_addr_t but that isn't correct.  Let me explain why.
>
> ram_addr_t represents a guest physical address.  From a ram_addr_t you can
> get a target_phys_addr_t.  Sometimes these are the same but they aren't
> always.
>
> You can have multiple ram_addr_t's pointing to the same target_phys_addr_t.
>  This is ram aliasing and it happens for a variety of reasons.  In general,
> it's pretty expensive to map a ram_addr_t to a target_phys_addr_t because,
> among other things, for a range of (ram_addr_t, size_t), you may have many
> (target_phys_addr_t, size) tuples that you have to deal with.
>
> vga_common_init() takes a target_phys_addr_t (well, it really takes an
> unsigned long, but that's a bug).  It takes this as an optimization.  It
> avoids having to do the conversion and ensures that it's one big linear
> region.
>
> For dirty tracking, we have a bitmap indexed by target_phys_addr_t in QEMU.
>  This means that we can happily set dirty bits based on
> target_phys_addr_t's.  We don't have to worry about what ram_addr_t it came
> from because they all map to the same bits.
>
> Since KVM uses a slot API, and that API is indexed in ram_addr_t's, we need
> to enable dirty tracking on the ram_addr_t's.  We don't have a ram_addr_t in
> the VGA code.
>
> The solution is pretty simple.  We need to keep track of the ram_addr_t's in
> the VGA code and enable dirty tracking on the appropriate ram_addr_ts.
>
> Regards,

My proposal is to let kvm interface to be:
void kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr,
target_phys_addr_t end_addr,
                                    ram_addr_t phys_offset)

to match, qemu then becomes:
void cpu_physical_sync_dirty_bitmap(target_phys_addr_t start_addr,
target_phys_addr_t end_addr,
                                    ram_addr_t phys_offset)

the caller does:

cpu_physical_sync_dirty_bitmap(s->map_addr, s->map_end, s->vram_offset);

This way, we tell kvm which particular area of the phys_ram_bitmap to
fill. I liked this
interface, because then we can have the behaviour of not synchronizing
back those bits
into qemu, by passing NULL as the third parameter.

I have a working version of it, will send shortly.




> Anthony Liguori
>
>> I hope this is the last version (Of course, once this is merged,
>> the optimizations of the optimization can start ;-) )
>>
>> I split it in 4 patches. The first two ones are just moving
>> things out of the way, and then #3 and #4 do the real thing.
>> #3 kvm-side, #4 overall qemu.
>>
>> They merge most of the suggestion Anthony and Stefano's sent
>> on last iteration.
>>
>> Hope you like it.
>>
>>
>>
>>
>
>
>
>



-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."