grub-devel.gnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Remove unconditional disablement of the watchdog
@ 2015-09-01 14:51 Arthur Mesh
  2015-09-08 16:42 ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 1 reply; 3+ messages in thread
From: Arthur Mesh @ 2015-09-01 14:51 UTC (permalink / raw)
  To: grub-devel

[-- Attachment #1: Type: text/plain, Size: 4067 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.

Remove the unconditional disablement of the watchdog, and create a command line
interface to enable/disable watchdog:

 efi-watchdog (enable|disable) <timeout>
---
 docs/grub.texi            |    8 ++++++
 grub-core/kern/efi/init.c |   60 ++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 65 insertions(+), 3 deletions(-)

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..f8a79eb 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,10 @@ grub_efi_init (void)
   /* Initialize the memory management system.  */
   grub_efi_mm_init ();
 
-  efi_call_4 (grub_efi_system_table->boot_services->set_watchdog_timer,
-	      0, 0, 0, NULL);
-
   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 +130,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] 3+ messages in thread

* Re: [PATCH] Remove unconditional disablement of the watchdog
  2015-09-01 14:51 [PATCH] Remove unconditional disablement of the watchdog Arthur Mesh
@ 2015-09-08 16:42 ` Vladimir 'φ-coder/phcoder' Serbinenko
  2015-09-09 15:53   ` Arthur Mesh
  0 siblings, 1 reply; 3+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2015-09-08 16:42 UTC (permalink / raw)
  To: The development of GNU GRUB

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

On 01.09.2015 07:51, Arthur Mesh wrote:
> 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.
> 
> Remove the unconditional disablement of the watchdog, and create a command line
> interface to enable/disable watchdog:
> 
This patch will break any current config on systems with watchdog. You
need to ensure that:
a) watchdog timeout is plenty to load config file even on very slow
netboot systems.
b) disable watchdog if the config didn't opt-in to have the watchdog.
>  efi-watchdog (enable|disable) <timeout>
> ---
>  docs/grub.texi            |    8 ++++++
>  grub-core/kern/efi/init.c |   60 ++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 65 insertions(+), 3 deletions(-)
> 
> 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..f8a79eb 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,10 @@ grub_efi_init (void)
>    /* Initialize the memory management system.  */
>    grub_efi_mm_init ();
>  
> -  efi_call_4 (grub_efi_system_table->boot_services->set_watchdog_timer,
> -	      0, 0, 0, NULL);
> -
>    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 +130,5 @@ grub_efi_fini (void)
>  {
>    grub_efidisk_fini ();
>    grub_console_fini ();
> +  grub_unregister_command (cmd_list);
>  }
> 
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

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

* Re: [PATCH] Remove unconditional disablement of the watchdog
  2015-09-08 16:42 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2015-09-09 15:53   ` Arthur Mesh
  0 siblings, 0 replies; 3+ messages in thread
From: Arthur Mesh @ 2015-09-09 15:53 UTC (permalink / raw)
  To: The development of GNU GRUB

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

On Tue, Sep 08, 2015 at 09:42:08AM -0700, Vladimir '??-coder/phcoder' Serbinenko wrote:
> This patch will break any current config on systems with watchdog.

That's true.

> You  need to ensure that:
> a) watchdog timeout is plenty to load config file even on very slow
> netboot systems.

Is there a way to achieve that programaticly and not defeat the purpose
of the watchdog? I tend to think the answer is no.

> b) disable watchdog if the config didn't opt-in to have the watchdog.

How about we leave the disabling of watchdog as is today, and instead
provide means to re-enabling it via cfg?

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] 3+ messages in thread

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

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-01 14:51 [PATCH] Remove unconditional disablement of the watchdog Arthur Mesh
2015-09-08 16:42 ` Vladimir 'φ-coder/phcoder' Serbinenko
2015-09-09 15:53   ` 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 NNTP newsgroup(s).