Re: [Qemu-devel] [PATCH 2/3] linux-user: Tidy and enforce reserved_va initialization

[Richard Henderson <rth@twiddle.net>]

On 10/03/2017 12:24 PM, Peter Maydell wrote:
> On 8 July 2017 at 03:50, Richard Henderson <rth@twiddle.net> wrote:
>> We had a check using TARGET_VIRT_ADDR_SPACE_BITS to make sure
>> that the allocation coming in from the command-line option was
>> not too large, but that didn't include target-specific knowledge
>> about other restrictions on user-space.
>>
>> Remove several target-specific hacks in linux-user/main.c.
>>
>> For MIPS and Nios, we can replace them with proper adjustments
>> to the respective target's TARGET_VIRT_ADDR_SPACE_BITS definition.
>>
>> For ARM, we had no existing ifdef but I suspect that the current
>> default value of 0xf7000000 was chosen with this in mind.  Define
>> a workable value in linux-user/arm/, and also document why the
>> special case is required.
>>
>> Signed-off-by: Richard Henderson <rth@twiddle.net>
>> ---
>>  linux-user/arm/target_cpu.h |  4 ++++
>>  target/mips/mips-defs.h     |  6 +++++-
>>  target/nios2/cpu.h          |  6 +++++-
>>  linux-user/main.c           | 38 +++++++++++++++++++++++++-------------
>>  4 files changed, 39 insertions(+), 15 deletions(-)
>>
>> diff --git a/linux-user/arm/target_cpu.h b/linux-user/arm/target_cpu.h
>> index d888219..c4f79eb 100644
>> --- a/linux-user/arm/target_cpu.h
>> +++ b/linux-user/arm/target_cpu.h
>> @@ -19,6 +19,10 @@
>>  #ifndef ARM_TARGET_CPU_H
>>  #define ARM_TARGET_CPU_H
>>
>> +/* We need to be able to map the commpage.
>> +   See validate_guest_space in linux-user/elfload.c.  */
>> +#define MAX_RESERVED_VA  0xfff00000ul
>> +
> 
> This should be 0xffff0000, but you'll need the bugfix patch I just sent
> out first.
> 
> (Why "UL" ? That's usually a wrong choice compared to either U or ULL.)

Because that matches the type of

+unsigned long reserved_va = MAX_RESERVED_VA;

Which, arguably, should be uintptr_t or size_t something instead, but that
would certainly be for a different patch.

If you prefer, since this is a 32-bit value, I could trim the define to U and
still be correct.


r~