diff mbox

Fix segfault with ram_size > 4095M without kvm

Message ID 20100223151314.GN17350@us.ibm.com
State New
Headers show

Commit Message

Ryan Harper Feb. 23, 2010, 3:13 p.m. UTC
Currently, x86_64-softmmu qemu segfaults when trying to use > 4095M memsize.
This patch adds a simple check and error message (much like the 2047 limit on
32-bit hosts) on ram_size in the control path after we determine we're
not using kvm

Upstream qemu-kvm is affected if using the -no-kvm option; this patch address
the segfault there as well.

Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
---
 vl.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

Comments

Aurelien Jarno Feb. 23, 2010, 5:02 p.m. UTC | #1
Ryan Harper a écrit :
> Currently, x86_64-softmmu qemu segfaults when trying to use > 4095M memsize.
> This patch adds a simple check and error message (much like the 2047 limit on
> 32-bit hosts) on ram_size in the control path after we determine we're
> not using kvm
> 
> Upstream qemu-kvm is affected if using the -no-kvm option; this patch address
> the segfault there as well.

It looks like workarounding the real bug. At some point both
i386-softmmu (via PAE) and x86_64-softmmu were able to support > 4GB of
memory. I remember adding the support long time ago, and testing it with
32GB of emulated RAM.


> Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
> ---
>  vl.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index db7a178..a659e98 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -5760,6 +5760,12 @@ int main(int argc, char **argv, char **envp)
>              fprintf(stderr, "failed to initialize KVM\n");
>              exit(1);
>          }
> +    } else {
> +        /* without kvm enabled, we can only support 4095 MB RAM */
> +        if (ram_size > (4095UL << 20)) {
> +            fprintf(stderr, "qemu: without kvm support at most 4095 MB RAM can be simulated\n");
> +            exit(1);
> +        }
>      }
>  
>      if (qemu_init_main_loop()) {
Alexander Graf Feb. 23, 2010, 8:30 p.m. UTC | #2
On 23.02.2010, at 18:02, Aurelien Jarno wrote:

> Ryan Harper a écrit :
>> Currently, x86_64-softmmu qemu segfaults when trying to use > 4095M memsize.
>> This patch adds a simple check and error message (much like the 2047 limit on
>> 32-bit hosts) on ram_size in the control path after we determine we're
>> not using kvm
>> 
>> Upstream qemu-kvm is affected if using the -no-kvm option; this patch address
>> the segfault there as well.
> 
> It looks like workarounding the real bug. At some point both
> i386-softmmu (via PAE) and x86_64-softmmu were able to support > 4GB of
> memory. I remember adding the support long time ago, and testing it with
> 32GB of emulated RAM.

Sounds like a perfect candidate for -stable then. For HEAD I agree that finding the cause would be the way to go.


Alex
Anthony Liguori Feb. 23, 2010, 9:07 p.m. UTC | #3
On 02/23/2010 02:30 PM, Alexander Graf wrote:
> On 23.02.2010, at 18:02, Aurelien Jarno wrote:
>
>    
>> Ryan Harper a écrit :
>>      
>>> Currently, x86_64-softmmu qemu segfaults when trying to use>  4095M memsize.
>>> This patch adds a simple check and error message (much like the 2047 limit on
>>> 32-bit hosts) on ram_size in the control path after we determine we're
>>> not using kvm
>>>
>>> Upstream qemu-kvm is affected if using the -no-kvm option; this patch address
>>> the segfault there as well.
>>>        
>> It looks like workarounding the real bug. At some point both
>> i386-softmmu (via PAE) and x86_64-softmmu were able to support>  4GB of
>> memory. I remember adding the support long time ago, and testing it with
>> 32GB of emulated RAM.
>>      
> Sounds like a perfect candidate for -stable then. For HEAD I agree that finding the cause would be the way to go.
>    

No, it's wrong.  A good candidate for -stable would be something that 
fixes the SEGV :-)

Regards,

Anthony Liguori

> Alex--
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Aurelien Jarno Feb. 23, 2010, 9:24 p.m. UTC | #4
On Tue, Feb 23, 2010 at 03:07:20PM -0600, Anthony Liguori wrote:
> On 02/23/2010 02:30 PM, Alexander Graf wrote:
> >On 23.02.2010, at 18:02, Aurelien Jarno wrote:
> >
> >>Ryan Harper a écrit :
> >>>Currently, x86_64-softmmu qemu segfaults when trying to use>  4095M memsize.
> >>>This patch adds a simple check and error message (much like the 2047 limit on
> >>>32-bit hosts) on ram_size in the control path after we determine we're
> >>>not using kvm
> >>>
> >>>Upstream qemu-kvm is affected if using the -no-kvm option; this patch address
> >>>the segfault there as well.
> >>It looks like workarounding the real bug. At some point both
> >>i386-softmmu (via PAE) and x86_64-softmmu were able to support>  4GB of
> >>memory. I remember adding the support long time ago, and testing it with
> >>32GB of emulated RAM.
> >Sounds like a perfect candidate for -stable then. For HEAD I agree that finding the cause would be the way to go.
> 
> No, it's wrong.  A good candidate for -stable would be something
> that fixes the SEGV :-)
> 

It actually depends on the patch and how invasive it is.

I'll bisect that later this week. For now what I can say it hasn't
worked for a lot of time. It works in 0.9.1, but not in 0.10.0. It
probably hasn't been noticed due to kqemu which was limiting the 
size to 2GB.
Ryan Harper Feb. 23, 2010, 10:55 p.m. UTC | #5
* Aurelien Jarno <aurelien@aurel32.net> [2010-02-23 11:37]:
> Ryan Harper a écrit :
> > Currently, x86_64-softmmu qemu segfaults when trying to use > 4095M memsize.
> > This patch adds a simple check and error message (much like the 2047 limit on
> > 32-bit hosts) on ram_size in the control path after we determine we're
> > not using kvm
> > 
> > Upstream qemu-kvm is affected if using the -no-kvm option; this patch address
> > the segfault there as well.
> 
> It looks like workarounding the real bug. At some point both
> i386-softmmu (via PAE) and x86_64-softmmu were able to support > 4GB of
> memory. I remember adding the support long time ago, and testing it with
> 32GB of emulated RAM.

Indeed it was a workaround.  I thought it was reasonable since we cap
the size for 32-bit at 2047; but Anthony mentioned that on 64-bit hosts
some targets had >4G support so we should fix the segfault.  here is the
backtrace I got from the core file:

Core was generated by `./x86_64-softmmu/qemu-system-x86_64 -m 4096'.
Program terminated with signal 11, Segmentation fault.
#0  0x00000000004d1a59 in tb_alloc_page (tb=0x7f33d111d010, n=0, page_addr=4295094272)
    at /home/rharper/work/git/qemu/exec.c:1125
1125        tb->page_next[n] = p->first_tb;
(gdb) bt
#0  0x00000000004d1a59 in tb_alloc_page (tb=0x7f33d111d010, n=0, page_addr=4295094272)
    at /home/rharper/work/git/qemu/exec.c:1125
#1  0x00000000004d1bf1 in tb_link_phys (tb=0x7f33d111d010, phys_pc=4295098352, phys_page2=18446744073709551615)
    at /home/rharper/work/git/qemu/exec.c:1215
#2  0x00000000004d1612 in tb_gen_code (env=0x2180ed0, pc=4294967280, cs_base=4294901760, flags=68, cflags=0)
    at /home/rharper/work/git/qemu/exec.c:913
#3  0x00000000004d849c in tb_find_slow (pc=4294967280, cs_base=4294901760, flags=68)
    at /home/rharper/work/git/qemu/cpu-exec.c:161
#4  0x00000000004d85b2 in tb_find_fast () at /home/rharper/work/git/qemu/cpu-exec.c:182
#5  0x00000000004d8cdc in cpu_x86_exec (env1=0x2180ed0) at /home/rharper/work/git/qemu/cpu-exec.c:579
#6  0x000000000040d686 in qemu_cpu_exec (env=0x2180ed0) at /home/rharper/work/git/qemu/vl.c:3895
#7  0x000000000040d76b in tcg_cpu_exec () at /home/rharper/work/git/qemu/vl.c:3924
#8  0x000000000040da39 in main_loop () at /home/rharper/work/git/qemu/vl.c:4042
#9  0x0000000000411a2f in main (argc=3, argv=0x7fff5782ab08, envp=0x7fff5782ab28)
    at /home/rharper/work/git/qemu/vl.c:6102


> 
> 
> > Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
> > ---
> >  vl.c |    6 ++++++
> >  1 files changed, 6 insertions(+), 0 deletions(-)
> > 
> > diff --git a/vl.c b/vl.c
> > index db7a178..a659e98 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -5760,6 +5760,12 @@ int main(int argc, char **argv, char **envp)
> >              fprintf(stderr, "failed to initialize KVM\n");
> >              exit(1);
> >          }
> > +    } else {
> > +        /* without kvm enabled, we can only support 4095 MB RAM */
> > +        if (ram_size > (4095UL << 20)) {
> > +            fprintf(stderr, "qemu: without kvm support at most 4095 MB RAM can be simulated\n");
> > +            exit(1);
> > +        }
> >      }
> >  
> >      if (qemu_init_main_loop()) {
> 
> 
> -- 
> Aurelien Jarno                          GPG: 1024D/F1BCDB73
> aurelien@aurel32.net                 http://www.aurel32.net
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/vl.c b/vl.c
index db7a178..a659e98 100644
--- a/vl.c
+++ b/vl.c
@@ -5760,6 +5760,12 @@  int main(int argc, char **argv, char **envp)
             fprintf(stderr, "failed to initialize KVM\n");
             exit(1);
         }
+    } else {
+        /* without kvm enabled, we can only support 4095 MB RAM */
+        if (ram_size > (4095UL << 20)) {
+            fprintf(stderr, "qemu: without kvm support at most 4095 MB RAM can be simulated\n");
+            exit(1);
+        }
     }
 
     if (qemu_init_main_loop()) {