Xen devel - Jul 2012 - [PATCH 3/4] exec, memory: Call to xen_modified_memory.

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;
@@ -3947,6 +3949,7 @@ static inline void stl_phys_internal(target_phys_addr_t
addr, uint32_t val,
             cpu_physical_memory_set_dirty_flags(addr1,
                 (0xff & ~CODE_DIRTY_FLAG));
         }
+        xen_modified_memory(addr1, 4);
     }
 }
 
@@ -4020,6 +4023,7 @@ static inline void stw_phys_internal(target_phys_addr_t
addr, uint32_t val,
             cpu_physical_memory_set_dirty_flags(addr1,
                 (0xff & ~CODE_DIRTY_FLAG));
         }
+        xen_modified_memory(addr1, 2);
     }
 }
 
diff --git a/memory.c b/memory.c
index aab4a31..4d004e2 100644
--- a/memory.c
+++ b/memory.c
@@ -19,6 +19,7 @@
 #include "bitops.h"
 #include "kvm.h"
 #include <assert.h>
+#include "hw/xen.h"
 
 #define WANT_EXEC_OBSOLETE
 #include "exec-obsolete.h"
@@ -1085,6 +1086,7 @@ void memory_region_set_dirty(MemoryRegion *mr,
target_phys_addr_t addr,
                              target_phys_addr_t size)
 {
     assert(mr->terminates);
+    xen_modified_memory(mr->ram_addr + addr, size);
     return cpu_physical_memory_set_dirty_range(mr->ram_addr + addr, size,
-1);
 }
 
-- 
Anthony PERARD

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.

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.

Paolo

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?