From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 12/21] Remove odd hack in vga.c Date: Thu, 30 Apr 2009 12:39:01 +0300 Message-ID: <49F971B5.8060701@redhat.com> References: <1241040038-17183-1-git-send-email-aliguori@us.ibm.com> <1241040038-17183-13-git-send-email-aliguori@us.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org To: Anthony Liguori Return-path: Received: from mx2.redhat.com ([66.187.237.31]:60737 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751335AbZD3JjF (ORCPT ); Thu, 30 Apr 2009 05:39:05 -0400 In-Reply-To: <1241040038-17183-13-git-send-email-aliguori@us.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: Anthony Liguori wrote: > I looked closely at the vga code in kvm-userspace a while ago and merged > every fix I could understand into upstream QEMU. This particular change makes > no sense to me. I could not figure out from revision history what it actually > fixed. I'm fairly certain it's not useful today. > > Signed-off-by: Anthony Liguori > --- > hw/vga.c | 27 ++++----------------------- > 1 files changed, 4 insertions(+), 23 deletions(-) > > diff --git a/hw/vga.c b/hw/vga.c > index d96f1be..385184a 100644 > --- a/hw/vga.c > +++ b/hw/vga.c > @@ -2227,33 +2227,14 @@ typedef struct PCIVGAState { > VGAState vga_state; > } PCIVGAState; > > -static int s1, s2; > - > -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(VGAState *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); > } > } > > This makes live migration and vga dirty tracking work together. Unfortunately since the last merge with qemu it's broken. We have a shared resource, the log_dirty flag of memory slots. We can't call log_start() and log_stop() from different users and expect things to work. One cleaner way to fix this is to add a parameter containing the mask which will be used by the client to access the qemu bytemap. log_start() can OR this parameter with its own copy, and log_stop() can AND NOT the same thing. When the local copy is nonzero, the slot dirty log is enabled. -- error compiling committee.c: too many arguments to function