Cmake custom compile-time config support - #10860
Conversation
Signed-off-by: Yi Wu <yi.wu2@arm.com>
Signed-off-by: Yi Wu <yi.wu2@arm.com>
Signed-off-by: Yi Wu <yi.wu2@arm.com>
minosgalanakis
left a comment
There was a problem hiding this comment.
Good starting point but some minor comments to address
gilles-peskine-arm
left a comment
There was a problem hiding this comment.
The comments from my review of the crypto PR apply here (with some variations for the test part). In addition, I'm not sure how to tweak the crypto configuration when setting up an Mbed TLS build tree.
Signed-off-by: Yi Wu <yi.wu2@arm.com>
Signed-off-by: Yi Wu <yi.wu2@arm.com>
Signed-off-by: Yi Wu <yi.wu2@arm.com>
Signed-off-by: Yi Wu <yi.wu2@arm.com>
Signed-off-by: Yi Wu <yi.wu2@arm.com>
gilles-peskine-arm
left a comment
There was a problem hiding this comment.
LGTM except I don't see why you couldn't set both mbedtls and TF-PSA-Crypto options. For example, the CI does builds with both a different mbedtls base file and a different crypto base file.
Comments from my review of the crypto PR also apply.
Signed-off-by: Yi Wu <yi.wu2@arm.com>
gilles-peskine-arm
left a comment
There was a problem hiding this comment.
The last commit broke MBEDTLS_CONFIG_BASE_FILE with a relative path (AFAICT it did work before). And one test seems ineffective. LGTM otherwise.
|
|
||
| msg "configure: resolve a relative base config from the source tree" | ||
| cmake -H"$MBEDTLS_ROOT_DIR" -B"$OUT_OF_SOURCE_DIR" \ | ||
| -DMBEDTLS_CONFIG_BASE_FILE=configs/config-ccm-psk-tls1_2.h \ |
There was a problem hiding this comment.
This seems wrong. I would expect MBEDTLS_CONFIG_BASE_FILE to be relative to the current directory. (MBEDTLS_CONFIG_FILE is less intuitive, because the C macro is eventually relative to the install path, and it's not obvious how that relates to the source tree or the build tree.)
| -DTF_PSA_CRYPTO_CONFIG_BASE_FILE="$MBEDTLS_ROOT_DIR/tf-psa-crypto/configs/crypto-config-symmetric-only.h" \ | ||
| -DTF_PSA_CRYPTO_CONFIG_UNSET=PSA_WANT_ALG_RIPEMD160 \ |
There was a problem hiding this comment.
Why not use the designated crypto config file to use with configs/config-ccm-psk-tls1_2.h? E.g.
| -DTF_PSA_CRYPTO_CONFIG_BASE_FILE="$MBEDTLS_ROOT_DIR/tf-psa-crypto/configs/crypto-config-symmetric-only.h" \ | |
| -DTF_PSA_CRYPTO_CONFIG_UNSET=PSA_WANT_ALG_RIPEMD160 \ | |
| -DTF_PSA_CRYPTO_CONFIG_BASE_FILE="$MBEDTLS_ROOT_DIR/tf-psa-crypto/configs/crypto-config-ccm-psk-tls1_2.h" \ | |
| -DTF_PSA_CRYPTO_CONFIG_UNSET=MBEDTLS_HAVE_TIME \ |
| set(MBEDTLS_CONFIG_FILE "" CACHE FILEPATH "Mbed TLS config file (overrides default).") | ||
| set(MBEDTLS_USER_CONFIG_FILE "" CACHE FILEPATH "Mbed TLS user config file (appended to default).") | ||
| set(MBEDTLS_CONFIG_BASE_FILE "" CACHE FILEPATH "Mbed TLS config to copy and optionally transform.") | ||
| set(MBEDTLS_CONFIG_BASE_FILE "" CACHE STRING "Mbed TLS config to copy and optionally transform.") |
There was a problem hiding this comment.
MBEDTLS_CONFIG_BASE_FILE is definitely a FILEPATH. It's a file that gets consumed by CMake. (Unlike MBEDTLS_CONFIG_FILE which is consumed by the C compiler and also meaningful after installation — that one might actually be a STRING, but changing that is out of scope here.)
| mkdir "$OUT_OF_SOURCE_DIR" | ||
| cd "$OUT_OF_SOURCE_DIR" | ||
|
|
||
| msg "build: combine Mbed TLS and TF-PSA-Crypto transformations" |
There was a problem hiding this comment.
I think this test is ineffective. Surely having a TF-PSA-Crypto transformation can only work when tf-psa-crypto has Mbed-TLS/TF-PSA-Crypto#868 ?
(But combining does seem to work if I check out the crypto PR in the submodule. With the caveat that TF_PSA_CRYPTO_CONFIG_BASE_FILE needs to be an absolute path on the current head, but it works with a relative path on 34538973d123e1995acf691606475bea205f1e0f.)
Signed-off-by: Yi Wu <yi.wu2@arm.com>
|
The CI is unhappy |
Signed-off-by: Yi Wu <yi.wu2@arm.com>
Signed-off-by: Yi Wu <yi.wu2@arm.com>
Fixes #10838.
Add support for selecting and customizing compile-time configs in CMake with:
MBEDTLS_CONFIG_BASE_FILEMBEDTLS_CONFIG_NAMEMBEDTLS_CONFIG_SETMBEDTLS_CONFIG_UNSETFor example:
CMake copies the selected base config into the build tree before applying the transformations. The original file is not modified.
MBEDTLS_CONFIG_FILEstill working as the config header passed and cannot be combined with the transformation options.Generated Mbed TLS and PSA configs are also installed correctly, so installed CMake targets do not reference the producer build tree.
Depends on Mbed-TLS/TF-PSA-Crypto#868 for the PSA CMake config support.
PR checklist