Skip to content

mbed-SPM updates #9823

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

Merged
merged 24 commits into from
Mar 3, 2019
Merged

mbed-SPM updates #9823

merged 24 commits into from
Mar 3, 2019

Conversation

orenc17
Copy link
Contributor

@orenc17 orenc17 commented Feb 24, 2019

Description

  • psa_wait_any() & psa_wait_interrupt() replaced with psa_wait()
  • psa_identity() is replaced with client_id in psa_msg_t
  • Prepare SPM tests to work with TFM
  • Fix print warnings
  • Update secure binaries for FUTURE_SEQUANA_PSA

needs to come in after #9666
This PR is required for #9908 & #9910

Reviewed internally kfnta/mbed-os#91

Pull request type

[ ] Fix
[X] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@ARMmbed/mbed-os-psa

Release Notes

@ciarmcom
Copy link
Member

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

@ciarmcom ciarmcom requested review from a team February 24, 2019 12:00
@mbed-ci
Copy link

mbed-ci commented Feb 24, 2019

Test run: SUCCESS

Summary: 12 of 12 test jobs passed
Build number : 1
Build artifacts

@NirSonnenschein
Copy link
Contributor

CI statred

@mikisch81
Copy link
Contributor

Last 3 commits were to align the PSA SPM tests to TF-M.

Tested on Musca target.

@alzix
Copy link
Contributor

alzix commented Feb 25, 2019

Todo list:
[x] psa_get return value must be handled
[ ] PSA header files - single version only. Use TF-M headers.
[x] Generated headers - verify file names and all the macros are actually "standard"
[ ] Error handling. SPM_PANIC macro must not be used from services and replaced by calls to abort()
[ ] Handle abort() calls - make sure to consult with core team

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 25, 2019

@alzix Just to make sure, this todo list completion is blocking this PR?

@@ -438,7 +438,9 @@ utest::v1::status_t case_teardown_handler(const Case *const source, const size_t

utest::v1::status_t test_setup(const size_t number_of_cases)
{
#ifndef NO_GREENTEA
Copy link
Contributor

Choose a reason for hiding this comment

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

This is dependency, there's another PR adding this to test framework?

Copy link
Contributor

Choose a reason for hiding this comment

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

Then should we drop this commit?

@mikisch81
Copy link
Contributor

Rebased this PR after all the merge-fest that was done..

@@ -0,0 +1,3 @@
{
"name": "usb"
Copy link
Contributor

Choose a reason for hiding this comment

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

???

Copy link
Contributor

Choose a reason for hiding this comment

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

It is from #9874

#include "TARGET_MBED_SPM/psa_defs.h"
#include "TARGET_MBED_SPM/COMPONENT_SPE/spm_server.h"
#include "TARGET_MBED_SPM/COMPONENT_SPE/spm_panic.h"
#else
#error "Compiling psa service header on non-secure target is not allowed"
Copy link
Contributor

Choose a reason for hiding this comment

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

note that the error is misleading.
TARGET_MBED_SPM is applied to both secure and nonsecure targets

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe #error "Compiling psa service header on a PSA emulated target is not allowed"

Copy link
Contributor

Choose a reason for hiding this comment

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

i would phrase it as "expected"

@mikisch81
Copy link
Contributor

@alzix @orenc17 Pushed generation of 'psa_manifest/sid.h' from the tfm generate script.
Did not test it yet.

@mikisch81
Copy link
Contributor

@alzix @orenc17 Now the generation of sid.h is in.

@0xc0170 0xc0170 added risk: R and removed risk: A labels Feb 28, 2019
@0xc0170
Copy link
Contributor

0xc0170 commented Feb 28, 2019

@orenc17 What is still to do here?

@mikisch81
Copy link
Contributor

@alzix need to review again

Copy link
Contributor

@alzix alzix left a comment

Choose a reason for hiding this comment

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

LGTM
we have registered followups tasks in our backlog.
This PR is required for 5.12, all the required changes will be introduced in followup PRs.

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 28, 2019

CI started

@NirSonnenschein
Copy link
Contributor

started CI

@mbed-ci
Copy link

mbed-ci commented Mar 3, 2019

Test run: SUCCESS

Summary: 13 of 13 test jobs passed
Build number : 3
Build artifacts

@NirSonnenschein
Copy link
Contributor

Note: PR doesn't need to come in after #9312 anymore and CI has passed. @0xc0170 @cmonr any objections to bringing this in?

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.

2 notes:

  • commits are very brief , it would be better to provide more details in some. For big PR's like this (101 files in 24 commits, more description needed. These are not small fixes). For instance "Update psa_wait() and client_id" why are we updating it and how? We need to get much better in describing changes. Please spend few minutes to provide details.
  • all new files should have SPDX identifier (should be fixed via new PR)

@0xc0170 0xc0170 merged commit b2fb3d7 into ARMmbed:master Mar 3, 2019
@orenc17
Copy link
Contributor Author

orenc17 commented Mar 3, 2019

@0xc0170 all the new files in this PR have SPDX identifier:
tools/psa/tfm/templates/sid.h.tpl
components/TARGET_PSA/inc/psa_manifest/sid.h

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants