diff mbox

[09/10] mipsn32-linux-user: Restrict address space to 31-bits.

Message ID 1360521050-29680-10-git-send-email-rth@twiddle.net
State New
Headers show

Commit Message

Richard Henderson Feb. 10, 2013, 6:30 p.m. UTC 
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 linux-user/main.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Richard Henderson Feb. 11, 2013, 4:08 p.m. UTC | #1 
On 2013-02-10 10:30, Richard Henderson wrote:
> Signed-off-by: Richard Henderson<rth@twiddle.net>
> ---
>   linux-user/main.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)

Just like o32, n32 will fail if we don't respect the 31-bit address limit.


r~
Peter Maydell Feb. 11, 2013, 5:44 p.m. UTC | #2 
On 10 February 2013 18:30, Richard Henderson <rth@twiddle.net> wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  linux-user/main.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 8c4dffd..25491ca 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -46,10 +46,10 @@ int gdbstub_port;
>  envlist_t *envlist;
>  const char *cpu_model;
>  unsigned long mmap_min_addr;
> +
>  #if defined(CONFIG_USE_GUEST_BASE)
>  unsigned long guest_base;
>  int have_guest_base;
> -#if (TARGET_LONG_BITS == 32) && (HOST_LONG_BITS == 64)
>  /*
>   * When running 32-on-64 we should make sure we can fit all of the possible
>   * guest address space into a contiguous chunk of virtual host memory.
> @@ -57,16 +57,16 @@ int have_guest_base;
>   * This way we will never overlap with our own libraries or binaries or stack
>   * or anything else that QEMU maps.
>   */
> -# ifdef TARGET_MIPS
> +# if HOST_LONG_BITS == 64 \
> +     && (defined(TARGET_ABI_MIPSO32) || defined(TARGET_ABI_MIPSN32))
>  /* MIPS only supports 31 bits of virtual address space for user space */
> -unsigned long reserved_va = 0x77000000;
> -# else
> +unsigned long reserved_va = 0x7f000000;
> +# elif (TARGET_LONG_BITS == 32) && (HOST_LONG_BITS == 64)
>  unsigned long reserved_va = 0xf7000000;
> -# endif
>  #else
>  unsigned long reserved_va;
>  #endif
> -#endif
> +#endif /* CONFIG_USE_GUEST_BASE */

Maybe clearer to pull out the "#if HOST_LONG_BITS == 64" into
its own #if rather than having it as an && term in both the #if
and the #elif?

I don't feel strongly either way though so
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM
Richard Henderson Feb. 11, 2013, 5:56 p.m. UTC | #3 
On 2013-02-11 09:44, Peter Maydell wrote:
>> >-# ifdef TARGET_MIPS
>> >+# if HOST_LONG_BITS == 64 \
>> >+     && (defined(TARGET_ABI_MIPSO32) || defined(TARGET_ABI_MIPSN32))
>> >  /* MIPS only supports 31 bits of virtual address space for user space */
>> >-unsigned long reserved_va = 0x77000000;
>> >-# else
>> >+unsigned long reserved_va = 0x7f000000;
>> >+# elif (TARGET_LONG_BITS == 32) && (HOST_LONG_BITS == 64)
>> >  unsigned long reserved_va = 0xf7000000;
>> >-# endif
>> >  #else
>> >  unsigned long reserved_va;
>> >  #endif
>> >-#endif
>> >+#endif /* CONFIG_USE_GUEST_BASE */
> Maybe clearer to pull out the "#if HOST_LONG_BITS == 64" into
> its own #if rather than having it as an && term in both the #if
> and the #elif?

In which case you get two "base" (unsigned long reserved_va;) cases, 
which didn't look clearer at all.  The only other thing I can think to 
do is

#if HLB == 64
# if MIPS
#  define RESERVED_MAX 0x7f000000
# elif TLB == 32
#  define RESERVED_MAX 0xf7000000
# endif
#endif
#ifndef RESERVED_MAX
# define RESERVED_MAX 0
#endif

unsigned long reserved_va = RESERVED_MAX;


r~
Peter Maydell Feb. 11, 2013, 5:58 p.m. UTC | #4 
On 11 February 2013 17:56, Richard Henderson <rth@twiddle.net> wrote:
> On 2013-02-11 09:44, Peter Maydell wrote:
>> Maybe clearer to pull out the "#if HOST_LONG_BITS == 64" into
>> its own #if rather than having it as an && term in both the #if
>> and the #elif?
>
>
> In which case you get two "base" (unsigned long reserved_va;) cases, which
> didn't look clearer at all.  The only other thing I can think to do is
>
> #if HLB == 64
> # if MIPS
> #  define RESERVED_MAX 0x7f000000
> # elif TLB == 32
> #  define RESERVED_MAX 0xf7000000
> # endif
> #endif
> #ifndef RESERVED_MAX
> # define RESERVED_MAX 0
> #endif
>
> unsigned long reserved_va = RESERVED_MAX;

I think that does look better. As I say, I don't feel
very strongly though.

-- PMM
Aurelien Jarno March 5, 2013, 2:03 p.m. UTC | #5 
On Sun, Feb 10, 2013 at 10:30:49AM -0800, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  linux-user/main.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 8c4dffd..25491ca 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -46,10 +46,10 @@ int gdbstub_port;
>  envlist_t *envlist;
>  const char *cpu_model;
>  unsigned long mmap_min_addr;
> +
>  #if defined(CONFIG_USE_GUEST_BASE)
>  unsigned long guest_base;
>  int have_guest_base;
> -#if (TARGET_LONG_BITS == 32) && (HOST_LONG_BITS == 64)
>  /*
>   * When running 32-on-64 we should make sure we can fit all of the possible
>   * guest address space into a contiguous chunk of virtual host memory.
> @@ -57,16 +57,16 @@ int have_guest_base;
>   * This way we will never overlap with our own libraries or binaries or stack
>   * or anything else that QEMU maps.
>   */
> -# ifdef TARGET_MIPS
> +# if HOST_LONG_BITS == 64 \
> +     && (defined(TARGET_ABI_MIPSO32) || defined(TARGET_ABI_MIPSN32))
>  /* MIPS only supports 31 bits of virtual address space for user space */
> -unsigned long reserved_va = 0x77000000;
> -# else
> +unsigned long reserved_va = 0x7f000000;

Is it really wanted to change 0x77 into 0x7f? If yes, I think the
commit log should explain why.

> +# elif (TARGET_LONG_BITS == 32) && (HOST_LONG_BITS == 64)
>  unsigned long reserved_va = 0xf7000000;
> -# endif
>  #else
>  unsigned long reserved_va;
>  #endif
> -#endif
> +#endif /* CONFIG_USE_GUEST_BASE */
>  
>  static void usage(void);
>  
> -- 
> 1.8.1.2
> 
>
Aurelien Jarno March 26, 2013, 7:50 p.m. UTC | #6 
On Tue, Mar 05, 2013 at 03:03:44PM +0100, Aurelien Jarno wrote:
> On Sun, Feb 10, 2013 at 10:30:49AM -0800, Richard Henderson wrote:
> > Signed-off-by: Richard Henderson <rth@twiddle.net>
> > ---
> >  linux-user/main.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/linux-user/main.c b/linux-user/main.c
> > index 8c4dffd..25491ca 100644
> > --- a/linux-user/main.c
> > +++ b/linux-user/main.c
> > @@ -46,10 +46,10 @@ int gdbstub_port;
> >  envlist_t *envlist;
> >  const char *cpu_model;
> >  unsigned long mmap_min_addr;
> > +
> >  #if defined(CONFIG_USE_GUEST_BASE)
> >  unsigned long guest_base;
> >  int have_guest_base;
> > -#if (TARGET_LONG_BITS == 32) && (HOST_LONG_BITS == 64)
> >  /*
> >   * When running 32-on-64 we should make sure we can fit all of the possible
> >   * guest address space into a contiguous chunk of virtual host memory.
> > @@ -57,16 +57,16 @@ int have_guest_base;
> >   * This way we will never overlap with our own libraries or binaries or stack
> >   * or anything else that QEMU maps.
> >   */
> > -# ifdef TARGET_MIPS
> > +# if HOST_LONG_BITS == 64 \
> > +     && (defined(TARGET_ABI_MIPSO32) || defined(TARGET_ABI_MIPSN32))
> >  /* MIPS only supports 31 bits of virtual address space for user space */
> > -unsigned long reserved_va = 0x77000000;
> > -# else
> > +unsigned long reserved_va = 0x7f000000;
> 
> Is it really wanted to change 0x77 into 0x7f? If yes, I think the
> commit log should explain why.
> 

Ping?
Richard Henderson March 26, 2013, 9:08 p.m. UTC | #7 
On 03/26/2013 12:50 PM, Aurelien Jarno wrote:
>>> > > -# ifdef TARGET_MIPS
>>> > > +# if HOST_LONG_BITS == 64 \
>>> > > +     && (defined(TARGET_ABI_MIPSO32) || defined(TARGET_ABI_MIPSN32))
>>> > >  /* MIPS only supports 31 bits of virtual address space for user space */
>>> > > -unsigned long reserved_va = 0x77000000;
>>> > > -# else
>>> > > +unsigned long reserved_va = 0x7f000000;
>> > 
>> > Is it really wanted to change 0x77 into 0x7f? If yes, I think the
>> > commit log should explain why.
>> > 
> Ping?

As far as I could remember, that 0x77 was totally arbitrary too,
and so I thought I'd "fix" it.  I don't recall more, sorry.


r~
diff mbox

Patch

diff --git a/linux-user/main.c b/linux-user/main.c
index 8c4dffd..25491ca 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -46,10 +46,10 @@  int gdbstub_port;
 envlist_t *envlist;
 const char *cpu_model;
 unsigned long mmap_min_addr;
+
 #if defined(CONFIG_USE_GUEST_BASE)
 unsigned long guest_base;
 int have_guest_base;
-#if (TARGET_LONG_BITS == 32) && (HOST_LONG_BITS == 64)
 /*
  * When running 32-on-64 we should make sure we can fit all of the possible
  * guest address space into a contiguous chunk of virtual host memory.
@@ -57,16 +57,16 @@  int have_guest_base;
  * This way we will never overlap with our own libraries or binaries or stack
  * or anything else that QEMU maps.
  */
-# ifdef TARGET_MIPS
+# if HOST_LONG_BITS == 64 \
+     && (defined(TARGET_ABI_MIPSO32) || defined(TARGET_ABI_MIPSN32))
 /* MIPS only supports 31 bits of virtual address space for user space */
-unsigned long reserved_va = 0x77000000;
-# else
+unsigned long reserved_va = 0x7f000000;
+# elif (TARGET_LONG_BITS == 32) && (HOST_LONG_BITS == 64)
 unsigned long reserved_va = 0xf7000000;
-# endif
 #else
 unsigned long reserved_va;
 #endif
-#endif
+#endif /* CONFIG_USE_GUEST_BASE */
 
 static void usage(void);