]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
sd-boot: Rework console input handling
authorJan Janssen <medhefgo@web.de>
Wed, 11 Aug 2021 12:59:46 +0000 (14:59 +0200)
committerJan Janssen <medhefgo@web.de>
Thu, 12 Aug 2021 14:10:02 +0000 (16:10 +0200)
Fixes: #15847
Probably fixes: #19191

src/boot/efi/boot.c
src/boot/efi/console.c
src/boot/efi/console.h

index ab29fb94bfe32f31d5658f080250ca5a378b47d8..96099e573961c7dcca05cef8c8b41417bcff5ebf 100644 (file)
@@ -142,7 +142,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;
 
@@ -400,7 +400,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)
@@ -445,7 +445,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;
@@ -495,7 +495,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);
@@ -526,11 +526,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);
@@ -555,12 +554,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;
 
@@ -660,7 +653,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 */
@@ -681,27 +674,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) {
@@ -804,7 +788,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){
@@ -812,7 +796,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.");
@@ -830,7 +814,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.");
@@ -2472,13 +2456,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;
 
index 532234030b7b5dcda3f774b55ec8c7464660dbf3..3f405113bdb341111597c88d4eab2db0080938a7 100644 (file)
 
 #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;
 
         assert(key);
 
         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) {
index 2c69af552a6f469620f6c8fc8bd010a658b87d1b..23848a9c58a0a4bbcba6a5e2c2dc3238e4710c65 100644 (file)
@@ -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);