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

[Alexander Graf]

On 06/21/2018 09:45 PM, Simon Glass wrote:
> 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.

Only if you expose host pointers as memory addresses. We don't anymore, 
and grub doesn't care whether a pointer is >4G.

>
>>> 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?

I see little reason for change. Also if you want to change it, it's out 
of scope of the efi_loader enablement ;).

>
>>> 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.

No, I'm mostly worried about people assuming that 2 invocations of a 
U-Boot binary give them the same console output, even when there are 
printf()s that contain a %p.


Alex