Currently, AUTOBOOT_KEYED and its variant AUTOBOOT_ENCRYPTION are broken when one has an external always-running watchdog device with a timeout shorter than the configured boot delay (in my case, I have a gpio-wdt one with a timeout of 1 second), because we fail to call WATCHDOG_RESET() in the loops where we wait for the bootdelay to elapse.

This is done implicitly in the !AUTOBOOT_KEYED case, i.e. abortboot_single_key(), because that loop contains a udelay(10000), and udelay() does a WATCHDOG_RESET().

To fix this, simply add similar udelay() calls in the other loops.

Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk --- common/autoboot.c | 3 +++ 1 file changed, 3 insertions(+)

diff --git a/common/autoboot.c b/common/autoboot.c index 63f2587941..cdafe76309 100644 --- a/common/autoboot.c +++ b/common/autoboot.c @@ -115,6 +115,7 @@ static int passwd_abort_crypt(uint64_t etime) presskey_len++; } } + udelay(10000); } while (never_timeout || get_ticks() <= etime);

return abort; @@ -206,6 +207,7 @@ static int passwd_abort_sha256(uint64_t etime) if (slow_equals(sha, sha_env, SHA256_SUM_LEN)) abort = 1; } + udelay(10000); } while (!abort && get_ticks() <= etime);

free(presskey); @@ -293,6 +295,7 @@ static int passwd_abort_key(uint64_t etime) abort = 1; } } + udelay(10000); } while (!abort && get_ticks() <= etime);

return abort;

In order to test that U-Boot actually maintains the watchdog device(s) during long-running busy-loops, such as those where we wait for the user to stop autoboot, we need a watchdog device that actually does something during those loops; we cannot test that behaviour via the DM test framework.

So introduce a relatively simple watchdog device which is simply based on calling the host OS' alarm() function; that has the nice property that a new call to alarm() simply sets a new deadline, and alarm(0) cancels any existing alarm. These properties are precisely what we need to implement start/reset/stop. We install our own handler so that we get a known message printed if and when the watchdog fires, and by just invoking that handler directly, we get expire_now for free.

The actual calls to the various OS functions (alarm, signal, raise) need to be done in os.c, and since the driver code cannot get access to the values of SIGALRM or SIG_DFL (that would require including a host header, and that's only os.c which can do that), we cannot simply do trivial wrappers for signal() and raise(), but instead create specialized functions just for use by this driver.

Apart from enabling this driver for sandbox{,64}_defconfig, also enable the wdt command which was useful for hand-testing this new driver (especially with running u-boot under strace).

Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk --- arch/sandbox/cpu/os.c | 17 ++++++ configs/sandbox64_defconfig | 2 + configs/sandbox_defconfig | 2 + drivers/watchdog/Kconfig | 8 +++ drivers/watchdog/Makefile | 1 + drivers/watchdog/sandbox_alarm-wdt.c | 79 ++++++++++++++++++++++++++++ include/os.h | 17 ++++++ 7 files changed, 126 insertions(+) create mode 100644 drivers/watchdog/sandbox_alarm-wdt.c

diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c index f937991139..4e8953b4d4 100644 --- a/arch/sandbox/cpu/os.c +++ b/arch/sandbox/cpu/os.c @@ -130,6 +130,23 @@ void os_exit(int exit_code) exit(exit_code); }

+unsigned int os_alarm(unsigned int seconds) +{ + return alarm(seconds); +} + +void os_set_alarm_handler(void (*handler)(int)) +{ + if (!handler) + handler = SIG_DFL; + signal(SIGALRM, handler); +} + +void os_raise_sigalrm(void) +{ + raise(SIGALRM); +} + int os_write_file(const char *fname, const void *buf, int size) { int fd; diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig index 290d1506c2..1870a46257 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -54,6 +54,7 @@ CONFIG_CMD_READ=y CONFIG_CMD_REMOTEPROC=y CONFIG_CMD_SPI=y CONFIG_CMD_USB=y +CONFIG_CMD_WDT=y CONFIG_BOOTP_DNS2=y CONFIG_CMD_TFTPPUT=y CONFIG_CMD_TFTPSRV=y @@ -234,6 +235,7 @@ CONFIG_SPLASH_SCREEN_ALIGN=y CONFIG_WDT=y CONFIG_WDT_GPIO=y CONFIG_WDT_SANDBOX=y +CONFIG_WDT_ALARM_SANDBOX=y CONFIG_FS_CBFS=y CONFIG_FS_CRAMFS=y CONFIG_CMD_DHRYSTONE=y diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index ab5d3f19bf..f117616db8 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -78,6 +78,7 @@ CONFIG_CMD_READ=y CONFIG_CMD_REMOTEPROC=y CONFIG_CMD_SPI=y CONFIG_CMD_USB=y +CONFIG_CMD_WDT=y CONFIG_CMD_AXI=y CONFIG_CMD_SETEXPR_FMT=y CONFIG_CMD_AB_SELECT=y @@ -311,6 +312,7 @@ CONFIG_W1_EEPROM_SANDBOX=y CONFIG_WDT=y CONFIG_WDT_GPIO=y CONFIG_WDT_SANDBOX=y +CONFIG_WDT_ALARM_SANDBOX=y CONFIG_FS_CBFS=y CONFIG_FS_CRAMFS=y CONFIG_ADDR_MAP=y diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index 50e6a1efba..53ec2331a7 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -279,6 +279,14 @@ config WDT_SANDBOX can be probed and supports all of the methods of WDT, but does not really do anything.

+config WDT_ALARM_SANDBOX + bool "Enable SIGALRM-based Watchdog Timer support for Sandbox" + depends on SANDBOX && WDT + help + Enable support for a SIGALRM-based watchdog timer in Sandbox. This is + a watchdog device based on the host OS' alarm() function, which will + kill the sandbox with SIGALRM unless properly maintained. + config WDT_SBSA bool "SBSA watchdog timer support" depends on WDT diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index 0e2f582a5f..446d961d7d 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -16,6 +16,7 @@ obj-$(CONFIG_DESIGNWARE_WATCHDOG) += designware_wdt.o obj-$(CONFIG_ULP_WATCHDOG) += ulp_wdog.o obj-$(CONFIG_$(SPL_TPL_)WDT) += wdt-uclass.o obj-$(CONFIG_WDT_SANDBOX) += sandbox_wdt.o +obj-$(CONFIG_WDT_ALARM_SANDBOX) += sandbox_alarm-wdt.o obj-$(CONFIG_WDT_APPLE) += apple_wdt.o obj-$(CONFIG_WDT_ARMADA_37XX) += armada-37xx-wdt.o obj-$(CONFIG_WDT_ASPEED) += ast_wdt.o diff --git a/drivers/watchdog/sandbox_alarm-wdt.c b/drivers/watchdog/sandbox_alarm-wdt.c new file mode 100644 index 0000000000..71bb5d924e --- /dev/null +++ b/drivers/watchdog/sandbox_alarm-wdt.c @@ -0,0 +1,79 @@ +#include <common.h> +#include <dm.h> +#include <os.h> +#include <wdt.h> + +struct alarm_wdt_priv { + unsigned int timeout_sec; +}; + +static void alarm_handler(int sig) +{ + const char *msg = "!!! ALARM !!!\n"; + + os_write(2, msg, strlen(msg)); + os_fd_restore(); + os_set_alarm_handler(NULL); + os_raise_sigalrm(); +} + +static int alarm_wdt_start(struct udevice *dev, u64 timeout, ulong flags) +{ + struct alarm_wdt_priv *priv = dev_get_priv(dev); + unsigned int sec; + + timeout = DIV_ROUND_UP(timeout, 1000); + sec = min_t(u64, UINT_MAX, timeout); + priv->timeout_sec = sec; + + os_alarm(0); + os_set_alarm_handler(alarm_handler); + os_alarm(sec); + + return 0; +} + +static int alarm_wdt_stop(struct udevice *dev) +{ + os_alarm(0); + os_set_alarm_handler(NULL); + + return 0; +} + +static int alarm_wdt_reset(struct udevice *dev) +{ + struct alarm_wdt_priv *priv = dev_get_priv(dev); + + os_alarm(priv->timeout_sec); + + return 0; +} + +static int alarm_wdt_expire_now(struct udevice *dev, ulong flags) +{ + alarm_handler(0); + + return 0; +} + + +static const struct wdt_ops alarm_wdt_ops = { + .start = alarm_wdt_start, + .reset = alarm_wdt_reset, + .stop = alarm_wdt_stop, + .expire_now = alarm_wdt_expire_now, +}; + +static const struct udevice_id alarm_wdt_ids[] = { + { .compatible = "sandbox,alarm-wdt" }, + {} +}; + +U_BOOT_DRIVER(alarm_wdt_sandbox) = { + .name = "alarm_wdt_sandbox", + .id = UCLASS_WDT, + .of_match = alarm_wdt_ids, + .ops = &alarm_wdt_ops, + .priv_auto = sizeof(struct alarm_wdt_priv), +}; diff --git a/include/os.h b/include/os.h index 148178787b..e6f469324c 100644 --- a/include/os.h +++ b/include/os.h @@ -108,6 +108,23 @@ int os_unlink(const char *pathname); */ void os_exit(int exit_code) __attribute__((noreturn));

+/** + * os_alarm() - access to the OS alarm() system call + */ +unsigned int os_alarm(unsigned int seconds); + +/** + * os_set_alarm_handler() - set handler for SIGALRM + * + * @handler: The handler function. Pass NULL for SIG_DFL. + */ +void os_set_alarm_handler(void (*handler)(int)); + +/** + * os_raise_sigalrm() - do raise(SIGALRM) + */ +void os_raise_sigalrm(void); + /** * os_tty_raw() - put tty into raw mode to mimic serial console better *