[U-Boot] [PATCH v6 03/13] efi: sandbox: Adjust memory usage for sandbox

[Alexander Graf]

On 06/14/2018 02:58 PM, Simon Glass wrote:
> Hi Alex,
>
> On 14 June 2018 at 04:12, Alexander Graf <agraf at suse.de> wrote:
>> On 06/13/2018 04:37 AM, Simon Glass wrote:
>>> With sandbox the U-Boot code is not mapped into the sandbox memory range
>>> so does not need to be excluded when allocating EFI memory. Update the EFI
>>> memory init code to take account of that.
>>>
>>> Also use mapmem instead of a cast to convert a memory address to a
>>> pointer.
>>>
>>> Signed-off-by: Simon Glass <sjg at chromium.org>
>>> ---
>>>
>>> Changes in v6: None
>>> Changes in v5: None
>>> Changes in v4: None
>>> Changes in v3: None
>>> Changes in v2:
>>> - Update to use mapmem instead of a cast
>>>
>>>    lib/efi_loader/efi_memory.c | 31 ++++++++++++++++++-------------
>>>    1 file changed, 18 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>>> index ec66af98ea..210e49ee8b 100644
>>> --- a/lib/efi_loader/efi_memory.c
>>> +++ b/lib/efi_loader/efi_memory.c
>>> @@ -9,6 +9,7 @@
>>>    #include <efi_loader.h>
>>>    #include <inttypes.h>
>>>    #include <malloc.h>
>>> +#include <mapmem.h>
>>>    #include <watchdog.h>
>>>    #include <linux/list_sort.h>
>>>    @@ -393,7 +394,7 @@ efi_status_t efi_allocate_pool(int pool_type,
>>> efi_uintn_t size, void **buffer)
>>>                                 &t);
>>>          if (r == EFI_SUCCESS) {
>>> -               struct efi_pool_allocation *alloc = (void *)(uintptr_t)t;
>>> +               struct efi_pool_allocation *alloc = map_sysmem(t, size);
>>
>> This is still the wrong spot. We don't want the conversion to happen when
>> going from an EFI internal address to an allocation, but when determining
>> which addresses are usable in the first place.
> There seem to be two ways to do this:
>
> 1. Record addresses (ulong) in the EFI tables and use map_sysmem()
> before returning an address in the allocator
> 2. Store pointers in the EFI tables using map_sysmem(), then do no
> mapping in the allocator
>
> I've gone with option 1 since:
>
> - the EFI addresses are not pointers
> - it makes sandbox consistent with other architectures which use an
> address rather than a pointer in EFI tables
> - we don't normally do pointer arithmetic on the results of map_sysmem()
> - we normally map the memory when it is used rather than when it is set up
>
> I think you are asking for option 2. I think that would get very
> confusing. The addresses where things actually end up in sandbox are
> best kept to sandbox.
>
> Overall I feel that you are either missing the point of sandbox
> addressing, or don't agree with how it is done. But it does work
> pretty well and we don't get a lot of confusion with sandbox pointers
> since we typically use the address until the last moment.

I've assembled a quick tree for version 2. With that I'm able to run a 
simple hello world efi application. Grub refuses to start because it 
wants memory in the low 32bit and also emits random PIO accessing 
functions, which obviously don't work work from user space.

But overall, I think this is the right path to tackle this:

   https://github.com/agraf/u-boot/tree/efi-sandbox


Alex