* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.