wolfssl
wolfssl copied to clipboard
Add RX64/RX71 SHA hardware support
Description
Adding support for SHA1, SHA224, and SHA256 hardware acceleration on Renesas RX64 and RX71 microcontrollers. This follows the a similar pattern as the Renesas TSIP support.
Testing
Tested with WolfSSL 5.6.0 running on a RX64M, verified through debugging that hardware acceleration functions were being called for SHA256 hashes, verified that operation continued as expected after enabling hardware acceleration, verified that several different known values calculated the correct SHA256 hash.
Speed comparison for hashing 64KB of data on a RX64M
| Hash Speed | SHA1 | SHA256 |
|---|---|---|
| Hardware | 4ms | 4ms |
| Software | 11ms | 89ms |
Checklist
- [ ] added tests
- [ ] updated/added doxygen
- [ ] updated appropriate READMEs
- [ ] Updated manual and documentation
Can one of the admins verify this patch?
Hi @TakayukiMatsuo
Please give this a cursory review.
OK to test. No contributor agreement on file yet.
Thank you very much for the work. Please take a look at my comments.
Thanks for the comments, it might be a while before I will have time to make updates
@miyazakh I've made some updates addressing the comments. What would be the next step here?
Hi @Tathorack I add other three comments about copyright. I am assigning @cconlon as a senior engineer review. He will review by himself or assign another senior engineer for final review.
Also, there are three PRB test failures that should be fixed.
Hello @cconlon Assigning this to you as final review. I have left open three comments about copyright. Could you give comments for those too?
Hi @Tathorack I add other three comments about copyright. I am assigning @cconlon as a senior engineer review. He will review by himself or assign another senior engineer for final review.
Also, there are three PRB test failures that should be fixed.
How do I get access to Jenkins to see what the test failures are? I created an account but it says "jhanserh is missing the Overall/Read permission". Otherwise is there a way I can run the tests locally?
Retest this please. Last run was too long ago, so need to rerun to see
Seems like I still cannot access the Jenkins builds
Seems like I still cannot access the Jenkins builds
The failures are because you have not rebased this PR to latest master and some of the commit messages have failed. There is no real failures I can see. Please rebase, squash and force push the PR.
@cconlon or @kaleb-himes will need to look into if we can give you Jenkins permissions?
Rebased and looks like all the checks have passed now
Signed contributor agreement received. Starting tests.
@embhorn @cconlon What are the next steps for getting this merged in?
Hi @Tathorack
The contributor agreement and the verbiage have been accepted. I've restarted the tests.
Thanks, @embhorn
@dgarske Do I need to rebase this again to fix the test failures?
retest this please.
@cconlon @TakayukiMatsuo
Please resume the review on this PR.
@dgarske Do I need to rebase this again to fix the test failures?
Jumping in here sorry. Yes @Tathorack please go ahead and rebase so your changes are on top of commit history, the way our automated build system attempts to checkout the changes in some cases is still having issues replaying the changes on top of master.
Update: Rebase resolved the checkout issues, thank you! I hit retry on the one failing test (some background stuff caused it to timeout prematurely, not a true failure)
@embhorn what's the next steps for this PR?
@cconlon
Reviewers have approved. Please merge when you are ready.
@dgarske I've addressed your comments and rebased on master
Retest this please
Hi @Tathorack , making sure you saw my comments for a few minor cleanups. I'd like to get this merged. Thanks, David Garske, wolfSSL
@dgarske I did see them, working on them now and some final testing. I should have some changes up by the end of the day. I noticed some of the tests failed, anything I need to do there to address them or should the be fixed with a rebase?
Wonderful. Things looks good. The tests for pipeline is a new one we are working on to speed things up. The multi-test is the "C++-style comments". The other had a network failure and when you push will resolve.