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.

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

Update the CC310 IAR libraries #9122

Merged
merged 4 commits into from
Dec 19, 2018
Merged

Conversation

RonEld
Copy link
Contributor

@RonEld RonEld commented Dec 17, 2018

Description

After testing the Mbed TLS On Target Tests with IAR, on NRF52840 platform, it was discovered that the Cryptocell library is not initiated correct. This fix replaces the libraries received by Mail, with libraries built from the official Cryptocell 310 release, version 1.1.0.1285, using IAR version 7.80.1.11864 and the NRF52840 platform registers base address.

Tested with GreenTea Mbed TLS On Target Tests.

Pull request type

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

@0xc0170
Copy link
Contributor

0xc0170 commented Dec 17, 2018

Can this be captured in the readme for these libs (version of the code used, toolchain version). Same would be good to have inthe commit msg as well

@RonEld
Copy link
Contributor Author

RonEld commented Dec 17, 2018

@0xc0170

Can this be captured in the readme for these libs (version of the code used, toolchain version). Same would be good to have inthe commit msg as well

In this case, I will update this PR with the ARM and GCC_ARM libraries as well, to be synced

@ciarmcom ciarmcom requested review from a team December 17, 2018 14:00
@ciarmcom
Copy link
Member

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

@RonEld
Copy link
Contributor Author

RonEld commented Dec 17, 2018

I am currently working on updating the other toolchain libraries for specific version information, and updating the readme file, as requested earlier.

Copy link
Contributor

@yanesca yanesca left a comment

Choose a reason for hiding this comment

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

The changes are only binaries, there is nothing to review.

@cmonr
Copy link
Contributor

cmonr commented Dec 17, 2018

Waiting on #9131 for a fix to Travis CI.

@RonEld
Copy link
Contributor Author

RonEld commented Dec 17, 2018

I have amended the commit message with version information, and uploaded the libraries for ARM and GCC_ARM with the specific version information.
in addition, I have updated the Readme.md file with the version information. Should we ask someone from the docs team to review the changes in the readme?

* The CC 310 libraries were built from version `arm_sw-cc310-1.1.0.1285`
* The `IAR` libraries were built using `IAR ANSI C/C++ Compiler V7.80.1.11864/W32 for ARM` with `--cpu Cortex-M4f`.
* The `ARM` libraries were built using `ARM Compiler 5.06 update 4 (build 422)` with `--cpu cortex-m4`.
* The `GCC_ARM` libraries were built using `arm-none-eabi-gcc 7.3.1 20180622 (release)` with `-mcpu=cortex-m4`.
Copy link
Contributor

Choose a reason for hiding this comment

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

tested with GCC ARM 6 as that one is supported, this would rather be recompiled

Ron Eldor added 4 commits December 18, 2018 18:04
Update the cryptocell 310 IAR libraries, since the previous ones were not
built correct. The libraries were built from the Cryptocell 310
release version 1.1.0.1285, using IAR version 7.80.1.11864.
Update the cryptocell 310 ARM libraries with known version.
The libraries were built from the Cryptocell 310 release version
1.1.0.1285, using `ARM Compiler 5.06 update 4 (build 422)`.
Update the cryptocell 310 GCC_ARM libraries with known version.
The libraries were built from the Cryptocell 310 release
version 1.1.0.1285, using arm-none-eabi-gcc 6.3.1 20170620 (release) (release).
Update the Cryptocell 310 readme file with the binary library version
information.
@RonEld
Copy link
Contributor Author

RonEld commented Dec 18, 2018

  1. I removed the previous commit with the build with gcc 7.3.1
  2. I amended 2446470 with the new version
  3. I rebuilt the library with gcc 6.3.1, and uploaded it in a new commit

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.

One remaining question: increase binary sizes (in case of IAR 240 percent reported by Github - is that correct) ?

You can check by viewing files in the PR, Github reports the size differences

@RonEld
Copy link
Contributor Author

RonEld commented Dec 19, 2018

@0xc0170

One remaining question: increase binary sizes (in case of IAR 240 percent reported by Github - is that correct) ?

Since the previous IAR libraries were actually broken. I assume this is correct. I haven't built them, and they were built with wchar defined as short ( Probably for IAR 8 ). This could explain why it was broken, and why the size was smaller.

@RonEld
Copy link
Contributor Author

RonEld commented Dec 19, 2018

@0xc0170

Should we ask someone from the docs team to review the changes in the readme?

@mbed-ci
Copy link

mbed-ci commented Dec 19, 2018

Test run: SUCCESS

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

@0xc0170
Copy link
Contributor

0xc0170 commented Dec 19, 2018

@AnotherButler for later review, she is currently OoO. Reviewed by a maintainer should be sufficient for this change

@cmonr cmonr merged commit 2446470 into ARMmbed:master Dec 19, 2018
cmonr pushed a commit that referenced this pull request Dec 19, 2018
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.

6 participants