-
Notifications
You must be signed in to change notification settings - Fork 209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
qemu_v8: Enable Rust examples build by default #717
Conversation
IBART failure seems unrelated to this PR, @jbech-linaro please have a look. |
Addressed review comments, thanks. |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>
Tags applied, thanks. |
@b49020 the QEMUv8
Should OPTEE_RUST_ENABLE be set to |
@jforissier Rust supports 32-bit TAs [1] too but I have to make them work with OP-TEE build system too. I will add corresponding support prior to change in the PR. [1] https://github.com/apache/incubator-teaclave-trustzone-sdk/blob/no-std/.github/workflows/ci.yml#L90 |
@b49020 excellent, thanks! |
@jforissier Now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@b49020 thanks for the update!
- A few comments below
Edit: Since I managed to run the Rust build an tests in a Docker container with the small changes mentioned below let's ignore the following
- With this PR, cargo
is a new dependency, it should be added to https://optee.readthedocs.io/en/latest/building/prerequisites.html. I will take care of adding it to the optee_os CI Docker image.
- When building on my laptop, on first run of make -j10 check
I get an error (printed several times):
"error: component download failed for cargo-x86_64-unknown-linux-gnu: could not rename downloaded file from '/home/jerome/.rustup/downloads/fc0055ac726cf9fe06dce3bfac393b2744e7ff0970766d02414c1c3e28d0eefa.partial' to '/home/jerome/.rustup/downloads/fc0055ac726cf9fe06dce3bfac393b2744e7ff0970766d02414c1c3e28d0eefa'"
I suspect something is being executed in parallel when it should only run once.
- When building in some Ubuntu 22.04 Docker image, I met the following error:
error[E0463]: can't find crate for std
|
= note: the aarch64-unknown-linux-gnu
target may not be installed
= help: consider downloading the target with rustup target add aarch64-unknown-linux-gnu
Is it some other dependency I'm missing?
There is a
...the However,
|
Ah I missed that, it is the reason for your build and test issues as
This won't be needed once setup.sh executes.
Can you retry this again as I have fixed |
Addressed comments. |
Also, please clean up the prior build too via:
|
It looks like the TA ELF file has something that the OP-TEE loader doesn't like. |
Ah, I can reproduce this issue now. Somehow it passed earlier because my build environment wasn't cleaned properly. I will dig into this issue. |
I can see at least two fields in the ELF header that are unexpected. OP-TEE should probably accept "OS/ABI: ARM", but I'm not so sure about flags being zero. build$ arm-linux-gnueabihf-readelf -h ../out-br/build/optee_rust_examples_ext-1.0/examples/acipher-rs/ta/target/arm-unknown-linux-gnueabihf/release/stripped_ta build$ $ arm-linux-gnueabihf-readelf -h ./build/optee_examples_ext-1.0/acipher/ta/out/a734eed9-d6a1-4244-aa50-7c99719e7b7b.stripped.elf |
As per ELF manpage [1], the flags field value is dependent on |
With OP-TEE/optee_os#6605 the TA panics.
The Rust environment doesn't produce a file (or symlink) called |
I hope debugging Rust TAs situation would improve further in future since we also have to map symbols to Rust code as well. However, the first step that I am trying to take here is enable 32-bit Rust TAs to run.
Thanks for catching that, the fix is already available here: apache/incubator-teaclave-trustzone-sdk#118 to be merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@b49020 many thanks for doing this and addressing the various issues. One minor comment below. With that fixed:
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>
Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (vexpress-qemu_armv8a)
First of all since Rust examples have been tested for Arm targets, so just enable them for Arm platforms only. Further to support different build configurations for host applications and TAs, the Rust examples build have been generalized. So now we only need to define following environment variables to build Rust examples: - TARGET_HOST: Rust toolchain target for host applications. - TARGET_TA: Rust toolchain target for TAs. - CROSS_COMPILE_HOST: GCC cross compiler path for host applications. - CROSS_COMPILE_TA: GCC cross compiler path for TAs - TA_DEV_KIT_DIR: Path to OP-TEE TA development kit directory. - OPTEE_CLIENT_EXPORT: Path to OP-TEE client export directory. Along with that the config option to enable Rust examples is renamed: s/OPTEE_RUST_ENABLE/RUST_ENABLE/. Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org> Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (vexpress-qemu_armv8a) Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
With no-std support it has significantly reduced time to build and test rust examples. So enable rust examples build by default for qemu_v8. Along with that add rust expect script to build repo to avoid check script duplication in OP-TEE rust SDK repo. Link: https://github.com/apache/incubator-teaclave-trustzone-sdk/blob/no-std/ci/qemu-check.exp Acked-by: Yuan Zhuang <yuanz@apache.org> Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org> Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (vexpress-qemu_armv8a) Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
Addressed comments and tags applied, thanks. |
Now that [1] is merged, Rust tests are enabled by default for QEMUv8 and there is no need for a separate Rust job in CI, so remove it. On the other hand, a couple of fixes are needed: - Update PATH so that the cargo command (which is installed locally during the build of the Rust SDK) can be found. - Disable Rust in the BTI+MTE+PAC test because the Rust examples fail to build with the supplied toolchain: /usr/local/bin/../lib/gcc/aarch64-unknown-linux-uclibc/12.2.0/../../../../aarch64-unknown-linux-uclibc/bin/ld.bfd: /tmp/rustcmQty55/libcompiler_builtins-76fca0633b54e12b.rlib(45c91108d938afe8-cpu_model.o): in function `init_have_lse_atomics': /cargo/registry/src/index.crates.io-6f17d22bba15001f/compiler_builtins-0.1.101/./lib/builtins/cpu_model.c:1075: undefined reference to `getauxval' ... Link: OP-TEE/build#717 [1] Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org> Acked-by: Etienne Carriere <etienne.carriere@foss.st.com>
Now that [1] is merged, Rust tests are enabled by default for QEMUv8 and there is no need for a separate Rust job in CI, so remove it. On the other hand, a couple of fixes are needed: - Update PATH so that the cargo command (which is installed locally during the build of the Rust SDK) can be found. - Disable Rust in the BTI+MTE+PAC test because the Rust examples fail to build with the supplied toolchain: /usr/local/bin/../lib/gcc/aarch64-unknown-linux-uclibc/12.2.0/../../../../aarch64-unknown-linux-uclibc/bin/ld.bfd: /tmp/rustcmQty55/libcompiler_builtins-76fca0633b54e12b.rlib(45c91108d938afe8-cpu_model.o): in function `init_have_lse_atomics': /cargo/registry/src/index.crates.io-6f17d22bba15001f/compiler_builtins-0.1.101/./lib/builtins/cpu_model.c:1075: undefined reference to `getauxval' ... - Also disable Rust in the test that enables ftrace, because the signature_verification-rs programm just hangs in this configuration. Link: OP-TEE/build#717 [1] Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Now that [1] is merged, Rust tests are enabled by default for QEMUv8 and there is no need for a separate Rust job in CI, so remove it. On the other hand, a couple of fixes are needed: - Update PATH so that the cargo command (which is installed locally during the build of the Rust SDK) can be found. - Disable Rust in the BTI+MTE+PAC test because the Rust examples fail to build with the supplied toolchain: /usr/local/bin/../lib/gcc/aarch64-unknown-linux-uclibc/12.2.0/../../../../aarch64-unknown-linux-uclibc/bin/ld.bfd: /tmp/rustcmQty55/libcompiler_builtins-76fca0633b54e12b.rlib(45c91108d938afe8-cpu_model.o): in function `init_have_lse_atomics': /cargo/registry/src/index.crates.io-6f17d22bba15001f/compiler_builtins-0.1.101/./lib/builtins/cpu_model.c:1075: undefined reference to `getauxval' ... - Also disable Rust in the test that enables ftrace, because the signature_verification-rs command just hangs in this configuration. Link: OP-TEE/build#717 [1] Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Now that [1] is merged, Rust tests are enabled by default for QEMUv8 and there is no need for a separate Rust job in CI, so remove it. On the other hand, a couple of fixes are needed: - Update PATH so that the cargo command (which is installed locally during the build of the Rust SDK) can be found. - Disable Rust in the BTI+MTE+PAC test because the Rust examples fail to build with the supplied toolchain: /usr/local/bin/../lib/gcc/aarch64-unknown-linux-uclibc/12.2.0/../../../../aarch64-unknown-linux-uclibc/bin/ld.bfd: /tmp/rustcmQty55/libcompiler_builtins-76fca0633b54e12b.rlib(45c91108d938afe8-cpu_model.o): in function `init_have_lse_atomics': /cargo/registry/src/index.crates.io-6f17d22bba15001f/compiler_builtins-0.1.101/./lib/builtins/cpu_model.c:1075: undefined reference to `getauxval' ... - Also disable Rust in the test that enables ftrace, because the signature_verification-rs command just hangs in this configuration. Link: OP-TEE/build#717 [1] Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org> Acked-by: Jens Wiklander <jens.wiklander@linaro.org> Acked-by: Etienne Carriere <etienne.carriere@foss.st.com>
Now that [1] is merged, Rust tests are enabled by default for QEMUv8 and there is no need for a separate Rust job in CI, so remove it. On the other hand, a couple of fixes are needed: - Update PATH so that the cargo command (which is installed locally during the build of the Rust SDK) can be found. - Disable Rust in the BTI+MTE+PAC test because the Rust examples fail to build with the supplied toolchain: /usr/local/bin/../lib/gcc/aarch64-unknown-linux-uclibc/12.2.0/../../../../aarch64-unknown-linux-uclibc/bin/ld.bfd: /tmp/rustcmQty55/libcompiler_builtins-76fca0633b54e12b.rlib(45c91108d938afe8-cpu_model.o): in function `init_have_lse_atomics': /cargo/registry/src/index.crates.io-6f17d22bba15001f/compiler_builtins-0.1.101/./lib/builtins/cpu_model.c:1075: undefined reference to `getauxval' ... - Also disable Rust in the test that enables ftrace, because the signature_verification-rs command just hangs in this configuration. Link: OP-TEE/build#717 [1] Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org> Acked-by: Jens Wiklander <jens.wiklander@linaro.org> Acked-by: Etienne Carriere <etienne.carriere@foss.st.com>
With no-std support it has significantly reduced time to build and test rust examples. So enable rust examples build by default for qemu_v8. Along with that add rust expect script to build repo to avoid check script duplication in OP-TEE rust SDK repo.
@DemesneGH Fyi..