From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 9A69FC636CC for ; Tue, 31 Jan 2023 12:04:19 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id D4EA8857F8; Tue, 31 Jan 2023 13:04:13 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b="Zipo3WKL"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 4448385518; Tue, 31 Jan 2023 13:03:52 +0100 (CET) Received: from mail-ej1-x635.google.com (mail-ej1-x635.google.com [IPv6:2a00:1450:4864:20::635]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 5979385518 for ; Tue, 31 Jan 2023 13:03:14 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=ilias.apalodimas@linaro.org Received: by mail-ej1-x635.google.com with SMTP id lu11so3573784ejb.3 for ; Tue, 31 Jan 2023 04:03:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=uG1YfLNax8u7UO9wILA/ruoI1n4VqXMUWRKPbWCRI+A=; b=Zipo3WKLNZmViMRa5kCA3mIehKwbQLTF8OeB9gpCAV+Fz6f0fbBX8WT4Q7vA041Ipq 8uNKI5K9gSillIb73A2ZMLxMl/xKFOf2go5qJsLq7Il5Vjcc4LnK0wlAbAE3OCnvSis7 uAEs/bFqrKHJFSnCuBYeRfx6sPhp0po/jeldZhFL9sRRdHqxc9cp9at8lWBQf82L2TMW YEcrnerdRLsj0QtozEccNbv4DrNILOoYRClPK1hKpkxuJJT6X+w5WU3ZKBHdRVDSetpH AFkyPOkZnBoZxO9yTjlldZPSCxJEBlHfoOS2TnAx77Sm6P9IASZ4cyNsbkfvMNpn1Dqr u1ow== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=uG1YfLNax8u7UO9wILA/ruoI1n4VqXMUWRKPbWCRI+A=; b=Dr3ZTw82SG0ruFfH8wtwyeOh2OkHhmjXAY94gVfYh2/8KDgXJKizfWBEHBQcrn5yea Jje/AtEtXjEKUJ5HeOebUgtRbED7BlaZHXxI1Fg9q3M9e1lHHAiAOFVqh+lhw8K4uFjb W6reGBIoO5zyLzRmR4CZW2i/wHOK9dsKst+8SzEKyUVEbmXfVWfm2sagL7LcQTcXOaq5 AoasKAMiBRmmtmZr2p4sgw/7cJ9JKdReTKQfrgDXIPgZ0HS1wYpLrxh05o/DSkriZyyg /SufM1EAZ0w+VHGtSc+BcGkPJWJGJIXxyjLDnHARzbr7rsEil1CL2xhJt5VX6kCAcjqe 4fGg== X-Gm-Message-State: AO0yUKXneM65RhOksjmR/VAZZxP3FnWctVA91RHU1cegQAPR9ocGd6RB MYNWINe/hiBXTsHVhKg6Xvz52A== X-Google-Smtp-Source: AK7set/Rp5TvhvGfDSx40heTpkZteZkglpDtW/b99A7T505lAC1wR3DmJOkng+bh9WihYuK3gWOfTQ== X-Received: by 2002:a17:906:71d7:b0:885:5682:7e52 with SMTP id i23-20020a17090671d700b0088556827e52mr10737297ejk.13.1675166593848; Tue, 31 Jan 2023 04:03:13 -0800 (PST) Received: from hera (ppp079167090036.access.hol.gr. [79.167.90.36]) by smtp.gmail.com with ESMTPSA id xo1-20020a170907bb8100b0088550a1ce74sm4399946ejc.120.2023.01.31.04.03.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 31 Jan 2023 04:03:13 -0800 (PST) Date: Tue, 31 Jan 2023 14:03:10 +0200 From: Ilias Apalodimas To: Tom Rini Cc: Heinrich Schuchardt , u-boot@lists.denx.de, Andre Przywara Subject: Re: [PATCH 1/1] efi_loader: stop watchdogs in ExitBootServices() Message-ID: References: <20230128085745.18389-1-heinrich.schuchardt@canonical.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean Hi all, On Mon, Jan 30, 2023 at 01:30:49PM -0500, Tom Rini wrote: > On Mon, Jan 30, 2023 at 01:13:55PM -0500, Tom Rini wrote: > > 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 > > > Signed-off-by: Heinrich Schuchardt > > > --- > > > 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 > > > #include > > > #include > > > +#include > > > #include > > > #include > > > #include > > > @@ -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? > > Because, to be clear, stopping hardware watchdogs is not to be done. The > one in-tree caller of wdt_stop_all is very questionable. You cannot > seriously stop a watchdog until someone else can hopefully resume it as > that violates the function of a hardware watchdog. A pure software > watchdog is one thing, and a hardware watchdog is another. I feel like > the most likely answer here is that someone needs to, still, push back > to the UEFI specification to get hardware watchdogs better understood > and handled, as it must never be stopped once started and if you cannot > reach the next stage in time, that's an engineering issue to resolve. My > first guess is that ExitBootServices should service the watchdog one > last time to ensure the largest window of time for the OS to take over > servicing of the watchdog. > There's two scenarios I can think of 1. After U-Boot is done it can disable the hardware watchdog. The kernel will go through the EFI-stub -> kernel proper -> watchdog gets re-initialized. In that case you are *hoping* that device won't hang in the efi-stub or until the wd is up again. 2. EFI makes sure the hardware wd gets configured with the highest allowed value. The efi-stub doesn't have any driver to refresh the wd, so we will again rely on the wd driver coming up and refreshing the timers. None of those is perfect, but I prefer the latter Regards /Ilias > -- > Tom