Menu time-out missing when GRUB is loaded quickly and `at_keyboard`
Dear GRUB folks, When the module `at_keyboard` is directly into the GRUB image (`--modules`), and GRUB is loaded really quickly, then the timer, which, after counting down to 0 (`GRUB_TIMEOUT`), starts the selected entry, is not shown. I noticed this issue on the ASRock E350M1 with coreboot and a small GRUB payload, and a PS/2 keyboard connected. Due to the missing time-out, I manually have to confirm the selected entry. By chance, the keyboard wasn’t connected setting up the system somewhere else, and that made it work as expected. So, it looks like, it’s related to `at_keyboard`, and some race, because the bigger default GRUB payload also does not show the problem. Luckily, it’s easily reproducible with GRUB’s standard `default_payload.elf` and QEMU. Please find the instructions below to reproduce the issue. $ git clone https://review.coreboot.org/coreboot $ cd coreboot $ # save attached grub.cfg in the directory $ util/scripts/config -e PAYLOAD_GRUB2 grep: .config: Datei oder Verzeichnis nicht gefunden $ util/scripts/config -e GRUB2_INCLUDE_RUNTIME_CONFIG_FILE $ util/scripts/config -e GRUB2_MASTER $ util/scripts/config -e CONFIG_COREBOOT_ROMSIZE_KB_2048 # default of 512 KB too small for GRUB payload $ util/scripts/config -e ANY_TOOLCHAIN $ # or: make crossgcc-i386 CPUS=`nproc` $ make olddefconfig $ make -j`nproc` $ qemu-system-x86_64 --version QEMU emulator version 3.1.0 (Debian 1:3.1+dfsg-2+b1) Copyright (c) 2003-2018 Fabrice Bellard and the QEMU Project developers $ qemu-system-x86_64 -M pc -bios build/coreboot.rom -serial stdio *No* time-out is shown. Telling QEMU to emulate a USB keyboard, indirectly disabling the PS/2 keyboard, the time-out *is* shown. $ qemu-system-x86_64 -M pc -bios build/coreboot.rom -serial stdio \ -usb -device usb-kbd Not including `at_keyboard` directly in GRUB’s “core image”, modules loaded automatically, the time-out is also shown. `set debug=at_keyb` did not show anything interesting. Can you reproduce that, and see what the problem is? Kind regards, Paul PS: Please find the instructions to build GRUB as a coreboot payload done by coreboot’s Kconfig system automatically, and how put it into the coreboot filesystem CBFS. ``` $ git clone https://git.savannah.gnu.org/git/grub.git/ $ cd grub $ git describe --tags --dirty grub-2.02-238-ga791dc0e3 $ ./autogen.sh $ ./configure --with-platform=coreboot $ make default_payload.elf # this includes `at_keyboard` $ cp default_payload.elf my/coreboot/folder/ $ cd my/coreboot/folder/ $ build/cbfstool build/coreboot.rom print $ build/cbfstool build/coreboot.rom remove -n fallback/payload $ build/cbfstool build/coreboot.rom add-payload -f payload.elf -n fallback/payload -c lzma ``` Kind regards, Paul
Date: Sat, 4 Feb 2019 22:39:37 +0100 Some terminals, like `grub-core/term/at_keyboard.c`, return `-1` in case they are not ready yet. if (! KEYBOARD_ISREADY (grub_inb (KEYBOARD_REG_STATUS))) return -1; Currently, that is treated as a key press, and the menu time-out is cancelled/cleared. This is unwanted, as the boot is stopped and the user manually has to select a menu entry. Therefore, adapt the condition to require the key value also to be greater than 0. `GRUB_TERM_NO_KEY` is defined as 0, so the condition could be collapsed to greater or equal than (≥) 0, but the compiler will probably do that for us anyway, so keep the cases separate for clarity. This is tested with coreboot, the GRUB default payload, and the configuration file `grub.cfg` below. For GRUB: $ ./autogen.sh $ ./configure --with-platform=coreboot $ make -j`nproc` $ make default_payload.elf For coreboot: $ more grub.cfg serial --unit 0 --speed 115200 set timeout=5 menuentry 'halt' { halt } $ build/cbfstool build/coreboot.rom add-payload \ -f /dev/shm/grub/default_payload.elf -n fallback/payload -c lzma $ build/cbfstool build/coreboot.rom add -f grub.cfg -n etc/grub.cfg -t raw $ qemu-system-x86_64 --version QEMU emulator version 3.1.0 (Debian 1:3.1+dfsg-2+b1) Copyright (c) 2003-2018 Fabrice Bellard and the QEMU Project developers $ qemu-system-x86_64 -M pc -bios build/coreboot.rom -serial stdio -nic none Currently, the time-out is cancelled/cleared. With the commit, it is not. With a small GRUB payload, this the problem is also reproducible on the ASRock E350M1. Link: http://lists.gnu.org/archive/html/grub-devel/2019-01/msg00037.html Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de> --- grub-core/normal/menu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/grub-core/normal/menu.c b/grub-core/normal/menu.c index e7a83c2d6..68159e771 100644 --- a/grub-core/normal/menu.c +++ b/grub-core/normal/menu.c @@ -698,7 +698,8 @@ run_menu (grub_menu_t menu, int nested, int *auto_boot) c = grub_getkey_noblock (); - if (c != GRUB_TERM_NO_KEY) + /* Negative values are returned on error */ + if ((c != GRUB_TERM_NO_KEY) && (c > 0)) { if (timeout >= 0) { -- 2.20.1
On Tue, Feb 05, 2019 at 05:29:13PM +0100, Paul Menzel wrote:
Date: Sat, 4 Feb 2019 22:39:37 +0100
Some terminals, like `grub-core/term/at_keyboard.c`, return `-1` in case they are not ready yet.
if (! KEYBOARD_ISREADY (grub_inb (KEYBOARD_REG_STATUS))) return -1;
Currently, that is treated as a key press, and the menu time-out is cancelled/cleared. This is unwanted, as the boot is stopped and the user manually has to select a menu entry. Therefore, adapt the condition to require the key value also to be greater than 0.
`GRUB_TERM_NO_KEY` is defined as 0, so the condition could be collapsed to greater or equal than (≥) 0, but the compiler will probably do that for us anyway, so keep the cases separate for clarity.
This is tested with coreboot, the GRUB default payload, and the configuration file `grub.cfg` below.
For GRUB:
$ ./autogen.sh $ ./configure --with-platform=coreboot $ make -j`nproc` $ make default_payload.elf
For coreboot:
$ more grub.cfg serial --unit 0 --speed 115200 set timeout=5
menuentry 'halt' { halt } $ build/cbfstool build/coreboot.rom add-payload \ -f /dev/shm/grub/default_payload.elf -n fallback/payload -c lzma $ build/cbfstool build/coreboot.rom add -f grub.cfg -n etc/grub.cfg -t raw $ qemu-system-x86_64 --version QEMU emulator version 3.1.0 (Debian 1:3.1+dfsg-2+b1) Copyright (c) 2003-2018 Fabrice Bellard and the QEMU Project developers $ qemu-system-x86_64 -M pc -bios build/coreboot.rom -serial stdio -nic none
Currently, the time-out is cancelled/cleared. With the commit, it is not.
With a small GRUB payload, this the problem is also reproducible on the ASRock E350M1.
Link: http://lists.gnu.org/archive/html/grub-devel/2019-01/msg00037.html Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> Daniel
participants (2)
-
Daniel Kiper -
Paul Menzel