[v5,2/6] tools: Add mkfwumdata tool for FWU metadata image

From: Masami Hiramatsu <masami.hiramatsu@linaro.org>

Add 'mkfwumdata' tool to generate FWU metadata image for the meta-data
partition to be used in A/B Update imeplementation.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 doc/mkfwumdata.1   |  89 ++++++++++++
 tools/Kconfig      |   9 ++
 tools/Makefile     |   4 +
 tools/mkfwumdata.c | 334 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 436 insertions(+)
 create mode 100644 doc/mkfwumdata.1
 create mode 100644 tools/mkfwumdata.c

On Tue, 11 Apr 2023 at 01:03, <jaswinder.singh@linaro.org> wrote:
>
> From: Masami Hiramatsu <masami.hiramatsu@linaro.org>
>
> Add 'mkfwumdata' tool to generate FWU metadata image for the meta-data
> partition to be used in A/B Update imeplementation.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> ---
>  doc/mkfwumdata.1   |  89 ++++++++++++
>  tools/Kconfig      |   9 ++
>  tools/Makefile     |   4 +
>  tools/mkfwumdata.c | 334 +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 436 insertions(+)
>  create mode 100644 doc/mkfwumdata.1
>  create mode 100644 tools/mkfwumdata.c
>
> diff --git a/doc/mkfwumdata.1 b/doc/mkfwumdata.1
> new file mode 100644
> index 0000000000..7dd718b26e
> --- /dev/null
> +++ b/doc/mkfwumdata.1
> @@ -0,0 +1,89 @@
> +.\" SPDX-License-Identifier: GPL-2.0-or-later
> +.\" Copyright (C) 2023 Jassi Brar <jaswinder.singh@linaro.org>
> +.TH MKFWUMDATA 1 2023-04-10 U-Boot
> +.SH NAME
> +mkfwumdata \- create FWU metadata image
> +.
> +.SH SYNOPSIS
> +.SY mkfwumdata
> +.OP \-a activeidx
> +.OP \-p previousidx
> +.OP \-g
> +.BI \-i\~ imagecount
> +.BI \-b\~ bankcount
> +.I UUIDs
> +.I outputimage
> +.YS
> +.SY mkfwumdata
> +.B \-h
> +.YS
> +.
> +.SH DESCRIPTION
> +.B mkfwumdata
> +creates metadata info to be used with FWU.
> +.
> +.SH OPTIONS
> +.TP
> +.B \-h
> +Print usage information and exit.
> +.
> +.TP
> +.B \-a
> +Set
> +.IR activeidx
> +as the currently active Bank. Default is 0.
> +.
> +.TP
> +.B \-p
> +Set
> +.IR previousidx
> +as the previous active Bank. Default is
> +.IR activeidx "-1"
> +or
> +.IR bankcount "-1,"
> +whichever is non-negative.
> +.
> +.TP
> +.B \-g
> +Convert the
> +.IR UUIDs
> +as GUIDs before use.
> +.
> +.TP
> +.B \-i
> +Specify there are
> +.IR imagecount
> +images in each bank.
> +.
> +.TP
> +.B \-b
> +Specify there are a total of
> +.IR bankcount
> +banks.
> +.
> +.TP
> +.IR UUIDs
> +Comma-separated list of UUIDs required to create the metadata :-
> +location_uuid,image_type_uuid,<images per bank uuid list of all banks>
> +.
> +.TP
> +.IR outputimage
> +Specify the name of the metadata image file to be created.
> +.
> +.SH BUGS
> +Please report bugs to the
> +.UR https://\:source\:.denx\:.de/\:u-boot/\:u-boot/\:issues
> +U-Boot bug tracker
> +.UE .
> +.SH EXAMPLES
> +Create a metadata image with 2 banks and 1 image/bank, BankAct=0, BankPrev=1:
> +.PP
> +.EX
> +.in +4
> +$ \c
> +.B mkfwumdata \-a 0 \-p 1 \-b 2 \-i 1 \\\\\&
> +.in +6
> +.B 17e86d77-41f9-4fd7-87ec-a55df9842de5,\\\\\&
> +.B 10c36d7d-ca52-b843-b7b9-f9d6c501d108,\\\\\&
> +.B 5a66a702-99fd-4fef-a392-c26e261a2828,a8f868a1-6e5c-4757-878d-ce63375ef2c0 \\\\\&
> +.B fwu-mdata.img
> diff --git a/tools/Kconfig b/tools/Kconfig
> index 539708f277..6e23f44d55 100644
> --- a/tools/Kconfig
> +++ b/tools/Kconfig
> @@ -157,4 +157,13 @@ config LUT_SEQUENCE
>         help
>           Look Up Table Sequence
>
> +config TOOLS_MKFWUMDATA
> +       bool "Build mkfwumdata command"
> +       default y if FWU_MULTI_BANK_UPDATE
> +       help
> +         This command allows users to create a raw image of the FWU
> +         metadata for initial installation of the FWU multi bank
> +         update on the board. The installation method depends on
> +         the platform.
> +
>  endmenu
> diff --git a/tools/Makefile b/tools/Makefile
> index 38699b069d..1e3fce0b1c 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -250,6 +250,10 @@ HOSTLDLIBS_mkeficapsule += \
>         $(shell pkg-config --libs uuid 2> /dev/null || echo "-luuid")
>  hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule
>
> +mkfwumdata-objs := mkfwumdata.o lib/crc32.o
> +HOSTLDLIBS_mkfwumdata += -luuid
> +hostprogs-$(CONFIG_TOOLS_MKFWUMDATA) += mkfwumdata
> +
>  # We build some files with extra pedantic flags to try to minimize things
>  # that won't build on some weird host compiler -- though there are lots of
>  # exceptions for files that aren't complaint.
> diff --git a/tools/mkfwumdata.c b/tools/mkfwumdata.c
> new file mode 100644
> index 0000000000..43dabf3b72
> --- /dev/null
> +++ b/tools/mkfwumdata.c
> @@ -0,0 +1,334 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2023, Linaro Limited
> + */
> +
> +#include <errno.h>
> +#include <getopt.h>
> +#include <limits.h>
> +#include <stdio.h>
> +#include <stdint.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <u-boot/crc.h>
> +#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
> +
> +typedef uint8_t u8;
> +typedef int16_t s16;
> +typedef uint16_t u16;
> +typedef uint32_t u32;
> +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[] = {
> +       {"banks", required_argument, NULL, 'b'},
> +       {"images", required_argument, NULL, 'i'},
> +       {"guid", required_argument, NULL, 'g'},
> +       {"active-bank", required_argument, NULL, 'a'},
> +       {"previous-bank", required_argument, NULL, 'p'},
> +       {"help", no_argument, NULL, 'h'},
> +       {NULL, 0, NULL, 0},
> +};
> +
> +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\n"
> +               "\t-b, --banks  <num>          Number of banks\n"

State that these arguments are mandatory.

> +               "\t-a, --active-bank  <num>    Active bank\n"
> +               "\t-p, --previous-bank  <num>  Previous active bank\n"

Could you state that active-bank default to 0
and previous-bank default to active bank minus 1 (or image number - 1)

> +               "\t-g, --guid                  Use GUID instead of UUID\n"
> +               "\t-h, --help                  print a help message\n"
> +               );
> +       fprintf(stderr, "  UUIDs list syntax:\n"
> +               "\t  <location uuid>,<image type uuid>,<images uuid list>\n"
> +               "\t     images uuid list syntax:\n"
> +               "\t        img_uuid_00,img_uuid_01...img_uuid_0b,\n"
> +               "\t        img_uuid_10,img_uuid_11...img_uuid_1b,\n"
> +               "\t        ...,\n"
> +               "\t        img_uuid_i0,img_uuid_i1...img_uuid_ib,\n"
> +               "\t          where 'b' and 'i' are number of banks and number\n"
> +               "\t          of images in a bank respectively.\n"
> +              );
> +}
> +
> +struct fwu_mdata_object {
> +       size_t images;
> +       size_t banks;
> +       size_t size;
> +       struct fwu_mdata *mdata;
> +};

I think an inline description for struct's is always welcome in U-Boot
source files.

> +
> +static int previous_bank, active_bank;
> +static bool __use_guid;
> +
> +static struct fwu_mdata_object *fwu_alloc_mdata(size_t images, size_t banks)
> +{
> +       struct fwu_mdata_object *mobj;
> +
> +       mobj = calloc(1, sizeof(*mobj));
> +       if (!mobj)
> +               return NULL;
> +
> +       mobj->size = sizeof(struct fwu_mdata) +
> +               (sizeof(struct fwu_image_entry) +
> +                sizeof(struct fwu_image_bank_info) * banks) * images;
> +       mobj->images = images;
> +       mobj->banks = banks;
> +
> +       mobj->mdata = calloc(1, mobj->size);
> +       if (!mobj->mdata) {
> +               free(mobj);
> +               return NULL;
> +       }
> +
> +       return mobj;
> +}
> +
> +static struct fwu_image_entry *
> +fwu_get_image(struct fwu_mdata_object *mobj, size_t idx)
> +{
> +       size_t offset;
> +
> +       offset = sizeof(struct fwu_mdata) +
> +               (sizeof(struct fwu_image_entry) +
> +                sizeof(struct fwu_image_bank_info) * mobj->banks) * idx;
> +
> +       return (struct fwu_image_entry *)((char *)mobj->mdata + offset);
> +}
> +
> +static struct fwu_image_bank_info *
> +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_image_entry) +
> +                sizeof(struct fwu_image_bank_info) * mobj->banks) * img_idx +
> +               sizeof(struct fwu_image_entry) +
> +               sizeof(struct fwu_image_bank_info) * bnk_idx;
> +
> +       return (struct fwu_image_bank_info *)((char *)mobj->mdata + offset);
> +}
> +
> +/**
> + * convert_uuid_to_guid() - convert UUID to GUID
> + * @buf:       UUID binary
> + *
> + * UUID and GUID have the same data structure, but their binary
> + * formats are different due to the endianness. See lib/uuid.c.
> + * Since uuid_parse() can handle only UUID, this function must
> + * be called to get correct data for GUID when parsing a string.
> + *
> + * The correct data will be returned in @buf.
> + */
> +static void convert_uuid_to_guid(unsigned char *buf)
> +{
> +       unsigned char c;
> +
> +       c = buf[0];
> +       buf[0] = buf[3];
> +       buf[3] = c;
> +       c = buf[1];
> +       buf[1] = buf[2];
> +       buf[2] = c;
> +
> +       c = buf[4];
> +       buf[4] = buf[5];
> +       buf[5] = c;
> +
> +       c = buf[6];
> +       buf[6] = buf[7];
> +       buf[7] = c;
> +}
> +
> +static int uuid_guid_parse(char *uuidstr, unsigned char *uuid)
> +{
> +       int ret;
> +
> +       ret = uuid_parse(uuidstr, uuid);
> +       if (ret < 0)
> +               return ret;
> +
> +       if (__use_guid)
> +               convert_uuid_to_guid(uuid);
> +
> +       return ret;

nitpicking: return 0

> +}
> +
> +static int
> +fwu_parse_fill_image_uuid(struct fwu_mdata_object *mobj,
> +                         size_t idx, char *uuids)
> +{
> +       struct fwu_image_entry *image = fwu_get_image(mobj, idx);
> +       struct fwu_image_bank_info *bank;
> +       char *p = uuids, *uuid;
> +       int i;
> +
> +       if (!image)
> +               return -ENOENT;
> +
> +       /* Image location UUID */
> +       uuid = strsep(&p, ",");
> +       if (!uuid)
> +               return -EINVAL;
> +
> +       if (strcmp(uuid, "0") &&
> +           uuid_guid_parse(uuid, (unsigned char *)&image->location_uuid) < 0)
> +               return -EINVAL;
> +
> +       /* Image type UUID */
> +       uuid = strsep(&p, ",");
> +       if (!uuid)
> +               return -EINVAL;
> +
> +       if (uuid_guid_parse(uuid, (unsigned char *)&image->image_type_uuid) < 0)
> +               return -EINVAL;
> +
> +       /* Fill bank image-UUID */
> +       for (i = 0; i < mobj->banks; i++) {
> +               bank = fwu_get_bank(mobj, idx, i);
> +               if (!bank)
> +                       return -ENOENT;
> +               bank->accepted = 1;
> +               uuid = strsep(&p, ",");
> +               if (!uuid)
> +                       return -EINVAL;
> +
> +               if (strcmp(uuid, "0") &&
> +                   uuid_guid_parse(uuid, (unsigned char *)&bank->image_uuid) < 0)
> +                       return -EINVAL;
> +       }
> +       return 0;
> +}
> +
> +/* Caller must ensure that @uuids[] has @mobj->images entries. */
> +static int fwu_parse_fill_uuids(struct fwu_mdata_object *mobj, char *uuids[])
> +{
> +       struct fwu_mdata *mdata = mobj->mdata;
> +       int i, ret;
> +
> +       mdata->version = FWU_MDATA_VERSION;
> +       mdata->active_index = active_bank;
> +       mdata->previous_active_index = previous_bank;
> +
> +       for (i = 0; i < mobj->images; i++) {
> +               ret = fwu_parse_fill_image_uuid(mobj, i, uuids[i]);
> +               if (ret < 0)
> +                       return ret;
> +       }
> +
> +       mdata->crc32 = crc32(0, (const unsigned char *)&mdata->version,
> +                            mobj->size - sizeof(uint32_t));
> +
> +       return 0;
> +}
> +
> +static int
> +fwu_make_mdata(size_t images, size_t banks, char *uuids[], char *output)
> +{
> +       struct fwu_mdata_object *mobj;
> +       FILE *file;
> +       int ret;
> +
> +       mobj = fwu_alloc_mdata(images, banks);
> +       if (!mobj)
> +               return -ENOMEM;
> +
> +       ret = fwu_parse_fill_uuids(mobj, uuids);
> +       if (ret < 0)
> +               goto done_make;
> +
> +       file = fopen(output, "w");
> +       if (!file) {
> +               ret = -errno;
> +               goto done_make;
> +       }
> +
> +       ret = fwrite(mobj->mdata, mobj->size, 1, file);
> +       if (ret != mobj->size)
> +               ret = -errno;
> +       else
> +               ret = 0;
> +
> +       fclose(file);
> +
> +done_make:
> +       free(mobj->mdata);
> +       free(mobj);
> +
> +       return ret;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +       unsigned long banks = 0, images = 0;
> +       int c, ret;
> +
> +       /* Explicitly initialize defaults */
> +       active_bank = 0;
> +       __use_guid = false;
> +       previous_bank = INT_MAX;

IMHO it would be better to init them where defined, at source file top

Regards,
Etienne

> +
> +       do {
> +               c = getopt_long(argc, argv, opts_short, options, NULL);
> +               switch (c) {
> +               case 'h':
> +                       print_usage();
> +                       return 0;
> +               case 'b':
> +                       banks = strtoul(optarg, NULL, 0);
> +                       break;
> +               case 'i':
> +                       images = strtoul(optarg, NULL, 0);
> +                       break;
> +               case 'g':
> +                       __use_guid = true;
> +                       break;
> +               case 'p':
> +                       previous_bank = strtoul(optarg, NULL, 0);
> +                       break;
> +               case 'a':
> +                       active_bank = strtoul(optarg, NULL, 0);
> +                       break;
> +               }
> +       } while (c != -1);
> +
> +       if (!banks || !images) {
> +               fprintf(stderr, "Error: The number of banks and images must not be 0.\n");
> +               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");
> +               print_usage();
> +               return -ERANGE;
> +       }
> +
> +       if (previous_bank == INT_MAX) {
> +               /* set to the earlier bank in round-robin scheme */
> +               previous_bank = active_bank > 0 ? active_bank - 1 : banks - 1;
> +       }
> +
> +       ret = fwu_make_mdata(images, banks, argv + optind, argv[argc - 1]);
> +       if (ret < 0)
> +               fprintf(stderr, "Error: Failed to parse and write image: %s\n",
> +                       strerror(-ret));
> +
> +       return ret;
> +}
> --
> 2.34.1
>