[RFC PATCH v2 0/3] accel/kvm: extend kvm memory listener to support

[RFC PATCH v2 0/3] accel/kvm: extend kvm memory listener to support

[Emanuele Giuseppe Esposito]

The aim of this serie is to prepare kvm memory listener to support atomic
memslots update. In order to do that, QEMU should take care of sending all
memslot updates in a single ioctl, so that they can all be processed
atomically.

In order to do that, implement kml->begin() and kml->commit() callbacks, and
change the logic by replacing every ioctl invocation in ->region_* and ->log_*
so that the struct kvm_userspace_memory_region are queued in a linked list that
is then traversed and processed in ->commit.

Patch 1 ensures that ->region_* and ->log_* are always wrapped by ->begin and
->commit.

---
v2:
- remove patch 1, as it is useless
- patch 2: instead of a linked list, use kvm_userspace_memory_region_list
- kvm_userspace_memory_region_list: add padding

Emanuele Giuseppe Esposito (3):
  linux-headers/linux/kvm.h: introduce kvm_userspace_memory_region_list
    ioctl
  accel/kvm/kvm-all.c: pass kvm_userspace_memory_region_entry instead
  kvm/kvm-all.c: listener should delay kvm_vm_ioctl to the commit phase

 accel/kvm/kvm-all.c       | 116 +++++++++++++++++++++++++++++---------
 include/sysemu/kvm_int.h  |   8 +++
 linux-headers/linux/kvm.h |  20 +++++++
 3 files changed, 117 insertions(+), 27 deletions(-)

-- 
2.31.1

[RFC PATCH v2 1/3] linux-headers/linux/kvm.h: introduce kvm_userspace_memory_region_list ioctl

[Emanuele Giuseppe Esposito]

Introduce new KVM_SET_USER_MEMORY_REGION_LIST ioctl and
kvm_userspace_memory_region_list that will be used to pass
multiple memory region updates at once to KVM.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 linux-headers/linux/kvm.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index f089349149..671cdfb8de 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -103,6 +103,24 @@ struct kvm_userspace_memory_region {
 	__u64 userspace_addr; /* start of the userspace allocated memory */
 };
 
+/* for KVM_SET_USER_MEMORY_REGION_LIST */
+struct kvm_userspace_memory_region_entry {
+	__u32 slot;
+	__u32 flags;
+	__u64 guest_phys_addr;
+	__u64 memory_size; /* bytes */
+	__u64 userspace_addr; /* start of the userspace allocated memory */
+	__u8 invalidate_slot;
+	__u8 padding[31];
+};
+
+/* for KVM_SET_USER_MEMORY_REGION_LIST */
+struct kvm_userspace_memory_region_list {
+	__u32 nent;
+	__u32 flags;
+	struct kvm_userspace_memory_region_entry entries[0];
+};
+
 /*
  * The bit 0 ~ bit 15 of kvm_memory_region::flags are visible for userspace,
  * other bits are reserved for kvm internal use which are defined in
@@ -1426,6 +1444,8 @@ struct kvm_vfio_spapr_tce {
 					struct kvm_userspace_memory_region)
 #define KVM_SET_TSS_ADDR          _IO(KVMIO,   0x47)
 #define KVM_SET_IDENTITY_MAP_ADDR _IOW(KVMIO,  0x48, __u64)
+#define KVM_SET_USER_MEMORY_REGION_LIST _IOW(KVMIO, 0x49, \
+					struct kvm_userspace_memory_region_list)
 
 /* enable ucontrol for s390 */
 struct kvm_s390_ucas_mapping {
-- 
2.31.1

[RFC PATCH v2 2/3] accel/kvm/kvm-all.c: pass kvm_userspace_memory_region_entry instead

[Emanuele Giuseppe Esposito]

It won't change anything from the kernel side, but prepares the logic
for KVM_SET_USER_MEMORY_REGION_LIST ioctl, where all requests are sent
at once.

Because QEMU does not send any memslot MOVE request to KVM, simplify
mem.invalidate_slot logic to only detect DELETE requests.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 accel/kvm/kvm-all.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 645f0a249a..e9947ec18b 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -360,7 +360,7 @@ int kvm_physical_memory_addr_from_host(KVMState *s, void *ram,
 static int kvm_set_user_memory_region(KVMMemoryListener *kml, KVMSlot *slot, bool new)
 {
     KVMState *s = kvm_state;
-    struct kvm_userspace_memory_region mem;
+    struct kvm_userspace_memory_region_entry mem;
     int ret;
 
     mem.slot = slot->slot | (kml->as_id << 16);
@@ -372,12 +372,29 @@ static int kvm_set_user_memory_region(KVMMemoryListener *kml, KVMSlot *slot, boo
         /* Set the slot size to 0 before setting the slot to the desired
          * value. This is needed based on KVM commit 75d61fbc. */
         mem.memory_size = 0;
+        mem.invalidate_slot = 1;
+        /*
+         * Note that mem is struct kvm_userspace_memory_region_entry, while the
+         * kernel expects a kvm_userspace_memory_region, so it will currently
+         * ignore mem->invalidate_slot and mem->padding.
+         */
         ret = kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem);
         if (ret < 0) {
             goto err;
         }
     }
     mem.memory_size = slot->memory_size;
+    /*
+     * Invalidate if it's a kvm memslot MOVE or DELETE operation, but
+     * currently QEMU does not perform any memslot MOVE operation.
+     */
+    mem.invalidate_slot = slot->memory_size == 0;
+
+    /*
+     * Note that mem is struct kvm_userspace_memory_region_entry, while the
+     * kernel expects a kvm_userspace_memory_region, so it will currently
+     * ignore mem->invalidate_slot and mem->padding.
+     */
     ret = kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem);
     slot->old_flags = mem.flags;
 err:
-- 
2.31.1

[RFC PATCH v2 3/3] kvm/kvm-all.c: listener should delay kvm_vm_ioctl to the commit phase

[Emanuele Giuseppe Esposito]

Instead of sending a single ioctl every time ->region_* or ->log_*
callbacks are called, "queue" all memory regions in a
kvm_userspace_memory_region_list that will be sent only when committing.

This allow the KVM kernel API to be extended and support multiple
memslots updates in a single call.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 accel/kvm/kvm-all.c      | 131 ++++++++++++++++++++++++++-------------
 include/sysemu/kvm_int.h |   8 +++
 2 files changed, 96 insertions(+), 43 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index e9947ec18b..9780f3d2da 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -357,56 +357,50 @@ int kvm_physical_memory_addr_from_host(KVMState *s, void *ram,
     return ret;
 }
 
+static struct kvm_userspace_memory_region_entry *kvm_memory_region_entry_get(
+                                                        KVMMemoryListener *kml)
+{
+    struct MemoryRegionNodeArray *arr = &kml->mem_array;
+    struct kvm_userspace_memory_region_list *list = arr->list;
+
+    if (list->nent == arr->max_entries) {
+        arr->max_entries += DEFAULT_KVM_MEMORY_REGION_ARRAY_GROW;
+        list = g_realloc(list,
+                              sizeof(struct kvm_userspace_memory_region_list) +
+                              arr->max_entries *
+                              sizeof(struct kvm_userspace_memory_region_entry));
+    }
+
+    return &list->entries[list->nent++];
+}
+
 static int kvm_set_user_memory_region(KVMMemoryListener *kml, KVMSlot *slot, bool new)
 {
-    KVMState *s = kvm_state;
-    struct kvm_userspace_memory_region_entry mem;
-    int ret;
+    struct kvm_userspace_memory_region_entry *mem;
+
+    mem = kvm_memory_region_entry_get(kml);
+
+    mem->slot = slot->slot | (kml->as_id << 16);
+    mem->guest_phys_addr = slot->start_addr;
+    mem->userspace_addr = (unsigned long)slot->ram;
+    mem->flags = slot->flags;
 
-    mem.slot = slot->slot | (kml->as_id << 16);
-    mem.guest_phys_addr = slot->start_addr;
-    mem.userspace_addr = (unsigned long)slot->ram;
-    mem.flags = slot->flags;
+    if (slot->memory_size && !new && (mem->flags ^ slot->old_flags) &
+        KVM_MEM_READONLY) {
+        struct kvm_userspace_memory_region_entry *mem2 = mem;
 
-    if (slot->memory_size && !new && (mem.flags ^ slot->old_flags) & KVM_MEM_READONLY) {
+        mem = kvm_memory_region_entry_get(kml);
+        memcpy(mem, mem2, sizeof(struct kvm_userspace_memory_region_entry));
         /* Set the slot size to 0 before setting the slot to the desired
          * value. This is needed based on KVM commit 75d61fbc. */
-        mem.memory_size = 0;
-        mem.invalidate_slot = 1;
-        /*
-         * Note that mem is struct kvm_userspace_memory_region_entry, while the
-         * kernel expects a kvm_userspace_memory_region, so it will currently
-         * ignore mem->invalidate_slot and mem->padding.
-         */
-        ret = kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem);
-        if (ret < 0) {
-            goto err;
-        }
+        mem2->memory_size = 0;
+        mem2->invalidate_slot = 1;
     }
-    mem.memory_size = slot->memory_size;
-    /*
-     * Invalidate if it's a kvm memslot MOVE or DELETE operation, but
-     * currently QEMU does not perform any memslot MOVE operation.
-     */
-    mem.invalidate_slot = slot->memory_size == 0;
+    mem->memory_size = slot->memory_size;
+    mem->invalidate_slot = slot->memory_size == 0;
 
-    /*
-     * Note that mem is struct kvm_userspace_memory_region_entry, while the
-     * kernel expects a kvm_userspace_memory_region, so it will currently
-     * ignore mem->invalidate_slot and mem->padding.
-     */
-    ret = kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem);
-    slot->old_flags = mem.flags;
-err:
-    trace_kvm_set_user_memory(mem.slot, mem.flags, mem.guest_phys_addr,
-                              mem.memory_size, mem.userspace_addr, ret);
-    if (ret < 0) {
-        error_report("%s: KVM_SET_USER_MEMORY_REGION failed, slot=%d,"
-                     " start=0x%" PRIx64 ", size=0x%" PRIx64 ": %s",
-                     __func__, mem.slot, slot->start_addr,
-                     (uint64_t)mem.memory_size, strerror(errno));
-    }
-    return ret;
+    slot->old_flags = mem->flags;
+    return 0;
 }
 
 static int do_kvm_destroy_vcpu(CPUState *cpu)
@@ -1534,12 +1528,54 @@ static void kvm_region_add(MemoryListener *listener,
 static void kvm_region_del(MemoryListener *listener,
                            MemoryRegionSection *section)
 {
-    KVMMemoryListener *kml = container_of(listener, KVMMemoryListener, listener);
+    KVMMemoryListener *kml = container_of(listener, KVMMemoryListener,
+                                          listener);
 
     kvm_set_phys_mem(kml, section, false);
     memory_region_unref(section->mr);
 }
 
+static void kvm_begin(MemoryListener *listener)
+{
+    KVMMemoryListener *kml = container_of(listener, KVMMemoryListener,
+                                          listener);
+    assert(kml->mem_array.list->nent == 0);
+}
+
+static void kvm_commit(MemoryListener *listener)
+{
+    KVMMemoryListener *kml = container_of(listener, KVMMemoryListener,
+                                          listener);
+    KVMState *s = kvm_state;
+    int i;
+
+    for (i = 0; i < kml->mem_array.list->nent; i++) {
+        struct kvm_userspace_memory_region_entry *mem;
+        int ret;
+
+        mem = &kml->mem_array.list->entries[i];
+
+        /*
+         * Note that mem is struct kvm_userspace_memory_region_entry, while the
+         * kernel expects a kvm_userspace_memory_region, so it will currently
+         * ignore mem->invalidate_slot and mem->padding.
+         */
+        ret = kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, mem);
+
+        trace_kvm_set_user_memory(mem->slot, mem->flags, mem->guest_phys_addr,
+                                  mem->memory_size, mem->userspace_addr, 0);
+
+        if (ret < 0) {
+            error_report("%s: KVM_SET_USER_MEMORY_REGION failed, slot=%d,"
+                         " start=0x%" PRIx64 ": %s",
+                         __func__, mem->slot,
+                         (uint64_t)mem->memory_size, strerror(errno));
+        }
+    }
+
+    kml->mem_array.list->nent = 0;
+}
+
 static void kvm_log_sync(MemoryListener *listener,
                          MemoryRegionSection *section)
 {
@@ -1681,8 +1717,16 @@ void kvm_memory_listener_register(KVMState *s, KVMMemoryListener *kml,
         kml->slots[i].slot = i;
     }
 
+    kml->mem_array.max_entries = DEFAULT_KVM_MEMORY_REGION_ARRAY_GROW;
+    kml->mem_array.list = g_malloc0(
+                            sizeof(struct kvm_userspace_memory_region_list) +
+                            sizeof(struct kvm_userspace_memory_region_entry) *
+                            kml->mem_array.max_entries);
+
     kml->listener.region_add = kvm_region_add;
     kml->listener.region_del = kvm_region_del;
+    kml->listener.begin = kvm_begin;
+    kml->listener.commit = kvm_commit;
     kml->listener.log_start = kvm_log_start;
     kml->listener.log_stop = kvm_log_stop;
     kml->listener.priority = 10;
@@ -2691,6 +2735,7 @@ err:
         close(s->fd);
     }
     g_free(s->memory_listener.slots);
+    g_free(s->memory_listener.mem_array.list);
 
     return ret;
 }
diff --git a/include/sysemu/kvm_int.h b/include/sysemu/kvm_int.h
index 1f5487d9b7..1adc1c8722 100644
--- a/include/sysemu/kvm_int.h
+++ b/include/sysemu/kvm_int.h
@@ -30,9 +30,17 @@ typedef struct KVMSlot
     ram_addr_t ram_start_offset;
 } KVMSlot;
 
+#define DEFAULT_KVM_MEMORY_REGION_ARRAY_GROW 10
+
+struct MemoryRegionNodeArray {
+    struct kvm_userspace_memory_region_list *list;
+    int max_entries;
+};
+
 typedef struct KVMMemoryListener {
     MemoryListener listener;
     KVMSlot *slots;
+    struct MemoryRegionNodeArray mem_array;
     int as_id;
 } KVMMemoryListener;
 
-- 
2.31.1