[PATCH v3 0/6] Improvements to EFI console and terminal drivers for Flicker Free Boot

[PATCH v3 0/6] Improvements to EFI console and terminal drivers for Flicker Free Boot

[Javier Martinez Canillas]

Hello,

This is a v3 of a patch-set that contains improvements for the EFI console and
terminal drivers that allows to avoid the switch to text-mode until is needed.

This is part of the patches we have in Fedora to support the Flicker Free Boot
feature [1], which allows to have a seamless boot experience with no graphical
transitions that aren't necessary. More info can be found in Hans' blog [2,3].

We have additional patches for this feature, for example template changes that
makes hiding the menu only when the previous boot was successful, and to make
GRUB less noisy and not print unnecessary output. But those will be posted in
the future once the paches on this set land.

[1]: https://fedoraproject.org/wiki/Changes/FlickerFreeBoot
[2]: https://hansdegoede.livejournal.com/19224.html
[3]: https://hansdegoede.livejournal.com/20632.html

Best regards,
Javier

Changes in v3:
- Make grub_console_read_key_stroke() return GRUB_ERR_NONE instead of 0.
- Make grub_prepare_for_text_output() return GRUB_ERR_NONE instead of 0.
- Code style cleanups.
- Fix a left over in the documentation.

Changes in v2:
- Mention in commit message that logic is unchanged after patch #1.
- Add better explanation about why the change in patch #1 is needed.
- Also mention in commit message that logic is unchanged after patch #2.
- Explain why the helper function needs to be available for other modules.
- Move patch #2 earlier in the patch-set since is a proparatory change.
- Explain why the getkeystatus() handler is needed for the EFI console driver.
- Code style cleanups.
- Fix commit message that had left overs from when F8 was used instead of F4.
- Update documentation to explain that F4 and SHIFT are used now besides ESC.
- Also fix comments to reflect that F4 is used now and use proper style.

Hans de Goede (5):
  kern/term: Make grub_getkeystatus helper function available everywhere
  efi/console: Add grub_console_read_key_stroke() helper function
  efi/console: Implement getkeystatus() support
  efi/console: Do not set text-mode until we actually need it
  kern/term: Accept ESC, F4 and holding SHIFT as user interrupt keys

Javier Martinez Canillas (1):
  efi/console: Move grub_console_set{colorstate,cursor} higher in the
    file

 docs/grub.texi                 |  33 ++---
 grub-core/commands/keystatus.c |  18 ---
 grub-core/commands/sleep.c     |   2 +-
 grub-core/kern/term.c          |  39 ++++++
 grub-core/normal/menu.c        |   2 +-
 grub-core/term/efi/console.c   | 244 ++++++++++++++++++++++-----------
 include/grub/term.h            |   6 +-
 7 files changed, 228 insertions(+), 116 deletions(-)

-- 
2.25.1

[PATCH v3 1/6] efi/console: Move grub_console_set{colorstate, cursor} higher in the file

[Javier Martinez Canillas]

This is just a preparatory patch to move the functions higher in the file,
since these will be called by the grub_prepare_for_text_output() function
that will be introduced in a later patch.

The logic is unchanged by this patch. Functions definitions are just moved
to avoid a forward declaration in a later patch, keeping the code clean.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---

Changes in v3: None
Changes in v2:
- Mention in commit message that logic is unchanged after patch #1.
- Add better explanation about why the change in patch #1 is needed.

 grub-core/term/efi/console.c | 82 ++++++++++++++++++------------------
 1 file changed, 41 insertions(+), 41 deletions(-)

diff --git a/grub-core/term/efi/console.c b/grub-core/term/efi/console.c
index 4840cc59d3f..59d9604472a 100644
--- a/grub-core/term/efi/console.c
+++ b/grub-core/term/efi/console.c
@@ -65,6 +65,47 @@ map_char (grub_uint32_t c)
   return c;
 }
 
+static void
+grub_console_setcolorstate (struct grub_term_output *term
+			    __attribute__ ((unused)),
+			    grub_term_color_state state)
+{
+  grub_efi_simple_text_output_interface_t *o;
+
+  if (grub_efi_is_finished)
+    return;
+
+  o = grub_efi_system_table->con_out;
+
+  switch (state) {
+    case GRUB_TERM_COLOR_STANDARD:
+      efi_call_2 (o->set_attributes, o, GRUB_TERM_DEFAULT_STANDARD_COLOR
+		  & 0x7f);
+      break;
+    case GRUB_TERM_COLOR_NORMAL:
+      efi_call_2 (o->set_attributes, o, grub_term_normal_color & 0x7f);
+      break;
+    case GRUB_TERM_COLOR_HIGHLIGHT:
+      efi_call_2 (o->set_attributes, o, grub_term_highlight_color & 0x7f);
+      break;
+    default:
+      break;
+  }
+}
+
+static void
+grub_console_setcursor (struct grub_term_output *term __attribute__ ((unused)),
+			int on)
+{
+  grub_efi_simple_text_output_interface_t *o;
+
+  if (grub_efi_is_finished)
+    return;
+
+  o = grub_efi_system_table->con_out;
+  efi_call_2 (o->enable_cursor, o, on);
+}
+
 static void
 grub_console_putchar (struct grub_term_output *term __attribute__ ((unused)),
 		      const struct grub_unicode_glyph *c)
@@ -281,47 +322,6 @@ grub_console_cls (struct grub_term_output *term __attribute__ ((unused)))
   efi_call_2 (o->set_attributes, o, orig_attr);
 }
 
-static void
-grub_console_setcolorstate (struct grub_term_output *term
-			    __attribute__ ((unused)),
-			    grub_term_color_state state)
-{
-  grub_efi_simple_text_output_interface_t *o;
-
-  if (grub_efi_is_finished)
-    return;
-
-  o = grub_efi_system_table->con_out;
-
-  switch (state) {
-    case GRUB_TERM_COLOR_STANDARD:
-      efi_call_2 (o->set_attributes, o, GRUB_TERM_DEFAULT_STANDARD_COLOR
-		  & 0x7f);
-      break;
-    case GRUB_TERM_COLOR_NORMAL:
-      efi_call_2 (o->set_attributes, o, grub_term_normal_color & 0x7f);
-      break;
-    case GRUB_TERM_COLOR_HIGHLIGHT:
-      efi_call_2 (o->set_attributes, o, grub_term_highlight_color & 0x7f);
-      break;
-    default:
-      break;
-  }
-}
-
-static void
-grub_console_setcursor (struct grub_term_output *term __attribute__ ((unused)),
-			int on)
-{
-  grub_efi_simple_text_output_interface_t *o;
-
-  if (grub_efi_is_finished)
-    return;
-
-  o = grub_efi_system_table->con_out;
-  efi_call_2 (o->enable_cursor, o, on);
-}
-
 static grub_err_t
 grub_efi_console_output_init (struct grub_term_output *term)
 {
-- 
2.25.1

[PATCH v3 2/6] kern/term: Make grub_getkeystatus helper function available everywhere

[Javier Martinez Canillas]

From: Hans de Goede <hdegoede@redhat.com>

Move grub_getkeystatushelper function from grub-core/commands/keystatus.c
to grub-core/kern/term.c and export it so that it can be used outside of
the keystatus command code too.

There's no logic change in this patch. The function definition is moved so
it can be called from grub-core/kern/term.c in a subsequent patch. It will
be used to determine if a SHIFT key has was held down and use that also to
interrupt the countdown, without the need to press a key at the right time.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---

Changes in v3: None
Changes in v2:
- Also mention in commit message that logic is unchanged after patch #2.
- Explain why the helper function needs to be available for other modules.
- Move patch #2 earlier in the patch-set since is a proparatory change.

 grub-core/commands/keystatus.c | 18 ------------------
 grub-core/kern/term.c          | 18 ++++++++++++++++++
 include/grub/term.h            |  1 +
 3 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/grub-core/commands/keystatus.c b/grub-core/commands/keystatus.c
index 460cf4e7e50..ff3f5878163 100644
--- a/grub-core/commands/keystatus.c
+++ b/grub-core/commands/keystatus.c
@@ -35,24 +35,6 @@ static const struct grub_arg_option options[] =
     {0, 0, 0, 0, 0, 0}
   };
 
-static int
-grub_getkeystatus (void)
-{
-  int status = 0;
-  grub_term_input_t term;
-
-  if (grub_term_poll_usb)
-    grub_term_poll_usb (0);
-
-  FOR_ACTIVE_TERM_INPUTS(term)
-  {
-    if (term->getkeystatus)
-      status |= term->getkeystatus (term);
-  }
-
-  return status;
-}
-
 static grub_err_t
 grub_cmd_keystatus (grub_extcmd_context_t ctxt,
 		    int argc __attribute__ ((unused)),
diff --git a/grub-core/kern/term.c b/grub-core/kern/term.c
index 07720ee6746..93bd3378d18 100644
--- a/grub-core/kern/term.c
+++ b/grub-core/kern/term.c
@@ -120,6 +120,24 @@ grub_getkey (void)
     }
 }
 
+int
+grub_getkeystatus (void)
+{
+  int status = 0;
+  grub_term_input_t term;
+
+  if (grub_term_poll_usb)
+    grub_term_poll_usb (0);
+
+  FOR_ACTIVE_TERM_INPUTS(term)
+  {
+    if (term->getkeystatus)
+      status |= term->getkeystatus (term);
+  }
+
+  return status;
+}
+
 void
 grub_refresh (void)
 {
diff --git a/include/grub/term.h b/include/grub/term.h
index 8117e2a24da..c215133383f 100644
--- a/include/grub/term.h
+++ b/include/grub/term.h
@@ -327,6 +327,7 @@ grub_term_unregister_output (grub_term_output_t term)
 void grub_putcode (grub_uint32_t code, struct grub_term_output *term);
 int EXPORT_FUNC(grub_getkey) (void);
 int EXPORT_FUNC(grub_getkey_noblock) (void);
+int EXPORT_FUNC(grub_getkeystatus) (void);
 void grub_cls (void);
 void EXPORT_FUNC(grub_refresh) (void);
 void grub_puts_terminal (const char *str, struct grub_term_output *term);
-- 
2.25.1

[PATCH v3 3/6] efi/console: Add grub_console_read_key_stroke() helper function

[Javier Martinez Canillas]

From: Hans de Goede <hdegoede@redhat.com>

This is a preparatory patch for adding getkeystatus() support to the
EFI console driver.

We can get modifier status through the simple_text_input read_key_stroke
method, but if a non-modifier key is (also) pressed the read_key_stroke
call will consume that key from the firmware's queue.

The new grub_console_read_key_stroke() helper buffers upto 1 key-stroke.
If it has a non-modifier key buffered, it will return that one, if its
buffer is empty, it will fills its buffer by getting a new key-stroke.

If called with consume=1 it will empty its buffer after copying the
key-data to the callers buffer, this is how getkey() will use it.

If called with consume=0 it will keep the last key-stroke buffered, this
is how getkeystatus() will call it. This means that if a non-modifier
key gets pressed, repeated getkeystatus() calls will return the modifiers
of that key-press until it is consumed by a getkey() call.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---

Changes in v3:
- Make grub_console_read_key_stroke() return GRUB_ERR_NONE instead of 0.

Changes in v2: None

 grub-core/term/efi/console.c | 54 ++++++++++++++++++++++++++++--------
 1 file changed, 42 insertions(+), 12 deletions(-)

diff --git a/grub-core/term/efi/console.c b/grub-core/term/efi/console.c
index 59d9604472a..0cb965a9931 100644
--- a/grub-core/term/efi/console.c
+++ b/grub-core/term/efi/console.c
@@ -193,27 +193,57 @@ grub_console_getkey_con (struct grub_term_input *term __attribute__ ((unused)))
   return grub_efi_translate_key(key);
 }
 
-static int
-grub_console_getkey_ex(struct grub_term_input *term)
+/*
+ * When more then just modifiers are pressed, our getkeystatus() consumes a
+ * press from the queue, this function buffers the press for the regular
+ * getkey() so that it does not get lost.
+ */
+static grub_err_t
+grub_console_read_key_stroke (
+                   grub_efi_simple_text_input_ex_interface_t *text_input,
+                   grub_efi_key_data_t *key_data_ret, int *key_ret,
+                   int consume)
 {
-  grub_efi_key_data_t key_data;
+  static grub_efi_key_data_t key_data;
   grub_efi_status_t status;
-  grub_efi_uint32_t kss;
-  int key = -1;
+  int key;
 
-  grub_efi_simple_text_input_ex_interface_t *text_input = term->data;
+  if (!text_input)
+    return GRUB_ERR_EOF;
 
-  status = efi_call_2 (text_input->read_key_stroke, text_input, &key_data);
+  key = grub_efi_translate_key (key_data.key);
+  if (key == GRUB_TERM_NO_KEY) {
+    status = efi_call_2 (text_input->read_key_stroke, text_input, &key_data);
+    if (status != GRUB_EFI_SUCCESS)
+      return GRUB_ERR_EOF;
 
-  if (status != GRUB_EFI_SUCCESS)
-    return GRUB_TERM_NO_KEY;
+    key = grub_efi_translate_key (key_data.key);
+  }
 
-  kss = key_data.key_state.key_shift_state;
-  key = grub_efi_translate_key(key_data.key);
+  *key_data_ret = key_data;
+  *key_ret = key;
 
-  if (key == GRUB_TERM_NO_KEY)
+  if (consume) {
+    key_data.key.scan_code = 0;
+    key_data.key.unicode_char = 0;
+  }
+
+  return GRUB_ERR_NONE;
+}
+
+static int
+grub_console_getkey_ex (struct grub_term_input *term)
+{
+  grub_efi_key_data_t key_data;
+  grub_efi_uint32_t kss;
+  grub_err_t err;
+  int key = -1;
+
+  err = grub_console_read_key_stroke (term->data, &key_data, &key, 1);
+  if (err != GRUB_ERR_NONE || key == GRUB_TERM_NO_KEY)
     return GRUB_TERM_NO_KEY;
 
+  kss = key_data.key_state.key_shift_state;
   if (kss & GRUB_EFI_SHIFT_STATE_VALID)
     {
       if ((kss & GRUB_EFI_LEFT_SHIFT_PRESSED
-- 
2.25.1

[PATCH v3 4/6] efi/console: Implement getkeystatus() support

[Javier Martinez Canillas]

From: Hans de Goede <hdegoede@redhat.com>

Implement getkeystatus() support in the EFI console driver.

This is needed because the logic to determine if a key was pressed to make
the menu countdown stop will be changed by a later patch to also take into
account the SHIFT key being held down.

For this reason the EFI console driver has to support getkeystatus() to
allow detecting that event.

Note that if a non-modifier key gets pressed and repeated calls to
getkeystatus() are made then it will return the modifier status at the
time of the non-modifier key, until that key-press gets consumed by a
getkey() call.

This is a side-effect of how the EFI simple-text-input protocol works
and cannot be avoided.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---

Changes in v3: None
Changes in v2:
- Explain why the getkeystatus() handler is needed for the EFI console driver.
- Code style cleanups.

 grub-core/term/efi/console.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/grub-core/term/efi/console.c b/grub-core/term/efi/console.c
index 0cb965a9931..64591b90aad 100644
--- a/grub-core/term/efi/console.c
+++ b/grub-core/term/efi/console.c
@@ -260,6 +260,39 @@ grub_console_getkey_ex (struct grub_term_input *term)
   return key;
 }
 
+static int
+grub_console_getkeystatus (struct grub_term_input *term)
+{
+  grub_efi_key_data_t key_data;
+  grub_efi_uint32_t kss;
+  int key, mods = 0;
+
+  if (grub_efi_is_finished)
+    return 0;
+
+  if (grub_console_read_key_stroke (term->data, &key_data, &key, 0))
+    return 0;
+
+  kss = key_data.key_state.key_shift_state;
+  if (kss & GRUB_EFI_SHIFT_STATE_VALID)
+    {
+      if (kss & GRUB_EFI_LEFT_SHIFT_PRESSED)
+        mods |= GRUB_TERM_STATUS_LSHIFT;
+      if (kss & GRUB_EFI_RIGHT_SHIFT_PRESSED)
+        mods |= GRUB_TERM_STATUS_RSHIFT;
+      if (kss & GRUB_EFI_LEFT_ALT_PRESSED)
+        mods |= GRUB_TERM_STATUS_LALT;
+      if (kss & GRUB_EFI_RIGHT_ALT_PRESSED)
+        mods |= GRUB_TERM_STATUS_RALT;
+      if (kss & GRUB_EFI_LEFT_CONTROL_PRESSED)
+        mods |= GRUB_TERM_STATUS_LCTRL;
+      if (kss & GRUB_EFI_RIGHT_CONTROL_PRESSED)
+        mods |= GRUB_TERM_STATUS_RCTRL;
+    }
+
+  return mods;
+}
+
 static grub_err_t
 grub_efi_console_input_init (struct grub_term_input *term)
 {
@@ -372,6 +405,7 @@ static struct grub_term_input grub_console_term_input =
   {
     .name = "console",
     .getkey = grub_console_getkey,
+    .getkeystatus = grub_console_getkeystatus,
     .init = grub_efi_console_input_init,
   };
 
-- 
2.25.1

[PATCH v3 5/6] efi/console: Do not set text-mode until we actually need it

[Javier Martinez Canillas]

From: Hans de Goede <hdegoede@redhat.com>

If we're running with a hidden menu we may never need text mode, so do not
change the video-mode to text until we actually need it.

This allows to boot a machine without unnecessary graphical transitions and
provide a seamless boot experience to users.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---

Changes in v3:
- Make grub_prepare_for_text_output() return GRUB_ERR_NONE instead of 0.

Changes in v2: None

 grub-core/term/efi/console.c | 74 +++++++++++++++++++++++-------------
 include/grub/term.h          |  4 +-
 2 files changed, 51 insertions(+), 27 deletions(-)

diff --git a/grub-core/term/efi/console.c b/grub-core/term/efi/console.c
index 64591b90aad..2f1ae85ba7d 100644
--- a/grub-core/term/efi/console.c
+++ b/grub-core/term/efi/console.c
@@ -24,6 +24,16 @@
 #include <grub/efi/api.h>
 #include <grub/efi/console.h>
 
+typedef enum {
+    GRUB_TEXT_MODE_UNDEFINED = -1,
+    GRUB_TEXT_MODE_UNAVAILABLE = 0,
+    GRUB_TEXT_MODE_AVAILABLE
+}
+grub_text_mode;
+
+static grub_text_mode text_mode = GRUB_TEXT_MODE_UNDEFINED;
+static grub_term_color_state text_colorstate = GRUB_TERM_COLOR_UNDEFINED;
+
 static grub_uint32_t
 map_char (grub_uint32_t c)
 {
@@ -106,15 +116,39 @@ grub_console_setcursor (struct grub_term_output *term __attribute__ ((unused)),
   efi_call_2 (o->enable_cursor, o, on);
 }
 
+static grub_err_t
+grub_prepare_for_text_output (struct grub_term_output *term)
+{
+  if (grub_efi_is_finished)
+    return GRUB_ERR_BAD_DEVICE;
+
+  if (text_mode != GRUB_TEXT_MODE_UNDEFINED)
+    return text_mode ? GRUB_ERR_NONE : GRUB_ERR_BAD_DEVICE;
+
+  if (! grub_efi_set_text_mode (1))
+    {
+      /* This really should never happen */
+      grub_error (GRUB_ERR_BAD_DEVICE, "cannot set text mode");
+      text_mode = GRUB_TEXT_MODE_UNAVAILABLE;
+      return GRUB_ERR_BAD_DEVICE;
+    }
+
+  grub_console_setcursor (term, 1);
+  if (text_colorstate != GRUB_TERM_COLOR_UNDEFINED)
+    grub_console_setcolorstate (term, text_colorstate);
+  text_mode = GRUB_TEXT_MODE_AVAILABLE;
+  return GRUB_ERR_NONE;
+}
+
 static void
-grub_console_putchar (struct grub_term_output *term __attribute__ ((unused)),
+grub_console_putchar (struct grub_term_output *term,
 		      const struct grub_unicode_glyph *c)
 {
   grub_efi_char16_t str[2 + 30];
   grub_efi_simple_text_output_interface_t *o;
   unsigned i, j;
 
-  if (grub_efi_is_finished)
+  if (grub_prepare_for_text_output (term) != GRUB_ERR_NONE)
     return;
 
   o = grub_efi_system_table->con_out;
@@ -327,14 +361,15 @@ grub_console_getkey (struct grub_term_input *term)
 }
 
 static struct grub_term_coordinate
-grub_console_getwh (struct grub_term_output *term __attribute__ ((unused)))
+grub_console_getwh (struct grub_term_output *term)
 {
   grub_efi_simple_text_output_interface_t *o;
   grub_efi_uintn_t columns, rows;
 
   o = grub_efi_system_table->con_out;
-  if (grub_efi_is_finished || efi_call_4 (o->query_mode, o, o->mode->mode,
-					  &columns, &rows) != GRUB_EFI_SUCCESS)
+  if (grub_prepare_for_text_output (term) != GRUB_ERR_NONE ||
+      efi_call_4 (o->query_mode, o, o->mode->mode,
+		  &columns, &rows) != GRUB_EFI_SUCCESS)
     {
       /* Why does this fail?  */
       columns = 80;
@@ -349,7 +384,7 @@ grub_console_getxy (struct grub_term_output *term __attribute__ ((unused)))
 {
   grub_efi_simple_text_output_interface_t *o;
 
-  if (grub_efi_is_finished)
+  if (grub_efi_is_finished || text_mode != GRUB_TEXT_MODE_AVAILABLE)
     return (struct grub_term_coordinate) { 0, 0 };
 
   o = grub_efi_system_table->con_out;
@@ -357,12 +392,12 @@ grub_console_getxy (struct grub_term_output *term __attribute__ ((unused)))
 }
 
 static void
-grub_console_gotoxy (struct grub_term_output *term __attribute__ ((unused)),
+grub_console_gotoxy (struct grub_term_output *term,
 		     struct grub_term_coordinate pos)
 {
   grub_efi_simple_text_output_interface_t *o;
 
-  if (grub_efi_is_finished)
+  if (grub_prepare_for_text_output (term) != GRUB_ERR_NONE)
     return;
 
   o = grub_efi_system_table->con_out;
@@ -375,7 +410,7 @@ grub_console_cls (struct grub_term_output *term __attribute__ ((unused)))
   grub_efi_simple_text_output_interface_t *o;
   grub_efi_int32_t orig_attr;
 
-  if (grub_efi_is_finished)
+  if (grub_efi_is_finished || text_mode != GRUB_TEXT_MODE_AVAILABLE)
     return;
 
   o = grub_efi_system_table->con_out;
@@ -385,19 +420,15 @@ grub_console_cls (struct grub_term_output *term __attribute__ ((unused)))
   efi_call_2 (o->set_attributes, o, orig_attr);
 }
 
-static grub_err_t
-grub_efi_console_output_init (struct grub_term_output *term)
-{
-  grub_efi_set_text_mode (1);
-  grub_console_setcursor (term, 1);
-  return 0;
-}
-
 static grub_err_t
 grub_efi_console_output_fini (struct grub_term_output *term)
 {
+  if (text_mode != GRUB_TEXT_MODE_AVAILABLE)
+    return 0;
+
   grub_console_setcursor (term, 0);
   grub_efi_set_text_mode (0);
+  text_mode = GRUB_TEXT_MODE_UNDEFINED;
   return 0;
 }
 
@@ -412,7 +443,6 @@ static struct grub_term_input grub_console_term_input =
 static struct grub_term_output grub_console_term_output =
   {
     .name = "console",
-    .init = grub_efi_console_output_init,
     .fini = grub_efi_console_output_fini,
     .putchar = grub_console_putchar,
     .getwh = grub_console_getwh,
@@ -428,14 +458,6 @@ static struct grub_term_output grub_console_term_output =
 void
 grub_console_init (void)
 {
-  /* FIXME: it is necessary to consider the case where no console control
-     is present but the default is already in text mode.  */
-  if (! grub_efi_set_text_mode (1))
-    {
-      grub_error (GRUB_ERR_BAD_DEVICE, "cannot set text mode");
-      return;
-    }
-
   grub_term_register_output ("console", &grub_console_term_output);
   grub_term_register_input ("console", &grub_console_term_input);
 }
diff --git a/include/grub/term.h b/include/grub/term.h
index c215133383f..9da03dc751e 100644
--- a/include/grub/term.h
+++ b/include/grub/term.h
@@ -75,9 +75,11 @@
 /* These are used to represent the various color states we use.  */
 typedef enum
   {
+    /* Used for uninitialized grub_term_color_state variables */
+    GRUB_TERM_COLOR_UNDEFINED = -1,
     /* The color used to display all text that does not use the
        user defined colors below.  */
-    GRUB_TERM_COLOR_STANDARD,
+    GRUB_TERM_COLOR_STANDARD = 0,
     /* The user defined colors for normal text.  */
     GRUB_TERM_COLOR_NORMAL,
     /* The user defined colors for highlighted text.  */
-- 
2.25.1

[PATCH v3 6/6] kern/term: Accept ESC, F4 and holding SHIFT as user interrupt keys

[Javier Martinez Canillas]

From: Hans de Goede <hdegoede@redhat.com>

On some devices the ESC key is the hotkey to enter the BIOS/EFI setup
screen, making it really hard to time pressing it right. Besides that
ESC is also pretty hard to discover for a user who does not know it
will unhide the menu.

This commit makes F4, which was chosen because is not used as a hotkey
to enter the BIOS setup by any vendor, also interrupt sleeps / stop the
menu countdown.

This solves the ESC gets into the BIOS setup and also somewhat solves
the discoverability issue, but leaves the timing issue unresolved.

This commit fixes the timing issue by also adding support for keeping
SHIFT pressed during boot to stop the menu countdown. This matches
what Ubuntu is doing, which should also help with discoverability.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---

Changes in v3:
- Code style cleanups.
- Fix a left over in the documentation.

Changes in v2:
- Fix commit message that had left overs from when F8 was used instead of F4.
- Update documentation to explain that F4 and SHIFT are used now besides ESC.
- Also fix comments to reflect that F4 is used now and use proper style.

 docs/grub.texi             | 33 +++++++++++++++++----------------
 grub-core/commands/sleep.c |  2 +-
 grub-core/kern/term.c      | 21 +++++++++++++++++++++
 grub-core/normal/menu.c    |  2 +-
 include/grub/term.h        |  1 +
 5 files changed, 41 insertions(+), 18 deletions(-)

diff --git a/docs/grub.texi b/docs/grub.texi
index 8e6f9acecfa..24d37421a23 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -1330,12 +1330,12 @@ menu and then wait for the timeout set by @samp{GRUB_TIMEOUT} to expire
 before booting the default entry.  Pressing a key interrupts the timeout.
 
 If this option is set to @samp{countdown} or @samp{hidden}, then, before
-displaying the menu, GRUB will wait for the timeout set by
-@samp{GRUB_TIMEOUT} to expire.  If @key{ESC} is pressed during that time, it
-will display the menu and wait for input.  If a hotkey associated with a
-menu entry is pressed, it will boot the associated menu entry immediately.
-If the timeout expires before either of these happens, it will boot the
-default entry.  In the @samp{countdown} case, it will show a one-line
+displaying the menu, GRUB will wait for the timeout set by @samp{GRUB_TIMEOUT}
+to expire.  If @key{ESC} or @key{F4} are pressed, or @key{SHIFT} is held down
+during that time, it will display the menu and wait for input.  If a hotkey
+associated with a menu entry is pressed, it will boot the associated menu entry
+immediately. If the timeout expires before either of these happens, it will
+boot the default entry.  In the @samp{countdown} case, it will show a one-line
 indication of the remaining time.
 
 @item GRUB_DEFAULT_BUTTON
@@ -1559,16 +1559,16 @@ configurations, but have better replacements:
 
 @table @samp
 @item GRUB_HIDDEN_TIMEOUT
-Wait this many seconds before displaying the menu.  If @key{ESC} is pressed
-during that time, display the menu and wait for input according to
-@samp{GRUB_TIMEOUT}.  If a hotkey associated with a menu entry is pressed,
-boot the associated menu entry immediately.  If the timeout expires before
-either of these happens, display the menu for the number of seconds
-specified in @samp{GRUB_TIMEOUT} before booting the default entry.
+Wait this many seconds before displaying the menu.  If @key{ESC} or @key{F4} are
+pressed, or @key{SHIFT} is held down during that time, display the menu and wait
+for input according to @samp{GRUB_TIMEOUT}.  If a hotkey associated with a menu
+entry is pressed, boot the associated menu entry immediately.  If the timeout
+expires before either of these happens, display the menu for the number of
+seconds specified in @samp{GRUB_TIMEOUT} before booting the default entry.
 
 If you set @samp{GRUB_HIDDEN_TIMEOUT}, you should also set
 @samp{GRUB_TIMEOUT=0} so that the menu is not displayed at all unless
-@key{ESC} is pressed.
+@key{ESC} or @key{F4} are pressed, or @key{SHIFT} is held down.
 
 This option is unset by default, and is deprecated in favour of the less
 confusing @samp{GRUB_TIMEOUT_STYLE=countdown} or
@@ -5162,9 +5162,10 @@ Alias for @code{hashsum --hash sha512 arg @dots{}}. See command @command{hashsum
 
 @deffn Command sleep [@option{--verbose}] [@option{--interruptible}] count
 Sleep for @var{count} seconds. If option @option{--interruptible} is given,
-allow @key{ESC} to interrupt sleep. With @option{--verbose} show countdown
-of remaining seconds. Exit code is set to 0 if timeout expired and to 1
-if timeout was interrupted by @key{ESC}.
+allow pressing @key{ESC}, @key{F4} or holding down @key{SHIFT} to interrupt
+sleep.  With @option{--verbose} show countdown of remaining seconds. Exit code
+is set to 0 if timeout expired and to 1 if timeout was interrupted using any
+of the mentioned keys.
 @end deffn
 
 
diff --git a/grub-core/commands/sleep.c b/grub-core/commands/sleep.c
index e77e7900fac..a1370b710c9 100644
--- a/grub-core/commands/sleep.c
+++ b/grub-core/commands/sleep.c
@@ -55,7 +55,7 @@ grub_interruptible_millisleep (grub_uint32_t ms)
   start = grub_get_time_ms ();
 
   while (grub_get_time_ms () - start < ms)
-    if (grub_getkey_noblock () == GRUB_TERM_ESC)
+    if (grub_key_is_interrupt (grub_getkey_noblock ()))
       return 1;
 
   return 0;
diff --git a/grub-core/kern/term.c b/grub-core/kern/term.c
index 93bd3378d18..14d59649832 100644
--- a/grub-core/kern/term.c
+++ b/grub-core/kern/term.c
@@ -138,6 +138,27 @@ grub_getkeystatus (void)
   return status;
 }
 
+int
+grub_key_is_interrupt (int key)
+{
+  /*
+   * ESC sometimes is the BIOS setup hotkey and may be hard to discover, also
+   * check F4, which was chosen because is not used as a hotkey to enter the
+   * BIOS setup by any vendor.
+   */
+  if (key == GRUB_TERM_ESC || key == GRUB_TERM_KEY_F4)
+    return 1;
+
+  /*
+   * Pressing keys at the right time during boot is hard to time, also allow
+   * interrupting sleeps / the menu countdown by keeping shift pressed.
+   */
+  if (grub_getkeystatus() & (GRUB_TERM_STATUS_LSHIFT | GRUB_TERM_STATUS_RSHIFT))
+    return 1;
+
+  return 0;
+}
+
 void
 grub_refresh (void)
 {
diff --git a/grub-core/normal/menu.c b/grub-core/normal/menu.c
index da66ad89180..8397886fa05 100644
--- a/grub-core/normal/menu.c
+++ b/grub-core/normal/menu.c
@@ -623,7 +623,7 @@ run_menu (grub_menu_t menu, int nested, int *auto_boot)
 	      if (entry >= 0)
 		break;
 	    }
-	  if (key == GRUB_TERM_ESC)
+	  if (grub_key_is_interrupt (key))
 	    {
 	      timeout = -1;
 	      break;
diff --git a/include/grub/term.h b/include/grub/term.h
index 9da03dc751e..3387cb0527c 100644
--- a/include/grub/term.h
+++ b/include/grub/term.h
@@ -330,6 +330,7 @@ void grub_putcode (grub_uint32_t code, struct grub_term_output *term);
 int EXPORT_FUNC(grub_getkey) (void);
 int EXPORT_FUNC(grub_getkey_noblock) (void);
 int EXPORT_FUNC(grub_getkeystatus) (void);
+int EXPORT_FUNC(grub_key_is_interrupt) (int key);
 void grub_cls (void);
 void EXPORT_FUNC(grub_refresh) (void);
 void grub_puts_terminal (const char *str, struct grub_term_output *term);
-- 
2.25.1

Re: [PATCH v3 0/6] Improvements to EFI console and terminal drivers for Flicker Free Boot

[Daniel Kiper]

On Wed, Apr 15, 2020 at 12:26:16PM +0200, Javier Martinez Canillas wrote:
> Hello,
>
> This is a v3 of a patch-set that contains improvements for the EFI console and
> terminal drivers that allows to avoid the switch to text-mode until is needed.
>
> This is part of the patches we have in Fedora to support the Flicker Free Boot
> feature [1], which allows to have a seamless boot experience with no graphical
> transitions that aren't necessary. More info can be found in Hans' blog [2,3].
>
> We have additional patches for this feature, for example template changes that
> makes hiding the menu only when the previous boot was successful, and to make
> GRUB less noisy and not print unnecessary output. But those will be posted in
> the future once the paches on this set land.

If there are no objections I will take this patchset next week.

Daniel