settings: its: enable tf-m PSA headers and run more tests #94881

dsseng · 2025-08-22T19:37:12Z

settings: its: enable tf-m PSA headers
tests: settings: its: run on mps2/an521/cpu0/ns

dsseng · 2025-08-22T19:39:44Z

subsys/settings/src/CMakeLists.txt

+if (CONFIG_SETTINGS_TFM_ITS)
+  # For psa/internal_trusted_storage.h in case of TF-M implementation
+  zephyr_library_link_libraries_ifdef(CONFIG_BUILD_WITH_TFM tfm_api)
+endif()


mbedtls and entropy modules do the same. There's also a more verbose option which I preferred not to use since CMake can handle it all with link_libraries:

if (CONFIG_BUILD_WITH_TFM) zephyr_library_include_directories( $<TARGET_PROPERTY:tfm,TFM_BINARY_DIR>/api_ns/interface/include ) endif()

samples/subsys/settings/sample.yaml

Avoid missing headers on some platforms like mps2/an521/cpu0/ns. Signed-off-by: Dmitrii Sharshakov <[email protected]>

Run on QEMU to make sure this is tested in the CI. Signed-off-by: Dmitrii Sharshakov <[email protected]>

Make sure ITS backend is tested in the CI. Signed-off-by: Dmitrii Sharshakov <[email protected]>

This private header can be used by tests for information on UID ranges that should be cleaned for testing. Signed-off-by: Dmitrii Sharshakov <[email protected]>

Run on mps2/an521/cpu0/ns. Signed-off-by: Dmitrii Sharshakov <[email protected]>

sonarqubecloud · 2025-08-25T09:19:13Z

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

dsseng · 2025-08-25T09:21:24Z

tests/subsys/settings/functional/src/settings_basic_test.c

+	psa_status_t status;
+
+	/* Remove all potentially accessed ITS entries in the UID range */
+	for (int i = 0; i < (sizeof(entries) / CONFIG_TFM_ITS_MAX_ASSET_SIZE) + 1; i++) {
+		status = psa_its_remove(ZEPHYR_PSA_SETTINGS_TFM_ITS_UID_RANGE_BEGIN + i);
+		zassert_true((status == PSA_SUCCESS) || (status == PSA_ERROR_DOES_NOT_EXIST),
+			"psa_its_remove failed");
+	}


For this to actually work and clear the in-memory storage it should either reboot the system or call the load function in a hacky way to reload in runtime. Another option would be to skip this for now as we only run this integration test with ITS on a QEMU target which boots with empty NVS.

addressed

dsseng · 2025-08-25T14:08:27Z

cc @seankyer @tomi-font

tomi-font

Nice work! Some not-so-major comments.

tomi-font · 2025-08-27T12:45:26Z

subsys/settings/src/CMakeLists.txt

@@ -16,3 +16,8 @@ zephyr_sources_ifdef(CONFIG_SETTINGS_SHELL settings_shell.c)
 zephyr_sources_ifdef(CONFIG_SETTINGS_ZMS settings_zms.c)
 zephyr_sources_ifdef(CONFIG_SETTINGS_TFM_ITS settings_its.c)
 zephyr_sources_ifdef(CONFIG_SETTINGS_RETENTION settings_retention.c)
+
+if (CONFIG_SETTINGS_TFM_ITS)


Suggested change

if (CONFIG_SETTINGS_TFM_ITS)

if(CONFIG_SETTINGS_TFM_ITS)

tomi-font · 2025-08-27T12:47:43Z

subsys/settings/src/CMakeLists.txt

@@ -16,3 +16,8 @@ zephyr_sources_ifdef(CONFIG_SETTINGS_SHELL settings_shell.c)
 zephyr_sources_ifdef(CONFIG_SETTINGS_ZMS settings_zms.c)
 zephyr_sources_ifdef(CONFIG_SETTINGS_TFM_ITS settings_its.c)
 zephyr_sources_ifdef(CONFIG_SETTINGS_RETENTION settings_retention.c)
+
+if (CONFIG_SETTINGS_TFM_ITS)
+  # For psa/internal_trusted_storage.h in case of TF-M implementation


Suggested change

# For psa/internal_trusted_storage.h in case of TF-M implementation

# For psa/internal_trusted_storage.h in case of TF-M ITS backend

? or well maybe it's redundant because of the if()

tomi-font · 2025-08-27T12:53:15Z

samples/subsys/settings/sample.yaml

+      - mps2/an521/cpu0/ns
+    extra_args:
+      - CONFIG_SETTINGS_TFM_ITS=y
+      - CONFIG_TFM_ITS_MAX_ASSET_SIZE_OVERRIDE=y


Why are we not also setting CONFIG_TFM_ITS_MAX_ASSET_SIZE here?

tomi-font · 2025-08-27T12:54:45Z

samples/subsys/settings/sample.yaml

+    platform_allow:
+      - mps2/an521/cpu0/ns


How about having a filter instead like the test? (CONFIG_TFM_PARTITION_INTERNAL_TRUSTED_STORAGE)

tomi-font · 2025-08-27T13:26:37Z

tests/subsys/settings/functional/psa/prj.conf

+CONFIG_ZTEST=y
+
+CONFIG_SETTINGS=y
+CONFIG_SETTINGS_RUNTIME=y


tomi-font · 2025-08-27T13:29:27Z

tests/subsys/settings/functional/psa/settings_test_psa.c

@@ -0,0 +1,6 @@
+/* SPDX-License-Identifier: Apache-2.0 */
+/* Copyright (c) 2022 Dmitrii Sharshakov <[email protected]> */


tomi-font · 2025-08-27T13:33:45Z

tests/subsys/settings/functional/psa/testcase.yaml

+    platform_allow:
+      - mps2/an521/cpu0/ns


Same comments than for the sample.

tomi-font · 2025-08-27T13:34:59Z

tests/subsys/settings/functional/src/settings_basic_test.c

+	psa_status_t status;
+
+	/* Remove all potentially accessed ITS entries in the UID range */
+	for (int i = 0; i < (sizeof(entries) / CONFIG_TFM_ITS_MAX_ASSET_SIZE) + 1; i++) {


extraneous parentheses

tomi-font · 2025-08-27T13:35:14Z

tests/subsys/settings/functional/src/settings_basic_test.c

+	/* Remove all potentially accessed ITS entries in the UID range */
+	for (int i = 0; i < (sizeof(entries) / CONFIG_TFM_ITS_MAX_ASSET_SIZE) + 1; i++) {
+		status = psa_its_remove(ZEPHYR_PSA_SETTINGS_TFM_ITS_UID_RANGE_BEGIN + i);
+		zassert_true((status == PSA_SUCCESS) || (status == PSA_ERROR_DOES_NOT_EXIST),


extraneous parentheses

tomi-font · 2025-08-27T13:39:12Z

subsys/settings/src/settings_its_priv.h

+	size_t val_len;
+};
+
+static struct setting_entry entries[CONFIG_SETTINGS_TFM_ITS_NUM_ENTRIES];


Having static struct setting_entry entries in the header file is wrong as we'll end up with multiple definitions of entries for every source file including this header. It's not that big of a problem right now as the test which includes this just uses sizeof(entries), but it's still not great.
Actually to make matters simpler why not just remove this line and move it back to settings_its.c and in the test use sizeof(struct settings_entry) * CONFIG_SETTINGS_TFM_ITS_NUM_ENTRIES?

zephyrbot added the area: Settings Settings subsystem label Aug 22, 2025

dsseng commented Aug 22, 2025

View reviewed changes

henrikbrixandersen added the area: TF-M ARM Trusted Firmware-M (TF-M) label Aug 23, 2025

dsseng mentioned this pull request Aug 23, 2025

settings: ITS backend updates #94819

Merged

zephyrbot added the area: Samples Samples label Aug 23, 2025

zephyrbot requested review from JarmouniA, kartben and nashif August 23, 2025 19:49

zephyrbot assigned kartben Aug 23, 2025

dsseng force-pushed the fix-tfm-its-build branch 2 times, most recently from e34c692 to a9dc16e Compare August 23, 2025 20:58

dsseng changed the title ~~settings: its: enable tf-m PSA headers~~ settings: its: enable tf-m PSA headers and run more tests Aug 24, 2025

dsseng force-pushed the fix-tfm-its-build branch from 6c4c736 to 318ccb2 Compare August 24, 2025 18:03

JarmouniA previously requested changes Aug 24, 2025

View reviewed changes

samples/subsys/settings/sample.yaml Outdated Show resolved Hide resolved

samples/subsys/settings/sample.yaml Show resolved Hide resolved

samples/subsys/settings/sample.yaml Show resolved Hide resolved

dsseng added 5 commits August 25, 2025 10:53

settings: its: enable TF-M PSA headers

409a913

Avoid missing headers on some platforms like mps2/an521/cpu0/ns. Signed-off-by: Dmitrii Sharshakov <[email protected]>

tests: settings: its: run on mps2/an521/cpu0/ns

9c1736e

Run on QEMU to make sure this is tested in the CI. Signed-off-by: Dmitrii Sharshakov <[email protected]>

samples: settings: run integration on QEMU with TF-M

6d6869c

Make sure ITS backend is tested in the CI. Signed-off-by: Dmitrii Sharshakov <[email protected]>

settings: its: move structure and entries to a header

b3969e5

This private header can be used by tests for information on UID ranges that should be cleaned for testing. Signed-off-by: Dmitrii Sharshakov <[email protected]>

tests: settings: its: enable functional tests for TF-M ITS

7754a2a

Run on mps2/an521/cpu0/ns. Signed-off-by: Dmitrii Sharshakov <[email protected]>

dsseng force-pushed the fix-tfm-its-build branch from 318ccb2 to 7754a2a Compare August 25, 2025 08:53

dsseng requested a review from JarmouniA August 25, 2025 08:53

dsseng commented Aug 25, 2025

View reviewed changes

dsseng mentioned this pull request Aug 25, 2025

settings: psa: add support for PS backend #94947

Draft

dsseng mentioned this pull request Aug 27, 2025

Add protected storage (PS) settings backend #94681

Open

tomi-font requested changes Aug 27, 2025

View reviewed changes

	# For psa/internal_trusted_storage.h in case of TF-M implementation
	# For psa/internal_trusted_storage.h in case of TF-M ITS backend

		@@ -0,0 +1,6 @@
		/* SPDX-License-Identifier: Apache-2.0 */
		/* Copyright (c) 2022 Dmitrii Sharshakov <[email protected]> */

settings: its: enable tf-m PSA headers and run more tests #94881

Are you sure you want to change the base?

settings: its: enable tf-m PSA headers and run more tests #94881

Conversation

dsseng commented Aug 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Aug 25, 2025

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsseng commented Aug 25, 2025

Uh oh!

tomi-font 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!