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

[Michal Simek]

On 6/21/22 12:57, Etienne Carriere wrote:
> On Thu, 9 Jun 2022 at 14:31, Sughosh Ganu <sughosh.ganu at linaro.org> 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
>> +
>> +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},
>> +};
>> +
>> +static void print_usage(void)
>> +{
>> +       fprintf(stderr, "Usage: mkfwumdata [options] <UUIDs list> [<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"
>> +               "\t-a, --active-bank  <num>    Active bank\n"
>> +               "\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>,<image uuid>[,<image uuid>]\n"
>> +               "\n\tYou must specify # of banks of image-uuid and # of images of the lists.\n"
> 
> It's not really explicit how many ',<image uuid>' occurrences are needed.
> Maybe:
>     In a <UUIDs list> item, there must be as many <image uuid>
> occurrences as the given number of banks.
>     There must be as many <UUIDs list> items as the given number of images.

+1 on this.
I was trying 2 bank 2 image configuration and it is not clear how it should be 
specified. Better description would be good.

Thanks,
Michal