[U-Boot] [PATCH 06/11] efi_loader: implement SetWatchdogTimer

[Alexander Graf]

On 08.10.17 06:57, Heinrich Schuchardt wrote:
> The watchdog is initialized with a 5 minute timeout period.
> It can be reset by SetWatchdogTimer.
> It is stopped by ExitBoottimeServices.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
>  cmd/bootefi.c                 |  1 +
>  include/efi_loader.h          |  4 +++
>  lib/efi_loader/Makefile       |  2 +-
>  lib/efi_loader/efi_boottime.c | 15 ++---------
>  lib/efi_loader/efi_watchdog.c | 59 +++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 67 insertions(+), 14 deletions(-)
>  create mode 100644 lib/efi_loader/efi_watchdog.c
> 
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index b7087e3da8..24958ada46 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -43,6 +43,7 @@ static void efi_init_obj_list(void)
>  #ifdef CONFIG_GENERATE_SMBIOS_TABLE
>  	efi_smbios_register();
>  #endif
> +	efi_watchdog_register();
>  
>  	/* Initialize EFI runtime services */
>  	efi_reset_system_init();
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index e1179b7dcd..223d8d8222 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -163,6 +163,8 @@ int efi_disk_register(void);
>  int efi_gop_register(void);
>  /* Called by bootefi to make the network interface available */
>  int efi_net_register(void);
> +/* Called by bootefi to make the watchdog available */
> +int efi_watchdog_register(void);
>  /* Called by bootefi to make SMBIOS tables available */
>  void efi_smbios_register(void);
>  
> @@ -171,6 +173,8 @@ efi_fs_from_path(struct efi_device_path *fp);
>  
>  /* Called by networking code to memorize the dhcp ack package */
>  void efi_net_set_dhcp_ack(void *pkt, int len);
> +/* Called by efi_set_watchdog_timer to reset the timer */
> +efi_status_t efi_set_watchdog(unsigned long timeout);
>  
>  /* Called from places to check whether a timer expired */
>  void efi_timer_check(void);
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index ddb978f650..83d879b686 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -17,7 +17,7 @@ endif
>  obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
>  obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o
>  obj-y += efi_memory.o efi_device_path_to_text.o efi_device_path.o
> -obj-y += efi_file.o efi_variable.o efi_bootmgr.o
> +obj-y += efi_file.o efi_variable.o efi_bootmgr.o efi_watchdog.o
>  obj-$(CONFIG_LCD) += efi_gop.o
>  obj-$(CONFIG_DM_VIDEO) += efi_gop.o
>  obj-$(CONFIG_PARTITIONS) += efi_disk.o
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index 30577f717e..81e7d818fc 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -155,18 +155,6 @@ void efi_signal_event(struct efi_event *event)
>  	event->is_queued = false;
>  }
>  
> -/*
> - * Write a debug message for an EPI API service that is not implemented yet.
> - *
> - * @funcname	function that is not yet implemented
> - * @return	EFI_UNSUPPORTED
> - */
> -static efi_status_t efi_unsupported(const char *funcname)
> -{
> -	debug("EFI: App called into unimplemented function %s\n", funcname);
> -	return EFI_EXIT(EFI_UNSUPPORTED);
> -}
> -
>  /*
>   * Raise the task priority level.
>   *
> @@ -1454,6 +1442,7 @@ static efi_status_t EFIAPI efi_exit_boot_services(void *image_handle,
>  	bootm_disable_interrupts();
>  
>  	/* Give the payload some time to boot */
> +	efi_set_watchdog(0);
>  	WATCHDOG_RESET();
>  
>  	return EFI_EXIT(EFI_SUCCESS);
> @@ -1514,7 +1503,7 @@ static efi_status_t EFIAPI efi_set_watchdog_timer(unsigned long timeout,
>  {
>  	EFI_ENTRY("%ld, 0x%"PRIx64", %ld, %p", timeout, watchdog_code,
>  		  data_size, watchdog_data);
> -	return efi_unsupported(__func__);
> +	return EFI_EXIT(efi_set_watchdog(timeout));
>  }
>  
>  /*
> diff --git a/lib/efi_loader/efi_watchdog.c b/lib/efi_loader/efi_watchdog.c
> new file mode 100644
> index 0000000000..50e95290ea
> --- /dev/null
> +++ b/lib/efi_loader/efi_watchdog.c
> @@ -0,0 +1,59 @@
> +/*
> + *  EFI device path interface

Not quite I guess?

> + *
> + *  Copyright (c) 2017 Heinrich Schuchardt
> + *
> + *  SPDX-License-Identifier:     GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <efi_loader.h>
> +
> +static struct efi_event *watchdog_timer_event;
> +
> +static void EFIAPI efi_watchdog_timer_notify(struct efi_event *event,
> +					     void *context)
> +{
> +	EFI_ENTRY("%p, %p", event, context);
> +
> +	printf("\nEFI: Watchdog timeout\n");
> +	EFI_CALL_VOID(efi_runtime_services.reset_system(EFI_RESET_COLD,
> +							EFI_SUCCESS, 0, NULL));
> +
> +	EFI_EXIT(EFI_UNSUPPORTED);
> +}
> +
> +efi_status_t efi_set_watchdog(unsigned long timeout)
> +{
> +	efi_status_t r;
> +
> +	if (timeout)
> +		/* Reset watchdog */
> +		r = efi_set_timer(watchdog_timer_event, EFI_TIMER_RELATIVE,
> +				  10000000 * timeout);
> +	else
> +		/* Deactivate watchdog */
> +		r = efi_set_timer(watchdog_timer_event, EFI_TIMER_STOP, 0);
> +	return r;
> +}
> +
> +/* This gets called from do_bootefi_exec(). */
> +int efi_watchdog_register(void)
> +{
> +	efi_status_t r;
> +
> +	r = efi_create_event(EVT_TIMER | EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
> +			     efi_watchdog_timer_notify, NULL,
> +			     &watchdog_timer_event);

How useful is a timer based watchdog without proper timer implementation?

Is there a specific case where you needed it working?

Also, is there a good reason not to use the hardware watchdog when it's
available?

> +	if (r != EFI_SUCCESS) {
> +		printf("ERROR: Failed to register watchdog event\n");
> +		return r;
> +	}
> +	/* Set watchdog to trigger after 5 minutes */
> +	r = efi_set_watchdog(300);

Is this expected somewhere? Does the spec mention default watchdog timeouts?


Alex

> +	if (r != EFI_SUCCESS) {
> +		printf("ERROR: Failed to set watchdog timer\n");
> +		return r;
> +	}
> +	return 0;
> +}
>