* [PATCH] vfio/migration: fix dirty pages lost bug for ram beyond 3G
@ 2020-03-06 9:41 Yan Zhao
2020-03-25 18:19 ` Alex Williamson
0 siblings, 1 reply; 3+ messages in thread
From: Yan Zhao @ 2020-03-06 9:41 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Tian, Yan Zhao, alex.williamson, intel-gvt-dev, kwankhede
the start address passing to
cpu_physical_memory_set_dirty_range() and
cpu_physical_memory_set_dirty_lebitmap() is the address within the
ram block plus ram block offset.
it's safe to set this start address to gpa if total memory is less than
3G, as ram block offset for pc.ram is 0. But if memory size is beyond
3G, gpa is not equal to the offset in its ram block.
e.g.
for gpa 0x100000000, its offset is actually 0xc0000000.
and for gpa 0x42f500000, its offset should be 0x3EF500000.
This fix is based on Kirti's VFIO live migration patch set v8.
https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg05542.html
(for PATCH v8 11/13
https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg05553.html
and PATCH v8 12/13
https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg05554.html
}
The idea behind it should also be applied to other VFIO live migraiton
patch series below:
https://lists.gnu.org/archive/html/qemu-devel/2019-11/msg01763.html
(Kirti v9)
https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg04912.html
(Yan)
https://lists.gnu.org/archive/html/qemu-devel/2018-04/msg01138.html
(Yulei RFC v4)
https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg05568.html
(Yulei RFC)
Cc: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
---
hw/vfio/common.c | 5 ++++-
hw/vfio/migration.c | 8 +++++---
include/hw/vfio/vfio-common.h | 3 ++-
3 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 6f36b02..ba1a8ef 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -799,6 +799,7 @@ static void vfio_listerner_log_sync(MemoryListener *listener,
MemoryRegionSection *section)
{
uint64_t start_addr, size, pfn_count;
+ uint64_t block_start;
VFIOGroup *group;
VFIODevice *vbasedev;
@@ -819,11 +820,13 @@ static void vfio_listerner_log_sync(MemoryListener *listener,
start_addr = TARGET_PAGE_ALIGN(section->offset_within_address_space);
size = int128_get64(section->size);
pfn_count = size >> TARGET_PAGE_BITS;
+ block_start = TARGET_PAGE_ALIGN(section->offset_within_region +
+ memory_region_get_ram_addr(section->mr));
QLIST_FOREACH(group, &vfio_group_list, next) {
QLIST_FOREACH(vbasedev, &group->device_list, next) {
vfio_get_dirty_page_list(vbasedev, start_addr >> TARGET_PAGE_BITS,
- pfn_count, TARGET_PAGE_SIZE);
+ pfn_count, TARGET_PAGE_SIZE, block_start);
}
}
}
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 640bea1..a19b957 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -279,7 +279,8 @@ static int vfio_load_device_config_state(QEMUFile *f, void *opaque)
void vfio_get_dirty_page_list(VFIODevice *vbasedev,
uint64_t start_pfn,
uint64_t pfn_count,
- uint64_t page_size)
+ uint64_t page_size,
+ uint64_t block_start)
{
VFIOMigration *migration = vbasedev->migration;
VFIORegion *region = &migration->region;
@@ -293,6 +294,7 @@ void vfio_get_dirty_page_list(VFIODevice *vbasedev,
while (total_pfns > 0) {
uint64_t bitmap_size, data_offset = 0;
uint64_t start = start_pfn + count;
+ uint64_t block_start_seg = block_start + count * page_size;
void *buf = NULL;
bool buffer_mmaped = false;
@@ -341,7 +343,7 @@ void vfio_get_dirty_page_list(VFIODevice *vbasedev,
break;
} else if (copied_pfns == VFIO_DEVICE_DIRTY_PFNS_ALL) {
/* Mark all pages dirty for this range */
- cpu_physical_memory_set_dirty_range(start * page_size,
+ cpu_physical_memory_set_dirty_range(block_start_seg,
total_pfns * page_size,
DIRTY_MEMORY_MIGRATION);
break;
@@ -382,7 +384,7 @@ void vfio_get_dirty_page_list(VFIODevice *vbasedev,
}
cpu_physical_memory_set_dirty_lebitmap((unsigned long *)buf,
- start * page_size,
+ block_start_seg,
copied_pfns);
count += copied_pfns;
total_pfns -= copied_pfns;
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 41ff5eb..6d868fa 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -220,6 +220,7 @@ int vfio_spapr_remove_window(VFIOContainer *container,
int vfio_migration_probe(VFIODevice *vbasedev, Error **errp);
void vfio_migration_finalize(VFIODevice *vbasedev);
void vfio_get_dirty_page_list(VFIODevice *vbasedev, uint64_t start_pfn,
- uint64_t pfn_count, uint64_t page_size);
+ uint64_t pfn_count, uint64_t page_size,
+ uint64_t block_start);
#endif /* HW_VFIO_VFIO_COMMON_H */
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] vfio/migration: fix dirty pages lost bug for ram beyond 3G
2020-03-06 9:41 [PATCH] vfio/migration: fix dirty pages lost bug for ram beyond 3G Yan Zhao
@ 2020-03-25 18:19 ` Alex Williamson
2020-03-26 20:11 ` Kirti Wankhede
0 siblings, 1 reply; 3+ messages in thread
From: Alex Williamson @ 2020-03-25 18:19 UTC (permalink / raw)
To: Yan Zhao, kwankhede; +Cc: Kevin Tian, intel-gvt-dev, qemu-devel
On Fri, 6 Mar 2020 17:41:29 +0800
Yan Zhao <yan.y.zhao@intel.com> wrote:
> the start address passing to
> cpu_physical_memory_set_dirty_range() and
> cpu_physical_memory_set_dirty_lebitmap() is the address within the
> ram block plus ram block offset.
>
> it's safe to set this start address to gpa if total memory is less than
> 3G, as ram block offset for pc.ram is 0. But if memory size is beyond
> 3G, gpa is not equal to the offset in its ram block.
>
> e.g.
> for gpa 0x100000000, its offset is actually 0xc0000000.
> and for gpa 0x42f500000, its offset should be 0x3EF500000.
>
> This fix is based on Kirti's VFIO live migration patch set v8.
> https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg05542.html
> (for PATCH v8 11/13
> https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg05553.html
> and PATCH v8 12/13
> https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg05554.html
> }
>
> The idea behind it should also be applied to other VFIO live migraiton
> patch series below:
> https://lists.gnu.org/archive/html/qemu-devel/2019-11/msg01763.html
> (Kirti v9)
> https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg04912.html
> (Yan)
> https://lists.gnu.org/archive/html/qemu-devel/2018-04/msg01138.html
> (Yulei RFC v4)
> https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg05568.html
> (Yulei RFC)
>
> Cc: Kevin Tian <kevin.tian@intel.com>
> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> ---
> hw/vfio/common.c | 5 ++++-
> hw/vfio/migration.c | 8 +++++---
> include/hw/vfio/vfio-common.h | 3 ++-
> 3 files changed, 11 insertions(+), 5 deletions(-)
Thanks for this, Yan.
Kirti, I never saw an acknowledgment of this, can you confirm you've
incorporated this into your latest? I do see we're making use of
memory_region_get_ram_addr() now. Thanks,
Alex
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 6f36b02..ba1a8ef 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -799,6 +799,7 @@ static void vfio_listerner_log_sync(MemoryListener *listener,
> MemoryRegionSection *section)
> {
> uint64_t start_addr, size, pfn_count;
> + uint64_t block_start;
> VFIOGroup *group;
> VFIODevice *vbasedev;
>
> @@ -819,11 +820,13 @@ static void vfio_listerner_log_sync(MemoryListener *listener,
> start_addr = TARGET_PAGE_ALIGN(section->offset_within_address_space);
> size = int128_get64(section->size);
> pfn_count = size >> TARGET_PAGE_BITS;
> + block_start = TARGET_PAGE_ALIGN(section->offset_within_region +
> + memory_region_get_ram_addr(section->mr));
>
> QLIST_FOREACH(group, &vfio_group_list, next) {
> QLIST_FOREACH(vbasedev, &group->device_list, next) {
> vfio_get_dirty_page_list(vbasedev, start_addr >> TARGET_PAGE_BITS,
> - pfn_count, TARGET_PAGE_SIZE);
> + pfn_count, TARGET_PAGE_SIZE, block_start);
> }
> }
> }
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index 640bea1..a19b957 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -279,7 +279,8 @@ static int vfio_load_device_config_state(QEMUFile *f, void *opaque)
> void vfio_get_dirty_page_list(VFIODevice *vbasedev,
> uint64_t start_pfn,
> uint64_t pfn_count,
> - uint64_t page_size)
> + uint64_t page_size,
> + uint64_t block_start)
> {
> VFIOMigration *migration = vbasedev->migration;
> VFIORegion *region = &migration->region;
> @@ -293,6 +294,7 @@ void vfio_get_dirty_page_list(VFIODevice *vbasedev,
> while (total_pfns > 0) {
> uint64_t bitmap_size, data_offset = 0;
> uint64_t start = start_pfn + count;
> + uint64_t block_start_seg = block_start + count * page_size;
> void *buf = NULL;
> bool buffer_mmaped = false;
>
> @@ -341,7 +343,7 @@ void vfio_get_dirty_page_list(VFIODevice *vbasedev,
> break;
> } else if (copied_pfns == VFIO_DEVICE_DIRTY_PFNS_ALL) {
> /* Mark all pages dirty for this range */
> - cpu_physical_memory_set_dirty_range(start * page_size,
> + cpu_physical_memory_set_dirty_range(block_start_seg,
> total_pfns * page_size,
> DIRTY_MEMORY_MIGRATION);
> break;
> @@ -382,7 +384,7 @@ void vfio_get_dirty_page_list(VFIODevice *vbasedev,
> }
>
> cpu_physical_memory_set_dirty_lebitmap((unsigned long *)buf,
> - start * page_size,
> + block_start_seg,
> copied_pfns);
> count += copied_pfns;
> total_pfns -= copied_pfns;
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 41ff5eb..6d868fa 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -220,6 +220,7 @@ int vfio_spapr_remove_window(VFIOContainer *container,
> int vfio_migration_probe(VFIODevice *vbasedev, Error **errp);
> void vfio_migration_finalize(VFIODevice *vbasedev);
> void vfio_get_dirty_page_list(VFIODevice *vbasedev, uint64_t start_pfn,
> - uint64_t pfn_count, uint64_t page_size);
> + uint64_t pfn_count, uint64_t page_size,
> + uint64_t block_start);
>
> #endif /* HW_VFIO_VFIO_COMMON_H */
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] vfio/migration: fix dirty pages lost bug for ram beyond 3G
2020-03-25 18:19 ` Alex Williamson
@ 2020-03-26 20:11 ` Kirti Wankhede
0 siblings, 0 replies; 3+ messages in thread
From: Kirti Wankhede @ 2020-03-26 20:11 UTC (permalink / raw)
To: Alex Williamson, Yan Zhao; +Cc: Kevin Tian, intel-gvt-dev, qemu-devel
On 3/25/2020 11:49 PM, Alex Williamson wrote:
> On Fri, 6 Mar 2020 17:41:29 +0800
> Yan Zhao <yan.y.zhao@intel.com> wrote:
>
>> the start address passing to
>> cpu_physical_memory_set_dirty_range() and
>> cpu_physical_memory_set_dirty_lebitmap() is the address within the
>> ram block plus ram block offset.
>>
>> it's safe to set this start address to gpa if total memory is less than
>> 3G, as ram block offset for pc.ram is 0. But if memory size is beyond
>> 3G, gpa is not equal to the offset in its ram block.
>>
>> e.g.
>> for gpa 0x100000000, its offset is actually 0xc0000000.
>> and for gpa 0x42f500000, its offset should be 0x3EF500000.
>>
>> This fix is based on Kirti's VFIO live migration patch set v8.
>> https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg05542.html
>> (for PATCH v8 11/13
>> https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg05553.html
>> and PATCH v8 12/13
>> https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg05554.html
>> }
>>
>> The idea behind it should also be applied to other VFIO live migraiton
>> patch series below:
>> https://lists.gnu.org/archive/html/qemu-devel/2019-11/msg01763.html
>> (Kirti v9)
>> https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg04912.html
>> (Yan)
>> https://lists.gnu.org/archive/html/qemu-devel/2018-04/msg01138.html
>> (Yulei RFC v4)
>> https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg05568.html
>> (Yulei RFC)
>>
>> Cc: Kevin Tian <kevin.tian@intel.com>
>> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
>> ---
>> hw/vfio/common.c | 5 ++++-
>> hw/vfio/migration.c | 8 +++++---
>> include/hw/vfio/vfio-common.h | 3 ++-
>> 3 files changed, 11 insertions(+), 5 deletions(-)
>
> Thanks for this, Yan.
>
> Kirti, I never saw an acknowledgment of this, can you confirm you've
> incorporated this into your latest? I do see we're making use of
> memory_region_get_ram_addr() now. Thanks,
>
We do saw this in our testing and I already had it in QEMU patches
during my testing.
Thanks,
Kirti
> Alex
>
>
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index 6f36b02..ba1a8ef 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -799,6 +799,7 @@ static void vfio_listerner_log_sync(MemoryListener *listener,
>> MemoryRegionSection *section)
>> {
>> uint64_t start_addr, size, pfn_count;
>> + uint64_t block_start;
>> VFIOGroup *group;
>> VFIODevice *vbasedev;
>>
>> @@ -819,11 +820,13 @@ static void vfio_listerner_log_sync(MemoryListener *listener,
>> start_addr = TARGET_PAGE_ALIGN(section->offset_within_address_space);
>> size = int128_get64(section->size);
>> pfn_count = size >> TARGET_PAGE_BITS;
>> + block_start = TARGET_PAGE_ALIGN(section->offset_within_region +
>> + memory_region_get_ram_addr(section->mr));
>>
>> QLIST_FOREACH(group, &vfio_group_list, next) {
>> QLIST_FOREACH(vbasedev, &group->device_list, next) {
>> vfio_get_dirty_page_list(vbasedev, start_addr >> TARGET_PAGE_BITS,
>> - pfn_count, TARGET_PAGE_SIZE);
>> + pfn_count, TARGET_PAGE_SIZE, block_start);
>> }
>> }
>> }
>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>> index 640bea1..a19b957 100644
>> --- a/hw/vfio/migration.c
>> +++ b/hw/vfio/migration.c
>> @@ -279,7 +279,8 @@ static int vfio_load_device_config_state(QEMUFile *f, void *opaque)
>> void vfio_get_dirty_page_list(VFIODevice *vbasedev,
>> uint64_t start_pfn,
>> uint64_t pfn_count,
>> - uint64_t page_size)
>> + uint64_t page_size,
>> + uint64_t block_start)
>> {
>> VFIOMigration *migration = vbasedev->migration;
>> VFIORegion *region = &migration->region;
>> @@ -293,6 +294,7 @@ void vfio_get_dirty_page_list(VFIODevice *vbasedev,
>> while (total_pfns > 0) {
>> uint64_t bitmap_size, data_offset = 0;
>> uint64_t start = start_pfn + count;
>> + uint64_t block_start_seg = block_start + count * page_size;
>> void *buf = NULL;
>> bool buffer_mmaped = false;
>>
>> @@ -341,7 +343,7 @@ void vfio_get_dirty_page_list(VFIODevice *vbasedev,
>> break;
>> } else if (copied_pfns == VFIO_DEVICE_DIRTY_PFNS_ALL) {
>> /* Mark all pages dirty for this range */
>> - cpu_physical_memory_set_dirty_range(start * page_size,
>> + cpu_physical_memory_set_dirty_range(block_start_seg,
>> total_pfns * page_size,
>> DIRTY_MEMORY_MIGRATION);
>> break;
>> @@ -382,7 +384,7 @@ void vfio_get_dirty_page_list(VFIODevice *vbasedev,
>> }
>>
>> cpu_physical_memory_set_dirty_lebitmap((unsigned long *)buf,
>> - start * page_size,
>> + block_start_seg,
>> copied_pfns);
>> count += copied_pfns;
>> total_pfns -= copied_pfns;
>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>> index 41ff5eb..6d868fa 100644
>> --- a/include/hw/vfio/vfio-common.h
>> +++ b/include/hw/vfio/vfio-common.h
>> @@ -220,6 +220,7 @@ int vfio_spapr_remove_window(VFIOContainer *container,
>> int vfio_migration_probe(VFIODevice *vbasedev, Error **errp);
>> void vfio_migration_finalize(VFIODevice *vbasedev);
>> void vfio_get_dirty_page_list(VFIODevice *vbasedev, uint64_t start_pfn,
>> - uint64_t pfn_count, uint64_t page_size);
>> + uint64_t pfn_count, uint64_t page_size,
>> + uint64_t block_start);
>>
>> #endif /* HW_VFIO_VFIO_COMMON_H */
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-03-26 20:12 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-06 9:41 [PATCH] vfio/migration: fix dirty pages lost bug for ram beyond 3G Yan Zhao
2020-03-25 18:19 ` Alex Williamson
2020-03-26 20:11 ` Kirti Wankhede
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.