fix bad physical address in kvm_update_dirty_pages_log()
PATCH 1/1 fix bad physical address in kvm_update_dirty_pages_log()
Hi,
Attaching patch that fix updating qemu dirty region. Previous kvm_update_dirty_pages_log() imp treat physical ram as if it is linear mapped to guest physical memory. This patch fix it by mapping physical ram to guest physical memory areas and for etch area call kvm_get_dirty_pages_range() with the correct address and size.
Thanks,
Yaniv
Quoted 1 line
From
1179bb5123a53392d705801dc37e491f0766957cMon Sep 17 00:00:00 2001
From: Yaniv Kamay <[email protected]>
Date: Tue, 24 Mar 2009 20:49:07 +0200
Subject: [PATCH] fix bad physical address in kvm_update_dirty_pages_log()
--- qemu/qemu-kvm.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 57 insertions(+), 4 deletions(-)
qemu/qemu-kvm.c +57 -4
@@ -1216,6 +1216,27 @@ static int kvm_get_dirty_bitmap_cb(unsigned long start, unsigned long len,return kvm_get_dirty_pages_log_range (start, bitmap, start, len); } +static int find_phys_area (ram_addr_t phys_ram_offset,ram_addr_t *offset,target_phys_addr_t *start, +ram_addr_t *size) +{ +struct mapping *now =NULL ; +struct mapping *map; + +for (map = mappings; map < mappings + nr_mappings; ++map) { +if (map->ram >= phys_ram_offset && (!now || map->ram < now->ram)) { + now = map; + } + } + +if (!now) { +return -1 ; + } + + *offset = now->ram - phys_ram_offset; + *start = now->phys; + *size = now->len; +return 0 ; +}/* * get kvm's dirty pages bitmap and update qemu's * we only care about physical ram, which resides in slots 0 and 3
@@ -1223,12 +1244,44 @@ static int kvm_get_dirty_bitmap_cb(unsigned long start, unsigned long len,int kvm_update_dirty_pages_log (void ) {int r =0 ; +ram_addr_t now =0 ; +ram_addr_t end = phys_ram_size; +ram_addr_t offset; +target_phys_addr_t area_start; +ram_addr_t area_size; +unsigned char *dirty_bitmap = kvm_dirty_bitmap; + +if (!dirty_bitmap) { +printf ("%s: no dirty bitmap\n" , __FUNCTION__); +return -1 ; + } +while (now < end && !find_phys_area (now, &offset, &area_start, &area_size)) { +if ((offset & ~TARGET_PAGE_MASK ) || (area_start & ~TARGET_PAGE_MASK ) || + (area_size & ~TARGET_PAGE_MASK )) { +printf ("%s: invalid mem area\n" , __FUNCTION__); +return -1 ; + } - r =kvm_get_dirty_pages_range (kvm_context,0 , phys_ram_size, - kvm_dirty_bitmap,NULL , - kvm_get_dirty_bitmap_cb); -return r; +if ((now += offset) >= end) { +break ; + } + +if (area_size > end - now) { +return -1 ; + } + dirty_bitmap += offset /TARGET_PAGE_SIZE /8 ; +if ((r =kvm_get_dirty_pages_range (kvm_context, area_start, area_size, dirty_bitmap,NULL , + kvm_get_dirty_bitmap_cb))) { +return r; + } + dirty_bitmap += area_size /TARGET_PAGE_SIZE /8 ; + now += area_size; +if (!now) { +break ; + } + } +return 0 ; } void kvm_qemu_log_memory(target_phys_addr_t start, target_phys_addr_t size,
Signature · 1 line
1.6.0.2
@@ -1223,12 +1244,44 @@ static int kvm_get_dirty_bitmap_cb(unsigned long start, unsigned long len,
int kvm_update_dirty_pages_log(void)
{
int r = 0;
+ ram_addr_t now = 0;
+ ram_addr_t end = phys_ram_size;
+ ram_addr_t offset;
+ target_phys_addr_t area_start;
+ ram_addr_t area_size;
+ unsigned char *dirty_bitmap = kvm_dirty_bitmap;+
+ if (!dirty_bitmap) {
+ printf("%s: no dirty bitmap\n", __FUNCTION__);
+ return -1;
+ }
nitpick: you probably want fprintf(stderr...).
+ while (now < end && !find_phys_area(now, &offset, &area_start,&area_size)) {
+ if ((offset & ~TARGET_PAGE_MASK) || (area_start &
~TARGET_PAGE_MASK) ||
+ (area_size &
~TARGET_PAGE_MASK)) {
+ printf("%s: invalid mem area\n", __FUNCTION__);
same here.
+ if ((now += offset) >= end) {
+ break;
+ }
+
+ if (area_size > end - now) {
+ return -1;
+ }
any reason why you're handling those two differently?
--
Glauber Costa.
"Free as in Freedom"
http://glommer.net
"The less confident you are, the more serious you have to act."
Quoted 37 lines · Glauber Costa
qemu/qemu-kvm.c +11
-0@@ -1223,12 +1244,44 @@ static int kvm_get_dirty_bitmap_cb(unsigned long start, unsigned long len, int kvm_update_dirty_pages_log(void) { int r = 0; +ram_addr_t now =0 ; +ram_addr_t end = phys_ram_size; +ram_addr_t offset; +target_phys_addr_t area_start; +ram_addr_t area_size; +unsigned char *dirty_bitmap = kvm_dirty_bitmap; + +if (!dirty_bitmap) { +printf ("%s: no dirty bitmap\n" , __FUNCTION__); +return -1 ; + }nitpick: you probably want fprintf(stderr...).
+ while (now < end && !find_phys_area(now, &offset,
&area_start,&area_size)) {
+ if ((offset & ~TARGET_PAGE_MASK) || (area_start&
~TARGET_PAGE_MASK) ||
+ (area_size&
~TARGET_PAGE_MASK)) {
+printf("%s: invalid mem area\n", __FUNCTION__);
same here.qemu/qemu-kvm.c +7
-0+if ((now += offset) >= end) { +break ; + } + +if (area_size > end - now) { +return -1 ; + }
Quoted 2 lines
any reason why you're handling those two differently?
The first is orderly end of mappings and the later is error.
Quoted 7 lines
--
Glauber Costa.
"Free as in Freedom"
http://glommer.net"The less confident you are, the more serious you have to act."
Quoted 32 lines · Yaniv Kamay
Quoted 31 lines · Glauber Costa
qemu/qemu-kvm.c +11
-0@@ -1223,12 +1244,44 @@ static int kvm_get_dirty_bitmap_cb(unsigned long start, unsigned long len, int kvm_update_dirty_pages_log(void) { int r = 0; + ram_addr_t now = 0; + ram_addr_t end = phys_ram_size; + ram_addr_t offset; + target_phys_addr_t area_start; + ram_addr_t area_size; + unsigned char *dirty_bitmap = kvm_dirty_bitmap; + + if (!dirty_bitmap) { + printf("%s: no dirty bitmap\n", __FUNCTION__); + return -1; + }nitpick: you probably want fprintf(stderr...).
+ while (now < end && !find_phys_area(now, &offset,
&area_start,&area_size)) {
+ if ((offset & ~TARGET_PAGE_MASK) || (area_start& ~TARGET_PAGE_MASK) ||
+ (area_size& ~TARGET_PAGE_MASK)) {
+ printf("%s: invalid mem area\n",__FUNCTION__); same here.
Quoted 11 lines
Quoted 9 lines
+ if ((now += offset) >= end) {
+ break;
+ }
+
+ if (area_size> end - now) {
+ return -1;
+ }
any reason why you're handling those two differently?The first is orderly end of mappings and the later is error.
ok. I believe the code reads better if you handle all the errors together.
but this is nitpick too. The idea of the patch looks correct to me.
Quoted 10 lines
Quoted 8 lines
--
Glauber Costa.
"Free as in Freedom"
http://glommer.net"The less confident you are, the more serious you have to act."
Signature · 5 lines
Glauber Costa. "Free as in Freedom" http://glommer.net "The less confident you are, the more serious you have to act."
Quoted 6 lines · Yaniv Kamay
Hi,
Attaching patch that fix updating qemu dirty region. Previous
kvm_update_dirty_pages_log()imp treat physical ram as if it is linear mapped to guest physical memory. This patch fix it by mapping physical ram to guest physical memory areas and for etch area callkvm_get_dirty_pages_range()with the correct address and size.
kvm_get_dirty_pages_range() already operates on guest physical memory, and kvm_get_dirty_pages_log_range() (which is called by the callback) translates it to ram_addr, so it should work, no? Maybe change the range parameters from (0, phys_ram_size) to (0, ~0UL)?
Signature · 1 line
error compiling committee.c: too many arguments to function