| Message ID | 20181218050257.20142-2-takahiro.akashi@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | efi_loader: run a specific efi application more easily | expand |
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; > > >
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;
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(-)