[PATCH 2/2] watchdog: add watchdog behavior configuration

Mark Kettenis
2020-09-23T17:01:54Z

> From: Michael Walle <michael@walle.cc>
> Date: Wed, 23 Sep 2020 18:45:27 +0200
>
> Let the user choose between three different behaviours of the watchdog:
> (1) Keep the watchdog disabled
> (2) Supervise u-boot
> (3) Supervise u-boot and the operating systen (default)
>
> Option (2) will disable the watchdog right before handing control to the
> operating system. This is useful when the OS is not aware of the
> watchdog. Option (3) doesn't disable the watchdog and assumes the OS
> will continue servicing.

(3) can't be the default, at least for EFI

The UEFI standard explicitly says that upon calling
ExitBootServices(), the watchdog timer is disabled.

In general, you can't expect an OS to have support for a particular
watchdog timer. So (3) only makes sense in cases where U-Boot is
bundled with an OS image.

>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> cmd/boot.c | 7 +++++++
> cmd/bootefi.c | 7 +++++++
> cmd/efidebug.c | 7 +++++++
> common/board_r.c | 2 +-
> common/bootm_os.c | 5 +++++
> common/spl/spl.c | 6 +++---
> drivers/watchdog/Kconfig | 24 ++++++++++++++++++++++++
> drivers/watchdog/wdt-uclass.c | 33 +++++++++++++++++++++++++--------
> include/wdt.h | 17 +++++++++++++++++
> 9 files changed, 96 insertions(+), 12 deletions(-)
>
> diff --git a/cmd/boot.c b/cmd/boot.c
> index 36aba22b30..2412410371 100644
> --- a/cmd/boot.c
> +++ b/cmd/boot.c
> @@ -10,6 +10,7 @@
> #include <common.h>
> #include <command.h>
> #include <net.h>
> +#include <wdt.h>
>
> #ifdef CONFIG_CMD_GO
>
> @@ -33,6 +34,9 @@ static int do_go(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>
> printf ("## Starting application at 0x%08lX ...\n", addr);
>
> + if (IS_ENABLED(CONFIG_WATCHDOG_SUPERVISE_U_BOOT))
> + stop_watchdog();
> +
> /*
> * pass address parameter as argv[0] (aka command name),
> * and all remaining args
> @@ -40,6 +44,9 @@ static int do_go(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> rc = do_go_exec ((void *)addr, argc - 1, argv + 1);
> if (rc != 0) rcode = 1;
>
> + if (IS_ENABLED(CONFIG_WATCHDOG_SUPERVISE_U_BOOT))
> + start_watchdog();
> +
> printf ("## Application terminated, rc = 0x%lX\n", rc);
> return rcode;
> }
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 40d5ef2b3a..21f6650174 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -24,6 +24,7 @@
> #include <memalign.h>
> #include <asm-generic/sections.h>
> #include <linux/linkage.h>
> +#include <wdt.h>
>
> DECLARE_GLOBAL_DATA_PTR;
>
> @@ -320,6 +321,9 @@ static efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options)
> efi_uintn_t exit_data_size = 0;
> u16 *exit_data = NULL;
>
> + if (IS_ENABLED(CONFIG_WATCHDOG_SUPERVISE_U_BOOT))
> + stop_watchdog();
> +
> /* Call our payload! */
> ret = EFI_CALL(efi_start_image(handle, &exit_data_size, &exit_data));
> if (ret != EFI_SUCCESS) {
> @@ -333,6 +337,9 @@ static efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options)
>
> efi_restore_gd();
>
> + if (IS_ENABLED(CONFIG_WATCHDOG_SUPERVISE_U_BOOT))
> + start_watchdog();
> +
> free(load_options);
>
> return ret;
> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> index 9874838b00..5fa0cd1df7 100644
> --- a/cmd/efidebug.c
> +++ b/cmd/efidebug.c
> @@ -16,6 +16,7 @@
> #include <mapmem.h>
> #include <search.h>
> #include <linux/ctype.h>
> +#include <wdt.h>
>
> #define BS systab.boottime
>
> @@ -1131,6 +1132,9 @@ static int do_efi_test_bootmgr(struct cmd_tbl *cmdtp, int flag,
> ret = efi_bootmgr_load(&image, &load_options);
> printf("efi_bootmgr_load() returned: %ld\n", ret & ~EFI_ERROR_MASK);
>
> + if (IS_ENABLED(CONFIG_WATCHDOG_SUPERVISE_U_BOOT))
> + stop_watchdog();
> +
> /* We call efi_start_image() even if error for test purpose. */
> ret = EFI_CALL(efi_start_image(image, &exit_data_size, &exit_data));
> printf("efi_start_image() returned: %ld\n", ret & ~EFI_ERROR_MASK);
> @@ -1139,6 +1143,9 @@ static int do_efi_test_bootmgr(struct cmd_tbl *cmdtp, int flag,
>
> efi_restore_gd();
>
> + if (IS_ENABLED(CONFIG_WATCHDOG_SUPERVISE_U_BOOT))
> + start_watchdog();
> +
> free(load_options);
> return CMD_RET_SUCCESS;
> }
> diff --git a/common/board_r.c b/common/board_r.c
> index 9b2fec701a..6734d3ad25 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -731,7 +731,7 @@ static init_fnc_t init_sequence_r[] = {
> stdio_init_tables,
> serial_initialize,
> initr_announce,
> -#if CONFIG_IS_ENABLED(WDT)
> +#if !IS_ENABLED(CONFIG_WATCHDOG_SUPERVISE_NOTHING)
> initr_watchdog,
> #endif
> INIT_FUNC_WATCHDOG_RESET
> diff --git a/common/bootm_os.c b/common/bootm_os.c
> index e9aaddf3e6..2b57a4e02c 100644
> --- a/common/bootm_os.c
> +++ b/common/bootm_os.c
> @@ -19,6 +19,7 @@
> #include <mapmem.h>
> #include <vxworks.h>
> #include <tee/optee.h>
> +#include <wdt.h>
>
> DECLARE_GLOBAL_DATA_PTR;
>
> @@ -612,6 +613,10 @@ int boot_selected_os(int argc, char *const argv[], int state,
> {
> arch_preboot_os();
> board_preboot_os();
> +
> + if (IS_ENABLED(CONFIG_WATCHDOG_SUPERVISE_U_BOOT))
> + stop_watchdog();
> +
> boot_fn(state, argc, argv, images);
>
> /* Stand-alone may return when 'autostart' is 'no' */
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index 4840d1d367..63c47c739c 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -639,9 +639,9 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
> spl_board_init();
> #endif
>
> -#if defined(CONFIG_SPL_WATCHDOG_SUPPORT) && CONFIG_IS_ENABLED(WDT)
> - initr_watchdog();
> -#endif
> + if (IS_ENABLED(CONFIG_SPL_WATCHDOG_SUPPORT) &&
> + !IS_ENABLED(CONFIG_WATCHDOG_SUPERVISE_NOTHING))
> + initr_watchdog();
>
> if (IS_ENABLED(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF) ||
> IS_ENABLED(CONFIG_SPL_ATF))
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 4532a40e45..861a0e012d 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1,5 +1,29 @@
> menu "Watchdog Timer Support"
>
> +choice
> + prompt "Watchdog behavior"
> + default WATCHDOG_SUPERVISE_OS
> + depends on WDT
> +
> +config WATCHDOG_SUPERVISE_NOTHING
> + bool "Supervise nothing"
> + help
> + No watchdog will be started.
> +
> +config WATCHDOG_SUPERVISE_U_BOOT
> + bool "Supervise U-Boot only"
> + help
> + Upon U-Boot startup the first watchdog will be started automatically
> + and stopped as soon as an operating system is booted.
> +
> +config WATCHDOG_SUPERVISE_OS
> + bool "Supervise U-boot and operating system"
> + help
> + Upon U-Boot startup the first watchdog will be started automatically
> + and kept running even after booting the operating system.
> + Be aware, that the operating system needs to service the watchdog!
> +endchoice
> +
> config WATCHDOG
> bool "Enable U-Boot watchdog reset"
> depends on !HW_WATCHDOG
> diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
> index e632f077f3..ba3f8dde11 100644
> --- a/drivers/watchdog/wdt-uclass.c
> +++ b/drivers/watchdog/wdt-uclass.c
> @@ -22,11 +22,10 @@ DECLARE_GLOBAL_DATA_PTR;
> * hw_margin_ms property.
> */
> static ulong reset_period = 1000;
> +static ulong wdt_timeout = WATCHDOG_TIMEOUT_SECS;
>
> int initr_watchdog(void)
> {
> - u32 timeout = WATCHDOG_TIMEOUT_SECS;
> -
> /*
> * Init watchdog: This will call the probe function of the
> * watchdog driver, enabling the use of the device
> @@ -42,21 +41,39 @@ int initr_watchdog(void)
> }
>
> if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)) {
> - timeout = dev_read_u32_default(gd->watchdog_dev, "timeout-sec",
> - WATCHDOG_TIMEOUT_SECS);
> + wdt_timeout = dev_read_u32_default(gd->watchdog_dev,
> + "timeout-sec",
> + WATCHDOG_TIMEOUT_SECS);
> reset_period = dev_read_u32_default(gd->watchdog_dev,
> "hw_margin_ms",
> 4 * reset_period) / 4;
> }
>
> - wdt_start(gd->watchdog_dev, timeout * 1000, 0);
> - gd->flags |= GD_FLG_WDT_READY;
> - printf("WDT: Started with%s servicing (%ds timeout)\n",
> - IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", timeout);
> + start_watchdog();
> + printf("WDT: Started with%s servicing (supervising %s, %lus timeout)\n",
> + IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out",
> + IS_ENABLED(CONFIG_WATCHDOG_SUPERVISE_U_BOOT) ? "U-Boot" : "OS",
> + wdt_timeout);
>
> return 0;
> }
>
> +void start_watchdog(void)
> +{
> + if (gd->watchdog_dev) {
> + wdt_start(gd->watchdog_dev, wdt_timeout * 1000, 0);
> + gd->flags |= GD_FLG_WDT_READY;
> + }
> +}
> +
> +void stop_watchdog(void)
> +{
> + if (gd->watchdog_dev) {
> + wdt_stop(gd->watchdog_dev);
> + gd->flags &= ~GD_FLG_WDT_READY;
> + }
> +}
> +
> int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
> {
> const struct wdt_ops *ops = device_get_ops(dev);
> diff --git a/include/wdt.h b/include/wdt.h
> index bc242c2eb2..d261cd0dd6 100644
> --- a/include/wdt.h
> +++ b/include/wdt.h
> @@ -105,6 +105,23 @@ struct wdt_ops {
> int (*expire_now)(struct udevice *dev, ulong flags);
> };
>
> +/*
> + * Initialize and start the global watchdog timer.
> + *
> + * @return: 0 if OK, -ve on error
> + */
> int initr_watchdog(void);
>
> +/*
> + * (Re)start the global watchdog timer. Used after returning from an EFI or
> + * application image.
> + */
> +void start_watchdog(void);
> +
> +/*
> + * Stop the global watchdog timer. Used before handing control to an EFI image
> + * or operating system.
> + */
> +void stop_watchdog(void);
> +
> #endif /* _WDT_H_ */
> --
> 2.20.1
>
>