Re: [PATCH 4/7] tools: mkeficapsule: Add support for parsing capsule params from config file

Schmidt, Malte
2023-06-16T13:13:00Z

Hi sughosh,

Am 16.06.2023 um 08:35 schrieb Sughosh Ganu:
> On Fri, 16 Jun 2023 at 10:48, Takahiro Akashi
> <takahiro.akashi@linaro.org> wrote:
>> On Fri, Jun 16, 2023 at 10:37:01AM +0530, Sughosh Ganu wrote:
>>> hi Takahiro,
>>>
>>> On Fri, 16 Jun 2023 at 10:16, Takahiro Akashi
>>> <takahiro.akashi@linaro.org> wrote:
>>>> Hi Sughosh,
>>>>
>>>> On Fri, Jun 16, 2023 at 09:56:33AM +0530, Sughosh Ganu wrote:
>>>>> On Thu, 15 Jun 2023 at 11:19, Takahiro Akashi
>>>>> <takahiro.akashi@linaro.org> wrote:
>>>>>> On Thu, Jun 15, 2023 at 10:09:06AM +0530, Sughosh Ganu wrote:
>>>>>>> On Wed, 14 Jun 2023 at 11:23, Takahiro Akashi
>>>>>>> <takahiro.akashi@linaro.org> wrote:
>>>>>>>> On Wed, Jun 14, 2023 at 10:56:23AM +0530, Sughosh Ganu wrote:
>>>>>>>>> hi Takahiro,
>>>>>>>>>
>>>>>>>>> On Wed, 14 Jun 2023 at 09:09, Takahiro Akashi
>>>>>>>>> <takahiro.akashi@linaro.org> wrote:
>>>>>>>>>> Hi Sughosh,
>>>>>>>>>>
>>>>>>>>>> I think this is a good extension to mkeficapsule, but
>>>>>>>>>>
>>>>>>>>>> On Tue, Jun 13, 2023 at 04:08:03PM +0530, Sughosh Ganu wrote:
>>>>>>>>>>> Add support for specifying the parameters needed for capsule
>>>>>>>>>>> generation through a config file, instead of passing them through
>>>>>>>>>>> command-line. Parameters for more than a single capsule file can be
>>>>>>>>>>> specified, resulting in generation of multiple capsules through a
>>>>>>>>>>> single invocation of the command.
>>>>>>>>>>>
>>>>>>>>>>> This path is to be used for generating capsules through a make target,
>>>>>>>>>>> with the parameters being parsed from the config file.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Sughosh Ganu<sughosh.ganu@linaro.org>
>>>>>>>>>>> ---
>>>>>>>>>>> tools/Kconfig | 9 +
>>>>>>>>>>> tools/Makefile | 1 +
>>>>>>>>>>> tools/eficapsule.h | 110 ++++++++++++
>>>>>>>>>>> tools/mkeficapsule.c | 106 +++++++-----
>>>>>>>>>>> tools/mkeficapsule_parse.c | 345 +++++++++++++++++++++++++++++++++++++
>>>>>>>>>>> 5 files changed, 531 insertions(+), 40 deletions(-)
>>>>>>>>>>> create mode 100644 tools/mkeficapsule_parse.c
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/tools/Kconfig b/tools/Kconfig
>>>>>>>>>>> index 539708f277..95f27b7c45 100644
>>>>>>>>>>> --- a/tools/Kconfig
>>>>>>>>>>> +++ b/tools/Kconfig
>>>>>>>>>>> @@ -98,6 +98,15 @@ config TOOLS_MKEFICAPSULE
>>>>>>>>>>> optionally sign that file. If you want to enable UEFI capsule
>>>>>>>>>>> update feature on your target, you certainly need this.
>>>>>>>>>>>
>>>>>>>>>>> +config EFI_CAPSULE_CFG_FILE
>>>>>>>>>>> + string "Path to the EFI Capsule Config File"
>>>>>>>>>>> + default ""
>>>>>>>>>>> + help
>>>>>>>>>>> + Path to the EFI capsule config file which provides the
>>>>>>>>>>> + parameters needed to build capsule(s). Parameters can be
>>>>>>>>>>> + provided for multiple payloads resulting in corresponding
>>>>>>>>>>> + capsule images being generated.
>>>>>>>>>>> +
>>>>>>>>>>> menuconfig FSPI_CONF_HEADER
>>>>>>>>>>> bool "FlexSPI Header Configuration"
>>>>>>>>>>> help
>>>>>>>>>>> diff --git a/tools/Makefile b/tools/Makefile
>>>>>>>>>>> index d793cf3bec..ef366f3d61 100644
>>>>>>>>>>> --- a/tools/Makefile
>>>>>>>>>>> +++ b/tools/Makefile
>>>>>>>>>>> @@ -250,6 +250,7 @@ HOSTLDLIBS_mkeficapsule += \
>>>>>>>>>>> HOSTLDLIBS_mkeficapsule += \
>>>>>>>>>>> $(shell pkg-config --libs uuid 2> /dev/null || echo "-luuid")
>>>>>>>>>>> hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule
>>>>>>>>>>> +mkeficapsule-objs := mkeficapsule.o mkeficapsule_parse.o
>>>>>>>>>>>
>>>>>>>>>>> # 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
>>>>>>>>>>> diff --git a/tools/eficapsule.h b/tools/eficapsule.h
>>>>>>>>>>> index 072a4b5598..42e66c6d6a 100644
>>>>>>>>>>> --- a/tools/eficapsule.h
>>>>>>>>>>> +++ b/tools/eficapsule.h
>>>>>>>>>>> @@ -52,6 +52,38 @@ typedef struct {
>>>>>>>>>>> /* flags */
>>>>>>>>>>> #define CAPSULE_FLAGS_PERSIST_ACROSS_RESET 0x00010000
>>>>>>>>>>>
>>>>>>>>>>> +enum capsule_type {
>>>>>>>>>>> + CAPSULE_NORMAL_BLOB = 0,
>>>>>>>>>>> + CAPSULE_ACCEPT,
>>>>>>>>>>> + CAPSULE_REVERT,
>>>>>>>>>>> +};
>>>>>>>>>>> +
>>>>>>>>>>> +/**
>>>>>>>>>>> + * struct efi_capsule_params - Capsule parameters
>>>>>>>>>>> + * @image_guid: Guid value of the payload input image
>>>>>>>>>>> + * @image_index: Image index value
>>>>>>>>>>> + * @hardware_instance: Hardware instance to be used for the image
>>>>>>>>>>> + * @monotonic_count: Monotonic count value to be used for signed capsule
>>>>>>>>>>> + * @privkey_file: Path to private key used in capsule signing
>>>>>>>>>>> + * @cert_file: Path to public key certificate used in capsule signing
>>>>>>>>>>> + * @input_file: Path to payload input image
>>>>>>>>>>> + * @capsule_file: Path to the output capsule file
>>>>>>>>>>> + * @oemflags: Oemflags to be populated in the capsule header
>>>>>>>>>>> + * @capsule: Capsule Type, normal or accept or revert
>>>>>>>>>>> + */
>>>>>>>>>>> +struct efi_capsule_params {
>>>>>>>>>>> + efi_guid_t *image_guid;
>>>>>>>>>>> + unsigned long image_index;
>>>>>>>>>>> + unsigned long hardware_instance;
>>>>>>>>>>> + uint64_t monotonic_count;
>>>>>>>>>>> + char *privkey_file;
>>>>>>>>>>> + char *cert_file;
>>>>>>>>>>> + char *input_file;
>>>>>>>>>>> + char *capsule_file;
>>>>>>>>>>> + unsigned long oemflags;
>>>>>>>>>>> + enum capsule_type capsule;
>>>>>>>>>>> +};
>>>>>>>>>>> +
>>>>>>>>>>> struct efi_capsule_header {
>>>>>>>>>>> efi_guid_t capsule_guid;
>>>>>>>>>>> uint32_t header_size;
>>>>>>>>>>> @@ -113,4 +145,82 @@ struct efi_firmware_image_authentication {
>>>>>>>>>>> struct win_certificate_uefi_guid auth_info;
>>>>>>>>>>> } __packed;
>>>>>>>>>>>
>>>>>>>>>>> +/**
>>>>>>>>>>> + * capsule_with_cfg_file() - Generate capsule from config file
>>>>>>>>>>> + * @cfg_file: Path to the config file
>>>>>>>>>>> + *
>>>>>>>>>>> + * Parse the capsule parameters from the config file and use the
>>>>>>>>>>> + * parameters for generating one or more capsules.
>>>>>>>>>>> + *
>>>>>>>>>>> + * Return: None
>>>>>>>>>>> + *
>>>>>>>>>>> + */
>>>>>>>>>>> +void capsule_with_cfg_file(const char *cfg_file);
>>>>>>>>>>> +
>>>>>>>>>>> +/**
>>>>>>>>>>> + * 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.
>>>>>>>>>>> + */
>>>>>>>>>>> +void convert_uuid_to_guid(unsigned char *buf);
>>>>>>>>>>> +
>>>>>>>>>>> +/**
>>>>>>>>>>> + * create_empty_capsule() - Generate an empty capsule
>>>>>>>>>>> + * @path: Path to the empty capsule file to be generated
>>>>>>>>>>> + * @guid: Guid value of the image for which empty capsule is generated
>>>>>>>>>>> + * @fw_accept: Flag to specify whether to generate accept or revert capsule
>>>>>>>>>>> + *
>>>>>>>>>>> + * Generate an empty capsule, either an accept or a revert capsule to be
>>>>>>>>>>> + * used to flag acceptance or rejection of an earlier executed firmware
>>>>>>>>>>> + * update operation. Being used in the FWU Multi Bank firmware update
>>>>>>>>>>> + * feature.
>>>>>>>>>>> + *
>>>>>>>>>>> + * Return: 0 if OK, -ve on error
>>>>>>>>>>> + *
>>>>>>>>>>> + */
>>>>>>>>>>> +int create_empty_capsule(char *path, efi_guid_t *guid, bool fw_accept);
>>>>>>>>>>> +
>>>>>>>>>>> +/**
>>>>>>>>>>> + * create_fwbin - create an uefi capsule file
>>>>>>>>>>> + * @path: Path to a created capsule file
>>>>>>>>>>> + * @bin: Path to a firmware binary to encapsulate
>>>>>>>>>>> + * @guid: GUID of related FMP driver
>>>>>>>>>>> + * @index: Index number in capsule
>>>>>>>>>>> + * @instance: Instance number in capsule
>>>>>>>>>>> + * @mcount: Monotonic count in authentication information
>>>>>>>>>>> + * @private_file: Path to a private key file
>>>>>>>>>>> + * @cert_file: Path to a certificate file
>>>>>>>>>>> + * @oemflags: Capsule OEM Flags, bits 0-15
>>>>>>>>>>> + *
>>>>>>>>>>> + * This function actually does the job of creating an uefi capsule file.
>>>>>>>>>>> + * All the arguments must be supplied.
>>>>>>>>>>> + * If either @private_file ror @cert_file is NULL, the capsule file
>>>>>>>>>>> + * won't be signed.
>>>>>>>>>>> + *
>>>>>>>>>>> + * Return:
>>>>>>>>>>> + * * 0 - on success
>>>>>>>>>>> + * * -1 - on failure
>>>>>>>>>>> + */
>>>>>>>>>>> +int create_fwbin(char *path, char *bin, efi_guid_t *guid,
>>>>>>>>>>> + unsigned long index, unsigned long instance,
>>>>>>>>>>> + uint64_t mcount, char *privkey_file, char *cert_file,
>>>>>>>>>>> + uint16_t oemflags);
>>>>>>>>>>> +
>>>>>>>>>>> +/**
>>>>>>>>>>> + * print_usage() - Print the command usage string
>>>>>>>>>>> + *
>>>>>>>>>>> + * Prints the standard command usage string. Called in the case
>>>>>>>>>>> + * of incorrect parameters being passed to the tool.
>>>>>>>>>>> + *
>>>>>>>>>>> + * Return: None
>>>>>>>>>>> + *
>>>>>>>>>>> + */
>>>>>>>>>>> +void print_usage(void);
>>>>>>>>>>> +
>>>>>>>>>>> #endif /* _EFI_CAPSULE_H */
>>>>>>>>>>> diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
>>>>>>>>>>> index b71537beee..711adf0439 100644
>>>>>>>>>>> --- a/tools/mkeficapsule.c
>>>>>>>>>>> +++ b/tools/mkeficapsule.c
>>>>>>>>>>> @@ -31,12 +31,6 @@ efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
>>>>>>>>>>>
>>>>>>>>>>> static const char *opts_short = "g:i:I:v:p:c:m:o:dhAR";
>>>>>>>>>>>
>>>>>>>>>>> -enum {
>>>>>>>>>>> - CAPSULE_NORMAL_BLOB = 0,
>>>>>>>>>>> - CAPSULE_ACCEPT,
>>>>>>>>>>> - CAPSULE_REVERT,
>>>>>>>>>>> -} capsule_type;
>>>>>>>>>>> -
>>>>>>>>>>> static struct option options[] = {
>>>>>>>>>>> {"guid", required_argument, NULL, 'g'},
>>>>>>>>>>> {"index", required_argument, NULL, 'i'},
>>>>>>>>>>> @@ -52,7 +46,16 @@ static struct option options[] = {
>>>>>>>>>>> {NULL, 0, NULL, 0},
>>>>>>>>>>> };
>>>>>>>>>>>
>>>>>>>>>>> -static void print_usage(void)
>>>>>>>>>>> +/**
>>>>>>>>>>> + * print_usage() - Print the command usage string
>>>>>>>>>>> + *
>>>>>>>>>>> + * Prints the standard command usage string. Called in the case
>>>>>>>>>>> + * of incorrect parameters being passed to the tool.
>>>>>>>>>>> + *
>>>>>>>>>>> + * Return: None
>>>>>>>>>>> + *
>>>>>>>>>>> + */
>>>>>>>>>>> +void print_usage(void)
>>>>>>>>>>> {
>>>>>>>>>>> fprintf(stderr, "Usage: %s [options] <image blob> \n"
>>>>>>>>>>> "Options:\n"
>>>>>>>>>>> @@ -400,10 +403,10 @@ static void free_sig_data(struct auth_context *ctx)
>>>>>>>>>>> * * 0 - on success
>>>>>>>>>>> * * -1 - on failure
>>>>>>>>>>> */
>>>>>>>>>>> -static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
>>>>>>>>>>> - unsigned long index, unsigned long instance,
>>>>>>>>>>> - uint64_t mcount, char *privkey_file, char *cert_file,
>>>>>>>>>>> - uint16_t oemflags)
>>>>>>>>>>> +int create_fwbin(char *path, char *bin, efi_guid_t *guid,
>>>>>>>>>>> + unsigned long index, unsigned long instance,
>>>>>>>>>>> + uint64_t mcount, char *privkey_file, char *cert_file,
>>>>>>>>>>> + uint16_t oemflags)
>>>>>>>>>>> {
>>>>>>>>>>> struct efi_capsule_header header;
>>>>>>>>>>> struct efi_firmware_management_capsule_header capsule;
>>>>>>>>>>> @@ -580,7 +583,21 @@ void convert_uuid_to_guid(unsigned char *buf)
>>>>>>>>>>> buf[7] = c;
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>> -static int create_empty_capsule(char *path, efi_guid_t *guid, bool fw_accept)
>>>>>>>>>>> +/**
>>>>>>>>>>> + * create_empty_capsule() - Generate an empty capsule
>>>>>>>>>>> + * @path: Path to the empty capsule file to be generated
>>>>>>>>>>> + * @guid: Guid value of the image for which empty capsule is generated
>>>>>>>>>>> + * @fw_accept: Flag to specify whether to generate accept or revert capsule
>>>>>>>>>>> + *
>>>>>>>>>>> + * Generate an empty capsule, either an accept or a revert capsule to be
>>>>>>>>>>> + * used to flag acceptance or rejection of an earlier executed firmware
>>>>>>>>>>> + * update operation. Being used in the FWU Multi Bank firmware update
>>>>>>>>>>> + * feature.
>>>>>>>>>>> + *
>>>>>>>>>>> + * Return: 0 if OK, -ve on error
>>>>>>>>>>> + *
>>>>>>>>>>> + */
>>>>>>>>>>> +int create_empty_capsule(char *path, efi_guid_t *guid, bool fw_accept)
>>>>>>>>>>> {
>>>>>>>>>>> struct efi_capsule_header header = { 0 };
>>>>>>>>>>> FILE *f = NULL;
>>>>>>>>>>> @@ -623,19 +640,7 @@ err:
>>>>>>>>>>> return ret;
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>> -/**
>>>>>>>>>>> - * main - main entry function of mkeficapsule
>>>>>>>>>>> - * @argc: Number of arguments
>>>>>>>>>>> - * @argv: Array of pointers to arguments
>>>>>>>>>>> - *
>>>>>>>>>>> - * Create an uefi capsule file, optionally signing it.
>>>>>>>>>>> - * Parse all the arguments and pass them on to create_fwbin().
>>>>>>>>>>> - *
>>>>>>>>>>> - * Return:
>>>>>>>>>>> - * * 0 - on success
>>>>>>>>>>> - * * -1 - on failure
>>>>>>>>>>> - */
>>>>>>>>>>> -int main(int argc, char **argv)
>>>>>>>>>>> +static void capsule_with_cmdline_params(int argc, char **argv)
>>>>>>>>>>> {
>>>>>>>>>>> efi_guid_t *guid;
>>>>>>>>>>> unsigned char uuid_buf[16];
>>>>>>>>>>> @@ -643,6 +648,7 @@ int main(int argc, char **argv)
>>>>>>>>>>> uint64_t mcount;
>>>>>>>>>>> unsigned long oemflags;
>>>>>>>>>>> char *privkey_file, *cert_file;
>>>>>>>>>>> + enum capsule_type capsule;
>>>>>>>>>>> int c, idx;
>>>>>>>>>>>
>>>>>>>>>>> guid = NULL;
>>>>>>>>>>> @@ -652,7 +658,7 @@ int main(int argc, char **argv)
>>>>>>>>>>> privkey_file = NULL;
>>>>>>>>>>> cert_file = NULL;
>>>>>>>>>>> dump_sig = 0;
>>>>>>>>>>> - capsule_type = CAPSULE_NORMAL_BLOB;
>>>>>>>>>>> + capsule = CAPSULE_NORMAL_BLOB;
>>>>>>>>>>> oemflags = 0;
>>>>>>>>>>> for (;;) {
>>>>>>>>>>> c = getopt_long(argc, argv, opts_short, options, &idx);
>>>>>>>>>>> @@ -702,20 +708,20 @@ int main(int argc, char **argv)
>>>>>>>>>>> dump_sig = 1;
>>>>>>>>>>> break;
>>>>>>>>>>> case 'A':
>>>>>>>>>>> - if (capsule_type) {
>>>>>>>>>>> + if (capsule) {
>>>>>>>>>>> fprintf(stderr,
>>>>>>>>>>> "Select either of Accept or Revert capsule generation\n");
>>>>>>>>>>> exit(1);
>>>>>>>>>>> }
>>>>>>>>>>> - capsule_type = CAPSULE_ACCEPT;
>>>>>>>>>>> + capsule = CAPSULE_ACCEPT;
>>>>>>>>>>> break;
>>>>>>>>>>> case 'R':
>>>>>>>>>>> - if (capsule_type) {
>>>>>>>>>>> + if (capsule) {
>>>>>>>>>>> fprintf(stderr,
>>>>>>>>>>> "Select either of Accept or Revert capsule generation\n");
>>>>>>>>>>> exit(1);
>>>>>>>>>>> }
>>>>>>>>>>> - capsule_type = CAPSULE_REVERT;
>>>>>>>>>>> + capsule = CAPSULE_REVERT;
>>>>>>>>>>> break;
>>>>>>>>>>> case 'o':
>>>>>>>>>>> oemflags = strtoul(optarg, NULL, 0);
>>>>>>>>>>> @@ -732,21 +738,21 @@ int main(int argc, char **argv)
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>> /* check necessary parameters */
>>>>>>>>>>> - if ((capsule_type == CAPSULE_NORMAL_BLOB &&
>>>>>>>>>>> - ((argc != optind + 2) || !guid ||
>>>>>>>>>>> - ((privkey_file && !cert_file) ||
>>>>>>>>>>> - (!privkey_file && cert_file)))) ||
>>>>>>>>>>> - (capsule_type != CAPSULE_NORMAL_BLOB &&
>>>>>>>>>>> - ((argc != optind + 1) ||
>>>>>>>>>>> - ((capsule_type == CAPSULE_ACCEPT) && !guid) ||
>>>>>>>>>>> - ((capsule_type == CAPSULE_REVERT) && guid)))) {
>>>>>>>>>>> + if ((capsule == CAPSULE_NORMAL_BLOB &&
>>>>>>>>>>> + ((argc != optind + 2) || !guid ||
>>>>>>>>>>> + ((privkey_file && !cert_file) ||
>>>>>>>>>>> + (!privkey_file && cert_file)))) ||
>>>>>>>>>>> + (capsule != CAPSULE_NORMAL_BLOB &&
>>>>>>>>>>> + ((argc != optind + 1) ||
>>>>>>>>>>> + (capsule == CAPSULE_ACCEPT && !guid) ||
>>>>>>>>>>> + (capsule == CAPSULE_REVERT && guid)))) {
>>>>>>>>>>> print_usage();
>>>>>>>>>>> exit(EXIT_FAILURE);
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>> - if (capsule_type != CAPSULE_NORMAL_BLOB) {
>>>>>>>>>>> + if (capsule != CAPSULE_NORMAL_BLOB) {
>>>>>>>>>>> if (create_empty_capsule(argv[argc - 1], guid,
>>>>>>>>>>> - capsule_type == CAPSULE_ACCEPT) < 0) {
>>>>>>>>>>> + capsule == CAPSULE_ACCEPT) < 0) {
>>>>>>>>>>> fprintf(stderr, "Creating empty capsule failed\n");
>>>>>>>>>>> exit(EXIT_FAILURE);
>>>>>>>>>>> }
>>>>>>>>>>> @@ -756,6 +762,26 @@ int main(int argc, char **argv)
>>>>>>>>>>> fprintf(stderr, "Creating firmware capsule failed\n");
>>>>>>>>>>> exit(EXIT_FAILURE);
>>>>>>>>>>> }
>>>>>>>>>>> +}
>>>>>>>>>>> +
>>>>>>>>>>> +/**
>>>>>>>>>>> + * main - main entry function of mkeficapsule
>>>>>>>>>>> + * @argc: Number of arguments
>>>>>>>>>>> + * @argv: Array of pointers to arguments
>>>>>>>>>>> + *
>>>>>>>>>>> + * Create an uefi capsule file, optionally signing it.
>>>>>>>>>>> + * Parse all the arguments and pass them on to create_fwbin().
>>>>>>>>>>> + *
>>>>>>>>>>> + * Return:
>>>>>>>>>>> + * * 0 - on success
>>>>>>>>>>> + * * -1 - on failure
>>>>>>>>>>> + */
>>>>>>>>>>> +int main(int argc, char **argv)
>>>>>>>>>>> +{
>>>>>>>>>>> + if (!strcmp(CONFIG_EFI_CAPSULE_CFG_FILE, ""))
>>>>>>>>>>> + capsule_with_cmdline_params(argc, argv);
>>>>>>>>>>> + else
>>>>>>>>>>> + capsule_with_cfg_file(CONFIG_EFI_CAPSULE_CFG_FILE);
>>>>>>>>>> I don't know where the macro, CONFIG_EFI_CAPSULE_CFG_FILE, comes from.
>>>>>>>>>> Anyhow, as a general rule, any host tool must be as generic as it should not
>>>>>>>>>> depend on a target's config.
>>>>>>>>>> (I was told so before.)
>>>>>>>>>>
>>>>>>>>>> So I would suggest that you add another command line, say "--config-file <file>",
>>>>>>>>>> to make the command generic.
>>>>>>>>> Yes, that would be something followed by most of the tools. The reason
>>>>>>>>> I did not add a command-line option for the confile file is because I
>>>>>>>>> want the capsule generation added as a make target. With the path to
>>>>>>>>> the config file specified through the Kconfig symbol, we can invoke
>>>>>>>>> 'make capsule', and it would build the capsules by parsing the
>>>>>>>>> parameters from the config file, taken from the Kconfig symbol. I know
>>>>>>>>> there are ways of specifying options when using a make command, but I
>>>>>>>>> don't think that is a clean way of doing things.
>>>>>>>> Not sure, but in your [5/7],
>>>>>>>> cmd_mkeficapsule = $(objtree)/tools/mkeficapsule --config-file $(CONFIG_EFI_CAPSULE_CFG_FILE)
>>>>>>>>
>>>>>>>> Doesn't this change work?
>>>>>>> So, I tried the above suggested change. But trying to run a make
>>>>>>> 'target' does not work without the .config file being present.
>>>>>> Not sure what you meant to say here.
>>>>>> Why don't you have .config when building U-Boot (or rather 'target')?
>>>>> Maybe I misunderstood your earlier comment, but I thought you were
>>>>> looking to build capsules without relying on a target config.
>>>> Not exactly.
>>>> The basic requirement, I believe, is that the exact same binary (with
>>>> the same set of functionalities) should be generated for any host tool
>>>> whatever a target's config, including tools-only_defconfig, is.
>>> Okay. I think I now understand what you are looking for. However, I
>>> believe if you want the same binary for both scenarios, the only way
>>> might be to drop the make target to generate capsules, and do it
>>> through the --config-file command-line option.
>> Again not sure what you're trying to do.
> Never mind. It works with the cfg-file being passed as a command-line
> parameter. I will make the change for the next version.
>
> -sughosh
>
>>> I will check if we can
>>> pass the config file as a parameter when building capsules as a
>>> target. I could not get it working when I tried it earlier though. If
>>> this is indeed not possible, do you have a strong opinion on having
>>> the same binary for both scenarios?
>> I have no reason why you can't.
>> If you see any failure, please give me more details about how
>> you are going to manage so that I can help you.
>>
>> -Takahiro Akashi
>>
>>> -sughosh
>>>
>>>> Is it clear now?
>>>>
>>>> -Takahiro Akashi
>>>>
>>>>> Which I
>>>>> believe cannot be done for a make target.
>>>>>
>>>>> -sughosh
>>>>>
>>>>>> -Takahiro Akashi
>>>>>>
>>>>>>> FWIW,
>>>>>>> the same is the case for building tools as well. I think that is the
>>>>>>> reason for the tools-only_defconfig.
>>>>>>>
>>>>>>> -sughosh
>>>>>>>
>>>>>>>> -Takahiro Akashi
>>>>>>>>
>>>>>>>>
>>>>>>>>> Given the use case of
>>>>>>>>> a make target, I hope we can use the Kconfig symbol for specifying the
>>>>>>>>> config file path.
>>>>>>>>>
>>>>>>>>> -sughosh
>>>>>>>>>
>>>>>>>>>> -Takahiro Akashi
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> exit(EXIT_SUCCESS);
>>>>>>>>>>> }
>>>>>>>>>>> diff --git a/tools/mkeficapsule_parse.c b/tools/mkeficapsule_parse.c
>>>>>>>>>>> new file mode 100644
>>>>>>>>>>> index 0000000000..ef4f3f6705
>>>>>>>>>>> --- /dev/null
>>>>>>>>>>> +++ b/tools/mkeficapsule_parse.c
>>>>>>>>>>> @@ -0,0 +1,345 @@
>>>>>>>>>>> +// SPDX-License-Identifier: GPL-2.0
>>>>>>>>>>> +/*
>>>>>>>>>>> + * Copyright 2023 Linaro Limited
>>>>>>>>>>> + */
>>>>>>>>>>> +
>>>>>>>>>>> +/*
>>>>>>>>>>> + * The code in this file adds parsing ability to the mkeficapsule
>>>>>>>>>>> + * tool. This allows specifying parameters needed to build the capsule
>>>>>>>>>>> + * through the config file instead of specifying them on the command-line.
>>>>>>>>>>> + * Parameters can be specified for more than one payload, generating the
>>>>>>>>>>> + * corresponding capsule files.
>>>>>>>>>>> + *
>>>>>>>>>>> + * The parameters are specified in a "key:value" pair. All the parameters
>>>>>>>>>>> + * that are currently supported by the mkeficapsule tool can be specified
>>>>>>>>>>> + * in the config file.
>>>>>>>>>>> + *
>>>>>>>>>>> + * The example below shows four payloads. The first payload is an example
>>>>>>>>>>> + * of generating a signed capsule. The second payload is an example of
>>>>>>>>>>> + * generating an unsigned capsule. The third payload is an accept empty
>>>>>>>>>>> + * capsule, while the fourth payload is the revert empty capsule, used
>>>>>>>>>>> + * for the multi-bank firmware update feature.
>>>>>>>>>>> + *
>>>>>>>>>>> + * This functionality can be easily extended to generate a single capsule
>>>>>>>>>>> + * comprising multiple payloads.
>>>>>>>>>>> +
>>>>>>>>>>> + {
>>>>>>>>>>> + image-guid: 02f4d760-cfd5-43bd-8e2d-a42acb33c660
>>>>>>>>>>> + hardware-instance: 0
>>>>>>>>>>> + monotonic-count: 1
>>>>>>>>>>> + payload: u-boot.bin
>>>>>>>>>>> + image-index: 1
>>>>>>>>>>> + private-key: /path/to/priv/key
>>>>>>>>>>> + pub-key-cert: /path/to/pub/key
>>>>>>>>>>> + capsule: u-boot.capsule
>>>>>>>>>>> + }
>>>>>>>>>>> + {
>>>>>>>>>>> + image-guid: 4ce292da-1dd8-428d-a1c2-77743ef8b96e
>>>>>>>>>>> + hardware-instance: 0
>>>>>>>>>>> + payload: u-boot.itb
>>>>>>>>>>> + image-index: 2
>>>>>>>>>>> + oemflags: 0x8000
>>>>>>>>>>> + capsule: fit.capsule
>>>>>>>>>>> + }
>>>>>>>>>>> + {
>>>>>>>>>>> + capsule-type: accept
>>>>>>>>>>> + image-guid: 4ce292da-1dd8-428d-a1c2-77743ef8b96e
>>>>>>>>>>> + capsule: accept.capsule
>>>>>>>>>>> + }
>>>>>>>>>>> + {
>>>>>>>>>>> + capsule-type: revert
>>>>>>>>>>> + capsule: revert.capsule
>>>>>>>>>>> + }
>>>>>>>>>>> +*/
>>>>>>>>>>> +
If i understand it correctly the EDK2 GenerateCapsule tool allows for
multiple
payloads inside one capsule by specifying a list of payloads in the
JSON-file.
I think something similar should be done here to support multiple payloads
inside one capsule. What about something like this:

{

content: [{
image-guid: 02f4d760-cfd5-43bd-8e2d-a42acb33c660
hardware-instance: 0
monotonic-count: 1
payload: u-boot.bin
image-index: 1
},{

image-guid: 02f4d760-cfd5-43bd-8e2d-a42acb33c660
hardware-instance: 1
monotonic-count: 1
payload: boot.bin
image-index: 2
}],

private-key: /path/to/priv/key
pub-key-cert: /path/to/pub/key
capsule: u-boot.capsule

}

?

Best Regards
Malte
>>>>>>>>>>> +#include <ctype.h>
>>>>>>>>>>> +#include <limits.h>
>>>>>>>>>>> +#include <stdio.h>
>>>>>>>>>>> +#include <stdlib.h>
>>>>>>>>>>> +#include <string.h>
>>>>>>>>>>> +
>>>>>>>>>>> +#include <uuid/uuid.h>
>>>>>>>>>>> +
>>>>>>>>>>> +#include "eficapsule.h"
>>>>>>>>>>> +
>>>>>>>>>>> +#define PARAMS_START "{"
>>>>>>>>>>> +#define PARAMS_END "}"
>>>>>>>>>>> +
>>>>>>>>>>> +#define PSTART 2
>>>>>>>>>>> +#define PEND 3
>>>>>>>>>>> +
>>>>>>>>>>> +#define MALLOC_FAIL_STR "Unable to allocate memory\n"
>>>>>>>>>>> +
>>>>>>>>>>> +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>>>>>>>>>>> +
>>>>>>>>>>> +const char *capsule_params[] = {
>>>>>>>>>>> + "image-guid", "image-index", "private-key",
>>>>>>>>>>> + "pub-key-cert", "payload", "capsule",
>>>>>>>>>>> + "hardware-instance", "monotonic-count",
>>>>>>>>>>> + "capsule-type", "oemflags" };
>>>>>>>>>>> +
>>>>>>>>>>> +static unsigned char params_start;
>>>>>>>>>>> +static unsigned char params_end;
>>>>>>>>>>> +
>>>>>>>>>>> +static void print_and_exit(const char *str)
>>>>>>>>>>> +{
>>>>>>>>>>> + fprintf(stderr, "%s", str);
>>>>>>>>>>> + exit(EXIT_FAILURE);
>>>>>>>>>>> +}
>>>>>>>>>>> +
>>>>>>>>>>> +static int param_delim_checks(char *line, unsigned char *token)
>>>>>>>>>>> +{
>>>>>>>>>>> + if (!strcmp(line, PARAMS_START)) {
>>>>>>>>>>> + if (params_start || !params_end) {
>>>>>>>>>>> + fprintf(stderr, "Earlier params processing still in progress. ");
>>>>>>>>>>> + fprintf(stderr, "Can't start processing a new params.\n");
>>>>>>>>>>> + exit(EXIT_FAILURE);
>>>>>>>>>>> + } else {
>>>>>>>>>>> + params_start = 1;
>>>>>>>>>>> + params_end = 0;
>>>>>>>>>>> + *token = PSTART;
>>>>>>>>>>> + return 1;
>>>>>>>>>>> + }
>>>>>>>>>>> + } else if (!strcmp(line, PARAMS_END)) {
>>>>>>>>>>> + if (!params_start) {
>>>>>>>>>>> + fprintf(stderr, "Cannot put end braces without start braces. ");
>>>>>>>>>>> + fprintf(stderr, "Please check the documentation for reference config file syntax\n");
>>>>>>>>>>> + exit(EXIT_FAILURE);
>>>>>>>>>>> + } else {
>>>>>>>>>>> + params_start = 0;
>>>>>>>>>>> + params_end = 1;
>>>>>>>>>>> + *token = PEND;
>>>>>>>>>>> + return 1;
>>>>>>>>>>> + }
>>>>>>>>>>> + } else if (!params_start) {
>>>>>>>>>>> + fprintf(stderr, "Params should be passed within braces. ");
>>>>>>>>>>> + fprintf(stderr, "Please check the documentation for reference config file syntax\n");
>>>>>>>>>>> + exit(EXIT_FAILURE);
>>>>>>>>>>> + }
>>>>>>>>>>> +
>>>>>>>>>>> + return 0;
>>>>>>>>>>> +}
>>>>>>>>>>> +
>>>>>>>>>>> +static void add_guid(efi_guid_t **guid_param, char *guid)
>>>>>>>>>>> +{
>>>>>>>>>>> + unsigned char uuid_buf[16];
>>>>>>>>>>> +
>>>>>>>>>>> + *guid_param = malloc(sizeof(efi_guid_t));
>>>>>>>>>>> + if (!*guid_param)
>>>>>>>>>>> + print_and_exit(MALLOC_FAIL_STR);
>>>>>>>>>>> +
>>>>>>>>>>> + if (uuid_parse(guid, uuid_buf))
>>>>>>>>>>> + print_and_exit("Wrong guid format\n");
>>>>>>>>>>> +
>>>>>>>>>>> + convert_uuid_to_guid(uuid_buf);
>>>>>>>>>>> + memcpy(*guid_param, uuid_buf, sizeof(efi_guid_t));
>>>>>>>>>>> +}
>>>>>>>>>>> +
>>>>>>>>>>> +static void add_string(char **dst, char *val)
>>>>>>>>>>> +{
>>>>>>>>>>> + *dst = strdup(val);
>>>>>>>>>>> + if (!*dst)
>>>>>>>>>>> + print_and_exit(MALLOC_FAIL_STR);
>>>>>>>>>>> +}
>>>>>>>>>>> +
>>>>>>>>>>> +static void match_and_populate_param(char *key, char *val,
>>>>>>>>>>> + struct efi_capsule_params *param)
>>>>>>>>>>> +{
>>>>>>>>>>> + int i;
>>>>>>>>>>> +
>>>>>>>>>>> + for (i = 0; i < ARRAY_SIZE(capsule_params); i++) {
>>>>>>>>>>> + if (!strcmp(key, capsule_params[i])) {
>>>>>>>>>>> + switch (i) {
>>>>>>>>>>> + case 0:
>>>>>>>>>>> + add_guid(&param->image_guid, val);
>>>>>>>>>>> + return;
>>>>>>>>>>> + case 1:
>>>>>>>>>>> + param->image_index = strtoul(val, NULL, 0);
>>>>>>>>>>> + if (param->image_index == ULONG_MAX)
>>>>>>>>>>> + print_and_exit("Enter a valid value of index bewtween 1-255");
>>>>>>>>>>> + return;
>>>>>>>>>>> + case 2:
>>>>>>>>>>> + add_string(&param->privkey_file, val);
>>>>>>>>>>> + return;
>>>>>>>>>>> + case 3:
>>>>>>>>>>> + add_string(&param->cert_file, val);
>>>>>>>>>>> + return;
>>>>>>>>>>> + case 4:
>>>>>>>>>>> + add_string(&param->input_file, val);
>>>>>>>>>>> + return;
>>>>>>>>>>> + case 5:
>>>>>>>>>>> + add_string(&param->capsule_file, val);
>>>>>>>>>>> + return;
>>>>>>>>>>> + case 6:
>>>>>>>>>>> + param->hardware_instance = strtoul(val, NULL, 0);
>>>>>>>>>>> + if (param->hardware_instance == ULONG_MAX)
>>>>>>>>>>> + print_and_exit("Enter a valid hardware instance value");
>>>>>>>>>>> + return;
>>>>>>>>>>> + case 7:
>>>>>>>>>>> + param->monotonic_count = strtoull(val, NULL, 0);
>>>>>>>>>>> + if (param->monotonic_count == ULLONG_MAX)
>>>>>>>>>>> + print_and_exit("Enter a valid monotonic count value");
>>>>>>>>>>> + return;
>>>>>>>>>>> + case 8:
>>>>>>>>>>> + if (!strcmp(val, "normal"))
>>>>>>>>>>> + param->capsule = CAPSULE_NORMAL_BLOB;
>>>>>>>>>>> + else if (!strcmp(val, "accept"))
>>>>>>>>>>> + param->capsule = CAPSULE_ACCEPT;
>>>>>>>>>>> + else if (!strcmp(val, "revert"))
>>>>>>>>>>> + param->capsule = CAPSULE_REVERT;
>>>>>>>>>>> + else
>>>>>>>>>>> + print_and_exit("Invalid type of capsule");
>>>>>>>>>>> +
>>>>>>>>>>> + return;
>>>>>>>>>>> + case 9:
>>>>>>>>>>> + param->oemflags = strtoul(val, NULL, 0);
>>>>>>>>>>> + if (param->oemflags > 0xffff)
>>>>>>>>>>> + print_and_exit("OemFlags must be between 0x0 and 0xffff\n");
>>>>>>>>>>> + return;
>>>>>>>>>>> + }
>>>>>>>>>>> + }
>>>>>>>>>>> + }
>>>>>>>>>>> +
>>>>>>>>>>> + fprintf(stderr, "Undefined param %s specified. ", key);
>>>>>>>>>>> + fprintf(stderr, "Please check the documentation for reference config file syntax\n");
>>>>>>>>>>> + exit(EXIT_FAILURE);
>>>>>>>>>>> +}
>>>>>>>>>>> +
>>>>>>>>>>> +static int get_capsule_params(char *line, struct efi_capsule_params *params)
>>>>>>>>>>> +{
>>>>>>>>>>> + char *key = NULL;
>>>>>>>>>>> + char *val = NULL;
>>>>>>>>>>> + unsigned char token;
>>>>>>>>>>> +
>>>>>>>>>>> + if (param_delim_checks(line, &token))
>>>>>>>>>>> + return token;
>>>>>>>>>>> +
>>>>>>>>>>> + key = strtok(line, ":");
>>>>>>>>>>> + if (key)
>>>>>>>>>>> + val = strtok(NULL, "\0");
>>>>>>>>>>> + else
>>>>>>>>>>> + print_and_exit("Expect the params in a key:value pair\n");
>>>>>>>>>>> +
>>>>>>>>>>> + match_and_populate_param(key, val, params);
>>>>>>>>>>> +
>>>>>>>>>>> + return 0;
>>>>>>>>>>> +}
>>>>>>>>>>> +
>>>>>>>>>>> +static char *skip_whitespace(char *line)
>>>>>>>>>>> +{
>>>>>>>>>>> + char *ptr, *newline;
>>>>>>>>>>> +
>>>>>>>>>>> + ptr = malloc(strlen(line) + 1);
>>>>>>>>>>> + if (!ptr)
>>>>>>>>>>> + print_and_exit(MALLOC_FAIL_STR);
>>>>>>>>>>> +
>>>>>>>>>>> + for (newline = ptr; *line; line++)
>>>>>>>>>>> + if (!isblank(*line))
>>>>>>>>>>> + *ptr++ = *line;
>>>>>>>>>>> + *ptr = '\0';
>>>>>>>>>>> + return newline;
>>>>>>>>>>> +}
>>>>>>>>>>> +
>>>>>>>>>>> +static int parse_capsule_payload_params(FILE *fp, struct efi_capsule_params *params)
>>>>>>>>>>> +{
>>>>>>>>>>> + char *line = NULL;
>>>>>>>>>>> + char *newline;
>>>>>>>>>>> + size_t n = 0;
>>>>>>>>>>> + ssize_t len;
>>>>>>>>>>> +
>>>>>>>>>>> + while ((len = getline(&line, &n, fp)) != -1) {
>>>>>>>>>>> + if (len == 1 && line[len - 1] == '\n')
>>>>>>>>>>> + continue;
>>>>>>>>>>> +
>>>>>>>>>>> + line[len - 1] = '\0';
>>>>>>>>>>> +
>>>>>>>>>>> + newline = skip_whitespace(line);
>>>>>>>>>>> +
>>>>>>>>>>> + if (newline[0] == '#')
>>>>>>>>>>> + continue;
>>>>>>>>>>> +
>>>>>>>>>>> + if (get_capsule_params(newline, params) == PEND)
>>>>>>>>>>> + return 0;
>>>>>>>>>>> + }
>>>>>>>>>>> +
>>>>>>>>>>> + if (errno == EINVAL || errno == ENOMEM) {
>>>>>>>>>>> + fprintf(stderr, "getline() returned an error %s reading the line\n",
>>>>>>>>>>> + strerror(errno));
>>>>>>>>>>> + exit(EXIT_FAILURE);
>>>>>>>>>>> + } else if (params_start == 1 || params_end == 0) {
>>>>>>>>>>> + fprintf(stderr, "Params should be passed within braces. ");
>>>>>>>>>>> + fprintf(stderr, "Please check the documentation for reference config file syntax\n");
>>>>>>>>>>> + exit(EXIT_FAILURE);
>>>>>>>>>>> + } else {
>>>>>>>>>>> + return -1;
>>>>>>>>>>> + }
>>>>>>>>>>> +}
>>>>>>>>>>> +
>>>>>>>>>>> +static void params_dependency_check(struct efi_capsule_params *params)
>>>>>>>>>>> +{
>>>>>>>>>>> + /* check necessary parameters */
>>>>>>>>>>> + if ((params->capsule == CAPSULE_NORMAL_BLOB &&
>>>>>>>>>>> + ((!params->input_file || !params->capsule_file ||
>>>>>>>>>>> + !params->image_guid) ||
>>>>>>>>>>> + ((params->privkey_file && !params->cert_file) ||
>>>>>>>>>>> + (!params->privkey_file && params->cert_file)))) ||
>>>>>>>>>>> + (params->capsule != CAPSULE_NORMAL_BLOB &&
>>>>>>>>>>> + (!params->capsule_file ||
>>>>>>>>>>> + (params->capsule == CAPSULE_ACCEPT && !params->image_guid) ||
>>>>>>>>>>> + (params->capsule == CAPSULE_REVERT && params->image_guid)))) {
>>>>>>>>>>> + print_usage();
>>>>>>>>>>> + exit(EXIT_FAILURE);
>>>>>>>>>>> + }
>>>>>>>>>>> +}
>>>>>>>>>>> +
>>>>>>>>>>> +static void generate_capsule(struct efi_capsule_params *params)
>>>>>>>>>>> +{
>>>>>>>>>>> + if (params->capsule != CAPSULE_NORMAL_BLOB) {
>>>>>>>>>>> + if (create_empty_capsule(params->capsule_file,
>>>>>>>>>>> + params->image_guid,
>>>>>>>>>>> + params->capsule ==
>>>>>>>>>>> + CAPSULE_ACCEPT) < 0)
>>>>>>>>>>> + print_and_exit("Creating empty capsule failed\n");
>>>>>>>>>>> + } else if (create_fwbin(params->capsule_file, params->input_file,
>>>>>>>>>>> + params->image_guid, params->image_index,
>>>>>>>>>>> + params->hardware_instance,
>>>>>>>>>>> + params->monotonic_count,
>>>>>>>>>>> + params->privkey_file,
>>>>>>>>>>> + params->cert_file,
>>>>>>>>>>> + (uint16_t)params->oemflags) < 0) {
>>>>>>>>>>> + print_and_exit("Creating firmware capsule failed\n");
>>>>>>>>>>> + }
>>>>>>>>>>> +}
>>>>>>>>>>> +
>>>>>>>>>>> +/**
>>>>>>>>>>> + * capsule_with_cfg_file() - Generate capsule from config file
>>>>>>>>>>> + * @cfg_file: Path to the config file
>>>>>>>>>>> + *
>>>>>>>>>>> + * Parse the capsule parameters from the config file and use the
>>>>>>>>>>> + * parameters for generating one or more capsules.
>>>>>>>>>>> + *
>>>>>>>>>>> + * Return: None
>>>>>>>>>>> + *
>>>>>>>>>>> + */
>>>>>>>>>>> +void capsule_with_cfg_file(const char *cfg_file)
>>>>>>>>>>> +{
>>>>>>>>>>> + FILE *fp;
>>>>>>>>>>> + struct efi_capsule_params params = { 0 };
>>>>>>>>>>> +
>>>>>>>>>>> + fp = fopen(cfg_file, "r");
>>>>>>>>>>> + if (!fp) {
>>>>>>>>>>> + fprintf(stderr, "Unable to open the capsule config file %s\n",
>>>>>>>>>>> + cfg_file);
>>>>>>>>>>> + exit(EXIT_FAILURE);
>>>>>>>>>>> + }
>>>>>>>>>>> +
>>>>>>>>>>> + params_start = 0;
>>>>>>>>>>> + params_end = 1;
>>>>>>>>>>> +
>>>>>>>>>>> + while (parse_capsule_payload_params(fp, &params) != -1) {
>>>>>>>>>>> + params_dependency_check(&params);
>>>>>>>>>>> + generate_capsule(&params);
>>>>>>>>>>> +
>>>>>>>>>>> + memset(&params, 0, sizeof(struct efi_capsule_params));
>>>>>>>>>>> + }
>>>>>>>>>>> +}
>>>>>>>>>>> --
>>>>>>>>>>> 2.34.1
>>>>>>>>>>>