From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:42581) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rv1od-0004dz-A2 for qemu-devel@nongnu.org; Wed, 08 Feb 2012 02:19:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Rv1oZ-0002Xd-Mv for qemu-devel@nongnu.org; Wed, 08 Feb 2012 02:19:43 -0500 Received: from mailout1.w1.samsung.com ([210.118.77.11]:10398) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rv1oZ-0002XA-FX for qemu-devel@nongnu.org; Wed, 08 Feb 2012 02:19:39 -0500 Received: from euspt1 (mailout1.w1.samsung.com [210.118.77.11]) by mailout1.w1.samsung.com (iPlanet Messaging Server 5.2 Patch 2 (built Jul 14 2004)) with ESMTP id <0LZ200D0ZCCMKI@mailout1.w1.samsung.com> for qemu-devel@nongnu.org; Wed, 08 Feb 2012 07:19:34 +0000 (GMT) Received: from [106.109.9.191] by spt1.w1.samsung.com (iPlanet Messaging Server 5.2 Patch 2 (built Jul 14 2004)) with ESMTPA id <0LZ2004R7CCLZF@spt1.w1.samsung.com> for qemu-devel@nongnu.org; Wed, 08 Feb 2012 07:19:34 +0000 (GMT) Date: Wed, 08 Feb 2012 11:19:32 +0400 From: Evgeny Voevodin In-reply-to: Message-id: <4F322204.20001@samsung.com> MIME-version: 1.0 Content-type: text/plain; charset=UTF-8; format=flowed Content-transfer-encoding: 7BIT References: Subject: Re: [Qemu-devel] [PATCH 1/6] memory: change dirty getting API to take a size List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: Dmitry Solodkiy , Avi Kivity , qemu-devel On 01/29/2012 11:13 PM, Blue Swirl wrote: > Instead of each device knowing or guessing the guest page size, > just pass the desired size of dirtied memory area. > > Signed-off-by: Blue Swirl > --- > arch_init.c | 7 ++++--- > exec-obsolete.h | 15 +++++++++++++-- > hw/framebuffer.c | 9 +-------- > hw/g364fb.c | 3 ++- > hw/sm501.c | 11 ++++------- > hw/tcx.c | 19 +++++++++---------- > hw/vga.c | 17 +++++------------ > memory.c | 5 +++-- > memory.h | 9 +++++---- > 9 files changed, 46 insertions(+), 49 deletions(-) > > diff --git a/arch_init.c b/arch_init.c > index 2366511..699bdd1 100644 > --- a/arch_init.c > +++ b/arch_init.c > @@ -141,7 +141,8 @@ static int ram_save_block(QEMUFile *f) > > do { > mr = block->mr; > - if (memory_region_get_dirty(mr, offset, DIRTY_MEMORY_MIGRATION)) { > + if (memory_region_get_dirty(mr, offset, TARGET_PAGE_SIZE, > + DIRTY_MEMORY_MIGRATION)) { > uint8_t *p; > int cont = (block == last_block) ? RAM_SAVE_FLAG_CONTINUE : 0; > > @@ -198,7 +199,7 @@ static ram_addr_t ram_save_remaining(void) > QLIST_FOREACH(block,&ram_list.blocks, next) { > ram_addr_t addr; > for (addr = 0; addr< block->length; addr += TARGET_PAGE_SIZE) { > - if (memory_region_get_dirty(block->mr, addr, > + if (memory_region_get_dirty(block->mr, addr, TARGET_PAGE_SIZE, > DIRTY_MEMORY_MIGRATION)) { > count++; > } > @@ -283,7 +284,7 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int > stage, void *opaque) > /* Make sure all dirty bits are set */ > QLIST_FOREACH(block,&ram_list.blocks, next) { > for (addr = 0; addr< block->length; addr += TARGET_PAGE_SIZE) { > - if (!memory_region_get_dirty(block->mr, addr, > + if (!memory_region_get_dirty(block->mr, addr, TARGET_PAGE_SIZE, > DIRTY_MEMORY_MIGRATION)) { > memory_region_set_dirty(block->mr, addr, TARGET_PAGE_SIZE); > } > diff --git a/exec-obsolete.h b/exec-obsolete.h > index d2749d3..94c23d0 100644 > --- a/exec-obsolete.h > +++ b/exec-obsolete.h > @@ -59,10 +59,21 @@ static inline int > cpu_physical_memory_get_dirty_flags(ram_addr_t addr) > return ram_list.phys_dirty[addr>> TARGET_PAGE_BITS]; > } > > -static inline int cpu_physical_memory_get_dirty(ram_addr_t addr, > +static inline int cpu_physical_memory_get_dirty(ram_addr_t start, > + ram_addr_t length, > int dirty_flags) > { > - return ram_list.phys_dirty[addr>> TARGET_PAGE_BITS]& dirty_flags; > + int ret = 0; > + uint8_t *p; > + ram_addr_t addr, end; > + > + end = TARGET_PAGE_ALIGN(start + length); > + start&= TARGET_PAGE_MASK; > + p = ram_list.phys_dirty + (start>> TARGET_PAGE_BITS); > + for (addr = start; addr< end; addr += TARGET_PAGE_SIZE) { > + ret |= *p++& dirty_flags; > + } > + return ret; > } > > static inline void cpu_physical_memory_set_dirty(ram_addr_t addr) > diff --git a/hw/framebuffer.c b/hw/framebuffer.c > index 6bf48dc..ea122fb 100644 > --- a/hw/framebuffer.c > +++ b/hw/framebuffer.c > @@ -87,15 +87,8 @@ void framebuffer_update_display( > dest += i * dest_row_pitch; > > for (; i< rows; i++) { > - target_phys_addr_t dirty_offset; > - dirty = 0; > - dirty_offset = 0; > - while (addr + dirty_offset< TARGET_PAGE_ALIGN(addr + src_width)) { > - dirty |= memory_region_get_dirty(mem, addr + dirty_offset, > + dirty = memory_region_get_dirty(mem, addr, addr + src_width, > DIRTY_MEMORY_VGA); > - dirty_offset += TARGET_PAGE_SIZE; > - } > - > if (dirty || invalidate) { > fn(opaque, dest, src, cols, dest_col_pitch); > if (first == -1) > diff --git a/hw/g364fb.c b/hw/g364fb.c > index 82b31f7..fa25033 100644 > --- a/hw/g364fb.c > +++ b/hw/g364fb.c > @@ -62,7 +62,8 @@ typedef struct G364State { > > static inline int check_dirty(G364State *s, ram_addr_t page) > { > - return memory_region_get_dirty(&s->mem_vram, page, DIRTY_MEMORY_VGA); > + return memory_region_get_dirty(&s->mem_vram, page, G364_PAGE_SIZE, > + DIRTY_MEMORY_VGA); > } > > static inline void reset_dirty(G364State *s, > diff --git a/hw/sm501.c b/hw/sm501.c > index 09c5894..94c0abf 100644 > --- a/hw/sm501.c > +++ b/hw/sm501.c > @@ -1323,15 +1323,12 @@ static void sm501_draw_crt(SM501State * s) > for (y = 0; y< height; y++) { > int update_hwc = draw_hwc_line ? within_hwc_y_range(s, y, 1) : 0; > int update = full_update || update_hwc; > - ram_addr_t page0 = offset& TARGET_PAGE_MASK; > - ram_addr_t page1 = (offset + width * src_bpp - 1)& TARGET_PAGE_MASK; > - ram_addr_t page; > + ram_addr_t page0 = offset; > + ram_addr_t page1 = offset + width * src_bpp - 1; > > /* check dirty flags for each line */ > - for (page = page0; page<= page1; page += TARGET_PAGE_SIZE) > - if (memory_region_get_dirty(&s->local_mem_region, page, > - DIRTY_MEMORY_VGA)) > - update = 1; > + update = memory_region_get_dirty(&s->local_mem_region, page0, page1, > + DIRTY_MEMORY_VGA); > > /* draw line and change status */ > if (update) { > diff --git a/hw/tcx.c b/hw/tcx.c > index 7743c05..6054779 100644 > --- a/hw/tcx.c > +++ b/hw/tcx.c > @@ -178,15 +178,13 @@ static inline int check_dirty(TCXState *s, > ram_addr_t page, ram_addr_t page24, > ram_addr_t cpage) > { > int ret; > - unsigned int off; > - > - ret = memory_region_get_dirty(&s->vram_mem, page, DIRTY_MEMORY_VGA); > - for (off = 0; off< TARGET_PAGE_SIZE * 4; off += TARGET_PAGE_SIZE) { > - ret |= memory_region_get_dirty(&s->vram_mem, page24 + off, > - DIRTY_MEMORY_VGA); > - ret |= memory_region_get_dirty(&s->vram_mem, cpage + off, > - DIRTY_MEMORY_VGA); > - } > + > + ret = memory_region_get_dirty(&s->vram_mem, page, TARGET_PAGE_SIZE, > + DIRTY_MEMORY_VGA); > + ret |= memory_region_get_dirty(&s->vram_mem, page24, TARGET_PAGE_SIZE * 4, > + DIRTY_MEMORY_VGA); > + ret |= memory_region_get_dirty(&s->vram_mem, cpage, TARGET_PAGE_SIZE * 4, > + DIRTY_MEMORY_VGA); > return ret; > } > > @@ -245,7 +243,8 @@ static void tcx_update_display(void *opaque) > } > > for(y = 0; y< ts->height; y += 4, page += TARGET_PAGE_SIZE) { > - if (memory_region_get_dirty(&ts->vram_mem, page, DIRTY_MEMORY_VGA)) { > + if (memory_region_get_dirty(&ts->vram_mem, page, TARGET_PAGE_SIZE, > + DIRTY_MEMORY_VGA)) { > if (y_start< 0) > y_start = y; > if (page< page_min) > diff --git a/hw/vga.c b/hw/vga.c > index 4dc2610..cf9b39f 100644 > --- a/hw/vga.c > +++ b/hw/vga.c > @@ -1742,17 +1742,10 @@ static void vga_draw_graphic(VGACommonState > *s, int full_update) > if (!(s->cr[0x17]& 2)) { > addr = (addr& ~0x8000) | ((y1& 2)<< 14); > } > - page0 = addr& TARGET_PAGE_MASK; > - page1 = (addr + bwidth - 1)& TARGET_PAGE_MASK; > - update = full_update | > - memory_region_get_dirty(&s->vram, page0, DIRTY_MEMORY_VGA) | > - memory_region_get_dirty(&s->vram, page1, DIRTY_MEMORY_VGA); > - if ((page1 - page0)> TARGET_PAGE_SIZE) { > - /* if wide line, can use another page */ > - update |= memory_region_get_dirty(&s->vram, > - page0 + TARGET_PAGE_SIZE, > - DIRTY_MEMORY_VGA); > - } > + page0 = addr; > + page1 = addr + bwidth - 1; > + update = memory_region_get_dirty(&s->vram, page0, page1, > + DIRTY_MEMORY_VGA); > /* explicit invalidation for the hardware cursor */ > update |= (s->invalidated_y_table[y>> 5]>> (y& 0x1f))& 1; > if (update) { > @@ -1798,7 +1791,7 @@ static void vga_draw_graphic(VGACommonState *s, > int full_update) > if (page_max>= page_min) { > memory_region_reset_dirty(&s->vram, > page_min, > - page_max + TARGET_PAGE_SIZE - page_min, > + page_max - page_min, > DIRTY_MEMORY_VGA); > } > memset(s->invalidated_y_table, 0, ((height + 31)>> 5) * 4); > diff --git a/memory.c b/memory.c > index ee4c98a..5e77d8a 100644 > --- a/memory.c > +++ b/memory.c > @@ -1136,10 +1136,11 @@ void memory_region_set_log(MemoryRegion *mr, > bool log, unsigned client) > } > > bool memory_region_get_dirty(MemoryRegion *mr, target_phys_addr_t addr, > - unsigned client) > + target_phys_addr_t size, unsigned client) > { > assert(mr->terminates); > - return cpu_physical_memory_get_dirty(mr->ram_addr + addr, 1<< client); > + return cpu_physical_memory_get_dirty(mr->ram_addr + addr, size, > + 1<< client); > } > > void memory_region_set_dirty(MemoryRegion *mr, target_phys_addr_t addr, > diff --git a/memory.h b/memory.h > index fa45b99..4cf8d2f 100644 > --- a/memory.h > +++ b/memory.h > @@ -380,20 +380,21 @@ void memory_region_set_offset(MemoryRegion *mr, > target_phys_addr_t offset); > void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client); > > /** > - * memory_region_get_dirty: Check whether a page is dirty for a specified > - * client. > + * memory_region_get_dirty: Check whether a range of bytes is dirty > + * for a specified client. > * > - * Checks whether a page has been written to since the last > + * Checks whether a range of bytes has been written to since the last > * call to memory_region_reset_dirty() with the same @client. Dirty logging > * must be enabled. > * > * @mr: the memory region being queried. > * @addr: the address (relative to the start of the region) being queried. > + * @size: the size of the range being queried. > * @client: the user of the logging information; %DIRTY_MEMORY_MIGRATION or > * %DIRTY_MEMORY_VGA. > */ > bool memory_region_get_dirty(MemoryRegion *mr, target_phys_addr_t addr, > - unsigned client); > + target_phys_addr_t size, unsigned client); > > /** > * memory_region_set_dirty: Mark a range of bytes as dirty in a memory region. Since this patch was applied to master, my realview-pbx board emulation with graphics support started to work about 10 times slower (or even more, didn't make measures, but I can see it with the naked eye). This happens as soon as Linux kernel found an XVGA display and started to use frame buffer. -- Kind regards, Evgeny Voevodin, Leading Software Engineer, ASWG, Moscow R&D center, Samsung Electronics e-mail: e.voevodin@samsung.com