Skip to content

Musca targets: align and clean up configurations #14467

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 14, 2021
Merged

Musca targets: align and clean up configurations #14467

merged 2 commits into from
Apr 14, 2021

Conversation

LDong-Arm
Copy link
Contributor

@LDong-Arm LDong-Arm commented Mar 24, 2021 •
edited
Loading

Summary of changes

Preceding PR: #14455

This PR fixes the out of memory crash when running mbed-os-example-blinky on Musca B1:

++ MbedOS Error Info ++
Error Status: 0x80010133 Code: 307 Module: 1
Error Message: Mutex: 0x0, System is out of memory
Location: 0xA084121
Error Value: 0x0
Current Thread: main Id: 0x20041334 Entry: 0xA081F7B StackSize: 0x1000 StackMem: 0x200415F0 SP: 0x20042864 
For more info, visit: https://mbed.com/s/error?error=0x80040106&tgt=TARGET_NAME

This PR enables mutexes on Musca B1 to fix this issue, and removes unneeded RTOS overrides from Musca S1.

Also, change the default baud rate to 115200 (aligned with the TF-M secure image, and Musca S1) to avoid broken text on serial when transitioning between secure and non-secure execution.

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)
[x] Tests / results supplied as part of this PR

Now Greentea tests can run on Musca B1.

Reviewers

@ARMmbed/mbed-os-security

LDong-Arm
LDong-Arm commented Mar 24, 2021
View reviewed changes
@ciarmcom ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Mar 24, 2021
@ciarmcom ciarmcom requested a review from a team March 24, 2021 17:30
@ciarmcom
Copy link
Member

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

@LDong-Arm LDong-Arm changed the title ARM_MUSCA_B1: fix out-of-memory crash of Greentea tests ARM_MUSCA_B1: add missing RTOS configuration Mar 26, 2021
@LDong-Arm LDong-Arm changed the title ARM_MUSCA_B1: add missing RTOS configuration ARM_MUSCA_B1: add missing RTOS & stdio configurations Mar 26, 2021
@LDong-Arm
Copy link
Contributor Author

@ARMmbed/mbed-os-core This PR was created some days ago, but should be a quick one anyway.

0xc0170
0xc0170 previously approved these changes Apr 6, 2021
View reviewed changes
Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still curious about the values. Looks like there's memory limit .

User stack size not sufficient but also we need to limit libspace allocation for threads?

@mergify mergify bot added needs: CI and removed needs: review labels Apr 6, 2021
@Patater
Copy link
Contributor

Patater commented Apr 6, 2021

Still curious about the values. Looks like there's memory limit .

Both TF-M itself as well as its tests rely on the Automatic Dynamic RAM allocation capability of CMSIS-RTOSv2. We should be able to be more conservative here in mbed-os and only enable what's needed for TF-M to operate. We don't need all this RAM wasted in the default general-purpose case (yes wasted, because the memory is dedicated to these memory pools and not available to any other use case; due to how TF-M's os_wrapper is designed and implemented for CMSIS-RTOSv2, we can't easily switch to using heap-allocated objects-- although maybe now that's possible if additional hooks [like "free thread"] have been added since our first port to Mbed OS). The mbed-os-tf-m-regression-tests test application can use a different configuration, to provide enough RTOS objects to run the tests; we don't need to copy this for the default Mbed OS configuration.

User stack size not sufficient but also we need to limit libspace allocation for threads?

The reason we reduced the stack size available for dynamically created threads was because, given the number of threads the TF-M regression tests employ, we'd run out of RAM if we used the default amount.

LDong-Arm added 2 commits April 6, 2021 18:11
@LDong-Arm
ARM_MUSCA_B1: default baud rate to 115200
bd92ca0 
The TF-M secure binary has a fixed baud rate of 115200. Having a
different baud rate on the non-secure side results in broken serial
outputs.
@LDong-Arm
TF-M targets: Clean up RTOS configuration
c4178b4 
On Armv8 targets, the PSA interface on the non-secure side only
requires mutexes, thus we remove other RTOS overrides to reduce
memory usage in general use cases.

TF-M and PSA test applications require more RTOS resources, and
they have their own configurations defined in
mbed-os-tf-m-regression-tests.

This commit also adds missing configuration for ARM_MUSCA_B1.
@mergify mergify bot dismissed 0xc0170’s stale review April 6, 2021 17:18

Pull request has been modified.

@LDong-Arm
Copy link
Contributor Author

LDong-Arm commented Apr 6, 2021 •
edited
Loading

@Patater Completely agree. For a general use case (incl. mbed-os-example-psa), only mutexes are needed by the PSA non-secure interface on Armv8 targets. I've removed all the other RTOS overrides, and mbed-os-tf-m-regressions-tests enables more resources in its own mbed_app.json.

@LDong-Arm
Copy link
Contributor Author

Ready for review again.

@LDong-Arm LDong-Arm changed the title ARM_MUSCA_B1: add missing RTOS & stdio configurations Musca targets: align and clean up configurations Apr 6, 2021
@0xc0170
Copy link
Contributor

0xc0170 commented Apr 13, 2021

CI started

@mbed-ci
Copy link

mbed-ci commented Apr 13, 2021

Jenkins CI Test : ✔️ SUCCESS

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_build-greentea-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-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_build-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM ✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-test ✔️
jenkins-ci/mbed-os-ci_greentea-test ✔️

@0xc0170 0xc0170 merged commit 033094f into ARMmbed:master Apr 14, 2021
@mergify mergify bot removed the ready for merge label Apr 14, 2021
@mbedmain mbedmain removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@0xc0170 0xc0170 0xc0170 approved these changes

+1 more reviewer

@gbrtth gbrtth gbrtth left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

component: core component: security release-version: 6.10.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@LDong-Arm @ciarmcom @Patater @0xc0170 @mbed-ci @gbrtth @mbedmain