modules: tfm: disable SECURE_UART when TFM_LOG_LEVEL_SILENCE #95582

JordanYates · 2025-09-06T03:11:21Z

Explicitly disable the SECURE_UART TFM define when CONFIG_TFM_LOG_LEVEL_SILENCE=y. The secure UART is only enabled by default on nRF platforms to match the current TF-M defaults.

This results in the default values from the TF-M repo being overridden, e.g:

set(SECURE_UART1                        ON         CACHE BOOL      "Enable secure UART1")

valeriosetti · 2025-09-09T12:58:02Z

modules/trusted-firmware-m/Kconfig.tfm

+	bool "TF-M configure UART instance as secure peripheral"
+	default y if SOC_FAMILY_NORDIC_NRF && !TFM_LOG_LEVEL_SILENCE


Do we really need to make this user selectable? What about making it an internal helper like:

Suggested change

bool "TF-M configure UART instance as secure peripheral"

default y if SOC_FAMILY_NORDIC_NRF && !TFM_LOG_LEVEL_SILENCE

default y if SOC_FAMILY_NORDIC_NRF || !TFM_LOG_LEVEL_SILENCE

I don't see why it shouldn't be user selectable. Maybe the user still wants the secure UART (for FW updates? not sure if possible) but still doesn't want any log output. Or maybe there is some custom log output backend (RTT).

Not sure if it was a typo or not, but changing the default to || is wrong, especially in the context of the other suggestion, since it forces all nRF parts to enable secure UART, even if there is no physical connection on the board.

I don't see why it shouldn't be user selectable. Maybe the user still wants the secure UART (for FW updates? not sure if possible) but still doesn't want any log output. Or maybe there is some custom log output backend (RTT).

Good point.

Not sure if it was a typo or not, but changing the default to || is wrong, especially in the context of the other suggestion, since it forces all nRF parts to enable secure UART, even if there is no physical connection on the board.

It seems to me that all Nordic boards set SECURE_UART1=ON in TF-M. This happens in a file which has a parent directory named common so I assume it's done for all Nordic boards.
I think that the wrong part of my proposal was that for a Nordic board it was basically ignoring !TFM_LOG_LEVEL_SILENCE which is the most relevant condition here, so I agree it was not correct. Sorry for that.

I did a quick check in TF-M on which platform enable SECURE_UART1 and it seems that Nordic are the only ones, so in conclusion the condition you wrote here mimic that part so I'm fine with that.
IMO a nice follow-up would be to remove the logic from TF-M and just reduce the condition above to simply default y if !TFM_LOG_LEVEL_SILENCE, but I guess that this would impact more platforms, so totally out of scope here.

This option should be default=y.
The user should set OFF in their own application.

Nope, that breaks platforms by enabling the define for platforms that don't support it. If the builds are still available you can see this in the CI history.

Nak on this PR.
This should be clarify before disable the serial port for all platform except Nordic.

Just to be clear, your NAK is because of a TF-M default that has been in place for at least 5 years?
zephyrproject-rtos/trusted-firmware-m@9ec67e6
Possibly longer, I don't care enough to search down where the option originally came from.

if I understood correctly, this change will replace the default in the TF-M boards (including all Out-Of-Tree boards).
If someone is developing a product with a board defined by a vendor inside TF-M mainline IMHO it is a big mistake! That is only a reference to make a development board run and make tests. A real TF-M application will have an OOT board with their own configuration and flash layout and this is changing the default on that boards without notice.

Fair enough, added to the migration guide

Maybe a config choice could help, to either enforce the secure UART on or off or leave it to TF-M native default config.

nandojve · 2025-09-11T09:50:55Z

modules/trusted-firmware-m/Kconfig.tfm

+	bool "TF-M configure UART instance as secure peripheral"
+	default y if SOC_FAMILY_NORDIC_NRF && !TFM_LOG_LEVEL_SILENCE


This option should be default=y.
The user should set OFF in their own application.

Explicitly disable the SECURE_UART TFM define when `CONFIG_TFM_LOG_LEVEL_SILENCE=y`. The secure UART is only enabled by default on nRF platforms to match the current TF-M defaults. Signed-off-by: Jordan Yates <[email protected]>

sonarqubecloud · 2025-09-11T11:25:05Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

nandojve · 2025-09-12T08:53:25Z

CC: @erwango , @etienne-lms

tomi-font · 2025-09-17T13:10:19Z

doc/releases/migration-guide-4.3.rst

+* The ``SECURE_UART1`` TF-M define is now controlled by the Zephyr buildsystem
+  :kconfig:option:`CONFIG_TFM_SECURE_UART`. This option will override any platform values previously


Suggested change

* The ``SECURE_UART1`` TF-M define is now controlled by the Zephyr buildsystem

:kconfig:option:`CONFIG_TFM_SECURE_UART`. This option will override any platform values previously

* The ``SECURE_UART1`` TF-M define is now controlled by the

:kconfig:option:`CONFIG_TFM_SECURE_UART` Kconfig option. This option will override any platform values previously

I was explicit about the "Zephyr build system" comment because TF-M also uses Kconfig internally (even though Zephyr doesn't use it) and I wanted to be clear about where the option sits.

Then maybe rather the following?

Suggested change

* The ``SECURE_UART1`` TF-M define is now controlled by the Zephyr buildsystem

:kconfig:option:`CONFIG_TFM_SECURE_UART`. This option will override any platform values previously

* The ``SECURE_UART1`` TF-M define is now controlled by Zephyr's

:kconfig:option:`CONFIG_TFM_SECURE_UART`. This option will override any platform values previously

modules/trusted-firmware-m/CMakeLists.txt

nandojve · 2025-09-18T07:37:52Z

I'll add DNM to ST have time to look on this.

JordanYates · 2025-09-18T08:17:18Z

I'll add DNM to ST have time to look on this.

Why ST specifically? It doesn't look like they use the define in any of their upstream boards?

JordanYates · 2025-09-18T08:19:39Z

Also, adding the DNM label makes the PR show up as failing CI, which is generally a pretty good way to make people not review it. If you want someone specifically to review it, please tag them, since I don't see anyone that I know is from ST on the review list anyway.

etienne-lms

Sorry for this late reply.
I'm fine with the idea of the change. Having more control on TF-M configuration from Zephyr configs is better IMHO.
@nandojve, you can remove the DNM label, at least from my perspective.

etienne-lms · 2025-09-18T12:26:17Z

modules/trusted-firmware-m/Kconfig.tfm

+	bool "TF-M configure UART instance as secure peripheral"
+	default y if SOC_FAMILY_NORDIC_NRF && !TFM_LOG_LEVEL_SILENCE


Maybe a config choice could help, to either enforce the secure UART on or off or leave it to TF-M native default config.

Vge0rge

It looks OK to me from Nordic's perspective as it does not change the default at the moment.

zephyrbot added the area: TF-M ARM Trusted Firmware-M (TF-M) label Sep 6, 2025

zephyrbot requested review from d3zd3z, ithinuel, tomi-font, valeriosetti, Vge0rge and wearyzen September 6, 2025 03:12

zephyrbot assigned d3zd3z Sep 6, 2025

JordanYates force-pushed the 250906_tfm_secure_uart_disable branch from 176e29d to 158d1e7 Compare September 6, 2025 06:14

valeriosetti requested changes Sep 9, 2025

View reviewed changes

valeriosetti self-requested a review September 11, 2025 09:31

valeriosetti previously approved these changes Sep 11, 2025

View reviewed changes

nandojve requested changes Sep 11, 2025

View reviewed changes

JordanYates dismissed valeriosetti’s stale review via c2c382a September 11, 2025 11:21

JordanYates force-pushed the 250906_tfm_secure_uart_disable branch from 158d1e7 to c2c382a Compare September 11, 2025 11:21

zephyrbot added the Release Notes To be mentioned in the release notes label Sep 11, 2025

zephyrbot requested review from cfriedt, jhedberg and kartben September 11, 2025 11:22

JordanYates requested review from nandojve and valeriosetti September 11, 2025 23:26

valeriosetti approved these changes Sep 12, 2025

View reviewed changes

tomi-font reviewed Sep 17, 2025

View reviewed changes

nandojve added the DNM This PR should not be merged (Do Not Merge) label Sep 18, 2025

etienne-lms reviewed Sep 18, 2025

View reviewed changes

JordanYates removed the DNM This PR should not be merged (Do Not Merge) label Sep 18, 2025

Vge0rge approved these changes Sep 22, 2025

View reviewed changes

		bool "TF-M configure UART instance as secure peripheral"
		default y if SOC_FAMILY_NORDIC_NRF && !TFM_LOG_LEVEL_SILENCE

	bool "TF-M configure UART instance as secure peripheral"
	default y if SOC_FAMILY_NORDIC_NRF && !TFM_LOG_LEVEL_SILENCE
	default y if SOC_FAMILY_NORDIC_NRF \|\| !TFM_LOG_LEVEL_SILENCE

		* The ``SECURE_UART1`` TF-M define is now controlled by the Zephyr buildsystem
		:kconfig:option:`CONFIG_TFM_SECURE_UART`. This option will override any platform values previously

modules: tfm: disable SECURE_UART when TFM_LOG_LEVEL_SILENCE #95582

Are you sure you want to change the base?

modules: tfm: disable SECURE_UART when TFM_LOG_LEVEL_SILENCE #95582

Conversation

JordanYates commented Sep 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

sonarqubecloud bot commented Sep 11, 2025

Quality Gate passed

Uh oh!

nandojve commented Sep 12, 2025

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!

Uh oh!

nandojve commented Sep 18, 2025

Uh oh!

JordanYates commented Sep 18, 2025

Uh oh!

JordanYates commented Sep 18, 2025

Uh oh!

etienne-lms left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Vge0rge left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JordanYates commented Sep 6, 2025 •

edited

Loading