[U-Boot] [PATCH v4 00/12] Add new OPTEE bootm support to u-boot

[U-Boot] [PATCH v4 00/12] Add new OPTEE bootm support to u-boot

[Bryan O'Donoghue]

v4:
- New type "optee" renamed to "tee-bootable". We discussed making the
  namespace here more logical and obvious in another thread.

  Kever may or may not end up adding "tee-combo".

  This patchset will result in
  "tee" and "tee-bootable" being valid names. Since "tee" is an existing
  image type the name will be maintained. - Tom

- Added doc/README.trusted-execution-environment
  This gives a brief introduction on TEE plus some links to the spec and
  the op-tee website.

  In then lays out the difference between these two types
  "tee" (tee-standalone)
  "tee-bootable"

  - Bryan, Philipp

- Small change made to comment on existing TEE - Bryan

- Reworded the Kconfig option "OPTEE"
  Makes a little bit more sense to me re-reading now - Bryan

- Add patch to define CONFIG_OPTEE_LOAD_ADDR
  An upcoming set of patches for a board will make use of this define in an
  OPTEE context.

v3:

- Rework printout to be added at the end as opposed to churn over three
  separate patches - Andrew

- Reword patch 006 to better explain the thinking behind new image type
  - Andrew

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 (12):
  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()
  image: Update comment for IH_TYPE_TEE
  tools: mkimage: add tee-bootable image type
  doc: TEE: Add documentation describing TEE in u-boot
  optee: Add optee_verify_bootm_image()
  optee: Add error printout
  bootm: optee: Add mechanism to validate a bootable TEE image
  optee: Add CONFIG_OPTEE_LOAD_ADDR

 common/bootm.c                           |  11 ++-
 common/image.c                           |   1 +
 doc/README.trusted-execution-environment | 123 +++++++++++++++++++++++++++++++
 include/image.h                          |   3 +-
 include/tee/optee.h                      |  41 +++++++++++
 lib/Kconfig                              |   1 +
 lib/Makefile                             |   1 +
 lib/optee/Kconfig                        |  29 ++++++++
 lib/optee/Makefile                       |   7 ++
 lib/optee/optee.c                        |  66 +++++++++++++++++
 tools/default_image.c                    |  25 +++++--
 11 files changed, 300 insertions(+), 8 deletions(-)
 create mode 100644 doc/README.trusted-execution-environment
 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 v4 01/12] 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   |  7 +++++++
 lib/optee/Makefile  |  7 +++++++
 lib/optee/optee.c   | 31 +++++++++++++++++++++++++++++++
 6 files changed, 63 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 4fd41c4..a4029a6 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -310,5 +310,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 0db41c1..35da570 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -18,6 +18,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..ed6405f
--- /dev/null
+++ b/lib/optee/Kconfig
@@ -0,0 +1,7 @@
+config OPTEE
+	bool "Support OPTEE images"
+	help
+	  Selecting this option enables the OPTEE library code and
+          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..2cc16d7
--- /dev/null
+++ b/lib/optee/optee.c
@@ -0,0 +1,31 @@
+/*
+ * Copyright (C) 2017 Linaro
+ * Bryan O'Donoghue <bryan.odonoghue@linaro.org>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <tee/optee.h>
+
+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) {
+		return -EINVAL;
+	}
+
+	return 0;
+}
-- 
2.7.4

[U-Boot] [PATCH v4 02/12] 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 ed6405f..2ca7be7 100644
--- a/lib/optee/Kconfig
+++ b/lib/optee/Kconfig
@@ -5,3 +5,11 @@ config OPTEE
           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 v4 03/12] 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 2ca7be7..9e9ef39 100644
--- a/lib/optee/Kconfig
+++ b/lib/optee/Kconfig
@@ -13,3 +13,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 v4 04/12] 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 v4 05/12] 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 v4 06/12] image: Update comment for IH_TYPE_TEE

[Bryan O'Donoghue]

Add "(TEE)" to the name of the existing TEE in the comment adjacent to its
definition. Later patches reuse the TEE name so introduce the reduced TLA
acronym properly here.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 include/image.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/image.h b/include/image.h
index 325b014..a2372de 100644
--- a/include/image.h
+++ b/include/image.h
@@ -269,7 +269,7 @@ enum {
 	IH_TYPE_ZYNQMPIMAGE,		/* Xilinx ZynqMP Boot Image */
 	IH_TYPE_FPGA,			/* FPGA Image */
 	IH_TYPE_VYBRIDIMAGE,	/* VYBRID .vyb Image */
-	IH_TYPE_TEE,            /* Trusted Execution Environment OS Image */
+	IH_TYPE_TEE,            /* Trusted Execution Environment (TEE) OS Image */
 	IH_TYPE_FIRMWARE_IVT,		/* Firmware Image with HABv4 IVT */
 	IH_TYPE_PMMC,            /* TI Power Management Micro-Controller Firmware */
 
-- 
2.7.4

[U-Boot] [PATCH v4 07/12] tools: mkimage: add tee-bootable image type

[Bryan O'Donoghue]

This patch adds support for bootable TEE images to mkimage. Currently
there is a (Trusted Execution Environment) TEE image type, the TEE image
type is installed to a memory location control is passed to the TEE and
then the TEE returns to u-boot.

flow #0:
BootROM -> u-boot -> tee -> u-boot -> onwards

For some TEE implementations, such as upstream OPTEE for i.MX6 and i.MX7
the boot flow is

flow #1:
BootROM -> u-boot -> optee -> kernel

This patch adds a new image type to mkimage -  IH_TYPE_TEE_BOOTABLE to
reflect this TEE boot flow and to facilitate additional OPTEE specific
verification of that image type - prior to handing control to that image.

The new image type enables us to more easily generate and validate a
bootable OPTEE image also, for example instead of generating an OPTEE image
like this:

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

we can instead generate images like this:
mkimage -A arm -T tee-bootable -C none -d ./out/arm-plat-imx/core/tee.bin
uTee.optee

That OPTEE image then will have a specific image type that bootm can
automatically identify and consequently perform additional optee-header
checks on.

Subsequent patches add logic to perform those optee-specific changes prior
to handing over control as described in flow #1 above.

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>
Link: http://mrvan.github.io/optee-imx6ul
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..e7785ce 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_TEE_BOOTABLE, "tee-bootable", "Trusted Execution Environment Bootable Image",},
 	{	-1,		    "",		  "",			},
 };
 
diff --git a/include/image.h b/include/image.h
index a2372de..d2c47ef 100644
--- a/include/image.h
+++ b/include/image.h
@@ -272,6 +272,7 @@ enum {
 	IH_TYPE_TEE,            /* Trusted Execution Environment (TEE) OS Image */
 	IH_TYPE_FIRMWARE_IVT,		/* Firmware Image with HABv4 IVT */
 	IH_TYPE_PMMC,            /* TI Power Management Micro-Controller Firmware */
+	IH_TYPE_TEE_BOOTABLE,		/* TEE Bootable Image */
 
 	IH_TYPE_COUNT,			/* Number of image types */
 };
diff --git a/tools/default_image.c b/tools/default_image.c
index 4e5568e..fc0b0c0 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_TEE_BOOTABLE))
 		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_TEE_BOOTABLE:
+		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 v4 08/12] doc: TEE: Add documentation describing TEE in u-boot

[Bryan O'Donoghue]

This patch adds a brief description of TEE in u-boot.

It gives a basic introduction, description of image generation with mkimage
plus the various ways u-boot can install or chainload a TEE.

Methods covered in this patch are

- tee-standalone
  This is method where u-boot loads a TEE into an area of DRAM or SRAM
  hands off control to a ROM callback or jumps into the TEE intself and
  then once the TEE is installed, returns control to u-boot.

- tee-bootable
  This is the method where u-boot chain-loads the TEE. In this case once
  u-boot hands off control to the TEE execution does not return to u-boot.

Subsequent methods of performing a TEE boot with u-boot may be added over
time, for example "tee-combo" is being discussed.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 doc/README.trusted-execution-environment | 123 +++++++++++++++++++++++++++++++
 1 file changed, 123 insertions(+)
 create mode 100644 doc/README.trusted-execution-environment

diff --git a/doc/README.trusted-execution-environment b/doc/README.trusted-execution-environment
new file mode 100644
index 0000000..12bf615
--- /dev/null
+++ b/doc/README.trusted-execution-environment
@@ -0,0 +1,123 @@
+Trusted Execution Environment
+=============================
+
+Overview
+--------
+Trusted Execution Environment (TEE) specifies a secure mode of execution of a
+processor. The TEE provides an isolted environment that runs in parallel to the
+rich execution environment meaning an environment such as a Linux based
+operating system.
+
+TEE may provide access to crypto keys or other pieces of secure silicon that are
+not available to the rich execution environment or TEE implementations may
+reside in secure sections of memory only accessible when running in a TEE
+context.
+
+The TEE specification is available here:
+https://www.globalplatform.org/specificationsdevice.asp
+
+In u-boot currently all TEE versions supported are based on the Open Portable
+Trusted Execution Environment project. OP-TEE is an open source implementation
+of a TEE.
+
+See https://www.op-tee.org/ for more details.
+
+Supported TEE methods
+---------------------
+
+In u-boot there are two means of installing a TEE
+
+- Installing a TEE (tee-standalone)
+
+  In this case u-boot is responsible for loading the TEE into memory, jumping
+  into the TEE and subsequently handling return of control back to u-boot.
+
+  u-boot then is responsible to load and boot a kernel and DTB in the normal
+  way.
+
+  BootROM/SPL
+        |
+        v
+      u-boot ---->
+                  TEE
+      u-boot <----
+        |
+        v
+      Linux
+
+- Chainloading via a TEE (tee-bootable)
+
+  In this case u-boot is responsible for loading the TEE into memory and handing
+  control to the TEE. The TEE then will enter into secure mode boot-strap itself
+  and hand control onto a subsequent boot stage - typically a Linux kernel.
+
+  When chain-loading in this way u-boot is reponsible for loading bootscripts,
+  Kernel, DTB etc into memory.
+
+  BootROM/SPL
+        |
+        v
+      u-boot
+        |
+        v
+       TEE
+        |
+        v
+      Linux
+
+Creating a TEE image with mkimage
+---------------------------------
+
+- "tee" (tee-standalone)
+
+  To identify this type of image to u-boot you should use mkimage like this:
+
+  mkimage -A arm -T tee -C none -d tee-image.bin uTee-standalone
+
+- "tee-bootable"
+
+  mkimage -A arm -T tee-bootable -C none -d tee.bin uTee-bootable
+
+Booting the image types
+-----------------------
+
+- tee-standalone
+
+  For a standalone TEE image you should create or reuse an existing board-port
+  and install the TEE into memory in the appropriate way for your architecture.
+
+  Some TEE implementations may reside in a special SRAM area or have special
+  ROM callbacks in order to setup the TEE correctly.
+
+  eg:
+  board/company/board_name.c
+
+  void board_tee_image_process(ulong tee_image, size_t tee_size)
+  {
+      /* Install TEE into memory as approrpiate here */
+  }
+
+  U_BOOT_FIT_LOADABLE_HANDLER(IH_TYPE_TEE, board_tee_image_process);
+
+- tee-bootable
+
+  For a bootable TEE image you need to load the TEE into an appropriate address
+  in DRAM.
+
+  Once done use the bootm command to execute the image.
+
+  eg:
+  => ext4load mmc 0:1 /lib/firmware/uTee-bootable 0x84000000
+  => bootm 0x84000000
+
+  ## Booting kernel from Legacy Image at 84000000 ...
+   Image Name:
+   Image Type:   ARM Linux Trusted Execution Environment Bootable Image (uncompressed)
+   Data Size:    249844 Bytes = 244 KiB
+   Load Address: 9dffffe4
+   Entry Point:  9e000000
+   Verifying Checksum ... OK
+  ## Flattened Device Tree blob at 83000000
+   Booting using the fdt blob at 0x83000000
+   Loading Trusted Execution Environment Bootable Image ... OK
+   Using Device Tree in place at 83000000, end 83009b4d
-- 
2.7.4

[U-Boot] [PATCH v4 09/12] 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   | 20 ++++++++++++++++++++
 2 files changed, 33 insertions(+)

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 2cc16d7..365c078 100644
--- a/lib/optee/optee.c
+++ b/lib/optee/optee.c
@@ -29,3 +29,23 @@ int optee_verify_image(struct optee_header *hdr, unsigned long tzdram_start,
 
 	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)
+		return ret;
+
+	if (image_load_addr + sizeof(*hdr) != hdr->init_load_addr_lo)
+		ret = -EINVAL;
+
+	return ret;
+}
-- 
2.7.4

[U-Boot] [PATCH v4 10/12] optee: Add error printout

[Bryan O'Donoghue]

When encountering an error in OPTEE verification print out various details
of the OPTEE header to aid in further debugging of encountered errors.

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 | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/lib/optee/optee.c b/lib/optee/optee.c
index 365c078..78a15e8 100644
--- a/lib/optee/optee.c
+++ b/lib/optee/optee.c
@@ -8,6 +8,12 @@
 #include <common.h>
 #include <tee/optee.h>
 
+#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)
 {
@@ -42,10 +48,19 @@ int optee_verify_bootm_image(unsigned long image_addr,
 
 	ret = optee_verify_image(hdr, tzdram_start, tzdram_len, image_len);
 	if (ret)
-		return ret;
+		goto error;
 
-	if (image_load_addr + sizeof(*hdr) != hdr->init_load_addr_lo)
+	if (image_load_addr + sizeof(*hdr) != hdr->init_load_addr_lo) {
 		ret = -EINVAL;
+		goto error;
+	}
+
+	return ret;
+error:
+	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 v4 11/12] bootm: optee: Add mechanism to validate a bootable TEE image

[Bryan O'Donoghue]

This patch makes it possible to verify the contents and location of an
a bootable TEE 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..3246ceb 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_TEE_BOOTABLE) {
+		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_TEE_BOOTABLE)) &&
 	    (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_TEE_BOOTABLE:
 			*os_data = image_get_data(hdr);
 			*os_len = image_get_data_size(hdr);
 			break;
-- 
2.7.4

[U-Boot] [PATCH v4 12/12] optee: Add CONFIG_OPTEE_LOAD_ADDR

[Bryan O'Donoghue]

CONFIG_OPTEE_LOAD_ADDR is used to tell u-boot where to load the OPTEE
binary into memory prior to handing off control to OPTEE.

We need to pull this value out of u-boot in order to produce an IMX IVT/CSF
signed pair for the purposes of secure boot. The best way to do that is to
have CONFIG_OPTEE_LOAD_ADDR appear in u-boot.cfg.

Adding new CONFIG entires to u-boot should be kconfig driven so this patch
does just that.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Ryan Harkin <ryan.harkin@linaro.org>
---
 lib/optee/Kconfig | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/optee/Kconfig b/lib/optee/Kconfig
index 9e9ef39..541f10a 100644
--- a/lib/optee/Kconfig
+++ b/lib/optee/Kconfig
@@ -6,6 +6,12 @@ config OPTEE
           OPTEE-specific checks before booting an OPTEE image created with
           mkimage.
 
+config OPTEE_LOAD_ADDR
+	hex "OPTEE load address"
+	default 0x00000000
+	help
+	  The load address of the bootable OPTEE binary.
+
 config OPTEE_TZDRAM_SIZE
 	hex "Amount of Trust-Zone RAM for the OPTEE image"
 	depends on OPTEE
-- 
2.7.4

[U-Boot] [PATCH v4 08/12] doc: TEE: Add documentation describing TEE in u-boot

[Dr. Philipp Tomsich]

> On 26 Feb 2018, at 13:36, Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:
> 
> This patch adds a brief description of TEE in u-boot.
> 
> It gives a basic introduction, description of image generation with mkimage
> plus the various ways u-boot can install or chainload a TEE.
> 
> Methods covered in this patch are
> 
> - tee-standalone
>  This is method where u-boot loads a TEE into an area of DRAM or SRAM
>  hands off control to a ROM callback or jumps into the TEE intself and
>  then once the TEE is installed, returns control to u-boot.
> 
> - tee-bootable
>  This is the method where u-boot chain-loads the TEE. In this case once
>  u-boot hands off control to the TEE execution does not return to u-boot.
> 
> Subsequent methods of performing a TEE boot with u-boot may be added over
> time, for example "tee-combo" is being discussed.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> doc/README.trusted-execution-environment | 123 +++++++++++++++++++++++++++++++
> 1 file changed, 123 insertions(+)
> create mode 100644 doc/README.trusted-execution-environment
> 
> diff --git a/doc/README.trusted-execution-environment b/doc/README.trusted-execution-environment
> new file mode 100644
> index 0000000..12bf615
> --- /dev/null
> +++ b/doc/README.trusted-execution-environment
> @@ -0,0 +1,123 @@
> +Trusted Execution Environment
> +=============================
> +
> +Overview
> +--------
> +Trusted Execution Environment (TEE) specifies a secure mode of execution of a
> +processor. The TEE provides an isolted environment that runs in parallel to the
> +rich execution environment meaning an environment such as a Linux based
> +operating system.
> +
> +TEE may provide access to crypto keys or other pieces of secure silicon that are
> +not available to the rich execution environment or TEE implementations may
> +reside in secure sections of memory only accessible when running in a TEE
> +context.
> +
> +The TEE specification is available here:
> +https://www.globalplatform.org/specificationsdevice.asp
> +
> +In u-boot currently all TEE versions supported are based on the Open Portable
> +Trusted Execution Environment project. OP-TEE is an open source implementation
> +of a TEE.
> +
> +See https://www.op-tee.org/ for more details.
> +
> +Supported TEE methods
> +---------------------
> +
> +In u-boot there are two means of installing a TEE
> +
> +- Installing a TEE (tee-standalone)
> +
> +  In this case u-boot is responsible for loading the TEE into memory, jumping
> +  into the TEE and subsequently handling return of control back to u-boot.
> +
> +  u-boot then is responsible to load and boot a kernel and DTB in the normal
> +  way.
> +
> +  BootROM/SPL
> +        |
> +        v
> +      u-boot ---->
> +                  TEE
> +      u-boot <----
> +        |
> +        v
> +      Linux
> +
> +- Chainloading via a TEE (tee-bootable)
> +
> +  In this case u-boot is responsible for loading the TEE into memory and handing
> +  control to the TEE. The TEE then will enter into secure mode boot-strap itself
> +  and hand control onto a subsequent boot stage - typically a Linux kernel.
> +
> +  When chain-loading in this way u-boot is reponsible for loading bootscripts,
> +  Kernel, DTB etc into memory.
> +
> +  BootROM/SPL
> +        |
> +        v
> +      u-boot
> +        |
> +        v
> +       TEE
> +        |
> +        v
> +      Linux
> +
> +Creating a TEE image with mkimage
> +---------------------------------
> +
> +- "tee" (tee-standalone)
> +
> +  To identify this type of image to u-boot you should use mkimage like this:
> +
> +  mkimage -A arm -T tee -C none -d tee-image.bin uTee-standalone

The type should be “tee-standalone” to avoid confusion with the boot-through
variety.

> +
> +- "tee-bootable"
> +
> +  mkimage -A arm -T tee-bootable -C none -d tee.bin uTee-bootable

Bootable doesn’t really describe this: both the -standalone and this version of
the OPTEE are bootable, but the difference is that this variant also contains the
next-stage payload. Either we keep Tom’s proposed -combo naming or we can
try to describe this as a “tee-with-payload” type.

> +
> +Booting the image types
> +-----------------------
> +
> +- tee-standalone
> +
> +  For a standalone TEE image you should create or reuse an existing board-port
> +  and install the TEE into memory in the appropriate way for your architecture.
> +
> +  Some TEE implementations may reside in a special SRAM area or have special
> +  ROM callbacks in order to setup the TEE correctly.
> +
> +  eg:
> +  board/company/board_name.c
> +
> +  void board_tee_image_process(ulong tee_image, size_t tee_size)
> +  {
> +      /* Install TEE into memory as approrpiate here */
> +  }
> +
> +  U_BOOT_FIT_LOADABLE_HANDLER(IH_TYPE_TEE, board_tee_image_process);
> +
> +- tee-bootable
> +
> +  For a bootable TEE image you need to load the TEE into an appropriate address
> +  in DRAM.
> +
> +  Once done use the bootm command to execute the image.
> +
> +  eg:
> +  => ext4load mmc 0:1 /lib/firmware/uTee-bootable 0x84000000
> +  => bootm 0x84000000
> +
> +  ## Booting kernel from Legacy Image at 84000000 ...
> +   Image Name:
> +   Image Type:   ARM Linux Trusted Execution Environment Bootable Image (uncompressed)
> +   Data Size:    249844 Bytes = 244 KiB
> +   Load Address: 9dffffe4
> +   Entry Point:  9e000000
> +   Verifying Checksum ... OK
> +  ## Flattened Device Tree blob at 83000000
> +   Booting using the fdt blob at 0x83000000
> +   Loading Trusted Execution Environment Bootable Image ... OK
> +   Using Device Tree in place at 83000000, end 83009b4d
> -- 
> 2.7.4
> 

[U-Boot] [PATCH v4 07/12] tools: mkimage: add tee-bootable image type

[Tom Rini]

On Mon, Feb 26, 2018 at 12:36:01PM +0000, Bryan O'Donoghue wrote:
> This patch adds support for bootable TEE images to mkimage. Currently
> there is a (Trusted Execution Environment) TEE image type, the TEE image
> type is installed to a memory location control is passed to the TEE and
> then the TEE returns to u-boot.
> 
> flow #0:
> BootROM -> u-boot -> tee -> u-boot -> onwards
> 
> For some TEE implementations, such as upstream OPTEE for i.MX6 and i.MX7
> the boot flow is
> 
> flow #1:
> BootROM -> u-boot -> optee -> kernel

Can you please re-phrase these flows in the way Andrew had, in the other
thread where it's clear what happens on the secure vs non-secure side?
I found that most helpful and I bet others reading the log in the future
will as well.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180226/3855c7d5/attachment.sig>

[U-Boot] [PATCH v4 08/12] doc: TEE: Add documentation describing TEE in u-boot

[Bryan O'Donoghue]

On 26/02/18 13:53, Dr. Philipp Tomsich wrote:
>> +  To identify this type of image to u-boot you should use mkimage like this:
>> +
>> +  mkimage -A arm -T tee -C none -d tee-image.bin uTee-standalone
> The type should be “tee-standalone” to avoid confusion with the boot-through
> variety.

Eh, I actually agree with you.

I was thinking that making the change from "tee" to "tee-standalone" and 
might break things for people with their continuous-integration 
jobs/scripts etc.

So I was going to send out an "RFC" patch changing "tee" to 
"tee-standalone".

I'm just as happy to make the name change in this set if it's an agreed 
thing with Tom and Andrew though.

> 
>> +
>> +- "tee-bootable"
>> +
>> +  mkimage -A arm -T tee-bootable -C none -d tee.bin uTee-bootable

> Bootable doesn’t really describe this: both the -standalone and this version of
> the OPTEE are bootable, but the difference is that this variant also contains the
> next-stage payload. Either we keep Tom’s proposed -combo naming or we can
> try to describe this as a “tee-with-payload” type.

Hmm - I thought Tom had suggested

"tee" - stays as is because that's what it's already called.
"tee-standalone" - which I've called "tee-bootable"

and then there was a potential addition "tee-combo" covering what Kever 
may or may not do with his SPL implementation.

but I opted for "tee-bootable" instead of "tee-standalone"..

I think we need two decisions here:

#1 Exiting:
"tee" - stay as
"tee-standalone" - new name more obvious - adds churn into mkimage namespace

#2 Bootable OPTEE (chainload):
"tee-bootable"
"tee-chainload"
"tee-with-payload" (not so sure about this myself)

Tom, Philipp, Andrew ?

--
bod

[U-Boot] [PATCH v4 08/12] doc: TEE: Add documentation describing TEE in u-boot

[Dr. Philipp Tomsich]

Bryan,

> On 26 Feb 2018, at 15:39, Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:
> 
> 
> 
> On 26/02/18 13:53, Dr. Philipp Tomsich wrote:
>>> +  To identify this type of image to u-boot you should use mkimage like this:
>>> +
>>> +  mkimage -A arm -T tee -C none -d tee-image.bin uTee-standalone
>> The type should be “tee-standalone” to avoid confusion with the boot-through
>> variety.
> 
> Eh, I actually agree with you.
> 
> I was thinking that making the change from "tee" to "tee-standalone" and might break things for people with their continuous-integration jobs/scripts etc.
> 
> So I was going to send out an "RFC" patch changing "tee" to "tee-standalone".
> 
> I'm just as happy to make the name change in this set if it's an agreed thing with Tom and Andrew though.
> 
>>> +
>>> +- "tee-bootable"
>>> +
>>> +  mkimage -A arm -T tee-bootable -C none -d tee.bin uTee-bootable
> 
>> Bootable doesn’t really describe this: both the -standalone and this version of
>> the OPTEE are bootable, but the difference is that this variant also contains the
>> next-stage payload. Either we keep Tom’s proposed -combo naming or we can
>> try to describe this as a “tee-with-payload” type.
> 
> Hmm - I thought Tom had suggested
> 
> "tee" - stays as is because that's what it's already called.
> "tee-standalone" - which I've called "tee-bootable"
> 
> and then there was a potential addition "tee-combo" covering what Kever may or may not do with his SPL implementation.
> 
> but I opted for "tee-bootable" instead of "tee-standalone"..
> 
> I think we need two decisions here:
> 
> #1 Exiting:
> "tee" - stay as
> "tee-standalone" - new name more obvious - adds churn into mkimage namespace
> 
> #2 Bootable OPTEE (chainload):
> "tee-bootable"
> "tee-chainload"
> "tee-with-payload" (not so sure about this myself)
> 
> Tom, Philipp, Andrew ?

Now, I am confused.

I thought there’s only two different paths: the one where the TEE is “installed” as
a secure OS (and then returns control to the bootloader) and the other one where
control os passed to the TEE and it “boot-through" into a next stage (whether a full
U-Boot or Linux).

The “boot-through” approach would be similar to what ATF does ...

Regards,
Philipp.

[U-Boot] [PATCH v4 08/12] doc: TEE: Add documentation describing TEE in u-boot

[Bryan O'Donoghue]

On 26/02/18 14:56, Dr. Philipp Tomsich wrote:
> Now, I am confused.

Fair enough.

In my view there are really only two required paths i.e. kever's stuff 
can use the existing "tee" type, let's not discuss a third option further.

So for clarity the proposal is

1. Maintain the existing "tee" as is.

    As regards changing the name of "tee" to "tee-standalone" I'd like
    to get Tom or Andrew (both) to say that's what is wanted.

    Since it's TI boards that are using the "tee" name in mkimage
    upstream the name-change is churn there.

    Andrew, Tom ?

2. Add a new bootable type
    The set of names we have for that is

    {tee-bootable, tee-chainload, tee-with-payload}

    I have no strong feelings about the name for the new type either way

    :)

[U-Boot] [PATCH v4 08/12] doc: TEE: Add documentation describing TEE in u-boot

[Andrew F. Davis]

On 02/26/2018 09:06 AM, Bryan O'Donoghue wrote:
> 
> 
> On 26/02/18 14:56, Dr. Philipp Tomsich wrote:
>> Now, I am confused.
> 
> Fair enough.
> 
> In my view there are really only two required paths i.e. kever's stuff
> can use the existing "tee" type, let's not discuss a third option further.
> 
> So for clarity the proposal is
> 
> 1. Maintain the existing "tee" as is.
> 
>    As regards changing the name of "tee" to "tee-standalone" I'd like
>    to get Tom or Andrew (both) to say that's what is wanted.
> 
>    Since it's TI boards that are using the "tee" name in mkimage
>    upstream the name-change is churn there.
> 
>    Andrew, Tom ?
> 

I wouldn't be too opposed to the name change if we decide to go down
this two type path, but I'm still not convinced we are doing the right
thing here.

Lets look at u-boot/include/image.h for a moment, the table of IH_TYPE_*
already has a type for what we are trying to do here: IH_TYPE_KERNEL.
The comment on this table describes "OS Kernel Images" as exactly what
you are doing with your TEE image.

To me what you really want to do is add a new IH_OS_*, which are defined
in a different table above (I see this is already done for ATF
(IH_OS_ARM_TRUSTED_FIRMWARE) which shares a similar “boot-through” flow
like Philipp pointed out).

So I'm still not sure what the technical reason you need a new "type" of
image, when adding your hooks to the existing IH_TYPE_KERNEL path in
U-Boot could be made to do the same thing when it encounters a IH_OS_TEE
OS image.

Andrew

> 2. Add a new bootable type
>    The set of names we have for that is
> 
>    {tee-bootable, tee-chainload, tee-with-payload}
> 
>    I have no strong feelings about the name for the new type either way
> 
>    :)

[U-Boot] [PATCH v4 08/12] doc: TEE: Add documentation describing TEE in u-boot

[Dr. Philipp Tomsich]

> On 26 Feb 2018, at 16:26, Andrew F. Davis <afd@ti.com> wrote:
> 
> On 02/26/2018 09:06 AM, Bryan O'Donoghue wrote:
>> 
>> 
>> On 26/02/18 14:56, Dr. Philipp Tomsich wrote:
>>> Now, I am confused.
>> 
>> Fair enough.
>> 
>> In my view there are really only two required paths i.e. kever's stuff
>> can use the existing "tee" type, let's not discuss a third option further.
>> 
>> So for clarity the proposal is
>> 
>> 1. Maintain the existing "tee" as is.
>> 
>>    As regards changing the name of "tee" to "tee-standalone" I'd like
>>    to get Tom or Andrew (both) to say that's what is wanted.
>> 
>>    Since it's TI boards that are using the "tee" name in mkimage
>>    upstream the name-change is churn there.
>> 
>>    Andrew, Tom ?
>> 
> 
> I wouldn't be too opposed to the name change if we decide to go down
> this two type path, but I'm still not convinced we are doing the right
> thing here.
> 
> Lets look at u-boot/include/image.h for a moment, the table of IH_TYPE_*
> already has a type for what we are trying to do here: IH_TYPE_KERNEL.
> The comment on this table describes "OS Kernel Images" as exactly what
> you are doing with your TEE image.
> 
> To me what you really want to do is add a new IH_OS_*, which are defined
> in a different table above (I see this is already done for ATF
> (IH_OS_ARM_TRUSTED_FIRMWARE) which shares a similar “boot-through” flow
> like Philipp pointed out).

The IH_OS_ARM_TRUSTED_FIRMWARE (sorry for not shortening this, 
when I added it) encodes a specific parameter passing convention for the
ATF (i.e. bl31_params_t, platform-specific parameters — and in the future
bl32_params_t for loading up an OPTEE on ARMv8).

The parameter passing convention for an OPTEE on ARMv7 is different
(see https://github.com/OP-TEE/optee_os/blob/master/core/arch/arm/kernel/generic_entry_a32.S#L186)
and therefore affords a separate OS type.

The thing I haven’t understood yet, though, is: how can the “returns to
link-register” variant exist, if the parameter passing (and entry code) in
the upstream OPTEE looks as it does...

> 
> So I'm still not sure what the technical reason you need a new "type" of
> image, when adding your hooks to the existing IH_TYPE_KERNEL path in
> U-Boot could be made to do the same thing when it encounters a IH_OS_TEE
> OS image.
> 
> Andrew
> 
>> 2. Add a new bootable type
>>    The set of names we have for that is
>> 
>>    {tee-bootable, tee-chainload, tee-with-payload}
>> 
>>    I have no strong feelings about the name for the new type either way
>> 
>>    :)

[U-Boot] [PATCH v4 08/12] doc: TEE: Add documentation describing TEE in u-boot

[Andrew F. Davis]

On 02/26/2018 09:32 AM, Dr. Philipp Tomsich wrote:
> 
>> On 26 Feb 2018, at 16:26, Andrew F. Davis <afd@ti.com> wrote:
>>
>> On 02/26/2018 09:06 AM, Bryan O'Donoghue wrote:
>>>
>>>
>>> On 26/02/18 14:56, Dr. Philipp Tomsich wrote:
>>>> Now, I am confused.
>>>
>>> Fair enough.
>>>
>>> In my view there are really only two required paths i.e. kever's stuff
>>> can use the existing "tee" type, let's not discuss a third option further.
>>>
>>> So for clarity the proposal is
>>>
>>> 1. Maintain the existing "tee" as is.
>>>
>>>    As regards changing the name of "tee" to "tee-standalone" I'd like
>>>    to get Tom or Andrew (both) to say that's what is wanted.
>>>
>>>    Since it's TI boards that are using the "tee" name in mkimage
>>>    upstream the name-change is churn there.
>>>
>>>    Andrew, Tom ?
>>>
>>
>> I wouldn't be too opposed to the name change if we decide to go down
>> this two type path, but I'm still not convinced we are doing the right
>> thing here.
>>
>> Lets look at u-boot/include/image.h for a moment, the table of IH_TYPE_*
>> already has a type for what we are trying to do here: IH_TYPE_KERNEL.
>> The comment on this table describes "OS Kernel Images" as exactly what
>> you are doing with your TEE image.
>>
>> To me what you really want to do is add a new IH_OS_*, which are defined
>> in a different table above (I see this is already done for ATF
>> (IH_OS_ARM_TRUSTED_FIRMWARE) which shares a similar “boot-through” flow
>> like Philipp pointed out).
> 
> The IH_OS_ARM_TRUSTED_FIRMWARE (sorry for not shortening this, 
> when I added it) encodes a specific parameter passing convention for the
> ATF (i.e. bl31_params_t, platform-specific parameters — and in the future
> bl32_params_t for loading up an OPTEE on ARMv8).
> 
> The parameter passing convention for an OPTEE on ARMv7 is different
> (see https://github.com/OP-TEE/optee_os/blob/master/core/arch/arm/kernel/generic_entry_a32.S#L186)
> and therefore affords a separate OS type.
> 

I agree, that is what I am suggesting. A new IH_OS_ and not a new image
type IH_TYPE_ as IH_TYPE_KERNEL is already the right type.

> The thing I haven’t understood yet, though, is: how can the “returns to
> link-register” variant exist, if the parameter passing (and entry code) in
> the upstream OPTEE looks as it does...
> 


When the SMC call to install a TEE is made the ROM saves the non-secure
context, so in OP-TEE we simply retrieve this context [0] from the ROM
memory and use it as our non-secure context to return to, this puts us
right back to where the SMC came from when OP-TEE install finishes.

This allows us to do stuff like starting TEE from kernel dynamically at
runtime, something I'm not sure how the other flow could support if it
always expects to be an intermediate boot stage.. but I guess that is
their problem to solve not mine :)

[0]
https://github.com/OP-TEE/optee_os/blob/master/core/arch/arm/plat-ti/main.c#L128

Andrew


>>
>> So I'm still not sure what the technical reason you need a new "type" of
>> image, when adding your hooks to the existing IH_TYPE_KERNEL path in
>> U-Boot could be made to do the same thing when it encounters a IH_OS_TEE
>> OS image.
>>
>> Andrew
>>
>>> 2. Add a new bootable type
>>>    The set of names we have for that is
>>>
>>>    {tee-bootable, tee-chainload, tee-with-payload}
>>>
>>>    I have no strong feelings about the name for the new type either way
>>>
>>>    :)
> 

[U-Boot] [PATCH v4 08/12] doc: TEE: Add documentation describing TEE in u-boot

[Bryan O'Donoghue]

On 26/02/18 15:26, Andrew F. Davis wrote:
> Lets look at u-boot/include/image.h for a moment, the table of IH_TYPE_*
> already has a type for what we are trying to do here: IH_TYPE_KERNEL.
> The comment on this table describes "OS Kernel Images" as exactly what
> you are doing with your TEE image.
> 
> To me what you really want to do is add a new IH_OS_*, which are defined
> in a different table above (I see this is already done for ATF
> (IH_OS_ARM_TRUSTED_FIRMWARE) which shares a similar “boot-through” flow
> like Philipp pointed out).
> 
> So I'm still not sure what the technical reason you need a new "type" of
> image, when adding your hooks to the existing IH_TYPE_KERNEL path in
> U-Boot could be made to do the same thing when it encounters a IH_OS_TEE

I'll have a look and see if your suggestion on IH_TYPE_KERNEL is 
feasible, it looks like is though I don't know for sure until I try.

i.e.
"mkimage -A arm -T kernel -C none -d tee.bin uTee.optee" + hook on the 
OPTEE header on the IH_TYPE_KERNEL path.

---
bod