On 26.12.15 19:09, Leif Lindholm wrote: > On Tue, Dec 22, 2015 at 02:57:51PM +0100, Alexander Graf wrote: >> When an EFI application runs, it has access to a few descriptor and callback >> tables to instruct the EFI compliant firmware to do things for it. The bulk >> of those interfaces are "boot time services". They handle all object >> management, >> and memory allocation. >> >> This patch adds support for the boot time services and also exposes a system >> table, which is the point of entry descriptor table for EFI payloads. > > One overall observation, and I may help track these down - but not all > for this review: this code uses EFI_UNSUPPORTED as a default > "something went wrong" error code, but this is not actually supported > by the specification. I'm pointing out a few of these, but it would be > preferable if we could crowdsource this a bit since there are quire a > few instances... > >> Signed-off-by: Alexander Graf <ag...@suse.de> >> --- >> include/efi_loader.h | 41 +++ >> lib/efi_loader/efi_boottime.c | 838 >> ++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 879 insertions(+) >> create mode 100644 lib/efi_loader/efi_boottime.c >> >> diff --git a/include/efi_loader.h b/include/efi_loader.h >> index da82354..ed7c389 100644 >> --- a/include/efi_loader.h >> +++ b/include/efi_loader.h >> @@ -24,14 +24,55 @@ >> #include <efi_api.h> >> #include <linux/list.h> >> >> +/* #define DEBUG_EFI */ >> + >> +#ifdef DEBUG_EFI >> +#define EFI_ENTRY(format, ...) do { \ >> + efi_restore_gd(); \ >> + printf("EFI: Entry %s(" format ")\n", __func__, ##__VA_ARGS__); \ >> + } while(0) >> +#else >> +#define EFI_ENTRY(format, ...) do { \ >> + efi_restore_gd(); \ >> + } while(0) >> +#endif >> + >> +#define EFI_EXIT(ret) efi_exit_func(ret); >> + >> +extern struct efi_system_table systab; >> + >> extern const efi_guid_t efi_guid_device_path; >> extern const efi_guid_t efi_guid_loaded_image; >> >> +struct efi_class_map { >> + const efi_guid_t *guid; >> + const void *interface; >> +}; >> + >> +struct efi_handler { >> + const efi_guid_t *guid; >> + efi_status_t (EFIAPI *open)(void *handle, >> + efi_guid_t *protocol, void **protocol_interface, >> + void *agent_handle, void *controller_handle, >> + uint32_t attributes); >> +}; >> + >> +struct efi_object { >> + struct list_head link; >> + struct efi_handler protocols[4]; >> + void *handle; >> +}; >> +extern struct list_head efi_obj_list; >> + >> efi_status_t efi_return_handle(void *handle, >> efi_guid_t *protocol, void **protocol_interface, >> void *agent_handle, void *controller_handle, >> uint32_t attributes); >> +void efi_timer_check(void); >> void *efi_load_pe(void *efi, struct efi_loaded_image *loaded_image_info); >> +void efi_save_gd(void); >> +void efi_restore_gd(void); >> +efi_status_t efi_exit_func(efi_status_t ret); >> >> #define EFI_LOADER_POOL_SIZE (128 * 1024 * 1024) >> void *efi_loader_alloc(uint64_t len); >> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c >> new file mode 100644 >> index 0000000..ed95962 >> --- /dev/null >> +++ b/lib/efi_loader/efi_boottime.c >> @@ -0,0 +1,838 @@ >> +/* >> + * EFI application boot time services >> + * >> + * Copyright (c) 2015 Alexander Graf >> + * >> + * This library is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU Lesser General Public >> + * License as published by the Free Software Foundation; either >> + * version 2.1 of the License, or (at your option) any later version. >> + * >> + * This library is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> + * Lesser General Public License for more details. >> + * >> + * You should have received a copy of the GNU Lesser General Public >> + * License along with this library; if not, write to the Free Software >> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, >> USA >> + * >> + * SPDX-License-Identifier: LGPL-2.1+ >> + */ >> + >> +#define DEBUG_EFI >> + >> +#include <common.h> >> +#include <efi_loader.h> >> +#include <malloc.h> >> +#include <asm/global_data.h> >> +#include <libfdt_env.h> >> +#include <u-boot/crc.h> >> +#include <bootm.h> >> +#include <inttypes.h> >> +#include <watchdog.h> >> + >> +DECLARE_GLOBAL_DATA_PTR; >> + >> +/* >> + * EFI can pass arbitrary additional "tables" containing vendor specific >> + * information to the payload. One such table is the FDT table which >> contains >> + * a pointer to a flattened device tree blob. >> + * >> + * In most cases we want to pass an FDT to the payload, so reserve one slot >> of >> + * config table space for it. The pointer gets populated by >> do_bootefi_exec(). >> + */ >> +static struct efi_configuration_table efi_conf_table[] = { >> + { >> + .guid = EFI_FDT_GUID, >> + }, >> +}; >> + >> +/* >> + * The "gd" pointer lives in a register on ARM and AArch64 that we declare >> + * fixed when compiling U-Boot. However, the payload does now know about >> that >> + * restriction so we need to manually swap its and our view of that >> register on >> + * EFI callback entry/exit. >> + */ >> +static volatile void *efi_gd, *app_gd; >> + >> +/* Called from do_bootefi_exec() */ >> +void efi_save_gd(void) >> +{ >> + efi_gd = gd; >> +} >> + >> +/* Called on every callback entry */ >> +void efi_restore_gd(void) >> +{ >> + if (gd != efi_gd) >> + app_gd = gd; >> + gd = efi_gd; >> +} >> + >> +/* Called on every callback exit */ >> +efi_status_t efi_exit_func(efi_status_t ret) >> +{ >> + gd = app_gd; >> + return ret; >> +} >> + >> +static efi_status_t efi_unsupported(const char *funcname) >> +{ >> +#ifdef DEBUG_EFI >> + printf("EFI: App called into unimplemented function %s\n", funcname); >> +#endif >> + return EFI_EXIT(EFI_UNSUPPORTED); > > Not always a legal return status. > >> +} >> + >> +static unsigned long efi_raise_tpl(unsigned long new_tpl) >> +{ >> + EFI_ENTRY("0x%lx", new_tpl); >> + return EFI_EXIT(efi_unsupported(__func__)); > > "Unlike other UEFI interface functions, EFI_BOOT_SERVICES.RaiseTPL() > does not return a status code. Instead, it returns the previous task > priority level, which is to be restored later with a matching call to > RestoreTPL()."
Since we don't do TPLs (or IRQs for that matter), I'll just return 0 here. > >> +} >> + >> +static void efi_restore_tpl(unsigned long old_tpl) >> +{ >> + EFI_ENTRY("0x%lx", old_tpl); >> + EFI_EXIT(efi_unsupported(__func__)); > > (void function, nothing to return) Yes, hence no return. EFI_EXIT deals with the gd swapping and efi_unsupported() gives me a nice debug message :). > >> +} >> + >> +static void *efi_alloc(uint64_t len, int memory_type) >> +{ >> + switch (memory_type) { >> + case EFI_LOADER_DATA: >> + return efi_loader_alloc(len); >> + default: >> + return malloc(len); >> + } >> +} >> + >> +static efi_status_t efi_allocate_pages(int type, int memory_type, >> + unsigned long pages, uint64_t *memory) >> +{ >> + u64 len = pages << 12; >> + efi_status_t r = EFI_SUCCESS; >> + >> + EFI_ENTRY("%d, %d, 0x%lx, %p", type, memory_type, pages, memory); >> + >> + switch (type) { >> + case 0: >> + /* Any page means we can go to efi_alloc */ >> + *memory = (unsigned long)efi_alloc(len, memory_type); >> + break; >> + case 1: >> + /* Max address */ >> + if (gd->relocaddr < *memory) { >> + *memory = (unsigned long)efi_alloc(len, memory_type); >> + break; >> + } >> + r = EFI_UNSUPPORTED; > > EFI_OUT_OF_RESOURCES/EFI_NOT_FOUND? > >> + break; >> + case 2: >> + /* Exact address, grant it. The addr is already in *memory. */ > > As far as I can tell, this is why GRUB works. Because it filters > through the memory map manually, requesting to allocate its heap at an > exact address in a region of free memory in the UEFI memory map. Yes. > The key is that EFI_LOADER_MEMORY will be used by applications loaded > as well as by U-Boot to load applications into. A simple example where > this could be problematic would be a large(ish) initrd loaded via initrd= > on kernel (stub loader) command line rather than via GRUB. Ah, so here the 128MB limit on the LOADER_DATA section might bite us? > >> + break; >> + default: > > It would actually be fair here to state that the above are the only > types supported by the UEFI specification, as opposed to not being > implemented. > >> + r = EFI_UNSUPPORTED; > > Actually, not a valid return value. > EFI_INVALID_PARAMETER > >> + break; >> + } >> + >> + return EFI_EXIT(r); >> +} >> + >> +static efi_status_t efi_free_pages(uint64_t memory, unsigned long pages) >> +{ >> + /* We don't free, let's cross our fingers we have plenty RAM */ >> + EFI_ENTRY("%"PRIx64", 0x%lx", memory, pages); >> + return EFI_EXIT(EFI_SUCCESS); >> +} >> + >> +/* >> + * Returns the EFI memory map. In our case, this looks pretty simple: >> + * >> + * ____________________________ TOM >> + * | | >> + * | Second half of U-Boot | >> + * |____________________________| &__efi_runtime_stop >> + * | | >> + * | EFI Runtime Services | >> + * |____________________________| &__efi_runtime_start >> + * | | >> + * | First half of U-Boot | >> + * |____________________________| start of EFI loader allocation space >> + * | | >> + * | Free RAM | >> + * |____________________________| CONFIG_SYS_SDRAM_BASE >> + * >> + * All pointers are extended to live on a 4k boundary. After exiting the >> boot >> + * services, only the EFI Runtime Services chunk of memory stays alive. >> + */ >> +static efi_status_t efi_get_memory_map(unsigned long *memory_map_size, >> + struct efi_mem_desc *memory_map, >> + unsigned long *map_key, >> + unsigned long *descriptor_size, >> + uint32_t *descriptor_version) >> +{ >> + struct efi_mem_desc efi_memory_map[] = { >> + { >> + /* RAM before U-Boot */ >> + .type = EFI_CONVENTIONAL_MEMORY, >> + .attribute = 1 << EFI_MEMORY_WB_SHIFT, >> + }, >> + { >> + /* First half of U-Boot */ >> + .type = EFI_LOADER_DATA, >> + .attribute = 1 << EFI_MEMORY_WB_SHIFT, >> + }, >> + { >> + /* EFI Runtime Services */ >> + .type = EFI_RUNTIME_SERVICES_CODE, >> + .attribute = 1 << EFI_MEMORY_WB_SHIFT, >> + }, >> + { >> + /* Second half of U-Boot */ >> + .type = EFI_LOADER_DATA, >> + .attribute = 1 << EFI_MEMORY_WB_SHIFT, >> + }, >> + }; >> + ulong runtime_start, runtime_end, runtime_len_pages, runtime_len; >> + >> + EFI_ENTRY("%p, %p, %p, %p, %p", memory_map_size, memory_map, map_key, >> + descriptor_size, descriptor_version); >> + >> + runtime_start = (ulong)&__efi_runtime_start & ~0xfffULL; >> + runtime_end = ((ulong)&__efi_runtime_stop + 0xfff) & ~0xfffULL; >> + runtime_len_pages = (runtime_end - runtime_start) >> 12; >> + runtime_len = runtime_len_pages << 12; >> + >> + /* Fill in where normal RAM is (up to U-Boot) */ >> + efi_memory_map[0].num_pages = gd->relocaddr >> 12; > > U-Boot question: is gd->relocaddr always the offset from start of RAM? > How does this work with gaps in memory map? U-Boot always relocates itself at TOM (or at least what we consider TOM here). gd->relocaddr is the physical address of the start of U-Boot which is right below TOM. > >> +#ifdef CONFIG_SYS_SDRAM_BASE >> + efi_memory_map[0].physical_start = CONFIG_SYS_SDRAM_BASE; >> + efi_memory_map[0].virtual_start = CONFIG_SYS_SDRAM_BASE; >> + efi_memory_map[0].num_pages -= CONFIG_SYS_SDRAM_BASE >> 12; > #else > #error "..." > ? If it's not defined, it's 0 :). >> +#endif >> + >> + /* Remove U-Boot from the available RAM view */ >> + efi_memory_map[0].num_pages -= gd->mon_len >> 12; >> + >> + /* Remove the malloc area from the available RAM view */ >> + efi_memory_map[0].num_pages -= TOTAL_MALLOC_LEN >> 12; >> + >> + /* Give us some space for the stack */ >> + efi_memory_map[0].num_pages -= (16 * 1024 * 1024) >> 12; >> + >> + /* Reserve the EFI loader pool */ >> + efi_memory_map[0].num_pages -= EFI_LOADER_POOL_SIZE >> 12; >> + >> + /* Cut out the runtime services */ >> + efi_memory_map[2].physical_start = runtime_start; >> + efi_memory_map[2].virtual_start = efi_memory_map[2].physical_start; >> + efi_memory_map[2].num_pages = runtime_len_pages; >> + >> + /* Allocate the rest to U-Boot */ >> + efi_memory_map[1].physical_start = efi_memory_map[0].physical_start + >> + (efi_memory_map[0].num_pages << 12); >> + efi_memory_map[1].virtual_start = efi_memory_map[1].physical_start; >> + efi_memory_map[1].num_pages = (runtime_start - >> + efi_memory_map[1].physical_start) >> 12; >> + >> + efi_memory_map[3].physical_start = runtime_start + runtime_len; >> + efi_memory_map[3].virtual_start = efi_memory_map[3].physical_start; >> + efi_memory_map[3].num_pages = (gd->ram_top - >> + efi_memory_map[3].physical_start) >> 12; >> + >> + *memory_map_size = sizeof(efi_memory_map); >> + >> + if (descriptor_size) >> + *descriptor_size = sizeof(struct efi_mem_desc); >> + >> + if (*memory_map_size < sizeof(efi_memory_map)) { >> + return EFI_EXIT(EFI_BUFFER_TOO_SMALL); >> + } >> + >> + if (memory_map) >> + memcpy(memory_map, efi_memory_map, sizeof(efi_memory_map)); >> + >> + return EFI_EXIT(EFI_SUCCESS); >> +} >> + >> +static efi_status_t efi_allocate_pool(int pool_type, unsigned long size, >> void **buffer) >> +{ >> + return efi_allocate_pages(0, pool_type, (size + 0xfff) >> 12, >> (void*)buffer); >> +} >> + >> +static efi_status_t efi_free_pool(void *buffer) >> +{ >> + return efi_free_pages((ulong)buffer, 0); >> +} >> + >> +/* >> + * Our event capabilities are very limited. Only support a single >> + * event to exist, so we don't need to maintain lists. >> + */ >> +static struct { >> + enum efi_event_type type; >> + u32 trigger_type; >> + u32 trigger_time; >> + u64 trigger_next; >> + unsigned long notify_tpl; >> + void (*notify_function) (void *event, void *context); >> + void *notify_context; >> +} efi_event; >> + >> +static efi_status_t efi_create_event(enum efi_event_type type, ulong >> notify_tpl, >> + void (*notify_function) (void *event, >> + void *context), >> + void *notify_context, void **event) >> +{ >> + EFI_ENTRY("%d, 0x%lx, %p, %p", type, notify_tpl, notify_function, >> + notify_context); >> + if (efi_event.notify_function) { >> + /* We only support one event at a time */ >> + return EFI_EXIT(EFI_UNSUPPORTED); > > EFI_OUT_OF_RESOURCES would be a better return value here. Yup. > >> + } >> + >> + efi_event.type = type; >> + efi_event.notify_tpl = notify_tpl; >> + efi_event.notify_function = notify_function; >> + efi_event.notify_context = notify_context; >> + *event = &efi_event; >> + >> + return EFI_EXIT(EFI_SUCCESS); >> +} >> + >> +/* >> + * Our timers have to work without interrupts, so we check whenever keyboard >> + * input or disk accesses happen if enough time elapsed for it to fire. >> + */ >> +void efi_timer_check(void) >> +{ >> + u64 now = timer_get_us(); >> + >> + if (now >= efi_event.trigger_next) { >> + /* Triggering! */ >> + if (efi_event.trigger_type == EFI_TIMER_PERIODIC) >> + efi_event.trigger_next += efi_event.trigger_time / 10; >> + efi_event.notify_function(&efi_event, efi_event.notify_context); >> + } >> + >> + WATCHDOG_RESET(); >> +} >> + >> +static efi_status_t efi_set_timer(void *event, int type, uint64_t >> trigger_time) >> +{ >> + /* We don't have 64bit division available everywhere, so limit timer >> + * distances to 32bit bits. */ >> + u32 trigger32 = trigger_time; > > Add a warning message if this limit is exceeded? ok > >> + >> + EFI_ENTRY("%p, %d, %"PRIx64, event, type, trigger_time); >> + if (event != &efi_event) { >> + /* We only support one event at a time */ >> + return EFI_EXIT(EFI_UNSUPPORTED); > > This function should only ever be called with an event successfully > created via create_event (and stored into efi_event). If we're called > with another event handle, EFI_INVALID_PARAMETER is the appropriate > error code. Sounds reasonable. > >> + } >> + >> + switch (type) { >> + case EFI_TIMER_STOP: >> + efi_event.trigger_next = -1ULL; >> + break; >> + case EFI_TIMER_PERIODIC: >> + case EFI_TIMER_RELATIVE: >> + efi_event.trigger_next = timer_get_us() + (trigger32 / 10); >> + break; >> + default: >> + return EFI_EXIT(EFI_UNSUPPORTED); >> + } >> + efi_event.trigger_type = type; >> + efi_event.trigger_time = trigger_time; >> + >> + return EFI_EXIT(EFI_SUCCESS); >> +} [...] >> +static const struct efi_boot_services efi_boot_services = { >> + .hdr = { >> + .headersize = sizeof(struct efi_table_hdr), >> + }, >> + .raise_tpl = efi_raise_tpl, >> + .restore_tpl = efi_restore_tpl, >> + .allocate_pages = efi_allocate_pages, >> + .free_pages = efi_free_pages, >> + .get_memory_map = efi_get_memory_map, >> + .allocate_pool = efi_allocate_pool, >> + .free_pool = efi_free_pool, >> + .create_event = efi_create_event, >> + .set_timer = efi_set_timer, >> + .wait_for_event = efi_wait_for_event, >> + .signal_event = efi_signal_event, >> + .close_event = efi_close_event, >> + .check_event = efi_check_event, >> + .install_protocol_interface = efi_install_protocol_interface, >> + .reinstall_protocol_interface = efi_reinstall_protocol_interface, >> + .uninstall_protocol_interface = efi_uninstall_protocol_interface, >> + .handle_protocol = efi_handle_protocol, >> + .reserved = NULL, >> + .register_protocol_notify = efi_register_protocol_notify, >> + .locate_handle = efi_locate_handle, >> + .locate_device_path = efi_locate_device_path, >> + .install_configuration_table = efi_install_configuration_table, >> + .load_image = efi_load_image, >> + .start_image = efi_start_image, >> + .exit = (void*)efi_exit, >> + .unload_image = efi_unload_image, >> + .exit_boot_services = efi_exit_boot_services, >> + .get_next_monotonic_count = efi_get_next_monotonic_count, >> + .stall = efi_stall, >> + .set_watchdog_timer = efi_set_watchdog_timer, >> + .connect_controller = efi_connect_controller, >> + .disconnect_controller = efi_disconnect_controller, >> + .open_protocol = efi_open_protocol, >> + .close_protocol = efi_close_protocol, >> + .open_protocol_information = efi_open_protocol_information, >> + .protocols_per_handle = efi_protocols_per_handle, >> + .locate_handle_buffer = efi_locate_handle_buffer, >> + .locate_protocol = efi_locate_protocol, >> + .install_multiple_protocol_interfaces = >> efi_install_multiple_protocol_interfaces, >> + .uninstall_multiple_protocol_interfaces = >> efi_uninstall_multiple_protocol_interfaces, >> + .calculate_crc32 = efi_calculate_crc32, >> + .copy_mem = efi_copy_mem, >> + .set_mem = efi_set_mem, >> +}; >> + >> + >> +static uint16_t firmware_vendor[] = { 'U','-','b','o','o','t',0 }; > > Surely, if we're being formal, that should be 'D', 'a', 's', ' ', > ... :) Heh :) Sure! > >> +struct efi_system_table systab = { >> + .hdr = { >> + .signature = EFI_SYSTEM_TABLE_SIGNATURE, >> + .revision = 0x20000, /* 2.0 */ > > Really, this should claim to support revision 2.5, if not 2.6 (soon > to be released). AArch64 support was only introduced in 2.4. Works for me :). Alex _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot