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

Anthony Liguori
2008-09-10T14:56:56Z

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