* [PATCH 0/2] efi_loader: use short-form DP for load options @ 2022-02-26 11:56 Heinrich Schuchardt 2022-02-26 11:56 ` [PATCH 1/2] efi_loader: support booting via short-form device-path Heinrich Schuchardt 2022-02-26 11:56 ` [PATCH 2/2] efi_loader: use short-form DP for load options Heinrich Schuchardt 0 siblings, 2 replies; 5+ messages in thread From: Heinrich Schuchardt @ 2022-02-26 11:56 UTC (permalink / raw) To: u-boot; +Cc: Ilias Apalodimas, Heinrich Schuchardt The GUID of partitions is sufficient for identification and will stay constant in the lifetime of a boot option. The preceding path of the device-path may change due to changes in the enumeration of devices. Create short-form device-paths in the 'efidebug boot add' command. Fix the support of short form device-paths in the implementation of LoadImage(). Heinrich Schuchardt (2): efi_loader: support booting via short-form device-path efi_loader: use short-form DP for load options cmd/efidebug.c | 15 +++++++++++---- include/efi_loader.h | 3 ++- lib/efi_loader/efi_boottime.c | 12 ++++++------ lib/efi_loader/efi_device_path.c | 21 +++++++++++++-------- 4 files changed, 32 insertions(+), 19 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] efi_loader: support booting via short-form device-path 2022-02-26 11:56 [PATCH 0/2] efi_loader: use short-form DP for load options Heinrich Schuchardt @ 2022-02-26 11:56 ` Heinrich Schuchardt 2022-02-26 11:56 ` [PATCH 2/2] efi_loader: use short-form DP for load options Heinrich Schuchardt 1 sibling, 0 replies; 5+ messages in thread From: Heinrich Schuchardt @ 2022-02-26 11:56 UTC (permalink / raw) To: u-boot; +Cc: Ilias Apalodimas, Heinrich Schuchardt The boot manager must support loading from boot options using a short-form device-path, e.g. one where the first element is a hard drive media path. See '3.1.2 Load Options Processing' in UEFI specification version 2.9. Fixes: 0e074d12393b ("efi_loader: carve out efi_load_image_from_file()") Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- lib/efi_loader/efi_boottime.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 82128ac1d5..173e636d60 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1940,7 +1940,7 @@ efi_status_t efi_load_image_from_path(bool boot_policy, { efi_handle_t device; efi_status_t ret; - struct efi_device_path *dp; + struct efi_device_path *dp, *rem; struct efi_load_file_protocol *load_file_protocol = NULL; efi_uintn_t buffer_size; uint64_t addr, pages; @@ -1951,18 +1951,18 @@ efi_status_t efi_load_image_from_path(bool boot_policy, *size = 0; dp = file_path; - ret = EFI_CALL(efi_locate_device_path( - &efi_simple_file_system_protocol_guid, &dp, &device)); + device = efi_dp_find_obj(dp, &rem); + ret = efi_search_protocol(device, &efi_simple_file_system_protocol_guid, + NULL); if (ret == EFI_SUCCESS) return efi_load_image_from_file(file_path, buffer, size); - ret = EFI_CALL(efi_locate_device_path( - &efi_guid_load_file_protocol, &dp, &device)); + ret = efi_search_protocol(device, &efi_guid_load_file_protocol, NULL); if (ret == EFI_SUCCESS) { guid = &efi_guid_load_file_protocol; } else if (!boot_policy) { guid = &efi_guid_load_file2_protocol; - ret = EFI_CALL(efi_locate_device_path(guid, &dp, &device)); + ret = efi_search_protocol(device, guid, NULL); } if (ret != EFI_SUCCESS) return EFI_NOT_FOUND; -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] efi_loader: use short-form DP for load options 2022-02-26 11:56 [PATCH 0/2] efi_loader: use short-form DP for load options Heinrich Schuchardt 2022-02-26 11:56 ` [PATCH 1/2] efi_loader: support booting via short-form device-path Heinrich Schuchardt @ 2022-02-26 11:56 ` Heinrich Schuchardt 2022-03-02 9:44 ` AKASHI Takahiro 1 sibling, 1 reply; 5+ messages in thread From: Heinrich Schuchardt @ 2022-02-26 11:56 UTC (permalink / raw) To: u-boot; +Cc: Ilias Apalodimas, Heinrich Schuchardt The GUID of partitions is sufficient for identification and will stay constant in the lifetime of a boot option. The preceding path of the device-path may change due to changes in the enumeration of devices. Therefore it is preferable to use the short-form of device-paths in load options. Adjust the 'efidebug boot add' command accordingly. Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- cmd/efidebug.c | 15 +++++++++++---- include/efi_loader.h | 3 ++- lib/efi_loader/efi_device_path.c | 21 +++++++++++++-------- 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/cmd/efidebug.c b/cmd/efidebug.c index 401d13cc4c..f62a4345fd 100644 --- a/cmd/efidebug.c +++ b/cmd/efidebug.c @@ -747,7 +747,7 @@ struct efi_device_path *create_initrd_dp(const char *dev, const char *part, const char *file) { - struct efi_device_path *tmp_dp = NULL, *tmp_fp = NULL; + struct efi_device_path *tmp_dp = NULL, *tmp_fp = NULL, *short_fp; struct efi_device_path *initrd_dp = NULL; efi_status_t ret; const struct efi_initrd_dp id_dp = { @@ -771,9 +771,12 @@ struct efi_device_path *create_initrd_dp(const char *dev, const char *part, printf("Cannot create device path for \"%s %s\"\n", part, file); goto out; } + short_fp = efi_dp_shorten(tmp_fp); + if (!short_fp) + short_fp = tmp_fp; initrd_dp = efi_dp_append((const struct efi_device_path *)&id_dp, - tmp_fp); + short_fp); out: efi_free_pool(tmp_dp); @@ -807,6 +810,7 @@ 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 *fp_free = NULL; struct efi_device_path *final_fp = NULL; struct efi_device_path *initrd_dp = NULL; struct efi_load_option lo; @@ -849,13 +853,16 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, /* file path */ ret = efi_dp_from_name(argv[3], argv[4], argv[5], - &device_path, &file_path); + &device_path, &fp_free); if (ret != EFI_SUCCESS) { printf("Cannot create device path for \"%s %s\"\n", argv[3], argv[4]); r = CMD_RET_FAILURE; goto out; } + file_path = efi_dp_shorten(fp_free); + if (!file_path) + file_path = fp_free; fp_size += efi_dp_size(file_path) + sizeof(struct efi_device_path); argc -= 5; @@ -927,7 +934,7 @@ out: efi_free_pool(final_fp); efi_free_pool(initrd_dp); efi_free_pool(device_path); - efi_free_pool(file_path); + efi_free_pool(fp_free); free(lo.label); return r; diff --git a/include/efi_loader.h b/include/efi_loader.h index e390d323a9..c7d6b7c7f3 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -725,7 +725,8 @@ extern void *efi_bounce_buffer; #define EFI_LOADER_BOUNCE_BUFFER_SIZE (64 * 1024 * 1024) #endif - +/* shorten device path */ +struct efi_device_path *efi_dp_shorten(struct efi_device_path *dp); struct efi_device_path *efi_dp_next(const struct efi_device_path *dp); int efi_dp_match(const struct efi_device_path *a, const struct efi_device_path *b); diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index dc787b4d3d..ddd5f132ec 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -122,20 +122,25 @@ int efi_dp_match(const struct efi_device_path *a, } } -/* +/** + * efi_dp_shorten() - shorten device-path + * * We can have device paths that start with a USB WWID or a USB Class node, * and a few other cases which don't encode the full device path with bus * hierarchy: * - * - MESSAGING:USB_WWID - * - MESSAGING:USB_CLASS - * - MEDIA:FILE_PATH - * - MEDIA:HARD_DRIVE - * - MESSAGING:URI + * * MESSAGING:USB_WWID + * * MESSAGING:USB_CLASS + * * MEDIA:FILE_PATH + * * MEDIA:HARD_DRIVE + * * MESSAGING:URI * * See UEFI spec (section 3.1.2, about short-form device-paths) + * + * @dp: original devie-path + * @Return: shortened device-path or NULL */ -static struct efi_device_path *shorten_path(struct efi_device_path *dp) +struct efi_device_path *efi_dp_shorten(struct efi_device_path *dp) { while (dp) { /* @@ -189,7 +194,7 @@ static struct efi_object *find_obj(struct efi_device_path *dp, bool short_path, } } - obj_dp = shorten_path(efi_dp_next(obj_dp)); + obj_dp = efi_dp_shorten(efi_dp_next(obj_dp)); } while (short_path && obj_dp); } -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] efi_loader: use short-form DP for load options 2022-02-26 11:56 ` [PATCH 2/2] efi_loader: use short-form DP for load options Heinrich Schuchardt @ 2022-03-02 9:44 ` AKASHI Takahiro 2022-03-03 11:01 ` Heinrich Schuchardt 0 siblings, 1 reply; 5+ messages in thread From: AKASHI Takahiro @ 2022-03-02 9:44 UTC (permalink / raw) To: Heinrich Schuchardt; +Cc: u-boot, Ilias Apalodimas On Sat, Feb 26, 2022 at 12:56:51PM +0100, Heinrich Schuchardt wrote: > The GUID of partitions is sufficient for identification and will stay > constant in the lifetime of a boot option. The preceding path of the > device-path may change due to changes in the enumeration of devices. > Therefore it is preferable to use the short-form of device-paths in load > options. Adjust the 'efidebug boot add' command accordingly. Since a "long" device path is still valid, I think that a user should be allowed to use a long device path especially if he or she wants to limit an interface for loading any image. Please add an option to efidebug to select a short or long path. Furthermore, I would like to ask you to add a test, as you always require me to do so, for loading from a short path. Otherwise, the feature will never be exercised in CI loop. -Takahiro Akashi > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > --- > cmd/efidebug.c | 15 +++++++++++---- > include/efi_loader.h | 3 ++- > lib/efi_loader/efi_device_path.c | 21 +++++++++++++-------- > 3 files changed, 26 insertions(+), 13 deletions(-) > > diff --git a/cmd/efidebug.c b/cmd/efidebug.c > index 401d13cc4c..f62a4345fd 100644 > --- a/cmd/efidebug.c > +++ b/cmd/efidebug.c > @@ -747,7 +747,7 @@ struct efi_device_path *create_initrd_dp(const char *dev, const char *part, > const char *file) > > { > - struct efi_device_path *tmp_dp = NULL, *tmp_fp = NULL; > + struct efi_device_path *tmp_dp = NULL, *tmp_fp = NULL, *short_fp; > struct efi_device_path *initrd_dp = NULL; > efi_status_t ret; > const struct efi_initrd_dp id_dp = { > @@ -771,9 +771,12 @@ struct efi_device_path *create_initrd_dp(const char *dev, const char *part, > printf("Cannot create device path for \"%s %s\"\n", part, file); > goto out; > } > + short_fp = efi_dp_shorten(tmp_fp); > + if (!short_fp) > + short_fp = tmp_fp; > > initrd_dp = efi_dp_append((const struct efi_device_path *)&id_dp, > - tmp_fp); > + short_fp); > > out: > efi_free_pool(tmp_dp); > @@ -807,6 +810,7 @@ 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 *fp_free = NULL; > struct efi_device_path *final_fp = NULL; > struct efi_device_path *initrd_dp = NULL; > struct efi_load_option lo; > @@ -849,13 +853,16 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, > > /* file path */ > ret = efi_dp_from_name(argv[3], argv[4], argv[5], > - &device_path, &file_path); > + &device_path, &fp_free); > if (ret != EFI_SUCCESS) { > printf("Cannot create device path for \"%s %s\"\n", > argv[3], argv[4]); > r = CMD_RET_FAILURE; > goto out; > } > + file_path = efi_dp_shorten(fp_free); > + if (!file_path) > + file_path = fp_free; > fp_size += efi_dp_size(file_path) + > sizeof(struct efi_device_path); > argc -= 5; > @@ -927,7 +934,7 @@ out: > efi_free_pool(final_fp); > efi_free_pool(initrd_dp); > efi_free_pool(device_path); > - efi_free_pool(file_path); > + efi_free_pool(fp_free); > free(lo.label); > > return r; > diff --git a/include/efi_loader.h b/include/efi_loader.h > index e390d323a9..c7d6b7c7f3 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -725,7 +725,8 @@ extern void *efi_bounce_buffer; > #define EFI_LOADER_BOUNCE_BUFFER_SIZE (64 * 1024 * 1024) > #endif > > - > +/* shorten device path */ > +struct efi_device_path *efi_dp_shorten(struct efi_device_path *dp); > struct efi_device_path *efi_dp_next(const struct efi_device_path *dp); > int efi_dp_match(const struct efi_device_path *a, > const struct efi_device_path *b); > diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c > index dc787b4d3d..ddd5f132ec 100644 > --- a/lib/efi_loader/efi_device_path.c > +++ b/lib/efi_loader/efi_device_path.c > @@ -122,20 +122,25 @@ int efi_dp_match(const struct efi_device_path *a, > } > } > > -/* > +/** > + * efi_dp_shorten() - shorten device-path > + * > * We can have device paths that start with a USB WWID or a USB Class node, > * and a few other cases which don't encode the full device path with bus > * hierarchy: > * > - * - MESSAGING:USB_WWID > - * - MESSAGING:USB_CLASS > - * - MEDIA:FILE_PATH > - * - MEDIA:HARD_DRIVE > - * - MESSAGING:URI > + * * MESSAGING:USB_WWID > + * * MESSAGING:USB_CLASS > + * * MEDIA:FILE_PATH > + * * MEDIA:HARD_DRIVE > + * * MESSAGING:URI > * > * See UEFI spec (section 3.1.2, about short-form device-paths) > + * > + * @dp: original devie-path > + * @Return: shortened device-path or NULL > */ > -static struct efi_device_path *shorten_path(struct efi_device_path *dp) > +struct efi_device_path *efi_dp_shorten(struct efi_device_path *dp) > { > while (dp) { > /* > @@ -189,7 +194,7 @@ static struct efi_object *find_obj(struct efi_device_path *dp, bool short_path, > } > } > > - obj_dp = shorten_path(efi_dp_next(obj_dp)); > + obj_dp = efi_dp_shorten(efi_dp_next(obj_dp)); > } while (short_path && obj_dp); > } > > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] efi_loader: use short-form DP for load options 2022-03-02 9:44 ` AKASHI Takahiro @ 2022-03-03 11:01 ` Heinrich Schuchardt 0 siblings, 0 replies; 5+ messages in thread From: Heinrich Schuchardt @ 2022-03-03 11:01 UTC (permalink / raw) To: AKASHI Takahiro; +Cc: u-boot, Ilias Apalodimas On 3/2/22 10:44, AKASHI Takahiro wrote: > On Sat, Feb 26, 2022 at 12:56:51PM +0100, Heinrich Schuchardt wrote: >> The GUID of partitions is sufficient for identification and will stay >> constant in the lifetime of a boot option. The preceding path of the >> device-path may change due to changes in the enumeration of devices. >> Therefore it is preferable to use the short-form of device-paths in load >> options. Adjust the 'efidebug boot add' command accordingly. > > Since a "long" device path is still valid, I think that a user should > be allowed to use a long device path especially if he or she wants to > limit an interface for loading any image. > Please add an option to efidebug to select a short or long path. > > Furthermore, I would like to ask you to add a test, as you always > require me to do so, for loading from a short path. > Otherwise, the feature will never be exercised in CI loop. > > -Takahiro Akashi Dear Takahiro, thanks for reviewing. In the next round I will alternative options -b/-B, -i/-I to choose between long and short device path. I will create a Python test exposes a initrd via LoadFile2 protocol and then use initrddump.efi as binary payload and to check the content of the initrd. Best regards Heinrich > > >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> >> --- >> cmd/efidebug.c | 15 +++++++++++---- >> include/efi_loader.h | 3 ++- >> lib/efi_loader/efi_device_path.c | 21 +++++++++++++-------- >> 3 files changed, 26 insertions(+), 13 deletions(-) >> >> diff --git a/cmd/efidebug.c b/cmd/efidebug.c >> index 401d13cc4c..f62a4345fd 100644 >> --- a/cmd/efidebug.c >> +++ b/cmd/efidebug.c >> @@ -747,7 +747,7 @@ struct efi_device_path *create_initrd_dp(const char *dev, const char *part, >> const char *file) >> >> { >> - struct efi_device_path *tmp_dp = NULL, *tmp_fp = NULL; >> + struct efi_device_path *tmp_dp = NULL, *tmp_fp = NULL, *short_fp; >> struct efi_device_path *initrd_dp = NULL; >> efi_status_t ret; >> const struct efi_initrd_dp id_dp = { >> @@ -771,9 +771,12 @@ struct efi_device_path *create_initrd_dp(const char *dev, const char *part, >> printf("Cannot create device path for \"%s %s\"\n", part, file); >> goto out; >> } >> + short_fp = efi_dp_shorten(tmp_fp); >> + if (!short_fp) >> + short_fp = tmp_fp; >> >> initrd_dp = efi_dp_append((const struct efi_device_path *)&id_dp, >> - tmp_fp); >> + short_fp); >> >> out: >> efi_free_pool(tmp_dp); >> @@ -807,6 +810,7 @@ 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 *fp_free = NULL; >> struct efi_device_path *final_fp = NULL; >> struct efi_device_path *initrd_dp = NULL; >> struct efi_load_option lo; >> @@ -849,13 +853,16 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, >> >> /* file path */ >> ret = efi_dp_from_name(argv[3], argv[4], argv[5], >> - &device_path, &file_path); >> + &device_path, &fp_free); >> if (ret != EFI_SUCCESS) { >> printf("Cannot create device path for \"%s %s\"\n", >> argv[3], argv[4]); >> r = CMD_RET_FAILURE; >> goto out; >> } >> + file_path = efi_dp_shorten(fp_free); >> + if (!file_path) >> + file_path = fp_free; >> fp_size += efi_dp_size(file_path) + >> sizeof(struct efi_device_path); >> argc -= 5; >> @@ -927,7 +934,7 @@ out: >> efi_free_pool(final_fp); >> efi_free_pool(initrd_dp); >> efi_free_pool(device_path); >> - efi_free_pool(file_path); >> + efi_free_pool(fp_free); >> free(lo.label); >> >> return r; >> diff --git a/include/efi_loader.h b/include/efi_loader.h >> index e390d323a9..c7d6b7c7f3 100644 >> --- a/include/efi_loader.h >> +++ b/include/efi_loader.h >> @@ -725,7 +725,8 @@ extern void *efi_bounce_buffer; >> #define EFI_LOADER_BOUNCE_BUFFER_SIZE (64 * 1024 * 1024) >> #endif >> >> - >> +/* shorten device path */ >> +struct efi_device_path *efi_dp_shorten(struct efi_device_path *dp); >> struct efi_device_path *efi_dp_next(const struct efi_device_path *dp); >> int efi_dp_match(const struct efi_device_path *a, >> const struct efi_device_path *b); >> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c >> index dc787b4d3d..ddd5f132ec 100644 >> --- a/lib/efi_loader/efi_device_path.c >> +++ b/lib/efi_loader/efi_device_path.c >> @@ -122,20 +122,25 @@ int efi_dp_match(const struct efi_device_path *a, >> } >> } >> >> -/* >> +/** >> + * efi_dp_shorten() - shorten device-path >> + * >> * We can have device paths that start with a USB WWID or a USB Class node, >> * and a few other cases which don't encode the full device path with bus >> * hierarchy: >> * >> - * - MESSAGING:USB_WWID >> - * - MESSAGING:USB_CLASS >> - * - MEDIA:FILE_PATH >> - * - MEDIA:HARD_DRIVE >> - * - MESSAGING:URI >> + * * MESSAGING:USB_WWID >> + * * MESSAGING:USB_CLASS >> + * * MEDIA:FILE_PATH >> + * * MEDIA:HARD_DRIVE >> + * * MESSAGING:URI >> * >> * See UEFI spec (section 3.1.2, about short-form device-paths) >> + * >> + * @dp: original devie-path >> + * @Return: shortened device-path or NULL >> */ >> -static struct efi_device_path *shorten_path(struct efi_device_path *dp) >> +struct efi_device_path *efi_dp_shorten(struct efi_device_path *dp) >> { >> while (dp) { >> /* >> @@ -189,7 +194,7 @@ static struct efi_object *find_obj(struct efi_device_path *dp, bool short_path, >> } >> } >> >> - obj_dp = shorten_path(efi_dp_next(obj_dp)); >> + obj_dp = efi_dp_shorten(efi_dp_next(obj_dp)); >> } while (short_path && obj_dp); >> } >> >> -- >> 2.34.1 >> ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-03-03 11:01 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-02-26 11:56 [PATCH 0/2] efi_loader: use short-form DP for load options Heinrich Schuchardt 2022-02-26 11:56 ` [PATCH 1/2] efi_loader: support booting via short-form device-path Heinrich Schuchardt 2022-02-26 11:56 ` [PATCH 2/2] efi_loader: use short-form DP for load options Heinrich Schuchardt 2022-03-02 9:44 ` AKASHI Takahiro 2022-03-03 11:01 ` Heinrich Schuchardt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox