From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:34049) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rv2SI-0005pf-Ig for qemu-devel@nongnu.org; Wed, 08 Feb 2012 03:00:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Rv2SG-0001vy-8E for qemu-devel@nongnu.org; Wed, 08 Feb 2012 03:00:42 -0500 Received: from mailout2.w1.samsung.com ([210.118.77.12]:16043) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rv2SF-0001ur-UR for qemu-devel@nongnu.org; Wed, 08 Feb 2012 03:00:40 -0500 Received: from euspt1 (mailout2.w1.samsung.com [210.118.77.12]) by mailout2.w1.samsung.com (iPlanet Messaging Server 5.2 Patch 2 (built Jul 14 2004)) with ESMTP id <0LZ200LOCE8XC9@mailout2.w1.samsung.com> for qemu-devel@nongnu.org; Wed, 08 Feb 2012 08:00:33 +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 <0LZ200GQGE8W3X@spt1.w1.samsung.com> for qemu-devel@nongnu.org; Wed, 08 Feb 2012 08:00:33 +0000 (GMT) Date: Wed, 08 Feb 2012 12:00:31 +0400 From: Evgeny Voevodin In-reply-to: <4F32274A.9070401@web.de> Message-id: <4F322B9F.5080801@samsung.com> MIME-version: 1.0 Content-type: text/plain; charset=UTF-8; format=flowed Content-transfer-encoding: 7BIT References: <4F322204.20001@samsung.com> <4F32274A.9070401@web.de> 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: Jan Kiszka Cc: Blue Swirl , qemu-devel , Dmitry Solodkiy , Avi Kivity On 02/08/2012 11:42 AM, Jan Kiszka wrote: > On 2012-02-08 08:19, Evgeny Voevodin wrote: >> 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. >> > Do [1] or [2] make a difference? > > Jan > > [1] http://thread.gmane.org/gmane.comp.emulators.qemu/134958 Helped. > [2] http://thread.gmane.org/gmane.comp.emulators.qemu/135244 > > -- Kind regards, Evgeny Voevodin, Leading Software Engineer, ASWG, Moscow R&D center, Samsung Electronics e-mail: e.voevodin@samsung.com