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

[Patrick DELAUNAY]

Hi

On 3/15/21 4:47 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           | 133 +++++++++++++++++++++++
>   3 files changed, 167 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 c8aa24d489..1b878c5a85 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-$(CONFIG_CMD_STM32PROG) += 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.
> + */

As we activate LOG feature, can you add the define:

#define LOG_CATEGORY LOGC_ARCH

> +#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);

(...)


> +
> +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;
> +
> +		pr_info("TZC region %u: %08x->%08x - filters 0x%x\n",
> +			i, base, top, (attr >> 0) & 0xf);
> +		pr_info("\t Secure access %s NSAID %08x\n",
> +			sec_access_str_from_attr(attr), nsaid);
Can you use "log_info" instead of "pr_info" here....
> +	}
> +}

except this 2 minors comment, Ok with the path

Patrick