[PATCH RFCv1 0/8] hw/arm/virt: Support dirty ring

[PATCH RFCv1 0/8] hw/arm/virt: Support dirty ring

[Gavin Shan]

This series intends to support dirty ring for live migration. The dirty
ring use discrete buffer to track dirty pages. For ARM64, the speciality
is to use backup bitmap to track dirty pages when there is no-running-vcpu
context. It's known that the backup bitmap needs to be synchronized when
KVM device "kvm-arm-gicv3" or "arm-its-kvm" has been enabled. The backup
bitmap is collected in the last stage of migration.

PATCH[1]    Synchronize linux-headers for dirty ring
PATCH[2-3]  Introduce indicator of the last stage migration and pass it
            all the way down
PATCH[4-5]  Introduce secondary bitmap, corresponding to the backup bitmap
PATCH[6-8]  Enable dirty ring for hw/arm/virt

Testing
=======
(1) kvm-unit-tests/its-pending-migration and kvm-unit-tests/its-migration with
    dirty ring or normal dirty page tracking mechanism. All test cases passed.

    QEMU=./qemu.main/build/qemu-system-aarch64 ACCEL=kvm \
    ./its-pending-migration

    QEMU=./qemu.main/build/qemu-system-aarch64 ACCEL=kvm \
    ./its-migration

    QEMU=./qemu.main/build/qemu-system-aarch64 ACCEL=kvm,dirty-ring-size=65536 \
    ./its-pending-migration

    QEMU=./qemu.main/build/qemu-system-aarch64 ACCEL=kvm,dirty-ring-size=65536 \
    ./its-migration

(2) Combinations of migration, post-copy migration, e1000e and virtio-net
    devices. All test cases passed.

    -netdev tap,id=net0,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown  \
    -device e1000e,bus=pcie.5,netdev=net0,mac=52:54:00:f1:26:a0

    -netdev tap,id=vnet0,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown \
    -device virtio-net-pci,bus=pcie.6,netdev=vnet0,mac=52:54:00:f1:26:b0

Gavin Shan (8):
  linux-headers: Update for dirty ring
  memory: Add last stage indicator to global dirty log synchronization
  migration: Add last stage indicator to global dirty log
    synchronization
  kvm: Introduce secondary dirty bitmap
  kvm: Synchronize secondary bitmap in last stage
  kvm: Add helper kvm_dirty_ring_init()
  hw/arm/virt: Enable backup bitmap for dirty ring
  kvm: Enable dirty ring for arm64

 accel/kvm/kvm-all.c           | 138 ++++++++++++++++++++++++----------
 hw/arm/virt.c                 |  26 +++++++
 include/exec/memory.h         |   5 +-
 include/sysemu/kvm_int.h      |   1 +
 linux-headers/asm-arm64/kvm.h |   1 +
 linux-headers/linux/kvm.h     |   2 +
 migration/dirtyrate.c         |   4 +-
 migration/ram.c               |  20 ++---
 softmmu/memory.c              |  10 +--
 target/arm/kvm64.c            |  25 ++++++
 target/arm/kvm_arm.h          |  12 +++
 11 files changed, 185 insertions(+), 59 deletions(-)

-- 
2.23.0

[PATCH RFCv1 1/8] linux-headers: Update for dirty ring

[Gavin Shan]

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 linux-headers/asm-arm64/kvm.h | 1 +
 linux-headers/linux/kvm.h     | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/linux-headers/asm-arm64/kvm.h b/linux-headers/asm-arm64/kvm.h
index 4bf2d7246e..a7cfefb3a8 100644
--- a/linux-headers/asm-arm64/kvm.h
+++ b/linux-headers/asm-arm64/kvm.h
@@ -43,6 +43,7 @@
 #define __KVM_HAVE_VCPU_EVENTS
 
 #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
+#define KVM_DIRTY_LOG_PAGE_OFFSET 64
 
 #define KVM_REG_SIZE(id)						\
 	(1U << (((id) & KVM_REG_SIZE_MASK) >> KVM_REG_SIZE_SHIFT))
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index ebdafa576d..5b4e0e4e68 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -1175,6 +1175,8 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_VM_DISABLE_NX_HUGE_PAGES 220
 #define KVM_CAP_S390_ZPCI_OP 221
 #define KVM_CAP_S390_CPU_TOPOLOGY 222
+#define KVM_CAP_DIRTY_LOG_RING_ACQ_REL 223
+#define KVM_CAP_DIRTY_LOG_RING_WITH_BITMAP 225
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
2.23.0

[PATCH RFCv1 2/8] memory: Add last stage indicator to global dirty log synchronization

[Gavin Shan]

The global dirty log synchronization is used when KVM and dirty ring
are enabled. There is a particularity for ARM64 where the backup
bitmap is used to track dirty pages in non-running-vcpu situations.
It means the dirty ring works with the combination of ring buffer
and backup bitmap. The dirty bits in the backup bitmap needs to
collected in the last stage of live migration.

In order to identify the last stage of live migration and pass it
down, an extra parameter is added to the relevant functions and
callback. This last stage information isn't used yet.

No functional change intended.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 accel/kvm/kvm-all.c   |  2 +-
 include/exec/memory.h |  5 +++--
 migration/dirtyrate.c |  4 ++--
 migration/ram.c       |  6 +++---
 softmmu/memory.c      | 10 +++++-----
 5 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 9b26582655..01a6a026af 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -1554,7 +1554,7 @@ static void kvm_log_sync(MemoryListener *listener,
     kvm_slots_unlock();
 }
 
-static void kvm_log_sync_global(MemoryListener *l)
+static void kvm_log_sync_global(MemoryListener *l, bool last_stage)
 {
     KVMMemoryListener *kml = container_of(l, KVMMemoryListener, listener);
     KVMState *s = kvm_state;
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 2e602a2fad..75b2fd9f48 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -929,8 +929,9 @@ struct MemoryListener {
      * its @log_sync must be NULL.  Vice versa.
      *
      * @listener: The #MemoryListener.
+     * @last_stage: The last stage to synchronize the log during migration
      */
-    void (*log_sync_global)(MemoryListener *listener);
+    void (*log_sync_global)(MemoryListener *listener, bool last_stage);
 
     /**
      * @log_clear:
@@ -2408,7 +2409,7 @@ MemoryRegionSection memory_region_find(MemoryRegion *mr,
  *
  * Synchronizes the dirty page log for all address spaces.
  */
-void memory_global_dirty_log_sync(void);
+void memory_global_dirty_log_sync(bool last_stage);
 
 /**
  * memory_global_dirty_log_sync: synchronize the dirty log for all memory
diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 4bfb97fc68..aecc8142e4 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -101,7 +101,7 @@ void global_dirty_log_change(unsigned int flag, bool start)
 static void global_dirty_log_sync(unsigned int flag, bool one_shot)
 {
     qemu_mutex_lock_iothread();
-    memory_global_dirty_log_sync();
+    memory_global_dirty_log_sync(false);
     if (one_shot) {
         memory_global_dirty_log_stop(flag);
     }
@@ -553,7 +553,7 @@ static void calculate_dirtyrate_dirty_bitmap(struct DirtyRateConfig config)
      * skip it unconditionally and start dirty tracking
      * from 2'round of log sync
      */
-    memory_global_dirty_log_sync();
+    memory_global_dirty_log_sync(false);
 
     /*
      * reset page protect manually and unconditionally.
diff --git a/migration/ram.c b/migration/ram.c
index 334309f1c6..d1b9b270ec 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1188,7 +1188,7 @@ static void migration_bitmap_sync(RAMState *rs)
     }
 
     trace_migration_bitmap_sync_start();
-    memory_global_dirty_log_sync();
+    memory_global_dirty_log_sync(false);
 
     qemu_mutex_lock(&rs->bitmap_mutex);
     WITH_RCU_READ_LOCK_GUARD() {
@@ -3817,7 +3817,7 @@ void colo_incoming_start_dirty_log(void)
     qemu_mutex_lock_iothread();
     qemu_mutex_lock_ramlist();
 
-    memory_global_dirty_log_sync();
+    memory_global_dirty_log_sync(false);
     WITH_RCU_READ_LOCK_GUARD() {
         RAMBLOCK_FOREACH_NOT_IGNORED(block) {
             ramblock_sync_dirty_bitmap(ram_state, block);
@@ -4114,7 +4114,7 @@ void colo_flush_ram_cache(void)
     void *src_host;
     unsigned long offset = 0;
 
-    memory_global_dirty_log_sync();
+    memory_global_dirty_log_sync(false);
     WITH_RCU_READ_LOCK_GUARD() {
         RAMBLOCK_FOREACH_NOT_IGNORED(block) {
             ramblock_sync_dirty_bitmap(ram_state, block);
diff --git a/softmmu/memory.c b/softmmu/memory.c
index 9d64efca26..1cc36ef028 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -2224,7 +2224,7 @@ void memory_region_set_dirty(MemoryRegion *mr, hwaddr addr,
  * If memory region `mr' is NULL, do global sync.  Otherwise, sync
  * dirty bitmap for the specified memory region.
  */
-static void memory_region_sync_dirty_bitmap(MemoryRegion *mr)
+static void memory_region_sync_dirty_bitmap(MemoryRegion *mr, bool last_stage)
 {
     MemoryListener *listener;
     AddressSpace *as;
@@ -2254,7 +2254,7 @@ static void memory_region_sync_dirty_bitmap(MemoryRegion *mr)
              * is to do a global sync, because we are not capable to
              * sync in a finer granularity.
              */
-            listener->log_sync_global(listener);
+            listener->log_sync_global(listener, last_stage);
             trace_memory_region_sync_dirty(mr ? mr->name : "(all)", listener->name, 1);
         }
     }
@@ -2318,7 +2318,7 @@ DirtyBitmapSnapshot *memory_region_snapshot_and_clear_dirty(MemoryRegion *mr,
 {
     DirtyBitmapSnapshot *snapshot;
     assert(mr->ram_block);
-    memory_region_sync_dirty_bitmap(mr);
+    memory_region_sync_dirty_bitmap(mr, false);
     snapshot = cpu_physical_memory_snapshot_and_clear_dirty(mr, addr, size, client);
     memory_global_after_dirty_log_sync();
     return snapshot;
@@ -2844,9 +2844,9 @@ bool memory_region_present(MemoryRegion *container, hwaddr addr)
     return mr && mr != container;
 }
 
-void memory_global_dirty_log_sync(void)
+void memory_global_dirty_log_sync(bool last_stage)
 {
-    memory_region_sync_dirty_bitmap(NULL);
+    memory_region_sync_dirty_bitmap(NULL, last_stage);
 }
 
 void memory_global_after_dirty_log_sync(void)
-- 
2.23.0

[PATCH RFCv1 3/8] migration: Add last stage indicator to global dirty log synchronization

[Gavin Shan]

For the pre-copy live migration scenario, the last stage indicator
is needed for KVM backend to collect the dirty pages from the backup
bitmap when dirty ring is used. The indicator isn't used so far.

No functional change intended.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 migration/ram.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index d1b9b270ec..6c9642edee 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1176,7 +1176,7 @@ static void migration_trigger_throttle(RAMState *rs)
     }
 }
 
-static void migration_bitmap_sync(RAMState *rs)
+static void migration_bitmap_sync(RAMState *rs, bool last_stage)
 {
     RAMBlock *block;
     int64_t end_time;
@@ -1188,7 +1188,7 @@ static void migration_bitmap_sync(RAMState *rs)
     }
 
     trace_migration_bitmap_sync_start();
-    memory_global_dirty_log_sync(false);
+    memory_global_dirty_log_sync(last_stage);
 
     qemu_mutex_lock(&rs->bitmap_mutex);
     WITH_RCU_READ_LOCK_GUARD() {
@@ -1222,7 +1222,7 @@ static void migration_bitmap_sync(RAMState *rs)
     }
 }
 
-static void migration_bitmap_sync_precopy(RAMState *rs)
+static void migration_bitmap_sync_precopy(RAMState *rs, bool last_stage)
 {
     Error *local_err = NULL;
 
@@ -1235,7 +1235,7 @@ static void migration_bitmap_sync_precopy(RAMState *rs)
         local_err = NULL;
     }
 
-    migration_bitmap_sync(rs);
+    migration_bitmap_sync(rs, last_stage);
 
     if (precopy_notify(PRECOPY_NOTIFY_AFTER_BITMAP_SYNC, &local_err)) {
         error_report_err(local_err);
@@ -2844,7 +2844,7 @@ void ram_postcopy_send_discard_bitmap(MigrationState *ms)
     RCU_READ_LOCK_GUARD();
 
     /* This should be our last sync, the src is now paused */
-    migration_bitmap_sync(rs);
+    migration_bitmap_sync(rs, false);
 
     /* Easiest way to make sure we don't resume in the middle of a host-page */
     rs->pss[RAM_CHANNEL_PRECOPY].last_sent_block = NULL;
@@ -3034,7 +3034,7 @@ static void ram_init_bitmaps(RAMState *rs)
         /* We don't use dirty log with background snapshots */
         if (!migrate_background_snapshot()) {
             memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION);
-            migration_bitmap_sync_precopy(rs);
+            migration_bitmap_sync_precopy(rs, false);
         }
     }
     qemu_mutex_unlock_ramlist();
@@ -3349,7 +3349,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
 
     WITH_RCU_READ_LOCK_GUARD() {
         if (!migration_in_postcopy()) {
-            migration_bitmap_sync_precopy(rs);
+            migration_bitmap_sync_precopy(rs, true);
         }
 
         ram_control_before_iterate(f, RAM_CONTROL_FINISH);
@@ -3407,7 +3407,7 @@ static void ram_save_pending(QEMUFile *f, void *opaque, uint64_t max_size,
         remaining_size < max_size) {
         qemu_mutex_lock_iothread();
         WITH_RCU_READ_LOCK_GUARD() {
-            migration_bitmap_sync_precopy(rs);
+            migration_bitmap_sync_precopy(rs, false);
         }
         qemu_mutex_unlock_iothread();
         remaining_size = rs->migration_dirty_pages * TARGET_PAGE_SIZE;
-- 
2.23.0

[PATCH RFCv1 4/8] kvm: Introduce secondary dirty bitmap

[Gavin Shan]

When dirty ring is enabled on ARM64, the backup bitmap may be used
to track the dirty pages in no-running-vcpu situations. The original
bitmap is the primary one, used for the dirty ring buffer. We need
the secondary bitmap to collect the backup bitmap for ARM64.

No functional change intended.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 accel/kvm/kvm-all.c      | 50 ++++++++++++++++++++++++++++++----------
 include/sysemu/kvm_int.h |  1 +
 2 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 01a6a026af..1a93985574 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -553,13 +553,29 @@ static void kvm_log_stop(MemoryListener *listener,
     }
 }
 
+static unsigned long *kvm_slot_dirty_bitmap(KVMSlot *slot, bool primary)
+{
+    if (primary) {
+        return slot->dirty_bmap;
+    }
+
+    return slot->dirty_bmap +
+           slot->dirty_bmap_size / sizeof(slot->dirty_bmap[0]);
+}
+
 /* get kvm's dirty pages bitmap and update qemu's */
-static void kvm_slot_sync_dirty_pages(KVMSlot *slot)
+static void kvm_slot_sync_dirty_pages(KVMSlot *slot, bool primary)
 {
+    KVMState *s = kvm_state;
+    unsigned long *bmap = kvm_slot_dirty_bitmap(slot, primary);
     ram_addr_t start = slot->ram_start_offset;
     ram_addr_t pages = slot->memory_size / qemu_real_host_page_size();
 
-    cpu_physical_memory_set_dirty_lebitmap(slot->dirty_bmap, start, pages);
+    if (!s->kvm_dirty_ring_with_bitmap && !primary) {
+        return;
+    }
+
+    cpu_physical_memory_set_dirty_lebitmap(bmap, start, pages);
 }
 
 static void kvm_slot_reset_dirty_pages(KVMSlot *slot)
@@ -572,6 +588,9 @@ static void kvm_slot_reset_dirty_pages(KVMSlot *slot)
 /* Allocate the dirty bitmap for a slot  */
 static void kvm_slot_init_dirty_bitmap(KVMSlot *mem)
 {
+    KVMState *s = kvm_state;
+    hwaddr bitmap_size, alloc_size;
+
     if (!(mem->flags & KVM_MEM_LOG_DIRTY_PAGES) || mem->dirty_bmap) {
         return;
     }
@@ -593,9 +612,11 @@ static void kvm_slot_init_dirty_bitmap(KVMSlot *mem)
      * And mem->memory_size is aligned to it (otherwise this mem can't
      * be registered to KVM).
      */
-    hwaddr bitmap_size = ALIGN(mem->memory_size / qemu_real_host_page_size(),
-                                        /*HOST_LONG_BITS*/ 64) / 8;
-    mem->dirty_bmap = g_malloc0(bitmap_size);
+    bitmap_size = ALIGN(mem->memory_size / qemu_real_host_page_size(),
+                        /*HOST_LONG_BITS*/ 64) / 8;
+    alloc_size = s->kvm_dirty_ring_with_bitmap ? 2 * bitmap_size : bitmap_size;
+
+    mem->dirty_bmap = g_malloc0(alloc_size);
     mem->dirty_bmap_size = bitmap_size;
 }
 
@@ -603,12 +624,16 @@ static void kvm_slot_init_dirty_bitmap(KVMSlot *mem)
  * Sync dirty bitmap from kernel to KVMSlot.dirty_bmap, return true if
  * succeeded, false otherwise
  */
-static bool kvm_slot_get_dirty_log(KVMState *s, KVMSlot *slot)
+static bool kvm_slot_get_dirty_log(KVMState *s, KVMSlot *slot, bool primary)
 {
     struct kvm_dirty_log d = {};
     int ret;
 
-    d.dirty_bitmap = slot->dirty_bmap;
+    if (!s->kvm_dirty_ring_with_bitmap && !primary) {
+        return false;
+    }
+
+    d.dirty_bitmap = kvm_slot_dirty_bitmap(slot, primary);
     d.slot = slot->slot | (slot->as_id << 16);
     ret = kvm_vm_ioctl(s, KVM_GET_DIRTY_LOG, &d);
 
@@ -839,8 +864,8 @@ static void kvm_physical_sync_dirty_bitmap(KVMMemoryListener *kml,
             /* We don't have a slot if we want to trap every access. */
             return;
         }
-        if (kvm_slot_get_dirty_log(s, mem)) {
-            kvm_slot_sync_dirty_pages(mem);
+        if (kvm_slot_get_dirty_log(s, mem, true)) {
+            kvm_slot_sync_dirty_pages(mem, true);
         }
         start_addr += slot_size;
         size -= slot_size;
@@ -1353,9 +1378,9 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
                 if (kvm_state->kvm_dirty_ring_size) {
                     kvm_dirty_ring_reap_locked(kvm_state, NULL);
                 } else {
-                    kvm_slot_get_dirty_log(kvm_state, mem);
+                    kvm_slot_get_dirty_log(kvm_state, mem, true);
                 }
-                kvm_slot_sync_dirty_pages(mem);
+                kvm_slot_sync_dirty_pages(mem, true);
             }
 
             /* unregister the slot */
@@ -1572,7 +1597,7 @@ static void kvm_log_sync_global(MemoryListener *l, bool last_stage)
     for (i = 0; i < s->nr_slots; i++) {
         mem = &kml->slots[i];
         if (mem->memory_size && mem->flags & KVM_MEM_LOG_DIRTY_PAGES) {
-            kvm_slot_sync_dirty_pages(mem);
+            kvm_slot_sync_dirty_pages(mem, true);
             /*
              * This is not needed by KVM_GET_DIRTY_LOG because the
              * ioctl will unconditionally overwrite the whole region.
@@ -3701,6 +3726,7 @@ static void kvm_accel_instance_init(Object *obj)
     s->kernel_irqchip_split = ON_OFF_AUTO_AUTO;
     /* KVM dirty ring is by default off */
     s->kvm_dirty_ring_size = 0;
+    s->kvm_dirty_ring_with_bitmap = false;
     s->notify_vmexit = NOTIFY_VMEXIT_OPTION_RUN;
     s->notify_window = 0;
 }
diff --git a/include/sysemu/kvm_int.h b/include/sysemu/kvm_int.h
index 60b520a13e..fdd5b1bde0 100644
--- a/include/sysemu/kvm_int.h
+++ b/include/sysemu/kvm_int.h
@@ -115,6 +115,7 @@ struct KVMState
     } *as;
     uint64_t kvm_dirty_ring_bytes;  /* Size of the per-vcpu dirty ring */
     uint32_t kvm_dirty_ring_size;   /* Number of dirty GFNs per ring */
+    bool kvm_dirty_ring_with_bitmap;
     struct KVMDirtyRingReaper reaper;
     NotifyVmexitOption notify_vmexit;
     uint32_t notify_window;
-- 
2.23.0

[PATCH RFCv1 5/8] kvm: Synchronize secondary bitmap in last stage

[Gavin Shan]

In the last stage of live migration or memory slot removal, the
backup bitmap needs to be synchronized.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 accel/kvm/kvm-all.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 1a93985574..9ec117c441 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -1377,10 +1377,12 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
                  */
                 if (kvm_state->kvm_dirty_ring_size) {
                     kvm_dirty_ring_reap_locked(kvm_state, NULL);
+                    kvm_slot_get_dirty_log(kvm_state, mem, false);
                 } else {
                     kvm_slot_get_dirty_log(kvm_state, mem, true);
                 }
                 kvm_slot_sync_dirty_pages(mem, true);
+                kvm_slot_sync_dirty_pages(mem, false);
             }
 
             /* unregister the slot */
@@ -1604,6 +1606,11 @@ static void kvm_log_sync_global(MemoryListener *l, bool last_stage)
              * However kvm dirty ring has no such side effect.
              */
             kvm_slot_reset_dirty_pages(mem);
+
+            if (s->kvm_dirty_ring_with_bitmap && last_stage &&
+                kvm_slot_get_dirty_log(s, mem, false)) {
+                kvm_slot_sync_dirty_pages(mem, false);
+            }
         }
     }
     kvm_slots_unlock();
-- 
2.23.0

[PATCH RFCv1 6/8] kvm: Add helper kvm_dirty_ring_init()

[Gavin Shan]

Due to multiple capabilities associated with the dirty ring for different
architectures: KVM_CAP_DIRTY_{LOG_RING, LOG_RING_ACQ_REL} for x86 and
arm64 separately. There will be more to be done in order to support the
dirty ring for arm64.

Lets add helper kvm_dirty_ring_init() to enable the dirty ring. With this,
the code looks a bit clean.

No functional change intended.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 accel/kvm/kvm-all.c | 73 ++++++++++++++++++++++++++++-----------------
 1 file changed, 46 insertions(+), 27 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 9ec117c441..399ef0f7e6 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -1476,6 +1476,49 @@ static int kvm_dirty_ring_reaper_init(KVMState *s)
     return 0;
 }
 
+static int kvm_dirty_ring_init(KVMState *s)
+{
+    uint64_t ring_bytes;
+    int ret;
+
+    /*
+     * Read the max supported pages. Fall back to dirty logging mode
+     * if the dirty ring isn't supported.
+     */
+    ret = kvm_vm_check_extension(s, KVM_CAP_DIRTY_LOG_RING);
+    if (ret <= 0) {
+        warn_report("KVM dirty ring not available, using bitmap method");
+        s->kvm_dirty_ring_size = 0;
+        return 0;
+    }
+
+    ring_bytes = s->kvm_dirty_ring_size * sizeof(struct kvm_dirty_gfn);
+    if (ring_bytes > ret) {
+        error_report("KVM dirty ring size %" PRIu32 " too big "
+                     "(maximum is %ld).  Please use a smaller value.",
+                     s->kvm_dirty_ring_size,
+                     (long)ret / sizeof(struct kvm_dirty_gfn));
+        ret = -EINVAL;
+        goto out;
+    }
+
+    ret = kvm_vm_enable_cap(s, KVM_CAP_DIRTY_LOG_RING, 0, ring_bytes);
+    if (ret) {
+        error_report("Enabling of KVM dirty ring failed: %s. "
+                     "Suggested minimum value is 1024.", strerror(-ret));
+        ret = -EIO;
+    }
+
+out:
+    if (ret) {
+        s->kvm_dirty_ring_size = 0;
+    } else {
+        s->kvm_dirty_ring_bytes = ring_bytes;
+    }
+
+    return ret;
+}
+
 static void kvm_region_add(MemoryListener *listener,
                            MemoryRegionSection *section)
 {
@@ -2545,33 +2588,9 @@ static int kvm_init(MachineState *ms)
      * dirty logging mode
      */
     if (s->kvm_dirty_ring_size > 0) {
-        uint64_t ring_bytes;
-
-        ring_bytes = s->kvm_dirty_ring_size * sizeof(struct kvm_dirty_gfn);
-
-        /* Read the max supported pages */
-        ret = kvm_vm_check_extension(s, KVM_CAP_DIRTY_LOG_RING);
-        if (ret > 0) {
-            if (ring_bytes > ret) {
-                error_report("KVM dirty ring size %" PRIu32 " too big "
-                             "(maximum is %ld).  Please use a smaller value.",
-                             s->kvm_dirty_ring_size,
-                             (long)ret / sizeof(struct kvm_dirty_gfn));
-                ret = -EINVAL;
-                goto err;
-            }
-
-            ret = kvm_vm_enable_cap(s, KVM_CAP_DIRTY_LOG_RING, 0, ring_bytes);
-            if (ret) {
-                error_report("Enabling of KVM dirty ring failed: %s. "
-                             "Suggested minimum value is 1024.", strerror(-ret));
-                goto err;
-            }
-
-            s->kvm_dirty_ring_bytes = ring_bytes;
-         } else {
-             warn_report("KVM dirty ring not available, using bitmap method");
-             s->kvm_dirty_ring_size = 0;
+        ret = kvm_dirty_ring_init(s);
+        if (ret < 0) {
+            goto err;
         }
     }
 
-- 
2.23.0

[PATCH RFCv1 7/8] hw/arm/virt: Enable backup bitmap for dirty ring

[Gavin Shan]

When KVM device "kvm-arm-gicv3" or "arm-its-kvm" is used, we have to
enable the backup bitmap for the dirty ring. Otherwise, the migration
will fail because those two devices are using the backup bitmap to track
dirty guest memory, corresponding to various hardware tables.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 hw/arm/virt.c        | 26 ++++++++++++++++++++++++++
 target/arm/kvm64.c   | 25 +++++++++++++++++++++++++
 target/arm/kvm_arm.h | 12 ++++++++++++
 3 files changed, 63 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ba47728288..b91b5972bc 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2025,6 +2025,8 @@ static void machvirt_init(MachineState *machine)
     VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(machine);
     MachineClass *mc = MACHINE_GET_CLASS(machine);
     const CPUArchIdList *possible_cpus;
+    const char *gictype = NULL;
+    const char *itsclass = NULL;
     MemoryRegion *sysmem = get_system_memory();
     MemoryRegion *secure_sysmem = NULL;
     MemoryRegion *tag_sysmem = NULL;
@@ -2072,6 +2074,30 @@ static void machvirt_init(MachineState *machine)
      */
     finalize_gic_version(vms);
 
+    /*
+     * When "kvm-arm-gicv3" or "arm-its-kvm" is used, the backup dirty
+     * bitmap has to be enabled for KVM dirty ring, before any memory
+     * slot is added. Otherwise, the migration will fail with the dirty
+     * ring.
+     */
+    if (kvm_enabled()) {
+        if (vms->gic_version != VIRT_GIC_VERSION_2) {
+            gictype = gicv3_class_name();
+        }
+
+        if (vms->gic_version != VIRT_GIC_VERSION_2 && vms->its) {
+            itsclass = its_class_name();
+        }
+
+        if (((gictype && !strcmp(gictype, "kvm-arm-gicv3")) ||
+             (itsclass && !strcmp(itsclass, "arm-its-kvm"))) &&
+            !kvm_arm_enable_dirty_ring_with_bitmap()) {
+            error_report("Failed to enable the backup bitmap for "
+                         "KVM dirty ring");
+            exit(1);
+        }
+    }
+
     if (vms->secure) {
         /*
          * The Secure view of the world is the same as the NonSecure,
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 1197253d12..91960e1cd3 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -764,6 +764,31 @@ bool kvm_arm_steal_time_supported(void)
     return kvm_check_extension(kvm_state, KVM_CAP_STEAL_TIME);
 }
 
+bool kvm_arm_enable_dirty_ring_with_bitmap(void)
+{
+    int ret;
+
+    /* No need to enable the backup bitmap if dirty ring isn't enabled */
+    if (!kvm_dirty_ring_enabled()) {
+        return true;
+    }
+
+    ret = kvm_check_extension(kvm_state, KVM_CAP_DIRTY_LOG_RING_WITH_BITMAP);
+    if (!ret) {
+        return false;
+    }
+
+    ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_DIRTY_LOG_RING_WITH_BITMAP,
+                            0, 1);
+    if (ret) {
+        return false;
+    }
+
+    kvm_state->kvm_dirty_ring_with_bitmap = true;
+
+    return true;
+}
+
 QEMU_BUILD_BUG_ON(KVM_ARM64_SVE_VQ_MIN != 1);
 
 uint32_t kvm_arm_sve_get_vls(CPUState *cs)
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 99017b635c..402281e61a 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -282,6 +282,13 @@ void kvm_arm_steal_time_finalize(ARMCPU *cpu, Error **errp);
  */
 bool kvm_arm_steal_time_supported(void);
 
+/**
+ * kvm_arm_enable_dirty_ring_with_bitmap:
+ * Returns: true if KVM dirty ring's backup bitmap is enabled
+ * and false otherwise.
+ */
+bool kvm_arm_enable_dirty_ring_with_bitmap(void);
+
 /**
  * kvm_arm_aarch32_supported:
  *
@@ -395,6 +402,11 @@ static inline bool kvm_arm_steal_time_supported(void)
     return false;
 }
 
+static inline bool kvm_arm_enable_dirty_ring_with_bitmap(void)
+{
+    return false;
+}
+
 /*
  * These functions should never actually be called without KVM support.
  */
-- 
2.23.0

[PATCH RFCv1 8/8] kvm: Enable dirty ring for arm64

[Gavin Shan]

arm64 has different capability from x86 to enable the dirty ring, which
is KVM_CAP_DIRTY_LOG_RING_ACQ_REL. To enable it in kvm_dirty_ring_init()
when KVM_CAP_DIRTY_LOG_RING isn't supported.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 accel/kvm/kvm-all.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 399ef0f7e6..8ab31865eb 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -1479,13 +1479,19 @@ static int kvm_dirty_ring_reaper_init(KVMState *s)
 static int kvm_dirty_ring_init(KVMState *s)
 {
     uint64_t ring_bytes;
+    unsigned int capability = KVM_CAP_DIRTY_LOG_RING;
     int ret;
 
     /*
      * Read the max supported pages. Fall back to dirty logging mode
      * if the dirty ring isn't supported.
      */
-    ret = kvm_vm_check_extension(s, KVM_CAP_DIRTY_LOG_RING);
+    ret = kvm_vm_check_extension(s, capability);
+    if (ret <= 0) {
+        capability = KVM_CAP_DIRTY_LOG_RING_ACQ_REL;
+        ret = kvm_vm_check_extension(s, capability);
+    }
+
     if (ret <= 0) {
         warn_report("KVM dirty ring not available, using bitmap method");
         s->kvm_dirty_ring_size = 0;
@@ -1502,7 +1508,7 @@ static int kvm_dirty_ring_init(KVMState *s)
         goto out;
     }
 
-    ret = kvm_vm_enable_cap(s, KVM_CAP_DIRTY_LOG_RING, 0, ring_bytes);
+    ret = kvm_vm_enable_cap(s, capability, 0, ring_bytes);
     if (ret) {
         error_report("Enabling of KVM dirty ring failed: %s. "
                      "Suggested minimum value is 1024.", strerror(-ret));
-- 
2.23.0

Re: [PATCH RFCv1 4/8] kvm: Introduce secondary dirty bitmap

[Juan Quintela]

Gavin Shan <gshan@redhat.com> wrote:
> When dirty ring is enabled on ARM64, the backup bitmap may be used
> to track the dirty pages in no-running-vcpu situations. The original
> bitmap is the primary one, used for the dirty ring buffer. We need
> the secondary bitmap to collect the backup bitmap for ARM64.
>
> No functional change intended.
>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
>  accel/kvm/kvm-all.c      | 50 ++++++++++++++++++++++++++++++----------
>  include/sysemu/kvm_int.h |  1 +
>  2 files changed, 39 insertions(+), 12 deletions(-)
>
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 01a6a026af..1a93985574 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -553,13 +553,29 @@ static void kvm_log_stop(MemoryListener *listener,
>      }
>  }
>  
> +static unsigned long *kvm_slot_dirty_bitmap(KVMSlot *slot, bool primary)
> +{
> +    if (primary) {
> +        return slot->dirty_bmap;
> +    }
> +
> +    return slot->dirty_bmap +
> +           slot->dirty_bmap_size / sizeof(slot->dirty_bmap[0]);
> +}


Why?
Just use two bitmaps and call it a day.

Later, Juan.

Re: [PATCH RFCv1 6/8] kvm: Add helper kvm_dirty_ring_init()

[Juan Quintela]

Gavin Shan <gshan@redhat.com> wrote:
> Due to multiple capabilities associated with the dirty ring for different
> architectures: KVM_CAP_DIRTY_{LOG_RING, LOG_RING_ACQ_REL} for x86 and
> arm64 separately. There will be more to be done in order to support the
> dirty ring for arm64.
>
> Lets add helper kvm_dirty_ring_init() to enable the dirty ring. With this,
> the code looks a bit clean.
>
> No functional change intended.
>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
>  accel/kvm/kvm-all.c | 73 ++++++++++++++++++++++++++++-----------------
>  1 file changed, 46 insertions(+), 27 deletions(-)
>
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 9ec117c441..399ef0f7e6 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -1476,6 +1476,49 @@ static int kvm_dirty_ring_reaper_init(KVMState *s)
>      return 0;
>  }
>  
> +static int kvm_dirty_ring_init(KVMState *s)
> +{
> +    uint64_t ring_bytes;
> +    int ret;
> +
> +    /*
> +     * Read the max supported pages. Fall back to dirty logging mode
> +     * if the dirty ring isn't supported.
> +     */
> +    ret = kvm_vm_check_extension(s, KVM_CAP_DIRTY_LOG_RING);
> +    if (ret <= 0) {
> +        warn_report("KVM dirty ring not available, using bitmap method");
> +        s->kvm_dirty_ring_size = 0;
> +        return 0;
> +    }
> +
> +    ring_bytes = s->kvm_dirty_ring_size * sizeof(struct kvm_dirty_gfn);
> +    if (ring_bytes > ret) {
> +        error_report("KVM dirty ring size %" PRIu32 " too big "
> +                     "(maximum is %ld).  Please use a smaller value.",
> +                     s->kvm_dirty_ring_size,
> +                     (long)ret / sizeof(struct kvm_dirty_gfn));
> +        ret = -EINVAL;
> +        goto out;
> +    }
> +
> +    ret = kvm_vm_enable_cap(s, KVM_CAP_DIRTY_LOG_RING, 0, ring_bytes);
> +    if (ret) {
> +        error_report("Enabling of KVM dirty ring failed: %s. "
> +                     "Suggested minimum value is 1024.", strerror(-ret));
> +        ret = -EIO;
> +    }
> +
> +out:
> +    if (ret) {
> +        s->kvm_dirty_ring_size = 0;
> +    } else {
> +        s->kvm_dirty_ring_bytes = ring_bytes;
> +    }
> +
> +    return ret;
> +}

If you split it, you don't need the goto.

static int kvm_dirty_ring_init(KVMState *s)
{
    s->kvm_dirty_ring_size = 0;
    /*
     * Read the max supported pages. Fall back to dirty logging mode
     * if the dirty ring isn't supported.
     */
    int ret = kvm_vm_check_extension(s, KVM_CAP_DIRTY_LOG_RING);
    if (ret <= 0) {
        warn_report("KVM dirty ring not available, using bitmap method");
        return 0;
    }

    uint64_t ring_bytes = s->kvm_dirty_ring_size * sizeof(struct kvm_dirty_gfn);
    if (ring_bytes > ret) {
        error_report("KVM dirty ring size %" PRIu32 " too big "
                     "(maximum is %ld).  Please use a smaller value.",
                     s->kvm_dirty_ring_size,
                     (long)ret / sizeof(struct kvm_dirty_gfn));
        return -EINVAL;
    }

    ret = kvm_vm_enable_cap(s, KVM_CAP_DIRTY_LOG_RING, 0, ring_bytes);
    if (ret) {
        error_report("Enabling of KVM dirty ring failed: %s. "
                     "Suggested minimum value is 1024.", strerror(-ret));
        return -EIO;
    }

    s->kvm_dirty_ring_bytes = ring_bytes;
    return ret;
}



> +
>  static void kvm_region_add(MemoryListener *listener,
>                             MemoryRegionSection *section)
>  {
> @@ -2545,33 +2588,9 @@ static int kvm_init(MachineState *ms)
>       * dirty logging mode
>       */
>      if (s->kvm_dirty_ring_size > 0) {
> -        uint64_t ring_bytes;
> -
> -        ring_bytes = s->kvm_dirty_ring_size * sizeof(struct kvm_dirty_gfn);
> -
> -        /* Read the max supported pages */
> -        ret = kvm_vm_check_extension(s, KVM_CAP_DIRTY_LOG_RING);
> -        if (ret > 0) {
> -            if (ring_bytes > ret) {
> -                error_report("KVM dirty ring size %" PRIu32 " too big "
> -                             "(maximum is %ld).  Please use a smaller value.",
> -                             s->kvm_dirty_ring_size,
> -                             (long)ret / sizeof(struct kvm_dirty_gfn));
> -                ret = -EINVAL;
> -                goto err;
> -            }
> -
> -            ret = kvm_vm_enable_cap(s, KVM_CAP_DIRTY_LOG_RING, 0, ring_bytes);
> -            if (ret) {
> -                error_report("Enabling of KVM dirty ring failed: %s. "
> -                             "Suggested minimum value is 1024.", strerror(-ret));
> -                goto err;
> -            }
> -
> -            s->kvm_dirty_ring_bytes = ring_bytes;
> -         } else {
> -             warn_report("KVM dirty ring not available, using bitmap method");
> -             s->kvm_dirty_ring_size = 0;
> +        ret = kvm_dirty_ring_init(s);
> +        if (ret < 0) {
> +            goto err;
>          }
>      }

Re: [PATCH RFCv1 7/8] hw/arm/virt: Enable backup bitmap for dirty ring

[Juan Quintela]

Gavin Shan <gshan@redhat.com> wrote:
> When KVM device "kvm-arm-gicv3" or "arm-its-kvm" is used, we have to
> enable the backup bitmap for the dirty ring. Otherwise, the migration
> will fail because those two devices are using the backup bitmap to track
> dirty guest memory, corresponding to various hardware tables.
>
> Signed-off-by: Gavin Shan <gshan@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

Re: [PATCH RFCv1 8/8] kvm: Enable dirty ring for arm64

[Juan Quintela]

Gavin Shan <gshan@redhat.com> wrote:
> arm64 has different capability from x86 to enable the dirty ring, which
> is KVM_CAP_DIRTY_LOG_RING_ACQ_REL. To enable it in kvm_dirty_ring_init()
> when KVM_CAP_DIRTY_LOG_RING isn't supported.
>
> Signed-off-by: Gavin Shan <gshan@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

Re: [PATCH RFCv1 4/8] kvm: Introduce secondary dirty bitmap

[Gavin Shan]

On 2/9/23 9:07 AM, Juan Quintela wrote:
> Gavin Shan <gshan@redhat.com> wrote:
>> When dirty ring is enabled on ARM64, the backup bitmap may be used
>> to track the dirty pages in no-running-vcpu situations. The original
>> bitmap is the primary one, used for the dirty ring buffer. We need
>> the secondary bitmap to collect the backup bitmap for ARM64.
>>
>> No functional change intended.
>>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>>   accel/kvm/kvm-all.c      | 50 ++++++++++++++++++++++++++++++----------
>>   include/sysemu/kvm_int.h |  1 +
>>   2 files changed, 39 insertions(+), 12 deletions(-)
>>
>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
>> index 01a6a026af..1a93985574 100644
>> --- a/accel/kvm/kvm-all.c
>> +++ b/accel/kvm/kvm-all.c
>> @@ -553,13 +553,29 @@ static void kvm_log_stop(MemoryListener *listener,
>>       }
>>   }
>>   
>> +static unsigned long *kvm_slot_dirty_bitmap(KVMSlot *slot, bool primary)
>> +{
>> +    if (primary) {
>> +        return slot->dirty_bmap;
>> +    }
>> +
>> +    return slot->dirty_bmap +
>> +           slot->dirty_bmap_size / sizeof(slot->dirty_bmap[0]);
>> +}
> 
> 
> Why?
> Just use two bitmaps and call it a day.
> 

Thanks for your review, Juan. Right, I had wrong assumption that the original
(primary) bitmap can't be reused. It's why the secondary bitmap is introduced.
The intention is to use the original (primary) bitmap to cover the dirty-ring
buffers while the secondary bitmap is to cover the backup bitmap, resident in
host kernel.

I think the original (primary) bitmap can be reused in this case. After the
dirty-ring buffer is synchronized to the original bitmap, which is updated
to the dirty bits. It can be reused to cover the backup bitmap. I will remove
the secondary bitmap in next revision.

Thanks,
Gavin

Re: [PATCH RFCv1 6/8] kvm: Add helper kvm_dirty_ring_init()

[Gavin Shan]

On 2/9/23 9:11 AM, Juan Quintela wrote:
> Gavin Shan <gshan@redhat.com> wrote:
>> Due to multiple capabilities associated with the dirty ring for different
>> architectures: KVM_CAP_DIRTY_{LOG_RING, LOG_RING_ACQ_REL} for x86 and
>> arm64 separately. There will be more to be done in order to support the
>> dirty ring for arm64.
>>
>> Lets add helper kvm_dirty_ring_init() to enable the dirty ring. With this,
>> the code looks a bit clean.
>>
>> No functional change intended.
>>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>>   accel/kvm/kvm-all.c | 73 ++++++++++++++++++++++++++++-----------------
>>   1 file changed, 46 insertions(+), 27 deletions(-)
>>
>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
>> index 9ec117c441..399ef0f7e6 100644
>> --- a/accel/kvm/kvm-all.c
>> +++ b/accel/kvm/kvm-all.c
>> @@ -1476,6 +1476,49 @@ static int kvm_dirty_ring_reaper_init(KVMState *s)
>>       return 0;
>>   }
>>   
>> +static int kvm_dirty_ring_init(KVMState *s)
>> +{
>> +    uint64_t ring_bytes;
>> +    int ret;
>> +
>> +    /*
>> +     * Read the max supported pages. Fall back to dirty logging mode
>> +     * if the dirty ring isn't supported.
>> +     */
>> +    ret = kvm_vm_check_extension(s, KVM_CAP_DIRTY_LOG_RING);
>> +    if (ret <= 0) {
>> +        warn_report("KVM dirty ring not available, using bitmap method");
>> +        s->kvm_dirty_ring_size = 0;
>> +        return 0;
>> +    }
>> +
>> +    ring_bytes = s->kvm_dirty_ring_size * sizeof(struct kvm_dirty_gfn);
>> +    if (ring_bytes > ret) {
>> +        error_report("KVM dirty ring size %" PRIu32 " too big "
>> +                     "(maximum is %ld).  Please use a smaller value.",
>> +                     s->kvm_dirty_ring_size,
>> +                     (long)ret / sizeof(struct kvm_dirty_gfn));
>> +        ret = -EINVAL;
>> +        goto out;
>> +    }
>> +
>> +    ret = kvm_vm_enable_cap(s, KVM_CAP_DIRTY_LOG_RING, 0, ring_bytes);
>> +    if (ret) {
>> +        error_report("Enabling of KVM dirty ring failed: %s. "
>> +                     "Suggested minimum value is 1024.", strerror(-ret));
>> +        ret = -EIO;
>> +    }
>> +
>> +out:
>> +    if (ret) {
>> +        s->kvm_dirty_ring_size = 0;
>> +    } else {
>> +        s->kvm_dirty_ring_bytes = ring_bytes;
>> +    }
>> +
>> +    return ret;
>> +}
> 
> If you split it, you don't need the goto.
> 
> static int kvm_dirty_ring_init(KVMState *s)
> {
>      s->kvm_dirty_ring_size = 0;
>      /*
>       * Read the max supported pages. Fall back to dirty logging mode
>       * if the dirty ring isn't supported.
>       */
>      int ret = kvm_vm_check_extension(s, KVM_CAP_DIRTY_LOG_RING);
>      if (ret <= 0) {
>          warn_report("KVM dirty ring not available, using bitmap method");
>          return 0;
>      }
> 
>      uint64_t ring_bytes = s->kvm_dirty_ring_size * sizeof(struct kvm_dirty_gfn);
>      if (ring_bytes > ret) {
>          error_report("KVM dirty ring size %" PRIu32 " too big "
>                       "(maximum is %ld).  Please use a smaller value.",
>                       s->kvm_dirty_ring_size,
>                       (long)ret / sizeof(struct kvm_dirty_gfn));
>          return -EINVAL;
>      }
> 
>      ret = kvm_vm_enable_cap(s, KVM_CAP_DIRTY_LOG_RING, 0, ring_bytes);
>      if (ret) {
>          error_report("Enabling of KVM dirty ring failed: %s. "
>                       "Suggested minimum value is 1024.", strerror(-ret));
>          return -EIO;
>      }
> 
>      s->kvm_dirty_ring_bytes = ring_bytes;
>      return ret;
> }
> 

Ok, thanks for your review. The goto will be removed in next revision.

> 
> 
>> +
>>   static void kvm_region_add(MemoryListener *listener,
>>                              MemoryRegionSection *section)
>>   {
>> @@ -2545,33 +2588,9 @@ static int kvm_init(MachineState *ms)
>>        * dirty logging mode
>>        */
>>       if (s->kvm_dirty_ring_size > 0) {
>> -        uint64_t ring_bytes;
>> -
>> -        ring_bytes = s->kvm_dirty_ring_size * sizeof(struct kvm_dirty_gfn);
>> -
>> -        /* Read the max supported pages */
>> -        ret = kvm_vm_check_extension(s, KVM_CAP_DIRTY_LOG_RING);
>> -        if (ret > 0) {
>> -            if (ring_bytes > ret) {
>> -                error_report("KVM dirty ring size %" PRIu32 " too big "
>> -                             "(maximum is %ld).  Please use a smaller value.",
>> -                             s->kvm_dirty_ring_size,
>> -                             (long)ret / sizeof(struct kvm_dirty_gfn));
>> -                ret = -EINVAL;
>> -                goto err;
>> -            }
>> -
>> -            ret = kvm_vm_enable_cap(s, KVM_CAP_DIRTY_LOG_RING, 0, ring_bytes);
>> -            if (ret) {
>> -                error_report("Enabling of KVM dirty ring failed: %s. "
>> -                             "Suggested minimum value is 1024.", strerror(-ret));
>> -                goto err;
>> -            }
>> -
>> -            s->kvm_dirty_ring_bytes = ring_bytes;
>> -         } else {
>> -             warn_report("KVM dirty ring not available, using bitmap method");
>> -             s->kvm_dirty_ring_size = 0;
>> +        ret = kvm_dirty_ring_init(s);
>> +        if (ret < 0) {
>> +            goto err;
>>           }
>>       }
> 
Thanks,
Gavin

Re: [PATCH RFCv1 2/8] memory: Add last stage indicator to global dirty log synchronization

[Peter Xu]

On Mon, Feb 06, 2023 at 07:20:04PM +0800, Gavin Shan wrote:
> The global dirty log synchronization is used when KVM and dirty ring
> are enabled. There is a particularity for ARM64 where the backup
> bitmap is used to track dirty pages in non-running-vcpu situations.
> It means the dirty ring works with the combination of ring buffer
> and backup bitmap. The dirty bits in the backup bitmap needs to
> collected in the last stage of live migration.
> 
> In order to identify the last stage of live migration and pass it
> down, an extra parameter is added to the relevant functions and
> callback. This last stage information isn't used yet.
> 
> No functional change intended.
> 
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
>  accel/kvm/kvm-all.c   |  2 +-
>  include/exec/memory.h |  5 +++--
>  migration/dirtyrate.c |  4 ++--
>  migration/ram.c       |  6 +++---

Better move the migration/ changes into the next patch.

Thanks,

-- 
Peter Xu

Re: [PATCH RFCv1 2/8] memory: Add last stage indicator to global dirty log synchronization

[Gavin Shan]

On 2/10/23 6:48 AM, Peter Xu wrote:
> On Mon, Feb 06, 2023 at 07:20:04PM +0800, Gavin Shan wrote:
>> The global dirty log synchronization is used when KVM and dirty ring
>> are enabled. There is a particularity for ARM64 where the backup
>> bitmap is used to track dirty pages in non-running-vcpu situations.
>> It means the dirty ring works with the combination of ring buffer
>> and backup bitmap. The dirty bits in the backup bitmap needs to
>> collected in the last stage of live migration.
>>
>> In order to identify the last stage of live migration and pass it
>> down, an extra parameter is added to the relevant functions and
>> callback. This last stage information isn't used yet.
>>
>> No functional change intended.
>>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>>   accel/kvm/kvm-all.c   |  2 +-
>>   include/exec/memory.h |  5 +++--
>>   migration/dirtyrate.c |  4 ++--
>>   migration/ram.c       |  6 +++---
> 
> Better move the migration/ changes into the next patch.
> 

Ok, I will combine PATCH[2/8] and PATCH[3/8] in next revision.

Thanks,
Gavin