Detect key modifier status in 'sleep --interruptible'
PATCH 1/1 Detect key modifier status in 'sleep --interruptible'
The attached patch arranges to make use of key modifier status in 'sleep --interruptible', so that you can interrupt the sleep by pressing Shift. The benefit of this is that on some architectures it makes it possible and even sometimes reasonable to use an extremely short sleep for a hiddenmenu-type arrangement - even a zero sleep - because on some architectures you can query key modifier state asynchronously without having to wait for make scancodes by way of key repeat or whatever, so all the user needs to do is hold Shift as they come out of the BIOS. While a zero sleep probably isn't a reasonable default for GRUB in general, it's very useful if you're building single-boot out-of-the-box systems where you just want to get on with booting the one installed OS.
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 ...
Thanks,
Signature · 1 line
Colin Watson [[email protected]]
commands/sleep.c +18 -3
@@ -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 ();
include/grub/i386/pc/console.h +1 -0
@@ -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 );
include/grub/term.h +11 -0
@@ -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);
kern/i386/pc/startup.S +32 -0
@@ -1285,6 +1285,38 @@ /* + * int grub_console_keystatus (void) + * BIOS call "INT 16H Function 12H" to get keyboard modifier status + * Call with %ah = 0x12 + * Return: %al = keyboard state: + * 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) + pushl %ebp + + call prot_to_real /* enter real mode */ + .code16 + + movb $0x12, %ah + int $0x16 + movw %ax, %dx + + DATA32 call real_to_prot + .code32 + + movw %dx, %ax + + /* Mask out numlock, capslock and insert state. */ + andl $0x0f0f, %eax + + popl %ebp + ret + + +/* * grub_uint16_t grub_console_getxy (void) * BIOS call "INT 10H Function 03h" to get cursor position * Call with %ah = 0x03
kern/term.c +6 -0
@@ -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 ) {
term/efi/console.c +8 -0
@@ -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 =
term/i386/pc/at_keyboard.c +9 -0
@@ -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)
term/i386/pc/console.c +1 -0
@@ -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 =
term/i386/pc/serial.c +8 -0
@@ -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 =
term/ieee1275/ofconsole.c +8 -0
@@ -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 =
term/usb_keyboard.c +41 -0
47 lines not on this page · raw message
util/console.c +8 -0
@@ -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 =
Quoted 3 lines
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.
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.
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. 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. In the case of at_keyboard it specifically means to return at_keyboard_status transformed to correct format.
Quoted 12 lines
Thanks,
--
Colin Watson [[email protected]]_______________________________________________
Grub-devel mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/grub-devel
Signature · 4 lines
Regards Vladimir 'phcoder' Serbinenko Personal git repository: http://repo.or.cz/w/grub2/phcoder.git
Quoted 8 lines · Vladimir 'phcoder' Serbinenko
Quoted 3 lines
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.
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.
Another possible problem is about configuration - configuration tool has to know if 0 timeout is permissible. It depends on both platform and used driver. But since this probably won't ever be a default config for mainline it's not our headache ;)
Signature · 4 lines
Regards Vladimir 'phcoder' Serbinenko Personal git repository: http://repo.or.cz/w/grub2/phcoder.git
Quoted 6 lines · Vladimir 'phcoder' Serbinenko
Quoted 3 lines · Colin Watson
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?
Quoted 3 lines
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?
Quoted 5 lines
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.
Quoted 4 lines
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,
Signature · 1 line
Colin Watson [[email protected]]
commands/sleep.c +18 -3
@@ -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 ();
include/grub/i386/pc/console.h +1 -0
@@ -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 );
include/grub/term.h +11 -0
@@ -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);
kern/i386/pc/startup.S +19 -0
@@ -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
kern/term.c +6 -0
@@ -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 ) {
term/efi/console.c +8 -0
@@ -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 =
term/i386/pc/at_keyboard.c +9 -0
@@ -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)
term/i386/pc/console.c +1 -0
@@ -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 =
term/i386/pc/serial.c +8 -0
@@ -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 =
term/ieee1275/ofconsole.c +8 -0
@@ -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 =
term/usb_keyboard.c +41 -0
47 lines not on this page · raw message
util/console.c +8 -0
@@ -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 =
Quoted 10 lines · Colin Watson
Quoted 6 lines · Vladimir 'phcoder' Serbinenko
Quoted 3 lines · Colin Watson
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?
We don't have any assignment from any Kristian, nor Red Hat, but we do have one from a Peter.
Which Peter were you talking about? I don't see him in CC.
Signature · 5 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
Quoted 9 lines · Robert Millan
Quoted 8 lines · Colin Watson
Quoted 6 lines · Vladimir 'phcoder' Serbinenko
Quoted 3 lines · Colin Watson
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.
Quoted 7 lines
Quoted 2 lines
Peter said they were already done, but if that's the case then fine. Peter/Kristian, can you double-check this?
We don't have any assignment from any Kristian, nor Red Hat, but we do have one from a Peter.
Which Peter were you talking about? I don't see him in CC.
Peter Jones, pjones at RH. I definitely CCed him in my mail but the mailing list seems to have eaten the CC for some reason.
The CVS history suggests that Kristian was the original author, though.
Signature · 1 line
Colin Watson [[email protected]]
Quoted 9 lines
Quoted 3 lines
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.
Quoted 4 lines
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?
I mean that upper layer code knows whether it was left or right shift but it doesn't know whether it was left or right alt.
Quoted 9 lines
Quoted 6 lines
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.
Why is it still asm? It can be done easily in C.
Actually here we have 2 extremes: nice code (C, transform to nice flags (only one shift or 2 conrols and alts)) or small code (asm, crude value). Where we choose the compromise is a question for maintainer, not me.
Another possibility is to put this code outside of kernel altogether. It saves bytes but may make approach ugly.
Signature · 4 lines
Regards Vladimir 'phcoder' Serbinenko Personal git repository: http://repo.or.cz/w/grub2/phcoder.git
Quoted 5 lines · Vladimir 'phcoder' Serbinenko
Why is it still asm? It can be done easily in C. Actually here we have 2 extremes: nice code (C, transform to nice flags (only one shift or 2 conrols and alts)) or small code (asm, crude value). Where we choose the compromise is a question for maintainer, not me.
Nice code, please. We're pressed for space, but not *that* much.
Besides, we can't use this asm code as-is, unless we get word from the people who wrote it (Colin wrote most of the patch, but not this).
Signature · 5 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
Quoted 7 lines · Robert Millan
Quoted 5 lines · Vladimir 'phcoder' Serbinenko
Why is it still asm? It can be done easily in C. Actually here we have 2 extremes: nice code (C, transform to nice flags (only one shift or 2 conrols and alts)) or small code (asm, crude value). Where we choose the compromise is a question for maintainer, not me.
Quoted 5 lines
Nice code, please. We're pressed for space, but not *that* much.
Besides, we can't use this asm code as-is, unless we get word from the people who wrote it (Colin wrote most of the patch, but not this).
In this case we don't need asm at all.
Quoted 13 lines
--
Robert MillanThe DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
_______________________________________________
Grub-devel mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/grub-devel
Signature · 4 lines
Regards Vladimir 'phcoder' Serbinenko Personal git repository: http://repo.or.cz/w/grub2/phcoder.git
Quoted 8 lines · Vladimir 'phcoder' Serbinenko
Quoted 7 lines · Robert Millan
Quoted 5 lines · Vladimir 'phcoder' Serbinenko
Why is it still asm? It can be done easily in C. Actually here we have 2 extremes: nice code (C, transform to nice flags (only one shift or 2 conrols and alts)) or small code (asm, crude value). Where we choose the compromise is a question for maintainer, not me.
Quoted 6 lines
Quoted 5 lines
Nice code, please. We're pressed for space, but not *that* much.
Besides, we can't use this asm code as-is, unless we get word from the people who wrote it (Colin wrote most of the patch, but not this).
In this case we don't need asm at all.
I didn't look much into this. I thought there was a new BIOS call being introduced?
Signature · 5 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
Quoted 11 lines · Robert Millan
Quoted 10 lines · Vladimir 'phcoder' Serbinenko
Quoted 9 lines · Robert Millan
Quoted 5 lines · Vladimir 'phcoder' Serbinenko
Why is it still asm? It can be done easily in C. Actually here we have 2 extremes: nice code (C, transform to nice flags (only one shift or 2 conrols and alts)) or small code (asm, crude value). Where we choose the compromise is a question for maintainer, not me.
Nice code, please. We're pressed for space, but not *that* much.
Quoted 8 lines
Quoted 4 lines
Quoted 3 lines
Besides, we can't use this asm code as-is, unless we get word from the people who wrote it (Colin wrote most of the patch, but not this).
In this case we don't need asm at all.
I didn't look much into this. I thought there was a new BIOS call being introduced?
In first version of patch yes but in the second version just checks a value in memory
Signature · 4 lines
Regards Vladimir 'phcoder' Serbinenko Personal git repository: http://repo.or.cz/w/grub2/phcoder.git
Quoted 3 lines · Robert Millan
Besides, we can't use this asm code as-is, unless we get word from the people who wrote it (Colin wrote most of the patch, but not this).
I've been pointed out that Red Hat has a blanket copyright assignment contract that covers all FSF software.
Therefore this is not a problem at all (though, we've rewritten it in C anyway for technical reasons).
Signature · 5 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
On Mon, Aug 24, 2009 at 12:46:29AM +0200, Vladimir 'phcoder' Serbinenko wrote:
[Could you please preserve attributions in your replies? I've reinserted one here so that it's clear who wrote what.]
Quoted 6 lines
Quoted 5 lines · Colin Watson
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.
Quoted 7 lines
Quoted 4 lines
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?
I mean that upper layer code knows whether it was left or right shift but it doesn't know whether it was left or right alt.
Oh, I see. There's no real need to distinguish here, so the attached patch removes the distinction. Note that this should NOT be applied as-is, as I have not updated the assembly implementation for this; Robert is going to clean-room this in C (which should take all of a minute or so :-) ), and I hope he can deal with the bit-banging at the same time.
Thanks,
Signature · 1 line
Colin Watson [[email protected]]
commands/sleep.c +16 -3
@@ -43,6 +43,20 @@grub_printf ("%d " , n); } +static int +grub_check_keyboard (void ) +{ +int mods =grub_keystatus (); +if (mods >=0 && (mods &GRUB_TERM_STATUS_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 +66,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 +87,7 @@if (n ==0 ) {/* Either `0' or broken input. */ -return 0 ; +return grub_check_keyboard (); } xy =grub_getxy ();
include/grub/i386/pc/console.h +1 -0
@@ -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 );
include/grub/term.h +10 -0
@@ -72,6 +72,12 @@#define GRUB_TERM_NEED_INIT (1 << 16) +/* Bitmasks for modifier keys returned by grub_keystatus. */ +#define GRUB_TERM_STATUS_SHIFT (1 << 0) +#define GRUB_TERM_STATUS_CTRL (1 << 1) +#define GRUB_TERM_STATUS_ALT (1 << 2) + +/* Unicode characters for fancy graphics. */ #define GRUB_TERM_DISP_LEFT 0x2190#define GRUB_TERM_DISP_UP 0x2191
@@ -157,6 +163,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 +284,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);
kern/i386/pc/startup.S +19 -0
@@ -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
kern/term.c +9 -0
@@ -140,6 +140,15 @@return (grub_cur_term_input->checkkey) (); } +int +grub_keystatus (void ) +{ +if (grub_cur_term_input->keystatus) +return (grub_cur_term_input->keystatus) (); +else +return 0 ; +} +grub_uint16_t grub_getxy (void ) {
term/efi/console.c +2 -0
@@ -337,6 +337,8 @@ .name = "console", .checkkey = grub_console_checkkey, .getkey = grub_console_getkey, + /* EFI does not support reading modifier key status. */ + .keystatus = NULL, }; static struct grub_term_output grub_console_term_output =
term/i386/pc/at_keyboard.c +3 -0
@@ -222,6 +222,9 @@ .fini = grub_keyboard_controller_fini, .checkkey = grub_at_keyboard_checkkey, .getkey = grub_at_keyboard_getkey, +/* Getting key modifier status is not possible without BIOS help; all we +* can do is spot when keys are pressed. */ + .keystatus = NULL, };GRUB_MOD_INIT (at_keyboard)
term/i386/pc/console.c +1 -0
@@ -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 =
term/i386/pc/serial.c +2 -0
@@ -465,6 +465,8 @@ .name = "serial", .checkkey = grub_serial_checkkey, .getkey = grub_serial_getkey, + /* We can't get key modifier status on serial consoles. */ + .keystatus = NULL, }; static struct grub_term_output grub_serial_term_output =
term/ieee1275/ofconsole.c +2 -0
@@ -396,6 +396,8 @@ .fini = grub_ofconsole_fini, .checkkey = grub_ofconsole_checkkey, .getkey = grub_ofconsole_getkey, + /* No attempt to support this for Open Firmware yet. */ + .keystatus = NULL, }; static struct grub_term_output grub_ofconsole_term_output =
term/usb_keyboard.c +40 -0
@@ -234,11 +234,51 @@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 || data[0 ] &0x20 ) + mods |=GRUB_TERM_STATUS_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" , .checkkey = grub_usb_keyboard_checkkey, .getkey = grub_usb_keyboard_getkey, + .keystatus = grub_usb_keyboard_keystatus, .next =0 };
util/console.c +2 -0
@@ -350,6 +350,8 @@ .name = "console", .checkkey = grub_ncurses_checkkey, .getkey = grub_ncurses_getkey, + /* ncurses can't tell us keyboard modifier status. */ + .keystatus = NULL, }; static struct grub_term_output grub_ncurses_term_output =
Quoted 19 lines · Colin Watson
On Mon, Aug 24, 2009 at 12:46:29AM +0200, Vladimir 'phcoder' Serbinenko wrote:
[Could you please preserve attributions in your replies? I've reinserted one here so that it's clear who wrote what.]
Quoted 6 lines
Quoted 5 lines · Colin Watson
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.
Quoted 7 lines
Quoted 4 lines
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?
I mean that upper layer code knows whether it was left or right shift but it doesn't know whether it was left or right alt.
Quoted 6 lines
Oh, I see. There's no real need to distinguish here, so the attached patch removes the distinction. Note that this should NOT be applied as-is, as I have not updated the assembly implementation for this; Robert is going to clean-room this in C (which should take all of a minute or so :-) ), and I hope he can deal with the bit-banging at the same time.
Hi,
I don't have time to test it, but what we want is roughly something like this. If you would please test and confirm it's working the way you expected, it can be committed.
Also, please remember to include a ChangeLog entry.
I ommitted the USB part of your patch, because I had some comments. Once that's fixed it can be added as well:
Review
Quoted 4 lines
(quoted diff) +4
-0+static int +grub_usb_keyboard_keystatus (void) +{ + unsigned char data[8];
Please use grub_uint8_t.
Review
Quoted 8 lines
(quoted diff) +8
-0+ for (i = 0; i < 50; i++) + { + /* Get_Report. */ + err = grub_usb_keyboard_getreport (usbdev, data); + + if (! err) + break; + }
If the 50 is a "timeout", it should be using grub_get_time_ms() instead; if it's a number given by spec (e.g. number of times an I/O operation must be performed), then please macroify it to indicate what it is.
Signature · 5 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
term.c +9 -0
@@ -140,6 +140,15 @@ grub_checkkey (void)return (grub_cur_term_input->checkkey) (); } +int +grub_getkeystatus (void ) +{ +if (grub_cur_term_input->getkeystatus) +return (grub_cur_term_input->getkeystatus) (); +else +return 0 ; +} +grub_uint16_t grub_getxy (void ) {
grub/term.h +11 -1
@@ -1,6 +1,6 @@/* * GRUB -- GRand Unified Bootloader -* Copyright (C) 2002,2003,2005,2007,2008 Free Software Foundation, Inc. +* Copyright (C) 2002,2003,2005,2007,2008,2009 Free Software Foundation, Inc. * * GRUB is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by
@@ -72,6 +72,12 @@ grub_term_color_state;#define GRUB_TERM_NEED_INIT (1 << 16) +/* Bitmasks for modifier keys returned by grub_getkeystatus. */ +#define GRUB_TERM_STATUS_SHIFT (1 << 0) +#define GRUB_TERM_STATUS_CTRL (1 << 1) +#define GRUB_TERM_STATUS_ALT (1 << 2) + +/* Unicode characters for fancy graphics. */ #define GRUB_TERM_DISP_LEFT 0x2190#define GRUB_TERM_DISP_UP 0x2191
@@ -157,6 +163,9 @@ struct grub_term_input/* Get a character. */ int (*getkey) (void ); + +/* Get keyboard modifier status. */ +int (*getkeystatus) (void ); };typedef struct grub_term_input *grub_term_input_t ;
@@ -275,6 +284,7 @@ void EXPORT_FUNC(grub_putcode) (grub_uingrub_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_getkeystatus ) (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);
grub/i386/pc/console.h +1 -0
@@ -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_getkeystatus (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 );
grub/i386/pc/memory.h +9 -0
@@ -78,6 +78,15 @@/* The data segment of the pseudo real mode. */ #define GRUB_MEMORY_MACHINE_PSEUDO_REAL_DSEG 0x20 +#define GRUB_MEMORY_MACHINE_BIOS_DATA_AREA_ADDR 0x400 + +static struct grub_machine_bios_data_area +{ + grub_uint8_t unused1[0x17]; + grub_uint16_t keyboard_status; /* 0x17 */ + grub_uint8_t unused2[0xf0 - 0x19]; +} +#ifndef ASM_FILE struct grub_machine_mmap_entry
sleep.c +17 -4
@@ -1,7 +1,7 @@/* sleep.c - Command to wait a specified number of seconds. */ /* * GRUB -- GRand Unified Bootloader -* Copyright (C) 2006,2007,2008 Free Software Foundation, Inc. +* Copyright (C) 2006,2007,2008,2009 Free Software Foundation, Inc. * * GRUB is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by
@@ -43,6 +43,20 @@ do_print (int n)grub_printf ("%d " , n); } +static int +grub_check_keyboard (void ) +{ +int mods =grub_getkeystatus (); +if (mods >=0 && (mods &GRUB_TERM_STATUS_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 +66,7 @@ grub_interruptible_millisleep (grub_uint 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 +87,7 @@ grub_cmd_sleep (grub_extcmd_t cmd, int aif (n ==0 ) {/* Either `0' or broken input. */ -return 0 ; +return grub_check_keyboard (); } xy =grub_getxy ();
i386/pc/console.c +11 -1
@@ -1,6 +1,6 @@/* * GRUB -- GRand Unified Bootloader -* Copyright (C) 2002,2003,2005,2007,2008 Free Software Foundation, Inc. +* Copyright (C) 2002,2003,2005,2007,2008,2009 Free Software Foundation, Inc. * * GRUB is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by
@@ -16,15 +16,25 @@* along with GRUB. If not, see <http://www.gnu.org/licenses/>. */ +#include <grub/machine/memory.h> #include <grub/machine/console.h> #include <grub/term.h> #include <grub/types.h> +static const struct grub_machine_bios_data_area *bios_data_area =GRUB_MEMORY_MACHINE_BIOS_DATA_AREA_ADDR ; + +static grub_uint16_t +grub_console_getkeystatus (void ) +{ +return bios_data_area->keyboard_status; +} +static struct grub_term_input grub_console_term_input = { .name ="console" , .checkkey = grub_console_checkkey, .getkey = grub_console_getkey, + .getkeystatus = grub_console_getkeystatus, }; static struct grub_term_output grub_console_term_output =
Quoted 7 lines · Robert Millan
Index:
include/grub/i386/pc/memory.h
===================================================================grub/i386/pc/memory.h +0
-0@@ -78,6 +78,15 @@/* The data segment of the pseudo real mode. */ #define GRUB_MEMORY_MACHINE_PSEUDO_REAL_DSEG 0x20
grub/i386/pc/memory.h line 89
Quoted 10 lines
grub/i386/pc/memory.h +9
-0+#define GRUB_MEMORY_MACHINE_BIOS_DATA_AREA_ADDR 0x400 + +static struct grub_machine_bios_data_area +{ +grub_uint8_t unused1[0x17 ]; +grub_uint16_t keyboard_status;/* 0x17 */ +grub_uint8_t unused2[0xf0 -0x19 ]; +} +
Btw, Colin pointed at this URL:
http://heim.ifi.uio.no/~stanisls/helppc/bios_data_area.html
I suppose it'd be a good idea to include it in a comment next to the struct definition.
Signature · 5 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
Quoted 10 lines · Robert Millan
I ommitted the USB part of your patch, because I had some comments. Once that's fixed it can be added as well:
Review
Quoted 4 lines
(quoted diff) +4
-0+static int +grub_usb_keyboard_keystatus (void) +{ + unsigned char data[8];Please use
grub_uint8_t.
Quoted 12 lines
Review
Quoted 8 lines
(quoted diff) +8
-0+ for (i = 0; i < 50; i++) + { + /* Get_Report. */ + err = grub_usb_keyboard_getreport (usbdev, data); + + if (! err) + break; + }If the 50 is a "timeout", it should be using
grub_get_time_ms()instead; if it's a number given by spec (e.g. number of times an I/O operation must be performed), then please macroify it to indicate what it is.
Perhaps we can apply the following patch first, then? I was following existing style, so the other code should be updated too.
I don't see anything in the USB HID spec about the number of times one must attempt to get a keyboard report, so I think it must simply be a timeout. USB frames are 1ms so waiting for 50ms should do.
2009-08-24 Colin Watson <[email protected]>
* term/usb_keyboard.c (grub_usb_keyboard_getreport): Make report' grub_uint8_t *. (grub_usb_keyboard_checkkey): Make data' elements grub_uint8_t. Use a 50-millisecond timeout rather than just repeating grub_usb_keyboard_getreport 50 times. (grub_usb_keyboard_getkey): Make `data' elements grub_uint8_t.
usb_keyboard.c +10 -6
@@ -97,7 +97,7 @@ }static grub_err_t -grub_usb_keyboard_getreport (grub_usb_device_t dev,unsigned char *report) +grub_usb_keyboard_getreport (grub_usb_device_t dev,grub_uint8_t *report) {return grub_usb_control_msg (dev, (1 <<7 ) | (1 <<5 ) |1 ,0x01 ,0 ,0 ,8 , (char *) report);
@@ -108,20 +108,24 @@static int grub_usb_keyboard_checkkey (void ) { -unsigned char data[8 ]; +grub_uint8_t data[8 ];int key; -int i;grub_err_t err; +grub_uint64_t currtime; +int timeout =50 ; data[2 ] =0 ; -for (i =0 ; i <50 ; i++) + currtime =grub_get_time_ms (); +do {/* Get_Report. */ err =grub_usb_keyboard_getreport (usbdev, data); -if (! err && data[2 ]) +/* Implement a timeout. */ +if (grub_get_time_ms () > currtime + timeout)break ; } +while (err || !data[2 ]);if (err || !data[2 ])return -1 ;
@@ -174,7 +178,7 @@ {int key;grub_err_t err; -unsigned char data[8 ]; +grub_uint8_t data[8 ];grub_uint64_t currtime;int timeout;static grub_usb_keyboard_repeat_t repeat =GRUB_HIDBOOT_REPEAT_NONE ;
Signature · 1 line
Colin Watson [[email protected]]
Quoted 3 lines · Colin Watson
Perhaps we can apply the following patch first, then? I was following existing style, so the other code should be updated too.
Ah, sorry I didn't know that. But thanks for fixing anyway :-)
Please go ahead (I assume you've tested it, at least with QEMU's USB keyboard).
Signature · 5 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
Quoted 2 lines · Robert Millan
Please go ahead (I assume you've tested it, at least with QEMU's USB keyboard).
Hmm. Does GRUB actually work with that right now? Even without my patch applied, this just gives me a prompt I can't type at:
./grub-mkrescue --modules='uhci usb_keyboard' --pkglibdir=. --grub-mkimage=./grub-mkimage ../t/disk qemu -boot d -cdrom ../t/disk -usb -usbdevice keyboard
Signature · 1 line
Colin Watson [[email protected]]
Quoted 9 lines · Colin Watson
Quoted 2 lines · Robert Millan
Please go ahead (I assume you've tested it, at least with QEMU's USB keyboard).
Hmm. Does GRUB actually work with that right now? Even without my patch applied, this just gives me a prompt I can't type at:
./grub-mkrescue --modules='uhci usb_keyboard' --pkglibdir=. --grub-mkimage=./grub-mkimage ../t/disk qemu -boot d -cdrom ../t/disk -usb -usbdevice keyboard
Ah, Felix pointed me at adding 'terminal_input.usb_keyboard' to grub.cfg. I'm working on testing this now.
Signature · 1 line
Colin Watson [[email protected]]
Quoted 8 lines · Robert Millan
Quoted 2 lines · Colin Watson
Perhaps we can apply the following patch first, then? I was following existing style, so the other code should be updated too.
Ah, sorry I didn't know that. But thanks for fixing anyway :-)
Please go ahead (I assume you've tested it, at least with QEMU's USB keyboard).
Committed this cleanup patch.
Signature · 1 line
Colin Watson [[email protected]]
Quoted 3 lines · Robert Millan
I don't have time to test it, but what we want is roughly something like this. If you would please test and confirm it's working the way you expected, it can be committed.
OK, here's a cleanup that (a) fixes some compilation errors in your patch and (b) adds USB keyboard support back in. How does this look?
2009-08-24 Colin Watson <[email protected]>
2009-08-24 Robert Millan <[email protected]>
Add getkeystatus' terminal method. Use it in sleep' to detect Shift being held down.
* include/grub/term.h (GRUB_TERM_STATUS_SHIFT, GRUB_TERM_STATUS_CTRL, GRUB_TERM_STATUS_ALT): Definitions for modifier key bitmasks.
(struct grub_term_input): Add `getkeystatus' member. (grub_getkeystatus): Add prototype.
* kern/term.c (grub_getkeystatus): New function.
* include/grub/i386/pc/memory.h (GRUB_MEMORY_MACHINE_BIOS_DATA_AREA_ADDR): New macro. (struct grub_machine_bios_data_area): Define necessary parts of BIOS Data Area layout.
* term/i386/pc/console.c (grub_console_getkeystatus): New function. (grub_console_term_input): Set `getkeystatus' member.
* term/usb_keyboard.c (grub_usb_keyboard_getkeystatus): New function.
(grub_usb_keyboard_term): Set `getkeystatus' member.
* commands/sleep.c (grub_check_keyboard): New function. (grub_interruptible_millisleep, grub_cmd_sleep): Use grub_check_keyboard, checking whether Shift is pressed in addition to checking for Escape.
term.c +9 -0
@@ -140,6 +140,15 @@return (grub_cur_term_input->checkkey) (); } +int +grub_getkeystatus (void ) +{ +if (grub_cur_term_input->getkeystatus) +return (grub_cur_term_input->getkeystatus) (); +else +return 0 ; +} +grub_uint16_t grub_getxy (void ) {
grub/term.h +11 -1
@@ -1,6 +1,6 @@/* * GRUB -- GRand Unified Bootloader -* Copyright (C) 2002,2003,2005,2007,2008 Free Software Foundation, Inc. +* Copyright (C) 2002,2003,2005,2007,2008,2009 Free Software Foundation, Inc. * * GRUB is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by
@@ -72,6 +72,12 @@#define GRUB_TERM_NEED_INIT (1 << 16) +/* Bitmasks for modifier keys returned by grub_getkeystatus. */ +#define GRUB_TERM_STATUS_SHIFT (1 << 0) +#define GRUB_TERM_STATUS_CTRL (1 << 1) +#define GRUB_TERM_STATUS_ALT (1 << 2) + +/* Unicode characters for fancy graphics. */ #define GRUB_TERM_DISP_LEFT 0x2190#define GRUB_TERM_DISP_UP 0x2191
@@ -157,6 +163,9 @@/* Get a character. */ int (*getkey) (void ); + +/* Get keyboard modifier status. */ +int (*getkeystatus) (void ); };typedef struct grub_term_input *grub_term_input_t ;
@@ -275,6 +284,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_getkeystatus ) (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);
grub/i386/pc/memory.h +12 -0
@@ -78,8 +78,20 @@/* The data segment of the pseudo real mode. */ #define GRUB_MEMORY_MACHINE_PSEUDO_REAL_DSEG 0x20 +#define GRUB_MEMORY_MACHINE_BIOS_DATA_AREA_ADDR 0x400 + #ifndef ASM_FILE +/* See http://heim.ifi.uio.no/~stanisls/helppc/bios_data_area.html for a + description of the BIOS Data Area layout. */ +struct grub_machine_bios_data_area +{ + grub_uint8_t unused1[0x17]; + grub_uint8_t keyboard_flag_lower; /* 0x17 */ + grub_uint8_t keyboard_flag_upper; /* 0x17 */ + grub_uint8_t unused2[0xf0 - 0x19]; +}; + struct grub_machine_mmap_entry { grub_uint32_t size;
sleep.c +17 -4
@@ -1,7 +1,7 @@/* sleep.c - Command to wait a specified number of seconds. */ /* * GRUB -- GRand Unified Bootloader -* Copyright (C) 2006,2007,2008 Free Software Foundation, Inc. +* Copyright (C) 2006,2007,2008,2009 Free Software Foundation, Inc. * * GRUB is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by
@@ -43,6 +43,20 @@grub_printf ("%d " , n); } +static int +grub_check_keyboard (void ) +{ +int mods =grub_getkeystatus (); +if (mods >=0 && (mods &GRUB_TERM_STATUS_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 +66,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 +87,7 @@if (n ==0 ) {/* Either `0' or broken input. */ -return 0 ; +return grub_check_keyboard (); } xy =grub_getxy ();
i386/pc/console.c +21 -1
@@ -1,6 +1,6 @@/* * GRUB -- GRand Unified Bootloader -* Copyright (C) 2002,2003,2005,2007,2008 Free Software Foundation, Inc. +* Copyright (C) 2002,2003,2005,2007,2008,2009 Free Software Foundation, Inc. * * GRUB is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by
@@ -16,15 +16,35 @@* along with GRUB. If not, see <http://www.gnu.org/licenses/>. */ +#include <grub/machine/memory.h> #include <grub/machine/console.h> #include <grub/term.h> #include <grub/types.h> +static const struct grub_machine_bios_data_area *bios_data_area =GRUB_MEMORY_MACHINE_BIOS_DATA_AREA_ADDR ; + +static int +grub_console_getkeystatus (void ) +{ +grub_uint8_t status = bios_data_area->keyboard_flag_lower; +int mods =0 ; + +if (status &0x03 ) + mods |=GRUB_TERM_STATUS_SHIFT ; +if (status &0x04 ) + mods |=GRUB_TERM_STATUS_CTRL ; +if (status &0x08 ) + mods |=GRUB_TERM_STATUS_ALT ; + +return mods; +} +static struct grub_term_input grub_console_term_input = { .name ="console" , .checkkey = grub_console_checkkey, .getkey = grub_console_getkey, + .getkeystatus = grub_console_getkeystatus, }; static struct grub_term_output grub_console_term_output =
usb_keyboard.c +44 -0
55 lines not on this page · raw message
Signature · 1 line
Colin Watson [[email protected]]
grub/i386/pc/memory.h line 93
Quoted 9 lines · Colin Watson
grub/i386/pc/memory.h +9
-0+/* See http://heim.ifi.uio.no/~stanisls/helppc/bios_data_area.html for a +description of the BIOS Data Area layout. */ +struct grub_machine_bios_data_area +{ +grub_uint8_t unused1[0x17 ]; +grub_uint8_t keyboard_flag_lower;/* 0x17 */ +grub_uint8_t keyboard_flag_upper;/* 0x17 */ +grub_uint8_t unused2[0xf0 -0x19 ]; +};
Why split the keyboard field in upper/lower? We have working 16-bit types :-)
sleep.c line 58
Quoted 13 lines
sleep.c +13
-0+static int +grub_check_keyboard (void ) +{ +int mods =grub_getkeystatus (); +if (mods >=0 && (mods &GRUB_TERM_STATUS_SHIFT ) !=0 ) +return 1 ; + +if (grub_checkkey () >=0 && +GRUB_TERM_ASCII_CHAR (grub_getkey ()) ==GRUB_TERM_ESC ) +return 1 ; + +return 0 ; +}
I'm not sure if hardcoding ESC and SHIFT here makes this a bit too ad-hoc.
IIRC you had a more generic approach in mind?
What does everyone think about this?
Signature · 5 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
Quoted 12 lines · Robert Millan
grub/i386/pc/memory.h line 93
Quoted 9 lines · Colin Watson
grub/i386/pc/memory.h +9
-0+/* See http://heim.ifi.uio.no/~stanisls/helppc/bios_data_area.html for a +description of the BIOS Data Area layout. */ +struct grub_machine_bios_data_area +{ +grub_uint8_t unused1[0x17 ]; +grub_uint8_t keyboard_flag_lower;/* 0x17 */ +grub_uint8_t keyboard_flag_upper;/* 0x17 */ +grub_uint8_t unused2[0xf0 -0x19 ]; +};Why split the keyboard field in upper/lower? We have working 16-bit types :-)
We only need the lower one and counterintuitively (for a little-endian system) I seemed to have to look at bits 8-11 of the 16-bit type rather than bits 0-3. Life was too short to worry about what was wrong and I decided to only fetch the byte we needed. I suppose we could delete the declaration of keyboard_flag_upper.
Quoted 7 lines
Quoted 7 lines
sleep.c +7
-0+static int +grub_check_keyboard (void ) +{ +int mods =grub_getkeystatus (); +if (mods >=0 && (mods &GRUB_TERM_STATUS_SHIFT ) !=0 ) +return 1 ; +
Quoted 8 lines
sleep.c line 58
Quoted 6 lines
sleep.c +6
-0+if (grub_checkkey () >=0 && +GRUB_TERM_ASCII_CHAR (grub_getkey ()) ==GRUB_TERM_ESC ) +return 1 ; + +return 0 ; +}I'm not sure if hardcoding ESC and SHIFT here makes this a bit too ad-hoc.
Maybe not much more than the previous hardcoding of Escape alone?
Quoted 1 line
IIRC you had a more generic approach in mind?
We talked on IRC about adding a magic "keystatus" environment variable that user code could inspect. I'm not sure whether this will actually make things any simpler at the moment, though - I don't think there's a way to do bitwise operations in the shell-like scripting interface (you can use 'test' to test greater/less than, but nothing like "&"). Would three magic variables, keystatus_shift, keystatus_ctrl, and keystatus_alt, be too inelegant?
I'm open to other ideas. 'sleep --interruptible 0' is not an entirely natural way to express what I'm looking for, so I'm not particularly attached to it. 'if sleep --interruptible 0; then set timeout=0; fi' is suboptimal because it means that if either (a) core.img is too old to have getkeystatus support or (b) the current console driver does not support getkeystatus then the menu will be skipped without a way to access it at run-time. I'd much rather have something that allowed zero-timeout-with-Shift-to-access-menu if and only if we have a new enough core.img and the console driver has getkeystatus support, and otherwise degraded to something else sensible.
Signature · 1 line
Colin Watson [[email protected]]
Quoted 10 lines · Colin Watson
I'm open to other ideas. 'sleep --interruptible 0' is not an entirely natural way to express what I'm looking for, so I'm not particularly attached to it. 'if sleep --interruptible 0; then set timeout=0; fi' is suboptimal because it means that if either (a) core.img is too old to have getkeystatus support or (b) the current console driver does not support getkeystatus then the menu will be skipped without a way to access it at run-time. I'd much rather have something that allowed zero-timeout-with-Shift-to-access-menu if and only if we have a new enough core.img and the console driver has getkeystatus support, and otherwise degraded to something else sensible.
Here's yet another version, following a discussion with Robert on IRC. I think that this now satisfies Vladimir's concerns about not breaking platforms that don't have a usable getkeystatus, and my concerns about backward compatibility with old core.img files. The new keystatus command is, I think, also a rather clearer way to express this.
I expect to use this something like this:
set timeout=10
if keystatus; then
if keystatus --shift; then
true
else
set timeout=0
fi
fiI left part of the sleep change in for some degree of consistency, but removed the change to the case where the sleep period is zero. I also adjusted the USB keyboard driver to tweak the idle rate temporarily to make sure that we get events even if a modifier isn't pressed or released during the timeout.
2009-08-24 Colin Watson <[email protected]>
2009-08-24 Robert Millan <[email protected]>
Add getkeystatus' terminal method. Use it in sleep' to detect Shift being held down. Add a new `keystatus' command to query it.
* include/grub/term.h (GRUB_TERM_STATUS_SHIFT, GRUB_TERM_STATUS_CTRL, GRUB_TERM_STATUS_ALT): Definitions for modifier key bitmasks.
(struct grub_term_input): Add `getkeystatus' member. (grub_getkeystatus): Add prototype.
* kern/term.c (grub_getkeystatus): New function.
* include/grub/i386/pc/memory.h (GRUB_MEMORY_MACHINE_BIOS_DATA_AREA_ADDR): New macro. (struct grub_machine_bios_data_area): Define necessary parts of BIOS Data Area layout.
* term/i386/pc/console.c (grub_console_getkeystatus): New function. (grub_console_term_input): Set `getkeystatus' member.
* term/usb_keyboard.c (grub_usb_keyboard_getkeystatus): New function.
(grub_usb_keyboard_term): Set `getkeystatus' member.
* commands/sleep.c (grub_check_keyboard): New function. (grub_interruptible_millisleep): Use grub_check_keyboard, checking whether Shift is pressed in addition to checking for Escape.
* commands/keystatus.c: New file.
* conf/common.rmk (pkglib_MODULES): Add keystatus.mod. (keystatus_mod_SOURCES): New variable. (keystatus_mod_CFLAGS): Likewise. (keystatus_mod_LDFLAGS): Likewise.
* conf/i386-coreboot.rmk (grub_emu_SOURCES): Add commands/keystatus.c.
* conf/i386-efi.rmk (grub_emu_SOURCES): Likewise.
* conf/i386-ieee1275.rmk (grub_emu_SOURCES): Likewise.
* conf/i386-pc.rmk (grub_emu_SOURCES): Likewise.
* conf/powerpc-ieee1275.rmk (grub_emu_SOURCES): Likewise.
* conf/sparc64-ieee1275.rmk (grub_emu_SOURCES): Likewise.
* conf/x86_64-efi.rmk (grub_emu_SOURCES): Likewise.
* DISTLIST: Add commands/keystatus.c.
common.rmk +7 -1
@@ -363,7 +363,8 @@ terminfo.mod test.mod blocklist.mod hexdump.mod \ read.mod sleep.mod loadenv.mod crc.mod parttool.mod \ msdospart.mod memrw.mod normal.mod sh.mod lua.mod \ - gptsync.mod true.mod probe.mod password.mod + gptsync.mod true.mod probe.mod password.mod \ + keystatus.mod # For password.mod. password_mod_SOURCES = commands/password.c
@@ -510,6 +511,11 @@ probe_mod_CFLAGS = $(COMMON_CFLAGS) probe_mod_LDFLAGS = $(COMMON_LDFLAGS) +# For keystatus.mod. +keystatus_mod_SOURCES = commands/keystatus.c +keystatus_mod_CFLAGS = $(COMMON_CFLAGS) +keystatus_mod_LDFLAGS = $(COMMON_LDFLAGS) + # For normal.mod. normal_mod_SOURCES = normal/main.c normal/cmdline.c normal/dyncmd.c \ normal/auth.c normal/autofs.c normal/handler.c \
i386-pc.rmk +1 -1
@@ -123,7 +123,7 @@ lib/hexdump.c commands/i386/pc/halt.c commands/reboot.c \ commands/gptsync.c commands/probe.c commands/xnu_uuid.c \ commands/i386/cpuid.c \ - commands/password.c \ + commands/password.c commands/keystatus.c \ disk/host.c disk/loopback.c disk/scsi.c \ fs/fshelp.c \ \
i386-efi.rmk +1 -1
@@ -37,7 +37,7 @@ commands/configfile.c commands/help.c \ commands/handler.c commands/ls.c commands/test.c \ commands/search.c commands/hexdump.c lib/hexdump.c \ - commands/halt.c commands/reboot.c \ + commands/halt.c commands/reboot.c commands/keystatus.c \ commands/i386/cpuid.c \ commands/password.c \ disk/loopback.c \
i386-ieee1275.rmk +1 -1
@@ -66,7 +66,7 @@ lib/hexdump.c commands/halt.c commands/reboot.c \ commands/gptsync.c commands/probe.c commands/xnu_uuid.c \ commands/i386/cpuid.c \ - commands/password.c \ + commands/password.c commands/keystatus.c \ disk/host.c disk/loopback.c \ \ fs/affs.c fs/cpio.c fs/fat.c fs/ext2.c fs/hfs.c \
x86_64-efi.rmk +1 -1
@@ -37,7 +37,7 @@ commands/search.c commands/hexdump.c lib/hexdump.c \ commands/halt.c commands/reboot.c \ commands/i386/cpuid.c \ - commands/password.c \ + commands/password.c commands/keystatus.c \ disk/loopback.c \ \ fs/affs.c fs/cpio.c fs/fat.c fs/ext2.c fs/hfs.c \
i386-coreboot.rmk +1 -1
@@ -110,7 +110,7 @@ commands/handler.c commands/ls.c commands/test.c \ commands/search.c commands/blocklist.c commands/hexdump.c \ commands/gptsync.c commands/probe.c commands/xnu_uuid.c \ - commands/password.c \ + commands/password.c commands/keystatus.c \ lib/hexdump.c commands/i386/cpuid.c \ disk/host.c disk/loopback.c \ \
powerpc-ieee1275.rmk +1 -1
@@ -46,7 +46,7 @@ commands/ls.c commands/blocklist.c commands/hexdump.c \ lib/hexdump.c commands/halt.c commands/reboot.c \ commands/gptsync.c commands/probe.c commands/xnu_uuid.c \ - commands/password.c \ + commands/password.c commands/keystatus.c \ disk/loopback.c \ \ fs/affs.c fs/cpio.c fs/fat.c fs/ext2.c fs/hfs.c \
sparc64-ieee1275.rmk +1 -1
@@ -103,7 +103,7 @@ commands/ls.c commands/blocklist.c commands/hexdump.c \ lib/hexdump.c commands/halt.c commands/reboot.c \ commands/gptsync.c commands/probe.c commands/xnu_uuid.c \ - commands/password.c \ + commands/password.c commands/keystatus.c \ disk/loopback.c \ \ fs/affs.c fs/cpio.c fs/fat.c fs/ext2.c fs/hfs.c \
term.c +9 -0
@@ -140,6 +140,15 @@return (grub_cur_term_input->checkkey) (); } +int +grub_getkeystatus (void ) +{ +if (grub_cur_term_input->getkeystatus) +return (grub_cur_term_input->getkeystatus) (); +else +return 0 ; +} +grub_uint16_t grub_getxy (void ) {
grub/term.h +11 -1
@@ -1,6 +1,6 @@/* * GRUB -- GRand Unified Bootloader -* Copyright (C) 2002,2003,2005,2007,2008 Free Software Foundation, Inc. +* Copyright (C) 2002,2003,2005,2007,2008,2009 Free Software Foundation, Inc. * * GRUB is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by
@@ -72,6 +72,12 @@#define GRUB_TERM_NEED_INIT (1 << 16) +/* Bitmasks for modifier keys returned by grub_getkeystatus. */ +#define GRUB_TERM_STATUS_SHIFT (1 << 0) +#define GRUB_TERM_STATUS_CTRL (1 << 1) +#define GRUB_TERM_STATUS_ALT (1 << 2) + +/* Unicode characters for fancy graphics. */ #define GRUB_TERM_DISP_LEFT 0x2190#define GRUB_TERM_DISP_UP 0x2191
@@ -157,6 +163,9 @@/* Get a character. */ int (*getkey) (void ); + +/* Get keyboard modifier status. */ +int (*getkeystatus) (void ); };typedef struct grub_term_input *grub_term_input_t ;
@@ -275,6 +284,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_getkeystatus ) (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);
grub/i386/pc/memory.h +11 -0
@@ -78,8 +78,19 @@/* The data segment of the pseudo real mode. */ #define GRUB_MEMORY_MACHINE_PSEUDO_REAL_DSEG 0x20 +#define GRUB_MEMORY_MACHINE_BIOS_DATA_AREA_ADDR 0x400 + #ifndef ASM_FILE +/* See http://heim.ifi.uio.no/~stanisls/helppc/bios_data_area.html for a + description of the BIOS Data Area layout. */ +struct grub_machine_bios_data_area +{ + grub_uint8_t unused1[0x17]; + grub_uint8_t keyboard_flag_lower; /* 0x17 */ + grub_uint8_t unused2[0xf0 - 0x18]; +}; + struct grub_machine_mmap_entry { grub_uint32_t size;
sleep.c +16 -3
@@ -1,7 +1,7 @@/* sleep.c - Command to wait a specified number of seconds. */ /* * GRUB -- GRand Unified Bootloader -* Copyright (C) 2006,2007,2008 Free Software Foundation, Inc. +* Copyright (C) 2006,2007,2008,2009 Free Software Foundation, Inc. * * GRUB is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by
@@ -43,6 +43,20 @@grub_printf ("%d " , n); } +static int +grub_check_keyboard (void ) +{ +int mods =grub_getkeystatus (); +if (mods >=0 && (mods &GRUB_TERM_STATUS_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 +66,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 ;
keystatus.c +81 -0
81 lines not on this page · raw message
i386/pc/console.c +21 -1
@@ -1,6 +1,6 @@/* * GRUB -- GRand Unified Bootloader -* Copyright (C) 2002,2003,2005,2007,2008 Free Software Foundation, Inc. +* Copyright (C) 2002,2003,2005,2007,2008,2009 Free Software Foundation, Inc. * * GRUB is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by
@@ -16,15 +16,35 @@* along with GRUB. If not, see <http://www.gnu.org/licenses/>. */ +#include <grub/machine/memory.h> #include <grub/machine/console.h> #include <grub/term.h> #include <grub/types.h> +static const struct grub_machine_bios_data_area *bios_data_area =GRUB_MEMORY_MACHINE_BIOS_DATA_AREA_ADDR ; + +static int +grub_console_getkeystatus (void ) +{ +grub_uint8_t status = bios_data_area->keyboard_flag_lower; +int mods =0 ; + +if (status &0x03 ) + mods |=GRUB_TERM_STATUS_SHIFT ; +if (status &0x04 ) + mods |=GRUB_TERM_STATUS_CTRL ; +if (status &0x08 ) + mods |=GRUB_TERM_STATUS_ALT ; + +return mods; +} +static struct grub_term_input grub_console_term_input = { .name ="console" , .checkkey = grub_console_checkkey, .getkey = grub_console_getkey, + .getkeystatus = grub_console_getkeystatus, }; static struct grub_term_output grub_console_term_output =
usb_keyboard.c +56 -0
67 lines not on this page · raw message
Signature · 1 line
Colin Watson [[email protected]]
Quoted 24 lines · Colin Watson
Index:
commands/sleep.c
===================================================================sleep.c +14
-0+static int +grub_check_keyboard (void ) +{ +int mods =grub_getkeystatus (); +if (mods >=0 && (mods &GRUB_TERM_STATUS_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)
sleep.c line 70
Quoted 7 lines
sleep.c +1
-2@@ -52,8 +66,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 ;
I'm still not convinced that we want the sleep bits. They seem too ad-hoc, and I'd like to hear what others think first.
But if I understood correctly, you plan on using keystatus command only?
Quoted 5 lines
sleep.c +5
-0+static int +grub_console_getkeystatus (void ) +{ +grub_uint8_t status = bios_data_area->keyboard_flag_lower; +int mods =0 ;
sleep.c
Quoted 10 lines
sleep.c +10
-0+ +if (status &0x03 ) + mods |=GRUB_TERM_STATUS_SHIFT ; +if (status &0x04 ) + mods |=GRUB_TERM_STATUS_CTRL ; +if (status &0x08 ) + mods |=GRUB_TERM_STATUS_ALT ; + +return mods; +}
This should be macroified (but for the time being, I have no problem with our internal representation matching the one in BIOS Data Area, hence no translation would be needed on i386-pc).
sleep.c
Quoted 10 lines
sleep.c +9
-0+/* Set idle time to the minimum offered by the spec (4 milliseconds) so +that we can find out the current state. */ +grub_usb_control_msg (usbdev,0x21 ,0x0A ,1 <<8 ,0 ,0 ,0 ); + +grub_usb_control_msg (usbdev, (1 <<7 ) | (1 <<5 ) |1 ,0x02 ,0 ,0 ,1 , (char *) data); + +/* Go back to reporting every time an event occurs and not more often than +that. */ +grub_usb_control_msg (usbdev,0x21 ,0x0A ,0 <<8 ,0 ,0 ,0 );
If they represent fixed numbers like registers/ports, these hex values should be macros too.
Signature · 5 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
Quoted 36 lines · Robert Millan
Quoted 24 lines · Colin Watson
Index:
commands/sleep.c
===================================================================sleep.c +14
-0+static int +grub_check_keyboard (void ) +{ +int mods =grub_getkeystatus (); +if (mods >=0 && (mods &GRUB_TERM_STATUS_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)sleep.c line 70
Quoted 7 lines
sleep.c +1
-2@@ -52,8 +66,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 ;I'm still not convinced that we want the sleep bits. They seem too ad-hoc, and I'd like to hear what others think first.
Quoted 1 line
But if I understood correctly, you plan on using keystatus command only?
If GRUB_HIDDEN_TIMEOUT is set to a non-zero value, I would use sleep rather than keystatus, I think. (At least, if I decided I wanted to support that configuration; I'm not quite sure yet.) In that case I'd want the sleep to be interruptible by pressing Shift for consistency with the zero-delay case.
However, since this is essentially an artifact of our configuration and doesn't introduce any new configuration file words, I could keep this part as an Ubuntu-specific patch with relatively little trouble.
Quoted 11 lines
Quoted 11 lines
sleep.c +11
-0+static int +grub_console_getkeystatus (void ) +{ +grub_uint8_t status = bios_data_area->keyboard_flag_lower; +int mods =0 ; + +if (status &0x03 ) + mods |=GRUB_TERM_STATUS_SHIFT ; +if (status &0x04 ) + mods |=GRUB_TERM_STATUS_CTRL ; +if (status &0x08 )
Quoted 8 lines
sleep.c
Quoted 4 lines
sleep.c +4
-0+ mods |=GRUB_TERM_STATUS_ALT ; + +return mods; +}This should be macroified (but for the time being, I have no problem with our internal representation matching the one in BIOS Data Area, hence no translation would be needed on i386-pc).
Vladimir objected to the inconsistency between distinguishing left and right Shift but not left and right Control or Alt, which is why our internal representation does not currently match. I agree it's clearer to macroify things.
Quoted 5 lines
Quoted 5 lines
sleep.c +5
-0+/* Set idle time to the minimum offered by the spec (4 milliseconds) so +that we can find out the current state. */ +grub_usb_control_msg (usbdev,0x21 ,0x0A ,1 <<8 ,0 ,0 ,0 ); + +grub_usb_control_msg (usbdev, (1 <<7 ) | (1 <<5 ) |1 ,0x02 ,0 ,0 ,1 , (char *) data);
Quoted 8 lines
sleep.c
Quoted 5 lines
sleep.c +4
-0+ +/* Go back to reporting every time an event occurs and not more often than +that. */ +grub_usb_control_msg (usbdev,0x21 ,0x0A ,0 <<8 ,0 ,0 ,0 );If they represent fixed numbers like registers/ports, these hex values should be macros too.
OK, and doing so helped me notice that I'd left a Get_Idle request in there by mistake from debugging. Updated patch follows. There are a few changes unrelated to the main body of the patch as a result; do you want these split out?
2009-08-24 Colin Watson <[email protected]>
2009-08-24 Robert Millan <[email protected]>
Add getkeystatus' terminal method. Add a new keystatus' command to query it.
* include/grub/term.h (GRUB_TERM_STATUS_SHIFT, GRUB_TERM_STATUS_CTRL, GRUB_TERM_STATUS_ALT): Definitions for modifier key bitmasks.
(struct grub_term_input): Add `getkeystatus' member. (grub_getkeystatus): Add prototype.
* kern/term.c (grub_getkeystatus): New function.
* include/grub/i386/pc/memory.h (GRUB_MEMORY_MACHINE_BIOS_DATA_AREA_ADDR): New macro. (struct grub_machine_bios_data_area): Define necessary parts of BIOS Data Area layout.
* term/i386/pc/console.c (grub_console_getkeystatus): New function. (grub_console_term_input): Set `getkeystatus' member.
* term/usb_keyboard.c (grub_usb_hid): Macroify HID protocol constants.
(grub_usb_keyboard_getreport): Likewise. (grub_usb_keyboard_checkkey): Likewise. (grub_usb_keyboard_getkeystatus): New function. (grub_usb_keyboard_term): Set `getkeystatus' member.
* commands/keystatus.c: New file.
* conf/common.rmk (pkglib_MODULES): Add keystatus.mod. (keystatus_mod_SOURCES): New variable. (keystatus_mod_CFLAGS): Likewise. (keystatus_mod_LDFLAGS): Likewise.
* conf/i386-coreboot.rmk (grub_emu_SOURCES): Add commands/keystatus.c.
* conf/i386-efi.rmk (grub_emu_SOURCES): Likewise.
* conf/i386-ieee1275.rmk (grub_emu_SOURCES): Likewise.
* conf/i386-pc.rmk (grub_emu_SOURCES): Likewise.
* conf/powerpc-ieee1275.rmk (grub_emu_SOURCES): Likewise.
* conf/sparc64-ieee1275.rmk (grub_emu_SOURCES): Likewise.
* conf/x86_64-efi.rmk (grub_emu_SOURCES): Likewise.
* DISTLIST: Add commands/keystatus.c.
common.rmk +7 -1
@@ -363,7 +363,8 @@ terminfo.mod test.mod blocklist.mod hexdump.mod \ read.mod sleep.mod loadenv.mod crc.mod parttool.mod \ msdospart.mod memrw.mod normal.mod sh.mod lua.mod \ - gptsync.mod true.mod probe.mod password.mod + gptsync.mod true.mod probe.mod password.mod \ + keystatus.mod # For password.mod. password_mod_SOURCES = commands/password.c
@@ -510,6 +511,11 @@ probe_mod_CFLAGS = $(COMMON_CFLAGS) probe_mod_LDFLAGS = $(COMMON_LDFLAGS) +# For keystatus.mod. +keystatus_mod_SOURCES = commands/keystatus.c +keystatus_mod_CFLAGS = $(COMMON_CFLAGS) +keystatus_mod_LDFLAGS = $(COMMON_LDFLAGS) + # For normal.mod. normal_mod_SOURCES = normal/main.c normal/cmdline.c normal/dyncmd.c \ normal/auth.c normal/autofs.c normal/handler.c \
i386-pc.rmk +1 -1
@@ -123,7 +123,7 @@ lib/hexdump.c commands/i386/pc/halt.c commands/reboot.c \ commands/gptsync.c commands/probe.c commands/xnu_uuid.c \ commands/i386/cpuid.c \ - commands/password.c \ + commands/password.c commands/keystatus.c \ disk/host.c disk/loopback.c disk/scsi.c \ fs/fshelp.c \ \
i386-efi.rmk +1 -1
@@ -37,7 +37,7 @@ commands/configfile.c commands/help.c \ commands/handler.c commands/ls.c commands/test.c \ commands/search.c commands/hexdump.c lib/hexdump.c \ - commands/halt.c commands/reboot.c \ + commands/halt.c commands/reboot.c commands/keystatus.c \ commands/i386/cpuid.c \ commands/password.c \ disk/loopback.c \
i386-ieee1275.rmk +1 -1
@@ -66,7 +66,7 @@ lib/hexdump.c commands/halt.c commands/reboot.c \ commands/gptsync.c commands/probe.c commands/xnu_uuid.c \ commands/i386/cpuid.c \ - commands/password.c \ + commands/password.c commands/keystatus.c \ disk/host.c disk/loopback.c \ \ fs/affs.c fs/cpio.c fs/fat.c fs/ext2.c fs/hfs.c \
x86_64-efi.rmk +1 -1
@@ -37,7 +37,7 @@ commands/search.c commands/hexdump.c lib/hexdump.c \ commands/halt.c commands/reboot.c \ commands/i386/cpuid.c \ - commands/password.c \ + commands/password.c commands/keystatus.c \ disk/loopback.c \ \ fs/affs.c fs/cpio.c fs/fat.c fs/ext2.c fs/hfs.c \
i386-coreboot.rmk +1 -1
@@ -110,7 +110,7 @@ commands/handler.c commands/ls.c commands/test.c \ commands/search.c commands/blocklist.c commands/hexdump.c \ commands/gptsync.c commands/probe.c commands/xnu_uuid.c \ - commands/password.c \ + commands/password.c commands/keystatus.c \ lib/hexdump.c commands/i386/cpuid.c \ disk/host.c disk/loopback.c \ \
powerpc-ieee1275.rmk +1 -1
@@ -46,7 +46,7 @@ commands/ls.c commands/blocklist.c commands/hexdump.c \ lib/hexdump.c commands/halt.c commands/reboot.c \ commands/gptsync.c commands/probe.c commands/xnu_uuid.c \ - commands/password.c \ + commands/password.c commands/keystatus.c \ disk/loopback.c \ \ fs/affs.c fs/cpio.c fs/fat.c fs/ext2.c fs/hfs.c \
sparc64-ieee1275.rmk +1 -1
@@ -103,7 +103,7 @@ commands/ls.c commands/blocklist.c commands/hexdump.c \ lib/hexdump.c commands/halt.c commands/reboot.c \ commands/gptsync.c commands/probe.c commands/xnu_uuid.c \ - commands/password.c \ + commands/password.c commands/keystatus.c \ disk/loopback.c \ \ fs/affs.c fs/cpio.c fs/fat.c fs/ext2.c fs/hfs.c \
term.c +9 -0
@@ -140,6 +140,15 @@return (grub_cur_term_input->checkkey) (); } +int +grub_getkeystatus (void ) +{ +if (grub_cur_term_input->getkeystatus) +return (grub_cur_term_input->getkeystatus) (); +else +return 0 ; +} +grub_uint16_t grub_getxy (void ) {
grub/term.h +11 -1
@@ -1,6 +1,6 @@/* * GRUB -- GRand Unified Bootloader -* Copyright (C) 2002,2003,2005,2007,2008 Free Software Foundation, Inc. +* Copyright (C) 2002,2003,2005,2007,2008,2009 Free Software Foundation, Inc. * * GRUB is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by
@@ -72,6 +72,12 @@#define GRUB_TERM_NEED_INIT (1 << 16) +/* Bitmasks for modifier keys returned by grub_getkeystatus. */ +#define GRUB_TERM_STATUS_SHIFT (1 << 0) +#define GRUB_TERM_STATUS_CTRL (1 << 1) +#define GRUB_TERM_STATUS_ALT (1 << 2) + +/* Unicode characters for fancy graphics. */ #define GRUB_TERM_DISP_LEFT 0x2190#define GRUB_TERM_DISP_UP 0x2191
@@ -157,6 +163,9 @@/* Get a character. */ int (*getkey) (void ); + +/* Get keyboard modifier status. */ +int (*getkeystatus) (void ); };typedef struct grub_term_input *grub_term_input_t ;
@@ -275,6 +284,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_getkeystatus ) (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);
grub/i386/pc/memory.h +11 -0
@@ -78,8 +78,19 @@/* The data segment of the pseudo real mode. */ #define GRUB_MEMORY_MACHINE_PSEUDO_REAL_DSEG 0x20 +#define GRUB_MEMORY_MACHINE_BIOS_DATA_AREA_ADDR 0x400 + #ifndef ASM_FILE +/* See http://heim.ifi.uio.no/~stanisls/helppc/bios_data_area.html for a + description of the BIOS Data Area layout. */ +struct grub_machine_bios_data_area +{ + grub_uint8_t unused1[0x17]; + grub_uint8_t keyboard_flag_lower; /* 0x17 */ + grub_uint8_t unused2[0xf0 - 0x18]; +}; + struct grub_machine_mmap_entry { grub_uint32_t size;
keystatus.c +81 -0
81 lines not on this page · raw message
i386/pc/console.c +26 -1
@@ -1,6 +1,6 @@/* * GRUB -- GRand Unified Bootloader -* Copyright (C) 2002,2003,2005,2007,2008 Free Software Foundation, Inc. +* Copyright (C) 2002,2003,2005,2007,2008,2009 Free Software Foundation, Inc. * * GRUB is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by
@@ -16,15 +16,40 @@* along with GRUB. If not, see <http://www.gnu.org/licenses/>. */ +#include <grub/machine/memory.h> #include <grub/machine/console.h> #include <grub/term.h> #include <grub/types.h> +static const struct grub_machine_bios_data_area *bios_data_area =GRUB_MEMORY_MACHINE_BIOS_DATA_AREA_ADDR ; + +#define KEYBOARD_LEFT_SHIFT (1 << 0) +#define KEYBOARD_RIGHT_SHIFT (1 << 1) +#define KEYBOARD_CTRL (1 << 2) +#define KEYBOARD_ALT (1 << 3) + +static int +grub_console_getkeystatus (void ) +{ +grub_uint8_t status = bios_data_area->keyboard_flag_lower; +int mods =0 ; + +if (status & (KEYBOARD_LEFT_SHIFT |KEYBOARD_RIGHT_SHIFT )) + mods |=GRUB_TERM_STATUS_SHIFT ; +if (status &KEYBOARD_CTRL ) + mods |=GRUB_TERM_STATUS_CTRL ; +if (status &KEYBOARD_ALT ) + mods |=GRUB_TERM_STATUS_ALT ; + +return mods; +} +static struct grub_term_input grub_console_term_input = { .name ="console" , .checkkey = grub_console_checkkey, .getkey = grub_console_getkey, + .getkeystatus = grub_console_getkeystatus, }; static struct grub_term_output grub_console_term_output =
usb_keyboard.c +77 -5
118 lines not on this page · raw message
Signature · 1 line
Colin Watson [[email protected]]
Quoted 12 lines · Colin Watson
Quoted 5 lines
I'm still not convinced that we want the sleep bits. They seem too ad-hoc, and I'd like to hear what others think first.
But if I understood correctly, you plan on using keystatus command only?
If
GRUB_HIDDEN_TIMEOUTis set to a non-zero value, I would use sleep rather than keystatus, I think. (At least, if I decided I wanted to support that configuration; I'm not quite sure yet.) In that case I'd want the sleep to be interruptible by pressing Shift for consistency with the zero-delay case.
Quoted 3 lines
However, since this is essentially an artifact of our configuration and doesn't introduce any new configuration file words, I could keep this part as an Ubuntu-specific patch with relatively little trouble.
Please do for now. I'd rather not rush when discussing this part.
Quoted 20 lines
Quoted 11 lines
Quoted 11 lines
i386/pc/console.c +11
-0+static int +grub_console_getkeystatus (void ) +{ +grub_uint8_t status = bios_data_area->keyboard_flag_lower; +int mods =0 ; + +if (status &0x03 ) + mods |=GRUB_TERM_STATUS_SHIFT ; +if (status &0x04 ) + mods |=GRUB_TERM_STATUS_CTRL ; +if (status &0x08 )Quoted 8 lines
i386/pc/console.c line 45
Quoted 4 lines
i386/pc/console.c +4
-0+ mods |=GRUB_TERM_STATUS_ALT ; + +return mods; +}This should be macroified (but for the time being, I have no problem with our internal representation matching the one in BIOS Data Area, hence no translation would be needed on i386-pc).
Quoted 4 lines
Vladimir objected to the inconsistency between distinguishing left and right Shift but not left and right Control or Alt, which is why our internal representation does not currently match. I agree it's clearer to macroify things.
Looks fine now, thanks.
Quoted 4 lines
OK, and doing so helped me notice that I'd left a
Get_Idlerequest in there by mistake from debugging. Updated patch follows. There are a few changes unrelated to the main body of the patch as a result; do you want these split out?
No need, this looks good. Please commit.
Signature · 5 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
Quoted 7 lines · Robert Millan
Quoted 6 lines · Colin Watson
If
GRUB_HIDDEN_TIMEOUTis set to a non-zero value, I would use sleep rather than keystatus, I think. (At least, if I decided I wanted to support that configuration; I'm not quite sure yet.) In that case I'd want the sleep to be interruptible by pressing Shift for consistency with the zero-delay case.
Quoted 5 lines
Quoted 3 lines
However, since this is essentially an artifact of our configuration and doesn't introduce any new configuration file words, I could keep this part as an Ubuntu-specific patch with relatively little trouble.
Please do for now. I'd rather not rush when discussing this part.
Consider it done.
Quoted 6 lines
Quoted 4 lines
OK, and doing so helped me notice that I'd left a
Get_Idlerequest in there by mistake from debugging. Updated patch follows. There are a few changes unrelated to the main body of the patch as a result; do you want these split out?No need, this looks good. Please commit.
Committed, thanks.
Signature · 1 line
Colin Watson [[email protected]]
term/ieee1275/ofconsole.c line 401
Quoted 10 lines · Colin Watson
diff -Nur -x '*.orig' -x '*~'
grub2-1.96+20090725/term/ieee1275/ofconsole.cgrub2-1.96+20090725.new/term/ieee1275/ofconsole.cterm/ieee1275/ofconsole.c +2
-0@@ -396,6 +396,8 @@ .fini = grub_ofconsole_fini, .checkkey = grub_ofconsole_checkkey, .getkey = grub_ofconsole_getkey, + /* No attempt to support this for Open Firmware yet. */ + .keystatus = NULL, };
Btw, I ommitted this from the last patch I sent, as it is unnecessary. The BSS is zero'ed before grub_main() so all uninitialized variables are implicitly 0. In this kind of structs, when a member is optional and we don't implement it, we just skip it.
Signature · 5 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
Oh, I didn't know there was a BIOS callback for this. Disregard what I said about at_keyboard then...
Signature · 5 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
term/i386/pc/at_keyboard.c line 210
Quoted 8 lines · Colin Watson
term/i386/pc/at_keyboard.c +8
-0+static int +grub_at_keyboard_keystatus (void ) +{ +/* FIXME: I don't know if getting key modifier status is possible without +* BIOS help. */ +return 0 ; +} +
What we do in these cases is leave the function pointer undefined, which means NULL value (and check this in the caller of course).
But in at_keyboard it's definitely possible (check how we handle e.g. CTRL or SHIFT keys).
Signature · 5 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
Quoted 13 lines · Robert Millan
term/i386/pc/at_keyboard.c line 210
Quoted 8 lines · Colin Watson
term/i386/pc/at_keyboard.c +8
-0+static int +grub_at_keyboard_keystatus (void ) +{ + /* FIXME: I don't know if getting key modifier status is possible without + * BIOS help. */ + return 0; +} +What we do in these cases is leave the function pointer undefined, which means NULL value (and check this in the caller of course).
Quoted 3 lines
But in
at_keyboardit's definitely possible (check how we handle e.g. CTRL or SHIFT keys).
Exactly what I meant
Quoted 13 lines
--
Robert MillanThe DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
_______________________________________________
Grub-devel mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/grub-devel
Signature · 4 lines
Regards Vladimir 'phcoder' Serbinenko Personal git repository: http://repo.or.cz/w/grub2/phcoder.git
Quoted 14 lines · Vladimir 'phcoder' Serbinenko
Quoted 13 lines · Robert Millan
term/i386/pc/at_keyboard.c line 210
Quoted 8 lines · Colin Watson
term/i386/pc/at_keyboard.c +8
-0+static int +grub_at_keyboard_keystatus (void ) +{ + /* FIXME: I don't know if getting key modifier status is possible without + * BIOS help. */ + return 0; +} +What we do in these cases is leave the function pointer undefined, which means NULL value (and check this in the caller of course).
Quoted 4 lines
Quoted 3 lines
But in
at_keyboardit's definitely possible (check how we handle e.g. CTRL or SHIFT keys).Exactly what I meant
Oh, I'm sorry, I sometimes miss things. Anyway, I hope it's clearer now that I repeated the same thing :-)
Signature · 5 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
Quoted 12 lines · Robert Millan
term/i386/pc/at_keyboard.c line 210
Quoted 8 lines · Colin Watson
term/i386/pc/at_keyboard.c +8
-0+static int +grub_at_keyboard_keystatus (void ) +{ +/* FIXME: I don't know if getting key modifier status is possible without +* BIOS help. */ +return 0 ; +} +What we do in these cases is leave the function pointer undefined, which means NULL value (and check this in the caller of course).
OK.
Quoted 2 lines
But in
at_keyboardit's definitely possible (check how we handle e.g. CTRL or SHIFT keys).
No, that code only spots make scan codes arriving after GRUB's terminal starts up. AFAICS it has no way to tell whether e.g. Shift was held down already when GRUB started (except through the vagaries of key repeat), which is important here.
Signature · 1 line
Colin Watson [[email protected]]
Quoted 6 lines · Colin Watson
Quoted 2 lines
But in
at_keyboardit's definitely possible (check how we handle e.g. CTRL or SHIFT keys).No, that code only spots make scan codes arriving after GRUB's terminal starts up. AFAICS it has no way to tell whether e.g. Shift was held down already when GRUB started
Ah, you're right on this..
Quoted 1 line
(except through the vagaries of key repeat),
..but on this too. Why not check for key repeat? The controller generates them for all keys AFAIK.
Signature · 5 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
Quoted 10 lines · Robert Millan
Quoted 6 lines · Colin Watson
Quoted 2 lines
But in
at_keyboardit's definitely possible (check how we handle e.g. CTRL or SHIFT keys).No, that code only spots make scan codes arriving after GRUB's terminal starts up. AFAICS it has no way to tell whether e.g. Shift was held down already when GRUB started
Ah, you're right on this..
In this particular case yes. But I can imagine other cases when it doesn't matter. E.g. if a distro wants to show a notice before booting single user entry and user wants to press shift to bypass this notice.
Quoted 18 lines
Quoted 1 line
(except through the vagaries of key repeat),
..but on this too. Why not check for key repeat? The controller generates them for all keys AFAIK.
--
Robert MillanThe DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
_______________________________________________
Grub-devel mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/grub-devel
Signature · 4 lines
Regards Vladimir 'phcoder' Serbinenko Personal git repository: http://repo.or.cz/w/grub2/phcoder.git
2009/8/24 Robert Millan <[email protected]>:
Quoted 8 lines · Colin Watson
Quoted 6 lines
Quoted 2 lines
But in
at_keyboardit's definitely possible (check how we handle e.g. CTRL or SHIFT keys).No, that code only spots make scan codes arriving after GRUB's terminal starts up. AFAICS it has no way to tell whether e.g. Shift was held down already when GRUB started
Quoted 6 lines
Ah, you're right on this..
Quoted 1 line
(except through the vagaries of key repeat),
..but on this too. Why not check for key repeat? The controller generates them for all keys AFAIK.
..unless turned off in the BIOS (which some seem to allow).
Thanks
Michal
Quoted 9 lines · Michal Suchanek
2009/8/24 Robert Millan <[email protected]>:
Quoted 8 lines · Colin Watson
Quoted 6 lines
Quoted 2 lines
But in
at_keyboardit's definitely possible (check how we handle e.g. CTRL or SHIFT keys).No, that code only spots make scan codes arriving after GRUB's terminal starts up. AFAICS it has no way to tell whether e.g. Shift was held down already when GRUB started
Quoted 8 lines
Quoted 6 lines
Ah, you're right on this..
Quoted 1 line
(except through the vagaries of key repeat),
..but on this too. Why not check for key repeat? The controller generates them for all keys AFAIK.
..unless turned off in the BIOS (which some seem to allow).
It's still hardware, so we can turn it back on. I guess we should be?
Signature · 5 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
Quoted 11 lines · Robert Millan
Quoted 3 lines · Colin Watson
No, that code only spots make scan codes arriving after GRUB's terminal starts up. AFAICS it has no way to tell whether e.g. Shift was held down already when GRUB started
Ah, you're right on this..
Quoted 1 line
(except through the vagaries of key repeat),
..but on this too. Why not check for key repeat? The controller generates them for all keys AFAIK.
If you want a delay of zero (i.e. "is Shift pressed? no? OK, boot immediately with no delay"), how long should you wait for key repeat? If there's a sensible answer to that question (IIRC key repeat speed is sometimes configurable in the BIOS) then we could do that ...
Signature · 1 line
Colin Watson [[email protected]]
Quoted 8 lines · Robert Millan
Quoted 6 lines · Colin Watson
Quoted 2 lines
But in
at_keyboardit's definitely possible (check how we handle e.g. CTRL or SHIFT keys).No, that code only spots make scan codes arriving after GRUB's terminal starts up. AFAICS it has no way to tell whether e.g. Shift was held down already when GRUB started
Quoted 6 lines
Ah, you're right on this..
Quoted 1 line
(except through the vagaries of key repeat),
..but on this too. Why not check for key repeat? The controller generates them for all keys AFAIK.
Or we could just make at_keyboard query the startup console for initial state and then update it afterwards.
But this has the problem that in a multi-keyboard scenario we might be using fake information.
Signature · 5 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."