[Qemu-devel] [PATCH 0/2] Probe the guest memory space when using -R

[Qemu-devel] [PATCH 0/2] Probe the guest memory space when using -R

[Meador Inge]

Hi,

This patch series fixes an issue that was discussed here [1] where -R
can fail when the mapped address space fails validation.  I fixed this
issue by (1) refactoring the guest space probing code into a single
function for initialing the guest space and (2) by calling the guest space
initializing code for both the case of reserving the guest memory space
upfront (-R) and the case where the initial memory space base/size are gleaned
from an ELF image.

Tested by going through various combinations of -R <size>, -B <base>,
-B <base> -R <size>, and neither -R or -B passed.  I also ran the libstdc++
testsuite through the MIPS, ARM, and Power usermode emulators with -R set.
No regressions.

NOTE: This does not fix the problem that was raised concerning mapped the
full 32-bit address space on a 64-bit system.  That will need to be another
patch.

[1] http://lists.nongnu.org/archive/html/qemu-devel/2012-06/msg04508.html

Signed-off-by: Meador Inge <meadori@codesourcery.com>

Meador Inge (2):
  linux-user: Factor out guest space probing into a function
  linux-user: Use init_guest_space when -R and -B are specified

 linux-user/elfload.c |  162 ++++++++++++++++++++++++++++++++++++++------------
 linux-user/main.c    |   35 ++---------
 linux-user/qemu.h    |   13 +++-
 3 files changed, 139 insertions(+), 71 deletions(-)

-- 
1.7.7.6

[Qemu-devel] [PATCH 1/2] linux-user: Factor out guest space probing into a function

[Meador Inge]

Signed-off-by: Meador Inge <meadori@codesourcery.com>
---
 linux-user/elfload.c |  111 +++++++++++++++++++++++++++++++++++---------------
 linux-user/qemu.h    |   11 +++++
 2 files changed, 89 insertions(+), 33 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index f3b1552..44b4bdb 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1385,6 +1385,74 @@ bool guest_validate_base(unsigned long guest_base)
 }
 #endif
 
+unsigned long init_guest_space(unsigned long host_start,
+                               unsigned long host_size,
+                               bool fixed)
+{
+    unsigned long current_start, real_start;
+    int flags;
+
+    /* Nothing to do here, move along.  */
+    if (!host_start && !host_size) {
+        return 0;
+    }
+
+    /* If just a starting address is given, then just verify that
+     * address.  */
+    if (host_start && !host_size) {
+        if (guest_validate_base(host_start)) {
+            return host_start;
+        } else {
+            return (unsigned long)-1;
+        }
+    }
+
+    /* Setup the initial flags and start address.  */
+    current_start = host_start & qemu_host_page_mask;
+    flags = MAP_ANONYMOUS | MAP_PRIVATE | MAP_NORESERVE;
+    if (fixed) {
+        flags |= MAP_FIXED;
+    }
+
+    /* Otherwise, a non-zero size region of memory needs to be mapped
+     * and validated.  */
+    while (1) {
+        /* Do not use mmap_find_vma here because that is limited to the
+         * guest address space.  We are going to make the
+         * guest address space fit whatever we're given.
+         */
+        real_start = (unsigned long)
+            mmap((void *)current_start, host_size, PROT_NONE, flags, -1, 0);
+        if (real_start == (unsigned long)-1) {
+            return (unsigned long)-1;
+        }
+
+        if ((real_start == current_start) && guest_validate_base(real_start)) {
+            break;
+        }
+
+        /* That address didn't work.  Unmap and try a different one.
+         * The address the host picked because is typically right at
+         * the top of the host address space and leaves the guest with
+         * no usable address space.  Resort to a linear search.  We
+         * already compensated for mmap_min_addr, so this should not
+         * happen often.  Probably means we got unlucky and host
+         * address space randomization put a shared library somewhere
+         * inconvenient.
+         */
+        munmap((void *)real_start, host_size);
+        current_start += qemu_host_page_size;
+        if (host_start == current_start) {
+            /* Theoretically possible if host doesn't have any suitably
+             * aligned areas.  Normally the first mmap will fail.
+             */
+            return (unsigned long)-1;
+        }
+    }
+
+    return real_start;
+}
+
 static void probe_guest_base(const char *image_name,
                              abi_ulong loaddr, abi_ulong hiaddr)
 {
@@ -1411,46 +1479,23 @@ static void probe_guest_base(const char *image_name,
             }
         }
         host_size = hiaddr - loaddr;
-        while (1) {
-            /* Do not use mmap_find_vma here because that is limited to the
-               guest address space.  We are going to make the
-               guest address space fit whatever we're given.  */
-            real_start = (unsigned long)
-                mmap((void *)host_start, host_size, PROT_NONE,
-                     MAP_ANONYMOUS | MAP_PRIVATE | MAP_NORESERVE, -1, 0);
-            if (real_start == (unsigned long)-1) {
-                goto exit_perror;
-            }
-            guest_base = real_start - loaddr;
-            if ((real_start == host_start) &&
-                guest_validate_base(guest_base)) {
-                break;
-            }
-            /* That address didn't work.  Unmap and try a different one.
-               The address the host picked because is typically right at
-               the top of the host address space and leaves the guest with
-               no usable address space.  Resort to a linear search.  We
-               already compensated for mmap_min_addr, so this should not
-               happen often.  Probably means we got unlucky and host
-               address space randomization put a shared library somewhere
-               inconvenient.  */
-            munmap((void *)real_start, host_size);
-            host_start += qemu_host_page_size;
-            if (host_start == loaddr) {
-                /* Theoretically possible if host doesn't have any suitably
-                   aligned areas.  Normally the first mmap will fail.  */
-                errmsg = "Unable to find space for application";
-                goto exit_errmsg;
-            }
+
+        /* Setup the initial guest memory space with ranges gleaned from
+         * the ELF image that is being loaded.
+         */
+        real_start = init_guest_space(host_start, host_size, 0);
+        if (real_start == (unsigned long)-1) {
+            errmsg = "Unable to find space for application";
+            goto exit_errmsg;
         }
+        guest_base = real_start - loaddr;
+
         qemu_log("Relocating guest address space from 0x"
                  TARGET_ABI_FMT_lx " to 0x%lx\n",
                  loaddr, real_start);
     }
     return;
 
-exit_perror:
-    errmsg = strerror(errno);
 exit_errmsg:
     fprintf(stderr, "%s: %s\n", image_name, errmsg);
     exit(-1);
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 7b299b7..c23dd8a 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -210,6 +210,17 @@ void fork_end(int child);
  */
 bool guest_validate_base(unsigned long guest_base);
 
+/* Creates the initial guest address space in the host memory space using the
+ * given host start address hint and size.  If fixed is specified, then the
+ * mapped address space must start at host_start.  If host_start and host_size
+ * are both zero then nothing is done and zero is returned. Otherwise, the
+ * guest address space is initialized and the real start address of the mapped
+ * memory space is returned or -1 if there is was error.
+ */
+unsigned long init_guest_space(unsigned long host_start,
+                               unsigned long host_size,
+                               bool fixed);
+
 #include "qemu-log.h"
 
 /* strace.c */
-- 
1.7.7.6

[Qemu-devel] [PATCH 2/2] linux-user: Use init_guest_space when -R and -B are specified

[Meador Inge]

Modify the driver to initialize the guest address space when -R or -B
is specified so that the reserved memory space can be probed.  Calling
'mmap' just once as is currently done is not guaranteed to succeed since
the host address space validation might fail.

Signed-off-by: Meador Inge <meadori@codesourcery.com>
---
 linux-user/elfload.c |   57 +++++++++++++++++++++++++++++++++++++++++++-------
 linux-user/main.c    |   35 +++++-------------------------
 linux-user/qemu.h    |    6 -----
 3 files changed, 55 insertions(+), 43 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 44b4bdb..8de96bc 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -332,9 +332,17 @@ enum
     ARM_HWCAP_ARM_VFPv3D16  = 1 << 13,
 };
 
-#define TARGET_HAS_GUEST_VALIDATE_BASE
-/* We want the opportunity to check the suggested base */
-bool guest_validate_base(unsigned long guest_base)
+#define TARGET_HAS_VALIDATE_GUEST_SPACE
+/* Return 1 if the proposed guest space is suitable for the guest.
+ * Return 0 if the proposed guest space isn't suitable, but another
+ * address space should be tried.
+ * Return -1 if there is no way the proposed guest space can be
+ * valid regardless of the base.
+ * The guest code may leave a page mapped and populate it if the
+ * address is suitable.
+ */
+static int validate_guest_space(unsigned long guest_base,
+                                unsigned long guest_size)
 {
     unsigned long real_start, test_page_addr;
 
@@ -342,6 +350,15 @@ bool guest_validate_base(unsigned long guest_base)
      * commpage at 0xffff0fxx
      */
     test_page_addr = guest_base + (0xffff0f00 & qemu_host_page_mask);
+
+    /* If the commpage lies within the already allocated guest space,
+     * then there is no way we can allocate it.
+     */
+    if (test_page_addr >= guest_base
+	&& test_page_addr <= (guest_base + guest_size)) {
+	  return -1;
+    }
+
     /* Note it needs to be writeable to let us initialise it */
     real_start = (unsigned long)
                  mmap((void *)test_page_addr, qemu_host_page_size,
@@ -1377,9 +1394,10 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc,
     return sp;
 }
 
-#ifndef TARGET_HAS_GUEST_VALIDATE_BASE
+#ifndef TARGET_HAS_VALIDATE_GUEST_SPACE
 /* If the guest doesn't have a validation function just agree */
-bool guest_validate_base(unsigned long guest_base)
+static int validate_guest_space(unsigned long guest_base,
+                                unsigned long guest_size)
 {
     return 1;
 }
@@ -1400,7 +1418,7 @@ unsigned long init_guest_space(unsigned long host_start,
     /* If just a starting address is given, then just verify that
      * address.  */
     if (host_start && !host_size) {
-        if (guest_validate_base(host_start)) {
+        if (validate_guest_space(host_start, host_size) == 1) {
             return host_start;
         } else {
             return (unsigned long)-1;
@@ -1417,6 +1435,8 @@ unsigned long init_guest_space(unsigned long host_start,
     /* Otherwise, a non-zero size region of memory needs to be mapped
      * and validated.  */
     while (1) {
+        unsigned long real_size = host_size;
+
         /* Do not use mmap_find_vma here because that is limited to the
          * guest address space.  We are going to make the
          * guest address space fit whatever we're given.
@@ -1427,8 +1447,27 @@ unsigned long init_guest_space(unsigned long host_start,
             return (unsigned long)-1;
         }
 
-        if ((real_start == current_start) && guest_validate_base(real_start)) {
-            break;
+        /* Ensure the address is properly aligned.  */
+        if (real_start & ~qemu_host_page_mask) {
+            munmap((void *)real_start, host_size);
+            real_size = host_size + qemu_host_page_size;
+            real_start = (unsigned long)
+                mmap((void *)real_start, real_size, PROT_NONE, flags, -1, 0);
+            if (real_start == (unsigned long)-1) {
+                return (unsigned long)-1;
+            }
+            real_start = HOST_PAGE_ALIGN(real_start);
+        }
+
+        /* Check to see if the address is valid.  */
+        if (!host_start || real_start == current_start) {
+            int valid = validate_guest_space(real_start, real_size);
+            if (valid == 1) {
+                break;
+            } else if (valid == -1) {
+                return (unsigned long)-1;
+            }
+            /* valid == 0, so try again. */
         }
 
         /* That address didn't work.  Unmap and try a different one.
@@ -1450,6 +1489,8 @@ unsigned long init_guest_space(unsigned long host_start,
         }
     }
 
+    qemu_log("Reserved 0x%lx bytes of guest address space\n", host_size);
+
     return real_start;
 }
 
diff --git a/linux-user/main.c b/linux-user/main.c
index d0e0e4f..ef56c09 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3425,39 +3425,16 @@ int main(int argc, char **argv, char **envp)
      */
     guest_base = HOST_PAGE_ALIGN(guest_base);
 
-    if (reserved_va) {
-        void *p;
-        int flags;
-
-        flags = MAP_ANONYMOUS | MAP_PRIVATE | MAP_NORESERVE;
-        if (have_guest_base) {
-            flags |= MAP_FIXED;
-        }
-        p = mmap((void *)guest_base, reserved_va, PROT_NONE, flags, -1, 0);
-        if (p == MAP_FAILED) {
+    if (reserved_va || have_guest_base) {
+        guest_base = init_guest_space(guest_base, reserved_va,
+                                      have_guest_base);
+        if (guest_base == (unsigned long)-1) {
             fprintf(stderr, "Unable to reserve guest address space\n");
             exit(1);
         }
-        guest_base = (unsigned long)p;
-        /* Make sure the address is properly aligned.  */
-        if (guest_base & ~qemu_host_page_mask) {
-            munmap(p, reserved_va);
-            p = mmap((void *)guest_base, reserved_va + qemu_host_page_size,
-                     PROT_NONE, flags, -1, 0);
-            if (p == MAP_FAILED) {
-                fprintf(stderr, "Unable to reserve guest address space\n");
-                exit(1);
-            }
-            guest_base = HOST_PAGE_ALIGN((unsigned long)p);
-        }
-        qemu_log("Reserved 0x%lx bytes of guest address space\n", reserved_va);
-        mmap_next_start = reserved_va;
-    }
 
-    if (reserved_va || have_guest_base) {
-        if (!guest_validate_base(guest_base)) {
-            fprintf(stderr, "Guest base/Reserved VA rejected by guest code\n");
-            exit(1);
+        if (reserved_va) {
+            mmap_next_start = reserved_va;
         }
     }
 #endif /* CONFIG_USE_GUEST_BASE */
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index c23dd8a..bdc82ff 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -204,12 +204,6 @@ int get_osversion(void);
 void fork_start(void);
 void fork_end(int child);
 
-/* Return true if the proposed guest_base is suitable for the guest.
- * The guest code may leave a page mapped and populate it if the
- * address is suitable.
- */
-bool guest_validate_base(unsigned long guest_base);
-
 /* Creates the initial guest address space in the host memory space using the
  * given host start address hint and size.  If fixed is specified, then the
  * mapped address space must start at host_start.  If host_start and host_size
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH 1/2] linux-user: Factor out guest space probing into a function

[Peter Maydell]

On 10 July 2012 16:57, Meador Inge <meadori@codesourcery.com> wrote:
> Signed-off-by: Meador Inge <meadori@codesourcery.com>
> ---
>  linux-user/elfload.c |  111 +++++++++++++++++++++++++++++++++++---------------
>  linux-user/qemu.h    |   11 +++++
>  2 files changed, 89 insertions(+), 33 deletions(-)
>
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index f3b1552..44b4bdb 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -1385,6 +1385,74 @@ bool guest_validate_base(unsigned long guest_base)
>  }
>  #endif
>
> +unsigned long init_guest_space(unsigned long host_start,
> +                               unsigned long host_size,
> +                               bool fixed)
> +{
> +    unsigned long current_start, real_start;
> +    int flags;
> +
> +    /* Nothing to do here, move along.  */
> +    if (!host_start && !host_size) {
> +        return 0;
> +    }

This is a check that wasn't in the pre-refactoring code. Is it actually
a possible case, or should we be asserting() (perhaps checking for
bad ELF files and printing a suitable error message earlier)?

> +
> +    /* If just a starting address is given, then just verify that
> +     * address.  */
> +    if (host_start && !host_size) {
> +        if (guest_validate_base(host_start)) {
> +            return host_start;
> +        } else {
> +            return (unsigned long)-1;
> +        }
> +    }
> +
> +    /* Setup the initial flags and start address.  */
> +    current_start = host_start & qemu_host_page_mask;
> +    flags = MAP_ANONYMOUS | MAP_PRIVATE | MAP_NORESERVE;
> +    if (fixed) {
> +        flags |= MAP_FIXED;
> +    }
> +
> +    /* Otherwise, a non-zero size region of memory needs to be mapped
> +     * and validated.  */
> +    while (1) {
> +        /* Do not use mmap_find_vma here because that is limited to the
> +         * guest address space.  We are going to make the
> +         * guest address space fit whatever we're given.
> +         */
> +        real_start = (unsigned long)
> +            mmap((void *)current_start, host_size, PROT_NONE, flags, -1, 0);
> +        if (real_start == (unsigned long)-1) {
> +            return (unsigned long)-1;
> +        }
> +
> +        if ((real_start == current_start) && guest_validate_base(real_start)) {

This doesn't look like it's going to be calling guest_validate_base()
on the same value as the pre-refactoring code: before this commit
we called g_v_b() on real_start - loaddr.

> +            break;
> +        }
> +
> +        /* That address didn't work.  Unmap and try a different one.
> +         * The address the host picked because is typically right at
> +         * the top of the host address space and leaves the guest with
> +         * no usable address space.  Resort to a linear search.  We
> +         * already compensated for mmap_min_addr, so this should not
> +         * happen often.  Probably means we got unlucky and host
> +         * address space randomization put a shared library somewhere
> +         * inconvenient.
> +         */
> +        munmap((void *)real_start, host_size);
> +        current_start += qemu_host_page_size;
> +        if (host_start == current_start) {
> +            /* Theoretically possible if host doesn't have any suitably
> +             * aligned areas.  Normally the first mmap will fail.
> +             */
> +            return (unsigned long)-1;
> +        }
> +    }
> +
> +    return real_start;
> +}
> +
>  static void probe_guest_base(const char *image_name,
>                               abi_ulong loaddr, abi_ulong hiaddr)
>  {
> @@ -1411,46 +1479,23 @@ static void probe_guest_base(const char *image_name,
>              }
>          }
>          host_size = hiaddr - loaddr;
> -        while (1) {
> -            /* Do not use mmap_find_vma here because that is limited to the
> -               guest address space.  We are going to make the
> -               guest address space fit whatever we're given.  */
> -            real_start = (unsigned long)
> -                mmap((void *)host_start, host_size, PROT_NONE,
> -                     MAP_ANONYMOUS | MAP_PRIVATE | MAP_NORESERVE, -1, 0);
> -            if (real_start == (unsigned long)-1) {
> -                goto exit_perror;
> -            }
> -            guest_base = real_start - loaddr;
> -            if ((real_start == host_start) &&
> -                guest_validate_base(guest_base)) {
> -                break;
> -            }
> -            /* That address didn't work.  Unmap and try a different one.
> -               The address the host picked because is typically right at
> -               the top of the host address space and leaves the guest with
> -               no usable address space.  Resort to a linear search.  We
> -               already compensated for mmap_min_addr, so this should not
> -               happen often.  Probably means we got unlucky and host
> -               address space randomization put a shared library somewhere
> -               inconvenient.  */
> -            munmap((void *)real_start, host_size);
> -            host_start += qemu_host_page_size;
> -            if (host_start == loaddr) {
> -                /* Theoretically possible if host doesn't have any suitably
> -                   aligned areas.  Normally the first mmap will fail.  */
> -                errmsg = "Unable to find space for application";
> -                goto exit_errmsg;
> -            }
> +
> +        /* Setup the initial guest memory space with ranges gleaned from
> +         * the ELF image that is being loaded.
> +         */
> +        real_start = init_guest_space(host_start, host_size, 0);

If we're declaring the 'fixed' argument as 'bool' we should be passing
'false' rather than '0' here.

> +        if (real_start == (unsigned long)-1) {
> +            errmsg = "Unable to find space for application";
> +            goto exit_errmsg;
>          }
> +        guest_base = real_start - loaddr;
> +
>          qemu_log("Relocating guest address space from 0x"
>                   TARGET_ABI_FMT_lx " to 0x%lx\n",
>                   loaddr, real_start);
>      }
>      return;
>
> -exit_perror:
> -    errmsg = strerror(errno);
>  exit_errmsg:
>      fprintf(stderr, "%s: %s\n", image_name, errmsg);
>      exit(-1);
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index 7b299b7..c23dd8a 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -210,6 +210,17 @@ void fork_end(int child);
>   */
>  bool guest_validate_base(unsigned long guest_base);
>
> +/* Creates the initial guest address space in the host memory space using the
> + * given host start address hint and size.  If fixed is specified, then the
> + * mapped address space must start at host_start.  If host_start and host_size
> + * are both zero then nothing is done and zero is returned. Otherwise, the
> + * guest address space is initialized and the real start address of the mapped
> + * memory space is returned or -1 if there is was error.

"was an error".

> + */
> +unsigned long init_guest_space(unsigned long host_start,
> +                               unsigned long host_size,
> +                               bool fixed);
> +
>  #include "qemu-log.h"
>
>  /* strace.c */
> --
> 1.7.7.6
>

Re: [Qemu-devel] [PATCH 1/2] linux-user: Factor out guest space probing into a function

[Meador Inge]

On 07/10/2012 11:12 AM, Peter Maydell wrote:

> On 10 July 2012 16:57, Meador Inge <meadori@codesourcery.com> wrote:
>> Signed-off-by: Meador Inge <meadori@codesourcery.com>
>> ---
>>  linux-user/elfload.c |  111 +++++++++++++++++++++++++++++++++++---------------
>>  linux-user/qemu.h    |   11 +++++
>>  2 files changed, 89 insertions(+), 33 deletions(-)
>>
>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>> index f3b1552..44b4bdb 100644
>> --- a/linux-user/elfload.c
>> +++ b/linux-user/elfload.c
>> @@ -1385,6 +1385,74 @@ bool guest_validate_base(unsigned long guest_base)
>>  }
>>  #endif
>>
>> +unsigned long init_guest_space(unsigned long host_start,
>> +                               unsigned long host_size,
>> +                               bool fixed)
>> +{
>> +    unsigned long current_start, real_start;
>> +    int flags;
>> +
>> +    /* Nothing to do here, move along.  */
>> +    if (!host_start && !host_size) {
>> +        return 0;
>> +    }
> 
> This is a check that wasn't in the pre-refactoring code. Is it actually
> a possible case, or should we be asserting() (perhaps checking for
> bad ELF files and printing a suitable error message earlier)?

Yeah, that shouldn't happen.  An assert should be sufficient.

>> +
>> +    /* If just a starting address is given, then just verify that
>> +     * address.  */
>> +    if (host_start && !host_size) {
>> +        if (guest_validate_base(host_start)) {
>> +            return host_start;
>> +        } else {
>> +            return (unsigned long)-1;
>> +        }
>> +    }
>> +
>> +    /* Setup the initial flags and start address.  */
>> +    current_start = host_start & qemu_host_page_mask;
>> +    flags = MAP_ANONYMOUS | MAP_PRIVATE | MAP_NORESERVE;
>> +    if (fixed) {
>> +        flags |= MAP_FIXED;
>> +    }
>> +
>> +    /* Otherwise, a non-zero size region of memory needs to be mapped
>> +     * and validated.  */
>> +    while (1) {
>> +        /* Do not use mmap_find_vma here because that is limited to the
>> +         * guest address space.  We are going to make the
>> +         * guest address space fit whatever we're given.
>> +         */
>> +        real_start = (unsigned long)
>> +            mmap((void *)current_start, host_size, PROT_NONE, flags, -1, 0);
>> +        if (real_start == (unsigned long)-1) {
>> +            return (unsigned long)-1;
>> +        }
>> +
>> +        if ((real_start == current_start) && guest_validate_base(real_start)) {
> 
> This doesn't look like it's going to be calling guest_validate_base()
> on the same value as the pre-refactoring code: before this commit
> we called g_v_b() on real_start - loaddr.

Gah, fixed.  Thanks for finding that.

>> +            break;
>> +        }
>> +
>> +        /* That address didn't work.  Unmap and try a different one.
>> +         * The address the host picked because is typically right at
>> +         * the top of the host address space and leaves the guest with
>> +         * no usable address space.  Resort to a linear search.  We
>> +         * already compensated for mmap_min_addr, so this should not
>> +         * happen often.  Probably means we got unlucky and host
>> +         * address space randomization put a shared library somewhere
>> +         * inconvenient.
>> +         */
>> +        munmap((void *)real_start, host_size);
>> +        current_start += qemu_host_page_size;
>> +        if (host_start == current_start) {
>> +            /* Theoretically possible if host doesn't have any suitably
>> +             * aligned areas.  Normally the first mmap will fail.
>> +             */
>> +            return (unsigned long)-1;
>> +        }
>> +    }
>> +
>> +    return real_start;
>> +}
>> +
>>  static void probe_guest_base(const char *image_name,
>>                               abi_ulong loaddr, abi_ulong hiaddr)
>>  {
>> @@ -1411,46 +1479,23 @@ static void probe_guest_base(const char *image_name,
>>              }
>>          }
>>          host_size = hiaddr - loaddr;
>> -        while (1) {
>> -            /* Do not use mmap_find_vma here because that is limited to the
>> -               guest address space.  We are going to make the
>> -               guest address space fit whatever we're given.  */
>> -            real_start = (unsigned long)
>> -                mmap((void *)host_start, host_size, PROT_NONE,
>> -                     MAP_ANONYMOUS | MAP_PRIVATE | MAP_NORESERVE, -1, 0);
>> -            if (real_start == (unsigned long)-1) {
>> -                goto exit_perror;
>> -            }
>> -            guest_base = real_start - loaddr;
>> -            if ((real_start == host_start) &&
>> -                guest_validate_base(guest_base)) {
>> -                break;
>> -            }
>> -            /* That address didn't work.  Unmap and try a different one.
>> -               The address the host picked because is typically right at
>> -               the top of the host address space and leaves the guest with
>> -               no usable address space.  Resort to a linear search.  We
>> -               already compensated for mmap_min_addr, so this should not
>> -               happen often.  Probably means we got unlucky and host
>> -               address space randomization put a shared library somewhere
>> -               inconvenient.  */
>> -            munmap((void *)real_start, host_size);
>> -            host_start += qemu_host_page_size;
>> -            if (host_start == loaddr) {
>> -                /* Theoretically possible if host doesn't have any suitably
>> -                   aligned areas.  Normally the first mmap will fail.  */
>> -                errmsg = "Unable to find space for application";
>> -                goto exit_errmsg;
>> -            }
>> +
>> +        /* Setup the initial guest memory space with ranges gleaned from
>> +         * the ELF image that is being loaded.
>> +         */
>> +        real_start = init_guest_space(host_start, host_size, 0);
> 
> If we're declaring the 'fixed' argument as 'bool' we should be passing
> 'false' rather than '0' here.

Fixed.

>> +        if (real_start == (unsigned long)-1) {
>> +            errmsg = "Unable to find space for application";
>> +            goto exit_errmsg;
>>          }
>> +        guest_base = real_start - loaddr;
>> +
>>          qemu_log("Relocating guest address space from 0x"
>>                   TARGET_ABI_FMT_lx " to 0x%lx\n",
>>                   loaddr, real_start);
>>      }
>>      return;
>>
>> -exit_perror:
>> -    errmsg = strerror(errno);
>>  exit_errmsg:
>>      fprintf(stderr, "%s: %s\n", image_name, errmsg);
>>      exit(-1);
>> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
>> index 7b299b7..c23dd8a 100644
>> --- a/linux-user/qemu.h
>> +++ b/linux-user/qemu.h
>> @@ -210,6 +210,17 @@ void fork_end(int child);
>>   */
>>  bool guest_validate_base(unsigned long guest_base);
>>
>> +/* Creates the initial guest address space in the host memory space using the
>> + * given host start address hint and size.  If fixed is specified, then the
>> + * mapped address space must start at host_start.  If host_start and host_size
>> + * are both zero then nothing is done and zero is returned. Otherwise, the
>> + * guest address space is initialized and the real start address of the mapped
>> + * memory space is returned or -1 if there is was error.
> 
> "was an error".

Fixed.

>> + */
>> +unsigned long init_guest_space(unsigned long host_start,
>> +                               unsigned long host_size,
>> +                               bool fixed);
>> +
>>  #include "qemu-log.h"
>>
>>  /* strace.c */
>> --
>> 1.7.7.6
>>

v2 patch coming soon.  Thanks for the review.

-- 
Meador Inge
CodeSourcery / Mentor Embedded
http://www.mentor.com/embedded-software