[U-Boot] [PATCH v3 00/18] efi_loader: manage protocols in a linked list (v3)

Up to now the protocols of an EFI handle where contained in an array of fixed size. With the patch series the protocols are managed in a linked list. This both saves memory and gives more flexibility.
The LocateDevicePath boot service is implemented according to the UEFI specification.
A unit test for the LocateDevicePath boot service and the device path to text protocol are added.
Some bug fixes are provided.
v3 Remove patch efi_loader: efi_bootmgr: do not make hidden assignments (A patch to add device path printing in printf() is pending.) Resubmit efi_loader: helloworld.c: remove superfluous include v2 Enhance the helloworld example to check if the image handle is passed corectly. Adjust the related py test.
Use a helper function to initialize EFI objects.
Further minor bug fixes.
Heinrich Schuchardt (18): efi_loader: helloworld.c: remove superfluous include efi_loader: size of media device path node represenation efi_loader: efi_dp_str should print path not node efi_loader: fix efi_convert_device_node_to_text efi_loader: reimplement LocateDevicePath efi_selftest: test EFI_DEVICE_PATH_TO_TEXT_PROTOCOL efi_loader: efi_disk: use efi_add_protocol efi_loader: efi_net: use efi_add_protocol efi_loader: efi_gop: use efi_add_protocol efi_loader: simplify efi_open_protocol efi_loader: simplify find_obj efi_loader: manage protocols in a linked list efi_selftest: compile without special compiler flags efi_selftest: add missing line feed efi_loader: output load options in helloworld test/py: check return code of helloworld efi_loader: pass handle of loaded image efi_loader: helper function to add EFI object to list
cmd/bootefi.c | 7 +- include/efi_loader.h | 8 +- lib/efi_loader/efi_boottime.c | 287 ++++++++++++--------- lib/efi_loader/efi_device_path.c | 43 ++-- lib/efi_loader/efi_device_path_to_text.c | 167 ++++++------ lib/efi_loader/efi_disk.c | 38 +-- lib/efi_loader/efi_gop.c | 16 +- lib/efi_loader/efi_net.c | 35 +-- lib/efi_loader/helloworld.c | 38 ++- lib/efi_selftest/Makefile | 24 +- lib/efi_selftest/efi_selftest.c | 2 +- lib/efi_selftest/efi_selftest_devicepath.c | 391 +++++++++++++++++++++++++++++ test/py/tests/test_efi_loader.py | 2 + 13 files changed, 770 insertions(+), 288 deletions(-) create mode 100644 lib/efi_selftest/efi_selftest_devicepath.c

Remove a superfluous include from helloworld.c
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 resubmitted patch added --- lib/efi_loader/helloworld.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/lib/efi_loader/helloworld.c b/lib/efi_loader/helloworld.c index 03e65ab133..77130a36dd 100644 --- a/lib/efi_loader/helloworld.c +++ b/lib/efi_loader/helloworld.c @@ -8,7 +8,6 @@ */
#include <common.h> -#include <part_efi.h> #include <efi_api.h>
efi_status_t EFIAPI efi_main(efi_handle_t handle,

In the format specifier we want to specify the maximum width in case an ending \0 is missing.
So slen must be used as precision and not as field width.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 no change v2 no change --- lib/efi_loader/efi_device_path_to_text.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c index 62771338f0..98617cf163 100644 --- a/lib/efi_loader/efi_device_path_to_text.c +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -153,7 +153,7 @@ static char *dp_media(char *s, struct efi_device_path *dp) struct efi_device_path_file_path *fp = (struct efi_device_path_file_path *)dp; int slen = (dp->length - sizeof(*dp)) / 2; - s += sprintf(s, "/%-*ls", slen, fp->str); + s += sprintf(s, "/%-.*ls", slen, fp->str); break; } default:

efi_dp_str is meant to print a device path and not a device node.
The old coding only worked because efi_convert_device_node_to_text was screwed up to expect paths instead of nodes.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 no change v2 no change --- lib/efi_loader/efi_device_path_to_text.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c index 98617cf163..ad248cb492 100644 --- a/lib/efi_loader/efi_device_path_to_text.c +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -208,13 +208,6 @@ static uint16_t *efi_convert_device_node_to_text( return out; }
-/* helper for debug prints.. efi_free_pool() the result. */ -uint16_t *efi_dp_str(struct efi_device_path *dp) -{ - return efi_convert_device_node_to_text(dp, true, true); -} - - static uint16_t EFIAPI *efi_convert_device_node_to_text_ext( struct efi_device_path *device_node, bool display_only, @@ -251,6 +244,12 @@ static uint16_t EFIAPI *efi_convert_device_path_to_text( return buffer; }
+/* helper for debug prints.. efi_free_pool() the result. */ +uint16_t *efi_dp_str(struct efi_device_path *dp) +{ + return EFI_CALL(efi_convert_device_path_to_text(dp, true, true)); +} + const struct efi_device_path_to_text_protocol efi_device_path_to_text = { .convert_device_node_to_text = efi_convert_device_node_to_text_ext, .convert_device_path_to_text = efi_convert_device_path_to_text,

We need to implement to different functions for the EFI_DEVICE_PATH_TO_TEXT_PROTOCOL: ConvertDeviceNodeToText ConvertDevicePathToText
A recent patch screwed up efi_convert_device_node_to_text to expect a device path and not a node.
The patch makes both service functions work again.
efi_convert_device_node_to_text is renamed to efi_convert_single_device_node_to_text and efi_convert_device_node_to_text_ext is renamed to efi_convert_device_node_to_text to avoid future confusion.
A test of ConvertDeviceNodeToText will be provided in a follow-up patch.
Fixes: adae4313cdd efi_loader: flesh out device-path to text Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 Correctly handle NULL values. v2 no change --- lib/efi_loader/efi_device_path_to_text.c | 154 ++++++++++++++++++------------- 1 file changed, 88 insertions(+), 66 deletions(-)
diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c index ad248cb492..43af5dfd8d 100644 --- a/lib/efi_loader/efi_device_path_to_text.c +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -12,12 +12,31 @@ #define MAC_OUTPUT_LEN 22 #define UNKNOWN_OUTPUT_LEN 23
+#define MAX_NODE_LEN 512 +#define MAX_PATH_LEN 1024 + const efi_guid_t efi_guid_device_path_to_text_protocol = EFI_DEVICE_PATH_TO_TEXT_PROTOCOL_GUID;
+static u16 *efi_str_to_u16(char *str) +{ + efi_uintn_t len; + u16 *out; + efi_status_t ret; + + len = strlen(str) + 1; + ret = efi_allocate_pool(EFI_ALLOCATE_ANY_PAGES, len * sizeof(u16), + (void **)&out); + if (ret != EFI_SUCCESS) + return NULL; + ascii2unicode(out, str); + out[len - 1] = 0; + return out; +} + static char *dp_unknown(char *s, struct efi_device_path *dp) { - s += sprintf(s, "/UNKNOWN(%04x,%04x)", dp->type, dp->sub_type); + s += sprintf(s, "UNKNOWN(%04x,%04x)", dp->type, dp->sub_type); return s; }
@@ -27,7 +46,7 @@ static char *dp_hardware(char *s, struct efi_device_path *dp) case DEVICE_PATH_SUB_TYPE_MEMORY: { struct efi_device_path_memory *mdp = (struct efi_device_path_memory *)dp; - s += sprintf(s, "/MemoryMapped(0x%x,0x%llx,0x%llx)", + s += sprintf(s, "MemoryMapped(0x%x,0x%llx,0x%llx)", mdp->memory_type, mdp->start_address, mdp->end_address); @@ -36,7 +55,7 @@ static char *dp_hardware(char *s, struct efi_device_path *dp) case DEVICE_PATH_SUB_TYPE_VENDOR: { struct efi_device_path_vendor *vdp = (struct efi_device_path_vendor *)dp; - s += sprintf(s, "/VenHw(%pUl)", &vdp->guid); + s += sprintf(s, "VenHw(%pUl)", &vdp->guid); break; } default: @@ -52,7 +71,7 @@ static char *dp_acpi(char *s, struct efi_device_path *dp) case DEVICE_PATH_SUB_TYPE_ACPI_DEVICE: { struct efi_device_path_acpi_path *adp = (struct efi_device_path_acpi_path *)dp; - s += sprintf(s, "/Acpi(PNP%04x", EISA_PNP_NUM(adp->hid)); + s += sprintf(s, "Acpi(PNP%04x", EISA_PNP_NUM(adp->hid)); if (adp->uid) s += sprintf(s, ",%d", adp->uid); s += sprintf(s, ")"); @@ -71,7 +90,7 @@ static char *dp_msging(char *s, struct efi_device_path *dp) case DEVICE_PATH_SUB_TYPE_MSG_USB: { struct efi_device_path_usb *udp = (struct efi_device_path_usb *)dp; - s += sprintf(s, "/Usb(0x%x,0x%x)", udp->parent_port_number, + s += sprintf(s, "Usb(0x%x,0x%x)", udp->parent_port_number, udp->usb_interface); break; } @@ -82,7 +101,7 @@ static char *dp_msging(char *s, struct efi_device_path *dp) if (mdp->if_type != 0 && mdp->if_type != 1) break;
- s += sprintf(s, "/MAC(%02x%02x%02x%02x%02x%02x,0x%1x)", + s += sprintf(s, "MAC(%02x%02x%02x%02x%02x%02x,0x%1x)", mdp->mac.addr[0], mdp->mac.addr[1], mdp->mac.addr[2], mdp->mac.addr[3], mdp->mac.addr[4], mdp->mac.addr[5], @@ -94,7 +113,7 @@ static char *dp_msging(char *s, struct efi_device_path *dp) struct efi_device_path_usb_class *ucdp = (struct efi_device_path_usb_class *)dp;
- s += sprintf(s, "/USBClass(%x,%x,%x,%x,%x)", + s += sprintf(s, "USBClass(%x,%x,%x,%x,%x)", ucdp->vendor_id, ucdp->product_id, ucdp->device_class, ucdp->device_subclass, ucdp->device_protocol); @@ -108,7 +127,7 @@ static char *dp_msging(char *s, struct efi_device_path *dp) "SDCard" : "MMC"; struct efi_device_path_sd_mmc_path *sddp = (struct efi_device_path_sd_mmc_path *)dp; - s += sprintf(s, "/%s(Slot%u)", typename, sddp->slot_number); + s += sprintf(s, "%s(Slot%u)", typename, sddp->slot_number); break; } default: @@ -128,15 +147,15 @@ static char *dp_media(char *s, struct efi_device_path *dp)
switch (hddp->signature_type) { case SIG_TYPE_MBR: - s += sprintf(s, "/HD(Part%d,Sig%08x)", + s += sprintf(s, "HD(Part%d,Sig%08x)", hddp->partition_number, *(uint32_t *)sig); break; case SIG_TYPE_GUID: - s += sprintf(s, "/HD(Part%d,Sig%pUl)", + s += sprintf(s, "HD(Part%d,Sig%pUl)", hddp->partition_number, sig); default: - s += sprintf(s, "/HD(Part%d,MBRType=%02x,SigType=%02x)", + s += sprintf(s, "HD(Part%d,MBRType=%02x,SigType=%02x)", hddp->partition_number, hddp->partmap_type, hddp->signature_type); } @@ -146,14 +165,16 @@ static char *dp_media(char *s, struct efi_device_path *dp) case DEVICE_PATH_SUB_TYPE_CDROM_PATH: { struct efi_device_path_cdrom_path *cddp = (struct efi_device_path_cdrom_path *)dp; - s += sprintf(s, "/CDROM(0x%x)", cddp->boot_entry); + s += sprintf(s, "CDROM(0x%x)", cddp->boot_entry); break; } case DEVICE_PATH_SUB_TYPE_FILE_PATH: { struct efi_device_path_file_path *fp = (struct efi_device_path_file_path *)dp; int slen = (dp->length - sizeof(*dp)) / 2; - s += sprintf(s, "/%-.*ls", slen, fp->str); + if (slen > MAX_NODE_LEN - 2) + slen = MAX_NODE_LEN - 2; + s += sprintf(s, "%-.*ls", slen, fp->str); break; } default: @@ -163,65 +184,59 @@ static char *dp_media(char *s, struct efi_device_path *dp) return s; }
-static uint16_t *efi_convert_device_node_to_text( - struct efi_device_path *dp, - bool display_only, - bool allow_shortcuts) +/* + * Converts a single node to a char string. + * + * @buffer output buffer + * @dp device path or node + * @return end of string + */ +static char *efi_convert_single_device_node_to_text( + char *buffer, + struct efi_device_path *dp) { - unsigned long len; - efi_status_t r; - char buf[512]; /* this ought be be big enough for worst case */ - char *str = buf; - uint16_t *out; - - while (dp) { - switch (dp->type) { - case DEVICE_PATH_TYPE_HARDWARE_DEVICE: - str = dp_hardware(str, dp); - break; - case DEVICE_PATH_TYPE_ACPI_DEVICE: - str = dp_acpi(str, dp); - break; - case DEVICE_PATH_TYPE_MESSAGING_DEVICE: - str = dp_msging(str, dp); - break; - case DEVICE_PATH_TYPE_MEDIA_DEVICE: - str = dp_media(str, dp); - break; - default: - str = dp_unknown(str, dp); - } + char *str = buffer;
- dp = efi_dp_next(dp); + switch (dp->type) { + case DEVICE_PATH_TYPE_HARDWARE_DEVICE: + str = dp_hardware(str, dp); + break; + case DEVICE_PATH_TYPE_ACPI_DEVICE: + str = dp_acpi(str, dp); + break; + case DEVICE_PATH_TYPE_MESSAGING_DEVICE: + str = dp_msging(str, dp); + break; + case DEVICE_PATH_TYPE_MEDIA_DEVICE: + str = dp_media(str, dp); + break; + default: + str = dp_unknown(str, dp); }
- *str++ = '\0'; - - len = str - buf; - r = efi_allocate_pool(EFI_ALLOCATE_ANY_PAGES, 2 * len, (void **)&out); - if (r != EFI_SUCCESS) - return NULL; - - ascii2unicode(out, buf); - out[len - 1] = 0; - - return out; + *str = '\0'; + return str; }
-static uint16_t EFIAPI *efi_convert_device_node_to_text_ext( +static uint16_t EFIAPI *efi_convert_device_node_to_text( struct efi_device_path *device_node, bool display_only, bool allow_shortcuts) { - uint16_t *buffer; + char str[MAX_NODE_LEN]; + uint16_t *text = NULL;
EFI_ENTRY("%p, %d, %d", device_node, display_only, allow_shortcuts);
- buffer = efi_convert_device_node_to_text(device_node, display_only, - allow_shortcuts); + if (!device_node) + goto out; + efi_convert_single_device_node_to_text(str, device_node); + + text = efi_str_to_u16(str);
+out: EFI_EXIT(EFI_SUCCESS); - return buffer; + return text; }
static uint16_t EFIAPI *efi_convert_device_path_to_text( @@ -229,19 +244,26 @@ static uint16_t EFIAPI *efi_convert_device_path_to_text( bool display_only, bool allow_shortcuts) { - uint16_t *buffer; + uint16_t *text = NULL; + char buffer[MAX_PATH_LEN]; + char *str = buffer;
EFI_ENTRY("%p, %d, %d", device_path, display_only, allow_shortcuts);
- /* - * Our device paths are all of depth one. So its is sufficient to - * to convert the first node. - */ - buffer = efi_convert_device_node_to_text(device_path, display_only, - allow_shortcuts); + if (!device_path) + goto out; + while (device_path && + str + MAX_NODE_LEN < buffer + MAX_PATH_LEN) { + *str++ = '/'; + str = efi_convert_single_device_node_to_text(str, device_path); + device_path = efi_dp_next(device_path); + } + + text = efi_str_to_u16(buffer);
+out: EFI_EXIT(EFI_SUCCESS); - return buffer; + return text; }
/* helper for debug prints.. efi_free_pool() the result. */ @@ -251,6 +273,6 @@ uint16_t *efi_dp_str(struct efi_device_path *dp) }
const struct efi_device_path_to_text_protocol efi_device_path_to_text = { - .convert_device_node_to_text = efi_convert_device_node_to_text_ext, + .convert_device_node_to_text = efi_convert_device_node_to_text, .convert_device_path_to_text = efi_convert_device_path_to_text, };

The current implementation of efi_locate_device_path does not match the UEFI specification. It completely ignores the protocol parameters.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 remove trailing dot in comment v2 no change --- lib/efi_loader/efi_boottime.c | 106 ++++++++++++++++++++++++++++++------------ 1 file changed, 76 insertions(+), 30 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 9b5512f086..565addfc3f 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1080,36 +1080,6 @@ static efi_status_t EFIAPI efi_locate_handle_ext( buffer_size, buffer)); }
-/* - * Get the device path and handle of an device implementing a protocol. - * - * This function implements the LocateDevicePath service. - * See the Unified Extensible Firmware Interface (UEFI) specification - * for details. - * - * @protocol GUID of the protocol - * @device_path device path - * @device handle of the device - * @return status code - */ -static efi_status_t EFIAPI efi_locate_device_path( - const efi_guid_t *protocol, - struct efi_device_path **device_path, - efi_handle_t *device) -{ - struct efi_object *efiobj; - - EFI_ENTRY("%pUl, %p, %p", protocol, device_path, device); - - efiobj = efi_dp_find_obj(*device_path, device_path); - if (!efiobj) - return EFI_EXIT(EFI_NOT_FOUND); - - *device = efiobj->handle; - - return EFI_EXIT(EFI_SUCCESS); -} - /* Collapses configuration table entries, removing index i */ static void efi_remove_configuration_table(int i) { @@ -1813,6 +1783,82 @@ static efi_status_t EFIAPI efi_locate_protocol(const efi_guid_t *protocol, return EFI_EXIT(EFI_NOT_FOUND); }
+/* + * Get the device path and handle of an device implementing a protocol. + * + * This function implements the LocateDevicePath service. + * See the Unified Extensible Firmware Interface (UEFI) specification + * for details. + * + * @protocol GUID of the protocol + * @device_path device path + * @device handle of the device + * @return status code + */ +static efi_status_t EFIAPI efi_locate_device_path( + const efi_guid_t *protocol, + struct efi_device_path **device_path, + efi_handle_t *device) +{ + struct efi_device_path *dp; + size_t i; + struct efi_handler *handler; + efi_handle_t *handles; + size_t len, len_dp; + size_t len_best = 0; + efi_uintn_t no_handles; + u8 *remainder; + efi_status_t ret; + + EFI_ENTRY("%pUl, %p, %p", protocol, device_path, device); + + if (!protocol || !device_path || !*device_path || !device) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + + /* Find end of device path */ + len = efi_dp_size(*device_path); + + /* Get all handles implementing the protocol */ + ret = EFI_CALL(efi_locate_handle_buffer(BY_PROTOCOL, protocol, NULL, + &no_handles, &handles)); + if (ret != EFI_SUCCESS) + goto out; + + for (i = 0; i < no_handles; ++i) { + /* Find the device path protocol */ + ret = efi_search_protocol(handles[i], &efi_guid_device_path, + &handler); + if (ret != EFI_SUCCESS) + continue; + dp = (struct efi_device_path *)handler->protocol_interface; + len_dp = efi_dp_size(dp); + /* + * This handle can only be a better fit + * if its device path length is longer than the best fit and + * if its device path length is shorter of equal the searched + * device path. + */ + if (len_dp <= len_best || len_dp > len) + continue; + /* Check if dp is a subpath of device_path */ + if (memcmp(*device_path, dp, len_dp)) + continue; + *device = handles[i]; + len_best = len_dp; + } + if (len_best) { + remainder = (u8 *)*device_path + len_best; + *device_path = (struct efi_device_path *)remainder; + ret = EFI_SUCCESS; + } else { + ret = EFI_NOT_FOUND; + } +out: + return EFI_EXIT(ret); +} + /* * Install multiple protocol interfaces. *

Provide a test for the EFI_DEVICE_PATH_TO_TEXT_PROTOCOL protocol.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 Print all installed device paths. v2 No change --- lib/efi_selftest/Makefile | 3 + lib/efi_selftest/efi_selftest_devicepath.c | 391 +++++++++++++++++++++++++++++ 2 files changed, 394 insertions(+) create mode 100644 lib/efi_selftest/efi_selftest_devicepath.c
diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile index 1851c17db6..d280eca5c3 100644 --- a/lib/efi_selftest/Makefile +++ b/lib/efi_selftest/Makefile @@ -11,6 +11,8 @@ CFLAGS_efi_selftest.o := $(CFLAGS_EFI) CFLAGS_REMOVE_efi_selftest.o := $(CFLAGS_NON_EFI) CFLAGS_efi_selftest_console.o := $(CFLAGS_EFI) CFLAGS_REMOVE_efi_selftest_console.o := $(CFLAGS_NON_EFI) +CFLAGS_efi_selftest_devicepathe.o := $(CFLAGS_EFI) +CFLAGS_REMOVE_efi_selftest_devicepath.o := $(CFLAGS_NON_EFI) CFLAGS_efi_selftest_events.o := $(CFLAGS_EFI) CFLAGS_REMOVE_efi_selftest_events.o := $(CFLAGS_NON_EFI) CFLAGS_efi_selftest_exitbootservices.o := $(CFLAGS_EFI) @@ -33,6 +35,7 @@ CFLAGS_REMOVE_efi_selftest_watchdog.o := $(CFLAGS_NON_EFI) obj-$(CONFIG_CMD_BOOTEFI_SELFTEST) += \ efi_selftest.o \ efi_selftest_console.o \ +efi_selftest_devicepath.o \ efi_selftest_events.o \ efi_selftest_exitbootservices.o \ efi_selftest_gop.o \ diff --git a/lib/efi_selftest/efi_selftest_devicepath.c b/lib/efi_selftest/efi_selftest_devicepath.c new file mode 100644 index 0000000000..55f4632b14 --- /dev/null +++ b/lib/efi_selftest/efi_selftest_devicepath.c @@ -0,0 +1,390 @@ +/* + * efi_selftest_devicepath + * + * Copyright (c) 2017 Heinrich Schuchardt xypron.glpk@gmx.de + * + * SPDX-License-Identifier: GPL-2.0+ + * + * This unit test checks the following protocol services: + * DevicePathToText + */ + +#include <efi_selftest.h> + +static struct efi_boot_services *boottime; + +static efi_handle_t handle1; +static efi_handle_t handle2; +static efi_handle_t handle3; + +struct interface { + void (EFIAPI * inc)(void); +} interface; + +static efi_guid_t guid_device_path = DEVICE_PATH_GUID; + +static efi_guid_t guid_device_path_to_text_protocol = + EFI_DEVICE_PATH_TO_TEXT_PROTOCOL_GUID; + +static efi_guid_t guid_protocol = + EFI_GUID(0xdbca4c98, 0x6cb0, 0x694d, + 0x08, 0x72, 0x81, 0x9c, 0x65, 0x0c, 0xbb, 0x7d); + +static efi_guid_t guid_vendor1 = + EFI_GUID(0xdbca4c98, 0x6cb0, 0x694d, + 0x08, 0x72, 0x81, 0x9c, 0x65, 0x0c, 0xbb, 0xb1); + +static efi_guid_t guid_vendor2 = + EFI_GUID(0xdbca4c98, 0x6cb0, 0x694d, + 0x08, 0x72, 0x81, 0x9c, 0x65, 0x0c, 0xbb, 0xa2); + +static efi_guid_t guid_vendor3 = + EFI_GUID(0xdbca4c98, 0x6cb0, 0x694d, + 0x08, 0x72, 0x81, 0x9c, 0x65, 0x0c, 0xbb, 0xc3); + +static u8 *dp1; +static u8 *dp2; +static u8 *dp3; + +struct efi_device_path_to_text_protocol *device_path_to_text; + +/* + * Setup unit test. + * + * Create three handles. Install a new protocol on two of them and + * provice device paths. + * + * handle1 + * guid interface + * handle2 + * guid interface + * handle3 + * + * @handle: handle of the loaded image + * @systable: system table + */ +static int setup(const efi_handle_t img_handle, + const struct efi_system_table *systable) +{ + struct efi_device_path_vendor vendor_node; + struct efi_device_path end_node; + efi_status_t ret; + + boottime = systable->boottime; + + ret = boottime->locate_protocol(&guid_device_path_to_text_protocol, + NULL, (void **)&device_path_to_text); + if (ret != EFI_SUCCESS) { + device_path_to_text = NULL; + efi_st_error( + "Device path to text protocol is not available.\n"); + return EFI_ST_FAILURE; + } + + ret = boottime->allocate_pool(EFI_LOADER_DATA, + sizeof(struct efi_device_path_vendor) + + sizeof(struct efi_device_path), + (void **)&dp1); + if (ret != EFI_SUCCESS) + goto out_of_memory; + + ret = boottime->allocate_pool(EFI_LOADER_DATA, 2 * + sizeof(struct efi_device_path_vendor) + + sizeof(struct efi_device_path), + (void **)&dp2); + if (ret != EFI_SUCCESS) + goto out_of_memory; + + ret = boottime->allocate_pool(EFI_LOADER_DATA, 3 * + sizeof(struct efi_device_path_vendor) + + sizeof(struct efi_device_path), + (void **)&dp3); + if (ret != EFI_SUCCESS) + goto out_of_memory; + + vendor_node.dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE; + vendor_node.dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR; + vendor_node.dp.length = sizeof(struct efi_device_path_vendor); + + boottime->copy_mem(&vendor_node.guid, &guid_vendor1, + sizeof(efi_guid_t)); + boottime->copy_mem(dp1, &vendor_node, + sizeof(struct efi_device_path_vendor)); + boottime->copy_mem(dp2, &vendor_node, + sizeof(struct efi_device_path_vendor)); + boottime->copy_mem(dp3, &vendor_node, + sizeof(struct efi_device_path_vendor)); + + boottime->copy_mem(&vendor_node.guid, &guid_vendor2, + sizeof(efi_guid_t)); + boottime->copy_mem(dp2 + sizeof(struct efi_device_path_vendor), + &vendor_node, sizeof(struct efi_device_path_vendor)); + boottime->copy_mem(dp3 + sizeof(struct efi_device_path_vendor), + &vendor_node, sizeof(struct efi_device_path_vendor)); + + boottime->copy_mem(&vendor_node.guid, &guid_vendor3, + sizeof(efi_guid_t)); + boottime->copy_mem(dp3 + 2 * sizeof(struct efi_device_path_vendor), + &vendor_node, sizeof(struct efi_device_path_vendor)); + + end_node.type = DEVICE_PATH_TYPE_END; + end_node.sub_type = DEVICE_PATH_SUB_TYPE_END; + end_node.length = sizeof(struct efi_device_path); + boottime->copy_mem(dp1 + sizeof(struct efi_device_path_vendor), + &end_node, sizeof(struct efi_device_path)); + boottime->copy_mem(dp2 + 2 * sizeof(struct efi_device_path_vendor), + &end_node, sizeof(struct efi_device_path)); + boottime->copy_mem(dp3 + 3 * sizeof(struct efi_device_path_vendor), + &end_node, sizeof(struct efi_device_path)); + + ret = boottime->install_protocol_interface(&handle1, + &guid_device_path, + EFI_NATIVE_INTERFACE, + dp1); + if (ret != EFI_SUCCESS) { + efi_st_error("InstallProtocolInterface failed\n"); + return EFI_ST_FAILURE; + } + ret = boottime->install_protocol_interface(&handle1, + &guid_protocol, + EFI_NATIVE_INTERFACE, + &interface); + if (ret != EFI_SUCCESS) { + efi_st_error("InstallProtocolInterface failed\n"); + return EFI_ST_FAILURE; + } + ret = boottime->install_protocol_interface(&handle2, + &guid_device_path, + EFI_NATIVE_INTERFACE, + dp2); + if (ret != EFI_SUCCESS) { + efi_st_error("InstallProtocolInterface failed\n"); + return EFI_ST_FAILURE; + } + ret = boottime->install_protocol_interface(&handle2, + &guid_protocol, + EFI_NATIVE_INTERFACE, + &interface); + if (ret != EFI_SUCCESS) { + efi_st_error("InstallProtocolInterface failed\n"); + return EFI_ST_FAILURE; + } + ret = boottime->install_protocol_interface(&handle3, + &guid_device_path, + EFI_NATIVE_INTERFACE, + dp3); + if (ret != EFI_SUCCESS) { + efi_st_error("InstallProtocolInterface failed\n"); + return EFI_ST_FAILURE; + } + return EFI_ST_SUCCESS; + +out_of_memory: + efi_st_error("Out of memory\n"); + return EFI_ST_FAILURE; +} + +/* + * Tear down unit test. + * + */ +static int teardown(void) +{ + efi_status_t ret; + + ret = boottime->uninstall_protocol_interface(&handle1, + &guid_device_path, + dp1); + if (ret != EFI_SUCCESS) + efi_st_todo("UninstallProtocolInterface failed\n"); + ret = boottime->uninstall_protocol_interface(&handle1, + &guid_protocol, + &interface); + if (ret != EFI_SUCCESS) + efi_st_todo("UninstallProtocolInterface failed\n"); + ret = boottime->uninstall_protocol_interface(&handle2, + &guid_device_path, + dp2); + if (ret != EFI_SUCCESS) + efi_st_todo("UninstallProtocolInterface failed\n"); + ret = boottime->uninstall_protocol_interface(&handle2, + &guid_protocol, + &interface); + if (ret != EFI_SUCCESS) + efi_st_todo("UninstallProtocolInterface failed\n"); + ret = boottime->uninstall_protocol_interface(&handle3, + &guid_device_path, + dp3); + if (ret != EFI_SUCCESS) + efi_st_todo("UninstallProtocolInterface failed\n"); + if (dp1) { + ret = boottime->free_pool(dp1); + if (ret != EFI_SUCCESS) { + efi_st_error("FreePool failed\n"); + return EFI_ST_FAILURE; + } + } + if (dp2) { + ret = boottime->free_pool(dp2); + if (ret != EFI_SUCCESS) { + efi_st_error("FreePool failed\n"); + return EFI_ST_FAILURE; + } + } + if (dp3) { + ret = boottime->free_pool(dp3); + if (ret != EFI_SUCCESS) { + efi_st_error("FreePool failed\n"); + return EFI_ST_FAILURE; + } + } + return EFI_ST_SUCCESS; +} + +/* + * Execute unit test. + * + */ +static int execute(void) +{ + struct efi_device_path *remaining_dp; + void *handle; + /* + * This device path node ends with the letter 't' of 'u-boot'. + * The following '.bin' does not belong to the node but is + * helps to test the correct truncation. + */ + struct { + struct efi_device_path dp; + u16 text[12]; + } __packed dp_node = { + { DEVICE_PATH_TYPE_MEDIA_DEVICE, + DEVICE_PATH_SUB_TYPE_FILE_PATH, + sizeof(struct efi_device_path) + 12}, + L"u-boot.bin", + }; + u16 *string; + efi_status_t ret; + efi_uintn_t i, no_handles; + efi_handle_t *handles; + struct efi_device_path *dp; + + /* Display all available device paths */ + ret = boottime->locate_handle_buffer(BY_PROTOCOL, + &guid_device_path, + NULL, &no_handles, &handles); + if (ret != EFI_SUCCESS) { + efi_st_error("Cannot retrieve device path protocols.\n"); + return EFI_ST_FAILURE; + } + + efi_st_printf("Installed device path protocols:\n"); + for (i = 0; i < no_handles; ++i) { + ret = boottime->open_protocol(handles[i], &guid_device_path, + (void **)&dp, NULL, NULL, + EFI_OPEN_PROTOCOL_GET_PROTOCOL); + if (ret != EFI_SUCCESS) { + efi_st_error("Cannot open device path protocol.\n"); + return EFI_ST_FAILURE; + } + string = device_path_to_text->convert_device_path_to_text( + dp, true, false); + if (!string) { + efi_st_error("ConvertDevicePathToText failed\n"); + return EFI_ST_FAILURE; + } + efi_st_printf("%ps\n", string); + ret = boottime->free_pool(string); + if (ret != EFI_SUCCESS) { + efi_st_error("FreePool failed\n"); + return EFI_ST_FAILURE; + } + ret = boottime->close_protocol(handles[i], &guid_device_path, + NULL, NULL); + if (ret != EFI_SUCCESS) + efi_st_todo("Cannot close device path protocol.\n"); + } + ret = boottime->free_pool(handles); + if (ret != EFI_SUCCESS) { + efi_st_error("FreePool failed\n"); + return EFI_ST_FAILURE; + } + efi_st_printf("\n"); + + /* Test ConvertDevicePathToText */ + string = device_path_to_text->convert_device_path_to_text( + (struct efi_device_path *)dp2, true, false); + if (!string) { + efi_st_error("ConvertDevicePathToText failed\n"); + return EFI_ST_FAILURE; + } + efi_st_printf("dp2: %ps\n", string); + if (efi_st_strcmp_16_8( + string, + "/VenHw(dbca4c98-6cb0-694d-0872-819c650cbbb1)/VenHw(dbca4c98-6cb0-694d-0872-819c650cbba2)") + ) { + efi_st_error("Incorrect text from ConvertDevicePathToText\n"); + return EFI_ST_FAILURE; + } + + ret = boottime->free_pool(string); + if (ret != EFI_SUCCESS) { + efi_st_error("FreePool failed\n"); + return EFI_ST_FAILURE; + } + + /* Test ConvertDeviceNodeToText */ + string = device_path_to_text->convert_device_node_to_text( + (struct efi_device_path *)&dp_node, true, false); + if (!string) { + efi_st_error("ConvertDeviceNodeToText failed\n"); + return EFI_ST_FAILURE; + } + efi_st_printf("dp_node: %ps\n", string); + ret = boottime->free_pool(string); + if (ret != EFI_SUCCESS) { + efi_st_error("FreePool failed\n"); + return EFI_ST_FAILURE; + } + if (efi_st_strcmp_16_8(string, "u-boot")) { + efi_st_error( + "Incorrect conversion by ConvertDeviceNodeToText\n"); + return EFI_ST_FAILURE; + } + + /* Test LocateDevicePath */ + remaining_dp = (struct efi_device_path *)dp3; + ret = boottime->locate_device_path(&guid_protocol, &remaining_dp, + &handle); + if (ret != EFI_SUCCESS) { + efi_st_error("LocateDevicePath failed\n"); + return EFI_ST_FAILURE; + } + if (handle != handle2) { + efi_st_error("LocateDevicePath returned wrong handle\n"); + return EFI_ST_FAILURE; + } + string = device_path_to_text->convert_device_path_to_text(remaining_dp, + true, false); + if (!string) { + efi_st_error("ConvertDevicePathToText failed\n"); + return EFI_ST_FAILURE; + } + efi_st_printf("remaining device path: %ps\n", string); + if (efi_st_strcmp_16_8(string, + "/VenHw(dbca4c98-6cb0-694d-0872-819c650cbbc3)") + ) { + efi_st_error("LocateDevicePath: wrong remaining device path\n"); + return EFI_ST_FAILURE; + } + + return EFI_ST_SUCCESS; +} + +EFI_UNIT_TEST(devicepath) = { + .name = "device path", + .phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT, + .setup = setup, + .execute = execute, + .teardown = teardown, +};

Use efi_add_protocol to install protocols.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 no change v2 no change --- lib/efi_loader/efi_disk.c | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index c6f0d732c1..1d6cf3122f 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -213,33 +213,40 @@ static void efi_disk_add_dev(const char *name, unsigned int part) { struct efi_disk_obj *diskobj; + efi_status_t ret;
/* Don't add empty devices */ if (!desc->lba) return;
diskobj = calloc(1, sizeof(*diskobj)); - if (!diskobj) { - printf("ERROR: Out of memory\n"); - return; - } + if (!diskobj) + goto out_of_memory; + + /* Hook up to the device list */ + list_add_tail(&diskobj->parent.link, &efi_obj_list);
/* Fill in object data */ diskobj->dp = efi_dp_from_part(desc, part); diskobj->part = part; - diskobj->parent.protocols[0].guid = &efi_block_io_guid; - diskobj->parent.protocols[0].protocol_interface = &diskobj->ops; - diskobj->parent.protocols[1].guid = &efi_guid_device_path; - diskobj->parent.protocols[1].protocol_interface = diskobj->dp; + diskobj->parent.handle = diskobj; + ret = efi_add_protocol(diskobj->parent.handle, &efi_block_io_guid, + &diskobj->ops); + if (ret != EFI_SUCCESS) + goto out_of_memory; + ret = efi_add_protocol(diskobj->parent.handle, &efi_guid_device_path, + diskobj->dp); + if (ret != EFI_SUCCESS) + goto out_of_memory; if (part >= 1) { diskobj->volume = efi_simple_file_system(desc, part, diskobj->dp); - diskobj->parent.protocols[2].guid = - &efi_simple_file_system_protocol_guid; - diskobj->parent.protocols[2].protocol_interface = - diskobj->volume; + ret = efi_add_protocol(diskobj->parent.handle, + &efi_simple_file_system_protocol_guid, + &diskobj->volume); + if (ret != EFI_SUCCESS) + goto out_of_memory; } - diskobj->parent.handle = diskobj; diskobj->ops = block_io_disk_template; diskobj->ifname = if_typename; diskobj->dev_index = dev_index; @@ -253,9 +260,9 @@ static void efi_disk_add_dev(const char *name, diskobj->media.io_align = desc->blksz; diskobj->media.last_block = desc->lba - offset; diskobj->ops.media = &diskobj->media; - - /* Hook up to the device list */ - list_add_tail(&diskobj->parent.link, &efi_obj_list); + return; +out_of_memory: + printf("ERROR: Out of memory\n"); }
static int efi_disk_create_eltorito(struct blk_desc *desc,

Use efi_add_protocol to add protocols.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 no change v2 no change --- lib/efi_loader/efi_net.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c index a7b101e830..8b2f682351 100644 --- a/lib/efi_loader/efi_net.c +++ b/lib/efi_loader/efi_net.c @@ -292,20 +292,26 @@ int efi_net_register(void)
/* We only expose the "active" eth device, so one is enough */ netobj = calloc(1, sizeof(*netobj)); - if (!netobj) { - printf("ERROR: Out of memory\n"); - return 1; - } + if (!netobj) + goto out_of_memory; + + /* Hook net up to the device list */ + list_add_tail(&netobj->parent.link, &efi_obj_list);
/* Fill in object data */ - netobj->parent.protocols[0].guid = &efi_net_guid; - netobj->parent.protocols[0].protocol_interface = &netobj->net; - netobj->parent.protocols[1].guid = &efi_guid_device_path; - netobj->parent.protocols[1].protocol_interface = - efi_dp_from_eth(); - netobj->parent.protocols[2].guid = &efi_pxe_guid; - netobj->parent.protocols[2].protocol_interface = &netobj->pxe; netobj->parent.handle = &netobj->net; + r = efi_add_protocol(netobj->parent.handle, &efi_net_guid, + &netobj->net); + if (r != EFI_SUCCESS) + goto out_of_memory; + r = efi_add_protocol(netobj->parent.handle, &efi_guid_device_path, + efi_dp_from_eth()); + if (r != EFI_SUCCESS) + goto out_of_memory; + r = efi_add_protocol(netobj->parent.handle, &efi_pxe_guid, + &netobj->pxe); + if (r != EFI_SUCCESS) + goto out_of_memory; netobj->net.revision = EFI_SIMPLE_NETWORK_PROTOCOL_REVISION; netobj->net.start = efi_net_start; netobj->net.stop = efi_net_stop; @@ -330,9 +336,6 @@ int efi_net_register(void) if (dhcp_ack) netobj->pxe_mode.dhcp_ack = *dhcp_ack;
- /* Hook net up to the device list */ - list_add_tail(&netobj->parent.link, &efi_obj_list); - /* * Create WaitForPacket event. */ @@ -365,4 +368,7 @@ int efi_net_register(void) }
return 0; +out_of_memory: + printf("ERROR: Out of memory\n"); + return 1; }

Use efi_add_protocol to add protocol.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 no change v2 no change --- lib/efi_loader/efi_gop.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c index 7370eeee37..7b74d6ef33 100644 --- a/lib/efi_loader/efi_gop.c +++ b/lib/efi_loader/efi_gop.c @@ -132,6 +132,7 @@ int efi_gop_register(void) u32 bpix, col, row; u64 fb_base, fb_size; void *fb; + efi_status_t ret;
#ifdef CONFIG_DM_VIDEO struct udevice *vdev; @@ -178,10 +179,17 @@ int efi_gop_register(void) return 1; }
+ /* Hook up to the device list */ + list_add_tail(&gopobj->parent.link, &efi_obj_list); + /* Fill in object data */ - gopobj->parent.protocols[0].guid = &efi_gop_guid; - gopobj->parent.protocols[0].protocol_interface = &gopobj->ops; gopobj->parent.handle = &gopobj->ops; + ret = efi_add_protocol(gopobj->parent.handle, &efi_gop_guid, + &gopobj->ops); + if (ret != EFI_SUCCESS) { + printf("ERROR: Out of memory\n"); + return 1; + } gopobj->ops.query_mode = gop_query_mode; gopobj->ops.set_mode = gop_set_mode; gopobj->ops.blt = gop_blt; @@ -210,8 +218,5 @@ int efi_gop_register(void) gopobj->bpix = bpix; gopobj->fb = fb;
- /* Hook up to the device list */ - list_add_tail(&gopobj->parent.link, &efi_obj_list); - return 0; }

Use function efi_search_protocol.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 no change v2 no change --- lib/efi_loader/efi_boottime.c | 36 ++++++------------------------------ 1 file changed, 6 insertions(+), 30 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 565addfc3f..4cc5b1e620 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -2051,8 +2051,7 @@ static efi_status_t EFIAPI efi_open_protocol( void **protocol_interface, void *agent_handle, void *controller_handle, uint32_t attributes) { - struct list_head *lhandle; - int i; + struct efi_handler *handler; efi_status_t r = EFI_INVALID_PARAMETER;
EFI_ENTRY("%p, %pUl, %p, %p, %p, 0x%x", handle, protocol, @@ -2065,8 +2064,6 @@ static efi_status_t EFIAPI efi_open_protocol( goto out; }
- EFI_PRINT_GUID("protocol", protocol); - switch (attributes) { case EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL: case EFI_OPEN_PROTOCOL_GET_PROTOCOL: @@ -2087,33 +2084,12 @@ static efi_status_t EFIAPI efi_open_protocol( goto out; }
- list_for_each(lhandle, &efi_obj_list) { - struct efi_object *efiobj; - efiobj = list_entry(lhandle, struct efi_object, link); - - if (efiobj->handle != handle) - continue; - - for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) { - struct efi_handler *handler = &efiobj->protocols[i]; - const efi_guid_t *hprotocol = handler->guid; - if (!hprotocol) - continue; - if (!guidcmp(hprotocol, protocol)) { - if (attributes != - EFI_OPEN_PROTOCOL_TEST_PROTOCOL) { - *protocol_interface = - handler->protocol_interface; - } - r = EFI_SUCCESS; - goto out; - } - } - goto unsupported; - } + r = efi_search_protocol(handle, protocol, &handler); + if (r != EFI_SUCCESS) + goto out;
-unsupported: - r = EFI_UNSUPPORTED; + if (attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) + *protocol_interface = handler->protocol_interface; out: return EFI_EXIT(r); }

Use function efi_search_protocol().
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 fix typo in commit message v2 no change --- lib/efi_loader/efi_device_path.c | 43 ++++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 24 deletions(-)
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 9027ae8efb..b4e2f933cb 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -128,32 +128,27 @@ static struct efi_object *find_obj(struct efi_device_path *dp, bool short_path, struct efi_object *efiobj;
list_for_each_entry(efiobj, &efi_obj_list, link) { - int i; - - for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) { - struct efi_handler *handler = &efiobj->protocols[i]; - struct efi_device_path *obj_dp; - - if (!handler->guid) - break; - - if (guidcmp(handler->guid, &efi_guid_device_path)) - continue; - - obj_dp = handler->protocol_interface; - - do { - if (efi_dp_match(dp, obj_dp) == 0) { - if (rem) { - *rem = ((void *)dp) + - efi_dp_size(obj_dp); - } - return efiobj; + struct efi_handler *handler; + struct efi_device_path *obj_dp; + efi_status_t ret; + + ret = efi_search_protocol(efiobj->handle, + &efi_guid_device_path, &handler); + if (ret != EFI_SUCCESS) + continue; + obj_dp = handler->protocol_interface; + + do { + if (efi_dp_match(dp, obj_dp) == 0) { + if (rem) { + *rem = ((void *)dp) + + efi_dp_size(obj_dp); } + return efiobj; + }
- obj_dp = shorten_path(efi_dp_next(obj_dp)); - } while (short_path && obj_dp); - } + obj_dp = shorten_path(efi_dp_next(obj_dp)); + } while (short_path && obj_dp); }
return NULL;

Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 no change v2 no change --- include/efi_loader.h | 6 ++- lib/efi_loader/efi_boottime.c | 107 ++++++++++++++++++++---------------------- lib/efi_loader/efi_disk.c | 1 + lib/efi_loader/efi_gop.c | 1 + lib/efi_loader/efi_net.c | 1 + 5 files changed, 58 insertions(+), 58 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index e1f0af3496..a73bbc1269 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -101,6 +101,8 @@ extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop; * interface (usually a struct with callback functions), this struct maps the * protocol GUID to the respective protocol interface */ struct efi_handler { + /* Link to the list of protocols of a handle */ + struct list_head link; const efi_guid_t *guid; void *protocol_interface; }; @@ -115,8 +117,8 @@ struct efi_handler { struct efi_object { /* Every UEFI object is part of a global object list */ struct list_head link; - /* We support up to 16 "protocols" an object can be accessed through */ - struct efi_handler protocols[16]; + /* The list of protocols */ + struct list_head protocols; /* The object spawner can either use this for data or as identifier */ void *handle; }; diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 4cc5b1e620..54e9c27db1 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -339,6 +339,7 @@ efi_status_t efi_create_handle(void **handle) return r; memset(obj, 0, sizeof(struct efi_object)); obj->handle = obj; + INIT_LIST_HEAD(&obj->protocols); list_add_tail(&obj->link, &efi_obj_list); *handle = obj; return r; @@ -715,18 +716,17 @@ efi_status_t efi_search_protocol(const void *handle, struct efi_handler **handler) { struct efi_object *efiobj; - size_t i; - struct efi_handler *protocol; + struct list_head *lhandle;
if (!handle || !protocol_guid) return EFI_INVALID_PARAMETER; efiobj = efi_search_obj(handle); if (!efiobj) return EFI_INVALID_PARAMETER; - for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) { - protocol = &efiobj->protocols[i]; - if (!protocol->guid) - continue; + list_for_each(lhandle, &efiobj->protocols) { + struct efi_handler *protocol; + + protocol = list_entry(lhandle, struct efi_handler, link); if (!guidcmp(protocol->guid, protocol_guid)) { if (handler) *handler = protocol; @@ -750,7 +750,6 @@ efi_status_t efi_add_protocol(const void *handle, const efi_guid_t *protocol, struct efi_object *efiobj; struct efi_handler *handler; efi_status_t ret; - size_t i;
efiobj = efi_search_obj(handle); if (!efiobj) @@ -761,16 +760,10 @@ efi_status_t efi_add_protocol(const void *handle, const efi_guid_t *protocol, handler = calloc(1, sizeof(struct efi_handler)); if (!handler) return EFI_OUT_OF_RESOURCES; - /* Install protocol in first empty slot. */ - for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) { - handler = &efiobj->protocols[i]; - if (handler->guid) - continue; - handler->guid = protocol; - handler->protocol_interface = protocol_interface; - return EFI_SUCCESS; - } - return EFI_OUT_OF_RESOURCES; + handler->guid = protocol; + handler->protocol_interface = protocol_interface; + list_add_tail(&handler->link, &efiobj->protocols); + return EFI_SUCCESS; }
/* @@ -790,10 +783,10 @@ efi_status_t efi_remove_protocol(const void *handle, const efi_guid_t *protocol, ret = efi_search_protocol(handle, protocol, &handler); if (ret != EFI_SUCCESS) return ret; - if (handler->protocol_interface != protocol_interface) - return EFI_NOT_FOUND; - handler->guid = NULL; - handler->protocol_interface = NULL; + if (guidcmp(handler->guid, protocol)) + return EFI_INVALID_PARAMETER; + list_del(&handler->link); + free(handler); return EFI_SUCCESS; }
@@ -806,17 +799,22 @@ efi_status_t efi_remove_protocol(const void *handle, const efi_guid_t *protocol, efi_status_t efi_remove_all_protocols(const void *handle) { struct efi_object *efiobj; - struct efi_handler *handler; - size_t i; + struct list_head *lhandle; + struct list_head *pos;
efiobj = efi_search_obj(handle); if (!efiobj) return EFI_INVALID_PARAMETER; + list_for_each_safe(lhandle, pos, &efiobj->protocols) { + struct efi_handler *protocol; + efi_status_t ret;
- for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) { - handler = &efiobj->protocols[i]; - handler->guid = NULL; - handler->protocol_interface = NULL; + protocol = list_entry(lhandle, struct efi_handler, link); + + ret = efi_remove_protocol(handle, protocol->guid, + protocol->protocol_interface); + if (ret != EFI_SUCCESS) + return ret; } return EFI_SUCCESS; } @@ -1171,6 +1169,7 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob if (device_path) info->device_handle = efi_dp_find_obj(device_path, NULL);
+ INIT_LIST_HEAD(&obj->protocols); list_add_tail(&obj->link, &efi_obj_list); /* * When asking for the device path interface, return @@ -1648,8 +1647,7 @@ static efi_status_t EFIAPI efi_protocols_per_handle(void *handle, { unsigned long buffer_size; struct efi_object *efiobj; - unsigned long i, j; - struct list_head *lhandle; + struct list_head *protocol_handle; efi_status_t r;
EFI_ENTRY("%p, %p, %p", handle, protocol_buffer, @@ -1660,36 +1658,33 @@ static efi_status_t EFIAPI efi_protocols_per_handle(void *handle,
*protocol_buffer = NULL; *protocol_buffer_count = 0; - list_for_each(lhandle, &efi_obj_list) { - efiobj = list_entry(lhandle, struct efi_object, link);
- if (efiobj->handle != handle) - continue; + efiobj = efi_search_obj(handle); + if (!efiobj) + return EFI_EXIT(EFI_INVALID_PARAMETER);
- /* Count protocols */ - for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) { - if (efiobj->protocols[i].guid) - ++*protocol_buffer_count; - } - /* Copy guids */ - if (*protocol_buffer_count) { - buffer_size = sizeof(efi_guid_t *) * - *protocol_buffer_count; - r = efi_allocate_pool(EFI_ALLOCATE_ANY_PAGES, - buffer_size, - (void **)protocol_buffer); - if (r != EFI_SUCCESS) - return EFI_EXIT(r); - j = 0; - for (i = 0; i < ARRAY_SIZE(efiobj->protocols); ++i) { - if (efiobj->protocols[i].guid) { - (*protocol_buffer)[j] = (void *) - efiobj->protocols[i].guid; - ++j; - } - } + /* Count protocols */ + list_for_each(protocol_handle, &efiobj->protocols) { + ++*protocol_buffer_count; + } + + /* Copy guids */ + if (*protocol_buffer_count) { + size_t j = 0; + + buffer_size = sizeof(efi_guid_t *) * *protocol_buffer_count; + r = efi_allocate_pool(EFI_ALLOCATE_ANY_PAGES, buffer_size, + (void **)protocol_buffer); + if (r != EFI_SUCCESS) + return EFI_EXIT(r); + list_for_each(protocol_handle, &efiobj->protocols) { + struct efi_handler *protocol; + + protocol = list_entry(protocol_handle, + struct efi_handler, link); + (*protocol_buffer)[j] = (void *)protocol->guid; + ++j; } - break; }
return EFI_EXIT(EFI_SUCCESS); diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 1d6cf3122f..af8db40e19 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -224,6 +224,7 @@ static void efi_disk_add_dev(const char *name, goto out_of_memory;
/* Hook up to the device list */ + INIT_LIST_HEAD(&diskobj->parent.protocols); list_add_tail(&diskobj->parent.link, &efi_obj_list);
/* Fill in object data */ diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c index 7b74d6ef33..498184d754 100644 --- a/lib/efi_loader/efi_gop.c +++ b/lib/efi_loader/efi_gop.c @@ -180,6 +180,7 @@ int efi_gop_register(void) }
/* Hook up to the device list */ + INIT_LIST_HEAD(&gopobj->parent.protocols); list_add_tail(&gopobj->parent.link, &efi_obj_list);
/* Fill in object data */ diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c index 8b2f682351..74a67c5365 100644 --- a/lib/efi_loader/efi_net.c +++ b/lib/efi_loader/efi_net.c @@ -296,6 +296,7 @@ int efi_net_register(void) goto out_of_memory;
/* Hook net up to the device list */ + INIT_LIST_HEAD(&netobj->parent.protocols); list_add_tail(&netobj->parent.link, &efi_obj_list);
/* Fill in object data */

As the selftest is not compiled as an EFI binary we do not need special compiler flags.
This avoids the checkarmreloc error on vexpress_ca15_tc2.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 no change v2 no change --- lib/efi_selftest/Makefile | 25 ------------------------- 1 file changed, 25 deletions(-)
diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile index d280eca5c3..837e86228e 100644 --- a/lib/efi_selftest/Makefile +++ b/lib/efi_selftest/Makefile @@ -7,31 +7,6 @@ # This file only gets included with CONFIG_EFI_LOADER set, so all # object inclusion implicitly depends on it
-CFLAGS_efi_selftest.o := $(CFLAGS_EFI) -CFLAGS_REMOVE_efi_selftest.o := $(CFLAGS_NON_EFI) -CFLAGS_efi_selftest_console.o := $(CFLAGS_EFI) -CFLAGS_REMOVE_efi_selftest_console.o := $(CFLAGS_NON_EFI) -CFLAGS_efi_selftest_devicepathe.o := $(CFLAGS_EFI) -CFLAGS_REMOVE_efi_selftest_devicepath.o := $(CFLAGS_NON_EFI) -CFLAGS_efi_selftest_events.o := $(CFLAGS_EFI) -CFLAGS_REMOVE_efi_selftest_events.o := $(CFLAGS_NON_EFI) -CFLAGS_efi_selftest_exitbootservices.o := $(CFLAGS_EFI) -CFLAGS_REMOVE_efi_selftest_exitbootservices.o := $(CFLAGS_NON_EFI) -CFLAGS_efi_selftest_gop.o := $(CFLAGS_EFI) -CFLAGS_REMOVE_efi_selftest_gop.o := $(CFLAGS_NON_EFI) -CFLAGS_efi_selftest_manageprotocols.o := $(CFLAGS_EFI) -CFLAGS_REMOVE_efi_selftest_manageprotocols.o := $(CFLAGS_NON_EFI) -CFLAGS_efi_selftest_snp.o := $(CFLAGS_EFI) -CFLAGS_REMOVE_efi_selftest_snp.o := $(CFLAGS_NON_EFI) -CFLAGS_efi_selftest_textoutput.o := $(CFLAGS_EFI) -CFLAGS_REVMOE_efi_selftest_textoutput.o := $(CFLAGS_NON_EFI) -CFLAGS_efi_selftest_tpl.o := $(CFLAGS_EFI) -CFLAGS_REMOVE_efi_selftest_tpl.o := $(CFLAGS_NON_EFI) -CFLAGS_efi_selftest_util.o := $(CFLAGS_EFI) -CFLAGS_REMOVE_efi_selftest_util.o := $(CFLAGS_NON_EFI) -CFLAGS_efi_selftest_watchdog.o := $(CFLAGS_EFI) -CFLAGS_REMOVE_efi_selftest_watchdog.o := $(CFLAGS_NON_EFI) - obj-$(CONFIG_CMD_BOOTEFI_SELFTEST) += \ efi_selftest.o \ efi_selftest_console.o \

Add a missing line feed for an error message.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 no change v2 new patch --- lib/efi_selftest/efi_selftest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_selftest/efi_selftest.c b/lib/efi_selftest/efi_selftest.c index 263b53f672..4e5a12c47c 100644 --- a/lib/efi_selftest/efi_selftest.c +++ b/lib/efi_selftest/efi_selftest.c @@ -239,7 +239,7 @@ efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle, ret = boottime->handle_protocol(image_handle, &efi_guid_loaded_image, (void **)&loaded_image); if (ret != EFI_SUCCESS) { - efi_st_error("Cannot open loaded image protocol"); + efi_st_error("Cannot open loaded image protocol\n"); return ret; }

We need to test if we pass a valid image handle when loading and EFI application. This cannot be done in efi_selftest as it is not loaded as an image.
So let's enhance helloworld a bit.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 no change v2 new patch --- lib/efi_loader/helloworld.c | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-)
diff --git a/lib/efi_loader/helloworld.c b/lib/efi_loader/helloworld.c index 77130a36dd..e59c24c788 100644 --- a/lib/efi_loader/helloworld.c +++ b/lib/efi_loader/helloworld.c @@ -5,19 +5,52 @@ * Written by Simon Glass sjg@chromium.org * * SPDX-License-Identifier: GPL-2.0+ + * + * This program demonstrates calling a boottime service. + * It writes a greeting and the load options to the console. */
#include <common.h> #include <efi_api.h>
+/* + * Entry point of the EFI application. + * + * @handle handle of the loaded image + * @systable system table + * @return status code + */ efi_status_t EFIAPI efi_main(efi_handle_t handle, struct efi_system_table *systable) { struct efi_simple_text_output_protocol *con_out = systable->con_out; struct efi_boot_services *boottime = systable->boottime; + struct efi_loaded_image *loaded_image; + const efi_guid_t loaded_image_guid = LOADED_IMAGE_GUID; + efi_status_t ret;
con_out->output_string(con_out, L"Hello, world!\n"); - boottime->exit(handle, 0, 0, NULL);
- return EFI_SUCCESS; + /* Get the loaded image protocol */ + ret = boottime->handle_protocol(handle, &loaded_image_guid, + (void **)&loaded_image); + if (ret != EFI_SUCCESS) { + con_out->output_string(con_out, + L"Cannot open loaded image protocol\n"); + goto out; + } + /* Output the load options */ + con_out->output_string(con_out, L"Load options: "); + if (loaded_image->load_options_size && loaded_image->load_options) + con_out->output_string(con_out, + (u16 *)loaded_image->load_options); + else + con_out->output_string(con_out, L"<none>"); + con_out->output_string(con_out, L"\n"); + +out: + boottime->exit(handle, ret, 0, NULL); + + /* We should never arrive here */ + return ret; }

On 11/26/2017 05:05 AM, Heinrich Schuchardt wrote:
We need to test if we pass a valid image handle when loading and EFI application. This cannot be done in efi_selftest as it is not loaded as an image.
So let's enhance helloworld a bit.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
This particular patch/commit bbf75dd9345d0b1a7ec7a50016547eb7c759b7af ("efi_loader: output load options in helloworld") was bisected and causes the Lamobo_R1_defconfig build using the toolchain at [1] to fail with:
/home/fainelli/work/toolchains/stbgcc-6.3-1.1/bin/arm-linux-ld.bfd: error: required section '.got' not found in the linker script /home/fainelli/work/toolchains/stbgcc-6.3-1.1/bin/arm-linux-ld.bfd: final link failed: Invalid operation scripts/Makefile.lib:409: recipe for target 'lib/efi_loader/helloworld_efi.so' failed make[2]: *** [lib/efi_loader/helloworld_efi.so] Error 1 rm lib/efi_loader/helloworld.o scripts/Makefile.build:425: recipe for target 'lib/efi_loader' failed make[1]: *** [lib/efi_loader] Error 2
[1]: https://github.com/Broadcom/stbgcc-6.3/releases/tag/stbgcc-6.3-1.1
v3 no change v2 new patch
lib/efi_loader/helloworld.c | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-)
diff --git a/lib/efi_loader/helloworld.c b/lib/efi_loader/helloworld.c index 77130a36dd..e59c24c788 100644 --- a/lib/efi_loader/helloworld.c +++ b/lib/efi_loader/helloworld.c @@ -5,19 +5,52 @@
- Written by Simon Glass sjg@chromium.org
- SPDX-License-Identifier: GPL-2.0+
- This program demonstrates calling a boottime service.
*/
- It writes a greeting and the load options to the console.
#include <common.h> #include <efi_api.h>
+/*
- Entry point of the EFI application.
- @handle handle of the loaded image
- @systable system table
- @return status code
- */
efi_status_t EFIAPI efi_main(efi_handle_t handle, struct efi_system_table *systable) { struct efi_simple_text_output_protocol *con_out = systable->con_out; struct efi_boot_services *boottime = systable->boottime;
struct efi_loaded_image *loaded_image;
const efi_guid_t loaded_image_guid = LOADED_IMAGE_GUID;
efi_status_t ret;
con_out->output_string(con_out, L"Hello, world!\n");
boottime->exit(handle, 0, 0, NULL);
return EFI_SUCCESS;
- /* Get the loaded image protocol */
- ret = boottime->handle_protocol(handle, &loaded_image_guid,
(void **)&loaded_image);
- if (ret != EFI_SUCCESS) {
con_out->output_string(con_out,
L"Cannot open loaded image protocol\n");
goto out;
- }
- /* Output the load options */
- con_out->output_string(con_out, L"Load options: ");
- if (loaded_image->load_options_size && loaded_image->load_options)
con_out->output_string(con_out,
(u16 *)loaded_image->load_options);
- else
con_out->output_string(con_out, L"<none>");
- con_out->output_string(con_out, L"\n");
+out:
- boottime->exit(handle, ret, 0, NULL);
- /* We should never arrive here */
- return ret;
}

On 12/10/2017 12:09 AM, Florian Fainelli wrote:
On 11/26/2017 05:05 AM, Heinrich Schuchardt wrote:
We need to test if we pass a valid image handle when loading and EFI application. This cannot be done in efi_selftest as it is not loaded as an image.
So let's enhance helloworld a bit.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
This particular patch/commit bbf75dd9345d0b1a7ec7a50016547eb7c759b7af ("efi_loader: output load options in helloworld") was bisected and causes the Lamobo_R1_defconfig build using the toolchain at [1] to fail with:
/home/fainelli/work/toolchains/stbgcc-6.3-1.1/bin/arm-linux-ld.bfd: error: required section '.got' not found in the linker script /home/fainelli/work/toolchains/stbgcc-6.3-1.1/bin/arm-linux-ld.bfd: final link failed: Invalid operation scripts/Makefile.lib:409: recipe for target 'lib/efi_loader/helloworld_efi.so' failed make[2]: *** [lib/efi_loader/helloworld_efi.so] Error 1 rm lib/efi_loader/helloworld.o scripts/Makefile.build:425: recipe for target 'lib/efi_loader' failed make[1]: *** [lib/efi_loader] Error 2
Isn't this fixed by http://git.denx.de/?p=u-boot.git;a=commit;h=3bb74f9800cdc4cf10a87f2725242c25... "efi_loader helloworld.efi: Fix building with -Os" ?
Best Regards
Heinrich
v3 no change v2 new patch
lib/efi_loader/helloworld.c | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-)
diff --git a/lib/efi_loader/helloworld.c b/lib/efi_loader/helloworld.c index 77130a36dd..e59c24c788 100644 --- a/lib/efi_loader/helloworld.c +++ b/lib/efi_loader/helloworld.c @@ -5,19 +5,52 @@
- Written by Simon Glass sjg@chromium.org
- SPDX-License-Identifier: GPL-2.0+
- This program demonstrates calling a boottime service.
- It writes a greeting and the load options to the console.
*/
#include <common.h> #include <efi_api.h>
+/*
- Entry point of the EFI application.
- @handle handle of the loaded image
- @systable system table
- @return status code
*/ efi_status_t EFIAPI efi_main(efi_handle_t handle, struct efi_system_table *systable) { struct efi_simple_text_output_protocol *con_out = systable->con_out; struct efi_boot_services *boottime = systable->boottime;
struct efi_loaded_image *loaded_image;
const efi_guid_t loaded_image_guid = LOADED_IMAGE_GUID;
efi_status_t ret;
con_out->output_string(con_out, L"Hello, world!\n");
boottime->exit(handle, 0, 0, NULL);
return EFI_SUCCESS;
- /* Get the loaded image protocol */
- ret = boottime->handle_protocol(handle, &loaded_image_guid,
(void **)&loaded_image);
- if (ret != EFI_SUCCESS) {
con_out->output_string(con_out,
L"Cannot open loaded image protocol\n");
goto out;
- }
- /* Output the load options */
- con_out->output_string(con_out, L"Load options: ");
- if (loaded_image->load_options_size && loaded_image->load_options)
con_out->output_string(con_out,
(u16 *)loaded_image->load_options);
- else
con_out->output_string(con_out, L"<none>");
- con_out->output_string(con_out, L"\n");
+out:
- boottime->exit(handle, ret, 0, NULL);
- /* We should never arrive here */
- return ret; }

On 12/09/2017 10:40 PM, Heinrich Schuchardt wrote:
On 12/10/2017 12:09 AM, Florian Fainelli wrote:
On 11/26/2017 05:05 AM, Heinrich Schuchardt wrote:
We need to test if we pass a valid image handle when loading and EFI application. This cannot be done in efi_selftest as it is not loaded as an image.
So let's enhance helloworld a bit.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
This particular patch/commit bbf75dd9345d0b1a7ec7a50016547eb7c759b7af ("efi_loader: output load options in helloworld") was bisected and causes the Lamobo_R1_defconfig build using the toolchain at [1] to fail with:
/home/fainelli/work/toolchains/stbgcc-6.3-1.1/bin/arm-linux-ld.bfd: error: required section '.got' not found in the linker script /home/fainelli/work/toolchains/stbgcc-6.3-1.1/bin/arm-linux-ld.bfd: final link failed: Invalid operation scripts/Makefile.lib:409: recipe for target 'lib/efi_loader/helloworld_efi.so' failed make[2]: *** [lib/efi_loader/helloworld_efi.so] Error 1 rm lib/efi_loader/helloworld.o scripts/Makefile.build:425: recipe for target 'lib/efi_loader' failed make[1]: *** [lib/efi_loader] Error 2
Isn't this fixed by http://git.denx.de/?p=u-boot.git;a=commit;h=3bb74f9800cdc4cf10a87f2725242c25...
"efi_loader helloworld.efi: Fix building with -Os" ?
Nope, my tree is at v2018.01-rc1-115-g335f7b1290ce which contains that commit already.

On 10.12.17 23:20, Florian Fainelli wrote:
On 12/09/2017 10:40 PM, Heinrich Schuchardt wrote:
On 12/10/2017 12:09 AM, Florian Fainelli wrote:
On 11/26/2017 05:05 AM, Heinrich Schuchardt wrote:
We need to test if we pass a valid image handle when loading and EFI application. This cannot be done in efi_selftest as it is not loaded as an image.
So let's enhance helloworld a bit.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
This particular patch/commit bbf75dd9345d0b1a7ec7a50016547eb7c759b7af ("efi_loader: output load options in helloworld") was bisected and causes the Lamobo_R1_defconfig build using the toolchain at [1] to fail with:
/home/fainelli/work/toolchains/stbgcc-6.3-1.1/bin/arm-linux-ld.bfd: error: required section '.got' not found in the linker script /home/fainelli/work/toolchains/stbgcc-6.3-1.1/bin/arm-linux-ld.bfd: final link failed: Invalid operation scripts/Makefile.lib:409: recipe for target 'lib/efi_loader/helloworld_efi.so' failed make[2]: *** [lib/efi_loader/helloworld_efi.so] Error 1 rm lib/efi_loader/helloworld.o scripts/Makefile.build:425: recipe for target 'lib/efi_loader' failed make[1]: *** [lib/efi_loader] Error 2
Isn't this fixed by http://git.denx.de/?p=u-boot.git;a=commit;h=3bb74f9800cdc4cf10a87f2725242c25...
"efi_loader helloworld.efi: Fix building with -Os" ?
Nope, my tree is at v2018.01-rc1-115-g335f7b1290ce which contains that commit already.
Yeah, but the issue is the same :(
$ nm lib/efi_loader/helloworld.o 00000000 T efi_main U memcpy
Gcc simply optimizes the loaded_image_guid assignment into a memcpy() and I can't find any way to prohibit it from doing that.
I guess the easiest way out of that mess is to just not do a dynamic struct assignment but instead put loaded_image_guid explicitly into the .rodata section.
I'll send a patch.
Alex

Check that helloworld.efi returns EFI_SUCCESS.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 no change v2 new patch --- test/py/tests/test_efi_loader.py | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/test/py/tests/test_efi_loader.py b/test/py/tests/test_efi_loader.py index 4d391e13ef..906ef2feaa 100644 --- a/test/py/tests/test_efi_loader.py +++ b/test/py/tests/test_efi_loader.py @@ -154,6 +154,8 @@ def test_efi_helloworld_net(u_boot_console): output = u_boot_console.run_command('bootefi %x' % addr) expected_text = 'Hello, world' assert expected_text in output + expected_text = '## Application terminated, r = 0' + assert expected_text in output
@pytest.mark.buildconfigspec('cmd_bootefi_hello') def test_efi_helloworld_builtin(u_boot_console):

The handle of a loaded image is the value of the handle member of the loaded image info object and not the address of the loaded image info.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 no change v2 new patch --- cmd/bootefi.c | 7 ++++--- lib/efi_loader/efi_boottime.c | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 67855ba685..e6b8ec11d5 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -254,7 +254,8 @@ static efi_status_t do_bootefi_exec(void *efi, void *fdt, dcache_disable(); /* flush cache before switch to EL2 */
/* Move into EL2 and keep running there */ - armv8_switch_to_el2((ulong)entry, (ulong)&loaded_image_info, + armv8_switch_to_el2((ulong)entry, + (ulong)&loaded_image_info_obj.handle, (ulong)&systab, 0, (ulong)efi_run_in_el2, ES_TO_AARCH64);
@@ -263,7 +264,7 @@ static efi_status_t do_bootefi_exec(void *efi, void *fdt, } #endif
- ret = efi_do_enter(&loaded_image_info, &systab, entry); + ret = efi_do_enter(loaded_image_info_obj.handle, &systab, entry);
exit: /* image has returned, loaded-image obj goes *poof*: */ @@ -350,7 +351,7 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) /* Transfer environment variable efi_selftest as load options */ set_load_options(&loaded_image_info, "efi_selftest"); /* Execute the test */ - r = efi_selftest(&loaded_image_info, &systab); + r = efi_selftest(loaded_image_info_obj.handle, &systab); efi_restore_gd(); free(loaded_image_info.load_options); list_del(&loaded_image_info_obj.link); diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 54e9c27db1..0fb2848ecf 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1317,7 +1317,7 @@ static efi_status_t EFIAPI efi_load_image(bool boot_policy,
info->system_table = &systab; info->parent_handle = parent_image; - *image_handle = info; + *image_handle = obj->handle;
return EFI_EXIT(EFI_SUCCESS); }

To avoid duplicate coding provide a helper function that initializes an EFI object and adds it to the EFI object list.
efi_exit() is the only place where we dereference a handle to obtain a protocol interface. Add a comment to the function.
Suggested-by: Alexander Graf agraf@suse.de Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 no change v2 new patch --- include/efi_loader.h | 2 ++ lib/efi_loader/efi_boottime.c | 40 +++++++++++++++++++++++++++++++++------- lib/efi_loader/efi_disk.c | 4 +--- lib/efi_loader/efi_gop.c | 4 +--- lib/efi_loader/efi_net.c | 4 +--- 5 files changed, 38 insertions(+), 16 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index a73bbc1269..c0caabddb1 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -186,6 +186,8 @@ void efi_restore_gd(void); void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map); /* Call this to set the current device name */ void efi_set_bootdev(const char *dev, const char *devnr, const char *path); +/* Add a new object to the object list. */ +void efi_add_handle(struct efi_object *obj); /* Create handle */ efi_status_t efi_create_handle(void **handle); /* Call this to validate a handle and find the EFI object for it */ diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 0fb2848ecf..a37fb25638 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -321,6 +321,23 @@ static efi_status_t EFIAPI efi_free_pool_ext(void *buffer) return EFI_EXIT(r); }
+/* + * Add a new object to the object list. + * + * The protocols list is initialized. + * The object handle is set. + * + * @obj object to be added + */ +void efi_add_handle(struct efi_object *obj) +{ + if (!obj) + return; + INIT_LIST_HEAD(&obj->protocols); + obj->handle = obj; + list_add_tail(&obj->link, &efi_obj_list); +} + /* * Create handle. * @@ -337,11 +354,8 @@ efi_status_t efi_create_handle(void **handle) (void **)&obj); if (r != EFI_SUCCESS) return r; - memset(obj, 0, sizeof(struct efi_object)); - obj->handle = obj; - INIT_LIST_HEAD(&obj->protocols); - list_add_tail(&obj->link, &efi_obj_list); - *handle = obj; + efi_add_handle(obj); + *handle = obj->handle; return r; }
@@ -1163,14 +1177,15 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob { efi_status_t ret;
+ /* Add internal object to object list */ + efi_add_handle(obj); + /* efi_exit() assumes that the handle points to the info */ obj->handle = info;
info->file_path = file_path; if (device_path) info->device_handle = efi_dp_find_obj(device_path, NULL);
- INIT_LIST_HEAD(&obj->protocols); - list_add_tail(&obj->link, &efi_obj_list); /* * When asking for the device path interface, return * bootefi_device_path @@ -1379,6 +1394,17 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle, efi_status_t exit_status, unsigned long exit_data_size, int16_t *exit_data) { + /* + * We require that the handle points to the original loaded + * image protocol interface. + * + * For getting the longjmp address this is safer than locating + * the protocol because the protocol may have been reinstalled + * pointing to another memory location. + * + * TODO: We should call the unload procedure of the loaded + * image protocol. + */ struct efi_loaded_image *loaded_image_info = (void*)image_handle;
EFI_ENTRY("%p, %ld, %ld, %p", image_handle, exit_status, diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index af8db40e19..68ba2cf7b2 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -224,13 +224,11 @@ static void efi_disk_add_dev(const char *name, goto out_of_memory;
/* Hook up to the device list */ - INIT_LIST_HEAD(&diskobj->parent.protocols); - list_add_tail(&diskobj->parent.link, &efi_obj_list); + efi_add_handle(&diskobj->parent);
/* Fill in object data */ diskobj->dp = efi_dp_from_part(desc, part); diskobj->part = part; - diskobj->parent.handle = diskobj; ret = efi_add_protocol(diskobj->parent.handle, &efi_block_io_guid, &diskobj->ops); if (ret != EFI_SUCCESS) diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c index 498184d754..3caddd5f84 100644 --- a/lib/efi_loader/efi_gop.c +++ b/lib/efi_loader/efi_gop.c @@ -180,11 +180,9 @@ int efi_gop_register(void) }
/* Hook up to the device list */ - INIT_LIST_HEAD(&gopobj->parent.protocols); - list_add_tail(&gopobj->parent.link, &efi_obj_list); + efi_add_handle(&gopobj->parent);
/* Fill in object data */ - gopobj->parent.handle = &gopobj->ops; ret = efi_add_protocol(gopobj->parent.handle, &efi_gop_guid, &gopobj->ops); if (ret != EFI_SUCCESS) { diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c index 74a67c5365..8c5d5b492c 100644 --- a/lib/efi_loader/efi_net.c +++ b/lib/efi_loader/efi_net.c @@ -296,11 +296,9 @@ int efi_net_register(void) goto out_of_memory;
/* Hook net up to the device list */ - INIT_LIST_HEAD(&netobj->parent.protocols); - list_add_tail(&netobj->parent.link, &efi_obj_list); + efi_add_handle(&netobj->parent);
/* Fill in object data */ - netobj->parent.handle = &netobj->net; r = efi_add_protocol(netobj->parent.handle, &efi_net_guid, &netobj->net); if (r != EFI_SUCCESS)

Hi,
On 26 November 2017 at 06:05, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Up to now the protocols of an EFI handle where contained in an array of fixed size. With the patch series the protocols are managed in a linked list. This both saves memory and gives more flexibility.
The LocateDevicePath boot service is implemented according to the UEFI specification.
A unit test for the LocateDevicePath boot service and the device path to text protocol are added.
Some bug fixes are provided.
v3 Remove patch efi_loader: efi_bootmgr: do not make hidden assignments (A patch to add device path printing in printf() is pending.) Resubmit efi_loader: helloworld.c: remove superfluous include v2 Enhance the helloworld example to check if the image handle is passed corectly. Adjust the related py test.
Use a helper function to initialize EFI objects. Further minor bug fixes.
Heinrich Schuchardt (18): efi_loader: helloworld.c: remove superfluous include efi_loader: size of media device path node represenation efi_loader: efi_dp_str should print path not node efi_loader: fix efi_convert_device_node_to_text efi_loader: reimplement LocateDevicePath efi_selftest: test EFI_DEVICE_PATH_TO_TEXT_PROTOCOL efi_loader: efi_disk: use efi_add_protocol efi_loader: efi_net: use efi_add_protocol efi_loader: efi_gop: use efi_add_protocol efi_loader: simplify efi_open_protocol efi_loader: simplify find_obj efi_loader: manage protocols in a linked list efi_selftest: compile without special compiler flags efi_selftest: add missing line feed efi_loader: output load options in helloworld test/py: check return code of helloworld efi_loader: pass handle of loaded image efi_loader: helper function to add EFI object to list
This series seems to have been hanging around for a while. Is it going to be applied soon?
Regards, Simon

On 11/27/2017 06:09 PM, Simon Glass wrote:
Hi,
On 26 November 2017 at 06:05, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Up to now the protocols of an EFI handle where contained in an array of fixed size. With the patch series the protocols are managed in a linked list. This both saves memory and gives more flexibility.
The LocateDevicePath boot service is implemented according to the UEFI specification.
A unit test for the LocateDevicePath boot service and the device path to text protocol are added.
Some bug fixes are provided.
v3 Remove patch efi_loader: efi_bootmgr: do not make hidden assignments (A patch to add device path printing in printf() is pending.) Resubmit efi_loader: helloworld.c: remove superfluous include v2 Enhance the helloworld example to check if the image handle is passed corectly. Adjust the related py test.
Use a helper function to initialize EFI objects. Further minor bug fixes.
Heinrich Schuchardt (18): efi_loader: helloworld.c: remove superfluous include efi_loader: size of media device path node represenation efi_loader: efi_dp_str should print path not node efi_loader: fix efi_convert_device_node_to_text efi_loader: reimplement LocateDevicePath efi_selftest: test EFI_DEVICE_PATH_TO_TEXT_PROTOCOL efi_loader: efi_disk: use efi_add_protocol efi_loader: efi_net: use efi_add_protocol efi_loader: efi_gop: use efi_add_protocol efi_loader: simplify efi_open_protocol efi_loader: simplify find_obj efi_loader: manage protocols in a linked list efi_selftest: compile without special compiler flags efi_selftest: add missing line feed efi_loader: output load options in helloworld test/py: check return code of helloworld efi_loader: pass handle of loaded image efi_loader: helper function to add EFI object to list
This series seems to have been hanging around for a while. Is it going to be applied soon?
Well, v3 was only one day old by the time you wrote :). But yes, on it.
Alex
participants (4)
-
Alexander Graf
-
Florian Fainelli
-
Heinrich Schuchardt
-
Simon Glass