Re: [PATCH] vfio/migration: fix dirty pages lost bug for ram beyond 3G

[Kirti Wankhede]

On 3/25/2020 11:49 PM, Alex Williamson wrote:
On Fri,  6 Mar 2020 17:41:29 +0800
Yan Zhao <yan.y.z...@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.t...@intel.com>
Signed-off-by: Yan Zhao <yan.y.z...@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 */