From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:58108) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QZnO4-0002jY-92 for qemu-devel@nongnu.org; Thu, 23 Jun 2011 13:08:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QZnO2-0008On-SU for qemu-devel@nongnu.org; Thu, 23 Jun 2011 13:08:16 -0400 Received: from mail-pw0-f45.google.com ([209.85.160.45]:44858) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QZnO2-0008Oj-Gs for qemu-devel@nongnu.org; Thu, 23 Jun 2011 13:08:14 -0400 Received: by pwi5 with SMTP id 5so1410570pwi.4 for ; Thu, 23 Jun 2011 10:08:12 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1308454790-13750-6-git-send-email-agraf@suse.de> References: <1308452228-11055-1-git-send-email-agraf@suse.de> <1308454790-13750-6-git-send-email-agraf@suse.de> Date: Thu, 23 Jun 2011 18:08:12 +0100 Message-ID: From: Peter Maydell Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH 06/11] exec.c: refactor cpu_physical_memory_map List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: "xen-devel@lists.xensource.com Devel" , "qemu-devel@nongnu.org Developers" , Stefano Stabellini On 19 June 2011 04:39, Alexander Graf wrote: > From: Stefano Stabellini > > Introduce qemu_ram_ptr_length that takes an address and a size as > parameters rather than just an address. > > Refactor cpu_physical_memory_map so that we call qemu_ram_ptr_length only > once rather than calling qemu_get_ram_ptr one time per page. > This is not only more efficient but also tries to simplify the logic of > the function. This change breaks cpu_physical_memory_map() in the case where the passed in physical memory address corresponds to a RAM block which has been mapped in at multiple physical addresses. Specifically, for Versatile Express this means we abort() when framebuffer_update_display() calls cpu_physical_memory_map() for an address which is in the second mapped area. > +/* Return a host pointer to guest's ram. Similar to qemu_get_ram_ptr > + * but takes a size argument */ > +void *qemu_ram_ptr_length(target_phys_addr_t addr, target_phys_addr_t *size) qemu_get_ram_ptr() takes a ramaddr_t, not a target_phys_addr_t; so should this, because we're just looking through the ram_list without doing physaddr to ramaddr translation. Conceptually size should also be a ram_addr_t*, although if you do that you can't just pass the plen pointer through to it. The old implementation of cpu_physical_memory_map() worked because it created the address to pass to qemu_get_ram_ptr() from the p->phys_offset it got from phys_page_find(). The new implementation needs to do something similar, not just pass a target_phys_addr_t to qemu_ram_ptr_length(). > + fprintf(stderr, "Bad ram offset %" PRIx64 "\n", (uint64_t)addr); > + abort(); > + > + *size = 0; > + return NULL; There doesn't seem much point in having code following an abort(). -- PMM