From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KdR7o-0000Sr-Sk for qemu-devel@nongnu.org; Wed, 10 Sep 2008 10:56:56 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KdR7o-0000SG-0A for qemu-devel@nongnu.org; Wed, 10 Sep 2008 10:56:56 -0400 Received: from [199.232.76.173] (port=54877 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KdR7n-0000S5-Lb for qemu-devel@nongnu.org; Wed, 10 Sep 2008 10:56:55 -0400 Received: from mail-gx0-f19.google.com ([209.85.217.19]:46009) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KdR7n-0006Z8-HT for qemu-devel@nongnu.org; Wed, 10 Sep 2008 10:56:55 -0400 Received: by gxk12 with SMTP id 12so13622002gxk.10 for ; Wed, 10 Sep 2008 07:56:53 -0700 (PDT) Message-ID: <48C7E000.8000903@codemonkey.ws> Date: Wed, 10 Sep 2008 09:56:00 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 4/10] Add dirty tracking for live migration References: <1220989802-13706-1-git-send-email-aliguori@us.ibm.com> <1220989802-13706-5-git-send-email-aliguori@us.ibm.com> <5d6222a80809100752t75a68593p86cebe5d490bdca@mail.gmail.com> In-Reply-To: <5d6222a80809100752t75a68593p86cebe5d490bdca@mail.gmail.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Glauber Costa Cc: Chris Wright , Uri Lublin , Anthony Liguori , qemu-devel@nongnu.org, kvm@vger.kernel.org Glauber Costa wrote: > On Tue, Sep 9, 2008 at 4:49 PM, Anthony Liguori 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 >> >> 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 >> #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