[PATCH v5 15/23] tools: Add mkfwumdata tool for FWU metadata image

[Patrick DELAUNAY]

Hi,

one minor remark.

On 6/9/22 14:30, Sughosh Ganu wrote:
> From: Masami Hiramatsu <masami.hiramatsu at linaro.org>
>
> Add 'mkfwumdata' tool which can generate an image of the FWU metadata
> which is required for initializing the platform.
>
> Usage:
>    mkfwumdata -i NR_IMAGES -b NR_BANKS [--guid] \
>      LOCATION_UUID0,IMAGE_TYPE_UUID0,BANK0_IMAGE_UUID[,BANK1_IMAGE_UUID[,...]] \
>      LOCATION_UUID1,... \
>      IMAGE_FILE
>
> '-i' takes the number of images and '-b' takes the number of
> banks. This takes lists of uuids for the images on arguments,
> and the last argument must be the output image file name.
>
> '--guid' (or '-g' in short) allows user to specify the location UUID
> and image IDs in GUID instead of UUID. This option is useful if the
> platform uses GPT partiotion. In this case, the UUID list
> (for an image) becomes;
>
>      DiskGUID,ParitionTypeGUID,UniquePartitionGUID,...
>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu at linaro.org>
> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> ---
>   tools/Kconfig      |   9 ++
>   tools/Makefile     |   4 +
>   tools/mkfwumdata.c | 298 +++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 311 insertions(+)
>   create mode 100644 tools/mkfwumdata.c
>
> diff --git a/tools/Kconfig b/tools/Kconfig
> index 117c921da3..3484be99d0 100644
> --- a/tools/Kconfig
> +++ b/tools/Kconfig
> @@ -98,4 +98,13 @@ config TOOLS_MKEFICAPSULE
>   	  optionally sign that file. If you want to enable UEFI capsule
>   	  update feature on your target, you certainly need this.
>   
> +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 9f2339666a..cd39e5ff6f 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -245,6 +245,10 @@ HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include
>   HOSTLDLIBS_mkeficapsule += -lgnutls -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..4eb304cae3
> --- /dev/null
> +++ b/tools/mkfwumdata.c
> @@ -0,0 +1,298 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#include <errno.h>
> +#include <getopt.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
> +

adding typedef should be avoided

can you include "asm/types.h"  or "linux/types.h" to use the generic define

> +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: Endianess conversion may be required for some arch. */
> +
> +static const char *opts_short = "b:i:a: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'},
> +	{"help", no_argument, NULL, 'h'},
> +	{NULL, 0, NULL, 0},
> +};
> +

[...]

Regards

Patrick