qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCHv3 0/2] pc: Fix startup with memory hotplug enabled
@ 2015-01-29 13:48 Peter Krempa
  2015-01-29 13:48 ` [Qemu-devel] [PATCHv3 1/2] vl.c: Fix error messages when parsing maxmem parameters Peter Krempa
  2015-01-29 13:48 ` [Qemu-devel] [PATCHv3 2/2] pc: memory: Validate alignment of maxram_size to page size Peter Krempa
  0 siblings, 2 replies; 4+ messages in thread
From: Peter Krempa @ 2015-01-29 13:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Krempa, Michael S. Tsirkin

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(-)

-- 
2.2.2

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Qemu-devel] [PATCHv3 1/2] vl.c: Fix error messages when parsing maxmem parameters
  2015-01-29 13:48 [Qemu-devel] [PATCHv3 0/2] pc: Fix startup with memory hotplug enabled Peter Krempa
@ 2015-01-29 13:48 ` Peter Krempa
  2015-01-29 13:57   ` Eric Blake
  2015-01-29 13:48 ` [Qemu-devel] [PATCHv3 2/2] pc: memory: Validate alignment of maxram_size to page size Peter Krempa
  1 sibling, 1 reply; 4+ messages in thread
From: Peter Krempa @ 2015-01-29 13:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Krempa, Michael S. Tsirkin

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 <pkrempa@redhat.com>
---

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(-)

diff --git a/vl.c b/vl.c
index 983259b..b162465 100644
--- a/vl.c
+++ b/vl.c
@@ -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) ||
-- 
2.2.2

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [Qemu-devel] [PATCHv3 2/2] pc: memory: Validate alignment of maxram_size to page size
  2015-01-29 13:48 [Qemu-devel] [PATCHv3 0/2] pc: Fix startup with memory hotplug enabled Peter Krempa
  2015-01-29 13:48 ` [Qemu-devel] [PATCHv3 1/2] vl.c: Fix error messages when parsing maxmem parameters Peter Krempa
@ 2015-01-29 13:48 ` Peter Krempa
  1 sibling, 0 replies; 4+ messages in thread
From: Peter Krempa @ 2015-01-29 13:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Krempa, Michael S. Tsirkin

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 <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 hw/i386/pc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index c7af6aa..8cf405a 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -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);

-- 
2.2.2

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCHv3 1/2] vl.c: Fix error messages when parsing maxmem parameters
  2015-01-29 13:48 ` [Qemu-devel] [PATCHv3 1/2] vl.c: Fix error messages when parsing maxmem parameters Peter Krempa
@ 2015-01-29 13:57   ` Eric Blake
  0 siblings, 0 replies; 4+ messages in thread
From: Eric Blake @ 2015-01-29 13:57 UTC (permalink / raw)
  To: Peter Krempa, qemu-devel; +Cc: Michael S. Tsirkin

[-- Attachment #1: Type: text/plain, Size: 742 bytes --]

On 01/29/2015 06:48 AM, Peter Krempa wrote:
> 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 <pkrempa@redhat.com>
> ---
> 
> 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(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-01-29 14:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-29 13:48 [Qemu-devel] [PATCHv3 0/2] pc: Fix startup with memory hotplug enabled Peter Krempa
2015-01-29 13:48 ` [Qemu-devel] [PATCHv3 1/2] vl.c: Fix error messages when parsing maxmem parameters Peter Krempa
2015-01-29 13:57   ` Eric Blake
2015-01-29 13:48 ` [Qemu-devel] [PATCHv3 2/2] pc: memory: Validate alignment of maxram_size to page size Peter Krempa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).