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 C4207C433EF for ; Fri, 1 Apr 2022 06:58:49 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 579E384241; Fri, 1 Apr 2022 08:58:47 +0200 (CEST) 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="muIcyjaN"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id C9E4484253; Fri, 1 Apr 2022 08:58:45 +0200 (CEST) Received: from mail-qk1-x735.google.com (mail-qk1-x735.google.com [IPv6:2607:f8b0:4864:20::735]) (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 C4B9D84209 for ; Fri, 1 Apr 2022 08:58:42 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=sughosh.ganu@linaro.org Received: by mail-qk1-x735.google.com with SMTP id r127so1391423qke.13 for ; Thu, 31 Mar 2022 23:58:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=V3G3PFW71talqOSFxzPmSFtr3ntBqmhpXMC3iRURFKk=; b=muIcyjaNGI++dEfmCtpqkC0vt3XITiDNbxMhu6rng1kr6T70tYdkc+exJXkzTJCfR5 BlMsT8gfc3fZGGQpUXm/GdJR3E5aG1x5ku5v6OWgb/B54LXoSyA9Hc8JVT44EDac8U53 pANWsoicWv8m2Z8wnfrGCDUF93t6OE8Cs2E+J4az6JEYWplxxK+AbnAh34HVIgVAfqxO nEdC5xucicyh4K05nrT68TP+2Gu9qn9iWtsIbFRSGldVJfSkNiu1GxF7JACD6m5rdjWo wMi6c7Mu8f1T0KTHogOVJvcKRdT3DQbR1XEqsgV1Y2fxGWRqzyUuxJBaoaVKUui6Ur50 /Uiw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=V3G3PFW71talqOSFxzPmSFtr3ntBqmhpXMC3iRURFKk=; b=s4EdPObd5BQwdNhDQpHwezoo25RpfWYhqj1I8Foa4ludNK+XO9nIgXGlUZAJuHxPnX +Zd9DBgVgb5m/XnLEbNuLJNClNcWJQL5csONQa44bnNfoQ6x8uYEAqUluwkIGU0nuSpZ navcy8YILc+nmkmQ/eB7VqacHcd1SuE6USAVaJ/CY8S+w/qDk8J7zsyZV46Y9KvHRrT1 QIj4y9HOwCihoTrYzss75rjoSZYHswfSqO0M9T5kbd6sq7nKlaondx9ETUVtLelCfqVg CXBIOgLvZxQqKRpT/CE9ti/hOFvqQRJD1u/4MuGgqkcqsqFbE+82ZiiHvlvwHh0CquH5 kmeg== X-Gm-Message-State: AOAM532c/UQ6rhzGNqLl7bD2IH0dGbNWA9KPGGQPSgKnFYjT9/G7k129 97Jry2EZ0uobaEsN1oyjh2N0PQ+aOdOrZGj+7Y3hHQ== X-Google-Smtp-Source: ABdhPJwxbwu0NKsNMAVrAZ77+a9dzo6aYOV/R48BQfs66VnWQ/ObdevUWfFbkPVSJn8ptRitenibqz7E1yMxgjgAKNk= X-Received: by 2002:a37:a308:0:b0:67e:428:bfa with SMTP id m8-20020a37a308000000b0067e04280bfamr5733608qke.546.1648796321261; Thu, 31 Mar 2022 23:58:41 -0700 (PDT) MIME-Version: 1.0 References: <20220331132750.1532722-1-sughosh.ganu@linaro.org> <20220331132750.1532722-5-sughosh.ganu@linaro.org> In-Reply-To: From: Sughosh Ganu Date: Fri, 1 Apr 2022 12:28:30 +0530 Message-ID: Subject: Re: [PATCH v4 4/8] board: Define set_dfu_alt_info() for boards with UEFI capsule update enabled To: Ilias Apalodimas Cc: u-boot@lists.denx.de, Heinrich Schuchardt , AKASHI Takahiro , Ying-Chun Liu , Tuomas Tynkkynen , Heiko Thiery , Frieder Schrempf , Michael Walle , Masami Hiramatsu , Jassi Brar , Michal Simek , Michal Simek Content-Type: text/plain; charset="UTF-8" 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 hi Ilias, On Fri, 1 Apr 2022 at 01:05, Ilias Apalodimas wrote: > > Hi Sughosh, > > Some nots below > > On Thu, Mar 31, 2022 at 06:57:46PM +0530, Sughosh Ganu wrote: > > Currently, there are a bunch of boards which enable the UEFI capsule > > update feature. The actual update of the firmware images is done > > through the dfu framework which uses the dfu_alt_info environment > > variable for getting information on the update, like device, partition > > number/address etc. Currently, these boards define the dfu_alt_info > > variable in the board config header, as an environment variable. With > > this, the variable can be modified from the u-boot command line and > > this can cause an incorrect update. > > > > To prevent this from happening, define the set_dfu_alt_info function > > in the board file, and select SET_DFU_ALT_INFO for all platforms which > > enable the capsule update feature. With the function defined, the dfu > > framework populates the dfu_alt_info variable through the board file, > > instead of fetching the variable from the environment, thus making the > > update more robust. > > > > Signed-off-by: Sughosh Ganu > > --- > > > > Changes since V3: > > * Do not remove the existing dfu_alt_info definitions made by > > platforms in the config files, as discussed with Masami. > > * Squash the selection of the SET_DFU_ALT_INFO config symbol for > > capsule update feature as part of this patch. > > > > > > .../imx8mp_rsb3720a1/imx8mp_rsb3720a1.c | 24 +++++++++++++++++ > > .../imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c | 24 +++++++++++++++++ > > board/emulation/common/qemu_dfu.c | 6 ++--- > > board/kontron/pitx_imx8m/pitx_imx8m.c | 24 +++++++++++++++++ > > board/kontron/sl-mx8mm/sl-mx8mm.c | 24 +++++++++++++++++ > > board/kontron/sl28/sl28.c | 25 ++++++++++++++++++ > > board/sandbox/sandbox.c | 26 +++++++++++++++++++ > > board/socionext/developerbox/developerbox.c | 26 +++++++++++++++++++ > > board/xilinx/zynq/board.c | 5 ++-- > > board/xilinx/zynqmp/zynqmp.c | 5 ++-- > > lib/efi_loader/Kconfig | 2 ++ > > 11 files changed, 184 insertions(+), 7 deletions(-) > > > > diff --git a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c > > index 1c953ba195..41154ca9f3 100644 > > --- a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c > > +++ b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c > > @@ -5,10 +5,12 @@ > > */ > > > > #include > > +#include > > #include > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -24,6 +26,7 @@ > > #include > > #include > > #include > > +#include > > #include > > > > DECLARE_GLOBAL_DATA_PTR; > > @@ -231,3 +234,24 @@ unsigned long spl_mmc_get_uboot_raw_sector(struct mmc *mmc) > > } > > } > > #endif /* CONFIG_SPL_MMC_SUPPORT */ > > + > > +#if defined(CONFIG_SET_DFU_ALT_INFO) > > + > > +#define DFU_ALT_BUF_LEN SZ_1K > > + > > +void set_dfu_alt_info(char *interface, char *devstr) > > +{ > > + ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN); > > + > > + if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) && > > + env_get("dfu_alt_info")) > > + return; > > Just add a helper function with this since we need to repeat it for every > board. Something like 'bool needs_runtime_dfu_alt_info() ' Okay > > + > > + memset(buf, 0, DFU_ALT_BUF_LEN); > > I'd prefer sizeof(buf) instead of explicitly calling the length. So the current boards are using this wrong. The ALLOC_CACHE_ALIGN_BUFFER actually defines an array named __buf, and a char *buf which points to the array. So the existing code in some boards was zeroing out only the sizeof(char *) number of bytes. The sandbox clang build throws up this warning, which is how I found it. -sughosh > > Otherwise LGTM, but I'd prefer if board maintainer had a look as well > > Thanks > /Ilias > > > + > > + snprintf(buf, DFU_ALT_BUF_LEN, > > + "mmc 2=flash-bin raw 0 0x1B00 mmcpart 1"); > > + > > + env_set("dfu_alt_info", buf); > > +} > > +#endif /* CONFIG_SET_DFU_ALT_INFO */ > > [...]