On Wed, Aug 12, 2009 at 06:10:18PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> Colin Watson wrote:
> > This is based on previous work in GRUB Legacy by Kristian and Peter
> > (CCed). Peter tells me that Red Hat has an assignment already on file,
> > and it looks like mine is finally making some progress ...
> 
> We would need RedHat and Kristian to sign some papers.

Peter said they were already done, but if that's the case then fine.
Peter/Kristian, can you double-check this?

> Constants are i386-pc specific. In particular left and right shift are
> distinguished but not ctrl or alt. We should either distinguish them
> all or none.

The values I picked for the constants were convenient for i386-pc, but
that's because it's the only architecture that currently needs an
assembly implementation and it seemed to make sense to me to put the
harder work of transforming values around into C code.

I'm not sure what you mean by "distinguish" here though. I included
constants for control and alt that are currently unused, which may
actually have been a mistake since the original Red Hat patch to GRUB 1
honoured those too in its hiddenmenu handling. Is that what you mean?

> Going to real mode to retrieve keyboard status is absolutely
> unnecessary: same info is available in BDA. Perhaps our console driver
> should switch to BDA altogether but I'm not sure that this will work
> since grub2 disables interrupts (tests needed) but it definitly works
> with keyboard status.

OK, fair enough. The attached updated patch makes this change.

> Driver should return a consistent value even if it doesn't support
> asynchronous query unless driver ignores modifier keys altogether
> (EFI, sigh) or has no way to know if they are pressed unless they are
> pressed together with another key.

I honestly don't see much value in this. Think about what it's being
used for; users are not going to know or guess that Shift only works to
interrupt the boot if you press some non-modifier key too. On the basis
of not including code we aren't going to need, I think it's better to
not attempt to support independent queries of modifier key status on
such drivers at all, and just pretend that no modifiers are pressed.

Thanks,

-- 
Colin Watson                                       [cjwat...@ubuntu.com]
diff -Nur -x '*.orig' -x '*~' grub2-1.96+20090725/commands/sleep.c grub2-1.96+20090725.new/commands/sleep.c
--- grub2-1.96+20090725/commands/sleep.c	2009-03-21 08:39:59.000000000 +0000
+++ grub2-1.96+20090725.new/commands/sleep.c	2009-08-12 18:27:38.000000000 +0100
@@ -43,6 +43,22 @@
   grub_printf ("%d    ", n);
 }
 
+static int
+grub_check_keyboard (void)
+{
+  int mods = grub_keystatus ();
+  if (mods >= 0 &&
+      (mods & (GRUB_TERM_STATUS_LEFT_SHIFT |
+	       GRUB_TERM_STATUS_RIGHT_SHIFT)) != 0)
+    return 1;
+
+  if (grub_checkkey () >= 0 &&
+      GRUB_TERM_ASCII_CHAR (grub_getkey ()) == GRUB_TERM_ESC)
+    return 1;
+
+  return 0;
+}
+
 /* Based on grub_millisleep() from kern/generic/millisleep.c.  */
 static int
 grub_interruptible_millisleep (grub_uint32_t ms)
@@ -52,8 +68,7 @@
   start = grub_get_time_ms ();
 
   while (grub_get_time_ms () - start < ms)
-    if (grub_checkkey () >= 0 &&
-	GRUB_TERM_ASCII_CHAR (grub_getkey ()) == GRUB_TERM_ESC)
+    if (grub_check_keyboard ())
       return 1;
 
   return 0;
@@ -74,7 +89,7 @@
   if (n == 0)
     {
       /* Either `0' or broken input.  */
-      return 0;
+      return grub_check_keyboard ();
     }
 
   xy = grub_getxy ();
diff -Nur -x '*.orig' -x '*~' grub2-1.96+20090725/include/grub/i386/pc/console.h grub2-1.96+20090725.new/include/grub/i386/pc/console.h
--- grub2-1.96+20090725/include/grub/i386/pc/console.h	2008-11-12 17:43:39.000000000 +0000
+++ grub2-1.96+20090725.new/include/grub/i386/pc/console.h	2009-08-12 18:27:38.000000000 +0100
@@ -42,6 +42,7 @@
 /* These are global to share code between C and asm.  */
 int grub_console_checkkey (void);
 int grub_console_getkey (void);
+int grub_console_keystatus (void);
 grub_uint16_t grub_console_getxy (void);
 void grub_console_gotoxy (grub_uint8_t x, grub_uint8_t y);
 void grub_console_cls (void);
diff -Nur -x '*.orig' -x '*~' grub2-1.96+20090725/include/grub/term.h grub2-1.96+20090725.new/include/grub/term.h
--- grub2-1.96+20090725/include/grub/term.h	2009-06-10 22:04:23.000000000 +0100
+++ grub2-1.96+20090725.new/include/grub/term.h	2009-08-12 18:27:38.000000000 +0100
@@ -72,6 +72,13 @@
 #define GRUB_TERM_NEED_INIT	(1 << 16)
 
 
+/* Bitmasks for modifier keys returned by grub_keystatus.  */
+#define GRUB_TERM_STATUS_ALT		(1 << 3)
+#define GRUB_TERM_STATUS_CTRL		(1 << 2)
+#define GRUB_TERM_STATUS_LEFT_SHIFT	(1 << 1)
+#define GRUB_TERM_STATUS_RIGHT_SHIFT	(1 << 0)
+
+
 /* Unicode characters for fancy graphics.  */
 #define GRUB_TERM_DISP_LEFT	0x2190
 #define GRUB_TERM_DISP_UP	0x2191
@@ -157,6 +164,9 @@
 
   /* Get a character.  */
   int (*getkey) (void);
+
+  /* Get keyboard modifier status.  */
+  int (*keystatus) (void);
 };
 typedef struct grub_term_input *grub_term_input_t;
 
@@ -275,6 +285,7 @@
 grub_ssize_t EXPORT_FUNC(grub_getcharwidth) (grub_uint32_t code);
 int EXPORT_FUNC(grub_getkey) (void);
 int EXPORT_FUNC(grub_checkkey) (void);
+int EXPORT_FUNC(grub_keystatus) (void);
 grub_uint16_t EXPORT_FUNC(grub_getwh) (void);
 grub_uint16_t EXPORT_FUNC(grub_getxy) (void);
 void EXPORT_FUNC(grub_gotoxy) (grub_uint8_t x, grub_uint8_t y);
diff -Nur -x '*.orig' -x '*~' grub2-1.96+20090725/kern/i386/pc/startup.S grub2-1.96+20090725.new/kern/i386/pc/startup.S
--- grub2-1.96+20090725/kern/i386/pc/startup.S	2009-08-12 18:27:36.000000000 +0100
+++ grub2-1.96+20090725.new/kern/i386/pc/startup.S	2009-08-12 18:33:21.000000000 +0100
@@ -1285,6 +1285,25 @@
 
 
 /*
+ * int grub_console_keystatus (void)
+ * Return keyboard modifier status in %al:
+ *	bit 3: alt key down
+ *	bit 2: ctrl key down
+ *	bit 1: left shift key down
+ *	bit 0: right shift key down
+ */
+FUNCTION(grub_console_keystatus)
+	/* keyboard status bits in BIOS Data Area */
+	movw	$0x0417, %bx
+	movb	(%bx), %al
+
+	/* Mask out everything but Shift, Ctrl, and Alt. */
+	andl	$0x000f, %eax
+
+	ret
+
+
+/*
  * grub_uint16_t grub_console_getxy (void)
  * BIOS call "INT 10H Function 03h" to get cursor position
  *	Call with	%ah = 0x03
diff -Nur -x '*.orig' -x '*~' grub2-1.96+20090725/kern/term.c grub2-1.96+20090725.new/kern/term.c
--- grub2-1.96+20090725/kern/term.c	2009-06-10 22:04:23.000000000 +0100
+++ grub2-1.96+20090725.new/kern/term.c	2009-08-12 18:27:38.000000000 +0100
@@ -140,6 +140,12 @@
   return (grub_cur_term_input->checkkey) ();
 }
 
+int
+grub_keystatus (void)
+{
+  return (grub_cur_term_input->keystatus) ();
+}
+
 grub_uint16_t
 grub_getxy (void)
 {
diff -Nur -x '*.orig' -x '*~' grub2-1.96+20090725/term/efi/console.c grub2-1.96+20090725.new/term/efi/console.c
--- grub2-1.96+20090725/term/efi/console.c	2009-06-10 22:04:23.000000000 +0100
+++ grub2-1.96+20090725.new/term/efi/console.c	2009-08-12 18:27:38.000000000 +0100
@@ -206,6 +206,13 @@
 }
 
 static int
+grub_console_keystatus (void)
+{
+  /* EFI does not support reading modifier key status.  */
+  return 0;
+}
+
+static int
 grub_console_getkey (void)
 {
   grub_efi_simple_input_interface_t *i;
@@ -337,6 +344,7 @@
     .name = "console",
     .checkkey = grub_console_checkkey,
     .getkey = grub_console_getkey,
+    .keystatus = grub_console_keystatus,
   };
 
 static struct grub_term_output grub_console_term_output =
diff -Nur -x '*.orig' -x '*~' grub2-1.96+20090725/term/i386/pc/at_keyboard.c grub2-1.96+20090725.new/term/i386/pc/at_keyboard.c
--- grub2-1.96+20090725/term/i386/pc/at_keyboard.c	2009-04-14 19:12:14.000000000 +0100
+++ grub2-1.96+20090725.new/term/i386/pc/at_keyboard.c	2009-08-12 18:27:38.000000000 +0100
@@ -200,6 +200,14 @@
   return key;
 }
 
+static int
+grub_at_keyboard_keystatus (void)
+{
+  /* FIXME: I don't know if getting key modifier status is possible without
+   * BIOS help.  */
+  return 0;
+}
+
 static grub_err_t
 grub_keyboard_controller_init (void)
 {
@@ -222,6 +230,7 @@
     .fini = grub_keyboard_controller_fini,
     .checkkey = grub_at_keyboard_checkkey,
     .getkey = grub_at_keyboard_getkey,
+    .keystatus = grub_at_keyboard_keystatus,
   };
 
 GRUB_MOD_INIT(at_keyboard)
diff -Nur -x '*.orig' -x '*~' grub2-1.96+20090725/term/i386/pc/console.c grub2-1.96+20090725.new/term/i386/pc/console.c
--- grub2-1.96+20090725/term/i386/pc/console.c	2009-04-14 19:12:14.000000000 +0100
+++ grub2-1.96+20090725.new/term/i386/pc/console.c	2009-08-12 18:27:38.000000000 +0100
@@ -25,6 +25,7 @@
     .name = "console",
     .checkkey = grub_console_checkkey,
     .getkey = grub_console_getkey,
+    .keystatus = grub_console_keystatus,
   };
 
 static struct grub_term_output grub_console_term_output =
diff -Nur -x '*.orig' -x '*~' grub2-1.96+20090725/term/i386/pc/serial.c grub2-1.96+20090725.new/term/i386/pc/serial.c
--- grub2-1.96+20090725/term/i386/pc/serial.c	2009-06-11 17:17:45.000000000 +0100
+++ grub2-1.96+20090725.new/term/i386/pc/serial.c	2009-08-12 18:27:38.000000000 +0100
@@ -259,6 +259,13 @@
   return c;
 }
 
+static int
+grub_serial_keystatus (void)
+{
+  /* We can't get key modifier status on serial consoles.  */
+  return 0;
+}
+
 /* Initialize a serial device. PORT is the port number for a serial device.
    SPEED is a DTE-DTE speed which must be one of these: 2400, 4800, 9600,
    19200, 38400, 57600 and 115200. WORD_LEN is the word length to be used
@@ -465,6 +472,7 @@
   .name = "serial",
   .checkkey = grub_serial_checkkey,
   .getkey = grub_serial_getkey,
+  .keystatus = grub_serial_keystatus,
 };
 
 static struct grub_term_output grub_serial_term_output =
diff -Nur -x '*.orig' -x '*~' grub2-1.96+20090725/term/ieee1275/ofconsole.c grub2-1.96+20090725.new/term/ieee1275/ofconsole.c
--- grub2-1.96+20090725/term/ieee1275/ofconsole.c	2009-06-10 22:04:23.000000000 +0100
+++ grub2-1.96+20090725.new/term/ieee1275/ofconsole.c	2009-08-12 18:27:38.000000000 +0100
@@ -227,6 +227,13 @@
   return key;
 }
 
+static int
+grub_ofconsole_keystatus (void)
+{
+  /* No attempt to support this for Open Firmware yet. */
+  return 0;
+}
+
 static grub_uint16_t
 grub_ofconsole_getxy (void)
 {
@@ -396,6 +403,7 @@
     .fini = grub_ofconsole_fini,
     .checkkey = grub_ofconsole_checkkey,
     .getkey = grub_ofconsole_getkey,
+    .keystatus = grub_ofconsole_keystatus,
   };
 
 static struct grub_term_output grub_ofconsole_term_output =
diff -Nur -x '*.orig' -x '*~' grub2-1.96+20090725/term/usb_keyboard.c grub2-1.96+20090725.new/term/usb_keyboard.c
--- grub2-1.96+20090725/term/usb_keyboard.c	2009-06-10 22:04:23.000000000 +0100
+++ grub2-1.96+20090725.new/term/usb_keyboard.c	2009-08-12 18:27:38.000000000 +0100
@@ -234,6 +234,47 @@
   return key;
 }
 
+static int
+grub_usb_keyboard_keystatus (void)
+{
+  unsigned char data[8];
+  int mods = 0;
+  int i;
+  grub_err_t err;
+
+  for (i = 0; i < 50; i++)
+    {
+      /* Get_Report.  */
+      err = grub_usb_keyboard_getreport (usbdev, data);
+
+      if (! err)
+	break;
+    }
+
+  if (err)
+    return -1;
+
+  grub_dprintf ("usb_keyboard",
+		"report: 0x%02x 0x%02x 0x%02x 0x%02x"
+		" 0x%02x 0x%02x 0x%02x 0x%02x\n",
+		data[0], data[1], data[2], data[3],
+		data[4], data[5], data[6], data[7]);
+
+  /* Check Shift, Control, and Alt status.  */
+  if (data[0] & 0x02)
+    mods |= GRUB_TERM_STATUS_LEFT_SHIFT;
+  if (data[0] & 0x20)
+    mods |= GRUB_TERM_STATUS_RIGHT_SHIFT;
+  if (data[0] & 0x01 || data[0] & 0x10)
+    mods |= GRUB_TERM_STATUS_CTRL;
+  if (data[0] & 0x04 || data[0] & 0x40)
+    mods |= GRUB_TERM_STATUS_ALT;
+
+  grub_errno = GRUB_ERR_NONE;
+
+  return mods;
+}
+
 static struct grub_term_input grub_usb_keyboard_term =
   {
     .name = "usb_keyboard",
diff -Nur -x '*.orig' -x '*~' grub2-1.96+20090725/util/console.c grub2-1.96+20090725.new/util/console.c
--- grub2-1.96+20090725/util/console.c	2009-07-07 21:03:03.000000000 +0100
+++ grub2-1.96+20090725.new/util/console.c	2009-08-12 18:27:38.000000000 +0100
@@ -258,6 +258,13 @@
   return c;
 }
 
+static int
+grub_ncurses_keystatus (void)
+{
+  /* ncurses can't tell us keyboard modifier status.  */
+  return 0;
+}
+
 static grub_uint16_t
 grub_ncurses_getxy (void)
 {
@@ -350,6 +357,7 @@
     .name = "console",
     .checkkey = grub_ncurses_checkkey,
     .getkey = grub_ncurses_getkey,
+    .keystatus = grub_ncurses_keystatus,
   };
 
 static struct grub_term_output grub_ncurses_term_output =
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to