[PATCH v4 0/5] stm32mp: Enable OP-TEE and TZC support in SPL

[PATCH v4 0/5] stm32mp: Enable OP-TEE and TZC support in SPL

[Alexandru Gagniuc]

The purpose of this series is to allow booting an OP-TEE image from
SPL, by corectly configuring the TrustZone (TZC) memory regions.

Although TZC400 is a generic silicon logic that could apply to other
mach- families, support is currently restricted to stm32mp. I have
neither a feasible way nor interest in validating this for other
chips. It's fairly trivial to move the code from mach- to arch/,
should another use case arise.

The configuration of the memory regions is devicetree-driven, so there
isn't much to hardcode, However the delineation between OP-TEE secure
memory and shared memory is not given in the devicetree. This is the
one thing that has to be hardcoded.

Changes since v3:
  - Pass a "phys_size_t *" to ofnode_get_addr_size() to resolve warning
  - s/u-boot,dm-pre-reloc/u-boot,dm-spl/ in devicetree for clarity

Changes since v2:
  - Use ram_get_info() instead of ofnode_read("st,mem-size")
  - Only hardcode shared memory size instead of TZDRAM size
  - Use log_info() instead of pr_info()

Changes since v1:
  - Removed "Weak functions are stupid" comment
  - Addressed blank line complaints from checkpatch

Alexandru Gagniuc (5):
  spl: mmc: Support OP-TEE payloads in Falcon mode
  spl: Introduce spl_board_prepare_for_optee() hook
  arm: stm32mp: Implement support for TZC 400 controller
  stm32mp1: spl: Configure TrustZone controller for OP-TEE
  ARM: dts: stm32mp: Add OP-TEE reserved memory to SPL dtb

 arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi |   3 +
 arch/arm/mach-stm32mp/Makefile           |   1 +
 arch/arm/mach-stm32mp/include/mach/tzc.h |  33 ++++++
 arch/arm/mach-stm32mp/spl.c              |  92 +++++++++++++++
 arch/arm/mach-stm32mp/tzc400.c           | 136 +++++++++++++++++++++++
 common/spl/spl.c                         |   5 +
 common/spl/spl_mmc.c                     |   6 +-
 include/spl.h                            |  14 +++
 8 files changed, 288 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/mach-stm32mp/include/mach/tzc.h
 create mode 100644 arch/arm/mach-stm32mp/tzc400.c

-- 
2.31.1

[PATCH v4 1/5] spl: mmc: Support OP-TEE payloads in Falcon mode

[Alexandru Gagniuc]

In general, Falcon mode means we're booting a linux kernel directly.
With FIT images, however, an OP-TEE secure kernel can be booted before
linux. Thus, if the next stage is an IH_OS_TEE, this isn't necessarily
a problem.

Of course, a general solution would involve mmc_load_image_raw_os()
only loading the binary, and leaving the decision of suitability to
someone else. However, a rework of the boot flow is beyond the scope
of this patch. Accept IH_OS_TEE as a valid OS value.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Reviewed-by: Tom Rini <trini@konsulko.com>
Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
---
 common/spl/spl_mmc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
index add2785b4e3..bab558d055f 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -230,8 +230,10 @@ static int mmc_load_image_raw_os(struct spl_image_info *spl_image,
 	if (ret)
 		return ret;
 
-	if (spl_image->os != IH_OS_LINUX) {
-		puts("Expected Linux image is not found. Trying to start U-boot\n");
+	if (spl_image->os != IH_OS_LINUX && spl_image->os != IH_OS_TEE) {
+		puts("Expected OS image is not found. Instead found ");
+		puts(genimg_get_os_name(spl_image->os));
+		puts(". Trying to start U-boot\n");
 		return -ENOENT;
 	}
 
-- 
2.31.1

[PATCH v4 2/5] spl: Introduce spl_board_prepare_for_optee() hook

[Alexandru Gagniuc]

OP-TEE requires some particular setup, which is not needed for linux
or other payloads. Add a hook for platform-specific code to perform
any OP-TEE related configuration and initialization.

A weak function is used because it is symmetrical to other
spl_board_prepare_for_*() implementations. A solution to avoid the use
of weak functions would trivially apply to all these implementations.
However, re-designing this is beyond the scope of this patch.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Reviewed-by: Tom Rini <trini@konsulko.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
---
 common/spl/spl.c |  5 +++++
 include/spl.h    | 14 ++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index a0a608fd772..56f6d47b164 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -165,6 +165,10 @@ __weak void spl_board_prepare_for_linux(void)
 	/* Nothing to do! */
 }
 
+__weak void spl_board_prepare_for_optee(void *fdt)
+{
+}
+
 __weak void spl_board_prepare_for_boot(void)
 {
 	/* Nothing to do! */
@@ -745,6 +749,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 #if CONFIG_IS_ENABLED(OPTEE)
 	case IH_OS_TEE:
 		debug("Jumping to U-Boot via OP-TEE\n");
+		spl_board_prepare_for_optee(spl_image.fdt_addr);
 		spl_optee_entry(NULL, NULL, spl_image.fdt_addr,
 				(void *)spl_image.entry_point);
 		break;
diff --git a/include/spl.h b/include/spl.h
index cee9a42ddb5..04ab2b6d7d8 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -388,6 +388,20 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
 			   const struct image_header *header);
 
 void spl_board_prepare_for_linux(void);
+
+/**
+ * spl_board_prepare_for_optee() - Prepare board for an OPTEE payload
+ *
+ * Prepares the board for booting an OP-TEE payload. Initialization is platform
+ * specific, and may include configuring the TrustZone memory, and other
+ * initialization steps required by OP-TEE.
+ * Note that @fdt is not used directly by OP-TEE. OP-TEE passes this @fdt to
+ * its normal world target. This target is not guaranteed to be u-boot, so @fdt
+ * changes that would normally be done by u-boot should be done in this step.
+ *
+ * @fdt: Devicetree that will be passed on, or NULL
+ */
+void spl_board_prepare_for_optee(void *fdt);
 void spl_board_prepare_for_boot(void);
 int spl_board_ubi_load_image(u32 boot_device);
 int spl_board_boot_device(u32 boot_device);
-- 
2.31.1

[PATCH v4 3/5] arm: stm32mp: Implement support for TZC 400 controller

[Alexandru Gagniuc]

The purpose of this change is to allow configuring TrustZone (TZC)
memory permissions. For example, OP-TEE expects TZC regions to be
configured in a very particular way. The API presented here is
intended to allow exactly that.

UCLASS support is not implemented, because it would not be too useful.
Changing TZC permissions needs to be done with care, so as not to cut
off access to memory we are currently using. One place where we can
use this is at the end of SPL, right before jumping to OP-TEE.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
---
 arch/arm/mach-stm32mp/Makefile           |   1 +
 arch/arm/mach-stm32mp/include/mach/tzc.h |  33 ++++++
 arch/arm/mach-stm32mp/tzc400.c           | 136 +++++++++++++++++++++++
 3 files changed, 170 insertions(+)
 create mode 100644 arch/arm/mach-stm32mp/include/mach/tzc.h
 create mode 100644 arch/arm/mach-stm32mp/tzc400.c

diff --git a/arch/arm/mach-stm32mp/Makefile b/arch/arm/mach-stm32mp/Makefile
index aa39867080d..879c1961fef 100644
--- a/arch/arm/mach-stm32mp/Makefile
+++ b/arch/arm/mach-stm32mp/Makefile
@@ -10,6 +10,7 @@ obj-y += bsec.o
 
 ifdef CONFIG_SPL_BUILD
 obj-y += spl.o
+obj-y += tzc400.o
 else
 obj-y += cmd_stm32prog/
 obj-$(CONFIG_CMD_STM32KEY) += cmd_stm32key.o
diff --git a/arch/arm/mach-stm32mp/include/mach/tzc.h b/arch/arm/mach-stm32mp/include/mach/tzc.h
new file mode 100644
index 00000000000..16db55c4645
--- /dev/null
+++ b/arch/arm/mach-stm32mp/include/mach/tzc.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Simple API for configuring TrustZone memory regions
+ *
+ * The premise is that the desired TZC layout is known beforehand, and it can
+ * be configured in one step. tzc_configure() provides this functionality.
+ */
+#ifndef MACH_TZC_H
+#define MACH_TZC_H
+
+#include <linux/types.h>
+
+enum tzc_sec_mode {
+	TZC_ATTR_SEC_NONE = 0,
+	TZC_ATTR_SEC_R = 1,
+	TZC_ATTR_SEC_W = 2,
+	TZC_ATTR_SEC_RW	 = 3
+};
+
+struct tzc_region {
+	uintptr_t base;
+	uintptr_t top;
+	enum tzc_sec_mode sec_mode;
+	uint16_t nsec_id;
+	uint16_t filters_mask;
+};
+
+int tzc_configure(uintptr_t tzc, const struct tzc_region *cfg);
+int tzc_disable_filters(uintptr_t tzc, uint16_t filters_mask);
+int tzc_enable_filters(uintptr_t tzc, uint16_t filters_mask);
+void tzc_dump_config(uintptr_t tzc);
+
+#endif /* MACH_TZC_H */
diff --git a/arch/arm/mach-stm32mp/tzc400.c b/arch/arm/mach-stm32mp/tzc400.c
new file mode 100644
index 00000000000..cdc4a40edaf
--- /dev/null
+++ b/arch/arm/mach-stm32mp/tzc400.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Simple API for configuring TrustZone memory restrictions for TZC400
+ */
+
+#define LOG_CATEGORY LOGC_ARCH
+
+#include <linux/iopoll.h>
+#include <mach/tzc.h>
+
+#define TZC_TIMEOUT_US		100
+
+#define TZC_BUILD_CONFIG	0x00
+#define TZC_ACTION		0x04
+#define TZC_ACTION_NONE		0
+#define TZC_ACTION_ERR		1
+#define TZC_ACTION_INT		2
+#define TZC_ACTION_INT_ERR	3
+#define TZC_GATE_KEEPER		0x08
+
+#define TZC_REGION0_OFFSET	0x100
+#define TZC_REGION_CFG_SIZE	0x20
+#define TZC_REGION1_OFFSET	0x120
+#define TZC_REGION_BASE		0x00
+#define TZC_REGION_TOP		0x08
+#define TZC_REGION_ATTRIBUTE	0x10
+#define TZC_REGION_ACCESS	0x14
+
+static uint32_t tzc_read(uintptr_t tzc, size_t reg)
+{
+	return readl(tzc + reg);
+}
+
+static void tzc_write(uintptr_t tzc, size_t reg, uint32_t val)
+{
+	writel(val, tzc + reg);
+}
+
+static uint16_t tzc_config_get_active_filters(const struct tzc_region *cfg)
+{
+	uint16_t active_filters = 0;
+
+	for ( ; cfg->top != 0; cfg++)
+		active_filters |= cfg->filters_mask;
+
+	return active_filters;
+}
+
+int tzc_configure(uintptr_t tzc, const struct tzc_region *cfg)
+{
+	uintptr_t region = tzc + TZC_REGION1_OFFSET;
+	uint32_t nsid, attr_reg, active_filters;
+	int ret;
+
+	active_filters = tzc_config_get_active_filters(cfg);
+	if (active_filters == 0)
+		return -EINVAL;
+
+	ret = tzc_disable_filters(tzc, active_filters);
+	if (ret < 0)
+		return ret;
+
+	for ( ; cfg->top != 0; cfg++, region += TZC_REGION_CFG_SIZE) {
+		attr_reg = (cfg->sec_mode & 0x03) << 30;
+		attr_reg |= (cfg->filters_mask & 0x03) << 0;
+		nsid = cfg->nsec_id & 0xffff;
+		nsid |= nsid << 16;
+
+		tzc_write(region, TZC_REGION_BASE, cfg->base);
+		tzc_write(region, TZC_REGION_TOP, cfg->top);
+		tzc_write(region, TZC_REGION_ACCESS, nsid);
+		tzc_write(region, TZC_REGION_ATTRIBUTE, attr_reg);
+	}
+
+	tzc_write(tzc, TZC_ACTION, TZC_ACTION_ERR);
+	return tzc_enable_filters(tzc, active_filters);
+}
+
+int tzc_disable_filters(uintptr_t tzc, uint16_t filters_mask)
+{
+	uint32_t gate = tzc_read(tzc, TZC_GATE_KEEPER);
+	uint32_t filter_status = filters_mask << 16;
+
+	gate &= ~filters_mask;
+	tzc_write(tzc, TZC_GATE_KEEPER, gate);
+
+	return readl_poll_timeout(tzc + TZC_GATE_KEEPER, gate,
+				 (gate & filter_status) == 0, TZC_TIMEOUT_US);
+}
+
+int tzc_enable_filters(uintptr_t tzc, uint16_t filters_mask)
+{
+	uint32_t gate = tzc_read(tzc, TZC_GATE_KEEPER);
+	uint32_t filter_status = filters_mask << 16;
+
+	gate |= filters_mask;
+	tzc_write(tzc, TZC_GATE_KEEPER, gate);
+
+	return readl_poll_timeout(tzc + TZC_GATE_KEEPER, gate,
+				 (gate & filter_status) == filter_status,
+				 TZC_TIMEOUT_US);
+}
+
+static const char *sec_access_str_from_attr(uint32_t attr)
+{
+	const char *const sec_mode[] = { "none", "RO  ", "WO  ", "RW  " };
+
+	return sec_mode[(attr >> 30) & 0x03];
+}
+
+void tzc_dump_config(uintptr_t tzc)
+{
+	uint32_t build_config, base, top, attr, nsaid;
+	int num_regions, i;
+	uintptr_t region;
+
+	build_config = tzc_read(tzc, TZC_BUILD_CONFIG);
+	num_regions = ((build_config >> 0) & 0x1f) + 1;
+
+	for (i = 0; i < num_regions; i++) {
+		region = tzc + TZC_REGION0_OFFSET + i * TZC_REGION_CFG_SIZE;
+
+		base = tzc_read(region, TZC_REGION_BASE);
+		top = tzc_read(region, TZC_REGION_TOP);
+		attr = tzc_read(region, TZC_REGION_ATTRIBUTE);
+		nsaid = tzc_read(region, TZC_REGION_ACCESS);
+
+		if (attr == 0 && nsaid == 0)
+			continue;
+
+		log_info("TZC region %u: %08x->%08x - filters 0x%x\n",
+			 i, base, top, (attr >> 0) & 0xf);
+		log_info("\t Secure access %s NSAID %08x\n",
+			 sec_access_str_from_attr(attr), nsaid);
+	}
+}
-- 
2.31.1

[PATCH v4 4/5] stm32mp1: spl: Configure TrustZone controller for OP-TEE

[Alexandru Gagniuc]

OP-TEE is very particular about how the TZC should be configured.
When booting an OP-TEE payload, an incorrect TZC configuration will
result in a panic.

Most information can be derived from the SPL devicetree. The only
information we don't have is the split between TZDRAM and shared
memory. This has to be hardcoded. The rest of the configuration is
fairly easy, and only requires 3 TZC regions. Configure them.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
 arch/arm/mach-stm32mp/spl.c | 92 +++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c
index b53659a698a..405eff68a3b 100644
--- a/arch/arm/mach-stm32mp/spl.c
+++ b/arch/arm/mach-stm32mp/spl.c
@@ -11,11 +11,13 @@
 #include <hang.h>
 #include <init.h>
 #include <log.h>
+#include <ram.h>
 #include <spl.h>
 #include <asm/cache.h>
 #include <asm/global_data.h>
 #include <asm/io.h>
 #include <asm/arch/sys_proto.h>
+#include <mach/tzc.h>
 #include <linux/libfdt.h>
 
 u32 spl_boot_device(void)
@@ -92,6 +94,96 @@ __weak int board_early_init_f(void)
 	return 0;
 }
 
+uint32_t stm32mp_get_dram_size(void)
+{
+	struct ram_info ram;
+	struct udevice *dev;
+	int ret;
+
+	if (uclass_get_device(UCLASS_RAM, 0, &dev))
+		return 0;
+
+	ret = ram_get_info(dev, &ram);
+	if (ret)
+		return 0;
+
+	return ram.size;
+}
+
+static int optee_get_reserved_memory(uint32_t *start, uint32_t *size)
+{
+	phys_size_t fdt_mem_size;
+	fdt_addr_t fdt_start;
+	ofnode node;
+
+	node = ofnode_path("/reserved-memory/optee");
+	if (!ofnode_valid(node))
+		return 0;
+
+	fdt_start = ofnode_get_addr_size(node, "reg", &fdt_mem_size);
+	*start = fdt_start;
+	*size = fdt_mem_size;
+	return (fdt_start < 0) ? fdt_start : 0;
+}
+
+#define CFG_SHMEM_SIZE			0x200000
+#define STM32_TZC_NSID_ALL		0xffff
+#define STM32_TZC_FILTER_ALL		3
+
+void stm32_init_tzc_for_optee(void)
+{
+	const uint32_t dram_size = stm32mp_get_dram_size();
+	const uintptr_t dram_top = STM32_DDR_BASE + (dram_size - 1);
+	uint32_t optee_base, optee_size, tee_shmem_base;
+	const uintptr_t tzc = STM32_TZC_BASE;
+	int ret;
+
+	if (dram_size == 0)
+		panic("Cannot determine DRAM size from devicetree\n");
+
+	ret = optee_get_reserved_memory(&optee_base, &optee_size);
+	if (ret < 0 || optee_size <= CFG_SHMEM_SIZE)
+		panic("Invalid OPTEE reserved memory in devicetree\n");
+
+	tee_shmem_base = optee_base + optee_size - CFG_SHMEM_SIZE;
+
+	const struct tzc_region optee_config[] = {
+		{
+			.base = STM32_DDR_BASE,
+			.top = optee_base - 1,
+			.sec_mode = TZC_ATTR_SEC_NONE,
+			.nsec_id = STM32_TZC_NSID_ALL,
+			.filters_mask = STM32_TZC_FILTER_ALL,
+		}, {
+			.base = optee_base,
+			.top = tee_shmem_base - 1,
+			.sec_mode = TZC_ATTR_SEC_RW,
+			.nsec_id = 0,
+			.filters_mask = STM32_TZC_FILTER_ALL,
+		}, {
+			.base = tee_shmem_base,
+			.top = dram_top,
+			.sec_mode = TZC_ATTR_SEC_NONE,
+			.nsec_id = STM32_TZC_NSID_ALL,
+			.filters_mask = STM32_TZC_FILTER_ALL,
+		}, {
+			.top = 0,
+		}
+	};
+
+	flush_dcache_all();
+
+	tzc_configure(tzc, optee_config);
+	tzc_dump_config(tzc);
+
+	dcache_disable();
+}
+
+void spl_board_prepare_for_optee(void *fdt)
+{
+	stm32_init_tzc_for_optee();
+}
+
 void board_init_f(ulong dummy)
 {
 	struct udevice *dev;
-- 
2.31.1

[PATCH v4 5/5] ARM: dts: stm32mp: Add OP-TEE reserved memory to SPL dtb

[Alexandru Gagniuc]

Add the "/reserved-memory/optee" node to the SPL devicetree. The
purpose is to allow configuring TZC regions when booting OP-TEE.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi b/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
index 67876192900..55d634f7db2 100644
--- a/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
+++ b/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
@@ -30,9 +30,12 @@
 	};
 
 	reserved-memory {
+		u-boot,dm-spl;
+
 		optee@de000000 {
 			reg = <0xde000000 0x02000000>;
 			no-map;
+			u-boot,dm-spl;
 		};
 	};
 
-- 
2.31.1

Re: [PATCH v4 5/5] ARM: dts: stm32mp: Add OP-TEE reserved memory to SPL dtb

[Patrick DELAUNAY]

Hi,

On 5/31/21 7:43 PM, Alexandru Gagniuc wrote:
> Add the "/reserved-memory/optee" node to the SPL devicetree. The
> purpose is to allow configuring TZC regions when booting OP-TEE.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
>   arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi b/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
> index 67876192900..55d634f7db2 100644
> --- a/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
> +++ b/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
> @@ -30,9 +30,12 @@
>   	};
>   
>   	reserved-memory {
> +		u-boot,dm-spl;
> +
>   		optee@de000000 {
>   			reg = <0xde000000 0x02000000>;
>   			no-map;
> +			u-boot,dm-spl;
>   		};
>   	};
>   

Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>

Thanks
Patrick

Re: [PATCH v4 0/5] stm32mp: Enable OP-TEE and TZC support in SPL

[Alex G.]

On 5/31/21 12:43 PM, Alexandru Gagniuc wrote:
> The purpose of this series is to allow booting an OP-TEE image from
> SPL, by corectly configuring the TrustZone (TZC) memory regions.

Any chance we could have this hit the merge window?

Alex

Re: [PATCH v4 0/5] stm32mp: Enable OP-TEE and TZC support in SPL

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 403 bytes --]

On Tue, Jul 06, 2021 at 10:18:44AM -0500, Alex G. wrote:

> On 5/31/21 12:43 PM, Alexandru Gagniuc wrote:
> > The purpose of this series is to allow booting an OP-TEE image from
> > SPL, by corectly configuring the TrustZone (TZC) memory regions.
> 
> Any chance we could have this hit the merge window?

For clarity, does this series depend on anything that's not already
merged?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v4 0/5] stm32mp: Enable OP-TEE and TZC support in SPL

[Alex G.]

On 7/6/21 10:45 AM, Tom Rini wrote:
> On Tue, Jul 06, 2021 at 10:18:44AM -0500, Alex G. wrote:
> 
>> On 5/31/21 12:43 PM, Alexandru Gagniuc wrote:
>>> The purpose of this series is to allow booting an OP-TEE image from
>>> SPL, by corectly configuring the TrustZone (TZC) memory regions.
>>
>> Any chance we could have this hit the merge window?
> 
> For clarity, does this series depend on anything that's not already
> merged?

Negative. This series can be applied standalone.

Alex

Re: [PATCH v4 4/5] stm32mp1: spl: Configure TrustZone controller for OP-TEE

[Patrice CHOTARD]

Hi Alexandru

Sorry for the delay, i just noticed that this patch didn't get reviewed neither by Patrick nor by me.
That's done ;-)


On 5/31/21 7:43 PM, Alexandru Gagniuc wrote:
> OP-TEE is very particular about how the TZC should be configured.
> When booting an OP-TEE payload, an incorrect TZC configuration will
> result in a panic.
> 
> Most information can be derived from the SPL devicetree. The only
> information we don't have is the split between TZDRAM and shared
> memory. This has to be hardcoded. The rest of the configuration is
> fairly easy, and only requires 3 TZC regions. Configure them.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
>  arch/arm/mach-stm32mp/spl.c | 92 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 92 insertions(+)
> 
> diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c
> index b53659a698a..405eff68a3b 100644
> --- a/arch/arm/mach-stm32mp/spl.c
> +++ b/arch/arm/mach-stm32mp/spl.c
> @@ -11,11 +11,13 @@
>  #include <hang.h>
>  #include <init.h>
>  #include <log.h>
> +#include <ram.h>
>  #include <spl.h>
>  #include <asm/cache.h>
>  #include <asm/global_data.h>
>  #include <asm/io.h>
>  #include <asm/arch/sys_proto.h>
> +#include <mach/tzc.h>
>  #include <linux/libfdt.h>
>  
>  u32 spl_boot_device(void)
> @@ -92,6 +94,96 @@ __weak int board_early_init_f(void)
>  	return 0;
>  }
>  
> +uint32_t stm32mp_get_dram_size(void)
> +{
> +	struct ram_info ram;
> +	struct udevice *dev;
> +	int ret;
> +
> +	if (uclass_get_device(UCLASS_RAM, 0, &dev))
> +		return 0;
> +
> +	ret = ram_get_info(dev, &ram);
> +	if (ret)
> +		return 0;
> +
> +	return ram.size;
> +}
> +
> +static int optee_get_reserved_memory(uint32_t *start, uint32_t *size)
> +{
> +	phys_size_t fdt_mem_size;
> +	fdt_addr_t fdt_start;
> +	ofnode node;
> +
> +	node = ofnode_path("/reserved-memory/optee");
> +	if (!ofnode_valid(node))
> +		return 0;
> +
> +	fdt_start = ofnode_get_addr_size(node, "reg", &fdt_mem_size);
> +	*start = fdt_start;
> +	*size = fdt_mem_size;
> +	return (fdt_start < 0) ? fdt_start : 0;
> +}
> +
> +#define CFG_SHMEM_SIZE			0x200000
> +#define STM32_TZC_NSID_ALL		0xffff
> +#define STM32_TZC_FILTER_ALL		3
> +
> +void stm32_init_tzc_for_optee(void)
> +{
> +	const uint32_t dram_size = stm32mp_get_dram_size();
> +	const uintptr_t dram_top = STM32_DDR_BASE + (dram_size - 1);
> +	uint32_t optee_base, optee_size, tee_shmem_base;
> +	const uintptr_t tzc = STM32_TZC_BASE;
> +	int ret;
> +
> +	if (dram_size == 0)
> +		panic("Cannot determine DRAM size from devicetree\n");
> +
> +	ret = optee_get_reserved_memory(&optee_base, &optee_size);
> +	if (ret < 0 || optee_size <= CFG_SHMEM_SIZE)
> +		panic("Invalid OPTEE reserved memory in devicetree\n");
> +
> +	tee_shmem_base = optee_base + optee_size - CFG_SHMEM_SIZE;
> +
> +	const struct tzc_region optee_config[] = {
> +		{
> +			.base = STM32_DDR_BASE,
> +			.top = optee_base - 1,
> +			.sec_mode = TZC_ATTR_SEC_NONE,
> +			.nsec_id = STM32_TZC_NSID_ALL,
> +			.filters_mask = STM32_TZC_FILTER_ALL,
> +		}, {
> +			.base = optee_base,
> +			.top = tee_shmem_base - 1,
> +			.sec_mode = TZC_ATTR_SEC_RW,
> +			.nsec_id = 0,
> +			.filters_mask = STM32_TZC_FILTER_ALL,
> +		}, {
> +			.base = tee_shmem_base,
> +			.top = dram_top,
> +			.sec_mode = TZC_ATTR_SEC_NONE,
> +			.nsec_id = STM32_TZC_NSID_ALL,
> +			.filters_mask = STM32_TZC_FILTER_ALL,
> +		}, {
> +			.top = 0,
> +		}
> +	};
> +
> +	flush_dcache_all();
> +
> +	tzc_configure(tzc, optee_config);
> +	tzc_dump_config(tzc);
> +
> +	dcache_disable();
> +}
> +
> +void spl_board_prepare_for_optee(void *fdt)
> +{
> +	stm32_init_tzc_for_optee();
> +}
> +
>  void board_init_f(ulong dummy)
>  {
>  	struct udevice *dev;
> 

Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>

Thanks
Patrice

Re: [PATCH v4 4/5] stm32mp1: spl: Configure TrustZone controller for OP-TEE

[Patrick DELAUNAY]

Hi,

On 5/31/21 7:43 PM, Alexandru Gagniuc wrote:
> OP-TEE is very particular about how the TZC should be configured.
> When booting an OP-TEE payload, an incorrect TZC configuration will
> result in a panic.
>
> Most information can be derived from the SPL devicetree. The only
> information we don't have is the split between TZDRAM and shared
> memory. This has to be hardcoded. The rest of the configuration is
> fairly easy, and only requires 3 TZC regions. Configure them.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
>   arch/arm/mach-stm32mp/spl.c | 92 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 92 insertions(+)
>

Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>

Thanks
Patrick

Re: [PATCH v4 1/5] spl: mmc: Support OP-TEE payloads in Falcon mode

[Patrick DELAUNAY]

Hi,

On 5/31/21 7:43 PM, Alexandru Gagniuc wrote:
> In general, Falcon mode means we're booting a linux kernel directly.
> With FIT images, however, an OP-TEE secure kernel can be booted before
> linux. Thus, if the next stage is an IH_OS_TEE, this isn't necessarily
> a problem.
>
> Of course, a general solution would involve mmc_load_image_raw_os()
> only loading the binary, and leaving the decision of suitability to
> someone else. However, a rework of the boot flow is beyond the scope
> of this patch. Accept IH_OS_TEE as a valid OS value.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>
> Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
> ---
>   common/spl/spl_mmc.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> index add2785b4e3..bab558d055f 100644
> --- a/common/spl/spl_mmc.c
> +++ b/common/spl/spl_mmc.c
> @@ -230,8 +230,10 @@ static int mmc_load_image_raw_os(struct spl_image_info *spl_image,
>   	if (ret)
>   		return ret;
>   
> -	if (spl_image->os != IH_OS_LINUX) {
> -		puts("Expected Linux image is not found. Trying to start U-boot\n");
> +	if (spl_image->os != IH_OS_LINUX && spl_image->os != IH_OS_TEE) {
> +		puts("Expected OS image is not found. Instead found ");
> +		puts(genimg_get_os_name(spl_image->os));
> +		puts(". Trying to start U-boot\n");
>   		return -ENOENT;
>   	}
>   


When I merge this patch on master branch, I get the error:

arm:  +   imx6dl_mamoj
+===================== WARNING ======================
+This board does not use CONFIG_DM_USB (Driver Model
+for  USB). Please update the board to use
+CONFIG_DM_USB before the v2019.07 release. Failure to
+update by the deadline may result in board removal.
+See doc/driver-model/migration.rst for more info.
+====================================================
+spl/u-boot-spl.bin exceeds file size limit:
+  limit:  0xefa0 bytes
+  actual: 0xf41d bytes
+  excess: 0x47d bytes
+make[1]: *** [Makefile:1997: spl/u-boot-spl.bin] Error 1
+make[1]: *** Deleting file 'spl/u-boot-spl.bin'
+make: *** [Makefile:177: sub-make] Error 2


The overhead:

$> tools/buildman/buildman -b TEST -c 2 imx6dl_mamoj

$> tools/buildman/buildman -b TEST -c 2 imx6dl_mamoj -seS

Summary of 2 commits for 1 boards (1 thread, 12 jobs per thread)

01: Merge branch '2021-07-14-build-and-host-updates'
        arm:  w+   imx6dl_mamoj
+===================== WARNING ======================
+This board does not use CONFIG_DM_USB (Driver Model
+for  USB). Please update the board to use
+CONFIG_DM_USB before the v2019.07 release. Failure to
+update by the deadline may result in board removal.
+See doc/driver-model/migration.rst for more info.
+====================================================
02: spl: mmc: Support OP-TEE payloads in Falcon mode
        arm:  +   imx6dl_mamoj
+spl/u-boot-spl.bin exceeds file size limit:
+  limit:  0xefa0 bytes
+  actual: 0xf41d bytes
+  excess: 0x47d bytes
+make[1]: *** [Makefile:1997: spl/u-boot-spl.bin] Error 1
+make[1]: *** Deleting file 'spl/u-boot-spl.bin'
+make: *** [Makefile:177: sub-make] Error 2
        arm: (for 1/1 boards) spl/u-boot-spl:all +2242.0 
spl/u-boot-spl:rodata +2162.0 spl/u-boot-spl:text +80.0


With details :

diff 
01_gc11f5abce8_Merge-branch-\'2021-0/imx6dl_mamoj/spl-u-boot-spl.sizes 
02_g0620649bfa_spl--mmc--Support-OP/imx6dl_mamoj/spl-u-boot-spl.sizes
157a158,159
 > 00000012 T get_table_entry
 > 00000012 T get_table_entry_name
171a174
 > 00000014 T genimg_get_os_name
531a535
 > 000000a8 r uimage_os
563d566
< 00000134 T spl_mmc_load
566a570
 > 0000014c T spl_mmc_load


This issue need to be solved before I accept and merge the serie.


See also ./common/spl/spl_fit.c:514:

       ............                                   This saves some
      * space by omitting the large table of OS types.
      */


But I think again about the title of this patch :

- spl: mmc: Support OP-TEE payloads in Falcon mode


If I unterstood after the serie the sequence for MMC is:

      ROM code => SPL => TEE (as raw OS) => U-Boot


but it is not really the Falcon mode and
OP-TEE + falcon mode is not really supported...


And the patch title is disturbing.


For me the correct sequence is in Falcon mode is :

     ROM code => SPL => TEE (secure world) => kernel (normal world)


With the TEE and the kernel loaded by the SPL......


and without falcon mode :

(A)    ROM code => SPL => TEE (secure world) => U-Boot

or

(B)    ROM code => SPL (TZ) => U-Boot (TZ) execute bootm => TEE (secure 
world) => kernel


what it your expected sequence in spl_load_simple_fit in this serie / in 
your defconfig ?

Today with the normal world address can be:

1/ = spl_image->entry_point (bootm_os.c in U-Boot proprer)

2/ = CONFIG_SYS_TEXT_BASE (hardcoded for SPL in spl_optee.S)


for 2/ When U-Boot is not used after SPL =  in falcon mode,

the LR register shoud be set to kernel entry point.

Patrick

Re: [PATCH v4 1/5] spl: mmc: Support OP-TEE payloads in Falcon mode

[Alex G.]

On 7/15/21 1:27 PM, Patrick DELAUNAY wrote:
> Hi,

[snip]

> When I merge this patch on master branch, I get the error:
> 
> arm:  +   imx6dl_mamoj
> +spl/u-boot-spl.bin exceeds file size limit:
> +  limit:  0xefa0 bytes
> +  actual: 0xf41d bytes
> +  excess: 0x47d bytes
> +make[1]: *** [Makefile:1997: spl/u-boot-spl.bin] Error 1
> +make[1]: *** Deleting file 'spl/u-boot-spl.bin'
> +make: *** [Makefile:177: sub-make] Error 2

> This issue need to be solved before I accept and merge the serie.

Okay, I'll have to drop the call to genimg_get_os_name().


> But I think again about the title of this patch :
> 
> - spl: mmc: Support OP-TEE payloads in Falcon mode
> 
> 
> If I unterstood after the serie the sequence for MMC is:
> 
>       ROM code => SPL => TEE (as raw OS) => U-Boot
> 
> 
> but it is not really the Falcon mode and
> OP-TEE + falcon mode is not really supported...
> 
> 
> And the patch title is disturbing.
> 
> 
> For me the correct sequence is in Falcon mode is :
> 
>      ROM code => SPL => TEE (secure world) => kernel (normal world)
> 

This is exactly the use case that this patch intends to support.

> 
> With the TEE and the kernel loaded by the SPL......
> 
> 
> and without falcon mode :
> 
> (A)    ROM code => SPL => TEE (secure world) => U-Boot
> 
> or
> 
> (B)    ROM code => SPL (TZ) => U-Boot (TZ) execute bootm => TEE (secure 
> world) => kernel
> 
> 
> what it your expected sequence in spl_load_simple_fit in this serie / in 
> your defconfig ?
> 
> Today with the normal world address can be:
> 
> 1/ = spl_image->entry_point (bootm_os.c in U-Boot proprer)
> 
> 2/ = CONFIG_SYS_TEXT_BASE (hardcoded for SPL in spl_optee.S)
> 
> 
> for 2/ When U-Boot is not used after SPL =  in falcon mode,
> 
> the LR register shoud be set to kernel entry point.

How does SPL know where OP-TEE should jump to? One could parse the FIT 
image, and try to figure out which of the loadables is the kernel. But 
what if there's a linux and u-boot, with different "/configurations" 
nodes? Figuring out where OP-TEE wants to start the normal world is a 
hard problem that the u=boot infrastructure is not prepared for.

The solution I'm using is to build OP-TEE with
	CFG_NS_ENTRY_ADDR=[linux entry addr]

Then LR is irrelevant.

Alex

[PATCH v5 0/5] stm32mp: Enable OP-TEE and TZC support in SPL

[Alexandru Gagniuc]

v4 branch was reported to have some issues with SPL becoming too big
on some platforms (e.g. imx6dl_mamoj) This is fixed by dropping the
call to genimg_get_os_name().

Alexandru Gagniuc (5):
  spl: mmc: Support OP-TEE payloads in Falcon mode
  spl: Introduce spl_board_prepare_for_optee() hook
  arm: stm32mp: Implement support for TZC 400 controller
  stm32mp1: spl: Configure TrustZone controller for OP-TEE
  ARM: dts: stm32mp: Add OP-TEE reserved memory to SPL dtb

 arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi |   3 +
 arch/arm/mach-stm32mp/Makefile           |   1 +
 arch/arm/mach-stm32mp/include/mach/tzc.h |  33 ++++++
 arch/arm/mach-stm32mp/spl.c              |  92 +++++++++++++++
 arch/arm/mach-stm32mp/tzc400.c           | 136 +++++++++++++++++++++++
 common/spl/spl.c                         |   5 +
 common/spl/spl_mmc.c                     |   4 +-
 include/spl.h                            |  14 +++
 8 files changed, 286 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/mach-stm32mp/include/mach/tzc.h
 create mode 100644 arch/arm/mach-stm32mp/tzc400.c

-- 
2.31.1

[PATCH v5 1/5] spl: mmc: Support OP-TEE payloads in Falcon mode

[Alexandru Gagniuc]

In general, Falcon mode means we're booting a linux kernel directly.
With FIT images, however, an OP-TEE secure kernel can be booted before
linux. Thus, if the next stage is an IH_OS_TEE, this isn't necessarily
a problem.

Of course, a general solution would involve mmc_load_image_raw_os()
only loading the binary, and leaving the decision of suitability to
someone else. However, a rework of the boot flow is beyond the scope
of this patch. Accept IH_OS_TEE as a valid OS value.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Reviewed-by: Tom Rini <trini@konsulko.com>
Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
---
 common/spl/spl_mmc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
index add2785b4e..c6bd3dab12 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -230,8 +230,8 @@ static int mmc_load_image_raw_os(struct spl_image_info *spl_image,
 	if (ret)
 		return ret;
 
-	if (spl_image->os != IH_OS_LINUX) {
-		puts("Expected Linux image is not found. Trying to start U-boot\n");
+	if (spl_image->os != IH_OS_LINUX && spl_image->os != IH_OS_TEE) {
+		puts("Expected image is not found. Trying to start U-boot\n");
 		return -ENOENT;
 	}
 
-- 
2.31.1

[PATCH v5 2/5] spl: Introduce spl_board_prepare_for_optee() hook

[Alexandru Gagniuc]

OP-TEE requires some particular setup, which is not needed for linux
or other payloads. Add a hook for platform-specific code to perform
any OP-TEE related configuration and initialization.

A weak function is used because it is symmetrical to other
spl_board_prepare_for_*() implementations. A solution to avoid the use
of weak functions would trivially apply to all these implementations.
However, re-designing this is beyond the scope of this patch.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Reviewed-by: Tom Rini <trini@konsulko.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
---
 common/spl/spl.c |  5 +++++
 include/spl.h    | 14 ++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index eba77cace6..2919fa3e92 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -167,6 +167,10 @@ __weak void spl_board_prepare_for_linux(void)
 	/* Nothing to do! */
 }
 
+__weak void spl_board_prepare_for_optee(void *fdt)
+{
+}
+
 __weak void spl_board_prepare_for_boot(void)
 {
 	/* Nothing to do! */
@@ -747,6 +751,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 #if CONFIG_IS_ENABLED(OPTEE)
 	case IH_OS_TEE:
 		debug("Jumping to U-Boot via OP-TEE\n");
+		spl_board_prepare_for_optee(spl_image.fdt_addr);
 		spl_optee_entry(NULL, NULL, spl_image.fdt_addr,
 				(void *)spl_image.entry_point);
 		break;
diff --git a/include/spl.h b/include/spl.h
index cee9a42ddb..04ab2b6d7d 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -388,6 +388,20 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
 			   const struct image_header *header);
 
 void spl_board_prepare_for_linux(void);
+
+/**
+ * spl_board_prepare_for_optee() - Prepare board for an OPTEE payload
+ *
+ * Prepares the board for booting an OP-TEE payload. Initialization is platform
+ * specific, and may include configuring the TrustZone memory, and other
+ * initialization steps required by OP-TEE.
+ * Note that @fdt is not used directly by OP-TEE. OP-TEE passes this @fdt to
+ * its normal world target. This target is not guaranteed to be u-boot, so @fdt
+ * changes that would normally be done by u-boot should be done in this step.
+ *
+ * @fdt: Devicetree that will be passed on, or NULL
+ */
+void spl_board_prepare_for_optee(void *fdt);
 void spl_board_prepare_for_boot(void);
 int spl_board_ubi_load_image(u32 boot_device);
 int spl_board_boot_device(u32 boot_device);
-- 
2.31.1

[PATCH v5 3/5] arm: stm32mp: Implement support for TZC 400 controller

[Alexandru Gagniuc]

The purpose of this change is to allow configuring TrustZone (TZC)
memory permissions. For example, OP-TEE expects TZC regions to be
configured in a very particular way. The API presented here is
intended to allow exactly that.

UCLASS support is not implemented, because it would not be too useful.
Changing TZC permissions needs to be done with care, so as not to cut
off access to memory we are currently using. One place where we can
use this is at the end of SPL, right before jumping to OP-TEE.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
---
 arch/arm/mach-stm32mp/Makefile           |   1 +
 arch/arm/mach-stm32mp/include/mach/tzc.h |  33 ++++++
 arch/arm/mach-stm32mp/tzc400.c           | 136 +++++++++++++++++++++++
 3 files changed, 170 insertions(+)
 create mode 100644 arch/arm/mach-stm32mp/include/mach/tzc.h
 create mode 100644 arch/arm/mach-stm32mp/tzc400.c

diff --git a/arch/arm/mach-stm32mp/Makefile b/arch/arm/mach-stm32mp/Makefile
index aa39867080..879c1961fe 100644
--- a/arch/arm/mach-stm32mp/Makefile
+++ b/arch/arm/mach-stm32mp/Makefile
@@ -10,6 +10,7 @@ obj-y += bsec.o
 
 ifdef CONFIG_SPL_BUILD
 obj-y += spl.o
+obj-y += tzc400.o
 else
 obj-y += cmd_stm32prog/
 obj-$(CONFIG_CMD_STM32KEY) += cmd_stm32key.o
diff --git a/arch/arm/mach-stm32mp/include/mach/tzc.h b/arch/arm/mach-stm32mp/include/mach/tzc.h
new file mode 100644
index 0000000000..16db55c464
--- /dev/null
+++ b/arch/arm/mach-stm32mp/include/mach/tzc.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Simple API for configuring TrustZone memory regions
+ *
+ * The premise is that the desired TZC layout is known beforehand, and it can
+ * be configured in one step. tzc_configure() provides this functionality.
+ */
+#ifndef MACH_TZC_H
+#define MACH_TZC_H
+
+#include <linux/types.h>
+
+enum tzc_sec_mode {
+	TZC_ATTR_SEC_NONE = 0,
+	TZC_ATTR_SEC_R = 1,
+	TZC_ATTR_SEC_W = 2,
+	TZC_ATTR_SEC_RW	 = 3
+};
+
+struct tzc_region {
+	uintptr_t base;
+	uintptr_t top;
+	enum tzc_sec_mode sec_mode;
+	uint16_t nsec_id;
+	uint16_t filters_mask;
+};
+
+int tzc_configure(uintptr_t tzc, const struct tzc_region *cfg);
+int tzc_disable_filters(uintptr_t tzc, uint16_t filters_mask);
+int tzc_enable_filters(uintptr_t tzc, uint16_t filters_mask);
+void tzc_dump_config(uintptr_t tzc);
+
+#endif /* MACH_TZC_H */
diff --git a/arch/arm/mach-stm32mp/tzc400.c b/arch/arm/mach-stm32mp/tzc400.c
new file mode 100644
index 0000000000..cdc4a40eda
--- /dev/null
+++ b/arch/arm/mach-stm32mp/tzc400.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Simple API for configuring TrustZone memory restrictions for TZC400
+ */
+
+#define LOG_CATEGORY LOGC_ARCH
+
+#include <linux/iopoll.h>
+#include <mach/tzc.h>
+
+#define TZC_TIMEOUT_US		100
+
+#define TZC_BUILD_CONFIG	0x00
+#define TZC_ACTION		0x04
+#define TZC_ACTION_NONE		0
+#define TZC_ACTION_ERR		1
+#define TZC_ACTION_INT		2
+#define TZC_ACTION_INT_ERR	3
+#define TZC_GATE_KEEPER		0x08
+
+#define TZC_REGION0_OFFSET	0x100
+#define TZC_REGION_CFG_SIZE	0x20
+#define TZC_REGION1_OFFSET	0x120
+#define TZC_REGION_BASE		0x00
+#define TZC_REGION_TOP		0x08
+#define TZC_REGION_ATTRIBUTE	0x10
+#define TZC_REGION_ACCESS	0x14
+
+static uint32_t tzc_read(uintptr_t tzc, size_t reg)
+{
+	return readl(tzc + reg);
+}
+
+static void tzc_write(uintptr_t tzc, size_t reg, uint32_t val)
+{
+	writel(val, tzc + reg);
+}
+
+static uint16_t tzc_config_get_active_filters(const struct tzc_region *cfg)
+{
+	uint16_t active_filters = 0;
+
+	for ( ; cfg->top != 0; cfg++)
+		active_filters |= cfg->filters_mask;
+
+	return active_filters;
+}
+
+int tzc_configure(uintptr_t tzc, const struct tzc_region *cfg)
+{
+	uintptr_t region = tzc + TZC_REGION1_OFFSET;
+	uint32_t nsid, attr_reg, active_filters;
+	int ret;
+
+	active_filters = tzc_config_get_active_filters(cfg);
+	if (active_filters == 0)
+		return -EINVAL;
+
+	ret = tzc_disable_filters(tzc, active_filters);
+	if (ret < 0)
+		return ret;
+
+	for ( ; cfg->top != 0; cfg++, region += TZC_REGION_CFG_SIZE) {
+		attr_reg = (cfg->sec_mode & 0x03) << 30;
+		attr_reg |= (cfg->filters_mask & 0x03) << 0;
+		nsid = cfg->nsec_id & 0xffff;
+		nsid |= nsid << 16;
+
+		tzc_write(region, TZC_REGION_BASE, cfg->base);
+		tzc_write(region, TZC_REGION_TOP, cfg->top);
+		tzc_write(region, TZC_REGION_ACCESS, nsid);
+		tzc_write(region, TZC_REGION_ATTRIBUTE, attr_reg);
+	}
+
+	tzc_write(tzc, TZC_ACTION, TZC_ACTION_ERR);
+	return tzc_enable_filters(tzc, active_filters);
+}
+
+int tzc_disable_filters(uintptr_t tzc, uint16_t filters_mask)
+{
+	uint32_t gate = tzc_read(tzc, TZC_GATE_KEEPER);
+	uint32_t filter_status = filters_mask << 16;
+
+	gate &= ~filters_mask;
+	tzc_write(tzc, TZC_GATE_KEEPER, gate);
+
+	return readl_poll_timeout(tzc + TZC_GATE_KEEPER, gate,
+				 (gate & filter_status) == 0, TZC_TIMEOUT_US);
+}
+
+int tzc_enable_filters(uintptr_t tzc, uint16_t filters_mask)
+{
+	uint32_t gate = tzc_read(tzc, TZC_GATE_KEEPER);
+	uint32_t filter_status = filters_mask << 16;
+
+	gate |= filters_mask;
+	tzc_write(tzc, TZC_GATE_KEEPER, gate);
+
+	return readl_poll_timeout(tzc + TZC_GATE_KEEPER, gate,
+				 (gate & filter_status) == filter_status,
+				 TZC_TIMEOUT_US);
+}
+
+static const char *sec_access_str_from_attr(uint32_t attr)
+{
+	const char *const sec_mode[] = { "none", "RO  ", "WO  ", "RW  " };
+
+	return sec_mode[(attr >> 30) & 0x03];
+}
+
+void tzc_dump_config(uintptr_t tzc)
+{
+	uint32_t build_config, base, top, attr, nsaid;
+	int num_regions, i;
+	uintptr_t region;
+
+	build_config = tzc_read(tzc, TZC_BUILD_CONFIG);
+	num_regions = ((build_config >> 0) & 0x1f) + 1;
+
+	for (i = 0; i < num_regions; i++) {
+		region = tzc + TZC_REGION0_OFFSET + i * TZC_REGION_CFG_SIZE;
+
+		base = tzc_read(region, TZC_REGION_BASE);
+		top = tzc_read(region, TZC_REGION_TOP);
+		attr = tzc_read(region, TZC_REGION_ATTRIBUTE);
+		nsaid = tzc_read(region, TZC_REGION_ACCESS);
+
+		if (attr == 0 && nsaid == 0)
+			continue;
+
+		log_info("TZC region %u: %08x->%08x - filters 0x%x\n",
+			 i, base, top, (attr >> 0) & 0xf);
+		log_info("\t Secure access %s NSAID %08x\n",
+			 sec_access_str_from_attr(attr), nsaid);
+	}
+}
-- 
2.31.1

[PATCH v5 4/5] stm32mp1: spl: Configure TrustZone controller for OP-TEE

[Alexandru Gagniuc]

OP-TEE is very particular about how the TZC should be configured.
When booting an OP-TEE payload, an incorrect TZC configuration will
result in a panic.

Most information can be derived from the SPL devicetree. The only
information we don't have is the split between TZDRAM and shared
memory. This has to be hardcoded. The rest of the configuration is
fairly easy, and only requires 3 TZC regions. Configure them.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
 arch/arm/mach-stm32mp/spl.c | 92 +++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c
index b53659a698..405eff68a3 100644
--- a/arch/arm/mach-stm32mp/spl.c
+++ b/arch/arm/mach-stm32mp/spl.c
@@ -11,11 +11,13 @@
 #include <hang.h>
 #include <init.h>
 #include <log.h>
+#include <ram.h>
 #include <spl.h>
 #include <asm/cache.h>
 #include <asm/global_data.h>
 #include <asm/io.h>
 #include <asm/arch/sys_proto.h>
+#include <mach/tzc.h>
 #include <linux/libfdt.h>
 
 u32 spl_boot_device(void)
@@ -92,6 +94,96 @@ __weak int board_early_init_f(void)
 	return 0;
 }
 
+uint32_t stm32mp_get_dram_size(void)
+{
+	struct ram_info ram;
+	struct udevice *dev;
+	int ret;
+
+	if (uclass_get_device(UCLASS_RAM, 0, &dev))
+		return 0;
+
+	ret = ram_get_info(dev, &ram);
+	if (ret)
+		return 0;
+
+	return ram.size;
+}
+
+static int optee_get_reserved_memory(uint32_t *start, uint32_t *size)
+{
+	phys_size_t fdt_mem_size;
+	fdt_addr_t fdt_start;
+	ofnode node;
+
+	node = ofnode_path("/reserved-memory/optee");
+	if (!ofnode_valid(node))
+		return 0;
+
+	fdt_start = ofnode_get_addr_size(node, "reg", &fdt_mem_size);
+	*start = fdt_start;
+	*size = fdt_mem_size;
+	return (fdt_start < 0) ? fdt_start : 0;
+}
+
+#define CFG_SHMEM_SIZE			0x200000
+#define STM32_TZC_NSID_ALL		0xffff
+#define STM32_TZC_FILTER_ALL		3
+
+void stm32_init_tzc_for_optee(void)
+{
+	const uint32_t dram_size = stm32mp_get_dram_size();
+	const uintptr_t dram_top = STM32_DDR_BASE + (dram_size - 1);
+	uint32_t optee_base, optee_size, tee_shmem_base;
+	const uintptr_t tzc = STM32_TZC_BASE;
+	int ret;
+
+	if (dram_size == 0)
+		panic("Cannot determine DRAM size from devicetree\n");
+
+	ret = optee_get_reserved_memory(&optee_base, &optee_size);
+	if (ret < 0 || optee_size <= CFG_SHMEM_SIZE)
+		panic("Invalid OPTEE reserved memory in devicetree\n");
+
+	tee_shmem_base = optee_base + optee_size - CFG_SHMEM_SIZE;
+
+	const struct tzc_region optee_config[] = {
+		{
+			.base = STM32_DDR_BASE,
+			.top = optee_base - 1,
+			.sec_mode = TZC_ATTR_SEC_NONE,
+			.nsec_id = STM32_TZC_NSID_ALL,
+			.filters_mask = STM32_TZC_FILTER_ALL,
+		}, {
+			.base = optee_base,
+			.top = tee_shmem_base - 1,
+			.sec_mode = TZC_ATTR_SEC_RW,
+			.nsec_id = 0,
+			.filters_mask = STM32_TZC_FILTER_ALL,
+		}, {
+			.base = tee_shmem_base,
+			.top = dram_top,
+			.sec_mode = TZC_ATTR_SEC_NONE,
+			.nsec_id = STM32_TZC_NSID_ALL,
+			.filters_mask = STM32_TZC_FILTER_ALL,
+		}, {
+			.top = 0,
+		}
+	};
+
+	flush_dcache_all();
+
+	tzc_configure(tzc, optee_config);
+	tzc_dump_config(tzc);
+
+	dcache_disable();
+}
+
+void spl_board_prepare_for_optee(void *fdt)
+{
+	stm32_init_tzc_for_optee();
+}
+
 void board_init_f(ulong dummy)
 {
 	struct udevice *dev;
-- 
2.31.1

[PATCH v5 5/5] ARM: dts: stm32mp: Add OP-TEE reserved memory to SPL dtb

[Alexandru Gagniuc]

Add the "/reserved-memory/optee" node to the SPL devicetree. The
purpose is to allow configuring TZC regions when booting OP-TEE.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi b/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
index 6787619290..55d634f7db 100644
--- a/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
+++ b/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
@@ -30,9 +30,12 @@
 	};
 
 	reserved-memory {
+		u-boot,dm-spl;
+
 		optee@de000000 {
 			reg = <0xde000000 0x02000000>;
 			no-map;
+			u-boot,dm-spl;
 		};
 	};
 
-- 
2.31.1

Re: [PATCH v5 0/5] stm32mp: Enable OP-TEE and TZC support in SPL

[Alex G.]

On 7/15/21 2:19 PM, Alexandru Gagniuc wrote:
> v4 branch was reported to have some issues with SPL becoming too big
> on some platforms (e.g. imx6dl_mamoj) This is fixed by dropping the
> call to genimg_get_os_name().

Ping for merge window.

> Alexandru Gagniuc (5):
>    spl: mmc: Support OP-TEE payloads in Falcon mode
>    spl: Introduce spl_board_prepare_for_optee() hook
>    arm: stm32mp: Implement support for TZC 400 controller
>    stm32mp1: spl: Configure TrustZone controller for OP-TEE
>    ARM: dts: stm32mp: Add OP-TEE reserved memory to SPL dtb
> 
>   arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi |   3 +
>   arch/arm/mach-stm32mp/Makefile           |   1 +
>   arch/arm/mach-stm32mp/include/mach/tzc.h |  33 ++++++
>   arch/arm/mach-stm32mp/spl.c              |  92 +++++++++++++++
>   arch/arm/mach-stm32mp/tzc400.c           | 136 +++++++++++++++++++++++
>   common/spl/spl.c                         |   5 +
>   common/spl/spl_mmc.c                     |   4 +-
>   include/spl.h                            |  14 +++
>   8 files changed, 286 insertions(+), 2 deletions(-)
>   create mode 100644 arch/arm/mach-stm32mp/include/mach/tzc.h
>   create mode 100644 arch/arm/mach-stm32mp/tzc400.c
> 

Re: [PATCH v5 1/5] spl: mmc: Support OP-TEE payloads in Falcon mode

[Patrick DELAUNAY]

Hi,

On 7/15/21 9:19 PM, Alexandru Gagniuc wrote:
> In general, Falcon mode means we're booting a linux kernel directly.
> With FIT images, however, an OP-TEE secure kernel can be booted before
> linux. Thus, if the next stage is an IH_OS_TEE, this isn't necessarily
> a problem.
>
> Of course, a general solution would involve mmc_load_image_raw_os()
> only loading the binary, and leaving the decision of suitability to
> someone else. However, a rework of the boot flow is beyond the scope
> of this patch. Accept IH_OS_TEE as a valid OS value.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>
> Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
> ---
>   common/spl/spl_mmc.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>


Applied to u-boot-stm/master, thanks!

Regards
Patrick

Re: [PATCH v5 2/5] spl: Introduce spl_board_prepare_for_optee() hook

[Patrick DELAUNAY]

Hi,

On 7/15/21 9:19 PM, Alexandru Gagniuc wrote:
> OP-TEE requires some particular setup, which is not needed for linux
> or other payloads. Add a hook for platform-specific code to perform
> any OP-TEE related configuration and initialization.
>
> A weak function is used because it is symmetrical to other
> spl_board_prepare_for_*() implementations. A solution to avoid the use
> of weak functions would trivially apply to all these implementations.
> However, re-designing this is beyond the scope of this patch.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
> ---
>   common/spl/spl.c |  5 +++++
>   include/spl.h    | 14 ++++++++++++++
>   2 files changed, 19 insertions(+)




Applied to u-boot-stm/master, thanks!

Regards
Patrick

Re: [PATCH v5 3/5] arm: stm32mp: Implement support for TZC 400 controller

[Patrick DELAUNAY]

Hi,

On 7/15/21 9:19 PM, Alexandru Gagniuc wrote:
> The purpose of this change is to allow configuring TrustZone (TZC)
> memory permissions. For example, OP-TEE expects TZC regions to be
> configured in a very particular way. The API presented here is
> intended to allow exactly that.
>
> UCLASS support is not implemented, because it would not be too useful.
> Changing TZC permissions needs to be done with care, so as not to cut
> off access to memory we are currently using. One place where we can
> use this is at the end of SPL, right before jumping to OP-TEE.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
> ---
>   arch/arm/mach-stm32mp/Makefile           |   1 +
>   arch/arm/mach-stm32mp/include/mach/tzc.h |  33 ++++++
>   arch/arm/mach-stm32mp/tzc400.c           | 136 +++++++++++++++++++++++
>   3 files changed, 170 insertions(+)
>   create mode 100644 arch/arm/mach-stm32mp/include/mach/tzc.h
>   create mode 100644 arch/arm/mach-stm32mp/tzc400.c
>

Applied to u-boot-stm/master, thanks!

Regards
Patrick

Re: [PATCH v5 4/5] stm32mp1: spl: Configure TrustZone controller for OP-TEE

[Patrick DELAUNAY]

hI,

On 7/15/21 9:19 PM, Alexandru Gagniuc wrote:
> OP-TEE is very particular about how the TZC should be configured.
> When booting an OP-TEE payload, an incorrect TZC configuration will
> result in a panic.
>
> Most information can be derived from the SPL devicetree. The only
> information we don't have is the split between TZDRAM and shared
> memory. This has to be hardcoded. The rest of the configuration is
> fairly easy, and only requires 3 TZC regions. Configure them.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
>   arch/arm/mach-stm32mp/spl.c | 92 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 92 insertions(+)
>

Applied to u-boot-stm/master, thanks!

Regards
Patrick

Re: [PATCH v5 5/5] ARM: dts: stm32mp: Add OP-TEE reserved memory to SPL dtb

[Patrick DELAUNAY]

Hi,


On 7/15/21 9:19 PM, Alexandru Gagniuc wrote:
> Add the "/reserved-memory/optee" node to the SPL devicetree. The
> purpose is to allow configuring TZC regions when booting OP-TEE.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
>   arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi | 3 +++
>   1 file changed, 3 insertions(+)

Applied to u-boot-stm/master, thanks!

Regards
Patrick