Re: [Qemu-devel] [PATCH 4/10] Add dirty tracking for live migration

[Anthony Liguori <anthony@codemonkey.ws>]

Glauber Costa wrote:
> On Tue, Sep 9, 2008 at 4:49 PM, Anthony Liguori <aliguori@us.ibm.com> wrote:
>   
>> This patch adds a dirty tracking bit for live migration.  We use 0x08 because
>> kqemu uses 0x04.
>>     
>
> For which purpose, and where is it? I think it deserves at least a
> comment on the source itself for future generations.
>   

This patch was originally written before kqemu was Open Sourced.  I only 
knew that kqemu used 0x08 because Fabrice mentioned it in reviewing the 
earliest version of this series.

I'll add a comment to the code.

>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>>
>> diff --git a/cpu-all.h b/cpu-all.h
>> index d350b30..fdac353 100644
>> --- a/cpu-all.h
>> +++ b/cpu-all.h
>> @@ -944,6 +944,7 @@ int cpu_memory_rw_debug(CPUState *env, target_ulong addr,
>>
>>  #define VGA_DIRTY_FLAG  0x01
>>  #define CODE_DIRTY_FLAG 0x02
>> +#define MIGRATION_DIRTY_FLAG 0x08
>>
>>  /* read dirty bit (return 0 or 1) */
>>  static inline int cpu_physical_memory_is_dirty(ram_addr_t addr)
>> @@ -966,6 +967,10 @@ void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end,
>>                                      int dirty_flags);
>>  void cpu_tlb_update_dirty(CPUState *env);
>>
>> +int cpu_physical_memory_set_dirty_tracking(int enable);
>> +
>> +int cpu_physical_memory_get_dirty_tracking(void);
>> +
>>  void dump_exec_info(FILE *f,
>>                     int (*cpu_fprintf)(FILE *f, const char *fmt, ...));
>>
>> diff --git a/exec.c b/exec.c
>> index 3ab4ad0..9dba5c8 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -38,6 +38,7 @@
>>  #include "qemu-common.h"
>>  #include "tcg.h"
>>  #include "hw/hw.h"
>> +#include "osdep.h"
>>  #if defined(CONFIG_USER_ONLY)
>>  #include <qemu.h>
>>  #endif
>> @@ -113,6 +114,7 @@ ram_addr_t phys_ram_size;
>>  int phys_ram_fd;
>>  uint8_t *phys_ram_base;
>>  uint8_t *phys_ram_dirty;
>> +static int in_migration;
>>  static ram_addr_t phys_ram_alloc_offset = 0;
>>  #endif
>>
>> @@ -1777,6 +1779,17 @@ void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end,
>>     }
>>  }
>>
>> +int cpu_physical_memory_set_dirty_tracking(int enable)
>> +{
>> +    in_migration = enable;
>> +    return 0;
>> +}
>> +
>> +int cpu_physical_memory_get_dirty_tracking(void)
>> +{
>> +    return in_migration;
>> +}
>> +
>>  static inline void tlb_update_dirty(CPUTLBEntry *tlb_entry)
>>  {
>>     ram_addr_t ram_addr;
>> @@ -2932,9 +2945,19 @@ void stl_phys_notdirty(target_phys_addr_t addr, uint32_t val)
>>         io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
>>         io_mem_write[io_index][2](io_mem_opaque[io_index], addr, val);
>>     } else {
>> -        ptr = phys_ram_base + (pd & TARGET_PAGE_MASK) +
>> -            (addr & ~TARGET_PAGE_MASK);
>> +        unsigned long addr1 = (pd & TARGET_PAGE_MASK) + (addr & ~TARGET_PAGE_MASK);
>> +        ptr = phys_ram_base + addr1;
>>         stl_p(ptr, val);
>> +
>> +        if (unlikely(in_migration)) {
>> +            if (!cpu_physical_memory_is_dirty(addr1)) {
>> +                /* invalidate code */
>> +                tb_invalidate_phys_page_range(addr1, addr1 + 4, 0);
>> +                /* set dirty bit */
>> +                phys_ram_dirty[addr1 >> TARGET_PAGE_BITS] |=
>> +                    (0xff & ~CODE_DIRTY_FLAG);
>> +            }
>> +        }
>>     }
>>  }
>>     
> did you mean MIGRATION_DIRTY_FLAG?
>   

No.  We want to set all of the dirty bits except for the 
CODE_DIRTY_FLAG.  If you look around the rest of the code, it's pretty 
much the standard thing to do.  Self-modifying code has to be handled 
specially.

Regards,

Anthony Liguori