-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Temporary disable Crypto HW accelerator on UBLOX_EVK_ODIN_W2 #10088
Conversation
Do we know if STM32F439ZI is also affected? Both boards use the same accelerator. |
Currently no failures except Odin board were reported. |
@avolinski, thank you for your changes. |
@avolinski here is the list of boards on the nightly job, I'm not sure if any additional ones use the STM32F439ZI chipset : |
This approach seems better than removing PSA support altogether, however we could be causing a performance degradation (possibly a big one) if this functionality is used in non-psa scenarios so there may be a bit of risk involved here as well |
starting CI pending reviews |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason that we need to disable sha1 and md5 for AES and attestation?
|
||
#define MBEDTLS_SHA1_ALT | ||
//#define MBEDTLS_SHA1_ALT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Why removing SHA1 & MD5
- Why not add a fix to the H/W driver? did you look at the spec maybe its an easy fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SHA found to be messing the attestation test when working with accelerator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
until the accelerator issue is settled no taking chances for sporadic possible failures
Before such a PR is proposed, could there be an issue in the first place that describes the problem and how to reproduce it ? |
Good point, should have tracking issue. Might be better to remove the lines completely rather to comment them out. Once fixed, the commit would be reverted and they are back. Removing something needs a reason in the commit msg - why is this being removed. "possible issues" is not sufficient explanation for removal. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment above ^^
Test run: SUCCESSSummary: 13 of 13 test jobs passed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last thing, if you can amend your last commit - add reason why is this being removed (what it's described in the issue) or just reference there the issue
As soon as commit msg is updated, will go into CI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems this is a good workaround for an old issue
done |
Test run: FAILEDSummary: 1 of 9 test jobs failed Failed test jobs:
|
looks like Jenkins issue: Loading library mbed-os-ci@master |
CI restarted |
Test run: SUCCESSSummary: 13 of 13 test jobs passed |
Description
Temporary disable work with Crypto HW accelerator on STM32F439xI chipset due to possible issue with it.
Switch to SW only.
Pull request type
Reviewers
@Patater @netanelgonen @NirSonnenschein
Release Notes
for 5.12rc3
Attached full compliance tests log: