Patchwork [V4,4/5] exec, memory: Call to xen_modified_memory.

login
register
mail settings
Submitter Anthony PERARD
Date Sept. 27, 2012, 11:12 a.m.
Message ID <1348744360-2989-5-git-send-email-anthony.perard@citrix.com>
Download mbox | patch
Permalink /patch/187330/
State New
Headers show

Comments

Anthony PERARD - Sept. 27, 2012, 11:12 a.m.
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   | 1 +
 memory.c | 2 ++
 2 files changed, 3 insertions(+)
Stefano Stabellini - Oct. 1, 2012, 10:36 a.m.
On Thu, 27 Sep 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>

If I am not mistaken, this is the last patch that needs reviewing.
Avi, are you OK with it?



>  exec.c   | 1 +
>  memory.c | 2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/exec.c b/exec.c
> index 366684c..1114a09 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -3427,6 +3427,7 @@ static void invalidate_and_set_dirty(target_phys_addr_t addr,
>          /* set dirty bit */
>          cpu_physical_memory_set_dirty_flags(addr, (0xff & ~CODE_DIRTY_FLAG));
>      }
> +    xen_modified_memory(addr, length);
>  }
>  
>  void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
> diff --git a/memory.c b/memory.c
> index 4f3ade0..015c544 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"
> @@ -1077,6 +1078,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
>
Avi Kivity - Oct. 2, 2012, 10:25 a.m.
On 10/01/2012 12:36 PM, Stefano Stabellini wrote:
> On Thu, 27 Sep 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>
> 
> If I am not mistaken, this is the last patch that needs reviewing.
> Avi, are you OK with it?
> 
> 
> 
>>  exec.c   | 1 +
>>  memory.c | 2 ++
>>  2 files changed, 3 insertions(+)
>> 
>> diff --git a/exec.c b/exec.c
>> index 366684c..1114a09 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -3427,6 +3427,7 @@ static void invalidate_and_set_dirty(target_phys_addr_t addr,
>>          /* set dirty bit */
>>          cpu_physical_memory_set_dirty_flags(addr, (0xff & ~CODE_DIRTY_FLAG));
>>      }
>> +    xen_modified_memory(addr, length);
>>  }
>>  
>>  void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
>> diff --git a/memory.c b/memory.c
>> index 4f3ade0..015c544 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"
>> @@ -1077,6 +1078,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);
>>  }

I would prefer this bit pushed into cpu_physical_set_dirty_range().
Possibly the first bit too?
Anthony PERARD - Oct. 2, 2012, 4:16 p.m.
On 10/02/2012 11:25 AM, Avi Kivity wrote:
> On 10/01/2012 12:36 PM, Stefano Stabellini wrote:
>> On Thu, 27 Sep 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>
>>
>> If I am not mistaken, this is the last patch that needs reviewing.
>> Avi, are you OK with it?
>>
>>
>>
>>>  exec.c   | 1 +
>>>  memory.c | 2 ++
>>>  2 files changed, 3 insertions(+)
>>>
>>> diff --git a/exec.c b/exec.c
>>> index 366684c..1114a09 100644
>>> --- a/exec.c
>>> +++ b/exec.c
>>> @@ -3427,6 +3427,7 @@ static void
invalidate_and_set_dirty(target_phys_addr_t addr,
>>>          /* set dirty bit */
>>>          cpu_physical_memory_set_dirty_flags(addr, (0xff &
~CODE_DIRTY_FLAG));
>>>      }
>>> +    xen_modified_memory(addr, length);
>>>  }
>>>
>>>  void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
>>> diff --git a/memory.c b/memory.c
>>> index 4f3ade0..015c544 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"
>>> @@ -1077,6 +1078,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);
>>>  }
>
> I would prefer this bit pushed into cpu_physical_set_dirty_range().

Ok, I will call the function from cpu_physical_memory_set_dirty_range()
instead of from memory_region_set_dirty.

> Possibly the first bit too?

But the call from invalidate_and_set_dirty, I can not remove it because
it does not work.  The xen function must be called without condition as
xen and qemu does not maintained the same dirtymap.
Avi Kivity - Oct. 2, 2012, 4:20 p.m.
On 10/02/2012 06:16 PM, Anthony PERARD wrote:
> 
>> Possibly the first bit too?
> 
> But the call from invalidate_and_set_dirty, I can not remove it because
> it does not work.  The xen function must be called without condition as
> xen and qemu does not maintained the same dirtymap.

Right, in fact we added invalidate_and_set_dirty() in order to stick the
xen call in it, I just forgot about it.  So the first bit is okay.

Patch

diff --git a/exec.c b/exec.c
index 366684c..1114a09 100644
--- a/exec.c
+++ b/exec.c
@@ -3427,6 +3427,7 @@  static void invalidate_and_set_dirty(target_phys_addr_t addr,
         /* set dirty bit */
         cpu_physical_memory_set_dirty_flags(addr, (0xff & ~CODE_DIRTY_FLAG));
     }
+    xen_modified_memory(addr, length);
 }
 
 void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
diff --git a/memory.c b/memory.c
index 4f3ade0..015c544 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"
@@ -1077,6 +1078,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);
 }