Skip to content
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

bootutil: Add better mode selection checks #2047

Merged
merged 2 commits into from
Sep 10, 2024
Merged

Conversation

nordicjm
Copy link
Collaborator

Improves the mode selection checks to prevent selecting multiple
conflicting modes as has been seen in TFM

Also reverts a change that was not needed

boot/bootutil/src/bootutil_priv.h Show resolved Hide resolved
boot/bootutil/src/bootutil_priv.h Show resolved Hide resolved
@davidvincze
Copy link
Collaborator

davidvincze commented Aug 28, 2024

Currently, it fails because it needs a recently merged patch in TF-M: https://review.trustedfirmware.org/c/TF-M/trusted-firmware-m/+/30761 (modifying the default value of direct_xip_revert).

I've just opened this PR to update the version of TF-M used in the fih-tests, however it also fails for a missing CMake dependency. @utzig could you help with updating the fih-test image on Docker Hub?
https://hub.docker.com/r/mcuboot/fih-test
Do you know who else has access to the Hub?

@utzig
Copy link
Member

utzig commented Sep 2, 2024

I've just opened this PR to update the version of TF-M used in the fih-tests, however it also fails for a missing CMake dependency. @utzig could you help with updating the fih-test image on Docker Hub? https://hub.docker.com/r/mcuboot/fih-test Do you know who else has access to the Hub?

Only I have access, but I'm pretty sure I sent @d3zd3z an invite, although he may not have accepted it since he's not showing up here. If you want one I can send to you. Or maybe it's also somewhat broken since they changed their plans for open-source projects. The best way forward is to migrate to Github's own hub which didn't exist back in the day.

Anyway, I tried rebuilding the image, but it will still fail because cmake for focal is 3.16.3 (see: https://launchpad.net/ubuntu/focal/+package/cmake). So I'm gonna try building with jammy which has a newer cmake release that is new enough to not break the build (see: https://launchpad.net/ubuntu/jammy/+package/cmake). If the build succeeds I'll push 0.0.3 and open a PR with the updates.

@nordicjm nordicjm force-pushed the modefixes branch 2 times, most recently from 2bd15a4 to e1fdffa Compare September 5, 2024 07:46
@nordicjm
Copy link
Collaborator Author

nordicjm commented Sep 5, 2024

@davidvincze any clue as to how to update these tests? Seemingly TF-M has split tests off into another repo, the instructions state:

.. code-block:: bash

    git clone https://git.trustedfirmware.org/TF-M/tf-m-tests.git
    git checkout <recommended tf-m-tests commit>

Regression Tests
================
For instructions on configuring, building and executing the regression tests
please refer to the documentation in **tf-m-tests** repository (to be added).
The regression test application is located under **/tests_reg** folder.
It is recommended to build both SPE and NSPE from that folder.

The basic commands for building the regression tests will be:

.. code-block:: bash

    cd </tf-m-tests/tests_reg>
    cmake -S spe -B build_spe -DTFM_PLATFORM=arm/mps2/an521 \
          -DCONFIG_TFM_SOURCE_PATH=<TF-M source dir> \
          -DTFM_TOOLCHAIN_FILE=<Absolute path to>/toolchain_ARMCLANG.cmake \
          -DTEST_S=ON -DTEST_NS=ON
    cmake --build build_spe -- install

    cmake -S . -B build_test -DCONFIG_SPE_PATH=<Absolute path to>/build_spe/api_ns
    cmake --build build_test

But this does not work:

[5/9] Performing configure step for 'TF-M'
FAILED: temp/src/TF-M-stamp/TF-M-configure /tmp/qq/trusted-firmware-m/tf-m-tests/tests_reg/build_spe/temp/src/TF-M-stamp/TF-M-configure 
cd /tmp/qq/trusted-firmware-m/tf-m-tests/tests_reg/build_spe/build-spe && /usr/bin/cmake -DCMAKE_INSTALL_PREFIX:PATH=/tmp/qq/trusted-firmware-m/tf-m-tests/tests_reg/build_spe/api_ns -DCONFIG_TFM_TEST_DIR=/tmp/qq/trusted-firmware-m/tf-m-tests/tests_reg/spe/../test/secure_regression -DCONFIG_TFM_TEST_CONFIG_FILE=/tmp/qq/trusted-firmware-m/tf-m-tests/tests_reg/spe/../test/config/config.cmake -GNinja -C/tmp/qq/trusted-firmware-m/tf-m-tests/tests_reg/build_spe/temp/tmp/TF-M-cache-.cmake -S /tmp/qq/trusted-firmware-m/tf-m-tests/tests_reg -B /tmp/qq/trusted-firmware-m/tf-m-tests/tests_reg/build_spe/build-spe && /usr/bin/cmake -E touch /tmp/qq/trusted-firmware-m/tf-m-tests/tests_reg/build_spe/temp/src/TF-M-stamp/TF-M-configure
loading initial cache file /tmp/qq/trusted-firmware-m/tf-m-tests/tests_reg/build_spe/temp/tmp/TF-M-cache-.cmake
CMake Error at CMakeLists.txt:10 (message):
  CONFIG_SPE_PATH = is not defined or incorrect.  Please provide full path to
  TF-M build artifacts using -DCONFIG_SPE_PATH=

This reverts commit ab4fb32.

This was a wrong change and was caused by a faulty TFM board

Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
Improves the mode selection checks to prevent selecting multiple
conflicting modes as has been seen in TFM

Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
@nordicjm nordicjm merged commit 43a49a3 into mcu-tools:main Sep 10, 2024
57 of 58 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants