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

Patrick DELAUNAY patrick.delaunay at foss.st.com
Fri May 28 11:59:08 CEST 2021


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 at 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 at foss.st.com>

Thanks
Patrick



More information about the U-Boot mailing list