[U-Boot] [PATCH v2 0/9] Add new OPTEE bootm support to u-boot

[U-Boot] [PATCH v2 0/9] Add new OPTEE bootm support to u-boot

[Bryan O'Donoghue]

v2:
- Added CONFIG_OPTEE_TZDRAM_BASE instead of #ifndef OPTEE_TZDRAM_BASE
  as an error. - Tom Rini

- Added Tested-by: Peng Fan <peng.fan@nxp.com> - as indicated

- Added better explanation text to patch 6/9
  "tools: mkimage: add optee image type"

- Fixed some checkpatch warnings in optee.c

v1:
This series adds a new OPTEE bootable image type to u-boot, which is
directly bootable with the bootm command.

There is already a TEE image type but, in this case the TEE firmware is
loaded into RAM, jumped into and then back out of. This image type is a
directly bootable image as described here :
http://mrvan.github.io/optee-imx6ul

Instead of reusing the Linux bootable image type instead a new image type
is defined, which allows us to perform additional image verification, prior
to handing off control via bootm.

OPTEE images get linked to a specific address at compile time and must be
loaded to this address too. This series extends out mkimage with a new
image type that allows the OPTEE binary link location to be validated
against CONFIG_OPTEE_TZDRAM_BASE and CONFIG_OPTEE_TZDRAM_SIZE respectively
prior to proceeding through the bootm phase.

Once applied you can generate a bootable OPTEE image like this

mkimage -A arm -T optee -C none -d ./out/arm-plat-imx/core/tee.bin uTee.optee

That image can then be booted directly by bootm. bootm will verify the
header contents of the OPTEE binary against the DRAM area carved out in
u-boot. If the defined DRAM area does not match the link address specified
we refuse to boot.

Kever - I'd like to suggest that your OPTEE SPL image takes a different
image type IH_TYPE_OPTEE_SPL ? to indicate the different behavior your
image type has versus a directly bootable bootm image.

Bryan O'Donoghue (9):
  optee: Add lib entries for sharing OPTEE code across ports
  optee: Add CONFIG_OPTEE_TZDRAM_SIZE
  optee: Add CONFIG_OPTEE_TZDRAM_BASE
  optee: Add optee_image_get_entry_point()
  optee: Add optee_image_get_load_addr()
  tools: mkimage: add optee image type
  optee: Add optee_verify_bootm_image()
  optee: Improve error printout
  bootm: optee: Add mechanism to validate an OPTEE image before boot

 common/bootm.c        | 11 ++++++++-
 common/image.c        |  1 +
 include/image.h       |  1 +
 include/tee/optee.h   | 41 ++++++++++++++++++++++++++++++++
 lib/Kconfig           |  1 +
 lib/Makefile          |  1 +
 lib/optee/Kconfig     | 24 +++++++++++++++++++
 lib/optee/Makefile    |  7 ++++++
 lib/optee/optee.c     | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/default_image.c | 25 ++++++++++++++-----
 10 files changed, 171 insertions(+), 7 deletions(-)
 create mode 100644 lib/optee/Kconfig
 create mode 100644 lib/optee/Makefile
 create mode 100644 lib/optee/optee.c

-- 
2.7.4

[U-Boot] [PATCH v2 1/9] optee: Add lib entries for sharing OPTEE code across ports

[Bryan O'Donoghue]

This patch adds code to lib to enable sharing of useful OPTEE code between
board-ports and architectures. The code on lib/optee/optee.c comes from the
TI omap2 port. Eventually the OMAP2 code will be patched to include the
shared code. The intention here is to add more useful OPTEE specific code
as more functionality gets added.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Harinarayan Bhatta <harinarayan@ti.com>
Cc: Andrew F. Davis <afd@ti.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Cc: Peng Fan <peng.fan@nxp.com>
Tested-by: Peng Fan <peng.fan@nxp.com>
---
 include/tee/optee.h | 16 ++++++++++++++++
 lib/Kconfig         |  1 +
 lib/Makefile        |  1 +
 lib/optee/Kconfig   |  8 ++++++++
 lib/optee/Makefile  |  7 +++++++
 lib/optee/optee.c   | 37 +++++++++++++++++++++++++++++++++++++
 6 files changed, 70 insertions(+)
 create mode 100644 lib/optee/Kconfig
 create mode 100644 lib/optee/Makefile
 create mode 100644 lib/optee/optee.c

diff --git a/include/tee/optee.h b/include/tee/optee.h
index 9ab0d08..8943afb 100644
--- a/include/tee/optee.h
+++ b/include/tee/optee.h
@@ -10,6 +10,8 @@
 #ifndef	_OPTEE_H
 #define _OPTEE_H
 
+#include <linux/errno.h>
+
 #define OPTEE_MAGIC             0x4554504f
 #define OPTEE_VERSION           1
 #define OPTEE_ARCH_ARM32        0
@@ -27,4 +29,18 @@ struct optee_header {
 	uint32_t paged_size;
 };
 
+#if defined(CONFIG_OPTEE)
+int optee_verify_image(struct optee_header *hdr, unsigned long tzdram_start,
+		       unsigned long tzdram_len, unsigned long image_len);
+#else
+static inline int optee_verify_image(struct optee_header *hdr,
+				     unsigned long tzdram_start,
+				     unsigned long tzdram_len,
+				     unsigned long image_len)
+{
+	return -EPERM;
+}
+
+#endif
+
 #endif /* _OPTEE_H */
diff --git a/lib/Kconfig b/lib/Kconfig
index 00ac650..2077f9c 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -288,5 +288,6 @@ endmenu
 
 source lib/efi/Kconfig
 source lib/efi_loader/Kconfig
+source lib/optee/Kconfig
 
 endmenu
diff --git a/lib/Makefile b/lib/Makefile
index 8cd779f..46813b6 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_FIT) += libfdt/
 obj-$(CONFIG_OF_LIVE) += of_live.o
 obj-$(CONFIG_CMD_DHRYSTONE) += dhry/
 obj-$(CONFIG_ARCH_AT91) += at91/
+obj-$(CONFIG_OPTEE) += optee/
 
 obj-$(CONFIG_AES) += aes.o
 obj-y += charset.o
diff --git a/lib/optee/Kconfig b/lib/optee/Kconfig
new file mode 100644
index 0000000..2e406fe
--- /dev/null
+++ b/lib/optee/Kconfig
@@ -0,0 +1,8 @@
+config OPTEE
+	bool "Support OPTEE images"
+	help
+	  U-Boot can be configured to boot OPTEE images.
+	  Selecting this option will enable shared OPTEE library code and
+          enable an OPTEE specific bootm command that will perform additional
+          OPTEE specific checks before booting an OPTEE image created with
+          mkimage.
diff --git a/lib/optee/Makefile b/lib/optee/Makefile
new file mode 100644
index 0000000..03e832f
--- /dev/null
+++ b/lib/optee/Makefile
@@ -0,0 +1,7 @@
+#
+# (C) Copyright 2017 Linaro
+#
+# SPDX-License-Identifier:	GPL-2.0+
+#
+
+obj-$(CONFIG_OPTEE) += optee.o
diff --git a/lib/optee/optee.c b/lib/optee/optee.c
new file mode 100644
index 0000000..64ceacd
--- /dev/null
+++ b/lib/optee/optee.c
@@ -0,0 +1,37 @@
+/*
+ * Copyright (C) 2017 Linaro
+ * Bryan O'Donoghue <bryan.odonoghue@linaro.org>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <tee/optee.h>
+
+#define optee_hdr_err_msg "OPTEE verification error tzdram 0x%08lx-0x%08lx " \
+			   "header lo=0x%08x hi=0x%08x size=0x%08x\n"
+
+int optee_verify_image(struct optee_header *hdr, unsigned long tzdram_start,
+		       unsigned long tzdram_len, unsigned long image_len)
+{
+	unsigned long tzdram_end = tzdram_start + tzdram_len;
+	uint32_t tee_file_size;
+
+	tee_file_size = hdr->init_size + hdr->paged_size +
+			sizeof(struct optee_header);
+
+	if (hdr->magic != OPTEE_MAGIC ||
+	    hdr->version != OPTEE_VERSION ||
+	    hdr->init_load_addr_hi > tzdram_end ||
+	    hdr->init_load_addr_lo < tzdram_start ||
+	    tee_file_size > tzdram_len ||
+	    tee_file_size != image_len ||
+	    (hdr->init_load_addr_lo + tee_file_size) > tzdram_end) {
+		printf(optee_hdr_err_msg, tzdram_start, tzdram_end,
+		       hdr->init_load_addr_lo, hdr->init_load_addr_hi,
+		       tee_file_size);
+		return -EINVAL;
+	}
+
+	return 0;
+}
-- 
2.7.4

[U-Boot] [PATCH v2 2/9] optee: Add CONFIG_OPTEE_TZDRAM_SIZE

[Bryan O'Donoghue]

OPTEE is currently linked to a specific area of memory called the TrustZone
DRAM. This patch adds a CONFIG entry for the default size of TrustZone DRAM
that a board-port can over-ride. The region that U-Boot sets aside for the
OPTEE run-time should be verified before attempting to hand off to the
OPTEE run-time. Each board-port should carefully ensure that the TZDRAM
size specified in the OPTEE build and the TZDRAM size specified in U-Boot
match-up.

Further patches will use TZDRAM size with other defines and variables to
carry out a degree of automated verification in U-Boot prior to trying to
boot an OPTEE image.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Harinarayan Bhatta <harinarayan@ti.com>
Cc: Andrew F. Davis <afd@ti.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Cc: Peng Fan <peng.fan@nxp.com>
Tested-by: Peng Fan <peng.fan@nxp.com>
---
 lib/optee/Kconfig | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/lib/optee/Kconfig b/lib/optee/Kconfig
index 2e406fe..41c0ab7 100644
--- a/lib/optee/Kconfig
+++ b/lib/optee/Kconfig
@@ -6,3 +6,11 @@ config OPTEE
           enable an OPTEE specific bootm command that will perform additional
           OPTEE specific checks before booting an OPTEE image created with
           mkimage.
+
+config OPTEE_TZDRAM_SIZE
+	hex "Amount of Trust-Zone RAM for the OPTEE image"
+	depends on OPTEE
+	default 0x3000000
+	help
+	  The size of pre-allocated Trust Zone DRAM to allocate for the OPTEE
+	  runtime.
-- 
2.7.4

[U-Boot] [PATCH v2 3/9] optee: Add CONFIG_OPTEE_TZDRAM_BASE

[Bryan O'Donoghue]

OPTEE is currently linked to a specific area of memory called the TrustZone
DRAM. This patch adds a CONFIG entry for the default address of TrustZone
DRAM that a board-port can over-ride. The region that U-Boot sets aside for
the OPTEE run-time should be verified before attempting to hand off to the
OPTEE run-time. Each board-port should carefully ensure that the TZDRAM
address specified in the OPTEE build and the TZDRAM address specified in
U-Boot match-up.

Further patches will use TZDRAM address with other defines and variables to
carry out a degree of automated verification in U-Boot prior to trying to
boot an OPTEE image.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Harinarayan Bhatta <harinarayan@ti.com>
Cc: Andrew F. Davis <afd@ti.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Cc: Peng Fan <peng.fan@nxp.com>
---
 lib/optee/Kconfig | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/lib/optee/Kconfig b/lib/optee/Kconfig
index 41c0ab7..a3b7332 100644
--- a/lib/optee/Kconfig
+++ b/lib/optee/Kconfig
@@ -14,3 +14,11 @@ config OPTEE_TZDRAM_SIZE
 	help
 	  The size of pre-allocated Trust Zone DRAM to allocate for the OPTEE
 	  runtime.
+
+config OPTEE_TZDRAM_BASE
+	hex "Base address of Trust-Zone RAM for the OPTEE image"
+	depends on OPTEE
+	default 0x9d000000
+	help
+	  The base address of pre-allocated Trust Zone DRAM for
+	  the OPTEE runtime.
-- 
2.7.4

[U-Boot] [PATCH v2 4/9] optee: Add optee_image_get_entry_point()

[Bryan O'Donoghue]

Add a helper function for extracting the least significant 32 bits from the
OPTEE entry point address, which will be good enough to load OPTEE binaries
up to (2^32)-1 bytes.

We may need to extend this out later on but for now (2^32)-1 should be
fine.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Harinarayan Bhatta <harinarayan@ti.com>
Cc: Andrew F. Davis <afd@ti.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Cc: Peng Fan <peng.fan@nxp.com>
Tested-by: Peng Fan <peng.fan@nxp.com>
---
 include/tee/optee.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/tee/optee.h b/include/tee/optee.h
index 8943afb..eb328d3 100644
--- a/include/tee/optee.h
+++ b/include/tee/optee.h
@@ -29,6 +29,13 @@ struct optee_header {
 	uint32_t paged_size;
 };
 
+static inline uint32_t optee_image_get_entry_point(const image_header_t *hdr)
+{
+	struct optee_header *optee_hdr = (struct optee_header *)(hdr + 1);
+
+	return optee_hdr->init_load_addr_lo;
+}
+
 #if defined(CONFIG_OPTEE)
 int optee_verify_image(struct optee_header *hdr, unsigned long tzdram_start,
 		       unsigned long tzdram_len, unsigned long image_len);
-- 
2.7.4

[U-Boot] [PATCH v2 5/9] optee: Add optee_image_get_load_addr()

[Bryan O'Donoghue]

This patch adds optee_image_get_load_addr() a helper function used to
calculate the load-address of an OPTEE image based on the lower
entry-point address given in the OPTEE header.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Harinarayan Bhatta <harinarayan@ti.com>
Cc: Andrew F. Davis <afd@ti.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Cc: Peng Fan <peng.fan@nxp.com>
Tested-by: Peng Fan <peng.fan@nxp.com>
---
 include/tee/optee.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/tee/optee.h b/include/tee/optee.h
index eb328d3..e782cb0 100644
--- a/include/tee/optee.h
+++ b/include/tee/optee.h
@@ -36,6 +36,11 @@ static inline uint32_t optee_image_get_entry_point(const image_header_t *hdr)
 	return optee_hdr->init_load_addr_lo;
 }
 
+static inline uint32_t optee_image_get_load_addr(const image_header_t *hdr)
+{
+	return optee_image_get_entry_point(hdr) - sizeof(struct optee_header);
+}
+
 #if defined(CONFIG_OPTEE)
 int optee_verify_image(struct optee_header *hdr, unsigned long tzdram_start,
 		       unsigned long tzdram_len, unsigned long image_len);
-- 
2.7.4

[U-Boot] [PATCH v2 6/9] tools: mkimage: add optee image type

[Bryan O'Donoghue]

This patch adds support for bootable OPTEE images to mkimage. Currently
there is a (Trusted Execution Environment) TEE image type, the TEE image
type is installed to a memory location with u-boot continuing to own the
boot process whereas the OPTEE image type defined here is a bootable image,
which typically wants to live at a defined location in memory. Defining a
new image type allows us to pull out the load address and entry point
defined in the OPTEE header and having a separate image type lays the
foundation for a subsequent patch to validate the OPTEE memory defined in a
board-port matches the link location specified in the OPTEE bootable
image.

example usage:

mkimage -A arm -T optee -C none -d ./out/arm-plat-imx/core/tee.bin
uTee.optee

making a separate image type means you don't need to specify things like
entry point and load address as you would if you were defining the OPTEE
image as a linux image.

mkimage -A arm -O linux -C none -a 0x9c0fffe4 -e 0x9c100000 -d
./out/arm-plat-imx/core/tee.bin uTee

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Harinarayan Bhatta <harinarayan@ti.com>
Cc: Andrew F. Davis <afd@ti.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Cc: Peng Fan <peng.fan@nxp.com>
Tested-by: Peng Fan <peng.fan@nxp.com>
---
 common/image.c        |  1 +
 include/image.h       |  1 +
 tools/default_image.c | 25 +++++++++++++++++++------
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/common/image.c b/common/image.c
index e9609cd..14e738b 100644
--- a/common/image.c
+++ b/common/image.c
@@ -161,6 +161,7 @@ static const table_entry_t uimage_type[] = {
 	{       IH_TYPE_TEE,        "tee",        "Trusted Execution Environment Image",},
 	{	IH_TYPE_FIRMWARE_IVT, "firmware_ivt", "Firmware with HABv4 IVT" },
 	{       IH_TYPE_PMMC,        "pmmc",        "TI Power Management Micro-Controller Firmware",},
+	{       IH_TYPE_OPTEE,       "optee",     "OPTEE Boot Image",},
 	{	-1,		    "",		  "",			},
 };
 
diff --git a/include/image.h b/include/image.h
index a128a62..9175624 100644
--- a/include/image.h
+++ b/include/image.h
@@ -271,6 +271,7 @@ enum {
 	IH_TYPE_TEE,            /* Trusted Execution Environment OS Image */
 	IH_TYPE_FIRMWARE_IVT,		/* Firmware Image with HABv4 IVT */
 	IH_TYPE_PMMC,            /* TI Power Management Micro-Controller Firmware */
+	IH_TYPE_OPTEE,			/* OPTEE Boot Image */
 
 	IH_TYPE_COUNT,			/* Number of image types */
 };
diff --git a/tools/default_image.c b/tools/default_image.c
index 4e5568e..5653933 100644
--- a/tools/default_image.c
+++ b/tools/default_image.c
@@ -18,6 +18,7 @@
 #include "mkimage.h"
 
 #include <image.h>
+#include <tee/optee.h>
 #include <u-boot/crc.h>
 
 static image_header_t header;
@@ -25,7 +26,8 @@ static image_header_t header;
 static int image_check_image_types(uint8_t type)
 {
 	if (((type > IH_TYPE_INVALID) && (type < IH_TYPE_FLATDT)) ||
-	    (type == IH_TYPE_KERNEL_NOLOAD) || (type == IH_TYPE_FIRMWARE_IVT))
+	    (type == IH_TYPE_KERNEL_NOLOAD) || (type == IH_TYPE_FIRMWARE_IVT) ||
+	    (type == IH_TYPE_OPTEE))
 		return EXIT_SUCCESS;
 	else
 		return EXIT_FAILURE;
@@ -90,6 +92,8 @@ static void image_set_header(void *ptr, struct stat *sbuf, int ifd,
 	uint32_t checksum;
 	time_t time;
 	uint32_t imagesize;
+	uint32_t ep;
+	uint32_t addr;
 
 	image_header_t * hdr = (image_header_t *)ptr;
 
@@ -99,18 +103,27 @@ static void image_set_header(void *ptr, struct stat *sbuf, int ifd,
 			sbuf->st_size - sizeof(image_header_t));
 
 	time = imagetool_get_source_date(params, sbuf->st_mtime);
-	if (params->type == IH_TYPE_FIRMWARE_IVT)
+	ep = params->ep;
+	addr = params->addr;
+	imagesize = sbuf->st_size - sizeof(image_header_t);
+
+	switch (params->type) {
+	case IH_TYPE_FIRMWARE_IVT:
 		/* Add size of CSF minus IVT */
 		imagesize = sbuf->st_size - sizeof(image_header_t) + 0x1FE0;
-	else
-		imagesize = sbuf->st_size - sizeof(image_header_t);
+		break;
+	case IH_TYPE_OPTEE:
+		addr = optee_image_get_load_addr(hdr);
+		ep = optee_image_get_entry_point(hdr);
+		break;
+	}
 
 	/* Build new header */
 	image_set_magic(hdr, IH_MAGIC);
 	image_set_time(hdr, time);
 	image_set_size(hdr, imagesize);
-	image_set_load(hdr, params->addr);
-	image_set_ep(hdr, params->ep);
+	image_set_load(hdr, addr);
+	image_set_ep(hdr, ep);
 	image_set_dcrc(hdr, checksum);
 	image_set_os(hdr, params->os);
 	image_set_arch(hdr, params->arch);
-- 
2.7.4

[U-Boot] [PATCH v2 7/9] optee: Add optee_verify_bootm_image()

[Bryan O'Donoghue]

This patch adds optee_verify_bootm_image() which will be subsequently used
to verify the parameters encoded in the OPTEE header match the memory
allocated to the OPTEE region, OPTEE header magic and version prior to
handing off control to the OPTEE image.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Harinarayan Bhatta <harinarayan@ti.com>
Cc: Andrew F. Davis <afd@ti.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Cc: Peng Fan <peng.fan@nxp.com>
Tested-by: Peng Fan <peng.fan@nxp.com>
---
 include/tee/optee.h | 13 +++++++++++++
 lib/optee/optee.c   | 34 ++++++++++++++++++++++++++++++----
 2 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/include/tee/optee.h b/include/tee/optee.h
index e782cb0..4b9e94c 100644
--- a/include/tee/optee.h
+++ b/include/tee/optee.h
@@ -55,4 +55,17 @@ static inline int optee_verify_image(struct optee_header *hdr,
 
 #endif
 
+#if defined(CONFIG_OPTEE)
+int optee_verify_bootm_image(unsigned long image_addr,
+			     unsigned long image_load_addr,
+			     unsigned long image_len);
+#else
+static inline int optee_verify_bootm_image(unsigned long image_addr,
+					   unsigned long image_load_addr,
+					   unsigned long image_len)
+{
+	return -EPERM;
+}
+#endif
+
 #endif /* _OPTEE_H */
diff --git a/lib/optee/optee.c b/lib/optee/optee.c
index 64ceacd..e28627d 100644
--- a/lib/optee/optee.c
+++ b/lib/optee/optee.c
@@ -9,7 +9,8 @@
 #include <tee/optee.h>
 
 #define optee_hdr_err_msg "OPTEE verification error tzdram 0x%08lx-0x%08lx " \
-			   "header lo=0x%08x hi=0x%08x size=0x%08x\n"
+			  "header 0x%08x-0x%08x size=0x%08lx arch=0x%08x" \
+			  "uimage params 0x%08lx-0x%08lx\n"
 
 int optee_verify_image(struct optee_header *hdr, unsigned long tzdram_start,
 		       unsigned long tzdram_len, unsigned long image_len)
@@ -27,11 +28,36 @@ int optee_verify_image(struct optee_header *hdr, unsigned long tzdram_start,
 	    tee_file_size > tzdram_len ||
 	    tee_file_size != image_len ||
 	    (hdr->init_load_addr_lo + tee_file_size) > tzdram_end) {
-		printf(optee_hdr_err_msg, tzdram_start, tzdram_end,
-		       hdr->init_load_addr_lo, hdr->init_load_addr_hi,
-		       tee_file_size);
 		return -EINVAL;
 	}
 
 	return 0;
 }
+
+int optee_verify_bootm_image(unsigned long image_addr,
+			     unsigned long image_load_addr,
+			     unsigned long image_len)
+{
+	struct optee_header *hdr = (struct optee_header *)image_addr;
+	unsigned long tzdram_start = CONFIG_OPTEE_TZDRAM_BASE;
+	unsigned long tzdram_len = CONFIG_OPTEE_TZDRAM_SIZE;
+
+	int ret;
+
+	ret = optee_verify_image(hdr, tzdram_start, tzdram_len, image_len);
+	if (ret)
+		goto error;
+
+	if (image_load_addr + sizeof(*hdr) != hdr->init_load_addr_lo) {
+		ret = -EINVAL;
+		goto error;
+	}
+
+	return ret;
+error:
+	printf(optee_hdr_err_msg, tzdram_start, tzdram_start + tzdram_len,
+	       hdr->init_load_addr_lo, hdr->init_load_addr_hi, image_len,
+	       hdr->arch, image_load_addr, image_load_addr + image_len);
+
+	return ret;
+}
-- 
2.7.4

[U-Boot] [PATCH v2 8/9] optee: Improve error printout

[Bryan O'Donoghue]

When encountering an error in OPTEE verification print out the address of
the header and image.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Harinarayan Bhatta <harinarayan@ti.com>
Cc: Andrew F. Davis <afd@ti.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Cc: Peng Fan <peng.fan@nxp.com>
Tested-by: Peng Fan <peng.fan@nxp.com>
---
 lib/optee/optee.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/lib/optee/optee.c b/lib/optee/optee.c
index e28627d..78a15e8 100644
--- a/lib/optee/optee.c
+++ b/lib/optee/optee.c
@@ -8,9 +8,11 @@
 #include <common.h>
 #include <tee/optee.h>
 
-#define optee_hdr_err_msg "OPTEE verification error tzdram 0x%08lx-0x%08lx " \
-			  "header 0x%08x-0x%08x size=0x%08lx arch=0x%08x" \
-			  "uimage params 0x%08lx-0x%08lx\n"
+#define optee_hdr_err_msg \
+	"OPTEE verification error:" \
+	"\n\thdr=%p image=0x%08lx magic=0x%08x tzdram 0x%08lx-0x%08lx " \
+	"\n\theader lo=0x%08x hi=0x%08x size=0x%08lx arch=0x%08x" \
+	"\n\tuimage params 0x%08lx-0x%08lx\n"
 
 int optee_verify_image(struct optee_header *hdr, unsigned long tzdram_start,
 		       unsigned long tzdram_len, unsigned long image_len)
@@ -55,9 +57,10 @@ int optee_verify_bootm_image(unsigned long image_addr,
 
 	return ret;
 error:
-	printf(optee_hdr_err_msg, tzdram_start, tzdram_start + tzdram_len,
-	       hdr->init_load_addr_lo, hdr->init_load_addr_hi, image_len,
-	       hdr->arch, image_load_addr, image_load_addr + image_len);
+	printf(optee_hdr_err_msg, hdr, image_addr, hdr->magic, tzdram_start,
+	       tzdram_start + tzdram_len, hdr->init_load_addr_lo,
+	       hdr->init_load_addr_hi, image_len, hdr->arch, image_load_addr,
+	       image_load_addr + image_len);
 
 	return ret;
 }
-- 
2.7.4

[U-Boot] [PATCH v2 9/9] bootm: optee: Add mechanism to validate an OPTEE image before boot

[Bryan O'Donoghue]

This patch makes it possible to verify the contents and location of an
OPTEE image in DRAM prior to handing off control to that image. If image
verification fails we won't try to boot any further.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Harinarayan Bhatta <harinarayan@ti.com>
Cc: Andrew F. Davis <afd@ti.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Cc: Peng Fan <peng.fan@nxp.com>
Tested-by: Peng Fan <peng.fan@nxp.com>
---
 common/bootm.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/common/bootm.c b/common/bootm.c
index adb1213..d528844 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -19,6 +19,7 @@
 #include <lzma/LzmaTypes.h>
 #include <lzma/LzmaDec.h>
 #include <lzma/LzmaTools.h>
+#include <tee/optee.h>
 #if defined(CONFIG_CMD_USB)
 #include <usb.h>
 #endif
@@ -201,6 +202,12 @@ static int bootm_find_os(cmd_tbl_t *cmdtp, int flag, int argc,
 	if (images.os.type == IH_TYPE_KERNEL_NOLOAD) {
 		images.os.load = images.os.image_start;
 		images.ep += images.os.load;
+	} else if (images.os.type == IH_TYPE_OPTEE) {
+		ret = optee_verify_bootm_image(images.os.image_start,
+					       images.os.load,
+					       images.os.image_len);
+		if (ret)
+			return ret;
 	}
 
 	images.os.start = map_to_sysmem(os_hdr);
@@ -275,7 +282,8 @@ static int bootm_find_other(cmd_tbl_t *cmdtp, int flag, int argc,
 {
 	if (((images.os.type == IH_TYPE_KERNEL) ||
 	     (images.os.type == IH_TYPE_KERNEL_NOLOAD) ||
-	     (images.os.type == IH_TYPE_MULTI)) &&
+	     (images.os.type == IH_TYPE_MULTI) ||
+	     (images.os.type == IH_TYPE_OPTEE)) &&
 	    (images.os.os == IH_OS_LINUX ||
 		 images.os.os == IH_OS_VXWORKS))
 		return bootm_find_images(flag, argc, argv);
@@ -827,6 +835,7 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
 		switch (image_get_type(hdr)) {
 		case IH_TYPE_KERNEL:
 		case IH_TYPE_KERNEL_NOLOAD:
+		case IH_TYPE_OPTEE:
 			*os_data = image_get_data(hdr);
 			*os_len = image_get_data_size(hdr);
 			break;
-- 
2.7.4

[U-Boot] [PATCH v2 8/9] optee: Improve error printout

[Andrew F. Davis]

On 01/19/2018 01:43 PM, Bryan O'Donoghue wrote:
> When encountering an error in OPTEE verification print out the address of
> the header and image.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Cc: Harinarayan Bhatta <harinarayan@ti.com>
> Cc: Andrew F. Davis <afd@ti.com>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Kever Yang <kever.yang@rock-chips.com>
> Cc: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Tested-by: Peng Fan <peng.fan@nxp.com>
> ---
>  lib/optee/optee.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/optee/optee.c b/lib/optee/optee.c
> index e28627d..78a15e8 100644
> --- a/lib/optee/optee.c
> +++ b/lib/optee/optee.c
> @@ -8,9 +8,11 @@
>  #include <common.h>
>  #include <tee/optee.h>
>  
> -#define optee_hdr_err_msg "OPTEE verification error tzdram 0x%08lx-0x%08lx " \
> -			  "header 0x%08x-0x%08x size=0x%08lx arch=0x%08x" \
> -			  "uimage params 0x%08lx-0x%08lx\n"
> +#define optee_hdr_err_msg \
> +	"OPTEE verification error:" \
> +	"\n\thdr=%p image=0x%08lx magic=0x%08x tzdram 0x%08lx-0x%08lx " \
> +	"\n\theader lo=0x%08x hi=0x%08x size=0x%08lx arch=0x%08x" \
> +	"\n\tuimage params 0x%08lx-0x%08lx\n"
>  
>  int optee_verify_image(struct optee_header *hdr, unsigned long tzdram_start,
>  		       unsigned long tzdram_len, unsigned long image_len)
> @@ -55,9 +57,10 @@ int optee_verify_bootm_image(unsigned long image_addr,
>  
>  	return ret;
>  error:
> -	printf(optee_hdr_err_msg, tzdram_start, tzdram_start + tzdram_len,
> -	       hdr->init_load_addr_lo, hdr->init_load_addr_hi, image_len,
> -	       hdr->arch, image_load_addr, image_load_addr + image_len);
> +	printf(optee_hdr_err_msg, hdr, image_addr, hdr->magic, tzdram_start,
> +	       tzdram_start + tzdram_len, hdr->init_load_addr_lo,
> +	       hdr->init_load_addr_hi, image_len, hdr->arch, image_load_addr,
> +	       image_load_addr + image_len);
>  

This all can be squashed up into the patches that introduce all this stuff.

>  	return ret;
>  }
> 

[U-Boot] [PATCH v2 6/9] tools: mkimage: add optee image type

[Andrew F. Davis]

On 01/19/2018 01:43 PM, Bryan O'Donoghue wrote:
> This patch adds support for bootable OPTEE images to mkimage. Currently
> there is a (Trusted Execution Environment) TEE image type, the TEE image
> type is installed to a memory location with u-boot continuing to own the
> boot process whereas the OPTEE image type defined here is a bootable image,
> which typically wants to live at a defined location in memory. Defining a
> new image type allows us to pull out the load address and entry point
> defined in the OPTEE header and having a separate image type lays the
> foundation for a subsequent patch to validate the OPTEE memory defined in a
> board-port matches the link location specified in the OPTEE bootable
> image.
> 
> example usage:
> 
> mkimage -A arm -T optee -C none -d ./out/arm-plat-imx/core/tee.bin
> uTee.optee
> 
> making a separate image type means you don't need to specify things like
> entry point and load address as you would if you were defining the OPTEE
> image as a linux image.
> 

I'm still not getting the reasoning for this all, you can have the load
address checks and relocation stuff inside your loadable handler:

U_BOOT_FIT_LOADABLE_HANDLER(IH_TYPE_TEE, board_tee_image_process);

Then define 'board_tee_image_process' to do whatever you want to your image.

> mkimage -A arm -O linux -C none -a 0x9c0fffe4 -e 0x9c100000 -d
> ./out/arm-plat-imx/core/tee.bin uTee
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Cc: Harinarayan Bhatta <harinarayan@ti.com>
> Cc: Andrew F. Davis <afd@ti.com>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Kever Yang <kever.yang@rock-chips.com>
> Cc: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Tested-by: Peng Fan <peng.fan@nxp.com>
> ---
>  common/image.c        |  1 +
>  include/image.h       |  1 +
>  tools/default_image.c | 25 +++++++++++++++++++------
>  3 files changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/common/image.c b/common/image.c
> index e9609cd..14e738b 100644
> --- a/common/image.c
> +++ b/common/image.c
> @@ -161,6 +161,7 @@ static const table_entry_t uimage_type[] = {
>  	{       IH_TYPE_TEE,        "tee",        "Trusted Execution Environment Image",},
>  	{	IH_TYPE_FIRMWARE_IVT, "firmware_ivt", "Firmware with HABv4 IVT" },
>  	{       IH_TYPE_PMMC,        "pmmc",        "TI Power Management Micro-Controller Firmware",},
> +	{       IH_TYPE_OPTEE,       "optee",     "OPTEE Boot Image",},
>  	{	-1,		    "",		  "",			},
>  };
>  
> diff --git a/include/image.h b/include/image.h
> index a128a62..9175624 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -271,6 +271,7 @@ enum {
>  	IH_TYPE_TEE,            /* Trusted Execution Environment OS Image */
>  	IH_TYPE_FIRMWARE_IVT,		/* Firmware Image with HABv4 IVT */
>  	IH_TYPE_PMMC,            /* TI Power Management Micro-Controller Firmware */
> +	IH_TYPE_OPTEE,			/* OPTEE Boot Image */
>  
>  	IH_TYPE_COUNT,			/* Number of image types */
>  };
> diff --git a/tools/default_image.c b/tools/default_image.c
> index 4e5568e..5653933 100644
> --- a/tools/default_image.c
> +++ b/tools/default_image.c
> @@ -18,6 +18,7 @@
>  #include "mkimage.h"
>  
>  #include <image.h>
> +#include <tee/optee.h>
>  #include <u-boot/crc.h>
>  
>  static image_header_t header;
> @@ -25,7 +26,8 @@ static image_header_t header;
>  static int image_check_image_types(uint8_t type)
>  {
>  	if (((type > IH_TYPE_INVALID) && (type < IH_TYPE_FLATDT)) ||
> -	    (type == IH_TYPE_KERNEL_NOLOAD) || (type == IH_TYPE_FIRMWARE_IVT))
> +	    (type == IH_TYPE_KERNEL_NOLOAD) || (type == IH_TYPE_FIRMWARE_IVT) ||
> +	    (type == IH_TYPE_OPTEE))
>  		return EXIT_SUCCESS;
>  	else
>  		return EXIT_FAILURE;
> @@ -90,6 +92,8 @@ static void image_set_header(void *ptr, struct stat *sbuf, int ifd,
>  	uint32_t checksum;
>  	time_t time;
>  	uint32_t imagesize;
> +	uint32_t ep;
> +	uint32_t addr;
>  
>  	image_header_t * hdr = (image_header_t *)ptr;
>  
> @@ -99,18 +103,27 @@ static void image_set_header(void *ptr, struct stat *sbuf, int ifd,
>  			sbuf->st_size - sizeof(image_header_t));
>  
>  	time = imagetool_get_source_date(params, sbuf->st_mtime);
> -	if (params->type == IH_TYPE_FIRMWARE_IVT)
> +	ep = params->ep;
> +	addr = params->addr;
> +	imagesize = sbuf->st_size - sizeof(image_header_t);
> +
> +	switch (params->type) {
> +	case IH_TYPE_FIRMWARE_IVT:
>  		/* Add size of CSF minus IVT */
>  		imagesize = sbuf->st_size - sizeof(image_header_t) + 0x1FE0;
> -	else
> -		imagesize = sbuf->st_size - sizeof(image_header_t);
> +		break;
> +	case IH_TYPE_OPTEE:
> +		addr = optee_image_get_load_addr(hdr);
> +		ep = optee_image_get_entry_point(hdr);
> +		break;
> +	}
>  
>  	/* Build new header */
>  	image_set_magic(hdr, IH_MAGIC);
>  	image_set_time(hdr, time);
>  	image_set_size(hdr, imagesize);
> -	image_set_load(hdr, params->addr);
> -	image_set_ep(hdr, params->ep);
> +	image_set_load(hdr, addr);
> +	image_set_ep(hdr, ep);
>  	image_set_dcrc(hdr, checksum);
>  	image_set_os(hdr, params->os);
>  	image_set_arch(hdr, params->arch);
> 

[U-Boot] [PATCH v2 6/9] tools: mkimage: add optee image type

[Bryan O'Donoghue]

On 19/01/18 20:14, Andrew F. Davis wrote:
> On 01/19/2018 01:43 PM, Bryan O'Donoghue wrote:
>> This patch adds support for bootable OPTEE images to mkimage. Currently
>> there is a (Trusted Execution Environment) TEE image type, the TEE image
>> type is installed to a memory location with u-boot continuing to own the
>> boot process whereas the OPTEE image type defined here is a bootable image,
>> which typically wants to live at a defined location in memory. Defining a
>> new image type allows us to pull out the load address and entry point
>> defined in the OPTEE header and having a separate image type lays the
>> foundation for a subsequent patch to validate the OPTEE memory defined in a
>> board-port matches the link location specified in the OPTEE bootable
>> image.
>>
>> example usage:
>>

>> uTee.optee
>>
>> making a separate image type means you don't need to specify things like
>> entry point and load address as you would if you were defining the OPTEE
>> image as a linux image.
>>
> 
> I'm still not getting the reasoning for this all, you can have the load
> address checks and relocation stuff inside your loadable handler:
> 
> U_BOOT_FIT_LOADABLE_HANDLER(IH_TYPE_TEE, board_tee_image_process
Hi Andrew,

As I understand it, that's a board-specific method, which wants to 
install a TEE (jump into a TEE and return to u-boot), whereas the aim 
with this patch-set is to chain-load and boot via TEE - OPTEE in this case.

I should make that clearer in the patch text description.

The example from Peng Fang

>> mkimage -A arm -O linux -C none -a 0x9c0fffe4 -e 0x9c100000 -d
>> ./out/arm-plat-imx/core/tee.bin uTee

is then simplified down to this

 >> mkimage -A arm -T optee -C none -d ./out/arm-plat-imx/core/tee.bin

and remove the requirement to pass load and entry point on the command line.

The boot-flow looks like this

BootROM -> u-boot {load kernel, dtb, optee} -> OPTEE -> Kernel

Whereas - as I understand it the flow on the TI examples you have is

BootROM -> u-boot {load kernel, dtb, TEE} -> TEE -> u-boot -> Kernel

Note: I do believe IH_TYPE_TEE is the right type to use for Kever's 
patch with the SPL - because again as I understand it - the model is

BootROM -> SPL -> Install TEE -> u-boot -> etc

---
bod

[U-Boot] [PATCH v2 8/9] optee: Improve error printout

[Bryan O'Donoghue]

On 19/01/18 20:08, Andrew F. Davis wrote:
> On 01/19/2018 01:43 PM, Bryan O'Donoghue wrote:
>> When encountering an error in OPTEE verification print out the address of
>> the header and image.

> 
> This all can be squashed up into the patches that introduce all this stuff.
> 

Sure np.

[U-Boot] [PATCH v2 6/9] tools: mkimage: add optee image type

[Andrew F. Davis]

On 01/19/2018 05:59 PM, Bryan O'Donoghue wrote:
> 
> 
> On 19/01/18 20:14, Andrew F. Davis wrote:
>> On 01/19/2018 01:43 PM, Bryan O'Donoghue wrote:
>>> This patch adds support for bootable OPTEE images to mkimage. Currently
>>> there is a (Trusted Execution Environment) TEE image type, the TEE image
>>> type is installed to a memory location with u-boot continuing to own the
>>> boot process whereas the OPTEE image type defined here is a bootable
>>> image,
>>> which typically wants to live at a defined location in memory.
>>> Defining a
>>> new image type allows us to pull out the load address and entry point
>>> defined in the OPTEE header and having a separate image type lays the
>>> foundation for a subsequent patch to validate the OPTEE memory
>>> defined in a
>>> board-port matches the link location specified in the OPTEE bootable
>>> image.
>>>
>>> example usage:
>>>
> 
>>> uTee.optee
>>>
>>> making a separate image type means you don't need to specify things like
>>> entry point and load address as you would if you were defining the OPTEE
>>> image as a linux image.
>>>
>>
>> I'm still not getting the reasoning for this all, you can have the load
>> address checks and relocation stuff inside your loadable handler:
>>
>> U_BOOT_FIT_LOADABLE_HANDLER(IH_TYPE_TEE, board_tee_image_process
> Hi Andrew,
> 
> As I understand it, that's a board-specific method, which wants to
> install a TEE (jump into a TEE and return to u-boot), whereas the aim
> with this patch-set is to chain-load and boot via TEE - OPTEE in this case.
> 

This is not board-specific, this is the flow all ARM boards I know of
use (except i.MX 6).

It is certainly possible to use the same flow on TI boards if we wanted,
we just don't as in my mind OP-TEE logically should jump back to the
non-secure side were it was called from so non-secure boot can continue.
Not act as another boot loader stage.

Is there some technical reason I am missing as to why you want to use
this alternate flow?

> I should make that clearer in the patch text description.
> 

That would be great.

> The example from Peng Fang
> 
>>> mkimage -A arm -O linux -C none -a 0x9c0fffe4 -e 0x9c100000 -d
>>> ./out/arm-plat-imx/core/tee.bin uTee
> 

I haven't used mkimage in a while, but how is this any different than
what we do with the kernel image? Why do we need to pull this info out
of the header when we don't do the same for Linux?

> is then simplified down to this
> 
>>> mkimage -A arm -T optee -C none -d ./out/arm-plat-imx/core/tee.bin
> > and remove the requirement to pass load and entry point on the command
> line.
> 

To me the save in this command (which should be handled automatically
during the build process), doesn't justify the additional complexity in
the boot flow.

> The boot-flow looks like this
> 
> BootROM -> u-boot {load kernel, dtb, optee} -> OPTEE -> Kernel
> 
> Whereas - as I understand it the flow on the TI examples you have is
> 
> BootROM -> u-boot {load kernel, dtb, TEE} -> TEE -> u-boot -> Kernel
> 


This is correct, we also on some platforms load additional firmware, so
returning to u-boot after each image is deployed is a must.

BootROM -> (u-boot  u-boot   u-boot  u-boot) -> kernel
             |      |  |     |   |      |
             v      |  v     |   v      |
             TEE ->    PMMC ->   Something else, etc..

You seem to be locking yourself in with your flow, as now TEE *must* be
the last item you load. What happens when you want to make secure calls
into OP-TEE from U-Boot? You won't be able to move it back in the
stages. (We are now doing this to handle some new Android secure-boot
requirements, so it is not that far-fetched)


> Note: I do believe IH_TYPE_TEE is the right type to use for Kever's
> patch with the SPL - because again as I understand it - the model is
> 
> BootROM -> SPL -> Install TEE -> u-boot -> etc
> 
> ---
> bod

[U-Boot] [PATCH v2 6/9] tools: mkimage: add optee image type

[Bryan O'Donoghue]

On 22/01/18 14:46, Andrew F. Davis wrote:

>> As I understand it, that's a board-specific method, which wants to
>> install a TEE (jump into a TEE and return to u-boot), whereas the aim
>> with this patch-set is to chain-load and boot via TEE - OPTEE in this case.
>>
> 
> This is not board-specific, this is the flow all ARM boards I know of
> use (except i.MX 6).

The OPTEE port I'm working with is i.MX 7, which chain-loads in this 
same way.

> Is there some technical reason I am missing as to why you want to use
> this alternate flow?

The reason is the upstream OPTEE port we are working with already uses 
this bootflow.

>> The example from Peng Fang
>>
>>>> mkimage -A arm -O linux -C none -a 0x9c0fffe4 -e 0x9c100000 -d
>>>> ./out/arm-plat-imx/core/tee.bin uTee
>>
> 
> I haven't used mkimage in a while, but how is this any different than
> what we do with the kernel image? Why do we need to pull this info out
> of the header when we don't do the same for Linux?

So for a kernel you are typically making a uImage of a compressed kernel 
image and therefore you have to pass load-address and entry point.

mkimage -A arm -O linux -T kernel -C none -a 0x80008000 -e 0x80008000 -n 
"Linux kernel" -d arch/arm/boot/zImage uImage

For the bootable OPTEE image case I'm proposing

1. A distinct image type
2. Based on that image type we validate the OPTEE header
    MAGIC, version, etc
3. Based on the OPTEE header we can validate the location the OPTEE
    binary gets loaded to.

Having a distinct image type makes it more robust.

>>>> mkimage -A arm -T optee -C none -d ./out/arm-plat-imx/core/tee.bin
>>> and remove the requirement to pass load and entry point on the command
>> line.
>>
> To me the save in this command (which should be handled automatically
> during the build process),

As above, it's about image generation, validation and load-address 
sanity checking.

I apologize for not making that clearer upfront - my bad, I'll attempt 
to flesh-out the patch descriptions to make the logic clearer.

---
bod