From 2d9fcfcfa38667ada306e095599944f941576e53 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Wed, 11 Aug 2021 14:59:46 +0200 Subject: [PATCH 21/21] sd-boot: Rework console input handling Fixes: #15847 Probably fixes: #19191 (cherry picked from commit e98d271e57f3d0356e444b6ea2d48836ee2769b0) --- src/boot/efi/boot.c | 55 +++++++--------------- src/boot/efi/console.c | 102 +++++++++++++++++++++++++++++------------ src/boot/efi/console.h | 2 +- 3 files changed, 91 insertions(+), 68 deletions(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index 54d704f0d1..b4f3b9605a 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -134,7 +134,7 @@ static BOOLEAN line_edit( uefi_call_wrapper(ST->ConOut->OutputString, 2, ST->ConOut, print); uefi_call_wrapper(ST->ConOut->SetCursorPosition, 3, ST->ConOut, cursor, y_pos); - err = console_key_read(&key, TRUE); + err = console_key_read(&key, 0); if (EFI_ERROR(err)) continue; @@ -387,7 +387,7 @@ static VOID print_status(Config *config, CHAR16 *loaded_image_path) { Print(L"OsIndicationsSupported: %d\n", indvar); Print(L"\n--- press key ---\n\n"); - console_key_read(&key, TRUE); + console_key_read(&key, 0); Print(L"timeout: %u\n", config->timeout_sec); if (config->timeout_sec_efivar >= 0) @@ -432,7 +432,7 @@ static VOID print_status(Config *config, CHAR16 *loaded_image_path) { Print(L"LoaderEntryDefault: %s\n", defaultstr); Print(L"\n--- press key ---\n\n"); - console_key_read(&key, TRUE); + console_key_read(&key, 0); for (UINTN i = 0; i < config->entry_count; i++) { ConfigEntry *entry; @@ -482,7 +482,7 @@ static VOID print_status(Config *config, CHAR16 *loaded_image_path) { entry->path, entry->next_name); Print(L"\n--- press key ---\n\n"); - console_key_read(&key, TRUE); + console_key_read(&key, 0); } uefi_call_wrapper(ST->ConOut->ClearScreen, 1, ST->ConOut); @@ -509,11 +509,10 @@ static BOOLEAN menu_run( UINTN y_max; CHAR16 *status; CHAR16 *clearline; - INTN timeout_remain; + UINTN timeout_remain = config->timeout_sec; INT16 idx; BOOLEAN exit = FALSE; BOOLEAN run = TRUE; - BOOLEAN wait = FALSE; graphics_mode(FALSE); uefi_call_wrapper(ST->ConIn->Reset, 2, ST->ConIn, FALSE); @@ -538,12 +537,6 @@ static BOOLEAN menu_run( y_max = 25; } - /* we check 10 times per second for a keystroke */ - if (config->timeout_sec > 0) - timeout_remain = config->timeout_sec * 10; - else - timeout_remain = -1; - idx_highlight = config->idx_default; idx_highlight_prev = 0; @@ -643,7 +636,7 @@ static BOOLEAN menu_run( if (timeout_remain > 0) { FreePool(status); - status = PoolPrint(L"Boot in %d sec.", (timeout_remain + 5) / 10); + status = PoolPrint(L"Boot in %d s.", timeout_remain); } /* print status at last line of screen */ @@ -664,27 +657,18 @@ static BOOLEAN menu_run( uefi_call_wrapper(ST->ConOut->OutputString, 2, ST->ConOut, clearline+1 + x + len); } - err = console_key_read(&key, wait); - if (EFI_ERROR(err)) { - /* timeout reached */ + err = console_key_read(&key, timeout_remain > 0 ? 1000 * 1000 : 0); + if (err == EFI_TIMEOUT) { + timeout_remain--; if (timeout_remain == 0) { exit = TRUE; break; } - /* sleep and update status */ - if (timeout_remain > 0) { - uefi_call_wrapper(BS->Stall, 1, 100 * 1000); - timeout_remain--; - continue; - } - - /* timeout disabled, wait for next key */ - wait = TRUE; + /* update status */ continue; - } - - timeout_remain = -1; + } else + timeout_remain = 0; /* clear status after keystroke */ if (status) { @@ -787,7 +771,7 @@ static BOOLEAN menu_run( config->timeout_sec_efivar, EFI_VARIABLE_NON_VOLATILE); if (config->timeout_sec_efivar > 0) - status = PoolPrint(L"Menu timeout set to %d sec.", config->timeout_sec_efivar); + status = PoolPrint(L"Menu timeout set to %d s.", config->timeout_sec_efivar); else status = StrDuplicate(L"Menu disabled. Hold down key at bootup to show menu."); } else if (config->timeout_sec_efivar <= 0){ @@ -795,7 +779,7 @@ static BOOLEAN menu_run( efivar_set( LOADER_GUID, L"LoaderConfigTimeout", NULL, EFI_VARIABLE_NON_VOLATILE); if (config->timeout_sec_config > 0) - status = PoolPrint(L"Menu timeout of %d sec is defined by configuration file.", + status = PoolPrint(L"Menu timeout of %d s is defined by configuration file.", config->timeout_sec_config); else status = StrDuplicate(L"Menu disabled. Hold down key at bootup to show menu."); @@ -813,7 +797,7 @@ static BOOLEAN menu_run( config->timeout_sec_efivar, EFI_VARIABLE_NON_VOLATILE); if (config->timeout_sec_efivar > 0) - status = PoolPrint(L"Menu timeout set to %d sec.", + status = PoolPrint(L"Menu timeout set to %d s.", config->timeout_sec_efivar); else status = StrDuplicate(L"Menu disabled. Hold down key at bootup to show menu."); @@ -2369,13 +2353,8 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) { else { UINT64 key; - err = console_key_read(&key, FALSE); - - if (err == EFI_NOT_READY) { - uefi_call_wrapper(BS->Stall, 1, 100 * 1000); - err = console_key_read(&key, FALSE); - } - + /* Block up to 100ms to give firmware time to get input working. */ + err = console_key_read(&key, 100 * 1000); if (!EFI_ERROR(err)) { INT16 idx; diff --git a/src/boot/efi/console.c b/src/boot/efi/console.c index 83619d2147..369c549daf 100644 --- a/src/boot/efi/console.c +++ b/src/boot/efi/console.c @@ -11,61 +11,105 @@ #define EFI_SIMPLE_TEXT_INPUT_EX_GUID &(EFI_GUID) EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL_GUID -EFI_STATUS console_key_read(UINT64 *key, BOOLEAN wait) { +static inline void EventClosep(EFI_EVENT *event) { + if (!*event) + return; + + uefi_call_wrapper(BS->CloseEvent, 1, *event); +} + +/* + * Reading input from the console sounds like an easy task to do, but thanks to broken + * firmware it is actually a nightmare. + * + * There is a ConIn and TextInputEx API for this. Ideally we want to use TextInputEx, + * because that gives us Ctrl/Alt/Shift key state information. Unfortunately, it is not + * always available and sometimes just non-functional. + * + * On the other hand we have ConIn, where some firmware likes to just freeze on us + * if we call ReadKeyStroke on it. + * + * Therefore, we use WaitForEvent on both ConIn and TextInputEx (if available) along + * with a timer event. The timer ensures there is no need to call into functions + * that might freeze on us, while still allowing us to show a timeout counter. + */ +EFI_STATUS console_key_read(UINT64 *key, UINT64 timeout_usec) { static EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL *TextInputEx; static BOOLEAN checked; UINTN index; EFI_INPUT_KEY k; EFI_STATUS err; + _cleanup_(EventClosep) EFI_EVENT timer = NULL; + EFI_EVENT events[3] = { ST->ConIn->WaitForKey }; + UINTN n_events = 1; if (!checked) { err = LibLocateProtocol(EFI_SIMPLE_TEXT_INPUT_EX_GUID, (VOID **)&TextInputEx); - if (EFI_ERROR(err)) + if (EFI_ERROR(err) || + uefi_call_wrapper(BS->CheckEvent, 1, TextInputEx->WaitForKeyEx) == EFI_INVALID_PARAMETER) + /* If WaitForKeyEx fails here, the firmware pretends it talks this + * protocol, but it really doesn't. */ TextInputEx = NULL; + else + events[n_events++] = TextInputEx->WaitForKeyEx; checked = TRUE; } - /* wait until key is pressed */ - if (wait) - uefi_call_wrapper(BS->WaitForEvent, 3, 1, &ST->ConIn->WaitForKey, &index); + if (timeout_usec > 0) { + err = uefi_call_wrapper(BS->CreateEvent, 5, EVT_TIMER, 0, NULL, NULL, &timer); + if (EFI_ERROR(err)) + return log_error_status_stall(err, L"Error creating timer event: %r", err); + + /* SetTimer expects 100ns units for some reason. */ + err = uefi_call_wrapper(BS->SetTimer, 3, timer, TimerRelative, timeout_usec * 10); + if (EFI_ERROR(err)) + return log_error_status_stall(err, L"Error arming timer event: %r", err); - if (TextInputEx) { + events[n_events++] = timer; + } + + err = uefi_call_wrapper(BS->WaitForEvent, 3, n_events, events, &index); + if (EFI_ERROR(err)) + return log_error_status_stall(err, L"Error waiting for events: %r", err); + + if (timeout_usec > 0 && timer == events[index]) + return EFI_TIMEOUT; + + /* TextInputEx might be ready too even if ConIn got to signal first. */ + if (TextInputEx && !EFI_ERROR(uefi_call_wrapper(BS->CheckEvent, 1, TextInputEx->WaitForKeyEx))) { EFI_KEY_DATA keydata; UINT64 keypress; + UINT32 shift = 0; err = uefi_call_wrapper(TextInputEx->ReadKeyStrokeEx, 2, TextInputEx, &keydata); - if (!EFI_ERROR(err)) { - UINT32 shift = 0; - - /* do not distinguish between left and right keys */ - if (keydata.KeyState.KeyShiftState & EFI_SHIFT_STATE_VALID) { - if (keydata.KeyState.KeyShiftState & (EFI_RIGHT_CONTROL_PRESSED|EFI_LEFT_CONTROL_PRESSED)) - shift |= EFI_CONTROL_PRESSED; - if (keydata.KeyState.KeyShiftState & (EFI_RIGHT_ALT_PRESSED|EFI_LEFT_ALT_PRESSED)) - shift |= EFI_ALT_PRESSED; - }; - - /* 32 bit modifier keys + 16 bit scan code + 16 bit unicode */ - keypress = KEYPRESS(shift, keydata.Key.ScanCode, keydata.Key.UnicodeChar); - if (keypress > 0) { - *key = keypress; - return 0; - } + if (EFI_ERROR(err)) + return err; + + /* do not distinguish between left and right keys */ + if (keydata.KeyState.KeyShiftState & EFI_SHIFT_STATE_VALID) { + if (keydata.KeyState.KeyShiftState & (EFI_RIGHT_CONTROL_PRESSED|EFI_LEFT_CONTROL_PRESSED)) + shift |= EFI_CONTROL_PRESSED; + if (keydata.KeyState.KeyShiftState & (EFI_RIGHT_ALT_PRESSED|EFI_LEFT_ALT_PRESSED)) + shift |= EFI_ALT_PRESSED; + }; + + /* 32 bit modifier keys + 16 bit scan code + 16 bit unicode */ + keypress = KEYPRESS(shift, keydata.Key.ScanCode, keydata.Key.UnicodeChar); + if (keypress > 0) { + *key = keypress; + return EFI_SUCCESS; } + + return EFI_NOT_READY; } - /* fallback for firmware which does not support SimpleTextInputExProtocol - * - * This is also called in case ReadKeyStrokeEx did not return a key, because - * some broken firmwares offer SimpleTextInputExProtocol, but never actually - * handle any key. */ err = uefi_call_wrapper(ST->ConIn->ReadKeyStroke, 2, ST->ConIn, &k); if (EFI_ERROR(err)) return err; *key = KEYPRESS(0, k.ScanCode, k.UnicodeChar); - return 0; + return EFI_SUCCESS; } static EFI_STATUS change_mode(UINTN mode) { diff --git a/src/boot/efi/console.h b/src/boot/efi/console.h index 2c69af552a..23848a9c58 100644 --- a/src/boot/efi/console.h +++ b/src/boot/efi/console.h @@ -16,5 +16,5 @@ enum console_mode_change_type { CONSOLE_MODE_MAX, }; -EFI_STATUS console_key_read(UINT64 *key, BOOLEAN wait); +EFI_STATUS console_key_read(UINT64 *key, UINT64 timeout_usec); EFI_STATUS console_set_mode(UINTN *mode, enum console_mode_change_type how); -- 2.33.0