From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LDGcm-0004n2-Gj for qemu-devel@nongnu.org; Thu, 18 Dec 2008 06:01:00 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LDGcl-0004mh-OS for qemu-devel@nongnu.org; Thu, 18 Dec 2008 06:00:59 -0500 Received: from [199.232.76.173] (port=55812 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LDGcl-0004me-A8 for qemu-devel@nongnu.org; Thu, 18 Dec 2008 06:00:59 -0500 Received: from mx1.redhat.com ([66.187.233.31]:38108) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LDGck-0000uT-Lq for qemu-devel@nongnu.org; Thu, 18 Dec 2008 06:00:58 -0500 Date: Thu, 18 Dec 2008 11:00:14 +0000 From: "Daniel P. Berrange" Subject: Re: [Qemu-devel] Re: [PATCH 5/5] cache slot lookup Message-ID: <20081218110014.GI23277@redhat.com> References: <1229546822-11972-1-git-send-email-glommer@redhat.com> <1229546822-11972-2-git-send-email-glommer@redhat.com> <1229546822-11972-3-git-send-email-glommer@redhat.com> <1229546822-11972-4-git-send-email-glommer@redhat.com> <1229546822-11972-5-git-send-email-glommer@redhat.com> <1229546822-11972-6-git-send-email-glommer@redhat.com> <494A1AC4.30704@redhat.com> <20081218104850.GB19123@poweredge.glommer> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20081218104850.GB19123@poweredge.glommer> Reply-To: "Daniel P. Berrange" , qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Ian.Jackson@eu.citrix.com, Avi Kivity , kvm@vger.kernel.org, stefano.stabellini@eu.citrix.com On Thu, Dec 18, 2008 at 08:48:50AM -0200, Glauber Costa wrote: > On Thu, Dec 18, 2008 at 11:41:24AM +0200, Avi Kivity wrote: > > Glauber Costa wrote: > >> record slot used in last lookup. For the common mmio case, > >> we'll usually access the same memory slot repeatedly. > >> > > > >> --- a/kvm-all.c > >> +++ b/kvm-all.c > >> @@ -75,16 +75,25 @@ static KVMSlot *kvm_alloc_slot(KVMState *s) > >> return NULL; > >> } > >> +static KVMSlot *last_slot = NULL; > >> + > >> static KVMSlot *kvm_lookup_slot(KVMState *s, target_phys_addr_t start_addr) > >> { > >> int i; > >> + > >> + if (last_slot && (start_addr >= last_slot->start_addr && > >> + start_addr < (last_slot->start_addr + last_slot->memory_size))) > >> + return last_slot; > >> + > >> for (i = 0; i < ARRAY_SIZE(s->slots); i++) { > >> KVMSlot *mem = &s->slots[i]; > >> if (start_addr >= mem->start_addr && > >> - start_addr < (mem->start_addr + mem->memory_size)) > >> + start_addr < (mem->start_addr + mem->memory_size)) { > >> + last_slot = mem; > >> return mem; > >> + } > >> } > >> > > > > This wasn't introduced by this patch, but the comparison is broken ion > > i386 hosts, where target_phys_addr_t is 32 bits wide. mem->start_addr + > > mem->memory_size can overflow (this in fact happens for the bios slot at > > 4G-128K) > > AFAIK, the assumption is that kvm will always be qemu-system-x86_64, due to > migration issues. Then, _target_ phys_addr_t is always 64 bit wide. > If it's not the case, then this is really a problem. Migration compatability is a problem for mgmt apps to solve & merely saying to use qemu-system-x86_64 everywhere is a tiny insignificant piece of the problem. AFAIK, current Fedora packages build a 32-bit 'qemu' for KVM on i386 and a qemu-system-x86_64' for x86_64, which happens to be called 'qemu-kvm' on both, to avoid clash with the base QEMU binary names. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|