[RFC PATCH 0/5] ARM: imx: HAB improvements

[RFC PATCH 0/5] ARM: imx: HAB improvements

[Marcin Niestroj]

Hi,

This is a set of patches with HAB-related improvements.
Looking forward to feedback about them. Thanks!

Regards,
Marcin

Marcin Niestroj (5):
  scripts: imx: add optional argument to hab_blocks command
  scripts: imx: Specify CSF using -i option for new CST compatibility
  ARM: imx: Update default image certificate for CST tool
  scripts: imx: Support encrypted boot with HABv4
  images: imx: Add targets for signed encrypted images

 arch/arm/mach-imx/Kconfig                     |  13 +-
 .../mach-imx/include/mach/habv4-imx6-gencsf.h |  14 ++
 images/Makefile.imx                           |   3 +
 scripts/Makefile.lib                          |   3 +-
 scripts/imx/imx-image.c                       |  30 ++-
 scripts/imx/imx.c                             | 186 +++++++++++++++++-
 scripts/imx/imx.h                             |   9 +
 7 files changed, 244 insertions(+), 14 deletions(-)

-- 
2.17.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[RFC PATCH 1/5] scripts: imx: add optional argument to hab_blocks command

[Marcin Niestroj]

hab_blocks command is used to specify image authentication blocks for
HAB. Currently it was configured to authenticate full barebox
image. However in case of booting from SD card and adding MBR
partition table, HAB authentication fails, as final boot image is
modified.

Add an optional argument to hab_blocks command, to select between
3 types of authentication areas:
 - full: whole barebox image will be authenticated (this is default to
   keep compatibility),
 - from-dcdofs: image area up to dcdofs is not authenticated, so any
   changes up to dcdofs are possible,
 - skip-mbr: image area from 440 to 512 bytes is excluded from beeing
   authenticated, which allows to add / modify MBR partition table
   after building barebox image.

Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
---
 scripts/imx/imx.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/scripts/imx/imx.c b/scripts/imx/imx.c
index fb6ac001e..b47671f14 100644
--- a/scripts/imx/imx.c
+++ b/scripts/imx/imx.c
@@ -310,15 +310,40 @@ static int do_hab(struct config_data *data, int argc, char *argv[])
 
 static int do_hab_blocks(struct config_data *data, int argc, char *argv[])
 {
+	const char *type;
 	char *str;
 	int ret;
 
 	if (!data->csf)
 		return -EINVAL;
 
-	ret = asprintf(&str, "Blocks = 0x%08x 0 %d \"%s\"\n",
-		       data->image_load_addr,
-		       data->load_size, data->outfile);
+	if (argc < 2)
+		type = "full";
+	else
+		type = argv[1];
+
+	if (!strcmp(type, "full")) {
+		ret = asprintf(&str, "Blocks = 0x%08x 0 %d \"%s\"\n",
+			       data->image_load_addr, data->load_size,
+			       data->outfile);
+	} else if (!strcmp(type, "from-dcdofs")) {
+		ret = asprintf(&str, "Blocks = 0x%08x 0x%x %d \"%s\"\n",
+			       data->image_load_addr + data->image_dcd_offset,
+			       data->image_dcd_offset,
+			       data->load_size - data->image_dcd_offset,
+			       data->outfile);
+	} else if (!strcmp(type, "skip-mbr")) {
+		ret = asprintf(&str,
+			       "Blocks = 0x%08x 0 440 \"%s\", \\\n"
+			       "         0x%08x 512 %d \"%s\"\n",
+			       data->image_load_addr, data->outfile,
+			       data->image_load_addr + 512,
+			       data->load_size - 512, data->outfile);
+	} else {
+		fprintf(stderr, "Invalid hab_blocks option: %s\n", type);
+		return -EINVAL;
+	}
+
 	if (ret < 0)
 		return -ENOMEM;
 
-- 
2.17.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[RFC PATCH 2/5] scripts: imx: Support CST version >= 2.3.2

[Marcin Niestroj]

New versions of CST (starting from 2.3.2 according to Release Notes)
require specifying '-i --input' option, instead of writing
configuration to standard input. I see few possibilities here:
1) Simply add '-i /dev/fd/0' as a fix to current implementation
   (this patch actually does that).
2) Reimplement code to save CSF content to file, instead of malloc'ed
   buffer.
3) Above methods will only support CST tool >= 2.3.2. To keep
   compatibility with older CST versions, we would need to check
   'cst -h' output to see if '-i' option is supported or not (note,
   that version returned by CST 2.3.3 is 'BLN_CST_MAIN_02.03.00').

Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
---

 scripts/imx/imx-image.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/imx/imx-image.c b/scripts/imx/imx-image.c
index b241e8c4b..9688a8ce1 100644
--- a/scripts/imx/imx-image.c
+++ b/scripts/imx/imx-image.c
@@ -546,7 +546,7 @@ static int hab_sign(struct config_data *data)
 		}
 	}
 
-	ret = asprintf(&command, "%s -o %s", cst, csffile);
+	ret = asprintf(&command, "%s -i /dev/fd/0 -o %s", cst, csffile);
 	if (ret < 0)
 		return -ENOMEM;
 
-- 
2.17.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[RFC PATCH 3/5] ARM: imx: Update default image certificate for CST tool

[Marcin Niestroj]

CST tool by default generates certificate with
IMG1_1_sha256_4096_65537_v3_usr_crt.pem file name instead. Update
Kconfig default to match that. Verified with CST version 2.3.3.

Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
---
 arch/arm/mach-imx/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index be56acd3a..29ebc654c 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -786,7 +786,7 @@ config HABV4_CSF_CRT_PEM
 
 config HABV4_IMG_CRT_PEM
 	string "Path to IMG certificate"
-	default "../crts/IMG_1_sha256_4096_65537_v3_usr_crt.pem"
+	default "../crts/IMG1_1_sha256_4096_65537_v3_usr_crt.pem"
 	help
 	  Path to the Image certificate, produced by the Freescale
 	  Public Key Infrastructure (PKI) script.
-- 
2.17.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[RFC PATCH 4/5] scripts: imx: Support encrypted boot with HABv4

[Marcin Niestroj]

.imxcfg configuration files support few more commands, all starting
with "hab_encrypt" string. That way it is possible to easily ignore
these commands, when image encryption was not requested. Hence, we can
use single .imxcfg file to generate signed and encrypted images in the
same build.

Images are encrypted in place by Freescale Code Signing Tool (cst),
using Data Encryption Key (DEK). This key needs to be encapsulated
by processor's hardware encryption engine to produce DEK blob, which
is unique for each device. DEK blob needs to be part of CSF area,
so we make enough space on the end of image to simply append it later,
e.g. during device flash procedure.

Introduced code was developed and tested on NXP i.MX6UL platform.

Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
---
Extending table with lot of hab_encrypt* commands is not super nice.
I am open to better ideas here!

 arch/arm/mach-imx/Kconfig                     |  11 ++
 .../mach-imx/include/mach/habv4-imx6-gencsf.h |  14 ++
 scripts/Makefile.lib                          |   3 +-
 scripts/imx/imx-image.c                       |  28 ++-
 scripts/imx/imx.c                             | 161 +++++++++++++++++-
 scripts/imx/imx.h                             |   9 +
 6 files changed, 214 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 29ebc654c..9822a7e8d 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -794,6 +794,17 @@ config HABV4_IMG_CRT_PEM
 	  This file will be inserted into the Command Sequence File
 	  (CSF) when using the CSF template that comes with barebox.
 
+config HABV4_DEK
+	string "Path to DEK"
+	default "../crts/dek"
+	help
+	  Path to the Data Encryption Key (DEK), produced by the
+	  Freescale Code Signing Tool (cst).
+
+	  Image will be encrypted using this key and enough space will
+	  be reserved on the end of the image, so DEK blob (unique for
+	  each device) can be simply appended during flashing.
+
 endif
 
 config HABV3
diff --git a/arch/arm/mach-imx/include/mach/habv4-imx6-gencsf.h b/arch/arm/mach-imx/include/mach/habv4-imx6-gencsf.h
index 0649caa0c..b4fdf512b 100644
--- a/arch/arm/mach-imx/include/mach/habv4-imx6-gencsf.h
+++ b/arch/arm/mach-imx/include/mach/habv4-imx6-gencsf.h
@@ -6,6 +6,7 @@
  * CONFIG_HABV4_TABLE_BIN
  * CONFIG_HABV4_CSF_CRT_PEM
  * CONFIG_HABV4_IMG_CRT_PEM
+ * CONFIG_HABV4_DEK
  */
 
 hab [Header]
@@ -43,3 +44,16 @@ hab [Authenticate Data]
 hab Verification index = 2
 
 hab_blocks
+
+hab_encrypt [Install Secret Key]
+hab_encrypt Verification index = 0
+hab_encrypt Target index = 0
+hab_encrypt_key CONFIG_HABV4_DEK
+hab_encrypt_key_length
+hab_encrypt_blob_address
+
+hab_encrypt [Decrypt Data]
+hab_encrypt Verification index = 0
+hab_encrypt Mac Bytes = 16
+
+hab_encrypt_blocks
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 272b5981e..9e432daee 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -432,7 +432,8 @@ imxcfg_cpp_flags  = -Wp,-MD,$(depfile) -nostdinc -x assembler-with-cpp \
       -DCONFIG_HABV3_IMG_CRT_DER=\"$(CONFIG_HABV3_IMG_CRT_DER)\" \
       -DCONFIG_HABV4_TABLE_BIN=\"$(CONFIG_HABV4_TABLE_BIN)\" \
       -DCONFIG_HABV4_CSF_CRT_PEM=\"$(CONFIG_HABV4_CSF_CRT_PEM)\" \
-      -DCONFIG_HABV4_IMG_CRT_PEM=\"$(CONFIG_HABV4_IMG_CRT_PEM)\"
+      -DCONFIG_HABV4_IMG_CRT_PEM=\"$(CONFIG_HABV4_IMG_CRT_PEM)\" \
+      -DCONFIG_HABV4_DEK=\"$(CONFIG_HABV4_DEK)\"
 
 dcd-tmp = $(subst $(comma),_,$(dot-target).dcd.tmp)
 
diff --git a/scripts/imx/imx-image.c b/scripts/imx/imx-image.c
index 9688a8ce1..eef6e0902 100644
--- a/scripts/imx/imx-image.c
+++ b/scripts/imx/imx-image.c
@@ -36,7 +36,6 @@
 #include <include/filetype.h>
 
 #define MAX_DCD 1024
-#define CSF_LEN 0x2000		/* length of the CSF (needed for HAB) */
 
 static uint32_t dcdtable[MAX_DCD];
 static int curdcd;
@@ -521,6 +520,7 @@ static int hab_sign(struct config_data *data)
 	struct stat s;
 	char *cst;
 	void *buf;
+	size_t csf_space = CSF_LEN;
 
 	cst = getenv("CST");
 	if (!cst)
@@ -601,15 +601,23 @@ static int hab_sign(struct config_data *data)
 		return -errno;
 	}
 
-	buf = malloc(CSF_LEN);
+	/*
+	 * DEK blob needs to be part of CSF area, in order to properly
+	 * load by ROM code. Make space to simply concatenate DEK blob
+	 * to the end of image during device flashing procedure.
+	 */
+	if (data->encrypt_image)
+		csf_space -= (data->dek_size + DEK_BLOB_OVERHEAD);
+
+	buf = malloc(csf_space);
 	if (!buf)
 		return -ENOMEM;
 
-	memset(buf, 0x5a, CSF_LEN);
+	memset(buf, 0x5a, csf_space);
 
-	if (s.st_size > CSF_LEN) {
-		fprintf(stderr, "CSF file size exceeds maximum CSF len of %d bytes\n",
-			CSF_LEN);
+	if (s.st_size > csf_space) {
+		fprintf(stderr, "CSF file size exceeds maximum CSF space of %zu bytes\n",
+			csf_space);
 	}
 
 	ret = xread(fd, buf, s.st_size);
@@ -620,7 +628,7 @@ static int hab_sign(struct config_data *data)
 
 	outfd = open(data->outfile, O_WRONLY | O_APPEND);
 
-	ret = xwrite(outfd, buf, CSF_LEN);
+	ret = xwrite(outfd, buf, csf_space);
 	if (ret < 0) {
 		fprintf(stderr, "write failed: %s\n", strerror(errno));
 		return -errno;
@@ -685,7 +693,7 @@ int main(int argc, char *argv[])
 
 	prgname = argv[0];
 
-	while ((opt = getopt(argc, argv, "c:hf:o:bdus")) != -1) {
+	while ((opt = getopt(argc, argv, "c:hf:o:bduse")) != -1) {
 		switch (opt) {
 		case 'c':
 			configfile = optarg;
@@ -708,6 +716,9 @@ int main(int argc, char *argv[])
 		case 'u':
 			create_usb_image = 1;
 			break;
+		case 'e':
+			data.encrypt_image = 1;
+			break;
 		case 'h':
 			usage(argv[0]);
 		default:
@@ -750,6 +761,7 @@ int main(int argc, char *argv[])
 	 */
 	data.load_size = roundup(data.image_size + HEADER_LEN, 0x1000);
 
+
 	ret = parse_config(&data, configfile);
 	if (ret)
 		exit(1);
diff --git a/scripts/imx/imx.c b/scripts/imx/imx.c
index b47671f14..e4deaff5b 100644
--- a/scripts/imx/imx.c
+++ b/scripts/imx/imx.c
@@ -22,6 +22,7 @@
 #include <string.h>
 #include <stdint.h>
 #include <errno.h>
+#include <sys/stat.h>
 #include <linux/kernel.h>
 #include <mach/imx_cpu_types.h>
 
@@ -29,6 +30,12 @@
 
 #define MAXARGS 32
 
+/*
+ * First word of bootloader image should be authenticated,
+ * encrypt the rest.
+ */
+#define ENCRYPT_OFFSET	(HEADER_LEN + 0x10)
+
 static int parse_line(char *line, char *argv[])
 {
 	int nargs = 0;
@@ -313,6 +320,7 @@ static int do_hab_blocks(struct config_data *data, int argc, char *argv[])
 	const char *type;
 	char *str;
 	int ret;
+	uint32_t signed_size = data->load_size;
 
 	if (!data->csf)
 		return -EINVAL;
@@ -322,15 +330,22 @@ static int do_hab_blocks(struct config_data *data, int argc, char *argv[])
 	else
 		type = argv[1];
 
+	/*
+	 * In case of encrypted image we reduce signed area to beginning
+	 * of encrypted area.
+	 */
+	if (data->encrypt_image)
+		signed_size = ENCRYPT_OFFSET;
+
 	if (!strcmp(type, "full")) {
 		ret = asprintf(&str, "Blocks = 0x%08x 0 %d \"%s\"\n",
-			       data->image_load_addr, data->load_size,
+			       data->image_load_addr, signed_size,
 			       data->outfile);
 	} else if (!strcmp(type, "from-dcdofs")) {
 		ret = asprintf(&str, "Blocks = 0x%08x 0x%x %d \"%s\"\n",
 			       data->image_load_addr + data->image_dcd_offset,
 			       data->image_dcd_offset,
-			       data->load_size - data->image_dcd_offset,
+			       signed_size - data->image_dcd_offset,
 			       data->outfile);
 	} else if (!strcmp(type, "skip-mbr")) {
 		ret = asprintf(&str,
@@ -338,7 +353,7 @@ static int do_hab_blocks(struct config_data *data, int argc, char *argv[])
 			       "         0x%08x 512 %d \"%s\"\n",
 			       data->image_load_addr, data->outfile,
 			       data->image_load_addr + 512,
-			       data->load_size - 512, data->outfile);
+			       signed_size - 512, data->outfile);
 	} else {
 		fprintf(stderr, "Invalid hab_blocks option: %s\n", type);
 		return -EINVAL;
@@ -354,6 +369,131 @@ static int do_hab_blocks(struct config_data *data, int argc, char *argv[])
 	return 0;
 }
 
+static int do_hab_encrypt(struct config_data *data, int argc, char *argv[])
+{
+	if (!data->encrypt_image)
+		return 0;
+
+	return do_hab(data, argc, argv);
+}
+
+static int do_hab_encrypt_key(struct config_data *data, int argc, char *argv[])
+{
+	char *str;
+	char *dekfile;
+	struct stat s;
+	int ret;
+
+	if (!data->csf)
+		return -EINVAL;
+
+	if (!data->encrypt_image)
+		return 0;
+
+	if (argc < 2)
+		return -EINVAL;
+
+	/* Strip quotation marks */
+	dekfile = argv[1] + 1;
+	dekfile[strlen(dekfile) - 1] = '\0';
+
+	ret = stat(dekfile, &s);
+	if (ret) {
+		fprintf(stderr, "failed to stat(%s) DEK: (%d) %s\n",
+			dekfile, errno, strerror(errno));
+		return -errno;
+	}
+
+	data->dek_size = s.st_size;
+
+	ret = asprintf(&str, "Key = \"%s\"\n", dekfile);
+	if (ret < 0)
+		return -ENOMEM;
+
+	ret = hab_add_str(data, str);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int do_hab_encrypt_key_length(struct config_data *data, int argc,
+				     char *argv[])
+{
+	char *str;
+	int ret;
+
+	if (!data->csf)
+		return -EINVAL;
+
+	if (!data->encrypt_image)
+		return 0;
+
+	if (!data->dek_size)
+		return -EINVAL;
+
+	ret = asprintf(&str, "Key Length = %zu\n", data->dek_size * 8);
+	if (ret < 0)
+		return -ENOMEM;
+
+	ret = hab_add_str(data, str);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int do_hab_encrypt_blob_address(struct config_data *data, int argc,
+				       char *argv[])
+{
+	char *str;
+	int ret;
+
+	if (!data->csf)
+		return -EINVAL;
+
+	if (!data->encrypt_image)
+		return 0;
+
+	ret = asprintf(&str,
+		       "Blob address = 0x%08zx\n",
+		       data->image_load_addr + data->load_size + CSF_LEN -
+				(DEK_BLOB_OVERHEAD + data->dek_size));
+	if (ret < 0)
+		return -ENOMEM;
+
+	ret = hab_add_str(data, str);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int do_hab_encrypt_blocks(struct config_data *data, int argc,
+				 char *argv[])
+{
+	char *str;
+	int ret;
+
+	if (!data->csf)
+		return -EINVAL;
+
+	if (!data->encrypt_image)
+		return 0;
+
+	ret = asprintf(&str, "Blocks = 0x%08x 0x%x %d \"%s\"\n",
+		       data->image_load_addr + ENCRYPT_OFFSET, ENCRYPT_OFFSET,
+		       data->load_size - ENCRYPT_OFFSET, data->outfile);
+	if (ret < 0)
+		return -ENOMEM;
+
+	ret = hab_add_str(data, str);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 static int do_super_root_key(struct config_data *data, int argc, char *argv[])
 {
 	int len;
@@ -417,6 +557,21 @@ struct command cmds[] = {
 	}, {
 		.name = "hab_blocks",
 		.parse = do_hab_blocks,
+	}, {
+		.name = "hab_encrypt",
+		.parse = do_hab_encrypt,
+	}, {
+		.name = "hab_encrypt_key",
+		.parse = do_hab_encrypt_key,
+	}, {
+		.name = "hab_encrypt_key_length",
+		.parse = do_hab_encrypt_key_length,
+	}, {
+		.name = "hab_encrypt_blob_address",
+		.parse = do_hab_encrypt_blob_address,
+	}, {
+		.name = "hab_encrypt_blocks",
+		.parse = do_hab_encrypt_blocks,
 	}, {
 		.name = "super_root_key",
 		.parse = do_super_root_key,
diff --git a/scripts/imx/imx.h b/scripts/imx/imx.h
index c7677f81a..efe8c7289 100644
--- a/scripts/imx/imx.h
+++ b/scripts/imx/imx.h
@@ -1,6 +1,13 @@
 
 #define HEADER_LEN 0x1000	/* length of the blank area + IVT + DCD */
+#define CSF_LEN 0x2000		/* length of the CSF (needed for HAB) */
 
+#define DEK_BLOB_HEADER 8	/* length of DEK blob header */
+#define DEK_BLOB_KEY 32		/* length of DEK blob AES-256 key */
+#define DEK_BLOB_MAC 16		/* length of DEK blob MAC */
+
+/* DEK blob length excluding DEK itself */
+#define DEK_BLOB_OVERHEAD (DEK_BLOB_HEADER + DEK_BLOB_KEY + DEK_BLOB_MAC)
 /*
  * ============================================================================
  * i.MX flash header v1 handling. Found on i.MX35 and i.MX51
@@ -80,6 +87,8 @@ struct config_data {
 	int (*nop)(const struct config_data *data);
 	int csf_space;
 	char *csf;
+	int encrypt_image;
+	size_t dek_size;
 };
 
 #define MAX_RECORDS_DCD_V2 1024
-- 
2.17.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[RFC PATCH 5/5] images: imx: Add targets for signed encrypted images

[Marcin Niestroj]

Add .esimximg target for signed and encrypted images.

Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
---
 images/Makefile.imx | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/images/Makefile.imx b/images/Makefile.imx
index 5fcfbbefb..c1a601ea9 100644
--- a/images/Makefile.imx
+++ b/images/Makefile.imx
@@ -20,6 +20,9 @@ $(obj)/%.simximg: $(obj)/% FORCE
 $(obj)/%.usimximg: $(obj)/% FORCE
 	$(call if_changed,imx_image,$(CFG_$(patsubst %.usimximg,%.imximg,$(@F))),-s -u)
 
+$(obj)/%.esimximg: $(obj)/% FORCE
+	$(call if_changed,imx_image,$(CFG_$(patsubst %.esimximg,%.imximg,$(@F))),-s -e)
+
 quiet_cmd_imx_sram_img ?= IMX-SRAM-IMG    $@
       cmd_imx_sram_img ?= cat $(obj)/$(patsubst %.imx-sram-img,%.pblb,$(2)) > $@; \
 		  $(call size_append, $(obj)/barebox.z) >> $@; \
-- 
2.17.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [RFC PATCH 2/5] scripts: imx: Support CST version >= 2.3.2

[Sascha Hauer]

On Wed, Jun 06, 2018 at 06:08:06PM +0200, Marcin Niestroj wrote:
> New versions of CST (starting from 2.3.2 according to Release Notes)
> require specifying '-i --input' option, instead of writing
> configuration to standard input. I see few possibilities here:
> 1) Simply add '-i /dev/fd/0' as a fix to current implementation
>    (this patch actually does that).
> 2) Reimplement code to save CSF content to file, instead of malloc'ed
>    buffer.
> 3) Above methods will only support CST tool >= 2.3.2. To keep
>    compatibility with older CST versions, we would need to check
>    'cst -h' output to see if '-i' option is supported or not (note,
>    that version returned by CST 2.3.3 is 'BLN_CST_MAIN_02.03.00').

You're kidding, no? My 2.3.1 also returns BLN_CST_MAIN_02.03.00. So they
managed to switch the tool to a completely incopatible API without even
increasing the version number. Great.

There's also:

4) Try both methods and see which one succeeds
5) evaluate a CST_VERSION environment variable, expect the new version
   if this is not given.

I think I am happy when we just do a hard switch to the new version like
done in your patch, but then the commit message should be reworded to
"require CST version >= 2.3.2", not "suppport". Also if the command
fails we should add a message like "CST failed, maybe it's too old. We
require at least CST-2.3.2"

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [RFC PATCH 0/5] ARM: imx: HAB improvements

[Sascha Hauer]

Hi Marcin,

On Wed, Jun 06, 2018 at 06:08:04PM +0200, Marcin Niestroj wrote:
> Hi,
> 
> This is a set of patches with HAB-related improvements.
> Looking forward to feedback about them. Thanks!

Apart from the comments made to 2/5 the rest looks fine.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox