grub-devel.gnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Make EFI watchdog behaviour configurable
@ 2015-09-10  0:11 Arthur Mesh
  2015-09-11 14:32 ` Andrei Borzenkov
  0 siblings, 1 reply; 5+ messages in thread
From: Arthur Mesh @ 2015-09-10  0:11 UTC (permalink / raw
  To: grub-devel

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

Starting with d9a0c9413e81d3c0affc6383693bdd28dc863a5c, GRUB unconditionally
disables watchdog on EFI platforms. This opens up a window (starting at GRUB's
grub_efi_init(), until OS re-enables it) when EFI system operates w/o watchdog.
If an EFI system gets stuck in that window, the chipset will never reset the
system.

Add `--enable-efi-watchdog' configure argument, which defaults to Off.
When enabled, efi watchdog will not be disabled.
Otherwise, efi watchdog will be disabled.

Create a command line interface to enable/disable watchdog:
 efi-watchdog (enable|disable) <timeout>
---
 configure.ac              |   18 +++++++++++++-
 docs/grub.texi            |    8 ++++++
 grub-core/kern/efi/init.c |   59 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index c864311..91de439 100644
--- a/configure.ac
+++ b/configure.ac
@@ -211,7 +211,10 @@ esac
 case "$platform" in
   coreboot)	machine_CPPFLAGS="$machine_CPPFLAGS -DGRUB_MACHINE_COREBOOT=1" ;;
   multiboot)	machine_CPPFLAGS="$machine_CPPFLAGS -DGRUB_MACHINE_MULTIBOOT=1" ;;
-  efi)		machine_CPPFLAGS="$machine_CPPFLAGS -DGRUB_MACHINE_EFI=1" ;;
+  efi)		machine_CPPFLAGS="$machine_CPPFLAGS -DGRUB_MACHINE_EFI=1"
+	if test x"$enable_efi_watchdog" = xyes ; then
+		machine_CPPFLAGS="$machine_CPPFLAGS -DEFI_WATCHDOG_LEAVE_ALONE=1"
+	fi ;;
   xen)		machine_CPPFLAGS="$machine_CPPFLAGS -DGRUB_MACHINE_XEN=1" ;;
   ieee1275)	machine_CPPFLAGS="$machine_CPPFLAGS -DGRUB_MACHINE_IEEE1275=1" ;;
   uboot)	machine_CPPFLAGS="$machine_CPPFLAGS -DGRUB_MACHINE_UBOOT=1" ;;
@@ -1705,6 +1708,10 @@ if test x"$enable_werror" != xno ; then
   HOST_CFLAGS="$HOST_CFLAGS -Werror"
 fi
 
+AC_ARG_ENABLE([efi-watchdog],
+	      [AS_HELP_STRING([--enable-efi-watchdog],
+                             [Do not explicitly disable EFI watchdog])])
+
 TARGET_CPP="$TARGET_CC -E"
 TARGET_CCAS=$TARGET_CC
 
@@ -1899,6 +1906,15 @@ echo efiemu runtime: Yes
 else
 echo efiemu runtime: No "($efiemu_excuse)"
 fi
+
+if [ x"$platform" = xefi ]; then
+if [ x"$enable_efi_watchdog" != xyes ]; then
+echo disable EFI watchdog: Yes
+else
+echo disable EFI watchdog: No
+fi
+fi
+
 if [ x"$grub_mkfont_excuse" = x ]; then
 echo grub-mkfont: Yes
 else
diff --git a/docs/grub.texi b/docs/grub.texi
index b9f41a7..4cfd50c 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -3784,6 +3784,7 @@ you forget a command, you can run the command @command{help}
 * distrust::                    Remove a pubkey from trusted keys
 * drivemap::                    Map a drive to another
 * echo::                        Display a line of text
+* efi-watchdog::                Manipulate EFI watchdog
 * eval::                        Evaluate agruments as GRUB commands
 * export::                      Export an environment variable
 * false::                       Do nothing, unsuccessfully
@@ -4192,6 +4193,13 @@ When interpreting backslash escapes, backslash followed by any other
 character will print that character.
 @end deffn
 
+@node efi-watchdog
+@subsection efi-watchdog
+
+@deffn Command efi-watchdog enable|disable <timeout>
+Enable or disable the system's watchdog timer. Only available in EFI targeted
+GRUB.
+@end deffn
 
 @node eval
 @subsection eval
diff --git a/grub-core/kern/efi/init.c b/grub-core/kern/efi/init.c
index e9c85de..47053d2 100644
--- a/grub-core/kern/efi/init.c
+++ b/grub-core/kern/efi/init.c
@@ -25,9 +25,62 @@
 #include <grub/env.h>
 #include <grub/mm.h>
 #include <grub/kernel.h>
+#include <grub/extcmd.h>
+#include <grub/command.h>
 
 grub_addr_t grub_modbase;
 
+static grub_command_t cmd_list;
+
+static grub_err_t
+grub_cmd_efi_watchdog (grub_command_t cmd  __attribute__ ((unused)),
+		       int argc, char **args)
+{
+    long input;
+    grub_efi_status_t status;
+    grub_efi_uintn_t timeout;
+
+    if (argc < 1)
+	return grub_error (GRUB_ERR_BAD_ARGUMENT,
+	    N_("efi-watchdog (enable|disable) <timeout>"));
+
+    if (grub_strcasecmp (args[0], "enable") == 0) {
+
+	if (argc != 2)
+	    return grub_error (GRUB_ERR_BAD_ARGUMENT,
+			       N_("efi-watchdog enable <timeout>"));
+
+	input = grub_strtol (args[1], 0, 0);
+
+	if (input >= 0) {
+	    timeout = input;
+	} else {
+	    return grub_error (GRUB_ERR_BAD_ARGUMENT,
+			       N_("<timeout> must be non-negative"));
+	}
+
+    } else if (grub_strcasecmp (args[0], "disable") == 0) {
+
+	if (argc != 1)
+	    return grub_error (GRUB_ERR_BAD_ARGUMENT,
+			       N_("efi-watchdog disable"));
+	timeout = 0;
+
+    } else {
+	return grub_error (GRUB_ERR_BAD_ARGUMENT,
+	    N_("efi-watchdog (enable|disable) <timeout>"));
+    }
+
+    status = efi_call_4 (grub_efi_system_table->boot_services->set_watchdog_timer,
+			 timeout, 0, sizeof(L"GRUB"), L"GRUB");
+
+    if (status != GRUB_EFI_SUCCESS)
+	return grub_error (GRUB_ERR_BUG,
+			   N_("EFI SetWatchdogTimer() bug"));
+    else
+	return GRUB_ERR_NONE;
+}
+
 void
 grub_efi_init (void)
 {
@@ -39,10 +92,15 @@ grub_efi_init (void)
   /* Initialize the memory management system.  */
   grub_efi_mm_init ();
 
+#ifndef EFI_WATCHDOG_LEAVE_ALONE
   efi_call_4 (grub_efi_system_table->boot_services->set_watchdog_timer,
 	      0, 0, 0, NULL);
+#endif
 
   grub_efidisk_init ();
+
+  cmd_list = grub_register_command ("efi-watchdog", grub_cmd_efi_watchdog, 0,
+				    N_("Enable/Disable system's watchdog timer."));
 }
 
 void (*grub_efi_net_config) (grub_efi_handle_t hnd, 
@@ -77,4 +135,5 @@ grub_efi_fini (void)
 {
   grub_efidisk_fini ();
   grub_console_fini ();
+  grub_unregister_command (cmd_list);
 }
-- 
1.7.9.5

[-- Attachment #2: Type: application/pgp-signature, Size: 648 bytes --]

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] Make EFI watchdog behaviour configurable
  2015-09-10  0:11 [PATCH] Make EFI watchdog behaviour configurable Arthur Mesh
@ 2015-09-11 14:32 ` Andrei Borzenkov
  2015-09-11 14:53   ` Arthur Mesh
  0 siblings, 1 reply; 5+ messages in thread
From: Andrei Borzenkov @ 2015-09-11 14:32 UTC (permalink / raw
  To: The development of GNU GRUB

10.09.2015 03:11, Arthur Mesh пишет:
> Starting with d9a0c9413e81d3c0affc6383693bdd28dc863a5c, GRUB unconditionally
> disables watchdog on EFI platforms. This opens up a window (starting at GRUB's
> grub_efi_init(), until OS re-enables it) when EFI system operates w/o watchdog.
> If an EFI system gets stuck in that window, the chipset will never reset the
> system.
>
> Add `--enable-efi-watchdog' configure argument, which defaults to Off.
> When enabled, efi watchdog will not be disabled.
> Otherwise, efi watchdog will be disabled.
>

Please, no. This will result in incompatible binaries without any 
indication at run-time that allows to distinguish between them.

> Create a command line interface to enable/disable watchdog:
>   efi-watchdog (enable|disable) <timeout>

Why is it not enough? You can always add it to your grub.cfg.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Make EFI watchdog behaviour configurable
  2015-09-11 14:32 ` Andrei Borzenkov
@ 2015-09-11 14:53   ` Arthur Mesh
  2015-09-11 15:24     ` Andrei Borzenkov
  0 siblings, 1 reply; 5+ messages in thread
From: Arthur Mesh @ 2015-09-11 14:53 UTC (permalink / raw
  To: The development of GNU GRUB






On 9/11/15, 7:32 AM, "grub-devel-bounces+amesh=juniper.net@gnu.org on behalf of Andrei Borzenkov" <grub-devel-bounces+amesh=juniper.net@gnu.org on behalf of arvidjaar@gmail.com> wrote:

>10.09.2015 03:11, Arthur Mesh пишет:
>> Add `--enable-efi-watchdog' configure argument, which defaults to Off.
>> When enabled, efi watchdog will not be disabled.
>> Otherwise, efi watchdog will be disabled.
>>
>
>Please, no. This will result in incompatible binaries without any 
>indication at run-time that allows to distinguish between them.

That's true, but I see no way around that w/o avoiding the initial call to 
disable the watchdog in the efi init routine (and Vladimir is against that idea).

>
>> Create a command line interface to enable/disable watchdog:
>>   efi-watchdog (enable|disable) <timeout>
>
>Why is it not enough? You can always add it to your grub.cfg.

Well, it still leaves a window of time where the system operates w/o a watchdog.
It could be a problem if grub.cfg resides on a network file system, and is not
accessible, in which case the system would hang.

Spasibo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Make EFI watchdog behaviour configurable
  2015-09-11 14:53   ` Arthur Mesh
@ 2015-09-11 15:24     ` Andrei Borzenkov
  2015-09-11 15:57       ` Arthur Mesh
  0 siblings, 1 reply; 5+ messages in thread
From: Andrei Borzenkov @ 2015-09-11 15:24 UTC (permalink / raw
  To: The development of GNU GRUB

11.09.2015 17:53, Arthur Mesh пишет:
>
>
>
>
>
> On 9/11/15, 7:32 AM, "grub-devel-bounces+amesh=juniper.net@gnu.org on behalf of Andrei Borzenkov" <grub-devel-bounces+amesh=juniper.net@gnu.org on behalf of arvidjaar@gmail.com> wrote:
>
>> 10.09.2015 03:11, Arthur Mesh пишет:
>>> Add `--enable-efi-watchdog' configure argument, which defaults to Off.
>>> When enabled, efi watchdog will not be disabled.
>>> Otherwise, efi watchdog will be disabled.
>>>
>>
>> Please, no. This will result in incompatible binaries without any
>> indication at run-time that allows to distinguish between them.
>
> That's true, but I see no way around that w/o avoiding the initial call to
> disable the watchdog in the efi init routine (and Vladimir is against that idea).
>
>>
>>> Create a command line interface to enable/disable watchdog:
>>>    efi-watchdog (enable|disable) <timeout>
>>
>> Why is it not enough? You can always add it to your grub.cfg.
>
> Well, it still leaves a window of time where the system operates w/o a watchdog.
> It could be a problem if grub.cfg resides on a network file system, and is not
> accessible, in which case the system would hang.

I'm fine with making it controlled by parameter passed to grub 
invocation or EFI variable. We discussed parameters several times but 
did not come to definitive conclusion hos interface should look like.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Make EFI watchdog behaviour configurable
  2015-09-11 15:24     ` Andrei Borzenkov
@ 2015-09-11 15:57       ` Arthur Mesh
  0 siblings, 0 replies; 5+ messages in thread
From: Arthur Mesh @ 2015-09-11 15:57 UTC (permalink / raw
  To: The development of GNU GRUB

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

On Fri, Sep 11, 2015 at 06:24:30PM +0300, Andrei Borzenkov wrote:
> >> Why is it not enough? You can always add it to your grub.cfg.
> >
> > Well, it still leaves a window of time where the system operates w/o a watchdog.
> > It could be a problem if grub.cfg resides on a network file system, and is not
> > accessible, in which case the system would hang.
> 
> I'm fine with making it controlled by parameter passed to grub 
> invocation or EFI variable. We discussed parameters several times but 
> did not come to definitive conclusion hos interface should look like.

BIOS wouldn't know when to tell GRUB to disable the watchdog. If it did,
BIOS wouldn't have to enable the watchdog to begin with.

EFI variables is a better idea, but unless we have a well defined
interface, it will get unwieldy very quickly..

Can we compromise and simply proceed with the efi-watchdog cli? If so,
I'll gladly submit a new patch.

Thanks


-- 
Arthur Mesh <amesh@juniper.net>
Juniper Networks
+1 408 936-4968

[-- Attachment #2: Type: application/pgp-signature, Size: 648 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-09-11 15:58 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-10  0:11 [PATCH] Make EFI watchdog behaviour configurable Arthur Mesh
2015-09-11 14:32 ` Andrei Borzenkov
2015-09-11 14:53   ` Arthur Mesh
2015-09-11 15:24     ` Andrei Borzenkov
2015-09-11 15:57       ` Arthur Mesh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).