[U-Boot] [PATCH 0/5] efi_loader: run a specific efi application more easily

This patch is a result from re-organizing my previous patches; a combination of [1] and part of [2] so as solely to provide several ways of executing a specific efi application explicitly. * bootmanager via BootNext variable * bootefi with boot id * run -e
[1] https://lists.denx.de/pipermail/u-boot/2018-November/349281.html [2] https://lists.denx.de/pipermail/u-boot/2018-November/346450.html
AKASHI Takahiro (5): efi_loader: bootmgr: support BootNext and BootCurrent variable behavior efi_loader: bootmgr: allow for running a given load option cmd: bootefi: carve out fdt parameter handling cmd: bootefi: run an EFI application of a specific load option cmd: run: add "-e" option to run an EFI application
cmd/bootefi.c | 71 +++++++++++++++++++++++++----------- cmd/bootefi.h | 3 ++ cmd/nvedit.c | 5 +++ common/cli.c | 31 ++++++++++++++++ include/command.h | 3 ++ include/efi_loader.h | 3 +- lib/efi_loader/efi_bootmgr.c | 46 ++++++++++++++++++++++- 7 files changed, 138 insertions(+), 24 deletions(-) create mode 100644 cmd/bootefi.h

See UEFI v2.7, section 3.1.2 for details of the specification.
With my efishell command[1], you can try as the following: => efi boot add 1 SHELL ... => efi boot add 2 HELLO ... => efi boot order 1 2 => efi bootmgr (starting SHELL ...) => efi setvar BootNext =H0200 => efi bootmgr (starting HELLO ...) => efi dumpvar <snip ...> BootCurrent: {boot,run}(blob) 00000000: 02 00 .. BootOrder: {boot,run}(blob) 00000000: 01 00 02 00 ....
Using "run -e" would be more human-friendly, though.
[1] https://lists.denx.de/pipermail/u-boot/2018-November/346450.html
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- lib/efi_loader/efi_bootmgr.c | 37 +++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index a095df3f540b..a54ae28343ce 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -145,11 +145,21 @@ 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", __func__, lo.label, lo.file_path);
+ attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS; + size = sizeof(n); + ret = rs->set_variable(L"BootCurrent", + (efi_guid_t *)&efi_global_variable_guid, + attributes, size, &n); + if (ret != EFI_SUCCESS) + goto error; + ret = efi_load_image_from_path(lo.file_path, &image);
if (ret != EFI_SUCCESS) @@ -173,16 +183,41 @@ error: void *efi_bootmgr_load(struct efi_device_path **device_path, struct efi_device_path **file_path) { - uint16_t *bootorder; + u16 bootnext, *bootorder; + u32 attributes; efi_uintn_t size; void *image = NULL; int i, num; + efi_status_t ret;
__efi_entry_check();
bs = systab.boottime; rs = systab.runtime;
+ /* BootNext */ + size = sizeof(bootnext); + ret = rs->get_variable(L"BootNext", + (efi_guid_t *)&efi_global_variable_guid, + NULL, &size, &bootnext); + if (!bootnext) + goto run_list; + + attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS; + size = 0; + ret = rs->set_variable(L"BootNext", + (efi_guid_t *)&efi_global_variable_guid, + attributes, size, &bootnext); + if (ret != EFI_SUCCESS) + goto error; + + image = try_load_entry(bootnext, device_path, file_path); + if (image) + goto error; + +run_list: + /* BootOrder */ bootorder = get_var(L"BootOrder", &efi_global_variable_guid, &size); if (!bootorder) goto error;

On 18.12.18 06:02, AKASHI Takahiro wrote:
See UEFI v2.7, section 3.1.2 for details of the specification.
With my efishell command[1], you can try as the following: => efi boot add 1 SHELL ... => efi boot add 2 HELLO ... => efi boot order 1 2 => efi bootmgr (starting SHELL ...) => efi setvar BootNext =H0200 => efi bootmgr (starting HELLO ...) => efi dumpvar <snip ...> BootCurrent: {boot,run}(blob) 00000000: 02 00 .. BootOrder: {boot,run}(blob) 00000000: 01 00 02 00 ....
Using "run -e" would be more human-friendly, though.
[1] https://lists.denx.de/pipermail/u-boot/2018-November/346450.html
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
lib/efi_loader/efi_bootmgr.c | 37 +++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index a095df3f540b..a54ae28343ce 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -145,11 +145,21 @@ 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", __func__, lo.label, lo.file_path);
attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_RUNTIME_ACCESS;
size = sizeof(n);
ret = rs->set_variable(L"BootCurrent",
(efi_guid_t *)&efi_global_variable_guid,
attributes, size, &n);
Every call into UEFI land (rs->foo(), bs->foo(), etc) has to go via the EFI_CALL() macro. Otherwise we may destroy the "gd" pointer on ARM.
if (ret != EFI_SUCCESS)
goto error;
ret = efi_load_image_from_path(lo.file_path, &image);
if (ret != EFI_SUCCESS)
@@ -173,16 +183,41 @@ error: void *efi_bootmgr_load(struct efi_device_path **device_path, struct efi_device_path **file_path) {
- uint16_t *bootorder;
u16 bootnext, *bootorder;
u32 attributes; efi_uintn_t size; void *image = NULL; int i, num;
efi_status_t ret;
__efi_entry_check();
bs = systab.boottime; rs = systab.runtime;
/* BootNext */
size = sizeof(bootnext);
ret = rs->get_variable(L"BootNext",
(efi_guid_t *)&efi_global_variable_guid,
NULL, &size, &bootnext);
... same here.
- if (!bootnext)
goto run_list;
- attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_RUNTIME_ACCESS;
- size = 0;
- ret = rs->set_variable(L"BootNext",
(efi_guid_t *)&efi_global_variable_guid,
attributes, size, &bootnext);
... same here.
Alex
- if (ret != EFI_SUCCESS)
goto error;
- image = try_load_entry(bootnext, device_path, file_path);
- if (image)
goto error;
+run_list:
- /* BootOrder */ bootorder = get_var(L"BootOrder", &efi_global_variable_guid, &size); if (!bootorder) goto error;

On Sun, Dec 23, 2018 at 03:03:51AM +0100, Alexander Graf wrote:
On 18.12.18 06:02, AKASHI Takahiro wrote:
See UEFI v2.7, section 3.1.2 for details of the specification.
With my efishell command[1], you can try as the following: => efi boot add 1 SHELL ... => efi boot add 2 HELLO ... => efi boot order 1 2 => efi bootmgr (starting SHELL ...) => efi setvar BootNext =H0200 => efi bootmgr (starting HELLO ...) => efi dumpvar <snip ...> BootCurrent: {boot,run}(blob) 00000000: 02 00 .. BootOrder: {boot,run}(blob) 00000000: 01 00 02 00 ....
Using "run -e" would be more human-friendly, though.
[1] https://lists.denx.de/pipermail/u-boot/2018-November/346450.html
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
lib/efi_loader/efi_bootmgr.c | 37 +++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index a095df3f540b..a54ae28343ce 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -145,11 +145,21 @@ 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", __func__, lo.label, lo.file_path);
attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_RUNTIME_ACCESS;
size = sizeof(n);
ret = rs->set_variable(L"BootCurrent",
(efi_guid_t *)&efi_global_variable_guid,
attributes, size, &n);
Every call into UEFI land (rs->foo(), bs->foo(), etc) has to go via the EFI_CALL() macro. Otherwise we may destroy the "gd" pointer on ARM.
OK, but let me make sure one thing: My efishell calls efi_* functions directly in some places as Not all the features can be implemented only with boot/runtime services. In those cases, we don't have to use EFI_CALL(), right?
Thanks, -Takahiro Akashi
if (ret != EFI_SUCCESS)
goto error;
ret = efi_load_image_from_path(lo.file_path, &image);
if (ret != EFI_SUCCESS)
@@ -173,16 +183,41 @@ error: void *efi_bootmgr_load(struct efi_device_path **device_path, struct efi_device_path **file_path) {
- uint16_t *bootorder;
u16 bootnext, *bootorder;
u32 attributes; efi_uintn_t size; void *image = NULL; int i, num;
efi_status_t ret;
__efi_entry_check();
bs = systab.boottime; rs = systab.runtime;
/* BootNext */
size = sizeof(bootnext);
ret = rs->get_variable(L"BootNext",
(efi_guid_t *)&efi_global_variable_guid,
NULL, &size, &bootnext);
... same here.
- if (!bootnext)
goto run_list;
- attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_RUNTIME_ACCESS;
- size = 0;
- ret = rs->set_variable(L"BootNext",
(efi_guid_t *)&efi_global_variable_guid,
attributes, size, &bootnext);
... same here.
Alex
- if (ret != EFI_SUCCESS)
goto error;
- image = try_load_entry(bootnext, device_path, file_path);
- if (image)
goto error;
+run_list:
- /* BootOrder */ bootorder = get_var(L"BootOrder", &efi_global_variable_guid, &size); if (!bootorder) goto error;

On 25.12.18 10:36, AKASHI Takahiro wrote:
On Sun, Dec 23, 2018 at 03:03:51AM +0100, Alexander Graf wrote:
On 18.12.18 06:02, AKASHI Takahiro wrote:
See UEFI v2.7, section 3.1.2 for details of the specification.
With my efishell command[1], you can try as the following: => efi boot add 1 SHELL ... => efi boot add 2 HELLO ... => efi boot order 1 2 => efi bootmgr (starting SHELL ...) => efi setvar BootNext =H0200 => efi bootmgr (starting HELLO ...) => efi dumpvar <snip ...> BootCurrent: {boot,run}(blob) 00000000: 02 00 .. BootOrder: {boot,run}(blob) 00000000: 01 00 02 00 ....
Using "run -e" would be more human-friendly, though.
[1] https://lists.denx.de/pipermail/u-boot/2018-November/346450.html
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
lib/efi_loader/efi_bootmgr.c | 37 +++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index a095df3f540b..a54ae28343ce 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -145,11 +145,21 @@ 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", __func__, lo.label, lo.file_path);
attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_RUNTIME_ACCESS;
size = sizeof(n);
ret = rs->set_variable(L"BootCurrent",
(efi_guid_t *)&efi_global_variable_guid,
attributes, size, &n);
Every call into UEFI land (rs->foo(), bs->foo(), etc) has to go via the EFI_CALL() macro. Otherwise we may destroy the "gd" pointer on ARM.
OK, but let me make sure one thing: My efishell calls efi_* functions directly in some places as Not all the features can be implemented only with boot/runtime services. In those cases, we don't have to use EFI_CALL(), right?
If your "efishell" is a UEFI binary, you can directly call boot/runtime services. If it runs as part of the U-Boot code base, every call to boot/runtime service callbacks *must* go through EFI_CALL().
Alex

On 12/26/18 10:23 PM, Alexander Graf wrote:
On 25.12.18 10:36, AKASHI Takahiro wrote:
On Sun, Dec 23, 2018 at 03:03:51AM +0100, Alexander Graf wrote:
On 18.12.18 06:02, AKASHI Takahiro wrote:
See UEFI v2.7, section 3.1.2 for details of the specification.
With my efishell command[1], you can try as the following: => efi boot add 1 SHELL ... => efi boot add 2 HELLO ... => efi boot order 1 2 => efi bootmgr (starting SHELL ...) => efi setvar BootNext =H0200 => efi bootmgr (starting HELLO ...) => efi dumpvar <snip ...> BootCurrent: {boot,run}(blob) 00000000: 02 00 .. BootOrder: {boot,run}(blob) 00000000: 01 00 02 00 ....
Using "run -e" would be more human-friendly, though.
[1] https://lists.denx.de/pipermail/u-boot/2018-November/346450.html
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
lib/efi_loader/efi_bootmgr.c | 37 +++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index a095df3f540b..a54ae28343ce 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -145,11 +145,21 @@ 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", __func__, lo.label, lo.file_path);
attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_RUNTIME_ACCESS;
size = sizeof(n);
ret = rs->set_variable(L"BootCurrent",
(efi_guid_t *)&efi_global_variable_guid,
attributes, size, &n);
Every call into UEFI land (rs->foo(), bs->foo(), etc) has to go via the EFI_CALL() macro. Otherwise we may destroy the "gd" pointer on ARM.
OK, but let me make sure one thing: My efishell calls efi_* functions directly in some places as Not all the features can be implemented only with boot/runtime services. In those cases, we don't have to use EFI_CALL(), right?
If your "efishell" is a UEFI binary, you can directly call boot/runtime services. If it runs as part of the U-Boot code base, every call to boot/runtime service callbacks *must* go through EFI_CALL().
No, that is not how the call counting has been set up. You will get an assert error if debugging is enabled. We use EFI_CALL when we want to call an API function from inside an API function.
Just have a look at all the selftest code. It never uses EFI_CALL.
In this respect the bootmgr code is broken.
Best regards
Heinrich
Alex

On 26.12.18 22:33, Heinrich Schuchardt wrote:
On 12/26/18 10:23 PM, Alexander Graf wrote:
On 25.12.18 10:36, AKASHI Takahiro wrote:
On Sun, Dec 23, 2018 at 03:03:51AM +0100, Alexander Graf wrote:
On 18.12.18 06:02, AKASHI Takahiro wrote:
See UEFI v2.7, section 3.1.2 for details of the specification.
With my efishell command[1], you can try as the following: => efi boot add 1 SHELL ... => efi boot add 2 HELLO ... => efi boot order 1 2 => efi bootmgr (starting SHELL ...) => efi setvar BootNext =H0200 => efi bootmgr (starting HELLO ...) => efi dumpvar <snip ...> BootCurrent: {boot,run}(blob) 00000000: 02 00 .. BootOrder: {boot,run}(blob) 00000000: 01 00 02 00 ....
Using "run -e" would be more human-friendly, though.
[1] https://lists.denx.de/pipermail/u-boot/2018-November/346450.html
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
lib/efi_loader/efi_bootmgr.c | 37 +++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index a095df3f540b..a54ae28343ce 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -145,11 +145,21 @@ 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", __func__, lo.label, lo.file_path);
attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_RUNTIME_ACCESS;
size = sizeof(n);
ret = rs->set_variable(L"BootCurrent",
(efi_guid_t *)&efi_global_variable_guid,
attributes, size, &n);
Every call into UEFI land (rs->foo(), bs->foo(), etc) has to go via the EFI_CALL() macro. Otherwise we may destroy the "gd" pointer on ARM.
OK, but let me make sure one thing: My efishell calls efi_* functions directly in some places as Not all the features can be implemented only with boot/runtime services. In those cases, we don't have to use EFI_CALL(), right?
If your "efishell" is a UEFI binary, you can directly call boot/runtime services. If it runs as part of the U-Boot code base, every call to boot/runtime service callbacks *must* go through EFI_CALL().
No, that is not how the call counting has been set up. You will get an assert error if debugging is enabled. We use EFI_CALL when we want to call an API function from inside an API function.
Ah, true. I stand corrected.
Just have a look at all the selftest code. It never uses EFI_CALL.
In this respect the bootmgr code is broken.
Yes. Maybe this calls for a few travis checks with debugging enabled?
Alex

On 12/18/18 6:02 AM, AKASHI Takahiro wrote:
See UEFI v2.7, section 3.1.2 for details of the specification.
With my efishell command[1], you can try as the following: => efi boot add 1 SHELL ... => efi boot add 2 HELLO ... => efi boot order 1 2 => efi bootmgr (starting SHELL ...) => efi setvar BootNext =H0200
If you already have an "efi boot" sub-command wouldn't the following syntax make more sense?
efi boot next 2
Best regards
Heinrich
=> efi bootmgr (starting HELLO ...) => efi dumpvar <snip ...> BootCurrent: {boot,run}(blob) 00000000: 02 00 .. BootOrder: {boot,run}(blob) 00000000: 01 00 02 00 ....
Using "run -e" would be more human-friendly, though.
[1] https://lists.denx.de/pipermail/u-boot/2018-November/346450.html
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
lib/efi_loader/efi_bootmgr.c | 37 +++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index a095df3f540b..a54ae28343ce 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -145,11 +145,21 @@ 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", __func__, lo.label, lo.file_path);
attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_RUNTIME_ACCESS;
size = sizeof(n);
ret = rs->set_variable(L"BootCurrent",
(efi_guid_t *)&efi_global_variable_guid,
attributes, size, &n);
if (ret != EFI_SUCCESS)
goto error;
ret = efi_load_image_from_path(lo.file_path, &image);
if (ret != EFI_SUCCESS)
@@ -173,16 +183,41 @@ error: void *efi_bootmgr_load(struct efi_device_path **device_path, struct efi_device_path **file_path) {
- uint16_t *bootorder;
u16 bootnext, *bootorder;
u32 attributes; efi_uintn_t size; void *image = NULL; int i, num;
efi_status_t ret;
__efi_entry_check();
bs = systab.boottime; rs = systab.runtime;
/* BootNext */
size = sizeof(bootnext);
ret = rs->get_variable(L"BootNext",
(efi_guid_t *)&efi_global_variable_guid,
NULL, &size, &bootnext);
if (!bootnext)
goto run_list;
attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_RUNTIME_ACCESS;
According to the UEFI spec you do not need any attributes when deleting a variable.
Please, add a comment describing what you are doing here:
/* Delete BootNext variable */
Best regards
Heinrich
- size = 0;
- ret = rs->set_variable(L"BootNext",
(efi_guid_t *)&efi_global_variable_guid,
attributes, size, &bootnext);
- if (ret != EFI_SUCCESS)
goto error;
- image = try_load_entry(bootnext, device_path, file_path);
- if (image)
goto error;
+run_list:
- /* BootOrder */ bootorder = get_var(L"BootOrder", &efi_global_variable_guid, &size); if (!bootorder) goto error;

On Thu, Dec 27, 2018 at 05:58:49AM +0100, Heinrich Schuchardt wrote:
On 12/18/18 6:02 AM, AKASHI Takahiro wrote:
See UEFI v2.7, section 3.1.2 for details of the specification.
With my efishell command[1], you can try as the following: => efi boot add 1 SHELL ... => efi boot add 2 HELLO ... => efi boot order 1 2 => efi bootmgr (starting SHELL ...) => efi setvar BootNext =H0200
If you already have an "efi boot" sub-command wouldn't the following syntax make more sense?
efi boot next 2
OK.
Best regards
Heinrich
=> efi bootmgr (starting HELLO ...) => efi dumpvar <snip ...> BootCurrent: {boot,run}(blob) 00000000: 02 00 .. BootOrder: {boot,run}(blob) 00000000: 01 00 02 00 ....
Using "run -e" would be more human-friendly, though.
[1] https://lists.denx.de/pipermail/u-boot/2018-November/346450.html
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
lib/efi_loader/efi_bootmgr.c | 37 +++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index a095df3f540b..a54ae28343ce 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -145,11 +145,21 @@ 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", __func__, lo.label, lo.file_path);
attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_RUNTIME_ACCESS;
size = sizeof(n);
ret = rs->set_variable(L"BootCurrent",
(efi_guid_t *)&efi_global_variable_guid,
attributes, size, &n);
if (ret != EFI_SUCCESS)
goto error;
ret = efi_load_image_from_path(lo.file_path, &image);
if (ret != EFI_SUCCESS)
@@ -173,16 +183,41 @@ error: void *efi_bootmgr_load(struct efi_device_path **device_path, struct efi_device_path **file_path) {
- uint16_t *bootorder;
u16 bootnext, *bootorder;
u32 attributes; efi_uintn_t size; void *image = NULL; int i, num;
efi_status_t ret;
__efi_entry_check();
bs = systab.boottime; rs = systab.runtime;
/* BootNext */
size = sizeof(bootnext);
ret = rs->get_variable(L"BootNext",
(efi_guid_t *)&efi_global_variable_guid,
NULL, &size, &bootnext);
if (!bootnext)
goto run_list;
attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_RUNTIME_ACCESS;
According to the UEFI spec you do not need any attributes when deleting a variable.
OK.
Please, add a comment describing what you are doing here:
/* Delete BootNext variable */
OK.
-Takahiro Akashi
Best regards
Heinrich
- size = 0;
- ret = rs->set_variable(L"BootNext",
(efi_guid_t *)&efi_global_variable_guid,
attributes, size, &bootnext);
- if (ret != EFI_SUCCESS)
goto error;
- image = try_load_entry(bootnext, device_path, file_path);
- if (image)
goto error;
+run_list:
- /* BootOrder */ bootorder = get_var(L"BootOrder", &efi_global_variable_guid, &size); if (!bootorder) goto error;

With an extra argument, efi_bootmgr_load() can now load an efi binary based on a "BootXXXX" variable specified.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- cmd/bootefi.c | 2 +- include/efi_loader.h | 3 ++- lib/efi_loader/efi_bootmgr.c | 9 ++++++++- 3 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 7012d72ab50d..3ebae1cdad08 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -452,7 +452,7 @@ static int do_bootefi_bootmgr_exec(void) void *addr; efi_status_t r;
- addr = efi_bootmgr_load(&device_path, &file_path); + addr = efi_bootmgr_load(-1, &device_path, &file_path); if (!addr) return 1;
diff --git a/include/efi_loader.h b/include/efi_loader.h index dd68cfce5c65..5a6321122c9c 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -551,7 +551,8 @@ struct efi_load_option {
void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data); unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data); -void *efi_bootmgr_load(struct efi_device_path **device_path, +void *efi_bootmgr_load(int boot_id, + struct efi_device_path **device_path, struct efi_device_path **file_path);
#else /* CONFIG_IS_ENABLED(EFI_LOADER) */ diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index a54ae28343ce..db391147fb2d 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -180,7 +180,8 @@ error: * available load-options, finding and returning the first one that can * be loaded successfully. */ -void *efi_bootmgr_load(struct efi_device_path **device_path, +void *efi_bootmgr_load(int boot_id, + struct efi_device_path **device_path, struct efi_device_path **file_path) { u16 bootnext, *bootorder; @@ -195,6 +196,12 @@ void *efi_bootmgr_load(struct efi_device_path **device_path, bs = systab.boottime; rs = systab.runtime;
+ /* specified boot option */ + if (boot_id != -1) { + image = try_load_entry(boot_id, device_path, file_path); + goto error; + } + /* BootNext */ size = sizeof(bootnext); ret = rs->get_variable(L"BootNext",

On 18.12.18 06:02, AKASHI Takahiro wrote:
With an extra argument, efi_bootmgr_load() can now load an efi binary based on a "BootXXXX" variable specified.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/bootefi.c | 2 +- include/efi_loader.h | 3 ++- lib/efi_loader/efi_bootmgr.c | 9 ++++++++- 3 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 7012d72ab50d..3ebae1cdad08 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -452,7 +452,7 @@ static int do_bootefi_bootmgr_exec(void) void *addr; efi_status_t r;
- addr = efi_bootmgr_load(&device_path, &file_path);
- addr = efi_bootmgr_load(-1, &device_path, &file_path);
Please make the -1 a special #define that is more verbose to readers. Something like EFI_BOOTMGR_DEFAULT_ORDER.
Alex

On Sun, Dec 23, 2018 at 03:05:42AM +0100, Alexander Graf wrote:
On 18.12.18 06:02, AKASHI Takahiro wrote:
With an extra argument, efi_bootmgr_load() can now load an efi binary based on a "BootXXXX" variable specified.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/bootefi.c | 2 +- include/efi_loader.h | 3 ++- lib/efi_loader/efi_bootmgr.c | 9 ++++++++- 3 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 7012d72ab50d..3ebae1cdad08 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -452,7 +452,7 @@ static int do_bootefi_bootmgr_exec(void) void *addr; efi_status_t r;
- addr = efi_bootmgr_load(&device_path, &file_path);
- addr = efi_bootmgr_load(-1, &device_path, &file_path);
Please make the -1 a special #define that is more verbose to readers. Something like EFI_BOOTMGR_DEFAULT_ORDER.
OK Add it to efi_loader.h
-Takahiro Akashi
Alex

The current way how command parameters, particularly "fdt addr," are handled makes it a bit complicated to add a subcommand-specific parameter. So just refactor the code and extract efi_handle_fdt().
This commit is a preparatory change for enhancing bootmgr sub-command.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
Revert "fixup! fixup! cmd: bootefi: carve out fdt parameter handling"
This reverts commit abc315426e37bdfb96a48d23c1fc96399b68baa5. --- cmd/bootefi.c | 49 +++++++++++++++++++++++++++++++++---------------- cmd/bootefi.h | 3 +++ 2 files changed, 36 insertions(+), 16 deletions(-) create mode 100644 cmd/bootefi.h
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 3ebae1cdad08..796ca6ee69ec 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -279,6 +279,31 @@ static void bootefi_run_finish(struct efi_loaded_image_obj *image_obj, efi_delete_handle(&image_obj->header); }
+int efi_handle_fdt(char *fdt_opt) +{ + unsigned long fdt_addr; + efi_status_t r; + + if (fdt_opt) { + fdt_addr = simple_strtoul(fdt_opt, NULL, 16); + if (!fdt_addr && *fdt_opt != '0') + return CMD_RET_USAGE; + + /* Install device tree */ + r = efi_install_fdt(fdt_addr); + if (r != EFI_SUCCESS) { + printf("ERROR: failed to install device tree\n"); + return CMD_RET_FAILURE; + } + } else { + /* Remove device tree. EFI_NOT_FOUND can be ignored here */ + efi_install_configuration_table(&efi_guid_fdt, NULL); + printf("WARNING: booting without device tree\n"); + } + + return CMD_RET_SUCCESS; +} + /** * do_bootefi_exec() - execute EFI binary * @@ -473,7 +498,6 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) unsigned long addr; char *saddr; efi_status_t r; - unsigned long fdt_addr;
/* Allow unaligned memory access */ allow_unaligned(); @@ -489,21 +513,6 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (argc < 2) return CMD_RET_USAGE;
- if (argc > 2) { - fdt_addr = simple_strtoul(argv[2], NULL, 16); - if (!fdt_addr && *argv[2] != '0') - return CMD_RET_USAGE; - /* Install device tree */ - r = efi_install_fdt(fdt_addr); - if (r != EFI_SUCCESS) { - printf("ERROR: failed to install device tree\n"); - return CMD_RET_FAILURE; - } - } else { - /* Remove device tree. EFI_NOT_FOUND can be ignored here */ - efi_install_configuration_table(&efi_guid_fdt, NULL); - printf("WARNING: booting without device tree\n"); - } #ifdef CONFIG_CMD_BOOTEFI_HELLO if (!strcmp(argv[1], "hello")) { ulong size = __efi_helloworld_end - __efi_helloworld_begin; @@ -521,6 +530,9 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) struct efi_loaded_image_obj *image_obj; struct efi_loaded_image *loaded_image_info;
+ if (efi_handle_fdt(argc > 2 ? argv[2] : NULL)) + return CMD_RET_FAILURE; + if (bootefi_test_prepare(&image_obj, &loaded_image_info, "\selftest", (uintptr_t)&efi_selftest, "efi_selftest")) @@ -533,6 +545,9 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) } else #endif if (!strcmp(argv[1], "bootmgr")) { + if (efi_handle_fdt(argc > 2 ? argv[2] : NULL)) + return CMD_RET_FAILURE; + return do_bootefi_bootmgr_exec(); } else { saddr = argv[1]; @@ -542,6 +557,8 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (!addr && *saddr != '0') return CMD_RET_USAGE;
+ if (efi_handle_fdt(argc > 2 ? argv[2] : NULL)) + return CMD_RET_FAILURE; }
printf("## Starting EFI application at %08lx ...\n", addr); diff --git a/cmd/bootefi.h b/cmd/bootefi.h new file mode 100644 index 000000000000..4e11ab1211cb --- /dev/null +++ b/cmd/bootefi.h @@ -0,0 +1,3 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ + +int efi_handle_fdt(char *fdt_opt);

On 18.12.18 06:02, AKASHI Takahiro wrote:
The current way how command parameters, particularly "fdt addr," are handled makes it a bit complicated to add a subcommand-specific parameter. So just refactor the code and extract efi_handle_fdt().
This commit is a preparatory change for enhancing bootmgr sub-command.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
Revert "fixup! fixup! cmd: bootefi: carve out fdt parameter handling"
This reverts commit abc315426e37bdfb96a48d23c1fc96399b68baa5.
You sure you wanted this to be part of the commit message? ;)
cmd/bootefi.c | 49 +++++++++++++++++++++++++++++++++---------------- cmd/bootefi.h | 3 +++ 2 files changed, 36 insertions(+), 16 deletions(-) create mode 100644 cmd/bootefi.h
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 3ebae1cdad08..796ca6ee69ec 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -279,6 +279,31 @@ static void bootefi_run_finish(struct efi_loaded_image_obj *image_obj, efi_delete_handle(&image_obj->header); }
+int efi_handle_fdt(char *fdt_opt) +{
- unsigned long fdt_addr;
- efi_status_t r;
- if (fdt_opt) {
fdt_addr = simple_strtoul(fdt_opt, NULL, 16);
if (!fdt_addr && *fdt_opt != '0')
return CMD_RET_USAGE;
/* Install device tree */
r = efi_install_fdt(fdt_addr);
if (r != EFI_SUCCESS) {
printf("ERROR: failed to install device tree\n");
return CMD_RET_FAILURE;
}
- } else {
/* Remove device tree. EFI_NOT_FOUND can be ignored here */
efi_install_configuration_table(&efi_guid_fdt, NULL);
printf("WARNING: booting without device tree\n");
- }
- return CMD_RET_SUCCESS;
+}
/**
- do_bootefi_exec() - execute EFI binary
@@ -473,7 +498,6 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) unsigned long addr; char *saddr; efi_status_t r;
unsigned long fdt_addr;
/* Allow unaligned memory access */ allow_unaligned();
@@ -489,21 +513,6 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (argc < 2) return CMD_RET_USAGE;
- if (argc > 2) {
fdt_addr = simple_strtoul(argv[2], NULL, 16);
if (!fdt_addr && *argv[2] != '0')
return CMD_RET_USAGE;
/* Install device tree */
r = efi_install_fdt(fdt_addr);
if (r != EFI_SUCCESS) {
printf("ERROR: failed to install device tree\n");
return CMD_RET_FAILURE;
}
- } else {
/* Remove device tree. EFI_NOT_FOUND can be ignored here */
efi_install_configuration_table(&efi_guid_fdt, NULL);
printf("WARNING: booting without device tree\n");
- }
#ifdef CONFIG_CMD_BOOTEFI_HELLO if (!strcmp(argv[1], "hello")) { ulong size = __efi_helloworld_end - __efi_helloworld_begin; @@ -521,6 +530,9 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) struct efi_loaded_image_obj *image_obj; struct efi_loaded_image *loaded_image_info;
if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
return CMD_RET_FAILURE;
- if (bootefi_test_prepare(&image_obj, &loaded_image_info, "\selftest", (uintptr_t)&efi_selftest, "efi_selftest"))
@@ -533,6 +545,9 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) } else #endif if (!strcmp(argv[1], "bootmgr")) {
if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
return CMD_RET_FAILURE;
- return do_bootefi_bootmgr_exec(); } else { saddr = argv[1];
@@ -542,6 +557,8 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (!addr && *saddr != '0') return CMD_RET_USAGE;
if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
return CMD_RET_FAILURE;
}
printf("## Starting EFI application at %08lx ...\n", addr);
diff --git a/cmd/bootefi.h b/cmd/bootefi.h new file mode 100644 index 000000000000..4e11ab1211cb --- /dev/null +++ b/cmd/bootefi.h @@ -0,0 +1,3 @@ +/* SPDX-License-Identifier: GPL-2.0+ */
This should be a separate patch.
+int efi_handle_fdt(char *fdt_opt);

On Sun, Dec 23, 2018 at 03:08:52AM +0100, Alexander Graf wrote:
On 18.12.18 06:02, AKASHI Takahiro wrote:
The current way how command parameters, particularly "fdt addr," are handled makes it a bit complicated to add a subcommand-specific parameter. So just refactor the code and extract efi_handle_fdt().
This commit is a preparatory change for enhancing bootmgr sub-command.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
Revert "fixup! fixup! cmd: bootefi: carve out fdt parameter handling"
This reverts commit abc315426e37bdfb96a48d23c1fc96399b68baa5.
You sure you wanted this to be part of the commit message? ;)
Damn!
-Takahiro Akashi
cmd/bootefi.c | 49 +++++++++++++++++++++++++++++++++---------------- cmd/bootefi.h | 3 +++ 2 files changed, 36 insertions(+), 16 deletions(-) create mode 100644 cmd/bootefi.h
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 3ebae1cdad08..796ca6ee69ec 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -279,6 +279,31 @@ static void bootefi_run_finish(struct efi_loaded_image_obj *image_obj, efi_delete_handle(&image_obj->header); }
+int efi_handle_fdt(char *fdt_opt) +{
- unsigned long fdt_addr;
- efi_status_t r;
- if (fdt_opt) {
fdt_addr = simple_strtoul(fdt_opt, NULL, 16);
if (!fdt_addr && *fdt_opt != '0')
return CMD_RET_USAGE;
/* Install device tree */
r = efi_install_fdt(fdt_addr);
if (r != EFI_SUCCESS) {
printf("ERROR: failed to install device tree\n");
return CMD_RET_FAILURE;
}
- } else {
/* Remove device tree. EFI_NOT_FOUND can be ignored here */
efi_install_configuration_table(&efi_guid_fdt, NULL);
printf("WARNING: booting without device tree\n");
- }
- return CMD_RET_SUCCESS;
+}
/**
- do_bootefi_exec() - execute EFI binary
@@ -473,7 +498,6 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) unsigned long addr; char *saddr; efi_status_t r;
unsigned long fdt_addr;
/* Allow unaligned memory access */ allow_unaligned();
@@ -489,21 +513,6 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (argc < 2) return CMD_RET_USAGE;
- if (argc > 2) {
fdt_addr = simple_strtoul(argv[2], NULL, 16);
if (!fdt_addr && *argv[2] != '0')
return CMD_RET_USAGE;
/* Install device tree */
r = efi_install_fdt(fdt_addr);
if (r != EFI_SUCCESS) {
printf("ERROR: failed to install device tree\n");
return CMD_RET_FAILURE;
}
- } else {
/* Remove device tree. EFI_NOT_FOUND can be ignored here */
efi_install_configuration_table(&efi_guid_fdt, NULL);
printf("WARNING: booting without device tree\n");
- }
#ifdef CONFIG_CMD_BOOTEFI_HELLO if (!strcmp(argv[1], "hello")) { ulong size = __efi_helloworld_end - __efi_helloworld_begin; @@ -521,6 +530,9 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) struct efi_loaded_image_obj *image_obj; struct efi_loaded_image *loaded_image_info;
if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
return CMD_RET_FAILURE;
- if (bootefi_test_prepare(&image_obj, &loaded_image_info, "\selftest", (uintptr_t)&efi_selftest, "efi_selftest"))
@@ -533,6 +545,9 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) } else #endif if (!strcmp(argv[1], "bootmgr")) {
if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
return CMD_RET_FAILURE;
- return do_bootefi_bootmgr_exec(); } else { saddr = argv[1];
@@ -542,6 +557,8 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (!addr && *saddr != '0') return CMD_RET_USAGE;
if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
return CMD_RET_FAILURE;
}
printf("## Starting EFI application at %08lx ...\n", addr);
diff --git a/cmd/bootefi.h b/cmd/bootefi.h new file mode 100644 index 000000000000..4e11ab1211cb --- /dev/null +++ b/cmd/bootefi.h @@ -0,0 +1,3 @@ +/* SPDX-License-Identifier: GPL-2.0+ */
This should be a separate patch.
+int efi_handle_fdt(char *fdt_opt);

With this patch applied, we will be able to selectively execute an EFI application by specifying a load option, say "1" for Boot0001, "2" for Boot0002 and so on.
=> bootefi bootmgr <fdt addr> 1, or bootefi bootmgr - 1
Please note that BootXXXX need not be included in "BootOrder".
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- cmd/bootefi.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 796ca6ee69ec..2fc52e3056d2 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -471,13 +471,13 @@ static efi_status_t bootefi_test_prepare
#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
-static int do_bootefi_bootmgr_exec(void) +static int do_bootefi_bootmgr_exec(int boot_id) { struct efi_device_path *device_path, *file_path; void *addr; efi_status_t r;
- addr = efi_bootmgr_load(-1, &device_path, &file_path); + addr = efi_bootmgr_load(boot_id, &device_path, &file_path); if (!addr) return 1;
@@ -545,10 +545,22 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) } else #endif if (!strcmp(argv[1], "bootmgr")) { - if (efi_handle_fdt(argc > 2 ? argv[2] : NULL)) - return CMD_RET_FAILURE; + char *endp; + int boot_id = -1; + + if (argc > 2) + if (efi_handle_fdt((argv[2][0] == '-') ? + NULL : argv[2])) + return CMD_RET_FAILURE; + + if (argc > 3) { + boot_id = (int)simple_strtoul(argv[3], &endp, 0); + if ((argv[3] + strlen(argv[3]) != endp) || + boot_id > 0xffff) + return CMD_RET_USAGE; + }
- return do_bootefi_bootmgr_exec(); + return do_bootefi_bootmgr_exec(boot_id); } else { saddr = argv[1];
@@ -589,7 +601,7 @@ static char bootefi_help_text[] = " Use environment variable efi_selftest to select a single test.\n" " Use 'setenv efi_selftest list' to enumerate all tests.\n" #endif - "bootefi bootmgr [fdt addr]\n" + "bootefi bootmgr [<fdt addr>|'-' [<boot id>]]\n" " - load and boot EFI payload based on BootOrder/BootXXXX variables.\n" "\n" " If specified, the device tree located at <fdt address> gets\n" @@ -597,7 +609,7 @@ static char bootefi_help_text[] = #endif
U_BOOT_CMD( - bootefi, 3, 0, do_bootefi, + bootefi, 5, 0, do_bootefi, "Boots an EFI payload from memory", bootefi_help_text );

On 18.12.18 06:02, AKASHI Takahiro wrote:
With this patch applied, we will be able to selectively execute an EFI application by specifying a load option, say "1" for Boot0001, "2" for Boot0002 and so on.
=> bootefi bootmgr <fdt addr> 1, or bootefi bootmgr - 1
Please note that BootXXXX need not be included in "BootOrder".
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/bootefi.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 796ca6ee69ec..2fc52e3056d2 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -471,13 +471,13 @@ static efi_status_t bootefi_test_prepare
#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
-static int do_bootefi_bootmgr_exec(void) +static int do_bootefi_bootmgr_exec(int boot_id) { struct efi_device_path *device_path, *file_path; void *addr; efi_status_t r;
- addr = efi_bootmgr_load(-1, &device_path, &file_path);
- addr = efi_bootmgr_load(boot_id, &device_path, &file_path); if (!addr) return 1;
@@ -545,10 +545,22 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) } else #endif if (!strcmp(argv[1], "bootmgr")) {
if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
return CMD_RET_FAILURE;
char *endp;
int boot_id = -1;
if (argc > 2)
if (efi_handle_fdt((argv[2][0] == '-') ?
NULL : argv[2]))
return CMD_RET_FAILURE;
This is slowly getting quite unreadable. How about you make the code less dense, but easier to grasp?
if (argc > 2) { const char *fdtstr = argv[2];
/* Special address "-" means no device tree */ if (fdtstr[0] == '-') fdtstr = NULL;
r = efi_handle_fdt(fdtstr);
if (r) return r; }
Alex
if (argc > 3) {
boot_id = (int)simple_strtoul(argv[3], &endp, 0);
if ((argv[3] + strlen(argv[3]) != endp) ||
boot_id > 0xffff)
return CMD_RET_USAGE;
}
return do_bootefi_bootmgr_exec();
} else { saddr = argv[1];return do_bootefi_bootmgr_exec(boot_id);
@@ -589,7 +601,7 @@ static char bootefi_help_text[] = " Use environment variable efi_selftest to select a single test.\n" " Use 'setenv efi_selftest list' to enumerate all tests.\n" #endif
- "bootefi bootmgr [fdt addr]\n"
- "bootefi bootmgr [<fdt addr>|'-' [<boot id>]]\n" " - load and boot EFI payload based on BootOrder/BootXXXX variables.\n" "\n" " If specified, the device tree located at <fdt address> gets\n"
@@ -597,7 +609,7 @@ static char bootefi_help_text[] = #endif
U_BOOT_CMD(
- bootefi, 3, 0, do_bootefi,
- bootefi, 5, 0, do_bootefi, "Boots an EFI payload from memory", bootefi_help_text
);

On Sun, Dec 23, 2018 at 03:15:16AM +0100, Alexander Graf wrote:
On 18.12.18 06:02, AKASHI Takahiro wrote:
With this patch applied, we will be able to selectively execute an EFI application by specifying a load option, say "1" for Boot0001, "2" for Boot0002 and so on.
=> bootefi bootmgr <fdt addr> 1, or bootefi bootmgr - 1
Please note that BootXXXX need not be included in "BootOrder".
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/bootefi.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 796ca6ee69ec..2fc52e3056d2 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -471,13 +471,13 @@ static efi_status_t bootefi_test_prepare
#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
-static int do_bootefi_bootmgr_exec(void) +static int do_bootefi_bootmgr_exec(int boot_id) { struct efi_device_path *device_path, *file_path; void *addr; efi_status_t r;
- addr = efi_bootmgr_load(-1, &device_path, &file_path);
- addr = efi_bootmgr_load(boot_id, &device_path, &file_path); if (!addr) return 1;
@@ -545,10 +545,22 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) } else #endif if (!strcmp(argv[1], "bootmgr")) {
if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
return CMD_RET_FAILURE;
char *endp;
int boot_id = -1;
if (argc > 2)
if (efi_handle_fdt((argv[2][0] == '-') ?
NULL : argv[2]))
return CMD_RET_FAILURE;
This is slowly getting quite unreadable. How about you make the code less dense, but easier to grasp?
if (argc > 2) { const char *fdtstr = argv[2];
/* Special address "-" means no device tree */ if (fdtstr[0] == '-') fdtstr = NULL; r = efi_handle_fdt(fdtstr); if (r) return r;
}
OK
-Takahiro Akashi
Alex
if (argc > 3) {
boot_id = (int)simple_strtoul(argv[3], &endp, 0);
if ((argv[3] + strlen(argv[3]) != endp) ||
boot_id > 0xffff)
return CMD_RET_USAGE;
}
return do_bootefi_bootmgr_exec();
} else { saddr = argv[1];return do_bootefi_bootmgr_exec(boot_id);
@@ -589,7 +601,7 @@ static char bootefi_help_text[] = " Use environment variable efi_selftest to select a single test.\n" " Use 'setenv efi_selftest list' to enumerate all tests.\n" #endif
- "bootefi bootmgr [fdt addr]\n"
- "bootefi bootmgr [<fdt addr>|'-' [<boot id>]]\n" " - load and boot EFI payload based on BootOrder/BootXXXX variables.\n" "\n" " If specified, the device tree located at <fdt address> gets\n"
@@ -597,7 +609,7 @@ static char bootefi_help_text[] = #endif
U_BOOT_CMD(
- bootefi, 3, 0, do_bootefi,
- bootefi, 5, 0, do_bootefi, "Boots an EFI payload from memory", bootefi_help_text
);

"run -e" allows for executing EFI application with a specific "BootXXXX" variable. If no "BootXXXX" is specified or "BootOrder" is specified, it tries to run an EFI application specified in the order of "bootOrder."
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- cmd/bootefi.c | 2 +- cmd/nvedit.c | 5 +++++ common/cli.c | 31 +++++++++++++++++++++++++++++++ include/command.h | 3 +++ 4 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 2fc52e3056d2..8122793d11c5 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -471,7 +471,7 @@ static efi_status_t bootefi_test_prepare
#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
-static int do_bootefi_bootmgr_exec(int boot_id) +int do_bootefi_bootmgr_exec(int boot_id) { struct efi_device_path *device_path, *file_path; void *addr; diff --git a/cmd/nvedit.c b/cmd/nvedit.c index de16c72c23f2..c0facabfc4fe 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -1343,8 +1343,13 @@ U_BOOT_CMD( U_BOOT_CMD_COMPLETE( run, CONFIG_SYS_MAXARGS, 1, do_run, "run commands in an environment variable", +#if defined(CONFIG_CMD_BOOTEFI) + "var -e [BootXXXX]\n" + " - load and run UEFI app based on 'BootXXXX' UEFI variable", +#else "var [...]\n" " - run the commands in the environment variable(s) 'var'", +#endif var_complete ); #endif diff --git a/common/cli.c b/common/cli.c index 51b8d5f85cbb..013dd2e51936 100644 --- a/common/cli.c +++ b/common/cli.c @@ -12,8 +12,10 @@ #include <cli.h> #include <cli_hush.h> #include <console.h> +#include <efi_loader.h> #include <fdtdec.h> #include <malloc.h> +#include "../cmd/bootefi.h"
DECLARE_GLOBAL_DATA_PTR;
@@ -125,6 +127,35 @@ int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (argc < 2) return CMD_RET_USAGE;
+#ifdef CONFIG_CMD_BOOTEFI + if (!strcmp(argv[1], "-e")) { + int boot_id = -1; + char *endp; + + if (argc == 3) { + if (!strcmp(argv[2], "BootOrder")) { + boot_id = -1; + } else if (!strncmp(argv[2], "Boot", 4)) { + boot_id = (int)simple_strtoul(&argv[2][4], + &endp, 0); + if ((argv[2] + strlen(argv[2]) != endp) || + boot_id > 0xffff) + return CMD_RET_USAGE; + } else { + return CMD_RET_USAGE; + } + } + + if (efi_init_obj_list()) + return CMD_RET_FAILURE; + + if (efi_handle_fdt(NULL)) + return CMD_RET_FAILURE; + + return do_bootefi_bootmgr_exec(boot_id); + } +#endif + for (i = 1; i < argc; ++i) { char *arg;
diff --git a/include/command.h b/include/command.h index 200c7a5e9f4e..9b7b876585d9 100644 --- a/include/command.h +++ b/include/command.h @@ -48,6 +48,9 @@ typedef struct cmd_tbl_s cmd_tbl_t; #if defined(CONFIG_CMD_RUN) extern int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); #endif +#if defined(CONFIG_CMD_BOOTEFI) +int do_bootefi_bootmgr_exec(int boot_id); +#endif
/* common/command.c */ int _do_help (cmd_tbl_t *cmd_start, int cmd_items, cmd_tbl_t * cmdtp, int

On 18.12.18 06:02, AKASHI Takahiro wrote:
"run -e" allows for executing EFI application with a specific "BootXXXX" variable. If no "BootXXXX" is specified or "BootOrder" is specified, it tries to run an EFI application specified in the order of "bootOrder."
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/bootefi.c | 2 +- cmd/nvedit.c | 5 +++++ common/cli.c | 31 +++++++++++++++++++++++++++++++ include/command.h | 3 +++ 4 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 2fc52e3056d2..8122793d11c5 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -471,7 +471,7 @@ static efi_status_t bootefi_test_prepare
#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
-static int do_bootefi_bootmgr_exec(int boot_id) +int do_bootefi_bootmgr_exec(int boot_id) { struct efi_device_path *device_path, *file_path; void *addr; diff --git a/cmd/nvedit.c b/cmd/nvedit.c index de16c72c23f2..c0facabfc4fe 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -1343,8 +1343,13 @@ U_BOOT_CMD( U_BOOT_CMD_COMPLETE( run, CONFIG_SYS_MAXARGS, 1, do_run, "run commands in an environment variable", +#if defined(CONFIG_CMD_BOOTEFI)
- "var -e [BootXXXX]\n"
- " - load and run UEFI app based on 'BootXXXX' UEFI variable",
+#else "var [...]\n" " - run the commands in the environment variable(s) 'var'", +#endif var_complete ); #endif diff --git a/common/cli.c b/common/cli.c index 51b8d5f85cbb..013dd2e51936 100644 --- a/common/cli.c +++ b/common/cli.c @@ -12,8 +12,10 @@ #include <cli.h> #include <cli_hush.h> #include <console.h> +#include <efi_loader.h> #include <fdtdec.h> #include <malloc.h> +#include "../cmd/bootefi.h"
DECLARE_GLOBAL_DATA_PTR;
@@ -125,6 +127,35 @@ int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (argc < 2) return CMD_RET_USAGE;
+#ifdef CONFIG_CMD_BOOTEFI
- if (!strcmp(argv[1], "-e")) {
int boot_id = -1;
char *endp;
if (argc == 3) {
if (!strcmp(argv[2], "BootOrder")) {
boot_id = -1;
} else if (!strncmp(argv[2], "Boot", 4)) {
boot_id = (int)simple_strtoul(&argv[2][4],
&endp, 0);
if ((argv[2] + strlen(argv[2]) != endp) ||
boot_id > 0xffff)
return CMD_RET_USAGE;
This duplicates the same logic you added to bootefi.c. Better reuse it. I guess you can just call a function inside bootefi.c from here if you detect -e:
if (!strcmp(argv[1], "-e")) return do_bootefi_run(cmdtp, flag, argc, argv);
and just handle it all inside bootefi.c at that point.
} else {
return CMD_RET_USAGE;
}
}
if (efi_init_obj_list())
return CMD_RET_FAILURE;
if (efi_handle_fdt(NULL))
return CMD_RET_FAILURE;
return do_bootefi_bootmgr_exec(boot_id);
- }
+#endif
- for (i = 1; i < argc; ++i) { char *arg;
diff --git a/include/command.h b/include/command.h index 200c7a5e9f4e..9b7b876585d9 100644 --- a/include/command.h +++ b/include/command.h @@ -48,6 +48,9 @@ typedef struct cmd_tbl_s cmd_tbl_t; #if defined(CONFIG_CMD_RUN) extern int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); #endif +#if defined(CONFIG_CMD_BOOTEFI) +int do_bootefi_bootmgr_exec(int boot_id); +#endif
I would prefer if nvedit.c includes efi_loader.h and we define it there?
Alex

On Sun, Dec 23, 2018 at 03:19:17AM +0100, Alexander Graf wrote:
On 18.12.18 06:02, AKASHI Takahiro wrote:
"run -e" allows for executing EFI application with a specific "BootXXXX" variable. If no "BootXXXX" is specified or "BootOrder" is specified, it tries to run an EFI application specified in the order of "bootOrder."
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/bootefi.c | 2 +- cmd/nvedit.c | 5 +++++ common/cli.c | 31 +++++++++++++++++++++++++++++++ include/command.h | 3 +++ 4 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 2fc52e3056d2..8122793d11c5 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -471,7 +471,7 @@ static efi_status_t bootefi_test_prepare
#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
-static int do_bootefi_bootmgr_exec(int boot_id) +int do_bootefi_bootmgr_exec(int boot_id) { struct efi_device_path *device_path, *file_path; void *addr; diff --git a/cmd/nvedit.c b/cmd/nvedit.c index de16c72c23f2..c0facabfc4fe 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -1343,8 +1343,13 @@ U_BOOT_CMD( U_BOOT_CMD_COMPLETE( run, CONFIG_SYS_MAXARGS, 1, do_run, "run commands in an environment variable", +#if defined(CONFIG_CMD_BOOTEFI)
- "var -e [BootXXXX]\n"
- " - load and run UEFI app based on 'BootXXXX' UEFI variable",
+#else "var [...]\n" " - run the commands in the environment variable(s) 'var'", +#endif var_complete ); #endif diff --git a/common/cli.c b/common/cli.c index 51b8d5f85cbb..013dd2e51936 100644 --- a/common/cli.c +++ b/common/cli.c @@ -12,8 +12,10 @@ #include <cli.h> #include <cli_hush.h> #include <console.h> +#include <efi_loader.h> #include <fdtdec.h> #include <malloc.h> +#include "../cmd/bootefi.h"
DECLARE_GLOBAL_DATA_PTR;
@@ -125,6 +127,35 @@ int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (argc < 2) return CMD_RET_USAGE;
+#ifdef CONFIG_CMD_BOOTEFI
- if (!strcmp(argv[1], "-e")) {
int boot_id = -1;
char *endp;
if (argc == 3) {
if (!strcmp(argv[2], "BootOrder")) {
boot_id = -1;
} else if (!strncmp(argv[2], "Boot", 4)) {
boot_id = (int)simple_strtoul(&argv[2][4],
&endp, 0);
if ((argv[2] + strlen(argv[2]) != endp) ||
boot_id > 0xffff)
return CMD_RET_USAGE;
This duplicates the same logic you added to bootefi.c. Better reuse it. I guess you can just call a function inside bootefi.c from here if you detect -e:
if (!strcmp(argv[1], "-e")) return do_bootefi_run(cmdtp, flag, argc, argv);
and just handle it all inside bootefi.c at that point.
Hmm, OK. In this case, the command syntax of bootmgr will be changed so as to accept "BootXXXX" instead of just an integer (as boot id).
Thanks, -Takahiro Akashi
} else {
return CMD_RET_USAGE;
}
}
if (efi_init_obj_list())
return CMD_RET_FAILURE;
if (efi_handle_fdt(NULL))
return CMD_RET_FAILURE;
return do_bootefi_bootmgr_exec(boot_id);
- }
+#endif
- for (i = 1; i < argc; ++i) { char *arg;
diff --git a/include/command.h b/include/command.h index 200c7a5e9f4e..9b7b876585d9 100644 --- a/include/command.h +++ b/include/command.h @@ -48,6 +48,9 @@ typedef struct cmd_tbl_s cmd_tbl_t; #if defined(CONFIG_CMD_RUN) extern int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); #endif +#if defined(CONFIG_CMD_BOOTEFI) +int do_bootefi_bootmgr_exec(int boot_id); +#endif
I would prefer if nvedit.c includes efi_loader.h and we define it there?
Alex

On 25.12.18 12:29, AKASHI Takahiro wrote:
On Sun, Dec 23, 2018 at 03:19:17AM +0100, Alexander Graf wrote:
On 18.12.18 06:02, AKASHI Takahiro wrote:
"run -e" allows for executing EFI application with a specific "BootXXXX" variable. If no "BootXXXX" is specified or "BootOrder" is specified, it tries to run an EFI application specified in the order of "bootOrder."
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/bootefi.c | 2 +- cmd/nvedit.c | 5 +++++ common/cli.c | 31 +++++++++++++++++++++++++++++++ include/command.h | 3 +++ 4 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 2fc52e3056d2..8122793d11c5 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -471,7 +471,7 @@ static efi_status_t bootefi_test_prepare
#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
-static int do_bootefi_bootmgr_exec(int boot_id) +int do_bootefi_bootmgr_exec(int boot_id) { struct efi_device_path *device_path, *file_path; void *addr; diff --git a/cmd/nvedit.c b/cmd/nvedit.c index de16c72c23f2..c0facabfc4fe 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -1343,8 +1343,13 @@ U_BOOT_CMD( U_BOOT_CMD_COMPLETE( run, CONFIG_SYS_MAXARGS, 1, do_run, "run commands in an environment variable", +#if defined(CONFIG_CMD_BOOTEFI)
- "var -e [BootXXXX]\n"
- " - load and run UEFI app based on 'BootXXXX' UEFI variable",
+#else "var [...]\n" " - run the commands in the environment variable(s) 'var'", +#endif var_complete ); #endif diff --git a/common/cli.c b/common/cli.c index 51b8d5f85cbb..013dd2e51936 100644 --- a/common/cli.c +++ b/common/cli.c @@ -12,8 +12,10 @@ #include <cli.h> #include <cli_hush.h> #include <console.h> +#include <efi_loader.h> #include <fdtdec.h> #include <malloc.h> +#include "../cmd/bootefi.h"
DECLARE_GLOBAL_DATA_PTR;
@@ -125,6 +127,35 @@ int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (argc < 2) return CMD_RET_USAGE;
+#ifdef CONFIG_CMD_BOOTEFI
- if (!strcmp(argv[1], "-e")) {
int boot_id = -1;
char *endp;
if (argc == 3) {
if (!strcmp(argv[2], "BootOrder")) {
boot_id = -1;
} else if (!strncmp(argv[2], "Boot", 4)) {
boot_id = (int)simple_strtoul(&argv[2][4],
&endp, 0);
if ((argv[2] + strlen(argv[2]) != endp) ||
boot_id > 0xffff)
return CMD_RET_USAGE;
This duplicates the same logic you added to bootefi.c. Better reuse it. I guess you can just call a function inside bootefi.c from here if you detect -e:
if (!strcmp(argv[1], "-e")) return do_bootefi_run(cmdtp, flag, argc, argv);
and just handle it all inside bootefi.c at that point.
Hmm, OK. In this case, the command syntax of bootmgr will be changed so as to accept "BootXXXX" instead of just an integer (as boot id).
Not necessarily. You can just move all the code you have here into a new function in bootefi.c. The parsing logic really should just be shared.
Alex

On Wed, Dec 26, 2018 at 10:24:45PM +0100, Alexander Graf wrote:
On 25.12.18 12:29, AKASHI Takahiro wrote:
On Sun, Dec 23, 2018 at 03:19:17AM +0100, Alexander Graf wrote:
On 18.12.18 06:02, AKASHI Takahiro wrote:
"run -e" allows for executing EFI application with a specific "BootXXXX" variable. If no "BootXXXX" is specified or "BootOrder" is specified, it tries to run an EFI application specified in the order of "bootOrder."
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/bootefi.c | 2 +- cmd/nvedit.c | 5 +++++ common/cli.c | 31 +++++++++++++++++++++++++++++++ include/command.h | 3 +++ 4 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 2fc52e3056d2..8122793d11c5 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -471,7 +471,7 @@ static efi_status_t bootefi_test_prepare
#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
-static int do_bootefi_bootmgr_exec(int boot_id) +int do_bootefi_bootmgr_exec(int boot_id) { struct efi_device_path *device_path, *file_path; void *addr; diff --git a/cmd/nvedit.c b/cmd/nvedit.c index de16c72c23f2..c0facabfc4fe 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -1343,8 +1343,13 @@ U_BOOT_CMD( U_BOOT_CMD_COMPLETE( run, CONFIG_SYS_MAXARGS, 1, do_run, "run commands in an environment variable", +#if defined(CONFIG_CMD_BOOTEFI)
- "var -e [BootXXXX]\n"
- " - load and run UEFI app based on 'BootXXXX' UEFI variable",
+#else "var [...]\n" " - run the commands in the environment variable(s) 'var'", +#endif var_complete ); #endif diff --git a/common/cli.c b/common/cli.c index 51b8d5f85cbb..013dd2e51936 100644 --- a/common/cli.c +++ b/common/cli.c @@ -12,8 +12,10 @@ #include <cli.h> #include <cli_hush.h> #include <console.h> +#include <efi_loader.h> #include <fdtdec.h> #include <malloc.h> +#include "../cmd/bootefi.h"
DECLARE_GLOBAL_DATA_PTR;
@@ -125,6 +127,35 @@ int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (argc < 2) return CMD_RET_USAGE;
+#ifdef CONFIG_CMD_BOOTEFI
- if (!strcmp(argv[1], "-e")) {
int boot_id = -1;
char *endp;
if (argc == 3) {
if (!strcmp(argv[2], "BootOrder")) {
boot_id = -1;
} else if (!strncmp(argv[2], "Boot", 4)) {
boot_id = (int)simple_strtoul(&argv[2][4],
&endp, 0);
if ((argv[2] + strlen(argv[2]) != endp) ||
boot_id > 0xffff)
return CMD_RET_USAGE;
This duplicates the same logic you added to bootefi.c. Better reuse it. I guess you can just call a function inside bootefi.c from here if you detect -e:
if (!strcmp(argv[1], "-e")) return do_bootefi_run(cmdtp, flag, argc, argv);
and just handle it all inside bootefi.c at that point.
Hmm, OK. In this case, the command syntax of bootmgr will be changed so as to accept "BootXXXX" instead of just an integer (as boot id).
Not necessarily. You can just move all the code you have here into a new function in bootefi.c. The parsing logic really should just be shared.
Ah ha, OK.
-Takahiro Akashi
Alex
participants (3)
-
AKASHI Takahiro
-
Alexander Graf
-
Heinrich Schuchardt