Re: [Qemu-devel] [PATCH v2 3/4] memory: change dirtying APIs to take a size

[Avi Kivity <avi@redhat.com>]

On 10/16/2011 06:50 PM, Blue Swirl wrote:
> Instead of each target knowing or guessing the guest page size,
> just pass the desired size of dirtied memory area. This should also
> improve performance due to memset() optimizations.

Looks good.

> cirrus_mem_writeb_mode4and5_8bpp(CirrusVGAState * s,
>  	val <<= 1;
>  	dst++;
>      }
> -    memory_region_set_dirty(&s->vga.vram, offset);
> -    memory_region_set_dirty(&s->vga.vram, offset + 7);
> +    memory_region_set_dirty(&s->vga.vram, offset, 1);
> +    memory_region_set_dirty(&s->vga.vram, offset + 7, 1);
>  }

Can be transformed into a single call (but better not in this patch).

>  static void cirrus_mem_writeb_mode4and5_16bpp(CirrusVGAState * s,
> @@ -1948,8 +1945,8 @@ static void
> cirrus_mem_writeb_mode4and5_16bpp(CirrusVGAState * s,
>  	val <<= 1;
>  	dst += 2;
>      }
> -    memory_region_set_dirty(&s->vga.vram, offset);
> -    memory_region_set_dirty(&s->vga.vram, offset + 15);
> +    memory_region_set_dirty(&s->vga.vram, offset, 1);
> +    memory_region_set_dirty(&s->vga.vram, offset + 15, 1);
>  }
>
>  /***************************************
> @@ -2039,7 +2036,8 @@ static void cirrus_vga_mem_write(void *opaque,
>  		mode = s->vga.gr[0x05] & 0x7;
>  		if (mode < 4 || mode > 5 || ((s->vga.gr[0x0B] & 0x4) == 0)) {
>  		    *(s->vga.vram_ptr + bank_offset) = mem_value;
> -		    memory_region_set_dirty(&s->vga.vram, bank_offset);
> +                    memory_region_set_dirty(&s->vga.vram, bank_offset,
> +                                            sizeof(mem_value));
>  		} else {
>  		    if ((s->vga.gr[0x0B] & 0x14) != 0x14) {
>  			cirrus_mem_writeb_mode4and5_8bpp(s, mode,
> @@ -2311,7 +2309,7 @@ static void cirrus_linear_write(void *opaque,
> target_phys_addr_t addr,
>  	mode = s->vga.gr[0x05] & 0x7;
>  	if (mode < 4 || mode > 5 || ((s->vga.gr[0x0B] & 0x4) == 0)) {
>  	    *(s->vga.vram_ptr + addr) = (uint8_t) val;
> -	    memory_region_set_dirty(&s->vga.vram, addr);
> +            memory_region_set_dirty(&s->vga.vram, addr, 1);
>  	} else {
>  	    if ((s->vga.gr[0x0B] & 0x14) != 0x14) {
>  		cirrus_mem_writeb_mode4and5_8bpp(s, mode, addr, val);
> diff --git a/hw/g364fb.c b/hw/g364fb.c
> index f00ee27..166839b 100644
> --- a/hw/g364fb.c
> +++ b/hw/g364fb.c
> @@ -268,12 +268,9 @@ static void g364fb_update_display(void *opaque)
>  static inline void g364fb_invalidate_display(void *opaque)
>  {
>      G364State *s = opaque;
> -    int i;
>
>      s->blanked = 0;
> -    for (i = 0; i < s->vram_size; i += G364_PAGE_SIZE) {
> -        memory_region_set_dirty(&s->mem_vram, i);
> -    }
> +    memory_region_set_dirty(&s->mem_vram, 0, s->vram_size);
>  }
>
>  static void g364fb_reset(G364State *s)
> @@ -385,7 +382,7 @@ static void g364fb_update_depth(G364State *s)
>
>  static void g364_invalidate_cursor_position(G364State *s)
>  {
> -    int ymin, ymax, start, end, i;
> +    int ymin, ymax, start, end;
>
>      /* invalidate only near the cursor */
>      ymin = s->cursor_position & 0xfff;
> @@ -393,9 +390,7 @@ static void g364_invalidate_cursor_position(G364State *s)
>      start = ymin * ds_get_linesize(s->ds);
>      end = (ymax + 1) * ds_get_linesize(s->ds);
>
> -    for (i = start; i < end; i += G364_PAGE_SIZE) {
> -        memory_region_set_dirty(&s->mem_vram, i);
> -    }
> +    memory_region_set_dirty(&s->mem_vram, start, end - start);
>  }
>
>  static void g364fb_ctrl_write(void *opaque,
> diff --git a/hw/qxl.c b/hw/qxl.c
> index 03848ed..b1e0d80 100644
> --- a/hw/qxl.c
> +++ b/hw/qxl.c
> @@ -341,10 +341,7 @@ static void init_qxl_ram(PCIQXLDevice *d)
>  /* can be called from spice server thread context */
>  static void qxl_set_dirty(MemoryRegion *mr, ram_addr_t addr, ram_addr_t end)
>  {
> -    while (addr < end) {
> -        memory_region_set_dirty(mr, addr);
> -        addr += TARGET_PAGE_SIZE;
> -    }
> +    memory_region_set_dirty(mr, addr, end - addr);
>  }
>
>  static void qxl_rom_set_dirty(PCIQXLDevice *qxl)
> diff --git a/hw/tcx.c b/hw/tcx.c
> index 309600d..e48c564 100644
> --- a/hw/tcx.c
> +++ b/hw/tcx.c
> @@ -53,21 +53,13 @@ static void tcx24_screen_dump(void *opaque, const
> char *filename);
>
>  static void tcx_set_dirty(TCXState *s)
>  {
> -    unsigned int i;
> -
> -    for (i = 0; i < MAXX * MAXY; i += TARGET_PAGE_SIZE) {
> -        cpu_physical_memory_set_dirty(s->vram_offset + i);
> -    }
> +    cpu_physical_memory_range_set_dirty(s->vram_offset, MAXX * MAXY);
>  }
>
>  static void tcx24_set_dirty(TCXState *s)
>  {
> -    unsigned int i;
> -
> -    for (i = 0; i < MAXX * MAXY * 4; i += TARGET_PAGE_SIZE) {
> -        cpu_physical_memory_set_dirty(s->vram24_offset + i);
> -        cpu_physical_memory_set_dirty(s->cplane_offset + i);
> -    }
> +    cpu_physical_memory_range_set_dirty(s->vram24_offset, MAXX * MAXY * 4);
> +    cpu_physical_memory_range_set_dirty(s->cplane_offset, MAXX * MAXY * 4);
>  }
>
>  static void update_palette_entries(TCXState *s, int start, int end)
> diff --git a/hw/vga.c b/hw/vga.c
> index ca79aa1..85176a6 100644
> --- a/hw/vga.c
> +++ b/hw/vga.c
> @@ -853,7 +853,7 @@ void vga_mem_writeb(VGACommonState *s,
> target_phys_addr_t addr, uint32_t val)
>              printf("vga: chain4: [0x" TARGET_FMT_plx "]\n", addr);
>  #endif
>              s->plane_updated |= mask; /* only used to detect font change */
> -            memory_region_set_dirty(&s->vram, addr);
> +            memory_region_set_dirty(&s->vram, addr, 1);
>          }
>      } else if (s->gr[5] & 0x10) {
>          /* odd/even mode (aka text mode mapping) */
> @@ -866,7 +866,7 @@ void vga_mem_writeb(VGACommonState *s,
> target_phys_addr_t addr, uint32_t val)
>              printf("vga: odd/even: [0x" TARGET_FMT_plx "]\n", addr);
>  #endif
>              s->plane_updated |= mask; /* only used to detect font change */
> -            memory_region_set_dirty(&s->vram, addr);
> +            memory_region_set_dirty(&s->vram, addr, 1);
>          }
>      } else {
>          /* standard VGA latched access */
> @@ -940,7 +940,7 @@ void vga_mem_writeb(VGACommonState *s,
> target_phys_addr_t addr, uint32_t val)
>          printf("vga: latch: [0x" TARGET_FMT_plx "] mask=0x%08x val=0x%08x\n",
>                 addr * 4, write_mask, val);
>  #endif
> -        memory_region_set_dirty(&s->vram, addr << 2);
> +        memory_region_set_dirty(&s->vram, addr << 2, sizeof(uint32_t));
>      }
>  }
>
> diff --git a/hw/vhost.c b/hw/vhost.c
> index 0870cb7..d96b186 100644
> --- a/hw/vhost.c
> +++ b/hw/vhost.c
> @@ -50,7 +50,7 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
>              ram_addr_t ram_addr;
>              bit -= 1;
>              ram_addr = cpu_get_physical_page_desc(addr + bit * VHOST_LOG_PAGE);
> -            cpu_physical_memory_set_dirty(ram_addr);
> +            cpu_physical_memory_range_set_dirty(ram_addr, VHOST_LOG_CHUNK);
>              log &= ~(0x1ull << bit);
>          }

Should be VHOST_LOG_PAGE.

-- 
error compiling committee.c: too many arguments to function