[U-Boot] [PATCH v3 07/21] efi: sandbox: Adjust memory usage for sandbox

[Simon Glass]

Hi Heinrich,

On 20 February 2018 at 10:12, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> On 02/19/2018 04:48 PM, 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 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 c273a7ba30..b113e08783 100644
>> --- a/lib/efi_loader/efi_memory.c
>> +++ b/lib/efi_loader/efi_memory.c
>> @@ -10,6 +10,7 @@
>>   #include <efi_loader.h>
>>   #include <inttypes.h>
>>   #include <malloc.h>
>> +#include <mapmem.h>
>>   #include <watchdog.h>
>>   #include <asm/global_data.h>
>>   #include <linux/list_sort.h>
>> @@ -389,7 +390,7 @@ efi_status_t efi_allocate_pool(int pool_type,
>> efi_uintn_t size, void **buffer)
>>         r = efi_allocate_pages(0, pool_type, num_pages, &t);
>>         if (r == EFI_SUCCESS) {
>> -               struct efi_pool_allocation *alloc = (void *)(uintptr_t)t;
>> +               struct efi_pool_allocation *alloc = map_sysmem(t, size);
>
>
> The AllocatePages Service has to return 4096 byte aligned memory.
> AllocatePool has to return 8 byte aligned memory.
>
> I cannot see that this page makes these guarantees.

Well I am not changing that code. All I am doing is correctly mapping
the long address to a sandbox pointer.

>
>>                 alloc->num_pages = num_pages;
>>                 *buffer = alloc->data;
>>         }
>> @@ -500,18 +501,22 @@ int efi_memory_init(void)
>>         efi_add_known_memory();
>
>
> You fail guarantee that the known memory is 4096 byte aligned.

Presumable the existing code does that, and I am not changing it.

>
>>   -     /* Add U-Boot */
>> -       uboot_start = (gd->start_addr_sp - uboot_stack_size) &
>> ~EFI_PAGE_MASK;
>> -       uboot_pages = (gd->ram_top - uboot_start) >> EFI_PAGE_SHIFT;
>> -       efi_add_memory_map(uboot_start, uboot_pages, EFI_LOADER_DATA,
>> false);
>> -
>> -       /* Add Runtime Services */
>> -       runtime_start = (ulong)&__efi_runtime_start & ~EFI_PAGE_MASK;
>> -       runtime_end = (ulong)&__efi_runtime_stop;
>> -       runtime_end = (runtime_end + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
>> -       runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
>> -       efi_add_memory_map(runtime_start, runtime_pages,
>> -                          EFI_RUNTIME_SERVICES_CODE, false);
>> +       if (!IS_ENABLED(CONFIG_SANDBOX)) {
>
>
> You are passing all memory aquired via mmap() in the above
> efi_add_known_memory() table.
>
> So here you have to mark any memory range as occupied that has already been
> given away or may be allocated by non-EFI functions from the mmap() assigned
> memory.

This is not mmap(), just a map from U-Boot ulong addresses to sandbox
pointers. So there should be no additional processing needed.

Regards,
Simon