From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:55322) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RZfaW-0008DY-RB for qemu-devel@nongnu.org; Sun, 11 Dec 2011 04:20:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RZfaV-0006Ln-PD for qemu-devel@nongnu.org; Sun, 11 Dec 2011 04:20:52 -0500 Received: from mx1.redhat.com ([209.132.183.28]:7321) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RZfaV-0006Lj-IU for qemu-devel@nongnu.org; Sun, 11 Dec 2011 04:20:51 -0500 Message-ID: <4EE475EE.4070103@redhat.com> Date: Sun, 11 Dec 2011 11:20:46 +0200 From: Avi Kivity MIME-Version: 1.0 References: In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/6] memory: change dirty setting APIs to take a size List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: qemu-devel On 12/10/2011 06:44 PM, Blue Swirl wrote: > Instead of each target knowing or guessing the guest page size, > just pass the desired size of dirtied memory area. This should also > improve performance due to memset() optimizations. > > > -static inline void cpu_physical_memory_set_dirty(ram_addr_t addr) > +static inline void cpu_physical_memory_range_set_dirty(ram_addr_t start, > + ram_addr_t size) > { Since you're changing all callers in one patch, might as well keep the shorter name. > - ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] = 0xff; > + start >>= TARGET_PAGE_BITS; > + size += TARGET_PAGE_SIZE - 1; > + size >>= TARGET_PAGE_BITS; > + This is wrong, consider start == 0xfff && size == 2; you only dirty one page. > + memset(&ram_list.phys_dirty[start], 0xff, size); > } > > static inline int cpu_physical_memory_set_dirty_flags(ram_addr_t addr, > diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c > @@ -1918,8 +1915,8 @@ static void > cirrus_mem_writeb_mode4and5_8bpp(CirrusVGAState * s, > val <<= 1; > dst++; > } > - memory_region_set_dirty(&s->vga.vram, offset); > - memory_region_set_dirty(&s->vga.vram, offset + 7); > + memory_region_set_dirty(&s->vga.vram, offset, 1); > + memory_region_set_dirty(&s->vga.vram, offset + 7, 1); > } memory_region_set_dirty(..., offset, 8) matches the preceding code better > -void memory_region_set_dirty(MemoryRegion *mr, target_phys_addr_t addr) > +void memory_region_set_dirty(MemoryRegion *mr, target_phys_addr_t addr, > + target_phys_addr_t size) > { > assert(mr->terminates); > - return cpu_physical_memory_set_dirty(mr->ram_addr + addr); > + return cpu_physical_memory_range_set_dirty(mr->ram_addr + addr, size); > } > > void memory_region_sync_dirty_bitmap(MemoryRegion *mr) > diff --git a/memory.h b/memory.h > index 53bf261..1f8b5a5 100644 > --- a/memory.h > +++ b/memory.h > @@ -318,10 +318,12 @@ bool memory_region_get_dirty(MemoryRegion *mr, > target_phys_addr_t addr, > * > * Marks a page as dirty, after it has been dirtied outside guest code. a range of bytes > * > - * @mr: the memory region being queried. > + * @mr: the memory region being dirtied. > * @addr: the address (relative to the start of the region) being dirtied. > + * @size: size of the range being dirtied. > */ > -void memory_region_set_dirty(MemoryRegion *mr, target_phys_addr_t addr); > +void memory_region_set_dirty(MemoryRegion *mr, target_phys_addr_t addr, > + target_phys_addr_t size); > > -- error compiling committee.c: too many arguments to function