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.

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

Update PSA defines and headers #9214

Closed
wants to merge 3 commits into from
Closed

Update PSA defines and headers #9214

wants to merge 3 commits into from

Conversation

orenc17
Copy link
Contributor

@orenc17 orenc17 commented Dec 31, 2018 •
edited
Loading

Description

  • Created PSA headers:
    • psa/client.h & psa/service.h mandated by PSA-FF.
    • psa/internal_trusted_storage.h as defined psa_trusted_storage_api.
  • Updated iovec type names according to spec changes.
  • Updated all services and tests to use the new defines and headers.
  • Preparation for TFM integration

Pull request type

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

Reviewers

@alzix @mikisch81 @Patater @danny4478 @adrianlshaw @athoelke

@ciarmcom ciarmcom requested review from alzix, danny4478, mikisch81, Patater and a team December 31, 2018 16:00
@ciarmcom
Copy link
Member

@orenc17, thank you for your changes.
@Patater @mikisch81 @alzix @danny4478@ARMmbed/mbed-os-tls @ARMmbed/mbed-os-maintainers please review.

alzix
alzix reviewed Jan 1, 2019
View reviewed changes
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

@ARMmbed/mbed-os-maintainers please do not merge before approved by @Patater.
Some changes here should be propagated to https://github.com/ARMmbed/mbed-crypto repo

alzix
alzix suggested changes Jan 1, 2019
View reviewed changes
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.

@orenc17 why we are adding a sub-module in this PR?
features/mbedtls/importer/TARGET_IGNORE/mbedcrypto please remove

danny4478
danny4478 reviewed Jan 1, 2019
View reviewed changes
Copy link
Contributor

@danny4478 danny4478 left a comment

Choose a reason for hiding this comment

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

Do we still need spm_client.h & spm_server.h?

0xc0170
0xc0170 reviewed Jan 2, 2019
View reviewed changes
features/mbedtls/inc/psa/crypto.h
@@ -61,20 +61,7 @@ extern "C" {
* @{
*/

#if defined(PSA_SUCCESS)
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this change be done in mbedtls (new version would update this) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes but, to not break the compilation i must include this chnage

Copy link
Contributor

Choose a reason for hiding this comment

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

cc @sbutcher-arm

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this change should be done upstream first via a new PR to https://github.com/ARMmbed/mbed-crypto

Copy link
Contributor

Choose a reason for hiding this comment

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

@Patater FYI ARMmbed/mbed-crypto#5

Copy link
Contributor

Choose a reason for hiding this comment

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

The necessary changes will come in through #9463

Copy link
Contributor

Choose a reason for hiding this comment

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

Than I suggest removing this file from this PR and wait for #9463 to be merged first.

@simonbutcher
Copy link
Contributor

@0xc0170 - You need to tag @ARMmbed/mbed-os-crypto, not the TLS team (or myself).

@0xc0170 0xc0170 requested a review from a team January 2, 2019 13:52
@orenc17 orenc17 mentioned this pull request Jan 2, 2019
Closed
@cmonr
Copy link
Contributor

cmonr commented Jan 3, 2019

@adbridge Fyi #9214 (comment)

This was referenced Jan 9, 2019
Merged
Closed
alzix
alzix approved these changes Jan 13, 2019
View reviewed changes
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

@alzix
Copy link
Contributor

alzix commented Jan 13, 2019

@Patater - please review and approve. It blocks #9312 .

@alzix
Copy link
Contributor

alzix commented Jan 17, 2019

@Patater - ping

@alzix
Copy link
Contributor

alzix commented Jan 23, 2019

depends on #9463

mikisch81
mikisch81 suggested changes Jan 25, 2019
View reviewed changes
TESTS/psa/entropy_inject/its_reset/COMPONENT_SPE/TARGET_MBED_SPM/psa_test_its_reset_partition.c Outdated Show resolved Hide resolved
...prot_internal_storage/its_reset/COMPONENT_SPE/TARGET_MBED_SPM/psa_test_its_reset_partition.c Outdated Show resolved Hide resolved
TESTS/psa/spm_client/COMPONENT_SPE/TARGET_MBED_SPM/psa_client_tests_part1_partition.c Outdated Show resolved Hide resolved
TESTS/psa/spm_server/COMPONENT_SPE/TARGET_MBED_SPM/psa_server_test_part1_partition.c Outdated Show resolved Hide resolved
TESTS/psa/spm_server/COMPONENT_SPE/TARGET_MBED_SPM/psa_server_test_part2_partition.c Outdated Show resolved Hide resolved
...GET_PSA/services/psa_prot_internal_storage/COMPONENT_SPE/TARGET_MBED_SPM/psa_its_partition.c Outdated Show resolved Hide resolved
features/mbedtls/inc/psa/crypto.h
@@ -61,20 +61,7 @@ extern "C" {
* @{
*/

#if defined(PSA_SUCCESS)
Copy link
Contributor

Choose a reason for hiding this comment

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

Than I suggest removing this file from this PR and wait for #9463 to be merged first.

rtos/TARGET_CORTEX/mbed_rtos_rtx.c Outdated Show resolved Hide resolved
rtos/TARGET_CORTEX/mbed_rtos_rtx.c Outdated Show resolved Hide resolved
tools/spm/templates/COMPONENT_SPE/TARGET_MBED_SPM/psa_NAME_partition.c.tpl Outdated Show resolved Hide resolved
@mikisch81 mikisch81 mentioned this pull request Jan 27, 2019
Merged
@orenc17 orenc17 mentioned this pull request Jan 29, 2019
Merged
@orenc17 orenc17 closed this Jan 30, 2019
@orenc17
Copy link
Contributor Author

orenc17 commented Jan 30, 2019

@Patater closing this as #9192 fix all the issues and includes it as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danny4478 danny4478 danny4478 left review comments

@0xc0170 0xc0170 0xc0170 left review comments

@mikisch81 mikisch81 mikisch81 approved these changes

@alzix alzix alzix approved these changes

@Patater Patater Awaiting requested review from Patater

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@orenc17 @ciarmcom @simonbutcher @cmonr @alzix @Patater @0xc0170 @mikisch81 @danny4478 @studavekar