Re: [PATCH] efi_loader: support EFI_OS_INDICATIONS_BOOT_TO_FW_UI

["Gary Guo" <gary@garyguo.net>]

On Fri Apr 3, 2026 at 2:22 PM BST, Simon Glass wrote:
> Hi Gary,
>
> On 2026-03-31T19:18:51, Gary Guo <gary@garyguo.net> wrote:
>> efi_loader: support EFI_OS_INDICATIONS_BOOT_TO_FW_UI
>>
>> EFI variable OsIndications have a bit EFI_OS_INDICATIONS_BOOT_TO_FW_UI
>> indicating that the OS (or next stage bootloader) requests booting into
>> firmware UI instead of continuing autoboot process.
>>
>> Support it by having autoboot.c asking EFI bootmgr if boot should continue,
>> which checks and clear the bit (so the next boot continue as normal).
>>
>> With this change, systemd-boot will show a "Reboot Into Firmware Interface"
>> option which will be able to drop back to u-boot. This can be useful if
>> bootdelay is configured to be -2.
>>
>> Signed-off-by: Gary Guo <gary@garyguo.net>
>
>> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
>> @@ -1290,6 +1290,51 @@ out:
>> +bool efi_should_abort_autoboot(void)
>> +{
>> +     efi_status_t ret;
>> +     u64 os_indications = 0x0;
>> +     efi_uintn_t size;
>> +     efi_status_t r;
>
> (I'm leaving the proper review to Ilias & Heinrich, this is just a few
> nits and a though)
>
> Please can you use a single variable name for both efi_status_t variables.

Ah, I think I copied the init part from somewhere else and forget to merge the
variable names.

I'll adjust the variable name in the next version after receiving more reviews.

>
>> +     ret = efi_init_obj_list();
>> +     if (ret != EFI_SUCCESS) {
>> +             log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
>> +                     ret & ~EFI_ERROR_MASK);
>
> The log message says "r = " but prints ret. The similar message in
> efi_bootmgr_run() just prints the value without a prefix.

Sorry, what do you mean by "without a prefix"? I think the code is identical to
the one in `efi_bootmgr_run`?

>
> I would also like to see the code which adjusts the variable placed in
> a function.

You mean the bit clearing part? What benefit does that provide, considering that
this is just a single function call?

Thanks,
Gary