Re: [PATCH] qemu-xen-traditionnal, Fix dirty logging during migration.

[Anthony PERARD <anthony.perard@citrix.com>]

On 23/07/12 11:42, Stefano Stabellini wrote:
> On Fri, 20 Jul 2012, Anthony PERARD wrote:
>> This moves the xen_modified_memory call from cpu_physical_memory_map to
>> cpu_physical_memory_unmap because the memory could be migrated before the
>> device model have written to it.
>>
>> The xen_ram_addr_from_mapcache function is imported from QEMU.
>>
>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>>
>> ---
>>   hw/xen_machine_fv.c |   39 +++++++++++++++++++++++++++++++++++++++
>>   i386-dm/exec-dm.c   |   10 ++++++----
>>   qemu-xen.h          |    1 +
>>   3 files changed, 46 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/xen_machine_fv.c b/hw/xen_machine_fv.c
>> index fdad42a..c358944 100644
>> --- a/hw/xen_machine_fv.c
>> +++ b/hw/xen_machine_fv.c
>> @@ -223,6 +223,45 @@ void qemu_invalidate_entry(uint8_t *buffer)
>>       qemu_free(entry);
>>   }
>>
>> +target_phys_addr_t xen_ram_addr_from_mapcache(void *ptr)
>> +{
>> +    struct map_cache *entry = NULL;
>> +    struct map_cache_rev *reventry;
>> +    target_phys_addr_t paddr_index;
>> +    int found = 0;
>> +
>> +    TAILQ_FOREACH(reventry, &locked_entries, next) {
>> +        if (reventry->vaddr_req == ptr) {
>> +            paddr_index = reventry->paddr_index;
>> +            found = 1;
>> +            break;
>> +        }
>> +    }
>> +    if (!found) {
>> +        fprintf(stderr, "%s, could not find %p\n", __func__, ptr);
>> +        TAILQ_FOREACH(reventry, &locked_entries, next) {
>> +            fprintf(stderr, "   %#10lx -> %p is present\n",
>> +                    reventry->paddr_index,
>> +                    reventry->vaddr_req);
>> +        }
>> +        abort();
>> +        return 0;
>> +    }
>> +
>> +    entry = &mapcache_entry[paddr_index % nr_buckets];
>> +    while (entry && (entry->paddr_index != paddr_index)) {
>> +        entry = entry->next;
>> +    }
>> +    if (!entry) {
>> +        fprintf(stderr,
>> +                "Trying to find address %p that is not in the mapcache!\n",
>> +                ptr);
>> +        return 0;
>> +    }
>> +    return (reventry->paddr_index << MCACHE_BUCKET_SHIFT) +
>> +        ((unsigned long) ptr - (unsigned long) entry->vaddr_base);
>> +}
>> +
>>   void qemu_invalidate_map_cache(void)
>>   {
>>       unsigned long i;
>> diff --git a/i386-dm/exec-dm.c b/i386-dm/exec-dm.c
>> index 96274d9..bf27a6a 100644
>> --- a/i386-dm/exec-dm.c
>> +++ b/i386-dm/exec-dm.c
>> @@ -820,10 +820,6 @@ void *cpu_physical_memory_map(target_phys_addr_t addr,
>>       if ((*plen) > l)
>>           *plen = l;
>>   #endif
>> -    if (xen_logdirty_enable)
>> -        xc_hvm_modified_memory(xc_handle, domid, addr >> TARGET_PAGE_BITS,
>> -                ((addr + l + TARGET_PAGE_SIZE - 1) >> TARGET_PAGE_BITS)
>> -                    - (addr >> TARGET_PAGE_BITS));
>>
>>       return qemu_map_cache(addr, 1);
>>   }
>> @@ -835,6 +831,12 @@ void *cpu_physical_memory_map(target_phys_addr_t addr,
>>   void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len,
>>                                  int is_write, target_phys_addr_t access_len)
>>   {
>> +    if (xen_logdirty_enable && is_write) {
>> +        target_phys_addr_t addr = xen_ram_addr_from_mapcache(buffer);
>> +        xc_hvm_modified_memory(xc_handle, domid, addr >> TARGET_PAGE_BITS,
>> +            ((addr + access_len + TARGET_PAGE_SIZE - 1) >> TARGET_PAGE_BITS)
>> +                - (addr >> TARGET_PAGE_BITS));
>> +    }
>>       qemu_invalidate_entry(buffer);
>>       cpu_notify_map_clients();
>>   }
>
> Considering that on qemu-xen-traditional there is just one caller of
> qemu_invalidate_entry, that is cpu_physical_memory_unmap, we can just
> add the xc_hvm_modified_memory call there and avoid introducing
> xen_ram_addr_from_mapcache.
>

OK, that would simplifies things.

-- 
Anthony PERARD