From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KxSht-0004p5-6h for qemu-devel@nongnu.org; Tue, 04 Nov 2008 15:40:57 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KxShq-0004ky-TS for qemu-devel@nongnu.org; Tue, 04 Nov 2008 15:40:56 -0500 Received: from [199.232.76.173] (port=47676 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KxShq-0004km-Pi for qemu-devel@nongnu.org; Tue, 04 Nov 2008 15:40:54 -0500 Received: from e33.co.us.ibm.com ([32.97.110.151]:46027) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1KxShr-0005wL-3o for qemu-devel@nongnu.org; Tue, 04 Nov 2008 15:40:55 -0500 Received: from d03relay02.boulder.ibm.com (d03relay02.boulder.ibm.com [9.17.195.227]) by e33.co.us.ibm.com (8.13.1/8.13.1) with ESMTP id mA4KeS1J018031 for ; Tue, 4 Nov 2008 13:40:28 -0700 Received: from d03av04.boulder.ibm.com (d03av04.boulder.ibm.com [9.17.195.170]) by d03relay02.boulder.ibm.com (8.13.8/8.13.8/NCO v9.1) with ESMTP id mA4KeouY127038 for ; Tue, 4 Nov 2008 13:40:50 -0700 Received: from d03av04.boulder.ibm.com (loopback [127.0.0.1]) by d03av04.boulder.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id mA4KenIc023539 for ; Tue, 4 Nov 2008 13:40:50 -0700 Message-ID: <4910B34F.8000608@us.ibm.com> Date: Tue, 04 Nov 2008 14:40:47 -0600 From: Anthony Liguori 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=ISO-8859-1; format=flowed 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: qemu-devel@nongnu.org, Stefano Stabellini Glauber Costa wrote: > diff --git a/exec.c b/exec.c > index ef1072b..2ed5e2b 100644 > --- a/exec.c > +++ b/exec.c > @@ -1823,6 +1823,17 @@ int cpu_physical_memory_get_dirty_tracking(void) > return in_migration; > } > > +void qemu_physical_sync_dirty_bitmap(ram_addr_t start_addr, ram_addr_t end_addr) > +{ > + if (kvm_enabled()) { > + ram_addr_t addr; > + kvm_physical_sync_dirty_bitmap(start_addr); > + for (addr = start_addr; addr < end_addr; addr += TARGET_PAGE_SIZE) > + phys_ram_dirty[addr >> TARGET_PAGE_BITS] |= kvm_physical_memory_get_dirty(addr); Might as well push this into kvm-all.c so that this becomes: if (kvm_enabled()) kvm_physical_sync_dirty_bitmap(start_addr, end_addr); And this very likely will become a hook one day. And I think the function should be renamed to cpu_physical_memory_update_dirty() to be more in line with the rest of the functions. > > + } > +} > + > 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 af9c9e6..bc1a3af 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: > @@ -248,6 +249,9 @@ typedef struct CirrusVGAState { > 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; > @@ -2618,6 +2622,69 @@ static CPUWriteMemoryFunc *cirrus_linear_bitblt_write[3] = { > cirrus_linear_bitblt_writel, > }; > > +static void set_vram_mapping(target_phys_addr_t begin, target_phys_addr_t end, ram_addr_t target) > +{ > + /* align begin and end address */ > + begin = begin & TARGET_PAGE_MASK; > + end = begin + VGA_RAM_SIZE; > + end = (end + TARGET_PAGE_SIZE -1 ) & TARGET_PAGE_MASK; > + > + cpu_register_physical_memory(begin, end - begin, target); > + > + if (kvm_enabled()) { > + /* XXX: per-slot dirty tracking in qemu may get rid of it */ > + kvm_log_start(begin, end - begin); > I feel confident that we can get rid of this. We just need a good suggestion. > + } > +} > + > +static void unset_vram_mapping(target_phys_addr_t begin, target_phys_addr_t end) > +{ > + /* align begin and end address */ > + end = begin + VGA_RAM_SIZE; > + begin = begin & TARGET_PAGE_MASK; > + end = (end + TARGET_PAGE_SIZE -1 ) & TARGET_PAGE_MASK; > + > + if (kvm_enabled()) { > + /* XXX: per-slot dirty tracking in qemu may get rid of it */ > + kvm_log_stop(begin, end - begin); > + } > + cpu_register_physical_memory(begin, end - begin, IO_MEM_UNASSIGNED); > This is a little odd. We only ever toggle the vram mapping from normal IO_RAM to MMIO. We should never switch it to IO_MEM_UNASSIGNED. > +int kvm_log_stop(target_phys_addr_t phys_addr, target_phys_addr_t len) > +{ > + > + KVMState *s = kvm_state; > + KVMSlot *mem = kvm_lookup_slot(s, phys_addr); > + > + if (mem == NULL) { > + fprintf(stderr, "BUG: %s: invalid parameters\n", __func__); > + return -EINVAL; > + } > + dprintf("slot %d: disabling logging\n", mem->region.slot); > + > + if (mem->logging_count--) > + return 0; > + > + return kvm_dirty_pages_log_change(mem, > + 0, > + KVM_MEM_LOG_DIRTY_PAGES); > +} > + > +#define BITMAP_WORD(mem) (sizeof(*(mem)->dirty_bitmap) * 8) > this doesn't need to be a macro. Can you split this patch up a bit? At least one patch containing the KVM slot changes. Also split out the cirrus verse standard VGA changes. I think if you switch things around to get rid of kvm_physical_get_dirty(), you'll find you can malloc the memory for the dirty bitmap at the time of sync() which should significantly reduce your overall complexity. Regards, Anthony Liguori