[U-Boot] [PATCH 00/16] efi: Enable basic sandbox support for EFI loader
Simon Glass
sjg at chromium.org
Mon Sep 25 02:15:57 UTC 2017
Hi Rob,
On 18 September 2017 at 09:07, Rob Clark <robdclark at gmail.com> wrote:
> On Mon, Sep 18, 2017 at 10:30 AM, Rob Clark <robdclark at gmail.com> wrote:
>> On Mon, Sep 18, 2017 at 9:31 AM, Rob Clark <robdclark at gmail.com> wrote:
>>> On Mon, Sep 18, 2017 at 9:18 AM, Rob Clark <robdclark at gmail.com> wrote:
>>>> On Sun, Sep 17, 2017 at 11:48 PM, Heinrich Schuchardt
>>>> <xypron.glpk at gmx.de> wrote:
>>>>> On 09/18/2017 12:59 AM, Simon Glass wrote:
>>>>>> A limitation of the EFI loader at present is that it does not build with
>>>>>> sandbox. This makes it hard to write tests, since sandbox is used for most
>>>>>> testing in U-Boot.
>>>>>>
>>>>>> This series enables the EFI loader feature. It allows sandbox to build and
>>>>>> run a trivial function which calls the EFI API to output a message.
>>>>>>
>>>>>> Much work remains but this should serve as a basis for adding tests more
>>>>>> easily for EFI loader.
>>>>>>
>>>>>> This series sits on top of Heinrich's recent EFI test series. It is
>>>>>> available at u-boot-dm/efi-working
>>>>>>
>>>>>>
>>>>>> Simon Glass (16):
>>>>>> efi: Update efi_smbios_register() to return error code
>>>>>> efi: Move the init check inside efi_init_obj_list()
>>>>>> efi: Add error checking for efi_init_obj_list()
>>>>>> efi: Add a TODO to efi_init_obj_list()
>>>>>> efi: Correct header order in efi_memory
>>>>>> efi: sandbox: Adjust memory setup for sandbox
>>>>>> sandbox: smbios: Update to support sandbox
>>>>>> sandbox: Add a setjmp() implementation
>>>>>> efi: sandbox: Add required linker sections
>>>>>> efi: sandbox: Add distroboot support
>>>>>> Define board_quiesce_devices() in a shared location
>>>>>> Add a comment for board_quiesce_devices()
>>>>>> efi: sandbox: Add relocation constants
>>>>>> efi: Add a comment about duplicated ELF constants
>>>>>> efi: sandbox: Enable EFI loader builder for sandbox
>>>>>> efi: sandbox: Add a simple 'bootefi test' command
>>>>>>
>>>>>> arch/arm/include/asm/u-boot-arm.h | 1 -
>>>>>> arch/sandbox/cpu/cpu.c | 13 ++++++++++
>>>>>> arch/sandbox/cpu/os.c | 17 ++++++++++++
>>>>>> arch/sandbox/cpu/u-boot.lds | 29 +++++++++++++++++++++
>>>>>> arch/sandbox/include/asm/setjmp.h | 21 +++++++++++++++
>>>>>> arch/sandbox/lib/Makefile | 2 +-
>>>>>> arch/sandbox/lib/sections.c | 12 +++++++++
>>>>>> arch/x86/include/asm/u-boot-x86.h | 1 -
>>>>>> arch/x86/lib/bootm.c | 4 ---
>>>>>> cmd/bootefi.c | 54 ++++++++++++++++++++++++++++++++++-----
>>>>>> common/bootm.c | 4 +++
>>>>>> configs/sandbox_defconfig | 1 +
>>>>>> include/bootm.h | 8 ++++++
>>>>>> include/config_distro_bootcmd.h | 2 +-
>>>>>> include/efi_loader.h | 13 ++++++++--
>>>>>> include/os.h | 21 +++++++++++++++
>>>>>> lib/efi_loader/Kconfig | 12 ++++++++-
>>>>>> lib/efi_loader/Makefile | 1 +
>>>>>> lib/efi_loader/efi_boottime.c | 4 +++
>>>>>> lib/efi_loader/efi_memory.c | 33 +++++++++++++-----------
>>>>>> lib/efi_loader/efi_runtime.c | 7 +++++
>>>>>> lib/efi_loader/efi_smbios.c | 6 +++--
>>>>>> lib/efi_loader/efi_test.c | 17 ++++++++++++
>>>>>> lib/smbios.c | 38 ++++++++++++++++++++-------
>>>>>> 24 files changed, 277 insertions(+), 44 deletions(-)
>>>>>> create mode 100644 arch/sandbox/include/asm/setjmp.h
>>>>>> create mode 100644 arch/sandbox/lib/sections.c
>>>>>> create mode 100644 lib/efi_loader/efi_test.c
>>>>>>
>>>>> Thanks for enabling efi_loader on sandbox. That will make many things
>>>>> easier.
>>>>>
>>>>> Unfortunately
>>>>> efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
>>>>> struct efi_system_table *systab)
>>>>> {
>>>>> ...
>>>>> boottime = systable->boottime;
>>>>> ...
>>>>> ret = boottime->allocate_pool(EFI_BOOT_SERVICES_DATA, map_size,
>>>>> (void **)&memory_map);
>>>>> leads to a segmentation fault:
>>>>
>>>> I'm seeing something similar, because:
>>>>
>>>> (gdb) print gd->bd->bi_dram[0]
>>>> $2 = {start = 0, size = 134217728}
>>>>
>>>> u-boot expects 1:1 phys:virt mapping, so that probably won't work.
>>>
>>> The following quick hack works.. something similar could probably be
>>> smashed in to ""
>>>
>>> --------
>>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>>> index cddafe2d43..da2079a4b1 100644
>>> --- a/lib/efi_loader/efi_memory.c
>>> +++ b/lib/efi_loader/efi_memory.c
>>> @@ -459,9 +459,10 @@ int efi_memory_init(void)
>>> unsigned long uboot_start, uboot_pages;
>>> unsigned long uboot_stack_size = 16 * 1024 * 1024;
>>>
>>> - efi_add_known_memory();
>>>
>>> if (!IS_ENABLED(CONFIG_SANDBOX)) {
>>> + efi_add_known_memory();
>>> +
>>> /* Add U-Boot */
>>> uboot_start = (gd->start_addr_sp - uboot_stack_size) &
>>> ~EFI_PAGE_MASK;
>>> @@ -476,6 +477,12 @@ 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);
>>> + } else {
>>> +#define SZ_256M 0x10000000
>>> + size_t sz = SZ_256M;
>>> + void *ram = os_malloc(sz);
>>> + efi_add_memory_map((uintptr_t)ram, sz >> EFI_PAGE_SHIFT,
>>> + EFI_CONVENTIONAL_MEMORY, false);
>>> }
>>>
>>> #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
>>> --------
>>>
>>> With that I'm at least getting further.. efi_allocate_pool()
>>> eventually fails, possibly making every small memory allocation page
>>> aligned means that 256m isn't enough..
>>
>> Ok, still just as hacky, but works a bit better:
>>
>> ---------
>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>> index cddafe2d43..b546b5e35d 100644
>> --- a/lib/efi_loader/efi_memory.c
>> +++ b/lib/efi_loader/efi_memory.c
>> @@ -14,6 +14,7 @@
>> #include <linux/list_sort.h>
>> #include <inttypes.h>
>> #include <watchdog.h>
>> +#include <os.h>
>>
>> DECLARE_GLOBAL_DATA_PTR;
>>
>> @@ -459,9 +460,9 @@ int efi_memory_init(void)
>> unsigned long uboot_start, uboot_pages;
>> unsigned long uboot_stack_size = 16 * 1024 * 1024;
>>
>> - efi_add_known_memory();
>> -
>> if (!IS_ENABLED(CONFIG_SANDBOX)) {
>> + efi_add_known_memory();
>> +
>> /* Add U-Boot */
>> uboot_start = (gd->start_addr_sp - uboot_stack_size) &
>> ~EFI_PAGE_MASK;
>> @@ -476,6 +477,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);
>> + } else {
>> +#define SZ_4K 0x00001000
>> +#define SZ_256M 0x10000000
>> + size_t sz = SZ_256M;
>> + uintptr_t ram = (uintptr_t)os_malloc(sz + SZ_4K) + SZ_4K;
>> + efi_add_memory_map(ram & ~EFI_PAGE_MASK, sz >> EFI_PAGE_SHIFT,
>> + EFI_CONVENTIONAL_MEMORY, false);
>> + gd->start_addr_sp = ~0;
>> }
>>
>> #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
>> ---------
>>
>> At this point it crashes in efi_load_pe() when it first tries to
>> dereference the address of the image passed in, ie. I'm running:
>>
>> host bind 0 x86_64-sct.img
>> load host 0:1 0x01000000 /efi/boot/shell.efi
>> bootefi 0x01000000
>>
>> Not sure if there is a better way to pick an address to load into. Or
>> maybe just assuming that PA==VA isn't a good idea in sandbox?
>>
>
> Ok, I realized there is map_sysmem().. which gets me further..
> efi_loader really expects identity mapping (PA==VA), and iirc this is
> what UEFI spec expects too so I wouldn't necessarily call it a bug in
> efi_loader.
Well we need to make it work :-)
In general casting an address to a pointer (or vice versa) should be
avoided in U-Boot now that we have map_sysmem(). It is incompatible
with sandbox.
Regards,
Simon
More information about the U-Boot
mailing list