[PATCH] grub-core/term/efi/console.c: Use EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL if available

[PATCH] grub-core/term/efi/console.c: Use EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL if available

[Josh Triplett]

The extended variant of the protocol supports reading key shift states,
as well as F11 and F12.  This allows more sophisticated line editing
keys to work, as well as the control keys used at the GRUB menu.
---

Written because I use EFI systems regularly, and I'd really like to have
better text editing in the GRUB menu on those systems.

Tested by creating and booting an EFI image, and using control
characters for command-line editing (e.g. Ctrl-u, Ctrl-k, or Ctrl-a).
Transparently falls back to ConIn if LocateProtocol fails, assuming
there exist any EFI systems that don't support
EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL.

Here's an appropriate entry for ChangeLog:

2013-12-28  Josh Triplett  <josh@joshtriplett.org>

	* grub-core/term/efi/console.c: Use EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL
	if available.  The extended variant of the protocol supports reading
	key shift states, as well as F11 and F12.  This allows more
	sophisticated line editing keys to work, as well as the control keys
	used at the GRUB menu.
	(efi_codes): Add F11 and F12.
	(grub_efi_translate_key): New function, factored out of
	grub_console_getkey.
	(grub_console_getkey): Use the Ex protocol if available, and translate
	shift states to corresponding GRUB key modifiers.
	(simple_text_input_ex_guid): New variable.
	(grub_efi_input_init): New function.  Attempt to locate the Ex
	protocol.
	(grub_console_term_input): Reference grub_efi_input_init.
	* include/grub/efi/api.h: Define new functions and structures for
	EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL.


Might also be worth mentioning in NEWS, under "New/improved terminal and
video support":
  * On EFI systems that support EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL, GRUB
    can now use that protocol to support control characters for line
    editing and menus.

 grub-core/term/efi/console.c | 73 ++++++++++++++++++++++++++++++++++----------
 include/grub/efi/api.h       | 54 ++++++++++++++++++++++++++++++++
 2 files changed, 111 insertions(+), 16 deletions(-)

diff --git a/grub-core/term/efi/console.c b/grub-core/term/efi/console.c
index a37eb84..ef05bd6 100644
--- a/grub-core/term/efi/console.c
+++ b/grub-core/term/efi/console.c
@@ -104,26 +104,12 @@ const unsigned efi_codes[] =
     GRUB_TERM_KEY_DC, GRUB_TERM_KEY_PPAGE, GRUB_TERM_KEY_NPAGE, GRUB_TERM_KEY_F1,
     GRUB_TERM_KEY_F2, GRUB_TERM_KEY_F3, GRUB_TERM_KEY_F4, GRUB_TERM_KEY_F5,
     GRUB_TERM_KEY_F6, GRUB_TERM_KEY_F7, GRUB_TERM_KEY_F8, GRUB_TERM_KEY_F9,
-    GRUB_TERM_KEY_F10, 0, 0, '\e'
+    GRUB_TERM_KEY_F10, GRUB_TERM_KEY_F11, GRUB_TERM_KEY_F12, '\e'
   };
 
-
 static int
-grub_console_getkey (struct grub_term_input *term __attribute__ ((unused)))
+grub_efi_translate_key (grub_efi_input_key_t key)
 {
-  grub_efi_simple_input_interface_t *i;
-  grub_efi_input_key_t key;
-  grub_efi_status_t status;
-
-  if (grub_efi_is_finished)
-    return 0;
-
-  i = grub_efi_system_table->con_in;
-  status = efi_call_2 (i->read_key_stroke, i, &key);
-
-  if (status != GRUB_EFI_SUCCESS)
-    return GRUB_TERM_NO_KEY;
-
   if (key.scan_code == 0)
     {
       /* Some firmware implementations use VT100-style codes against the spec.
@@ -142,6 +128,51 @@ grub_console_getkey (struct grub_term_input *term __attribute__ ((unused)))
   return GRUB_TERM_NO_KEY;
 }
 
+static int
+grub_console_getkey (struct grub_term_input *term)
+{
+  grub_efi_simple_input_interface_t *i;
+  grub_efi_input_key_t key;
+  grub_efi_status_t status;
+  int result;
+
+  if (grub_efi_is_finished)
+    return 0;
+
+  if (term->data)
+    {
+      grub_efi_simple_text_input_ex_interface_t *iex;
+      grub_efi_key_data_t key_data;
+      grub_efi_uint32_t shift_state;
+
+      iex = term->data;
+      status = efi_call_2 (iex->read_key_stroke_ex, iex, &key_data);
+      if (status != GRUB_EFI_SUCCESS)
+	return GRUB_TERM_NO_KEY;
+
+      result = grub_efi_translate_key (key_data.key);
+      shift_state = key_data.key_state.key_shift_state;
+      if (result == GRUB_TERM_NO_KEY || !(shift_state & GRUB_EFI_SHIFT_STATE_VALID))
+	return result;
+
+      if (shift_state & (GRUB_EFI_RIGHT_SHIFT_PRESSED | GRUB_EFI_LEFT_SHIFT_PRESSED))
+	result |= GRUB_TERM_SHIFT;
+      if (shift_state & (GRUB_EFI_RIGHT_CONTROL_PRESSED | GRUB_EFI_LEFT_CONTROL_PRESSED))
+	result |= GRUB_TERM_CTRL;
+      if (shift_state & (GRUB_EFI_RIGHT_ALT_PRESSED | GRUB_EFI_LEFT_ALT_PRESSED))
+	result |= GRUB_TERM_ALT;
+      return result;
+    }
+
+  i = grub_efi_system_table->con_in;
+  status = efi_call_2 (i->read_key_stroke, i, &key);
+
+  if (status != GRUB_EFI_SUCCESS)
+    return GRUB_TERM_NO_KEY;
+
+  return grub_efi_translate_key (key);
+}
+
 static struct grub_term_coordinate
 grub_console_getwh (struct grub_term_output *term __attribute__ ((unused)))
 {
@@ -242,6 +273,15 @@ grub_console_setcursor (struct grub_term_output *term __attribute__ ((unused)),
   efi_call_2 (o->enable_cursor, o, on);
 }
 
+static grub_efi_guid_t simple_text_input_ex_guid = EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL_GUID;
+
+static grub_err_t
+grub_efi_input_init (struct grub_term_input *term)
+{
+  term->data = grub_efi_locate_protocol(&simple_text_input_ex_guid, NULL);
+  return 0;
+}
+
 static grub_err_t
 grub_efi_console_init (struct grub_term_output *term)
 {
@@ -261,6 +301,7 @@ grub_efi_console_fini (struct grub_term_output *term)
 static struct grub_term_input grub_console_term_input =
   {
     .name = "console",
+    .init = grub_efi_input_init,
     .getkey = grub_console_getkey,
   };
 
diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
index e5dd543..4e02e88 100644
--- a/include/grub/efi/api.h
+++ b/include/grub/efi/api.h
@@ -71,6 +71,18 @@
 
 #define GRUB_EFI_OPTIONAL_PTR	0x00000001
 
+#define GRUB_EFI_SHIFT_STATE_VALID	0x80000000
+#define GRUB_EFI_RIGHT_SHIFT_PRESSED	0x00000001
+#define GRUB_EFI_LEFT_SHIFT_PRESSED	0x00000002
+#define GRUB_EFI_RIGHT_CONTROL_PRESSED	0x00000004
+#define GRUB_EFI_LEFT_CONTROL_PRESSED	0x00000008
+#define GRUB_EFI_RIGHT_ALT_PRESSED	0x00000010
+#define GRUB_EFI_LEFT_ALT_PRESSED	0x00000020
+#define GRUB_EFI_RIGHT_LOGO_PRESSED	0x00000040
+#define GRUB_EFI_LEFT_LOGO_PRESSED	0x00000080
+#define GRUB_EFI_MENU_KEY_PRESSED	0x00000100
+#define GRUB_EFI_SYS_REQ_PRESSED	0x00000200
+
 #define GRUB_EFI_LOADED_IMAGE_GUID	\
   { 0x5b1b31a1, 0x9562, 0x11d2, \
     { 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b } \
@@ -952,6 +964,20 @@ struct grub_efi_input_key
 };
 typedef struct grub_efi_input_key grub_efi_input_key_t;
 
+struct grub_efi_key_state
+{
+  grub_efi_uint32_t key_shift_state;
+  grub_efi_uint8_t key_toggle_state;
+};
+typedef struct grub_efi_key_state grub_efi_key_state_t;
+
+struct grub_efi_key_data
+{
+  grub_efi_input_key_t key;
+  grub_efi_key_state_t key_state;
+};
+typedef struct grub_efi_key_data grub_efi_key_data_t;
+
 struct grub_efi_simple_text_output_mode
 {
   grub_efi_int32_t max_mode;
@@ -1294,6 +1320,34 @@ struct grub_efi_simple_input_interface
 };
 typedef struct grub_efi_simple_input_interface grub_efi_simple_input_interface_t;
 
+struct grub_efi_simple_text_input_ex_interface
+{
+  grub_efi_status_t
+  (*reset) (struct grub_efi_simple_text_input_ex_interface *this,
+	    grub_efi_boolean_t extended_verification);
+
+  grub_efi_status_t
+  (*read_key_stroke_ex) (struct grub_efi_simple_text_input_ex_interface *this,
+			 grub_efi_key_data_t *key_data);
+
+  grub_efi_event_t wait_for_key_ex;
+
+  grub_efi_status_t
+  (*set_state) (struct grub_efi_simple_text_input_ex_interface *this,
+		grub_uint8_t *key_toggle_state);
+
+  grub_efi_status_t
+  (*register_key_notify) (struct grub_efi_simple_text_input_ex_interface *this,
+			  grub_efi_key_data_t *key_data,
+			  grub_efi_status_t (*key_notification_function)(grub_efi_key_data_t *),
+			  grub_efi_handle_t *notify_handle);
+
+  grub_efi_status_t
+  (*unregister_key_notify) (struct grub_efi_simple_text_input_ex_interface *this,
+			    grub_efi_handle_t notification_handle);
+};
+typedef struct grub_efi_simple_text_input_ex_interface grub_efi_simple_text_input_ex_interface_t;
+
 struct grub_efi_simple_text_output_interface
 {
   grub_efi_status_t
-- 
1.8.5.2

Re: [PATCH] grub-core/term/efi/console.c: Use EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL if available

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 759 bytes --]

On 29.12.2013 08:04, Josh Triplett wrote:
> +  term->data = grub_efi_locate_protocol(&simple_text_input_ex_guid, NULL);
This will find the first handle that provides
EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL, not necessarily the same as normal
console and you may inadvertently switch to another console. You have to
look for protocols on input handle.

> +      if (shift_state & (GRUB_EFI_RIGHT_SHIFT_PRESSED |
GRUB_EFI_LEFT_SHIFT_PRESSED))
> +	result |= GRUB_TERM_SHIFT;
The keys which have natural meaning for shifted version never should
receive GRUB_TERM_SHIFT. E.g. Shift+d should give 'D', not
'd'|GRUB_TERM_SHIFT.
Easiest way is to add GRUB_TERM_SHIFT only if GRUB_TERM_EXTENDED is
already present.
What about providing keystatus function?


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 291 bytes --]

Re: [PATCH] grub-core/term/efi/console.c: Use EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL if available

[Josh Triplett]

On Mon, Dec 30, 2013 at 08:47:25AM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 29.12.2013 08:04, Josh Triplett wrote:
> > +  term->data = grub_efi_locate_protocol(&simple_text_input_ex_guid, NULL);
> This will find the first handle that provides
> EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL, not necessarily the same as normal
> console and you may inadvertently switch to another console. You have to
> look for protocols on input handle.

Will do in v2.

> > +      if (shift_state & (GRUB_EFI_RIGHT_SHIFT_PRESSED |
> GRUB_EFI_LEFT_SHIFT_PRESSED))
> > +	result |= GRUB_TERM_SHIFT;
> The keys which have natural meaning for shifted version never should
> receive GRUB_TERM_SHIFT. E.g. Shift+d should give 'D', not
> 'd'|GRUB_TERM_SHIFT.
> Easiest way is to add GRUB_TERM_SHIFT only if GRUB_TERM_EXTENDED is
> already present.

The EFI interface already does that; the documentation for
EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL explicitly states that it will not
return the shift modifiers for shift+character combinations that compose
into a single Unicode character.  So, shift+'d' will indeed show up as
'D' with shift not set, while shift+left will show up as shift+left.  I
did explicitly test that case; it works exactly as it should.

Also, filtering on GRUB_TERM_EXTENDED wouldn't give the right results
for things like shift+space, shift+tab, or shift+backspace, since the
corresponding unshifted characters don't have GRUB_TERM_EXTENDED set.

> What about providing keystatus function?

The protocol doesn't have a function to return the current modifier
state, only to return a character with accompanying modifiers.

- Josh Triplett

[PATCHv2] grub-core/term/efi/console.c: Use EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL if available

[Josh Triplett]

The extended variant of the protocol supports reading key shift states,
as well as F11 and F12.  This allows more sophisticated line editing
keys to work, as well as the control keys used at the GRUB menu.
---

v2: Use OpenProtocol on ConIn, rather than LocateProtocol, to ensure
that the instance of EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL matches the
existing console.

Tested by creating and booting an EFI image, and using control
characters for command-line editing (e.g. Ctrl-u, Ctrl-k, or Ctrl-a).
Transparently falls back to ConIn if OpenProtocol fails, assuming there
exist any EFI systems that don't support
EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL.

Here's an appropriate entry for ChangeLog:

2013-12-28  Josh Triplett  <josh@joshtriplett.org>

	* grub-core/term/efi/console.c: Use EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL
	if available.  The extended variant of the protocol supports reading
	key shift states, as well as F11 and F12.  This allows more
	sophisticated line editing keys to work, as well as the control keys
	used at the GRUB menu.
	(efi_codes): Add F11 and F12.
	(grub_efi_translate_key): New function, factored out of
	grub_console_getkey.
	(grub_console_getkey): Use the Ex protocol if available, and translate
	shift states to corresponding GRUB key modifiers.
	(simple_text_input_ex_guid): New variable.
	(grub_efi_input_init): New function.  Attempt to open the Ex protocol.
	(grub_console_term_input): Reference grub_efi_input_init.
	* include/grub/efi/api.h: Define new functions and structures for
	EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL.


Might also be worth mentioning in NEWS, under "New/improved terminal and
video support":
  * On EFI systems that support EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL, GRUB
    can now use that protocol to support control characters for line
    editing and menus.

 grub-core/term/efi/console.c | 75 ++++++++++++++++++++++++++++++++++----------
 include/grub/efi/api.h       | 54 +++++++++++++++++++++++++++++++
 2 files changed, 113 insertions(+), 16 deletions(-)

diff --git a/grub-core/term/efi/console.c b/grub-core/term/efi/console.c
index a37eb84..88caaa8 100644
--- a/grub-core/term/efi/console.c
+++ b/grub-core/term/efi/console.c
@@ -104,26 +104,12 @@ const unsigned efi_codes[] =
     GRUB_TERM_KEY_DC, GRUB_TERM_KEY_PPAGE, GRUB_TERM_KEY_NPAGE, GRUB_TERM_KEY_F1,
     GRUB_TERM_KEY_F2, GRUB_TERM_KEY_F3, GRUB_TERM_KEY_F4, GRUB_TERM_KEY_F5,
     GRUB_TERM_KEY_F6, GRUB_TERM_KEY_F7, GRUB_TERM_KEY_F8, GRUB_TERM_KEY_F9,
-    GRUB_TERM_KEY_F10, 0, 0, '\e'
+    GRUB_TERM_KEY_F10, GRUB_TERM_KEY_F11, GRUB_TERM_KEY_F12, '\e'
   };
 
-
 static int
-grub_console_getkey (struct grub_term_input *term __attribute__ ((unused)))
+grub_efi_translate_key (grub_efi_input_key_t key)
 {
-  grub_efi_simple_input_interface_t *i;
-  grub_efi_input_key_t key;
-  grub_efi_status_t status;
-
-  if (grub_efi_is_finished)
-    return 0;
-
-  i = grub_efi_system_table->con_in;
-  status = efi_call_2 (i->read_key_stroke, i, &key);
-
-  if (status != GRUB_EFI_SUCCESS)
-    return GRUB_TERM_NO_KEY;
-
   if (key.scan_code == 0)
     {
       /* Some firmware implementations use VT100-style codes against the spec.
@@ -142,6 +128,51 @@ grub_console_getkey (struct grub_term_input *term __attribute__ ((unused)))
   return GRUB_TERM_NO_KEY;
 }
 
+static int
+grub_console_getkey (struct grub_term_input *term)
+{
+  grub_efi_simple_input_interface_t *i;
+  grub_efi_input_key_t key;
+  grub_efi_status_t status;
+  int result;
+
+  if (grub_efi_is_finished)
+    return 0;
+
+  if (term->data)
+    {
+      grub_efi_simple_text_input_ex_interface_t *iex;
+      grub_efi_key_data_t key_data;
+      grub_efi_uint32_t shift_state;
+
+      iex = term->data;
+      status = efi_call_2 (iex->read_key_stroke_ex, iex, &key_data);
+      if (status != GRUB_EFI_SUCCESS)
+	return GRUB_TERM_NO_KEY;
+
+      result = grub_efi_translate_key (key_data.key);
+      shift_state = key_data.key_state.key_shift_state;
+      if (result == GRUB_TERM_NO_KEY || !(shift_state & GRUB_EFI_SHIFT_STATE_VALID))
+	return result;
+
+      if (shift_state & (GRUB_EFI_RIGHT_SHIFT_PRESSED | GRUB_EFI_LEFT_SHIFT_PRESSED))
+	result |= GRUB_TERM_SHIFT;
+      if (shift_state & (GRUB_EFI_RIGHT_CONTROL_PRESSED | GRUB_EFI_LEFT_CONTROL_PRESSED))
+	result |= GRUB_TERM_CTRL;
+      if (shift_state & (GRUB_EFI_RIGHT_ALT_PRESSED | GRUB_EFI_LEFT_ALT_PRESSED))
+	result |= GRUB_TERM_ALT;
+      return result;
+    }
+
+  i = grub_efi_system_table->con_in;
+  status = efi_call_2 (i->read_key_stroke, i, &key);
+
+  if (status != GRUB_EFI_SUCCESS)
+    return GRUB_TERM_NO_KEY;
+
+  return grub_efi_translate_key (key);
+}
+
 static struct grub_term_coordinate
 grub_console_getwh (struct grub_term_output *term __attribute__ ((unused)))
 {
@@ -242,6 +273,17 @@ grub_console_setcursor (struct grub_term_output *term __attribute__ ((unused)),
   efi_call_2 (o->enable_cursor, o, on);
 }
 
+static grub_efi_guid_t simple_text_input_ex_guid = EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL_GUID;
+
+static grub_err_t
+grub_efi_input_init (struct grub_term_input *term)
+{
+  term->data = grub_efi_open_protocol(grub_efi_system_table->console_in_handler,
+				      &simple_text_input_ex_guid,
+				      GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+  return 0;
+}
+
 static grub_err_t
 grub_efi_console_init (struct grub_term_output *term)
 {
@@ -261,6 +303,7 @@ grub_efi_console_fini (struct grub_term_output *term)
 static struct grub_term_input grub_console_term_input =
   {
     .name = "console",
+    .init = grub_efi_input_init,
     .getkey = grub_console_getkey,
   };
 
diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
index e5dd543..4e02e88 100644
--- a/include/grub/efi/api.h
+++ b/include/grub/efi/api.h
@@ -71,6 +71,18 @@
 
 #define GRUB_EFI_OPTIONAL_PTR	0x00000001
 
+#define GRUB_EFI_SHIFT_STATE_VALID	0x80000000
+#define GRUB_EFI_RIGHT_SHIFT_PRESSED	0x00000001
+#define GRUB_EFI_LEFT_SHIFT_PRESSED	0x00000002
+#define GRUB_EFI_RIGHT_CONTROL_PRESSED	0x00000004
+#define GRUB_EFI_LEFT_CONTROL_PRESSED	0x00000008
+#define GRUB_EFI_RIGHT_ALT_PRESSED	0x00000010
+#define GRUB_EFI_LEFT_ALT_PRESSED	0x00000020
+#define GRUB_EFI_RIGHT_LOGO_PRESSED	0x00000040
+#define GRUB_EFI_LEFT_LOGO_PRESSED	0x00000080
+#define GRUB_EFI_MENU_KEY_PRESSED	0x00000100
+#define GRUB_EFI_SYS_REQ_PRESSED	0x00000200
+
 #define GRUB_EFI_LOADED_IMAGE_GUID	\
   { 0x5b1b31a1, 0x9562, 0x11d2, \
     { 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b } \
@@ -952,6 +964,20 @@ struct grub_efi_input_key
 };
 typedef struct grub_efi_input_key grub_efi_input_key_t;
 
+struct grub_efi_key_state
+{
+  grub_efi_uint32_t key_shift_state;
+  grub_efi_uint8_t key_toggle_state;
+};
+typedef struct grub_efi_key_state grub_efi_key_state_t;
+
+struct grub_efi_key_data
+{
+  grub_efi_input_key_t key;
+  grub_efi_key_state_t key_state;
+};
+typedef struct grub_efi_key_data grub_efi_key_data_t;
+
 struct grub_efi_simple_text_output_mode
 {
   grub_efi_int32_t max_mode;
@@ -1294,6 +1320,34 @@ struct grub_efi_simple_input_interface
 };
 typedef struct grub_efi_simple_input_interface grub_efi_simple_input_interface_t;
 
+struct grub_efi_simple_text_input_ex_interface
+{
+  grub_efi_status_t
+  (*reset) (struct grub_efi_simple_text_input_ex_interface *this,
+	    grub_efi_boolean_t extended_verification);
+
+  grub_efi_status_t
+  (*read_key_stroke_ex) (struct grub_efi_simple_text_input_ex_interface *this,
+			 grub_efi_key_data_t *key_data);
+
+  grub_efi_event_t wait_for_key_ex;
+
+  grub_efi_status_t
+  (*set_state) (struct grub_efi_simple_text_input_ex_interface *this,
+		grub_uint8_t *key_toggle_state);
+
+  grub_efi_status_t
+  (*register_key_notify) (struct grub_efi_simple_text_input_ex_interface *this,
+			  grub_efi_key_data_t *key_data,
+			  grub_efi_status_t (*key_notification_function)(grub_efi_key_data_t *),
+			  grub_efi_handle_t *notify_handle);
+
+  grub_efi_status_t
+  (*unregister_key_notify) (struct grub_efi_simple_text_input_ex_interface *this,
+			    grub_efi_handle_t notification_handle);
+};
+typedef struct grub_efi_simple_text_input_ex_interface grub_efi_simple_text_input_ex_interface_t;
+
 struct grub_efi_simple_text_output_interface
 {
   grub_efi_status_t
-- 
1.8.5.2