ARMmbed / mbed-os-tf-m-regression-tests

An Mbed OS application that runs the TF-M regression tests
2 stars 12 forks source link

Switch to using TF-M v1.2 #67

Closed jainvikas8 closed 3 years ago

jainvikas8 commented 3 years ago

This update is related to making CoreIPC.config working with TF-M v1.2.

build_tfm.py:

psa_builder.py

tfm_ns_import.yaml

Reviwers : @LDong-Arm @evedon @Patater @urutva

jainvikas8 commented 3 years ago

Travis should pass after - https://github.com/ARMmbed/mbed-os/pull/14187 is merged

jainvikas8 commented 3 years ago

Forced pushed to use ninja.

LDong-Arm commented 3 years ago

Presumably this fixes #12 too.

jainvikas8 commented 3 years ago

Forced pushed to use TARGET_TFM_LATEST folder.

jainvikas8 commented 3 years ago

@jainvikas8 You need to add ninja-build to the list packages (line 26) in .travis.yml. Hopefully Travis CI will pass then.

Good, at least it tests ninja is not installed.

jainvikas8 commented 3 years ago

Forced pushed, to add ninja build

LDong-Arm commented 3 years ago

The copying of crypto_extra.h by tfm_ns_import.yaml overrides https://github.com/ARMmbed/mbed-os/commit/e2e2d0b1f6145c5378c0f64dd5000ea5c2291149. A basic question is, do we need to copy these files every time (since they are already committed to Mbed OS), or is that .yaml mainly to allow testing of any local changes? If we keep the copying, then the solution would be adding the patch to https://github.com/ARMmbed/trusted-firmware-m/tree/mbed-tfm-1.2 too.

jainvikas8 commented 3 years ago

The copying of crypto_extra.h by tfm_ns_import.yaml overrides ARMmbed/mbed-os@e2e2d0b. A basic question is, do we need to copy these files every time (since they are already committed to Mbed OS), or is that .yaml mainly to allow testing of any local changes? If we keep the copying, then the solution would be adding the patch to https://github.com/ARMmbed/trusted-firmware-m/tree/mbed-tfm-1.2 too.

I think we need to raise this - https://github.com/ARMmbed/mbed-os/commit/e2e2d0b1f6145c5378c0f64dd5000ea5c2291149 as a patch on https://github.com/ARMmbed/trusted-firmware-m/tree/mbed-tfm-1.2 and also check if we see the same issue on Vanilla TF-M v1.2 with ARM toolchain.

To answer the question, this file doesn't need to be copied every time unless we are upgrading from TF-M v1.2 -> TF-M1.x (for same or new target), because we mostly use the same PSA API version for all the targets.

But this will help when we do continuous integration of Mbed OS + TFM latest

LDong-Arm commented 3 years ago

I think we need to raise this - ARMmbed/mbed-os@e2e2d0b as a patch on https://github.com/ARMmbed/trusted-firmware-m/tree/mbed-tfm-1.2 and also check if we see the same issue on Vanilla TF-M v1.2 with ARM toolchain.

I will raise it. Otherwise we won't be able to compile Mbed applications including the regression and PSA tests.

jainvikas8 commented 3 years ago

Forced pushed to fix review comments.

jainvikas8 commented 3 years ago

I think we need to raise this - ARMmbed/mbed-os@e2e2d0b as a patch on https://github.com/ARMmbed/trusted-firmware-m/tree/mbed-tfm-1.2 and also check if we see the same issue on Vanilla TF-M v1.2 with ARM toolchain.

I will raise it. Otherwise we won't be able to compile Mbed applications including the regression and PSA tests.

I have checked the Vanilla TF-M build with ARM toolchain, it doesn't seem to have this issue. So this is related to Mbed CLI 1 when using Mbed OS.

LDong-Arm commented 3 years ago

I have checked the Vanilla TF-M build with ARM toolchain, it doesn't seem to have this issue. So this is related to Mbed CLI 1 when using Mbed OS.

It's not related to the toolchain or Mbed CLI, the problem is Mbed TLS compatibility. Vanilla TF-M only compiles a small part of Mbed TLS that it uses, but Mbed OS compiles the whole Mbed TLS because some other components (e.g. network) needs it. One such file is pk.c which requires ARMmbed/mbed-os@e2e2d0b. So we need to fix that.

LDong-Arm commented 3 years ago

https://github.com/ARMmbed/trusted-firmware-m/pull/11 created as a follow-up