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 35CA2C6FD1F for ; Thu, 16 Mar 2023 08:25:30 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id ADF1F85A07; Thu, 16 Mar 2023 09:25:27 +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="GRiJa2Sl"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id E248985A07; Thu, 16 Mar 2023 09:25:25 +0100 (CET) Received: from mail-ed1-x52c.google.com (mail-ed1-x52c.google.com [IPv6:2a00:1450:4864:20::52c]) (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 B7B4085A03 for ; Thu, 16 Mar 2023 09:25:21 +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-ed1-x52c.google.com with SMTP id r11so4354408edd.5 for ; Thu, 16 Mar 2023 01:25:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1678955121; 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=vnuvzBAiMDVpa2V2lPAraWEhn3owOabpc16+rCxF6PA=; b=GRiJa2SlY3lSb5e49vIiJMshR7emNQHm/Sc5kYAckjbZCSWakEW+02O3N9cev924cV /JCHoiyTe5h9TIhmcu+Gcd2geEYD5RRhH16kNY2BrOgkWUtVajJWyb4vQFVKFtQhmO8U Ck01u5fyKZwKxJRQosL3SbaTNDWxgozqHgiZ15moSG7cfHiRv6KEwo+1pDijvKRUS4RV lbiPxyXAMbTjkZBnJ0INgpMwhFCnoH1g+wpXw8Bfwuw6sId9x7eqtLVJIslzRVYWRJYX bZdeEf/tNxZZo37Zr14KiOdp73sJbFzzspQ/ljy8UFEyD5+rbLeFMbp7Du2SE5U7gWl+ QTRA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678955121; 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=vnuvzBAiMDVpa2V2lPAraWEhn3owOabpc16+rCxF6PA=; b=WTQ1ozgUcod8VD0Wt3Vt/Sv7JcO0spmeLXgQcTSmQ/XAezviMenLT9oszJhb3vcE0P tJVvURvKyvPT7sG8JiabXbOtiKa2BJgklQUVNh4s/TweZ5SxAVGywil+/wu9Ii/SK42U A8F2yWeOOSfR9fiq+yNhBUo4s/yrj3aYYNR+xEgh3RFYg59PGCzEjwsj/3zPw9wQVsb8 b99fSyJ08IWS2beLQKrUczwMalzCWB2VuCJ8r0Bo9OdRfIOcqsoUv5P+K7OSFZtsorqB ghgdXIhI3yeKUk5quBcm+SE2e9kd8C/ENSyNiUfrxjex1xNiE1D1omkmDtMVnDwcB+Ex 8LsQ== X-Gm-Message-State: AO0yUKWlE91o0jq8VzD1lt31GqYk4tRhKk8jGsWYKUhEe4GvK6wo7Hf+ AKCf1aI6twPxFvgvjQ0d9jsATw== X-Google-Smtp-Source: AK7set/bIOR2bsPKG/A3+V/dg+bV8EtsQEdUV2DynWXsSzODS4CYiVCcxp+hYOMoXjXaV82YkXQPhg== X-Received: by 2002:a17:907:d13:b0:878:5372:a34b with SMTP id gn19-20020a1709070d1300b008785372a34bmr12561408ejc.45.1678955121162; Thu, 16 Mar 2023 01:25:21 -0700 (PDT) Received: from hera (ppp176092130041.access.hol.gr. [176.92.130.41]) by smtp.gmail.com with ESMTPSA id i24-20020a170906851800b00930d505a567sm168238ejx.128.2023.03.16.01.25.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 16 Mar 2023 01:25:20 -0700 (PDT) Date: Thu, 16 Mar 2023 10:25:18 +0200 From: Ilias Apalodimas To: jassisinghbrar@gmail.com Cc: u-boot@lists.denx.de, sughosh.ganu@linaro.org, etienne.carriere@linaro.org, trini@konsulko.com, sjg@chromium.org, xypron.glpk@gmx.de, patrick.delaunay@foss.st.com, patrice.chotard@foss.st.com, Jassi Brar Subject: Re: [PATCH v6 3/7] fwu: move meta-data management in core Message-ID: References: <20230306231747.1888513-1-jassisinghbrar@gmail.com> <20230306231828.1888580-1-jassisinghbrar@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230306231828.1888580-1-jassisinghbrar@gmail.com> 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.8 at phobos.denx.de X-Virus-Status: Clean On Mon, Mar 06, 2023 at 05:18:28PM -0600, jassisinghbrar@gmail.com wrote: > From: Jassi Brar > > Instead of each i/f having to implement their own meta-data verification > and storage, move the logic in common code. This simplifies the i/f code > much simpler and compact. > > Signed-off-by: Jassi Brar > --- > drivers/fwu-mdata/fwu-mdata-uclass.c | 34 +++++++ > include/fwu.h | 41 +++++++++ > lib/fwu_updates/fwu.c | 131 ++++++++++++++++++++++++++- > 3 files changed, 201 insertions(+), 5 deletions(-) > > diff --git a/drivers/fwu-mdata/fwu-mdata-uclass.c b/drivers/fwu-mdata/fwu-mdata-uclass.c > index b477e9603f..e03773c584 100644 > --- a/drivers/fwu-mdata/fwu-mdata-uclass.c > +++ b/drivers/fwu-mdata/fwu-mdata-uclass.c > @@ -16,6 +16,40 @@ > #include > #include > > +/** > + * fwu_read_mdata() - Wrapper around fwu_mdata_ops.read_mdata() > + * > + * Return: 0 if OK, -ve on error > + */ > +int fwu_read_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary) > +{ > + const struct fwu_mdata_ops *ops = device_get_ops(dev); > + > + if (!ops->read_mdata) { > + log_debug("read_mdata() method not defined\n"); > + return -ENOSYS; > + } > + > + return ops->read_mdata(dev, mdata, primary); > +} > + > +/** > + * fwu_write_mdata() - Wrapper around fwu_mdata_ops.write_mdata() > + * > + * Return: 0 if OK, -ve on error > + */ > +int fwu_write_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary) > +{ > + const struct fwu_mdata_ops *ops = device_get_ops(dev); > + > + if (!ops->write_mdata) { > + log_debug("write_mdata() method not defined\n"); > + return -ENOSYS; > + } > + > + return ops->write_mdata(dev, mdata, primary); > +} > + > /** > * fwu_get_mdata_part_num() - Get the FWU metadata partition numbers > * @dev: FWU metadata device > diff --git a/include/fwu.h b/include/fwu.h > index 0919ced812..13f8fdeb28 100644 > --- a/include/fwu.h > +++ b/include/fwu.h > @@ -24,6 +24,26 @@ struct fwu_mdata_gpt_blk_priv { > * @update_mdata() - Update the FWU metadata copy > */ > struct fwu_mdata_ops { > + /** > + * read_mdata() - Populate the asked FWU metadata copy > + * @dev: FWU metadata device > + * @mdata: Output FWU mdata read > + * @primary: If primary or secondary copy of metadata is to be read > + * > + * Return: 0 if OK, -ve on error > + */ > + int (*read_mdata)(struct udevice *dev, struct fwu_mdata *mdata, bool primary); > + > + /** > + * write_mdata() - Write the given FWU metadata copy > + * @dev: FWU metadata device > + * @mdata: Copy of the FWU metadata to write > + * @primary: If primary or secondary copy of metadata is to be written > + * > + * Return: 0 if OK, -ve on error > + */ > + int (*write_mdata)(struct udevice *dev, struct fwu_mdata *mdata, bool primary); > + > /** > * check_mdata() - Check if the FWU metadata is valid > * @dev: FWU device > @@ -126,6 +146,27 @@ struct fwu_mdata_ops { > EFI_GUID(0x0c996046, 0xbcc0, 0x4d04, 0x85, 0xec, \ > 0xe1, 0xfc, 0xed, 0xf1, 0xc6, 0xf8) > > +/** > + * fwu_read_mdata() - Wrapper around fwu_mdata_ops.read_mdata() > + */ > +int fwu_read_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary); > + > +/** > + * fwu_write_mdata() - Wrapper around fwu_mdata_ops.write_mdata() > + */ > +int fwu_write_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary); > + > +/** > + * fwu_get_verified_mdata() - Read, verify and return the FWU metadata > + * > + * Read both the metadata copies from the storage media, verify their checksum, > + * and ascertain that both copies match. If one of the copies has gone bad, > + * restore it from the good copy. > + * > + * Return: 0 if OK, -ve on error > +*/ > +int fwu_get_verified_mdata(struct fwu_mdata *mdata); > + > /** > * fwu_check_mdata_validity() - Check for validity of the FWU metadata copies > * > diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c > index 5313d07302..8f1c05ad1c 100644 > --- a/lib/fwu_updates/fwu.c > +++ b/lib/fwu_updates/fwu.c > @@ -15,13 +15,13 @@ > #include > #include > > +#include > + > +static struct fwu_mdata g_mdata; /* = {0} makes uninit crc32 always invalid */ > +static struct udevice *g_dev; > static u8 in_trial; > static u8 boottime_check; > > -#include > -#include > -#include > - > enum { > IMAGE_ACCEPT_SET = 1, > IMAGE_ACCEPT_CLEAR, > @@ -161,6 +161,127 @@ static int fwu_get_image_type_id(u8 *image_index, efi_guid_t *image_type_id) > return -ENOENT; > } > > +/** > + * fwu_sync_mdata() - Update given meta-data partition(s) with the copy provided > + * @mdata: FWU metadata structure > + * @part: Bitmask of FWU metadata partitions to be written to > + * > + * Return: 0 if OK, -ve on error > + */ > +static int fwu_sync_mdata(struct fwu_mdata *mdata, int part) > +{ > + void *buf = &mdata->version; > + int err; > + > + if (part == BOTH_PARTS) { > + err = fwu_sync_mdata(mdata, SECONDARY_PART); > + if (err) > + return err; > + part = PRIMARY_PART; > + } > + > + /* > + * Calculate the crc32 for the updated FWU metadata > + * and put the updated value in the FWU metadata crc32 > + * field > + */ > + mdata->crc32 = crc32(0, buf, sizeof(*mdata) - sizeof(u32)); > + > + err = fwu_write_mdata(g_dev, mdata, part == PRIMARY_PART); > + if (err) { > + log_err("Unable to write %s mdata\n", > + part == PRIMARY_PART ? "primary": "secondary"); > + return err; > + } > + > + /* update the cached copy of meta-data */ > + memcpy(&g_mdata, mdata, sizeof(struct fwu_mdata)); > + > + return 0; > +} > + > +static inline int mdata_crc_check(struct fwu_mdata *mdata) > +{ > + void *buf = &mdata->version; > + u32 calc_crc32 = crc32(0, buf, sizeof(*mdata) - sizeof(u32)); > + > + return calc_crc32 == mdata->crc32 ? 0 : -EINVAL; > +} > + > +/** > + * fwu_get_verified_mdata() - Read, verify and return the FWU metadata > + * @mdata: Output FWU metadata read or NULL > + * > + * Read both the metadata copies from the storage media, verify their checksum, > + * and ascertain that both copies match. If one of the copies has gone bad, > + * restore it from the good copy. > + * > + * Return: 0 if OK, -ve on error > + */ > +int fwu_get_verified_mdata(struct fwu_mdata *mdata) > +{ > + int err; > + bool parts_ok[2] = { false }; > + struct fwu_mdata s, *parts_mdata[2]; > + > + parts_mdata[0] = &g_mdata; > + parts_mdata[1] = &s; > + > + /* if mdata already read and ready */ > + err = mdata_crc_check(parts_mdata[0]); > + if (!err) > + goto ret_mdata; > + /* else read, verify and, if needed, fix mdata */ > + > + for (int i = 0; i < 2; i++) { > + parts_ok[i] = false; > + err = fwu_read_mdata(g_dev, parts_mdata[i], !i); > + if (!err) { > + err = mdata_crc_check(parts_mdata[i]); > + if (!err) > + parts_ok[i] = true; > + else > + log_debug("mdata : %s crc32 failed\n", i ? "secondary": "primary"); > + } > + } > + > + if (parts_ok[0] && parts_ok[1]) { > + /* > + * Before returning, check that both the > + * FWU metadata copies are the same. > + */ > + err = memcmp(parts_mdata[0], parts_mdata[1], sizeof(struct fwu_mdata)); > + if (!err) > + goto ret_mdata; > + > + /* > + * If not, populate the secondary partition from the > + * primary partition copy. > + */ > + log_info("Both FWU metadata copies are valid but do not match."); > + log_info(" Restoring the secondary partition from the primary\n"); > + parts_ok[1] = false; > + } > + > + for (int i = 0; i < 2; i++) { > + if (parts_ok[i]) > + continue; > + > + memcpy(parts_mdata[i], parts_mdata[1-i], sizeof(struct fwu_mdata)); > + err = fwu_sync_mdata(parts_mdata[i], i ? SECONDARY_PART : PRIMARY_PART); > + if (err) { > + log_debug("mdata : %s write failed\n", i ? "secondary": "primary"); > + return err; > + } > + } > + > +ret_mdata: > + if (!err && mdata) > + memcpy(mdata, parts_mdata[0], sizeof(struct fwu_mdata)); > + > + return err; > +} > + > /** > * fwu_verify_mdata() - Verify the FWU metadata > * @mdata: FWU metadata structure > @@ -436,7 +557,7 @@ int fwu_get_image_index(u8 *image_index) > } > } > > - log_debug("Partition with the image type %pUs not found\n", > + log_err("Partition with the image type %pUs not found\n", > &image_type_id); > > out: > -- > 2.34.1 > Reviewed-by: Ilias Apalodimas