[PATCH v2] s390x: Reject unaligned RAM sizes

[David Hildenbrand]

Historically, we fixed up the RAM size (rounded it down), to fit into
storage increments (maximum number of storage increments is 1020). Since
commit 3a12fc61af5c ("390x/s390-virtio-ccw: use memdev for RAM"), we no
longer consider the fixed-up size when allcoating the RAM block - which
will break migration.

Let's simply drop that manual fixup code and let the user supply sane
RAM sizes. This will bail out early when trying to migrate (and make
an existing guest with e.g., 12345 MB non-migratable), but maybe we
should have rejected such RAM sizes right from the beginning. One can
consider it a BUG that we don't supply what the user asked for.

As we no longer fixup maxram_size as well, make other users use ram_size
instead. Keep using maxram_size when setting the maximum ram size in KVM,
as that will come in handy in the future when supporting memory hotplug
(in contrast, storage keys and storage attributes for hotplugged memory
 will have to be migrated per RAM block in the future).

This fixes (or rather rejects early):

1. Migrating older QEMU to upstream QEMU (e.g., with "-m 1235M"), as the
   RAM block size changed.

2. Migrating upstream QEMU to upstream QEMU (e.g., with "-m 1235M"), as
   we receive storage attributes for memory we don't expect (as we fixed up
   ram_size and maxram_size).

Note that a migration from older QEMU is still possible - the migration
target has to be started with the properly (down) aligned size (e.g.,
"-m 1234M" instead of "-m 1235M"). The following table can be used to write
a conversion script to automate migration in environments where
this is relevant.

 VM size (<=) | Alignment
--------------------------
      1020M   |     1M
      2040M   |     2M
      4080M   |     4M
      8160M   |     8M
     16320M   |    16M
     32640M   |    32M
     65280M   |    64M
    130560M   |   128M
    261120M   |   256M
    522240M   |   512M
   1044480M   |     1G
   2088960M   |     2G
   4177920M   |     4G
   8355840M   |     8G

Fixes: 3a12fc61af5c ("390x/s390-virtio-ccw: use memdev for RAM")
Reported-by: Lukáš Doktor <address@hidden>
Cc: Igor Mammedov <address@hidden>
Cc: Dr. David Alan Gilbert <address@hidden>
Signed-off-by: David Hildenbrand <address@hidden>
---

v1 -> v2:
- Don't use global ram_size
- Add more details how migration from older QEMU can still work, along
  with a table.

---
 hw/s390x/s390-skeys.c        |  2 +-
 hw/s390x/s390-stattrib-kvm.c |  4 ++--
 hw/s390x/sclp.c              | 20 +++++++++++---------
 3 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
index 5da6e5292f..a9a4ae7b39 100644
--- a/hw/s390x/s390-skeys.c
+++ b/hw/s390x/s390-skeys.c
@@ -176,7 +176,7 @@ static void qemu_s390_skeys_init(Object *obj)
     QEMUS390SKeysState *skeys = QEMU_S390_SKEYS(obj);
     MachineState *machine = MACHINE(qdev_get_machine());
 
-    skeys->key_count = machine->maxram_size / TARGET_PAGE_SIZE;
+    skeys->key_count = machine->ram_size / TARGET_PAGE_SIZE;
     skeys->keydata = g_malloc0(skeys->key_count);
 }
 
diff --git a/hw/s390x/s390-stattrib-kvm.c b/hw/s390x/s390-stattrib-kvm.c
index c7e1f35524..f89d8d9d16 100644
--- a/hw/s390x/s390-stattrib-kvm.c
+++ b/hw/s390x/s390-stattrib-kvm.c
@@ -85,7 +85,7 @@ static int kvm_s390_stattrib_set_stattr(S390StAttribState *sa,
 {
     KVMS390StAttribState *sas = KVM_S390_STATTRIB(sa);
     MachineState *machine = MACHINE(qdev_get_machine());
-    unsigned long max = machine->maxram_size / TARGET_PAGE_SIZE;
+    unsigned long max = machine->ram_size / TARGET_PAGE_SIZE;
 
     if (start_gfn + count > max) {
         error_report("Out of memory bounds when setting storage attributes");
@@ -104,7 +104,7 @@ static void kvm_s390_stattrib_synchronize(S390StAttribState 
*sa)
 {
     KVMS390StAttribState *sas = KVM_S390_STATTRIB(sa);
     MachineState *machine = MACHINE(qdev_get_machine());
-    unsigned long max = machine->maxram_size / TARGET_PAGE_SIZE;
+    unsigned long max = machine->ram_size / TARGET_PAGE_SIZE;
     /* We do not need to reach the maximum buffer size allowed */
     unsigned long cx, len = KVM_S390_SKEYS_MAX / 2;
     int r;
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index af0bfbc2ec..bbf6364511 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -327,7 +327,7 @@ out:
 static void sclp_memory_init(SCLPDevice *sclp)
 {
     MachineState *machine = MACHINE(qdev_get_machine());
-    ram_addr_t initial_mem = machine->ram_size;
+    uint64_t initial_mem = machine->ram_size;
     int increment_size = 20;
 
     /* The storage increment size is a multiple of 1M and is a power of 2.
@@ -339,15 +339,17 @@ static void sclp_memory_init(SCLPDevice *sclp)
     }
     sclp->increment_size = increment_size;
 
-    /* The core memory area needs to be aligned with the increment size.
-     * In effect, this can cause the user-specified memory size to be rounded
-     * down to align with the nearest increment boundary. */
+    /*
+     * The core memory area needs to be aligned to the increment size. In
+     * case it's not aligned, bail out.
+     */
     initial_mem = initial_mem >> increment_size << increment_size;
-
-    machine->ram_size = initial_mem;
-    machine->maxram_size = initial_mem;
-    /* let's propagate the changed ram size into the global variable. */
-    ram_size = initial_mem;
+    if (initial_mem != machine->ram_size) {
+        error_report("RAM size not aligned to storage increments."
+                     " Possible aligned RAM size: %" PRIu64 " MB",
+                     initial_mem / MiB);
+        exit(1);
+    }
 }
 
 static void sclp_init(Object *obj)
-- 
2.25.1