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

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

[Markus Armbruster]

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

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 | 27 +++++++++++++++------------
 vl.c    |  4 +++-
 2 files changed, 18 insertions(+), 13 deletions(-)

-- 
1.7.11.2

[Qemu-devel] [PATCH 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 -monitor stdio -m 0.8
QEMU 1.1.50 monitor - type 'help' for more information
(qemu) 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>
---
See also
http://lists.nongnu.org/archive/html/qemu-devel/2012-06/msg02813.html

 vl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/vl.c b/vl.c
index d01256a..b411d45 100644
--- a/vl.c
+++ b/vl.c
@@ -2708,11 +2708,13 @@ 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) {
                     fprintf(stderr, "qemu: ram size too large\n");
                     exit(1);
                 }
+                if (value & 0x1fff) {
+                    value = (value + 0x1fff) & ~0x1fff;
+                }
                 ram_size = value;
                 break;
             }
-- 
1.7.11.2

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

[Avi Kivity]

On 08/14/2012 02:58 PM, Markus Armbruster wrote:
> Partial pages make little sense and don't work.  Ensure the RAM size
> is a multiple of any possible target's page size.
> 
> index d01256a..b411d45 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2708,11 +2708,13 @@ 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) {
>                      fprintf(stderr, "qemu: ram size too large\n");
>                      exit(1);
>                  }
> +                if (value & 0x1fff) {
> +                    value = (value + 0x1fff) & ~0x1fff;
> +                }

value = QEMU_ALIGN_UP(value, 8192);


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

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

[Markus Armbruster]

Avi Kivity <avi@redhat.com> writes:

> On 08/14/2012 02:58 PM, Markus Armbruster wrote:
>> Partial pages make little sense and don't work.  Ensure the RAM size
>> is a multiple of any possible target's page size.
>> 
>> index d01256a..b411d45 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2708,11 +2708,13 @@ 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) {
>>                      fprintf(stderr, "qemu: ram size too large\n");
>>                      exit(1);
>>                  }
>> +                if (value & 0x1fff) {
>> +                    value = (value + 0x1fff) & ~0x1fff;
>> +                }
>
> value = QEMU_ALIGN_UP(value, 8192);

I looked for such a macro, but my greps missed.  Thanks!

[Qemu-devel] [PATCH 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 | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index e8bcfc0..1597fe6 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -337,32 +337,35 @@ 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);
+    /* 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 2/2] pc: Fix RTC CMOS info on RAM for ram_size < 1MiB

[Blue Swirl]

On Tue, Aug 14, 2012 at 11:58 AM, Markus Armbruster <armbru@redhat.com> wrote:
> 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 | 27 +++++++++++++++------------
>  1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/hw/pc.c b/hw/pc.c
> index e8bcfc0..1597fe6 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -337,32 +337,35 @@ 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)

Please add braces.

> +        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);
> +    /* memory between 16MiB and 4GiB */
> +    if (ram_size > 16 * 1024 * 1024)

Also here.

> +        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 2/2] pc: Fix RTC CMOS info on RAM for ram_size < 1MiB

[Markus Armbruster]

Blue Swirl <blauwirbel@gmail.com> writes:

> On Tue, Aug 14, 2012 at 11:58 AM, Markus Armbruster <armbru@redhat.com> wrote:
>> 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 | 27 +++++++++++++++------------
>>  1 file changed, 15 insertions(+), 12 deletions(-)
>>
>> diff --git a/hw/pc.c b/hw/pc.c
>> index e8bcfc0..1597fe6 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -337,32 +337,35 @@ 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)
>
> Please add braces.

Done in v2.

[...]