| Message ID | 20210305222303.2866284-5-ilias.apalodimas@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/6] efi_selftest: Remove loadfile2 for initrd selftests | expand |
On 05.03.21 23:23, Ilias Apalodimas wrote: > The UEFI spec allow a packed array of UEFI device paths in the > FilePathList[] of an EFI_LOAD_OPTION. The first file path must > describe the loaded image but the rest are OS specific. > > Previous patches parse the device path and try to use the second > member of the array as an initrd. So let's modify efidebug slightly > and install the second file described in the command line as the > initrd device path. > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > --- > cmd/efidebug.c | 193 ++++++++++++++---- > doc/board/emulation/qemu_capsule_update.rst | 4 +- > doc/uefi/uefi.rst | 2 +- > .../test_efi_capsule/test_capsule_firmware.py | 6 +- > test/py/tests/test_efi_secboot/test_signed.py | 16 +- > .../test_efi_secboot/test_signed_intca.py | 8 +- > .../tests/test_efi_secboot/test_unsigned.py | 8 +- > 7 files changed, 179 insertions(+), 58 deletions(-) > > diff --git a/cmd/efidebug.c b/cmd/efidebug.c > index bbbcb0a54643..9a1c471a3eaa 100644 > --- a/cmd/efidebug.c > +++ b/cmd/efidebug.c > @@ -9,6 +9,8 @@ > #include <common.h> > #include <command.h> > #include <efi_dt_fixup.h> > +#include <efi_helper.h> > +#include <efi_load_initrd.h> > #include <efi_loader.h> > #include <efi_rng.h> > #include <exports.h> > @@ -19,6 +21,7 @@ > #include <part.h> > #include <search.h> > #include <linux/ctype.h> > +#include <linux/err.h> > > #define BS systab.boottime > #define RT systab.runtime > @@ -794,6 +797,65 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag, > return CMD_RET_SUCCESS; > } > > +/** > + * add_initrd_instance() - Append a device path to load_options pointing to an > + * inirtd > + * > + * @argc: Number of arguments > + * @argv: Argument array > + * @file_path Existing device path, the new instance will be appended > + * Return: Pointer to the device path or ERR_PTR > + * > + */ > +static > +struct efi_device_path *add_initrd_instance(const char *dev, const char *part, > + const char *file, > + const struct efi_device_path *fp, > + efi_uintn_t *fp_size) > +{ > + struct efi_device_path *tmp_dp = NULL, *tmp_fp = NULL; > + struct efi_device_path *final_fp = NULL, *initrd_dp = NULL; > + efi_status_t ret; > + const struct efi_initrd_dp id_dp = { > + .vendor = { > + { > + DEVICE_PATH_TYPE_MEDIA_DEVICE, > + DEVICE_PATH_SUB_TYPE_VENDOR_PATH, > + sizeof(id_dp.vendor), > + }, > + EFI_INITRD_MEDIA_GUID, > + }, > + .end = { > + DEVICE_PATH_TYPE_END, > + DEVICE_PATH_SUB_TYPE_END, > + sizeof(id_dp.end), > + } > + }; > + > + ret = efi_dp_from_name(dev, part, file, &tmp_dp, &tmp_fp); > + if (ret != EFI_SUCCESS) { > + printf("Cannot create device path for \"%s %s\"\n", part, file); > + goto out; > + } > + > + initrd_dp = > + efi_dp_append_instance((const struct efi_device_path *)&id_dp, Please, pass &id_dp.end here to avoid a superfluous end node. Best regards Heinrich > + tmp_fp); > + if (!initrd_dp) { > + printf("Cannot append media vendor device path path\n"); > + goto out; > + } > + final_fp = efi_dp_concat(fp, initrd_dp); > + *fp_size = efi_dp_size(fp) + efi_dp_size(initrd_dp) + > + (2 * sizeof(struct efi_device_path)); > + > +out: > + efi_free_pool(initrd_dp); > + efi_free_pool(tmp_dp); > + efi_free_pool(tmp_fp); > + return final_fp ? final_fp : ERR_PTR(-EINVAL); > +} > + > /** > * do_efi_boot_add() - set UEFI load option > * > @@ -806,7 +868,9 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag, > * > * Implement efidebug "boot add" sub-command. Create or change UEFI load option. > * > - * efidebug boot add <id> <label> <interface> <devnum>[:<part>] <file> <options> > + * efidebug boot add -b <id> <label> <interface> <devnum>[:<part>] <file> > + * -i <file> <interface2> <devnum2>[:<part>] <initrd> > + * -s '<options>' > */ > static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, > int argc, char *const argv[]) > @@ -819,55 +883,98 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, > size_t label_len, label_len16; > u16 *label; > struct efi_device_path *device_path = NULL, *file_path = NULL; > + struct efi_device_path *final_fp = NULL; > struct efi_load_option lo; > void *data = NULL; > efi_uintn_t size; > + efi_uintn_t fp_size; > efi_status_t ret; > int r = CMD_RET_SUCCESS; > - > - if (argc < 6 || argc > 7) > - return CMD_RET_USAGE; > - > - id = (int)simple_strtoul(argv[1], &endp, 16); > - if (*endp != '\0' || id > 0xffff) > - return CMD_RET_USAGE; > - > - sprintf(var_name, "Boot%04X", id); > - p = var_name16; > - utf8_utf16_strncpy(&p, var_name, 9); > + int i; > > guid = efi_global_variable_guid; > > /* attributes */ > lo.attributes = LOAD_OPTION_ACTIVE; /* always ACTIVE */ > + lo.optional_data = NULL; > + lo.label = NULL; > > - /* label */ > - label_len = strlen(argv[2]); > - label_len16 = utf8_utf16_strnlen(argv[2], label_len); > - label = malloc((label_len16 + 1) * sizeof(u16)); > - if (!label) > - return CMD_RET_FAILURE; > - lo.label = label; /* label will be changed below */ > - utf8_utf16_strncpy(&label, argv[2], label_len); > + /* search for -b first since the rest of the arguments depends on that */ > + for (i = 0; i < argc; i++) { > + if (!strcmp(argv[i], "-b")) { > + if (argc < i + 5 || lo.label) { > + r = CMD_RET_USAGE; > + goto out; > + } > + id = (int)simple_strtoul(argv[i + 1], &endp, 16); > + if (*endp != '\0' || id > 0xffff) > + return CMD_RET_USAGE; > + > + sprintf(var_name, "Boot%04X", id); > + p = var_name16; > + utf8_utf16_strncpy(&p, var_name, 9); > + > + /* label */ > + label_len = strlen(argv[i + 2]); > + label_len16 = utf8_utf16_strnlen(argv[i + 2], label_len); > + label = malloc((label_len16 + 1) * sizeof(u16)); > + if (!label) > + return CMD_RET_FAILURE; > + lo.label = label; /* label will be changed below */ > + utf8_utf16_strncpy(&label, argv[i + 2], label_len); > + > + /* file path */ > + ret = efi_dp_from_name(argv[i + 3], argv[i + 4], > + argv[i + 5], &device_path, > + &file_path); > + if (ret != EFI_SUCCESS) { > + printf("Cannot create device path for \"%s %s\"\n", > + argv[3], argv[4]); > + r = CMD_RET_FAILURE; > + goto out; > + break; > + } > + fp_size = efi_dp_size(file_path) + > + sizeof(struct efi_device_path); > + } > + } > > - /* file path */ > - ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path, > - &file_path); > - if (ret != EFI_SUCCESS) { > - printf("Cannot create device path for \"%s %s\"\n", > - argv[3], argv[4]); > + if (!file_path) { > + printf("You need to specify an image before an initrd.\n"); > r = CMD_RET_FAILURE; > goto out; > } > - lo.file_path = file_path; > - lo.file_path_length = efi_dp_size(file_path) > - + sizeof(struct efi_device_path); /* for END */ > > - /* optional data */ > - if (argc == 6) > - lo.optional_data = NULL; > - else > - lo.optional_data = (const u8 *)argv[6]; > + /* add now add initrd and extra data */ > + for (i = 0; i < argc; i++) { > + if (!strcmp(argv[i], "-i")) { > + if (argc < i + 3 || final_fp) { > + r = CMD_RET_USAGE; > + goto out; > + } > + > + final_fp = add_initrd_instance(argv[i + 1], argv[i + 2], > + argv[i + 3], file_path, > + &fp_size); > + if (IS_ERR(final_fp)) { > + r = CMD_RET_FAILURE; > + goto out; > + } > + > + /* add_initrd_instance allocates a new device path */ > + efi_free_pool(file_path); > + file_path = final_fp; > + } else if (!strcmp(argv[i], "-s")) { > + if (argc < i + 1 || lo.optional_data) { > + r = CMD_RET_USAGE; > + goto out; > + } > + lo.optional_data = (const u8 *)argv[i + 1]; > + } > + } > + > + lo.file_path = file_path; > + lo.file_path_length = fp_size; > > size = efi_serialize_load_option(&lo, (u8 **)&data); > if (!size) { > @@ -951,11 +1058,14 @@ static int do_efi_boot_rm(struct cmd_tbl *cmdtp, int flag, > */ > static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t *size) > { > + struct efi_device_path *initrd_path = NULL; > struct efi_load_option lo; > char *label, *p; > size_t label_len16, label_len; > u16 *dp_str; > efi_status_t ret; > + efi_uintn_t initrd_dp_size; > + const efi_guid_t lf2_initrd_guid = EFI_INITRD_MEDIA_GUID; > > ret = efi_deserialize_load_option(&lo, data, size); > if (ret != EFI_SUCCESS) { > @@ -986,6 +1096,14 @@ static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t *size) > printf(" file_path: %ls\n", dp_str); > efi_free_pool(dp_str); > > + initrd_path = efi_dp_from_lo(&lo, &initrd_dp_size, lf2_initrd_guid); > + if (initrd_path) { > + dp_str = efi_dp_str(initrd_path); > + printf(" initrd_path: %ls\n", dp_str); > + efi_free_pool(dp_str); > + efi_free_pool(initrd_path); > + } > + > printf(" data:\n"); > print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1, > lo.optional_data, *size, true); > @@ -1555,7 +1673,10 @@ static int do_efidebug(struct cmd_tbl *cmdtp, int flag, > static char efidebug_help_text[] = > " - UEFI Shell-like interface to configure UEFI environment\n" > "\n" > - "efidebug boot add <bootid> <label> <interface> <devnum>[:<part>] <file path> [<load options>]\n" > + "efidebug boot add " > + "-b <bootid> <label> <interface> <devnum>[:<part>] <file path> " > + "-i <interface> <devnum>[:<part>] <initrd file path> " > + "-s '<optional data>'\n" > " - set UEFI BootXXXX variable\n" > " <load options> will be passed to UEFI application\n" > "efidebug boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n" > @@ -1599,7 +1720,7 @@ static char efidebug_help_text[] = > #endif > > U_BOOT_CMD( > - efidebug, 10, 0, do_efidebug, > + efidebug, CONFIG_SYS_MAXARGS, 0, do_efidebug, > "Configure UEFI environment", > efidebug_help_text > ); > diff --git a/doc/board/emulation/qemu_capsule_update.rst b/doc/board/emulation/qemu_capsule_update.rst > index 9fec75f8f1c9..33ce4bcd32ea 100644 > --- a/doc/board/emulation/qemu_capsule_update.rst > +++ b/doc/board/emulation/qemu_capsule_update.rst > @@ -60,7 +60,7 @@ to be pointing to the EFI System Partition which contains the capsule > file. The BootNext, BootXXXX and OsIndications variables can be set > using the following commands:: > > - => efidebug boot add 0 Boot0000 virtio 0:1 <capsule_file_name> > + => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name> > => efidebug boot next 0 > => setenv -e -nv -bs -rt -v OsIndications =0x04 > => saveenv > @@ -198,7 +198,7 @@ command line:: > 3. Set the following environment and UEFI boot variables > > => setenv -e -nv -bs -rt -v OsIndications =0x04 > - => efidebug boot add 0 Boot0000 virtio 0:1 <capsule_file_name> > + => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name> > => efidebug boot next 0 > => saveenv > > diff --git a/doc/uefi/uefi.rst b/doc/uefi/uefi.rst > index 5a67737c1579..b3494c22e073 100644 > --- a/doc/uefi/uefi.rst > +++ b/doc/uefi/uefi.rst > @@ -178,7 +178,7 @@ Now in U-Boot install the keys on your board:: > > Set up boot parameters on your board:: > > - efidebug boot add 1 HELLO mmc 0:1 /helloworld.efi.signed "" > + efidebug boot add -b 1 HELLO mmc 0:1 /helloworld.efi.signed "" > > Now your board can run the signed image via the boot manager (see below). > You can also try this sequence by running Pytest, test_efi_secboot, > diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware.py b/test/py/tests/test_efi_capsule/test_capsule_firmware.py > index f006fa95d650..e8b0a1575453 100644 > --- a/test/py/tests/test_efi_capsule/test_capsule_firmware.py > +++ b/test/py/tests/test_efi_capsule/test_capsule_firmware.py > @@ -39,7 +39,7 @@ class TestEfiCapsuleFirmwareFit(object): > with u_boot_console.log.section('Test Case 1-a, before reboot'): > output = u_boot_console.run_command_list([ > 'host bind 0 %s' % disk_img, > - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""', > + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""', > 'efidebug boot order 1', > 'env set -e OsIndications', > 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"', > @@ -114,7 +114,7 @@ class TestEfiCapsuleFirmwareFit(object): > with u_boot_console.log.section('Test Case 2-a, before reboot'): > output = u_boot_console.run_command_list([ > 'host bind 0 %s' % disk_img, > - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""', > + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""', > 'efidebug boot order 1', > 'env set -e -nv -bs -rt OsIndications =0x0000000000000004', > 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"', > @@ -188,7 +188,7 @@ class TestEfiCapsuleFirmwareFit(object): > with u_boot_console.log.section('Test Case 3-a, before reboot'): > output = u_boot_console.run_command_list([ > 'host bind 0 %s' % disk_img, > - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""', > + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""', > 'efidebug boot order 1', > 'env set -e -nv -bs -rt OsIndications =0x0000000000000004', > 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"', > diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py > index 863685e215b7..75f5ea772300 100644 > --- a/test/py/tests/test_efi_secboot/test_signed.py > +++ b/test/py/tests/test_efi_secboot/test_signed.py > @@ -28,7 +28,7 @@ class TestEfiSignedImage(object): > # Test Case 1a, run signed image if no PK > output = u_boot_console.run_command_list([ > 'host bind 0 %s' % disk_img, > - 'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""', > + 'efidebug boot add -b 1 HELLO1 host 0:1 /helloworld.efi.signed ""', > 'efidebug boot next 1', > 'bootefi bootmgr']) > assert 'Hello, world!' in ''.join(output) > @@ -36,7 +36,7 @@ class TestEfiSignedImage(object): > with u_boot_console.log.section('Test Case 1b'): > # Test Case 1b, run unsigned image if no PK > output = u_boot_console.run_command_list([ > - 'efidebug boot add 2 HELLO2 host 0:1 /helloworld.efi ""', > + 'efidebug boot add -b 2 HELLO2 host 0:1 /helloworld.efi ""', > 'efidebug boot next 2', > 'bootefi bootmgr']) > assert 'Hello, world!' in ''.join(output) > @@ -58,13 +58,13 @@ class TestEfiSignedImage(object): > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > assert 'Failed to set EFI variable' not in ''.join(output) > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""', > + 'efidebug boot add -b 1 HELLO1 host 0:1 /helloworld.efi.signed ""', > 'efidebug boot next 1', > 'efidebug test bootmgr']) > assert('\'HELLO1\' failed' in ''.join(output)) > assert('efi_start_image() returned: 26' in ''.join(output)) > output = u_boot_console.run_command_list([ > - 'efidebug boot add 2 HELLO2 host 0:1 /helloworld.efi ""', > + 'efidebug boot add -b 2 HELLO2 host 0:1 /helloworld.efi ""', > 'efidebug boot next 2', > 'efidebug test bootmgr']) > assert '\'HELLO2\' failed' in ''.join(output) > @@ -104,7 +104,7 @@ class TestEfiSignedImage(object): > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > assert 'Failed to set EFI variable' not in ''.join(output) > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""', > 'efidebug boot next 1', > 'efidebug test bootmgr']) > assert '\'HELLO\' failed' in ''.join(output) > @@ -142,7 +142,7 @@ class TestEfiSignedImage(object): > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > assert 'Failed to set EFI variable' not in ''.join(output) > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""', > 'efidebug boot next 1', > 'efidebug test bootmgr']) > assert '\'HELLO\' failed' in ''.join(output) > @@ -169,7 +169,7 @@ class TestEfiSignedImage(object): > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > assert 'Failed to set EFI variable' not in ''.join(output) > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed_2sigs ""', > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed_2sigs ""', > 'efidebug boot next 1', > 'efidebug test bootmgr']) > assert 'Hello, world!' in ''.join(output) > @@ -227,7 +227,7 @@ class TestEfiSignedImage(object): > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > assert 'Failed to set EFI variable' not in ''.join(output) > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""', > 'efidebug boot next 1', > 'bootefi bootmgr']) > assert 'Hello, world!' in ''.join(output) > diff --git a/test/py/tests/test_efi_secboot/test_signed_intca.py b/test/py/tests/test_efi_secboot/test_signed_intca.py > index 70d6be00e8a8..0849572a5143 100644 > --- a/test/py/tests/test_efi_secboot/test_signed_intca.py > +++ b/test/py/tests/test_efi_secboot/test_signed_intca.py > @@ -39,7 +39,7 @@ class TestEfiSignedImageIntca(object): > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""', > + 'efidebug boot add -b 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""', > 'efidebug boot next 1', > 'efidebug test bootmgr']) > assert '\'HELLO_a\' failed' in ''.join(output) > @@ -48,7 +48,7 @@ class TestEfiSignedImageIntca(object): > with u_boot_console.log.section('Test Case 1b'): > # Test Case 1b, signed and authenticated by root CA > output = u_boot_console.run_command_list([ > - 'efidebug boot add 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""', > + 'efidebug boot add -b 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""', > 'efidebug boot next 2', > 'bootefi bootmgr']) > assert 'Hello, world!' in ''.join(output) > @@ -70,7 +70,7 @@ class TestEfiSignedImageIntca(object): > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', > + 'efidebug boot add -b 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', > 'efidebug boot next 1', > 'efidebug test bootmgr']) > assert '\'HELLO_abc\' failed' in ''.join(output) > @@ -116,7 +116,7 @@ class TestEfiSignedImageIntca(object): > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', > + 'efidebug boot add -b 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', > 'efidebug boot next 1', > 'efidebug test bootmgr']) > assert 'Hello, world!' in ''.join(output) > diff --git a/test/py/tests/test_efi_secboot/test_unsigned.py b/test/py/tests/test_efi_secboot/test_unsigned.py > index 56f56e19eb84..8e026f7566ad 100644 > --- a/test/py/tests/test_efi_secboot/test_unsigned.py > +++ b/test/py/tests/test_efi_secboot/test_unsigned.py > @@ -35,7 +35,7 @@ class TestEfiUnsignedImage(object): > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', > 'efidebug boot next 1', > 'bootefi bootmgr']) > assert '\'HELLO\' failed' in ''.join(output) > @@ -64,7 +64,7 @@ class TestEfiUnsignedImage(object): > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', > 'efidebug boot next 1', > 'bootefi bootmgr']) > assert 'Hello, world!' in ''.join(output) > @@ -88,7 +88,7 @@ class TestEfiUnsignedImage(object): > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', > 'efidebug boot next 1', > 'bootefi bootmgr']) > assert '\'HELLO\' failed' in ''.join(output) > @@ -106,7 +106,7 @@ class TestEfiUnsignedImage(object): > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', > 'efidebug boot next 1', > 'bootefi bootmgr']) > assert '\'HELLO\' failed' in ''.join(output) >
On Thu, Mar 11, 2021 at 01:38:33PM +0100, Heinrich Schuchardt wrote: > > + } > > + > > + initrd_dp = > > + efi_dp_append_instance((const struct efi_device_path *)&id_dp, > > Please, pass &id_dp.end here to avoid a superfluous end node. Wont this make efi_dp_size() loop endlessly? Regards /Ilias > > Best regards > > Heinrich > > > + tmp_fp); > > + if (!initrd_dp) { > > + printf("Cannot append media vendor device path path\n"); > > + goto out; > > + } > > + final_fp = efi_dp_concat(fp, initrd_dp); > > + *fp_size = efi_dp_size(fp) + efi_dp_size(initrd_dp) + > > + (2 * sizeof(struct efi_device_path)); > > + > > +out: > > + efi_free_pool(initrd_dp); > > + efi_free_pool(tmp_dp); > > + efi_free_pool(tmp_fp); > > + return final_fp ? final_fp : ERR_PTR(-EINVAL); > > +} > > + > > /** > > * do_efi_boot_add() - set UEFI load option > > * > > @@ -806,7 +868,9 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag, > > * > > * Implement efidebug "boot add" sub-command. Create or change UEFI load option. > > * > > - * efidebug boot add <id> <label> <interface> <devnum>[:<part>] <file> <options> > > + * efidebug boot add -b <id> <label> <interface> <devnum>[:<part>] <file> > > + * -i <file> <interface2> <devnum2>[:<part>] <initrd> > > + * -s '<options>' > > */ > > static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, > > int argc, char *const argv[]) > > @@ -819,55 +883,98 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, > > size_t label_len, label_len16; > > u16 *label; > > struct efi_device_path *device_path = NULL, *file_path = NULL; > > + struct efi_device_path *final_fp = NULL; > > struct efi_load_option lo; > > void *data = NULL; > > efi_uintn_t size; > > + efi_uintn_t fp_size; > > efi_status_t ret; > > int r = CMD_RET_SUCCESS; > > - > > - if (argc < 6 || argc > 7) > > - return CMD_RET_USAGE; > > - > > - id = (int)simple_strtoul(argv[1], &endp, 16); > > - if (*endp != '\0' || id > 0xffff) > > - return CMD_RET_USAGE; > > - > > - sprintf(var_name, "Boot%04X", id); > > - p = var_name16; > > - utf8_utf16_strncpy(&p, var_name, 9); > > + int i; > > > > guid = efi_global_variable_guid; > > > > /* attributes */ > > lo.attributes = LOAD_OPTION_ACTIVE; /* always ACTIVE */ > > + lo.optional_data = NULL; > > + lo.label = NULL; > > > > - /* label */ > > - label_len = strlen(argv[2]); > > - label_len16 = utf8_utf16_strnlen(argv[2], label_len); > > - label = malloc((label_len16 + 1) * sizeof(u16)); > > - if (!label) > > - return CMD_RET_FAILURE; > > - lo.label = label; /* label will be changed below */ > > - utf8_utf16_strncpy(&label, argv[2], label_len); > > + /* search for -b first since the rest of the arguments depends on that */ > > + for (i = 0; i < argc; i++) { > > + if (!strcmp(argv[i], "-b")) { > > + if (argc < i + 5 || lo.label) { > > + r = CMD_RET_USAGE; > > + goto out; > > + } > > + id = (int)simple_strtoul(argv[i + 1], &endp, 16); > > + if (*endp != '\0' || id > 0xffff) > > + return CMD_RET_USAGE; > > + > > + sprintf(var_name, "Boot%04X", id); > > + p = var_name16; > > + utf8_utf16_strncpy(&p, var_name, 9); > > + > > + /* label */ > > + label_len = strlen(argv[i + 2]); > > + label_len16 = utf8_utf16_strnlen(argv[i + 2], label_len); > > + label = malloc((label_len16 + 1) * sizeof(u16)); > > + if (!label) > > + return CMD_RET_FAILURE; > > + lo.label = label; /* label will be changed below */ > > + utf8_utf16_strncpy(&label, argv[i + 2], label_len); > > + > > + /* file path */ > > + ret = efi_dp_from_name(argv[i + 3], argv[i + 4], > > + argv[i + 5], &device_path, > > + &file_path); > > + if (ret != EFI_SUCCESS) { > > + printf("Cannot create device path for \"%s %s\"\n", > > + argv[3], argv[4]); > > + r = CMD_RET_FAILURE; > > + goto out; > > + break; > > + } > > + fp_size = efi_dp_size(file_path) + > > + sizeof(struct efi_device_path); > > + } > > + } > > > > - /* file path */ > > - ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path, > > - &file_path); > > - if (ret != EFI_SUCCESS) { > > - printf("Cannot create device path for \"%s %s\"\n", > > - argv[3], argv[4]); > > + if (!file_path) { > > + printf("You need to specify an image before an initrd.\n"); > > r = CMD_RET_FAILURE; > > goto out; > > } > > - lo.file_path = file_path; > > - lo.file_path_length = efi_dp_size(file_path) > > - + sizeof(struct efi_device_path); /* for END */ > > > > - /* optional data */ > > - if (argc == 6) > > - lo.optional_data = NULL; > > - else > > - lo.optional_data = (const u8 *)argv[6]; > > + /* add now add initrd and extra data */ > > + for (i = 0; i < argc; i++) { > > + if (!strcmp(argv[i], "-i")) { > > + if (argc < i + 3 || final_fp) { > > + r = CMD_RET_USAGE; > > + goto out; > > + } > > + > > + final_fp = add_initrd_instance(argv[i + 1], argv[i + 2], > > + argv[i + 3], file_path, > > + &fp_size); > > + if (IS_ERR(final_fp)) { > > + r = CMD_RET_FAILURE; > > + goto out; > > + } > > + > > + /* add_initrd_instance allocates a new device path */ > > + efi_free_pool(file_path); > > + file_path = final_fp; > > + } else if (!strcmp(argv[i], "-s")) { > > + if (argc < i + 1 || lo.optional_data) { > > + r = CMD_RET_USAGE; > > + goto out; > > + } > > + lo.optional_data = (const u8 *)argv[i + 1]; > > + } > > + } > > + > > + lo.file_path = file_path; > > + lo.file_path_length = fp_size; > > > > size = efi_serialize_load_option(&lo, (u8 **)&data); > > if (!size) { > > @@ -951,11 +1058,14 @@ static int do_efi_boot_rm(struct cmd_tbl *cmdtp, int flag, > > */ > > static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t *size) > > { > > + struct efi_device_path *initrd_path = NULL; > > struct efi_load_option lo; > > char *label, *p; > > size_t label_len16, label_len; > > u16 *dp_str; > > efi_status_t ret; > > + efi_uintn_t initrd_dp_size; > > + const efi_guid_t lf2_initrd_guid = EFI_INITRD_MEDIA_GUID; > > > > ret = efi_deserialize_load_option(&lo, data, size); > > if (ret != EFI_SUCCESS) { > > @@ -986,6 +1096,14 @@ static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t *size) > > printf(" file_path: %ls\n", dp_str); > > efi_free_pool(dp_str); > > > > + initrd_path = efi_dp_from_lo(&lo, &initrd_dp_size, lf2_initrd_guid); > > + if (initrd_path) { > > + dp_str = efi_dp_str(initrd_path); > > + printf(" initrd_path: %ls\n", dp_str); > > + efi_free_pool(dp_str); > > + efi_free_pool(initrd_path); > > + } > > + > > printf(" data:\n"); > > print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1, > > lo.optional_data, *size, true); > > @@ -1555,7 +1673,10 @@ static int do_efidebug(struct cmd_tbl *cmdtp, int flag, > > static char efidebug_help_text[] = > > " - UEFI Shell-like interface to configure UEFI environment\n" > > "\n" > > - "efidebug boot add <bootid> <label> <interface> <devnum>[:<part>] <file path> [<load options>]\n" > > + "efidebug boot add " > > + "-b <bootid> <label> <interface> <devnum>[:<part>] <file path> " > > + "-i <interface> <devnum>[:<part>] <initrd file path> " > > + "-s '<optional data>'\n" > > " - set UEFI BootXXXX variable\n" > > " <load options> will be passed to UEFI application\n" > > "efidebug boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n" > > @@ -1599,7 +1720,7 @@ static char efidebug_help_text[] = > > #endif > > > > U_BOOT_CMD( > > - efidebug, 10, 0, do_efidebug, > > + efidebug, CONFIG_SYS_MAXARGS, 0, do_efidebug, > > "Configure UEFI environment", > > efidebug_help_text > > ); > > diff --git a/doc/board/emulation/qemu_capsule_update.rst b/doc/board/emulation/qemu_capsule_update.rst > > index 9fec75f8f1c9..33ce4bcd32ea 100644 > > --- a/doc/board/emulation/qemu_capsule_update.rst > > +++ b/doc/board/emulation/qemu_capsule_update.rst > > @@ -60,7 +60,7 @@ to be pointing to the EFI System Partition which contains the capsule > > file. The BootNext, BootXXXX and OsIndications variables can be set > > using the following commands:: > > > > - => efidebug boot add 0 Boot0000 virtio 0:1 <capsule_file_name> > > + => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name> > > => efidebug boot next 0 > > => setenv -e -nv -bs -rt -v OsIndications =0x04 > > => saveenv > > @@ -198,7 +198,7 @@ command line:: > > 3. Set the following environment and UEFI boot variables > > > > => setenv -e -nv -bs -rt -v OsIndications =0x04 > > - => efidebug boot add 0 Boot0000 virtio 0:1 <capsule_file_name> > > + => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name> > > => efidebug boot next 0 > > => saveenv > > > > diff --git a/doc/uefi/uefi.rst b/doc/uefi/uefi.rst > > index 5a67737c1579..b3494c22e073 100644 > > --- a/doc/uefi/uefi.rst > > +++ b/doc/uefi/uefi.rst > > @@ -178,7 +178,7 @@ Now in U-Boot install the keys on your board:: > > > > Set up boot parameters on your board:: > > > > - efidebug boot add 1 HELLO mmc 0:1 /helloworld.efi.signed "" > > + efidebug boot add -b 1 HELLO mmc 0:1 /helloworld.efi.signed "" > > > > Now your board can run the signed image via the boot manager (see below). > > You can also try this sequence by running Pytest, test_efi_secboot, > > diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware.py b/test/py/tests/test_efi_capsule/test_capsule_firmware.py > > index f006fa95d650..e8b0a1575453 100644 > > --- a/test/py/tests/test_efi_capsule/test_capsule_firmware.py > > +++ b/test/py/tests/test_efi_capsule/test_capsule_firmware.py > > @@ -39,7 +39,7 @@ class TestEfiCapsuleFirmwareFit(object): > > with u_boot_console.log.section('Test Case 1-a, before reboot'): > > output = u_boot_console.run_command_list([ > > 'host bind 0 %s' % disk_img, > > - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""', > > + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""', > > 'efidebug boot order 1', > > 'env set -e OsIndications', > > 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"', > > @@ -114,7 +114,7 @@ class TestEfiCapsuleFirmwareFit(object): > > with u_boot_console.log.section('Test Case 2-a, before reboot'): > > output = u_boot_console.run_command_list([ > > 'host bind 0 %s' % disk_img, > > - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""', > > + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""', > > 'efidebug boot order 1', > > 'env set -e -nv -bs -rt OsIndications =0x0000000000000004', > > 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"', > > @@ -188,7 +188,7 @@ class TestEfiCapsuleFirmwareFit(object): > > with u_boot_console.log.section('Test Case 3-a, before reboot'): > > output = u_boot_console.run_command_list([ > > 'host bind 0 %s' % disk_img, > > - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""', > > + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""', > > 'efidebug boot order 1', > > 'env set -e -nv -bs -rt OsIndications =0x0000000000000004', > > 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"', > > diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py > > index 863685e215b7..75f5ea772300 100644 > > --- a/test/py/tests/test_efi_secboot/test_signed.py > > +++ b/test/py/tests/test_efi_secboot/test_signed.py > > @@ -28,7 +28,7 @@ class TestEfiSignedImage(object): > > # Test Case 1a, run signed image if no PK > > output = u_boot_console.run_command_list([ > > 'host bind 0 %s' % disk_img, > > - 'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""', > > + 'efidebug boot add -b 1 HELLO1 host 0:1 /helloworld.efi.signed ""', > > 'efidebug boot next 1', > > 'bootefi bootmgr']) > > assert 'Hello, world!' in ''.join(output) > > @@ -36,7 +36,7 @@ class TestEfiSignedImage(object): > > with u_boot_console.log.section('Test Case 1b'): > > # Test Case 1b, run unsigned image if no PK > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 2 HELLO2 host 0:1 /helloworld.efi ""', > > + 'efidebug boot add -b 2 HELLO2 host 0:1 /helloworld.efi ""', > > 'efidebug boot next 2', > > 'bootefi bootmgr']) > > assert 'Hello, world!' in ''.join(output) > > @@ -58,13 +58,13 @@ class TestEfiSignedImage(object): > > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""', > > + 'efidebug boot add -b 1 HELLO1 host 0:1 /helloworld.efi.signed ""', > > 'efidebug boot next 1', > > 'efidebug test bootmgr']) > > assert('\'HELLO1\' failed' in ''.join(output)) > > assert('efi_start_image() returned: 26' in ''.join(output)) > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 2 HELLO2 host 0:1 /helloworld.efi ""', > > + 'efidebug boot add -b 2 HELLO2 host 0:1 /helloworld.efi ""', > > 'efidebug boot next 2', > > 'efidebug test bootmgr']) > > assert '\'HELLO2\' failed' in ''.join(output) > > @@ -104,7 +104,7 @@ class TestEfiSignedImage(object): > > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""', > > 'efidebug boot next 1', > > 'efidebug test bootmgr']) > > assert '\'HELLO\' failed' in ''.join(output) > > @@ -142,7 +142,7 @@ class TestEfiSignedImage(object): > > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""', > > 'efidebug boot next 1', > > 'efidebug test bootmgr']) > > assert '\'HELLO\' failed' in ''.join(output) > > @@ -169,7 +169,7 @@ class TestEfiSignedImage(object): > > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed_2sigs ""', > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed_2sigs ""', > > 'efidebug boot next 1', > > 'efidebug test bootmgr']) > > assert 'Hello, world!' in ''.join(output) > > @@ -227,7 +227,7 @@ class TestEfiSignedImage(object): > > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""', > > 'efidebug boot next 1', > > 'bootefi bootmgr']) > > assert 'Hello, world!' in ''.join(output) > > diff --git a/test/py/tests/test_efi_secboot/test_signed_intca.py b/test/py/tests/test_efi_secboot/test_signed_intca.py > > index 70d6be00e8a8..0849572a5143 100644 > > --- a/test/py/tests/test_efi_secboot/test_signed_intca.py > > +++ b/test/py/tests/test_efi_secboot/test_signed_intca.py > > @@ -39,7 +39,7 @@ class TestEfiSignedImageIntca(object): > > assert 'Failed to set EFI variable' not in ''.join(output) > > > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""', > > + 'efidebug boot add -b 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""', > > 'efidebug boot next 1', > > 'efidebug test bootmgr']) > > assert '\'HELLO_a\' failed' in ''.join(output) > > @@ -48,7 +48,7 @@ class TestEfiSignedImageIntca(object): > > with u_boot_console.log.section('Test Case 1b'): > > # Test Case 1b, signed and authenticated by root CA > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""', > > + 'efidebug boot add -b 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""', > > 'efidebug boot next 2', > > 'bootefi bootmgr']) > > assert 'Hello, world!' in ''.join(output) > > @@ -70,7 +70,7 @@ class TestEfiSignedImageIntca(object): > > assert 'Failed to set EFI variable' not in ''.join(output) > > > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', > > + 'efidebug boot add -b 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', > > 'efidebug boot next 1', > > 'efidebug test bootmgr']) > > assert '\'HELLO_abc\' failed' in ''.join(output) > > @@ -116,7 +116,7 @@ class TestEfiSignedImageIntca(object): > > assert 'Failed to set EFI variable' not in ''.join(output) > > > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', > > + 'efidebug boot add -b 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', > > 'efidebug boot next 1', > > 'efidebug test bootmgr']) > > assert 'Hello, world!' in ''.join(output) > > diff --git a/test/py/tests/test_efi_secboot/test_unsigned.py b/test/py/tests/test_efi_secboot/test_unsigned.py > > index 56f56e19eb84..8e026f7566ad 100644 > > --- a/test/py/tests/test_efi_secboot/test_unsigned.py > > +++ b/test/py/tests/test_efi_secboot/test_unsigned.py > > @@ -35,7 +35,7 @@ class TestEfiUnsignedImage(object): > > assert 'Failed to set EFI variable' not in ''.join(output) > > > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', > > 'efidebug boot next 1', > > 'bootefi bootmgr']) > > assert '\'HELLO\' failed' in ''.join(output) > > @@ -64,7 +64,7 @@ class TestEfiUnsignedImage(object): > > assert 'Failed to set EFI variable' not in ''.join(output) > > > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', > > 'efidebug boot next 1', > > 'bootefi bootmgr']) > > assert 'Hello, world!' in ''.join(output) > > @@ -88,7 +88,7 @@ class TestEfiUnsignedImage(object): > > assert 'Failed to set EFI variable' not in ''.join(output) > > > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', > > 'efidebug boot next 1', > > 'bootefi bootmgr']) > > assert '\'HELLO\' failed' in ''.join(output) > > @@ -106,7 +106,7 @@ class TestEfiUnsignedImage(object): > > assert 'Failed to set EFI variable' not in ''.join(output) > > > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', > > 'efidebug boot next 1', > > 'bootefi bootmgr']) > > assert '\'HELLO\' failed' in ''.join(output) > > >
Ilias, Sorry, but I will have to repeat my question for better understandings. On Sat, Mar 06, 2021 at 12:23:01AM +0200, Ilias Apalodimas wrote: > The UEFI spec allow a packed array of UEFI device paths in the > FilePathList[] of an EFI_LOAD_OPTION. The first file path must > describe the loaded image but the rest are OS specific. > > Previous patches parse the device path and try to use the second > member of the array as an initrd. So let's modify efidebug slightly > and install the second file described in the command line as the > initrd device path. > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > --- > cmd/efidebug.c | 193 ++++++++++++++---- > doc/board/emulation/qemu_capsule_update.rst | 4 +- > doc/uefi/uefi.rst | 2 +- > .../test_efi_capsule/test_capsule_firmware.py | 6 +- > test/py/tests/test_efi_secboot/test_signed.py | 16 +- > .../test_efi_secboot/test_signed_intca.py | 8 +- > .../tests/test_efi_secboot/test_unsigned.py | 8 +- > 7 files changed, 179 insertions(+), 58 deletions(-) > > diff --git a/cmd/efidebug.c b/cmd/efidebug.c > index bbbcb0a54643..9a1c471a3eaa 100644 > --- a/cmd/efidebug.c > +++ b/cmd/efidebug.c > @@ -9,6 +9,8 @@ > #include <common.h> > #include <command.h> > #include <efi_dt_fixup.h> > +#include <efi_helper.h> > +#include <efi_load_initrd.h> > #include <efi_loader.h> > #include <efi_rng.h> > #include <exports.h> > @@ -19,6 +21,7 @@ > #include <part.h> > #include <search.h> > #include <linux/ctype.h> > +#include <linux/err.h> > > #define BS systab.boottime > #define RT systab.runtime > @@ -794,6 +797,65 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag, > return CMD_RET_SUCCESS; > } > > +/** > + * add_initrd_instance() - Append a device path to load_options pointing to an > + * inirtd > + * > + * @argc: Number of arguments > + * @argv: Argument array > + * @file_path Existing device path, the new instance will be appended > + * Return: Pointer to the device path or ERR_PTR > + * > + */ > +static > +struct efi_device_path *add_initrd_instance(const char *dev, const char *part, > + const char *file, > + const struct efi_device_path *fp, > + efi_uintn_t *fp_size) > +{ > + struct efi_device_path *tmp_dp = NULL, *tmp_fp = NULL; > + struct efi_device_path *final_fp = NULL, *initrd_dp = NULL; > + efi_status_t ret; > + const struct efi_initrd_dp id_dp = { > + .vendor = { > + { > + DEVICE_PATH_TYPE_MEDIA_DEVICE, > + DEVICE_PATH_SUB_TYPE_VENDOR_PATH, > + sizeof(id_dp.vendor), > + }, > + EFI_INITRD_MEDIA_GUID, > + }, > + .end = { > + DEVICE_PATH_TYPE_END, > + DEVICE_PATH_SUB_TYPE_END, > + sizeof(id_dp.end), > + } > + }; > + > + ret = efi_dp_from_name(dev, part, file, &tmp_dp, &tmp_fp); > + if (ret != EFI_SUCCESS) { > + printf("Cannot create device path for \"%s %s\"\n", part, file); > + goto out; > + } > + > + initrd_dp = > + efi_dp_append_instance((const struct efi_device_path *)&id_dp, > + tmp_fp); > + if (!initrd_dp) { > + printf("Cannot append media vendor device path path\n"); > + goto out; > + } > + final_fp = efi_dp_concat(fp, initrd_dp); > + *fp_size = efi_dp_size(fp) + efi_dp_size(initrd_dp) + > + (2 * sizeof(struct efi_device_path)); > + > +out: > + efi_free_pool(initrd_dp); > + efi_free_pool(tmp_dp); > + efi_free_pool(tmp_fp); > + return final_fp ? final_fp : ERR_PTR(-EINVAL); > +} > + > /** > * do_efi_boot_add() - set UEFI load option > * > @@ -806,7 +868,9 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag, > * > * Implement efidebug "boot add" sub-command. Create or change UEFI load option. > * > - * efidebug boot add <id> <label> <interface> <devnum>[:<part>] <file> <options> > + * efidebug boot add -b <id> <label> <interface> <devnum>[:<part>] <file> > + * -i <file> <interface2> <devnum2>[:<part>] <initrd> > + * -s '<options>' We discussed another syntax: efidebug boot add <id> ... efidebug boot add-initrd <id> <initrd path> (Please don't care detailed syntax for now.) What is the difficulty that you have had to implement this type of interface? Even if we follow your new syntax, Why do we need '-b' option? "<id> <label> <interface> <devnum>[:<part>] <file>" are all mandatory arguments, aren't they? -Takahiro Akashi > */ > static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, > int argc, char *const argv[]) > @@ -819,55 +883,98 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, > size_t label_len, label_len16; > u16 *label; > struct efi_device_path *device_path = NULL, *file_path = NULL; > + struct efi_device_path *final_fp = NULL; > struct efi_load_option lo; > void *data = NULL; > efi_uintn_t size; > + efi_uintn_t fp_size; > efi_status_t ret; > int r = CMD_RET_SUCCESS; > - > - if (argc < 6 || argc > 7) > - return CMD_RET_USAGE; > - > - id = (int)simple_strtoul(argv[1], &endp, 16); > - if (*endp != '\0' || id > 0xffff) > - return CMD_RET_USAGE; > - > - sprintf(var_name, "Boot%04X", id); > - p = var_name16; > - utf8_utf16_strncpy(&p, var_name, 9); > + int i; > > guid = efi_global_variable_guid; > > /* attributes */ > lo.attributes = LOAD_OPTION_ACTIVE; /* always ACTIVE */ > + lo.optional_data = NULL; > + lo.label = NULL; > > - /* label */ > - label_len = strlen(argv[2]); > - label_len16 = utf8_utf16_strnlen(argv[2], label_len); > - label = malloc((label_len16 + 1) * sizeof(u16)); > - if (!label) > - return CMD_RET_FAILURE; > - lo.label = label; /* label will be changed below */ > - utf8_utf16_strncpy(&label, argv[2], label_len); > + /* search for -b first since the rest of the arguments depends on that */ > + for (i = 0; i < argc; i++) { > + if (!strcmp(argv[i], "-b")) { > + if (argc < i + 5 || lo.label) { > + r = CMD_RET_USAGE; > + goto out; > + } > + id = (int)simple_strtoul(argv[i + 1], &endp, 16); > + if (*endp != '\0' || id > 0xffff) > + return CMD_RET_USAGE; > + > + sprintf(var_name, "Boot%04X", id); > + p = var_name16; > + utf8_utf16_strncpy(&p, var_name, 9); > + > + /* label */ > + label_len = strlen(argv[i + 2]); > + label_len16 = utf8_utf16_strnlen(argv[i + 2], label_len); > + label = malloc((label_len16 + 1) * sizeof(u16)); > + if (!label) > + return CMD_RET_FAILURE; > + lo.label = label; /* label will be changed below */ > + utf8_utf16_strncpy(&label, argv[i + 2], label_len); > + > + /* file path */ > + ret = efi_dp_from_name(argv[i + 3], argv[i + 4], > + argv[i + 5], &device_path, > + &file_path); > + if (ret != EFI_SUCCESS) { > + printf("Cannot create device path for \"%s %s\"\n", > + argv[3], argv[4]); > + r = CMD_RET_FAILURE; > + goto out; > + break; > + } > + fp_size = efi_dp_size(file_path) + > + sizeof(struct efi_device_path); > + } > + } > > - /* file path */ > - ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path, > - &file_path); > - if (ret != EFI_SUCCESS) { > - printf("Cannot create device path for \"%s %s\"\n", > - argv[3], argv[4]); > + if (!file_path) { > + printf("You need to specify an image before an initrd.\n"); > r = CMD_RET_FAILURE; > goto out; > } > - lo.file_path = file_path; > - lo.file_path_length = efi_dp_size(file_path) > - + sizeof(struct efi_device_path); /* for END */ > > - /* optional data */ > - if (argc == 6) > - lo.optional_data = NULL; > - else > - lo.optional_data = (const u8 *)argv[6]; > + /* add now add initrd and extra data */ > + for (i = 0; i < argc; i++) { > + if (!strcmp(argv[i], "-i")) { > + if (argc < i + 3 || final_fp) { > + r = CMD_RET_USAGE; > + goto out; > + } > + > + final_fp = add_initrd_instance(argv[i + 1], argv[i + 2], > + argv[i + 3], file_path, > + &fp_size); > + if (IS_ERR(final_fp)) { > + r = CMD_RET_FAILURE; > + goto out; > + } > + > + /* add_initrd_instance allocates a new device path */ > + efi_free_pool(file_path); > + file_path = final_fp; > + } else if (!strcmp(argv[i], "-s")) { > + if (argc < i + 1 || lo.optional_data) { > + r = CMD_RET_USAGE; > + goto out; > + } > + lo.optional_data = (const u8 *)argv[i + 1]; > + } > + } > + > + lo.file_path = file_path; > + lo.file_path_length = fp_size; > > size = efi_serialize_load_option(&lo, (u8 **)&data); > if (!size) { > @@ -951,11 +1058,14 @@ static int do_efi_boot_rm(struct cmd_tbl *cmdtp, int flag, > */ > static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t *size) > { > + struct efi_device_path *initrd_path = NULL; > struct efi_load_option lo; > char *label, *p; > size_t label_len16, label_len; > u16 *dp_str; > efi_status_t ret; > + efi_uintn_t initrd_dp_size; > + const efi_guid_t lf2_initrd_guid = EFI_INITRD_MEDIA_GUID; > > ret = efi_deserialize_load_option(&lo, data, size); > if (ret != EFI_SUCCESS) { > @@ -986,6 +1096,14 @@ static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t *size) > printf(" file_path: %ls\n", dp_str); > efi_free_pool(dp_str); > > + initrd_path = efi_dp_from_lo(&lo, &initrd_dp_size, lf2_initrd_guid); > + if (initrd_path) { > + dp_str = efi_dp_str(initrd_path); > + printf(" initrd_path: %ls\n", dp_str); > + efi_free_pool(dp_str); > + efi_free_pool(initrd_path); > + } > + > printf(" data:\n"); > print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1, > lo.optional_data, *size, true); > @@ -1555,7 +1673,10 @@ static int do_efidebug(struct cmd_tbl *cmdtp, int flag, > static char efidebug_help_text[] = > " - UEFI Shell-like interface to configure UEFI environment\n" > "\n" > - "efidebug boot add <bootid> <label> <interface> <devnum>[:<part>] <file path> [<load options>]\n" > + "efidebug boot add " > + "-b <bootid> <label> <interface> <devnum>[:<part>] <file path> " > + "-i <interface> <devnum>[:<part>] <initrd file path> " > + "-s '<optional data>'\n" > " - set UEFI BootXXXX variable\n" > " <load options> will be passed to UEFI application\n" > "efidebug boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n" > @@ -1599,7 +1720,7 @@ static char efidebug_help_text[] = > #endif > > U_BOOT_CMD( > - efidebug, 10, 0, do_efidebug, > + efidebug, CONFIG_SYS_MAXARGS, 0, do_efidebug, > "Configure UEFI environment", > efidebug_help_text > ); > diff --git a/doc/board/emulation/qemu_capsule_update.rst b/doc/board/emulation/qemu_capsule_update.rst > index 9fec75f8f1c9..33ce4bcd32ea 100644 > --- a/doc/board/emulation/qemu_capsule_update.rst > +++ b/doc/board/emulation/qemu_capsule_update.rst > @@ -60,7 +60,7 @@ to be pointing to the EFI System Partition which contains the capsule > file. The BootNext, BootXXXX and OsIndications variables can be set > using the following commands:: > > - => efidebug boot add 0 Boot0000 virtio 0:1 <capsule_file_name> > + => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name> > => efidebug boot next 0 > => setenv -e -nv -bs -rt -v OsIndications =0x04 > => saveenv > @@ -198,7 +198,7 @@ command line:: > 3. Set the following environment and UEFI boot variables > > => setenv -e -nv -bs -rt -v OsIndications =0x04 > - => efidebug boot add 0 Boot0000 virtio 0:1 <capsule_file_name> > + => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name> > => efidebug boot next 0 > => saveenv > > diff --git a/doc/uefi/uefi.rst b/doc/uefi/uefi.rst > index 5a67737c1579..b3494c22e073 100644 > --- a/doc/uefi/uefi.rst > +++ b/doc/uefi/uefi.rst > @@ -178,7 +178,7 @@ Now in U-Boot install the keys on your board:: > > Set up boot parameters on your board:: > > - efidebug boot add 1 HELLO mmc 0:1 /helloworld.efi.signed "" > + efidebug boot add -b 1 HELLO mmc 0:1 /helloworld.efi.signed "" > > Now your board can run the signed image via the boot manager (see below). > You can also try this sequence by running Pytest, test_efi_secboot, > diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware.py b/test/py/tests/test_efi_capsule/test_capsule_firmware.py > index f006fa95d650..e8b0a1575453 100644 > --- a/test/py/tests/test_efi_capsule/test_capsule_firmware.py > +++ b/test/py/tests/test_efi_capsule/test_capsule_firmware.py > @@ -39,7 +39,7 @@ class TestEfiCapsuleFirmwareFit(object): > with u_boot_console.log.section('Test Case 1-a, before reboot'): > output = u_boot_console.run_command_list([ > 'host bind 0 %s' % disk_img, > - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""', > + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""', > 'efidebug boot order 1', > 'env set -e OsIndications', > 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"', > @@ -114,7 +114,7 @@ class TestEfiCapsuleFirmwareFit(object): > with u_boot_console.log.section('Test Case 2-a, before reboot'): > output = u_boot_console.run_command_list([ > 'host bind 0 %s' % disk_img, > - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""', > + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""', > 'efidebug boot order 1', > 'env set -e -nv -bs -rt OsIndications =0x0000000000000004', > 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"', > @@ -188,7 +188,7 @@ class TestEfiCapsuleFirmwareFit(object): > with u_boot_console.log.section('Test Case 3-a, before reboot'): > output = u_boot_console.run_command_list([ > 'host bind 0 %s' % disk_img, > - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""', > + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""', > 'efidebug boot order 1', > 'env set -e -nv -bs -rt OsIndications =0x0000000000000004', > 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"', > diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py > index 863685e215b7..75f5ea772300 100644 > --- a/test/py/tests/test_efi_secboot/test_signed.py > +++ b/test/py/tests/test_efi_secboot/test_signed.py > @@ -28,7 +28,7 @@ class TestEfiSignedImage(object): > # Test Case 1a, run signed image if no PK > output = u_boot_console.run_command_list([ > 'host bind 0 %s' % disk_img, > - 'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""', > + 'efidebug boot add -b 1 HELLO1 host 0:1 /helloworld.efi.signed ""', > 'efidebug boot next 1', > 'bootefi bootmgr']) > assert 'Hello, world!' in ''.join(output) > @@ -36,7 +36,7 @@ class TestEfiSignedImage(object): > with u_boot_console.log.section('Test Case 1b'): > # Test Case 1b, run unsigned image if no PK > output = u_boot_console.run_command_list([ > - 'efidebug boot add 2 HELLO2 host 0:1 /helloworld.efi ""', > + 'efidebug boot add -b 2 HELLO2 host 0:1 /helloworld.efi ""', > 'efidebug boot next 2', > 'bootefi bootmgr']) > assert 'Hello, world!' in ''.join(output) > @@ -58,13 +58,13 @@ class TestEfiSignedImage(object): > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > assert 'Failed to set EFI variable' not in ''.join(output) > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""', > + 'efidebug boot add -b 1 HELLO1 host 0:1 /helloworld.efi.signed ""', > 'efidebug boot next 1', > 'efidebug test bootmgr']) > assert('\'HELLO1\' failed' in ''.join(output)) > assert('efi_start_image() returned: 26' in ''.join(output)) > output = u_boot_console.run_command_list([ > - 'efidebug boot add 2 HELLO2 host 0:1 /helloworld.efi ""', > + 'efidebug boot add -b 2 HELLO2 host 0:1 /helloworld.efi ""', > 'efidebug boot next 2', > 'efidebug test bootmgr']) > assert '\'HELLO2\' failed' in ''.join(output) > @@ -104,7 +104,7 @@ class TestEfiSignedImage(object): > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > assert 'Failed to set EFI variable' not in ''.join(output) > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""', > 'efidebug boot next 1', > 'efidebug test bootmgr']) > assert '\'HELLO\' failed' in ''.join(output) > @@ -142,7 +142,7 @@ class TestEfiSignedImage(object): > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > assert 'Failed to set EFI variable' not in ''.join(output) > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""', > 'efidebug boot next 1', > 'efidebug test bootmgr']) > assert '\'HELLO\' failed' in ''.join(output) > @@ -169,7 +169,7 @@ class TestEfiSignedImage(object): > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > assert 'Failed to set EFI variable' not in ''.join(output) > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed_2sigs ""', > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed_2sigs ""', > 'efidebug boot next 1', > 'efidebug test bootmgr']) > assert 'Hello, world!' in ''.join(output) > @@ -227,7 +227,7 @@ class TestEfiSignedImage(object): > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > assert 'Failed to set EFI variable' not in ''.join(output) > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""', > 'efidebug boot next 1', > 'bootefi bootmgr']) > assert 'Hello, world!' in ''.join(output) > diff --git a/test/py/tests/test_efi_secboot/test_signed_intca.py b/test/py/tests/test_efi_secboot/test_signed_intca.py > index 70d6be00e8a8..0849572a5143 100644 > --- a/test/py/tests/test_efi_secboot/test_signed_intca.py > +++ b/test/py/tests/test_efi_secboot/test_signed_intca.py > @@ -39,7 +39,7 @@ class TestEfiSignedImageIntca(object): > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""', > + 'efidebug boot add -b 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""', > 'efidebug boot next 1', > 'efidebug test bootmgr']) > assert '\'HELLO_a\' failed' in ''.join(output) > @@ -48,7 +48,7 @@ class TestEfiSignedImageIntca(object): > with u_boot_console.log.section('Test Case 1b'): > # Test Case 1b, signed and authenticated by root CA > output = u_boot_console.run_command_list([ > - 'efidebug boot add 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""', > + 'efidebug boot add -b 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""', > 'efidebug boot next 2', > 'bootefi bootmgr']) > assert 'Hello, world!' in ''.join(output) > @@ -70,7 +70,7 @@ class TestEfiSignedImageIntca(object): > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', > + 'efidebug boot add -b 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', > 'efidebug boot next 1', > 'efidebug test bootmgr']) > assert '\'HELLO_abc\' failed' in ''.join(output) > @@ -116,7 +116,7 @@ class TestEfiSignedImageIntca(object): > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', > + 'efidebug boot add -b 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', > 'efidebug boot next 1', > 'efidebug test bootmgr']) > assert 'Hello, world!' in ''.join(output) > diff --git a/test/py/tests/test_efi_secboot/test_unsigned.py b/test/py/tests/test_efi_secboot/test_unsigned.py > index 56f56e19eb84..8e026f7566ad 100644 > --- a/test/py/tests/test_efi_secboot/test_unsigned.py > +++ b/test/py/tests/test_efi_secboot/test_unsigned.py > @@ -35,7 +35,7 @@ class TestEfiUnsignedImage(object): > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', > 'efidebug boot next 1', > 'bootefi bootmgr']) > assert '\'HELLO\' failed' in ''.join(output) > @@ -64,7 +64,7 @@ class TestEfiUnsignedImage(object): > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', > 'efidebug boot next 1', > 'bootefi bootmgr']) > assert 'Hello, world!' in ''.join(output) > @@ -88,7 +88,7 @@ class TestEfiUnsignedImage(object): > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', > 'efidebug boot next 1', > 'bootefi bootmgr']) > assert '\'HELLO\' failed' in ''.join(output) > @@ -106,7 +106,7 @@ class TestEfiUnsignedImage(object): > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', > 'efidebug boot next 1', > 'bootefi bootmgr']) > assert '\'HELLO\' failed' in ''.join(output) > -- > 2.30.1 >
Akashi-san > > [...] > > +/** > > + * add_initrd_instance() - Append a device path to load_options pointing to an > > + * inirtd > > + if (!initrd_dp) { > > + printf("Cannot append media vendor device path path\n"); > > + goto out; > > + } > > + final_fp = efi_dp_concat(fp, initrd_dp); > > + *fp_size = efi_dp_size(fp) + efi_dp_size(initrd_dp) + > > + (2 * sizeof(struct efi_device_path)); > > + > > +out: > > + efi_free_pool(initrd_dp); > > + efi_free_pool(tmp_dp); > > + efi_free_pool(tmp_fp); > > + return final_fp ? final_fp : ERR_PTR(-EINVAL); > > +} > > + > > /** > > * do_efi_boot_add() - set UEFI load option > > * > > @@ -806,7 +868,9 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag, > > * > > * Implement efidebug "boot add" sub-command. Create or change UEFI load option. > > * > > - * efidebug boot add <id> <label> <interface> <devnum>[:<part>] <file> <options> > > + * efidebug boot add -b <id> <label> <interface> <devnum>[:<part>] <file> > > + * -i <file> <interface2> <devnum2>[:<part>] <initrd> > > + * -s '<options>' > > We discussed another syntax: > efidebug boot add <id> ... > efidebug boot add-initrd <id> <initrd path> > (Please don't care detailed syntax for now.) Yep and I completely agree this is a better format but ... > > What is the difficulty that you have had to implement this type of > interface? The problem is that the initrd and kernel eventually go into *one* Boot#### variable. So it's much easier to treat them in a single command as a bundle. Otherwise you'll have to start adding checks on the initrd code to make sure the Boot### variable exists before you append an initrd. Then you have to deserialize the existing stored device path in Boot####, append the initrd and serialize it again (and last time I checked this is not as easy as it sounds). Also what happens if you edit a Boot#### option and that option has an initrd? You have to pick up the existing initrd and move it over? Or do we silently delete it? Something like: efidebug boot add 0001 efidebug boot add-initrd 0001 efidebug boot add 0001 or even efidebug boot add 0001 efidebug boot add-initrd 0001 efidebug boot add-initrd 0001 Again it looks better. It's probably easier to use, but this is an efidebug command. efibootmgr etc is supposed to set this correctly, so does the extra effort and code worth it? > > Even if we follow your new syntax, > Why do we need '-b' option? > "<id> <label> <interface> <devnum>[:<part>] <file>" are all mandatory arguments, > aren't they? Yes, it's just felt a bit more natural to add args on everything. If other feel the same way I can obviously remove it. Regards /Ilias > > -Takahiro Akashi > > > > > */ > > static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, > > int argc, char *const argv[]) > > @@ -819,55 +883,98 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, > > size_t label_len, label_len16; > > u16 *label; > > struct efi_device_path *device_path = NULL, *file_path = NULL; > > + struct efi_device_path *final_fp = NULL; > > struct efi_load_option lo; > > void *data = NULL; > > efi_uintn_t size; > > + efi_uintn_t fp_size; > > efi_status_t ret; > > int r = CMD_RET_SUCCESS; > > - > > - if (argc < 6 || argc > 7) > > - return CMD_RET_USAGE; > > - > > - id = (int)simple_strtoul(argv[1], &endp, 16); > > - if (*endp != '\0' || id > 0xffff) > > - return CMD_RET_USAGE; > > - > > - sprintf(var_name, "Boot%04X", id); > > - p = var_name16; > > - utf8_utf16_strncpy(&p, var_name, 9); > > + int i; > > > > guid = efi_global_variable_guid; > > > > /* attributes */ > > lo.attributes = LOAD_OPTION_ACTIVE; /* always ACTIVE */ > > + lo.optional_data = NULL; > > + lo.label = NULL; > > > > - /* label */ > > - label_len = strlen(argv[2]); > > - label_len16 = utf8_utf16_strnlen(argv[2], label_len); > > - label = malloc((label_len16 + 1) * sizeof(u16)); > > - if (!label) > > - return CMD_RET_FAILURE; > > - lo.label = label; /* label will be changed below */ > > - utf8_utf16_strncpy(&label, argv[2], label_len); > > + /* search for -b first since the rest of the arguments depends on that */ > > + for (i = 0; i < argc; i++) { > > + if (!strcmp(argv[i], "-b")) { > > + if (argc < i + 5 || lo.label) { > > + r = CMD_RET_USAGE; > > + goto out; > > + } > > + id = (int)simple_strtoul(argv[i + 1], &endp, 16); > > + if (*endp != '\0' || id > 0xffff) > > + return CMD_RET_USAGE; > > + > > + sprintf(var_name, "Boot%04X", id); > > + p = var_name16; > > + utf8_utf16_strncpy(&p, var_name, 9); > > + > > + /* label */ > > + label_len = strlen(argv[i + 2]); > > + label_len16 = utf8_utf16_strnlen(argv[i + 2], label_len); > > + label = malloc((label_len16 + 1) * sizeof(u16)); > > + if (!label) > > + return CMD_RET_FAILURE; > > + lo.label = label; /* label will be changed below */ > > + utf8_utf16_strncpy(&label, argv[i + 2], label_len); > > + > > + /* file path */ > > + ret = efi_dp_from_name(argv[i + 3], argv[i + 4], > > + argv[i + 5], &device_path, > > + &file_path); > > + if (ret != EFI_SUCCESS) { > > + printf("Cannot create device path for \"%s %s\"\n", > > + argv[3], argv[4]); > > + r = CMD_RET_FAILURE; > > + goto out; > > + break; > > + } > > + fp_size = efi_dp_size(file_path) + > > + sizeof(struct efi_device_path); > > + } > > + } > > > > - /* file path */ > > - ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path, > > - &file_path); > > - if (ret != EFI_SUCCESS) { > > - printf("Cannot create device path for \"%s %s\"\n", > > - argv[3], argv[4]); > > + if (!file_path) { > > + printf("You need to specify an image before an initrd.\n"); > > r = CMD_RET_FAILURE; > > goto out; > > } > > - lo.file_path = file_path; > > - lo.file_path_length = efi_dp_size(file_path) > > - + sizeof(struct efi_device_path); /* for END */ > > > > - /* optional data */ > > - if (argc == 6) > > - lo.optional_data = NULL; > > - else > > - lo.optional_data = (const u8 *)argv[6]; > > + /* add now add initrd and extra data */ > > + for (i = 0; i < argc; i++) { > > + if (!strcmp(argv[i], "-i")) { > > + if (argc < i + 3 || final_fp) { > > + r = CMD_RET_USAGE; > > + goto out; > > + } > > + > > + final_fp = add_initrd_instance(argv[i + 1], argv[i + 2], > > + argv[i + 3], file_path, > > + &fp_size); > > + if (IS_ERR(final_fp)) { > > + r = CMD_RET_FAILURE; > > + goto out; > > + } > > + > > + /* add_initrd_instance allocates a new device path */ > > + efi_free_pool(file_path); > > + file_path = final_fp; > > + } else if (!strcmp(argv[i], "-s")) { > > + if (argc < i + 1 || lo.optional_data) { > > + r = CMD_RET_USAGE; > > + goto out; > > + } > > + lo.optional_data = (const u8 *)argv[i + 1]; > > + } > > + } > > + > > + lo.file_path = file_path; > > + lo.file_path_length = fp_size; > > > > size = efi_serialize_load_option(&lo, (u8 **)&data); > > if (!size) { > > @@ -951,11 +1058,14 @@ static int do_efi_boot_rm(struct cmd_tbl *cmdtp, int flag, > > */ > > static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t *size) > > { > > + struct efi_device_path *initrd_path = NULL; > > struct efi_load_option lo; > > char *label, *p; > > size_t label_len16, label_len; > > u16 *dp_str; > > efi_status_t ret; > > + efi_uintn_t initrd_dp_size; > > + const efi_guid_t lf2_initrd_guid = EFI_INITRD_MEDIA_GUID; > > > > ret = efi_deserialize_load_option(&lo, data, size); > > if (ret != EFI_SUCCESS) { > > @@ -986,6 +1096,14 @@ static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t *size) > > printf(" file_path: %ls\n", dp_str); > > efi_free_pool(dp_str); > > > > + initrd_path = efi_dp_from_lo(&lo, &initrd_dp_size, lf2_initrd_guid); > > + if (initrd_path) { > > + dp_str = efi_dp_str(initrd_path); > > + printf(" initrd_path: %ls\n", dp_str); > > + efi_free_pool(dp_str); > > + efi_free_pool(initrd_path); > > + } > > + > > printf(" data:\n"); > > print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1, > > lo.optional_data, *size, true); > > @@ -1555,7 +1673,10 @@ static int do_efidebug(struct cmd_tbl *cmdtp, int flag, > > static char efidebug_help_text[] = > > " - UEFI Shell-like interface to configure UEFI environment\n" > > "\n" > > - "efidebug boot add <bootid> <label> <interface> <devnum>[:<part>] <file path> [<load options>]\n" > > + "efidebug boot add " > > + "-b <bootid> <label> <interface> <devnum>[:<part>] <file path> " > > + "-i <interface> <devnum>[:<part>] <initrd file path> " > > + "-s '<optional data>'\n" > > " - set UEFI BootXXXX variable\n" > > " <load options> will be passed to UEFI application\n" > > "efidebug boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n" > > @@ -1599,7 +1720,7 @@ static char efidebug_help_text[] = > > #endif > > > > U_BOOT_CMD( > > - efidebug, 10, 0, do_efidebug, > > + efidebug, CONFIG_SYS_MAXARGS, 0, do_efidebug, > > "Configure UEFI environment", > > efidebug_help_text > > ); > > diff --git a/doc/board/emulation/qemu_capsule_update.rst b/doc/board/emulation/qemu_capsule_update.rst > > index 9fec75f8f1c9..33ce4bcd32ea 100644 > > --- a/doc/board/emulation/qemu_capsule_update.rst > > +++ b/doc/board/emulation/qemu_capsule_update.rst > > @@ -60,7 +60,7 @@ to be pointing to the EFI System Partition which contains the capsule > > file. The BootNext, BootXXXX and OsIndications variables can be set > > using the following commands:: > > > > - => efidebug boot add 0 Boot0000 virtio 0:1 <capsule_file_name> > > + => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name> > > => efidebug boot next 0 > > => setenv -e -nv -bs -rt -v OsIndications =0x04 > > => saveenv > > @@ -198,7 +198,7 @@ command line:: > > 3. Set the following environment and UEFI boot variables > > > > => setenv -e -nv -bs -rt -v OsIndications =0x04 > > - => efidebug boot add 0 Boot0000 virtio 0:1 <capsule_file_name> > > + => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name> > > => efidebug boot next 0 > > => saveenv > > > > diff --git a/doc/uefi/uefi.rst b/doc/uefi/uefi.rst > > index 5a67737c1579..b3494c22e073 100644 > > --- a/doc/uefi/uefi.rst > > +++ b/doc/uefi/uefi.rst > > @@ -178,7 +178,7 @@ Now in U-Boot install the keys on your board:: > > > > Set up boot parameters on your board:: > > > > - efidebug boot add 1 HELLO mmc 0:1 /helloworld.efi.signed "" > > + efidebug boot add -b 1 HELLO mmc 0:1 /helloworld.efi.signed "" > > > > Now your board can run the signed image via the boot manager (see below). > > You can also try this sequence by running Pytest, test_efi_secboot, > > diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware.py b/test/py/tests/test_efi_capsule/test_capsule_firmware.py > > index f006fa95d650..e8b0a1575453 100644 > > --- a/test/py/tests/test_efi_capsule/test_capsule_firmware.py > > +++ b/test/py/tests/test_efi_capsule/test_capsule_firmware.py > > @@ -39,7 +39,7 @@ class TestEfiCapsuleFirmwareFit(object): > > with u_boot_console.log.section('Test Case 1-a, before reboot'): > > output = u_boot_console.run_command_list([ > > 'host bind 0 %s' % disk_img, > > - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""', > > + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""', > > 'efidebug boot order 1', > > 'env set -e OsIndications', > > 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"', > > @@ -114,7 +114,7 @@ class TestEfiCapsuleFirmwareFit(object): > > with u_boot_console.log.section('Test Case 2-a, before reboot'): > > output = u_boot_console.run_command_list([ > > 'host bind 0 %s' % disk_img, > > - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""', > > + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""', > > 'efidebug boot order 1', > > 'env set -e -nv -bs -rt OsIndications =0x0000000000000004', > > 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"', > > @@ -188,7 +188,7 @@ class TestEfiCapsuleFirmwareFit(object): > > with u_boot_console.log.section('Test Case 3-a, before reboot'): > > output = u_boot_console.run_command_list([ > > 'host bind 0 %s' % disk_img, > > - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""', > > + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""', > > 'efidebug boot order 1', > > 'env set -e -nv -bs -rt OsIndications =0x0000000000000004', > > 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"', > > diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py > > index 863685e215b7..75f5ea772300 100644 > > --- a/test/py/tests/test_efi_secboot/test_signed.py > > +++ b/test/py/tests/test_efi_secboot/test_signed.py > > @@ -28,7 +28,7 @@ class TestEfiSignedImage(object): > > # Test Case 1a, run signed image if no PK > > output = u_boot_console.run_command_list([ > > 'host bind 0 %s' % disk_img, > > - 'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""', > > + 'efidebug boot add -b 1 HELLO1 host 0:1 /helloworld.efi.signed ""', > > 'efidebug boot next 1', > > 'bootefi bootmgr']) > > assert 'Hello, world!' in ''.join(output) > > @@ -36,7 +36,7 @@ class TestEfiSignedImage(object): > > with u_boot_console.log.section('Test Case 1b'): > > # Test Case 1b, run unsigned image if no PK > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 2 HELLO2 host 0:1 /helloworld.efi ""', > > + 'efidebug boot add -b 2 HELLO2 host 0:1 /helloworld.efi ""', > > 'efidebug boot next 2', > > 'bootefi bootmgr']) > > assert 'Hello, world!' in ''.join(output) > > @@ -58,13 +58,13 @@ class TestEfiSignedImage(object): > > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""', > > + 'efidebug boot add -b 1 HELLO1 host 0:1 /helloworld.efi.signed ""', > > 'efidebug boot next 1', > > 'efidebug test bootmgr']) > > assert('\'HELLO1\' failed' in ''.join(output)) > > assert('efi_start_image() returned: 26' in ''.join(output)) > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 2 HELLO2 host 0:1 /helloworld.efi ""', > > + 'efidebug boot add -b 2 HELLO2 host 0:1 /helloworld.efi ""', > > 'efidebug boot next 2', > > 'efidebug test bootmgr']) > > assert '\'HELLO2\' failed' in ''.join(output) > > @@ -104,7 +104,7 @@ class TestEfiSignedImage(object): > > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""', > > 'efidebug boot next 1', > > 'efidebug test bootmgr']) > > assert '\'HELLO\' failed' in ''.join(output) > > @@ -142,7 +142,7 @@ class TestEfiSignedImage(object): > > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""', > > 'efidebug boot next 1', > > 'efidebug test bootmgr']) > > assert '\'HELLO\' failed' in ''.join(output) > > @@ -169,7 +169,7 @@ class TestEfiSignedImage(object): > > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed_2sigs ""', > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed_2sigs ""', > > 'efidebug boot next 1', > > 'efidebug test bootmgr']) > > assert 'Hello, world!' in ''.join(output) > > @@ -227,7 +227,7 @@ class TestEfiSignedImage(object): > > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > > assert 'Failed to set EFI variable' not in ''.join(output) > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""', > > 'efidebug boot next 1', > > 'bootefi bootmgr']) > > assert 'Hello, world!' in ''.join(output) > > diff --git a/test/py/tests/test_efi_secboot/test_signed_intca.py b/test/py/tests/test_efi_secboot/test_signed_intca.py > > index 70d6be00e8a8..0849572a5143 100644 > > --- a/test/py/tests/test_efi_secboot/test_signed_intca.py > > +++ b/test/py/tests/test_efi_secboot/test_signed_intca.py > > @@ -39,7 +39,7 @@ class TestEfiSignedImageIntca(object): > > assert 'Failed to set EFI variable' not in ''.join(output) > > > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""', > > + 'efidebug boot add -b 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""', > > 'efidebug boot next 1', > > 'efidebug test bootmgr']) > > assert '\'HELLO_a\' failed' in ''.join(output) > > @@ -48,7 +48,7 @@ class TestEfiSignedImageIntca(object): > > with u_boot_console.log.section('Test Case 1b'): > > # Test Case 1b, signed and authenticated by root CA > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""', > > + 'efidebug boot add -b 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""', > > 'efidebug boot next 2', > > 'bootefi bootmgr']) > > assert 'Hello, world!' in ''.join(output) > > @@ -70,7 +70,7 @@ class TestEfiSignedImageIntca(object): > > assert 'Failed to set EFI variable' not in ''.join(output) > > > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', > > + 'efidebug boot add -b 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', > > 'efidebug boot next 1', > > 'efidebug test bootmgr']) > > assert '\'HELLO_abc\' failed' in ''.join(output) > > @@ -116,7 +116,7 @@ class TestEfiSignedImageIntca(object): > > assert 'Failed to set EFI variable' not in ''.join(output) > > > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', > > + 'efidebug boot add -b 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', > > 'efidebug boot next 1', > > 'efidebug test bootmgr']) > > assert 'Hello, world!' in ''.join(output) > > diff --git a/test/py/tests/test_efi_secboot/test_unsigned.py b/test/py/tests/test_efi_secboot/test_unsigned.py > > index 56f56e19eb84..8e026f7566ad 100644 > > --- a/test/py/tests/test_efi_secboot/test_unsigned.py > > +++ b/test/py/tests/test_efi_secboot/test_unsigned.py > > @@ -35,7 +35,7 @@ class TestEfiUnsignedImage(object): > > assert 'Failed to set EFI variable' not in ''.join(output) > > > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', > > 'efidebug boot next 1', > > 'bootefi bootmgr']) > > assert '\'HELLO\' failed' in ''.join(output) > > @@ -64,7 +64,7 @@ class TestEfiUnsignedImage(object): > > assert 'Failed to set EFI variable' not in ''.join(output) > > > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', > > 'efidebug boot next 1', > > 'bootefi bootmgr']) > > assert 'Hello, world!' in ''.join(output) > > @@ -88,7 +88,7 @@ class TestEfiUnsignedImage(object): > > assert 'Failed to set EFI variable' not in ''.join(output) > > > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', > > 'efidebug boot next 1', > > 'bootefi bootmgr']) > > assert '\'HELLO\' failed' in ''.join(output) > > @@ -106,7 +106,7 @@ class TestEfiUnsignedImage(object): > > assert 'Failed to set EFI variable' not in ''.join(output) > > > > output = u_boot_console.run_command_list([ > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', > > 'efidebug boot next 1', > > 'bootefi bootmgr']) > > assert '\'HELLO\' failed' in ''.join(output) > > -- > > 2.30.1 > >
On Fri, Mar 12, 2021 at 06:55:57AM +0200, Ilias Apalodimas wrote: > Akashi-san > > > > > [...] > > > +/** > > > + * add_initrd_instance() - Append a device path to load_options pointing to an > > > + * inirtd > > > + if (!initrd_dp) { > > > + printf("Cannot append media vendor device path path\n"); > > > + goto out; > > > + } > > > + final_fp = efi_dp_concat(fp, initrd_dp); > > > + *fp_size = efi_dp_size(fp) + efi_dp_size(initrd_dp) + > > > + (2 * sizeof(struct efi_device_path)); > > > + > > > +out: > > > + efi_free_pool(initrd_dp); > > > + efi_free_pool(tmp_dp); > > > + efi_free_pool(tmp_fp); > > > + return final_fp ? final_fp : ERR_PTR(-EINVAL); > > > +} > > > + > > > /** > > > * do_efi_boot_add() - set UEFI load option > > > * > > > @@ -806,7 +868,9 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag, > > > * > > > * Implement efidebug "boot add" sub-command. Create or change UEFI load option. > > > * > > > - * efidebug boot add <id> <label> <interface> <devnum>[:<part>] <file> <options> > > > + * efidebug boot add -b <id> <label> <interface> <devnum>[:<part>] <file> > > > + * -i <file> <interface2> <devnum2>[:<part>] <initrd> > > > + * -s '<options>' > > > > We discussed another syntax: > > efidebug boot add <id> ... > > efidebug boot add-initrd <id> <initrd path> > > (Please don't care detailed syntax for now.) > > Yep and I completely agree this is a better format but ... > > > > > What is the difficulty that you have had to implement this type of > > interface? > > The problem is that the initrd and kernel eventually go into *one* Boot#### > variable. So it's much easier to treat them in a single command as a bundle. > > Otherwise you'll have to start adding checks on the initrd code to make > sure the Boot### variable exists before you append an initrd. > Then you have to deserialize the existing stored device path in Boot####, > append the initrd and serialize it again (and last time I checked this is not > as easy as it sounds). If we take the format like: kernel path,end(0xff), VenMedia(INITRD),initrd1 path,end(0xff), VenMedia(INITRD),initrd2 path,end(0xff), all that we have to do is - deserialize the boot option variable, - append initrd device path to a list (after kernel device path), - serialize all the option together, Appending is quite simple, isn't it? (because we will not have to parse a device path list.) > Also what happens if you edit a Boot#### option and that option has an initrd? If I were you, > You have to pick up the existing initrd and move it over? Or do we silently > delete it? > Something like: > efidebug boot add 0001 > efidebug boot add-initrd 0001 > efidebug boot add 0001 even with the current implementation, > efidebug boot add 0001 > efidebug boot add 0001 those sequence will overwrite the existing variable and, deeleting initrd by the second "efidebug boot add" would make sense. > or even > > efidebug boot add 0001 > efidebug boot add-initrd 0001 > efidebug boot add-initrd 0001 I think that it will imply "appending initrd to a device path list." -Takahiro Akashi > Again it looks better. It's probably easier to use, but this is an efidebug > command. efibootmgr etc is supposed to set this correctly, so does the extra > effort and code worth it? > > > > > Even if we follow your new syntax, > > Why do we need '-b' option? > > "<id> <label> <interface> <devnum>[:<part>] <file>" are all mandatory arguments, > > aren't they? > > Yes, it's just felt a bit more natural to add args on everything. If other > feel the same way I can obviously remove it. > > > Regards > /Ilias > > > > -Takahiro Akashi > > > > > > > > > */ > > > static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, > > > int argc, char *const argv[]) > > > @@ -819,55 +883,98 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, > > > size_t label_len, label_len16; > > > u16 *label; > > > struct efi_device_path *device_path = NULL, *file_path = NULL; > > > + struct efi_device_path *final_fp = NULL; > > > struct efi_load_option lo; > > > void *data = NULL; > > > efi_uintn_t size; > > > + efi_uintn_t fp_size; > > > efi_status_t ret; > > > int r = CMD_RET_SUCCESS; > > > - > > > - if (argc < 6 || argc > 7) > > > - return CMD_RET_USAGE; > > > - > > > - id = (int)simple_strtoul(argv[1], &endp, 16); > > > - if (*endp != '\0' || id > 0xffff) > > > - return CMD_RET_USAGE; > > > - > > > - sprintf(var_name, "Boot%04X", id); > > > - p = var_name16; > > > - utf8_utf16_strncpy(&p, var_name, 9); > > > + int i; > > > > > > guid = efi_global_variable_guid; > > > > > > /* attributes */ > > > lo.attributes = LOAD_OPTION_ACTIVE; /* always ACTIVE */ > > > + lo.optional_data = NULL; > > > + lo.label = NULL; > > > > > > - /* label */ > > > - label_len = strlen(argv[2]); > > > - label_len16 = utf8_utf16_strnlen(argv[2], label_len); > > > - label = malloc((label_len16 + 1) * sizeof(u16)); > > > - if (!label) > > > - return CMD_RET_FAILURE; > > > - lo.label = label; /* label will be changed below */ > > > - utf8_utf16_strncpy(&label, argv[2], label_len); > > > + /* search for -b first since the rest of the arguments depends on that */ > > > + for (i = 0; i < argc; i++) { > > > + if (!strcmp(argv[i], "-b")) { > > > + if (argc < i + 5 || lo.label) { > > > + r = CMD_RET_USAGE; > > > + goto out; > > > + } > > > + id = (int)simple_strtoul(argv[i + 1], &endp, 16); > > > + if (*endp != '\0' || id > 0xffff) > > > + return CMD_RET_USAGE; > > > + > > > + sprintf(var_name, "Boot%04X", id); > > > + p = var_name16; > > > + utf8_utf16_strncpy(&p, var_name, 9); > > > + > > > + /* label */ > > > + label_len = strlen(argv[i + 2]); > > > + label_len16 = utf8_utf16_strnlen(argv[i + 2], label_len); > > > + label = malloc((label_len16 + 1) * sizeof(u16)); > > > + if (!label) > > > + return CMD_RET_FAILURE; > > > + lo.label = label; /* label will be changed below */ > > > + utf8_utf16_strncpy(&label, argv[i + 2], label_len); > > > + > > > + /* file path */ > > > + ret = efi_dp_from_name(argv[i + 3], argv[i + 4], > > > + argv[i + 5], &device_path, > > > + &file_path); > > > + if (ret != EFI_SUCCESS) { > > > + printf("Cannot create device path for \"%s %s\"\n", > > > + argv[3], argv[4]); > > > + r = CMD_RET_FAILURE; > > > + goto out; > > > + break; > > > + } > > > + fp_size = efi_dp_size(file_path) + > > > + sizeof(struct efi_device_path); > > > + } > > > + } > > > > > > - /* file path */ > > > - ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path, > > > - &file_path); > > > - if (ret != EFI_SUCCESS) { > > > - printf("Cannot create device path for \"%s %s\"\n", > > > - argv[3], argv[4]); > > > + if (!file_path) { > > > + printf("You need to specify an image before an initrd.\n"); > > > r = CMD_RET_FAILURE; > > > goto out; > > > } > > > - lo.file_path = file_path; > > > - lo.file_path_length = efi_dp_size(file_path) > > > - + sizeof(struct efi_device_path); /* for END */ > > > > > > - /* optional data */ > > > - if (argc == 6) > > > - lo.optional_data = NULL; > > > - else > > > - lo.optional_data = (const u8 *)argv[6]; > > > + /* add now add initrd and extra data */ > > > + for (i = 0; i < argc; i++) { > > > + if (!strcmp(argv[i], "-i")) { > > > + if (argc < i + 3 || final_fp) { > > > + r = CMD_RET_USAGE; > > > + goto out; > > > + } > > > + > > > + final_fp = add_initrd_instance(argv[i + 1], argv[i + 2], > > > + argv[i + 3], file_path, > > > + &fp_size); > > > + if (IS_ERR(final_fp)) { > > > + r = CMD_RET_FAILURE; > > > + goto out; > > > + } > > > + > > > + /* add_initrd_instance allocates a new device path */ > > > + efi_free_pool(file_path); > > > + file_path = final_fp; > > > + } else if (!strcmp(argv[i], "-s")) { > > > + if (argc < i + 1 || lo.optional_data) { > > > + r = CMD_RET_USAGE; > > > + goto out; > > > + } > > > + lo.optional_data = (const u8 *)argv[i + 1]; > > > + } > > > + } > > > + > > > + lo.file_path = file_path; > > > + lo.file_path_length = fp_size; > > > > > > size = efi_serialize_load_option(&lo, (u8 **)&data); > > > if (!size) { > > > @@ -951,11 +1058,14 @@ static int do_efi_boot_rm(struct cmd_tbl *cmdtp, int flag, > > > */ > > > static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t *size) > > > { > > > + struct efi_device_path *initrd_path = NULL; > > > struct efi_load_option lo; > > > char *label, *p; > > > size_t label_len16, label_len; > > > u16 *dp_str; > > > efi_status_t ret; > > > + efi_uintn_t initrd_dp_size; > > > + const efi_guid_t lf2_initrd_guid = EFI_INITRD_MEDIA_GUID; > > > > > > ret = efi_deserialize_load_option(&lo, data, size); > > > if (ret != EFI_SUCCESS) { > > > @@ -986,6 +1096,14 @@ static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t *size) > > > printf(" file_path: %ls\n", dp_str); > > > efi_free_pool(dp_str); > > > > > > + initrd_path = efi_dp_from_lo(&lo, &initrd_dp_size, lf2_initrd_guid); > > > + if (initrd_path) { > > > + dp_str = efi_dp_str(initrd_path); > > > + printf(" initrd_path: %ls\n", dp_str); > > > + efi_free_pool(dp_str); > > > + efi_free_pool(initrd_path); > > > + } > > > + > > > printf(" data:\n"); > > > print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1, > > > lo.optional_data, *size, true); > > > @@ -1555,7 +1673,10 @@ static int do_efidebug(struct cmd_tbl *cmdtp, int flag, > > > static char efidebug_help_text[] = > > > " - UEFI Shell-like interface to configure UEFI environment\n" > > > "\n" > > > - "efidebug boot add <bootid> <label> <interface> <devnum>[:<part>] <file path> [<load options>]\n" > > > + "efidebug boot add " > > > + "-b <bootid> <label> <interface> <devnum>[:<part>] <file path> " > > > + "-i <interface> <devnum>[:<part>] <initrd file path> " > > > + "-s '<optional data>'\n" > > > " - set UEFI BootXXXX variable\n" > > > " <load options> will be passed to UEFI application\n" > > > "efidebug boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n" > > > @@ -1599,7 +1720,7 @@ static char efidebug_help_text[] = > > > #endif > > > > > > U_BOOT_CMD( > > > - efidebug, 10, 0, do_efidebug, > > > + efidebug, CONFIG_SYS_MAXARGS, 0, do_efidebug, > > > "Configure UEFI environment", > > > efidebug_help_text > > > ); > > > diff --git a/doc/board/emulation/qemu_capsule_update.rst b/doc/board/emulation/qemu_capsule_update.rst > > > index 9fec75f8f1c9..33ce4bcd32ea 100644 > > > --- a/doc/board/emulation/qemu_capsule_update.rst > > > +++ b/doc/board/emulation/qemu_capsule_update.rst > > > @@ -60,7 +60,7 @@ to be pointing to the EFI System Partition which contains the capsule > > > file. The BootNext, BootXXXX and OsIndications variables can be set > > > using the following commands:: > > > > > > - => efidebug boot add 0 Boot0000 virtio 0:1 <capsule_file_name> > > > + => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name> > > > => efidebug boot next 0 > > > => setenv -e -nv -bs -rt -v OsIndications =0x04 > > > => saveenv > > > @@ -198,7 +198,7 @@ command line:: > > > 3. Set the following environment and UEFI boot variables > > > > > > => setenv -e -nv -bs -rt -v OsIndications =0x04 > > > - => efidebug boot add 0 Boot0000 virtio 0:1 <capsule_file_name> > > > + => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name> > > > => efidebug boot next 0 > > > => saveenv > > > > > > diff --git a/doc/uefi/uefi.rst b/doc/uefi/uefi.rst > > > index 5a67737c1579..b3494c22e073 100644 > > > --- a/doc/uefi/uefi.rst > > > +++ b/doc/uefi/uefi.rst > > > @@ -178,7 +178,7 @@ Now in U-Boot install the keys on your board:: > > > > > > Set up boot parameters on your board:: > > > > > > - efidebug boot add 1 HELLO mmc 0:1 /helloworld.efi.signed "" > > > + efidebug boot add -b 1 HELLO mmc 0:1 /helloworld.efi.signed "" > > > > > > Now your board can run the signed image via the boot manager (see below). > > > You can also try this sequence by running Pytest, test_efi_secboot, > > > diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware.py b/test/py/tests/test_efi_capsule/test_capsule_firmware.py > > > index f006fa95d650..e8b0a1575453 100644 > > > --- a/test/py/tests/test_efi_capsule/test_capsule_firmware.py > > > +++ b/test/py/tests/test_efi_capsule/test_capsule_firmware.py > > > @@ -39,7 +39,7 @@ class TestEfiCapsuleFirmwareFit(object): > > > with u_boot_console.log.section('Test Case 1-a, before reboot'): > > > output = u_boot_console.run_command_list([ > > > 'host bind 0 %s' % disk_img, > > > - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""', > > > + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""', > > > 'efidebug boot order 1', > > > 'env set -e OsIndications', > > > 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"', > > > @@ -114,7 +114,7 @@ class TestEfiCapsuleFirmwareFit(object): > > > with u_boot_console.log.section('Test Case 2-a, before reboot'): > > > output = u_boot_console.run_command_list([ > > > 'host bind 0 %s' % disk_img, > > > - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""', > > > + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""', > > > 'efidebug boot order 1', > > > 'env set -e -nv -bs -rt OsIndications =0x0000000000000004', > > > 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"', > > > @@ -188,7 +188,7 @@ class TestEfiCapsuleFirmwareFit(object): > > > with u_boot_console.log.section('Test Case 3-a, before reboot'): > > > output = u_boot_console.run_command_list([ > > > 'host bind 0 %s' % disk_img, > > > - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""', > > > + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""', > > > 'efidebug boot order 1', > > > 'env set -e -nv -bs -rt OsIndications =0x0000000000000004', > > > 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"', > > > diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py > > > index 863685e215b7..75f5ea772300 100644 > > > --- a/test/py/tests/test_efi_secboot/test_signed.py > > > +++ b/test/py/tests/test_efi_secboot/test_signed.py > > > @@ -28,7 +28,7 @@ class TestEfiSignedImage(object): > > > # Test Case 1a, run signed image if no PK > > > output = u_boot_console.run_command_list([ > > > 'host bind 0 %s' % disk_img, > > > - 'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""', > > > + 'efidebug boot add -b 1 HELLO1 host 0:1 /helloworld.efi.signed ""', > > > 'efidebug boot next 1', > > > 'bootefi bootmgr']) > > > assert 'Hello, world!' in ''.join(output) > > > @@ -36,7 +36,7 @@ class TestEfiSignedImage(object): > > > with u_boot_console.log.section('Test Case 1b'): > > > # Test Case 1b, run unsigned image if no PK > > > output = u_boot_console.run_command_list([ > > > - 'efidebug boot add 2 HELLO2 host 0:1 /helloworld.efi ""', > > > + 'efidebug boot add -b 2 HELLO2 host 0:1 /helloworld.efi ""', > > > 'efidebug boot next 2', > > > 'bootefi bootmgr']) > > > assert 'Hello, world!' in ''.join(output) > > > @@ -58,13 +58,13 @@ class TestEfiSignedImage(object): > > > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > > > assert 'Failed to set EFI variable' not in ''.join(output) > > > output = u_boot_console.run_command_list([ > > > - 'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""', > > > + 'efidebug boot add -b 1 HELLO1 host 0:1 /helloworld.efi.signed ""', > > > 'efidebug boot next 1', > > > 'efidebug test bootmgr']) > > > assert('\'HELLO1\' failed' in ''.join(output)) > > > assert('efi_start_image() returned: 26' in ''.join(output)) > > > output = u_boot_console.run_command_list([ > > > - 'efidebug boot add 2 HELLO2 host 0:1 /helloworld.efi ""', > > > + 'efidebug boot add -b 2 HELLO2 host 0:1 /helloworld.efi ""', > > > 'efidebug boot next 2', > > > 'efidebug test bootmgr']) > > > assert '\'HELLO2\' failed' in ''.join(output) > > > @@ -104,7 +104,7 @@ class TestEfiSignedImage(object): > > > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > > > assert 'Failed to set EFI variable' not in ''.join(output) > > > output = u_boot_console.run_command_list([ > > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', > > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""', > > > 'efidebug boot next 1', > > > 'efidebug test bootmgr']) > > > assert '\'HELLO\' failed' in ''.join(output) > > > @@ -142,7 +142,7 @@ class TestEfiSignedImage(object): > > > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > > > assert 'Failed to set EFI variable' not in ''.join(output) > > > output = u_boot_console.run_command_list([ > > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', > > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""', > > > 'efidebug boot next 1', > > > 'efidebug test bootmgr']) > > > assert '\'HELLO\' failed' in ''.join(output) > > > @@ -169,7 +169,7 @@ class TestEfiSignedImage(object): > > > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > > > assert 'Failed to set EFI variable' not in ''.join(output) > > > output = u_boot_console.run_command_list([ > > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed_2sigs ""', > > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed_2sigs ""', > > > 'efidebug boot next 1', > > > 'efidebug test bootmgr']) > > > assert 'Hello, world!' in ''.join(output) > > > @@ -227,7 +227,7 @@ class TestEfiSignedImage(object): > > > 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) > > > assert 'Failed to set EFI variable' not in ''.join(output) > > > output = u_boot_console.run_command_list([ > > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', > > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""', > > > 'efidebug boot next 1', > > > 'bootefi bootmgr']) > > > assert 'Hello, world!' in ''.join(output) > > > diff --git a/test/py/tests/test_efi_secboot/test_signed_intca.py b/test/py/tests/test_efi_secboot/test_signed_intca.py > > > index 70d6be00e8a8..0849572a5143 100644 > > > --- a/test/py/tests/test_efi_secboot/test_signed_intca.py > > > +++ b/test/py/tests/test_efi_secboot/test_signed_intca.py > > > @@ -39,7 +39,7 @@ class TestEfiSignedImageIntca(object): > > > assert 'Failed to set EFI variable' not in ''.join(output) > > > > > > output = u_boot_console.run_command_list([ > > > - 'efidebug boot add 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""', > > > + 'efidebug boot add -b 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""', > > > 'efidebug boot next 1', > > > 'efidebug test bootmgr']) > > > assert '\'HELLO_a\' failed' in ''.join(output) > > > @@ -48,7 +48,7 @@ class TestEfiSignedImageIntca(object): > > > with u_boot_console.log.section('Test Case 1b'): > > > # Test Case 1b, signed and authenticated by root CA > > > output = u_boot_console.run_command_list([ > > > - 'efidebug boot add 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""', > > > + 'efidebug boot add -b 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""', > > > 'efidebug boot next 2', > > > 'bootefi bootmgr']) > > > assert 'Hello, world!' in ''.join(output) > > > @@ -70,7 +70,7 @@ class TestEfiSignedImageIntca(object): > > > assert 'Failed to set EFI variable' not in ''.join(output) > > > > > > output = u_boot_console.run_command_list([ > > > - 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', > > > + 'efidebug boot add -b 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', > > > 'efidebug boot next 1', > > > 'efidebug test bootmgr']) > > > assert '\'HELLO_abc\' failed' in ''.join(output) > > > @@ -116,7 +116,7 @@ class TestEfiSignedImageIntca(object): > > > assert 'Failed to set EFI variable' not in ''.join(output) > > > > > > output = u_boot_console.run_command_list([ > > > - 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', > > > + 'efidebug boot add -b 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', > > > 'efidebug boot next 1', > > > 'efidebug test bootmgr']) > > > assert 'Hello, world!' in ''.join(output) > > > diff --git a/test/py/tests/test_efi_secboot/test_unsigned.py b/test/py/tests/test_efi_secboot/test_unsigned.py > > > index 56f56e19eb84..8e026f7566ad 100644 > > > --- a/test/py/tests/test_efi_secboot/test_unsigned.py > > > +++ b/test/py/tests/test_efi_secboot/test_unsigned.py > > > @@ -35,7 +35,7 @@ class TestEfiUnsignedImage(object): > > > assert 'Failed to set EFI variable' not in ''.join(output) > > > > > > output = u_boot_console.run_command_list([ > > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', > > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', > > > 'efidebug boot next 1', > > > 'bootefi bootmgr']) > > > assert '\'HELLO\' failed' in ''.join(output) > > > @@ -64,7 +64,7 @@ class TestEfiUnsignedImage(object): > > > assert 'Failed to set EFI variable' not in ''.join(output) > > > > > > output = u_boot_console.run_command_list([ > > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', > > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', > > > 'efidebug boot next 1', > > > 'bootefi bootmgr']) > > > assert 'Hello, world!' in ''.join(output) > > > @@ -88,7 +88,7 @@ class TestEfiUnsignedImage(object): > > > assert 'Failed to set EFI variable' not in ''.join(output) > > > > > > output = u_boot_console.run_command_list([ > > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', > > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', > > > 'efidebug boot next 1', > > > 'bootefi bootmgr']) > > > assert '\'HELLO\' failed' in ''.join(output) > > > @@ -106,7 +106,7 @@ class TestEfiUnsignedImage(object): > > > assert 'Failed to set EFI variable' not in ''.join(output) > > > > > > output = u_boot_console.run_command_list([ > > > - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', > > > + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', > > > 'efidebug boot next 1', > > > 'bootefi bootmgr']) > > > assert '\'HELLO\' failed' in ''.join(output) > > > -- > > > 2.30.1 > > >
Akashi-san, > On Fri, Mar 12, 2021 at 02:23:13PM +0900, AKASHI Takahiro wrote: > On Fri, Mar 12, 2021 at 06:55:57AM +0200, Ilias Apalodimas wrote: > > Akashi-san > > > > > > > > [...] > > > > +/** > > > > + * add_initrd_instance() - Append a device path to load_options pointing to an > > > > + * inirtd > > > > + if (!initrd_dp) { > > > > + printf("Cannot append media vendor device path path\n"); > > > > + goto out; > > > > + } > > > > + final_fp = efi_dp_concat(fp, initrd_dp); > > > > + *fp_size = efi_dp_size(fp) + efi_dp_size(initrd_dp) + > > > > + (2 * sizeof(struct efi_device_path)); > > > > + > > > > +out: > > > > + efi_free_pool(initrd_dp); > > > > + efi_free_pool(tmp_dp); > > > > + efi_free_pool(tmp_fp); > > > > + return final_fp ? final_fp : ERR_PTR(-EINVAL); > > > > +} > > > > + > > > > /** > > > > * do_efi_boot_add() - set UEFI load option > > > > * > > > > @@ -806,7 +868,9 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag, > > > > * > > > > * Implement efidebug "boot add" sub-command. Create or change UEFI load option. > > > > * > > > > - * efidebug boot add <id> <label> <interface> <devnum>[:<part>] <file> <options> > > > > + * efidebug boot add -b <id> <label> <interface> <devnum>[:<part>] <file> > > > > + * -i <file> <interface2> <devnum2>[:<part>] <initrd> > > > > + * -s '<options>' > > > > > > We discussed another syntax: > > > efidebug boot add <id> ... > > > efidebug boot add-initrd <id> <initrd path> > > > (Please don't care detailed syntax for now.) > > > > Yep and I completely agree this is a better format but ... > > > > > > > > What is the difficulty that you have had to implement this type of > > > interface? > > > > The problem is that the initrd and kernel eventually go into *one* Boot#### > > variable. So it's much easier to treat them in a single command as a bundle. > > > > Otherwise you'll have to start adding checks on the initrd code to make > > sure the Boot### variable exists before you append an initrd. > > Then you have to deserialize the existing stored device path in Boot####, > > append the initrd and serialize it again (and last time I checked this is not > > as easy as it sounds). > > If we take the format like: > kernel path,end(0xff), > VenMedia(INITRD),initrd1 path,end(0xff), > VenMedia(INITRD),initrd2 path,end(0xff), > > all that we have to do is > - deserialize the boot option variable, > - append initrd device path to a list (after kernel device path), > - serialize all the option together, Sure but the serialize/deserialize is not as easy as it sounds and requires code as well for the optional data etc. Again I am not saying it's not doable, or even more elegant. I am saying the extra code doesn't seemt to worth the effort right now. Especially since we support a *single* initrd on the loading anyway and no dtbs. > > Appending is quite simple, isn't it? > (because we will not have to parse a device path list.) > Yes and i've mentioned most of this on the mailing list. We did choose the other format though... > > Also what happens if you edit a Boot#### option and that option has an initrd? > If I were you, > > > You have to pick up the existing initrd and move it over? Or do we silently > > delete it? > > Something like: > > efidebug boot add 0001 > > efidebug boot add-initrd 0001 > > efidebug boot add 0001 > > even with the current implementation, > > efidebug boot add 0001 > > efidebug boot add 0001 > those sequence will overwrite the existing variable and, > deeleting initrd by the second "efidebug boot add" would make sense. Yea but that feels 'natural' because it's a single command. Which is also the case with the current code. In multiple commands you wouldn't expect the initrd to away, unless you knew it was stored in a Boot option. Thanks /Ilias
On Fri, Mar 12, 2021 at 07:37:55AM +0200, Ilias Apalodimas wrote: > Akashi-san, > > On Fri, Mar 12, 2021 at 02:23:13PM +0900, AKASHI Takahiro wrote: > > On Fri, Mar 12, 2021 at 06:55:57AM +0200, Ilias Apalodimas wrote: > > > Akashi-san > > > > > > > > > > > [...] > > > > > +/** > > > > > + * add_initrd_instance() - Append a device path to load_options pointing to an > > > > > + * inirtd > > > > > + if (!initrd_dp) { > > > > > + printf("Cannot append media vendor device path path\n"); > > > > > + goto out; > > > > > + } > > > > > + final_fp = efi_dp_concat(fp, initrd_dp); > > > > > + *fp_size = efi_dp_size(fp) + efi_dp_size(initrd_dp) + > > > > > + (2 * sizeof(struct efi_device_path)); > > > > > + > > > > > +out: > > > > > + efi_free_pool(initrd_dp); > > > > > + efi_free_pool(tmp_dp); > > > > > + efi_free_pool(tmp_fp); > > > > > + return final_fp ? final_fp : ERR_PTR(-EINVAL); > > > > > +} > > > > > + > > > > > /** > > > > > * do_efi_boot_add() - set UEFI load option > > > > > * > > > > > @@ -806,7 +868,9 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag, > > > > > * > > > > > * Implement efidebug "boot add" sub-command. Create or change UEFI load option. > > > > > * > > > > > - * efidebug boot add <id> <label> <interface> <devnum>[:<part>] <file> <options> > > > > > + * efidebug boot add -b <id> <label> <interface> <devnum>[:<part>] <file> > > > > > + * -i <file> <interface2> <devnum2>[:<part>] <initrd> > > > > > + * -s '<options>' > > > > > > > > We discussed another syntax: > > > > efidebug boot add <id> ... > > > > efidebug boot add-initrd <id> <initrd path> > > > > (Please don't care detailed syntax for now.) > > > > > > Yep and I completely agree this is a better format but ... > > > > > > > > > > > What is the difficulty that you have had to implement this type of > > > > interface? > > > > > > The problem is that the initrd and kernel eventually go into *one* Boot#### > > > variable. So it's much easier to treat them in a single command as a bundle. > > > > > > Otherwise you'll have to start adding checks on the initrd code to make > > > sure the Boot### variable exists before you append an initrd. > > > Then you have to deserialize the existing stored device path in Boot####, > > > append the initrd and serialize it again (and last time I checked this is not > > > as easy as it sounds). > > > > If we take the format like: > > kernel path,end(0xff), > > VenMedia(INITRD),initrd1 path,end(0xff), > > VenMedia(INITRD),initrd2 path,end(0xff), > > > > all that we have to do is > > - deserialize the boot option variable, > > - append initrd device path to a list (after kernel device path), > > - serialize all the option together, > > Sure but the serialize/deserialize is not as easy as it sounds and requires > code as well for the optional data etc. I don't still understand why it's not so easy. Because I'm the author of that function? > Again I am not saying it's not doable, or even more elegant. I am saying the > extra code doesn't seemt to worth the effort right now. Especially since we > support a *single* initrd on the loading anyway and no dtbs. Better user interface would pay the efforts of implementation. > > > > Appending is quite simple, isn't it? > > (because we will not have to parse a device path list.) > > > > Yes and i've mentioned most of this on the mailing list. > We did choose the other format though... Simply, who objected it? I remember that Heinrich has a similar idea. So who else? > > > Also what happens if you edit a Boot#### option and that option has an initrd? > > If I were you, > > > > > You have to pick up the existing initrd and move it over? Or do we silently > > > delete it? > > > Something like: > > > efidebug boot add 0001 > > > efidebug boot add-initrd 0001 > > > efidebug boot add 0001 > > > > even with the current implementation, > > > efidebug boot add 0001 > > > efidebug boot add 0001 > > those sequence will overwrite the existing variable and, > > deeleting initrd by the second "efidebug boot add" would make sense. > > Yea but that feels 'natural' because it's a single command. Which is also the > case with the current code. In multiple commands you wouldn't expect the > initrd to away, unless you knew it was stored in a Boot option. Well, if it's your concern, we can print a warning, say You're trying to rewrite BootXXXX variable (you will lost the existing data). Are you sure [y/n]? But I don't think it's even worth doing so because you can simply type "Ctrl-p" twice at the command line if you want to run "efidebug boot add-initrd ..." again :) -Takahiro Akashi > > Thanks > /Ilias
Akashi-san, [...] > > > > Then you have to deserialize the existing stored device path in Boot####, > > > > append the initrd and serialize it again (and last time I checked this is not > > > > as easy as it sounds). > > > > > > If we take the format like: > > > kernel path,end(0xff), > > > VenMedia(INITRD),initrd1 path,end(0xff), > > > VenMedia(INITRD),initrd2 path,end(0xff), > > > > > > all that we have to do is > > > - deserialize the boot option variable, > > > - append initrd device path to a list (after kernel device path), > > > - serialize all the option together, > > > > Sure but the serialize/deserialize is not as easy as it sounds and requires > > code as well for the optional data etc. > > I don't still understand why it's not so easy. > Because I'm the author of that function? You have to deserialize the load option, split it on the device path end node, attach a new device path, and append the optional data, while doing the necessary utf8/16 conversions at the same time. The alternative is do nothing at all and serialize it in one go. It's not hard, but it's not something it would be my first priority to code. Especially if 99.9% of the use cases will have a single initrd. > > > Again I am not saying it's not doable, or even more elegant. I am saying the > > extra code doesn't seemt to worth the effort right now. Especially since we > > support a *single* initrd on the loading anyway and no dtbs. > > Better user interface would pay the efforts of implementation. > > > > > > > Appending is quite simple, isn't it? > > > (because we will not have to parse a device path list.) > > > > > > > Yes and i've mentioned most of this on the mailing list. > > We did choose the other format though... > > Simply, who objected it? > I remember that Heinrich has a similar idea. > So who else? Again that proposal is part of my mail as well. Heinrich, Samer and I agreed this is the format we want to use since since you dont need to replicate the VenMEdia node for each array element. > > > > > Also what happens if you edit a Boot#### option and that option has an initrd? > > > If I were you, > > > > > > > You have to pick up the existing initrd and move it over? Or do we silently > > > > delete it? > > > > Something like: > > > > efidebug boot add 0001 > > > > efidebug boot add-initrd 0001 > > > > efidebug boot add 0001 > > > > > > even with the current implementation, > > > > efidebug boot add 0001 > > > > efidebug boot add 0001 > > > those sequence will overwrite the existing variable and, > > > deeleting initrd by the second "efidebug boot add" would make sense. > > > > Yea but that feels 'natural' because it's a single command. Which is also the > > case with the current code. In multiple commands you wouldn't expect the > > initrd to away, unless you knew it was stored in a Boot option. > > Well, if it's your concern, we can print a warning, say > You're trying to rewrite BootXXXX variable (you will lost the existing data). > Are you sure [y/n]? > > But I don't think it's even worth doing so > because you can simply type "Ctrl-p" twice at the command line > if you want to run "efidebug boot add-initrd ..." again :) That's the least of my worries tbh. I don't really mind if we delete it silenmtly, as long as it's documented/justified. > > -Takahiro Akashi > > > > Thanks /Ilias
On 12.03.21 05:44, AKASHI Takahiro wrote: > Ilias, > > Sorry, but I will have to repeat my question for better understandings. > > On Sat, Mar 06, 2021 at 12:23:01AM +0200, Ilias Apalodimas wrote: >> The UEFI spec allow a packed array of UEFI device paths in the >> FilePathList[] of an EFI_LOAD_OPTION. The first file path must >> describe the loaded image but the rest are OS specific. >> >> Previous patches parse the device path and try to use the second >> member of the array as an initrd. So let's modify efidebug slightly >> and install the second file described in the command line as the >> initrd device path. >> >> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> >> --- >> cmd/efidebug.c | 193 ++++++++++++++---- >> doc/board/emulation/qemu_capsule_update.rst | 4 +- >> doc/uefi/uefi.rst | 2 +- >> .../test_efi_capsule/test_capsule_firmware.py | 6 +- >> test/py/tests/test_efi_secboot/test_signed.py | 16 +- >> .../test_efi_secboot/test_signed_intca.py | 8 +- >> .../tests/test_efi_secboot/test_unsigned.py | 8 +- >> 7 files changed, 179 insertions(+), 58 deletions(-) >> >> diff --git a/cmd/efidebug.c b/cmd/efidebug.c >> index bbbcb0a54643..9a1c471a3eaa 100644 >> --- a/cmd/efidebug.c >> +++ b/cmd/efidebug.c >> @@ -9,6 +9,8 @@ >> #include <common.h> >> #include <command.h> >> #include <efi_dt_fixup.h> >> +#include <efi_helper.h> >> +#include <efi_load_initrd.h> >> #include <efi_loader.h> >> #include <efi_rng.h> >> #include <exports.h> >> @@ -19,6 +21,7 @@ >> #include <part.h> >> #include <search.h> >> #include <linux/ctype.h> >> +#include <linux/err.h> >> >> #define BS systab.boottime >> #define RT systab.runtime >> @@ -794,6 +797,65 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag, >> return CMD_RET_SUCCESS; >> } >> >> +/** >> + * add_initrd_instance() - Append a device path to load_options pointing to an >> + * inirtd >> + * >> + * @argc: Number of arguments >> + * @argv: Argument array >> + * @file_path Existing device path, the new instance will be appended >> + * Return: Pointer to the device path or ERR_PTR >> + * >> + */ >> +static >> +struct efi_device_path *add_initrd_instance(const char *dev, const char *part, >> + const char *file, >> + const struct efi_device_path *fp, >> + efi_uintn_t *fp_size) >> +{ >> + struct efi_device_path *tmp_dp = NULL, *tmp_fp = NULL; >> + struct efi_device_path *final_fp = NULL, *initrd_dp = NULL; >> + efi_status_t ret; >> + const struct efi_initrd_dp id_dp = { >> + .vendor = { >> + { >> + DEVICE_PATH_TYPE_MEDIA_DEVICE, >> + DEVICE_PATH_SUB_TYPE_VENDOR_PATH, >> + sizeof(id_dp.vendor), >> + }, >> + EFI_INITRD_MEDIA_GUID, >> + }, >> + .end = { >> + DEVICE_PATH_TYPE_END, >> + DEVICE_PATH_SUB_TYPE_END, >> + sizeof(id_dp.end), >> + } >> + }; >> + >> + ret = efi_dp_from_name(dev, part, file, &tmp_dp, &tmp_fp); >> + if (ret != EFI_SUCCESS) { >> + printf("Cannot create device path for \"%s %s\"\n", part, file); >> + goto out; >> + } >> + >> + initrd_dp = >> + efi_dp_append_instance((const struct efi_device_path *)&id_dp, >> + tmp_fp); >> + if (!initrd_dp) { >> + printf("Cannot append media vendor device path path\n"); >> + goto out; >> + } >> + final_fp = efi_dp_concat(fp, initrd_dp); >> + *fp_size = efi_dp_size(fp) + efi_dp_size(initrd_dp) + >> + (2 * sizeof(struct efi_device_path)); >> + >> +out: >> + efi_free_pool(initrd_dp); >> + efi_free_pool(tmp_dp); >> + efi_free_pool(tmp_fp); >> + return final_fp ? final_fp : ERR_PTR(-EINVAL); >> +} >> + >> /** >> * do_efi_boot_add() - set UEFI load option >> * >> @@ -806,7 +868,9 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag, >> * >> * Implement efidebug "boot add" sub-command. Create or change UEFI load option. >> * >> - * efidebug boot add <id> <label> <interface> <devnum>[:<part>] <file> <options> >> + * efidebug boot add -b <id> <label> <interface> <devnum>[:<part>] <file> >> + * -i <file> <interface2> <devnum2>[:<part>] <initrd> >> + * -s '<options>' > > We discussed another syntax: > efidebug boot add <id> ... > efidebug boot add-initrd <id> <initrd path> > (Please don't care detailed syntax for now.) Thanks for revieweing. Ilias and I discussed the syntax several times. As Ilias pointed out the code and the test cases will be a bit simpler if there is only one command. There are less corner cases that we have to test like what happens if add-initrd is called without a prior definition of a binary. I think using multiple commands is harder for the user to comprehend. Setting the Boot#### variables will typically done from Linux when a new kernel arrives or by a capsule update. The efidebug command will be less common. I prefer to stick with Ilias suggestion. > > What is the difficulty that you have had to implement this type of > interface? > > Even if we follow your new syntax, > Why do we need '-b' option? > "<id> <label> <interface> <devnum>[:<part>] <file>" are all mandatory arguments, > aren't they? Yes, a boot option without binary is not allowable. There is no real need for '-b'. We had the same discussion for requiring -bs when setting a variable with setenv -e which is equally superfluous where you insisted on adding the parameter for clarity. Same here. For clarity -b is nice. For code size reduction dropping it makes sense. Best regards Heinrich > > -Takahiro Akashi > > > >> */ >> static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, >> int argc, char *const argv[]) >> @@ -819,55 +883,98 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, >> size_t label_len, label_len16; >> u16 *label; >> struct efi_device_path *device_path = NULL, *file_path = NULL; >> + struct efi_device_path *final_fp = NULL; >> struct efi_load_option lo; >> void *data = NULL; >> efi_uintn_t size; >> + efi_uintn_t fp_size; >> efi_status_t ret; >> int r = CMD_RET_SUCCESS; >> - >> - if (argc < 6 || argc > 7) >> - return CMD_RET_USAGE; >> - >> - id = (int)simple_strtoul(argv[1], &endp, 16); >> - if (*endp != '\0' || id > 0xffff) >> - return CMD_RET_USAGE; >> - >> - sprintf(var_name, "Boot%04X", id); >> - p = var_name16; >> - utf8_utf16_strncpy(&p, var_name, 9); >> + int i; >> >> guid = efi_global_variable_guid; >> >> /* attributes */ >> lo.attributes = LOAD_OPTION_ACTIVE; /* always ACTIVE */ >> + lo.optional_data = NULL; >> + lo.label = NULL; >> >> - /* label */ >> - label_len = strlen(argv[2]); >> - label_len16 = utf8_utf16_strnlen(argv[2], label_len); >> - label = malloc((label_len16 + 1) * sizeof(u16)); >> - if (!label) >> - return CMD_RET_FAILURE; >> - lo.label = label; /* label will be changed below */ >> - utf8_utf16_strncpy(&label, argv[2], label_len); >> + /* search for -b first since the rest of the arguments depends on that */ >> + for (i = 0; i < argc; i++) { >> + if (!strcmp(argv[i], "-b")) { >> + if (argc < i + 5 || lo.label) { >> + r = CMD_RET_USAGE; >> + goto out; >> + } >> + id = (int)simple_strtoul(argv[i + 1], &endp, 16); >> + if (*endp != '\0' || id > 0xffff) >> + return CMD_RET_USAGE; >> + >> + sprintf(var_name, "Boot%04X", id); >> + p = var_name16; >> + utf8_utf16_strncpy(&p, var_name, 9); >> + >> + /* label */ >> + label_len = strlen(argv[i + 2]); >> + label_len16 = utf8_utf16_strnlen(argv[i + 2], label_len); >> + label = malloc((label_len16 + 1) * sizeof(u16)); >> + if (!label) >> + return CMD_RET_FAILURE; >> + lo.label = label; /* label will be changed below */ >> + utf8_utf16_strncpy(&label, argv[i + 2], label_len); >> + >> + /* file path */ >> + ret = efi_dp_from_name(argv[i + 3], argv[i + 4], >> + argv[i + 5], &device_path, >> + &file_path); >> + if (ret != EFI_SUCCESS) { >> + printf("Cannot create device path for \"%s %s\"\n", >> + argv[3], argv[4]); >> + r = CMD_RET_FAILURE; >> + goto out; >> + break; >> + } >> + fp_size = efi_dp_size(file_path) + >> + sizeof(struct efi_device_path); >> + } >> + } >> >> - /* file path */ >> - ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path, >> - &file_path); >> - if (ret != EFI_SUCCESS) { >> - printf("Cannot create device path for \"%s %s\"\n", >> - argv[3], argv[4]); >> + if (!file_path) { >> + printf("You need to specify an image before an initrd.\n"); >> r = CMD_RET_FAILURE; >> goto out; >> } >> - lo.file_path = file_path; >> - lo.file_path_length = efi_dp_size(file_path) >> - + sizeof(struct efi_device_path); /* for END */ >> >> - /* optional data */ >> - if (argc == 6) >> - lo.optional_data = NULL; >> - else >> - lo.optional_data = (const u8 *)argv[6]; >> + /* add now add initrd and extra data */ >> + for (i = 0; i < argc; i++) { >> + if (!strcmp(argv[i], "-i")) { >> + if (argc < i + 3 || final_fp) { >> + r = CMD_RET_USAGE; >> + goto out; >> + } >> + >> + final_fp = add_initrd_instance(argv[i + 1], argv[i + 2], >> + argv[i + 3], file_path, >> + &fp_size); >> + if (IS_ERR(final_fp)) { >> + r = CMD_RET_FAILURE; >> + goto out; >> + } >> + >> + /* add_initrd_instance allocates a new device path */ >> + efi_free_pool(file_path); >> + file_path = final_fp; >> + } else if (!strcmp(argv[i], "-s")) { >> + if (argc < i + 1 || lo.optional_data) { >> + r = CMD_RET_USAGE; >> + goto out; >> + } >> + lo.optional_data = (const u8 *)argv[i + 1]; >> + } >> + } >> + >> + lo.file_path = file_path; >> + lo.file_path_length = fp_size; >> >> size = efi_serialize_load_option(&lo, (u8 **)&data); >> if (!size) { >> @@ -951,11 +1058,14 @@ static int do_efi_boot_rm(struct cmd_tbl *cmdtp, int flag, >> */ >> static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t *size) >> { >> + struct efi_device_path *initrd_path = NULL; >> struct efi_load_option lo; >> char *label, *p; >> size_t label_len16, label_len; >> u16 *dp_str; >> efi_status_t ret; >> + efi_uintn_t initrd_dp_size; >> + const efi_guid_t lf2_initrd_guid = EFI_INITRD_MEDIA_GUID; >> >> ret = efi_deserialize_load_option(&lo, data, size); >> if (ret != EFI_SUCCESS) { >> @@ -986,6 +1096,14 @@ static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t *size) >> printf(" file_path: %ls\n", dp_str); >> efi_free_pool(dp_str); >> >> + initrd_path = efi_dp_from_lo(&lo, &initrd_dp_size, lf2_initrd_guid); >> + if (initrd_path) { >> + dp_str = efi_dp_str(initrd_path); >> + printf(" initrd_path: %ls\n", dp_str); >> + efi_free_pool(dp_str); >> + efi_free_pool(initrd_path); >> + } >> + >> printf(" data:\n"); >> print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1, >> lo.optional_data, *size, true); >> @@ -1555,7 +1673,10 @@ static int do_efidebug(struct cmd_tbl *cmdtp, int flag, >> static char efidebug_help_text[] = >> " - UEFI Shell-like interface to configure UEFI environment\n" >> "\n" >> - "efidebug boot add <bootid> <label> <interface> <devnum>[:<part>] <file path> [<load options>]\n" >> + "efidebug boot add " >> + "-b <bootid> <label> <interface> <devnum>[:<part>] <file path> " >> + "-i <interface> <devnum>[:<part>] <initrd file path> " >> + "-s '<optional data>'\n" >> " - set UEFI BootXXXX variable\n" >> " <load options> will be passed to UEFI application\n" >> "efidebug boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n" >> @@ -1599,7 +1720,7 @@ static char efidebug_help_text[] = >> #endif >> >> U_BOOT_CMD( >> - efidebug, 10, 0, do_efidebug, >> + efidebug, CONFIG_SYS_MAXARGS, 0, do_efidebug, >> "Configure UEFI environment", >> efidebug_help_text >> ); >> diff --git a/doc/board/emulation/qemu_capsule_update.rst b/doc/board/emulation/qemu_capsule_update.rst >> index 9fec75f8f1c9..33ce4bcd32ea 100644 >> --- a/doc/board/emulation/qemu_capsule_update.rst >> +++ b/doc/board/emulation/qemu_capsule_update.rst >> @@ -60,7 +60,7 @@ to be pointing to the EFI System Partition which contains the capsule >> file. The BootNext, BootXXXX and OsIndications variables can be set >> using the following commands:: >> >> - => efidebug boot add 0 Boot0000 virtio 0:1 <capsule_file_name> >> + => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name> >> => efidebug boot next 0 >> => setenv -e -nv -bs -rt -v OsIndications =0x04 >> => saveenv >> @@ -198,7 +198,7 @@ command line:: >> 3. Set the following environment and UEFI boot variables >> >> => setenv -e -nv -bs -rt -v OsIndications =0x04 >> - => efidebug boot add 0 Boot0000 virtio 0:1 <capsule_file_name> >> + => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name> >> => efidebug boot next 0 >> => saveenv >> >> diff --git a/doc/uefi/uefi.rst b/doc/uefi/uefi.rst >> index 5a67737c1579..b3494c22e073 100644 >> --- a/doc/uefi/uefi.rst >> +++ b/doc/uefi/uefi.rst >> @@ -178,7 +178,7 @@ Now in U-Boot install the keys on your board:: >> >> Set up boot parameters on your board:: >> >> - efidebug boot add 1 HELLO mmc 0:1 /helloworld.efi.signed "" >> + efidebug boot add -b 1 HELLO mmc 0:1 /helloworld.efi.signed "" >> >> Now your board can run the signed image via the boot manager (see below). >> You can also try this sequence by running Pytest, test_efi_secboot, >> diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware.py b/test/py/tests/test_efi_capsule/test_capsule_firmware.py >> index f006fa95d650..e8b0a1575453 100644 >> --- a/test/py/tests/test_efi_capsule/test_capsule_firmware.py >> +++ b/test/py/tests/test_efi_capsule/test_capsule_firmware.py >> @@ -39,7 +39,7 @@ class TestEfiCapsuleFirmwareFit(object): >> with u_boot_console.log.section('Test Case 1-a, before reboot'): >> output = u_boot_console.run_command_list([ >> 'host bind 0 %s' % disk_img, >> - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""', >> + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""', >> 'efidebug boot order 1', >> 'env set -e OsIndications', >> 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"', >> @@ -114,7 +114,7 @@ class TestEfiCapsuleFirmwareFit(object): >> with u_boot_console.log.section('Test Case 2-a, before reboot'): >> output = u_boot_console.run_command_list([ >> 'host bind 0 %s' % disk_img, >> - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""', >> + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""', >> 'efidebug boot order 1', >> 'env set -e -nv -bs -rt OsIndications =0x0000000000000004', >> 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"', >> @@ -188,7 +188,7 @@ class TestEfiCapsuleFirmwareFit(object): >> with u_boot_console.log.section('Test Case 3-a, before reboot'): >> output = u_boot_console.run_command_list([ >> 'host bind 0 %s' % disk_img, >> - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""', >> + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""', >> 'efidebug boot order 1', >> 'env set -e -nv -bs -rt OsIndications =0x0000000000000004', >> 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"', >> diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py >> index 863685e215b7..75f5ea772300 100644 >> --- a/test/py/tests/test_efi_secboot/test_signed.py >> +++ b/test/py/tests/test_efi_secboot/test_signed.py >> @@ -28,7 +28,7 @@ class TestEfiSignedImage(object): >> # Test Case 1a, run signed image if no PK >> output = u_boot_console.run_command_list([ >> 'host bind 0 %s' % disk_img, >> - 'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""', >> + 'efidebug boot add -b 1 HELLO1 host 0:1 /helloworld.efi.signed ""', >> 'efidebug boot next 1', >> 'bootefi bootmgr']) >> assert 'Hello, world!' in ''.join(output) >> @@ -36,7 +36,7 @@ class TestEfiSignedImage(object): >> with u_boot_console.log.section('Test Case 1b'): >> # Test Case 1b, run unsigned image if no PK >> output = u_boot_console.run_command_list([ >> - 'efidebug boot add 2 HELLO2 host 0:1 /helloworld.efi ""', >> + 'efidebug boot add -b 2 HELLO2 host 0:1 /helloworld.efi ""', >> 'efidebug boot next 2', >> 'bootefi bootmgr']) >> assert 'Hello, world!' in ''.join(output) >> @@ -58,13 +58,13 @@ class TestEfiSignedImage(object): >> 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) >> assert 'Failed to set EFI variable' not in ''.join(output) >> output = u_boot_console.run_command_list([ >> - 'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""', >> + 'efidebug boot add -b 1 HELLO1 host 0:1 /helloworld.efi.signed ""', >> 'efidebug boot next 1', >> 'efidebug test bootmgr']) >> assert('\'HELLO1\' failed' in ''.join(output)) >> assert('efi_start_image() returned: 26' in ''.join(output)) >> output = u_boot_console.run_command_list([ >> - 'efidebug boot add 2 HELLO2 host 0:1 /helloworld.efi ""', >> + 'efidebug boot add -b 2 HELLO2 host 0:1 /helloworld.efi ""', >> 'efidebug boot next 2', >> 'efidebug test bootmgr']) >> assert '\'HELLO2\' failed' in ''.join(output) >> @@ -104,7 +104,7 @@ class TestEfiSignedImage(object): >> 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) >> assert 'Failed to set EFI variable' not in ''.join(output) >> output = u_boot_console.run_command_list([ >> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', >> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""', >> 'efidebug boot next 1', >> 'efidebug test bootmgr']) >> assert '\'HELLO\' failed' in ''.join(output) >> @@ -142,7 +142,7 @@ class TestEfiSignedImage(object): >> 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) >> assert 'Failed to set EFI variable' not in ''.join(output) >> output = u_boot_console.run_command_list([ >> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', >> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""', >> 'efidebug boot next 1', >> 'efidebug test bootmgr']) >> assert '\'HELLO\' failed' in ''.join(output) >> @@ -169,7 +169,7 @@ class TestEfiSignedImage(object): >> 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) >> assert 'Failed to set EFI variable' not in ''.join(output) >> output = u_boot_console.run_command_list([ >> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed_2sigs ""', >> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed_2sigs ""', >> 'efidebug boot next 1', >> 'efidebug test bootmgr']) >> assert 'Hello, world!' in ''.join(output) >> @@ -227,7 +227,7 @@ class TestEfiSignedImage(object): >> 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) >> assert 'Failed to set EFI variable' not in ''.join(output) >> output = u_boot_console.run_command_list([ >> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', >> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""', >> 'efidebug boot next 1', >> 'bootefi bootmgr']) >> assert 'Hello, world!' in ''.join(output) >> diff --git a/test/py/tests/test_efi_secboot/test_signed_intca.py b/test/py/tests/test_efi_secboot/test_signed_intca.py >> index 70d6be00e8a8..0849572a5143 100644 >> --- a/test/py/tests/test_efi_secboot/test_signed_intca.py >> +++ b/test/py/tests/test_efi_secboot/test_signed_intca.py >> @@ -39,7 +39,7 @@ class TestEfiSignedImageIntca(object): >> assert 'Failed to set EFI variable' not in ''.join(output) >> >> output = u_boot_console.run_command_list([ >> - 'efidebug boot add 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""', >> + 'efidebug boot add -b 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""', >> 'efidebug boot next 1', >> 'efidebug test bootmgr']) >> assert '\'HELLO_a\' failed' in ''.join(output) >> @@ -48,7 +48,7 @@ class TestEfiSignedImageIntca(object): >> with u_boot_console.log.section('Test Case 1b'): >> # Test Case 1b, signed and authenticated by root CA >> output = u_boot_console.run_command_list([ >> - 'efidebug boot add 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""', >> + 'efidebug boot add -b 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""', >> 'efidebug boot next 2', >> 'bootefi bootmgr']) >> assert 'Hello, world!' in ''.join(output) >> @@ -70,7 +70,7 @@ class TestEfiSignedImageIntca(object): >> assert 'Failed to set EFI variable' not in ''.join(output) >> >> output = u_boot_console.run_command_list([ >> - 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', >> + 'efidebug boot add -b 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', >> 'efidebug boot next 1', >> 'efidebug test bootmgr']) >> assert '\'HELLO_abc\' failed' in ''.join(output) >> @@ -116,7 +116,7 @@ class TestEfiSignedImageIntca(object): >> assert 'Failed to set EFI variable' not in ''.join(output) >> >> output = u_boot_console.run_command_list([ >> - 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', >> + 'efidebug boot add -b 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', >> 'efidebug boot next 1', >> 'efidebug test bootmgr']) >> assert 'Hello, world!' in ''.join(output) >> diff --git a/test/py/tests/test_efi_secboot/test_unsigned.py b/test/py/tests/test_efi_secboot/test_unsigned.py >> index 56f56e19eb84..8e026f7566ad 100644 >> --- a/test/py/tests/test_efi_secboot/test_unsigned.py >> +++ b/test/py/tests/test_efi_secboot/test_unsigned.py >> @@ -35,7 +35,7 @@ class TestEfiUnsignedImage(object): >> assert 'Failed to set EFI variable' not in ''.join(output) >> >> output = u_boot_console.run_command_list([ >> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', >> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', >> 'efidebug boot next 1', >> 'bootefi bootmgr']) >> assert '\'HELLO\' failed' in ''.join(output) >> @@ -64,7 +64,7 @@ class TestEfiUnsignedImage(object): >> assert 'Failed to set EFI variable' not in ''.join(output) >> >> output = u_boot_console.run_command_list([ >> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', >> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', >> 'efidebug boot next 1', >> 'bootefi bootmgr']) >> assert 'Hello, world!' in ''.join(output) >> @@ -88,7 +88,7 @@ class TestEfiUnsignedImage(object): >> assert 'Failed to set EFI variable' not in ''.join(output) >> >> output = u_boot_console.run_command_list([ >> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', >> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', >> 'efidebug boot next 1', >> 'bootefi bootmgr']) >> assert '\'HELLO\' failed' in ''.join(output) >> @@ -106,7 +106,7 @@ class TestEfiUnsignedImage(object): >> assert 'Failed to set EFI variable' not in ''.join(output) >> >> output = u_boot_console.run_command_list([ >> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', >> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', >> 'efidebug boot next 1', >> 'bootefi bootmgr']) >> assert '\'HELLO\' failed' in ''.join(output) >> -- >> 2.30.1 >>
diff --git a/cmd/efidebug.c b/cmd/efidebug.c index bbbcb0a54643..9a1c471a3eaa 100644 --- a/cmd/efidebug.c +++ b/cmd/efidebug.c @@ -9,6 +9,8 @@ #include <common.h> #include <command.h> #include <efi_dt_fixup.h> +#include <efi_helper.h> +#include <efi_load_initrd.h> #include <efi_loader.h> #include <efi_rng.h> #include <exports.h> @@ -19,6 +21,7 @@ #include <part.h> #include <search.h> #include <linux/ctype.h> +#include <linux/err.h> #define BS systab.boottime #define RT systab.runtime @@ -794,6 +797,65 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag, return CMD_RET_SUCCESS; } +/** + * add_initrd_instance() - Append a device path to load_options pointing to an + * inirtd + * + * @argc: Number of arguments + * @argv: Argument array + * @file_path Existing device path, the new instance will be appended + * Return: Pointer to the device path or ERR_PTR + * + */ +static +struct efi_device_path *add_initrd_instance(const char *dev, const char *part, + const char *file, + const struct efi_device_path *fp, + efi_uintn_t *fp_size) +{ + struct efi_device_path *tmp_dp = NULL, *tmp_fp = NULL; + struct efi_device_path *final_fp = NULL, *initrd_dp = NULL; + efi_status_t ret; + const struct efi_initrd_dp id_dp = { + .vendor = { + { + DEVICE_PATH_TYPE_MEDIA_DEVICE, + DEVICE_PATH_SUB_TYPE_VENDOR_PATH, + sizeof(id_dp.vendor), + }, + EFI_INITRD_MEDIA_GUID, + }, + .end = { + DEVICE_PATH_TYPE_END, + DEVICE_PATH_SUB_TYPE_END, + sizeof(id_dp.end), + } + }; + + ret = efi_dp_from_name(dev, part, file, &tmp_dp, &tmp_fp); + if (ret != EFI_SUCCESS) { + printf("Cannot create device path for \"%s %s\"\n", part, file); + goto out; + } + + initrd_dp = + efi_dp_append_instance((const struct efi_device_path *)&id_dp, + tmp_fp); + if (!initrd_dp) { + printf("Cannot append media vendor device path path\n"); + goto out; + } + final_fp = efi_dp_concat(fp, initrd_dp); + *fp_size = efi_dp_size(fp) + efi_dp_size(initrd_dp) + + (2 * sizeof(struct efi_device_path)); + +out: + efi_free_pool(initrd_dp); + efi_free_pool(tmp_dp); + efi_free_pool(tmp_fp); + return final_fp ? final_fp : ERR_PTR(-EINVAL); +} + /** * do_efi_boot_add() - set UEFI load option * @@ -806,7 +868,9 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag, * * Implement efidebug "boot add" sub-command. Create or change UEFI load option. * - * efidebug boot add <id> <label> <interface> <devnum>[:<part>] <file> <options> + * efidebug boot add -b <id> <label> <interface> <devnum>[:<part>] <file> + * -i <file> <interface2> <devnum2>[:<part>] <initrd> + * -s '<options>' */ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) @@ -819,55 +883,98 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, size_t label_len, label_len16; u16 *label; struct efi_device_path *device_path = NULL, *file_path = NULL; + struct efi_device_path *final_fp = NULL; struct efi_load_option lo; void *data = NULL; efi_uintn_t size; + efi_uintn_t fp_size; efi_status_t ret; int r = CMD_RET_SUCCESS; - - if (argc < 6 || argc > 7) - return CMD_RET_USAGE; - - id = (int)simple_strtoul(argv[1], &endp, 16); - if (*endp != '\0' || id > 0xffff) - return CMD_RET_USAGE; - - sprintf(var_name, "Boot%04X", id); - p = var_name16; - utf8_utf16_strncpy(&p, var_name, 9); + int i; guid = efi_global_variable_guid; /* attributes */ lo.attributes = LOAD_OPTION_ACTIVE; /* always ACTIVE */ + lo.optional_data = NULL; + lo.label = NULL; - /* label */ - label_len = strlen(argv[2]); - label_len16 = utf8_utf16_strnlen(argv[2], label_len); - label = malloc((label_len16 + 1) * sizeof(u16)); - if (!label) - return CMD_RET_FAILURE; - lo.label = label; /* label will be changed below */ - utf8_utf16_strncpy(&label, argv[2], label_len); + /* search for -b first since the rest of the arguments depends on that */ + for (i = 0; i < argc; i++) { + if (!strcmp(argv[i], "-b")) { + if (argc < i + 5 || lo.label) { + r = CMD_RET_USAGE; + goto out; + } + id = (int)simple_strtoul(argv[i + 1], &endp, 16); + if (*endp != '\0' || id > 0xffff) + return CMD_RET_USAGE; + + sprintf(var_name, "Boot%04X", id); + p = var_name16; + utf8_utf16_strncpy(&p, var_name, 9); + + /* label */ + label_len = strlen(argv[i + 2]); + label_len16 = utf8_utf16_strnlen(argv[i + 2], label_len); + label = malloc((label_len16 + 1) * sizeof(u16)); + if (!label) + return CMD_RET_FAILURE; + lo.label = label; /* label will be changed below */ + utf8_utf16_strncpy(&label, argv[i + 2], label_len); + + /* file path */ + ret = efi_dp_from_name(argv[i + 3], argv[i + 4], + argv[i + 5], &device_path, + &file_path); + if (ret != EFI_SUCCESS) { + printf("Cannot create device path for \"%s %s\"\n", + argv[3], argv[4]); + r = CMD_RET_FAILURE; + goto out; + break; + } + fp_size = efi_dp_size(file_path) + + sizeof(struct efi_device_path); + } + } - /* file path */ - ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path, - &file_path); - if (ret != EFI_SUCCESS) { - printf("Cannot create device path for \"%s %s\"\n", - argv[3], argv[4]); + if (!file_path) { + printf("You need to specify an image before an initrd.\n"); r = CMD_RET_FAILURE; goto out; } - lo.file_path = file_path; - lo.file_path_length = efi_dp_size(file_path) - + sizeof(struct efi_device_path); /* for END */ - /* optional data */ - if (argc == 6) - lo.optional_data = NULL; - else - lo.optional_data = (const u8 *)argv[6]; + /* add now add initrd and extra data */ + for (i = 0; i < argc; i++) { + if (!strcmp(argv[i], "-i")) { + if (argc < i + 3 || final_fp) { + r = CMD_RET_USAGE; + goto out; + } + + final_fp = add_initrd_instance(argv[i + 1], argv[i + 2], + argv[i + 3], file_path, + &fp_size); + if (IS_ERR(final_fp)) { + r = CMD_RET_FAILURE; + goto out; + } + + /* add_initrd_instance allocates a new device path */ + efi_free_pool(file_path); + file_path = final_fp; + } else if (!strcmp(argv[i], "-s")) { + if (argc < i + 1 || lo.optional_data) { + r = CMD_RET_USAGE; + goto out; + } + lo.optional_data = (const u8 *)argv[i + 1]; + } + } + + lo.file_path = file_path; + lo.file_path_length = fp_size; size = efi_serialize_load_option(&lo, (u8 **)&data); if (!size) { @@ -951,11 +1058,14 @@ static int do_efi_boot_rm(struct cmd_tbl *cmdtp, int flag, */ static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t *size) { + struct efi_device_path *initrd_path = NULL; struct efi_load_option lo; char *label, *p; size_t label_len16, label_len; u16 *dp_str; efi_status_t ret; + efi_uintn_t initrd_dp_size; + const efi_guid_t lf2_initrd_guid = EFI_INITRD_MEDIA_GUID; ret = efi_deserialize_load_option(&lo, data, size); if (ret != EFI_SUCCESS) { @@ -986,6 +1096,14 @@ static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t *size) printf(" file_path: %ls\n", dp_str); efi_free_pool(dp_str); + initrd_path = efi_dp_from_lo(&lo, &initrd_dp_size, lf2_initrd_guid); + if (initrd_path) { + dp_str = efi_dp_str(initrd_path); + printf(" initrd_path: %ls\n", dp_str); + efi_free_pool(dp_str); + efi_free_pool(initrd_path); + } + printf(" data:\n"); print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1, lo.optional_data, *size, true); @@ -1555,7 +1673,10 @@ static int do_efidebug(struct cmd_tbl *cmdtp, int flag, static char efidebug_help_text[] = " - UEFI Shell-like interface to configure UEFI environment\n" "\n" - "efidebug boot add <bootid> <label> <interface> <devnum>[:<part>] <file path> [<load options>]\n" + "efidebug boot add " + "-b <bootid> <label> <interface> <devnum>[:<part>] <file path> " + "-i <interface> <devnum>[:<part>] <initrd file path> " + "-s '<optional data>'\n" " - set UEFI BootXXXX variable\n" " <load options> will be passed to UEFI application\n" "efidebug boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n" @@ -1599,7 +1720,7 @@ static char efidebug_help_text[] = #endif U_BOOT_CMD( - efidebug, 10, 0, do_efidebug, + efidebug, CONFIG_SYS_MAXARGS, 0, do_efidebug, "Configure UEFI environment", efidebug_help_text ); diff --git a/doc/board/emulation/qemu_capsule_update.rst b/doc/board/emulation/qemu_capsule_update.rst index 9fec75f8f1c9..33ce4bcd32ea 100644 --- a/doc/board/emulation/qemu_capsule_update.rst +++ b/doc/board/emulation/qemu_capsule_update.rst @@ -60,7 +60,7 @@ to be pointing to the EFI System Partition which contains the capsule file. The BootNext, BootXXXX and OsIndications variables can be set using the following commands:: - => efidebug boot add 0 Boot0000 virtio 0:1 <capsule_file_name> + => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name> => efidebug boot next 0 => setenv -e -nv -bs -rt -v OsIndications =0x04 => saveenv @@ -198,7 +198,7 @@ command line:: 3. Set the following environment and UEFI boot variables => setenv -e -nv -bs -rt -v OsIndications =0x04 - => efidebug boot add 0 Boot0000 virtio 0:1 <capsule_file_name> + => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name> => efidebug boot next 0 => saveenv diff --git a/doc/uefi/uefi.rst b/doc/uefi/uefi.rst index 5a67737c1579..b3494c22e073 100644 --- a/doc/uefi/uefi.rst +++ b/doc/uefi/uefi.rst @@ -178,7 +178,7 @@ Now in U-Boot install the keys on your board:: Set up boot parameters on your board:: - efidebug boot add 1 HELLO mmc 0:1 /helloworld.efi.signed "" + efidebug boot add -b 1 HELLO mmc 0:1 /helloworld.efi.signed "" Now your board can run the signed image via the boot manager (see below). You can also try this sequence by running Pytest, test_efi_secboot, diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware.py b/test/py/tests/test_efi_capsule/test_capsule_firmware.py index f006fa95d650..e8b0a1575453 100644 --- a/test/py/tests/test_efi_capsule/test_capsule_firmware.py +++ b/test/py/tests/test_efi_capsule/test_capsule_firmware.py @@ -39,7 +39,7 @@ class TestEfiCapsuleFirmwareFit(object): with u_boot_console.log.section('Test Case 1-a, before reboot'): output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img, - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""', + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""', 'efidebug boot order 1', 'env set -e OsIndications', 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"', @@ -114,7 +114,7 @@ class TestEfiCapsuleFirmwareFit(object): with u_boot_console.log.section('Test Case 2-a, before reboot'): output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img, - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""', + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""', 'efidebug boot order 1', 'env set -e -nv -bs -rt OsIndications =0x0000000000000004', 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"', @@ -188,7 +188,7 @@ class TestEfiCapsuleFirmwareFit(object): with u_boot_console.log.section('Test Case 3-a, before reboot'): output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img, - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""', + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""', 'efidebug boot order 1', 'env set -e -nv -bs -rt OsIndications =0x0000000000000004', 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"', diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py index 863685e215b7..75f5ea772300 100644 --- a/test/py/tests/test_efi_secboot/test_signed.py +++ b/test/py/tests/test_efi_secboot/test_signed.py @@ -28,7 +28,7 @@ class TestEfiSignedImage(object): # Test Case 1a, run signed image if no PK output = u_boot_console.run_command_list([ 'host bind 0 %s' % disk_img, - 'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""', + 'efidebug boot add -b 1 HELLO1 host 0:1 /helloworld.efi.signed ""', 'efidebug boot next 1', 'bootefi bootmgr']) assert 'Hello, world!' in ''.join(output) @@ -36,7 +36,7 @@ class TestEfiSignedImage(object): with u_boot_console.log.section('Test Case 1b'): # Test Case 1b, run unsigned image if no PK output = u_boot_console.run_command_list([ - 'efidebug boot add 2 HELLO2 host 0:1 /helloworld.efi ""', + 'efidebug boot add -b 2 HELLO2 host 0:1 /helloworld.efi ""', 'efidebug boot next 2', 'bootefi bootmgr']) assert 'Hello, world!' in ''.join(output) @@ -58,13 +58,13 @@ class TestEfiSignedImage(object): 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) assert 'Failed to set EFI variable' not in ''.join(output) output = u_boot_console.run_command_list([ - 'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""', + 'efidebug boot add -b 1 HELLO1 host 0:1 /helloworld.efi.signed ""', 'efidebug boot next 1', 'efidebug test bootmgr']) assert('\'HELLO1\' failed' in ''.join(output)) assert('efi_start_image() returned: 26' in ''.join(output)) output = u_boot_console.run_command_list([ - 'efidebug boot add 2 HELLO2 host 0:1 /helloworld.efi ""', + 'efidebug boot add -b 2 HELLO2 host 0:1 /helloworld.efi ""', 'efidebug boot next 2', 'efidebug test bootmgr']) assert '\'HELLO2\' failed' in ''.join(output) @@ -104,7 +104,7 @@ class TestEfiSignedImage(object): 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) assert 'Failed to set EFI variable' not in ''.join(output) output = u_boot_console.run_command_list([ - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""', 'efidebug boot next 1', 'efidebug test bootmgr']) assert '\'HELLO\' failed' in ''.join(output) @@ -142,7 +142,7 @@ class TestEfiSignedImage(object): 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) assert 'Failed to set EFI variable' not in ''.join(output) output = u_boot_console.run_command_list([ - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""', 'efidebug boot next 1', 'efidebug test bootmgr']) assert '\'HELLO\' failed' in ''.join(output) @@ -169,7 +169,7 @@ class TestEfiSignedImage(object): 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) assert 'Failed to set EFI variable' not in ''.join(output) output = u_boot_console.run_command_list([ - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed_2sigs ""', + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed_2sigs ""', 'efidebug boot next 1', 'efidebug test bootmgr']) assert 'Hello, world!' in ''.join(output) @@ -227,7 +227,7 @@ class TestEfiSignedImage(object): 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK']) assert 'Failed to set EFI variable' not in ''.join(output) output = u_boot_console.run_command_list([ - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""', + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""', 'efidebug boot next 1', 'bootefi bootmgr']) assert 'Hello, world!' in ''.join(output) diff --git a/test/py/tests/test_efi_secboot/test_signed_intca.py b/test/py/tests/test_efi_secboot/test_signed_intca.py index 70d6be00e8a8..0849572a5143 100644 --- a/test/py/tests/test_efi_secboot/test_signed_intca.py +++ b/test/py/tests/test_efi_secboot/test_signed_intca.py @@ -39,7 +39,7 @@ class TestEfiSignedImageIntca(object): assert 'Failed to set EFI variable' not in ''.join(output) output = u_boot_console.run_command_list([ - 'efidebug boot add 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""', + 'efidebug boot add -b 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""', 'efidebug boot next 1', 'efidebug test bootmgr']) assert '\'HELLO_a\' failed' in ''.join(output) @@ -48,7 +48,7 @@ class TestEfiSignedImageIntca(object): with u_boot_console.log.section('Test Case 1b'): # Test Case 1b, signed and authenticated by root CA output = u_boot_console.run_command_list([ - 'efidebug boot add 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""', + 'efidebug boot add -b 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""', 'efidebug boot next 2', 'bootefi bootmgr']) assert 'Hello, world!' in ''.join(output) @@ -70,7 +70,7 @@ class TestEfiSignedImageIntca(object): assert 'Failed to set EFI variable' not in ''.join(output) output = u_boot_console.run_command_list([ - 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', + 'efidebug boot add -b 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', 'efidebug boot next 1', 'efidebug test bootmgr']) assert '\'HELLO_abc\' failed' in ''.join(output) @@ -116,7 +116,7 @@ class TestEfiSignedImageIntca(object): assert 'Failed to set EFI variable' not in ''.join(output) output = u_boot_console.run_command_list([ - 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', + 'efidebug boot add -b 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""', 'efidebug boot next 1', 'efidebug test bootmgr']) assert 'Hello, world!' in ''.join(output) diff --git a/test/py/tests/test_efi_secboot/test_unsigned.py b/test/py/tests/test_efi_secboot/test_unsigned.py index 56f56e19eb84..8e026f7566ad 100644 --- a/test/py/tests/test_efi_secboot/test_unsigned.py +++ b/test/py/tests/test_efi_secboot/test_unsigned.py @@ -35,7 +35,7 @@ class TestEfiUnsignedImage(object): assert 'Failed to set EFI variable' not in ''.join(output) output = u_boot_console.run_command_list([ - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', 'efidebug boot next 1', 'bootefi bootmgr']) assert '\'HELLO\' failed' in ''.join(output) @@ -64,7 +64,7 @@ class TestEfiUnsignedImage(object): assert 'Failed to set EFI variable' not in ''.join(output) output = u_boot_console.run_command_list([ - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', 'efidebug boot next 1', 'bootefi bootmgr']) assert 'Hello, world!' in ''.join(output) @@ -88,7 +88,7 @@ class TestEfiUnsignedImage(object): assert 'Failed to set EFI variable' not in ''.join(output) output = u_boot_console.run_command_list([ - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', 'efidebug boot next 1', 'bootefi bootmgr']) assert '\'HELLO\' failed' in ''.join(output) @@ -106,7 +106,7 @@ class TestEfiUnsignedImage(object): assert 'Failed to set EFI variable' not in ''.join(output) output = u_boot_console.run_command_list([ - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""', + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""', 'efidebug boot next 1', 'bootefi bootmgr']) assert '\'HELLO\' failed' in ''.join(output)
The UEFI spec allow a packed array of UEFI device paths in the FilePathList[] of an EFI_LOAD_OPTION. The first file path must describe the loaded image but the rest are OS specific. Previous patches parse the device path and try to use the second member of the array as an initrd. So let's modify efidebug slightly and install the second file described in the command line as the initrd device path. Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> --- cmd/efidebug.c | 193 ++++++++++++++---- doc/board/emulation/qemu_capsule_update.rst | 4 +- doc/uefi/uefi.rst | 2 +- .../test_efi_capsule/test_capsule_firmware.py | 6 +- test/py/tests/test_efi_secboot/test_signed.py | 16 +- .../test_efi_secboot/test_signed_intca.py | 8 +- .../tests/test_efi_secboot/test_unsigned.py | 8 +- 7 files changed, 179 insertions(+), 58 deletions(-) -- 2.30.1