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

Enable PSA tests for fastmodel #11589

Merged
merged 4 commits into from
Oct 4, 2019
Merged

Enable PSA tests for fastmodel #11589

merged 4 commits into from
Oct 4, 2019

Conversation

jamesbeyond
Copy link
Contributor

Description

Enable PSA test for Fastmodels

Pull request type

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

Reviewers

Release Notes

@jamesbeyond
Copy link
Contributor Author

test results:

target platform_name test suite result elapsed_time (sec) copy_method
FVP_MPS2_M3-GCC_ARM FVP_MPS2_M3 components-target_psa-tests-compliance_attestation-test_a001 OK 24.49 default
FVP_MPS2_M3-GCC_ARM FVP_MPS2_M3 components-target_psa-tests-compliance_its-test_s001 OK 9.18 default
FVP_MPS2_M3-GCC_ARM FVP_MPS2_M3 components-target_psa-tests-compliance_its-test_s002 OK 9.64 default
FVP_MPS2_M3-GCC_ARM FVP_MPS2_M3 components-target_psa-tests-compliance_its-test_s004 OK 9.22 default
FVP_MPS2_M3-GCC_ARM FVP_MPS2_M3 components-target_psa-tests-compliance_its-test_s005 OK 9.35 default
FVP_MPS2_M3-GCC_ARM FVP_MPS2_M3 components-target_psa-tests-compliance_its-test_s006 OK 9.2 default
FVP_MPS2_M3-GCC_ARM FVP_MPS2_M3 components-target_psa-tests-compliance_its-test_s007 OK 9.35 default
FVP_MPS2_M3-GCC_ARM FVP_MPS2_M3 components-target_psa-tests-compliance_its-test_s008 OK 9.38 default
FVP_MPS2_M3-GCC_ARM FVP_MPS2_M3 components-target_psa-tests-compliance_its-test_s009 OK 9.5 default
FVP_MPS2_M3-GCC_ARM FVP_MPS2_M3 components-target_psa-tests-compliance_its-test_s010 OK 9.34 default
FVP_MPS2_M3-GCC_ARM FVP_MPS2_M3 tests-psa-attestation OK 15.11 default
FVP_MPS2_M3-GCC_ARM FVP_MPS2_M3 tests-psa-crypto_init OK 9.67 default
FVP_MPS2_M3-GCC_ARM FVP_MPS2_M3 tests-psa-its_ps OK 9.43 default

@ciarmcom ciarmcom requested a review from a team September 27, 2019 17:00
@ciarmcom
Copy link
Member

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

@0xc0170 0xc0170 requested a review from a team September 30, 2019 11:17
Patater
Patater suggested changes Sep 30, 2019
View reviewed changes
targets/TARGET_ARM_FM/TARGET_FVP_MPS2/trng_api.c Outdated
*output_length = length;

return 0;

Copy link
Contributor

Choose a reason for hiding this comment

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

Style: Would recommend removing extra newline here.

targets/TARGET_ARM_FM/TARGET_FVP_MPS2/trng_api.c Outdated
int trng_get_bytes(trng_t *obj, uint8_t *output, size_t length, size_t *output_length)
{
(void)obj;
(void)output;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why cast output to void here? It looks like we are using output in the loop below. Which compiler warning are you trying to avoid with this cast? I am not sure it is necessary, but if it is necessary, I am curious which compiler is making the warning and what the warning says.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, missed that

targets/targets.json
@@ -8609,7 +8609,7 @@
"ARM_FM": {
"inherits": ["Target"],
"public": false,
"extra_labels": ["ARM_FM"]
"extra_labels": ["ARM_FM","PSA"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding the PSA label requires that the target supports KVStore, due to how PSA storage is currently implemented in Mbed OS. Is KVStore supported with fast models?

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, KVStore is supported by FastModels

@jamesbeyond
Copy link
Contributor Author

@Patater thanks for the review, your comments been addressed.

@0xc0170 0xc0170 requested a review from Patater October 1, 2019 10:45
@0xc0170
Copy link
Contributor

0xc0170 commented Oct 1, 2019

I run CI meanwhile

@mbed-ci
Copy link

mbed-ci commented Oct 1, 2019

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_cloud-client-test

@jamesbeyond
Copy link
Contributor Author

I run CI meanwhile

The failures seems not related, I'll restart the test

@mbed-ci
Copy link

mbed-ci commented Oct 1, 2019

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_cloud-client-test

Patater
Patater approved these changes Oct 1, 2019
View reviewed changes
Copy link
Contributor

@Patater Patater left a comment

Choose a reason for hiding this comment

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

OK for this target to be used for testing.

@mbed-ci
Copy link

mbed-ci commented Oct 1, 2019

Test run: SUCCESS

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

@jamesbeyond
Copy link
Contributor Author

ping @ARMmbed/mbed-os-maintainers , could you review see if this ready to merge ?

@jamesbeyond
Copy link
Contributor Author

@adbridge ready to merge? 😉

@adbridge
Copy link
Contributor

adbridge commented Oct 4, 2019

@jamesbeyond sorry things will be a bit slower over the next week as Martin is out so I am having to balance 2 jobs :)

@adbridge adbridge merged commit 7eb8807 into ARMmbed:master Oct 4, 2019
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

@Patater Patater Patater approved these changes

Assignees
No one assigned
Labels
release-version: 5.14.1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jamesbeyond @ciarmcom @0xc0170 @mbed-ci @adbridge @Patater