qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH 1/2] linux-user: Factor out guest space probing into a function
Date: Tue, 10 Jul 2012 17:12:57 +0100

On 10 July 2012 16:57, Meador Inge <address@hidden> wrote:
> Signed-off-by: Meador Inge <address@hidden>
> ---
>  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
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]