Complete the move of driver wrappers to the dispatch/ directory by asmellby · Pull Request #809 · Mbed-TLS/TF-PSA-Crypto

asmellby · 2026-06-11T11:48:06Z

Description

The TF-PSA-Crypto 1.1 release introduced a dispatch directory with the following description:

A new directory dispatch has been added at the root of the repository
to eventually host all code that dispatches cryptographic operations to
drivers, such as psa_crypto_driver_wrappers_no_static.c. For the time
being, it only contains psa_crypto_driver_wrappers_no_static.h.

This PR completes the move of generated driver wrapper files and API headers to the dispatch directory.

Resolves #833

PR checklist

Please remove the segment/s on either side of the | symbol as appropriate, and add any relevant link/s to the end of the line.
If the provided content is part of the present PR remove the # symbol.

changelog provided
framework PR provided scripts: Add dispatch include path and new generation location mbedtls-framework#307
TF-PSA-Crypto development PR provided (this PR)
TF-PSA-Crypto 1.1 PR not required because: not a bugfix, repository structure change is not entirely backwards compatible
mbedtls development PR provided Complete the move of TF-PSA-Crypto driver wrappers to dispatch/ directory mbedtls#10780
mbedtls 4.1 PR not required because: does not affect mbedtls
mbedtls 3.6 PR not required because: does not affect mbedtls
tests provided

gilles-peskine-arm

I think this is moving in the direction we want, thanks. But I've asked Ronald to review as well, as he's the architect of the directory structure and build scripts. We're currently focusing on some security fixes for our next release, so it might be a while before he can review.

I'm not sure what users are supposed to be able to do. Whatever it is, it needs to be documented and tested.

I reviewed 56449b8

gilles-peskine-arm · 2026-06-11T16:08:25Z

 set(TF_PSA_CRYPTO_FRAMEWORK_DIR ${CMAKE_CURRENT_SOURCE_DIR}/framework)

+if(NOT DEFINED TF_PSA_CRYPTO_DISPATCH_DIR)
+    set(TF_PSA_CRYPTO_DISPATCH_DIR ${CMAKE_CURRENT_SOURCE_DIR}/dispatch)


From prior experience, I think CMakeLists.txt in Mbed TLS will need to know about the new location to populate TF_PSA_CRYPTO_PRIVATE_INCLUDE_DIRS. It isn't properly extracting the information from TF-PSA-Crypto.

Opened Mbed-TLS/mbedtls#10780

Allow supplying a custom dispatch directory using the TF_PSA_CRYPTO_DISPATCH_DIR variable. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

Use the dispatch directory for all driver wrapper files. When file generation is enabled, generate into a dispatch/ build directory. Move the generation logic into the dispatch CMakeLists.txt. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

The dispatch/include/ directory is added as a public include directory to contain dispatch and driver-specific include files. It is currently empty. In the installed CMake package, the new directory is collapsed into the existing structure to maintain backward compatibility. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

The crypto driver context headers are tightly coupled with the dispatch implementation. Move them to the dispatch include directory. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

Let the generated files directly be part of the dispatch target instead of using an intermediate custom target. Directly link drivers with the dispatch target, using the correct flavor for static and shared libraries. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

Set the dispatch build directory explicitly to support out-of-tree dispatch directories. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

Add a testcase for the use of TF_PSA_CRYPTO_DISPATCH_DIR to set a custom dispatch directory. The custom dispatch in the testcase is created by copying and generating the vanilla dispatch implementation into the test program directory. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

Add a testcase for the use of TF_PSA_CRYPTO_DISPATCH_DIR to set a custom dispatch directory when building and installing a CMake package for TF-PSA-Crypto. The custom dispatch in the testcase is created by copying and generating the vanilla dispatch implementation into the test program directory. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

Document the new include path and TF_PSA_CRYPTO_DISPATCH_DIR. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

Simplify check for the new dispatch location and add new location to check_names.py Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

gilles-peskine-arm

Looks good to me, except I'd like one point clarified in the documentation. I don't know much about CMake, so my review of CMake code is superficial.

Our CI appears to be stuck today (ticket filed), so results might not be available until Monday.

gilles-peskine-arm · 2026-06-15T08:58:00Z

Note that the CI isn't happy. There's at least a missing include directory in CMake builds.

Remove exclusion of driver wrappers from check_names.py Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

Make it explicit that this option should be an absolute path. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

The dispatch include dir was missing from the doxyfile and from the config check test. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

Some test cases (tf_psa_crypto_test_memsan_constant_flow_psa) rely on reusing a generated version of the driver wrapper from a previous CMake invocation. If the driver wrapper doesn't exist in the source directory but does exist in the build directory, use the file from the build directory. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

The test driver include paths are included in core and drivers. Also include them in other object libraries. This is needed to support the move of driver wrappers from core to dispatch. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

valeriosetti · 2026-07-01T12:41:46Z

I manually kicked the CI because it didn't automatically start after the last push

ronald-cron-arm · 2026-07-08T08:10:48Z

@asmellby Sorry for the delay.

I've started looking at this PR. It seems to me that it addresses two separate things:

Completing the move of the generated driver wrapper files and API headers to the dispatch directory.
Adding support for selecting a dispatch directory different from the repository's dispatch directory.

I would prefer to handle (1) first and (2) in a follow-up PR. Would you be able to split the changes accordingly?

asmellby · 2026-07-08T08:44:29Z

I would prefer to handle (1) first and (2) in a follow-up PR. Would you be able to split the changes accordingly?

I could do that, but that will require a rebase, which the contribution guidelines tell me not to do without approval from reviewers. So I'll need some guidance: To what degree should I clean up the commits when I rebase? Am I allowed to merge fixup commits into the initial commits to create a more reviewable changeset, or should I keep them separate to preserve the (now no longer entirely real) history?

gilles-peskine-arm · 2026-07-08T08:52:18Z

I would prefer to keep (1) and (2) in the same pull request, because (2) checks that (1) has been done right.

But if (1) and (2) are separate, please provide histories that help me review incrementally:

A history with the same commit structure, so I can review with git range-diff development 1c6cde11420cf17340a385dfb43b9453dc211c44 head1.
A restructured history with a step step1 in one PR, and a step step2 such that head1 and step2 have the same content.

(Where rebases get really bad is if there's both a change of base and a change of commit structure.)

gilles-peskine-arm · 2026-07-08T08:54:01Z

By the way, for reviewing, the merge conflict on the framework submodule is not a concern. However, I haven't made this review a priority due to the CI failure.

Note, I will be offline from tomorrow to 19 July.

asmellby · 2026-07-08T08:57:00Z

By the way, for reviewing, the merge conflict on the framework submodule is not a concern. However, I haven't made this review a priority due to the CI failure.

CI passes for the TF-PSA-Crypto tests as far as I can tell. The CI failure is on the Mbed TLS tests, which needs the corresponding change in Mbed-TLS/mbedtls#10780 to pass. How can I tell the TF-PSA-Crypto CI to use that Mbed TLS PR?

ronald-cron-arm · 2026-07-08T09:00:55Z

By the way, for reviewing, the merge conflict on the framework submodule is not a concern. However, I haven't made this review a priority due to the CI failure.

CI passes for the TF-PSA-Crypto tests as far as I can tell. The CI failure is on the Mbed TLS tests, which needs the corresponding change in Mbed-TLS/mbedtls#10780 to pass. How can I tell the TF-PSA-Crypto CI to use that Mbed TLS PR?

You cannot. We will need to merge the Mbed TLS PR first.

ronald-cron-arm · 2026-07-08T09:04:50Z

I would prefer to keep (1) and (2) in the same pull request, because (2) checks that (1) has been done right.

@gilles-peskine-arm I am not sure what do you mean by that. Could you elaborate?

gilles-peskine-arm · 2026-07-08T09:10:16Z

If we've moved too few files or too many files, the test with an alternative dispatch directory has a good change of telling us. Without this test, nothing will tell us.

ronald-cron-arm · 2026-07-08T15:06:49Z

If we've moved too few files or too many files, the test with an alternative dispatch directory has a good change of telling us. Without this test, nothing will tell us.

The tests with an alternative dispatch directory mostly reproduce the content of the dispatch directory into a custom directory, so I am not sure how they would help check whether we have moved too few or too many files into that directory.

More generally, I may have missed some discussion, but I am not sure what we are trying to achieve with the TF_PSA_CRYPTO_DISPATCH_DIR option. What are the intended use cases for this option?

gilles-peskine-arm · 2026-07-08T17:01:23Z

The tests with an alternative dispatch directory mostly reproduce the content of the dispatch directory into a custom directory, so I am not sure how they would help check whether we have moved too few or too many files into that directory.

Ah, yes, they wouldn't really help with that, my bad.

More generally, I may have missed some discussion, but I am not sure what we are trying to achieve with the TF_PSA_CRYPTO_DISPATCH_DIR option. What are the intended use cases for this option?

Ah, my bad, I've mentioned it before but I hadn't put it in writing. I've filed #833.

gilles-peskine-arm

At 7655efb, LGTM except that I don't understand 2de8d4e.

gilles-peskine-arm · 2026-07-08T17:06:13Z

-    )
+    if(EXISTS "${CMAKE_CURRENT_BINARY_DIR}/psa_crypto_driver_wrappers_no_static.c" AND NOT
+       EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/psa_crypto_driver_wrappers_no_static.c")
+        # Some test cases (tf_psa_crypto_test_memsan_constant_flow_psa) rely on


Not “test cases” (this would be a stanza in tests/suites/*.data, but “test components”.

But it's weird to justify a feature by a test component. I don't quite understand why this is needed.

Agree that it's weird. Added a comment below about why it's needed, but that it could be made less explicit by relying on implicit CMake search paths for relative paths, which was what the core did before. That would avoid explicitly calling out the test component -- the build system behavior would be exactly the same, though.

gilles-peskine-arm · 2026-07-08T17:07:04Z

+    if(EXISTS "${CMAKE_CURRENT_BINARY_DIR}/psa_crypto_driver_wrappers_no_static.c" AND NOT
+       EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/psa_crypto_driver_wrappers_no_static.c")


This should be an elseif.

gilles-peskine-arm · 2026-07-08T17:09:35Z

+    if(EXISTS "${CMAKE_CURRENT_BINARY_DIR}/psa_crypto_driver_wrappers_no_static.c" AND NOT
+       EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/psa_crypto_driver_wrappers_no_static.c")


I don't understand why this condition is needed now. Is it fixing a preexisting bug?

We run our CI with GEN_FILES=ON. It's supposed to work with GEN_FILES=OFF, but that's not something we test.

The test component tf_psa_crypto_test_memsan_constant_flow_psa runs CMake twice -- first with GEN_FILES=ON to generate the driver wrapper. Then, it runs again with GEN_FILES=OFF and CMAKE_BUILD_TYPE=MemSan, relying on the previous invocation's copy of the generated driver wrapper to still exist and be picked up by the build system.

Introduced in 93a8114

This creates a weird scenario where the build system on that second pass needs to be able to pick up the file from the generation directory, despite GEN_FILES=OFF. The previous implementation in core handled this by using a relative path, which makes CMake check both the source and build directory: https://github.com/Mbed-TLS/TF-PSA-Crypto/blob/development/core/CMakeLists.txt#L3-L14
I can simplify this to match the way it was done in core before, they should be equivalent if my reading of https://cmake.org/cmake/help/v3.31/prop_tgt/SOURCES.html is correct -- what I did is just more explicit about the intention.

#135 (comment) contains some discussion of why it's done this way for that test component.

Thanks for the explanation.

I'm ok with this code if you improve the comments explaining why, mainly that it's about GEN_FILES=ON followed by GEN_FILES=OFF. I don't understand why we did that in 93a8114, as opposed to e.g. framework/scripts/make_generated_files.py or just directly running the build without touching GEN_FILES.

But I'm a CMake novice. Please wait for someone who is more experienced with CMake (Ronald) to give their opinion.

asmellby mentioned this pull request Jun 11, 2026

scripts: Add dispatch include path and new generation location Mbed-TLS/mbedtls-framework#307

Open

5 tasks

asmellby marked this pull request as ready for review June 11, 2026 11:58

gilles-peskine-arm added size-s Estimated task size: small (~2d) needs-review Every commit must be reviewed by at least two team members needs-reviewer This PR needs someone to pick it up for review priority-high High priority - will be reviewed soon labels Jun 11, 2026

gilles-peskine-arm added this to Roadmap pull requests (new board) Jun 11, 2026

github-project-automation Bot moved this to In Development in Roadmap pull requests (new board) Jun 11, 2026

gilles-peskine-arm requested review from gilles-peskine-arm and ronald-cron-arm June 11, 2026 15:28

gilles-peskine-arm removed the needs-reviewer This PR needs someone to pick it up for review label Jun 11, 2026

gilles-peskine-arm requested changes Jun 11, 2026

View reviewed changes

gilles-peskine-arm added needs-work and removed needs-review Every commit must be reviewed by at least two team members labels Jun 11, 2026

gilles-peskine-arm mentioned this pull request Jun 11, 2026

[zep noup] Move driver wrappers to dispatch directory zephyrproject-rtos/TF-PSA-Crypto#11

Open

asmellby added 5 commits June 12, 2026 10:55

Add CMake variable for dispatch directory

f377875

Allow supplying a custom dispatch directory using the TF_PSA_CRYPTO_DISPATCH_DIR variable. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

Move crypto_driver_contexts to dispatch include directory

1a6c3b2

The crypto driver context headers are tightly coupled with the dispatch implementation. Move them to the dispatch include directory. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

Update framework scripts to support the dispatch separation

e22eb7b

Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

asmellby force-pushed the feature/dispatch-library branch from 56449b8 to e22eb7b Compare June 12, 2026 09:52

asmellby added 6 commits June 12, 2026 13:24

Explicitly set dispatch build directory

6cf05a9

Set the dispatch build directory explicitly to support out-of-tree dispatch directories. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

Add changelog entry and documentation for the dispatch changes

0c7a4ed

Document the new include path and TF_PSA_CRYPTO_DISPATCH_DIR. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

Update framework scripts

1c6cde1

Simplify check for the new dispatch location and add new location to check_names.py Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

gilles-peskine-arm requested changes Jun 12, 2026

View reviewed changes

Comment thread CMakeLists.txt Outdated

gilles-peskine-arm added the needs-ci Needs to pass CI tests label Jun 12, 2026

asmellby added 4 commits June 18, 2026 15:01

Update framework scripts

0d3a7ee

Remove exclusion of driver wrappers from check_names.py Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

Clarify documentation for TF_PSA_CRYPTO_DISPATCH_DIR

8b7c1d9

Make it explicit that this option should be an absolute path. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

Add dispatch include dir to API doc and config check

8ca5cb0

The dispatch include dir was missing from the doxyfile and from the config check test. Signed-off-by: Aksel Skauge Mellbye <aksel.mellbye@silabs.com>

asmellby mentioned this pull request Jun 18, 2026

Complete the move of TF-PSA-Crypto driver wrappers to dispatch/ directory Mbed-TLS/mbedtls#10780

Open

8 tasks

gilles-peskine-arm requested changes Jul 8, 2026

View reviewed changes

		if(EXISTS "${CMAKE_CURRENT_BINARY_DIR}/psa_crypto_driver_wrappers_no_static.c" AND NOT
		EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/psa_crypto_driver_wrappers_no_static.c")

Uh oh!

Conversation

asmellby commented Jun 11, 2026 • edited by gilles-peskine-arm Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

PR checklist

Uh oh!

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gilles-peskine-arm commented Jun 15, 2026

Uh oh!

valeriosetti commented Jul 1, 2026

Uh oh!

ronald-cron-arm commented Jul 8, 2026

Uh oh!

asmellby commented Jul 8, 2026

Uh oh!

gilles-peskine-arm commented Jul 8, 2026

Uh oh!

gilles-peskine-arm commented Jul 8, 2026

Uh oh!

asmellby commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ronald-cron-arm commented Jul 8, 2026

Uh oh!

ronald-cron-arm commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gilles-peskine-arm commented Jul 8, 2026

Uh oh!

ronald-cron-arm commented Jul 8, 2026

Uh oh!

gilles-peskine-arm commented Jul 8, 2026

Uh oh!

gilles-peskine-arm 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!

asmellby Jul 8, 2026 • 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

asmellby commented Jun 11, 2026 •

edited by gilles-peskine-arm

Loading

asmellby commented Jul 8, 2026 •

edited

Loading

ronald-cron-arm commented Jul 8, 2026 •

edited

Loading

asmellby Jul 8, 2026 •

edited

Loading