[PATCH v3 0/5] stm32mp: Enable OP-TEE and TZC support in SPL
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 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 | 90 +++++++++++++++ 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, 286 insertions(+), 2 deletions(-) create mode 100644 arch/arm/mach-stm32mp/include/mach/tzc.h create mode 100644 arch/arm/mach-stm32mp/tzc400.c
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 add2785b4e..bab558d055 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; }
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 556a91ab53..190c8fd1b3 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -162,6 +162,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! */ @@ -723,6 +727,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 4f6e0e53f5..a3bedbc797 100644 --- a/include/spl.h +++ b/include/spl.h @@ -396,6 +396,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);
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 --- 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); + } +}
Hi,
On 4/15/21 6:48 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
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);- }
+}
Any reason to prefer uint16_t and uint32_t ?
See checkpatch warning
arch/arm/mach-stm32mp/include/mach/tzc.h:24: check: Prefer kernel type 'u16' over 'uint16_t' arch/arm/mach-stm32mp/include/mach/tzc.h:25: check: Prefer kernel type 'u16' over 'uint16_t' arch/arm/mach-stm32mp/tzc400.c:41: check: Prefer kernel type 'u16' over 'uint16_t' arch/arm/mach-stm32mp/tzc400.c:52: check: Prefer kernel type 'u32' over 'uint32_t' arch/arm/mach-stm32mp/tzc400.c:81: check: Prefer kernel type 'u32' over 'uint32_t' arch/arm/mach-stm32mp/tzc400.c:82: check: Prefer kernel type 'u32' over 'uint32_t' arch/arm/mach-stm32mp/tzc400.c:93: check: Prefer kernel type 'u32' over 'uint32_t' arch/arm/mach-stm32mp/tzc400.c:94: check: Prefer kernel type 'u32' over 'uint32_t' arch/arm/mach-stm32mp/tzc400.c:113: check: Prefer kernel type 'u32' over 'uint32_t'
But except these remarks:
Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com
Thanks Patrick
Hi Patrick,
On 5/28/21 4:59 AM, Patrick DELAUNAY wrote:
Hi,
Any reason to prefer uint16_t and uint32_t ?
I use standard C types. u32, _u32, and __u32 are non-standard, and they have different meanings depending on how they're used. I avoid them for this reason.
Alex
See checkpatch warning
arch/arm/mach-stm32mp/include/mach/tzc.h:24: check: Prefer kernel type 'u16' over 'uint16_t' arch/arm/mach-stm32mp/include/mach/tzc.h:25: check: Prefer kernel type 'u16' over 'uint16_t' arch/arm/mach-stm32mp/tzc400.c:41: check: Prefer kernel type 'u16' over 'uint16_t' arch/arm/mach-stm32mp/tzc400.c:52: check: Prefer kernel type 'u32' over 'uint32_t' arch/arm/mach-stm32mp/tzc400.c:81: check: Prefer kernel type 'u32' over 'uint32_t' arch/arm/mach-stm32mp/tzc400.c:82: check: Prefer kernel type 'u32' over 'uint32_t' arch/arm/mach-stm32mp/tzc400.c:93: check: Prefer kernel type 'u32' over 'uint32_t' arch/arm/mach-stm32mp/tzc400.c:94: check: Prefer kernel type 'u32' over 'uint32_t' arch/arm/mach-stm32mp/tzc400.c:113: check: Prefer kernel type 'u32' over 'uint32_t'
But except these remarks:
Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com
Thanks Patrick
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 | 90 +++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+)
diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c index b53659a698..1e26f18794 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,94 @@ __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) +{ + ofnode node; + fdt_addr_t fdt_start; + + node = ofnode_path("/reserved-memory/optee"); + if (!ofnode_valid(node)) + return 0; + + fdt_start = ofnode_get_addr_size(node, "reg", size); + *start = fdt_start; + 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;
Hi,
On 4/15/21 6:48 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 | 90 +++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+)
diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c index b53659a698..1e26f18794 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,94 @@ __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) +{
- ofnode node;
- fdt_addr_t fdt_start;
- node = ofnode_path("/reserved-memory/optee");
- if (!ofnode_valid(node))
return 0;- fdt_start = ofnode_get_addr_size(node, "reg", size);
warning here because size is 'fdt_size_t *' not 'u32*'
arch/arm/mach-stm32mp/spl.c:122:48: warning: passing argument 3 of ‘ofnode_get_addr_size’ from inc
- *start = fdt_start;
- 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);
Dump is always require, even for nomal boot, or only for debug cases ?
- dcache_disable();
You disable cache why,
it is not supported by OP-TEE ?
=> if it is a generic issue it should be in spl.c for case IH_OS_TEE or in
spl_optee_entry() and not in board specific weak function
as it done in bl31_entry for TF-A
Regards
Patrick
On 5/28/21 5:22 AM, Patrick DELAUNAY wrote:
Hi,
On 4/15/21 6:48 PM, Alexandru Gagniuc wrote:
[snip]
+ fdt_start = ofnode_get_addr_size(node, "reg", size);
warning here because size is 'fdt_size_t *' not 'u32*'
arch/arm/mach-stm32mp/spl.c:122:48: warning: passing argument 3 of ‘ofnode_get_addr_size’ from inc
Fixed in v4
+ tzc_configure(tzc, optee_config); + tzc_dump_config(tzc);
Dump is always require, even for nomal boot, or only for debug cases ?
Because tzc_dump_config() uses log_info(), only prints when the appropriate log level is enabled. This is by design, so that we don't need extra logic here to invoke dump().
+ dcache_disable();
You disable cache why,
it is not supported by OP-TEE ?
=> if it is a generic issue it should be in spl.c for case IH_OS_TEE or in
spl_optee_entry() and not in board specific weak function
as it done in bl31_entry for TF-A
I don't know if this can be safely generalized. stm32mp is the only platform to enable TZC in SPL, which brings special constraints:
We're running in secure mode, and we've touched memory that will be reserved to the normal world, for example the linux devicetree. Once we enable TZC, we can't touch that memory anymore. This could happen later as the CPU is evicting cache lines.
To make sure we don't hit a TZC violation as cache lines are evicted, we both flush the dcache, and disable the dcache. Doing this another way is not tested, and I can't guarantee that other variations will work reliably.
Alex
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..9c4100c39c 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-pre-reloc; + optee@de000000 { reg = <0xde000000 0x02000000>; no-map; + u-boot,dm-pre-reloc; }; };
Hi Alexandru,
On 4/15/21 6:48 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 6787619290..9c4100c39c 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-pre-reloc;- optee@de000000 { reg = <0xde000000 0x02000000>; no-map;
u-boot,dm-pre-reloc;
As it is only needed by SPL, and not by U-Boot before relocation, can you replace
u-boot,dm-pre-reloc;
by
u-boot,dm-spl;
it should be more clear.
Regards
Patrick
participants (3)
-
Alex G. -
Alexandru Gagniuc
-
Patrick DELAUNAY