qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 1/6] memory: change dirty getting API to take a


From: Jan Kiszka
Subject: Re: [Qemu-devel] [PATCH 1/6] memory: change dirty getting API to take a size
Date: Wed, 08 Feb 2012 08:42:02 +0100
User-agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666

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<address@hidden>
>> ---
>>   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
[2] http://thread.gmane.org/gmane.comp.emulators.qemu/135244


Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]