Re: [Qemu-devel] [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Stefano Stabellini <stefano.stabellini@eu.citrix.com>]

On Thu, 19 Jul 2012, Paolo Bonzini wrote:
> Il 17/07/2012 20:06, Stefano Stabellini ha scritto:
> > On Tue, 17 Jul 2012, Anthony PERARD wrote:
> >> This patch add some calls to xen_modified_memory to notify Xen about dirtybits
> >> during migration.
> >>
> >> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> >> ---
> >>  exec.c   |    4 ++++
> >>  memory.c |    2 ++
> >>  2 files changed, 6 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/exec.c b/exec.c
> >> index c9fa17d..9f7a4f7 100644
> >> --- a/exec.c
> >> +++ b/exec.c
> >> @@ -3438,6 +3438,7 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
> >>                      cpu_physical_memory_set_dirty_flags(
> >>                          addr1, (0xff & ~CODE_DIRTY_FLAG));
> >>                  }
> >> +                xen_modified_memory(addr1, TARGET_PAGE_SIZE);
> >>                  qemu_put_ram_ptr(ptr);
> >>              }
> >>          } else {
> >> @@ -3623,6 +3624,7 @@ void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len,
> >>      if (buffer != bounce.buffer) {
> >>          if (is_write) {
> >>              ram_addr_t addr1 = qemu_ram_addr_from_host_nofail(buffer);
> >> +            xen_modified_memory(addr1, access_len);
> >>              while (access_len) {
> >>                  unsigned l;
> >>                  l = TARGET_PAGE_SIZE;
> > 
> > You need to add xen_modified_memory in cpu_physical_memory_map, rather
> > than cpu_physical_memory_unmap.
> 
> No, adding it to map is wrong, because the RAM save routine can migrate
> (and mark as non-dirty) the read buffers _before_ the device models have
> written to them.

You are correct, in fact this looks like a bug in the current qemu-xen
(non-upstream) codebase too!

What I think that we should do is only mark the memory as dirty
if(is_write) in cpu_physical_memory_unmap, like you are doing in this
patch.

Anthony, can you write a patch to change the behavior in
qemu-xen-traditional too?