Re: Patch that fixes an 'at_keyboard' module issue (unreliable key press
From:
Michael Bideau
Subject:
Re: Patch that fixes an 'at_keyboard' module issue (unreliable key presses)
Date:
Wed, 11 Sep 2019 00:29:42 +0200
Hi Paul and everyone,
My turn to be sorry for the late reply. I was on my first sailing cruise last week, no computer on board. Scary (45 knots), beautiful and a real team effort. Like grub dev' ahah.
My answer inline below...
Best regards,
Michael
Le mardi 03 septembre 2019 à 13:48 +0200, Paul Menzel a écrit : > Dear Michael, > > > I am sorry for the late reply. > > > On 2019-08-29 00:24, Michael Bideau wrote: > > > Le mardi 27 août 2019 à 11:57 +0200, Paul Menzel a écrit : > > > On 8/24/19 9:09 PM, Michael Bideau wrote: > > > > > > > This patch fixes an issue that prevented the 'at_keyboard' > > > > module > > > > to work (for me). > > > > > > > > The cause is a bad/wrong return value in the function > > > > 'grub_at_keyboard_getkey()' in file > > > > 'grub-core/term/at_keyboard.c' at line 234. > > > > > > > > ///////// patch ///////// > > > > diff --git a/grub-core/term/at_keyboard.c b/grub- > > > > core/term/at_keyboard.c > > > > index f0a986eb1..597111077 100644 > > > > --- a/grub-core/term/at_keyboard.c > > > > +++ b/grub-core/term/at_keyboard.c > > > > @@ -234,7 +234,7 @@ grub_at_keyboard_getkey (struct > > > > grub_term_input > > > > *term __attribute__ ((unused))) > > > > return GRUB_TERM_NO_KEY; > > > > > > > > if (! KEYBOARD_ISREADY (grub_inb (KEYBOARD_REG_STATUS))) > > > > - return -1; > > > > + return GRUB_TERM_NO_KEY; > > > > at_key = grub_inb (KEYBOARD_REG_DATA); > > > > old_led = ps2_state.led_status; > > > > ///////// end of patch ///////// > > > > > > > > > > > > My symptoms were to have an unresponsive keyboard: keys needed > > > > to be pressed 10x and more to effectively be printed, sometimes > > > > generating multiple key presses (after 1 or 2 sec of no > > > > printing). Very problematic for typing passphrase in early > > > > stage > > > > (with GRUB_ENABLE_CRYPTODISK). When switching to 'console' > > > > terminal input, keyboard works perfectly. It also worked great > > > > with grub 2.02 packaged by Debian (2.02+dfsg1- 20). It was not > > > > an > > > > output issue, but an input one. > > > > > > […] > > > I think I had a similar issue and tried to fix it in commit > > > d3a3543a > > > (normal/menu: Do not treat error values as key presses) [1], > > > present > > > in GRUB 2.04. Do you have that commit in your tree? > > > > Yes I have this/your commit in my tree. > > > > ///////// your patch d3a3543a ///////// > > diff --git a/grub-core/normal/menu.c b/grub-core/normal/menu.c > > index e7a83c2d6..d5e0c79a7 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) > > { > > ///////// end of your patch d3a3543a ///////// > > > > For what I understand, your patch seems to "only" address > > normal/menu > > timeout issue (but I might be wrong, I'm still very new to grub2 > > code), > > but mine fixes the keyboard (if input is 'at_keyboard') wherever it > > is > > used in the normal/menu or in the grub terminal/shell or even with > > the > > cryptomount utility (typing passphrase was my issue at the > > beginning). > > > > To be clear, with the last checkout (commit 4e75b2ae3), that > > includes > > your commit d3a3543a, I have my keyboard not working reliably (as > > described previously), and with my patch it works perfectly > > (meaning > > in the grub terminal/shell I can type commands seamlessly and even > > use different keyboard layouts). > > You are correct. Thank you for the analysis. Could you please create > a > “proper” commit with a commit message (most can be taken from your > mail), and send that to this list?
The "proper" commit :
///////// show 261dd4318 ///////// at_keyboard: fix unreliable key presses
This patch fixes an issue that prevented the 'at_keyboard' module to work (for me).
The cause is a bad/wrong return value in the function 'grub_at_keyboard_getkey()' in file 'grub-core/term/at_keyboard.c' at line 237.
My symptoms were to have an unresponsive keyboard: keys needed to be pressed 10x and more to effectively be printed, sometimes generating multiple key presses (after 1 or 2 sec of no printing). Very problematic for typing passphrase in early stage (with GRUB_ENABLE_CRYPTODISK). When switching to 'console' terminal input, keyboard works perfectly. It also worked great with grub 2.02 packaged by Debian (2.02+dfsg1-20). It was not an output issue, but an input one.
I've managed to analyse the issue and found where it came from: the following commit: ---------- commit ---------- Commit: 216950a4eea1a1ead1c28eaca94e34ea2ef2ad19 Author: Vladimir Serbinenko <address@hidden> Date: Mon May 8 21:41:22 2017 +0200
at_keyboard: Split protocol from controller code.
On vexpress controller is different but protocol is the same, so reuse the code. ---------- end of commit ----------
3 lines where moved from the function 'fetch_key()' in file 'grub-core/term/at_keyboard.c', to the begining of function 'grub_at_keyboard_getkey()' (same file). But returning '-1' made sense when in function 'fetch_key()' but not anymore in function 'grub_at_keyboard_getkey()', which should return 'GRUB_TERM_NO_KEY'.
I think it was just an incomplete cut-paste, missing a small manual correction. Better fix it. ///////// end of commit /////////
Please, feel free to edit/cut the commit message as you wish.
> By the way, do you only have this problem on one machine or all? Can > it > be reproduced in QEMU?
Actually I only have that issue with my libvirt-qemu-kvm virtual machines (where I do grub tweaking and theming). I've never used qemu directly, only through libvirt and with a GUI (virt-manager).
But I can provide you the log of libvirt containing the qemu command, and the dump of my VM configuration (and some host informations). See below.
Hope this helps. If you need more informations, I'll be glad to dig.
> > PS: there is a typo in my introduction of my patch: it is at the > > line > > 237 and not 234 as mentioned. Do you think I should answer to my > > first > > email to say it (so people not reading this sub-thread will know)? > > You can fix that in the commit message. > > Please ask if you need further help.
Thank you very much for your help and kind support.