TI: Refactor firewall enablement to be generic and enable firewall for DTHEv2 #7653

jsuhaas22 · 2025-12-16T05:29:22Z

This PR firewalls DTHEv2 module, which is used in AM62L SoC.

SA2UL is already firewall'ed, and the code for doing that is in SA2UL's driver. Therefore this PR first makes that code block a generic ti_sci function, and then calls it in SA2UL and DTHEv2.

github-actions · 2025-12-16T05:29:51Z

FYI @glneo

shiva-ti · 2025-12-16T06:26:34Z

core/arch/arm/plat-k3/drivers/ti_sci.c

+	start_address = RNG_BASE;
+	end_address = RNG_BASE + RNG_REG_SIZE - 1;
+	permissions[num_perm++] = (FW_BIG_ARM_PRIVID << 16) | FW_SECURE_ONLY;
+#if defined(PLATFORM_FLAVOR_am62x) || \


@jsuhaas22 I don't see the Platform checks being used in earlier sa2ul.c file while setting permissions. Can you please confirm this will not create an issue for Jacinto/am64 platforms

@shiva-ti I wrote this code a few months back. Since then, the #ifdefined block in SA2UL was removed in commit 7c90e11. But since the function is a new and separate one in ti_sci.c, nothing was caught during the rebase.

I have removed that part now, and tested it on AM62L and AM62x.

Pratham-T

Hi @jsuhaas22, some comments below.

Pratham-T · 2025-12-16T06:38:07Z

core/arch/arm/plat-k3/drivers/ti_sci.c

+int ti_sci_init_rng_fwl(uint16_t fwl_id, uint16_t sec_accel_region)
+{
+	uint16_t rng_region = RNG_TI_SCI_FW_RGN_ID;
+	uint8_t owner_index = OPTEE_HOST_ID;
+	uint8_t owner_privid = 0;
+	uint16_t owner_permission_bits = 0;
+	uint32_t control = 0;
+	uint32_t permissions[FWL_MAX_PRIVID_SLOTS] = { };
+	uint32_t num_perm = 0;
+	uint64_t start_address = 0;
+	uint64_t end_address = 0;
+	int ret = 0;
+
+	/* Try to claim Security Accelerator firewall for ourselves */
+	ret = ti_sci_change_fwl_owner(fwl_id, sec_accel_region, owner_index,
+				      &owner_privid, &owner_permission_bits);
+	if (ret) {
+		/*
+		 * This is not fatal, it just means we are on an HS device
+		 * where the DMSC already owns the accelerator. On GP we need
+		 * to do additional setup for access permissions below.
+		 */
+		DMSG("Could not change Security Accelerator firewall owner");
+	} else {
+		IMSG("Fixing Security Accelerator firewall owner in GP device");
+
+		/* Get current firewall configuration */
+		ret = ti_sci_get_fwl_region(fwl_id, sec_accel_region, 1,
+					    &control, permissions,
+					    &start_address, &end_address);
+		if (ret) {
+			EMSG("Could not get firewall region information");
+			return TEE_ERROR_GENERIC;
+		}
+
+		/* Modify firewall to allow all others access*/
+		control = FW_BACKGROUND_REGION | FW_ENABLE_REGION;
+		permissions[0] = (FW_WILDCARD_PRIVID << 16) | FW_NON_SECURE;
+		ret = ti_sci_set_fwl_region(fwl_id, sec_accel_region, 1,
+					    control, permissions,
+					    0x0, UINT32_MAX);
+		if (ret) {
+			EMSG("Could not set firewall region information");
+			return TEE_ERROR_GENERIC;
+		}
+	}
+	/* Claim the TRNG firewall for ourselves */
+	ret = ti_sci_change_fwl_owner(fwl_id, rng_region, owner_index,
+				      &owner_privid, &owner_permission_bits);
+	if (ret) {
+		EMSG("Could not change TRNG firewall owner");
+		return TEE_ERROR_GENERIC;
+	}


This is actually unnecessary for AM62L. The eip76 TRNG block is outside the DTHEv2 block and has a separate firewall ID. So the code acquiring security accelerator region is essentially doing nothing in HS devices and is configuring 2 regions in the TRNG firewall in GP devices as you are using the same fwl ID for both.

Done @Pratham-T

Pratham-T · 2025-12-16T06:41:32Z

core/arch/arm/plat-k3/drivers/ti_sci.c

 	return 0;
 }

+int ti_sci_init_rng_fwl(uint16_t fwl_id, uint16_t sec_accel_region)


This is very specific to rng devices and does not look fit to keep in ti_sci.c, which has only ti_sci API calls. Can this be moved in some crypto specific common file and be refactored to address my other comment as well?

glneo · 2025-12-19T18:48:40Z

core/arch/arm/plat-k3/drivers/ti_crypto.c

+ * Texas Instruments Crypto Operations
+ *
+ * Copyright (C) 2025 Texas Instruments Incorporated - https://www.ti.com/
+ *	Suhaas Joshi <s-joshi@ti.com>


Bold of you to take existing code, move it to a different file, and claim it as your own..

I usually see only one name in the copyright section, so I assumed whoever creates the file should put theirs there. Sorry, fixed.

glneo · 2025-12-19T18:49:01Z

core/arch/arm/plat-k3/drivers/ti_crypto.c

+#include "ti_crypto.h"
+
+TEE_Result ti_crypto_init_rng_fwl(uint16_t fwl_id, uint16_t fwl_region)
+


Random newline.

glneo · 2025-12-19T18:52:31Z

core/arch/arm/plat-k3/drivers/ti_crypto.c

+	ret = ti_sci_change_fwl_owner(fwl_id, fwl_region, owner_index,
+				      &owner_privid, &owner_permission_bits);
+	if (ret) {
+#if !defined(PLATFORM_FLAVOR_am62lx)


This wasn't in the original. Do only the refactor in its own commit, all other changes go in follow up commits. That makes it easier to see what you are changing vs just moving around.

glneo · 2025-12-30T17:47:38Z

First commit looks better, thanks.

For the second, the original intention of fetching the current configuration is that we should be modifying that, not completely replacing it. But yes, currently we do overwrite all the returned setting so fetching first doesn't really do anything.

Anyway, for the series, Reviewed-by: Andrew Davis <afd@ti.com>

jenswi-linaro · 2026-01-02T07:09:38Z

@Pratham-T do you have any more comments?

Pratham-T

LGTM.

Reviewed-by: T Pratham <t-pratham@ti.com>

etienne-lms · 2026-01-08T14:59:25Z

core/arch/arm/plat-k3/drivers/ti_crypto.c

+#include <platform_config.h>
+#include <drivers/ti_sci.h>


Nitpicking: could you swap thee 2 lines above (alphabetical order)?
And add <trace.h>.

etienne-lms · 2026-01-08T15:01:56Z

core/arch/arm/plat-k3/drivers/ti_crypto.c

+	uint32_t num_perm = 0;
+	uint64_t start_address = 0;
+	uint64_t end_address = 0;
+	TEE_Result ret = TEE_SUCCESS;


Suggested change

TEE_Result ret = TEE_SUCCESS;

int ret = 0;

I have done this, but why? We are assigning TEE error codes to this variable throughout the function, so isn't TEE_RESULT more appropriate?

etienne-lms · 2026-01-08T15:02:58Z

core/arch/arm/plat-k3/drivers/ti_crypto.c

+		return TEE_ERROR_GENERIC;
+	}
+
+	return ret;


Prefer explicit return TEE_SUCCESS, for consistency.

core/arch/arm/plat-k3/drivers/ti_crypto.h

+ * ti_crypto_init_rng_fwl() - Initialize RNG firewall in SA2UL/DTHEv2
+ *
+ * Sets the firewall for the RNG region in SA2UL/DTHEv2 to allow access by only
+ * the secure world.


etienne-lms · 2026-01-08T15:08:24Z

core/arch/arm/plat-k3/drivers/ti_crypto.h

+ */
+
+#ifndef TI_CRYPTO_H
+#define TI_CRYPTO_H


Since this is exported to drivers, would be better as:

#ifndef DRIVERS_TI_CRYPTO_H #define DRIVERS_TI_CRYPTO_H

Not a strong opinion, seen the other header files in this directory.

etienne-lms · 2026-01-08T15:09:15Z

core/arch/arm/plat-k3/drivers/ti_crypto.h

+#include <compiler.h>
+#include <stdint.h>
+#include <util.h>
+#include <io.h>
+#include <initcall.h>


Could you sort in alphabetical order?
Actually could be replaced with:

Suggested change

#include <compiler.h>

#include <stdint.h>

#include <util.h>

#include <io.h>

#include <initcall.h>

#include <tee_api_types.h>

#include <stdint.h>

etienne-lms · 2026-01-08T15:14:39Z

core/arch/arm/plat-k3/drivers/dthev2.c

 static TEE_Result dthev2_init(void)
 {
 	TEE_Result result = TEE_SUCCESS;
+	int ret = 0;


Remove ret and use result instead in the lines added below.

jsuhaas22 · 2026-01-09T01:46:32Z

Implemented your comments, @etienne-lms

@glneo and @Pratham-T -- since commits 1 and 3 changed, I have removed your "Rewiewed by: "'s, please review again.

etienne-lms · 2026-01-09T06:19:18Z

core/arch/arm/plat-k3/drivers/sa2ul.c

-		EMSG("Could not set firewall region information");
-		return TEE_ERROR_GENERIC;
+		EMSG("Failed to enable firewalls for TRNG device");
+		return ret;


ret is not a TEE_Result value.

Suggested change

return ret;

return TEE_ERROR_GENERIC;

etienne-lms · 2026-01-09T06:22:13Z

core/arch/arm/plat-k3/drivers/ti_crypto.c

 			return TEE_ERROR_GENERIC;
 		}
 	}
-


I suggest to preserve this empty line.

sa2ul_init() contains code to set firewall for SA2UL RNG. However, almost the same code can also be used to firewall DTHEv2 RNG. Therefore refactor this code into a separate function in the ti_sci driver. Signed-off-by: Suhaas Joshi <s-joshi@ti.com>

The ti_crypto_init_rng_fwl() function gets firewall configurations before setting new ones. This is pointless, since we are not using the configurations that we get anywhere. Therefore remove these blocks of code. Signed-off-by: Suhaas Joshi <s-joshi@ti.com> Reviewed-by: Andrew Davis <afd@ti.com> Reviewed-by: T Pratham <t-pratham@ti.com>

Set firewall to protect DTHEv2 RNG from non-secure world. Signed-off-by: Suhaas Joshi <s-joshi@ti.com>

Pratham-T · 2026-01-09T12:14:30Z

core/arch/arm/plat-k3/drivers/ti_crypto.h

+#include <tee_api_types.h>
+#include <stdint.h>


Since we are already knee deep, make this alphabetical as well.

Pratham-T · 2026-01-09T12:54:52Z

core/arch/arm/plat-k3/drivers/dthev2.c


+	result = ti_crypto_init_rng_fwl(DTHEv2_TI_SCI_FW_ID,
+					DTHEv2_TI_SCI_FW_RGN_ID);
+	if (result) {


Check the result with != TEE_SUCCESS

jsuhaas22 force-pushed the ti-fwl-dthev2-rng branch from ff2a234 to d38db5d Compare December 16, 2025 05:40

shiva-ti reviewed Dec 16, 2025

View reviewed changes

jsuhaas22 force-pushed the ti-fwl-dthev2-rng branch from d38db5d to 3271ba9 Compare December 16, 2025 06:41

Pratham-T suggested changes Dec 16, 2025

View reviewed changes

jsuhaas22 force-pushed the ti-fwl-dthev2-rng branch 4 times, most recently from fd18f46 to cd2cec7 Compare December 19, 2025 06:47

glneo reviewed Dec 19, 2025

View reviewed changes

jsuhaas22 force-pushed the ti-fwl-dthev2-rng branch 2 times, most recently from 063a414 to bb20bba Compare December 23, 2025 05:42

jsuhaas22 force-pushed the ti-fwl-dthev2-rng branch from bb20bba to 1a56e8c Compare December 31, 2025 05:02

Pratham-T approved these changes Jan 7, 2026

View reviewed changes

jsuhaas22 force-pushed the ti-fwl-dthev2-rng branch from 1a56e8c to 144353b Compare January 7, 2026 09:57

etienne-lms reviewed Jan 8, 2026

View reviewed changes

jsuhaas22 force-pushed the ti-fwl-dthev2-rng branch from 144353b to b4b61ba Compare January 9, 2026 01:44

jsuhaas22 force-pushed the ti-fwl-dthev2-rng branch from b4b61ba to 5bf7afb Compare January 9, 2026 01:49

etienne-lms reviewed Jan 9, 2026

View reviewed changes

jsuhaas22 force-pushed the ti-fwl-dthev2-rng branch from 5bf7afb to f7ae0b6 Compare January 9, 2026 08:48

jsuhaas22 added 3 commits January 9, 2026 14:32

plat-k3: drivers: Set firewall for DTHEv2 RNG

a165ae7

Set firewall to protect DTHEv2 RNG from non-secure world. Signed-off-by: Suhaas Joshi <s-joshi@ti.com>

jsuhaas22 force-pushed the ti-fwl-dthev2-rng branch from f7ae0b6 to a165ae7 Compare January 9, 2026 09:03

Pratham-T reviewed Jan 9, 2026

View reviewed changes

		#include "ti_crypto.h"

		TEE_Result ti_crypto_init_rng_fwl(uint16_t fwl_id, uint16_t fwl_region)

TI: Refactor firewall enablement to be generic and enable firewall for DTHEv2 #7653

Are you sure you want to change the base?

TI: Refactor firewall enablement to be generic and enable firewall for DTHEv2 #7653

Conversation

jsuhaas22 commented Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pratham-T left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glneo commented Dec 30, 2025

Uh oh!

jenswi-linaro commented Jan 2, 2026

Uh oh!

Pratham-T left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsuhaas22 commented Jan 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jsuhaas22 commented Dec 16, 2025 •

edited

Loading

Pratham-T left a comment •

edited

Loading