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 F1F6DC07E9D for ; Mon, 26 Sep 2022 10:08:56 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 4354B84B9A; Mon, 26 Sep 2022 12:08:55 +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="tZM1+nlP"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 55FEE84D96; Mon, 26 Sep 2022 12:08:53 +0200 (CEST) Received: from mail-qv1-xf2c.google.com (mail-qv1-xf2c.google.com [IPv6:2607:f8b0:4864:20::f2c]) (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 6D81181F71 for ; Mon, 26 Sep 2022 12:08:50 +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-qv1-xf2c.google.com with SMTP id j8so3973341qvt.13 for ; Mon, 26 Sep 2022 03:08:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date; bh=C4+IsWcOPAIuKHyrmAQWLF4C25F3a5pGXxVxUkHO+tM=; b=tZM1+nlPTBaFA4OZbyB0bJPqNN8G3dFSxOOUpX8tNvkUE2wEUJK95p+6oema1ekFrc lY6Bnp/leB1X7AVKejJPXoApkytL8ZpfiUAYlBcdddsTeb87/1QViW550Q0b2WQr8kWu z8O5a+oPMio7R9q3KM0lhtHn4e2vCUBxpJG6E3Q6Mk3neXWInsz5o4G5sEQQvpK6v01k TVxg3RuEv4zp+ym0VR/OLOwbkskctgj2FtgIQ/8SDmdcSwQUhcF8rFDXHnPXsBMTVkt1 Fne6n6nYqQpKmESgx87l30YVUlbxeLMiyqAEOtCLyfirZ8WMDF84QqeApCsXTYNykCF3 89Ww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date; bh=C4+IsWcOPAIuKHyrmAQWLF4C25F3a5pGXxVxUkHO+tM=; b=OjAWZCXJurJyEGK4E/cz9XbMJtZ7CZCMdQRSQ45kQm2E+jsPuIwHit29C93us1qdFP HvKM3uWmQqu6e5cPTxfwiXaOVRUXJOoOg4l8c0sGzYXKOq7OaGMwFvrDShpSvD4CcbFU x7KMFJXVCnT6hvtZy2gjLolOE+IlMSw+dP/6bQ/zufub/1JYdvT9gDgxQ1vbAqft6fA/ WiqfinCuYsxN91xs96+/Da3wAdxd0aUudk5Z5SmRc5zhK9bDh9nvoigzWqu+hQq7u5cR mVV/GdTW25p4k85wvvlgJJdijALETu3a+kYSh8Yf8I6JArA1Zk2wk7CpzjcM3hzTAsid wj3w== X-Gm-Message-State: ACrzQf2PQjEC+b0V4pe8tj5gixOeXIl06OiE6bMq6JzYunFYUVAnWt0s V0ONrIGPQQWkDmsu+gPputtDxCqurrNghnKboQq8iw== X-Google-Smtp-Source: AMsMyM7ZnTm5BKtAn/eJiftFMTQwZUKtQ3UNq6m7MFVjnfRXT2CcLtnjCKt04Z1YLkQfqW7YSn9PHO3necfvaf8aFqI= X-Received: by 2002:a05:6214:230f:b0:4af:6ed2:9ee5 with SMTP id gc15-20020a056214230f00b004af6ed29ee5mr3145028qvb.90.1664186929173; Mon, 26 Sep 2022 03:08:49 -0700 (PDT) MIME-Version: 1.0 References: <20220915081451.633983-1-sughosh.ganu@linaro.org> <20220915081451.633983-10-sughosh.ganu@linaro.org> In-Reply-To: From: Sughosh Ganu Date: Mon, 26 Sep 2022 15:38:38 +0530 Message-ID: Subject: Re: [PATCH v10 09/15] FWU: Add boot time checks as highlighted by the FWU specification To: Jassi Brar Cc: u-boot@lists.denx.de, Heinrich Schuchardt , Ilias Apalodimas , Takahiro Akashi , Patrick Delaunay , Patrice Chotard , Simon Glass , Bin Meng , Tom Rini , Etienne Carriere , Michal Simek , Jassi Brar 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.6 at phobos.denx.de X-Virus-Status: Clean On Mon, 26 Sept 2022 at 08:29, Jassi Brar wrote: > > On Thu, Sep 15, 2022 at 3:17 AM Sughosh Ganu wrote: > > .... > > diff --git a/include/fwu.h b/include/fwu.h > > index 484289ed4f..d5f77ce83c 100644 > > --- a/include/fwu.h > > +++ b/include/fwu.h > > @@ -256,4 +256,17 @@ int fwu_plat_get_update_index(uint *update_idx); > > * > > */ > > void fwu_plat_get_bootidx(uint *boot_idx); > > > Or simply return the boot_idx instead of modifying the pointed variable This is following the prototype that is being used for all the functions. Would prefer to have that consistency. > > ..... > > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c > > index c633fcd91e..557e97de4a 100644 > > --- a/lib/efi_loader/efi_setup.c > > +++ b/lib/efi_loader/efi_setup.c > > @@ -199,6 +199,7 @@ static efi_status_t __efi_init_early(void) > > goto out; > > > > ret = efi_disk_init(); > > + > > out: > > return ret; > > } > We can do without this change in this patchset :) Will remove. > > > ..... > > +static int fwu_trial_state_check(struct udevice *dev) > > +{ > > + int ret; > > + efi_status_t status; > > + efi_uintn_t var_size; > > + u16 trial_state_ctr; > > + u32 var_attributes; > > + struct fwu_mdata mdata = { 0 }; > > + > > + ret = fwu_get_mdata(dev, &mdata); > > + if (ret) > > + return ret; > > + > > + if ((trial_state = in_trial_state(&mdata))) { > > > This may raise warnings on code checkers. So maybe move the assignment > out of the check. I did get a compiler warning asking me to use the parentheses around the assignment. But I can move the assignment out. > > > ..... > > +static int fwu_boottime_checks(void *ctx, struct event *event) > > +{ > > + int ret; > > + struct udevice *dev; > > + u32 boot_idx, active_idx; > > + > > + ret = fwu_get_dev_mdata(&dev, NULL); > > + if (ret) > > + return ret; > > + > > + ret = fwu_mdata_check(dev); > > + if (ret) { > > + return 0; > > + } > > + > > + /* > > + * Get the Boot Index, i.e. the bank from > > + * which the platform has booted. This value > > + * gets passed from the ealier stage bootloader > > + * which booted u-boot, e.g. tf-a. If the > > + * boot index is not the same as the > > + * active_index read from the FWU metadata, > > + * update the active_index. > > + */ > > + fwu_plat_get_bootidx(&boot_idx); > > + if (boot_idx >= CONFIG_FWU_NUM_BANKS) { > > + log_err("Received incorrect value of boot_index\n"); > > + return 0; > > + } > > + > > + ret = fwu_get_active_index(&active_idx); > > + if (ret) { > > + log_err("Unable to read active_index\n"); > > + return 0; > > + } > > + > > + if (boot_idx != active_idx) { > > + log_info("Boot idx %u is not matching active idx %u, changing active_idx\n", > > + boot_idx, active_idx); > > + ret = fwu_update_active_index(boot_idx); > > + if (!ret) > > + boottime_check = 1; > > > We may not want to do anything FWU (accept, reject, modify mdata) > until we reboot, if we are recovering from last bad upgrade. So maybe > not set boottime_check Actually, the difference between the boot bank and active bank will happen when there is some kind of corruption on the media due to which the platform could not boot from the active bank(could also be due to repeated wd timeouts). What issue do you see in attempting to update the other bank in case of a mismatch. -sughosh