[v2,17/21] tools: mkfwumdata: migrate to metadata version 2

Message ID	20240212074712.3657076-18-sughosh.ganu@linaro.org
State	New
Headers	show Delivered-To: patch@linaro.org Received-SPF: pass (google.com: domain of u-boot-bounces@lists.denx.de designates 2a01:238:438b:c500:173d:9f52:ddab:ee01 as permitted sender) client-ip=2a01:238:438b:c500:173d:9f52:ddab:ee01; From: Sughosh Ganu <sughosh.ganu@linaro.org> To: u-boot@lists.denx.de Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>, Ilias Apalodimas <ilias.apalodimas@linaro.org>, Masahisa Kojima <masahisa.kojima@linaro.org>, Patrice Chotard <patrice.chotard@foss.st.com>, Patrick Delaunay <patrick.delaunay@foss.st.com>, Yann Gautier <yann.gautier@st.com>, Etienne Carriere <etienne.carriere@foss.st.com>, Michal Simek <michal.simek@amd.com>, Jassi Brar <jaswinder.singh@linaro.org>, Sughosh Ganu <sughosh.ganu@linaro.org> Subject: [PATCH v2 17/21] tools: mkfwumdata: migrate to metadata version 2 Date: Mon, 12 Feb 2024 13:17:08 +0530 Message-Id: <20240212074712.3657076-18-sughosh.ganu@linaro.org> In-Reply-To: <20240212074712.3657076-1-sughosh.ganu@linaro.org> References: <20240212074712.3657076-1-sughosh.ganu@linaro.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: list Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" <u-boot-bounces@lists.denx.de>
Series	FWU: Migrate FWU metadata to version 2 \| expand [v2,00/21] FWU: Migrate FWU metadata to version 2 [v2,01/21] configs: fwu: remove FWU configs for metadata V2 migration [v2,02/21] fwu: metadata: migrate to version 2 of the structure [v2,03/21] drivers: fwu: add the size parameter to the metadata access API's [v2,04/21] fwu: add helper functions for getting image description offsets [v2,05/21] fwu: make changes to support version 2 of FWU metadata [v2,06/21] fwu: add some API's for metadata version 2 access [v2,07/21] drivers: fwu: mtd: allocate buffer for image info dynamically [v2,08/21] drivers: fwu: allocate memory for metadata copies [v2,09/21] fwu: add a function to put a bank in Trial State [v2,10/21] capsule: accept a bank on a successful update [v2,11/21] fwu: mtd: modify the DFU API's to align with metadata version 2 [v2,12/21] efi_firmware: fwu: do not read FWU metadata on sandbox [v2,13/21] efi_firmware: fwu: get the number of FWU banks at runtime [v2,14/21] cmd: fwu: align the command with metadata version 2 [v2,15/21] test: fwu: align the FWU metadata access test with version 2 [v2,16/21] fwu: remove the config symbols for number of banks and images [v2,17/21] tools: mkfwumdata: migrate to metadata version 2 [v2,18/21] tools: mkfwumdata: add logic to append vendor data to the FWU metadata [v2,19/21] tools: mkfwumdata: fix the size parameter to the fwrite call [v2,20/21] configs: fwu: re-enable FWU configs [v2,21/21] doc: fwu: make changes for supporting FWU Metadata version 2

Message ID

20240212074712.3657076-18-sughosh.ganu@linaro.org

State

New

Headers

Received-SPF: pass (google.com: domain of u-boot-bounces@lists.denx.de
 designates 2a01:238:438b:c500:173d:9f52:ddab:ee01 as permitted sender)
 client-ip=2a01:238:438b:c500:173d:9f52:ddab:ee01;
From: Sughosh Ganu <sughosh.ganu@linaro.org>
To: u-boot@lists.denx.de
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>,
 Ilias Apalodimas <ilias.apalodimas@linaro.org>,
 Masahisa Kojima <masahisa.kojima@linaro.org>,
 Patrice Chotard <patrice.chotard@foss.st.com>,
 Patrick Delaunay <patrick.delaunay@foss.st.com>,
 Yann Gautier <yann.gautier@st.com>,
 Etienne Carriere <etienne.carriere@foss.st.com>,
 Michal Simek <michal.simek@amd.com>,
 Jassi Brar <jaswinder.singh@linaro.org>,
 Sughosh Ganu <sughosh.ganu@linaro.org>
Subject: [PATCH v2 17/21] tools: mkfwumdata: migrate to metadata version 2
Date: Mon, 12 Feb 2024 13:17:08 +0530
Message-Id: <20240212074712.3657076-18-sughosh.ganu@linaro.org>
In-Reply-To: <20240212074712.3657076-1-sughosh.ganu@linaro.org>
References: <20240212074712.3657076-1-sughosh.ganu@linaro.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: list
Errors-To: u-boot-bounces@lists.denx.de
Sender: "U-Boot" <u-boot-bounces@lists.denx.de>

Series

FWU: Migrate FWU metadata to version 2 | expand

Commit Message

Sughosh Ganu Feb. 12, 2024, 7:47 a.m. UTC

Migrate the metadata generation tool to generate the version 2
metadata.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---

Changes since V1:
* Compute location of struct fwu_fw_store_desc using pointer
  arithmetic.

 tools/mkfwumdata.c | 45 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 34 insertions(+), 11 deletions(-)

Comments

Michal Simek Feb. 15, 2024, 2:31 p.m. UTC | #1

Hi,

On 2/12/24 08:47, Sughosh Ganu wrote:
> Migrate the metadata generation tool to generate the version 2
> metadata.
> 
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
> 
> Changes since V1:
> * Compute location of struct fwu_fw_store_desc using pointer
>    arithmetic.
> 
>   tools/mkfwumdata.c | 45 ++++++++++++++++++++++++++++++++++-----------
>   1 file changed, 34 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/mkfwumdata.c b/tools/mkfwumdata.c
> index 9732a8ddc5..fb847e3a78 100644
> --- a/tools/mkfwumdata.c
> +++ b/tools/mkfwumdata.c
> @@ -14,12 +14,13 @@
>   #include <unistd.h>
>   #include <uuid/uuid.h>
>   
> -/* This will dynamically allocate the fwu_mdata */
> -#define CONFIG_FWU_NUM_BANKS		0
> -#define CONFIG_FWU_NUM_IMAGES_PER_BANK	0
> -
>   /* Since we can not include fwu.h, redefine version here. */
> -#define FWU_MDATA_VERSION		1
> +#define FWU_MDATA_VERSION		2
> +
> +#define MAX_BANKS			4
> +
> +#define BANK_INVALID			0xFF
> +#define BANK_ACCEPTED			0xFC

I think in previous version only active bank was accepted not others.
I don't think it is wrong behavior but please consider to select it too.
I was just surprised to see both banks in that state.

Thanks,
Michal

Sughosh Ganu Feb. 19, 2024, 11:42 a.m. UTC | #2

hi Michal,

On Thu, 15 Feb 2024 at 20:01, Michal Simek <michal.simek@amd.com> wrote:
>
> Hi,
>
> On 2/12/24 08:47, Sughosh Ganu wrote:
> > Migrate the metadata generation tool to generate the version 2
> > metadata.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > ---
> >
> > Changes since V1:
> > * Compute location of struct fwu_fw_store_desc using pointer
> >    arithmetic.
> >
> >   tools/mkfwumdata.c | 45 ++++++++++++++++++++++++++++++++++-----------
> >   1 file changed, 34 insertions(+), 11 deletions(-)
> >
> > diff --git a/tools/mkfwumdata.c b/tools/mkfwumdata.c
> > index 9732a8ddc5..fb847e3a78 100644
> > --- a/tools/mkfwumdata.c
> > +++ b/tools/mkfwumdata.c
> > @@ -14,12 +14,13 @@
> >   #include <unistd.h>
> >   #include <uuid/uuid.h>
> >
> > -/* This will dynamically allocate the fwu_mdata */
> > -#define CONFIG_FWU_NUM_BANKS         0
> > -#define CONFIG_FWU_NUM_IMAGES_PER_BANK       0
> > -
> >   /* Since we can not include fwu.h, redefine version here. */
> > -#define FWU_MDATA_VERSION            1
> > +#define FWU_MDATA_VERSION            2
> > +
> > +#define MAX_BANKS                    4
> > +
> > +#define BANK_INVALID                 0xFF
> > +#define BANK_ACCEPTED                        0xFC
>
> I think in previous version only active bank was accepted not others.
> I don't think it is wrong behavior but please consider to select it too.
> I was just surprised to see both banks in that state.

In the current version as well, we do accept all the images, for all
banks. This is happening in the fwu_parse_fill_image_uuid() function,
where on line 207 we are setting the accepted field to 1 for all banks
for a given image.

-sughosh

diff --git a/tools/mkfwumdata.c b/tools/mkfwumdata.c
index 9732a8ddc5..fb847e3a78 100644
--- a/tools/mkfwumdata.c
+++ b/tools/mkfwumdata.c
@@ -14,12 +14,13 @@ 
 #include <unistd.h>
 #include <uuid/uuid.h>
 
-/* This will dynamically allocate the fwu_mdata */
-#define CONFIG_FWU_NUM_BANKS		0
-#define CONFIG_FWU_NUM_IMAGES_PER_BANK	0
-
 /* Since we can not include fwu.h, redefine version here. */
-#define FWU_MDATA_VERSION		1
+#define FWU_MDATA_VERSION		2
+
+#define MAX_BANKS			4
+
+#define BANK_INVALID			0xFF
+#define BANK_ACCEPTED			0xFC
 
 typedef uint8_t u8;
 typedef int16_t s16;
@@ -29,8 +30,6 @@  typedef uint64_t u64;
 
 #include <fwu_mdata.h>
 
-/* TODO: Endianness conversion may be required for some arch. */
-
 static const char *opts_short = "b:i:a:p:gh";
 
 static struct option options[] = {
@@ -48,7 +47,7 @@  static void print_usage(void)
 	fprintf(stderr, "Usage: mkfwumdata [options] <UUIDs list> <output file>\n");
 	fprintf(stderr, "Options:\n"
 		"\t-i, --images <num>          Number of images (mandatory)\n"
-		"\t-b, --banks  <num>          Number of banks (mandatory)\n"
+		"\t-b, --banks  <num>          Number of banks(1-4) (mandatory)\n"
 		"\t-a, --active-bank  <num>    Active bank (default=0)\n"
 		"\t-p, --previous-bank  <num>  Previous active bank (default=active_bank - 1)\n"
 		"\t-g, --guid                  Use GUID instead of UUID\n"
@@ -85,6 +84,7 @@  static struct fwu_mdata_object *fwu_alloc_mdata(size_t images, size_t banks)
 		return NULL;
 
 	mobj->size = sizeof(struct fwu_mdata) +
+		sizeof(struct fwu_fw_store_desc) +
 		(sizeof(struct fwu_image_entry) +
 		 sizeof(struct fwu_image_bank_info) * banks) * images;
 	mobj->images = images;
@@ -105,6 +105,7 @@  fwu_get_image(struct fwu_mdata_object *mobj, size_t idx)
 	size_t offset;
 
 	offset = sizeof(struct fwu_mdata) +
+		sizeof(struct fwu_fw_store_desc) +
 		(sizeof(struct fwu_image_entry) +
 		 sizeof(struct fwu_image_bank_info) * mobj->banks) * idx;
 
@@ -117,6 +118,7 @@  fwu_get_bank(struct fwu_mdata_object *mobj, size_t img_idx, size_t bnk_idx)
 	size_t offset;
 
 	offset = sizeof(struct fwu_mdata) +
+		sizeof(struct fwu_fw_store_desc) +
 		(sizeof(struct fwu_image_entry) +
 		 sizeof(struct fwu_image_bank_info) * mobj->banks) * img_idx +
 		sizeof(struct fwu_image_entry) +
@@ -188,7 +190,7 @@  fwu_parse_fill_image_uuid(struct fwu_mdata_object *mobj,
 		return -EINVAL;
 
 	if (strcmp(uuid, "0") &&
-	    uuid_guid_parse(uuid, (unsigned char *)&image->location_uuid) < 0)
+	    uuid_guid_parse(uuid, (unsigned char *)&image->location_guid) < 0)
 		return -EINVAL;
 
 	/* Image type UUID */
@@ -196,7 +198,7 @@  fwu_parse_fill_image_uuid(struct fwu_mdata_object *mobj,
 	if (!uuid)
 		return -EINVAL;
 
-	if (uuid_guid_parse(uuid, (unsigned char *)&image->image_type_uuid) < 0)
+	if (uuid_guid_parse(uuid, (unsigned char *)&image->image_type_guid) < 0)
 		return -EINVAL;
 
 	/* Fill bank image-UUID */
@@ -210,7 +212,7 @@  fwu_parse_fill_image_uuid(struct fwu_mdata_object *mobj,
 			return -EINVAL;
 
 		if (strcmp(uuid, "0") &&
-		    uuid_guid_parse(uuid, (unsigned char *)&bank->image_uuid) < 0)
+		    uuid_guid_parse(uuid, (unsigned char *)&bank->image_guid) < 0)
 			return -EINVAL;
 	}
 	return 0;
@@ -220,11 +222,26 @@  fwu_parse_fill_image_uuid(struct fwu_mdata_object *mobj,
 static int fwu_parse_fill_uuids(struct fwu_mdata_object *mobj, char *uuids[])
 {
 	struct fwu_mdata *mdata = mobj->mdata;
+	struct fwu_fw_store_desc *fw_desc;
 	int i, ret;
 
 	mdata->version = FWU_MDATA_VERSION;
 	mdata->active_index = active_bank;
 	mdata->previous_active_index = previous_bank;
+	mdata->metadata_size = mobj->size;
+	mdata->desc_offset = sizeof(struct fwu_mdata);
+
+	for (i = 0; i < MAX_BANKS; i++)
+		mdata->bank_state[i] = i < mobj->banks ?
+			BANK_ACCEPTED : BANK_INVALID;
+
+	fw_desc = (struct fwu_fw_store_desc *)((u8 *)mdata + sizeof(*mdata));
+	fw_desc->num_banks = mobj->banks;
+	fw_desc->num_images = mobj->images;
+	fw_desc->img_entry_size = sizeof(struct fwu_image_entry) +
+		(sizeof(struct fwu_image_bank_info) * mobj->banks);
+	fw_desc->bank_info_entry_size =
+		sizeof(struct fwu_image_bank_info);
 
 	for (i = 0; i < mobj->images; i++) {
 		ret = fwu_parse_fill_image_uuid(mobj, i, uuids[i]);
@@ -313,6 +330,12 @@  int main(int argc, char *argv[])
 		return -EINVAL;
 	}
 
+	if (banks > MAX_BANKS) {
+		fprintf(stderr, "Error: The number of banks must not be more than %u.\n",
+			MAX_BANKS);
+		return -EINVAL;
+	}
+
 	/* This command takes UUIDs * images and output file. */
 	if (optind + images + 1 != argc) {
 		fprintf(stderr, "Error: UUID list or output file is not specified or too much.\n");

[v2,17/21] tools: mkfwumdata: migrate to metadata version 2

Commit Message

Comments

Patch