On 02.08.19 15:32, 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. > > Beside an invalid use of API, the approach also introduced migration > issue, since RAM chunks for each KVM_SLOT_MAX_BYTES are transferred in > migration stream as separate RAMBlocks. > > After discussion [1], it was agreed to break migration from older > QEMU for guest with RAM >8Tb (as it was relatively new (since 2.12) > and considered to be not actually used downstream). > Migration should keep working for guests with less than 8TB and for > more than 8TB with QEMU 4.2 and newer binary. > In case user tries to migrate more than 8TB guest, between incompatible > QEMU versions, migration should fail gracefully due to non-exiting > RAMBlock ID or RAMBlock size mismatch. > > Taking in account above and that now KVM code is able to split too > big MemorySection into several memslots, stop abusing > memory_region_allocate_system_memory() > and use only one memory region for RAM. > > 1) [PATCH RFC v2 4/4] s390: do not call > memory_region_allocate_system_memory() multiple times > > Signed-off-by: Igor Mammedov <imamm...@redhat.com> > --- > v3: > - drop migration compat code > > PS: > I don't have access to a suitable system to test it. > --- > hw/s390x/s390-virtio-ccw.c | 21 +++------------------ > 1 file changed, 3 insertions(+), 18 deletions(-) > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index 0c03ffb7c7..e30058df38 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -154,27 +154,12 @@ static void virtio_ccw_register_hcalls(void) > static void s390_memory_init(ram_addr_t mem_size) > { > MemoryRegion *sysmem = get_system_memory(); > - ram_addr_t chunk, offset = 0; > - unsigned int number = 0; > + MemoryRegion *ram = g_new(MemoryRegion, 1); > Error *local_err = NULL; > - gchar *name; > > /* 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); > - } > - g_free(name); > + memory_region_allocate_system_memory(ram, NULL, "s390.ram", mem_size); > + memory_region_add_subregion(sysmem, 0, ram); > > /* > * Configure the maximum page size. As no memory devices were created >
Reviewed-by: David Hildenbrand <da...@redhat.com> We have to remember putting it into the next release notes. (or do we even want to get this into v4.1 ?) -- Thanks, David / dhildenb