integrate tf-psa-crypto 1.x#1
integrate tf-psa-crypto 1.x#1valeriosetti wants to merge 4 commits intozephyrproject-rtos:zephyr_tf-psa-crypto_1.0.99_e5fdb3fe2from
Conversation
valeriosetti
force-pushed
the
zephyr-tfpsacrypto-1.0
branch
from
aab7015 to
d0df3e1
Compare
valeriosetti
force-pushed
the
zephyr-tfpsacrypto-1.0
branch
from
d0df3e1 to
70ba97c
Compare
valeriosetti
changed the base branch from
zephyr_tf-psa-crypto_1.0.99_088771225
to
zephyr_tf-psa-crypto_1.0.99_e5fdb3fe2
valeriosetti
force-pushed
the
zephyr-tfpsacrypto-1.0
branch
2 times, most recently
from
a3ca844 to
b8a61f0
Compare
tejlmand
left a comment
tejlmand
left a comment
There was a problem hiding this comment.
Please read the module guidelines: https://docs.zephyrproject.org/latest/develop/modules.html#contributing-changes-to-modules
Especially the section on module contribution.
There was a problem hiding this comment.
[zep noup] is not an established syntax in Zephyr.
There was a problem hiding this comment.
It is in the Mbed TLS/TF-M repos: https://github.com/zephyrproject-rtos/mbedtls/blob/zephyr_mbedtls_v3.6.5/README.md
There was a problem hiding this comment.
Yes, being a submodule of Mbed TLS I followed the same rules specified in that README file.
| # set(version_number_files | ||
| # doxygen/input/doc_mainpage.h | ||
| # doxygen/tfpsacrypto.doxyfile) | ||
| # foreach(file ${version_number_files}) | ||
| # configure_file(${file}.in | ||
| # ${PROJECT_SOURCE_DIR}/${file}) | ||
| # endforeach(file) |
There was a problem hiding this comment.
please read: https://docs.zephyrproject.org/latest/develop/modules.html#contributing-changes-to-modules
Submitting and merging changes directly to a module’s codebase, that is, before they have been merged in the corresponding external project repository, should be limited to:
- changes required due to updates in the zephyr main tree
- urgent changes that should not wait to be merged in the external project first, such as fixes to security vulnerabilities.
I'm not sure those changes are acceptable for a Zephyr patch.
If this is a problem, I suggest opening a PR upstream trying to make TF-PSA crypto more modular.
There was a problem hiding this comment.
I tried to ping Mbed TLS developers on this, but I haven't received any answer so far. I will try to ping them again and if needed I will open a PR upstream to handle this gating this block of code with GEN_FILES.
| @@ -0,0 +1,2922 @@ | |||
| /* | |||
There was a problem hiding this comment.
TF-PSA-Crypto repo by default automatically generates some files. In order
to achieve this Python scripts are being used and these require "framework"
submodule to be present.
sounds like we do need framework.
Please remove this commit 87b2208 from the PR then.
See also: https://docs.zephyrproject.org/latest/develop/modules.html#contributing-changes-to-modules
Non-trivial changes to a module’s codebase, including changes in the module design or functionality should be discouraged, if the module has an upstream project repository. In that case, such changes shall be submitted to the upstream project, directly.
There was a problem hiding this comment.
framework module was removed this morning following a request from @tomi-font. His idea is that unless this submodule is absolutely required for Zephyr it can be skipped and save disk space.
framework is only required when generating code at build time, but when this project is included from zephyr GEN_FILES=OFF and indeed the CI of zephyrproject-rtos/zephyr#104031 is not complaining for this change.
mldsa-native is being checked out by Zephyr's west tool. Framework instead contains test data files and python scripts. Since we don't use Mbed TLS' testing system and we don't generate files at build time, the framework repo is completely removed. "CMakeLists.txt" is updated to forcedly ignore this change. Signed-off-by: Valerio Setti <vsetti@baylibre.com>
This causes issues on the BabbleSim CI in Zephyr. Signed-off-by: Valerio Setti <vsetti@baylibre.com>
TF-PSA-Crypto repo by default automatically generates some files. In order to achieve this Python scripts are being used and these require "framework" submodule to be present. Since we might need to patch some of these files and also save some disk space by not having "framework" repo checked out, we check out these auto-generated files. Signed-off-by: Valerio Setti <vsetti@baylibre.com>
These modules were removed from TF-PSA-Crypto, but they are still used in some Zephyr module so this commit brings them back. Problematic modules are: - DES, DHM --> HostAP - ECDH --> ESP32 WiFi and BT drivers Here's the commits where they were taken from in TF-PSA-Crypto upstream repo: - DES: cb16940 - DHM: 5b636e8 - ECDH: 882a2cd Signed-off-by: Valerio Setti <vsetti@baylibre.com>
valeriosetti
force-pushed
the
zephyr-tfpsacrypto-1.0
branch
from
b8a61f0 to
0a428b2
Compare
3 participants
NOTE do not merge this for now!
This is being tested from zephyrproject-rtos/zephyr#104031