[U-Boot] [PATCH v3 0/1] efi_loader: support BootNext and BootCurrent
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(-)
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");
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");
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");
participants (2)
-
AKASHI Takahiro -
Heinrich Schuchardt