scripts: Add dispatch include path and new generation location #307

Open
asmellby wants to merge 5 commits from asmellby/feature/dispatch-library into main
asmellby commented 2026-06-11 06:41:49 -05:00 (Migrated from github.com)

Description

PSA crypto driver dispatch files are generated into dispatch/ instead of core/ in newer TF-PSA-Crypto versions. API headers that rely on driver and dispatch implementation details are also moved from include/ to dispatch/include/.

Update the framework scripts to work with both the old and new layouts.

PR checklist

  • TF-PSA-Crypto development PR provided Mbed-TLS/TF-PSA-Crypto#809
  • TF-PSA-Crypto 1.1 PR not required because: not a bugfix, repository structure change is not entirely backwards compatible
  • mbedtls development PR provided Mbed-TLS/mbedtls#10780
  • mbedtls 4.1 PR not required because: does not affect mbedtls
  • mbedtls 3.6 PR not required because: does not affect mbedtls
## Description PSA crypto driver dispatch files are generated into `dispatch/` instead of `core/` in newer TF-PSA-Crypto versions. API headers that rely on driver and dispatch implementation details are also moved from `include/` to `dispatch/include/`. Update the framework scripts to work with both the old and new layouts. ## PR checklist - [x] **TF-PSA-Crypto development PR** provided Mbed-TLS/TF-PSA-Crypto#809 - [x] **TF-PSA-Crypto 1.1 PR** not required because: not a bugfix, repository structure change is not entirely backwards compatible - [x] **mbedtls development PR** provided Mbed-TLS/mbedtls#10780 - [x] **mbedtls 4.1 PR** not required because: does not affect mbedtls - [x] **mbedtls 3.6 PR** not required because: does not affect mbedtls
ronald-cron-arm (Migrated from github.com) reviewed 2026-06-11 06:41:49 -05:00
gilles-peskine-arm commented 2026-06-11 06:45:12 -05:00 (Migrated from github.com)

(Priority note: this fits under our “PSA crypto driver standardization” work, for which we have allocated a little bandwidth this quarter. Thanks Aksel for contributing it!)

(Priority note: this fits under our “PSA crypto driver standardization” work, for which we have allocated a little bandwidth this quarter. Thanks Aksel for contributing it!)
gilles-peskine-arm (Migrated from github.com) requested changes 2026-06-11 12:06:55 -05:00
gilles-peskine-arm (Migrated from github.com) left a comment

I don't think tf_psa_crypto_version() is necessary. But if it is, it needs to pass the static checks. Otherwise looks good to me.

I don't think `tf_psa_crypto_version()` is necessary. But if it is, it needs to pass the static checks. Otherwise looks good to me.
@ -22,11 +22,15 @@ COMMON_GENERATION_SCRIPTS = [
]
gilles-peskine-arm (Migrated from github.com) commented 2026-06-11 10:36:32 -05:00

What's the point of checking the version? I'd prefer to avoid relying on any version number.

What's the point of checking the version? I'd prefer to avoid relying on any version number.
@ -171,6 +171,7 @@ def main():
else:
parser.add_argument('--include', '-I',
action='append', default=['tf-psa-crypto/include',
'tf-psa-crypto/dispatch/include',
gilles-peskine-arm (Migrated from github.com) commented 2026-06-11 11:44:13 -05:00

There's still a reference to core/psa_crypto_driver_wrappers.h in scripts/check_names.py, and even a reference to library/psa_crypto_driver_wrappers.h which only exists in the 3.6 branch. It was introduced in 634e0d25cb and I think it's now obsolete (especially since check_names.py passes just fine with the generated files in dispatch), but I'm not completely sure. If it's obsolete, we might as well remove it now, although it would be strictly speaking out of scope.

There's still a reference to `core/psa_crypto_driver_wrappers.h` in `scripts/check_names.py`, and even a reference to `library/psa_crypto_driver_wrappers.h` which only exists in the 3.6 branch. It was introduced in 634e0d25cb and I think it's now obsolete (especially since `check_names.py` passes just fine with the generated files in `dispatch`), but I'm not completely sure. If it's obsolete, we might as well remove it now, although it would be strictly speaking out of scope.
asmellby (Migrated from github.com) reviewed 2026-06-12 04:42:51 -05:00
@ -22,11 +22,15 @@ COMMON_GENERATION_SCRIPTS = [
]
asmellby (Migrated from github.com) commented 2026-06-12 04:42:51 -05:00

I did it to limit when the new layout was considered to releases that are "supposed to" have the new layout. But unconditionally allowing the new layout does mean we avoid the version check. Updated to remove the version check.

I did it to limit when the new layout was considered to releases that are "supposed to" have the new layout. But unconditionally allowing the new layout does mean we avoid the version check. Updated to remove the version check.
asmellby (Migrated from github.com) reviewed 2026-06-12 04:46:29 -05:00
@ -171,6 +171,7 @@ def main():
else:
parser.add_argument('--include', '-I',
action='append', default=['tf-psa-crypto/include',
'tf-psa-crypto/dispatch/include',
asmellby (Migrated from github.com) commented 2026-06-12 04:46:29 -05:00

Good catch. Updated to include the new dispatch include path (which was missing) and generalizing the excluded autogen files. I agree that it looks like the script passes even if psa_crypto_driver_wrappers.h isn't excluded, so if you want me to, I can remove the exclusion instead. I don't know if the lack of "bad" symbols in the generated file is a long-term commitment or just how things are right now.

Good catch. Updated to include the new dispatch include path (which was missing) and generalizing the excluded autogen files. I agree that it looks like the script passes even if `psa_crypto_driver_wrappers.h` isn't excluded, so if you want me to, I can remove the exclusion instead. I don't know if the lack of "bad" symbols in the generated file is a long-term commitment or just how things are right now.
gilles-peskine-arm (Migrated from github.com) reviewed 2026-06-12 09:35:13 -05:00
gilles-peskine-arm (Migrated from github.com) left a comment

LGTM, but let's not add to the complexity in check_names, please remove the psa_crypto_driver_wrappers.h exclusion.

LGTM, but let's not add to the complexity in `check_names`, please remove the `psa_crypto_driver_wrappers.h` exclusion.
@ -171,6 +171,7 @@ def main():
else:
parser.add_argument('--include', '-I',
action='append', default=['tf-psa-crypto/include',
'tf-psa-crypto/dispatch/include',
gilles-peskine-arm (Migrated from github.com) commented 2026-06-12 09:34:24 -05:00

The problem isn't with symbols that violate our naming conventions, but about symbols that look like a typo because check_names finds their use but not their definition or vice versa.

Since 634e0d25cb, we've both fixed a few bugs in check_names.py and changed the structure of the driver wrapper code. Today check_names.py passes on 3.6 if I remove all references to psa_crypto_driver_wrappers.h (all of them in exclude lists) (69fa1da16d). So please go ahead and do that.

The problem isn't with symbols that violate our naming conventions, but about symbols that look like a typo because `check_names` finds their use but not their definition or vice versa. Since 634e0d25cb9e75c54c72391be28da4c2f7a1def0, we've both fixed a few bugs in `check_names.py` and changed the structure of the driver wrapper code. Today `check_names.py` passes on 3.6 if I remove all references to `psa_crypto_driver_wrappers.h` (all of them in exclude lists) (69fa1da16dede0eeab60451a08549546bfbe32f9). So please go ahead and do that.
asmellby (Migrated from github.com) reviewed 2026-06-15 02:45:45 -05:00
@ -171,6 +171,7 @@ def main():
else:
parser.add_argument('--include', '-I',
action='append', default=['tf-psa-crypto/include',
'tf-psa-crypto/dispatch/include',
asmellby (Migrated from github.com) commented 2026-06-15 02:45:44 -05:00

Done.

Done.
gilles-peskine-arm (Migrated from github.com) approved these changes 2026-07-08 12:12:41 -05:00
gilles-peskine-arm (Migrated from github.com) left a comment

LGTM at 957b8a1b74

LGTM at 957b8a1b7492054a18eee5f20e3f7f626be06503
This pull request is broken due to missing fork information.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin 'asmellby/feature/dispatch-library:asmellby/feature/dispatch-library'
git switch 'asmellby/feature/dispatch-library'

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch 'main'
git merge --no-ff 'asmellby/feature/dispatch-library'
git switch 'asmellby/feature/dispatch-library'
git rebase 'main'
git switch 'main'
git merge --ff-only 'asmellby/feature/dispatch-library'
git switch 'asmellby/feature/dispatch-library'
git rebase 'main'
git switch 'main'
git merge --no-ff 'asmellby/feature/dispatch-library'
git switch 'main'
git merge --squash 'asmellby/feature/dispatch-library'
git switch 'main'
git merge --ff-only 'asmellby/feature/dispatch-library'
git switch 'main'
git merge 'asmellby/feature/dispatch-library'
git push origin 'main'
Sign in to join this conversation.
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-Mbed-TLS/mbedtls-framework!307
No description provided.