[U-Boot] [PATCH v2 07/11] sandbox: Map host memory for efi_loader

[Simon Glass]

Hi Heinrich,

On 14 June 2018 at 13:21, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> On 06/14/2018 09:02 PM, Simon Glass wrote:
>> Hi Alex,
>>
>> On 14 June 2018 at 12:22, Alexander Graf <agraf at suse.de> wrote:
>>> With efi_loader we do not control payload applications, so we can not
>>> teach them about the difference between virtual and physical addresses.
>>>
>>> Instead, let's just always map host virtual addresses in the efi memory
>>> map. That way we can be sure that all memory allocation functions always
>>> return consumable pointers.
>>>
>>> Signed-off-by: Alexander Graf <agraf at suse.de>
>>>
>>> ---
>>>
>>> v1 -> v2:
>>>
>>>   - only compile efi_add_known_memory if efi_loader is enabled
>>> ---
>>>  arch/sandbox/cpu/cpu.c | 20 ++++++++++++++++++++
>>>  1 file changed, 20 insertions(+)
>>
>> NAK.
>>
>> You should not point sandbox pointers into the EFI tables. I know it
>> looks like a clever shortcut, but it is not correct. It will mess up
>> logging and debugging, since those pointers bear no easily accessible
>> relationship to U-Boot address.
>
> Hello Simon,
>
> why do we need this Babylonic confusion of addresses which do not point
> to valid memory and pointers that are not valid addresses where
> everybody is getting lost?
>
> Simply use only addresses with there mmap'ed values and get rid of all
> conversions. Use your board file to adjust kernel_addr_r and the like to
> the address range that Linux offered you.

No one is getting lost. It has been working fine for a long time.
Sandbox was introduced almost 7 years ago. It has been a huge benefit
in terms of productivity and testing.

We are not necessarily running on Linux, but in any case, we get ugly
addresses which expose the underlying machine architecture, which is
not relevant for tests, and just introduces confusion. Sandbox is
intended to be pure U-Boot, just like you might boot up on a 32-bit
ARM or x86 machine.

The conversions existing in any case, since we must case between
addresses and pointers. This just makes it explicit in a convenient,
type-safe manner.

Regards,
Simon