Re: [Qemu-devel] [PATCH 28/38] memory: make cpu_physical_memory_reset_dirty() take a length parameter

[Orit Wasserman <owasserm@redhat.com>]

On 12/17/2013 05:26 PM, Juan Quintela wrote:
> We have an end parameter in all the callers, and this make it coherent
> with the rest of cpu_physical_memory_* functions, that also take a
> length parameter.
>
> Once here, move the start/end calculation to
> tlb_reset_dirty_range_all() as we don't need it here anymore.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>   cputlb.c                       |  3 +--
>   exec.c                         | 19 ++++++++-----------
>   include/exec/memory-internal.h |  2 +-
>   memory.c                       |  8 ++------
>   4 files changed, 12 insertions(+), 20 deletions(-)
>
> diff --git a/cputlb.c b/cputlb.c
> index 911d764..a5805e1 100644
> --- a/cputlb.c
> +++ b/cputlb.c
> @@ -127,8 +127,7 @@ void tlb_flush_page(CPUArchState *env, target_ulong addr)
>      can be detected */
>   void tlb_protect_code(ram_addr_t ram_addr)
>   {
> -    cpu_physical_memory_reset_dirty(ram_addr,
> -                                    ram_addr + TARGET_PAGE_SIZE,
> +    cpu_physical_memory_reset_dirty(ram_addr, TARGET_PAGE_SIZE,
>                                       DIRTY_MEMORY_CODE);
>   }
>
> diff --git a/exec.c b/exec.c
> index a2f89eb..ba5989c 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -723,11 +723,14 @@ found:
>       return block;
>   }
>
> -static void tlb_reset_dirty_range_all(ram_addr_t start, ram_addr_t end,
> -                                      uintptr_t length)
> +static void tlb_reset_dirty_range_all(ram_addr_t start, ram_addr_t length)
>   {
> -    RAMBlock *block;
>       ram_addr_t start1;
> +    RAMBlock *block;
> +    ram_addr_t end;
> +
> +    end = TARGET_PAGE_ALIGN(start + length);
> +    start &= TARGET_PAGE_MASK;
>
>       block = qemu_get_ram_block(start);
>       assert(block == qemu_get_ram_block(end - 1));
> @@ -736,21 +739,15 @@ static void tlb_reset_dirty_range_all(ram_addr_t start, ram_addr_t end,
>   }
>
>   /* Note: start and end must be within the same ram block.  */
> -void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end,
> +void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t length,
>                                        unsigned client)
>   {
> -    uintptr_t length;
> -
> -    start &= TARGET_PAGE_MASK;
> -    end = TARGET_PAGE_ALIGN(end);
> -
> -    length = end - start;
>       if (length == 0)
>           return;
>       cpu_physical_memory_clear_dirty_range(start, length, client);
>
>       if (tcg_enabled()) {
> -        tlb_reset_dirty_range_all(start, end, length);
> +        tlb_reset_dirty_range_all(start, length);
>       }
>   }
>
> diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
> index 771b23f..cb2249f 100644
> --- a/include/exec/memory-internal.h
> +++ b/include/exec/memory-internal.h
> @@ -102,7 +102,7 @@ static inline void cpu_physical_memory_clear_dirty_range(ram_addr_t start,
>       bitmap_clear(ram_list.dirty_memory[client], page, end - page);
>   }
>
> -void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end,
> +void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t length,
>                                        unsigned client);
>
>   #endif
> diff --git a/memory.c b/memory.c
> index a490cbd..c010296 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1191,9 +1191,7 @@ bool memory_region_test_and_clear_dirty(MemoryRegion *mr, hwaddr addr,
>       assert(mr->terminates);
>       ret = cpu_physical_memory_get_dirty(mr->ram_addr + addr, size, client);
>       if (ret) {
> -        cpu_physical_memory_reset_dirty(mr->ram_addr + addr,
> -                                        mr->ram_addr + addr + size,
> -                                        client);
> +        cpu_physical_memory_reset_dirty(mr->ram_addr + addr, size, client);
>       }
>       return ret;
>   }
> @@ -1239,9 +1237,7 @@ void memory_region_reset_dirty(MemoryRegion *mr, hwaddr addr,
>                                  hwaddr size, unsigned client)
>   {
>       assert(mr->terminates);
> -    cpu_physical_memory_reset_dirty(mr->ram_addr + addr,
> -                                    mr->ram_addr + addr + size,
> -                                    client);
> +    cpu_physical_memory_reset_dirty(mr->ram_addr + addr, size, client);
>   }
>
>   void *memory_region_get_ram_ptr(MemoryRegion *mr)
>

Reviewed-by: Orit Wasserman <owasserm@redhat.com>