Hi Rob, On 18 September 2017 at 09:07, Rob Clark <robdcl...@gmail.com> wrote: > On Mon, Sep 18, 2017 at 10:30 AM, Rob Clark <robdcl...@gmail.com> wrote: >> On Mon, Sep 18, 2017 at 9:31 AM, Rob Clark <robdcl...@gmail.com> wrote: >>> On Mon, Sep 18, 2017 at 9:18 AM, Rob Clark <robdcl...@gmail.com> wrote: >>>> On Sun, Sep 17, 2017 at 11:48 PM, Heinrich Schuchardt >>>> <xypron.g...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot