CMake: Fix Mbed TLS compilation on Cortex-M0/0+/1/M23 #14529

LDong-Arm · 2021-04-09T13:38:27Z

Summary of changes

Due to a known issue in Mbed TLS's architecture determination (Mbed-TLS/mbedtls#1077), we get the error

error: inline assembly requires more registers than available

when compiling bignum.c for Cortex-M0/0+/1/M23 which do not have the macro __thumb2__ set by the compiler.

The workaround is to define the macro MULADDC_CANNOT_USE_R7 which is already defined by Mbed CLI 1 but missing in our CMake support.

Fixes ARMmbed/mbed-os-example-lorawan#220

Impact of changes

Migration actions required

Documentation

None.

Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@ARMmbed/mbed-os-core @Patater

ciarmcom · 2021-04-09T14:00:18Z

@LDong-Arm, thank you for your changes.
@Patater @ARMmbed/mbed-os-maintainers please review.

harmut01

LGTM

Pull request has been modified.

LDong-Arm · 2021-04-09T14:03:11Z

LGTM

@harmut01 Thanks for the review, but I just force-pushed and changed the approach 😉

0xc0170 · 2021-04-12T08:16:33Z

CI started

mbed-ci · 2021-04-12T09:48:53Z

Jenkins CI Test : ❌ FAILED

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-example-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_greentea-test	✔️
jenkins-ci/mbed-os-ci_cmake-example-test	❌

LDong-Arm · 2021-04-12T10:16:07Z

mbed-os-example-crash-reporting in cmake-example-test failed on K64F. Being a Cortex-M4F target it doesn't look related to this PR. I'll try to reproduce it anyway.

LDong-Arm · 2021-04-12T10:44:14Z

It's a CI issue that the mbed-os-example-crash-reporting test is too time-dependent.

mbedhtrun waited one second after the image is flashed, but the application was already running in the background. And after one second, the first line (= System will be rebooted due to a fatal error =) expected by the log was already missed:

[1618220306.79][GLRM][INF] waiting 1.00 sec after reset
[1618220307.85][GLRM][TXD] mbedmbedmbedmbedmbedmbedmbedmbedmbedmbed
[1618220307.85][CONN][WRN] skipping __sync packet (specified with --sync=0)
[1618220307.85][CONN][WRN] UnicodeDecodeError encountered!
[1618220307.85][CONN][RXD] = Reboot count(=1) reached maximum, system will halt after rebooting =

@0xc0170 @saheerb This happens from time to time and not just for one PR: https://jenkins-mbedos.oss.arm.com/blue/organizations/jenkins/mbed-os-ci_cmake-example-test/activity We might need a permanent solution.

saheerb · 2021-04-12T11:52:18Z

Yea, what I think is the application is restarted before greentea could process.

@jamesbeyond I think removing first line will make test more robust. What do you think?

LDong-Arm · 2021-04-12T12:45:08Z

Yea, what I think is the application is restarted before greentea could process.

@jamesbeyond I think removing first line will make test more robust. What do you think?

Well, this still relies on the timing of the example. The permanent solution would be an option in mbedgt to skip the reset and/or wait, useful for testing examples.

For now, I wonder if rerunning the CI could allow us to get this PR in.

saheerb · 2021-04-12T12:49:50Z

Yea, what I think is the application is restarted before greentea could process.
@jamesbeyond I think removing first line will make test more robust. What do you think?

Well, this still relies on the timing of the example. The permanent solution would be an option in mbedgt to skip the reset and/or wait, useful for testing examples.

For now, I wonder if rerunning the CI could allow us to get this PR in.

The main issue, what I think here is, depending on the restart attempt the first line will change. If we are not checking the first line, then timing wont matter, would it?

For now, I wonder if rerunning the CI could allow us to get this PR in.
We always try that :)

LDong-Arm · 2021-04-12T13:08:12Z

The main issue, what I think here is, depending on the restart attempt the first line will change. If we are not checking the first line, then timing wont matter, would it?

@saheerb
This isn't the problem. The log is
(many lines above)

= System will be rebooted due to a fatal error =
= Reboot count(=1) reached maximum, system will halt after rebooting =

(many lines below)

So,

The log we are matching is the unchangeable line above the count
We're relying on this line being printed after the 1 sec wait, and timing can be unreliable

If timing is correct, we should get = System will be rebooted due to a fatal error = no matter it's a first attempt or not.

LDong-Arm · 2021-04-12T14:49:54Z

This failure is seen in lots of places. Need to fix it probably.

LDong-Arm · 2021-04-15T08:35:30Z

This can be merged. The label wasn't updated automatically.

Patater · 2021-04-15T08:42:44Z

connectivity/mbedtls/CMakeLists.txt

@@ -101,4 +101,6 @@ target_sources(mbed-mbedtls
 target_compile_definitions(mbed-mbedtls
    INTERFACE
        MBED_CONF_MBEDTLS_PRESENT=1
+        # workaround for https://github.com/ARMmbed/mbedtls/issues/1077
+        MULADDC_CANNOT_USE_R7


Can we make this conditional on an M0 or M0+ core being used?

Let me check why this workaround isn't needed for GCC. But anyway the workaround is probably to set it for M0/M0+.

Done. The condition also includes M1 and M23 (e.g. NUMAKER_IOT_M263A), added in my latest push.

As for GCC, it compiles exactly the same assembly but doesn't fail, probably related to some optimisation.

Due to a known issue in Mbed TLS's architecture determination (Mbed-TLS/mbedtls#1077), we get the error error: inline assembly requires more registers than available when compiling `bignum.c` for Cortex-M0/0+/1/M23 which do not have the macro `__thumb2__` set by the compiler. The workaround is to define the macro `MULADDC_CANNOT_USE_R7` which is already defined by Mbed CLI 1 but missing in our CMake support. Fixes ARMmbed/mbed-os-example-lorawan#220

Pull request has been modified.

0xc0170 · 2021-04-15T10:37:43Z

CI started

mbed-ci · 2021-04-15T12:20:04Z

Jenkins CI Test : ✔️ SUCCESS

Build Number: 4 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-example-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-test	✔️
jenkins-ci/mbed-os-ci_greentea-test	✔️

0xc0170 · 2021-04-15T12:24:44Z

@Patater all good now ?

Patater · 2021-04-15T12:28:36Z

Yep, looks good

msmttchr · 2021-08-23T19:40:54Z

@LDong-Arm, sorry for unrelated question, but I was under the impression that mbedtls requires an Armv8-A, Armv9-A and Armv8-M core (see https://www.trustedfirmware.org/), but actually this fix is for Armv6. Is mbedtls able to run in Armv6?

LDong-Arm · 2021-08-24T09:15:22Z

@LDong-Arm, sorry for unrelated question, but I was under the impression that mbedtls requires an Armv8-A, Armv9-A and Armv8-M core (see https://www.trustedfirmware.org/), but actually this fix is for Armv6. Is mbedtls able to run in Armv6?

@msmttchr Mbed TLS is a generic TLS and Crypto library that runs on all kinds of hardware, even including legacy Arm architectures (prior to Armv6) and other platforms such as x86, MIPS, etc. So it is able to run on Armv6, and Mbed OS has a few v6 targets.

What you mentioned is Trusted Firmware, and the firmware itself mainly targets relatively recent Arm platforms. Trusted Firmware M uses Mbed TLS as a dependency.

This was referenced Apr 9, 2021

CMake build fails with error: inline assembly requires more registers than available ARMmbed/mbed-os-example-lorawan#220

Closed

make target list for applications in examples_cmake.json consistent with examples.json #14507

Merged

ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Apr 9, 2021

ciarmcom requested review from Patater and a team April 9, 2021 14:00

ciarmcom added needs: review component: tools labels Apr 9, 2021

harmut01 previously approved these changes Apr 9, 2021

View reviewed changes

LDong-Arm force-pushed the cmake_MULADDC branch from c23113f to 5f05845 Compare April 9, 2021 14:02

LDong-Arm requested a review from harmut01 April 9, 2021 14:04

harmut01 previously approved these changes Apr 9, 2021

View reviewed changes

mergify bot added needs: CI and removed needs: review labels Apr 9, 2021

0xc0170 previously approved these changes Apr 12, 2021

View reviewed changes

mergify bot added needs: work and removed needs: CI labels Apr 12, 2021

This was referenced Apr 12, 2021

Check for duplicate status callbacks before adding to the list #14497

Merged

STM32: Standard Pin Names #14480

Merged

Patater previously requested changes Apr 15, 2021

View reviewed changes

LDong-Arm force-pushed the cmake_MULADDC branch from 5f05845 to 86e7bc5 Compare April 15, 2021 10:04

LDong-Arm changed the title ~~CMake: Fix Mbed TLS compilation on Cortex-M0/0+/1~~ CMake: Fix Mbed TLS compilation on Cortex-M0/0+/1/M23 Apr 15, 2021

LDong-Arm requested review from Patater, harmut01 and 0xc0170 April 15, 2021 10:07

0xc0170 approved these changes Apr 15, 2021

View reviewed changes

mergify bot added needs: CI and removed needs: work labels Apr 15, 2021

harmut01 approved these changes Apr 15, 2021

View reviewed changes

mergify bot added ready for merge and removed needs: CI labels Apr 15, 2021

Patater approved these changes Apr 15, 2021

View reviewed changes

0xc0170 merged commit 7f42511 into ARMmbed:master Apr 15, 2021

mergify bot removed the ready for merge label Apr 15, 2021

mbedmain added release-version: 6.10.0 Release-pending labels Apr 16, 2021

mbedmain removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Apr 26, 2021

CMake: Fix Mbed TLS compilation on Cortex-M0/0+/1/M23 #14529

CMake: Fix Mbed TLS compilation on Cortex-M0/0+/1/M23 #14529

Uh oh!

Conversation

LDong-Arm commented Apr 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

Uh oh!

ciarmcom commented Apr 9, 2021

Uh oh!

harmut01 left a comment

Choose a reason for hiding this comment

Uh oh!

LDong-Arm commented Apr 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xc0170 commented Apr 12, 2021

Uh oh!

mbed-ci commented Apr 12, 2021

Jenkins CI Test : ❌ FAILED

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

Uh oh!

LDong-Arm commented Apr 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LDong-Arm commented Apr 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

saheerb commented Apr 12, 2021

Uh oh!

LDong-Arm commented Apr 12, 2021

Uh oh!

saheerb commented Apr 12, 2021

Uh oh!

LDong-Arm commented Apr 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LDong-Arm commented Apr 12, 2021

Uh oh!

LDong-Arm commented Apr 15, 2021

Uh oh!

Patater Apr 15, 2021

Choose a reason for hiding this comment

Uh oh!

LDong-Arm Apr 15, 2021

Choose a reason for hiding this comment

Uh oh!

LDong-Arm Apr 15, 2021

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Apr 15, 2021

Uh oh!

mbed-ci commented Apr 15, 2021

Jenkins CI Test : ✔️ SUCCESS

Build Number: 4 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

Uh oh!

0xc0170 commented Apr 15, 2021

Uh oh!

Patater commented Apr 15, 2021

Uh oh!

msmttchr commented Aug 23, 2021

Uh oh!

LDong-Arm commented Aug 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

LDong-Arm commented Apr 9, 2021 •

edited

Loading

LDong-Arm commented Apr 9, 2021 •

edited

Loading

LDong-Arm commented Apr 12, 2021 •

edited

Loading

LDong-Arm commented Apr 12, 2021 •

edited

Loading

LDong-Arm commented Apr 12, 2021 •

edited

Loading

LDong-Arm commented Aug 24, 2021 •

edited

Loading