From 6efdd7fec5106205240332bd3b7fd2f93d4d9d4c Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Wed, 4 Oct 2023 12:55:52 +0100 Subject: [PATCH] sd-boot: add way to disable the 100ms delay when timeout=0 Currently we have a 100ms delay which allows for people to enter/show the boot menu even when timeout is set to zero. In a handful of cases, that may not be needed - both in terms of access policy, as well as latency. For example: the option to provide the boot menu may be hidden behind an "expert only" UX in the OS, to avoid end users from accidentally entering it. In addition, the current 100ms input polling may cause unexpected additional delays in the boot. Some example numbers from my SteamDeck: - boot counting/rename/flush doubles 300us -> 600us - seed/hash setup doubles 900us -> 1800us - kernel/image load gets ~40% slower 107ms -> 167ms It's not entirely clear why the UEFI calls gets slower, nevertheless the information in itself proves useful. This commit introduces a new option "menu-disabled", which omits the 100ms delay. The option is documented throughout the manual pages as well as the Boot Loader Specification. v2: - use STR_IN_SET v3: - drop erroneous whitespace v4: - add a new LoaderFeature bit, - don't change ABI keep TIMEOUT_* tokens the same - move new token in the 64bit range, update API and storage for it - change inc/dec behaviour to TIMEOUT_MIN : TIMEOUT_MENU_FORCE - user cannot opt-in from sd-boot itself, add assert_not_reached() v5: - s/Menu disablement control/Menu can be disabled/ - rewrap comments to 109 - use SYNTHETIC_ERRNO(EOPNOTSUPP) Signed-off-by: Emil Velikov --- docs/BOOT_LOADER_INTERFACE.md | 6 ++- man/bootctl.xml | 9 +++-- man/loader.conf.xml | 5 ++- src/boot/bootctl-set-efivar.c | 22 ++++++++++- src/boot/bootctl-status.c | 1 + src/boot/efi/boot.c | 57 ++++++++++++++++++--------- src/fundamental/efivars-fundamental.h | 1 + 7 files changed, 73 insertions(+), 28 deletions(-) diff --git a/docs/BOOT_LOADER_INTERFACE.md b/docs/BOOT_LOADER_INTERFACE.md index 79d1224057f..f915cbacc99 100644 --- a/docs/BOOT_LOADER_INTERFACE.md +++ b/docs/BOOT_LOADER_INTERFACE.md @@ -35,8 +35,9 @@ variables. All EFI variables use the vendor UUID non-numeric string values are also accepted. A value of `menu-force` will disable the timeout and show the menu indefinitely. If set to `0` or `menu-hidden` the default entry is booted immediately without showing a menu. - The boot loader should provide a way to interrupt this by for example - listening for key presses for a brief moment before booting. + Unless a value of `menu-disabled` is set, the boot loader should provide a + way to interrupt this by for example listening for key presses for a brief + moment before booting. * Similarly, the EFI variable `LoaderConfigTimeoutOneShot` contains a boot menu timeout for a single following boot. It is set by the OS in order to request @@ -80,6 +81,7 @@ variables. All EFI variables use the vendor UUID * `1 << 4` → The boot loader supports boot counting as described in [Automatic Boot Assessment](AUTOMATIC_BOOT_ASSESSMENT.md). * `1 << 5` → The boot loader supports looking for boot menu entries in the Extended Boot Loader Partition. * `1 << 6` → The boot loader supports passing a random seed to the OS. + * `1 << 13` → The boot loader honours `menu-disabled` option when set. * The EFI variable `LoaderSystemToken` contains binary random data, persistently set by the OS installer. Boot loaders that support passing diff --git a/man/bootctl.xml b/man/bootctl.xml index 02ae5e1231a..b2091eedf07 100644 --- a/man/bootctl.xml +++ b/man/bootctl.xml @@ -177,10 +177,10 @@ systemd.time7 for details about the syntax of time spans. - If this is set to or no menu is shown and - the default entry will be booted immediately, while setting this to - disables the timeout while always showing the menu. When an empty string ("") is specified the - bootloader will revert to its default menu timeout. + If this is set to or or + , no menu is shown and the default entry will be booted immediately, while + setting this to disables the timeout while always showing the menu. + When an empty string ("") is specified the bootloader will revert to its default menu timeout. @@ -555,6 +555,7 @@ Current Boot Loader: ← details about sd-boot or anothe ✓ Support for passing random seed to OS ✓ Load drop-in drivers ✓ Boot loader sets ESP information + ✓ Menu can be disabled ESP: /dev/disk/by-partuuid/01234567-89ab-cdef-dead-beef00000000 File: └─/EFI/systemd/systemd-bootx64.efi diff --git a/man/loader.conf.xml b/man/loader.conf.xml index a94f09c1ad2..a7db82da1c8 100644 --- a/man/loader.conf.xml +++ b/man/loader.conf.xml @@ -138,8 +138,9 @@ will be stored as an EFI variable in that case, overriding this option. - If set to menu-hidden or 0 (the default) no menu - is shown and the default entry will be booted immediately. The menu can be shown + If set to menu-disabled or menu-hidden or 0 + (the default), no menu is shown and the default entry will be booted immediately. Unless + menu-disabled is used, the menu can be shown by pressing and holding a key before systemd-boot is launched. Setting this to menu-force disables the timeout while always showing the menu. diff --git a/src/boot/bootctl-set-efivar.c b/src/boot/bootctl-set-efivar.c index 71507c1e1da..2fe3c442211 100644 --- a/src/boot/bootctl-set-efivar.c +++ b/src/boot/bootctl-set-efivar.c @@ -6,6 +6,7 @@ #include "bootctl.h" #include "bootctl-set-efivar.h" #include "efivars.h" +#include "efi-loader.h" #include "stdio-util.h" #include "utf8.h" #include "virt.h" @@ -14,12 +15,15 @@ static int parse_timeout(const char *arg1, char16_t **ret_timeout, size_t *ret_t char utf8[DECIMAL_STR_MAX(usec_t)]; char16_t *encoded; usec_t timeout; + bool menu_disabled = false; int r; assert(arg1); assert(ret_timeout); assert(ret_timeout_size); + assert_cc(STRLEN("menu-disabled") < ELEMENTSOF(utf8)); + /* Note: Since there is no way to query if the booloader supports the string tokens, we explicitly * set their numerical value(s) instead. This means that some of the sd-boot internal ABI has leaked * although the ship has sailed and the side-effects are self-contained. @@ -28,7 +32,18 @@ static int parse_timeout(const char *arg1, char16_t **ret_timeout, size_t *ret_t timeout = USEC_INFINITY; else if (streq(arg1, "menu-hidden")) timeout = 0; - else { + else if (streq(arg1, "menu-disabled")) { + uint64_t loader_features = 0; + + (void) efi_loader_get_features(&loader_features); + if (!(loader_features & EFI_LOADER_FEATURE_MENU_DISABLE)) { + if (!arg_graceful) + return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "Loader does not support 'menu-disabled': %m"); + + log_warning("Loader does not support 'menu-disabled', setting anyway."); + } + menu_disabled = true; + } else { r = parse_time(arg1, &timeout, USEC_PER_SEC); if (r < 0) return log_error_errno(r, "Failed to parse timeout '%s': %m", arg1); @@ -36,7 +51,10 @@ static int parse_timeout(const char *arg1, char16_t **ret_timeout, size_t *ret_t log_warning("Timeout is too long and will be treated as 'menu-force' instead."); } - xsprintf(utf8, USEC_FMT, MIN(timeout / USEC_PER_SEC, UINT32_MAX)); + if (menu_disabled) + xsprintf(utf8, "menu-disabled"); + else + xsprintf(utf8, USEC_FMT, MIN(timeout / USEC_PER_SEC, UINT32_MAX)); encoded = utf8_to_utf16(utf8, SIZE_MAX); if (!encoded) diff --git a/src/boot/bootctl-status.c b/src/boot/bootctl-status.c index a1fe3364331..16b2eaed072 100644 --- a/src/boot/bootctl-status.c +++ b/src/boot/bootctl-status.c @@ -372,6 +372,7 @@ int verb_status(int argc, char *argv[], void *userdata) { { EFI_LOADER_FEATURE_DEVICETREE, "Support Type #1 devicetree field" }, { EFI_LOADER_FEATURE_SECUREBOOT_ENROLL, "Enroll SecureBoot keys" }, { EFI_LOADER_FEATURE_RETAIN_SHIM, "Retain SHIM protocols" }, + { EFI_LOADER_FEATURE_MENU_DISABLE, "Menu can be disabled" }, }; static const struct { uint64_t flag; diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index 43891df9348..48fc79ea232 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -79,9 +79,9 @@ typedef struct { size_t n_entries; size_t idx_default; size_t idx_default_efivar; - uint32_t timeout_sec; /* Actual timeout used (efi_main() override > efivar > config). */ - uint32_t timeout_sec_config; - uint32_t timeout_sec_efivar; + uint64_t timeout_sec; /* Actual timeout used (efi_main() override > efivar > config). */ + uint64_t timeout_sec_config; + uint64_t timeout_sec_efivar; char16_t *entry_default_config; char16_t *entry_default_efivar; char16_t *entry_oneshot; @@ -110,14 +110,18 @@ typedef struct { * * The other values may be set by systemd-boot itself and changing those will lead to functional regression * when new version of systemd-boot is installed. + * + * All the 64bit values are not ABI and will never be written to an efi variable. */ enum { - TIMEOUT_MIN = 1, - TIMEOUT_MAX = UINT32_MAX - 2U, - TIMEOUT_UNSET = UINT32_MAX - 1U, - TIMEOUT_MENU_FORCE = UINT32_MAX, - TIMEOUT_MENU_HIDDEN = 0, - TIMEOUT_TYPE_MAX = UINT32_MAX, + TIMEOUT_MIN = 1, + TIMEOUT_MAX = UINT32_MAX - 2U, + TIMEOUT_UNSET = UINT32_MAX - 1U, + TIMEOUT_MENU_FORCE = UINT32_MAX, + TIMEOUT_MENU_HIDDEN = 0, + TIMEOUT_TYPE_MAX = UINT32_MAX, + TIMEOUT_MENU_DISABLED = (uint64_t)UINT32_MAX + 1U, + TIMEOUT_TYPE_MAX64 = UINT64_MAX, }; enum { @@ -410,6 +414,9 @@ static char16_t* update_timeout_efivar(Config *config, bool inc) { case TIMEOUT_UNSET: config->timeout_sec = inc ? TIMEOUT_MENU_FORCE : TIMEOUT_UNSET; break; + case TIMEOUT_MENU_DISABLED: + config->timeout_sec = inc ? TIMEOUT_MIN : TIMEOUT_MENU_FORCE; + break; case TIMEOUT_MENU_FORCE: config->timeout_sec = inc ? TIMEOUT_MENU_HIDDEN : TIMEOUT_MENU_FORCE; break; @@ -425,12 +432,14 @@ static char16_t* update_timeout_efivar(Config *config, bool inc) { switch (config->timeout_sec) { case TIMEOUT_UNSET: return xstrdup16(u"Menu timeout defined by configuration file."); + case TIMEOUT_MENU_DISABLED: + assert_not_reached(); case TIMEOUT_MENU_FORCE: return xstrdup16(u"Timeout disabled, menu will always be shown."); case TIMEOUT_MENU_HIDDEN: - return xstrdup16(u"Menu disabled. Hold down key at bootup to show menu."); + return xstrdup16(u"Menu hidden. Hold down key at bootup to show menu."); default: - return xasprintf("Menu timeout set to %u s.", config->timeout_sec_efivar); + return xasprintf("Menu timeout set to %u s.", (uint32_t)config->timeout_sec_efivar); } } @@ -454,16 +463,18 @@ static bool ps_continue(void) { !IN_SET(key, KEYPRESS(0, SCAN_ESC, 0), KEYPRESS(0, 0, 'q'), KEYPRESS(0, 0, 'Q')); } -static void print_timeout_status(const char *label, uint32_t t) { +static void print_timeout_status(const char *label, uint64_t t) { switch (t) { case TIMEOUT_UNSET: return; + case TIMEOUT_MENU_DISABLED: + return (void) printf("%s: menu-disabled\n", label); case TIMEOUT_MENU_FORCE: return (void) printf("%s: menu-force\n", label); case TIMEOUT_MENU_HIDDEN: return (void) printf("%s: menu-hidden\n", label); default: - return (void) printf("%s: %u s\n", label, t); + return (void) printf("%s: %u s\n", label, (uint32_t)t); } } @@ -658,7 +669,7 @@ static bool menu_run( size_t x_start = 0, y_start = 0, y_status = 0, x_max, y_max; _cleanup_(strv_freep) char16_t **lines = NULL; _cleanup_free_ char16_t *clearline = NULL, *separator = NULL, *status = NULL; - uint32_t timeout_efivar_saved = config->timeout_sec_efivar; + uint64_t timeout_efivar_saved = config->timeout_sec_efivar; uint32_t timeout_remain = config->timeout_sec == TIMEOUT_MENU_FORCE ? 0 : config->timeout_sec; int64_t console_mode_initial = ST->ConOut->Mode->Mode, console_mode_efivar_saved = config->console_mode_efivar; size_t default_efivar_saved = config->idx_default_efivar; @@ -1104,6 +1115,8 @@ static bool menu_run( case TIMEOUT_UNSET: efivar_unset(MAKE_GUID_PTR(LOADER), u"LoaderConfigTimeout", EFI_VARIABLE_NON_VOLATILE); break; + case TIMEOUT_MENU_DISABLED: + assert_not_reached(); case TIMEOUT_MENU_FORCE: efivar_set(MAKE_GUID_PTR(LOADER), u"LoaderConfigTimeout", u"menu-force", EFI_VARIABLE_NON_VOLATILE); break; @@ -1111,6 +1124,7 @@ static bool menu_run( efivar_set(MAKE_GUID_PTR(LOADER), u"LoaderConfigTimeout", u"menu-hidden", EFI_VARIABLE_NON_VOLATILE); break; default: + assert(config->timeout_sec_efivar < UINT32_MAX); efivar_set_uint_string(MAKE_GUID_PTR(LOADER), u"LoaderConfigTimeout", config->timeout_sec_efivar, EFI_VARIABLE_NON_VOLATILE); } @@ -1183,7 +1197,9 @@ static void config_defaults_load_from_file(Config *config, char *content) { while ((line = line_get_key_value(content, " \t", &pos, &key, &value))) if (streq8(key, "timeout")) { - if (streq8( value, "menu-force")) + if (streq8(value, "menu-disabled")) + config->timeout_sec_config = TIMEOUT_MENU_DISABLED; + else if (streq8(value, "menu-force")) config->timeout_sec_config = TIMEOUT_MENU_FORCE; else if (streq8(value, "menu-hidden")) config->timeout_sec_config = TIMEOUT_MENU_HIDDEN; @@ -1499,7 +1515,7 @@ static void boot_entry_add_type1( TAKE_PTR(entry); } -static EFI_STATUS efivar_get_timeout(const char16_t *var, uint32_t *ret_value) { +static EFI_STATUS efivar_get_timeout(const char16_t *var, uint64_t *ret_value) { _cleanup_free_ char16_t *value = NULL; EFI_STATUS err; @@ -1510,6 +1526,10 @@ static EFI_STATUS efivar_get_timeout(const char16_t *var, uint32_t *ret_value) { if (err != EFI_SUCCESS) return err; + if (streq16(value, u"menu-disabled")) { + *ret_value = TIMEOUT_MENU_DISABLED; + return EFI_SUCCESS; + } if (streq16(value, u"menu-force")) { *ret_value = TIMEOUT_MENU_FORCE; return EFI_SUCCESS; @@ -2510,6 +2530,7 @@ static void export_variables( EFI_LOADER_FEATURE_DEVICETREE | EFI_LOADER_FEATURE_SECUREBOOT_ENROLL | EFI_LOADER_FEATURE_RETAIN_SHIM | + EFI_LOADER_FEATURE_MENU_DISABLE | 0; _cleanup_free_ char16_t *infostr = NULL, *typestr = NULL; @@ -2666,9 +2687,9 @@ static EFI_STATUS run(EFI_HANDLE image) { "No loader found. Configuration files in \\loader\\entries\\*.conf are needed."); /* select entry or show menu when key is pressed or timeout is set */ - if (config.force_menu || config.timeout_sec > 0) + if (config.force_menu || !IN_SET(config.timeout_sec, TIMEOUT_MENU_HIDDEN, TIMEOUT_MENU_DISABLED)) menu = true; - else { + else if (config.timeout_sec != TIMEOUT_MENU_DISABLED) { uint64_t key; /* Block up to 100ms to give firmware time to get input working. */ diff --git a/src/fundamental/efivars-fundamental.h b/src/fundamental/efivars-fundamental.h index 8c9e7ddef28..2d25d2248f6 100644 --- a/src/fundamental/efivars-fundamental.h +++ b/src/fundamental/efivars-fundamental.h @@ -22,6 +22,7 @@ #define EFI_LOADER_FEATURE_DEVICETREE (UINT64_C(1) << 10) #define EFI_LOADER_FEATURE_SECUREBOOT_ENROLL (UINT64_C(1) << 11) #define EFI_LOADER_FEATURE_RETAIN_SHIM (UINT64_C(1) << 12) +#define EFI_LOADER_FEATURE_MENU_DISABLE (UINT64_C(1) << 13) /* Features of the stub, i.e. systemd-stub */ #define EFI_STUB_FEATURE_REPORT_BOOT_PARTITION (UINT64_C(1) << 0)