[U-Boot] [PATCH v2 03/14] efi_loader: bootmgr: allow for running a given load option

Alexander Graf
2018-12-03T13:54:57Z

On 03.12.18 04:20, AKASHI Takahiro wrote:
> On Mon, Dec 03, 2018 at 12:22:37AM +0100, Alexander Graf wrote:
>>
>>
>> On 05.11.18 10:06, 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>
>>
>> I don't see you changing the caller, so this hunk won't compile on its own?
>
> You're correct.
>
>> Please make sure that every single step in your patch set compiles.
>
> I will try to double-check in the future.
>
>>> ---
>>> include/efi_loader.h | 3 ++-
>>> lib/efi_loader/efi_bootmgr.c | 8 +++++++-
>>> 2 files changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>>> index d83de906fbce..ce0f420b5004 100644
>>> --- a/include/efi_loader.h
>>> +++ b/include/efi_loader.h
>>> @@ -540,7 +540,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 a095df3f540b..74eb832a8c92 100644
>>> --- a/lib/efi_loader/efi_bootmgr.c
>>> +++ b/lib/efi_loader/efi_bootmgr.c
>>> @@ -170,7 +170,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)
>>> {
>>> uint16_t *bootorder;
>>> @@ -183,6 +184,11 @@ void *efi_bootmgr_load(struct efi_device_path **device_path,
>>> bs = systab.boottime;
>>> rs = systab.runtime;
>>>
>>> + if (boot_id != -1) {
>>> + image = try_load_entry(boot_id, device_path, file_path);
>>> + goto error;
>>
>> I think at this point it might be better to split the function, no?
>
> It's one way to fix, but I want simply to fix the issue by
> modifying the caller, do_bootefi_bootmgr_exec() because
>
>> You're not actually reusing any code from efi_bootmgr_load with the 2
>> instantiation types (explicit boot id, loop through boot order).
>
> I will add "BootNext" support to efi_bootmgr_load() so that it will be
> a single point of loading. See my relevant patch:
> https://lists.denx.de/pipermail/u-boot/2018-November/349281.html

I don't fully understand. I would expect BootNext to only take effect
when *not* booting an explicit ID? So that would fit quite nicely into a
function based split.

Alex