[U-Boot] [PATCH v8 12/30] sandbox: Try to start the RAM buffer at a particular address

[Simon Glass]

Hi Alex,

On 21 June 2018 at 03:58, Alexander Graf <agraf at suse.de> wrote:
> On 06/21/2018 04:02 AM, Simon Glass wrote:
>>
>> Hi Alex,
>>
>> On 20 June 2018 at 02:51, Alexander Graf <agraf at suse.de> wrote:
>>>
>>> On 06/20/2018 12:02 AM, Simon Glass wrote:
>>>>
>>>> Hi Alex,
>>>>
>>>> On 18 June 2018 at 08:45, Alexander Graf <agraf at suse.de> wrote:
>>>>>
>>>>> On 06/18/2018 04:08 PM, Simon Glass wrote:
>>>>>>
>>>>>> Use a starting address of 256MB which should be available. This helps
>>>>>> to
>>>>>> make sandbox RAM buffers pointers more recognisable.
>>>>>>
>>>>>> Signed-off-by: Simon Glass <sjg at chromium.org>
>>>>>
>>>>>
>>>>> Nak, this has a non-0 chance of failing, in case something else is
>>>>> already
>>>>> mapped at that address. You don't want to have your CI blow up 1% of
>>>>> the
>>>>> time due to this.
>>>>
>>>> It's just a hint though. Everything will still work if it doesn't get
>>>> this exact address.
>>>
>>>
>>> I don't see what it buys us then.
>>
>> These are my thoughts:
>>
>> 1. We get an address before 4GB which is needed for grub (so far as I can
>> tell)
>
>
> We only need that in the memory map which you want virtual (U-Boot address
> space) anyway. So there's no need to also have the Linux address be <4GB.

Grub cannot work without <4GB, right? I don't mind either way, but I
think it that if we are picking an address, picking a smaller one is
better.

>
>> 2. It will normally be a nice, easy-on-the-eyes address
>
>
> I'd rather say it's going to confuse people, because now it's easy to
> mistake for a U-Boot address.

That's possibly true, although at present it is outside the range of
valid U-Boot addresses. What do you suggest?

>
>> 3. If it isn't, it will hopefully still be an address below 4GB
>> 4. Even if not (e.g. on some very strange system) it will still allow
>> sandbox to start up
>>
>> But anyway, as I said, this will never fail, it is just a hint. So I
>> think your original comment is incorrect.
>
>
> It will not fail, it will just potentially not map at exactly the spot you
> wanted the map at, which IMHO would cause even more confusion.
>
> Imagine people now start to assume that %p output is stable between
> invocations - because it is 99% of the cases. They start to integrate that
> into their CI suites and suddenly things fail 1% of the time because the
> linker figured today's a great day to map glibc at that address.

Remember these are internal sandbox addresses. They should not be
visible to most of the code and it is very hard for me to imagine a
test that could fail because the address is mapped in different
places. How would you access the address from a test? Are you thinking
of something that would assume the ram buffer address is something in
particular and then call map_to_sysmem() on the pointer? I am really
not sure that we should worry about that too much.

Otherwise I just cannot imagine what sort of test you are referring to here?

Regards,
Simon