Add TZC400 device tree bindings + configure STM32 TZC400 using the device tree on stm32mp1 platforms #7064

GseoC · 2024-10-02T09:22:24Z

This P-R adds a device tree binding file to be able to describe the TZC400 memory regions with their firewall attributes configuration in the device tree.

Refactor the stm32 platform TZC400 driver to support this feature and add the DT configuration of a memory region on the stm32mp135f-dk platform.

Note: I tried to make a generic TZC400 binding file so that it can be used by other platforms, let me know what you think of it. These "bindings" are more like macros to be used in the DT.

gagachang · 2024-10-03T05:31:43Z

Hi @GseoC , may I know why the permissions are embedded in optee_framebuffer node with st,protreg property ?

Another way is to embed them into tzc400 node, like:

&tzc400 {
-	memory-region = <&optee_framebuffer>;
+	memory-region = <&optee_framebuffer DT_TZC_REGION_S_RDWR 0>;
};

Any special consideration for this ? Thanks!

GseoC · 2024-10-03T07:39:29Z

Hi @GseoC , may I know why the permissions are embedded in optee_framebuffer node with st,protreg property ?

Another way is to embed them into tzc400 node, like:
&tzc400 {
-	memory-region = <&optee_framebuffer>;
+	memory-region = <&optee_framebuffer DT_TZC_REGION_S_RDWR 0>;
};
Any special consideration for this ? Thanks!

Hi @gagachang,

I cannot do this because the memory-region dt-bindings file states that the content of the property is a phandle array : memory region YAML. Each phandle is a /reserved-memory child node assigned to the device.

For now I have to use a proprietary property- but I hope that I'll be able to soon use a generic one that is a variant of access-controllers to be able to describe firewall configurations in the device tree. I've proposed a patch at Linux upstream: Patch but since I'm alone (for now) to push for this, maintainers aren't keen on replying fast...

gagachang · 2024-10-04T02:31:51Z

Hi @GseoC , Thanks for information!

I cannot do this because the memory-region dt-bindings file states that the content of the property is a phandle array : memory region YAML. Each phandle is a /reserved-memory child node assigned to the device.

I am trying to understand the access-controllers property.
IIUC, the memory-region property is only used when the phandle is a /reserved-memory child node.
If the phandle is not a /reserved-memory child node (e.g., uart5 in your Patch), we should use the access-controllers property instead. Am I correct?

For now I have to use a proprietary property- but I hope that I'll be able to soon use a generic one that is a variant of access-controllers to be able to describe firewall configurations in the device tree. I've proposed a patch at Linux upstream: Patch but since I'm alone (for now) to push for this, maintainers aren't keen on replying fast...

I saw your #7066. Is st,decprot the proprietary property you mentioned ?

+ &etzpc {
+	st,decprot =
+		<&etzpc DECPROT(STM32MP1_ETZPC_USART1_ID, DECPROT_NS_RW, DECPROT_UNLOCK)>,
+		<&etzpc DECPROT(STM32MP1_ETZPC_SPI6_ID, DECPROT_NS_RW, DECPROT_UNLOCK)>,
+		<&etzpc DECPROT(STM32MP1_ETZPC_I2C4_ID, DECPROT_NS_RW, DECPROT_UNLOCK)>,
+		<&etzpc DECPROT(STM32MP1_ETZPC_I2C6_ID, DECPROT_NS_RW, DECPROT_UNLOCK)>,
+		<&etzpc DECPROT(STM32MP1_ETZPC_RNG1_ID, DECPROT_NS_RW, DECPROT_UNLOCK)>,
+		<&etzpc DECPROT(STM32MP1_ETZPC_HASH1_ID, DECPROT_NS_RW, DECPROT_UNLOCK)>,
+		<&etzpc DECPROT(STM32MP1_ETZPC_CRYP1_ID, DECPROT_NS_RW, DECPROT_UNLOCK)>,
+		<&etzpc DECPROT(STM32MP1_ETZPC_DDRCTRL_ID, DECPROT_NS_R_S_W, DECPROT_LOCK)>,
+		<&etzpc DECPROT(STM32MP1_ETZPC_DDRPHYC_ID, DECPROT_NS_R_S_W, DECPROT_LOCK)>,
+		<&etzpc DECPROT(STM32MP1_ETZPC_STGENC_ID, DECPROT_S_RW, DECPROT_LOCK)>,
+		<&etzpc DECPROT(STM32MP1_ETZPC_BKPSRAM_ID, DECPROT_S_RW, DECPROT_LOCK)>,
+		<&etzpc DECPROT(STM32MP1_ETZPC_IWDG1_ID, DECPROT_S_RW, DECPROT_LOCK)>;
+ };

GseoC · 2024-10-04T07:40:59Z

I am trying to understand the access-controllers property.
IIUC, the memory-region property is only used when the phandle is a /reserved-memory child node.
If the phandle is not a /reserved-memory child node (e.g., uart5 in your Patch), we should use the access-controllers property instead. Am I correct?

The access-controllers property is basically used to link an access controller(that can be a firewall controller) to its consumers. Its consumers can either be subnodes or completely separate nodes in the device tree. It should be used when a device needs a service from an access controller(In OP-TEE case, a firewall controller) on a resource that is under its responsibility.

As for the firewall configuration, for now, there's no way to describe a firewall controller configuration with a generic property. Hence my effort on the above linux patch.

I saw your #7066. Is st,decprot the proprietary property you mentioned ?

The st,decprot property is used to describe ETZPC firewall boot configuration. It is a proprietary property and only makes sense for STMicroelectronics' ETZPC driver implementation. If I could use a generic property: something like default-access-config I would :) . And i would try to implement ops in the OP-TEE firewall framework to be able to set a firewall configuration. See this P-R

GseoC · 2024-10-04T07:46:13Z

I am trying to understand the access-controllers property.
IIUC, the memory-region property is only used when the phandle is a /reserved-memory child node.
If the phandle is not a /reserved-memory child node (e.g., uart5 in your Patch), we should use the access-controllers property instead. Am I correct?

The access-controllers property is basically used to link an access controller(that can be a firewall controller) to its consumers. Its consumers can either be subnodes or completely separate nodes in the device tree. It should be used when a device needs a service from an access controller(In OP-TEE case, a firewall controller) on a resource that is under its responsibility.

As for the firewall configuration, for now, there's no way to describe a firewall controller configuration with a generic property. Hence my effort on the above linux patch.

I saw your #7066. Is st,decprot the proprietary property you mentioned ?

The st,decprot property is used to describe ETZPC firewall boot configuration. It is a proprietary property and only makes sense for STMicroelectronics' ETZPC driver implementation. If I could use a generic property: something like default-access-config I would :) . And i would try to implement ops in the OP-TEE firewall framework to be able to set a firewall configuration. See this P-R

(STM32 use-case) For our platforms, we may need to perform some firewall operations before using a peripheral to be able to access it. See in core/drivers/firewall/stm32_rifsc.c, the function stm32_rifsc_dt_probe_bus(). We do lighter checks for the ETZPC (Only check that the peripheral is secure before using it in OP-TEE)

gagachang · 2024-10-04T09:00:09Z

Thank you @GseoC !
I am working on RISC-V access protection controller.
We may need similar way to configure the registers of access protection controller using the device tree.
We will refer to your effort. Best wishes on your patch.

core/include/dt-bindings/firewall/tzc400.h

core/include/drivers/stm32mp_dt_bindings.h

core/include/dt-bindings/firewall/stm32mp13-tzc400.h

etienne-lms · 2024-10-04T07:35:31Z

core/include/dt-bindings/firewall/stm32mp15-tzc400.h

+
+#define TZC_REGION_NSEC_ALL_ACCESS_RDWR \
+	(TZC_REGION_ACCESS_RDWR(STM32MP1_TZC_A7_ID) | \
+	 TZC_REGION_ACCESS_RDWR(STM32MP1_TZC_GPU_ID) | \


Could you use the same order as for lines 10 to 20?

core/arch/arm/plat-stm32mp1/plat_tzc400.c

GseoC · 2024-10-04T12:26:32Z

Thank you @GseoC ! I am working on RISC-V access protection controller. We may need similar way to configure the registers of access protection controller using the device tree. We will refer to your effort. Best wishes on your patch.

Good to hear that we share that need, it makes more sense to find a proper generic solution.

@gagachang, I'll send another version of the other linux patch soon. Do you want to be put as To/CC of the patch so you can give your opinion or state that you may be interested in it?

gagachang · 2024-10-07T01:39:42Z

@gagachang, I'll send another version of the other linux patch soon. Do you want to be put as To/CC of the patch so you can give your opinion or state that you may be interested in it?

Sure! Please CC that patch to my email: [email protected]. Thanks!

GseoC · 2024-10-07T17:08:25Z

Comments addressed.

core/arch/arm/plat-stm32mp1/plat_tzc400.c

GseoC · 2024-10-14T09:17:18Z

Comments addressed

GseoC · 2024-10-25T15:07:04Z

@etienne-lms, any more comments?

etienne-lms

2 last comments. LGTM otherwise.
Can you squash the fixup commits?

core/arch/arm/plat-stm32mp1/plat_tzc400.c

etienne-lms · 2024-10-28T11:06:23Z

core/arch/arm/plat-stm32mp1/plat_tzc400.c

+	if (reg_exclude->base == reg->region.base &&
+	    reg_exclude->top == reg->region.top) {
+		/* Remove this entry */
+		SLIST_REMOVE(&nsec_region_list, reg, tzc_region_non_sec, link);


free(reg)?

I'm changing to SLIST_FOREACH_SAFE above then.

Actually sorry, there's no need as the free occurs outside the loop. I'm reverting this change

GseoC · 2024-10-28T12:29:07Z

Comments addressed and squashed

etienne-lms

Reviewed-by: Etienne Carriere <[email protected]> with below comment addressed. Other checkpatch reports are false positive related to trace message instruction exceeding 80char/line.

The other CI test errors are due to https://xenbits.xen.org/git-http/xen.git not responding. Likely a temporary issue in Xen repositories server.

etienne-lms · 2024-10-28T13:39:53Z

core/arch/arm/plat-stm32mp1/plat_tzc400.c

+{
+	TEE_Result res = TEE_SUCCESS;
+	unsigned int index = 0;
+	index = tzc_dev->nb_reg_used;


Reported by checkpatch:

unsigned int index = 0; + index = tzc_dev->nb_reg_used;

or

unsigned int index = tzc_dev->nb_reg_used;

For added flexibility, the TZC400 configuration could be set through the device tree. Add macros to be able to do so. Signed-off-by: Gatien Chevallier <[email protected]> Reviewed-by: Etienne Carriere <[email protected]>

Add stm32 specific peripheral IDs for the TZC400 configuration. Signed-off-by: Gatien Chevallier <[email protected]> Reviewed-by: Etienne Carriere <[email protected]>

Add the TZC400 node in the stm32mp151 SoC device tree file and default enable it. Signed-off-by: Gatien Chevallier <[email protected]> Reviewed-by: Etienne Carriere <[email protected]>

Add the usage of device tree memory regions defined to configure the TZC400 firewall controller. Signed-off-by: Gatien Chevallier <[email protected]> Reviewed-by: Etienne Carriere <[email protected]>

Add support for the TZC400 configuration for the optee_framebuffer memory region on the stm32mp135f-dk board Signed-off-by: Gatien Chevallier <[email protected]> Reviewed-by: Etienne Carriere <[email protected]>

GseoC · 2024-10-28T14:18:30Z

Comment addressed and tags applied, thanks

GseoC force-pushed the stm32-tzc400 branch from f652f16 to d28c175 Compare October 2, 2024 09:33

etienne-lms reviewed Oct 4, 2024

View reviewed changes

GseoC force-pushed the stm32-tzc400 branch from d28c175 to 8b44757 Compare October 7, 2024 17:03

etienne-lms reviewed Oct 11, 2024

View reviewed changes

GseoC force-pushed the stm32-tzc400 branch from 8b44757 to 1526854 Compare October 14, 2024 09:17

etienne-lms reviewed Oct 28, 2024

View reviewed changes

GseoC force-pushed the stm32-tzc400 branch from 1526854 to a3e0061 Compare October 28, 2024 12:28

GseoC force-pushed the stm32-tzc400 branch from a3e0061 to 4ef4835 Compare October 28, 2024 12:38

etienne-lms reviewed Oct 28, 2024

View reviewed changes

GseoC added 5 commits October 28, 2024 15:15

dt-bindings: add support for the TZC400 configuration

74fac61

For added flexibility, the TZC400 configuration could be set through the device tree. Add macros to be able to do so. Signed-off-by: Gatien Chevallier <[email protected]> Reviewed-by: Etienne Carriere <[email protected]>

dt-bindings: add stm32 bindings for TZC400 platform configuration

2c40acf

Add stm32 specific peripheral IDs for the TZC400 configuration. Signed-off-by: Gatien Chevallier <[email protected]> Reviewed-by: Etienne Carriere <[email protected]>

dts: stm32: add TZC400 node in the stm32mp151 SoC device tree file

126bc1a

Add the TZC400 node in the stm32mp151 SoC device tree file and default enable it. Signed-off-by: Gatien Chevallier <[email protected]> Reviewed-by: Etienne Carriere <[email protected]>

plat-stm32mp1: update the plat_tzc400 driver to support device tree

4d5cce3

Add the usage of device tree memory regions defined to configure the TZC400 firewall controller. Signed-off-by: Gatien Chevallier <[email protected]> Reviewed-by: Etienne Carriere <[email protected]>

dts: stm32: secure optee_framebuffer memory region on stm32mp135f-dk

cc88984

Add support for the TZC400 configuration for the optee_framebuffer memory region on the stm32mp135f-dk board Signed-off-by: Gatien Chevallier <[email protected]> Reviewed-by: Etienne Carriere <[email protected]>

GseoC force-pushed the stm32-tzc400 branch from 4ef4835 to cc88984 Compare October 28, 2024 14:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add TZC400 device tree bindings + configure STM32 TZC400 using the device tree on stm32mp1 platforms #7064

Add TZC400 device tree bindings + configure STM32 TZC400 using the device tree on stm32mp1 platforms #7064

GseoC commented Oct 2, 2024 •

edited

Loading

gagachang commented Oct 3, 2024

GseoC commented Oct 3, 2024 •

edited

Loading

gagachang commented Oct 4, 2024

GseoC commented Oct 4, 2024

GseoC commented Oct 4, 2024

gagachang commented Oct 4, 2024

etienne-lms Oct 4, 2024

GseoC commented Oct 4, 2024

gagachang commented Oct 7, 2024

GseoC commented Oct 7, 2024

GseoC commented Oct 14, 2024

GseoC commented Oct 25, 2024

etienne-lms left a comment

etienne-lms Oct 28, 2024

GseoC Oct 28, 2024

GseoC Oct 28, 2024

GseoC commented Oct 28, 2024

etienne-lms left a comment

etienne-lms Oct 28, 2024

GseoC commented Oct 28, 2024

Add TZC400 device tree bindings + configure STM32 TZC400 using the device tree on stm32mp1 platforms #7064

Are you sure you want to change the base?

Add TZC400 device tree bindings + configure STM32 TZC400 using the device tree on stm32mp1 platforms #7064

Conversation

GseoC commented Oct 2, 2024 • edited Loading

gagachang commented Oct 3, 2024

GseoC commented Oct 3, 2024 • edited Loading

gagachang commented Oct 4, 2024

GseoC commented Oct 4, 2024

GseoC commented Oct 4, 2024

gagachang commented Oct 4, 2024

etienne-lms Oct 4, 2024

Choose a reason for hiding this comment

GseoC commented Oct 4, 2024

gagachang commented Oct 7, 2024

GseoC commented Oct 7, 2024

GseoC commented Oct 14, 2024

GseoC commented Oct 25, 2024

etienne-lms left a comment

Choose a reason for hiding this comment

etienne-lms Oct 28, 2024

Choose a reason for hiding this comment

GseoC Oct 28, 2024

Choose a reason for hiding this comment

GseoC Oct 28, 2024

Choose a reason for hiding this comment

GseoC commented Oct 28, 2024

etienne-lms left a comment

Choose a reason for hiding this comment

etienne-lms Oct 28, 2024

Choose a reason for hiding this comment

GseoC commented Oct 28, 2024

GseoC commented Oct 2, 2024 •

edited

Loading

GseoC commented Oct 3, 2024 •

edited

Loading