[Bugfix] Hang on new mac laptops

[Bugfix] Hang on new mac laptops

[phcoder]

Hello. On new mac laptops waiting for keyboard event causes random 
hangs. Bugfix attached
Thanks
Vladimir 'phcoder' Serbinenko

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 1967)
+++ ChangeLog	(working copy)
@@ -1,3 +1,10 @@
+2009-02-03  Vladimir Serbinenko <phcoder@gmail.com>
+
+	Don't rely on event for keyboard on EFI. It causes random hangs
+	Great thanks to Florian Idelberger for testing and helping finding 
this bug
+
+	* term/efi/console.c: Replace wait for event with grub_millisleep (10)
+	
  2009-02-01  Felix Zielcke  <fzielcke@z-51.de>

  	* INSTALL: Note that we now require at least autconf 2.59 and that LZO
Index: term/efi/console.c
===================================================================
--- term/efi/console.c	(revision 1967)
+++ term/efi/console.c	(working copy)
@@ -226,9 +226,10 @@

    do
      {
-      status = efi_call_3 (b->wait_for_event, 1, &(i->wait_for_key), 
&index);
+      /*      status = efi_call_3 (b->wait_for_event, 1, 
&(i->wait_for_key), &index);
        if (status != GRUB_EFI_SUCCESS)
-        return -1;
+        return -1;*/
+      grub_millisleep (10);

        grub_console_checkkey ();
      }

Re: [Bugfix] Hang on new mac laptops

[Pavel Roskin]

On Tue, 2009-02-03 at 22:30 +0100, phcoder wrote:
> -      status = efi_call_3 (b->wait_for_event, 1, &(i->wait_for_key), 
> &index);
> +      /*      status = efi_call_3 (b->wait_for_event, 1, 
> &(i->wait_for_key), &index);
>         if (status != GRUB_EFI_SUCCESS)
> -        return -1;
> +        return -1;*/
> +      grub_millisleep (10);

Please don't leave commented out code.  If you want a comment to be
there, you can write something.  If you want to preserve the file
history, Subversion does it already.

-- 
Regards,
Pavel Roskin

Re: [Bugfix] Hang on new mac laptops

[phcoder]

[-- Attachment #1: Type: text/plain, Size: 565 bytes --]

Fixed
Pavel Roskin wrote:
> On Tue, 2009-02-03 at 22:30 +0100, phcoder wrote:
>> -      status = efi_call_3 (b->wait_for_event, 1, &(i->wait_for_key), 
>> &index);
>> +      /*      status = efi_call_3 (b->wait_for_event, 1, 
>> &(i->wait_for_key), &index);
>>         if (status != GRUB_EFI_SUCCESS)
>> -        return -1;
>> +        return -1;*/
>> +      grub_millisleep (10);
> 
> Please don't leave commented out code.  If you want a comment to be
> there, you can write something.  If you want to preserve the file
> history, Subversion does it already.
> 


[-- Attachment #2: efikey.diff --]
[-- Type: text/x-diff, Size: 1069 bytes --]

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 1967)
+++ ChangeLog	(working copy)
@@ -1,3 +1,10 @@
+2009-02-04  Vladimir Serbinenko <phcoder@gmail.com>
+
+	Don't rely on event for keyboard on EFI. It causes random hangs
+	Great thanks to Florian Idelberger for testing and helping finding this bug 
+
+	* term/efi/console.c: Replace wait for event with grub_millisleep (10)
+	
 2009-02-01  Felix Zielcke  <fzielcke@z-51.de>
 
 	* INSTALL: Note that we now require at least autconf 2.59 and that LZO
Index: term/efi/console.c
===================================================================
--- term/efi/console.c	(revision 1967)
+++ term/efi/console.c	(working copy)
@@ -226,9 +226,9 @@
 
   do
     {
-      status = efi_call_3 (b->wait_for_event, 1, &(i->wait_for_key), &index);
-      if (status != GRUB_EFI_SUCCESS)
-        return -1;
+      /* Don't use wait_for_event it's known 
+	 to cause hangs on some mac models */
+      grub_millisleep (10);
       
       grub_console_checkkey ();
     }

Re: [Bugfix] Hang on new mac laptops

[Robert Millan]

On Wed, Feb 04, 2009 at 08:56:28AM +0100, phcoder wrote:
> -      status = efi_call_3 (b->wait_for_event, 1, &(i->wait_for_key), &index);
> -      if (status != GRUB_EFI_SUCCESS)
> -        return -1;
> +      /* Don't use wait_for_event it's known 
> +	 to cause hangs on some mac models */
> +      grub_millisleep (10);

I'm fine with the workaround, but please could you check if at_keyboard.mod
works there?  It's not the first time we have trouble due to buggy keyboard
drivers, I'd be very happy if we could avoid more of the same.

  insmod at_keyboard
  terminal_input at_keyboard

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

Re: [Bugfix] Hang on new mac laptops

[step21]

[-- Attachment #1: Type: text/plain, Size: 1457 bytes --]

I made an *.efi with a few modules (for convenience and cause the minimal
build somehow didn't want to load anything, but that could have been me)
Then I loaded at_keyboard with "insmod at_keyboard" that worked. Then when I
do "terminal_input at_keyboard" it doesn't give me a new prompt and seems to
"hang".
How to further debug this? What is expected behaviour?


On Sat, Feb 7, 2009 at 11:34 PM, Robert Millan <rmh@aybabtu.com> wrote:

> On Wed, Feb 04, 2009 at 08:56:28AM +0100, phcoder wrote:
> > -      status = efi_call_3 (b->wait_for_event, 1, &(i->wait_for_key),
> &index);
> > -      if (status != GRUB_EFI_SUCCESS)
> > -        return -1;
> > +      /* Don't use wait_for_event it's known
> > +      to cause hangs on some mac models */
> > +      grub_millisleep (10);
>
> I'm fine with the workaround, but please could you check if at_keyboard.mod
> works there?  It's not the first time we have trouble due to buggy keyboard
> drivers, I'd be very happy if we could avoid more of the same.
>
>  insmod at_keyboard
>  terminal_input at_keyboard
>
> --
> 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."
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 2136 bytes --]

Re: [Bugfix] Hang on new mac laptops

[Robert Millan]

On Sun, Feb 08, 2009 at 06:02:27PM +0100, step21 wrote:
> I made an *.efi with a few modules (for convenience and cause the minimal
> build somehow didn't want to load anything, but that could have been me)
> Then I loaded at_keyboard with "insmod at_keyboard" that worked. Then when I
> do "terminal_input at_keyboard" it doesn't give me a new prompt and seems to
> "hang".
> How to further debug this?

Before insmod:

  set debug=at_keyboard

and check the output (of both commands)

> What is expected behaviour?

Expected behaviour is that your prompt doesn't hang like you described,
but lets you continue typing instead.

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

Re: [Bugfix] Hang on new mac laptops

[step21]

[-- Attachment #1: Type: text/plain, Size: 1226 bytes --]

ok makes sense. I did set debug=at_keyboard (before insmod I think)
but not output at all.


On Sun, Feb 8, 2009 at 7:00 PM, Robert Millan <rmh@aybabtu.com> wrote:

> On Sun, Feb 08, 2009 at 06:02:27PM +0100, step21 wrote:
> > I made an *.efi with a few modules (for convenience and cause the minimal
> > build somehow didn't want to load anything, but that could have been me)
> > Then I loaded at_keyboard with "insmod at_keyboard" that worked. Then
> when I
> > do "terminal_input at_keyboard" it doesn't give me a new prompt and seems
> to
> > "hang".
> > How to further debug this?
>
> Before insmod:
>
>  set debug=at_keyboard
>
> and check the output (of both commands)
>
> > What is expected behaviour?
>
> Expected behaviour is that your prompt doesn't hang like you described,
> but lets you continue typing instead.
>
> --
> 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."
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 1888 bytes --]

Re: [Bugfix] Hang on new mac laptops

[Robert Millan]

On Sun, Feb 08, 2009 at 07:52:39PM +0100, step21 wrote:
> ok makes sense. I did set debug=at_keyboard (before insmod I think)
> but not output at all.

Sorry, it should have been debug=atkeyb.

Also, you need a #define DEBUG_AT_KEYBOARD line as mentioned on IRC.

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

Re: [Bugfix] Hang on new mac laptops

[step21]

[-- Attachment #1: Type: text/plain, Size: 1250 bytes --]

Ok, confirmed that keyboard is attached with usb. lsusb wans't very helpful
cause apart from wireless and bluetooth it only mentioned "apple devices"
and "linux foundation devices" however os x has a similiar command line tool
ioreg which with -p IOUSB can be restricted to usb. Output
http://pastebin.com/m4d3e9c83
I then updated to the usb patch with rev 1985, and compiled etc. debug still
seemed to do nothing and as previously terminal_input at_keyboard forces to
manually swithc the machine off.


On Sun, Feb 8, 2009 at 8:55 PM, Robert Millan <rmh@aybabtu.com> wrote:

> On Sun, Feb 08, 2009 at 07:52:39PM +0100, step21 wrote:
> > ok makes sense. I did set debug=at_keyboard (before insmod I think)
> > but not output at all.
>
> Sorry, it should have been debug=atkeyb.
>
> Also, you need a #define DEBUG_AT_KEYBOARD line as mentioned on IRC.
>
> --
> 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."
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 1892 bytes --]

Re: [Bugfix] Hang on new mac laptops

[Robert Millan]

On Mon, Feb 09, 2009 at 02:46:01AM +0100, step21 wrote:
> Ok, confirmed that keyboard is attached with usb. lsusb wans't very helpful
> cause apart from wireless and bluetooth it only mentioned "apple devices"
> and "linux foundation devices" however os x has a similiar command line tool
> ioreg which with -p IOUSB can be restricted to usb. Output
> http://pastebin.com/m4d3e9c83
> I then updated to the usb patch with rev 1985, and compiled etc. debug still
> seemed to do nothing and as previously terminal_input at_keyboard forces to
> manually swithc the machine off.

Use the following:

  insmod uhci ; insmod usb_keyboard ; terminal_input usb_keyboard

It is important to use semicolons, since after "insmod uhci" your keyboard
will stop working (and hopefully be restored with "terminal_input usb_keyboard")

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

Re: [Bugfix] Hang on new mac laptops

[phcoder]

Hello this issue still persists and unfortunately usb keyboard didn't 
fix the issue. Perhaps we should commit this as at least temporary 
solution for the problem?
phcoder wrote:
> Fixed
> Pavel Roskin wrote:
>> On Tue, 2009-02-03 at 22:30 +0100, phcoder wrote:
>>> -      status = efi_call_3 (b->wait_for_event, 1, &(i->wait_for_key), 
>>> &index);
>>> +      /*      status = efi_call_3 (b->wait_for_event, 1, 
>>> &(i->wait_for_key), &index);
>>>         if (status != GRUB_EFI_SUCCESS)
>>> -        return -1;
>>> +        return -1;*/
>>> +      grub_millisleep (10);
>>
>> Please don't leave commented out code.  If you want a comment to be
>> there, you can write something.  If you want to preserve the file
>> history, Subversion does it already.
>>
> 


-- 

Regards
Vladimir 'phcoder' Serbinenko