From: Evgeny Voevodin <e.voevodin@samsung.com>
To: Jan Kiszka <jan.kiszka@web.de>
Cc: Blue Swirl <blauwirbel@gmail.com>,
qemu-devel <qemu-devel@nongnu.org>,
Dmitry Solodkiy <d.solodkiy@samsung.com>,
Avi Kivity <avi@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 1/6] memory: change dirty getting API to take a size
Date: Wed, 08 Feb 2012 12:00:31 +0400 [thread overview]
Message-ID: <4F322B9F.5080801@samsung.com> (raw)
In-Reply-To: <4F32274A.9070401@web.de>
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<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.
>>
> 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
prev parent reply other threads:[~2012-02-08 8:00 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
2012-02-08 7:42 ` Jan Kiszka
2012-02-08 8:00 ` Evgeny Voevodin [this message]
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=4F322B9F.5080801@samsung.com \
--to=e.voevodin@samsung.com \
--cc=avi@redhat.com \
--cc=blauwirbel@gmail.com \
--cc=d.solodkiy@samsung.com \
--cc=jan.kiszka@web.de \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.