[U-Boot] [PATCH v8 02/30] efi: sandbox: Adjust memory usage for sandbox

[Simon Glass]

Hi Heinrich,

On 20 June 2018 at 00:10, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> On 06/18/2018 04:08 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.
>
> This is not reflected in the patch.
>
>>
>> Signed-off-by: Simon Glass <sjg at chromium.org>
>> ---
>>
>> Changes in v8: None
>> Changes in v7:
>> - Move some of the code from efi_memory_init() into a separate function
>>
>> 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 | 16 ++++++++++++----
>>  1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>> index ec66af98ea..c6410613c7 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>
>
> I cannot see any use of this include in the patch.

Please see ^^^^^^^ below, for example.

>
>>  #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);

^^^^^^^^^^^^

>>               alloc->num_pages = num_pages;
>>               *buffer = alloc->data;
>>       }
>> @@ -496,14 +497,13 @@ __weak void efi_add_known_memory(void)
>>       }
>>  }
>>
>> -int efi_memory_init(void)
>> +/* Add memory regions for U-Boot's memory and for the runtime services code */
>> +static void add_u_boot_and_runtime(void)
>>  {
>>       unsigned long runtime_start, runtime_end, runtime_pages;
>>       unsigned long uboot_start, uboot_pages;
>>       unsigned long uboot_stack_size = 16 * 1024 * 1024;
>>
>> -     efi_add_known_memory();
>> -
>>       /* 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;
>> @@ -516,6 +516,14 @@ int efi_memory_init(void)
>>       runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
>>       efi_add_memory_map(runtime_start, runtime_pages,
>>                          EFI_RUNTIME_SERVICES_CODE, false);
>> +}
>> +
>> +int efi_memory_init(void)
>> +{
>> +     efi_add_known_memory();
>> +
>> +     if (!IS_ENABLED(CONFIG_SANDBOX))
>> +             add_u_boot_and_runtime();
>
> Is the sandbox not using relocation? A comment is missing in the code
> here to explain why add_u_boot_and_runtime() should not be called for
> the sandbox.

That's right, sandbox does not use relocation. But the real point is
that its code is not within the sandbox memory map. Sandbox uses a
emulated RAM buffer, separate from the U-Boot code and stack.

Regards,
Simon