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

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

[Marcin Niestroj]

Hi,

This is a set of patches with HAB-related improvements. Basically it is
updated version of RFC [1] I've posted some time ago.

Changes RFC -> v1:
 * drop "[RFC PATCH 2/5] scripts: imx: Support CST version >= 2.3.2",
   as there is new implementation in the mailing list already [2]
 * add "[PATCH 5/5] scripts: imx: Do not include autoconf.h"
 * update "[PATCH 3/5] scripts: imx: Support encrypted boot with HABv4"
   and "[PATCH 4/5] images: imx: Add targets for signed encrypted images"

[1] https://www.spinics.net/lists/u-boot-v2/msg33203.html
[2] https://www.spinics.net/lists/u-boot-v2/msg34332.html

Marcin Niestroj (5):
  scripts: imx: add optional argument to hab_blocks command
  ARM: imx: Update default image certificate for CST tool
  scripts: imx: Support encrypted boot with HABv4
  images: imx: Add targets for signed encrypted images
  scripts: imx: Do not include autoconf.h

 arch/arm/mach-imx/Kconfig                     |   2 +-
 .../mach-imx/include/mach/habv4-imx6-gencsf.h |  13 ++
 arch/arm/mach-imx/include/mach/imx-header.h   |  10 +
 images/Makefile.imx                           |   8 +
 scripts/Makefile.lib                          |   1 -
 scripts/imx/imx-image.c                       |  27 ++-
 scripts/imx/imx.c                             | 183 +++++++++++++++++-
 7 files changed, 231 insertions(+), 13 deletions(-)

-- 
2.18.0


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

[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>
---
Changes rfc -> v1: none

 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 d3786b6e1..21206387e 100644
--- a/scripts/imx/imx.c
+++ b/scripts/imx/imx.c
@@ -317,15 +317,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.18.0


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

[PATCH 2/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.

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

 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 7cb9138d2..eaf76703f 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -791,7 +791,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.18.0


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

[PATCH 3/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>
---
Changes rfc -> v1:
 * Do not use fixed DEK path from Kconfig, but generate DEK path based
   on output filename. This supports parallel builds and allows CST tool
   to generate new DEK for every board.
 * Explicitly specify DEK size in .imxcfg

 .../mach-imx/include/mach/habv4-imx6-gencsf.h |  13 ++
 arch/arm/mach-imx/include/mach/imx-header.h   |  10 ++
 scripts/imx/imx-image.c                       |  27 ++-
 scripts/imx/imx.c                             | 158 +++++++++++++++++-
 4 files changed, 197 insertions(+), 11 deletions(-)

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..581887960 100644
--- a/arch/arm/mach-imx/include/mach/habv4-imx6-gencsf.h
+++ b/arch/arm/mach-imx/include/mach/habv4-imx6-gencsf.h
@@ -43,3 +43,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
+hab_encrypt_key_length 256
+hab_encrypt_blob_address
+
+hab_encrypt [Decrypt Data]
+hab_encrypt Verification index = 0
+hab_encrypt Mac Bytes = 16
+
+hab_encrypt_blocks
diff --git a/arch/arm/mach-imx/include/mach/imx-header.h b/arch/arm/mach-imx/include/mach/imx-header.h
index c9b2a5881..3c92aecf1 100644
--- a/arch/arm/mach-imx/include/mach/imx-header.h
+++ b/arch/arm/mach-imx/include/mach/imx-header.h
@@ -4,6 +4,14 @@
 #include <linux/types.h>
 
 #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)
 
 /*
  * ============================================================================
@@ -94,6 +102,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
diff --git a/scripts/imx/imx-image.c b/scripts/imx/imx-image.c
index 452a544bc..5ed64af47 100644
--- a/scripts/imx/imx-image.c
+++ b/scripts/imx/imx-image.c
@@ -43,7 +43,6 @@
  * HEADER_SIZE
  */
 #define MAX_DCD ((HEADER_LEN - FLASH_HEADER_OFFSET - sizeof(struct imx_flash_header_v2)) / sizeof(u32))
-#define CSF_LEN 0x2000		/* length of the CSF (needed for HAB) */
 
 static uint32_t dcdtable[MAX_DCD];
 static int curdcd;
@@ -533,6 +532,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)
@@ -613,15 +613,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);
@@ -632,7 +640,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;
@@ -707,7 +715,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;
@@ -730,6 +738,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:
diff --git a/scripts/imx/imx.c b/scripts/imx/imx.c
index 21206387e..eeb9b69c0 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;
@@ -320,6 +327,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;
@@ -329,15 +337,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,
@@ -345,7 +360,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;
@@ -361,6 +376,128 @@ 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;
+	int ret;
+
+	if (!data->csf)
+		return -EINVAL;
+
+	if (!data->encrypt_image)
+		return 0;
+
+	ret = asprintf(&dekfile, "%s.dek", data->outfile);
+	if (ret < 0)
+		return -ENOMEM;
+
+	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[])
+{
+	unsigned int dek_bits;
+	char *str;
+	int ret;
+
+	if (!data->csf)
+		return -EINVAL;
+
+	if (!data->encrypt_image)
+		return 0;
+
+	if (argc < 2)
+		return -EINVAL;
+
+	dek_bits = strtoul(argv[1], NULL, 0);
+
+	if (dek_bits != 128 && dek_bits != 192 && dek_bits != 256) {
+		fprintf(stderr, "wrong dek size (%u)\n", dek_bits);
+		return -EINVAL;
+	}
+
+	data->dek_size = dek_bits / 8;
+
+	ret = asprintf(&str, "Key Length = %u\n", dek_bits);
+	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;
@@ -424,6 +561,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,
-- 
2.18.0


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

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

[Marcin Niestroj]

Add .esimximg and .esimximg.dek targets for signed and encrypted
images and their corresponding DEKs. Also add rule to generate final
.img.dek files.

As an example, adding encrypted images for imx6ull_evk would look like
this:

  FILE_barebox-nxp-imx6ull-evk-encrypted.img = start_nxp_imx6ull_evk.pblx.esimximg
  image-$(CONFIG_MACH_NXP_IMX6ULL_EVK) += barebox-nxp-imx6ull-evk-encrypted.img

  FILE_barebox-nxp-imx6ull-evk-encrypted.img.dek = start_nxp_imx6ull_evk.pblx.esimximg.dek
  image-$(CONFIG_MACH_NXP_IMX6ULL_EVK) += barebox-nxp-imx6ull-evk-encrypted.img.dek

Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
---
Changes rfc -> v1:
 * Add Makefile rule for .dek files

 images/Makefile.imx | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/images/Makefile.imx b/images/Makefile.imx
index aefc52af4..156eb6478 100644
--- a/images/Makefile.imx
+++ b/images/Makefile.imx
@@ -20,6 +20,14 @@ $(obj)/%.simximg: $(obj)/% FORCE
 $(obj)/%.usimximg: $(obj)/% FORCE
 	$(call if_changed,imx_image,$(CFG_$(patsubst %.usimximg,%.imximg,$(@F))),-s -u)
 
+$(obj)/%.esimximg $(obj)/%.esimximg.dek: $(obj)/% FORCE
+	$(call if_changed,imx_image,$(CFG_$(patsubst %.esimximg,%.imximg,$(@F))),-s -e)
+
+.SECONDEXPANSION:
+$(obj)/%.img.dek: $(obj)/$$(FILE_$$(@F))
+	$(Q)if [ -z $(FILE_$(@F)) ]; then echo "FILE_$(@F) empty!"; false; fi
+	$(call if_changed,shipped)
+
 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.18.0


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

[PATCH 5/5] scripts: imx: Do not include autoconf.h

[Marcin Niestroj]

All required defines are passed with command line -D options, so there
is no need to include full autoconf.h header. Additionally there were
compile time warnings printed when passed variables (CONFIG_HABV*) had
embedded environment variables, because macros in autoconf.h were
redefining these passed as command line arguments (-include is
processed later than -D option).

Remove including autoconf.h header, so CONFIG_HABV* macros are only
defined with command line arguments, properly expanding embedded
variables.

Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
---
Changes rfc -> v1: new patch

 scripts/Makefile.lib | 1 -
 1 file changed, 1 deletion(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 3b1308605..c254b5b79 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -433,7 +433,6 @@ cmd_imximage_S_dcd=						\
 
 imxcfg_cpp_flags  = -Wp,-MD,$(depfile) -nostdinc -x assembler-with-cpp \
       -I $(srctree)/include -I $(srctree)/arch/arm/mach-imx/include \
-      -include include/generated/autoconf.h \
       -DCONFIG_HABV3_SRK_PEM=\"$(CONFIG_HABV3_SRK_PEM)\" \
       -DCONFIG_HABV3_CSF_CRT_DER=\"$(CONFIG_HABV3_CSF_CRT_DER)\" \
       -DCONFIG_HABV3_IMG_CRT_DER=\"$(CONFIG_HABV3_IMG_CRT_DER)\" \
-- 
2.18.0


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

Re: [PATCH 5/5] scripts: imx: Do not include autoconf.h

[Sascha Hauer]

On Mon, Sep 03, 2018 at 12:57:15PM +0200, Marcin Niestroj wrote:
> All required defines are passed with command line -D options, so there
> is no need to include full autoconf.h header. Additionally there were
> compile time warnings printed when passed variables (CONFIG_HABV*) had
> embedded environment variables, because macros in autoconf.h were
> redefining these passed as command line arguments (-include is
> processed later than -D option).
> 
> Remove including autoconf.h header, so CONFIG_HABV* macros are only
> defined with command line arguments, properly expanding embedded
> variables.
> 
> Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
> ---
> Changes rfc -> v1: new patch
> 
>  scripts/Makefile.lib | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 3b1308605..c254b5b79 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -433,7 +433,6 @@ cmd_imximage_S_dcd=						\
>  
>  imxcfg_cpp_flags  = -Wp,-MD,$(depfile) -nostdinc -x assembler-with-cpp \
>        -I $(srctree)/include -I $(srctree)/arch/arm/mach-imx/include \
> -      -include include/generated/autoconf.h \
>        -DCONFIG_HABV3_SRK_PEM=\"$(CONFIG_HABV3_SRK_PEM)\" \
>        -DCONFIG_HABV3_CSF_CRT_DER=\"$(CONFIG_HABV3_CSF_CRT_DER)\" \
>        -DCONFIG_HABV3_IMG_CRT_DER=\"$(CONFIG_HABV3_IMG_CRT_DER)\" \

the inclusion of autoconf.h is there so that we can use the CONFIG_*
macros. See for example rch/arm/boards/tqma53/flash-header.imxcfg:36

#ifdef CONFIG_MACH_TQMA53_1GB_RAM

What we can do instead is replace the -include with "-I
$(objtree)/include" so that we can do a "#include
<generated/autoconf.h>" in arch/arm/boards/tqma53/flash-header.imxcfg.
Would that be fine aswell?

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: [PATCH 5/5] scripts: imx: Do not include autoconf.h

[Marcin Niestrój]

Hi Sasha,

Sascha Hauer <s.hauer@pengutronix.de> writes:

> On Mon, Sep 03, 2018 at 12:57:15PM +0200, Marcin Niestroj wrote:
>> All required defines are passed with command line -D options, so there
>> is no need to include full autoconf.h header. Additionally there were
>> compile time warnings printed when passed variables (CONFIG_HABV*) had
>> embedded environment variables, because macros in autoconf.h were
>> redefining these passed as command line arguments (-include is
>> processed later than -D option).
>> 
>> Remove including autoconf.h header, so CONFIG_HABV* macros are only
>> defined with command line arguments, properly expanding embedded
>> variables.
>> 
>> Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
>> ---
>> Changes rfc -> v1: new patch
>> 
>>  scripts/Makefile.lib | 1 -
>>  1 file changed, 1 deletion(-)
>> 
>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>> index 3b1308605..c254b5b79 100644
>> --- a/scripts/Makefile.lib
>> +++ b/scripts/Makefile.lib
>> @@ -433,7 +433,6 @@ cmd_imximage_S_dcd=						\
>>  
>>  imxcfg_cpp_flags  = -Wp,-MD,$(depfile) -nostdinc -x assembler-with-cpp \
>>        -I $(srctree)/include -I $(srctree)/arch/arm/mach-imx/include \
>> -      -include include/generated/autoconf.h \
>>        -DCONFIG_HABV3_SRK_PEM=\"$(CONFIG_HABV3_SRK_PEM)\" \
>>        -DCONFIG_HABV3_CSF_CRT_DER=\"$(CONFIG_HABV3_CSF_CRT_DER)\" \
>>        -DCONFIG_HABV3_IMG_CRT_DER=\"$(CONFIG_HABV3_IMG_CRT_DER)\" \
>
> the inclusion of autoconf.h is there so that we can use the CONFIG_*
> macros. See for example rch/arm/boards/tqma53/flash-header.imxcfg:36
>
> #ifdef CONFIG_MACH_TQMA53_1GB_RAM

Okay, so we cannot drop autoconf.h inclusion. That also means that we
need some other way to expand embedded variables in CONFIG_HAB* Kconfig
options. The problem is that autoconf.h overwrites (redefines) all
macros specified at command line.

>
> What we can do instead is replace the -include with "-I
> $(objtree)/include" so that we can do a "#include
> <generated/autoconf.h>" in arch/arm/boards/tqma53/flash-header.imxcfg.
> Would that be fine aswell?

This will also result in autoconf.h redefining macros specified at
command line, but only for that board (because of explicit "include
<generated/autoconf.h>"). Problem will surface once again when someone
will start using HAB for that board.

Maybe we should expand all macros from autoconf.h? For example process
autoconf.h *somehow* and produce autoconf-expanded.h, which we could use
with "-include" option. This is just an idea, I do not know how to
implement that. Any other ideas?

>
> Sascha

-- 
Regards,
Marcin

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

Re: [PATCH 5/5] scripts: imx: Do not include autoconf.h

[Sascha Hauer]

On Tue, Sep 04, 2018 at 11:38:46AM +0200, Marcin Niestrój wrote:
> Hi Sasha,
> 
> Sascha Hauer <s.hauer@pengutronix.de> writes:
> 
> > On Mon, Sep 03, 2018 at 12:57:15PM +0200, Marcin Niestroj wrote:
> >> All required defines are passed with command line -D options, so there
> >> is no need to include full autoconf.h header. Additionally there were
> >> compile time warnings printed when passed variables (CONFIG_HABV*) had
> >> embedded environment variables, because macros in autoconf.h were
> >> redefining these passed as command line arguments (-include is
> >> processed later than -D option).
> >> 
> >> Remove including autoconf.h header, so CONFIG_HABV* macros are only
> >> defined with command line arguments, properly expanding embedded
> >> variables.
> >> 
> >> Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
> >> ---
> >> Changes rfc -> v1: new patch
> >> 
> >>  scripts/Makefile.lib | 1 -
> >>  1 file changed, 1 deletion(-)
> >> 
> >> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> >> index 3b1308605..c254b5b79 100644
> >> --- a/scripts/Makefile.lib
> >> +++ b/scripts/Makefile.lib
> >> @@ -433,7 +433,6 @@ cmd_imximage_S_dcd=						\
> >>  
> >>  imxcfg_cpp_flags  = -Wp,-MD,$(depfile) -nostdinc -x assembler-with-cpp \
> >>        -I $(srctree)/include -I $(srctree)/arch/arm/mach-imx/include \
> >> -      -include include/generated/autoconf.h \
> >>        -DCONFIG_HABV3_SRK_PEM=\"$(CONFIG_HABV3_SRK_PEM)\" \
> >>        -DCONFIG_HABV3_CSF_CRT_DER=\"$(CONFIG_HABV3_CSF_CRT_DER)\" \
> >>        -DCONFIG_HABV3_IMG_CRT_DER=\"$(CONFIG_HABV3_IMG_CRT_DER)\" \
> >
> > the inclusion of autoconf.h is there so that we can use the CONFIG_*
> > macros. See for example rch/arm/boards/tqma53/flash-header.imxcfg:36
> >
> > #ifdef CONFIG_MACH_TQMA53_1GB_RAM
> 
> Okay, so we cannot drop autoconf.h inclusion. That also means that we
> need some other way to expand embedded variables in CONFIG_HAB* Kconfig
> options. The problem is that autoconf.h overwrites (redefines) all
> macros specified at command line.
> 
> >
> > What we can do instead is replace the -include with "-I
> > $(objtree)/include" so that we can do a "#include
> > <generated/autoconf.h>" in arch/arm/boards/tqma53/flash-header.imxcfg.
> > Would that be fine aswell?
> 
> This will also result in autoconf.h redefining macros specified at
> command line, but only for that board (because of explicit "include
> <generated/autoconf.h>"). Problem will surface once again when someone
> will start using HAB for that board.
> 
> Maybe we should expand all macros from autoconf.h? For example process
> autoconf.h *somehow* and produce autoconf-expanded.h, which we could use
> with "-include" option. This is just an idea, I do not know how to
> implement that. Any other ideas?

FYI I just posted another idea.

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: [PATCH 0/5] ARM: imx: HAB improvements

[Sascha Hauer]

On Mon, Sep 03, 2018 at 12:57:10PM +0200, Marcin Niestroj wrote:
> Hi,
> 
> This is a set of patches with HAB-related improvements. Basically it is
> updated version of RFC [1] I've posted some time ago.
> 
> Changes RFC -> v1:
>  * drop "[RFC PATCH 2/5] scripts: imx: Support CST version >= 2.3.2",
>    as there is new implementation in the mailing list already [2]
>  * add "[PATCH 5/5] scripts: imx: Do not include autoconf.h"
>  * update "[PATCH 3/5] scripts: imx: Support encrypted boot with HABv4"
>    and "[PATCH 4/5] images: imx: Add targets for signed encrypted images"
> 
> [1] https://www.spinics.net/lists/u-boot-v2/msg33203.html
> [2] https://www.spinics.net/lists/u-boot-v2/msg34332.html
> 
> Marcin Niestroj (5):
>   scripts: imx: add optional argument to hab_blocks command
>   ARM: imx: Update default image certificate for CST tool
>   scripts: imx: Support encrypted boot with HABv4
>   images: imx: Add targets for signed encrypted images
>   scripts: imx: Do not include autoconf.h

Applied 1-4 for now. The fifths patch needs some adjustments.

Sascha

> 
>  arch/arm/mach-imx/Kconfig                     |   2 +-
>  .../mach-imx/include/mach/habv4-imx6-gencsf.h |  13 ++
>  arch/arm/mach-imx/include/mach/imx-header.h   |  10 +
>  images/Makefile.imx                           |   8 +
>  scripts/Makefile.lib                          |   1 -
>  scripts/imx/imx-image.c                       |  27 ++-
>  scripts/imx/imx.c                             | 183 +++++++++++++++++-
>  7 files changed, 231 insertions(+), 13 deletions(-)
> 
> -- 
> 2.18.0
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
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