Re: Patch that fixes an 'at_keyboard' module issue (unreliable key presses)

[Paul Menzel]

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?

By the way, do you only have this problem on one machine or all? Can it
be reproduced in QEMU?

> 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.


Kind regards,

Paul


>> [1]: 
>> https://git.savannah.gnu.org/cgit/grub.git/commit/?id=d3a3543a5666c1dd180ae6027948ca753dcffc18

smime.p7s
Description: S/MIME Cryptographic Signature