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 B856FC05027 for ; Wed, 1 Feb 2023 09:00:56 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 7EC8F85733; Wed, 1 Feb 2023 10:00:54 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=canonical.com 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=canonical.com header.i=@canonical.com header.b="QuJ3BjMt"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 26A5F858B3; Wed, 1 Feb 2023 10:00:45 +0100 (CET) Received: from smtp-relay-internal-0.canonical.com (smtp-relay-internal-0.canonical.com [185.125.188.122]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 2FA32853AA for ; Wed, 1 Feb 2023 10:00:24 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=canonical.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=heinrich.schuchardt@canonical.com Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-0.canonical.com (Postfix) with ESMTPS id 35C28442FE for ; Wed, 1 Feb 2023 09:00:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1675242023; bh=IrpFuwyUEEUvSTKyO+yOsAW15flTJrobJYbXyYyHUyY=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=QuJ3BjMtIEVl9X8g7gvvXuPPh+Gsax1OYuENwPAfmX86I3Fup6AtNGLPF1L1wWQVX ayG1mfH2Ydc1NXzVtxxFx3iMsROShJd9sY5cxkJZxT1TYL9EOejS8c0KfVaftmyWbT s5WHjiwqFD/Ve8fwCjFcXRMea9TV3MNpt103WH8lm0amIbm46oAQUZrSccmVHc9+Ce QS4MNff2m1p1fX/MQvH0SRP6Dh1XuA26tSaa9AJJfnywsQBK2eZg0WlzQIC+yNOxUz oOh9/PpbVgKIi/lQ6odGOdQ8Byp2sqM/MC0dyb23vHZMmJS0ZIARd89BSvQ+zf0BWS ltvGyf/hxCpTA== Received: by mail-wm1-f69.google.com with SMTP id bg24-20020a05600c3c9800b003db0ddddb6fso10134024wmb.0 for ; Wed, 01 Feb 2023 01:00:23 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=IrpFuwyUEEUvSTKyO+yOsAW15flTJrobJYbXyYyHUyY=; b=5mvEyruKfuTWLOOF9THYyvx7hPBUYe2PnYfzgw12HDDjJGv57Cr4LzgBhWWq/OY4L6 ZQQrHMSttvEd4/mohUNOc79WOm9bLOYde5P4wxE8SdoIb8tNXXv0SMMKqMZ46fn4HcFa wWy5tLtNXmzmE1+PEeluEMo0gOOec77v/vgTcpaECJfNeTSyXn0dvRxeWFBPs/IH+9wz M2B0qKQE+EV9+wxL8FNH4Gmxg+bA01qLp8qHCq2RbA4K3ql+5bHkym4UgOsqtEjDy8mD R3SzMBkCLrJzlzJBqGPRfBoMVoqQLprODkQwEqB2E3YJ77bJ7EZ+tNSx+CWnGPCpc6xd UAwg== X-Gm-Message-State: AO0yUKXo/RZportIv/vGol3s2ltpqk1LM9gJ2Wl3bQoh4j8eQ+9i+iqo 7Wb1JK4n08EmGhENzgY59dNmQyoAhbRIwKfzITk0GWH/AFF3mT0Nz2MrHT2Ommkxuz99ucZ582t +pCNz0fR44hwaoTsHhX3OdTMgCs5JFnc= X-Received: by 2002:a05:600c:cca:b0:3db:1919:41b5 with SMTP id fk10-20020a05600c0cca00b003db191941b5mr1251598wmb.21.1675242022348; Wed, 01 Feb 2023 01:00:22 -0800 (PST) X-Google-Smtp-Source: AK7set/PMrMSaCQt7fMdQvAfFD0jJB9ysVxIUwiVVWBx4vXzN5I8qbRZfqmS6tqXMWgY3dnu7YK9XQ== X-Received: by 2002:a05:600c:cca:b0:3db:1919:41b5 with SMTP id fk10-20020a05600c0cca00b003db191941b5mr1251569wmb.21.1675242022020; Wed, 01 Feb 2023 01:00:22 -0800 (PST) Received: from [192.168.123.94] (ip-088-152-145-137.um26.pools.vodafone-ip.de. [88.152.145.137]) by smtp.gmail.com with ESMTPSA id p17-20020a05600c05d100b003dc521f336esm1071812wmd.14.2023.02.01.01.00.21 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 01 Feb 2023 01:00:21 -0800 (PST) Message-ID: Date: Wed, 1 Feb 2023 10:00:20 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Subject: Re: [PATCH 1/1] efi_loader: stop watchdogs in ExitBootServices() Content-Language: en-US To: Tom Rini Cc: u-boot@lists.denx.de, Andre Przywara , Ilias Apalodimas , Rasmus Villemoes References: <20230128085745.18389-1-heinrich.schuchardt@canonical.com> <071ebaa1-2f62-1628-106d-4f4e441c4f0d@prevas.dk> From: Heinrich Schuchardt In-Reply-To: <071ebaa1-2f62-1628-106d-4f4e441c4f0d@prevas.dk> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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 On 2/1/23 09:32, Rasmus Villemoes wrote: > On 31/01/2023 16.07, Tom Rini wrote: >> On Tue, Jan 31, 2023 at 02:03:10PM +0200, Ilias Apalodimas wrote: >>> 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. >> >> You cannot stop the hardware watchdog, period. I think in the previous >> thread about this it was noted that some hardware watchdogs cannot be >> disabled, it's not function that the watchdog supports. Someone needs to >> go and talk with the UEFI specification people and get this addressed. >> There is no sane path for "disable the hardware watchdog". >> > > Indeed. > > But I think one reasonable thing to do would be to say "ok, the payload > is now ready to assume responsibility, so on the U-Boot side we stop > _petting_ the watchdog(s)" (i.e. nowadays that would mean deregistering > them from the cyclic framework), even if the payload still performs > calls into U-Boot where we would otherwise use the opportunity to feed > the watchdog. And of course it's reasonable in that case to do one last > ping. Because it's also a recipe for disaster if, say, both the payload > and U-Boot toggles the same gpio or frobs the same SOC registers. > > Unrelated, but does anybody know who "the UEFI specification people" are > and how to reach out? > > Rasmus > After ExitBootServices() the memory occupied by U-Boot will be reused by the operating system. Don't expect any U-Boot interrupt vector code to exist after this point. If the hardware watchdog is not configured to immediately reset the CPU but create an interrupt instead, anything may happen. @Tom Are all hardware watchdogs used in U-Boot configured to immediately reset the CPU? The UEFI Forum's site is https://uefi.org/. Bugs are reported via https://bugzilla.tianocore.org/. For changing the spec you will have to create a change request in their 'Mantis' system. Best regards Heinrich