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 7B912C433F5 for ; Tue, 1 Feb 2022 03:17:07 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 97E7E83677; Tue, 1 Feb 2022 04:16:56 +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="zWHZ5e1v"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 1670283025; Tue, 1 Feb 2022 04:16:54 +0100 (CET) Received: from mail-pg1-x52a.google.com (mail-pg1-x52a.google.com [IPv6:2607:f8b0:4864:20::52a]) (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 1A60681E25 for ; Tue, 1 Feb 2022 04:16:51 +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=takahiro.akashi@linaro.org Received: by mail-pg1-x52a.google.com with SMTP id t32so14077313pgm.7 for ; Mon, 31 Jan 2022 19:16:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:mail-followup-to:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=GUAq8OtjmlypaUn0hL7ehnVqpepoi7NSeQf8xhPg79c=; b=zWHZ5e1v1O35p2nf6Gh0EN4+4VyKZQqyvylhUEN4sYPEL03Cg5YE+K5stIFE+lnXAv 9UFOrUv2/0cjwfcnIUVv89HZfDzHbkEmapTYUfgU8hK+Iju9qtC5Wm1vvJQJiqLPvvJC 8ubI60cffTVdYSEfr70crDrvK3Qp0LzUeWrww0GyVl5iiy7l7YZAmtJypG2maPZM1Qvt 9miCcu3ynausZ4RM88H+X1gPuZfLeARWlqkxZ1XZCOZEA3HsGtS2e+5uATVYlC1Szdss cpb769oNM3n8HaKJqRNmeN7HXMgRZ46h6NBsRa9yj+ua+5EgJpONAUmEdHllbcWUq0Wx mmcA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id :mail-followup-to:references:mime-version:content-disposition :content-transfer-encoding:in-reply-to; bh=GUAq8OtjmlypaUn0hL7ehnVqpepoi7NSeQf8xhPg79c=; b=Z6gfWWtxr4j9Y4E7z5/Xh86xTF6fYILTMleIOUZ2Md8ry0Dqoy8i+8L7+tTBu+RF8d B7pH2mz44AmWQLFVToD6O14VcbjBRrO0LEBy5Se/7e8QA1nR0Tj+ZlFWcUbQ8YlxGsyH gYE0TpFHxQHggZqA5r+f/88s9zghjathif78NNrG5ORZL7Koe7YEBXjTu3+wj+XqEckA FPg8gnFdCTjrfBdXi0E4+k+aHKn1JvCwftjTpI08IwjF2Yi6teOdXdu2nzMwKH5TfzSS c4+eOKe7ZZAN2AxkDnr13yV2vJiX6BY4PJ7hheIte/EexTrQ2D1sDLuHHmvM3dyPp0zg NN+Q== X-Gm-Message-State: AOAM533KjjSi4pzVAVL3bPdJxuxhm5pShBLGwbIZZKz0+oBDmfYgsNUm WPRjSiJj3JW/h/BnEtGrtKw1nA== X-Google-Smtp-Source: ABdhPJxGkHX+CICAHIFIbwbbXK0u0Stq/w4Ep35KXK+un/AObYBoNfxP3V33BXeid4ktRDFIGiodCA== X-Received: by 2002:a05:6a00:1345:: with SMTP id k5mr23260934pfu.29.1643685409245; Mon, 31 Jan 2022 19:16:49 -0800 (PST) Received: from laputa ([2400:4050:c3e1:100:a8:b825:f6dd:417]) by smtp.gmail.com with ESMTPSA id b6sm20340987pfl.126.2022.01.31.19.16.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 31 Jan 2022 19:16:48 -0800 (PST) Date: Tue, 1 Feb 2022 12:16:43 +0900 From: AKASHI Takahiro To: Masami Hiramatsu Cc: Heinrich Schuchardt , Patrick Delaunay , Patrice Chotard , Alexander Graf , Simon Glass , Bin Meng , Ilias Apalodimas , Jose Marinho , Tom Rini , Etienne Carriere , Sughosh Ganu , Paul Liu , Grant Likely , u-boot@lists.denx.de Subject: Re: [PATCH 2/2] efi_loader: Reset system after CapsuleUpdate on disk Message-ID: <20220201031643.GA67325@laputa> Mail-Followup-To: AKASHI Takahiro , Masami Hiramatsu , Heinrich Schuchardt , Patrick Delaunay , Patrice Chotard , Alexander Graf , Simon Glass , Bin Meng , Ilias Apalodimas , Jose Marinho , Tom Rini , Etienne Carriere , Sughosh Ganu , Paul Liu , Grant Likely , u-boot@lists.denx.de References: <164362071931.312714.14793570668139727572.stgit@localhost> <164362073982.312714.10153796355309762567.stgit@localhost> <4bbb4e3a-e521-6dac-d7a7-8f031b611896@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit 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.5 at phobos.denx.de X-Virus-Status: Clean On Tue, Feb 01, 2022 at 11:26:38AM +0900, Masami Hiramatsu wrote: > Hi Heinrich, > > 2022年1月31日(月) 21:17 Heinrich Schuchardt : > > > > On 1/31/22 12:19, Grant Likely wrote: > > > On 31/01/2022 09:19, Masami Hiramatsu wrote: > > >> Add a config option to reset system soon after processing capsule update > > >> on disk. This is required in UEFI specification 2.9 Section 8.5.5 > > >> "Delivery of Capsules via file on Mass Storage device" as; > > >> > > >> In all cases that a capsule is identified for processing the > > >> system is > > >> restarted after capsule processing is completed. > > >> > > >> Signed-off-by: Masami Hiramatsu > > > > > > Is there known use cases for making this an option? Feels a bit like > > > option creep that is too easy to choose the wrong setting. > > > > > > Otherwise, this looks good to me. > > > > > > g. > > > > > >> --- > > >> lib/efi_loader/Kconfig | 10 ++++++++++ > > >> lib/efi_loader/efi_capsule.c | 9 +++++++++ > > >> 2 files changed, 19 insertions(+) > > >> > > >> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > >> index 24f9a2bb75..db05c3ad90 100644 > > >> --- a/lib/efi_loader/Kconfig > > >> +++ b/lib/efi_loader/Kconfig > > >> @@ -146,6 +146,16 @@ config EFI_IGNORE_OSINDICATIONS > > >> without setting the > > >> EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED > > >> flag in variable OsIndications. > > >> > > >> +config EFI_RESET_AFTER_CAPSULE_ON_DISK > > >> + bool "Reset right after CapsuleUpdate on-disk" > > >> + depends on EFI_CAPSULE_ON_DISK > > >> + default y > > >> + help > > >> + UEFI specification requests the system to be restarted after > > >> capsule > > >> + processing is complete. This implements that, but for some > > >> reason, > > >> + if you want to keep the (old) system running after the capsule > > >> update > > >> + on-disk, you can say 'n' here. > > >> + > > >> config EFI_CAPSULE_ON_DISK_EARLY > > >> bool "Initiate capsule-on-disk at U-Boot boottime" > > >> depends on EFI_CAPSULE_ON_DISK > > >> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > > >> index 98dab1c6f5..44d4fa2f82 100644 > > >> --- a/lib/efi_loader/efi_capsule.c > > >> +++ b/lib/efi_loader/efi_capsule.c > > >> @@ -1142,6 +1142,15 @@ efi_status_t efi_launch_capsules(void) > > >> free(files[i]); > > >> free(files); > > >> > > >> + /* > > >> + * UEFI spec requires to reset system after complete processing > > >> capsule > > >> + * update on the storage. > > >> + */ > > >> + if (IS_ENABLED(CONFIG_EFI_RESET_AFTER_CAPSULE_ON_DISK)) { > > > > We should not allow configuring a non-compliant behavior. > > OK, let me remove this option then. > > > > > >> + log_info("Restarting the system to boot the updated > > >> firmware.\n"); > > > > I am missing a success message for each installed capsule. > > Indeed, but this reboot will be done unconditionally. > let me add a message when the capsule update is successfully completed. > Thank you, While I don't object to adding a message, I'd rather see that a sub-command, like "efidebug capsule show-result," be added to efidebug command. Please note the result for each capsule will be recorded in "CapsuleNNNN" variable which may contain additional information. (I haven't implemented efi_variable_result_fmp though.) -Takahiro Akashi > > > > > >> + do_reset(NULL, 0, 0, NULL); > > > > do_reset() may return. How about: > > > > panic("Reboot after firmware update"); > > > > This will hang if do_reset() returns. > > > > >> + } > > >> + > > >> if (IS_ENABLED(CONFIG_EFI_ESRT)) { > > > > We don't need this code anymore if we call panic() above. > > > > Best regards > > > > Heinrich > > > > >> /* Rebuild the ESRT to reflect any updated FW images. */ > > >> ret = efi_esrt_populate(); > > >> > > > > > > > -- > Masami Hiramatsu