Re: [Qemu-devel] [PATCH 10/10] linux-user: init_guest_space: Try to make ARM space+commpage continuous

[Laurent Vivier <laurent@vivier.eu>]

Le 02/03/2018 à 15:13, Peter Maydell a écrit :
> On 28 December 2017 at 18:08, Luke Shumaker <lukeshu@lukeshu.com> wrote:
>> From: Luke Shumaker <lukeshu@parabola.nu>
>>
>> At a fixed distance after the usable memory that init_guest_space maps, for
>> 32-bit ARM targets we also need to map a commpage.  The normal
>> init_guest_space logic doesn't keep this in mind when searching for an
>> address range.
>>
>> If !host_start, then try to find a big continuous segment where we can put
>> both the usable memory and the commpage; we then munmap that segment and
>> set current_start to that address; and let the normal code mmap the usable
>> memory and the commpage separately.  That is: if we don't have hint of
>> where to start looking for memory, come up with one that is better than
>> NULL.  Depending on host_size and guest_start, there may or may not be a
>> gap between the usable memory and the commpage, so this is slightly more
>> restrictive than it needs to be; but it's only a hint, so that's OK.
>>
>> We only do that for !host start, because if host_start, then either:
>>  - we got an address passed in with -B, in which case we don't want to
>>    interfere with what the user said;
>>  - or host_start is based off of the ELF image's loaddr.  The check "if
>>    (host_start && real_start != current_start)" suggests that we really
>>    want lowest available address that is >= loaddr.  I don't know why that
>>    is, but I'm trusting that Paul Brook knew what he was doing when he
>>    wrote the original version of that check in
>>    c581deda322080e8beb88b2e468d4af54454e4b3 way back in 2010.
>>
>> Signed-off-by: Luke Shumaker <lukeshu@parabola.nu>
>> ---
>>  linux-user/elfload.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 49 insertions(+)
>>
>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>> index 7736ea2c3a..cd3a7d877d 100644
>> --- a/linux-user/elfload.c
>> +++ b/linux-user/elfload.c
>> @@ -1857,6 +1857,55 @@ unsigned long init_guest_space(unsigned long host_start,
>>
>>      /* Otherwise, a non-zero size region of memory needs to be mapped
>>       * and validated.  */
>> +
>> +#if defined(TARGET_ARM) && !defined(TARGET_AARCH64)
>> +    /* On 32-bit ARM, we need to map not just the usable memory, but
>> +     * also the commpage.  Try to find a suitable place by allocating
>> +     * a big chunk for all of it.  If host_start, then the naive
>> +     * strategy probably does good enough.
>> +     */
>> +    if (!host_start) {
>> +        unsigned long guest_full_size, host_full_size, real_start;
>> +
>> +        guest_full_size =
>> +            (0xffff0f00 & qemu_host_page_mask) + qemu_host_page_size;
> 
> I think this is probably more clearly written as 0x100000000ULL,
> since rounding down to the host-page-size then adding the host-page-size
> gets us the full 32-bit size of the guest address space.

Perhaps, I've missed something, but it seems not true.

On x86_64, we have:

qemu_host_page_mask = 0xfffffffffffff000
qemu_host_page_size = 0x0000000000001000

but

0xffff0f00 & 0xfffffffffffff000 = 0xffff0000
then
0xffff0000 + 0x0000000000001000 = 0xffff1000

Thanks,
Laurent