[Top][All Lists]
[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
>