From: Evgeny Voevodin <e.voevodin@samsung.com>
To: Blue Swirl <blauwirbel@gmail.com>
Cc: Dmitry Solodkiy <d.solodkiy@samsung.com>,
Avi Kivity <avi@redhat.com>, qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [PATCH 1/6] memory: change dirty getting API to take a size
Date: Wed, 08 Feb 2012 11:19:32 +0400 [thread overview]
Message-ID: <4F322204.20001@samsung.com> (raw)
In-Reply-To: <CAAu8pHus7v7YR8nYnZD+ZitiV_nN_o3MM7zWe0PgMKZ=TgOt6w@mail.gmail.com>
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<blauwirbel@gmail.com>
> ---
> 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
next prev parent reply other threads:[~2012-02-08 7:19 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-01-29 19:13 [Qemu-devel] [PATCH 1/6] memory: change dirty getting API to take a size Blue Swirl
2012-02-08 7:19 ` Evgeny Voevodin [this message]
2012-02-08 7:42 ` Jan Kiszka
2012-02-08 8:00 ` Evgeny Voevodin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4F322204.20001@samsung.com \
--to=e.voevodin@samsung.com \
--cc=avi@redhat.com \
--cc=blauwirbel@gmail.com \
--cc=d.solodkiy@samsung.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).