[PATCH 0/4] Generate capsules from config files

[PATCH 0/4] Generate capsules from config files

[Sughosh Ganu]

Add support for generating capsules through a config file. Multiple
capsules can be generated through a single invocation of the
mkeficapsule tool, where the capsule parameters are fetched from the
config file.

The format of the config file pretty similar to the one used by the
capsule generation tool in EDKII.

The functionality is enabled on the sandbox variant and the EFI
capsule update test is now configured to generate the capsules through
a config file. The sandbox_flattree variant generates capsules through
binman, thus covering both the paths.

Sughosh Ganu (4):
  tools: mkeficapsule: Add support for parsing capsule params from
    config file
  doc: Document capsule generation through a config file
  sandbox: capsule: Add a config file for generating capsules
  test: capsule: Generate capsules through config file

 configs/sandbox_defconfig                     |   2 +
 doc/develop/uefi/uefi.rst                     |  70 ++++
 .../test_efi_capsule/capsule_gen_binman.dts   |   2 +
 test/py/tests/test_efi_capsule/conftest.py    |   7 +
 .../test_efi_capsule/sandbox_capsule_cfg.txt  | 162 ++++++++
 tools/Kconfig                                 |  15 +
 tools/Makefile                                |   1 +
 tools/eficapsule.h                            | 114 ++++++
 tools/mkeficapsule.c                          |  87 +++--
 tools/mkeficapsule_parse.c                    | 352 ++++++++++++++++++
 10 files changed, 781 insertions(+), 31 deletions(-)
 create mode 100644 test/py/tests/test_efi_capsule/sandbox_capsule_cfg.txt
 create mode 100644 tools/mkeficapsule_parse.c

-- 
2.34.1

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

[Sughosh Ganu]

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.

The config file can then be passed to the mkeficapsule tool in such
manner

 $ ./tools/mkeficapsule -f <path/to/the/config/file>

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
 tools/Kconfig              |  15 ++
 tools/Makefile             |   1 +
 tools/eficapsule.h         | 114 ++++++++++++
 tools/mkeficapsule.c       |  87 +++++----
 tools/mkeficapsule_parse.c | 352 +++++++++++++++++++++++++++++++++++++
 5 files changed, 538 insertions(+), 31 deletions(-)
 create mode 100644 tools/mkeficapsule_parse.c

diff --git a/tools/Kconfig b/tools/Kconfig
index 6e23f44d55..9408b60fac 100644
--- a/tools/Kconfig
+++ b/tools/Kconfig
@@ -98,6 +98,21 @@ 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"
+	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.
+
+config EFI_USE_CAPSULE_CFG_FILE
+	bool "Use the config file for generating capsules"
+	help
+	  Boolean option used to specify if the EFI capsules are to
+	  be generated through parameters specified via the config
+	  file or through command line.
+
 menuconfig FSPI_CONF_HEADER
 	bool "FlexSPI Header Configuration"
 	help
diff --git a/tools/Makefile b/tools/Makefile
index 1aa1e36137..ce7217e623 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -251,6 +251,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
 
 mkfwumdata-objs := mkfwumdata.o generated/lib/crc32.o
 HOSTLDLIBS_mkfwumdata += -luuid
diff --git a/tools/eficapsule.h b/tools/eficapsule.h
index 6efd07d2eb..71a08b62e6 100644
--- a/tools/eficapsule.h
+++ b/tools/eficapsule.h
@@ -54,6 +54,12 @@ typedef struct {
 /* flags */
 #define CAPSULE_FLAGS_PERSIST_ACROSS_RESET      0x00010000
 
+enum capsule_type {
+	CAPSULE_NORMAL_BLOB = 0,
+	CAPSULE_ACCEPT,
+	CAPSULE_REVERT,
+};
+
 struct efi_capsule_header {
 	efi_guid_t capsule_guid;
 	uint32_t header_size;
@@ -145,4 +151,112 @@ struct fmp_payload_header_params {
 	uint32_t fw_version;
 };
 
+/**
+ * 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
+ * @fmp: FMP payload header used for storing firmware version
+ * @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;
+	struct fmp_payload_header_params fmp;
+	uint64_t monotonic_count;
+	char *privkey_file;
+	char *cert_file;
+	char *input_file;
+	char *capsule_file;
+	unsigned long oemflags;
+	enum capsule_type capsule;
+};
+
+/**
+ * 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
+ * @fmp:	FMP header params
+ * @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,
+		 struct fmp_payload_header_params *fmp_ph_params,
+		 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 b8fc6069b5..3ea68d75aa 100644
--- a/tools/mkeficapsule.c
+++ b/tools/mkeficapsule.c
@@ -29,13 +29,7 @@ static const char *tool_name = "mkeficapsule";
 efi_guid_t efi_guid_fm_capsule = EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
 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:dhARD";
-
-enum {
-	CAPSULE_NORMAL_BLOB = 0,
-	CAPSULE_ACCEPT,
-	CAPSULE_REVERT,
-} capsule_type;
+static const char *opts_short = "g:i:I:v:p:c:m:o:f:dhARD";
 
 static struct option options[] = {
 	{"guid", required_argument, NULL, 'g'},
@@ -50,11 +44,21 @@ static struct option options[] = {
 	{"fw-revert", no_argument, NULL, 'R'},
 	{"capoemflag", required_argument, NULL, 'o'},
 	{"dump-capsule", no_argument, NULL, 'D'},
+	{"cfg-file", required_argument, NULL, 'f'},
 	{"help", no_argument, NULL, 'h'},
 	{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> <output file>\n"
 		"Options:\n"
@@ -71,6 +75,7 @@ static void print_usage(void)
 		"\t-R, --fw-revert  firmware revert capsule, takes no GUID, no image blob\n"
 		"\t-o, --capoemflag Capsule OEM Flag, an integer between 0x0000 and 0xffff\n"
 		"\t-D, --dump-capsule          dump the contents of the capsule headers\n"
+		"\t-f, --cfg-file <config file> config file with capsule parameters\n"
 		"\t-h, --help                  print a help message\n",
 		tool_name);
 }
@@ -390,6 +395,7 @@ static void free_sig_data(struct auth_context *ctx)
  * @guid:	GUID of related FMP driver
  * @index:	Index number in capsule
  * @instance:	Instance number in capsule
+ * @fmp:	FMP header params
  * @mcount:	Monotonic count in authentication information
  * @private_file:	Path to a private key file
  * @cert_file:	Path to a certificate file
@@ -404,11 +410,11 @@ 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,
-			struct fmp_payload_header_params *fmp_ph_params,
-			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,
+		 struct fmp_payload_header_params *fmp_ph_params,
+		 uint64_t mcount, char *privkey_file, char *cert_file,
+		 uint16_t oemflags)
 {
 	struct efi_capsule_header header;
 	struct efi_firmware_management_capsule_header capsule;
@@ -606,7 +612,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;
@@ -879,6 +899,8 @@ int main(int argc, char **argv)
 	unsigned long oemflags;
 	bool capsule_dump;
 	char *privkey_file, *cert_file;
+	char *cfg_file;
+	enum capsule_type capsule;
 	int c, idx;
 	struct fmp_payload_header_params fmp_ph_params = { 0 };
 
@@ -889,8 +911,9 @@ int main(int argc, char **argv)
 	privkey_file = NULL;
 	cert_file = NULL;
 	capsule_dump = false;
+	cfg_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);
@@ -944,20 +967,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);
@@ -970,6 +993,10 @@ int main(int argc, char **argv)
 		case 'D':
 			capsule_dump = true;
 			break;
+		case 'f':
+			cfg_file = optarg;
+			capsule_with_cfg_file(cfg_file);
+			exit(EXIT_SUCCESS);
 		default:
 			print_usage();
 			exit(EXIT_SUCCESS);
@@ -986,21 +1013,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);
 		}
@@ -1010,6 +1037,4 @@ int main(int argc, char **argv)
 		fprintf(stderr, "Creating firmware capsule failed\n");
 		exit(EXIT_FAILURE);
 	}
-
-	exit(EXIT_SUCCESS);
 }
diff --git a/tools/mkeficapsule_parse.c b/tools/mkeficapsule_parse.c
new file mode 100644
index 0000000000..0b010706d5
--- /dev/null
+++ b/tools/mkeficapsule_parse.c
@@ -0,0 +1,352 @@
+// 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
+	    fw-version: 2
+	    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
+	    fw-version: 10
+	    oemflags: 0x8000
+	    capsule: fit.capsule
+	}
+	{
+	    capsule-type: accept
+	    image-guid: 4ce292da-1dd8-428d-a1c2-77743ef8b96e
+	    capsule: accept.capsule
+	}
+	{
+	    capsule-type: revert
+	    capsule: revert.capsule
+	}
+*/
+
+#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", "fw-version" };
+
+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;
+			case 10:
+				param->fmp.fw_version = strtoul(val, NULL, 0);
+				param->fmp.have_header = true;
+				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->fmp,
+				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

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

[Simon Glass]

Hi Sughosh,

On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu <sughosh.ganu@linaro.org> 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.
>
> The config file can then be passed to the mkeficapsule tool in such
> manner
>
>  $ ./tools/mkeficapsule -f <path/to/the/config/file>
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
>  tools/Kconfig              |  15 ++
>  tools/Makefile             |   1 +
>  tools/eficapsule.h         | 114 ++++++++++++
>  tools/mkeficapsule.c       |  87 +++++----
>  tools/mkeficapsule_parse.c | 352 +++++++++++++++++++++++++++++++++++++
>  5 files changed, 538 insertions(+), 31 deletions(-)
>  create mode 100644 tools/mkeficapsule_parse.c

This patch keeps coming back :-)

Can we not add multiple capsules in the binman description? Why do we
need a new file format? How can binman decode images produced in this
way?

Also, could we get sandbox to produce one EFI capsule as part of the
normal build? I think that discussion trailed off.

Regards,
Simon

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1569 bytes --]

On Tue, Nov 21, 2023 at 03:11:50PM -0700, Simon Glass wrote:
> Hi Sughosh,
> 
> On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu <sughosh.ganu@linaro.org> 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.
> >
> > The config file can then be passed to the mkeficapsule tool in such
> > manner
> >
> >  $ ./tools/mkeficapsule -f <path/to/the/config/file>
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > ---
> >  tools/Kconfig              |  15 ++
> >  tools/Makefile             |   1 +
> >  tools/eficapsule.h         | 114 ++++++++++++
> >  tools/mkeficapsule.c       |  87 +++++----
> >  tools/mkeficapsule_parse.c | 352 +++++++++++++++++++++++++++++++++++++
> >  5 files changed, 538 insertions(+), 31 deletions(-)
> >  create mode 100644 tools/mkeficapsule_parse.c
> 
> This patch keeps coming back :-)
> 
> Can we not add multiple capsules in the binman description? Why do we
> need a new file format? How can binman decode images produced in this
> way?
> 
> Also, could we get sandbox to produce one EFI capsule as part of the
> normal build? I think that discussion trailed off.

I believe part of the answer here is to support the config file format
that other capsule generators use. Do I recall this all right, Sughosh?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

[Sughosh Ganu]

hi Tom,

On Wed, 22 Nov 2023 at 03:54, Tom Rini <trini@konsulko.com> wrote:
>
> On Tue, Nov 21, 2023 at 03:11:50PM -0700, Simon Glass wrote:
> > Hi Sughosh,
> >
> > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu <sughosh.ganu@linaro.org> 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.
> > >
> > > The config file can then be passed to the mkeficapsule tool in such
> > > manner
> > >
> > >  $ ./tools/mkeficapsule -f <path/to/the/config/file>
> > >
> > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > ---
> > >  tools/Kconfig              |  15 ++
> > >  tools/Makefile             |   1 +
> > >  tools/eficapsule.h         | 114 ++++++++++++
> > >  tools/mkeficapsule.c       |  87 +++++----
> > >  tools/mkeficapsule_parse.c | 352 +++++++++++++++++++++++++++++++++++++
> > >  5 files changed, 538 insertions(+), 31 deletions(-)
> > >  create mode 100644 tools/mkeficapsule_parse.c
> >
> > This patch keeps coming back :-)
> >
> > Can we not add multiple capsules in the binman description? Why do we
> > need a new file format? How can binman decode images produced in this
> > way?
> >
> > Also, could we get sandbox to produce one EFI capsule as part of the
> > normal build? I think that discussion trailed off.
>
> I believe part of the answer here is to support the config file format
> that other capsule generators use. Do I recall this all right, Sughosh?

Yes, that is correct. There are some other reasons as well, which I
have mentioned in the reply to Simon's email.

-sughosh

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

[Sughosh Ganu]

hi Simon,

On Wed, 22 Nov 2023 at 03:42, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Sughosh,
>
> On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu <sughosh.ganu@linaro.org> 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.
> >
> > The config file can then be passed to the mkeficapsule tool in such
> > manner
> >
> >  $ ./tools/mkeficapsule -f <path/to/the/config/file>
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > ---
> >  tools/Kconfig              |  15 ++
> >  tools/Makefile             |   1 +
> >  tools/eficapsule.h         | 114 ++++++++++++
> >  tools/mkeficapsule.c       |  87 +++++----
> >  tools/mkeficapsule_parse.c | 352 +++++++++++++++++++++++++++++++++++++
> >  5 files changed, 538 insertions(+), 31 deletions(-)
> >  create mode 100644 tools/mkeficapsule_parse.c
>
> This patch keeps coming back :-)
>
> Can we not add multiple capsules in the binman description? Why do we
> need a new file format? How can binman decode images produced in this
> way?

So as Tom mentions, this brings parity with respect to the other
capsule generation tool in EDKII that generates capsules. IIRC, this
is something which even Xilix was interested in, and Michal had kind
of gone through these patches earlier. Lastly, it would be good to
have support in U-Boot's mkeficapsule tool for generating a single
capsule file with multiple payloads, and having support for this
functionality helps in that goal.

Also, you might have noticed that, since your objection to the last
series, I have removed putting this in binman. So now, this aspect of
the capsule generation would only be supported through the
command-line invocation of the tool.

>
> Also, could we get sandbox to produce one EFI capsule as part of the
> normal build? I think that discussion trailed off.

Yes, apologies for missing out on this. Slipped my mind. Would you
want, say, all the non-signed capsules to be generated as part of the
sandbox build?

-sughosh

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

[Ilias Apalodimas]

Hi all,

On Wed, 22 Nov 2023 at 07:23, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> hi Simon,
>
> On Wed, 22 Nov 2023 at 03:42, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Sughosh,
> >
> > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu <sughosh.ganu@linaro.org> 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.
> > >
> > > The config file can then be passed to the mkeficapsule tool in such
> > > manner
> > >
> > >  $ ./tools/mkeficapsule -f <path/to/the/config/file>
> > >
> > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > ---
> > >  tools/Kconfig              |  15 ++
> > >  tools/Makefile             |   1 +
> > >  tools/eficapsule.h         | 114 ++++++++++++
> > >  tools/mkeficapsule.c       |  87 +++++----
> > >  tools/mkeficapsule_parse.c | 352 +++++++++++++++++++++++++++++++++++++
> > >  5 files changed, 538 insertions(+), 31 deletions(-)
> > >  create mode 100644 tools/mkeficapsule_parse.c
> >
> > This patch keeps coming back :-)
> >
> > Can we not add multiple capsules in the binman description? Why do we
> > need a new file format? How can binman decode images produced in this
> > way?
>
> So as Tom mentions, this brings parity with respect to the other
> capsule generation tool in EDKII that generates capsules. IIRC, this
> is something which even Xilix was interested in, and Michal had kind
> of gone through these patches earlier. Lastly, it would be good to
> have support in U-Boot's mkeficapsule tool for generating a single
> capsule file with multiple payloads, and having support for this
> functionality helps in that goal.
>
> Also, you might have noticed that, since your objection to the last
> series, I have removed putting this in binman. So now, this aspect of
> the capsule generation would only be supported through the
> command-line invocation of the tool.

I think that overall the approach is sane. mkeficapsule is currently
supported and compiled for distros, so the multiple payload support is
useful. If we want to add support to binman, instead of rewriting this
in python, we could just call that tool for parsing and creating
capsules

Thanks
/Ilias
>
> >
> > Also, could we get sandbox to produce one EFI capsule as part of the
> > normal build? I think that discussion trailed off.
>
> Yes, apologies for missing out on this. Slipped my mind. Would you
> want, say, all the non-signed capsules to be generated as part of the
> sandbox build?
>
> -sughosh

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

[Sughosh Ganu]

hi Ilias,

On Wed, 22 Nov 2023 at 13:06, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi all,
>
> On Wed, 22 Nov 2023 at 07:23, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> >
> > hi Simon,
> >
> > On Wed, 22 Nov 2023 at 03:42, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu <sughosh.ganu@linaro.org> 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.
> > > >
> > > > The config file can then be passed to the mkeficapsule tool in such
> > > > manner
> > > >
> > > >  $ ./tools/mkeficapsule -f <path/to/the/config/file>
> > > >
> > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > > ---
> > > >  tools/Kconfig              |  15 ++
> > > >  tools/Makefile             |   1 +
> > > >  tools/eficapsule.h         | 114 ++++++++++++
> > > >  tools/mkeficapsule.c       |  87 +++++----
> > > >  tools/mkeficapsule_parse.c | 352 +++++++++++++++++++++++++++++++++++++
> > > >  5 files changed, 538 insertions(+), 31 deletions(-)
> > > >  create mode 100644 tools/mkeficapsule_parse.c
> > >
> > > This patch keeps coming back :-)
> > >
> > > Can we not add multiple capsules in the binman description? Why do we
> > > need a new file format? How can binman decode images produced in this
> > > way?
> >
> > So as Tom mentions, this brings parity with respect to the other
> > capsule generation tool in EDKII that generates capsules. IIRC, this
> > is something which even Xilix was interested in, and Michal had kind
> > of gone through these patches earlier. Lastly, it would be good to
> > have support in U-Boot's mkeficapsule tool for generating a single
> > capsule file with multiple payloads, and having support for this
> > functionality helps in that goal.
> >
> > Also, you might have noticed that, since your objection to the last
> > series, I have removed putting this in binman. So now, this aspect of
> > the capsule generation would only be supported through the
> > command-line invocation of the tool.
>
> I think that overall the approach is sane. mkeficapsule is currently
> supported and compiled for distros, so the multiple payload support is
> useful. If we want to add support to binman, instead of rewriting this
> in python, we could just call that tool for parsing and creating
> capsules

Given the amount of time these patches have been under review(also
number of iterations), I would request that this series be reviewed
and merged first. I think there is general consensus that there is
value to have this functionality in the mkeficapsule tool. If it is
deemed fit to support this through binman as well, that task can be
taken up separately. Thanks.

-sughosh

>
> Thanks
> /Ilias
> >
> > >
> > > Also, could we get sandbox to produce one EFI capsule as part of the
> > > normal build? I think that discussion trailed off.
> >
> > Yes, apologies for missing out on this. Slipped my mind. Would you
> > want, say, all the non-signed capsules to be generated as part of the
> > sandbox build?
> >
> > -sughosh

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

[Simon Glass]

Hi Sughosh,

On Wed, 22 Nov 2023 at 00:40, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> hi Ilias,
>
> On Wed, 22 Nov 2023 at 13:06, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > Hi all,
> >
> > On Wed, 22 Nov 2023 at 07:23, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > >
> > > hi Simon,
> > >
> > > On Wed, 22 Nov 2023 at 03:42, Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Sughosh,
> > > >
> > > > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu <sughosh.ganu@linaro.org> 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.
> > > > >
> > > > > The config file can then be passed to the mkeficapsule tool in such
> > > > > manner
> > > > >
> > > > >  $ ./tools/mkeficapsule -f <path/to/the/config/file>
> > > > >
> > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > > > ---
> > > > >  tools/Kconfig              |  15 ++
> > > > >  tools/Makefile             |   1 +
> > > > >  tools/eficapsule.h         | 114 ++++++++++++
> > > > >  tools/mkeficapsule.c       |  87 +++++----
> > > > >  tools/mkeficapsule_parse.c | 352 +++++++++++++++++++++++++++++++++++++
> > > > >  5 files changed, 538 insertions(+), 31 deletions(-)
> > > > >  create mode 100644 tools/mkeficapsule_parse.c
> > > >
> > > > This patch keeps coming back :-)
> > > >
> > > > Can we not add multiple capsules in the binman description? Why do we
> > > > need a new file format? How can binman decode images produced in this
> > > > way?
> > >
> > > So as Tom mentions, this brings parity with respect to the other
> > > capsule generation tool in EDKII that generates capsules. IIRC, this
> > > is something which even Xilix was interested in, and Michal had kind
> > > of gone through these patches earlier. Lastly, it would be good to
> > > have support in U-Boot's mkeficapsule tool for generating a single
> > > capsule file with multiple payloads, and having support for this
> > > functionality helps in that goal.
> > >
> > > Also, you might have noticed that, since your objection to the last
> > > series, I have removed putting this in binman. So now, this aspect of
> > > the capsule generation would only be supported through the
> > > command-line invocation of the tool.
> >
> > I think that overall the approach is sane. mkeficapsule is currently
> > supported and compiled for distros, so the multiple payload support is
> > useful. If we want to add support to binman, instead of rewriting this
> > in python, we could just call that tool for parsing and creating
> > capsules
>
> Given the amount of time these patches have been under review(also
> number of iterations), I would request that this series be reviewed
> and merged first. I think there is general consensus that there is
> value to have this functionality in the mkeficapsule tool. If it is
> deemed fit to support this through binman as well, that task can be
> taken up separately. Thanks.

The point you are missing is that it is the entire goal of 'skirting
around' binman which is suspect.

If there is a need to generate an output file from the build, we
should support it in binman. If people start creating configuration
files all over the place, then they are not using binman, right?

I understand that there are pre-existing vendor-specific config files,
etc. that the EFI thing is a grey area, but I cannot imagine that this
patch would lead to a good outcome.

The goal of binman is to bring order to the chaos of firmware
packaging...we cannot do that if it is not actually used.

So let's figure out what is missing from binman's capsule generation
(multiple capsules? accept/reject capsules?) and how best to add it.

>
> -sughosh
>
> >
> > Thanks
> > /Ilias
> > >
> > > >
> > > > Also, could we get sandbox to produce one EFI capsule as part of the
> > > > normal build? I think that discussion trailed off.
> > >
> > > Yes, apologies for missing out on this. Slipped my mind. Would you
> > > want, say, all the non-signed capsules to be generated as part of the
> > > sandbox build?
> > >
> > > -sughosh

Regards,
Simon

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

[Sughosh Ganu]

hi Simon,

On Thu, 30 Nov 2023 at 08:16, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Sughosh,
>
> On Wed, 22 Nov 2023 at 00:40, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> >
> > hi Ilias,
> >
> > On Wed, 22 Nov 2023 at 13:06, Ilias Apalodimas
> > <ilias.apalodimas@linaro.org> wrote:
> > >
> > > Hi all,
> > >
> > > On Wed, 22 Nov 2023 at 07:23, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > > >
> > > > hi Simon,
> > > >
> > > > On Wed, 22 Nov 2023 at 03:42, Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > Hi Sughosh,
> > > > >
> > > > > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu <sughosh.ganu@linaro.org> 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.
> > > > > >
> > > > > > The config file can then be passed to the mkeficapsule tool in such
> > > > > > manner
> > > > > >
> > > > > >  $ ./tools/mkeficapsule -f <path/to/the/config/file>
> > > > > >
> > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > > > > ---
> > > > > >  tools/Kconfig              |  15 ++
> > > > > >  tools/Makefile             |   1 +
> > > > > >  tools/eficapsule.h         | 114 ++++++++++++
> > > > > >  tools/mkeficapsule.c       |  87 +++++----
> > > > > >  tools/mkeficapsule_parse.c | 352 +++++++++++++++++++++++++++++++++++++
> > > > > >  5 files changed, 538 insertions(+), 31 deletions(-)
> > > > > >  create mode 100644 tools/mkeficapsule_parse.c
> > > > >
> > > > > This patch keeps coming back :-)
> > > > >
> > > > > Can we not add multiple capsules in the binman description? Why do we
> > > > > need a new file format? How can binman decode images produced in this
> > > > > way?
> > > >
> > > > So as Tom mentions, this brings parity with respect to the other
> > > > capsule generation tool in EDKII that generates capsules. IIRC, this
> > > > is something which even Xilix was interested in, and Michal had kind
> > > > of gone through these patches earlier. Lastly, it would be good to
> > > > have support in U-Boot's mkeficapsule tool for generating a single
> > > > capsule file with multiple payloads, and having support for this
> > > > functionality helps in that goal.
> > > >
> > > > Also, you might have noticed that, since your objection to the last
> > > > series, I have removed putting this in binman. So now, this aspect of
> > > > the capsule generation would only be supported through the
> > > > command-line invocation of the tool.
> > >
> > > I think that overall the approach is sane. mkeficapsule is currently
> > > supported and compiled for distros, so the multiple payload support is
> > > useful. If we want to add support to binman, instead of rewriting this
> > > in python, we could just call that tool for parsing and creating
> > > capsules
> >
> > Given the amount of time these patches have been under review(also
> > number of iterations), I would request that this series be reviewed
> > and merged first. I think there is general consensus that there is
> > value to have this functionality in the mkeficapsule tool. If it is
> > deemed fit to support this through binman as well, that task can be
> > taken up separately. Thanks.
>
> The point you are missing is that it is the entire goal of 'skirting
> around' binman which is suspect.
>
> If there is a need to generate an output file from the build, we
> should support it in binman. If people start creating configuration
> files all over the place, then they are not using binman, right?
>
> I understand that there are pre-existing vendor-specific config files,
> etc. that the EFI thing is a grey area, but I cannot imagine that this
> patch would lead to a good outcome.
>
> The goal of binman is to bring order to the chaos of firmware
> packaging...we cannot do that if it is not actually used.
>
> So let's figure out what is missing from binman's capsule generation
> (multiple capsules? accept/reject capsules?) and how best to add it.

I think I need to jog your memory back a bit. For context, I have
jotted down the points.

* The mkeficapsule tool generates capsules in U-Boot.
* Currently, when the tool is invoked from the command-line, the
capsules are generated by passing the capsule parameters as cmd-line
options.
* I had earlier added support for generating the capsules as part of
U-Boot build, through binman. This support has been merged.
* I had followed these patches up with another series [1] which
generates capsules by parsing the capsule parameters through a config
file instead of cmd-line options.
* This series also had patches which were attempting to integrate this
functionality into binman [2].
* As part of reviewing the patch series, you had objected to adding
this support in binman, primarily because this way of specifying the
capsule parameters goes against the normal way of image description in
binman [3].
* I have described in this mail thread about why we need to have
support for generating capsules through config files [4].

So, in essence, this functionality is needed to be added to the tool.
I have earlier tried integrating this in binman, but that was
rejected. So, the way I see this moving ahead is to first add support
for this feature in the tool, and then see if this can also be added
in binman. As I mentioned earlier, I am fine with this functionality
not getting integrated with binman, if this contravenes the idea of
describing images in binman, and if no exceptions can be made on that
regard. But please do understand that no attempt is being made at
'skirting around binman'. In fact, I had worked on your earlier
objection of using absolute paths in testing this functionality in
binman. But then you had put the other objection of how this does not
follow the idea or concept of image description in binman. Hence this
approach.

-sughosh

[1] - https://lore.kernel.org/u-boot/20230908120002.29851-1-sughosh.ganu@linaro.org/
[2] - https://lore.kernel.org/u-boot/20230908120002.29851-4-sughosh.ganu@linaro.org/
[3] - https://lore.kernel.org/u-boot/CAPnjgZ3Zi3hmQg7d5+0461hRK+5O_V89PY=QxMwQarhhqUW6VQ@mail.gmail.com/
[4] - https://lore.kernel.org/u-boot/CADg8p94EvJc6HNKvk2_XWaDb97u28vGfZasnv2je4yJw1k860Q@mail.gmail.com/

>
> >
> > -sughosh
> >
> > >
> > > Thanks
> > > /Ilias
> > > >
> > > > >
> > > > > Also, could we get sandbox to produce one EFI capsule as part of the
> > > > > normal build? I think that discussion trailed off.
> > > >
> > > > Yes, apologies for missing out on this. Slipped my mind. Would you
> > > > want, say, all the non-signed capsules to be generated as part of the
> > > > sandbox build?
> > > >
> > > > -sughosh
>
> Regards,
> Simon

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

[Simon Glass]

Hi Sughosh,

On Thu, 30 Nov 2023 at 23:39, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> hi Simon,
>
> On Thu, 30 Nov 2023 at 08:16, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Sughosh,
> >
> > On Wed, 22 Nov 2023 at 00:40, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > >
> > > hi Ilias,
> > >
> > > On Wed, 22 Nov 2023 at 13:06, Ilias Apalodimas
> > > <ilias.apalodimas@linaro.org> wrote:
> > > >
> > > > Hi all,
> > > >
> > > > On Wed, 22 Nov 2023 at 07:23, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > > > >
> > > > > hi Simon,
> > > > >
> > > > > On Wed, 22 Nov 2023 at 03:42, Simon Glass <sjg@chromium.org> wrote:
> > > > > >
> > > > > > Hi Sughosh,
> > > > > >
> > > > > > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu <sughosh.ganu@linaro.org> 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.
> > > > > > >
> > > > > > > The config file can then be passed to the mkeficapsule tool in such
> > > > > > > manner
> > > > > > >
> > > > > > >  $ ./tools/mkeficapsule -f <path/to/the/config/file>
> > > > > > >
> > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > > > > > ---
> > > > > > >  tools/Kconfig              |  15 ++
> > > > > > >  tools/Makefile             |   1 +
> > > > > > >  tools/eficapsule.h         | 114 ++++++++++++
> > > > > > >  tools/mkeficapsule.c       |  87 +++++----
> > > > > > >  tools/mkeficapsule_parse.c | 352 +++++++++++++++++++++++++++++++++++++
> > > > > > >  5 files changed, 538 insertions(+), 31 deletions(-)
> > > > > > >  create mode 100644 tools/mkeficapsule_parse.c
> > > > > >
> > > > > > This patch keeps coming back :-)
> > > > > >
> > > > > > Can we not add multiple capsules in the binman description? Why do we
> > > > > > need a new file format? How can binman decode images produced in this
> > > > > > way?
> > > > >
> > > > > So as Tom mentions, this brings parity with respect to the other
> > > > > capsule generation tool in EDKII that generates capsules. IIRC, this
> > > > > is something which even Xilix was interested in, and Michal had kind
> > > > > of gone through these patches earlier. Lastly, it would be good to
> > > > > have support in U-Boot's mkeficapsule tool for generating a single
> > > > > capsule file with multiple payloads, and having support for this
> > > > > functionality helps in that goal.
> > > > >
> > > > > Also, you might have noticed that, since your objection to the last
> > > > > series, I have removed putting this in binman. So now, this aspect of
> > > > > the capsule generation would only be supported through the
> > > > > command-line invocation of the tool.
> > > >
> > > > I think that overall the approach is sane. mkeficapsule is currently
> > > > supported and compiled for distros, so the multiple payload support is
> > > > useful. If we want to add support to binman, instead of rewriting this
> > > > in python, we could just call that tool for parsing and creating
> > > > capsules
> > >
> > > Given the amount of time these patches have been under review(also
> > > number of iterations), I would request that this series be reviewed
> > > and merged first. I think there is general consensus that there is
> > > value to have this functionality in the mkeficapsule tool. If it is
> > > deemed fit to support this through binman as well, that task can be
> > > taken up separately. Thanks.
> >
> > The point you are missing is that it is the entire goal of 'skirting
> > around' binman which is suspect.
> >
> > If there is a need to generate an output file from the build, we
> > should support it in binman. If people start creating configuration
> > files all over the place, then they are not using binman, right?
> >
> > I understand that there are pre-existing vendor-specific config files,
> > etc. that the EFI thing is a grey area, but I cannot imagine that this
> > patch would lead to a good outcome.
> >
> > The goal of binman is to bring order to the chaos of firmware
> > packaging...we cannot do that if it is not actually used.
> >
> > So let's figure out what is missing from binman's capsule generation
> > (multiple capsules? accept/reject capsules?) and how best to add it.
>
> I think I need to jog your memory back a bit. For context, I have
> jotted down the points.
>
> * The mkeficapsule tool generates capsules in U-Boot.
> * Currently, when the tool is invoked from the command-line, the
> capsules are generated by passing the capsule parameters as cmd-line
> options.
> * I had earlier added support for generating the capsules as part of
> U-Boot build, through binman. This support has been merged.
> * I had followed these patches up with another series [1] which
> generates capsules by parsing the capsule parameters through a config
> file instead of cmd-line options.
> * This series also had patches which were attempting to integrate this
> functionality into binman [2].
> * As part of reviewing the patch series, you had objected to adding
> this support in binman, primarily because this way of specifying the
> capsule parameters goes against the normal way of image description in
> binman [3].
> * I have described in this mail thread about why we need to have
> support for generating capsules through config files [4].

Thanks for that. Yes this fits with my memory, such as it is.

I regard the tool as a binman tool, though, in the sense that binman
is responsible for generating the images. For example, binman calls
mkimage to handle FIT images. People are free to write a script to
call mkimage manually, but the in-tree use of mkimage is supposed to
be with binman.

>
> So, in essence, this functionality is needed to be added to the tool.
> I have earlier tried integrating this in binman, but that was
> rejected. So, the way I see this moving ahead is to first add support
> for this feature in the tool, and then see if this can also be added
> in binman. As I mentioned earlier, I am fine with this functionality
> not getting integrated with binman, if this contravenes the idea of
> describing images in binman, and if no exceptions can be made on that
> regard. But please do understand that no attempt is being made at
> 'skirting around binman'. In fact, I had worked on your earlier
> objection of using absolute paths in testing this functionality in
> binman. But then you had put the other objection of how this does not
> follow the idea or concept of image description in binman. Hence this
> approach.

It would be easier to talk this through f2f, but I will make an attempt here.

The goal here is for people to be able to create an EFI capsule (or
more than one?) automatically as part of the build. Since the config
is in the binman description, putting it somewhere else doesn't help
anyone. It will lead to .cfg files in U-Boot and Binman will just be a
side show. That is my concern.

I would really like to see a capsule generated for sandbox so I can
understand this better.

Anyway, I hope that makes sense? If not, I think we should discuss it
f2f or VC or somehow.

Regards,
Simon

>
> -sughosh
>
> [1] - https://lore.kernel.org/u-boot/20230908120002.29851-1-sughosh.ganu@linaro.org/
> [2] - https://lore.kernel.org/u-boot/20230908120002.29851-4-sughosh.ganu@linaro.org/
> [3] - https://lore.kernel.org/u-boot/CAPnjgZ3Zi3hmQg7d5+0461hRK+5O_V89PY=QxMwQarhhqUW6VQ@mail.gmail.com/
> [4] - https://lore.kernel.org/u-boot/CADg8p94EvJc6HNKvk2_XWaDb97u28vGfZasnv2je4yJw1k860Q@mail.gmail.com/
>
> >
> > >
> > > -sughosh
> > >
> > > >
> > > > Thanks
> > > > /Ilias
> > > > >
> > > > > >
> > > > > > Also, could we get sandbox to produce one EFI capsule as part of the
> > > > > > normal build? I think that discussion trailed off.
> > > > >
> > > > > Yes, apologies for missing out on this. Slipped my mind. Would you
> > > > > want, say, all the non-signed capsules to be generated as part of the
> > > > > sandbox build?
> > > > >
> > > > > -sughosh
> >
> > Regards,
> > Simon

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

[Sughosh Ganu]

hi Simon,

On Sat, 2 Dec 2023 at 00:02, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Sughosh,
>
> On Thu, 30 Nov 2023 at 23:39, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> >
> > hi Simon,
> >
> > On Thu, 30 Nov 2023 at 08:16, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Wed, 22 Nov 2023 at 00:40, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > > >
> > > > hi Ilias,
> > > >
> > > > On Wed, 22 Nov 2023 at 13:06, Ilias Apalodimas
> > > > <ilias.apalodimas@linaro.org> wrote:
> > > > >
> > > > > Hi all,
> > > > >
> > > > > On Wed, 22 Nov 2023 at 07:23, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > > > > >
> > > > > > hi Simon,
> > > > > >
> > > > > > On Wed, 22 Nov 2023 at 03:42, Simon Glass <sjg@chromium.org> wrote:
> > > > > > >
> > > > > > > Hi Sughosh,
> > > > > > >
> > > > > > > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu <sughosh.ganu@linaro.org> 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.
> > > > > > > >
> > > > > > > > The config file can then be passed to the mkeficapsule tool in such
> > > > > > > > manner
> > > > > > > >
> > > > > > > >  $ ./tools/mkeficapsule -f <path/to/the/config/file>
> > > > > > > >
> > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > > > > > > ---
> > > > > > > >  tools/Kconfig              |  15 ++
> > > > > > > >  tools/Makefile             |   1 +
> > > > > > > >  tools/eficapsule.h         | 114 ++++++++++++
> > > > > > > >  tools/mkeficapsule.c       |  87 +++++----
> > > > > > > >  tools/mkeficapsule_parse.c | 352 +++++++++++++++++++++++++++++++++++++
> > > > > > > >  5 files changed, 538 insertions(+), 31 deletions(-)
> > > > > > > >  create mode 100644 tools/mkeficapsule_parse.c
> > > > > > >
> > > > > > > This patch keeps coming back :-)
> > > > > > >
> > > > > > > Can we not add multiple capsules in the binman description? Why do we
> > > > > > > need a new file format? How can binman decode images produced in this
> > > > > > > way?
> > > > > >
> > > > > > So as Tom mentions, this brings parity with respect to the other
> > > > > > capsule generation tool in EDKII that generates capsules. IIRC, this
> > > > > > is something which even Xilix was interested in, and Michal had kind
> > > > > > of gone through these patches earlier. Lastly, it would be good to
> > > > > > have support in U-Boot's mkeficapsule tool for generating a single
> > > > > > capsule file with multiple payloads, and having support for this
> > > > > > functionality helps in that goal.
> > > > > >
> > > > > > Also, you might have noticed that, since your objection to the last
> > > > > > series, I have removed putting this in binman. So now, this aspect of
> > > > > > the capsule generation would only be supported through the
> > > > > > command-line invocation of the tool.
> > > > >
> > > > > I think that overall the approach is sane. mkeficapsule is currently
> > > > > supported and compiled for distros, so the multiple payload support is
> > > > > useful. If we want to add support to binman, instead of rewriting this
> > > > > in python, we could just call that tool for parsing and creating
> > > > > capsules
> > > >
> > > > Given the amount of time these patches have been under review(also
> > > > number of iterations), I would request that this series be reviewed
> > > > and merged first. I think there is general consensus that there is
> > > > value to have this functionality in the mkeficapsule tool. If it is
> > > > deemed fit to support this through binman as well, that task can be
> > > > taken up separately. Thanks.
> > >
> > > The point you are missing is that it is the entire goal of 'skirting
> > > around' binman which is suspect.
> > >
> > > If there is a need to generate an output file from the build, we
> > > should support it in binman. If people start creating configuration
> > > files all over the place, then they are not using binman, right?
> > >
> > > I understand that there are pre-existing vendor-specific config files,
> > > etc. that the EFI thing is a grey area, but I cannot imagine that this
> > > patch would lead to a good outcome.
> > >
> > > The goal of binman is to bring order to the chaos of firmware
> > > packaging...we cannot do that if it is not actually used.
> > >
> > > So let's figure out what is missing from binman's capsule generation
> > > (multiple capsules? accept/reject capsules?) and how best to add it.
> >
> > I think I need to jog your memory back a bit. For context, I have
> > jotted down the points.
> >
> > * The mkeficapsule tool generates capsules in U-Boot.
> > * Currently, when the tool is invoked from the command-line, the
> > capsules are generated by passing the capsule parameters as cmd-line
> > options.
> > * I had earlier added support for generating the capsules as part of
> > U-Boot build, through binman. This support has been merged.
> > * I had followed these patches up with another series [1] which
> > generates capsules by parsing the capsule parameters through a config
> > file instead of cmd-line options.
> > * This series also had patches which were attempting to integrate this
> > functionality into binman [2].
> > * As part of reviewing the patch series, you had objected to adding
> > this support in binman, primarily because this way of specifying the
> > capsule parameters goes against the normal way of image description in
> > binman [3].
> > * I have described in this mail thread about why we need to have
> > support for generating capsules through config files [4].
>
> Thanks for that. Yes this fits with my memory, such as it is.
>
> I regard the tool as a binman tool, though, in the sense that binman
> is responsible for generating the images. For example, binman calls
> mkimage to handle FIT images. People are free to write a script to
> call mkimage manually, but the in-tree use of mkimage is supposed to
> be with binman.

No issues with supporting the functionality through binman. However, I
think that if binman is to be used as the encompassing packaging tool
which in turn calls other tools like mkimage or mkeficapsule, it
should make allowances for accepting formats, or ways of describing
payloads that might not sit with it's core design of "each image is a
DT node".

>
> >
> > So, in essence, this functionality is needed to be added to the tool.
> > I have earlier tried integrating this in binman, but that was
> > rejected. So, the way I see this moving ahead is to first add support
> > for this feature in the tool, and then see if this can also be added
> > in binman. As I mentioned earlier, I am fine with this functionality
> > not getting integrated with binman, if this contravenes the idea of
> > describing images in binman, and if no exceptions can be made on that
> > regard. But please do understand that no attempt is being made at
> > 'skirting around binman'. In fact, I had worked on your earlier
> > objection of using absolute paths in testing this functionality in
> > binman. But then you had put the other objection of how this does not
> > follow the idea or concept of image description in binman. Hence this
> > approach.
>
> It would be easier to talk this through f2f, but I will make an attempt here.
>
> The goal here is for people to be able to create an EFI capsule (or
> more than one?) automatically as part of the build.

That is one of the goals, yes. But the other goal is also to add
functionality to the base capsule generation tool which extends the
tool to support generating capsules through the config file. I would
be more than happy if this can be plumbed in as part of the build.


>  Since the config
> is in the binman description, putting it somewhere else doesn't help
> anyone. It will lead to .cfg files in U-Boot and Binman will just be a
> side show. That is my concern.

Okay. But I don't think that is the main issue here. For example, we
can require the .cfg files to be placed in the board dir, similar to
how the capsule keys are placed. The issue here is that can binman
make allowances(IMO it should) to have the capsule payloads be
specified through a .cfg file parameter, instead of the usual way of
specifying it as a DT node. I think if this allowance can be made, it
solves all the issues here.

>
> I would really like to see a capsule generated for sandbox so I can
> understand this better.

I will work on the capsule generation for sandbox. I will generate
signed capsules, as you suggested in this mail thread as part of the
sandbox U-Boot build. However, this will be using the currently merged
capsule generation logic, where all the capsule parameters, it's
payload included, are specified in binman nodes. I suspect this won't
give you a picture of capsule generation through config files.

>
> Anyway, I hope that makes sense? If not, I think we should discuss it
> f2f or VC or somehow.

Sure. I will be happy to have a VC anytime if you think that would
help. Let me know and we can sync up over IRC. Thanks.

-sughosh

>
> Regards,
> Simon
>
> >
> > -sughosh
> >
> > [1] - https://lore.kernel.org/u-boot/20230908120002.29851-1-sughosh.ganu@linaro.org/
> > [2] - https://lore.kernel.org/u-boot/20230908120002.29851-4-sughosh.ganu@linaro.org/
> > [3] - https://lore.kernel.org/u-boot/CAPnjgZ3Zi3hmQg7d5+0461hRK+5O_V89PY=QxMwQarhhqUW6VQ@mail.gmail.com/
> > [4] - https://lore.kernel.org/u-boot/CADg8p94EvJc6HNKvk2_XWaDb97u28vGfZasnv2je4yJw1k860Q@mail.gmail.com/
> >
> > >
> > > >
> > > > -sughosh
> > > >
> > > > >
> > > > > Thanks
> > > > > /Ilias
> > > > > >
> > > > > > >
> > > > > > > Also, could we get sandbox to produce one EFI capsule as part of the
> > > > > > > normal build? I think that discussion trailed off.
> > > > > >
> > > > > > Yes, apologies for missing out on this. Slipped my mind. Would you
> > > > > > want, say, all the non-signed capsules to be generated as part of the
> > > > > > sandbox build?
> > > > > >
> > > > > > -sughosh
> > >
> > > Regards,
> > > Simon

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

[Simon Glass]

Hi Sughosh,

On Mon, Dec 4, 2023 at 7:15 AM Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> hi Simon,
>
> On Sat, 2 Dec 2023 at 00:02, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Sughosh,
> >
> > On Thu, 30 Nov 2023 at 23:39, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > >
> > > hi Simon,
> > >
> > > On Thu, 30 Nov 2023 at 08:16, Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Sughosh,
> > > >
> > > > On Wed, 22 Nov 2023 at 00:40, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > > > >
> > > > > hi Ilias,
> > > > >
> > > > > On Wed, 22 Nov 2023 at 13:06, Ilias Apalodimas
> > > > > <ilias.apalodimas@linaro.org> wrote:
> > > > > >
> > > > > > Hi all,
> > > > > >
> > > > > > On Wed, 22 Nov 2023 at 07:23, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > > > > > >
> > > > > > > hi Simon,
> > > > > > >
> > > > > > > On Wed, 22 Nov 2023 at 03:42, Simon Glass <sjg@chromium.org> wrote:
> > > > > > > >
> > > > > > > > Hi Sughosh,
> > > > > > > >
> > > > > > > > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu <sughosh.ganu@linaro.org> 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.
> > > > > > > > >
> > > > > > > > > The config file can then be passed to the mkeficapsule tool in such
> > > > > > > > > manner
> > > > > > > > >
> > > > > > > > >  $ ./tools/mkeficapsule -f <path/to/the/config/file>
> > > > > > > > >
> > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > > > > > > > ---
> > > > > > > > >  tools/Kconfig              |  15 ++
> > > > > > > > >  tools/Makefile             |   1 +
> > > > > > > > >  tools/eficapsule.h         | 114 ++++++++++++
> > > > > > > > >  tools/mkeficapsule.c       |  87 +++++----
> > > > > > > > >  tools/mkeficapsule_parse.c | 352 +++++++++++++++++++++++++++++++++++++
> > > > > > > > >  5 files changed, 538 insertions(+), 31 deletions(-)
> > > > > > > > >  create mode 100644 tools/mkeficapsule_parse.c
> > > > > > > >
> > > > > > > > This patch keeps coming back :-)
> > > > > > > >
> > > > > > > > Can we not add multiple capsules in the binman description? Why do we
> > > > > > > > need a new file format? How can binman decode images produced in this
> > > > > > > > way?
> > > > > > >
> > > > > > > So as Tom mentions, this brings parity with respect to the other
> > > > > > > capsule generation tool in EDKII that generates capsules. IIRC, this
> > > > > > > is something which even Xilix was interested in, and Michal had kind
> > > > > > > of gone through these patches earlier. Lastly, it would be good to
> > > > > > > have support in U-Boot's mkeficapsule tool for generating a single
> > > > > > > capsule file with multiple payloads, and having support for this
> > > > > > > functionality helps in that goal.
> > > > > > >
> > > > > > > Also, you might have noticed that, since your objection to the last
> > > > > > > series, I have removed putting this in binman. So now, this aspect of
> > > > > > > the capsule generation would only be supported through the
> > > > > > > command-line invocation of the tool.
> > > > > >
> > > > > > I think that overall the approach is sane. mkeficapsule is currently
> > > > > > supported and compiled for distros, so the multiple payload support is
> > > > > > useful. If we want to add support to binman, instead of rewriting this
> > > > > > in python, we could just call that tool for parsing and creating
> > > > > > capsules
> > > > >
> > > > > Given the amount of time these patches have been under review(also
> > > > > number of iterations), I would request that this series be reviewed
> > > > > and merged first. I think there is general consensus that there is
> > > > > value to have this functionality in the mkeficapsule tool. If it is
> > > > > deemed fit to support this through binman as well, that task can be
> > > > > taken up separately. Thanks.
> > > >
> > > > The point you are missing is that it is the entire goal of 'skirting
> > > > around' binman which is suspect.
> > > >
> > > > If there is a need to generate an output file from the build, we
> > > > should support it in binman. If people start creating configuration
> > > > files all over the place, then they are not using binman, right?
> > > >
> > > > I understand that there are pre-existing vendor-specific config files,
> > > > etc. that the EFI thing is a grey area, but I cannot imagine that this
> > > > patch would lead to a good outcome.
> > > >
> > > > The goal of binman is to bring order to the chaos of firmware
> > > > packaging...we cannot do that if it is not actually used.
> > > >
> > > > So let's figure out what is missing from binman's capsule generation
> > > > (multiple capsules? accept/reject capsules?) and how best to add it.
> > >
> > > I think I need to jog your memory back a bit. For context, I have
> > > jotted down the points.
> > >
> > > * The mkeficapsule tool generates capsules in U-Boot.
> > > * Currently, when the tool is invoked from the command-line, the
> > > capsules are generated by passing the capsule parameters as cmd-line
> > > options.
> > > * I had earlier added support for generating the capsules as part of
> > > U-Boot build, through binman. This support has been merged.
> > > * I had followed these patches up with another series [1] which
> > > generates capsules by parsing the capsule parameters through a config
> > > file instead of cmd-line options.
> > > * This series also had patches which were attempting to integrate this
> > > functionality into binman [2].
> > > * As part of reviewing the patch series, you had objected to adding
> > > this support in binman, primarily because this way of specifying the
> > > capsule parameters goes against the normal way of image description in
> > > binman [3].
> > > * I have described in this mail thread about why we need to have
> > > support for generating capsules through config files [4].
> >
> > Thanks for that. Yes this fits with my memory, such as it is.
> >
> > I regard the tool as a binman tool, though, in the sense that binman
> > is responsible for generating the images. For example, binman calls
> > mkimage to handle FIT images. People are free to write a script to
> > call mkimage manually, but the in-tree use of mkimage is supposed to
> > be with binman.
>
> No issues with supporting the functionality through binman. However, I
> think that if binman is to be used as the encompassing packaging tool
> which in turn calls other tools like mkimage or mkeficapsule, it
> should make allowances for accepting formats, or ways of describing
> payloads that might not sit with it's core design of "each image is a
> DT node".
>
> >
> > >
> > > So, in essence, this functionality is needed to be added to the tool.
> > > I have earlier tried integrating this in binman, but that was
> > > rejected. So, the way I see this moving ahead is to first add support
> > > for this feature in the tool, and then see if this can also be added
> > > in binman. As I mentioned earlier, I am fine with this functionality
> > > not getting integrated with binman, if this contravenes the idea of
> > > describing images in binman, and if no exceptions can be made on that
> > > regard. But please do understand that no attempt is being made at
> > > 'skirting around binman'. In fact, I had worked on your earlier
> > > objection of using absolute paths in testing this functionality in
> > > binman. But then you had put the other objection of how this does not
> > > follow the idea or concept of image description in binman. Hence this
> > > approach.
> >
> > It would be easier to talk this through f2f, but I will make an attempt here.
> >
> > The goal here is for people to be able to create an EFI capsule (or
> > more than one?) automatically as part of the build.
>
> That is one of the goals, yes. But the other goal is also to add
> functionality to the base capsule generation tool which extends the
> tool to support generating capsules through the config file. I would
> be more than happy if this can be plumbed in as part of the build.
>
>
> >  Since the config
> > is in the binman description, putting it somewhere else doesn't help
> > anyone. It will lead to .cfg files in U-Boot and Binman will just be a
> > side show. That is my concern.
>
> Okay. But I don't think that is the main issue here. For example, we
> can require the .cfg files to be placed in the board dir, similar to
> how the capsule keys are placed. The issue here is that can binman
> make allowances(IMO it should) to have the capsule payloads be
> specified through a .cfg file parameter, instead of the usual way of
> specifying it as a DT node. I think if this allowance can be made, it
> solves all the issues here.
>
> >
> > I would really like to see a capsule generated for sandbox so I can
> > understand this better.
>
> I will work on the capsule generation for sandbox. I will generate
> signed capsules, as you suggested in this mail thread as part of the
> sandbox U-Boot build. However, this will be using the currently merged
> capsule generation logic, where all the capsule parameters, it's
> payload included, are specified in binman nodes. I suspect this won't
> give you a picture of capsule generation through config files.

Is there is progress on this? I would still really like to see a
single capsule generated by the sandbox build.

>
> >
> > Anyway, I hope that makes sense? If not, I think we should discuss it
> > f2f or VC or somehow.
>
> Sure. I will be happy to have a VC anytime if you think that would
> help. Let me know and we can sync up over IRC. Thanks.

Perhaps we can do this in the new year. I think it would help to do a
patch for the above first, though, since it might answer many of my
questions.

Regards,
Simon

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

[Sughosh Ganu]

hi Simon,

On Wed, 27 Dec 2023 at 23:19, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Sughosh,
>
> On Mon, Dec 4, 2023 at 7:15 AM Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> >
> > hi Simon,
> >
> > On Sat, 2 Dec 2023 at 00:02, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Thu, 30 Nov 2023 at 23:39, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > > >
> > > > hi Simon,
> > > >
> > > > On Thu, 30 Nov 2023 at 08:16, Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > Hi Sughosh,
> > > > >
> > > > > On Wed, 22 Nov 2023 at 00:40, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > > > > >
> > > > > > hi Ilias,
> > > > > >
> > > > > > On Wed, 22 Nov 2023 at 13:06, Ilias Apalodimas
> > > > > > <ilias.apalodimas@linaro.org> wrote:
> > > > > > >
> > > > > > > Hi all,
> > > > > > >
> > > > > > > On Wed, 22 Nov 2023 at 07:23, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > > > > > > >
> > > > > > > > hi Simon,
> > > > > > > >
> > > > > > > > On Wed, 22 Nov 2023 at 03:42, Simon Glass <sjg@chromium.org> wrote:
> > > > > > > > >
> > > > > > > > > Hi Sughosh,
> > > > > > > > >
> > > > > > > > > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu <sughosh.ganu@linaro.org> 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.
> > > > > > > > > >
> > > > > > > > > > The config file can then be passed to the mkeficapsule tool in such
> > > > > > > > > > manner
> > > > > > > > > >
> > > > > > > > > >  $ ./tools/mkeficapsule -f <path/to/the/config/file>
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > > > > > > > > ---
> > > > > > > > > >  tools/Kconfig              |  15 ++
> > > > > > > > > >  tools/Makefile             |   1 +
> > > > > > > > > >  tools/eficapsule.h         | 114 ++++++++++++
> > > > > > > > > >  tools/mkeficapsule.c       |  87 +++++----
> > > > > > > > > >  tools/mkeficapsule_parse.c | 352 +++++++++++++++++++++++++++++++++++++
> > > > > > > > > >  5 files changed, 538 insertions(+), 31 deletions(-)
> > > > > > > > > >  create mode 100644 tools/mkeficapsule_parse.c
> > > > > > > > >
> > > > > > > > > This patch keeps coming back :-)
> > > > > > > > >
> > > > > > > > > Can we not add multiple capsules in the binman description? Why do we
> > > > > > > > > need a new file format? How can binman decode images produced in this
> > > > > > > > > way?
> > > > > > > >
> > > > > > > > So as Tom mentions, this brings parity with respect to the other
> > > > > > > > capsule generation tool in EDKII that generates capsules. IIRC, this
> > > > > > > > is something which even Xilix was interested in, and Michal had kind
> > > > > > > > of gone through these patches earlier. Lastly, it would be good to
> > > > > > > > have support in U-Boot's mkeficapsule tool for generating a single
> > > > > > > > capsule file with multiple payloads, and having support for this
> > > > > > > > functionality helps in that goal.
> > > > > > > >
> > > > > > > > Also, you might have noticed that, since your objection to the last
> > > > > > > > series, I have removed putting this in binman. So now, this aspect of
> > > > > > > > the capsule generation would only be supported through the
> > > > > > > > command-line invocation of the tool.
> > > > > > >
> > > > > > > I think that overall the approach is sane. mkeficapsule is currently
> > > > > > > supported and compiled for distros, so the multiple payload support is
> > > > > > > useful. If we want to add support to binman, instead of rewriting this
> > > > > > > in python, we could just call that tool for parsing and creating
> > > > > > > capsules
> > > > > >
> > > > > > Given the amount of time these patches have been under review(also
> > > > > > number of iterations), I would request that this series be reviewed
> > > > > > and merged first. I think there is general consensus that there is
> > > > > > value to have this functionality in the mkeficapsule tool. If it is
> > > > > > deemed fit to support this through binman as well, that task can be
> > > > > > taken up separately. Thanks.
> > > > >
> > > > > The point you are missing is that it is the entire goal of 'skirting
> > > > > around' binman which is suspect.
> > > > >
> > > > > If there is a need to generate an output file from the build, we
> > > > > should support it in binman. If people start creating configuration
> > > > > files all over the place, then they are not using binman, right?
> > > > >
> > > > > I understand that there are pre-existing vendor-specific config files,
> > > > > etc. that the EFI thing is a grey area, but I cannot imagine that this
> > > > > patch would lead to a good outcome.
> > > > >
> > > > > The goal of binman is to bring order to the chaos of firmware
> > > > > packaging...we cannot do that if it is not actually used.
> > > > >
> > > > > So let's figure out what is missing from binman's capsule generation
> > > > > (multiple capsules? accept/reject capsules?) and how best to add it.
> > > >
> > > > I think I need to jog your memory back a bit. For context, I have
> > > > jotted down the points.
> > > >
> > > > * The mkeficapsule tool generates capsules in U-Boot.
> > > > * Currently, when the tool is invoked from the command-line, the
> > > > capsules are generated by passing the capsule parameters as cmd-line
> > > > options.
> > > > * I had earlier added support for generating the capsules as part of
> > > > U-Boot build, through binman. This support has been merged.
> > > > * I had followed these patches up with another series [1] which
> > > > generates capsules by parsing the capsule parameters through a config
> > > > file instead of cmd-line options.
> > > > * This series also had patches which were attempting to integrate this
> > > > functionality into binman [2].
> > > > * As part of reviewing the patch series, you had objected to adding
> > > > this support in binman, primarily because this way of specifying the
> > > > capsule parameters goes against the normal way of image description in
> > > > binman [3].
> > > > * I have described in this mail thread about why we need to have
> > > > support for generating capsules through config files [4].
> > >
> > > Thanks for that. Yes this fits with my memory, such as it is.
> > >
> > > I regard the tool as a binman tool, though, in the sense that binman
> > > is responsible for generating the images. For example, binman calls
> > > mkimage to handle FIT images. People are free to write a script to
> > > call mkimage manually, but the in-tree use of mkimage is supposed to
> > > be with binman.
> >
> > No issues with supporting the functionality through binman. However, I
> > think that if binman is to be used as the encompassing packaging tool
> > which in turn calls other tools like mkimage or mkeficapsule, it
> > should make allowances for accepting formats, or ways of describing
> > payloads that might not sit with it's core design of "each image is a
> > DT node".
> >
> > >
> > > >
> > > > So, in essence, this functionality is needed to be added to the tool.
> > > > I have earlier tried integrating this in binman, but that was
> > > > rejected. So, the way I see this moving ahead is to first add support
> > > > for this feature in the tool, and then see if this can also be added
> > > > in binman. As I mentioned earlier, I am fine with this functionality
> > > > not getting integrated with binman, if this contravenes the idea of
> > > > describing images in binman, and if no exceptions can be made on that
> > > > regard. But please do understand that no attempt is being made at
> > > > 'skirting around binman'. In fact, I had worked on your earlier
> > > > objection of using absolute paths in testing this functionality in
> > > > binman. But then you had put the other objection of how this does not
> > > > follow the idea or concept of image description in binman. Hence this
> > > > approach.
> > >
> > > It would be easier to talk this through f2f, but I will make an attempt here.
> > >
> > > The goal here is for people to be able to create an EFI capsule (or
> > > more than one?) automatically as part of the build.
> >
> > That is one of the goals, yes. But the other goal is also to add
> > functionality to the base capsule generation tool which extends the
> > tool to support generating capsules through the config file. I would
> > be more than happy if this can be plumbed in as part of the build.
> >
> >
> > >  Since the config
> > > is in the binman description, putting it somewhere else doesn't help
> > > anyone. It will lead to .cfg files in U-Boot and Binman will just be a
> > > side show. That is my concern.
> >
> > Okay. But I don't think that is the main issue here. For example, we
> > can require the .cfg files to be placed in the board dir, similar to
> > how the capsule keys are placed. The issue here is that can binman
> > make allowances(IMO it should) to have the capsule payloads be
> > specified through a .cfg file parameter, instead of the usual way of
> > specifying it as a DT node. I think if this allowance can be made, it
> > solves all the issues here.
> >
> > >
> > > I would really like to see a capsule generated for sandbox so I can
> > > understand this better.
> >
> > I will work on the capsule generation for sandbox. I will generate
> > signed capsules, as you suggested in this mail thread as part of the
> > sandbox U-Boot build. However, this will be using the currently merged
> > capsule generation logic, where all the capsule parameters, it's
> > payload included, are specified in binman nodes. I suspect this won't
> > give you a picture of capsule generation through config files.
>
> Is there is progress on this? I would still really like to see a
> single capsule generated by the sandbox build.

I will come up with a patch to move the generation of signed capsules
for sandbox as part of the build. We can take the discussion forward
once this patch gets in. But I would really love to see progress on
this series, without or without binman support. Thanks.

-sughosh

>
> >
> > >
> > > Anyway, I hope that makes sense? If not, I think we should discuss it
> > > f2f or VC or somehow.
> >
> > Sure. I will be happy to have a VC anytime if you think that would
> > help. Let me know and we can sync up over IRC. Thanks.
>
> Perhaps we can do this in the new year. I think it would help to do a
> patch for the above first, though, since it might answer many of my
> questions.
>
> Regards,
> Simon

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

[Simon Glass]

Hi Sughosh,

On Fri, Dec 29, 2023 at 6:53 AM Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> hi Simon,
>
> On Wed, 27 Dec 2023 at 23:19, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Sughosh,
> >
> > On Mon, Dec 4, 2023 at 7:15 AM Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > >
> > > hi Simon,
> > >
> > > On Sat, 2 Dec 2023 at 00:02, Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Sughosh,
> > > >
> > > > On Thu, 30 Nov 2023 at 23:39, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > > > >
> > > > > hi Simon,
> > > > >
> > > > > On Thu, 30 Nov 2023 at 08:16, Simon Glass <sjg@chromium.org> wrote:
> > > > > >
> > > > > > Hi Sughosh,
> > > > > >
> > > > > > On Wed, 22 Nov 2023 at 00:40, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > > > > > >
> > > > > > > hi Ilias,
> > > > > > >
> > > > > > > On Wed, 22 Nov 2023 at 13:06, Ilias Apalodimas
> > > > > > > <ilias.apalodimas@linaro.org> wrote:
> > > > > > > >
> > > > > > > > Hi all,
> > > > > > > >
> > > > > > > > On Wed, 22 Nov 2023 at 07:23, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > > > > > > > >
> > > > > > > > > hi Simon,
> > > > > > > > >
> > > > > > > > > On Wed, 22 Nov 2023 at 03:42, Simon Glass <sjg@chromium.org> wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Sughosh,
> > > > > > > > > >
> > > > > > > > > > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu <sughosh.ganu@linaro.org> 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.
> > > > > > > > > > >
> > > > > > > > > > > The config file can then be passed to the mkeficapsule tool in such
> > > > > > > > > > > manner
> > > > > > > > > > >
> > > > > > > > > > >  $ ./tools/mkeficapsule -f <path/to/the/config/file>
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > > > > > > > > > ---
> > > > > > > > > > >  tools/Kconfig              |  15 ++
> > > > > > > > > > >  tools/Makefile             |   1 +
> > > > > > > > > > >  tools/eficapsule.h         | 114 ++++++++++++
> > > > > > > > > > >  tools/mkeficapsule.c       |  87 +++++----
> > > > > > > > > > >  tools/mkeficapsule_parse.c | 352 +++++++++++++++++++++++++++++++++++++
> > > > > > > > > > >  5 files changed, 538 insertions(+), 31 deletions(-)
> > > > > > > > > > >  create mode 100644 tools/mkeficapsule_parse.c
> > > > > > > > > >
> > > > > > > > > > This patch keeps coming back :-)
> > > > > > > > > >
> > > > > > > > > > Can we not add multiple capsules in the binman description? Why do we
> > > > > > > > > > need a new file format? How can binman decode images produced in this
> > > > > > > > > > way?
> > > > > > > > >
> > > > > > > > > So as Tom mentions, this brings parity with respect to the other
> > > > > > > > > capsule generation tool in EDKII that generates capsules. IIRC, this
> > > > > > > > > is something which even Xilix was interested in, and Michal had kind
> > > > > > > > > of gone through these patches earlier. Lastly, it would be good to
> > > > > > > > > have support in U-Boot's mkeficapsule tool for generating a single
> > > > > > > > > capsule file with multiple payloads, and having support for this
> > > > > > > > > functionality helps in that goal.
> > > > > > > > >
> > > > > > > > > Also, you might have noticed that, since your objection to the last
> > > > > > > > > series, I have removed putting this in binman. So now, this aspect of
> > > > > > > > > the capsule generation would only be supported through the
> > > > > > > > > command-line invocation of the tool.
> > > > > > > >
> > > > > > > > I think that overall the approach is sane. mkeficapsule is currently
> > > > > > > > supported and compiled for distros, so the multiple payload support is
> > > > > > > > useful. If we want to add support to binman, instead of rewriting this
> > > > > > > > in python, we could just call that tool for parsing and creating
> > > > > > > > capsules
> > > > > > >
> > > > > > > Given the amount of time these patches have been under review(also
> > > > > > > number of iterations), I would request that this series be reviewed
> > > > > > > and merged first. I think there is general consensus that there is
> > > > > > > value to have this functionality in the mkeficapsule tool. If it is
> > > > > > > deemed fit to support this through binman as well, that task can be
> > > > > > > taken up separately. Thanks.
> > > > > >
> > > > > > The point you are missing is that it is the entire goal of 'skirting
> > > > > > around' binman which is suspect.
> > > > > >
> > > > > > If there is a need to generate an output file from the build, we
> > > > > > should support it in binman. If people start creating configuration
> > > > > > files all over the place, then they are not using binman, right?
> > > > > >
> > > > > > I understand that there are pre-existing vendor-specific config files,
> > > > > > etc. that the EFI thing is a grey area, but I cannot imagine that this
> > > > > > patch would lead to a good outcome.
> > > > > >
> > > > > > The goal of binman is to bring order to the chaos of firmware
> > > > > > packaging...we cannot do that if it is not actually used.
> > > > > >
> > > > > > So let's figure out what is missing from binman's capsule generation
> > > > > > (multiple capsules? accept/reject capsules?) and how best to add it.
> > > > >
> > > > > I think I need to jog your memory back a bit. For context, I have
> > > > > jotted down the points.
> > > > >
> > > > > * The mkeficapsule tool generates capsules in U-Boot.
> > > > > * Currently, when the tool is invoked from the command-line, the
> > > > > capsules are generated by passing the capsule parameters as cmd-line
> > > > > options.
> > > > > * I had earlier added support for generating the capsules as part of
> > > > > U-Boot build, through binman. This support has been merged.
> > > > > * I had followed these patches up with another series [1] which
> > > > > generates capsules by parsing the capsule parameters through a config
> > > > > file instead of cmd-line options.
> > > > > * This series also had patches which were attempting to integrate this
> > > > > functionality into binman [2].
> > > > > * As part of reviewing the patch series, you had objected to adding
> > > > > this support in binman, primarily because this way of specifying the
> > > > > capsule parameters goes against the normal way of image description in
> > > > > binman [3].
> > > > > * I have described in this mail thread about why we need to have
> > > > > support for generating capsules through config files [4].
> > > >
> > > > Thanks for that. Yes this fits with my memory, such as it is.
> > > >
> > > > I regard the tool as a binman tool, though, in the sense that binman
> > > > is responsible for generating the images. For example, binman calls
> > > > mkimage to handle FIT images. People are free to write a script to
> > > > call mkimage manually, but the in-tree use of mkimage is supposed to
> > > > be with binman.
> > >
> > > No issues with supporting the functionality through binman. However, I
> > > think that if binman is to be used as the encompassing packaging tool
> > > which in turn calls other tools like mkimage or mkeficapsule, it
> > > should make allowances for accepting formats, or ways of describing
> > > payloads that might not sit with it's core design of "each image is a
> > > DT node".
> > >
> > > >
> > > > >
> > > > > So, in essence, this functionality is needed to be added to the tool.
> > > > > I have earlier tried integrating this in binman, but that was
> > > > > rejected. So, the way I see this moving ahead is to first add support
> > > > > for this feature in the tool, and then see if this can also be added
> > > > > in binman. As I mentioned earlier, I am fine with this functionality
> > > > > not getting integrated with binman, if this contravenes the idea of
> > > > > describing images in binman, and if no exceptions can be made on that
> > > > > regard. But please do understand that no attempt is being made at
> > > > > 'skirting around binman'. In fact, I had worked on your earlier
> > > > > objection of using absolute paths in testing this functionality in
> > > > > binman. But then you had put the other objection of how this does not
> > > > > follow the idea or concept of image description in binman. Hence this
> > > > > approach.
> > > >
> > > > It would be easier to talk this through f2f, but I will make an attempt here.
> > > >
> > > > The goal here is for people to be able to create an EFI capsule (or
> > > > more than one?) automatically as part of the build.
> > >
> > > That is one of the goals, yes. But the other goal is also to add
> > > functionality to the base capsule generation tool which extends the
> > > tool to support generating capsules through the config file. I would
> > > be more than happy if this can be plumbed in as part of the build.
> > >
> > >
> > > >  Since the config
> > > > is in the binman description, putting it somewhere else doesn't help
> > > > anyone. It will lead to .cfg files in U-Boot and Binman will just be a
> > > > side show. That is my concern.
> > >
> > > Okay. But I don't think that is the main issue here. For example, we
> > > can require the .cfg files to be placed in the board dir, similar to
> > > how the capsule keys are placed. The issue here is that can binman
> > > make allowances(IMO it should) to have the capsule payloads be
> > > specified through a .cfg file parameter, instead of the usual way of
> > > specifying it as a DT node. I think if this allowance can be made, it
> > > solves all the issues here.
> > >
> > > >
> > > > I would really like to see a capsule generated for sandbox so I can
> > > > understand this better.
> > >
> > > I will work on the capsule generation for sandbox. I will generate
> > > signed capsules, as you suggested in this mail thread as part of the
> > > sandbox U-Boot build. However, this will be using the currently merged
> > > capsule generation logic, where all the capsule parameters, it's
> > > payload included, are specified in binman nodes. I suspect this won't
> > > give you a picture of capsule generation through config files.
> >
> > Is there is progress on this? I would still really like to see a
> > single capsule generated by the sandbox build.
>
> I will come up with a patch to move the generation of signed capsules
> for sandbox as part of the build. We can take the discussion forward
> once this patch gets in. But I would really love to see progress on
> this series, without or without binman support. Thanks.

Just to be clear, this is independent of the tests you have written,
which are fine as is. I am just looking for a single capsule to be
built.

Re this series, I am still unsure whether this will result in .cfg
files appearing in U-Boot, or what this will actually be used for. I
am hoping that with a sandbox build to play around with, it will
become clearer. That said, if people want to add new features to
U-Boot, we normally take them.

Regards,
Simon


>
> -sughosh
>
> >
> > >
> > > >
> > > > Anyway, I hope that makes sense? If not, I think we should discuss it
> > > > f2f or VC or somehow.
> > >
> > > Sure. I will be happy to have a VC anytime if you think that would
> > > help. Let me know and we can sync up over IRC. Thanks.
> >
> > Perhaps we can do this in the new year. I think it would help to do a
> > patch for the above first, though, since it might answer many of my
> > questions.
> >
> > Regards,
> > Simon

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

[Simon Glass]

Hi Sughosh,

On Tue, 21 Nov 2023 at 22:23, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> hi Simon,
>
> On Wed, 22 Nov 2023 at 03:42, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Sughosh,
> >
> > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu <sughosh.ganu@linaro.org> 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.
> > >
> > > The config file can then be passed to the mkeficapsule tool in such
> > > manner
> > >
> > >  $ ./tools/mkeficapsule -f <path/to/the/config/file>
> > >
> > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > ---
> > >  tools/Kconfig              |  15 ++
> > >  tools/Makefile             |   1 +
> > >  tools/eficapsule.h         | 114 ++++++++++++
> > >  tools/mkeficapsule.c       |  87 +++++----
> > >  tools/mkeficapsule_parse.c | 352 +++++++++++++++++++++++++++++++++++++
> > >  5 files changed, 538 insertions(+), 31 deletions(-)
> > >  create mode 100644 tools/mkeficapsule_parse.c
> >
> > This patch keeps coming back :-)
> >
> > Can we not add multiple capsules in the binman description? Why do we
> > need a new file format? How can binman decode images produced in this
> > way?
>
> So as Tom mentions, this brings parity with respect to the other
> capsule generation tool in EDKII that generates capsules. IIRC, this
> is something which even Xilix was interested in, and Michal had kind
> of gone through these patches earlier. Lastly, it would be good to
> have support in U-Boot's mkeficapsule tool for generating a single
> capsule file with multiple payloads, and having support for this
> functionality helps in that goal.
>
> Also, you might have noticed that, since your objection to the last
> series, I have removed putting this in binman. So now, this aspect of
> the capsule generation would only be supported through the
> command-line invocation of the tool.

That sounds like the opposite of what I was asking for...

>
> >
> > Also, could we get sandbox to produce one EFI capsule as part of the
> > normal build? I think that discussion trailed off.
>
> Yes, apologies for missing out on this. Slipped my mind. Would you
> want, say, all the non-signed capsules to be generated as part of the
> sandbox build?

Just one would be good. Probably using a signed one makes more sense
since it is more realistic?

You don't need to change any of the existing tests...I just mean to
add a single capsule generated as part of the sandbox binman
invocation.

The problem is that, at the moment, I cannot make much sense of the
build/binman integration since nothing actually uses it. Once sandbox
uses it, I will be able to understand it better and suggest ways to
expand the binman support.

Regards,
Simon

[PATCH 2/4] doc: Document capsule generation through a config file

[Sughosh Ganu]

The UEFI capsule can now be generate by specifying the capsule
parameters through a config file. Additionally, the capsules can be
generated as part of u-boot build, through binman. Highlight these
changes in the documentation.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
 doc/develop/uefi/uefi.rst | 70 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
index fb16ac743a..b9a9ed5491 100644
--- a/doc/develop/uefi/uefi.rst
+++ b/doc/develop/uefi/uefi.rst
@@ -318,6 +318,76 @@ Run the following command
       --guid <image GUID> \
       <capsule_file_name>
 
+Alternatively, the capsules can be generated through a config
+file. When generating the capsules through a config file, the Kconfig
+symbol CONFIG_EFI_CAPSULE_CFG_FILE is to be used for specifying the
+path to the config file.
+
+The config file describes the parameters that are used for generating
+one or more capsules. The parameters for a given capsule file are
+specified within curly braces, in the form of "key:value" pairs. All
+the parameters that are currently supported by the mkeficapsule tool
+can be specified through the config file.
+
+The following are some example payload parameters specified through
+the config file.
+
+.. code-block:: none
+
+	{
+	    image-guid: 02f4d760-cfd5-43bd-8e2d-a42acb33c660
+	    hardware-instance: 0
+	    monotonic-count: 1
+	    payload: u-boot.bin
+	    image-index: 1
+	    fw-version: 2
+	    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
+	    fw-version: 7
+	    oemflags: 0x8000
+	    capsule: fit.capsule
+	}
+	{
+	    capsule-type: accept
+	    image-guid: 4ce292da-1dd8-428d-a1c2-77743ef8b96e
+	    capsule: accept.capsule
+	}
+	{
+	    capsule-type: revert
+	    capsule: revert.capsule
+	}
+
+The following are the keys that specify the capsule parameters
+
+..code-block:: none
+
+    image-guid: Image GUID
+    image-index: Image index value
+    fw-version: Image version
+    private-key: Path to the private key file used for capsule signing
+    pub-key-cert: Path to the public key crt file used for capsule signing
+    payload: Path to the capsule payload file
+    capsule: Path to the output capsule file that is generated
+    hardware-instance: Hardware Instance value
+    monotonic-count: Monotonic count value
+    capsule-type: Specifies capsule type. normal(default), accept or revert
+    oemflags: 16bit Oemflags value to be used(populated in capsule header)
+
+When generating capsules through a config file, the command would look
+like
+
+.. code-block:: console
+
+    $ mkeficapsule --cfg-file </path/to/the/config/file>
+
+
 Capsule with firmware version
 *****************************
 
-- 
2.34.1

Re: [PATCH 2/4] doc: Document capsule generation through a config file

[Ilias Apalodimas]

Hi Sughosh,

On Tue, Nov 21, 2023 at 12:30:26PM +0530, Sughosh Ganu wrote:
> The UEFI capsule can now be generate by specifying the capsule
> parameters through a config file. Additionally, the capsules can be
> generated as part of u-boot build, through binman. Highlight these
> changes in the documentation.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
>  doc/develop/uefi/uefi.rst | 70 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
>
> diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
> index fb16ac743a..b9a9ed5491 100644
> --- a/doc/develop/uefi/uefi.rst
> +++ b/doc/develop/uefi/uefi.rst
> @@ -318,6 +318,76 @@ Run the following command
>        --guid <image GUID> \
>        <capsule_file_name>
>
> +Alternatively, the capsules can be generated through a config
> +file. When generating the capsules through a config file, the Kconfig
> +symbol CONFIG_EFI_CAPSULE_CFG_FILE is to be used for specifying the
> +path to the config file.

This sounds a bit weird. The tool is compiled and used by distros. How are
they supposed to use the config file?

Briefly looking at the patches, we use those config options on testing.
Can't we add a flag in the tool itself that can be used for normal use
cases and limit those Kconfig flags in our testing only?  Or alternatively
the paths for the config files can be hardcoded for sandbox and we get rid
of the config options entirely?

Thanks
/Ilias
> +
> +The config file describes the parameters that are used for generating
> +one or more capsules. The parameters for a given capsule file are
> +specified within curly braces, in the form of "key:value" pairs. All
> +the parameters that are currently supported by the mkeficapsule tool
> +can be specified through the config file.
> +
> +The following are some example payload parameters specified through
> +the config file.
> +
> +.. code-block:: none
> +
> +	{
> +	    image-guid: 02f4d760-cfd5-43bd-8e2d-a42acb33c660
> +	    hardware-instance: 0
> +	    monotonic-count: 1
> +	    payload: u-boot.bin
> +	    image-index: 1
> +	    fw-version: 2
> +	    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
> +	    fw-version: 7
> +	    oemflags: 0x8000
> +	    capsule: fit.capsule
> +	}
> +	{
> +	    capsule-type: accept
> +	    image-guid: 4ce292da-1dd8-428d-a1c2-77743ef8b96e
> +	    capsule: accept.capsule
> +	}
> +	{
> +	    capsule-type: revert
> +	    capsule: revert.capsule
> +	}
> +
> +The following are the keys that specify the capsule parameters
> +
> +..code-block:: none
> +
> +    image-guid: Image GUID
> +    image-index: Image index value
> +    fw-version: Image version
> +    private-key: Path to the private key file used for capsule signing
> +    pub-key-cert: Path to the public key crt file used for capsule signing
> +    payload: Path to the capsule payload file
> +    capsule: Path to the output capsule file that is generated
> +    hardware-instance: Hardware Instance value
> +    monotonic-count: Monotonic count value
> +    capsule-type: Specifies capsule type. normal(default), accept or revert
> +    oemflags: 16bit Oemflags value to be used(populated in capsule header)
> +
> +When generating capsules through a config file, the command would look
> +like
> +
> +.. code-block:: console
> +
> +    $ mkeficapsule --cfg-file </path/to/the/config/file>
> +
> +
>  Capsule with firmware version
>  *****************************
>
> --
> 2.34.1
>

Re: [PATCH 2/4] doc: Document capsule generation through a config file

[Sughosh Ganu]

hi Ilias,

On Tue, 23 Jan 2024 at 21:53, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Sughosh,
>
> On Tue, Nov 21, 2023 at 12:30:26PM +0530, Sughosh Ganu wrote:
> > The UEFI capsule can now be generate by specifying the capsule
> > parameters through a config file. Additionally, the capsules can be
> > generated as part of u-boot build, through binman. Highlight these
> > changes in the documentation.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > ---
> >  doc/develop/uefi/uefi.rst | 70 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 70 insertions(+)
> >
> > diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
> > index fb16ac743a..b9a9ed5491 100644
> > --- a/doc/develop/uefi/uefi.rst
> > +++ b/doc/develop/uefi/uefi.rst
> > @@ -318,6 +318,76 @@ Run the following command
> >        --guid <image GUID> \
> >        <capsule_file_name>
> >
> > +Alternatively, the capsules can be generated through a config
> > +file. When generating the capsules through a config file, the Kconfig
> > +symbol CONFIG_EFI_CAPSULE_CFG_FILE is to be used for specifying the
> > +path to the config file.
>
> This sounds a bit weird. The tool is compiled and used by distros. How are
> they supposed to use the config file?
>
> Briefly looking at the patches, we use those config options on testing.
> Can't we add a flag in the tool itself that can be used for normal use
> cases and limit those Kconfig flags in our testing only?  Or alternatively
> the paths for the config files can be hardcoded for sandbox and we get rid
> of the config options entirely?

It is correct that the Kconfig symbol is used primarily for testing.
We can indeed use a fixed path for the sandbox based testing. That is
the approach that was taken for the public keys used for capsule
authentication. Will make changes to the patches accordingly. Thanks.

-sughosh

>
> Thanks
> /Ilias
> > +
> > +The config file describes the parameters that are used for generating
> > +one or more capsules. The parameters for a given capsule file are
> > +specified within curly braces, in the form of "key:value" pairs. All
> > +the parameters that are currently supported by the mkeficapsule tool
> > +can be specified through the config file.
> > +
> > +The following are some example payload parameters specified through
> > +the config file.
> > +
> > +.. code-block:: none
> > +
> > +     {
> > +         image-guid: 02f4d760-cfd5-43bd-8e2d-a42acb33c660
> > +         hardware-instance: 0
> > +         monotonic-count: 1
> > +         payload: u-boot.bin
> > +         image-index: 1
> > +         fw-version: 2
> > +         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
> > +         fw-version: 7
> > +         oemflags: 0x8000
> > +         capsule: fit.capsule
> > +     }
> > +     {
> > +         capsule-type: accept
> > +         image-guid: 4ce292da-1dd8-428d-a1c2-77743ef8b96e
> > +         capsule: accept.capsule
> > +     }
> > +     {
> > +         capsule-type: revert
> > +         capsule: revert.capsule
> > +     }
> > +
> > +The following are the keys that specify the capsule parameters
> > +
> > +..code-block:: none
> > +
> > +    image-guid: Image GUID
> > +    image-index: Image index value
> > +    fw-version: Image version
> > +    private-key: Path to the private key file used for capsule signing
> > +    pub-key-cert: Path to the public key crt file used for capsule signing
> > +    payload: Path to the capsule payload file
> > +    capsule: Path to the output capsule file that is generated
> > +    hardware-instance: Hardware Instance value
> > +    monotonic-count: Monotonic count value
> > +    capsule-type: Specifies capsule type. normal(default), accept or revert
> > +    oemflags: 16bit Oemflags value to be used(populated in capsule header)
> > +
> > +When generating capsules through a config file, the command would look
> > +like
> > +
> > +.. code-block:: console
> > +
> > +    $ mkeficapsule --cfg-file </path/to/the/config/file>
> > +
> > +
> >  Capsule with firmware version
> >  *****************************
> >
> > --
> > 2.34.1
> >

[PATCH 3/4] sandbox: capsule: Add a config file for generating capsules

[Sughosh Ganu]

Support has been added to the mkeficapsule tool to generate capsules
by parsing the capsule parameters through a config file. Add a config
file for generating capsules. These capsules will be used for testing
the capsule update feature on sandbox platform.

Enable generation of capsules through the config file on the sandbox
variant.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
 configs/sandbox_defconfig                     |   2 +
 .../test_efi_capsule/sandbox_capsule_cfg.txt  | 162 ++++++++++++++++++
 2 files changed, 164 insertions(+)
 create mode 100644 test/py/tests/test_efi_capsule/sandbox_capsule_cfg.txt

diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index bc5bcb2a62..f42e01e100 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -344,6 +344,8 @@ CONFIG_EFI_CAPSULE_ON_DISK=y
 CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
 CONFIG_EFI_CAPSULE_AUTHENTICATE=y
 CONFIG_EFI_CAPSULE_ESL_FILE="board/sandbox/capsule_pub_esl_good.esl"
+CONFIG_EFI_CAPSULE_CFG_FILE="test/py/tests/test_efi_capsule/sandbox_capsule_cfg.txt"
+CONFIG_EFI_USE_CAPSULE_CFG_FILE=y
 CONFIG_EFI_SECURE_BOOT=y
 CONFIG_TEST_FDTDEC=y
 CONFIG_UNIT_TEST=y
diff --git a/test/py/tests/test_efi_capsule/sandbox_capsule_cfg.txt b/test/py/tests/test_efi_capsule/sandbox_capsule_cfg.txt
new file mode 100644
index 0000000000..2897c1904e
--- /dev/null
+++ b/test/py/tests/test_efi_capsule/sandbox_capsule_cfg.txt
@@ -0,0 +1,162 @@
+{
+	image-index: 1
+	image-guid: 09D7CF52-0720-4710-91D1-08469B7FE9C8
+	payload: u-boot.bin.new
+	capsule: Test01
+}
+{
+	image-index: 2
+	image-guid: 5A7021F5-FEF2-48B4-AABA-832E777418C0
+	payload: u-boot.env.new
+	capsule: Test02
+}
+{
+	image-index: 1
+	image-guid: 058B7D83-50D5-4C47-A195-60D86AD341C4
+	payload: u-boot.bin.new
+	capsule: Test03
+
+}
+{
+	image-index: 1
+	image-guid: 3673B45D-6A7C-46F3-9E60-ADABB03F7937
+	payload: u-boot_bin_env.itb
+	capsule: Test04
+
+}
+{
+	image-index: 1
+	image-guid: 058B7D83-50D5-4C47-A195-60D86AD341C4
+	payload: u-boot_bin_env.itb
+	capsule: Test05
+
+}
+{
+	image-index: 1
+	image-guid: 058B7D83-50D5-4C47-A195-60D86AD341C4
+	payload: u-boot_bin_env.itb
+	capsule: Test05
+}
+{
+	image-index: 1
+	monotonic-count: 1
+	private-key: SIGNER.key
+	pub-key-cert: SIGNER.crt
+	image-guid: 09D7CF52-0720-4710-91D1-08469B7FE9C8
+	payload: u-boot.bin.new
+	capsule: Test11
+}
+{
+	image-index: 1
+	monotonic-count: 1
+	private-key: SIGNER2.key
+	pub-key-cert: SIGNER2.crt
+	image-guid: 09D7CF52-0720-4710-91D1-08469B7FE9C8
+	payload: u-boot.bin.new
+	capsule: Test12
+}
+{
+	image-index: 1
+	monotonic-count: 1
+	private-key: SIGNER.key
+	pub-key-cert: SIGNER.crt
+	image-guid: 3673B45D-6A7C-46F3-9E60-ADABB03F7937
+	payload: u-boot_bin_env.itb
+	capsule: Test13
+}
+{
+	image-index: 1
+	monotonic-count: 1
+	private-key: SIGNER2.key
+	pub-key-cert: SIGNER2.crt
+	image-guid: 3673B45D-6A7C-46F3-9E60-ADABB03F7937
+	payload: u-boot_bin_env.itb
+	capsule: Test14
+}
+{
+	image-index: 1
+	fw-version: 5
+	image-guid: 09D7CF52-0720-4710-91D1-08469B7FE9C8
+	payload: u-boot.bin.new
+	capsule: Test101
+}
+{
+	image-index: 2
+	fw-version: 10
+	image-guid: 5A7021F5-FEF2-48B4-AABA-832E777418C0
+	payload: u-boot.env.new
+	capsule: Test102
+}
+{
+	image-index: 1
+	fw-version: 2
+	image-guid: 09D7CF52-0720-4710-91D1-08469B7FE9C8
+	payload: u-boot.bin.new
+	capsule: Test103
+
+}
+{
+	image-index: 1
+	fw-version: 5
+	image-guid: 3673B45D-6A7C-46F3-9E60-ADABB03F7937
+	payload: u-boot_bin_env.itb
+	capsule: Test104
+}
+{
+	image-index: 1
+	fw-version: 2
+	image-guid: 3673B45D-6A7C-46F3-9E60-ADABB03F7937
+	payload: u-boot_bin_env.itb
+	capsule: Test105
+
+}
+{
+	image-index: 1
+	monotonic-count: 1
+	fw-version: 5
+	private-key: SIGNER.key
+	pub-key-cert: SIGNER.crt
+	image-guid: 09D7CF52-0720-4710-91D1-08469B7FE9C8
+	payload: u-boot.bin.new
+	capsule: Test111
+}
+{
+	image-index: 2
+	monotonic-count: 1
+	fw-version: 10
+	private-key: SIGNER.key
+	pub-key-cert: SIGNER.crt
+	image-guid: 5A7021F5-FEF2-48B4-AABA-832E777418C0
+	payload: u-boot.env.new
+	capsule: Test112
+}
+{
+	image-index: 1
+	monotonic-count: 1
+	fw-version: 2
+	private-key: SIGNER.key
+	pub-key-cert: SIGNER.crt
+	image-guid: 09D7CF52-0720-4710-91D1-08469B7FE9C8
+	payload: u-boot.bin.new
+	capsule: Test113
+}
+{
+	image-index: 1
+	fw-version: 5
+	monotonic-count: 1
+	private-key: SIGNER.key
+	pub-key-cert: SIGNER.crt
+	image-guid: 3673B45D-6A7C-46F3-9E60-ADABB03F7937
+	payload: u-boot_bin_env.itb
+	capsule: Test114
+}
+{
+	image-index: 1
+	fw-version: 2
+	monotonic-count: 1
+	private-key: SIGNER.key
+	pub-key-cert: SIGNER.crt
+	image-guid: 3673B45D-6A7C-46F3-9E60-ADABB03F7937
+	payload: u-boot_bin_env.itb
+	capsule: Test115
+}
-- 
2.34.1

[PATCH 4/4] test: capsule: Generate capsules through config file

[Sughosh Ganu]

Add logic to generate capsules through a config file while testing the
EFI capsule update functionality. Capsule generation through config
file is enabled on the sandbox variant.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
 test/py/tests/test_efi_capsule/capsule_gen_binman.dts | 2 ++
 test/py/tests/test_efi_capsule/conftest.py            | 7 +++++++
 2 files changed, 9 insertions(+)

diff --git a/test/py/tests/test_efi_capsule/capsule_gen_binman.dts b/test/py/tests/test_efi_capsule/capsule_gen_binman.dts
index e8a1858509..c4e9ebac46 100644
--- a/test/py/tests/test_efi_capsule/capsule_gen_binman.dts
+++ b/test/py/tests/test_efi_capsule/capsule_gen_binman.dts
@@ -54,6 +54,7 @@
 		};
 	};
 
+#if !defined(CONFIG_EFI_USE_CAPSULE_CFG_FILE)
 	capsule1 {
 		filename = "Test01";
 		efi-capsule {
@@ -319,3 +320,4 @@
 		};
 	};
 };
+#endif /* CONFIG_EFI_USE_CAPSULE_CFG_FILE */
diff --git a/test/py/tests/test_efi_capsule/conftest.py b/test/py/tests/test_efi_capsule/conftest.py
index dd41da9284..d3702180ef 100644
--- a/test/py/tests/test_efi_capsule/conftest.py
+++ b/test/py/tests/test_efi_capsule/conftest.py
@@ -87,6 +87,13 @@ def efi_capsule_data(request, u_boot_config):
                    % (u_boot_config.source_dir, u_boot_config.build_dir, data_dir, data_dir, data_dir), shell=True)
         os.environ['PYTHONPATH'] = pythonpath
 
+        capsule_cfg_file_gen = u_boot_config.buildconfig.get('config_efi_use_capsule_cfg_file')
+        if capsule_cfg_file_gen:
+            cfg_file = u_boot_config.source_dir + '/' +  u_boot_config.buildconfig.get('config_efi_capsule_cfg_file')[1:-1]
+            check_call('cd %s; '
+                       '%s/tools/mkeficapsule -f %s'
+                       % (data_dir, u_boot_config.build_dir, cfg_file), shell=True)
+
         # Create a disk image with EFI system partition
         check_call('virt-make-fs --partition=gpt --size=+1M --type=vfat %s %s' %
                    (mnt_point, image_path), shell=True)
-- 
2.34.1

Re: [PATCH 0/4] Generate capsules from config files

[Bence Balogh]

Hello Sughosh,

We are planning to use these changes in the Corstone-1000 platform.
Do you have a timeline when these are going to get merged?

Thanks,
Bence

On Tue, Nov 21, 2023 at 12:30:24PM +0530, Sughosh Ganu wrote:
> 
> Add support for generating capsules through a config file. Multiple
> capsules can be generated through a single invocation of the
> mkeficapsule tool, where the capsule parameters are fetched from the
> config file.
> 
> The format of the config file pretty similar to the one used by the
> capsule generation tool in EDKII.
> 
> The functionality is enabled on the sandbox variant and the EFI
> capsule update test is now configured to generate the capsules through
> a config file. The sandbox_flattree variant generates capsules through
> binman, thus covering both the paths.
> 
> Sughosh Ganu (4):
>   tools: mkeficapsule: Add support for parsing capsule params from
>     config file
>   doc: Document capsule generation through a config file
>   sandbox: capsule: Add a config file for generating capsules
>   test: capsule: Generate capsules through config file
> 
>  configs/sandbox_defconfig                     |   2 +
>  doc/develop/uefi/uefi.rst                     |  70 ++++
>  .../test_efi_capsule/capsule_gen_binman.dts   |   2 +
>  test/py/tests/test_efi_capsule/conftest.py    |   7 +
>  .../test_efi_capsule/sandbox_capsule_cfg.txt  | 162 ++++++++
>  tools/Kconfig                                 |  15 +
>  tools/Makefile                                |   1 +
>  tools/eficapsule.h                            | 114 ++++++
>  tools/mkeficapsule.c                          |  87 +++--
>  tools/mkeficapsule_parse.c                    | 352 ++++++++++++++++++
>  10 files changed, 781 insertions(+), 31 deletions(-)
>  create mode 100644 test/py/tests/test_efi_capsule/sandbox_capsule_cfg.txt
>  create mode 100644 tools/mkeficapsule_parse.c
> 
> -- 
> 2.34.1
> 
> 

Re: [PATCH 0/4] Generate capsules from config files

[Sughosh Ganu]

hi Bence,

On Tue, 12 Mar 2024 at 20:28, Bence Balogh <bence.balogh@arm.com> wrote:
>
> Hello Sughosh,
>
> We are planning to use these changes in the Corstone-1000 platform.
> Do you have a timeline when these are going to get merged?

I will prioritize this task since you want to use these changes. I
need to send some changes which Simon has asked for. Once those
changes go through, these patches also should be merged. Will work on
this task. Thanks for letting me know.

-sughosh

>
> Thanks,
> Bence
>
> On Tue, Nov 21, 2023 at 12:30:24PM +0530, Sughosh Ganu wrote:
> >
> > Add support for generating capsules through a config file. Multiple
> > capsules can be generated through a single invocation of the
> > mkeficapsule tool, where the capsule parameters are fetched from the
> > config file.
> >
> > The format of the config file pretty similar to the one used by the
> > capsule generation tool in EDKII.
> >
> > The functionality is enabled on the sandbox variant and the EFI
> > capsule update test is now configured to generate the capsules through
> > a config file. The sandbox_flattree variant generates capsules through
> > binman, thus covering both the paths.
> >
> > Sughosh Ganu (4):
> >   tools: mkeficapsule: Add support for parsing capsule params from
> >     config file
> >   doc: Document capsule generation through a config file
> >   sandbox: capsule: Add a config file for generating capsules
> >   test: capsule: Generate capsules through config file
> >
> >  configs/sandbox_defconfig                     |   2 +
> >  doc/develop/uefi/uefi.rst                     |  70 ++++
> >  .../test_efi_capsule/capsule_gen_binman.dts   |   2 +
> >  test/py/tests/test_efi_capsule/conftest.py    |   7 +
> >  .../test_efi_capsule/sandbox_capsule_cfg.txt  | 162 ++++++++
> >  tools/Kconfig                                 |  15 +
> >  tools/Makefile                                |   1 +
> >  tools/eficapsule.h                            | 114 ++++++
> >  tools/mkeficapsule.c                          |  87 +++--
> >  tools/mkeficapsule_parse.c                    | 352 ++++++++++++++++++
> >  10 files changed, 781 insertions(+), 31 deletions(-)
> >  create mode 100644 test/py/tests/test_efi_capsule/sandbox_capsule_cfg.txt
> >  create mode 100644 tools/mkeficapsule_parse.c
> >
> > --
> > 2.34.1
> >
> >