On 07/10/2012 11:12 AM, Peter Maydell wrote:
> On 10 July 2012 16:57, Meador Inge <mead...@codesourcery.com> wrote:
>> Signed-off-by: Meador Inge <mead...@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
