From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KxjXK-0007RX-1I for qemu-devel@nongnu.org; Wed, 05 Nov 2008 09:39:10 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KxjXJ-0007RD-Fi for qemu-devel@nongnu.org; Wed, 05 Nov 2008 09:39:09 -0500 Received: from [199.232.76.173] (port=43014 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KxjXJ-0007RA-3d for qemu-devel@nongnu.org; Wed, 05 Nov 2008 09:39:09 -0500 Received: from smtp.eu.citrix.com ([62.200.22.115]:51083) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KxjXI-0001bb-FZ for qemu-devel@nongnu.org; Wed, 05 Nov 2008 09:39:08 -0500 Message-ID: <4911B0CE.5070108@eu.citrix.com> Date: Wed, 05 Nov 2008 14:42:22 +0000 From: Stefano Stabellini MIME-Version: 1.0 Subject: Re: [Qemu-devel] vga optmization References: <20081103173111.GC30410@poweredge.glommer> <491034BC.2050806@eu.citrix.com> <49106171.5080209@redhat.com> <491063C1.4080202@eu.citrix.com> <20081104202846.GB27481@poweredge.glommer> In-Reply-To: <20081104202846.GB27481@poweredge.glommer> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Glauber Costa Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org Glauber Costa wrote: >> That's also how currently qemu-xen works. >> I am glad that we agree :) > > I'm attaching a new version. Let me know if it's better this way. > Yes, this is much better thanks. I still have comments about possible improvements, so I wrote patch (against your patch). This patch is not meant to be applied, is only meant to be read (I believe that C code is more meaningful than English :). Some of the changes include: - instead of adding cirrus_lfb_addr, add a more generic lfb_addr to VGAState, so that can be reused in the future for possible stdvga only mappings; - instead of using cirrus_lfb_mapped as a boolean, use it as the mapping address, it is more useful that way; - instead of keeping the kvm dirty map always enabled, enable it only when the framebuffer is linear and in graphical mode; - look at the changes to vga.c, there is a simple check to reduce the dirty area to sync. It would be nice to check if the last two changes are actually a performance improvement. diff -r a5bcebe9e2bc hw/cirrus_vga.c --- a/hw/cirrus_vga.c Wed Nov 05 12:09:13 2008 +0000 +++ b/hw/cirrus_vga.c Wed Nov 05 14:29:54 2008 +0000 @@ -249,9 +249,6 @@ int cirrus_linear_io_addr; int cirrus_linear_bitblt_io_addr; int cirrus_mmio_io_addr; - uint32_t cirrus_lfb_addr; - uint32_t cirrus_lfb_end; - uint32_t cirrus_lfb_mapped; uint32_t cirrus_addr_mask; uint32_t linear_mmio_mask; uint8_t cirrus_shadow_gr0; @@ -2655,9 +2652,10 @@ static void map_linear_vram(CirrusVGAState *s) { - if (!s->cirrus_lfb_mapped && s->cirrus_lfb_addr && s->cirrus_lfb_end) { - set_vram_mapping(s->cirrus_lfb_addr, s->cirrus_lfb_end, s->vram_offset); - s->cirrus_lfb_mapped = 1; + if (!s->map_addr && s->lfb_addr && s->lfb_end) { + set_vram_mapping(s->lfb_addr, s->lfb_end, s->vram_offset); + s->map_addr = s->lfb_addr; + s->map_end = s->lfb_end; } if(!(s->cirrus_srcptr != s->cirrus_srcptr_end) @@ -2674,10 +2672,10 @@ static void unmap_linear_vram(CirrusVGAState *s) { - if (s->cirrus_lfb_mapped && s->cirrus_lfb_addr && s->cirrus_lfb_end) { - unset_vram_mapping(s->cirrus_lfb_addr, - s->cirrus_lfb_end); - s->cirrus_lfb_mapped = 0; + if (s->map_addr && s->lfb_addr && s->lfb_end) { + unset_vram_mapping(s->lfb_addr, + s->lfb_end); + s->map_addr = s->map_end = 0; } cpu_register_physical_memory(isa_mem_base + 0x000a0000, 0x20000, @@ -3331,9 +3329,9 @@ cpu_register_physical_memory(addr + 0x1000000, 0x400000, s->cirrus_linear_bitblt_io_addr); - s->cirrus_lfb_mapped = 0; - s->cirrus_lfb_addr = addr; - s->cirrus_lfb_end = addr + VGA_RAM_SIZE; + s->map_addr = 0; + s->lfb_addr = addr; + s->lfb_end = addr + VGA_RAM_SIZE; } static void cirrus_pci_mmio_map(PCIDevice *d, int region_num, diff -r a5bcebe9e2bc hw/vga.c --- a/hw/vga.c Wed Nov 05 12:09:13 2008 +0000 +++ b/hw/vga.c Wed Nov 05 14:29:54 2008 +0000 @@ -1244,6 +1244,9 @@ vga_draw_glyph8_func *vga_draw_glyph8; vga_draw_glyph9_func *vga_draw_glyph9; + /* disable dirty bit tracking */ + // kvm_log_stop(); + full_update |= update_palette16(s); palette = s->last_palette; @@ -1569,7 +1572,8 @@ uint32_t v, addr1, addr; vga_draw_line_func *vga_draw_line; - qemu_physical_sync_dirty_bitmap(s->vram_offset, s->vram_offset + VGA_RAM_SIZE); + /* Enable dirty bit tracking */ + // kvm_log_start full_update |= update_basic_params(s); @@ -1657,6 +1661,31 @@ s->cursor_invalidate(s); line_offset = s->line_offset; + + if (s->lfb_addr) { + if (height - 1 > s->line_compare || multi_run || (s->cr[0x17] & 3) != 3) { + /* Tricky things happen, just track all video memory */ + start = 0; + end = s->vram_size; + } else { + /* Tricky things won't have any effect, i.e. we are in the very simple + * (and very usual) case of a linear buffer. */ + /* use page table dirty bit tracking for the LFB plus border */ + start = (s->start_addr * 4) & TARGET_PAGE_MASK; + end = ((s->start_addr * 4 + height * line_offset) + TARGET_PAGE_SIZE - 1) & TARGET_PAGE_MASK; + } + + for (y = 0 ; y < start; y += TARGET_PAGE_SIZE) + /* We will not read that anyway. */ + cpu_physical_memory_set_dirty(s->vram_offset + y); + + qemu_physical_sync_dirty_bitmap(s->lfb_addr + y, end); + + for ( ; y < s->vram_size; y += TARGET_PAGE_SIZE) + /* We will not read that anyway. */ + cpu_physical_memory_set_dirty(s->vram_offset + y); + } + #if 0 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]); @@ -1746,6 +1775,10 @@ return; if (s->last_scr_width <= 0 || s->last_scr_height <= 0) return; + + /* disable dirty bit tracking */ + // kvm_log_stop(); + if (s->ds->depth == 8) val = s->rgb_to_pixel(0, 0, 0); else diff -r a5bcebe9e2bc hw/vga_int.h --- a/hw/vga_int.h Wed Nov 05 12:09:13 2008 +0000 +++ b/hw/vga_int.h Wed Nov 05 14:29:54 2008 +0000 @@ -106,6 +106,10 @@ unsigned int bios_size; \ target_phys_addr_t base_ctrl; \ int it_shift; \ + unsigned long lfb_addr; \ + unsigned long lfb_end; \ + uint32_t map_addr; \ + uint32_t map_end; \ PCIDevice *pci_dev; \ uint32_t latch; \ uint8_t sr_index; \