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

bn_mul.h: disable MULADDC code for cpu before armv6 #2324

Closed
wants to merge 1 commit into from
Closed

bn_mul.h: disable MULADDC code for cpu before armv6 #2324

wants to merge 1 commit into from

Conversation

vhenriet-sfy
Copy link

The umaal instruction is available for ARM cpu with DSP starting with armv6 thus it does not work on armv5e

Requires Backporting

2.16.0

@hanno-becker hanno-becker added bug tracking needs-review Every commit must be reviewed by at least two team members, component-crypto Crypto primitives and low-level interfaces labels Jan 2, 2019
@RonEld
Copy link
Contributor

RonEld commented Jan 2, 2019

@VHenriet Thank you for your contribution!
Please confirm this is your Mbed account

@ciarmcom
Copy link

ciarmcom commented Jan 2, 2019

ARM Internal Ref: IOTSSL-2696

@vhenriet-sfy
Copy link
Author

@VHenriet Thank you for your contribution!
Please confirm this is your Mbed account

I confirm that it is my account.

hanno-becker
hanno-becker suggested changes Jan 2, 2019
View reviewed changes
Copy link

@hanno-becker hanno-becker left a comment

Choose a reason for hiding this comment

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

The change looks good to me, thanks @VHenriet! Could you please add an entry to the ChangeLog under the Bugfix category?

@hanno-becker hanno-becker added the needs-backports Backports are missing or are pending review and approval. label Jan 3, 2019
@hanno-becker
Copy link

@VHenriet Did you intentionally close the PR?

@vhenriet-sfy
Copy link
Author

no :(
I force pushed my fork to update the changelog within the same commit

@vhenriet-sfy vhenriet-sfy reopened this Jan 3, 2019
RonEld
RonEld reviewed Jan 3, 2019
View reviewed changes
ChangeLog Outdated

Bugfix
* Fix compilation issue on ARM platform before armv6. The `umaal` arm
instruction is only available on ARM cpu with DSP starting with armv6.
Copy link
Contributor

@RonEld RonEld Jan 3, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Please credit yourself in the ChangeLog entry

Suggested change
instruction is only available on ARM cpu with DSP starting with armv6.
instruction is only available on ARM cpu with DSP starting with armv6. Fixed by vhenriet #2324.

Copy link
Contributor

Choose a reason for hiding this comment

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

Unless you prefer to use your real name

Copy link

Choose a reason for hiding this comment

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

@VHenriet Do you have time to adapt the ChangeLog as my colleague @RonEld suggested?

@vhenriet-sfy
bn_mul.h: disable MULADDC code for cpu before armv6
1ce8019 
The umaal instruction is available for ARM cpu with DSP starting with armv6 thus it does not work on armv5e
@andresag01
Copy link
Contributor

@hanno-arm @RonEld: It seems that your review comments were addressed, could you take another look?

@rettichschnidi
Copy link

rettichschnidi commented Jun 13, 2020 •
edited
Loading

I think the problem addressed here has been fixed by PR #2169 and therefore is resolved since the release of 2.16.3.

@gilles-peskine-arm
Copy link
Contributor

Thanks. An equivalent patch was submitted in #2169 and it was merged a while ago.

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

@hanno-becker hanno-becker hanno-becker requested changes

@andresag01 andresag01 andresag01 approved these changes

@RonEld RonEld RonEld approved these changes

Assignees
No one assigned
Labels
bug component-crypto Crypto primitives and low-level interfaces needs-backports Backports are missing or are pending review and approval. needs-review Every commit must be reviewed by at least two team members,
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@vhenriet-sfy @RonEld @ciarmcom @hanno-becker @andresag01 @rettichschnidi @gilles-peskine-arm