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

NUCLEO_F756ZG / mbedTLS_SHA1 hw acceleration #4158

Merged
merged 2 commits into from Jul 31, 2017

Conversation

Projects
None yet
@adustm
Member

adustm commented Apr 11, 2017

Description

mbedtls_SHA1 hw acceleration for NUCLEO_F756ZG

Status

READY

Related PRs

Same as #3957 on another branch

This PR needs #3954 + #3947 to be merged first

Steps to test or reproduce

same as in #3947

@adustm

This comment has been minimized.

Member

adustm commented Apr 11, 2017

@RonEld

functionally, I believe it will work, however I would consider replacing the dynamically allocated buffer (ctx->sbuf) to a buffer in the context of size SHA1_BLOCK_SIZE (64) .Whenever it gets to 64, call HAL_HASH_SHA1_Accumulate with this buffer. amd another call to HAL_HASH_SHA1_Accumulate with the rest of the input. This will save memcpy to intermediate buffer, it will save memory fragmentation, so probably improve performance as well. (sometimes two calls to HAL_HASH_SHA1_Accumulate but I think it is faster then memcpy to buffer and memory fragmentation)

#define MBEDTLS_SHA1_C

This comment has been minimized.

@RonEld

RonEld Apr 12, 2017

Contributor

Please remove

This comment has been minimized.

@adustm

This comment has been minimized.

@RonEld

RonEld May 23, 2017

Contributor

@adustm I think it wasn't removed

This comment has been minimized.

@adustm

adustm May 23, 2017

Member

Correct, I missed this one because this branch is focusing on F7 family.

}
if (ctx->sbuf !=NULL) { // allocation occured
memcpy(ctx->sbuf, ptr, size);
ctx->flag = 1;

This comment has been minimized.

@RonEld

RonEld Apr 12, 2017

Contributor

please rename flag to something more clear, such as sbuf_flag

This comment has been minimized.

@adustm

adustm May 22, 2017

Member

not applicable with the new storing mechanism

@0xc0170

This comment has been minimized.

Member

0xc0170 commented May 16, 2017

retest uvisor

@adustm adustm force-pushed the adustm:STM_sha1_f756zg branch from 97fc115 May 22, 2017

@adustm

This comment has been minimized.

Member

adustm commented May 22, 2017

This branch is now aligned with the NUCLEO_F439ZI.

@gilles-peskine-arm

This comment has been minimized.

gilles-peskine-arm commented May 23, 2017

Almost identical to PR #4157 so I won't comment on this one.

@andresag01

This comment has been minimized.

Contributor

andresag01 commented May 24, 2017

This is equivalent to #4162 and I have already reviewed that PR.

@adustm

This comment has been minimized.

Member

adustm commented Jun 12, 2017

Hello,
@0xc0170 do you know what is failing in the Cam-CI uvisor Build & Test script ?
Kind regards
Armelle

@sg- sg- requested a review from yanesca Jun 15, 2017

@adustm adustm force-pushed the adustm:STM_sha1_f756zg branch Jun 16, 2017

@adustm adustm changed the base branch from mbed-os-workshop-17q2 to master Jun 16, 2017

@adustm

This comment has been minimized.

Member

adustm commented Jun 16, 2017

Hello
Approvals are ok, rebase is done.
Kind regards

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Jun 16, 2017

@adustm

This comment has been minimized.

Member

adustm commented Jun 19, 2017

Hello, the scripts are still running after 3 days, I guess it should be executed again ?

Thanks in advance
Armelle

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jun 19, 2017

@adustm Yes, I noticed today as well, notified already CI owners to restart the jobs in another PR

cc @tommikas @miklis

@tommikas

This comment has been minimized.

Contributor

tommikas commented Jun 19, 2017

@0xc0170 Only travis seems to be hanging in this one.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jun 19, 2017

I restarted Travis, the job is https://travis-ci.org/ARMmbed/mbed-os/builds/243562660

@yanesca

Looks good to me.

@0xc0170 0xc0170 closed this Jun 19, 2017

@0xc0170 0xc0170 removed the needs: CI label Jun 19, 2017

@0xc0170 0xc0170 reopened this Jun 19, 2017

@adustm

This comment has been minimized.

Member

adustm commented Jun 20, 2017

Hello @RonEld
Your review approval is missing here. Kind regards
Armelle

@RonEld

RonEld approved these changes Jun 20, 2017

@adustm

This comment has been minimized.

Member

adustm commented Jun 22, 2017

bump ?

@adbridge

This comment has been minimized.

Contributor

adbridge commented Jun 23, 2017

@adustm sorry for the delays, our ci has been tied up with the 5.5.0 and 5.5.1 releases.

@adbridge

This comment has been minimized.

Contributor

adbridge commented Jun 23, 2017

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented Jun 23, 2017

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 622

Example Build failed!

@0xc0170 0xc0170 added needs: CI and removed needs: work labels Jul 4, 2017

@adustm

This comment has been minimized.

Member

adustm commented Jul 6, 2017

hello, I guess the next step is another morph test ?
Kind regards

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jul 6, 2017

@adustm Yep

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented Jul 6, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 742

All builds and test passed!

@adustm

This comment has been minimized.

Member

adustm commented Jul 7, 2017

Hello,
Happy to see the morph test pass at last !
Does anyone know how the morph test has been fixed ?
Kind regards

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jul 10, 2017

Does anyone know how the morph test has been fixed ?

We fixed few targets - real issues with timings mostly, and host tests improvements that exercise those time tests

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jul 13, 2017

@adustm Is there any dependency for this PR? Or ready to go?

@adustm adustm force-pushed the adustm:STM_sha1_f756zg branch to 45dcf59 Jul 25, 2017

@adustm

This comment has been minimized.

Member

adustm commented Jul 25, 2017

Hello @0xc0170

I just rebased this PR on the master. It is ready to go.

Kind regards
Armelle

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jul 25, 2017

Thank you @adustm , we will retrigger the tests

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jul 25, 2017

/morph test-nightly

@mbed-bot

This comment has been minimized.

mbed-bot commented Jul 25, 2017

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 886

Example Build failed!

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jul 26, 2017

/morph test-nightly

@mbed-bot

This comment has been minimized.

mbed-bot commented Jul 28, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 897

All builds and test passed!

@theotherjimmy theotherjimmy merged commit b72a1f6 into ARMmbed:master Jul 31, 2017

4 checks passed

Cam-CI uvisor Build & Test Success
Details
ci/morph-test-nightly Job has completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@adustm adustm deleted the adustm:STM_sha1_f756zg branch Oct 11, 2018

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