On 12/10/2011 06:45 PM, Blue Swirl wrote: > Instead of each target knowing or guessing the guest page size, > iterate through the dirty ranges. > > Signed-off-by: Blue Swirl <blauwir...@gmail.com> > --- > cpu-all.h | 30 ++++++++++++++++++++++++++++++ > hw/tcx.c | 54 ++++++++++++++++++++++-------------------------------- > hw/vga.c | 16 +++++++--------- > memory.c | 16 ++++++++++++++++ > memory.h | 24 ++++++++++++++++++++++++ > 5 files changed, 99 insertions(+), 41 deletions(-) > > diff --git a/cpu-all.h b/cpu-all.h > index 0cb62ca..a5c6670 100644 > --- a/cpu-all.h > +++ b/cpu-all.h > @@ -574,6 +574,36 @@ int cpu_physical_memory_set_dirty_tracking(int enable); > > int cpu_physical_memory_get_dirty_tracking(void); > > +static inline void cpu_physical_memory_range_find_dirty(ram_addr_t start, > + ram_addr_t end, > + ram_addr_t *pstart, > + ram_addr_t *pend, > + int flags) > +{ > + ram_addr_t idx; > + > + start >>= TARGET_PAGE_BITS; > + end += TARGET_PAGE_SIZE - 1; > + end >>= TARGET_PAGE_BITS; > + > + for (idx = start; idx < end; idx++) { > + if (ram_list.phys_dirty[idx] & flags) { > + *pstart = idx << TARGET_PAGE_BITS; > + for (; idx < end; idx++) { > + if (!(ram_list.phys_dirty[idx] & flags)) { > + *pend = (idx << TARGET_PAGE_BITS) - 1; > + return; > + } > + } > + *pend = (end << TARGET_PAGE_BITS) - 1;
This uses a convention of fully inclusive ranges, not semi inclusive which is what we usually use. > + return; > + } > + } > + /* everything pristine */ > + *pstart = (end << TARGET_PAGE_BITS) - 1; > + *pend = (end << TARGET_PAGE_BITS) - 1; > +} > + I prefer this to be implemented using memory_region primitives, less work for me to covert afterwards. Also, no need to inline this. > int cpu_physical_sync_dirty_bitmap(target_phys_addr_t start_addr, > target_phys_addr_t end_addr); > > diff --git a/hw/tcx.c b/hw/tcx.c > index fd45ce8..d031cbd 100644 > --- a/hw/tcx.c > +++ b/hw/tcx.c > @@ -212,7 +212,7 @@ static inline void reset_dirty(TCXState *ts, > ram_addr_t page_min, > static void tcx_update_display(void *opaque) > { > TCXState *ts = opaque; > - ram_addr_t page, page_min, page_max; > + ram_addr_t page, page_min, page_max, p; > int y, y_start, dd, ds; > uint8_t *d, *s; > void (*f)(TCXState *s1, uint8_t *dst, const uint8_t *src, int width); > @@ -244,37 +244,28 @@ static void tcx_update_display(void *opaque) > return; > } > > - 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 (y_start < 0) > - y_start = y; > - if (page < page_min) > - page_min = page; > - if (page > page_max) > - page_max = page; > - f(ts, d, s, ts->width); > - d += dd; > - s += ds; > - f(ts, d, s, ts->width); > - d += dd; > - s += ds; > - f(ts, d, s, ts->width); > - d += dd; > - s += ds; > - f(ts, d, s, ts->width); > - d += dd; > - s += ds; > - } else { > - if (y_start >= 0) { > - /* flush to display */ > - dpy_update(ts->ds, 0, y_start, > - ts->width, y - y_start); > - y_start = -1; > - } > - d += dd * 4; > - s += ds * 4; > + assert(MAXX == 1024); > + y = 0; > + for (p = 0; p < MAXX * MAXY;) { > + target_phys_addr_t dirty_start, dirty_end; > + > + memory_region_find_dirty(&ts->vram_mem, p, MAXX * MAXY, &dirty_start, > + &dirty_end, DIRTY_MEMORY_VGA); > + if (dirty_start == MAXX * MAXY - 1) { > + break; > + } Gives no way to indicate that just that one byte is dirty (possible if MAXX * MAXY == 1 (mod TARGET_PAGE_SIZE)) > + page = dirty_start; > + f(ts, d + (page >> 10) * dd, s + page, dirty_end - dirty_start); > + if (y_start < 0) { > + page_min = dirty_start; > + /* divide by MAXX */ > + y_start = page_min >> 10; page_min / MAXX? > } > + page_max = dirty_end; > + y = page_max >> 10; > + p = dirty_end + 1; > } > + > if (y_start >= 0) { > /* flush to display */ > dpy_update(ts->ds, 0, y_start, > @@ -282,8 +273,7 @@ static void tcx_update_display(void *opaque) > } > /* reset modified pages */ > if (page_max >= page_min) { > - memory_region_reset_dirty(&ts->vram_mem, > - page_min, page_max + TARGET_PAGE_SIZE, > + memory_region_reset_dirty(&ts->vram_mem, page_min, page_max, > DIRTY_MEMORY_VGA); fully-inclusive/semi-inclusive mixup > /** > + * memory_region_find_dirty: Find dirty ranges in a memory range for a > + * specified client. > + * > + * Returns lowest contiguous dirty memory range within the specified > memory range. > + * Dirty logging must be enabled. > + * > + * @mr: the memory region being queried. > + * @start: the start address (relative to the start of the region) of the > + * region being searched. > + * @end: the end address (relative to the start of the region) of the > + * region being searched. > + * *@pstart: the returned start address (relative to the start of the region) > + * of the lowest contiguous dirty range found, or @end if not found. > + * *@pend: the returned end address (relative to the start of the region) > + * of the lowest contiguous dirty range found. > + * @client: the user of the logging information; %DIRTY_MEMORY_MIGRATION or > + * %DIRTY_MEMORY_VGA. > + */ > +void memory_region_find_dirty(MemoryRegion *mr, target_phys_addr_t start, > + target_phys_addr_t end, > + target_phys_addr_t *pstart, > + target_phys_addr_t *pend, > + unsigned client); > Might wrap in a MEMORY_REGION_FOR_EACH_DIRTY_RANGE() macro. -- error compiling committee.c: too many arguments to function