* [PATCH 0/1] OP-TEE support for barebox
@ 2019-01-29 11:10 Rouven Czerwinski
2019-01-29 11:10 ` [PATCH 1/1] ARM: Initial OP-TEE support Rouven Czerwinski
0 siblings, 1 reply; 6+ messages in thread
From: Rouven Czerwinski @ 2019-01-29 11:10 UTC (permalink / raw)
To: barebox; +Cc: Rouven Czerwinski
This is a rebase and update of the testing patch posted by Sascha Hauer
in 2017.
The FIXMEs in the code have been addressed and additional support for
loading optee as a builtin firmware has been added.
Rouven Czerwinski
Sascha Hauer (1):
ARM: Initial OP-TEE support
arch/arm/cpu/Makefile | 2 +-
arch/arm/cpu/start-kernel-optee.S | 14 ++++-
arch/arm/cpu/start.c | 3 +-
arch/arm/include/asm/armlinux.h | 2 +-
arch/arm/include/asm/barebox-arm.h | 9 ++-
arch/arm/lib32/armlinux.c | 10 ++-
arch/arm/lib32/bootm.c | 109 +++++++++++++++++++++++++++++-
arch/arm/lib32/bootu.c | 2 +-
arch/arm/lib32/bootz.c | 2 +-
commands/bootm.c | 11 ++-
common/Kconfig | 33 +++++++++-
common/bootm.c | 6 ++-
firmware/Kconfig | 3 +-
firmware/Makefile | 2 +-
include/bootm.h | 3 +-
include/tee/optee.h | 30 ++++++++-
16 files changed, 234 insertions(+), 7 deletions(-)
create mode 100644 arch/arm/cpu/start-kernel-optee.S
create mode 100644 include/tee/optee.h
base-commit: 60e12093cf3288086b62612bb3cf565a0b4320aa
--
git-series 0.9.1
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] ARM: Initial OP-TEE support
2019-01-29 11:10 [PATCH 0/1] OP-TEE support for barebox Rouven Czerwinski
@ 2019-01-29 11:10 ` Rouven Czerwinski
2019-01-29 11:15 ` Rouven Czerwinski
2019-01-29 22:35 ` Andrey Smirnov
0 siblings, 2 replies; 6+ messages in thread
From: Rouven Czerwinski @ 2019-01-29 11:10 UTC (permalink / raw)
To: barebox; +Cc: Rouven Czerwinski
From: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
This adds initial support for OP-TEE, see https://www.op-tee.org/
barebox starts in secure mode as usual. When booting a kernel
the bootm code also loads the optee_os binary. Instead of jumping
into the kernel barebox jumps into the optee_os binary and puts
the kernel execution address into the lr register. OP-TEE then
jumps into the kernel in nonsecure mode.
The optee_os binary is passed with the -t option to bootm or
with global.bootm.tee.
Optionally OP-TEE can be compiled into barebox using the builtin firmware
feature. Enable the Kconfig option and place or link your tee binary as
optee.bin into the firmware directory.
The amount of SDRAM which is kept free for OP-TEE is configurable.
This patch was tested on a i.MX6 Nitrogen6x board.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
---
arch/arm/cpu/Makefile | 2 +-
arch/arm/cpu/start-kernel-optee.S | 14 ++++-
arch/arm/cpu/start.c | 3 +-
arch/arm/include/asm/armlinux.h | 2 +-
arch/arm/include/asm/barebox-arm.h | 9 ++-
arch/arm/lib32/armlinux.c | 10 ++-
arch/arm/lib32/bootm.c | 109 +++++++++++++++++++++++++++++-
arch/arm/lib32/bootu.c | 2 +-
arch/arm/lib32/bootz.c | 2 +-
commands/bootm.c | 11 ++-
common/Kconfig | 33 +++++++++-
common/bootm.c | 6 ++-
firmware/Kconfig | 3 +-
firmware/Makefile | 2 +-
include/bootm.h | 3 +-
include/tee/optee.h | 30 ++++++++-
16 files changed, 234 insertions(+), 7 deletions(-)
create mode 100644 arch/arm/cpu/start-kernel-optee.S
create mode 100644 include/tee/optee.h
diff --git a/arch/arm/cpu/Makefile b/arch/arm/cpu/Makefile
index a35db43..e6e74e0 100644
--- a/arch/arm/cpu/Makefile
+++ b/arch/arm/cpu/Makefile
@@ -12,6 +12,8 @@ obj-y += start.o entry.o
obj-pbl-y += setupc$(S64).o cache$(S64).o
+obj-$(CONFIG_BOOTM_OPTEE) += start-kernel-optee.o
+
#
# Any variants can be called as start-armxyz.S
#
diff --git a/arch/arm/cpu/start-kernel-optee.S b/arch/arm/cpu/start-kernel-optee.S
new file mode 100644
index 0000000..ce5ac17
--- /dev/null
+++ b/arch/arm/cpu/start-kernel-optee.S
@@ -0,0 +1,14 @@
+#include <linux/linkage.h>
+
+ENTRY(start_kernel_optee)
+ /*
+ * r0 = optee
+ * r1 = kernel
+ * r2 = oftree
+ */
+ mov r4, r0
+ mov r0, #0
+ mov lr, r1
+ mov r1, #0
+ bx r4
+ENDPROC(start_kernel_optee)
diff --git a/arch/arm/cpu/start.c b/arch/arm/cpu/start.c
index 768fa9e..e78a820 100644
--- a/arch/arm/cpu/start.c
+++ b/arch/arm/cpu/start.c
@@ -226,6 +226,9 @@ __noreturn void barebox_non_pbl_start(unsigned long membase,
mem_malloc_init((void *)malloc_start, (void *)malloc_end - 1);
+ if (IS_ENABLED(CONFIG_BOOTM_OPTEE))
+ of_add_reserve_entry(endmem - CONFIG_BOOTM_OPTEE_SIZE, endmem - 1);
+
pr_debug("starting barebox...\n");
start_barebox();
diff --git a/arch/arm/include/asm/armlinux.h b/arch/arm/include/asm/armlinux.h
index 135f11b..6af9896 100644
--- a/arch/arm/include/asm/armlinux.h
+++ b/arch/arm/include/asm/armlinux.h
@@ -40,6 +40,6 @@ struct image_data;
void start_linux(void *adr, int swap, unsigned long initrd_address,
unsigned long initrd_size, void *oftree,
- enum arm_security_state);
+ enum arm_security_state, void *optee);
#endif /* __ARCH_ARMLINUX_H */
diff --git a/arch/arm/include/asm/barebox-arm.h b/arch/arm/include/asm/barebox-arm.h
index e065b47..95765f9 100644
--- a/arch/arm/include/asm/barebox-arm.h
+++ b/arch/arm/include/asm/barebox-arm.h
@@ -109,6 +109,15 @@ void *barebox_arm_boot_dtb(void);
static inline unsigned long arm_mem_stack_top(unsigned long membase,
unsigned long endmem)
{
+ /*
+ * FIXME:
+ * OP-TEE expects to be executed somewhere at the end of RAM.
+ * Since we normally occupy all RAM at the end, move ourselves
+ * a bit lower.
+ */
+ if (IS_ENABLED(CONFIG_BOOTM_OPTEE))
+ return endmem - CONFIG_BOOTM_OPTEE_SIZE - SZ_64K;
+
return endmem - SZ_64K;
}
diff --git a/arch/arm/lib32/armlinux.c b/arch/arm/lib32/armlinux.c
index c970f02..a1e3375 100644
--- a/arch/arm/lib32/armlinux.c
+++ b/arch/arm/lib32/armlinux.c
@@ -258,9 +258,11 @@ static void setup_tags(unsigned long initrd_address,
}
+void start_kernel_optee(void *optee, void *kernel, void *oftree);
+
void start_linux(void *adr, int swap, unsigned long initrd_address,
unsigned long initrd_size, void *oftree,
- enum arm_security_state state)
+ enum arm_security_state state, void *optee)
{
void (*kernel)(int zero, int arch, void *params) = adr;
void *params = NULL;
@@ -294,5 +296,9 @@ void start_linux(void *adr, int swap, unsigned long initrd_address,
__asm__ __volatile__("mcr p15, 0, %0, c1, c0" :: "r" (reg));
}
- kernel(0, architecture, params);
+ if (optee) {
+ start_kernel_optee(optee, kernel, oftree);
+ } else {
+ kernel(0, architecture, params);
+ }
}
diff --git a/arch/arm/lib32/bootm.c b/arch/arm/lib32/bootm.c
index 4cf570e..69bf5bb 100644
--- a/arch/arm/lib32/bootm.c
+++ b/arch/arm/lib32/bootm.c
@@ -4,6 +4,7 @@
#include <command.h>
#include <driver.h>
#include <environment.h>
+#include <firmware.h>
#include <image.h>
#include <init.h>
#include <fs.h>
@@ -19,6 +20,7 @@
#include <binfmt.h>
#include <restart.h>
#include <globalvar.h>
+#include <tee/optee.h>
#include <asm/byteorder.h>
#include <asm/setup.h>
@@ -133,11 +135,96 @@ static int get_kernel_addresses(size_t image_size,
return 0;
}
+static int optee_verify_header_request_region(struct image_data *data, struct optee_header *hdr)
+{
+ int ret;
+
+ if (hdr->magic != OPTEE_MAGIC) {
+ printf("Invalid header magic 0x%08x, expected 0x%08x\n",
+ hdr->magic, OPTEE_MAGIC);
+ ret = -EINVAL;
+ return ret;
+ }
+
+ if (hdr->arch != OPTEE_ARCH_ARM32 || hdr->init_load_addr_hi) {
+ printf("Only 32bit supported\n");
+ ret = -EINVAL;
+ return ret;
+ }
+
+ data->tee_res = request_sdram_region("TEE", hdr->init_load_addr_lo, hdr->init_size);
+ if (!data->tee_res) {
+ ret = -EBUSY;
+ printf("Cannot request SDRAM region 0x%08x-0x%08x: %s\n",
+ hdr->init_load_addr_lo, hdr->init_load_addr_lo + hdr->init_size - 1,
+ strerror(-ret));
+ }
+
+ ret = 0;
+ return ret;
+}
+
+static int bootm_load_tee_from_firmware(struct image_data *data)
+{
+ int ret;
+
+ u8 *optee;
+ size_t optee_size;
+
+ struct optee_header hdr;
+
+ get_builtin_firmware(optee_bin, &optee, &optee_size);
+
+ memcpy(optee, &hdr, sizeof(hdr));
+
+ ret = optee_verify_header_request_region(data, &hdr);
+ if (ret < 0)
+ return ret;
+
+ memcpy(optee, (void *)data->tee_res->start, hdr.init_size);
+
+ printf("Copied optee binary to 0x%08x, size 0x%08x\n", data->tee_res->start, hdr.init_size);
+
+ ret = 0;
+ return ret;
+}
+
+static int bootm_load_tee_from_file(struct image_data *data)
+{
+ int fd, ret;
+ struct optee_header hdr;
+
+ fd = open(data->tee_file, O_RDONLY);
+ if (fd < 0)
+ return fd;
+
+ ret = read(fd, &hdr, sizeof(hdr));
+ if (ret < 0)
+ goto out;
+
+ optee_verify_header_request_region(data, &hdr);
+ if (ret < 0)
+ goto out;
+
+ ret = read_full(fd, (void *)data->tee_res->start, hdr.init_size);
+ if (ret < 0)
+ goto out;
+
+ printf("Read optee file to 0x%08x, size 0x%08x\n", data->tee_res->start, hdr.init_size);
+
+ ret = 0;
+out:
+ close(fd);
+
+ return ret;
+}
+
static int __do_bootm_linux(struct image_data *data, unsigned long free_mem,
int swap, void *fdt)
{
unsigned long kernel;
unsigned long initrd_start = 0, initrd_size = 0, initrd_end = 0;
+ void *tee;
enum arm_security_state state = bootm_arm_security_state();
void *fdt_load_address = NULL;
int ret;
@@ -189,6 +276,21 @@ static int __do_bootm_linux(struct image_data *data, unsigned long free_mem,
return ret;
}
+ if (IS_ENABLED(CONFIG_BOOTM_OPTEE)) {
+ if (data->tee_file) {
+ ret = bootm_load_tee_from_file(data);
+ if (ret)
+ return ret;
+ } else {
+ if (IS_ENABLED(CONFIG_BOOTM_OPTEE_FIRMWARE)) {
+ ret = bootm_load_tee_from_firmware(data);
+ if (ret)
+ return ret;
+ }
+ }
+ }
+
+
if (bootm_verbose(data)) {
printf("\nStarting kernel at 0x%08lx", kernel);
if (initrd_size)
@@ -210,8 +312,13 @@ static int __do_bootm_linux(struct image_data *data, unsigned long free_mem,
if (data->dryrun)
return 0;
+ if (data->tee_res)
+ tee = (void *)data->tee_res->start;
+ else
+ tee = NULL;
+
start_linux((void *)kernel, swap, initrd_start, initrd_size,
- fdt_load_address, state);
+ fdt_load_address, state, tee);
restart_machine();
diff --git a/arch/arm/lib32/bootu.c b/arch/arm/lib32/bootu.c
index d811da3..24c744d 100644
--- a/arch/arm/lib32/bootu.c
+++ b/arch/arm/lib32/bootu.c
@@ -26,7 +26,7 @@ static int do_bootu(int argc, char *argv[])
oftree = of_get_fixed_tree(NULL);
#endif
- start_linux(kernel, 0, 0, 0, oftree, ARM_STATE_SECURE);
+ start_linux(kernel, 0, 0, 0, oftree, ARM_STATE_SECURE, NULL);
return 1;
}
diff --git a/arch/arm/lib32/bootz.c b/arch/arm/lib32/bootz.c
index c0ffd93..a2a26ac 100644
--- a/arch/arm/lib32/bootz.c
+++ b/arch/arm/lib32/bootz.c
@@ -112,7 +112,7 @@ static int do_bootz(int argc, char *argv[])
oftree = of_get_fixed_tree(NULL);
#endif
- start_linux(zimage, swap, 0, 0, oftree, ARM_STATE_SECURE);
+ start_linux(zimage, swap, 0, 0, oftree, ARM_STATE_SECURE, NULL);
return 0;
diff --git a/commands/bootm.c b/commands/bootm.c
index c7cbdbe..100c2e9 100644
--- a/commands/bootm.c
+++ b/commands/bootm.c
@@ -45,7 +45,7 @@
#include <magicvar.h>
#include <asm-generic/memory_layout.h>
-#define BOOTM_OPTS_COMMON "sca:e:vo:fd"
+#define BOOTM_OPTS_COMMON "sca:e:vo:fdt:"
#ifdef CONFIG_BOOTM_INITRD
#define BOOTM_OPTS BOOTM_OPTS_COMMON "L:r:"
@@ -96,6 +96,9 @@ static int do_bootm(int argc, char *argv[])
case 'd':
data.dryrun = 1;
break;
+ case 't':
+ data.tee_file = optarg;
+ break;
default:
return COMMAND_ERROR_USAGE;
}
@@ -134,6 +137,9 @@ BAREBOX_CMD_HELP_OPT ("-e OFFS\t","entry point to the image relative to start (0
#ifdef CONFIG_OFTREE
BAREBOX_CMD_HELP_OPT ("-o DTB\t","specify open firmware device tree")
#endif
+#ifdef CONFIG_BOOTM_OPTEE
+BAREBOX_CMD_HELP_OPT ("-t TEE\t","specify TEE image")
+#endif
#ifdef CONFIG_BOOTM_VERBOSE
BAREBOX_CMD_HELP_OPT ("-v\t","verbose")
#endif
@@ -153,6 +159,9 @@ BAREBOX_CMD_START(bootm)
#ifdef CONFIG_BOOTM_VERBOSE
"v"
#endif
+#ifdef CONFIG_BOOTM_OPTEE
+ "t"
+#endif
"] IMAGE")
BAREBOX_CMD_GROUP(CMD_GRP_BOOT)
BAREBOX_CMD_HELP(cmd_bootm_help)
diff --git a/common/Kconfig b/common/Kconfig
index 2ad9215..d5858b4 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -659,6 +659,39 @@ config BOOTM_FORCE_SIGNED_IMAGES
are refused to boot. Effectively this means only FIT images can be booted
since they are the only supported image type that support signing.
+config BOOTM_OPTEE
+ bool
+ prompt "support booting OP-TEE"
+ depends on BOOTM && ARM
+ help
+ OP-TEE is a trusted execution environment (TEE). With this option
+ enabled barebox supports starting optee_os as part of the bootm command.
+ Instead of the kernel bootm starts the optee_os binary which then starts
+ the kernel in nonsecure mode. Pass the optee_os binary with the -t option
+ or in the global.bootm.tee variable.
+
+config BOOTM_OPTEE_SIZE
+ hex
+ default 0x02000000
+ prompt "OP-TEE Memory Size"
+ depends on BOOTM_OPTEE
+ help
+ Size to reserve in main memory for OP-TEE.
+ Can be smaller than the actual size used by OP-TEE, this is used to prevent
+ barebox from allocating memory in this area.
+
+config BOOTM_OPTEE_FIRMWARE
+ bool
+ prompt "load OP-TEE from builtin firmware"
+ depends on BOOTM_OPTEE
+ select FIRMWARE_OPTEE
+ help
+ Build OP-TEE into the barebox binary as a firmware image.
+ Place your tee.bin into the firmware directory or the external firmware
+ directory if defined.
+ Barebox will use the builtin optee.bin if a separate tee_file is not
+ defined.
+
config BLSPEC
depends on FLEXIBLE_BOOTARGS
depends on !SHELL_NONE
diff --git a/common/bootm.c b/common/bootm.c
index 36f6c41..d7232f6 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -58,6 +58,7 @@ void bootm_data_init_defaults(struct bootm_data *data)
data->initrd_address = UIMAGE_INVALID_ADDRESS;
data->os_address = UIMAGE_SOME_ADDRESS;
data->oftree_file = getenv_nonempty("global.bootm.oftree");
+ data->tee_file = getenv_nonempty("global.bootm.tee");
data->os_file = getenv_nonempty("global.bootm.image");
getenv_ul("global.bootm.image.loadaddr", &data->os_address);
getenv_ul("global.bootm.initrd.loadaddr", &data->initrd_address);
@@ -553,6 +554,8 @@ int bootm_boot(struct bootm_data *bootm_data)
bootm_image_name_and_part(bootm_data->os_file, &data->os_file, &data->os_part);
bootm_image_name_and_part(bootm_data->oftree_file, &data->oftree_file, &data->oftree_part);
bootm_image_name_and_part(bootm_data->initrd_file, &data->initrd_file, &data->initrd_part);
+ if (bootm_data->tee_file)
+ data->tee_file = xstrdup(bootm_data->tee_file);
data->verbose = bootm_data->verbose;
data->verify = bootm_data->verify;
data->force = bootm_data->force;
@@ -693,6 +696,7 @@ err_out:
free(data->os_file);
free(data->oftree_file);
free(data->initrd_file);
+ free(data->tee_file);
free(data);
return ret;
@@ -703,6 +707,7 @@ static int bootm_init(void)
globalvar_add_simple("bootm.image", NULL);
globalvar_add_simple("bootm.image.loadaddr", NULL);
globalvar_add_simple("bootm.oftree", NULL);
+ globalvar_add_simple("bootm.tee", NULL);
globalvar_add_simple_bool("bootm.appendroot", &bootm_appendroot);
if (IS_ENABLED(CONFIG_BOOTM_INITRD)) {
globalvar_add_simple("bootm.initrd", NULL);
@@ -727,6 +732,7 @@ BAREBOX_MAGICVAR_NAMED(global_bootm_image_loadaddr, global.bootm.image.loadaddr,
BAREBOX_MAGICVAR_NAMED(global_bootm_initrd, global.bootm.initrd, "bootm default initrd");
BAREBOX_MAGICVAR_NAMED(global_bootm_initrd_loadaddr, global.bootm.initrd.loadaddr, "bootm default initrd loadaddr");
BAREBOX_MAGICVAR_NAMED(global_bootm_oftree, global.bootm.oftree, "bootm default oftree");
+BAREBOX_MAGICVAR_NAMED(global_bootm_tee, global.bootm.tee, "bootm default tee image");
BAREBOX_MAGICVAR_NAMED(global_bootm_verify, global.bootm.verify, "bootm default verify level");
BAREBOX_MAGICVAR_NAMED(global_bootm_verbose, global.bootm.verbose, "bootm default verbosity level (0=quiet)");
BAREBOX_MAGICVAR_NAMED(global_bootm_appendroot, global.bootm.appendroot, "Add root= option to Kernel to mount rootfs from the device the Kernel comes from");
diff --git a/firmware/Kconfig b/firmware/Kconfig
index a6f79e8..9dc9b49 100644
--- a/firmware/Kconfig
+++ b/firmware/Kconfig
@@ -10,4 +10,7 @@ config FIRMWARE_IMX_LPDDR4_PMU_TRAIN
config FIRMWARE_IMX8MQ_ATF
bool
+config FIRMWARE_OPTEE
+ bool
+
endmenu
diff --git a/firmware/Makefile b/firmware/Makefile
index 7f4dc49..5606d71 100644
--- a/firmware/Makefile
+++ b/firmware/Makefile
@@ -11,6 +11,8 @@ firmware-$(CONFIG_FIRMWARE_IMX_LPDDR4_PMU_TRAIN) += \
firmware-$(CONFIG_FIRMWARE_IMX8MQ_ATF) += imx/imx8m-bl31.bin
+firmware-$(CONFIG_FIRMWARE_OPTEE) += optee.bin
+
# Create $(fwabs) from $(CONFIG_EXTRA_FIRMWARE_DIR) -- if it doesn't have a
# leading /, it's relative to $(srctree).
fwdir := $(subst $(quote),,$(CONFIG_EXTRA_FIRMWARE_DIR))
diff --git a/include/bootm.h b/include/bootm.h
index fdc73f7..5ce3318 100644
--- a/include/bootm.h
+++ b/include/bootm.h
@@ -16,6 +16,7 @@ struct bootm_data {
const char *os_file;
const char *initrd_file;
const char *oftree_file;
+ const char *tee_file;
int verbose;
enum bootm_verify verify;
bool force;
@@ -86,6 +87,8 @@ struct image_data {
* it.
*/
void *os_header;
+ char *tee_file;
+ struct resource *tee_res;
enum bootm_verify verify;
int verbose;
diff --git a/include/tee/optee.h b/include/tee/optee.h
new file mode 100644
index 0000000..8cfe06d
--- /dev/null
+++ b/include/tee/optee.h
@@ -0,0 +1,30 @@
+/*
+ * OP-TEE related definitions
+ *
+ * (C) Copyright 2016 Linaro Limited
+ * Andrew F. Davis <andrew.davis@xxxxxxxxxx>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#ifndef _OPTEE_H
+#define _OPTEE_H
+
+#define OPTEE_MAGIC 0x4554504f
+#define OPTEE_VERSION 1
+#define OPTEE_ARCH_ARM32 0
+#define OPTEE_ARCH_ARM64 1
+
+struct optee_header {
+ uint32_t magic;
+ uint8_t version;
+ uint8_t arch;
+ uint16_t flags;
+ uint32_t init_size;
+ uint32_t init_load_addr_hi;
+ uint32_t init_load_addr_lo;
+ uint32_t init_mem_usage;
+ uint32_t paged_size;
+};
+
+#endif /* _OPTEE_H */
--
git-series 0.9.1
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] ARM: Initial OP-TEE support
2019-01-29 11:10 ` [PATCH 1/1] ARM: Initial OP-TEE support Rouven Czerwinski
@ 2019-01-29 11:15 ` Rouven Czerwinski
2019-01-29 22:35 ` Andrey Smirnov
1 sibling, 0 replies; 6+ messages in thread
From: Rouven Czerwinski @ 2019-01-29 11:15 UTC (permalink / raw)
To: barebox
On Tue, 2019-01-29 at 12:10 +0100, Rouven Czerwinski wrote:
> From: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
>
> This adds initial support for OP-TEE, see https://www.op-tee.org/
>
> barebox starts in secure mode as usual. When booting a kernel
> the bootm code also loads the optee_os binary. Instead of jumping
> into the kernel barebox jumps into the optee_os binary and puts
> the kernel execution address into the lr register. OP-TEE then
> jumps into the kernel in nonsecure mode.
>
> The optee_os binary is passed with the -t option to bootm or
> with global.bootm.tee.
>
> Optionally OP-TEE can be compiled into barebox using the builtin
> firmware
> feature. Enable the Kconfig option and place or link your tee binary
> as
> optee.bin into the firmware directory.
>
> The amount of SDRAM which is kept free for OP-TEE is configurable.
>
> This patch was tested on a i.MX6 Nitrogen6x board.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> ---
> arch/arm/cpu/Makefile | 2 +-
> arch/arm/cpu/start-kernel-optee.S | 14 ++++-
> arch/arm/cpu/start.c | 3 +-
> arch/arm/include/asm/armlinux.h | 2 +-
> arch/arm/include/asm/barebox-arm.h | 9 ++-
> arch/arm/lib32/armlinux.c | 10 ++-
> arch/arm/lib32/bootm.c | 109
> +++++++++++++++++++++++++++++-
> arch/arm/lib32/bootu.c | 2 +-
> arch/arm/lib32/bootz.c | 2 +-
> commands/bootm.c | 11 ++-
> common/Kconfig | 33 +++++++++-
> common/bootm.c | 6 ++-
> firmware/Kconfig | 3 +-
> firmware/Makefile | 2 +-
> include/bootm.h | 3 +-
> include/tee/optee.h | 30 ++++++++-
> 16 files changed, 234 insertions(+), 7 deletions(-)
> create mode 100644 arch/arm/cpu/start-kernel-optee.S
> create mode 100644 include/tee/optee.h
>
> diff --git a/arch/arm/cpu/Makefile b/arch/arm/cpu/Makefile
> index a35db43..e6e74e0 100644
> --- a/arch/arm/cpu/Makefile
> +++ b/arch/arm/cpu/Makefile
> @@ -12,6 +12,8 @@ obj-y += start.o entry.o
>
> obj-pbl-y += setupc$(S64).o cache$(S64).o
>
> +obj-$(CONFIG_BOOTM_OPTEE) += start-kernel-optee.o
> +
> #
> # Any variants can be called as start-armxyz.S
> #
> diff --git a/arch/arm/cpu/start-kernel-optee.S b/arch/arm/cpu/start-
> kernel-optee.S
> new file mode 100644
> index 0000000..ce5ac17
> --- /dev/null
> +++ b/arch/arm/cpu/start-kernel-optee.S
> @@ -0,0 +1,14 @@
> +#include <linux/linkage.h>
> +
> +ENTRY(start_kernel_optee)
> + /*
> + * r0 = optee
> + * r1 = kernel
> + * r2 = oftree
> + */
> + mov r4, r0
> + mov r0, #0
> + mov lr, r1
> + mov r1, #0
> + bx r4
> +ENDPROC(start_kernel_optee)
> diff --git a/arch/arm/cpu/start.c b/arch/arm/cpu/start.c
> index 768fa9e..e78a820 100644
> --- a/arch/arm/cpu/start.c
> +++ b/arch/arm/cpu/start.c
> @@ -226,6 +226,9 @@ __noreturn void barebox_non_pbl_start(unsigned
> long membase,
>
> mem_malloc_init((void *)malloc_start, (void *)malloc_end -
> 1);
>
> + if (IS_ENABLED(CONFIG_BOOTM_OPTEE))
> + of_add_reserve_entry(endmem -
> CONFIG_BOOTM_OPTEE_SIZE, endmem - 1);
> +
> pr_debug("starting barebox...\n");
>
> start_barebox();
> diff --git a/arch/arm/include/asm/armlinux.h
> b/arch/arm/include/asm/armlinux.h
> index 135f11b..6af9896 100644
> --- a/arch/arm/include/asm/armlinux.h
> +++ b/arch/arm/include/asm/armlinux.h
> @@ -40,6 +40,6 @@ struct image_data;
>
> void start_linux(void *adr, int swap, unsigned long initrd_address,
> unsigned long initrd_size, void *oftree,
> - enum arm_security_state);
> + enum arm_security_state, void *optee);
>
> #endif /* __ARCH_ARMLINUX_H */
> diff --git a/arch/arm/include/asm/barebox-arm.h
> b/arch/arm/include/asm/barebox-arm.h
> index e065b47..95765f9 100644
> --- a/arch/arm/include/asm/barebox-arm.h
> +++ b/arch/arm/include/asm/barebox-arm.h
> @@ -109,6 +109,15 @@ void *barebox_arm_boot_dtb(void);
> static inline unsigned long arm_mem_stack_top(unsigned long membase,
> unsigned long endmem)
> {
> + /*
> + * FIXME:
> + * OP-TEE expects to be executed somewhere at the end of
> RAM.
> + * Since we normally occupy all RAM at the end, move
> ourselves
> + * a bit lower.
> + */
^
That FIXME is fixed but still in the code, will fix in v2.
> + if (IS_ENABLED(CONFIG_BOOTM_OPTEE))
> + return endmem - CONFIG_BOOTM_OPTEE_SIZE - SZ_64K;
> +
> return endmem - SZ_64K;
> }
>
> diff --git a/arch/arm/lib32/armlinux.c b/arch/arm/lib32/armlinux.c
> index c970f02..a1e3375 100644
> --- a/arch/arm/lib32/armlinux.c
> +++ b/arch/arm/lib32/armlinux.c
> @@ -258,9 +258,11 @@ static void setup_tags(unsigned long
> initrd_address,
>
> }
>
> +void start_kernel_optee(void *optee, void *kernel, void *oftree);
> +
> void start_linux(void *adr, int swap, unsigned long initrd_address,
> unsigned long initrd_size, void *oftree,
> - enum arm_security_state state)
> + enum arm_security_state state, void *optee)
> {
> void (*kernel)(int zero, int arch, void *params) = adr;
> void *params = NULL;
> @@ -294,5 +296,9 @@ void start_linux(void *adr, int swap, unsigned
> long initrd_address,
> __asm__ __volatile__("mcr p15, 0, %0, c1, c0" :: "r"
> (reg));
> }
>
> - kernel(0, architecture, params);
> + if (optee) {
> + start_kernel_optee(optee, kernel, oftree);
> + } else {
> + kernel(0, architecture, params);
> + }
> }
> diff --git a/arch/arm/lib32/bootm.c b/arch/arm/lib32/bootm.c
> index 4cf570e..69bf5bb 100644
> --- a/arch/arm/lib32/bootm.c
> +++ b/arch/arm/lib32/bootm.c
> @@ -4,6 +4,7 @@
> #include <command.h>
> #include <driver.h>
> #include <environment.h>
> +#include <firmware.h>
> #include <image.h>
> #include <init.h>
> #include <fs.h>
> @@ -19,6 +20,7 @@
> #include <binfmt.h>
> #include <restart.h>
> #include <globalvar.h>
> +#include <tee/optee.h>
>
> #include <asm/byteorder.h>
> #include <asm/setup.h>
> @@ -133,11 +135,96 @@ static int get_kernel_addresses(size_t
> image_size,
> return 0;
> }
>
> +static int optee_verify_header_request_region(struct image_data
> *data, struct optee_header *hdr)
> +{
> + int ret;
> +
> + if (hdr->magic != OPTEE_MAGIC) {
> + printf("Invalid header magic 0x%08x, expected
> 0x%08x\n",
> + hdr->magic, OPTEE_MAGIC);
> + ret = -EINVAL;
> + return ret;
> + }
> +
> + if (hdr->arch != OPTEE_ARCH_ARM32 || hdr->init_load_addr_hi)
> {
> + printf("Only 32bit supported\n");
> + ret = -EINVAL;
> + return ret;
> + }
> +
> + data->tee_res = request_sdram_region("TEE", hdr-
> >init_load_addr_lo, hdr->init_size);
> + if (!data->tee_res) {
> + ret = -EBUSY;
> + printf("Cannot request SDRAM region 0x%08x-0x%08x:
> %s\n",
> + hdr->init_load_addr_lo, hdr-
> >init_load_addr_lo + hdr->init_size - 1,
> + strerror(-ret));
> + }
> +
> + ret = 0;
> + return ret;
> +}
> +
> +static int bootm_load_tee_from_firmware(struct image_data *data)
> +{
> + int ret;
> +
> + u8 *optee;
> + size_t optee_size;
> +
> + struct optee_header hdr;
> +
> + get_builtin_firmware(optee_bin, &optee, &optee_size);
> +
> + memcpy(optee, &hdr, sizeof(hdr));
> +
> + ret = optee_verify_header_request_region(data, &hdr);
> + if (ret < 0)
> + return ret;
> +
> + memcpy(optee, (void *)data->tee_res->start, hdr.init_size);
> +
> + printf("Copied optee binary to 0x%08x, size 0x%08x\n", data-
> >tee_res->start, hdr.init_size);
> +
> + ret = 0;
> + return ret;
> +}
> +
> +static int bootm_load_tee_from_file(struct image_data *data)
> +{
> + int fd, ret;
> + struct optee_header hdr;
> +
> + fd = open(data->tee_file, O_RDONLY);
> + if (fd < 0)
> + return fd;
> +
> + ret = read(fd, &hdr, sizeof(hdr));
> + if (ret < 0)
> + goto out;
> +
> + optee_verify_header_request_region(data, &hdr);
> + if (ret < 0)
> + goto out;
> +
> + ret = read_full(fd, (void *)data->tee_res->start,
> hdr.init_size);
> + if (ret < 0)
> + goto out;
> +
> + printf("Read optee file to 0x%08x, size 0x%08x\n", data-
> >tee_res->start, hdr.init_size);
> +
> + ret = 0;
> +out:
> + close(fd);
> +
> + return ret;
> +}
> +
> static int __do_bootm_linux(struct image_data *data, unsigned long
> free_mem,
> int swap, void *fdt)
> {
> unsigned long kernel;
> unsigned long initrd_start = 0, initrd_size = 0, initrd_end
> = 0;
> + void *tee;
> enum arm_security_state state = bootm_arm_security_state();
> void *fdt_load_address = NULL;
> int ret;
> @@ -189,6 +276,21 @@ static int __do_bootm_linux(struct image_data
> *data, unsigned long free_mem,
> return ret;
> }
>
> + if (IS_ENABLED(CONFIG_BOOTM_OPTEE)) {
> + if (data->tee_file) {
> + ret = bootm_load_tee_from_file(data);
> + if (ret)
> + return ret;
> + } else {
> + if (IS_ENABLED(CONFIG_BOOTM_OPTEE_FIRMWARE))
> {
> + ret =
> bootm_load_tee_from_firmware(data);
> + if (ret)
> + return ret;
> + }
> + }
> + }
> +
> +
> if (bootm_verbose(data)) {
> printf("\nStarting kernel at 0x%08lx", kernel);
> if (initrd_size)
> @@ -210,8 +312,13 @@ static int __do_bootm_linux(struct image_data
> *data, unsigned long free_mem,
> if (data->dryrun)
> return 0;
>
> + if (data->tee_res)
> + tee = (void *)data->tee_res->start;
> + else
> + tee = NULL;
> +
> start_linux((void *)kernel, swap, initrd_start, initrd_size,
> - fdt_load_address, state);
> + fdt_load_address, state, tee);
>
> restart_machine();
>
> diff --git a/arch/arm/lib32/bootu.c b/arch/arm/lib32/bootu.c
> index d811da3..24c744d 100644
> --- a/arch/arm/lib32/bootu.c
> +++ b/arch/arm/lib32/bootu.c
> @@ -26,7 +26,7 @@ static int do_bootu(int argc, char *argv[])
> oftree = of_get_fixed_tree(NULL);
> #endif
>
> - start_linux(kernel, 0, 0, 0, oftree, ARM_STATE_SECURE);
> + start_linux(kernel, 0, 0, 0, oftree, ARM_STATE_SECURE,
> NULL);
>
> return 1;
> }
> diff --git a/arch/arm/lib32/bootz.c b/arch/arm/lib32/bootz.c
> index c0ffd93..a2a26ac 100644
> --- a/arch/arm/lib32/bootz.c
> +++ b/arch/arm/lib32/bootz.c
> @@ -112,7 +112,7 @@ static int do_bootz(int argc, char *argv[])
> oftree = of_get_fixed_tree(NULL);
> #endif
>
> - start_linux(zimage, swap, 0, 0, oftree, ARM_STATE_SECURE);
> + start_linux(zimage, swap, 0, 0, oftree, ARM_STATE_SECURE,
> NULL);
>
> return 0;
>
> diff --git a/commands/bootm.c b/commands/bootm.c
> index c7cbdbe..100c2e9 100644
> --- a/commands/bootm.c
> +++ b/commands/bootm.c
> @@ -45,7 +45,7 @@
> #include <magicvar.h>
> #include <asm-generic/memory_layout.h>
>
> -#define BOOTM_OPTS_COMMON "sca:e:vo:fd"
> +#define BOOTM_OPTS_COMMON "sca:e:vo:fdt:"
>
> #ifdef CONFIG_BOOTM_INITRD
> #define BOOTM_OPTS BOOTM_OPTS_COMMON "L:r:"
> @@ -96,6 +96,9 @@ static int do_bootm(int argc, char *argv[])
> case 'd':
> data.dryrun = 1;
> break;
> + case 't':
> + data.tee_file = optarg;
> + break;
> default:
> return COMMAND_ERROR_USAGE;
> }
> @@ -134,6 +137,9 @@ BAREBOX_CMD_HELP_OPT ("-e OFFS\t","entry point to
> the image relative to start (0
> #ifdef CONFIG_OFTREE
> BAREBOX_CMD_HELP_OPT ("-o DTB\t","specify open firmware device
> tree")
> #endif
> +#ifdef CONFIG_BOOTM_OPTEE
> +BAREBOX_CMD_HELP_OPT ("-t TEE\t","specify TEE image")
> +#endif
> #ifdef CONFIG_BOOTM_VERBOSE
> BAREBOX_CMD_HELP_OPT ("-v\t","verbose")
> #endif
> @@ -153,6 +159,9 @@ BAREBOX_CMD_START(bootm)
> #ifdef CONFIG_BOOTM_VERBOSE
> "v"
> #endif
> +#ifdef CONFIG_BOOTM_OPTEE
> + "t"
> +#endif
> "] IMAGE")
> BAREBOX_CMD_GROUP(CMD_GRP_BOOT)
> BAREBOX_CMD_HELP(cmd_bootm_help)
> diff --git a/common/Kconfig b/common/Kconfig
> index 2ad9215..d5858b4 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -659,6 +659,39 @@ config BOOTM_FORCE_SIGNED_IMAGES
> are refused to boot. Effectively this means only FIT
> images can be booted
> since they are the only supported image type that support
> signing.
>
> +config BOOTM_OPTEE
> + bool
> + prompt "support booting OP-TEE"
> + depends on BOOTM && ARM
> + help
> + OP-TEE is a trusted execution environment (TEE). With this
> option
> + enabled barebox supports starting optee_os as part of the
> bootm command.
> + Instead of the kernel bootm starts the optee_os binary
> which then starts
> + the kernel in nonsecure mode. Pass the optee_os binary
> with the -t option
> + or in the global.bootm.tee variable.
> +
> +config BOOTM_OPTEE_SIZE
> + hex
> + default 0x02000000
> + prompt "OP-TEE Memory Size"
> + depends on BOOTM_OPTEE
> + help
> + Size to reserve in main memory for OP-TEE.
> + Can be smaller than the actual size used by OP-TEE, this
> is used to prevent
> + barebox from allocating memory in this area.
> +
> +config BOOTM_OPTEE_FIRMWARE
> + bool
> + prompt "load OP-TEE from builtin firmware"
> + depends on BOOTM_OPTEE
> + select FIRMWARE_OPTEE
> + help
> + Build OP-TEE into the barebox binary as a firmware image.
> + Place your tee.bin into the firmware directory or the
> external firmware
> + directory if defined.
> + Barebox will use the builtin optee.bin if a separate
> tee_file is not
> + defined.
> +
> config BLSPEC
> depends on FLEXIBLE_BOOTARGS
> depends on !SHELL_NONE
> diff --git a/common/bootm.c b/common/bootm.c
> index 36f6c41..d7232f6 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -58,6 +58,7 @@ void bootm_data_init_defaults(struct bootm_data
> *data)
> data->initrd_address = UIMAGE_INVALID_ADDRESS;
> data->os_address = UIMAGE_SOME_ADDRESS;
> data->oftree_file = getenv_nonempty("global.bootm.oftree");
> + data->tee_file = getenv_nonempty("global.bootm.tee");
> data->os_file = getenv_nonempty("global.bootm.image");
> getenv_ul("global.bootm.image.loadaddr", &data->os_address);
> getenv_ul("global.bootm.initrd.loadaddr", &data-
> >initrd_address);
> @@ -553,6 +554,8 @@ int bootm_boot(struct bootm_data *bootm_data)
> bootm_image_name_and_part(bootm_data->os_file, &data-
> >os_file, &data->os_part);
> bootm_image_name_and_part(bootm_data->oftree_file, &data-
> >oftree_file, &data->oftree_part);
> bootm_image_name_and_part(bootm_data->initrd_file, &data-
> >initrd_file, &data->initrd_part);
> + if (bootm_data->tee_file)
> + data->tee_file = xstrdup(bootm_data->tee_file);
> data->verbose = bootm_data->verbose;
> data->verify = bootm_data->verify;
> data->force = bootm_data->force;
> @@ -693,6 +696,7 @@ err_out:
> free(data->os_file);
> free(data->oftree_file);
> free(data->initrd_file);
> + free(data->tee_file);
> free(data);
>
> return ret;
> @@ -703,6 +707,7 @@ static int bootm_init(void)
> globalvar_add_simple("bootm.image", NULL);
> globalvar_add_simple("bootm.image.loadaddr", NULL);
> globalvar_add_simple("bootm.oftree", NULL);
> + globalvar_add_simple("bootm.tee", NULL);
> globalvar_add_simple_bool("bootm.appendroot",
> &bootm_appendroot);
> if (IS_ENABLED(CONFIG_BOOTM_INITRD)) {
> globalvar_add_simple("bootm.initrd", NULL);
> @@ -727,6 +732,7 @@
> BAREBOX_MAGICVAR_NAMED(global_bootm_image_loadaddr,
> global.bootm.image.loadaddr,
> BAREBOX_MAGICVAR_NAMED(global_bootm_initrd, global.bootm.initrd,
> "bootm default initrd");
> BAREBOX_MAGICVAR_NAMED(global_bootm_initrd_loadaddr,
> global.bootm.initrd.loadaddr, "bootm default initrd loadaddr");
> BAREBOX_MAGICVAR_NAMED(global_bootm_oftree, global.bootm.oftree,
> "bootm default oftree");
> +BAREBOX_MAGICVAR_NAMED(global_bootm_tee, global.bootm.tee, "bootm
> default tee image");
> BAREBOX_MAGICVAR_NAMED(global_bootm_verify, global.bootm.verify,
> "bootm default verify level");
> BAREBOX_MAGICVAR_NAMED(global_bootm_verbose, global.bootm.verbose,
> "bootm default verbosity level (0=quiet)");
> BAREBOX_MAGICVAR_NAMED(global_bootm_appendroot,
> global.bootm.appendroot, "Add root= option to Kernel to mount rootfs
> from the device the Kernel comes from");
> diff --git a/firmware/Kconfig b/firmware/Kconfig
> index a6f79e8..9dc9b49 100644
> --- a/firmware/Kconfig
> +++ b/firmware/Kconfig
> @@ -10,4 +10,7 @@ config FIRMWARE_IMX_LPDDR4_PMU_TRAIN
> config FIRMWARE_IMX8MQ_ATF
> bool
>
> +config FIRMWARE_OPTEE
> + bool
> +
> endmenu
> diff --git a/firmware/Makefile b/firmware/Makefile
> index 7f4dc49..5606d71 100644
> --- a/firmware/Makefile
> +++ b/firmware/Makefile
> @@ -11,6 +11,8 @@ firmware-$(CONFIG_FIRMWARE_IMX_LPDDR4_PMU_TRAIN) +=
> \
>
> firmware-$(CONFIG_FIRMWARE_IMX8MQ_ATF) += imx/imx8m-bl31.bin
>
> +firmware-$(CONFIG_FIRMWARE_OPTEE) += optee.bin
> +
> # Create $(fwabs) from $(CONFIG_EXTRA_FIRMWARE_DIR) -- if it doesn't
> have a
> # leading /, it's relative to $(srctree).
> fwdir := $(subst $(quote),,$(CONFIG_EXTRA_FIRMWARE_DIR))
> diff --git a/include/bootm.h b/include/bootm.h
> index fdc73f7..5ce3318 100644
> --- a/include/bootm.h
> +++ b/include/bootm.h
> @@ -16,6 +16,7 @@ struct bootm_data {
> const char *os_file;
> const char *initrd_file;
> const char *oftree_file;
> + const char *tee_file;
> int verbose;
> enum bootm_verify verify;
> bool force;
> @@ -86,6 +87,8 @@ struct image_data {
> * it.
> */
> void *os_header;
> + char *tee_file;
> + struct resource *tee_res;
>
> enum bootm_verify verify;
> int verbose;
> diff --git a/include/tee/optee.h b/include/tee/optee.h
> new file mode 100644
> index 0000000..8cfe06d
> --- /dev/null
> +++ b/include/tee/optee.h
> @@ -0,0 +1,30 @@
> +/*
> + * OP-TEE related definitions
> + *
> + * (C) Copyright 2016 Linaro Limited
> + * Andrew F. Davis <andrew.davis@xxxxxxxxxx>
> + *
> + * SPDX-License-Identifier: BSD-2-Clause
> + */
> +
> +#ifndef _OPTEE_H
> +#define _OPTEE_H
> +
> +#define OPTEE_MAGIC 0x4554504f
> +#define OPTEE_VERSION 1
> +#define OPTEE_ARCH_ARM32 0
> +#define OPTEE_ARCH_ARM64 1
> +
> +struct optee_header {
> + uint32_t magic;
> + uint8_t version;
> + uint8_t arch;
> + uint16_t flags;
> + uint32_t init_size;
> + uint32_t init_load_addr_hi;
> + uint32_t init_load_addr_lo;
> + uint32_t init_mem_usage;
> + uint32_t paged_size;
> +};
> +
> +#endif /* _OPTEE_H */
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] ARM: Initial OP-TEE support
2019-01-29 11:10 ` [PATCH 1/1] ARM: Initial OP-TEE support Rouven Czerwinski
2019-01-29 11:15 ` Rouven Czerwinski
@ 2019-01-29 22:35 ` Andrey Smirnov
2019-01-30 7:16 ` Rouven Czerwinski
1 sibling, 1 reply; 6+ messages in thread
From: Andrey Smirnov @ 2019-01-29 22:35 UTC (permalink / raw)
To: Rouven Czerwinski; +Cc: Barebox List
On Tue, Jan 29, 2019 at 3:11 AM Rouven Czerwinski
<r.czerwinski@pengutronix.de> wrote:
>
> From: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
>
> This adds initial support for OP-TEE, see https://www.op-tee.org/
>
> barebox starts in secure mode as usual. When booting a kernel
> the bootm code also loads the optee_os binary. Instead of jumping
> into the kernel barebox jumps into the optee_os binary and puts
> the kernel execution address into the lr register. OP-TEE then
> jumps into the kernel in nonsecure mode.
>
> The optee_os binary is passed with the -t option to bootm or
> with global.bootm.tee.
>
> Optionally OP-TEE can be compiled into barebox using the builtin firmware
> feature. Enable the Kconfig option and place or link your tee binary as
> optee.bin into the firmware directory.
>
> The amount of SDRAM which is kept free for OP-TEE is configurable.
>
> This patch was tested on a i.MX6 Nitrogen6x board.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> ---
> arch/arm/cpu/Makefile | 2 +-
> arch/arm/cpu/start-kernel-optee.S | 14 ++++-
> arch/arm/cpu/start.c | 3 +-
> arch/arm/include/asm/armlinux.h | 2 +-
> arch/arm/include/asm/barebox-arm.h | 9 ++-
> arch/arm/lib32/armlinux.c | 10 ++-
> arch/arm/lib32/bootm.c | 109 +++++++++++++++++++++++++++++-
> arch/arm/lib32/bootu.c | 2 +-
> arch/arm/lib32/bootz.c | 2 +-
> commands/bootm.c | 11 ++-
> common/Kconfig | 33 +++++++++-
> common/bootm.c | 6 ++-
> firmware/Kconfig | 3 +-
> firmware/Makefile | 2 +-
> include/bootm.h | 3 +-
> include/tee/optee.h | 30 ++++++++-
> 16 files changed, 234 insertions(+), 7 deletions(-)
> create mode 100644 arch/arm/cpu/start-kernel-optee.S
> create mode 100644 include/tee/optee.h
>
> diff --git a/arch/arm/cpu/Makefile b/arch/arm/cpu/Makefile
> index a35db43..e6e74e0 100644
> --- a/arch/arm/cpu/Makefile
> +++ b/arch/arm/cpu/Makefile
> @@ -12,6 +12,8 @@ obj-y += start.o entry.o
>
> obj-pbl-y += setupc$(S64).o cache$(S64).o
>
> +obj-$(CONFIG_BOOTM_OPTEE) += start-kernel-optee.o
> +
> #
> # Any variants can be called as start-armxyz.S
> #
> diff --git a/arch/arm/cpu/start-kernel-optee.S b/arch/arm/cpu/start-kernel-optee.S
> new file mode 100644
> index 0000000..ce5ac17
> --- /dev/null
> +++ b/arch/arm/cpu/start-kernel-optee.S
> @@ -0,0 +1,14 @@
> +#include <linux/linkage.h>
> +
> +ENTRY(start_kernel_optee)
> + /*
> + * r0 = optee
> + * r1 = kernel
> + * r2 = oftree
> + */
> + mov r4, r0
> + mov r0, #0
> + mov lr, r1
> + mov r1, #0
> + bx r4
> +ENDPROC(start_kernel_optee)
> diff --git a/arch/arm/cpu/start.c b/arch/arm/cpu/start.c
> index 768fa9e..e78a820 100644
> --- a/arch/arm/cpu/start.c
> +++ b/arch/arm/cpu/start.c
> @@ -226,6 +226,9 @@ __noreturn void barebox_non_pbl_start(unsigned long membase,
>
> mem_malloc_init((void *)malloc_start, (void *)malloc_end - 1);
>
> + if (IS_ENABLED(CONFIG_BOOTM_OPTEE))
> + of_add_reserve_entry(endmem - CONFIG_BOOTM_OPTEE_SIZE, endmem - 1);
> +
> pr_debug("starting barebox...\n");
>
> start_barebox();
> diff --git a/arch/arm/include/asm/armlinux.h b/arch/arm/include/asm/armlinux.h
> index 135f11b..6af9896 100644
> --- a/arch/arm/include/asm/armlinux.h
> +++ b/arch/arm/include/asm/armlinux.h
> @@ -40,6 +40,6 @@ struct image_data;
>
> void start_linux(void *adr, int swap, unsigned long initrd_address,
> unsigned long initrd_size, void *oftree,
> - enum arm_security_state);
> + enum arm_security_state, void *optee);
>
> #endif /* __ARCH_ARMLINUX_H */
> diff --git a/arch/arm/include/asm/barebox-arm.h b/arch/arm/include/asm/barebox-arm.h
> index e065b47..95765f9 100644
> --- a/arch/arm/include/asm/barebox-arm.h
> +++ b/arch/arm/include/asm/barebox-arm.h
> @@ -109,6 +109,15 @@ void *barebox_arm_boot_dtb(void);
> static inline unsigned long arm_mem_stack_top(unsigned long membase,
> unsigned long endmem)
> {
> + /*
> + * FIXME:
> + * OP-TEE expects to be executed somewhere at the end of RAM.
> + * Since we normally occupy all RAM at the end, move ourselves
> + * a bit lower.
> + */
> + if (IS_ENABLED(CONFIG_BOOTM_OPTEE))
> + return endmem - CONFIG_BOOTM_OPTEE_SIZE - SZ_64K;
You can probably change it to:
if (IS_ENABLED(CONFIG_BOOTM_OPTEE))
endmem -= CONFIG_BOOTM_OPTEE_SIZE;
to avoid hard-coding SZ_64K in two places
> +
> return endmem - SZ_64K;
> }
>
> diff --git a/arch/arm/lib32/armlinux.c b/arch/arm/lib32/armlinux.c
> index c970f02..a1e3375 100644
> --- a/arch/arm/lib32/armlinux.c
> +++ b/arch/arm/lib32/armlinux.c
> @@ -258,9 +258,11 @@ static void setup_tags(unsigned long initrd_address,
>
> }
>
> +void start_kernel_optee(void *optee, void *kernel, void *oftree);
> +
> void start_linux(void *adr, int swap, unsigned long initrd_address,
> unsigned long initrd_size, void *oftree,
> - enum arm_security_state state)
> + enum arm_security_state state, void *optee)
> {
> void (*kernel)(int zero, int arch, void *params) = adr;
> void *params = NULL;
> @@ -294,5 +296,9 @@ void start_linux(void *adr, int swap, unsigned long initrd_address,
> __asm__ __volatile__("mcr p15, 0, %0, c1, c0" :: "r" (reg));
> }
>
> - kernel(0, architecture, params);
> + if (optee) {
> + start_kernel_optee(optee, kernel, oftree);
> + } else {
> + kernel(0, architecture, params);
> + }
Just out of curiosity, can we return back to this function in
non-secure mode and just do:
if (optee)
optee();
kernel(0, architecture, params);
?
> }
> diff --git a/arch/arm/lib32/bootm.c b/arch/arm/lib32/bootm.c
> index 4cf570e..69bf5bb 100644
> --- a/arch/arm/lib32/bootm.c
> +++ b/arch/arm/lib32/bootm.c
> @@ -4,6 +4,7 @@
> #include <command.h>
> #include <driver.h>
> #include <environment.h>
> +#include <firmware.h>
> #include <image.h>
> #include <init.h>
> #include <fs.h>
> @@ -19,6 +20,7 @@
> #include <binfmt.h>
> #include <restart.h>
> #include <globalvar.h>
> +#include <tee/optee.h>
>
> #include <asm/byteorder.h>
> #include <asm/setup.h>
> @@ -133,11 +135,96 @@ static int get_kernel_addresses(size_t image_size,
> return 0;
> }
>
> +static int optee_verify_header_request_region(struct image_data *data, struct optee_header *hdr)
> +{
> + int ret;
This function uses "ret" in some really superfluous way. I'd consider
just dropping "ret".
> +
> + if (hdr->magic != OPTEE_MAGIC) {
> + printf("Invalid header magic 0x%08x, expected 0x%08x\n",
> + hdr->magic, OPTEE_MAGIC);
> + ret = -EINVAL;
> + return ret;
> + }
> +
> + if (hdr->arch != OPTEE_ARCH_ARM32 || hdr->init_load_addr_hi) {
> + printf("Only 32bit supported\n");
> + ret = -EINVAL;
> + return ret;
> + }
> +
> + data->tee_res = request_sdram_region("TEE", hdr->init_load_addr_lo, hdr->init_size);
> + if (!data->tee_res) {
> + ret = -EBUSY;
Underlying call to __request_region() in request_sdram_region() can
also return -EINVAL if, for example, requested region is too big. I
don't think hard-coding -EBUSY, really adds any useful info here.
> + printf("Cannot request SDRAM region 0x%08x-0x%08x: %s\n",
> + hdr->init_load_addr_lo, hdr->init_load_addr_lo + hdr->init_size - 1,
> + strerror(-ret));
> + }
> +
> + ret = 0;
> + return ret;
> +}
> +
> +static int bootm_load_tee_from_firmware(struct image_data *data)
> +{
> + int ret;
> +
> + u8 *optee;
> + size_t optee_size;
> +
> + struct optee_header hdr;
> +
> + get_builtin_firmware(optee_bin, &optee, &optee_size);
> +
> + memcpy(optee, &hdr, sizeof(hdr));
> +
> + ret = optee_verify_header_request_region(data, &hdr);
> + if (ret < 0)
> + return ret;
> +
> + memcpy(optee, (void *)data->tee_res->start, hdr.init_size);
> +
> + printf("Copied optee binary to 0x%08x, size 0x%08x\n", data->tee_res->start, hdr.init_size);
> +
> + ret = 0;
> + return ret;
> +}
What if we instead do:
get_builtin_firmware(optee_bin, &optee, &optee_size);
add_mem_device("optee", optee, optee_size, 0);
and then handle that case via "/dev/optee" and
bootm_load_tee_from_file()? Is there any reason it wouldn't work?
Moreso, if that can be done we should consider just providing an API
to handle built-in firmware case and delegating that job to individual
board files, so that we have the freedom of having per-board version
of OP-TEE firmware.
> +
> +static int bootm_load_tee_from_file(struct image_data *data)
> +{
> + int fd, ret;
> + struct optee_header hdr;
> +
> + fd = open(data->tee_file, O_RDONLY);
> + if (fd < 0)
> + return fd;
Purely optional, but please consider changing this code to be a bit
more POSIX compliant and avoid relying on getting proper error codes
from various API calls. Calls to open(), read() and read_full() should
just check if the return value is negative and return -errno if so. On
the bright side this should allow you to get rid of "ret", since you
can just do:
if (read(...) < 0)
goto out;
...
out:
...
return -errno
> +
> + ret = read(fd, &hdr, sizeof(hdr));
It's very unlikely to happen, but, strictly speaking, read() doesn't
guarantee to read all sizeof(hdr) bytes, so I'd use read_full() here
as well.
> + if (ret < 0)
> + goto out;
> +
> + optee_verify_header_request_region(data, &hdr);
> + if (ret < 0)
> + goto out;
> +
> + ret = read_full(fd, (void *)data->tee_res->start, hdr.init_size);
> + if (ret < 0)
> + goto out;
> +
> + printf("Read optee file to 0x%08x, size 0x%08x\n", data->tee_res->start, hdr.init_size);
> +
> + ret = 0;
> +out:
> + close(fd);
> +
> + return ret;
> +}
> +
> static int __do_bootm_linux(struct image_data *data, unsigned long free_mem,
> int swap, void *fdt)
> {
> unsigned long kernel;
> unsigned long initrd_start = 0, initrd_size = 0, initrd_end = 0;
> + void *tee;
> enum arm_security_state state = bootm_arm_security_state();
> void *fdt_load_address = NULL;
> int ret;
> @@ -189,6 +276,21 @@ static int __do_bootm_linux(struct image_data *data, unsigned long free_mem,
> return ret;
> }
>
> + if (IS_ENABLED(CONFIG_BOOTM_OPTEE)) {
> + if (data->tee_file) {
> + ret = bootm_load_tee_from_file(data);
> + if (ret)
> + return ret;
> + } else {
> + if (IS_ENABLED(CONFIG_BOOTM_OPTEE_FIRMWARE)) {
> + ret = bootm_load_tee_from_firmware(data);
> + if (ret)
> + return ret;
> + }
> + }
> + }
> +
> +
> if (bootm_verbose(data)) {
> printf("\nStarting kernel at 0x%08lx", kernel);
> if (initrd_size)
> @@ -210,8 +312,13 @@ static int __do_bootm_linux(struct image_data *data, unsigned long free_mem,
> if (data->dryrun)
> return 0;
>
> + if (data->tee_res)
> + tee = (void *)data->tee_res->start;
> + else
> + tee = NULL;
> +
> start_linux((void *)kernel, swap, initrd_start, initrd_size,
> - fdt_load_address, state);
> + fdt_load_address, state, tee);
>
> restart_machine();
>
> diff --git a/arch/arm/lib32/bootu.c b/arch/arm/lib32/bootu.c
> index d811da3..24c744d 100644
> --- a/arch/arm/lib32/bootu.c
> +++ b/arch/arm/lib32/bootu.c
> @@ -26,7 +26,7 @@ static int do_bootu(int argc, char *argv[])
> oftree = of_get_fixed_tree(NULL);
> #endif
>
> - start_linux(kernel, 0, 0, 0, oftree, ARM_STATE_SECURE);
> + start_linux(kernel, 0, 0, 0, oftree, ARM_STATE_SECURE, NULL);
>
> return 1;
> }
> diff --git a/arch/arm/lib32/bootz.c b/arch/arm/lib32/bootz.c
> index c0ffd93..a2a26ac 100644
> --- a/arch/arm/lib32/bootz.c
> +++ b/arch/arm/lib32/bootz.c
> @@ -112,7 +112,7 @@ static int do_bootz(int argc, char *argv[])
> oftree = of_get_fixed_tree(NULL);
> #endif
>
> - start_linux(zimage, swap, 0, 0, oftree, ARM_STATE_SECURE);
> + start_linux(zimage, swap, 0, 0, oftree, ARM_STATE_SECURE, NULL);
>
> return 0;
>
> diff --git a/commands/bootm.c b/commands/bootm.c
> index c7cbdbe..100c2e9 100644
> --- a/commands/bootm.c
> +++ b/commands/bootm.c
> @@ -45,7 +45,7 @@
> #include <magicvar.h>
> #include <asm-generic/memory_layout.h>
>
> -#define BOOTM_OPTS_COMMON "sca:e:vo:fd"
> +#define BOOTM_OPTS_COMMON "sca:e:vo:fdt:"
>
> #ifdef CONFIG_BOOTM_INITRD
> #define BOOTM_OPTS BOOTM_OPTS_COMMON "L:r:"
> @@ -96,6 +96,9 @@ static int do_bootm(int argc, char *argv[])
> case 'd':
> data.dryrun = 1;
> break;
> + case 't':
> + data.tee_file = optarg;
> + break;
> default:
> return COMMAND_ERROR_USAGE;
> }
> @@ -134,6 +137,9 @@ BAREBOX_CMD_HELP_OPT ("-e OFFS\t","entry point to the image relative to start (0
> #ifdef CONFIG_OFTREE
> BAREBOX_CMD_HELP_OPT ("-o DTB\t","specify open firmware device tree")
> #endif
> +#ifdef CONFIG_BOOTM_OPTEE
> +BAREBOX_CMD_HELP_OPT ("-t TEE\t","specify TEE image")
> +#endif
> #ifdef CONFIG_BOOTM_VERBOSE
> BAREBOX_CMD_HELP_OPT ("-v\t","verbose")
> #endif
> @@ -153,6 +159,9 @@ BAREBOX_CMD_START(bootm)
> #ifdef CONFIG_BOOTM_VERBOSE
> "v"
> #endif
> +#ifdef CONFIG_BOOTM_OPTEE
> + "t"
> +#endif
> "] IMAGE")
> BAREBOX_CMD_GROUP(CMD_GRP_BOOT)
> BAREBOX_CMD_HELP(cmd_bootm_help)
> diff --git a/common/Kconfig b/common/Kconfig
> index 2ad9215..d5858b4 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -659,6 +659,39 @@ config BOOTM_FORCE_SIGNED_IMAGES
> are refused to boot. Effectively this means only FIT images can be booted
> since they are the only supported image type that support signing.
>
> +config BOOTM_OPTEE
> + bool
> + prompt "support booting OP-TEE"
> + depends on BOOTM && ARM
> + help
> + OP-TEE is a trusted execution environment (TEE). With this option
> + enabled barebox supports starting optee_os as part of the bootm command.
> + Instead of the kernel bootm starts the optee_os binary which then starts
> + the kernel in nonsecure mode. Pass the optee_os binary with the -t option
> + or in the global.bootm.tee variable.
> +
> +config BOOTM_OPTEE_SIZE
> + hex
> + default 0x02000000
> + prompt "OP-TEE Memory Size"
> + depends on BOOTM_OPTEE
> + help
> + Size to reserve in main memory for OP-TEE.
> + Can be smaller than the actual size used by OP-TEE, this is used to prevent
> + barebox from allocating memory in this area.
> +
> +config BOOTM_OPTEE_FIRMWARE
> + bool
> + prompt "load OP-TEE from builtin firmware"
> + depends on BOOTM_OPTEE
> + select FIRMWARE_OPTEE
> + help
> + Build OP-TEE into the barebox binary as a firmware image.
> + Place your tee.bin into the firmware directory or the external firmware
> + directory if defined.
> + Barebox will use the builtin optee.bin if a separate tee_file is not
> + defined.
> +
> config BLSPEC
> depends on FLEXIBLE_BOOTARGS
> depends on !SHELL_NONE
> diff --git a/common/bootm.c b/common/bootm.c
> index 36f6c41..d7232f6 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -58,6 +58,7 @@ void bootm_data_init_defaults(struct bootm_data *data)
> data->initrd_address = UIMAGE_INVALID_ADDRESS;
> data->os_address = UIMAGE_SOME_ADDRESS;
> data->oftree_file = getenv_nonempty("global.bootm.oftree");
> + data->tee_file = getenv_nonempty("global.bootm.tee");
> data->os_file = getenv_nonempty("global.bootm.image");
> getenv_ul("global.bootm.image.loadaddr", &data->os_address);
> getenv_ul("global.bootm.initrd.loadaddr", &data->initrd_address);
> @@ -553,6 +554,8 @@ int bootm_boot(struct bootm_data *bootm_data)
> bootm_image_name_and_part(bootm_data->os_file, &data->os_file, &data->os_part);
> bootm_image_name_and_part(bootm_data->oftree_file, &data->oftree_file, &data->oftree_part);
> bootm_image_name_and_part(bootm_data->initrd_file, &data->initrd_file, &data->initrd_part);
> + if (bootm_data->tee_file)
> + data->tee_file = xstrdup(bootm_data->tee_file);
> data->verbose = bootm_data->verbose;
> data->verify = bootm_data->verify;
> data->force = bootm_data->force;
> @@ -693,6 +696,7 @@ err_out:
> free(data->os_file);
> free(data->oftree_file);
> free(data->initrd_file);
> + free(data->tee_file);
> free(data);
>
> return ret;
> @@ -703,6 +707,7 @@ static int bootm_init(void)
> globalvar_add_simple("bootm.image", NULL);
> globalvar_add_simple("bootm.image.loadaddr", NULL);
> globalvar_add_simple("bootm.oftree", NULL);
> + globalvar_add_simple("bootm.tee", NULL);
> globalvar_add_simple_bool("bootm.appendroot", &bootm_appendroot);
> if (IS_ENABLED(CONFIG_BOOTM_INITRD)) {
> globalvar_add_simple("bootm.initrd", NULL);
> @@ -727,6 +732,7 @@ BAREBOX_MAGICVAR_NAMED(global_bootm_image_loadaddr, global.bootm.image.loadaddr,
> BAREBOX_MAGICVAR_NAMED(global_bootm_initrd, global.bootm.initrd, "bootm default initrd");
> BAREBOX_MAGICVAR_NAMED(global_bootm_initrd_loadaddr, global.bootm.initrd.loadaddr, "bootm default initrd loadaddr");
> BAREBOX_MAGICVAR_NAMED(global_bootm_oftree, global.bootm.oftree, "bootm default oftree");
> +BAREBOX_MAGICVAR_NAMED(global_bootm_tee, global.bootm.tee, "bootm default tee image");
> BAREBOX_MAGICVAR_NAMED(global_bootm_verify, global.bootm.verify, "bootm default verify level");
> BAREBOX_MAGICVAR_NAMED(global_bootm_verbose, global.bootm.verbose, "bootm default verbosity level (0=quiet)");
> BAREBOX_MAGICVAR_NAMED(global_bootm_appendroot, global.bootm.appendroot, "Add root= option to Kernel to mount rootfs from the device the Kernel comes from");
> diff --git a/firmware/Kconfig b/firmware/Kconfig
> index a6f79e8..9dc9b49 100644
> --- a/firmware/Kconfig
> +++ b/firmware/Kconfig
> @@ -10,4 +10,7 @@ config FIRMWARE_IMX_LPDDR4_PMU_TRAIN
> config FIRMWARE_IMX8MQ_ATF
> bool
>
> +config FIRMWARE_OPTEE
> + bool
> +
> endmenu
> diff --git a/firmware/Makefile b/firmware/Makefile
> index 7f4dc49..5606d71 100644
> --- a/firmware/Makefile
> +++ b/firmware/Makefile
> @@ -11,6 +11,8 @@ firmware-$(CONFIG_FIRMWARE_IMX_LPDDR4_PMU_TRAIN) += \
>
> firmware-$(CONFIG_FIRMWARE_IMX8MQ_ATF) += imx/imx8m-bl31.bin
>
> +firmware-$(CONFIG_FIRMWARE_OPTEE) += optee.bin
> +
> # Create $(fwabs) from $(CONFIG_EXTRA_FIRMWARE_DIR) -- if it doesn't have a
> # leading /, it's relative to $(srctree).
> fwdir := $(subst $(quote),,$(CONFIG_EXTRA_FIRMWARE_DIR))
> diff --git a/include/bootm.h b/include/bootm.h
> index fdc73f7..5ce3318 100644
> --- a/include/bootm.h
> +++ b/include/bootm.h
> @@ -16,6 +16,7 @@ struct bootm_data {
> const char *os_file;
> const char *initrd_file;
> const char *oftree_file;
> + const char *tee_file;
> int verbose;
> enum bootm_verify verify;
> bool force;
> @@ -86,6 +87,8 @@ struct image_data {
> * it.
> */
> void *os_header;
> + char *tee_file;
> + struct resource *tee_res;
>
> enum bootm_verify verify;
> int verbose;
> diff --git a/include/tee/optee.h b/include/tee/optee.h
> new file mode 100644
> index 0000000..8cfe06d
> --- /dev/null
> +++ b/include/tee/optee.h
> @@ -0,0 +1,30 @@
> +/*
> + * OP-TEE related definitions
> + *
> + * (C) Copyright 2016 Linaro Limited
> + * Andrew F. Davis <andrew.davis@xxxxxxxxxx>
> + *
> + * SPDX-License-Identifier: BSD-2-Clause
> + */
> +
> +#ifndef _OPTEE_H
> +#define _OPTEE_H
> +
> +#define OPTEE_MAGIC 0x4554504f
> +#define OPTEE_VERSION 1
> +#define OPTEE_ARCH_ARM32 0
> +#define OPTEE_ARCH_ARM64 1
> +
> +struct optee_header {
> + uint32_t magic;
> + uint8_t version;
> + uint8_t arch;
> + uint16_t flags;
> + uint32_t init_size;
> + uint32_t init_load_addr_hi;
> + uint32_t init_load_addr_lo;
> + uint32_t init_mem_usage;
> + uint32_t paged_size;
> +};
> +
> +#endif /* _OPTEE_H */
> --
> git-series 0.9.1
>
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] ARM: Initial OP-TEE support
2019-01-29 22:35 ` Andrey Smirnov
@ 2019-01-30 7:16 ` Rouven Czerwinski
2019-01-30 23:24 ` Andrey Smirnov
0 siblings, 1 reply; 6+ messages in thread
From: Rouven Czerwinski @ 2019-01-30 7:16 UTC (permalink / raw)
To: Andrey Smirnov; +Cc: Barebox List
On Tue, 2019-01-29 at 14:35 -0800, Andrey Smirnov wrote:
> On Tue, Jan 29, 2019 at 3:11 AM Rouven Czerwinski
> <r.czerwinski@pengutronix.de> wrote:
> >
> > From: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
> >
> > This adds initial support for OP-TEE, see https://www.op-tee.org/
> >
> > barebox starts in secure mode as usual. When booting a kernel
> > the bootm code also loads the optee_os binary. Instead of jumping
> > into the kernel barebox jumps into the optee_os binary and puts
> > the kernel execution address into the lr register. OP-TEE then
> > jumps into the kernel in nonsecure mode.
> >
> > The optee_os binary is passed with the -t option to bootm or
> > with global.bootm.tee.
> >
> > Optionally OP-TEE can be compiled into barebox using the builtin
> > firmware
> > feature. Enable the Kconfig option and place or link your tee
> > binary as
> > optee.bin into the firmware directory.
> >
> > The amount of SDRAM which is kept free for OP-TEE is configurable.
> >
> > This patch was tested on a i.MX6 Nitrogen6x board.
> >
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > ---
> > arch/arm/cpu/Makefile | 2 +-
> > arch/arm/cpu/start-kernel-optee.S | 14 ++++-
> > arch/arm/cpu/start.c | 3 +-
> > arch/arm/include/asm/armlinux.h | 2 +-
> > arch/arm/include/asm/barebox-arm.h | 9 ++-
> > arch/arm/lib32/armlinux.c | 10 ++-
> > arch/arm/lib32/bootm.c | 109
> > +++++++++++++++++++++++++++++-
> > arch/arm/lib32/bootu.c | 2 +-
> > arch/arm/lib32/bootz.c | 2 +-
> > commands/bootm.c | 11 ++-
> > common/Kconfig | 33 +++++++++-
> > common/bootm.c | 6 ++-
> > firmware/Kconfig | 3 +-
> > firmware/Makefile | 2 +-
> > include/bootm.h | 3 +-
> > include/tee/optee.h | 30 ++++++++-
> > 16 files changed, 234 insertions(+), 7 deletions(-)
> > create mode 100644 arch/arm/cpu/start-kernel-optee.S
> > create mode 100644 include/tee/optee.h
> >
> > diff --git a/arch/arm/cpu/Makefile b/arch/arm/cpu/Makefile
> > index a35db43..e6e74e0 100644
> > --- a/arch/arm/cpu/Makefile
> > +++ b/arch/arm/cpu/Makefile
> > @@ -12,6 +12,8 @@ obj-y += start.o entry.o
> >
> > obj-pbl-y += setupc$(S64).o cache$(S64).o
> >
> > +obj-$(CONFIG_BOOTM_OPTEE) += start-kernel-optee.o
> > +
> > #
> > # Any variants can be called as start-armxyz.S
> > #
> > diff --git a/arch/arm/cpu/start-kernel-optee.S
> > b/arch/arm/cpu/start-kernel-optee.S
> > new file mode 100644
> > index 0000000..ce5ac17
> > --- /dev/null
> > +++ b/arch/arm/cpu/start-kernel-optee.S
> > @@ -0,0 +1,14 @@
> > +#include <linux/linkage.h>
> > +
> > +ENTRY(start_kernel_optee)
> > + /*
> > + * r0 = optee
> > + * r1 = kernel
> > + * r2 = oftree
> > + */
> > + mov r4, r0
> > + mov r0, #0
> > + mov lr, r1
> > + mov r1, #0
> > + bx r4
> > +ENDPROC(start_kernel_optee)
> > diff --git a/arch/arm/cpu/start.c b/arch/arm/cpu/start.c
> > index 768fa9e..e78a820 100644
> > --- a/arch/arm/cpu/start.c
> > +++ b/arch/arm/cpu/start.c
> > @@ -226,6 +226,9 @@ __noreturn void barebox_non_pbl_start(unsigned
> > long membase,
> >
> > mem_malloc_init((void *)malloc_start, (void *)malloc_end -
> > 1);
> >
> > + if (IS_ENABLED(CONFIG_BOOTM_OPTEE))
> > + of_add_reserve_entry(endmem -
> > CONFIG_BOOTM_OPTEE_SIZE, endmem - 1);
> > +
> > pr_debug("starting barebox...\n");
> >
> > start_barebox();
> > diff --git a/arch/arm/include/asm/armlinux.h
> > b/arch/arm/include/asm/armlinux.h
> > index 135f11b..6af9896 100644
> > --- a/arch/arm/include/asm/armlinux.h
> > +++ b/arch/arm/include/asm/armlinux.h
> > @@ -40,6 +40,6 @@ struct image_data;
> >
> > void start_linux(void *adr, int swap, unsigned long
> > initrd_address,
> > unsigned long initrd_size, void *oftree,
> > - enum arm_security_state);
> > + enum arm_security_state, void *optee);
> >
> > #endif /* __ARCH_ARMLINUX_H */
> > diff --git a/arch/arm/include/asm/barebox-arm.h
> > b/arch/arm/include/asm/barebox-arm.h
> > index e065b47..95765f9 100644
> > --- a/arch/arm/include/asm/barebox-arm.h
> > +++ b/arch/arm/include/asm/barebox-arm.h
> > @@ -109,6 +109,15 @@ void *barebox_arm_boot_dtb(void);
> > static inline unsigned long arm_mem_stack_top(unsigned long
> > membase,
> > unsigned long endmem)
> > {
> > + /*
> > + * FIXME:
> > + * OP-TEE expects to be executed somewhere at the end of
> > RAM.
> > + * Since we normally occupy all RAM at the end, move
> > ourselves
> > + * a bit lower.
> > + */
> > + if (IS_ENABLED(CONFIG_BOOTM_OPTEE))
> > + return endmem - CONFIG_BOOTM_OPTEE_SIZE - SZ_64K;
>
> You can probably change it to:
>
> if (IS_ENABLED(CONFIG_BOOTM_OPTEE))
> endmem -= CONFIG_BOOTM_OPTEE_SIZE;
>
> to avoid hard-coding SZ_64K in two places
>
sensible, will fix.
> > +
> > return endmem - SZ_64K;
> > }
> >
> > diff --git a/arch/arm/lib32/armlinux.c b/arch/arm/lib32/armlinux.c
> > index c970f02..a1e3375 100644
> > --- a/arch/arm/lib32/armlinux.c
> > +++ b/arch/arm/lib32/armlinux.c
> > @@ -258,9 +258,11 @@ static void setup_tags(unsigned long
> > initrd_address,
> >
> > }
> >
> > +void start_kernel_optee(void *optee, void *kernel, void *oftree);
> > +
> > void start_linux(void *adr, int swap, unsigned long
> > initrd_address,
> > unsigned long initrd_size, void *oftree,
> > - enum arm_security_state state)
> > + enum arm_security_state state, void *optee)
> > {
> > void (*kernel)(int zero, int arch, void *params) = adr;
> > void *params = NULL;
> > @@ -294,5 +296,9 @@ void start_linux(void *adr, int swap, unsigned
> > long initrd_address,
> > __asm__ __volatile__("mcr p15, 0, %0, c1, c0" ::
> > "r" (reg));
> > }
> >
> > - kernel(0, architecture, params);
> > + if (optee) {
> > + start_kernel_optee(optee, kernel, oftree);
> > + } else {
> > + kernel(0, architecture, params);
> > + }
>
> Just out of curiosity, can we return back to this function in
> non-secure mode and just do:
>
> if (optee)
> optee();
> kernel(0, architecture, params);
>
> ?
I don't quite get your intention here. We can't start OP-TEE in non-
secure mode and at least currently OP-TEE starts without returning back
to barebox.
>
> > }
> > diff --git a/arch/arm/lib32/bootm.c b/arch/arm/lib32/bootm.c
> > index 4cf570e..69bf5bb 100644
> > --- a/arch/arm/lib32/bootm.c
> > +++ b/arch/arm/lib32/bootm.c
> > @@ -4,6 +4,7 @@
> > #include <command.h>
> > #include <driver.h>
> > #include <environment.h>
> > +#include <firmware.h>
> > #include <image.h>
> > #include <init.h>
> > #include <fs.h>
> > @@ -19,6 +20,7 @@
> > #include <binfmt.h>
> > #include <restart.h>
> > #include <globalvar.h>
> > +#include <tee/optee.h>
> >
> > #include <asm/byteorder.h>
> > #include <asm/setup.h>
> > @@ -133,11 +135,96 @@ static int get_kernel_addresses(size_t
> > image_size,
> > return 0;
> > }
> >
> > +static int optee_verify_header_request_region(struct image_data
> > *data, struct optee_header *hdr)
> > +{
> > + int ret;
>
> This function uses "ret" in some really superfluous way. I'd consider
> just dropping "ret".
yes, will fix.
>
> > +
> > + if (hdr->magic != OPTEE_MAGIC) {
> > + printf("Invalid header magic 0x%08x, expected
> > 0x%08x\n",
> > + hdr->magic, OPTEE_MAGIC);
> > + ret = -EINVAL;
> > + return ret;
> > + }
> > +
> > + if (hdr->arch != OPTEE_ARCH_ARM32 || hdr-
> > >init_load_addr_hi) {
> > + printf("Only 32bit supported\n");
> > + ret = -EINVAL;
> > + return ret;
> > + }
> > +
> > + data->tee_res = request_sdram_region("TEE", hdr-
> > >init_load_addr_lo, hdr->init_size);
> > + if (!data->tee_res) {
> > + ret = -EBUSY;
>
> Underlying call to __request_region() in request_sdram_region() can
> also return -EINVAL if, for example, requested region is too big. I
> don't think hard-coding -EBUSY, really adds any useful info here.
will fix.
> > + printf("Cannot request SDRAM region 0x%08x-0x%08x:
> > %s\n",
> > + hdr->init_load_addr_lo, hdr-
> > >init_load_addr_lo + hdr->init_size - 1,
> > + strerror(-ret));
> > + }
> > +
> > + ret = 0;
> > + return ret;
> > +}
> > +
> > +static int bootm_load_tee_from_firmware(struct image_data *data)
> > +{
> > + int ret;
> > +
> > + u8 *optee;
> > + size_t optee_size;
> > +
> > + struct optee_header hdr;
> > +
> > + get_builtin_firmware(optee_bin, &optee, &optee_size);
> > +
> > + memcpy(optee, &hdr, sizeof(hdr));
> > +
> > + ret = optee_verify_header_request_region(data, &hdr);
> > + if (ret < 0)
> > + return ret;
> > +
> > + memcpy(optee, (void *)data->tee_res->start, hdr.init_size);
> > +
> > + printf("Copied optee binary to 0x%08x, size 0x%08x\n",
> > data->tee_res->start, hdr.init_size);
> > +
> > + ret = 0;
> > + return ret;
> > +}
>
> What if we instead do:
>
> get_builtin_firmware(optee_bin, &optee, &optee_size);
> add_mem_device("optee", optee, optee_size, 0);
>
> and then handle that case via "/dev/optee" and
> bootm_load_tee_from_file()? Is there any reason it wouldn't work?
Thats a good idea to expose the idea and simplify the tee loading code.
> Moreso, if that can be done we should consider just providing an API
> to handle built-in firmware case and delegating that job to
> individual
> board files, so that we have the freedom of having per-board version
> of OP-TEE firmware.
The idea here was to not include the OP-TEE binary into the barebox
upstream repository and only provide the option to include optee for
production devices.
I can't think of a use case where we want to include OP-TEE into an
upstream barebox, since the features for OP-TEE (included early TAs,
memory size,...) are quite specific for the use case.
> > +static int bootm_load_tee_from_file(struct image_data *data)
> > +{
> > + int fd, ret;
> > + struct optee_header hdr;
> > +
> > + fd = open(data->tee_file, O_RDONLY);
> > + if (fd < 0)
> > + return fd;
>
> Purely optional, but please consider changing this code to be a bit
> more POSIX compliant and avoid relying on getting proper error codes
> from various API calls. Calls to open(), read() and read_full()
> should
> just check if the return value is negative and return -errno if so.
> On
> the bright side this should allow you to get rid of "ret", since you
> can just do:
>
> if (read(...) < 0)
> goto out;
>
> ...
>
> out:
> ...
> return -errno
>
sounds good, will do.
> > +
> > + ret = read(fd, &hdr, sizeof(hdr));
>
> It's very unlikely to happen, but, strictly speaking, read() doesn't
> guarantee to read all sizeof(hdr) bytes, so I'd use read_full() here
> as well.
will fix.
>
> > + if (ret < 0)
> > + goto out;
> > +
> > + optee_verify_header_request_region(data, &hdr);
> > + if (ret < 0)
> > + goto out;
> > +
> > + ret = read_full(fd, (void *)data->tee_res->start,
> > hdr.init_size);
> > + if (ret < 0)
> > + goto out;
> > +
> > + printf("Read optee file to 0x%08x, size 0x%08x\n", data-
> > >tee_res->start, hdr.init_size);
> > +
> > + ret = 0;
> > +out:
> > + close(fd);
> > +
> > + return ret;
> > +}
> > +
> > static int __do_bootm_linux(struct image_data *data, unsigned long
> > free_mem,
> > int swap, void *fdt)
> > {
> > unsigned long kernel;
> > unsigned long initrd_start = 0, initrd_size = 0, initrd_end
> > = 0;
> > + void *tee;
> > enum arm_security_state state = bootm_arm_security_state();
> > void *fdt_load_address = NULL;
> > int ret;
> > @@ -189,6 +276,21 @@ static int __do_bootm_linux(struct image_data
> > *data, unsigned long free_mem,
> > return ret;
> > }
> >
> > + if (IS_ENABLED(CONFIG_BOOTM_OPTEE)) {
> > + if (data->tee_file) {
> > + ret = bootm_load_tee_from_file(data);
> > + if (ret)
> > + return ret;
> > + } else {
> > + if
> > (IS_ENABLED(CONFIG_BOOTM_OPTEE_FIRMWARE)) {
> > + ret =
> > bootm_load_tee_from_firmware(data);
> > + if (ret)
> > + return ret;
> > + }
> > + }
> > + }
> > +
> > +
> > if (bootm_verbose(data)) {
> > printf("\nStarting kernel at 0x%08lx", kernel);
> > if (initrd_size)
> > @@ -210,8 +312,13 @@ static int __do_bootm_linux(struct image_data
> > *data, unsigned long free_mem,
> > if (data->dryrun)
> > return 0;
> >
> > + if (data->tee_res)
> > + tee = (void *)data->tee_res->start;
> > + else
> > + tee = NULL;
> > +
> > start_linux((void *)kernel, swap, initrd_start,
> > initrd_size,
> > - fdt_load_address, state);
> > + fdt_load_address, state, tee);
> >
> > restart_machine();
> >
> > diff --git a/arch/arm/lib32/bootu.c b/arch/arm/lib32/bootu.c
> > index d811da3..24c744d 100644
> > --- a/arch/arm/lib32/bootu.c
> > +++ b/arch/arm/lib32/bootu.c
> > @@ -26,7 +26,7 @@ static int do_bootu(int argc, char *argv[])
> > oftree = of_get_fixed_tree(NULL);
> > #endif
> >
> > - start_linux(kernel, 0, 0, 0, oftree, ARM_STATE_SECURE);
> > + start_linux(kernel, 0, 0, 0, oftree, ARM_STATE_SECURE,
> > NULL);
> >
> > return 1;
> > }
> > diff --git a/arch/arm/lib32/bootz.c b/arch/arm/lib32/bootz.c
> > index c0ffd93..a2a26ac 100644
> > --- a/arch/arm/lib32/bootz.c
> > +++ b/arch/arm/lib32/bootz.c
> > @@ -112,7 +112,7 @@ static int do_bootz(int argc, char *argv[])
> > oftree = of_get_fixed_tree(NULL);
> > #endif
> >
> > - start_linux(zimage, swap, 0, 0, oftree, ARM_STATE_SECURE);
> > + start_linux(zimage, swap, 0, 0, oftree, ARM_STATE_SECURE,
> > NULL);
> >
> > return 0;
> >
> > diff --git a/commands/bootm.c b/commands/bootm.c
> > index c7cbdbe..100c2e9 100644
> > --- a/commands/bootm.c
> > +++ b/commands/bootm.c
> > @@ -45,7 +45,7 @@
> > #include <magicvar.h>
> > #include <asm-generic/memory_layout.h>
> >
> > -#define BOOTM_OPTS_COMMON "sca:e:vo:fd"
> > +#define BOOTM_OPTS_COMMON "sca:e:vo:fdt:"
> >
> > #ifdef CONFIG_BOOTM_INITRD
> > #define BOOTM_OPTS BOOTM_OPTS_COMMON "L:r:"
> > @@ -96,6 +96,9 @@ static int do_bootm(int argc, char *argv[])
> > case 'd':
> > data.dryrun = 1;
> > break;
> > + case 't':
> > + data.tee_file = optarg;
> > + break;
> > default:
> > return COMMAND_ERROR_USAGE;
> > }
> > @@ -134,6 +137,9 @@ BAREBOX_CMD_HELP_OPT ("-e OFFS\t","entry point
> > to the image relative to start (0
> > #ifdef CONFIG_OFTREE
> > BAREBOX_CMD_HELP_OPT ("-o DTB\t","specify open firmware device
> > tree")
> > #endif
> > +#ifdef CONFIG_BOOTM_OPTEE
> > +BAREBOX_CMD_HELP_OPT ("-t TEE\t","specify TEE image")
> > +#endif
> > #ifdef CONFIG_BOOTM_VERBOSE
> > BAREBOX_CMD_HELP_OPT ("-v\t","verbose")
> > #endif
> > @@ -153,6 +159,9 @@ BAREBOX_CMD_START(bootm)
> > #ifdef CONFIG_BOOTM_VERBOSE
> > "v"
> > #endif
> > +#ifdef CONFIG_BOOTM_OPTEE
> > + "t"
> > +#endif
> > "] IMAGE")
> > BAREBOX_CMD_GROUP(CMD_GRP_BOOT)
> > BAREBOX_CMD_HELP(cmd_bootm_help)
> > diff --git a/common/Kconfig b/common/Kconfig
> > index 2ad9215..d5858b4 100644
> > --- a/common/Kconfig
> > +++ b/common/Kconfig
> > @@ -659,6 +659,39 @@ config BOOTM_FORCE_SIGNED_IMAGES
> > are refused to boot. Effectively this means only FIT
> > images can be booted
> > since they are the only supported image type that support
> > signing.
> >
> > +config BOOTM_OPTEE
> > + bool
> > + prompt "support booting OP-TEE"
> > + depends on BOOTM && ARM
> > + help
> > + OP-TEE is a trusted execution environment (TEE). With
> > this option
> > + enabled barebox supports starting optee_os as part of the
> > bootm command.
> > + Instead of the kernel bootm starts the optee_os binary
> > which then starts
> > + the kernel in nonsecure mode. Pass the optee_os binary
> > with the -t option
> > + or in the global.bootm.tee variable.
> > +
> > +config BOOTM_OPTEE_SIZE
> > + hex
> > + default 0x02000000
> > + prompt "OP-TEE Memory Size"
> > + depends on BOOTM_OPTEE
> > + help
> > + Size to reserve in main memory for OP-TEE.
> > + Can be smaller than the actual size used by OP-TEE, this
> > is used to prevent
> > + barebox from allocating memory in this area.
> > +
> > +config BOOTM_OPTEE_FIRMWARE
> > + bool
> > + prompt "load OP-TEE from builtin firmware"
> > + depends on BOOTM_OPTEE
> > + select FIRMWARE_OPTEE
> > + help
> > + Build OP-TEE into the barebox binary as a firmware image.
> > + Place your tee.bin into the firmware directory or the
> > external firmware
> > + directory if defined.
> > + Barebox will use the builtin optee.bin if a separate
> > tee_file is not
> > + defined.
> > +
> > config BLSPEC
> > depends on FLEXIBLE_BOOTARGS
> > depends on !SHELL_NONE
> > diff --git a/common/bootm.c b/common/bootm.c
> > index 36f6c41..d7232f6 100644
> > --- a/common/bootm.c
> > +++ b/common/bootm.c
> > @@ -58,6 +58,7 @@ void bootm_data_init_defaults(struct bootm_data
> > *data)
> > data->initrd_address = UIMAGE_INVALID_ADDRESS;
> > data->os_address = UIMAGE_SOME_ADDRESS;
> > data->oftree_file = getenv_nonempty("global.bootm.oftree");
> > + data->tee_file = getenv_nonempty("global.bootm.tee");
> > data->os_file = getenv_nonempty("global.bootm.image");
> > getenv_ul("global.bootm.image.loadaddr", &data-
> > >os_address);
> > getenv_ul("global.bootm.initrd.loadaddr", &data-
> > >initrd_address);
> > @@ -553,6 +554,8 @@ int bootm_boot(struct bootm_data *bootm_data)
> > bootm_image_name_and_part(bootm_data->os_file, &data-
> > >os_file, &data->os_part);
> > bootm_image_name_and_part(bootm_data->oftree_file, &data-
> > >oftree_file, &data->oftree_part);
> > bootm_image_name_and_part(bootm_data->initrd_file, &data-
> > >initrd_file, &data->initrd_part);
> > + if (bootm_data->tee_file)
> > + data->tee_file = xstrdup(bootm_data->tee_file);
> > data->verbose = bootm_data->verbose;
> > data->verify = bootm_data->verify;
> > data->force = bootm_data->force;
> > @@ -693,6 +696,7 @@ err_out:
> > free(data->os_file);
> > free(data->oftree_file);
> > free(data->initrd_file);
> > + free(data->tee_file);
> > free(data);
> >
> > return ret;
> > @@ -703,6 +707,7 @@ static int bootm_init(void)
> > globalvar_add_simple("bootm.image", NULL);
> > globalvar_add_simple("bootm.image.loadaddr", NULL);
> > globalvar_add_simple("bootm.oftree", NULL);
> > + globalvar_add_simple("bootm.tee", NULL);
> > globalvar_add_simple_bool("bootm.appendroot",
> > &bootm_appendroot);
> > if (IS_ENABLED(CONFIG_BOOTM_INITRD)) {
> > globalvar_add_simple("bootm.initrd", NULL);
> > @@ -727,6 +732,7 @@
> > BAREBOX_MAGICVAR_NAMED(global_bootm_image_loadaddr,
> > global.bootm.image.loadaddr,
> > BAREBOX_MAGICVAR_NAMED(global_bootm_initrd, global.bootm.initrd,
> > "bootm default initrd");
> > BAREBOX_MAGICVAR_NAMED(global_bootm_initrd_loadaddr,
> > global.bootm.initrd.loadaddr, "bootm default initrd loadaddr");
> > BAREBOX_MAGICVAR_NAMED(global_bootm_oftree, global.bootm.oftree,
> > "bootm default oftree");
> > +BAREBOX_MAGICVAR_NAMED(global_bootm_tee, global.bootm.tee, "bootm
> > default tee image");
> > BAREBOX_MAGICVAR_NAMED(global_bootm_verify, global.bootm.verify,
> > "bootm default verify level");
> > BAREBOX_MAGICVAR_NAMED(global_bootm_verbose, global.bootm.verbose,
> > "bootm default verbosity level (0=quiet)");
> > BAREBOX_MAGICVAR_NAMED(global_bootm_appendroot,
> > global.bootm.appendroot, "Add root= option to Kernel to mount
> > rootfs from the device the Kernel comes from");
> > diff --git a/firmware/Kconfig b/firmware/Kconfig
> > index a6f79e8..9dc9b49 100644
> > --- a/firmware/Kconfig
> > +++ b/firmware/Kconfig
> > @@ -10,4 +10,7 @@ config FIRMWARE_IMX_LPDDR4_PMU_TRAIN
> > config FIRMWARE_IMX8MQ_ATF
> > bool
> >
> > +config FIRMWARE_OPTEE
> > + bool
> > +
> > endmenu
> > diff --git a/firmware/Makefile b/firmware/Makefile
> > index 7f4dc49..5606d71 100644
> > --- a/firmware/Makefile
> > +++ b/firmware/Makefile
> > @@ -11,6 +11,8 @@ firmware-$(CONFIG_FIRMWARE_IMX_LPDDR4_PMU_TRAIN)
> > += \
> >
> > firmware-$(CONFIG_FIRMWARE_IMX8MQ_ATF) += imx/imx8m-bl31.bin
> >
> > +firmware-$(CONFIG_FIRMWARE_OPTEE) += optee.bin
> > +
> > # Create $(fwabs) from $(CONFIG_EXTRA_FIRMWARE_DIR) -- if it
> > doesn't have a
> > # leading /, it's relative to $(srctree).
> > fwdir := $(subst $(quote),,$(CONFIG_EXTRA_FIRMWARE_DIR))
> > diff --git a/include/bootm.h b/include/bootm.h
> > index fdc73f7..5ce3318 100644
> > --- a/include/bootm.h
> > +++ b/include/bootm.h
> > @@ -16,6 +16,7 @@ struct bootm_data {
> > const char *os_file;
> > const char *initrd_file;
> > const char *oftree_file;
> > + const char *tee_file;
> > int verbose;
> > enum bootm_verify verify;
> > bool force;
> > @@ -86,6 +87,8 @@ struct image_data {
> > * it.
> > */
> > void *os_header;
> > + char *tee_file;
> > + struct resource *tee_res;
> >
> > enum bootm_verify verify;
> > int verbose;
> > diff --git a/include/tee/optee.h b/include/tee/optee.h
> > new file mode 100644
> > index 0000000..8cfe06d
> > --- /dev/null
> > +++ b/include/tee/optee.h
> > @@ -0,0 +1,30 @@
> > +/*
> > + * OP-TEE related definitions
> > + *
> > + * (C) Copyright 2016 Linaro Limited
> > + * Andrew F. Davis <andrew.davis@xxxxxxxxxx>
> > + *
> > + * SPDX-License-Identifier: BSD-2-Clause
Just a note here, I talked to the OP-TEE Maintainers on Github [1], and
they are willing to dual license the struct definitions, so we can
still provide barebox as purely GPL.
[1]: https://github.com/OP-TEE/optee_os/issues/2771
> > + */
> > +
> > +#ifndef _OPTEE_H
> > +#define _OPTEE_H
> > +
> > +#define OPTEE_MAGIC 0x4554504f
> > +#define OPTEE_VERSION 1
> > +#define OPTEE_ARCH_ARM32 0
> > +#define OPTEE_ARCH_ARM64 1
> > +
> > +struct optee_header {
> > + uint32_t magic;
> > + uint8_t version;
> > + uint8_t arch;
> > + uint16_t flags;
> > + uint32_t init_size;
> > + uint32_t init_load_addr_hi;
> > + uint32_t init_load_addr_lo;
> > + uint32_t init_mem_usage;
> > + uint32_t paged_size;
> > +};
> > +
> > +#endif /* _OPTEE_H */
> > --
> > git-series 0.9.1
> >
> > _______________________________________________
> > barebox mailing list
> > barebox@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/barebox
>
>
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] ARM: Initial OP-TEE support
2019-01-30 7:16 ` Rouven Czerwinski
@ 2019-01-30 23:24 ` Andrey Smirnov
0 siblings, 0 replies; 6+ messages in thread
From: Andrey Smirnov @ 2019-01-30 23:24 UTC (permalink / raw)
To: Rouven Czerwinski; +Cc: Barebox List
On Tue, Jan 29, 2019 at 11:16 PM Rouven Czerwinski
<r.czerwinski@pengutronix.de> wrote:
>
> On Tue, 2019-01-29 at 14:35 -0800, Andrey Smirnov wrote:
> > On Tue, Jan 29, 2019 at 3:11 AM Rouven Czerwinski
> > <r.czerwinski@pengutronix.de> wrote:
> > >
> > > From: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
> > >
> > > This adds initial support for OP-TEE, see https://www.op-tee.org/
> > >
> > > barebox starts in secure mode as usual. When booting a kernel
> > > the bootm code also loads the optee_os binary. Instead of jumping
> > > into the kernel barebox jumps into the optee_os binary and puts
> > > the kernel execution address into the lr register. OP-TEE then
> > > jumps into the kernel in nonsecure mode.
> > >
> > > The optee_os binary is passed with the -t option to bootm or
> > > with global.bootm.tee.
> > >
> > > Optionally OP-TEE can be compiled into barebox using the builtin
> > > firmware
> > > feature. Enable the Kconfig option and place or link your tee
> > > binary as
> > > optee.bin into the firmware directory.
> > >
> > > The amount of SDRAM which is kept free for OP-TEE is configurable.
> > >
> > > This patch was tested on a i.MX6 Nitrogen6x board.
> > >
> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > > ---
> > > arch/arm/cpu/Makefile | 2 +-
> > > arch/arm/cpu/start-kernel-optee.S | 14 ++++-
> > > arch/arm/cpu/start.c | 3 +-
> > > arch/arm/include/asm/armlinux.h | 2 +-
> > > arch/arm/include/asm/barebox-arm.h | 9 ++-
> > > arch/arm/lib32/armlinux.c | 10 ++-
> > > arch/arm/lib32/bootm.c | 109
> > > +++++++++++++++++++++++++++++-
> > > arch/arm/lib32/bootu.c | 2 +-
> > > arch/arm/lib32/bootz.c | 2 +-
> > > commands/bootm.c | 11 ++-
> > > common/Kconfig | 33 +++++++++-
> > > common/bootm.c | 6 ++-
> > > firmware/Kconfig | 3 +-
> > > firmware/Makefile | 2 +-
> > > include/bootm.h | 3 +-
> > > include/tee/optee.h | 30 ++++++++-
> > > 16 files changed, 234 insertions(+), 7 deletions(-)
> > > create mode 100644 arch/arm/cpu/start-kernel-optee.S
> > > create mode 100644 include/tee/optee.h
> > >
> > > diff --git a/arch/arm/cpu/Makefile b/arch/arm/cpu/Makefile
> > > index a35db43..e6e74e0 100644
> > > --- a/arch/arm/cpu/Makefile
> > > +++ b/arch/arm/cpu/Makefile
> > > @@ -12,6 +12,8 @@ obj-y += start.o entry.o
> > >
> > > obj-pbl-y += setupc$(S64).o cache$(S64).o
> > >
> > > +obj-$(CONFIG_BOOTM_OPTEE) += start-kernel-optee.o
> > > +
> > > #
> > > # Any variants can be called as start-armxyz.S
> > > #
> > > diff --git a/arch/arm/cpu/start-kernel-optee.S
> > > b/arch/arm/cpu/start-kernel-optee.S
> > > new file mode 100644
> > > index 0000000..ce5ac17
> > > --- /dev/null
> > > +++ b/arch/arm/cpu/start-kernel-optee.S
> > > @@ -0,0 +1,14 @@
> > > +#include <linux/linkage.h>
> > > +
> > > +ENTRY(start_kernel_optee)
> > > + /*
> > > + * r0 = optee
> > > + * r1 = kernel
> > > + * r2 = oftree
> > > + */
> > > + mov r4, r0
> > > + mov r0, #0
> > > + mov lr, r1
> > > + mov r1, #0
> > > + bx r4
> > > +ENDPROC(start_kernel_optee)
> > > diff --git a/arch/arm/cpu/start.c b/arch/arm/cpu/start.c
> > > index 768fa9e..e78a820 100644
> > > --- a/arch/arm/cpu/start.c
> > > +++ b/arch/arm/cpu/start.c
> > > @@ -226,6 +226,9 @@ __noreturn void barebox_non_pbl_start(unsigned
> > > long membase,
> > >
> > > mem_malloc_init((void *)malloc_start, (void *)malloc_end -
> > > 1);
> > >
> > > + if (IS_ENABLED(CONFIG_BOOTM_OPTEE))
> > > + of_add_reserve_entry(endmem -
> > > CONFIG_BOOTM_OPTEE_SIZE, endmem - 1);
> > > +
> > > pr_debug("starting barebox...\n");
> > >
> > > start_barebox();
> > > diff --git a/arch/arm/include/asm/armlinux.h
> > > b/arch/arm/include/asm/armlinux.h
> > > index 135f11b..6af9896 100644
> > > --- a/arch/arm/include/asm/armlinux.h
> > > +++ b/arch/arm/include/asm/armlinux.h
> > > @@ -40,6 +40,6 @@ struct image_data;
> > >
> > > void start_linux(void *adr, int swap, unsigned long
> > > initrd_address,
> > > unsigned long initrd_size, void *oftree,
> > > - enum arm_security_state);
> > > + enum arm_security_state, void *optee);
> > >
> > > #endif /* __ARCH_ARMLINUX_H */
> > > diff --git a/arch/arm/include/asm/barebox-arm.h
> > > b/arch/arm/include/asm/barebox-arm.h
> > > index e065b47..95765f9 100644
> > > --- a/arch/arm/include/asm/barebox-arm.h
> > > +++ b/arch/arm/include/asm/barebox-arm.h
> > > @@ -109,6 +109,15 @@ void *barebox_arm_boot_dtb(void);
> > > static inline unsigned long arm_mem_stack_top(unsigned long
> > > membase,
> > > unsigned long endmem)
> > > {
> > > + /*
> > > + * FIXME:
> > > + * OP-TEE expects to be executed somewhere at the end of
> > > RAM.
> > > + * Since we normally occupy all RAM at the end, move
> > > ourselves
> > > + * a bit lower.
> > > + */
> > > + if (IS_ENABLED(CONFIG_BOOTM_OPTEE))
> > > + return endmem - CONFIG_BOOTM_OPTEE_SIZE - SZ_64K;
> >
> > You can probably change it to:
> >
> > if (IS_ENABLED(CONFIG_BOOTM_OPTEE))
> > endmem -= CONFIG_BOOTM_OPTEE_SIZE;
> >
> > to avoid hard-coding SZ_64K in two places
> >
> sensible, will fix.
>
> > > +
> > > return endmem - SZ_64K;
> > > }
> > >
> > > diff --git a/arch/arm/lib32/armlinux.c b/arch/arm/lib32/armlinux.c
> > > index c970f02..a1e3375 100644
> > > --- a/arch/arm/lib32/armlinux.c
> > > +++ b/arch/arm/lib32/armlinux.c
> > > @@ -258,9 +258,11 @@ static void setup_tags(unsigned long
> > > initrd_address,
> > >
> > > }
> > >
> > > +void start_kernel_optee(void *optee, void *kernel, void *oftree);
> > > +
> > > void start_linux(void *adr, int swap, unsigned long
> > > initrd_address,
> > > unsigned long initrd_size, void *oftree,
> > > - enum arm_security_state state)
> > > + enum arm_security_state state, void *optee)
> > > {
> > > void (*kernel)(int zero, int arch, void *params) = adr;
> > > void *params = NULL;
> > > @@ -294,5 +296,9 @@ void start_linux(void *adr, int swap, unsigned
> > > long initrd_address,
> > > __asm__ __volatile__("mcr p15, 0, %0, c1, c0" ::
> > > "r" (reg));
> > > }
> > >
> > > - kernel(0, architecture, params);
> > > + if (optee) {
> > > + start_kernel_optee(optee, kernel, oftree);
> > > + } else {
> > > + kernel(0, architecture, params);
> > > + }
> >
> > Just out of curiosity, can we return back to this function in
> > non-secure mode and just do:
> >
> > if (optee)
> > optee();
> > kernel(0, architecture, params);
> >
> > ?
> I don't quite get your intention here. We can't start OP-TEE in non-
> secure mode and at least currently OP-TEE starts without returning back
> to barebox.
>
AFAICT the reason start_kernel_optee() exists is to initialize LR with
desired return address and facilitate starting kernel without
returning to Barebox. What I am trying to figure out is why of if the
"without returning" part is important. Can the OP-TEE start sequence
be modified to just execute OP-TEE, return back to the original caller
and then jump to kernel in a separate step? IOW, can
start_kernel_optee() be dropped and OP-TEE initialized via a regular
C-call?
> >
> > > }
> > > diff --git a/arch/arm/lib32/bootm.c b/arch/arm/lib32/bootm.c
> > > index 4cf570e..69bf5bb 100644
> > > --- a/arch/arm/lib32/bootm.c
> > > +++ b/arch/arm/lib32/bootm.c
> > > @@ -4,6 +4,7 @@
> > > #include <command.h>
> > > #include <driver.h>
> > > #include <environment.h>
> > > +#include <firmware.h>
> > > #include <image.h>
> > > #include <init.h>
> > > #include <fs.h>
> > > @@ -19,6 +20,7 @@
> > > #include <binfmt.h>
> > > #include <restart.h>
> > > #include <globalvar.h>
> > > +#include <tee/optee.h>
> > >
> > > #include <asm/byteorder.h>
> > > #include <asm/setup.h>
> > > @@ -133,11 +135,96 @@ static int get_kernel_addresses(size_t
> > > image_size,
> > > return 0;
> > > }
> > >
> > > +static int optee_verify_header_request_region(struct image_data
> > > *data, struct optee_header *hdr)
> > > +{
> > > + int ret;
> >
> > This function uses "ret" in some really superfluous way. I'd consider
> > just dropping "ret".
> yes, will fix.
>
> >
> > > +
> > > + if (hdr->magic != OPTEE_MAGIC) {
> > > + printf("Invalid header magic 0x%08x, expected
> > > 0x%08x\n",
> > > + hdr->magic, OPTEE_MAGIC);
> > > + ret = -EINVAL;
> > > + return ret;
> > > + }
> > > +
> > > + if (hdr->arch != OPTEE_ARCH_ARM32 || hdr-
> > > >init_load_addr_hi) {
> > > + printf("Only 32bit supported\n");
> > > + ret = -EINVAL;
> > > + return ret;
> > > + }
> > > +
> > > + data->tee_res = request_sdram_region("TEE", hdr-
> > > >init_load_addr_lo, hdr->init_size);
> > > + if (!data->tee_res) {
> > > + ret = -EBUSY;
> >
> > Underlying call to __request_region() in request_sdram_region() can
> > also return -EINVAL if, for example, requested region is too big. I
> > don't think hard-coding -EBUSY, really adds any useful info here.
> will fix.
>
> > > + printf("Cannot request SDRAM region 0x%08x-0x%08x:
> > > %s\n",
> > > + hdr->init_load_addr_lo, hdr-
> > > >init_load_addr_lo + hdr->init_size - 1,
> > > + strerror(-ret));
> > > + }
> > > +
> > > + ret = 0;
> > > + return ret;
> > > +}
> > > +
> > > +static int bootm_load_tee_from_firmware(struct image_data *data)
> > > +{
> > > + int ret;
> > > +
> > > + u8 *optee;
> > > + size_t optee_size;
> > > +
> > > + struct optee_header hdr;
> > > +
> > > + get_builtin_firmware(optee_bin, &optee, &optee_size);
> > > +
> > > + memcpy(optee, &hdr, sizeof(hdr));
> > > +
> > > + ret = optee_verify_header_request_region(data, &hdr);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + memcpy(optee, (void *)data->tee_res->start, hdr.init_size);
> > > +
> > > + printf("Copied optee binary to 0x%08x, size 0x%08x\n",
> > > data->tee_res->start, hdr.init_size);
> > > +
> > > + ret = 0;
> > > + return ret;
> > > +}
> >
> > What if we instead do:
> >
> > get_builtin_firmware(optee_bin, &optee, &optee_size);
> > add_mem_device("optee", optee, optee_size, 0);
> >
> > and then handle that case via "/dev/optee" and
> > bootm_load_tee_from_file()? Is there any reason it wouldn't work?
> Thats a good idea to expose the idea and simplify the tee loading code.
>
> > Moreso, if that can be done we should consider just providing an API
> > to handle built-in firmware case and delegating that job to
> > individual
> > board files, so that we have the freedom of having per-board version
> > of OP-TEE firmware.
> The idea here was to not include the OP-TEE binary into the barebox
> upstream repository and only provide the option to include optee for
> production devices.
> I can't think of a use case where we want to include OP-TEE into an
> upstream barebox, since the features for OP-TEE (included early TAs,
> memory size,...) are quite specific for the use case.
>
Even if OP-TEE blob is never shipped in upstream repo, it seems that
it is entirely possible to have a use-case where user would have two+
boards of the same CPU architecture requiring different OP-TEE blobs.
Or is it a scenario that just doesn't exist in real-life?
What I am trying to say is that right now there's a central place
where get_builtin_firmware() is called that, besides hard-coding a
single OP-TEE binary, also requires a Kconfig option to be optional.
It seems that providing a simple macro like say:
#define export_builtin_firmware(fw, name) \
do { \
const u8 *__blob; \
size_t __size; \
get_builtin_firmware(fw, &__blob, &__size); \
add_mem_device(name, __blob, __size, 0); \
} while (0)
and leaving the job of calling that to individual board code will
allow you to drop extra Kconfig code and support multi or single
built-in OP-TEE builds.
Thanks,
Andrey Smirnov
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-01-30 23:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-29 11:10 [PATCH 0/1] OP-TEE support for barebox Rouven Czerwinski
2019-01-29 11:10 ` [PATCH 1/1] ARM: Initial OP-TEE support Rouven Czerwinski
2019-01-29 11:15 ` Rouven Czerwinski
2019-01-29 22:35 ` Andrey Smirnov
2019-01-30 7:16 ` Rouven Czerwinski
2019-01-30 23:24 ` Andrey Smirnov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox