Re: [Qemu-devel] [PATCH 11/38] memory: cpu_physical_memory_mask_dirty_range() always clears a single flag

[Orit Wasserman <owasserm@redhat.com>]

On 12/17/2013 05:25 PM, Juan Quintela wrote:
> Document it
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>   cputlb.c                       |  4 ++--
>   exec.c                         | 19 ++++++++++---------
>   include/exec/memory-internal.h | 40 ++++++++++++++++++----------------------
>   include/exec/memory.h          |  3 ---
>   memory.c                       | 10 ++++------
>   5 files changed, 34 insertions(+), 42 deletions(-)
>
> diff --git a/cputlb.c b/cputlb.c
> index 72452e5..dfd747a 100644
> --- a/cputlb.c
> +++ b/cputlb.c
> @@ -129,7 +129,7 @@ void tlb_protect_code(ram_addr_t ram_addr)
>   {
>       cpu_physical_memory_reset_dirty(ram_addr,
>                                       ram_addr + TARGET_PAGE_SIZE,
> -                                    CODE_DIRTY_FLAG);
> +                                    DIRTY_MEMORY_CODE);
>   }
>
>   /* update the TLB so that writes in physical page 'phys_addr' are no longer
> @@ -137,7 +137,7 @@ void tlb_protect_code(ram_addr_t ram_addr)
>   void tlb_unprotect_code_phys(CPUArchState *env, ram_addr_t ram_addr,
>                                target_ulong vaddr)
>   {
> -    cpu_physical_memory_set_dirty_flag(ram_addr, CODE_DIRTY_FLAG);
> +    cpu_physical_memory_set_dirty_flag(ram_addr, DIRTY_MEMORY_CODE);
>   }
>
>   static bool tlb_is_dirty_ram(CPUTLBEntry *tlbe)
> diff --git a/exec.c b/exec.c
> index 45fdb84..9996da2 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -737,7 +737,7 @@ 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,
> -                                     int dirty_flags)
> +                                     unsigned client)
>   {
>       uintptr_t length;
>
> @@ -747,7 +747,7 @@ void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end,
>       length = end - start;
>       if (length == 0)
>           return;
> -    cpu_physical_memory_mask_dirty_range(start, length, dirty_flags);
> +    cpu_physical_memory_mask_dirty_range(start, length, client);
>
>       if (tcg_enabled()) {
>           tlb_reset_dirty_range_all(start, end, length);
> @@ -1484,7 +1484,7 @@ found:
>   static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
>                                  uint64_t val, unsigned size)
>   {
> -    if (!cpu_physical_memory_get_dirty_flag(ram_addr, CODE_DIRTY_FLAG)) {
> +    if (!cpu_physical_memory_get_dirty_flag(ram_addr, DIRTY_MEMORY_CODE)) {
>           tb_invalidate_phys_page_fast(ram_addr, size);
>       }
>       switch (size) {
> @@ -1500,8 +1500,8 @@ static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
>       default:
>           abort();
>       }
> -    cpu_physical_memory_set_dirty_flag(ram_addr, MIGRATION_DIRTY_FLAG);
> -    cpu_physical_memory_set_dirty_flag(ram_addr, VGA_DIRTY_FLAG);
> +    cpu_physical_memory_set_dirty_flag(ram_addr, DIRTY_MEMORY_MIGRATION);
> +    cpu_physical_memory_set_dirty_flag(ram_addr, DIRTY_MEMORY_VGA);
>       /* we remove the notdirty callback only if the code has been
>          flushed */
>       if (cpu_physical_memory_is_dirty(ram_addr)) {
> @@ -1911,8 +1911,8 @@ static void invalidate_and_set_dirty(hwaddr addr,
>           /* invalidate code */
>           tb_invalidate_phys_page_range(addr, addr + length, 0);
>           /* set dirty bit */
> -        cpu_physical_memory_set_dirty_flag(addr, VGA_DIRTY_FLAG);
> -        cpu_physical_memory_set_dirty_flag(addr, MIGRATION_DIRTY_FLAG);
> +        cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_VGA);
> +        cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_MIGRATION);
>       }
>       xen_modified_memory(addr, length);
>   }
> @@ -2494,8 +2494,9 @@ void stl_phys_notdirty(hwaddr addr, uint32_t val)
>                   /* invalidate code */
>                   tb_invalidate_phys_page_range(addr1, addr1 + 4, 0);
>                   /* set dirty bit */
> -                cpu_physical_memory_set_dirty_flag(addr1, MIGRATION_DIRTY_FLAG);
> -                cpu_physical_memory_set_dirty_flag(addr1, VGA_DIRTY_FLAG);
> +                cpu_physical_memory_set_dirty_flag(addr1,
> +                                                   DIRTY_MEMORY_MIGRATION);
> +                cpu_physical_memory_set_dirty_flag(addr1, DIRTY_MEMORY_VGA);
>               }
>           }
>       }
> diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
> index 681d63b..b58010f 100644
> --- a/include/exec/memory-internal.h
> +++ b/include/exec/memory-internal.h
> @@ -40,29 +40,25 @@ void *qemu_get_ram_ptr(ram_addr_t addr);
>   void qemu_ram_free(ram_addr_t addr);
>   void qemu_ram_free_from_ptr(ram_addr_t addr);
>
> -#define VGA_DIRTY_FLAG       0x01
> -#define CODE_DIRTY_FLAG      0x02
> -#define MIGRATION_DIRTY_FLAG 0x08
> -
>   static inline bool cpu_physical_memory_get_dirty_flag(ram_addr_t addr,
> -                                                      int dirty_flag)
> +                                                      unsigned client)
>   {
> -    return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] & dirty_flag;
> +    return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] & (1 << client);
>   }
>
>   /* read dirty bit (return 0 or 1) */
>   static inline bool cpu_physical_memory_is_dirty(ram_addr_t addr)
>   {
> -    bool vga = cpu_physical_memory_get_dirty_flag(addr, VGA_DIRTY_FLAG);
> -    bool code = cpu_physical_memory_get_dirty_flag(addr, CODE_DIRTY_FLAG);
> +    bool vga = cpu_physical_memory_get_dirty_flag(addr, DIRTY_MEMORY_VGA);
> +    bool code = cpu_physical_memory_get_dirty_flag(addr, DIRTY_MEMORY_CODE);
>       bool migration =
> -        cpu_physical_memory_get_dirty_flag(addr, MIGRATION_DIRTY_FLAG);
> +        cpu_physical_memory_get_dirty_flag(addr, DIRTY_MEMORY_MIGRATION);
>       return vga && code && migration;
>   }
>
>   static inline int cpu_physical_memory_get_dirty(ram_addr_t start,
>                                                   ram_addr_t length,
> -                                                int dirty_flag)
> +                                                unsigned client)
>   {
>       int ret = 0;
>       ram_addr_t addr, end;
> @@ -70,28 +66,28 @@ static inline int cpu_physical_memory_get_dirty(ram_addr_t start,
>       end = TARGET_PAGE_ALIGN(start + length);
>       start &= TARGET_PAGE_MASK;
>       for (addr = start; addr < end; addr += TARGET_PAGE_SIZE) {
> -        ret |= cpu_physical_memory_get_dirty_flag(addr, dirty_flag);
> +        ret |= cpu_physical_memory_get_dirty_flag(addr, client);
>       }
>       return ret;
>   }
>
>   static inline void cpu_physical_memory_set_dirty_flag(ram_addr_t addr,
> -                                                      int dirty_flag)
> +                                                      unsigned client)
>   {
> -    ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] |= dirty_flag;
> +    ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] |= (1 << client);
>   }
>
>   static inline void cpu_physical_memory_set_dirty(ram_addr_t addr)
>   {
> -    cpu_physical_memory_set_dirty_flag(addr, MIGRATION_DIRTY_FLAG);
> -    cpu_physical_memory_set_dirty_flag(addr, VGA_DIRTY_FLAG);
> -    cpu_physical_memory_set_dirty_flag(addr, CODE_DIRTY_FLAG);
> +    cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_MIGRATION);
> +    cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_VGA);
> +    cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_CODE);
>   }
>
> -static inline int cpu_physical_memory_clear_dirty_flags(ram_addr_t addr,
> -                                                        int dirty_flags)
> +static inline int cpu_physical_memory_clear_dirty_flag(ram_addr_t addr,
> +                                                       unsigned client)
>   {
> -    int mask = ~dirty_flags;
> +    int mask = ~(1 << client);
>
>       return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] &= mask;
>   }
> @@ -111,19 +107,19 @@ static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start,
>
>   static inline void cpu_physical_memory_mask_dirty_range(ram_addr_t start,
>                                                           ram_addr_t length,
> -                                                        int dirty_flags)
> +                                                        unsigned client)
>   {
>       ram_addr_t addr, end;
>
>       end = TARGET_PAGE_ALIGN(start + length);
>       start &= TARGET_PAGE_MASK;
>       for (addr = start; addr < end; addr += TARGET_PAGE_SIZE) {
> -        cpu_physical_memory_clear_dirty_flags(addr, dirty_flags);
> +        cpu_physical_memory_clear_dirty_flag(addr, client);
>       }
>   }
>
>   void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end,
> -                                     int dirty_flags);
> +                                     unsigned client);
>
>   #endif
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 480dfbf..b8e76f4 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -33,9 +33,6 @@
>   typedef struct MemoryRegionOps MemoryRegionOps;
>   typedef struct MemoryRegionMmio MemoryRegionMmio;
>
> -/* Must match *_DIRTY_FLAGS in cpu-all.h.  To be replaced with dynamic
> - * registration.
> - */
>   #define DIRTY_MEMORY_VGA       0
>   #define DIRTY_MEMORY_CODE      1
>   #define DIRTY_MEMORY_MIGRATION 3
> diff --git a/memory.c b/memory.c
> index fb52e1c..a490cbd 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1174,8 +1174,7 @@ bool memory_region_get_dirty(MemoryRegion *mr, hwaddr addr,
>                                hwaddr size, unsigned client)
>   {
>       assert(mr->terminates);
> -    return cpu_physical_memory_get_dirty(mr->ram_addr + addr, size,
> -                                         1 << client);
> +    return cpu_physical_memory_get_dirty(mr->ram_addr + addr, size, client);
>   }
>
>   void memory_region_set_dirty(MemoryRegion *mr, hwaddr addr,
> @@ -1190,12 +1189,11 @@ bool memory_region_test_and_clear_dirty(MemoryRegion *mr, hwaddr addr,
>   {
>       bool ret;
>       assert(mr->terminates);
> -    ret = cpu_physical_memory_get_dirty(mr->ram_addr + addr, size,
> -                                        1 << client);
> +    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,
> -                                        1 << client);
> +                                        client);
>       }
>       return ret;
>   }
> @@ -1243,7 +1241,7 @@ void memory_region_reset_dirty(MemoryRegion *mr, hwaddr addr,
>       assert(mr->terminates);
>       cpu_physical_memory_reset_dirty(mr->ram_addr + addr,
>                                       mr->ram_addr + addr + size,
> -                                    1 << client);
> +                                    client);
>   }
>
>   void *memory_region_get_ram_ptr(MemoryRegion *mr)
>

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