Re: [PATCH 1/1] efi_loader: stop watchdogs in ExitBootServices()

[Tom Rini <trini@konsulko.com>]

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

On Sat, Jan 28, 2023 at 09:57:45AM +0100, Heinrich Schuchardt wrote:

> The UEFI specification requires for ExitBootServices() that "the boot
> services watchdog timer is disabled". We already disable the software
> watchdog. We should additionally disable the hardware watchdogs.
> 
> Reported-by: Andre Przywara <andre.przywara@arm.com>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>  lib/efi_loader/efi_boottime.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index ba28989f36..71215af9d2 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -19,6 +19,7 @@
>  #include <u-boot/crc.h>
>  #include <usb.h>
>  #include <watchdog.h>
> +#include <wdt.h>
>  #include <asm/global_data.h>
>  #include <asm/setjmp.h>
>  #include <linux/libfdt_env.h>
> @@ -2171,6 +2172,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
>  			list_del(&evt->link);
>  	}
>  
> +	/* Disable watchdogs */
> +	efi_set_watchdog(0);
> +	if IS_ENABLED(CONFIG_WDT)
> +		wdt_stop_all();
> +
>  	if (!efi_st_keep_devices) {
>  		bootm_disable_interrupts();
>  		if (IS_ENABLED(CONFIG_USB_DEVICE))
> @@ -2196,10 +2202,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
>  
>  	/* Recalculate CRC32 */
>  	efi_update_table_header_crc32(&systab.hdr);
> -
> -	/* Give the payload some time to boot */
> -	efi_set_watchdog(0);
> -	schedule();
>  out:
>  	if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
>  		if (ret != EFI_SUCCESS)

I thought we had rejected going down this path since the UEFI spec is
unhelpfully wrong if it insists this?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]