Skip to contentSkip to conversation

pc: Fix startup with memory hotplug enabled

PATCH In review Peter Krempa 2 participants 4 messages Jan 29, 2015

lore
Share & tools
lore

Peter Krempa

author
#0 lore raw

v3 cover pc: Fix startup with memory hotplug enabled

Tweak error messages to make sense and add check to verify that maxmem_size is properly aligned right away rather than just crashing afterwards.

Peter Krempa (2):
vl.c: Fix error messages when parsing maxmem parameters
pc: memory: Validate alignment of maxram_size to page size

 hw/i386/pc.c |  7 +++++++
 vl.c         | 34 ++++++++++++++++------------------
 2 files changed, 23 insertions(+), 18 deletions(-)
Signature · 1 line
2.2.2
author
#1 lore raw

PATCH v3 1/2 vl.c: Fix error messages when parsing maxmem parameters

Produce more human readable error messages and fix few spelling mistakes.

Also remove a redundant check for the max memory size.

Signed-off-by Peter Krempa <[email protected]>

---

Notes:
Version 3:
- forgot a word in one of the error messages

Version 2:
- fixed spacing in error message
- changed control flow to allow maxmem == ram_size in case slots == 0

 vl.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)
vl.c +16 -18
index 983259b..b162465 100644
@@ -2694,29 +2694,27 @@ static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size)
         uint64_t slots;
 
         sz = qemu_opt_get_size(opts, "maxmem", 0);
+        slots = qemu_opt_get_number(opts, "slots", 0);
         if (sz < ram_size) {
-            error_report("invalid -m option value: maxmem "
-                    "(0x%" PRIx64 ") <= initial memory (0x"
-                    RAM_ADDR_FMT ")", sz, ram_size);
+            error_report("invalid value of -m option maxmem: "
+                         "maximum memory size (0x%" PRIx64 ") must be at least "
+                         "the initial memory size (0x" RAM_ADDR_FMT ")",
+                         sz, ram_size);
             exit(EXIT_FAILURE);
-        }
-
-        slots = qemu_opt_get_number(opts, "slots", 0);
-        if ((sz > ram_size) && !slots) {
-            error_report("invalid -m option value: maxmem "
-                    "(0x%" PRIx64 ") more than initial memory (0x"
-                    RAM_ADDR_FMT ") but no hotplug slots where "
-                    "specified", sz, ram_size);
+        } else if (sz > ram_size) {
+            if (!slots) {
+                error_report("invalid value of -m option: maxmem was "
+                             "specified, but no hotplug slots were specified");
+                exit(EXIT_FAILURE);
+            }
+        } else if (slots) {
+            error_report("invalid value of -m option maxmem: "
+                         "memory slots were specified but maximum memory size "
+                         "(0x%" PRIx64 ") is equal to the initial memory size "
+                         "(0x" RAM_ADDR_FMT ")", sz, ram_size);
             exit(EXIT_FAILURE);
         }
 
-        if ((sz <= ram_size) && slots) {
-            error_report("invalid -m option value:  %"
-                    PRIu64 " hotplug slots where specified but "
-                    "maxmem (0x%" PRIx64 ") <= initial memory (0x"
-                    RAM_ADDR_FMT ")", slots, sz, ram_size);
-            exit(EXIT_FAILURE);
-        }
         *maxram_size = sz;
         *ram_slots = slots;
     } else if ((!maxmem_str && slots_str) ||
Signature · 1 line
2.2.2
Reviewed-by on v3 1/2
#2 lore raw

Re: [Qemu-devel] [PATCHv3 1/2] vl.c: Fix error messages when parsing maxmem parameters

Quoted 12 lines · Peter Krempa

Produce more human readable error messages and fix few spelling mistakes.

Also remove a redundant check for the max memory size.

Signed-off-by Peter Krempa <[email protected]>

---

Notes:
Version 3:
- forgot a word in one of the error messages

Quoted 6 lines

Version 2:
- fixed spacing in error message
- changed control flow to allow maxmem == ram_size in case slots == 0

 vl.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)
Reviewed-by Eric Blake <[email protected]>
Signature · 2 lines
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org
author
#3 lore raw

PATCH v3 2/2 pc: memory: Validate alignment of maxram_size to page size

In reply to Peter Krempa

If the maxram_size is not aligned and dimm devices were added on the command line qemu would terminate with a rather unhelpful message:

ERROR:hw/mem/pc-dimm.c:150:pc_dimm_get_free_addr: assertion failed:
(QEMU_ALIGN_UP(address_space_size, align) == address_space_size)

In case no dimm device was originally added on the commandline qemu exits on the assertion failure.

Signed-off-by Peter Krempa <[email protected]>
Reviewed-by Eric Blake <[email protected]>
---
 hw/i386/pc.c | 7 +++++++
 1 file changed, 7 insertions(+)
hw/i386/pc.c +7 -0
index c7af6aa..8cf405a 100644
@@ -1246,6 +1246,13 @@ FWCfgState *pc_memory_init(MachineState *machine,
             exit(EXIT_FAILURE);
         }
 
+        if (QEMU_ALIGN_UP(machine->maxram_size,
+                          TARGET_PAGE_SIZE) != machine->maxram_size) {
+            error_report("maximum memory size must by aligned to multiple of "
+                         "%d bytes", TARGET_PAGE_SIZE);
+            exit(EXIT_FAILURE);
+        }
+
         pcms->hotplug_memory_base =
             ROUND_UP(0x100000000ULL + above_4g_mem_size, 1ULL << 30);
Signature · 1 line
2.2.2