[U-Boot] [PATCH v4 04/16] sandbox: smbios: Update to support sandbox

[Simon Glass]

Hi Alex,

On 7 June 2018 at 12:47, Alexander Graf <agraf at suse.de> wrote:
>
>
> On 07.06.18 22:41, Simon Glass wrote:
>> Hi Alex,
>>
>> On 7 June 2018 at 12:36, Alexander Graf <agraf at suse.de> wrote:
>>>
>>>
>>> On 07.06.18 22:25, Simon Glass wrote:
>>>> Hi Alex,
>>>>
>>>> On 3 June 2018 at 04:13, Alexander Graf <agraf at suse.de> wrote:
>>>>>
>>>>>
>>>>> On 25.05.18 04:42, Simon Glass wrote:
>>>>>> Hi Alex,
>>>>>>
>>>>>> On 24 May 2018 at 06:24, Alexander Graf <agraf at suse.de> wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 16.05.18 17:42, Simon Glass wrote:
>>>>>>>> At present this code casts addresses to pointers so cannot be used with
>>>>>>>> sandbox. Update it to use mapmem instead.
>>>>>>>>
>>>>>>>> Signed-off-by: Simon Glass <sjg at chromium.org>
>>>>>>>
>>>>>>> I really dislike the whole fact that you have to call map_sysmem() at
>>>>>>> all. I don't quite understand the whole point of it either - it just
>>>>>>> seems to clutter the code and make it harder to follow.
>>>>>>
>>>>>> The purpose is to map U-Boot addresses (e.g. 0x1234) to actual
>>>>>> user-space addresses in sandbox (gd->arch.ram_buf + 0x1234).
>>>>>>
>>>>>> Otherwise we cannot write tests which use particular addresses, and
>>>>>> people have to worry about the host memory layout when using sandbox.
>>>>>
>>>>> Not if we write a smart enough linker script. I can try to see when I
>>>>> get around to give you an example. But basically all we need to do is
>>>>> reserve a section for guest ram at a constant virtual address.
>>>>
>>>> Yes, but ideally that would be 0, or something small.
>>>
>>> You can't do 0 because 0 is protected on a good number of OSs. And if it
>>> can't be 0, better use something that makes pointers easy to read.
>>
>> Yes this is one reason for map_sysmem().
>>
>>>
>>>>
>>>>>
>>>>>>> Can't we just simply make sandbox behave like any other target instead?
>>>>>>
>>>>>> Actually that's the goal of the sandbox support. Memory is modelled as
>>>>>> a contiguous chunk starting at 0x0, regardless of what the OS actually
>>>>>> gives U-Boot in terms of addresses.
>>>>>
>>>>> Most platforms don't have RAM start at 0x0 (and making sure nobody
>>>>> assumes it does start at 0 is a good thing). The only bit we need to
>>>>> make sure is that it always starts at *the same* address on every
>>>>> invocation. But if that address is 256MB, things should still be fine.
>>>>
>>>> Yes but putting a 10000000 base address on everything is a bit of a pain.
>>>
>>> Why? It's what we do on arm systems that have ram starting at higher
>>> offsets already.
>>
>> It's a pain because you have to type 1 and 5-6 zeroes before you can
>> get to the address you want. Otherwise sandbox just segfaults, which
>> is annoying.
>
> It's the same as any other device that does not have RAM starting at 0.
> The benefit of it is that you manage to catch NULL pointer accesses
> quite easily, which I guess is something you'll want from a testing target.

You're confusing the U-Boot memory address with the pointer address.
If you use NULL in sandbox it will fault. But address 0 is valid.

>
> Also, you shouldn't use hard addresses in the first place. That's why we
> have $kernel_addr_r and friends. As long as you stick to those, nothing
> should change for you at all.

See for example test_fit.py where it is very useful to know the
addresses we are using.

Of course we can remove this constraint, but it does make things more
painful in sandbox. Also IMO using open casts to convert between
addresses and pointers is not desirable, as they are not really tagged
in any way. With map_sysmem(), etc., they are explicit and obvious.

Regards,
Simon