[PATCH] efi_loader: support EFI_OS_INDICATIONS_BOOT_TO_FW_UI

[PATCH] efi_loader: support EFI_OS_INDICATIONS_BOOT_TO_FW_UI

[Gary Guo]

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>
---
 common/autoboot.c            |  4 +++-
 include/efi_loader.h         |  2 ++
 lib/efi_loader/efi_bootmgr.c | 45 ++++++++++++++++++++++++++++++++++++
 lib/efi_loader/efi_setup.c   |  4 ++++
 4 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/common/autoboot.c b/common/autoboot.c
index 1783ef92c94c..b889407612dc 100644
--- a/common/autoboot.c
+++ b/common/autoboot.c
@@ -11,6 +11,7 @@
 #include <cli.h>
 #include <command.h>
 #include <console.h>
+#include <efi_loader.h>
 #include <env.h>
 #include <errno.h>
 #include <fdtdec.h>
@@ -503,7 +504,8 @@ void autoboot_command(const char *s)
 {
 	debug("### main_loop: bootcmd=\"%s\"\n", s ? s : "<UNDEFINED>");
 
-	if (s && (stored_bootdelay == -2 ||
+	if (s && !(IS_ENABLED(CONFIG_EFI_BOOTMGR) && efi_should_abort_autoboot()) &&
+	    (stored_bootdelay == -2 ||
 		 (stored_bootdelay != -1 && !abortboot(stored_bootdelay)))) {
 		bool lock;
 		int prev;
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 3e70ac070550..449697ad13b3 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -583,6 +583,8 @@ efi_status_t efi_bootmgr_get_unused_bootoption(u16 *buf,
 efi_status_t efi_bootmgr_update_media_device_boot_option(void);
 /* Delete selected boot option */
 efi_status_t efi_bootmgr_delete_boot_option(u16 boot_index);
+/* Check if autoboot should be aborted based on EFI vars */
+bool efi_should_abort_autoboot(void);
 /* Invoke EFI boot manager */
 efi_status_t efi_bootmgr_run(void *fdt);
 /* search the boot option index in BootOrder */
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index a687f4d8e85c..39f255ac9de4 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -1290,6 +1290,51 @@ out:
 	return ret;
 }
 
+/**
+ * efi_should_abort_autoboot() - check if autoboot should be aborted per EFI vars
+ *
+ * Return:	true if autoboot should abort
+ */
+bool efi_should_abort_autoboot(void)
+{
+	efi_status_t ret;
+	u64 os_indications = 0x0;
+	efi_uintn_t size;
+	efi_status_t r;
+
+	/* Initialize EFI drivers */
+	ret = efi_init_obj_list();
+	if (ret != EFI_SUCCESS) {
+		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
+			ret & ~EFI_ERROR_MASK);
+		return false;
+	}
+
+	size = sizeof(os_indications);
+	r = efi_get_variable_int(u"OsIndications", &efi_global_variable_guid,
+				 NULL, &size, &os_indications, NULL);
+	if (r != EFI_SUCCESS || size != sizeof(os_indications))
+		return false;
+
+	if (!(os_indications & EFI_OS_INDICATIONS_BOOT_TO_FW_UI))
+		return false;
+
+	/* Clear the BOOT_TO_FW_UI so next boot returns to normal */
+	os_indications &= ~EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
+	r = efi_set_variable_int(u"OsIndications",
+				 &efi_global_variable_guid,
+				 EFI_VARIABLE_NON_VOLATILE |
+				 EFI_VARIABLE_BOOTSERVICE_ACCESS |
+				 EFI_VARIABLE_RUNTIME_ACCESS,
+				 sizeof(os_indications),
+				 &os_indications, false);
+
+	if (r != EFI_SUCCESS)
+		log_err("Setting %ls failed\n", L"OsIndications");
+
+	return true;
+}
+
 /**
  * efi_bootmgr_run() - execute EFI boot manager
  * @fdt:	Flat device tree
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index f06cf49e4435..0ad02e9fe0dd 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -156,6 +156,10 @@ static efi_status_t efi_init_os_indications(void)
 {
 	u64 os_indications_supported = 0;
 
+	if (IS_ENABLED(CONFIG_EFI_BOOTMGR))
+		os_indications_supported |=
+			EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
+
 	if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
 		os_indications_supported |=
 			EFI_OS_INDICATIONS_CAPSULE_RESULT_VAR_SUPPORTED;
-- 
2.51.2

Re: [PATCH] efi_loader: support EFI_OS_INDICATIONS_BOOT_TO_FW_UI

[Heinrich Schuchardt]

Am 31. März 2026 21:18:51 MESZ schrieb Gary Guo <gary@garyguo.net>:
>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>

Thank you Gary for the patch. I am currently on travel and will have to look at it once I have returned.

@Ilias
Should we preferably integrate the detection of this bit into efi_init_early()?

Best regards

Heinrich



>---
> common/autoboot.c            |  4 +++-
> include/efi_loader.h         |  2 ++
> lib/efi_loader/efi_bootmgr.c | 45 ++++++++++++++++++++++++++++++++++++
> lib/efi_loader/efi_setup.c   |  4 ++++
> 4 files changed, 54 insertions(+), 1 deletion(-)
>
>diff --git a/common/autoboot.c b/common/autoboot.c
>index 1783ef92c94c..b889407612dc 100644
>--- a/common/autoboot.c
>+++ b/common/autoboot.c
>@@ -11,6 +11,7 @@
> #include <cli.h>
> #include <command.h>
> #include <console.h>
>+#include <efi_loader.h>
> #include <env.h>
> #include <errno.h>
> #include <fdtdec.h>
>@@ -503,7 +504,8 @@ void autoboot_command(const char *s)
> {
> 	debug("### main_loop: bootcmd=\"%s\"\n", s ? s : "<UNDEFINED>");
> 
>-	if (s && (stored_bootdelay == -2 ||
>+	if (s && !(IS_ENABLED(CONFIG_EFI_BOOTMGR) && efi_should_abort_autoboot()) &&
>+	    (stored_bootdelay == -2 ||
> 		 (stored_bootdelay != -1 && !abortboot(stored_bootdelay)))) {
> 		bool lock;
> 		int prev;
>diff --git a/include/efi_loader.h b/include/efi_loader.h
>index 3e70ac070550..449697ad13b3 100644
>--- a/include/efi_loader.h
>+++ b/include/efi_loader.h
>@@ -583,6 +583,8 @@ efi_status_t efi_bootmgr_get_unused_bootoption(u16 *buf,
> efi_status_t efi_bootmgr_update_media_device_boot_option(void);
> /* Delete selected boot option */
> efi_status_t efi_bootmgr_delete_boot_option(u16 boot_index);
>+/* Check if autoboot should be aborted based on EFI vars */
>+bool efi_should_abort_autoboot(void);
> /* Invoke EFI boot manager */
> efi_status_t efi_bootmgr_run(void *fdt);
> /* search the boot option index in BootOrder */
>diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
>index a687f4d8e85c..39f255ac9de4 100644
>--- a/lib/efi_loader/efi_bootmgr.c
>+++ b/lib/efi_loader/efi_bootmgr.c
>@@ -1290,6 +1290,51 @@ out:
> 	return ret;
> }
> 
>+/**
>+ * efi_should_abort_autoboot() - check if autoboot should be aborted per EFI vars
>+ *
>+ * Return:	true if autoboot should abort
>+ */
>+bool efi_should_abort_autoboot(void)
>+{
>+	efi_status_t ret;
>+	u64 os_indications = 0x0;
>+	efi_uintn_t size;
>+	efi_status_t r;
>+
>+	/* Initialize EFI drivers */
>+	ret = efi_init_obj_list();
>+	if (ret != EFI_SUCCESS) {
>+		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
>+			ret & ~EFI_ERROR_MASK);
>+		return false;
>+	}
>+
>+	size = sizeof(os_indications);
>+	r = efi_get_variable_int(u"OsIndications", &efi_global_variable_guid,
>+				 NULL, &size, &os_indications, NULL);
>+	if (r != EFI_SUCCESS || size != sizeof(os_indications))
>+		return false;
>+
>+	if (!(os_indications & EFI_OS_INDICATIONS_BOOT_TO_FW_UI))
>+		return false;
>+
>+	/* Clear the BOOT_TO_FW_UI so next boot returns to normal */
>+	os_indications &= ~EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
>+	r = efi_set_variable_int(u"OsIndications",
>+				 &efi_global_variable_guid,
>+				 EFI_VARIABLE_NON_VOLATILE |
>+				 EFI_VARIABLE_BOOTSERVICE_ACCESS |
>+				 EFI_VARIABLE_RUNTIME_ACCESS,
>+				 sizeof(os_indications),
>+				 &os_indications, false);
>+
>+	if (r != EFI_SUCCESS)
>+		log_err("Setting %ls failed\n", L"OsIndications");
>+
>+	return true;
>+}
>+
> /**
>  * efi_bootmgr_run() - execute EFI boot manager
>  * @fdt:	Flat device tree
>diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
>index f06cf49e4435..0ad02e9fe0dd 100644
>--- a/lib/efi_loader/efi_setup.c
>+++ b/lib/efi_loader/efi_setup.c
>@@ -156,6 +156,10 @@ static efi_status_t efi_init_os_indications(void)
> {
> 	u64 os_indications_supported = 0;
> 
>+	if (IS_ENABLED(CONFIG_EFI_BOOTMGR))
>+		os_indications_supported |=
>+			EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
>+
> 	if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
> 		os_indications_supported |=
> 			EFI_OS_INDICATIONS_CAPSULE_RESULT_VAR_SUPPORTED;

Re: [PATCH] efi_loader: support EFI_OS_INDICATIONS_BOOT_TO_FW_UI

[Ilias Apalodimas]

Hi Heinrich,

On Fri, 3 Apr 2026 at 13:30, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Am 31. März 2026 21:18:51 MESZ schrieb Gary Guo <gary@garyguo.net>:
> >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>
>
> Thank you Gary for the patch. I am currently on travel and will have to look at it once I have returned.

On vacation here as well. I'll be off next week as well, nut I might
have time to take a closer look

>
> @Ilias
> Should we preferably integrate the detection of this bit into efi_init_early()?

Which part? This patch also touched EFI variables, which are not
initialized in the eraly code.

Thanks
/Ilias
>
> Best regards
>
> Heinrich
>
>
>
> >---
> > common/autoboot.c            |  4 +++-
> > include/efi_loader.h         |  2 ++
> > lib/efi_loader/efi_bootmgr.c | 45 ++++++++++++++++++++++++++++++++++++
> > lib/efi_loader/efi_setup.c   |  4 ++++
> > 4 files changed, 54 insertions(+), 1 deletion(-)
> >
> >diff --git a/common/autoboot.c b/common/autoboot.c
> >index 1783ef92c94c..b889407612dc 100644
> >--- a/common/autoboot.c
> >+++ b/common/autoboot.c
> >@@ -11,6 +11,7 @@
> > #include <cli.h>
> > #include <command.h>
> > #include <console.h>
> >+#include <efi_loader.h>
> > #include <env.h>
> > #include <errno.h>
> > #include <fdtdec.h>
> >@@ -503,7 +504,8 @@ void autoboot_command(const char *s)
> > {
> >       debug("### main_loop: bootcmd=\"%s\"\n", s ? s : "<UNDEFINED>");
> >
> >-      if (s && (stored_bootdelay == -2 ||
> >+      if (s && !(IS_ENABLED(CONFIG_EFI_BOOTMGR) && efi_should_abort_autoboot()) &&
> >+          (stored_bootdelay == -2 ||
> >                (stored_bootdelay != -1 && !abortboot(stored_bootdelay)))) {
> >               bool lock;
> >               int prev;
> >diff --git a/include/efi_loader.h b/include/efi_loader.h
> >index 3e70ac070550..449697ad13b3 100644
> >--- a/include/efi_loader.h
> >+++ b/include/efi_loader.h
> >@@ -583,6 +583,8 @@ efi_status_t efi_bootmgr_get_unused_bootoption(u16 *buf,
> > efi_status_t efi_bootmgr_update_media_device_boot_option(void);
> > /* Delete selected boot option */
> > efi_status_t efi_bootmgr_delete_boot_option(u16 boot_index);
> >+/* Check if autoboot should be aborted based on EFI vars */
> >+bool efi_should_abort_autoboot(void);
> > /* Invoke EFI boot manager */
> > efi_status_t efi_bootmgr_run(void *fdt);
> > /* search the boot option index in BootOrder */
> >diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> >index a687f4d8e85c..39f255ac9de4 100644
> >--- a/lib/efi_loader/efi_bootmgr.c
> >+++ b/lib/efi_loader/efi_bootmgr.c
> >@@ -1290,6 +1290,51 @@ out:
> >       return ret;
> > }
> >
> >+/**
> >+ * efi_should_abort_autoboot() - check if autoboot should be aborted per EFI vars
> >+ *
> >+ * Return:    true if autoboot should abort
> >+ */
> >+bool efi_should_abort_autoboot(void)
> >+{
> >+      efi_status_t ret;
> >+      u64 os_indications = 0x0;
> >+      efi_uintn_t size;
> >+      efi_status_t r;
> >+
> >+      /* Initialize EFI drivers */
> >+      ret = efi_init_obj_list();
> >+      if (ret != EFI_SUCCESS) {
> >+              log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> >+                      ret & ~EFI_ERROR_MASK);
> >+              return false;
> >+      }
> >+
> >+      size = sizeof(os_indications);
> >+      r = efi_get_variable_int(u"OsIndications", &efi_global_variable_guid,
> >+                               NULL, &size, &os_indications, NULL);
> >+      if (r != EFI_SUCCESS || size != sizeof(os_indications))
> >+              return false;
> >+
> >+      if (!(os_indications & EFI_OS_INDICATIONS_BOOT_TO_FW_UI))
> >+              return false;
> >+
> >+      /* Clear the BOOT_TO_FW_UI so next boot returns to normal */
> >+      os_indications &= ~EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
> >+      r = efi_set_variable_int(u"OsIndications",
> >+                               &efi_global_variable_guid,
> >+                               EFI_VARIABLE_NON_VOLATILE |
> >+                               EFI_VARIABLE_BOOTSERVICE_ACCESS |
> >+                               EFI_VARIABLE_RUNTIME_ACCESS,
> >+                               sizeof(os_indications),
> >+                               &os_indications, false);
> >+
> >+      if (r != EFI_SUCCESS)
> >+              log_err("Setting %ls failed\n", L"OsIndications");
> >+
> >+      return true;
> >+}
> >+
> > /**
> >  * efi_bootmgr_run() - execute EFI boot manager
> >  * @fdt:      Flat device tree
> >diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> >index f06cf49e4435..0ad02e9fe0dd 100644
> >--- a/lib/efi_loader/efi_setup.c
> >+++ b/lib/efi_loader/efi_setup.c
> >@@ -156,6 +156,10 @@ static efi_status_t efi_init_os_indications(void)
> > {
> >       u64 os_indications_supported = 0;
> >
> >+      if (IS_ENABLED(CONFIG_EFI_BOOTMGR))
> >+              os_indications_supported |=
> >+                      EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
> >+
> >       if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
> >               os_indications_supported |=
> >                       EFI_OS_INDICATIONS_CAPSULE_RESULT_VAR_SUPPORTED;
>

Re: [PATCH] efi_loader: support EFI_OS_INDICATIONS_BOOT_TO_FW_UI

[Simon Glass]

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.

> +     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.

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

Regards,
Simon

Re: [PATCH] efi_loader: support EFI_OS_INDICATIONS_BOOT_TO_FW_UI

[Gary Guo]

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

Re: [PATCH] efi_loader: support EFI_OS_INDICATIONS_BOOT_TO_FW_UI

[Simon Glass]

Hi Gary,

On Fri, 3 Apr 2026 at 09:41, Gary Guo <gary@garyguo.net> wrote:
>
> 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 mean you don't need r = in the message. You could say (err %lu) perhaps?

>
> >
> > 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?

There is reading and write. Being able to control that could end up in
a command at some point. It is a useful feature. Up to you though.

Regards,
Simon

Re: [PATCH] efi_loader: support EFI_OS_INDICATIONS_BOOT_TO_FW_UI

[Heinrich Schuchardt]

Am 3. April 2026 18:31:14 MESZ schrieb Simon Glass <sjg@chromium.org>:
>Hi Gary,
>
>On Fri, 3 Apr 2026 at 09:41, Gary Guo <gary@garyguo.net> wrote:
>>
>> 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 &

The return value does not help anybody to understand what went wrong as there are a lot of different failure modes.

We should write specific messages inside the routine and not a message in the caller.

It might make sense to have a generic translation of EFI return codes to a string message in vsprintf.c. Which end user would know what error 14 is?

Best regards

Heinrich

 ~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 mean you don't need r = in the message. You could say (err %lu) perhaps?
>
>>
>> >
>> > 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?
>
>There is reading and write. Being able to control that could end up in
>a command at some point. It is a useful feature. Up to you though.
>
>Regards,
>Simon