* [U-Boot] [PATCH v3 0/1] efi_loader: support BootNext and BootCurrent
@ 2019-03-08 1:29 AKASHI Takahiro
2019-03-08 1:29 ` [U-Boot] [PATCH v3 1/1] efi_loader: bootmgr: support BootNext and BootCurrent variable behavior AKASHI Takahiro
0 siblings, 1 reply; 4+ messages in thread
From: AKASHI Takahiro @ 2019-03-08 1:29 UTC (permalink / raw)
To: u-boot
This patch was originally posted as a single one, but then was
merged in my "run -e." Now I would like to post it on its own.
With this patch, EFI Boot Manager will handles BootNext and BootCurrent
variable as UEFI specification describes.
Changes in v3 (Mar 8, 2019)
* add error messages around BootNext
* delete BootNext anyway when processing BootOrder
Changes in v2 (Mar 5, 2019)
* extract this patch from my "run -e" patch set
* use efi_[get|set]_variable instead of 'rs->*'
* add EFI_CALL
* cosmetic changes
AKASHI Takahiro (1):
efi_loader: bootmgr: support BootNext and BootCurrent variable
behavior
lib/efi_loader/efi_bootmgr.c | 54 +++++++++++++++++++++++++++++++++---
1 file changed, 50 insertions(+), 4 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 4+ messages in thread* [U-Boot] [PATCH v3 1/1] efi_loader: bootmgr: support BootNext and BootCurrent variable behavior 2019-03-08 1:29 [U-Boot] [PATCH v3 0/1] efi_loader: support BootNext and BootCurrent AKASHI Takahiro @ 2019-03-08 1:29 ` AKASHI Takahiro 2019-03-08 7:06 ` Heinrich Schuchardt 0 siblings, 1 reply; 4+ messages in thread From: AKASHI Takahiro @ 2019-03-08 1:29 UTC (permalink / raw) To: u-boot See UEFI v2.7, section 3.1.2 for details of the specification. With efidebug command, you can run any EFI boot option as follows: => efi boot add 1 SHELL ... => efi boot add 2 HELLO ... => efi boot order 1 2 => efi bootmgr (starting SHELL ...) => efi boot next 2 => efi bootmgr (starting HELLO ...) => env print -e <snip ...> BootCurrent: {boot,run}(blob) 00000000: 02 00 .. BootOrder: {boot,run}(blob) 00000000: 01 00 02 00 .... Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- lib/efi_loader/efi_bootmgr.c | 54 +++++++++++++++++++++++++++++++++--- 1 file changed, 50 insertions(+), 4 deletions(-) diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 417016102b48..64fc4449446b 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -141,6 +141,7 @@ static void *try_load_entry(uint16_t n, struct efi_device_path **device_path, efi_deserialize_load_option(&lo, load_option); if (lo.attributes & LOAD_OPTION_ACTIVE) { + u32 attributes; efi_status_t ret; debug("%s: trying to load \"%ls\" from %pD\n", @@ -151,6 +152,16 @@ static void *try_load_entry(uint16_t n, struct efi_device_path **device_path, if (ret != EFI_SUCCESS) goto error; + attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS; + size = sizeof(n); + ret = EFI_CALL(efi_set_variable( + L"BootCurrent", + (efi_guid_t *)&efi_global_variable_guid, + attributes, size, &n)); + if (ret != EFI_SUCCESS) + goto error; + printf("Booting: %ls\n", lo.label); efi_dp_split_file_path(lo.file_path, device_path, file_path); } @@ -162,21 +173,56 @@ error: } /* - * Attempt to load, in the order specified by BootOrder EFI variable, the - * available load-options, finding and returning the first one that can - * be loaded successfully. + * Attempt to load from BootNext or in the order specified by BootOrder + * EFI variable, the available load-options, finding and returning + * the first one that can be loaded successfully. */ void *efi_bootmgr_load(struct efi_device_path **device_path, struct efi_device_path **file_path) { - uint16_t *bootorder; + u16 bootnext, *bootorder; efi_uintn_t size; void *image = NULL; int i, num; + efi_status_t ret; bs = systab.boottime; rs = systab.runtime; + /* BootNext */ + bootnext = 0; + size = sizeof(bootnext); + ret = EFI_CALL(efi_get_variable(L"BootNext", + (efi_guid_t *)&efi_global_variable_guid, + NULL, &size, &bootnext)); + if (ret == EFI_SUCCESS) { + /* delete BootNext */ + ret = EFI_CALL(efi_set_variable( + L"BootNext", + (efi_guid_t *)&efi_global_variable_guid, + 0, 0, &bootnext)); + if (ret == EFI_SUCCESS) { + image = try_load_entry(bootnext, + device_path, file_path); + if (image) + return image; + } else { + printf("BootNext note deleted\n"); + } + } else if (ret != EFI_NOT_FOUND) { + if (ret == EFI_BUFFER_TOO_SMALL) + printf("BootNext must be 16-bit integer\n"); + + /* delete BootNext */ + ret = EFI_CALL(efi_set_variable( + L"BootNext", + (efi_guid_t *)&efi_global_variable_guid, + 0, 0, &bootnext)); + if (ret != EFI_SUCCESS) + printf("BootNext note deleted\n"); + } + + /* BootOrder */ bootorder = get_var(L"BootOrder", &efi_global_variable_guid, &size); if (!bootorder) { printf("BootOrder not defined\n"); -- 2.20.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH v3 1/1] efi_loader: bootmgr: support BootNext and BootCurrent variable behavior 2019-03-08 1:29 ` [U-Boot] [PATCH v3 1/1] efi_loader: bootmgr: support BootNext and BootCurrent variable behavior AKASHI Takahiro @ 2019-03-08 7:06 ` Heinrich Schuchardt 2019-03-19 8:36 ` AKASHI Takahiro 0 siblings, 1 reply; 4+ messages in thread From: Heinrich Schuchardt @ 2019-03-08 7:06 UTC (permalink / raw) To: u-boot On 3/8/19 2:29 AM, AKASHI Takahiro wrote: > See UEFI v2.7, section 3.1.2 for details of the specification. > > With efidebug command, you can run any EFI boot option as follows: > => efi boot add 1 SHELL ... > => efi boot add 2 HELLO ... > => efi boot order 1 2 > => efi bootmgr > (starting SHELL ...) > > => efi boot next 2 > => efi bootmgr > (starting HELLO ...) > => env print -e > <snip ...> > BootCurrent: {boot,run}(blob) > 00000000: 02 00 .. > BootOrder: {boot,run}(blob) > 00000000: 01 00 02 00 .... > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > lib/efi_loader/efi_bootmgr.c | 54 +++++++++++++++++++++++++++++++++--- > 1 file changed, 50 insertions(+), 4 deletions(-) > > diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c > index 417016102b48..64fc4449446b 100644 > --- a/lib/efi_loader/efi_bootmgr.c > +++ b/lib/efi_loader/efi_bootmgr.c > @@ -141,6 +141,7 @@ static void *try_load_entry(uint16_t n, struct efi_device_path **device_path, > efi_deserialize_load_option(&lo, load_option); > > if (lo.attributes & LOAD_OPTION_ACTIVE) { > + u32 attributes; > efi_status_t ret; > > debug("%s: trying to load \"%ls\" from %pD\n", > @@ -151,6 +152,16 @@ static void *try_load_entry(uint16_t n, struct efi_device_path **device_path, > if (ret != EFI_SUCCESS) > goto error; > > + attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS | > + EFI_VARIABLE_RUNTIME_ACCESS; > + size = sizeof(n); > + ret = EFI_CALL(efi_set_variable( > + L"BootCurrent", > + (efi_guid_t *)&efi_global_variable_guid, > + attributes, size, &n)); > + if (ret != EFI_SUCCESS) > + goto error; > + > printf("Booting: %ls\n", lo.label); > efi_dp_split_file_path(lo.file_path, device_path, file_path); > } > @@ -162,21 +173,56 @@ error: > } > > /* > - * Attempt to load, in the order specified by BootOrder EFI variable, the > - * available load-options, finding and returning the first one that can > - * be loaded successfully. > + * Attempt to load from BootNext or in the order specified by BootOrder > + * EFI variable, the available load-options, finding and returning > + * the first one that can be loaded successfully. > */ > void *efi_bootmgr_load(struct efi_device_path **device_path, > struct efi_device_path **file_path) > { > - uint16_t *bootorder; > + u16 bootnext, *bootorder; > efi_uintn_t size; > void *image = NULL; > int i, num; > + efi_status_t ret; > > bs = systab.boottime; > rs = systab.runtime; > > + /* BootNext */ > + bootnext = 0; > + size = sizeof(bootnext); > + ret = EFI_CALL(efi_get_variable(L"BootNext", > + (efi_guid_t *)&efi_global_variable_guid, > + NULL, &size, &bootnext)); > + if (ret == EFI_SUCCESS) { if (ret == EFI_SUCCESS && size == sizeof(u16)) If we do not check the size the user will never be informed that his system is corrupted. > + /* delete BootNext */ > + ret = EFI_CALL(efi_set_variable( > + L"BootNext", > + (efi_guid_t *)&efi_global_variable_guid, > + 0, 0, &bootnext)); > + if (ret == EFI_SUCCESS) { > + image = try_load_entry(bootnext, > + device_path, file_path); > + if (image) > + return image; > + } else { > + printf("BootNext note deleted\n"); %s/note/not/ "Deleting BootNext failed\n" would make it clearer that this is an error. > + } > + } else if (ret != EFI_NOT_FOUND) { > + if (ret == EFI_BUFFER_TOO_SMALL) > + printf("BootNext must be 16-bit integer\n"); > + > + /* delete BootNext */ > + ret = EFI_CALL(efi_set_variable( > + L"BootNext", > + (efi_guid_t *)&efi_global_variable_guid, > + 0, 0, &bootnext)); > + if (ret != EFI_SUCCESS) > + printf("BootNext note deleted\n"); %s/note/not/ Or maybe "Deleting BootNext failed\n" The same logic can be implemented without duplicate code but with size check: /* BootNext */ size = sizeof(bootnext); ret = EFI_CALL(efi_get_variable(L"BootNext", (efi_guid_t *)&efi_global_variable_guid, NULL, &size, &bootnext)); if (ret != EFI_NOT_FOUND) { if (ret == EFI_BUFFER_TOO_SMALL || size != sizeof(u16)) printf("BootNext must be 16-bit integer\n"); /* delete BootNext */ ret = EFI_CALL(efi_set_variable(L"BootNext", (efi_guid_t *)&efi_global_variable_guid, 0, 0, &bootnext)); if (ret != EFI_SUCCESS) printf("Deleting BootNext failed\n"); } if (ret == EFI_SUCCESS && size == sizeof(u16)) { image = try_load_entry(bootnext, device_path, file_path); if (image) return image; } Best regards Heinrich > + } > + > + /* BootOrder */ > bootorder = get_var(L"BootOrder", &efi_global_variable_guid, &size); > if (!bootorder) { > printf("BootOrder not defined\n"); > ^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH v3 1/1] efi_loader: bootmgr: support BootNext and BootCurrent variable behavior 2019-03-08 7:06 ` Heinrich Schuchardt @ 2019-03-19 8:36 ` AKASHI Takahiro 0 siblings, 0 replies; 4+ messages in thread From: AKASHI Takahiro @ 2019-03-19 8:36 UTC (permalink / raw) To: u-boot Heinrich, On Fri, Mar 08, 2019 at 08:06:06AM +0100, Heinrich Schuchardt wrote: > On 3/8/19 2:29 AM, AKASHI Takahiro wrote: > > See UEFI v2.7, section 3.1.2 for details of the specification. > > > > With efidebug command, you can run any EFI boot option as follows: > > => efi boot add 1 SHELL ... > > => efi boot add 2 HELLO ... > > => efi boot order 1 2 > > => efi bootmgr > > (starting SHELL ...) > > > > => efi boot next 2 > > => efi bootmgr > > (starting HELLO ...) > > => env print -e > > <snip ...> > > BootCurrent: {boot,run}(blob) > > 00000000: 02 00 .. > > BootOrder: {boot,run}(blob) > > 00000000: 01 00 02 00 .... > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > --- > > lib/efi_loader/efi_bootmgr.c | 54 +++++++++++++++++++++++++++++++++--- > > 1 file changed, 50 insertions(+), 4 deletions(-) > > > > diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c > > index 417016102b48..64fc4449446b 100644 > > --- a/lib/efi_loader/efi_bootmgr.c > > +++ b/lib/efi_loader/efi_bootmgr.c > > @@ -141,6 +141,7 @@ static void *try_load_entry(uint16_t n, struct efi_device_path **device_path, > > efi_deserialize_load_option(&lo, load_option); > > > > if (lo.attributes & LOAD_OPTION_ACTIVE) { > > + u32 attributes; > > efi_status_t ret; > > > > debug("%s: trying to load \"%ls\" from %pD\n", > > @@ -151,6 +152,16 @@ static void *try_load_entry(uint16_t n, struct efi_device_path **device_path, > > if (ret != EFI_SUCCESS) > > goto error; > > > > + attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS | > > + EFI_VARIABLE_RUNTIME_ACCESS; > > + size = sizeof(n); > > + ret = EFI_CALL(efi_set_variable( > > + L"BootCurrent", > > + (efi_guid_t *)&efi_global_variable_guid, > > + attributes, size, &n)); > > + if (ret != EFI_SUCCESS) > > + goto error; > > + > > printf("Booting: %ls\n", lo.label); > > efi_dp_split_file_path(lo.file_path, device_path, file_path); > > } > > @@ -162,21 +173,56 @@ error: > > } > > > > /* > > - * Attempt to load, in the order specified by BootOrder EFI variable, the > > - * available load-options, finding and returning the first one that can > > - * be loaded successfully. > > + * Attempt to load from BootNext or in the order specified by BootOrder > > + * EFI variable, the available load-options, finding and returning > > + * the first one that can be loaded successfully. > > */ > > void *efi_bootmgr_load(struct efi_device_path **device_path, > > struct efi_device_path **file_path) > > { > > - uint16_t *bootorder; > > + u16 bootnext, *bootorder; > > efi_uintn_t size; > > void *image = NULL; > > int i, num; > > + efi_status_t ret; > > > > bs = systab.boottime; > > rs = systab.runtime; > > > > + /* BootNext */ > > + bootnext = 0; > > + size = sizeof(bootnext); > > + ret = EFI_CALL(efi_get_variable(L"BootNext", > > + (efi_guid_t *)&efi_global_variable_guid, > > + NULL, &size, &bootnext)); > > + if (ret == EFI_SUCCESS) { > > if (ret == EFI_SUCCESS && size == sizeof(u16)) As I said before, even if size == 1 (byte), the system can work perfectly, but I don't care any more. > If we do not check the size the user will never be informed that his > system is corrupted. > > > + /* delete BootNext */ > > + ret = EFI_CALL(efi_set_variable( > > + L"BootNext", > > + (efi_guid_t *)&efi_global_variable_guid, > > + 0, 0, &bootnext)); > > + if (ret == EFI_SUCCESS) { > > + image = try_load_entry(bootnext, > > + device_path, file_path); > > + if (image) > > + return image; > > + } else { > > + printf("BootNext note deleted\n"); > > %s/note/not/ > > "Deleting BootNext failed\n" would make it clearer that this is an error. > > > > + } > > + } else if (ret != EFI_NOT_FOUND) { > > + if (ret == EFI_BUFFER_TOO_SMALL) > > + printf("BootNext must be 16-bit integer\n"); > > + > > + /* delete BootNext */ > > + ret = EFI_CALL(efi_set_variable( > > + L"BootNext", > > + (efi_guid_t *)&efi_global_variable_guid, > > + 0, 0, &bootnext)); > > + if (ret != EFI_SUCCESS) > > + printf("BootNext note deleted\n"); > > %s/note/not/ > > Or maybe "Deleting BootNext failed\n" > > The same logic can be implemented without duplicate code but with size > check: > > /* BootNext */ > size = sizeof(bootnext); > ret = EFI_CALL(efi_get_variable(L"BootNext", > (efi_guid_t *)&efi_global_variable_guid, > NULL, &size, &bootnext)); > if (ret != EFI_NOT_FOUND) { > if (ret == EFI_BUFFER_TOO_SMALL || size != sizeof(u16)) > printf("BootNext must be 16-bit integer\n"); > /* delete BootNext */ > ret = EFI_CALL(efi_set_variable(L"BootNext", > (efi_guid_t *)&efi_global_variable_guid, > 0, 0, &bootnext)); > if (ret != EFI_SUCCESS) > printf("Deleting BootNext failed\n"); > } > if (ret == EFI_SUCCESS && size == sizeof(u16)) { > image = try_load_entry(bootnext, > device_path, file_path); > if (image) > return image; > } Strictly speaking, this code doesn't work correctly. If * get_variable() failed for some reason (neither NOT_FOUND nor BUFFER_TOO_SMALL), then "size" still remains to be 2, and * set_variable() succeeded, then "ret" got EFI_SUCCESS, "if (ret == EFI_SUCCESS && size == sizeof(u16))" will wrongly match. I will fix this issue any way in v4, but the code will look a bit ugly. Thanks, -Takahiro Akashi > Best regards > > Heinrich > > > + } > > + > > + /* BootOrder */ > > bootorder = get_var(L"BootOrder", &efi_global_variable_guid, &size); > > if (!bootorder) { > > printf("BootOrder not defined\n"); > > > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-03-19 8:36 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-03-08 1:29 [U-Boot] [PATCH v3 0/1] efi_loader: support BootNext and BootCurrent AKASHI Takahiro 2019-03-08 1:29 ` [U-Boot] [PATCH v3 1/1] efi_loader: bootmgr: support BootNext and BootCurrent variable behavior AKASHI Takahiro 2019-03-08 7:06 ` Heinrich Schuchardt 2019-03-19 8:36 ` AKASHI Takahiro
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox