[Qemu-devel] [PATCH RFC v2 0/4] s390: stop abusing memory_region_allocate_system_memory()

[Qemu-devel] [PATCH RFC v2 0/4] s390: stop abusing memory_region_allocate_system_memory()

[Igor Mammedov]

Changelog:
  since v1:
    - include 4.2 machines patch for adding compat RAM layout on top
    - 2/4 add missing in v1 patch for splitting too big MemorySection on
          several memslots
    - 3/4 amend code path on alias destruction to ensure that RAMBlock is
          cleaned properly
    - 4/4 add compat machine code to keep old layout (migration-wise) for
          4.1 and older machines 


While looking into unifying guest RAM allocation to use hostmem backends
for initial RAM (especially when -mempath is used) and retiring
memory_region_allocate_system_memory() API, leaving only single hostmem backend,
I was inspecting how currently it is used by boards and it turns out several
boards abuse it by calling the function several times (despite documented contract
forbiding it).

s390 is one of such boards where KVM limitation on memslot size got propagated
to board design and memory_region_allocate_system_memory() was abused to satisfy
KVM requirement for max RAM chunk where memory region alias would suffice.

Unfortunately, memory_region_allocate_system_memory() usage created migration
dependency where guest RAM is transferred in migration stream as several RAMBlocks
if it's more than KVM_SLOT_MAX_BYTES.

In order to replace these several RAM chunks with a single memdev and keep it
working with KVM memslot size limit and migration compatible, following was done:
   * [2/4] split too big RAM chunk inside of KVM code on several memory slots
           if necessary
   * [4/4] use memory region aliases to partition hostmem backend RAM on
           KVM_SLOT_MAX_BYTES chunks, which should keep KVM side working
   * [3/4] hacked memory region aliases (to ram memory regions only) to have
           its own RAMBlocks pointing to RAM chunks owned by aliased memory
           region. While it's admittedly a hack, but it's relatively simple and
           allows board code re-shape migration stream as necessary

           I haven't tried to use migratable aliases on x86 machines, but with it
           it could be possible to drop legacy RAM allocation and compat knob
           (cd5ff8333a) dropping '-numa node,mem' completely even for old machines.

PS:
   Tested with ping pong cross version migration on s390 machine and x86 pc/q35
   (with reduced KVM_SLOT_MAX_BYTES since I don't have access to large
    enough host)

CC: pbonzini@redhat.com
CC: qemu-s390x@nongnu.org
CC: borntraeger@de.ibm.com
CC: thuth@redhat.com
CC: david@redhat.com
CC: cohuck@redhat.com


Cornelia Huck (1):
  hw: add compat machines for 4.2

Igor Mammedov (3):
  kvm: s390: split too big memory section on several memslots
  memory: make MemoryRegion alias migratable
  s390: do not call memory_region_allocate_system_memory() multiple
    times

 include/hw/boards.h                |  3 ++
 include/hw/i386/pc.h               |  3 ++
 include/hw/s390x/s390-virtio-ccw.h | 14 ++++++
 include/sysemu/kvm_int.h           |  1 +
 accel/kvm/kvm-all.c                | 77 +++++++++++++++++------------
 exec.c                             |  9 ++--
 hw/arm/virt.c                      |  9 +++-
 hw/core/machine.c                  |  3 ++
 hw/i386/pc.c                       |  3 ++
 hw/i386/pc_piix.c                  | 14 +++++-
 hw/i386/pc_q35.c                   | 13 ++++-
 hw/ppc/spapr.c                     | 15 +++++-
 hw/s390x/s390-virtio-ccw.c         | 79 ++++++++++++++++++++----------
 memory.c                           |  6 +++
 target/s390x/kvm.c                 |  1 +
 15 files changed, 183 insertions(+), 67 deletions(-)

-- 
2.18.1

[Qemu-devel] [PATCH RFC v2 1/4] hw: add compat machines for 4.2

[Igor Mammedov]

From: Cornelia Huck <cohuck@redhat.com>

Add 4.2 machine types for arm/i440fx/q35/s390x/spapr.

For i440fx and q35, unversioned cpu models are still translated
to -v1, as 0788a56bd1ae ("i386: Make unversioned CPU models be
aliases") states this should only transition to the latest cpu
model version in 4.3 (or later).

Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 include/hw/boards.h        |  3 +++
 include/hw/i386/pc.h       |  3 +++
 hw/arm/virt.c              |  9 ++++++++-
 hw/core/machine.c          |  3 +++
 hw/i386/pc.c               |  3 +++
 hw/i386/pc_piix.c          | 14 +++++++++++++-
 hw/i386/pc_q35.c           | 13 ++++++++++++-
 hw/ppc/spapr.c             | 15 +++++++++++++--
 hw/s390x/s390-virtio-ccw.c | 14 +++++++++++++-
 9 files changed, 71 insertions(+), 6 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index a71d1a53a5..d9ec37d807 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -317,6 +317,9 @@ struct MachineState {
     } \
     type_init(machine_initfn##_register_types)
 
+extern GlobalProperty hw_compat_4_1[];
+extern const size_t hw_compat_4_1_len;
+
 extern GlobalProperty hw_compat_4_0[];
 extern const size_t hw_compat_4_0_len;
 
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 859b64c51d..c840e39251 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -302,6 +302,9 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
 int e820_get_num_entries(void);
 bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
 
+extern GlobalProperty pc_compat_4_1[];
+extern const size_t pc_compat_4_1_len;
+
 extern GlobalProperty pc_compat_4_0[];
 extern const size_t pc_compat_4_0_len;
 
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index d9496c9363..64e3fc3440 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2046,10 +2046,17 @@ static void machvirt_machine_init(void)
 }
 type_init(machvirt_machine_init);
 
+static void virt_machine_4_2_options(MachineClass *mc)
+{
+}
+DEFINE_VIRT_MACHINE_AS_LATEST(4, 2)
+
 static void virt_machine_4_1_options(MachineClass *mc)
 {
+    virt_machine_4_2_options(mc);
+    compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
 }
-DEFINE_VIRT_MACHINE_AS_LATEST(4, 1)
+DEFINE_VIRT_MACHINE(4, 1)
 
 static void virt_machine_4_0_options(MachineClass *mc)
 {
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 28a475ad97..fd76dad859 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -27,6 +27,9 @@
 #include "hw/pci/pci.h"
 #include "hw/mem/nvdimm.h"
 
+GlobalProperty hw_compat_4_1[] = {};
+const size_t hw_compat_4_1_len = G_N_ELEMENTS(hw_compat_4_1);
+
 GlobalProperty hw_compat_4_0[] = {
     { "VGA",            "edid", "false" },
     { "secondary-vga",  "edid", "false" },
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 549c437050..2aa1c49701 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -116,6 +116,9 @@ struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
 /* Physical Address of PVH entry point read from kernel ELF NOTE */
 static size_t pvh_start_addr;
 
+GlobalProperty pc_compat_4_1[] = {};
+const size_t pc_compat_4_1_len = G_N_ELEMENTS(pc_compat_4_1);
+
 GlobalProperty pc_compat_4_0[] = {};
 const size_t pc_compat_4_0_len = G_N_ELEMENTS(pc_compat_4_0);
 
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index c2280c72ef..24e71be1d2 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -433,7 +433,7 @@ static void pc_i440fx_machine_options(MachineClass *m)
     machine_class_allow_dynamic_sysbus_dev(m, TYPE_RAMFB_DEVICE);
 }
 
-static void pc_i440fx_4_1_machine_options(MachineClass *m)
+static void pc_i440fx_4_2_machine_options(MachineClass *m)
 {
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     pc_i440fx_machine_options(m);
@@ -442,6 +442,18 @@ static void pc_i440fx_4_1_machine_options(MachineClass *m)
     pcmc->default_cpu_version = 1;
 }
 
+DEFINE_I440FX_MACHINE(v4_2, "pc-i440fx-4.2", NULL,
+                      pc_i440fx_4_2_machine_options);
+
+static void pc_i440fx_4_1_machine_options(MachineClass *m)
+{
+    pc_i440fx_4_2_machine_options(m);
+    m->alias = NULL;
+    m->is_default = 0;
+    compat_props_add(m->compat_props, hw_compat_4_1, hw_compat_4_1_len);
+    compat_props_add(m->compat_props, pc_compat_4_1, pc_compat_4_1_len);
+}
+
 DEFINE_I440FX_MACHINE(v4_1, "pc-i440fx-4.1", NULL,
                       pc_i440fx_4_1_machine_options);
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 397e1fdd2f..e61260762c 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -365,7 +365,7 @@ static void pc_q35_machine_options(MachineClass *m)
     m->max_cpus = 288;
 }
 
-static void pc_q35_4_1_machine_options(MachineClass *m)
+static void pc_q35_4_2_machine_options(MachineClass *m)
 {
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     pc_q35_machine_options(m);
@@ -373,6 +373,17 @@ static void pc_q35_4_1_machine_options(MachineClass *m)
     pcmc->default_cpu_version = 1;
 }
 
+DEFINE_Q35_MACHINE(v4_2, "pc-q35-4.2", NULL,
+                   pc_q35_4_2_machine_options);
+
+static void pc_q35_4_1_machine_options(MachineClass *m)
+{
+    pc_q35_4_2_machine_options(m);
+    m->alias = NULL;
+    compat_props_add(m->compat_props, hw_compat_4_1, hw_compat_4_1_len);
+    compat_props_add(m->compat_props, pc_compat_4_1, pc_compat_4_1_len);
+}
+
 DEFINE_Q35_MACHINE(v4_1, "pc-q35-4.1", NULL,
                    pc_q35_4_1_machine_options);
 
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 821f0d4a49..6c8aa60545 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4426,15 +4426,26 @@ static const TypeInfo spapr_machine_info = {
     }                                                                \
     type_init(spapr_machine_register_##suffix)
 
+/*
+ * pseries-4.2
+ */
+static void spapr_machine_4_2_class_options(MachineClass *mc)
+{
+    /* Defaults for the latest behaviour inherited from the base class */
+}
+
+DEFINE_SPAPR_MACHINE(4_2, "4.2", true);
+
 /*
  * pseries-4.1
  */
 static void spapr_machine_4_1_class_options(MachineClass *mc)
 {
-    /* Defaults for the latest behaviour inherited from the base class */
+    spapr_machine_4_2_class_options(mc);
+    compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
 }
 
-DEFINE_SPAPR_MACHINE(4_1, "4.1", true);
+DEFINE_SPAPR_MACHINE(4_1, "4.1", false);
 
 /*
  * pseries-4.0
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 5b6a9a4e55..593b34e0e2 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -660,14 +660,26 @@ bool css_migration_enabled(void)
     }                                                                         \
     type_init(ccw_machine_register_##suffix)
 
+static void ccw_machine_4_2_instance_options(MachineState *machine)
+{
+}
+
+static void ccw_machine_4_2_class_options(MachineClass *mc)
+{
+}
+DEFINE_CCW_MACHINE(4_2, "4.2", true);
+
 static void ccw_machine_4_1_instance_options(MachineState *machine)
 {
+    ccw_machine_4_2_instance_options(machine);
 }
 
 static void ccw_machine_4_1_class_options(MachineClass *mc)
 {
+    ccw_machine_4_2_class_options(mc);
+    compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
 }
-DEFINE_CCW_MACHINE(4_1, "4.1", true);
+DEFINE_CCW_MACHINE(4_1, "4.1", false);
 
 static void ccw_machine_4_0_instance_options(MachineState *machine)
 {
-- 
2.18.1

[Qemu-devel] [PATCH RFC v2 2/4] kvm: s390: split too big memory section on several memslots

[Igor Mammedov]

Max memslot size supported by kvm on s390 is 8Tb,
move logic of splitting RAM on chunks upto 8T to KVM code.

This way it will hide KVM specific restrictions in KVM code
and won't affect baord level design decisions. Which would allow
us to avoid misusing memory_region_allocate_system_memory() API
and eventually use a single hostmem backend for guest RAM.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
patch prepares only KVM side for switching to single RAM memory region
other patche will take care of migration issues that improper use
of memory_region_allocate_system_memory() introduced
---
 include/hw/s390x/s390-virtio-ccw.h | 10 ++++
 include/sysemu/kvm_int.h           |  1 +
 accel/kvm/kvm-all.c                | 77 ++++++++++++++++++------------
 hw/s390x/s390-virtio-ccw.c         |  9 ----
 target/s390x/kvm.c                 |  1 +
 5 files changed, 58 insertions(+), 40 deletions(-)

diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
index 8aa27199c9..00632f94b4 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -21,6 +21,16 @@
 #define S390_MACHINE_CLASS(klass) \
     OBJECT_CLASS_CHECK(S390CcwMachineClass, (klass), TYPE_S390_CCW_MACHINE)
 
+/*
+ * KVM does only support memory slots up to KVM_MEM_MAX_NR_PAGES pages
+ * as the dirty bitmap must be managed by bitops that take an int as
+ * position indicator. If we have a guest beyond that we will split off
+ * new subregions. The split must happen on a segment boundary (1MB).
+ */
+#define KVM_MEM_MAX_NR_PAGES ((1ULL << 31) - 1)
+#define SEG_MSK (~0xfffffULL)
+#define KVM_SLOT_MAX_BYTES ((KVM_MEM_MAX_NR_PAGES * TARGET_PAGE_SIZE) & SEG_MSK)
+
 typedef struct S390CcwMachineState {
     /*< private >*/
     MachineState parent_obj;
diff --git a/include/sysemu/kvm_int.h b/include/sysemu/kvm_int.h
index 31df465fdc..7f7520bce2 100644
--- a/include/sysemu/kvm_int.h
+++ b/include/sysemu/kvm_int.h
@@ -41,4 +41,5 @@ typedef struct KVMMemoryListener {
 void kvm_memory_listener_register(KVMState *s, KVMMemoryListener *kml,
                                   AddressSpace *as, int as_id);
 
+void kvm_set_max_memslot_size(hwaddr max_slot_size);
 #endif
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index f450f25295..9a5ee0dc4f 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -138,6 +138,7 @@ bool kvm_direct_msi_allowed;
 bool kvm_ioeventfd_any_length_allowed;
 bool kvm_msi_use_devid;
 static bool kvm_immediate_exit;
+static hwaddr kvm_max_slot_size = ~0;
 
 static const KVMCapabilityInfo kvm_required_capabilites[] = {
     KVM_CAP_INFO(USER_MEMORY),
@@ -951,6 +952,12 @@ kvm_check_extension_list(KVMState *s, const KVMCapabilityInfo *list)
     return NULL;
 }
 
+void kvm_set_max_memslot_size(hwaddr max_slot_size)
+{
+    g_assert(ROUND_UP(max_slot_size, qemu_real_host_page_size) == max_slot_size);
+    kvm_max_slot_size = max_slot_size;
+}
+
 static void kvm_set_phys_mem(KVMMemoryListener *kml,
                              MemoryRegionSection *section, bool add)
 {
@@ -958,7 +965,7 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
     int err;
     MemoryRegion *mr = section->mr;
     bool writeable = !mr->readonly && !mr->rom_device;
-    hwaddr start_addr, size;
+    hwaddr start_addr, size, slot_size;
     void *ram;
 
     if (!memory_region_is_ram(mr)) {
@@ -983,41 +990,49 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
     kvm_slots_lock(kml);
 
     if (!add) {
-        mem = kvm_lookup_matching_slot(kml, start_addr, size);
-        if (!mem) {
-            goto out;
-        }
-        if (mem->flags & KVM_MEM_LOG_DIRTY_PAGES) {
-            kvm_physical_sync_dirty_bitmap(kml, section);
-        }
+        do {
+            slot_size = kvm_max_slot_size < size ? kvm_max_slot_size : size;
+            mem = kvm_lookup_matching_slot(kml, start_addr, slot_size);
+            if (!mem) {
+                goto out;
+            }
+            if (mem->flags & KVM_MEM_LOG_DIRTY_PAGES) {
+                kvm_physical_sync_dirty_bitmap(kml, section);
+            }
 
-        /* unregister the slot */
-        g_free(mem->dirty_bmap);
-        mem->dirty_bmap = NULL;
-        mem->memory_size = 0;
-        mem->flags = 0;
-        err = kvm_set_user_memory_region(kml, mem, false);
-        if (err) {
-            fprintf(stderr, "%s: error unregistering slot: %s\n",
-                    __func__, strerror(-err));
-            abort();
-        }
+            /* unregister the slot */
+            g_free(mem->dirty_bmap);
+            mem->dirty_bmap = NULL;
+            mem->memory_size = 0;
+            mem->flags = 0;
+            err = kvm_set_user_memory_region(kml, mem, false);
+            if (err) {
+                fprintf(stderr, "%s: error unregistering slot: %s\n",
+                        __func__, strerror(-err));
+                abort();
+            }
+            start_addr += slot_size;
+        } while ((size -= slot_size));
         goto out;
     }
 
     /* register the new slot */
-    mem = kvm_alloc_slot(kml);
-    mem->memory_size = size;
-    mem->start_addr = start_addr;
-    mem->ram = ram;
-    mem->flags = kvm_mem_flags(mr);
-
-    err = kvm_set_user_memory_region(kml, mem, true);
-    if (err) {
-        fprintf(stderr, "%s: error registering slot: %s\n", __func__,
-                strerror(-err));
-        abort();
-    }
+    do {
+        slot_size = kvm_max_slot_size < size ? kvm_max_slot_size : size;
+        mem = kvm_alloc_slot(kml);
+        mem->memory_size = slot_size;
+        mem->start_addr = start_addr;
+        mem->ram = ram;
+        mem->flags = kvm_mem_flags(mr);
+
+        err = kvm_set_user_memory_region(kml, mem, true);
+        if (err) {
+            fprintf(stderr, "%s: error registering slot: %s\n", __func__,
+                    strerror(-err));
+            abort();
+        }
+        start_addr += slot_size;
+    } while ((size -= slot_size));
 
 out:
     kvm_slots_unlock(kml);
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 593b34e0e2..073672f9cb 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -151,15 +151,6 @@ static void virtio_ccw_register_hcalls(void)
                                    virtio_ccw_hcall_early_printk);
 }
 
-/*
- * KVM does only support memory slots up to KVM_MEM_MAX_NR_PAGES pages
- * as the dirty bitmap must be managed by bitops that take an int as
- * position indicator. If we have a guest beyond that we will split off
- * new subregions. The split must happen on a segment boundary (1MB).
- */
-#define KVM_MEM_MAX_NR_PAGES ((1ULL << 31) - 1)
-#define SEG_MSK (~0xfffffULL)
-#define KVM_SLOT_MAX_BYTES ((KVM_MEM_MAX_NR_PAGES * TARGET_PAGE_SIZE) & SEG_MSK)
 static void s390_memory_init(ram_addr_t mem_size)
 {
     MemoryRegion *sysmem = get_system_memory();
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 6e814c230b..d0267da8e1 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -347,6 +347,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
      */
     /* kvm_vm_enable_cap(s, KVM_CAP_S390_AIS, 0); */
 
+    kvm_set_max_memslot_size(KVM_SLOT_MAX_BYTES);
     return 0;
 }
 
-- 
2.18.1

[Qemu-devel] [PATCH RFC v2 3/4] memory: make MemoryRegion alias migratable

[Igor Mammedov]

use qemu_ram_alloc_from_ptr() to create aliased RAMBlock
to the part of original memory region.

Change is migration safe as we do not migrate every existing RAMBlock
anymore, to make it migratable code has to explicitly call
vmstate_register_ram() on MemoryRegion that owns RAMBlock.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
PS:
tested ping-pong migration between new and old QEMU for x86 pc/q35
and s390 machines.

CC: dgilbert@redhat.com

 exec.c   | 9 +++++----
 memory.c | 6 ++++++
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/exec.c b/exec.c
index 3e78de3b8f..f5e9699632 100644
--- a/exec.c
+++ b/exec.c
@@ -2313,7 +2313,7 @@ static void ram_block_add(RAMBlock *new_block, Error **errp, bool shared)
                                         new_block->used_length,
                                         DIRTY_CLIENTS_ALL);
 
-    if (new_block->host) {
+    if (new_block->host && !new_block->mr->alias) {
         qemu_ram_setup_dump(new_block->host, new_block->max_length);
         qemu_madvise(new_block->host, new_block->max_length, QEMU_MADV_HUGEPAGE);
         /* MADV_DONTFORK is also needed by KVM in absence of synchronous MMU */
@@ -2497,7 +2497,7 @@ void qemu_ram_free(RAMBlock *block)
         return;
     }
 
-    if (block->host) {
+    if (block->host && !block->mr->alias) {
         ram_block_notify_remove(block->host, block->max_length);
     }
 
@@ -2671,7 +2671,8 @@ RAMBlock *qemu_ram_block_from_host(void *ptr, bool round_offset,
 
     rcu_read_lock();
     block = atomic_rcu_read(&ram_list.mru_block);
-    if (block && block->host && host - block->host < block->max_length) {
+    if (block && !block->mr->alias && block->host &&
+        host - block->host < block->max_length) {
         goto found;
     }
 
@@ -2680,7 +2681,7 @@ RAMBlock *qemu_ram_block_from_host(void *ptr, bool round_offset,
         if (block->host == NULL) {
             continue;
         }
-        if (host - block->host < block->max_length) {
+        if (!block->mr->alias && host - block->host < block->max_length) {
             goto found;
         }
     }
diff --git a/memory.c b/memory.c
index 5d8c9a9234..d7c3609ce3 100644
--- a/memory.c
+++ b/memory.c
@@ -1678,6 +1678,12 @@ void memory_region_init_alias(MemoryRegion *mr,
     memory_region_init(mr, owner, name, size);
     mr->alias = orig;
     mr->alias_offset = offset;
+    if (orig->ram_block && size) {
+        mr->destructor = memory_region_destructor_ram;
+        mr->ram_block = qemu_ram_alloc_from_ptr(size,
+                                                orig->ram_block->host + offset,
+                                                mr, &error_fatal);
+    }
 }
 
 void memory_region_init_rom_nomigrate(MemoryRegion *mr,
-- 
2.18.1

[Qemu-devel] [PATCH RFC v2 4/4] s390: do not call memory_region_allocate_system_memory() multiple times

[Igor Mammedov]

s390 was trying to solve limited KVM memslot size issue by abusing
memory_region_allocate_system_memory(), which breaks API contract
where the function might be called only once.

s390 should have used memory aliases to fragment inital memory into
smaller chunks to satisfy KVM's memslot limitation. But its a bit
late now, since allocated pieces are transfered in migration stream
separately, so it's not possible to just replace broken layout with
correct one. To workaround issue, MemoryRegion alases are made
migratable and this patch switches to use them to split big initial
RAM chunk into smaller pieces (KVM_SLOT_MAX_BYTES max) and registers
aliases for migration. That should keep migration compatible with
previous QEMU versions.

New machine types (since 4.2) will use single memory region, which
will get transimitted in migration stream as a whole RAMBlock.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
I don't have access to a suitable system to test it, so I've simulated
it with smaller chunks on x84 host. Ping-pong migration between old
and new QEMU worked fine. KVM part should be fine as memslots
using mapped MemoryRegions (in this case it would be aliases) as
far as I know but is someone could test it on big enough host it
would be nice.
---
 include/hw/s390x/s390-virtio-ccw.h |  4 +++
 hw/s390x/s390-virtio-ccw.c         | 56 +++++++++++++++++++++---------
 2 files changed, 43 insertions(+), 17 deletions(-)

diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
index 00632f94b4..f9ed3737f8 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -21,6 +21,9 @@
 #define S390_MACHINE_CLASS(klass) \
     OBJECT_CLASS_CHECK(S390CcwMachineClass, (klass), TYPE_S390_CCW_MACHINE)
 
+#define S390_MACHINE_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(S390CcwMachineClass, (obj), TYPE_S390_CCW_MACHINE)
+
 /*
  * KVM does only support memory slots up to KVM_MEM_MAX_NR_PAGES pages
  * as the dirty bitmap must be managed by bitops that take an int as
@@ -50,6 +53,7 @@ typedef struct S390CcwMachineClass {
     bool cpu_model_allowed;
     bool css_migration_enabled;
     bool hpage_1m_allowed;
+    bool split_ram_layout;
 } S390CcwMachineClass;
 
 /* runtime-instrumentation allowed by the machine */
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 073672f9cb..9160c1ed0a 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -151,28 +151,47 @@ static void virtio_ccw_register_hcalls(void)
                                    virtio_ccw_hcall_early_printk);
 }
 
-static void s390_memory_init(ram_addr_t mem_size)
+static void s390_memory_init(MachineState *ms)
 {
+    S390CcwMachineClass *s390mc = S390_MACHINE_GET_CLASS(ms);
     MemoryRegion *sysmem = get_system_memory();
-    ram_addr_t chunk, offset = 0;
+    MemoryRegion *ram = g_new(MemoryRegion, 1);
     unsigned int number = 0;
     Error *local_err = NULL;
-    gchar *name;
+    ram_addr_t mem_size = ms->ram_size;
+    gchar *name = g_strdup_printf(s390mc->split_ram_layout ?
+                                  "s390.whole.ram" : "s390.ram");
 
     /* allocate RAM for core */
-    name = g_strdup_printf("s390.ram");
-    while (mem_size) {
-        MemoryRegion *ram = g_new(MemoryRegion, 1);
-        uint64_t size = mem_size;
-
-        /* KVM does not allow memslots >= 8 TB */
-        chunk = MIN(size, KVM_SLOT_MAX_BYTES);
-        memory_region_allocate_system_memory(ram, NULL, name, chunk);
-        memory_region_add_subregion(sysmem, offset, ram);
-        mem_size -= chunk;
-        offset += chunk;
-        g_free(name);
-        name = g_strdup_printf("s390.ram.%u", ++number);
+    memory_region_allocate_system_memory(ram, NULL, name, mem_size);
+
+    /* migration compatible RAM handling for 4.1 and older machines */
+    if (s390mc->split_ram_layout) {
+       ram_addr_t chunk, offset = 0;
+       /*
+        * memory_region_allocate_system_memory() registers allocated RAM for
+        * migration, however for compat reasons the RAM should be passed over
+        * as RAMBlocks of the size upto KVM_SLOT_MAX_BYTES. So unregister just
+        * allocated RAM so it won't be migrated directly. Aliases will take care
+        * of segmenting RAM into legacy chunks that migration compatible.
+        */
+       vmstate_unregister_ram(ram, NULL);
+       name = g_strdup_printf("s390.ram");
+       while (mem_size) {
+           MemoryRegion *alias = g_new(MemoryRegion, 1);
+
+           /* KVM does not allow memslots >= 8 TB */
+           chunk = MIN(mem_size, KVM_SLOT_MAX_BYTES);
+           memory_region_init_alias(alias, NULL, name, ram, offset, chunk);
+           vmstate_register_ram_global(alias);
+           memory_region_add_subregion(sysmem, offset, alias);
+           mem_size -= chunk;
+           offset += chunk;
+           g_free(name);
+           name = g_strdup_printf("s390.ram.%u", ++number);
+       }
+    } else {
+       memory_region_add_subregion(sysmem, 0, ram);
     }
     g_free(name);
 
@@ -257,7 +276,7 @@ static void ccw_init(MachineState *machine)
 
     s390_sclp_init();
     /* init memory + setup max page size. Required for the CPU model */
-    s390_memory_init(machine->ram_size);
+    s390_memory_init(machine);
 
     /* init CPUs (incl. CPU model) early so s390_has_feature() works */
     s390_init_cpus(machine);
@@ -667,8 +686,11 @@ static void ccw_machine_4_1_instance_options(MachineState *machine)
 
 static void ccw_machine_4_1_class_options(MachineClass *mc)
 {
+    S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
+
     ccw_machine_4_2_class_options(mc);
     compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
+    s390mc->split_ram_layout = true;
 }
 DEFINE_CCW_MACHINE(4_1, "4.1", false);
 
-- 
2.18.1

Re: [Qemu-devel] [PATCH RFC v2 0/4] s390: stop abusing memory_region_allocate_system_memory()

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190802093854.5343-1-imammedo@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PATCH RFC v2 0/4] s390: stop abusing memory_region_allocate_system_memory()
Message-id: 20190802093854.5343-1-imammedo@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20190802093854.5343-1-imammedo@redhat.com -> patchew/20190802093854.5343-1-imammedo@redhat.com
Submodule 'capstone' (https://git.qemu.org/git/capstone.git) registered for path 'capstone'
Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc'
Submodule 'roms/QemuMacDrivers' (https://git.qemu.org/git/QemuMacDrivers.git) registered for path 'roms/QemuMacDrivers'
Submodule 'roms/SLOF' (https://git.qemu.org/git/SLOF.git) registered for path 'roms/SLOF'
Submodule 'roms/edk2' (https://git.qemu.org/git/edk2.git) registered for path 'roms/edk2'
Submodule 'roms/ipxe' (https://git.qemu.org/git/ipxe.git) registered for path 'roms/ipxe'
Submodule 'roms/openbios' (https://git.qemu.org/git/openbios.git) registered for path 'roms/openbios'
Submodule 'roms/openhackware' (https://git.qemu.org/git/openhackware.git) registered for path 'roms/openhackware'
Submodule 'roms/opensbi' (https://git.qemu.org/git/opensbi.git) registered for path 'roms/opensbi'
Submodule 'roms/qemu-palcode' (https://git.qemu.org/git/qemu-palcode.git) registered for path 'roms/qemu-palcode'
Submodule 'roms/seabios' (https://git.qemu.org/git/seabios.git/) registered for path 'roms/seabios'
Submodule 'roms/seabios-hppa' (https://git.qemu.org/git/seabios-hppa.git) registered for path 'roms/seabios-hppa'
Submodule 'roms/sgabios' (https://git.qemu.org/git/sgabios.git) registered for path 'roms/sgabios'
Submodule 'roms/skiboot' (https://git.qemu.org/git/skiboot.git) registered for path 'roms/skiboot'
Submodule 'roms/u-boot' (https://git.qemu.org/git/u-boot.git) registered for path 'roms/u-boot'
Submodule 'roms/u-boot-sam460ex' (https://git.qemu.org/git/u-boot-sam460ex.git) registered for path 'roms/u-boot-sam460ex'
Submodule 'slirp' (https://git.qemu.org/git/libslirp.git) registered for path 'slirp'
Submodule 'tests/fp/berkeley-softfloat-3' (https://git.qemu.org/git/berkeley-softfloat-3.git) registered for path 'tests/fp/berkeley-softfloat-3'
Submodule 'tests/fp/berkeley-testfloat-3' (https://git.qemu.org/git/berkeley-testfloat-3.git) registered for path 'tests/fp/berkeley-testfloat-3'
Submodule 'ui/keycodemapdb' (https://git.qemu.org/git/keycodemapdb.git) registered for path 'ui/keycodemapdb'
Cloning into 'capstone'...
Submodule path 'capstone': checked out '22ead3e0bfdb87516656453336160e0a37b066bf'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '88f18909db731a627456f26d779445f84e449536'
Cloning into 'roms/QemuMacDrivers'...
Submodule path 'roms/QemuMacDrivers': checked out '90c488d5f4a407342247b9ea869df1c2d9c8e266'
Cloning into 'roms/SLOF'...
Submodule path 'roms/SLOF': checked out 'ba1ab360eebe6338bb8d7d83a9220ccf7e213af3'
Cloning into 'roms/edk2'...
Submodule path 'roms/edk2': checked out '20d2e5a125e34fc8501026613a71549b2a1a3e54'
Submodule 'SoftFloat' (https://github.com/ucb-bar/berkeley-softfloat-3.git) registered for path 'ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3'
Submodule 'CryptoPkg/Library/OpensslLib/openssl' (https://github.com/openssl/openssl) registered for path 'CryptoPkg/Library/OpensslLib/openssl'
Cloning into 'ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3'...
Submodule path 'roms/edk2/ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3': checked out 'b64af41c3276f97f0e181920400ee056b9c88037'
Cloning into 'CryptoPkg/Library/OpensslLib/openssl'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl': checked out '50eaac9f3337667259de725451f201e784599687'
Submodule 'boringssl' (https://boringssl.googlesource.com/boringssl) registered for path 'boringssl'
Submodule 'krb5' (https://github.com/krb5/krb5) registered for path 'krb5'
Submodule 'pyca.cryptography' (https://github.com/pyca/cryptography.git) registered for path 'pyca-cryptography'
Cloning into 'boringssl'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/boringssl': checked out '2070f8ad9151dc8f3a73bffaa146b5e6937a583f'
Cloning into 'krb5'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/krb5': checked out 'b9ad6c49505c96a088326b62a52568e3484f2168'
Cloning into 'pyca-cryptography'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/pyca-cryptography': checked out '09403100de2f6f1cdd0d484dcb8e620f1c335c8f'
Cloning into 'roms/ipxe'...
Submodule path 'roms/ipxe': checked out 'de4565cbe76ea9f7913a01f331be3ee901bb6e17'
Cloning into 'roms/openbios'...
Submodule path 'roms/openbios': checked out 'c79e0ecb84f4f1ee3f73f521622e264edd1bf174'
Cloning into 'roms/openhackware'...
Submodule path 'roms/openhackware': checked out 'c559da7c8eec5e45ef1f67978827af6f0b9546f5'
Cloning into 'roms/opensbi'...
Submodule path 'roms/opensbi': checked out 'ce228ee0919deb9957192d723eecc8aaae2697c6'
Cloning into 'roms/qemu-palcode'...
Submodule path 'roms/qemu-palcode': checked out 'bf0e13698872450164fa7040da36a95d2d4b326f'
Cloning into 'roms/seabios'...
Submodule path 'roms/seabios': checked out 'a5cab58e9a3fb6e168aba919c5669bea406573b4'
Cloning into 'roms/seabios-hppa'...
Submodule path 'roms/seabios-hppa': checked out '0f4fe84658165e96ce35870fd19fc634e182e77b'
Cloning into 'roms/sgabios'...
Submodule path 'roms/sgabios': checked out 'cbaee52287e5f32373181cff50a00b6c4ac9015a'
Cloning into 'roms/skiboot'...
Submodule path 'roms/skiboot': checked out '261ca8e779e5138869a45f174caa49be6a274501'
Cloning into 'roms/u-boot'...
Submodule path 'roms/u-boot': checked out 'd3689267f92c5956e09cc7d1baa4700141662bff'
Cloning into 'roms/u-boot-sam460ex'...
Submodule path 'roms/u-boot-sam460ex': checked out '60b3916f33e617a815973c5a6df77055b2e3a588'
Cloning into 'slirp'...
Submodule path 'slirp': checked out 'f0da6726207b740f6101028b2992f918477a4b08'
Cloning into 'tests/fp/berkeley-softfloat-3'...
Submodule path 'tests/fp/berkeley-softfloat-3': checked out 'b64af41c3276f97f0e181920400ee056b9c88037'
Cloning into 'tests/fp/berkeley-testfloat-3'...
Submodule path 'tests/fp/berkeley-testfloat-3': checked out '5a59dcec19327396a011a17fd924aed4fec416b3'
Cloning into 'ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce'
Switched to a new branch 'test'
a0de4e7 s390: do not call memory_region_allocate_system_memory() multiple times
705a271 memory: make MemoryRegion alias migratable
b592e60 kvm: s390: split too big memory section on several memslots
a1acd7e hw: add compat machines for 4.2

=== OUTPUT BEGIN ===
1/4 Checking commit a1acd7e26c3b (hw: add compat machines for 4.2)
2/4 Checking commit b592e609ff83 (kvm: s390: split too big memory section on several memslots)
WARNING: line over 80 characters
#36: FILE: accel/kvm/kvm-all.c:957:
+    g_assert(ROUND_UP(max_slot_size, qemu_real_host_page_size) == max_slot_size);

total: 0 errors, 1 warnings, 148 lines checked

Patch 2/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/4 Checking commit 705a2717d9f8 (memory: make MemoryRegion alias migratable)
4/4 Checking commit a0de4e7c7b91 (s390: do not call memory_region_allocate_system_memory() multiple times)
ERROR: suspect code indent for conditional statements (4, 7)
#65: FILE: hw/s390x/s390-virtio-ccw.c:169:
+    if (s390mc->split_ram_layout) {
+       ram_addr_t chunk, offset = 0;

ERROR: suspect code indent for conditional statements (7, 11)
#76: FILE: hw/s390x/s390-virtio-ccw.c:180:
+       while (mem_size) {
+           MemoryRegion *alias = g_new(MemoryRegion, 1);

total: 2 errors, 0 warnings, 97 lines checked

Patch 4/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190802093854.5343-1-imammedo@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH RFC v2 0/4] s390: stop abusing memory_region_allocate_system_memory()

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190802093854.5343-1-imammedo@redhat.com/



Hi,

This series failed build test on s390x host. Please find the details below.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
set -e

echo
echo "=== ENV ==="
env

echo
echo "=== PACKAGES ==="
rpm -qa

echo
echo "=== UNAME ==="
uname -a

CC=$HOME/bin/cc
INSTALL=$PWD/install
BUILD=$PWD/build
mkdir -p $BUILD $INSTALL
SRC=$PWD
cd $BUILD
$SRC/configure --cc=$CC --prefix=$INSTALL
make -j4
# XXX: we need reliable clean up
# make check -j4 V=1
make install
=== TEST SCRIPT END ===

  CC      s390x-softmmu/target/s390x/kvm.o
  CC      sparc64-softmmu/qapi/qapi-introspect.o
/var/tmp/patchew-tester-tmp-rimg5ia0/src/target/s390x/kvm.c: In function ‘kvm_arch_init’:
/var/tmp/patchew-tester-tmp-rimg5ia0/src/target/s390x/kvm.c:350:5: error: implicit declaration of function ‘kvm_set_max_memslot_size’; did you mean ‘kvm_get_max_memslots’? [-Werror=implicit-function-declaration]
  350 |     kvm_set_max_memslot_size(KVM_SLOT_MAX_BYTES);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~
      |     kvm_get_max_memslots
/var/tmp/patchew-tester-tmp-rimg5ia0/src/target/s390x/kvm.c:350:5: error: nested extern declaration of ‘kvm_set_max_memslot_size’ [-Werror=nested-externs]
cc1: all warnings being treated as errors
make[1]: *** [/var/tmp/patchew-tester-tmp-rimg5ia0/src/rules.mak:69: target/s390x/kvm.o] Error 1
make: *** [Makefile:472: s390x-softmmu/all] Error 2


The full log is available at
http://patchew.org/logs/20190802093854.5343-1-imammedo@redhat.com/testing.s390x/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH RFC v2 4/4] s390: do not call memory_region_allocate_system_memory() multiple times

[David Hildenbrand]

On 02.08.19 11:38, Igor Mammedov wrote:
> s390 was trying to solve limited KVM memslot size issue by abusing
> memory_region_allocate_system_memory(), which breaks API contract
> where the function might be called only once.
> 
> s390 should have used memory aliases to fragment inital memory into
> smaller chunks to satisfy KVM's memslot limitation. But its a bit
> late now, since allocated pieces are transfered in migration stream
> separately, so it's not possible to just replace broken layout with
> correct one. To workaround issue, MemoryRegion alases are made
> migratable and this patch switches to use them to split big initial
> RAM chunk into smaller pieces (KVM_SLOT_MAX_BYTES max) and registers
> aliases for migration. That should keep migration compatible with
> previous QEMU versions.
> 
> New machine types (since 4.2) will use single memory region, which
> will get transimitted in migration stream as a whole RAMBlock.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> I don't have access to a suitable system to test it, so I've simulated
> it with smaller chunks on x84 host. Ping-pong migration between old
> and new QEMU worked fine. KVM part should be fine as memslots
> using mapped MemoryRegions (in this case it would be aliases) as
> far as I know but is someone could test it on big enough host it
> would be nice.
> ---
>  include/hw/s390x/s390-virtio-ccw.h |  4 +++
>  hw/s390x/s390-virtio-ccw.c         | 56 +++++++++++++++++++++---------
>  2 files changed, 43 insertions(+), 17 deletions(-)
> 
> diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
> index 00632f94b4..f9ed3737f8 100644
> --- a/include/hw/s390x/s390-virtio-ccw.h
> +++ b/include/hw/s390x/s390-virtio-ccw.h
> @@ -21,6 +21,9 @@
>  #define S390_MACHINE_CLASS(klass) \
>      OBJECT_CLASS_CHECK(S390CcwMachineClass, (klass), TYPE_S390_CCW_MACHINE)
>  
> +#define S390_MACHINE_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(S390CcwMachineClass, (obj), TYPE_S390_CCW_MACHINE)
> +
>  /*
>   * KVM does only support memory slots up to KVM_MEM_MAX_NR_PAGES pages
>   * as the dirty bitmap must be managed by bitops that take an int as
> @@ -50,6 +53,7 @@ typedef struct S390CcwMachineClass {
>      bool cpu_model_allowed;
>      bool css_migration_enabled;
>      bool hpage_1m_allowed;
> +    bool split_ram_layout;
>  } S390CcwMachineClass;
>  
>  /* runtime-instrumentation allowed by the machine */
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 073672f9cb..9160c1ed0a 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -151,28 +151,47 @@ static void virtio_ccw_register_hcalls(void)
>                                     virtio_ccw_hcall_early_printk);
>  }
>  
> -static void s390_memory_init(ram_addr_t mem_size)
> +static void s390_memory_init(MachineState *ms)
>  {
> +    S390CcwMachineClass *s390mc = S390_MACHINE_GET_CLASS(ms);
>      MemoryRegion *sysmem = get_system_memory();
> -    ram_addr_t chunk, offset = 0;
> +    MemoryRegion *ram = g_new(MemoryRegion, 1);
>      unsigned int number = 0;
>      Error *local_err = NULL;
> -    gchar *name;
> +    ram_addr_t mem_size = ms->ram_size;
> +    gchar *name = g_strdup_printf(s390mc->split_ram_layout ?
> +                                  "s390.whole.ram" : "s390.ram");
>  
>      /* allocate RAM for core */
> -    name = g_strdup_printf("s390.ram");
> -    while (mem_size) {
> -        MemoryRegion *ram = g_new(MemoryRegion, 1);
> -        uint64_t size = mem_size;
> -
> -        /* KVM does not allow memslots >= 8 TB */
> -        chunk = MIN(size, KVM_SLOT_MAX_BYTES);
> -        memory_region_allocate_system_memory(ram, NULL, name, chunk);
> -        memory_region_add_subregion(sysmem, offset, ram);
> -        mem_size -= chunk;
> -        offset += chunk;
> -        g_free(name);
> -        name = g_strdup_printf("s390.ram.%u", ++number);
> +    memory_region_allocate_system_memory(ram, NULL, name, mem_size);
> +
> +    /* migration compatible RAM handling for 4.1 and older machines */
> +    if (s390mc->split_ram_layout) {
> +       ram_addr_t chunk, offset = 0;
> +       /*
> +        * memory_region_allocate_system_memory() registers allocated RAM for
> +        * migration, however for compat reasons the RAM should be passed over
> +        * as RAMBlocks of the size upto KVM_SLOT_MAX_BYTES. So unregister just
> +        * allocated RAM so it won't be migrated directly. Aliases will take care
> +        * of segmenting RAM into legacy chunks that migration compatible.
> +        */
> +       vmstate_unregister_ram(ram, NULL);
> +       name = g_strdup_printf("s390.ram");
> +       while (mem_size) {
> +           MemoryRegion *alias = g_new(MemoryRegion, 1);
> +
> +           /* KVM does not allow memslots >= 8 TB */
> +           chunk = MIN(mem_size, KVM_SLOT_MAX_BYTES);
> +           memory_region_init_alias(alias, NULL, name, ram, offset, chunk);
> +           vmstate_register_ram_global(alias);
> +           memory_region_add_subregion(sysmem, offset, alias);
> +           mem_size -= chunk;
> +           offset += chunk;
> +           g_free(name);
> +           name = g_strdup_printf("s390.ram.%u", ++number);
> +       }
> +    } else {
> +       memory_region_add_subregion(sysmem, 0, ram);
>      }
>      g_free(name);
>  
> @@ -257,7 +276,7 @@ static void ccw_init(MachineState *machine)
>  
>      s390_sclp_init();
>      /* init memory + setup max page size. Required for the CPU model */
> -    s390_memory_init(machine->ram_size);
> +    s390_memory_init(machine);
>  
>      /* init CPUs (incl. CPU model) early so s390_has_feature() works */
>      s390_init_cpus(machine);
> @@ -667,8 +686,11 @@ static void ccw_machine_4_1_instance_options(MachineState *machine)
>  
>  static void ccw_machine_4_1_class_options(MachineClass *mc)
>  {
> +    S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
> +
>      ccw_machine_4_2_class_options(mc);
>      compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
> +    s390mc->split_ram_layout = true;
>  }
>  DEFINE_CCW_MACHINE(4_1, "4.1", false);
>  
> 

As discussed, I am not sure if adding that compat code really is worth
it. :)

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH RFC v2 4/4] s390: do not call memory_region_allocate_system_memory() multiple times

[Christian Borntraeger]

On 02.08.19 12:25, David Hildenbrand wrote:
> On 02.08.19 11:38, Igor Mammedov wrote:
>> s390 was trying to solve limited KVM memslot size issue by abusing
>> memory_region_allocate_system_memory(), which breaks API contract
>> where the function might be called only once.
>>
>> s390 should have used memory aliases to fragment inital memory into
>> smaller chunks to satisfy KVM's memslot limitation. But its a bit
>> late now, since allocated pieces are transfered in migration stream
>> separately, so it's not possible to just replace broken layout with
>> correct one. To workaround issue, MemoryRegion alases are made
>> migratable and this patch switches to use them to split big initial
>> RAM chunk into smaller pieces (KVM_SLOT_MAX_BYTES max) and registers
>> aliases for migration. That should keep migration compatible with
>> previous QEMU versions.
>>
>> New machine types (since 4.2) will use single memory region, which
>> will get transimitted in migration stream as a whole RAMBlock.
>>
>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>> ---
>> I don't have access to a suitable system to test it, so I've simulated
>> it with smaller chunks on x84 host. Ping-pong migration between old
>> and new QEMU worked fine. KVM part should be fine as memslots
>> using mapped MemoryRegions (in this case it would be aliases) as
>> far as I know but is someone could test it on big enough host it
>> would be nice.
>> ---
>>  include/hw/s390x/s390-virtio-ccw.h |  4 +++
>>  hw/s390x/s390-virtio-ccw.c         | 56 +++++++++++++++++++++---------
>>  2 files changed, 43 insertions(+), 17 deletions(-)
>>
>> diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
>> index 00632f94b4..f9ed3737f8 100644
>> --- a/include/hw/s390x/s390-virtio-ccw.h
>> +++ b/include/hw/s390x/s390-virtio-ccw.h
>> @@ -21,6 +21,9 @@
>>  #define S390_MACHINE_CLASS(klass) \
>>      OBJECT_CLASS_CHECK(S390CcwMachineClass, (klass), TYPE_S390_CCW_MACHINE)
>>  
>> +#define S390_MACHINE_GET_CLASS(obj) \
>> +    OBJECT_GET_CLASS(S390CcwMachineClass, (obj), TYPE_S390_CCW_MACHINE)
>> +
>>  /*
>>   * KVM does only support memory slots up to KVM_MEM_MAX_NR_PAGES pages
>>   * as the dirty bitmap must be managed by bitops that take an int as
>> @@ -50,6 +53,7 @@ typedef struct S390CcwMachineClass {
>>      bool cpu_model_allowed;
>>      bool css_migration_enabled;
>>      bool hpage_1m_allowed;
>> +    bool split_ram_layout;
>>  } S390CcwMachineClass;
>>  
>>  /* runtime-instrumentation allowed by the machine */
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index 073672f9cb..9160c1ed0a 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -151,28 +151,47 @@ static void virtio_ccw_register_hcalls(void)
>>                                     virtio_ccw_hcall_early_printk);
>>  }
>>  
>> -static void s390_memory_init(ram_addr_t mem_size)
>> +static void s390_memory_init(MachineState *ms)
>>  {
>> +    S390CcwMachineClass *s390mc = S390_MACHINE_GET_CLASS(ms);
>>      MemoryRegion *sysmem = get_system_memory();
>> -    ram_addr_t chunk, offset = 0;
>> +    MemoryRegion *ram = g_new(MemoryRegion, 1);
>>      unsigned int number = 0;
>>      Error *local_err = NULL;
>> -    gchar *name;
>> +    ram_addr_t mem_size = ms->ram_size;
>> +    gchar *name = g_strdup_printf(s390mc->split_ram_layout ?
>> +                                  "s390.whole.ram" : "s390.ram");
>>  
>>      /* allocate RAM for core */
>> -    name = g_strdup_printf("s390.ram");
>> -    while (mem_size) {
>> -        MemoryRegion *ram = g_new(MemoryRegion, 1);
>> -        uint64_t size = mem_size;
>> -
>> -        /* KVM does not allow memslots >= 8 TB */
>> -        chunk = MIN(size, KVM_SLOT_MAX_BYTES);
>> -        memory_region_allocate_system_memory(ram, NULL, name, chunk);
>> -        memory_region_add_subregion(sysmem, offset, ram);
>> -        mem_size -= chunk;
>> -        offset += chunk;
>> -        g_free(name);
>> -        name = g_strdup_printf("s390.ram.%u", ++number);
>> +    memory_region_allocate_system_memory(ram, NULL, name, mem_size);
>> +
>> +    /* migration compatible RAM handling for 4.1 and older machines */
>> +    if (s390mc->split_ram_layout) {
>> +       ram_addr_t chunk, offset = 0;
>> +       /*
>> +        * memory_region_allocate_system_memory() registers allocated RAM for
>> +        * migration, however for compat reasons the RAM should be passed over
>> +        * as RAMBlocks of the size upto KVM_SLOT_MAX_BYTES. So unregister just
>> +        * allocated RAM so it won't be migrated directly. Aliases will take care
>> +        * of segmenting RAM into legacy chunks that migration compatible.
>> +        */
>> +       vmstate_unregister_ram(ram, NULL);
>> +       name = g_strdup_printf("s390.ram");
>> +       while (mem_size) {
>> +           MemoryRegion *alias = g_new(MemoryRegion, 1);
>> +
>> +           /* KVM does not allow memslots >= 8 TB */
>> +           chunk = MIN(mem_size, KVM_SLOT_MAX_BYTES);
>> +           memory_region_init_alias(alias, NULL, name, ram, offset, chunk);
>> +           vmstate_register_ram_global(alias);
>> +           memory_region_add_subregion(sysmem, offset, alias);
>> +           mem_size -= chunk;
>> +           offset += chunk;
>> +           g_free(name);
>> +           name = g_strdup_printf("s390.ram.%u", ++number);
>> +       }
>> +    } else {
>> +       memory_region_add_subregion(sysmem, 0, ram);
>>      }
>>      g_free(name);
>>  
>> @@ -257,7 +276,7 @@ static void ccw_init(MachineState *machine)
>>  
>>      s390_sclp_init();
>>      /* init memory + setup max page size. Required for the CPU model */
>> -    s390_memory_init(machine->ram_size);
>> +    s390_memory_init(machine);
>>  
>>      /* init CPUs (incl. CPU model) early so s390_has_feature() works */
>>      s390_init_cpus(machine);
>> @@ -667,8 +686,11 @@ static void ccw_machine_4_1_instance_options(MachineState *machine)
>>  
>>  static void ccw_machine_4_1_class_options(MachineClass *mc)
>>  {
>> +    S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
>> +
>>      ccw_machine_4_2_class_options(mc);
>>      compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
>> +    s390mc->split_ram_layout = true;
>>  }
>>  DEFINE_CCW_MACHINE(4_1, "4.1", false);
>>  
>>
> 
> As discussed, I am not sure if adding that compat code really is worth
> it. :)

Agreed. As long as forward migration (e.g. 4.2->4.3 will work) and we document
that migration will fails for 2.12->.....4.1 for guests >=8TB we  can simplify
things.

Re: [Qemu-devel] [PATCH RFC v2 4/4] s390: do not call memory_region_allocate_system_memory() multiple times

[Igor Mammedov]

On Fri, 2 Aug 2019 12:27:50 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> On 02.08.19 12:25, David Hildenbrand wrote:
> > On 02.08.19 11:38, Igor Mammedov wrote:  
> >> s390 was trying to solve limited KVM memslot size issue by abusing
> >> memory_region_allocate_system_memory(), which breaks API contract
> >> where the function might be called only once.
> >>
> >> s390 should have used memory aliases to fragment inital memory into
> >> smaller chunks to satisfy KVM's memslot limitation. But its a bit
> >> late now, since allocated pieces are transfered in migration stream
> >> separately, so it's not possible to just replace broken layout with
> >> correct one. To workaround issue, MemoryRegion alases are made
> >> migratable and this patch switches to use them to split big initial
> >> RAM chunk into smaller pieces (KVM_SLOT_MAX_BYTES max) and registers
> >> aliases for migration. That should keep migration compatible with
> >> previous QEMU versions.
> >>
> >> New machine types (since 4.2) will use single memory region, which
> >> will get transimitted in migration stream as a whole RAMBlock.
> >>
> >> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> >> ---
> >> I don't have access to a suitable system to test it, so I've simulated
> >> it with smaller chunks on x84 host. Ping-pong migration between old
> >> and new QEMU worked fine. KVM part should be fine as memslots
> >> using mapped MemoryRegions (in this case it would be aliases) as
> >> far as I know but is someone could test it on big enough host it
> >> would be nice.
> >> ---
> >>  include/hw/s390x/s390-virtio-ccw.h |  4 +++
> >>  hw/s390x/s390-virtio-ccw.c         | 56 +++++++++++++++++++++---------
> >>  2 files changed, 43 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
> >> index 00632f94b4..f9ed3737f8 100644
> >> --- a/include/hw/s390x/s390-virtio-ccw.h
> >> +++ b/include/hw/s390x/s390-virtio-ccw.h
> >> @@ -21,6 +21,9 @@
> >>  #define S390_MACHINE_CLASS(klass) \
> >>      OBJECT_CLASS_CHECK(S390CcwMachineClass, (klass), TYPE_S390_CCW_MACHINE)
> >>  
> >> +#define S390_MACHINE_GET_CLASS(obj) \
> >> +    OBJECT_GET_CLASS(S390CcwMachineClass, (obj), TYPE_S390_CCW_MACHINE)
> >> +
> >>  /*
> >>   * KVM does only support memory slots up to KVM_MEM_MAX_NR_PAGES pages
> >>   * as the dirty bitmap must be managed by bitops that take an int as
> >> @@ -50,6 +53,7 @@ typedef struct S390CcwMachineClass {
> >>      bool cpu_model_allowed;
> >>      bool css_migration_enabled;
> >>      bool hpage_1m_allowed;
> >> +    bool split_ram_layout;
> >>  } S390CcwMachineClass;
> >>  
> >>  /* runtime-instrumentation allowed by the machine */
> >> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> >> index 073672f9cb..9160c1ed0a 100644
> >> --- a/hw/s390x/s390-virtio-ccw.c
> >> +++ b/hw/s390x/s390-virtio-ccw.c
> >> @@ -151,28 +151,47 @@ static void virtio_ccw_register_hcalls(void)
> >>                                     virtio_ccw_hcall_early_printk);
> >>  }
> >>  
> >> -static void s390_memory_init(ram_addr_t mem_size)
> >> +static void s390_memory_init(MachineState *ms)
> >>  {
> >> +    S390CcwMachineClass *s390mc = S390_MACHINE_GET_CLASS(ms);
> >>      MemoryRegion *sysmem = get_system_memory();
> >> -    ram_addr_t chunk, offset = 0;
> >> +    MemoryRegion *ram = g_new(MemoryRegion, 1);
> >>      unsigned int number = 0;
> >>      Error *local_err = NULL;
> >> -    gchar *name;
> >> +    ram_addr_t mem_size = ms->ram_size;
> >> +    gchar *name = g_strdup_printf(s390mc->split_ram_layout ?
> >> +                                  "s390.whole.ram" : "s390.ram");
> >>  
> >>      /* allocate RAM for core */
> >> -    name = g_strdup_printf("s390.ram");
> >> -    while (mem_size) {
> >> -        MemoryRegion *ram = g_new(MemoryRegion, 1);
> >> -        uint64_t size = mem_size;
> >> -
> >> -        /* KVM does not allow memslots >= 8 TB */
> >> -        chunk = MIN(size, KVM_SLOT_MAX_BYTES);
> >> -        memory_region_allocate_system_memory(ram, NULL, name, chunk);
> >> -        memory_region_add_subregion(sysmem, offset, ram);
> >> -        mem_size -= chunk;
> >> -        offset += chunk;
> >> -        g_free(name);
> >> -        name = g_strdup_printf("s390.ram.%u", ++number);
> >> +    memory_region_allocate_system_memory(ram, NULL, name, mem_size);
> >> +
> >> +    /* migration compatible RAM handling for 4.1 and older machines */
> >> +    if (s390mc->split_ram_layout) {
> >> +       ram_addr_t chunk, offset = 0;
> >> +       /*
> >> +        * memory_region_allocate_system_memory() registers allocated RAM for
> >> +        * migration, however for compat reasons the RAM should be passed over
> >> +        * as RAMBlocks of the size upto KVM_SLOT_MAX_BYTES. So unregister just
> >> +        * allocated RAM so it won't be migrated directly. Aliases will take care
> >> +        * of segmenting RAM into legacy chunks that migration compatible.
> >> +        */
> >> +       vmstate_unregister_ram(ram, NULL);
> >> +       name = g_strdup_printf("s390.ram");
> >> +       while (mem_size) {
> >> +           MemoryRegion *alias = g_new(MemoryRegion, 1);
> >> +
> >> +           /* KVM does not allow memslots >= 8 TB */
> >> +           chunk = MIN(mem_size, KVM_SLOT_MAX_BYTES);
> >> +           memory_region_init_alias(alias, NULL, name, ram, offset, chunk);
> >> +           vmstate_register_ram_global(alias);
> >> +           memory_region_add_subregion(sysmem, offset, alias);
> >> +           mem_size -= chunk;
> >> +           offset += chunk;
> >> +           g_free(name);
> >> +           name = g_strdup_printf("s390.ram.%u", ++number);
> >> +       }
> >> +    } else {
> >> +       memory_region_add_subregion(sysmem, 0, ram);
> >>      }
> >>      g_free(name);
> >>  
> >> @@ -257,7 +276,7 @@ static void ccw_init(MachineState *machine)
> >>  
> >>      s390_sclp_init();
> >>      /* init memory + setup max page size. Required for the CPU model */
> >> -    s390_memory_init(machine->ram_size);
> >> +    s390_memory_init(machine);
> >>  
> >>      /* init CPUs (incl. CPU model) early so s390_has_feature() works */
> >>      s390_init_cpus(machine);
> >> @@ -667,8 +686,11 @@ static void ccw_machine_4_1_instance_options(MachineState *machine)
> >>  
> >>  static void ccw_machine_4_1_class_options(MachineClass *mc)
> >>  {
> >> +    S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
> >> +
> >>      ccw_machine_4_2_class_options(mc);
> >>      compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
> >> +    s390mc->split_ram_layout = true;
> >>  }
> >>  DEFINE_CCW_MACHINE(4_1, "4.1", false);
> >>  
> >>  
> > 
> > As discussed, I am not sure if adding that compat code really is worth
> > it. :)  
> 
> Agreed. As long as forward migration (e.g. 4.2->4.3 will work) and we document
> that migration will fails for 2.12->.....4.1 for guests >=8TB we  can simplify
> things.

I'll drop 3/4 and compat code in this patch.

Christian,

could you test this series on big enough host to ensure that KVM memslot splitting
part works as expected (i.e. it's possible to start >8TB guest)?

Re: [Qemu-devel] [PATCH RFC v2 4/4] s390: do not call memory_region_allocate_system_memory() multiple times

[Christian Borntraeger]

On 02.08.19 13:40, Igor Mammedov wrote:
> On Fri, 2 Aug 2019 12:27:50 +0200
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
>> On 02.08.19 12:25, David Hildenbrand wrote:
>>> On 02.08.19 11:38, Igor Mammedov wrote:  
>>>> s390 was trying to solve limited KVM memslot size issue by abusing
>>>> memory_region_allocate_system_memory(), which breaks API contract
>>>> where the function might be called only once.
>>>>
>>>> s390 should have used memory aliases to fragment inital memory into
>>>> smaller chunks to satisfy KVM's memslot limitation. But its a bit
>>>> late now, since allocated pieces are transfered in migration stream
>>>> separately, so it's not possible to just replace broken layout with
>>>> correct one. To workaround issue, MemoryRegion alases are made
>>>> migratable and this patch switches to use them to split big initial
>>>> RAM chunk into smaller pieces (KVM_SLOT_MAX_BYTES max) and registers
>>>> aliases for migration. That should keep migration compatible with
>>>> previous QEMU versions.
>>>>
>>>> New machine types (since 4.2) will use single memory region, which
>>>> will get transimitted in migration stream as a whole RAMBlock.
>>>>
>>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>>>> ---
>>>> I don't have access to a suitable system to test it, so I've simulated
>>>> it with smaller chunks on x84 host. Ping-pong migration between old
>>>> and new QEMU worked fine. KVM part should be fine as memslots
>>>> using mapped MemoryRegions (in this case it would be aliases) as
>>>> far as I know but is someone could test it on big enough host it
>>>> would be nice.
>>>> ---
>>>>  include/hw/s390x/s390-virtio-ccw.h |  4 +++
>>>>  hw/s390x/s390-virtio-ccw.c         | 56 +++++++++++++++++++++---------
>>>>  2 files changed, 43 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
>>>> index 00632f94b4..f9ed3737f8 100644
>>>> --- a/include/hw/s390x/s390-virtio-ccw.h
>>>> +++ b/include/hw/s390x/s390-virtio-ccw.h
>>>> @@ -21,6 +21,9 @@
>>>>  #define S390_MACHINE_CLASS(klass) \
>>>>      OBJECT_CLASS_CHECK(S390CcwMachineClass, (klass), TYPE_S390_CCW_MACHINE)
>>>>  
>>>> +#define S390_MACHINE_GET_CLASS(obj) \
>>>> +    OBJECT_GET_CLASS(S390CcwMachineClass, (obj), TYPE_S390_CCW_MACHINE)
>>>> +
>>>>  /*
>>>>   * KVM does only support memory slots up to KVM_MEM_MAX_NR_PAGES pages
>>>>   * as the dirty bitmap must be managed by bitops that take an int as
>>>> @@ -50,6 +53,7 @@ typedef struct S390CcwMachineClass {
>>>>      bool cpu_model_allowed;
>>>>      bool css_migration_enabled;
>>>>      bool hpage_1m_allowed;
>>>> +    bool split_ram_layout;
>>>>  } S390CcwMachineClass;
>>>>  
>>>>  /* runtime-instrumentation allowed by the machine */
>>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>>> index 073672f9cb..9160c1ed0a 100644
>>>> --- a/hw/s390x/s390-virtio-ccw.c
>>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>>> @@ -151,28 +151,47 @@ static void virtio_ccw_register_hcalls(void)
>>>>                                     virtio_ccw_hcall_early_printk);
>>>>  }
>>>>  
>>>> -static void s390_memory_init(ram_addr_t mem_size)
>>>> +static void s390_memory_init(MachineState *ms)
>>>>  {
>>>> +    S390CcwMachineClass *s390mc = S390_MACHINE_GET_CLASS(ms);
>>>>      MemoryRegion *sysmem = get_system_memory();
>>>> -    ram_addr_t chunk, offset = 0;
>>>> +    MemoryRegion *ram = g_new(MemoryRegion, 1);
>>>>      unsigned int number = 0;
>>>>      Error *local_err = NULL;
>>>> -    gchar *name;
>>>> +    ram_addr_t mem_size = ms->ram_size;
>>>> +    gchar *name = g_strdup_printf(s390mc->split_ram_layout ?
>>>> +                                  "s390.whole.ram" : "s390.ram");
>>>>  
>>>>      /* allocate RAM for core */
>>>> -    name = g_strdup_printf("s390.ram");
>>>> -    while (mem_size) {
>>>> -        MemoryRegion *ram = g_new(MemoryRegion, 1);
>>>> -        uint64_t size = mem_size;
>>>> -
>>>> -        /* KVM does not allow memslots >= 8 TB */
>>>> -        chunk = MIN(size, KVM_SLOT_MAX_BYTES);
>>>> -        memory_region_allocate_system_memory(ram, NULL, name, chunk);
>>>> -        memory_region_add_subregion(sysmem, offset, ram);
>>>> -        mem_size -= chunk;
>>>> -        offset += chunk;
>>>> -        g_free(name);
>>>> -        name = g_strdup_printf("s390.ram.%u", ++number);
>>>> +    memory_region_allocate_system_memory(ram, NULL, name, mem_size);
>>>> +
>>>> +    /* migration compatible RAM handling for 4.1 and older machines */
>>>> +    if (s390mc->split_ram_layout) {
>>>> +       ram_addr_t chunk, offset = 0;
>>>> +       /*
>>>> +        * memory_region_allocate_system_memory() registers allocated RAM for
>>>> +        * migration, however for compat reasons the RAM should be passed over
>>>> +        * as RAMBlocks of the size upto KVM_SLOT_MAX_BYTES. So unregister just
>>>> +        * allocated RAM so it won't be migrated directly. Aliases will take care
>>>> +        * of segmenting RAM into legacy chunks that migration compatible.
>>>> +        */
>>>> +       vmstate_unregister_ram(ram, NULL);
>>>> +       name = g_strdup_printf("s390.ram");
>>>> +       while (mem_size) {
>>>> +           MemoryRegion *alias = g_new(MemoryRegion, 1);
>>>> +
>>>> +           /* KVM does not allow memslots >= 8 TB */
>>>> +           chunk = MIN(mem_size, KVM_SLOT_MAX_BYTES);
>>>> +           memory_region_init_alias(alias, NULL, name, ram, offset, chunk);
>>>> +           vmstate_register_ram_global(alias);
>>>> +           memory_region_add_subregion(sysmem, offset, alias);
>>>> +           mem_size -= chunk;
>>>> +           offset += chunk;
>>>> +           g_free(name);
>>>> +           name = g_strdup_printf("s390.ram.%u", ++number);
>>>> +       }
>>>> +    } else {
>>>> +       memory_region_add_subregion(sysmem, 0, ram);
>>>>      }
>>>>      g_free(name);
>>>>  
>>>> @@ -257,7 +276,7 @@ static void ccw_init(MachineState *machine)
>>>>  
>>>>      s390_sclp_init();
>>>>      /* init memory + setup max page size. Required for the CPU model */
>>>> -    s390_memory_init(machine->ram_size);
>>>> +    s390_memory_init(machine);
>>>>  
>>>>      /* init CPUs (incl. CPU model) early so s390_has_feature() works */
>>>>      s390_init_cpus(machine);
>>>> @@ -667,8 +686,11 @@ static void ccw_machine_4_1_instance_options(MachineState *machine)
>>>>  
>>>>  static void ccw_machine_4_1_class_options(MachineClass *mc)
>>>>  {
>>>> +    S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
>>>> +
>>>>      ccw_machine_4_2_class_options(mc);
>>>>      compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
>>>> +    s390mc->split_ram_layout = true;
>>>>  }
>>>>  DEFINE_CCW_MACHINE(4_1, "4.1", false);
>>>>  
>>>>  
>>>
>>> As discussed, I am not sure if adding that compat code really is worth
>>> it. :)  
>>
>> Agreed. As long as forward migration (e.g. 4.2->4.3 will work) and we document
>> that migration will fails for 2.12->.....4.1 for guests >=8TB we  can simplify
>> things.
> 
> I'll drop 3/4 and compat code in this patch.
> 
> Christian,
> 
> could you test this series on big enough host to ensure that KVM memslot splitting
> part works as expected (i.e. it's possible to start >8TB guest)?

Can you send a v3 with all changes?