| Message ID | 1341935833-2655-2-git-send-email-meadori@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
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 >
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.
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 */
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(-)