[Qemu-devel] [PATCH v2 0/2] Fix two bugs related to ram_size

[Qemu-devel] [PATCH v2 0/2] Fix two bugs related to ram_size

[Markus Armbruster]

There are more, but let's start with these two.

v2:
* Use QEMU_ALIGN_UP (Avi)
* Cover size overflow
* Coding style (Blue)

Markus Armbruster (2):
  vl: Round argument of -m up to multiple of 8KiB
  pc: Fix RTC CMOS info on RAM for ram_size < 1MiB

 hw/pc.c | 31 ++++++++++++++++++-------------
 vl.c    |  5 ++---
 2 files changed, 20 insertions(+), 16 deletions(-)

-- 
1.7.11.2

[Qemu-devel] [PATCH v2 1/2] vl: Round argument of -m up to multiple of 8KiB

[Markus Armbruster]

Partial pages make little sense and don't work.  Ensure the RAM size
is a multiple of any possible target's page size.

Fixes

    $ qemu-system-x86_64 -nodefaults -S -vnc :0 -m 0.8
    qemu-system-x86_64: /work/armbru/qemu/exec.c:2255: register_subpage: Assertion `existing->mr->subpage || existing->mr == &io_mem_unassigned' failed.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 vl.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/vl.c b/vl.c
index d01256a..dd6b861 100644
--- a/vl.c
+++ b/vl.c
@@ -2708,12 +2708,11 @@ int main(int argc, char **argv, char **envp)
                     fprintf(stderr, "qemu: invalid ram size: %s\n", optarg);
                     exit(1);
                 }
-
-                if (value != (uint64_t)(ram_addr_t)value) {
+                ram_size = QEMU_ALIGN_UP((uint64_t)value, 8192);
+                if (ram_size < value) {
                     fprintf(stderr, "qemu: ram size too large\n");
                     exit(1);
                 }
-                ram_size = value;
                 break;
             }
             case QEMU_OPTION_mempath:
-- 
1.7.11.2

[Qemu-devel] [PATCH v2 2/2] pc: Fix RTC CMOS info on RAM for ram_size < 1MiB

[Markus Armbruster]

pc_cmos_init() always claims 640KiB base memory, and ram_size - 1MiB
extended memory.  The latter can underflow to "lots of extended
memory".  Fix both, and clean up some.

Note: SeaBIOS currently requires 1MiB of RAM, and doesn't check
whether it got enough.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/pc.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index e8bcfc0..24df1e0 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -337,32 +337,37 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
     /* various important CMOS locations needed by PC/Bochs bios */
 
     /* memory size */
-    val = 640; /* base memory in K */
+    /* base memory (first MiB) */
+    val = MIN(ram_size / 1024, 640);
     rtc_set_memory(s, 0x15, val);
     rtc_set_memory(s, 0x16, val >> 8);
-
-    val = (ram_size / 1024) - 1024;
+    /* extended memory (next 64MiB) */
+    if (ram_size > 1024 * 1024) {
+        val = (ram_size - 1024 * 1024) / 1024;
+    } else {
+        val = 0;
+    }
     if (val > 65535)
         val = 65535;
     rtc_set_memory(s, 0x17, val);
     rtc_set_memory(s, 0x18, val >> 8);
     rtc_set_memory(s, 0x30, val);
     rtc_set_memory(s, 0x31, val >> 8);
-
-    if (above_4g_mem_size) {
-        rtc_set_memory(s, 0x5b, (unsigned int)above_4g_mem_size >> 16);
-        rtc_set_memory(s, 0x5c, (unsigned int)above_4g_mem_size >> 24);
-        rtc_set_memory(s, 0x5d, (uint64_t)above_4g_mem_size >> 32);
-    }
-
-    if (ram_size > (16 * 1024 * 1024))
-        val = (ram_size / 65536) - ((16 * 1024 * 1024) / 65536);
-    else
+    /* memory between 16MiB and 4GiB */
+    if (ram_size > 16 * 1024 * 1024) {
+        val = (ram_size - 16 * 1024 * 1024) / 65536;
+    } else {
         val = 0;
+    }
     if (val > 65535)
         val = 65535;
     rtc_set_memory(s, 0x34, val);
     rtc_set_memory(s, 0x35, val >> 8);
+    /* memory above 4GiB */
+    val = above_4g_mem_size / 65536;
+    rtc_set_memory(s, 0x5b, val);
+    rtc_set_memory(s, 0x5c, val >> 8);
+    rtc_set_memory(s, 0x5d, val >> 16);
 
     /* set the number of CPU */
     rtc_set_memory(s, 0x5f, smp_cpus - 1);
-- 
1.7.11.2

Re: [Qemu-devel] [PATCH v2 0/2] Fix two bugs related to ram_size

[Markus Armbruster]

Sent the wrong one.  v3 coming.  Sorry for the inconvenience.

Re: [Qemu-devel] [PATCH v2 0/2] Fix two bugs related to ram_size

[Michael Tokarev]

On 15.08.2012 13:31, Markus Armbruster wrote:
> There are more, but let's start with these two.

Yes, there are more of the same theme.  In particular,
why transparent huge pages support has never been
applied?  IIRC it went to nowhere after a question
about memory sizing and alignment popped up -- see
for example this thread:

 http://lists.gnu.org/archive/html/qemu-devel/2010-03/msg01250.html

It has nothing to do with the patchset, but...

Thanks,

/mjt

Re: [Qemu-devel] [PATCH v2 0/2] Fix two bugs related to ram_size

[Avi Kivity]

On 08/15/2012 02:44 PM, Michael Tokarev wrote:
> On 15.08.2012 13:31, Markus Armbruster wrote:
>> There are more, but let's start with these two.
> 
> Yes, there are more of the same theme.  In particular,
> why transparent huge pages support has never been
> applied?  IIRC it went to nowhere after a question
> about memory sizing and alignment popped up -- see
> for example this thread:
> 
>  http://lists.gnu.org/archive/html/qemu-devel/2010-03/msg01250.html

See 36b586284e678d.


-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH v2 0/2] Fix two bugs related to ram_size

[Michael Tokarev]

On 15.08.2012 15:46, Avi Kivity wrote:
> On 08/15/2012 02:44 PM, Michael Tokarev wrote:

>> Yes, there are more of the same theme.  In particular,
>> why transparent huge pages support has never been
>> applied?  IIRC it went to nowhere after a question
>> about memory sizing and alignment popped up -- see
>> for example this thread:
>>
>>  http://lists.gnu.org/archive/html/qemu-devel/2010-03/msg01250.html
> 
> See 36b586284e678d.

Oh.  I missed that one.  Thank you Avi!

Now I wonder why there's so many questions about THP not
being used with qemu-kvm.  I just fired up a random guest
with 1Gb memory -- AnonHugePages in /proc/meminfo is still
0.  Hum...

But this is a different topic now.  Thanks!

/mjt