Patchwork Fix segfault with ram_size > 4095M without kvm

login
register
mail settings
Submitter Aurelien Jarno
Date March 4, 2010, 9:27 p.m.
Message ID <20100304212724.GH5860@hall.aurel32.net>
Download mbox | patch
Permalink /patch/46980/
State New
Headers show

Comments

Aurelien Jarno - March 4, 2010, 9:27 p.m.
On Tue, Feb 23, 2010 at 06:02:15PM +0100, 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.

I have looked into that, and actually one patch to get full support for
 > 4GB of memory was not merged:


While this patch is acceptable for qemu i386, it creates a big L1 table
for x86_64 or other 64-bit architectures, resulting in huge memory 
overhead.

The recent multilevel tables patches from Richard Henderson should fix 
the problem for HEAD (I haven't found time to look at them in details).

As this is not something we really want to backport, your patch makes
sense in stable-0.12.


> > 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
> 
> 
>
Ryan Harper - March 4, 2010, 9:34 p.m.
* Aurelien Jarno <aurelien@aurel32.net> [2010-03-04 15:27]:
> On Tue, Feb 23, 2010 at 06:02:15PM +0100, 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.
> 
> I have looked into that, and actually one patch to get full support for
>  > 4GB of memory was not merged:

Thanks for looking into this.

> 
> diff --git a/exec.c b/exec.c
> index 8389c54..b0bb058 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -166,7 +166,7 @@ typedef struct PhysPageDesc {
>   */
>  #define L1_BITS (TARGET_VIRT_ADDR_SPACE_BITS - L2_BITS - TARGET_PAGE_BITS)
>  #else
> -#define L1_BITS (32 - L2_BITS - TARGET_PAGE_BITS)
> +#define L1_BITS (TARGET_PHYS_ADDR_SPACE_BITS - L2_BITS - TARGET_PAGE_BITS)
>  #endif
> 
>  #define L1_SIZE (1 << L1_BITS)
> 
> While this patch is acceptable for qemu i386, it creates a big L1 table
> for x86_64 or other 64-bit architectures, resulting in huge memory 
> overhead.
> 
> The recent multilevel tables patches from Richard Henderson should fix 
> the problem for HEAD (I haven't found time to look at them in details).
> 
> As this is not something we really want to backport, your patch makes
> sense in stable-0.12.

Anthony, do you want me to resend and rebase against 0.12-stable?
Aurelien Jarno - March 6, 2010, 9:31 p.m.
On Thu, Mar 04, 2010 at 03:34:34PM -0600, Ryan Harper wrote:
> * Aurelien Jarno <aurelien@aurel32.net> [2010-03-04 15:27]:
> > On Tue, Feb 23, 2010 at 06:02:15PM +0100, 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.
> > 
> > I have looked into that, and actually one patch to get full support for
> >  > 4GB of memory was not merged:
> 
> Thanks for looking into this.
> 
> > 
> > diff --git a/exec.c b/exec.c
> > index 8389c54..b0bb058 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -166,7 +166,7 @@ typedef struct PhysPageDesc {
> >   */
> >  #define L1_BITS (TARGET_VIRT_ADDR_SPACE_BITS - L2_BITS - TARGET_PAGE_BITS)
> >  #else
> > -#define L1_BITS (32 - L2_BITS - TARGET_PAGE_BITS)
> > +#define L1_BITS (TARGET_PHYS_ADDR_SPACE_BITS - L2_BITS - TARGET_PAGE_BITS)
> >  #endif
> > 
> >  #define L1_SIZE (1 << L1_BITS)
> > 
> > While this patch is acceptable for qemu i386, it creates a big L1 table
> > for x86_64 or other 64-bit architectures, resulting in huge memory 
> > overhead.
> > 
> > The recent multilevel tables patches from Richard Henderson should fix 
> > the problem for HEAD (I haven't found time to look at them in details).
> > 
> > As this is not something we really want to backport, your patch makes
> > sense in stable-0.12.
> 
> Anthony, do you want me to resend and rebase against 0.12-stable?
> 

The patch applies correctly on stable-0.12. I have just applied it.

Patch

diff --git a/exec.c b/exec.c
index 8389c54..b0bb058 100644
--- a/exec.c
+++ b/exec.c
@@ -166,7 +166,7 @@  typedef struct PhysPageDesc {
  */
 #define L1_BITS (TARGET_VIRT_ADDR_SPACE_BITS - L2_BITS - TARGET_PAGE_BITS)
 #else
-#define L1_BITS (32 - L2_BITS - TARGET_PAGE_BITS)
+#define L1_BITS (TARGET_PHYS_ADDR_SPACE_BITS - L2_BITS - TARGET_PAGE_BITS)
 #endif
 
 #define L1_SIZE (1 << L1_BITS)